Artifact e96e04e7124f0c042287dd82048c210ffb47daf833b723b8e135ce61abc878f9:
- File
internal/checkout/checkin.go
— part of check-in
[a317896683]
at
2026-10-03 16:16:26
on branch trunk
— fix(checkout): refuse to load a version with missing content (#231) (#240)
* fix(checkout): refuse to load a version with missing content (#231)
LoadVFile wrote a vfile row with rid=0 for each file whose content the repo does not hold (a phantom, or a delta on one), as a partial sync leaves it. In fossil's vfile rid=0 means "added", so those rows were indistinguishable from pending adds, and Extract then panicked in content.Expand(0) trying to write them.
LoadVFile now does what fossil's checkout does: it resolves every file first and refuses the version, naming up to ten missing files and wrapping errMissingContent, before changing vfile at all. It no longer returns a missing count, which no caller used.
Create is the one caller that tolerates the refusal. A tip that a partial sync has not finished is no reason to fail CreateCheckout: the caller may be about to extract an older, complete version (EdgeSync's ExtractTo). vfile then stays empty instead of claiming files the repo cannot produce. Create's environment defaulting, shared with Open, moves to resolveEnv, and the initial load to loadInitialVFile.
Tests cover the refusal (vfile untouched, files named), Extract leaving the checkout as it was, Create on a partial tip followed by Extract of a complete version, and parity with the fossil binary, which refuses the same repository with "missing content".
* fix(checkout): refuse missing content in Update before writing (#231)
Update wrote changed files to disk and only then reloaded vfile, which now refuses a version with missing content. A target sharing a missing file with the current version (its content is never read, because it does not change) could therefore leave disk on the target while vvar and vfile stayed on the old version. Like fossil's update, Update now resolves the target's content before writing anything, through resolveFiles, the resolution step split out of LoadVFile.
Update moves its target selection into updateTarget, reuses Extract's checkinTime, and builds its result through sortedUpdateResult, bringing it under 70 lines. Create now reports a tip it could not load to the observer instead of tolerating it silently.
* fix(checkout): report Update refusals to observers; tighten checks (#231)
The missing-content check in Update moves after ExtractStarted, so a refused update still reaches observers as an ExtractCompleted carrying the error, as other Update failures do. A negative target from the public UpdateOpts is an error, not a panic. resolveFiles errors carry their own prefix instead of LoadVFile's.
Adds the entry and exit assertions the new helpers were missing, a compile-time check on missingNamesMax, and wraps lines over 100 columns; tests check Version's error instead of discarding it. (user: [email protected], size: 18931) [annotate] [blame] [check-ins using]
package checkout import ( "context" "database/sql" "errors" "fmt" "os" "strconv" "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/manifest" ) // Enqueue adds files to the commit staging queue. If the queue is empty (nil), // all changed files are implicitly enqueued. Once Enqueue is called, only // explicitly enqueued files will be committed. func (c *Checkout) Enqueue(opts EnqueueOpts) error { if c == nil { panic("checkout.Enqueue: nil *Checkout") } if c.checkinQueue == nil { c.checkinQueue = make(map[string]bool) } for _, p := range opts.Paths { c.checkinQueue[p] = true if opts.Callback != nil { if err := opts.Callback(p); err != nil { return fmt.Errorf("checkout.Enqueue: callback for %s: %w", p, err) } } } return nil } // Dequeue removes files from the commit staging queue. If opts.Paths is empty, // clears the entire queue (restoring implicit all-files behavior). // Returns error for API consistency / future-proofing. func (c *Checkout) Dequeue(opts DequeueOpts) error { if c == nil { panic("checkout.Dequeue: nil *Checkout") } if len(opts.Paths) == 0 { c.checkinQueue = nil // dequeue all return nil } for _, p := range opts.Paths { delete(c.checkinQueue, p) } return nil } // IsEnqueued returns true if the named file will be included in the next commit. // If the queue is nil (never initialized), all changed files are implicitly enqueued. // If the queue exists but is empty (len == 0), nothing is enqueued. // Returns error for API consistency / future-proofing. func (c *Checkout) IsEnqueued(name string) (bool, error) { if c == nil { panic("checkout.IsEnqueued: nil *Checkout") } if c.checkinQueue == nil { return true, nil // nil queue = all changed files implicitly enqueued } return c.checkinQueue[name], nil } // DiscardQueue clears the commit staging queue, restoring implicit all-files behavior. // Returns error for API consistency / future-proofing. func (c *Checkout) DiscardQueue() error { if c == nil { panic("checkout.DiscardQueue: nil *Checkout") } c.checkinQueue = nil return nil } // vfileEntry holds a single vfile row for commit processing. type vfileCommitEntry struct { pathname string origname string // prior pathname if renamed, else empty changed bool deleted bool rid int64 isexe bool } // collectVFileEntries queries vfile for all entries of the given version and // classifies them into changed, deleted, and all-entries map. func (c *Checkout) collectVFileEntries(vid libfossil.FslID) ( entries map[string]vfileCommitEntry, changedFiles []string, deletedFiles []string, err error, ) { rows, err := c.db.Query(` SELECT pathname, origname, CAST(chnged AS INTEGER), CAST(deleted AS INTEGER), rid, CAST(isexe AS INTEGER) FROM vfile WHERE vid = ? `, int64(vid)) if err != nil { return nil, nil, nil, fmt.Errorf("checkout.Commit: query vfile: %w", err) } defer rows.Close() entries = make(map[string]vfileCommitEntry) for rows.Next() { var pathname string var origname sql.NullString var chnged, deleted, isexe int var rid sql.NullInt64 if err := rows.Scan(&pathname, &origname, &chnged, &deleted, &rid, &isexe); err != nil { return nil, nil, nil, fmt.Errorf("checkout.Commit: scan vfile: %w", err) } // A rename is only a rename when origname differs from the current // pathname; canonical Fossil stores origname==pathname for unmodified // files (checkin.c:161), so collapse that case to "no rename" here. orig := "" if origname.Valid && origname.String != pathname { orig = origname.String } entries[pathname] = vfileCommitEntry{ pathname: pathname, origname: orig, changed: chnged > 0, deleted: deleted > 0, rid: rid.Int64, isexe: isexe > 0, } if deleted > 0 { deletedFiles = append(deletedFiles, pathname) } else if chnged > 0 || rid.Int64 == 0 { // rid=0 means newly added (never committed) — always treat // as changed even if ScanChanges reset chnged to 0. changedFiles = append(changedFiles, pathname) } } if err := rows.Err(); err != nil { return nil, nil, nil, fmt.Errorf("checkout.Commit: vfile rows: %w", err) } return entries, changedFiles, deletedFiles, nil } // buildCommitFiles constructs the complete file list for a new checkin by // starting from the parent manifest, applying vfile changes (modified/deleted), // and loading unchanged file content from the repo. func (c *Checkout) buildCommitFiles( parentRID libfossil.FslID, vfEntries map[string]vfileCommitEntry, changedFiles []string, deletedFiles []string, shouldInclude func(string) bool, ) ([]manifest.File, error) { var parentFiles []manifest.FileEntry if parentRID != 0 { var err error parentFiles, err = manifest.ListFiles(c.repo, parentRID) if err != nil { return nil, fmt.Errorf("checkout.Commit: list parent files: %w", err) } } // Build a map of parent files (name → FileEntry for O(1) lookup) parentFileMap := make(map[string]manifest.FileEntry, len(parentFiles)) for _, pf := range parentFiles { parentFileMap[pf.Name] = pf } // Start from parent file set fileMap := make(map[string]manifest.File, len(parentFiles)) for _, pf := range parentFiles { fileMap[pf.Name] = manifest.File{ Name: pf.Name, Perm: pf.Perm, } } // 1. Remove deleted files for _, name := range deletedFiles { if shouldInclude(name) { delete(fileMap, name) } } // 2. Update changed files with new content from disk loadedFromDisk := make(map[string]bool, len(changedFiles)) for _, name := range changedFiles { if !shouldInclude(name) { continue } fullPath, err := c.safePath(name) if err != nil { return nil, fmt.Errorf("checkout.Commit: path traversal in %s: %w", name, err) } fileData, err := c.env.Storage.ReadFile(fullPath) if err != nil { return nil, fmt.Errorf("checkout.Commit: read %s: %w", name, err) } perm := "" if existing, ok := fileMap[name]; ok { perm = existing.Perm } // The executable marker is re-derived from the live on-disk mode by // restatCommitPerms below, for every included non-symlink file (not // only ones change scanning flagged) — so permission is decided // later, from the filesystem, rather than from vfile.isexe here. fileMap[name] = manifest.File{ Name: name, Content: fileData, Perm: perm, } loadedFromDisk[name] = true } // 2b. Apply renames: retire each renamed file's old pathname and record // the prior name on the surviving new-name entry, so the file is emitted // exactly once as a rename F-card rather than twice. c.applyRenames(fileMap, vfEntries, shouldInclude) // 3. For unchanged files, load content from the repo. // // loadedFromDisk (not len(entry.Content) > 0) is the sentinel for // "already handled in step 2" — a legitimately empty file's Content is // indistinguishable from "not yet loaded" by length alone, and using // length here mistook a newly-added zero-length file for one still // needing a blob lookup it doesn't have (issue #68). for name, entry := range fileMap { if loadedFromDisk[name] { continue } // A tracked file this commit is scoped to (shouldInclude) but that // is missing from disk cannot be silently carried forward with its // stale content — that would record a commit claiming content that // no longer exists (issue #79). A file outside this commit's scope // (relevant only for an explicit Enqueue) is left untouched exactly // as before: canonical Fossil only fails on a missing file that is // actually part of the commit being made. // // Only files with a live vfile row are checked — matching // restatCommitPerms and canonical Fossil's own vfile-driven manifest // loop. A name present only in the parent manifest (no vfEntries // row) isn't tracked by this checkout's vfile at all; there is // nothing to have "gone missing" from a local scan that never // covered it. if _, hasVFileRow := vfEntries[name]; hasVFileRow && shouldInclude(name) { fullPath, err := c.safePath(name) if err != nil { return nil, fmt.Errorf("checkout.Commit: path traversal in %s: %w", name, err) } if _, err := c.env.Storage.Stat(fullPath); err != nil { if errors.Is(err, os.ErrNotExist) { return nil, fmt.Errorf( "checkout.Commit: tracked file missing from disk: %s", name, ) } return nil, fmt.Errorf("checkout.Commit: stat %s: %w", name, err) } } var fileRID int64 if ve, ok := vfEntries[name]; ok { fileRID = ve.rid } else if pf, ok := parentFileMap[name]; ok { // File is in parent but not in vfile — look up RID by UUID var rid int64 err := c.repo.DB().QueryRow("SELECT rid FROM blob WHERE uuid = ?", pf.UUID).Scan(&rid) if err != nil { return nil, fmt.Errorf("checkout.Commit: resolve RID for %s: %w", name, err) } fileRID = rid } if fileRID == 0 { return nil, fmt.Errorf("checkout.Commit: no RID for unchanged file %s", name) } fileData, err := content.Expand(c.repo.DB(), libfossil.FslID(fileRID)) if err != nil { return nil, fmt.Errorf("checkout.Commit: expand %s: %w", name, err) } fileMap[name] = manifest.File{ Name: name, Content: fileData, Perm: entry.Perm, OldName: entry.OldName, } } // 4. Independently re-stat every included file's on-disk executable bit, // belt-and-braces, regardless of whether change scanning flagged it. if err := c.restatCommitPerms(fileMap, vfEntries, shouldInclude); err != nil { return nil, err } // Convert to slice commitFiles := make([]manifest.File, 0, len(fileMap)) for _, f := range fileMap { commitFiles = append(commitFiles, f) } return commitFiles, nil } // applyRenames rewrites fileMap so each renamed file is emitted exactly once, // under its new name, carrying its prior name for the rename F-card. // // It is driven off vfile.origname rather than the changed-files list because a // pure rename (identical content) is not flagged as changed by ScanChanges — // so without this pass the new name would never enter fileMap and the old name // would carry forward from the parent manifest, double-emitting the content // (the #51 defect). This mirrors canonical Fossil, whose manifest query // selects rows where pathname!=origname independent of the chnged flag // (src/checkin.c:1939-1944). // // The retired old name is deleted from fileMap unless it has independently // reappeared as its own live vfile pathname (a rename that reuses a freed // name), in which case that entry is left for the normal passes to handle. // // A renamed row that is also marked deleted retires both names and emits // nothing, matching canonical Fossil's exclusion of deleted rows from the // manifest query. func (c *Checkout) applyRenames( fileMap map[string]manifest.File, vfEntries map[string]vfileCommitEntry, shouldInclude func(string) bool, ) { if fileMap == nil { panic("checkout.applyRenames: fileMap must not be nil") } if shouldInclude == nil { panic("checkout.applyRenames: shouldInclude must not be nil") } for name, ve := range vfEntries { if ve.origname == "" || ve.origname == name { continue } if !shouldInclude(name) { continue } // A renamed file that was subsequently deleted must not be emitted at // all — under either name. Canonical Fossil's manifest query excludes // such rows outright (WHERE NOT deleted OR NOT is_selected), and the // deleted-files pass above cannot retire the old name, since it only // knows the file's current pathname. if ve.deleted { if _, reAdded := vfEntries[ve.origname]; !reAdded { delete(fileMap, ve.origname) } delete(fileMap, name) continue } // Ensure a new-name entry exists so the file is emitted under its new // name. A content-changed rename already has one (from the changed-file // pass); a pure rename seeds an empty entry whose content the // unchanged-file pass fills from the vfile blob. entry, ok := fileMap[name] if !ok { entry = manifest.File{Name: name, Perm: fileMap[ve.origname].Perm} } entry.OldName = ve.origname fileMap[name] = entry if _, reAdded := vfEntries[ve.origname]; !reAdded { delete(fileMap, ve.origname) } } } // restatCommitPerms re-derives each included file's executable permission // directly from its live on-disk mode, overriding whatever change scanning // captured earlier. This exists because change scanning compares content // hashes: a file whose only edit is a chmod has an unchanged hash, so // vfile.chnged/isexe can go stale and silently carry forward the wrong // permission. Manifest generation must consult the filesystem itself for // every file it is about to serialize — mirroring canonical Fossil's // belt-and-braces re-stat in src/checkin.c — rather than trust a cached // flag a second time. // // Only files that actually have a vfile row are considered — matching // canonical Fossil, whose manifest loop is driven by a query over vfile // (JOIN blob) rather than the union of parent-manifest and vfile names. // Files carried forward from the parent manifest but no longer present in // the local vfile (should not happen once LoadVFile has run, but is not an // invariant this function enforces) are left untouched, since there is no // live vfile row to justify a re-stat. Symlinks (Perm == "l") and files not // selected for this commit are also left untouched. On Windows, // os.FileMode never carries an owner-execute bit, so modeIsExecutable // always returns false and this is a no-op. // // A file that has gone missing from disk (deleted outside of Unmanage, or // otherwise untracked-but-not-told) is skipped, carrying its existing Perm // forward unchanged. This function's job is fixing missed mode changes, not // detecting stale tracking, so it stays lenient here — but that leniency is // safe because buildCommitFiles already rejected any in-scope missing file // before this function ever runs (issue #79); a missing file only reaches // here at all when it was out of scope (shouldInclude false), in which case // carrying its Perm forward unchanged is correct. Any other Stat error // (permission denied, I/O fault, ...) is a real problem and still fails // the commit. func (c *Checkout) restatCommitPerms( fileMap map[string]manifest.File, vfEntries map[string]vfileCommitEntry, shouldInclude func(string) bool, ) error { if fileMap == nil { panic("checkout.restatCommitPerms: fileMap must not be nil") } if shouldInclude == nil { panic("checkout.restatCommitPerms: shouldInclude must not be nil") } for name := range vfEntries { entry, ok := fileMap[name] if !ok { continue // deleted, or otherwise dropped from this commit } if entry.Perm == "l" { continue } if !shouldInclude(name) { continue } fullPath, err := c.safePath(name) if err != nil { return fmt.Errorf("checkout.Commit: path traversal in %s: %w", name, err) } info, err := c.env.Storage.Stat(fullPath) if err != nil { if errors.Is(err, os.ErrNotExist) { continue // missing on disk: carry the existing Perm forward unchanged } return fmt.Errorf("checkout.Commit: stat %s: %w", name, err) } if modeIsExecutable(info.Mode()) { entry.Perm = "x" } else { entry.Perm = "" } if entry.Perm != "" && entry.Perm != "x" { panic("checkout.restatCommitPerms: re-derived perm must be \"\" or \"x\"") } fileMap[name] = entry } return nil } // finalizeCommit updates vvar, reloads vfile, and clears the checkin queue // after a successful manifest.Checkin. func (c *Checkout) finalizeCommit(newRID libfossil.FslID, newUUID string) error { if err := setVVar(c.db, "checkout", strconv.FormatInt(int64(newRID), 10)); err != nil { return fmt.Errorf("checkout.Commit: set checkout vvar: %w", err) } if err := setVVar(c.db, "checkout-hash", newUUID); err != nil { return fmt.Errorf("checkout.Commit: set checkout-hash vvar: %w", err) } if err := c.LoadVFile(newRID, true); err != nil { return fmt.Errorf("checkout.Commit: reload vfile: %w", err) } c.checkinQueue = nil return nil } // Commit creates a new checkin from staged files in the checkout. // Returns the new manifest RID and UUID. func (c *Checkout) Commit(opts CommitOpts) (libfossil.FslID, string, error) { if c == nil { panic("checkout.Commit: nil *Checkout") } parentRID, _, err := c.Version() if err != nil { return 0, "", fmt.Errorf("checkout.Commit: %w", err) } if err := c.ScanChanges(ScanHash); err != nil { return 0, "", fmt.Errorf("checkout.Commit: scan: %w", err) } if opts.PreCommitCheck != nil { if err := opts.PreCommitCheck(); err != nil { return 0, "", fmt.Errorf("checkout.Commit: pre-commit check: %w", err) } } vfEntries, changedFiles, deletedFiles, err := c.collectVFileEntries(parentRID) if err != nil { return 0, "", err } queueActive := c.checkinQueue != nil && len(c.checkinQueue) > 0 shouldInclude := func(name string) bool { if !queueActive { return true } return c.checkinQueue[name] } enqueuedCount := len(changedFiles) if queueActive { enqueuedCount = len(c.checkinQueue) } ctx := c.obs.CommitStarted(context.Background(), CommitStart{ FilesEnqueued: enqueuedCount, Branch: opts.Branch, User: opts.User, }) var result CommitEnd defer func() { c.obs.CommitCompleted(ctx, result) }() commitFiles, err := c.buildCommitFiles( parentRID, vfEntries, changedFiles, deletedFiles, shouldInclude, ) if err != nil { result.Err = err return 0, "", err } commitTime := opts.Time if commitTime.IsZero() { commitTime = c.env.Clock.Now() } // Build T-cards from CommitOpts.Branch and Tags. var tagCards []deck.TagCard if opts.Branch != "" { tagCards = append(tagCards, deck.TagCard{ Type: deck.TagPropagating, Name: "branch", UUID: "*", Value: opts.Branch, }) tagCards = append(tagCards, deck.TagCard{ Type: deck.TagSingleton, Name: "sym-" + opts.Branch, UUID: "*", }) } for _, t := range opts.Tags { tagCards = append(tagCards, deck.TagCard{ Type: deck.TagSingleton, Name: t, UUID: "*", }) } checkinOpts := manifest.CheckinOpts{ Files: commitFiles, Comment: opts.Message, User: opts.User, Parent: parentRID, Time: commitTime, Delta: opts.Delta, } if len(tagCards) > 0 { checkinOpts.Tags = tagCards } newRID, newUUID, err := manifest.Checkin(c.repo, checkinOpts) if err != nil { result.Err = fmt.Errorf("checkout.Commit: checkin: %w", err) return 0, "", result.Err } if err := c.finalizeCommit(newRID, newUUID); err != nil { result.Err = err return 0, "", err } result.RID = newRID result.UUID = newUUID result.FilesCommit = len(commitFiles) return newRID, newUUID, nil }