On 2011-06-05 10:30, Niels Thykier wrote:
> On 2011-05-30 02:11, Niels Thykier wrote:
>
> I have invoked "do-cracy" on this (though I am still option to
> suggestions on this) and here is what I got so far.
> 

Hey

More do-cracy[1].

In case you do not know or have not guessed what I meant with RFR and
ITC, then it is "Request For Review" and "Intend To Commit" (respectively).
  There are a TODO marker in the patch that I will do before committing
(possibly in a separate commit); the only other change I can think of
would be requiring a "--no-cfg" (to be used by our test-suite) and
possibly some "--no-X" options.

[1] Okay - I admit I was bored.

>> I have been looking at #460350 (plus clones) and was thinking maybe we
>> could do something similar to dpkg-source and its
>> debian/source/{local-,}options files.  That is, we allow options to be
>> listed in lintianrc (without leading "--" and only in long-form etc.)
>>
> 
> I have been using this approach; the prototype only accepts a very small
> (and not necessarily useful) subset of the boolean options.
>   Unfortunately --display-info is not one of them, as it is internally
> mapped as a -L option.
> 
> The options supported by this patch are:
>  * info
>  * display-experimental
>  * no-override
>  * show-overrides
> 

Extended to also include:
 * color
 * display-info
 * display-level
 * fail-on-warnings
 * pedantic

> There is a little print(f) debug to STDERR that informs you what the
> final value of these options are.  I will remove that before merging
> this into the master branch.
> 

Still there

>> Since lintianrc is currently used for setting variables, I suggest we
>> keep backwards compatibility; any line starting with "LINTIAN_" will be
>> read as an "old" variable and let it be translated to the relevant
>> option (e.g. LINTIAN_ROOT -> root).
>>
> 
> No changes here (namely because the affected parts are not boolean options).
> 

So, I learned that LINTIAN_ROOT cannot be changed in the config file.
For now I have just left the only variables in place and not bothered to
add code to replace them with options.
  Unless there are any request for this I will probably leave it as this
