Skip to content

worktree: conditionally allow worktree on VFS-enabled repos #868

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tyrielv wants to merge 1 commit into
base: vfs-2.53.0
Choose a base branch
from
Open

worktree: conditionally allow worktree on VFS-enabled repos #868

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion builtin/worktree.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,14 @@ static int add(int ac, const char **av, const char *prefix,
if (ac < 1 || ac > 2)
usage_with_options(git_worktree_add_usage, options);

/*
* When the virtual file system is active, skip checkout during
* worktree creation. The VFS layer will handle the checkout
* after the worktree structure is set up.
*/
if (gvfs_config_is_set(the_repository, GVFS_USE_VIRTUAL_FILESYSTEM))
opts.checkout = 0;
Copy link
Member

@dscho dscho Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point, we probably also want to prevent a pattern that I, personally, use quite often: Putting worktrees inside the main working directory. That is, in my main microsoft/git checkout, I sometimes call git worktree add --detach for-testing, and the directory for-testing now lives at the top-level directory of the primary worktree.

I think we need to prevent that because nested worktrees are incompatible with VFS, right?

Copy link
Author

@tyrielv tyrielv Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - currently I'm intending to block it in the GVFS pre-command hook but we could block it here if you prefer.


path = prefix_filename(prefix, av[0]);
branch = ac < 2 ? "HEAD" : av[1];
used_new_branch_options = new_branch || new_branch_force;
Expand Down Expand Up @@ -1358,6 +1366,21 @@ static int delete_git_work_tree(struct worktree *wt)
return ret;
}

/*
* Check if a pre-command hook has already verified worktree cleanliness
* and written a marker file to skip git's own check. VFSForGit uses this
* to unmount ProjFS after its own status check; without it, git's status
* call would fail because the virtual filesystem is no longer available.
*/
static int should_skip_clean_check(struct worktree *wt)
{
char *path = repo_common_path(the_repository,
"worktrees/%s/skip-clean-check", wt->id);
int skip = file_exists(path);
free(path);
return skip;
}

