[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

lintian: r751 - in trunk: checks debian lib testset testset/relations/debian



Author: rra
Date: 2006-09-24 07:58:23 +0200 (Sun, 24 Sep 2006)
New Revision: 751

Modified:
   trunk/checks/fields
   trunk/checks/fields.desc
   trunk/debian/changelog
   trunk/lib/Dep.pm
   trunk/testset/relations/debian/control
   trunk/testset/tags.cdbs-test
   trunk/testset/tags.relations
Log:
* checks/fields{.desc,}:
  + [RA] Add a check for duplicate build dependencies.  (Closes: #359178)
* lib/Dep.pm:
  + [RA] Add an unparse function to take an internal representation and
    convert it back to human-readable text.
  + [RA] Significantly improve and rewrite the get_dups function to only
    find relations that imply each other and to return the complete
    duplicate dependencies in sets.


Modified: trunk/checks/fields
===================================================================
--- trunk/checks/fields	2006-09-24 04:23:11 UTC (rev 750)
+++ trunk/checks/fields	2006-09-24 05:58:23 UTC (rev 751)
@@ -626,10 +626,19 @@
 			}
 		}
 	}
+
+	# Check for duplicates.
+	my $build_all = $depend{'build-depends'} || '';
+	$build_all .= ', ' if $depend{'build-depends'} && $depend{'build-depends-indep'};
+	$build_all .= $depend{'build-depends-indep'} || '';
+	my @dups = Dep::get_dups(Dep::parse($build_all));
+	for my $dup (@dups) {
+		tag "package-has-a-duplicate-build-relation", join (', ', @$dup);
+	}
+
+	# Make sure build dependencies and conflicts are consistent.
 	$depend{'build-depends'} = Dep::parse($depend{'build-depends'} || '');
 	$depend{'build-depends-indep'} = Dep::parse($depend{'build-depends-indep'} || '');
-
-	# Make sure build dependencies and conflicts are consistent.
 	for ($depend{'build-conflicts'}, $depend{'build-conflicts-indep'}) {
 		next unless $_;
 		for my $conflict (split /\s*,\s*/, $_) {

Modified: trunk/checks/fields.desc
===================================================================
--- trunk/checks/fields.desc	2006-09-24 04:23:11 UTC (rev 750)
+++ trunk/checks/fields.desc	2006-09-24 05:58:23 UTC (rev 751)
@@ -515,6 +515,12 @@
  in that case, as it can only be fixed at the package-tools level (e.g., in
  dpkg)
 
+Tag: package-has-a-duplicate-build-relation
+Type: warning
+Info: The package declares the given build relations on the same package
+ in either Build-Depends or Build-Depends-Indep, but the build relations
+ imply each other and are therefore redundant.
+
 Tag: needlessly-depends-on-awk
 Type: error
 Info: The package seems to declare a relation on awk. awk is a virtual

Modified: trunk/debian/changelog
===================================================================
--- trunk/debian/changelog	2006-09-24 04:23:11 UTC (rev 750)
+++ trunk/debian/changelog	2006-09-24 05:58:23 UTC (rev 751)
@@ -3,10 +3,11 @@
   * checks/debconf:
     + [CW] Don't trigger partially-translated-question when Choices-C exists
       but not Description-C.
-  * checks/fields.desc:
+  * checks/fields{.desc,}:
     + [RA] Improve the long descriptions of the warnings about dependencies
       on essential or build-essential packages to make it clearer that the
       correct resolution is normally to omit the dependency and why.
+    + [RA] Add a check for duplicate build dependencies.  (Closes: #359178)
   * checks/files:
     + [RA] Don't consider files named license.rb to be extraneous
       licenses.  Patch from NIIBE Yutaka.  (Closes: #387269)
@@ -29,8 +30,13 @@
       aa|bb implies aa|bb|cc.  Fix the comparison of dependencies for
       inverse implication to catch many more cases.  (The latter code
       isn't currently used in lintian.)
+    + [RA] Add an unparse function to take an internal representation and
+      convert it back to human-readable text.
+    + [RA] Significantly improve and rewrite the get_dups function to only
+      find relations that imply each other and to return the complete
+      duplicate dependencies in sets.
 
- -- Russ Allbery <rra@debian.org>  Sat, 23 Sep 2006 21:19:28 -0700
+ -- Russ Allbery <rra@debian.org>  Sat, 23 Sep 2006 22:57:34 -0700
 
 lintian (1.23.24) unstable; urgency=low
 

Modified: trunk/lib/Dep.pm
===================================================================
--- trunk/lib/Dep.pm	2006-09-24 04:23:11 UTC (rev 750)
+++ trunk/lib/Dep.pm	2006-09-24 05:58:23 UTC (rev 751)
@@ -58,21 +58,54 @@
 
 # Convert a dependency line into the internal format.
 # Non-local callers may store the results of this routine.
-sub parse 
-{	my @deps;
-    for (split(/\s*,\s*/, $_[0])) 
-	{ 	my @alts;
-
-		if ( $_ =~ /^perl\s+\|\s+perl5$/ or $_ =~ /^perl5\s+\|\s+perl\s+/ )
-			{ $_ = "perl5"; }
-		for (split(/\s*\|\s*/, $_)) { push(@alts, Dep::Pred($_)); }
-		if (@alts == 1) { push(@deps, $alts[0]); } 
-		else { push(@deps, ['OR', @alts]); }
+sub parse {
+    my @deps;
+    for (split(/\s*,\s*/, $_[0])) {
+	my @alts;
+	if (/^perl\s+\|\s+perl5$/ or /^perl5\s+\|\s+perl\s+/) {
+	    $_ = 'perl5';
+	}
+	for (split(/\s*\|\s*/, $_)) {
+	    push(@alts, Dep::Pred($_));
+	}
+	if (@alts == 1) {
+	    push(@deps, $alts[0]);
+	} else {
+	    push(@deps, ['OR', @alts]);
+	}
     }
     return $deps[0] if @deps == 1;
     return ['AND', @deps];
 }
 
+# Take the internal format and convert it back to text.  Note that what this
+# generates for NOT isn't valid Debian dependency syntax.
+sub unparse {
+    my ($p) = @_;
+    if ($p->[0] eq 'PRED') {
+	my $text = $p->[1];
+	if (defined $p->[2]) {
+	    $text .= " ($p->[2] $p->[3])";
+	}
+	if (defined $p->[4]) {
+	    $text .= " [$p->[4]]";
+	}
+	return $text;
+    } elsif ($p->[0] eq 'AND' || $p->[0] eq 'OR') {
+	my $sep = ($p->[0] eq 'AND') ? ', ' : ' | ';
+	my $text = '';
+	my $i = 1;
+	while ($i < @$p) {
+	    $text .= $sep if $text;
+	    $text .= unparse($p->[$i++]);
+	}
+	return $text;
+    } elsif ($p->[0] eq 'NOT') {
+	return '! ' . unparse($p->[1]);
+    }
+    return undef;
+}
+
 # ---------------------------------
 
 # Takes two predicate formulas and returns true iff the second can be
@@ -485,20 +518,60 @@
     return ::spawn('dpkg', '--compare-versions', @_) == 0;
 }
 
+# ---------------------------------
+
+# Return a list of duplicated relations.  Each member of the list will be an
+# anonymous array holding the set of relations that are considered duplicated.
+# Two relations are considered duplicates if one implies the other.
 sub get_dups {
-    my $relation = shift;
+    my $p = shift;
 
-    if ($relation->[0] ne 'AND') {
+    if ($p->[0] ne 'AND') {
 	return ();
     }
 
-    shift @$relation;
-    my %seen;
-    foreach my $i (@$relation) {
-	next if ($i->[0] eq 'OR');  #assume OR is always ok
-	$seen{$i->[1]}++;
+    # The logic here is a bit complex in order to merge sets of duplicate
+    # dependencies.  We want foo (<< 2), foo (>> 1), foo (= 1.5) to end up as
+    # one set of dupliactes, even though the first doesn't imply the second.
+    #
+    # $dups holds a hash, where the key is the earliest dependency in a set
+    # and the value is a hash whose keys are the other dependencies in the
+    # set.  $seen holds a map from package names to the duplicate sets that
+    # they're part of, if they're not the earliest package in a set.  If
+    # either of the dependencies in a duplicate pair were already seen, add
+    # the missing one of the pair to the existing set rather than creating a
+    # new one.
+    my (%dups, %seen);
+    for (my $i = 1; $i < @$p; $i++) {
+	for (my $j = $i + 1; $j < @$p; $j++) {
+	    if (Dep::implies($p->[$i], $p->[$j]) || Dep::implies($p->[$j], $p->[$i])) {
+		my $first = unparse($p->[$i]);
+		my $second = unparse($p->[$j]);
+		if ($seen{$first}) {
+		    $dups{$seen{$first}}->{$second} = $j;
+		    $seen{$second} = $seen{$first};
+		} elsif ($seen{$second}) {
+		    $dups{$seen{$second}}->{$first} = $i;
+		    $seen{$first} = $seen{$second};
+		} else {
+		    $dups{$first} ||= {};
+		    $dups{$first}->{$second} = $j;
+		    $seen{$second} = $first;
+		}
+	    }
+	}
     }
-    return grep {$seen{$_} > 1} keys(%seen);
+
+    # The sort maintains the original order in which we encountered the
+    # dependencies, just in case that helps the user find the problems,
+    # despite the fact we're using a hash.
+    return map {
+        [ $_,
+          sort {
+              $dups{$_}->{$a} <=> $dups{$_}->{$b}
+          } keys %{ $dups{$_} }
+        ]
+    } keys %dups;
 }
 
 # ---------------------------------

Modified: trunk/testset/relations/debian/control
===================================================================
--- trunk/testset/relations/debian/control	2006-09-24 04:23:11 UTC (rev 750)
+++ trunk/testset/relations/debian/control	2006-09-24 05:58:23 UTC (rev 751)
@@ -2,7 +2,10 @@
 Section: misc
 Priority: optional
 Build-Depends: mail-transport-agent, libc6-dev, findutils, foo (>> 2) [!amd64 !i386], bar, arch-test1 [i386], arch-test2 [!i386], quilt (>= 0.40), perl
-Build-Depends-Indep: make, bash, debmake, build-essential
+Build-Depends-Indep: make, bash, debmake, build-essential, baz (= 2.0),
+  baz (<= 2.0), baz, car (>= 1.0), car (<= 2.0), caz (= 1.0) [amd64],
+  caz (>= 2.0) [i386], caz (= 2.0) [i386 powerpc], perl (>= 5.0),
+  foo (<< 4) [!amd64 !i386], foo (= 3) [!amd64 !i386]
 Build-Conflicts: foo [amd64 i386], bar [alpha test], xlibs-dev, arch-test1 [powerpc], arch-test2 [!sparc]
 Build-Conflicts-Indep: debmake [!powerpc]
 Maintainer: Debian QA Group <packages@qa.debian.org>

Modified: trunk/testset/tags.cdbs-test
===================================================================
--- trunk/testset/tags.cdbs-test	2006-09-24 04:23:11 UTC (rev 750)
+++ trunk/testset/tags.cdbs-test	2006-09-24 05:58:23 UTC (rev 751)
@@ -3,3 +3,4 @@
 E: cdbs-test source: package-lacks-versioned-build-depends-on-debhelper 5
 I: cdbs-test source: build-depends-without-arch-dep yada
 W: cdbs-test source: native-package-with-dash-version
+W: cdbs-test source: package-has-a-duplicate-build-relation cdbs, cdbs

Modified: trunk/testset/tags.relations
===================================================================
--- trunk/testset/tags.relations	2006-09-24 04:23:11 UTC (rev 750)
+++ trunk/testset/tags.relations	2006-09-24 05:58:23 UTC (rev 751)
@@ -4,7 +4,7 @@
 E: relations source: build-depends-on-essential-package-without-using-version build-depends-indep: bash
 E: relations source: build-depends-on-essential-package-without-using-version build-depends: findutils
 E: relations source: build-depends-on-obsolete-package build-depends-indep: debmake
-E: relations source: debian-control-with-duplicate-fields provides: 32, 33
+E: relations source: debian-control-with-duplicate-fields provides: 35, 36
 E: relations source: depends-on-build-essential-package-without-using-version libc6-dev [build-depends: libc6-dev]
 E: relations source: depends-on-build-essential-package-without-using-version make [build-depends-indep: make]
 E: relations source: invalid-arch-string-in-source-relation test [build-conflicts: bar [alpha test]]
@@ -34,6 +34,10 @@
 I: relations: unknown-field-in-control bugs
 I: relations: unknown-field-in-control origin
 W: relations source: ancient-standards-version 3.1.1 (current is 3.7.2)
+W: relations source: package-has-a-duplicate-build-relation baz (= 2.0), baz (<= 2.0), baz
+W: relations source: package-has-a-duplicate-build-relation caz (>= 2.0) [i386], caz (= 2.0) [i386 powerpc]
+W: relations source: package-has-a-duplicate-build-relation foo (>> 2) [!amd64 !i386], foo (<< 4) [!amd64 !i386], foo (= 3) [!amd64 !i386]
+W: relations source: package-has-a-duplicate-build-relation perl, perl (>= 5.0)
 W: relations source: redundant-origin-field
 W: relations source: virtual-package-depends-without-real-package-depends build-depends: mail-transport-agent
 W: relations-multiple-libs: description-synopsis-might-not-be-phrased-properly



Reply to: