GNU bug report logs - #9579
distcheck does not detect incomplete uninstall as advertised

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 9579 in the body.
You can then email your comments to 9579 AT debbugs.gnu.org in the normal way.

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

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

From: Nick Bowler <nbowler <at> elliptictech.com>
To: bug-automake <at> gnu.org
Subject: distcheck does not detect incomplete uninstall as advertised
Date: Thu, 22 Sep 2011 16:48:46 -0400

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.  The relevant text is quoted here:

> distcheck also checks that the uninstall rule works properly, both for
> ordinary and DESTDIR builds. It does this by invoking ‘make uninstall’,
> and then it checks the install tree to see if any files are left over.
> This check will make sure that you correctly coded your
> uninstall-related rules.

However, this does not seem to be the case.  I realized that I had
completely forgotten to write an uninstall-local rule to go with my
install-data-local rule, yet distcheck remained completely silent on
the matter.

I'm using autoconf-2.68 and automake-1.11.1.  The issue also occurs with
latest automake git at the time of writing.  Here's an example:

% cat >Makefile.am <<'EOF'
dist_data_DATA = foo
EXTRA_DIST = bar

install-data-local:
	$(MKDIR_P) '$(DESTDIR)$(datadir)'
	cp '$(srcdir)/bar' '$(DESTDIR)$(datadir)/bar'
EOF

% cat >configure.ac <<'EOF'
AC_INIT([test], [1.0])

AM_INIT_AUTOMAKE([-Wall -Werror foreign])

AC_CONFIG_FILES([Makefile])
AC_OUTPUT
EOF

% cat >foo <<'EOF'
foo
EOF

% cat >bar <<'EOF'
bar
EOF

A quick test reveals that the uninstall target of my Makefile is
definitely broken:

  % ./configure --prefix=$PWD/inst
  % make install
  % find inst -type f
  inst/share/foo
  inst/share/bar
  % make uninstall
  % find inst -type f
  inst/share/bar

As expected, "bar" is erroneously left in the install tree.  However,
when we go to run make distcheck:

  % make distcheck
  [...]
  ==========================================
  test-1.0 archives ready for distribution: 
  test-1.0.tar.gz
  ==========================================

Based on the quoted paragraph in the manual, I had expected distcheck to
fail.

Cheers,
-- 
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)

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

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: bug-automake <at> gnu.org
Cc: 9579 <at> debbugs.gnu.org, Nick Bowler <nbowler <at> elliptictech.com>
Subject: Re: bug#9579: distcheck does not detect incomplete uninstall as
	advertised
Date: Fri, 23 Sep 2011 16:02:11 +0200

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.

Thanks,
  Stefano

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

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: bug-automake <at> gnu.org
Cc: 9579 <at> debbugs.gnu.org, Nick Bowler <nbowler <at> elliptictech.com>,
	automake-patches <at> gnu.org
Subject: Re: bug#9579: distcheck does not detect incomplete uninstall as
	advertised
Date: Fri, 23 Sep 2011 21:46:57 +0200

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

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

[0001-distuninstallcheck-fail-also-when-only-one-file-is-l.patch (text/x-patch, inline)]

From 26b2e20c1369e11f696cf6c92888603a36a7cecc Mon Sep 17 00:00:00 2001
Message-Id: <26b2e20c1369e11f696cf6c92888603a36a7cecc.1316807120.git.stefano.lattarini <at> gmail.com>
From: Stefano Lattarini <stefano.lattarini <at> 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.lattarini <at> 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.lattarini <at> 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-kasahr <at> sra.co.jp
 Nathanael Nerode	neroden <at> twcny.rr.com
 Nelson H. F. Beebe	beebe <at> math.utah.edu
 Nicholas Wourms		nwourms <at> netscape.net
+Nick Bowler		nbowler <at> elliptictech.com
 Nicolas Joly		njoly <at> pasteur.fr
 Nicolas Thiery		nthiery <at> Icare.mines.edu
 NightStrike		nightstrike <at> 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

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

From: Eric Blake <eblake <at> redhat.com>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: 9579 <at> debbugs.gnu.org, nbowler <at> elliptictech.com, automake-patches <at> gnu.org
Subject: Re: bug#9579: distcheck does not detect incomplete uninstall as
	advertised
