configure shell game

[Darren Tucker]

Hi.

I was recently reminded of this nonsense from the Solaris /bin/sh while
running a regression test:

run test proxy-connect.sh ...
[...]/proxy-connect.sh: warning: line 6: `...` obsolete, use $(...)
[...]/proxy-connect.sh: warning: line 6: `...` obsolete, use $(...)
[...]/proxy-connect.sh: warning: line 15: `...` obsolete, use $(...)

One way to fix that is to use a shell that doesn't suck, of which there
are several candidates.  Configure even already finds them: I started
on this a while back but chickened out as it was very close to a release
at the time.

There are some potential complications.  Configure has several SHELL
variables, which AFAICT are:

1) SHELL: /bin/sh or equivalent, set real early in the guts of configure.

2) CONFIG_SHELL: The shell configure will feed itself to if it decides, or
   is told, that /bin/sh is insufficiently capable to run it.

3) TEST_MINUS_S_SH: one we set configure, which is used for the sole
   purpose of doing a single 'test -S [socket]' on what may be an prngd
   socket.

4) SH: one I added that gets passed to SHELL in the Makefile when I started
   this the first time.

5) TEST_SHELL: this is the shell the regression tests use to run themselves.

Unfortutely someone (almost certainly me) did this:

AC_SUBST([TEST_SHELL], [sh])

and

TEST_SHELL=$SHELL  # let configure find us a capable shell

which sorta works, but is often the least-optimal of the candidates (#1)
for running the regression tests.

I want to merge #3 into #4 and use that instead.  I also want to rename
it, since while 'SH' is relatively accurate, it's *real* hard to grep
for in a project named "OpenSSH".

Can anyone see any down sides of doing this?

diff --git a/.github/configs b/.github/configs
index a7bb698e2..5131e65b3 100755
--- a/.github/configs
+++ b/.github/configs
@@ -279,9 +279,7 @@ case "${TARGET_HOST}" in
 	# These are slow real or virtual machines so skip the slowest tests
 	# (which tend to be thw ones that transfer lots of data) so that the
 	# test run does not time out.
-	# The agent-restrict test fails due to some quoting issue when run
-	# with sh or ksh so specify bash for now.
-	TEST_TARGET="t-exec unit TEST_SHELL=bash"
+	TEST_TARGET="t-exec unit"
 	SKIP_LTESTS="rekey sftp"
 	;;
     dfly58*|dfly60*)
diff --git a/Makefile.in b/Makefile.in
index 730e598d0..19cceb150 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -1,4 +1,4 @@
-SHELL=@SH@
+SHELL=@POSIX_SHELL@
 
 AUTORECONF=autoreconf
 
@@ -30,7 +30,7 @@ SSH_SK_HELPER=$(libexecdir)/ssh-sk-helper
 PRIVSEP_PATH=@PRIVSEP_PATH@
 SSH_PRIVSEP_USER=@SSH_PRIVSEP_USER@
 STRIP_OPT=@STRIP_OPT@
-TEST_SHELL=@TEST_SHELL@
+TEST_SHELL=@POSIX_SHELL@
 BUILDDIR=@abs_top_builddir@
 SK_STANDALONE=@SK_STANDALONE@
 COMPATINCLUDES="$(BUILDDIR)/@COMPATINCLUDES@"
diff --git a/configure.ac b/configure.ac
index 530c8e316..36a6b34d5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -53,17 +53,11 @@ AC_CHECK_TOOLS([AR], [ar])
 AC_PATH_PROG([CAT], [cat])
 AC_PATH_PROG([KILL], [kill])
 AC_PATH_PROG([SED], [sed])