for now (unless we decide to deprecate --checksums anyway - see #629453)

>> Format changes of lintianrc aside, we will also need to refactor the
>> frontend.  Here I propose that we delay interpretation of options (save
>> for stuff like --cfg); the exact idea I had in mind was to stuff all
>> options (with value) into a hash (GetOptions can do that for us as I
>> recall) and as we parse the lintianrc file, we will options from there
>> to the hash (as appropriate).
>>
> 
> So far there not been a need to really delay interpretation; since the
> patch only deals with boolean values (all of which defaulted to
> false/off); that being said, I do smell complications with the -L.


Turns out that there is hardly any validation (that had to be
relocated).  Moving the validation of $color was all so far.

>   As far as I can tell --display-info is the only option that maps to
> -L/--display-level; but --display-level is sensitive to the order.  That
> being said; I strongly suspect that the amount of people using -L is
> very limited.
>   Asking them to add a >=wishlish to their display-level option instead
> of using display-info in the config seems like a reasonable solution to
> the order problem.  Particularly we can disallow the usage of both
> display-info and display-level in the config at the same time.
> 
> I think it makes sense to ignore display-level and display-info, if
> either --display-info or --display-level was passed on command-line.
> 
> For the parsing of display-level in the config, I am thinking of using a
> space separation, so:
> 
>   -L ">=important" -L "+>=normal/possible" -L +minor/certain
> 
> becomes
> 
>   display-level= >=important +>=normal/possible +minor/certain
> 

Done this.

> 
> I also think we should disallow the usage of dont-check-part,
> tags-from-file, ftp-master-rejects, check-part and
> suppress-tags{,-from-file} and rely on people to create profiles for that.
>   I would also disallow actions (unpack, check etc.) since I do not
> really think they make sense to have in the rc file.
> 

Also done (since they would have to be explicitly added to the
"%cfghash" to be allowed).

Unless anyone feels there are any complications I will probably commit
this in two-three days time.

Thank you in advance,
~Niels

>From cb7905634f1844672d811ca6ca31d0ee9c22c378 Mon Sep 17 00:00:00 2001
From: Niels Thykier <ni...@thykier.net>
Date: Mon, 6 Jun 2011 22:49:15 +0200
Subject: [PATCH] Allow some options to appear in lintianrc

This patch allows a subset of the lintian options to appear in
the lintianrc file.  All options in the config file are
currently overrules if the same option are passed via the
command-line.

The only "special" options are display-info and display-level,
which (unlike via command-line) cannot both appear in the
lintianrc.

If an option appears twice in the lintianrc, only the first
value is considered.

TODO BEFORE PUSHING TO MASTER BRANCH:
 - update documentation (manpage and manual)
 - remove print(f) debug in f/lintian

Signed-off-by: Niels Thykier <ni...@thykier.net>
---
 frontend/lintian |  137 ++++++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 108 insertions(+), 29 deletions(-)

diff --git a/frontend/lintian b/frontend/lintian
index 1fff26e..816c32a9 100755
--- a/frontend/lintian
+++ b/frontend/lintian
@@ -47,15 +47,9 @@ my $quiet = 0;			#flag for -q|--quiet switch
 my $debug = 0;
 my $check_everything = 0;	#flag for -a|--all switch
 my $lintian_info = 0;		#flag for -i|--info switch
-our $display_experimentaltags = 0; #flag for -E|--display-experimental switch
-our $display_pedantictags = 0;	#flag for --pedantic switch
 our $ftpmaster_tags = 0;	#flag for -F|--ftp-master-rejects switch
-our $no_override = 0;		#flag for -o|--no-override switch
-our $show_overrides = 0;	#flag for --show-overrides switch
-my $color = 'never';		#flag for --color switch
 my $check_checksums = 0;	#flag for -m|--md5sums|--checksums switch
 my $allow_root = 0;		#flag for --allow-root switch
-my $fail_on_warnings = 0;       #flag for --fail-on-warnings switch
 my $keep_lab = 0;		#flag for --keep-lab switch
 my $packages_file = 0;		#string for the -p option
 our $OPT_LINTIAN_LAB = '';	#string for the --lab option
@@ -66,6 +60,8 @@ our $OPT_LINTIAN_AREA = '';	#string for the --area option
 # These options can also be used via default or environment variables
 our $LINTIAN_CFG = '';		#config file to use
 our $LINTIAN_ROOT;		#location of the lintian modules
+my %opt;                        #hash of some flags from cmd or cfg
+my %conf_opt;                   #names of options set in the cfg file
 
 my $experimental_output_opts = undef;
 
@@ -354,6 +350,36 @@ sub record_display_source {
     $display_source{$_[1]} = 1;
 }
 
+# Process display-info and display-level options in cfg files
+#  - dies if display-info and display-level are used together
+#  - adds the relevant display level unless the command-line
+#    added something to it.
+#  - uses @display_level to track cmd-line appearences of
+#    --display-level/--display-info
+sub cfg_display_level {
+    my ($var, $val) = @_;
+    if ($var eq 'display-info'){
+	die "display-info and display-level may not both appear in the config file.\n"
+	    if $conf_opt{'display-level'};
+
+	return unless $val; # case "display-info=no"
+	push @display_level, [ '+', '>=', 'wishlist' ] unless @display_level;
+    } elsif ($var eq 'display-level'){
+	die "display-info and display-level may not both appear in the config file.\n"
+	    if $conf_opt{'display-info'};
+
+	return if @display_level;
+	$val =~ s/^\s++//;
+	$val =~ s/\s++$//;
+	print STDERR "N: display-level = $val\n";
+	foreach my $dl (split m/\s++/, $val) {
+	    print STDERR "N: display-level -> $dl\n";
+	    record_display_level('display-level', $dl);
+	}
+    }
+
+}
+
 # Hash used to process commandline options
 my %opthash = (			# ------------------ actions
 	       'setup-lab|S' => \&record_action,
@@ -377,21 +403,21 @@ my %opthash = (			# ------------------ actions
 	       'quiet|q' => \$quiet,
 
 	       # ------------------ behaviour options
-	       'info|i' => \$lintian_info,
+	       'info|i' => \$opt{'info'},
 	       'display-info|I' => \&display_infotags,
-	       'display-experimental|E' => \$display_experimentaltags,
-	       'pedantic' => \$display_pedantictags,
+	       'display-experimental|E' => \$opt{'display-experimental'},
+	       'pedantic' => \$opt{'pedantic'},
 	       'display-level|L=s' => \&record_display_level,
 	       'display-source=s' => \&record_display_source,
 	       'suppress-tags=s' => \&record_suppress_tags,
 	       'suppress-tags-from-file=s' => \&record_suppress_tags_from_file,
-	       'no-override|o' => \$no_override,
-	       'show-overrides' => \$show_overrides,
-	       'color=s' => \$color,
+	       'no-override|o' => \$opt{'no-override'},
+	       'show-overrides' => \$opt{'show-overrides'},
+	       'color=s' => \$opt{'color'},
 	       'unpack-info|U=s' => \&record_unpack_info,
 	       'checksums|md5sums|m' => \$check_checksums,
 	       'allow-root' => \$allow_root,
-	       'fail-on-warnings' => \$fail_on_warnings,
+	       'fail-on-warnings' => \$opt{'fail-on-warnings'},
 	       'keep-lab' => \$keep_lab,
 
 	       # Note: Ubuntu has (and other derivatives might gain) a
@@ -419,6 +445,19 @@ my %opthash = (			# ------------------ actions
 	       'exp-output:s' => \$experimental_output_opts,
 	      );
 
+# Options that can appear in the config file
+my %cfghash = (
+	       'color'                => \$opt{'color'},
+	       'display-experimental' => \$opt{'display-experimental'},
+	       'display-info'         => \&cfg_display_level,
+	       'display-level'        => \&cfg_display_level,
+	       'fail-on-warnings'     => \$opt{'fail-on-warnings'},
+	       'info'                 => \$opt{'info'},
+	       'pedantic'             => \$opt{'pedantic'},
+	       'no-override'          => \$opt{'no-override'},
+	       'show-overrides'       => \$opt{'show-overrides'},
+    );
+
 # init commandline parser
 Getopt::Long::config('bundling', 'no_getopt_compat', 'no_auto_abbrev');
 
@@ -447,11 +486,6 @@ if (($check_everything or $packages_file) and $#ARGV+1 > 0) {
     undef $packages_file;
 }
 
-# check permitted values for --color
-if ($color and $color !~ /^(?:never|always|auto|html)$/) {
-    die "invalid argument to --color: $color\n";
-}
-
 # check specified action
 $action = 'check' unless $action;
 
@@ -509,12 +543,57 @@ if ($LINTIAN_CFG) {
 	    }
 	}
 	unless ($found) {
+	    # check if it is a config option
+	    if (m/^\s*([-a-z]+)\s*=\s*(.*\S)\s*$/o){
+		my ($var, $val) = ($1, $2);
+		my $ref = $cfghash{$var};
+		die "Unknown configuration variable $var at line: ${.}.\n"
+		    unless $ref;
+		if (exists $conf_opt{$var}){
+		    print STDERR "Configuration variable $var appears more than once\n";
+		    print STDERR " in $LINTIAN_CFG (line: $.) - Using the first value!\n";
+		    next;
+		}
+		$conf_opt{$var} = 1;
+		$found = 1;
+		if ($val =~ m/^y(?:es)|true$/o){
+		    $val = 1;
+		} elsif ($val =~ m/^no?|false$/o){
+		    $val = 0;
+		}
+		if (ref $ref eq 'SCALAR'){
+		    # Check it was already set
+		    next if defined $$ref;
+		    $$ref = $val;
+		} elsif (ref $ref eq 'CODE'){
+		    $ref->($var, $val);
+		}
+
+	    }
+	}
+	unless ($found) {
 	    die "syntax error in configuration file: $_\n";
 	}
     }
     close(CFG);
 }
 
