Content-Transfer-Encoding: 7bit
Content-Type: multipart/mixed; boundary="_----------=_1660168128166060"
MIME-Version: 1.0
Date: Wed, 10 Aug 2022 21:48:48 +0000
From: "Roland Illig" <rillig@netbsd.org>
Subject: CVS commit: pkgsrc/pkgtools/lintpkgsrc/files
To: pkgsrc-changes@NetBSD.org
Reply-To: rillig@netbsd.org
Message-Id: <20220810214848.276CDFB1A@cvs.NetBSD.org>
Sender: pkgsrc-changes-owner@NetBSD.org
Precedence: bulk

This is a multi-part message in MIME format.

--_----------=_1660168128166060
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset="US-ASCII"

Module Name:	pkgsrc
Committed By:	rillig
Date:		Wed Aug 10 21:48:48 UTC 2022

Modified Files:
	pkgsrc/pkgtools/lintpkgsrc/files: lintpkgsrc.pl
	pkgsrc/pkgtools/lintpkgsrc/files/t: glob.t parse_makefile.t

Log Message:
lintpkgsrc: fix evaluation of conditions in makefiles

Previously, evaluating the conditions was a wild mixture of concepts
taken from bmake's cond.c, without attention to detail. For example, in
the condition 'exists(VAR:Mpattern)', the ':Mpattern' doesn't make any
sense, still lintpkgsrc parsed it as a modifier.

Instead of evaluating the ':M' modifier only in conditions, a better
idea is to have a general subroutine that expands an arbitrary strings,
taking care of all kinds of modifiers.

While the previous code was conceptually wrong, the number of practical
difference is small. Still, it's better to be precise and accurate,
there are enough places with bad heuristics in the rest of the
lintpkgsrc code.


To generate a diff of this commit:
cvs rdiff -u -r1.70 -r1.71 pkgsrc/pkgtools/lintpkgsrc/files/lintpkgsrc.pl
cvs rdiff -u -r1.6 -r1.7 pkgsrc/pkgtools/lintpkgsrc/files/t/glob.t
cvs rdiff -u -r1.5 -r1.6 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.


--_----------=_1660168128166060
Content-Disposition: inline
Content-Length: 6275
Content-Transfer-Encoding: binary
Content-Type: text/x-diff; charset=us-ascii

Modified files:

Index: pkgsrc/pkgtools/lintpkgsrc/files/lintpkgsrc.pl
diff -u pkgsrc/pkgtools/lintpkgsrc/files/lintpkgsrc.pl:1.70 pkgsrc/pkgtools/lintpkgsrc/files/lintpkgsrc.pl:1.71
--- pkgsrc/pkgtools/lintpkgsrc/files/lintpkgsrc.pl:1.70	Wed Aug 10 20:16:55 2022
+++ pkgsrc/pkgtools/lintpkgsrc/files/lintpkgsrc.pl	Wed Aug 10 21:48:47 2022
@@ -1,6 +1,6 @@
 #!@PERL5@
 
-# $NetBSD: lintpkgsrc.pl,v 1.70 2022/08/10 20:16:55 rillig Exp $
+# $NetBSD: lintpkgsrc.pl,v 1.71 2022/08/10 21:48:47 rillig Exp $
 
 # Written by David Brownlee <abs@netbsd.org>.
 #
@@ -275,6 +275,52 @@ sub expand_var($value, $vars) {
 	$value;
 }
 
