- Added check for unused license files

- Improved allowed filenames for patches
- Added license for pkglint itself
- Cleaned up Makefile and TODO

3 files changed, 30 insertions, 21 deletions

diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile
index 944ea2057cd..93bc9c913de 100644
--- a/pkgtools/pkglint/Makefile
+++ b/pkgtools/pkglint/Makefile
@@ -1,4 +1,4 @@
-# $NetBSD: Makefile,v 1.434 2013/09/12 11:18:41 jperkin Exp $
+# $NetBSD: Makefile,v 1.435 2013/10/12 18:09:59 rillig Exp $
 
 # Note: if you update the version number, please have a look at the
 # changes between the CVS tag "pkglint_current" and HEAD.
@@ -11,6 +11,7 @@ DISTFILES=	# none
 OWNER=		wiz@NetBSD.org
 HOMEPAGE=	http://www.NetBSD.org/docs/pkgsrc/
 COMMENT=	Verifier for NetBSD packages
+LICENSE=	2-clause-bsd
 
 DEPENDS+=	p5-Digest-SHA1-[0-9]*:../../security/p5-Digest-SHA1
 DEPENDS+=	p5-enum>=1.016:../../devel/p5-enum
@@ -48,13 +49,6 @@ SUBST_STAGE.mappaths=	pre-install
 SUBST_FILES.mappaths+=	pkglint.pl
 SUBST_SED.mappaths+=	-e s\|${FILESDIR}\|${PREFIX}/share/pkglint\|g
 
-# Note: This target is only intended for use by the pkglint author.
-.PHONY: quick-install
-quick-install:
-	${RM} -rf ${WRKSRC}
-	${MKDIR} ${WRKSRC}
-	${MAKE} do-extract subst-pkglint do-build do-install selftest clean
-
 do-extract:
 	cd ${FILESDIR} && ${CP} build.pl pkglint.0 pkglint.1 pkglint.pl pkglint.t plist-clash.pl ${WRKSRC}
 	mkdir ${WRKSRC}/PkgLint
diff --git a/pkgtools/pkglint/TODO b/pkgtools/pkglint/TODO
index a8a526c889f..74f0e5e192a 100644
--- a/pkgtools/pkglint/TODO
+++ b/pkgtools/pkglint/TODO
@@ -1,18 +1,12 @@
-$NetBSD: TODO,v 1.77 2013/10/12 15:01:45 rillig Exp $
+$NetBSD: TODO,v 1.78 2013/10/12 18:09:59 rillig Exp $
 
 Please add your own entries at the bottom of this file. If possible,
 include the name of an example package where a warning should occur.
 
 * When you understand some behavior of the code, document it by
   adding automated tests to pkglint.t!
-* fix false positive warnings
 * warn about the use of ${WRKDIR:=...}, as this construct should only
   be used with lists.
-* record with each substring its source location to be able to make
-  even more precise diagnostics
-* Many of the checking routines need some context to provide better
-  warnings. For example, checkline_foo() may need to know what has
-  happened in the file before that line.
 * Add checks for binary packages. See Debian/lintian for ideas.
 * Of the user-defined variables, some may be used at load-time and some
   don't. Find out how pkglint can distinguish them.
diff --git a/pkgtools/pkglint/files/pkglint.pl b/pkgtools/pkglint/files/pkglint.pl
index 492a19ccc14..46cb3893bab 100644
--- a/pkgtools/pkglint/files/pkglint.pl
+++ b/pkgtools/pkglint/files/pkglint.pl
@@ -1,5 +1,5 @@
 #! @PERL@
-# $NetBSD: pkglint.pl,v 1.859 2013/09/01 05:30:15 obache Exp $
+# $NetBSD: pkglint.pl,v 1.860 2013/10/12 18:09:59 rillig Exp $
 #
 
 # pkglint - static analyzer and checker for pkgsrc packages
