On 02/25/2012 07:10 PM, Nick Bowler wrote:
> Hi Stefano,
>
Hi Nick, and thanks for all the feedback.
> One comment below:
>
> On 2012-02-25 14:39 +0100, Stefano Lattarini wrote:
> [...]
>> And here is the documentation about the fact that a dist-hook should be ready
>> to deal with read-only files. I will apply the attached patch soonish to
>> master
>> if there is no objection.
> [...]
>> +@noindent
>> +Note that the @code{dist-hook} recipe shouldn't assume that the
>> +regular files in the distribution directory are writable; this
>> +might not be the case if one is packaging from a read-only source
>> +tree, or when a @code{make distcheck} is being done. So, if the
>> +@code{dist-hook} wants to modify the content of an existing file
>> +in the distribution directory, it should explicitly ensure to make
>> +it readable first:
> ^^^^^^^^
> writable
>
> I also would drop the words "ensure to".
>
Good catches.
I've fixed that, started doing a couple of other improvements, and finally
basically re-wrote the patch (hopefully in a better way). The new version
of the patch, that I've pushed, is attached. This should settle the issue,
so I'll close this report by tomorrow if there are no further objection.
Thanks,
Stefano
>From e64729023df43c192f7e67c6c17549d68604a8e6 Mon Sep 17 00:00:00 2001
Message-Id: <e64729023df43c192f7e67c6c17549d68604a8e6.1330197794.git.stefano.lattar...@gmail.com>
From: Stefano Lattarini <stefano.lattar...@gmail.com>
Date: Sat, 25 Feb 2012 14:38:22 +0100
Subject: [PATCH] docs: improve 'dist-hook' documentation
* doc/automake.texi (The dist Hook): Explicitly document the fact
that the dist-hook should account for the case where the source
tree is read-only, mostly for the benefit of distcheck. Since
we are at it, do some minor unrelated rewordings, and remove
obsolescent advice. Motivated by the discussion on automake
bug#10878.
* tests/disthook-perms.test: Renamed ...
* tests/disthook.test: ... to this, and extended.
* tests/list-of-tests.mk: Adjust.
---
doc/automake.texi | 35 ++++++++++++------
tests/disthook-perms.test | 59 ------------------------------
tests/disthook.test | 89 +++++++++++++++++++++++++++++++++++++++++++++
tests/list-of-tests.mk | 2 +-
4 files changed, 113 insertions(+), 72 deletions(-)
delete mode 100755 tests/disthook-perms.test
create mode 100755 tests/disthook.test
diff --git a/doc/automake.texi b/doc/automake.texi
index c2c2a21..3bb365b 100644
--- a/doc/automake.texi
+++ b/doc/automake.texi
@@ -8420,24 +8420,35 @@ nodist_foo_SOURCES = do-not-distribute.c
Occasionally it is useful to be able to change the distribution before
it is packaged up. If the @code{dist-hook} rule exists, it is run
-after the distribution directory is filled, but before the actual tar
-(or shar) file is created. One way to use this is for distributing
-files in subdirectories for which a new @file{Makefile.am} is overkill:
+after the distribution directory is filled, but before the actual
+distribution archives are created. One way to use this is for
+removing unnecessary files that get recursively included by specifying
+a directory in @code{EXTRA_DIST}:
@example
+EXTRA_DIST = doc
dist-hook:
- mkdir $(distdir)/random
- cp -p $(srcdir)/random/a1 $(srcdir)/random/a2 $(distdir)/random
+ rm -rf `find $(distdir)/doc -type d -name .svn`
@end example
-Another way to use this is for removing unnecessary files that get
-recursively included by specifying a directory in EXTRA_DIST:
-
-@example
-EXTRA_DIST = doc
-
+@c The caveates described here should be documented in 'disthook.test'.
+@noindent
+Note that the @code{dist-hook} recipe shouldn't assume that the regular
+files in the distribution directory are writable; this might not be the
+case if one is packaging from a read-only source tree, or when a
+@code{make distcheck} is being done. For similar reasons, the recipe
+shouldn't assume that the subdirectories put into the distribution
+directory as effect of having them listed in @code{EXTRA_DIST} are
+writable. So, if the @code{dist-hook} recipe wants to modify the
+content of an existing file (or @code{EXTRA_DIST} subdirectory) in the
+distribution directory, it should explicitly to make it writable first:
+
+@example
+EXTRA_DIST = README doc
dist-hook:
- rm -rf `find $(distdir)/doc -type d -name .svn`
+ chmod u+w $(distdir)/README $(distdir)/doc
+ echo "Distribution date: `date`" >> README
+ rm -f $(distdir)/doc/HACKING
@end example
@vindex distdir
diff --git a/tests/disthook-perms.test b/tests/disthook-perms.test
deleted file mode 100755
index a5f0acb..0000000
--- a/tests/disthook-perms.test
+++ /dev/null
@@ -1,59 +0,0 @@
-#! /bin/sh
-# Copyright (C) 2012 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 the user can use the 'dist-hook' target to modify
-# permissions of distributed files before putting them in the
-# distribution tarball. See automake bug#10878.
-
-. ./defs || Exit 1
-
-echo AC_OUTPUT >> configure.ac
-
-cat > Makefile.am <<'END'
-EXTRA_DIST = write execute
-dist-hook:
- chmod u+w $(distdir)/write
- chmod u+x $(distdir)/execute
-END
-
-$ACLOCAL
-$AUTOMAKE
-$AUTOCONF
-
-./configure
-
-echo Will be clobbered > write
-cat > execute <<'END'
-#!/bin/sh
-echo I run successfully
-END
-
-chmod a-w write
-chmod a-x execute
-
-$MAKE
-$MAKE dist
-
-test -f $distdir.tar.gz
-gzip -dc $distdir.tar.gz | tar xvf -
-
-cd $distdir
-echo clobber clobber > write
-cat write | grep 'clobber clobber'
-./execute
-./execute | grep 'I run successfully'
-
-:
diff --git a/tests/disthook.test b/tests/disthook.test
new file mode 100755
index 0000000..9804e99
--- /dev/null
+++ b/tests/disthook.test
@@ -0,0 +1,89 @@
+#! /bin/sh
+# Copyright (C) 2012 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 'dist-hook' works. See automake bug#10878.
+
+. ./defs || Exit 1
+
+echo AC_OUTPUT >> configure.ac
+
+cat > Makefile.am <<'END'
+EXTRA_DIST = write execute removed doc
+
+removed:
+ echo I will be deleted > $@
+DISTCLEANFILES = removed
+
+dist-hook:
+ chmod u+w $(distdir)/write $(distdir)/doc
+ chmod u+x $(distdir)/execute
+ rm -f $(distdir)/removed
+ rm -f $(distdir)/doc/HACKING
+ rm -f $(distdir)/doc/RELEASE-DATE
+ date > $(distdir)/doc/RELEASE-DATE
+ echo all is ok > $(distdir)/write
+
+check-local:
+ ls -l $(srcdir) $(srcdir)/doc
+ test "`cat $(srcdir)/write`" = "all is ok"
+ test -f $(srcdir)/doc/README
+ test -f $(srcdir)/doc/RELEASE-DATE
+ test ! -f $(srcdir)/removed
+ test ! -r $(srcdir)/removed
+ test ! -f $(srcdir)/doc/HACKING
+ test ! -r $(srcdir)/doc/HACKING
+ $(srcdir)/execute
+ $(srcdir)/execute | grep 'I run successfully'
+## Sanity check.
+ echo ok > $(srcdir)/../distcheck-run
+END
+
+$ACLOCAL
+$AUTOMAKE
+$AUTOCONF
+
+./configure
+mkdir doc
+: > doc/README
+: > doc/HACKING
+echo will be clobbered > write
+cat > execute <<'END'
+#!/bin/sh
+echo I run successfully
+END
+
+chmod a-w write
+chmod a-x execute
+
+$MAKE distdir
+ls -l $distdir $distdir/doc
+cd $distdir
+test "`cat write`" = "all is ok"
+test ! -f removed
+test ! -r removed
+test -f doc/README
+test -f doc/RELEASE-DATE
+test ! -f doc/HACING
+test ! -r doc/HACING
+./execute
+./execute | grep 'I run successfully'
+cd ..
+
+
+$MAKE distcheck
+test -f distcheck-run
+
+:
diff --git a/tests/list-of-tests.mk b/tests/list-of-tests.mk
index 5507ee5..89cff2f 100644
--- a/tests/list-of-tests.mk
+++ b/tests/list-of-tests.mk
@@ -355,7 +355,7 @@ distcom4.test \
distcom5.test \
distcom-subdir.test \
distdir.test \
-disthook-perms.test \
+disthook.test \
distlinks.test \
distlinksbrk.test \
distname.test \
--
1.7.9
