On 03/14, Johannes Schindelin wrote:
> Hi Brandon,
>
> On Tue, 14 Mar 2017, Brandon Williams wrote:
>
> > In a future patch child processes which act on submodules need a little
> > more context about the original command that was invoked. This patch
> > teaches git to use the prefix stored in `GIT_INTERNAL_TOPLEVEL_PREFIX`
> > if another prefix wasn't found during the git directory setup process.
>
> Missing SOB ;-)
>
Thanks for that catch. I'm expecting some discussion on this patch in
particular (and cc'd you since you've been working on this area of the
code) so I'll make sure to add it in the next reroll.
> > diff --git a/setup.c b/setup.c
> > index 8f64fbdfb..c8492ea8a 100644
> > --- a/setup.c
> > +++ b/setup.c
> > @@ -940,8 +940,14 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
> > const char *setup_git_directory_gently(int *nongit_ok)
> > {
> > const char *prefix;
> > + const char *env_prefix;
>
> I'd just append this to the previous line (`const char *prefix,
> *env_prefix`).
That would be cleaner.
>
> > prefix = setup_git_directory_gently_1(nongit_ok);
> > + env_prefix = getenv(GIT_TOPLEVEL_PREFIX_ENVIRONMENT);
> > +
> > + if (env_prefix)
> > + prefix = env_prefix;
>
> The commit message claims that env_prefix is used if no other prefix was
> found, but this code ignores any prefix if the environment variable was
> set.
>
> Which version is correct?
Well, as you can tell I flip-flopped on what I thought the best course
of action would be. For my intentions (submodule-centric) I don't
believe they would ever both have a value so it doesn't matter to me
which it is. Though future users may want a particular order of
precedence.
>
> Ciao,
> Johannes
--
Brandon Williams