pkgtools/pkglint: update to 19.4.11

Changes since 19.4.10:

The use of PKG_OPTIONS and PKG_BUILD_OPTIONS in buildlink3.mk and other
files is checked for common mistakes.

Checking the indentation of a continuation line no longer crashes in edge
cases.

16 files changed, 357 insertions, 58 deletions

diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile
index 965a4cf21a8..a35746cd92f 100644
--- a/pkgtools/pkglint/Makefile
+++ b/pkgtools/pkglint/Makefile
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.633 2020/03/07 23:35:35 rillig Exp $
+# $NetBSD: Makefile,v 1.634 2020/03/15 11:31:24 rillig Exp $
 
-PKGNAME=	pkglint-19.4.10
+PKGNAME=	pkglint-19.4.11
 CATEGORIES=	pkgtools
 DISTNAME=	tools
 MASTER_SITES=	${MASTER_SITE_GITHUB:=golang/}
diff --git a/pkgtools/pkglint/files/buildlink3.go b/pkgtools/pkglint/files/buildlink3.go
index 124dddcb35d..910313da1c0 100644
--- a/pkgtools/pkglint/files/buildlink3.go
+++ b/pkgtools/pkglint/files/buildlink3.go
@@ -13,8 +13,8 @@ type Buildlink3Checker struct {
 	abi, api         *DependencyPattern
 }
 
-func CheckLinesBuildlink3Mk(mklines *MkLines) {
-	(&Buildlink3Checker{mklines: mklines}).Check()
+func NewBuildlink3Checker(mklines *MkLines) *Buildlink3Checker {
+	return &Buildlink3Checker{mklines: mklines}
 }
 
 func (ck *Buildlink3Checker) Check() {
@@ -182,6 +182,10 @@ func (ck *Buildlink3Checker) checkMainPart(mlex *MkLinesLexer) bool {
 		case mkline.IsDirective() && mkline.Directive() == "endif":
 			indentLevel--
 		}
+
+		mkline.ForEachUsed(func(varUse *MkVarUse, time VucTime) {
+			ck.checkVarUse(varUse, mkline)
+		})
 	}
 
 	if indentLevel > 0 {
@@ -195,6 +199,32 @@ func (ck *Buildlink3Checker) checkMainPart(mlex *MkLinesLexer) bool {
 	return true
 }
 
+func (ck *Buildlink3Checker) checkVarUse(varUse *MkVarUse, mkline *MkLine) {
+	varname := varUse.varname
+	if varname == "PKG_OPTIONS" {
+		mkline.Errorf("PKG_OPTIONS is not available in buildlink3.mk files.")
+		mkline.Explain(
+			"The buildlink3.mk file of a package is only ever included",
+			"by other packages, never by the package itself.",
+			"Therefore it does not make sense to use the variable PKG_OPTIONS",
+			"in this place since it contains the package options of a random",
+			"package that happens to include this file.",
+			"",
+			"To access the options of this package, see mk/pkg-build-options.mk.")
+	}
+
+	if varnameBase(varname) == "PKG_BUILD_OPTIONS" {
+		param := varnameParam(varname)
+		if param != "" && param != ck.pkgbase {
+			mkline.Warnf("Wrong PKG_BUILD_OPTIONS, expected %q instead of %q.",
+				ck.pkgbase, param)
+			mkline.Explain(
+				"The variable parameter for PKG_BUILD_OPTIONS must correspond",
+				"to the value of \"pkgbase\" above.")
+		}
+	}
+}
+
 func (ck *Buildlink3Checker) checkVarassign(mkline *MkLine, pkgbase string) {
 	varname, value := mkline.Varname(), mkline.Value()
 	doCheck := false
diff --git a/pkgtools/pkglint/files/buildlink3_test.go b/pkgtools/pkglint/files/buildlink3_test.go
index 76103062383..9f2599b73e2 100644
--- a/pkgtools/pkglint/files/buildlink3_test.go
+++ b/pkgtools/pkglint/files/buildlink3_test.go
@@ -2,6 +2,10 @@ package pkglint
 
 import "gopkg.in/check.v1"
 
+func CheckLinesBuildlink3Mk(mklines *MkLines) {
+	NewBuildlink3Checker(mklines).Check()
+}
+
 // This test ensures that CheckLinesBuildlink3Mk really checks for
 // buildlink3.mk files that are included by the buildlink3.mk file
 // but not by the package.
@@ -473,6 +477,25 @@ func (s *Suite) Test_CheckLinesBuildlink3Mk__invalid_dependency_patterns(c *chec
 		"WARN: buildlink3.mk:10: Invalid dependency pattern \"hs-X11!=1.6.1.2nb2\".")
 }
 
+func (s *Suite) Test_CheckLinesBuildlink3Mk__PKG_OPTIONS(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpOption("option", "")
+	t.SetUpPkgsrc()
+	t.SetUpPackage("category/package")
+	t.CreateFileBuildlink3("category/package/buildlink3.mk",
+		".include \"../../mk/bsd.fast.prefs.mk\"",
+		".if ${PKG_OPTIONS:Moption}",
+		".endif")
+	t.FinishSetUp()
+
+	G.Check(t.File("category/package/buildlink3.mk"))
+
+	t.CheckOutputLines(
+		"ERROR: ~/category/package/buildlink3.mk:13: " +
+			"PKG_OPTIONS is not available in buildlink3.mk files.")
+}
+
 // Just for branch coverage.
 func (s *Suite) Test_Buildlink3Checker_Check__no_tracing(c *check.C) {
 	t := s.Init(c)
@@ -661,6 +684,37 @@ func (s *Suite) Test_Buildlink3Checker_checkMainPart__comment_at_end_of_file(c *
 		"WARN: ~/category/package/buildlink3.mk:14: The file should end here.")
 }
 
+func (s *Suite) Test_Buildlink3Checker_checkVarUse__PKG_BUILD_OPTIONS(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpOption("option", "")
+	t.SetUpPkgsrc()
+	t.CreateFileLines("mk/pkg-build-options.mk")
+	t.SetUpPackage("category/package")
+	t.CreateFileBuildlink3("category/package/buildlink3.mk",
+		"pkgbase := unrelated",
+		".include \"../../mk/pkg-build-options.mk\"",
+		"",
+		".if ${PKG_BUILD_OPTIONS:Moption}", // missing variable parameter
+		".endif",
+		"",
+		".if ${PKG_BUILD_OPTIONS.package:Moption}", // wrong variable parameter
+		".endif",
+		"",
+		".if ${PKG_BUILD_OPTIONS.unrelated:Moption}", // corresponds to pkgbase above
+		".endif")
+	t.FinishSetUp()
+
+	G.Check(t.File("category/package/buildlink3.mk"))
+
+	// TODO: Warn about PKG_BUILD_OPTIONS.package since that variable is not defined.
+	t.CheckOutputLines(
+		"WARN: ~/category/package/buildlink3.mk:15: "+
+			"PKG_BUILD_OPTIONS is used but not defined.",
+		"WARN: ~/category/package/buildlink3.mk:21: "+
+			"Wrong PKG_BUILD_OPTIONS, expected \"package\" instead of \"unrelated\".")
+}
+
 // As of October 2018, pkglint parses package dependencies a little
 // different than the pkg_* tools.
 // In all but two cases this works, this is one of the exceptions.
diff --git a/pkgtools/pkglint/files/check_test.go b/pkgtools/pkglint/files/check_test.go
index be89c8e5bbe..6d66e754a51 100644
--- a/pkgtools/pkglint/files/check_test.go
+++ b/pkgtools/pkglint/files/check_test.go
@@ -581,15 +581,19 @@ func (t *Tester) CreateFileDummyPatch(filename RelPath) {
 }
 
 func (t *Tester) CreateFileBuildlink3(filename RelPath, customLines ...string) {
+	lower := filename.Dir().Base()
+	t.CreateFileBuildlink3Id(filename, lower, customLines...)
+}
+
+func (t *Tester) CreateFileBuildlink3Id(filename RelPath, id string, customLines ...string) {
 	// Buildlink3.mk files only make sense in category/package directories.
 	assert(G.Pkgsrc.Rel(t.File(filename)).Count() == 3)
 
 	dir := filename.Dir().Clean()
-	lower := dir.Base()
 	// see pkgtools/createbuildlink/files/createbuildlink, "package specific variables"
-	upper := strings.Replace(strings.ToUpper(lower), "-", "_", -1)
+	upperID := strings.Replace(strings.ToUpper(id), "-", "_", -1)
 
-	width := tabWidthSlice("BUILDLINK_API_DEPENDS.", lower, "+=\t")
+	width := tabWidthSlice("BUILDLINK_API_DEPENDS.", id, "+=\t")
 
 	aligned := func(format string, args ...interface{}) string {
 		msg := sprintf(format, args...)
@@ -603,21 +607,21 @@ func (t *Tester) CreateFileBuildlink3(filename RelPath, customLines ...string) {
 	lines = append(lines,
 		MkCvsID,
 		"",
-		sprintf("BUILDLINK_TREE+=\t%s", lower),
+		sprintf("BUILDLINK_TREE+=\t%s", id),
 		"",
-		sprintf(".if !defined(%s_BUILDLINK3_MK)", upper),
-		sprintf("%s_BUILDLINK3_MK:=", upper),
+		sprintf(".if !defined(%s_BUILDLINK3_MK)", upperID),
+		sprintf("%s_BUILDLINK3_MK:=", upperID),
 		"",
-		aligned("BUILDLINK_API_DEPENDS.%s+=", lower)+sprintf("%s>=0", lower),
-		aligned("BUILDLINK_PKGSRCDIR.%s?=", lower)+sprintf("../../%s", dir),
-		aligned("BUILDLINK_DEPMETHOD.%s?=", lower)+"build",
+		aligned("BUILDLINK_API_DEPENDS.%s+=", id)+sprintf("%s>=0", id),
+		aligned("BUILDLINK_PKGSRCDIR.%s?=", id)+sprintf("../../%s", dir),
+		aligned("BUILDLINK_DEPMETHOD.%s?=", id)+"build",
 		"")
 	lines = append(lines, customLines...)
 	lines = append(lines,
 		"",
-		sprintf(".endif # %s_BUILDLINK3_MK", upper),
+		sprintf(".endif # %s_BUILDLINK3_MK", upperID),
 		"",
-		sprintf("BUILDLINK_TREE+=\t-%s", lower))
+		sprintf("BUILDLINK_TREE+=\t-%s", id))
 
 	t.CreateFileLines(filename, lines...)
 }
diff --git a/pkgtools/pkglint/files/mkcondchecker.go b/pkgtools/pkglint/files/mkcondchecker.go
index ebe0915d5dd..7617b04fa26 100644
--- a/pkgtools/pkglint/files/mkcondchecker.go
+++ b/pkgtools/pkglint/files/mkcondchecker.go
@@ -82,12 +82,21 @@ func (ck *MkCondChecker) checkNotEmpty(not *MkCond) {
 	// This applies especially to the ${VAR:Mpattern} form.
 	//
 	// See MkCondChecker.simplify.
-	if !G.Experimental {
+	if !hasPrefix(not.Empty.varname, "PKG_BUILD_OPTIONS.") {
 		return
 	}
 
-	ck.MkLine.Notef("!empty(%s%s) can be replaced with the simpler %s.",
-		not.Empty.varname, not.Empty.Mod(), not.Empty.String())
+	fix := ck.MkLine.Autofix()
+	from := sprintf("!empty(%s%s)", not.Empty.varname, not.Empty.Mod())
+	to := not.Empty.String()
+	fix.Notef("%s can be replaced with %s.", from, to)
+	fix.Explain(
+		"Besides being simpler to read, the expression will also fail",
+		"quickly with a \"Malformed conditional\" error from bmake",
+		"if it should ever be undefined at this point.",
+		"This catches typos and other programming mistakes.")
+	fix.Replace(from, to)
+	fix.Apply()
 }
 
 // checkEmpty checks a condition of the form empty(VAR),
diff --git a/pkgtools/pkglint/files/mkcondchecker_test.go b/pkgtools/pkglint/files/mkcondchecker_test.go
index ce4c7fce9d0..b6ec7a31437 100644
--- a/pkgtools/pkglint/files/mkcondchecker_test.go
+++ b/pkgtools/pkglint/files/mkcondchecker_test.go
@@ -219,7 +219,7 @@ func (s *Suite) Test_MkCondChecker_Check__comparing_PKGSRC_COMPILER_with_eqeq(c
 func (s *Suite) Test_MkCondChecker_checkNotEmpty(c *check.C) {
 	t := s.Init(c)
 
-	G.Experimental = true
+	t.SetUpVartypes()
 
 	test := func(cond string, diagnostics ...string) {
 		mklines := t.NewMkLines("filename.mk",
@@ -233,8 +233,17 @@ func (s *Suite) Test_MkCondChecker_checkNotEmpty(c *check.C) {
 	}
 
 	test("!empty(VAR)",
-		// TODO: Add a :U modifier if VAR might be undefined.
-		"NOTE: filename.mk:1: !empty(VAR) can be replaced with the simpler ${VAR}.")
+
+		// Only a few variables are suggested to use the simpler form,
+		// because of the side-effect when the variable is undefined.
+		// VAR is not one of these variables.
+		nil...)
+
+	test(
+		"!empty(PKG_BUILD_OPTIONS.package:Moption)",
+
+		"NOTE: filename.mk:1: !empty(PKG_BUILD_OPTIONS.package:Moption) "+
+			"can be replaced with ${PKG_BUILD_OPTIONS.package:Moption}.")
 }
 
 func (s *Suite) Test_MkCondChecker_checkEmpty(c *check.C) {
@@ -395,13 +404,14 @@ func (s *Suite) Test_MkCondChecker_simplify(c *check.C) {
 	t.Chdir("category/package")
 
 	// The Anything type suppresses the warnings from type checking.
-	// BtUnknown would not work, see Pkgsrc.VariableType.
+	// BtUnknown would not work here, see Pkgsrc.VariableType.
 	btAnything := &BasicType{"Anything", func(cv *VartypeCheck) {}}
 
 	// For simplifying the expressions, it is necessary to know whether
 	// a variable can be undefined. Undefined variables need the
-	// :U modifier, otherwise bmake will complain about "malformed
-	// conditions", which is not entirely precise since the expression
+	// :U modifier or must be enclosed in double quotes, otherwise
+	// bmake will complain about a "Malformed conditional". That error
+	// message is not entirely precise since the expression
 	// is syntactically valid, it's just the evaluation that fails.
 	//
 	// Some variables such as MACHINE_ARCH are in scope from the very
@@ -474,6 +484,7 @@ func (s *Suite) Test_MkCondChecker_simplify(c *check.C) {
 		doTest(true, before, after, diagnostics...)
 	}
 	testBeforeAndAfterPrefs := func(before, after string, diagnostics ...string) {
+		doTest(false, before, after, diagnostics...)
 		doTest(true, before, after, diagnostics...)
 	}
 
@@ -919,13 +930,24 @@ func (s *Suite) Test_MkCondChecker_simplify(c *check.C) {
 	// replaced automatically, see mkCondLiteralChars.
 	// TODO: Add tests for all characters that are special in string literals or patterns.
 	// TODO: Then, extend the set of characters for which the expressions are simplified.
-	testBeforeAndAfterPrefs(
+	testBeforePrefs(
+		".if ${PREFS_DEFINED:M&&}",
+		".if ${PREFS_DEFINED:M&&}",
+
+		"WARN: filename.mk:3: To use PREFS_DEFINED at load time, .include \"../../mk/bsd.prefs.mk\" first.")
+	testAfterPrefs(
 		".if ${PREFS_DEFINED:M&&}",
 		".if ${PREFS_DEFINED:M&&}",
 
 		nil...)
 
-	testBeforeAndAfterPrefs(
+	testBeforePrefs(
+		".if ${PREFS:M&&}",
+		".if ${PREFS:M&&}",
+
+		// TODO: Warn that the :U is missing.
+		"WARN: filename.mk:3: To use PREFS at load time, .include \"../../mk/bsd.prefs.mk\" first.")
+	testAfterPrefs(
 		".if ${PREFS:M&&}",
 		".if ${PREFS:M&&}",
 
@@ -947,7 +969,19 @@ func (s *Suite) Test_MkCondChecker_simplify(c *check.C) {
 
 	// The characters <=> may be used unescaped in :M and :N patterns
 	// but not in .if conditions. There they must be enclosed in quotes.
-	testBeforeAndAfterPrefs(
+	testBeforePrefs(
+		".if ${PREFS_DEFINED:M<=>}",
+		".if ${PREFS_DEFINED:U} == \"<=>\"",
+
+		"NOTE: filename.mk:3: PREFS_DEFINED can be "+
+			"compared using the simpler \"${PREFS_DEFINED:U} == \"<=>\"\" "+
+			"instead of matching against \":M<=>\".",
+		"WARN: filename.mk:3: To use PREFS_DEFINED at load time, "+
+			".include \"../../mk/bsd.prefs.mk\" first.",
+		"AUTOFIX: filename.mk:3: "+
+			"Replacing \"${PREFS_DEFINED:M<=>}\" "+
+			"with \"${PREFS_DEFINED:U} == \\\"<=>\\\"\".")
+	testAfterPrefs(
 		".if ${PREFS_DEFINED:M<=>}",
 		".if ${PREFS_DEFINED} == \"<=>\"",
 
diff --git a/pkgtools/pkglint/files/mkvarusechecker.go b/pkgtools/pkglint/files/mkvarusechecker.go
index 01964291922..d76376b53c6 100644
--- a/pkgtools/pkglint/files/mkvarusechecker.go
+++ b/pkgtools/pkglint/files/mkvarusechecker.go
@@ -33,6 +33,7 @@ func (ck *MkVarUseChecker) Check(vuc *VarUseContext) {
 	ck.checkToolsPlatform()
 	ck.checkBuildDefs()
 	ck.checkDeprecated()
+	ck.checkPkgBuildOptions()
 
 	NewMkLineChecker(ck.MkLines, ck.MkLine).
 		checkTextVarUse(ck.use.varname, ck.vartype, vuc.time)
@@ -738,3 +739,26 @@ func (ck *MkVarUseChecker) checkDeprecated() {
 
 	ck.MkLine.Warnf("Use of %q is deprecated. %s", varname, instead)
 }
+
+func (ck *MkVarUseChecker) checkPkgBuildOptions() {
+	pkg := ck.MkLines.pkg
+	if pkg == nil {
+		return
+	}
+	varname := ck.use.varname
+	if !hasPrefix(varname, "PKG_BUILD_OPTIONS.") {
+		return
+	}
+	param := varnameParam(varname)
+	if pkg.seenPkgbase.Seen(param) {
+		return
+	}
+
+	ck.MkLine.Warnf("The PKG_BUILD_OPTIONS for %q are not available to this package.",
+		param)
+	ck.MkLine.Explain(
+		"The variable parameter for PKG_BUILD_OPTIONS must correspond",
+		"to the value of \"pkgbase\" above.",
+		"",
+		"For more information, see mk/pkg-build-options.mk.")
+}
diff --git a/pkgtools/pkglint/files/mkvarusechecker_test.go b/pkgtools/pkglint/files/mkvarusechecker_test.go
index c2638fd11f0..46f3458d294 100644
--- a/pkgtools/pkglint/files/mkvarusechecker_test.go
+++ b/pkgtools/pkglint/files/mkvarusechecker_test.go
@@ -1325,3 +1325,42 @@ func (s *Suite) Test_MkVarUseChecker_checkDeprecated(c *check.C) {
 		"WARN: filename.mk:3: Use of \"USE_CROSSBASE\" is deprecated. "+
 			"Has been removed.")
 }
+
+// This test demonstrates some typos that an inexperienced pkgsrc developer
+// might make. This scenario is not intended to be realistic.
+func (s *Suite) Test_MkVarUseChecker_checkPkgBuildOptions(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpOption("option", "")
+	t.SetUpPackage("category/package",
+		".include \"../../category/lib/buildlink3.mk\"")
+	t.SetUpPackage("category/lib")
+	t.CreateFileLines("mk/pkg-build-options.mk")
+	t.CreateFileBuildlink3("category/package/buildlink3.mk",
+		".include \"../../mk/bsd.fast.prefs.mk\"",
+		"",
+		".if ${PKG_BUILD_OPTIONS.lib:Moption}", // Too early
+		".endif",
+		"",
+		".if ${PKG_BUILD_OPTIONS.unrelated:Moption}",
+		".include \"../../category/lib/buildlink3.mk\"",
+		".endif",
+		"",
+		".if ${PKG_BUILD_OPTIONS.lib:Moption}", // Only defined conditionally
+		".endif")
+	t.CreateFileBuildlink3("category/lib/buildlink3.mk",
+		"pkgbase := lib",
+		".include \"../../mk/pkg-build-options.mk\"")
+	t.Chdir("category/package")
+	t.FinishSetUp()
+
+	G.Check(".")
+
+	t.CheckOutputLines(
+		"WARN: Makefile:20: \"../../category/lib/buildlink3.mk\" is included unconditionally here "+
+			"and conditionally in buildlink3.mk:18 (depending on PKG_BUILD_OPTIONS.unrelated).",
+		"WARN: buildlink3.mk:17: The PKG_BUILD_OPTIONS for \"unrelated\" are not available to this package.",
+		"WARN: buildlink3.mk:14: Wrong PKG_BUILD_OPTIONS, expected \"package\" instead of \"lib\".",
+		"WARN: buildlink3.mk:17: Wrong PKG_BUILD_OPTIONS, expected \"package\" instead of \"unrelated\".",
+		"WARN: buildlink3.mk:21: Wrong PKG_BUILD_OPTIONS, expected \"package\" instead of \"lib\".")
+}
diff --git a/pkgtools/pkglint/files/options.go b/pkgtools/pkglint/files/options.go
index e08af3c9d9c..e9a84c8d978 100755
--- a/pkgtools/pkglint/files/options.go
+++ b/pkgtools/pkglint/files/options.go
@@ -1,8 +1,9 @@
 package pkglint
 
-func CheckLinesOptionsMk(mklines *MkLines) {
+func CheckLinesOptionsMk(mklines *MkLines, buildlinkID string) {
 	ck := OptionsLinesChecker{
 		mklines,
+		buildlinkID,
 		false,
 		make(map[string]*MkLine),
 		false,
@@ -16,7 +17,8 @@ func CheckLinesOptionsMk(mklines *MkLines) {
 //
 // See mk/bsd.options.mk for a detailed description.
 type OptionsLinesChecker struct {
-	mklines *MkLines
+	mklines     *MkLines
+	buildlinkID string
 
 	declaredArbitrary         bool
 	declaredOptions           map[string]*MkLine
@@ -49,6 +51,18 @@ func (ck *OptionsLinesChecker) collect() {
 		if !seenInclude {
 			if !seenPkgOptionsVar && mkline.IsVarassign() && mkline.Varname() == "PKG_OPTIONS_VAR" {
 				seenPkgOptionsVar = true
+				buildlinkID := ck.buildlinkID
+				optionsID := varnameParam(mkline.Value())
+				if buildlinkID != "" && optionsID != "" && buildlinkID != optionsID {
+					mkline.Warnf("The buildlink3 identifier %q should be the same as the options identifier %q.",
+						buildlinkID, optionsID)
+					mkline.Explain(
+						"Having different identifiers refer to the same package",
+						"is confusing for the pkgsrc user.",
+						"The pkgsrc infrastructure doesn't care though",
+						"if the identifiers in PKG_OPTIONS.* and PKG_BUILD_OPTIONS.*",
+						"are the same or not.")
+				}
 			}
 			seenInclude = mkline.IsInclude() && mkline.IncludedFile() == "../../mk/bsd.options.mk"
 		}
diff --git a/pkgtools/pkglint/files/options_test.go b/pkgtools/pkglint/files/options_test.go
index bd3fd304b46..aae9e26ab61 100755
--- a/pkgtools/pkglint/files/options_test.go
+++ b/pkgtools/pkglint/files/options_test.go
@@ -88,7 +88,7 @@ func (s *Suite) Test_CheckLinesOptionsMk__literal(c *check.C) {
 	t.Chdir("category/package")
 	t.FinishSetUp()
 
-	CheckLinesOptionsMk(mklines)
+	CheckLinesOptionsMk(mklines, "")
 
 	t.CheckOutputLines(
 		"WARN: ~/category/package/options.mk:4: "+
@@ -124,7 +124,7 @@ func (s *Suite) Test_CheckLinesOptionsMk__literal_in_for_loop(c *check.C) {
 	t.Chdir("category/package")
 	t.FinishSetUp()
 
-	CheckLinesOptionsMk(mklines)
+	CheckLinesOptionsMk(mklines, "")
 
 	t.CheckOutputLines(
 		"WARN: ~/category/package/options.mk:5: "+
@@ -158,7 +158,7 @@ func (s *Suite) Test_CheckLinesOptionsMk__prefs(c *check.C) {
 	t.Chdir("category/package")
 	t.FinishSetUp()
 
-	CheckLinesOptionsMk(mklines)
+	CheckLinesOptionsMk(mklines, "")
 
 	t.CheckOutputEmpty()
 }
@@ -184,7 +184,7 @@ func (s *Suite) Test_CheckLinesOptionsMk__variable_order(c *check.C) {
 	t.Chdir("category/package")
 	t.FinishSetUp()
 
-	CheckLinesOptionsMk(mklines)
+	CheckLinesOptionsMk(mklines, "")
 
 	t.CheckOutputLines(
 		"WARN: ~/category/package/options.mk:3: " +
@@ -201,7 +201,7 @@ func (s *Suite) Test_CheckLinesOptionsMk__empty(c *check.C) {
 	t.Chdir("category/package")
 	t.FinishSetUp()
 
-	CheckLinesOptionsMk(mklines)
+	CheckLinesOptionsMk(mklines, "")
 
 	t.CheckOutputLines(
 		"ERROR: ~/category/package/options.mk: "+
@@ -240,7 +240,7 @@ func (s *Suite) Test_CheckLinesOptionsMk__conditionals(c *check.C) {
 		".endif")
 	t.FinishSetUp()
 
-	CheckLinesOptionsMk(mklines)
+	CheckLinesOptionsMk(mklines, "")
 
 	t.CheckOutputLines(
 		// This warning comes from VarTypeCheck.PkgOption
@@ -300,7 +300,7 @@ func (s *Suite) Test_CheckLinesOptionsMk(c *check.C) {
 		".elif !empty(PKG_OPTIONS:Msqlite)",
 		".endif")
 
-	CheckLinesOptionsMk(mklines)
+	CheckLinesOptionsMk(mklines, "")
 
 	t.CheckOutputLines(
 		"WARN: ~/category/package/options.mk:6: l is used but not defined.",
@@ -335,7 +335,7 @@ func (s *Suite) Test_CheckLinesOptionsMk__unexpected_line(c *check.C) {
 		"pre-configure:",
 		"\techo \"In the pre-configure stage.\"")
 
-	CheckLinesOptionsMk(mklines)
+	CheckLinesOptionsMk(mklines, "")
 
 	t.CheckOutputLines(
 		"WARN: ~/category/package/options.mk:6: "+
@@ -375,7 +375,7 @@ func (s *Suite) Test_CheckLinesOptionsMk__malformed_condition(c *check.C) {
 		".if ${OPSYS} == 'Darwin'",
 		".endif")
 
-	CheckLinesOptionsMk(mklines)
+	CheckLinesOptionsMk(mklines, "")
 
 	t.CheckOutputLines(
 		"WARN: category/package/options.mk:15: Invalid condition, unrecognized part: \"${OPSYS} == 'Darwin'\".")
@@ -618,7 +618,7 @@ func (s *Suite) Test_CheckLinesOptionsMk__indirect_supported_options_parentheses
 		".  endif",
 		".endfor")
 
-	CheckLinesOptionsMk(mklines)
+	CheckLinesOptionsMk(mklines, "")
 
 	t.CheckOutputLines(
 		"WARN: ~/category/package/options.mk:5: "+
@@ -645,7 +645,7 @@ func (s *Suite) Test_CheckLinesOptionsMk__handled_but_not_supported(c *check.C)
 		".if ${PKG_OPTIONS:Moption}",
 		".endif")
 
-	CheckLinesOptionsMk(mklines)
+	CheckLinesOptionsMk(mklines, "")
 
 	t.CheckOutputLines(
 		"WARN: ~/category/package/options.mk:8: " +
@@ -670,7 +670,7 @@ func (s *Suite) Test_CheckLinesOptionsMk__supported_but_not_checked(c *check.C)
 		".if ${PKG_OPTIONS:Mopt${:Uion}}",
 		".endif")
 
-	CheckLinesOptionsMk(mklines)
+	CheckLinesOptionsMk(mklines, "")
 
 	// Pkglint does not expand the ${:Uion}, therefore it doesn't know that
 	// the option is indeed handled. Because of this uncertainty, pkglint
diff --git a/pkgtools/pkglint/files/package.go b/pkgtools/pkglint/files/package.go
index 5014cffd7e8..fec7a7340d4 100644
--- a/pkgtools/pkglint/files/package.go
+++ b/pkgtools/pkglint/files/package.go
@@ -16,17 +16,21 @@ const rePkgname = `^([\w\-.+]+)-(\d[.0-9A-Z_a-z]*)$`
 // This is necessary because variables in Makefiles may be used before they are defined,
 // and such dependencies often span multiple files that are included indirectly.
 type Package struct {
-	dir                  CurrPath     // The directory of the package, for resolving files
-	Pkgpath              PkgsrcPath   // e.g. "category/pkgdir"
-	Pkgdir               PackagePath  // PKGDIR from the package Makefile
-	Filesdir             PackagePath  // FILESDIR from the package Makefile
-	Patchdir             PackagePath  // PATCHDIR from the package Makefile
-	DistinfoFile         PackagePath  // DISTINFO_FILE from the package Makefile
-	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
-	Plist                PlistContent // Files and directories mentioned in the PLIST files
+	dir     CurrPath   // The directory of the package, for resolving files
+	Pkgpath PkgsrcPath // e.g. "category/pkgdir"
+
+	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
+	buildlinkID          string
+	optionsID            string
+
+	Pkgdir       PackagePath  // PKGDIR from the package Makefile
+	Filesdir     PackagePath  // FILESDIR from the package Makefile
+	Patchdir     PackagePath  // PATCHDIR from the package Makefile
+	DistinfoFile PackagePath  // DISTINFO_FILE from the package Makefile
+	Plist        PlistContent // Files and directories mentioned in the PLIST files
 
 	vars      Scope
 	redundant *RedundantScope
@@ -55,6 +59,9 @@ type Package struct {
 	// TODO: Be more precise about the purpose of this field.
 	seenInclude bool
 
+	// The identifiers for which PKG_BUILD_OPTIONS is defined.
+	seenPkgbase Once
+
 	// During both load() and check(), tells whether bsd.prefs.mk has
 	// already been loaded directly or indirectly.
 	//
@@ -160,6 +167,11 @@ func (pkg *Package) load() ([]CurrPath, *MkLines, *MkLines) {
 		if isRelevantMk(filename, basename) {
 			fragmentMklines := LoadMk(filename, pkg, MustSucceed)
 			pkg.collectConditionalIncludes(fragmentMklines)
+			fragmentMklines.ForEach(func(mkline *MkLine) {
+				if mkline.IsVarassign() && mkline.Varname() == "pkgbase" {
+					pkg.seenPkgbase.FirstTime(mkline.Value())
+				}
+			})
 		}
 		if hasPrefix(basename, "PLIST") {
 			pkg.loadPlistDirs(filename)
@@ -311,6 +323,10 @@ func (pkg *Package) parseLine(mklines *MkLines, mkline *MkLine, allLines *MkLine
 			}
 			pkg.vars.Define(varname, mkline)
 		}
+
+		if varname == "pkgbase" {
+			pkg.seenPkgbase.FirstTime(mkline.Value())
+		}
 	}
 	return true
 }
diff --git a/pkgtools/pkglint/files/package_test.go b/pkgtools/pkglint/files/package_test.go
index 1b61b598290..0bc06a815d3 100644
--- a/pkgtools/pkglint/files/package_test.go
+++ b/pkgtools/pkglint/files/package_test.go
@@ -320,6 +320,46 @@ func (s *Suite) Test_Package__case_insensitive(c *check.C) {
 	t.CheckOutputEmpty()
 }
 
+// This package has several identifiers that all differ:
+//  - it lives in the directory "package"
+//  - the package name is "pkgname"
+//  - it downloads "distname-1.0.tar.gz"
+//    (in some places the distname is used as the package name)
+//  - in options.mk its name is "optid"
+//  - in buildlink3.mk its name is "bl3id"
+// All these identifiers should ideally be the same.
+// For historic reasons, the package directory and the package name
+// may differ.
+func (s *Suite) Test_Package__different_package_identifiers(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpPackage("category/package",
+		"DISTNAME=\tdistname-1.0",
+		"PKGNAME=\tpkgname-1.0")
+	t.CreateFileLines("mk/bsd.options.mk")
+	t.CreateFileLines("category/package/options.mk",
+		MkCvsID,
+		"",
+		"PKG_OPTIONS_VAR=\tPKG_OPTIONS.optid",
+		"PKG_SUPPORTED_OPTIONS=\t# none",
+		"",
+		".include \"../../mk/bsd.options.mk\"",
+		"",
+		"# Nothing to do here")
+	t.CreateFileBuildlink3Id("category/package/buildlink3.mk", "bl3id")
+	t.Chdir("category/package")
+	t.FinishSetUp()
+
+	G.checkdirPackage(".")
+
+	t.CheckOutputLines(
+		"ERROR: buildlink3.mk:3: Package name mismatch "+
+			"between \"bl3id\" in this file and \"pkgname\" from Makefile:4.",
+		"WARN: options.mk:3: The buildlink3 identifier \"bl3id\" "+
+			"should be the same as the options identifier \"optid\".")
+
+}
+
 func (s *Suite) Test_NewPackage(c *check.C) {
 	t := s.Init(c)
 
diff --git a/pkgtools/pkglint/files/pkglint.go b/pkgtools/pkglint/files/pkglint.go
index 579e50498f2..858d1a9bb07 100644
--- a/pkgtools/pkglint/files/pkglint.go
+++ b/pkgtools/pkglint/files/pkglint.go
@@ -571,7 +571,11 @@ func (p *Pkglint) checkReg(filename CurrPath, basename string, depth int, pkg *P
 
 	case basename == "buildlink3.mk":
 		if mklines := LoadMk(filename, pkg, NotEmpty|LogErrors); mklines != nil {
-			CheckLinesBuildlink3Mk(mklines)
+			ck := NewBuildlink3Checker(mklines)
+			ck.Check()
+			if pkg != nil {
+				pkg.buildlinkID = ck.pkgbase
+			}
 		}
 
 	case hasPrefix(basename, "DESCR"):
@@ -594,7 +598,11 @@ func (p *Pkglint) checkReg(filename CurrPath, basename string, depth int, pkg *P
 
 	case basename == "options.mk":
 		if mklines := LoadMk(filename, pkg, NotEmpty|LogErrors); mklines != nil {
-			CheckLinesOptionsMk(mklines)
+			buildlinkID := ""
+			if pkg != nil {
+				buildlinkID = pkg.buildlinkID
+			}
+			CheckLinesOptionsMk(mklines, buildlinkID)
 		}
 
 	case matches(basename, `^patch-[-\w.~+]*\w$`):
diff --git a/pkgtools/pkglint/files/util_test.go b/pkgtools/pkglint/files/util_test.go
index b8f9358c027..5070d495b60 100644
--- a/pkgtools/pkglint/files/util_test.go
+++ b/pkgtools/pkglint/files/util_test.go
@@ -304,6 +304,12 @@ func (s *Suite) Test_alignWith(c *check.C) {
 	test("1234567890=", "V=", "1234567890=")
 }
 
+func (s *Suite) Test_alignmentToWidths(c *check.C) {
+	t := s.Init(c)
+
+	t.CheckEquals(alignmentToWidths(8, 72), "\t\t\t\t\t\t\t\t")
+}
+
 func (s *Suite) Test_indent(c *check.C) {
 	t := s.Init(c)
 
diff --git a/pkgtools/pkglint/files/varalignblock.go b/pkgtools/pkglint/files/varalignblock.go
index c891072be28..68e77b9a674 100644
--- a/pkgtools/pkglint/files/varalignblock.go
+++ b/pkgtools/pkglint/files/varalignblock.go
@@ -656,9 +656,6 @@ func (info *varalignLine) alignContinuation(valueColumn, rightMarginColumn int)
 		fix.Notef("The continuation backslash should be preceded by a single space.")
 	} else {
 		newSpace = alignmentToWidths(info.uptoValueWidth(), rightMarginColumn)
-		if newSpace == "" {
-			newSpace = " "
-		}
 		fix.Notef(
 			"The continuation backslash should be in column %d, not %d.",
 			rightMarginColumn+1, column+1)
@@ -824,7 +821,7 @@ func (p *varalignParts) uptoValueWidth() int {
 	if p.value != "" {
 		return p.spaceAfterValueColumn()
 	} else {
-		return p.varnameOpColumn()
+		return p.spaceBeforeValueColumn()
 	}
 }
 
diff --git a/pkgtools/pkglint/files/varalignblock_test.go b/pkgtools/pkglint/files/varalignblock_test.go
index d4d15e6d6cf..32576372361 100644
--- a/pkgtools/pkglint/files/varalignblock_test.go
+++ b/pkgtools/pkglint/files/varalignblock_test.go
@@ -2063,6 +2063,30 @@ func (s *Suite) Test_VaralignBlock__var_tab_value63_space_cont_tab8_value71_spac
 	vt.Run()
 }
 
+func (s *Suite) Test_VaralignBlock__var8_tabs80_cont_tab_value_tabs72_cont_tab_value_tabs72_cont_tab_value9(c *check.C) {
+	vt := NewVaralignTester(s, c)
+	vt.Input(
+		"VAR...7=\t\t\t\t\t\t\t\t\t\\",
+		"\t.\t\t\t\t\t\t\t\t\\",
+		"\t.\t\t\t\t\t\t\t\t\\",
+		"\t.")
+	vt.InputDetab(
+		"VAR...7=                                                                        \\",
+		"        .                                                               \\",
+		"        .                                                               \\",
+		"        .")
+	vt.Diagnostics(
+		"NOTE: Makefile:1: The continuation backslash should be in column 73, not 81.")
+	vt.Autofixes(
+		"AUTOFIX: Makefile:1: Replacing \"\\t\\t\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\\t\\t\\t\".")
+	vt.Fixed(
+		"VAR...7=                                                                \\",
+		"        .                                                               \\",
+		"        .                                                               \\",
+		"        .")
+	vt.Run()
+}
+
 // Up to 2020-01-27, pkglint removed all spaces before the backslash,
 // which was against the rule of having at least one space.
 func (s *Suite) Test_VaralignBlock__right_margin(c *check.C) {