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: Tue Oct 1 21:38:00 UTC 2019
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile
pkgsrc/pkgtools/pkglint/files: alternatives.go category.go
check_test.go distinfo.go distinfo_test.go licenses_test.go
logging.go logging_test.go mkline.go mkline_test.go
mklinechecker.go mklinechecker_test.go mklines_test.go mkparser.go
mkparser_test.go mktokenslexer.go mktokenslexer_test.go
mktypes_test.go package.go package_test.go pkglint.go
pkglint_test.go pkgsrc.go pkgsrc_test.go shell.go shell_test.go
substcontext.go tools_test.go toplevel.go toplevel_test.go util.go
util_test.go vardefs.go vargroups.go vargroups_test.go vartype.go
vartypecheck.go vartypecheck_test.go
pkgsrc/pkgtools/pkglint/files/textproc: lexer.go lexer_test.go
Log Message:
pkgtools/pkglint: update to 19.3.0
Changes since 5.7.24:
* There is no need to ask the dummy MAINTAINER from url2pkg whether
committing changes is ok.
* When autofixing a condition like !empty(PKGPATH:Mliteral), don't
generate unnecessary parentheses around ${PKGPATH} == literal.
* In a _VARGROUPS section, the public variables should be listed
before the private variables, to put important things first.
* When pkglint suggests to be run again with the -e, -fs or -F options,
repeat the whole command line, to allow for copy-and-paste.
* The checks for PKGPATH are fixed and enhanced. It is not a relative
path like in ../../category/package, but relative to the pkgsrc root.
* Unintended file globbing in sed commands such as s,.*,any, gets a
warning.
* MASTER_SITES should normally end with a slash, in rare cases an
equals sign or a colon are correct, too.
* Detect redundant directories in INSTALLATION_DIRS.
To generate a diff of this commit:
cvs rdiff -u -r1.598 -r1.599 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.13 -r1.14 pkgsrc/pkgtools/pkglint/files/alternatives.go \
pkgsrc/pkgtools/pkglint/files/mktypes_test.go
cvs rdiff -u -r1.21 -r1.22 pkgsrc/pkgtools/pkglint/files/category.go
cvs rdiff -u -r1.48 -r1.49 pkgsrc/pkgtools/pkglint/files/check_test.go \
pkgsrc/pkgtools/pkglint/files/mklines_test.go
cvs rdiff -u -r1.35 -r1.36 pkgsrc/pkgtools/pkglint/files/distinfo.go \
pkgsrc/pkgtools/pkglint/files/pkgsrc.go
cvs rdiff -u -r1.32 -r1.33 pkgsrc/pkgtools/pkglint/files/distinfo_test.go \
pkgsrc/pkgtools/pkglint/files/mkparser_test.go
cvs rdiff -u -r1.23 -r1.24 pkgsrc/pkgtools/pkglint/files/licenses_test.go
cvs rdiff -u -r1.27 -r1.28 pkgsrc/pkgtools/pkglint/files/logging.go \
pkgsrc/pkgtools/pkglint/files/substcontext.go
cvs rdiff -u -r1.20 -r1.21 pkgsrc/pkgtools/pkglint/files/logging_test.go \
pkgsrc/pkgtools/pkglint/files/toplevel.go
cvs rdiff -u -r1.58 -r1.59 pkgsrc/pkgtools/pkglint/files/mkline.go
cvs rdiff -u -r1.67 -r1.68 pkgsrc/pkgtools/pkglint/files/mkline_test.go
cvs rdiff -u -r1.46 -r1.47 pkgsrc/pkgtools/pkglint/files/mklinechecker.go
cvs rdiff -u -r1.42 -r1.43 \
pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
cvs rdiff -u -r1.33 -r1.34 pkgsrc/pkgtools/pkglint/files/mkparser.go \
pkgsrc/pkgtools/pkglint/files/util_test.go
cvs rdiff -u -r1.1 -r1.2 pkgsrc/pkgtools/pkglint/files/mktokenslexer.go
cvs rdiff -u -r1.3 -r1.4 pkgsrc/pkgtools/pkglint/files/mktokenslexer_test.go
cvs rdiff -u -r1.62 -r1.63 pkgsrc/pkgtools/pkglint/files/package.go \
pkgsrc/pkgtools/pkglint/files/vartypecheck.go
cvs rdiff -u -r1.53 -r1.54 pkgsrc/pkgtools/pkglint/files/package_test.go
cvs rdiff -u -r1.60 -r1.61 pkgsrc/pkgtools/pkglint/files/pkglint.go
cvs rdiff -u -r1.47 -r1.48 pkgsrc/pkgtools/pkglint/files/pkglint_test.go
cvs rdiff -u -r1.29 -r1.30 pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go
cvs rdiff -u -r1.45 -r1.46 pkgsrc/pkgtools/pkglint/files/shell.go
cvs rdiff -u -r1.52 -r1.53 pkgsrc/pkgtools/pkglint/files/shell_test.go \
pkgsrc/pkgtools/pkglint/files/util.go
cvs rdiff -u -r1.19 -r1.20 pkgsrc/pkgtools/pkglint/files/tools_test.go \
pkgsrc/pkgtools/pkglint/files/toplevel_test.go
cvs rdiff -u -r1.72 -r1.73 pkgsrc/pkgtools/pkglint/files/vardefs.go
cvs rdiff -u -r1.2 -r1.3 pkgsrc/pkgtools/pkglint/files/vargroups.go \
pkgsrc/pkgtools/pkglint/files/vargroups_test.go
cvs rdiff -u -r1.36 -r1.37 pkgsrc/pkgtools/pkglint/files/vartype.go
cvs rdiff -u -r1.54 -r1.55 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
cvs rdiff -u -r1.5 -r1.6 pkgsrc/pkgtools/pkglint/files/textproc/lexer.go \
pkgsrc/pkgtools/pkglint/files/textproc/lexer_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.598 pkgsrc/pkgtools/pkglint/Makefile:1.599
--- pkgsrc/pkgtools/pkglint/Makefile:1.598 Thu Sep 26 20:10:51 2019
+++ pkgsrc/pkgtools/pkglint/Makefile Tue Oct 1 21:37:59 2019
@@ -1,7 +1,6 @@
-# $NetBSD: Makefile,v 1.598 2019/09/26 20:10:51 bsiegert Exp $
+# $NetBSD: Makefile,v 1.599 2019/10/01 21:37:59 rillig Exp $
-PKGNAME= pkglint-5.7.24
-PKGREVISION= 1
+PKGNAME= pkglint-19.3.0
CATEGORIES= pkgtools
DISTNAME= tools
MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/}
Index: pkgsrc/pkgtools/pkglint/files/alternatives.go
diff -u pkgsrc/pkgtools/pkglint/files/alternatives.go:1.13 pkgsrc/pkgtools/pkglint/files/alternatives.go:1.14
--- pkgsrc/pkgtools/pkglint/files/alternatives.go:1.13 Sun Jul 14 21:25:47 2019
+++ pkgsrc/pkgtools/pkglint/files/alternatives.go Tue Oct 1 21:37:59 2019
@@ -49,7 +49,7 @@ func CheckFileAlternatives(filename stri
if !m {
line.Errorf("Invalid line %q.", line.Text)
line.Explain(
- sprintf("Run %q for more information.", makeHelp("alternatives")))
+ sprintf("Run %q for more information.", bmakeHelp("alternatives")))
continue
}
Index: pkgsrc/pkgtools/pkglint/files/mktypes_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mktypes_test.go:1.13 pkgsrc/pkgtools/pkglint/files/mktypes_test.go:1.14
--- pkgsrc/pkgtools/pkglint/files/mktypes_test.go:1.13 Sun Jul 14 21:25:47 2019
+++ pkgsrc/pkgtools/pkglint/files/mktypes_test.go Tue Oct 1 21:37:59 2019
@@ -99,7 +99,7 @@ func (s *Suite) Test_MkVarUseModifier_Ma
t.CheckEquals(ok, true)
t.CheckEquals(regex, false)
- t.CheckEquals(from, "\\/")
+ t.CheckEquals(from, "/")
t.CheckEquals(to, "\\:")
t.CheckEquals(options, "")
}
Index: pkgsrc/pkgtools/pkglint/files/category.go
diff -u pkgsrc/pkgtools/pkglint/files/category.go:1.21 pkgsrc/pkgtools/pkglint/files/category.go:1.22
--- pkgsrc/pkgtools/pkglint/files/category.go:1.21 Sun Jul 14 21:25:47 2019
+++ pkgsrc/pkgtools/pkglint/files/category.go Tue Oct 1 21:37:59 2019
@@ -158,7 +158,7 @@ func CheckdirCategory(dir string) {
var recurseInto []string
for _, msub := range mSubdirs {
if !msub.line.IsCommentedVarassign() {
- recurseInto = append(recurseInto, dir+"/"+msub.name)
+ recurseInto = append(recurseInto, joinPath(dir, msub.name))
}
}
G.Todo = append(recurseInto, G.Todo...)
Index: pkgsrc/pkgtools/pkglint/files/check_test.go
diff -u pkgsrc/pkgtools/pkglint/files/check_test.go:1.48 pkgsrc/pkgtools/pkglint/files/check_test.go:1.49
--- pkgsrc/pkgtools/pkglint/files/check_test.go:1.48 Sun Aug 25 21:44:37 2019
+++ pkgsrc/pkgtools/pkglint/files/check_test.go Tue Oct 1 21:37:59 2019
@@ -125,6 +125,7 @@ func Test(t *testing.T) { check.TestingT
type Tester struct {
c *check.C // Only usable during the test method itself
testName string
+ argv []string // from the last invocation of Tester.SetUpCommandLine
stdout bytes.Buffer
stderr bytes.Buffer
@@ -149,7 +150,9 @@ func (t *Tester) SetUpCommandLine(args .
prevTracing := trace.Tracing
defer func() { trace.Tracing = prevTracing }()
- exitcode := G.ParseCommandLine(append([]string{"pkglint"}, args...))
+ argv := append([]string{"pkglint"}, args...)
+ t.argv = argv
+ exitcode := G.ParseCommandLine(argv)
if exitcode != -1 && exitcode != 0 {
t.CheckOutputEmpty()
t.c.Fatalf("Cannot parse command line: %#v", args)
@@ -516,7 +519,7 @@ func (t *Tester) File(relativeFileName s
if t.relCwd != "" {
return path.Clean(relativeFileName)
}
- return path.Clean(t.tmpdir + "/" + relativeFileName)
+ return path.Clean(joinPath(t.tmpdir, relativeFileName))
}
// Copy copies a file inside the temporary directory.
@@ -1155,3 +1158,12 @@ func (t *Tester) CheckFileLinesDetab(rel
// development.
func (t *Tester) Use(functions ...interface{}) {
}
+
+func (t *Tester) Shquote(format string, rels ...string) string {
+ var subs []interface{}
+ for _, rel := range rels {
+ quoted := shquote(path.Join(t.tmpdir, rel))
+ subs = append(subs, strings.Replace(quoted, t.tmpdir, "~", -1))
+ }
+ return sprintf(format, subs...)
+}
Index: pkgsrc/pkgtools/pkglint/files/mklines_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.48 pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.49
--- pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.48 Tue Jul 30 18:16:13 2019
+++ pkgsrc/pkgtools/pkglint/files/mklines_test.go Tue Oct 1 21:37:59 2019
@@ -716,6 +716,7 @@ func (s *Suite) Test_MkLines_Check__inde
mklines.Check()
t.CheckOutputLines(
+ "ERROR: ~/module.mk:3: There is no package in \"category/package\".",
"NOTE: ~/module.mk:5: This directive should be indented by 2 spaces.",
"NOTE: ~/module.mk:7: This directive should be indented by 2 spaces.")
}
Index: pkgsrc/pkgtools/pkglint/files/distinfo.go
diff -u pkgsrc/pkgtools/pkglint/files/distinfo.go:1.35 pkgsrc/pkgtools/pkglint/files/distinfo.go:1.36
--- pkgsrc/pkgtools/pkglint/files/distinfo.go:1.35 Sun Jun 30 20:56:19 2019
+++ pkgsrc/pkgtools/pkglint/files/distinfo.go Tue Oct 1 21:37:59 2019
@@ -67,7 +67,7 @@ func (ck *distinfoLinesChecker) parse()
return no
case ck.pkg == nil:
return unknown
- case fileExists(ck.pkg.File(ck.patchdir + "/" + prevFilename)):
+ case fileExists(ck.pkg.File(joinPath(ck.patchdir, prevFilename))):
return yes
default:
return no
@@ -195,7 +195,7 @@ func (ck *distinfoLinesChecker) checkAlg
distdir := G.Pkgsrc.File("distfiles")
- distfile := cleanpath(distdir + "/" + info.filename())
+ distfile := cleanpath(joinPath(distdir, info.filename()))
if !fileExists(distfile) {
// It's a rare situation that the explanation is generated
@@ -315,7 +315,7 @@ func (ck *distinfoLinesChecker) checkUnr
if file.Mode().IsRegular() && ck.infos[patchName].isPatch != yes && hasPrefix(patchName, "patch-") {
line := NewLineWhole(ck.lines.Filename)
line.Errorf("Patch %q is not recorded. Run %q.",
- line.PathToFile(ck.pkg.File(ck.patchdir+"/"+patchName)),
+ line.PathToFile(ck.pkg.File(joinPath(ck.patchdir, patchName))),
bmake("makepatchsum"))
}
}
@@ -371,7 +371,7 @@ func (ck *distinfoLinesChecker) checkUnc
hash := info.hash
line := info.line
- patchFileName := ck.patchdir + "/" + patchName
+ patchFileName := joinPath(ck.patchdir, patchName)
resolvedPatchFileName := ck.pkg.File(patchFileName)
if ck.distinfoIsCommitted && !isCommitted(resolvedPatchFileName) {
line.Warnf("%s is registered in distinfo but not added to CVS.", line.PathToFile(resolvedPatchFileName))
Index: pkgsrc/pkgtools/pkglint/files/pkgsrc.go
diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.35 pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.36
--- pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.35 Sun Sep 8 22:47:47 2019
+++ pkgsrc/pkgtools/pkglint/files/pkgsrc.go Tue Oct 1 21:37:59 2019
@@ -264,7 +264,7 @@ func (src *Pkgsrc) checkToplevelUnusedLi
for _, licenseFile := range src.ReadDir("licenses") {
licenseName := licenseFile.Name()
if !G.InterPackage.LicenseUsed(licenseName) {
- licensePath := licensesDir + "/" + licenseName
+ licensePath := joinPath(licensesDir, licenseName)
NewLineWhole(licensePath).Warnf("This license seems to be unused.")
}
}
@@ -597,7 +597,7 @@ func (src *Pkgsrc) loadDocChanges() {
src.LastChange = make(map[string]*Change)
for _, filename := range filenames {
- changes := src.loadDocChangesFromFile(docDir + "/" + filename)
+ changes := src.loadDocChangesFromFile(joinPath(docDir, filename))
for _, change := range changes {
src.LastChange[change.Pkgpath] = change
if change.Action == Renamed || change.Action == Moved {
@@ -841,7 +841,7 @@ func (src *Pkgsrc) ReadDir(dirName strin
var relevantFiles []os.FileInfo
for _, dirent := range files {
name := dirent.Name()
- if !dirent.IsDir() || !isIgnoredFilename(name) && !isEmptyDir(dir+"/"+name) {
+ if !dirent.IsDir() || !isIgnoredFilename(name) && !isEmptyDir(joinPath(dir, name)) {
relevantFiles = append(relevantFiles, dirent)
}
}
@@ -855,7 +855,7 @@ func (src *Pkgsrc) ReadDir(dirName strin
// NewPkgsrc("/usr/pkgsrc").File("distfiles") => "/usr/pkgsrc/distfiles"
func (src *Pkgsrc) File(relativeName string) string {
// TODO: Package.File resolves variables, Pkgsrc.File doesn't. They should behave the same.
- return cleanpath(src.topdir + "/" + relativeName)
+ return cleanpath(joinPath(src.topdir, relativeName))
}
// ToRel returns the path of `filename`, relative to the pkgsrc top directory.
@@ -988,12 +988,12 @@ func (src *Pkgsrc) guessVariableType(var
varbase := varnameBase(varname)
switch {
case hasSuffix(varbase, "DIRS"):
- return listType(BtPathmask, aclpAllRuntime)
+ return listType(BtPathPattern, aclpAllRuntime)
case hasSuffix(varbase, "DIR") && !hasSuffix(varbase, "DESTDIR"), hasSuffix(varname, "_HOME"):
// TODO: hasSuffix(varbase, "BASE")
return plainType(BtPathname, aclpAllRuntime)
case hasSuffix(varbase, "FILES"):
- return listType(BtPathmask, aclpAllRuntime)
+ return listType(BtPathPattern, aclpAllRuntime)
case hasSuffix(varbase, "FILE"):
return plainType(BtPathname, aclpAllRuntime)
case hasSuffix(varbase, "PATH"):
@@ -1020,7 +1020,7 @@ func (src *Pkgsrc) guessVariableType(var
// TODO: Add BtGuard for inclusion guards, since these variables may only be checked using defined().
return plainType(BtUnknown, aclpAll)
case hasSuffix(varbase, "_SKIP"):
- return listType(BtPathmask, aclpAllRuntime)
+ return listType(BtPathPattern, aclpAllRuntime)
}
// Variables whose name doesn't match the above patterns may be
Index: pkgsrc/pkgtools/pkglint/files/distinfo_test.go
diff -u pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.32 pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.33
--- pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.32 Sun Jul 14 21:25:47 2019
+++ pkgsrc/pkgtools/pkglint/files/distinfo_test.go Tue Oct 1 21:37:59 2019
@@ -177,7 +177,7 @@ func (s *Suite) Test_distinfoLinesChecke
"",
".include \"../mk/misc/category.mk\"")
- t.Main("-r", "-Wall", "-Call", t.File("."))
+ t.Main("-r", "-Wall", "-Call", ".")
t.CheckOutputLines(
"ERROR: ~/category/package1/distinfo:3: "+
@@ -201,7 +201,7 @@ func (s *Suite) Test_distinfoLinesChecke
"WARN: ~/licenses/gnu-gpl-v2: This license seems to be unused.",
"8 errors and 1 warning found.",
- "(Run \"pkglint -e\" to show explanations.)")
+ t.Shquote("(Run \"pkglint -e -r -Wall -Call %s\" to show explanations.)", "."))
// Ensure that hex.DecodeString does not waste memory here.
t.CheckEquals(len(G.InterPackage.hashes["SHA512:distfile-1.0.tar.gz"].hash), 8)
Index: pkgsrc/pkgtools/pkglint/files/mkparser_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.32 pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.33
--- pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.32 Sun Sep 8 22:47:47 2019
+++ pkgsrc/pkgtools/pkglint/files/mkparser_test.go Tue Oct 1 21:37:59 2019
@@ -378,6 +378,24 @@ func (s *Suite) Test_MkParser_VarUse(c *
varuseText("${${", "${"),
"WARN: Test_MkParser_VarUse.mk:1: Missing closing \"}\" for \"\".",
"WARN: Test_MkParser_VarUse.mk:1: Missing closing \"}\" for \"${\".")
+
+ test("${arbitrary :Mpattern:---:Q}",
+ varuseText("${arbitrary :Mpattern:---:Q}", "arbitrary ", "Mpattern", "Q"),
+ // TODO: Swap the order of these message
+ "WARN: Test_MkParser_VarUse.mk:1: Invalid variable modifier \"---\" for \"arbitrary \".",
+ "WARN: Test_MkParser_VarUse.mk:1: Invalid part \" \" after variable name \"arbitrary\".")
+
+ // Variable names containing spaces do not occur in pkgsrc.
+ // Technically they are possible:
+ //
+ // VARNAME= name with spaces
+ // ${VARNAME}= value
+ //
+ // all:
+ // @echo ${name with spaces:Q}''
+ test("${arbitrary text}",
+ varuse("arbitrary text"),
+ "WARN: Test_MkParser_VarUse.mk:1: Invalid part \" text\" after variable name \"arbitrary\".")
}
func (s *Suite) Test_MkParser_varUseModifier__invalid_ts_modifier_with_warning(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/licenses_test.go
diff -u pkgsrc/pkgtools/pkglint/files/licenses_test.go:1.23 pkgsrc/pkgtools/pkglint/files/licenses_test.go:1.24
--- pkgsrc/pkgtools/pkglint/files/licenses_test.go:1.23 Sat Apr 20 17:43:24 2019
+++ pkgsrc/pkgtools/pkglint/files/licenses_test.go Tue Oct 1 21:37:59 2019
@@ -9,39 +9,38 @@ func (s *Suite) Test_LicenseChecker_Chec
t.CreateFileLines("licenses/gnu-gpl-v2",
"The licenses for most software are designed to take away ...")
- mkline := t.NewMkLine("Makefile", 7, "LICENSE=dummy")
- licenseChecker := LicenseChecker{nil, mkline}
- licenseChecker.Check("gpl-v2", opAssign)
+ test := func(licenseValue string, diagnostics ...string) {
+ mklines := t.NewMkLines("Makefile",
+ "LICENSE=\t"+licenseValue)
- t.CheckOutputLines(
- "WARN: Makefile:7: License file ~/licenses/gpl-v2 does not exist.")
-
- licenseChecker.Check("no-profit shareware", opAssign)
+ mklines.ForEach(func(mkline *MkLine) {
+ (&LicenseChecker{mklines, mkline}).Check(mkline.Value(), opAssign)
+ })
- t.CheckOutputLines(
- "ERROR: Makefile:7: Parse error for license condition \"no-profit shareware\".")
+ t.CheckOutput(diagnostics)
+ }
- licenseChecker.Check("no-profit AND shareware", opAssign)
+ test("gpl-v2",
+ "WARN: Makefile:1: License file ~/licenses/gpl-v2 does not exist.")
- t.CheckOutputLines(
- "WARN: Makefile:7: License file ~/licenses/no-profit does not exist.",
- "ERROR: Makefile:7: License \"no-profit\" must not be used.",
- "WARN: Makefile:7: License file ~/licenses/shareware does not exist.",
- "ERROR: Makefile:7: License \"shareware\" must not be used.")
+ test("no-profit shareware",
+ "ERROR: Makefile:1: Parse error for license condition \"no-profit shareware\".")
- licenseChecker.Check("gnu-gpl-v2", opAssign)
+ test("no-profit AND shareware",
+ "WARN: Makefile:1: License file ~/licenses/no-profit does not exist.",
+ "ERROR: Makefile:1: License \"no-profit\" must not be used.",
+ "WARN: Makefile:1: License file ~/licenses/shareware does not exist.",
+ "ERROR: Makefile:1: License \"shareware\" must not be used.")
- t.CheckOutputEmpty()
-
- licenseChecker.Check("gnu-gpl-v2 AND gnu-gpl-v2 OR gnu-gpl-v2", opAssign)
-
- t.CheckOutputLines(
- "ERROR: Makefile:7: AND and OR operators in license conditions can only be combined using parentheses.")
+ test("gnu-gpl-v2",
+ nil...)
- licenseChecker.Check("(gnu-gpl-v2 OR gnu-gpl-v2) AND gnu-gpl-v2", opAssign)
+ test("gnu-gpl-v2 AND gnu-gpl-v2 OR gnu-gpl-v2",
+ "ERROR: Makefile:1: AND and OR operators in license conditions can only be combined using parentheses.")
- t.CheckOutputEmpty()
+ test("(gnu-gpl-v2 OR gnu-gpl-v2) AND gnu-gpl-v2",
+ nil...)
}
func (s *Suite) Test_LicenseChecker_checkName__LICENSE_FILE(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/logging.go
diff -u pkgsrc/pkgtools/pkglint/files/logging.go:1.27 pkgsrc/pkgtools/pkglint/files/logging.go:1.28
--- pkgsrc/pkgtools/pkglint/files/logging.go:1.27 Thu Sep 12 21:15:48 2019
+++ pkgsrc/pkgtools/pkglint/files/logging.go Tue Oct 1 21:37:59 2019
@@ -6,6 +6,7 @@ import (
"netbsd.org/pkglint/histogram"
"netbsd.org/pkglint/textproc"
"path"
+ "strings"
)
type Logger struct {
@@ -118,7 +119,7 @@ func (l *Logger) Explain(explanation ...
l.out.WriteLine("")
}
-func (l *Logger) ShowSummary() {
+func (l *Logger) ShowSummary(args []string) {
if l.Opts.Quiet || l.Opts.Autofix {
return
}
@@ -147,14 +148,22 @@ func (l *Logger) ShowSummary() {
l.out.WriteLine("Looks fine.")
}
+ commandLine := func(arg string) string {
+ argv := append([]string{args[0], arg}, args[1:]...)
+ for i := range argv {
+ argv[i] = shquote(argv[i])
+ }
+ return strings.Join(argv, " ")
+ }
+
if l.explanationsAvailable && !l.Opts.Explain {
- l.out.WriteLine("(Run \"pkglint -e\" to show explanations.)")
+ l.out.WriteLine(sprintf("(Run \"%s\" to show explanations.)", commandLine("-e")))
}
if l.autofixAvailable {
if !l.Opts.ShowAutofix {
- l.out.WriteLine("(Run \"pkglint -fs\" to show what can be fixed automatically.)")
+ l.out.WriteLine(sprintf("(Run \"%s\" to show what can be fixed automatically.)", commandLine("-fs")))
}
- l.out.WriteLine("(Run \"pkglint -F\" to automatically fix some issues.)")
+ l.out.WriteLine(sprintf("(Run \"%s\" to automatically fix some issues.)", commandLine("-F")))
}
}
Index: pkgsrc/pkgtools/pkglint/files/substcontext.go
diff -u pkgsrc/pkgtools/pkglint/files/substcontext.go:1.27 pkgsrc/pkgtools/pkglint/files/substcontext.go:1.28
--- pkgsrc/pkgtools/pkglint/files/substcontext.go:1.27 Sun Jul 14 21:25:47 2019
+++ pkgsrc/pkgtools/pkglint/files/substcontext.go Tue Oct 1 21:37:59 2019
@@ -251,7 +251,7 @@ func (ctx *SubstContext) suggestSubstVar
tokens, _ := splitIntoShellTokens(mkline.Line, mkline.Value())
for _, token := range tokens {
- varname := ctx.extractVarname(mkline.UnquoteShell(token))
+ varname := ctx.extractVarname(mkline.UnquoteShell(token, false))
if varname == "" {
continue
}
Index: pkgsrc/pkgtools/pkglint/files/logging_test.go
diff -u pkgsrc/pkgtools/pkglint/files/logging_test.go:1.20 pkgsrc/pkgtools/pkglint/files/logging_test.go:1.21
--- pkgsrc/pkgtools/pkglint/files/logging_test.go:1.20 Thu Sep 12 21:15:48 2019
+++ pkgsrc/pkgtools/pkglint/files/logging_test.go Tue Oct 1 21:37:59 2019
@@ -591,7 +591,7 @@ func (s *Suite) Test_Logger_ShowSummary_
// Neither the warning nor the corresponding explanation are logged.
line.Warnf("Filtered warning.")
line.Explain("Explanation for the above warning.")
- G.Logger.ShowSummary()
+ G.Logger.ShowSummary(t.argv)
// Since the above warning is filtered out by the --only option,
// adding --explain to the options would not show any explanation.
@@ -603,13 +603,13 @@ func (s *Suite) Test_Logger_ShowSummary_
line.Warnf("This warning is interesting.")
line.Explain("This explanation is available.")
- G.Logger.ShowSummary()
+ G.Logger.ShowSummary(t.argv)
t.CheckEquals(G.Logger.explanationsAvailable, true)
t.CheckOutputLines(
"WARN: Makefile:27: This warning is interesting.",
"1 warning found.",
- "(Run \"pkglint -e\" to show explanations.)")
+ "(Run \"pkglint -e --only interesting\" to show explanations.)")
}
func (s *Suite) Test_Logger_ShowSummary__looks_fine(c *check.C) {
@@ -617,7 +617,7 @@ func (s *Suite) Test_Logger_ShowSummary_
logger := Logger{out: NewSeparatorWriter(&t.stdout)}
- logger.ShowSummary()
+ logger.ShowSummary([]string{"pkglint"})
t.CheckOutputLines(
"Looks fine.")
@@ -630,7 +630,7 @@ func (s *Suite) Test_Logger_ShowSummary_
logger.Logf(Error, "", "", ".", ".")
logger.Logf(Warn, "", "", ".", ".")
- logger.ShowSummary()
+ logger.ShowSummary([]string{"pkglint"})
t.CheckOutputLines(
"ERROR: .",
@@ -648,7 +648,7 @@ func (s *Suite) Test_Logger_ShowSummary_
logger.Logf(Warn, "", "", "4.", "4.")
logger.Logf(Warn, "", "", "5.", "5.")
- logger.ShowSummary()
+ logger.ShowSummary([]string{"pkglint"})
t.CheckOutputLines(
"ERROR: 1.",
@@ -665,7 +665,7 @@ func (s *Suite) Test_Logger_ShowSummary_
logger := Logger{out: NewSeparatorWriter(&t.stdout)}
logger.Opts.Quiet = true
- logger.ShowSummary()
+ logger.ShowSummary([]string{"pkglint"})
t.CheckOutputEmpty()
}
@@ -678,7 +678,7 @@ func (s *Suite) Test_Logger_ShowSummary_
logger.Logf(Error, "", "", ".", ".")
logger.Logf(Warn, "", "", ".", ".")
- logger.ShowSummary()
+ logger.ShowSummary([]string{"pkglint"})
t.CheckOutputLines(
"ERROR: .",
@@ -693,7 +693,7 @@ func (s *Suite) Test_Logger_ShowSummary_
logger.Explain(
"Explanation.")
- logger.ShowSummary()
+ logger.ShowSummary([]string{"pkglint"})
t.CheckOutputLines(
"ERROR: .",
@@ -712,7 +712,7 @@ func (s *Suite) Test_Logger_ShowSummary_
// Since the --explain option is already given, it need not be advertised.
logger.Opts.Explain = true
- logger.ShowSummary()
+ logger.ShowSummary([]string{"pkglint"})
t.CheckOutputLines(
"ERROR: .",
@@ -725,7 +725,7 @@ func (s *Suite) Test_Logger_ShowSummary_
logger := Logger{out: NewSeparatorWriter(&t.stdout)}
logger.autofixAvailable = true // See SaveAutofixChanges
- logger.ShowSummary()
+ logger.ShowSummary([]string{"pkglint"})
t.CheckOutputLines(
"Looks fine.",
@@ -740,7 +740,7 @@ func (s *Suite) Test_Logger_ShowSummary_
logger.autofixAvailable = true // See SaveAutofixChanges
logger.Opts.ShowAutofix = true
- logger.ShowSummary()
+ logger.ShowSummary([]string{"pkglint"})
// Since the --show-autofix option is already given, it need not be advertised.
// But the --autofix option is not given, therefore mention it.
@@ -756,7 +756,7 @@ func (s *Suite) Test_Logger_ShowSummary_
logger.autofixAvailable = true // See SaveAutofixChanges
logger.Opts.Autofix = true
- logger.ShowSummary()
+ logger.ShowSummary([]string{"pkglint"})
// Since the --autofix option is already given, it need not be advertised.
// Mentioning the --show-autofix option would be pointless here since the
@@ -766,6 +766,20 @@ func (s *Suite) Test_Logger_ShowSummary_
t.CheckOutputEmpty()
}
+func (s *Suite) Test_Logger_ShowSummary__quoting(c *check.C) {
+ t := s.Init(c)
+
+ logger := Logger{out: NewSeparatorWriter(&t.stdout)}
+ logger.errors = 1
+ logger.explanationsAvailable = true
+
+ logger.ShowSummary([]string{"pkglint", "--only", "string with 'quotes'"})
+
+ t.CheckOutputLines(
+ "1 error found.",
+ "(Run \"pkglint -e --only 'string with '\\''quotes'\\'''\" to show explanations.)")
+}
+
// In rare cases, the explanations for the same warning may differ
// when they appear in different contexts. In such a case, if the
// warning is suppressed, the explanation must not appear on its own.
@@ -931,7 +945,7 @@ func (s *Suite) Test_Logger_Diag__source
t.SetUpPackage("category/package2",
"PATCHDIR=\t../../category/dependency/patches")
- t.Main("--source", "-Wall", t.File("category/package1"), t.File("category/package2"))
+ t.Main("--source", "-Wall", "category/package1", "category/package2")
t.CheckOutputLines(
"ERROR: ~/category/package1/distinfo: "+
@@ -947,7 +961,8 @@ func (s *Suite) Test_Logger_Diag__source
"Run \""+confMake+" makepatchsum\".",
"",
"3 errors found.",
- "(Run \"pkglint -e\" to show explanations.)")
+ t.Shquote("(Run \"pkglint -e --source -Wall %s %s\" to show explanations.)",
+ "category/package1", "category/package2"))
}
func (s *Suite) Test_Logger_shallBeLogged(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/toplevel.go
diff -u pkgsrc/pkgtools/pkglint/files/toplevel.go:1.20 pkgsrc/pkgtools/pkglint/files/toplevel.go:1.21
--- pkgsrc/pkgtools/pkglint/files/toplevel.go:1.20 Sun Jul 14 21:25:47 2019
+++ pkgsrc/pkgtools/pkglint/files/toplevel.go Tue Oct 1 21:37:59 2019
@@ -45,7 +45,7 @@ func (ctx *Toplevel) checkSubdir(mkline
}
}
- if containsVarRef(subdir) || !fileExists(ctx.dir+"/"+subdir+"/Makefile") {
+ if containsVarRef(subdir) || !fileExists(joinPath(ctx.dir, subdir, "Makefile")) {
return
}
@@ -63,6 +63,6 @@ func (ctx *Toplevel) checkSubdir(mkline
ctx.previousSubdir = subdir
if !mkline.IsCommentedVarassign() {
- ctx.subdirs = append(ctx.subdirs, ctx.dir+"/"+subdir)
+ ctx.subdirs = append(ctx.subdirs, joinPath(ctx.dir, subdir))
}
}
Index: pkgsrc/pkgtools/pkglint/files/mkline.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline.go:1.58 pkgsrc/pkgtools/pkglint/files/mkline.go:1.59
--- pkgsrc/pkgtools/pkglint/files/mkline.go:1.58 Thu Sep 12 21:15:48 2019
+++ pkgsrc/pkgtools/pkglint/files/mkline.go Tue Oct 1 21:37:59 2019
@@ -549,40 +549,81 @@ var notSpace = textproc.Space.Inverse()
//
// Compare devel/bmake/files/str.c, function brk_string.
//
-// TODO: Compare with brk_string from devel/bmake, especially for backticks.
+// See UnquoteShell.
func (mkline *MkLine) ValueFields(value string) []string {
- if trace.Tracing {
- defer trace.Call(mkline, value)()
- }
+ var fields []string
+ var field strings.Builder
- p := NewShTokenizer(mkline.Line, value, false)
- atoms := p.ShAtoms()
+ lexer := NewMkTokensLexer(mkline.Tokenize(value, false))
+ lexer.SkipHspace()
- if len(atoms) > 0 && atoms[0].Type == shtSpace {
- atoms = atoms[1:]
+ emit := func() {
+ if field.Len() > 0 {
+ fields = append(fields, field.String())
+ field.Reset()
+ }
}
- var word strings.Builder
- var words []string
- for _, atom := range atoms {
- if atom.Type == shtSpace && atom.Quoting == shqPlain {
- words = append(words, word.String())
- word.Reset()
+ plain := func() {
+ varUse := lexer.NextVarUse()
+ if varUse != nil {
+ field.WriteString(varUse.Text)
} else {
- word.WriteString(atom.MkText)
+ field.WriteByte(lexer.NextByte())
}
}
- if word.Len() > 0 && atoms[len(atoms)-1].Quoting == shqPlain {
- words = append(words, word.String())
- word.Reset()
- }
- // TODO: Handle parse errors
- word.WriteString(p.parser.Rest())
- rest := word.String()
- _ = rest
+ for !lexer.EOF() {
+ switch {
+ case lexer.SkipByte('\''):
+ // Note: bmake's brk_string treats single quotes and double
+ // quotes in the same way regarding backslash escape sequences.
+ // It seems this is a mistake, and until this is confirmed to
+ // not be a bug, pkglint parses single quotes like in the shell.
+ field.WriteByte('\'')
+ for {
+ if lexer.EOF() {
+ return fields // without the incomplete last field
+ } else if lexer.SkipByte('\'') {
+ field.WriteByte('\'')
+ break
+ } else {
+ plain()
+ }
+ }
+
+ case lexer.SkipByte('"'):
+ field.WriteByte('"')
+ for {
+ if lexer.EOF() {
+ return fields // without the incomplete last field
+ } else if lexer.SkipByte('"') {
+ field.WriteByte('"')
+ break
+ } else if lexer.SkipByte('\\') {
+ field.WriteByte('\\')
+ plain()
+ } else {
+ plain()
+ }
+ }
+
+ case lexer.SkipByte(' '), lexer.SkipByte('\t'), lexer.SkipByte('\n'):
+ emit()
+
+ case lexer.SkipByte('\\'):
+ field.WriteByte('\\')
+ if !lexer.EOF() {
+ plain()
+ }
+
+ default:
+ plain()
+ }
+ }
+ emit()
- return words
+ return fields
}
func (mkline *MkLine) ValueTokens() ([]*MkToken, string) {
@@ -1093,47 +1134,75 @@ func (mkline *MkLine) ForEachUsed(action
}
}
-func (*MkLine) UnquoteShell(str string) string {
+// UnquoteShell removes one level of double and single quotes,
+// like in the shell.
+//
+// See ValueFields.
+func (mkline *MkLine) UnquoteShell(str string, warn bool) string {
var sb strings.Builder
- n := len(str)
+ lexer := NewMkTokensLexer(mkline.Tokenize(str, false))
+
+ plain := func() {
+ varUse := lexer.NextVarUse()
+ if varUse != nil {
+ sb.WriteString(varUse.Text)
+ } else {
+ sb.WriteByte(lexer.NextByte())
+ }
+ }
outer:
- for i := 0; i < n; i++ {
- switch str[i] {
- case '"':
- for i++; i < n; i++ {
- switch str[i] {
- case '"':
+ for !lexer.EOF() {
+ switch {
+ case lexer.SkipByte('"'):
+ for !lexer.EOF() {
+ if lexer.SkipByte('"') {
continue outer
- case '\\':
- i++
- if i < n {
- sb.WriteByte(str[i])
+ } else if lexer.SkipByte('\\') {
+ if !lexer.EOF() {
+ plain()
}
- default:
- sb.WriteByte(str[i])
+ } else {
+ plain()
}
}
- case '\'':
- for i++; i < n && str[i] != '\''; i++ {
- sb.WriteByte(str[i])
+ case lexer.SkipByte('\''):
+ for !lexer.EOF() && !lexer.SkipByte('\'') {
+ plain()
}
- case '\\':
- i++
- if i < n {
- sb.WriteByte(str[i])
+ case lexer.SkipByte('\\'):
+ if !lexer.EOF() {
+ plain()
}
default:
- sb.WriteByte(str[i])
+ if warn {
+ mkline.checkFileGlobbing(lexer.PeekByte(), str)
+ }
+ plain()
}
}
return sb.String()
}
+func (mkline *MkLine) checkFileGlobbing(ch int, str string) {
+ if !(ch == '*' || ch == '?' || ch == '[') {
+ return
+ }
+
+ if !mkline.once.FirstTimeSlice("unintended file globbing", string(ch)) {
+ return
+ }
+
+ mkline.Warnf("The %q in the word %q may lead to unintended file globbing.",
+ string(ch), str)
+ mkline.Explain(
+ "To fix this, enclose the word in \"double\" or 'single' quotes.")
+}
+
type MkOperator uint8
const (
Index: pkgsrc/pkgtools/pkglint/files/mkline_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.67 pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.68
--- pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.67 Thu Sep 12 21:15:48 2019
+++ pkgsrc/pkgtools/pkglint/files/mkline_test.go Tue Oct 1 21:37:59 2019
@@ -1048,6 +1048,31 @@ func (s *Suite) Test_MkLine_VariableNeed
t.CheckOutputEmpty()
}
+func (s *Suite) Test_MkLine_VariableNeedsQuoting__only_D_modifier(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpVartypes()
+
+ mklines := t.SetUpFileMkLines("Makefile",
+ MkCvsID,
+ "",
+ "SUBST_CLASSES+=\t\turl2pkg",
+ "SUBST_STAGE.url2pkg=\tpost-configure",
+ "SUBST_FILES.url2pkg=\t*.in",
+ "SUBST_SED.url2pkg=\t-e 's,@PKGSRCDIR@,${BATCH:D${PKGSRCDIR}},'")
+
+ mklines.Check()
+
+ // Since the value of the BATCH variable does not appear in the output,
+ // there should be no warning saying that "BATCH should be quoted".
+ // If any, the variable PKGSRCDIR should be quoted, but that is a safe
+ // variable since it is a pkgsrc-specific directory and it appears as
+ // part of a word, therefore it cannot result in an empty string.
+ // FIXME: Don't warn in this situation.
+ t.CheckOutputLines(
+ "WARN: ~/Makefile:6: The variable BATCH should be quoted as part of a shell word.")
+}
+
// As of October 2018, these examples from real pkgsrc end up in the
// final "unknown" case.
func (s *Suite) Test_MkLine_VariableNeedsQuoting__uncovered_cases(c *check.C) {
@@ -1338,6 +1363,56 @@ func (s *Suite) Test_MkLine_ValueFields(
"${VAR2}two",
"words;;;",
"'word three'")
+
+ test("\"double quotes\" group words",
+ "\"double quotes\"",
+ "group",
+ "words")
+
+ test("\"unfinished",
+ nil...) // the rest is silently discarded
+
+ test("'single quotes' group words",
+ "'single quotes'",
+ "group",
+ "words")
+
+ test("'unfinished",
+ nil...) // the rest is silently discarded
+
+ // This is how it works in bmake.
+ test("'\\' ' end",
+ "'\\'") // the "' end" is silently discarded
+
+ // This is how it works in pkglint.
+ test("'\\' end",
+ "'\\'",
+ "end")
+
+ test("`backticks do not group words`",
+ "`backticks",
+ "do",
+ "not",
+ "group",
+ "words`")
+
+ test("plain${VAR}plain",
+ "plain${VAR}plain")
+
+ test("\"${DOUBLE}\" \"\\${DOUBLE}\"",
+ "\"${DOUBLE}\"",
+ "\"\\${DOUBLE}\"")
+
+ test("'${SINGLE}' '\\${SINGLE}'",
+ "'${SINGLE}'",
+ "'\\${SINGLE}'")
+
+ test("\"\"''\"\"",
+ "\"\"''\"\"")
+
+ test("$@ $<",
+ "$@",
+ "$<")
}
// Before 2018-11-26, this test panicked.
@@ -2024,9 +2099,11 @@ func (s *Suite) Test_MkLine_ForEachUsed(
func (s *Suite) Test_MkLine_UnquoteShell(c *check.C) {
t := s.Init(c)
- test := func(input, output string) {
- unquoted := (*MkLine).UnquoteShell(nil, input)
+ test := func(input, output string, diagnostics ...string) {
+ mkline := t.NewMkLine("filename.mk", 1, "")
+ unquoted := mkline.UnquoteShell(input, true)
t.CheckEquals(unquoted, output)
+ t.CheckOutput(diagnostics)
}
test("", "")
@@ -2046,8 +2123,28 @@ func (s *Suite) Test_MkLine_UnquoteShell
test("\\", "")
test("\"\\", "")
test("'", "")
- test("\"$(\"", "$(")
+
+ test("\"$(\"", "$(\"",
+ "WARN: filename.mk:1: Missing closing \")\" for \"\\\"\".",
+ "WARN: filename.mk:1: Invalid part \"\\\"\" after variable name \"\".")
+
test("`", "`")
+
+ // Quotes inside a varuse are not unquoted.
+ test("${VAR}", "${VAR}")
+ test("${VAR:S,',',g}", "${VAR:S,',',g}")
+
+ test("\"*?[\"", "*?[")
+ test("'*?['", "*?[")
+
+ test("*?[", "*?[",
+ "WARN: filename.mk:1: The \"*\" in the word \"*?[\" may lead to unintended file globbing.",
+ "WARN: filename.mk:1: The \"?\" in the word \"*?[\" may lead to unintended file globbing.",
+ "WARN: filename.mk:1: The \"[\" in the word \"*?[\" may lead to unintended file globbing.")
+
+ test("'single'*\"double\"", "single*double",
+ "WARN: filename.mk:1: The \"*\" in the word \"'single'*\\\"double\\\"\" "+
+ "may lead to unintended file globbing.")
}
func (s *Suite) Test_MkLineParser_unescapeComment(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.46 pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.47
--- pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.46 Thu Sep 12 21:15:48 2019
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker.go Tue Oct 1 21:37:59 2019
@@ -1285,6 +1285,29 @@ func (ck MkLineChecker) checkVarassignMi
mkline.Notef("Consider setting NOT_FOR_PLATFORM instead of " +
"PKG_SKIP_REASON depending on ${OPSYS}.")
}
+
+ ck.checkVarassignMiscRedundantInstallationDirs()
+}
+
+func (ck MkLineChecker) checkVarassignMiscRedundantInstallationDirs() {
+ mkline := ck.MkLine
+ varname := mkline.Varname()
+
+ switch {
+ case G.Pkg == nil,
+ varname != "INSTALLATION_DIRS",
+ !matches(G.Pkg.vars.LastValue("AUTO_MKDIRS"), `^[Yy][Ee][Ss]$`):
+ return
+ }
+
+ for _, dir := range mkline.ValueFields(mkline.Value()) {
+ if G.Pkg.Plist.Dirs[dir] != nil {
+ mkline.Notef("The directory %q is redundant in %s.", dir, varname)
+ mkline.Explain(
+ "This package defines AUTO_MKDIR, and the directory is contained in the PLIST.",
+ "Therefore it will be created anyways.")
+ }
+ }
}
func (ck MkLineChecker) checkVarassignLeftBsdPrefs() {
@@ -1587,14 +1610,6 @@ func (ck MkLineChecker) simplifyConditio
quote := condStr(matches(pattern, `[^\-/0-9@A-Za-z]`), "\"", "")
to := "${" + varname + "} " + op + " " + quote + pattern + quote
-
- // TODO: Check in more cases whether the parentheses are really necessary.
- // In a !!${VAR} expression, parentheses are necessary.
- needParen := !toplevel
- if needParen {
- to = "(" + to + ")"
- }
-
return from, to
}
@@ -1618,6 +1633,8 @@ func (ck MkLineChecker) simplifyConditio
continue
}
+ // FIXME: This transformation is only valid if the variable is guaranteed to
+ // be defined. If that's not the case, the :U modifier must be added.
fix := ck.MkLine.Autofix()
fix.Notef("%s should be compared using %s instead of matching against %q.",
varname, condStr(positive == notEmpty, "==", "!="), ":"+modifier.Text)
@@ -1735,7 +1752,7 @@ func (ck MkLineChecker) CheckRelativePat
return
}
- abs := path.Dir(mkline.Filename) + "/" + resolvedPath
+ abs := joinPath(path.Dir(mkline.Filename), resolvedPath)
if _, err := os.Stat(abs); err != nil {
if mustExist && !ck.MkLines.indentation.HasExists(resolvedPath) {
mkline.Errorf("Relative path %q does not exist.", resolvedPath)
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.42 pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.43
--- pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.42 Thu Sep 12 21:15:48 2019
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go Tue Oct 1 21:37:59 2019
@@ -1323,9 +1323,9 @@ func (s *Suite) Test_MkLineChecker_check
func (s *Suite) Test_MkLineChecker_checkVarusePermissions__load_time_in_condition(c *check.C) {
t := s.Init(c)
- G.Pkgsrc.vartypes.DefineParse("LOAD_TIME", BtPathmask, List,
+ G.Pkgsrc.vartypes.DefineParse("LOAD_TIME", BtPathPattern, List,
"special:filename.mk: use-loadtime")
- G.Pkgsrc.vartypes.DefineParse("RUN_TIME", BtPathmask, List,
+ G.Pkgsrc.vartypes.DefineParse("RUN_TIME", BtPathPattern, List,
"special:filename.mk: use")
mklines := t.NewMkLines("filename.mk",
@@ -1342,9 +1342,9 @@ func (s *Suite) Test_MkLineChecker_check
func (s *Suite) Test_MkLineChecker_checkVarusePermissions__load_time_in_for_loop(c *check.C) {
t := s.Init(c)
- G.Pkgsrc.vartypes.DefineParse("LOAD_TIME", BtPathmask, List,
+ G.Pkgsrc.vartypes.DefineParse("LOAD_TIME", BtPathPattern, List,
"special:filename.mk: use-loadtime")
- G.Pkgsrc.vartypes.DefineParse("RUN_TIME", BtPathmask, List,
+ G.Pkgsrc.vartypes.DefineParse("RUN_TIME", BtPathPattern, List,
"special:filename.mk: use")
mklines := t.NewMkLines("filename.mk",
@@ -1491,7 +1491,7 @@ func (s *Suite) Test_MkLineChecker_check
func (s *Suite) Test_MkLineChecker_checkVarusePermissions__usable_only_at_loadtime_in_other_file(c *check.C) {
t := s.Init(c)
- G.Pkgsrc.vartypes.DefineParse("VAR", BtFileName, NoVartypeOptions,
+ G.Pkgsrc.vartypes.DefineParse("VAR", BtFilename, NoVartypeOptions,
"*: set, use-loadtime")
mklines := t.NewMkLines("Makefile",
MkCvsID,
@@ -1754,6 +1754,9 @@ func (s *Suite) Test_MkLineChecker_Check
test("../../other/does-not-exist",
"ERROR: ~/category/package/Makefile:1: Relative path \"../../other/does-not-exist/Makefile\" does not exist.")
+
+ test("${OTHER_PACKAGE}",
+ nil...)
}
// PR pkg/46570, item 2
@@ -1782,24 +1785,24 @@ func (s *Suite) Test_MkLineChecker_Check
t.SetUpVartypes()
mklines := t.NewMkLines("x11/xkeyboard-config/Makefile",
- "FILES_SUBST+=XKBCOMP_SYMLINK=${${XKBBASE}/xkbcomp:L:Q}",
- "FILES_SUBST+=XKBCOMP_SYMLINK=${${XKBBASE}/xkbcomp:Q}")
+ MkCvsID,
+ "FILES_SUBST+=\tXKBCOMP_SYMLINK=${${XKBBASE}/xkbcomp:L:Q}",
+ "FILES_SUBST+=\tXKBCOMP_SYMLINK=${${XKBBASE}/xkbcomp:Q}")
- MkLineChecker{mklines, mklines.mklines[0]}.Check()
- MkLineChecker{mklines, mklines.mklines[1]}.Check()
+ mklines.Check()
- // In line 1, don't warn that ${XKBBASE}/xkbcomp is used but not defined.
+ // In line 2, don't warn that ${XKBBASE}/xkbcomp is used but not defined.
// This is because the :L modifier interprets everything before as an expression
// instead of a variable name.
//
- // In line 2 the :L modifier is missing, therefore ${XKBBASE}/xkbcomp is the
+ // In line 3 the :L modifier is missing, therefore ${XKBBASE}/xkbcomp is the
// name of another variable, and that variable is not known. Only XKBBASE is known.
//
- // In line 2, warn about the invalid "/" as part of the variable name.
+ // In line 3, warn about the invalid "/" as part of the variable name.
t.CheckOutputLines(
- "WARN: x11/xkeyboard-config/Makefile:2: "+
+ "WARN: x11/xkeyboard-config/Makefile:3: "+
"Invalid part \"/xkbcomp\" after variable name \"${XKBBASE}\".",
- "WARN: x11/xkeyboard-config/Makefile:2: XKBBASE is used but not defined.")
+ "WARN: x11/xkeyboard-config/Makefile:3: XKBBASE is used but not defined.")
}
func (s *Suite) Test_MkLineChecker_checkDirectiveCond__comparison_with_shell_command(c *check.C) {
@@ -1860,10 +1863,18 @@ func (s *Suite) Test_MkLineChecker_check
ck := MkLineChecker{mklines, mklines.mklines[1]}
t.SetUpCommandLine("-Wall")
- ck.checkDirectiveCond()
+ mklines.ForEach(func(mkline *MkLine) {
+ if mkline == mklines.mklines[1] {
+ ck.checkDirectiveCond()
+ }
+ })
t.SetUpCommandLine("-Wall", "--autofix")
- ck.checkDirectiveCond()
+ mklines.ForEach(func(mkline *MkLine) {
+ if mkline == mklines.mklines[1] {
+ ck.checkDirectiveCond()
+ }
+ })
mklines.SaveAutofixChanges()
afterMklines := t.LoadMkInclude("module.mk")
@@ -1944,18 +1955,18 @@ func (s *Suite) Test_MkLineChecker_check
// test for emptiness, therefore the diagnostics should suggest
// the != operator instead of ==.
"NOTE: module.mk:2: PKGPATH should be compared using == instead of matching against \":Mpattern\".",
- "AUTOFIX: module.mk:2: Replacing \"!empty(PKGPATH:Mpattern)\" with \"(${PKGPATH} == pattern)\".",
+ "AUTOFIX: module.mk:2: Replacing \"!empty(PKGPATH:Mpattern)\" with \"${PKGPATH} == pattern\".",
- // TODO: This condition could be simplified even more.
+ // TODO: The ! and == could be combined into a !=.
// Luckily the !! pattern doesn't occur in practice.
- ".if !(${PKGPATH} == pattern)")
+ ".if !${PKGPATH} == pattern")
test(".if empty(PKGPATH:Mpattern) || 0",
"NOTE: module.mk:2: PKGPATH should be compared using != instead of matching against \":Mpattern\".",
- "AUTOFIX: module.mk:2: Replacing \"empty(PKGPATH:Mpattern)\" with \"(${PKGPATH} != pattern)\".",
+ "AUTOFIX: module.mk:2: Replacing \"empty(PKGPATH:Mpattern)\" with \"${PKGPATH} != pattern\".",
- ".if (${PKGPATH} != pattern) || 0")
+ ".if ${PKGPATH} != pattern || 0")
// No note in this case since there is no implicit !empty around the varUse.
test(".if ${PKGPATH:Mpattern} != ${OTHER}",
@@ -1984,9 +1995,9 @@ func (s *Suite) Test_MkLineChecker_check
".if !!${PKGPATH:Mpattern}",
"NOTE: module.mk:2: PKGPATH should be compared using != instead of matching against \":Mpattern\".",
- "AUTOFIX: module.mk:2: Replacing \"!${PKGPATH:Mpattern}\" with \"(${PKGPATH} != pattern)\".",
+ "AUTOFIX: module.mk:2: Replacing \"!${PKGPATH:Mpattern}\" with \"${PKGPATH} != pattern\".",
- ".if !(${PKGPATH} != pattern)")
+ ".if !${PKGPATH} != pattern")
// This pattern with spaces doesn't make sense at all in the :M
// modifier since it can never match.
@@ -1995,7 +2006,8 @@ func (s *Suite) Test_MkLineChecker_check
test(
".if ${PKGPATH:Mpattern with spaces}",
- "WARN: module.mk:2: The pathname pattern \"pattern with spaces\" contains the invalid characters \" \".",
+ "WARN: module.mk:2: The pathname pattern \"pattern with spaces\" "+
+ "contains the invalid characters \" \".",
".if ${PKGPATH:Mpattern with spaces}")
// TODO: ".if ${PKGPATH} == \"pattern with spaces\"")
@@ -2003,7 +2015,8 @@ func (s *Suite) Test_MkLineChecker_check
test(
".if ${PKGPATH:M'pattern with spaces'}",
- "WARN: module.mk:2: The pathname pattern \"'pattern with spaces'\" contains the invalid characters \"' '\".",
+ "WARN: module.mk:2: The pathname pattern \"'pattern with spaces'\" "+
+ "contains the invalid characters \"' '\".",
".if ${PKGPATH:M'pattern with spaces'}")
// TODO: ".if ${PKGPATH} == 'pattern with spaces'")
@@ -2011,7 +2024,8 @@ func (s *Suite) Test_MkLineChecker_check
test(
".if ${PKGPATH:M&&}",
- "WARN: module.mk:2: The pathname pattern \"&&\" contains the invalid characters \"&&\".",
+ "WARN: module.mk:2: The pathname pattern \"&&\" "+
+ "contains the invalid characters \"&&\".",
".if ${PKGPATH:M&&}")
// TODO: ".if ${PKGPATH} == '&&'")
@@ -2051,11 +2065,10 @@ func (s *Suite) Test_MkLineChecker_check
"NOTE: module.mk:2: PKGPATH should be compared using == instead of matching against \":Mpath1\".",
"NOTE: module.mk:2: PKGPATH should be compared using == instead of matching against \":Mpath2\".",
- "AUTOFIX: module.mk:2: Replacing \"${PKGPATH:Mpath1}\" with \"(${PKGPATH} == path1)\".",
- "AUTOFIX: module.mk:2: Replacing \"${PKGPATH:Mpath2}\" with \"(${PKGPATH} == path2)\".",
+ "AUTOFIX: module.mk:2: Replacing \"${PKGPATH:Mpath1}\" with \"${PKGPATH} == path1\".",
+ "AUTOFIX: module.mk:2: Replacing \"${PKGPATH:Mpath2}\" with \"${PKGPATH} == path2\".",
- // TODO: remove the redundant parentheses
- ".if (${PKGPATH} == path1) || (${PKGPATH} == path2)")
+ ".if ${PKGPATH} == path1 || ${PKGPATH} == path2")
test(
".if (((((${PKGPATH:Mpath})))))",
@@ -2064,6 +2077,15 @@ func (s *Suite) Test_MkLineChecker_check
"AUTOFIX: module.mk:2: Replacing \"${PKGPATH:Mpath}\" with \"${PKGPATH} == path\".",
".if (((((${PKGPATH} == path)))))")
+
+ // Note: this combination doesn't make sense since the patterns "one" and "two" don't overlap.
+ test(
+ ".if ${PKGPATH:Mone:Mtwo}",
+
+ "NOTE: module.mk:2: PKGPATH should be compared using == instead of matching against \":Mone\".",
+ "NOTE: module.mk:2: PKGPATH should be compared using == instead of matching against \":Mtwo\".",
+
+ ".if ${PKGPATH:Mone:Mtwo}")
}
func (s *Suite) Test_MkLineChecker_checkDirectiveCond__comparing_PKGSRC_COMPILER_with_eqeq(c *check.C) {
@@ -2109,12 +2131,14 @@ func (s *Suite) Test_MkLineChecker_check
words := mkline.Fields()
- t.CheckDeepEquals(words, []string{"`pkg-config pidgin --cflags`"})
+ // bmake handles backticks in the same way, treating them as ordinary characters
+ t.CheckDeepEquals(words, []string{"`pkg-config", "pidgin", "--cflags`"})
ck := MkLineChecker{mklines, mklines.mklines[1]}
ck.checkVartype("CFLAGS", opAssignAppend, "`pkg-config pidgin --cflags`", "")
// No warning about "`pkg-config" being an unknown CFlag.
+ // As of September 2019, there is no such check anymore in pkglint.
t.CheckOutputEmpty()
}
Index: pkgsrc/pkgtools/pkglint/files/mkparser.go
diff -u pkgsrc/pkgtools/pkglint/files/mkparser.go:1.33 pkgsrc/pkgtools/pkglint/files/mkparser.go:1.34
--- pkgsrc/pkgtools/pkglint/files/mkparser.go:1.33 Sun Sep 8 22:47:47 2019
+++ pkgsrc/pkgtools/pkglint/files/mkparser.go Tue Oct 1 21:37:59 2019
@@ -289,7 +289,7 @@ func (p *MkParser) varUseModifier(varnam
}
modifier := lexer.Since(mark)
- // ${SOURCES:%.c=%.o} or ${:!uname -a:[2]}
+ // ${SOURCES:%.c=%.o} or ${:!uname -a!:[2]}
if contains(modifier, "=") || (hasPrefix(modifier, "!") && hasSuffix(modifier, "!")) {
return modifier
}
@@ -330,6 +330,10 @@ func (p *MkParser) varUseModifierSubst(c
lexer.Skip(1)
separator := byte(sep)
+ unescape := func(s string) string {
+ return strings.Replace(s, "\\"+string(separator), string(separator), -1)
+ }
+
isOther := func(b byte) bool {
return b != separator && b != '$' && b != '\\'
}
@@ -360,7 +364,7 @@ func (p *MkParser) varUseModifierSubst(c
lexer.SkipByte('^')
skipOther()
lexer.SkipByte('$')
- from = lexer.Since(fromStart)
+ from = unescape(lexer.Since(fromStart))
if !lexer.SkipByte(separator) {
return
@@ -368,7 +372,7 @@ func (p *MkParser) varUseModifierSubst(c
toStart := lexer.Mark()
skipOther()
- to = lexer.Since(toStart)
+ to = unescape(lexer.Since(toStart))
if !lexer.SkipByte(separator) {
return
Index: pkgsrc/pkgtools/pkglint/files/util_test.go
diff -u pkgsrc/pkgtools/pkglint/files/util_test.go:1.33 pkgsrc/pkgtools/pkglint/files/util_test.go:1.34
--- pkgsrc/pkgtools/pkglint/files/util_test.go:1.33 Sun Sep 8 22:47:47 2019
+++ pkgsrc/pkgtools/pkglint/files/util_test.go Tue Oct 1 21:37:59 2019
@@ -936,10 +936,10 @@ func (s *Suite) Test_FileCache_Evict__so
t.Check(cache.Get("filename6.mk", 0), check.NotNil)
}
-func (s *Suite) Test_makeHelp(c *check.C) {
+func (s *Suite) Test_bmakeHelp(c *check.C) {
t := s.Init(c)
- t.CheckEquals(makeHelp("subst"), confMake+" help topic=subst")
+ t.CheckEquals(bmakeHelp("subst"), confMake+" help topic=subst")
}
func (s *Suite) Test_hasAlnumPrefix(c *check.C) {
@@ -1169,3 +1169,16 @@ func (s *Suite) Test_StringInterner(c *c
t.CheckEquals(si.Intern("Hello, world"), "Hello, world")
t.CheckEquals(si.Intern("Hello, world"[0:5]), "Hello")
}
+
+func (s *Suite) Test_shquote(c *check.C) {
+ t := s.Init(c)
+
+ test := func(in, out string) {
+ t.CheckEquals(shquote(in), out)
+ }
+
+ test("", "''")
+ test("'", "''\\'''")
+ test("simple", "simple")
+ test("~", "'~'")
+}
Index: pkgsrc/pkgtools/pkglint/files/mktokenslexer.go
diff -u pkgsrc/pkgtools/pkglint/files/mktokenslexer.go:1.1 pkgsrc/pkgtools/pkglint/files/mktokenslexer.go:1.2
--- pkgsrc/pkgtools/pkglint/files/mktokenslexer.go:1.1 Sun Mar 10 19:01:50 2019
+++ pkgsrc/pkgtools/pkglint/files/mktokenslexer.go Tue Oct 1 21:37:59 2019
@@ -48,12 +48,12 @@ func (m *MkTokensLexer) Rest() string {
// NextVarUse returns the next varuse token, unless there is some plain text
// before it. In that case or at EOF, it returns nil.
-func (m *MkTokensLexer) NextVarUse() *MkVarUse {
+func (m *MkTokensLexer) NextVarUse() *MkToken {
if m.Lexer.EOF() && len(m.tokens) > 0 && m.tokens[0].Varuse != nil {
token := m.tokens[0]
m.tokens = m.tokens[1:]
m.next()
- return token.Varuse
+ return token
}
return nil
}
Index: pkgsrc/pkgtools/pkglint/files/mktokenslexer_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mktokenslexer_test.go:1.3 pkgsrc/pkgtools/pkglint/files/mktokenslexer_test.go:1.4
--- pkgsrc/pkgtools/pkglint/files/mktokenslexer_test.go:1.3 Sun Jul 14 21:25:47 2019
+++ pkgsrc/pkgtools/pkglint/files/mktokenslexer_test.go Tue Oct 1 21:37:59 2019
@@ -37,49 +37,53 @@ func (s *Suite) Test_MkTokensLexer__sing
func (s *Suite) Test_MkTokensLexer__single_varuse_token(c *check.C) {
t := s.Init(c)
- lexer := NewMkTokensLexer([]*MkToken{{"${VAR:Mpattern}", NewMkVarUse("VAR", "Mpattern")}})
+ tokens := []*MkToken{{"${VAR:Mpattern}", NewMkVarUse("VAR", "Mpattern")}}
+ lexer := NewMkTokensLexer(tokens)
t.CheckEquals(lexer.EOF(), false)
t.CheckEquals(lexer.PeekByte(), -1)
- t.CheckDeepEquals(lexer.NextVarUse(), NewMkVarUse("VAR", "Mpattern"))
+ t.CheckDeepEquals(lexer.NextVarUse(), tokens[0])
}
func (s *Suite) Test_MkTokensLexer__plain_then_varuse(c *check.C) {
t := s.Init(c)
- lexer := NewMkTokensLexer([]*MkToken{
+ tokens := []*MkToken{
{"plain text", nil},
- {"${VAR:Mpattern}", NewMkVarUse("VAR", "Mpattern")}})
+ {"${VAR:Mpattern}", NewMkVarUse("VAR", "Mpattern")}}
+ lexer := NewMkTokensLexer(tokens)
t.CheckEquals(lexer.NextBytesSet(textproc.Digit.Inverse()), "plain text")
- t.CheckDeepEquals(lexer.NextVarUse(), NewMkVarUse("VAR", "Mpattern"))
+ t.CheckDeepEquals(lexer.NextVarUse(), tokens[1])
t.CheckEquals(lexer.EOF(), true)
}
func (s *Suite) Test_MkTokensLexer__varuse_varuse_varuse(c *check.C) {
t := s.Init(c)
- lexer := NewMkTokensLexer([]*MkToken{
+ tokens := []*MkToken{
{"${dirs:O:u}", NewMkVarUse("dirs", "O", "u")},
{"${VAR:Mpattern}", NewMkVarUse("VAR", "Mpattern")},
- {"${.TARGET}", NewMkVarUse(".TARGET")}})
+ {"${.TARGET}", NewMkVarUse(".TARGET")}}
+ lexer := NewMkTokensLexer(tokens)
- t.CheckDeepEquals(lexer.NextVarUse(), NewMkVarUse("dirs", "O", "u"))
- t.CheckDeepEquals(lexer.NextVarUse(), NewMkVarUse("VAR", "Mpattern"))
- t.CheckDeepEquals(lexer.NextVarUse(), NewMkVarUse(".TARGET"))
+ t.CheckDeepEquals(lexer.NextVarUse(), tokens[0])
+ t.CheckDeepEquals(lexer.NextVarUse(), tokens[1])
+ t.CheckDeepEquals(lexer.NextVarUse(), tokens[2])
t.Check(lexer.NextVarUse(), check.IsNil)
}
func (s *Suite) Test_MkTokensLexer__mark_reset_since_in_initial_state(c *check.C) {
t := s.Init(c)
- lexer := NewMkTokensLexer([]*MkToken{
+ tokens := []*MkToken{
{"${dirs:O:u}", NewMkVarUse("dirs", "O", "u")},
{"${VAR:Mpattern}", NewMkVarUse("VAR", "Mpattern")},
- {"${.TARGET}", NewMkVarUse(".TARGET")}})
+ {"${.TARGET}", NewMkVarUse(".TARGET")}}
+ lexer := NewMkTokensLexer(tokens)
start := lexer.Mark()
- t.CheckDeepEquals(lexer.NextVarUse(), NewMkVarUse("dirs", "O", "u"))
+ t.CheckDeepEquals(lexer.NextVarUse(), tokens[0])
middle := lexer.Mark()
t.CheckEquals(lexer.Rest(), "${VAR:Mpattern}${.TARGET}")
lexer.Reset(start)
@@ -127,12 +131,13 @@ func (s *Suite) Test_MkTokensLexer__mark
func (s *Suite) Test_MkTokensLexer__mark_reset_since_after_varuse(c *check.C) {
t := s.Init(c)
- lexer := NewMkTokensLexer([]*MkToken{
+ tokens := []*MkToken{
{"${VAR:Mpattern}", NewMkVarUse("VAR", "Mpattern")},
- {"rest", nil}})
+ {"rest", nil}}
+ lexer := NewMkTokensLexer(tokens)
start := lexer.Mark()
- t.CheckDeepEquals(lexer.NextVarUse(), NewMkVarUse("VAR", "Mpattern"))
+ t.CheckEquals(lexer.NextVarUse(), tokens[0])
end := lexer.Mark()
t.CheckEquals(lexer.Rest(), "rest")
lexer.Reset(start)
@@ -166,17 +171,18 @@ func (s *Suite) Test_MkTokensLexer__mult
func (s *Suite) Test_MkTokensLexer__multiple_marks_in_varuse(c *check.C) {
t := s.Init(c)
- lexer := NewMkTokensLexer([]*MkToken{
+ tokens := []*MkToken{
{"${VAR1}", NewMkVarUse("VAR1")},
{"${VAR2}", NewMkVarUse("VAR2")},
- {"${VAR3}", NewMkVarUse("VAR3")}})
+ {"${VAR3}", NewMkVarUse("VAR3")}}
+ lexer := NewMkTokensLexer(tokens)
start := lexer.Mark()
- t.CheckDeepEquals(lexer.NextVarUse(), NewMkVarUse("VAR1"))
+ t.CheckDeepEquals(lexer.NextVarUse(), tokens[0])
middle := lexer.Mark()
- t.CheckDeepEquals(lexer.NextVarUse(), NewMkVarUse("VAR2"))
+ t.CheckDeepEquals(lexer.NextVarUse(), tokens[1])
further := lexer.Mark()
- t.CheckDeepEquals(lexer.NextVarUse(), NewMkVarUse("VAR3"))
+ t.CheckDeepEquals(lexer.NextVarUse(), tokens[2])
end := lexer.Mark()
t.CheckEquals(lexer.Rest(), "")
lexer.Reset(middle)
@@ -232,15 +238,16 @@ func (s *Suite) Test_MkTokensLexer__cons
func (s *Suite) Test_MkTokensLexer__peek_after_varuse(c *check.C) {
t := s.Init(c)
- lexer := NewMkTokensLexer([]*MkToken{
+ tokens := []*MkToken{
{"${VAR}", NewMkVarUse("VAR")},
{"${VAR}", NewMkVarUse("VAR")},
- {"text", nil}})
+ {"text", nil}}
+ lexer := NewMkTokensLexer(tokens)
- t.CheckDeepEquals(lexer.NextVarUse(), NewMkVarUse("VAR"))
+ t.CheckDeepEquals(lexer.NextVarUse(), tokens[0])
t.CheckEquals(lexer.PeekByte(), -1)
- t.CheckDeepEquals(lexer.NextVarUse(), NewMkVarUse("VAR"))
+ t.CheckDeepEquals(lexer.NextVarUse(), tokens[1])
t.CheckEquals(lexer.PeekByte(), int('t'))
}
Index: pkgsrc/pkgtools/pkglint/files/package.go
diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.62 pkgsrc/pkgtools/pkglint/files/package.go:1.63
--- pkgsrc/pkgtools/pkglint/files/package.go:1.62 Sun Sep 8 22:47:47 2019
+++ pkgsrc/pkgtools/pkglint/files/package.go Tue Oct 1 21:37:59 2019
@@ -101,7 +101,7 @@ func NewPackage(dir string) *Package {
// as resolved from the package's directory.
// Variables that are known in the package are resolved, e.g. ${PKGDIR}.
func (pkg *Package) File(relativeFileName string) string {
- return cleanpath(resolveVariableRefs(nil, pkg.dir+"/"+relativeFileName))
+ return cleanpath(resolveVariableRefs(nil /* XXX: or maybe some mklines? */, joinPath(pkg.dir, relativeFileName)))
}
// Rel returns the path by which the given filename (as seen from the
@@ -497,7 +497,7 @@ func (pkg *Package) loadIncluded(mkline
dirname, _ := path.Split(includingFile)
dirname = cleanpath(dirname)
- fullIncluded := dirname + "/" + includedFile
+ fullIncluded := joinPath(dirname, includedFile)
relIncludedFile := relpath(pkg.dir, fullIncluded)
if !pkg.diveInto(includingFile, includedFile) {
@@ -535,7 +535,7 @@ func (pkg *Package) loadIncluded(mkline
dirname = pkgBasedir
- fullIncludedFallback := dirname + "/" + includedFile
+ fullIncludedFallback := joinPath(dirname, includedFile)
includedMklines = LoadMk(fullIncludedFallback, 0)
if includedMklines == nil {
return nil, false
@@ -598,7 +598,7 @@ func (pkg *Package) resolveIncludedFile(
// TODO: resolveVariableRefs uses G.Pkg implicitly. It should be made explicit.
// TODO: Try to combine resolveVariableRefs and ResolveVarsInRelativePath.
- includedFile := resolveVariableRefs(nil, mkline.ResolveVarsInRelativePath(mkline.IncludedFile()))
+ includedFile := resolveVariableRefs(nil /* XXX: or maybe some mklines? */, mkline.ResolveVarsInRelativePath(mkline.IncludedFile()))
if containsVarRef(includedFile) {
if trace.Tracing && !contains(includingFilename, "/mk/") {
trace.Stepf("%s:%s: Skipping unresolvable include file %q.",
@@ -1195,7 +1195,7 @@ func (pkg *Package) checkFileMakefileExt
"are extension.mk, module.mk, version.mk.",
"",
"These topic files should be documented properly so that their",
- sprintf("content can be queried using %q.", makeHelp("help")))
+ sprintf("content can be queried using %q.", bmakeHelp("help")))
}
// checkOwnerMaintainer checks files that are about to be committed.
@@ -1211,7 +1211,7 @@ func (pkg *Package) checkOwnerMaintainer
owner := pkg.vars.LastValue("OWNER")
maintainer := pkg.vars.LastValue("MAINTAINER")
- if maintainer == "pkgsrc-users%NetBSD.org@localhost" {
+ if maintainer == "pkgsrc-users%NetBSD.org@localhost" || maintainer == "INSERT_YOUR_MAIL_ADDRESS_HERE" {
maintainer = ""
}
if owner == "" && maintainer == "" {
@@ -1260,7 +1260,7 @@ func (pkg *Package) checkFreeze(filename
line.Notef("Pkgsrc is frozen since %s.", freezeStart)
line.Explain(
"During a pkgsrc freeze, changes to pkgsrc should only be made very carefully.",
- "See https://www.netbsd.org/developers/pkgsrc/ for the exact rules.")
+ "See https://www.NetBSD.org/developers/pkgsrc/ for the exact rules.")
}
func (pkg *Package) checkIncludeConditionally(mkline *MkLine, indentation *Indentation) {
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.62 pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.63
--- pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.62 Wed Aug 21 16:45:17 2019
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck.go Tue Oct 1 21:37:59 2019
@@ -2,6 +2,7 @@ package pkglint
import (
"netbsd.org/pkglint/regex"
+ "netbsd.org/pkglint/textproc"
"path"
"sort"
"strings"
@@ -159,9 +160,69 @@ func (cv *VartypeCheck) AwkCommand() {
}
}
+// BasicRegularExpression checks for a basic regular expression, as
+// defined by POSIX.
+//
+// When they are used in a list variable (as for CHECK_FILES_SKIP), they
+// cannot include spaces. Instead, a dot or [[:space:]] must be used.
+// The regular expressions do not need any quotation for the shell; all
+// quoting issues are handled by the pkgsrc infrastructure.
+//
+// The opposite situation is when the regular expression is part of a sed
+// command. In such a case the shell quoting is undone before checking the
+// regular expression, and this is where spaces and tabs can appear.
+//
+// TODO: Add check for EREs as well.
+//
+// See https://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap09.html#tag_09_03.
+// See https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html#tag_09_03.
func (cv *VartypeCheck) BasicRegularExpression() {
- if trace.Tracing {
- trace.Step1("Unchecked basic regular expression: %q", cv.Value)
+
+ // same order as in the OpenGroup spec
+ allowedAfterBackslash := textproc.NewByteSet(")({}1-9.[\\*^$")
+
+ lexer := textproc.NewLexer(cv.ValueNoVar)
+
+ parseCharacterClass := func() {
+ for !lexer.EOF() {
+ if lexer.SkipByte('\\') {
+ if !lexer.EOF() {
+ lexer.Skip(1)
+ }
+ } else if lexer.SkipByte(']') {
+ return
+ } else {
+ lexer.Skip(1)
+ }
+ }
+ }
+
+ parseBackslash := func() {
+ if lexer.EOF() {
+ return
+ }
+
+ if !lexer.TestByteSet(allowedAfterBackslash) {
+ cv.Warnf("In a basic regular expression, a backslash followed by %q is undefined.", lexer.Rest()[:1])
+ cv.Explain(
+ "Only the characters . [ \\ * ^ $ may be escaped using a backslash.",
+ "Except when the escaped character appears in a character class like [\\.a-z].",
+ "",
+ "To fix this, remove the backslash before the character.")
+ }
+ lexer.Skip(1)
+ }
+
+ for !lexer.EOF() {
+ switch {
+ case lexer.SkipByte('['):
+ parseCharacterClass()
+
+ case lexer.SkipByte('\\'):
+ parseBackslash()
+
+ case lexer.Skip(1):
+ }
}
}
@@ -492,26 +553,28 @@ func (cv *VartypeCheck) Enum(allowedValu
}
func (cv *VartypeCheck) FetchURL() {
+ fetchURL := cv.Value
+ url := strings.TrimPrefix(fetchURL, "-")
+ hyphen := condStr(len(fetchURL) > len(url), "-", "")
+ hyphenSubst := condStr(hyphen != "", ":S,^,-,", "")
- // TODO: Handle leading "-".
-
- cv.URL()
+ cv.WithValue(url).URL()
for siteURL, siteName := range G.Pkgsrc.MasterSiteURLToVar {
- if hasPrefix(cv.Value, siteURL) {
- subdir := cv.Value[len(siteURL):]
- if hasPrefix(cv.Value, "https://github.com/";) {
+ if hasPrefix(url, siteURL) {
+ subdir := url[len(siteURL):]
+ if hasPrefix(url, "https://github.com/";) {
subdir = strings.SplitAfter(subdir, "/")[0]
- cv.Warnf("Please use ${%s:=%s} instead of %q and run %q for further instructions.",
- siteName, subdir, cv.Value[:len(siteURL)+len(subdir)], makeHelp("github"))
+ cv.Warnf("Please use ${%s%s:=%s} instead of %q and run %q for further instructions.",
+ siteName, hyphenSubst, subdir, hyphen+url[:len(siteURL)+len(subdir)], bmakeHelp("github"))
} else {
- cv.Warnf("Please use ${%s:=%s} instead of %q.", siteName, subdir, cv.Value)
+ cv.Warnf("Please use ${%s%s:=%s} instead of %q.", siteName, hyphenSubst, subdir, hyphen+url)
}
return
}
}
- tokens := cv.MkLine.Tokenize(cv.Value, false)
+ tokens := cv.MkLine.Tokenize(url, false)
for _, token := range tokens {
varUse := token.Varuse
if varUse == nil {
@@ -538,6 +601,26 @@ func (cv *VartypeCheck) FetchURL() {
cv.Errorf("The subdirectory in %s must end with a slash.", name)
}
}
+
+ switch {
+ case cv.Op == opUseMatch,
+ hasSuffix(fetchURL, "/"),
+ hasSuffix(fetchURL, "="),
+ hasSuffix(fetchURL, ":"),
+ hasPrefix(fetchURL, "-"),
+ len(tokens) == 0 || tokens[len(tokens)-1].Varuse != nil:
+ break
+
+ default:
+ cv.Warnf("The fetch URL %q should end with a slash.", fetchURL)
+ cv.Explain(
+ "The filename from DISTFILES is appended directly to this base URL.",
+ "Therefore it should typically end with a slash, or sometimes with",
+ "an equals sign or a colon.",
+ "",
+ "To specify a full URL directly, prefix it with a hyphen, such as in",
+ "-https://example.org/distfile-1.0.tar.gz.";)
+ }
}
// Filename checks that filenames use only limited special characters.
@@ -563,7 +646,7 @@ func (cv *VartypeCheck) Filename() {
invalid)
}
-func (cv *VartypeCheck) FileMask() {
+func (cv *VartypeCheck) FilePattern() {
// TODO: Decide whether to call this a "mask" or a "pattern", and use only that word everywhere.
@@ -857,10 +940,10 @@ func (cv *VartypeCheck) Pathlist() {
}
}
-// PathMask is a shell pattern for pathnames, possibly including slashes.
+// PathPattern is a shell pattern for pathnames, possibly including slashes.
//
-// See FileMask.
-func (cv *VartypeCheck) PathMask() {
+// See FilePattern.
+func (cv *VartypeCheck) PathPattern() {
invalid := replaceAll(cv.ValueNoVar, `[%*+,\-./0-9?@A-Z\[\]_a-z~]`, "")
if invalid == "" {
return
@@ -945,15 +1028,36 @@ func (cv *VartypeCheck) PkgOptionsVar()
}
}
-// PkgPath checks a directory name relative to the top-level pkgsrc directory.
+// Pkgpath checks a directory name relative to the top-level pkgsrc directory.
//
// Despite its name, it is more similar to RelativePkgDir than to RelativePkgPath.
-func (cv *VartypeCheck) PkgPath() {
- pkgsrcdir := cv.MkLine.PathToFile(G.Pkgsrc.File("."))
- MkLineChecker{cv.MkLines, cv.MkLine}.CheckRelativePkgdir(pkgsrcdir + "/" + cv.Value)
+func (cv *VartypeCheck) Pkgpath() {
+ cv.Pathname()
+
+ pkgpath := cv.Value
+ if pkgpath != cv.ValueNoVar || cv.Op == opUseMatch {
+ return
+ }
+
+ if !G.Wip && hasPrefix(pkgpath, "wip/") {
+ cv.MkLine.Errorf("A main pkgsrc package must not depend on a pkgsrc-wip package.")
+ }
+
+ if !fileExists(G.Pkgsrc.File(joinPath(pkgpath, "Makefile"))) {
+ cv.MkLine.Errorf("There is no package in %q.",
+ relpath(path.Dir(cv.MkLine.Filename), G.Pkgsrc.File(pkgpath)))
+ return
+ }
+
+ if !matches(pkgpath, `^([^./][^/]*/[^./][^/]*)$`) {
+ cv.MkLine.Errorf("%q is not a valid path to a package.", pkgpath)
+ cv.MkLine.Explain(
+ "A path to a package has the form \"category/pkgbase\".",
+ "It is relative to the pkgsrc root.")
+ }
}
-func (cv *VartypeCheck) PkgRevision() {
+func (cv *VartypeCheck) Pkgrevision() {
if !matches(cv.Value, `^[1-9]\d*$`) {
cv.Warnf("%s must be a positive integer number.", cv.Varname)
}
@@ -1099,38 +1203,65 @@ func (cv *VartypeCheck) SedCommands() {
ntokens := len(tokens)
ncommands := 0
+ extended := false
+
+ checkSedCommand := func(quotedCommand string) {
+ // TODO: Remember the extended flag for the whole file, especially
+ // for SUBST_SED.* variables.
+ if !extended {
+ command := cv.MkLine.UnquoteShell(quotedCommand, true)
+ if !hasPrefix(command, "s") {
+ return
+ }
+
+ // The :C modifier is similar enough for parsing.
+ ok, _, from, _, _ := MkVarUseModifier{"C" + command[1:]}.MatchSubst()
+ if !ok {
+ return
+ }
+
+ cv.WithValue(from).BasicRegularExpression()
+ }
+ }
for i := 0; i < ntokens; i++ {
token := tokens[i]
switch {
case token == "-e":
- if i+1 < ntokens {
- // Check the real sed command here.
- i++
- ncommands++
- if ncommands > 1 {
- cv.Notef("Each sed command should appear in an assignment of its own.")
- cv.Explain(
- "For example, instead of",
- " SUBST_SED.foo+= -e s,command1,, -e s,command2,,",
- "use",
- " SUBST_SED.foo+= -e s,command1,,",
- " SUBST_SED.foo+= -e s,command2,,",
- "",
- "This way, short sed commands cannot be hidden at the end of a line.")
- }
- } else {
+ if i+1 >= ntokens {
cv.Errorf("The -e option to sed requires an argument.")
+ break
}
+
+ // Check the real sed command here.
+ i++
+ ncommands++
+ if ncommands > 1 {
+ cv.Notef("Each sed command should appear in an assignment of its own.")
+ cv.Explain(
+ "For example, instead of",
+ " SUBST_SED.foo+= -e s,command1,, -e s,command2,,",
+ "use",
+ " SUBST_SED.foo+= -e s,command1,,",
+ " SUBST_SED.foo+= -e s,command2,,",
+ "",
+ "This way, short sed commands cannot be hidden at the end of a line.")
+ }
+
+ checkSedCommand(tokens[i])
+
case token == "-E":
// Switch to extended regular expressions mode.
+ extended = true
case token == "-n":
// Don't print lines per default.
case matches(token, `^["']?(\d+|/.*/)?s`):
+ // TODO: "prefix with" instead of "use".
cv.Notef("Please always use \"-e\" in sed commands, even if there is only one substitution.")
+ checkSedCommand(token)
default:
cv.Warnf("Unknown sed command %q.", token)
Index: pkgsrc/pkgtools/pkglint/files/package_test.go
diff -u pkgsrc/pkgtools/pkglint/files/package_test.go:1.53 pkgsrc/pkgtools/pkglint/files/package_test.go:1.54
--- pkgsrc/pkgtools/pkglint/files/package_test.go:1.53 Sun Sep 8 22:47:47 2019
+++ pkgsrc/pkgtools/pkglint/files/package_test.go Tue Oct 1 21:37:59 2019
@@ -1551,7 +1551,7 @@ func (s *Suite) Test_Package_checkUpdate
"WARN: category/pkg2/Makefile:4: This package should be updated to 2.0 ([nice new features]).",
"NOTE: category/pkg3/Makefile:4: This package is newer than the update request to 3.0 ([security update]).",
"4 warnings and 2 notes found.",
- "(Run \"pkglint -e\" to show explanations.)")
+ "(Run \"pkglint -e -Wall,no-space category/pkg1 category/pkg2 category/pkg3\" to show explanations.)")
}
func (s *Suite) Test_NewPackage(c *check.C) {
@@ -2625,12 +2625,30 @@ func (s *Suite) Test_Package_checkOwnerM
G.Check(pkg)
+ // No warning for the patches directory, only for regular files.
t.CheckOutputLines(
"NOTE: ~/category/package/Makefile: " +
"Please only commit changes that " +
"maintainer%example.org@localhost would approve.")
}
+func (s *Suite) Test_Package_checkOwnerMaintainer__url2pkg(c *check.C) {
+ t := s.Init(c)
+
+ G.Username = "example-user"
+ pkg := t.SetUpPackage("category/package",
+ "MAINTAINER=\tINSERT_YOUR_MAIL_ADDRESS_HERE")
+ t.CreateFileLines("category/package/CVS/Entries",
+ "/Makefile//modified//")
+ t.FinishSetUp()
+
+ G.Check(pkg)
+
+ t.CheckOutputLines(
+ "WARN: ~/category/package/Makefile:8: " +
+ "\"INSERT_YOUR_MAIL_ADDRESS_HERE\" is not a valid mail address.")
+}
+
func (s *Suite) Test_Package_checkFreeze(c *check.C) {
t := s.Init(c)
@@ -2648,7 +2666,7 @@ func (s *Suite) Test_Package_checkFreeze
"NOTE: ~/category/package/Makefile: Pkgsrc is frozen since 2018-03-20.",
"",
"\tDuring a pkgsrc freeze, changes to pkgsrc should only be made very",
- "\tcarefully. See https://www.netbsd.org/developers/pkgsrc/ for the",
+ "\tcarefully. See https://www.NetBSD.org/developers/pkgsrc/ for the",
"\texact rules.",
"")
}
Index: pkgsrc/pkgtools/pkglint/files/pkglint.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint.go:1.60 pkgsrc/pkgtools/pkglint/files/pkglint.go:1.61
--- pkgsrc/pkgtools/pkglint/files/pkglint.go:1.60 Sun Sep 8 22:47:47 2019
+++ pkgsrc/pkgtools/pkglint/files/pkglint.go Tue Oct 1 21:37:59 2019
@@ -198,7 +198,7 @@ func (pkglint *Pkglint) Main(stdout io.W
pkglint.Pkgsrc.checkToplevelUnusedLicenses()
- pkglint.Logger.ShowSummary()
+ pkglint.Logger.ShowSummary(args)
if pkglint.Logger.errors != 0 {
return 1
}
@@ -272,7 +272,7 @@ func (pkglint *Pkglint) prepareMainLoop(
dummyLine.Fatalf("%q must be inside a pkgsrc tree.", firstDir)
}
- pkglint.Pkgsrc = NewPkgsrc(firstDir + "/" + relTopdir)
+ pkglint.Pkgsrc = NewPkgsrc(joinPath(firstDir, relTopdir))
pkglint.Wip = matches(pkglint.Pkgsrc.ToRel(firstDir), `^wip(/|$)`) // Same as in Pkglint.Check.
pkglint.Pkgsrc.LoadInfrastructure()
@@ -431,7 +431,7 @@ func (pkglint *Pkglint) checkdirPackage(
// Returns the pkgsrc top-level directory, relative to the given directory.
func findPkgsrcTopdir(dirname string) string {
for _, dir := range [...]string{".", "..", "../..", "../../.."} {
- if fileExists(dirname + "/" + dir + "/mk/bsd.pkg.mk") {
+ if fileExists(joinPath(dirname, dir, "mk/bsd.pkg.mk")) {
return dir
}
}
Index: pkgsrc/pkgtools/pkglint/files/pkglint_test.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.47 pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.48
--- pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.47 Sun Sep 8 22:47:47 2019
+++ pkgsrc/pkgtools/pkglint/files/pkglint_test.go Tue Oct 1 21:37:59 2019
@@ -215,7 +215,7 @@ func (s *Suite) Test_Pkglint_Main__compl
"Size (checkperms-1.12.tar.gz) = 6621 bytes",
"SHA1 (patch-checkperms.c) = asdfasdf") // Invalid SHA-1 checksum
- t.Main("-Wall", "-Call", t.File("sysutils/checkperms"))
+ t.Main("-Wall", "-Call", "sysutils/checkperms")
t.CheckOutputLines(
"NOTE: ~/sysutils/checkperms/Makefile:3: "+
@@ -231,9 +231,9 @@ func (s *Suite) Test_Pkglint_Main__compl
"WARN: ~/sysutils/checkperms/patches/patch-checkperms.c:12: Premature end of patch hunk "+
"(expected 1 lines to be deleted and 0 lines to be added).",
"4 errors, 2 warnings and 1 note found.",
- "(Run \"pkglint -e\" to show explanations.)",
- "(Run \"pkglint -fs\" to show what can be fixed automatically.)",
- "(Run \"pkglint -F\" to automatically fix some issues.)")
+ t.Shquote("(Run \"pkglint -e -Wall -Call %s\" to show explanations.)", "sysutils/checkperms"),
+ t.Shquote("(Run \"pkglint -fs -Wall -Call %s\" to show what can be fixed automatically.)", "sysutils/checkperms"),
+ t.Shquote("(Run \"pkglint -F -Wall -Call %s\" to automatically fix some issues.)", "sysutils/checkperms"))
}
func (s *Suite) Test_Pkglint_Main__autofix_exitcode(c *check.C) {
@@ -611,7 +611,7 @@ func (s *Suite) Test_Pkglint_checkReg__a
t.CheckOutputLines(
"ERROR: ~/category/package/ALTERNATIVES:1: Alternative implementation \"bin/gnu-tar\" must be an absolute path.",
"1 error found.",
- "(Run \"pkglint -e\" to show explanations.)")
+ t.Shquote("(Run \"pkglint -e %s\" to show explanations.)", "category/package/ALTERNATIVES"))
}
// Just for branch coverage.
Index: pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go
diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.29 pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.30
--- pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.29 Tue Jul 30 18:31:43 2019
+++ pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go Tue Oct 1 21:37:59 2019
@@ -464,7 +464,7 @@ func (s *Suite) Test_Pkgsrc_loadDocChang
t.CheckOutputLines(
"WARN: ~/category/package/Makefile:3: The package is being downgraded from 2.0 (see ../../doc/CHANGES-2018:9) to 1.0.",
"1 warning found.",
- "(Run \"pkglint -e\" to show explanations.)")
+ t.Shquote("(Run \"pkglint -e %s\" to show explanations.)", "category/package"))
// Only when the global checks are enabled, the errors from doc/CHANGES are shown.
t.Main("-Cglobal", "-Wall", ".")
@@ -475,7 +475,7 @@ func (s *Suite) Test_Pkgsrc_loadDocChang
"WARN: ~/doc/CHANGES-2018:8: Unknown doc/CHANGES line: \tInvalid pkgpath to 1.16 [rillig 2019-06-16]",
"WARN: ~/doc/CHANGES-2018:9: Year \"2019\" for category/package does not match the filename ~/doc/CHANGES-2018.",
"4 warnings found.",
- "(Run \"pkglint -e\" to show explanations.)")
+ t.Shquote("(Run \"pkglint -e -Cglobal -Wall %s\" to show explanations.)", "."))
}
func (s *Suite) Test_Pkgsrc_loadDocChangesFromFile__wrong_indentation(c *check.C) {
@@ -496,7 +496,7 @@ func (s *Suite) Test_Pkgsrc_loadDocChang
"WARN: ~/doc/CHANGES-2018:5: Package changes should be indented using a single tab, not \" \".",
"WARN: ~/doc/CHANGES-2018:6: Package changes should be indented using a single tab, not \" \\t\".",
"2 warnings found.",
- "(Run \"pkglint -e\" to show explanations.)")
+ t.Shquote("(Run \"pkglint -e -Cglobal -Wall %s\" to show explanations.)", "category/package"))
}
// Once or twice in a decade, changes to the pkgsrc infrastructure are also
@@ -967,12 +967,12 @@ func (s *Suite) Test_Pkgsrc_VariableType
t1 := G.Pkgsrc.VariableType(nil, "FONT_DIRS")
c.Assert(t1, check.NotNil)
- t.CheckEquals(t1.String(), "PathMask (list, guessed)")
+ t.CheckEquals(t1.String(), "PathPattern (list, guessed)")
t2 := G.Pkgsrc.VariableType(nil, "FONT_DIRS.ttf")
c.Assert(t2, check.NotNil)
- t.CheckEquals(t2.String(), "PathMask (list, guessed)")
+ t.CheckEquals(t2.String(), "PathPattern (list, guessed)")
}
// Guessing the variable type also works for variables that are
@@ -1019,7 +1019,7 @@ func (s *Suite) Test_Pkgsrc_VariableType
"WARN: ~/category/package/Makefile:21: PKGSRC_UNKNOWN_ENV is defined but not used.",
"WARN: ~/category/package/Makefile:21: ABCPATH is used but not defined.",
"2 warnings found.",
- "(Run \"pkglint -e\" to show explanations.)")
+ t.Shquote("(Run \"pkglint -e -Wall %s\" to show explanations.)", "category/package"))
}
func (s *Suite) Test_Pkgsrc_guessVariableType__SKIP(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/shell.go
diff -u pkgsrc/pkgtools/pkglint/files/shell.go:1.45 pkgsrc/pkgtools/pkglint/files/shell.go:1.46
--- pkgsrc/pkgtools/pkglint/files/shell.go:1.45 Sun Sep 8 22:47:47 2019
+++ pkgsrc/pkgtools/pkglint/files/shell.go Tue Oct 1 21:37:59 2019
@@ -189,8 +189,8 @@ func (ck *ShellLineChecker) checkVaruseT
case (quoting == shqSquot || quoting == shqDquot) && matches(varname, `^(?:.*DIR|.*FILE|.*PATH|.*_VAR|PREFIX|.*BASE|PKGNAME)$`):
// This is ok as long as these variables don't have embedded [$\\"'`].
- case quoting == shqDquot && varuse.IsQ():
- ck.Warnf("The :Q modifier should not be used inside double quotes.")
+ case quoting != shqPlain && varuse.IsQ():
+ ck.Warnf("The :Q modifier should not be used inside quotes.")
ck.Explain(
"The :Q modifier is intended for embedding a string into a shell program.",
"It escapes all characters that have a special meaning in shell programs.",
@@ -312,7 +312,7 @@ func (ck *ShellLineChecker) CheckShellCo
"hidden behind the scenes.",
"",
// TODO: Provide a copy-and-paste example.
- sprintf("Run %q for more information.", makeHelp("subst")))
+ sprintf("Run %q for more information.", bmakeHelp("subst")))
if contains(shelltext, "#") {
line.Explain(
"When migrating to the SUBST framework, pay attention to \"#\" characters.",
@@ -375,7 +375,7 @@ func (ck *ShellLineChecker) CheckShellCo
}
walker.Callback.AndOr = func(andor *MkShAndOr) {
if G.Opts.WarnExtra && !*pSetE && walker.Current().Index != 0 {
- spc.checkSetE(walker.Parent(1).(*MkShList), walker.Current().Index, andor)
+ spc.checkSetE(walker.Parent(1).(*MkShList), walker.Current().Index)
}
}
walker.Callback.Pipeline = func(pipeline *MkShPipeline) {
@@ -976,7 +976,7 @@ func (spc *ShellProgramChecker) canFail(
return true
}
-func (spc *ShellProgramChecker) checkSetE(list *MkShList, index int, andor *MkShAndOr) {
+func (spc *ShellProgramChecker) checkSetE(list *MkShList, index int) {
if trace.Tracing {
defer trace.Call0()()
}
Index: pkgsrc/pkgtools/pkglint/files/shell_test.go
diff -u pkgsrc/pkgtools/pkglint/files/shell_test.go:1.52 pkgsrc/pkgtools/pkglint/files/shell_test.go:1.53
--- pkgsrc/pkgtools/pkglint/files/shell_test.go:1.52 Thu Sep 12 21:15:48 2019
+++ pkgsrc/pkgtools/pkglint/files/shell_test.go Tue Oct 1 21:37:59 2019
@@ -183,11 +183,18 @@ func (s *Suite) Test_ShellLineChecker_Ch
"NOTE: filename.mk:1: The :Q modifier isn't necessary for ${PKGNAME} here.")
test("echo \"${CFLAGS:Q}\"", // VucQuotDquot
- "WARN: filename.mk:1: The :Q modifier should not be used inside double quotes.",
+
+ // ShellLineChecker.checkVaruseToken
+ "WARN: filename.mk:1: The :Q modifier should not be used inside quotes.",
+
+ // ShellLineChecker.checkVaruseToken
+ // MkLineChecker.CheckVaruse
+ // MkLineChecker.checkVarUseQuoting
"WARN: filename.mk:1: Please use ${CFLAGS:M*:Q} instead of ${CFLAGS:Q} "+
"and make sure the variable appears outside of any quoting characters.")
test("echo '${COMMENT:Q}'", // VucQuotSquot
+ "WARN: filename.mk:1: The :Q modifier should not be used inside quotes.",
"WARN: filename.mk:1: Please move ${COMMENT:Q} outside of any quoting characters.")
test("echo target=$@ exitcode=$$? '$$' \"\\$$\"",
@@ -1463,6 +1470,33 @@ func (s *Suite) Test_SimpleCommandChecke
"NOTE: filename.mk:1: You can use \"INSTALLATION_DIRS+= share/other\" instead of \"${INSTALL_DATA_DIR}\".")
}
+func (s *Suite) Test_SimpleCommandChecker_checkAutoMkdirs__redundant(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package",
+ "AUTO_MKDIRS=\tyes",
+ "INSTALLATION_DIRS+=\tshare/redundant",
+ "INSTALLATION_DIRS+=\tnot/redundant ${EGDIR}")
+ t.CreateFileLines("category/package/PLIST",
+ PlistCvsID,
+ "share/redundant/file",
+ "${EGDIR}/file")
+
+ t.Main("-Wall", "-q", "category/package")
+
+ t.CheckOutputLines(
+ "NOTE: ~/category/package/Makefile:21: The directory \"share/redundant\" "+
+ "is redundant in INSTALLATION_DIRS.",
+ // The below is not proven to be always correct. It assumes that a
+ // variable in the Makefile has the same value as the corresponding
+ // variable from PLIST_SUBST. Violating this assumption would be
+ // confusing to the pkgsrc developers, therefore it's a safe bet.
+ // A notable counterexample is PKGNAME in PLIST, which corresponds
+ // to PKGNAME_NOREV in the package Makefile.
+ "NOTE: ~/category/package/Makefile:22: The directory \"${EGDIR}\" "+
+ "is redundant in INSTALLATION_DIRS.")
+}
+
// The AUTO_MKDIRS code in mk/install/install.mk (install-dirs-from-PLIST)
// skips conditional directories, as well as directories with placeholders.
func (s *Suite) Test_SimpleCommandChecker_checkAutoMkdirs__conditional_PLIST(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/util.go
diff -u pkgsrc/pkgtools/pkglint/files/util.go:1.52 pkgsrc/pkgtools/pkglint/files/util.go:1.53
--- pkgsrc/pkgtools/pkglint/files/util.go:1.52 Thu Sep 12 21:15:48 2019
+++ pkgsrc/pkgtools/pkglint/files/util.go Tue Oct 1 21:37:59 2019
@@ -238,7 +238,7 @@ func isEmptyDir(filename string) bool {
if isIgnoredFilename(name) {
continue
}
- if dirent.IsDir() && isEmptyDir(filename+"/"+name) {
+ if dirent.IsDir() && isEmptyDir(joinPath(filename, name)) {
continue
}
return false
@@ -255,7 +255,7 @@ func getSubdirs(filename string) []strin
var subdirs []string
for _, dirent := range dirents {
name := dirent.Name()
- if dirent.IsDir() && !isIgnoredFilename(name) && !isEmptyDir(filename+"/"+name) {
+ if dirent.IsDir() && !isIgnoredFilename(name) && !isEmptyDir(joinPath(filename, name)) {
subdirs = append(subdirs, name)
}
}
@@ -278,7 +278,7 @@ func dirglob(dirname string) []string {
var filenames []string
for _, info := range infos {
if !(isIgnoredFilename(info.Name())) {
- filenames = append(filenames, cleanpath(dirname+"/"+info.Name()))
+ filenames = append(filenames, cleanpath(joinPath(dirname, info.Name())))
}
}
return filenames
@@ -444,6 +444,13 @@ func mkopSubst(s string, left bool, from
})
}
+func joinPath(a, b string, others ...string) string {
+ if len(others) == 0 {
+ return a + "/" + b
+ }
+ return a + "/" + b + "/" + strings.Join(others, "/")
+}
+
// relpath returns the relative path from the directory "from"
// to the filesystem entry "to".
//
@@ -502,7 +509,7 @@ func relpath(from, to string) (result st
fromTop, err := filepath.Rel(absTopdir, absTo)
assertNil(err, "relpath from topdir %q to %q", absTopdir, absTo)
- result = cleanpath(filepath.ToSlash(toTop) + "/" + filepath.ToSlash(fromTop))
+ result = cleanpath(joinPath(filepath.ToSlash(toTop), filepath.ToSlash(fromTop)))
if trace.Tracing {
trace.Stepf("relpath from %q to %q = %q", cfrom, cto, result)
@@ -513,7 +520,7 @@ func relpath(from, to string) (result st
func abspath(filename string) string {
abs := filename
if !filepath.IsAbs(filename) {
- abs = G.cwd + "/" + abs
+ abs = joinPath(G.cwd, abs)
}
return path.Clean(abs)
}
@@ -1108,7 +1115,7 @@ func (c *FileCache) key(filename string)
return path.Clean(filename)
}
-func makeHelp(topic string) string { return bmake("help topic=" + topic) }
+func bmakeHelp(topic string) string { return bmake("help topic=" + topic) }
func bmake(target string) string { return sprintf("%s %s", confMake, target) }
@@ -1353,7 +1360,10 @@ func (s *StringSet) AddAll(elements []st
}
}
-func (s *StringSet) Contains(needle string) bool {
- _, found := s.seen[needle]
- return found
+// See mk/tools/shquote.sh.
+func shquote(s string) string {
+ if matches(s, `^[!%+,\-./0-9:=@A-Z_a-z]+$`) {
+ return s
+ }
+ return "'" + strings.Replace(s, "'", "'\\''", -1) + "'"
}
Index: pkgsrc/pkgtools/pkglint/files/tools_test.go
diff -u pkgsrc/pkgtools/pkglint/files/tools_test.go:1.19 pkgsrc/pkgtools/pkglint/files/tools_test.go:1.20
--- pkgsrc/pkgtools/pkglint/files/tools_test.go:1.19 Sun Jul 14 21:25:47 2019
+++ pkgsrc/pkgtools/pkglint/files/tools_test.go Tue Oct 1 21:37:59 2019
@@ -144,7 +144,7 @@ func (s *Suite) Test_Tools__USE_TOOLS_pr
"WARN: ~/module.mk:5: Unknown shell command \"${AWK}\".",
"WARN: ~/module.mk:5: AWK is used but not defined.",
"2 warnings found.",
- "(Run \"pkglint -e\" to show explanations.)")
+ t.Shquote("(Run \"pkglint -e -Wall %s\" to show explanations.)", "module.mk"))
}
// It may happen that a tool is first defined without knowing its
Index: pkgsrc/pkgtools/pkglint/files/toplevel_test.go
diff -u pkgsrc/pkgtools/pkglint/files/toplevel_test.go:1.19 pkgsrc/pkgtools/pkglint/files/toplevel_test.go:1.20
--- pkgsrc/pkgtools/pkglint/files/toplevel_test.go:1.19 Sun Jul 14 21:25:47 2019
+++ pkgsrc/pkgtools/pkglint/files/toplevel_test.go Tue Oct 1 21:37:59 2019
@@ -101,7 +101,7 @@ func (s *Suite) Test_CheckdirToplevel__r
t.CheckOutputLines(
"WARN: ~/category/package/Makefile:20: UNKNOWN is defined but not used.",
"1 warning found.",
- "(Run \"pkglint -e\" to show explanations.)")
+ t.Shquote("(Run \"pkglint -e -Wall -r %s\" to show explanations.)", "."))
}
func (s *Suite) Test_CheckdirToplevel__recursive_inter_package(c *check.C) {
@@ -130,7 +130,7 @@ func (s *Suite) Test_CheckdirToplevel__r
"WARN: ~/category/package/Makefile:20: UNKNOWN is defined but not used.",
"WARN: ~/licenses/gnu-gpl-v2: This license seems to be unused.",
"2 warnings found.",
- "(Run \"pkglint -e\" to show explanations.)")
+ t.Shquote("(Run \"pkglint -e -Wall -Call -r %s\" to show explanations.)", "."))
}
func (s *Suite) Test_CheckdirToplevel__indentation(c *check.C) {
@@ -150,7 +150,7 @@ func (s *Suite) Test_CheckdirToplevel__i
t.CheckOutputLines(
"NOTE: ~/Makefile:4: This variable value should be aligned to column 17.",
"Looks fine.",
- "(Run \"pkglint -e\" to show explanations.)",
- "(Run \"pkglint -fs\" to show what can be fixed automatically.)",
- "(Run \"pkglint -F\" to automatically fix some issues.)")
+ t.Shquote("(Run \"pkglint -e -Wall %s\" to show explanations.)", "."),
+ t.Shquote("(Run \"pkglint -fs -Wall %s\" to show what can be fixed automatically.)", "."),
+ t.Shquote("(Run \"pkglint -F -Wall %s\" to automatically fix some issues.)", "."))
}
Index: pkgsrc/pkgtools/pkglint/files/vardefs.go
diff -u pkgsrc/pkgtools/pkglint/files/vardefs.go:1.72 pkgsrc/pkgtools/pkglint/files/vardefs.go:1.73
--- pkgsrc/pkgtools/pkglint/files/vardefs.go:1.72 Thu Sep 12 21:15:48 2019
+++ pkgsrc/pkgtools/pkglint/files/vardefs.go Tue Oct 1 21:37:59 2019
@@ -510,7 +510,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.usr("MOTIFBASE", BtPathname)
reg.usr("PKGINFODIR", BtPathname)
- reg.usr("PKGMANDIR", BtPathname)
+ reg.usr("PKGMANDIR", BtPrefixPathname)
reg.usr("PKGGNUDIR", BtPathname)
reg.usr("BSDSRCDIR", BtPathname)
reg.usr("BSDXSRCDIR", BtPathname)
@@ -529,10 +529,10 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.usr("PATCH_FUZZ_FACTOR", enum("none -F0 -F1 -F2 -F3"))
reg.usrlist("ACCEPTABLE_LICENSES", BtIdentifier)
reg.usr("SPECIFIC_PKGS", BtYes)
- reg.usrlist("SITE_SPECIFIC_PKGS", BtPkgPath)
- reg.usrlist("HOST_SPECIFIC_PKGS", BtPkgPath)
- reg.usrlist("GROUP_SPECIFIC_PKGS", BtPkgPath)
- reg.usrlist("USER_SPECIFIC_PKGS", BtPkgPath)
+ reg.usrlist("SITE_SPECIFIC_PKGS", BtPkgpath)
+ reg.usrlist("HOST_SPECIFIC_PKGS", BtPkgpath)
+ reg.usrlist("GROUP_SPECIFIC_PKGS", BtPkgpath)
+ reg.usrlist("USER_SPECIFIC_PKGS", BtPkgpath)
reg.usr("FAILOVER_FETCH", BtYes)
reg.usrlist("MASTER_SORT", BtUnknown)
reg.usrlist("MASTER_SORT_REGEX", BtUnknown)
@@ -629,7 +629,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.usr("ICECAST_SOURCE_BUFFSIZE", BtInteger)
reg.usr("IMAP_UW_CCLIENT_MBOX_FMT",
enum("mbox mbx mh mmdf mtx mx news phile tenex unix"))
- reg.usr("IMAP_UW_MAILSPOOLHOME", BtFileName)
+ reg.usr("IMAP_UW_MAILSPOOLHOME", BtFilename)
reg.usr("IMDICTDIR", BtPathname)
reg.usr("INN_DATA_DIR", BtPathname)
reg.usr("INN_USER", BtUserGroupName)
@@ -711,7 +711,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.usr("PILRC_USE_GTK", BtYesNo)
reg.usr("PKG_JVM_DEFAULT", jvms)
reg.usr("POPTOP_USE_MPPE", BtYes)
- reg.usr("PROCMAIL_MAILSPOOLHOME", BtFileName)
+ reg.usr("PROCMAIL_MAILSPOOLHOME", BtFilename)
// Comma-separated list of string or integer literals.
reg.usr("PROCMAIL_TRUSTED_IDS", BtUnknown)
reg.usr("PVM_SSH", BtPathname)
@@ -740,7 +740,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.usr("RSSH_CVS_PATH", BtPathname)
reg.usr("RSSH_RDIST_PATH", BtPathname)
reg.usr("RSSH_RSYNC_PATH", BtPathname)
- reg.usrlist("SAWFISH_THEMES", BtFileName)
+ reg.usrlist("SAWFISH_THEMES", BtFilename)
reg.usr("SCREWS_GROUP", BtUserGroupName)
reg.usr("SCREWS_USER", BtUserGroupName)
reg.usr("SDIST_PAWD", enum("pawd pwd"))
@@ -783,13 +783,13 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.sys(".TARGET", BtPathname)
reg.sys("@", BtPathname)
reg.pkglistbl3("ALL_ENV", BtShellWord)
- reg.pkg("ALTERNATIVES_FILE", BtFileName)
+ reg.pkg("ALTERNATIVES_FILE", BtFilename)
reg.pkglist("ALTERNATIVES_SRC", BtPathname)
reg.pkg("APACHE_MODULE", BtYes)
reg.sys("AR", BtShellCommand)
reg.sys("AS", BtShellCommand)
reg.pkglist("AUTOCONF_REQD", BtVersion)
- reg.pkglist("AUTOMAKE_OVERRIDE", BtPathmask)
+ reg.pkglist("AUTOMAKE_OVERRIDE", BtPathPattern)
reg.pkglist("AUTOMAKE_REQD", BtVersion)
reg.pkg("AUTO_MKDIRS", BtYesNo)
reg.usr("BATCH", BtYes)
@@ -841,7 +841,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.acl("BUILDLINK_DIR", BtPathname,
PackageSettable,
"*: use")
- reg.bl3list("BUILDLINK_FILES.*", BtPathmask)
+ reg.bl3list("BUILDLINK_FILES.*", BtPathPattern)
reg.pkgbl3("BUILDLINK_FILES_CMD.*", BtShellCommand)
reg.acllist("BUILDLINK_INCDIRS.*", BtPathname,
PackageSettable,
@@ -917,7 +917,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
PackageSettable,
"builtin.mk: set, use, use-loadtime",
"Makefile, Makefile.*, *.mk: use, use-loadtime")
- reg.acl("BUILTIN_PKG.*", BtPkgName,
+ reg.acl("BUILTIN_PKG.*", BtPkgname,
PackageSettable,
"builtin.mk: set, use, use-loadtime")
reg.pkglistbl3("BUILTIN_FIND_FILES_VAR", BtVariableName)
@@ -953,27 +953,27 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkglist("CHECK_FILES_SKIP", BtBasicRegularExpression)
reg.pkg("CHECK_FILES_SUPPORTED", BtYesNo)
reg.usr("CHECK_HEADERS", BtYesNo)
- reg.pkglist("CHECK_HEADERS_SKIP", BtPathmask)
+ reg.pkglist("CHECK_HEADERS_SKIP", BtPathPattern)
reg.usr("CHECK_INTERPRETER", BtYesNo)
- reg.pkglist("CHECK_INTERPRETER_SKIP", BtPathmask)
+ reg.pkglist("CHECK_INTERPRETER_SKIP", BtPathPattern)
reg.usr("CHECK_PERMS", BtYesNo)
- reg.pkglist("CHECK_PERMS_SKIP", BtPathmask)
+ reg.pkglist("CHECK_PERMS_SKIP", BtPathPattern)
reg.usr("CHECK_PORTABILITY", BtYesNo)
- reg.pkglist("CHECK_PORTABILITY_SKIP", BtPathmask)
+ reg.pkglist("CHECK_PORTABILITY_SKIP", BtPathPattern)
reg.usr("CHECK_RELRO", BtYesNo)
- reg.pkglist("CHECK_RELRO_SKIP", BtPathmask)
+ reg.pkglist("CHECK_RELRO_SKIP", BtPathPattern)
reg.pkg("CHECK_RELRO_SUPPORTED", BtYesNo)
reg.pkg("CHECK_SHLIBS", BtYesNo)
- reg.pkglist("CHECK_SHLIBS_SKIP", BtPathmask)
+ reg.pkglist("CHECK_SHLIBS_SKIP", BtPathPattern)
reg.pkg("CHECK_SHLIBS_SUPPORTED", BtYesNo)
- reg.pkglist("CHECK_WRKREF_SKIP", BtPathmask)
+ reg.pkglist("CHECK_WRKREF_SKIP", BtPathPattern)
reg.pkg("CMAKE_ARG_PATH", BtPathname)
reg.pkglist("CMAKE_ARGS", BtShellWord)
reg.pkglist("CMAKE_ARGS.*", BtShellWord)
- reg.pkglist("CMAKE_DEPENDENCIES_REWRITE", BtPathmask) // Relative to WRKSRC
- reg.pkglist("CMAKE_MODULE_PATH_OVERRIDE", BtPathmask) // Relative to WRKSRC
+ reg.pkglist("CMAKE_DEPENDENCIES_REWRITE", BtPathPattern) // Relative to WRKSRC
+ reg.pkglist("CMAKE_MODULE_PATH_OVERRIDE", BtPathPattern) // Relative to WRKSRC
reg.pkg("CMAKE_PKGSRC_BUILD_FLAGS", BtYesNo)
- reg.pkglist("CMAKE_PREFIX_PATH", BtPathmask)
+ reg.pkglist("CMAKE_PREFIX_PATH", BtPathPattern)
reg.pkg("CMAKE_USE_GNU_INSTALL_DIRS", BtYesNo)
reg.pkg("CMAKE_INSTALL_PREFIX", BtPathname) // The default is ${PREFIX}.
reg.pkgappend("COMMENT", BtComment)
@@ -988,10 +988,10 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkg("CONFIGURE_HAS_LIBDIR", BtYesNo)
reg.pkg("CONFIGURE_HAS_MANDIR", BtYesNo)
reg.pkg("CONFIGURE_SCRIPT", BtPathname)
- reg.pkglist("CONFIG_GUESS_OVERRIDE", BtPathmask)
- reg.pkglist("CONFIG_STATUS_OVERRIDE", BtPathmask)
+ reg.pkglist("CONFIG_GUESS_OVERRIDE", BtPathPattern)
+ reg.pkglist("CONFIG_STATUS_OVERRIDE", BtPathPattern)
reg.pkg("CONFIG_SHELL", BtPathname)
- reg.pkglist("CONFIG_SUB_OVERRIDE", BtPathmask)
+ reg.pkglist("CONFIG_SUB_OVERRIDE", BtPathPattern)
reg.pkglist("CONFLICTS", BtDependency)
reg.pkgappend("CONF_FILES", BtConfFiles)
reg.pkg("CONF_FILES_MODE", enum("0644 0640 0600 0400"))
@@ -1016,15 +1016,15 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkg("DESTDIR_VARNAME", BtVariableName)
reg.sys("DEVOSSAUDIO", BtPathname)
reg.sys("DEVOSSSOUND", BtPathname)
- reg.pkglist("DISTFILES", BtFileName)
+ reg.pkglist("DISTFILES", BtFilename)
reg.pkg("DISTINFO_FILE", BtRelativePkgPath)
- reg.pkg("DISTNAME", BtFileName)
+ reg.pkg("DISTNAME", BtFilename)
reg.pkg("DIST_SUBDIR", BtPathname)
reg.pkglist("DJB_BUILD_ARGS", BtShellWord)
reg.pkglist("DJB_BUILD_TARGETS", BtIdentifier)
reg.pkgappend("DJB_CONFIG_CMDS", BtShellCommands)
reg.pkglist("DJB_CONFIG_DIRS", BtWrksrcSubdirectory)
- reg.pkg("DJB_CONFIG_HOME", BtFileName)
+ reg.pkg("DJB_CONFIG_HOME", BtFilename)
reg.pkg("DJB_CONFIG_PREFIX", BtPathname)
reg.pkglist("DJB_INSTALL_TARGETS", BtIdentifier)
reg.pkg("DJB_MAKE_TARGETS", BtYesNo)
@@ -1079,7 +1079,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.sys("EXTRACT_CMD", BtShellCommand)
reg.pkg("EXTRACT_DIR", BtPathname)
reg.pkg("EXTRACT_DIR.*", BtPathname)
- reg.pkglist("EXTRACT_ELEMENTS", BtPathmask)
+ reg.pkglist("EXTRACT_ELEMENTS", BtPathPattern)
reg.pkglist("EXTRACT_ENV", BtShellWord)
reg.pkglist("EXTRACT_ONLY", BtPathname)
reg.pkglist("EXTRACT_OPTS", BtShellWord)
@@ -1109,7 +1109,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkgappend("GENERATE_PLIST", BtShellCommands)
reg.pkg("GITHUB_PROJECT", BtIdentifier)
reg.pkg("GITHUB_TAG", BtIdentifier)
- reg.pkg("GITHUB_RELEASE", BtFileName)
+ reg.pkg("GITHUB_RELEASE", BtFilename)
reg.pkg("GITHUB_TYPE", enum("tag release"))
reg.pkgrat("GMAKE_REQD", BtVersion)
// Some packages need to set GNU_ARCH.i386 to either i486 or i586.
@@ -1175,9 +1175,9 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.sys("JAVA_BINPREFIX", BtPathname)
reg.pkg("JAVA_CLASSPATH", BtShellWord)
reg.pkg("JAVA_HOME", BtPathname)
- reg.pkg("JAVA_NAME", BtFileName)
+ reg.pkg("JAVA_NAME", BtFilename)
reg.pkglist("JAVA_UNLIMIT", enum("cmdsize datasize stacksize"))
- reg.pkglist("JAVA_WRAPPERS", BtFileName)
+ reg.pkglist("JAVA_WRAPPERS", BtFilename)
reg.pkg("JAVA_WRAPPER_BIN.*", BtPathname)
reg.sys("KRB5BASE", BtPathname)
reg.pkglist("KRB5_ACCEPTED", enum("heimdal mit-krb5"))
@@ -1194,7 +1194,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkglist("LIBS", BtLdFlag)
reg.pkglist("LIBS.*", BtLdFlag)
reg.sys("LIBTOOL", BtShellCommand)
- reg.pkglist("LIBTOOL_OVERRIDE", BtPathmask)
+ reg.pkglist("LIBTOOL_OVERRIDE", BtPathPattern)
reg.pkglistrat("LIBTOOL_REQD", BtVersion)
reg.pkgappend("LICENCE", BtLicense)
reg.pkgappend("LICENSE", BtLicense)
@@ -1205,7 +1205,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.sys("LOWER_OPSYS", BtIdentifier)
reg.sys("LOWER_VENDOR", BtIdentifier)
reg.syslist("LP64PLATFORMS", BtMachinePlatformPattern)
- reg.pkglist("LTCONFIG_OVERRIDE", BtPathmask)
+ reg.pkglist("LTCONFIG_OVERRIDE", BtPathPattern)
reg.sysload("MACHINE_ARCH", enumMachineArch)
reg.sysload("MACHINE_GNU_ARCH", enumMachineGnuArch)
reg.sysload("MACHINE_GNU_PLATFORM", BtMachineGnuPlatform)
@@ -1256,8 +1256,8 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkglistrat("NOT_FOR_BULK_PLATFORM", BtMachinePlatformPattern)
reg.pkglistrat("NOT_FOR_PLATFORM", BtMachinePlatformPattern)
reg.pkgrat("NOT_FOR_UNPRIVILEGED", BtYesNo)
- reg.pkglistrat("NOT_PAX_ASLR_SAFE", BtPathmask)
- reg.pkglistrat("NOT_PAX_MPROTECT_SAFE", BtPathmask)
+ reg.pkglistrat("NOT_PAX_ASLR_SAFE", BtPathPattern)
+ reg.pkglistrat("NOT_PAX_MPROTECT_SAFE", BtPathPattern)
reg.pkg("NO_BIN_ON_CDROM", BtRestricted)
reg.pkg("NO_BIN_ON_FTP", BtRestricted)
reg.pkgload("NO_BUILD", BtYes)
@@ -1282,12 +1282,12 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkg("OVERRIDE_DIRDEPTH*", BtInteger)
reg.pkg("OVERRIDE_GNU_CONFIG_SCRIPTS", BtYes)
reg.pkg("OWNER", BtMailAddress)
- reg.pkglist("OWN_DIRS", BtPathmask)
+ reg.pkglist("OWN_DIRS", BtPathPattern)
reg.pkglist("OWN_DIRS_PERMS", BtPerms)
reg.sys("PAMBASE", BtPathname)
reg.usr("PAM_DEFAULT", enum("linux-pam openpam solaris-pam"))
reg.pkgload("PATCHDIR", BtRelativePkgPath)
- reg.pkglist("PATCHFILES", BtFileName)
+ reg.pkglist("PATCHFILES", BtFilename)
reg.pkglist("PATCH_ARGS", BtShellWord)
reg.pkglist("PATCH_DIST_ARGS", BtShellWord)
reg.pkg("PATCH_DIST_CAT", BtShellCommand)
@@ -1339,19 +1339,19 @@ func (reg *VarTypeRegistry) Init(src *Pk
PackageSettable,
"builtin.mk: set, append",
"special:pkgconfig-builtin.mk: use-loadtime")
- reg.pkglist("PKGCONFIG_OVERRIDE", BtPathmask)
+ reg.pkglist("PKGCONFIG_OVERRIDE", BtPathPattern)
reg.pkg("PKGCONFIG_OVERRIDE_STAGE", BtStage)
reg.pkg("PKGDIR", BtRelativePkgDir)
reg.sys("PKGDIRMODE", BtFileMode)
reg.sys("PKGLOCALEDIR", BtPathname)
- reg.pkg("PKGNAME", BtPkgName)
- reg.sys("PKGNAME_NOREV", BtPkgName)
- reg.sysload("PKGPATH", BtPathname)
+ reg.pkg("PKGNAME", BtPkgname)
+ reg.sys("PKGNAME_NOREV", BtPkgname)
+ reg.sysload("PKGPATH", BtPkgpath)
reg.sys("PKGREPOSITORY", BtUnknown)
// This variable is special in that it really only makes sense to
// be set in a package Makefile.
- // See VartypeCheck.PkgRevision for details.
- reg.acl("PKGREVISION", BtPkgRevision,
+ // See VartypeCheck.Pkgrevision for details.
+ reg.acl("PKGREVISION", BtPkgrevision,
PackageSettable,
"Makefile: set")
reg.sys("PKGSRCDIR", BtPathname)
@@ -1364,7 +1364,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.syslist("PKGTOOLS_ENV", BtShellWord)
reg.sys("PKGVERSION", BtVersion)
reg.sys("PKGVERSION_NOREV", BtVersion) // Without the nb* part.
- reg.sys("PKGWILDCARD", BtFileMask)
+ reg.sys("PKGWILDCARD", BtFilePattern)
reg.sysload("PKG_ADMIN", BtShellCommand)
reg.sys("PKG_APACHE", enum("apache24"))
reg.pkglistrat("PKG_APACHE_ACCEPTED", enum("apache24"))
@@ -1456,8 +1456,8 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.acl("PREFIX", BtPathname,
UserSettable,
"*: use")
- // BtPathname instead of BtPkgPath since the original package doesn't exist anymore.
- // It would be more precise to check for a PkgPath that doesn't exist anymore.
+ // BtPathname instead of BtPkgpath since the original package doesn't exist anymore.
+ // It would be more precise to check for a Pkgpath that doesn't exist anymore.
reg.pkg("PREV_PKGPATH", BtPathname)
reg.acl("PRINT_PLIST_AWK", BtAwkCommand,
PackageSettable,
@@ -1483,7 +1483,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.usr("PYTHON_VERSION_REQD", BtVersion)
reg.pkglist("PYTHON_VERSIONED_DEPENDENCIES", BtPythonDependency)
reg.sys("RANLIB", BtShellCommand)
- reg.pkglist("RCD_SCRIPTS", BtFileName)
+ reg.pkglist("RCD_SCRIPTS", BtFilename)
// TODO: Is the definition in www/squid3/Makefile detected as being redundant?
// No, but it could if the RedundancyScope were able to resolve ${FILESDIR}
// to "files".
@@ -1499,17 +1499,17 @@ func (reg *VarTypeRegistry) Init(src *Pk
// is a plain string.
reg.pkg("REPLACE.*", BtUnknown)
- reg.pkglist("REPLACE_AWK", BtPathmask)
- reg.pkglist("REPLACE_BASH", BtPathmask)
- reg.pkglist("REPLACE_CSH", BtPathmask)
- reg.pkglist("REPLACE_FILES.*", BtPathmask)
+ reg.pkglist("REPLACE_AWK", BtPathPattern)
+ reg.pkglist("REPLACE_BASH", BtPathPattern)
+ reg.pkglist("REPLACE_CSH", BtPathPattern)
+ reg.pkglist("REPLACE_FILES.*", BtPathPattern)
reg.pkglist("REPLACE_INTERPRETER", BtIdentifier)
- reg.pkglist("REPLACE_KSH", BtPathmask)
- reg.pkglist("REPLACE_LOCALEDIR_PATTERNS", BtFileMask)
- reg.pkglist("REPLACE_LUA", BtPathmask)
- reg.pkglist("REPLACE_PERL", BtPathmask)
- reg.pkglist("REPLACE_PYTHON", BtPathmask)
- reg.pkglist("REPLACE_SH", BtPathmask)
+ reg.pkglist("REPLACE_KSH", BtPathPattern)
+ reg.pkglist("REPLACE_LOCALEDIR_PATTERNS", BtFilePattern)
+ reg.pkglist("REPLACE_LUA", BtPathPattern)
+ reg.pkglist("REPLACE_PERL", BtPathPattern)
+ reg.pkglist("REPLACE_PYTHON", BtPathPattern)
+ reg.pkglist("REPLACE_SH", BtPathPattern)
reg.pkglist("REQD_DIRS", BtPathname)
reg.pkglist("REQD_DIRS_PERMS", BtPerms)
reg.pkglist("REQD_FILES", BtPathname)
@@ -1518,7 +1518,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkg("RESTRICTED", BtMessage)
reg.usr("ROOT_USER", BtUserGroupName)
reg.usr("ROOT_GROUP", BtUserGroupName)
- reg.pkglist("RPMIGNOREPATH", BtPathmask)
+ reg.pkglist("RPMIGNOREPATH", BtPathPattern)
reg.acl("RUBY_BASE",
reg.enumFromDirs(src, "lang", `^ruby(\d+)$`, "ruby$1", "ruby22 ruby23 ruby24 ruby25"),
SystemProvided,
@@ -1543,16 +1543,16 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.sys("SHAREOWN", BtUserGroupName)
reg.sys("SHCOMMENT", BtShellCommand)
reg.sys("SHLIBTOOL", BtShellCommand)
- reg.pkglist("SHLIBTOOL_OVERRIDE", BtPathmask)
+ reg.pkglist("SHLIBTOOL_OVERRIDE", BtPathPattern)
reg.sysload("SHLIB_TYPE",
enum("COFF ECOFF ELF SOM XCOFF Mach-O PE PEwin a.out aixlib dylib none"))
reg.pkglist("SITES.*", BtFetchURL)
reg.usr("SMF_PREFIS", BtPathname)
reg.pkg("SMF_SRCDIR", BtPathname)
- reg.pkg("SMF_NAME", BtFileName)
+ reg.pkg("SMF_NAME", BtFilename)
reg.pkg("SMF_MANIFEST", BtPathname)
reg.pkglist("SMF_INSTANCES", BtIdentifier)
- reg.pkglist("SMF_METHODS", BtFileName)
+ reg.pkglist("SMF_METHODS", BtFilename)
reg.pkg("SMF_METHOD_SRC.*", BtPathname)
reg.pkg("SMF_METHOD_SHELL", BtShellCommand)
reg.pkglist("SPECIAL_PERMS", BtPerms)
@@ -1560,13 +1560,13 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.sys("STRIP", BtShellCommand) // see mk/tools/strip.mk
// Only valid in the top-level and the category Makefiles.
- reg.acllist("SUBDIR", BtFileName,
+ reg.acllist("SUBDIR", BtFilename,
PackageSettable,
"Makefile: append")
reg.pkglistbl3("SUBST_CLASSES", BtIdentifier)
reg.pkglistbl3("SUBST_CLASSES.*", BtIdentifier) // OPSYS-specific
- reg.pkglistbl3("SUBST_FILES.*", BtPathmask)
+ reg.pkglistbl3("SUBST_FILES.*", BtPathPattern)
reg.pkgbl3("SUBST_FILTER_CMD.*", BtShellCommand)
reg.pkgbl3("SUBST_MESSAGE.*", BtMessage)
reg.pkgappendbl3("SUBST_SED.*", BtSedCommands)
@@ -1580,7 +1580,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkglist("TEST_TARGET", BtIdentifier)
reg.pkglistrat("TEXINFO_REQD", BtVersion)
reg.pkglistbl3("TOOL_DEPENDS", BtDependencyWithPath)
- reg.syslist("TOOLS_ALIASES", BtFileName)
+ reg.syslist("TOOLS_ALIASES", BtFilename)
reg.syslist("TOOLS_BROKEN", BtTool)
reg.sys("TOOLS_CMD.*", BtPathname)
reg.pkglist("TOOLS_CREATE", BtTool)
@@ -1596,10 +1596,10 @@ func (reg *VarTypeRegistry) Init(src *Pk
enum("cputime datasize memorysize stacksize"))
reg.usr("UNPRIVILEGED_USER", BtUserGroupName)
reg.usr("UNPRIVILEGED_GROUP", BtUserGroupName)
- reg.pkglist("UNWRAP_FILES", BtPathmask)
+ reg.pkglist("UNWRAP_FILES", BtPathPattern)
reg.usrlist("UPDATE_TARGET", BtIdentifier)
reg.pkg("USERGROUP_PHASE", enum("configure build pre-install"))
- reg.usrlist("USER_ADDITIONAL_PKGS", BtPkgPath)
+ reg.usrlist("USER_ADDITIONAL_PKGS", BtPkgpath)
reg.pkg("USE_BSD_MAKEFILE", BtYes)
// USE_BUILTIN.* is usually set by the builtin.mk file, after checking
Index: pkgsrc/pkgtools/pkglint/files/vargroups.go
diff -u pkgsrc/pkgtools/pkglint/files/vargroups.go:1.2 pkgsrc/pkgtools/pkglint/files/vargroups.go:1.3
--- pkgsrc/pkgtools/pkglint/files/vargroups.go:1.2 Thu Sep 12 21:15:48 2019
+++ pkgsrc/pkgtools/pkglint/files/vargroups.go Tue Oct 1 21:37:59 2019
@@ -54,6 +54,11 @@ func (ck *VargroupsChecker) init() {
ck.ignVars = make(map[string]*MkLine)
ck.sortedVars = make(map[string]*MkLine)
ck.listedVars = make(map[string]*MkLine)
+ userPrivate := ""
+ pkgPrivate := ""
+ sysPrivate := ""
+ defPrivate := ""
+ usePrivate := ""
group := ""
@@ -65,7 +70,7 @@ func (ck *VargroupsChecker) init() {
}
}
- appendTo := func(vars map[string]*MkLine, mkline *MkLine, public bool) {
+ appendTo := func(vars map[string]*MkLine, mkline *MkLine, publicGroup bool, firstPrivate *string) {
checkGroupName(mkline)
for _, varname := range mkline.ValueFields(mkline.Value()) {
@@ -73,11 +78,22 @@ func (ck *VargroupsChecker) init() {
continue
}
- if public && hasPrefix(varname, "_") {
+ private := hasPrefix(varname, "_")
+ if publicGroup && private {
mkline.Warnf("%s should list only variables that start with a letter, not %q.",
mkline.Varname(), varname)
}
+ if firstPrivate != nil {
+ if *firstPrivate != "" && !private {
+ mkline.Warnf("The public variable %s should be listed before the private variable %s.",
+ varname, *firstPrivate)
+ }
+ if private && *firstPrivate == "" {
+ *firstPrivate = varname
+ }
+ }
+
if ck.registered[varname] != nil {
mkline.Warnf("Duplicate variable name %s, already appeared in %s.",
varname, mkline.RefTo(ck.registered[varname]))
@@ -105,17 +121,17 @@ func (ck *VargroupsChecker) init() {
case "_VARGROUPS":
group = mkline.Value()
case "_USER_VARS.*":
- appendTo(ck.userVars, mkline, true)
+ appendTo(ck.userVars, mkline, true, &userPrivate)
case "_PKG_VARS.*":
- appendTo(ck.pkgVars, mkline, true)
+ appendTo(ck.pkgVars, mkline, true, &pkgPrivate)
case "_SYS_VARS.*":
- appendTo(ck.sysVars, mkline, true)
+ appendTo(ck.sysVars, mkline, true, &sysPrivate)
case "_DEF_VARS.*":
- appendTo(ck.defVars, mkline, false)
+ appendTo(ck.defVars, mkline, false, &defPrivate)
case "_USE_VARS.*":
- appendTo(ck.useVars, mkline, false)
+ appendTo(ck.useVars, mkline, false, &usePrivate)
case "_IGN_VARS.*":
- appendTo(ck.ignVars, mkline, false)
+ appendTo(ck.ignVars, mkline, false, nil)
case "_SORTED_VARS.*":
appendToStyle(ck.sortedVars, mkline)
case "_LISTED_VARS.*":
Index: pkgsrc/pkgtools/pkglint/files/vargroups_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vargroups_test.go:1.2 pkgsrc/pkgtools/pkglint/files/vargroups_test.go:1.3
--- pkgsrc/pkgtools/pkglint/files/vargroups_test.go:1.2 Thu Sep 12 21:15:48 2019
+++ pkgsrc/pkgtools/pkglint/files/vargroups_test.go Tue Oct 1 21:37:59 2019
@@ -202,7 +202,8 @@ func (s *Suite) Test_VargroupsChecker__u
"",
".if ${USER_VAR:U}",
".endif",
- "BUILD_DEFS+=\t${_USER_VARS.group}")
+ "BUILD_DEFS+=\t\t${_USER_VARS.group}",
+ "BUILD_DEFS_EFFECTS+=\t${_SYS_VARS.group}")
mklines.Check()
@@ -220,6 +221,8 @@ func (s *Suite) Test_VargroupsChecker__i
"",
"_VARGROUPS+=\t\tgroup",
"_IGN_VARS.group=\tPREFER_*",
+ "_IGN_VARS.group+=\t[",
+ "_UNDERSCORE=\t\t_", // This is not an isVargroups name.
"",
".if ${PREFER_PKGSRC:U} || ${WRKOBJDIR:U}",
".endif")
@@ -227,5 +230,38 @@ func (s *Suite) Test_VargroupsChecker__i
mklines.Check()
t.CheckOutputLines(
- "WARN: Makefile:6: Variable WRKOBJDIR is used but not mentioned in the _VARGROUPS section.")
+ "WARN: Makefile:5: \"[\" is not a valid variable name pattern.",
+ "WARN: Makefile:6: Variable names starting with an underscore (_UNDERSCORE) "+
+ "are reserved for internal pkgsrc use.",
+ "WARN: Makefile:6: _UNDERSCORE is defined but not used.",
+ "WARN: Makefile:6: Variable _UNDERSCORE is defined but not mentioned in the _VARGROUPS section.",
+ "WARN: Makefile:8: Variable WRKOBJDIR is used but not mentioned in the _VARGROUPS section.")
+}
+
+func (s *Suite) Test_VargroupsChecker__private_before_public(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpVartypes()
+
+ mklines := t.NewMkLines("Makefile",
+ MkCvsID,
+ "",
+ "_VARGROUPS+=\t\tgroup",
+ "_DEF_VARS.group=\t_PRIVATE1 _PRIVATE2 PUBLIC",
+ "_PRIVATE1=\t\tprivate",
+ "_PRIVATE2=\t\tprivate",
+ "PUBLIC=\t\t\tpublic")
+
+ mklines.Check()
+
+ t.CheckOutputLines(
+ "WARN: Makefile:4: The public variable PUBLIC should be listed "+
+ "before the private variable _PRIVATE1.",
+ "WARN: Makefile:5: Variable names starting with an underscore (_PRIVATE1) "+
+ "are reserved for internal pkgsrc use.",
+ "WARN: Makefile:5: _PRIVATE1 is defined but not used.",
+ "WARN: Makefile:6: Variable names starting with an underscore (_PRIVATE2) "+
+ "are reserved for internal pkgsrc use.",
+ "WARN: Makefile:6: _PRIVATE2 is defined but not used.",
+ "WARN: Makefile:7: PUBLIC is defined but not used.")
}
Index: pkgsrc/pkgtools/pkglint/files/vartype.go
diff -u pkgsrc/pkgtools/pkglint/files/vartype.go:1.36 pkgsrc/pkgtools/pkglint/files/vartype.go:1.37
--- pkgsrc/pkgtools/pkglint/files/vartype.go:1.36 Wed Aug 21 16:45:17 2019
+++ pkgsrc/pkgtools/pkglint/files/vartype.go Tue Oct 1 21:37:59 2019
@@ -250,7 +250,7 @@ func (bt *BasicType) NeedsQ() bool {
BtDistSuffix,
BtEmulPlatform,
BtFileMode,
- BtFileName,
+ BtFilename,
BtIdentifier,
BtInteger,
BtMachineGnuPlatform,
@@ -258,10 +258,10 @@ func (bt *BasicType) NeedsQ() bool {
BtOption,
BtPathname,
BtPerl5Packlist,
- BtPkgName,
+ BtPkgname,
BtPkgOptionsVar,
- BtPkgPath,
- BtPkgRevision,
+ BtPkgpath,
+ BtPkgrevision,
BtPrefixPathname,
BtPythonDependency,
BtRPkgName,
@@ -314,8 +314,8 @@ var (
BtDistSuffix = &BasicType{"DistSuffix", (*VartypeCheck).DistSuffix}
BtEmulPlatform = &BasicType{"EmulPlatform", (*VartypeCheck).EmulPlatform}
BtFetchURL = &BasicType{"FetchURL", (*VartypeCheck).FetchURL}
- BtFileName = &BasicType{"Filename", (*VartypeCheck).Filename}
- BtFileMask = &BasicType{"FileMask", (*VartypeCheck).FileMask}
+ BtFilename = &BasicType{"Filename", (*VartypeCheck).Filename}
+ BtFilePattern = &BasicType{"FilePattern", (*VartypeCheck).FilePattern}
BtFileMode = &BasicType{"FileMode", (*VartypeCheck).FileMode}
BtGccReqd = &BasicType{"GccReqd", (*VartypeCheck).GccReqd}
BtHomepage = &BasicType{"Homepage", (*VartypeCheck).Homepage}
@@ -330,14 +330,14 @@ var (
BtMessage = &BasicType{"Message", (*VartypeCheck).Message}
BtOption = &BasicType{"Option", (*VartypeCheck).Option}
BtPathlist = &BasicType{"Pathlist", (*VartypeCheck).Pathlist}
- BtPathmask = &BasicType{"PathMask", (*VartypeCheck).PathMask}
+ BtPathPattern = &BasicType{"PathPattern", (*VartypeCheck).PathPattern}
BtPathname = &BasicType{"Pathname", (*VartypeCheck).Pathname}
BtPerl5Packlist = &BasicType{"Perl5Packlist", (*VartypeCheck).Perl5Packlist}
BtPerms = &BasicType{"Perms", (*VartypeCheck).Perms}
- BtPkgName = &BasicType{"Pkgname", (*VartypeCheck).Pkgname}
- BtPkgPath = &BasicType{"PkgPath", (*VartypeCheck).PkgPath}
+ BtPkgname = &BasicType{"Pkgname", (*VartypeCheck).Pkgname}
+ BtPkgpath = &BasicType{"Pkgpath", (*VartypeCheck).Pkgpath}
BtPkgOptionsVar = &BasicType{"PkgOptionsVar", (*VartypeCheck).PkgOptionsVar}
- BtPkgRevision = &BasicType{"PkgRevision", (*VartypeCheck).PkgRevision}
+ BtPkgrevision = &BasicType{"Pkgrevision", (*VartypeCheck).Pkgrevision}
BtPrefixPathname = &BasicType{"PrefixPathname", (*VartypeCheck).PrefixPathname}
BtPythonDependency = &BasicType{"PythonDependency", (*VartypeCheck).PythonDependency}
BtRPkgName = &BasicType{"RPkgName", (*VartypeCheck).RPkgName}
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.54 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.55
--- pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.54 Wed Aug 21 16:45:17 2019
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go Tue Oct 1 21:37:59 2019
@@ -6,9 +6,9 @@ import (
func (s *Suite) Test_VartypeCheck_AwkCommand(c *check.C) {
t := s.Init(c)
- vt := NewVartypeCheckTester(t, (*VartypeCheck).AwkCommand)
+ vt := NewVartypeCheckTester(t, BtAwkCommand)
- vt.Varname("PLIST_AWK")
+ vt.Varname("PRINT_PLIST_AWK")
vt.Op(opAssignAppend)
vt.Values(
"{print $0}",
@@ -30,25 +30,65 @@ func (s *Suite) Test_VartypeCheck_AwkCom
func (s *Suite) Test_VartypeCheck_BasicRegularExpression(c *check.C) {
t := s.Init(c)
- vt := NewVartypeCheckTester(t, (*VartypeCheck).BasicRegularExpression)
+ vt := NewVartypeCheckTester(t, BtBasicRegularExpression)
- vt.Varname("REPLACE_FILES.pl")
+ vt.Varname("CHECK_FILES_SKIP")
vt.Values(
".*\\.pl$",
- ".*\\.pl$$")
- t.DisableTracing()
- vt.Values(
- ".*\\.pl$",
- ".*\\.pl$$")
+ ".*\\.pl$$",
+ "\u1E9E",
+ "\\(capture\\)\\1",
+ "\\+")
vt.Output(
"WARN: filename.mk:1: Internal pkglint error in MkLine.Tokenize at \"$\".",
- "WARN: filename.mk:3: Internal pkglint error in MkLine.Tokenize at \"$\".")
+ "WARN: filename.mk:5: In a basic regular expression, a backslash followed by \"+\" is undefined.")
+
+ // Check for special characters that appear outside of character classes.
+ vt.Values(
+ "\u0007",
+ " !\"\"\\#$$%&''()*+,-./09:;<=>?",
+ "@AZ[\\\\]^_``az{|}~",
+ "\t")
+
+ vt.OutputEmpty()
+
+ vt.Values(
+ "?",
+ "\\?",
+ "\\\\?",
+ "\\\\\\?")
+ vt.Output(
+ "WARN: filename.mk:22: In a basic regular expression, a backslash followed by \"?\" is undefined.",
+ "WARN: filename.mk:24: In a basic regular expression, a backslash followed by \"?\" is undefined.")
+
+ vt.Values(
+ "package-[0-9][0-9.]*",
+ "unclosed-[",
+ // TODO: Warn about the unclosed character class.
+ "backslash-[\\")
+
+ vt.OutputEmpty()
+
+ vt.Values(
+ // TODO: Warn about incomplete regular expression escape
+ "\\",
+ "\\\\")
+
+ vt.OutputEmpty()
+
+ vt.Values(
+ "${VAR}*",
+ "\\?${VAR}\\/")
+
+ vt.Output(
+ "WARN: filename.mk:52: In a basic regular expression, a backslash followed by \"?\" is undefined.",
+ "WARN: filename.mk:52: In a basic regular expression, a backslash followed by \"/\" is undefined.")
}
func (s *Suite) Test_VartypeCheck_BuildlinkDepmethod(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).BuildlinkDepmethod)
+ vt := NewVartypeCheckTester(s.Init(c), BtBuildlinkDepmethod)
vt.Varname("BUILDLINK_DEPMETHOD.libc")
vt.Op(opAssignDefault)
@@ -63,7 +103,7 @@ func (s *Suite) Test_VartypeCheck_Buildl
func (s *Suite) Test_VartypeCheck_Category(c *check.C) {
t := s.Init(c)
- vt := NewVartypeCheckTester(t, (*VartypeCheck).Category)
+ vt := NewVartypeCheckTester(t, BtCategory)
t.CreateFileLines("filesyscategory/Makefile",
"# empty")
@@ -83,7 +123,7 @@ func (s *Suite) Test_VartypeCheck_Catego
}
func (s *Suite) Test_VartypeCheck_CFlag(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).CFlag)
+ vt := NewVartypeCheckTester(s.Init(c), BtCFlag)
vt.tester.SetUpTool("pkg-config", "", AtRunTime)
@@ -131,7 +171,7 @@ func (s *Suite) Test_VartypeCheck_CFlag(
func (s *Suite) Test_VartypeCheck_Comment(c *check.C) {
t := s.Init(c)
- vt := NewVartypeCheckTester(t, (*VartypeCheck).Comment)
+ vt := NewVartypeCheckTester(t, BtComment)
G.Pkg = NewPackage(t.File("category/converter"))
G.Pkg.EffectivePkgbase = "converter"
@@ -169,7 +209,7 @@ func (s *Suite) Test_VartypeCheck_Commen
}
func (s *Suite) Test_VartypeCheck_ConfFiles(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).ConfFiles)
+ vt := NewVartypeCheckTester(s.Init(c), BtConfFiles)
vt.Varname("CONF_FILES")
vt.Op(opAssignAppend)
@@ -188,7 +228,7 @@ func (s *Suite) Test_VartypeCheck_ConfFi
// See Test_MkParser_Dependency.
func (s *Suite) Test_VartypeCheck_Dependency(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).Dependency)
+ vt := NewVartypeCheckTester(s.Init(c), BtDependency)
vt.Varname("CONFLICTS")
vt.Op(opAssignAppend)
@@ -298,7 +338,7 @@ func (s *Suite) Test_VartypeCheck_Depend
func (s *Suite) Test_VartypeCheck_DependencyWithPath(c *check.C) {
t := s.Init(c)
- vt := NewVartypeCheckTester(t, (*VartypeCheck).DependencyWithPath)
+ vt := NewVartypeCheckTester(t, BtDependencyWithPath)
t.CreateFileLines("category/package/Makefile")
t.CreateFileLines("databases/py-sqlite3/Makefile")
@@ -367,7 +407,7 @@ func (s *Suite) Test_VartypeCheck_Depend
}
func (s *Suite) Test_VartypeCheck_DistSuffix(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).DistSuffix)
+ vt := NewVartypeCheckTester(s.Init(c), BtDistSuffix)
vt.Varname("EXTRACT_SUFX")
vt.Values(
@@ -380,7 +420,7 @@ func (s *Suite) Test_VartypeCheck_DistSu
}
func (s *Suite) Test_VartypeCheck_EmulPlatform(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).EmulPlatform)
+ vt := NewVartypeCheckTester(s.Init(c), BtEmulPlatform)
vt.Varname("EMUL_PLATFORM")
vt.Values(
@@ -411,7 +451,9 @@ func (s *Suite) Test_VartypeCheck_EmulPl
}
func (s *Suite) Test_VartypeCheck_Enum(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), enum("jdk1 jdk2 jdk4").checker)
+ basicType := enum("jdk1 jdk2 jdk4")
+ G.Pkgsrc.vartypes.Define("JDK", basicType, UserSettable)
+ vt := NewVartypeCheckTester(s.Init(c), basicType)
vt.Varname("JDK")
vt.Op(opUseMatch)
@@ -471,7 +513,7 @@ func (s *Suite) Test_VartypeCheck_FetchU
"MASTER_SITE_OWN=\thttps://example.org/";)
t.FinishSetUp()
- vt := NewVartypeCheckTester(t, (*VartypeCheck).FetchURL)
+ vt := NewVartypeCheckTester(t, BtFetchURL)
t.SetUpMasterSite("MASTER_SITE_GNU", "http://ftp.gnu.org/pub/gnu/";)
t.SetUpMasterSite("MASTER_SITE_GITHUB", "https://github.com/";)
@@ -501,7 +543,9 @@ func (s *Suite) Test_VartypeCheck_FetchU
vt.Values(
"https://example.org/download.cgi?filename=filename&sha1=12341234";)
- vt.OutputEmpty()
+ vt.Output(
+ "WARN: filename.mk:11: The fetch URL \"https://example.org/download.cgi"; +
+ "?filename=filename&sha1=12341234\" should end with a slash.")
vt.Values(
"http://example.org/distfiles/";,
@@ -509,7 +553,12 @@ func (s *Suite) Test_VartypeCheck_FetchU
"http://example.org/download?filename=<distfile>;version=<version>")
vt.Output(
- "WARN: filename.mk:23: \"http://example.org/download?filename=<distfile>;version=<version>\" is not a valid URL.")
+ "WARN: filename.mk:22: The fetch URL \"http://example.org/download"+
+ "?filename=distfile;version=1.0\" should end with a slash.",
+ "WARN: filename.mk:23: \"http://example.org/download"+
+ "?filename=<distfile>;version=<version>\" is not a valid URL.",
+ "WARN: filename.mk:23: The fetch URL \"http://example.org/download"+
+ "?filename=<distfile>;version=<version>\" should end with a slash.")
vt.Values(
"${MASTER_SITE_GITHUB:S,^,-,:=project/archive/${DISTFILE}}")
@@ -523,6 +572,7 @@ func (s *Suite) Test_VartypeCheck_FetchU
// As of June 2019, the :S modifier is not analyzed since it is unusual.
vt.Values(
+ "${MASTER_SITE_GNU:S,$,subdir,}",
"${MASTER_SITE_GNU:S,$,subdir/,}")
vt.OutputEmpty()
@@ -532,12 +582,35 @@ func (s *Suite) Test_VartypeCheck_FetchU
"WARN: filename.mk:51: Please use ${MASTER_SITE_GITHUB:=transmission/} " +
"instead of \"https://github.com/transmission/\"; " +
"and run \"" + confMake + " help topic=github\" for further instructions.")
+
+ vt.Values(
+ "-https://example.org/distfile.tar.gz";,
+ "-http://ftp.gnu.org/pub/gnu/bash-5.0.tar.gz";,
+ "-http://ftp.gnu.org/pub/gnu/bash/bash-5.0.tar.gz";)
+
+ vt.Output(
+ "WARN: filename.mk:62: Please use ${MASTER_SITE_GNU:S,^,-,:=bash-5.0.tar.gz} "+
+ "instead of \"-http://ftp.gnu.org/pub/gnu/bash-5.0.tar.gz\".";,
+ "WARN: filename.mk:63: Please use ${MASTER_SITE_GNU:S,^,-,:=bash/bash-5.0.tar.gz} "+
+ "instead of \"-http://ftp.gnu.org/pub/gnu/bash/bash-5.0.tar.gz\".";)
+
+ vt.Values(
+ "https://example.org/pub";,
+ "https://example.org/$@";, // Doesn't make sense at all.
+ "https://example.org/?f=";,
+ "https://example.org/download:";,
+ "https://example.org/download?";)
+
+ vt.Output(
+ "WARN: filename.mk:71: The fetch URL \"https://example.org/pub\"; should end with a slash.",
+ "WARN: filename.mk:72: \"https://example.org/$@\"; is not a valid URL.",
+ "WARN: filename.mk:75: The fetch URL \"https://example.org/download?\"; should end with a slash.")
}
func (s *Suite) Test_VartypeCheck_FetchURL__without_package(c *check.C) {
t := s.Init(c)
- vt := NewVartypeCheckTester(t, (*VartypeCheck).FetchURL)
+ vt := NewVartypeCheckTester(t, BtFetchURL)
vt.Varname("MASTER_SITES")
vt.Values(
@@ -549,9 +622,9 @@ func (s *Suite) Test_VartypeCheck_FetchU
}
func (s *Suite) Test_VartypeCheck_Filename(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).Filename)
+ vt := NewVartypeCheckTester(s.Init(c), BtFilename)
- vt.Varname("FNAME")
+ vt.Varname("JAVA_NAME")
vt.Values(
"Filename with spaces.docx",
"OS/2-manual.txt")
@@ -571,41 +644,41 @@ func (s *Suite) Test_VartypeCheck_Filena
"contains the invalid characters \" \".")
}
-func (s *Suite) Test_VartypeCheck_FileMask(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).FileMask)
+func (s *Suite) Test_VartypeCheck_FilePattern(c *check.C) {
+ vt := NewVartypeCheckTester(s.Init(c), BtFilePattern)
- vt.Varname("FNAME")
+ vt.Varname("PKGWILDCARD")
vt.Values(
"filename.txt",
"*.txt",
"[12345].txt",
"[0-9].txt",
"???.txt",
- "FileMask with spaces.docx",
+ "FilePattern with spaces.docx",
"OS/2-manual.txt")
vt.Output(
- "WARN: filename.mk:6: The filename pattern \"FileMask with spaces.docx\" "+
+ "WARN: filename.mk:6: The filename pattern \"FilePattern with spaces.docx\" "+
"contains the invalid characters \" \".",
"WARN: filename.mk:7: The filename pattern \"OS/2-manual.txt\" "+
"contains the invalid character \"/\".")
vt.Op(opUseMatch)
vt.Values(
- "FileMask with spaces.docx")
+ "FilePattern with spaces.docx")
// There's no guarantee that a filename only contains [A-Za-z0-9.].
// Therefore it might be necessary to allow all characters here.
// TODO: Investigate whether this restriction is useful in practice.
vt.Output(
- "WARN: filename.mk:11: The filename pattern \"FileMask with spaces.docx\" " +
+ "WARN: filename.mk:11: The filename pattern \"FilePattern with spaces.docx\" " +
"contains the invalid characters \" \".")
}
func (s *Suite) Test_VartypeCheck_FileMode(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).FileMode)
+ vt := NewVartypeCheckTester(s.Init(c), BtFileMode)
- vt.Varname("HIGHSCORE_PERMS")
+ vt.Varname("GAMEMODE")
vt.Values(
"u+rwx",
"0600",
@@ -630,7 +703,7 @@ func (s *Suite) Test_VartypeCheck_FileMo
}
func (s *Suite) Test_VartypeCheck_GccReqd(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).GccReqd)
+ vt := NewVartypeCheckTester(s.Init(c), BtGccReqd)
vt.Varname("GCC_REQD")
vt.Op(opAssignAppend)
@@ -649,7 +722,7 @@ func (s *Suite) Test_VartypeCheck_GccReq
func (s *Suite) Test_VartypeCheck_Homepage(c *check.C) {
t := s.Init(c)
- vt := NewVartypeCheckTester(t, (*VartypeCheck).Homepage)
+ vt := NewVartypeCheckTester(t, BtHomepage)
vt.Varname("HOMEPAGE")
vt.Values(
@@ -712,7 +785,7 @@ func (s *Suite) Test_VartypeCheck_Homepa
func (s *Suite) Test_VartypeCheck_Identifier(c *check.C) {
t := s.Init(c)
- vt := NewVartypeCheckTester(t, (*VartypeCheck).Identifier)
+ vt := NewVartypeCheckTester(t, BtIdentifier)
vt.Varname("MYSQL_CHARSET")
vt.Values(
@@ -740,9 +813,9 @@ func (s *Suite) Test_VartypeCheck_Identi
func (s *Suite) Test_VartypeCheck_Integer(c *check.C) {
t := s.Init(c)
- vt := NewVartypeCheckTester(t, (*VartypeCheck).Integer)
+ vt := NewVartypeCheckTester(t, BtInteger)
- vt.Varname("NUMBER")
+ vt.Varname("MAKE_JOBS")
vt.Values(
"${OTHER_VAR}",
"123",
@@ -755,7 +828,7 @@ func (s *Suite) Test_VartypeCheck_Intege
}
func (s *Suite) Test_VartypeCheck_LdFlag(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).LdFlag)
+ vt := NewVartypeCheckTester(s.Init(c), BtLdFlag)
vt.tester.SetUpTool("pkg-config", "", AtRunTime)
@@ -812,7 +885,7 @@ func (s *Suite) Test_VartypeCheck_Licens
// Also registers the PERL5_LICENSE variable in the package.
mklines.collectVariables()
- vt := NewVartypeCheckTester(t, (*VartypeCheck).License)
+ vt := NewVartypeCheckTester(t, BtLicense)
vt.Varname("LICENSE")
vt.Values(
@@ -837,7 +910,7 @@ func (s *Suite) Test_VartypeCheck_Licens
}
func (s *Suite) Test_VartypeCheck_MachineGnuPlatform(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).MachineGnuPlatform)
+ vt := NewVartypeCheckTester(s.Init(c), BtMachineGnuPlatform)
vt.Varname("MACHINE_GNU_PLATFORM")
vt.Op(opUseMatch)
@@ -865,7 +938,7 @@ func (s *Suite) Test_VartypeCheck_Machin
}
func (s *Suite) Test_VartypeCheck_MachinePlatformPattern(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).MachinePlatformPattern)
+ vt := NewVartypeCheckTester(s.Init(c), BtMachinePlatformPattern)
vt.Varname("ONLY_FOR_PLATFORM")
vt.Op(opUseMatch)
@@ -916,7 +989,7 @@ func (s *Suite) Test_VartypeCheck_Machin
}
func (s *Suite) Test_VartypeCheck_MailAddress(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).MailAddress)
+ vt := NewVartypeCheckTester(s.Init(c), BtMailAddress)
vt.Varname("MAINTAINER")
vt.Values(
@@ -933,7 +1006,7 @@ func (s *Suite) Test_VartypeCheck_MailAd
}
func (s *Suite) Test_VartypeCheck_Message(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).Message)
+ vt := NewVartypeCheckTester(s.Init(c), BtMessage)
vt.Varname("SUBST_MESSAGE.id")
vt.Values(
@@ -945,7 +1018,7 @@ func (s *Suite) Test_VartypeCheck_Messag
}
func (s *Suite) Test_VartypeCheck_Option(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).Option)
+ vt := NewVartypeCheckTester(s.Init(c), BtOption)
G.Pkgsrc.PkgOptions["documented"] = "Option description"
G.Pkgsrc.PkgOptions["undocumented"] = ""
@@ -966,7 +1039,7 @@ func (s *Suite) Test_VartypeCheck_Option
}
func (s *Suite) Test_VartypeCheck_Pathlist(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).Pathlist)
+ vt := NewVartypeCheckTester(s.Init(c), BtPathlist)
vt.Varname("PATH")
vt.Values(
@@ -980,8 +1053,8 @@ func (s *Suite) Test_VartypeCheck_Pathli
"WARN: filename.mk:2: \"/directory with spaces\" is not a valid pathname.")
}
-func (s *Suite) Test_VartypeCheck_PathMask(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).PathMask)
+func (s *Suite) Test_VartypeCheck_PathPattern(c *check.C) {
+ vt := NewVartypeCheckTester(s.Init(c), BtPathPattern)
vt.Varname("DISTDIRS")
vt.Values(
@@ -999,7 +1072,7 @@ func (s *Suite) Test_VartypeCheck_PathMa
}
func (s *Suite) Test_VartypeCheck_Pathname(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).Pathname)
+ vt := NewVartypeCheckTester(s.Init(c), BtPathname)
vt.Varname("EGDIR")
vt.Values(
@@ -1016,7 +1089,7 @@ func (s *Suite) Test_VartypeCheck_Pathna
}
func (s *Suite) Test_VartypeCheck_Perl5Packlist(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).Perl5Packlist)
+ vt := NewVartypeCheckTester(s.Init(c), BtPerl5Packlist)
vt.Varname("PERL5_PACKLIST")
vt.Values(
@@ -1028,7 +1101,7 @@ func (s *Suite) Test_VartypeCheck_Perl5P
}
func (s *Suite) Test_VartypeCheck_Perms(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).Perms)
+ vt := NewVartypeCheckTester(s.Init(c), BtPerms)
vt.Varname("CONF_FILES_PERMS")
vt.Op(opAssignAppend)
@@ -1046,7 +1119,7 @@ func (s *Suite) Test_VartypeCheck_Perms(
}
func (s *Suite) Test_VartypeCheck_Pkgname(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).Pkgname)
+ vt := NewVartypeCheckTester(s.Init(c), BtPkgname)
vt.Varname("PKGNAME")
vt.Values(
@@ -1077,9 +1150,9 @@ func (s *Suite) Test_VartypeCheck_Pkgnam
}
func (s *Suite) Test_VartypeCheck_PkgOptionsVar(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).PkgOptionsVar)
+ vt := NewVartypeCheckTester(s.Init(c), BtPkgOptionsVar)
- vt.Varname("PKG_OPTIONS_VAR.screen")
+ vt.Varname("PKG_OPTIONS_VAR")
vt.Values(
"PKG_OPTIONS.${PKGBASE}",
"PKG_OPTIONS.anypkgbase",
@@ -1091,11 +1164,13 @@ func (s *Suite) Test_VartypeCheck_PkgOpt
"of the form \"PKG_OPTIONS.*\", not \"PKG_OPTS.mc\".")
}
-func (s *Suite) Test_VartypeCheck_PkgPath(c *check.C) {
+func (s *Suite) Test_VartypeCheck_Pkgpath(c *check.C) {
t := s.Init(c)
- vt := NewVartypeCheckTester(t, (*VartypeCheck).PkgPath)
+ vt := NewVartypeCheckTester(t, BtPkgpath)
+ t.CreateFileLines("category/Makefile")
t.CreateFileLines("category/other-package/Makefile")
+ t.CreateFileLines("wip/package/Makefile")
t.Chdir("category/package")
vt.Varname("PKGPATH")
@@ -1103,17 +1178,38 @@ func (s *Suite) Test_VartypeCheck_PkgPat
"category/other-package",
"${OTHER_VAR}",
"invalid",
- "../../invalid/relative")
+ "../../invalid/relative",
+ "wip/package",
+ "category",
+ "&&")
vt.Output(
- "ERROR: filename.mk:3: Relative path \"../../invalid/Makefile\" does not exist.",
- "WARN: filename.mk:3: \"../../invalid\" is not a valid relative package directory.",
- "ERROR: filename.mk:4: Relative path \"../../../../invalid/relative/Makefile\" does not exist.",
- "WARN: filename.mk:4: \"../../../../invalid/relative\" is not a valid relative package directory.")
+ "ERROR: filename.mk:3: There is no package in \"../../invalid\".",
+ "ERROR: filename.mk:4: There is no package in \"../../../../invalid/relative\".",
+ "ERROR: filename.mk:5: A main pkgsrc package must not depend on a pkgsrc-wip package.",
+ "ERROR: filename.mk:6: \"category\" is not a valid path to a package.",
+ "WARN: filename.mk:7: The pathname \"&&\" contains the invalid characters \"&&\".",
+ "ERROR: filename.mk:7: There is no package in \"../../&&\".")
+
+ G.Wip = true
+
+ vt.Values(
+ "wip/package")
+
+ vt.OutputEmpty()
+
+ vt.Op(opUseMatch)
+
+ vt.Values(
+ "pattern",
+ "&special&")
+
+ vt.Output(
+ "WARN: filename.mk:22: The pathname pattern \"&special&\" contains the invalid characters \"&&\".")
}
-func (s *Suite) Test_VartypeCheck_PkgRevision(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).PkgRevision)
+func (s *Suite) Test_VartypeCheck_Pkgrevision(c *check.C) {
+ vt := NewVartypeCheckTester(s.Init(c), BtPkgrevision)
vt.Varname("PKGREVISION")
vt.Values(
@@ -1131,7 +1227,7 @@ func (s *Suite) Test_VartypeCheck_PkgRev
}
func (s *Suite) Test_VartypeCheck_PrefixPathname(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).PrefixPathname)
+ vt := NewVartypeCheckTester(s.Init(c), BtPrefixPathname)
vt.Varname("PKGMANDIR")
vt.Values(
@@ -1143,7 +1239,7 @@ func (s *Suite) Test_VartypeCheck_Prefix
}
func (s *Suite) Test_VartypeCheck_PythonDependency(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).PythonDependency)
+ vt := NewVartypeCheckTester(s.Init(c), BtPythonDependency)
vt.Varname("PYTHON_VERSIONED_DEPENDENCIES")
vt.Values(
@@ -1157,7 +1253,7 @@ func (s *Suite) Test_VartypeCheck_Python
}
func (s *Suite) Test_VartypeCheck_RPkgName(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).RPkgName)
+ vt := NewVartypeCheckTester(s.Init(c), BtRPkgName)
vt.Varname("R_PKGNAME")
vt.Values(
@@ -1180,7 +1276,7 @@ func (s *Suite) Test_VartypeCheck_RPkgNa
}
func (s *Suite) Test_VartypeCheck_RPkgVer(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).RPkgVer)
+ vt := NewVartypeCheckTester(s.Init(c), BtRPkgVer)
vt.Varname("R_PKGVER")
vt.Values(
@@ -1201,7 +1297,7 @@ func (s *Suite) Test_VartypeCheck_RPkgVe
func (s *Suite) Test_VartypeCheck_RelativePkgPath(c *check.C) {
t := s.Init(c)
- vt := NewVartypeCheckTester(t, (*VartypeCheck).RelativePkgPath)
+ vt := NewVartypeCheckTester(t, BtRelativePkgPath)
t.CreateFileLines("category/other-package/Makefile")
t.Chdir("category/package")
@@ -1221,7 +1317,7 @@ func (s *Suite) Test_VartypeCheck_Relati
}
func (s *Suite) Test_VartypeCheck_Restricted(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).Restricted)
+ vt := NewVartypeCheckTester(s.Init(c), BtRestricted)
vt.Varname("NO_BIN_ON_CDROM")
vt.Values(
@@ -1232,7 +1328,7 @@ func (s *Suite) Test_VartypeCheck_Restri
}
func (s *Suite) Test_VartypeCheck_SedCommands(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).SedCommands)
+ vt := NewVartypeCheckTester(s.Init(c), BtSedCommands)
vt.Varname("SUBST_SED.dummy")
vt.Values(
@@ -1262,10 +1358,49 @@ func (s *Suite) Test_VartypeCheck_SedCom
"WARN: filename.mk:11: Unclosed shell variable starting at \"$${unclosedShellVar\".")
}
+func (s *Suite) Test_VartypeCheck_SedCommands__experimental(c *check.C) {
+ vt := NewVartypeCheckTester(s.Init(c), BtSedCommands)
+ G.Experimental = true
+
+ vt.Varname("SUBST_SED.dummy")
+
+ vt.Values(
+ "-e s,???,questions,",
+ "-e 's?from?to?g'",
+ "-E -e 's,from,to,g'")
+
+ vt.Output(
+ "WARN: filename.mk:1: The \"?\" in the word \"s,???,questions,\" may lead to unintended file globbing.")
+
+ vt.Values(
+ "-e s,?,replacement,",
+ "-e s,\\?,replacement,",
+ "-e s,\\\\?,replacement,",
+ "-e s,\\\\\\?,replacement,")
+
+ vt.Output(
+ "WARN: filename.mk:11: The \"?\" in the word \"s,?,replacement,\" may lead to unintended file globbing.",
+ "WARN: filename.mk:13: The \"?\" in the word \"s,\\\\\\\\?,replacement,\" may lead to unintended file globbing.",
+ "WARN: filename.mk:13: In a basic regular expression, a backslash followed by \"?\" is undefined.",
+ "WARN: filename.mk:14: In a basic regular expression, a backslash followed by \"?\" is undefined.")
+
+ vt.Values(
+ "-e s/dir\\\\/file/other-file/")
+
+ // No warning about backslash followed by "/" being undefined.
+ vt.OutputEmpty()
+
+ vt.Values(
+ "-e 's, ,space,g'",
+ "-e 's,\t,tab,g'")
+
+ vt.OutputEmpty()
+}
+
func (s *Suite) Test_VartypeCheck_ShellCommand(c *check.C) {
t := s.Init(c)
t.SetUpVartypes()
- vt := NewVartypeCheckTester(t, (*VartypeCheck).ShellCommand)
+ vt := NewVartypeCheckTester(t, BtShellCommand)
vt.Varname("INSTALL_CMD")
vt.Values(
@@ -1287,7 +1422,7 @@ func (s *Suite) Test_VartypeCheck_ShellC
t := s.Init(c)
t.SetUpVartypes()
t.SetUpTool("echo", "ECHO", AtRunTime)
- vt := NewVartypeCheckTester(t, (*VartypeCheck).ShellCommands)
+ vt := NewVartypeCheckTester(t, BtShellCommands)
vt.Varname("GENERATE_PLIST")
vt.Values(
@@ -1299,7 +1434,7 @@ func (s *Suite) Test_VartypeCheck_ShellC
}
func (s *Suite) Test_VartypeCheck_Stage(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).Stage)
+ vt := NewVartypeCheckTester(s.Init(c), BtStage)
vt.Varname("SUBST_STAGE.dummy")
vt.Values(
@@ -1314,7 +1449,7 @@ func (s *Suite) Test_VartypeCheck_Stage(
func (s *Suite) Test_VartypeCheck_Tool(c *check.C) {
t := s.Init(c)
- vt := NewVartypeCheckTester(t, (*VartypeCheck).Tool)
+ vt := NewVartypeCheckTester(t, BtTool)
t.SetUpTool("tool1", "", AtRunTime)
t.SetUpTool("tool2", "", AtRunTime)
@@ -1371,9 +1506,9 @@ func (s *Suite) Test_VartypeCheck_Tool(c
func (s *Suite) Test_VartypeCheck_URL(c *check.C) {
t := s.Init(c)
- vt := NewVartypeCheckTester(t, (*VartypeCheck).URL)
+ vt := NewVartypeCheckTester(t, BtURL)
- vt.Varname("HOMEPAGE")
+ vt.Varname("LATEX2HTML_ICONPATH")
vt.Values(
"# none",
"${OTHER_VAR}",
@@ -1417,7 +1552,7 @@ func (s *Suite) Test_VartypeCheck_URL(c
func (s *Suite) Test_VartypeCheck_UserGroupName(c *check.C) {
t := s.Init(c)
- vt := NewVartypeCheckTester(t, (*VartypeCheck).UserGroupName)
+ vt := NewVartypeCheckTester(t, BtUserGroupName)
vt.Varname("APACHE_USER")
vt.Values(
@@ -1433,7 +1568,7 @@ func (s *Suite) Test_VartypeCheck_UserGr
}
func (s *Suite) Test_VartypeCheck_VariableName(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).VariableName)
+ vt := NewVartypeCheckTester(s.Init(c), BtVariableName)
vt.Varname("BUILD_DEFS")
vt.Values(
@@ -1447,7 +1582,7 @@ func (s *Suite) Test_VartypeCheck_Variab
}
func (s *Suite) Test_VartypeCheck_VariableNamePattern(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).VariableNamePattern)
+ vt := NewVartypeCheckTester(s.Init(c), BtVariableNamePattern)
vt.Varname("_SORTED_VARS.group")
vt.Values(
@@ -1464,7 +1599,7 @@ func (s *Suite) Test_VartypeCheck_Variab
}
func (s *Suite) Test_VartypeCheck_Version(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).Version)
+ vt := NewVartypeCheckTester(s.Init(c), BtVersion)
vt.Varname("PERL5_REQD")
vt.Op(opAssignAppend)
@@ -1495,9 +1630,9 @@ func (s *Suite) Test_VartypeCheck_Versio
}
func (s *Suite) Test_VartypeCheck_WrapperReorder(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).WrapperReorder)
+ vt := NewVartypeCheckTester(s.Init(c), BtWrapperReorder)
- vt.Varname("WRAPPER_REORDER")
+ vt.Varname("WRAPPER_REORDER_CMDS")
vt.Op(opAssignAppend)
vt.Values(
"reorder:l:first:second",
@@ -1509,7 +1644,7 @@ func (s *Suite) Test_VartypeCheck_Wrappe
}
func (s *Suite) Test_VartypeCheck_WrapperTransform(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).WrapperTransform)
+ vt := NewVartypeCheckTester(s.Init(c), BtWrapperTransform)
vt.Varname("WRAPPER_TRANSFORM_CMDS")
vt.Op(opAssignAppend)
@@ -1529,7 +1664,7 @@ func (s *Suite) Test_VartypeCheck_Wrappe
}
func (s *Suite) Test_VartypeCheck_WrksrcSubdirectory(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).WrksrcSubdirectory)
+ vt := NewVartypeCheckTester(s.Init(c), BtWrksrcSubdirectory)
vt.Varname("BUILD_DIRS")
vt.Op(opAssignAppend)
@@ -1556,7 +1691,7 @@ func (s *Suite) Test_VartypeCheck_Wrksrc
}
func (s *Suite) Test_VartypeCheck_Yes(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).Yes)
+ vt := NewVartypeCheckTester(s.Init(c), BtYes)
vt.Varname("APACHE_MODULE")
vt.Values(
@@ -1568,7 +1703,7 @@ func (s *Suite) Test_VartypeCheck_Yes(c
"WARN: filename.mk:2: APACHE_MODULE should be set to YES or yes.",
"WARN: filename.mk:3: APACHE_MODULE should be set to YES or yes.")
- vt.Varname("PKG_DEVELOPER")
+ vt.Varname("BUILD_USES_MSGFMT")
vt.Op(opUseMatch)
vt.Values(
"yes",
@@ -1576,15 +1711,15 @@ func (s *Suite) Test_VartypeCheck_Yes(c
"${YESVAR}")
vt.Output(
- "WARN: filename.mk:11: PKG_DEVELOPER should only be used in a \".if defined(...)\" condition.",
- "WARN: filename.mk:12: PKG_DEVELOPER should only be used in a \".if defined(...)\" condition.",
- "WARN: filename.mk:13: PKG_DEVELOPER should only be used in a \".if defined(...)\" condition.")
+ "WARN: filename.mk:11: BUILD_USES_MSGFMT should only be used in a \".if defined(...)\" condition.",
+ "WARN: filename.mk:12: BUILD_USES_MSGFMT should only be used in a \".if defined(...)\" condition.",
+ "WARN: filename.mk:13: BUILD_USES_MSGFMT should only be used in a \".if defined(...)\" condition.")
}
func (s *Suite) Test_VartypeCheck_YesNo(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).YesNo)
+ vt := NewVartypeCheckTester(s.Init(c), BtYesNo)
- vt.Varname("GNU_CONFIGURE")
+ vt.Varname("PKG_DEVELOPER")
vt.Values(
"yes",
"no",
@@ -1592,14 +1727,14 @@ func (s *Suite) Test_VartypeCheck_YesNo(
"${YESVAR}")
vt.Output(
- "WARN: filename.mk:3: GNU_CONFIGURE should be set to YES, yes, NO, or no.",
- "WARN: filename.mk:4: GNU_CONFIGURE should be set to YES, yes, NO, or no.")
+ "WARN: filename.mk:3: PKG_DEVELOPER should be set to YES, yes, NO, or no.",
+ "WARN: filename.mk:4: PKG_DEVELOPER should be set to YES, yes, NO, or no.")
}
func (s *Suite) Test_VartypeCheck_YesNoIndirectly(c *check.C) {
- vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).YesNoIndirectly)
+ vt := NewVartypeCheckTester(s.Init(c), BtYesNoIndirectly)
- vt.Varname("GNU_CONFIGURE")
+ vt.Varname("IS_BUILTIN.pkgbase")
vt.Values(
"yes",
"no",
@@ -1607,24 +1742,24 @@ func (s *Suite) Test_VartypeCheck_YesNoI
"${YESVAR}")
vt.Output(
- "WARN: filename.mk:3: GNU_CONFIGURE should be set to YES, yes, NO, or no.")
+ "WARN: filename.mk:3: IS_BUILTIN.pkgbase should be set to YES, yes, NO, or no.")
}
// VartypeCheckTester helps to test the many different checks in VartypeCheck.
// It keeps track of the current variable, operator, filename, line number,
// so that the test can focus on the interesting details.
type VartypeCheckTester struct {
- tester *Tester
- checker func(cv *VartypeCheck)
- filename string
- lineno int
- varname string
- op MkOperator
+ tester *Tester
+ basicType *BasicType
+ filename string
+ lineno int
+ varname string
+ op MkOperator
}
// NewVartypeCheckTester starts the test with a filename of "filename", at line 1,
// with "=" as the operator. The variable has to be initialized explicitly.
-func NewVartypeCheckTester(t *Tester, checker func(cv *VartypeCheck)) *VartypeCheckTester {
+func NewVartypeCheckTester(t *Tester, basicType *BasicType) *VartypeCheckTester {
// This is necessary to know whether the variable name is a list type
// since in such a case each value is split into the list elements.
@@ -1632,16 +1767,14 @@ func NewVartypeCheckTester(t *Tester, ch
t.SetUpVartypes()
}
- return &VartypeCheckTester{
- t,
- checker,
- "filename.mk",
- 1,
- "",
- opAssign}
+ return &VartypeCheckTester{t, basicType, "filename.mk", 1, "", opAssign}
}
func (vt *VartypeCheckTester) Varname(varname string) {
+ vartype := G.Pkgsrc.VariableType(nil, varname)
+ assertNotNil(vartype)
+ assert(vartype.basicType == vt.basicType)
+
vt.varname = varname
vt.nextSection()
}
@@ -1708,7 +1841,7 @@ func (vt *VartypeCheckTester) Values(val
for _, lineValue := range lineValues {
valueNovar := mkline.WithoutMakeVariables(lineValue)
vc := VartypeCheck{mklines, mkline, varname, vt.op, lineValue, valueNovar, comment, false}
- vt.checker(&vc)
+ vt.basicType.checker(&vc)
}
}
Index: pkgsrc/pkgtools/pkglint/files/textproc/lexer.go
diff -u pkgsrc/pkgtools/pkglint/files/textproc/lexer.go:1.5 pkgsrc/pkgtools/pkglint/files/textproc/lexer.go:1.6
--- pkgsrc/pkgtools/pkglint/files/textproc/lexer.go:1.5 Sun Mar 10 19:01:50 2019
+++ pkgsrc/pkgtools/pkglint/files/textproc/lexer.go Tue Oct 1 21:38:00 2019
@@ -130,6 +130,13 @@ func (l *Lexer) SkipByte(b byte) bool {
return false
}
+// NextByte returns the next byte.
+func (l *Lexer) NextByte() byte {
+ b := l.rest[0]
+ l.rest = l.rest[1:]
+ return b
+}
+
// NextBytesFunc chops off the longest prefix (possibly empty) consisting
// solely of bytes for which fn returns true.
func (l *Lexer) NextBytesFunc(fn func(b byte) bool) string {
Index: pkgsrc/pkgtools/pkglint/files/textproc/lexer_test.go
diff -u pkgsrc/pkgtools/pkglint/files/textproc/lexer_test.go:1.5 pkgsrc/pkgtools/pkglint/files/textproc/lexer_test.go:1.6
--- pkgsrc/pkgtools/pkglint/files/textproc/lexer_test.go:1.5 Sun Jul 14 21:25:47 2019
+++ pkgsrc/pkgtools/pkglint/files/textproc/lexer_test.go Tue Oct 1 21:38:00 2019
@@ -143,6 +143,16 @@ func (s *Suite) Test_Lexer_SkipByte(c *c
c.Check(lexer.SkipByte(0), equals, false) // This is not a C string.
}
+func (s *Suite) Test_Lexer_NextByte(c *check.C) {
+ lexer := NewLexer("byte")
+
+ c.Check(lexer.NextByte(), equals, byte('b'))
+ c.Check(lexer.NextByte(), equals, byte('y'))
+ c.Check(lexer.NextByte(), equals, byte('t'))
+ c.Check(lexer.NextByte(), equals, byte('e'))
+ c.Check(lexer.NextByte, check.PanicMatches, "^runtime error: index out of range.*")
+}
+
func (s *Suite) Test_Lexer_NextBytesFunc(c *check.C) {
lexer := NewLexer("an alphanumerical string")
Home |
Main Index |
Thread Index |
Old Index