On 2014-03-03 16:09, Johannes Schauer wrote: > Package: lintian > Version: 2.5.22 > Severity: wishlist > Tags: patch > > Hi, > > I implemented a preliminary patch for lintian to understand and check > the restriction syntax described in this document: > https://wiki.debian.org/BuildProfileSpec >
Hi, Thanks for looking into this. > Please tell me what needs to be changed so that I can get this patch > into an acceptable shape and excuse my weak perl-foo :) > Certainly; find my comments interleaved in your patch below. > Depending on the outcome of this thread [1] an amendment might be > necessary in the future to allow a new field in DEBIAN/control. > > cheers, josch > Noted; generally that is just modifying (a subset of the) data files matching data/{fields,common}/*-fields. > [1] http://lists.debian.org/20140221113013.18244.3579@hoothoot > > > 0001-Build-profiles-support.patch > > >>From 33a76313fc1ecb5d7fd970c5ed41b9da12e85269 Mon Sep 17 00:00:00 2001 > From: josch <j.scha...@email.de> > Date: Mon, 3 Mar 2014 15:58:28 +0100 > Subject: [PATCH] Build-profiles support > > - added 3 new tags to detect errors in restriction list syntax > * invalid-restriction-term-in-source-relation > * invalid-restriction-namespace-in-source-relation > * invalid-restriction-label-in-source-relation > - added 4 new tags to ensure that if restrictions lists are used, a > versioned build dependency on dpkg-dev and (if applicable) debhelper > is added and no conflicts with them exist > * restriction-list-without-versioned-dpkg-dev-dependency > * restriction-list-with-versioned-dpkg-dev-conflict > * restriction-list-with-debhelper-without-debhelper-version > * restriction-list-with-debhelper-with-conflicting-debhelper-version > --- > checks/fields.desc | 50 +++++++++++++++++ > checks/fields.pm | 64 > ++++++++++++++++++---- > lib/Lintian/Relation.pm | 18 +++++- > .../debian/debian/control.in | 6 +- > t/tests/fields-build-depends-general/tags | 7 +++ > 5 files changed, 131 insertions(+), 14 deletions(-) > > diff --git a/checks/fields.desc b/checks/fields.desc > index d55c11a..6061b95 100644 > --- a/checks/fields.desc > +++ b/checks/fields.desc > @@ -637,6 +637,56 @@ Info: The architecture string in this source relation > has some > negated. This is not permitted by Policy. Either all architectures must > be negated or none of them may be. > > +[...] > + > +Tag: invalid-restriction-label-in-source-relation > +Severity: important > +Certainty: possible > +Info: The restriction list in the source relation includes a term with > + an unknown label. The only allowed labels are "stage1", "stage2", "notest" > + and "cross". > + This is inconsistent when the check, which only seem to be allowing "stage1" and "notest" > [...] > + > +Tag: restriction-list-with-versioned-dpkg-dev-conflict > +Severity: normal > +Certainty: certain > +Info: If a restriction list appears in the build dependencies, then the > + source package has to build depend on dpkg-dev (>= 1.17.2) for minimal > + restriction list support. It must not conflict with version 1.17.2. > + > [...] > + > +Tag: restriction-list-with-debhelper-with-conflicting-debhelper-version > +Severity: normal > +Certainty: certain > +Info: If a restriction list appears in the build dependencies and the > + package uses debhelper, then the source package has to depend on at least > + debhelper 9.20140227. It must not conflict with version 9.20140227. > + Usually, we don't bother doing "conflict" testing. At least, we skip it for regular debhelper dependencies and many other cases. > Tag: depends-on-build-essential-package-without-using-version > Severity: important > Certainty: certain > diff --git a/checks/fields.pm b/checks/fields.pm > index aeee252..2cae275 100644 > --- a/checks/fields.pm > +++ b/checks/fields.pm > @@ -730,7 +730,7 @@ sub run { > && $known_obsolete_emacs{$alternatives[0]->[0]}); > > for my $part_d (@alternatives) { > - my ($d_pkg, $d_march, $d_version, $d_arch, $rest, > + my ($d_pkg, $d_march, $d_version, $d_arch, $d_restr, > $rest, > $part_d_orig) > = @$part_d; > If $d_restr is not used in the scope, please consider replacing it with "undef" to signal that. (The diff suggests it is unused). > @@ -935,6 +935,7 @@ sub run { > any { $_ eq $_[0] } qw(build-depends build-depends-indep); > }; > > + my $restrictions_used = 0; > my %depend; > for my $field ( > qw(build-depends build-depends-indep build-conflicts > build-conflicts-indep) > @@ -956,7 +957,7 @@ sub run { > && &$is_dep_field($field)); > > for my $part_d (@alternatives) { > - my ($d_pkg, $d_march, $d_version, $d_arch, $rest, > + my ($d_pkg, $d_march, $d_version, $d_arch, $d_restr, > $rest, > $part_d_orig) > = @$part_d; > > @@ -968,6 +969,27 @@ sub run { > } > } > > + if (scalar @{$d_restr} >= 1) { > + $restrictions_used = 1; > + } > + > + for my $restr (@{$d_restr}) { > + my $dotcount = () = $restr =~ /\./g; You probably want: my $dotcount = $restr =~ tr/.//; Alternatively: my $dotcount; $dotcount++ while ($dotcount =~ m/\./g); (Note that the tr does /not/ need escaping of the period.) > + if ($dotcount != 1) { > + tag > 'invalid-restriction-term-in-source-relation', > + "$restr [$field: $part_d_orig]"; > + next; > + } > + $restr =~ s/^!//; > + my ($ns, $label) = split(/\./, $restr, 2); > + tag > 'invalid-restriction-namespace-in-source-relation', > + "$ns [$field: $part_d_orig]" > + unless $ns eq "profile"; > + tag > 'invalid-restriction-label-in-source-relation', > + "$label [$field: $part_d_orig]" > + unless any { $_ eq $label } ("stage1", > "notest"); Two things here; one, Lintian emit "invalid-restriction-label-in-source-relation" when namespace is not "profile". This seems wrong to me. If Lintian does not know the namespace, Lintian should not claim to know the labels of said namespace. Secondly, I got a feeling that we want to move this to a data file (if not now, then eventually). > + } > + > if ( $d_pkg =~ m/^openjdk-\d+-doc$/o > or $d_pkg eq 'classpath-doc'){ > tag 'build-depends-on-specific-java-doc-package', > @@ -1077,6 +1099,24 @@ sub run { > } > } > > + # if restrictions are found in the build-depends/conflicts, then > + # package must build-depend on dpkg (>= 1.17.2) > + if ($restrictions_used) { > + my $build_conflicts_all = $info->relation('build-conflicts-all'); > + tag 'restriction-list-without-versioned-dpkg-dev-dependency' > + unless ($build_all->implies('dpkg-dev (>= 1.17.2)')); > + tag 'restriction-list-with-versioned-dpkg-dev-conflict' > + if ($build_conflicts_all->implies_inverse('dpkg-dev (<< > 1.17.2)')); > + # if the package uses debhelper then it must require and not > + # conflict with version >= 9.20140227 > + if ($build_all->implies('debhelper')) { > + tag > 'restriction-list-with-debhelper-without-debhelper-version' > + unless ($build_all->implies('debhelper (>= 9.20140227)')); > + tag > 'restriction-list-with-debhelper-with-conflicting-debhelper-version' > + if ($build_conflicts_all->implies_inverse('debhelper (<< > 9.20140227)')); > + } > + } > + As mentioned earlier; we tend not to do the "conflict" tests. Feel free to skip them entirely. > my (@arch_dep_pkgs, @dbg_pkgs); > foreach my $gproc ($group->get_binary_processables) { > my $binpkg = $gproc->pkg_name; > @@ -1238,16 +1278,16 @@ sub run { > return; > } > > -# splits "foo:bar (>= 1.2.3) [!i386 ia64]" into > -# ( "foo", "bar", [ ">=", "1.2.3" ], [ [ "i386", "ia64" ], 1 ], "" ) > -# ^^^ ^^ > -# count of negated arches, if ! was given || > -# rest (should always be "" for valid dependencies) > +# splits "foo:bar (>= 1.2.3) [!i386 ia64] <!profile.stage1 !profile.notest>" > into > +# ( "foo", "bar", [ ">=", "1.2.3" ], [ [ "i386", "ia64" ], 1 ], [ > "!profile.stage1" "!profile.notest" ], "" ) > +# ^^^ > ^^ > +# count of negated arches, if ! was given > || > +# rest (should > always be "" for valid dependencies) > sub _split_dep { > my $dep = shift; > - my ($pkg, $dmarch, $version, $darch) = ('', '', ['',''], [[], 0]); > + my ($pkg, $dmarch, $version, $darch, $restr) = ('', '', ['',''], [[], > 0], []); > > - my $pkgname = $1 if $dep =~ s/^\s*([^\s\[\(]+)\s*//; > + my $pkgname = $1 if $dep =~ s/^\s*([^<\s\[\(]+)\s*//; > if (defined $pkgname) { > ($pkg, $dmarch) = split /:/, $pkgname, 2; > $dmarch //= ''; # Ensure it is defined (in case there is no ":") > @@ -1267,9 +1307,13 @@ sub _split_dep { > } > $darch->[1] = $negated; > } > + if ($dep && $dep =~ s/\s*<([^>]+)>\s*//) { > + my $t = $1; > + $restr = [split /\s+/, $t]; > + } > } > > - return ($pkg, $dmarch, $version, $darch, $dep); > + return ($pkg, $dmarch, $version, $darch, $restr, $dep); > } > > sub perl_core_has_version { > diff --git a/lib/Lintian/Relation.pm b/lib/Lintian/Relation.pm > index 1ba783c..4ec8238 100644 > --- a/lib/Lintian/Relation.pm > +++ b/lib/Lintian/Relation.pm I think the "new_noarch" constructor might need to be updated to ignore these profiles as well. It is used to flag some missing build-dependencies in checks/rules.pm and checks/debhelper.pm (look for "_noarch"). Basically these checks ensures that a dependency is (probably) present on some architecture (and now also profile) when a given tool is used. Example: If you call dh_python, then there should be a dependency for the package providing dh_python. It may be architecture/profile dependent (e.g. not used during stage1), but some profile should need that dependency (else you wouldn't need to call dh_python). > [...] > > # Optimise the memory usage of the array. Understanding this > @@ -351,6 +356,13 @@ sub implies_element { > $$q[1] = '' unless defined $$q[1]; > return if $$p[1] ne $$q[1]; > > + # Since the restriction list is not a set (as the architecture list) > there > + # is no way to calculate a superset or subset of one another. > Furthermore, > + # the evaluation depends on which build profiles are currently activated. > + # With n being the number of possible build profiles, 2^n checks would > + # have to be done. We decide not to do that (yet). > + return if defined $$p[6] or defined $$q[6]; > + O(n^2) checking? Ugh. I suppose we could "normalise" the order before the comparison for slightly more accuracy, but still... ugh. Actually, if we (for a moment) ignore negative profiles (e.g. imagine a world without "!profile.stage1"), wouldn't this just be checking that the "larger" of the two is a subset of the other (assuming both are defined)? Can you give me an example of where this O(n^2) kicks in? > # If the names match, then the only difference is in the architecture or > # version clauses. First, check architecture. The architectures for p > # must be a superset of the architectures for q. > diff --git a/t/tests/fields-build-depends-general/debian/debian/control.in > b/t/tests/fields-build-depends-general/debian/debian/control.in > index f6546b9..12acd64 100644 > --- a/t/tests/fields-build-depends-general/debian/debian/control.in > +++ b/t/tests/fields-build-depends-general/debian/debian/control.in > [...] > diff --git a/t/tests/fields-build-depends-general/tags > b/t/tests/fields-build-depends-general/tags > index 0c0222f..c5413f0 100644 > --- a/t/tests/fields-build-depends-general/tags > +++ b/t/tests/fields-build-depends-general/tags > @@ -9,9 +9,16 @@ E: fields-build-depends-general source: > depends-on-build-essential-package-witho > E: fields-build-depends-general source: > invalid-arch-string-in-source-relation all [build-depends: foo [all]] > E: fields-build-depends-general source: > invalid-arch-string-in-source-relation i3!86 [build-depends: baz [source > i3!86]] > E: fields-build-depends-general source: > invalid-arch-string-in-source-relation source [build-depends: baz [source > i3!86]] > +E: fields-build-depends-general source: > invalid-restriction-label-in-source-relation bar [build-depends: bpfail2 > <foo.bar>] > +E: fields-build-depends-general source: > invalid-restriction-namespace-in-source-relation foo [build-depends: bpfail2 > <foo.bar>] > +E: fields-build-depends-general source: > invalid-restriction-term-in-source-relation foobar [build-depends: bpfail1 > <foobar>] > I: fields-build-depends-general source: > build-depends-on-python-dev-with-no-arch-any > I: fields-build-depends-general source: > ored-build-depends-on-obsolete-package build-depends: xlibmesa-gl-dev > W: fields-build-depends-general source: build-depends-on-1-revision > build-depends: revision-1 (>= 1.0-1) > W: fields-build-depends-general source: > build-depends-on-versioned-berkeley-db build-depends:libdb5.1++-dev > W: fields-build-depends-general source: > build-depends-on-versioned-berkeley-db build-depends:libdb5.1-java-dev > W: fields-build-depends-general source: depends-on-packaging-dev > build-depends > +W: fields-build-depends-general source: > restriction-list-with-debhelper-with-conflicting-debhelper-version > +W: fields-build-depends-general source: > restriction-list-with-debhelper-without-debhelper-version > +W: fields-build-depends-general source: > restriction-list-with-versioned-dpkg-dev-conflict > +W: fields-build-depends-general source: > restriction-list-without-versioned-dpkg-dev-dependency > -- 1.8.5.3 > Please add the new tags to the "Test-For" field in t/tests/fields-build-depends-general/desc (Please keep the field sorted by name of the tags) If you have done it right, you should be able to run "private/update-coverage" and t/COVERAGE will *not* list any of your new tags. Feel free to omit the changes to t/COVERAGE though. Once again, thanks for looking into this. ~Niels -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org