Bug#1011150: git-buildpackage: suggestions improving the packaging

[Nicolas Boulenguez]

Package: git-buildpackage
Version: 0.9.22
Severity: wishlist
Tags: patch

Hello.
The attached commits may slightly help the maintainance of the Debian
packaging.
Please apply the ones you agree with.
Thanks.

[Guido Günther]

Hi Nicolas,
I've applied most of these and skipped some. I'm not totally opposed,
it's more that I like the current way a bit more.

> Subject: [PATCH 1/8] debhelper: make package prefix explicit in configuration
applied

> Subject: [PATCH 2/8] debian/rules: simplify installation of zsh and pk4
applied

> Subject: [PATCH 3/8] rules: simplify thanks to debhelper compat 13
applied

> Subject: [PATCH 4/8] debian/rules: remove obsolete and unused Make variables
applied

> Subject: [PATCH 5/8] debhelper: generate some lists in configuration files
i've skipped this one. I'm fine with using more globs but rather not use
a script as it might defeat dh_listmissing.

> Subject: [PATCH 6/8] debian/clean: add forgotten files
this one doesn't apply

> Subject: [PATCH 7/8] Makefile: spare unneeded recursive Make invokations
I skipped that one just because I think being a bit more explicit is
o.k. here.

> Subject: [PATCH 8/8] debian/copyrigt: switch to machine-readable format 1.0
applied as well.

Thanks a lot.
-- Guido

[Nicolas Boulenguez]

First, thanks for welcoming suggestions and taking the time to explain
your concerns.
I have rebased the remaining suggestions, split them in smaller
commits, and adapted some.

1 is a matter of taste.

2 should be consensual, but may need a rebase if 1 is not

> > Subject: [PATCH 5/8] debhelper: generate some lists in configuration files
> i've skipped this one. I'm fine with using more globs but rather not use
> a script as it might defeat dh_listmissing.

3 should be consensual

4
I think that the solution I have selected prevents such problems.
However, I fully agree with your concern so I have made the intention
explicit in comments near the code.
If my reasoning does not convince you, let’s forget my suggestion and
keep a hand-written list.

> > Subject: [PATCH 7/8] Makefile: spare unneeded recursive Make invokations
> I skipped that one just because I think being a bit more explicit is
> o.k. here.

5 fixes a bug

6 is more explicit in my opinion because debian/rules explicitly
contains the word 'export', but you decide.

[Guido Günther]

Hi,

On Thu, May 26, 2022 at 02:11:21AM +0200, Nicolas Boulenguez wrote:
> First, thanks for welcoming suggestions and taking the time to explain
> your concerns.
> I have rebased the remaining suggestions, split them in smaller
> commits, and adapted some.
>
> 1 is a matter of taste.

I've applied that one.

>
> 2 should be consensual, but may need a rebase if 1 is not

I've applied that one too.

>
> > > Subject: [PATCH 5/8] debhelper: generate some lists in configuration files
> > i've skipped this one. I'm fine with using more globs but rather not use
> > a script as it might defeat dh_listmissing.
>
> 3 should be consensual

I've applied that one too.

>
> 4
> I think that the solution I have selected prevents such problems.
> However, I fully agree with your concern so I have made the intention
> explicit in comments near the code.
> If my reasoning does not convince you, let’s forget my suggestion and
> keep a hand-written list.

I'd prefer that at the moment, yes.

> > > Subject: [PATCH 7/8] Makefile: spare unneeded recursive Make invokations
> > I skipped that one just because I think being a bit more explicit is
> > o.k. here.
>
> 5 fixes a bug

I've applied that one too.

> 6 is more explicit in my opinion because debian/rules explicitly
> contains the word 'export', but you decide.

I'm on the fence for this one ;)

Thanks a log for your patches!
-- Guido

