Re: [PATCH 5/5] Improved test silent5.test.

[Stefano Lattarini]

At Sunday 25 April 2010, Ralf Wildenhues <address@hidden> 
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 <address@hidden>
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  <address@hidden>
 
+       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