Re: rev-parse --show-toplevel broken during exec'ed rebase?

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Jul 19, 2018, 22:09 UTC
Message-ID: <20180719220929.6550-1-szeder.dev@gmail.com>
In-Reply-To: <nycvar.QRO.7.76.6.1807192225260.71@tvgsbejvaqbjf.bet>

Show 29 quoted lines

> On Mon, 16 Jul 2018, Junio C Hamano wrote:
> 
> > Jeff King <peff@peff.net> writes:
> > 
> > > None of which is too surprising. The root of the bug is in the
> > > conversion to rebase--helper, I think, when presumably we started
> > > setting GIT_DIR at all (but I didn't dig further). Then 09d7b6c6fa fixed
> > > _one_ fallout of that, which was relative paths, but didn't help the
> > > subdirectory case.
> > >
> > > Just reading over this thread, I suspect the simplest fix is to pass
> > > GIT_DIR and GIT_WORK_TREE together, which is almost always the right
> > > thing to do.
> > 
> > Perhaps.  Not exporting GIT_DIR (unless the end-user already did to
> > the environment before starting "git rebase"---it would be a bad
> > change to unexport it unconditionally) may probably be a way to make
> > rebase--helper conversion more faithful to the original scripted
> > Porcelain, but I suspect in practice always giving GIT_DIR and
> > GIT_WORK_TREE would work well for many existing hooks.
> 
> Forgetting the code in git-sh-setup, are we?
> 
> git_dir_init() rather specifically set GIT_DIR to the absolute path, and
> since that variable is already exported, the `exec` commands launched via
> `git-rebase--interactive` all saw it.
> 
> That is the reason why the sequencer.c was taught to set GIT_DIR to an
> absolute path rathern than not setting it: for backwards compatibility.

GIT_DIR was not exported to 'exec' commands during an interactive rebase prior to 18633e1a22 (rebase -i: use the rebase--helper builtin, 2017-02-09) (nor was GIT_PREFIX):

  $ git log -Sgit_dir_init master git-rebase*.sh
  # Nothing.
  $ git checkout 18633e1a22a6^ && make -j4 prefix=/tmp/BEFORE install
  <....>
  $ git checkout 18633e1a22a6 && make -j4 prefix=/tmp/AFTER install
  <....>
  $ GIT_EDITOR='sed -i -e "1ix set |grep ^GIT"' /tmp/BEFORE/bin/git rebase -i HEAD^
  Executing: set |grep ^GIT
  GIT_CHERRY_PICK_HELP=$'\nWhen you have resolved this problem, run "git rebase --continue".\nIf you prefer to skip this patch, run "git rebase --skip" instead.\nTo check out the original branch and stop rebasing, run "git rebase --abort".\n'
  GIT_EDITOR='sed -i -e "1ix set |grep ^GIT"'
  GIT_INTERNAL_GETTEXT_SH_SCHEME=gnu
  GIT_REFLOG_ACTION='rebase -i (start): checkout HEAD^'
  warning: notes ref refs/notes/commits is invalid
  Successfully rebased and updated refs/heads/master.
  $ GIT_EDITOR='sed -i -e "1ix set |grep ^GIT"' /tmp/AFTER/bin/git rebase -i HEAD^
  Executing: set |grep ^GIT
  GIT_CHERRY_PICK_HELP='
  GIT_DIR='.git'
  GIT_EDITOR='sed -i -e "1ix set |grep ^GIT"'
  GIT_INTERNAL_GETTEXT_SH_SCHEME='gnu'
  GIT_PREFIX=''
  GIT_REFLOG_ACTION='rebase -i (start): checkout HEAD^'
  warning: notes ref refs/notes/commits is invalid
  Successfully rebased and updated refs/heads/master.

And then recently came 226c0ddd0d (exec_cmd: RUNTIME_PREFIX on some POSIX systems, 2018-04-10), which then started to export GIT_EXEC_PATH to 'exec' commands as well...