pkgtools/pkglint: hotfix for release 5.5.15

* Fixed detection of redundant variable definitions.
* Fixed check for PATH environment variable.

6 files changed, 74 insertions, 7 deletions

diff --git a/pkgtools/pkglint/files/mkline.go b/pkgtools/pkglint/files/mkline.go
index 0cce30af516..1814b65906c 100644
--- a/pkgtools/pkglint/files/mkline.go
+++ b/pkgtools/pkglint/files/mkline.go
@@ -276,10 +276,33 @@ func (mkline *MkLineImpl) Tokenize(s string) []*MkToken {
 	return tokens
 }
 
+// ValueSplit splits the variable value of an assignment line,
+// taking care of variable references. For example, when the value
+// "/bin:${PATH:S,::,::,}" is split at ":", it results in
+// {"/bin", "${PATH:S,::,::,}"}.
+func (mkline *MkLineImpl) ValueSplit(value string, separator string) []string {
+	tokens := mkline.Tokenize(value)
+	var split []string
+	for _, token := range tokens {
+		if split == nil {
+			split = []string{""}
+		}
+		if token.Varuse == nil && contains(token.Text, separator) {
+			subs := strings.Split(token.Text, separator)
+			split[len(split)-1] += subs[0]
+			split = append(split, subs[1:]...)
+		} else {
+			split[len(split)-1] += token.Text
+		}
+	}
+	return split
+}
+
 func (mkline *MkLineImpl) WithoutMakeVariables(value string) string {
 	valueNovar := value
 	for {
 		var m []string
+		// TODO: properly parse nested variables
 		m, valueNovar = regex.ReplaceFirst(valueNovar, `\$\{[^{}]*\}`, "")
 		if m == nil {
 			return valueNovar
diff --git a/pkgtools/pkglint/files/mkline_test.go b/pkgtools/pkglint/files/mkline_test.go
index 1c2d285bec3..fca2b66f201 100644
--- a/pkgtools/pkglint/files/mkline_test.go
+++ b/pkgtools/pkglint/files/mkline_test.go
@@ -802,6 +802,35 @@ func (s *Suite) Test_MkLine_ConditionVars(c *check.C) {
 	c.Check(mkline.ConditionVars(), equals, "OPSYS")
 }
 
+func (s *Suite) Test_MkLine_ValueSplit(c *check.C) {
+	t := s.Init(c)
+
+	checkSplit := func(value string, expected ...string) {
+		mkline := t.NewMkLine("Makefile", 1, "PATH=\t"+value)
+		split := mkline.ValueSplit(value, ":")
+		c.Check(split, deepEquals, expected)
+	}
+
+	checkSplit("#empty",
+		[]string(nil)...)
+
+	checkSplit("/bin",
+		"/bin")
+
+	checkSplit("/bin:/sbin",
+		"/bin",
+		"/sbin")
+
+	checkSplit("${DESTDIR}/bin:/bin/${SUBDIR}",
+		"${DESTDIR}/bin",
+		"/bin/${SUBDIR}")
+
+	checkSplit("/bin:${DESTDIR}${PREFIX}:${DESTDIR:S,/,\\:,:S,:,:,}/sbin",
+		"/bin",
+		"${DESTDIR}${PREFIX}",
+		"${DESTDIR:S,/,\\:,:S,:,:,}/sbin")
+}
+
 func (s *Suite) Test_MatchVarassign(c *check.C) {
 	s.Init(c)
 
diff --git a/pkgtools/pkglint/files/mklines.go b/pkgtools/pkglint/files/mklines.go
index 179698edf42..f2e599e4ed3 100644
--- a/pkgtools/pkglint/files/mklines.go
+++ b/pkgtools/pkglint/files/mklines.go
@@ -312,7 +312,7 @@ func (mklines *MkLines) CheckRedundantVariables() {
 		return true
 	}
 	scope.OnIgnore = func(old, new MkLine) {
-		if isRelevant(old, new) {
+		if isRelevant(old, new) && old.Value() == new.Value() {
 			old.Notef("Definition of %s is redundant because of %s.", new.Varname(), new.ReferenceFrom(old.Line))
 		}
 	}
diff --git a/pkgtools/pkglint/files/mklines_test.go b/pkgtools/pkglint/files/mklines_test.go
index 6a5a2603ca1..6d830144bc6 100644
--- a/pkgtools/pkglint/files/mklines_test.go
+++ b/pkgtools/pkglint/files/mklines_test.go
@@ -588,6 +588,17 @@ func (s *Suite) Test_MkLines_CheckRedundantVariables(c *check.C) {
 		"WARN: module.mk:1: Variable VAR is overwritten in line 3.")
 }
 
+func (s *Suite) Test_MkLines_CheckRedundantVariables__different_value(c *check.C) {
+	t := s.Init(c)
+	mklines := t.NewMkLines("module.mk",
+		"VAR=\tvalue ${OTHER}",
+		"VAR?=\tdifferent value")
+
+	mklines.CheckRedundantVariables()
+
+	t.CheckOutputEmpty()
+}
+
 func (s *Suite) Test_MkLines_CheckRedundantVariables__procedure_call(c *check.C) {
 	t := s.Init(c)
 	mklines := t.NewMkLines("mk/pthread.buildlink3.mk",
diff --git a/pkgtools/pkglint/files/vartypecheck.go b/pkgtools/pkglint/files/vartypecheck.go
index 05b4f5daf57..40f4d81cd09 100644
--- a/pkgtools/pkglint/files/vartypecheck.go
+++ b/pkgtools/pkglint/files/vartypecheck.go
@@ -669,19 +669,22 @@ func (cv *VartypeCheck) Option() {
 	line.Errorf("Invalid option name %q. Option names must start with a lowercase letter and be all-lowercase.", value)
 }
 
-// The PATH environment variable
+// Pathlist checks variables like the PATH environment variable.
 func (cv *VartypeCheck) Pathlist() {
+	// Sometimes, variables called PATH contain a single pathname,
+	// especially those with auto-guessed type from MkLineImpl.VariableType.
 	if !contains(cv.Value, ":") && cv.Guessed {
 		MkLineChecker{cv.MkLine}.CheckVartypePrimitive(cv.Varname, BtPathname, cv.Op, cv.Value, cv.MkComment, cv.Guessed)
 		return
 	}
 
-	for _, path := range strings.Split(cv.Value, ":") {
-		if contains(path, "${") {
+	for _, path := range cv.MkLine.ValueSplit(cv.Value, ":") {
+		if hasPrefix(path, "${") {
 			continue
 		}
 
-		if !matches(path, `^[-0-9A-Za-z._~+%/]*$`) {
+		pathNoVar := cv.MkLine.WithoutMakeVariables(path)
+		if !matches(pathNoVar, `^[-0-9A-Za-z._~+%/]*$`) {
 			cv.Line.Warnf("%q is not a valid pathname.", path)
 		}
 
diff --git a/pkgtools/pkglint/files/vartypecheck_test.go b/pkgtools/pkglint/files/vartypecheck_test.go
index cfa20db2daa..cfa0160d793 100644
--- a/pkgtools/pkglint/files/vartypecheck_test.go
+++ b/pkgtools/pkglint/files/vartypecheck_test.go
@@ -394,10 +394,11 @@ func (s *Suite) Test_VartypeCheck_Pathlist(c *check.C) {
 	t := s.Init(c)
 
 	runVartypeChecks(t, "PATH", opAssign, (*VartypeCheck).Pathlist,
-		"/usr/bin:/usr/sbin:.:${LOCALBASE}/bin")
+		"/usr/bin:/usr/sbin:.::${LOCALBASE}/bin:${HOMEPAGE:S,https://,,}")
 
 	t.CheckOutputLines(
-		"WARN: fname:1: All components of PATH (in this case \".\") should be absolute paths.")
+		"WARN: fname:1: All components of PATH (in this case \".\") should be absolute paths.",
+		"WARN: fname:1: All components of PATH (in this case \"\") should be absolute paths.")
 }
 
 func (s *Suite) Test_VartypeCheck_Perms(c *check.C) {