On Friday 23 September 2011, Stefano Lattarini wrote:
> Hi Nick, thanks for the report.
>
> On Thursday 22 September 2011, Nick Bowler wrote:
> > Hello,
> >
> > In the Automake manual, ยง14.4 "Checking the Distribution"
> >
> >
> > https://www.gnu.org/software/automake/manual/automake.html#Checking-the-Distribution
> >
> > claims that make distcheck will verify that make uninstall works
> > correctly.
> >
> > [SNIP]
> >
> > However, this does not seem to always be the case.
> >
> > [SNIP]
> >
> I've manged to reproduce this, and I confirm it's a bug. I'll take a
> look soonish.
>
Apparently, it was a simple bug. Attached is the patch I'll push to maint
in a couple of days to fix it. As usual, reviews welcome.
Thanks,
Stefano
From 26b2e20c1369e11f696cf6c92888603a36a7cecc Mon Sep 17 00:00:00 2001
Message-Id: <26b2e20c1369e11f696cf6c92888603a36a7cecc.1316807120.git.stefano.lattar...@gmail.com>
From: Stefano Lattarini <stefano.lattar...@gmail.com>
Date: Fri, 23 Sep 2011 16:06:59 +0200
Subject: [PATCH] distuninstallcheck: fail also when only one file is left installed
This change fixes automake bug#9579.
* lib/am/distdir.am (distuninstallcheck): Be stricter in ignoring
a potential `dir' file created by install-info and left installed.
Also, be more careful about "this can't happen" kind of errors.
(am__distuninstallcheck_listfiles): New internal helper macro.
* tests/distcheck-pr9579.test: New test.
* tests/Makefile.am (TESTS): Add it.
* NEWS, THANKS: Update.
Report by Nick Bowler.
---
ChangeLog | 13 +++++++++
Makefile.in | 11 +++++--
NEWS | 3 ++
THANKS | 1 +
lib/am/distdir.am | 21 +++++++++++---
tests/Makefile.am | 1 +
tests/Makefile.in | 1 +
tests/distcheck-pr9579.test | 64 +++++++++++++++++++++++++++++++++++++++++++
8 files changed, 107 insertions(+), 8 deletions(-)
create mode 100755 tests/distcheck-pr9579.test
diff --git a/ChangeLog b/ChangeLog
index 47aee92..2ba979c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2011-09-23 Stefano Lattarini <stefano.lattar...@gmail.com>
+
+ distuninstallcheck: fail also when only one file is left installed
+ This change fixes automake bug#9579.
+ * lib/am/distdir.am (distuninstallcheck): Be stricter in ignoring
+ a potential `dir' file created by install-info and left installed.
+ Also, be more careful about "this can't happen" kind of errors.
+ (am__distuninstallcheck_listfiles): New internal helper macro.
+ * tests/distcheck-pr9579.test: New test.
+ * tests/Makefile.am (TESTS): Add it.
+ * NEWS, THANKS: Update.
+ Report by Nick Bowler.
+
2011-09-22 Stefano Lattarini <stefano.lattar...@gmail.com>
tests: fix tests on aclocal search path precedences
diff --git a/Makefile.in b/Makefile.in
index 35a9cbd..f32c50c 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -145,6 +145,8 @@ am__relativize = \
DIST_ARCHIVES = $(distdir).tar.gz $(distdir).tar.bz2
GZIP_ENV = --best
distuninstallcheck_listfiles = find . -type f -print
+am__distuninstallcheck_listfiles = $(distuninstallcheck_listfiles) \
+ | grep -v '/share/info/dir$$'
distcleancheck_listfiles = find . -type f -print
ACLOCAL = @ACLOCAL@
AMTAR = @AMTAR@
@@ -753,13 +755,16 @@ distcheck: dist
list='$(DIST_ARCHIVES)'; for i in $$list; do echo $$i; done) | \
sed -e 1h -e 1s/./=/g -e 1p -e 1x -e '$$p' -e '$$x'
distuninstallcheck:
- @$(am__cd) '$(distuninstallcheck_dir)' \
- && test `$(distuninstallcheck_listfiles) | wc -l` -le 1 \
+ @$(am__cd) '$(distuninstallcheck_dir)' || { \
+ echo 'ERROR: cannot chdir into $(distuninstallcheck_dir)' >&2; \
+ exit 1; \
+ }; \
+ test `$(am__distuninstallcheck_listfiles) | wc -l` -eq 0 \
|| { echo "ERROR: files left after uninstall:" ; \
if test -n "$(DESTDIR)"; then \
echo " (check DESTDIR support)"; \
fi ; \
- $(distuninstallcheck_listfiles) ; \
+ $(am__distuninstallcheck_listfiles) ; \
exit 1; } >&2
distcleancheck: distclean
@if test '$(srcdir)' = . ; then \
diff --git a/NEWS b/NEWS
index b696977..df8fb5a 100644
--- a/NEWS
+++ b/NEWS
@@ -61,6 +61,9 @@ Bugs fixed in 1.11.0a:
* Long standing bugs:
+ - "make distcheck" now correctly complains also when "make uninstall"
+ leaves one and only one file installed in $(prefix).
+
- Automake now warns about more primary/directory invalid combinations,
such as "doc_LIBRARIES" or "pkglib_PROGRAMS".
diff --git a/THANKS b/THANKS
index f83e1fc..b840088 100644
--- a/THANKS
+++ b/THANKS
@@ -244,6 +244,7 @@ Motoyuki Kasahara m-kas...@sra.co.jp
Nathanael Nerode nero...@twcny.rr.com
Nelson H. F. Beebe be...@math.utah.edu
Nicholas Wourms nwou...@netscape.net
+Nick Bowler nbow...@elliptictech.com
Nicolas Joly nj...@pasteur.fr
Nicolas Thiery nthi...@icare.mines.edu
NightStrike nightstr...@gmail.com
diff --git a/lib/am/distdir.am b/lib/am/distdir.am
index c2dd7c5..d551267 100644
--- a/lib/am/distdir.am
+++ b/lib/am/distdir.am
@@ -516,16 +516,27 @@ distcheck: dist
## from distcheck, so that they can be overridden by the user.
.PHONY: distuninstallcheck
distuninstallcheck_listfiles = find . -type f -print
+## The `dir' file (created by install-info) might still exist after
+## uninstall, so we must be prepared to account for it. The following
+## check assumes that the package author hasn't changed ${infodir} nor
+## ${datarootdir} in strange ways; if he has done so, then he should be
+## prepared to define a custom $(distuninstallcheck_listfiles) as well.
+## Also, this check is slighlty laxer than we'd like, but obtaining a
+## 100% precision would be too tricky to be really worth, so we declare
+## this good enough.
+am__distuninstallcheck_listfiles = $(distuninstallcheck_listfiles) \
+ | grep -v '/share/info/dir$$'
distuninstallcheck:
-## We use -le 1 because the `dir' file (created by install-info)
-## might still exist after uninstall.
- @$(am__cd) '$(distuninstallcheck_dir)' \
- && test `$(distuninstallcheck_listfiles) | wc -l` -le 1 \
+ @$(am__cd) '$(distuninstallcheck_dir)' || { \
+ echo 'ERROR: cannot chdir into $(distuninstallcheck_dir)' >&2; \
+ exit 1; \
+ }; \
+ test `$(am__distuninstallcheck_listfiles) | wc -l` -eq 0 \
|| { echo "ERROR: files left after uninstall:" ; \
if test -n "$(DESTDIR)"; then \
echo " (check DESTDIR support)"; \
fi ; \
- $(distuninstallcheck_listfiles) ; \
+ $(am__distuninstallcheck_listfiles) ; \
exit 1; } >&2
## Define distcleancheck_listfiles and distcleancheck separately
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 1d258c9..85f62f2 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -335,6 +335,7 @@ distcheck-hook.test \
distcheck-hook2.test \
distcheck-missing-m4.test \
distcheck-outdated-m4.test \
+distcheck-pr9579.test \
dmalloc.test \
doc-parsing-buglets-colneq-subst.test \
doc-parsing-buglets-tabs.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index 7e9bc20..4e482b6 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -613,6 +613,7 @@ distcheck-hook.test \
distcheck-hook2.test \
distcheck-missing-m4.test \
distcheck-outdated-m4.test \
+distcheck-pr9579.test \
dmalloc.test \
doc-parsing-buglets-colneq-subst.test \
doc-parsing-buglets-tabs.test \
diff --git a/tests/distcheck-pr9579.test b/tests/distcheck-pr9579.test
new file mode 100755
index 0000000..b443b68
--- /dev/null
+++ b/tests/distcheck-pr9579.test
@@ -0,0 +1,64 @@
+#! /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/>.
+
+# Check against automake bug#9579: distcheck does not always detect
+# incomplete uninstall as advertised.
+
+. ./defs || Exit 1
+
+set -e
+
+cat >> configure.in << 'END'
+AC_OUTPUT
+END
+
+# NOTE: the use of `dir' as the name of the data file installed by hand
+# is deliberate, and enhances coverage -- see definition and comments of
+# lib/am/distdir.am:$(am__distuninstallcheck_listfiles).
+
+cat > Makefile.am << 'END'
+dist_data_DATA = foo
+EXTRA_DIST = dir
+install-data-local:
+ $(MKDIR_P) '$(DESTDIR)$(datadir)'
+ cp '$(srcdir)/dir' '$(DESTDIR)$(datadir)/dir'
+END
+
+: > foo
+: > dir
+
+$ACLOCAL
+$AUTOMAKE
+$AUTOCONF
+
+./configure --prefix="`pwd`/inst"
+
+# Sanity checks.
+$MAKE install
+find inst -type f
+test -f inst/share/foo
+test -f inst/share/dir
+# We expect the uninstall target of our Makefile to be definitely broken.
+$MAKE uninstall
+test -f inst/share/dir
+
+$MAKE distcheck >output 2>&1 && { cat output; Exit 1; }
+cat output
+
+$FGREP 'ERROR: files left after uninstall:' output
+grep '/share/dir *$' output
+
+:
--
1.7.2.3
