RE: [Test Breakage 2.46.0-rc0] Test t0021.35 fails on NonStop

From: rsbecker@nexbridge.com <rsbecker@nexbridge.com>
Date: Jul 14, 2024, 17:29 UTC
Message-ID: <003301dad613$62b40aa0$281c1fe0$@nexbridge.com>
In-Reply-To: <xmqqttgr9aeb.fsf@gitster.g>

On Sunday, July 14, 2024 1:00 PM, Junio C Hamano wrote:

Show 7 quoted lines

><rsbecker@nexbridge.com> writes:
>
>> This looks like a different between ksh and bash. Under bash, the test
>> works. I can live with that but will have to force bash to be used as
>> the shebang #!/bin/sh defaults to ksh on this box.
>
>It turns out that the version of ksh I used in my description does not seem

to grok

>"local" at all. I vaguely recall that we've written off various hobbist
>reimplementation of ksh as unusable enough, but this one is ksh93 direct

from

>AT&T Research.
>
>I guess when we said "as long as we limit our use to a simple 'this

variable has

>visibility limited to the function and its children'
>and nothing else, it is portable enough across practically everybody we

care about",

Show 9 quoted lines

>we have written off the real ksh, too.
>
>In the meantime, we may want to document this in a more prominent way.
>Perhaps like so:
>
>-------- >8 --------------- >8 --------------- >8 --------
>Subject: doc: guide to use of "local" shell language construct
>
>The scripted Porcelain commands do not allow use of "local" because it is

not

>universally supported, but we use it liberally in our test scripts, which

means some

>POSIX compliant shells (like "ksh93") can not be used to run our tests.
>
>Document the status quo, and hint that we might want to change the

situation in

Show 9 quoted lines

>the fiture.
>
>Signed-off-by: Junio C Hamano <gitster@pobox.com>
>---
> Documentation/CodingGuidelines | 4 +++-
> t/README                       | 8 ++++++++
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
>diff --git c/Documentation/CodingGuidelines

w/Documentation/CodingGuidelines

Show 14 quoted lines

>index 1d92b2da03..68b7210f48 100644
>--- c/Documentation/CodingGuidelines
>+++ w/Documentation/CodingGuidelines
>@@ -186,7 +186,9 @@ For shell scripts specifically (not exhaustive):
>  - Even though "local" is not part of POSIX, we make heavy use of it
>    in our test suite.  We do not use it in scripted Porcelains, and
>    hopefully nobody starts using "local" before they are reimplemented
>-   in C ;-)
>+   in C ;-) Notably, ksh (not just reimplementations but the real one
>+   from AT&T Research) does not support "local" and cannot be used,
>+   which we might want to reconsider.
>
>  - Some versions of shell do not understand "export variable=value",
>    so we write "variable=value" and then "export variable" on two diff

--git

Show 18 quoted lines

>c/t/README w/t/README index d9e0e07506..1d39d8cfd5 100644
>--- c/t/README
>+++ w/t/README
>@@ -850,6 +850,14 @@ And here are the "don'ts:"
>    but the best indication is to just run the tests with prove(1),
>    it'll complain if anything is amiss.
>
>+ - Don't overuse "local"
>+
>+   Because strictly POSIX-compliant shells do not have to support
>+   "local", we avoid using it in our scripted Porcelain scripts, but
>+   we have allowed use of "local" in test scripts.  We may want to
>+   reconsider this and rewrite our tests to also run on shells like
>+   ksh93.  Do not add new use of "local" unnecessarily.
>+
>
> Skipping tests
> --------------

Thanks. I approve. I'm currently working on trying to get the test suite to run under bash. It looks like TEST_SHELL_PATH is not propagated to the inner make -C test. My current approach is to run the inner make without the outer make. Otherwise I am forced to use ksh, which is known not to work. Will advice when this runs - I have to rebuild, and that takes about an hour.