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.6.8



details:   https://anonhg.NetBSD.org/pkgsrc/rev/a7abd250d2f1
branches:  trunk
changeset: 388282:a7abd250d2f1
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Sun Dec 02 23:12:43 2018 +0000

description:
pkgtools/pkglint: update to 5.6.8

Changes since 5.6.7:

In pkgsrc-wip, if the first line of a file contains an expanded CVS Id,
it is not an error but only a note that it should be an unexpanded CVS
Id. The autofix for this no longer inserts a new line but replaces the
existing line.

Several refactorings and small improvements to the existing diagnostics.

diffstat:

 pkgtools/pkglint/Makefile                       |    4 +-
 pkgtools/pkglint/files/autofix.go               |    7 +-
 pkgtools/pkglint/files/buildlink3_test.go       |    4 +-
 pkgtools/pkglint/files/category_test.go         |    2 +-
 pkgtools/pkglint/files/licenses.go              |    2 +-
 pkgtools/pkglint/files/line.go                  |    2 +
 pkgtools/pkglint/files/lines.go                 |    4 +-
 pkgtools/pkglint/files/lines_test.go            |   12 +-
 pkgtools/pkglint/files/mkline.go                |   55 ++++-
 pkgtools/pkglint/files/mkline_test.go           |    4 +-
 pkgtools/pkglint/files/mklinechecker.go         |  141 ++++++++-----
 pkgtools/pkglint/files/mklinechecker_test.go    |  238 ++++++++++++++++-------
 pkgtools/pkglint/files/mklines.go               |  188 +++++++++++-------
 pkgtools/pkglint/files/mklines_test.go          |  143 +++++++++-----
 pkgtools/pkglint/files/mklines_varalign_test.go |    2 +-
 pkgtools/pkglint/files/package.go               |    8 +-
 pkgtools/pkglint/files/package_test.go          |    4 +-
 pkgtools/pkglint/files/pkglint.go               |    4 +-
 pkgtools/pkglint/files/pkgsrc.go                |    4 +-
 pkgtools/pkglint/files/substcontext.go          |    2 +-
 pkgtools/pkglint/files/tools_test.go            |   14 +-
 pkgtools/pkglint/files/vardefs.go               |    3 +-
 pkgtools/pkglint/files/vartype.go               |    5 +
 pkgtools/pkglint/files/vartypecheck_test.go     |   16 +-
 24 files changed, 565 insertions(+), 303 deletions(-)

diffs (truncated from 2010 to 300 lines):

diff -r e92739c7d0bd -r a7abd250d2f1 pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Sun Dec 02 21:51:06 2018 +0000
+++ b/pkgtools/pkglint/Makefile Sun Dec 02 23:12:43 2018 +0000
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.559 2018/12/02 01:57:48 rillig Exp $
+# $NetBSD: Makefile,v 1.560 2018/12/02 23:12:43 rillig Exp $
 
-PKGNAME=       pkglint-5.6.7
+PKGNAME=       pkglint-5.6.8
 CATEGORIES=    pkgtools
 DISTNAME=      tools
 MASTER_SITES=  ${MASTER_SITE_GITHUB:=golang/}
