pkgtools/pkglint: update to 22.2.4

Changes since 22.2.3:

Manual pages must not be listed in the ALTERNATIVES file. Instead, they
are handled automatically based on the program in bin/ or sbin/.

Detect packages that use the tool pkg-config even though they don't need
it.

7 files changed, 162 insertions, 29 deletions

diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile
index 72f2912121e..6642f8c53c8 100644
--- a/pkgtools/pkglint/Makefile
+++ b/pkgtools/pkglint/Makefile
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.723 2022/07/24 20:07:20 rillig Exp $
+# $NetBSD: Makefile,v 1.724 2022/07/28 06:37:04 rillig Exp $
 
-PKGNAME=	pkglint-22.2.3
+PKGNAME=	pkglint-22.2.4
 CATEGORIES=	pkgtools
 DISTNAME=	tools
 MASTER_SITES=	${MASTER_SITE_GITHUB:=golang/}
diff --git a/pkgtools/pkglint/files/alternatives.go b/pkgtools/pkglint/files/alternatives.go
index 41839ff4dc8..daeaa38e28a 100644
--- a/pkgtools/pkglint/files/alternatives.go
+++ b/pkgtools/pkglint/files/alternatives.go
@@ -47,10 +47,9 @@ func (ck *AlternativesChecker) checkLine(line *Line, plistFiles map[RelPath]*Pli
 			line.Errorf("Alternative wrapper %q must not appear in the PLIST.", wrapper)
 		}
 		if !wrapper.HasPrefixText("bin/") &&
-			!wrapper.HasPrefixText("@PKGMANDIR@/") &&
 			!wrapper.HasPrefixText("sbin/") {
 			line.Errorf("Alternative wrapper %q must be in "+
-				"\"bin\", \"@PKGMANDIR@\" or \"sbin\".", wrapper)
+				"\"bin\" or \"sbin\".", wrapper)
 		}
 	}
 
