At Sunday 25 April 2010, Ralf Wildenhues <ralf.wildenh...@gmx.de> wrote: > > > --- a/tests/silent5.test > > +++ b/tests/silent5.test > > > > @@ -116,7 +119,7 @@ do > > $MAKE >stdout || { cat stdout; Exit 1; } > > cat stdout > > grep ' -c' stdout && Exit 1 > > - grep ' -o ' stdout && Exit 1 > > + grep ' -o' stdout && Exit 1 > > This test has become less instead of more strict; oversight or > purpose? Why it should be less strict? Now even a command like: gcc -ofoo foo.o would make the test fail, while previously a command like: gcc -o foo foo.o was required.
> > grep mv stdout && Exit 1 On the contrary, this seems too much strict, since it would fail (with GNU make at least) if the automake source tree is placed in a directory whose name contains the `mv' substring. The other silent*.test tests might have a similar problem too. This should IMHO be addressed in a different patch series. WDYT? > > grep 'CXX .*foo1\.' stdout > > @@ -139,10 +142,13 @@ do > > grep 'CXXLD .*baz' stdout > > grep 'CCLD .*bla' stdout > > > > + # Ensure a clean rebuild. > > $MAKE clean > > + rm -f foo5.c foo6.[ch] sub/baz5.c sub/baz6.[ch] > > Hmm, I wonder if doing clean and rebuild with the same V flag (and > without removing generated sources in between) would be a sensible > addition on top of this: it might trigger a different set of rules > (as I think you may be able to see with heirloom make). Yes, more sanity checks wouldn't hurt I guess. Should I amend the patch, or are you going to do that yourself? > > + > > + # Ensure a clean reconfiguration/rebuild. > > $MAKE clean > > $MAKE maintainer-clean > > + rm -f foo5.c foo6.[ch] sub/baz5.c sub/baz6.[ch] > > Wait, maintainer-clean should have removed all these files at this > point (and some of the other lex/yacc tests should have this > tested, too). Does that not work for you? No, it works just fine; but in the (unlikely) case of a failure, it could cause false positives (or even false negatives!) in silent5.test. Also, silent5.test is not supposed to test `maintainer-clean' w.r.t. Lex/Yacc; if such tests are required, they should be placed in a different test script IMO (and I see that this is indeed the case, since `maintainer-check' is already tested at least in lex3.test, yacc4.test and yacc7.test). > Other than the above nits, this series looks fairly good to me. > You could add something like 'Split out from silent5.test' to the > test descriptions, but that's a really minor point. > > Did you run 'make maintainer-check' on the series? *Blush* Obviously no; and in fact `make maintainer-check' spuriously fails with: ./tests/silentcxx.test: cout << "Hello World!" << endl; Use here documents with "END" and "EOF" only, for greppability. make: *** [sc_tests_here_document_format] Error 1 I really think that the maintainer-checks should be made more configurable esp. w.r.t. whitelists (but this is obviously material for an unrelated patch series); for the moment, I just amended the patch to avoid that spurious warning (updated patch is attached). > Thanks for working on this, > Ralf Thanks to you for your quick review. Regards, Stefano
From 571cfbdb78acb90644c445392d30aa58dca24146 Mon Sep 17 00:00:00 2001 From: Stefano Lattarini <stefano.lattar...@gmail.com> Date: Thu, 22 Apr 2010 21:08:10 +0200 Subject: [PATCH 3/5] New test silentcxx.test (Automake/C++ silent-mode). * tests/silentcxx.test: New test. * tests/Makefile.am (TESTS): Updated accordingly. --- ChangeLog | 4 ++ tests/Makefile.am | 1 + tests/Makefile.in | 1 + tests/silentcxx.test | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 0 deletions(-) create mode 100755 tests/silentcxx.test diff --git a/ChangeLog b/ChangeLog index b810f6d..64b3edb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,9 @@ 2010-04-22 Stefano Lattarini <stefano.lattar...@gmail.com> + New test `silentcxx.test' (Automake silent-mode with C++). + * tests/silentcxx.test: New test. + * tests/Makefile.am (TESTS): Updated accordingly. + New test `silentyacc.test' (Automake silent-mode with Yacc). * tests/silentyacc.test: New test. * tests/Makefile.am (TESTS): Updated accordingly. diff --git a/tests/Makefile.am b/tests/Makefile.am index 600eda6..f2f7fd5 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -624,6 +624,7 @@ silent6.test \ silent7.test \ silent8.test \ silent9.test \ +silentcxx.test \ silentlex.test \ silentyacc.test \ sinclude.test \ diff --git a/tests/Makefile.in b/tests/Makefile.in index 5fb8bd1..4b8ab84 100644 --- a/tests/Makefile.in +++ b/tests/Makefile.in @@ -865,6 +865,7 @@ silent6.test \ silent7.test \ silent8.test \ silent9.test \ +silentcxx.test \ silentlex.test \ silentyacc.test \ sinclude.test \ diff --git a/tests/silentcxx.test b/tests/silentcxx.test new file mode 100755 index 0000000..d8f9b01 --- /dev/null +++ b/tests/silentcxx.test @@ -0,0 +1,106 @@ +#!/bin/sh +# Copyright (C) 2010 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2, or (at your option) +# any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +# Check silent-rules mode for C++. + +required='g++' +. ./defs + +set -e + +mkdir sub + +cat >>configure.in <<'EOF' +AM_SILENT_RULES +AC_PROG_CXX +AC_CONFIG_FILES([sub/Makefile]) +AC_OUTPUT +EOF + +cat > Makefile.am <<'EOF' +# Need generic and non-generic rules. +bin_PROGRAMS = foo1 foo2 +foo1_SOURCES = foo.cpp baz.cxx quux.cc +foo2_SOURCES = $(foo1_SOURCES) +foo2_CXXFLAGS = $(AM_CXXFLAGS) +SUBDIRS = sub +EOF + +cat > sub/Makefile.am <<'EOF' +AUTOMAKE_OPTIONS = subdir-objects +# Need generic and non-generic rules. +bin_PROGRAMS = bar1 bar2 +bar1_SOURCES = bar.cpp +bar2_SOURCES = $(bar1_SOURCES) +bar2_CXXFLAGS = $(AM_CXXFLAGS) +EOF + +cat > foo.cpp <<'EOF' +using namespace std; /* C compilers fail on this */ +int main() { return 0; } +EOF + +# let's try out other extensions too +echo 'class Baz { public: int i; };' > baz.cxx +echo 'class Quux { public: bool b; };' > quux.cc + +cp foo.cpp sub/bar.cpp + +$ACLOCAL +$AUTOMAKE --add-missing +$AUTOCONF + +# configure once for fastdep, once for non-fastdep, once for nodep +for config_args in \ + '' \ + am_cv_CC_dependencies_compiler_type=gcc \ + --disable-dependency-tracking +do + ./configure $config_args --enable-silent-rules + $MAKE >stdout || { cat stdout; Exit 1; } + cat stdout + grep ' -c' stdout && Exit 1 + grep ' -o' stdout && Exit 1 + grep mv stdout && Exit 1 + + grep 'CXX .*foo\.' stdout + grep 'CXX .*baz\.' stdout + grep 'CXX .*quux\.' stdout + grep 'CXX .*bar\.' stdout + $EGREP 'CXXLD .*foo1' stdout + $EGREP 'CXXLD .*bar1' stdout + $EGREP 'CXXLD .*foo2' stdout + $EGREP 'CXXLD .*bar2' stdout + + # Ensure a clean rebuild. + $MAKE clean + + $MAKE V=1 >stdout || { cat stdout; Exit 1; } + cat stdout + grep ' -c ' stdout + grep ' -o ' stdout + + grep 'CXX ' stdout && Exit 1 + grep 'CC ' stdout && Exit 1 + grep 'LD ' stdout && Exit 1 + + # Ensure a clean reconfiguration/rebuild. + $MAKE clean + $MAKE maintainer-clean + +done + +: -- 1.6.5