- Added the -d|--debug command line option, which replaces -Wdebug. The

  latter has been removed.
- Added -Wvarorder (default: disabled) to check the ordering of variables
  in simple packages.
- Using ${WRKSRC}/.. is only considered a warning, not an error, because
  there are packages that work nevertheless. Using this construct remains
  discouraged.
- Replaced most of the TODOs in the source code with code that prints
  a warning in --debug mode.
- The characters '.' and '+' are allowed in BuildlinkPackages.
- Variables *_FLAGS and *_ARGS that have their type guessed based on the
  name don't have to be assigned using the += operator.
- Variables that are assigned using the != operator are not checked for
  type compliance.
- README files are not marked as unknown when they appear in a package
  directory.

3 files changed, 58 insertions, 33 deletions

diff --git a/pkgtools/pkglint/files/pkglint.0 b/pkgtools/pkglint/files/pkglint.0
index 62aaa416f10..ef5589ac6e5 100644
--- a/pkgtools/pkglint/files/pkglint.0
+++ b/pkgtools/pkglint/files/pkglint.0
@@ -37,6 +37,13 @@ DDEESSCCRRIIPPTTIIOONN
            --WW{{[[nnoo--]]wwaarrnn,,......}}   Enable or disable specific warnings.  For a
                                list of warnings, see below.
 
