pkgtools/pkglint: update to 21.1.1

Changes since 21.1.0:

Added check for packages that have been migrated to Meson but still
refer to their previous build system, most often involving GNU make and
GNU autotools.  Suggested by nia.

6 files changed, 146 insertions, 13 deletions

diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile
index 45cdb747f35..38279caaa3c 100644
--- a/pkgtools/pkglint/Makefile
+++ b/pkgtools/pkglint/Makefile
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.682 2021/03/25 21:55:05 rillig Exp $
+# $NetBSD: Makefile,v 1.683 2021/04/17 18:10:14 rillig Exp $
 
-PKGNAME=	pkglint-21.1.0
+PKGNAME=	pkglint-21.1.1
 CATEGORIES=	pkgtools
 DISTNAME=	tools
 MASTER_SITES=	${MASTER_SITE_GITHUB:=golang/}
diff --git a/pkgtools/pkglint/files/mkline.go b/pkgtools/pkglint/files/mkline.go
index b1e159b1bbf..b9cdd7f0ef4 100644
--- a/pkgtools/pkglint/files/mkline.go
+++ b/pkgtools/pkglint/files/mkline.go
@@ -935,12 +935,13 @@ func (mkline *MkLine) checkFileGlobbing(ch int, str string) {
 		return
 	}
 
-	if !mkline.once.FirstTimeSlice("unintended file globbing", string(ch)) {
+	chStr := string(rune(ch))
+	if !mkline.once.FirstTimeSlice("unintended file globbing", chStr) {
 		return
 	}
 
 	mkline.Warnf("The %q in the word %q may lead to unintended file globbing.",
-		string(ch), str)
+		chStr, str)
 	mkline.Explain(
 		"To fix this, enclose the word in \"double\" or 'single' quotes.")
 }
diff --git a/pkgtools/pkglint/files/package.go b/pkgtools/pkglint/files/package.go
index 6a0d3a864eb..f1b53c3fb49 100644
--- a/pkgtools/pkglint/files/package.go
+++ b/pkgtools/pkglint/files/package.go
@@ -735,6 +735,8 @@ func (pkg *Package) checkfilePackageMakefile(filename CurrPath, mklines *MkLines
 
 	pkg.CheckVarorder(mklines)
 
+	pkg.checkMeson(mklines)
+
 	SaveAutofixChanges(mklines.lines)
 }
 
@@ -1178,6 +1180,65 @@ func (pkg *Package) checkUseLanguagesCompilerMk(mklines *MkLines) {
 	})
 }
 
+// checkMeson checks for typical leftover snippets from packages that used
+// GNU autotools or another build system, before being migrated to Meson.
+func (pkg *Package) checkMeson(mklines *MkLines) {
+
+	if pkg.Includes("../../devel/meson/build.mk") == nil {
+		return
+	}
+
+	pkg.checkMesonGnuMake(mklines)
+	pkg.checkMesonConfigureArgs()
+	pkg.checkMesonPython(mklines)
+}
+
+func (pkg *Package) checkMesonGnuMake(mklines *MkLines) {
+	gmake := mklines.Tools.ByName("gmake")
+	if gmake != nil && gmake.UsableAtRunTime() {
+		mkline := NewLineWhole(pkg.File("."))
+		mkline.Warnf("Meson packages usually don't need GNU make.")
+		mkline.Explain(
+			"After migrating a package from GNU make to Meson,",
+			"GNU make is typically not needed anymore.")
+	}
+}
+
+func (pkg *Package) checkMesonConfigureArgs() {
+	if mkline := pkg.vars.FirstDefinition("CONFIGURE_ARGS"); mkline != nil {
+		mkline.Warnf("Meson packages usually don't need CONFIGURE_ARGS.")
+		mkline.Explain(
+			"After migrating a package from GNU make to Meson,",
+			"CONFIGURE_ARGS are typically not needed anymore.")
+	}
+}
+
+func (pkg *Package) checkMesonPython(mklines *MkLines) {
+
+	if mklines.allVars.IsDefined("PYTHON_FOR_BUILD_ONLY") {
+		return
+	}
+
+	for path := range pkg.unconditionalIncludes {
+		if path.ContainsPath("lang/python") {
+			goto warn
+		}
+	}
+	return
+
+warn:
+	mkline := NewLineWhole(pkg.File("."))
+	mkline.Warnf("Meson packages usually need Python only at build time.")
+	mkline.Explain(
+		"The Meson build system is implemented in Python,",
+		"therefore packages that use Meson need Python",
+		"as a build-time dependency.",
+		"After building the package, it is typically independent from Python.",
+		"",
+		"To change the Python dependency to build-time,",
+		"set PYTHON_FOR_BUILD_ONLY=yes in the package Makefile.")
+}
+
 func (pkg *Package) determineEffectivePkgVars() {
 	distnameLine := pkg.vars.FirstDefinition("DISTNAME")
 	pkgnameLine := pkg.vars.FirstDefinition("PKGNAME")
diff --git a/pkgtools/pkglint/files/package_test.go b/pkgtools/pkglint/files/package_test.go
index ae4609a5cdb..190d0074103 100644
--- a/pkgtools/pkglint/files/package_test.go
+++ b/pkgtools/pkglint/files/package_test.go
@@ -2618,6 +2618,77 @@ func (s *Suite) Test_Package_checkUseLanguagesCompilerMk__endian_mk(c *check.C)
 			"Modifying USE_LANGUAGES after including ../../mk/compiler.mk has no effect.")
 }
 
