GNU bug report logs - #10878
"make dist" with read-only srcdir generates read-only tarball

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 10878 in the body.
You can then email your comments to 10878 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: "make dist" with read-only srcdir generates read-only tarball
Date: Fri, 24 Feb 2012 10:09:38 -0500

Hi,

When using a read-only srcdir, "make dist" will generate a tarball whose
permissions have all files read-only (though directories are writable).
I noticed this because "dist-hook" targets cannot actually modify any
files in distdir as they're all read-only (without first running chmod,
anyway), causing distcheck failures.  While the GNU coding standards do
not demand write permissions on files in distribution tarballs (I'm not
really sure why), Automake's behaviour in the build directory should not
depend on whether or not srcdir is writable.

Tested with both automake-1.11.1 and git master.

Here's a simple example:

  % cat >configure.ac <<'EOF'
AC_INIT([test], [1.0])
AM_INIT_AUTOMAKE([foreign])
AC_CONFIG_FILES([Makefile])
AC_OUTPUT
EOF

  % cat >Makefile.am <<'EOF'
EOF

  % autoreconf -is
  % ./configure && make dist
  % tar tvf test-1.0.tar.gz

drwxr-xr-x 550/509           0 2012-02-24 09:56 test-1.0/
-rw-r--r-- 550/509          89 2012-02-24 09:56 test-1.0/configure.ac
-rw-r--r-- 550/509       22000 2012-02-24 09:56 test-1.0/aclocal.m4
-rwxr-xr-x 550/509       98862 2012-02-24 09:56 test-1.0/configure
-rw-r--r-- 550/509           0 2012-02-24 09:56 test-1.0/Makefile.am
-rw-r--r-- 550/509       13264 2012-02-24 09:56 test-1.0/Makefile.in
-rwxr-xr-x 550/509       11419 2009-12-10 12:58 test-1.0/missing
-rwxr-xr-x 550/509       13663 2009-12-10 12:58 test-1.0/install-sh

  (notice correct user write permissions on all files)

  % tar xf test-1.0.tar.gz
  % chmod -R a-w test-1.0
  % mkdir build && cd build
  % ../test-1.0/configure && make dist
  % tar tvf test-1.0.tar.gz

drwxr-xr-x 550/509           0 2012-02-24 09:59 test-1.0/
-r--r--r-- 550/509          89 2012-02-24 09:56 test-1.0/configure.ac
-r--r--r-- 550/509       22000 2012-02-24 09:56 test-1.0/aclocal.m4
-r-xr-xr-x 550/509       98862 2012-02-24 09:56 test-1.0/configure
-r--r--r-- 550/509           0 2012-02-24 09:56 test-1.0/Makefile.am
-r--r--r-- 550/509       13264 2012-02-24 09:56 test-1.0/Makefile.in
-r-xr-xr-x 550/509       11419 2009-12-10 12:58 test-1.0/missing
-r-xr-xr-x 550/509       13663 2009-12-10 12:58 test-1.0/install-sh

  (notice that user write permissions are missing)

Automake should at least add user write permissions to all files in
distdir prior to running dist-hook (and hence prior to generating the
distribution tarball).

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

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

From: Eric Blake <eblake <at> redhat.com>
To: Nick Bowler <nbowler <at> elliptictech.com>
Cc: 10878 <at> debbugs.gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Fri, 24 Feb 2012 09:12:54 -0700

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

On 02/24/2012 08:09 AM, Nick Bowler wrote:
> Hi,
> 
> When using a read-only srcdir, "make dist" will generate a tarball whose
> permissions have all files read-only (though directories are writable).
> I noticed this because "dist-hook" targets cannot actually modify any
> files in distdir as they're all read-only (without first running chmod,
> anyway), causing distcheck failures.  While the GNU coding standards do
> not demand write permissions on files in distribution tarballs (I'm not
> really sure why), Automake's behaviour in the build directory should not
> depend on whether or not srcdir is writable.
> 
> Tested with both automake-1.11.1 and git master.

> 
>   (notice that user write permissions are missing)
> 
> Automake should at least add user write permissions to all files in
> distdir prior to running dist-hook (and hence prior to generating the
> distribution tarball).

Automake must not add write permissions to files that were intended to
be shipped as read-only.  For example, coreutils intentionally converts
generated-but-distributed files to read-only, so that users are more
likely to notice that they should edit the source that generates the
file, and not the generated file itself.

So how do you propose to tell automake which files are supposed to be
read-only, vs. those that should be writable even if the tarball was
re-created from a read-only srcdir?

