3.91:

- added options for enabling/disabling specific checks
- added options for enabling/disabling specific warnings
- improved the --help message
and corresponding man page updates.
From Roland Illig with slight fixes by me.

3 files changed, 307 insertions, 87 deletions

diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile
index c6191a63e52..3f26c635f56 100644
--- a/pkgtools/pkglint/Makefile
+++ b/pkgtools/pkglint/Makefile
@@ -1,7 +1,7 @@
-# $NetBSD: Makefile,v 1.203 2004/09/05 23:12:36 seb Exp $
+# $NetBSD: Makefile,v 1.204 2004/09/24 15:33:26 wiz Exp $
 #
 
-DISTNAME=	pkglint-3.90
+DISTNAME=	pkglint-3.91
 CATEGORIES=	pkgtools devel
 MASTER_SITES=	# empty
 DISTFILES=	# empty
diff --git a/pkgtools/pkglint/files/pkglint.1 b/pkgtools/pkglint/files/pkglint.1
index fc064b1717e..7336214336b 100644
--- a/pkgtools/pkglint/files/pkglint.1
+++ b/pkgtools/pkglint/files/pkglint.1
@@ -1,18 +1,19 @@
-.\"	$NetBSD: pkglint.1,v 1.7 2003/05/28 16:32:22 wiz Exp $
+.\"	$NetBSD: pkglint.1,v 1.8 2004/09/24 15:33:26 wiz 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>.
 .\" All Rights Reserved.  Absolutely no warranty.
 .\"
-.Dd September 5, 2000
+.\" Roland Illig <roland.illig@gmx.de>, 2004.
+.\"
+.Dd September 23, 2004
 .Dt PKGLINT 1
 .Sh NAME
 .Nm pkglint
 .Nd a verifier for pkgsrc directories
 .Sh SYNOPSIS
 .Nm pkglint
-.Op Fl hvIN
-.Op Fl B Ar n
+.Op Fl options
 .Op Ar dir
 .Sh DESCRIPTION
 .Nm
@@ -43,7 +44,9 @@ to
 (by default,
 .Ar n
 is 1.)
-.It Fl h
+.It Fl C{[no-]check,...}
+Enable or disable specific checks. For a list of checks, see below.
+.It Fl h Ns | Ns Fl -help
 Show the summary of command line options, then exit.
 .It Fl I
 Show the Makefile that is constructed by including all the files that
@@ -54,27 +57,82 @@ New package flag.
 Adds several checks specific to newly submitted packages.
 If you are willing to submit the directory to be checked as a new package,
 use this option.
-.It Fl v
+This option is deprecated.
+Use
+.Fl C Ns Ar newpkg
+instead.
+.It Fl v Ns | Ns Fl -verbose
 Be verbose.
 Show the progress report for items that are being checked.
+.It Fl W{[no-]warn,...}
+Enable or disable specific warnings.
+For a list of warnings, see below.
+.El
+.Pp
+.Sy Checks
+.Bl -tag -width Fl
+.It all
+Enable all checks.
+.It none
+Disable all checks.
+Currently the Makefile is checked in every case to
+get meta information about the package structure.
+.It [no-]MESSAGE
+Check MESSAGE files.
+.It [no-]PLIST
+Check PLIST files.
+.It [no-]distinfo
+Check the distinfo file.
+.It [no-]extra
+Check remaining files in the package directory.
+.It [no-]newpkg
+Enable special checks for uncommitted packages.
+.It [no-]patches
+Check the pkgsrc specific patch files.
+.El
+.Pp
+.Sy Warnings
+.Bl -tag -width Fl
+.It all
+Enable all warnings.
+.It none
+Disable all warnings.
+.It [no-]absname
+Warn if a file contains an absolute pathname.
+.It [no-]directcmd
+Warn if a system command name is used instead of a variable (e.g. sed
+instead of ${SED}).
+.It [no-]exec
+Warn if a file in the package directory is executable.
+Most files should not be executable until they are installed.
+.It [no-]paren
+Warn if variables are used like $(VAR) in Makefiles.
+They should always be used like ${VAR}.
+.It [no-]patches
+Warn if a patch file patches more than one file or if the patches differ
+from what is recorded in the distinfo file.
+.El
+.Pp
+.Bl -tag -width Fl
+.Sy Other arguments
 .It dir
 The pkgsrc directory to be checked.
 If omitted, all checks will be performed on the current directory.
 .El
 .Sh FILES
 .Bl -tag -width /usr/share/mk/bsd.port.mk -compact
