Artifact 4ee56ea67a4c5b1a779a16c1127e0b70ad8b1f43c9ee3c27138076c86e3f8f83:
- File
internal/manifest/crosslink_cancel_test.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: 8847) [annotate] [blame] [check-ins using]
package manifest import ( "context" "errors" "fmt" "testing" "time" "github.com/danmestas/go-libfossil/internal/blob" "github.com/danmestas/go-libfossil/internal/deck" ) // pollCancelCtx is a context that reports live for its first livePolls calls to // Done() and cancelled from then on. The sweep polls its context once every // crosslinkCancelCheckStride candidates, so a context that survives the first // poll and dies before the second can only be observed by the *batched* check // -- the i==0 check alone can never see it. That is what makes the test below // discriminate: neuter the stride and this context is never sampled a second // time, so the sweep runs to completion and the test fails. type pollCancelCtx struct { context.Context livePolls int polls int live chan struct{} // never closed dead chan struct{} // closed at construction } func newPollCancelCtx(livePolls int) *pollCancelCtx { dead := make(chan struct{}) close(dead) return &pollCancelCtx{ Context: context.Background(), livePolls: livePolls, live: make(chan struct{}), dead: dead, } } func (c *pollCancelCtx) Done() <-chan struct{} { c.polls++ if c.polls <= c.livePolls { return c.live } return c.dead } func (c *pollCancelCtx) Err() error { if c.polls <= c.livePolls { return nil } return context.Canceled } // TestCrosslinkContextObservesCancellationMidSweep pins that the crosslink // sweep's *batched* cancellation check actually fires. Crosslink is the one // phase of a clone that walks the whole received repository in a single call // with no round boundary to fall back on, so without an in-loop check a clone // deadline cannot interrupt it -- the "ran long past its deadline, never // completed" symptom of #120 at large-repository scale. // // The corpus is deliberately larger than crosslinkCancelCheckStride so the // sweep polls its context more than once, and the context is live for the first // poll (i == 0) and cancelled for the second (i == stride). A sweep that only // checked at i == 0 -- or whose stride were large enough to never sample again // -- would run to completion and return a nil error here. func TestCrosslinkContextObservesCancellationMidSweep(t *testing.T) { r := setupTestRepo(t) // Comfortably more than crosslinkCancelCheckStride, so the sweep polls its // context at least twice. Spelled as a literal rather than derived from the // stride on purpose: a corpus that shrank or grew in lockstep with the // stride could never detect the stride being neutered. const candidates = 768 for i := range candidates { if _, _, err := blob.Store(r.DB(), fmt.Appendf(nil, "candidate blob %d, not a manifest", i)); err != nil { t.Fatalf("blob.Store(%d): %v", i, err) } } ctx := newPollCancelCtx(1) linked, err := CrosslinkContext(ctx, r) if !errors.Is(err, context.Canceled) { t.Fatalf("CrosslinkContext error = %v, want context.Canceled "+ "(the batched in-loop check did not fire; polls=%d)", err, ctx.polls) } if ctx.polls < 2 { t.Errorf("context polled %d time(s); the sweep must sample it more than "+ "once for the batched check to mean anything", ctx.polls) } if linked != 0 { t.Errorf("linked = %d, want 0 (no candidate here is a real manifest)", linked) } } // TestCrosslinkContextObservesPreCancelledContext keeps the cheap i == 0 case // honest: an already-cancelled context aborts before any work is done. func TestCrosslinkContextObservesPreCancelledContext(t *testing.T) { r := setupTestRepo(t) if _, _, err := blob.Store(r.DB(), []byte("candidate blob, not a manifest")); err != nil { t.Fatalf("blob.Store: %v", err) } ctx, cancel := context.WithCancel(context.Background()) cancel() linked, err := CrosslinkContext(ctx, r) if !errors.Is(err, context.Canceled) { t.Fatalf("CrosslinkContext error = %v, want context.Canceled", err) } if linked != 0 { t.Errorf("linked = %d, want 0 on immediate cancellation", linked) } } // TestCrosslinkRepairsLeafAfterCancelledSweep pins issue #143: a sweep that is // cancelled after committing at least one batch must still run the leaf/tag // repair pass before returning, rather than leaving derived leaf/tag state // stale until some later sweep happens to link an artifact. // // Method: build one real root check-in (childless, so it is a leaf) plus enough // filler blobs to push the candidate set past a single batch. leaf is populated // only by the deferred repair pass -- crosslinkCheckinTables never touches it -- // so a check-in that is committed but whose sweep skips the repair never lands // in leaf. Cancel at the second batch's boundary, with the first batch (holding // the check-in) already committed: the only path that returns linked > 0 // together with a context error, since a mid-batch cancellation rolls its batch // back and commits nothing. Then run a second, zero-link sweep and confirm the // repaired leaf state persists across it rather than the stale window reopening. func TestCrosslinkRepairsLeafAfterCancelledSweep(t *testing.T) { r := setupTestRepo(t) d := r.DB() _, fileUUID, err := blob.Store(d, []byte("root file content")) if err != nil { t.Fatalf("blob.Store file: %v", err) } ci := &deck.Deck{ Type: deck.Checkin, C: "root checkin", D: time.Date(2024, 1, 15, 10, 0, 0, 0, time.UTC), U: deck.User("testuser"), F: []deck.FileCard{{Name: "file.txt", UUID: fileUUID}}, R: "0000000000000000000000000000000000000000", // A root check-in declares its branch -- both fossil's own `init` and // this package's Checkin do -- and leaf only ever considers a check-in // that carries a branch tag. See repairLeafTable. T: []deck.TagCard{ {Type: deck.TagPropagating, Name: "branch", UUID: "*", Value: "trunk"}, {Type: deck.TagSingleton, Name: "sym-trunk", UUID: "*"}, }, } ciBytes, err := ci.Marshal() if err != nil { t.Fatalf("Marshal checkin: %v", err) } checkinRid, _, err := blob.Store(d, ciBytes) if err != nil { t.Fatalf("blob.Store checkin: %v", err) } // Pad past one batch so the sweep reaches a second batch boundary. The // check-in and its file blob sit at the front by rid, well inside batch 0. for i := 0; i < crosslinkBatchSize-1; i++ { if _, _, err := blob.Store(d, fmt.Appendf(nil, "filler %d not a manifest", i)); err != nil { t.Fatalf("blob.Store filler %d: %v", i, err) } } // Stay live through the whole first batch, die at the second batch's // boundary poll: one boundary poll before batch 0, then one in-batch poll // per stride within it. Derived from the constants so it tracks any // retuning of either the batch size or the stride. inBatchPolls := (crosslinkBatchSize + crosslinkCancelCheckStride - 1) / crosslinkCancelCheckStride ctx := newPollCancelCtx(1 + inBatchPolls) linked, err := CrosslinkContext(ctx, r) if !errors.Is(err, context.Canceled) { t.Fatalf("CrosslinkContext error = %v, want context.Canceled (polls=%d)", err, ctx.polls) } if linked < 1 { t.Fatalf("linked = %d, want >= 1 (the first batch commits the check-in before cancellation)", linked) } leafCount := func(when string) int { var n int if scanErr := d.QueryRow("SELECT count(*) FROM leaf WHERE rid=?", checkinRid).Scan(&n); scanErr != nil { t.Fatalf("leaf query %s: %v", when, scanErr) } return n } // The fix: the cancelled sweep still ran the leaf repair, so the childless // check-in is recorded as a leaf despite the interruption. if got := leafCount("after cancelled sweep"); got != 1 { t.Fatalf("leaf rows for rid=%d after cancelled sweep = %d, want 1 "+ "(the cancelled sweep skipped the leaf repair, issue #143)", checkinRid, got) } // A later sweep whose remaining candidates all link zero artifacts skips // the repair gate itself; the state repaired above must already be correct // and simply persist, rather than staying stale until a future sweep links // something. n, err := Crosslink(r) if err != nil { t.Fatalf("second Crosslink: %v", err) } if n != 0 { t.Fatalf("second Crosslink linked = %d, want 0 (no remaining candidate is a manifest)", n) } if got := leafCount("after following zero-link sweep"); got != 1 { t.Fatalf("leaf rows for rid=%d after zero-link sweep = %d, want 1 "+ "(stale leaf state persisted across the zero-link sweep)", checkinRid, got) } } // TestCrosslinkStillWorksWithoutContext keeps the historical Crosslink entry // point honest: it must behave exactly as before, supplying its own background // context, so the ~30 existing callers need no change. func TestCrosslinkStillWorksWithoutContext(t *testing.T) { r := setupTestRepo(t) if _, _, err := blob.Store(r.DB(), []byte("candidate blob, not a manifest")); err != nil { t.Fatalf("blob.Store: %v", err) } if _, err := Crosslink(r); err != nil { t.Fatalf("Crosslink: %v", err) } }