pkgsrc-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[pkgsrc/trunk]: pkgsrc/pkgtools/pkglint pkgtools/pkglint: update to 19.4.10
details: https://anonhg.NetBSD.org/pkgsrc/rev/af5108ec3c4c
branches: trunk
changeset: 424230:af5108ec3c4c
user: rillig <rillig%pkgsrc.org@localhost>
date: Sat Mar 07 23:35:35 2020 +0000
description:
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.
diffstat:
pkgtools/pkglint/Makefile | 4 +-
pkgtools/pkglint/files/homepage_test.go | 14 +-
pkgtools/pkglint/files/logging.go | 2 +-
pkgtools/pkglint/files/mkcondchecker.go | 1 +
pkgtools/pkglint/files/mkline_test.go | 11 +-
pkgtools/pkglint/files/mklines.go | 6 +-
pkgtools/pkglint/files/mklines_test.go | 15 +-
pkgtools/pkglint/files/mkvarusechecker.go | 65 +++++++++
pkgtools/pkglint/files/mkvarusechecker_test.go | 130 +++++++++++++++++++
pkgtools/pkglint/files/package.go | 2 +-
pkgtools/pkglint/files/pkglint.go | 4 +-
pkgtools/pkglint/files/pkgsrc.go | 88 ++++++++++++-
pkgtools/pkglint/files/shell.go | 3 +-
pkgtools/pkglint/files/shell_test.go | 20 ++-
pkgtools/pkglint/files/tools.go | 11 +-
pkgtools/pkglint/files/tools_test.go | 12 +-
pkgtools/pkglint/files/util.go | 165 +++++++++++++++---------
pkgtools/pkglint/files/util_test.go | 44 +++---
pkgtools/pkglint/files/varalignblock.go | 6 +-
pkgtools/pkglint/files/varalignblock_test.go | 8 +-
pkgtools/pkglint/files/vardefs.go | 5 +-
pkgtools/pkglint/files/vartype.go | 6 +-
22 files changed, 480 insertions(+), 142 deletions(-)
diffs (truncated from 1216 to 300 lines):
diff -r 705f110598a7 -r af5108ec3c4c pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Sat Mar 07 23:30:22 2020 +0000
+++ b/pkgtools/pkglint/Makefile Sat Mar 07 23:35:35 2020 +0000
@@ -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/}
diff -r 705f110598a7 -r af5108ec3c4c pkgtools/pkglint/files/homepage_test.go
--- a/pkgtools/pkglint/files/homepage_test.go Sat Mar 07 23:30:22 2020 +0000
+++ b/pkgtools/pkglint/files/homepage_test.go Sat Mar 07 23:35:35 2020 +0000
@@ -64,8 +64,8 @@
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 @@
"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 @@
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 @@
"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.
diff -r 705f110598a7 -r af5108ec3c4c pkgtools/pkglint/files/logging.go
--- a/pkgtools/pkglint/files/logging.go Sat Mar 07 23:30:22 2020 +0000
+++ b/pkgtools/pkglint/files/logging.go Sat Mar 07 23:35:35 2020 +0000
@@ -368,7 +368,7 @@
}
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"))))
diff -r 705f110598a7 -r af5108ec3c4c pkgtools/pkglint/files/mkcondchecker.go
--- a/pkgtools/pkglint/files/mkcondchecker.go Sat Mar 07 23:30:22 2020 +0000
+++ b/pkgtools/pkglint/files/mkcondchecker.go Sat Mar 07 23:35:35 2020 +0000
@@ -29,6 +29,7 @@
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)
}
diff -r 705f110598a7 -r af5108ec3c4c pkgtools/pkglint/files/mkline_test.go
--- a/pkgtools/pkglint/files/mkline_test.go Sat Mar 07 23:30:22 2020 +0000
+++ b/pkgtools/pkglint/files/mkline_test.go Sat Mar 07 23:35:35 2020 +0000
@@ -1034,6 +1034,10 @@
}
// 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 @@
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
diff -r 705f110598a7 -r af5108ec3c4c pkgtools/pkglint/files/mklines.go
--- a/pkgtools/pkglint/files/mklines.go Sat Mar 07 23:30:22 2020 +0000
+++ b/pkgtools/pkglint/files/mklines.go Sat Mar 07 23:35:35 2020 +0000
@@ -194,9 +194,9 @@
// 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)
})
}
diff -r 705f110598a7 -r af5108ec3c4c pkgtools/pkglint/files/mklines_test.go
--- a/pkgtools/pkglint/files/mklines_test.go Sat Mar 07 23:30:22 2020 +0000
+++ b/pkgtools/pkglint/files/mklines_test.go Sat Mar 07 23:35:35 2020 +0000
@@ -494,7 +494,8 @@
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 @@
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 @@
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 @@
mklines.Check()
t.CheckDeepEquals(
- keys(mklines.allVars.firstDef),
+ keys(mklines.allVars.vs),
[]string{
"BUILTIN_FIND_FILES_VAR",
"BUILTIN_FIND_HEADERS_VAR",
diff -r 705f110598a7 -r af5108ec3c4c pkgtools/pkglint/files/mkvarusechecker.go
--- a/pkgtools/pkglint/files/mkvarusechecker.go Sat Mar 07 23:30:22 2020 +0000
+++ b/pkgtools/pkglint/files/mkvarusechecker.go Sat Mar 07 23:35:35 2020 +0000
@@ -27,8 +27,10 @@
ck.checkVarname(vuc.time)
ck.checkModifiers()
+ ck.checkAssignable(vuc)
ck.checkQuoting(vuc)
+ ck.checkToolsPlatform()
ck.checkBuildDefs()
ck.checkDeprecated()
@@ -445,6 +447,35 @@
"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 @@
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
diff -r 705f110598a7 -r af5108ec3c4c pkgtools/pkglint/files/mkvarusechecker_test.go
--- a/pkgtools/pkglint/files/mkvarusechecker_test.go Sat Mar 07 23:30:22 2020 +0000
+++ b/pkgtools/pkglint/files/mkvarusechecker_test.go Sat Mar 07 23:35:35 2020 +0000
@@ -957,6 +957,77 @@
"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)
Home |
Main Index |
Thread Index |
Old Index