> From f5bafea563ed3e1a7bc8b8b19b02007b13cf487e Mon Sep 17 00:00:00 2001
> From: Nicolas Boulenguez <nic...@debian.org>
> Date: Thu, 26 May 2022 00:47:22 +0200
> Subject: [PATCH 1/6] Move some cleaning from debian/rules to debian/clean
>
> ---
> debian/clean | 2 ++
> debian/rules | 2 --
> 2 files changed, 2 insertions(+), 2 deletions(-)
> create mode 100644 debian/clean
>
> diff --git a/debian/clean b/debian/clean
> new file mode 100644
> index 00000000..0ac9e25f
> --- /dev/null
> +++ b/debian/clean
> @@ -0,0 +1,2 @@
> +build/
> +gbp/version.py
> diff --git a/debian/rules b/debian/rules
> index 0802452d..ad61ba7a 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -13,6 +13,4 @@ execute_after_dh_fixperms:
> chmod a+x debian/git-buildpackage/usr/lib/python3/dist-packages/gbp/scripts/supercommand.py
>
> execute_after_dh_auto_clean:
> - rm -rf build/
> make -C docs/ clean
> - -rm gbp/version.py
> --
> 2.30.2
>

> From 73dfa8e10bfc46a987906648285083787968e912 Mon Sep 17 00:00:00 2001
> From: Nicolas Boulenguez <nic...@debian.org>
> Date: Thu, 26 May 2022 00:49:25 +0200
> Subject: [PATCH 2/6] debian/clean: add forgotten files
>
> ---
> debian/clean | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/debian/clean b/debian/clean
> index 0ac9e25f..026cf3bf 100644
> --- a/debian/clean
> +++ b/debian/clean
> @@ -1,2 +1,5 @@
> build/
> +coverage.xml
> +gbp.egg-info/
> gbp/version.py
> +nosetests.xml
> --
> 2.30.2
>

> From cc6df249062c7604df8f175ecb524bcffe97a2db Mon Sep 17 00:00:00 2001
> From: Nicolas Boulenguez <nic...@debian.org>
> Date: Thu, 26 May 2022 01:44:07 +0200
> Subject: [PATCH 3/6] debian/rules: clean __pycache__ directories generated
> during the build
>
> ---
> debian/rules | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/debian/rules b/debian/rules
> index ad61ba7a..a20be4bb 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -14,3 +14,4 @@ execute_after_dh_fixperms:
>
> execute_after_dh_auto_clean:
> make -C docs/ clean
> + find gbp -name __init__.py -printf '%h/__pycache__/\n' | xargs rm -fr
> --
> 2.30.2
>