-- 
Eric Blake   eblake <at> redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

[signature.asc (application/pgp-signature, attachment)]

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

From: Nick Bowler <nbowler <at> elliptictech.com>
To: Eric Blake <eblake <at> redhat.com>
Cc: 10878 <at> debbugs.gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Fri, 24 Feb 2012 12:26:04 -0500

On 2012-02-24 09:12 -0700, Eric Blake wrote:
> On 02/24/2012 08:09 AM, Nick Bowler wrote:
[...]
> > Automake should at least add user write permissions to all files in
> > distdir prior to running dist-hook (and hence prior to generating the
> > distribution tarball).
> 
> Automake must not add write permissions to files that were intended to
> be shipped as read-only.  For example, coreutils intentionally converts
> generated-but-distributed files to read-only, so that users are more
> likely to notice that they should edit the source that generates the
> file, and not the generated file itself.

This sounds to me like a weird special case.  Coreutils would still work
correctly if the files were writable (otherwise it wouldn't build at all
on some filesystems).  Generated files should also have a statement to
that effect in them, if at all possible.

Regardless, I think it's a bad idea and frustrating for users to have
any read-only files in a source tarball at all.

> So how do you propose to tell automake which files are supposed to be
> read-only, vs. those that should be writable even if the tarball was
> re-created from a read-only srcdir?

With my above proposed solution, the way to do this would be to use a
dist-hook to remove the write permissions from the relevant files at
"make dist" time.

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

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

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Eric Blake <eblake <at> redhat.com>
Cc: Nick Bowler <nbowler <at> elliptictech.com>, 10878 <at> debbugs.gnu.org,
	automake-patches <at> gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Fri, 24 Feb 2012 18:37:39 +0100

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

severity 10878 wishlist
tags 10878 wontfix
close 10878
thanks


On 02/24/2012 08:09 AM, Nick Bowler wrote:
>
> Automake should at least add user write permissions to all files in
> distdir prior to running dist-hook (and hence prior to generating the
> distribution tarball).
>
I disagree; in case the user wants to change the permission of the distributed
files from the ones that they have in the builddir/srcdir, he can do so in the
dist-hook already, with full control (I've just added a tiny test to verify
this behaviour, see attached patch).  So we don't need to change the automake
behaviour in this respect (but a documentation improvement might be nice, in
case someone feels like writing it).

On 02/24/2012 05:12 PM, Eric Blake replied:
>
> Automake must not add write permissions to files that were intended to
> be shipped as read-only.  For example, coreutils intentionally converts
> generated-but-distributed files to read-only, so that users are more
> likely to notice that they should edit the source that generates the
> file, and not the generated file itself.
> 
> So how do you propose to tell automake which files are supposed to be
> read-only, vs. those that should be writable even if the tarball was
> re-created from a read-only srcdir?
>
With a dist-hook ;-)

Regards,
   Stefano

[0001-coverage-test-that-dist-hook-can-modify-file-permiss.patch (text/x-diff, attachment)]

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

From: Nick Bowler <nbowler <at> elliptictech.com>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: 10878 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>,
	automake-patches <at> gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Fri, 24 Feb 2012 12:53:38 -0500

On 2012-02-24 18:37 +0100, Stefano Lattarini wrote:
[...]
> On 02/24/2012 08:09 AM, Nick Bowler wrote:
> > Automake should at least add user write permissions to all files in
> > distdir prior to running dist-hook (and hence prior to generating the
> > distribution tarball).
>
> I disagree; in case the user wants to change the permission of the distributed
> files from the ones that they have in the builddir/srcdir, he can do so in the
> dist-hook already, with full control (I've just added a tiny test to verify
> this behaviour, see attached patch).  So we don't need to change the automake
> behaviour in this respect (but a documentation improvement might be nice, in
> case someone feels like writing it).

In that case, the manual should include a note that you should not
expect "make dist" to work properly with a read-only source tree.
Distcheck should also refrain from testing this.

As I mentioned in my original mail, "make distcheck" currently attempts
to run "make dist" with a read-only source tree, and it is causing
failures in my package because of the broken permissions.

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

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

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Nick Bowler <nbowler <at> elliptictech.com>
Cc: 10878 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>,
	automake-patches <at> gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Fri, 24 Feb 2012 19:19:32 +0100

On 02/24/2012 06:53 PM, Nick Bowler wrote:
> On 2012-02-24 18:37 +0100, Stefano Lattarini wrote:
> [...]
>> On 02/24/2012 08:09 AM, Nick Bowler wrote:
>>> Automake should at least add user write permissions to all files in
>>> distdir prior to running dist-hook (and hence prior to generating the
>>> distribution tarball).
>>
>> I disagree; in case the user wants to change the permission of the distributed
>> files from the ones that they have in the builddir/srcdir, he can do so in the
>> dist-hook already, with full control (I've just added a tiny test to verify
>> this behaviour, see attached patch).  So we don't need to change the automake
>> behaviour in this respect (but a documentation improvement might be nice, in
>> case someone feels like writing it).
> 
> In that case, the manual should include a note that you should not
> expect "make dist" to work properly with a read-only source tree.
>
But it's the package that expects its distributed files to be writable
that is assuming too much; if such package wants its expectation to
safely hold, it should add something like this in its 'dist-hook':

    find $(distdir) -exec chmod u+w '{}' ';'

> Distcheck should also refrain from testing this.
>
Nope; distcheck testing that the distributed package builds correctly with
a read-only source tree is a feature, not a bug, since the GCS require this.
If you don't care about this requirment, than you need to provide a
'distcheck-hook' that renders your source directory writable before it
gets used by "distcheck" (I'll update my patch to have it test also such
usage).

> As I mentioned in my original mail, "make distcheck" currently attempts
> to run "make dist" with a read-only source tree,
>
As it should do.

> and it is causing failures in my package because of the broken
> permissions.
>

HTH,
  Stefano

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

From: Nick Bowler <nbowler <at> elliptictech.com>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: 10878 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>,
	automake-patches <at> gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Fri, 24 Feb 2012 13:34:36 -0500

On 2012-02-24 19:19 +0100, Stefano Lattarini wrote:
> On 02/24/2012 06:53 PM, Nick Bowler wrote:
> > On 2012-02-24 18:37 +0100, Stefano Lattarini wrote:
> > [...]
> >> On 02/24/2012 08:09 AM, Nick Bowler wrote:
> >>> Automake should at least add user write permissions to all files in
> >>> distdir prior to running dist-hook (and hence prior to generating the
> >>> distribution tarball).
> >>
> >> I disagree; in case the user wants to change the permission of the distributed
> >> files from the ones that they have in the builddir/srcdir, he can do so in the
> >> dist-hook already, with full control (I've just added a tiny test to verify
> >> this behaviour, see attached patch).  So we don't need to change the automake
> >> behaviour in this respect (but a documentation improvement might be nice, in
> >> case someone feels like writing it).
> > 
> > In that case, the manual should include a note that you should not
> > expect "make dist" to work properly with a read-only source tree.
>
> But it's the package that expects its distributed files to be writable
> that is assuming too much; if such package wants its expectation to
> safely hold, it should add something like this in its 'dist-hook':
> 
>     find $(distdir) -exec chmod u+w '{}' ';'

So I've done this now, but note that this is actually harder than you
make it out to be.  If the package adds prerequisites to the dist-hook
target (I find it is generally good practice for -hook and -local
targets to use prerequisites with commands instead of putting commands
directly in those targets) to the dist-hook target, one would want the
permissions to be fixed *before* executing the prerequisite targets.
But this cannot be done by simply adding things directly to the
dist-hook target.

> > Distcheck should also refrain from testing this.
>
> Nope; distcheck testing that the distributed package builds correctly with
> a read-only source tree is a feature, not a bug, since the GCS require this.

I'm not talking about building the package, which absolutely should work
from a read-only source tree.  I'm talking about creating a distribution
tarball, with "make dist": something only package maintainers (that's
me!) will generally do.

As it stands, "make dist" currently does not work properly from a
read-only source tree.  The tarball generated by my minimal example (an
empty Makefile.am!) is clearly broken, and I would not want to be caught
distributing such a thing.

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

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

From: Eric Blake <eblake <at> redhat.com>
To: Nick Bowler <nbowler <at> elliptictech.com>
Cc: 10878 <at> debbugs.gnu.org, Stefano Lattarini <stefano.lattarini <at> gmail.com>,
	automake-patches <at> gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Fri, 24 Feb 2012 12:10:07 -0700

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

On 02/24/2012 11:34 AM, Nick Bowler wrote:
>> But it's the package that expects its distributed files to be writable
>> that is assuming too much; if such package wants its expectation to
>> safely hold, it should add something like this in its 'dist-hook':
>>
>>     find $(distdir) -exec chmod u+w '{}' ';'

