Bug#857316: dgit: please make --build-products-dir configurable
Hello Ian,
On Mon 23 Jul 2018 at 11:16AM +0100, Ian Jackson wrote:
> Why does the setting of $buildproductsdir need to be done in
> build_or_push_prep_early, before pushing or notpushing ? I guess it
> wouldn't make sense to have different settings, but the business with
> localising $access_forpush is a wrinkle which is best avoided where
> possible.
>
> So I think it might be better to introduce build_or_push_prep,
> which I guess would be called in prep_push after pushing and in
> build_prep after build_prep_early.
Good, I'm happy to avoid localising $access_forpush indeed.
> Style: I normally spell:
>
> +my $bpd_from_cfg = access_cfg('build-products-dir', 'RETURN-UNDEF');
> +$buildproductsdir = $bpd_from_cfg // '..';
>
> like this:
>
> +$buildproductsdir = access_cfg('build-products-dir', 'RETURN-UNDEF')
> // '..';
>
> And then,
>
> +if (!defined $buildproductsdir) {
> *$buildproductsdir = access_cfg('build-products-dir', 'RETURN-UNDEF')
> // '..';
> +}
>
> could be spelled
>
> *$buildproductsdir //= access_cfg('build-products-dir', 'RETURN-UNDEF')
> // '..';
>
> or
>
> *$buildproductsdir //= access_cfg('build-products-dir', 'RETURN-UNDEF');
> *$buildproductsdir //= '..';
I find this style obscure. But I guess it is good to be consistent with
the rest of the script. Changed.
> I don't much like the new test case because it's a whole new test case
> just for this. I think it would be better to add the configuration
> option for a cross-section of existing tests.
>
> Probably, that would be,
> * introduce a new t-buildproductsdir-config subroutine
> * add it to the top of a selection of existing tests
>
> Do you want me to do that ?
I've written the routine; it is probably best if you choose where to add
it.
--
Sean Whitton
From fe2c1726319f0702a5c715f5204e3b64d9d07509 Mon Sep 17 00:00:00 2001
From: Sean Whitton
Date: Mon, 23 Jul 2018 12:10:02 +0800
Subject: [PATCH 1/2] dgit: add dgit.default.build-products-dir git
configuration key
Signed-off-by: Sean Whitton
---
dgit | 9 -
dgit.1 | 11 +++
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/dgit b/dgit
index 357adc9..a3d3b6b 100755
--- a/dgit
+++ b/dgit
@@ -63,7 +63,7 @@ our @ropts;
our $sign = 1;
our $dryrun_level = 0;
our $changesfile;
-our $buildproductsdir = '..';
+our $buildproductsdir;
our $new_package = 0;
our $ignoredirty = 0;
our $rmonerror = 1;
@@ -4715,6 +4715,7 @@ sub prep_push () {
parseopts();
build_or_push_prep_early();
pushing();
+build_or_push_prep();
check_not_dirty();
my $specsuite;
if (@ARGV==0) {
@@ -6088,6 +6089,11 @@ sub cmd_clean () {
maybe_unapply_patches_again();
}
+sub build_or_push_prep () {
+$buildproductsdir //= access_cfg('build-products-dir', 'RETURN-UNDEF');
+$buildproductsdir //= '..';
+}
+
sub build_or_push_prep_early () {
our $build_or_push_prep_early_done //= 0;
return if $build_or_push_prep_early_done++;
@@ -6106,6 +6112,7 @@ sub build_prep_early () {
sub build_prep () {
build_prep_early();
+build_or_push_prep();
clean_tree();
build_maybe_quilt_fixup();
if ($rmchanges) {
diff --git a/dgit.1 b/dgit.1
index 1460938..ddb0c0a 100644
--- a/dgit.1
+++ b/dgit.1
@@ -842,6 +842,11 @@ regardless of this option.
Specifies where to find the built files to be uploaded.
By default, dgit looks in the parent directory
.RB ( .. ).
+
+Also see the
+.BI dgit.default.build-products-dir
+configuration option
+(which this command line option overrides).
.TP
.BI --no-rm-on-error
Do not delete the destination directory if clone fails.
@@ -1096,6 +1101,12 @@ on the dgit command line.
.LP
Settings likely to be useful for an end user include:
.TP
+.BI dgit.default.build-products-dir
+Specifies where to find the built files to be uploaded,
+when --build-products-dir is not specified. The default is
+the parent directory
+.RB ( .. ).
+.TP
.BR dgit-suite. \fIsuite\fR .distro " \fIdistro\fR"
Specifies the distro for a suite. dgit keys off the suite name (which
appears in changelogs etc.), and uses that to determine the distro
--
2.11.0
From 2551bbde4f9fed0732e10defa7e2ed2191a12775 Mon Sep 17 00:00:00 2001
From: Sean Whitton
Date: Tue, 24 Jul 2018 11:15:56 +0800
Subject: [PATCH 2/2] test suite lib: add t-buildproductsdir-config
Signed-off-by: Sean Whitton
---
tests/lib | 5 +
1 file changed, 5 insertions(+)
diff --git a/tests/lib b/tests/lib
index 99345ce..3fda1bc 100644
--- a/tests/lib
+++ b/tests/lib
@@ -1164,3 +1164,8 @@ for import in ${autoimport-gnupg}; do
;;
esac
done
+
+t-buildproductsdir-config () {
+ # use --local, not t-git-config, because the value is relative
+ git config --local dgit.default.build-products-dir ../bpd
+}
--
2.11.0
signature.asc
Description: PGP signature
Bug#857316: dgit: please make --build-products-dir configurable
Sean Whitton writes ("Bug#857316: dgit: please make --build-products-dir
configurable"):
> On Thu 09 Mar 2017 at 11:58PM +0100, Mattia Rizzolo wrote:
> > I build my packages with pbuilder, and it is set up to put the results
> > in ~/pbuilder/results/$distribution/$architecture/. I'd like dgit to go
> > look for the .changes to upload there, and I currently accomplish that
> > by passing `--build-products-dir ~/pbuilder/result/sid/amd64` to
> > `dgit push`. I'd like to configure that statically, so I don't have to
> > keep specifying it.
>
> Here are patches implementing this.
Thanks.
Why does the setting of $buildproductsdir need to be done in
build_or_push_prep_early, before pushing or notpushing ? I guess it
wouldn't make sense to have different settings, but the business with
localising $access_forpush is a wrinkle which is best avoided where
possible.
So I think it might be better to introduce build_or_push_prep,
which I guess would be called in prep_push after pushing and in
build_prep after build_prep_early.
Style: I normally spell:
+my $bpd_from_cfg = access_cfg('build-products-dir', 'RETURN-UNDEF');
+$buildproductsdir = $bpd_from_cfg // '..';
like this:
+$buildproductsdir = access_cfg('build-products-dir', 'RETURN-UNDEF')
// '..';
And then,
+if (!defined $buildproductsdir) {
*$buildproductsdir = access_cfg('build-products-dir', 'RETURN-UNDEF')
// '..';
+}
could be spelled
*$buildproductsdir //= access_cfg('build-products-dir', 'RETURN-UNDEF') //
'..';
or
*$buildproductsdir //= access_cfg('build-products-dir', 'RETURN-UNDEF');
*$buildproductsdir //= '..';
I don't much like the new test case because it's a whole new test case
just for this. I think it would be better to add the configuration
option for a cross-section of existing tests.
Probably, that would be,
* introduce a new t-buildproductsdir-config subroutine
* add it to the top of a selection of existing tests
Do you want me to do that ?
Thanks,
Ian.
--
Ian JacksonThese opinions are my own.
If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.
Bug#857316: dgit: please make --build-products-dir configurable
control: tag -1 +patch
Hello,
On Thu 09 Mar 2017 at 11:58PM +0100, Mattia Rizzolo wrote:
> I build my packages with pbuilder, and it is set up to put the results
> in ~/pbuilder/results/$distribution/$architecture/. I'd like dgit to go
> look for the .changes to upload there, and I currently accomplish that
> by passing `--build-products-dir ~/pbuilder/result/sid/amd64` to
> `dgit push`. I'd like to configure that statically, so I don't have to
> keep specifying it.
Here are patches implementing this.
--
Sean Whitton
From 90f2e4779ce807671eef958a6c01ed77a5ae485f Mon Sep 17 00:00:00 2001
From: Sean Whitton
Date: Mon, 23 Jul 2018 12:10:02 +0800
Subject: [PATCH 1/2] dgit: add dgit.default.build-products-dir git
configuration key
Signed-off-by: Sean Whitton
---
dgit | 7 ++-
dgit.1 | 11 +++
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/dgit b/dgit
index 357adc9..01b5499 100755
--- a/dgit
+++ b/dgit
@@ -63,7 +63,7 @@ our @ropts;
our $sign = 1;
our $dryrun_level = 0;
our $changesfile;
-our $buildproductsdir = '..';
+our $buildproductsdir;
our $new_package = 0;
our $ignoredirty = 0;
our $rmonerror = 1;
@@ -6096,6 +6096,11 @@ sub build_or_push_prep_early () {
$isuite = getfield $clogp, 'Distribution';
$package = getfield $clogp, 'Source';
$version = getfield $clogp, 'Version';
+if (!defined $buildproductsdir) {
+local $access_forpush;
+my $bpd_from_cfg = access_cfg('build-products-dir', 'RETURN-UNDEF');
+$buildproductsdir = $bpd_from_cfg // '..';
+}
}
sub build_prep_early () {
diff --git a/dgit.1 b/dgit.1
index 1460938..ddb0c0a 100644
--- a/dgit.1
+++ b/dgit.1
@@ -842,6 +842,11 @@ regardless of this option.
Specifies where to find the built files to be uploaded.
By default, dgit looks in the parent directory
.RB ( .. ).
+
+Also see the
+.BI dgit.default.build-products-dir
+configuration option
+(which this command line option overrides).
.TP
.BI --no-rm-on-error
Do not delete the destination directory if clone fails.
@@ -1096,6 +1101,12 @@ on the dgit command line.
.LP
Settings likely to be useful for an end user include:
.TP
+.BI dgit.default.build-products-dir
+Specifies where to find the built files to be uploaded,
+when --build-products-dir is not specified. The default is
+the parent directory
+.RB ( .. ).
+.TP
.BR dgit-suite. \fIsuite\fR .distro " \fIdistro\fR"
Specifies the distro for a suite. dgit keys off the suite name (which
appears in changelogs etc.), and uses that to determine the distro
--
2.11.0
From 247c68ea4610148e89cdaaa50650a4f0c5c63583 Mon Sep 17 00:00:00 2001
From: Sean Whitton
Date: Mon, 23 Jul 2018 16:42:33 +0800
Subject: [PATCH 2/2] test suite: add push-buildproductsdir-conf
Based on push-buildproductsdir.
Signed-off-by: Sean Whitton
---
debian/tests/control | 2 +-
tests/tests/push-buildproductsdir-conf | 32
2 files changed, 33 insertions(+), 1 deletion(-)
create mode 100755 tests/tests/push-buildproductsdir-conf
diff --git a/debian/tests/control b/debian/tests/control
index dfd73bb..eb76fac 100644
--- a/debian/tests/control
+++ b/debian/tests/control
@@ -60,7 +60,7 @@ Tests: trustingpolicy-replay
Tests-Directory: tests/tests
Depends: dgit, dgit-infrastructure, devscripts, debhelper (>=8), fakeroot, build-essential, chiark-utils-bin, bc, dput-ng
-Tests: absurd-gitapply badcommit-rewrite build-modes build-modes-asplit build-modes-gbp-asplit checkout clone-clogsigpipe clone-gitnosuite clone-nogit debpolicy-dbretry debpolicy-newreject debpolicy-quilt-gbp defdistro-rpush defdistro-setup distropatches-reject dpkgsourceignores-correct drs-clone-nogit drs-push-masterupdate drs-push-rejects dsd-clone-nogit dsd-divert fetch-localgitonly fetch-somegit-notlast gbp-orig gitconfig gitworktree import-dsc import-maintmangle import-native import-nonnative import-tarbomb inarchivecopy mismatches-contents mismatches-dscchanges multisuite newtag-clone-nogit oldnewtagalt oldtag-clone-nogit orig-include-exclude orig-include-exclude-chkquery overwrite-chkclog overwrite-junk overwrite-splitbrains overwrite-version protocol-compat push-buildproductsdir push-newpackage push-newrepeat push-nextdgit push-source push-source-with-changes quilt quilt-gbp quilt-gbp-build-modes quilt-singlepatch quilt-splitbrains quilt-useremail rpush sourceonlypolicy tag-updates test-list-uptodate unrepresentable version-opt
+Tests: absurd-gitapply badcommit-rewrite build-modes build-modes-asplit build-modes-gbp-asplit checkout clone-clogsigpipe clone-gitnosuite clone-nogit debpolicy-dbretry debpolicy-newreject debpolicy-quilt-gbp defdistro-rpush defdistro-setup distropatches-reject dpkgsourceignores-correct drs-clone-nogit drs-push-masterupdate drs-push-rejects dsd-clone-nogit dsd-divert fetch-localgitonly fetch-somegit-notlast gbp-orig gitconfig gitworktree import-dsc import-maintmangle import-native import-nonnative import-tarbomb inarchivecopy
Bug#857316: dgit: please make --build-products-dir configurable
On Fri, Mar 10, 2017 at 12:55:44AM +, Ian Jackson wrote: > > PS: to be on the safe side I currently delete the *.deb and the > > *_amd64.changes from that directory before running `dgit dput`, so I'm > > sure it won't try to upload binaries but only the _source.changes. > > What's the heuristic dgit uses to pick a .changes file, and how would I > > do to avoid that extra work of removing the binaries? > > I think you may find -C useful: you can specify explicitly the > .changes file. dgit won't upload anything but whatever is in that. Yeah, I used that a couple of times, but giving that it requires changing every time I found it annoying (i.e. `dgit -wc pu` is not enough as it is with --build-products-dir) > Also, anyway, dgit is a bit cautious if it sees multiple .changes > files. I think you will probably find it will complain, rather than > doing the wrong thing. Try it with --damp-run and see ? I will, at the next NMU :) -- regards, Mattia Rizzolo GPG Key: 66AE 2B4A FCCF 3F52 DA18 4D18 4B04 3FCD B944 4540 .''`. more about me: https://mapreri.org : :' : Launchpad user: https://launchpad.net/~mapreri `. `'` Debian QA page: https://qa.debian.org/developer.php?login=mattia `- signature.asc Description: PGP signature
Bug#857316: dgit: please make --build-products-dir configurable
Mattia Rizzolo writes ("Bug#857316: dgit: please make --build-products-dir
configurable"):
> package: dgit
>
> I build my packages with pbuilder, and it is set up to put the results
> in ~/pbuilder/results/$distribution/$architecture/. I'd like dgit to go
> look for the .changes to upload there, and I currently accomplish that
> by passing `--build-products-dir ~/pbuilder/result/sid/amd64` to
> `dgit push`. I'd like to configure that statically, so I don't have to
> keep specifying it.
dgit needs better pbuilder integration generally. (There's another
bug for that.)
But I think a config option for the default --build-products-dir would
be a good thing too (or maybe part of that). So I agree with this bug
report.
> PS: to be on the safe side I currently delete the *.deb and the
> *_amd64.changes from that directory before running `dgit dput`, so I'm
> sure it won't try to upload binaries but only the _source.changes.
> What's the heuristic dgit uses to pick a .changes file, and how would I
> do to avoid that extra work of removing the binaries?
I think you may find -C useful: you can specify explicitly the
.changes file. dgit won't upload anything but whatever is in that.
Also, anyway, dgit is a bit cautious if it sees multiple .changes
files. I think you will probably find it will complain, rather than
doing the wrong thing. Try it with --damp-run and see ?
Regards,
Ian.
--
Ian Jackson <ijack...@chiark.greenend.org.uk> These opinions are my own.
If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.
Bug#857316: dgit: please make --build-products-dir configurable
package: dgit I build my packages with pbuilder, and it is set up to put the results in ~/pbuilder/results/$distribution/$architecture/. I'd like dgit to go look for the .changes to upload there, and I currently accomplish that by passing `--build-products-dir ~/pbuilder/result/sid/amd64` to `dgit push`. I'd like to configure that statically, so I don't have to keep specifying it. PS: to be on the safe side I currently delete the *.deb and the *_amd64.changes from that directory before running `dgit dput`, so I'm sure it won't try to upload binaries but only the _source.changes. What's the heuristic dgit uses to pick a .changes file, and how would I do to avoid that extra work of removing the binaries? -- regards, Mattia Rizzolo GPG Key: 66AE 2B4A FCCF 3F52 DA18 4D18 4B04 3FCD B944 4540 .''`. more about me: https://mapreri.org : :' : Launchpad user: https://launchpad.net/~mapreri `. `'` Debian QA page: https://qa.debian.org/developer.php?login=mattia `- signature.asc Description: PGP signature
