[debhelper-devel] "Rules-Requires-Root: no" vs ExtUtils::Install + dh_strip_nondeterminism

[Axel Beckert]

Hi Mattia,

Mattia Rizzolo wrote:
> On Sat, Oct 28, 2017 at 02:21:37PM +0200, Axel Beckert wrote:
> > Do I understand it correctly that with "Rules-Requires-Root: no" the
> > root:root ownership is only set by dh_builddeb?
> 
> Well, by dpkg-deb of course :)
> Check the --root-owner-group option of dpkg-deb 1.19.0.

Ah, passed through. Thanks for the hint.

> Whilst I conisder ExtUtils::MakeMaker's behaviour fairly weird

As mentioned, I can see some reasoning from the
non-binary-distribution world.

> (and besides, if left alone it wouldn't be complian with Policy
> §10.9

Right.

> probably that's hidden by dh_fixperms?),

As far as my debugging went: Yes, dh_fixperms cleans up that mess and
especially hides it well.

> I might be heavily downgrading the issue you found, and simply say that
> File::StripNondeterminism::handlers::png is too eager in the open().
> Amongst the various handlers, png and ar are the only to that uses +<
> while opening the original file,

Ah, nice! I did indeed assume that most handlers were written in the
same way and that this issue is far more widespread inside strip-nd.

So it's also by far not such a big issue as I initially thought. Most
Perl packages don't include PNG images and even fewer have ar
archives.

>  but the png one has no reason to.

Indeed. It looks as if the filehandle is also used for writing on a
first glance, but this changes if one digs a little bit deeper.

> I tried live patching strip-nd by chaging the
>     open(my $fh, '+<', $filename)
> to
>     open(my $fh, '<', $filename)
> and indeed I could get a build completely identical to the one done
> without R³ and unpatched strip-nd (and the log confirms files are still
> being processed by strip-nd.

Did your example contain a PNG which needs a fixup?

But you're right that the "+" is not needed: The filehandle itself is
not passed to copy_data(). And copy_data() uses copy() from File::Copy
which surely does not rely on some specific filehandle being open. So
you are very likely right — which is great. :-)

Looking at
https://anonscm.debian.org/git/reproducible/strip-nondeterminism.git/tree/t/fixtures/png
it seems as if tEXt.png.in is a case where strip-nd should change
something (while the other examples contain symlinks and hence I
expect those being examples where nothing should be changed).

So if the test suite passes with your patch, the patch should be fine.

Maybe the test suite should do a variant with read-only files which
need to be modified, too, to verify that this case works, too, and
continues to work.

If that case fails, it might suffice to first unlink the old file
explicitly and then copy the temporary file.

> So, I think that to handle this issue we should just drop that single
> byte from strip-nd and consider it done.

\O/

> This leaves the ar handler.  I don't think there are many (if any) perl
> packages shipping ar files that aren't otherwise writable,

*nod*

> but I believe anybody with some perl knowledge could change the
> strip-nd ar handler to use the same temporary file structure uesd by
> the other handlers as well.

If that combination (read-only ar archive inside a package) ever pops
up, I can have a look.

		Regards, Axel
-- 
 ,''`.  |  Axel Beckert <abe at debian.org>, https://people.debian.org/~abe/
: :' :  |  Debian Developer, ftp.ch.debian.org Admin
`. `'   |  4096R: 2517 B724 C5F6 CA99 5329  6E61 2FF9 CD59 6126 16B5
  `-    |  1024D: F067 EA27 26B9 C3FC 1486  202E C09E 1D89 9593 0EDE