#802284 git-buildpackage: should assemble overlay before issuing 'debian/rules' commands

#802284#5
Date:
2015-10-19 01:51:08 UTC
From:
To:
When building a package in “overlay” mode, the upstream source is by
definition not in the VCS working tree until the overlay is assembled.

So commands such as ‘debian/rules clean’ will not work, if issued
before the overlay is assembled:

    $ gbp config buildpackage.overlay
    buildpackage.overlay=true

    $ ls -1F
    debian/

    $ gbp buildpackage --source
    dh clean --with python3 --buildsystem=pybuild
       dh_testdir -O--buildsystem=pybuild
       debian/rules override_dh_auto_clean
    make[1]: Entering directory '/home/bignose/Projects/debian/python-adventure/trunk'
    dh_auto_clean
    E: pybuild pybuild:88: cannot detect build system, please use --system option or set PYBUILD_SYSTEM env. variable
    dh_auto_clean: pybuild --clean -i python{version} -p 3.4 --dir . returned exit code 11
    debian/rules:97: recipe for target 'override_dh_auto_clean' failed
    […]

The ‘gbp buildpackage’ command, for a package in “overlay” mode,
should not assume any of the ‘debian/rules’ commands will work in the
absence of the upstream source. That includes ‘clean’.

Instead, before issuing the ‘debian/rules clean’ command, the overlay
needs to be assembled: get the upstream source for the package, then
overlay the Debian packaging files onto it. Only after that can
‘debian/rules clean’ be expected to work in the exported tree.

#802284#10
Date:
2015-10-19 06:50:49 UTC
From:
To:
Hi,

I somehow agree here although I don't think the clean command makes much
sense in the overlay case so just setting the cleaner to /bin/true works
around this and maybe we should just skip it in overlay mode altogether?
Cheers,
 -- Guido

#802284#15
Date:
2015-10-19 07:27:34 UTC
From:
To:
For comparison, the ‘bzr-buildpackage’ and ‘svn-buildpackage’ tools
both construct the overlay (they call it “merge with upstream”), and
then call ‘debian/rules clean’.

I think ‘bzr-buildpackage’, at least, uses ‘dpkg-buildpackage’ and
‘dpkg-source’, which call ‘debian/rules clean’ on the resulting source
tree.

    $ bzr-buildpackage --source
    Building using working tree
    Building package in merge mode
    Looking for a way to retrieve the upstream tarball
    Upstream tarball already exists in build directory, using that
    Building the package in /home/bignose/Projects/debian/python-adventure/bzr/build-area/python-adventure-1.3, using debuild -S
     dpkg-buildpackage -rfakeroot -d -us -uc -S
    dpkg-buildpackage: source package python-adventure
    dpkg-buildpackage: source version 1.3-1
    dpkg-buildpackage: source distribution UNRELEASED
    dpkg-buildpackage: source changed by Ben Finney <ben+debian@benfinney.id.au>
     dpkg-source --before-build python-adventure-1.3
     fakeroot debian/rules clean
    dh clean --with python3 --buildsystem=pybuild
       dh_testdir -O--buildsystem=pybuild
       debian/rules override_dh_auto_clean
    make[1]: Entering directory '/home/bignose/Projects/debian/python-adventure/bzr/build-area/python-adventure-1.3'
    dh_auto_clean
    […]

This is done so that the ‘debian/rules clean’ target can perform any
cleaning of the source tree needed before build. This ensures the
build is repeatable.

I would expect ‘git-buildpackage’ should as closely as feasible mirror
the overlay behaviour of the original ‘svn-buildpackage’, and later
‘bzr-buildpackage’ and ‘hg-buildpackage’.

That includes, AFAICT, constructing the overlaid source tree, then
calling ‘debian/rules clean’ (maybe using ‘dpkg-source’ to do it)
inside the resulting source tree, before continuing the build.

#802284#20
Date:
2015-10-20 06:14:30 UTC
From:
To:
Hi,

