Re: [PATCH 1/2] t1404: add a bunch of tests of D/F conflicts

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Oct 25, 2017, 08:03 UTC
Message-ID: <e66694a3-98e2-33aa-4cdd-dac41d2a89d5@alum.mit.edu>
In-Reply-To: <20171024194634.pigmvgoqtdexvjhc@sigill.intra.peff.net>

On 10/24/2017 09:46 PM, Jeff King wrote:

Show 17 quoted lines

> On Tue, Oct 24, 2017 at 12:19:26PM -0400, Eric Sunshine wrote:
> 
>> On Tue, Oct 24, 2017 at 11:16 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>>> diff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh
>>> @@ -34,6 +34,86 @@ test_update_rejected () {
>>> +# Test adding and deleting D/F-conflicting references in a single
>>> +# transaction.
>>> +df_test() {
>>> +       local prefix="$1"
>>> +       shift
>>> +       local pack=:
>>
>> Isn't "local" a Bash-ism we want to keep out of the test scripts?
> 
> Yeah. It's supported by dash and many other shells, but we do try to
> avoid it[1]. I think in this case we could just drop it (but keep
> setting the "local foo" ones to empty with "foo=".

I do wish that we could allow "local", as it avoids a lot of headaches and potential breakage. According to [1],

> However, every single POSIX-compliant shell I've tested implements the
> 'local' keyword, which lets you declare variables that won't be
> returned from the current function. So nowadays you can safely count
> on it working.

He mentions that ksh93 doesn't support "local", but that it differs from POSIX in many other ways, too.

Perhaps we could slip in a couple of "local" as a compatibility test to see if anybody complains, like we did with a couple of C features recently.

But I agree that this bug fix is not the correct occasion to change policy on something like this, so I'll reroll without "local".

Michael

[1] http://apenwarr.ca/log/?m=201102#28