[PATCH] {maint} maintcheck: look for problematic names of testcases

[Stefano Lattarini]

Reference to original thread:
 <http://lists.gnu.org/archive/html/automake-patches/2011-03/msg00012.html>

(Sorry for the delay).

On Friday 04 March 2011, Stefano Lattarini wrote:
> On Friday 04 March 2011, Ralf Wildenhues wrote:
> > Hi Stefano,
> > 
> > * Stefano Lattarini wrote on Fri, Mar 04, 2011 at 09:52:25AM CET:
> > > * tests/comments-in-var-defn.test: The configure.in stub created
> > > by default, which has the AC_INIT first argument obtained by the
> > > test name, causes autoconf 2.62 to fail with a spurious error
> > > message like: "configure.in:1: error: defn: undefined macro:".
> > > Thus, to prevent this, the test is renamed to ...
> > > * tests/comments-in-var-def.test: ... this.
> > > * tests/Makefile.am (TESTS): Updated.
> > 
> > > OK to apply the attached patch to a temporary bugfixing branch based
> > > off of  commit v1.11-263-g6318add (in which the offending test was
> > > introduced), and merge into master?
> > 
> > Okay, though in general it would be preferable to avoid such bugs in
> > tests/defs so we wouldn't have to remember them in the future.
> > 
> Or maybe we could define a "maintainer-check" that looks at the
> testcases' names and complains about suspicious ones...  I'll
> give it a shot unless someone beats me.
>
Done in the attached patch.

I'll push in 72 hours if there is no objection.

Regards,
   Stefano

From 76a6e791ba5ce20738575ff418fabca09e62d5d7 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <address@hidden>
Date: Mon, 14 Mar 2011 12:44:53 +0100
Subject: [PATCH] maintcheck: look for problematic names of testcases

The configure.in stub created by default by `tests/defs' obtains
the first argument of AC_INIT from the test name, and this can
cause some supported autoconf versions to fail with a spurious
error if that test name contains the name of an m4 builtin (e.g.,
`defn' or `undefine').

See for example the bug fixed by commit v1.11-287-g1325a8a.

This change add a maintainer check that warns about test names
which are possibly problematic in this regard.

* Makefile.am (sc_test_names): New maintainer-check target.
(syntax_check_rules): Add it.
(m4_builtins): New helper variable.
(TESTS): Updated according to the following renamings.
* tests/include.test: Renamed ...
* tests/hdr-vars-defined-once.test: ... to this.
* tests/sinclude.test: Renamed ...
* tests/m4-inclusion.test: ... to this, and simplified
accordingly.
* tests/include2.test: Renamed ...
* tests/dist-included-parent-dir.test: ... to this, for
consistency.
---
 ChangeLog                                          |   24 ++++++++
 Makefile.am                                        |   56 ++++++++++++++++++++
 Makefile.in                                        |   53 ++++++++++++++++++
 tests/Makefile.am                                  |    6 +-
 tests/Makefile.in                                  |    6 +-
 ...include2.test => dist-included-parent-dir.test} |    0
 tests/{include.test => hdr-vars-defined-once.test} |    0
 tests/{sinclude.test => m4-inclusion.test}         |    8 +---
 8 files changed, 140 insertions(+), 13 deletions(-)
 rename tests/{include2.test => dist-included-parent-dir.test} (100%)
 rename tests/{include.test => hdr-vars-defined-once.test} (100%)
 rename tests/{sinclude.test => m4-inclusion.test} (83%)

diff --git a/ChangeLog b/ChangeLog
index 804fae6..f58287e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,27 @@
+2011-03-14  Stefano Lattarini  <address@hidden>
+
+       maintcheck: look for problematic names of testcases
+       The configure.in stub created by default by `tests/defs' obtains
+       the first argument of AC_INIT from the test name, and this can
+       cause some supported autoconf versions to fail with a spurious
+       error if that test name contains the name of an m4 builtin (e.g.,
+       `defn' or `undefine').
+       See for example the bug fixed by commit v1.11-287-g1325a8a.
+       This change add a maintainer check that warns about test names
+       which are possibly problematic in this regard.
+       * Makefile.am (sc_test_names): New maintainer-check target.
+       (syntax_check_rules): Add it.
+       (m4_builtins): New helper variable.
+       (TESTS): Updated according to the following renamings.
+       * tests/include.test: Renamed ...
+       * tests/hdr-vars-defined-once.test: ... to this.
+       * tests/sinclude.test: Renamed ...
+       * tests/m4-inclusion.test: ... to this, and simplified
+       accordingly.
+       * tests/include2.test: Renamed ...
+       * tests/dist-included-parent-dir.test: ... to this, for
+       consistency.
+
 2011-03-04  Stefano Lattarini  <address@hidden>
 
        tests: fix bug in alloca*.test