-AC_PATH_PROG([TEST_MINUS_S_SH], [bash])
-AC_PATH_PROG([TEST_MINUS_S_SH], [ksh])
-AC_PATH_PROG([TEST_MINUS_S_SH], [sh])
-AC_PATH_PROG([SH], [bash])
-AC_PATH_PROG([SH], [ksh])
-AC_PATH_PROG([SH], [sh])
+AC_PATH_PROGS([POSIX_SHELL], [bash ksh sh])
 AC_PATH_PROG([GROFF], [groff])
 AC_PATH_PROGS([NROFF], [nroff awf])
 AC_PATH_PROG([MANDOC], [mandoc])
 AC_PATH_TOOL([PKGCONFIG], [pkg-config], [no])
-AC_SUBST([TEST_SHELL], [sh])
 
 dnl select manpage formatter to be used to build "cat" format pages.
 if test "x$MANDOC" != "x" ; then
@@ -1282,7 +1276,6 @@ if (setsockopt(s, IPPROTO_IP, IP_TOS, &one, sizeof(one)) == -1)
 		fi
 		],
 	)
-	TEST_SHELL=$SHELL	# let configure find us a capable shell
 	;;
 *-*-sunos4*)
 	CPPFLAGS="$CPPFLAGS -DSUNOS4"
@@ -1327,7 +1320,6 @@ if (setsockopt(s, IPPROTO_IP, IP_TOS, &one, sizeof(one)) == -1)
 	AC_DEFINE([BROKEN_SETREGID])
 	AC_DEFINE([PASSWD_NEEDS_USERNAME], [1], [must supply username to passwd])
 	AC_DEFINE([LOCKED_PASSWD_STRING], ["*LK*"])
-	TEST_SHELL=$SHELL	# let configure find us a capable shell
 	;;
 # UnixWare 7.x, OpenUNIX 8
 *-*-sysv5*)
@@ -1340,7 +1332,6 @@ if (setsockopt(s, IPPROTO_IP, IP_TOS, &one, sizeof(one)) == -1)
 	AC_DEFINE([BROKEN_SETREGID])
 	AC_DEFINE([PASSWD_NEEDS_USERNAME])
 	AC_DEFINE([BROKEN_TCGETATTR_ICANON])
-	TEST_SHELL=$SHELL	# let configure find us a capable shell
 	case "$host" in
 	*-*-sysv5SCO_SV*)	# SCO OpenServer 6.x
 		maildir=/var/spool/mail
@@ -1379,7 +1370,6 @@ if (setsockopt(s, IPPROTO_IP, IP_TOS, &one, sizeof(one)) == -1)
 	AC_DEFINE([PASSWD_NEEDS_USERNAME])
 	AC_CHECK_FUNCS([getluid setluid])
 	MANTYPE=man
-	TEST_SHELL=$SHELL	# let configure find us a capable shell
 	SKIP_DISABLE_LASTLOG_DEFINE=yes
 	;;
 *-dec-osf*)
@@ -3649,7 +3639,7 @@ AC_ARG_WITH([prngd-socket],
 			AC_MSG_CHECKING([for PRNGD/EGD socket])
 			# Insert other locations here
 			for sock in /var/run/egd-pool /dev/egd-pool /etc/entropy; do
-				if test -r $sock && $TEST_MINUS_S_SH -c "test -S $sock -o -p $sock" ; then
+				if test -r $sock && $POSIX_SHELL -c "test -S $sock -o -p $sock" ; then
 					PRNGD_SOCKET="$sock"
 					AC_DEFINE_UNQUOTED([PRNGD_SOCKET], ["$PRNGD_SOCKET"])
 					break;
@@ -5908,6 +5898,7 @@ CFLAGS_NOPIE=`echo "$CFLAGS" | sed 's/^-fPIE //;s/ -fPIE//g'`
 AC_SUBST([LDFLAGS_NOPIE])
 AC_SUBST([CFLAGS_NOPIE])
 AC_SUBST([COMPATINCLUDES])
+AC_SUBST([POSIX_SHELL])
 
 AC_EXEEXT
 AC_CONFIG_FILES([Makefile buildpkg.sh opensshd.init openssh.xml \

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA
    Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.