+sub eval_mk_cond_func($func, $arg, $vars) {
+	if ($func eq 'defined') {
+		my $varname = expand_var($arg, $vars);
+		defined $vars->{$varname} ? 1 : 0;
+
+	} elsif ($func eq 'empty') {
+
+		# 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);
+
+			my $value = $vars->{$varname};
+			return 1 unless defined $value;
+
+			$value = expand_var($value, $vars);
+
+			$pattern =~ s/([{.+])/\\$1/g;
+			$pattern =~ s/\*/.*/g;
+			$pattern =~ s/\?/./g;
+			$pattern = '^' . $pattern . '$';
+
+			foreach my $word (split(/\s+/, $value)) {
+				return 0 if $word =~ /$pattern/;
+			}
+			return 1;
+		} elsif ($arg =~ /:M/) {
+			debug("Unsupported ':M' modifier in '$arg'\n");
+		}
+
+		my $value = expand_var("\${$arg}", $vars);
+		defined $value && $value =~ /\S/ ? 0 : 1;
+
+	} elsif ($func eq 'exists') {
+		my $fname = expand_var($arg, $vars);
+		-e $fname ? 1 : 0;
+
+	} elsif ($func eq 'make') {
+		0;
+
+	} else { # $func eq 'target'
+		0;
+	}
+}
+
 sub parse_eval_make_false($line, $vars) {
 	my $false = 0;
 	my $test = expand_var($line, $vars);
@@ -287,49 +333,9 @@ sub parse_eval_make_false($line, $vars) 
 	debug("conditional: $test\n");
 
 	while ($test =~ /(target|empty|make|defined|exists)\s*\(([^()]+)\)/) {
-		my ($testname, $varname) = ($1, $2);
-		my $var;
-
-		# Implement (some of) make's :M modifier
-		if ($varname =~ /^([^:]+):M(.+)$/) {
-			$varname = $1;
-			my $match = $2;
-
-			$var = $${vars}{$varname};
-			$var = expand_var($var, $vars)
-			    if defined $var;
-
-			$match =~ s/([{.+])/\\$1/g;
-			$match =~ s/\*/.*/g;
-			$match =~ s/\?/./g;
-			$match = '^' . $match . '$';
-			$var = ($var =~ /$match/)
-			    if defined $var;
-		} else {
-			$var = $${vars}{$varname};
-			$var = expand_var($var, $vars)
-			    if defined $var;
-		}
-
-		if (defined $var && $var eq $magic_undefined) {
-			$var = undef;
-		}
-
-		if ($testname eq 'exists') {
-			$_ = -e $varname ? 1 : 0;
-
-		} elsif ($testname eq 'defined') {
-			$_ = defined $var ? 1 : 0;
-
-		} elsif ($testname eq 'empty') {
-			$_ = !defined $var || $var eq '' ? 1 : 0;
-
-		} else {
-			# XXX Could do something with target
-			$_ = 0;
-		}
-
-		$test =~ s/$testname\s*\([^()]+\)/$_/;
+		my ($func, $arg) = ($1, $2);
+		my $cond = eval_mk_cond_func($func, $arg, $vars);
+		$test =~ s/$func\s*\([^()]+\)/$cond/;
 		debug("conditional: update to $test\n");
 	}
 

Index: pkgsrc/pkgtools/lintpkgsrc/files/t/glob.t
diff -u pkgsrc/pkgtools/lintpkgsrc/files/t/glob.t:1.6 pkgsrc/pkgtools/lintpkgsrc/files/t/glob.t:1.7
--- pkgsrc/pkgtools/lintpkgsrc/files/t/glob.t:1.6	Tue Aug  9 18:35:43 2022
+++ pkgsrc/pkgtools/lintpkgsrc/files/t/glob.t	Wed Aug 10 21:48:47 2022
@@ -1,10 +1,10 @@
-# $NetBSD: glob.t,v 1.6 2022/08/09 18:35:43 rillig Exp $
+# $NetBSD: glob.t,v 1.7 2022/08/10 21:48:47 rillig Exp $
 
 use strict;
 use warnings;
 use Test;
 
-BEGIN { plan tests => 12, onfail => sub { die } }
+BEGIN { plan tests => 27, onfail => sub { die } }
 
 require('../lintpkgsrc.pl');
 

Index: pkgsrc/pkgtools/lintpkgsrc/files/t/parse_makefile.t
diff -u pkgsrc/pkgtools/lintpkgsrc/files/t/parse_makefile.t:1.5 pkgsrc/pkgtools/lintpkgsrc/files/t/parse_makefile.t:1.6
--- pkgsrc/pkgtools/lintpkgsrc/files/t/parse_makefile.t:1.5	Wed Aug 10 07:12:52 2022
+++ pkgsrc/pkgtools/lintpkgsrc/files/t/parse_makefile.t	Wed Aug 10 21:48:47 2022
@@ -1,4 +1,4 @@
-# $NetBSD: parse_makefile.t,v 1.5 2022/08/10 07:12:52 rillig Exp $
+# $NetBSD: parse_makefile.t,v 1.6 2022/08/10 21:48:47 rillig Exp $
 
 use strict;
 use warnings;
@@ -6,7 +6,7 @@ use File::Slurp;
 use File::Temp;
 use Test;
 
-BEGIN { plan tests => 6, onfail => sub { die } }
+BEGIN { plan tests => 29, onfail => sub { die } }
 
 require('../lintpkgsrc.pl');
 
@@ -69,6 +69,50 @@ sub test_expand_modifiers() {
 	ok($vars->{VAR}, '<HUE>');
 }
 
+sub test_eval_mk_cond_func() {
+	my $vars = {
+	    'EMPTY'    => '',
+	    'SPACE'    => ' ',
+	    'WORD'     => 'word',
+	    'WORDS'    => 'word1 word2',
+	    'DEV_NULL' => '/dev/null',
+	};
+
+	ok(eval_mk_cond_func('defined', '', $vars), 0);
+	ok(eval_mk_cond_func('defined', 'UNDEF', $vars), 0);
+	ok(eval_mk_cond_func('defined', 'EMPTY', $vars), 1);
+	ok(eval_mk_cond_func('defined', 'SPACE', $vars), 1);
+	ok(eval_mk_cond_func('defined', 'WORDS', $vars), 1);
+
+	# TODO: The expression '${}' doesn't expand to an empty string.
+	ok(eval_mk_cond_func('empty', '', $vars), 0);
+
+	ok(eval_mk_cond_func('empty', 'EMPTY', $vars), 1);
+	ok(eval_mk_cond_func('empty', 'SPACE', $vars), 1);
+	ok(eval_mk_cond_func('empty', 'WORD', $vars), 0);
+	ok(eval_mk_cond_func('empty', 'WORDS', $vars), 0);
+
+	ok(eval_mk_cond_func('empty', 'EMPTY:M*', $vars), 1);
+	ok(eval_mk_cond_func('empty', 'SPACE:M*', $vars), 1);
+	ok(eval_mk_cond_func('empty', 'WORD:Mword', $vars), 0);
+	ok(eval_mk_cond_func('empty', 'WORD:Mword1', $vars), 1);
+	ok(eval_mk_cond_func('empty', 'WORD:M*', $vars), 0);
+
+	# Only expressions with a single ':M' modifier are supported.
+	# The expression '${WORD:Mword:Mword}' is not expanded further,
+	# and is thus nonempty.
+	ok(eval_mk_cond_func('empty', 'WORD:Mword:Mword', $vars), 0);
+
+	ok(eval_mk_cond_func('exists', '/dev/null', $vars), 1);
+	ok(eval_mk_cond_func('exists', '${DEV_NULL}', $vars), 1);
+	ok(eval_mk_cond_func('exists', '/random-46699840-8aca', $vars), 0);
+
+	ok(eval_mk_cond_func('make', 'anything', $vars), 0);
+
+	ok(eval_mk_cond_func('target', 'anything', $vars), 0);
+}
+
 test_expand_var();
 test_parse_makefile_vars();
 test_expand_modifiers();
+test_eval_mk_cond_func();


--_----------=_1660168128166060--