pkgsrc-Changes archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
CVS commit: pkgsrc/pkgtools/pkglint
Module Name: pkgsrc
Committed By: rillig
Date: Sat Jan 18 21:56:10 UTC 2020
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile
pkgsrc/pkgtools/pkglint/files: buildlink3.go check_test.go licenses.go
mkassignchecker.go mkcondchecker.go mkcondchecker_test.go mkline.go
mkline_test.go mklinechecker.go mklinechecker_test.go
mklines_test.go mkparser.go mkparser_test.go
mkvarusechecker_test.go package.go path.go path_test.go vardefs.go
vartype.go vartypecheck.go vartypecheck_test.go
Log Message:
pkgtools/pkglint: update to 19.4.4
Changes since 19.4.3:
Packages that still use http in their HOMEPAGE URL generate warnings that
the URL should use https instead.
To generate a diff of this commit:
cvs rdiff -u -r1.625 -r1.626 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.30 -r1.31 pkgsrc/pkgtools/pkglint/files/buildlink3.go
cvs rdiff -u -r1.64 -r1.65 pkgsrc/pkgtools/pkglint/files/check_test.go
cvs rdiff -u -r1.31 -r1.32 pkgsrc/pkgtools/pkglint/files/licenses.go
cvs rdiff -u -r1.4 -r1.5 pkgsrc/pkgtools/pkglint/files/mkassignchecker.go \
pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go
cvs rdiff -u -r1.3 -r1.4 pkgsrc/pkgtools/pkglint/files/mkcondchecker.go
cvs rdiff -u -r1.73 -r1.74 pkgsrc/pkgtools/pkglint/files/mkline.go
cvs rdiff -u -r1.78 -r1.79 pkgsrc/pkgtools/pkglint/files/mkline_test.go \
pkgsrc/pkgtools/pkglint/files/package.go \
pkgsrc/pkgtools/pkglint/files/vartypecheck.go
cvs rdiff -u -r1.61 -r1.62 pkgsrc/pkgtools/pkglint/files/mklinechecker.go
cvs rdiff -u -r1.57 -r1.58 \
pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
cvs rdiff -u -r1.59 -r1.60 pkgsrc/pkgtools/pkglint/files/mklines_test.go
cvs rdiff -u -r1.41 -r1.42 pkgsrc/pkgtools/pkglint/files/mkparser.go
cvs rdiff -u -r1.38 -r1.39 pkgsrc/pkgtools/pkglint/files/mkparser_test.go
cvs rdiff -u -r1.5 -r1.6 \
pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go
cvs rdiff -u -r1.7 -r1.8 pkgsrc/pkgtools/pkglint/files/path.go
cvs rdiff -u -r1.8 -r1.9 pkgsrc/pkgtools/pkglint/files/path_test.go
cvs rdiff -u -r1.87 -r1.88 pkgsrc/pkgtools/pkglint/files/vardefs.go
cvs rdiff -u -r1.45 -r1.46 pkgsrc/pkgtools/pkglint/files/vartype.go
cvs rdiff -u -r1.72 -r1.73 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.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/Makefile
diff -u pkgsrc/pkgtools/pkglint/Makefile:1.625 pkgsrc/pkgtools/pkglint/Makefile:1.626
--- pkgsrc/pkgtools/pkglint/Makefile:1.625 Sat Jan 11 15:48:28 2020
+++ pkgsrc/pkgtools/pkglint/Makefile Sat Jan 18 21:56:09 2020
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.625 2020/01/11 15:48:28 rillig Exp $
+# $NetBSD: Makefile,v 1.626 2020/01/18 21:56:09 rillig Exp $
-PKGNAME= pkglint-19.4.3
+PKGNAME= pkglint-19.4.4
CATEGORIES= pkgtools
DISTNAME= tools
MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/}
Index: pkgsrc/pkgtools/pkglint/files/buildlink3.go
diff -u pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.30 pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.31
--- pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.30 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/buildlink3.go Sat Jan 18 21:56:09 2020
@@ -202,7 +202,7 @@ func (ck *Buildlink3Checker) checkVarass
if varname == "BUILDLINK_ABI_DEPENDS."+pkgbase {
ck.abiLine = mkline
parser := NewMkParser(nil, value)
- if dp := parser.Dependency(); dp != nil && parser.EOF() {
+ if dp := parser.DependencyPattern(); dp != nil && parser.EOF() {
ck.abi = dp
}
doCheck = true
@@ -211,7 +211,7 @@ func (ck *Buildlink3Checker) checkVarass
if varname == "BUILDLINK_API_DEPENDS."+pkgbase {
ck.apiLine = mkline
parser := NewMkParser(nil, value)
- if dp := parser.Dependency(); dp != nil && parser.EOF() {
+ if dp := parser.DependencyPattern(); dp != nil && parser.EOF() {
ck.api = dp
}
doCheck = true
Index: pkgsrc/pkgtools/pkglint/files/check_test.go
diff -u pkgsrc/pkgtools/pkglint/files/check_test.go:1.64 pkgsrc/pkgtools/pkglint/files/check_test.go:1.65
--- pkgsrc/pkgtools/pkglint/files/check_test.go:1.64 Mon Jan 6 20:38:42 2020
+++ pkgsrc/pkgtools/pkglint/files/check_test.go Sat Jan 18 21:56:09 2020
@@ -145,6 +145,9 @@ func Test__qa(t *testing.T) {
ck.Configure("vargroups.go", "*", "*", -intqa.EMissingTest) // TODO
ck.Configure("vartype.go", "*", "*", -intqa.EMissingTest) // TODO
+ // Don't require tests for helper methods.
+ ck.Configure("*.go", "VartypeCheck", "[a-z]*", -intqa.EMissingTest)
+
// For now, don't require tests for all the test code.
// Having good coverage for the main code is more important.
ck.Configure("*_test.go", "*", "*", -intqa.EMissingTest)
Index: pkgsrc/pkgtools/pkglint/files/licenses.go
diff -u pkgsrc/pkgtools/pkglint/files/licenses.go:1.31 pkgsrc/pkgtools/pkglint/files/licenses.go:1.32
--- pkgsrc/pkgtools/pkglint/files/licenses.go:1.31 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/licenses.go Sat Jan 18 21:56:09 2020
@@ -28,8 +28,10 @@ func (lc *LicenseChecker) checkName(lice
pkg := lc.MkLines.pkg
if pkg != nil {
if mkline := pkg.vars.FirstDefinition("LICENSE_FILE"); mkline != nil {
- rel := mkline.ResolveVarsInRelativePath(NewRelPathString(mkline.Value()), lc.MkLines.pkg)
- licenseFile = pkg.File(NewPackagePath(rel))
+ // TODO: Not every path is relative to the package directory.
+ rel := NewPackagePathString(mkline.Value())
+ relResolved := mkline.ResolveVarsInRelativePath(rel, lc.MkLines.pkg)
+ licenseFile = pkg.File(relResolved)
}
}
if licenseFile.IsEmpty() {
Index: pkgsrc/pkgtools/pkglint/files/mkassignchecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mkassignchecker.go:1.4 pkgsrc/pkgtools/pkglint/files/mkassignchecker.go:1.5
--- pkgsrc/pkgtools/pkglint/files/mkassignchecker.go:1.4 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/mkassignchecker.go Sat Jan 18 21:56:09 2020
@@ -284,7 +284,7 @@ func (ck *MkAssignChecker) checkVarassig
return
}
- if mkline.HasRationale() {
+ if mkline.Rationale() != "" {
return
}
Index: pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go:1.4 pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go:1.5
--- pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go:1.4 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go Sat Jan 18 21:56:09 2020
@@ -90,6 +90,7 @@ func (s *Suite) Test_MkCondChecker_Check
// Doesn't occur in practice since it is surprising that the ! applies
// to the comparison operator, and not to one of its arguments.
test(".if !${VAR} == value",
+ "WARN: filename.mk:4: The ! should use parentheses or be merged into the comparison operator.",
"WARN: filename.mk:4: VAR is used but not defined.")
// Doesn't occur in practice since this string can never be empty.
@@ -215,6 +216,27 @@ func (s *Suite) Test_MkCondChecker_Check
"ERROR: Makefile:6: Use ${PKGSRC_COMPILER:Ngcc} instead of the != operator.")
}
+func (s *Suite) Test_MkCondChecker_checkNotEmpty(c *check.C) {
+ t := s.Init(c)
+
+ G.Experimental = true
+
+ test := func(cond string, diagnostics ...string) {
+ mklines := t.NewMkLines("filename.mk",
+ ".if "+cond)
+ mkline := mklines.mklines[0]
+ ck := NewMkCondChecker(mkline, mklines)
+
+ ck.checkNotEmpty(mkline.Cond().Not)
+
+ t.CheckOutput(diagnostics)
+ }
+
+ test("!empty(VAR)",
+ // FIXME: Add a :U modifier if VAR might be undefined.
+ "NOTE: filename.mk:1: !empty(VAR) can be replaced with the simpler ${VAR}.")
+}
+
func (s *Suite) Test_MkCondChecker_checkEmpty(c *check.C) {
t := s.Init(c)
@@ -1150,3 +1172,32 @@ func (s *Suite) Test_MkCondChecker_check
nil...)
}
+
+func (s *Suite) Test_MkCondChecker_checkNotCompare(c *check.C) {
+ t := s.Init(c)
+
+ test := func(cond string, diagnostics ...string) {
+ mklines := t.NewMkLines("filename.mk",
+ ".if "+cond)
+ mkline := mklines.mklines[0]
+ ck := NewMkCondChecker(mkline, mklines)
+
+ ck.checkNotCompare(mkline.Cond().Not)
+
+ t.CheckOutput(diagnostics)
+ }
+
+ test("!${VAR} == value",
+ "WARN: filename.mk:1: The ! should use parentheses "+
+ "or be merged into the comparison operator.")
+
+ test("!${VAR} != value",
+ "WARN: filename.mk:1: The ! should use parentheses "+
+ "or be merged into the comparison operator.")
+
+ test("!(${VAR} == value)",
+ nil...)
+
+ test("!${VAR}",
+ nil...)
+}
Index: pkgsrc/pkgtools/pkglint/files/mkcondchecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mkcondchecker.go:1.3 pkgsrc/pkgtools/pkglint/files/mkcondchecker.go:1.4
--- pkgsrc/pkgtools/pkglint/files/mkcondchecker.go:1.3 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/mkcondchecker.go Sat Jan 18 21:56:09 2020
@@ -36,9 +36,10 @@ func (ck *MkCondChecker) Check() {
// Skip subconditions that have already been handled as part of the !(...).
done := make(map[interface{}]bool)
- checkNotEmpty := func(not *MkCond) {
+ checkNot := func(not *MkCond) {
empty := not.Empty
if empty != nil {
+ ck.checkNotEmpty(not)
ck.checkEmpty(empty, true, true)
done[empty] = true
}
@@ -48,6 +49,8 @@ func (ck *MkCondChecker) Check() {
ck.checkEmpty(varUse, false, false)
done[varUse] = true
}
+
+ ck.checkNotCompare(not)
}
checkEmpty := func(empty *MkVarUse) {
@@ -63,13 +66,29 @@ func (ck *MkCondChecker) Check() {
}
cond.Walk(&MkCondCallback{
- Not: checkNotEmpty,
+ Not: checkNot,
Empty: checkEmpty,
Var: checkVar,
Compare: ck.checkCompare,
VarUse: checkVarUse})
}
+func (ck *MkCondChecker) checkNotEmpty(not *MkCond) {
+ // Consider suggesting ${VAR} instead of !empty(VAR) since it is
+ // shorter and avoids unnecessary negation, which makes the
+ // expression less confusing.
+ //
+ // This applies especially to the ${VAR:Mpattern} form.
+ //
+ // See MkCondChecker.simplify.
+ if !G.Experimental {
+ return
+ }
+
+ ck.MkLine.Notef("!empty(%s%s) can be replaced with the simpler %s.",
+ not.Empty.varname, not.Empty.Mod(), not.Empty.String())
+}
+
// checkEmpty checks a condition of the form empty(VAR),
// empty(VAR:Mpattern) or ${VAR:Mpattern} in an .if directive.
func (ck *MkCondChecker) checkEmpty(varuse *MkVarUse, fromEmpty bool, neg bool) {
@@ -90,6 +109,7 @@ func (ck *MkCondChecker) checkEmptyExpr(
"",
"\tempty(VARNAME:Mpattern)",
"\t${VARNAME:Mpattern} == \"\"",
+ "\t!${VARNAME:Mpattern}",
"",
"Instead of !empty(${VARNAME:Mpattern}), you should write either of the following:",
"",
@@ -129,7 +149,6 @@ var mkCondModifierPatternLiteral = textp
// * fromEmpty is true for the form empty(VAR...), and false for ${VAR...}.
//
// * neg is true for the form !empty(VAR...), and false for empty(VAR...).
-// It also applies to the ${VAR} form.
func (ck *MkCondChecker) simplify(varuse *MkVarUse, fromEmpty bool, neg bool) {
varname := varuse.varname
mods := varuse.modifiers
@@ -300,3 +319,11 @@ func (ck *MkCondChecker) checkCompareVar
fix.Replace("${PKGSRC_COMPILER} "+op+" \""+value+"\"", "${PKGSRC_COMPILER:"+matchOp+value+"}")
fix.Apply()
}
+
+func (ck *MkCondChecker) checkNotCompare(not *MkCond) {
+ if not.Compare == nil {
+ return
+ }
+
+ ck.MkLine.Warnf("The ! should use parentheses or be merged into the comparison operator.")
+}
Index: pkgsrc/pkgtools/pkglint/files/mkline.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline.go:1.73 pkgsrc/pkgtools/pkglint/files/mkline.go:1.74
--- pkgsrc/pkgtools/pkglint/files/mkline.go:1.73 Mon Jan 6 20:38:42 2020
+++ pkgsrc/pkgtools/pkglint/files/mkline.go Sat Jan 18 21:56:09 2020
@@ -74,8 +74,11 @@ func (mkline *MkLine) String() string {
func (mkline *MkLine) HasComment() bool { return mkline.splitResult.hasComment }
-func (mkline *MkLine) HasRationale() bool { return mkline.splitResult.rationale != "" }
-
+// Rationale returns the comments that are close enough to this line.
+//
+// These comments are used to suppress pkglint warnings,
+// such as for BROKEN, NOT_FOR_PLATFORMS, MAKE_JOBS_SAFE,
+// and HOMEPAGE using http instead of https.
func (mkline *MkLine) Rationale() string { return mkline.splitResult.rationale }
// Comment returns the comment after the first unescaped #.
@@ -564,7 +567,8 @@ func (*MkLine) WithoutMakeVariables(valu
return valueNovar.String()
}
-func (mkline *MkLine) ResolveVarsInRelativePath(relativePath RelPath, pkg *Package) RelPath {
+func (mkline *MkLine) ResolveVarsInRelativePath(relativePath PackagePath, pkg *Package) PackagePath {
+ // TODO: Not every path is relative to the package directory.
if !containsVarUse(relativePath.String()) {
return relativePath.CleanPath()
}
Index: pkgsrc/pkgtools/pkglint/files/mkline_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.78 pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.79
--- pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.78 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/mkline_test.go Sat Jan 18 21:56:09 2020
@@ -535,7 +535,7 @@ func (s *Suite) Test_MkLine_ResolveVarsI
mkline := mklines.mklines[0]
var pkg *Package = nil
- test := func(before RelPath, after RelPath) {
+ test := func(before PackagePath, after PackagePath) {
t.CheckEquals(mkline.ResolveVarsInRelativePath(before, pkg), after)
}
@@ -1164,7 +1164,7 @@ func (s *Suite) Test_MkLine_VariableNeed
"",
"GO_SRCPATH=\t\t${HOMEPAGE:S,https://,,}";,
"LINKER_RPATH_FLAG:=\t${LINKER_RPATH_FLAG:S/-rpath/& /}",
- "HOMEPAGE=\t\thttp://godoc.org/${GO_SRCPATH}";,
+ "HOMEPAGE=\t\thttps://godoc.org/${GO_SRCPATH}";,
"PATH:=\t\t\t${PREFIX}/cross/bin:${PATH}",
"NO_SRC_ON_FTP=\t\t${RESTRICTED}")
Index: pkgsrc/pkgtools/pkglint/files/package.go
diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.78 pkgsrc/pkgtools/pkglint/files/package.go:1.79
--- pkgsrc/pkgtools/pkglint/files/package.go:1.78 Sat Jan 11 15:47:58 2020
+++ pkgsrc/pkgtools/pkglint/files/package.go Sat Jan 18 21:56:09 2020
@@ -395,7 +395,8 @@ func (pkg *Package) loadIncluded(mkline
func (pkg *Package) resolveIncludedFile(mkline *MkLine, includingFilename CurrPath) RelPath {
// TODO: Try to combine resolveVariableRefs and ResolveVarsInRelativePath.
- resolved := mkline.ResolveVarsInRelativePath(mkline.IncludedFile(), pkg)
+ // TODO: Not every relative path is relative to the package directory.
+ resolved := mkline.ResolveVarsInRelativePath(NewPackagePath(mkline.IncludedFile()), pkg)
includedText := resolveVariableRefs(resolved.String(), nil, pkg)
includedFile := NewRelPathString(includedText)
if containsVarUse(includedText) {
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.78 pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.79
--- pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.78 Sat Jan 11 15:47:58 2020
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck.go Sat Jan 18 21:56:09 2020
@@ -289,11 +289,11 @@ func (cv *VartypeCheck) ConfFiles() {
}
}
-func (cv *VartypeCheck) Dependency() {
+func (cv *VartypeCheck) DependencyPattern() {
value := cv.Value
parser := NewMkParser(nil, value)
- deppat := parser.Dependency()
+ deppat := parser.DependencyPattern()
rest := parser.Rest()
if deppat != nil && deppat.Wildcard == "" && (rest == "{,nb*}" || rest == "{,nb[0-9]*}") {
@@ -384,8 +384,9 @@ func (cv *VartypeCheck) DependencyWithPa
parts := cv.MkLine.ValueSplit(value, ":")
if len(parts) == 2 {
pattern := parts[0]
- relpath := NewRelPathString(parts[1])
- pathParts := relpath.Parts()
+ packagePath := NewPackagePathString(parts[1])
+ relPath := packagePath.AsRelPath()
+ pathParts := relPath.Parts()
pkg := pathParts[len(pathParts)-1]
if len(pathParts) >= 2 && pathParts[0] == ".." && pathParts[1] != ".." {
@@ -393,8 +394,9 @@ func (cv *VartypeCheck) DependencyWithPa
cv.MkLine.ExplainRelativeDirs()
}
- if !containsVarUse(relpath.String()) {
- MkLineChecker{cv.MkLines, cv.MkLine}.CheckRelativePkgdir(relpath)
+ if !containsVarUse(packagePath.String()) {
+ ck := MkLineChecker{cv.MkLines, cv.MkLine}
+ ck.CheckRelativePkgdir(relPath, packagePath)
}
switch pkg {
@@ -406,7 +408,7 @@ func (cv *VartypeCheck) DependencyWithPa
cv.Warnf("Please use USE_TOOLS+=gmake instead of this dependency.")
}
- cv.WithValue(pattern).Dependency()
+ cv.WithValue(pattern).DependencyPattern()
return
}
@@ -630,7 +632,11 @@ func (cv *VartypeCheck) GccReqd() {
func (cv *VartypeCheck) Homepage() {
cv.URL()
+ cv.homepageBasedOnMasterSites()
+ cv.homepageHttp()
+}
+func (cv *VartypeCheck) homepageBasedOnMasterSites() {
m, wrong, sitename, subdir := match3(cv.Value, `^(\$\{(MASTER_SITE\w+)(?::=([\w\-/]+))?\})`)
if !m {
return
@@ -670,6 +676,70 @@ func (cv *VartypeCheck) Homepage() {
fix.Apply()
}
+func (cv *VartypeCheck) homepageHttp() {
+ m, host := match1(cv.Value, `http://([A-Za-z0-9-.]+)`)
+ if !m {
+ return
+ }
+
+ rationale := cv.MkLine.Rationale()
+ if containsWord(rationale, "http") || containsWord(rationale, "https") {
+ return
+ }
+
+ hasAnySuffix := func(s string, suffixes ...string) bool {
+ for _, suffix := range suffixes {
+ if hasSuffix(s, suffix) {
+ dotIndex := len(s) - len(suffix)
+ if dotIndex == 0 || s[dotIndex-1] == '.' {
+ return true
+ }
+ }
+ }
+ return false
+ }
+
+ // Don't warn about sites that don't support https at all.
+ if hasAnySuffix(host,
+ "www.gnustep.org", // 2020-01-18
+ "aspell.net", // 2020-01-18
+ ) {
+ return
+ }
+
+ supportsHttps := hasAnySuffix(host,
+ "apache.org",
+ "archive.org",
+ "ctan.org",
+ "freedesktop.org",
+ "github.com",
+ "github.io",
+ "gnome.org",
+ "gnu.org",
+ "kde.org",
+ "kldp.net",
+ "linuxfoundation.org",
+ "NetBSD.org",
+ "nongnu.org",
+ "sf.net",
+ "sourceforge.net",
+ "tryton.org",
+ "tug.org")
+
+ fix := cv.Autofix()
+ fix.Warnf("HOMEPAGE should use https instead of http.")
+ if supportsHttps {
+ fix.Replace("http", "https")
+ }
+ fix.Explain(
+ "To provide secure communication by default,",
+ "the HOMEPAGE URL should use the https protocol if available.",
+ "",
+ "If the HOMEPAGE really does not support https,",
+ "add a comment near the HOMEPAGE variable stating this clearly.")
+ fix.Apply()
+}
+
// Identifier checks for valid identifiers in various contexts, limiting the
// valid characters to A-Za-z0-9_.
func (cv *VartypeCheck) IdentifierDirect() {
@@ -1148,17 +1218,21 @@ func (cv *VartypeCheck) RPkgVer() {
}
}
-// RelativePkgDir refers to a package directory, e.g. ../../category/pkgbase.
+// RelativePkgDir refers from one package directory to another package
+// directory, e.g. ../../category/pkgbase.
func (cv *VartypeCheck) RelativePkgDir() {
if NewPath(cv.Value).IsAbs() {
cv.Errorf("The path %q must be relative.", cv.Value)
return
}
- MkLineChecker{cv.MkLines, cv.MkLine}.CheckRelativePkgdir(NewRelPathString(cv.Value))
+ ck := MkLineChecker{cv.MkLines, cv.MkLine}
+ packagePath := NewPackagePathString(cv.Value)
+ ck.CheckRelativePkgdir(packagePath.AsRelPath(), packagePath)
}
-// RelativePkgPath refers to a file or directory, e.g. ../../category/pkgbase,
+// RelativePkgPath refers from one package directory to another file
+// or directory, e.g. ../../category/pkgbase,
// ../../category/pkgbase/Makefile.
//
// See RelativePkgDir, which requires a directory, not a file.
@@ -1168,7 +1242,9 @@ func (cv *VartypeCheck) RelativePkgPath(
return
}
- MkLineChecker{cv.MkLines, cv.MkLine}.CheckRelativePath(NewRelPathString(cv.Value), true)
+ packagePath := NewPackagePathString(cv.Value)
+ ck := MkLineChecker{cv.MkLines, cv.MkLine}
+ ck.CheckRelativePath(packagePath, packagePath.AsRelPath(), true)
}
func (cv *VartypeCheck) Restricted() {
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.61 pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.62
--- pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.61 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker.go Sat Jan 18 21:56:09 2020
@@ -236,7 +236,8 @@ func (ck MkLineChecker) checkInclude() {
if trace.Tracing {
trace.Stepf("includingFile=%s includedFile=%s", mkline.Filename, includedFile)
}
- ck.CheckRelativePath(includedFile, mustExist)
+ // TODO: Not every path is relative to the package directory.
+ ck.CheckRelativePath(NewPackagePath(includedFile), includedFile, mustExist)
switch {
case includedFile.HasBase("Makefile"):
@@ -269,7 +270,7 @@ func (ck MkLineChecker) checkInclude() {
mkline.Warnf("Please write \"USE_TOOLS+= intltool\" instead of this line.")
case includedFile != "builtin.mk" && includedFile.HasSuffixPath("builtin.mk"):
- if mkline.Basename != "hacks.mk" && !mkline.HasRationale() {
+ if mkline.Basename != "hacks.mk" && mkline.Rationale() == "" {
fix := mkline.Autofix()
fix.Errorf("%q must not be included directly. Include %q instead.",
includedFile, includedFile.DirNoClean().JoinNoClean("buildlink3.mk"))
@@ -294,26 +295,28 @@ func (ck MkLineChecker) checkDirectiveIn
// CheckRelativePath checks a relative path that leads to the directory of another package
// or to a subdirectory thereof or a file within there.
-func (ck MkLineChecker) CheckRelativePath(relativePath RelPath, mustExist bool) {
+func (ck MkLineChecker) CheckRelativePath(pp PackagePath, rel RelPath, mustExist bool) {
+ // TODO: Not every path is relative to the package directory.
if trace.Tracing {
- defer trace.Call(relativePath, mustExist)()
+ defer trace.Call(rel, mustExist)()
}
mkline := ck.MkLine
- if !G.Wip && relativePath.ContainsPath("wip") {
+ if !G.Wip && rel.ContainsPath("wip") {
mkline.Errorf("A main pkgsrc package must not depend on a pkgsrc-wip package.")
}
- resolvedPath := mkline.ResolveVarsInRelativePath(relativePath, ck.MkLines.pkg)
+ resolvedPath := mkline.ResolveVarsInRelativePath(pp, ck.MkLines.pkg)
if containsVarUse(resolvedPath.String()) {
return
}
- abs := mkline.Filename.DirNoClean().JoinNoClean(resolvedPath)
+ resolvedRel := resolvedPath.AsRelPath()
+ abs := mkline.Filename.DirNoClean().JoinNoClean(resolvedRel)
if !abs.Exists() {
- pkgsrcPath := G.Pkgsrc.Rel(ck.MkLine.File(resolvedPath))
+ pkgsrcPath := G.Pkgsrc.Rel(ck.MkLine.File(resolvedRel))
if mustExist && !ck.MkLines.indentation.HasExists(pkgsrcPath) {
- mkline.Errorf("Relative path %q does not exist.", resolvedPath)
+ mkline.Errorf("Relative path %q does not exist.", rel)
}
return
}
@@ -353,17 +356,19 @@ func (ck MkLineChecker) CheckRelativePat
//
// When used in .include directives, the relative package directories must be written
// with the leading ../.. anyway, so the benefit might not be too big at all.
-func (ck MkLineChecker) CheckRelativePkgdir(pkgdir RelPath) {
+func (ck MkLineChecker) CheckRelativePkgdir(rel RelPath, pkgdir PackagePath) {
+ // TODO: Not every path is relative to the package directory.
if trace.Tracing {
defer trace.Call(pkgdir)()
}
mkline := ck.MkLine
- ck.CheckRelativePath(pkgdir.JoinNoClean("Makefile"), true)
+ makefile := pkgdir.JoinNoClean("Makefile")
+ ck.CheckRelativePath(makefile, makefile.AsRelPath(), true)
pkgdir = mkline.ResolveVarsInRelativePath(pkgdir, ck.MkLines.pkg)
if !matches(pkgdir.String(), `^\.\./\.\./([^./][^/]*/[^./][^/]*)$`) && !containsVarUse(pkgdir.String()) {
- mkline.Warnf("%q is not a valid relative package directory.", pkgdir)
+ mkline.Warnf("%q is not a valid relative package directory.", rel)
mkline.Explain(
"A relative pathname always starts with \"../../\", followed",
"by a category, a slash and a the directory name of the package.",
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.57 pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.58
--- pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.57 Mon Jan 6 20:38:42 2020
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go Sat Jan 18 21:56:09 2020
@@ -703,13 +703,14 @@ func (s *Suite) Test_MkLineChecker_Check
t.CreateFileLines("other/package/Makefile")
- test := func(relativePkgdir RelPath, diagnostics ...string) {
+ test := func(relativePkgdir PackagePath, diagnostics ...string) {
// Must be in the filesystem because of directory references.
mklines := t.SetUpFileMkLines("category/package/Makefile",
"# dummy")
checkRelativePkgdir := func(mkline *MkLine) {
- MkLineChecker{mklines, mkline}.CheckRelativePkgdir(relativePkgdir)
+ ck := MkLineChecker{mklines, mkline}
+ ck.CheckRelativePkgdir(relativePkgdir.AsRelPath(), relativePkgdir)
}
mklines.ForEach(checkRelativePkgdir)
Index: pkgsrc/pkgtools/pkglint/files/mklines_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.59 pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.60
--- pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.59 Mon Jan 6 20:38:42 2020
+++ pkgsrc/pkgtools/pkglint/files/mklines_test.go Sat Jan 18 21:56:09 2020
@@ -423,7 +423,7 @@ func (s *Suite) Test_MkLines_collectRati
mklines.collectRationale()
var actual []string
mklines.ForEach(func(mkline *MkLine) {
- actual = append(actual, condStr(mkline.HasRationale(), "R ", "- ")+mkline.Text)
+ actual = append(actual, condStr(mkline.Rationale() != "", "R ", "- ")+mkline.Text)
})
t.CheckDeepEquals(actual, specs)
}
Index: pkgsrc/pkgtools/pkglint/files/mkparser.go
diff -u pkgsrc/pkgtools/pkglint/files/mkparser.go:1.41 pkgsrc/pkgtools/pkglint/files/mkparser.go:1.42
--- pkgsrc/pkgtools/pkglint/files/mkparser.go:1.41 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/mkparser.go Sat Jan 18 21:56:09 2020
@@ -113,7 +113,7 @@ func (p *MkParser) mkCondCompare() *MkCo
if cond != nil {
lexer.SkipHspace()
if lexer.SkipByte(')') {
- return cond
+ return &MkCond{Paren: cond}
}
}
lexer.Reset(mark)
@@ -247,11 +247,6 @@ func (p *MkParser) mkCondFunc() *MkCond
}
}
- // TODO: Consider suggesting ${VAR} instead of !empty(VAR) since it is shorter and
- // avoids unnecessary negation, which makes the expression less confusing.
- // This applies especially to the ${VAR:Mpattern} form.
- // See MkCondChecker.simplify.
-
case "commands", "exists", "make", "target":
argMark := lexer.Mark()
for p.mklex.VarUse() != nil || lexer.SkipBytesFunc(func(b byte) bool { return b != '$' && b != ')' }) {
@@ -341,8 +336,8 @@ type DependencyPattern struct {
Wildcard string // "[0-9]*", "1.5.*", "${PYVER}"
}
-// Dependency parses a dependency pattern like "pkg>=1<2" or "pkg-[0-9]*".
-func (p *MkParser) Dependency() *DependencyPattern {
+// DependencyPattern parses a dependency pattern like "pkg>=1<2" or "pkg-[0-9]*".
+func (p *MkParser) DependencyPattern() *DependencyPattern {
lexer := p.lexer
parseVersion := func() string {
@@ -454,6 +449,7 @@ type MkCond struct {
Term *MkCondTerm
Compare *MkCondCompare
Call *MkCondCall
+ Paren *MkCond
}
type MkCondCompare struct {
Left MkCondTerm
@@ -483,6 +479,7 @@ type MkCondCallback struct {
Empty func(empty *MkVarUse)
Compare func(left *MkCondTerm, op string, right *MkCondTerm)
Call func(name string, arg string)
+ Paren func(cond *MkCond)
// Var is called for every atomic expression that consists solely
// of a variable use, possibly enclosed in double quotes, but without
@@ -560,6 +557,12 @@ func (w *MkCondWalker) Walk(cond *MkCond
callback.Call(call.Name, call.Arg)
}
w.walkStr(cond.Call.Arg, callback)
+
+ case cond.Paren != nil:
+ if callback.Paren != nil {
+ callback.Paren(cond.Paren)
+ }
+ w.Walk(cond.Paren, callback)
}
}
Index: pkgsrc/pkgtools/pkglint/files/mkparser_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.38 pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.39
--- pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.38 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/mkparser_test.go Sat Jan 18 21:56:09 2020
@@ -8,7 +8,37 @@ import (
func (s *Suite) Test_MkParser_MkCond(c *check.C) {
t := s.Init(c)
b := NewMkTokenBuilder()
- varUse := b.VarUse
+
+ cmp := func(left MkCondTerm, op string, right MkCondTerm) *MkCond {
+ return &MkCond{Compare: &MkCondCompare{left, op, right}}
+ }
+ cvar := func(name string, modifiers ...string) MkCondTerm {
+ return MkCondTerm{Var: b.VarUse(name, modifiers...)}
+ }
+ cstr := func(s string) MkCondTerm { return MkCondTerm{Str: s} }
+ cnum := func(s string) MkCondTerm { return MkCondTerm{Num: s} }
+
+ termVar := func(varname string, mods ...string) *MkCond {
+ return &MkCond{Term: &MkCondTerm{Var: b.VarUse(varname, mods...)}}
+ }
+ termNum := func(num string) *MkCond {
+ return &MkCond{Term: &MkCondTerm{Num: num}}
+ }
+ termStr := func(str string) *MkCond {
+ return &MkCond{Term: &MkCondTerm{Str: str}}
+ }
+
+ or := func(args ...*MkCond) *MkCond { return &MkCond{Or: args} }
+ and := func(args ...*MkCond) *MkCond { return &MkCond{And: args} }
+ not := func(cond *MkCond) *MkCond { return &MkCond{Not: cond} }
+ call := func(name string, arg string) *MkCond {
+ return &MkCond{Call: &MkCondCall{name, arg}}
+ }
+ empty := func(varname string, mods ...string) *MkCond {
+ return &MkCond{Empty: b.VarUse(varname, mods...)}
+ }
+ defined := func(varname string) *MkCond { return &MkCond{Defined: varname} }
+ paren := func(cond *MkCond) *MkCond { return &MkCond{Paren: cond} }
testRest := func(input string, expectedTree *MkCond, expectedRest string) {
// As of July 2019 p.MkCond does not emit warnings;
@@ -22,158 +52,144 @@ func (s *Suite) Test_MkParser_MkCond(c *
test := func(input string, expectedTree *MkCond) {
testRest(input, expectedTree, "")
}
- varTerm := func(name string, modifiers ...string) MkCondTerm {
- return MkCondTerm{Var: varUse(name, modifiers...)}
- }
- str := func(s string) MkCondTerm { return MkCondTerm{Str: s} }
- num := func(s string) MkCondTerm { return MkCondTerm{Num: s} }
-
- t.Use(testRest, test, varTerm)
test("${OPSYS:MNetBSD}",
- &MkCond{Term: &MkCondTerm{Var: varUse("OPSYS", "MNetBSD")}})
+ termVar("OPSYS", "MNetBSD"))
test("defined(VARNAME)",
- &MkCond{Defined: "VARNAME"})
+ defined("VARNAME"))
test("empty(VARNAME)",
- &MkCond{Empty: varUse("VARNAME")})
+ empty("VARNAME"))
test("!empty(VARNAME)",
- &MkCond{Not: &MkCond{Empty: varUse("VARNAME")}})
+ not(empty("VARNAME")))
test("!empty(VARNAME:M[yY][eE][sS])",
- &MkCond{Not: &MkCond{Empty: varUse("VARNAME", "M[yY][eE][sS]")}})
+ not(empty("VARNAME", "M[yY][eE][sS]")))
// Colons are unescaped at this point because they cannot be mistaken for separators anymore.
test("!empty(USE_TOOLS:Mautoconf\\:run)",
- &MkCond{Not: &MkCond{Empty: varUse("USE_TOOLS", "Mautoconf:run")}})
+ not(empty("USE_TOOLS", "Mautoconf:run")))
test("${VARNAME} != \"Value\"",
- &MkCond{Compare: &MkCondCompare{varTerm("VARNAME"), "!=", str("Value")}})
+ cmp(cvar("VARNAME"), "!=", cstr("Value")))
test("${VARNAME:Mi386} != \"Value\"",
- &MkCond{Compare: &MkCondCompare{varTerm("VARNAME", "Mi386"), "!=", str("Value")}})
+ cmp(cvar("VARNAME", "Mi386"), "!=", cstr("Value")))
test("${VARNAME} != Value",
- &MkCond{Compare: &MkCondCompare{varTerm("VARNAME"), "!=", str("Value")}})
+ cmp(cvar("VARNAME"), "!=", cstr("Value")))
test("\"${VARNAME}\" != Value",
- &MkCond{Compare: &MkCondCompare{varTerm("VARNAME"), "!=", str("Value")}})
+ cmp(cvar("VARNAME"), "!=", cstr("Value")))
test("${pkg} == \"${name}\"",
- &MkCond{Compare: &MkCondCompare{varTerm("pkg"), "==", varTerm("name")}})
+ cmp(cvar("pkg"), "==", cvar("name")))
test("\"${pkg}\" == \"${name}\"",
- &MkCond{Compare: &MkCondCompare{varTerm("pkg"), "==", varTerm("name")}})
+ cmp(cvar("pkg"), "==", cvar("name")))
// The right-hand side is not analyzed further to keep the data types simple.
test("${ABC} == \"${A}B${C}\"",
- &MkCond{Compare: &MkCondCompare{varTerm("ABC"), "==", str("${A}B${C}")}})
+ cmp(cvar("ABC"), "==", cstr("${A}B${C}")))
test("${ABC} == \"${A}\\\"${B}\\\\${C}$${shellvar}${D}\"",
- &MkCond{Compare: &MkCondCompare{varTerm("ABC"), "==", str("${A}\"${B}\\${C}$${shellvar}${D}")}})
+ cmp(cvar("ABC"), "==", cstr("${A}\"${B}\\${C}$${shellvar}${D}")))
test("exists(/etc/hosts)",
- &MkCond{Call: &MkCondCall{"exists", "/etc/hosts"}})
+ call("exists", "/etc/hosts"))
test("exists(${PREFIX}/var)",
- &MkCond{Call: &MkCondCall{"exists", "${PREFIX}/var"}})
+ call("exists", "${PREFIX}/var"))
test("${OPSYS} == \"NetBSD\" || ${OPSYS} == \"OpenBSD\"",
- &MkCond{Or: []*MkCond{
- {Compare: &MkCondCompare{varTerm("OPSYS"), "==", str("NetBSD")}},
- {Compare: &MkCondCompare{varTerm("OPSYS"), "==", str("OpenBSD")}}}})
+ or(
+ cmp(cvar("OPSYS"), "==", cstr("NetBSD")),
+ cmp(cvar("OPSYS"), "==", cstr("OpenBSD"))))
test("${OPSYS} == \"NetBSD\" && ${MACHINE_ARCH} == \"i386\"",
- &MkCond{And: []*MkCond{
- {Compare: &MkCondCompare{varTerm("OPSYS"), "==", str("NetBSD")}},
- {Compare: &MkCondCompare{varTerm("MACHINE_ARCH"), "==", str("i386")}}}})
+ and(
+ cmp(cvar("OPSYS"), "==", cstr("NetBSD")),
+ cmp(cvar("MACHINE_ARCH"), "==", cstr("i386"))))
test("defined(A) && defined(B) || defined(C) && defined(D)",
- &MkCond{Or: []*MkCond{
- {And: []*MkCond{
- {Defined: "A"},
- {Defined: "B"}}},
- {And: []*MkCond{
- {Defined: "C"},
- {Defined: "D"}}}}})
+ or(
+ and(defined("A"), defined("B")),
+ and(defined("C"), defined("D"))))
test("${MACHINE_ARCH:Mi386} || ${MACHINE_OPSYS:MNetBSD}",
- &MkCond{Or: []*MkCond{
- {Term: &MkCondTerm{Var: varUse("MACHINE_ARCH", "Mi386")}},
- {Term: &MkCondTerm{Var: varUse("MACHINE_OPSYS", "MNetBSD")}}}})
+ or(
+ termVar("MACHINE_ARCH", "Mi386"),
+ termVar("MACHINE_OPSYS", "MNetBSD")))
test("${VAR} == \"${VAR}suffix\"",
- &MkCond{Compare: &MkCondCompare{varTerm("VAR"), "==", str("${VAR}suffix")}})
+ cmp(cvar("VAR"), "==", cstr("${VAR}suffix")))
// Exotic cases
// ".if 0" can be used to skip over a block of code.
test("0",
- &MkCond{Term: &MkCondTerm{Num: "0"}})
+ termNum("0"))
test("0xCAFEBABE",
- &MkCond{Term: &MkCondTerm{Num: "0xCAFEBABE"}})
+ termNum("0xCAFEBABE"))
test("${VAR} == 0xCAFEBABE",
- &MkCond{
- Compare: &MkCondCompare{
- varTerm("VAR"),
- "==",
- num("0xCAFEBABE")}})
+ cmp(cvar("VAR"), "==", cnum("0xCAFEBABE")))
test("! ( defined(A) && empty(VARNAME) )",
- &MkCond{Not: &MkCond{
- And: []*MkCond{
- {Defined: "A"},
- {Empty: varUse("VARNAME")}}}})
+ not(paren(and(defined("A"), empty("VARNAME")))))
test("${REQD_MAJOR} > ${MAJOR}",
- &MkCond{Compare: &MkCondCompare{varTerm("REQD_MAJOR"), ">", varTerm("MAJOR")}})
+ cmp(cvar("REQD_MAJOR"), ">", cvar("MAJOR")))
test("${OS_VERSION} >= 6.5",
- &MkCond{Compare: &MkCondCompare{varTerm("OS_VERSION"), ">=", num("6.5")}})
+ cmp(cvar("OS_VERSION"), ">=", cnum("6.5")))
test("${OS_VERSION} == 5.3",
- &MkCond{Compare: &MkCondCompare{varTerm("OS_VERSION"), "==", num("5.3")}})
+ cmp(cvar("OS_VERSION"), "==", cnum("5.3")))
test("!empty(${OS_VARIANT:MIllumos})", // Probably not intended
- &MkCond{Not: &MkCond{Empty: varUse("${OS_VARIANT:MIllumos}")}})
+ not(empty("${OS_VARIANT:MIllumos}")))
- // There may be whitespace before the parenthesis; see devel/bmake/files/cond.c:^compare_function.
+ // There may be whitespace before the parenthesis.
+ // See devel/bmake/files/cond.c:/^compare_function/.
test("defined (VARNAME)",
- &MkCond{Defined: "VARNAME"})
+ defined("VARNAME"))
test("${\"${PKG_OPTIONS:Moption}\":?--enable-option:--disable-option}",
- &MkCond{Term: &MkCondTerm{Var: varUse("\"${PKG_OPTIONS:Moption}\"", "?--enable-option:--disable-option")}})
+ termVar("\"${PKG_OPTIONS:Moption}\"", "?--enable-option:--disable-option"))
// Contrary to most other programming languages, the == operator binds
- // more tightly that the ! operator.
+ // more tightly than the ! operator.
//
- // TODO: Since this operator precedence is surprising there should be a warning,
- // suggesting to replace "!${VAR} == value" with "${VAR} != value".
+ // See MkCondChecker.checkNotCompare.
test("!${VAR} == value",
- &MkCond{Not: &MkCond{Compare: &MkCondCompare{varTerm("VAR"), "==", str("value")}}})
+ not(cmp(cvar("VAR"), "==", cstr("value"))))
// The left-hand side of the comparison can be a quoted string.
test("\"${VAR}suffix\" == value",
- &MkCond{Compare: &MkCondCompare{MkCondTerm{Str: "${VAR}suffix"}, "==", MkCondTerm{Str: "value"}}})
+ cmp(cstr("${VAR}suffix"), "==", cstr("value")))
test("\"${VAR}str\"",
- &MkCond{Term: &MkCondTerm{Str: "${VAR}str"}})
+ termStr("${VAR}str"))
test("commands(show-var)",
- &MkCond{Call: &MkCondCall{"commands", "show-var"}})
+ call("commands", "show-var"))
test("exists(/usr/bin)",
- &MkCond{Call: &MkCondCall{"exists", "/usr/bin"}})
+ call("exists", "/usr/bin"))
test("make(show-var)",
- &MkCond{Call: &MkCondCall{"make", "show-var"}})
+ call("make", "show-var"))
test("target(do-build)",
- &MkCond{Call: &MkCondCall{"target", "do-build"}})
+ call("target", "do-build"))
+
+ // TODO: ok "${q}text${q} == ${VAR}"
+ // TODO: fail "text${q} == ${VAR}"
+ // TODO: ok "${VAR} == ${q}text${q}"
// Errors
@@ -202,11 +218,11 @@ func (s *Suite) Test_MkParser_MkCond(c *
"exists(/unfinished")
testRest("!empty(PKG_OPTIONS:Msndfile) || defined(PKG_OPTIONS:Msamplerate)",
- &MkCond{Not: &MkCond{Empty: varUse("PKG_OPTIONS", "Msndfile")}},
+ not(empty("PKG_OPTIONS", "Msndfile")),
"|| defined(PKG_OPTIONS:Msamplerate)")
testRest("${LEFT} &&",
- &MkCond{Term: &MkCondTerm{Var: varUse("LEFT")}},
+ termVar("LEFT"),
"&&")
testRest("\"unfinished string literal",
@@ -243,7 +259,7 @@ func (s *Suite) Test_MkParser_MkCond(c *
// Too many closing parentheses are a syntax error.
testRest("(${VAR}))",
- &MkCond{Term: &MkCondTerm{Var: varUse("VAR")}},
+ paren(termVar("VAR")),
")")
// The left-hand side of the comparison cannot be an unquoted string literal.
@@ -356,12 +372,12 @@ func (s *Suite) Test_MkParser_isPkgbaseP
test("_client", false) // The combination foo-_client looks strange.
}
-func (s *Suite) Test_MkParser_Dependency(c *check.C) {
+func (s *Suite) Test_MkParser_DependencyPattern(c *check.C) {
t := s.Init(c)
testRest := func(pattern string, expected DependencyPattern, rest string) {
parser := NewMkParser(nil, pattern)
- dp := parser.Dependency()
+ dp := parser.DependencyPattern()
if c.Check(dp, check.NotNil) {
t.CheckEquals(*dp, expected)
t.CheckEquals(parser.Rest(), rest)
@@ -370,7 +386,7 @@ func (s *Suite) Test_MkParser_Dependency
testNil := func(pattern string) {
parser := NewMkParser(nil, pattern)
- dp := parser.Dependency()
+ dp := parser.DependencyPattern()
if c.Check(dp, check.IsNil) {
t.CheckEquals(parser.Rest(), pattern)
}
@@ -488,17 +504,19 @@ func (s *Suite) Test_MkCondWalker_Walk(c
events = append(events, sprintf("%14s %s", name, strings.Join(args, ", ")))
}
- // TODO: Add callbacks for And, Or, Not if needed.
- // Especially Not(Empty(VARNAME)) should be an interesting case.
+ // XXX: Add callbacks for And and Or if needed.
mkline.Cond().Walk(&MkCondCallback{
- Defined: func(varname string) {
+ func(cond *MkCond) {
+ addEvent("not")
+ },
+ func(varname string) {
addEvent("defined", varname)
},
- Empty: func(varuse *MkVarUse) {
+ func(varuse *MkVarUse) {
addEvent("empty", varuseStr(varuse))
},
- Compare: func(left *MkCondTerm, op string, right *MkCondTerm) {
+ func(left *MkCondTerm, op string, right *MkCondTerm) {
assert(left.Var != nil)
switch {
case right.Var != nil:
@@ -509,13 +527,16 @@ func (s *Suite) Test_MkCondWalker_Walk(c
addEvent("compareVarStr", varuseStr(left.Var), right.Str)
}
},
- Call: func(name string, arg string) {
+ func(name string, arg string) {
addEvent("call", name, arg)
},
- Var: func(varuse *MkVarUse) {
+ func(cond *MkCond) {
+ addEvent("paren")
+ },
+ func(varuse *MkVarUse) {
addEvent("var", varuseStr(varuse))
},
- VarUse: func(varuse *MkVarUse) {
+ func(varuse *MkVarUse) {
addEvent("varUse", varuseStr(varuse))
}})
@@ -533,9 +554,13 @@ func (s *Suite) Test_MkCondWalker_Walk(c
" varUse NUM",
" defined VAR",
" varUse VAR",
+ " not ",
" call exists, file.mk",
" call exists, ${FILE}",
" varUse FILE",
+ " paren ",
+ " paren ",
+ " paren ",
" var NONEMPTY",
" varUse NONEMPTY"})
}
Index: pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go:1.5 pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go:1.6
--- pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go:1.5 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go Sat Jan 18 21:56:09 2020
@@ -754,7 +754,7 @@ func (s *Suite) Test_MkVarUseChecker_war
t.CheckEquals(toolDependsType.AlternativeFiles(aclpUseLoadtime), "")
apiDependsType := G.Pkgsrc.VariableType(nil, "BUILDLINK_API_DEPENDS.*")
- t.CheckEquals(apiDependsType.String(), "Dependency (list, package-settable)")
+ t.CheckEquals(apiDependsType.String(), "DependencyPattern (list, package-settable)")
t.CheckEquals(apiDependsType.AlternativeFiles(aclpUse), "")
t.CheckEquals(apiDependsType.AlternativeFiles(aclpUseLoadtime), "buildlink3.mk or builtin.mk only")
Index: pkgsrc/pkgtools/pkglint/files/path.go
diff -u pkgsrc/pkgtools/pkglint/files/path.go:1.7 pkgsrc/pkgtools/pkglint/files/path.go:1.8
--- pkgsrc/pkgtools/pkglint/files/path.go:1.7 Fri Dec 13 01:39:23 2019
+++ pkgsrc/pkgtools/pkglint/files/path.go Sat Jan 18 21:56:09 2020
@@ -421,8 +421,28 @@ func (p PackagePath) JoinNoClean(other R
return NewPackagePathString(p.AsPath().JoinNoClean(other).String())
}
+func (p PackagePath) CleanPath() PackagePath {
+ return NewPackagePathString(p.AsPath().CleanPath().String())
+}
+
func (p PackagePath) IsEmpty() bool { return p.AsPath().IsEmpty() }
+func (p PackagePath) HasPrefixPath(sub Path) bool {
+ return p.AsPath().HasPrefixPath(sub)
+}
+
+func (p PackagePath) ContainsPath(sub Path) bool {
+ return p.AsPath().ContainsPath(sub)
+}
+
+func (p PackagePath) ContainsText(contained string) bool {
+ return p.AsPath().ContainsText(contained)
+}
+
+func (p PackagePath) Replace(from, to string) PackagePath {
+ return NewPackagePathString(strings.Replace(string(p), from, to, -1))
+}
+
// RelPath is a path that is relative to some base directory that is not
// further specified.
type RelPath string
Index: pkgsrc/pkgtools/pkglint/files/path_test.go
diff -u pkgsrc/pkgtools/pkglint/files/path_test.go:1.8 pkgsrc/pkgtools/pkglint/files/path_test.go:1.9
--- pkgsrc/pkgtools/pkglint/files/path_test.go:1.8 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/path_test.go Sat Jan 18 21:56:09 2020
@@ -1234,6 +1234,16 @@ func (s *Suite) Test_PackagePath_JoinNoC
"../../category/package/patches/patch-aa")
}
+func (s *Suite) Test_PackagePath_CleanPath(c *check.C) {
+ t := s.Init(c)
+
+ test := func(p PackagePath, cleaned PackagePath) {
+ t.CheckEquals(p.CleanPath(), cleaned)
+ }
+
+ test("a/b/../../c/d/../.././e/../f", "a/b/../../e/../f")
+}
+
func (s *Suite) Test_PackagePath_IsEmpty(c *check.C) {
t := s.Init(c)
@@ -1245,6 +1255,55 @@ func (s *Suite) Test_PackagePath_IsEmpty
test(".", false)
}
+func (s *Suite) Test_PackagePath_HasPrefixPath(c *check.C) {
+ t := s.Init(c)
+
+ test := func(p PackagePath, sub Path, hasPrefix bool) {
+ t.CheckEquals(p.HasPrefixPath(sub), hasPrefix)
+ }
+
+ test("/root/subdir", "subdir", false)
+ test("/root/subdir", "/root", true)
+ test("/root/subdir", "/r", false)
+}
+
+func (s *Suite) Test_PackagePath_ContainsPath(c *check.C) {
+ t := s.Init(c)
+
+ test := func(p PackagePath, sub Path, hasPrefix bool) {
+ t.CheckEquals(p.ContainsPath(sub), hasPrefix)
+ }
+
+ test("/root/subdir", "subdir", true)
+ test("/root/subdir", "/root", true)
+ test("/root/subdir", "/r", false)
+}
+
+func (s *Suite) Test_PackagePath_ContainsText(c *check.C) {
+ t := s.Init(c)
+
+ test := func(p PackagePath, sub string, hasPrefix bool) {
+ t.CheckEquals(p.ContainsText(sub), hasPrefix)
+ }
+
+ test("/root/subdir", "subdir", true)
+ test("/root/subdir", "/root", true)
+ test("/root/subdir", "/r", true)
+ test("/root/subdir", "t//sub", false)
+}
+
+func (s *Suite) Test_PackagePath_Replace(c *check.C) {
+ t := s.Init(c)
+
+ test := func(p PackagePath, from, to string, result PackagePath) {
+ t.CheckEquals(p.Replace(from, to), result)
+ }
+
+ test("dir/file", "dir", "other", "other/file")
+ test("dir/file", "r", "sk", "disk/file")
+ test("aaa/file", "a", "sub/", "sub/sub/sub//file")
+}
+
func (s *Suite) Test_NewRelPath(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/vardefs.go
diff -u pkgsrc/pkgtools/pkglint/files/vardefs.go:1.87 pkgsrc/pkgtools/pkglint/files/vardefs.go:1.88
--- pkgsrc/pkgtools/pkglint/files/vardefs.go:1.87 Sat Jan 11 15:47:58 2020
+++ pkgsrc/pkgtools/pkglint/files/vardefs.go Sat Jan 18 21:56:09 2020
@@ -886,11 +886,11 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkglistrat("BROKEN_ON_PLATFORM", BtMachinePlatformPattern)
reg.syslist("BSD_MAKE_ENV", BtShellWord)
// TODO: Align the permissions of the various BUILDLINK_*.* variables with each other.
- reg.acllist("BUILDLINK_ABI_DEPENDS.*", BtDependency,
+ reg.acllist("BUILDLINK_ABI_DEPENDS.*", BtDependencyPattern,
PackageSettable,
"buildlink3.mk, builtin.mk: append, use-loadtime",
"*: append")
- reg.acllist("BUILDLINK_API_DEPENDS.*", BtDependency,
+ reg.acllist("BUILDLINK_API_DEPENDS.*", BtDependencyPattern,
PackageSettable,
"buildlink3.mk, builtin.mk: append, use-loadtime",
"*: append")
@@ -1066,7 +1066,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkglist("CONFIG_STATUS_OVERRIDE", BtPathPattern)
reg.pkg("CONFIG_SHELL", BtPathname)
reg.pkglist("CONFIG_SUB_OVERRIDE", BtPathPattern)
- reg.pkglist("CONFLICTS", BtDependency)
+ reg.pkglist("CONFLICTS", BtDependencyPattern)
reg.pkgappend("CONF_FILES", BtConfFiles)
reg.pkg("CONF_FILES_MODE", enum("0644 0640 0600 0400"))
reg.pkglist("CONF_FILES_PERMS", BtPerms)
@@ -1145,7 +1145,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.usr("EMUL_PLATFORM", BtEmulPlatform)
reg.pkglist("EMUL_PLATFORMS", BtEmulPlatform)
reg.usrlist("EMUL_PREFER", BtEmulPlatform)
- reg.pkglist("EMUL_REQD", BtDependency)
+ reg.pkglist("EMUL_REQD", BtDependencyPattern)
reg.usr("EMUL_TYPE.*", enum("native builtin suse suse-10.0 suse-12.1 suse-13.1"))
reg.sys("ERROR_CAT", BtShellCommand)
reg.sys("ERROR_MSG", BtShellCommand)
@@ -1673,7 +1673,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkgbl3("SUBST_STAGE.*", BtStage)
reg.pkglistbl3("SUBST_VARS.*", BtVariableName)
- reg.pkglist("SUPERSEDES", BtDependency)
+ reg.pkglist("SUPERSEDES", BtDependencyPattern)
reg.pkglist("TEST_DEPENDS", BtDependencyWithPath)
reg.pkglist("TEST_DIRS", BtWrksrcSubdirectory)
reg.pkglist("TEST_ENV", BtShellWord)
Index: pkgsrc/pkgtools/pkglint/files/vartype.go
diff -u pkgsrc/pkgtools/pkglint/files/vartype.go:1.45 pkgsrc/pkgtools/pkglint/files/vartype.go:1.46
--- pkgsrc/pkgtools/pkglint/files/vartype.go:1.45 Sat Jan 11 15:47:58 2020
+++ pkgsrc/pkgtools/pkglint/files/vartype.go Sat Jan 18 21:56:09 2020
@@ -411,7 +411,7 @@ var (
BtCFlag = &BasicType{"CFlag", (*VartypeCheck).CFlag}
BtComment = &BasicType{"Comment", (*VartypeCheck).Comment}
BtConfFiles = &BasicType{"ConfFiles", (*VartypeCheck).ConfFiles}
- BtDependency = &BasicType{"Dependency", (*VartypeCheck).Dependency}
+ BtDependencyPattern = &BasicType{"DependencyPattern", (*VartypeCheck).DependencyPattern}
BtDependencyWithPath = &BasicType{"DependencyWithPath", (*VartypeCheck).DependencyWithPath}
BtDistSuffix = &BasicType{"DistSuffix", (*VartypeCheck).DistSuffix}
BtEmulPlatform = &BasicType{"EmulPlatform", (*VartypeCheck).EmulPlatform}
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.72 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.73
--- pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.72 Sat Jan 11 15:47:58 2020
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go Sat Jan 18 21:56:09 2020
@@ -362,9 +362,9 @@ func (s *Suite) Test_VartypeCheck_ConfFi
"WARN: filename.mk:5: The destination file \"/etc/bootrc\" should start with a variable reference.")
}
-// See Test_MkParser_Dependency.
-func (s *Suite) Test_VartypeCheck_Dependency(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), BtDependency)
+// See Test_MkParser_DependencyPattern.
+func (s *Suite) Test_VartypeCheck_DependencyPattern(c *check.C) {
+ vt := NewVartypeCheckTester(s.Init(c), BtDependencyPattern)
vt.Varname("CONFLICTS")
vt.Op(opAssignAppend)
@@ -919,6 +919,7 @@ func (s *Suite) Test_VartypeCheck_Homepa
"${MASTER_SITES}")
vt.Output(
+ "WARN: filename.mk:1: HOMEPAGE should use https instead of http.",
"WARN: filename.mk:3: HOMEPAGE should not be defined in terms of MASTER_SITEs.")
pkg := NewPackage(t.File("category/package"))
@@ -972,6 +973,27 @@ func (s *Suite) Test_VartypeCheck_Homepa
"WARN: filename.mk:41: HOMEPAGE should not be defined in terms of MASTER_SITEs.")
}
+func (s *Suite) Test_VartypeCheck_Homepage__http(c *check.C) {
+ t := s.Init(c)
+ vt := NewVartypeCheckTester(t, BtHomepage)
+
+ vt.Varname("HOMEPAGE")
+ vt.Values(
+ "http://www.gnustep.org/";,
+ "http://www.pkgsrc.org/";,
+ "http://project.sourceforge.net/";,
+ "http://sf.net/p/project/";,
+ "http://example.org/ # doesn't support https",
+ "http://example.org/ # only supports http",
+ "http://asf.net/";)
+
+ vt.Output(
+ "WARN: filename.mk:2: HOMEPAGE should use https instead of http.",
+ "WARN: filename.mk:3: HOMEPAGE should use https instead of http.",
+ "WARN: filename.mk:4: HOMEPAGE should use https instead of http.",
+ "WARN: filename.mk:7: HOMEPAGE should use https instead of http.")
+}
+
func (s *Suite) Test_VartypeCheck_IdentifierDirect(c *check.C) {
t := s.Init(c)
vt := NewVartypeCheckTester(t, BtIdentifierDirect)
@@ -1651,6 +1673,20 @@ func (s *Suite) Test_VartypeCheck_Relati
"ERROR: filename.mk:4: Relative path \"invalid\" does not exist.",
"ERROR: filename.mk:5: Relative path \"../../invalid/relative\" does not exist.",
"ERROR: filename.mk:6: The path \"/absolute\" must be relative.")
+
+ vt.File("../../mk/infra.mk")
+ vt.Values(
+ "../package",
+ "../../category/other-package",
+ "../../missing/package",
+ "../../category/missing")
+
+ vt.Output(
+ "ERROR: ../../mk/infra.mk:1: Relative path \"../package\" does not exist.",
+ // FIXME: This directory _does_ exist.
+ "ERROR: ../../mk/infra.mk:2: Relative path \"../../category/other-package\" does not exist.",
+ "ERROR: ../../mk/infra.mk:3: Relative path \"../../missing/package\" does not exist.",
+ "ERROR: ../../mk/infra.mk:4: Relative path \"../../category/missing\" does not exist.")
}
func (s *Suite) Test_VartypeCheck_Restricted(c *check.C) {
@@ -2322,6 +2358,7 @@ func (vt *VartypeCheckTester) Values(val
line := vt.tester.NewLine(vt.filename, vt.lineno, text)
mklines := NewMkLines(NewLines(vt.filename, []*Line{line}), vt.pkg, nil)
+ mklines.collectRationale()
vt.lineno++
mklines.ForEach(func(mkline *MkLine) { test(mklines, mkline, value) })
Home |
Main Index |
Thread Index |
Old Index