Artifact 3db4f84de57611238b6adf45edf185a6c9a734fcf317ad4cbb0920ca6e08730d:
- File
internal/deck/parse.go
— part of check-in
[5e6ef35173]
at
2026-07-28 02:35:56
on branch trunk
— fix: derive mlink, plink, tagxref and leaf the way canonical fossil does (#193) (#211)
A full clone of the Fossil self-hosting repository disagreed with what canonical fossil derives from the SAME artifacts: mlink -3,351 rows, tagxref -632, plink -5, leaf -1. `fossil rebuild` on our own clone put every one of them back, so the artifact set was complete and the fault was entirely in crosslink-time derivation.
Six independent causes, each verified against `fossil rebuild` on a 67.6k-blob corpus clone:
mlink, -3,351. An mlink row records a parent-to-child TRANSITION, and a merge check-in has more than one parent. Canonical runs the same file-level diff once per parent -- primary, then each merge parent, then each cherrypick source -- and flags the extra rows isaux=1; we only ever diffed against the primary parent. add_mlink is now ported whole (internal/manifest/addmlink.go), which also brings the two passes the old code approximated: the delta-vs-delta revert/resurrect pass, and the baseline-child deletion pass with its mperm=0 delete rows. "Added by merge" (pid=-1) follows canonical's rule too -- a count over the rows a file produced, not a name lookup in the merge parents. A parent whose manifest cannot be read now yields NO rows rather than a whole tree of bogus "added" rows, and the rows are derived later from the parent's side once it arrives (linkPlinkChildren), so the result no longer depends on the order the sweep visits artifacts in.
tagxref, -612 of -632. A check-in's T-card does not have to name the check-in carrying it: `fossil merge --integrate` records the branch it closed as `T +closed <hash>` on the merge check-in itself. Canonical applies a check-in's, a control artifact's and a technote's T-cards through one code path; we applied only the self-referential ones on check-ins and none at all on technotes.
tagxref, the rest. A wiki-
or event-<id> tag's value is the W-card body measured after LEADING whitespace is skipped, not the raw card length.
Artifact identification. The sweep parses every blob in the repository, so what counts as an artifact decides which rows exist at all. Three gaps: a bare card letter with no arguments is legal (fossil's own first check-in spells its parentless-ness as a bare "P" card) and rejecting it dropped that check-in entirely; an N-card after a P-card is legal in a technote for historical compatibility, and rejecting it dropped real technote revisions with their event-<id> and bgcolor tags; and the check-in-versus-control decision turns on a comment, files or parents, which made `merge --integrate` check-ins read as control artifacts. Fossil's required-card table now gates linking (isArtifact), on both the sweep and the dephantomize cascade, so a file whose first line parses as a Z-card no longer links as a dateless check-in and a wiki revision missing its U-card no longer links at all.
plink, -5. The ancestry a check-in declares is a property of the check-in, not of what happens to be in the repository when it is crosslinked. A parent hash with no blob yet now reserves a phantom the way canonical's uuid_to_rid does, so the edge exists whether the parent arrives in a later round or never; skipping it lost the edge for good, because nothing revisits an already-linked artifact.
leaf, -1. Nothing calls leaf_rebuild() outside `fossil leaves --recompute`; the table is maintained by leaf_check() over the rids leaf_eventually_check() schedules -- each rid a branch tag lands on, plus that rid's parents. Drawing candidates from event instead both missed phantom parents (which fossil does list) and added check-ins that carry no branch tag and no ancestry (which it does not).
After: a fresh corpus clone and `fossil rebuild` of that clone agree row for row on mlink, plink, tagxref, leaf AND event -- 104,919 / 23,741 / 51,766 / 1,426 / 27,099 either way, and `fossil test-integrity` reports 0 errors.
Not addressed here: the local commit path (manifest.insertMlinks) still writes one mlink row per file rather than per change, and does not record merge-parent transitions. That is a separate divergence from the clone parity this issue is about.
Co-authored-by: Dan Mestas <[email protected]> (user: [email protected], size: 14142) [annotate] [blame] [check-ins using]
package deck import ( "bytes" "fmt" "strconv" "strings" "time" "github.com/danmestas/go-libfossil/internal/hash" ) // isHexToken reports whether s is non-empty and consists solely of the // lowercase hex digits an artifact hash is built from. §4.7.16 rejects a // T-card name that is entirely hex because it is indistinguishable from a // hash target. func isHexToken(s string) bool { if s == "" { return false } for i := 0; i < len(s); i++ { c := s[i] isHexDigit := (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') if !isHexDigit { return false } } return true } func Parse(data []byte) (*Deck, error) { if data == nil { panic("deck.Parse: data must not be nil") } // A clearsigned blob carries PGP/SSH framing around the manifest; strip // it once so both the Z-card check and card parsing see the inner content // (the artifact's content-addressed hash, taken over the raw blob // elsewhere, is unaffected). stripClearsign returns data unchanged for the // common non-clearsigned case. content := stripClearsign(data) if err := verifyZ(content); err != nil { return nil, fmt.Errorf("deck.Parse: %w", err) } // body is safe: verifyZ above guarantees len(content) >= 35. body := content[:len(content)-35] d := &Deck{} var lastCard byte var seen seenCards reader := bytes.NewReader(body) for reader.Len() > 0 { line, err := readLine(reader) if err != nil || len(line) == 0 { continue } card := line[0] // §4.5.1 occurrence classes. The card-type ordering check below // permits equal consecutive letters, so the duplicate guard for // single-occurrence types has to be separate from it. W is // single-occurrence too, so the claim precedes the W branch. if err := seen.claim(card); err != nil { return nil, fmt.Errorf("deck.Parse: %w", err) } if card == 'W' { if card < lastCard { return nil, fmt.Errorf("deck.Parse: card 'W' out of order (after '%c')", lastCard) } lastCard = card sizeStr := strings.TrimSpace(line[2:]) size, err := strconv.Atoi(sizeStr) if err != nil { return nil, fmt.Errorf("deck.Parse: bad W size: %w", err) } content := make([]byte, size) n, readErr := reader.Read(content) if readErr != nil && n != size { return nil, fmt.Errorf("deck.Parse: W content read: %w", readErr) } if n != size { return nil, fmt.Errorf("deck.Parse: W content: got %d, want %d", n, size) } reader.ReadByte() // trailing newline d.W = content continue } if card < lastCard && !isLegacyTechnoteNAfterP(d, card, lastCard) { return nil, fmt.Errorf("deck.Parse: card '%c' out of order (after '%c')", card, lastCard) } lastCard = card // A card letter on a line by itself carries no arguments. That is // legal, not malformed: the repeatable cards accept an empty argument // list, and Fossil's own first check-in // (a28c83647dfa805f05f3204a7e146eb1f0d90505, "initial empty baseline") // spells its parentless-ness as a bare "P" card. Rejecting the line // dropped that check-in from crosslink entirely. Cards that do require // arguments still reject the empty string in their own parser. if len(line) > 1 && line[1] != ' ' { return nil, fmt.Errorf("deck.Parse: malformed: %q", line) } var args string if len(line) > 2 { args = line[2:] } if err := parseCard(d, card, args); err != nil { return nil, fmt.Errorf("deck.Parse: %w", err) } } d.Type = inferType(d) return d, nil } // isLegacyTechnoteNAfterP reports whether an N-card following a P-card is the // one out-of-order sequence Fossil accepts. // // Cards are otherwise required to be in lexicographical order, but a technote // generator bug shipped before 2021-02-10 emitted the P-card ahead of the // N-card, and the ordering check that would have caught it was itself broken // until the same date. Fossil keeps accepting those artifacts for historical // compatibility (src/manifest.c, tickets 15d04de574383d61 and 5e67a7f4041a36ad) // and so must this parser: rejecting them dropped real technote revisions -- // along with their event-<id> and bgcolor tags -- out of crosslink (issue #193). // // The exception is narrow on purpose: only N after P, and only once the E-card // that makes the artifact a technote has already been seen. func isLegacyTechnoteNAfterP(d *Deck, card, lastCard byte) bool { return card == 'N' && lastCard == 'P' && d.E != nil } func readLine(r *bytes.Reader) (string, error) { var b strings.Builder for { c, err := r.ReadByte() if err != nil { return b.String(), nil } if c == '\n' { return b.String(), nil } b.WriteByte(c) } } func parseCard(d *Deck, card byte, args string) error { if d == nil { panic("deck.parseCard: d must not be nil") } switch card { case 'A': return parseACard(d, args) case 'D': return parseDCard(d, args) case 'E': return parseECard(d, args) case 'F': return parseFCard(d, args) case 'J': return parseJCard(d, args) case 'Q': return parseQCard(d, args) case 'T': return parseTCard(d, args) // Simple cards stay inline: case 'B': d.B = strings.TrimSpace(args) return nil case 'C': d.C = FossilDecode(args) return nil case 'G': d.G = strings.TrimSpace(args) return nil case 'H': d.H = FossilDecode(args) return nil case 'I': d.I = strings.TrimSpace(args) return nil case 'K': d.K = strings.TrimSpace(args) return nil case 'L': d.L = FossilDecode(args) return nil case 'M': return parseMCard(d, args) case 'N': d.N = strings.TrimSpace(args) return nil case 'P': // Repeatable, and neither sorted nor deduplicated (§4.5.2): P // order is semantic, primary parent first. Appending — not // assigning — is what makes a second P card add parents rather // than replace them (§4.5.1). d.P = append(d.P, strings.Fields(args)...) return nil case 'R': d.R = strings.TrimSpace(args) return nil case 'U': return parseUCard(d, args) case 'Z': return parseZCard(args) default: return fmt.Errorf("unknown card '%c'", card) } } func parseACard(d *Deck, args string) error { parts := strings.SplitN(args, " ", 3) if len(parts) < 2 { return fmt.Errorf("A-card needs 2+ fields") } ac := &AttachmentCard{Filename: FossilDecode(parts[0]), Target: parts[1]} if len(parts) == 3 { ac.Source = parts[2] } d.A = ac return nil } func parseDCard(d *Deck, args string) error { t, err := parseTimestamp(args) if err != nil { return fmt.Errorf("D-card: %w", err) } d.D = t return nil } func parseECard(d *Deck, args string) error { parts := strings.SplitN(args, " ", 2) if len(parts) != 2 { return fmt.Errorf("E-card needs datetime and uuid") } t, err := parseTimestamp(parts[0]) if err != nil { return fmt.Errorf("E-card: %w", err) } d.E = &EventCard{Date: t, UUID: parts[1]} return nil } func parseFCard(d *Deck, args string) error { parts := strings.Fields(args) if len(parts) == 0 { return fmt.Errorf("empty F-card") } fc := FileCard{Name: FossilDecode(parts[0])} if len(parts) >= 2 { fc.UUID = parts[1] } if len(parts) >= 3 { fc.Perm = parts[2] } if len(parts) >= 4 { fc.OldName = FossilDecode(parts[3]) } // §4.5.2: strictly ascending by decoded filename. Decoding has already // happened above, so the check sees the post-decode name the merge walk // and baseline binary search of §8.2 will later rely on. if n := len(d.F); n > 0 { if err := requireAscending('F', d.F[n-1].Name, fc.Name); err != nil { return err } } d.F = append(d.F, fc) return nil } func parseMCard(d *Deck, args string) error { hash := strings.TrimSpace(args) // §4.5.2: strictly ascending by hash. if n := len(d.M); n > 0 { if err := requireAscending('M', d.M[n-1], hash); err != nil { return err } } d.M = append(d.M, hash) return nil } func parseJCard(d *Deck, args string) error { parts := strings.SplitN(args, " ", 2) // §4.7.8: the value is escape-decoded, the field name is stored // verbatim (canonical manifest.c defossilizes only zValue and compares // the raw zName). jf := TicketField{Name: parts[0]} if len(parts) == 2 { jf.Value = FossilDecode(parts[1]) } // §4.5.2: strictly ascending by field name. if n := len(d.J); n > 0 { if err := requireAscending('J', d.J[n-1].Name, jf.Name); err != nil { return err } } d.J = append(d.J, jf) return nil } func parseQCard(d *Deck, args string) error { if len(args) < 2 { return fmt.Errorf("Q-card too short") } cp := CherryPick{IsBackout: args[0] == '-'} rest := args[1:] parts := strings.SplitN(rest, " ", 2) cp.Target = parts[0] if len(parts) == 2 { cp.Baseline = parts[1] } d.Q = append(d.Q, cp) return nil } func parseTCard(d *Deck, args string) error { if len(args) < 2 { return fmt.Errorf("T-card too short") } // §4.7.16: the first character of the name token MUST be +, - or *. // Rejecting anything else also keeps the sign a single byte, so the // ordering key assembled from it cannot silently gain a second byte // through UTF-8 expansion. switch TagType(args[0]) { case TagSingleton, TagPropagating, TagCancel: default: return fmt.Errorf("T-card name must begin with '+', '-' or '*', got %q", args[:1]) } tc := TagCard{Type: TagType(args[0])} parts := strings.SplitN(args[1:], " ", 3) if len(parts) < 2 { return fmt.Errorf("T-card needs name and uuid") } // §4.7.16: the name and value tokens are escape-decoded (canonical // defossilizes both), while the target token is stored verbatim -- never // decoded on the way in, so it is never encoded on the way out. Decoding // the value matters on ordinary input: `fossil branch new 'my branch'` // writes `T *branch * my\sbranch`, and leaving that raw names the branch // literally "my\sbranch" all the way through crosslink. tc.Name = FossilDecode(parts[0]) tc.UUID = parts[1] if len(parts) == 3 { tc.Value = FossilDecode(parts[2]) } // §4.7.16: a name that after the sign is entirely hex digits is // ambiguous with a hash, so reject it. The escape-decoded name is used; // an all-hex token carries no escapes, so decoding leaves it unchanged. if isHexToken(tc.Name) { return fmt.Errorf("T-card name %q must not be entirely hexadecimal", tc.Name) } // §4.7.16: the target must be a valid artifact hash or the literal '*'. // hash.IsValidHash panics on an empty string, so the '*' and non-empty // checks come first. validTarget := tc.UUID == "*" if !validTarget && tc.UUID != "" { validTarget = hash.IsValidHash(tc.UUID) } if !validTarget { return fmt.Errorf("T-card target %q is neither a valid artifact hash nor '*'", tc.UUID) } if n := len(d.T); n > 0 { if err := requireTagAscending(d.T[n-1], tc); err != nil { return err } } d.T = append(d.T, tc) return nil } // requireTagAscending enforces the two-level T rule of §4.5.2: ascending by // tag name, then by target hash, with equal names requiring a strictly // increasing hash. Per §4.7.16 the primary key is the decoded full name // token including its leading sign character; tagOrderKey builds it. func requireTagAscending(previous, current TagCard) error { previousName := tagOrderKey(previous) currentName := tagOrderKey(current) switch cmp := Compare(currentName, previousName); { case cmp > 0: return nil case cmp < 0: return fmt.Errorf("T-card %q out of order after %q", currentName, previousName) default: if Compare(current.UUID, previous.UUID) > 0 { return nil } return fmt.Errorf("T-card %q target %q out of order after %q", currentName, current.UUID, previous.UUID) } } // parseUCard resolves the U-card's decoded value the same way fossil's // own parser does (src/manifest.c:1008-1016): a present-but-empty U-card // becomes the literal "anonymous". A wholly absent U-card never calls // this function at all, so d.U stays nil — the third state a bare string // field cannot represent. Resolving here, once, means every downstream // crosslink call site can bind d.U directly and get the right SQL value // (NULL, "anonymous", or the login) without repeating this logic. // // args is trimmed before the emptiness check, matching canonical fossil's // next_token() (which skips leading whitespace) and every sibling card // parser in this file (B/G/I/K/M/N/R all TrimSpace); otherwise a // whitespace-only U-card ("U \n") would store literal whitespace instead // of resolving to "anonymous". func parseUCard(d *Deck, args string) error { user := FossilDecode(strings.TrimSpace(args)) if user == "" { user = "anonymous" } d.U = &user return nil } func parseTimestamp(s string) (time.Time, error) { s = strings.TrimSpace(s) for _, layout := range []string{ "2006-01-02T15:04:05.000", "2006-01-02T15:04:05", } { if t, err := time.Parse(layout, s); err == nil { return t, nil } } s = strings.Replace(s, "t", "T", 1) for _, layout := range []string{ "2006-01-02T15:04:05.000", "2006-01-02T15:04:05", } { if t, err := time.Parse(layout, s); err == nil { return t, nil } } return time.Time{}, fmt.Errorf("cannot parse timestamp %q", s) } func inferType(d *Deck) ArtifactType { switch { case len(d.M) > 0: return Cluster case d.G != "" || d.H != "" || d.I != "": return ForumPost case d.A != nil: return Attachment case d.K != "": return Ticket case d.L != "": return Wiki case d.E != nil: return Event // Canonical Fossil settles the remaining ambiguity -- check-in versus // control artifact, both of which are D/T/U/Z-shaped -- on whether the // artifact describes a tree position: a comment, files, or parents // (src/manifest.c, "If the artifact type has not yet been determined"). // The C-card is the part that is easy to miss, and it matters: `fossil // merge --integrate` writes a check-in that has a comment and a `T +closed` // card but no F-cards and no P-card, and reading those as control // artifacts lost their tags (issue #193). The R-card is not part of // canonical's test but implies a file tree, so it stays here. case d.C != "" || len(d.F) > 0 || len(d.P) > 0 || d.R != "": return Checkin case len(d.T) > 0: return Control default: return Checkin } }