Re: bug#10878: "make dist" with read-only srcdir generates read-only tarball

[Nick Bowler]

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/)