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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Peff,

On Tue, 16 Jun 2020, Jeff King wrote:

> 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 defined

Indeed.

> 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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux