Bug#1030325: lintian: archive-liberty-mismatch (in section for firmware-nvidia-gsp) non-free-firmware vs non-free
Hi,
there are two variants in which this tag is emitted, with different
parameters:
One is
archive-liberty-mismatch (in section for $installable) $area1 vs $area2 [$path]
The other one is:
archive-liberty-mismatch (in source paragraph) $area1 -> $area2 [$path]
Kibi's patch patches the second variant and Andreas' patch patches the
first variant — which was also the one emitted by running Lintian
against his package.
So it makes sense that Kibi's patch didn't fix Andreas' false
positive. Nevertheless Kibi found a likely problematic part of the
code.
Axel Beckert wrote:
> > changing this:
> > for my $inferior_liberty ('non-free', 'contrib') {
> > into this:
> > for my $inferior_liberty ('non-free-firmware', 'non-free', 'contrib') {
> […]
> > # must remain inferior
> > last
> > if $inferior_liberty eq $source_liberty;
>
> Thanks for that analysis. I suspect this type of change is probably
> necessary. Will have a closer look at it.
I must admit, I'm not sure what the check emitting the second variant
(and patched with Kibi's patch) actually does check for. (I have a vague
idea, but that's it.)
And unfortunately I don't find any example for it I could test it
against. Both, lintian's test suite as well as (the in this case
luckily outdated
https://lintian.debian.org/tags/archive-liberty-mismatch only know
examples of the first type. And
https://udd.debian.org/lintian-tag.cgi?tag=archive-liberty-mismatch
doesn't report any anymore. (Probably doesn't check non-free packages.
Or lags a bit behind.)
So I tried to figure out when this got added and to see if there's an
accompaning bug report:
Figured out that tag got renamed twice and wasn't mentioned in
debian/changelog by its name upon its introduction. Which didn't make
searching for that bug report easier.
So I tried git log and git blame. That ended quickly at a commit with
the commit message "Split the check debian/control into 26 smaller
checks.". And then "Move all checks into the module name space under
./lib". And then "Capitalize module names for checks in camel case;
drop underscores." And then "Move check control-file to
'debian/control'." And then "checks: Rename all checks to include a
".pm" extension". *sigh*
At that point I at least reached the commit which renamed the tag for
the first time: d431e568dc33b6f862466218908d337fff43626b from 2009.
>From there on searching through "git -log -p -- checks/control-file"
helped.
There seemed to be no bug report associated, the tag has been
introduced by Russ in commit
204fbd7b78a95e4d940ee1eea647fe365ccaaa4f in 2006. But back then the
code looks very different and only seems to check binary packages.
So I ended up following every change in that tag with
git log -p -- lib/Lintian/Check/Archive/Liberty/Mismatch.pm lib/Lintian/Check/Debian/Control.pm checks/Debian/Control.pm checks/debian/control.pm checks/control-file.pm checks/control-file
The first commit which added a second variant checking source against
binary packages is 2e83812c19e1466bc05e65bb3fa52b33e33305e8 by Ivo De
Decker from 2019, i.e. rather recently:
Check for sources in the "main" section with only binaries in the contrib section. (Closes: #928126)
But that code is still very clear and readable. And its obvious what
it does.
So actually the commit that changed the code to become way more
convoluted and unreadable was this one:
commit 1edef669729b04bc67d480b81568dfa8e5e3ae0f
Author: Felix Lechner <felix.lechner@lease-up.com>
Date: Mon Nov 1 05:03:13 2021 -0700
Split the check debian/control into 26 smaller checks.
Gbp-Dch: ignore
Yes, this one. Took me a few attempts to realize, too. And no, it
didn't just split up code. It basically rewrote the check so much
(maybe even from scratch?) that I can't really recognize which part of
the new code resembles what part in the old file. Especially that "in
ascending order of liberty" got introduced there.
It also renamed the emitted tag from section-area-mismatch to
archive-liberty-mismatch without even mentioning it the commit
message. 🙄
Not to mention that really inappropriate "Gbp-Dch: ignore" for such a
profound rewrite. 🤌
And since there's no test for that variant, I can't even tell if it
still works properly. Not even thinking about changing it without
knowing how to test it.
So I'll keep my fingers of that code shortly before the soft freeze
unless someone gives me an example that either triggers the second
variant of this tag or one that doesn't trigger it, but should.
> > The attached patch does make the error go away by adding an exception for
> > 'non-free-firmware build from non-free' similar to 'contrib built from main'
>
> > --- /usr/share/lintian/lib/Lintian/Check/Archive/Liberty/Mismatch.pm.orig 2023-02-03 00:30:29.782861611 +0100
> > +++ /usr/share/lintian/lib/Lintian/Check/Archive/Liberty/Mismatch.pm 2023-02-03 01:02:12.441805855 +0100
> > @@ -87,6 +87,9 @@
> > # special exception for contrib built from main
> > next
> > if $source_liberty eq 'main' && $installable_liberty eq 'contrib';
> > + # and non-free-firmware built from non-free
> > + next
> > + if $source_liberty eq 'non-free' && $installable_liberty eq 'non-free-firmware';
> >
> > my $control_item= $self->processable->debian_control->item;
> > my $position = $installable_fields->position('Section');
>
> Thanks for that patch, too.
Will apply that one with some fine-tuning wrt. to line length, etc.
And then release Lintian 2.116.3.
Regards, Axel (Software Archaeologist)
--
,''`. | Axel Beckert <abe@debian.org>, https://people.debian.org/~abe/
: :' : | Debian Developer, ftp.ch.debian.org Admin
`. `' | 4096R: 2517 B724 C5F6 CA99 5329 6E61 2FF9 CD59 6126 16B5
`- | 1024D: F067 EA27 26B9 C3FC 1486 202E C09E 1D89 9593 0EDE
Reply to: