* Your change to ada/cats/run_acats in February
@ 2010-09-28 17:43 Richard Kenner
2010-09-28 18:07 ` Rainer Orth
0 siblings, 1 reply; 5+ messages in thread
From: Richard Kenner @ 2010-09-28 17:43 UTC (permalink / raw)
To: ro; +Cc: gcc-patches
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?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Your change to ada/cats/run_acats in February
2010-09-28 17:43 Your change to ada/cats/run_acats in February Richard Kenner
@ 2010-09-28 18:07 ` Rainer Orth
2010-09-28 18:26 ` Richard Kenner
0 siblings, 1 reply; 5+ messages in thread
From: Rainer Orth @ 2010-09-28 18:07 UTC (permalink / raw)
To: Richard Kenner; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Your change to ada/cats/run_acats in February
2010-09-28 18:07 ` Rainer Orth
@ 2010-09-28 18:26 ` Richard Kenner
2010-09-28 20:16 ` Eric Botcazou
0 siblings, 1 reply; 5+ messages in thread
From: Richard Kenner @ 2010-09-28 18:26 UTC (permalink / raw)
To: ro; +Cc: gcc-patches
> 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?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Your change to ada/cats/run_acats in February
2010-09-28 18:26 ` Richard Kenner
@ 2010-09-28 20:16 ` Eric Botcazou
2010-09-30 12:48 ` Eric Botcazou
0 siblings, 1 reply; 5+ messages in thread
From: Eric Botcazou @ 2010-09-28 20:16 UTC (permalink / raw)
To: Richard Kenner; +Cc: gcc-patches, ro
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Your change to ada/cats/run_acats in February
2010-09-28 20:16 ` Eric Botcazou
@ 2010-09-30 12:48 ` Eric Botcazou
0 siblings, 0 replies; 5+ messages in thread
From: Eric Botcazou @ 2010-09-30 12:48 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Kenner, ro
[-- 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+"$@"}
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-09-30 6:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-28 17:43 Your change to ada/cats/run_acats in February Richard Kenner
2010-09-28 18:07 ` Rainer Orth
2010-09-28 18:26 ` Richard Kenner
2010-09-28 20:16 ` Eric Botcazou
2010-09-30 12:48 ` Eric Botcazou
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 read-only IMAP folder(s) and NNTP newsgroup(s).