Repository navigation
snapshots: no second copy of a save already kept - #56
ChakraFusion wants to merge 3 commits into
Conversation
A newly tracked game had no recorded baseline (Game.LastManifestHash), a snapshot taken by hand left the one from before, and changing a game's exclusions cleared it. Each time, the next file event of any kind - a log written into a folder just excluded from syncing - or the catch-up a watch runs when it starts read as the save changing, and an automatic snapshot was taken of a save already kept. With cloud sync on, that copy was then the device's newest save, and a newer one another device had put in the cloud was no longer offered. TestIgnored_ASaveFromTheCloudLeavesAnIgnoredFileAlone failed that way on Windows CI whenever the extra snapshot came after the other device's (seen once in 150 runs with the race detector, with the daemons' logs). - The initial snapshot and snapshots taken by hand (app, `opensave snapshot`, snapshot everything) record the save as it was just before them (daemon.SnapshotWithBaseline). A change made while one is being written differs from it and is still snapshotted by the watcher. Worst case one snapshot too many, as before; never a change taken for the baseline. - An exclusion change restates the recorded hash under the new rules only when that claims nothing new: the save is as it was at the last snapshot under the old rules, and the new rules bring no file back into view. Otherwise it is cleared as before. The app and `opensave exclude` share it (daemon.LastSnapshotHashUnder). Tests: e2e TestBaseline_AFileInAnExcludedFolderTakesNoSnapshot (fails on dev-public: 2 snapshots), TestBaseline_ASnapshotByHandIsNotTakenAgain (fails without the change: 3), TestBaseline_TheFirstChangeAfterTrackingIsSnapshotted, and TestLastSnapshotHashUnder for the four cases. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
@ChakraFusion is attempting to deploy a commit to the sivadaboi's projects Team on Vercel. A member of the Team first needs to authorize it. |
…versions follow an exclusion change at once Port of upstream PR Liquid-co#56: the initial snapshot and snapshots taken by hand record the save as it was just before them (daemon.SnapshotWithBaseline), and an exclusion change restates the recorded hash under the new rules when that provably claims nothing new (daemon.LastSnapshotHashUnder). Here in the terms the watcher hashes in: the game's rules with the device settings kept out of sync (IgnoreText). That exposed a gap in save versions. A change to a game's own exclusions re-took the version's hash only at the next start (the device-settings check), never when it was made. Until then the version kept a hash under the old rules, which any later hash reads as the same save measured differently (sameSaveAs): a change made after the rule change was taken into the version's hash by the next sync instead of becoming a new version, and never reached the other device. The automatic snapshot that followed every rule change used to hide it by recording a version first. TestJourney_DeviceSpecificConfigNeverTravels failed 5/5 that way once that snapshot was gone. Now the app and `opensave exclude` re-take it at once (daemon.ExclusionsChanged, via AdoptExclusionView), when the save is unchanged under the old rules. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
The failing job is the Linux unit tests: |
…files; Stop waits for a catch-up
Follow-up to the first commit, found from this PR's own CI failure
(TestAddingAnOrdinarySaveLocationStillWorks, Linux: "TempDir RemoveAll
cleanup: ... 002: directory not empty").
- The hash the watcher compares against covers every save location, so
adding or removing one changed it while no file changed. With a baseline
now recorded at tracking, the catch-up the re-watch runs then snapshotted
a save already kept. A location change now restates the hash when the
save is as it was at the last snapshot and an added location holds no
file yet (daemon.LocationsChanged, from the app and `opensave locations`).
A location that brings files is still snapshotted: the backup should
hold them.
- Stop cancelled the catch-up worker without waiting for it, so a catch-up
already writing a snapshot went on after Stop returned - into a daemon
home the test was removing ("002" is the test server's home), or past a
closed database as the app quits. Stop now waits for it, bounded like
stopping a watch.
Tests: e2e TestBaseline_AddingASaveLocationTakesNoSnapshotUnlessItBringsFiles
(fails without the change: 2 snapshots) and TestStopWaitsForACatchUpUnderWay
(fails without the change: Stop returns mid-snapshot).
Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
A correction to my note above: I had that wrong, and it was caused by this PR. "002" is the test server's own home folder, not the extra location. Something wrote into it after the daemon had stopped. With this PR, a newly tracked game has a recorded baseline, so adding a save location (which changes the hash, since it covers every location) made the re-watch's catch-up take a snapshot of an unchanged save. And Fixed in a second commit here: a location change restates the hash when the save is unchanged and the new location holds no files yet (one that brings files is still snapshotted), and |
A newly tracked game's first snapshot runs in the background. When a snapshot was taken by hand while it ran, the hand-taken one could finish first and record its value, and the first snapshot's older value then replaced it - the watcher read the save as changed and took a snapshot of one already kept. Seen in a daemon log on the fork: "auto-snapshot created" right after a snapshot taken by hand of the same files. SnapshotWithBaseline now records its value only while the recorded one is still what it was when the snapshot started (Store.SetLastManifestHashIf). When it is not, another snapshot recorded something newer, and that stands. Test: TestSetLastManifestHashIf_DoesNotReplaceANewerValue. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…files; Stop waits for a catch-up; a baseline never replaces a newer one Port of upstream Liquid-co#56's second and third commits: - Adding or removing a save location restates the last-snapshot hash when the save is unchanged and an added location holds no file yet (daemon.LocationsChanged), here in the watcher's terms (IgnoreText). - The watcher's Stop waits, bounded, for a catch-up already writing a snapshot. - A hash taken before a snapshot is recorded only while no newer one was recorded meanwhile (Store.SetLastManifestHashIf). Without it, a snapshot taken by hand while a newly tracked game's first one ran could have its value replaced by the older one, and the watcher snapshotted the save again. Tests: as upstream; here the location test looks for a new snapshot rather than counting, since an older one a newer holds entirely is removed. And TestCloud_SyncLocalRepairsATruncatedRemoteCopy changes the save only after tracking's snapshot: made before it, the change was in that one too, which was then removed for the identical newer one, leaving the cloud copy the test truncates without a local snapshot to repair it (failed 4-8 in 10). Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
The latest run's failure (CI #328, Windows e2e shard 2: "test timed out after 30m0s") isn't from this PR. The goroutine dump shows a relay manifest request that had waited 8 minutes for a reply ( |
This branch has not been deployed
1 participant
Found by catching the CI failure on #52 (
TestIgnored_ASaveFromTheCloudLeavesAnIgnoredFileAlone, "offers = []") with the daemons' logs: it failed once in 150 race-detector runs on Windows runners. It's a real bug, not the test.Cause: the watcher compares a game's save against the hash recorded at its last snapshot (
Game.LastManifestHash). A newly tracked game had none, a snapshot taken by hand left the old one in place, and changing a game's exclusions cleared it. Each time, the next file event of any kind (a log written into a folder that was just excluded), or the catch-up a watch runs when it starts, read as the save changing. An automatic snapshot was then taken of a save already kept. With cloud sync on, that copy is the device's newest save, so a newer one another device has put in the cloud is no longer offered. In the failing run, the deck's extra snapshot came 2 s after the desktop's upload, so no offer appeared.Fix:
opensave snapshot, snapshot everything) record the save as it was just before them (daemon.SnapshotWithBaseline). A change made while one is being written differs from it and is still snapshotted by the watcher. The worst case is one snapshot too many, as before; a change is never taken for the baseline (the concern incatchUpOne's comment).opensave excludeshare it (daemon.LastSnapshotHashUnder).Tests:
TestBaseline_AFileInAnExcludedFolderTakesNoSnapshot, which fails ondev-public(2 snapshots instead of 1).TestBaseline_ASnapshotByHandIsNotTakenAgain, which fails without the change (3 instead of 2).TestBaseline_TheFirstChangeAfterTrackingIsSnapshotted.TestLastSnapshotHashUnderfor the four cases (unchanged and narrower; changed since; a file brought back; nothing recorded).🤖 Generated with Claude Code