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

Bug#460174: [lintian] patch



Package: lintian
Version: 2.5.14
control: tags -1 + patch

Please review the following patches
From e765d57be8f6c18d5d68811766ac87f5de5c0736 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastien=20ROUCARI=C3=88S?= <roucaries.bastien@gmail.com>
Date: Wed, 28 Aug 2013 15:02:10 +0200
Subject: [PATCH 1/3] Factorize the rules check between required and
 recommanded

Use a single hash for the rules checking between required and recommended.
---
 checks/rules.pm |   46 ++++++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/checks/rules.pm b/checks/rules.pm
index bfb667c..ae272c5 100644
--- a/checks/rules.pm
+++ b/checks/rules.pm
@@ -17,6 +17,7 @@ package Lintian::rules;
 use strict;
 use warnings;
 use autodie;
+use Carp qw(croak);
 
 use List::MoreUtils qw(any);
 
@@ -94,10 +95,13 @@ my @RULE_CLEAN_DEPENDS =(
 );
 
 # The following targets are required per Policy.
-my %required = map { $_ => 1 } qw(build binary binary-arch binary-indep clean);
+my %required = map { $_ => 'required' } qw(build binary binary-arch binary-indep clean);
 
 # The following targets are recommended per Policy.
-my %recommended = map { $_ => 1 } qw(build-arch build-indep);
+my %recommended = map { $_ => 'recommended' } qw(build-arch build-indep);
+
+# The following rules are required or recommanded per policy
+my %policyrules = ( %required, %recommended);
 
 # Rules about required debhelper command ordering.  Each command is put into a
 # class and the tag is issued if they're called in the wrong order for the
@@ -167,8 +171,7 @@ sub run {
             my $targets = $KNOWN_MAKEFILES->value($makefile);
             if (defined $targets){
                 foreach my $target (split m/\s*+,\s*+/o, $targets){
-                    $seen{$target}++ if $required{$target};
-                    $seen{$target}++ if $recommended{$target};
+                    $seen{$target}++ if $policyrules{$target};
                 }
             } else {
                 $includes = 1;
@@ -257,15 +260,13 @@ sub run {
                         # we ought to "delay" it was a "=" variable rather
                         # than ":=" or "+=".
                         for (split m/\s++/o, rstrip($val)) {
-                            $seen{$_}++ if $required{$_};
-                            $seen{$_}++ if $recommended{$_};
+                            $seen{$_}++ if $policyrules{$_};
                         }
                         last;
                     }
                     # We don't know, so just mark the target as seen.
                 }
-                $seen{$_}++ if $required{$_};
-                $seen{$_}++ if $recommended{$_};
+                $seen{$_}++ if $policyrules{$_};
             }
             next; #.PHONY implies the rest will not match
         }