I agree.

> 
> I'm not talking about building the package, which absolutely should work
> from a read-only source tree.  I'm talking about creating a distribution
> tarball, with "make dist": something only package maintainers (that's
> me!) will generally do.

That's where we are arguing that you are wrong.  The GNU Coding
Standards requires that _anyone_ can run 'make dist', and not just
'package maintainers'.  That's the whole point of software freedom:
You, as a recipient of the tarball, should be just as free to
redistribute your modifications (including rebuilding a tarball) as the
upstream maintainer you got the package from, even if you are starting
from your (possibly read-only) copy of the expanded tarball.

Which is _why_ 'make distcheck' intentionally checks that 'make dist'
from a read-only source tarball will accurately create a tarball.  This
is a _feature_ of automake's 'make distcheck'.  If you want to guarantee
that the generated tarball has certain permissions, then you, as the
package author, must add something in your dist-hook to guarantee it,
since automake cannot know which files you _need_ to leave writable.

-- 
Eric Blake   eblake <at> redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

[signature.asc (application/pgp-signature, attachment)]

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

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Nick Bowler <nbowler <at> elliptictech.com>
Cc: 10878 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>,
	automake-patches <at> gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Fri, 24 Feb 2012 20:25:28 +0100

On 02/24/2012 07:34 PM, Nick Bowler wrote:
>
> On 2012-02-24 19:19 +0100, Stefano Lattarini wrote:
>>
>> But it's the package that expects its distributed files to be writable
>> that is assuming too much; if such package wants its expectation to
>> safely hold, it should add something like this in its 'dist-hook':
>>
>>     find $(distdir) -exec chmod u+w '{}' ';'
> 
> So I've done this now, but note that this is actually harder than you
> make it out to be.
>
True, but only a little ...

> If the package adds prerequisites to the dist-hook target ... [SNIP]
>
... you can do this:

  make-distdir-writable:
	find $(distdir) -exec chmod u+w '{}' ';'
  dist_hook_prereqs = foo bar
  dist-hook: $(dist_hook_prereqs)
  $(dist_hook_prereqs): make-distdir-writable

> (I find it is generally good practice for -hook and -local
> targets to use prerequisites with commands instead of putting commands
> directly in those targets)
>
JFTR, I agree.

> to the dist-hook target, one would want the
> permissions to be fixed *before* executing the prerequisite targets.
> But this cannot be done by simply adding things directly to the
> dist-hook target.
> 
>>> Distcheck should also refrain from testing this.
>>
>> Nope; distcheck testing that the distributed package builds correctly with
>> a read-only source tree is a feature, not a bug, since the GCS require this.
> 
> I'm not talking about building the package, which absolutely should work
> from a read-only source tree.  I'm talking about creating a distribution
> tarball,
>
But the distribution tarball created by automake *won't* have write
permissions stripped (assuming they were present in the first place,
of course).

> with "make dist": something only package maintainers (that's
> me!) will generally do.
> 
> As it stands, "make dist" currently does not work properly from a
> read-only source tree.
>
I honestly don't see why not; if you don't want to put readonly files in
the distributed tarball, don't create the tarball from a readonly srcdir.
It's not like automake is removing the write permissions itslef (now,
that would quite be a bug!); it's you who have removed it before creating
the tarball:

  % tar xf test-1.0.tar.gz
  % chmod -R a-w test-1.0 # <-- !HERE!
  % mkdir build && cd build
  % ../test-1.0/configure && make dist
  % tar tvf test-1.0.tar.gz

To stress this again: if *you* had removed the executable bit from your
test scripts, would you blame it on automake if "make distcheck" stopped
working?  I don't think so.

> The tarball generated by my minimal example (an empty Makefile.am!) is
> clearly broken, and I would not want to be caught distributing such a thing.
> 
> Cheers,

Regards,
  Stefano

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

From: Nick Bowler <nbowler <at> elliptictech.com>
To: Eric Blake <eblake <at> redhat.com>
Cc: 10878 <at> debbugs.gnu.org, Stefano Lattarini <stefano.lattarini <at> gmail.com>,
	automake-patches <at> gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Fri, 24 Feb 2012 14:34:49 -0500

On 2012-02-24 12:10 -0700, Eric Blake wrote:
> On 02/24/2012 11:34 AM, Nick Bowler wrote:
> >> But it's the package that expects its distributed files to be writable
> >> that is assuming too much; if such package wants its expectation to
> >> safely hold, it should add something like this in its 'dist-hook':
> >>
> >>     find $(distdir) -exec chmod u+w '{}' ';'
> 
> I agree.
> 
> > 
> > I'm not talking about building the package, which absolutely should work
> > from a read-only source tree.  I'm talking about creating a distribution
> > tarball, with "make dist": something only package maintainers (that's
> > me!) will generally do.
> 
> That's where we are arguing that you are wrong.  The GNU Coding
> Standards requires that _anyone_ can run 'make dist', and not just
> 'package maintainers'.

OK, but I was responding to a part of Stefano's message (which you
elided) which said "... *builds* correctly with a read-only source
tree" (emphasis added).  Regardless, I cannot find where this supposed
GCS requirement for "make dist" (or even ordinary builds, for that
matter) to work on read-only source trees comes from.  Do you have a
reference?

> That's the whole point of software freedom: You, as a recipient of the
> tarball, should be just as free to redistribute your modifications
> (including rebuilding a tarball) as the upstream maintainer you got
> the package from, even if you are starting from your (possibly
> read-only) copy of the expanded tarball.

If someone is redistributing modifications, then presumably they have a
writable source tree in order to create their modifications in the first
place.  They can then run "make dist" from the modified tree.  When the
files in a tarball are read-only, however, users who want to modify
source files are presented with a totally gratuitous (and frustrating!)
extra step.

I also didn't say anything about any "upstream maintainer": when someone
is distributing a modified version, then they are for all intents and
purposes a maintainer of their own version.  People creating a
distribution tarball from modified sources can reasonably expect to
require extra tools or setup over and above what's required to merely
build the package.  For example, they will probably need to have GNU
Automake installed.

> Which is _why_ 'make distcheck' intentionally checks that 'make dist'
> from a read-only source tarball will accurately create a tarball.

It checks that it creates a tarball, but as I mentioned in another
paragraph (which you elided), it does not check that the tarball is
accurate.  In my original example, "make distcheck" failed to notice
that the generated tarball was wrong: i.e., it was not the same as the
original tarball it was testing.

> This is a _feature_ of automake's 'make distcheck'.  If you want to
> guarantee that the generated tarball has certain permissions, then
> you, as the package author, must add something in your dist-hook to
> guarantee it, since automake cannot know which files you _need_ to
> leave writable.

The answer is: all of them!

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

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

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Nick Bowler <nbowler <at> elliptictech.com>
Cc: 10878 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>,
	automake-patches <at> gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Fri, 24 Feb 2012 20:35:06 +0100

On 02/24/2012 08:25 PM, Stefano Lattarini wrote:
> (I'll update my patch to have it test also such usage).
>
And here it is the updated patch.  I will push it shortly if there is
no objection.

Regards,
  Stefano

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

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Nick Bowler <nbowler <at> elliptictech.com>
Cc: 10878 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>,
	automake-patches <at> gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Fri, 24 Feb 2012 20:39:06 +0100

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

On 02/24/2012 08:35 PM, Stefano Lattarini wrote:
> On 02/24/2012 08:25 PM, Stefano Lattarini wrote:
>> (I'll update my patch to have it test also such usage).
>>
> And here it is the updated patch.  I will push it shortly if there is
> no objection.
> 
Maybe it would have been nice actually attaching the patch ... ;-)

Sorry for the confusion,
  Stefano

[0001-coverage-dist-hook-and-distcheck-hook-can-modify-fil.patch (text/x-diff, attachment)]

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

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Nick Bowler <nbowler <at> elliptictech.com>
Cc: 10878 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>,
	automake-patches <at> gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Fri, 24 Feb 2012 20:48:13 +0100

On 02/24/2012 08:34 PM, Nick Bowler wrote:
>
> On 2012-02-24 12:10 -0700, Eric Blake wrote:
>
> [BIG SNIP]
>
>> Which is _why_ 'make distcheck' intentionally checks that 'make dist'
>> from a read-only source tarball will accurately create a tarball.
> 
> It checks that it creates a tarball, but as I mentioned in another
> paragraph (which you elided), it does not check that the tarball is
> accurate.
>
Well, it tries at least, by checking common symptoms for inaccuracy
(failure to build with a read-only source tree, "make distclean"
leaving files in the build tree, "make dist" failing from an
extracted tarball, ...).  Of course, it cannot be perfect, and
there are tons of subtle potential breakages it won't catch; but
IMHO it does a good work overall.

> In my original example, "make distcheck" failed to notice that
> the generated tarball was wrong: i.e., it was not the same as the
> original tarball it was testing.
>
How so?  Honest question, that doesn't seem clear from your
descriptions and explanations so far.

>> This is a _feature_ of automake's 'make distcheck'.  If you want to
>> guarantee that the generated tarball has certain permissions, then
>> you, as the package author, must add something in your dist-hook to
>> guarantee it, since automake cannot know which files you _need_ to
>> leave writable.
> 
> The answer is: all of them!
>
In light of our discussion so far, implementing this would seems like
a gratuitous modification to me.  Let me explain why.  Both the existing
behaviour and the one you propose are non-broken and have their merits,
and both would be quite easy to implement on a per-project basis if the
other behaviour were the default.  Given this consideration, for the
sake of simplicity and backward-compatibility, I simply prefer to leave
the pre-existing behaviour in place.

Regards,
  Stefano

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

From: Nick Bowler <nbowler <at> elliptictech.com>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: 10878 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>,
	automake-patches <at> gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Fri, 24 Feb 2012 15:15:02 -0500

On 2012-02-24 20:25 +0100, Stefano Lattarini wrote:
> On 02/24/2012 07:34 PM, Nick Bowler wrote:
> >
> > On 2012-02-24 19:19 +0100, Stefano Lattarini wrote:
> >>
> >> But it's the package that expects its distributed files to be writable
> >> that is assuming too much; if such package wants its expectation to
> >> safely hold, it should add something like this in its 'dist-hook':
> >>
> >>     find $(distdir) -exec chmod u+w '{}' ';'
> > 
> > So I've done this now, but note that this is actually harder than you
> > make it out to be.
> >
> True, but only a little ...
> 
> > If the package adds prerequisites to the dist-hook target ... [SNIP]
> >
> ... you can do this:
> 
>   make-distdir-writable:
> 	find $(distdir) -exec chmod u+w '{}' ';'
>   dist_hook_prereqs = foo bar
>   dist-hook: $(dist_hook_prereqs)
>   $(dist_hook_prereqs): make-distdir-writable

Yes, this is more or less what I did.  The annoying thing is that now
one needs to embed knowledge about either "make-distdir-writable" or
"dist_hook_prereqs" into multiple locations in the makefile.

> > (I find it is generally good practice for -hook and -local
> > targets to use prerequisites with commands instead of putting commands
> > directly in those targets)
> >
> JFTR, I agree.

Interestingly, I find no mention at all of this trick in the automake
manual.  I'm sure I read about it in _some_ manual, I wonder which it
was?

> >>> Distcheck should also refrain from testing this.
[...]
> > As it stands, "make dist" currently does not work properly from a
> > read-only source tree.
>
> I honestly don't see why not; if you don't want to put readonly files in
> the distributed tarball, don't create the tarball from a readonly srcdir.
> It's not like automake is removing the write permissions itslef (now,
> that would quite be a bug!); it's you who have removed it before creating
> the tarball:
> 
>   % tar xf test-1.0.tar.gz
>   % chmod -R a-w test-1.0 # <-- !HERE!

So I agree that, in the example I posted, it was a direct consequence of
my actions.  When I first posted I assumed that non-user-writable files
in tarballs was universally undesirable, so I only tried to demonstrate
that.  Clearly that is not the case, and people really do want to
include read-only files in their tarballs.  I think such policy
decisions belong in dist-hook rather than according to whatever the
current state of the source tree happens to be, but I guess it can't
be changed now without introducing regressions.

So let's actually put some stuff in Makefile.am to demonstrate something
closer to the problem I'm having.

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

dist-hook:
	echo bar > $(distdir)/foo
EOF

  % cat >foo <<'EOF'
EOF

  % autoreconf -is
  % ./configure && make distcheck
  [...]
  make  \
    top_distdir="test-1.0" distdir="test-1.0" \
    dist-hook
  make[2]: Entering directory `/tmp/testcase/test-1.0/_build'
  echo bar > test-1.0/foo
  /bin/sh: test-1.0/foo: Permission denied

>   % mkdir build && cd build
>   % ../test-1.0/configure && make dist
>   % tar tvf test-1.0.tar.gz

This was just a minimal example.  The real problem is that "make
distcheck" does something isomorphic to the above, and expects it to
work.  If your dist-hook modifies distdir, then it does not work (my
particular issue was actually way more specific than the above: the
dist-hook would actually work properly most of the time.  It only failed
when srcdir was read-only and distdir was on a certain NFS mount on my
laptop...).

> To stress this again: if *you* had removed the executable bit from your
> test scripts, would you blame it on automake if "make distcheck" stopped
> working?  I don't think so.

No, but if "make distcheck" was doing the removal, as is the case here,
then I *would* blame automake.

(Aside: I actually ran into the exact problem of executable bits being
stripped by a different build system, but I digress).

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

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

From: Nick Bowler <nbowler <at> elliptictech.com>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: 10878 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>,
	automake-patches <at> gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Fri, 24 Feb 2012 15:36:00 -0500

On 2012-02-24 20:39 +0100, Stefano Lattarini wrote:
> On 02/24/2012 08:35 PM, Stefano Lattarini wrote:
> > On 02/24/2012 08:25 PM, Stefano Lattarini wrote:
> >> (I'll update my patch to have it test also such usage).
> >>
> > And here it is the updated patch.  I will push it shortly if there is
> > no objection.
> > 
> Maybe it would have been nice actually attaching the patch ... ;-)
> 
> Sorry for the confusion,
>   Stefano

> From 8f6a79c2a4ff5557142b552c96cfa171e43f52ab Mon Sep 17 00:00:00 2001
> Message-Id: <8f6a79c2a4ff5557142b552c96cfa171e43f52ab.1330112034.git.stefano.lattarini <at> gmail.com>
> From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
> Date: Fri, 24 Feb 2012 18:34:04 +0100
> Subject: [PATCH] coverage: dist-hook and distcheck-hook can modify file
>  permission
> 
> See automake bug#10878.
> 
> * tests/dist-hook-perms.test: New test, checking that the user can
> use the 'dist-hook' target to modify permissions of distributed
> files before putting them in the distribution tarball.
> * tests/distcheck-writable-srcdir.test: New test, checking that the
> user can use 'distcheck-hook' to ensure his packages is built by
> "make distcheck" with a writable srcdir.  This goes against the GNU
> Coding Standards, but some package authors might not care about
> them too much, and it's nice to support their use case too.

FWIW, I really do care about being able to build the package from a
read-only srcdir, and I absolutely want to make sure it works.  So it's
great that distcheck tests this.

I would even be happy for "make dist" to work in this scenario, and it
does with a hook to fix up the permissions (modulo the increased
maintainence burden described elsethread), so I suppose I'll just have
to stick with that.

I use the rule that no part of the build should write to srcdir, ever:
so it should be possible to do a successful VPATH build with a
maintainer-cleaned, read-only srcdir.  I believe this actually goes
above and beyond what Automake provides.  The exception to this is
during initial bootstrap after checkout from git, which must write to
srcdir for obvious reasons.  (Aside: I just tested this and discovered
that there was a source file missing from the distribution that
distcheck didn't notice because only maintainer-clean builds failed.
Oops!)

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

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

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Nick Bowler <nbowler <at> elliptictech.com>
Cc: 10878 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>,
	automake-patches <at> gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Sat, 25 Feb 2012 00:11:34 +0100

On 02/24/2012 09:15 PM, Nick Bowler wrote:
>
> On 2012-02-24 20:25 +0100, Stefano Lattarini wrote:
>
>> On 02/24/2012 07:34 PM, Nick Bowler wrote:
>>
>>> (I find it is generally good practice for -hook and -local
>>> targets to use prerequisites with commands instead of putting commands
>>> directly in those targets)
>>>
>> JFTR, I agree.
> 
> Interestingly, I find no mention at all of this trick in the automake
> manual.
>
Because it's not a trick, but a matter of preference :-)  Preference that
we apparently share, but which doesn't necessarily need not to be shared
by other people.

> I'm sure I read about it in _some_ manual, I wonder which it was?
> 
No idea here, sorry.

> So let's actually put some stuff in Makefile.am to demonstrate something
> closer to the problem I'm having.
> 
>   % cat >Makefile.am <<'EOF'
> EXTRA_DIST = foo
> 
> dist-hook:
> 	echo bar > $(distdir)/foo
> EOF
> 
>   % cat >foo <<'EOF'
> EOF
> 
>   % autoreconf -is
>   % ./configure && make distcheck
>   [...]
>   make  \
>     top_distdir="test-1.0" distdir="test-1.0" \
>     dist-hook
>   make[2]: Entering directory `/tmp/testcase/test-1.0/_build'
>   echo bar > test-1.0/foo
>   /bin/sh: test-1.0/foo: Permission denied
>
Ah, this is a better example.  Indeed we have a problem here (at the very
least a documentation one).  I'm thus re-opening this bug report.

