checkout: preserve skip-worktree for virtual filesystem paths#915
Draft
tyrielv wants to merge 1 commit into
Draft
checkout: preserve skip-worktree for virtual filesystem paths#915tyrielv wants to merge 1 commit into
tyrielv wants to merge 1 commit into
Conversation
1cbd16b to
c882265
Compare
When 'git checkout <tree> -- <pathspec>' updates the index with entries from the source tree, update_some() creates a new cache entry that unconditionally clears skip-worktree. In a virtual filesystem repo (core.virtualfilesystem is set), this causes checkout_entry() to attempt unlink() on files that exist only as virtual projections with no physical NTFS entry. The unlink fails with ENOENT, producing 'error: unable to unlink old' messages and exit code 255. Fix this in three places: 1. update_some(): Propagate CE_SKIP_WORKTREE from the existing index entry to the replacement entry when core_virtualfilesystem is set. 2. mark_ce_for_checkout_overlay(): Allow skip-worktree entries that have CE_UPDATE (set by update_some for tree entries) to still match the pathspec, so report_path_error() does not reject them. 3. checkout_worktree(): Skip checkout_entry() for entries that have both CE_MATCHED and CE_SKIP_WORKTREE, since the virtual filesystem provider will serve the correct content from the updated projection. The index is updated to the new tree entry's OID while the working tree write is skipped entirely. The virtual filesystem provider re-reads the updated index and serves the correct content on next access. Same approach as the CE_NEW_SKIP_WORKTREE propagation in deleted_entry() (unpack-trees.c, PR microsoft#865) which avoids unnecessary lstats on virtualized paths during branch switches. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
c882265 to
59ba655
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
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.
Problem
git checkout <tree> -- <pathspec>fails witherror: unable to unlink old '<path>': No such file or directoryand exit code 255 when the target file is a virtual ProjFS projection that has not been materialized as a placeholder in a GVFS-mounted repo.Files that have been accessed (and thus have a ProjFS placeholder with a physical NTFS entry) are unaffected —
unlink()works fine for those. The failure is specific to files that exist only in the virtual projection:lstat()succeeds (ProjFS intercepts stat calls for projected files) butunlink()fails because there is no physical NTFS entry to delete.Even without ProjFS, the unfixed code path is wasteful: it clears skip-worktree, writes the file to disk, and triggers ProjFS placeholder creation — all unnecessary because the virtual filesystem provider will serve the correct content once the index is updated.
Root Cause
update_some()inbuiltin/checkout.ccreates a replacement cache entry withcreate_ce_flags(0)which unconditionally clears skip-worktree. This causesmark_ce_for_checkout_overlay()to mark the entry for checkout, andcheckout_entry_ca()attemptsunlink()+write_entry()on a path that may only exist as a virtual projection with no physical file.Fix
Three changes in
builtin/checkout.c, all gated oncore_virtualfilesystemand/orce_skip_worktree():update_some(): PropagateCE_SKIP_WORKTREEfrom the existing index entry to the replacement entry whencore_virtualfilesystemis set. The index is updated with the new tree entry's OID while the virtual filesystem flag is preserved.mark_ce_for_checkout_overlay(): Allow skip-worktree entries that haveCE_UPDATE(set byupdate_somefor changed tree entries) to still match the pathspec, soreport_path_error()does not reject them.checkout_worktree(): Skipcheckout_entry()for entries that have bothCE_MATCHEDandCE_SKIP_WORKTREE. The virtual filesystem provider will serve the correct content from the updated projection on next access.Non-VFS repos and hydrated files (skip-worktree cleared by the VFS hook) are completely unaffected.
Related
Same approach as the
CE_NEW_SKIP_WORKTREEpropagation in #865 which avoids unnecessary lstats on virtualized paths during branch switches. Both bugs stem from skip-worktree being dropped on replacement cache entries, causing unnecessary physical filesystem operations on virtual files.Tests
Two new tests in
t/t1093-virtualfilesystem.sh:checkout path preserves skip-worktree: Creates two commits with different file content, enables VFS mode with 0% hydration, removes the physical file, runs
checkout HEAD~1 -- file, and verifies the index is updated while no file is written to disk.restore --staged after checkout path: Verifies that
restore --staged --ignore-skip-worktree-bitscorrectly restores the index entry back to HEAD after a checkout-path operation with skip-worktree preserved.Verified that test 1 fails without the product code change (checkout writes the file to disk) and passes with it.