On Mon, Mar 15, 2004 at 05:10:15PM +0100, Marc 'HE' Brockschmidt wrote:
> I looked through the lintian bug list and (after that) read some
> source code a few days ago. The check/fields script seemed to be pretty
> unmaintainable (the signs were things like a large, unreachable chunk of
> code) and i wanted to change that.
> 
> I've rewritten the thing (more or less) from scratch, but only had time
> for a few checks (meaning "everything seems to work as it should with
> two really broken packages").
> 
> Please review/test.

I reviewed most of your new check (its too late at night for the
dependency check things ;). Attached is a review, mostly FIXME
statements and a few simple bug fixes.

A good start but needs further work.

Gruesse,
-- 
Frank Lichtenheld <[EMAIL PROTECTED]>
www: http://www.djpig.de/
--- /home/djpig/fields  Sun Mar 28 01:54:46 2004
+++ fields.new  Sun Mar 28 01:54:07 2004
@@ -27,7 +27,7 @@
 use lib "$ENV{'LINTIAN_ROOT'}/checks/";
 use common_data;
 
-($#ARGV == 1) or fail("syntax: deb-format <pkg> <type>");
+($#ARGV == 1) or fail("syntax: fields.new <pkg> <type>");
 my $pkg = shift;
 my $type = shift;
 my $version;
@@ -71,6 +71,9 @@
        } else {
                print "E: $pkg $type: bad-version-number\n";
        }
+
+       #FIXME: checks for "If there is no debian_revision then hyphens are not 
allowed; if there is no epoch then colons are not allowed."
+       #FIXME: check for "[epoch:] This is a _single_ (generally small) 
unsigned integer"
 }
 
 #---- Architecture
@@ -90,6 +93,7 @@
        }
 
        for my $arch (@archs) {
+           # FIXME: don't ever hide lists of allowed/known things in the code
                unless (grep { $_ eq $arch } qw(all alpha any arm hppa 
hurd-i386 i386 ia64 m68k mips mipsel powerpc s390 sh sparc)) {
                        print "E: $pkg $type: unknown-architecture\n";
                }
@@ -101,6 +105,8 @@
        }
 }
 
+#FIXME: subarchitecture for udebs
+
 #---- Maintainer
 #---- Uploader
 
@@ -126,6 +132,7 @@
 #---- Source
 
 if (not open (FH, "fields/source")) {
+    #FIXME: binary packages don't need a source field
        print "E: $pkg $type: no-source-field\n";
 } else {
        my $source = <FH>;
@@ -138,6 +145,7 @@
                        print "E: $pkg $type: 
source-field-does-not-match-pkg-name $_\n";
                }
        } else {
+#FIXME: split out regexes that are used more than once
                if ($source !~ /[A-Z0-9][-+\.A-Z0-9]+                      
#Package name
                                \s*                                        
                                
(?:\((?:\d+:)?(?:[-\.+:A-Z0-9]+?)(?:-[\.+A-Z0-9]+)?\))?\s*$/ix) { #Version
@@ -156,8 +164,8 @@
 
        print "E: $pkg $type: essential-in-source-package\n" if ($type eq 
"source");
        print "E: $pkg $type: essential-no-not-needed\n" if ($essential eq 
"no");
-       print "E: $pkg $type: unknown-essential-value\n" if ($essential ne "no" 
&& $essential ne "yes");
-       print "W: $pkg $type: new-essential-package\n" if ($essential eq "yes" 
&& ! $known_essential{$pkg});
+       print "E: $pkg $type: unknown-essential-value\n" if ($essential ne "no" 
and $essential ne "yes");
+       print "W: $pkg $type: new-essential-package\n" if ($essential eq "yes" 
and ! $known_essential{$pkg});
 }
 
 #---- Section
@@ -174,6 +182,7 @@
        print "E: $pkg $type: non-us-spelling\n" if ($1 && $1 ne "non-US");
        $section =~ s!^(non-free|contrib)/!!;
 
+#FIXME: doesn't handles contrib/non-free
        print "W: $pkg $type: unknown-section $section\n" unless 
$known_sections{$section};
 }
 
@@ -192,7 +201,8 @@
 
 #---- Package relations (binary package)
 
-if ($type eq "binary") {
+#TODO: not fully checked by me, yet
+if (($type eq "binary") || ($type eq 'udeb')) {
        for my $field (qw(depends pre-depends recommends suggests conflicts 
provides replaces)) {
                if (open(FH, "fields/$field")) {
                        #Get data and clean it
@@ -202,6 +212,7 @@
 
                        my (@seen_libstdcs, @seen_tcls, @seen_tclxs, @seen_tks, 
@seen_tkxs, @seen_libpngs);
 
+#FIXME: this check is fully broken, or better: where is the check?
                        print "E: $pkg $type: alternates-not-allowed $field\n"
                            if (! grep { $_ eq $field } qw(depends pre-depends 
recommends suggests));
 
@@ -273,7 +284,7 @@
 }
 
 #---- Package relations (source package)
-
+#TODO: not fully checked by me, yet
 if ($type eq "source") {
        for my $field (qw(build-depends build-depends-indep build-conflicts 
build-conflicts-indep)) {
                if (open(FH, "fields/$field")) {
@@ -290,6 +301,7 @@
                                        my ($d_pkg, $d_version, $d_arch, $rest, 
$part_d_orig) = @$part_d;
 
                                        for my $arch (@{$d_arch->[0]}) {
+#FIXME: valid arch list in code, don't do it
                                                print "E: $pkg $type: 
invalid-arch-string-in-source-relation $arch [$field: $part_d_orig]\n"
                                                    unless (grep { $arch eq $_ 
} qw(alpha arm hppa hurd-i386 i386 ia64 m68k mips mipsel powerpc s390 sh 
sparc));
                                        }
@@ -302,6 +314,10 @@
        }
 }
 
+#FIXME: where did build-depends-indep-without-arch-indep go?
+#FIXME: Standards-Version is recommended, should give an error
+#FIXME: Merge the installer-menu-item check in
+
 #----- Field checks (without checking the value)
 
 for my $field (glob("fields/*")) {
@@ -315,6 +331,7 @@
 
        print "N: $pkg $type: unknown-field-in-changes\n"
            if ($type eq "binary" && ! $known_binary_fields{$field});
+#FIXME: merge the udeb changes in
 }
 
 exit 0;
@@ -383,7 +400,7 @@
 
        if (! $name) {
                print "E: $pkg $type: $f-name-missing\n";
-       } elsif ($name =~ /^\S+\s+\S+/) {
+       } elsif ($name !~ /^\S+\s+\S+/) {
                print "W: $pkg $type: $f-not-full-name\n";
        }
                        

Reply via email to