pkgsrc-Changes archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
CVS commit: pkgsrc/pkgtools/pkglint
Module Name: pkgsrc
Committed By: rillig
Date: Sat Mar 7 23:35:35 UTC 2020
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile
pkgsrc/pkgtools/pkglint/files: homepage_test.go logging.go
mkcondchecker.go mkline_test.go mklines.go mklines_test.go
mkvarusechecker.go mkvarusechecker_test.go package.go pkglint.go
pkgsrc.go shell.go shell_test.go tools.go tools_test.go util.go
util_test.go varalignblock.go varalignblock_test.go vardefs.go
vartype.go
Log Message:
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.
To generate a diff of this commit:
cvs rdiff -u -r1.632 -r1.633 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.3 -r1.4 pkgsrc/pkgtools/pkglint/files/homepage_test.go
cvs rdiff -u -r1.41 -r1.42 pkgsrc/pkgtools/pkglint/files/logging.go
cvs rdiff -u -r1.4 -r1.5 pkgsrc/pkgtools/pkglint/files/mkcondchecker.go
cvs rdiff -u -r1.79 -r1.80 pkgsrc/pkgtools/pkglint/files/mkline_test.go
cvs rdiff -u -r1.68 -r1.69 pkgsrc/pkgtools/pkglint/files/mklines.go
cvs rdiff -u -r1.62 -r1.63 pkgsrc/pkgtools/pkglint/files/mklines_test.go
cvs rdiff -u -r1.6 -r1.7 pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go \
pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go
cvs rdiff -u -r1.81 -r1.82 pkgsrc/pkgtools/pkglint/files/package.go
cvs rdiff -u -r1.74 -r1.75 pkgsrc/pkgtools/pkglint/files/pkglint.go
cvs rdiff -u -r1.50 -r1.51 pkgsrc/pkgtools/pkglint/files/pkgsrc.go
cvs rdiff -u -r1.58 -r1.59 pkgsrc/pkgtools/pkglint/files/shell.go
cvs rdiff -u -r1.64 -r1.65 pkgsrc/pkgtools/pkglint/files/shell_test.go
cvs rdiff -u -r1.22 -r1.23 pkgsrc/pkgtools/pkglint/files/tools.go
cvs rdiff -u -r1.24 -r1.25 pkgsrc/pkgtools/pkglint/files/tools_test.go
cvs rdiff -u -r1.73 -r1.74 pkgsrc/pkgtools/pkglint/files/util.go
cvs rdiff -u -r1.48 -r1.49 pkgsrc/pkgtools/pkglint/files/util_test.go
cvs rdiff -u -r1.17 -r1.18 pkgsrc/pkgtools/pkglint/files/varalignblock.go
cvs rdiff -u -r1.14 -r1.15 \
pkgsrc/pkgtools/pkglint/files/varalignblock_test.go
cvs rdiff -u -r1.89 -r1.90 pkgsrc/pkgtools/pkglint/files/vardefs.go
cvs rdiff -u -r1.47 -r1.48 pkgsrc/pkgtools/pkglint/files/vartype.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.632 pkgsrc/pkgtools/pkglint/Makefile:1.633
--- pkgsrc/pkgtools/pkglint/Makefile:1.632 Mon Feb 17 20:22:21 2020
+++ pkgsrc/pkgtools/pkglint/Makefile Sat Mar 7 23:35:35 2020
@@ -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/}
Index: pkgsrc/pkgtools/pkglint/files/homepage_test.go
diff -u pkgsrc/pkgtools/pkglint/files/homepage_test.go:1.3 pkgsrc/pkgtools/pkglint/files/homepage_test.go:1.4
--- pkgsrc/pkgtools/pkglint/files/homepage_test.go:1.3 Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/homepage_test.go Sat Mar 7 23:35:35 2020
@@ -64,8 +64,8 @@ func (s *Suite) Test_HomepageChecker_che
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_che
"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_che
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_che
"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.
Index: pkgsrc/pkgtools/pkglint/files/logging.go
diff -u pkgsrc/pkgtools/pkglint/files/logging.go:1.41 pkgsrc/pkgtools/pkglint/files/logging.go:1.42
--- pkgsrc/pkgtools/pkglint/files/logging.go:1.41 Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/logging.go Sat Mar 7 23:35:35 2020
@@ -368,7 +368,7 @@ func (l *Logger) ShowSummary(args []stri
}
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"))))
Index: pkgsrc/pkgtools/pkglint/files/mkcondchecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mkcondchecker.go:1.4 pkgsrc/pkgtools/pkglint/files/mkcondchecker.go:1.5
--- pkgsrc/pkgtools/pkglint/files/mkcondchecker.go:1.4 Sat Jan 18 21:56:09 2020
+++ pkgsrc/pkgtools/pkglint/files/mkcondchecker.go Sat Mar 7 23:35:35 2020
@@ -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)
}
Index: pkgsrc/pkgtools/pkglint/files/mkline_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.79 pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.80
--- pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.79 Sat Jan 18 21:56:09 2020
+++ pkgsrc/pkgtools/pkglint/files/mkline_test.go Sat Mar 7 23:35:35 2020
@@ -1034,6 +1034,10 @@ func (s *Suite) Test_MkLine_VariableNeed
}
// 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_VariableNeed
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
Index: pkgsrc/pkgtools/pkglint/files/mklines.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines.go:1.68 pkgsrc/pkgtools/pkglint/files/mklines.go:1.69
--- pkgsrc/pkgtools/pkglint/files/mklines.go:1.68 Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/mklines.go Sat Mar 7 23:35:35 2020
@@ -194,9 +194,9 @@ func (mklines *MkLines) collectDocumente
// 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)
})
}
Index: pkgsrc/pkgtools/pkglint/files/mklines_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.62 pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.63
--- pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.62 Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/mklines_test.go Sat Mar 7 23:35:35 2020
@@ -494,7 +494,8 @@ func (s *Suite) Test_MkLines_collectUsed
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_collectUsed
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_collectDocu
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_collectVari
mklines.Check()
t.CheckDeepEquals(
- keys(mklines.allVars.firstDef),
+ keys(mklines.allVars.vs),
[]string{
"BUILTIN_FIND_FILES_VAR",
"BUILTIN_FIND_HEADERS_VAR",
Index: pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go:1.6 pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go:1.7
--- pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go:1.6 Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go Sat Mar 7 23:35:35 2020
@@ -27,8 +27,10 @@ func (ck *MkVarUseChecker) Check(vuc *Va
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) warnToolLoadT
"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) warnRedundant
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
Index: pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go:1.6 pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go:1.7
--- pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go:1.6 Sat Jan 18 21:56:09 2020
+++ pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go Sat Mar 7 23:35:35 2020
@@ -957,6 +957,77 @@ func (s *Suite) Test_MkVarUseChecker_war
"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_fix
"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)
Index: pkgsrc/pkgtools/pkglint/files/package.go
diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.81 pkgsrc/pkgtools/pkglint/files/package.go:1.82
--- pkgsrc/pkgtools/pkglint/files/package.go:1.81 Mon Feb 17 20:22:21 2020
+++ pkgsrc/pkgtools/pkglint/files/package.go Sat Mar 7 23:35:35 2020
@@ -135,7 +135,7 @@ func (pkg *Package) load() ([]CurrPath,
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))
}
Index: pkgsrc/pkgtools/pkglint/files/pkglint.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint.go:1.74 pkgsrc/pkgtools/pkglint/files/pkglint.go:1.75
--- pkgsrc/pkgtools/pkglint/files/pkglint.go:1.74 Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/pkglint.go Sat Mar 7 23:35:35 2020
@@ -384,13 +384,13 @@ func resolveVariableRefs(text string, mk
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
}
}
Index: pkgsrc/pkgtools/pkglint/files/pkgsrc.go
diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.50 pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.51
--- pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.50 Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/pkgsrc.go Sat Mar 7 23:35:35 2020
@@ -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 {
Index: pkgsrc/pkgtools/pkglint/files/shell.go
diff -u pkgsrc/pkgtools/pkglint/files/shell.go:1.58 pkgsrc/pkgtools/pkglint/files/shell.go:1.59
--- pkgsrc/pkgtools/pkglint/files/shell.go:1.58 Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/shell.go Sat Mar 7 23:35:35 2020
@@ -168,7 +168,8 @@ func (scc *SimpleCommandChecker) handleC
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
}
Index: pkgsrc/pkgtools/pkglint/files/shell_test.go
diff -u pkgsrc/pkgtools/pkglint/files/shell_test.go:1.64 pkgsrc/pkgtools/pkglint/files/shell_test.go:1.65
--- pkgsrc/pkgtools/pkglint/files/shell_test.go:1.64 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/shell_test.go Sat Mar 7 23:35:35 2020
@@ -45,6 +45,20 @@ func (s *Suite) Test_SimpleCommandChecke
"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_Ch
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_Ch
"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.")
Index: pkgsrc/pkgtools/pkglint/files/tools.go
diff -u pkgsrc/pkgtools/pkglint/files/tools.go:1.22 pkgsrc/pkgtools/pkglint/files/tools.go:1.23
--- pkgsrc/pkgtools/pkglint/files/tools.go:1.22 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/tools.go Sat Mar 7 23:35:35 2020
@@ -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 st
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 {
Index: pkgsrc/pkgtools/pkglint/files/tools_test.go
diff -u pkgsrc/pkgtools/pkglint/files/tools_test.go:1.24 pkgsrc/pkgtools/pkglint/files/tools_test.go:1.25
--- pkgsrc/pkgtools/pkglint/files/tools_test.go:1.24 Sun Dec 8 00:06:38 2019
+++ pkgsrc/pkgtools/pkglint/files/tools_test.go Sat Mar 7 23:35:35 2020
@@ -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_UsableAtLoadTi
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)
}
Index: pkgsrc/pkgtools/pkglint/files/util.go
diff -u pkgsrc/pkgtools/pkglint/files/util.go:1.73 pkgsrc/pkgtools/pkglint/files/util.go:1.74
--- pkgsrc/pkgtools/pkglint/files/util.go:1.73 Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/util.go Sat Mar 7 23:35:35 2020
@@ -210,12 +210,16 @@ func condInt(cond bool, trueValue, false
}
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, m
}
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
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
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
// - 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
// 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
// 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
// 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
// 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 s
// 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
}
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)
// 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
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 != "" {
Index: pkgsrc/pkgtools/pkglint/files/util_test.go
diff -u pkgsrc/pkgtools/pkglint/files/util_test.go:1.48 pkgsrc/pkgtools/pkglint/files/util_test.go:1.49
--- pkgsrc/pkgtools/pkglint/files/util_test.go:1.48 Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/util_test.go Sat Mar 7 23:35:35 2020
@@ -543,9 +543,10 @@ func (s *Suite) Test_Scope__commented_va
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 *che
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 *
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 *ch
"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")
}
Index: pkgsrc/pkgtools/pkglint/files/varalignblock.go
diff -u pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.17 pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.18
--- pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.17 Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/varalignblock.go Sat Mar 7 23:35:35 2020
@@ -562,6 +562,7 @@ func (info *varalignLine) alignValueSing
}
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) isCanonicalFollo
}
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
}
Index: pkgsrc/pkgtools/pkglint/files/varalignblock_test.go
diff -u pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.14 pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.15
--- pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.14 Mon Feb 17 20:22:21 2020
+++ pkgsrc/pkgtools/pkglint/files/varalignblock_test.go Sat Mar 7 23:35:35 2020
@@ -2073,7 +2073,7 @@ func (s *Suite) Test_VaralignBlock__righ
"\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_alignV
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)
}
Index: pkgsrc/pkgtools/pkglint/files/vardefs.go
diff -u pkgsrc/pkgtools/pkglint/files/vardefs.go:1.89 pkgsrc/pkgtools/pkglint/files/vardefs.go:1.90
--- pkgsrc/pkgtools/pkglint/files/vardefs.go:1.89 Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/vardefs.go Sat Mar 7 23:35:35 2020
@@ -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 *Pk
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)
Index: pkgsrc/pkgtools/pkglint/files/vartype.go
diff -u pkgsrc/pkgtools/pkglint/files/vartype.go:1.47 pkgsrc/pkgtools/pkglint/files/vartype.go:1.48
--- pkgsrc/pkgtools/pkglint/files/vartype.go:1.47 Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/vartype.go Sat Mar 7 23:35:35 2020
@@ -182,7 +182,7 @@ func (perms ACLPermissions) String() str
}
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(perm
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
}
Home |
Main Index |
Thread Index |
Old Index