@@ -367,9 +367,11 @@ my $is_internal;		# Is the current item from the infrastructure?
 #
 
 my $ipc_distinfo;		# Maps "$alg:$fname" => "checksum".
+my %ipc_used_licenses;		# { license name => true }
+my $ipc_checking_root_recursively; # For checking unused licenses
 
 # Context of the package that is currently checked.
-my $pkgpath;			# The relative path to the package within PKGSRC.
+my $pkgpath;			# The relative path to the package within PKGSRC
 my $pkgdir;			# PKGDIR from the package Makefile
 my $filesdir;			# FILESDIR from the package Makefile
 my $patchdir;			# PATCHDIR from the package Makefile
@@ -2111,9 +2113,8 @@ sub parse_mk_cond($$) {
 sub parse_licenses($) {
 	my ($licenses) = @_;
 
-	# XXX: this is clearly cheating
 	$licenses =~ s,\${PERL5_LICENSE},gnu-gpl-v2 OR artistic,g;
-	$licenses =~ s,[()]|AND|OR,,g;
+	$licenses =~ s,[()]|AND|OR,,g; # XXX: treats OR like AND
 	my @licenses = split(/\s+/, $licenses);
 	return \@licenses;
 }
@@ -2437,6 +2438,18 @@ sub checkword_absolute_pathname($$) {
 	}
 }
 
+sub check_unused_licenses() {
+
+	for my $licensefile (<${cwd_pkgsrcdir}/licenses/*>) {
+		if (-f $licensefile) {
+			my $licensename = basename($licensefile);
+			if (!exists($ipc_used_licenses{$licensename})) {
+				log_warning($licensefile, NO_LINES, "This license seems to be unused.");
+			}
+		}
+	}
+}
+
 sub checkpackage_possible_downgrade() {
 
 	$opt_debug_trace and log_debug(NO_FILE, NO_LINES, "checkpackage_possible_downgrade");
@@ -4125,7 +4138,10 @@ sub checkline_mk_vartype_basic($$$$$$$$) {
 					my $license_file_line = $pkgctx_vardef->{"LICENSE_FILE"};
 
 					$license_file = "${current_dir}/" . resolve_relative_path($license_file_line->get("value"), false);
+				} else {
+					$ipc_used_licenses{$license} = true;
 				}
+
 				if (!-f $license_file) {
 					$line->log_warning("License file ".normalize_pathname($license_file)." does not exist.");
 				}
@@ -7076,10 +7092,10 @@ sub checkfile($) {
 	} elsif ($basename =~ m"^MESSAGE") {
 		$opt_check_MESSAGE and checkfile_MESSAGE($fname);
 
-	} elsif ($basename =~ m"^patch-[-A-Za-z0-9_\.~]*$") {
+	} elsif ($basename =~ m"^patch-[-A-Za-z0-9_.~+]*[A-Za-z0-9_]$") {
 		$opt_check_patches and checkfile_patch($fname);
 
-	} elsif ($fname =~ m"(?:^|/)patches/manual-[^/]*$") {
+	} elsif ($fname =~ m"(?:^|/)patches/manual[^/]*$") {
 		$opt_debug_unchecked and log_debug($fname, NO_LINE_NUMBER, "Unchecked file \"${fname}\".");
 
 	} elsif ($fname =~ m"(?:^|/)patches/[^/]*$") {
@@ -7197,6 +7213,7 @@ sub checkdir_root() {
 	checklines_mk($lines);
 
 	if ($opt_recursive) {
+		$ipc_checking_root_recursively = true;
 		push(@todo_items, @subdirs);
 	}
 }
@@ -7557,6 +7574,10 @@ sub main() {
 		checkitem(shift(@todo_items));
 	}
 
+	if ($ipc_checking_root_recursively) {
+		check_unused_licenses();
+	}
+
 	PkgLint::Logging::print_summary_and_exit($opt_quiet);
 }