pkgsrc-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[pkgsrc/trunk]: pkgsrc/pkgtools/lintpkgsrc/files lintpkgsrc: clean up evaluat...
details: https://anonhg.NetBSD.org/pkgsrc/rev/c49bf70b0231
branches: trunk
changeset: 383152:c49bf70b0231
user: rillig <rillig%pkgsrc.org@localhost>
date: Sat Aug 13 10:23:40 2022 +0000
description:
lintpkgsrc: clean up evaluation of conditions in makefiles
Having a variable named 'false' was confusing. Having a stack of
if-states in which 0 means true and 1 means false was even more
confusing. Using these magic numbers in the debug log without any
explanation was misleading.
The code, as well as the debug log, now uses the strings 'not_yet',
'active' and 'done' for the state of the conditionals. While it still
requires a bit of thought to read the debug log, it's at least possible
now.
diffstat:
pkgtools/lintpkgsrc/files/lintpkgsrc.pl | 105 +++++++++++++-------------
pkgtools/lintpkgsrc/files/t/parse_makefile.t | 18 +++-
2 files changed, 66 insertions(+), 57 deletions(-)
diffs (220 lines):
diff -r ab85265d2802 -r c49bf70b0231 pkgtools/lintpkgsrc/files/lintpkgsrc.pl
--- a/pkgtools/lintpkgsrc/files/lintpkgsrc.pl Sat Aug 13 10:07:26 2022 +0000
+++ b/pkgtools/lintpkgsrc/files/lintpkgsrc.pl Sat Aug 13 10:23:40 2022 +0000
@@ -1,6 +1,6 @@
#!@PERL5@
-# $NetBSD: lintpkgsrc.pl,v 1.82 2022/08/13 09:33:43 rillig Exp $
+# $NetBSD: lintpkgsrc.pl,v 1.83 2022/08/13 10:23:40 rillig Exp $
# Written by David Brownlee <abs%netbsd.org@localhost>.
#
@@ -310,9 +310,7 @@
}
}
-# TODO: The word 'false' is confusing.
-sub parse_eval_make_false($line, $vars) {
- my $false = 0;
+sub eval_mk_cond($line, $vars) {
my $test = expand_exprs($line, $vars);
# XXX This is _so_ wrong - need to parse this correctly
@@ -329,24 +327,23 @@
debug("conditional: update to $test\n");
}
- while ($test =~ /([^\s()\|\&]+)\s+(!=|==)\s+([^\s()]+)/) {
+ while ($test =~ /([^\s()\|\&]+) \s+ (!=|==) \s+ ([^\s()]+)/x) {
my $result = 0 + (($2 eq '==') ? ($1 eq $3) : ($1 ne $3));
- $test =~ s/[^\s()\|\&]+\s+(!=|==)\s+[^\s()]+/$result/;
+ $test =~ s/[^\s()\|\&]+ \s+ (!=|==) \s+ [^\s()]+/$result/x;
}
- if ($test !~ /[^<>\d()\s&|.!]/) {
+ if ($test =~ /^[ <> \d () \s & | . ! ]+$/xx) {
debug("eval test $test\n");
- $false = eval "($test) ? 0 : 1";
- if (!defined $false) {
- fail("Eval failed $line - $test");
- }
- debug('conditional: evaluated to ' . ($false ? 0 : 1) . "\n");
+ $! = undef;
+ my $result = eval "($test) ? 1 : 0";
+ defined $result or fail("Eval '$test' failed in '$line': $@");
+ debug("conditional: evaluated to " . ($result ? 'true' : 'false') . "\n");
+ $result;
} else {
- $false = 0;
- debug("conditional: defaulting to 0\n");
+ debug("conditional: defaulting '$test' to true\n");
+ 1;
}
- $false;
}
sub parse_makefile_line_include($file, $incfile,
@@ -398,6 +395,7 @@
return;
}
+ # FIXME: .CURDIR doesn't change, but .PARSEDIR does.
my $NEWCURDIR = $incfile;
$NEWCURDIR =~ s#/[^/]*$##;
push @$incdirs, $NEWCURDIR
@@ -491,7 +489,6 @@
my %vars;
my %incfiles; # Cache of previously included files
my @incdirs; # Directories in which to check for includes
- my @if_false; # 0:true 1:false 2:nested-false&nomore-elsif
my @lines;
open(FILE, $file) or return undef;
@@ -509,19 +506,14 @@
}
$vars{BSD_PKG_MK} = 'YES';
- if ($cwd) {
- $vars{'.CURDIR'} = $cwd;
- } elsif ($file =~ m#(.*)/#) {
- $vars{'.CURDIR'} = $1;
- } else {
- $vars{'.CURDIR'} = getcwd;
- }
-
- push @incdirs, $vars{'.CURDIR'};
+ my $curdir = $cwd || ($file =~ m#(.*)/# ? $1 : getcwd);
+ $vars{'.CURDIR'} = $curdir;
+ push @incdirs, $curdir;
if ($opt{L}) {
print "$file\n";
}
+ my @if_state; # 'not_yet', 'active', 'done'
while (defined($_ = shift @lines)) {
s/(*negative_lookbehind:\\)#.*//;
s/\s+$//;
@@ -538,42 +530,53 @@
# Conditionals
#
- if (m#^ \. \s* if(|def|ndef) \s+ (.*) #x) {
- my $type = $1;
- if ($if_false[-1]) {
- push @if_false, 2;
+ if (m#^ \. \s* (if|ifdef|ifndef) \s+ (.*) #x) {
+ my ($kind, $cond) = ($1, $2);
- } elsif ($type eq '') {
- # Straight if
- push @if_false, parse_eval_make_false($2, \%vars);
+ if (@if_state > 0 && $if_state[-1] ne 'active') {
+ push @if_state, 'done';
+
+ } elsif ($kind eq 'if') {
+ my $result = eval_mk_cond($cond, \%vars);
+ push @if_state, $result ? 'active' : 'not_yet';
} else {
- my $false = !defined $vars{expand_exprs($2, \%vars)};
- if ($type eq 'ndef') {
- $false = !$false;
- }
- push @if_false, $false ? 1 : 0;
+ my $varname = expand_exprs($cond, \%vars);
+
+ # bmake also allows '.ifdef A && B'.
+ debug("not implemented: .ifdef $varname\n")
+ if $cond =~ /\s/;
+
+ my $result = $kind eq 'ifdef'
+ ? defined($vars{$varname})
+ : !defined($vars{$varname});
+ push @if_state, $result ? 'active' : 'not_yet';
}
- debug("$file: .if$type (! @if_false)\n");
+
+ debug("$file: .$kind @if_state\n");
- } elsif (m#^ \. \s* elif \s+ (.*)#x && @if_false) {
- if ($if_false[-1] == 0) {
- $if_false[-1] = 2;
- } elsif ($if_false[-1] == 1
- && !parse_eval_make_false($1, \%vars)) {
- $if_false[-1] = 0;
+ } elsif ( # XXX: bmake also knows '.elifdef' and '.elifnmake'.
+ m#^ \. \s* elif \s+ (.*)#x && @if_state > 0) {
+ my ($cond) = ($1);
+
+ if ($if_state[-1] eq 'active') {
+ $if_state[-1] = 'done';
+ } elsif ($if_state[-1] eq 'not_yet'
+ && eval_mk_cond($cond, \%vars)) {
+ $if_state[-1] = 'active';
}
- debug("$file: .elif (! @if_false)\n");
+ debug("$file: .elif @if_state\n");
- } elsif (m#^ \. \s* else \b #x && @if_false) {
- $if_false[-1] = $if_false[-1] == 1 ? 0 : 1;
- debug("$file: .else (! @if_false)\n");
+ } elsif (m#^ \. \s* else \b #x && @if_state > 0) {
+ $if_state[-1] =
+ $if_state[-1] eq 'not_yet' ? 'active' : 'done';
+ debug("$file: .else @if_state\n");
} elsif (m#^\. \s* endif \b #x) {
- pop(@if_false);
- debug("$file: .endif (! @if_false)\n");
+ pop @if_state;
+ debug("$file: .endif @if_state\n");
- } elsif ($if_false[-1]) {
+ } elsif (@if_state > 0 && $if_state[-1] ne 'active') {
# Skip branches whose condition evaluated to false.
} elsif (m#^\. \s* include \s+ "([^"]+)" #x) {
diff -r ab85265d2802 -r c49bf70b0231 pkgtools/lintpkgsrc/files/t/parse_makefile.t
--- a/pkgtools/lintpkgsrc/files/t/parse_makefile.t Sat Aug 13 10:07:26 2022 +0000
+++ b/pkgtools/lintpkgsrc/files/t/parse_makefile.t Sat Aug 13 10:23:40 2022 +0000
@@ -1,4 +1,4 @@
-# $NetBSD: parse_makefile.t,v 1.10 2022/08/12 22:45:14 rillig Exp $
+# $NetBSD: parse_makefile.t,v 1.11 2022/08/13 10:23:40 rillig Exp $
use strict;
use warnings;
@@ -154,7 +154,7 @@
ok(eval_mk_cond_func('target', 'anything', $vars), 0);
}
-sub test_parse_eval_make_false() {
+sub test_eval_mk_cond() {
my $vars = {
'EMPTY' => '',
'SPACE' => ' ',
@@ -163,9 +163,15 @@
'DEV_NULL' => '/dev/null',
};
- # 1 means false, 0 means true.
- ok(parse_eval_make_false('defined(UNDEF)', $vars), 1);
- ok(parse_eval_make_false('defined(EMPTY)', $vars), 0);
+ ok(eval_mk_cond('defined(UNDEF)', $vars), 0);
+ ok(eval_mk_cond('defined(EMPTY)', $vars), 1);
+
+ # The regular expression above the 'eval' prevents most strange
+ # expressions from being evaluated. There might be ways to circumvent
+ # this plausibility check. The numbers in the below example form the
+ # string literal 'exit', and there might be a way to extend this
+ # expression to 'exit(6)', just for fun.
+ ok(eval_mk_cond('101.120.105.116', $vars), 1);
}
test_expand_exprs();
@@ -173,4 +179,4 @@
test_parse_makefile_vars_cond();
test_expand_modifiers();
test_eval_mk_cond_func();
-test_parse_eval_make_false();
+test_eval_mk_cond();
Home |
Main Index |
Thread Index |
Old Index