+func (s *Suite) Test_Package_checkMesonGnuMake(c *check.C) {
+	t := s.Init(c)
+
+	t.CreateFileLines("devel/meson/build.mk")
+	t.SetUpTool("gmake", "", AtRunTime)
+	t.SetUpPackage("category/package",
+		"USE_TOOLS+=\tgmake",
+		"",
+		".include \"../../devel/meson/build.mk\"")
+	t.Chdir("category/package")
+	t.FinishSetUp()
+
+	G.Check(".")
+
+	// XXX: Giving the line number would be nice.
+	t.CheckOutputLines(
+		"WARN: Meson packages usually don't need GNU make.")
+}
+
+func (s *Suite) Test_Package_checkMesonConfigureArgs(c *check.C) {
+	t := s.Init(c)
+
+	t.CreateFileLines("devel/meson/build.mk")
+	t.SetUpPackage("category/package",
+		"CONFIGURE_ARGS+=\t--enable-feature",
+		"",
+		".include \"../../devel/meson/build.mk\"")
+	t.Chdir("category/package")
+	t.FinishSetUp()
+
+	G.Check(".")
+
+	t.CheckOutputLines(
+		"WARN: Makefile:20: Meson packages usually don't need CONFIGURE_ARGS.")
+}
+
+func (s *Suite) Test_Package_checkMesonPython(c *check.C) {
+	t := s.Init(c)
+
+	t.CreateFileLines("devel/meson/build.mk")
+	t.CreateFileLines("lang/python/application.mk")
+	t.SetUpPackage("category/package",
+		"PYTHON_FOR_BUILD_ONLY=\ttool",
+		"",
+		".include \"../../lang/python/application.mk\"",
+		".include \"../../devel/meson/build.mk\"")
+	t.Chdir("category/package")
+	t.FinishSetUp()
+
+	G.Check(".")
+
+	t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_Package_checkMesonPython__missing_PYTHON_FOR_BUILD_ONLY(c *check.C) {
+	t := s.Init(c)
+
+	t.CreateFileLines("devel/meson/build.mk")
+	t.CreateFileLines("lang/python/application.mk")
+	t.SetUpPackage("category/package",
+		".include \"../../lang/python/application.mk\"",
+		".include \"../../devel/meson/build.mk\"")
+	t.Chdir("category/package")
+	t.FinishSetUp()
+
+	G.Check(".")
+
+	t.CheckOutputLines(
+		"WARN: Meson packages usually need Python only at build time.")
+}
+
 // PKGNAME is stronger than DISTNAME.
 func (s *Suite) Test_Package_determineEffectivePkgVars__precedence(c *check.C) {
 	t := s.Init(c)
diff --git a/pkgtools/pkglint/files/patches.go b/pkgtools/pkglint/files/patches.go
index 7abe6a5d742..330308be139 100644
--- a/pkgtools/pkglint/files/patches.go
+++ b/pkgtools/pkglint/files/patches.go
@@ -15,11 +15,7 @@ type PatchChecker struct {
 	previousLineEmpty bool
 }
 
-const (
-	rePatchUniFileDel = `^---[\t ]([^\t ]+)(?:[\t ]+(.*))?$`
-	rePatchUniFileAdd = `^\+\+\+[\t ]([^\t ]+)(?:[\t ]+(.*))?$`
-	rePatchUniHunk    = `^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@(.*)$`
-)
+const rePatchUniFileDel = `^---[\t ]([^\t ]+)(?:[\t ]+(.*))?$`
 
 func (ck *PatchChecker) Check(pkg *Package) {
 	if ck.lines.CheckCvsID(0, ``, "") {
@@ -35,6 +31,9 @@ func (ck *PatchChecker) Check(pkg *Package) {
 	var patchedFiles []Path
 	for !ck.llex.EOF() {
 		line := ck.llex.CurrentLine()
+
+		const rePatchUniFileAdd = `^\+\+\+[\t ]([^\t ]+)(?:[\t ]+(.*))?$`
+
 		if ck.llex.SkipRegexp(rePatchUniFileDel) {
 			if m := ck.llex.NextRegexp(rePatchUniFileAdd); m != nil {
 				patchedFile := NewPath(m[1])
@@ -104,7 +103,7 @@ func (ck *PatchChecker) checkUnifiedDiff(patchedFile Path) {
 	linesDiff := 0
 	hasHunks := false
 	for {
-		m := ck.llex.NextRegexp(rePatchUniHunk)
+		m := ck.llex.NextRegexp(`^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@(.*)$`)
 		if m == nil {
 			break
 		}
@@ -396,7 +395,7 @@ func (ck *PatchChecker) checktextCvsID(text string) {
 		return
 	}
 	if m, tagname := match1(text, `\$(Author|Date|Header|Id|Locker|Log|Name|RCSfile|Revision|Source|State|NetBSD)(?::[^\$]*)?\$`); m {
-		if matches(text, rePatchUniHunk) {
+		if matches(text, `^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@(.*)$`) {
 			ck.llex.PreviousLine().Warnf("Found CVS tag \"$%s$\". Please remove it.", tagname)
 		} else {
 			ck.llex.PreviousLine().Warnf("Found CVS tag \"$%s$\". Please remove it by reducing the number of context lines using pkgdiff or \"diff -U[210]\".", tagname)
diff --git a/pkgtools/pkglint/files/path.go b/pkgtools/pkglint/files/path.go
index 4ec9993e03a..e22695a2e01 100644
--- a/pkgtools/pkglint/files/path.go
+++ b/pkgtools/pkglint/files/path.go
@@ -25,7 +25,7 @@ func (p Path) GoString() string { return sprintf("%q", string(p)) }
 // which is usually a sign of an uninitialized variable.
 func (p Path) IsEmpty() bool { return p == "" }
 
-// Returns the directory of the path, with only minimal cleaning.
+// Dir returns the directory of the path, with only minimal cleaning.
 // Only redundant dots and slashes are removed, and only at the end.
 func (p Path) Dir() Path {
 	s := p.String()
@@ -188,7 +188,8 @@ func (p Path) CleanDot() Path {
 	return NewPath(strings.Join(parts, "/"))
 }
 
-// Differs from path.Clean in that only "../../" is replaced, not "../".
+// CleanPath is similar to path.Clean.
+// The difference is that only "../../" is replaced, not "../".
 // Also, the initial directory is always kept.
 // This is to provide the package path as context in deeply nested .include chains.
 func (p Path) CleanPath() Path {