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:
"""
if (package_arch($package) ne package_arch($dh{LINK_DOC})) {
"""
will never be true during a binNMU. This is because $package will never
be an arch:all package (as they are not acted on) and $dh{LINK_DOC} will
be an architecture dependent package (per definition of this case).
Accordingly there will never be a warning or an error.
We could resolve this by creating an
"$dh{LINK_DOC} >= $dh{VERSION}, $dh{LINK_DOC} << $dh{VERSION}+bA~"
dependency. Unfortunately, the dependency is only created on source
uploads (when the arch:all package is built), so that would /not/ be an
retroactive bug fix. :-/
Test cases:
* python-apt/0.9.3.11
- even with my patch, a binNMU the build succeeds (without any
warnings).
* thunar/1.6.3
- with my patch, the issue is detected and the build is aborted.
For testing a binNMU build, use:
"dch -p --bin-nmu '' && dpkg-buildpackage -B"
~Niels
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:
diff --git a/t/size b/t/size
index d8b9896..95177f4 100755
--- a/t/size
+++ b/t/size
@@ -24,6 +24,7 @@ foreach my $file (@progs) {
}
close IN;
print "# $file has $lines lines, max length is $maxlength\n";
- ok($lines < 200, $file);
- ok($maxlength < 160, $file);
+ ok($lines < 200, $file) or diag("$file has $lines lines (< 200)");
+ ok($maxlength < 160, $file)
+ or diag("$file has $maxlength char long lines (< 160)");
}
