Parents and children of check-in [e5be6a9199]
|
2026-07-23
| ||
| 11:17 |
fix: cover ScanJulianDay's ordering consumers, keep the float64 branch exact (#60) (#121)
Issue #60's triage was stale: the live truncation bug it describes was already closed on main by 11967ff (#62), which added millisecond rounding to timeToJulian. What remained was a coverage gap, not a defect. An earlier attempt on this branch also normalized the float64 branch onto the millisecond grid, on the theory that a driver-independent helper is strictly better. That was wrong, and this commit does not do it. A scanned float64 IS the value stored in the row, so snapping it can only move it away from what the row holds. Timeline's pagination cursor depends on the opposite property: its predicate `e.mtime < ? OR (e.mtime = ? AND b.rid < ?)` requires the cursor to compare exactly equal to the row it came from. SQLite's own julianday() computes iJD/86400000.0, a different expression from TimeToJulian's julianEpoch + ms/86400000.0, and the two land one ulp apart on a real fraction of instants — so normalizing alters mtimes written by upstream Fossil, the library's primary real-world input. Raised, the cursor row repeats as the next page's first row; lowered, rows are skipped. Both are silent wrong answers. The float64 branch therefore stays verbatim, and its doc comment now records why that exactness is load-bearing rather than incidental. The same reasoning applies to tag propagation's `tagxref.mtime < ?`, where SQLite compares the bound value against raw stored floats: an exact bound is the correct one there too, so no normalized variant is introduced for it either. The one genuine transform stays where it belongs, in the time.Time branch, which must round because ncruces hands back DATETIME columns carrying its own sub-millisecond conversion noise. Its comment now also records the limit of that recovery: an mtime written at finer than millisecond resolution cannot be reproduced through a time.Time at all, and rounding the float64 branch to match would discard precision on both drivers rather than recover any. Non-functional cleanup: millisPerDay and julianDayFromMillis replace two open-coded copies of the same expression. Coverage added (no build tag; these run under both drivers): - db: TestScanJulianDayFloatBranchIsBitExact sweeps julianday()-computed mtimes and asserts each scans back bit-for-bit, with a guard that fails the test if the sweep ever stops containing divergent values. Verified to fail against the normalizing version and pass against this one. - db: TestScanJulianDayIsDriverIndependent proves the float64 and time.Time representations of one millisecond-aligned write converge, with noise in both directions. TestScanJulianDaySentinelZero pins the "no mtime" sentinel that bisect's `mtime != 0` guard depends on. - tag: TestPropagateAllOrderingAdjacentMillisecond drives the `tagxref.mtime < ?` decision through ScanJulianDay with adjacent-millisecond mtimes; it fails under a truncating conversion and passes under the rounding one, and passes under modernc regardless — the reason this class went unnoticed twice. Consumer classification (ScanJulianDay): - manifest/timeline.go NewCursor: ordering/equality, requires exactness - tag/propagate.go PropagateAll -> `tagxref.mtime < ?`: ordering (now covered) - tag/propagate.go childMtime -> mtimeQueue.Less: ordering, traversal-order only (visited set makes it result-neutral) - manifest/log.go, bisect/bisect.go: display-only Known limitation, left alone deliberately: julianToTime truncates where timeToJulian rounds, so the two are not inverses at sub-millisecond resolution. Pre-existing, now documented; ScanTime's consumers are display paths with no ordering boundary. This commit does not widen the gap. check-in: 78a793dd83 user: [email protected] tags: trunk | |
| 10:46 |
fix: return error when clone stalls with phantoms outstanding (#129)
* fix: return error when clone stalls with phantoms outstanding Clone's convergence loop broke out of its round loop as soon as the phantom count stopped decreasing and returned success, even when the phantom table was still non-empty. That leaves the caller holding a repository missing content with no error and exit code 0. Stopping on a stall is still correct — spinning forever would be worse — but reporting it as success is not. checkStop now returns a PhantomStallError when the loop halts with phantoms remaining, giving the caller a programmatically-reachable count plus a sample of the missing UUIDs for diagnosis. A clean clone still returns nil with an empty phantom table. Fixes #64. * fix: preserve partial CloneResult when Clone returns an error Clone's named result return was never assigned on the error paths — cs.run's cloneResult stayed in a local variable, so a caller catching a PhantomStallError (or a crosslink failure) got a nil result even though a partial one, with the round/blob counts collected so far, was sitting right there. Assign it before returning on both paths. * fix: make phantom stall error sample deterministic newPhantomStallError previously took an arbitrary 10 entries from map iteration order before sorting, so the sample was nondeterministic across runs when more than 10 phantoms were outstanding. Sort all UUIDs first, then take the lexicographically-smallest 10, matching what the doc comment already implied. Addresses a non-blocking nit from review on #64. check-in: e5be6a9199 user: [email protected] tags: trunk | |
| 10:41 |
fix: zero-length file commit panic and stale missing-file carry-forward (#128)
* fix: zero-length file commit panic and stale missing-file carry-forward Two related bugs in the checkin path: #68 - Committing a zero-length file panicked in blob.Store, which treated an empty artifact (a normal, well-known Fossil blob) as invalid input. Worse, manifest.Checkin's postcondition defer fired unconditionally during the panic's stack unwind and re-panicked with its own unrelated "manifestRid must be positive" message, masking the real cause. Fixed both: blob.Store now accepts zero-length (and nil) content, and Checkin's defer re-panics with the original panic value instead of asserting a postcondition that was never going to hold mid-unwind. Fixing the panic surfaced a second bug in the same family: buildCommitFiles used len(Content) > 0 to detect "already loaded from disk," which can't distinguish a genuinely empty file from one not yet loaded, so a newly added empty file failed with "no RID for unchanged file." Replaced the length sentinel with an explicit loaded-set. #79 - A tracked file deleted from disk without Unmanage/Remove was silently carried forward into the new manifest with its last-committed content, rather than being detected. Verified against fossil 2.28: canonical fails a commit when a missing tracked file is in scope (either the implicit "commit everything" case, or explicitly named), but leaves an out-of-scope missing file alone. buildCommitFiles now stats every in-scope tracked file before treating it as unchanged and errors clearly, naming the file, when it's gone; a file outside the commit's scope (relevant only to an explicit Enqueue) is left untouched exactly as before. Updated the two existing tests that had documented the old silent-carry-forward behavior as deliberately deferred to this issue. * fix: add CHANGELOG entry and unwrap-safe stat checks for #68/#79 Adds the CHANGELOG entries the #79 acceptance criteria required for a user-visible behavior change (missing tracked file now hard-fails a commit instead of silently carrying stale content forward) and for the #68 zero-length-file panic fix. Also switches checkin.go's two os.IsNotExist(err) checks to errors.Is(err, os.ErrNotExist), which correctly unwraps the %w-wrapped os.ErrNotExist that MemStorage.Stat returns. Production was already correct (OSStorage.Stat returns a raw *PathError), but the tests were only exercising a generic fallback error branch instead of the intended "tracked file missing from disk" message. check-in: 0351167ae5 user: [email protected] tags: trunk | |