Hi,
On Fri, Sep 21, 2018 at 03:07:19PM +0200, Andreas Beckmann wrote:
> On 2018-09-21 13:44, Guido Günther wrote:
> >> In 'features' I've implemented two things:
> >> * %(version)s substitution for tarball-dir
> >> * preexport hooks that run before the tarballs get selected from the
> >> tarball-dir (the hook can so some magic to bring the tarballs into
> >> existence)
> >> These are at least proof-of-concept implementations, they may lack
> >> proper documentation etc.
>
> > The preexport and version substitution stuff looks good as well and I'm
> > happy to pull it in if it gets tests and doc updates (as you
> > suggested).
>
> I can write documentation if you point me to the correct files ...
Docs:
docs/manpages/gbp-buildpackage.xml
If you want to add more details see
docs/chapters/building.xml
Docs can be built with
make -C docs
if you have gbp's build-deps installed.
> but I'm not sure how to write proper tests for these things.
For the preexport hook just extend
https://git.sigxcpu.org/cgit/git-buildpackage/tree/tests/component/deb/test_buildpackage.py#n53
For the version replacement a unit test of the path replacement function
will already do most of the work. Since tarball-dir is also used for
exporting tarballs this could be extended
https://git.sigxcpu.org/cgit/git-buildpackage/tree/tests/component/deb/test_buildpackage.py#n194
with a test that uses the --tarball-dir='../foo-%(version)s'.
> > Only thing I'm unsure is if we really want version_to_tag here or just
> > replace the upstream version verbatim. With version_to_tag the
> > directories would need to be named like the debian tag instead of the
> > upsteam version. Was that your intend or was it just simpler to do?
> > Cheers,
>
> Yes, this was just a hack - it was just the easiest interface I could
> find to do the substitution ... and while I only mentioned %(version)s,
> it should probably support the other variants as well, to not cause
> confusion. Maybe repo should have a generic method to substitute the
> %(version)s family of placeholders in an arbitrary string, that could be
> used for both cases?
>
> repo.a_better_name_for_substitute_version_placeholder(string, version)
Since in this case this is not a property of the DebianGitRepository
(which deals with the Debian specific aspects of a git repo like
mangling versions so they're representable in git) it can just be a
patsubst function "somewhere". If we wouldn't need it in export-orig as
well having it in buildpackage.py would be fine but in this case I'd add
it as a static method to gbp/pkg/pkgpolicy.py.
Although this bug deals with two things I'm happy to pull them in
independently.
Cheers,
-- Guido