+           --dd|----ddeebbuugg          Enable warnings for parsing errors and other
+                               things that haven't been completed yet.  This
+                               option is separated from the `-W' options
+                               because the diagnostics are too hard to under-
+                               stand without detailed knowledge of pkglint's
+                               source code.
+
            --ee|----eexxppllaaiinn        Print further explanations for diagnostics.
                                Sometimes the reasons for diagnostics are not
                                obvious and need further explanation.
@@ -103,9 +110,6 @@ DDEESSCCRRIIPPTTIIOONN
 
            [[nnoo--]]aabbssnnaammee        Warn if a file contains an absolute pathname.
 
-           [[nnoo--]]ddeebbuugg          Enable some warnings that are only useful for
-                               debugging pkglint itself.
-
            [[nnoo--]]ddiirreeccttccmmdd      Warn if a system command name is used instead
                                of a variable (e.g. sed instead of ${SED}).
 
@@ -125,9 +129,13 @@ DDEESSCCRRIIPPTTIIOONN
 
            [[nnoo--]]ssppaaccee          Emit notes for inconsistent use of white-space.
 
-           [[nnoo--]]ttyyppeess          Warn for some Makefile variables if their
+           [[nnoo--]]ttyyppeess          Warn for some _M_a_k_e_f_i_l_e variables if their
                                assigned values do not match their type.
 
+           [[nnoo--]]vvaarroorrddeerr       Warn if the variables in a package _M_a_k_e_f_i_l_es
+                               are not ordered in the way it is described the
+                               pkgsrc guide.
+
      OOtthheerr aarrgguummeennttss
 
            _d_i_r _._._.             The pkgsrc directories to be checked.  If omit-
@@ -173,4 +181,4 @@ BBUUGGSS
      If you don't understand the messages, feel free to ask on the
      <tech-pkg@NetBSD.org> mailing list.
 
-NetBSD 3.0                     January 12, 2006                     NetBSD 3.0
+pkgsrc                         January 20, 2006                         pkgsrc
diff --git a/pkgtools/pkglint/files/pkglint.1 b/pkgtools/pkglint/files/pkglint.1
index eb442cb2d7c..789c06f9150 100644
--- a/pkgtools/pkglint/files/pkglint.1
+++ b/pkgtools/pkglint/files/pkglint.1
@@ -1,4 +1,4 @@
-.\"	$NetBSD: pkglint.1,v 1.35 2006/01/12 21:50:43 wiz Exp $
+.\"	$NetBSD: pkglint.1,v 1.36 2006/01/20 13:33:37 rillig Exp $
 .\"	From FreeBSD: portlint.1,v 1.8 1997/11/25 14:53:14 itojun Exp
 .\"
 .\" Copyright (c) 1997 by Jun-ichiro Itoh <itojun@itojun.org>.
@@ -7,7 +7,7 @@
 .\" Roland Illig <roland.illig@gmx.de>, 2004, 2005.
 .\" Roland Illig <rillig@NetBSD.org>, 2005.
 .\"
-.Dd January 12, 2006
+.Dd January 20, 2006
 .Dt PKGLINT 1
 .Sh NAME
 .Nm pkglint
@@ -59,6 +59,13 @@ version number and exit.
 .It Fl W{[no-]warn,...}
 Enable or disable specific warnings.
 For a list of warnings, see below.
+.It Fl d Ns | Ns Fl -debug
+Enable warnings for parsing errors and other things that haven't been
+completed yet.
+This option is separated from the
+.Ql -W
+options because the diagnostics are too hard to understand without
+detailed knowledge of pkglint's source code.
 .It Fl e Ns | Ns Fl -explain
 Print further explanations for diagnostics.
 Sometimes the reasons for diagnostics are not obvious and need further
@@ -125,8 +132,6 @@ Enable all warnings.
 Disable all warnings.
 .It Cm [no-]absname
 Warn if a file contains an absolute pathname.
-.It Cm [no-]debug
-Enable some warnings that are only useful for debugging pkglint itself.
 .It Cm [no-]directcmd
 Warn if a system command name is used instead of a variable (e.g. sed
 instead of ${SED}).
@@ -144,8 +149,14 @@ and shell variables themselves.
 .It Cm [no-]space
 Emit notes for inconsistent use of white-space.
 .It Cm [no-]types
-Warn for some Makefile variables if their assigned values do not match
+Warn for some
+.Pa Makefile
+variables if their assigned values do not match
 their type.
+.It Cm [no-]varorder
+Warn if the variables in a package
+.Pa Makefile Ns
+s are not ordered in the way it is described the pkgsrc guide.
 .El
 .Pp
 .\" =======================================================================
diff --git a/pkgtools/pkglint/files/pkglint.pl b/pkgtools/pkglint/files/pkglint.pl
index 96601b35013..05d281c07ff 100644
--- a/pkgtools/pkglint/files/pkglint.pl
+++ b/pkgtools/pkglint/files/pkglint.pl
@@ -1,5 +1,5 @@
 #! @PERL@
-# $NetBSD: pkglint.pl,v 1.477 2006/01/17 23:01:17 rillig Exp $
+# $NetBSD: pkglint.pl,v 1.478 2006/01/20 13:33:37 rillig Exp $
 #
 
 # pkglint - static analyzer and checker for pkgsrc packages
@@ -996,7 +996,6 @@ my (%checks) = (
 );
 
 my $opt_warn_absname	= true;
-my $opt_warn_debug	= false;
 my $opt_warn_directcmd	= true;
 my $opt_warn_extra	= false;
 my $opt_warn_order	= true;
@@ -1004,9 +1003,9 @@ my $opt_warn_plist_sort	= false;
 my $opt_warn_quoting	= false;
 my $opt_warn_space	= false;
 my $opt_warn_types	= true;
+my $opt_warn_varorder	= false;
 my (%warnings) = (
 	"absname"	=> [\$opt_warn_absname, "warn about use of absolute file names"],
-	"debug"		=> [\$opt_warn_debug, "enable some warnings that are useful for debugging pkglint"],
 	"directcmd"	=> [\$opt_warn_directcmd, "warn about use of direct command names instead of Make variables"],
 	"extra"		=> [\$opt_warn_extra, "enable some extra warnings"],
 	"order"		=> [\$opt_warn_order, "warn if Makefile entries are unordered"],
@@ -1014,9 +1013,11 @@ my (%warnings) = (
 	"quoting"	=> [\$opt_warn_quoting, "warn about quoting issues"],
 	"space"		=> [\$opt_warn_space, "warn about inconsistent use of white-space"],
 	"types"		=> [\$opt_warn_types, "do some simple type checking in Makefiles"],
+	"varorder"	=> [\$opt_warn_varorder, "warn about the ordering of variables"],
 );
 
 my $opt_autofix		= false;
+my $opt_debug		= false;
 my $opt_dumpmakefile	= false;
 my $opt_import		= false;
 my $opt_klickibunti	= false;	# experimental
@@ -1049,6 +1050,8 @@ my (@options) = (
 		my ($opt, $val) = @_;
 		parse_multioption($val, \%warnings);
 	  } ],
+	[ "-d|--debug", "Print additional warnings that are mostly of use to pkglint's author",
+	  "debug|d", \$opt_debug ],
 	[ "-e|--explain", "Explain the diagnostics or give further help",
 	  "explain|e", sub {
 		PkgLint::Logging::set_explain();
@@ -1351,8 +1354,7 @@ sub get_vartypes_map() {
 		log_fatal($fname, NO_LINE_NUMBER, "Cannot be read.");
 	}
 
-# TODO: Enable when the time is ripe.
-if (false) {
+if ($opt_warn_varorder) {
 	# Additionally, scan mk/defaults/mk.conf for variable
 	# definitions. All these variables are reserved for the user and
 	# must not be set within packages.
@@ -2020,7 +2022,7 @@ sub checkline_mk_text($$) {
 	checkline_trailing_whitespace($line);
 
 	if ($text =~ qr"\$\{WRKSRC\}/\.\./") {
-		$line->log_error("Using \"\${WRKSRC}/..\" is conceptually wrong. Use a combination of WRKSRC, CONFIGURE_DIRS and BUILD_DIRS instead.");
+		$line->log_warning("Using \"\${WRKSRC}/..\" is conceptually wrong. Please use a combination of WRKSRC, CONFIGURE_DIRS and BUILD_DIRS instead.");
 	}
 
 	$rest = $text;
@@ -2045,8 +2047,10 @@ sub checkline_mk_shellword($$$) {
 	my ($line, $shellword, $check_quoting) = @_;
 	my ($rest, $state);
 
-	if ($shellword =~ qr"^\$\{${regex_varname}(?::.+)?\}$") {
-		# TODO: Check whether the variable needs quoting or not.
+	if ($shellword =~ qr"^\$\{(${regex_varname})(:.+)?\}$") {
+		my ($varname, $mod) = ($1, $2);
+
+		$opt_debug and $line->log_warning("Not sure whether the variable ${varname} needs quoting.");
 		return;
 	}
 
@@ -2214,7 +2218,7 @@ sub checkline_mk_shellword($$$) {
 		}
 	}
 	if ($rest ne "") {
-		$opt_warn_debug && $line->log_error("[checkline_mk_shellword] " . statename->[$state] . ": rest=${rest}");
+		$opt_debug && $line->log_error("[checkline_mk_shellword] " . statename->[$state] . ": rest=${rest}");
 	}
 }
 
@@ -2381,7 +2385,7 @@ sub checkline_mk_shelltext($$) {
 	}
 
 	if ($rest ne "") {
-		$opt_warn_debug && $line->log_error("[checkline_mk_shelltext] " . scst_statename->[$state] . ": rest=${rest}");
+		$opt_debug && $line->log_error("[checkline_mk_shelltext] " . scst_statename->[$state] . ": rest=${rest}");
 	}
 }
 
@@ -2402,7 +2406,7 @@ sub checkline_mk_vartype_basic($$$$$$) {
 	}
 
 	if ($type eq "AwkCommand") {
-		$opt_warn_debug and $line->log_warning("Unchecked AWK command: ${value}");
+		$opt_debug and $line->log_warning("Unchecked AWK command: ${value}");
 
 	} elsif ($type eq "BuildlinkDepmethod") {
 		if ($value ne $value_novar) {
@@ -2418,7 +2422,7 @@ sub checkline_mk_vartype_basic($$$$$$) {
 		}
 
 	} elsif ($type eq "BuildlinkPackages") {
-		if ($value !~ qr"^(?:\$\{BUILDLINK_PACKAGES:N[-0-9A-Z_a-z]+\}|[-0-9A-Z_a-z]+)$") {
+		if ($value !~ qr"^(?:\$\{BUILDLINK_PACKAGES:N[+\-.0-9A-Z_a-z]+\}|[+\-.0-9A-Z_a-z]+)$") {
 			$line->log_warning("Invalid value for ${varname}.");
 		}
 
@@ -2466,16 +2470,15 @@ sub checkline_mk_vartype_basic($$$$$$) {
 		} elsif ($value =~ qr"^-[DU]([0-9A-Z_a-z]+)") {
 			my ($macname) = ($1);
 
-			# TODO: Check for invalid macro names.
+			$opt_debug and $line->log_warning("Unknown macro ${macname} in ${varname}.");
 
 		} elsif ($value =~ qr"^-I(.*)") {
 			my ($dirname) = ($1);
 
-			# TODO: Check for invalid directory names.
+			$opt_debug and $line->log_warning("Unknown directory ${dirname} in ${varname}.");
 
 		} elsif ($value =~ qr"^-[OWfgm]") {
-			# TODO: Discuss which compiler flags should be allowed
-			# to be set by the package author.
+			$opt_debug and $line->log_warning("Undiscussed compiler flag ${value} in ${varname}.");
 
 		} elsif ($value =~ qr"^-.*") {
 			$line->log_warning("Unknown compiler flag \"${value}\".");
@@ -2779,7 +2782,7 @@ sub checkline_mk_vartype_basic($$$$$$) {
 		}
 
 	} elsif ($type eq "WrkdirSubdirectory") {
-		# TODO: check for ${WRKDIR}/${DISTNAME}/foo
+		$opt_debug and $line->log_warning("Unchecked subdirectory \"${value}\" of \${WRKSRC}.");
 
 	} elsif ($type eq "WrksrcSubdirectory") {
 		if ($value =~ qr"^(\$\{WRKSRC\})(?:/(.*))?") {
@@ -2856,8 +2859,8 @@ sub checkline_mk_vartype($$$$$) {
 				: ($varname =~ qr"_GROUP$") ? "UserGroupName"
 				: ($varname =~ qr"_ENV$") ? "List+ of ShellWord"
 				: ($varname =~ qr"_CMD$") ? "ShellCommand"
-				: ($varname =~ qr"_ARGS$") ? "List+ of ShellWord"
-				: ($varname =~ qr"_FLAGS$") ? "List+ of ShellWord"
+				: ($varname =~ qr"_ARGS$") ? "List of ShellWord"
+				: ($varname =~ qr"_FLAGS$") ? "List of ShellWord"
 				: $type;
 			if (defined($type)) {
 				$line->log_info("The guessed type of ${varname} is \"${type}\".");
@@ -2866,10 +2869,13 @@ sub checkline_mk_vartype($$$$$) {
 
 		if (!defined($type)) {
 			if ($varname !~ qr"_MK$") {
-				$opt_warn_debug and $line->log_warning("[checkline_mk_vartype] Unchecked variable ${varname}.");
+				$opt_debug and $line->log_warning("[checkline_mk_vartype] Unchecked variable ${varname}.");
 			}
 			checkline_mk_text($line, $value);
 
+		} elsif ($op eq "!=") {
+			$opt_debug and $line->log_info("Use of !=: ${value}");
+
 		} elsif ($type =~ qr"^List(!?)(\+?)(?: of (.*))?$") {
 			my ($internal_list, $append_only, $element_type) = ($1 eq "!", $2 eq "+", $3);
 			my (@words, $rest);
@@ -2898,7 +2904,7 @@ sub checkline_mk_vartype($$$$$) {
 			}
 
 			if ($rest !~ qr"^\s*$") {
-				$opt_warn_debug and $line->log_warning("Invalid shell word \"${value}\" at the end.");
+				$opt_debug and $line->log_warning("Invalid shell word \"${value}\" at the end.");
 			}
 
 		} else {
@@ -3206,7 +3212,7 @@ sub checklines_mk($) {
 				}
 
 			} elsif ($directive eq "if" || $directive eq "elif") {
-				# TODO
+				$opt_debug and $line->log_warning("Unchecked conditional \"${args}\".");
 
 			} elsif ($directive eq "ifdef" || $directive eq "ifndef") {
 				if ($args =~ qr"\s") {
@@ -3834,7 +3840,7 @@ sub checkfile($) {
 	} elsif ($basename =~ qr"^PLIST") {
 		$opt_check_PLIST and checkfile_PLIST($fname);
 
-	} elsif ($basename eq "TODO") {
+	} elsif ($basename eq "TODO" || $basename eq "README") {
 		# Ok
 
 	} elsif (!-T $fname) {