-.\".It FreeBSD:
-.\".It Pa /usr/share/mk/bsd.port.mk
-.\"master Makefile for ports
-.\".It Pa /usr/ports/*
-.\"port collection
-.\".Pp
-.\".It NetBSD:
 .It Pa /usr/pkgsrc/mk/bsd.pkg.mk
 master Makefile for pkgsrc
 .It Pa /usr/pkgsrc/*
 pkgsrc collection
 .El
+.Sh EXAMPLES
+.Bl -tag -width Fl
+.Ic "pkglint -Cnone,patches ."
+Checks the patches of the package in the current directory.
+.Ic "pkglint -Wall /usr/pkgsrc/devel"
+Checks the category Makefile and reports any warnings it can find.
+.El
 .Sh DIAGNOSTICS
 Messages will be sent to standard output, not standard error.
 .Bl -tag -width "WARN: foobaa"
@@ -83,8 +141,8 @@ This type of error messages suggests that there is some fatal error
 in the pkgsrc directory.
 For example, if some files need a rewrite, or if
 some necessary files are missing, this message will show up.
-This kind of error should be fixed BEFORE submitting
-a pkgsrc via send-pr to the comitters.
+This kind of error should be fixed BEFORE submitting a package via
+.Xr send-pr 1 .
 .\"If a submitter submit it without update, committers will need to rewrite
 .\"on behalf of the submitters, which may result in delay of
 .\"the development of operating system itself.
@@ -107,11 +165,14 @@ This type of messages is used in verbose mode
 .El
 .Sh AUTHORS
 Jun-ichiro Itoh
-.Aq itojun@itojun.org
-and
+.Aq itojun@itojun.org ,
 Yoshishige Arai
-.Aq ryo2@on.rim.or.jp .
+.Aq ryo2@on.rim.or.jp ,
+and Roland Illig
+.Aq roland.illig@gmx.de .
 Many people have contributed patches and comments/suggestions.
 .Sh BUGS
 .Nm
-is not a magic wand, as described above.
+still emits too many false positive warnings.
+Many of the warnings concerning Makefiles do not print the file
+and line number where the warning originated.
diff --git a/pkgtools/pkglint/files/pkglint.pl b/pkgtools/pkglint/files/pkglint.pl
index ed829f7b9fe..16f73d67a13 100644
--- a/pkgtools/pkglint/files/pkglint.pl
+++ b/pkgtools/pkglint/files/pkglint.pl
@@ -11,7 +11,7 @@
 # Freely redistributable.  Absolutely no warranty.
 #
 # From Id: portlint.pl,v 1.64 1998/02/28 02:34:05 itojun Exp
-# $NetBSD: pkglint.pl,v 1.118 2004/08/24 15:18:29 wiz Exp $
+# $NetBSD: pkglint.pl,v 1.119 2004/09/24 15:33:26 wiz Exp $
 #
 # This version contains lots of changes necessary for NetBSD packages
 # done by Hubert Feyrer <hubertf@netbsd.org>,
@@ -81,6 +81,9 @@ my $verbose_flag	= false;
 sub log_message($$$$)
 {
 	my ($file, $lineno, $type, $message) = @_;
+	if ($file ne NO_FILE) {
+		$file =~ s,^(?:\./)+,,;
+	}
 	if ($file eq NO_FILE) {
 		printf("%s: %s\n", $type, $message);
 	} elsif ($lineno == NO_LINE_NUMBER) {
@@ -204,7 +207,7 @@ package main;
 use strict;
 use warnings;
 
-use Getopt::Std;
+use Getopt::Long qw(:config no_ignore_case bundling require_order);
 use File::Basename;
 use FileHandle;
 use Cwd;
@@ -229,13 +232,50 @@ my $conf_distver	= '@DISTVER@';
 my $conf_make		= '@MAKE@';
 
 # Command Line Options
-my $opt_extrafile	= true; # check all files we can find for simple errors
-my $opt_parenwarn	= true; # warn about use for $(VAR) instead of ${VAR}
-my $opt_committer	= true; # check items especially for package developers
-my $opt_newpackage	= false; # consider this package new (uncommitted)
-my $opt_dumpmakefile	= false; # dump the Makefile after parsing
-my $opt_contblank	= 1; # number of allowed contiguous blank lines
-my $opt_packagedir	= "."; # directory to check
+my $opt_committer	= true;
+my $opt_dumpmakefile	= false;
+my $opt_contblank	= 1;
+my $opt_packagedir	= ".";
+my (%options) = (
+	"-p"		=> "warn about use of \$(VAR) instead of \${VAR}",
+	"-d"		=> "check items useful for package developers",
+	"-I"		=> "dump the Makefile after parsing",
+	"-N"		=> "assume a new (still uncommitted) package",
+	"-B#"		=> "allow # contiguous blank lines in Makefiles",
+	"-C{check,...}"	=> "enable or disable specific checks",
+	"-W{warn,...}"	=> "enable or disable specific warnings",
+	"-h|--help"	=> "print a detailed help message",
+	"-V|--version"	=> "print the version number of pkglint",
+	"-v|--verbose"	=> "print progress messages on STDERR",
+);
+
+my $opt_check_distinfo	= true;
+my $opt_check_extra	= true;
+my $opt_check_MESSAGE	= true;
+my $opt_check_patches	= true;
+my $opt_check_PLIST	= true;
+my $opt_check_newpkg	= false;
+my (%checks) = (
+	"distinfo"	=> [\$opt_check_distinfo, "check distinfo file"],
+	"extra"		=> [\$opt_check_extra, "check various additional files"],
+	"MESSAGE"	=> [\$opt_check_MESSAGE, "check MESSAGE files"],
+	"patches"	=> [\$opt_check_patches, "check patches"],
+	"PLIST"		=> [\$opt_check_PLIST, "check PLIST files"],
+	"newpkg"	=> [\$opt_check_newpkg, "special checks for uncommitted packages"],
+);
+
+my $opt_warn_patches	= true;
+my $opt_warn_exec	= true;
+my $opt_warn_absname	= true;
+my $opt_warn_directcmd	= true;
+my $opt_warn_paren	= true;
+my (%warnings) = (
+	"patches"	=> [\$opt_warn_patches, "warn on non-optimal patch files"],
+	"exec"		=> [\$opt_warn_exec, "warn if source files are executable"],
+	"absname"	=> [\$opt_warn_absname, "warn about use of absolute file names"],
+	"directcmd"	=> [\$opt_warn_directcmd, "warn about use of direct command names instead of Make variables"],
+	"paren"		=> [\$opt_warn_paren, "warn about usa of \$(VAR) instead of \${VAR} in Makefiles"],
+);
 
 # Constants
 my $regex_rcsidstr	= qr"\$($conf_rcsidstr)(?::[^\$]*|)\$";
@@ -280,29 +320,127 @@ sub is_predefined($);
 sub category_check();
 sub check_package();
 
+sub print_table($$)
+{
+	my ($out, $table) = @_;
+	my (@width) = ();
+	foreach my $row (@$table) {
+		foreach my $i (0..(scalar(@$row)-1)) {
+			if (!defined($width[$i]) || length($row->[$i]) > $width[$i]) {
+				$width[$i] = length($row->[$i]);
+			}
+		}
+	}
+	foreach my $row (@$table) {
+		my ($max) = (scalar(@$row) - 1);
+		foreach my $i (0..$max) {
+			if ($i != 0) {
+				print $out ("  ");
+			}
+			print $out ($row->[$i]);
+			if ($i != $max) {
+				print $out (" " x ($width[$i] - length($row->[$i])));
+			}
+		}
+		print $out ("\n");
+	}
+	return 1;
+}
+
+sub help($$$) {
+	my ($out, $exitval, $show_all) = @_;
+	my ($prog) = (basename($0));
+	print $out ("usage: $prog [options] [package_directory]\n\n");
+
+	my (@option_table) = ();
+	foreach my $opt (sort keys %options) {
+		push(@option_table, ["  ", $opt, $options{$opt}]);
+	}
+	print $out ("options:\n");
+	print_table($out, \@option_table);
+	print $out ("\n");
+
+	if (!$show_all) {
+		exit($exitval);
+	}
+
+	my (@checks_table) = (
+		["  ", "all", "", "enable all checks"],
+		["  ", "none", "", "disable all checks"],
+	);
+	foreach my $check (sort keys %checks) {
+		push(@checks_table, [ "  ", $check,
+			(${$checks{$check}->[0]} ? "(enabled)" : "(disabled)"),
+			$checks{$check}->[1]]);
+	}
+	print $out ("checks: (use \"check\" to enable, \"no-check\" to disable)\n");
+	print_table($out, \@checks_table);
+	print $out ("\n");
+
+	my (@warnings_table) = (
+		["  ", "all", "", "enable all warnings"],
+		["  ", "none", "", "disable all warnings"],
+	);
+	foreach my $warning (sort keys %warnings) {
+		push(@warnings_table, [ "  ", $warning,
+			(${$warnings{$warning}->[0]} ? "(enabled)" : "(disabled)"),
+			$warnings{$warning}->[1]]);
+	}
+	print $out ("warnings: (use \"warn\" to enable, \"no-warn\" to disable)\n");
+	print_table($out, \@warnings_table);
+	print $out ("\n");
+
+	exit($exitval);
+}
+
+sub parse_multioption($$) {
+	my ($value, $optdefs) = @_;
+	foreach my $opt (split(qr",", $value)) {
+		if ($opt eq "none") {
+			foreach my $key (keys %$optdefs) {
+				${$optdefs->{$key}->[0]} = false;
+			}
+		} elsif ($opt eq "all") {
+			foreach my $key (keys %$optdefs) {
+				${$optdefs->{$key}->[0]} = true;
+			}
+		} else {
+			my ($value) = (($opt =~ s/^no-//) ? false : true);
+			if (exists($optdefs->{$opt})) {
+				${$optdefs->{$opt}->[0]} = $value;
+			} else {
+				help(*STDERR, 1, 0);
+			}
+		}
+	}
+	return true;
+}
+
 sub parse_command_line() {
-	my %opts = ();
-	getopts('hINB:qvV', \%opts);
-	if ($opts{"h"}) {
-		my $prog = basename($0);
-		print STDERR <<EOF;
-usage: $prog [-qvIN] [-B#] [package_directory]
-	-v	verbose mode
-	-V	version ($conf_distver)
-	-I	show Makefile (with all included files)
-	-N	writing a new package
-	-B#	allow # contiguous blank lines (default: $opt_contblank line)
-EOF
-		exit 0;
+	my (%options) = (
+		"warning|W=s" => sub {
+			my ($opt, $val) = @_;
+			parse_multioption($val, \%warnings);
+		},
+		"check|C=s" => sub {
+			my ($opt, $val) = @_;
+			parse_multioption($val, \%checks);
+		},
+		"help|h" => sub { help(*STDOUT, 0, 1); },
+		"newpackage|N" => \$opt_check_newpkg,
+		"verbose|v" => sub { PkgLint::Logging::set_verbose(true); },
+		"version|V" => sub { print("$conf_distver\n"); exit(0); },
+		"conblank|B=i" => \$opt_contblank,
+		"dumpmakefile|I" => \$opt_dumpmakefile,
+	);
+	{
+		local $SIG{__WARN__} = sub {};
+		if (!GetOptions(%options)) {
+			help(*STDERR, 1, false);
+		}
 	}
-	if ($opts{"v"}) { PkgLint::Logging::set_verbose(true); }
-	if ($opts{"N"}) { $opt_newpackage = true; }
-	if ($opts{"I"}) { $opt_dumpmakefile = true; }
-	if ($opts{"B"}) { $opt_contblank = $opts{"B"}; }
-	if (scalar(@ARGV)) { $opt_packagedir = shift(@ARGV); }
-	if ($opts{"V"}) {
-		print "$conf_distver\n";
-		exit;
+	if (@ARGV) {
+		$opt_packagedir = shift(@ARGV);
 	}
 	return true;
 }
@@ -371,39 +509,40 @@ EOF
 	my @checker = ("$pkgdir/DESCR");
 	my %checker = ("$pkgdir/DESCR", \&checkfile_DESCR);
 
-	if ($opt_extrafile) {
-		foreach my $i ((<$opt_packagedir/$filesdir/*>, <$opt_packagedir/$pkgdir/*>)) {
+	if ($opt_check_MESSAGE) {
+		foreach my $msg (<$opt_packagedir/$filesdir/*>, <$opt_packagedir/$pkgdir/*>) {
+			if ($msg =~ qr"MESSAGE") {
+				push(@checker, $msg);
+				$checker{$msg} = \&checkfile_MESSAGE;
+			}
+		}
+	}
+	if ($opt_check_PLIST) {
+		foreach my $plist (<$opt_packagedir/$filesdir/*>, <$opt_packagedir/$pkgdir/*>) {
+			if ($plist =~ qr"PLIST") {
+				push(@checker, $plist);
+				$checker{$plist} = \&checkfile_PLIST;
+			}
+		}
+	}
+	if ($opt_check_patches) {
+		foreach my $i (<$opt_packagedir/$patchdir/patch-*>) {
 			next if (! -T $i);
-			next if ($i =~ /distinfo$/);
-			next if ($i =~ /Makefile$/);
 			$i =~ s/^\Q$opt_packagedir\E\///;
 			next if (defined $checker{$i});
-			if ($i =~ /MESSAGE/) {
-				unshift(@checker, $i);
-				$checker{$i} = \&checkfile_MESSAGE;
-			} elsif ($i =~ /PLIST/) {
-			        unshift(@checker, $i);
-				$checker{$i} = \&checkfile_PLIST;
-			} else {
-			        push(@checker, $i);
-				$checker{$i} = \&checkpathname;
-			}
+			push(@checker, $i);
+			$checker{$i} = \&checkfile_patches_patch;
 		}
 	}
-	foreach my $i (<$opt_packagedir/$patchdir/patch-*>) {
-		next if (! -T $i);
-		$i =~ s/^\Q$opt_packagedir\E\///;
-		next if (defined $checker{$i});
-		push(@checker, $i);
-		$checker{$i} = \&checkfile_patches_patch;
-	}
-	if (-f "$opt_packagedir/$distinfo") {
-		my $i = "$distinfo";
-		next if (defined $checker{$i});
-		push(@checker, $i);
-		$checker{$i} = \&checkfile_distinfo;
+	if ($opt_check_distinfo) {
+		if (-f "$opt_packagedir/$distinfo") {
+			my $i = "$distinfo";
+			next if (defined $checker{$i});
+			push(@checker, $i);
+			$checker{$i} = \&checkfile_distinfo;
+		}
 	}
-	{
+	if ($opt_check_distinfo && $opt_check_patches) {
 		# Make sure there's a distinfo if there are patches
 		my $patches = false;
 		patch:
@@ -417,6 +556,18 @@ EOF
 			log_warning(NO_FILE, NO_LINE_NUMBER, "no $opt_packagedir/$distinfo file. Please run '$conf_make makepatchsum'.");
 		}
 	}
+	if ($opt_check_extra) {
+		foreach my $i ((<$opt_packagedir/$filesdir/*>, <$opt_packagedir/$pkgdir/*>)) {
+			next if (! -T $i);
+			$i =~ s/^\Q$opt_packagedir\E\///;
+			next if ($i =~ qr"(?:distinfo$|Makefile$|PLIST|MESSAGE)");
+			next if (defined $checker{$i});
+
+			push(@checker, $i);
+			$checker{$i} = \&checkpathname;
+		}
+	}
+
 	foreach my $i (@checker) {
 		log_info(NO_FILE, NO_LINE_NUMBER, "checking $i.");
 		if (! -f "$opt_packagedir/$i") {
@@ -728,7 +879,7 @@ sub checkfile_PLIST($) {
 sub checkperms($) {
 	my ($file) = @_;
 
-	if (-f $file && -x $file) {
+	if ($opt_warn_exec && -f $file && -x $file) {
 		log_warning($file, NO_LINE_NUMBER, "should not be executable.");
 	}
 	return true;
@@ -783,6 +934,10 @@ sub check_for_multiple_patches($) {
 	my ($lines) = @_;
 	my ($files_in_patch, $patch_state, $line_type);
 
+	if (!$opt_warn_patches) {
+		# all warnings are disabled, so why should we check?
+		return;
+	}
 	$files_in_patch = 0;
 	$patch_state = "";
 	foreach my $line (@$lines) {
@@ -980,7 +1135,7 @@ sub checkfile_Makefile($) {
 	#
 	# whole file: $(VARIABLE)
 	#
-	if ($opt_parenwarn) {
+	if ($opt_warn_paren) {
 		log_info(NO_FILE, NO_LINE_NUMBER, "checking for \$(VARIABLE).");
 		if ($whole =~ /\$\([\w\d]+\)/) {
 			log_warning(NO_FILE, NO_LINE_NUMBER, "use \${VARIABLE}, instead of ".
@@ -1169,10 +1324,12 @@ EOF
 	$j =~ s/\n#[\n]*/\n#/;
 	# ...nor COMMENTs
 	$j =~ s/\nCOMMENT[\t ]*=[\t ]*[^\n]*\n/\nCOMMENT=#replaced\n/;