> From 2b3e0c9c61b63c59db02c9bf917286a6bcac63a4 Mon Sep 17 00:00:00 2001
> From: Nicolas Boulenguez <nic...@debian.org>
> Date: Tue, 17 May 2022 15:27:41 +0200
> Subject: [PATCH 4/6] debhelper: generate some lists in configuration files
>
> ---
> debian/git-buildpackage-rpm.install | 17 ++++++++---
> debian/git-buildpackage.install | 47 ++++++++---------------------
> debian/not-installed | 15 ++++-----
> 3 files changed, 32 insertions(+), 47 deletions(-)
> mode change 100644 => 100755 debian/git-buildpackage-rpm.install
> mode change 100644 => 100755 debian/git-buildpackage.install
> mode change 100644 => 100755 debian/not-installed
>
> diff --git a/debian/git-buildpackage-rpm.install b/debian/git-buildpackage-rpm.install
> old mode 100644
> new mode 100755
> index f79d4f17..0d1c755d
> --- a/debian/git-buildpackage-rpm.install
> +++ b/debian/git-buildpackage-rpm.install
> @@ -1,6 +1,13 @@
> +#!/bin/sh
> +set -C -e -f -u
> +
> +# This script is normally executed after dh_auto_install and could
> +# just parse debian/tmp. It parses gbp/ instead so that:
> +# * it can be checked at any time by simply executing it.
> +# * it does not interfer with dh_listmissing.
> +find gbp -path '*rpm*.py' -printf \
> + 'usr/lib/python3.*/dist-packages/%p usr/lib/python3/dist-packages/%h\n'
> +
> +cat <<EOF
> +
> usr/bin/gbp-builder-mock /usr/share/git-buildpackage/
> -usr/lib/python3.*/dist-packages/gbp/rpm usr/lib/python3/dist-packages/gbp/
> -usr/lib/python3.*//dist-packages/gbp/scripts/import_srpm.py usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/scripts/pq_rpm.py usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/scripts/buildpackage_rpm.py usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/scripts/rpm_ch.py usr/lib/python3/dist-packages/gbp/scripts/
> diff --git a/debian/git-buildpackage.install b/debian/git-buildpackage.install
> old mode 100644
> new mode 100755
> index 956ac8c3..65a44bbb
> --- a/debian/git-buildpackage.install
> +++ b/debian/git-buildpackage.install
> @@ -1,41 +1,18 @@
> +#!/bin/sh
> +set -C -e -f -u
> +
> +# This script is normally executed after dh_auto_install and could
> +# just parse debian/tmp. It parses gbp/ instead so that:
> +# * it can be checked at any time by simply executing it.
> +# * it does not interfer with dh_listmissing.
> +find gbp -name '*.py' -a ! -path '*rpm*' -printf \
> + 'usr/lib/python3.*/dist-packages/%p usr/lib/python3/dist-packages/%h\n'
> +
> +cat <<EOF
> +
> debian/zsh/_gbp usr/share/zsh/vendor-completions
> debian/pk4/gbp usr/share/pk4/hooks-available/unpack
> usr/bin/gbp
> usr/bin/git-pbuilder
> usr/lib/python3.*/dist-packages/gbp-* usr/lib/python3/dist-packages/
> -usr/lib/python3.*/dist-packages/gbp/command_wrappers.py usr/lib/python3/dist-packages/gbp/
> -usr/lib/python3.*/dist-packages/gbp/config.py usr/lib/python3/dist-packages/gbp/
> -usr/lib/python3.*/dist-packages/gbp/dch.py usr/lib/python3/dist-packages/gbp/
> -usr/lib/python3.*/dist-packages/gbp/deb/ usr/lib/python3/dist-packages/gbp/
> -usr/lib/python3.*/dist-packages/gbp/errors.py usr/lib/python3/dist-packages/gbp/
> -usr/lib/python3.*/dist-packages/gbp/format.py usr/lib/python3/dist-packages/gbp/
> -usr/lib/python3.*/dist-packages/gbp/git/ usr/lib/python3/dist-packages/gbp/
> -usr/lib/python3.*/dist-packages/gbp/__init__.py usr/lib/python3/dist-packages/gbp/
> -usr/lib/python3.*/dist-packages/gbp/log.py usr/lib/python3/dist-packages/gbp/
> -usr/lib/python3.*/dist-packages/gbp/notifications.py usr/lib/python3/dist-packages/gbp/
> -usr/lib/python3.*/dist-packages/gbp/paths.py usr/lib/python3/dist-packages/gbp/
> -usr/lib/python3.*/dist-packages/gbp/patch_series.py usr/lib/python3/dist-packages/gbp/
> -usr/lib/python3.*/dist-packages/gbp/pkg/ usr/lib/python3/dist-packages/gbp/
> -usr/lib/python3.*/dist-packages/gbp/scripts/buildpackage.py usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/scripts/clone.py usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/scripts/common/ usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/scripts/config.py usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/scripts/create_remote_repo.py usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/scripts/dch.py usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/scripts/import_dsc.py usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/scripts/import_dscs.py usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/scripts/import_orig.py usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/scripts/import_ref.py usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/scripts/export_orig.py usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/scripts/__init__.py usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/scripts/pq.py usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/scripts/pristine_tar.py usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/scripts/pull.py usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/scripts/push.py usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/scripts/setup_gitattributes.py usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/scripts/supercommand.py usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/scripts/tag.py usr/lib/python3/dist-packages/gbp/scripts/
> -usr/lib/python3.*/dist-packages/gbp/tmpfile.py usr/lib/python3/dist-packages/gbp/
> -usr/lib/python3.*/dist-packages/gbp/tristate.py usr/lib/python3/dist-packages/gbp/
> -usr/lib/python3.*/dist-packages/gbp/version.py usr/lib/python3/dist-packages/gbp/
> usr/share/git-buildpackage/gbp.conf etc/git-buildpackage/
> diff --git a/debian/not-installed b/debian/not-installed
> old mode 100644
> new mode 100755
> index 0a7a6e06..0af2a8fd
> --- a/debian/not-installed
> +++ b/debian/not-installed
> @@ -1,8 +1,9 @@
> -debian/tmp/usr/lib/python3.*/dist-packages/gbp/deb/__pycache__
> -debian/tmp/usr/lib/python3.*/dist-packages/gbp/git/__pycache__
> -debian/tmp/usr/lib/python3.*/dist-packages/gbp/pkg/__pycache__
> -debian/tmp/usr/lib/python3.*/dist-packages/gbp/__pycache__
> -debian/tmp/usr/lib/python3.*/dist-packages/gbp/rpm/__pycache__
> -debian/tmp/usr/lib/python3.*/dist-packages/gbp/scripts/common/__pycache__
> -debian/tmp/usr/lib/python3.*/dist-packages/gbp/scripts/__pycache__
> +#!/bin/sh
> +set -C -e -f -u
>
> +# This script is normally executed after dh_auto_install and could
> +# just parse debian/tmp. It parses gbp/ instead so that:
> +# * it can be checked at any time by simply executing it.
> +# * it does not blindly silent dh_listmissing.
> +find gbp -name __init__.py -printf \
> + 'usr/lib/python3.*/dist-packages/%h/__pycache__\n'
> --
> 2.30.2
>

