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

Re: checks/fields rewrite



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 <djpig@debian.org>
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 to: