Re: [PATCH v2] CodingGuidelines: explicitly allow "local" for test scripts
- From
Jeff King <peff@peff.net>
- Date
- May 5, 2021, 14:16 UTC
- Message-ID
- <YJKo02fG/riIetpu@coredump.intra.peff.net>
- In-Reply-To
- <6092180651fdc_105ac2088a@natae.notmuch>
On Tue, May 04, 2021 at 10:59:02PM -0500, Felipe Contreras wrote:
Show 21 quoted lines
> > I believe there's a bash compatibility mode that enables this, but I
> > don't see support for it in the version Debian ships[0], which doesn't
> > enable that mode:
> >
> > ksh -c 'function f { local x="foo"; echo $x; }; f'
> > ksh: f[1]: local: not found [No such file or directory]
>
> I tried to build ksh myself to make sure, and apparently there's a mess
> of different versions of it. I manually applied some patches and tried
> three times before giving up.
>
> It's clear the Debian version and the Arch Linux version are completely
> different.
>
> > Regardless of the specifics, I think we can both agree that it doesn't
> > work in sh-style functions, so for Git's purposes, AT&T ksh does not
> > meet our needs in terms of support for local.
>
> Indeed. But it also probably doesn't meet many other needs. A quick
> check shows 54 failed tests just on t0000-basic.sh. I don't think it's
> something we should even consider.Yeah, we have visited this off and on over the years (links below). There are some modernized ksh variants that work OK (like mksh), but I think we've declared ksh93 as not worth it.
-Peff
[0] https://lore.kernel.org/git/31108626.20150508231514@gmail.com/ [1] https://lore.kernel.org/git/xmqqinxt3kwq.fsf@gitster.mtv.corp.google.com/