Re: [Bug] Rebase from worktree subdir is broken (was Re: [PATCH v5 07/11] rebase: do not attempt to remove startup_info->original_cwd)

[Elijah Newren <newren@gmail.com>]

On Tue, Jan 25, 2022 at 12:27 PM Glen Choo <chooglen@google.com> wrote:
>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > 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@gmail.com>
> > ---
> >  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!

> 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).

> ---
>  t/t3400-rebase.sh | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
> index 23dbd3c82e..8b8b66538b 100755
> --- a/t/t3400-rebase.sh
> +++ b/t/t3400-rebase.sh
> @@ -416,4 +416,33 @@ 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" &&
> +               # create commit with foo/bar/baz
> +               mkdir -p foo/bar &&
> +               touch foo/bar/baz &&
> +               git add foo/bar/baz &&
> +               git commit -m "add foo/bar/baz" &&
> +               # create commit with a/b/c
> +               mkdir -p a/b &&
> +               touch a/b/c &&
> +               git add a/b/c &&
> +               git commit -m "add a/b/c" &&
> +               # create another branch for our other worktree
> +               git branch other &&
> +               git worktree add ../other-wt other &&
> +               (
> +                       cd ../other-wt &&
> +                       mkdir -p random/dir &&
> +                       (
> +                               cd random/dir &&
> +                               git rebase --onto HEAD^^ HEAD^  # drops the HEAD^ commit
> +                       )
> +               )
> +       )
> +'
> +
>  test_done
> --
> 2.35.0.rc0.227.g00780c9af4-goog