CVS commit: pkgsrc/pkgtools/lintpkgsrc/files

["Roland Illig" <rillig%netbsd.org@localhost>]

Module Name:    pkgsrc
Committed By:   rillig
Date:           Fri Aug 12 22:18:35 UTC 2022

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

Log Message:
lintpkgsrc: clean up subroutine names, test conditionals


To generate a diff of this commit:
cvs rdiff -u -r1.77 -r1.78 pkgsrc/pkgtools/lintpkgsrc/files/lintpkgsrc.pl
cvs rdiff -u -r1.6 -r1.7 pkgsrc/pkgtools/lintpkgsrc/files/t/parse_makefile.t
cvs rdiff -u -r1.10 -r1.11 pkgsrc/pkgtools/lintpkgsrc/files/t/pkgversion.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.77 pkgsrc/pkgtools/lintpkgsrc/files/lintpkgsrc.pl:1.78
--- pkgsrc/pkgtools/lintpkgsrc/files/lintpkgsrc.pl:1.77 Fri Aug 12 20:53:01 2022
+++ pkgsrc/pkgtools/lintpkgsrc/files/lintpkgsrc.pl      Fri Aug 12 22:18:35 2022
@@ -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@localhost>.
 #
@@ -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, $
 
 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 = un
                                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 = un
                        # 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 = un
                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;
                                        }

Index: pkgsrc/pkgtools/lintpkgsrc/files/t/parse_makefile.t
diff -u pkgsrc/pkgtools/lintpkgsrc/files/t/parse_makefile.t:1.6 pkgsrc/pkgtools/lintpkgsrc/files/t/parse_makefile.t:1.7
--- pkgsrc/pkgtools/lintpkgsrc/files/t/parse_makefile.t:1.6     Wed Aug 10 21:48:47 2022
+++ pkgsrc/pkgtools/lintpkgsrc/files/t/parse_makefile.t Fri Aug 12 22:18:35 2022
@@ -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 
 
 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();

Index: pkgsrc/pkgtools/lintpkgsrc/files/t/pkgversion.t
diff -u pkgsrc/pkgtools/lintpkgsrc/files/t/pkgversion.t:1.10 pkgsrc/pkgtools/lintpkgsrc/files/t/pkgversion.t:1.11
--- pkgsrc/pkgtools/lintpkgsrc/files/t/pkgversion.t:1.10        Thu Aug  4 05:50:54 2022
+++ pkgsrc/pkgtools/lintpkgsrc/files/t/pkgversion.t     Fri Aug 12 22:18:35 2022
@@ -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 => su
 
 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 ? '=='