Artifact cfee900567e8517dc692d2b1a555102b07a0f89a54c9aba7f26653dbdc7f9d61:
- File
internal/checkout/update.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: 14594) [annotate] [blame] [check-ins using]
package checkout import ( "context" "database/sql" "fmt" "os" "path/filepath" "sort" "strconv" "time" "github.com/danmestas/go-libfossil/internal/content" libfossil "github.com/danmestas/go-libfossil/internal/fsltype" "github.com/danmestas/go-libfossil/internal/manifest" "github.com/danmestas/go-libfossil/internal/merge" ) // CalcUpdateVersion finds the latest leaf checkin to update to. // Returns 0 if the checkout is already at the latest version. // // Panics if c is nil (TigerStyle precondition). func (c *Checkout) CalcUpdateVersion() (libfossil.FslID, error) { if c == nil { panic("checkout.CalcUpdateVersion: nil *Checkout") } currentRID, _, err := c.Version() if err != nil { return 0, fmt.Errorf("checkout.CalcUpdateVersion: %w", err) } var tipRID int64 err = c.repo.DB().QueryRow(` SELECT l.rid FROM leaf l JOIN event e ON e.objid=l.rid WHERE e.type='ci' ORDER BY e.mtime DESC LIMIT 1 `).Scan(&tipRID) if err != nil { if err == sql.ErrNoRows { return 0, nil // no checkins at all } return 0, fmt.Errorf("checkout.CalcUpdateVersion: %w", err) } if libfossil.FslID(tipRID) == currentRID { return 0, nil // already at latest } return libfossil.FslID(tipRID), nil } // updateFileMaps holds name→UUID maps for the current, target, and ancestor versions. type updateFileMaps struct { current map[string]string target map[string]string ancestor map[string]string allNames map[string]bool } // buildFileMaps builds name→UUID maps for the current, target, and ancestor // file lists, plus the union of all file names. func buildFileMaps(currentFiles, targetFiles, ancestorFiles []manifest.FileEntry) updateFileMaps { m := updateFileMaps{ current: make(map[string]string, len(currentFiles)), target: make(map[string]string, len(targetFiles)), ancestor: make(map[string]string, len(ancestorFiles)), allNames: make(map[string]bool, len(currentFiles)+len(targetFiles)), } for _, f := range currentFiles { m.current[f.Name] = f.UUID m.allNames[f.Name] = true } for _, f := range targetFiles { m.target[f.Name] = f.UUID m.allNames[f.Name] = true } for _, f := range ancestorFiles { m.ancestor[f.Name] = f.UUID } return m } // processFileUpdates iterates over all file names and applies update/merge // logic for each one. Returns the paths actually written, removed, and // conflicted — not merely counts — so the caller can report exactly which // files a consumer needs to inspect. // // Bounded: the loop and the three returned slices are each bounded by // len(maps.allNames), the union of the current and target manifests for // this update — finite for any real checkin. func (c *Checkout) processFileUpdates( ctx context.Context, maps updateFileMaps, strategy merge.Strategy, opts UpdateOpts, checkinTime time.Time, ) (filesWritten, filesRemoved, conflicted []string, err error) { for name := range maps.allNames { curUUID, inCurrent := maps.current[name] tgtUUID, inTarget := maps.target[name] ancUUID := maps.ancestor[name] change, ferr := c.updateFile( name, curUUID, tgtUUID, ancUUID, inCurrent, inTarget, strategy, opts.DryRun, ) if ferr != nil { return filesWritten, filesRemoved, conflicted, fmt.Errorf("checkout.Update: %w", ferr) } if change == UpdateNone { continue } // Apply checkin mtime to written files. if opts.SetMTime && !opts.DryRun && !checkinTime.IsZero() && change != UpdateRemoved { fullPath, _ := c.safePath(name) if fullPath != "" { _ = c.env.Storage.Chtimes(fullPath, checkinTime, checkinTime) } } switch change { case UpdateAdded, UpdateUpdated, UpdateMerged: filesWritten = append(filesWritten, name) case UpdateConflictMerged: filesWritten = append(filesWritten, name) conflicted = append(conflicted, name) c.obs.Error(ctx, fmt.Errorf( "checkout.Update: merge conflict in %s", name, )) case UpdateRemoved: filesRemoved = append(filesRemoved, name) } c.obs.ExtractFileCompleted(ctx, name, change) if opts.Callback != nil { if cerr := opts.Callback(name, change); cerr != nil { return filesWritten, filesRemoved, conflicted, fmt.Errorf( "checkout.Update: callback for %s: %w", name, cerr, ) } } } return filesWritten, filesRemoved, conflicted, nil } // buildUpdateMaps gathers file lists for current, target, and their common // ancestor, then builds the combined name→UUID maps for 3-way comparison. func (c *Checkout) buildUpdateMaps(currentRID, target libfossil.FslID) (updateFileMaps, error) { currentFiles, err := manifest.ListFiles(c.repo, currentRID) if err != nil { return updateFileMaps{}, fmt.Errorf("checkout.Update: list current files: %w", err) } targetFiles, err := manifest.ListFiles(c.repo, target) if err != nil { return updateFileMaps{}, fmt.Errorf("checkout.Update: list target files: %w", err) } ancestor, err := merge.FindCommonAncestor(c.repo, currentRID, target) if err != nil { ancestor = currentRID } var ancestorFiles []manifest.FileEntry if ancestor == currentRID { ancestorFiles = currentFiles } else { ancestorFiles, err = manifest.ListFiles(c.repo, ancestor) if err != nil { return updateFileMaps{}, fmt.Errorf("checkout.Update: list ancestor files: %w", err) } } return buildFileMaps(currentFiles, targetFiles, ancestorFiles), nil } // Update updates the checkout to a new version, performing 3-way merge where // needed to preserve local modifications. // // If opts.TargetRID is 0, CalcUpdateVersion is used to find the target. // If the target equals the current version, Update returns a zero-value // UpdateResult and a nil error (nothing to do). // // A non-nil error means the update genuinely failed; the returned // UpdateResult's contents are not meaningful in that case (see UpdateResult // for the outcome mapping). Update never returns a non-nil error together // with a non-empty UpdateResult.Conflicted — a conflicted merge is a // success, not a failure. // // Panics if c is nil (TigerStyle precondition). func (c *Checkout) Update(opts UpdateOpts) (UpdateResult, error) { if c == nil { panic("checkout.Update: nil *Checkout") } target, currentRID, err := c.updateTarget(opts.TargetRID) if err != nil { return UpdateResult{}, err } if target == 0 { return UpdateResult{}, nil // nothing to update, or already there } ctx := c.obs.ExtractStarted(context.Background(), ExtractStart{ Operation: "update", TargetRID: target, }) var filesWritten, filesRemoved, conflicted []string var updateErr error defer func() { c.obs.ExtractCompleted(ctx, ExtractEnd{ Operation: "update", TargetRID: target, FilesWritten: len(filesWritten), FilesRemoved: len(filesRemoved), Conflicts: len(conflicted), Err: updateErr, }) }() // Refuse a target whose content is not all here before writing any file, // as fossil's update does; finalizeUpdate would refuse it too, but only // after disk had moved. if _, _, err := c.resolveFiles(target); err != nil { updateErr = fmt.Errorf("checkout.Update: %w", err) return UpdateResult{}, updateErr } maps, err := c.buildUpdateMaps(currentRID, target) if err != nil { updateErr = err return UpdateResult{}, updateErr } strategy, ok := merge.StrategyByName("three-way") if !ok { updateErr = fmt.Errorf("checkout.Update: three-way merge strategy not registered") return UpdateResult{}, updateErr } var checkinTime time.Time if opts.SetMTime && !opts.DryRun { checkinTime = c.checkinTime(target) } filesWritten, filesRemoved, conflicted, updateErr = c.processFileUpdates(ctx, maps, strategy, opts, checkinTime) if updateErr != nil { return UpdateResult{}, updateErr } // Finalize: reload vfile and update vvar if updateErr = c.finalizeUpdate(target); updateErr != nil { return UpdateResult{}, updateErr } return sortedUpdateResult(filesWritten, filesRemoved, conflicted), nil } // sortedUpdateResult builds Update's result with each path list sorted. // processFileUpdates walks a map, so the order the lists were built in is // randomized by the Go runtime; sorting makes the result deterministic, so // golden-file and reflect.DeepEqual assertions against it are meaningful. func sortedUpdateResult(written, removed, conflicted []string) UpdateResult { sort.Strings(written) sort.Strings(removed) sort.Strings(conflicted) return UpdateResult{ FilesWritten: written, FilesRemoved: removed, Conflicted: conflicted, } } // updateTarget picks the version Update moves to and the current one. A // target of 0 means there is nothing to do: no newer version, or the // checkout is already there. func (c *Checkout) updateTarget( requested libfossil.FslID, ) (target, current libfossil.FslID, err error) { if c == nil { panic("checkout.updateTarget: nil *Checkout") } if requested < 0 { return 0, 0, fmt.Errorf("checkout.Update: invalid target rid %d", requested) } target = requested if target == 0 { target, err = c.CalcUpdateVersion() if err != nil { return 0, 0, fmt.Errorf("checkout.Update: %w", err) } if target == 0 { return 0, 0, nil } } current, _, err = c.Version() if err != nil { return 0, 0, fmt.Errorf("checkout.Update: %w", err) } if current == target { return 0, current, nil } if target <= 0 { panic("checkout.updateTarget: chose a non-positive target") } return target, current, nil } // finalizeUpdate reloads vfile for the target version, looks up its UUID, // and updates the vvar checkout/checkout-hash entries. func (c *Checkout) finalizeUpdate(target libfossil.FslID) error { if err := c.LoadVFile(target, true); err != nil { return fmt.Errorf("checkout.Update: reload vfile: %w", err) } var targetUUID string err := c.repo.DB().QueryRow("SELECT uuid FROM blob WHERE rid = ?", int64(target)).Scan(&targetUUID) if err != nil { return fmt.Errorf("checkout.Update: query target uuid: %w", err) } if err := setVVar(c.db, "checkout", strconv.FormatInt(int64(target), 10)); err != nil { return fmt.Errorf("checkout.Update: %w", err) } if err := setVVar(c.db, "checkout-hash", targetUUID); err != nil { return fmt.Errorf("checkout.Update: %w", err) } return nil } // updateFile handles the update logic for a single file across three versions. // Returns the UpdateChange type applied, or UpdateNone if the file was skipped. func (c *Checkout) updateFile( name string, curUUID, tgtUUID, ancUUID string, inCurrent, inTarget bool, strategy merge.Strategy, dryRun bool, ) (UpdateChange, error) { switch { case !inCurrent && inTarget: // New file in target — extract and write if !dryRun { if err := c.writeFileFromUUID(name, tgtUUID); err != nil { return UpdateNone, fmt.Errorf("add %s: %w", name, err) } } return UpdateAdded, nil case inCurrent && !inTarget: // File removed in target — delete from disk if !dryRun { fullPath, err := c.safePath(name) if err != nil { return UpdateNone, fmt.Errorf("remove %s: %w", name, err) } if err := c.env.Storage.Remove(fullPath); err != nil { // Ignore errors if file already missing if !os.IsNotExist(err) { return UpdateNone, fmt.Errorf("remove %s: %w", name, err) } } } return UpdateRemoved, nil case inCurrent && inTarget: if curUUID == tgtUUID { // Same content — skip return UpdateNone, nil } // Different content — determine who changed if curUUID == ancUUID { // Current matches ancestor, target changed — fast-forward if !dryRun { if err := c.writeFileFromUUID(name, tgtUUID); err != nil { return UpdateNone, fmt.Errorf("update %s: %w", name, err) } } return UpdateUpdated, nil } if tgtUUID == ancUUID { // Target matches ancestor, current changed — keep current return UpdateNone, nil } // Both changed — 3-way merge needed return c.mergeFile(name, ancUUID, curUUID, tgtUUID, strategy, dryRun) default: // Not in current, not in target — impossible given allNames construction return UpdateNone, nil } } // mergeFile performs a 3-way merge of a file that was modified in both current and target. func (c *Checkout) mergeFile( name, ancUUID, curUUID, tgtUUID string, strategy merge.Strategy, dryRun bool, ) (UpdateChange, error) { // Expand all three versions ancData, err := c.expandUUID(ancUUID) if err != nil { return UpdateNone, fmt.Errorf("merge %s: expand ancestor: %w", name, err) } curData, err := c.expandUUID(curUUID) if err != nil { return UpdateNone, fmt.Errorf("merge %s: expand current: %w", name, err) } tgtData, err := c.expandUUID(tgtUUID) if err != nil { return UpdateNone, fmt.Errorf("merge %s: expand target: %w", name, err) } result, err := strategy.Merge(ancData, curData, tgtData) if err != nil { return UpdateNone, fmt.Errorf("merge %s: %w", name, err) } if !dryRun { fullPath, err := c.safePath(name) if err != nil { return UpdateNone, fmt.Errorf("merge %s: path traversal: %w", name, err) } parentDir := filepath.Dir(fullPath) if err := c.env.Storage.MkdirAll(parentDir, os.FileMode(0o755)); err != nil { return UpdateNone, fmt.Errorf("merge %s: mkdir: %w", name, err) } if err := c.env.Storage.WriteFile(fullPath, result.Content, os.FileMode(0o644)); err != nil { return UpdateNone, fmt.Errorf("merge %s: write: %w", name, err) } } if result.Clean { return UpdateMerged, nil } return UpdateConflictMerged, nil } // writeFileFromUUID expands a blob by UUID and writes it to the checkout directory. func (c *Checkout) writeFileFromUUID(name, uuid string) error { data, err := c.expandUUID(uuid) if err != nil { return fmt.Errorf("writeFileFromUUID %s: %w", name, err) } fullPath, err := c.safePath(name) if err != nil { return fmt.Errorf("writeFileFromUUID %s: path traversal: %w", name, err) } parentDir := filepath.Dir(fullPath) if err := c.env.Storage.MkdirAll(parentDir, os.FileMode(0o755)); err != nil { return fmt.Errorf("mkdir %s: %w", parentDir, err) } if err := c.env.Storage.WriteFile(fullPath, data, os.FileMode(0o644)); err != nil { return fmt.Errorf("write %s: %w", fullPath, err) } return nil } // expandUUID resolves a UUID to its expanded blob content. func (c *Checkout) expandUUID(uuid string) ([]byte, error) { rid, ok := content.AvailableByUUID(c.repo.DB(), uuid) if !ok { return nil, fmt.Errorf("blob not found for uuid %s", uuid) } data, err := content.Expand(c.repo.DB(), rid) if err != nil { return nil, fmt.Errorf("expand rid %d (uuid %s): %w", rid, uuid, err) } return data, nil }