diff -r e92739c7d0bd -r a7abd250d2f1 pkgtools/pkglint/files/autofix.go
--- a/pkgtools/pkglint/files/autofix.go Sun Dec 02 21:51:06 2018 +0000
+++ b/pkgtools/pkglint/files/autofix.go Sun Dec 02 23:12:43 2018 +0000
@@ -303,6 +303,10 @@
 }
 
 func (fix *Autofix) Realign(mkline MkLine, newWidth int) {
+
+       // XXX: Check whether this method can be implemented as Custom fix.
+       // This complicated code should not be in the Autofix type.
+
        fix.assertRealLine()
        G.Assertf(mkline.IsMultiline(), "Line must be a multiline.")
        G.Assertf(mkline.IsVarassign() || mkline.IsCommentedVarassign(), "Line must be a variable assignment.")
@@ -315,8 +319,7 @@
        oldWidth := 0      // The minimum required indentation in the original lines.
 
        {
-               // Parsing the continuation marker as variable value
-               // is cheating but works well.
+               // Parsing the continuation marker as variable value is cheating but works well.
                text := strings.TrimSuffix(mkline.raw[0].orignl, "\n")
                m, _, _, _, _, valueAlign, value, _, _ := MatchVarassign(text)
                if m && value != "\\" {
diff -r e92739c7d0bd -r a7abd250d2f1 pkgtools/pkglint/files/buildlink3_test.go
--- a/pkgtools/pkglint/files/buildlink3_test.go Sun Dec 02 21:51:06 2018 +0000
+++ b/pkgtools/pkglint/files/buildlink3_test.go Sun Dec 02 23:12:43 2018 +0000
@@ -8,7 +8,7 @@
        t.SetupVartypes()
        t.CreateFileLines("x11/Xbae/Makefile")
        t.CreateFileLines("mk/motif.buildlink3.mk")
-       mklines := t.SetupFileMkLines("buildlink3.mk",
+       mklines := t.SetupFileMkLines("category/package/buildlink3.mk",
                MkRcsID,
                "# XXX This file was created automatically using createbuildlink-@PKGVERSION@",
                "",
@@ -30,7 +30,7 @@
        ChecklinesBuildlink3Mk(mklines)
 
        t.CheckOutputLines(
-               "ERROR: ~/buildlink3.mk:2: This comment indicates unfinished work (url2pkg).")
+               "ERROR: ~/category/package/buildlink3.mk:2: This comment indicates unfinished work (url2pkg).")
 }
 
 // Before version 5.3, pkglint wrongly warned here.
diff -r e92739c7d0bd -r a7abd250d2f1 pkgtools/pkglint/files/category_test.go
--- a/pkgtools/pkglint/files/category_test.go   Sun Dec 02 21:51:06 2018 +0000
+++ b/pkgtools/pkglint/files/category_test.go   Sun Dec 02 23:12:43 2018 +0000
@@ -23,7 +23,7 @@
                "NOTE: ~/archivers/Makefile:2: This variable value should be aligned to column 17.",
                "NOTE: ~/archivers/Makefile:3: This variable value should be aligned with tabs, not spaces, to column 17.",
                "NOTE: ~/archivers/Makefile:4: This variable value should be aligned to column 17.",
-               "ERROR: ~/archivers/Makefile:6: \"../mk/category.mk\" does not exist.",
+               "ERROR: ~/archivers/Makefile:6: Relative path \"../mk/category.mk\" does not exist.",
                "NOTE: ~/archivers/Makefile:1: Empty line expected after this line.",
                "ERROR: ~/archivers/Makefile:2: COMMENT= line expected.",
                "NOTE: ~/archivers/Makefile:1: Empty line expected after this line.", // XXX: Duplicate.
diff -r e92739c7d0bd -r a7abd250d2f1 pkgtools/pkglint/files/licenses.go
--- a/pkgtools/pkglint/files/licenses.go        Sun Dec 02 21:51:06 2018 +0000
+++ b/pkgtools/pkglint/files/licenses.go        Sun Dec 02 23:12:43 2018 +0000
@@ -26,7 +26,7 @@
        licenseFile := ""
        if G.Pkg != nil {
                if mkline := G.Pkg.vars.FirstDefinition("LICENSE_FILE"); mkline != nil {
-                       licenseFile = G.Pkg.File(mkline.ResolveVarsInRelativePath(mkline.Value(), false))
+                       licenseFile = G.Pkg.File(mkline.ResolveVarsInRelativePath(mkline.Value()))
                }
        }
        if licenseFile == "" {
diff -r e92739c7d0bd -r a7abd250d2f1 pkgtools/pkglint/files/line.go
--- a/pkgtools/pkglint/files/line.go    Sun Dec 02 21:51:06 2018 +0000
+++ b/pkgtools/pkglint/files/line.go    Sun Dec 02 23:12:43 2018 +0000
@@ -35,6 +35,8 @@
 type Line = *LineImpl
 
 type LineImpl struct {
+       // TODO: Consider storing pointers to the Filename and Basename instead of strings to save memory.
+       // But first find out where and why pkglint needs so much memory (200 MB for a full recursive run over pkgsrc + wip).
        Filename  string // uses / as directory separator on all platforms
        Basename  string // the last component of the Filename
        firstLine int32  // zero means the whole file, -1 means EOF
diff -r e92739c7d0bd -r a7abd250d2f1 pkgtools/pkglint/files/lines.go
--- a/pkgtools/pkglint/files/lines.go   Sun Dec 02 21:51:06 2018 +0000
+++ b/pkgtools/pkglint/files/lines.go   Sun Dec 02 23:12:43 2018 +0000
@@ -45,7 +45,7 @@
 
                if G.Wip && expanded != "" {
                        fix := line.Autofix()
-                       fix.Errorf("Expected exactly %q.", suggestedPrefix+"$"+"NetBSD$")
+                       fix.Notef("Expected exactly %q.", suggestedPrefix+"$"+"NetBSD$")
                        fix.Explain(
                                "Several files in pkgsrc must contain the CVS Id, so that their",
                                "current version can be traced back later from a binary package.",
@@ -59,7 +59,7 @@
                                "",
                                "To preserve the history of the CVS Id, should that ever be needed,",
                                "remove the leading $.")
-                       fix.InsertBefore(suggestedPrefix + "$" + "NetBSD$")
+                       fix.ReplaceRegex(`.*`, suggestedPrefix+"$"+"NetBSD$", 1)
                        fix.Apply()
                }
 
diff -r e92739c7d0bd -r a7abd250d2f1 pkgtools/pkglint/files/lines_test.go
--- a/pkgtools/pkglint/files/lines_test.go      Sun Dec 02 21:51:06 2018 +0000
+++ b/pkgtools/pkglint/files/lines_test.go      Sun Dec 02 23:12:43 2018 +0000
@@ -47,8 +47,18 @@
        G.CheckDirent(t.File("wip/package"))
 
        t.CheckOutputLines(
-               "ERROR: ~/wip/package/file1.mk:1: Expected exactly \"# $"+"NetBSD$\".",
+               "NOTE: ~/wip/package/file1.mk:1: Expected exactly \"# $"+"NetBSD$\".",
                "ERROR: ~/wip/package/file3.mk:1: Expected \"# $"+"NetBSD$\".",
                "ERROR: ~/wip/package/file4.mk:1: Expected \"# $"+"NetBSD$\".",
                "ERROR: ~/wip/package/file5.mk:1: Expected \"# $"+"NetBSD$\".")
+
+       G.Logger.Opts.Autofix = true
+
+       G.CheckDirent(t.File("wip/package"))
+
+       t.CheckOutputLines(
+               "AUTOFIX: ~/wip/package/file1.mk:1: Replacing \"# $NetBSD: lines_test.go,v 1.2 2018/12/02 23:12:43 rillig Exp $\" with \"# $NetBSD: lines_test.go,v 1.2 2018/12/02 23:12:43 rillig Exp 
$\".",
+               "AUTOFIX: ~/wip/package/file3.mk:1: Inserting a line \"# $NetBSD: lines_test.go,v 1.2 2018/12/02 23:12:43 rillig Exp $\" before this line.",
+               "AUTOFIX: ~/wip/package/file4.mk:1: Inserting a line \"# $NetBSD: lines_test.go,v 1.2 2018/12/02 23:12:43 rillig Exp $\" before this line.",
+               "AUTOFIX: ~/wip/package/file5.mk:1: Inserting a line \"# $NetBSD: lines_test.go,v 1.2 2018/12/02 23:12:43 rillig Exp $\" before this line.")
 }
diff -r e92739c7d0bd -r a7abd250d2f1 pkgtools/pkglint/files/mkline.go
--- a/pkgtools/pkglint/files/mkline.go  Sun Dec 02 21:51:06 2018 +0000
+++ b/pkgtools/pkglint/files/mkline.go  Sun Dec 02 23:12:43 2018 +0000
@@ -31,6 +31,7 @@
        value       string     // The trimmed value
        valueMk     []*MkToken // The value, sent through splitIntoMkWords
        valueMkRest string     // nonempty in case of parse errors
+       fields      []string   // The value, space-separated according to shell quoting rules
        comment     string
 }
 type mkLineShell struct {
@@ -43,9 +44,10 @@
        indent    string // the space between the leading "." and the directive
        directive string // "if", "else", "for", etc.
        args      string
-       comment   string // mainly interesting for .endif and .endfor
-       elseLine  MkLine // for .if (filled in later)
-       cond      MkCond // for .if and .elif (filled in later as needed)
+       comment   string   // mainly interesting for .endif and .endfor
+       elseLine  MkLine   // for .if (filled in later)
+       cond      MkCond   // for .if and .elif (filled in on first access)
+       fields    []string // the arguments for the .for loop (filled in on first access)
 }
 type mkLineInclude = *mkLineIncludeImpl // See https://github.com/golang/go/issues/28045
 type mkLineIncludeImpl struct {
@@ -110,6 +112,7 @@
                        strings.Replace(value, "\\#", "#", -1),
                        nil,
                        "",
+                       nil,
                        comment}}
        }
 
@@ -128,7 +131,7 @@
        }
 
        if m, indent, directive, args, comment := matchMkDirective(text); m {
-               return &MkLineImpl{line, &mkLineDirectiveImpl{indent, directive, args, comment, nil, nil}}
+               return &MkLineImpl{line, &mkLineDirectiveImpl{indent, directive, args, comment, nil, nil, nil}}
        }
 
        if m, indent, directive, includedFile := MatchMkInclude(text); m {
@@ -472,6 +475,41 @@
        return assign.valueMk
 }
 
