Elijah Newren <newren@xxxxxxxxx> writes: > On Tue, Jan 25, 2022 at 12:27 PM Glen Choo <chooglen@xxxxxxxxxx> wrote: >> >> "Elijah Newren via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: >> >> > From: Elijah Newren <newren@xxxxxxxxx> >> > >> > Since rebase spawns a `checkout` subprocess, make sure we run that from >> > the startup_info->original_cwd directory, so that the checkout process >> > knows to protect that directory. >> > >> > Signed-off-by: Elijah Newren <newren@xxxxxxxxx> >> > --- >> > sequencer.c | 2 ++ >> > t/t2501-cwd-empty.sh | 4 ++-- >> > 2 files changed, 4 insertions(+), 2 deletions(-) >> > >> > diff --git a/sequencer.c b/sequencer.c >> > index ea96837cde3..83f257e7fa4 100644 >> > --- a/sequencer.c >> > +++ b/sequencer.c >> > @@ -4228,6 +4228,8 @@ static int run_git_checkout(struct repository *r, struct replay_opts *opts, >> > >> > cmd.git_cmd = 1; >> > >> > + if (startup_info->original_cwd) >> > + cmd.dir = startup_info->original_cwd; >> > strvec_push(&cmd.args, "checkout"); >> > strvec_push(&cmd.args, commit); >> > strvec_pushf(&cmd.env_array, GIT_REFLOG_ACTION "=%s", action); >> > diff --git a/t/t2501-cwd-empty.sh b/t/t2501-cwd-empty.sh >> > index b1182390ba3..52335a8afe9 100755 >> > --- a/t/t2501-cwd-empty.sh >> > +++ b/t/t2501-cwd-empty.sh >> > @@ -166,11 +166,11 @@ test_expect_success 'cherry-pick fails if cwd needs to be removed' ' >> > ' >> > >> > test_expect_success 'rebase does not clean cwd incidentally' ' >> > - test_incidental_dir_removal failure git rebase reverted >> > + test_incidental_dir_removal success git rebase reverted >> > ' >> > >> > test_expect_success 'rebase fails if cwd needs to be removed' ' >> > - test_required_dir_removal failure git rebase fd_conflict >> > + test_required_dir_removal success git rebase fd_conflict >> > ' >> > >> > test_expect_success 'revert does not clean cwd incidentally' ' >> > -- >> > gitgitgadget >> >> This commit (which is already in master) introduces a bug that breaks >> rebase when rebasing inside a subdirectory of a worktree. You can see >> that the below test fails with: >> >> error: The following untracked working tree files would be overwritten by merge: >> a/b/c >> Please move or remove them before you merge. > > Thanks for the detailed report -- with a full testcase! Glad to be of help :) >> This only affects subdirectories in worktrees, i.e. rebasing anywhere in >> the `main-wt` directory is fine, and rebasing from the top of `other-wt` >> is fine, but `other-wt/any/other/dir` fails. >> >> I haven't tracked down the root cause yet, but judging from the commit, >> I would suppose that the checkout is being spawned in the wrong >> directory, causing the files to not be cleaned up. > > There's nothing wrong with running checkout from a subdirectory. It > is unfortunate that setup.c auto-discovers both the git directory and > the working tree, but sets GIT_DIR without setting GIT_WORK_TREE in > the case of a non-main worktree; it's not particularly friendly for > subcommands. Of course, it's also unfortunate that sequencer still > forks subprocesses other than those requested by a user with e.g. > --exec. > > But, anyway, I've got a patch that I'll send as soon as it passes CI > (https://github.com/git/git/pull/1205). I wish I had seen this email before diving into debugging it myself since your fix is more comprehensive, but it was a good learning experience nonetheless.
