[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] {maint} maintcheck: look for problematic names of testcases
From: |
Stefano Lattarini |
Subject: |
Re: [PATCH] {maint} maintcheck: look for problematic names of testcases |
Date: |
Thu, 17 Mar 2011 12:00:03 +0100 |
User-agent: |
KMail/1.13.3 (Linux/2.6.30-2-686; KDE/4.4.4; i686; ; ) |
On Wednesday 16 March 2011, Ralf Wildenhues wrote:
> * Stefano Lattarini wrote on Mon, Mar 14, 2011 at 12:51:29PM CET:
> > > 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.
>
> OK with nits addressed.
>
> Thanks,
> Ralf
>
> > 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').
>
> I'd say "name of a macro", because I don't think it's only builtins
> that are affected.
>
True.
> For example, AC_DEFUN wouldn't work either. So, it would
> be good to also test that we don't use prefixes _?A[CUMHS]_ or _?m4_
> either (can be done in followup patch).
>
Nah, I'd rather make the current patch more correct right away.
> > --- 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).
>
> either "builtin" or s/ an//
>
I went for this: "... (chiefly because they might contain an m4/m4sugar
builtin or macro name)".
> > +m4_builtins = \
> > + bpatsubst \
> > + ... \
> > + undivert
> > +sc_test_names:
> > + @m4_builtins_rx=`echo $(m4_builtins) | sed 's/ /|/g'`; \
> > + m4_builtins_rx="\\<($$m4_builtins_rx)\\>|\\<_?m4[a-zA-Z0-9_]"; \
> ^
>
> This underscore plus question doesn't make sense to me. Why not this?
> \\<m4_[a-zA-Z0-9]
>
Because that woul fail to take into account the m4sugar internal macros.
BTW, yourself suggested above to use `_?m4_'... am I missing something?
-*-*-
Attached is what I've squashed in, plus the final amended patch.
I will push shortly (by tomorrow) if there is no objection.
Thanks,
Stefano
diff --git a/ChangeLog b/ChangeLog
index f58287e..d2865ea 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,11 +1,12 @@
-2011-03-14 Stefano Lattarini <address@hidden>
+2011-03-17 Stefano Lattarini <address@hidden>
+ Ralf Wildenhues <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').
+ error if that test name contains the name of an m4 or m4sugar
+ builtin or macro (e.g., `defn' or `m4_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.
diff --git a/Makefile.am b/Makefile.am
index 7b0f9b1..316619f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -154,8 +154,10 @@ 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/m4sugar builtin or macro name).
m4_builtins = \
+ __gnu__ \
+ __unix__ \
bpatsubst \
bregexp \
builtin \
@@ -182,6 +184,8 @@ m4_builtins = \
index \
indir \
len \
+ m4exit \
+ m4wrap \
maketemp \
mkstemp \
patsubst \
@@ -200,11 +204,11 @@ m4_builtins = \
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 \
+ @m4_builtin_rx=`echo $(m4_builtins) | sed 's/ /|/g'`; \
+ m4_macro_rx="\\<($$m4_builtin_rx)\\>|\\<_?(A[CUMHS]|m4)_"; \
+ if ls tests/*.test | LC_ALL=C grep -E "$$m4_macro_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; \
+ echo "Avoid test names that contain names of m4 macros" 1>&2; \
exit 1; \
fi
diff --git a/Makefile.in b/Makefile.in
index dfc9fd2..ec327ed 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -307,6 +307,8 @@ sc_tabs_in_texi \
sc_at_in_texi
m4_builtins = \
+ __gnu__ \
+ __unix__ \
bpatsubst \
bregexp \
builtin \
@@ -333,6 +335,8 @@ m4_builtins = \
index \
indir \
len \
+ m4exit \
+ m4wrap \
maketemp \
mkstemp \
patsubst \
@@ -926,11 +930,11 @@ dist-hook:
$(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 \
+ @m4_builtin_rx=`echo $(m4_builtins) | sed 's/ /|/g'`; \
+ m4_macro_rx="\\<($$m4_builtin_rx)\\>|\\<_?(A[CUMHS]|m4)_"; \
+ if ls tests/*.test | LC_ALL=C grep -E "$$m4_macro_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; \
+ echo "Avoid test names that contain names of m4 macros" 1>&2; \
exit 1; \
fi
From e36003d0cf689ab737e774ea5faa07b2375e0d0a 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 or m4sugar
builtin or macro (e.g., `defn' or `m4_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 | 25 ++++++++
Makefile.am | 61 ++++++++++++++++++++
Makefile.in | 58 +++++++++++++++++++
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, 151 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..d2865ea 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,28 @@
+2011-03-17 Stefano Lattarini <address@hidden>
+ Ralf Wildenhues <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 or m4sugar
+ builtin or macro (e.g., `defn' or `m4_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..316619f 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,66 @@ 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 builtin or macro name).
+m4_builtins = \
+ __gnu__ \
+ __unix__ \
+ bpatsubst \
+ bregexp \
+ builtin \
+ changecom \
+ changequote \
+ changeword \
+ debugfile \
+ debugmode \
+ decr \
+ define \
+ defn \
+ divert \
+ divnum \
+ dnl \
+ dumpdef \
+ errprint \
+ esyscmd \
+ eval \
+ format \
+ ifdef \
+ ifelse \
+ include \
+ incr \
+ index \
+ indir \
+ len \
+ m4exit \
+ m4wrap \
+ maketemp \
+ mkstemp \
+ patsubst \
+ popdef \
+ pushdef \
+ regexp \
+ shift \
+ sinclude \
+ substr \
+ symbols \
+ syscmd \
+ sysval \
+ traceoff \
+ traceon \
+ translit \
+ undefine \
+ undivert
+sc_test_names:
+ @m4_builtin_rx=`echo $(m4_builtins) | sed 's/ /|/g'`; \
+ m4_macro_rx="\\<($$m4_builtin_rx)\\>|\\<_?(A[CUMHS]|m4)_"; \
+ if ls tests/*.test | LC_ALL=C grep -E "$$m4_macro_rx"; then \
+ echo "the names of the tests above can be problematic" 1>&2; \
+ echo "Avoid test names that contain names of m4 macros" 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..ec327ed 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,55 @@ sc_unquoted_DESTDIR \
sc_tabs_in_texi \
sc_at_in_texi
+m4_builtins = \
+ __gnu__ \
+ __unix__ \
+ bpatsubst \
+ bregexp \
+ builtin \
+ changecom \
+ changequote \
+ changeword \
+ debugfile \
+ debugmode \
+ decr \
+ define \
+ defn \
+ divert \
+ divnum \
+ dnl \
+ dumpdef \
+ errprint \
+ esyscmd \
+ eval \
+ format \
+ ifdef \
+ ifelse \
+ include \
+ incr \
+ index \
+ indir \
+ len \
+ m4exit \
+ m4wrap \
+ 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 +929,14 @@ dist-hook:
.PHONY: $(syntax_check_rules)
$(syntax_check_rules): automake aclocal
maintainer-check: $(syntax_check_rules)
+sc_test_names:
+ @m4_builtin_rx=`echo $(m4_builtins) | sed 's/ /|/g'`; \
+ m4_macro_rx="\\<($$m4_builtin_rx)\\>|\\<_?(A[CUMHS]|m4)_"; \
+ if ls tests/*.test | LC_ALL=C grep -E "$$m4_macro_rx"; then \
+ echo "the names of the tests above can be problematic" 1>&2; \
+ echo "Avoid test names that contain names of m4 macros" 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
- [PATCH] {maint} maintcheck: look for problematic names of testcases, (continued)
- [PATCH] {maint} maintcheck: look for problematic names of testcases, Stefano Lattarini, 2011/03/14
- Re: [PATCH] {maint} maintcheck: look for problematic names of testcases, Ralf Wildenhues, 2011/03/15
- Re: [PATCH] {maint} maintcheck: look for problematic names of testcases, Stefano Lattarini, 2011/03/15
- Re: [PATCH] {maint} maintcheck: look for problematic names of testcases, Ralf Wildenhues, 2011/03/16
- Re: [PATCH] {maint} maintcheck: look for problematic names of testcases, Ralf Wildenhues, 2011/03/16
- Re: [PATCH] {maint} maintcheck: look for problematic names of testcases, Stefano Lattarini, 2011/03/16
- Re: [PATCH] {maint} maintcheck: look for problematic names of testcases, Stefano Lattarini, 2011/03/16
- Re: [PATCH] {maint} maintcheck: look for problematic names of testcases, Ralf Wildenhues, 2011/03/16
- Re: [PATCH] {maint} maintcheck: look for problematic names of testcases, Stefano Lattarini, 2011/03/16
- Re: [PATCH] {maint} maintcheck: look for problematic names of testcases, Ralf Wildenhues, 2011/03/16
- Re: [PATCH] {maint} maintcheck: look for problematic names of testcases,
Stefano Lattarini <=
- Re: [PATCH] {maint} maintcheck: look for problematic names of testcases, Stefano Lattarini, 2011/03/21