I'd be happy to apply a patch to achieve this behaviour. Given that I'm
not a overlay user myself and currently stall on other things
already. A testcase using e.g. tests/component/deb/test_buildpackage.py
to make sure it doesn't get broken again would be welcome too.
Cheers,
 -- Guido

#802284#25
Date:
2016-01-08 14:08:47 UTC
From:
To:
Version: git-buildpackage/0.7.1

This was fixed in 0.7.1 but not closed via the changelog.

Thanks again for the patch!
 -- Guido

#802284#30
Date:
2016-01-12 06:38:47 UTC
From:
To:
Control: reopen -1
Control: found -1  git-buildpackage/0.7.1

I think this bug report was closed in error; the message that closes
this report does not seem to address the bug.

Bug#802284 is not about ‘buildpackage-rpm’. As far as I know, there's
no patch submitted to the bug report.

I've confirmed the reported behaviour is still observed in
“git-buildpackage/0.7.1” as originally reported.

I think bug#802284 still needs to be addressed, and so I'm re-opening
the report.

#802284#39
Date:
2016-01-12 07:28:18 UTC
From:
To:
Yept, the bug I meant to close was 796198, sorry and thanks for spotting this.

Cheers,
 -- Guido

#802284#44
Date:
2016-02-12 08:11:36 UTC
From:
To:
Control: found -1 git-buildpackage/0.7.2

I am only a user of the ‘foo-buildpackage’ programs, I don't
understand well enough how they are expected to work internally.

This bug report describes the expected behaviour to fix this bug. I
don't think I currently have the competence with the code base to
implement that behaviour.

(Incidentally, I fetched the source for ‘git-buildpackage’ and there
are no signed tags for releases; the latest release tag I see is
“debian/0.6.9” yet the current release in Debian is “0.7.2”. Have I
fetched from the wrong repository?)

#802284#51
Date:
2016-02-12 12:50:45 UTC
From:
To:
Well you don't tell me where you fetched the source from but the one
listed in debian/control has signed tags (including 0.7.2).
Cheers,
 -- Guido

#802284#56
Date:
2016-02-17 06:59:25 UTC
From:
To:
I have implemented this behaviour and it seems to work correctly.

Please merge from my ‘issue/802284-overlay-before-rules’ branch
<URL:https://notabug.org/bignose/debian_git-buildpackage/src/issue/802284-overlay-before-rules>
if you approve of the implementation.

I was not able to figure out the test suite enough to make a test
case. I have started a discussion separately for that.

#802284#63
Date:
2017-01-22 04:48:47 UTC
From:
To:
The patch has been updated against the latest release.

Please merge from my ‘issue/802284-overlay-before-rules’ branch
<URL:https://notabug.org/bignose/debian_git-buildpackage/src/issue/802284-overlay-before-rules>
if you approve of the implementation.

#802284#68
Date:
2017-01-22 16:29:31 UTC
From:
To:
Hi Ben,

Thanks for updating this. I had a quick look and it seems your
820247ae963a37daea2b63dc067aacd2928 moves the check for uncommitted
changes into

    if not options.tag_only:

that means the test will be skipped when this options is set which is
bad.  So I think we need to do:

   * leave the code as is for !options.overlay
   * have the code as you have it for optiions.overlay

Cheers,
 -- Guido

#802284#73
Date:
2017-01-29 02:30:38 UTC
From:
To:
I couldn't find a commit with that hash. I think you probably mean
commit ‘abeec820247ae963a37daea2b63dc067aacd2928’.

In response to your feedback I have re-based the branch, so that
difference is moot :-)

Okay. I found that large function difficult to navigate; it is not
clear how to achieve the goal of that commit (“Only run the cleaner
command after exporting source tree.”) without just moving it to that
location.

To tease apart the separate actions a little, I have extracted the
statements cleaning the working tree, to a separate function. Now the
change of when to do the clean is much simpler, see
<URL:https://notabug.org/bignose/debian_git-buildpackage/commit/df80f5e5efc1ec54694338b158983da9f9180c46>.

How would you like that commit to change so that the clean happens
when it should?

#802284#78
Date:
2017-02-02 14:59:42 UTC
From:
To:
Moving the cleaning/checking part out of main is exactly what I had in
mind (and so I already pulled that change in). If you now leave the
check where it is for for all cases except *options.overlay and not
options.tag_only) and have the check executed where you have it now for
overlay-mode we have everything covered. If you supply the testcases for
overlay mode I can handle the ones for tag-only so we spot and possible
breackage.

Cheers,
 -- Guido

#802284#83
Date:
2017-02-03 10:41:02 UTC
From:
To:
Thank you, I'm glad that helped.

Sorry, you've lost me, I could not follow the conditional statements
there.

For my purpose in this bug report, the clean should happen only in the
exported tree.

I'm not clear on what other conditional branches you have in mind. Can
you show pseudocode of what you mean?

#802284#88
Date:
2017-08-08 23:38:50 UTC
From:
To:
Ping. I'm still needing to understand your most recent message, can
you help explain?

Sorry, you've lost me, I could not follow the conditional statements
there.

For my purpose in this bug report, the clean should happen only in the
exported tree.

I'm not clear on what other conditional branches you have in mind. Can
you show pseudocode of what you mean?

#802284#93
Date:
2017-08-11 14:01:38 UTC
From:
To:
Hi,

I didn't get around to have a closer look but what you can do right now
is call the clean command as prebuild hook since it gets the build_dir
passed in as GBP_BUILD_DIR and is run right before the build.