diff --git a/Makefile.am b/Makefile.am
index d19d974..5a37b41 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -111,6 +111,7 @@ dist-hook:
 # Some simple checks, and then ordinary check.  These are only really
 # guaranteed to work on my machine.
 syntax_check_rules = \
+sc_test_names \
 sc_diff_automake_in_automake \
 sc_perl_syntax \
 sc_no_brace_variable_expansions \
@@ -151,6 +152,61 @@ sc_at_in_texi
 $(syntax_check_rules): automake aclocal
 maintainer-check: $(syntax_check_rules)
 
+## Look for test whose names can cause spurious failures when used as
+## first argument to AC_INIT (chiefly because they might contain an
+## m4/m4sugar builtins).
+m4_builtins = \
+  bpatsubst \
+  bregexp \
+  builtin \
+  changecom \
+  changequote \
+  changeword \
+  debugfile \
+  debugmode \
+  decr \
+  define \
+  defn \
+  divert \
+  divnum \
+  dumpdef \
+  errprint \
+  esyscmd \
+  eval \
+  format \
+  ifdef \
+  ifelse \
+  include \
+  incr \
+  index \
+  indir \
+  len \
+  maketemp \
+  mkstemp \
+  patsubst \
+  popdef \
+  pushdef \
+  regexp \
+  shift \
+  sinclude \
+  substr \
+  symbols \
+  syscmd \
+  sysval \
+  traceoff \
+  traceon \
+  translit \
+  undefine \
+  undivert
+sc_test_names:
+       @m4_builtins_rx=`echo $(m4_builtins) | sed 's/ /|/g'`; \
+        m4_builtins_rx="\\<($$m4_builtins_rx)\\>|\\<_?m4[a-zA-Z0-9_]"; \
+        if ls tests/*.test | LC_ALL=C grep -E "$$m4_builtins_rx"; then \
+          echo "the names of the tests above can be problematic" 1>&2; \
+          echo "Avoid test names that contain names of m4 builtins" 1>&2; \
+          exit 1; \
+        fi
+
 ## This check avoids accidental configure substitutions in the source.
 ## There are exactly 8 lines that should be modified.  This works out
 ## to 28 lines of diffs.
diff --git a/Makefile.in b/Makefile.in
index 25d580d..31368b9 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -269,6 +269,7 @@ do_subst = sed \
 # Some simple checks, and then ordinary check.  These are only really
 # guaranteed to work on my machine.
 syntax_check_rules = \
+sc_test_names \
 sc_diff_automake_in_automake \
 sc_perl_syntax \
 sc_no_brace_variable_expansions \
@@ -305,6 +306,50 @@ sc_unquoted_DESTDIR \
 sc_tabs_in_texi \
 sc_at_in_texi
 
+m4_builtins = \
+  bpatsubst \
+  bregexp \
+  builtin \
+  changecom \
+  changequote \
+  changeword \
+  debugfile \
+  debugmode \
+  decr \
+  define \
+  defn \
+  divert \
+  divnum \
+  dumpdef \
+  errprint \
+  esyscmd \
+  eval \
+  format \
+  ifdef \
+  ifelse \
+  include \
+  incr \
+  index \
+  indir \
+  len \
+  maketemp \
+  mkstemp \
+  patsubst \
+  popdef \
+  pushdef \
+  regexp \
+  shift \
+  sinclude \
+  substr \
+  symbols \
+  syscmd \
+  sysval \
+  traceoff \
+  traceon \
+  translit \
+  undefine \
+  undivert
+
 WGET = wget
 WGET_SV_CVS = $(WGET) http://savannah.gnu.org/cgi-bin/viewcvs/~checkout~/
 WGET_SV_GIT_CF = $(WGET) 
'http://git.savannah.gnu.org/gitweb/?p=config.git;a=blob_plain;hb=HEAD;f='
@@ -879,6 +924,14 @@ dist-hook:
 .PHONY: $(syntax_check_rules)
 $(syntax_check_rules): automake aclocal
 maintainer-check: $(syntax_check_rules)
+sc_test_names:
+       @m4_builtins_rx=`echo $(m4_builtins) | sed 's/ /|/g'`; \
+        m4_builtins_rx="\\<($$m4_builtins_rx)\\>|\\<_?m4[a-zA-Z0-9_]"; \
+        if ls tests/*.test | LC_ALL=C grep -E "$$m4_builtins_rx"; then \
+          echo "the names of the tests above can be problematic" 1>&2; \
+          echo "Avoid test names that contain names of m4 builtins" 1>&2; \
+          exit 1; \
+        fi
 
 sc_diff_automake_in_automake:
        @if test `diff $(srcdir)/automake.in automake | wc -l` -ne 28; then \
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 3fdb90a..4becdbb 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -288,6 +288,7 @@ dirforbid.test \
 dirlist.test \
 dirlist2.test \
 discover.test \
+dist-included-parent-dir.test \
 distcleancheck.test \
 distcom2.test \
 distcom3.test \
@@ -362,6 +363,7 @@ gnuwarn2.test \
 gnits.test \
 gnits2.test \
 gnits3.test \
+hdr-vars-defined-once.test \
 header.test \
 help.test \
 help2.test \
@@ -381,8 +383,6 @@ help-upc.test \
 hfs.test \
 hosts.test \
 implicit.test \
-include.test \
-include2.test \
 info.test \
 init.test \
 init2.test \
@@ -487,6 +487,7 @@ ltlibobjs.test \
 ltlibsrc.test \
 ltorder.test \
 lzma.test \
+m4-inclusion.test \
 maintclean.test \
 make.test \
 makej.test \
@@ -662,7 +663,6 @@ silent-lex-gcc.test \
 silent-lex-generic.test \
 silent-yacc-gcc.test \
 silent-yacc-generic.test \
-sinclude.test \
 srcsub.test \
 srcsub2.test \
 space.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index cd00833..b9b1f6e 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -558,6 +558,7 @@ dirforbid.test \
 dirlist.test \
 dirlist2.test \
 discover.test \
+dist-included-parent-dir.test \
 distcleancheck.test \
 distcom2.test \
 distcom3.test \
@@ -632,6 +633,7 @@ gnuwarn2.test \
 gnits.test \
 gnits2.test \
 gnits3.test \
+hdr-vars-defined-once.test \
 header.test \
 help.test \
 help2.test \
@@ -651,8 +653,6 @@ help-upc.test \
 hfs.test \
 hosts.test \
 implicit.test \
-include.test \
-include2.test \
 info.test \
 init.test \
 init2.test \
@@ -757,6 +757,7 @@ ltlibobjs.test \
 ltlibsrc.test \
 ltorder.test \
 lzma.test \
+m4-inclusion.test \
 maintclean.test \
 make.test \
 makej.test \
@@ -932,7 +933,6 @@ silent-lex-gcc.test \
 silent-lex-generic.test \
 silent-yacc-gcc.test \
 silent-yacc-generic.test \
-sinclude.test \
 srcsub.test \
 srcsub2.test \
 space.test \
diff --git a/tests/include2.test b/tests/dist-included-parent-dir.test
similarity index 100%
rename from tests/include2.test
rename to tests/dist-included-parent-dir.test
diff --git a/tests/include.test b/tests/hdr-vars-defined-once.test
similarity index 100%
rename from tests/include.test
rename to tests/hdr-vars-defined-once.test
diff --git a/tests/sinclude.test b/tests/m4-inclusion.test
similarity index 83%
rename from tests/sinclude.test
rename to tests/m4-inclusion.test
index cc8898b..025d44f 100755
--- a/tests/sinclude.test
+++ b/tests/m4-inclusion.test
@@ -20,13 +20,7 @@
 
 set -e
 
-# Overwrite configure.in, because the default uses `sinclude' as package
-# name and this play havoc with Autoconf on some platforms (`sinclude'
-# is an m4 macro).
-cat > configure.in <<EOF
-AC_INIT([amsinclude], [1.0])
-AM_INIT_AUTOMAKE
-AC_CONFIG_FILES([Makefile])
+cat >> configure.in <<'EOF'
 sinclude([doesntexist.m4])
 EOF
 
-- 
1.7.2.3