Re: [PATCH 1/9] init: allow overriding the default branch name for new repositories

From: Jeff King <peff@peff.net>
Date: Jun 16, 2020, 12:25 UTC
Message-ID: <20200616122559.GA666057@coredump.intra.peff.net>
In-Reply-To: <nycvar.QRO.7.76.6.2006141055510.56@tvgsbejvaqbjf.bet>

On Sun, Jun 14, 2020 at 10:57:41AM +0200, Johannes Schindelin wrote:

Show 33 quoted lines

> > >> Why adding yet another environment variable instead of relying only on a
> > >> config option?  I understand it's for the tests, but can't we add a
> > >> shell function in test-lib.sh (and friends) that tries to read
> > >> `GIT_TEST_DEFAULT_BRANCH_NAME', and, if it exists, sets
> > >> `core.defaultBranchName'?
> > >
> > > Can you produce such a patch that does it cleanly?  My knee jerk
> > > reaction is that I would suspect that you end up having to touch
> > > many places in the t/ scripts, but if you prove otherwise, that
> > > would certainly be appreciated.
> > >
> > > And no,
> > >
> > >     git () { command git -c core.defaultBranchName=master "$@" }
> > >
> > > is not an acceptable solution.
> > >
> >
> > I wanted to to do something like this:
> >
> >   if test -n "$GIT_TEST_DEFAULT_BRANCH_NAME";
> >   then
> >       git config core.defaultBranchName "$GIT_TEST_DEFAULT_BRANCH_NAME"
> >   fi
> >
> > But since we do not have a repository to store the config, it won't
> > work.  Sorry for the noise.
> 
> We actually would have `~/.gitconfig` because `HOME` is set to `t/trash
> directory.<test-name>/`.
> 
> However, that would cause all kinds of issues when test scripts expect the
> directory to be pristine, containing only `.git/` but not `.gitconfig`.

Putting:

  GIT_CONFIG_PARAMETERS="'core.defaultBranchName=...'"

into the environment would work (and yes, you need the single quotes embedded in the variable), and solves all of the complaints above. Further "git -c" invocations properly append to it. But:

  - there are a few tests which explicitly tweak that variable

  - it technically changes any tests of "-c" because now we'd never
    cover the case where we start without the variable defined

I think baking in a special environment variable like you have is not so bad. If this did become too common a pattern, though (special test-only environment variables that do have a separate config option), I wouldn't be opposed to a GIT_TEST_CONFIG_PARAMETERS which takes precedence over other config, and comes with a big warning label that it shouldn't be relied upon outside the test suite. That's equally ugly to GIT_TEST_DEFAULT_BRANCH_NAME, but at least solves the problem once for all of them. I'm just not sure we have enough "all of them" to make it worth doing.

-Peff