@@ -282,11 +283,8 @@ sub run {
                 if ($target =~ m/%/o) {
                     my $pattern = quotemeta $target;
                     $pattern =~ s/\\%/.*/g;
-                    for my $required (keys %required) {
-                        $seen{$required}++ if $required =~ m/$pattern/;
-                    }
-                    for my $recommended (keys %recommended) {
-                        $seen{$recommended}++ if $recommended =~ m/$pattern/;
+                    for my $policyrules (keys %policyrules) {
+                        $seen{$policyrules}++ if $policyrules =~ m/$pattern/;
                     }
                 } else {
                     # Is it $(VAR) ?
@@ -299,15 +297,13 @@ sub run {
                             # than ":=" or "+=".
                             local $_;
                             for (split m/\s++/o, rstrip($val)) {
-                                $seen{$_}++ if $required{$_};
-                                $seen{$_}++ if $recommended{$_};
+                                $seen{$_}++ if $policyrules{$_};
                             }
                             last;
                         }
                         # We don't know, so just mark the target as seen.
                     }
-                    $seen{$target}++ if $required{$target};
-                    $seen{$target}++ if $recommended{$target};
+                    $seen{$target}++ if $policyrules{$target};
                 }
                 if (any { $target =~ /$_/ } @arch_rules) {
                     push(@arch_rules, @depends);
@@ -365,14 +361,16 @@ sub run {
     unless ($includes) {
         my $rec = 0;
         # Make sure all the required rules were seen.
-        for my $target (sort keys %required) {
-            tag 'debian-rules-missing-required-target', $target
-              unless $seen{$target};
-        }
-        for my $target (sort keys %recommended) {
+        for my $target (sort keys %policyrules) {
             unless ($seen{$target}) {
-                tag 'debian-rules-missing-recommended-target', $target;
-                $rec++;
+                if($policyrules{$target} eq 'required') {
+                    tag 'debian-rules-missing-required-target', $target;
+                } elsif ($policyrules{$target} eq 'recommended') {
+                    tag 'debian-rules-missing-recommended-target', $target;
+                    $rec++;
+                } else {
+                    croak 'unknown type of policy rules';
+                }
             }
         }
 
-- 
1.7.10.4

From 24b33f1b2418919648223fc76fce75fae40e8e37 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastien=20ROUCARI=C3=88S?= <roucaries.bastien@gmail.com>
Date: Wed, 28 Aug 2013 15:43:02 +0200
Subject: [PATCH 2/3] Add a check for  get-orig-source

This will close #460174.
---
 checks/rules.desc                  |    3 +++
 checks/rules.pm                    |   23 ++++++++++++++++-------
 t/tests/rules-missing-targets/desc |    3 ++-
 t/tests/rules-missing-targets/tags |    1 +
 4 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/checks/rules.desc b/checks/rules.desc
index 5a9dfe7..1e9fcd0 100644
--- a/checks/rules.desc
+++ b/checks/rules.desc
@@ -57,6 +57,9 @@ Info: The <tt>debian/rules</tt> file for this package does not provide
    build-stamp:
 	build here
  .
+ If your packaged is repacked from non dfsg source, you should add a
+ get-orig-source target.
+ .
  These targets will be required by policy in the future, so should be
  added to prevent future breakage.
 
diff --git a/checks/rules.pm b/checks/rules.pm
index ae272c5..dff9158 100644
--- a/checks/rules.pm
+++ b/checks/rules.pm
@@ -98,10 +98,12 @@ my @RULE_CLEAN_DEPENDS =(
 my %required = map { $_ => 'required' } qw(build binary binary-arch binary-indep clean);
 
 # The following targets are recommended per Policy.
-my %recommended = map { $_ => 'recommended' } qw(build-arch build-indep);
+my %recommendedbuild = map { $_ => 'recommended_allindep' } qw(build-arch build-indep);
 
-# The following rules are required or recommanded per policy
-my %policyrules = ( %required, %recommended);
+my %recommendeddfsg = map { $_ => 'recommended_dfsg' } qw(get-orig-source);
+
+# The following rules are required or recommended per policy
+my %policyrules = ( %required, %recommendedbuild, %recommendeddfsg);
 
 # Rules about required debhelper command ordering.  Each command is put into a
 # class and the tag is issued if they're called in the wrong order for the
@@ -129,6 +131,9 @@ sub run {
     }
 
     my $architecture = $info->field('architecture', '');
+    my $version = $info->field('version');
+    # If the version field is missing, we assume a neutral non-native one.
+    $version = '0-1' unless defined $version;
 
     open(my $rules_fd, '<', $rules);
 
@@ -359,22 +364,26 @@ sub run {
     close($rules_fd);
 
     unless ($includes) {
-        my $rec = 0;
+        my $rec_allindep = 0;
         # Make sure all the required rules were seen.
         for my $target (sort keys %policyrules) {
             unless ($seen{$target}) {
                 if($policyrules{$target} eq 'required') {
                     tag 'debian-rules-missing-required-target', $target;
-                } elsif ($policyrules{$target} eq 'recommended') {
+                } elsif ($policyrules{$target} eq 'recommended_allindep') {
                     tag 'debian-rules-missing-recommended-target', $target;
-                    $rec++;
+                    $rec_allindep++; 
+                } elsif ($policyrules{$target} eq 'recommended_dfsg') {
+                    if ($version =~ /(dfsg|debian|ds)/) {
+                        tag 'debian-rules-missing-recommended-target', $target;
+                    }
                 } else {
                     croak 'unknown type of policy rules';
                 }
             }
         }
 
-        if ($rec) {
+        if ($rec_allindep) {
             my $all = 0;
             my $notall = 0;
             foreach my $p ($group->get_processables) {
diff --git a/t/tests/rules-missing-targets/desc b/t/tests/rules-missing-targets/desc
index f92d7bb..97e10f0 100644
--- a/t/tests/rules-missing-targets/desc
+++ b/t/tests/rules-missing-targets/desc
@@ -1,6 +1,7 @@
 Testname: rules-missing-targets
 Sequence: 6000
-Version: 1.0
+Version: 1.0+dfsg-1
+Type: non-native
 Description: Test for missing targets in debian/rules
 Test-For:
  debian-rules-missing-recommended-target
diff --git a/t/tests/rules-missing-targets/tags b/t/tests/rules-missing-targets/tags
index ef2f56a..4ac83d5 100644
--- a/t/tests/rules-missing-targets/tags
+++ b/t/tests/rules-missing-targets/tags
@@ -1,3 +1,4 @@
 W: rules-missing-targets source: debian-rules-missing-recommended-target build-arch
 W: rules-missing-targets source: debian-rules-missing-recommended-target build-indep
+W: rules-missing-targets source: debian-rules-missing-recommended-target get-orig-source
 W: rules-missing-targets source: package-would-benefit-from-build-arch-targets
-- 
1.7.10.4

From d644762ec1e49720bfcf66accf08c893c44c3913 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastien=20ROUCARI=C3=88S?= <roucaries.bastien@gmail.com>
Date: Wed, 28 Aug 2013 19:04:40 +0200
Subject: [PATCH 3/3] Push to Lintian::data checking of rules

Push to an external data the checking of required/recommended rule.
---
 checks/rules.pm         |   34 ++++++++++++++--------------------
 data/rules/policy-rules |   15 +++++++++++++++
 2 files changed, 29 insertions(+), 20 deletions(-)
 create mode 100644 data/rules/policy-rules

diff --git a/checks/rules.pm b/checks/rules.pm
index dff9158..d3f4da3 100644
--- a/checks/rules.pm
+++ b/checks/rules.pm
@@ -34,6 +34,7 @@ our $ANYPYTHON_DEPEND
 
 my $KNOWN_MAKEFILES = Lintian::Data->new('rules/known-makefiles', '\|\|');
 my $DEPRECATED_MAKEFILES = Lintian::Data->new('rules/deprecated-makefiles');
+our $POLICYRULES = Lintian::Data->new('rules/policy-rules', qr/\s++/);
 
 # Certain build tools must be listed in Build-Depends even if there are no
 # arch-specific packages because they're required in order to run the clean
@@ -97,14 +98,6 @@ my @RULE_CLEAN_DEPENDS =(
 # The following targets are required per Policy.
 my %required = map { $_ => 'required' } qw(build binary binary-arch binary-indep clean);
 
-# The following targets are recommended per Policy.
-my %recommendedbuild = map { $_ => 'recommended_allindep' } qw(build-arch build-indep);
-
-my %recommendeddfsg = map { $_ => 'recommended_dfsg' } qw(get-orig-source);
-
-# The following rules are required or recommended per policy
-my %policyrules = ( %required, %recommendedbuild, %recommendeddfsg);
-
 # Rules about required debhelper command ordering.  Each command is put into a
 # class and the tag is issued if they're called in the wrong order for the
 # classes.  Unknown commands won't trigger this flag.
@@ -176,7 +169,7 @@ sub run {
             my $targets = $KNOWN_MAKEFILES->value($makefile);
             if (defined $targets){
                 foreach my $target (split m/\s*+,\s*+/o, $targets){
-                    $seen{$target}++ if $policyrules{$target};
+                    $seen{$target}++ if $POLICYRULES->known($target);
                 }
             } else {
                 $includes = 1;
@@ -265,13 +258,13 @@ sub run {
                         # we ought to "delay" it was a "=" variable rather
                         # than ":=" or "+=".
                         for (split m/\s++/o, rstrip($val)) {
-                            $seen{$_}++ if $policyrules{$_};
+                            $seen{$_}++ if $POLICYRULES->known($_);
                         }
                         last;
                     }
                     # We don't know, so just mark the target as seen.
                 }
-                $seen{$_}++ if $policyrules{$_};
+                $seen{$_}++ if $POLICYRULES->known($_);
             }
             next; #.PHONY implies the rest will not match
         }
@@ -288,8 +281,8 @@ sub run {
                 if ($target =~ m/%/o) {
                     my $pattern = quotemeta $target;
                     $pattern =~ s/\\%/.*/g;
-                    for my $policyrules (keys %policyrules) {
-                        $seen{$policyrules}++ if $policyrules =~ m/$pattern/;
+                    foreach(my $rulebypolicy = $POLICYRULES->all) {
+                        $seen{$rulebypolicy}++ if $rulebypolicy =~ m/$pattern/;
                     }
                 } else {
                     # Is it $(VAR) ?
@@ -302,13 +295,13 @@ sub run {
                             # than ":=" or "+=".
                             local $_;
                             for (split m/\s++/o, rstrip($val)) {
-                                $seen{$_}++ if $policyrules{$_};
+                                $seen{$_}++ if $POLICYRULES->known($_);
                             }
                             last;
                         }
                         # We don't know, so just mark the target as seen.
                     }
-                    $seen{$target}++ if $policyrules{$target};
+                    $seen{$target}++ if $POLICYRULES->known($target);
                 }
                 if (any { $target =~ /$_/ } @arch_rules) {
                     push(@arch_rules, @depends);
@@ -366,19 +359,20 @@ sub run {
     unless ($includes) {
         my $rec_allindep = 0;
         # Make sure all the required rules were seen.
-        for my $target (sort keys %policyrules) {
+        foreach my $target ($POLICYRULES->all) {
             unless ($seen{$target}) {
-                if($policyrules{$target} eq 'required') {
+                my $typerule = $POLICYRULES->value($target);
+                if($typerule eq 'required') {
                     tag 'debian-rules-missing-required-target', $target;
-                } elsif ($policyrules{$target} eq 'recommended_allindep') {
+                } elsif ($typerule eq 'recommended_allindep') {
                     tag 'debian-rules-missing-recommended-target', $target;
                     $rec_allindep++; 
-                } elsif ($policyrules{$target} eq 'recommended_dfsg') {
+                } elsif ($typerule eq 'recommended_dfsg') {
                     if ($version =~ /(dfsg|debian|ds)/) {
                         tag 'debian-rules-missing-recommended-target', $target;
                     }
                 } else {
-                    croak 'unknown type of policy rules';
+                    croak 'unknown type of policy rules: '.($typerule || '""');
                 }
             }
         }
diff --git a/data/rules/policy-rules b/data/rules/policy-rules
new file mode 100644
index 0000000..cda9397
--- /dev/null
+++ b/data/rules/policy-rules
@@ -0,0 +1,15 @@
+# add required/recommended rules
+# format is 
+# name-of-rules type
+# with type is:
+# - required for required rules
+# - recommended_allindep for recommended rules that will benefit for spliting in all/indep variant
+# - recommended_dfsg for recommended rules for non dfsg package
+build             required
+binary            required
+binary-arch       required
+binary-indep      required
+clean             required
+build-arch        recommended_allindep
+build-indep       recommended_allindep
+get-orig-source   recommended_dfsg
-- 
1.7.10.4


Reply to: