bug#47382: runtest doesn't work with Solaris 10 /bin/sh

[Jacob Bachmeyer]

Rainer Orth wrote:
> Jacob Bachmeyer <jcb62281@gmail.com> writes:
>
>   
> > I had expected that some system out there might do this when I resisted
> > pressure to more aggressively use newer shell constructs in the 
> > "dejagnu" multipurpose launcher script.
> >
> > The previous maintainer had changed this because shellcheck complains about
> > the old `backticks` form.  I have chosen rework the script to return to ``
> > instead of $() and `expr ...` instead of $((...)) before rc2.
> >
> > As I am unable to test on Solaris, these reports are particularly helpful.
> >     
>
> As it happens, you could get access: there are gcc210 (Solaris 10) and
> gcc211 (Solaris 11.3) in the GCC compile farm.  Not saying you should,
> of course, it's just an option.
>   

I will look into it.

> [...]
> > This issue should be fixed in commit
> > e3b14d8555c5cabad03b9ccaa6aa1976ed590201 on the dejagnu-1.6.3 branch.  
> > After the 1.6.3 release, bugfixes on that branch will be forward-ported to
> > master.
> >     
>
> Unfortunately, I got to testing this only now (with rc2), and the
> results are not encouraging, unfortunately:
>
> * first, I get
>
>   runtest: !: not found
>
>   another construct not supported by the original bourne shell.  While
>   looking at this, I noticed that this is in
>
> if ! command -v "$expectbin" > /dev/null ; then
>
>   However, /bin/sh doesn't have an internal command either.  As it
>   happens, an /usr/bin/command exists for POSIX.1 compatibility, which
>   just calls /usr/bin/ksh.  This may be ok in this particular case, but
>   if it works in general from /bin/sh scripts remains a question.
>   

The system providing an external command(1) command should not be a 
problem, even if its implementation using the /usr/bin/ksh builtin is 
somewhat amusing.

This also suggests that we are "almost there" in terms of making runtest 
run with Solaris 10 /bin/sh; the very next command in the script is to 
exec Expect, which gets us away from the shell and its limitations.  
(Give up?  One step away from success?  No!)

The Autoconf shell guidelines specifically say that using ! like that in 
"if" is not portable.  This is fixed in commit 
c35660505e4ec0a79a1ca0e5ea88d6e78caa1778 which applies this patch to 
runtest:

8<----
diff --git a/runtest b/runtest
index b2e0a4c..807a6ae 100755
--- a/runtest
+++ b/runtest
@@ -152,7 +152,7 @@ if [ -z "$runpath" ] ; then
    exit 1
fi

-if ! command -v "$expectbin" > /dev/null ; then
+if command -v "$expectbin" > /dev/null ; then :; else
    echo "ERROR: unable to find expect in the PATH"
    exit 1
fi
8<----

> * There's also
>
> Native configuration is /vol/src/gnu/dejagnu/dejagnu-1.6.3-rc2/config.guess: syn
> tax error at line 35: `me=$' unexpected
>
>   another POSIX shell construct, and less easily avoided because this
>   comes from upstream.
>   

This is a bigger problem.  I do not want to maintain forked config.guess 
and/or config.sub scripts.

> * Next, I had
>
> /vol/src/gnu/dejagnu/dejagnu-1.6.3-rc2/dejagnu: Variants=gawk awk tcl exp bash 
> sh: is not an identifier

This seems to be a minor issue, fixed in commit 
40c351f14a4342d63dcca6a9c72ae81486a1f375 which applies this patch to 
dejagnu:  (but see below)

8<----
diff --git a/dejagnu b/dejagnu
index 695c4e5..44c8962 100755
--- a/dejagnu
+++ b/dejagnu
@@ -53,7 +53,8 @@
# ##end

# list of extensions supported for commands in priority order
-readonly Variants="gawk awk tcl exp bash sh"
+Variants='gawk awk tcl exp bash sh'
+readonly Variants

## Recognize options

8<----


>   I'd already noticed another instance of the problem after I'd sent the
>   original PR: while e.g. autoconf-generated configure and related in
>   gcc are very careful to invoke config.guess only prefixed with $SHELL,
>   runtest.exp exec's it directly, again falling into the trap of POSIX
>   shell constructs (also $(), maybe more).
>
> [...]
>
> Seeing all this suggests to me that my suggestion of substituting
>
> #!@SHELL@
>
> into the DejaGnu shell scripts at build time (and making sure other
> invocations of shell scripts are prefixed with a POSIX shell, too) is a
> safer approach and avoids cripling the scripts with bourne shell constructs
> that cannot even be tested reliably.

So far, none of these changes have been close to crippling the scripts.

Using "#!@SHELL@" is not an option because I want DejaGnu to be able to 
run from the source tree, at least on reasonably modern Free systems.  
The ability to run DejaGnu from a Git checkout should help make up for 
the loss of support for the old Cygnus tree layout.  However, 
"#!/bin/sh" on the first line is similarly unique, so...

>  After all, users of Solaris 10 (or
> any other system without a POSIX /bin/sh) are well accustomed to setting
> CONFIG_SHELL to either /bin/ksh or /bin/bash to avoid issues with the
> ancient bourne shell.
>
> I've tried that by hacking #!/bin/ksh into the dejagnu and runtest
> scripts in the source dir as well as prefixing the exec ... config.guess
> in runtest.exp with /bin/ksh either.  This way, all runtest tests PASS.
>   

... I am considering arranging for configure to patch the shell scripts 
exactly like that.  At minimum, the config.* scripts that get installed 
will need to be patched to use the configure SHELL in their #! lines 
even if I do manage to get the launcher scripts to run under Solaris 10 
/bin/sh.

Since the runtest launcher script has previously run under ancient 
Bourne shells, I consider it failing to run while any systems still use 
such ancient shells to be a regression that needs to be fixed.  The 
dejagnu launcher script is newer and more complex, so it may actually 
need a newer shell, but (as you have reported) there still seems to be a 
problem with Solaris 10 /bin/ksh.

> However, there are more errors still:
>
>                 === launcher Summary ===
>
> # of expected passes            5
> # of unexpected failures        45
> # of unsupported tests          2
>   

Can you post the launcher.log file?  The dejagnu script is fairly 
simple, and I suspect that I may be able to deduce the causes of those 
failures, especially if they are like the "report-card" failure that 
follows.

>                 === report-card Summary ===
>
> # of unresolved testcases       2
>
> Running /vol/src/gnu/dejagnu/dejagnu-1.6.3-rc2/testsuite/report-card.all/onetest
> .exp ...
> spawn /bin/sh -c cd /vol/gcc/obj/dejagnu/dejagnu-1.6.3-rc2/testsuite/report-card
> .all/onetest && exec /vol/src/gnu/dejagnu/dejagnu-1.6.3-rc2/dejagnu report-card^
> M
> expr: syntax error
> ERROR: could not resolve command dejagnu-report-card
>
> There are obviously more hardcoded uses of /bin/sh here and elsewhere,
> but I haven't looked for those yet.

That hardcoded use of /bin/sh only changes directory and execs 
$LAUNCHER, which should run with /bin/ksh if you have patched its #! line.

The "dejagnu" script is run, but fails to locate the report-card 
subcommand.  I would be interested in the output of "/bin/ksh -x 
./dejagnu report-card" in the source directory, assuming that "-x" 
produces an execution trace from ksh as it does from bash (... and 
likewise for Solaris 10 /bin/sh after applying the patch above to change 
the use of the readonly command).  There are two likely candidates I see 
for this error:  one is an `expr :` match to detect a leading "-" and 
the other is a possibility that `expr $# \> 0` in a while loop test is 
somehow being executed as `expr \> 0` under some condition.  Both of 
these hypotheses imply unexpected behavior if not outright bugs in 
Solaris 10.

There is a possibility here that dejagnu may be tickling a bug in 
Solaris 10 /bin/ksh and may work with /bin/sh, or may not work with 
either of them.


-- Jacob