Re: [FYI] {maint} maintcheck: avoid few spurious failures

[Stefano Lattarini]

On Monday 20 June 2011, Ralf Wildenhues wrote:
> * Stefano Lattarini wrote on Mon, Jun 20, 2011 at 05:05:45PM CEST:
> > * Makefile.am (sc_tests_plain_aclocal, sc_tests_plain_perl,
> > sc_tests_plain_autoconf, sc_tests_plain_automake,
> > sc_tests_plain_autoupate): Be stricter in matching an erroneous
> > literal command, i.e., `aclocal', `automake', `perl', etc.
> 
> >  sc_tests_plain_automake:
> > -   @if grep -v '^#' $(srcdir)/tests/*.test | grep -E ':[   
> > ]*automake([^:]|$$)'; then \
> > +   @if grep -v '^#' $(srcdir)/tests/*.test | grep -E ':[   
> > ]*automake\>([^:]|$$)'; then \
> 
> The RE that was there before was there specifically to emulate the
> nonportable '\>' construct.  Now, I'm not sure I should fight for using
> Posix compatible regular expressions in maintainer-check rules (seems I
> lost that battle earlier already),
>
Well, notice that I've just followed the existing practice in using GNU
grep extensions in the maintcheck rules; for example, the use of `\b'
was there before I even knew about the existence of Automake:

  $ cat -n Makefile.am
  531  sc_tests_plain_sleep:
  532          @if grep -E '\bsleep +[12345]\b' $(srcdir)/tests/*.test; then \
  ...
  537  ## fgrep and egrep are not required by POSIX.
  538  sc_tests_plain_egrep_fgrep:
  539          @if grep -E '\b[ef]grep\b' $(srcdir)/tests/*.test ; then \
  ...
  $ git blame Makefile.am
  e44668c6 (Alexandre Duret-Lutz 2003-01-10 ... 532) ...
  f3a8b03a (Alexandre Duret-Lutz 2002-05-31 ... 539) ...

> but if you require GNU grep, please be consistent and remove the
> now-unneeded stuff afterwards and the -E.
>
OK, I will push the attached patch if that's OK with you.  And since I
was at it, I noticed that the `sc_tests_plain_*' checks were incomplete,
as they didn't look for autoreconf, autoheader and autom4te too.  What
fixing this with the second attached patch?

> >  sc_tests_plain_aclocal:
> > -   @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[      ]*aclocal'; 
> > then \
> > +   @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[      ]*aclocal\>'; 
> > then \
> >       echo 'Do not run "aclocal" in the above tests.  Use "$$ACLOCAL" 
> > instead.' 1>&2;  \
> >       exit 1; \
> >     fi
> 
> Thanks,
> Ralf
> 

Regards,
  Stefano

From 8b66bd1bd4addfe9327455ee98d444597975d797 Mon Sep 17 00:00:00 2001
Message-Id: <address@hidden>
From: Stefano Lattarini <address@hidden>
Date: Mon, 20 Jun 2011 23:42:56 +0200
Subject: [PATCH 1/2] maintcheck: minor cleanup in rules

* Makefile.am (sc_tests_plain_automake): Make the "blacklisted"
regular expression simpler. Avoid now-useless use of the
`-E' grep option.  This also improve syncing with the other
`sc_tests_plain_*' targets.
* tests/location.test: Avoid spuriously triggering a failure of
that check.

Suggestion by Ralf Wildenhues.
---
 ChangeLog           |   11 +++++++++++
 Makefile.am         |    2 +-
 Makefile.in         |    2 +-
 tests/location.test |   38 +++++++++++++++++++-------------------
 4 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 82aac1e..f171f19 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2011-06-21  Stefano Lattarini  <address@hidden>
+
+       maintcheck: minor cleanup in rules
+       * Makefile.am (sc_tests_plain_automake): Make the "blacklisted"
+       regular expression simpler. Avoid now-useless use of the
+       `-E' grep option.  This also improve syncing with the other
+       `sc_tests_plain_*' targets.
+       * tests/location.test: Avoid spuriously triggering a failure of
+       that check.
+       Suggestion by Ralf Wildenhues.
+
 2011-06-20  Stefano Lattarini  <address@hidden>
 
        maintcheck: avoid few spurious failures
diff --git a/Makefile.am b/Makefile.am
index 9277986..978ec4c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -380,7 +380,7 @@ sc_tests_plain_autoupdate:
 
 ## Tests should never call automake directly.
 sc_tests_plain_automake:
-       @if grep -v '^#' $(srcdir)/tests/*.test | grep -E ':[   
]*automake\>([^:]|$$)'; then \
+       @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[      ]*automake\>'; 
then \
          echo 'Do not run "automake" in the above tests.  Use "$$AUTOMAKE" 
instead.' 1>&2;  \
          exit 1; \
        fi
diff --git a/Makefile.in b/Makefile.in
index 10d57b9..242a073 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -1083,7 +1083,7 @@ sc_tests_plain_autoupdate:
        fi
 
 sc_tests_plain_automake:
-       @if grep -v '^#' $(srcdir)/tests/*.test | grep -E ':[   
]*automake\>([^:]|$$)'; then \
+       @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[      ]*automake\>'; 
then \
          echo 'Do not run "automake" in the above tests.  Use "$$AUTOMAKE" 
instead.' 1>&2;  \
          exit 1; \
        fi
diff --git a/tests/location.test b/tests/location.test
index 13a2183..9e602fd 100755
--- a/tests/location.test
+++ b/tests/location.test
@@ -49,25 +49,25 @@ AUTOMAKE_fails
 
 # Smash the useless difference of lib file locations.
 sed 's,^.*lib/am/\([a-z]*\.am\),\1,' stderr >observed
-cat >expected <<\EOF
-Makefile.am:12: VAR multiply defined in condition TRUE ...
-Makefile.am:8: ... `VAR' previously defined here
-automake: libfoo_a_OBJECTS should not be defined
-Makefile.am:3:   while processing library `libfoo.a'
-automake: use `libfoo_a_LDADD', not `libfoo_a_LIBADD'
-Makefile.am:3:   while processing library `libfoo.a'
-library.am: deprecated feature: target `libfoo.a' overrides `libfoo.a$(EXEEXT)'
-library.am: change your target to read `libfoo.a$(EXEEXT)'
-Makefile.am:3:   while processing library `libfoo.a'
-program.am: target `libfoo.a$(EXEEXT)' was defined here
-Makefile.am:1:   while processing program `libfoo.a'
-program.am: redefinition of `libfoo.a$(EXEEXT)'...
-Makefile.am:1:   while processing program `libfoo.a'
-library.am: ... `libfoo.a' previously defined here
-Makefile.am:3:   while processing library `libfoo.a'
-tags.am: redefinition of `ctags'...
-program.am: ... `ctags$(EXEEXT)' previously defined here
-Makefile.am:6:   while processing program `ctags'
+unindent >expected <<'EOF'
+  Makefile.am:12: VAR multiply defined in condition TRUE ...
+  Makefile.am:8: ... `VAR' previously defined here
+  automake: libfoo_a_OBJECTS should not be defined
+  Makefile.am:3:   while processing library `libfoo.a'
+  automake: use `libfoo_a_LDADD', not `libfoo_a_LIBADD'
+  Makefile.am:3:   while processing library `libfoo.a'
+  library.am: deprecated feature: target `libfoo.a' overrides 
`libfoo.a$(EXEEXT)'
+  library.am: change your target to read `libfoo.a$(EXEEXT)'
+  Makefile.am:3:   while processing library `libfoo.a'
+  program.am: target `libfoo.a$(EXEEXT)' was defined here
+  Makefile.am:1:   while processing program `libfoo.a'
+  program.am: redefinition of `libfoo.a$(EXEEXT)'...
+  Makefile.am:1:   while processing program `libfoo.a'
+  library.am: ... `libfoo.a' previously defined here
+  Makefile.am:3:   while processing library `libfoo.a'
+  tags.am: redefinition of `ctags'...
+  program.am: ... `ctags$(EXEEXT)' previously defined here
+  Makefile.am:6:   while processing program `ctags'
 EOF
 
 diff expected observed || Exit 1
-- 
1.7.2.3

From d662e866731d49ea7dc67942e9a4a688def3f7ad Mon Sep 17 00:00:00 2001
Message-Id: <address@hidden>
In-Reply-To: <address@hidden>
References: <address@hidden>
From: Stefano Lattarini <address@hidden>
Date: Mon, 20 Jun 2011 23:52:34 +0200
Subject: [PATCH 2/2] maintcheck: extend 'sc_tests_plain_*' checks

* Makefile.am (sc_tests_plain_autom4te): New check.
(sc_tests_plain_autoreconf): Likewise.
(sc_tests_plain_autoheader): Likewise.
(syntax_check_rules): Update.
---
 ChangeLog   |    8 ++++++++
 Makefile.am |   24 ++++++++++++++++++++++++
 Makefile.in |   21 +++++++++++++++++++++
 3 files changed, 53 insertions(+), 0 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index f171f19..76ce5d6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2011-06-21  Stefano Lattarini  <address@hidden>
 
+       maintcheck: extend 'sc_tests_plain_*' checks
+       * Makefile.am (sc_tests_plain_autom4te): New check.
+       (sc_tests_plain_autoreconf): Likewise.
+       (sc_tests_plain_autoheader): Likewise.
+       (syntax_check_rules): Update.
+
+2011-06-21  Stefano Lattarini  <address@hidden>
+
        maintcheck: minor cleanup in rules
        * Makefile.am (sc_tests_plain_automake): Make the "blacklisted"
        regular expression simpler. Avoid now-useless use of the
diff --git a/Makefile.am b/Makefile.am
index 978ec4c..66835b2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -136,6 +136,9 @@ sc_tests_plain_make \
 sc_tests_plain_autoconf \
 sc_tests_plain_autoupdate \
 sc_tests_plain_automake \
+sc_tests_plain_autom4te \
+sc_tests_plain_autoheader \
+sc_tests_plain_autoreconf \
 sc_tests_here_document_format \
 sc_tests_Exit_not_exit \
 sc_tests_automake_fails \
@@ -385,6 +388,27 @@ sc_tests_plain_automake:
          exit 1; \
        fi
 
+## Tests should never call autoheader directly.
+sc_tests_plain_autoheader:
+       @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[      
]*autoheader\>'; then \
+         echo 'Do not run "automake" in the above tests.  Use "$$AUTOHEADER" 
instead.' 1>&2;  \
+         exit 1; \
+       fi
+
+## Tests should never call autoreconf directly.
+sc_tests_plain_autoreconf:
+       @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[      
]*autoreconf\>'; then \
+         echo 'Do not run "automake" in the above tests.  Use "$$AUTORECONF" 
instead.' 1>&2;  \
+         exit 1; \
+       fi
+
+## Tests should never call autom4te directly.
+sc_tests_plain_autom4te:
+       @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[      ]*autom4te\>'; 
then \
+         echo 'Do not run "automake" in the above tests.  Use "$$AUTOM4TE" 
instead.' 1>&2;  \
+         exit 1; \
+       fi
+
 ## Tests should only use END and EOF for here documents
 ## (so that the next test is effective).
 sc_tests_here_document_format:
diff --git a/Makefile.in b/Makefile.in
index 242a073..e03c92e 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -297,6 +297,9 @@ sc_tests_plain_make \
 sc_tests_plain_autoconf \
 sc_tests_plain_autoupdate \
 sc_tests_plain_automake \
+sc_tests_plain_autom4te \
+sc_tests_plain_autoheader \
+sc_tests_plain_autoreconf \
 sc_tests_here_document_format \
 sc_tests_Exit_not_exit \
 sc_tests_automake_fails \
@@ -1088,6 +1091,24 @@ sc_tests_plain_automake:
          exit 1; \
        fi
 
+sc_tests_plain_autoheader:
+       @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[      
]*autoheader\>'; then \
+         echo 'Do not run "automake" in the above tests.  Use "$$AUTOHEADER" 
instead.' 1>&2;  \
+         exit 1; \
+       fi
+
+sc_tests_plain_autoreconf:
+       @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[      
]*autoreconf\>'; then \
+         echo 'Do not run "automake" in the above tests.  Use "$$AUTORECONF" 
instead.' 1>&2;  \
+         exit 1; \
+       fi
+
+sc_tests_plain_autom4te:
+       @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[      ]*autom4te\>'; 
then \
+         echo 'Do not run "automake" in the above tests.  Use "$$AUTOM4TE" 
instead.' 1>&2;  \
+         exit 1; \
+       fi
+
 sc_tests_here_document_format:
        @if grep '<<' $(srcdir)/tests/*.test | grep -v 'END' | grep -v 'EOF'; 
then \
          echo 'Use here documents with "END" and "EOF" only, for 
greppability.' 1>&2; \
-- 
1.7.2.3