+## REMOVE THIS - for debugging only
+foreach my $var (sort keys %cfghash){
+    my $val = $opt{$var}//'<unset>';
+    $val = '<N/A - code-handler>'
+	if ref $cfghash{$var} eq 'CODE' && $conf_opt{$var};
+    print STDERR "N: $var -> $val\n";
+}
+
+# check permitted values for --color / color
+#  - We set the default to 'never' here; because we cannot do
+#    it before the config check.
+$opt{'color'} = 'never' unless defined $opt{'color'};
+if ($opt{'color'} and $opt{'color'} !~ /^(?:never|always|auto|html)$/) {
+    die "The color value must be one of \"never\", \"always\", \"auto\" or \"html\"\n";
+}
+
 # environment variables overwrite settings in conf file:
 foreach (VARS) {
     no strict 'refs';
@@ -605,8 +684,8 @@ if (defined $experimental_output_opts) {
 $Lintian::Output::GLOBAL->verbose($verbose);
 $Lintian::Output::GLOBAL->debug($debug);
 $Lintian::Output::GLOBAL->quiet($quiet);
-$Lintian::Output::GLOBAL->color($color);
-$Lintian::Output::GLOBAL->showdescription($lintian_info);
+$Lintian::Output::GLOBAL->color($opt{'color'});
+$Lintian::Output::GLOBAL->showdescription($opt{'info'});
 
 # Now that we can load the data, process the -F or --ftp-master-rejects
 # option.
@@ -630,9 +709,9 @@ debug_msg(1,
     );
 
 our $TAGS = Lintian::Tags->new;
-$TAGS->show_experimental($display_experimentaltags);
-$TAGS->show_pedantic($display_pedantictags);
-$TAGS->show_overrides($show_overrides);
+$TAGS->show_experimental($opt{'display-experimental'});
+$TAGS->show_pedantic($opt{'pedantic'});
+$TAGS->show_overrides($opt{'show-overrides'});
 $TAGS->sources(keys %display_source) if %display_source;
 $TAGS->only(split(/,/, $check_tags)) if defined $check_tags;
 $TAGS->suppress(keys %suppress_tags) if %suppress_tags;
@@ -985,7 +1064,7 @@ if ($action eq 'unpack') {
 my $map = Lintian::DepMap::Properties->new();
 my $collmap = Lintian::DepMap::Properties->new();
 
-unless ($no_override) {
+unless ($opt{'no-override'}) {
     # add the override-file collection
     $map->add('coll-override-file', {'type' => 'collection', 'name' =>  'override-file'});
     $collmap->add('coll-override-file', {'type' => 'collection', 'name' =>  'override-file'});
@@ -1045,7 +1124,7 @@ foreach my $gname (sort $pool->get_group_names()) {
 
 $TAGS->file_end();
 
-if ($action eq 'check' and not $no_override and not $show_overrides) {
+if ($action eq 'check' and not $opt{'no-override'} and not $opt{'show-overrides'}) {
     my $errors = $overrides{errors} || 0;
     my $warnings = $overrides{warnings} || 0;
     my $info = $overrides{info} || 0;
@@ -1267,7 +1346,7 @@ sub auto_clean_package {
 sub post_pkg_process_overrides{
     my ($pkg_path) = @_;
     # report unused overrides
-    if (not $no_override) {
+    if (not $opt{'no-override'}) {
 	my $overrides = $TAGS->overrides($pkg_path);
 
 	for my $tag (sort keys %$overrides) {
@@ -1287,7 +1366,7 @@ sub post_pkg_process_overrides{
     }
 
     # Report override statistics.
-    if (not $no_override and not $show_overrides) {
+    if (not $opt{'no-override'} and not $opt{'show-overrides'}) {
 	my $stats = $TAGS->statistics($pkg_path);
 	my $errors = $stats->{overrides}{types}{E} || 0;
 	my $warnings = $stats->{overrides}{types}{W} || 0;
@@ -1467,7 +1546,7 @@ sub process_group {
 	    next;
 	}
 
-	unless ($no_override) {
+	unless ($opt{'no-override'}) {
 	    if ($collmap->done('coll-override-file')) {
 		debug_msg(1, 'Override file collected, loading it ...');
 		$TAGS->file_overrides("$base/override")
@@ -1511,7 +1590,7 @@ sub process_group {
 	    my $stats = $TAGS->statistics($pkg_path);
 	    if ($stats->{types}{E}) {
 		$exit_code = 1;
-	    } elsif ($fail_on_warnings && $stats->{types}{W}) {
+	    } elsif ($opt{'fail-on-warnings'} && $stats->{types}{W}) {
 		$exit_code = 1;
 	    }
 	}
-- 
1.7.4.4

Reply via email to