CVS commit: pkgsrc/pkgtools/pkglint/files

["Roland Illig" <rillig%netbsd.org@localhost>]

Module Name:    pkgsrc
Committed By:   rillig
Date:           Sat Jul 28 20:44:45 UTC 2018

Modified Files:
        pkgsrc/pkgtools/pkglint/files: mkline.go mkline_test.go mklines.go
            mklines_test.go vartypecheck.go vartypecheck_test.go

Log Message:
pkgtools/pkglint: hotfix for release 5.5.15

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


To generate a diff of this commit:
cvs rdiff -u -r1.33 -r1.34 pkgsrc/pkgtools/pkglint/files/mkline.go
cvs rdiff -u -r1.36 -r1.37 pkgsrc/pkgtools/pkglint/files/mkline_test.go
cvs rdiff -u -r1.27 -r1.28 pkgsrc/pkgtools/pkglint/files/mklines.go \
    pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
cvs rdiff -u -r1.22 -r1.23 pkgsrc/pkgtools/pkglint/files/mklines_test.go
cvs rdiff -u -r1.35 -r1.36 pkgsrc/pkgtools/pkglint/files/vartypecheck.go

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: pkgsrc/pkgtools/pkglint/files/mkline.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline.go:1.33 pkgsrc/pkgtools/pkglint/files/mkline.go:1.34
--- pkgsrc/pkgtools/pkglint/files/mkline.go:1.33        Sat Jul 28 18:31:23 2018
+++ pkgsrc/pkgtools/pkglint/files/mkline.go     Sat Jul 28 20:44:45 2018
@@ -276,10 +276,33 @@ func (mkline *MkLineImpl) Tokenize(s str
        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

Index: pkgsrc/pkgtools/pkglint/files/mkline_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.36 pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.37
--- pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.36   Sat Jul 28 18:31:23 2018
+++ pkgsrc/pkgtools/pkglint/files/mkline_test.go        Sat Jul 28 20:44:45 2018
@@ -802,6 +802,35 @@ func (s *Suite) Test_MkLine_ConditionVar
        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)
 

Index: pkgsrc/pkgtools/pkglint/files/mklines.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines.go:1.27 pkgsrc/pkgtools/pkglint/files/mklines.go:1.28
--- pkgsrc/pkgtools/pkglint/files/mklines.go:1.27       Sat Jul 28 18:31:23 2018
+++ pkgsrc/pkgtools/pkglint/files/mklines.go    Sat Jul 28 20:44:45 2018
@@ -312,7 +312,7 @@ func (mklines *MkLines) CheckRedundantVa
                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))
                }
        }
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.27 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.28
--- pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.27     Sat Jul 28 18:31:23 2018
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go  Sat Jul 28 20:44:45 2018
@@ -394,10 +394,11 @@ func (s *Suite) Test_VartypeCheck_Pathli
        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) {

Index: pkgsrc/pkgtools/pkglint/files/mklines_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.22 pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.23
--- pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.22  Sat Jul 28 18:31:23 2018
+++ pkgsrc/pkgtools/pkglint/files/mklines_test.go       Sat Jul 28 20:44:45 2018
@@ -588,6 +588,17 @@ func (s *Suite) Test_MkLines_CheckRedund
                "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",

Index: pkgsrc/pkgtools/pkglint/files/vartypecheck.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.35 pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.36
--- pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.35  Sat Jul 28 18:31:23 2018
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck.go       Sat Jul 28 20:44:45 2018
@@ -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)
                }