Your change to ada/cats/run_acats in February

Your change to ada/cats/run_acats in February

[Richard Kenner]

ChangeLog says:

        * ada/acats/run_acats: Run run_all.sh with $SHELL.

Why?  That seems wrong.  If I have $SHELL as /bin/csh, you still want to
run run_all.sh with /bin/sh.  I think it should be "/bin/sh", not $SHELL.

Do you agree?

Re: Your change to ada/cats/run_acats in February

[Rainer Orth]

kenner@vlsi1.ultra.nyu.edu (Richard Kenner) writes:

> ChangeLog says:
>
>         * ada/acats/run_acats: Run run_all.sh with $SHELL.
>
> Why?  That seems wrong.  If I have $SHELL as /bin/csh, you still want to
> run run_all.sh with /bin/sh.  I think it should be "/bin/sh", not $SHELL.
>
> Do you agree?

Not at all.  On the one hand, see the discussion in the patch submission

	PATCH: Fix several Ada testing problems on IRIX (PR ada/32547)
        http://gcc.gnu.org/ml/gcc-patches/2010-01/msg00594.html

on the other hand, running shell scripts with $(SHELL) (which stems from
configure and $CONFIG_SHELL) instead of hardcoding /bin/sh, which may be
limited/buggy on several platforms, is common practice throughout the
tree.

SHELL is explicitly set in the Makefiles, and unless you run make -e to
override that from the environment, there's no problem.  Otherwise, you
wouldn't be able to build GCC with some csh-family login shell at all.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: Your change to ada/cats/run_acats in February

[Richard Kenner]

> Not at all.  On the one hand, see the discussion in the patch submission
> 
> 	PATCH: Fix several Ada testing problems on IRIX (PR ada/32547)
>         http://gcc.gnu.org/ml/gcc-patches/2010-01/msg00594.html
> 
> on the other hand, running shell scripts with $(SHELL) (which stems from
> configure and $CONFIG_SHELL) instead of hardcoding /bin/sh, which may be
> limited/buggy on several platforms, is common practice throughout the
> tree.
> 
> SHELL is explicitly set in the Makefiles, and unless you run make -e to
> override that from the environment, there's no problem.  Otherwise, you
> wouldn't be able to build GCC with some csh-family login shell at all.

Well, it doesn't seem to be in the case in this specific file!  I have
SHELL set to /bin/csh and if I just do "make check", it fails unless I
change that line from $SHELL to /bin/sh.  And indeed if I add the line
"echo $SHELL" to the front of that file and say "make check-acats", it
echos "/bin/csh".

And I understand why: what's set properly is the *make* variable $(SHELL)
and that is indeed fine.  But this is the *shell* variable $SHELL, which
nobody touches!

So how about adding a arg to run_acats where make passes in its $(SHELL)
and that's used in the last line?

Re: Your change to ada/cats/run_acats in February

[Eric Botcazou]

> So how about adding a arg to run_acats where make passes in its $(SHELL)
> and that's used in the last line?

Let's remove this $SHELL thing, it already wasn't necessary when it was added.

-- 
Eric Botcazou

Re: Your change to ada/cats/run_acats in February

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 269 bytes --]

> Let's remove this $SHELL thing, it already wasn't necessary when it was
> added.

Tested on i586-suse-linux, applied on mainline and 4.5 branch.


2010-09-30  Eric Botcazou  <ebotcazou@adacore.com>

	* ada/acats/run_acats: Revert revision 157037.


-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-diff, Size: 361 bytes --]

Index: ada/acats/run_acats
===================================================================
--- ada/acats/run_acats	(revision 164716)
+++ ada/acats/run_acats	(working copy)
@@ -70,4 +70,4 @@ chmod +x host_gnatmake
 # Limit the stack to 16MB for stack checking
 ulimit -s 16384
 
-exec $SHELL $testdir/run_all.sh ${1+"$@"}
+exec $testdir/run_all.sh ${1+"$@"}