pkgsrc-Changes archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
CVS commit: pkgsrc/pkgtools/lintpkgsrc/files
Module Name: pkgsrc
Committed By: rillig
Date: Sat Aug 13 10:23:40 UTC 2022
Modified Files:
pkgsrc/pkgtools/lintpkgsrc/files: lintpkgsrc.pl
pkgsrc/pkgtools/lintpkgsrc/files/t: parse_makefile.t
Log Message:
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.
To generate a diff of this commit:
cvs rdiff -u -r1.82 -r1.83 pkgsrc/pkgtools/lintpkgsrc/files/lintpkgsrc.pl
cvs rdiff -u -r1.10 -r1.11 \
pkgsrc/pkgtools/lintpkgsrc/files/t/parse_makefile.t
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: pkgsrc/pkgtools/lintpkgsrc/files/lintpkgsrc.pl
diff -u pkgsrc/pkgtools/lintpkgsrc/files/lintpkgsrc.pl:1.82 pkgsrc/pkgtools/lintpkgsrc/files/lintpkgsrc.pl:1.83
--- pkgsrc/pkgtools/lintpkgsrc/files/lintpkgsrc.pl:1.82 Sat Aug 13 09:33:43 2022
+++ pkgsrc/pkgtools/lintpkgsrc/files/lintpkgsrc.pl Sat Aug 13 10:23:40 2022
@@ -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 @@ sub eval_mk_cond_func($func, $arg, $vars
}
}
-# 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 @@ sub parse_eval_make_false($line, $vars)
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 @@ sub parse_makefile_line_include($file, $
return;
}
+ # FIXME: .CURDIR doesn't change, but .PARSEDIR does.
my $NEWCURDIR = $incfile;
$NEWCURDIR =~ s#/[^/]*$##;
push @$incdirs, $NEWCURDIR
@@ -491,7 +489,6 @@ sub parse_makefile_vars($file, $cwd = un
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 @@ sub parse_makefile_vars($file, $cwd = un
}
$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 @@ sub parse_makefile_vars($file, $cwd = un
# Conditionals
#
- if (m#^ \. \s* if(|def|ndef) \s+ (.*) #x) {
- my $type = $1;
- if ($if_false[-1]) {
- push @if_false, 2;
-
- } elsif ($type eq '') {
- # Straight if
- push @if_false, parse_eval_make_false($2, \%vars);
+ if (m#^ \. \s* (if|ifdef|ifndef) \s+ (.*) #x) {
+ my ($kind, $cond) = ($1, $2);
+
+ 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: .$kind @if_state\n");
+
+ } 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: .if$type (! @if_false)\n");
+ debug("$file: .elif @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;
- }
- debug("$file: .elif (! @if_false)\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) {
Index: pkgsrc/pkgtools/lintpkgsrc/files/t/parse_makefile.t
diff -u pkgsrc/pkgtools/lintpkgsrc/files/t/parse_makefile.t:1.10 pkgsrc/pkgtools/lintpkgsrc/files/t/parse_makefile.t:1.11
--- pkgsrc/pkgtools/lintpkgsrc/files/t/parse_makefile.t:1.10 Fri Aug 12 22:45:14 2022
+++ pkgsrc/pkgtools/lintpkgsrc/files/t/parse_makefile.t Sat Aug 13 10:23:40 2022
@@ -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 @@ sub test_eval_mk_cond_func() {
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 @@ sub test_parse_eval_make_false() {
'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();
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