pkgtools/pkglint: update to 5.7.22

Changes since 5.7.21:

* The files from wip/mk do not belong to the main pkgsrc infrastructure.
  Therefore, when loading the package Makefile to look for defined but
  unused variables, parsing doesn't stop in these files.

9 files changed, 314 insertions, 65 deletions

diff --git a/pkgtools/pkglint/files/check_test.go b/pkgtools/pkglint/files/check_test.go
index 29f32c6485a..eec22715e47 100644
--- a/pkgtools/pkglint/files/check_test.go
+++ b/pkgtools/pkglint/files/check_test.go
@@ -233,8 +233,7 @@ func (t *Tester) LoadMkInclude(relativeFileName string) *MkLines {
 			lines = append(lines, mkline.Line)
 
 			if mkline.IsInclude() {
-				included := cleanpath(path.Dir(filename) + "/" + mkline.IncludedFile())
-				load(included)
+				load(mkline.IncludedFileFull())
 			}
 		}
 	}
@@ -681,6 +680,7 @@ func (t *Tester) FinishSetUp() {
 }
 
 // Main runs the pkglint main program with the given command line arguments.
+// Other than in the other tests, the -Wall option is not added implicitly.
 //
 // Arguments that name existing files or directories in the temporary test
 // directory are transformed to their actual paths.
diff --git a/pkgtools/pkglint/files/mkline.go b/pkgtools/pkglint/files/mkline.go
index 4a371e691af..1876f3c1da7 100644
--- a/pkgtools/pkglint/files/mkline.go
+++ b/pkgtools/pkglint/files/mkline.go
@@ -421,12 +421,17 @@ func (mkline *MkLine) MustExist() bool { return mkline.data.(*mkLineInclude).mus
 
 func (mkline *MkLine) IncludedFile() string { return mkline.data.(*mkLineInclude).includedFile }
 
+func (mkline *MkLine) IncludedFileFull() string {
+	return cleanpath(path.Join(path.Dir(mkline.Filename), mkline.IncludedFile()))
+}
+
 func (mkline *MkLine) Targets() string { return mkline.data.(mkLineDependency).targets }
 
 func (mkline *MkLine) Sources() string { return mkline.data.(mkLineDependency).sources }
 
-// ConditionalVars applies to .include lines and is a space-separated
-// list of those variable names on which the inclusion depends.
+// ConditionalVars applies to .include lines and contains the
+// variable names on which the inclusion depends.
+//
 // It is initialized later, step by step, when parsing other lines.
 func (mkline *MkLine) ConditionalVars() []string {
 	return mkline.data.(*mkLineInclude).conditionalVars
@@ -1367,16 +1372,16 @@ func (ind *Indentation) IsConditional() bool {
 //
 // Variables named *_MK are excluded since they are usually not interesting.
 func (ind *Indentation) Varnames() []string {
-	var varnames []string
+	varnames := NewStringSet()
 	for _, level := range ind.levels {
 		for _, levelVarname := range level.conditionalVars {
 			// multiple-inclusion guard must be filtered out earlier.
 			assert(!hasSuffix(levelVarname, "_MK"))
 
-			varnames = append(varnames, levelVarname)
+			varnames.Add(levelVarname)
 		}
 	}
-	return varnames
+	return varnames.Elements
 }
 
 // Args returns the arguments of the innermost .if, .elif or .for.
diff --git a/pkgtools/pkglint/files/mkline_test.go b/pkgtools/pkglint/files/mkline_test.go
index 82fb1a97a02..81d9b1d3e06 100644
--- a/pkgtools/pkglint/files/mkline_test.go
+++ b/pkgtools/pkglint/files/mkline_test.go
@@ -1931,12 +1931,11 @@ func (s *Suite) Test_Indentation_Varnames__repetition(c *check.C) {
 
 	G.Check(t.File("category/package"))
 
-	// TODO: It feels wrong that OPSYS is mentioned twice here.
-	//  Why only twice and not three times?
 	t.CheckOutputLines(
-		"WARN: ~/category/package/buildlink3.mk:14: " +
-			"\"../../category/other/buildlink3.mk\" is included conditionally here " +
-			"(depending on OPSYS, OPSYS) and unconditionally in Makefile:20.")
+		"WARN: ~/category/package/Makefile:20: " +
+			"\"../../category/other/buildlink3.mk\" is included " +
+			"unconditionally here and " +
+			"conditionally in buildlink3.mk:14 (depending on OPSYS).")
 }
 
 func (s *Suite) Test_MkLine_ForEachUsed(c *check.C) {
diff --git a/pkgtools/pkglint/files/mklines.go b/pkgtools/pkglint/files/mklines.go
index e9517217327..9fedaf309c3 100644
--- a/pkgtools/pkglint/files/mklines.go
+++ b/pkgtools/pkglint/files/mklines.go
@@ -90,6 +90,9 @@ func (mklines *MkLines) Check() {
 	mklines.collectVariables()
 	mklines.collectPlistVars()
 	mklines.collectElse()
+	if G.Pkg != nil {
+		G.Pkg.collectConditionalIncludes(mklines)
+	}
 
 	// In the second pass, the actual checks are done.
 	mklines.checkAll()
@@ -237,7 +240,7 @@ func (mklines *MkLines) ForEach(action func(mkline *MkLine)) {
 // ForEachEnd calls the action for each line, until the action returns false.
 // It keeps track of the indentation and all conditional variables.
 // At the end, atEnd is called with the last line as its argument.
-func (mklines *MkLines) ForEachEnd(action func(mkline *MkLine) bool, atEnd func(lastMkline *MkLine)) {
+func (mklines *MkLines) ForEachEnd(action func(mkline *MkLine) bool, atEnd func(lastMkline *MkLine)) bool {
 
 	// XXX: To avoid looping over the lines multiple times, it would
 	// be nice to have an interface LinesChecker that checks a single topic.
@@ -247,9 +250,11 @@ func (mklines *MkLines) ForEachEnd(action func(mkline *MkLine) bool, atEnd func(
 	mklines.indentation = NewIndentation()
 	mklines.Tools.SeenPrefs = false
 
+	result := true
 	for _, mkline := range mklines.mklines {
 		mklines.indentation.TrackBefore(mkline)
 		if !action(mkline) {
+			result = false
 			break
 		}
 		mklines.indentation.TrackAfter(mkline)
@@ -259,6 +264,8 @@ func (mklines *MkLines) ForEachEnd(action func(mkline *MkLine) bool, atEnd func(
 		atEnd(mklines.mklines[len(mklines.mklines)-1])
 	}
 	mklines.indentation = nil
+
+	return result
 }
 
 // ExpandLoopVar searches the surrounding .for loops for the given
diff --git a/pkgtools/pkglint/files/package.go b/pkgtools/pkglint/files/package.go
index 7f2449fb327..9270846ed81 100644
--- a/pkgtools/pkglint/files/package.go
+++ b/pkgtools/pkglint/files/package.go
@@ -23,7 +23,7 @@ type Package struct {
 	Filesdir             string       // FILESDIR from the package Makefile
 	Patchdir             string       // PATCHDIR from the package Makefile
 	DistinfoFile         string       // DISTINFO_FILE from the package Makefile
-	EffectivePkgname     string       // PKGNAME or DISTNAME from the package Makefile, including nb13
+	EffectivePkgname     string       // PKGNAME or DISTNAME from the package Makefile, including nb13, can be empty
 	EffectivePkgbase     string       // EffectivePkgname without the version
 	EffectivePkgversion  string       // The version part of the effective PKGNAME, excluding nb13
 	EffectivePkgnameLine *MkLine      // The origin of the three Effective* values
@@ -56,6 +56,8 @@ type Package struct {
 	unconditionalIncludes map[string]*MkLine
 
 	IgnoreMissingPatches bool // In distinfo, don't warn about patches that cannot be found.
+
+	Once Once
 }
 
 func NewPackage(dir string) *Package {
@@ -112,6 +114,13 @@ func (pkg *Package) Rel(filename string) string {
 	return relpath(pkg.dir, filename)
 }
 
+// Returns whether the given file (relative to the package directory)
+// is included somewhere in the package, either directly or indirectly.
+func (pkg *Package) Includes(filename string) bool {
+	return pkg.unconditionalIncludes[filename] != nil ||
+		pkg.conditionalIncludes[filename] != nil
+}
+
 func (pkg *Package) checkPossibleDowngrade() {
 	if trace.Tracing {
 		defer trace.Call0()()
@@ -160,9 +169,13 @@ func (pkg *Package) checkPossibleDowngrade() {
 	}
 }
 
-// checkLinesBuildlink3Inclusion checks whether the package Makefile and
-// the corresponding buildlink3.mk agree for all included buildlink3.mk
-// files whether they are included conditionally or unconditionally.
+// checkLinesBuildlink3Inclusion checks whether the package Makefile includes
+// at least those buildlink3.mk files that are included by the buildlink3.mk
+// file of the package.
+//
+// The other direction is not checked since it is perfectly fine for a package
+// to have more dependencies than are needed for buildlink the package.
+// (This might be worth re-checking though.)
 func (pkg *Package) checkLinesBuildlink3Inclusion(mklines *MkLines) {
 	if trace.Tracing {
 		defer trace.Call0()()
@@ -173,7 +186,7 @@ func (pkg *Package) checkLinesBuildlink3Inclusion(mklines *MkLines) {
 	for _, mkline := range mklines.mklines {
 		if mkline.IsInclude() {
 			includedFile := mkline.IncludedFile()
-			if matches(includedFile, `^\.\./\.\./.*/buildlink3\.mk`) {
+			if hasSuffix(includedFile, "/buildlink3.mk") {
 				includedFiles[includedFile] = mkline
 				if pkg.bl3[includedFile] == nil {
 					mkline.Warnf("%s is included by this file but not by the package.", includedFile)
@@ -210,6 +223,7 @@ func (pkg *Package) load() ([]string, *MkLines, *MkLines) {
 
 	// Determine the used variables and PLIST directories before checking any of the Makefile fragments.
 	// TODO: Why is this code necessary? What effect does it have?
+	pkg.collectConditionalIncludes(mklines)
 	for _, filename := range files {
 		basename := path.Base(filename)
 		if (hasPrefix(basename, "Makefile.") || hasSuffix(filename, ".mk")) &&
@@ -218,6 +232,7 @@ func (pkg *Package) load() ([]string, *MkLines, *MkLines) {
 			!contains(filename, pkg.Filesdir+"/") {
 			fragmentMklines := LoadMk(filename, MustSucceed)
 			fragmentMklines.collectUsedVariables()
+			pkg.collectConditionalIncludes(fragmentMklines)
 		}
 		if hasPrefix(basename, "PLIST") {
 			pkg.loadPlistDirs(filename)
@@ -382,21 +397,30 @@ func (pkg *Package) loadPackageMakefile() (*MkLines, *MkLines) {
 	return mainLines, allLines
 }
 
+func (pkg *Package) collectConditionalIncludes(mklines *MkLines) {
+	mklines.ForEach(func(mkline *MkLine) {
+		if mkline.IsInclude() {
+			mkline.SetConditionalVars(mklines.indentation.Varnames())
+
+			key := pkg.Rel(mkline.IncludedFileFull())
+			if mklines.indentation.IsConditional() {
+				pkg.conditionalIncludes[key] = mkline
+			} else {
+				pkg.unconditionalIncludes[key] = mkline
+			}
+		}
+	})
+}
+
 // TODO: What is allLines used for, is it still necessary? Would it be better as a field in Package?
 func (pkg *Package) parse(mklines *MkLines, allLines *MkLines, includingFileForUsedCheck string) bool {
 	if trace.Tracing {
 		defer trace.Call1(mklines.lines.Filename)()
 	}
 
-	result := true
-
-	lineAction := func(mkline *MkLine) bool {
-		result = pkg.parseLine(mklines, mkline, allLines)
-		return result
-	}
-
-	atEnd := func(mkline *MkLine) {}
-	mklines.ForEachEnd(lineAction, atEnd)
+	result := mklines.ForEachEnd(
+		func(mkline *MkLine) bool { return pkg.parseLine(mklines, mkline, allLines) },
+		func(mkline *MkLine) {})
 
 	if includingFileForUsedCheck != "" {
 		mklines.CheckUsedBy(G.Pkgsrc.ToRel(includingFileForUsedCheck))
@@ -540,15 +564,11 @@ func (*Package) diveInto(includingFile string, includedFile string) bool {
 		return false
 	}
 
-	if !contains(includingFile, "/mk/") {
-		return true
+	if contains(includingFile, "/mk/") && !hasPrefix(G.Pkgsrc.ToRel(includingFile), "wip/mk") {
+		return hasSuffix(includingFile, "buildlink3.mk") && hasSuffix(includedFile, "builtin.mk")
 	}
 
-	if hasSuffix(includingFile, "buildlink3.mk") && hasSuffix(includedFile, "builtin.mk") {
-		return true
-	}
-
-	return false
+	return true
 }
 
 func (pkg *Package) collectSeenInclude(mkline *MkLine, includedFile string) {
@@ -588,7 +608,7 @@ func (pkg *Package) resolveIncludedFile(mkline *MkLine, includingFilename string
 	}
 
 	if mkline.Basename != "buildlink3.mk" {
-		if matches(includedFile, `^\.\./\.\./(.*)/buildlink3\.mk$`) {
+		if hasSuffix(includedFile, "/buildlink3.mk") {
 			pkg.bl3[includedFile] = mkline
 			if trace.Tracing {
 				trace.Step1("Buildlink3 file in package: %q", includedFile)
@@ -666,10 +686,19 @@ func (pkg *Package) checkfilePackageMakefile(filename string, mklines *MkLines,
 
 	pkg.checkUpdate()
 
+	// TODO: Maybe later collect the conditional includes from allLines
+	//  instead of mklines. This will lead to about 6000 new warnings
+	//  though.
+	//  pkg.collectConditionalIncludes(allLines)
+
 	allLines.collectVariables()    // To get the tool definitions
 	mklines.Tools = allLines.Tools // TODO: also copy the other collected data
 	mklines.Check()
 
+	if false && pkg.EffectivePkgname != "" && pkg.Includes("../../lang/python/extension.mk") {
+		pkg.EffectivePkgnameLine.Warnf("The PKGNAME of Python extensions should start with ${PYPKGPREFIX}.")
+	}
+
 	pkg.CheckVarorder(mklines)
 
 	SaveAutofixChanges(mklines.lines)
@@ -1232,27 +1261,34 @@ func (pkg *Package) checkFreeze(filename string) {
 }
 
 func (pkg *Package) checkIncludeConditionally(mkline *MkLine, indentation *Indentation) {
-	mkline.SetConditionalVars(indentation.Varnames())
+	if IsPrefs(mkline.IncludedFile()) {
+		return
+	}
 
-	includedFile := mkline.IncludedFile()
-	key := pkg.Rel(mkline.IncludedFile())
+	key := pkg.Rel(mkline.IncludedFileFull())
 
 	if indentation.IsConditional() {
-		pkg.conditionalIncludes[key] = mkline
 		if other := pkg.unconditionalIncludes[key]; other != nil {
+			if !pkg.Once.FirstTimeSlice("checkIncludeConditionally", mkline.Location.String(), other.Location.String()) {
+				return
+			}
+
 			mkline.Warnf(
 				"%q is included conditionally here (depending on %s) "+
 					"and unconditionally in %s.",
-				cleanpath(includedFile), strings.Join(mkline.ConditionalVars(), ", "), mkline.RefTo(other))
+				cleanpath(mkline.IncludedFile()), strings.Join(mkline.ConditionalVars(), ", "), mkline.RefTo(other))
 		}
 
 	} else {
-		pkg.unconditionalIncludes[key] = mkline
 		if other := pkg.conditionalIncludes[key]; other != nil {
+			if !pkg.Once.FirstTimeSlice("checkIncludeConditionally", other.Location.String(), mkline.Location.String()) {
+				return
+			}
+
 			mkline.Warnf(
 				"%q is included unconditionally here "+
 					"and conditionally in %s (depending on %s).",
-				cleanpath(includedFile), mkline.RefTo(other), strings.Join(other.ConditionalVars(), ", "))
+				cleanpath(mkline.IncludedFile()), mkline.RefTo(other), strings.Join(other.ConditionalVars(), ", "))
 		}
 	}
 
@@ -1322,12 +1358,7 @@ func (pkg *Package) checkUseLanguagesCompilerMk(mklines *MkLines) {
 	}
 
 	handleInclude := func(mkline *MkLine) {
-		dirname, _ := path.Split(mkline.Filename)
-		dirname = cleanpath(dirname)
-		fullIncluded := dirname + "/" + mkline.IncludedFile()
-		relIncludedFile := relpath(pkg.dir, fullIncluded)
-
-		seen.FirstTime(relIncludedFile)
+		_ = seen.FirstTime(pkg.Rel(mkline.IncludedFileFull()))
 	}
 
 	mklines.ForEach(func(mkline *MkLine) {
diff --git a/pkgtools/pkglint/files/package_test.go b/pkgtools/pkglint/files/package_test.go
index d3b0a308e1e..38b9e25fd16 100644
--- a/pkgtools/pkglint/files/package_test.go
+++ b/pkgtools/pkglint/files/package_test.go
@@ -27,6 +27,24 @@ func (s *Suite) Test_Package_checkLinesBuildlink3Inclusion__file_but_not_package
 			"but not by the package.")
 }
 
+// Several files from the pkgsrc infrastructure are named *.buildlink3.mk,
+// even though they don't follow the typical file format for buildlink3.mk
+// files. Therefore they are ignored by this check.
+func (s *Suite) Test_Package_checkLinesBuildlink3Inclusion__infra_buildlink_file(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpPackage("category/package",
+		".include \"../../mk/motif.buildlink3.mk\"")
+	t.CreateFileDummyBuildlink3("category/package/buildlink3.mk",
+		".include \"../../mk/motif.buildlink3.mk\"")
+	t.CreateFileLines("mk/motif.buildlink3.mk",
+		MkCvsID)
+
+	t.Main("--quiet", "-Wall", "category/package")
+
+	t.CheckOutputEmpty()
+}
+
 func (s *Suite) Test_Package_checkLinesBuildlink3Inclusion__package_but_not_file(c *check.C) {
 	t := s.Init(c)
 
@@ -698,6 +716,75 @@ func (s *Suite) Test_Package_determineEffectivePkgVars__ineffective_C_modifier(c
 	t.CheckOutputEmpty()
 }
 
+func (s *Suite) Test_Package_determineEffectivePkgVars__Python_prefix(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpPackage("category/package",
+		"PKGNAME=\tpackage-2.0",
+		".include \"../../lang/python/extension.mk\"")
+	t.CreateFileLines("lang/python/extension.mk",
+		MkCvsID)
+
+	t.Main("-Wall", "category/package")
+
+	t.CheckOutputLines(
+		"Looks fine.")
+	// TODO: Wait for joerg's answer before enabling this check.
+	//t.CheckOutputLines(
+	//	"WARN: ~/category/package/Makefile:4: The PKGNAME of Python extensions should start with ${PYPKGPREFIX}.",
+	//	"1 warning found.")
+}
+
+func (s *Suite) Test_Package_determineEffectivePkgVars__Python_prefix_PKGNAME_variable(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpPackage("category/package",
+		"PKGNAME=\t${VAR}-package-2.0",
+		".include \"../../lang/python/extension.mk\"")
+	t.CreateFileLines("lang/python/extension.mk",
+		MkCvsID,
+		"VAR=\tvalue")
+
+	t.Main("-Wall", "category/package")
+
+	// Since PKGNAME starts with a variable, pkglint doesn't investigate
+	// further what the possible value of this variable could be. If it
+	// did, it would see that the prefix is not PYPKGPREFIX and would
+	// complain.
+	t.CheckOutputLines(
+		"Looks fine.")
+}
+
+// As of August 2019, pkglint loads the package files in alphabetical order.
+// This means that the package Makefile is loaded early, and includes by
+// other files may be invisible yet. This applies to both Makefile.* and to
+// *.mk since both of these appear later.
+//
+// The effects of these files are nevertheless visible at the right time
+// because the package Makefile is loaded including all its included files.
+func (s *Suite) Test_Package_determineEffectivePkgVars__Python_prefix_late(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpPackage("category/package",
+		"PKGNAME=\tpackage-2.0",
+		".include \"common.mk\"")
+	t.CreateFileLines("category/package/common.mk",
+		MkCvsID,
+		".include \"../../lang/python/extension.mk\"")
+	t.CreateFileLines("lang/python/extension.mk",
+		MkCvsID)
+
+	t.Main("-Wall", "category/package")
+
+	// TODO: Wait for joerg's answer before enabling this check.
+	t.CheckOutputLines(
+		"Looks fine.")
+	//t.CheckOutputLines(
+	//	"WARN: ~/category/package/Makefile:4: "+
+	//		"The PKGNAME of Python extensions should start with ${PYPKGPREFIX}.",
+	//	"1 warning found.")
+}
+
 func (s *Suite) Test_Package_checkPossibleDowngrade(c *check.C) {
 	t := s.Init(c)
 
@@ -1195,16 +1282,79 @@ func (s *Suite) Test_Package_checkIncludeConditionally__conditional_and_uncondit
 	G.checkdirPackage(".")
 
 	t.CheckOutputLines(
-		"WARN: options.mk:4: \"../../devel/zlib/buildlink3.mk\" is "+
-			"included conditionally here (depending on PKG_OPTIONS) "+
-			"and unconditionally in Makefile:20.",
-		"WARN: options.mk:6: \"../../sysutils/coreutils/buildlink3.mk\" is "+
-			"included unconditionally here "+
-			"and conditionally in Makefile:22 (depending on OPSYS).",
+		"WARN: Makefile:20: \"../../devel/zlib/buildlink3.mk\" is included "+
+			"unconditionally here "+
+			"and conditionally in options.mk:4 (depending on PKG_OPTIONS).",
+		"WARN: Makefile:22: \"../../sysutils/coreutils/buildlink3.mk\" is included "+
+			"conditionally here (depending on OPSYS) and "+
+			"unconditionally in options.mk:6.",
 		"WARN: options.mk:3: Expected definition of PKG_OPTIONS_VAR.")
 }
 
-func (s *Suite) Test_Package_checkIncludeConditionally__mixed(c *check.C) {
+func (s *Suite) Test_Package_checkIncludeConditionally__unconditionally_first(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpPackage("category/package")
+	t.Chdir("category/package")
+	t.CreateFileLines("including.mk",
+		MkCvsID,
+		"",
+		".include \"included.mk\"",
+		".if ${OPSYS} == \"Linux\"",
+		".include \"included.mk\"",
+		".endif")
+	t.CreateFileLines("included.mk",
+		MkCvsID)
+	t.FinishSetUp()
+
+	G.Check(".")
+
+	t.CheckOutputLines(
+		"WARN: including.mk:3: \"included.mk\" is included " +
+			"unconditionally here and conditionally in line 5 (depending on OPSYS).")
+}
+
+func (s *Suite) Test_Package_checkIncludeConditionally__only_conditionally(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpPackage("category/package",
+		".if ${OPSYS} == \"Linux\"",
+		".include \"included.mk\"",
+		".endif")
+	t.Chdir("category/package")
+	t.CreateFileLines("included.mk",
+		MkCvsID)
+	t.FinishSetUp()
+
+	G.Check(".")
+
+	t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_Package_checkIncludeConditionally__conditionally_first(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpPackage("category/package")
+	t.Chdir("category/package")
+	t.CreateFileLines("including.mk",
+		MkCvsID,
+		"",
+		".if ${OPSYS} == \"Linux\"",
+		".include \"included.mk\"",
+		".endif",
+		".include \"included.mk\"")
+	t.CreateFileLines("included.mk",
+		MkCvsID)
+	t.FinishSetUp()
+
+	G.Check(".")
+
+	t.CheckOutputLines(
+		"WARN: including.mk:4: \"included.mk\" is included " +
+			"conditionally here (depending on OPSYS) and unconditionally in line 6.")
+}
+
+func (s *Suite) Test_Package_checkIncludeConditionally__included_multiple_times(c *check.C) {
 	t := s.Init(c)
 
 	t.SetUpPackage("category/package")
@@ -1228,12 +1378,34 @@ func (s *Suite) Test_Package_checkIncludeConditionally__mixed(c *check.C) {
 	G.Check(".")
 
 	t.CheckOutputLines(
+		"WARN: including.mk:3: \"included.mk\" is included "+
+			"unconditionally here and conditionally in line 10 (depending on OPSYS).",
 		"WARN: including.mk:5: \"included.mk\" is included "+
-			"conditionally here (depending on OPSYS) and unconditionally in line 3.",
+			"conditionally here (depending on OPSYS) and unconditionally in line 8.",
 		"WARN: including.mk:8: \"included.mk\" is included "+
-			"unconditionally here and conditionally in line 5 (depending on OPSYS).",
-		"WARN: including.mk:10: \"included.mk\" is included "+
-			"conditionally here (depending on OPSYS) and unconditionally in line 8.")
+			"unconditionally here and conditionally in line 10 (depending on OPSYS).")
+}
+
+// For preferences files, it doesn't matter whether they are included
+// conditionally or unconditionally since at the end they are included
+// anyway by the infrastructure.
+func (s *Suite) Test_Package_checkIncludeConditionally__prefs(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpPackage("category/package")
+	t.Chdir("category/package")
+	t.CreateFileLines("including.mk",
+		MkCvsID,
+		"",
+		".include \"../../mk/bsd.prefs.mk\"",
+		".if ${OPSYS} == \"Linux\"",
+		".include \"../../mk/bsd.prefs.mk\"",
+		".endif")
+	t.FinishSetUp()
+
+	G.Check(".")
+
+	t.CheckOutputEmpty()
 }
 
 func (s *Suite) Test_Package_checkIncludeConditionally__other_directory(c *check.C) {
@@ -2196,6 +2368,7 @@ func (s *Suite) Test_Package_collectSeenInclude__builtin_mk(c *check.C) {
 
 func (s *Suite) Test_Package_diveInto(c *check.C) {
 	t := s.Init(c)
+	t.Chdir(".")
 
 	test := func(including, included string, expected bool) {
 		actual := (*Package)(nil).diveInto(including, included)
@@ -2235,6 +2408,10 @@ func (s *Suite) Test_Package_diveInto(c *check.C) {
 	test("../../mk/one.mk", "two.mk", false)
 	test("../../mk/one.mk", "../../mk/two.mk", false)
 	test("../../mk/one.mk", "../lang/go/version.mk", false)
+
+	// wip/mk doesn't count as infrastructure since it is often used as a
+	// second layer, using the API of the main mk/ infrastructure.
+	test("wip/mk/cargo-binary.mk", "../../lang/rust/cargo.mk", true)
 }
 
 func (s *Suite) Test_Package_collectSeenInclude__multiple(c *check.C) {
@@ -2734,3 +2911,36 @@ func (s *Suite) Test_Package_checkPlist__PERL5_USE_PACKLIST_yes(c *check.C) {
 	t.CheckOutputLines(
 		"WARN: ~/category/p5-Packlist/Makefile:20: This package should not have a PLIST file.")
 }
+
+func (s *Suite) Test_Package_Includes(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpPackage("category/package",
+		".include \"unconditionally.mk\"",
+		".if 0",
+		".include \"never.mk\"",
+		".endif",
+		".if ${OPSYS} == Linux",
+		".include \"conditionally.mk\"",
+		".endif")
+	t.CreateFileLines("category/package/unconditionally.mk",
+		MkCvsID)
+	t.CreateFileLines("category/package/conditionally.mk",
+		MkCvsID)
+	t.CreateFileLines("category/package/never.mk",
+		MkCvsID)
+	t.FinishSetUp()
+
+	pkg := NewPackage(t.File("category/package"))
+
+	pkg.load()
+
+	t.CheckEquals(pkg.Includes("unconditionally.mk"), true)
+	t.CheckEquals(pkg.Includes("conditionally.mk"), true)
+	t.CheckEquals(pkg.Includes("other.mk"), false)
+
+	// TODO: Strictly speaking, never.mk should be in conditionalIncludes.
+	//  This is an edge case though. See collectConditionalIncludes and
+	//  Indentation.IsConditional for the current implementation.
+	t.CheckEquals(pkg.conditionalIncludes["never.mk"], (*MkLine)(nil))
+}
diff --git a/pkgtools/pkglint/files/pkglint.go b/pkgtools/pkglint/files/pkglint.go
index 97bf9fc1cf7..f1a016521af 100644
--- a/pkgtools/pkglint/files/pkglint.go
+++ b/pkgtools/pkglint/files/pkglint.go
@@ -350,9 +350,6 @@ func (pkglint *Pkglint) ParseCommandLine(args []string) int {
 //
 // It sets up all the global state (infrastructure, wip) for accurately
 // classifying the entry.
-//
-// During tests, it assumes that Pkgsrc.LoadInfrastructure has been called.
-// It is the most high-level method for testing pkglint.
 func (pkglint *Pkglint) Check(dirent string) {
 	if trace.Tracing {
 		defer trace.Call1(dirent)()
diff --git a/pkgtools/pkglint/files/varalignblock.go b/pkgtools/pkglint/files/varalignblock.go
index d07f021b44d..2656ab43c65 100644
--- a/pkgtools/pkglint/files/varalignblock.go
+++ b/pkgtools/pkglint/files/varalignblock.go
@@ -411,7 +411,7 @@ func (*VaralignBlock) realignMultiEmptyInitial(info *varalignLine, newWidth int)
 
 	fix := info.mkline.Autofix()
 	if newSpace == " " {
-		fix.Notef("This outlier variable should be aligned with a single space.")
+		fix.Notef("This outlier variable value should be aligned with a single space.")
 	} else if hasSpace && column != oldColumn {
 		fix.Notef("This variable value should be aligned with tabs, not spaces, to column %d.", column+1)
 	} else if column != oldColumn {
diff --git a/pkgtools/pkglint/files/varalignblock_test.go b/pkgtools/pkglint/files/varalignblock_test.go
index 3b4e31a69c0..46c29942cc7 100644
--- a/pkgtools/pkglint/files/varalignblock_test.go
+++ b/pkgtools/pkglint/files/varalignblock_test.go
@@ -1055,7 +1055,7 @@ func (s *Suite) Test_VaralignBlock__outlier_in_follow_continuation(c *check.C) {
 		"38 38",
 		"   24")
 	vt.Diagnostics(
-		"NOTE: ~/Makefile:2: This outlier variable should be aligned with a single space.")
+		"NOTE: ~/Makefile:2: This outlier variable value should be aligned with a single space.")
 	vt.Autofixes(
 		"AUTOFIX: ~/Makefile:2: Replacing \"\" with \" \".")
 	vt.Fixed(
@@ -2426,7 +2426,7 @@ func (s *Suite) Test_VaralignBlock_realignMultiEmptyInitial(c *check.C) {
 	mklines.Check()
 
 	t.CheckOutputLines(
-		"NOTE: filename.mk:3: This outlier variable should be aligned with a single space.")
+		"NOTE: filename.mk:3: This outlier variable value should be aligned with a single space.")
 }
 
 func (s *Suite) Test_VaralignBlock_realignMultiEmptyInitial__spaces(c *check.C) {