Re: [PATCH 1/9] init: allow overriding the default branch name for new repositories
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jun 18, 2020, 10:17 UTC
- Message-ID
- <nycvar.QRO.7.76.6.2006181216440.54@tvgsbejvaqbjf.bet>
- In-Reply-To
- <20200616122559.GA666057@coredump.intra.peff.net>
Hi Peff,
On Tue, 16 Jun 2020, Jeff King wrote:
Show 48 quoted lines
> On Sun, Jun 14, 2020 at 10:57:41AM +0200, Johannes Schindelin wrote:
>
> > > >> 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 definedIndeed.
Show 9 quoted lines
> 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.
FWIW I do not plan on using that variable for a long time. And it is not in this here patch series any longer, so let's discuss it in the future patch contribution of mine.
Thanks, Dscho