Re: [PATCH v2 5/5] ci: add support for GitLab CI

From: Jeff King <peff@peff.net>
Date: Oct 31, 2023, 19:36 UTC
Message-ID: <20231031193629.GB875658@coredump.intra.peff.net>
In-Reply-To: <d00b02e9-fb05-44bc-90ee-1851ef98dd26@gmail.com>

On Fri, Oct 27, 2023 at 02:17:02PM +0100, Phillip Wood wrote:

Show 27 quoted lines

> On 27/10/2023 12:01, Oswald Buddenhagen wrote:
> > On Fri, Oct 27, 2023 at 11:25:41AM +0200, Patrick Steinhardt wrote:
> > > +    export GIT_PROVE_OPTS="--timer --jobs $(nproc)"
> > > +    export GIT_TEST_OPTS="--verbose-log -x"
> > > 
> > fwiw (as this is again only copied), export with assignment is a
> > bash-ism
> 
> Not according to https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#export
> 
>     SYNOPSIS
> 
>         export name[=word]...
> 
>     DESCRIPTION
> 
>         The shell shall give the export attribute to the variables
>         corresponding to the specified names, which shall cause them
>         to be in the environment of subsequently executed commands. If
>         the name of a variable is followed by = word, then the value
>         of that variable shall be set to word.
> 
> It is true that in our test suite we separate a variable assignment when
> exporting. Presumably that is because someone reported that their shell did
> not support the "export name=WORD" syntax in the past. As we're already
> using this syntax with the same docker images in Github Actions I think we
> can assume it is safe here.

I've wondered about the origin of this myself, and tried to do some digging. All of the commits I found removing "export var=val" vaguely say "unportable" or "some shells can't handle", etc.

The oldest mention I found on the mailing list was this thread:

  https://lore.kernel.org/git/7vfyb0wexo.fsf@assigned-by-dhcp.cox.net/

which is even more explicit about its vagueness.

I wouldn't be surprised if SunOS/Solaris /bin/sh had problems with it, as that has been a common headache shell in the past. But I think we finally declared it unusable for other reasons (and they ship a more capable shell in /usr/xpg6, if anybody even still wants to build on those operating systems these days).

So it's possible that avoiding "export var=val" is mostly superstition, and we could loosen our rules these days. But some things to consider:

  1. Some people may prefer reading the separated form (Junio indicates
     such in the thread linked above, but I don't know how strong a
     preference that is).

  2. We won't really know if there is a odd-ball shell that rejects it
     unless we make a change and wait for a while to see if anybody
     screams. The existing ones in ci/ show that it is not a problem for
     the platforms where we run CI, but I suspect the scripts in t/ see
     a wider audience.

I don't think this has any real bearing on the patches being proposed, but I have been curious about this for our other scripts for a while now.

-Peff