+// Fields applies to variable assignments and .for loops.
+// For variable assignments, it returns the right-hand side, properly split into words.
+// For .for loops, it returns all arguments (including variable names), properly split into words.
+func (mkline *MkLineImpl) Fields() []string {
+       if mkline.IsVarassign() {
+               value := mkline.Value()
+               if value == "" {
+                       return nil
+               }
+
+               assign := mkline.data.(mkLineAssign)
+               if assign.fields != nil {
+                       return assign.fields
+               }
+
+               assign.fields = mkline.ValueFields(value)
+               return assign.fields
+       }
+
+       // For .for loops.
+       args := mkline.Args()
+       if args == "" {
+               return nil
+       }
+
+       directive := mkline.data.(mkLineDirective)
+       if directive.fields != nil {
+               return directive.fields
+       }
+
+       directive.fields = mkline.ValueFields(args)
+       return directive.fields
+
+}
+
 func (mkline *MkLineImpl) WithoutMakeVariables(value string) string {
        valueNovar := ""
        for _, token := range NewMkParser(nil, value, false).MkTokens() {
@@ -482,7 +520,7 @@
        return valueNovar
 }
 
-func (mkline *MkLineImpl) ResolveVarsInRelativePath(relativePath string, adjustDepth bool) string {
+func (mkline *MkLineImpl) ResolveVarsInRelativePath(relativePath string) string {
 
        var basedir string
        if G.Pkg != nil {
@@ -526,13 +564,6 @@
                tmp = strings.Replace(tmp, "${PKGDIR}", G.Pkg.Pkgdir, -1)
        }
 
-       // TODO: What is this good for, and in which cases does it make a difference?
-       if adjustDepth {
-               if hasPrefix(tmp, "../../") && !hasPrefix(tmp[6:], ".") {
-                       tmp = pkgsrcdir + "/" + tmp[6:]
-               }
-       }
-
        tmp = cleanpath(tmp)
 
        if trace.Tracing && relativePath != tmp {
diff -r e92739c7d0bd -r a7abd250d2f1 pkgtools/pkglint/files/mkline_test.go
--- a/pkgtools/pkglint/files/mkline_test.go     Sun Dec 02 21:51:06 2018 +0000
+++ b/pkgtools/pkglint/files/mkline_test.go     Sun Dec 02 23:12:43 2018 +0000
@@ -395,7 +395,7 @@
                MkRcsID,
                "GENERATE_PLIST= cd ${DESTDIR}${PREFIX}; ${FIND} * \\( -type f -or -type l \\) | ${SORT};")
 
-       G.Mk.DetermineDefinedVariables()
+       G.Mk.collectDefinedVariables()
        MkLineChecker{G.Mk.mklines[1]}.Check()
 
        t.CheckOutputLines(
@@ -916,7 +916,7 @@
        mkline := mklines.mklines[0]
 
        checkResolve := func(before string, after string) {
-               c.Check(mkline.ResolveVarsInRelativePath(before, false), equals, after)
+               c.Check(mkline.ResolveVarsInRelativePath(before), equals, after)
        }
 
        checkResolve("", ".")
diff -r e92739c7d0bd -r a7abd250d2f1 pkgtools/pkglint/files/mklinechecker.go
--- a/pkgtools/pkglint/files/mklinechecker.go   Sun Dec 02 21:51:06 2018 +0000
+++ b/pkgtools/pkglint/files/mklinechecker.go   Sun Dec 02 23:12:43 2018 +0000
@@ -162,7 +162,7 @@
                ck.checkDirectiveFor(forVars, ind)
 
        case directive == "undef":
-               for _, varname := range fields(args) {
+               for _, varname := range mkline.Fields() {
                        if forVars[varname] {
                                mkline.Notef("Using \".undef\" after a \".for\" loop is unnecessary.")
                        }
@@ -203,8 +203,8 @@
 
                        if matches(forvar, `^[_a-z][_a-z0-9]*$`) {
                                // Fine.
-                       } else if strings.IndexFunc(forvar, func(r rune) bool { return 'A' <= r && r <= 'Z' }) != -1 {
-                               mkline.Warnf(".for variable names should not contain uppercase letters.")
+                       } else if matches(forvar, `^[A-Z_a-z][0-9A-Z_a-z]*$`) {
+                               mkline.Warnf("The variable name %q in the .for loop should not contain uppercase letters.", forvar)
                        } else {
                                mkline.Errorf("Invalid variable name %q.", forvar)
                        }
@@ -764,18 +764,18 @@
        }
 }
 
-func (ck MkLineChecker) checkVarassignDecreasingVersions(varname, value string) {
+func (ck MkLineChecker) checkVarassignDecreasingVersions() {
        if trace.Tracing {
-               defer trace.Call2(varname, value)()
+               defer trace.Call0()()
        }
 
        mkline := ck.MkLine



Home | Main Index | Thread Index | Old Index