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

[Eric Sunshine <sunshine@sunshineco.com>]

On Sat, Feb 5, 2022 at 6:23 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Thu, Jan 27, 2022 at 3:03 PM Elijah Newren <newren@gmail.com> wrote:
> > Some comments on the various code changes:
> >    * clone/push/fetch related:
> >      * there are *many* subprocesses involved in fetch/push and friends,
> >        and they typically need to know the GIT_DIR they are operating on
> >      * this involves: fetch-patch.c, connected.c, bundle.c, clone.c,
> >        transport-helper.c, receive-pack.c, upload-pack.c
> >      * this accounts for the majority of this patch
>
> It does feel a bit like a bandaid to insert new code at these
> locations to set GIT_DIR manually. It's not clear to readers why
> GIT_DIR is needed for these specific cases, nor what the impact is
> when it is not set. Thus, one wonders if such a blanket approach is
> indeed required or if a more narrow and directed fix can be applied,
> such as calling subprograms with an explicit --git-dir= rather than
> setting GIT_DIR with its potentially more broad and
> difficult-to-reason-about impact.

I meant to ask here what was the nature of the various failures you
were seeing without GIT_DIR being set, and whether you had considered
tackling those failures with --git-dir= instead of GIT_DIR. If so,
were the problems too difficult to overcome by --git-dir= alone?
Fleshing out the commit message with such information might be
worthwhile.

By the way, I also didn't mean to imply that the
"difficult-to-reason-about GIT_DIR interaction in relation to
subprograms" problem is new to this patch; it isn't. That problem has
been around for ages (at least since 2007-08-01) but it wasn't as
obvious since the setting of GIT_DIR was so far removed from other
code which runs subprograms, thus readers likely wouldn't be thinking
about GIT_DIR when reading the code which runs subprograms. This patch
only makes the problem more obvious since readers now see the
sequence:

    (1) set GIT_DIR
    (2) launch subprogram

So, a reader is more likely to wonder why GIT_DIR is needed for the
subprogram and what possible wider side-effects it might have.