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: Wed Apr 3 21:49:51 UTC 2019
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile
pkgsrc/pkgtools/pkglint/files: autofix.go check_test.go licenses.go
licenses_test.go mkline.go mkline_test.go mklinechecker.go
mklinechecker_test.go mklines.go mklines_test.go mkparser.go
mkparser_test.go mktypes.go mktypes_test.go package.go
package_test.go pkglint.go pkglint_test.go pkgsrc.go pkgsrc_test.go
redundantscope.go shell.go shell_test.go testnames_test.go util.go
util_test.go var.go vardefs.go vardefs_test.go vartype_test.go
vartypecheck.go vartypecheck_test.go
Log Message:
pkgtools/pkglint: update to 5.7.4
Changes since 5.7.3:
* Warn about dependency patterns that are missing a version number,
such as ${PYPKGPREFIX}-sqlite3:../../databases/py-sqlite3.
* Suggest to replace the := assignment operator with the :sh modifier,
in some cases where the variable is not obviously used at load time.
To generate a diff of this commit:
cvs rdiff -u -r1.572 -r1.573 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.18 -r1.19 pkgsrc/pkgtools/pkglint/files/autofix.go
cvs rdiff -u -r1.36 -r1.37 pkgsrc/pkgtools/pkglint/files/check_test.go \
pkgsrc/pkgtools/pkglint/files/shell.go
cvs rdiff -u -r1.21 -r1.22 pkgsrc/pkgtools/pkglint/files/licenses.go \
pkgsrc/pkgtools/pkglint/files/licenses_test.go \
pkgsrc/pkgtools/pkglint/files/pkgsrc.go
cvs rdiff -u -r1.49 -r1.50 pkgsrc/pkgtools/pkglint/files/mkline.go \
pkgsrc/pkgtools/pkglint/files/pkglint.go
cvs rdiff -u -r1.54 -r1.55 pkgsrc/pkgtools/pkglint/files/mkline_test.go
cvs rdiff -u -r1.32 -r1.33 pkgsrc/pkgtools/pkglint/files/mklinechecker.go
cvs rdiff -u -r1.28 -r1.29 \
pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
cvs rdiff -u -r1.44 -r1.45 pkgsrc/pkgtools/pkglint/files/mklines.go \
pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
cvs rdiff -u -r1.39 -r1.40 pkgsrc/pkgtools/pkglint/files/mklines_test.go
cvs rdiff -u -r1.25 -r1.26 pkgsrc/pkgtools/pkglint/files/mkparser.go \
pkgsrc/pkgtools/pkglint/files/mkparser_test.go \
pkgsrc/pkgtools/pkglint/files/util_test.go
cvs rdiff -u -r1.12 -r1.13 pkgsrc/pkgtools/pkglint/files/mktypes.go
cvs rdiff -u -r1.8 -r1.9 pkgsrc/pkgtools/pkglint/files/mktypes_test.go
cvs rdiff -u -r1.48 -r1.49 pkgsrc/pkgtools/pkglint/files/package.go
cvs rdiff -u -r1.41 -r1.42 pkgsrc/pkgtools/pkglint/files/package_test.go
cvs rdiff -u -r1.35 -r1.36 pkgsrc/pkgtools/pkglint/files/pkglint_test.go
cvs rdiff -u -r1.19 -r1.20 pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go
cvs rdiff -u -r1.2 -r1.3 pkgsrc/pkgtools/pkglint/files/redundantscope.go \
pkgsrc/pkgtools/pkglint/files/var.go
cvs rdiff -u -r1.42 -r1.43 pkgsrc/pkgtools/pkglint/files/shell_test.go
cvs rdiff -u -r1.3 -r1.4 pkgsrc/pkgtools/pkglint/files/testnames_test.go
cvs rdiff -u -r1.40 -r1.41 pkgsrc/pkgtools/pkglint/files/util.go
cvs rdiff -u -r1.57 -r1.58 pkgsrc/pkgtools/pkglint/files/vardefs.go
cvs rdiff -u -r1.11 -r1.12 pkgsrc/pkgtools/pkglint/files/vardefs_test.go
cvs rdiff -u -r1.16 -r1.17 pkgsrc/pkgtools/pkglint/files/vartype_test.go
cvs rdiff -u -r1.52 -r1.53 pkgsrc/pkgtools/pkglint/files/vartypecheck.go
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: pkgsrc/pkgtools/pkglint/Makefile
diff -u pkgsrc/pkgtools/pkglint/Makefile:1.572 pkgsrc/pkgtools/pkglint/Makefile:1.573
--- pkgsrc/pkgtools/pkglint/Makefile:1.572 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/Makefile Wed Apr 3 21:49:51 2019
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.572 2019/03/24 13:58:38 rillig Exp $
+# $NetBSD: Makefile,v 1.573 2019/04/03 21:49:51 rillig Exp $
-PKGNAME= pkglint-5.7.3
+PKGNAME= pkglint-5.7.4
CATEGORIES= pkgtools
DISTNAME= tools
MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/}
Index: pkgsrc/pkgtools/pkglint/files/autofix.go
diff -u pkgsrc/pkgtools/pkglint/files/autofix.go:1.18 pkgsrc/pkgtools/pkglint/files/autofix.go:1.19
--- pkgsrc/pkgtools/pkglint/files/autofix.go:1.18 Sun Mar 10 19:01:50 2019
+++ pkgsrc/pkgtools/pkglint/files/autofix.go Wed Apr 3 21:49:51 2019
@@ -82,7 +82,7 @@ func (fix *Autofix) Explain(explanation
fix.explanation = explanation
}
-// ReplaceAfter replaces "from" with "to", a single time.
+// Replace replaces "from" with "to", a single time.
func (fix *Autofix) Replace(from string, to string) {
fix.ReplaceAfter("", from, to)
}
@@ -227,7 +227,9 @@ func (fix *Autofix) Delete() {
}
// Anyway has the effect of showing the diagnostic even when nothing can
-// be fixed automatically, but only if neither --show-autofix nor
+// be fixed automatically.
+//
+// As usual, the diagnostic is only shown if neither --show-autofix nor
// --autofix mode is given.
func (fix *Autofix) Anyway() {
fix.anyway = !G.Logger.IsAutofix()
Index: pkgsrc/pkgtools/pkglint/files/check_test.go
diff -u pkgsrc/pkgtools/pkglint/files/check_test.go:1.36 pkgsrc/pkgtools/pkglint/files/check_test.go:1.37
--- pkgsrc/pkgtools/pkglint/files/check_test.go:1.36 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/check_test.go Wed Apr 3 21:49:51 2019
@@ -162,7 +162,7 @@ func (t *Tester) SetUpCommandLine(args .
//
// See SetUpTool for registering tools like echo, awk, perl.
func (t *Tester) SetUpVartypes() {
- G.Pkgsrc.vartypes.Init(G.Pkgsrc)
+ G.Pkgsrc.vartypes.Init(&G.Pkgsrc)
}
func (t *Tester) SetUpMasterSite(varname string, urls ...string) {
@@ -531,22 +531,27 @@ func (t *Tester) Remove(relativeFileName
// include("including.mk",
// include("other.mk",
// "VAR= other"),
-// include("module.mk",
+// include("subdir/module.mk",
// "VAR= module",
-// include("version.mk",
+// include("subdir/version.mk",
// "VAR= version"),
-// include("env.mk",
+// include("subdir/env.mk",
// "VAR= env")))
//
// mklines := get("including.mk")
// module := get("module.mk")
+//
+// The filenames passed to the include function are all relative to the
+// same location, but that location is irrelevant in practice. The generated
+// .include lines take the relative paths into account. For example, when
+// subdir/module.mk includes subdir/version.mk, the include line is just:
+// .include "version.mk"
func (t *Tester) SetUpHierarchy() (
include func(filename string, args ...interface{}) MkLines,
get func(string) MkLines) {
files := map[string]MkLines{}
- // FIXME: Define where the filename is relative to: to the file, or to the current directory.
include = func(filename string, args ...interface{}) MkLines {
var lines []Line
lineno := 1
@@ -561,7 +566,7 @@ func (t *Tester) SetUpHierarchy() (
case string:
addLine(arg)
case MkLines:
- text := sprintf(".include %q", arg.lines.FileName)
+ text := sprintf(".include %q", relpath(path.Dir(filename), arg.lines.FileName))
addLine(text)
lines = append(lines, arg.lines.Lines...)
default:
@@ -570,9 +575,7 @@ func (t *Tester) SetUpHierarchy() (
}
mklines := NewMkLines(NewLines(filename, lines))
- // FIXME: This filename must be relative to the including file.
- G.Assertf(files[filename] == nil, "MkLines with name %q already exist.", filename)
- // FIXME: This filename must be relative to the base directory.
+ G.Assertf(files[filename] == nil, "MkLines with name %q already exists.", filename)
files[filename] = mklines
return mklines
}
@@ -585,6 +588,37 @@ func (t *Tester) SetUpHierarchy() (
return
}
+// Demonstrates that Tester.SetUpHierarchy uses relative paths for the
+// .include directives.
+func (s *Suite) Test_Tester_SetUpHierarchy(c *check.C) {
+ t := s.Init(c)
+
+ include, get := t.SetUpHierarchy()
+ include("including.mk",
+ include("other.mk",
+ "VAR= other"),
+ include("subdir/module.mk",
+ "VAR= module",
+ include("subdir/version.mk",
+ "VAR= version"),
+ include("subdir/env.mk",
+ "VAR= env")))
+
+ mklines := get("including.mk")
+
+ mklines.ForEach(func(mkline MkLine) { mkline.Notef("Text is: %s", mkline.Text) })
+
+ t.CheckOutputLines(
+ "NOTE: including.mk:1: Text is: .include \"other.mk\"",
+ "NOTE: other.mk:1: Text is: VAR= other",
+ "NOTE: including.mk:2: Text is: .include \"subdir/module.mk\"",
+ "NOTE: subdir/module.mk:1: Text is: VAR= module",
+ "NOTE: subdir/module.mk:2: Text is: .include \"version.mk\"",
+ "NOTE: subdir/version.mk:1: Text is: VAR= version",
+ "NOTE: subdir/module.mk:3: Text is: .include \"env.mk\"",
+ "NOTE: subdir/env.mk:1: Text is: VAR= env")
+}
+
// Check delegates a check to the check.Check function.
// Thereby, there is no need to distinguish between c.Check and t.Check
// in the test code.
@@ -691,8 +725,8 @@ func (t *Tester) NewMkLine(filename stri
return NewMkLine(t.NewLine(filename, lineno, text))
}
-func (t *Tester) NewShellLineChecker(filename string, lineno int, text string) *ShellLineChecker {
- return NewShellLineChecker(t.NewMkLine(filename, lineno, text))
+func (t *Tester) NewShellLineChecker(mklines MkLines, filename string, lineno int, text string) *ShellLineChecker {
+ return NewShellLineChecker(mklines, t.NewMkLine(filename, lineno, text))
}
// NewLines returns a list of simple lines that belong together.
Index: pkgsrc/pkgtools/pkglint/files/shell.go
diff -u pkgsrc/pkgtools/pkglint/files/shell.go:1.36 pkgsrc/pkgtools/pkglint/files/shell.go:1.37
--- pkgsrc/pkgtools/pkglint/files/shell.go:1.36 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/shell.go Wed Apr 3 21:49:51 2019
@@ -16,11 +16,12 @@ import (
// Or it is a variable assignment line from a Makefile with a left-hand
// side variable that is of some shell-like type; see Vartype.IsShell.
type ShellLineChecker struct {
- mkline MkLine
+ MkLines MkLines
+ mkline MkLine
}
-func NewShellLineChecker(mkline MkLine) *ShellLineChecker {
- return &ShellLineChecker{mkline}
+func NewShellLineChecker(mklines MkLines, mkline MkLine) *ShellLineChecker {
+ return &ShellLineChecker{mklines, mkline}
}
var shellCommandsType = &Vartype{lkNone, BtShellCommands, []ACLEntry{{"*", aclpAllRuntime}}, false}
@@ -41,7 +42,7 @@ func (ck *ShellLineChecker) CheckWord(to
// to the MkLineChecker. Examples for these are ${VAR:Mpattern} or $@.
p := NewMkParser(nil, token, false)
if varuse := p.VarUse(); varuse != nil && p.EOF() {
- MkLineChecker{ck.mkline}.CheckVaruse(varuse, shellWordVuc)
+ MkLineChecker{ck.MkLines, ck.mkline}.CheckVaruse(varuse, shellWordVuc)
return
}
@@ -184,7 +185,7 @@ func (ck *ShellLineChecker) checkVaruseT
}
vuc := VarUseContext{shellCommandsType, vucTimeUnknown, quoting.ToVarUseContext(), true}
- MkLineChecker{ck.mkline}.CheckVaruse(varuse, &vuc)
+ MkLineChecker{ck.MkLines, ck.mkline}.CheckVaruse(varuse, &vuc)
return true
}
@@ -375,7 +376,7 @@ func (ck *ShellLineChecker) checkHiddenA
case !contains(hiddenAndSuppress, "@"):
// Nothing is hidden at all.
- case hasPrefix(G.Mk.target, "show-") || hasSuffix(G.Mk.target, "-message"):
+ case hasPrefix(ck.MkLines.target, "show-") || hasSuffix(ck.MkLines.target, "-message"):
// In these targets, all commands may be hidden.
case hasPrefix(rest, "#"):
@@ -470,7 +471,7 @@ func (scc *SimpleCommandChecker) checkCo
case scc.handleComment():
break
default:
- if G.Opts.WarnExtra && !(G.Mk != nil && G.Mk.indentation.DependsOn("OPSYS")) {
+ if G.Opts.WarnExtra && !(scc.MkLines != nil && scc.MkLines.indentation.DependsOn("OPSYS")) {
scc.mkline.Warnf("Unknown shell command %q.", shellword)
G.Explain(
"To make the package portable to all platforms that pkgsrc supports,",
@@ -490,7 +491,7 @@ func (scc *SimpleCommandChecker) handleT
command := scc.strcmd.Name
- tool, usable := G.Tool(command, scc.time)
+ tool, usable := G.Tool(scc.MkLines, command, scc.time)
if tool != nil && !usable {
scc.mkline.Warnf("The %q tool is used but not added to USE_TOOLS.", command)
@@ -530,14 +531,14 @@ func (scc *SimpleCommandChecker) handleC
if varuse := parser.VarUse(); varuse != nil && parser.EOF() {
varname := varuse.varname
- if vartype := G.Pkgsrc.VariableType(varname); vartype != nil && vartype.basicType.name == "ShellCommand" {
+ if vartype := G.Pkgsrc.VariableType(scc.MkLines, varname); vartype != nil && vartype.basicType.name == "ShellCommand" {
scc.checkInstallCommand(shellword)
return true
}
// When the package author has explicitly defined a command
// variable, assume it to be valid.
- if G.Mk != nil && G.Mk.vars.DefinedSimilar(varname) {
+ if scc.MkLines != nil && scc.MkLines.vars.DefinedSimilar(varname) {
return true
}
if G.Pkg != nil && G.Pkg.vars.DefinedSimilar(varname) {
@@ -875,7 +876,7 @@ func (spc *ShellProgramChecker) canFail(
case "set":
}
- tool, _ := G.Tool(cmdName, RunTime)
+ tool, _ := G.Tool(spc.MkLines, cmdName, RunTime)
if tool == nil {
return true
}
@@ -939,7 +940,7 @@ func (ck *ShellLineChecker) checkInstall
defer trace.Call0()()
}
- if G.Mk == nil || !matches(G.Mk.target, `^(?:pre|do|post)-install$`) {
+ if ck.MkLines == nil || !matches(ck.MkLines.target, `^(?:pre|do|post)-install$`) {
return
}
@@ -1025,34 +1026,3 @@ func splitIntoShellTokens(line Line, tex
return
}
-
-// Example: "word1 word2;;;" => "word1", "word2;;;"
-// Compare devel/bmake/files/str.c, function brk_string.
-//
-// TODO: Move to mkline.go or mkparser.go.
-//
-// TODO: Document what this function should be used for.
-//
-// TODO: Compare with brk_string from devel/bmake, especially for backticks.
-func splitIntoMkWords(line Line, text string) (words []string, rest string) {
- if trace.Tracing {
- defer trace.Call(line, text)()
- }
-
- p := NewShTokenizer(line, text, false)
- atoms := p.ShAtoms()
- word := ""
- for _, atom := range atoms {
- if atom.Type == shtSpace && atom.Quoting == shqPlain {
- words = append(words, word)
- word = ""
- } else {
- word += atom.MkText
- }
- }
- if word != "" && atoms[len(atoms)-1].Quoting == shqPlain {
- words = append(words, word)
- word = ""
- }
- return words, word + p.parser.Rest()
-}
Index: pkgsrc/pkgtools/pkglint/files/licenses.go
diff -u pkgsrc/pkgtools/pkglint/files/licenses.go:1.21 pkgsrc/pkgtools/pkglint/files/licenses.go:1.22
--- pkgsrc/pkgtools/pkglint/files/licenses.go:1.21 Thu Feb 21 22:49:03 2019
+++ pkgsrc/pkgtools/pkglint/files/licenses.go Wed Apr 3 21:49:51 2019
@@ -3,11 +3,12 @@ package pkglint
import "netbsd.org/pkglint/licenses"
type LicenseChecker struct {
- MkLine MkLine
+ MkLines MkLines
+ MkLine MkLine
}
func (lc *LicenseChecker) Check(value string, op MkOperator) {
- expanded := resolveVariableRefs(value) // For ${PERL5_LICENSE}
+ expanded := resolveVariableRefs(lc.MkLines, value) // For ${PERL5_LICENSE}
cond := licenses.Parse(ifelseStr(op == opAssignAppend, "append-placeholder ", "") + expanded)
if cond == nil {
Index: pkgsrc/pkgtools/pkglint/files/licenses_test.go
diff -u pkgsrc/pkgtools/pkglint/files/licenses_test.go:1.21 pkgsrc/pkgtools/pkglint/files/licenses_test.go:1.22
--- pkgsrc/pkgtools/pkglint/files/licenses_test.go:1.21 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/licenses_test.go Wed Apr 3 21:49:51 2019
@@ -11,7 +11,7 @@ func (s *Suite) Test_LicenseChecker_Chec
"The licenses for most software are designed to take away ...")
mkline := t.NewMkLine("Makefile", 7, "LICENSE=dummy")
- licenseChecker := LicenseChecker{mkline}
+ licenseChecker := LicenseChecker{nil, mkline}
licenseChecker.Check("gpl-v2", opAssign)
t.CheckOutputLines(
Index: pkgsrc/pkgtools/pkglint/files/pkgsrc.go
diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.21 pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.22
--- pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.21 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/pkgsrc.go Wed Apr 3 21:49:51 2019
@@ -45,8 +45,8 @@ type Pkgsrc struct {
vartypes VarTypeRegistry
}
-func NewPkgsrc(dir string) *Pkgsrc {
- src := Pkgsrc{
+func NewPkgsrc(dir string) Pkgsrc {
+ return Pkgsrc{
dir,
make(map[string]bool),
NewTools(),
@@ -60,8 +60,6 @@ func NewPkgsrc(dir string) *Pkgsrc {
NewScope(),
make(map[string]string),
NewVarTypeRegistry()}
-
- return &src
}
func (src *Pkgsrc) loadDefaultBuildDefs() {
@@ -368,8 +366,6 @@ func (src *Pkgsrc) loadUntypedVars() {
handleMkFile := func(path string) {
mklines := LoadMk(path, 0)
if mklines != nil && len(mklines.mklines) > 0 {
- G.Assertf(G.Mk == nil, "asdf")
- G.Mk = mklines // FIXME: This is because defineVar uses G.Mk instead of the method receiver.
mklines.collectDefinedVariables()
mklines.collectUsedVariables()
for varname, mkline := range mklines.vars.firstDef {
@@ -378,7 +374,6 @@ func (src *Pkgsrc) loadUntypedVars() {
for varname, mkline := range mklines.vars.used {
define(varnameCanon(varname), mkline)
}
- G.Mk = nil
}
}
@@ -893,7 +888,7 @@ func (src *Pkgsrc) loadPkgOptions() {
// VariableType returns the type of the variable
// (possibly guessed based on the variable name),
// or nil if the type cannot even be guessed.
-func (src *Pkgsrc) VariableType(varname string) (vartype *Vartype) {
+func (src *Pkgsrc) VariableType(mklines MkLines, varname string) (vartype *Vartype) {
if trace.Tracing {
defer trace.Call(varname, trace.Result(&vartype))()
}
@@ -906,19 +901,19 @@ func (src *Pkgsrc) VariableType(varname
return vartype
}
- if tool := G.ToolByVarname(varname); tool != nil {
+ if tool := G.ToolByVarname(mklines, varname); tool != nil {
if trace.Tracing {
trace.Stepf("Use of tool %+v", tool)
}
perms := aclpUse
- if tool.Validity == AfterPrefsMk && G.Mk.Tools.SeenPrefs {
+ if tool.Validity == AfterPrefsMk && mklines.Tools.SeenPrefs {
perms |= aclpUseLoadtime
}
return &Vartype{lkNone, BtShellCommand, []ACLEntry{{"*", perms}}, false}
}
if m, toolVarname := match1(varname, `^TOOLS_(.*)`); m {
- if tool := G.ToolByVarname(toolVarname); tool != nil {
+ if tool := G.ToolByVarname(mklines, toolVarname); tool != nil {
return &Vartype{lkNone, BtPathname, []ACLEntry{{"*", aclpUse}}, false}
}
}
Index: pkgsrc/pkgtools/pkglint/files/mkline.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline.go:1.49 pkgsrc/pkgtools/pkglint/files/mkline.go:1.50
--- pkgsrc/pkgtools/pkglint/files/mkline.go:1.49 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/mkline.go Wed Apr 3 21:49:51 2019
@@ -423,47 +423,55 @@ func (mkline *MkLineImpl) ValueSplit(val
var notSpace = textproc.Space.Inverse()
-// ValueFields splits the given value, taking care of variable references.
-// Example:
+// ValueFields splits the given value in the same way as the :M variable
+// modifier, taking care of variable references. Example:
//
-// ValueFields("${VAR:Udefault value} ${VAR2}two words")
+// ValueFields("${VAR:Udefault value} ${VAR2}two words;;; 'word three'")
// => "${VAR:Udefault value}"
// "${VAR2}two"
-// "words"
+// "words;;;"
+// "'word three'"
//
// Note that even though the first word contains a space, it is not split
-// at that point since the space is inside a variable use.
+// at that point since the space is inside a variable use. Shell tokens
+// such as semicolons are also treated as normal characters. Only double
+// and single quotes are interpreted.
+//
+// Compare devel/bmake/files/str.c, function brk_string.
+//
+// TODO: Compare with brk_string from devel/bmake, especially for backticks.
func (mkline *MkLineImpl) ValueFields(value string) []string {
- tokens := mkline.Tokenize(value, false)
- var split []string
- cont := false
-
- out := func(s string) {
- if cont {
- split[len(split)-1] += s
- } else {
- split = append(split, s)
- }
+ if trace.Tracing {
+ defer trace.Call(mkline, value)()
+ }
+
+ p := NewShTokenizer(mkline.Line, value, false)
+ atoms := p.ShAtoms()
+
+ if len(atoms) > 0 && atoms[0].Type == shtSpace {
+ atoms = atoms[1:]
}
- for _, token := range tokens {
- if token.Varuse != nil {
- out(token.Text)
- cont = true
+ word := ""
+ var words []string
+ for _, atom := range atoms {
+ if atom.Type == shtSpace && atom.Quoting == shqPlain {
+ words = append(words, word)
+ word = ""
} else {
- lexer := textproc.NewLexer(token.Text)
- for !lexer.EOF() {
- for lexer.NextBytesSet(textproc.Space) != "" {
- cont = false
- }
- if word := lexer.NextBytesSet(notSpace); word != "" {
- out(word)
- cont = true
- }
- }
+ word += atom.MkText
}
}
- return split
+ if word != "" && atoms[len(atoms)-1].Quoting == shqPlain {
+ words = append(words, word)
+ word = ""
+ }
+
+ // TODO: Handle parse errors
+ rest := word + p.parser.Rest()
+ _ = rest
+
+ return words
}
func (mkline *MkLineImpl) ValueTokens() ([]*MkToken, string) {
@@ -748,6 +756,12 @@ func splitMkLine(text string) (main stri
mainWithSpaces := main
main = rtrimHspace(main)
spaceBeforeComment = mainWithSpaces[len(main):]
+ } else {
+ restWithoutSpace := strings.TrimRightFunc(rest, func(r rune) bool { return isHspace(byte(r)) })
+ if len(restWithoutSpace) < len(rest) {
+ spaceBeforeComment = rest[len(restWithoutSpace):]
+ rest = restWithoutSpace
+ }
}
return
@@ -797,9 +811,9 @@ func matchMkDirective(text string) (m bo
// This decision depends on many factors, such as whether the type of the context is
// a list of things, whether the variable is a list, whether it can contain only
// safe characters, and so on.
-func (mkline *MkLineImpl) VariableNeedsQuoting(varname string, vartype *Vartype, vuc *VarUseContext) (needsQuoting YesNoUnknown) {
+func (mkline *MkLineImpl) VariableNeedsQuoting(mklines MkLines, varuse *MkVarUse, vartype *Vartype, vuc *VarUseContext) (needsQuoting YesNoUnknown) {
if trace.Tracing {
- defer trace.Call(varname, vartype, vuc, trace.Result(&needsQuoting))()
+ defer trace.Call(varuse, vartype, vuc, trace.Result(&needsQuoting))()
}
// TODO: Systematically test this function, each and every case, from top to bottom.
@@ -845,7 +859,7 @@ func (mkline *MkLineImpl) VariableNeedsQ
// Pkglint assumes that the tool definitions don't include very
// special characters, so they can safely be used inside any quotes.
- if tool := G.ToolByVarname(varname); tool != nil {
+ if tool := G.ToolByVarname(mklines, varuse.varname); tool != nil {
switch vuc.quoting {
case VucQuotPlain:
if !vuc.IsWordPart {
@@ -879,6 +893,14 @@ func (mkline *MkLineImpl) VariableNeedsQ
if vucVartype.basicType == BtHomepage && vartype.basicType == BtFetchURL {
return no // Just for HOMEPAGE=${MASTER_SITE_*:=subdir/}.
}
+
+ // .for dir in ${PATH:C,:, ,g}
+ for _, modifier := range varuse.modifiers {
+ if modifier.ChangesWords() {
+ return unknown
+ }
+ }
+
return yes
}
@@ -889,42 +911,35 @@ func (mkline *MkLineImpl) VariableNeedsQ
}
if trace.Tracing {
- trace.Step1("Don't know whether :Q is needed for %q", varname)
+ trace.Step1("Don't know whether :Q is needed for %q", varuse.varname)
}
return unknown
}
-func (mkline *MkLineImpl) DetermineUsedVariables() []string {
- // TODO: It would be good to have these variables as MkVarUse objects
- // including the context in which they are used.
-
- var varnames []string
+// ForEachUsed calls the action for each variable that is used in the line.
+func (mkline *MkLineImpl) ForEachUsed(action func(varUse *MkVarUse, time vucTime)) {
- add := func(varname string) {
- varnames = append(varnames, varname)
- }
+ var searchIn func(text string, time vucTime) // mutually recursive with searchInVarUse
- var searchIn func(text string) // mutually recursive with searchInVarUse
-
- searchInVarUse := func(varuse *MkVarUse) {
+ searchInVarUse := func(varuse *MkVarUse, time vucTime) {
varname := varuse.varname
if !varuse.IsExpression() {
- add(varname)
+ action(varuse, time)
}
- searchIn(varname)
+ searchIn(varname, time)
for _, mod := range varuse.modifiers {
- searchIn(mod.Text)
+ searchIn(mod.Text, time)
}
}
- searchIn = func(text string) {
+ searchIn = func(text string, time vucTime) {
if !contains(text, "$") {
return
}
for _, token := range NewMkParser(nil, text, false).MkTokens() {
if token.Varuse != nil {
- searchInVarUse(token.Varuse)
+ searchInVarUse(token.Varuse, time)
}
}
}
@@ -932,27 +947,28 @@ func (mkline *MkLineImpl) DetermineUsedV
switch {
case mkline.IsVarassign():
- searchIn(mkline.Varname())
- searchIn(mkline.Value())
+ searchIn(mkline.Varname(), vucTimeParse)
+ searchIn(mkline.Value(), mkline.Op().Time())
case mkline.IsDirective() && mkline.Directive() == "for":
- searchIn(mkline.Args())
+ searchIn(mkline.Args(), vucTimeParse)
case mkline.IsDirective() && mkline.Cond() != nil:
- mkline.Cond().Walk(&MkCondCallback{VarUse: searchInVarUse})
+ mkline.Cond().Walk(&MkCondCallback{
+ VarUse: func(varuse *MkVarUse) {
+ searchInVarUse(varuse, vucTimeParse)
+ }})
case mkline.IsShellCommand():
- searchIn(mkline.ShellCommand())
+ searchIn(mkline.ShellCommand(), vucTimeRun)
case mkline.IsDependency():
- searchIn(mkline.Targets())
- searchIn(mkline.Sources())
+ searchIn(mkline.Targets(), vucTimeParse)
+ searchIn(mkline.Sources(), vucTimeParse)
case mkline.IsInclude():
- searchIn(mkline.IncludedFile())
+ searchIn(mkline.IncludedFile(), vucTimeParse)
}
-
- return varnames
}
func (mkline *MkLineImpl) UnquoteShell(str string) string {
@@ -1028,6 +1044,15 @@ func (op MkOperator) String() string {
return [...]string{"=", "!=", ":=", "+=", "?=", "use", "use-loadtime", "use-match"}[op]
}
+// Time returns the time at which the right-hand side of the assignment is
+// evaluated.
+func (op MkOperator) Time() vucTime {
+ if op == opAssignShell || op == opAssignEval {
+ return vucTimeParse
+ }
+ return vucTimeRun
+}
+
// VarUseContext defines the context in which a variable is defined
// or used. Whether that is allowed depends on:
//
Index: pkgsrc/pkgtools/pkglint/files/pkglint.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint.go:1.49 pkgsrc/pkgtools/pkglint/files/pkglint.go:1.50
--- pkgsrc/pkgtools/pkglint/files/pkglint.go:1.49 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/pkglint.go Wed Apr 3 21:49:51 2019
@@ -22,9 +22,8 @@ const confVersion = "@VERSION@"
// Pkglint is a container for all global variables of this Go package.
type Pkglint struct {
Opts CmdOpts // Command line options.
- Pkgsrc *Pkgsrc // Global data, mostly extracted from mk/*, never nil.
+ Pkgsrc Pkgsrc // Global data, mostly extracted from mk/*.
Pkg *Package // The package that is currently checked, or nil.
- Mk MkLines // The Makefile (or fragment) that is currently checked, or nil.
Todo []string // The files or directories that still need to be checked.
Wip bool // Is the currently checked file or package from pkgsrc-wip?
@@ -423,7 +422,7 @@ func findPkgsrcTopdir(dirname string) st
return ""
}
-func resolveVariableRefs(text string) (resolved string) {
+func resolveVariableRefs(mklines MkLines, text string) (resolved string) {
// TODO: How does this fit into the Scope type, which is newer than this function?
if !contains(text, "${") {
@@ -441,8 +440,8 @@ func resolveVariableRefs(text string) (r
return value
}
}
- if G.Mk != nil {
- if value, ok := G.Mk.vars.LastValueFound(varname); ok {
+ if mklines != nil {
+ if value, ok := mklines.vars.LastValueFound(varname); ok {
return value
}
}
@@ -487,7 +486,7 @@ func CheckLinesDescr(lines Lines) {
if contains(line.Text, "${") {
for _, token := range NewMkParser(nil, line.Text, false).MkTokens() {
- if token.Varuse != nil && G.Pkgsrc.VariableType(token.Varuse.varname) != nil {
+ if token.Varuse != nil && G.Pkgsrc.VariableType(nil, token.Varuse.varname) != nil {
line.Notef("Variables are not expanded in the DESCR file.")
}
}
@@ -729,14 +728,14 @@ func CheckLinesTrailingEmptyLines(lines
// The command can be "sed" or "gsed" or "${SED}".
// If a tool is returned, usable tells whether that tool has been added
// to USE_TOOLS in the current scope (file or package).
-func (pkglint *Pkglint) Tool(command string, time ToolTime) (tool *Tool, usable bool) {
+func (pkglint *Pkglint) Tool(mklines MkLines, command string, time ToolTime) (tool *Tool, usable bool) {
varname := ""
p := NewMkParser(nil, command, false)
if varUse := p.VarUse(); varUse != nil && p.EOF() {
varname = varUse.varname
}
- tools := pkglint.tools()
+ tools := pkglint.tools(mklines)
if t := tools.ByName(command); t != nil {
if tools.Usable(t, time) {
@@ -762,13 +761,13 @@ func (pkglint *Pkglint) Tool(command str
// It is not guaranteed to be usable (added to USE_TOOLS), only defined;
// that must be checked by the calling code,
// see Tool.UsableAtLoadTime and Tool.UsableAtRunTime.
-func (pkglint *Pkglint) ToolByVarname(varname string) *Tool {
- return pkglint.tools().ByVarname(varname)
+func (pkglint *Pkglint) ToolByVarname(mklines MkLines, varname string) *Tool {
+ return pkglint.tools(mklines).ByVarname(varname)
}
-func (pkglint *Pkglint) tools() *Tools {
- if pkglint.Mk != nil {
- return pkglint.Mk.Tools
+func (pkglint *Pkglint) tools(mklines MkLines) *Tools {
+ if mklines != nil {
+ return mklines.Tools
} else {
return pkglint.Pkgsrc.Tools
}
Index: pkgsrc/pkgtools/pkglint/files/mkline_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.54 pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.55
--- pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.54 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/mkline_test.go Wed Apr 3 21:49:51 2019
@@ -160,24 +160,30 @@ func (s *Suite) Test_NewMkLine__autofix_
"VARNAME +=\t${VARNAME}",
"VARNAME+ =\t${VARNAME+}",
"VARNAME+ +=\t${VARNAME+}",
+ "VARNAME+ ?=\t${VARNAME}",
"pkgbase := pkglint")
CheckFileMk(filename)
t.CheckOutputLines(
- "NOTE: ~/Makefile:2: Unnecessary space after variable name \"VARNAME\".")
+ "NOTE: ~/Makefile:2: Unnecessary space after variable name \"VARNAME\".",
+
+ // The assignment operators other than = and += cannot lead to ambiguities.
+ "NOTE: ~/Makefile:5: Unnecessary space after variable name \"VARNAME+\".")
t.SetUpCommandLine("-Wspace", "--autofix")
CheckFileMk(filename)
t.CheckOutputLines(
- "AUTOFIX: ~/Makefile:2: Replacing \"VARNAME +=\" with \"VARNAME+=\".")
+ "AUTOFIX: ~/Makefile:2: Replacing \"VARNAME +=\" with \"VARNAME+=\".",
+ "AUTOFIX: ~/Makefile:5: Replacing \"VARNAME+ ?=\" with \"VARNAME+?=\".")
t.CheckFileLines("Makefile",
MkRcsID+"",
"VARNAME+=\t${VARNAME}",
"VARNAME+ =\t${VARNAME+}",
"VARNAME+ +=\t${VARNAME+}",
+ "VARNAME+?=\t${VARNAME}",
"pkgbase := pkglint")
}
@@ -269,7 +275,7 @@ func (s *Suite) Test_VarUseContext_Strin
t := s.Init(c)
t.SetUpVartypes()
- vartype := G.Pkgsrc.VariableType("PKGNAME")
+ vartype := G.Pkgsrc.VariableType(nil, "PKGNAME")
vuc := VarUseContext{vartype, vucTimeUnknown, VucQuotBackt, false}
c.Check(vuc.String(), equals, "(Pkgname time:unknown quoting:backt wordpart:false)")
@@ -361,8 +367,8 @@ func (s *Suite) Test_MkLine_VariableNeed
mkline := t.NewMkLine("filename.mk", 1, "PKGNAME:= ${UNKNOWN}")
t.SetUpVartypes()
- vuc := VarUseContext{G.Pkgsrc.VariableType("PKGNAME"), vucTimeParse, VucQuotUnknown, false}
- nq := mkline.VariableNeedsQuoting("UNKNOWN", nil, &vuc)
+ vuc := VarUseContext{G.Pkgsrc.VariableType(nil, "PKGNAME"), vucTimeParse, VucQuotUnknown, false}
+ nq := mkline.VariableNeedsQuoting(nil, &MkVarUse{"UNKNOWN", nil}, nil, &vuc)
c.Check(nq, equals, unknown)
}
@@ -375,11 +381,11 @@ func (s *Suite) Test_MkLine_VariableNeed
mkline := t.NewMkLine("Makefile", 95, "MASTER_SITES=\t${HOMEPAGE}")
vuc := VarUseContext{G.Pkgsrc.vartypes.Canon("MASTER_SITES"), vucTimeRun, VucQuotPlain, false}
- nq := mkline.VariableNeedsQuoting("HOMEPAGE", G.Pkgsrc.vartypes.Canon("HOMEPAGE"), &vuc)
+ nq := mkline.VariableNeedsQuoting(nil, &MkVarUse{"HOMEPAGE", nil}, G.Pkgsrc.vartypes.Canon("HOMEPAGE"), &vuc)
c.Check(nq, equals, no)
- MkLineChecker{mkline}.checkVarassign()
+ MkLineChecker{nil, mkline}.checkVarassign()
t.CheckOutputEmpty() // Up to version 5.3.6, pkglint warned about a missing :Q here, which was wrong.
}
@@ -391,7 +397,7 @@ func (s *Suite) Test_MkLine_VariableNeed
t.SetUpMasterSite("MASTER_SITE_SOURCEFORGE", "http://downloads.sourceforge.net/sourceforge/";)
mkline := t.NewMkLine("Makefile", 96, "MASTER_SITES=\t${MASTER_SITE_SOURCEFORGE:=squirrel-sql/}")
- MkLineChecker{mkline}.checkVarassign()
+ MkLineChecker{nil, mkline}.checkVarassign()
// Assigning lists to lists is ok.
t.CheckOutputEmpty()
@@ -404,7 +410,7 @@ func (s *Suite) Test_MkLine_VariableNeed
mkline := t.NewMkLine("builtin.mk", 3,
"USE_BUILTIN.Xfixes!=\t${PKG_ADMIN} pmatch 'pkg-[0-9]*' ${BUILTIN_PKG.Xfixes:Q}")
- MkLineChecker{mkline}.checkVarassign()
+ MkLineChecker{nil, mkline}.checkVarassign()
t.CheckOutputLines(
"WARN: builtin.mk:3: PKG_ADMIN should not be used at load time in any file.",
@@ -418,7 +424,7 @@ func (s *Suite) Test_MkLine_VariableNeed
mkline := t.NewMkLine("Makefile", 3,
"SUBST_SED.hpath=\t-e 's|^\\(INSTALL[\t:]*=\\).*|\\1${INSTALL}|'")
- MkLineChecker{mkline}.checkVarassign()
+ MkLineChecker{nil, mkline}.checkVarassign()
t.CheckOutputLines(
"WARN: Makefile:3: Please use ${INSTALL:Q} instead of ${INSTALL} " +
@@ -432,12 +438,12 @@ func (s *Suite) Test_MkLine_VariableNeed
t.SetUpTool("find", "FIND", AtRunTime)
t.SetUpTool("sort", "SORT", AtRunTime)
G.Pkg = NewPackage(t.File("category/pkgbase"))
- G.Mk = t.NewMkLines("Makefile",
+ mklines := t.NewMkLines("Makefile",
MkRcsID,
"GENERATE_PLIST= cd ${DESTDIR}${PREFIX}; ${FIND} * \\( -type f -or -type l \\) | ${SORT};")
- G.Mk.collectDefinedVariables()
- MkLineChecker{G.Mk.mklines[1]}.Check()
+ mklines.collectDefinedVariables()
+ MkLineChecker{mklines, mklines.mklines[1]}.Check()
t.CheckOutputLines(
"WARN: Makefile:2: The exitcode of \"${FIND}\" at the left of the | operator is ignored.")
@@ -447,11 +453,11 @@ func (s *Suite) Test_MkLine_VariableNeed
t := s.Init(c)
t.SetUpVartypes()
- G.Mk = t.NewMkLines("Makefile",
+ mklines := t.NewMkLines("Makefile",
MkRcsID,
"EGDIR=\t${EGDIR}/${MACHINE_GNU_PLATFORM}")
- MkLineChecker{G.Mk.mklines[1]}.Check()
+ MkLineChecker{mklines, mklines.mklines[1]}.Check()
t.CheckOutputEmpty()
}
@@ -485,11 +491,11 @@ func (s *Suite) Test_MkLine_VariableNeed
t := s.Init(c)
t.SetUpVartypes()
- G.Mk = t.NewMkLines("Makefile",
+ mklines := t.NewMkLines("Makefile",
MkRcsID,
"MASTER_SITES=${HOMEPAGE}archive/")
- MkLineChecker{G.Mk.mklines[1]}.Check()
+ MkLineChecker{mklines, mklines.mklines[1]}.Check()
t.CheckOutputEmpty() // Don't suggest to use ${HOMEPAGE:Q}.
}
@@ -523,13 +529,13 @@ func (s *Suite) Test_MkLine_VariableNeed
t.SetUpVartypes()
t.SetUpTool("awk", "AWK", AtRunTime)
t.SetUpTool("echo", "ECHO", AtRunTime)
- G.Mk = t.NewMkLines("xpi.mk",
+ mklines := t.NewMkLines("xpi.mk",
MkRcsID,
"\t id=$$(${AWK} '{print}' < ${WRKSRC}/idfile) && echo \"$$id\"",
"\t id=`${AWK} '{print}' < ${WRKSRC}/idfile` && echo \"$$id\"")
- MkLineChecker{G.Mk.mklines[1]}.Check()
- MkLineChecker{G.Mk.mklines[2]}.Check()
+ MkLineChecker{mklines, mklines.mklines[1]}.Check()
+ MkLineChecker{mklines, mklines.mklines[2]}.Check()
// Don't suggest to use ${AWK:Q}.
t.CheckOutputLines(
@@ -543,13 +549,13 @@ func (s *Suite) Test_MkLine_VariableNeed
t := s.Init(c)
t.SetUpVartypes()
- G.Mk = t.NewMkLines("x11/mlterm/Makefile",
+ mklines := t.NewMkLines("x11/mlterm/Makefile",
MkRcsID,
"SUBST_SED.link=-e 's|(LIBTOOL_LINK).*(LIBS)|& ${LDFLAGS:M*:Q}|g'",
"SUBST_SED.link=-e 's|(LIBTOOL_LINK).*(LIBS)|& '${LDFLAGS:M*:Q}'|g'")
- MkLineChecker{G.Mk.mklines[1]}.Check()
- MkLineChecker{G.Mk.mklines[2]}.Check()
+ MkLineChecker{mklines, mklines.mklines[1]}.Check()
+ MkLineChecker{mklines, mklines.mklines[2]}.Check()
t.CheckOutputLines(
"WARN: x11/mlterm/Makefile:2: Please move ${LDFLAGS:M*:Q} outside of any quoting characters.")
@@ -565,11 +571,11 @@ func (s *Suite) Test_MkLine_VariableNeed
t := s.Init(c)
t.SetUpVartypes()
- G.Mk = t.NewMkLines("Makefile",
+ mklines := t.NewMkLines("Makefile",
MkRcsID,
"PKG_SUGGESTED_OPTIONS+=\t${PKG_DEFAULT_OPTIONS:Mcdecimal} ${PKG_OPTIONS.py-trytond:Mcdecimal}")
- MkLineChecker{G.Mk.mklines[1]}.Check()
+ MkLineChecker{mklines, mklines.mklines[1]}.Check()
// No warning about a missing :Q modifier.
t.CheckOutputEmpty()
@@ -581,11 +587,11 @@ func (s *Suite) Test_MkLine_VariableNeed
t.SetUpTool("echo", "ECHO", AtRunTime)
t.SetUpTool("sh", "SH", AtRunTime)
t.SetUpVartypes()
- G.Mk = t.NewMkLines("x11/labltk/Makefile",
+ mklines := t.NewMkLines("x11/labltk/Makefile",
MkRcsID,
"CONFIGURE_ARGS+=\t-tklibs \"`${SH} -c '${ECHO} $$TK_LD_FLAGS'`\"")
- MkLineChecker{G.Mk.mklines[1]}.Check()
+ MkLineChecker{mklines, mklines.mklines[1]}.Check()
// Don't suggest ${ECHO:Q} here.
t.CheckOutputEmpty()
@@ -595,10 +601,10 @@ func (s *Suite) Test_MkLine_VariableNeed
t := s.Init(c)
t.SetUpVartypes()
- G.Mk = t.NewMkLines("x11/qt5-qtbase/Makefile.common",
+ mklines := t.NewMkLines("x11/qt5-qtbase/Makefile.common",
"BUILDLINK_TRANSFORM+=opt:-ldl:${BUILDLINK_LDADD.dl:M*}")
- MkLineChecker{G.Mk.mklines[0]}.Check()
+ MkLineChecker{mklines, mklines.mklines[0]}.Check()
// Note: The :M* modifier is not necessary, since this is not a GNU Configure package.
t.CheckOutputLines(
@@ -609,10 +615,10 @@ func (s *Suite) Test_MkLine_VariableNeed
t := s.Init(c)
t.SetUpVartypes()
- G.Mk = t.NewMkLines("benchmarks/iozone/Makefile",
+ mklines := t.NewMkLines("benchmarks/iozone/Makefile",
"SUBST_MESSAGE.crlf=\tStripping EOL CR in ${REPLACE_PERL}")
- MkLineChecker{G.Mk.mklines[0]}.Check()
+ MkLineChecker{mklines, mklines.mklines[0]}.Check()
// Don't suggest ${REPLACE_PERL:Q}.
t.CheckOutputEmpty()
@@ -622,12 +628,12 @@ func (s *Suite) Test_MkLine_VariableNeed
t := s.Init(c)
t.SetUpVartypes()
- G.Mk = t.NewMkLines("audio/jack-rack/Makefile",
+ mklines := t.NewMkLines("audio/jack-rack/Makefile",
MkRcsID,
"LADSPA_PLUGIN_PATH=\t${PREFIX}/lib/ladspa",
"CPPFLAGS+=\t\t-DLADSPA_PATH=\"\\\"${LADSPA_PLUGIN_PATH}\\\"\"")
- G.Mk.Check()
+ mklines.Check()
t.CheckOutputLines(
"WARN: audio/jack-rack/Makefile:3: The variable LADSPA_PLUGIN_PATH should be quoted as part of a shell word.")
@@ -637,11 +643,11 @@ func (s *Suite) Test_MkLine_VariableNeed
t := s.Init(c)
t.SetUpVartypes()
- G.Mk = t.NewMkLines("x11/eterm/Makefile",
+ mklines := t.NewMkLines("x11/eterm/Makefile",
MkRcsID,
"DISTFILES=\t${DEFAULT_DISTFILES} ${PIXMAP_FILES}")
- G.Mk.Check()
+ mklines.Check()
// Don't warn about missing :Q modifiers.
t.CheckOutputLines(
@@ -653,11 +659,11 @@ func (s *Suite) Test_MkLine_VariableNeed
t.SetUpMasterSite("MASTER_SITE_GNOME", "http://ftp.gnome.org/";)
t.SetUpVartypes()
- G.Mk = t.NewMkLines("x11/gtk3/Makefile",
+ mklines := t.NewMkLines("x11/gtk3/Makefile",
MkRcsID,
"MASTER_SITES=\tftp://ftp.gtk.org/${PKGNAME}/ ${MASTER_SITE_GNOME:=subdir/}")
- MkLineChecker{G.Mk.mklines[1]}.checkVarassignRightVaruse()
+ MkLineChecker{mklines, mklines.mklines[1]}.checkVarassignRightVaruse()
t.CheckOutputEmpty() // Don't warn about missing :Q modifiers.
}
@@ -672,7 +678,7 @@ func (s *Suite) Test_MkLine_VariableNeed
"",
"CONFIGURE_ENV+=\tSYS_TAR_COMMAND_PATH=${TOOLS_TAR:Q}")
- MkLineChecker{mklines.mklines[2]}.checkVarassignRightVaruse()
+ MkLineChecker{mklines, mklines.mklines[2]}.checkVarassignRightVaruse()
// The TOOLS_* variables only contain the path to the tool,
// without any additional arguments that might be necessary
@@ -693,8 +699,8 @@ func (s *Suite) Test_MkLine_VariableNeed
"COMPILE_CMD=\tcc `${CAT} ${WRKDIR}/compileflags`",
"COMMENT_CMD=\techo `echo ${COMMENT}`")
- MkLineChecker{mklines.mklines[2]}.checkVarassignRightVaruse()
- MkLineChecker{mklines.mklines[3]}.checkVarassignRightVaruse()
+ MkLineChecker{mklines, mklines.mklines[2]}.checkVarassignRightVaruse()
+ MkLineChecker{mklines, mklines.mklines[3]}.checkVarassignRightVaruse()
// Both CAT and WRKDIR are safe from quoting, therefore no warnings.
// But COMMENT may contain arbitrary characters and therefore must
@@ -807,12 +813,6 @@ func (s *Suite) Test_MkLine_VariableNeed
"\tIf these rules seem to be incorrect, please ask on the",
"\ttech-pkg%NetBSD.org@localhost mailing list.",
"",
- "WARN: ~/Makefile:4: Please use ${LINKER_RPATH_FLAG:S/-rpath/& /:Q} "+
- "instead of ${LINKER_RPATH_FLAG:S/-rpath/& /}.",
- "",
- "\tSee the pkgsrc guide, section \"Echoing a string exactly as-is\":",
- "\thttps://www.NetBSD.org/docs/pkgsrc/pkgsrc.html#echo-literal";,
- "",
"WARN: ~/Makefile:4: LINKER_RPATH_FLAG should not be used at load time in any file.",
"",
"\tMany variables, especially lists of something, get their values",
@@ -893,7 +893,7 @@ func (s *Suite) Test_MkLine_VariableNeed
// Just for branch coverage.
trace.Tracing = false
- MkLineChecker{mklines.mklines[2]}.Check()
+ MkLineChecker{mklines, mklines.mklines[2]}.Check()
t.CheckOutputEmpty()
}
@@ -1021,8 +1021,8 @@ func (s *Suite) Test_MkLine_Fields__vara
test("word '${VAR}single ${VAR}' \"\t\"",
"word",
- "'${VAR}single", "${VAR}'", // FIXME: should be a single word.
- "\"", "\"") // FIXME: should be a single word.
+ "'${VAR}single ${VAR}'",
+ "\"\t\"")
}
func (s *Suite) Test_MkLine_Fields__for(c *check.C) {
@@ -1053,8 +1053,27 @@ func (s *Suite) Test_MkLine_Fields__for(
"i",
"in",
"word",
- "'${VAR}single", "${VAR}'", // FIXME: should be a single word.
- "\"", "\"") // FIXME: should be a single word.
+ "'${VAR}single ${VAR}'",
+ "\"\t\"")
+}
+
+func (s *Suite) Test_MkLine_Fields__semicolons(c *check.C) {
+ t := s.Init(c)
+
+ mkline := t.NewMkLine("filename.mk", 123, "VAR=\tword1 word2;;;")
+ words := mkline.Fields()
+
+ c.Check(words, deepEquals, []string{"word1", "word2;;;"})
+}
+
+func (s *Suite) Test_MkLine_Fields__varuse_with_embedded_space(c *check.C) {
+ t := s.Init(c)
+
+ mkline := t.NewMkLine("filename.mk", 123, "VAR=\t${VAR:S/ /_/g}")
+
+ words := mkline.Fields()
+
+ c.Check(words, deepEquals, []string{"${VAR:S/ /_/g}"})
}
func (s *Suite) Test_MkLine_ValueFields(c *check.C) {
@@ -1071,10 +1090,12 @@ func (s *Suite) Test_MkLine_ValueFields(
"two",
"${THREE:Uthree:Nsome \tspaces}")
- test("${VAR:Udefault value} ${VAR2}two words",
+ // The example from the ValueFields documentation.
+ test("${VAR:Udefault value} ${VAR2}two words;;; 'word three'",
"${VAR:Udefault value}",
"${VAR2}two",
- "words")
+ "words;;;",
+ "'word three'")
}
// Before 2018-11-26, this test panicked.
@@ -1093,6 +1114,31 @@ func (s *Suite) Test_MkLine_ValueFields_
"${WRKSRC}")
}
+func (s *Suite) Test_MkLine_ValueFields__compared_to_splitIntoShellTokens(c *check.C) {
+ t := s.Init(c)
+ url := "http://registry.gimp.org/file/fix-ca.c?action=download&id=9884&file=";
+ mkline := t.NewMkLine("filename.mk", 123, "MASTER_SITES=\t"+url)
+
+ words, rest := splitIntoShellTokens(dummyLine, url) // Doesn't really make sense
+
+ c.Check(words, check.DeepEquals, []string{
+ "http://registry.gimp.org/file/fix-ca.c?action=download";,
+ "&",
+ "id=9884",
+ "&",
+ "file="})
+ c.Check(rest, equals, "")
+
+ words = mkline.ValueFields(url)
+
+ c.Check(words, check.DeepEquals, []string{url})
+
+ words = mkline.ValueFields("a b \"c c c\" d;;d;; \"e\"''`` 'rest")
+
+ c.Check(words, check.DeepEquals, []string{"a", "b", "\"c c c\"", "d;;d;;", "\"e\"''``"})
+ // TODO: c.Check(rest, equals, "'rest")
+}
+
func (s *Suite) Test_MkLine_ValueTokens(c *check.C) {
t := s.Init(c)
@@ -1269,6 +1315,29 @@ func (s *Suite) Test_MatchVarassign(c *c
test("VAR=value #comment", false, "VAR", "", "=", "VAR=", "value", " ", "#comment")
test("NFILES=${FILES:[#]}", false, "NFILES", "", "=", "NFILES=", "${FILES:[#]}", "", "")
+ // To humans, the base variable name seems to be SITES_, being parameterized
+ // with distfile-1.0.tar.gz. For pkglint though, the base variable name is
+ // SITES_distfile-1.
+ test("SITES_distfile-1.0.tar.gz=https://example.org/";,
+ false,
+ "SITES_distfile-1.0.tar.gz",
+ "",
+ "=",
+ "SITES_distfile-1.0.tar.gz=",
+ "https://example.org/";,
+ "",
+ "")
+
+ test("SITES_${distfile}=https://example.org/";,
+ false,
+ "SITES_${distfile}",
+ "",
+ "=",
+ "SITES_${distfile}=",
+ "https://example.org/";,
+ "",
+ "")
+
testInvalid("\tVAR=value")
testInvalid("?=value")
testInvalid("<=value")
@@ -1474,7 +1543,7 @@ func (s *Suite) Test_Indentation_Varname
"(depending on OPSYS, OPSYS) and unconditionally in Makefile:20.")
}
-func (s *Suite) Test_MkLine_DetermineUsedVariables(c *check.C) {
+func (s *Suite) Test_MkLine_ForEachUsed(c *check.C) {
t := s.Init(c)
mklines := t.NewMkLines("Makefile",
@@ -1495,30 +1564,32 @@ func (s *Suite) Test_MkLine_DetermineUse
var varnames []string
for _, mkline := range mklines.mklines {
- varnames = append(varnames, mkline.DetermineUsedVariables()...)
+ mkline.ForEachUsed(func(varUse *MkVarUse, time vucTime) {
+ varnames = append(varnames, time.String()+" "+varUse.varname)
+ })
}
c.Check(varnames, deepEquals, []string{
- "VALUE",
- "OPSYS",
- "endianness",
+ "run VALUE",
+ "parse OPSYS",
+ "parse endianness",
// "Hello" is not a variable name, the :L modifier makes it an expression.
- "two",
- "TARGETS",
- "SOURCES",
- "OTHER_FILE",
-
- "VAR.${param}",
- "param",
- "VAR",
- "VAR2",
- "VAR",
- "pattern",
- "ROUND_PARENTHESES",
+ "parse two",
+ "parse TARGETS",
+ "parse SOURCES",
+ "parse OTHER_FILE",
+
+ "run VAR.${param}",
+ "run param",
+ "run VAR",
+ "run VAR2",
+ "run VAR",
+ "run pattern",
+ "run ROUND_PARENTHESES",
// Shell variables are ignored here.
- "<",
- "@",
- "x"})
+ "run <",
+ "run @",
+ "run x"})
}
func (s *Suite) Test_MkLine_UnquoteShell(c *check.C) {
@@ -1857,14 +1928,23 @@ func (s *Suite) Test_splitMkLine(c *chec
false,
"")
- // When a parse error occurs, the comment is not parsed and the main text
- // is not trimmed to the right, to keep as much original information as
- // possible.
+ // Even if there is a parse error in the main part,
+ // the comment is extracted.
+ test("text before ${UNCLOSED# comment",
+ "text before ",
+ tokens(text("text before ")),
+ "${UNCLOSED",
+ "",
+ true,
+ " comment")
+
+ // Even in case of parse errors, the space before the comment is parsed
+ // correctly.
test("text before ${UNCLOSED # comment",
"text before ",
tokens(text("text before ")),
- "${UNCLOSED ", // FIXME: put the space into spaceBeforeComment
- "", // FIXME: the space is missing here
+ "${UNCLOSED",
+ " ",
true,
" comment")
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.32 pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.33
--- pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.32 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker.go Wed Apr 3 21:49:51 2019
@@ -11,7 +11,8 @@ import (
// MkLineChecker provides checks for a single line from a Makefile fragment.
type MkLineChecker struct {
- MkLine MkLine
+ MkLines MkLines
+ MkLine MkLine
}
func (ck MkLineChecker) Check() {
@@ -60,7 +61,7 @@ func (ck MkLineChecker) checkShellComman
}
ck.checkText(shellCommand)
- NewShellLineChecker(mkline).CheckShellCommandLine(shellCommand)
+ NewShellLineChecker(ck.MkLines, mkline).CheckShellCommandLine(shellCommand)
}
func (ck MkLineChecker) checkInclude() {
@@ -70,7 +71,7 @@ func (ck MkLineChecker) checkInclude() {
mkline := ck.MkLine
if mkline.Indent() != "" {
- ck.checkDirectiveIndentation(G.Mk.indentation.Depth("include"))
+ ck.checkDirectiveIndentation(ck.MkLines.indentation.Depth("include"))
}
includedFile := mkline.IncludedFile()
@@ -223,14 +224,14 @@ func (ck MkLineChecker) checkDirectiveFo
// whether guessed was true or false.
forLoopType := Vartype{lkShell, btForLoop, []ACLEntry{{"*", aclpAllRead}}, false}
forLoopContext := VarUseContext{&forLoopType, vucTimeParse, VucQuotPlain, false}
- for _, itemsVar := range mkline.DetermineUsedVariables() {
- ck.CheckVaruse(&MkVarUse{itemsVar, nil}, &forLoopContext)
- }
+ mkline.ForEachUsed(func(varUse *MkVarUse, time vucTime) {
+ ck.CheckVaruse(varUse, &forLoopContext)
+ })
}
}
func (ck MkLineChecker) checkDirectiveIndentation(expectedDepth int) {
- if G.Mk == nil || !G.Opts.WarnSpace {
+ if ck.MkLines == nil || !G.Opts.WarnSpace {
return
}
mkline := ck.MkLine
@@ -298,7 +299,7 @@ func (ck MkLineChecker) checkVarassignLe
mkline := ck.MkLine
varname := mkline.Varname()
op := mkline.Op()
- vartype := G.Pkgsrc.VariableType(varname)
+ vartype := G.Pkgsrc.VariableType(ck.MkLines, varname)
if vartype == nil {
return
}
@@ -399,11 +400,19 @@ func (ck MkLineChecker) CheckVaruse(varu
}
varname := varuse.varname
- vartype := G.Pkgsrc.VariableType(varname)
- ck.checkVaruseUndefined(vartype, varname)
+ vartype := G.Pkgsrc.VariableType(ck.MkLines, varname)
+ ck.checkVaruseUndefined(vartype, varname)
ck.checkVaruseModifiers(varuse, vartype)
+ ck.checkVarUseVarname(varuse)
+ ck.checkVarusePermissions(varname, vartype, vuc)
+ ck.checkVarUseQuoting(varuse, vartype, vuc)
+ ck.checkVarUseBuildDefs(varname)
+ ck.checkVaruseDeprecated(varuse)
+ ck.checkTextVarUse(varname, vartype, vuc.time)
+}
+func (ck MkLineChecker) checkVarUseVarname(varuse *MkVarUse) {
if varuse.varname == "@" {
ck.MkLine.Warnf("Please use %q instead of %q.", "${.TARGET}", "$@")
G.Explain(
@@ -411,59 +420,60 @@ func (ck MkLineChecker) CheckVaruse(varu
"of the same name.")
}
- ck.checkVarusePermissions(varname, vartype, vuc)
-
- if varname == "LOCALBASE" && !G.Infrastructure {
+ if varuse.varname == "LOCALBASE" && !G.Infrastructure {
fix := ck.MkLine.Autofix()
fix.Warnf("Please use PREFIX instead of LOCALBASE.")
fix.ReplaceRegex(`\$\{LOCALBASE\b`, "${PREFIX", 1)
fix.Apply()
}
+}
- needsQuoting := mkline.VariableNeedsQuoting(varname, vartype, vuc)
-
- if G.Opts.WarnQuoting && vuc.quoting != VucQuotUnknown && needsQuoting != unknown {
- // FIXME: Why "Shellword" when there's no indication that this is actually a shell type?
- // It's for splitting the value into tokens, taking "double" and 'single' quotes into account.
- ck.CheckVaruseShellword(varname, vartype, vuc, varuse.Mod(), needsQuoting == yes)
+func (ck MkLineChecker) checkVarUseBuildDefs(varname string) {
+ if !(G.Pkgsrc.UserDefinedVars.Defined(varname) && !G.Pkgsrc.IsBuildDef(varname)) {
+ return
}
- if G.Pkgsrc.UserDefinedVars.Defined(varname) && !G.Pkgsrc.IsBuildDef(varname) {
- if !G.Mk.buildDefs[varname] && G.Mk.FirstTimeSlice("BUILD_DEFS", varname) {
- mkline.Warnf("The user-defined variable %s is used but not added to BUILD_DEFS.", varname)
- G.Explain(
- "When a pkgsrc package is built, many things can be configured by the",
- "pkgsrc user in the mk.conf file.",
- "All these configurations should be recorded in the binary package",
- "so the package can be reliably rebuilt.",
- "The BUILD_DEFS variable contains a list of all these",
- "user-settable variables, so please add your variable to it, too.")
- }
+ if !(!ck.MkLines.buildDefs[varname] && ck.MkLines.FirstTimeSlice("BUILD_DEFS", varname)) {
+ return
}
- ck.checkVaruseDeprecated(varuse)
-
- ck.checkTextVarUse(varname, vartype, vuc.time)
+ ck.MkLine.Warnf("The user-defined variable %s is used but not added to BUILD_DEFS.", varname)
+ ck.MkLine.Explain(
+ "When a pkgsrc package is built, many things can be configured by the",
+ "pkgsrc user in the mk.conf file.",
+ "All these configurations should be recorded in the binary package",
+ "so the package can be reliably rebuilt.",
+ "The BUILD_DEFS variable contains a list of all these",
+ "user-settable variables, so please add your variable to it, too.")
}
func (ck MkLineChecker) checkVaruseUndefined(vartype *Vartype, varname string) {
switch {
+
case !G.Opts.WarnExtra:
- break
+ return
+
case vartype != nil && !vartype.guessed:
// Well-known variables are probably defined by the infrastructure.
- case varIsDefinedSimilar(G.Pkg, G.Mk, varname):
- break
+ return
+
+ case ck.MkLines != nil && (ck.MkLines.vars.DefinedSimilar(varname) || ck.MkLines.forVars[varname]):
+ return
+
+ case G.Pkg != nil && G.Pkg.vars.DefinedSimilar(varname):
+ return
+
case containsVarRef(varname):
- break
+ return
+
case G.Pkgsrc.vartypes.DefinedCanon(varname):
- break
- case G.Mk == nil || !G.Mk.FirstTimeSlice("used but not defined: ", varname):
- break
+ return
- default:
- ck.MkLine.Warnf("%s is used but not defined.", varname)
+ case ck.MkLines == nil || !ck.MkLines.FirstTimeSlice("used but not defined: ", varname):
+ return
}
+
+ ck.MkLine.Warnf("%s is used but not defined.", varname)
}
func (ck MkLineChecker) checkVaruseModifiers(varuse *MkVarUse, vartype *Vartype) {
@@ -572,8 +582,8 @@ func (ck MkLineChecker) checkVarusePermi
// whether bsd.prefs.mk has been included. That file examines the
// tools that have been added to USE_TOOLS up to this point and
// makes their variables available for use at load time.
- if tool := G.ToolByVarname(varname); tool != nil {
- if !tool.UsableAtLoadTime(G.Mk.Tools.SeenPrefs) {
+ if tool := G.ToolByVarname(ck.MkLines, varname); tool != nil {
+ if !tool.UsableAtLoadTime(ck.MkLines.Tools.SeenPrefs) {
ck.warnVaruseToolLoadTime(varname, tool)
}
return
@@ -588,7 +598,7 @@ func (ck MkLineChecker) checkVarusePermi
return
}
- if G.Mk != nil && !G.Mk.FirstTimeSlice("checkVarusePermissions", varname) {
+ if ck.MkLines != nil && !ck.MkLines.FirstTimeSlice("checkVarusePermissions", varname) {
return
}
@@ -715,13 +725,21 @@ func (ck MkLineChecker) warnVaruseToolLo
"except in the package Makefile itself.")
}
-// CheckVaruseShellword checks whether a variable use of the form ${VAR}
+// checkVarUseWords checks whether a variable use of the form ${VAR}
// or ${VAR:modifiers} is allowed in a certain context.
-func (ck MkLineChecker) CheckVaruseShellword(varname string, vartype *Vartype, vuc *VarUseContext, mod string, needsQuoting bool) {
- if trace.Tracing {
- defer trace.Call(varname, vartype, vuc, mod, needsQuoting)()
+func (ck MkLineChecker) checkVarUseQuoting(varUse *MkVarUse, vartype *Vartype, vuc *VarUseContext) {
+ if !G.Opts.WarnQuoting || vuc.quoting == VucQuotUnknown {
+ return
}
+ needsQuoting := ck.MkLine.VariableNeedsQuoting(ck.MkLines, varUse, vartype, vuc)
+ if needsQuoting == unknown {
+ return
+ }
+
+ varname := varUse.varname
+ mod := varUse.Mod()
+
// In GNU configure scripts, a few variables need to be passed through
// the :M* operator before they reach the configure scripts. Otherwise
// the leading or trailing spaces will lead to strange caching errors
@@ -735,7 +753,7 @@ func (ck MkLineChecker) CheckVaruseShell
if mod == ":M*:Q" && !needMstar {
mkline.Notef("The :M* modifier is not needed here.")
- } else if needsQuoting {
+ } else if needsQuoting == yes {
modNoQ := strings.TrimSuffix(mod, ":Q")
modNoM := strings.TrimSuffix(modNoQ, ":M*")
correctMod := modNoM + ifelseStr(needMstar, ":M*:Q", ":Q")
@@ -774,6 +792,7 @@ func (ck MkLineChecker) CheckVaruseShell
fix.Explain(
seeGuide("Echoing a string exactly as-is", "echo-literal"))
fix.Replace("${"+varname+mod+"}", "${"+varname+correctMod+"}")
+ fix.Anyway()
fix.Apply()
} else {
mkline.Warnf("Please use ${%s%s} instead of ${%s%s} and make sure"+
@@ -807,7 +826,7 @@ func (ck MkLineChecker) CheckVaruseShell
}
}
- if hasSuffix(mod, ":Q") && !needsQuoting {
+ if hasSuffix(mod, ":Q") && needsQuoting != yes {
bad := "${" + varname + mod + "}"
good := "${" + varname + strings.TrimSuffix(mod, ":Q") + "}"
@@ -866,6 +885,7 @@ func (ck MkLineChecker) checkVarassignDe
func (ck MkLineChecker) checkVarassign() {
ck.checkVarassignLeft()
+ ck.checkVarassignOp()
ck.checkVarassignRight()
}
@@ -887,6 +907,57 @@ func (ck MkLineChecker) checkVarassignLe
vucTimeParse)
}
+func (ck MkLineChecker) checkVarassignOp() {
+ ck.checkVarassignOpShell()
+}
+
+func (ck MkLineChecker) checkVarassignOpShell() {
+ mkline := ck.MkLine
+
+ switch {
+ case mkline.Op() != opAssignShell:
+ return
+
+ case mkline.VarassignComment() != "":
+ return
+
+ case mkline.Basename == "builtin.mk":
+ // These are typically USE_BUILTIN.* and BUILTIN_VERSION.*.
+ // Authors of builtin.mk files usually know what they're doing.
+ return
+
+ case G.Pkg == nil || G.Pkg.vars.UsedAtLoadTime(mkline.Varname()):
+ return
+ }
+
+ mkline.Notef("Consider the :sh modifier instead of != for %q.", mkline.Value())
+ mkline.Explain(
+ "For variable assignments using the != operator, the shell command",
+ "is run every time the file is parsed.",
+ "In some cases this is too early, and the command may not yet be installed.",
+ "In other cases the command is executed more often than necessary.",
+ "Most commands don't need to be executed for \"make clean\", for example.",
+ "",
+ "The :sh modifier defers execution until the variable value is actually needed.",
+ "On the other hand, this means the command is executed each time the variable",
+ "is evaluated.",
+ "",
+ "Example:",
+ "",
+ "\tEARLY_YEAR!= date +%Y",
+ "",
+ "\tLATE_YEAR_CMD= date +%Y",
+ "\tLATE_YEAR= ${LATE_YEAR_CMD:sh}",
+ "",
+ "\t# or, in a single line:",
+ "\tLATE_YEAR= ${date +%Y:L:sh}",
+ "",
+ "To suppress this note, provide an explanation in a comment at the end",
+ "of the line, or force the variable to be evaluated at load time,",
+ "by using it at the right-hand side of the := operator, or in an .if",
+ "or .for directive.")
+}
+
// checkVarassignLeft checks everything to the right of the assignment operator.
func (ck MkLineChecker) checkVarassignRight() {
mkline := ck.MkLine
@@ -930,7 +1001,11 @@ func (ck MkLineChecker) checkVarassignLe
return
}
- if varIsUsedSimilar(G.Pkg, G.Mk, varname) {
+ if ck.MkLines != nil && ck.MkLines.vars.UsedSimilar(varname) {
+ return
+ }
+
+ if G.Pkg != nil && G.Pkg.vars.UsedSimilar(varname) {
return
}
@@ -944,7 +1019,7 @@ func (ck MkLineChecker) checkVarassignLe
return
}
- if G.Mk == nil || !G.Mk.FirstTimeSlice("defined but not used: ", varname) {
+ if ck.MkLines == nil || !ck.MkLines.FirstTimeSlice("defined but not used: ", varname) {
return
}
@@ -970,7 +1045,7 @@ func (ck MkLineChecker) checkVarassignRi
time = vucTimeParse
}
- vartype := G.Pkgsrc.VariableType(mkline.Varname())
+ vartype := G.Pkgsrc.VariableType(ck.MkLines, mkline.Varname())
if op == opAssignShell {
vartype = shellCommandsType
}
@@ -1069,7 +1144,7 @@ func (ck MkLineChecker) checkVarassignMi
// No autofix since it doesn't occur anymore.
}
- if varname == "PKG_SKIP_REASON" && G.Mk.indentation.DependsOn("OPSYS") {
+ if varname == "PKG_SKIP_REASON" && ck.MkLines.indentation.DependsOn("OPSYS") {
// TODO: Provide autofix for simple cases, like ".if ${OPSYS} == SunOS".
mkline.Notef("Consider setting NOT_FOR_PLATFORM instead of " +
"PKG_SKIP_REASON depending on ${OPSYS}.")
@@ -1091,8 +1166,8 @@ func (ck MkLineChecker) checkVarassignLe
if !G.Opts.WarnExtra ||
G.Infrastructure ||
mkline.Op() != opAssignDefault ||
- G.Mk.Tools.SeenPrefs ||
- !G.Mk.FirstTime("include bsd.prefs.mk before using ?=") {
+ ck.MkLines.Tools.SeenPrefs ||
+ !ck.MkLines.FirstTime("include bsd.prefs.mk before using ?=") {
return
}
@@ -1116,7 +1191,7 @@ func (ck MkLineChecker) checkVartype(var
}
mkline := ck.MkLine
- vartype := G.Pkgsrc.VariableType(varname)
+ vartype := G.Pkgsrc.VariableType(ck.MkLines, varname)
if op == opAssignAppend {
// XXX: MayBeAppendedTo also depends on the current file, see checkVarusePermissions.
@@ -1144,7 +1219,7 @@ func (ck MkLineChecker) checkVartype(var
break
default:
- words, _ := splitIntoMkWords(mkline.Line, value)
+ words := mkline.ValueFields(value)
for _, word := range words {
ck.CheckVartypeBasic(varname, vartype.basicType, op, word, comment, vartype.guessed)
}
@@ -1162,7 +1237,7 @@ func (ck MkLineChecker) CheckVartypeBasi
mkline := ck.MkLine
valueNoVar := mkline.WithoutMakeVariables(value)
- ctx := VartypeCheck{mkline, varname, op, value, valueNoVar, comment, guessed}
+ ctx := VartypeCheck{ck.MkLines, mkline, varname, op, value, valueNoVar, comment, guessed}
checker.checker(&ctx)
}
@@ -1286,7 +1361,7 @@ func (ck MkLineChecker) checkDirectiveCo
if m, positive, pattern := modifier.MatchMatch(); m && (positive || len(modifiers) == 1) {
ck.checkVartype(varname, opUseMatch, pattern, "")
- vartype := G.Pkgsrc.VariableType(varname)
+ vartype := G.Pkgsrc.VariableType(ck.MkLines, varname)
if matches(pattern, `^[\w-/]+$`) && vartype != nil && !vartype.IsConsideredList() {
ck.MkLine.Notef("%s should be compared using %s instead of matching against %q.",
varname, ifelseStr(positive, "==", "!="), ":"+modifier.Text)
@@ -1373,7 +1448,7 @@ func (ck MkLineChecker) CheckRelativePat
abs := path.Dir(mkline.Filename) + "/" + resolvedPath
if _, err := os.Stat(abs); err != nil {
- if mustExist && !(G.Mk != nil && G.Mk.indentation.IsCheckedFile(resolvedPath)) {
+ if mustExist && !(ck.MkLines != nil && ck.MkLines.indentation.IsCheckedFile(resolvedPath)) {
mkline.Errorf("Relative path %q does not exist.", resolvedPath)
}
return
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.28 pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.29
--- pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.28 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go Wed Apr 3 21:49:51 2019
@@ -11,8 +11,9 @@ func (s *Suite) Test_MkLineChecker_check
mklines := t.NewMkLines("module.mk",
MkRcsID,
"_VARNAME=\tvalue")
+ // Only to prevent "defined but not used".
+ mklines.vars.Use("_VARNAME", mklines.mklines[1], vucTimeRun)
- mklines.vars.Use("_VARNAME", mklines.mklines[1])
mklines.Check()
t.CheckOutputLines(
@@ -305,6 +306,39 @@ func (s *Suite) Test_MkLineChecker_check
func (s *Suite) Test_MkLineChecker_checkDirectiveFor(c *check.C) {
t := s.Init(c)
+ t.SetUpVartypes()
+ mklines := t.NewMkLines("for.mk",
+ MkRcsID,
+ ".for dir in ${PATH:C,:, ,g}",
+ ".endfor",
+ "",
+ ".for dir in ${PATH}",
+ ".endfor",
+ "",
+ ".for dir in ${PATH:M*/bin}",
+ ".endfor")
+
+ mklines.Check()
+
+ t.CheckOutputLines(
+ // FIXME: PATH may actually be used at load time.
+ "WARN: for.mk:2: PATH should not be used at load time in any file.",
+
+ // No warning about :Q in line 2 since the :C modifier converts the
+ // colon-separated list into a space-separated list, as required by
+ // the .for loop.
+
+ // This warning is correct since PATH is separated by colons, not by spaces.
+ "WARN: for.mk:5: Please use ${PATH:Q} instead of ${PATH}.",
+
+ // This warning is also correct since the :M modifier doesn't change the
+ // word boundaries.
+ "WARN: for.mk:8: Please use ${PATH:M*/bin:Q} instead of ${PATH:M*/bin}.")
+}
+
+func (s *Suite) Test_MkLineChecker_checkDirectiveFor__infrastructure(c *check.C) {
+ t := s.Init(c)
+
t.SetUpPkgsrc()
t.CreateFileLines("mk/file.mk",
MkRcsID,
@@ -316,7 +350,8 @@ func (s *Suite) Test_MkLineChecker_check
G.Check(t.File("mk/file.mk"))
- // Pkglint doesn't care about trivial syntax errors, bmake will already catch these.
+ // Pkglint doesn't care about trivial syntax errors like the "=" instead
+ // of "in" above; bmake will already catch these.
t.CheckOutputEmpty()
}
@@ -347,7 +382,7 @@ func (s *Suite) Test_MkLineChecker_check
t.SetUpVartypes()
// Since COMMENT is defined in vardefs.go its type is certain instead of guessed.
- vartype := G.Pkgsrc.VariableType("COMMENT")
+ vartype := G.Pkgsrc.VariableType(nil, "COMMENT")
c.Assert(vartype, check.NotNil)
c.Check(vartype.basicType.name, equals, "Comment")
@@ -419,9 +454,8 @@ func (s *Suite) Test_MkLineChecker_check
test := func(cond string, output ...string) {
mklines := t.NewMkLines("filename.mk",
cond)
- G.Mk = mklines
mklines.ForEach(func(mkline MkLine) {
- MkLineChecker{mkline}.checkDirectiveCond()
+ MkLineChecker{mklines, mkline}.checkDirectiveCond()
})
t.CheckOutput(output)
}
@@ -499,8 +533,8 @@ func (s *Suite) Test_MkLineChecker_check
"TRACE: 1 2 + MkLineChecker.checkVarusePermissions(\"VAR\", (no-type time:parse quoting:plain wordpart:false))",
"TRACE: 1 2 3 No type definition found for \"VAR\".",
"TRACE: 1 2 - MkLineChecker.checkVarusePermissions(\"VAR\", (no-type time:parse quoting:plain wordpart:false))",
- "TRACE: 1 2 + (*MkLineImpl).VariableNeedsQuoting(\"VAR\", (*pkglint.Vartype)(nil), (no-type time:parse quoting:plain wordpart:false))",
- "TRACE: 1 2 - (*MkLineImpl).VariableNeedsQuoting(\"VAR\", (*pkglint.Vartype)(nil), (no-type time:parse quoting:plain wordpart:false), \"=>\", unknown)",
+ "TRACE: 1 2 + (*MkLineImpl).VariableNeedsQuoting(${VAR:Mpattern1:Mpattern2}, (*pkglint.Vartype)(nil), (no-type time:parse quoting:plain wordpart:false))",
+ "TRACE: 1 2 - (*MkLineImpl).VariableNeedsQuoting(${VAR:Mpattern1:Mpattern2}, (*pkglint.Vartype)(nil), (no-type time:parse quoting:plain wordpart:false), \"=>\", unknown)",
"TRACE: 1 - MkLineChecker.CheckVaruse(filename.mk:1, ${VAR:Mpattern1:Mpattern2}, (no-type time:parse quoting:plain wordpart:false))",
"TRACE: - MkLineChecker.checkDirectiveCond(\"${VAR:Mpattern1:Mpattern2} == comparison\")",
"TRACE: + (*MkParser).mkCondAtom(\"${VAR:Mpattern1:Mpattern2} == comparison\")",
@@ -620,6 +654,77 @@ func (s *Suite) Test_MkLineChecker_check
t.CheckOutputEmpty()
}
+func (s *Suite) Test_MkLineChecker_checkVarassignOpShell(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpTool("uname", "UNAME", AfterPrefsMk)
+ t.SetUpTool("echo", "", AtRunTime)
+ t.SetUpPkgsrc()
+ t.SetUpPackage("category/package",
+ ".include \"standalone.mk\"")
+ t.CreateFileLines("category/package/standalone.mk",
+ MkRcsID,
+ "",
+ ".include \"../../mk/bsd.prefs.mk\"",
+ "",
+ "OPSYS_NAME!=\t${UNAME}",
+ ".if ${OPSYS_NAME} == \"NetBSD\"",
+ ".endif",
+ "",
+ "OS_NAME!=\t${UNAME}",
+ "",
+ "MUST_BE_EARLY!=\techo 123 # must be evaluated early",
+ "",
+ "show-package-vars: .PHONY",
+ "\techo OS_NAME=${OS_NAME:Q}",
+ "\techo MUST_BE_EARLY=${MUST_BE_EARLY:Q}")
+
+ G.Check(t.File("category/package/standalone.mk"))
+
+ // There is no warning about any variable since no package is currently
+ // being checked, therefore pkglint cannot decide whether the variable
+ // is used a load time.
+ t.CheckOutputEmpty()
+
+ t.SetUpCommandLine("-Wall", "--explain")
+ G.Check(t.File("category/package"))
+
+ // There is no warning for OPSYS_NAME since that variable is used at
+ // load time. In such a case the command has to be executed anyway,
+ // and executing it exactly once is the best thing to do.
+ //
+ // There is no warning for MUST_BE_EARLY since the comment provides the
+ // reason that this command really has to be executed at load time.
+ t.CheckOutputLines(
+ "NOTE: ~/category/package/standalone.mk:9: Consider the :sh modifier instead of != for \"${UNAME}\".",
+ "",
+ "\tFor variable assignments using the != operator, the shell command is",
+ "\trun every time the file is parsed. In some cases this is too early,",
+ "\tand the command may not yet be installed. In other cases the command",
+ "\tis executed more often than necessary. Most commands don't need to",
+ "\tbe executed for \"make clean\", for example.",
+ "",
+ "\tThe :sh modifier defers execution until the variable value is",
+ "\tactually needed. On the other hand, this means the command is",
+ "\texecuted each time the variable is evaluated.",
+ "",
+ "\tExample:",
+ "",
+ "\t\tEARLY_YEAR!= date +%Y",
+ "",
+ "\t\tLATE_YEAR_CMD= date +%Y",
+ "\t\tLATE_YEAR= ${LATE_YEAR_CMD:sh}",
+ "",
+ "\t\t# or, in a single line:",
+ "\t\tLATE_YEAR= ${date +%Y:L:sh}",
+ "",
+ "\tTo suppress this note, provide an explanation in a comment at the",
+ "\tend of the line, or force the variable to be evaluated at load time,",
+ "\tby using it at the right-hand side of the := operator, or in an .if",
+ "\tor .for directive.",
+ "")
+}
+
func (s *Suite) Test_MkLineChecker_checkVarassignRightVaruse(c *check.C) {
t := s.Init(c)
@@ -1147,7 +1252,7 @@ func (s *Suite) Test_MkLineChecker_Check
"# dummy")
mklines.ForEach(func(mkline MkLine) {
- ck := MkLineChecker{mkline}
+ ck := MkLineChecker{mklines, mkline}
ck.CheckRelativePkgdir("../pkgbase")
ck.CheckRelativePkgdir("../../other/package")
@@ -1189,8 +1294,8 @@ func (s *Suite) Test_MkLineChecker_Check
"FILES_SUBST+=XKBCOMP_SYMLINK=${${XKBBASE}/xkbcomp:L:Q}",
"FILES_SUBST+=XKBCOMP_SYMLINK=${${XKBBASE}/xkbcomp:Q}")
- MkLineChecker{mklines.mklines[0]}.Check()
- MkLineChecker{mklines.mklines[1]}.Check()
+ MkLineChecker{mklines, mklines.mklines[0]}.Check()
+ MkLineChecker{mklines, mklines.mklines[1]}.Check()
// In line 1, don't warn that ${XKBBASE}/xkbcomp is used but not defined.
// This is because the :L modifier interprets everything before as an expression
@@ -1255,7 +1360,7 @@ func (s *Suite) Test_MkLineChecker_check
t.SetUpVartypes()
mkline := t.NewMkLine("module.mk", 123, ".if ${PKGPATH} == \"category/package\"")
- ck := MkLineChecker{mkline}
+ ck := MkLineChecker{nil, mkline}
// FIXME: checkDirectiveCondEmpty cannot know whether it is empty(...) or !empty(...).
// It must know that to generate the proper diagnostics.
@@ -1305,17 +1410,16 @@ func (s *Suite) Test_MkLineChecker_check
t := s.Init(c)
t.SetUpVartypes()
- G.Mk = t.NewMkLines("chat/pidgin-icb/Makefile",
+ mklines := t.NewMkLines("chat/pidgin-icb/Makefile",
MkRcsID,
"CFLAGS+=\t`pkg-config pidgin --cflags`")
- mkline := G.Mk.mklines[1]
+ mkline := mklines.mklines[1]
- words, rest := splitIntoMkWords(mkline.Line, mkline.Value())
+ words := mkline.Fields()
c.Check(words, deepEquals, []string{"`pkg-config pidgin --cflags`"})
- c.Check(rest, equals, "")
- ck := MkLineChecker{G.Mk.mklines[1]}
+ ck := MkLineChecker{mklines, mklines.mklines[1]}
ck.checkVartype("CFLAGS", opAssignAppend, "`pkg-config pidgin --cflags`", "")
// No warning about "`pkg-config" being an unknown CFlag.
@@ -1346,7 +1450,7 @@ func (s *Suite) Test_MkLineChecker_check
mkline := t.NewMkLine("filename.mk", 123, ".if 0")
// Calling this method is only useful in the context of a whole file.
- MkLineChecker{mkline}.checkDirectiveIndentation(4)
+ MkLineChecker{nil, mkline}.checkDirectiveIndentation(4)
t.CheckOutputEmpty()
}
@@ -1412,7 +1516,7 @@ func (s *Suite) Test_MkLineChecker_check
".endif")
}
-func (s *Suite) Test_MkLineChecker_CheckVaruseShellword(c *check.C) {
+func (s *Suite) Test_MkLineChecker_checkVarUseQuoting(c *check.C) {
t := s.Init(c)
t.SetUpVartypes()
@@ -1435,7 +1539,7 @@ func (s *Suite) Test_MkLineChecker_Check
"WARN: ~/options.mk:4: The variable PATH should be quoted as part of a shell word.")
}
-func (s *Suite) Test_MkLineChecker_CheckVaruseShellword__mstar(c *check.C) {
+func (s *Suite) Test_MkLineChecker_checkVarUseQuoting__mstar(c *check.C) {
t := s.Init(c)
t.SetUpCommandLine("-Wall,no-space")
@@ -1459,7 +1563,7 @@ func (s *Suite) Test_MkLineChecker_Check
"WARN: ~/options.mk:4: ADA_FLAGS is used but not defined.")
}
-func (s *Suite) Test_MkLineChecker_CheckVaruseShellword__mstar_not_needed(c *check.C) {
+func (s *Suite) Test_MkLineChecker_checkVarUseQuoting__mstar_not_needed(c *check.C) {
t := s.Init(c)
t.SetUpCommandLine("-Wall,no-space")
@@ -1481,7 +1585,7 @@ func (s *Suite) Test_MkLineChecker_Check
"NOTE: ~/category/package/Makefile:21: The :M* modifier is not needed here.")
}
-func (s *Suite) Test_MkLineChecker_CheckVaruseShellword__q_not_needed(c *check.C) {
+func (s *Suite) Test_MkLineChecker_checkVarUseQuoting__q_not_needed(c *check.C) {
t := s.Init(c)
pkg := t.SetUpPackage("category/package",
@@ -1543,7 +1647,7 @@ func (s *Suite) Test_MkLineChecker_Check
"CPPPATH.Linux=\t/usr/bin/cpp")
G.Pkgsrc.LoadInfrastructure()
- ck := MkLineChecker{t.NewMkLine("module.mk", 101, "COMMENT=\t${CPPPATH.SunOS}")}
+ ck := MkLineChecker{nil, t.NewMkLine("module.mk", 101, "COMMENT=\t${CPPPATH.SunOS}")}
ck.CheckVaruse(NewMkVarUse("CPPPATH.SunOS"), &VarUseContext{
vartype: &Vartype{
@@ -1678,7 +1782,7 @@ func (s *Suite) Test_MkLineChecker_check
mkline := t.NewMkLine("mk/compiler/gcc.mk", 150,
"CC:=\t${CC:C/^/_asdf_/1:M_asdf_*:S/^_asdf_//}")
- MkLineChecker{mkline}.Check()
+ MkLineChecker{nil, mkline}.Check()
// FIXME: The check is called two times, even though it only produces a single NOTE.
t.CheckOutputLines(
@@ -1717,7 +1821,7 @@ func (s *Suite) Test_MkLineChecker_Check
mkline := t.NewMkLine("module.mk", 123,
"\t${_PKG_SILENT}${_PKG_DEBUG} :")
- MkLineChecker{mkline}.Check()
+ MkLineChecker{nil, mkline}.Check()
t.CheckOutputLines(
"WARN: module.mk:123: Use of _PKG_SILENT and _PKG_DEBUG is deprecated. Use ${RUN} instead.")
Index: pkgsrc/pkgtools/pkglint/files/mklines.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines.go:1.44 pkgsrc/pkgtools/pkglint/files/mklines.go:1.45
--- pkgsrc/pkgtools/pkglint/files/mklines.go:1.44 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/mklines.go Wed Apr 3 21:49:51 2019
@@ -74,10 +74,10 @@ func NewMkLines(lines Lines) MkLines {
// UseVar remembers that the given variable is used in the given line.
// This controls the "defined but not used" warning.
-func (mklines *MkLinesImpl) UseVar(mkline MkLine, varname string) {
- mklines.vars.Use(varname, mkline)
+func (mklines *MkLinesImpl) UseVar(mkline MkLine, varname string, time vucTime) {
+ mklines.vars.Use(varname, mkline, time)
if G.Pkg != nil {
- G.Pkg.vars.Use(varname, mkline)
+ G.Pkg.vars.Use(varname, mkline, time)
}
}
@@ -86,9 +86,6 @@ func (mklines *MkLinesImpl) Check() {
defer trace.Call1(mklines.lines.FileName)()
}
- G.Mk = mklines
- defer func() { G.Mk = nil }()
-
// In the first pass, all additions to BUILD_DEFS and USE_TOOLS
// are collected to make the order of the definitions irrelevant.
mklines.collectUsedVariables()
@@ -128,7 +125,7 @@ func (mklines *MkLinesImpl) checkAll() {
mklines.Tools.SeenPrefs = true
}
- ck := MkLineChecker{mkline}
+ ck := MkLineChecker{mklines, mkline}
ck.Check()
varalign.Process(mkline)
@@ -184,7 +181,7 @@ func (mklines *MkLinesImpl) checkAll() {
func (mklines *MkLinesImpl) checkVarassignPlist(mkline MkLine) {
switch mkline.Varcanon() {
case "PLIST_VARS":
- for _, id := range mkline.ValueFields(resolveVariableRefs(mkline.Value())) {
+ for _, id := range mkline.ValueFields(resolveVariableRefs(mklines, mkline.Value())) {
if !mklines.plistVarSkip && mklines.plistVarSet[id] == nil {
mkline.Warnf("%q is added to PLIST_VARS, but PLIST.%s is not defined in this file.", id, id)
}
@@ -244,7 +241,7 @@ func (mklines *MkLinesImpl) collectDefin
continue
}
- defineVar(G.Pkg, mklines, mkline, mkline.Varname())
+ mklines.defineVar(G.Pkg, mkline, mkline.Varname())
varcanon := mkline.Varcanon()
switch varcanon {
@@ -267,22 +264,22 @@ func (mklines *MkLinesImpl) collectDefin
}
case "PLIST_VARS":
- for _, id := range mkline.ValueFields(resolveVariableRefs(mkline.Value())) {
+ for _, id := range mkline.ValueFields(resolveVariableRefs(mklines, mkline.Value())) {
if trace.Tracing {
trace.Step1("PLIST.%s is added to PLIST_VARS.", id)
}
if containsVarRef(id) {
- mklines.UseVar(mkline, "PLIST.*")
+ mklines.UseVar(mkline, "PLIST.*", mkline.Op().Time())
mklines.plistVarSkip = true
} else {
- mklines.UseVar(mkline, "PLIST."+id)
+ mklines.UseVar(mkline, "PLIST."+id, mkline.Op().Time())
}
}
case "SUBST_VARS.*":
for _, substVar := range mkline.Fields() {
- mklines.UseVar(mkline, varnameCanon(substVar))
+ mklines.UseVar(mkline, varnameCanon(substVar), mkline.Op().Time())
if trace.Tracing {
trace.Step1("varuse %s", substVar)
}
@@ -290,20 +287,28 @@ func (mklines *MkLinesImpl) collectDefin
case "OPSYSVARS":
for _, opsysVar := range mkline.Fields() {
- mklines.UseVar(mkline, opsysVar+".*")
- defineVar(G.Pkg, mklines, mkline, opsysVar)
+ mklines.UseVar(mkline, opsysVar+".*", mkline.Op().Time())
+ mklines.defineVar(G.Pkg, mkline, opsysVar)
}
}
}
}
+// defineVar marks a variable as defined in both the current package and the current file.
+func (mklines *MkLinesImpl) defineVar(pkg *Package, mkline MkLine, varname string) {
+ mklines.vars.Define(varname, mkline)
+ if pkg != nil {
+ pkg.vars.Define(varname, mkline)
+ }
+}
+
func (mklines *MkLinesImpl) collectPlistVars() {
// TODO: The PLIST_VARS code above looks very similar.
for _, mkline := range mklines.mklines {
if mkline.IsVarassign() {
switch mkline.Varcanon() {
case "PLIST_VARS":
- for _, id := range mkline.ValueFields(resolveVariableRefs(mkline.Value())) {
+ for _, id := range mkline.ValueFields(resolveVariableRefs(mklines, mkline.Value())) {
if containsVarRef(id) {
mklines.plistVarSkip = true
} else {
@@ -330,9 +335,9 @@ func (mklines *MkLinesImpl) collectElse(
func (mklines *MkLinesImpl) collectUsedVariables() {
for _, mkline := range mklines.mklines {
- for _, varname := range mkline.DetermineUsedVariables() {
- mklines.UseVar(mkline, varname)
- }
+ mkline.ForEachUsed(func(varUse *MkVarUse, time vucTime) {
+ mklines.UseVar(mkline, varUse.varname, time)
+ })
}
mklines.collectDocumentedVariables()
@@ -357,7 +362,7 @@ func (mklines *MkLinesImpl) collectDocum
if commentLines >= 3 && relevant {
for varname, mkline := range scope.used {
mklines.vars.Define(varname, mkline)
- mklines.vars.Use(varname, mkline)
+ mklines.vars.Use(varname, mkline, vucTimeRun)
}
}
@@ -393,7 +398,7 @@ func (mklines *MkLinesImpl) collectDocum
varcanon := varnameCanon(varname)
if varcanon == strings.ToUpper(varcanon) && matches(varcanon, `[A-Z]`) && parser.EOF() {
scope.Define(varcanon, mkline)
- scope.Use(varcanon, mkline)
+ scope.Use(varcanon, mkline, vucTimeRun)
}
if words[1] == "Copyright" {
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.44 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.45
--- pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.44 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go Wed Apr 3 21:49:51 2019
@@ -172,55 +172,114 @@ func (s *Suite) Test_VartypeCheck_ConfFi
"WARN: filename.mk:5: The destination file \"/etc/bootrc\" should start with a variable reference.")
}
+// See Test_MkParser_Dependency.
func (s *Suite) Test_VartypeCheck_Dependency(c *check.C) {
vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).Dependency)
vt.Varname("CONFLICTS")
vt.Op(opAssignAppend)
+
+ // comparison operators
vt.Values(
- "Perl",
"perl5>=5.22",
+ "libkipi>=0.1.5<4.0",
+ "gtk2+>=2.16")
+ vt.OutputEmpty()
+
+ // pattern matching
+ vt.Values(
+ "perl-5*",
"perl5-*",
+ "perl-5.22",
"perl5-5.22.*",
- "perl5-[5.10-5.22]*",
- "py-docs",
+ "gtksourceview-sharp-2.0-[0-9]*")
+ vt.Output(
+ "WARN: filename.mk:11: Please use \"5.*\" instead of \"5*\" as the version pattern.",
+ "WARN: filename.mk:12: Please use \"perl5-[0-9]*\" instead of \"perl5-*\".",
+ "WARN: filename.mk:13: Please use \"5.22{,nb*}\" instead of \"5.22\" as the version pattern.",
+ "WARN: filename.mk:15: The version pattern \"2.0-[0-9]*\" should not contain a hyphen.")
+
+ // nb suffix
+ vt.Values(
"perl5-5.22.*{,nb*}",
- "libkipi>=0.1.5<4.0",
- "gtk2+>=2.16",
- "perl-5.22",
- "perl-5*",
- "gtksourceview-sharp-2.0-[0-9]*",
"perl-5.22{,nb*}",
"perl-5.22{,nb[0-9]*}",
"mbrola-301h{,nb[0-9]*}",
+ "ncurses-${NC_VERS}{,nb*}",
+ "gnome-control-center>=2.20.1{,nb*}",
+ "gnome-control-center>=2.20.1{,nb[0-9]*}")
+ vt.Output(
+ "WARN: filename.mk:26: Dependency patterns of the form pkgbase>=1.0 don't need the \"{,nb*}\" extension.",
+ "WARN: filename.mk:27: Dependency patterns of the form pkgbase>=1.0 don't need the \"{,nb*}\" extension.")
+
+ // alternative patterns, using braces or brackets
+ vt.Values(
"mpg123{,-esound,-nas}>=0.59.18",
"mysql*-{client,server}-[0-9]*",
- "postgresql8[0-35-9]-${module}-[0-9]*",
- "ncurses-${NC_VERS}{,nb*}",
"{ssh{,6}-[0-9]*,openssh-[0-9]*}",
- "gnome-control-center>=2.20.1{,nb*}",
- "gnome-control-center>=2.20.1{,nb[0-9]*}",
- "package-1.0|garbage",
+ "libao-[a-z]*-[0-9]*")
+ vt.OutputEmpty()
+
+ // variables
+ vt.Values(
+ "postgresql8[0-35-9]-${module}-[0-9]*",
"${_EMACS_CONFLICTS.${_EMACS_FLAVOR}}",
- "package>=1.0:../../category/package",
- "package-1.0>=1.0.3")
+ "${PYPKGPREFIX}-sqlite3",
+ "${PYPKGPREFIX}-sqlite3-${VERSION}",
+ "${PYPKGPREFIX}-sqlite3-${PYSQLITE_REQD}",
+ "${PYPKGPREFIX}-sqlite3>=${PYSQLITE_REQD}",
+ "${EMACS_PACKAGE}>=${EMACS_MAJOR}",
+
+ // The "*" is ambiguous. It could either continue the PKGBASE or
+ // start the version number.
+ "${PKGNAME_NOREV:S/jdk/jre/}*",
+
+ // The canonical form is "{,nb*}" instead of "{nb*,}".
+ // Plus, mentioning nb* is not necessary when using >=.
+ "dovecot>=${PKGVERSION_NOREV}{nb*,}",
+
+ "oxygen-icons>=${KF5VER}{,nb[0-9]*}",
+
+ // The following pattern should have "]*}" instead of "]}*".
+ "ja-vflib-lib-${VFLIB_VERSION}{,nb[0-9]}*",
+
+ // The following pattern uses both ">=" and "*", which doesn't make sense.
+ "${PYPKGPREFIX}-sphinx>=1.2.3nb1*",
+
+ "{${NETSCAPE_PREFERRED:C/:/,/g}}-[0-9]*")
vt.Output(
- "WARN: filename.mk:1: Invalid dependency pattern \"Perl\".",
- "WARN: filename.mk:3: Please use \"perl5-[0-9]*\" instead of \"perl5-*\".",
- "WARN: filename.mk:5: Only [0-9]* is allowed in the numeric part of a dependency.",
- "WARN: filename.mk:5: The version pattern \"[5.10-5.22]*\" should not contain a hyphen.",
- "WARN: filename.mk:6: Invalid dependency pattern \"py-docs\".",
- "WARN: filename.mk:10: Please use \"5.22{,nb*}\" instead of \"5.22\" as the version pattern.",
- "WARN: filename.mk:11: Please use \"5.*\" instead of \"5*\" as the version pattern.",
- "WARN: filename.mk:12: The version pattern \"2.0-[0-9]*\" should not contain a hyphen.",
- "WARN: filename.mk:21: Dependency patterns of the form pkgbase>=1.0 don't need the \"{,nb*}\" extension.",
- "WARN: filename.mk:22: Dependency patterns of the form pkgbase>=1.0 don't need the \"{,nb*}\" extension.",
- "WARN: filename.mk:23: Invalid dependency pattern \"package-1.0|garbage\".",
+ "WARN: filename.mk:43: Invalid dependency pattern \"${PYPKGPREFIX}-sqlite3\".",
+ // This pattern is invalid because the variable name doesn't contain "VER".
+ "WARN: filename.mk:45: Invalid dependency pattern \"${PYPKGPREFIX}-sqlite3-${PYSQLITE_REQD}\".",
+ "WARN: filename.mk:48: Invalid dependency pattern \"${PKGNAME_NOREV:S/jdk/jre/}*\".",
+ "WARN: filename.mk:49: Invalid dependency pattern \"dovecot>=${PKGVERSION_NOREV}{nb*,}\".",
+ "WARN: filename.mk:50: Dependency patterns of the form pkgbase>=1.0 don't need the \"{,nb*}\" extension.",
+ "WARN: filename.mk:51: Invalid dependency pattern \"ja-vflib-lib-${VFLIB_VERSION}{,nb[0-9]}*\".",
+ "WARN: filename.mk:52: Invalid dependency pattern \"${PYPKGPREFIX}-sphinx>=1.2.3nb1*\".")
+
+ // invalid dependency patterns
+ vt.Values(
+ "Perl",
+ "py-docs",
+ "perl5-[5.10-5.22]*",
+ "package-1.0|garbage",
+ "package>=1.0:../../category/package",
+ "package-1.0>=1.0.3",
+ // This should be regarded as invalid since the [a-z0-9] might either
+ // continue the PKGBASE or start the version number.
+ "${RUBY_PKGPREFIX}-theme-[a-z0-9]*")
+ vt.Output(
+ "WARN: filename.mk:61: Invalid dependency pattern \"Perl\".",
+ "WARN: filename.mk:62: Invalid dependency pattern \"py-docs\".",
+ "WARN: filename.mk:63: Only [0-9]* is allowed in the numeric part of a dependency.",
+ "WARN: filename.mk:63: The version pattern \"[5.10-5.22]*\" should not contain a hyphen.",
+ "WARN: filename.mk:64: Invalid dependency pattern \"package-1.0|garbage\".",
// TODO: Mention that the path should be removed.
- "WARN: filename.mk:25: Invalid dependency pattern \"package>=1.0:../../category/package\".",
+ "WARN: filename.mk:65: Invalid dependency pattern \"package>=1.0:../../category/package\".",
// TODO: Mention that version numbers in a pkgbase must be appended directly, without hyphen.
- "WARN: filename.mk:26: Invalid dependency pattern \"package-1.0>=1.0.3\".")
+ "WARN: filename.mk:66: Invalid dependency pattern \"package-1.0>=1.0.3\".",
+ "WARN: filename.mk:67: Invalid dependency pattern \"${RUBY_PKGPREFIX}-theme-[a-z0-9]*\".")
}
func (s *Suite) Test_VartypeCheck_DependencyWithPath(c *check.C) {
@@ -228,6 +287,7 @@ func (s *Suite) Test_VartypeCheck_Depend
vt := NewVartypeCheckTester(t, (*VartypeCheck).DependencyWithPath)
t.CreateFileLines("category/package/Makefile")
+ t.CreateFileLines("databases/py-sqlite3/Makefile")
t.CreateFileLines("devel/gettext/Makefile")
t.CreateFileLines("devel/gmake/Makefile")
t.CreateFileLines("devel/py-module/Makefile")
@@ -241,35 +301,56 @@ func (s *Suite) Test_VartypeCheck_Depend
"Perl",
"perl5>=5.22:../perl5",
"perl5>=5.24:../../lang/perl5",
- "broken0.12.1:../../x11/alacarte",
- "broken[0-9]*:../../x11/alacarte",
- "broken[0-9]*../../x11/alacarte",
- "broken>=:../../x11/alacarte",
- "broken=0:../../x11/alacarte",
- "broken=:../../x11/alacarte",
- "broken-:../../x11/alacarte",
- "broken>:../../x11/alacarte",
"gtk2+>=2.16:../../x11/alacarte",
"gettext-[0-9]*:../../devel/gettext",
- "gmake-[0-9]*:../../devel/gmake",
- "${PYPKGPREFIX}-module>=0:../../devel/py-module")
+ "gmake-[0-9]*:../../devel/gmake")
vt.Output(
"WARN: ~/category/package/filename.mk:1: Invalid dependency pattern with path \"Perl\".",
- "WARN: ~/category/package/filename.mk:2: Dependencies should have the form \"../../category/package\".",
+ "WARN: ~/category/package/filename.mk:2: Dependency paths should have the form \"../../category/package\".",
+ "ERROR: ~/category/package/filename.mk:2: Relative path \"../perl5\" does not exist.",
+ "WARN: ~/category/package/filename.mk:2: \"../perl5\" is not a valid relative package directory.",
+ "WARN: ~/category/package/filename.mk:2: Please use USE_TOOLS+=perl:run instead of this dependency.",
"ERROR: ~/category/package/filename.mk:3: Relative path \"../../lang/perl5\" does not exist.",
"ERROR: ~/category/package/filename.mk:3: There is no package in \"lang/perl5\".",
"WARN: ~/category/package/filename.mk:3: Please use USE_TOOLS+=perl:run instead of this dependency.",
- "WARN: ~/category/package/filename.mk:4: Invalid dependency pattern \"broken0.12.1\".",
- "WARN: ~/category/package/filename.mk:5: Invalid dependency pattern \"broken[0-9]*\".",
- "WARN: ~/category/package/filename.mk:6: Invalid dependency pattern with path \"broken[0-9]*../../x11/alacarte\".",
- "WARN: ~/category/package/filename.mk:7: Invalid dependency pattern \"broken>=\".",
- "WARN: ~/category/package/filename.mk:8: Invalid dependency pattern \"broken=0\".",
- "WARN: ~/category/package/filename.mk:9: Invalid dependency pattern \"broken=\".",
- "WARN: ~/category/package/filename.mk:10: Invalid dependency pattern \"broken-\".",
- "WARN: ~/category/package/filename.mk:11: Invalid dependency pattern \"broken>\".",
- "WARN: ~/category/package/filename.mk:13: Please use USE_TOOLS+=msgfmt instead of this dependency.",
- "WARN: ~/category/package/filename.mk:14: Please use USE_TOOLS+=gmake instead of this dependency.")
+ "WARN: ~/category/package/filename.mk:5: Please use USE_TOOLS+=msgfmt instead of this dependency.",
+ "WARN: ~/category/package/filename.mk:6: Please use USE_TOOLS+=gmake instead of this dependency.")
+
+ vt.Values(
+ "broken0.12.1:../../x11/alacarte", // missing version
+ "broken[0-9]*:../../x11/alacarte", // missing version
+ "broken[0-9]*../../x11/alacarte", // missing colon
+ "broken>=:../../x11/alacarte", // incomplete comparison
+ "broken=0:../../x11/alacarte", // invalid comparison operator
+ "broken=:../../x11/alacarte", // incomplete comparison
+ "broken-:../../x11/alacarte", // incomplete pattern
+ "broken>:../../x11/alacarte") // incomplete comparison
+
+ vt.Output(
+ "WARN: ~/category/package/filename.mk:11: Invalid dependency pattern \"broken0.12.1\".",
+ "WARN: ~/category/package/filename.mk:12: Invalid dependency pattern \"broken[0-9]*\".",
+ "WARN: ~/category/package/filename.mk:13: Invalid dependency pattern with path \"broken[0-9]*../../x11/alacarte\".",
+ "WARN: ~/category/package/filename.mk:14: Invalid dependency pattern \"broken>=\".",
+ "WARN: ~/category/package/filename.mk:15: Invalid dependency pattern \"broken=0\".",
+ "WARN: ~/category/package/filename.mk:16: Invalid dependency pattern \"broken=\".",
+ "WARN: ~/category/package/filename.mk:17: Invalid dependency pattern \"broken-\".",
+ "WARN: ~/category/package/filename.mk:18: Invalid dependency pattern \"broken>\".")
+
+ vt.Values(
+ "${PYPKGPREFIX}-sqlite3:../../${MY_PKGPATH.py-sqlite3}",
+ "${PYPKGPREFIX}-sqlite3:../../databases/py-sqlite3",
+ "${DEPENDS.NetBSD}",
+ "${DEPENDENCY_PATTERN.py-sqlite3}:${DEPENDENCY_PATH.py-sqlite}",
+ "${PYPKGPREFIX}-module>=0:../../devel/py-module",
+ "${EMACS_PACKAGE}>=${EMACS_MAJOR}:${EMACS_PKGDIR}",
+ "{${NETSCAPE_PREFERRED:C/:/,/g}}-[0-9]*:../../www/${NETSCAPE_PREFERRED:C/:.*//}")
+
+ vt.Output(
+ "WARN: ~/category/package/filename.mk:21: "+
+ "Invalid dependency pattern \"${PYPKGPREFIX}-sqlite3\".",
+ "WARN: ~/category/package/filename.mk:22: "+
+ "Invalid dependency pattern \"${PYPKGPREFIX}-sqlite3\".")
}
func (s *Suite) Test_VartypeCheck_DistSuffix(c *check.C) {
@@ -397,7 +478,7 @@ func (s *Suite) Test_VartypeCheck_FetchU
vt.Values(
"https://example.org/download.cgi?filename=filename&sha1=12341234";)
- t.CheckOutputEmpty()
+ vt.OutputEmpty()
vt.Values(
"http://example.org/distfiles/";,
@@ -405,7 +486,7 @@ func (s *Suite) Test_VartypeCheck_FetchU
"http://example.org/download?filename=<distfile>;version=<version>")
vt.Output(
- "WARN: filename.mk:8: \"http://example.org/download?filename=<distfile>;version=<version>\" is not a valid URL.")
+ "WARN: filename.mk:23: \"http://example.org/download?filename=<distfile>;version=<version>\" is not a valid URL.")
}
func (s *Suite) Test_VartypeCheck_Filename(c *check.C) {
@@ -517,7 +598,7 @@ func (s *Suite) Test_VartypeCheck_Homepa
// doesn't define MASTER_SITES, that variable cannot be expanded, which means
// the warning cannot refer to its value.
vt.Output(
- "WARN: filename.mk:4: HOMEPAGE should not be defined in terms of MASTER_SITEs.")
+ "WARN: filename.mk:11: HOMEPAGE should not be defined in terms of MASTER_SITEs.")
delete(G.Pkg.vars.firstDef, "MASTER_SITES")
delete(G.Pkg.vars.lastDef, "MASTER_SITES")
@@ -528,7 +609,7 @@ func (s *Suite) Test_VartypeCheck_Homepa
"${MASTER_SITES}")
vt.Output(
- "WARN: filename.mk:5: HOMEPAGE should not be defined in terms of MASTER_SITEs. " +
+ "WARN: filename.mk:21: HOMEPAGE should not be defined in terms of MASTER_SITEs. " +
"Use https://cdn.NetBSD.org/pub/pkgsrc/distfiles/ directly.")
delete(G.Pkg.vars.firstDef, "MASTER_SITES")
@@ -542,7 +623,7 @@ func (s *Suite) Test_VartypeCheck_Homepa
// When MASTER_SITES itself makes use of another variable, pkglint doesn't
// resolve that variable and just outputs the simple variant of this warning.
vt.Output(
- "WARN: filename.mk:6: HOMEPAGE should not be defined in terms of MASTER_SITEs.")
+ "WARN: filename.mk:31: HOMEPAGE should not be defined in terms of MASTER_SITEs.")
}
@@ -1424,7 +1505,7 @@ func (vt *VartypeCheckTester) Values(val
return varname + space + opStr + value
}
- test := func(mkline MkLine, value string) {
+ test := func(mklines MkLines, mkline MkLine, value string) {
varname := vt.varname
comment := ""
if mkline.IsVarassign() {
@@ -1437,21 +1518,19 @@ func (vt *VartypeCheckTester) Values(val
effectiveValue = mkline.Value()
}
- vartype := G.Pkgsrc.VariableType(varname)
+ vartype := G.Pkgsrc.VariableType(nil, varname)
// See MkLineChecker.checkVartype.
var lineValues []string
if vartype == nil || vartype.kindOfList == lkNone {
lineValues = []string{effectiveValue}
} else {
- var rest string
- lineValues, rest = splitIntoMkWords(mkline.Line, effectiveValue)
- vt.tester.Check(rest, equals, "")
+ lineValues = mkline.ValueFields(effectiveValue)
}
for _, lineValue := range lineValues {
valueNovar := mkline.WithoutMakeVariables(lineValue)
- vc := VartypeCheck{mkline, varname, vt.op, lineValue, valueNovar, comment, false}
+ vc := VartypeCheck{mklines, mkline, varname, vt.op, lineValue, valueNovar, comment, false}
vt.checker(&vc)
}
}
@@ -1463,7 +1542,7 @@ func (vt *VartypeCheckTester) Values(val
mklines := NewMkLines(NewLines(vt.filename, []Line{line}))
vt.lineno++
- mklines.ForEach(func(mkline MkLine) { test(mkline, value) })
+ mklines.ForEach(func(mkline MkLine) { test(mklines, mkline, value) })
}
}
@@ -1471,10 +1550,12 @@ func (vt *VartypeCheckTester) Values(val
// the same as the expectedLines.
func (vt *VartypeCheckTester) Output(expectedLines ...string) {
vt.tester.CheckOutputLines(expectedLines...)
+ vt.nextSection()
}
func (vt *VartypeCheckTester) OutputEmpty() {
vt.tester.CheckOutputEmpty()
+ vt.nextSection()
}
func (vt *VartypeCheckTester) nextSection() {
Index: pkgsrc/pkgtools/pkglint/files/mklines_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.39 pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.40
--- pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.39 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/mklines_test.go Wed Apr 3 21:49:51 2019
@@ -95,11 +95,17 @@ func (s *Suite) Test_MkLines__varuse_sh_
"qore-version=\tqore --short-version | ${SED} -e s/-.*//",
"PLIST_SUBST+=\tQORE_VERSION=\"${qore-version:sh}\"")
- vars2 := mklines.mklines[1].DetermineUsedVariables()
+ var vars2 []string
+ mklines.mklines[1].ForEachUsed(func(varUse *MkVarUse, time vucTime) {
+ vars2 = append(vars2, varUse.varname)
+ })
c.Check(vars2, deepEquals, []string{"SED"})
- vars3 := mklines.mklines[2].DetermineUsedVariables()
+ var vars3 []string
+ mklines.mklines[2].ForEachUsed(func(varUse *MkVarUse, time vucTime) {
+ vars3 = append(vars3, varUse.varname)
+ })
// qore-version, despite its unusual name, is a pretty normal Make variable.
c.Check(vars3, deepEquals, []string{"qore-version"})
@@ -365,7 +371,6 @@ func (s *Suite) Test_MkLines_collectUsed
mklines := t.NewMkLines("filename.mk",
"\t${VAR}")
mkline := mklines.mklines[0]
- G.Mk = mklines
mklines.collectUsedVariables()
@@ -385,7 +390,6 @@ func (s *Suite) Test_MkLines_collectUsed
"\t${outer.${inner}}")
assignMkline := mklines.mklines[2]
shellMkline := mklines.mklines[5]
- G.Mk = mklines
mklines.collectUsedVariables()
@@ -876,14 +880,14 @@ func (s *Suite) Test_MkLines_Check__MAST
t.SetUpMasterSite("MASTER_SITE_GITHUB", "https://github.com/";)
t.SetUpVartypes()
- G.Mk = t.NewMkLines("devel/catch/Makefile",
+ mklines := t.NewMkLines("devel/catch/Makefile",
MkRcsID,
"HOMEPAGE=\t${MASTER_SITE_GITHUB:=philsquared/Catch/}",
"HOMEPAGE=\t${MASTER_SITE_GITHUB}",
"HOMEPAGE=\t${MASTER_SITES}",
"HOMEPAGE=\t${MASTER_SITES}${GITHUB_PROJECT}")
- G.Mk.Check()
+ mklines.Check()
t.CheckOutputLines(
"WARN: devel/catch/Makefile:2: HOMEPAGE should not be defined in terms of MASTER_SITEs. "+
@@ -931,7 +935,7 @@ func (s *Suite) Test_MkLines_Check__extr
t.SetUpCommandLine("-Wextra")
t.SetUpVartypes()
G.Pkg = NewPackage(t.File("category/pkgbase"))
- G.Mk = t.NewMkLines("options.mk",
+ mklines := t.NewMkLines("options.mk",
MkRcsID,
"",
".for word in ${PKG_FAIL_REASON}",
@@ -944,7 +948,7 @@ func (s *Suite) Test_MkLines_Check__extr
"CONDITIONAL=\"@comment\"",
"BUILD_DIRS=\t${WRKSRC}/../build")
- G.Mk.Check()
+ mklines.Check()
t.CheckOutputLines(
"NOTE: options.mk:5: Please use \"# empty\", \"# none\" or \"# yes\" instead of \"# defined\".",
Index: pkgsrc/pkgtools/pkglint/files/mkparser.go
diff -u pkgsrc/pkgtools/pkglint/files/mkparser.go:1.25 pkgsrc/pkgtools/pkglint/files/mkparser.go:1.26
--- pkgsrc/pkgtools/pkglint/files/mkparser.go:1.25 Sun Mar 10 19:01:50 2019
+++ pkgsrc/pkgtools/pkglint/files/mkparser.go Wed Apr 3 21:49:51 2019
@@ -603,7 +603,7 @@ func (p *MkParser) PkgbasePattern() stri
for {
if p.VarUse() != nil ||
lexer.SkipRegexp(G.res.Compile(`^[\w.*+,{}]+`)) ||
- lexer.SkipRegexp(G.res.Compile(`^\[[\d-]+\]`)) {
+ lexer.SkipRegexp(G.res.Compile(`^\[[\w-]+\]`)) {
continue
}
@@ -633,6 +633,7 @@ type DependencyPattern struct {
Wildcard string // "[0-9]*", "1.5.*", "${PYVER}"
}
+// Dependency parses a dependency pattern like "pkg>=1<2" or "pkg-[0-9]*".
func (p *MkParser) Dependency() *DependencyPattern {
lexer := p.lexer
Index: pkgsrc/pkgtools/pkglint/files/mkparser_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.25 pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.26
--- pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.25 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/mkparser_test.go Wed Apr 3 21:49:51 2019
@@ -679,9 +679,8 @@ func (s *Suite) Test_MkParser_PkgbasePat
// as a version number.
testRest("pkgbase-${PKGNAME:C/^.*-//}-[0-9]*", "pkgbase", "-${PKGNAME:C/^.*-//}-[0-9]*")
- // Using the [a-z] pattern in the package base is not seen in the wild.
- // Therefore this rather strange parsing result is ok.
- testRest("pkgbase-[a-z]-1.0", "pkgbase-", "[a-z]-1.0")
+ // Using the [a-z] pattern in the package base is only rarely seen in the wild.
+ testRest("pkgbase-[a-z]*-1.0", "pkgbase-[a-z]*", "-1.0")
// This is a valid dependency pattern, but it's more complicated
// than the patterns pkglint can handle as of January 2019.
Index: pkgsrc/pkgtools/pkglint/files/util_test.go
diff -u pkgsrc/pkgtools/pkglint/files/util_test.go:1.25 pkgsrc/pkgtools/pkglint/files/util_test.go:1.26
--- pkgsrc/pkgtools/pkglint/files/util_test.go:1.25 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/util_test.go Wed Apr 3 21:49:51 2019
@@ -373,7 +373,8 @@ func (s *Suite) Test_Scope_Used(c *check
t := s.Init(c)
scope := NewScope()
- scope.Use("VAR.param", t.NewMkLine("file.mk", 1, "\techo ${VAR.param}"))
+ mkline := t.NewMkLine("file.mk", 1, "\techo ${VAR.param}")
+ scope.Use("VAR.param", mkline, vucTimeRun)
c.Check(scope.Used("VAR.param"), equals, true)
c.Check(scope.Used("VAR.other"), equals, false)
Index: pkgsrc/pkgtools/pkglint/files/mktypes.go
diff -u pkgsrc/pkgtools/pkglint/files/mktypes.go:1.12 pkgsrc/pkgtools/pkglint/files/mktypes.go:1.13
--- pkgsrc/pkgtools/pkglint/files/mktypes.go:1.12 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/mktypes.go Wed Apr 3 21:49:51 2019
@@ -137,6 +137,39 @@ func (m MkVarUseModifier) MatchMatch() (
func (m MkVarUseModifier) IsToLower() bool { return m.Text == "tl" }
+// ChangesWords returns true if applying this modifier to a list variable
+// may change the number of words in the list, or their boundaries.
+func (m MkVarUseModifier) ChangesWords() bool {
+ text := m.Text
+
+ // See MkParser.VarUseModifiers for the meaning of these modifiers.
+ switch text[0] {
+
+ case 'E', 'H', 'M', 'N', 'O', 'R', 'T':
+ return false
+
+ case 'C', 'Q', 'S':
+ // For the :C and :S modifiers, a more detailed analysis could reveal
+ // cases that don't change the structure, such as :S,a,b,g or
+ // :C,[0-9A-Za-z_],.,g, but not :C,x,,g.
+ return true
+ }
+
+ switch text {
+
+ case "tl", "tu":
+ return false
+
+ case "sh", "tW", "tw":
+ return true
+ }
+
+ // If in doubt, be pessimistic. As of March 2019, the only code that
+ // actually uses this function doesn't issue a possibly wrong warning
+ // in such a case.
+ return true
+}
+
func (vu *MkVarUse) Mod() string {
var mod strings.Builder
for _, modifier := range vu.modifiers {
Index: pkgsrc/pkgtools/pkglint/files/mktypes_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mktypes_test.go:1.8 pkgsrc/pkgtools/pkglint/files/mktypes_test.go:1.9
--- pkgsrc/pkgtools/pkglint/files/mktypes_test.go:1.8 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/mktypes_test.go Wed Apr 3 21:49:51 2019
@@ -28,6 +28,46 @@ func (list *MkShList) AddCommand(command
return list.AddAndOr(andOr)
}
+func (s *Suite) Test_MkVarUseModifier_ChangesWords(c *check.C) {
+ t := s.Init(c)
+
+ test := func(modifier string, changes bool) {
+ mod := MkVarUseModifier{modifier}
+ t.Check(mod.ChangesWords(), equals, changes)
+ }
+
+ test("E", false)
+ test("R", false)
+ test("Mpattern", false)
+ test("Npattern", false)
+ test("S,from,to,", true)
+ test("C,from,to,", true)
+ test("tl", false)
+ test("tu", false)
+ test("sh", true)
+
+ test("unknown", true)
+}
+
+// Ensures that ChangesWords cannot be called with an empty string as modifier.
+func (s *Suite) Test_MkVarUseModifier_ChangesWords__empty(c *check.C) {
+ t := s.Init(c)
+
+ mkline := t.NewMkLine("filename.mk", 123, "\t${VAR:}")
+
+ n := 0
+ mkline.ForEachUsed(func(varUse *MkVarUse, time vucTime) {
+ n += 100
+ for _, mod := range varUse.modifiers {
+ mod.ChangesWords()
+ n++
+ }
+ })
+
+ t.CheckOutputEmpty()
+ t.Check(n, equals, 100)
+}
+
func (s *Suite) Test_MkVarUseModifier_MatchSubst(c *check.C) {
mod := MkVarUseModifier{"S/from/to/1g"}
Index: pkgsrc/pkgtools/pkglint/files/package.go
diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.48 pkgsrc/pkgtools/pkglint/files/package.go:1.49
--- pkgsrc/pkgtools/pkglint/files/package.go:1.48 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/package.go Wed Apr 3 21:49:51 2019
@@ -94,7 +94,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(pkg.dir + "/" + relativeFileName))
+ return cleanpath(resolveVariableRefs(nil, pkg.dir+"/"+relativeFileName))
}
func (pkg *Package) checkPossibleDowngrade() {
@@ -288,11 +288,11 @@ func (pkg *Package) loadPackageMakefile(
pkg.Patchdir = pkg.vars.LastValue("PATCHDIR")
// See lang/php/ext.mk
- if varIsDefinedSimilar(pkg, nil, "PHPEXT_MK") {
- if !varIsDefinedSimilar(pkg, nil, "USE_PHP_EXT_PATCHES") {
+ if pkg.vars.DefinedSimilar("PHPEXT_MK") {
+ if !pkg.vars.DefinedSimilar("USE_PHP_EXT_PATCHES") {
pkg.Patchdir = "patches"
}
- if varIsDefinedSimilar(pkg, nil, "PECL_VERSION") {
+ if pkg.vars.DefinedSimilar("PECL_VERSION") {
pkg.DistinfoFile = "distinfo"
} else {
pkg.IgnoreMissingPatches = true
@@ -495,7 +495,7 @@ func (pkg *Package) findIncludedFile(mkl
// TODO: resolveVariableRefs uses G.Pkg implicitly. It should be made explicit.
// TODO: Try to combine resolveVariableRefs and ResolveVarsInRelativePath.
- includedFile = resolveVariableRefs(mkline.ResolveVarsInRelativePath(mkline.IncludedFile()))
+ includedFile = resolveVariableRefs(nil, mkline.ResolveVarsInRelativePath(mkline.IncludedFile()))
if containsVarRef(includedFile) {
if trace.Tracing && !contains(includingFilename, "/mk/") {
trace.Stepf("%s:%s: Skipping include file %q. This may result in false warnings.",
Index: pkgsrc/pkgtools/pkglint/files/package_test.go
diff -u pkgsrc/pkgtools/pkglint/files/package_test.go:1.41 pkgsrc/pkgtools/pkglint/files/package_test.go:1.42
--- pkgsrc/pkgtools/pkglint/files/package_test.go:1.41 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/package_test.go Wed Apr 3 21:49:51 2019
@@ -560,12 +560,20 @@ func (s *Suite) Test_Package__varuse_at_
G.Check(t.File("category/pkgbase"))
t.CheckOutputLines(
+ "NOTE: ~/category/pkgbase/Makefile:14: Consider the :sh modifier instead of != for \"echo false=${FALSE:Q}\".",
"WARN: ~/category/pkgbase/Makefile:14: To use the tool ${FALSE} at load time, bsd.prefs.mk has to be included before.",
- // TODO: "before including bsd.prefs.mk in line ###".
+ "NOTE: ~/category/pkgbase/Makefile:15: Consider the :sh modifier instead of != for \"echo nice=${NICE:Q}\".",
+
+ // TODO: replace "at load time" with "before including bsd.prefs.mk in line ###".
// TODO: ${NICE} could be used at load time if it were added to USE_TOOLS earlier.
"WARN: ~/category/pkgbase/Makefile:15: The tool ${NICE} cannot be used at load time.",
+
+ "NOTE: ~/category/pkgbase/Makefile:16: Consider the :sh modifier instead of != for \"echo true=${TRUE:Q}\".",
"WARN: ~/category/pkgbase/Makefile:16: To use the tool ${TRUE} at load time, bsd.prefs.mk has to be included before.",
- "WARN: ~/category/pkgbase/Makefile:25: The tool ${NICE} cannot be used at load time.")
+ "NOTE: ~/category/pkgbase/Makefile:24: Consider the :sh modifier instead of != for \"echo false=${FALSE:Q}\".",
+ "NOTE: ~/category/pkgbase/Makefile:25: Consider the :sh modifier instead of != for \"echo nice=${NICE:Q}\".",
+ "WARN: ~/category/pkgbase/Makefile:25: The tool ${NICE} cannot be used at load time.",
+ "NOTE: ~/category/pkgbase/Makefile:26: Consider the :sh modifier instead of != for \"echo true=${TRUE:Q}\".")
}
func (s *Suite) Test_Package_loadPackageMakefile(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/pkglint_test.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.35 pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.36
--- pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.35 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/pkglint_test.go Wed Apr 3 21:49:51 2019
@@ -425,7 +425,7 @@ func (s *Suite) Test_resolveVariableRefs
// TODO: It may be better to define MkLines.Resolve and Package.Resolve,
// to clearly state the scope of the involved variables.
- resolved := resolveVariableRefs("gcc-${GCC_VERSION}")
+ resolved := resolveVariableRefs(nil, "gcc-${GCC_VERSION}")
c.Check(resolved, equals, "gcc-${GCC_VERSION}")
}
@@ -437,14 +437,14 @@ func (s *Suite) Test_resolveVariableRefs
mkline2 := t.NewMkLine("filename.mk", 11, "SECOND=\t${THIRD}")
mkline3 := t.NewMkLine("filename.mk", 12, "THIRD=\tgot it")
G.Pkg = NewPackage(t.File("category/pkgbase"))
- defineVar(G.Pkg, nil, mkline1, "FIRST")
- defineVar(G.Pkg, nil, mkline2, "SECOND")
- defineVar(G.Pkg, nil, mkline3, "THIRD")
+ G.Pkg.vars.Define("FIRST", mkline1)
+ G.Pkg.vars.Define("SECOND", mkline2)
+ G.Pkg.vars.Define("THIRD", mkline3)
// TODO: Add a similar test in which some of the variables are defined
// conditionally or with differing values, just to see what pkglint does
// in such a case.
- resolved := resolveVariableRefs("you ${FIRST}")
+ resolved := resolveVariableRefs(nil, "you ${FIRST}")
c.Check(resolved, equals, "you got it")
}
@@ -459,7 +459,7 @@ func (s *Suite) Test_resolveVariableRefs
G.Pkg = NewPackage(t.File("category/pkg"))
G.Pkg.vars.Define("GST_PLUGINS0.10_TYPE", mkline)
- resolved := resolveVariableRefs("gst-plugins0.10-${GST_PLUGINS0.10_TYPE}/distinfo")
+ resolved := resolveVariableRefs(nil, "gst-plugins0.10-${GST_PLUGINS0.10_TYPE}/distinfo")
c.Check(resolved, equals, "gst-plugins0.10-x11/distinfo")
}
@@ -631,15 +631,15 @@ func (s *Suite) Test_Pkglint_Tool__prefe
t := s.Init(c)
mkline := t.NewMkLine("dummy.mk", 123, "DUMMY=\tvalue")
- G.Mk = t.NewMkLines("Makefile", MkRcsID)
+ mklines := t.NewMkLines("Makefile", MkRcsID)
global := G.Pkgsrc.Tools.Define("tool", "TOOL", mkline)
- local := G.Mk.Tools.Define("tool", "TOOL", mkline)
+ local := mklines.Tools.Define("tool", "TOOL", mkline)
global.Validity = Nowhere
local.Validity = AtRunTime
- loadTimeTool, loadTimeUsable := G.Tool("tool", LoadTime)
- runTimeTool, runTimeUsable := G.Tool("tool", RunTime)
+ loadTimeTool, loadTimeUsable := G.Tool(mklines, "tool", LoadTime)
+ runTimeTool, runTimeUsable := G.Tool(mklines, "tool", RunTime)
c.Check(loadTimeTool, equals, local)
c.Check(loadTimeUsable, equals, false)
@@ -650,11 +650,11 @@ func (s *Suite) Test_Pkglint_Tool__prefe
func (s *Suite) Test_Pkglint_Tool__lookup_by_name_fallback(c *check.C) {
t := s.Init(c)
- G.Mk = t.NewMkLines("Makefile", MkRcsID)
+ mklines := t.NewMkLines("Makefile", MkRcsID)
t.SetUpTool("tool", "", Nowhere)
- loadTimeTool, loadTimeUsable := G.Tool("tool", LoadTime)
- runTimeTool, runTimeUsable := G.Tool("tool", RunTime)
+ loadTimeTool, loadTimeUsable := G.Tool(mklines, "tool", LoadTime)
+ runTimeTool, runTimeUsable := G.Tool(mklines, "tool", RunTime)
// The tool is returned even though it may not be used at the moment.
// The calling code must explicitly check for usability.
@@ -670,15 +670,15 @@ func (s *Suite) Test_Pkglint_Tool__looku
t := s.Init(c)
mkline := t.NewMkLine("dummy.mk", 123, "DUMMY=\tvalue")
- G.Mk = t.NewMkLines("Makefile", MkRcsID)
+ mklines := t.NewMkLines("Makefile", MkRcsID)
global := G.Pkgsrc.Tools.Define("tool", "TOOL", mkline)
- local := G.Mk.Tools.Define("tool", "TOOL", mkline)
+ local := mklines.Tools.Define("tool", "TOOL", mkline)
global.Validity = Nowhere
local.Validity = AtRunTime
- loadTimeTool, loadTimeUsable := G.Tool("${TOOL}", LoadTime)
- runTimeTool, runTimeUsable := G.Tool("${TOOL}", RunTime)
+ loadTimeTool, loadTimeUsable := G.Tool(mklines, "${TOOL}", LoadTime)
+ runTimeTool, runTimeUsable := G.Tool(mklines, "${TOOL}", RunTime)
c.Check(loadTimeTool, equals, local)
c.Check(loadTimeUsable, equals, false)
@@ -690,11 +690,11 @@ func (s *Suite) Test_Pkglint_Tool__looku
func (s *Suite) Test_Pkglint_Tool__lookup_by_varname_fallback(c *check.C) {
t := s.Init(c)
- G.Mk = t.NewMkLines("Makefile", MkRcsID)
+ mklines := t.NewMkLines("Makefile", MkRcsID)
G.Pkgsrc.Tools.def("tool", "TOOL", false, Nowhere)
- loadTimeTool, loadTimeUsable := G.Tool("${TOOL}", LoadTime)
- runTimeTool, runTimeUsable := G.Tool("${TOOL}", RunTime)
+ loadTimeTool, loadTimeUsable := G.Tool(mklines, "${TOOL}", LoadTime)
+ runTimeTool, runTimeUsable := G.Tool(mklines, "${TOOL}", RunTime)
c.Check(loadTimeTool.String(), equals, "tool:TOOL::Nowhere")
c.Check(loadTimeUsable, equals, false)
@@ -706,11 +706,11 @@ func (s *Suite) Test_Pkglint_Tool__looku
func (s *Suite) Test_Pkglint_Tool__lookup_by_varname_fallback_runtime(c *check.C) {
t := s.Init(c)
- G.Mk = t.NewMkLines("Makefile", MkRcsID)
+ mklines := t.NewMkLines("Makefile", MkRcsID)
G.Pkgsrc.Tools.def("tool", "TOOL", false, AtRunTime)
- loadTimeTool, loadTimeUsable := G.Tool("${TOOL}", LoadTime)
- runTimeTool, runTimeUsable := G.Tool("${TOOL}", RunTime)
+ loadTimeTool, loadTimeUsable := G.Tool(mklines, "${TOOL}", LoadTime)
+ runTimeTool, runTimeUsable := G.Tool(mklines, "${TOOL}", RunTime)
c.Check(loadTimeTool.String(), equals, "tool:TOOL::AtRunTime")
c.Check(loadTimeUsable, equals, false)
@@ -722,23 +722,23 @@ func (s *Suite) Test_Pkglint_ToolByVarna
t := s.Init(c)
mkline := t.NewMkLine("dummy.mk", 123, "DUMMY=\tvalue")
- G.Mk = t.NewMkLines("Makefile", MkRcsID)
+ mklines := t.NewMkLines("Makefile", MkRcsID)
global := G.Pkgsrc.Tools.Define("tool", "TOOL", mkline)
- local := G.Mk.Tools.Define("tool", "TOOL", mkline)
+ local := mklines.Tools.Define("tool", "TOOL", mkline)
global.Validity = Nowhere
local.Validity = AtRunTime
- c.Check(G.ToolByVarname("TOOL"), equals, local)
+ c.Check(G.ToolByVarname(mklines, "TOOL"), equals, local)
}
func (s *Suite) Test_Pkglint_ToolByVarname(c *check.C) {
t := s.Init(c)
- G.Mk = t.NewMkLines("Makefile", MkRcsID)
+ mklines := t.NewMkLines("Makefile", MkRcsID)
G.Pkgsrc.Tools.def("tool", "TOOL", false, AtRunTime)
- c.Check(G.ToolByVarname("TOOL").String(), equals, "tool:TOOL::AtRunTime")
+ c.Check(G.ToolByVarname(mklines, "TOOL").String(), equals, "tool:TOOL::AtRunTime")
}
func (s *Suite) Test_Pkglint_checkReg__other(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go
diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.19 pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.20
--- pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.19 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go Wed Apr 3 21:49:51 2019
@@ -573,7 +573,7 @@ func (s *Suite) Test_Pkgsrc_VariableType
t.SetUpVartypes()
test := func(varname string, vartype string) {
- actualType := G.Pkgsrc.VariableType(varname)
+ actualType := G.Pkgsrc.VariableType(nil, varname)
if vartype == "" {
c.Check(actualType, check.IsNil)
} else {
@@ -602,12 +602,12 @@ func (s *Suite) Test_Pkgsrc_VariableType
t.SetUpVartypes()
- t1 := G.Pkgsrc.VariableType("FONT_DIRS")
+ t1 := G.Pkgsrc.VariableType(nil, "FONT_DIRS")
c.Assert(t1, check.NotNil)
c.Check(t1.String(), equals, "List of PathMask (guessed)")
- t2 := G.Pkgsrc.VariableType("FONT_DIRS.ttf")
+ t2 := G.Pkgsrc.VariableType(nil, "FONT_DIRS.ttf")
c.Assert(t2, check.NotNil)
c.Check(t2.String(), equals, "List of PathMask (guessed)")
@@ -638,15 +638,15 @@ func (s *Suite) Test_Pkgsrc_VariableType
G.Main("pkglint", "-Wall", pkg)
- if typ := G.Pkgsrc.VariableType("PKGSRC_MAKE_ENV"); c.Check(typ, check.NotNil) {
+ if typ := G.Pkgsrc.VariableType(nil, "PKGSRC_MAKE_ENV"); c.Check(typ, check.NotNil) {
c.Check(typ.String(), equals, "List of ShellWord (guessed)")
}
- if typ := G.Pkgsrc.VariableType("CPPPATH"); c.Check(typ, check.NotNil) {
+ if typ := G.Pkgsrc.VariableType(nil, "CPPPATH"); c.Check(typ, check.NotNil) {
c.Check(typ.String(), equals, "Pathlist (guessed)")
}
- if typ := G.Pkgsrc.VariableType("OSNAME.Other"); c.Check(typ, check.NotNil) {
+ if typ := G.Pkgsrc.VariableType(nil, "OSNAME.Other"); c.Check(typ, check.NotNil) {
c.Check(typ.String(), equals, "Unknown")
}
@@ -672,10 +672,15 @@ func (s *Suite) Test_Pkgsrc_guessVariabl
mklines.Check()
- // FIXME: The permissions in guessVariableType say allRuntime, which excludes
- // aclpUseLoadtime. Therefore there should be a warning about the VarUse in
- // the .if line.
- // The check in MkLineChecker.checkVarusePermissions is disabled for guessed types.
+ vartype := G.Pkgsrc.VariableType(mklines, "MY_CHECK_SKIP")
+ t.Check(vartype.guessed, equals, true)
+ t.Check(vartype.EffectivePermissions("filename.mk"), equals, aclpAllRuntime)
+
+ // The permissions for MY_CHECK_SKIP say aclpAllRuntime, which excludes
+ // aclpUseLoadtime. Therefore there should be a warning about the VarUse in
+ // the .if line. As of March 2019, pkglint skips the permissions check for
+ // guessed variables since that variable might have an entirely different
+ // meaning; see MkLineChecker.checkVarusePermissions.
//
// There is no warning for the += operator in line 3 since the variable type
// (although guessed) is a list of things, and lists may be appended to.
Index: pkgsrc/pkgtools/pkglint/files/redundantscope.go
diff -u pkgsrc/pkgtools/pkglint/files/redundantscope.go:1.2 pkgsrc/pkgtools/pkglint/files/redundantscope.go:1.3
--- pkgsrc/pkgtools/pkglint/files/redundantscope.go:1.2 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/redundantscope.go Wed Apr 3 21:49:51 2019
@@ -153,12 +153,13 @@ func (s *RedundantScope) handleVarassign
func (s *RedundantScope) handleVarUse(mkline MkLine) {
switch {
case mkline.IsVarassign():
- for _, varname := range mkline.DetermineUsedVariables() {
+ mkline.ForEachUsed(func(varUse *MkVarUse, time vucTime) {
+ varname := varUse.varname
info := s.get(varname)
info.vari.Read(mkline)
info.lastAction = 1
s.access(varname)
- }
+ })
case mkline.IsDirective():
// TODO: Handle varuse for conditions and loops.
Index: pkgsrc/pkgtools/pkglint/files/var.go
diff -u pkgsrc/pkgtools/pkglint/files/var.go:1.2 pkgsrc/pkgtools/pkglint/files/var.go:1.3
--- pkgsrc/pkgtools/pkglint/files/var.go:1.2 Sun Mar 10 19:01:50 2019
+++ pkgsrc/pkgtools/pkglint/files/var.go Wed Apr 3 21:49:51 2019
@@ -179,7 +179,9 @@ func (v *Var) Write(mkline MkLine, condi
}
v.conditionalVars.AddAll(conditionVarnames)
- v.refs.AddAll(mkline.DetermineUsedVariables())
+ mkline.ForEachUsed(func(varUse *MkVarUse, time vucTime) {
+ v.refs.Add(varUse.varname)
+ })
v.refs.AddAll(conditionVarnames)
v.update(mkline, &v.valueInfra)
Index: pkgsrc/pkgtools/pkglint/files/shell_test.go
diff -u pkgsrc/pkgtools/pkglint/files/shell_test.go:1.42 pkgsrc/pkgtools/pkglint/files/shell_test.go:1.43
--- pkgsrc/pkgtools/pkglint/files/shell_test.go:1.42 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/shell_test.go Wed Apr 3 21:49:51 2019
@@ -96,13 +96,6 @@ func (s *Suite) Test_splitIntoShellToken
c.Check(rest, equals, "")
}
-func (s *Suite) Test_splitIntoMkWords__semicolons(c *check.C) {
- words, rest := splitIntoMkWords(dummyLine, "word1 word2;;;")
-
- c.Check(words, deepEquals, []string{"word1", "word2;;;"})
- c.Check(rest, equals, "")
-}
-
func (s *Suite) Test_splitIntoShellTokens__varuse_with_embedded_space(c *check.C) {
words, rest := splitIntoShellTokens(dummyLine, "${VAR:S/ /_/g}")
@@ -110,13 +103,6 @@ func (s *Suite) Test_splitIntoShellToken
c.Check(rest, equals, "")
}
-func (s *Suite) Test_splitIntoMkWords__varuse_with_embedded_space(c *check.C) {
- words, rest := splitIntoMkWords(dummyLine, "${VAR:S/ /_/g}")
-
- c.Check(words, deepEquals, []string{"${VAR:S/ /_/g}"})
- c.Check(rest, equals, "")
-}
-
func (s *Suite) Test_splitIntoShellTokens__redirect(c *check.C) {
words, rest := splitIntoShellTokens(dummyLine, "echo 1>output 2>>append 3>|clobber 4>&5 6<input >>append")
@@ -154,11 +140,11 @@ func (s *Suite) Test_ShellLineChecker_Ch
t.SetUpTool("unzip", "UNZIP_CMD", AtRunTime)
test := func(shellCommand string, diagnostics ...string) {
- G.Mk = t.NewMkLines("filename.mk",
+ mklines := t.NewMkLines("filename.mk",
"\t"+shellCommand)
- ck := NewShellLineChecker(G.Mk.mklines[0])
+ ck := NewShellLineChecker(mklines, mklines.mklines[0])
- G.Mk.ForEach(func(mkline MkLine) {
+ mklines.ForEach(func(mkline MkLine) {
ck.CheckShellCommandLine(ck.mkline.ShellCommand())
})
@@ -276,11 +262,11 @@ func (s *Suite) Test_ShellLineChecker_Ch
t := s.Init(c)
test := func(shellCommand string) {
- G.Mk = t.NewMkLines("filename.mk",
+ mklines := t.NewMkLines("filename.mk",
"\t"+shellCommand)
- G.Mk.ForEach(func(mkline MkLine) {
- ck := NewShellLineChecker(mkline)
+ mklines.ForEach(func(mkline MkLine) {
+ ck := NewShellLineChecker(mklines, mkline)
ck.CheckShellCommandLine(mkline.ShellCommand())
})
}
@@ -303,9 +289,9 @@ func (s *Suite) Test_ShellLineChecker_Ch
t.SetUpVartypes()
t.SetUpTool("echo", "", AtRunTime)
- G.Mk = t.NewMkLines("Makefile",
+ mklines := t.NewMkLines("Makefile",
"\techo ${PKGNAME:Q}")
- ck := NewShellLineChecker(G.Mk.mklines[0])
+ ck := NewShellLineChecker(mklines, mklines.mklines[0])
ck.CheckShellCommandLine("echo ${PKGNAME:Q}")
@@ -319,9 +305,9 @@ func (s *Suite) Test_ShellLineChecker_Ch
t.SetUpCommandLine("-Wall", "--show-autofix")
t.SetUpVartypes()
t.SetUpTool("echo", "", AtRunTime)
- G.Mk = t.NewMkLines("Makefile",
+ mklines := t.NewMkLines("Makefile",
"\techo ${PKGNAME:Q}")
- ck := NewShellLineChecker(G.Mk.mklines[0])
+ ck := NewShellLineChecker(mklines, mklines.mklines[0])
ck.CheckShellCommandLine("echo ${PKGNAME:Q}")
@@ -336,9 +322,9 @@ func (s *Suite) Test_ShellLineChecker_Ch
t.SetUpCommandLine("-Wall", "--autofix")
t.SetUpVartypes()
t.SetUpTool("echo", "", AtRunTime)
- G.Mk = t.NewMkLines("Makefile",
+ mklines := t.NewMkLines("Makefile",
"\techo ${PKGNAME:Q}")
- ck := NewShellLineChecker(G.Mk.mklines[0])
+ ck := NewShellLineChecker(mklines, mklines.mklines[0])
ck.CheckShellCommandLine("echo ${PKGNAME:Q}")
@@ -358,7 +344,7 @@ func (s *Suite) Test_ShellProgramChecker
t.SetUpTool("printf", "", AtRunTime)
t.SetUpTool("sed", "", AtRunTime)
t.SetUpTool("right-side", "", AtRunTime)
- G.Mk = t.NewMkLines("Makefile",
+ mklines := t.NewMkLines("Makefile",
"\t echo | right-side",
"\t sed s,s,s, | right-side",
"\t printf | sed s,s,s, | right-side ",
@@ -371,8 +357,8 @@ func (s *Suite) Test_ShellProgramChecker
"\t var=value | right-side",
"\t if :; then :; fi | right-side")
- for _, mkline := range G.Mk.mklines {
- ck := NewShellLineChecker(mkline)
+ for _, mkline := range mklines.mklines {
+ ck := NewShellLineChecker(mklines, mkline)
ck.CheckShellCommandLine(mkline.ShellCommand())
}
@@ -391,9 +377,9 @@ func (s *Suite) Test_ShellLineChecker_Ch
t := s.Init(c)
t.SetUpVartypes()
- G.Mk = t.NewMkLines("filename.mk",
+ mklines := t.NewMkLines("filename.mk",
"# dummy")
- ck := NewShellLineChecker(G.Mk.mklines[0])
+ ck := NewShellLineChecker(mklines, mklines.mklines[0])
// foobar="`echo \"foo bar\"`"
text := "foobar=\"`echo \\\"foo bar\\\"`\""
@@ -403,12 +389,12 @@ func (s *Suite) Test_ShellLineChecker_Ch
c.Check(tokens, deepEquals, []string{text})
c.Check(rest, equals, "")
- G.Mk.ForEach(func(mkline MkLine) { ck.CheckWord(text, false, RunTime) })
+ mklines.ForEach(func(mkline MkLine) { ck.CheckWord(text, false, RunTime) })
t.CheckOutputLines(
"WARN: filename.mk:1: Unknown shell command \"echo\".")
- G.Mk.ForEach(func(mkline MkLine) { ck.CheckShellCommandLine(text) })
+ mklines.ForEach(func(mkline MkLine) { ck.CheckShellCommandLine(text) })
// No parse errors
t.CheckOutputLines(
@@ -420,9 +406,9 @@ func (s *Suite) Test_ShellLineChecker_Ch
t.SetUpVartypes()
t.SetUpTool("pax", "", AtRunTime)
- G.Mk = t.NewMkLines("filename.mk",
+ mklines := t.NewMkLines("filename.mk",
"# dummy")
- ck := NewShellLineChecker(G.Mk.mklines[0])
+ ck := NewShellLineChecker(mklines, mklines.mklines[0])
ck.CheckShellCommandLine("pax -rwpp -s /.*~$$//g . ${DESTDIR}${PREFIX}")
@@ -435,13 +421,11 @@ func (s *Suite) Test_ShellLineChecker_Ch
t.SetUpVartypes()
test := func(shellWord string, checkQuoting bool) {
- ck := t.NewShellLineChecker("dummy.mk", 1, "\t echo "+shellWord)
-
// Provide a storage for the "used but not defined" warnings.
// See checkVaruseUndefined and checkVarassignLeftNotUsed.
- G.Mk = NewMkLines(NewLines("dummy.mk", nil))
- defer func() { G.Mk = nil }()
+ mklines := NewMkLines(NewLines("dummy.mk", nil))
+ ck := t.NewShellLineChecker(mklines, "dummy.mk", 1, "\t echo "+shellWord)
ck.CheckWord(shellWord, checkQuoting, RunTime)
}
@@ -506,7 +490,7 @@ func (s *Suite) Test_ShellLineChecker_Ch
func (s *Suite) Test_ShellLineChecker_CheckWord__dollar_without_variable(c *check.C) {
t := s.Init(c)
- ck := t.NewShellLineChecker("filename.mk", 1, "# dummy")
+ ck := t.NewShellLineChecker(nil, "filename.mk", 1, "# dummy")
ck.CheckWord("/.*~$$//g", false, RunTime) // Typical argument to pax(1).
@@ -517,7 +501,7 @@ func (s *Suite) Test_ShellLineChecker_Ch
t := s.Init(c)
t.SetUpTool("find", "FIND", AtRunTime)
- ck := t.NewShellLineChecker("filename.mk", 1, "\tfind . -exec rm -rf {} \\+")
+ ck := t.NewShellLineChecker(nil, "filename.mk", 1, "\tfind . -exec rm -rf {} \\+")
ck.CheckShellCommandLine(ck.mkline.ShellCommand())
@@ -528,7 +512,7 @@ func (s *Suite) Test_ShellLineChecker_Ch
func (s *Suite) Test_ShellLineChecker_CheckWord__squot_dollar(c *check.C) {
t := s.Init(c)
- ck := t.NewShellLineChecker("filename.mk", 1, "\t'$")
+ ck := t.NewShellLineChecker(nil, "filename.mk", 1, "\t'$")
ck.CheckWord(ck.mkline.ShellCommand(), false, RunTime)
@@ -542,7 +526,7 @@ func (s *Suite) Test_ShellLineChecker_Ch
func (s *Suite) Test_ShellLineChecker_CheckWord__dquot_dollar(c *check.C) {
t := s.Init(c)
- ck := t.NewShellLineChecker("filename.mk", 1, "\t\"$")
+ ck := t.NewShellLineChecker(nil, "filename.mk", 1, "\t\"$")
ck.CheckWord(ck.mkline.ShellCommand(), false, RunTime)
@@ -554,7 +538,7 @@ func (s *Suite) Test_ShellLineChecker_Ch
func (s *Suite) Test_ShellLineChecker_CheckWord__dollar_subshell(c *check.C) {
t := s.Init(c)
- ck := t.NewShellLineChecker("filename.mk", 1, "\t$$(echo output)")
+ ck := t.NewShellLineChecker(nil, "filename.mk", 1, "\t$$(echo output)")
ck.CheckWord(ck.mkline.ShellCommand(), false, RunTime)
@@ -566,12 +550,12 @@ func (s *Suite) Test_ShellLineChecker_Ch
t := s.Init(c)
t.SetUpVartypes()
- G.Mk = t.NewMkLines("chat/ircII/Makefile",
+ mklines := t.NewMkLines("chat/ircII/Makefile",
MkRcsID,
"CONFIGURE_ARGS+=--mandir=${DESTDIR}${PREFIX}/man",
"CONFIGURE_ARGS+=--mandir=${DESTDIR}${PREFIX}/${PKGMANDIR}")
- G.Mk.Check()
+ mklines.Check()
t.CheckOutputLines(
"WARN: chat/ircII/Makefile:2: Please use ${PKGMANDIR} instead of \"man\".",
@@ -608,7 +592,7 @@ func (s *Suite) Test_ShellLineChecker_un
// direct, forcing test is only to reach the code coverage.
atoms := []*ShAtom{
NewShAtom(shtText, "`", shqBackt)}
- NewShellLineChecker(mkline).
+ NewShellLineChecker(nil, mkline).
unescapeBackticks(&atoms, shqBackt)
t.CheckOutputLines(
@@ -674,15 +658,15 @@ func (s *Suite) Test_ShellLineChecker_Ch
echo := t.SetUpTool("echo", "ECHO", AtRunTime)
echo.MustUseVarForm = true
- G.Mk = t.NewMkLines("filename.mk",
+ mklines := t.NewMkLines("filename.mk",
"# dummy")
mkline := t.NewMkLine("filename.mk", 3, "# dummy")
- MkLineChecker{mkline}.checkText("echo \"hello, world\"")
+ MkLineChecker{mklines, mkline}.checkText("echo \"hello, world\"")
t.CheckOutputEmpty()
- NewShellLineChecker(mkline).CheckShellCommandLine("echo \"hello, world\"")
+ NewShellLineChecker(mklines, mkline).CheckShellCommandLine("echo \"hello, world\"")
t.CheckOutputLines(
"WARN: filename.mk:3: Please use \"${ECHO}\" instead of \"echo\".")
@@ -698,7 +682,7 @@ func (s *Suite) Test_ShellLineChecker_Ch
t.SetUpTool("sed", "SED", AtRunTime)
text := "\tfor f in *.pl; do ${SED} s,@PREFIX@,${PREFIX}, < $f > $f.tmp && ${MV} $f.tmp $f; done"
- ck := t.NewShellLineChecker("Makefile", 3, text)
+ ck := t.NewShellLineChecker(nil, "Makefile", 3, text)
ck.mkline.Tokenize(ck.mkline.ShellCommand(), true)
ck.CheckShellCommandLine(text)
@@ -723,11 +707,11 @@ func (s *Suite) Test_ShellLineChecker_Ch
func (s *Suite) Test_ShellLineChecker_checkInstallCommand(c *check.C) {
t := s.Init(c)
- G.Mk = t.NewMkLines("filename.mk",
+ mklines := t.NewMkLines("filename.mk",
"# dummy")
- G.Mk.target = "do-install"
+ mklines.target = "do-install"
- ck := t.NewShellLineChecker("filename.mk", 1, "\tdummy")
+ ck := t.NewShellLineChecker(mklines, "filename.mk", 1, "\tdummy")
ck.checkInstallCommand("sed")
@@ -740,38 +724,13 @@ func (s *Suite) Test_ShellLineChecker_ch
"WARN: filename.mk:1: ${CP} should not be used to install files.")
}
-func (s *Suite) Test_splitIntoMkWords(c *check.C) {
- url := "http://registry.gimp.org/file/fix-ca.c?action=download&id=9884&file=";
-
- words, rest := splitIntoShellTokens(dummyLine, url) // Doesn't really make sense
-
- c.Check(words, check.DeepEquals, []string{
- "http://registry.gimp.org/file/fix-ca.c?action=download";,
- "&",
- "id=9884",
- "&",
- "file="})
- c.Check(rest, equals, "")
-
- words, rest = splitIntoMkWords(dummyLine, url)
-
- c.Check(words, check.DeepEquals, []string{
- "http://registry.gimp.org/file/fix-ca.c?action=download&id=9884&file="})
- c.Check(rest, equals, "")
-
- words, rest = splitIntoMkWords(dummyLine, "a b \"c c c\" d;;d;; \"e\"''`` 'rest")
-
- c.Check(words, check.DeepEquals, []string{"a", "b", "\"c c c\"", "d;;d;;", "\"e\"''``"})
- c.Check(rest, equals, "'rest")
-}
-
func (s *Suite) Test_ShellLineChecker_CheckShellCommandLine__sed_and_mv(c *check.C) {
t := s.Init(c)
t.SetUpVartypes()
t.SetUpTool("sed", "SED", AtRunTime)
t.SetUpTool("mv", "MV", AtRunTime)
- ck := t.NewShellLineChecker("Makefile", 85, "\t${RUN} ${SED} 's,#,// comment:,g' filename > filename.tmp; ${MV} filename.tmp filename")
+ ck := t.NewShellLineChecker(nil, "Makefile", 85, "\t${RUN} ${SED} 's,#,// comment:,g' filename > filename.tmp; ${MV} filename.tmp filename")
ck.CheckShellCommandLine(ck.mkline.ShellCommand())
@@ -782,7 +741,7 @@ func (s *Suite) Test_ShellLineChecker_Ch
func (s *Suite) Test_ShellLineChecker_CheckShellCommandLine__subshell(c *check.C) {
t := s.Init(c)
- ck := t.NewShellLineChecker("Makefile", 85, "\t${RUN} uname=$$(uname)")
+ ck := t.NewShellLineChecker(nil, "Makefile", 85, "\t${RUN} uname=$$(uname)")
ck.CheckShellCommandLine(ck.mkline.ShellCommand())
@@ -794,7 +753,7 @@ func (s *Suite) Test_ShellLineChecker_Ch
t := s.Init(c)
t.SetUpVartypes()
- ck := t.NewShellLineChecker("Makefile", 85, "\t${RUN} ${INSTALL_DATA_DIR} ${DESTDIR}${PREFIX}/dir1 ${DESTDIR}${PREFIX}/dir2")
+ ck := t.NewShellLineChecker(nil, "Makefile", 85, "\t${RUN} ${INSTALL_DATA_DIR} ${DESTDIR}${PREFIX}/dir1 ${DESTDIR}${PREFIX}/dir2")
ck.CheckShellCommandLine(ck.mkline.ShellCommand())
@@ -821,7 +780,7 @@ func (s *Suite) Test_ShellLineChecker_Ch
t := s.Init(c)
t.SetUpVartypes()
- ck := t.NewShellLineChecker("Makefile", 85, "\t${RUN} ${INSTALL} -d ${DESTDIR}${PREFIX}/dir1 ${DESTDIR}${PREFIX}/dir2")
+ ck := t.NewShellLineChecker(nil, "Makefile", 85, "\t${RUN} ${INSTALL} -d ${DESTDIR}${PREFIX}/dir1 ${DESTDIR}${PREFIX}/dir2")
ck.CheckShellCommandLine(ck.mkline.ShellCommand())
@@ -852,7 +811,7 @@ func (s *Suite) Test_ShellLineChecker_ch
t.SetUpTool("grep", "GREP", AtRunTime)
test := func(lineno int, input string) {
- ck := t.NewShellLineChecker("module.mk", lineno, "\t"+input)
+ ck := t.NewShellLineChecker(nil, "module.mk", lineno, "\t"+input)
ck.checkWordQuoting(ck.mkline.ShellCommand(), true, RunTime)
}
@@ -889,7 +848,7 @@ func (s *Suite) Test_ShellLineChecker_un
t := s.Init(c)
test := func(lineno int, input string, expectedOutput string, expectedRest string) {
- ck := t.NewShellLineChecker("dummy.mk", lineno, "# dummy")
+ ck := t.NewShellLineChecker(nil, "dummy.mk", lineno, "# dummy")
tok := NewShTokenizer(nil, input, false)
atoms := tok.ShAtoms()
@@ -958,7 +917,7 @@ func (s *Suite) Test_ShellLineChecker_un
t.SetUpTool("echo", "", AtRunTime)
mkline := t.NewMkLine("dummy.mk", 13, "\t var=\"`echo \"\"`\"")
- MkLineChecker{mkline}.Check()
+ MkLineChecker{nil, mkline}.Check()
t.CheckOutputLines(
"WARN: dummy.mk:13: Double quotes inside backticks inside double quotes are error prone.")
@@ -1143,7 +1102,7 @@ func (s *Suite) Test_SimpleCommandChecke
mkline := t.NewMkLine("file.mk", 3, "\t# comment; continuation")
- MkLineChecker{mkline}.Check()
+ MkLineChecker{nil, mkline}.Check()
t.CheckOutputLines(
"WARN: file.mk:3: A shell comment should not contain semicolons.")
Index: pkgsrc/pkgtools/pkglint/files/testnames_test.go
diff -u pkgsrc/pkgtools/pkglint/files/testnames_test.go:1.3 pkgsrc/pkgtools/pkglint/files/testnames_test.go:1.4
--- pkgsrc/pkgtools/pkglint/files/testnames_test.go:1.3 Mon Dec 17 00:15:39 2018
+++ pkgsrc/pkgtools/pkglint/files/testnames_test.go Wed Apr 3 21:49:51 2019
@@ -14,6 +14,7 @@ func (s *Suite) Test__test_names(c *chec
ck.AllowPrefix("Varalign", "mklines_varalign.go")
ck.AllowPrefix("ShellParser", "mkshparser.go")
ck.AllowCamelCaseDescriptions(
+ "compared_to_splitIntoShellTokens",
"comparing_YesNo_variable_to_string",
"enumFrom",
"enumFromDirs",
Index: pkgsrc/pkgtools/pkglint/files/util.go
diff -u pkgsrc/pkgtools/pkglint/files/util.go:1.40 pkgsrc/pkgtools/pkglint/files/util.go:1.41
--- pkgsrc/pkgtools/pkglint/files/util.go:1.40 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/util.go Wed Apr 3 21:49:51 2019
@@ -290,30 +290,6 @@ func varnameParam(varname string) string
return ""
}
-// defineVar marks a variable as defined in both the current package and the current file.
-func defineVar(pkg *Package, mklines MkLines, mkline MkLine, varname string) {
- if mklines != nil {
- mklines.vars.Define(varname, mkline)
- }
- if pkg != nil {
- pkg.vars.Define(varname, mkline)
- }
-}
-
-// varIsDefinedSimilar tests whether the variable (or its canonicalized form)
-// is defined in the current package or in the current file.
-func varIsDefinedSimilar(pkg *Package, mklines MkLines, varname string) bool {
- return mklines != nil && (mklines.vars.DefinedSimilar(varname) || mklines.forVars[varname]) ||
- pkg != nil && pkg.vars.DefinedSimilar(varname)
-}
-
-// varIsUsedSimilar tests whether the variable (or its canonicalized form)
-// is used in the current package or in the current file.
-func varIsUsedSimilar(pkg *Package, mklines MkLines, varname string) bool {
- return mklines != nil && mklines.vars.UsedSimilar(varname) ||
- pkg != nil && pkg.vars.UsedSimilar(varname)
-}
-
func fileExists(filename string) bool {
st, err := os.Stat(filename)
return err == nil && st.Mode().IsRegular()
@@ -507,11 +483,12 @@ func (o *Once) check(key uint64) bool {
// TODO: Merge this code with Var, which defines essentially the
// same features.
type Scope struct {
- firstDef map[string]MkLine // TODO: Can this be removed?
- lastDef map[string]MkLine
- value map[string]string
- used map[string]MkLine
- fallback map[string]string
+ firstDef map[string]MkLine // TODO: Can this be removed?
+ lastDef map[string]MkLine
+ value map[string]string
+ used map[string]MkLine
+ usedAtLoadTime map[string]bool
+ fallback map[string]string
}
func NewScope() Scope {
@@ -520,6 +497,7 @@ func NewScope() Scope {
make(map[string]MkLine),
make(map[string]string),
make(map[string]MkLine),
+ make(map[string]bool),
make(map[string]string)}
}
@@ -565,21 +543,21 @@ func (s *Scope) Fallback(varname string,
}
// Use marks the variable and its canonicalized form as used.
-func (s *Scope) Use(varname string, line MkLine) {
- if s.used[varname] == nil {
- s.used[varname] = line
- if trace.Tracing {
- trace.Step2("Using %q in %s", varname, line.String())
+func (s *Scope) Use(varname string, line MkLine, time vucTime) {
+ use := func(name string) {
+ if s.used[name] == nil {
+ s.used[name] = line
+ if trace.Tracing {
+ trace.Step2("Using %q in %s", name, line.String())
+ }
}
- }
-
- varcanon := varnameCanon(varname)
- if varcanon != varname && s.used[varcanon] == nil {
- s.used[varcanon] = line
- if trace.Tracing {
- trace.Step2("Using %q in %s", varcanon, line.String())
+ if time == vucTimeParse {
+ s.usedAtLoadTime[name] = true
}
}
+
+ use(varname)
+ use(varnameCanon(varname))
}
// Defined tests whether the variable is defined.
@@ -624,6 +602,12 @@ func (s *Scope) UsedSimilar(varname stri
return s.used[varnameCanon(varname)] != nil
}
+// UsedAtLoadTime returns true if the variable is used at load time
+// somewhere.
+func (s *Scope) UsedAtLoadTime(varname string) bool {
+ return s.usedAtLoadTime[varname]
+}
+
// FirstDefinition returns the line in which the variable has been defined first.
//
// Having multiple definitions is typical in the branches of "if" statements.
Index: pkgsrc/pkgtools/pkglint/files/vardefs.go
diff -u pkgsrc/pkgtools/pkglint/files/vardefs.go:1.57 pkgsrc/pkgtools/pkglint/files/vardefs.go:1.58
--- pkgsrc/pkgtools/pkglint/files/vardefs.go:1.57 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/vardefs.go Wed Apr 3 21:49:51 2019
@@ -1007,7 +1007,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
sys("FAM_TYPE", enum("fam gamin"))
pkglist("FETCH_BEFORE_ARGS", BtShellWord)
pkglist("FETCH_MESSAGE", BtShellWord)
- pkg("FILESDIR", BtRelativePkgPath)
+ pkgload("FILESDIR", BtRelativePkgPath)
pkglist("FILES_SUBST", BtShellWord)
syslist("FILES_SUBST_SED", BtShellWord)
pkglist("FIX_RPATH", BtVariableName)
@@ -1220,7 +1220,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
pkglist("OWN_DIRS_PERMS", BtPerms)
sys("PAMBASE", BtPathname)
usr("PAM_DEFAULT", enum("linux-pam openpam solaris-pam"))
- pkg("PATCHDIR", BtRelativePkgPath)
+ pkgload("PATCHDIR", BtRelativePkgPath)
pkglist("PATCHFILES", BtFileName)
pkglist("PATCH_ARGS", BtShellWord)
pkglist("PATCH_DIST_ARGS", BtShellWord)
Index: pkgsrc/pkgtools/pkglint/files/vardefs_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vardefs_test.go:1.11 pkgsrc/pkgtools/pkglint/files/vardefs_test.go:1.12
--- pkgsrc/pkgtools/pkglint/files/vardefs_test.go:1.11 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/vardefs_test.go Wed Apr 3 21:49:51 2019
@@ -6,7 +6,7 @@ func (s *Suite) Test_VarTypeRegistry_Ini
t := s.Init(c)
src := NewPkgsrc(t.File("."))
- src.vartypes.Init(src)
+ src.vartypes.Init(&src)
c.Check(src.vartypes.Canon("BSD_MAKE_ENV").basicType.name, equals, "ShellWord")
c.Check(src.vartypes.Canon("USE_BUILTIN.*").basicType.name, equals, "YesNoIndirectly")
@@ -48,7 +48,7 @@ func (s *Suite) Test_VarTypeRegistry_Ini
t.SetUpVartypes()
test := func(varname, values string) {
- vartype := G.Pkgsrc.VariableType(varname).String()
+ vartype := G.Pkgsrc.VariableType(nil, varname).String()
c.Check(vartype, equals, values)
}
@@ -70,7 +70,7 @@ func (s *Suite) Test_VarTypeRegistry_Ini
t.SetUpVartypes()
test := func(varname, values string) {
- vartype := G.Pkgsrc.VariableType(varname).String()
+ vartype := G.Pkgsrc.VariableType(nil, varname).String()
c.Check(vartype, equals, values)
}
Index: pkgsrc/pkgtools/pkglint/files/vartype_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartype_test.go:1.16 pkgsrc/pkgtools/pkglint/files/vartype_test.go:1.17
--- pkgsrc/pkgtools/pkglint/files/vartype_test.go:1.16 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/vartype_test.go Wed Apr 3 21:49:51 2019
@@ -156,8 +156,8 @@ func (s *Suite) Test_Vartype_IsConsidere
t.SetUpVartypes()
- c.Check(G.Pkgsrc.VariableType("COMMENT").IsConsideredList(), equals, false)
- c.Check(G.Pkgsrc.VariableType("DEPENDS").IsConsideredList(), equals, true)
- c.Check(G.Pkgsrc.VariableType("PKG_FAIL_REASON").IsConsideredList(), equals, true)
- c.Check(G.Pkgsrc.VariableType("CONF_FILES").IsConsideredList(), equals, true)
+ c.Check(G.Pkgsrc.VariableType(nil, "COMMENT").IsConsideredList(), equals, false)
+ c.Check(G.Pkgsrc.VariableType(nil, "DEPENDS").IsConsideredList(), equals, true)
+ c.Check(G.Pkgsrc.VariableType(nil, "PKG_FAIL_REASON").IsConsideredList(), equals, true)
+ c.Check(G.Pkgsrc.VariableType(nil, "CONF_FILES").IsConsideredList(), equals, true)
}
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.52 pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.53
--- pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.52 Sun Mar 24 13:58:38 2019
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck.go Wed Apr 3 21:49:51 2019
@@ -8,6 +8,8 @@ import (
// VartypeCheck groups together the various checks for variables of the different types.
type VartypeCheck struct {
+ MkLines MkLines
+
// Note: if "go vet" or "go test" complains about a "variable with invalid type", update to go1.11.4.
// See https://github.com/golang/go/issues/28972.
// That doesn't help though since pkglint contains these "more convoluted alias declarations"
@@ -274,7 +276,7 @@ func (cv *VartypeCheck) Comment() {
// When a package is installed, the example file is installed as usual
// and is then copied to its final location.
func (cv *VartypeCheck) ConfFiles() {
- words, _ := splitIntoMkWords(cv.MkLine.Line, cv.Value)
+ words := cv.MkLine.ValueFields(cv.Value)
if len(words)%2 != 0 {
cv.Warnf("Values for %s should always be pairs of paths.", cv.Varname)
}
@@ -298,7 +300,9 @@ func (cv *VartypeCheck) Dependency() {
parser := NewMkParser(nil, value, false)
deppat := parser.Dependency()
- if deppat != nil && deppat.Wildcard == "" && (parser.Rest() == "{,nb*}" || parser.Rest() == "{,nb[0-9]*}") {
+ rest := parser.Rest()
+
+ if deppat != nil && deppat.Wildcard == "" && (rest == "{,nb*}" || rest == "{,nb[0-9]*}") {
cv.Warnf("Dependency patterns of the form pkgbase>=1.0 don't need the \"{,nb*}\" extension.")
G.Explain(
"The \"{,nb*}\" extension is only necessary for dependencies of the",
@@ -307,12 +311,12 @@ func (cv *VartypeCheck) Dependency() {
"For dependency patterns using the comparison operators,",
"this is not necessary.")
- } else if deppat == nil && contains(value, "{") {
+ } else if deppat == nil && contains(cv.ValueNoVar, "{") {
// Don't warn about complicated patterns like "{ssh{,6}>=0,openssh>=0}"
// that pkglint doesn't understand as of January 2019.
return
- } else if deppat == nil || !parser.EOF() {
+ } else if deppat == nil || rest != "" {
cv.Warnf("Invalid dependency pattern %q.", value)
G.Explain(
"Typical dependencies have the following forms:",
@@ -377,12 +381,27 @@ func (cv *VartypeCheck) Dependency() {
func (cv *VartypeCheck) DependencyWithPath() {
value := cv.Value
- if value != cv.ValueNoVar {
- return // It's probably not worth checking this.
+ valueNoVar := cv.ValueNoVar
+
+ if valueNoVar == "" || valueNoVar == ":" {
+ return
}
- if m, pattern, relpath, pkg := match3(value, `(.*):(\.\./\.\./[^/]+/([^/]+))$`); m {
- MkLineChecker{cv.MkLine}.CheckRelativePkgdir(relpath)
+ parts := cv.MkLine.ValueSplit(value, ":")
+ if len(parts) == 2 {
+ pattern := parts[0]
+ relpath := parts[1]
+ pathParts := cv.MkLine.ValueSplit(relpath, "/")
+ pkg := pathParts[len(pathParts)-1]
+
+ if matches(relpath, `^\.\./[^.]`) {
+ cv.Warnf("Dependency paths should have the form \"../../category/package\".")
+ cv.MkLine.ExplainRelativeDirs()
+ }
+
+ if !containsVarRef(relpath) {
+ MkLineChecker{cv.MkLines, cv.MkLine}.CheckRelativePkgdir(relpath)
+ }
switch pkg {
case "gettext":
@@ -394,12 +413,7 @@ func (cv *VartypeCheck) DependencyWithPa
}
cv.WithValue(pattern).Dependency()
- return
- }
- if matches(value, `:\.\./[^/]+$`) {
- cv.Warnf("Dependencies should have the form \"../../category/package\".")
- cv.MkLine.ExplainRelativeDirs()
return
}
@@ -660,7 +674,7 @@ func (cv *VartypeCheck) LdFlag() {
}
func (cv *VartypeCheck) License() {
- (&LicenseChecker{cv.MkLine}).Check(cv.Value, cv.Op)
+ (&LicenseChecker{cv.MkLines, cv.MkLine}).Check(cv.Value, cv.Op)
}
func (cv *VartypeCheck) MachineGnuPlatform() {
@@ -748,7 +762,7 @@ func (cv *VartypeCheck) Option() {
}
if m, optname := match1(value, `^-?([a-z][-0-9a-z+]*)$`); m {
- if G.Mk != nil && !G.Mk.FirstTimeSlice("option:", optname) {
+ if cv.MkLines != nil && !cv.MkLines.FirstTimeSlice("option:", optname) {
return
}
@@ -879,7 +893,7 @@ func (cv *VartypeCheck) PkgOptionsVar()
// 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.MkLine}.CheckRelativePkgdir(pkgsrcdir + "/" + cv.Value)
+ MkLineChecker{cv.MkLines, cv.MkLine}.CheckRelativePkgdir(pkgsrcdir + "/" + cv.Value)
}
func (cv *VartypeCheck) PkgRevision() {
@@ -964,7 +978,7 @@ func (cv *VartypeCheck) PythonDependency
// RelativePkgDir refers to a package directory, e.g. ../../category/pkgbase.
func (cv *VartypeCheck) RelativePkgDir() {
- MkLineChecker{cv.MkLine}.CheckRelativePkgdir(cv.Value)
+ MkLineChecker{cv.MkLines, cv.MkLine}.CheckRelativePkgdir(cv.Value)
}
// RelativePkgPath refers to a file or directory, e.g. ../../category/pkgbase,
@@ -972,7 +986,7 @@ func (cv *VartypeCheck) RelativePkgDir()
//
// See RelativePkgDir, which requires a directory, not a file.
func (cv *VartypeCheck) RelativePkgPath() {
- MkLineChecker{cv.MkLine}.CheckRelativePath(cv.Value, true)
+ MkLineChecker{cv.MkLines, cv.MkLine}.CheckRelativePath(cv.Value, true)
}
func (cv *VartypeCheck) Restricted() {
@@ -1045,16 +1059,16 @@ func (cv *VartypeCheck) ShellCommand() {
return
}
setE := true
- NewShellLineChecker(cv.MkLine).CheckShellCommand(cv.Value, &setE, RunTime)
+ NewShellLineChecker(cv.MkLines, cv.MkLine).CheckShellCommand(cv.Value, &setE, RunTime)
}
// ShellCommands checks for zero or more shell commands, each terminated with a semicolon.
func (cv *VartypeCheck) ShellCommands() {
- NewShellLineChecker(cv.MkLine).CheckShellCommands(cv.Value, RunTime)
+ NewShellLineChecker(cv.MkLines, cv.MkLine).CheckShellCommands(cv.Value, RunTime)
}
func (cv *VartypeCheck) ShellWord() {
- NewShellLineChecker(cv.MkLine).CheckWord(cv.Value, true, RunTime)
+ NewShellLineChecker(cv.MkLines, cv.MkLine).CheckWord(cv.Value, true, RunTime)
}
func (cv *VartypeCheck) Stage() {
@@ -1071,7 +1085,7 @@ func (cv *VartypeCheck) Tool() {
// no warning for package-defined tool definitions
} else if m, toolname, tooldep := match2(cv.Value, `^([-\w]+|\[)(?::(\w+))?$`); m {
- if tool, _ := G.Tool(toolname, RunTime); tool == nil {
+ if tool, _ := G.Tool(cv.MkLines, toolname, RunTime); tool == nil {
cv.Errorf("Unknown tool %q.", toolname)
}
Home |
Main Index |
Thread Index |
Old Index