Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[pkgsrc/trunk]: pkgsrc/pkgtools/pkglint pkgtools/pkglint: update to 20.1.3
details: https://anonhg.NetBSD.org/pkgsrc/rev/add24c47560f
branches: trunk
changeset: 430414:add24c47560f
user: rillig <rillig%pkgsrc.org@localhost>
date: Thu Apr 30 21:15:03 2020 +0000
description:
pkgtools/pkglint: update to 20.1.3
Changes since 20.1.2:
Stricter check for Python version numbers. Before, 25 and 26 had not
been marked as wrong.
In assignments like PKGNAME=${DISTNAME:S,from,to,}, modifiers that don't
have any effect generate a note. Most of these modifiers are redundant
or outdated.
Patches must not add well-known absolute paths like /usr/pkg, /var and
/etc since these must be overridable by the pkgsrc user. Other absolute
paths continue to be allowed.
diffstat:
pkgtools/pkglint/Makefile | 4 +-
pkgtools/pkglint/files/homepage_test.go | 3 +-
pkgtools/pkglint/files/mkassignchecker_test.go | 20 +-
pkgtools/pkglint/files/mklinechecker.go | 2 +-
pkgtools/pkglint/files/mklinechecker_test.go | 2 +-
pkgtools/pkglint/files/mktypes.go | 10 +-
pkgtools/pkglint/files/mktypes_test.go | 34 ++--
pkgtools/pkglint/files/mkvarusechecker_test.go | 4 +-
pkgtools/pkglint/files/package.go | 11 +-
pkgtools/pkglint/files/package_test.go | 45 ++++++-
pkgtools/pkglint/files/patches.go | 87 ++++++++++++-
pkgtools/pkglint/files/patches_test.go | 162 +++++++++++++++++++++++++
pkgtools/pkglint/files/pkglint.go | 6 +
pkgtools/pkglint/files/pkgver/vercmp.go | 23 +-
pkgtools/pkglint/files/plist.go | 7 +-
pkgtools/pkglint/files/regex/regex.go | 14 --
pkgtools/pkglint/files/util.go | 10 +-
pkgtools/pkglint/files/vardefs.go | 12 +-
pkgtools/pkglint/files/vartypecheck.go | 4 +-
19 files changed, 366 insertions(+), 94 deletions(-)
diffs (truncated from 824 to 300 lines):
diff -r 68b5d86f0381 -r add24c47560f pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Thu Apr 30 20:35:19 2020 +0000
+++ b/pkgtools/pkglint/Makefile Thu Apr 30 21:15:03 2020 +0000
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.641 2020/04/13 19:46:44 rillig Exp $
+# $NetBSD: Makefile,v 1.642 2020/04/30 21:15:03 rillig Exp $
-PKGNAME= pkglint-20.1.2
+PKGNAME= pkglint-20.1.3
CATEGORIES= pkgtools
DISTNAME= tools
MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/}
diff -r 68b5d86f0381 -r add24c47560f pkgtools/pkglint/files/homepage_test.go
--- a/pkgtools/pkglint/files/homepage_test.go Thu Apr 30 20:35:19 2020 +0000
+++ b/pkgtools/pkglint/files/homepage_test.go Thu Apr 30 21:15:03 2020 +0000
@@ -393,7 +393,8 @@
"https://no-such-name.example.org/";,
// The "unknown network error" is for compatibility with Go < 1.13.
`^WARN: filename\.mk:1: Homepage "https://no-such-name.example.org/"; `+
- `cannot be checked: (name not found|timeout|unknown network error:.*)$`)
+ `cannot be checked: `+
+ `(name not found|timeout|connection refused|unknown network error:.*)$`)
// Syntactically invalid URLs are silently skipped since VartypeCheck.URL
// already warns about them.
diff -r 68b5d86f0381 -r add24c47560f pkgtools/pkglint/files/mkassignchecker_test.go
--- a/pkgtools/pkglint/files/mkassignchecker_test.go Thu Apr 30 20:35:19 2020 +0000
+++ b/pkgtools/pkglint/files/mkassignchecker_test.go Thu Apr 30 21:15:03 2020 +0000
@@ -995,23 +995,25 @@
mklines.Check()
- // Half of these warnings are from VartypeCheck.Version, the
- // other half are from checkVarassignDecreasingVersions.
- // Strictly speaking some of them are redundant, but that would
- // mean to reject only variable references in checkVarassignDecreasingVersions.
- // This is probably ok.
- // TODO: Fix this.
+ // Half of these warnings are from VartypeCheck.Enum,
+ // the other half are from checkVarassignDecreasingVersions.
+ // Strictly speaking some of them are redundant, but that's ok.
+ // They all need to be fixed in the end.
t.CheckOutputLines(
- "WARN: Makefile:2: Invalid version number \"__future__\".",
+ "WARN: Makefile:2: \"__future__\" is not valid for PYTHON_VERSIONS_ACCEPTED. "+
+ "Use one of { 27 36 37 38 } instead.",
"ERROR: Makefile:2: Value \"__future__\" for "+
"PYTHON_VERSIONS_ACCEPTED must be a positive integer.",
- "WARN: Makefile:3: Invalid version number \"-13\".",
+ "WARN: Makefile:3: \"-13\" is not valid for PYTHON_VERSIONS_ACCEPTED. "+
+ "Use one of { 27 36 37 38 } instead.",
"ERROR: Makefile:3: Value \"-13\" for "+
"PYTHON_VERSIONS_ACCEPTED must be a positive integer.",
"ERROR: Makefile:4: Value \"${PKGVERSION_NOREV}\" for "+
"PYTHON_VERSIONS_ACCEPTED must be a positive integer.",
"WARN: Makefile:5: The values for PYTHON_VERSIONS_ACCEPTED "+
- "should be in decreasing order (37 before 36).")
+ "should be in decreasing order (37 before 36).",
+ "WARN: Makefile:6: \"25\" is not valid for PYTHON_VERSIONS_ACCEPTED. "+
+ "Use one of { 27 36 37 38 } instead.")
}
func (s *Suite) Test_MkAssignChecker_checkVarassignMiscRedundantInstallationDirs__AUTO_MKDIRS_yes(c *check.C) {
diff -r 68b5d86f0381 -r add24c47560f pkgtools/pkglint/files/mklinechecker.go
--- a/pkgtools/pkglint/files/mklinechecker.go Thu Apr 30 20:35:19 2020 +0000
+++ b/pkgtools/pkglint/files/mklinechecker.go Thu Apr 30 21:15:03 2020 +0000
@@ -98,7 +98,7 @@
"",
"Example:",
"",
- "\tWRKSRC=\t${WRKDIR}",
+ "\tWRKSRC=\t\t${WRKDIR}",
"\tCONFIGURE_DIRS=\t${WRKSRC}/lib ${WRKSRC}/src",
"\tBUILD_DIRS=\t${WRKSRC}/lib ${WRKSRC}/src ${WRKSRC}/cmd",
"",
diff -r 68b5d86f0381 -r add24c47560f pkgtools/pkglint/files/mklinechecker_test.go
--- a/pkgtools/pkglint/files/mklinechecker_test.go Thu Apr 30 20:35:19 2020 +0000
+++ b/pkgtools/pkglint/files/mklinechecker_test.go Thu Apr 30 21:15:03 2020 +0000
@@ -161,7 +161,7 @@
"",
"\tExample:",
"",
- "\t\tWRKSRC=\t${WRKDIR}",
+ "\t\tWRKSRC=\t\t${WRKDIR}",
"\t\tCONFIGURE_DIRS=\t${WRKSRC}/lib ${WRKSRC}/src",
"\t\tBUILD_DIRS=\t${WRKSRC}/lib ${WRKSRC}/src ${WRKSRC}/cmd",
"",
diff -r 68b5d86f0381 -r add24c47560f pkgtools/pkglint/files/mktypes.go
--- a/pkgtools/pkglint/files/mktypes.go Thu Apr 30 20:35:19 2020 +0000
+++ b/pkgtools/pkglint/files/mktypes.go Thu Apr 30 21:15:03 2020 +0000
@@ -40,7 +40,7 @@
func (vu *MkVarUse) String() string { return sprintf("${%s%s}", vu.varname, vu.Mod()) }
type MkVarUseModifier struct {
- Text string
+ Text string // The text of the modifier, without the initial colon.
}
func (m MkVarUseModifier) IsQ() bool { return m.Text == "Q" }
@@ -59,13 +59,13 @@
//
// Example:
// MkVarUseModifier{"S,name,file,g"}.Subst("distname-1.0") => "distfile-1.0"
-func (m MkVarUseModifier) Subst(str string) (string, bool) {
+func (m MkVarUseModifier) Subst(str string) (bool, string) {
// XXX: The call to MatchSubst is usually redundant because MatchSubst
// is typically called directly before calling Subst.
// This comes from a time when there was no boolean return value.
ok, isRegex, from, to, options := m.MatchSubst()
if !ok {
- return "", false
+ return false, ""
}
leftAnchor := hasPrefix(from, "^")
@@ -86,14 +86,14 @@
if isRegex {
// XXX: Maybe implement regular expression substitutions later.
- return "", false
+ return false, ""
}
ok, result := m.EvalSubst(str, leftAnchor, from, rightAnchor, to, options)
if trace.Tracing && ok && result != str {
trace.Stepf("Subst: %q %q => %q", str, m.Text, result)
}
- return result, ok
+ return ok, result
}
// mkopSubst evaluates make(1)'s :S substitution operator.
diff -r 68b5d86f0381 -r add24c47560f pkgtools/pkglint/files/mktypes_test.go
--- a/pkgtools/pkglint/files/mktypes_test.go Thu Apr 30 20:35:19 2020 +0000
+++ b/pkgtools/pkglint/files/mktypes_test.go Thu Apr 30 21:15:03 2020 +0000
@@ -90,60 +90,60 @@
func (s *Suite) Test_MkVarUseModifier_Subst(c *check.C) {
t := s.Init(c)
- test := func(mod, str, result string, ok bool) {
+ test := func(mod, str string, ok bool, result string) {
m := MkVarUseModifier{mod}
- actualResult, actualOk := m.Subst(str)
+ actualOk, actualResult := m.Subst(str)
t.CheckDeepEquals(
- []interface{}{actualResult, actualOk},
- []interface{}{result, ok})
+ []interface{}{actualOk, actualResult},
+ []interface{}{ok, result})
}
- test("???", "anything", "", false)
+ test("???", "anything", false, "")
- test("S,from,to,", "from", "to", true)
+ test("S,from,to,", "from", true, "to")
- test("C,from,to,", "from", "to", true)
+ test("C,from,to,", "from", true, "to")
- test("C,syntax error", "anything", "", false)
+ test("C,syntax error", "anything", false, "")
// The substitution modifier does not match, therefore
// the value is returned unmodified, but successful.
- test("C,no_match,replacement,", "value", "value", true)
+ test("C,no_match,replacement,", "value", true, "value")
// As of December 2019, pkglint doesn't know how to handle
// complicated :C modifiers.
- test("C,.*,,", "anything", "", false)
+ test("C,.*,,", "anything", false, "")
// When given a modifier that is not actually a :S or :C, Subst
// doesn't do anything.
- test("Mpattern", "anything", "", false)
+ test("Mpattern", "anything", false, "")
- test("S,from,to,", "from a to b", "to a to b", true)
+ test("S,from,to,", "from a to b", true, "to a to b")
// Since the replacement text is not a simple string, the :C modifier
// cannot be treated like the :S modifier. The variable could contain
// one of the special characters that would need to be escaped in the
// replacement text.
- test("C,from,${VAR},", "from a to b", "", false)
+ test("C,from,${VAR},", "from a to b", false, "")
// As of December 2019, nothing is substituted. If pkglint should ever
// handle variables in the modifier, this test would need to provide a
// context in which to resolve the variables. If that happens, the
// .TARGET variable needs to be set to "target".
- test("S/$@/replaced/", "The target", "The target", true)
- test("S,${PREFIX},/prefix,", "${PREFIX}/dir", "", false)
+ test("S/$@/replaced/", "The target", true, "The target")
+ test("S,${PREFIX},/prefix,", "${PREFIX}/dir", false, "")
// Just for code coverage.
t.DisableTracing()
- test("S,long,long long,g", "A long story", "A long long story", true)
+ test("S,long,long long,g", "A long story", true, "A long long story")
t.EnableTracing()
// And now again with full tracing, to investigate cases where
// pkglint produces results that are not easily understandable.
t.EnableTracingToLog()
- test("S,long,long long,g", "A long story", "A long long story", true)
+ test("S,long,long long,g", "A long story", true, "A long long story")
t.EnableTracing()
t.CheckOutputLines(
"TRACE: Subst: \"A long story\" " +
diff -r 68b5d86f0381 -r add24c47560f pkgtools/pkglint/files/mkvarusechecker_test.go
--- a/pkgtools/pkglint/files/mkvarusechecker_test.go Thu Apr 30 20:35:19 2020 +0000
+++ b/pkgtools/pkglint/files/mkvarusechecker_test.go Thu Apr 30 21:15:03 2020 +0000
@@ -592,13 +592,13 @@
t.SetUpVartypes()
mklines := t.NewMkLines("file.mk",
MkCvsID,
- "IGNORE_PKG.package=\t${ONLY_FOR_UNPRIVILEGED}")
+ "IGNORE_PKG.package=\t${NOT_FOR_UNPRIVILEGED}")
mklines.Check()
t.CheckOutputLines(
"WARN: file.mk:2: IGNORE_PKG.package should be set to YES or yes.",
- "WARN: file.mk:2: ONLY_FOR_UNPRIVILEGED should not be used indirectly at load time (via IGNORE_PKG.package).")
+ "WARN: file.mk:2: NOT_FOR_UNPRIVILEGED should not be used indirectly at load time (via IGNORE_PKG.package).")
}
// This test is only here for branch coverage.
diff -r 68b5d86f0381 -r add24c47560f pkgtools/pkglint/files/package.go
--- a/pkgtools/pkglint/files/package.go Thu Apr 30 20:35:19 2020 +0000
+++ b/pkgtools/pkglint/files/package.go Thu Apr 30 21:15:03 2020 +0000
@@ -275,7 +275,6 @@
return mainLines, allLines
}
-// 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 CurrPath, main bool) bool {
if trace.Tracing {
defer trace.Call(mklines.lines.Filename)()
@@ -891,7 +890,6 @@
{"NOT_FOR_COMPILER", many},
{"ONLY_FOR_COMPILER", many},
{"NOT_FOR_UNPRIVILEGED", optional},
- {"ONLY_FOR_UNPRIVILEGED", optional},
emptyLine,
{"BUILD_DEPENDS", many},
{"TOOL_DEPENDS", many},
@@ -1147,7 +1145,7 @@
effname := pkgname
if distname != "" && effname != "" {
- merged, ok := pkg.pkgnameFromDistname(effname, distname)
+ merged, ok := pkg.pkgnameFromDistname(effname, distname, pkgnameLine)
if ok {
effname = merged
}
@@ -1209,7 +1207,7 @@
return ""
}
-func (pkg *Package) pkgnameFromDistname(pkgname, distname string) (string, bool) {
+func (pkg *Package) pkgnameFromDistname(pkgname, distname string, diag Diagnoser) (string, bool) {
tokens, rest := NewMkLexer(pkgname, nil).MkTokens()
if rest != "" {
return "", false
@@ -1228,7 +1226,10 @@
for _, mod := range token.Varuse.modifiers {
if mod.IsToLower() {
newDistname = strings.ToLower(newDistname)
- } else if subst, ok := mod.Subst(newDistname); ok {
+ } else if ok, subst := mod.Subst(newDistname); ok {
+ if subst == newDistname && !containsVarUse(subst) {
+ diag.Notef("The modifier :%s does not have an effect.", mod.Text)
+ }
newDistname = subst
} else {
return "", false
diff -r 68b5d86f0381 -r add24c47560f pkgtools/pkglint/files/package_test.go
--- a/pkgtools/pkglint/files/package_test.go Thu Apr 30 20:35:19 2020 +0000
+++ b/pkgtools/pkglint/files/package_test.go Thu Apr 30 21:15:03 2020 +0000
@@ -2609,6 +2609,43 @@
pkg.check(files, mklines, allLines)
t.CheckEquals(pkg.EffectivePkgname, "distname-1.0")
+ t.CheckOutputLines(
+ "NOTE: ~/category/package/Makefile:4: " +
+ "The modifier :C:does_not_match:replacement: does not have an effect.")
+}
+
+func (s *Suite) Test_Package_determineEffectivePkgVars__ineffective_S_modifier_with_variable(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package",
+ "VERSION=\t1.008",
+ "DISTNAME=\tdistname-v${VERSION}",
+ "PKGNAME=\t${DISTNAME:S/v1/1/}")
Home |
Main Index |
Thread Index |
Old Index