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 5.7.4



details:   https://anonhg.NetBSD.org/pkgsrc/rev/2cd067e01788
branches:  trunk
changeset: 394102:2cd067e01788
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Wed Apr 03 21:49:51 2019 +0000

description:
pkgtools/pkglint: update to 5.7.4

Changes since 5.7.3:

* Warn about dependency patterns that are missing a version number,
  such as ${PYPKGPREFIX}-sqlite3:../../databases/py-sqlite3.

* Suggest to replace the := assignment operator with the :sh modifier,
  in some cases where the variable is not obviously used at load time.

diffstat:

 pkgtools/pkglint/Makefile                    |    4 +-
 pkgtools/pkglint/files/autofix.go            |    6 +-
 pkgtools/pkglint/files/check_test.go         |   56 +++++-
 pkgtools/pkglint/files/licenses.go           |    5 +-
 pkgtools/pkglint/files/licenses_test.go      |    2 +-
 pkgtools/pkglint/files/mkline.go             |  143 +++++++++------
 pkgtools/pkglint/files/mkline_test.go        |  234 ++++++++++++++++++--------
 pkgtools/pkglint/files/mklinechecker.go      |  199 +++++++++++++++-------
 pkgtools/pkglint/files/mklinechecker_test.go |  152 ++++++++++++++--
 pkgtools/pkglint/files/mklines.go            |   47 +++--
 pkgtools/pkglint/files/mklines_test.go       |   20 +-
 pkgtools/pkglint/files/mkparser.go           |    3 +-
 pkgtools/pkglint/files/mkparser_test.go      |    5 +-
 pkgtools/pkglint/files/mktypes.go            |   33 +++
 pkgtools/pkglint/files/mktypes_test.go       |   40 ++++
 pkgtools/pkglint/files/package.go            |   10 +-
 pkgtools/pkglint/files/package_test.go       |   12 +-
 pkgtools/pkglint/files/pkglint.go            |   25 +-
 pkgtools/pkglint/files/pkglint_test.go       |   56 +++---
 pkgtools/pkglint/files/pkgsrc.go             |   17 +-
 pkgtools/pkglint/files/pkgsrc_test.go        |   25 +-
 pkgtools/pkglint/files/redundantscope.go     |    5 +-
 pkgtools/pkglint/files/shell.go              |   56 +----
 pkgtools/pkglint/files/shell_test.go         |  133 +++++----------
 pkgtools/pkglint/files/testnames_test.go     |    1 +
 pkgtools/pkglint/files/util.go               |   66 ++----
 pkgtools/pkglint/files/util_test.go          |    3 +-
 pkgtools/pkglint/files/var.go                |    4 +-
 pkgtools/pkglint/files/vardefs.go            |    4 +-
 pkgtools/pkglint/files/vardefs_test.go       |    6 +-
 pkgtools/pkglint/files/vartype_test.go       |    8 +-
 pkgtools/pkglint/files/vartypecheck.go       |   58 ++++--
 pkgtools/pkglint/files/vartypecheck_test.go  |  203 ++++++++++++++++-------
 33 files changed, 1032 insertions(+), 609 deletions(-)

diffs (truncated from 3678 to 300 lines):

diff -r c5e4bc10485b -r 2cd067e01788 pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Wed Apr 03 19:10:26 2019 +0000
+++ b/pkgtools/pkglint/Makefile Wed Apr 03 21:49:51 2019 +0000
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.572 2019/03/24 13:58:38 rillig Exp $
+# $NetBSD: Makefile,v 1.573 2019/04/03 21:49:51 rillig Exp $
 
-PKGNAME=       pkglint-5.7.3
+PKGNAME=       pkglint-5.7.4
 CATEGORIES=    pkgtools
 DISTNAME=      tools
 MASTER_SITES=  ${MASTER_SITE_GITHUB:=golang/}
diff -r c5e4bc10485b -r 2cd067e01788 pkgtools/pkglint/files/autofix.go
--- a/pkgtools/pkglint/files/autofix.go Wed Apr 03 19:10:26 2019 +0000
+++ b/pkgtools/pkglint/files/autofix.go Wed Apr 03 21:49:51 2019 +0000
@@ -82,7 +82,7 @@
        fix.explanation = explanation
 }
 
