Bug#460350: RFR/ITC: Default options in lintianrc
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 <niels@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 <niels@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 to: