pkgtools/pkglint: update to 21.2.4

Changes since 21.2.3:

Fixed loading of the tool definitions.  This adds 76 warnings for
packages that use tools without adding them to USE_TOOLS.  It also fixes
the warning about gmake and Meson.

11 files changed, 63 insertions, 30 deletions

diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile
index 38277f360dc..29cc0e9c4fc 100644
--- a/pkgtools/pkglint/Makefile
+++ b/pkgtools/pkglint/Makefile
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.694 2021/08/12 05:29:41 rillig Exp $
+# $NetBSD: Makefile,v 1.695 2021/08/14 08:19:49 rillig Exp $
 
-PKGNAME=	pkglint-21.2.3
+PKGNAME=	pkglint-21.2.4
 CATEGORIES=	pkgtools
 DISTNAME=	tools
 MASTER_SITES=	${MASTER_SITE_GITHUB:=golang/}
diff --git a/pkgtools/pkglint/files/mkline_test.go b/pkgtools/pkglint/files/mkline_test.go
index 2e77fae2dc0..34aaf110d24 100644
--- a/pkgtools/pkglint/files/mkline_test.go
+++ b/pkgtools/pkglint/files/mkline_test.go
@@ -703,7 +703,7 @@ func (s *Suite) Test_MkLine_VariableNeedsQuoting__command_in_command(c *check.C)
 		MkCvsID,
 		"GENERATE_PLIST= cd ${DESTDIR}${PREFIX}; ${FIND} * \\( -type f -or -type l \\) | ${SORT};")
 
-	mklines.collectVariables()
+	mklines.collectVariables(false, true)
 	MkLineChecker{mklines, mklines.mklines[1]}.Check()
 
 	t.CheckOutputLines(
diff --git a/pkgtools/pkglint/files/mklines.go b/pkgtools/pkglint/files/mklines.go
index a98cc90b46d..977471c217e 100644
--- a/pkgtools/pkglint/files/mklines.go
+++ b/pkgtools/pkglint/files/mklines.go
@@ -116,7 +116,7 @@ func (mklines *MkLines) Check() {
 	// are collected to make the order of the definitions irrelevant.
 	mklines.collectRationale()
 	mklines.collectUsedVariables()
-	mklines.collectVariables()
+	mklines.collectVariables(false, true)
 	mklines.collectPlistVars()
 	if mklines.pkg != nil {
 		mklines.pkg.collectConditionalIncludes(mklines)
@@ -249,12 +249,14 @@ func (mklines *MkLines) collectDocumentedVariables() {
 	finish()
 }
 
-func (mklines *MkLines) collectVariables() {
-	mklines.ForEach(mklines.collectVariable)
+func (mklines *MkLines) collectVariables(infrastructure bool, addToUseTools bool) {
+	mklines.ForEach(func(mkline *MkLine) {
+		mklines.collectVariable(mkline, infrastructure, addToUseTools)
+	})
 }
 
-func (mklines *MkLines) collectVariable(mkline *MkLine) {
-	mklines.Tools.ParseToolLine(mklines, mkline, false, true)
+func (mklines *MkLines) collectVariable(mkline *MkLine, infrastructure bool, addToUseTools bool) {
+	mklines.Tools.ParseToolLine(mklines, mkline, infrastructure, addToUseTools)
 
 	if !mkline.IsVarassignMaybeCommented() {
 		return
diff --git a/pkgtools/pkglint/files/mklines_test.go b/pkgtools/pkglint/files/mklines_test.go
index f3325660605..abc800624af 100644
--- a/pkgtools/pkglint/files/mklines_test.go
+++ b/pkgtools/pkglint/files/mklines_test.go
@@ -681,7 +681,7 @@ func (s *Suite) Test_MkLines_collectVariables__no_tracing(c *check.C) {
 		"SUBST_VARS.id+=\tVAR3")
 	t.DisableTracing()
 
-	mklines.collectVariables()
+	mklines.collectVariables(false, false)
 
 	t.CheckOutputEmpty()
 }
@@ -807,9 +807,9 @@ func (s *Suite) Test_MkLines_checkAll__collect_else(c *check.C) {
 		".elif 0",
 		".endif")
 
-	// As a side-effect of MkLines.ForEach,
+	// As a side effect of MkLines.ForEach,
 	// the HasElseBranch in the lines is updated.
-	mklines.collectVariables()
+	mklines.collectVariables(false, false)
 
 	t.CheckEquals(mklines.mklines[2].HasElseBranch(), false)
 	t.CheckEquals(mklines.mklines[5].HasElseBranch(), true)
diff --git a/pkgtools/pkglint/files/package.go b/pkgtools/pkglint/files/package.go
index db78ce44195..3e34508f0d8 100644
--- a/pkgtools/pkglint/files/package.go
+++ b/pkgtools/pkglint/files/package.go
@@ -710,8 +710,8 @@ func (pkg *Package) checkfilePackageMakefile(filename CurrPath, mklines *MkLines
 	//  though.
 	// pkg.collectConditionalIncludes(allLines)
 
-	allLines.collectVariables()    // To get the tool definitions
-	mklines.Tools = allLines.Tools // TODO: also copy the other collected data
+	allLines.collectVariables(false, true) // To get the tool definitions
+	mklines.Tools = allLines.Tools         // TODO: also copy the other collected data
 
 	// TODO: Checking only mklines instead of allLines ignores the
 	//  .include lines. For example, including "options.mk" does not
@@ -1194,7 +1194,7 @@ func (pkg *Package) checkMeson(mklines *MkLines) {
 
 func (pkg *Package) checkMesonGnuMake(mklines *MkLines) {
 	gmake := mklines.Tools.ByName("gmake")
-	if G.Experimental && gmake != nil && gmake.UsableAtRunTime() {
+	if gmake != nil && gmake.UsableAtRunTime() {
 		mkline := NewLineWhole(pkg.File("."))
 		mkline.Warnf("Meson packages usually don't need GNU make.")
 		mkline.Explain(
diff --git a/pkgtools/pkglint/files/package_test.go b/pkgtools/pkglint/files/package_test.go
index d7acf064cec..9a34cf7e319 100644
--- a/pkgtools/pkglint/files/package_test.go
+++ b/pkgtools/pkglint/files/package_test.go
@@ -2619,14 +2619,6 @@ func (s *Suite) Test_Package_checkUseLanguagesCompilerMk__endian_mk(c *check.C)
 }
 
 func (s *Suite) Test_Package_checkMesonGnuMake(c *check.C) {
-
-	// False positive in x11/libxkbcommon, 2021-08-12.
-	//
-	// It seems that the Tools registry is not initialized properly since
-	// x11/libxkbcommon does not mention gmake at all, and 'bmake show-all'
-	// also does not contain 'gmake'.
-	G.Experimental = true
-
 	t := s.Init(c)
 
 	t.CreateFileLines("devel/meson/build.mk")
diff --git a/pkgtools/pkglint/files/pkglint_test.go b/pkgtools/pkglint/files/pkglint_test.go
index 4983af4b02e..52d373c6763 100644
--- a/pkgtools/pkglint/files/pkglint_test.go
+++ b/pkgtools/pkglint/files/pkglint_test.go
@@ -719,7 +719,7 @@ func (s *Suite) Test_resolveVariableRefs__scope_precedence(c *check.C) {
 	mklines := t.NewMkLines("filename.mk",
 		MkCvsID,
 		"ORIGIN=\tfilename.mk")
-	mklines.collectVariables()
+	mklines.collectVariables(false, false)
 	pkg := NewPackage(t.File("category/package"))
 	pkg.vars.Define("ORIGIN", t.NewMkLine("other.mk", 123, "ORIGIN=\tpackage"))
 
@@ -750,7 +750,7 @@ func (s *Suite) Test_resolveVariableRefs__indeterminate(c *check.C) {
 	pkg.vars.Define("PKGVAR", t.NewMkLine("filename.mk", 123, "PKGVAR!=\tcommand"))
 	mklines := t.NewMkLinesPkg("filename.mk", pkg,
 		"VAR!=\tcommand")
-	mklines.collectVariables()
+	mklines.collectVariables(false, false)
 
 	resolved := resolveVariableRefs("${VAR} ${PKGVAR}", mklines, nil)
 
diff --git a/pkgtools/pkglint/files/pkgsrc.go b/pkgtools/pkglint/files/pkgsrc.go
index 959be58542f..20787615288 100644
--- a/pkgtools/pkglint/files/pkgsrc.go
+++ b/pkgtools/pkglint/files/pkgsrc.go
@@ -842,7 +842,7 @@ func (src *Pkgsrc) loadUntypedVars() {
 
 	handleMkFile := func(path CurrPath) {
 		mklines := LoadMk(path, nil, MustSucceed)
-		mklines.collectVariables()
+		mklines.collectVariables(false, true) // FIXME
 		mklines.collectUsedVariables()
 		mklines.allVars.forEach(func(varname string, data *scopeVar) {
 			if data.firstDef != nil {
diff --git a/pkgtools/pkglint/files/pkgsrc_test.go b/pkgtools/pkglint/files/pkgsrc_test.go
index 7e9c4f8f2bc..c2aa2dd7d82 100644
--- a/pkgtools/pkglint/files/pkgsrc_test.go
+++ b/pkgtools/pkglint/files/pkgsrc_test.go
@@ -934,7 +934,7 @@ func (s *Suite) Test_Pkgsrc_loadUntypedVars__badly_named_directory(c *check.C) {
 	t.CheckOutputEmpty()
 }
 
-func (s *Suite) Test_Pkgsrc_loadUntypedVars__loop_variable(c *check.C) {
+func (s *Suite) Test_Pkgsrc_loadUntypedVars__local_varnames(c *check.C) {
 	t := s.Init(c)
 
 	t.CreateFileLines("mk/check/check-files.mk",
@@ -969,6 +969,41 @@ func (s *Suite) Test_Pkgsrc_loadUntypedVars__loop_variable(c *check.C) {
 			"Invalid part \"/\" after variable name \"\".")
 }
 
+// An optional tool is not available by default.
+// A package can choose to include a tool by adding it to USE_TOOLS.
+func (s *Suite) Test_Pkgsrc_loadUntypedVars__tools(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpPackage("category/package",
+		"do-configure:",
+		"\tlinux-tool",
+		"\toptional-tool")
+	t.CreateFileLines("mk/tools/default.mk",
+		MkCvsID,
+		"",
+		// This tool is available by default.
+		".if ${OPSYS} == Linux",
+		"TOOLS_CREATE+=\tlinux-tool",
+		".endif",
+		"",
+		// This tool is only available if it is listed in USE_TOOLS.
+		".if ${_USE_TOOLS:Moptional-tool}",
+		"TOOLS_CREATE+=\toptional-tool",
+		".endif")
+	t.FinishSetUp()
+
+	G.Check(t.File("category/package"))
+
+	// FIXME: The command names must be recognized.
+	// TODO: linux-tool must be available to the package.
+	// TODO: optional-tool must not be available to the package.
+	t.CheckOutputLines(
+		"WARN: ~/category/package/Makefile:21: "+
+			"Unknown shell command \"linux-tool\".",
+		"WARN: ~/category/package/Makefile:22: "+
+			"Unknown shell command \"optional-tool\".")
+}
+
 func (s *Suite) Test_Pkgsrc_Latest__multiple_candidates(c *check.C) {
 	t := s.Init(c)
 
diff --git a/pkgtools/pkglint/files/tools.go b/pkgtools/pkglint/files/tools.go
index 1c9b97eabac..20834aa2940 100644
--- a/pkgtools/pkglint/files/tools.go
+++ b/pkgtools/pkglint/files/tools.go
@@ -245,7 +245,11 @@ func (tr *Tools) ParseToolLine(mklines *MkLines, mkline *MkLine, fromInfrastruct
 		case "TOOLS_CREATE":
 			for _, name := range mkline.ValueFields(value) {
 				if tr.IsValidToolName(name) {
-					tr.def(name, "", false, AtRunTime, nil)
+					validity := AtRunTime
+					if mklines.indentation.DependsOn("_USE_TOOLS") {
+						validity = Nowhere // see mk/tools/replace.mk
+					}
+					tr.def(name, "", false, validity, nil)
 				}
 			}
 
diff --git a/pkgtools/pkglint/files/tools_test.go b/pkgtools/pkglint/files/tools_test.go
index 6c454462596..25a324acabe 100644
--- a/pkgtools/pkglint/files/tools_test.go
+++ b/pkgtools/pkglint/files/tools_test.go
@@ -44,7 +44,7 @@ func (s *Suite) Test_Tool_UsableAtLoadTime__pkgconfig_builtin_mk(c *check.C) {
 			"To use the tool ${TOOL1} at load time, " +
 			"bsd.prefs.mk has to be included before.")
 	// Maybe an explanation might help here.
-	// There is surprisingly few feedback on any of the explanations
+	// There is surprisingly little feedback on any of the explanations
 	// though (about 0 in 10 years), therefore I don't even know
 	// whether anyone reads them.
 }
@@ -447,7 +447,7 @@ func (s *Suite) Test_Tools__aliases_in_for_loop(c *check.C) {
 		"TOOLS_ALIASES.ggrep+=\t${t}",
 		".endfor")
 
-	mklines.collectVariables() // calls ParseToolLine internally
+	mklines.collectVariables(false, true) // calls ParseToolLine internally
 
 	t.CheckDeepEquals(
 		mklines.Tools.ByName("ggrep").Aliases,
@@ -592,7 +592,7 @@ func (s *Suite) Test_Tools_ParseToolLine__invalid_tool_name(c *check.C) {
 		"_TOOLS.${t}=\t${t}",
 		".endfor")
 
-	mklines.collectVariables()
+	mklines.collectVariables(false, true)
 	t.Check(mklines.Tools.byName, check.HasLen, 1)
 	t.CheckEquals(mklines.Tools.ByName("tool").String(), "tool:::Nowhere:abc")