GNU bug report logs - #9037
distcheck should check for missing m4 files too (was: Re: bug#9026: Supporting $ACLOCAL_PATH?)

Previous Next

To reply to this bug, email your comments to 9037 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Peter Johansson <trojkan <at> gmail.com>
Cc: 9026 <at> debbugs.gnu.org, ludo <at> gnu.org, bug-automake <at> gnu.org,
	Bruno Haible <bruno <at> clisp.org>
Subject: distcheck should check for missing m4 files too (was: Re: bug#9026:
	Supporting $ACLOCAL_PATH?)
Date: Sat, 9 Jul 2011 16:56:43 +0200

On Saturday 09 July 2011, Peter Johansson wrote:
> Hi Bruno,
> 
> On 7/8/11 5:24 PM, Bruno Haible wrote:
> > +If you are using GNU @code{automake} 1.10 or newer, it is even easier:
> > +Add the line
> > +
> > +@example
> > +ACLOCAL_AMFLAGS = --install -I m4
> > +@end example
> > +
> > +@noindent
> > +to your top level @file{Makefile.am}, and run @samp{aclocal --install -I m4}.
> > +This will copy the needed files to the @file{m4/} subdirectory automatically,
> > +before updating @file{aclocal.m4}.
> > +
> I used to do this as it is an easy way to stay up to date with 3rd party 
> m4 files. I stopped doing this, however, after Ralf Wildenhues made me 
> aware there is a risk doing so. The risk is that aclocal will copy 3rd 
> party m4 files into m4 not only for you but also for your users if they 
> happen to run aclocal. Say, e.g., that a user want to build a somewhat 
> old version of your package from git; he bootstraps and as he has newer 
> versions of the m4 files available on his system aclocal copies them 
> into m4, which may cause problems as they are not necessarily compatible 
> with your configure.ac. To avoid this from happen, I've removed the 
> --install flag from my packages and calls aclocal --install -I m4 
> frequently instead.
>
Oh, good point, I hadn't thought about the precise semantics of `--install'
when I wrote my answer.

> 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.
I'm not sure how easy or difficult that would be, though, and I can't
look into it right now; so I'm opening a new bug report to make sure we
won't forget about the issue.

Thanks,
  Stefano

Message #8 received at 9037 <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: ludo <at> gnu.org
Cc: Peter Johansson <trojkan <at> gmail.com>, bruno <at> clisp.org, 9037 <at> debbugs.gnu.org
Subject: Re: bug#9037: distcheck should check for missing m4 files too
Date: Thu, 1 Sep 2011 15:04:34 +0200

[Message part 1 (text/plain, inline)]

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>

On Saturday 09 July 2011, Stefano Lattarini wrote:
> On Saturday 09 July 2011, Peter Johansson wrote:
> > Hi Bruno,
> > 
> > On 7/8/11 5:24 PM, Bruno Haible wrote:
> > > +If you are using GNU @code{automake} 1.10 or newer, it is even easier:
> > > +Add the line
> > > +
> > > +@example
> > > +ACLOCAL_AMFLAGS = --install -I m4
> > > +@end example
> > > +
> > > +@noindent
> > > +to your top level @file{Makefile.am}, and run @samp{aclocal --install -I m4}.
> > > +This will copy the needed files to the @file{m4/} subdirectory automatically,
> > > +before updating @file{aclocal.m4}.
> > > +
> > I used to do this as it is an easy way to stay up to date with 3rd party 
> > m4 files. I stopped doing this, however, after Ralf Wildenhues made me 
> > aware there is a risk doing so. The risk is that aclocal will copy 3rd 
> > party m4 files into m4 not only for you but also for your users if they 
> > happen to run aclocal. Say, e.g., that a user want to build a somewhat 
> > old version of your package from git; he bootstraps and as he has newer 
> > versions of the m4 files available on his system aclocal copies them 
> > into m4, which may cause problems as they are not necessarily compatible 
> > with your configure.ac. To avoid this from happen, I've removed the 
> > --install flag from my packages and calls aclocal --install -I m4 
> > frequently instead.
> >
> Oh, good point, I hadn't thought about the precise semantics of `--install'
> when I wrote my answer.
> 
> > 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

[Message part 2 (text/html, inline)]

[distcheck-m4.test (application/x-shellscript, inline)]

Message #11 received at 9037 <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: ludo <at> gnu.org
Cc: Peter Johansson <trojkan <at> gmail.com>, bruno <at> clisp.org, 9037 <at> debbugs.gnu.org
Subject: Re: bug#9037: distcheck should check for missing m4 files too
Date: Fri, 9 Sep 2011 11:41:53 +0200

[Message part 1 (text/plain, inline)]

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

[Message part 2 (text/html, inline)]

[0001-coverage-distcheck-hook-to-catch-missing-outdated-.m.patch (text/x-patch, inline)]

From 2d5a0d87ddeaaab474e4fc1d9816feb275711384 Mon Sep 17 00:00:00 2001
Message-Id: <2d5a0d87ddeaaab474e4fc1d9816feb275711384.1315560967.git.stefano.lattarini <at> gmail.com>
From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
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  <stefano.lattarini <at> gmail.com>
+
+	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  <stefano.lattarini <at> gmail.com>
 
 	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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.