sequencer, stash: fix running from worktree subdir
PATCH 1/1 sequencer, stash: fix running from worktree subdir
From: Elijah Newren <[email protected]>
In commits bc3ae46b42 ("rebase: do not attempt to remove startup_info->original_cwd", 2021-12-09) and 0fce211ccc ("stash: do not attempt to remove startup_info->original_cwd", 2021-12-09), we wanted to allow the subprocess to know which directory the parent process was running from, so that the subprocess could protect it. However...
When run from a non-main worktree, setup_git_directory() will note that the discovered git directory (/PATH/TO/.git/worktree/non-main-worktree) does not match DEFAULT_GIT_DIR_ENVIRONMENT (see setup_discovered_git_dir()), and decide to set GIT_DIR in the environment. This matters because...
Whenever git is run with the GIT_DIR environment variable set, and GIT_WORK_TREE not set, it presumes that '.' is the working tree. So...
This combination results in the subcommand being very confused about the working tree. Fix it by also setting the GIT_WORK_TREE environment variable along with setting cmd.dir.
A possibly more involved fix we could consider for later would be to make setup.c set GIT_WORK_TREE whenever (a) it discovers both the git directory and the working tree and (b) it decides to set GIT_DIR in the environment. I did not attempt that here as such would be too big of a change for a 2.35.1 release.
---
[2.35 regression] sequencer, stash: fix running from worktree subdir
Sorry for the regression in Git 2.35; thanks for the report Glen (and Eric for feedback on earlier patches we've been bouncing back and forth that I've incorporated here)
Previous discussion thread starting over here:
https://lore.kernel.org/git/[email protected]/
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1205%2Fnewren%2Ffix-rebase-subdir-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1205/newren/fix-rebase-subdir-v1
Pull-Request: https://github.com/git/git/pull/1205
builtin/stash.c | 6 +++++- sequencer.c | 5 ++++- t/t3400-rebase.sh | 21 +++++++++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-)
builtin/stash.c +5 -1
@@ -1539,8 +1539,12 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int qstruct child_process cp =CHILD_PROCESS_INIT ; cp.git_cmd =1 ; -if (startup_info->original_cwd) +if (startup_info->original_cwd) { cp.dir = startup_info->original_cwd; +strvec_pushf (&cp.env_array,"%s=%s" , +GIT_WORK_TREE_ENVIRONMENT , + the_repository->worktree); + }strvec_pushl (&cp.args,"clean" ,"--force" ,"--quiet" ,"-d" ,":/" ,NULL );if (include_untracked ==INCLUDE_ALL_FILES )
sequencer.c +4 -1
@@ -4223,8 +4223,11 @@ static int run_git_checkout(struct repository *r, struct replay_opts *opts, cmd.git_cmd =1 ; -if (startup_info->original_cwd) +if (startup_info->original_cwd) { cmd.dir = startup_info->original_cwd; +strvec_pushf (&cmd.env_array,"%s=%s" , +GIT_WORK_TREE_ENVIRONMENT , r->worktree); + }strvec_push (&cmd.args,"checkout" );strvec_push (&cmd.args, commit);strvec_pushf (&cmd.env_array,GIT_REFLOG_ACTION "=%s" , action);
t/t3400-rebase.sh +21 -0
@@ -416,4 +416,25 @@ test_expect_success MINGW,SYMLINKS_WINDOWS 'rebase when .git/logs is a symlink' mv actual_logs .git/logs ' +test_expect_success 'rebase when inside worktree subdirectory' ' + git init main-wt && + ( + cd main-wt && + git commit --allow-empty -m "initial" && + mkdir -p foo/bar && + test_commit foo/bar/baz && + mkdir -p a/b && + test_commit a/b/c && + # create another branch for our other worktree + git branch other && + git worktree add ../other-wt other && + cd ../other-wt && + # create and cd into a subdirectory + mkdir -p random/dir && + cd random/dir && + # now do the rebase + git rebase --onto HEAD^^ HEAD^ # drops the HEAD^ commit + ) +' + test_done
base-commit: 89bece5c8c96f0b962cfc89e63f82d603fd60bed
Signature · 1 line
gitgitgadget
Quoted 21 lines · Elijah Newren via GitGitGadget
From: Elijah Newren <[email protected]>
In commits
bc3ae46b42("rebase: do not attempt to removestartup_info->original_cwd", 2021-12-09) and0fce211ccc("stash: do not attempt to removestartup_info->original_cwd", 2021-12-09), we wanted to allow the subprocess to know which directory the parent process was running from, so that the subprocess could protect it. However...When run from a non-main worktree,
setup_git_directory()will note that the discovered git directory (/PATH/TO/.git/worktree/non-main-worktree) does not matchDEFAULT_GIT_DIR_ENVIRONMENT(seesetup_discovered_git_dir()), and decide to setGIT_DIRin the environment. This matters because...Whenever git is run with the
GIT_DIRenvironment variable set, andGIT_WORK_TREEnot set, it presumes that '.' is the working tree. So...This combination results in the subcommand being very confused about the working tree. Fix it by also setting the
GIT_WORK_TREEenvironment variable along with setting cmd.dir.
Quoted 5 lines
A possibly more involved fix we could consider for later would be to make
setup.csetGIT_WORK_TREEwhenever (a) it discovers both the git directory and the working tree and (b) it decides to setGIT_DIRin the environment. I did not attempt that here as such would be too big of a change for a 2.35.1 release.
As the commit message explains, GIT_DIR and GIT_WORK_TREE are closely linked, and this interaction is subtle enough that we'd want to guard against it instead of policing it manually. So, yes, setting them together makes a lot of sense.
Quoted 60 lines
builtin/stash.c | 6 +++++- sequencer.c | 5 ++++- t/t3400-rebase.sh | 21 +++++++++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-)builtin/stash.c +5
-1@@ -1539,8 +1539,12 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int qstruct child_process cp =CHILD_PROCESS_INIT ; cp.git_cmd =1 ; -if (startup_info->original_cwd) +if (startup_info->original_cwd) { cp.dir = startup_info->original_cwd; +strvec_pushf (&cp.env_array,"%s=%s" , +GIT_WORK_TREE_ENVIRONMENT , + the_repository->worktree); + }strvec_pushl (&cp.args,"clean" ,"--force" ,"--quiet" ,"-d" ,":/" ,NULL );if (include_untracked ==INCLUDE_ALL_FILES )sequencer.c +4
-1@@ -4223,8 +4223,11 @@ static int run_git_checkout(struct repository *r, struct replay_opts *opts, cmd.git_cmd =1 ; -if (startup_info->original_cwd) +if (startup_info->original_cwd) { cmd.dir = startup_info->original_cwd; +strvec_pushf (&cmd.env_array,"%s=%s" , +GIT_WORK_TREE_ENVIRONMENT , r->worktree); + }strvec_push (&cmd.args,"checkout" );strvec_push (&cmd.args, commit);strvec_pushf (&cmd.env_array,GIT_REFLOG_ACTION "=%s" , action);t/t3400-rebase.sh +12
-0@@ -416,4 +416,25 @@ test_expect_success MINGW,SYMLINKS_WINDOWS 'rebase when .git/logs is a symlink' mv actual_logs .git/logs ' +test_expect_success 'rebase when inside worktree subdirectory' ' + git init main-wt && + ( + cd main-wt && + git commit --allow-empty -m "initial" && + mkdir -p foo/bar && + test_commit foo/bar/baz && + mkdir -p a/b && + test_commit a/b/c && + # create another branch for our other worktree + git branch other && + git worktree add ../other-wt other &&
t/t3400-rebase.sh test_expect_success MINGW,SYMLINKS_WINDOWS 'rebase when .git/logs is a symlink' · line 440
Quoted 10 lines
t/t3400-rebase.sh +9
-0+ cd ../other-wt && + # create and cd into a subdirectory + mkdir -p random/dir && + cd random/dir && + # now do the rebase + git rebase --onto HEAD^^ HEAD^ # drops the HEAD^ commit + ) +' + test_done
Looks good :)
Quoted 27 lines · Elijah Newren via GitGitGadget
From: Elijah Newren <[email protected]>
In commits
bc3ae46b42("rebase: do not attempt to removestartup_info->original_cwd", 2021-12-09) and0fce211ccc("stash: do not attempt to removestartup_info->original_cwd", 2021-12-09), we wanted to allow the subprocess to know which directory the parent process was running from, so that the subprocess could protect it. However...When run from a non-main worktree,
setup_git_directory()will note that the discovered git directory (/PATH/TO/.git/worktree/non-main-worktree) does not matchDEFAULT_GIT_DIR_ENVIRONMENT(seesetup_discovered_git_dir()), and decide to setGIT_DIRin the environment. This matters because...Whenever git is run with the
GIT_DIRenvironment variable set, andGIT_WORK_TREEnot set, it presumes that '.' is the working tree. So...This combination results in the subcommand being very confused about the working tree. Fix it by also setting the
GIT_WORK_TREEenvironment variable along with setting cmd.dir.A possibly more involved fix we could consider for later would be to make
setup.csetGIT_WORK_TREEwhenever (a) it discovers both the git directory and the working tree and (b) it decides to setGIT_DIRin the environment. I did not attempt that here as such would be too big of a change for a 2.35.1 release.
Quoted 4 lines
Test-case-by Glen Choo <[email protected]>Signed-off-by Elijah Newren <[email protected]>---
[2.35 regression] sequencer, stash: fix running from worktree subdir
Thanks. This is the kind of fix we all should be concentrating on in the first week after the release.
Very much appreciated.
Will queue.
Quoted 67 lines
builtin/stash.c +5
-1@@ -1539,8 +1539,12 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int qstruct child_process cp =CHILD_PROCESS_INIT ; cp.git_cmd =1 ; -if (startup_info->original_cwd) +if (startup_info->original_cwd) { cp.dir = startup_info->original_cwd; +strvec_pushf (&cp.env_array,"%s=%s" , +GIT_WORK_TREE_ENVIRONMENT , + the_repository->worktree); + }strvec_pushl (&cp.args,"clean" ,"--force" ,"--quiet" ,"-d" ,":/" ,NULL );if (include_untracked ==INCLUDE_ALL_FILES )sequencer.c +4
-1@@ -4223,8 +4223,11 @@ static int run_git_checkout(struct repository *r, struct replay_opts *opts, cmd.git_cmd =1 ; -if (startup_info->original_cwd) +if (startup_info->original_cwd) { cmd.dir = startup_info->original_cwd; +strvec_pushf (&cmd.env_array,"%s=%s" , +GIT_WORK_TREE_ENVIRONMENT , r->worktree); + }strvec_push (&cmd.args,"checkout" );strvec_push (&cmd.args, commit);strvec_pushf (&cmd.env_array,GIT_REFLOG_ACTION "=%s" , action);t/t3400-rebase.sh +21
-0@@ -416,4 +416,25 @@ test_expect_success MINGW,SYMLINKS_WINDOWS 'rebase when .git/logs is a symlink' mv actual_logs .git/logs ' +test_expect_success 'rebase when inside worktree subdirectory' ' + git init main-wt && + ( + cd main-wt && + git commit --allow-empty -m "initial" && + mkdir -p foo/bar && + test_commit foo/bar/baz && + mkdir -p a/b && + test_commit a/b/c && + # create another branch for our other worktree + git branch other && + git worktree add ../other-wt other && + cd ../other-wt && + # create and cd into a subdirectory + mkdir -p random/dir && + cd random/dir && + # now do the rebase + git rebase --onto HEAD^^ HEAD^ # drops the HEAD^ commit + ) +' + test_donebase-commit:
89bece5c8c96f0b962cfc89e63f82d603fd60bed