lintpkgsrc: clean up subroutine names, test conditionals

3 files changed, 102 insertions, 43 deletions

diff --git a/pkgtools/lintpkgsrc/files/lintpkgsrc.pl b/pkgtools/lintpkgsrc/files/lintpkgsrc.pl
index 6d49c11ccdb..a650db497d6 100755
--- a/pkgtools/lintpkgsrc/files/lintpkgsrc.pl
+++ b/pkgtools/lintpkgsrc/files/lintpkgsrc.pl
@@ -1,6 +1,6 @@
 #!@PERL5@
 
-# $NetBSD: lintpkgsrc.pl,v 1.77 2022/08/12 20:53:01 rillig Exp $
+# $NetBSD: lintpkgsrc.pl,v 1.78 2022/08/12 22:18:35 rillig Exp $
 
 # Written by David Brownlee <abs@netbsd.org>.
 #
@@ -234,7 +234,7 @@ sub split_pkgversion($pkgversion) {
 	@temp;
 }
 
-sub pkgversioncmp($va, $op, $vb) {
+sub pkgversion_cmp($va, $op, $vb) {
 	my ($nb_a, @a) = split_pkgversion($va);
 	my ($nb_b, @b) = split_pkgversion($vb);
 
@@ -252,7 +252,7 @@ sub pkgversioncmp($va, $op, $vb) {
 
 # Return a copy of $value in which trivial variable expressions are replaced
 # with their variable values.
-sub expand_var($value, $vars) {
+sub expand_exprs($value, $vars) {
 	while ($value =~ /\$\{([-\w.]+)\}/) {
 		$value = defined $vars->{$1}
 		    ? "$`$vars->{$1}$'"
@@ -263,7 +263,7 @@ sub expand_var($value, $vars) {
 
 sub eval_mk_cond_func($func, $arg, $vars) {
 	if ($func eq 'defined') {
-		my $varname = expand_var($arg, $vars);
+		my $varname = expand_exprs($arg, $vars);
 		defined $vars->{$varname} ? 1 : 0;
 
 	} elsif ($func eq 'empty') {
@@ -271,19 +271,22 @@ sub eval_mk_cond_func($func, $arg, $vars) {
 		# Implement (some of) make's :M modifier
 		if ($arg =~ /^ ([^:]+) :M ([^:]+) $/x) {
 			my ($varname, $pattern) = ($1, $2);
-			$varname = expand_var($varname, $vars);
-			$pattern = expand_var($pattern, $vars);
+			$varname = expand_exprs($varname, $vars);
+			$pattern = expand_exprs($pattern, $vars);
 
 			my $value = $vars->{$varname};
 			return 1 unless defined $value;
 
-			$value = expand_var($value, $vars);
+			$value = expand_exprs($value, $vars);
 
 			$pattern =~ s/([{.+])/\\$1/g;
 			$pattern =~ s/\*/.*/g;
 			$pattern =~ s/\?/./g;
 			$pattern = '^' . $pattern . '$';
 
+			# XXX: Splitting by whitespace is not correct, but
+			#  it's good enough for lists with only unquoted
+			#  words. See devel/bmake/files/str.c:brk_string.
 			foreach my $word (split(/\s+/, $value)) {
 				return 0 if $word =~ /$pattern/;
 			}
@@ -292,11 +295,11 @@ sub eval_mk_cond_func($func, $arg, $vars) {
 			debug("Unsupported ':M' modifier in '$arg'\n");
 		}
 
-		my $value = expand_var("\${$arg}", $vars);
+		my $value = expand_exprs("\${$arg}", $vars);
 		defined $value && $value =~ /\S/ ? 0 : 1;
 
 	} elsif ($func eq 'exists') {
-		my $fname = expand_var($arg, $vars);
+		my $fname = expand_exprs($arg, $vars);
 		-e $fname ? 1 : 0;
 
 	} elsif ($func eq 'make') {
@@ -307,9 +310,10 @@ sub eval_mk_cond_func($func, $arg, $vars) {
 	}
 }
 
+# TODO: The word 'false' is confusing.
 sub parse_eval_make_false($line, $vars) {
 	my $false = 0;
-	my $test = expand_var($line, $vars);
+	my $test = expand_exprs($line, $vars);
 
 	# XXX This is _so_ wrong - need to parse this correctly
 	$test =~ s/""/\r/g;
@@ -320,23 +324,19 @@ sub parse_eval_make_false($line, $vars) {
 
 	while ($test =~ /(target|empty|make|defined|exists)\s*\(([^()]+)\)/) {
 		my ($func, $arg) = ($1, $2);
-		my $cond = eval_mk_cond_func($func, $arg, $vars);
-		$test =~ s/$func\s*\([^()]+\)/$cond/;
+		my $result = eval_mk_cond_func($func, $arg, $vars);
+		$test =~ s/$func\s*\([^()]+\)/$result/;
 		debug("conditional: update to $test\n");
 	}
 
 	while ($test =~ /([^\s()\|\&]+)\s+(!=|==)\s+([^\s()]+)/) {
-		if ($2 eq '==') {
-			$_ = $1 eq $3 ? 1 : 0;
-		} else {
-			$_ = $1 ne $3 ? 1 : 0;
-		}
-		$test =~ s/[^\s()\|\&]+\s+(!=|==)\s+[^\s()]+/$_/;
+		my $result = 0 + (($2 eq '==') ? ($1 eq $3) : ($1 ne $3));
+		$test =~ s/[^\s()\|\&]+\s+(!=|==)\s+[^\s()]+/$result/;
 	}
 
 	if ($test !~ /[^<>\d()\s&|.!]/) {
 		debug("eval test $test\n");
-		$false = eval "($test)?0:1";
+		$false = eval "($test) ? 0 : 1";
 		if (!defined $false) {
 			fail("Eval failed $line - $test");
 		}
@@ -411,7 +411,7 @@ sub parse_makefile_line_include($file, $incfile,
 
 sub parse_makefile_line_var($varname, $op, $value, $vars) {
 	if ($op eq ':=') {
-		$vars->{$varname} = expand_var($value, $vars);
+		$vars->{$varname} = expand_exprs($value, $vars);
 	} elsif ($op eq '+=' && defined $vars->{$varname}) {
 		$vars->{$varname} .= " $value";
 	} elsif ($op eq '?=' && defined $vars->{$varname}) {
@@ -543,7 +543,7 @@ sub parse_makefile_vars($file, $cwd = undef) {
 				push @if_false, parse_eval_make_false($2, \%vars);
 
 			} else {
-				my $false = !defined $vars{expand_var($2, \%vars)};
+				my $false = !defined $vars{expand_exprs($2, \%vars)};
 				if ($type eq 'ndef') {
 					$false = !$false;
 				}
@@ -572,7 +572,7 @@ sub parse_makefile_vars($file, $cwd = undef) {
 			# Skip branches whose condition evaluated to false.
 
 		} elsif (m#^\. \s* include \s+ "([^"]+)" #x) {
-			my $incfile = expand_var($1, \%vars);
+			my $incfile = expand_exprs($1, \%vars);
 
 			parse_makefile_line_include($file, $incfile,
 			    \@incdirs, \%incfiles, \@lines, \%vars);
@@ -597,7 +597,7 @@ sub parse_makefile_vars($file, $cwd = undef) {
 		foreach my $key (keys %vars) {
 			next if index($vars{$key}, '$') == -1;
 
-			$_ = expand_var($vars{$key}, \%vars);
+			$_ = expand_exprs($vars{$key}, \%vars);
 			if ($_ ne $vars{$key}) {
 				$vars{$key} = $_;
 				$loop = 1;
@@ -836,7 +836,7 @@ sub package_globmatch($pkgmatch) {
 						last;
 					}
 				} else {
-					if (pkgversioncmp($pkgver->pkgversion, $test, $matchver)) {
+					if (pkgversion_cmp($pkgver->pkgversion, $test, $matchver)) {
 						$matchver = undef;
 						last;
 					}
diff --git a/pkgtools/lintpkgsrc/files/t/parse_makefile.t b/pkgtools/lintpkgsrc/files/t/parse_makefile.t
index e5f5c4d137d..5c76ab41a7a 100644
--- a/pkgtools/lintpkgsrc/files/t/parse_makefile.t
+++ b/pkgtools/lintpkgsrc/files/t/parse_makefile.t
@@ -1,4 +1,4 @@
-# $NetBSD: parse_makefile.t,v 1.6 2022/08/10 21:48:47 rillig Exp $
+# $NetBSD: parse_makefile.t,v 1.7 2022/08/12 22:18:35 rillig Exp $
 
 use strict;
 use warnings;
@@ -10,31 +10,46 @@ BEGIN { plan tests => 29, onfail => sub { die } }
 
 require('../lintpkgsrc.pl');
 
-sub test_expand_var() {
+sub enable_debug_logging() {
+	export_for_test()->{opt}->{D} = 1;
+}
+
+sub disable_debug_logging() {
+	export_for_test()->{opt}->{D} = 0;
+}
+
+sub test_expand_exprs() {
 	my %vars = (
 	    CFLAGS      => '${CFLAGS_OPT} ${CFLAGS_WARN} ${CFLAGS_ERR}',
 	    CFLAGS_WARN => '${CFLAGS_WARN_ALL}',
 	    CFLAGS_OPT  => '-Os',
 	    CFLAGS_ERR  => '${CFLAGS_WARN_ALL:M*error=*}',
+	    '2_DOLLAR'  => '$${CFLAGS_OPT}',
+	    '3_DOLLAR'  => '$$${CFLAGS_OPT}',
 	);
 
-	my $cflags = expand_var('<${CFLAGS}>', \%vars);
+	ok(expand_exprs('<${CFLAGS}>', \%vars),
+	    '<-Os M_a_G_i_C_uNdEfInEd ${CFLAGS_WARN_ALL:M*error=*}>');
 
-	ok($cflags, '<-Os M_a_G_i_C_uNdEfInEd ${CFLAGS_WARN_ALL:M*error=*}>')
+	# FIXME: '$$' must be preserved, the result must be '$${CFLAGS_OPT}'.
+	ok(expand_exprs('<${2_DOLLAR}>', \%vars),
+	    '<$-Os>');
+	ok(expand_exprs('<${3_DOLLAR}>', \%vars),
+	    '<$$-Os>');
 }
 
 sub test_parse_makefile_vars() {
 	my $dir = File::Temp->newdir();
 	my $file = "$dir/filename.mk";
 
-	write_file($file,
-	    "# comment\n",
-	    "VAR=\tvalue\n",
-	    "COMMENT=\tvalue#comment\n",
-	    "MULTI=\tone\\\n",
-	    "\ttwo\\\n",
-	    "three#comment\n"
-	);
+	write_file($file, map { "$_\n" } (
+	    '# comment',
+	    "VAR=\tvalue",
+	    "COMMENT=\tvalue#comment",
+	    "MULTI=\tone\\",
+	    "\ttwo\\",
+	    'three#comment',
+	));
 
 	my $vars = parse_makefile_vars($file, undef);
 
@@ -52,11 +67,39 @@ sub test_parse_makefile_vars() {
 	ok($vars->{VAR}, 'value');
 }
 
+sub test_parse_makefile_vars_cond() {
+	my $dir = File::Temp->newdir();
+	my $file = "$dir/filename.mk";
+
+	write_file($file, map { "$_\n" } (
+	    '.if ${COND} == then',
+	    'BRANCH= then',
+	    '.elif ${COND} == elif',
+	    'BRANCH= elif',
+	    '.else',
+	    'BRANCH= else',
+	    '.endif',
+	));
+
+	my $vars;
+	export_for_test()->{default_vars}->{COND} = 'then';
+	$vars = parse_makefile_vars($file, undef);
+	ok($vars->{BRANCH}, 'then');
+
+	export_for_test()->{default_vars}->{COND} = 'elif';
+	$vars = parse_makefile_vars($file, undef);
+	ok($vars->{BRANCH}, 'elif');
+
+	# XXX: The string 'anything else' would not work due to the space.
+	export_for_test()->{default_vars}->{COND} = 'anything_else';
+	$vars = parse_makefile_vars($file, undef);
+	ok($vars->{BRANCH}, 'else');
+}
+
 sub test_expand_modifiers() {
 	my $vars = {
 	    REF => 'VALUE',
 	};
-	export_for_test()->{opt}->{D} = 1;
 
 	expand_modifiers('file.mk', 'VAR', '<', 'REF', 'S,U,X,', '>', $vars);
 
@@ -112,7 +155,23 @@ sub test_eval_mk_cond_func() {
 	ok(eval_mk_cond_func('target', 'anything', $vars), 0);
 }
 
-test_expand_var();
+sub test_parse_eval_make_false() {
+	my $vars = {
+	    'EMPTY'    => '',
+	    'SPACE'    => ' ',
+	    'WORD'     => 'word',
+	    'WORDS'    => 'word1 word2',
+	    '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);
+}
+
+test_expand_exprs();
 test_parse_makefile_vars();
+test_parse_makefile_vars_cond();
 test_expand_modifiers();
 test_eval_mk_cond_func();
+test_parse_eval_make_false();
diff --git a/pkgtools/lintpkgsrc/files/t/pkgversion.t b/pkgtools/lintpkgsrc/files/t/pkgversion.t
index b075bfffaed..52ab9c6f7cb 100644
--- a/pkgtools/lintpkgsrc/files/t/pkgversion.t
+++ b/pkgtools/lintpkgsrc/files/t/pkgversion.t
@@ -1,4 +1,4 @@
-# $NetBSD: pkgversion.t,v 1.10 2022/08/04 05:50:54 rillig Exp $
+# $NetBSD: pkgversion.t,v 1.11 2022/08/12 22:18:35 rillig Exp $
 
 use strict;
 use warnings;
@@ -9,8 +9,8 @@ BEGIN { plan tests => 1781, onfail => sub { die } }
 
 require('../lintpkgsrc.pl');
 
-ok(pkgversioncmp('3.4', '<', '3.4'), '');
-ok(pkgversioncmp('3.4', '<=', '3.4'), 1);
+ok(pkgversion_cmp('3.4', '<', '3.4'), '');
+ok(pkgversion_cmp('3.4', '<=', '3.4'), 1);
 
 # See pkgtools/pkglint/files/pkgver/vercmp_test.go.
 my @split_version_tests = (
@@ -80,8 +80,8 @@ foreach my $i (0 .. $#versions) {
 	foreach my $j (0 .. $#versions) {
 		foreach my $vi (@{$versions[$i]}) {
 			foreach my $vj (@{$versions[$j]}) {
-				my $actual = pkgversioncmp($vi, '<', $vj) ? '<'
-				    : pkgversioncmp($vi, '<=', $vj) ? '=='
+				my $actual = pkgversion_cmp($vi, '<', $vj) ? '<'
+				    : pkgversion_cmp($vi, '<=', $vj) ? '=='
 				    : '>';
 				my $expected = $i < $j ? '<'
 				    : $i == $j ? '=='