pkgtools/pkglint: update to 19.4.10

Changes since 19.4.9:

In continuation lines with long values, pkglint no longer suggests to
move the continuation backslash in the middle of the variable value, as
that would be impossible.

Warn when a shell command is assigned to a variable that only takes
pathnames. Shell commands can contain command line options, and these
are not pathnames.

The TOOLS_PLATFORM.tool variables are not defined on every platform.
When these variables are used outside an OPSYS check, a warning lists
the platforms where the tool is undefined or only defined conditionally.
This commit is contained in:
rillig 2020-03-07 23:35:35 +00:00
parent 50b9bd6d40
commit f9486e2bfe
22 changed files with 480 additions and 142 deletions

View file

@ -1,6 +1,6 @@
# $NetBSD: Makefile,v 1.632 2020/02/17 20:22:21 rillig Exp $
# $NetBSD: Makefile,v 1.633 2020/03/07 23:35:35 rillig Exp $
PKGNAME= pkglint-19.4.9
PKGNAME= pkglint-19.4.10
CATEGORIES= pkgtools
DISTNAME= tools
MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/}

View file

@ -64,8 +64,8 @@ func (s *Suite) Test_HomepageChecker_checkBasedOnMasterSites(c *check.C) {
vt.Output(
"WARN: filename.mk:11: HOMEPAGE should not be defined in terms of MASTER_SITEs.")
delete(pkg.vars.firstDef, "MASTER_SITES")
delete(pkg.vars.lastDef, "MASTER_SITES")
pkg.vars.v("MASTER_SITES").firstDef = nil
pkg.vars.v("MASTER_SITES").lastDef = nil
pkg.vars.Define("MASTER_SITES", t.NewMkLine(pkg.File("Makefile"), 5,
"MASTER_SITES=\thttps://cdn.NetBSD.org/pub/pkgsrc/distfiles/"))
@ -76,8 +76,8 @@ func (s *Suite) Test_HomepageChecker_checkBasedOnMasterSites(c *check.C) {
"WARN: filename.mk:21: HOMEPAGE should not be defined in terms of MASTER_SITEs. " +
"Use https://cdn.NetBSD.org/pub/pkgsrc/distfiles/ directly.")
delete(pkg.vars.firstDef, "MASTER_SITES")
delete(pkg.vars.lastDef, "MASTER_SITES")
pkg.vars.v("MASTER_SITES").firstDef = nil
pkg.vars.v("MASTER_SITES").lastDef = nil
pkg.vars.Define("MASTER_SITES", t.NewMkLine(pkg.File("Makefile"), 5,
"MASTER_SITES=\t${MASTER_SITE_GITHUB}"))
@ -89,8 +89,8 @@ func (s *Suite) Test_HomepageChecker_checkBasedOnMasterSites(c *check.C) {
vt.Output(
"WARN: filename.mk:31: HOMEPAGE should not be defined in terms of MASTER_SITEs.")
delete(pkg.vars.firstDef, "MASTER_SITES")
delete(pkg.vars.lastDef, "MASTER_SITES")
pkg.vars.v("MASTER_SITES").firstDef = nil
pkg.vars.v("MASTER_SITES").lastDef = nil
pkg.vars.Define("MASTER_SITES", t.NewMkLine(pkg.File("Makefile"), 5,
"MASTER_SITES=\t# none"))
@ -393,7 +393,7 @@ func (s *Suite) Test_HomepageChecker_checkReachable(c *check.C) {
"https://no-such-name.example.org/",
// The "unknown network error" is for compatibility with Go < 1.13.
`^WARN: filename\.mk:1: Homepage "https://no-such-name.example.org/" `+
`cannot be checked: (name not found|unknown network error:.*)$`)
`cannot be checked: (name not found|timeout|unknown network error:.*)$`)
// Syntactically invalid URLs are silently skipped since VartypeCheck.URL
// already warns about them.

View file

@ -368,7 +368,7 @@ func (l *Logger) ShowSummary(args []string) {
}
l.out.Write(sprintf("%s found.\n",
joinSkipEmptyCambridge("and",
joinCambridge("and",
num(l.errors, "error", "errors"),
num(l.warnings, "warning", "warnings"),
num(l.notes, "note", "notes"))))

View file

@ -29,6 +29,7 @@ func (ck *MkCondChecker) Check() {
checkVarUse := func(varuse *MkVarUse) {
var vartype *Vartype // TODO: Insert a better type guess here.
// See Test_MkVarUseChecker_checkAssignable__shell_command_in_exists.
vuc := VarUseContext{vartype, VucLoadTime, VucQuotPlain, false}
NewMkVarUseChecker(varuse, ck.MkLines, mkline).Check(&vuc)
}

View file

@ -1034,6 +1034,10 @@ func (s *Suite) Test_MkLine_VariableNeedsQuoting__shellword_part(c *check.C) {
}
// Tools, when used in a shell command, must not be quoted.
// Shell commands may have command line arguments, pathnames must not.
// The original intention of having both CONFIG_SHELL and CONFIG_SHELL_FLAGS
// was to separate the command from its arguments.
// It doesn't hurt though if the command includes some of the arguments as well.
func (s *Suite) Test_MkLine_VariableNeedsQuoting__tool_in_shell_command(c *check.C) {
t := s.Init(c)
@ -1043,11 +1047,14 @@ func (s *Suite) Test_MkLine_VariableNeedsQuoting__tool_in_shell_command(c *check
mklines := t.SetUpFileMkLines("Makefile",
MkCvsID,
"",
"CONFIG_SHELL=\t${BASH}")
"CONFIG_SHELL=\t${BASH}",
"DIST_SUBDIR=\t${BASH}")
mklines.Check()
t.CheckOutputEmpty()
t.CheckOutputLines(
"WARN: ~/Makefile:4: Incompatible types: " +
"BASH (type \"ShellCommand\") cannot be assigned to type \"Pathname\".")
}
// This test provides code coverage for the "switch vuc.quoting" in the case

View file

@ -194,9 +194,9 @@ func (mklines *MkLines) collectDocumentedVariables() {
// The commentLines include the the line containing the variable name,
// leaving 2 of these 3 lines for the actual documentation.
if commentLines >= 3 && relevant {
forEachStringMkLine(scope.used, func(varname string, mkline *MkLine) {
mklines.allVars.Define(varname, mkline)
mklines.allVars.Use(varname, mkline, VucRunTime)
scope.forEach(func(varname string, data *scopeVar) {
mklines.allVars.Define(varname, data.used)
mklines.allVars.Use(varname, data.used, VucRunTime)
})
}

View file

@ -494,7 +494,8 @@ func (s *Suite) Test_MkLines_collectUsedVariables__simple(c *check.C) {
mklines.collectUsedVariables()
t.CheckDeepEquals(mklines.allVars.used, map[string]*MkLine{"VAR": mkline})
t.Check(mklines.allVars.vs, check.HasLen, 1)
t.CheckEquals(mklines.allVars.v("VAR").used, mkline)
t.CheckEquals(mklines.allVars.FirstUse("VAR"), mkline)
}
@ -513,7 +514,7 @@ func (s *Suite) Test_MkLines_collectUsedVariables__nested(c *check.C) {
mklines.collectUsedVariables()
t.CheckEquals(len(mklines.allVars.used), 5)
t.CheckEquals(len(mklines.allVars.vs), 5)
t.CheckEquals(mklines.allVars.FirstUse("lparam"), assignMkline)
t.CheckEquals(mklines.allVars.FirstUse("rparam"), assignMkline)
t.CheckEquals(mklines.allVars.FirstUse("inner"), shellMkline)
@ -570,9 +571,11 @@ func (s *Suite) Test_MkLines_collectDocumentedVariables(c *check.C) {
mklines.collectDocumentedVariables()
var varnames []string
for varname, mkline := range mklines.allVars.used {
varnames = append(varnames, sprintf("%s (line %s)", varname, mkline.Linenos()))
}
mklines.allVars.forEach(func(varname string, data *scopeVar) {
if data.used != nil {
varnames = append(varnames, sprintf("%s (line %s)", varname, data.used.Linenos()))
}
})
sort.Strings(varnames)
expected := []string{
@ -694,7 +697,7 @@ func (s *Suite) Test_MkLines_collectVariables__find_files_and_headers(c *check.C
mklines.Check()
t.CheckDeepEquals(
keys(mklines.allVars.firstDef),
keys(mklines.allVars.vs),
[]string{
"BUILTIN_FIND_FILES_VAR",
"BUILTIN_FIND_HEADERS_VAR",

View file

@ -27,8 +27,10 @@ func (ck *MkVarUseChecker) Check(vuc *VarUseContext) {
ck.checkVarname(vuc.time)
ck.checkModifiers()
ck.checkAssignable(vuc)
ck.checkQuoting(vuc)
ck.checkToolsPlatform()
ck.checkBuildDefs()
ck.checkDeprecated()
@ -445,6 +447,35 @@ func (ck *MkVarUseChecker) warnToolLoadTime(varname string, tool *Tool) {
"except in the package Makefile itself.")
}
func (ck *MkVarUseChecker) checkAssignable(vuc *VarUseContext) {
leftType := vuc.vartype
if leftType == nil || leftType.basicType != BtPathname {
return
}
rightType := G.Pkgsrc.VariableType(ck.MkLines, ck.use.varname)
if rightType == nil || rightType.basicType != BtShellCommand {
return
}
mkline := ck.MkLine
if mkline.Varcanon() == "PKG_SHELL.*" {
switch ck.use.varname {
case "SH", "BASH", "TOOLS_PLATFORM.sh":
return
}
}
mkline.Warnf(
"Incompatible types: %s (type %q) cannot be assigned to type %q.",
ck.use.varname, rightType.basicType.name, leftType.basicType.name)
mkline.Explain(
"Shell commands often start with a pathname.",
"They could also start with a list of environment variable",
"definitions, since that is accepted by the shell.",
"They can also contain addition command line arguments",
"that are not filenames at all.")
}
// checkVarUseWords checks whether a variable use of the form ${VAR}
// or ${VAR:modifiers} is allowed in a certain context.
func (ck *MkVarUseChecker) checkQuoting(vuc *VarUseContext) {
@ -638,6 +669,40 @@ func (ck *MkVarUseChecker) warnRedundantModifierQ(mod string) {
fix.Apply()
}
func (ck *MkVarUseChecker) checkToolsPlatform() {
if ck.MkLine.IsDirective() {
return
}
varname := ck.use.varname
if varnameCanon(varname) != "TOOLS_PLATFORM.*" {
return
}
indentation := ck.MkLines.indentation
switch {
case indentation.DependsOn("OPSYS"),
indentation.DependsOn("MACHINE_PLATFORM"),
indentation.DependsOn(varname):
// TODO: Only return if the conditional is on the correct OPSYS.
return
}
toolName := varnameParam(varname)
tool := G.Pkgsrc.Tools.ByName(toolName)
if tool == nil {
return
}
if len(tool.undefinedOn) > 0 {
ck.MkLine.Warnf("%s is undefined on %s.",
varname, joinCambridge("and", tool.undefinedOn...))
} else if len(tool.conditionalOn) > 0 {
ck.MkLine.Warnf("%s may be undefined on %s.",
varname, joinCambridge("and", tool.conditionalOn...))
}
}
func (ck *MkVarUseChecker) checkBuildDefs() {
varname := ck.use.varname

View file

@ -957,6 +957,77 @@ func (s *Suite) Test_MkVarUseChecker_warnToolLoadTime__local_tool(c *check.C) {
"WARN: ~/category/package/Makefile:7: The tool ${MK_TOOL} cannot be used at load time.")
}
func (s *Suite) Test_MkVarUseChecker_checkAssignable(c *check.C) {
t := s.Init(c)
t.SetUpVartypes()
mklines := t.NewMkLines("filename.mk",
"BUILTIN_FIND_FILES_VAR:=\tBIN_FILE",
"BUILTIN_FIND_FILES.BIN_FILE=\t${TOOLS_PLATFORM.file} /bin/file /usr/bin/file")
mklines.ForEach(func(mkline *MkLine) {
ck := NewMkAssignChecker(mkline, mklines)
ck.checkVarassignRight()
})
t.CheckOutputLines(
"WARN: filename.mk:2: Incompatible types: " +
"TOOLS_PLATFORM.file (type \"ShellCommand\") " +
"cannot be assigned to type \"Pathname\".")
}
// NetBSD's chsh program only allows a simple pathname for the shell, without
// any command line arguments. This makes sense since the shell is started
// using execve, not system (which would require shell-like argument parsing).
//
// Under the assumption that TOOLS_PLATFORM.sh does not contain any command
// line arguments, it's ok in that special case. This covers most of the
// real-life situations where this type mismatch (Pathname := ShellCommand)
// occurs.
func (s *Suite) Test_MkVarUseChecker_checkAssignable__shell_command_to_pathname(c *check.C) {
t := s.Init(c)
t.SetUpVartypes()
t.SetUpTool("sh", "SH", AtRunTime)
t.SetUpTool("bash", "BASH", AtRunTime)
mklines := t.NewMkLines("filename.mk",
"PKG_SHELL.user=\t${TOOLS_PLATFORM.sh}",
"PKG_SHELL.user=\t${SH}",
"PKG_SHELL.user=\t${BASH}")
mklines.ForEach(func(mkline *MkLine) {
ck := NewMkAssignChecker(mkline, mklines)
ck.checkVarassignRight()
})
t.CheckOutputLines(
"WARN: filename.mk:1: Please use ${TOOLS_PLATFORM.sh:Q} " +
"instead of ${TOOLS_PLATFORM.sh}.")
}
func (s *Suite) Test_MkVarUseChecker_checkAssignable__shell_command_in_exists(c *check.C) {
t := s.Init(c)
t.SetUpTool("sh", "SH", AfterPrefsMk)
t.SetUpTool("bash", "BASH", AfterPrefsMk)
t.SetUpPkgsrc()
t.Chdir(".")
t.FinishSetUp()
mklines := t.NewMkLines("filename.mk",
MkCvsID,
".include \"mk/bsd.prefs.mk\"",
".if exists(${TOOLS_PLATFORM.sh})",
".elif exists(${SH})",
".elif exists(${BASH})",
".endif")
mklines.Check()
// TODO: Call MkVarUseChecker.checkAssignable with a VarUseContext of type
// BtPathname here.
t.CheckOutputEmpty()
}
func (s *Suite) Test_MkVarUseChecker_checkQuoting(c *check.C) {
t := s.Init(c)
@ -1153,6 +1224,65 @@ func (s *Suite) Test_MkVarUseChecker_fixQuotingModifiers(c *check.C) {
"AUTOFIX: ~/filename.mk:5: Replacing \"${CFLAGS:N*:Q}\" with \"${CFLAGS:N*:M*:Q}\".")
}
func (s *Suite) Test_MkVarUseChecker_checkToolsPlatform(c *check.C) {
t := s.Init(c)
t.SetUpPkgsrc()
t.SetUpTool("available", "", AfterPrefsMk)
t.SetUpTool("cond1", "", AfterPrefsMk)
t.SetUpTool("cond2", "", AfterPrefsMk)
t.SetUpTool("undefined", "", AfterPrefsMk)
t.CreateFileLines("mk/tools/tools.NetBSD.mk",
"TOOLS_PLATFORM.available?=\t/bin/available",
"TOOLS_PLATFORM.cond1?=\t/usr/cond1",
"TOOLS_PLATFORM.cond2?=\t/usr/cond2",
"TOOLS_PLATFORM.undefined?=\t/usr/undefined")
t.CreateFileLines("mk/tools/tools.SunOS.mk",
"TOOLS_PLATFORM.available?=\t/bin/available",
"",
".if exists(/usr/gnu/bin/cond1)",
"TOOLS_PLATFORM.cond1?=\t/usr/gnu/bin/cond1",
".endif",
"",
".if exists(/usr/gnu/bin/cond2)",
"TOOLS_PLATFORM.cond2?=\t/usr/gnu/bin/cond2",
".else",
"TOOLS_PLATFORM.cond2?=\t/usr/sfw/bin/cond2",
".endif",
"",
"# No definition for undefined.")
t.Chdir(".")
t.FinishSetUp()
mklines := t.NewMkLines("filename.mk",
MkCvsID,
"",
".include \"mk/bsd.prefs.mk\"",
"",
".if ${OPSYS} == SunOS",
"post-build:",
"\t${TOOLS_PLATFORM.available}",
"\t${TOOLS_PLATFORM.cond1}",
"\t${TOOLS_PLATFORM.cond2}",
"\t${TOOLS_PLATFORM.undefined}",
".endif",
"",
"do-build:",
"\t${TOOLS_PLATFORM.available}",
"\t${TOOLS_PLATFORM.cond1}",
"\t${TOOLS_PLATFORM.cond2}",
"\t${TOOLS_PLATFORM.undefined}",
"",
".if defined(TOOLS_PLATFORM.undefined)",
"\t${TOOLS_PLATFORM.undefined}",
".endif")
mklines.Check()
t.CheckOutputLines(
"WARN: filename.mk:15: TOOLS_PLATFORM.cond1 may be undefined on SunOS.",
"WARN: filename.mk:17: TOOLS_PLATFORM.undefined is undefined on SunOS.")
}
func (s *Suite) Test_MkVarUseChecker_checkBuildDefs(c *check.C) {
t := s.Init(c)

View file

@ -135,7 +135,7 @@ func (pkg *Package) load() ([]CurrPath, *MkLines, *MkLines) {
files = append(files, pkg.File(pkg.Pkgdir).ReadPaths()...)
}
files = append(files, pkg.File(pkg.Patchdir).ReadPaths()...)
if pkg.DistinfoFile != NewPackagePathString(pkg.vars.fallback["DISTINFO_FILE"]) {
if pkg.DistinfoFile != NewPackagePathString(pkg.vars.v("DISTINFO_FILE").fallback) {
files = append(files, pkg.File(pkg.DistinfoFile))
}

View file

@ -384,13 +384,13 @@ func resolveVariableRefs(text string, mklines *MkLines, pkg *Package) string {
if mklines != nil {
// TODO: At load time, use mklines.loadVars instead.
if value, ok := mklines.allVars.LastValueFound(varname); ok {
if value, found, indeterminate := mklines.allVars.LastValueFound(varname); found && !indeterminate {
return value
}
}
if pkg != nil {
if value, ok := pkg.vars.LastValueFound(varname); ok {
if value, found, indeterminate := pkg.vars.LastValueFound(varname); found && !indeterminate {
return value
}
}

View file

@ -465,11 +465,84 @@ func (src *Pkgsrc) loadTools() {
})
}
src.loadToolsPlatform()
if trace.Tracing {
tools.Trace()
}
}
func (src *Pkgsrc) loadToolsPlatform() {
var systems []string
scopes := make(map[string]*RedundantScope)
for _, mkFile := range src.File("mk/tools").ReadPaths() {
m, opsys := match1(mkFile.Base(), `^tools\.(.+)\.mk$`)
if !m {
continue
}
systems = append(systems, opsys)
mklines := LoadMk(mkFile, nil, MustSucceed)
scope := NewRedundantScope()
// Suppress any warnings, just compute the variable state.
scope.IsRelevant = func(*MkLine) bool { return false }
scope.Check(mklines)
scopes[opsys] = scope
}
// 0 = undefined, 1 = conditional, 2 = definitely assigned
type status int
statusByNameAndOpsys := make(map[string]map[string]status)
for opsys, scope := range scopes {
for varname, varinfo := range scope.vars {
if varnameCanon(varname) == "TOOLS_PLATFORM.*" {
var s status
if varinfo.vari.IsConditional() {
if len(varinfo.vari.WriteLocations()) == 1 {
s = 1
} else {
// TODO: Don't just count the number of assignments,
// check whether they definitely assign the variable.
// See substScope.
s = 2
}
} else if varinfo.vari.IsConstant() {
s = 2
} else {
continue
}
name := varnameParam(varname)
if statusByNameAndOpsys[name] == nil {
statusByNameAndOpsys[name] = make(map[string]status)
}
statusByNameAndOpsys[name][opsys] = s
}
}
}
for name, tool := range src.Tools.byName {
undefined := make(map[string]bool)
conditional := make(map[string]bool)
for _, opsys := range systems {
undefined[opsys] = true
conditional[opsys] = true
}
for opsys, status := range statusByNameAndOpsys[name] {
switch status {
case 1:
delete(undefined, opsys)
case 2:
delete(undefined, opsys)
delete(conditional, opsys)
}
}
tool.undefinedOn = keysSorted(undefined)
tool.conditionalOn = keysSorted(conditional)
}
}
func (src *Pkgsrc) addBuildDefs(varnames ...string) {
for _, varname := range varnames {
src.buildDefs[varname] = true
@ -680,11 +753,16 @@ func (src *Pkgsrc) loadUntypedVars() {
mklines := LoadMk(path, nil, MustSucceed)
mklines.collectVariables()
mklines.collectUsedVariables()
def := func(varname string, mkline *MkLine) {
define(varnameCanon(varname), mkline)
}
forEachStringMkLine(mklines.allVars.firstDef, def)
forEachStringMkLine(mklines.allVars.used, def)
mklines.allVars.forEach(func(varname string, data *scopeVar) {
if data.firstDef != nil {
define(varnameCanon(varname), data.firstDef)
}
})
mklines.allVars.forEach(func(varname string, data *scopeVar) {
if data.used != nil {
define(varnameCanon(varname), data.used)
}
})
}
handleFile := func(pathName string, info os.FileInfo, err error) error {

View file

@ -168,7 +168,8 @@ func (scc *SimpleCommandChecker) handleCommandVariable() bool {
varname := varuse.varname
if vartype := G.Pkgsrc.VariableType(scc.mklines, varname); vartype != nil && vartype.basicType.name == "ShellCommand" {
vartype := G.Pkgsrc.VariableType(scc.mklines, varname)
if vartype != nil && (vartype.basicType == BtShellCommand || vartype.basicType == BtPathname) {
scc.checkInstallCommand(shellword)
return true
}

View file

@ -45,6 +45,20 @@ func (s *Suite) Test_SimpleCommandChecker_checkCommandStart__unknown_default(c *
"WARN: Makefile:8: UNKNOWN_TOOL is used but not defined.")
}
// Despite its name, the TOOLS_PATH.* name the whole shell command,
// not just the path of its executable.
func (s *Suite) Test_SimpleCommandChecker_checkCommandStart__TOOLS_PATH(c *check.C) {
t := s.Init(c)
t.SetUpPackage("category/package",
"CONFIG_SHELL=\t${TOOLS_PATH.bash}")
t.Chdir("category/package")
t.FinishSetUp()
G.checkdirPackage(".")
t.CheckOutputEmpty()
}
func (s *Suite) Test_SimpleCommandChecker_checkInstallCommand(c *check.C) {
t := s.Init(c)
@ -841,6 +855,7 @@ func (s *Suite) Test_ShellLineChecker_CheckShellCommandLine(c *check.C) {
t := s.Init(c)
t.SetUpVartypes()
t.SetUpTool("[", "TEST", AtRunTime)
t.SetUpTool("awk", "AWK", AtRunTime)
t.SetUpTool("cp", "CP", AtRunTime)
t.SetUpTool("echo", "", AtRunTime)
@ -927,16 +942,13 @@ func (s *Suite) Test_ShellLineChecker_CheckShellCommandLine(c *check.C) {
"WARN: filename.mk:1: The exitcode of \"${UNZIP_CMD}\" at the left of the | operator is ignored.")
// From x11/wxGTK28/Makefile
// TODO: Why is TOOLS_PATH.msgfmt not recognized?
// At least, the warning should be more specific, mentioning USE_TOOLS.
test(""+
"set -e; cd ${WRKSRC}/locale; "+
"for lang in *.po; do "+
" [ \"$${lang}\" = \"wxstd.po\" ] && continue; "+
" ${TOOLS_PATH.msgfmt} -c -o \"$${lang%.po}.mo\" \"$${lang}\"; "+
"done",
"WARN: filename.mk:1: Unknown shell command \"[\".",
"WARN: filename.mk:1: Unknown shell command \"${TOOLS_PATH.msgfmt}\".")
nil...)
test("@cp from to",
"WARN: filename.mk:1: The shell command \"cp\" should not be hidden.")

View file

@ -33,6 +33,15 @@ type Tool struct {
MustUseVarForm bool
Validity Validity
Aliases []string
// The operating systems on which the tool is defined conditionally,
// usually by enclosing the tool definition in an ".if exists".
// See mk/tools/tools.*.mk.
conditionalOn []string
// The operating systems on which the tool is not defined at all.
// See mk/tools/tools.*.mk.
undefinedOn []string
}
func (tool *Tool) String() string {
@ -152,7 +161,7 @@ func (tr *Tools) Define(name, varname string, mkline *MkLine) *Tool {
func (tr *Tools) def(name, varname string, mustUseVarForm bool, validity Validity, aliases []string) *Tool {
assert(tr.IsValidToolName(name))
fresh := Tool{name, varname, mustUseVarForm, validity, aliases}
fresh := Tool{name, varname, mustUseVarForm, validity, aliases, nil, nil}
tool := tr.byName[name]
if tool == nil {

View file

@ -5,15 +5,15 @@ import "gopkg.in/check.v1"
func (s *Suite) Test_Tool_UsableAtLoadTime(c *check.C) {
t := s.Init(c)
nowhere := Tool{"nowhere", "", false, Nowhere, nil}
nowhere := Tool{"nowhere", "", false, Nowhere, nil, nil, nil}
t.CheckEquals(nowhere.UsableAtLoadTime(false), false)
t.CheckEquals(nowhere.UsableAtLoadTime(true), false)
load := Tool{"load", "", false, AfterPrefsMk, nil}
load := Tool{"load", "", false, AfterPrefsMk, nil, nil, nil}
t.CheckEquals(load.UsableAtLoadTime(false), false)
t.CheckEquals(load.UsableAtLoadTime(true), true)
run := Tool{"run", "", false, AtRunTime, nil}
run := Tool{"run", "", false, AtRunTime, nil, nil, nil}
t.CheckEquals(run.UsableAtLoadTime(false), false)
t.CheckEquals(run.UsableAtLoadTime(true), false)
}
@ -52,13 +52,13 @@ func (s *Suite) Test_Tool_UsableAtLoadTime__pkgconfig_builtin_mk(c *check.C) {
func (s *Suite) Test_Tool_UsableAtRunTime(c *check.C) {
t := s.Init(c)
nowhere := Tool{"nowhere", "", false, Nowhere, nil}
nowhere := Tool{"nowhere", "", false, Nowhere, nil, nil, nil}
t.CheckEquals(nowhere.UsableAtRunTime(), false)
load := Tool{"load", "", false, AfterPrefsMk, nil}
load := Tool{"load", "", false, AfterPrefsMk, nil, nil, nil}
t.CheckEquals(load.UsableAtRunTime(), true)
run := Tool{"run", "", false, AtRunTime, nil}
run := Tool{"run", "", false, AtRunTime, nil, nil, nil}
t.CheckEquals(run.UsableAtRunTime(), true)
}

View file

@ -210,12 +210,16 @@ func condInt(cond bool, trueValue, falseValue int) int {
}
func keysJoined(m map[string]bool) string {
return strings.Join(keysSorted(m), " ")
}
func keysSorted(m map[string]bool) []string {
var keys []string
for key := range m {
keys = append(keys, key)
}
sort.Strings(keys)
return strings.Join(keys, " ")
return keys
}
func copyStringMkLine(m map[string]*MkLine) map[string]*MkLine {
@ -617,22 +621,30 @@ func (o *Once) check(key uint64) bool {
//
// See also RedundantScope.
type Scope struct {
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
vs map[string]*scopeVar
}
type scopeVar struct {
firstDef *MkLine
lastDef *MkLine
value string
used *MkLine
fallback string
usedAtLoadTime bool
indeterminate bool
}
func NewScope() Scope {
return Scope{
make(map[string]*MkLine),
make(map[string]*MkLine),
make(map[string]string),
make(map[string]*MkLine),
make(map[string]bool),
make(map[string]string)}
return Scope{make(map[string]*scopeVar)}
}
func (s *Scope) v(varname string) *scopeVar {
if v, found := s.vs[varname]; found {
return v
}
var sv scopeVar
s.vs[varname] = &sv
return &sv
}
// Define marks the variable and its canonicalized form as defined.
@ -645,8 +657,9 @@ func (s *Scope) Define(varname string, mkline *MkLine) {
}
func (s *Scope) def(name string, mkline *MkLine) {
if s.firstDef[name] == nil {
s.firstDef[name] = mkline
v := s.v(name)
if v.firstDef == nil {
v.firstDef = mkline
if trace.Tracing {
trace.Step2("Defining %q for the first time in %s", name, mkline.String())
}
@ -654,7 +667,7 @@ func (s *Scope) def(name string, mkline *MkLine) {
trace.Step2("Defining %q in %s", name, mkline.String())
}
s.lastDef[name] = mkline
v.lastDef = mkline
// In most cases the defining lines are indeed variable assignments.
// Exceptions are comments from documentation sections, which still mark
@ -669,35 +682,37 @@ func (s *Scope) def(name string, mkline *MkLine) {
value := mkline.Value()
if trace.Tracing {
trace.Stepf("Scope.Define.append %s: %s = %q + %q",
mkline.String(), name, s.value[name], value)
mkline.String(), name, v.value, value)
}
s.value[name] += " " + value
v.value += " " + value
case opAssignDefault:
if _, set := s.value[name]; !set {
s.value[name] = mkline.Value()
if v.value == "" && !v.indeterminate {
v.value = mkline.Value()
}
case opAssignShell:
delete(s.value, name)
v.value = ""
v.indeterminate = true
default:
s.value[name] = mkline.Value()
v.value = mkline.Value()
}
}
func (s *Scope) Fallback(varname string, value string) {
s.fallback[varname] = value
s.v(varname).fallback = value
}
// Use marks the variable and its canonicalized form as used.
func (s *Scope) Use(varname string, line *MkLine, time VucTime) {
use := func(name string) {
if s.used[name] == nil {
s.used[name] = line
v := s.v(name)
if v.used == nil {
v.used = line
if trace.Tracing {
trace.Step2("Using %q in %s", name, line.String())
}
}
if time == VucLoadTime {
s.usedAtLoadTime[name] = true
v.usedAtLoadTime = true
}
}
@ -710,7 +725,7 @@ func (s *Scope) Use(varname string, line *MkLine, time VucTime) {
// - mentioned in a commented variable assignment,
// - mentioned in a documentation comment.
func (s *Scope) Mentioned(varname string) *MkLine {
return s.firstDef[varname]
return s.v(varname).firstDef
}
// IsDefined tests whether the variable is defined.
@ -719,7 +734,7 @@ func (s *Scope) Mentioned(varname string) *MkLine {
// Even if IsDefined returns true, FirstDefinition doesn't necessarily return true
// since the latter ignores the default definitions from vardefs.go, keyword dummyVardefMkline.
func (s *Scope) IsDefined(varname string) bool {
mkline := s.firstDef[varname]
mkline := s.v(varname).firstDef
return mkline != nil && mkline.IsVarassign()
}
@ -745,21 +760,21 @@ func (s *Scope) IsDefinedSimilar(varname string) bool {
// IsUsed tests whether the variable is used.
// It does NOT test the canonicalized variable name.
func (s *Scope) IsUsed(varname string) bool {
return s.used[varname] != nil
return s.v(varname).used != nil
}
// IsUsedSimilar tests whether the variable or its canonicalized form is used.
func (s *Scope) IsUsedSimilar(varname string) bool {
if s.used[varname] != nil {
if s.v(varname).used != nil {
return true
}
return s.used[varnameCanon(varname)] != nil
return s.v(varnameCanon(varname)).used != nil
}
// IsUsedAtLoadTime returns true if the variable is used at load time
// somewhere.
func (s *Scope) IsUsedAtLoadTime(varname string) bool {
return s.usedAtLoadTime[varname]
return s.v(varname).usedAtLoadTime
}
// FirstDefinition returns the line in which the variable has been defined first.
@ -771,7 +786,7 @@ func (s *Scope) IsUsedAtLoadTime(varname string) bool {
// round: the including file sets a value first, and the included file then
// assigns a default value using ?=.
func (s *Scope) FirstDefinition(varname string) *MkLine {
mkline := s.firstDef[varname]
mkline := s.v(varname).firstDef
if mkline != nil && mkline.IsVarassign() {
lastLine := s.LastDefinition(varname)
if trace.Tracing && lastLine != mkline {
@ -792,7 +807,7 @@ func (s *Scope) FirstDefinition(varname string) *MkLine {
// round: the including file sets a value first, and the included file then
// assigns a default value using ?=.
func (s *Scope) LastDefinition(varname string) *MkLine {
mkline := s.lastDef[varname]
mkline := s.v(varname).lastDef
if mkline != nil && mkline.IsVarassign() {
return mkline
}
@ -804,10 +819,10 @@ func (s *Scope) LastDefinition(varname string) *MkLine {
// mk/defaults/mk.conf for documentation.
func (s *Scope) Commented(varname string) *MkLine {
var mklines []*MkLine
if first := s.firstDef[varname]; first != nil {
if first := s.v(varname).firstDef; first != nil {
mklines = append(mklines, first)
}
if last := s.lastDef[varname]; last != nil {
if last := s.v(varname).lastDef; last != nil {
mklines = append(mklines, last)
}
@ -827,7 +842,7 @@ func (s *Scope) Commented(varname string) *MkLine {
}
func (s *Scope) FirstUse(varname string) *MkLine {
return s.used[varname]
return s.v(varname).used
}
// LastValue returns the value from the last variable definition.
@ -837,28 +852,50 @@ func (s *Scope) FirstUse(varname string) *MkLine {
// was not found, or that the variable value cannot be determined
// reliably. To distinguish these cases, call LastValueFound instead.
func (s *Scope) LastValue(varname string) string {
value, _ := s.LastValueFound(varname)
value, _, _ := s.LastValueFound(varname)
return value
}
func (s *Scope) LastValueFound(varname string) (value string, found bool) {
value, found = s.value[varname]
if !found {
value, found = s.fallback[varname]
func (s *Scope) LastValueFound(varname string) (value string, found bool, indeterminate bool) {
v := s.vs[varname]
if v == nil {
return
}
return
value = v.value
found = v.firstDef != nil && v.firstDef.IsVarassign()
indeterminate = v.indeterminate
if found {
return
}
return v.fallback, v.fallback != "", v.indeterminate
}
func (s *Scope) DefineAll(other Scope) {
var varnames []string
for varname := range other.firstDef {
for varname := range other.vs {
varnames = append(varnames, varname)
}
sort.Strings(varnames)
for _, varname := range varnames {
s.Define(varname, other.firstDef[varname])
s.Define(varname, other.lastDef[varname])
v := other.vs[varname]
if v.firstDef != nil {
s.Define(varname, v.firstDef)
s.Define(varname, v.lastDef)
}
}
}
func (s *Scope) forEach(action func(varname string, data *scopeVar)) {
var keys []string
for key := range s.vs {
keys = append(keys, key)
}
sort.Strings(keys)
for _, key := range keys {
action(key, s.vs[key])
}
}
@ -1183,32 +1220,32 @@ func joinSkipEmpty(sep string, elements ...string) string {
return strings.Join(nonempty, sep)
}
func joinSkipEmptyCambridge(conn string, elements ...string) string {
var nonempty []string
// joinCambridge returns "first, second conn third".
// It is used when each element is a single word.
// Empty elements are ignored completely.
func joinCambridge(conn string, elements ...string) string {
parts := make([]string, 0, 2+2*len(elements))
for _, element := range elements {
if element != "" {
nonempty = append(nonempty, element)
parts = append(parts, ", ", element)
}
}
var sb strings.Builder
for i, element := range nonempty {
if i > 0 {
if i == len(nonempty)-1 {
sb.WriteRune(' ')
sb.WriteString(conn)
sb.WriteRune(' ')
} else {
sb.WriteString(", ")
}
}
sb.WriteString(element)
if len(parts) == 0 {
return ""
}
if len(parts) < 4 {
return parts[1]
}
return sb.String()
parts = append(parts[1:len(parts)-2], " ", conn, " ", parts[len(parts)-1])
return strings.Join(parts, "")
}
func joinSkipEmptyOxford(conn string, elements ...string) string {
// joinOxford returns "first, second, conn third".
// It is used when each element may consist of multiple words.
// Empty elements are ignored completely.
func joinOxford(conn string, elements ...string) string {
var nonempty []string
for _, element := range elements {
if element != "" {

View file

@ -543,9 +543,10 @@ func (s *Suite) Test_Scope__commented_varassign(c *check.C) {
t.CheckEquals(scope.Mentioned("VAR"), mkline)
t.CheckEquals(scope.Commented("VAR"), mkline)
value, found := scope.LastValueFound("VAR")
value, found, indeterminate := scope.LastValueFound("VAR")
t.CheckEquals(value, "")
t.CheckEquals(found, false)
t.CheckEquals(indeterminate, false)
}
func (s *Suite) Test_Scope_Define(c *check.C) {
@ -553,39 +554,40 @@ func (s *Suite) Test_Scope_Define(c *check.C) {
scope := NewScope()
test := func(line string, ok bool, value string) {
test := func(line string, found, indeterminate bool, value string) {
mkline := t.NewMkLine("file.mk", 123, line)
scope.Define("BUILD_DIRS", mkline)
actualValue, actualFound := scope.LastValueFound("BUILD_DIRS")
actualValue, actualFound, actualIndeterminate := scope.LastValueFound("BUILD_DIRS")
t.CheckEquals(actualValue, value)
t.CheckEquals(actualFound, ok)
t.CheckEquals(scope.value["BUILD_DIRS"], value)
t.CheckDeepEquals(
[]interface{}{actualFound, actualIndeterminate, actualValue},
[]interface{}{found, indeterminate, value})
t.CheckEquals(scope.vs["BUILD_DIRS"].value, value)
}
test("BUILD_DIRS?=\tdefault",
true, "default")
true, false, "default")
test(
"BUILD_DIRS=\tone two three",
true, "one two three")
true, false, "one two three")
test(
"BUILD_DIRS+=\tfour",
true, "one two three four")
true, false, "one two three four")
// Later default assignments do not have an effect.
test("BUILD_DIRS?=\tdefault",
true, "one two three four")
true, false, "one two three four")
test("BUILD_DIRS!=\techo dynamic",
false, "")
true, true, "")
// FIXME: This is not correct. The shell assignment sets the variable,
// after which all further default assignments are ignored.
// The shell assignment above sets the variable to an indeterminate
// value, after which all further default assignments are ignored.
test("BUILD_DIRS?=\tdefault after shell assignment",
true, "default after shell assignment")
true, true, "")
}
func (s *Suite) Test_Scope_Mentioned(c *check.C) {
@ -727,9 +729,7 @@ func (s *Suite) Test_Scope_DefineAll(c *check.C) {
dst := NewScope()
dst.DefineAll(src)
c.Check(dst.firstDef, check.HasLen, 0)
c.Check(dst.lastDef, check.HasLen, 0)
c.Check(dst.used, check.HasLen, 0)
c.Check(dst.vs, check.HasLen, 0)
src.Define("VAR", t.NewMkLine("file.mk", 1, "VAR=value"))
dst.DefineAll(src)
@ -1020,23 +1020,23 @@ func (s *Suite) Test_joinSkipEmpty(c *check.C) {
"one, two, three")
}
func (s *Suite) Test_joinSkipEmptyCambridge(c *check.C) {
func (s *Suite) Test_joinCambridge(c *check.C) {
t := s.Init(c)
t.CheckDeepEquals(
joinSkipEmptyCambridge("and", "", "one", "", "", "two", "", "three"),
joinCambridge("and", "", "one", "", "", "two", "", "three"),
"one, two and three")
t.CheckDeepEquals(
joinSkipEmptyCambridge("and", "", "one", "", ""),
joinCambridge("and", "", "one", "", ""),
"one")
}
func (s *Suite) Test_joinSkipEmptyOxford(c *check.C) {
func (s *Suite) Test_joinOxford(c *check.C) {
t := s.Init(c)
t.CheckDeepEquals(
joinSkipEmptyOxford("and", "", "one", "", "", "two", "", "three"),
joinOxford("and", "", "one", "", "", "two", "", "three"),
"one, two, and three")
}

View file

@ -562,6 +562,7 @@ func (info *varalignLine) alignValueSingle(newWidth int) {
}
fix.ReplaceAt(info.rawIndex, info.spaceBeforeValueIndex(), oldSpace, newSpace)
fix.Apply()
info.spaceBeforeValue = newSpace
}
func (info *varalignLine) alignValueInitial(newWidth int) {
@ -855,10 +856,7 @@ func (p *varalignParts) isCanonicalFollow() bool {
}
func (p *varalignParts) isTooLongFor(valueColumn int) bool {
// FIXME: As of 2020-01-27, this method is called from
// Test_VaralignBlock__right_margin with valueColumn == 0,
// which doesn't make sense. It should at least be 8.
column := tabWidthAppend(valueColumn, p.value)
column := tabWidthAppend(imax(valueColumn, 8), p.value)
if p.isContinuation() {
column += 2
}

View file

@ -2073,7 +2073,7 @@ func (s *Suite) Test_VaralignBlock__right_margin(c *check.C) {
"\t............................................................70 \t\t\\",
"\t........................................................66")
vt.Diagnostics(
"NOTE: Makefile:2: The continuation backslash should be in column 73, not 81.",
"NOTE: Makefile:2: The continuation backslash should be preceded by a single space.",
"NOTE: Makefile:3: The continuation backslash should be in column 73, not 81.")
vt.Autofixes(
"AUTOFIX: Makefile:2: Replacing \"\\t\" with \" \".",
@ -3585,11 +3585,7 @@ func (s *Suite) Test_varalignLine_alignValueSingle(c *check.C) {
t.CheckEqualsf(
mkline.RawText(0), after,
"Line.raw.text, autofix=%v", autofix)
// As of 2019-12-11, the info fields are not updated
// accordingly, but they should.
// FIXME: update info accordingly
t.CheckEqualsf(info.String(), before,
t.CheckEqualsf(info.String(), after,
"info.String, autofix=%v", autofix)
}

View file

@ -421,7 +421,7 @@ func (reg *VarTypeRegistry) enumFrom(
G.Logger.TechFatalf(
mklines.lines.Filename,
"Must contain at least 1 variable definition for %s.",
joinSkipEmptyCambridge("or", varcanons...))
joinCambridge("or", varcanons...))
}
if trace.Tracing {
@ -1067,7 +1067,8 @@ func (reg *VarTypeRegistry) Init(src *Pkgsrc) {
reg.pkg("CONFIGURE_SCRIPT", BtPathname)
reg.pkglist("CONFIG_GUESS_OVERRIDE", BtPathPattern)
reg.pkglist("CONFIG_STATUS_OVERRIDE", BtPathPattern)
reg.pkg("CONFIG_SHELL", BtPathname)
reg.pkg("CONFIG_SHELL", BtShellCommand)
reg.cmdline("CONFIG_SHELL_FLAGS", BtShellWord, List)
reg.pkglist("CONFIG_SUB_OVERRIDE", BtPathPattern)
reg.pkglist("CONFLICTS", BtDependencyPattern)
reg.pkgappend("CONF_FILES", BtConfFiles)

View file

@ -182,7 +182,7 @@ func (perms ACLPermissions) String() string {
}
func (perms ACLPermissions) HumanString() string {
return joinSkipEmptyOxford("or",
return joinOxford("or",
condStr(perms.Contains(aclpSet), "set", ""),
condStr(perms.Contains(aclpSetDefault), "given a default value", ""),
condStr(perms.Contains(aclpAppend), "appended to", ""),
@ -265,12 +265,12 @@ func (vt *Vartype) AlternativeFiles(perms ACLPermissions) string {
neg = merge(neg)
}
positive := joinSkipEmptyCambridge("or", pos...)
positive := joinCambridge("or", pos...)
if positive == "" {
return ""
}
negative := joinSkipEmptyCambridge("or", neg...)
negative := joinCambridge("or", neg...)
if negative == "" {
return positive
}