(I'll have a closer look how to reshuffle things after fixing 2 other
bugs I'm currently on, sorry to keep you waiting).
Cheers,
 -- Guido

#802284#98
Date:
2017-08-14 01:36:24 UTC
From:
To:
Control: submitter -1 !
Control: found -1 git-buildpackage/0.8.18
from issuing a clean *before* building the source? Is there some
existing hook that I should disable?

As best I can tell, there is no request for “clean” prior to exporting
the source. Yet it happens in response to a request to build the
source:

=====
$ gbp --version
gbp 0.8.18

$ gbp buildpackage --git-overlay --git-cleaner=/bin/true
gbp:info: All Orig tarballs 'pelican_3.7.1.orig.tar.gz' found at '../tarballs'
gbp:info: Building with (cowbuilder) for sid
gbp:info: Extracting pelican_3.7.1.orig.tar.gz to '/home/bignose/Projects/debian/pelican/build-area/pelican-tmp'
gbp:info: Exporting 'WC' to '/home/bignose/Projects/debian/pelican/build-area/pelican-tmp'
gbp:info: Moving '/home/bignose/Projects/debian/pelican/build-area/pelican-tmp' to '/home/bignose/Projects/debian/pelican/build-area/pelican-3.7.1'
Building with cowbuilder for distribution sid
I: using cowbuilder as pbuilder
dh clean --with python2,sphinxdoc  --buildsystem=pybuild
   dh_auto_clean -O--buildsystem=pybuild
I: pybuild base:184: python2.7 setup.py clean
running clean
removing '/home/bignose/Projects/debian/pelican/build-area/pelican-3.7.1/.pybuild/pythonX.Y_2.7/build' (and everything under it)
'build/bdist.linux-x86_64' does not exist -- can't clean it
'build/scripts-2.7' does not exist -- can't clean it
   debian/rules override_dh_clean
make[1]: Entering directory '/home/bignose/Projects/debian/pelican/build-area/pelican-3.7.1'
dh_clean
rm -rf pelican.egg-info build/html
make[1]: Leaving directory '/home/bignose/Projects/debian/pelican/build-area/pelican-3.7.1'
dpkg-source: info: using source format '3.0 (quilt)'
dpkg-source: error: unwanted binary file: debian/theme-substitute/notmyidea/static/images/forkme_left_white_ffffff.png
dpkg-source: error: unwanted binary file: debian/theme-substitute/notmyidea/static/images/forkme_right_red_aa0000.png
dpkg-source: error: detected 2 unwanted binary files (add them in debian/source/include-binaries to allow their inclusion).
gbp:error: 'git-pbuilder' failed: it exited with 29
=====

So the problem is that a “clean” is occurring implicitly before
building the source, even when I specify ‘--git-cleaner=/bin/true’.

Thank you for working on this!

#802284#107
Date:
2017-08-14 01:58:33 UTC
From:
To:
Control: submitter -1 !

My apologies, that does not demonstrate anything to do with this bug
(it's an error from unwanted files in the source directory). Ignore
that session output.

I will try your suggested workaround.

#802284#114
Date:
2017-08-14 02:11:34 UTC
From:
To:
Hi Ben,

If you check with --git-verbose …

… one can see that this is already within cowbuilder …

… so that's not a issue of gbp itself, it's cowbuilder triggering a
clean in the current directory. You can work around this by passing
"-nc":

    $ gbp buildpackage --git-overlay --git-cleaner=/bin/true -nc

(unfortunately cowbuilder does not understand pbuilder's
--pdebuild-internal).

This is not perfect since the -nc also affects the build _inside_ the
chroot. I'm cc'ing the cowbuilder maintainers since maybe they have
better idea. What we'd want to have for cowbuilder is:

    - do no clean outside the chroot (since we know we're clean when
      using --export-dir and --overlay)
    - but do as before inside the chroot

I couldn't find a bug on that. cowbuilder maintainers, does this sound
sane? People (including me) seem to get confused by this form time to
time and it would be great if we could find a solution. What about
adding a '--no-initial-clean'?

I would not call this "working on it" yet but at least I've shuffled
around some code there recently to make this simpler.

Thanks for keeping this alive!
Cheers,
 -- Guido

#802284#119
Date:
2017-11-19 13:23:52 UTC
From:
To:
Hi Ben,
dug this one out again since I really would like to see this adressed
and I'm still unsure if we really need to do something:


So this looks like bzr-buildpackage is calling

   debuild -S

after assembling the overlay. Wouldn't that be equivalent of calling

   gbp buildpackage --git-postexport="debuild -S" …

If this works we could update the documentation (which at the moment
doesn't talk about --git-overlay at all).

This should be achievable by the above. I'd rather not use the
--git-cleaner for that since this command is responsible for cleaning
the git working tree.

Cheers,
 -- Guido

#802284#124
Date:
2017-12-01 03:22:15 UTC
From:
To:
Thank you for returning to this. I also want this to be addressed to
completion.

On the manual pages ‘gbp(1)’ and ‘gbp.conf(5)’ I can't see the
‘--git-postexport’ option described. Where is that documented?

#802284#129
Date:
2017-12-01 06:40:42 UTC
From:
To:
system *before* assembling the overlay:

=====
$ gbp buildpackage --git-postexport='debuild -S'
gbp:info: Building with (cowbuilder) for sid
gbp:info: Postexport hook set, delaying tarball creation
gbp:info: Exporting 'WC' to '/home/bignose/Projects/debian/xkcdpass/build-area/xkcdpass-tmp'
 dpkg-buildpackage -rfakeroot -us -uc -ui -S
dpkg-buildpackage: info: source package xkcdpass
dpkg-buildpackage: info: source version 1.14.2-1
dpkg-buildpackage: info: source distribution UNRELEASED
dpkg-buildpackage: info: source changed by Ben Finney <bignose@debian.org>
 dpkg-source --before-build xkcdpass-tmp
 fakeroot debian/rules clean
dh clean --with bash-completion,python3 --buildsystem=pybuild
   debian/rules override_dh_auto_clean
make[1]: Entering directory '/home/bignose/Projects/debian/xkcdpass/build-area/xkcdpass-tmp'
dh_auto_clean
E: pybuild pybuild:96: cannot detect build system, please use --system option or set PYBUILD_SYSTEM env. variable
dh_auto_clean: pybuild --clean -i python{version} -p 3.6 returned exit code 11
debian/rules:48: recipe for target 'override_dh_auto_clean' failed
make[1]: *** [override_dh_auto_clean] Error 25
make[1]: Leaving directory '/home/bignose/Projects/debian/xkcdpass/build-area/xkcdpass-tmp'
debian/rules:35: recipe for target 'clean' failed
make: *** [clean] Error 2
dpkg-buildpackage: error: fakeroot debian/rules clean subprocess returned exit status 2
debuild: fatal error at line 1151:
dpkg-buildpackage -rfakeroot -us -uc -ui -S failed
gbp:error: Postexport-hook 'debuild -S' failed: it exited with 29
=====

That may be what you expected to see, but I'm not sure.

#802284#134
Date:
2017-12-01 08:31:32 UTC
From:
To:
Hi,

man gbp-buildpackage has

#802284#139
Date:
2017-12-01 10:35:00 UTC
From:
To:
Hi,

This is wired and it looks different for me. However it has another
problem since the upstream tarball is not in the build-area at this
point "debuild -S" will complain. That's o.k. since the postexport hook
is exactly for messing with the upstream sources. However we also have a
prebuild hook:

    gbp --git-pbuilder --git-export-dir=../build-area  --git-no-pristine-tar --git-overlay --git-prebuild="debuild -S"

I've put all the options here _and_ removed debian/gbp.conf from your
example package. You can drop further options depending on where you
want to get the upstream tarball from.

So in summary I think the prebuild hook is what you want to use. I've
clarified hook usage in the docs a bit.

Cheers,
 -- Guido

#802284#144
Date:
2018-06-09 00:19:02 UTC
From:
To:
Control: found -1 git-buildpackage/0.9.9
Control: affects -1 src:python-coverage

My point in reporting this bug is that “hooks” should not be needed.

Git-BuildPackage should simply assemble the package source as an
overlay, *before* calling ‘debuild -S’ or ‘debian/rules clean’ or
anything else which requires the source assembled in that directory.

That shouldn't require defining a hook; it should be the default
behaviour in response to “this package source should be assembled as
an overlay” (the ‘overlay = true’ option).

#802284#153
Date:
2023-08-14 02:09:25 UTC
From:
To:
Control: found -1 git-buildpackage/0.9.32
Control: block 1045476 by -1
Control: block 1046726 by -1
Control: block 1047140 by -1
Control: block 1047269 by -1
Control: block 1047406 by -1
Control: block 1047672 by -1
Control: block 1048821 by -1
Control: summary -1 0

Git-BuildPackage should assemble the package source as an overlay (when
“overlay” is specified), *before* calling ‘debuild -S’ or ‘debian/rules
clean’ or anything else which requires the source assembled in that
directory.

For the benefit of readers of the blocked bug reports: Bug#802284 needs to
be resolved, by having Git-BuildPackage not attempt to run ‘debian/rules’
until the source working tree is assembled correctly. Currently it attempts
to run ‘debian/rules clean’ in a tree that does not yet have the upstream
source, so the build fails.

I have followed this advice in many packages as a workaround for this bug.

Now though, there are a number of bugs in those packages that are *caused*
by making the 'clean' command a no-op.

I am marking this bug as blocking resolution of those reports.

#802284#176
Date:
2023-08-14 02:21:17 UTC
From:
To:
Control: found -1 git-buildpackage/0.9.32
Control: block 1045476 by -1
Control: block 1046726 by -1
Control: block 1047140 by -1
Control: block 1047269 by -1
Control: block 1047406 by -1
Control: block 1047672 by -1
Control: block 1048821 by -1
Control: summary -1 0

Git-BuildPackage should assemble the package source as an overlay,
*before* calling ‘debuild -S’ or ‘debian/rules clean’ or anything
else which requires the source assembled in that directory.

For the benefit of readers of the blocked bug reports: Bug#802284 describes
the behaviour that Git-BuildPackage is incorrectly attempting to run
‘debian/rules clean’ before it has assembled a source working tree for
building.

In many packages I have followed this advice while waiting for this bug to
be fixed. It has worked around the issue until now.

However, there are now several bugs reported that are *caused* by setting
the ‘cleaner’ to a no-op. I am marking this bug as blocking resolution of
those bugs.