Artifact df6787492b4f2cac2d4bc706151d6d451672a91670b3220555b5464927f5487c:
- File
internal/manifest/manifest.go
— part of check-in
[1f59012870]
at
2026-07-23 10:17:46
on branch trunk
— manifest: visit Crosslink candidates in delta-chain order (#126)
* manifest: create forumpost on demand, matching canonical rebuild
Crosslink's candidate query names the forumpost table unconditionally, but canonical fossil creates it lazily -- only once a forum artifact needs it -- and fossil rebuild drops it for a repository that never populated it. Crosslink now recreates the table itself before the sweep runs, matching canonical's on-demand behavior instead of erroring with "no such table: forumpost" against a rebuilt repo.
Audited the rest of the on-demand-in-canonical set (ticket, ticketchng) against the current schema: both are created eagerly by canonical at init and survive rebuild, so they don't share this problem.
Added TestCrosslinkForumPostTableDropped (drops the table directly) and TestCrosslinkAfterFossilRebuild (drives an actual fossil rebuild first) to cover the regression, and updated the existing fossilbin comparison test's comment now that the rebuild-last ordering is no longer required.
* manifest: visit crosslink candidates in delta-chain order
Crosslink swept candidates in ascending rid order, but content_deltify stores a blob's older versions as deltas against its newer ones, so delta chains run the opposite direction: high-rid root down to low-rid ancestors. An ascending sweep's first candidate of a chain therefore materializes the whole chain, and the rest are consumed arbitrarily far in the future -- the working set of a whole-repository sweep was the entire repository expanded (8.2 GiB for the Fossil SCM repository).
Candidates are now topologically sorted so a delta's base is always visited before its dependents (deltaChainOrder), matching Fossil's own rebuild_step shape: root first, each descendant one delta application from an already-expanded predecessor. The working set this sweep needs is now bounded by how many chains are in flight at once, not by repository size.
That reordering also changes the order artifact side effects get written in, since content expansion and artifact crosslinking share one loop. Most of what Crosslink writes turned out to already be a pure function of its own artifact (event/plink/mlink/tagxref-origin rows) and needed no change. Two things generalize across the whole plink graph rather than any one artifact and were genuinely order-sensitive:
- leaf table bookkeeping, which inserted the new checkin and deleted its parent -- correct only when parents are linked before their children, which delta-chain order no longer guarantees. - tag propagation, which ran once per checkin from its immediate parent -- reachable only to children plink already held at that moment.
Both are now full, order-independent repair passes (repairLeafTable, repairTagPropagation) run once after the sweep, mirroring Fossil's own manifest_crosslink_end + leaf_rebuild() shape: defer the order-sensitive work and repair it once rather than trying to preserve order through the sweep. tag.propagate's mtime-gated, plink-live-at-call-time design turned out to already be confluent -- replaying every self-declared tag origin once, in any order, converges on the same tagxref state -- so the repair is just doing that instead of depending on visiting order to get there implicitly. Tech-note event replacement and forum thread titles turned out to already have the same confluent shape and needed no change; left comments explaining why.
Also fixes a tag.PropagateAll crash on any origin with a NULL tagxref value (e.g. CrosslinkCluster's cluster tag), surfaced by the new propagation repair actually reaching those rows for the first time.
Regression coverage: a real three-generation checkin chain, forced into a genuine manifest delta chain, that would have left two of three checkins untagged and all three marked leaf under the old per-checkin propagation and incremental leaf bookkeeping.
* fix: address review nits on crosslink order-sensitivity and delta cycles
- deltaChainOrder now returns an error instead of panicking when the candidate delta graph fails to drain. The delta table is on-disk data that can arrive over sync from an untrusted peer, so a cycle (corrupt or hostile repo) should surface as an error rather than crash the process.
- snapshotDerived's digest now covers tagxref (rid/tagid/tagtype/ srcid/origid/value), and TestFossilBinaryReadsCrosslinkedRepo compares it against canonical fossil rebuild -- closing the gap the test previously called out by name ("tagxref is the notable gap").
- Added TestFossilBinaryReadsCrosslinkedRepoBranchAndMerge, exercising the order-sensitive paths incrementalHistory's single-branch history cannot reach: a feature branch diverging from trunk, and a merge commit whose own branch tag must come from its primary parent, not the merged-in one. event/plink/tagxref match canonical fossil row-for-row on this topology.
leaf and mlink are deliberately excluded from that comparison: the fixture surfaced that both have pre-existing gaps against canonical fossil (repairLeafTable counts any plink edge rather than only primary-parent edges; insertCheckinMlinks writes a row for every F-card rather than only changed ones) that predate this fix and are out of scope for #102/#103.
(user: [email protected], size: 12056) [annotate] [blame] [check-ins using]
package manifest import ( "fmt" "time" "github.com/danmestas/go-libfossil/db" "github.com/danmestas/go-libfossil/internal/blob" "github.com/danmestas/go-libfossil/internal/content" "github.com/danmestas/go-libfossil/internal/deck" libfossil "github.com/danmestas/go-libfossil/internal/fsltype" "github.com/danmestas/go-libfossil/internal/hash" "github.com/danmestas/go-libfossil/internal/repo" "github.com/danmestas/go-libfossil/internal/tag" ) type CheckinOpts struct { Files []File Comment string User string Parent libfossil.FslID MergeParents []libfossil.FslID // additional parents beyond Parent (for merge commits) Delta bool Time time.Time Tags []deck.TagCard } type File struct { Name string Content []byte Perm string } func Checkin(r *repo.Repo, opts CheckinOpts) (manifestRid libfossil.FslID, manifestUUID string, err error) { if r == nil { panic("manifest.Checkin: r must not be nil") } if len(opts.Files) == 0 { panic("manifest.Checkin: opts.Files must not be empty") } if opts.User == "" { panic("manifest.Checkin: opts.User must not be empty") } defer func() { if err == nil && manifestRid <= 0 { panic("manifest.Checkin: manifestRid must be positive on success") } }() if opts.Time.IsZero() { opts.Time = time.Now().UTC() } var inlineTCards []deck.TagCard err = r.WithTx(func(tx *db.Tx) error { fCards, fileRids, txErr := storeFileBlobs(tx, opts.Files) if txErr != nil { return txErr } d, txErr := buildCheckinDeck(tx, opts, fCards) if txErr != nil { return txErr } inlineTCards = d.T // capture for post-tx tag processing manifestRid, manifestUUID, txErr = insertCheckinBlob(tx, d) if txErr != nil { return txErr } if txErr := insertMlinks(tx, opts, manifestRid); txErr != nil { return txErr } // Store the parent check-in manifest as a delta against this one, // the same backwards direction used for file content. Canonical does // this at the end of commit (src/checkin.c:3234, // `content_deltify(vid, &nvid, 1, 0)`). File content is handled by // insertMlinks above; this covers the manifests themselves. if opts.Parent > 0 { if _, txErr := content.Deltify(tx, opts.Parent, manifestRid); txErr != nil { return fmt.Errorf("deltify parent manifest: %w", txErr) } } if txErr := markLeafAndEvent(tx, opts, manifestRid); txErr != nil { return txErr } // Mark file blobs as unsent so sync pushes them. // (unclustered is handled by blob.Store automatically.) for _, frid := range fileRids { if _, err := tx.Exec("INSERT OR IGNORE INTO unsent(rid) VALUES(?)", frid); err != nil { return fmt.Errorf("unsent file: %w", err) } } return nil }) if err != nil { return 0, "", fmt.Errorf("manifest.Checkin: %w", err) } // Process inline T-cards (branch, sym-trunk, etc.) after the transaction // completes so the event/plink tables are populated for tag propagation. for _, tc := range inlineTCards { if tc.UUID != "*" { continue // non-self T-cards are for control artifacts } var tagType int switch tc.Type { case deck.TagPropagating: tagType = tag.TagPropagating case deck.TagSingleton: tagType = tag.TagSingleton case deck.TagCancel: tagType = tag.TagCancel default: continue } if err := tag.ApplyTag(r, tag.ApplyOpts{ TargetRID: manifestRid, SrcRID: manifestRid, TagName: tc.Name, TagType: tagType, Value: tc.Value, MTime: libfossil.TimeToJulian(opts.Time), }); err != nil { return 0, "", fmt.Errorf("manifest.Checkin inline tag %q: %w", tc.Name, err) } } // Inherit propagating tags (branch, sym-*, bgcolor) from the primary parent. // Without this, a new commit on an existing branch would not carry its // parent's branch tag — breaking branch-aware queries. A local commit is // always linked immediately after its parent exists, so calling this // here (rather than deferring to a repair pass) is safe; crosslink's // sync-import path defers the equivalent work to repairTagPropagation // instead, since candidates there are not visited in parent-before-child // order. Both converge on the same tagxref state. if opts.Parent > 0 { if err := tag.PropagateAll(r.DB(), opts.Parent); err != nil { return 0, "", fmt.Errorf("manifest.Checkin propagate from parent: %w", err) } } return manifestRid, manifestUUID, nil } func storeFileBlobs(tx *db.Tx, files []File) ([]deck.FileCard, []libfossil.FslID, error) { fCards := make([]deck.FileCard, len(files)) var rids []libfossil.FslID for i, f := range files { rid, uuid, err := blob.Store(tx, f.Content) if err != nil { return nil, nil, fmt.Errorf("storing file %q: %w", f.Name, err) } fCards[i] = deck.FileCard{Name: f.Name, UUID: uuid, Perm: f.Perm} rids = append(rids, rid) } return fCards, rids, nil } func buildCheckinDeck(tx *db.Tx, opts CheckinOpts, fCards []deck.FileCard) (*deck.Deck, error) { d := &deck.Deck{ Type: deck.Checkin, C: opts.Comment, D: opts.Time, F: fCards, U: deck.User(opts.User), } // Parents: primary parent first, then any merge parents. Fossil renders // this as a space-separated P-card; plink marks i==0 as the primary. if opts.Parent > 0 { var parentUUID string if err := tx.QueryRow("SELECT uuid FROM blob WHERE rid=?", opts.Parent).Scan(&parentUUID); err != nil { return nil, fmt.Errorf("parent uuid: %w", err) } d.P = []string{parentUUID} for _, mp := range opts.MergeParents { if mp <= 0 || mp == opts.Parent { continue } var mpUUID string if err := tx.QueryRow("SELECT uuid FROM blob WHERE rid=?", mp).Scan(&mpUUID); err != nil { return nil, fmt.Errorf("merge parent uuid: %w", err) } d.P = append(d.P, mpUUID) } } // Tags: use custom tags if provided, otherwise default trunk tags for initial checkin if len(opts.Tags) > 0 { d.T = opts.Tags } else if opts.Parent == 0 { d.T = []deck.TagCard{ {Type: deck.TagPropagating, Name: "branch", UUID: "*", Value: "trunk"}, {Type: deck.TagSingleton, Name: "sym-trunk", UUID: "*"}, } } // Delta manifest support if opts.Delta && opts.Parent > 0 { if err := applyDelta(tx, d, fCards, opts.Parent); err != nil { return nil, err } } // R-card (always over full file set) rDeck := &deck.Deck{F: fCards} getContent := func(uuid string) ([]byte, error) { rid, ok := content.AvailableByUUID(tx, uuid) if !ok { return nil, fmt.Errorf("blob not found: %s", uuid) } return content.Expand(tx, rid) } rHash, err := rDeck.ComputeR(getContent) if err != nil { return nil, fmt.Errorf("R-card: %w", err) } d.R = rHash return d, nil } func insertCheckinBlob(tx *db.Tx, d *deck.Deck) (libfossil.FslID, string, error) { manifestBytes, err := d.Marshal() if err != nil { return 0, "", fmt.Errorf("marshal: %w", err) } rid, uuid, err := blob.Store(tx, manifestBytes) if err != nil { return 0, "", fmt.Errorf("store manifest: %w", err) } return rid, uuid, nil } func insertMlinks(tx *db.Tx, opts CheckinOpts, manifestRid libfossil.FslID) error { for _, f := range opts.Files { fnid, err := ensureFilename(tx, f.Name) if err != nil { return fmt.Errorf("filename %q: %w", f.Name, err) } fileUUID := hash.SHA1(f.Content) fileRid, _ := blob.Exists(tx, fileUUID) if err := insertMlinkRow(tx, manifestRid, int64(fileRid), fnid, "", f.Perm, opts.Parent, opts.MergeParents); err != nil { return err } } return nil } func markLeafAndEvent(tx *db.Tx, opts CheckinOpts, manifestRid libfossil.FslID) error { // plink: one row per parent; primary (isprim=1) is opts.Parent, merge // parents are secondary (isprim=0). Matches the sync-path layout in // crosslink.go so merged history looks identical however it arrived. mtime := libfossil.TimeToJulian(opts.Time) allParents := collectParents(opts) for i, pid := range allParents { isPrim := 0 if i == 0 { isPrim = 1 } if _, err := tx.Exec( "INSERT INTO plink(pid, cid, isprim, mtime) VALUES(?, ?, ?, ?)", pid, manifestRid, isPrim, mtime, ); err != nil { return fmt.Errorf("plink: %w", err) } } // event if _, err := tx.Exec( "INSERT INTO event(type, mtime, objid, user, comment) VALUES('ci', ?, ?, ?, ?)", libfossil.TimeToJulian(opts.Time), manifestRid, opts.User, opts.Comment, ); err != nil { return fmt.Errorf("event: %w", err) } // leaf: this checkin is new; remove every parent from leaf so merge // commits correctly supersede both sides. if _, err := tx.Exec("INSERT OR IGNORE INTO leaf(rid) VALUES(?)", manifestRid); err != nil { return fmt.Errorf("leaf insert: %w", err) } for _, pid := range allParents { if _, err := tx.Exec("DELETE FROM leaf WHERE rid=?", pid); err != nil { return fmt.Errorf("leaf delete parent: %w", err) } } // Mark manifest as unsent so sync pushes it (unclustered is handled by blob.Store). if _, err := tx.Exec("INSERT OR IGNORE INTO unsent(rid) VALUES(?)", manifestRid); err != nil { return fmt.Errorf("unsent: %w", err) } return nil } // collectParents returns the primary parent followed by any merge parents, // skipping zero entries and de-duplicating. Used for both the plink insert // loop and the leaf cleanup so both see the same set. func collectParents(opts CheckinOpts) []libfossil.FslID { if opts.Parent <= 0 { return nil } seen := map[libfossil.FslID]bool{opts.Parent: true} out := []libfossil.FslID{opts.Parent} for _, mp := range opts.MergeParents { if mp <= 0 || seen[mp] { continue } seen[mp] = true out = append(out, mp) } return out } func ensureFilename(tx *db.Tx, name string) (int64, error) { if tx == nil { panic("manifest.ensureFilename: tx must not be nil") } if name == "" { panic("manifest.ensureFilename: name must not be empty") } var fnid int64 err := tx.QueryRow("SELECT fnid FROM filename WHERE name=?", name).Scan(&fnid) if err == nil { return fnid, nil } result, err := tx.Exec("INSERT INTO filename(name) VALUES(?)", name) if err != nil { return 0, err } return result.LastInsertId() } func GetManifest(r *repo.Repo, rid libfossil.FslID) (result *deck.Deck, err error) { if r == nil { panic("manifest.GetManifest: r must not be nil") } if rid <= 0 { panic("manifest.GetManifest: rid must be positive") } defer func() { if err == nil && result == nil { panic("manifest.GetManifest: result must not be nil on success") } }() data, err := content.Expand(r.DB(), rid) if err != nil { return nil, fmt.Errorf("manifest.GetManifest: %w", err) } return deck.Parse(data) } func applyDelta(tx *db.Tx, d *deck.Deck, fullFCards []deck.FileCard, parentRid libfossil.FslID) error { parentData, err := content.Expand(tx, parentRid) if err != nil { return fmt.Errorf("expand parent: %w", err) } parentDeck, err := deck.Parse(parentData) if err != nil { return fmt.Errorf("parse parent: %w", err) } baselineUUID := parentDeck.B if baselineUUID == "" { var puuid string tx.QueryRow("SELECT uuid FROM blob WHERE rid=?", parentRid).Scan(&puuid) baselineUUID = puuid } baseRid, ok := content.AvailableByUUID(tx, baselineUUID) if !ok { return fmt.Errorf("baseline %s not found", baselineUUID) } baseData, err := content.Expand(tx, baseRid) if err != nil { return fmt.Errorf("expand baseline: %w", err) } baseDeck, err := deck.Parse(baseData) if err != nil { return fmt.Errorf("parse baseline: %w", err) } baseFiles := make(map[string]string) for _, f := range baseDeck.F { baseFiles[f.Name] = f.UUID } var deltaFCards []deck.FileCard currentFiles := make(map[string]bool) for _, f := range fullFCards { currentFiles[f.Name] = true if baseUUID, exists := baseFiles[f.Name]; !exists || baseUUID != f.UUID { deltaFCards = append(deltaFCards, f) } } for name := range baseFiles { if !currentFiles[name] { deltaFCards = append(deltaFCards, deck.FileCard{Name: name}) } } if len(deltaFCards) < len(fullFCards) { d.B = baselineUUID d.F = deltaFCards } return nil }