Re: [PATCH v2] sequencer: pass absolute GIT_WORK_TREE to exec commands

From: Jeff King <peff@peff.net>
Date: Jul 14, 2018, 21:19 UTC
Message-ID: <20180714211950.GA6430@sigill.intra.peff.net>
In-Reply-To: <20180714183859.97988-1-sandals@crustytoothpaste.net>

On Sat, Jul 14, 2018 at 06:38:59PM +0000, brian m. carlson wrote:

Show 13 quoted lines

> The sequencer currently passes GIT_DIR, but not GIT_WORK_TREE, to exec
> commands.  In that configuration, we assume that whatever directory
> we're in is the top level of the work tree, and git rev-parse
> --show-toplevel responds accordingly.  However, when we're in a
> subdirectory, that isn't correct: we respond with the subdirectory as
> the top level, resulting in unexpected behavior.
> 
> Ensure that we pass GIT_WORK_TREE as well as GIT_DIR so that git
> operations within subdirectories work correctly.
> 
> Note that we are guaranteed to have a work tree in this case: the
> relevant sequencer functions are called only from revert, cherry-pick,
> and rebase--helper; all of these commands require a working tree.

Thanks for digging into this corner case.

It's possible, of course, that some day we may have a caller without a work-tree, but it seems pretty unlikely for sequencer code. If we did, then:

Show 10 quoted lines

> diff --git a/sequencer.c b/sequencer.c
> index 5354d4d51e..c8e16f9168 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2636,6 +2636,8 @@ static int do_exec(const char *command_line)
>  	fprintf(stderr, "Executing: %s\n", command_line);
>  	child_argv[0] = command_line;
>  	argv_array_pushf(&child_env, "GIT_DIR=%s", absolute_path(get_git_dir()));
> +	argv_array_pushf(&child_env, "GIT_WORK_TREE=%s",
> +			 absolute_path(get_git_work_tree()));

I think this would just segfault (feeding NULL to absolute_path). That's probably OK, as it would be a big red flag to whomever is trying to teach the sequencer to work in a bare repo. :)

So this version looks good to me.

-Peff