On Sunday, July 14, 2024 1:00 PM, Junio C Hamano wrote: ><rsbecker@xxxxxxxxxxxxx> 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@xxxxxxxxx> >--- > 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 > -------------- 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.
