diff options
author | rillig <rillig@pkgsrc.org> | 2006-05-31 08:46:00 +0000 |
---|---|---|
committer | rillig <rillig@pkgsrc.org> | 2006-05-31 08:46:00 +0000 |
commit | c821274d364327d5d1c2802293ff425cf79cce49 (patch) | |
tree | ac623968715485e964c498fd2cd9f22a3bea43f3 /pkgtools/pkglint | |
parent | ef3ee853a6cfccf02707cbea778db3b8cfce4bad (diff) | |
download | pkgsrc-c821274d364327d5d1c2802293ff425cf79cce49.tar.gz |
Added a check for variables that are evaluated at load time. Especially
for lists of something, this can lead to unexpected behavior. Currently,
only the variables BUILDLINK_DEPENDS, BUILDLINK_PACKAGES and PKG_OPTIONS
may be looked at during load time.
This warning reveals the bad practice to "patch" CONFIGURE_ARGS at load
time, for example:
> CONFIGURE_ARGS:= ${CONFIGURE_ARGS:S/--disable-esd/--enable-esd/}
WARN: audio/bmp-esound/Makefile:14: CONFIGURE_ARGS should not be
evaluated at load time.
Diffstat (limited to 'pkgtools/pkglint')
-rw-r--r-- | pkgtools/pkglint/files/makevars.map | 8 | ||||
-rw-r--r-- | pkgtools/pkglint/files/pkglint.pl | 63 |
2 files changed, 58 insertions, 13 deletions
diff --git a/pkgtools/pkglint/files/makevars.map b/pkgtools/pkglint/files/makevars.map index 7b99ba3f14a..eaf5cbc8fbc 100644 --- a/pkgtools/pkglint/files/makevars.map +++ b/pkgtools/pkglint/files/makevars.map @@ -1,4 +1,4 @@ -# $NetBSD: makevars.map,v 1.97 2006/05/31 06:35:52 rillig Exp $ +# $NetBSD: makevars.map,v 1.98 2006/05/31 08:46:00 rillig Exp $ # # This file contains the guessed type of some variables, according to @@ -123,7 +123,7 @@ BUILDLINK_CFLAGS.* List of CFlag [] BUILDLINK_CPPFLAGS.* List of CFlag [] BUILDLINK_DEPENDS InternalList of Dependency [b:a] BUILDLINK_DEPMETHOD.* BuildlinkDepmethod [b:d,m:as,c:a,*.mk:a] -BUILDLINK_DEPTH BuildlinkDepth [b:s] +BUILDLINK_DEPTH BuildlinkDepth [b:ps, builtin.mk:ps] BUILDLINK_FILES.* List of Pathmask [b:a, builtin.mk:a] BUILDLINK_FILES_CMD.* List of ShellWord [] # ^^ ShellCommand @@ -133,7 +133,7 @@ BUILDLINK_LDADD.* List of LdFlag [builtin.mk:ads] BUILDLINK_LDFLAGS.* List of LdFlag [] BUILDLINK_LIBDIRS.* List of Pathname [b:a] BUILDLINK_LIBS.* List of LdFlag [b:a] -BUILDLINK_PACKAGES BuildlinkPackages [b:as] +BUILDLINK_PACKAGES BuildlinkPackages [b:aps] BUILDLINK_PASSTHRU_DIRS List of Pathname [m:a,c:a,b:a,h:a] BUILDLINK_PASSTHRU_RPATHDIRS List of Pathname [m:a,c:a,b:a,h:a] BUILDLINK_PKGSRCDIR.* RelativePkgDir [b:d] @@ -425,7 +425,7 @@ PKG_JVMS_ACCEPTED List of { blackdown-jdk13 jdk jdk14 kaffe sun-jdk13 sun-jdk14 PKG_JVM_DEFAULT Unchecked [] PKG_LEGACY_OPTIONS List of Option PKG_LIBTOOL Pathname [m:s] -PKG_OPTIONS List of Option [bsd.options.mk:s,*:] +PKG_OPTIONS List of Option [bsd.options.mk:s,*:pu] PKG_OPTIONS.* List of Option [*:] PKG_OPTIONS_DEPRECATED_WARNINGS List of ShellWord PKG_OPTIONS_GROUP.* List of Option [o:s,m:s] diff --git a/pkgtools/pkglint/files/pkglint.pl b/pkgtools/pkglint/files/pkglint.pl index 61b61da2734..5d179a720fa 100644 --- a/pkgtools/pkglint/files/pkglint.pl +++ b/pkgtools/pkglint/files/pkglint.pl @@ -1,5 +1,5 @@ #! @PERL@ -# $NetBSD: pkglint.pl,v 1.593 2006/05/31 08:10:45 rillig Exp $ +# $NetBSD: pkglint.pl,v 1.594 2006/05/31 08:46:00 rillig Exp $ # # pkglint - static analyzer and checker for pkgsrc packages @@ -1227,7 +1227,7 @@ BEGIN { VUC_TYPE_UNKNOWN VUC_SHELLWORD_UNKNOWN VUC_SHELLWORD_PLAIN VUC_SHELLWORD_DQUOT VUC_SHELLWORD_SQUOT VUC_SHELLWORD_BACKT - VUC_EXTENT_UNKOWN VUC_EXTENT_FULL VUC_EXTENT_WORD + VUC_EXTENT_UNKNOWN VUC_EXTENT_FULL VUC_EXTENT_WORD VUC_EXTENT_WORD_PART ); } @@ -1348,7 +1348,7 @@ BEGIN { VUC_TYPE_UNKNOWN VUC_SHELLWORD_UNKNOWN VUC_SHELLWORD_PLAIN VUC_SHELLWORD_DQUOT VUC_SHELLWORD_SQUOT VUC_SHELLWORD_BACKT - VUC_EXTENT_UNKOWN VUC_EXTENT_FULL VUC_EXTENT_WORD + VUC_EXTENT_UNKNOWN VUC_EXTENT_FULL VUC_EXTENT_WORD VUC_EXTENT_WORD_PART ); } @@ -2347,9 +2347,7 @@ sub backtrace() { for (my $i = $#callers; $i >= 0; $i--) { my $info = $callers[$i]; - log_debug(NO_FILE, NO_LINE_NUMBER, - sprintf("%sline %4d in %s", (" " x ($n - 1 - $i)), - $info->[0], $info->[1])); + log_debug(NO_FILE, NO_LINE_NUMBER, sprintf(" at line %4d in %s", $info->[0], $info->[1])); } } @@ -2421,6 +2419,27 @@ sub determine_used_variables($) { } } +sub extract_used_variables($$) { + my ($line, $text) = @_; + my ($rest, $result); + + $rest = $text; + $result = []; + while ($rest =~ s/^(?:[^\$]+|\$[\$*<>?\@]|\$\{([0-9A-Z_a-z]+)(?::(?:[^\${}]|\$[^{])+)?\})//) { + my ($varname) = ($1); + + if (defined($varname)) { + push(@{$result}, $varname); + } + } + + if ($rest ne "") { + $opt_debug and $line->log_warning("Could not extract variables: ${rest}"); + } + + return $result; +} + my $check_pkglint_version_done = false; sub check_pkglint_version() { @@ -2563,7 +2582,7 @@ sub variable_needs_quoting($$$) { my $type = get_variable_type($line, $varname); my ($want_list, $have_list); - if (!defined($type)) { + if (!defined($type) || !defined($context->type)) { return dont_know; } @@ -2919,7 +2938,13 @@ sub checkline_mk_varuse($$$$) { my $perms = get_variable_perms($line, $varname); if ($context->time == VUC_TIME_LOAD && index($perms, "p") == -1) { - $line->log_warning("${varname} should not be used at load time."); + $line->log_warning("${varname} should not be evaluated at load time."); + $line->explain_warning( + "Many variables, especially lists of something, get their values", + "incrementally. Therefore it is generally unsafe to rely on their value", + "until it is clear that it will never change again. This point is", + "reached when the whole package Makefile is loaded and execution of the", + "shell commands starts."); } } @@ -3647,7 +3672,7 @@ sub checkline_mk_vartype_basic($$$$$$$) { } else { $line->log_warning("Invalid file mode ${value}."); } - + } elsif ($type eq "Identifier") { if ($value ne $value_novar) { #$line->log_warning("Identifiers should be given directly."); @@ -4102,6 +4127,7 @@ sub checkline_mk_vartype($$$$$) { sub checkline_mk_varassign($$$$$) { my ($line, $varname, $op, $value, $comment) = @_; + my ($used_vars); my $varbase = varname_base($varname); my $varcanon = varname_canon($varname); @@ -4191,6 +4217,25 @@ sub checkline_mk_varassign($$$$$) { "variable for collecting the list of PLIST substitutions and later", "append that variable with PLIST_SUBST+= \${MY_PLIST_SUBST}."); } + + use constant op_to_use_time => { + ":=" => VUC_TIME_LOAD, + "!=" => VUC_TIME_LOAD, + "=" => VUC_TIME_RUN, + "+=" => VUC_TIME_RUN, + "?=" => VUC_TIME_RUN + }; + + $used_vars = extract_used_variables($line, $value); + my $vuc = PkgLint::VarUseContext->new( + op_to_use_time->{$op}, + get_variable_type($line, $varname), + VUC_SHELLWORD_UNKNOWN, + VUC_EXTENT_UNKNOWN + ); + foreach my $used_var (@{$used_vars}) { + checkline_mk_varuse($line, $used_var, "", $vuc); + } } # |