static int remove_worktree(int ac, const char **av, const char *prefix,
struct repository *repo UNUSED)
{
Expand Down Expand Up @@ -1397,7 +1420,7 @@ static int remove_worktree(int ac, const char **av, const char *prefix,
strbuf_release(&errmsg);

if (file_exists(wt->path)) {
if (!force)
if (!force && !should_skip_clean_check(wt))
Copy link
Member

@dscho dscho Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to make this independent from VFS? I have a slight preference to guard this !should_skip_clean_check(wt) call behind core_virtualfilesystems, i.e.

if (!force && !(core_virtualfilesystems && should_skip_clean_check(wt)))

check_clean_worktree(wt, av[0]);

ret |= delete_git_work_tree(wt);
Expand Down
3 changes: 2 additions & 1 deletion git.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,8 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv, struct
die("'git %s' is not supported on a GVFS repo", p->cmd);

if (!help && p->option & BLOCK_ON_VFS_ENABLED && gvfs_config_is_set(repo, GVFS_USE_VIRTUAL_FILESYSTEM))
die("'git %s' is not supported when using the virtual file system", p->cmd);
if (strcmp(p->cmd, "worktree") != 0 || !gvfs_config_is_set(repo, GVFS_SUPPORTS_WORKTREES))
die("'git %s' is not supported when using the virtual file system", p->cmd);
Copy link
Member

@dscho dscho Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to add an explicit if (GVFS_SUPPORTS_WORKTREES) die(...) to cmd_worktree() and take away the BLOCK_ON_VFS_ENABLED flag from worktree.


if (run_pre_command_hook(the_repository, argv))
die("pre-command hook aborted command");
Expand Down
7 changes: 7 additions & 0 deletions gvfs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ struct repository;
#define GVFS_BLOCK_FILTERS_AND_EOL_CONVERSIONS (1 << 6)
#define GVFS_PREFETCH_DURING_FETCH (1 << 7)

/*
* When set, this flag indicates that the VFS layer supports
* git worktrees. This allows `git worktree add/remove` to
* operate on VFS-enabled repositories.
*/
#define GVFS_SUPPORTS_WORKTREES (1 << 8)

#define GVFS_ANY_MASK 0xFFFFFFFF

int gvfs_config_is_set(struct repository *r, int mask);
Expand Down
50 changes: 49 additions & 1 deletion t/t0402-block-command-on-gvfs.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,55 @@ not_with_gvfs update-index --index-version 2
not_with_gvfs update-index --skip-worktree
not_with_gvfs update-index --no-skip-worktree
not_with_gvfs update-index --split-index
not_with_gvfs worktree list

# worktree is conditionally allowed: blocked when VFS enabled without
# GVFS_SUPPORTS_WORKTREES, but core.gvfs=true sets all bits including
# SUPPORTS_WORKTREES, so we test the blocked case with a specific bitmask.
test_expect_success 'worktree blocked with VFS but without SUPPORTS_WORKTREES' '
test_config core.gvfs 95 &&
test_must_fail git worktree list 2>err &&
test_grep "not supported when using the virtual file system" err
'

# core.gvfs bitmask values:
# GVFS_USE_VIRTUAL_FILESYSTEM = (1 << 3) = 8
# GVFS_SUPPORTS_WORKTREES = (1 << 8) = 256
# A typical VFS-enabled repo has core.gvfs=95 (bits 0-4,6).
# Adding SUPPORTS_WORKTREES: 95 + 256 = 351.

test_expect_success 'setup for worktree tests' '
test_commit initial
'

test_expect_success 'worktree allowed when SUPPORTS_WORKTREES bit is set' '
test_when_finished "git worktree remove --force ../allowed-wt 2>/dev/null; true" &&
Copy link
Member

@dscho dscho Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would hide legitimate errors, though. Could we use test ! -d ../allowed-wt || git worktree remote --force ../allowed-wt instead, please? That will succeed if the worktree does not exist, and fail if the worktree cannot be removed.

test_config core.gvfs 351 &&
Copy link
Member

@dscho dscho Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

351 is a bit magic, right? How about using $((0x15f)) # everything except GVFS_BLOCK_FILTERS_AND_EOL_CONVERSIONS? Or maybe we can simply turn core.gvfs = true, i.e. GVFS_BLOCK_FILTERS_AND_EOL_CONVERSIONS may be included?

git worktree add ../allowed-wt &&
test_path_exists ../allowed-wt/.git
'

test_expect_success 'worktree add forces --no-checkout when VFS active' '
test_when_finished "git worktree remove --force ../nocheckout-wt 2>/dev/null; true" &&
test_config core.gvfs 351 &&
git worktree add ../nocheckout-wt &&
test_path_exists ../nocheckout-wt/.git &&
! test_path_exists ../nocheckout-wt/initial.t
Copy link
Member

@dscho dscho Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should probable be folded into the preceding test case, it performs the same steps modulo this one (a code comment should help with debugging regressions, if any should occur in the future).

'

test_expect_success 'worktree list works with SUPPORTS_WORKTREES' '
test_when_finished "git worktree remove --force ../list-wt 2>/dev/null; true" &&
test_config core.gvfs 351 &&
git worktree add ../list-wt &&
git worktree list >out &&
grep "list-wt" out
Copy link
Member

@dscho dscho Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fold this into the preceding test case; Shell scripting is quite expensive on Windows, and I'd like to keep the test suite rather tight (a goal I seem not to share with upstream Git).

'

test_expect_success 'worktree remove works with SUPPORTS_WORKTREES' '
test_config core.gvfs 351 &&
git worktree add ../remove-wt &&
git worktree remove --force ../remove-wt &&
! test_path_exists ../remove-wt
Copy link
Member

@dscho dscho Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we fold also this into the preceding test case, please?

'

test_expect_success 'test gc --auto succeeds when disabled via config' '
test_config core.gvfs true &&
Expand Down
Loading