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

Re: checks/fields rewrite



Frank Lichtenheld <djpig@debian.org> writes:
> 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.

OK, let's see...

> +	#FIXME: checks for "If there is no debian_revision then hyphens are not allowed; if there is no epoch then colons are not allowed."

We can't check that. If the upstream version contains a hyphen and there
is no debian revision, the part of the upstream version is parsed as
debian revision. That's the way apt and the others work, and there is no
way around that problem. Same for epoch.
One *could* warn about weird looking debian_revisions, but OTOH there
are packages with an upstream version X.Y-1 (where the check wouldn't
warn) and backports use really weird debian revision, so lintian would
warn there. No, too many false positives.

> +	#FIXME: check for "[epoch:] This is a _single_ (generally small) unsigned integer"

An integer is not a single _digit_, \d+ is IMO ok.

>  #---- Architecture
> @@ -90,6 +93,7 @@
>  	}
>  
>  	for my $arch (@archs) {
> +	    # FIXME: don't ever hide lists of allowed/known things in the code

OK, fixed, is now using %known_archs form common_data.

> +#FIXME: subarchitecture for udebs
> +

I have to look into those changes...

>  if (not open (FH, "fields/source")) {
> +    #FIXME: binary packages don't need a source field

Fixed.

>  	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

It's used once (or do you mean the first part for the package name)?

>  #---- 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

It does, i remove the contrib/non-free prefix if it's there (see the
line of code above your comment)
  
> @@ -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?

Don't understand what you mean. The @seen_* arrays are used *after* the
loop.

>  					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));

Fixed.

> +#FIXME: where did build-depends-indep-without-arch-indep go?

*Argh*. Removed it together with build-depends-without-arch-dep.

> +#FIXME: Standards-Version is recommended, should give an error

That'd be a new check, i wanted to replace the old script first.

I've attached a new version.

Marc
-- 
$_=')(hBCdzVnS})3..0}_$;//::niam/s~=)]3[))_$(rellac(=_$({pam(esrever })e$.)4/3*
)e$(htgnel+23(rhc,"u"(kcapnu ,""nioj ;|_- |/+9-0z-aZ-A|rt~=e$;_$=e${pam tnirp{y
V2ajFGabus} yV2ajFGa&{gwmclBHIbus}gwmclBHI&{yVGa09mbbus}yVGa09mb&{hBCdzVnSbus';
s/\n//g;s/bus/\nbus/g;eval scalar reverse   # <mailto:marc@marcbrockschmidt.de>

Attachment: fields.new
Description: Binary data

Attachment: pgp7X3U8yRxes.pgp
Description: PGP signature


Reply to: