[Letsencrypt-devel] Bug#810720: libdpkg-perl: check_patches_applied does not apply patches if the first patch emits a warning

[Apollon Oikonomopoulos]

Control: tags -1 - moreinfo

Hi Guillem,

On 21:22 Tue 12 Jan     , Guillem Jover wrote:
> Control: tags -1 unreproducible moreinfo
> 
> On Mon, 2016-01-11 at 17:07:43 +0200, Apollon Oikonomopoulos wrote:
> > Package: libdpkg-perl
> > Version: 1.18.4
> > Severity: normal
> 
> > The quilt package format's check_patches_applied() silently fails to 
> > apply patches if the first patch in the series emits a warning. An 
> > example is the current version of python-letsencrypt (0.1.1-3), whose 
> > first patch in the series patches a file twice - thus emitting a 
> > warning:
> > 
> >  tmp/letsencrypt-0.1.1 $ dpkg-source --before-build .
> >  dpkg-source: warning: diff 'letsencrypt-0.1.1/debian/patches/permission-failures.patch' patches file letsencrypt-0.1.1/letsencrypt/plugins/webroot_test.py twice
> >  dpkg-source: warning: diff 'letsencrypt-0.1.1/debian/patches/permission-failures.patch' patches file letsencrypt-0.1.1/letsencrypt/plugins/webroot_test.py twice
> >  /tmp/letsencrypt-0.1.1 $ 
> > 
> > Re-ordering the patches in the series file leads to the whole series 
> > being applied normally. Since patching the same file twice is a warning 
> > and not an error, it should not stop dpkg-source --before-build from 
> > applying the series.
> 
> Actually it's patch itself which fails, because in dry-mode it does not
> keep track of the previous patches that want to modify the same file and
> does not record the changes w/o actually applying the them. The following
> command shows the problem:

You're right! I was misled by the fact that `quilt push` could apply the 
patch successfully. But yes, a patch patching the same file 
incrementally can obviously not be applied in dry-run mode.

> 
> I guess the only thing I can do here is error out whenever the first diff
> patches a file multiple times.

Part of my confusion arised from the fact that no error messages were 
shown, just a warning that is supposed to be harmless. So perhaps the 
error messages from the patch dry-run should be output to offer some 
guidance. Erroring out at this stage will also help clarify things, 
because otherwise dpkg-source -b will (likely) fail with unexpected 
upstream source changes, causing a bit of confusion.

> In any case you should fix this in the source package, either by
> merging the hunks into a single patch (what's the point of a patch
> series correcting the same previously introduced changes anyway? :),
> or by splitting the different patches into independent files. Or
> rearranging the patch as you mentioned.

I agree. I'm Cc'ing the package maintainers, since I'm not directly 
involved with the package myself.

Thanks,
Apollon