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.lattar...@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
From 71bae85a00a5de739267876d6351ddb664cb6658 Mon Sep 17 00:00:00 2001 Message-Id: <71bae85a00a5de739267876d6351ddb664cb6658.1317111869.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/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.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/distcheck-override-infodir.test: Likewise. + * tests/Makefile.am (TESTS): Add them. + * 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..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-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..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
diff --git a/ChangeLog b/ChangeLog index 2ba979c..875245f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,4 @@ -2011-09-23 Stefano Lattarini <stefano.lattar...@gmail.com> +2011-09-26 Stefano Lattarini <stefano.lattar...@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 + :