On Tue, Jan 25, 2022 at 4:30 PM Glen Choo <chooglen@xxxxxxxxxx> wrote: > > 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. Yeah, sorry, it took me quite a bit of debugging as well before figuring all this stuff out, so I don't think I sent the email in time to save you the work.
