On Monday 03 January 2011, Ralf Wildenhues wrote: > Hi Stefano, > > * Stefano Lattarini wrote on Mon, Jan 03, 2011 at 03:04:05PM CET: > > * tests/maken3.test (check_targets): Remove redundant call to > > 'set -e'. > > * tests/maken4.test: Likewise. > > * tests/ansi5.test: Call 'set -e' just after './defs' has been > > sourced. > > * tests/ansi6.test: Likewise. > > * tests/ansi7.test: Likewise. > > * tests/cond16.test: Likewise. > > * tests/cond17.test: Likewise. > > * tests/cond18.test: Likewise. > > * tests/cond19.test: Likewise. > > * tests/cond20.test: Likewise. > > * tests/cond21.test: Likewise. > > * tests/instdat2.test: Likewise. > > * tests/instdir-texi.test: Likewise. > > * tests/parallel-tests3.test: Likewise. > > * tests/remake1a.test: Likewise. > > * tests/ccnoco.test: Likewise, and add trailing `:' command. > > * tests/comment4.test: Likewise. > > * tests/gcj4.test: Likewise. > > * tests/nodist2.test: Likewise. > > * tests/nodist3.test: Enable 'errexit' shell flag (this should > > have been done in commit v1.11-248-g317e17b, but the relevant > > hunk has been forgotten somehow). > > * tests/output.test: Likewise. > > * tests/gnits2.test: Likewise, and display captured stderr to > > script's stderr, not to script's stdout. > > * tests/gnits3.test: Likewise. Also, prefer 'cat' over 'echo' > > to append to Makefile.am, and really check that the exit status > > of "make installcheck" indicates failure. > [...] > > --- a/tests/maken3.test > > +++ b/tests/maken3.test > > > @@ -122,7 +122,6 @@ $AUTOCONF > > > > check_targets () > > { > > - set -e > > for target in \ > > all install install-strip uninstall clean distclean check \ > > info html dvi pdf ps \ > > > --- a/tests/maken4.test > > +++ b/tests/maken4.test > > > @@ -124,7 +124,6 @@ $AUTOCONF > > > > check_targets () > > { > > - set -e > > for target in \ > > all install install-strip uninstall clean distclean check \ > > info html dvi pdf ps \ > > These two were added by me in fear of shells resetting errexit upon > function entry (like some shells do with -x aka xtrace). Luckily I > couldn't find any shells which do this upon testing (by looking for 'e' > in the output of) > sh -ec 'f () { echo $-; }; f' > > but generally, here's a nice (and more complete than autoconf.texi) list > of issues with set -e: http://www.in-ulm.de/~mascheck/various/set-e/ > > Please, before removing such seemingly superfluous code, consider > first whether it may have been added for a reason. > > If you do this research, then I don't have to, and can review more > patches. > > Patch is OK. > > Thanks, > Ralf > JFTR: I considered the removal of those `set -e' just obvious because all the other testcases which use shell functions to do their checks do not bother to call `set -e' upon entering such functions. Moreover, `git diff' showed me that those `set -e' calls in maken{3,4}.test had been there from the very first version of the tests, which made clear that they had not been added in response to a real failure or to some weird shell bug.
Regards, Stefano