>> To stress this again: if *you* had removed the executable bit from your
>> test scripts, would you blame it on automake if "make distcheck" stopped
>> working?  I don't think so.
> 
> No, but if "make distcheck" was doing the removal, as is the case here,
> then I *would* blame automake.
>
Fair enough.  Maybe we should explicitly document the "make distcheck"
behaviour in more details (including your example) ...

Thanks,
  Stefano

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

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Nick Bowler <nbowler <at> elliptictech.com>
Cc: 10878 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>,
	automake-patches <at> gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Sat, 25 Feb 2012 13:41:30 +0100

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

On 02/25/2012 12:11 AM, Stefano Lattarini wrote:
>
> Ah, this is a better example.  Indeed we have a problem here (at the very
> least a documentation one).
>
As a first step, the attached patch should improve the existing documentation
on "make distcheck" a little.  I will apply soonish to master if there is no
objection.

Regards,
  Stefano

[0001-docs-improve-make-distcheck-documentation.patch (text/x-diff, attachment)]

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

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Nick Bowler <nbowler <at> elliptictech.com>
Cc: 10878 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>,
	automake-patches <at> gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Sat, 25 Feb 2012 13:48:48 +0100

On 02/24/2012 09:36 PM, Nick Bowler wrote:
>
> I use the rule that no part of the build should write to srcdir, ever:
> so it should be possible to do a successful VPATH build with a
> maintainer-cleaned, read-only srcdir.
>
Note that automake does not honour this expectation (for example,
distributed info files are generated in the srcdir).  Such a policy
of "always place generated but distributed files in the $(srcdir)"
might become more common and even advised in the future, due to subtle
breakages that might be triggered by having an outdated version of a
generated file in srcdir and a "more fresh" one in the builddir; for
a real-world example of such a breakage, see:
  <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10852#11>
and following messages.

> I believe this actually goes above and beyond what Automake provides.
>
True, especially in light of the above.

Regards,
  Stefano

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

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Nick Bowler <nbowler <at> elliptictech.com>
Cc: 10878 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>,
	automake-patches <at> gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Sat, 25 Feb 2012 14:39:27 +0100

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

On 02/25/2012 01:41 PM, Stefano Lattarini wrote:
> On 02/25/2012 12:11 AM, Stefano Lattarini wrote:
>>
>> Ah, this is a better example.  Indeed we have a problem here (at the very
>> least a documentation one).
>>
> As a first step, the attached patch should improve the existing documentation
> on "make distcheck" a little.  I will apply soonish to master if there is no
> objection.
> 
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.

Regards,
  Stefano

[0001-docs-improve-make-distcheck-documentation.patch (text/x-diff, attachment)]

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

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Nick Bowler <nbowler <at> elliptictech.com>
Cc: 10878 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>,
	automake-patches <at> gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Sat, 25 Feb 2012 18:41:29 +0100

On 02/25/2012 01:41 PM, Stefano Lattarini wrote:
> On 02/25/2012 12:11 AM, Stefano Lattarini wrote:
>>
>> Ah, this is a better example.  Indeed we have a problem here (at the very
>> least a documentation one).
>>
> As a first step, the attached patch should improve the existing documentation
> on "make distcheck" a little.  I will apply soonish to master if there is no
> objection.
> 
Pushed now.

Regards,
  Stefano

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

From: Nick Bowler <nbowler <at> elliptictech.com>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: 10878 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>,
	automake-patches <at> gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Sat, 25 Feb 2012 13:10:03 -0500

Hi Stefano,

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".

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

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

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Nick Bowler <nbowler <at> elliptictech.com>
Cc: 10878 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>,
	automake-patches <at> gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Sat, 25 Feb 2012 20:25:54 +0100

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

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

[0001-docs-improve-dist-hook-documentation.patch (text/x-diff, attachment)]

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

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Nick Bowler <nbowler <at> elliptictech.com>
Cc: 10878 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>,
	automake-patches <at> gnu.org
Subject: Re: bug#10878: "make dist" with read-only srcdir generates read-only
	tarball
Date: Sun, 26 Feb 2012 10:34:02 +0100

tags 10878 patch
close 10878
thanks

On 02/25/2012 08:25 PM, Stefano Lattarini wrote:
>
> I'll close this report by tomorrow if there are no further objection.
> 
Bug closed.

Thanks,
  Stefano

Previous Next

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