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/4b437042edca
branches:  trunk
changeset: 412480:4b437042edca
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 cb24f573542b -r 4b437042edca 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 cb24f573542b -r 4b437042edca 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 cb24f573542b -r 4b437042edca 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 cb24f573542b -r 4b437042edca 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 cb24f573542b -r 4b437042edca 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 cb24f573542b -r 4b437042edca 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 cb24f573542b -r 4b437042edca 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 cb24f573542b -r 4b437042edca 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 cb24f573542b -r 4b437042edca 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