Date: Fri, 23 Sep 2011 14:18:13 -0600

On 09/23/2011 01:46 PM, Stefano Lattarini wrote:
> 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.
> 
> +++ 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

s/slighlty/slightly/

s/ a$//

> +## 100% precision would be too tricky to be really worth, so we declare

-- 
Eric Blake   eblake <at> redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

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

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Eric Blake <eblake <at> redhat.com>
Cc: 9579 <at> debbugs.gnu.org, nbowler <at> elliptictech.com, automake-patches <at> gnu.org
Subject: Re: bug#9579: distcheck does not detect incomplete uninstall as
	advertised
Date: Sat, 24 Sep 2011 15:36:23 +0200

Hi Eric, thanks for the quick review.

On Friday 23 September 2011, Eric Blake wrote:
> On 09/23/2011 01:46 PM, Stefano Lattarini wrote:
> > 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.
> > 
> > +++ 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
> 
> s/slighlty/slightly/
> 
> s/ a$//
> 
Consider these fixed.  I've also bumped the copyright years in
`lib/am/distdir.am' (I had forgotten to do so in the previous version
of the patch).

I will push by tomorrow if there is no objection by then.

Thanks,
  Stefano

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

From: Nick Bowler <nbowler <at> elliptictech.com>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: 9579 <at> debbugs.gnu.org, bug-automake <at> gnu.org, automake-patches <at> gnu.org
Subject: Re: bug#9579: distcheck does not detect incomplete uninstall as
	advertised
Date: Mon, 26 Sep 2011 15:06:47 -0400

Hi Stefano,

On 2011-09-23 21:46 +0200, Stefano Lattarini wrote:
> 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.lattarini <at> gmail.com>
> From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
> Date: Fri, 23 Sep 2011 16:06:59 +0200
> Subject: [PATCH] distuninstallcheck: fail also when only one file is left installed

I've tested this on my project, and now distcheck fails when I revert my
uninstall-local rule.  It's perhaps a bit unfortunate that

  make infodir='${prefix}/somewhere_else' distcheck

no longer works, but I agree that it seems non-trivial to make that
work properly.

Thanks,
-- 
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)

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

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Nick Bowler <nbowler <at> elliptictech.com>
Cc: 9579 <at> debbugs.gnu.org, bug-automake <at> gnu.org, automake-patches <at> gnu.org
Subject: Re: bug#9579: distcheck does not detect incomplete uninstall as
	advertised
Date: Tue, 27 Sep 2011 10:28:03 +0200

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

On Monday 26 September 2011, Nick Bowler wrote:
> Hi Stefano,
> 
> On 2011-09-23 21:46 +0200, Stefano Lattarini wrote:
> > 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: Stefano Lattarini <stefano.lattarini <at> gmail.com>
> > Date: Fri, 23 Sep 2011 16:06:59 +0200
> > Subject: [PATCH] distuninstallcheck: fail also when only one file is left installed
> 
> I've tested this on my project, and now distcheck fails when I revert my
> uninstall-local rule.  It's perhaps a bit unfortunate that
> 
>   make infodir='${prefix}/somewhere_else' distcheck
> 
> no longer works, but I agree that it seems non-trivial to make that
> work properly.
>
Well, I don't agree anymore on this ;-) -- by complicating my patch just
a little bit, we can apparently cater for this situation as well.  See the
attached squash-in (I've also attached the updated patch, for reference).

So I'll give a couple of days for further comments, than I'll push this
improved patch instead of the previous one.

Thanks,
  Stefano

[0001-distuninstallcheck-fail-also-when-only-one-file-is-l.patch (text/x-patch, inline)]

From 71bae85a00a5de739267876d6351ddb664cb6658 Mon Sep 17 00:00:00 2001
Message-Id: <71bae85a00a5de739267876d6351ddb664cb6658.1317111869.git.stefano.lattarini <at> gmail.com>
From: Stefano Lattarini <stefano.lattarini <at> 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/distcheck-override-infodir.test: Likewise.
* tests/Makefile.am (TESTS): Add them.
* NEWS, THANKS: Update.

Report by Nick Bowler.
---
 ChangeLog                             |   14 +++++
 Makefile.in                           |   14 ++++-
 NEWS                                  |    3 +
 THANKS                                |    1 +
 lib/am/distdir.am                     |   22 ++++++--
 tests/Makefile.am                     |    2 +
 tests/Makefile.in                     |    2 +
 tests/distcheck-override-infodir.test |   63 +++++++++++++++++++++
 tests/distcheck-pr9579.test           |   98 +++++++++++++++++++++++++++++++++
 9 files changed, 212 insertions(+), 7 deletions(-)
 create mode 100755 tests/distcheck-override-infodir.test
 create mode 100755 tests/distcheck-pr9579.test

diff --git a/ChangeLog b/ChangeLog
index 47aee92..875245f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2011-09-26  Stefano Lattarini  <stefano.lattarini <at> 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/distcheck-override-infodir.test: Likewise.
+	* tests/Makefile.am (TESTS): Add them.
+	* NEWS, THANKS: Update.
+	Report by Nick Bowler.
+
 2011-09-22  Stefano Lattarini  <stefano.lattarini <at> gmail.com>
 
 	tests: fix tests on aclocal search path precedences
diff --git a/Makefile.in b/Makefile.in
index 35a9cbd..a08e567 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) \
+  | sed 's|^\./|${prefix}/|' | grep -v '${infodir}/dir$$'
 distcleancheck_listfiles = find . -type f -print
 ACLOCAL = @ACLOCAL@
 AMTAR = @AMTAR@
@@ -753,8 +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 \
+	@test -n '$(distuninstallcheck_dir)' || { \
+	  echo 'ERROR: trying to run $@ with and empty' \
+	       '$$(distuninstallcheck_dir)' >&2; \
+	  exit 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)"; \
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-kasahr <at> sra.co.jp
 Nathanael Nerode	neroden <at> twcny.rr.com
 Nelson H. F. Beebe	beebe <at> math.utah.edu
 Nicholas Wourms		nwourms <at> netscape.net
+Nick Bowler		nbowler <at> elliptictech.com
 Nicolas Joly		njoly <at> pasteur.fr
 Nicolas Thiery		nthiery <at> Icare.mines.edu
 NightStrike		nightstrike <at> gmail.com
diff --git a/lib/am/distdir.am b/lib/am/distdir.am
index c2dd7c5..d3b34e1 100644
--- a/lib/am/distdir.am
+++ b/lib/am/distdir.am
@@ -1,6 +1,6 @@
 ## automake - create Makefile.in from Makefile.am
 ## Copyright (C) 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009,
-## 2010 Free Software Foundation, Inc.
+## 2010, 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
@@ -516,11 +516,23 @@ 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 is not 100% strict, but is definitely good enough, and even
+## accounts for overridden ${infodir}.
+am__distuninstallcheck_listfiles = $(distuninstallcheck_listfiles) \
+  | sed 's|^\./|${prefix}/|' | grep -v '${infodir}/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 \
+	@test -n '$(distuninstallcheck_dir)' || { \
+	  echo 'ERROR: trying to run $@ with and empty' \
+	       '$$(distuninstallcheck_dir)' >&2; \
+	  exit 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)"; \
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 1d258c9..5791588 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -335,6 +335,8 @@ distcheck-hook.test \
 distcheck-hook2.test \
 distcheck-missing-m4.test \
 distcheck-outdated-m4.test \
+distcheck-pr9579.test \
+distcheck-override-infodir.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..1e1cb79 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -613,6 +613,8 @@ distcheck-hook.test \
 distcheck-hook2.test \
 distcheck-missing-m4.test \
 distcheck-outdated-m4.test \
+distcheck-pr9579.test \
+distcheck-override-infodir.test \
 dmalloc.test \
 doc-parsing-buglets-colneq-subst.test \
 doc-parsing-buglets-tabs.test \
diff --git a/tests/distcheck-override-infodir.test b/tests/distcheck-override-infodir.test
new file mode 100755
index 0000000..8c54cfb
--- /dev/null
+++ b/tests/distcheck-override-infodir.test
@@ -0,0 +1,63 @@
+#! /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 that we can override ${infodir} while having distcheck still
+# working.  Relate to automake bug#9579.
+
+required='makeinfo tex texi2dvi'
+. ./defs || Exit 1
+
+set -e
+
+cat >> configure.in << 'END'
+AC_OUTPUT
+END
+
+cat > Makefile.am << 'END'
+infodir = ${prefix}/blah/blah/foobar
+info_TEXINFOS = main.texi
+## Sanity check.
+installcheck-local:
+	if test x$${infodir+set} != xset; then \
+	  ls -l "$(DESTDIR)/$(prefix)/blah/blah/foobar/" || exit 1; \
+	  test -f "$(DESTDIR)/$(prefix)/blah/blah/foobar/dir" || exit 1; \
+	else \
+	  ls -l "$(DESTDIR)/$$infodir/" || exit 1; \
+	  test -f "$(DESTDIR)/$$infodir/dir" || exit 1; \
+	fi
+END
+
+cat > main.texi << 'END'
+\input texinfo
+@setfilename main.info
+@settitle main
+@node Top
+Hello walls.
+@bye
+END
+
+$ACLOCAL
+$AUTOMAKE -a
+$AUTOCONF
+
+./configure
+$MAKE
+
+$MAKE distcheck
+infodir="`pwd`"/_info $MAKE -e distcheck
+test -f _info/dir || Exit 99 # Sanity check.
+
+:
diff --git a/tests/distcheck-pr9579.test b/tests/distcheck-pr9579.test
new file mode 100755
index 0000000..1d738ba
--- /dev/null
+++ b/tests/distcheck-pr9579.test
@@ -0,0 +1,98 @@
+#! /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
+rm -rf inst
+
+$MAKE distcheck >output 2>&1 && { cat output; Exit 1; }
+cat output
+
+$FGREP 'ERROR: files left after uninstall:' output
+grep '/share/dir *$' output
+
+# More trickier corner cases.
+
+cat > Makefile.am << 'END'
+EXTRA_DIST = dir
+install-data-local:
+install-data-local:
+	$(MKDIR_P) '$(DESTDIR)$(prefix)/mu/share/info'
+	cp '$(srcdir)/dir' '$(DESTDIR)$(prefix)/mu/share/info'
+	$(MKDIR_P) '$(DESTDIR)$(infodir)/more'
+	cp '$(srcdir)/dir' '$(DESTDIR)$(infodir)/more'
+END
+
+$AUTOMAKE
+./config.status Makefile
+
+# Sanity checks, again.
+$MAKE install
+find inst -type f
+test -f inst/mu/share/info/dir
+test -f inst/share/info/more/dir
+# We expect the uninstall target of our Makefile to be definitely broken.
+$MAKE uninstall
+test -f inst/mu/share/info/dir
+test -f inst/share/info/more/dir
+rm -rf inst
+
+$MAKE distcheck >output 2>&1 && { cat output; Exit 1; }
+cat output
+
+$FGREP 'ERROR: files left after uninstall:' output
+grep '/mu/share/info/dir *$' output
+grep '/share/info/more/dir *$' output
+
+:
-- 
1.7.2.3

[squash-in.diff (text/x-patch, inline)]

diff --git a/ChangeLog b/ChangeLog
index 2ba979c..875245f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,4 +1,4 @@
-2011-09-23  Stefano Lattarini  <stefano.lattarini <at> gmail.com>
+2011-09-26  Stefano Lattarini  <stefano.lattarini <at> gmail.com>
 
 	distuninstallcheck: fail also when only one file is left installed
 	This change fixes automake bug#9579.
@@ -7,7 +7,8 @@
 	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.
+	* tests/distcheck-override-infodir.test: Likewise.
+	* tests/Makefile.am (TESTS): Add them.
 	* NEWS, THANKS: Update.
 	Report by Nick Bowler.
 
diff --git a/Makefile.in b/Makefile.in
index f32c50c..a08e567 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -146,7 +146,7 @@ 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$$'
+  | sed 's|^\./|${prefix}/|' | grep -v '${infodir}/dir$$'
 distcleancheck_listfiles = find . -type f -print
 ACLOCAL = @ACLOCAL@
 AMTAR = @AMTAR@
@@ -755,7 +755,12 @@ 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 -n '$(distuninstallcheck_dir)' || { \
+	  echo 'ERROR: trying to run $@ with and empty' \
+	       '$$(distuninstallcheck_dir)' >&2; \
+	  exit 1; \
+	}; \
+	$(am__cd) '$(distuninstallcheck_dir)' || { \
 	  echo 'ERROR: cannot chdir into $(distuninstallcheck_dir)' >&2; \
 	  exit 1; \
 	}; \
@@ -764,7 +769,7 @@ distuninstallcheck:
 	        if test -n "$(DESTDIR)"; then \
 	          echo "  (check DESTDIR support)"; \
 	        fi ; \
-	        $(am__distuninstallcheck_listfiles) ; \
+	        $(distuninstallcheck_listfiles) ; \
 	        exit 1; } >&2
 distcleancheck: distclean
 	@if test '$(srcdir)' = . ; then \
diff --git a/lib/am/distdir.am b/lib/am/distdir.am
index ea9af94..d3b34e1 100644
--- a/lib/am/distdir.am
+++ b/lib/am/distdir.am
@@ -518,16 +518,17 @@ distcheck: dist
 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 slightly laxer than we'd like, but obtaining 100%
-## precision would be too tricky to be really worth, so we declare this
-## good enough.
+## check is not 100% strict, but is definitely good enough, and even
+## accounts for overridden ${infodir}.
 am__distuninstallcheck_listfiles = $(distuninstallcheck_listfiles) \
-                                     | grep -v '/share/info/dir$$'
+  | sed 's|^\./|${prefix}/|' | grep -v '${infodir}/dir$$'
 distuninstallcheck:
-	@$(am__cd) '$(distuninstallcheck_dir)' || { \
+	@test -n '$(distuninstallcheck_dir)' || { \
+	  echo 'ERROR: trying to run $@ with and empty' \
+	       '$$(distuninstallcheck_dir)' >&2; \
+	  exit 1; \
+	}; \
+	$(am__cd) '$(distuninstallcheck_dir)' || { \
 	  echo 'ERROR: cannot chdir into $(distuninstallcheck_dir)' >&2; \
 	  exit 1; \
 	}; \
@@ -536,7 +537,7 @@ distuninstallcheck:
 	        if test -n "$(DESTDIR)"; then \
 	          echo "  (check DESTDIR support)"; \
 	        fi ; \
-	        $(am__distuninstallcheck_listfiles) ; \
+	        $(distuninstallcheck_listfiles) ; \
 	        exit 1; } >&2
 
 ## Define distcleancheck_listfiles and distcleancheck separately
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 85f62f2..5791588 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -336,6 +336,7 @@ distcheck-hook2.test \
 distcheck-missing-m4.test \
 distcheck-outdated-m4.test \
 distcheck-pr9579.test \
+distcheck-override-infodir.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 4e482b6..1e1cb79 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -614,6 +614,7 @@ distcheck-hook2.test \
 distcheck-missing-m4.test \
 distcheck-outdated-m4.test \
 distcheck-pr9579.test \
+distcheck-override-infodir.test \
 dmalloc.test \
 doc-parsing-buglets-colneq-subst.test \
 doc-parsing-buglets-tabs.test \
diff --git a/tests/distcheck-override-infodir.test b/tests/distcheck-override-infodir.test
new file mode 100755
index 0000000..8c54cfb
--- /dev/null
+++ b/tests/distcheck-override-infodir.test
@@ -0,0 +1,63 @@
+#! /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 that we can override ${infodir} while having distcheck still
+# working.  Relate to automake bug#9579.
+
+required='makeinfo tex texi2dvi'
+. ./defs || Exit 1
+
+set -e
+
+cat >> configure.in << 'END'
+AC_OUTPUT
+END
+
+cat > Makefile.am << 'END'
+infodir = ${prefix}/blah/blah/foobar
+info_TEXINFOS = main.texi
+## Sanity check.
+installcheck-local:
+	if test x$${infodir+set} != xset; then \
+	  ls -l "$(DESTDIR)/$(prefix)/blah/blah/foobar/" || exit 1; \
+	  test -f "$(DESTDIR)/$(prefix)/blah/blah/foobar/dir" || exit 1; \
+	else \
+	  ls -l "$(DESTDIR)/$$infodir/" || exit 1; \
+	  test -f "$(DESTDIR)/$$infodir/dir" || exit 1; \
+	fi
+END
+
+cat > main.texi << 'END'
+\input texinfo
+@setfilename main.info
+@settitle main
+@node Top
+Hello walls.
+@bye
+END
+
+$ACLOCAL
+$AUTOMAKE -a
+$AUTOCONF
+
+./configure
+$MAKE
+
+$MAKE distcheck
+infodir="`pwd`"/_info $MAKE -e distcheck
+test -f _info/dir || Exit 99 # Sanity check.
+
+:
diff --git a/tests/distcheck-pr9579.test b/tests/distcheck-pr9579.test
index b443b68..1d738ba 100755
--- a/tests/distcheck-pr9579.test
+++ b/tests/distcheck-pr9579.test
@@ -54,6 +54,7 @@ test -f inst/share/dir
 # We expect the uninstall target of our Makefile to be definitely broken.
 $MAKE uninstall
 test -f inst/share/dir
+rm -rf inst
 
 $MAKE distcheck >output 2>&1 && { cat output; Exit 1; }
 cat output
@@ -61,4 +62,37 @@ cat output
 $FGREP 'ERROR: files left after uninstall:' output
 grep '/share/dir *$' output
 
+# More trickier corner cases.
+
+cat > Makefile.am << 'END'
+EXTRA_DIST = dir
+install-data-local:
+install-data-local:
+	$(MKDIR_P) '$(DESTDIR)$(prefix)/mu/share/info'
+	cp '$(srcdir)/dir' '$(DESTDIR)$(prefix)/mu/share/info'
+	$(MKDIR_P) '$(DESTDIR)$(infodir)/more'
+	cp '$(srcdir)/dir' '$(DESTDIR)$(infodir)/more'
+END
+
+$AUTOMAKE
+./config.status Makefile
+
+# Sanity checks, again.
+$MAKE install
+find inst -type f
+test -f inst/mu/share/info/dir
+test -f inst/share/info/more/dir
+# We expect the uninstall target of our Makefile to be definitely broken.
+$MAKE uninstall
+test -f inst/mu/share/info/dir
+test -f inst/share/info/more/dir
+rm -rf inst
+
+$MAKE distcheck >output 2>&1 && { cat output; Exit 1; }
+cat output
+
+$FGREP 'ERROR: files left after uninstall:' output
+grep '/mu/share/info/dir *$' output
+grep '/share/info/more/dir *$' output
+
 :

Message #42 received at 9579-done <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Nick Bowler <nbowler <at> elliptictech.com>,
 9579-done <at> debbugs.gnu.org
Cc: automake-patches <at> gnu.org
Subject: Re: bug#9579: distcheck does not detect incomplete uninstall as
	advertised
Date: Sat, 1 Oct 2011 19:23:58 +0200

On Tuesday 27 September 2011, Stefano Lattarini wrote:
> On Monday 26 September 2011, Nick Bowler wrote:
> > Hi Stefano,
> > 
> > On 2011-09-23 21:46 +0200, Stefano Lattarini wrote:
> > > 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: Stefano Lattarini <stefano.lattarini <at> gmail.com>
> > > Date: Fri, 23 Sep 2011 16:06:59 +0200
> > > Subject: [PATCH] distuninstallcheck: fail also when only one file is left installed
> > 
> > I've tested this on my project, and now distcheck fails when I revert my
> > uninstall-local rule.  It's perhaps a bit unfortunate that
> > 
> >   make infodir='${prefix}/somewhere_else' distcheck
> > 
> > no longer works, but I agree that it seems non-trivial to make that
> > work properly.
> >
> Well, I don't agree anymore on this ;-) -- by complicating my patch just
> a little bit, we can apparently cater for this situation as well.  See the
> attached squash-in (I've also attached the updated patch, for reference).
> 
> So I'll give a couple of days for further comments, than I'll push this
> improved patch instead of the previous one.
> 
> Thanks,
>   Stefano
> 
Pushed now, so I'm closing the bug report.

Regards,
  Stefano

Previous Next

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