Subject: Re: bug#9037: distcheck should check for missing m4 files too

From: Stefano Lattarini <[email-masked]>
Date: Fri, 9 Sep 2011 11:41:53 +0200

On Thursday 01 September 2011, Stefano Lattarini wrote:
> Hello automakers, and sorry for the delay.
> 
> References:
>  <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=9037>
>  <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=9026>
> 
> [SNIP]
>
> > > There is, obviously, a risk doing this way, as mentioned above in this 
> > > thread, because if I'm not careful I may release a tarball with missing 
> > > m4 files. Would distcheck detect a missing m4 file, or would it be 
> > > possible to modify distcheck so it could warn about this case?
> > >
> > I think improving distcheck to catch such an error would be worthwhile.
> >
> OK, I've managed to come up with a test case that demonstrates how one can
> use a "distcheck-hook" to diagnose this kind of errors.  I'm not sure if
> this should integrated into the automake-generated distcheck recipe proper
> (maybe only when a new automake option, say "check-m4-distribution", is
> used?), or if we should report Peter's description of the potential
> problems with `--install' in the manual, and add our distcheck-hook there
> in a new example (to be kept synced with the new testcase).
> 
> Opinions?
> 
> Regards,
>   Stefano
>
I've recooked the patch to take advantage of the recent improvements
in maint w.r.t. `acdir' overriding (see commit `v1.11-441-g30f99cb'
"aclocal: more granularity in acdir overriding").  Since I was at it,
I've increased coverage a bit to ensure that the proposed idiom also
catch outdated .m4 files, not only missing ones.

Attached is the patch that I've pushed (to maint).  Now we should only
decide whether to document the idiom, or integrate it into distcheck
proper.

Regards,
  Stefano
From 2d5a0d87ddeaaab474e4fc1d9816feb275711384 Mon Sep 17 00:00:00 2001
Message-Id: <2d5a0d87d[email-masked]>
From: Stefano Lattarini <[email-masked]>
Date: Thu, 1 Sep 2011 17:42:41 +0200
Subject: [PATCH] coverage: distcheck-hook to catch missing/outdated *.m4 files

Related to automake bug#9037.

* tests/distcheck-missing-m4.test: New test.
* tests/distcheck-outdated-m4.test: Likewise.
* tests/distcheck-hook-m4.am: New data file, used by the new
tests.
* tests/Makefile.am (distcheck-m4-missing.log,
distcheck-m4-outdated.log): Depend on it.
(EXTRA_DIST): Distribute it.
(TESTS): Add the new tests.
---
 ChangeLog                        |   13 +++++
 tests/Makefile.am                |    5 ++
 tests/Makefile.in                |    7 ++-
 tests/distcheck-hook-m4.am       |   30 +++++++++++
 tests/distcheck-missing-m4.test  |  102 ++++++++++++++++++++++++++++++++++++++
 tests/distcheck-outdated-m4.test |   88 ++++++++++++++++++++++++++++++++
 6 files changed, 244 insertions(+), 1 deletions(-)
 create mode 100644 tests/distcheck-hook-m4.am
 create mode 100755 tests/distcheck-missing-m4.test
 create mode 100755 tests/distcheck-outdated-m4.test

diff --git a/ChangeLog b/ChangeLog
index c25b41e..696150b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2011-09-09  Stefano Lattarini  <[email-masked]>
+
+	coverage: distcheck-hook to catch missing/outdated *.m4 files
+	Related to automake bug#9037.
+	* tests/distcheck-missing-m4.test: New test.
+	* tests/distcheck-outdated-m4.test: Likewise.
+	* tests/distcheck-hook-m4.am: New data file, used by the new
+	tests.
+	* tests/Makefile.am (distcheck-m4-missing.log,
+	distcheck-m4-outdated.log): Depend on it.
+	(EXTRA_DIST): Distribute it.
+	(TESTS): Add the new tests.
+
 2011-09-06  Stefano Lattarini  <[email-masked]>
 
 	aclocal: more granularity in acdir overriding
diff --git a/tests/Makefile.am b/tests/Makefile.am
index f01a6dd..35b96fe 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -327,6 +327,8 @@ distcheck-configure-flags-am.test \
 distcheck-configure-flags-subpkg.test \
 distcheck-hook.test \
 distcheck-hook2.test \
+distcheck-missing-m4.test \
+distcheck-outdated-m4.test \
 dmalloc.test \
 doc-parsing-buglets-colneq-subst.test \
 doc-parsing-buglets-tabs.test \
@@ -880,6 +882,9 @@ $(parallel_tests)
 
 EXTRA_DIST = ChangeLog-old gen-parallel-tests $(TESTS)
 
+distcheck-missing-m4.log distcheck-outdated-m4.log: distcheck-hook-m4.am
+EXTRA_DIST += distcheck-hook-m4.am
+
 # Each test case depends on defs, aclocal, and automake.
 check_SCRIPTS = defs aclocal-$(APIVERSION) automake-$(APIVERSION)
 
diff --git a/tests/Makefile.in b/tests/Makefile.in
index eed1ef9..2227ee8 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -605,6 +605,8 @@ distcheck-configure-flags-am.test \
 distcheck-configure-flags-subpkg.test \
 distcheck-hook.test \
 distcheck-hook2.test \
+distcheck-missing-m4.test \
+distcheck-outdated-m4.test \
 dmalloc.test \
 doc-parsing-buglets-colneq-subst.test \
 doc-parsing-buglets-tabs.test \
@@ -1156,7 +1158,8 @@ yflags.test \
 yflags2.test \
 $(parallel_tests)
 
-EXTRA_DIST = ChangeLog-old gen-parallel-tests $(TESTS)
+EXTRA_DIST = ChangeLog-old gen-parallel-tests $(TESTS) \
+	distcheck-hook-m4.am
 
 # Each test case depends on defs, aclocal, and automake.
 check_SCRIPTS = defs aclocal-$(APIVERSION) automake-$(APIVERSION)
@@ -1494,6 +1497,8 @@ $(parallel_tests): $(parallel_tests:-p.test=.test) Makefile.am
 	  < $(srcdir)/$$input >$@
 	chmod a+rx $@
 
+distcheck-missing-m4.log distcheck-outdated-m4.log: distcheck-hook-m4.am
+
 clean-local: clean-local-check
 .PHONY: clean-local-check
 clean-local-check:
diff --git a/tests/distcheck-hook-m4.am b/tests/distcheck-hook-m4.am
new file mode 100644
index 0000000..88681e9
--- /dev/null
+++ b/tests/distcheck-hook-m4.am
@@ -0,0 +1,30 @@
+## Data files for some tests.  Not used in the automake build system.
+##
+## Copyright (C) 2011 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/>.
+
+distcheck-hook:
+	@fatal () { echo "$@: $$*" >&2; exit 1; }; \
+	$(am__cd) $(distdir) && chmod u+w . && mkdir _m4 \
+	  || fatal "cannot setup distdir"; \
+	$(ACLOCAL) -I _m4 $(ACLOCAL_AMFLAGS) --install --output=_am.m4 \
+	  || fatal "cannot regenerate aclocal.m4"; \
+	lst=`ls _m4 | tr '\012\015' '  '`; \
+	if test -n "$$lst"; then \
+	  fatal "required m4 files not distributed or outdated: $$lst"; \
+	fi; \
+	$(AUTOCONF) -o /dev/null || fatal "can't remake configure"; \
+	rm -rf _m4 _am.m4 autom4te.cache && chmod a-w . \
+	  || fatal "cannot reset distdir"
diff --git a/tests/distcheck-missing-m4.test b/tests/distcheck-missing-m4.test
new file mode 100755
index 0000000..f4e0eda
--- /dev/null
+++ b/tests/distcheck-missing-m4.test
@@ -0,0 +1,102 @@
+#! /bin/sh
+# Copyright (C) 2011 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/>.
+
+# Test that we can define a distcheck-hook to diagnose m4 files
+# missing from the dist tarball (interaction with `--install').
+# See automake bug#9037.
+
+. ./defs || Exit 1
+
+set -e
+
+cwd=`pwd` || fatal_ "cannot get current working directory"
+
+cp "$testsrcdir"/distcheck-hook-m4.am . \
+  || fatal_ "cannot fetch makefile fragment \`distcheck-hook-m4.am'"
+
+cat > Makefile.am << 'END'
+## The lack of `--install' here is meant.
+ACLOCAL_AMFLAGS = -I m4
+include $(srcdir)/distcheck-hook-m4.am
+END
+
+cat >> configure.in << 'END'
+dnl We *deliberately* don't use m4_pattern_forbid here.
+AC_OUTPUT
+MY_FOO
+MY_BAR
+MY_BAZ
+END
+
+mkdir m4 acdir acdir1 acdir2
+
+cat > acdir/dirlist << END
+$cwd/acdir1
+$cwd/acdir2
+END
+
+echo 'AC_DEFUN([MY_FOO], [:])' > m4/foo.m4
+echo 'AC_DEFUN([MY_BAR], [:])' > acdir1/bar.m4
+echo 'AC_DEFUN([MY_BAZ], [:])' > acdir1/baz.m4
+echo 'AC_DEFUN([MY_QUX], [:])' > acdir2/qux.m4
+
+ACLOCAL="$ACLOCAL --system-acdir=$cwd/acdir"; export ACLOCAL
+
+# We don't use `--install' here.  Our distcheck-hook should catch this.
+$ACLOCAL -I m4
+$AUTOCONF
+$EGREP 'MY_(FOO|BAR|BAZ)' configure && Exit 1 # Sanity check.
+$AUTOMAKE
+
+./configure
+
+$MAKE distcheck >output 2>&1 && { cat output; Exit 1; }
+cat output
+$EGREP "required m4 file.*not distributed.* bar.m4( |$)" output
+$EGREP "required m4 file.*not distributed.* baz.m4( |$)" output
+# Check that we don't fail for spurious errors.
+$EGREP -i 'mkdir:|autom4te.*\.cache|:.*(permission|denied)' output && Exit 1
+
+# Now we use `--install', and "make distcheck" should pass.
+$ACLOCAL -I m4 --install
+test -f m4/bar.m4 # Sanity check.
+test -f m4/baz.m4 # Likewise.
+using_gmake || $MAKE Makefile
+$MAKE distcheck
+
+# We start to use a new "third-party" macro from a new .m4 file, but forget
+# to re-run "aclocal --install" by hand, relying on automatic remake rules.
+# Our distcheck-hook should catch this too.
+echo MY_QUX >> configure.in
+
+$MAKE
+$EGREP 'MY_(FOO|BAR|BAZ|QUX)' configure && Exit 1 # Sanity check.
+
+$MAKE distcheck >output 2>&1 && { cat output; Exit 1; }
+cat output
+$EGREP "required m4 file.*not distributed.* qux.m4( |$)" output
+# Check that we don't fail for spurious errors.
+$EGREP -i 'mkdir:|autom4te.*\.cache|permission|denied' output && Exit 1
+# Check that we don't complain for files that should have been found.
+grep " ba[rz].m4" output && Exit 1
+
+# Now we again use `--install', and "make distcheck" should pass.
+$ACLOCAL -I m4 --install
+test -f m4/qux.m4 # Sanity check.
+using_gmake || $MAKE Makefile
+$MAKE distcheck
+
+:
diff --git a/tests/distcheck-outdated-m4.test b/tests/distcheck-outdated-m4.test
new file mode 100755
index 0000000..74f2922
--- /dev/null
+++ b/tests/distcheck-outdated-m4.test
@@ -0,0 +1,88 @@
+#! /bin/sh
+# Copyright (C) 2011 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/>.
+
+# Test that we can define a distcheck-hook to diagnose outdated m4
+# files in a dist tarball (interaction with `--install').
+# See automake bug#9037.
+
+. ./defs || Exit 1
+
+set -e
+
+cwd=`pwd` || fatal_ "cannot get current working directory"
+
+cp "$testsrcdir"/distcheck-hook-m4.am . \
+  || fatal_ "cannot fetch makefile fragment \`distcheck-hook-m4.am'"
+
+cat > Makefile.am << 'END'
+## The lack of `--install' here is meant.
+ACLOCAL_AMFLAGS = -I m4
+include $(srcdir)/distcheck-hook-m4.am
+END
+
+cat >> configure.in << 'END'
+AC_OUTPUT
+MY_FOO
+MY_BAR
+MY_BAZ
+END
+
+mkdir m4 acdir
+echo 'AC_DEFUN([MY_FOO], [:])' > m4/foo.m4
+echo 'AC_DEFUN([MY_BAR], [:])' > acdir/bar.m4
+cat > acdir/baz.m4 << 'END'
+# serial 1
+AC_DEFUN([MY_BAZ], [:])
+END
+
+ACLOCAL="$ACLOCAL --system-acdir=$cwd/acdir"; export ACLOCAL
+
+# We don't use `--install' here.  Our distcheck-hook should catch this.
+$ACLOCAL -I m4 --install
+$AUTOCONF
+$AUTOMAKE
+
+./configure
+$MAKE distcheck # Sanity check.
+
+# We start to use a new "third-party" macro in a new version
+# of a pre-existing third-party m4 file, but forget to re-run
+# "aclocal --install" by hand, relying on automatic remake
+# rules.  Our distcheck-hook should catch this too.
+echo MY_ZARDOZ >> configure.in
+
+cat > acdir/baz.m4 << 'END'
+# serial 2
+AC_DEFUN([MY_BAZ], [:])
+AC_DEFUN([MY_ZARDOZ], [:])
+END
+
+$MAKE # Rebuild configure and makefiles.
+$MAKE distcheck >output 2>&1 && { cat output; Exit 1; }
+cat output
+
+$EGREP "required m4 file.* outdated.* baz.m4( |$)" output
+# Check that we don't fail for spurious errors.
+$EGREP -i 'mkdir:|autom4te.*\.cache|permission|denied' output && Exit 1
+# Check that we don't complain for files that should have been found.
+$EGREP " (foo|bar).m4" output && Exit 1
+
+# Now we again use `--install', and "make distcheck" should pass.
+$ACLOCAL -I m4 --install
+using_gmake || $MAKE Makefile
+$MAKE distcheck
+
+:
-- 
1.7.2.3