[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[RFC] Refactoring maintainer checks (use perl, add whitelisting)
|
From: |
Stefano Lattarini |
|
Subject: |
[RFC] Refactoring maintainer checks (use perl, add whitelisting) |
|
Date: |
Mon, 26 Jul 2010 13:58:08 +0200 |
|
User-agent: |
KMail/1.13.3 (Linux/2.6.30-2-686; KDE/4.4.4; i686; ; ) |
Hello automakers.
This patch substitutes (almost) all the sed/grep maintainer syntax checks
in `Makefile.am' with a new perl script `maintcheck.pl'. Currently, this
is a monolithic script, but it allows the selection of a subset of checks
to be run (i.e. it doesn't force all tests to be run). Also, it could be
easily modified to let placing the definitions of checks (and the list
of files these checks are applied to) into an external "config" file
(basically, this would most probably be a perl script to be sourced with
the "do" perl builtin).
This new script allows a greater flexibilty in the definition of new
syntax checks, and, IMHO most importantly, allows to whitelist known
"false positives".
Admittedly, these new perl-based maintainer checks are quite slower, and
might take up to twice the running time than the old sed+grep maintainer
checks, but since we are speaking of 21-against-10 seconds on my slow
machine, that souldn't be a problem IMO. And, BTW, on a faster machine,
the running times are 9s (for new maintcheck) against 7s (for old
maintchecks) ...
Opinions? Comments? Objections?
Regards,
Stefano
-*-*-*-
[PATCH] Refactor maintainer checks.
* Makefile.am (sc_diff_aclocal_in_aclocal): New target, similar
to `sc_diff_automake_in_automake'.
(sc_tests_required_after_defs): Modified to consider also tests
in the builddir, and not to bail out at the first error.
(sc_no_brace_variable_expansions): Target removed, its check
are now run (more or less) by the new script `maintcheck.pl'.
(sc_rm_minus_f): Likewise.
(sc_no_for_variable_in_macro): Likewise.
(sc_mkinstalldirs): Likewise.
(sc_pre_normal_post_install_uninstall): Likewise.
(sc_perl_no_undef): Likewise.
(sc_perl_no_split_regex_space): Likewise.
(sc_cd_in_backquotes): Likewise.
(sc_cd_relative_dir): Likewise.
(sc_perl_at_uscore_in_scalar_context): Likewise.
(sc_perl_local_no_parens): Likewise.
(sc_perl_local): Likewise.
(sc_AMDEP_TRUE_in_automake_in): Likewise.
(sc_tests_make_without_am_makeflags): Likewise.
(sc_tests_plain_make): Likewise.
(sc_tests_plain_autoconf): Likewise.
(sc_tests_plain_autoupdate): Likewise.
(sc_tests_plain_automake): Likewise.
(sc_tests_here_document_format): Likewise.
(sc_tests_Exit_not_exit): Likewise.
(sc_tests_automake_fails): Likewise.
(sc_tests_plain_aclocal): Likewise.
(sc_tests_plain_perl): Likewise.
(sc_tests_overriding_macros_on_cmdline): Likewise.
(sc_tests_plain_sleep): Likewise.
(sc_tests_plain_egrep_fgrep): Likewise.
(sc_mkdir_p): Likewise.
(sc_perl_at_substs): Likewise.
(sc_unquoted_DESTDIR): Likewise.
(sc_tabs_in_texi): Likewise.
(sc_at_in_texi): Likewise.
(sc_maintcheck_pl): New target, around the new `maintcheck.pl'
script.
(EXTRA_DIST): Add the new script `maintcheck.pl'.
* maintcheck.pl: New maintainer-specific script.
---
ChangeLog | 44 +++++
Makefile.am | 348 ++++-------------------------------
Makefile.in | 296 +++---------------------------
maintcheck.pl | 571 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 676 insertions(+), 583 deletions(-)
create mode 100755 maintcheck.pl
From 408f119662265fceab83b01eb211e0c55c1b3626 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <address@hidden>
Date: Tue, 8 Jun 2010 20:04:50 +0200
Subject: [PATCH] Refactor maintainer checks.
* Makefile.am (sc_diff_aclocal_in_aclocal): New target, similar
to `sc_diff_automake_in_automake'.
(sc_tests_required_after_defs): Modified to consider also tests
in the builddir, and not to bail out at the first error.
(sc_no_brace_variable_expansions): Target removed, its check
are now run (more or less) by the new script `maintcheck.pl'.
(sc_rm_minus_f): Likewise.
(sc_no_for_variable_in_macro): Likewise.
(sc_mkinstalldirs): Likewise.
(sc_pre_normal_post_install_uninstall): Likewise.
(sc_perl_no_undef): Likewise.
(sc_perl_no_split_regex_space): Likewise.
(sc_cd_in_backquotes): Likewise.
(sc_cd_relative_dir): Likewise.
(sc_perl_at_uscore_in_scalar_context): Likewise.
(sc_perl_local_no_parens): Likewise.
(sc_perl_local): Likewise.
(sc_AMDEP_TRUE_in_automake_in): Likewise.
(sc_tests_make_without_am_makeflags): Likewise.
(sc_tests_plain_make): Likewise.
(sc_tests_plain_autoconf): Likewise.
(sc_tests_plain_autoupdate): Likewise.
(sc_tests_plain_automake): Likewise.
(sc_tests_here_document_format): Likewise.
(sc_tests_Exit_not_exit): Likewise.
(sc_tests_automake_fails): Likewise.
(sc_tests_plain_aclocal): Likewise.
(sc_tests_plain_perl): Likewise.
(sc_tests_overriding_macros_on_cmdline): Likewise.
(sc_tests_plain_sleep): Likewise.
(sc_tests_plain_egrep_fgrep): Likewise.
(sc_mkdir_p): Likewise.
(sc_perl_at_substs): Likewise.
(sc_unquoted_DESTDIR): Likewise.
(sc_tabs_in_texi): Likewise.
(sc_at_in_texi): Likewise.
(sc_maintcheck_pl): New target, around the new `maintcheck.pl'
script.
(EXTRA_DIST): Add the new script `maintcheck.pl'.
* maintcheck.pl: New maintainer-specific script.
---
ChangeLog | 44 +++++
Makefile.am | 348 ++++-------------------------------
Makefile.in | 296 +++---------------------------
maintcheck.pl | 572 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 677 insertions(+), 583 deletions(-)
create mode 100755 maintcheck.pl
diff --git a/ChangeLog b/ChangeLog
index 251ff2e..383816b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,47 @@
+2010-07-26 Stefano Lattarini <address@hidden>
+
+ Refactor maintainer checks.
+ * Makefile.am (sc_diff_aclocal_in_aclocal): New target, similar
+ to `sc_diff_automake_in_automake'.
+ (sc_tests_required_after_defs): Modified to consider also tests
+ in the builddir, and not to bail out at the first error.
+ (sc_no_brace_variable_expansions): Target removed, its check
+ are now run (more or less) by the new script `maintcheck.pl'.
+ (sc_rm_minus_f): Likewise.
+ (sc_no_for_variable_in_macro): Likewise.
+ (sc_mkinstalldirs): Likewise.
+ (sc_pre_normal_post_install_uninstall): Likewise.
+ (sc_perl_no_undef): Likewise.
+ (sc_perl_no_split_regex_space): Likewise.
+ (sc_cd_in_backquotes): Likewise.
+ (sc_cd_relative_dir): Likewise.
+ (sc_perl_at_uscore_in_scalar_context): Likewise.
+ (sc_perl_local_no_parens): Likewise.
+ (sc_perl_local): Likewise.
+ (sc_AMDEP_TRUE_in_automake_in): Likewise.
+ (sc_tests_make_without_am_makeflags): Likewise.
+ (sc_tests_plain_make): Likewise.
+ (sc_tests_plain_autoconf): Likewise.
+ (sc_tests_plain_autoupdate): Likewise.
+ (sc_tests_plain_automake): Likewise.
+ (sc_tests_here_document_format): Likewise.
+ (sc_tests_Exit_not_exit): Likewise.
+ (sc_tests_automake_fails): Likewise.
+ (sc_tests_plain_aclocal): Likewise.
+ (sc_tests_plain_perl): Likewise.
+ (sc_tests_overriding_macros_on_cmdline): Likewise.
+ (sc_tests_plain_sleep): Likewise.
+ (sc_tests_plain_egrep_fgrep): Likewise.
+ (sc_mkdir_p): Likewise.
+ (sc_perl_at_substs): Likewise.
+ (sc_unquoted_DESTDIR): Likewise.
+ (sc_tabs_in_texi): Likewise.
+ (sc_at_in_texi): Likewise.
+ (sc_maintcheck_pl): New target, around the new `maintcheck.pl'
+ script.
+ (EXTRA_DIST): Add the new script `maintcheck.pl'.
+ * maintcheck.pl: New maintainer-specific script.
+
2010-07-18 Stefano Lattarini <address@hidden>
Improve and extend test cond5.test.
diff --git a/Makefile.am b/Makefile.am
index 3aead75..b728227 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -41,6 +41,7 @@ EXTRA_DIST = \
ChangeLog.04 \
ChangeLog.09 \
bootstrap \
+ maintcheck.pl \
$(AUTOMAKESOURCES)
## Make versioned links. We only run the transform on the root name;
@@ -112,341 +113,60 @@ dist-hook:
# guaranteed to work on my machine.
syntax_check_rules = \
sc_diff_automake_in_automake \
+sc_diff_aclocal_in_aclocal \
sc_perl_syntax \
-sc_no_brace_variable_expansions \
-sc_rm_minus_f \
-sc_no_for_variable_in_macro \
-sc_mkinstalldirs \
-sc_pre_normal_post_install_uninstall \
-sc_perl_no_undef \
-sc_perl_no_split_regex_space \
-sc_cd_in_backquotes \
-sc_cd_relative_dir \
-sc_perl_at_uscore_in_scalar_context \
-sc_perl_local_no_parens \
-sc_perl_local \
-sc_AMDEP_TRUE_in_automake_in \
-sc_tests_make_without_am_makeflags \
-sc_tests_plain_make \
-sc_tests_plain_autoconf \
-sc_tests_plain_autoupdate \
-sc_tests_plain_automake \
-sc_tests_here_document_format \
-sc_tests_Exit_not_exit \
-sc_tests_automake_fails \
-sc_tests_plain_aclocal \
-sc_tests_plain_perl \
sc_tests_required_after_defs \
-sc_tests_overriding_macros_on_cmdline \
-sc_tests_plain_sleep \
-sc_tests_plain_egrep_fgrep \
-sc_mkdir_p \
-sc_perl_at_substs \
-sc_unquoted_DESTDIR \
-sc_tabs_in_texi \
-sc_at_in_texi
+sc_maintcheck_pl
.PHONY: $(syntax_check_rules)
$(syntax_check_rules): automake aclocal
maintainer-check: $(syntax_check_rules)
-## 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.
+## The grater majority of static syntax checks are run by the
+## `maintcheck.pl' script.
+sc_maintcheck_pl:
+ cd $(top_srcdir) && $(PERL) -w maintcheck.pl --enable-checks=ALL
+
+## These checks avoid accidental configure substitutions in the source.
+## There are exactly 8 lines that should be modified in automake,in, and
+## 9 in aclocal.in. This works out to 28 lines of diffs for automake,
+## and 30 for aclocal.
sc_diff_automake_in_automake:
@if test `diff $(srcdir)/automake.in automake | wc -l` -ne 28; then \
echo "found too many diffs between automake.in and automake" 1>&2; \
diff -c $(srcdir)/automake.in automake; \
exit 1; \
fi
+sc_diff_aclocal_in_aclocal:
+ @if test `diff $(srcdir)/aclocal.in aclocal | wc -l` -ne 30; then \
+ echo "found too many diffs between aclocal.in and aclocal" 1>&2; \
+ diff -c $(srcdir)/aclocal.in aclocal; \
+ exit 1; \
+ fi
-## Syntax check with default Perl (on my machine, Perl 5).
+## Syntax check with default Perl.
sc_perl_syntax:
perllibdir="./lib$(PATH_SEPARATOR)$(srcdir)/lib" $(PERL) -c -w automake
perllibdir="./lib$(PATH_SEPARATOR)$(srcdir)/lib" $(PERL) -c -w aclocal
-## expect no instances of '${...}'. However, $${...} is ok, since that
-## is a shell construct, not a Makefile construct.
-sc_no_brace_variable_expansions:
- @if grep -F '$${' $(srcdir)/lib/am/[a-z]*.am | \
- grep -F -v '$$$$'; then \
- echo "Found too many uses of '\$${' in the lines above." 1>&2; \
- exit 1; \
- else :; fi
-
-## Make sure `rm' is called with `-f'.
-sc_rm_minus_f:
- @if grep -v '^#' $(srcdir)/lib/am/[a-z]*.am $(srcdir)/tests/*.test | \
- grep -E '\<rm ([^-]|\-[^f ]*\>)'; then \
- echo "Suspicious 'rm' invocation." 1>&2; \
- exit 1; \
- else :; fi
-
-## Never use something like `for file in $(FILES)', this doesn't work
-## if FILES is empty or if it contains shell meta characters (e.g. $ is
-## commonly used in Java filenames).
-sc_no_for_variable_in_macro:
- @if grep 'for .* in \$$(' $(srcdir)/lib/am/[a-z]*.am; then \
- echo 'Use "list=$$(mumble); for var in $$$$list".' 1>&2 ; \
- exit 1; \
- else :; fi
-
-## Make sure all invocations of mkinstalldirs are correct.
-sc_mkinstalldirs:
- @if grep -n 'mkinstalldirs' $(srcdir)/lib/am/[a-z]*.am | \
- grep -F -v '$$(mkinstalldirs)'; then \
- echo "Found incorrect use of mkinstalldirs in the lines above" 1>&2; \
- exit 1; \
- else :; fi
-
-## Make sure all calls to PRE/NORMAL/POST_INSTALL/UNINSTALL
-sc_pre_normal_post_install_uninstall:
- @if grep -E -n '\((PRE|NORMAL|POST)_(|UN)INSTALL\)' \
- $(srcdir)/lib/am/[a-z]*.am | \
- grep -v ':##' | grep -v ': @\$$('; then \
- echo "Found incorrect use of PRE/NORMAL/POST_INSTALL/UNINSTALL in the
lines above" 1>&2; \
- exit 1; \
- else :; fi
-
-## We never want to use "undef", only "delete", but for $/.
-sc_perl_no_undef:
- @if grep -n -w 'undef ' $(srcdir)/automake.in | \
- grep -F -v 'undef $$/'; then \
- echo "Found undef in automake.in; use delete instead" 1>&2; \
- exit 1; \
- fi
-
-## We never want split (/ /,...), only split (' ', ...).
-sc_perl_no_split_regex_space:
- @if grep -n 'split (/ /' $(srcdir)/automake.in; then \
- echo "Found bad split in the lines above." 1>&2; \
- exit 1; \
- fi
-
-## Look for cd within backquotes
-sc_cd_in_backquotes:
- @if grep -n '^[^#]*` *cd ' $(srcdir)/automake.in \
- $(srcdir)/lib/am/*.am; then \
- echo "Consider using \$$(am__cd) in the lines above." 1>&2; \
- exit 1; \
- fi
-
-## Look for cd to a relative directory (may be influenced by CDPATH).
-## Skip some known directories that are OK.
-sc_cd_relative_dir:
- @if grep -n '^[^#]*cd ' $(srcdir)/automake.in \
- $(srcdir)/lib/am/*.am | \
- grep -v 'echo.*cd ' | \
- grep -v 'am__cd =' | \
- grep -v '^[^#]*cd [./]' | \
- grep -v '^[^#]*cd \$$(top_builddir)' | \
- grep -v '^[^#]*cd "\$$\$$am__cwd' | \
- grep -v '^[^#]*cd \$$(abs' | \
- grep -v '^[^#]*cd "\$$(DESTDIR)'; then \
- echo "Consider using \$$(am__cd) in the lines above." 1>&2; \
- exit 1; \
- fi
-
-## Using @_ in a scalar context is most probably a programming error.
-sc_perl_at_uscore_in_scalar_context:
- @if grep -Hn 'address@hidden) ] *= address@hidden'
$(srcdir)/automake.in; then \
- echo "Using @_ in a scalar context in the lines above." 1>&2; \
- exit 1; \
- fi
-
-## Forbid using parens with `local' to ease counting.
-sc_perl_local_no_parens:
- @if grep '^[ \t]*local *(' $(srcdir)/automake.in; then \
- echo "Don't use \`local' with parens: use several \`local' above."
>&2; \
- exit 1; \
- fi
-
-## Allow only `local $_' in Automake.
-sc_perl_local:
- @if grep -v '^[ \t]*local \$$_;' $(srcdir)/automake.in | \
- grep '^[ \t]*local [^*]'; then \
- echo "Please avoid \`local'." 1>&2; \
- exit 1; \
- fi
-
-## Don't let AMDEP_TRUE substitution appear in automake.in.
-sc_AMDEP_TRUE_in_automake_in:
- @if grep '@AMDEP''_TRUE@' $(srcdir)/automake.in; then \
- echo "Don't put AMDEP_TRUE substitution in automake.in" 1>&2; \
- exit 1; \
- fi
-
-## Tests should never call make directly.
-sc_tests_make_without_am_makeflags:
- @if grep '^[^#].*(MAKE) ' $(srcdir)/lib/am/*.am $(srcdir)/automake.in |\
- grep -v 'AM_MAKEFLAGS'; then \
- echo 'Use $$(MAKE) $$(AM_MAKEFLAGS).' 1>&2; \
- exit 1; \
- fi
-
-## Tests should never call make directly.
-sc_tests_plain_make:
- @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[ ]*make'; then \
- echo 'Do not run "make" in the above tests. Use "$$MAKE" instead.'
1>&2; \
- exit 1; \
- fi
-
-## Tests should never call autoconf directly.
-sc_tests_plain_autoconf:
- @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[ ]*autoconf';
then \
- echo 'Do not run "autoconf" in the above tests. Use "$$AUTOCONF"
instead.' 1>&2; \
- exit 1; \
- fi
-
-## Tests should never call autoupdate directly.
-sc_tests_plain_autoupdate:
- @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[ ]*autoupdate';
then \
- echo 'Do not run "autoupdate" in the above tests. Use "$$AUTOUPDATE"
instead.' 1>&2; \
- exit 1; \
- fi
-
-## Tests should never call automake directly.
-sc_tests_plain_automake:
- @if grep -v '^#' $(srcdir)/tests/*.test | grep -E ':[
]*automake([^:]|$$)'; then \
- echo 'Do not run "automake" in the above tests. Use "$$AUTOMAKE"
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:
- @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; \
- exit 1; \
- fi
-
-## Tests should never call exit directly, but use Exit.
-## This is so that the exit status is transported correctly across the 0 trap.
-## Ignore comments, and ignore one perl line in ext2.test.
-sc_tests_Exit_not_exit:
- @found=false; for file in $(srcdir)/tests/*.test; do \
- res=`sed -n -e '/^#/d; /^\$$PERL/d' -e '/<<.*END/,/^END/b' \
- -e '/<<.*EOF/,/^EOF/b' -e '/exit [$$0-9]/p' $$file`; \
- if test -n "$$res"; then \
- echo "$$file:$$res"; \
- found=true; \
- fi; \
- done; \
- if $$found; then \
- echo 'Do not call plain "exit", use "Exit" instead, in above tests.'
1>&2; \
- exit 1; \
- fi
-
-## Use AUTOMAKE_fails when appropriate
-sc_tests_automake_fails:
- @if grep -v '^#' $(srcdir)/tests/*.test | grep
'\$$AUTOMAKE.*&&.*[eE]xit'; then \
- echo 'Use AUTOMAKE_fails + grep to catch automake failures in the
above tests.' 1>&2; \
- exit 1; \
- fi
-
-## Tests should never call aclocal directly.
-sc_tests_plain_aclocal:
- @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
-
-## Tests should never call perl directly.
-sc_tests_plain_perl:
- @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[ ]*perl'; then \
- echo 'Do not run "perl" in the above tests. Use "$$PERL" instead.'
1>&2; \
- exit 1; \
- fi
-
## Setting `required' after sourcing `./defs' is a bug.
sc_tests_required_after_defs:
- @for file in $(srcdir)/tests/*.test; do \
- if out=`sed -n '/defs/,$${/required=/p;}' $$file`; test -n "$$out";
then \
- echo 'Do not set "required" after sourcing "defs" in '"$$file:
$$out" 1>&2; \
- exit 1; \
+ @estatus=0; \
+ if test x$(srcdir) = x$(builddir); then \
+ files='$(srcdir)/tests/*.test $(builddir)/tests/*.test'; \
+ else \
+ files='$(srcdir)/tests/*.test $(builddir)/tests/*.test'; \
+ fi; \
+ for file in $$files; do \
+ test -f $$file || continue; \
+ out=`sed -n '/defs/,$${/required=/p;}' $$file`; \
+ if test -n "$$out"; then \
+ echo '$@: do not set 'required' after sourcing "defs" in' \
+ "$$file: $$out" 1>&2; \
+ estatus=1; \
fi; \
- done
-
-## Overriding a Makefile macro on the command line is not portable when
-## recursive targets are used. Better use an envvar. SHELL is an exception,
-## POSIX says it can't come from the environment. V and DESTDIRS are
exceptions,
-## too, as package authors are urged not to initialize them anywhere.
-sc_tests_overriding_macros_on_cmdline:
- @if grep -E '\$$MAKE .*(SHELL=.*=|=.*SHELL=)' $(srcdir)/tests/*.test;
then \
- echo 'Rewrite "$$MAKE foo=bar SHELL=$$SHELL" as "foo=bar $$MAKE -e
SHELL=$$SHELL"' 1>&2; \
- echo ' in the above lines, it is more portable.' 1>&2; \
- exit 1; \
- fi
- @if sed 's/DESTDIR=[^ ]*//; s/SHELL=[^ ]*//; s/V=[^ ]*//'
$(srcdir)/tests/*.test | \
- grep '\$$MAKE .*=' ; then \
- echo 'Rewrite "$$MAKE foo=bar" as "foo=bar $$MAKE -e" in the above
lines,' 1>&2; \
- echo 'it is more portable.' 1>&2; \
- exit 1; \
- fi
- @if grep 'SHELL=.*\$$MAKE' $(srcdir)/tests/*.test; then \
- echo '$$MAKE ignores the SHELL envvar, use "$$MAKE SHELL=$$SHELL" in'
1>&2; \
- echo 'the above lines.' 1>&2; \
- exit 1; \
- fi
-
-## Never use `sleep 1' to create files with different timestamps.
-## Use `$sleep' instead. Some filesystems (e.g., Windows') have only
-## a 2sec resolution.
-sc_tests_plain_sleep:
- @if grep -E '\bsleep +[12345]\b' $(srcdir)/tests/*.test; then \
- echo 'Do not use "sleep x" in the above tests. Use "$$sleep"
instead.' 1>&2; \
- exit 1; \
- fi
-
-## fgrep and egrep are not required by POSIX.
-sc_tests_plain_egrep_fgrep:
- @if grep -E '\b[ef]grep\b' $(srcdir)/tests/*.test ; then \
- echo 'Do not use egrep or fgrep in test cases. Use $$FGREP or
$$EGREP.' 1>&2; \
- exit 1; \
- fi
- @if grep -E '\b[ef]grep\b' $(srcdir)/lib/am/*.am $(srcdir)/m4/*.m4;
then \
- echo 'Do not use egrep or fgrep in the above files, they are not
portable.' 1>&2; \
- exit 1; \
- fi
-
-sc_mkdir_p:
- @if grep 'mkdir_p' $(srcdir)/automake.in \
- $(srcdir)/lib/am/*.am $(srcdir)/tests/*.test; then \
- echo 'Do not use mkdir_p in the above files, use MKDIR_P.' 1>&2; \
- exit 1; \
- fi
-
-## Try to make sure all @...@ substitutions are covered by our
-## substitution rule.
-sc_perl_at_substs:
- @if test `grep -E 'address@hidden@' aclocal | wc -l` -ne 0; then \
- echo "Unresolved @...@ substitution in aclocal" 1>&2; \
- exit 1; \
- fi
- @if test `grep -E 'address@hidden@' automake | wc -l` -ne 0; then \
- echo "Unresolved @...@ substitution in automake" 1>&2; \
- exit 1; \
- fi
-
-sc_unquoted_DESTDIR:
- @if grep -E "[^\'\"]\\\$$\(DESTDIR" $(srcdir)/lib/am/*.am; then \
- echo 'Suspicious unquoted DESTDIR uses.' 1>&2 ; \
- exit 1; \
- fi
-
-sc_tabs_in_texi:
- @if grep ' ' $(srcdir)/doc/automake.texi; then \
- echo 'Do not use tabs in the manual.' 1>&2; \
- exit 1; \
- fi
-
-sc_at_in_texi:
- @if grep -E '(address@hidden|^)@([ address@hidden|$$)'
$(srcdir)/doc/automake.texi; \
- then \
- echo 'Unescaped @.' 1>&2; \
- exit 1; \
- fi
-
+ done; \
+ exit $$estatus
git-dist: maintainer-check
## Make sure the NEWS file is up-to-date.
diff --git a/Makefile.in b/Makefile.in
index 528b80e..57a60c1 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -250,6 +250,7 @@ EXTRA_DIST = \
ChangeLog.04 \
ChangeLog.09 \
bootstrap \
+ maintcheck.pl \
$(AUTOMAKESOURCES)
do_subst = sed \
@@ -270,39 +271,10 @@ do_subst = sed \
# guaranteed to work on my machine.
syntax_check_rules = \
sc_diff_automake_in_automake \
+sc_diff_aclocal_in_aclocal \
sc_perl_syntax \
-sc_no_brace_variable_expansions \
-sc_rm_minus_f \
-sc_no_for_variable_in_macro \
-sc_mkinstalldirs \
-sc_pre_normal_post_install_uninstall \
-sc_perl_no_undef \
-sc_perl_no_split_regex_space \
-sc_cd_in_backquotes \
-sc_cd_relative_dir \
-sc_perl_at_uscore_in_scalar_context \
-sc_perl_local_no_parens \
-sc_perl_local \
-sc_AMDEP_TRUE_in_automake_in \
-sc_tests_make_without_am_makeflags \
-sc_tests_plain_make \
-sc_tests_plain_autoconf \
-sc_tests_plain_autoupdate \
-sc_tests_plain_automake \
-sc_tests_here_document_format \
-sc_tests_Exit_not_exit \
-sc_tests_automake_fails \
-sc_tests_plain_aclocal \
-sc_tests_plain_perl \
sc_tests_required_after_defs \
-sc_tests_overriding_macros_on_cmdline \
-sc_tests_plain_sleep \
-sc_tests_plain_egrep_fgrep \
-sc_mkdir_p \
-sc_perl_at_substs \
-sc_unquoted_DESTDIR \
-sc_tabs_in_texi \
-sc_at_in_texi
+sc_maintcheck_pl
WGET = wget
WGET_SV_CVS = $(WGET) http://savannah.gnu.org/cgi-bin/viewcvs/~checkout~/
@@ -874,257 +846,43 @@ dist-hook:
$(syntax_check_rules): automake aclocal
maintainer-check: $(syntax_check_rules)
+sc_maintcheck_pl:
+ cd $(top_srcdir) && $(PERL) -w maintcheck.pl --enable-checks=ALL
+
sc_diff_automake_in_automake:
@if test `diff $(srcdir)/automake.in automake | wc -l` -ne 28; then \
echo "found too many diffs between automake.in and automake" 1>&2; \
diff -c $(srcdir)/automake.in automake; \
exit 1; \
fi
+sc_diff_aclocal_in_aclocal:
+ @if test `diff $(srcdir)/aclocal.in aclocal | wc -l` -ne 30; then \
+ echo "found too many diffs between aclocal.in and aclocal" 1>&2; \
+ diff -c $(srcdir)/aclocal.in aclocal; \
+ exit 1; \
+ fi
sc_perl_syntax:
perllibdir="./lib$(PATH_SEPARATOR)$(srcdir)/lib" $(PERL) -c -w automake
perllibdir="./lib$(PATH_SEPARATOR)$(srcdir)/lib" $(PERL) -c -w aclocal
-sc_no_brace_variable_expansions:
- @if grep -F '$${' $(srcdir)/lib/am/[a-z]*.am | \
- grep -F -v '$$$$'; then \
- echo "Found too many uses of '\$${' in the lines above." 1>&2; \
- exit 1; \
- else :; fi
-
-sc_rm_minus_f:
- @if grep -v '^#' $(srcdir)/lib/am/[a-z]*.am $(srcdir)/tests/*.test | \
- grep -E '\<rm ([^-]|\-[^f ]*\>)'; then \
- echo "Suspicious 'rm' invocation." 1>&2; \
- exit 1; \
- else :; fi
-
-sc_no_for_variable_in_macro:
- @if grep 'for .* in \$$(' $(srcdir)/lib/am/[a-z]*.am; then \
- echo 'Use "list=$$(mumble); for var in $$$$list".' 1>&2 ; \
- exit 1; \
- else :; fi
-
-sc_mkinstalldirs:
- @if grep -n 'mkinstalldirs' $(srcdir)/lib/am/[a-z]*.am | \
- grep -F -v '$$(mkinstalldirs)'; then \
- echo "Found incorrect use of mkinstalldirs in the lines above" 1>&2; \
- exit 1; \
- else :; fi
-
-sc_pre_normal_post_install_uninstall:
- @if grep -E -n '\((PRE|NORMAL|POST)_(|UN)INSTALL\)' \
- $(srcdir)/lib/am/[a-z]*.am | \
- grep -v ':##' | grep -v ': @\$$('; then \
- echo "Found incorrect use of PRE/NORMAL/POST_INSTALL/UNINSTALL in the
lines above" 1>&2; \
- exit 1; \
- else :; fi
-
-sc_perl_no_undef:
- @if grep -n -w 'undef ' $(srcdir)/automake.in | \
- grep -F -v 'undef $$/'; then \
- echo "Found undef in automake.in; use delete instead" 1>&2; \
- exit 1; \
- fi
-
-sc_perl_no_split_regex_space:
- @if grep -n 'split (/ /' $(srcdir)/automake.in; then \
- echo "Found bad split in the lines above." 1>&2; \
- exit 1; \
- fi
-
-sc_cd_in_backquotes:
- @if grep -n '^[^#]*` *cd ' $(srcdir)/automake.in \
- $(srcdir)/lib/am/*.am; then \
- echo "Consider using \$$(am__cd) in the lines above." 1>&2; \
- exit 1; \
- fi
-
-sc_cd_relative_dir:
- @if grep -n '^[^#]*cd ' $(srcdir)/automake.in \
- $(srcdir)/lib/am/*.am | \
- grep -v 'echo.*cd ' | \
- grep -v 'am__cd =' | \
- grep -v '^[^#]*cd [./]' | \
- grep -v '^[^#]*cd \$$(top_builddir)' | \
- grep -v '^[^#]*cd "\$$\$$am__cwd' | \
- grep -v '^[^#]*cd \$$(abs' | \
- grep -v '^[^#]*cd "\$$(DESTDIR)'; then \
- echo "Consider using \$$(am__cd) in the lines above." 1>&2; \
- exit 1; \
- fi
-
-sc_perl_at_uscore_in_scalar_context:
- @if grep -Hn 'address@hidden) ] *= address@hidden'
$(srcdir)/automake.in; then \
- echo "Using @_ in a scalar context in the lines above." 1>&2; \
- exit 1; \
- fi
-
-sc_perl_local_no_parens:
- @if grep '^[ \t]*local *(' $(srcdir)/automake.in; then \
- echo "Don't use \`local' with parens: use several \`local' above."
>&2; \
- exit 1; \
- fi
-
-sc_perl_local:
- @if grep -v '^[ \t]*local \$$_;' $(srcdir)/automake.in | \
- grep '^[ \t]*local [^*]'; then \
- echo "Please avoid \`local'." 1>&2; \
- exit 1; \
- fi
-
-sc_AMDEP_TRUE_in_automake_in:
- @if grep '@AMDEP''_TRUE@' $(srcdir)/automake.in; then \
- echo "Don't put AMDEP_TRUE substitution in automake.in" 1>&2; \
- exit 1; \
- fi
-
-sc_tests_make_without_am_makeflags:
- @if grep '^[^#].*(MAKE) ' $(srcdir)/lib/am/*.am $(srcdir)/automake.in |\
- grep -v 'AM_MAKEFLAGS'; then \
- echo 'Use $$(MAKE) $$(AM_MAKEFLAGS).' 1>&2; \
- exit 1; \
- fi
-
-sc_tests_plain_make:
- @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[ ]*make'; then \
- echo 'Do not run "make" in the above tests. Use "$$MAKE" instead.'
1>&2; \
- exit 1; \
- fi
-
-sc_tests_plain_autoconf:
- @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[ ]*autoconf';
then \
- echo 'Do not run "autoconf" in the above tests. Use "$$AUTOCONF"
instead.' 1>&2; \
- exit 1; \
- fi
-
-sc_tests_plain_autoupdate:
- @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[ ]*autoupdate';
then \
- echo 'Do not run "autoupdate" in the above tests. Use "$$AUTOUPDATE"
instead.' 1>&2; \
- exit 1; \
- fi
-
-sc_tests_plain_automake:
- @if grep -v '^#' $(srcdir)/tests/*.test | grep -E ':[
]*automake([^:]|$$)'; then \
- echo 'Do not run "automake" in the above tests. Use "$$AUTOMAKE"
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; \
- exit 1; \
- fi
-
-sc_tests_Exit_not_exit:
- @found=false; for file in $(srcdir)/tests/*.test; do \
- res=`sed -n -e '/^#/d; /^\$$PERL/d' -e '/<<.*END/,/^END/b' \
- -e '/<<.*EOF/,/^EOF/b' -e '/exit [$$0-9]/p' $$file`; \
- if test -n "$$res"; then \
- echo "$$file:$$res"; \
- found=true; \
- fi; \
- done; \
- if $$found; then \
- echo 'Do not call plain "exit", use "Exit" instead, in above tests.'
1>&2; \
- exit 1; \
- fi
-
-sc_tests_automake_fails:
- @if grep -v '^#' $(srcdir)/tests/*.test | grep
'\$$AUTOMAKE.*&&.*[eE]xit'; then \
- echo 'Use AUTOMAKE_fails + grep to catch automake failures in the
above tests.' 1>&2; \
- exit 1; \
- fi
-
-sc_tests_plain_aclocal:
- @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
-
-sc_tests_plain_perl:
- @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[ ]*perl'; then \
- echo 'Do not run "perl" in the above tests. Use "$$PERL" instead.'
1>&2; \
- exit 1; \
- fi
-
sc_tests_required_after_defs:
- @for file in $(srcdir)/tests/*.test; do \
- if out=`sed -n '/defs/,$${/required=/p;}' $$file`; test -n "$$out";
then \
- echo 'Do not set "required" after sourcing "defs" in '"$$file:
$$out" 1>&2; \
- exit 1; \
+ @estatus=0; \
+ if test x$(srcdir) = x$(builddir); then \
+ files='$(srcdir)/tests/*.test $(builddir)/tests/*.test'; \
+ else \
+ files='$(srcdir)/tests/*.test $(builddir)/tests/*.test'; \
+ fi; \
+ for file in $$files; do \
+ test -f $$file || continue; \
+ out=`sed -n '/defs/,$${/required=/p;}' $$file`; \
+ if test -n "$$out"; then \
+ echo '$@: do not set 'required' after sourcing "defs" in' \
+ "$$file: $$out" 1>&2; \
+ estatus=1; \
fi; \
- done
-
-sc_tests_overriding_macros_on_cmdline:
- @if grep -E '\$$MAKE .*(SHELL=.*=|=.*SHELL=)' $(srcdir)/tests/*.test;
then \
- echo 'Rewrite "$$MAKE foo=bar SHELL=$$SHELL" as "foo=bar $$MAKE -e
SHELL=$$SHELL"' 1>&2; \
- echo ' in the above lines, it is more portable.' 1>&2; \
- exit 1; \
- fi
- @if sed 's/DESTDIR=[^ ]*//; s/SHELL=[^ ]*//; s/V=[^ ]*//'
$(srcdir)/tests/*.test | \
- grep '\$$MAKE .*=' ; then \
- echo 'Rewrite "$$MAKE foo=bar" as "foo=bar $$MAKE -e" in the above
lines,' 1>&2; \
- echo 'it is more portable.' 1>&2; \
- exit 1; \
- fi
- @if grep 'SHELL=.*\$$MAKE' $(srcdir)/tests/*.test; then \
- echo '$$MAKE ignores the SHELL envvar, use "$$MAKE SHELL=$$SHELL" in'
1>&2; \
- echo 'the above lines.' 1>&2; \
- exit 1; \
- fi
-
-sc_tests_plain_sleep:
- @if grep -E '\bsleep +[12345]\b' $(srcdir)/tests/*.test; then \
- echo 'Do not use "sleep x" in the above tests. Use "$$sleep"
instead.' 1>&2; \
- exit 1; \
- fi
-
-sc_tests_plain_egrep_fgrep:
- @if grep -E '\b[ef]grep\b' $(srcdir)/tests/*.test ; then \
- echo 'Do not use egrep or fgrep in test cases. Use $$FGREP or
$$EGREP.' 1>&2; \
- exit 1; \
- fi
- @if grep -E '\b[ef]grep\b' $(srcdir)/lib/am/*.am $(srcdir)/m4/*.m4;
then \
- echo 'Do not use egrep or fgrep in the above files, they are not
portable.' 1>&2; \
- exit 1; \
- fi
-
-sc_mkdir_p:
- @if grep 'mkdir_p' $(srcdir)/automake.in \
- $(srcdir)/lib/am/*.am $(srcdir)/tests/*.test; then \
- echo 'Do not use mkdir_p in the above files, use MKDIR_P.' 1>&2; \
- exit 1; \
- fi
-
-sc_perl_at_substs:
- @if test `grep -E 'address@hidden@' aclocal | wc -l` -ne 0; then \
- echo "Unresolved @...@ substitution in aclocal" 1>&2; \
- exit 1; \
- fi
- @if test `grep -E 'address@hidden@' automake | wc -l` -ne 0; then \
- echo "Unresolved @...@ substitution in automake" 1>&2; \
- exit 1; \
- fi
-
-sc_unquoted_DESTDIR:
- @if grep -E "[^\'\"]\\\$$\(DESTDIR" $(srcdir)/lib/am/*.am; then \
- echo 'Suspicious unquoted DESTDIR uses.' 1>&2 ; \
- exit 1; \
- fi
-
-sc_tabs_in_texi:
- @if grep ' ' $(srcdir)/doc/automake.texi; then \
- echo 'Do not use tabs in the manual.' 1>&2; \
- exit 1; \
- fi
-
-sc_at_in_texi:
- @if grep -E '(address@hidden|^)@([ address@hidden|$$)'
$(srcdir)/doc/automake.texi; \
- then \
- echo 'Unescaped @.' 1>&2; \
- exit 1; \
- fi
+ done; \
+ exit $$estatus
git-dist: maintainer-check
@if sed 1q $(srcdir)/NEWS | grep -e "$(VERSION)" > /dev/null; then :;
else \
diff --git a/maintcheck.pl b/maintcheck.pl
new file mode 100755
index 0000000..0402f3e
--- /dev/null
+++ b/maintcheck.pl
@@ -0,0 +1,572 @@
+#!/usr/bin/env perl
+# -*- perl -*-
+# Script to run static maintainer checks.
+# Especially useful to look for and spot deprecated commands/constructs.
+# Written by Stefano Lattarini.
+#
+# 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 3, 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/>.
+#
+
+require 5.008; # FIXME: does this script work with an older version too?
+
+BEGIN { $^W = 1; }
+use strict;
+use warnings FATAL => "all";
+
+(my $me = $0) =~ s|.*/||;
+
+# Tell whether we should avoid complaining on whitelisted failures.
+my $use_whitelist = 1;
+
+# Files our checks might be runned against.
+# FIXME FIXME FIXME -- support $(srcdir) != $(builddir)
+my %files = (
+ automake_in => [ 'automake.in' ],
+ automake => [ 'automake' ],
+ aclocal_in => [ 'aclocal.in' ],
+ aclocal => [ 'aclocal' ],
+ # relevant input files processed by config.status
+ in => [ qw(automake.in aclocal.in tests/defs.in) ],
+ # relevant output of config.status substitutions from *.in files
+ out => [ qw(automake aclocal tests/defs) ],
+ # makefile fragments
+ am => [ glob('lib/am/[a-z]*.am') ],
+ # perl modules
+ pm => [ glob('lib/Automake/*.pm') ],
+ # unprocessed perl scripts
+ pl => [ qw(automake.in aclocal.in maintcheck.pl) ],
+ # perl test scripts
+ pl_test => [ glob('lib/Automake/tests/*.pl') ],
+ # shell test scripts and libraries
+ sh_test => [ 'tests/defs.in', glob('tests/*.test') ],
+ # simple text file
+ txt => [ qw(README HACKING README-alpha tests/README) ],
+ # texinfo files
+ texi => [ 'doc/automake.texi' ],
+);
+
+ALL_FILES:{
+ my %all_files = ();
+ foreach my $lst (values %files) {
+ foreach my $f (@{$lst}) {
+ $all_files{$f} = 1;
+ }
+ }
+ $files{all} = [ +keys(%all_files) ];
+}
+
+# The hash of static checks to be run. Configurable by the user
+# through the command line. Note that no check is run by default!
+my %enabled_checks;
+
+# The hash of all possible static checks to be run on test scripts.
+# Indexed by check name, it's values are proper hash references.
+my %all_checks;
+
+
+### DEFINE THE CHECKS ###
+
+$all_checks{tabs_in_texi} = {
+ bad_match => qr/\t/,
+ description => 'tab characters in texinfo files.',
+ rewrite_tips => 'do not use tabs in texinfo files.',
+ files => $files{texi},
+};
+
+$all_checks{at_in_texi} = {
+ bad_match => qr/(?:address@hidden|^)@(?:address@hidden|[\$])/,
+ description => 'unescaped address@hidden character in texinfo files.',
+ files => $files{texi},
+};
+
+# Trailing whitespaces should be zapped.
+$all_checks{no_trailing_spaces} = {
+ bad_match => qr/\s+$/,
+ description => 'trailing white spaces',
+ files => $files{all},
+};
+
+# The `$status' variable is special in Zsh, and should never be used or
+# assigned.
+$all_checks{status_shell_variable} = {
+ bad_match => qr/\bstatus=|[\$]\{?status\b/,
+ description => 'use of `$status` shell variable',
+ rewrite_tips => 'use `$?` for exit status, `rc` as variable name',
+ must_skip => qr/^\s*#/,
+ files => address@hidden, @{$files{sh_test}}],
+};
+
+# In makefile fragments, expect no instances of '${...}'.
+# However, $${...} is ok, since that is a shell construct,
+# not a Makefile construct.
+$all_checks{no_brace_variable_expansions} = {
+ bad_match => qr/(?:^|[^\$])[\$]{/,
+ description => 'make variable expanded with `${...}`',
+ rewrite_tips => 'use `$(...)` instead of ${...}',
+ files => $files{am},
+};
+
+# Make sure `rm' is always called only as either `rm -f' or `rm -rf'.
+$all_checks{rm_minus_f} = {
+ bad_match => qr/\brm\s(?!(-f\b|-rf\b))/,
+ description => 'suspicious `rm` invocation',
+ rewrite_tips => 'always use `rm -rf` or `rm -f`',
+ must_skip => qr/^\s*#/,
+ files => address@hidden, @{$files{automake_in}}, @{$files{sh_test}}],
+};
+
+# In makefile fragments, never use something like `for file in $(FILES)',
+# as this doesn't work if FILES is empty or if it contains shell meta
+# characters (e.g. $ is commonly used in Java filenames).
+$all_checks{no_for_variable_in_macro} = {
+ bad_match => qr/\bfor\s.*\sin\s[\$]\(/,
+ description => 'suspicious `for` loop in makefile rule',
+ rewrite_tips => 'use "list=$(mumble); for var in $$list" instead',
+ files => address@hidden, @{$files{automake_in}}],
+};
+
+# $(DESTDIR) should never be used unquoted in makefile fragments.
+$all_checks{unquoted_DESTDIR} = {
+ bad_match => qr/(?:^|[^'"])[\$]\(DESTDIR/,
+ description => 'suspicious unquoted DESTDIR use(s)',
+ files => address@hidden, @{$files{automake_in}}],
+};
+
+# $(mkdir_p) is obsolete, we should use $(MKDIR_P).
+$all_checks{mkdir_p} = {
+ bad_match => qr/\bmkdir_p\b/,
+ description => 'suspicious string `mkdir_p` seen',
+ rewrite_tips => 'do not use $(mkdir_p), use $(MKDIR_P) instead',
+ files => address@hidden, @{$files{automake_in}}, @{$files{sh_test}}],
+};
+
+# Make sure all invocations of mkinstalldirs are correct.
+$all_checks{mkinstalldirs} = {
+ bad_match => sub {
+ local $_ = shift;
+ s/[\$]\(mkinstalldirs\)//g;
+ return (m/mkinstalldirs/ ? 1 : 0);
+ },
+ description => 'incorrect use of mkinstalldirs',
+ must_skip => qr/^\s*##/,
+ files => $files{am},
+};
+
+# Make sure all calls to PRE/NORMAL/POST_INSTALL/UNINSTALL
+$all_checks{pre_normal_post_install_uninstall} = {
+ bad_match => sub {
+ local $_ = shift;
+ (my $file_base = shift) =~ s,.*/,,;
+ s/\b(?:PRE|NORMAL|POST)_(?:UN)?INSTALL\b/POST_INSTALL/g;
+ s/address@hidden(POST_INSTALL\)//;
+ s/^POST_INSTALL = :$// if $file_base eq 'header-vars.am';
+ return (m/POST_INSTALL/ ? 1 : 0);
+ },
+ description => 'incorrect use of PRE/NORMAL/POST_INSTALL/UNINSTALL',
+ must_skip => qr/^\s*##/,
+ files => $files{am},
+};
+
+# Look for cd within backquotes in makefile fragments.
+$all_checks{cd_in_backquotes} = {
+ bad_match => qr/`.*cd\s/, # crude, yet doesn't give false positives
+ description => 'possible use of bare `cd` into backquote',
+ rewrite_tips => 'consider using $(am__cd) instead',
+ must_skip => qr/^\s*#/,
+ files => address@hidden, @{$files{automake_in}},],
+};
+
+# In makefile fragments, Look for cd to a relative directory (may be
+# influenced by CDPATH).
+$all_checks{cd_relative_dir} = {
+ bad_match => sub {
+ local $_ = shift;
+ return 0 if /\bcd\s/; # optimize
+ # Skip some known directories that are OK.
+ s{\bcd \.\.?/}{cd /x/};
+ s{\bcd "[\$][\$]am__cwd"}{cd /x};
+ s{\bcd "[\$]\(DESTDIR\)}{cd /dst/};
+ s{\bcd [\$]\(top_builddir\) &&}{cd /x &&};
+ # Skip "fake" cd (used to verbosely display commands to be run).
+ s{\becho "[^"]*\bcd [^"]*"}{echo x};
+ s{\becho '[^']*\bcd [^']*'}{echo x};
+ return (m{\bcd\s+[^/]} ? 1 : 0);
+ },
+ description => 'possibly dangerous use of bare `cd`',
+ rewrite_tips => 'consider using `$(am__cd)` instead',
+ must_skip => qr/^\s*#/,
+ files => address@hidden, @{$files{automake_in}},],
+};
+
+# Recursive make calls must be passed down the $(AM_MAKEFLAGS) variable.
+$all_checks{tests_make_without_am_makeflags} = {
+ bad_match => sub {
+ local $_ = shift;
+ return 0 if index($_, '$(MAKE)') < 0; # optimize
+ # consider the `\' before `$' for strings embedded in automake.in
+ s/(\\?)[\$]\(MAKE\) \1[\$]\(AM_MAKEFLAGS\)/recursive-MAKE/g;
+ return (index('$(MAKE)', $_) >= 0 ? 1 : 0);
+ },
+ description => 'bad recusrsive make call',
+ rewrite_tips => 'use `$(MAKE) $(AM_MAKEFLAGS)` for make recursion',
+ must_skip => qr/^\s*#/,
+ files => address@hidden, @{$files{automake_in}},],
+ # see the comments in that file for why we can whitelist these
+ whitelist => [ qw{
+ lib/am/multilib.am:29
+ lib/am/multilib.am:31
+ lib/am/multilib.am:37
+ lib/am/multilib.am:39
+ lib/am/multilib.am:41
+ lib/am/multilib.am:43
+ } ],
+};
+
+# Try to make sure all @...@ substitutions in automake and aclocal scripts
+# are covered by our substitution rule.
+$all_checks{perl_at_substs} = {
+ bad_match => qr/@\w+@/,
+ description => 'unresolved "@...@" substitution',
+ must_skip => qr/^\s*#/,
+ files => $files{out},
+};
+
+## Don't let AMDEP_TRUE substitution appear in automake.in.
+$all_checks{AMDEP_TRUE_in_automake_in} = {
+ bad_match => qr/address@hidden@/,
+ description => 'invalid AMDEP_TRUE usage',
+ rewrite_tips => 'do not put AMDEP_TRUE substitution in automake.in',
+ must_skip => qr/^\s*#/,
+ files => $files{automake_in},
+};
+
+# In perl code, we never want to use "undef", only "delete", but for $/.
+$all_checks{no_undef_var_in_perl} = {
+ bad_match => qr{\bundef\s+(?![\$]/)},
+ description => 'unexpected use of `undef` in perl code',
+ rewrite_tips => 'use `delete` instead (`delete $x`, not `undef $x`)',
+ must_skip => qr/^\s*#/,
+ files => address@hidden, @{$files{pl_test}}, @{$files{pm}},],
+};
+
+# In perl code, we never want split (/ /,...), only split (' ', ...).
+$all_checks{perl_no_split_regex_space} = {
+ bad_match => qr{\bsplit\s*\(?/ /},
+ description => 'bad usage of built-in `split`',
+ rewrite_tips => 'use split(" ", ...) to split strings at whitespaces',
+ files => address@hidden, @{$files{pl_test}}, @{$files{pm}},],
+ whitelist => ['maintcheck.pl:262'],
+};
+
+# In perl, using @_ in a scalar context is most probably a
+# programming error.
+$all_checks{perl_at_uscore_in_scalar_context} = {
+ bad_match => qr/[\$].*[^) address@hidden/, # a bit tricky
+ description => 'using @_ in a scalar contex',
+ files => address@hidden, @{$files{pl_test}}, @{$files{pm}},],
+};
+
+# In perl, forbid using parens with `local' to ease counting.
+# FIXME: is this check still relevant, given that we allow only
+# FIXME: `$_' to be localized?
+$all_checks{perl_local_no_parens} = {
+ bad_match => qr/\blocal\s*\(/,
+ description => 'do not use `local` with parenthesis',
+ rewrite_tips => 'try to use use several `local` instead',
+ must_skip => qr/^\s*#/,
+ files => address@hidden, @{$files{pl_test}}, @{$files{pm}},],
+};
+
+# Allow only `local $_' in perl.
+$all_checks{perl_local} = {
+ bad_match => sub {
+ local $_ = shift;
+ s/(?:^|\s)local [\$]_\b//;
+ return (m/(?:^|\s)address@hidden&\*]/ ? 1 : 0);
+ },
+ description => '`local` should be used only for `$_`',
+ must_skip => qr/^\s*#/,
+ files => address@hidden, @{$files{pl_test}}, @{$files{pm}},],
+ whitelist => ['lib/Automake/FileUtils.pm:362'],
+};
+
+# Never use `sleep 1' to create files with different timestamps.
+# Use `$sleep' instead. Some filesystems (e.g., Windows') have only
+# a 2sec resolution.
+$all_checks{tests_plain_sleep} = {
+ bad_match => qr/\bsleep\s+[12345]\b/,
+ description => 'suspicious `sleep` command',
+ rewrite_tips => 'do not use `sleep x`, use `$sleep` instead',
+ files => $files{sh_test},
+};
+
+# Tests should only use e.g. <<'EOF' (not <<\EOF or <<"EOF") for quoted
+# here documents, for consistency and easier greppability.
+$all_checks{tests_here_document_no_backslash} = {
+ bad_match => qr/<<\s*["\\]/,
+ description => 'here-document quoted with backslash or double-quote',
+ rewrite_tips => 'use only unquoted or single-quoted here documents',
+ must_skip => qr/^\s*#/,
+ files => $files{sh_test},
+};
+
+# Tests should only use END and EOF for here documents, for greppability.
+$all_checks{tests_here_document_format} = {
+ bad_match => sub {
+ local $_ = shift;
+ return 0 if index($_, '<<') < 0;
+ return 0 if s/.*<<\s*(["']?)(?:EOF|END)\1\s*$//;
+ return 0 if s/.*<<\s*\\(?:EOF|END)\s*$//;
+ return 1;
+ },
+ description => 'bad here-document delimiter',
+ rewrite_tips => 'use here documents with "END" and "EOF" only',
+ files => $files{sh_test},
+};
+
+# Tests should (almost) never call exit directly, but use Exit.
+# This is so that the exit status is transported correctly across the 0 trap.
+# There are some exceptions, though, e.g. when we are sure that the trap has
+# not yet been installed.
+$all_checks{Exit_not_exit} = {
+ bad_match => qr/\bexit\b/,
+ description => 'unexpected use of bare `exit` builtin in test',
+ rewrite_tips => 'use the `Exit` function instead',
+ must_skip => sub {
+ my ($line, $file, undef) = @_;
+ $line =~ /^\s*#/ and return 1; # ignore comment lines
+ # in generated tests `NAME-p.test' ...
+ if ($file =~ m{^tests/[^/]+-p\.test$}) {
+ # ... ignore ...
+ return 1
+ # ... these lines.
+ if ($line eq '. ./defs || exit 99' or
+ $line =~ m{^test -n "[\$]srcdir" || exit 99( #.*)?$});
+ }
+ return 0; # else, line is not to be ignored
+ },
+ files => $files{sh_test},
+ skip_heredoc => 1,
+ whitelist => [ qw{
+ tests/defs.in
+ tests/distlinksbrk.test:71
+ tests/ext2.test:68
+ tests/gnits2.test:108
+ tests/gnits3.test:86
+ } ],
+};
+
+# Overriding a Makefile macro on the command line is not portable when
+# recursive targets are used. Better use an env var. SHELL is an
+# exception, POSIX says it can't come from the environment. V and DESTDIR
+# are exceptions, too, as package authors are urged not to initialize them
+# anywhere.
+$all_checks{tests_overriding_make_macros_on_cmdline} = {
+ bad_match => sub {
+ local $_ = shift;
+ return 0 unless /[\$]MAKE\s.*=/;
+ s/\b(?:SHELL|DESTDIR|V)=\S*//g;
+ return (m/=/ ? 1 : 0);
+ },
+ description => 'bad command-line override of a Makefile macro',
+ rewrite_tips => 'rewrite `$MAKE foo=bar` as `foo=bar $MAKE -e`',
+ files => $files{sh_test},
+};
+$all_checks{tests_overriding_make_SHELL_in_environment} = {
+ bad_match => qr/\bSHELL=.*[\$]MAKE\b/,
+ description => 'make ignores the SHELL environment variable',
+ rewrite_tips => 'use e.g. `$MAKE SHELL=$SHELL` instead',
+ files => $files{sh_test},
+};
+
+# Tests should never call any of automake, aclocal, autoconf, autoheader,
+# autoupdate, make, perl, egrep or fgrep directly.
+DEFINE_CHECKS_NO_PLAIN_COMMANDS: {
+ my @invalid_commands = qw(
+ perl fgrep egrep
+ autoconf autoupdate autoheader
+ automake aclocal
+ );
+ my %whitelists = (
+ automake => [qw{
+ tests/cond5.test:66
+ tests/install2.test:26
+ tests/lex3.test:30
+ tests/subobj3.test:27
+ }],
+ aclocal => [qw{
+ tests/confdeps.test:24
+ tests/confdeps.test:29
+ }],
+ autoconf => [qw{
+ tests/missing6.test:50
+ tests/reqd2.test:41
+ tests/txinfo29.test:50
+ }],
+ );
+ foreach my $cmd (@invalid_commands) {
+ my $subst_cmd = '$' . uc($cmd);
+ $all_checks{"tests_plain_$cmd"} = {
+ bad_match => qr/(?:^|\s)$cmd(?:\$|\s)/,
+ description => "invalid plain command `$cmd`",
+ rewrite_tips => "use `$subst_cmd` instead",
+ must_skip => qr/^\s*#/,
+ files => $files{sh_test},
+ whitelist => ($whitelists{$cmd} || []),
+ };
+ }
+}
+
+# Use AUTOMAKE_fails when appropriate
+$all_checks{tests_automake_fails} = {
+ bad_match => qr/[\$]AUTOMAKE\b.*&&.*\b[eE]xit\b/,
+ description => 'wrong way to catch expected automake failure',
+ rewrite_tips => 'use `AUTOMAKE_fails` and grep captured stderr',
+ must_skip => qr/^\s*#/,
+ files => $files{sh_test},
+};
+
+### END OF CHECKS' DEFINITIONS ###
+
+
+OPTION_PARSING: {
+ use Getopt::Long qw/GetOptions/;
+ my $checks = '';
+ GetOptions(
+ "use-whitelist!" => \$use_whitelist,
+ "enable-checks|enable-check=s" => sub { $checks .= ",$_[1]" },
+ ) or exit 2;
+ $checks =~ s/^,//;
+ # It seems nice that `--enable-check=foo-bar' is equivalent
+ # to `--enable-check=foo_bar'.
+ $checks =~ s/-/_/g;
+ my %checks = map { $_ => 1 } split(/,/, $checks);
+ if ($checks{'ALL'}) {
+ %enabled_checks = %all_checks;
+ } else {
+ foreach my $c (keys %checks) {
+ $enabled_checks{$c} = $all_checks{$c} or do {
+ print STDERR "Invalid check `$c'\n";
+ exit 2;
+ }
+ }
+ }
+ @ARGV and do { print STDERR "Too many arguments\n"; exit 2 };
+}
+
+# Normalize checks, also looking for errors in their definition.
+NORMALIZE_CHECKS:
+while (my ($name, $c) = each %enabled_checks) {
+ foreach my $k (qw/bad_match files/) {
+ defined $c->{$k} or die "$me: check $name: `$k' missing.\n";
+ }
+ # Convert list of files to a "key-only" dictionary, so that
+ # membership can be easily tested.
+ $c->{files} = { +map { $_ => 1 } @{$c->{files}} };
+ defined $c->{must_skip} or $c->{must_skip} = sub { return 0; };
+ defined $c->{skip_heredoc} or $c->{skip_heredoc} = 0;
+ $c->{bad_lines} = [];
+ foreach my $k (qw/bad_match must_skip/) {
+ my $reftyp = ref($c->{$k}) || ""; # be sure it's defined
+ if ($reftyp =~ /code/i) {
+ 1; # nothing to do
+ } elsif ($reftyp =~ /regexp/i) {
+ my $rx = $c->{$k}; # *required* for closure to work correctly
+ $c->{$k} = sub { return (shift =~ $rx ? 1 : 0) };
+ } elsif (not $reftyp) {
+ die "$me: check $name: `$k': not a reference.\n";
+ } else {
+ die "$me: check $name: `$k': bad reference type: $reftyp.\n";
+ }
+ }
+}
+
+my (@io_errors, @sc_errors);
+
+FILE_LOOP:
+foreach my $file (@{$files{all}}) {
+ # Decide what checks this file should be subjected to.
+ my %checks_for_this_file = ();
+ foreach my $k (keys %enabled_checks) {
+ $checks_for_this_file{$k} = $enabled_checks{$k}
+ if $enabled_checks{$k}->{files}->{$file};
+ }
+ # Skip the file if no check is applicable.
+ next FILE_LOOP unless %checks_for_this_file;
+ unless(open FILE, "<$file") {
+ push @io_errors, "$file: cannot open: $!";
+ next FILE_LOOP;
+ }
+ # In shell scripts, we must ignore here documents.
+ my $here_doc_delim = '';
+ my $with_here_docs = ($file =~ /\.test$/ ? 1 : 0);
+ LINE_LOOP:
+ while (<FILE>) {
+ chomp; # remove trailing newline
+ if ($with_here_docs) {
+ if ($here_doc_delim) {
+ $here_doc_delim = '' if $_ eq $here_doc_delim;
+ } else {
+ $here_doc_delim = $2 if /<<\s*('?)(\w+)\1\s*$/;
+ }
+ }
+ CHECKS_LOOP:
+ foreach my $c (values %checks_for_this_file) {
+ next CHECKS_LOOP if $c->{must_skip}->($_, $file, $.);
+ next CHECKS_LOOP if $c->{skip_heredoc} and $here_doc_delim;
+ if ($use_whitelist) {
+ my $whitelist = $c->{whitelist} || [];
+ if (grep /^\Q$file\E(:\s*$.)?\s*$/, @$whitelist) {
+ next CHECKS_LOOP;
+ }
+ }
+ if ($c->{bad_match}->($_, $file, $.)) {
+ push @{$c->{bad_lines}}, "$file:$.: $_";
+ }
+ }
+ }
+ close FILE or push @io_errors, "$file: cannot close: $!";
+}
+
+BUILD_FAILURE_DETAILS:
+while (my ($name, $c) = each %enabled_checks) {
+ if (@{$c->{bad_lines}}) {
+ my $msg = "";
+ if (defined $c->{description}) {
+ $msg .= "$name: In the lines below: $c->{description}\n";
+ } else {
+ $msg .= "$name: Bad lines below\n";
+ }
+ if (defined $c->{rewrite_tips}) {
+ (my $tips = $c->{rewrite_tips}) =~ s/^/$name: /gm;
+ $msg .= "$tips\n";
+ }
+ $msg .= " " . join("\n ", @{$c->{bad_lines}}) . "\n";
+ $msg .= "$name: Check failed, sorry.";
+ push @sc_errors, $msg;
+ }
+}
+
+REPORT: {
+ my $msg = "";
+ $msg .= "---\n" . join("\n---\n", @sc_errors) . "\n---\n"
+ if @sc_errors;
+ $msg .= join("\n", map {"I/O ERROR: $_"} @io_errors). "\n---\n"
+ if @io_errors;
+ die "$msg" . "$me: FAILED.\n" if $msg;
+}
+
+# vim: ft=perl et sw=4 ts=4
--
1.7.1
- [RFC] Refactoring maintainer checks (use perl, add whitelisting),
Stefano Lattarini <=