@@ -95,9 +94,6 @@ func (ck *AlternativesChecker) checkAlternativePlist(line *Line, alternative str
 	if plistFiles[rel] != nil || pkg.vars.IsDefined("ALTERNATIVES_SRC") {
 		return
 	}
-	if plistFiles[rel.Replace("${PKGMANDIR}", "man")] != nil {
-		return
-	}
 
 	if plistName == alternative {
 		line.Errorf("Alternative implementation %q must appear in the PLIST.",
diff --git a/pkgtools/pkglint/files/alternatives_test.go b/pkgtools/pkglint/files/alternatives_test.go
index a6733de5e38..0950f29398d 100644
--- a/pkgtools/pkglint/files/alternatives_test.go
+++ b/pkgtools/pkglint/files/alternatives_test.go
@@ -72,7 +72,7 @@ func (s *Suite) Test_AlternativesChecker_Check__PLIST(c *check.C) {
 		"ERROR: ALTERNATIVES:6: Alternative implementation \"${PREFIX}/bin/firefox\" must appear in the PLIST.",
 		"ERROR: ALTERNATIVES:6: Alternative implementation \"${PREFIX}/bin/firefox\" must be an absolute path.",
 		"ERROR: ALTERNATIVES:7: Alternative wrapper \"highscores\" "+
-			"must be in \"bin\", \"@PKGMANDIR@\" or \"sbin\".",
+			"must be in \"bin\" or \"sbin\".",
 		"ERROR: ALTERNATIVES:7: Alternative implementation \"@VARBASE@/game/scores\" "+
 			"must appear in the PLIST as \"${VARBASE}/game/scores\".")
 
@@ -120,7 +120,7 @@ func (s *Suite) Test_AlternativesChecker_checkLine__absolute(c *check.C) {
 
 	t.CheckOutputLines(
 		"ERROR: ~/ALTERNATIVES:1: Alternative wrapper \"relative\" "+
-			"must be in \"bin\", \"@PKGMANDIR@\" or \"sbin\".",
+			"must be in \"bin\" or \"sbin\".",
 		"ERROR: ~/ALTERNATIVES:2: Alternative wrapper \"/absolute\" "+
 			"must be relative to PREFIX.")
 }
@@ -160,11 +160,11 @@ func (s *Suite) Test_AlternativesChecker_checkLine__dir(c *check.C) {
 
 	t.CheckOutputLines(
 		"ERROR: ~/ALTERNATIVES:1: Alternative wrapper \"in/typo\" "+
-			"must be in \"bin\", \"@PKGMANDIR@\" or \"sbin\".",
+			"must be in \"bin\" or \"sbin\".",
 		"ERROR: ~/ALTERNATIVES:3: Alternative wrapper \"typo/program\" "+
-			"must be in \"bin\", \"@PKGMANDIR@\" or \"sbin\".",
+			"must be in \"bin\" or \"sbin\".",
 		"ERROR: ~/ALTERNATIVES:4: Alternative wrapper \"man/man1/program.1\" "+
-			"must be in \"bin\", \"@PKGMANDIR@\" or \"sbin\".")
+			"must be in \"bin\" or \"sbin\".")
 }
 
 func (s *Suite) Test_AlternativesChecker_checkAlternativeAbs(c *check.C) {
@@ -211,12 +211,9 @@ func (s *Suite) Test_AlternativesChecker_checkAlternativePlist__conditional(c *c
 			"must appear in the PLIST as \"bin/not-found\".")
 }
 
-// When a man page is mentioned in the ALTERNATIVES file, it must use the
-// PKGMANDIR variable. In the PLIST files though, there is some magic
-// in the pkgsrc infrastructure that maps man/ to ${PKGMANDIR}, which
-// leads to a bit less typing.
-//
-// Seen in graphics/py-blockdiag.
+// Manual pages must not be listed in the ALTERNATIVES file.
+// Instead, they are handled automatically based on the program in bin/ or
+// sbin/.
 func (s *Suite) Test_AlternativesChecker_checkAlternativePlist__PKGMANDIR(c *check.C) {
 	t := s.Init(c)
 
@@ -230,5 +227,13 @@ func (s *Suite) Test_AlternativesChecker_checkAlternativePlist__PKGMANDIR(c *che
 
 	G.Check(t.File("category/package"))
 
-	t.CheckOutputEmpty()
+	t.CheckOutputLines(
+		"ERROR: ~/category/package/ALTERNATIVES:1: "+
+			"Alternative wrapper \"@PKGMANDIR@/man1/blockdiag\" "+
+			"must be in \"bin\" or \"sbin\".",
+		"ERROR: ~/category/package/ALTERNATIVES:1: "+
+			"Alternative implementation "+
+			"\"@PREFIX@/@PKGMANDIR@/man1/blockdiag-@PYVERSSUFFIX@.1\" "+
+			"must appear in the PLIST as "+
+			"\"${PKGMANDIR}/man1/blockdiag-${PYVERSSUFFIX}.1\".")
 }
diff --git a/pkgtools/pkglint/files/mkcondsimplifier.go b/pkgtools/pkglint/files/mkcondsimplifier.go
index 451a30753c7..5e1e9ab3893 100644
--- a/pkgtools/pkglint/files/mkcondsimplifier.go
+++ b/pkgtools/pkglint/files/mkcondsimplifier.go
@@ -136,7 +136,31 @@ func (s *MkCondSimplifier) simplifyMatch(varuse *MkVarUse, fromEmpty bool, neg b
 		return
 	}
 
-	if !(fromEmpty && positive && !exact && vartype != nil) {
+	if !fromEmpty {
+		return // Already using the simple form.
+	}
+
+	// Only simplify the form ':M' for now, but not ':N'.
+	// The form ':M' is much more popular than ':N'.
+	if !positive {
+		return
+	}
+
+	// For now, only suggest the replacement if the pattern
+	// actually contains wildcards. This mainly affects
+	// PKG_OPTIONS in the second part of options.mk files.
+	// There are many of these, and the pkgsrc developers got
+	// used to using '!empty' for them.
+	if exact {
+		return
+	}
+
+	// TODO: Even if the variable type is not known,
+	//  the RedundantScope may know that the variable is always
+	//  defined at this point, so that the variable expression
+	//  does not need the modifier ':U'.
+	//  Example: doc/guide/Makefile.common, _GUIDE_OUTPUTS.
+	if vartype == nil {
 		return
 	}
 
diff --git a/pkgtools/pkglint/files/package.go b/pkgtools/pkglint/files/package.go
index d0a1ee48069..19962704513 100644
--- a/pkgtools/pkglint/files/package.go
+++ b/pkgtools/pkglint/files/package.go
@@ -47,13 +47,12 @@ type Package struct {
 	bl3Data map[Buildlink3ID]*Buildlink3Data
 
 	// Remembers the Makefile fragments that have already been included.
-	// The key to the map is the filename relative to the package directory.
 	// Typical keys are "../../category/package/buildlink3.mk".
 	//
 	// TODO: Include files with multiple-inclusion guard only once.
 	//
 	// TODO: Include files without multiple-inclusion guard as often as needed.
-	included Once
+	included IncludedMap
 
 	// Does the package have any .includes?
 	//
@@ -110,7 +109,7 @@ func NewPackage(dir CurrPath) *Package {
 		vars:                  NewScope(),
 		bl3:                   make(map[PackagePath]*MkLine),
 		bl3Data:               make(map[Buildlink3ID]*Buildlink3Data),
-		included:              Once{},
+		included:              NewIncludedMap(),
 		conditionalIncludes:   make(map[PackagePath]*MkLine),
 		unconditionalIncludes: make(map[PackagePath]*MkLine),
 	}
@@ -295,8 +294,8 @@ func (pkg *Package) parse(mklines *MkLines, allLines *MkLines, includingFileForU
 	filename := mklines.lines.Filename
 	if mklines.lines.BaseName == "buildlink3.mk" {
 		builtin := filename.Dir().JoinNoClean("builtin.mk").CleanPath()
-		builtinRel := G.Pkgsrc.Relpath(pkg.dir, builtin)
-		if pkg.included.FirstTime(builtinRel.String()) && builtin.IsFile() {
+		builtinRel := NewPackagePath(G.Pkgsrc.Relpath(pkg.dir, builtin))
+		if pkg.included.FirstTime(builtinRel) && builtin.IsFile() {
 			builtinMkLines := LoadMk(builtin, pkg, MustSucceed|LogErrors)
 			pkg.parse(builtinMkLines, allLines, "", false)
 		}
@@ -377,13 +376,13 @@ func (pkg *Package) loadIncluded(mkline *MkLine, includingFile CurrPath) (includ
 	dirname, _ := includingFile.Split()
 	dirname = dirname.CleanPath()
 	fullIncluded := dirname.JoinNoClean(includedFile)
-	relIncludedFile := G.Pkgsrc.Relpath(pkg.dir, fullIncluded)
 
 	if !pkg.shouldDiveInto(includingFile, includedFile) {
 		return nil, true
 	}
 
-	if !pkg.included.FirstTime(relIncludedFile.String()) {
+	relIncludedFile := NewPackagePath(G.Pkgsrc.Relpath(pkg.dir, fullIncluded))
+	if !pkg.included.FirstTime(relIncludedFile) {
 		return nil, true
 	}
 
@@ -604,6 +603,7 @@ func (pkg *Package) check(filenames []CurrPath, mklines, allLines *MkLines) {
 	}
 
 	pkg.checkDistfilesInDistinfo(allLines)
+	pkg.checkPkgConfig(allLines)
 }
 
 func (pkg *Package) checkDescr(filenames []CurrPath, mklines *MkLines) {
@@ -655,6 +655,34 @@ func (pkg *Package) checkDistfilesInDistinfo(mklines *MkLines) {
 	}
 }
 
+func (pkg *Package) checkPkgConfig(allLines *MkLines) {
+	pkgConfig := allLines.Tools.ByName("pkg-config")
+	if pkgConfig == nil || !pkgConfig.UsableAtRunTime() {
+		return
+	}
+
+	for included := range pkg.included.m {
+		included := included.String()
+		if hasSuffix(included, "buildlink3.mk") ||
+			hasSuffix(included, "/mk/apache.mk") ||
+			hasSuffix(included, "/mk/apache.module.mk") {
+			return
+		}
+	}
+
+	mkline := allLines.mklines[0]
+	mkline.Warnf("The package uses the tool \"pkg-config\" " +
+		"but doesn't include any buildlink3 file.")
+	mkline.Explain(
+		"The pkgsrc tool wrappers replace the \"pkg-config\" command",
+		"with a pkg-config implementation that looks in the buildlink3",
+		"directory.",
+		"This directory is populated by including the dependencies via",
+		"the buildlink3.mk files.",
+		"Since this package does not include any such files, the buildlink3",
+		"directory will be empty and pkg-config will not find anything.")
+}
+
 func (pkg *Package) checkfilePackageMakefile(filename CurrPath, mklines *MkLines, allLines *MkLines) {
 	if trace.Tracing {
 		defer trace.Call(filename)()
@@ -1802,6 +1830,34 @@ func NewPlistContent() PlistContent {
 		make(map[string]bool)}
 }
 
+// IncludedMap remembers which files the package Makefile has included,
+// including indirect files.
+// See Once.
+type IncludedMap struct {
+	m     map[PackagePath]struct{}
+	Trace bool
+}
+
+func NewIncludedMap() IncludedMap {
+	return IncludedMap{make(map[PackagePath]struct{}), false}
+}
+
+func (im *IncludedMap) FirstTime(p PackagePath) bool {
+	_, found := im.m[p]
+	if !found {
+		im.m[p] = struct{}{}
+		if im.Trace {
+			G.Logger.out.WriteLine("FirstTime: " + p.String())
+		}
+	}
+	return !found
+}
+
+func (im *IncludedMap) Seen(p PackagePath) bool {
+	_, seen := im.m[p]
+	return seen
+}
+
 // matchPkgname tests whether the string has the form of a package name that
 // does not contain any variable expressions.
 func matchPkgname(s string) (m bool, base string, version string) {
diff --git a/pkgtools/pkglint/files/package_test.go b/pkgtools/pkglint/files/package_test.go
index cb553260022..f74074c0ba0 100644
--- a/pkgtools/pkglint/files/package_test.go
+++ b/pkgtools/pkglint/files/package_test.go
@@ -1581,6 +1581,58 @@ func (s *Suite) Test_Package_checkDistfilesInDistinfo__no_distfiles(c *check.C)
 		"WARN: distinfo: This file should not exist.")
 }
 
+func (s *Suite) Test_Package_checkPkgConfig__no_buildlink3(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpTool("pkg-config", "", Nowhere)
+	t.SetUpPackage("category/package",
+		"USE_TOOLS+=\tpkg-config")
+	t.Chdir("category/package")
+	t.FinishSetUp()
+
+	pkg := NewPackage(".")
+	pkg.Check()
+
+	t.CheckOutputLines(
+		"WARN: Makefile:1: The package uses the tool \"pkg-config\" " +
+			"but doesn't include any buildlink3 file.")
+}
+
+func (s *Suite) Test_Package_checkPkgConfig__plain_buildlink3(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpTool("pkg-config", "", Nowhere)
+	t.SetUpPackage("category/package",
+		"USE_TOOLS+=\tpkg-config",
+		".include \"../../devel/library/buildlink3.mk\"")
+	t.SetUpPackage("devel/library")
+	t.CreateFileBuildlink3("devel/library/buildlink3.mk")
+	t.Chdir("category/package")
+	t.FinishSetUp()
+
+	pkg := NewPackage(".")
+	pkg.Check()
+
+	t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_Package_checkPkgConfig__mk_buildlink3(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpTool("pkg-config", "", Nowhere)
+	t.SetUpPackage("category/package",
+		"USE_TOOLS+=\tpkg-config",
+		".include \"../../mk/curses.buildlink3.mk\"")
+	t.CreateFileLines("mk/curses.buildlink3.mk")
+	t.Chdir("category/package")
+	t.FinishSetUp()
+
+	pkg := NewPackage(".")
+	pkg.Check()
+
+	t.CheckOutputEmpty()
+}
+
 func (s *Suite) Test_Package_checkfilePackageMakefile__GNU_CONFIGURE(c *check.C) {
 	t := s.Init(c)
 
diff --git a/pkgtools/pkglint/files/plist.go b/pkgtools/pkglint/files/plist.go
index 43ecb7094d1..625db8452ca 100644
--- a/pkgtools/pkglint/files/plist.go
+++ b/pkgtools/pkglint/files/plist.go
@@ -444,7 +444,7 @@ func (ck *PlistChecker) checkPathShareIcons(pline *PlistLine) {
 
 	if hasPrefix(text, "share/icons/hicolor/") && pkg.Pkgpath != "graphics/hicolor-icon-theme" {
 		f := "../../graphics/hicolor-icon-theme/buildlink3.mk"
-		if !pkg.included.Seen(f) && ck.once.FirstTime("hicolor-icon-theme") {
+		if !pkg.included.Seen(NewPackagePathString(f)) && ck.once.FirstTime("hicolor-icon-theme") {
 			pline.Errorf("Packages that install hicolor icons must include %q in the Makefile.", f)
 		}
 	}
@@ -459,7 +459,7 @@ func (ck *PlistChecker) checkPathShareIcons(pline *PlistLine) {
 
 	if hasPrefix(text, "share/icons/gnome") && pkg.Pkgpath != "graphics/gnome-icon-theme" {
 		f := "../../graphics/gnome-icon-theme/buildlink3.mk"
-		if !pkg.included.Seen(f) {
+		if !pkg.included.Seen(NewPackagePathString(f)) {
 			pline.Errorf("The package Makefile must include %q.", f)
 			pline.Explain(
 				"Packages that install GNOME icons must maintain the icon theme",