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