> From e5eadf75333b17a7aba226ebafdfdca28a5e34a1 Mon Sep 17 00:00:00 2001
> From: Nicolas Boulenguez <nic...@debian.org>
> Date: Thu, 26 May 2022 01:53:54 +0200
> Subject: [PATCH 5/6] debian/rules: really export GBP_NETWORK_TESTS to tests
>
> $(MAKE) GBP_NETWORK_TESTS=1
> was setting a Make variable in the sub-Make, but not exporting it for
> test/*.py subprocesses.
> ---
> Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index db5ad88e..6134edab 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -6,7 +6,7 @@ TEST_LOCALE?=C.UTF-8
> all: syntax-check test
>
> all+net:
> - $(MAKE) GBP_NETWORK_TESTS=1 all
> + GBP_NETWORK_TESTS=1 $(MAKE) all
>
> test:
> export HOME=/nonexisting; \
> --
> 2.30.2
>

> From 1b41bebd6543cd7a46596fc79da21590c8399ffc Mon Sep 17 00:00:00 2001
> From: Nicolas Boulenguez <nic...@debian.org>
> Date: Tue, 17 May 2022 15:30:22 +0200
> Subject: [PATCH 6/6] Makefile: spare unneeded recursive Make invokations
>
> ---
> Makefile | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 6134edab..edf39af6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -5,8 +5,10 @@ TEST_LOCALE?=C.UTF-8
>
> all: syntax-check test
>
> -all+net:
> - GBP_NETWORK_TESTS=1 $(MAKE) all
> +# This means that 'make all+net' is a convenient shortcut for
> +# 'GBP_NETWORK_TESTS=1 make all'.
> +all+net: export GBP_NETWORK_TESTS := 1
> +all+net: all
>
> test:
> export HOME=/nonexisting; \
> @@ -22,9 +24,8 @@ syntax-check:
> flake8 $(FLAKE_OPTS)
> flake8 $(FLAKE_OPTS) $(PY_EXAMPLES)
>
> -docs:
> +docs: apidocs
> $(MAKE) -C docs
> - $(MAKE) apidocs
>
> apidocs:
> mkdir -p build
> --
> 2.30.2
>