-// ReplaceAfter replaces "from" with "to", a single time.
+// Replace replaces "from" with "to", a single time.
 func (fix *Autofix) Replace(from string, to string) {
        fix.ReplaceAfter("", from, to)
 }
@@ -227,7 +227,9 @@
 }
 
 // Anyway has the effect of showing the diagnostic even when nothing can
-// be fixed automatically, but only if neither --show-autofix nor
+// be fixed automatically.
+//
+// As usual, the diagnostic is only shown if neither --show-autofix nor
 // --autofix mode is given.
 func (fix *Autofix) Anyway() {
        fix.anyway = !G.Logger.IsAutofix()
diff -r c5e4bc10485b -r 2cd067e01788 pkgtools/pkglint/files/check_test.go
--- a/pkgtools/pkglint/files/check_test.go      Wed Apr 03 19:10:26 2019 +0000
+++ b/pkgtools/pkglint/files/check_test.go      Wed Apr 03 21:49:51 2019 +0000
@@ -162,7 +162,7 @@
 //
 // See SetUpTool for registering tools like echo, awk, perl.
 func (t *Tester) SetUpVartypes() {
-       G.Pkgsrc.vartypes.Init(G.Pkgsrc)
+       G.Pkgsrc.vartypes.Init(&G.Pkgsrc)
 }
 
 func (t *Tester) SetUpMasterSite(varname string, urls ...string) {
@@ -531,22 +531,27 @@
 //  include("including.mk",
 //      include("other.mk",
 //          "VAR= other"),
-//      include("module.mk",
+//      include("subdir/module.mk",
 //          "VAR= module",
-//          include("version.mk",
+//          include("subdir/version.mk",
 //              "VAR= version"),
-//          include("env.mk",
+//          include("subdir/env.mk",
 //              "VAR= env")))
 //
 //  mklines := get("including.mk")
 //  module := get("module.mk")
+//
+// The filenames passed to the include function are all relative to the
+// same location, but that location is irrelevant in practice. The generated
+// .include lines take the relative paths into account. For example, when
+// subdir/module.mk includes subdir/version.mk, the include line is just:
+//  .include "version.mk"
 func (t *Tester) SetUpHierarchy() (
        include func(filename string, args ...interface{}) MkLines,
        get func(string) MkLines) {
 
        files := map[string]MkLines{}
 
-       // FIXME: Define where the filename is relative to: to the file, or to the current directory.
        include = func(filename string, args ...interface{}) MkLines {
                var lines []Line
                lineno := 1
@@ -561,7 +566,7 @@
                        case string:
                                addLine(arg)
                        case MkLines:
-                               text := sprintf(".include %q", arg.lines.FileName)
+                               text := sprintf(".include %q", relpath(path.Dir(filename), arg.lines.FileName))
                                addLine(text)
                                lines = append(lines, arg.lines.Lines...)
                        default:
@@ -570,9 +575,7 @@
                }
 
                mklines := NewMkLines(NewLines(filename, lines))
-               // FIXME: This filename must be relative to the including file.
-               G.Assertf(files[filename] == nil, "MkLines with name %q already exist.", filename)
-               // FIXME: This filename must be relative to the base directory.
+               G.Assertf(files[filename] == nil, "MkLines with name %q already exists.", filename)
                files[filename] = mklines
                return mklines
        }
@@ -585,6 +588,37 @@
        return
 }
 
+// Demonstrates that Tester.SetUpHierarchy uses relative paths for the
+// .include directives.
+func (s *Suite) Test_Tester_SetUpHierarchy(c *check.C) {
+       t := s.Init(c)
+
+       include, get := t.SetUpHierarchy()
+       include("including.mk",
+               include("other.mk",
+                       "VAR= other"),
+               include("subdir/module.mk",
+                       "VAR= module",
+                       include("subdir/version.mk",
+                               "VAR= version"),
+                       include("subdir/env.mk",
+                               "VAR= env")))
+
+       mklines := get("including.mk")
+
+       mklines.ForEach(func(mkline MkLine) { mkline.Notef("Text is: %s", mkline.Text) })
+
+       t.CheckOutputLines(
+               "NOTE: including.mk:1: Text is: .include \"other.mk\"",
+               "NOTE: other.mk:1: Text is: VAR= other",
+               "NOTE: including.mk:2: Text is: .include \"subdir/module.mk\"",
+               "NOTE: subdir/module.mk:1: Text is: VAR= module",
+               "NOTE: subdir/module.mk:2: Text is: .include \"version.mk\"",
+               "NOTE: subdir/version.mk:1: Text is: VAR= version",
+               "NOTE: subdir/module.mk:3: Text is: .include \"env.mk\"",
+               "NOTE: subdir/env.mk:1: Text is: VAR= env")
+}
+
 // Check delegates a check to the check.Check function.
 // Thereby, there is no need to distinguish between c.Check and t.Check
 // in the test code.
@@ -691,8 +725,8 @@
        return NewMkLine(t.NewLine(filename, lineno, text))
 }
 
