worktree: conditionally allow worktree on VFS-enabled repos#868
worktree: conditionally allow worktree on VFS-enabled repos#868tyrielv wants to merge 2 commits intomicrosoft:vfs-2.53.0from
Conversation
Add GVFS_SUPPORTS_WORKTREES flag (1<<8) to core.gvfs bitmask. When set, allow git worktree commands to run on VFS-enabled repos instead of blocking them with BLOCK_ON_VFS_ENABLED. Force --no-checkout during worktree add when VFS is active so ProjFS can be mounted before files are projected. Support skip-clean-check marker file in worktree gitdir: if .git/worktrees/<name>/skip-clean-check exists, skip the cleanliness check during worktree remove. This allows VFSForGit's pre-command hook to unmount ProjFS after its own status check, then let git proceed without re-checking (which would fail without the virtual filesystem). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
dscho
left a comment
dscho
left a comment
There was a problem hiding this comment.
Looks pretty good, even if I left a lot of suggestions (which boil down to only a few, in essence, though). This is very close. Thank you!
tyrielv
force-pushed
the
tyrielv/gvfs-worktree
branch
2 times, most recently
from
9f960d2 to
bb6390a
Compare
Move the VFS worktree block from the generic BLOCK_ON_VFS_ENABLED check in run_builtin() to an explicit check in cmd_worktree(). This keeps the worktree entry in the commands table clean (RUN_SETUP only) and makes the VFS guard self-contained in builtin/worktree.c. Guard the skip-clean-check marker behind core_virtualfilesystem so it only takes effect when VFS is active. Consolidate the worktree tests into a single test case that covers add (with --no-checkout verification), list, and remove. Use core.gvfs=true instead of the magic number 351. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
tyrielv
force-pushed
the
tyrielv/gvfs-worktree
branch
from
bb6390a to
2337249
Compare
mjcheetham
left a comment
mjcheetham
left a comment
There was a problem hiding this comment.
Just a nit about commenting where 95 comes from for core.gvfs in a test.
nit-nit: Should we also squash the addressing feedback commit into the main commit (with updated message)? I see Copilot as a co-author, and I've found it sadly doesn't like to amend or rebase without being explictly told to keep commits looking 'nice' 😄
| # worktree is conditionally allowed: blocked when VFS enabled without | ||
| # GVFS_SUPPORTS_WORKTREES. | ||
| test_expect_success 'worktree blocked with VFS but without SUPPORTS_WORKTREES' ' | ||
| test_config core.gvfs 95 && |
There was a problem hiding this comment.
95 (0b01011111 / 0x5F) happens to set the undefined bit 5 - this could cause issues if bit 5 gets a purpose in the future (or had one in the past?).
#define GVFS_SKIP_SHA_ON_INDEX (1 << 0)
#define GVFS_BLOCK_COMMANDS (1 << 1)
#define GVFS_MISSING_OK (1 << 2)
#define GVFS_NO_DELETE_OUTSIDE_SPARSECHECKOUT (1 << 3)
#define GVFS_USE_VIRTUAL_FILESYSTEM (1 << 3) /*aliased*/
#define GVFS_FETCH_SKIP_REACHABILITY_AND_UPLOADPACK (1 << 4)
/* undefined */
#define GVFS_BLOCK_FILTERS_AND_EOL_CONVERSIONS (1 << 6)
#define GVFS_PREFETCH_DURING_FETCH (1 << 7)
#define GVFS_SUPPORTS_WORKTREES (1 << 8)The test really only cares about GVFS_USE_VIRTUAL_FILESYSTEM so how about 8 as a value for core.gvfs (0x8)?
Either way we should maybe comment what 95 means and what bit we're really gating on in the test behaviour (like we do in the test below saying true is "all bits including SUPPORTS_WORKTREES").
There was a problem hiding this comment.
Good point. Seeing as the test does not really exercise much, I am fine with risking bit 5 to be set, but I'd prefer to use $((0xffff & ~(1<<8))) if that works...
Probably. Seeing as I blocked Tyrie for so long, I planned on doing that during the next rebase, though. |
dscho
left a comment
dscho
left a comment
There was a problem hiding this comment.
@tyrielv while I agree with @mjcheetham on both points (the magic "95" constant, and avoiding an "add-on-top" fixup commit), I feel bad for forcing you to wait for my review for so long, so I'd offer to merge this as-is and make the follow-up fixes myself during the next rebase. Or you can make those changes if you want, and have the time. Up to you.
3 participants
Add GVFS_SUPPORTS_WORKTREES flag (1<<8) to core.gvfs bitmask. When set, allow git worktree commands to run on VFS-enabled repos instead of blocking them with BLOCK_ON_VFS_ENABLED.
Force --no-checkout during worktree add when VFS is active so ProjFS can be mounted before files are projected.
Support skip-clean-check marker file in worktree gitdir: if .git/worktrees//skip-clean-check exists, skip the cleanliness check during worktree remove. This allows VFSForGit's pre-command hook to unmount ProjFS after its own status check, then let git proceed without re-checking (which would fail without the virtual filesystem).
The corresponding change in VFSForGit is microsoft/VFSForGit#1911