Control: tags -1 patch
On 2014-11-16 20:02, Niels Thykier wrote:
> On 2014-11-16 17:39, Niels Thykier wrote:
>> Hi Bernhard,
>>
>> Thanks for looking into this. :)
>>
>> [...]
>>
>> I think the patch is definitely a good start. However, I do think that
>> detecting and error'ing out on the "broken binNMU" case should be
>> trivially enough and much better than "silently"[1] continuing with the
>> build.
>> I will look into extending your patch to handle that case as well.
>>
>> ~Niels
>>
>> [1] There will be the warning, but no one reads the logs from the
>> buildds before it is too late.
>>
>> [...]
>
> I got an extended patch (see attached diff file) that errors out on
> *some* binNMUs. Turns out that there is a case it does not handle and
> that will be difficult to deal with.
>
> The problem is when an arch:all package links to an arch:any package.
> In binNMU build, dh_installdocs will not see the "arch:all" package as
> it is told not to act on it. This means that:
>
> [...]
Ok, I got a solution for this as well. I have attached 3 patches to
solve the issue. The first one is pretty much the same as the previous
one. The second one is to avoid a test failure because my 3rd patch
pushes the size of dh_installdocs beyond the limit of 200 (non-comment)
lines.
The third patch makes dh_installdocs able to detect the "arch:all
--link-doc arch:any" case by keeping track of which packages have been
filtered out by an (implicit) -a option from DH_INTERNAL_OPTIONS).
~Niels
>From 453efef7661048188d91ef1cdd1e530902bc56d4 Mon Sep 17 00:00:00 2001
From: Niels Thykier <ni...@thykier.net>
Date: Sun, 16 Nov 2014 20:18:33 +0100
Subject: [PATCH 1/3] dh_installdocs: Error out on some unsafe binNMUs with
--link-doc
Signed-off-by: Niels Thykier <ni...@thykier.net>
---
dh_installdocs | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/dh_installdocs b/dh_installdocs
index 3eefcdf..af10898 100755
--- a/dh_installdocs
+++ b/dh_installdocs
@@ -158,6 +158,8 @@ init(options => {
"link-doc=s" => \$dh{LINK_DOC},
});
+my $called_getpackages = 0;
+
foreach my $package (@{$dh{DOPACKAGES}}) {
next if is_udeb($package);
@@ -166,6 +168,27 @@ foreach my $package (@{$dh{DOPACKAGES}}) {
my $link_doc=($dh{LINK_DOC} && $dh{LINK_DOC} ne $package);
if ($link_doc) {
+ getpackages('both') unless $called_getpackages++;
+
+ if (package_arch($package) ne package_arch($dh{LINK_DOC})) {
+ if (compat(9)) {
+ my $changelog=pkgfile($package, 'changelog') || 'debian/changelog';
+ if (! -e $changelog) {
+ error("could not find changelog $changelog");
+ }
+
+ open(my $fd, '<', $changelog) or error("open $changelog: $!");
+ my $line = <$fd>;
+ close($fd);
+
+ warning("WARNING: --link-doc between architecture all and not all packages breaks binNMUs");
+ if (defined($line) && $line =~ m/\A\S.*;.*\bbinary-only=yes/) {
+ error("Aborting build as this is a binNMU (leading to a broken package)");
+ }
+ } else {
+ error("--link-doc not allowed between ${package} and $dh{LINK_DOC} (one is all and the other not)");
+ }
+ }
# Make sure that the parent directory exists.
if (! -d "$tmp/usr/share/doc" && ! -l "$tmp/usr/share/doc") {
doit("install","-g",0,"-o",0,"-d","$tmp/usr/share/doc");
@@ -345,3 +368,9 @@ This program is a part of debhelper.
Joey Hess <jo...@debian.org>
=cut
+
+# Local Variables:
+# indent-tabs-mode: t
+# tab-width: 4
+# cperl-indent-level: 4
+# End:
--
2.1.3
>From 1bdddc8ba25cac2947cde4e5fcbac24f820478f1 Mon Sep 17 00:00:00 2001
From: Niels Thykier <ni...@thykier.net>
Date: Sun, 16 Nov 2014 21:26:37 +0100
Subject: [PATCH 2/3] t/size: Ignore empty lines when counting size of a tool
Signed-off-by: Niels Thykier <ni...@thykier.net>
---
t/size | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/size b/t/size
index d8b9896..bf7847d 100755
--- a/t/size
+++ b/t/size
@@ -18,7 +18,7 @@ foreach my $file (@progs) {
while (<IN>) {
$cutting=1 if /^=/;
$cutting=0 if /^=cut/;
- next if $cutting || /^(=|\s*\#)/;
+ next if $cutting || /^(=|\s*\#)/ || /^\s*+$/;
$lines++;
$maxlength=length($_) if length($_) > $maxlength;
}
--
2.1.3
>From 43ff757e0a64eeded33447efbd214a098c8fab64 Mon Sep 17 00:00:00 2001
From: Niels Thykier <ni...@thykier.net>
Date: Sun, 16 Nov 2014 21:27:00 +0100
Subject: [PATCH 3/3] dh_installdocs: Error out on unsafe binNMUs with
--link-doc
Detect during a binNMU when an call to dh_installdocs (with
--link-doc) is unsafe by also looking at packages that dh_installdocs
/would/ process during an "full" build.
Signed-off-by: Niels Thykier <ni...@thykier.net>
---
Debian/Debhelper/Dh_Getopt.pm | 17 ++++++++++++++++-
dh_installdocs | 20 +++++++++++++++++---
2 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/Debian/Debhelper/Dh_Getopt.pm b/Debian/Debhelper/Dh_Getopt.pm
index e4f3e47..3468071 100644
--- a/Debian/Debhelper/Dh_Getopt.pm
+++ b/Debian/Debhelper/Dh_Getopt.pm
@@ -182,7 +182,7 @@ sub split_options_string {
sub parseopts {
my %params=@_;
- my @ARGV_extra;
+ my (@ARGV_extra, %internal_excluded);
# DH_INTERNAL_OPTIONS is used to pass additional options from
# dh through an override target to a command.
@@ -199,6 +199,7 @@ sub parseopts {
foreach my $package (getpackages()) {
if (! grep { $_ eq $package } @{$dh{DOPACKAGES}}) {
$exclude_package{$package}=1;
+ $internal_excluded{$package}=1;
}
}
}
@@ -260,6 +261,14 @@ sub parseopts {
$packages_seen{$package}=1;
push @package_list, $package;
}
+ } elsif ($internal_excluded{$package}) {
+ # Record packages we would have processed if not for
+ # DH_INTERNAL_OPTIONS.
+ # We need this for dh_installdocs to check for broken
+ # binNMUs with --link-doc
+ push @{$dh{_INTERNAL_EXCL_DOPACKAGES}}, $package;
+ # Remove it to avoid duplicates
+ delete $internal_excluded{$package};
}
}
@{$dh{DOPACKAGES}}=@package_list;
@@ -286,3 +295,9 @@ sub parseopts {
}
1
+
+# Local Variables:
+# indent-tabs-mode: t
+# tab-width: 4
+# cperl-indent-level: 4
+# End:
diff --git a/dh_installdocs b/dh_installdocs
index af10898..30899a0 100755
--- a/dh_installdocs
+++ b/dh_installdocs
@@ -159,6 +159,7 @@ init(options => {
});
my $called_getpackages = 0;
+my $link_doc_arch;
foreach my $package (@{$dh{DOPACKAGES}}) {
next if is_udeb($package);
@@ -168,9 +169,22 @@ foreach my $package (@{$dh{DOPACKAGES}}) {
my $link_doc=($dh{LINK_DOC} && $dh{LINK_DOC} ne $package);
if ($link_doc) {
- getpackages('both') unless $called_getpackages++;
+ my $has_issue = 0;
+ unless ($called_getpackages++) {
+ # Called for the side-effect of making package_arch work.
+ getpackages('both');
+ $link_doc_arch = package_arch($dh{LINK_DOC});
+ # Check for broken --link-doc during binNMUs.
+ for my $excl_pkg (@{$dh{_INTERNAL_EXCL_DOPACKAGES}}) {
+ if (package_arch($excl_pkg) ne $link_doc_arch) {
+ $has_issue = $excl_pkg;
+ last;
+ }
+ }
+ }
- if (package_arch($package) ne package_arch($dh{LINK_DOC})) {
+ $has_issue = $package if (package_arch($package) ne $link_doc_arch);
+ if ($has_issue) {
if (compat(9)) {
my $changelog=pkgfile($package, 'changelog') || 'debian/changelog';
if (! -e $changelog) {
@@ -186,7 +200,7 @@ foreach my $package (@{$dh{DOPACKAGES}}) {
error("Aborting build as this is a binNMU (leading to a broken package)");
}
} else {
- error("--link-doc not allowed between ${package} and $dh{LINK_DOC} (one is all and the other not)");
+ error("--link-doc not allowed between ${has_issue} and $dh{LINK_DOC} (one is all and the other not)");
}
}
# Make sure that the parent directory exists.
--
2.1.3
