Bug#852264: gbp buildpackage: doesn't pass options correctly anymore

[Guido Günther]

Hi James,
On Mon, Jan 23, 2017 at 09:51:06PM +0000, James Clarke wrote:
> Control: tags -1 pending
> (I see you already reassigned)
> 
> On 23 Jan 2017, at 20:21, Guido Günther <agx at sigxcpu.org> wrote:
> > control: tags -1 -unreproducible
> > control: gbp buildpackage quotes arguments twice with GIT_PBUILDER_AUTOCONF=no
> > 
> > On Mon, Jan 23, 2017 at 11:26:44AM +0100, Raphaël Halimi wrote:
> >> Le 23/01/2017 à 08:29, Guido Günther a écrit :
> >>>> A couples of lines above, I can see:
> >>>> 
> >>>> I: Generating source changes file for original dsc
> >>>> dpkg-genchanges: error: unknown option ''-v0.9-1''
> >>> 
> >>> I'm not seeing double quotes here. We changed quoting in 80a1c39
> >>> (0.8.10) so there might be a bug but I can't reproduce this with
> >>> pbuilder 0.227 and 0.228.1.
> >> 
> >> Sorry if I wasn't clear. Those are not double quotes, but two pairs of
> >> single quotes.
> > 
> > I meant doubled quotes - sorry for the confusion ;)
> > 
> > [..snip..]
> >> raph at arche:~/Divers/dev/debian/mine/official/tlp$ DIST=jessie gbp buildpackage -v0.9-1
> >> Building with pbuilder
> >> I: Distribution set to jessie (environment variable)
> >> I: using pbuilder as pbuilder
> >> dpkg-checkbuilddeps: error: Unmet build dependencies: dh-systemd
> >> W: Unmet build-dependency in source
> >> dh_testdir
> >> dh_testroot
> >> # add here commands to clean up after the build process.
> >> /usr/bin/make clean
> >> make[1]: Entering directory '/home/raph/Divers/dev/debian/mine/official/tlp'
> >> rm -f tlp tlp-functions tlp-nop tlp-rdw-nm tlp-rdw.rules tlp-rdw-udev tlp-rf tlp.rules tlp-run-on tlp.service tlp-sleep.service tlp-stat tlp.upstart tlp-usb-udev
> >> make[1]: Leaving directory '/home/raph/Divers/dev/debian/mine/official/tlp'
> >> dh_clean
> >> dpkg-source: info: using source format '3.0 (quilt)'
> >> dpkg-source: info: building tlp using existing ./tlp_0.9.orig.tar.gz
> >> dpkg-source: info: building tlp in tlp_0.9-2~bpo8+1.debian.tar.xz
> >> dpkg-source: info: building tlp in tlp_0.9-2~bpo8+1.dsc
> >> I: Generating source changes file for original dsc
> >> dpkg-genchanges: error: unknown option ''-v0.9-1''    
> >> 
> >> Use --help for program usage information.
> >> gbp:error: 'BUILDER=pbuilder GIT_PBUILDER_AUTOCONF=no /usr/bin/git-pbuilder -v0.9-1' failed: it exited with 2
> >> ----->%-----
> > 
> > That's the difference. You're using 'GIT_PBUILDER_AUTOCONF=no' (which
> > then used pbuilder instead of cowbuilder):
> > 
> > With GIT_PBUILDER_AUTOCONF=no git-pbuilder invokes pdebuild like
> > 
> >    pdebuild --pbuilder cowbuilder --debbuildopts ' '\''-v1.0'\''' -- --hookdir /home/agx/.pbuilder/hooks
> > 
> > and without GIT_PBUILDER_AUTOCONF=no:
> > 
> >    pdebuild --buildresult ../ --pbuilder cowbuilder --debbuildopts ' '\''-v1.0'\''' -- --hookdir /home/agx/.pbuilder/hooks --basepath /var/cache/pbuilder/tmpbuild//base-sid.cow
> > 
> > The quoting looks indentical to me so it seems cowbuilder is happy with
> > the quoting while pbuilder isn't. I'm cc'ing the the pbuilder
> > maintainers for their input since I think pbuilder should accept the
> > input (and it did in 0.227). Note that unqoted doesn't work either:
> > 
> >    pdebuild --pbuilder cowbuilder --debbuildopts -v1.0 -- --hookdir /home/agx/.pbuilder/hooks
> > 
> > which I think it should. Raphaël, could you check if downgrading
> > pbuilder 0.227 works for you too.
> 
> This was a bug introduced in pbuilder 0.228.1. The key thing is that the
> GIT_PBUILDER_AUTOCONF=no branch does not specify a BUILDRESULT, so it defaults
> to /var/cache/pbuilder/result. To support backwards compatibility, this ends up
> generating *two* _source.changes:
> 
> 1. *Before* the build in the chroot, using the .dsc generated by dpkg-source.
> This is placed in ../ alongside the .dsc. Note that in the normal case,
> BUILDRESULT is *also* ../, and so pdebuild skips generating this
> _source.changes file, since the .dsc will be replaced by 2.
> 
> 2. *After* the build in the chroot, if SOURCE_ONLY_CHANGES=yes. This is copied
> back to BUILDRESULT.
> 
> The second one was fine, but you're hitting the first case. Unfortunately the
> two cases were expanding the variables differently; 1. ended up doing less
> expansion than 2., so you end up with single quotes not being removed. This has
> been fixed in git by [1].
> 
> (In case you're wondering, the single quotes get stripped correctly, but when
> building up the list of arguments to dpkg-genchanges, they are added around
> each, which is why both the final examples you gave have the problem.)

Thanks for the detailed explanation and the fix!
Cheers,
 -- Guido