-func (t *Tester) NewShellLineChecker(filename string, lineno int, text string) *ShellLineChecker {
-       return NewShellLineChecker(t.NewMkLine(filename, lineno, text))
+func (t *Tester) NewShellLineChecker(mklines MkLines, filename string, lineno int, text string) *ShellLineChecker {
+       return NewShellLineChecker(mklines, t.NewMkLine(filename, lineno, text))
 }
 
 // NewLines returns a list of simple lines that belong together.
diff -r c5e4bc10485b -r 2cd067e01788 pkgtools/pkglint/files/licenses.go
--- a/pkgtools/pkglint/files/licenses.go        Wed Apr 03 19:10:26 2019 +0000
+++ b/pkgtools/pkglint/files/licenses.go        Wed Apr 03 21:49:51 2019 +0000
@@ -3,11 +3,12 @@
 import "netbsd.org/pkglint/licenses"
 
 type LicenseChecker struct {
-       MkLine MkLine
+       MkLines MkLines
+       MkLine  MkLine
 }
 
 func (lc *LicenseChecker) Check(value string, op MkOperator) {
-       expanded := resolveVariableRefs(value) // For ${PERL5_LICENSE}
+       expanded := resolveVariableRefs(lc.MkLines, value) // For ${PERL5_LICENSE}
        cond := licenses.Parse(ifelseStr(op == opAssignAppend, "append-placeholder ", "") + expanded)
 
        if cond == nil {
diff -r c5e4bc10485b -r 2cd067e01788 pkgtools/pkglint/files/licenses_test.go
--- a/pkgtools/pkglint/files/licenses_test.go   Wed Apr 03 19:10:26 2019 +0000
+++ b/pkgtools/pkglint/files/licenses_test.go   Wed Apr 03 21:49:51 2019 +0000
@@ -11,7 +11,7 @@
                "The licenses for most software are designed to take away ...")
        mkline := t.NewMkLine("Makefile", 7, "LICENSE=dummy")
 
-       licenseChecker := LicenseChecker{mkline}
+       licenseChecker := LicenseChecker{nil, mkline}
        licenseChecker.Check("gpl-v2", opAssign)
 
        t.CheckOutputLines(
diff -r c5e4bc10485b -r 2cd067e01788 pkgtools/pkglint/files/mkline.go
--- a/pkgtools/pkglint/files/mkline.go  Wed Apr 03 19:10:26 2019 +0000
+++ b/pkgtools/pkglint/files/mkline.go  Wed Apr 03 21:49:51 2019 +0000
@@ -423,47 +423,55 @@
 
 var notSpace = textproc.Space.Inverse()
 
-// ValueFields splits the given value, taking care of variable references.
-// Example:
+// ValueFields splits the given value in the same way as the :M variable
+// modifier, taking care of variable references. Example:
 //
-//  ValueFields("${VAR:Udefault value} ${VAR2}two words")
+//  ValueFields("${VAR:Udefault value} ${VAR2}two words;;; 'word three'")
 //  => "${VAR:Udefault value}"
 //     "${VAR2}two"
-//     "words"
+//     "words;;;"
+//     "'word three'"
 //
 // Note that even though the first word contains a space, it is not split
-// at that point since the space is inside a variable use.
+// at that point since the space is inside a variable use. Shell tokens
+// such as semicolons are also treated as normal characters. Only double
+// and single quotes are interpreted.
+//
+// Compare devel/bmake/files/str.c, function brk_string.
+//
+// TODO: Compare with brk_string from devel/bmake, especially for backticks.
 func (mkline *MkLineImpl) ValueFields(value string) []string {
-       tokens := mkline.Tokenize(value, false)
-       var split []string
-       cont := false
+       if trace.Tracing {
+               defer trace.Call(mkline, value)()
+       }
 
-       out := func(s string) {
-               if cont {
-                       split[len(split)-1] += s
-               } else {
-                       split = append(split, s)
-               }
+       p := NewShTokenizer(mkline.Line, value, false)
+       atoms := p.ShAtoms()
+
+       if len(atoms) > 0 && atoms[0].Type == shtSpace {
+               atoms = atoms[1:]
        }
 
-       for _, token := range tokens {
-               if token.Varuse != nil {
-                       out(token.Text)
-                       cont = true
+       word := ""
+       var words []string
+       for _, atom := range atoms {
+               if atom.Type == shtSpace && atom.Quoting == shqPlain {
+                       words = append(words, word)
+                       word = ""
                } else {
-                       lexer := textproc.NewLexer(token.Text)
-                       for !lexer.EOF() {
-                               for lexer.NextBytesSet(textproc.Space) != "" {
-                                       cont = false
-                               }
-                               if word := lexer.NextBytesSet(notSpace); word != "" {
-                                       out(word)
-                                       cont = true
-                               }
-                       }
+                       word += atom.MkText
                }
        }
-       return split
+       if word != "" && atoms[len(atoms)-1].Quoting == shqPlain {
+               words = append(words, word)
+               word = ""
+       }
+
+       // TODO: Handle parse errors
+       rest := word + p.parser.Rest()
+       _ = rest
+
+       return words
 }
 
 func (mkline *MkLineImpl) ValueTokens() ([]*MkToken, string) {
@@ -748,6 +756,12 @@
                mainWithSpaces := main
                main = rtrimHspace(main)
                spaceBeforeComment = mainWithSpaces[len(main):]
+       } else {
+               restWithoutSpace := strings.TrimRightFunc(rest, func(r rune) bool { return isHspace(byte(r)) })
+               if len(restWithoutSpace) < len(rest) {
+                       spaceBeforeComment = rest[len(restWithoutSpace):]
+                       rest = restWithoutSpace
+               }
        }
 
        return
@@ -797,9 +811,9 @@
 // This decision depends on many factors, such as whether the type of the context is
 // a list of things, whether the variable is a list, whether it can contain only
 // safe characters, and so on.
-func (mkline *MkLineImpl) VariableNeedsQuoting(varname string, vartype *Vartype, vuc *VarUseContext) (needsQuoting YesNoUnknown) {
+func (mkline *MkLineImpl) VariableNeedsQuoting(mklines MkLines, varuse *MkVarUse, vartype *Vartype, vuc *VarUseContext) (needsQuoting YesNoUnknown) {
        if trace.Tracing {
-               defer trace.Call(varname, vartype, vuc, trace.Result(&needsQuoting))()
+               defer trace.Call(varuse, vartype, vuc, trace.Result(&needsQuoting))()
        }
 
        // TODO: Systematically test this function, each and every case, from top to bottom.
@@ -845,7 +859,7 @@
 
        // Pkglint assumes that the tool definitions don't include very
        // special characters, so they can safely be used inside any quotes.
-       if tool := G.ToolByVarname(varname); tool != nil {
+       if tool := G.ToolByVarname(mklines, varuse.varname); tool != nil {
                switch vuc.quoting {
                case VucQuotPlain:
                        if !vuc.IsWordPart {



Home | Main Index | Thread Index | Old Index