dehydrate: fix backup databases path regression + un-skip triaged tests (stacked on #2043)#2044
Draft
tyrielv wants to merge 12 commits into
Draft
dehydrate: fix backup databases path regression + un-skip triaged tests (stacked on #2043)#2044tyrielv wants to merge 12 commits into
tyrielv wants to merge 12 commits into
Conversation
Backups from 'gvfs dehydrate' accumulate under <enlistment>\dehydrate_backup\<timestamp> and are never cleaned up, so disk space isn't reclaimed after a successful dehydrate. Add opt-in cleanup: - '--discard-backup' deletes the current run's backup folder after a successful dehydrate (the backup is still created transiently for safety and only removed on success). - 'gvfs dehydrate prune-backups' is a sub-verb that only deletes backups left by previous runs and never performs a dehydrate. When neither is used, the success message now points users at both options. Backup management is scoped to the dehydrate verb, so it does not affect 'gvfs sparse --prune', which reuses DehydrateVerb internally. Adds CLI parse tests (GvfsMainCliTests) and functional tests (DehydrateTests). Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
The DehydrateTests fixture was marked [SkipInCI] at the class level (commit b395f5e) as atrophied. Ran the full fixture locally to triage it. Move the skip from the fixture to individual tests: - 13 verified-passing tests (including the two new backup tests) now run in CI. - 4 tests are genuinely atrophied (backup layout gained a 'databases' folder; --no-status now exits 0; nonexistent-folder now reports success; locked-folder returns exit 7) and keep [SkipInCI] with specific reasons. - 13 folder tests could not be verified because the fixture hangs locally on ProjFS 'provider temporarily unavailable' after ~15 mount/dehydrate cycles; they keep [SkipInCI] pending triage. The atrophied/unverified tests are addressed in a stacked follow-up. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
… dehydrates CI slice 4 failed: the shared EnlistmentPerFixture cannot run consecutive full dehydrates -- the first succeeds, later ones fail to back up (leaving the enlistment dirty, which cascades). My earlier verification passed only because each test ran against a fresh enlistment in isolation. - Convert the two new feature tests (FolderDehydrateWithDiscardBackupShouldDeleteBackup, DehydratePruneBackupsShouldDeleteExistingBackups) to use folder dehydration, the reliable default path, so --discard-backup and prune-backups still get real CI coverage. - Re-skip FullDehydrateShouldSucceedInCommonCase and DehydrateShouldSucceedEvenIfObjectCacheIsDeleted: they need a per-test-case enlistment to run alongside other full dehydrates. Triage in follow-up. Verified locally: both folder-based tests pass when run together. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
tyrielv
force-pushed
the
tyrielv/dehydrate-fix-atrophied-tests
branch
from
0225796 to
66faf34
Compare
Adds the --discard-backup flag and the 'gvfs dehydrate prune-backups' sub-verb, a backward-compatible new feature, so bump the minor version 2.0 -> 2.1. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
tyrielv
force-pushed
the
tyrielv/dehydrate-fix-atrophied-tests
branch
from
66faf34 to
c808544
Compare
…ability A dehydrate backup can contain ProjFS placeholders (e.g. from a folder dehydrate). Deleting them right after the remount can transiently fail with 'The provider that supports file system virtualization is temporarily unavailable', which made --discard-backup leave the backup behind. Add PhysicalFileSystem.TryWaitForDirectoryDelete (a directory analogue of the existing TryWaitForDelete) and use it for both --discard-backup and 'prune-backups', retrying with a short backoff so the space is reliably reclaimed. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
tyrielv
force-pushed
the
tyrielv/dehydrate-fix-atrophied-tests
branch
from
c808544 to
dd96b23
Compare
…-delete fix The moved folder contents in a dehydrate backup are ProjFS placeholders orphaned outside the virtualization root. The inline --discard-backup delete fails with ERROR_VIRTUALIZATION_TEMPORARILY_UNAVAILABLE and does not recover with retry (added last commit). Deleting orphaned placeholders needs a proper fix (native reparse-point delete, or unmount-before-delete). Skip the functional test for now so CI is green; the prune-backups test still exercises deletion via a separate process. Feature parsing is covered by GvfsMainCliTests. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
tyrielv
force-pushed
the
tyrielv/dehydrate-fix-atrophied-tests
branch
from
dd96b23 to
4a5a66d
Compare
A folder-dehydrate backup contains ProjFS placeholders that were moved outside the virtualization root. The process that performed the dehydrate cannot delete them (the delete fails with ERROR_VIRTUALIZATION_TEMPORARILY_UNAVAILABLE and does not recover with retry), but a freshly-spawned process deletes them with a plain recursive delete. This was confirmed empirically and matches CI: the in-process --discard-backup delete failed while the separate 'prune-backups' process succeeded. Delegate the deletion to a separate 'gvfs dehydrate prune-backups' process: - prune-backups gains a hidden --backup-path option to delete a single backup (with a safety check that the path is under the enlistment's dehydrate_backup folder) and returns a non-zero exit code on failure. - --discard-backup launches that process. By default it runs detached (StartBackgroundVFS4GProcess) so large backups don't block the command. - New --wait-for-backup-delete makes it wait for the deletion and report the result; the functional test uses it for determinism. Applies to both folder and full dehydrate. No native reparse-point code needed. Un-skip FolderDehydrateWithDiscardBackupShouldDeleteBackup (now uses --wait-for-backup-delete). Adds CLI parse coverage for --wait-for-backup-delete and prune-backups --backup-path. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
tyrielv
force-pushed
the
tyrielv/dehydrate-fix-atrophied-tests
branch
from
4a5a66d to
15f26de
Compare
CI slice 4 showed the --discard-backup delete still failing with 'provider temporarily unavailable' for 15+ seconds, while the separate 'prune-backups' process spawned by the test framework deleted the same kind of backup instantly. The two child processes were identical (same verb, same working directory, both UseShellExecute=false) except for their parent: a child of the dehydrating gvfs.exe fails, a child of an unrelated process succeeds. The failure is therefore caused by the child inheriting the dehydrating process's handles, not by timing (retry never recovered). For --wait-for-backup-delete, launch the prune-backups child with StartBackgroundVFS4GProcess (ShellExecuteEx, which does not inherit the parent's handles and runs from the install directory) and then WaitForExit and check the exit code, instead of ProcessHelper.Run (which inherits handles). The default background path already used StartBackgroundVFS4GProcess, so both paths now avoid handle inheritance. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
tyrielv
force-pushed
the
tyrielv/dehydrate-fix-atrophied-tests
branch
from
15f26de to
908ef47
Compare
Two CI runs showed the --discard-backup delete failing identically regardless of how the child prune-backups process was launched (ProcessHelper.Run, then StartBackgroundVFS4GProcess). In both runs the separate prune-backups process spawned by the test AFTER the dehydrate process exited deleted an equivalent backup instantly. The one consistent difference: the delete fails (persistently, for 15+ seconds) only while the dehydrating gvfs.exe process is still alive. Waiting for the deletion keeps that process alive, which is exactly what blocks the deletion. So a synchronous --wait-for-backup-delete cannot work. Remove it and always run the deletion in a detached 'gvfs dehydrate prune-backups' process that completes after this process exits. The functional test now polls for the backup folder to disappear. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
tyrielv
force-pushed
the
tyrielv/dehydrate-fix-atrophied-tests
branch
from
908ef47 to
127bb32
Compare
The backup deletion kept failing in CI with 'provider ... temporarily unavailable'. Root cause: CI runs test fixtures in parallel (Parallelizable(ParallelScope.Fixtures)), so many enlistments drive ProjFS concurrently. A normal recursive delete enumerates the moved backup placeholders by opening directory handles WITHOUT FILE_FLAG_OPEN_REPARSE_POINT, which recalls through the ProjFS provider; while the provider is busy (or no longer projecting the moved folders) that returns ERROR_VIRTUALIZATION_TEMPORARILY_UNAVAILABLE, and retrying does not help. Add NativeMethods.DeleteDirectoryWithoutProviderRecall, which recursively deletes a tree opening every handle with FILE_FLAG_OPEN_REPARSE_POINT (+ FILE_FLAG_BACKUP_SEMANTICS for directories, FILE_FLAG_DELETE_ON_CLOSE to delete). This enumerates and deletes the reparse points directly and never contacts the provider, so it is immune to provider contention. PhysicalFileSystem.TryDeleteDirectoryWithoutProviderRecall wraps it with a light retry, and 'gvfs dehydrate prune-backups' uses it. Verified: 887/887 unit tests pass; discard + prune functional tests pass locally. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
tyrielv
force-pushed
the
tyrielv/dehydrate-fix-atrophied-tests
branch
from
127bb32 to
89f4373
Compare
A dehydrate backup contains ProjFS placeholders moved outside the virtualization root. They can only be deleted while the repo is UNMOUNTED: while mounted, deleting them fails with ERROR_VIRTUALIZATION_TEMPORARILY_UNAVAILABLE and no retry, process-isolation, or FILE_FLAG_OPEN_REPARSE_POINT avoids it (all tried and failed in CI). The bug only reproduces when the dehydrated folder was hydrated first, which is why it showed up in CI (shared fixture) but not in isolated local runs. Delete the backup during the dehydrate's existing unmounted window instead: - Full dehydrate (CLI): RunFullDehydrate deletes the backup after moving src and before remounting. - Folder dehydrate (mount): DehydrateFolders.Request carries a DiscardBackup flag; the mount deletes the backup inside BackupFoldersWhileUnmounted (while unmounted) and reports success via Response.BackupDiscarded. - Deletion is synchronous; no detached process or polling. prune-backups (for old backups) no longer auto-unmounts (that could break the user's running processes and is unintuitive). It tries to delete and, if it fails while the repo is mounted, tells the user to unmount and retry. Removed the hidden --backup-path option and the native reparse-point delete (a dead end -- OPEN_REPARSE_POINT does not bypass the ProjFS filter). Tests now hydrate the folder before dehydrating (required to reproduce the real scenario) and assert synchronous deletion. Covers discard, prune-when-unmounted, and prune-while-mounted-guidance. 887/887 unit tests and 58 CLI parse tests pass; the three functional tests pass locally. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Follow-up triage of the DehydrateTests fixture (stacked on the --discard-backup PR). Product fixes: - Full dehydrate backed up the .gvfs databases to a stray top-level 'databases' folder instead of '.gvfs/databases'. This was an accidental regression in f0243fe (the folder-dehydrate path and the code comment both use '.gvfs/databases'). Restore the correct location. - Update the --no-status option help: combining it with --folders is now supported (per the per-folder rewrite in c8298bd); the old text still claimed it was disallowed. Test changes: - Un-skip DehydrateShouldBackupFiles (now passes with the databases-path fix). - Remove FolderDehydrateDirtyStatusWithNoStatusShouldFail: it asserted the now-removed '--no-status not valid with --folders' error; combining is intentionally allowed. - Remove FolderDehydrateFolderThatDoesNotExist: dehydrating a nonexistent folder is now a successful no-op; the test is obsolete. Verified locally: DehydrateShouldBackupFiles, FullDehydrateWithDiscardBackupShouldDeleteBackup, DehydratePruneBackupsShouldDeleteExistingBackups all pass. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
tyrielv
force-pushed
the
tyrielv/dehydrate-fix-atrophied-tests
branch
from
89f4373 to
4f3b01a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #2043 (stacked on
tyrielv/dehydrate-delete-backup). While un-skippingthe
DehydrateTestsfixture for #2043, I triaged the tests that commitb395f5echad marked atrophied. Two of them turned out to flag real behavior issues, not stale
expectations.
Product fixes
gvfs dehydratebacked the.gvfsdatabases up to a stray top-level
dehydrate_backup\<ts>\databases\folder insteadof
dehydrate_backup\<ts>\.gvfs\databases\. This was an accidental regression inf0243fea— the folder-dehydrate path uses.gvfs\databases, the helper parameteris named
backupGvfs, and the code comment says "Create backup .gvfs databasesdirectory". Restored the correct location.
--no-statushelp text. Combining--no-statuswith--foldersis nowsupported (per the per-folder rewrite in
c8298bd1); the option description stillclaimed it was disallowed "to prevent data loss". Updated to describe the actual
behavior.
Test changes
DehydrateShouldBackupFiles— passes with the databases-path fix.FolderDehydrateDirtyStatusWithNoStatusShouldFail— asserted theremoved
--no-status not valid with --folderserror; combining is intentionallyallowed now.
FolderDehydrateFolderThatDoesNotExist— dehydrating a nonexistentfolder is now a successful no-op; the test is obsolete.
Still skipped (future triage)
The remaining
[SkipInCI]folder tests could not be verified locally because thefull fixture hangs on ProjFS "provider temporarily unavailable" after ~15
mount/dehydrate cycles (
FolderDehydrateFolderThatWasDeletedhangs outright). Theyremain skipped with per-test reasons, to be triaged separately.
Testing
Verified locally (isolated runs):
DehydrateShouldBackupFiles,FullDehydrateWithDiscardBackupShouldDeleteBackup,DehydratePruneBackupsShouldDeleteExistingBackupsall pass.Work item: AB#62005440