git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: <rsbecker@nexbridge.com>
Cc: <git@vger.kernel.org>
Subject: Re: [Test Breakage 2.46.0-rc0] Test t0021.35 fails on NonStop
Date: Sun, 14 Jul 2024 10:00:12 -0700	[thread overview]
Message-ID: <xmqqttgr9aeb.fsf@gitster.g> (raw)
In-Reply-To: <001f01dad5f1$e518e6e0$af4ab4a0$@nexbridge.com> (rsbecker@nexbridge.com's message of "Sun, 14 Jul 2024 09:29:41 -0400")

<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", 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 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
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 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
 --------------

  reply	other threads:[~2024-07-14 17:00 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-13 16:38 [Test Breakage 2.46.0-rc0] Test t0021.35 fails on NonStop rsbecker
2024-07-14  2:53 ` Junio C Hamano
2024-07-14 13:29   ` rsbecker
2024-07-14 17:00     ` Junio C Hamano [this message]
2024-07-14 17:29       ` rsbecker
2024-07-14 18:15       ` brian m. carlson
2024-07-14 18:28         ` rsbecker
2024-07-14 22:01           ` brian m. carlson
2024-07-14 22:14             ` rsbecker
2024-07-15 15:20             ` Junio C Hamano
2024-07-15 15:32               ` rsbecker
2024-07-15 16:41                 ` Junio C Hamano
2024-07-15 17:39                   ` rsbecker
2024-07-15 18:33                     ` Junio C Hamano
2024-07-15 19:03                       ` rsbecker
2024-07-15 15:05         ` Junio C Hamano
2024-07-14 21:27       ` Eric Sunshine
2024-07-14 13:58   ` rsbecker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqttgr9aeb.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=rsbecker@nexbridge.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).