-	foreach my $i (keys %cmdnames) {
-		if ($j =~ /[ \t\/@]$i[ \t\n;]/) {
-			log_warning(NO_FILE, NO_LINE_NUMBER, "possible direct use of command \"$i\" ".
-				"found. Use $cmdnames{$i} instead.");
+	if ($opt_warn_directcmd) {
+		foreach my $i (keys %cmdnames) {
+			if ($j =~ /[ \t\/@]$i[ \t\n;]/) {
+				log_warning(NO_FILE, NO_LINE_NUMBER, "possible direct use of command \"$i\" ".
+					"found. Use $cmdnames{$i} instead.");
+			}
 		}
 	}
 
@@ -1242,9 +1399,9 @@ EOF
 				"right before \$$conf_rcsidstr\$ tag.");
 		}
 		if ($2 ne '') {
-			if (PkgLint::Logging::is_verbose || $opt_newpackage) {	# XXX
+			if (PkgLint::Logging::is_verbose || $opt_check_newpkg) {	# XXX
 				log_warning(NO_FILE, NO_LINE_NUMBER, "".
-				    ($opt_newpackage ? 'for new package, '
+				    ($opt_check_newpkg ? 'for new package, '
 					      : 'is it a new package? if so, ').
 				    "make \$$conf_rcsidstr\$ tag in comment ".
 				    "section empty, to make CVS happy.");
@@ -1815,7 +1972,9 @@ sub abspathname($$) {
 		if ($i ne '') {
 			$i =~ s/\s.*$//;
 			$i =~ s/['"].*$//;
-			log_warning($file, NO_LINE_NUMBER, "possible use of absolute pathname \"$i\".");
+			if ($opt_warn_absname) {
+				log_warning($file, NO_LINE_NUMBER, "possible use of absolute pathname \"$i\".");
+			}
 		}
 	}