pkgsrc-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[pkgsrc/trunk]: pkgsrc/pkgtools/pkglint/files pkgtools/pkglint: hotfix for re...
details: https://anonhg.NetBSD.org/pkgsrc/rev/cc3d2bc7b864
branches: trunk
changeset: 310977:cc3d2bc7b864
user: rillig <rillig%pkgsrc.org@localhost>
date: Sat Jul 28 20:44:45 2018 +0000
description:
pkgtools/pkglint: hotfix for release 5.5.15
* Fixed detection of redundant variable definitions.
* Fixed check for PATH environment variable.
diffstat:
pkgtools/pkglint/files/mkline.go | 23 +++++++++++++++++++++++
pkgtools/pkglint/files/mkline_test.go | 29 +++++++++++++++++++++++++++++
pkgtools/pkglint/files/mklines.go | 2 +-
pkgtools/pkglint/files/mklines_test.go | 11 +++++++++++
pkgtools/pkglint/files/vartypecheck.go | 11 +++++++----
pkgtools/pkglint/files/vartypecheck_test.go | 5 +++--
6 files changed, 74 insertions(+), 7 deletions(-)
diffs (156 lines):
diff -r d2087937a5b7 -r cc3d2bc7b864 pkgtools/pkglint/files/mkline.go
--- a/pkgtools/pkglint/files/mkline.go Sat Jul 28 18:31:33 2018 +0000
+++ b/pkgtools/pkglint/files/mkline.go Sat Jul 28 20:44:45 2018 +0000
@@ -276,10 +276,33 @@
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 -r d2087937a5b7 -r cc3d2bc7b864 pkgtools/pkglint/files/mkline_test.go
--- a/pkgtools/pkglint/files/mkline_test.go Sat Jul 28 18:31:33 2018 +0000
+++ b/pkgtools/pkglint/files/mkline_test.go Sat Jul 28 20:44:45 2018 +0000
@@ -802,6 +802,35 @@
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 -r d2087937a5b7 -r cc3d2bc7b864 pkgtools/pkglint/files/mklines.go
--- a/pkgtools/pkglint/files/mklines.go Sat Jul 28 18:31:33 2018 +0000
+++ b/pkgtools/pkglint/files/mklines.go Sat Jul 28 20:44:45 2018 +0000
@@ -312,7 +312,7 @@
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 -r d2087937a5b7 -r cc3d2bc7b864 pkgtools/pkglint/files/mklines_test.go
--- a/pkgtools/pkglint/files/mklines_test.go Sat Jul 28 18:31:33 2018 +0000
+++ b/pkgtools/pkglint/files/mklines_test.go Sat Jul 28 20:44:45 2018 +0000
@@ -588,6 +588,17 @@
"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 -r d2087937a5b7 -r cc3d2bc7b864 pkgtools/pkglint/files/vartypecheck.go
--- a/pkgtools/pkglint/files/vartypecheck.go Sat Jul 28 18:31:33 2018 +0000
+++ b/pkgtools/pkglint/files/vartypecheck.go Sat Jul 28 20:44:45 2018 +0000
@@ -669,19 +669,22 @@
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 -r d2087937a5b7 -r cc3d2bc7b864 pkgtools/pkglint/files/vartypecheck_test.go
--- a/pkgtools/pkglint/files/vartypecheck_test.go Sat Jul 28 18:31:33 2018 +0000
+++ b/pkgtools/pkglint/files/vartypecheck_test.go Sat Jul 28 20:44:45 2018 +0000
@@ -394,10 +394,11 @@
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) {
Home |
Main Index |
Thread Index |
Old Index