Artifact 2d60e48a70781ff2a29817bfb6f6a897ef47e4e7e4235c8511b6bbb85c3aafe9:
- File
internal/manifest/manifest.go
— part of check-in
[2368cad91a]
at
2026-07-19 20:55:48
on branch trunk
— Deltify predecessors on the commit path (#87)
* Deltify predecessors on the commit path
Every locally-created artifact was stored whole. The delta table was in the schema and stayed empty, so a repository grew with the number of commits rather than the amount of change. blob.StoreDelta existed and worked but had no caller on the commit path; the only delta-store in use was blob.StoreDeltaRaw, from the sync receive path, for deltas that arrive pre-encoded. The write path never made a deltify decision at all.
Port canonical Fossil's content_deltify (2.28 src/content.c:852-947) as content.Deltify. Fossil deltifies backwards: a new artifact is always stored whole and its PREDECESSOR is rewritten as a delta against it, so the tip of every chain is full content and reading the newest version of a file stays a single blob load. Chain depth becomes an artifact's age in revisions rather than something that compounds, because an artifact already stored as a delta is never re-deltified.
blob.StoreDelta turned out to be the wrong primitive to wire up: it inserts a new row as a delta, whereas the policy needs to rewrite an existing row. It is left alone. content.Deltify lives in the content package because it needs Expand, which blob cannot import.
The policy is stated once, in content.Deltify, with each rule's canonical line cited: the 50-byte floors, the 75% ratio, the never-redeltify rule that bounds depth, the private-into-public refusal, and the loop break that undeltas the source and then still declines the pairing. Two call sites pass pairs, mirroring canonical's: insertMlinkRow for file content (manifest.c:1562) and Checkin for the parent manifest (checkin.c:3234). insertMlinkRow is shared by the fresh-commit and crosslink paths, so file deltification cannot drift between them.
On a 12-revision, 4-file history: 60 artifacts, 44 delta-encoded (73%), p50 chain depth 4, max 11, and 23,994 content bytes measured as sum(length(content)) -- down from 58,973 with everything whole. A repository produced this way passes fossil 2.28 test-integrity (60/60 blobs, 0 errors) and fossil rebuild, which re-derives every table by walking the chains.
* Bound the deltify ancestor walk, require a transaction, harden tests
Review follow-ups on the deltification policy.
Bound the ancestor walk. It was a bare loop with no cycle guard: a sync peer can shape a cyclic delta graph, because blob.StoreDeltaRaw records delta(rid,srcid) links from the wire without a cycle check, and an unbounded walk would spin inside the write transaction. Guard with the seen-set idiom content.walkDeltaChain already uses, plus a step cap. In the current ordering IsAvailable runs first on both rids and follows the same links under its own depth bound, so Deltify declines before the walk begins -- but that protection is incidental to a different question, so the bound belongs to the loop that needs it. Both behaviours are now pinned: the walk reports a cycle, and Deltify declines an ungrounded source.
Move both size floors ahead of the walk. Canonical returns on a sub-50-byte target (content.c:877-885) before entering the candidate loop, so it never reaches the content_undelta on :902. Running the walk first meant a small target whose successor chained back to it would inflate the source to full content and then decline anyway -- a storage regression and exactly the divergence from canonical's layout the surrounding comment claims to avoid.
Take *db.Tx instead of db.Querier. The rewrite is two statements, and a blob holding delta bytes with no delta row expands as though it were full content: silent corruption rather than a detectable error. Canonical wraps the pair in db_begin_transaction (content.c:938-941); the signature now makes that non-negotiable at compile time. Both call sites already passed a transaction.
Three tests were named for behaviours they did not pin, and each is now verified to fail when the behaviour is reverted:
- The 75% ratio test had no assertion at all; deleting the rule left it passing. It now asserts the pair is declined and stays whole. - The loop-break test could not distinguish declining from proceeding, so the undelta-then-proceed behaviour corrected in the previous commit was untested. It now asserts the target is left whole. - The manifest test checked only "at least one", so a regression from 11-of-12 to 3-of-12 passed. It now checks each index and names the manifest that stopped being encoded.
Also cover the source-side 50-byte floor, which the tiny-artifact test could not reach because it made both artifacts tiny and returned at the target check first, and assert that Undelta restores blob.size while Deltify leaves it alone (content.c:761 vs :935).
Gate the fossil-binary test on REQUIRE_FOSSIL_BIN, set in CI. A skip is invisible without -v, so a run with no fossil binary was byte-identical to a pass -- for the one criterion our own tests cannot substitute for.
Document that blob.StoreDelta is not the commit path's primitive. It inserts a new row as a delta; deltifying rewrites an existing one. It has no non-test callers and the package doc advertised it as the delta-storing entry point.
* Document what the deltify loop-safety argument rests on
Two comments, no behaviour change.
The IsAvailable(srcRid) guard is ours, not canonical's: content.c guards only content_is_available(rid) and then calls content_get(srcid) unguarded. It is also load-bearing beyond phantoms, because it screens out the cyclic delta graphs that would otherwise reach the ancestor walk. Say both, at the guard.
And say at the walk why its seen-set never fires in practice, so that someone reordering the prologue meets the reason rather than concluding the bound is dead code. The note lived only in the test file, which is not where that reader is looking. (user: [email protected], size: 11834) [annotate] [blame] [check-ins using]
package manifest import ( "fmt" "time" "github.com/danmestas/libfossil/db" "github.com/danmestas/libfossil/internal/blob" "github.com/danmestas/libfossil/internal/content" "github.com/danmestas/libfossil/internal/deck" libfossil "github.com/danmestas/libfossil/internal/fsltype" "github.com/danmestas/libfossil/internal/hash" "github.com/danmestas/libfossil/internal/repo" "github.com/danmestas/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. Matches the // corresponding PropagateAll call in crosslink's applyInlineTags so the // fresh-commit and sync-import paths 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 }