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



details:   https://anonhg.NetBSD.org/pkgsrc/rev/28809f93995e
branches:  trunk
changeset: 345012:28809f93995e
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Sat Nov 30 20:35:11 2019 +0000

description:
pkgtools/pkglint: update to 19.3.12

Changes since 19.3.11:

The command line option -Wstyle has been removed since it didn't have
any effect.

License names may contain underscores. This fixes 3 parse errors and 2
wrong notes about seemingly unused licenses.

The parser for Makefile variables has been improved for some edge cases.
The :M and :N modifiers behave surprisingly when they contain unbalanced
parentheses or braces. Pkglint now parses them in the same way as bmake,
but doesn't warn since these cases are not actually used in pkgsrc.

diffstat:

 pkgtools/pkglint/Makefile                        |    4 +-
 pkgtools/pkglint/files/autofix.go                |    4 +-
 pkgtools/pkglint/files/autofix_test.go           |   13 +-
 pkgtools/pkglint/files/category.go               |    2 +-
 pkgtools/pkglint/files/distinfo.go               |    4 +-
 pkgtools/pkglint/files/licenses/licenses.go      |    2 +-
 pkgtools/pkglint/files/licenses/licenses_test.go |   35 ++++
 pkgtools/pkglint/files/mklexer.go                |  123 ++++++++++++----
 pkgtools/pkglint/files/mklexer_test.go           |  168 +++++++++++++++++++++++
 pkgtools/pkglint/files/mkline.go                 |    9 +-
 pkgtools/pkglint/files/mklineparser.go           |    2 +-
 pkgtools/pkglint/files/mkparser.go               |    2 +-
 pkgtools/pkglint/files/mktokenslexer.go          |    2 +-
 pkgtools/pkglint/files/package.go                |    8 +-
 pkgtools/pkglint/files/path.go                   |    7 +-
 pkgtools/pkglint/files/path_test.go              |   31 ++++-
 pkgtools/pkglint/files/pkglint.0                 |    5 +-
 pkgtools/pkglint/files/pkglint.1                 |    6 +-
 pkgtools/pkglint/files/pkglint.go                |   17 +-
 pkgtools/pkglint/files/pkglint_test.go           |    1 -
 pkgtools/pkglint/files/pkgsrc.go                 |   10 +-
 pkgtools/pkglint/files/pkgsrc_test.go            |    2 +-
 pkgtools/pkglint/files/shtokenizer_test.go       |   12 +
 pkgtools/pkglint/files/textproc/lexer.go         |    3 +-
 pkgtools/pkglint/files/toplevel.go               |    2 +-
 pkgtools/pkglint/files/util.go                   |  140 ++++++++++--------
 pkgtools/pkglint/files/util_test.go              |  162 ++++++++++++----------
 27 files changed, 555 insertions(+), 221 deletions(-)

diffs (truncated from 1279 to 300 lines):

diff -r c176563afbbb -r 28809f93995e pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Sat Nov 30 19:59:34 2019 +0000
+++ b/pkgtools/pkglint/Makefile Sat Nov 30 20:35:11 2019 +0000
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.611 2019/11/27 22:10:06 rillig Exp $
+# $NetBSD: Makefile,v 1.612 2019/11/30 20:35:11 rillig Exp $
 
-PKGNAME=       pkglint-19.3.11
+PKGNAME=       pkglint-19.3.12
 CATEGORIES=    pkgtools
 DISTNAME=      tools
 MASTER_SITES=  ${MASTER_SITE_GITHUB:=golang/}
diff -r c176563afbbb -r 28809f93995e pkgtools/pkglint/files/autofix.go
--- a/pkgtools/pkglint/files/autofix.go Sat Nov 30 19:59:34 2019 +0000
+++ b/pkgtools/pkglint/files/autofix.go Sat Nov 30 20:35:11 2019 +0000
@@ -442,8 +442,8 @@
        }
 
        if G.Testing {
-               abs := abspath(lines.Filename)
-               absTmp := abspath(NewPathSlash(os.TempDir()))
+               abs := G.Abs(lines.Filename)
+               absTmp := G.Abs(NewPathSlash(os.TempDir()))
                assertf(abs.HasPrefixPath(absTmp), "%q must be inside %q", abs, absTmp)
        }
 
diff -r c176563afbbb -r 28809f93995e pkgtools/pkglint/files/autofix_test.go
--- a/pkgtools/pkglint/files/autofix_test.go    Sat Nov 30 19:59:34 2019 +0000
+++ b/pkgtools/pkglint/files/autofix_test.go    Sat Nov 30 20:35:11 2019 +0000
@@ -578,8 +578,7 @@
                "AUTOFIX: filename.mk:1: Replacing \"=\" with \"+=\".")
 
        // If the text at the precisely given position does not match,
-       // the note is still printed because of the fix.Anyway(), but
-       // nothing is replaced automatically.
+       // the note is still printed, but nothing is replaced automatically.
        test(
                lines(
                        "VAR=value1 \\",
@@ -997,9 +996,9 @@
                "AUTOFIX: filename:5: Replacing \"text\" with \"replacement\".")
 }
 
-// In --autofix mode or --show-autofix mode, the fix.Anyway doesn't
-// have any effect, therefore the errors from such autofixes are
-// not counted, and the exitcode stays at 0.
+// In --autofix mode or --show-autofix mode, those errors that have
+// been automatically fixed are not counted, and the others are filtered
+// out, therefore the exitcode stays at 0.
 func (s *Suite) Test_Autofix_Apply__anyway_error(c *check.C) {
        t := s.Init(c)
 
@@ -1036,8 +1035,8 @@
        // Nothing is replaced since, as of June 2019, pkglint doesn't
        // know which of the three "word" should be replaced.
        //
-       // The note is not logged since fix.Anyway only applies when neither
-       // --show-autofix nor --autofix is given in the command line.
+       // The note is not logged since --show-autofix nor --autofix is
+       // given in the command line.
        t.CheckOutputEmpty()
        t.CheckFileLines("filename",
                "word word word")
diff -r c176563afbbb -r 28809f93995e pkgtools/pkglint/files/category.go
--- a/pkgtools/pkglint/files/category.go        Sat Nov 30 19:59:34 2019 +0000
+++ b/pkgtools/pkglint/files/category.go        Sat Nov 30 20:35:11 2019 +0000
@@ -158,7 +158,7 @@
                var recurseInto []Path
                for _, msub := range mSubdirs {
                        if !msub.line.IsCommentedVarassign() {
-                               recurseInto = append(recurseInto, joinPath(dir, msub.name))
+                               recurseInto = append(recurseInto, dir.JoinNoClean(msub.name))
                        }
                }
                G.Todo.PushFront(recurseInto...)
diff -r c176563afbbb -r 28809f93995e pkgtools/pkglint/files/distinfo.go
--- a/pkgtools/pkglint/files/distinfo.go        Sat Nov 30 19:59:34 2019 +0000
+++ b/pkgtools/pkglint/files/distinfo.go        Sat Nov 30 20:35:11 2019 +0000
@@ -194,7 +194,7 @@
 
        distdir := G.Pkgsrc.File("distfiles")
 
-       distfile := cleanpath(joinPath(distdir, info.filename()))
+       distfile := cleanpath(distdir.JoinNoClean(info.filename()))
        if !distfile.IsFile() {
 
                // It's a rare situation that the explanation is generated
@@ -370,7 +370,7 @@
        hash := info.hash
        line := info.line
 
-       patchFileName := joinPath(ck.patchdir, patchName)
+       patchFileName := ck.patchdir.JoinNoClean(patchName)
        resolvedPatchFileName := ck.pkg.File(patchFileName)
        if ck.distinfoIsCommitted && !isCommitted(resolvedPatchFileName) {
                line.Warnf("%s is registered in distinfo but not added to CVS.", line.PathToFile(resolvedPatchFileName))
diff -r c176563afbbb -r 28809f93995e pkgtools/pkglint/files/licenses/licenses.go
--- a/pkgtools/pkglint/files/licenses/licenses.go       Sat Nov 30 19:59:34 2019 +0000
+++ b/pkgtools/pkglint/files/licenses/licenses.go       Sat Nov 30 20:35:11 2019 +0000
@@ -67,7 +67,7 @@
        error  string
 }
 
-var licenseNameChars = textproc.NewByteSet("A-Za-z0-9---.")
+var licenseNameChars = textproc.NewByteSet("A-Za-z0-9---._")
 
 func (lexer *licenseLexer) Lex(llval *liyySymType) int {
        lex := lexer.lexer
diff -r c176563afbbb -r 28809f93995e pkgtools/pkglint/files/licenses/licenses_test.go
--- a/pkgtools/pkglint/files/licenses/licenses_test.go  Sat Nov 30 19:59:34 2019 +0000
+++ b/pkgtools/pkglint/files/licenses/licenses_test.go  Sat Nov 30 20:35:11 2019 +0000
@@ -4,6 +4,7 @@
        "encoding/json"
        "gopkg.in/check.v1"
        "netbsd.org/pkglint/intqa"
+       "netbsd.org/pkglint/textproc"
        "strings"
        "testing"
 )
@@ -22,6 +23,7 @@
        testDeep("gnu-gpl-v2", NewName("gnu-gpl-v2"))
 
        test("gnu-gpl-v2", "{Name:gnu-gpl-v2}")
+       test("citrix_ica-license", "{Name:citrix_ica-license}")
 
        test("a AND b", "{And:true,Children:[{Name:a},{Name:b}]}")
        test("a OR b", "{Or:true,Children:[{Name:a},{Name:b}]}")
@@ -108,6 +110,39 @@
        c.Check(out, check.DeepEquals, []string{"a", "b", "OR", "()", "c", "d", "AND", "()", "AND"})
 }
 
+func (s *Suite) Test_licenseLexer_Lex(c *check.C) {
+
+       test := func(text string, tokenType int) {
+               lexer := &licenseLexer{lexer: textproc.NewLexer(text)}
+               var token liyySymType
+               lex := lexer.Lex(&token)
+               c.Check(lex, check.Equals, tokenType)
+       }
+       testName := func(text string, name string) {
+               lexer := &licenseLexer{lexer: textproc.NewLexer(text)}
+               var token liyySymType
+               lex := lexer.Lex(&token)
+               c.Check(lex, check.Equals, ltNAME)
+               c.Check(token.Node.Name, check.Equals, name)
+       }
+
+       test("", 0)
+       test("(", ltOPEN)
+       test(")", ltCLOSE)
+       test("AND", ltAND)
+       test("OR", ltOR)
+       test("?", -1)
+       test("license-name", ltNAME)
+       test("license_name", ltNAME)
+
+       test("AND rest", ltAND)
+       test("AND-rest", ltNAME)
+
+       testName("license-name", "license-name")
+       testName("license_name", "license_name")
+       testName("AND-rest", "AND-rest")
+}
+
 func NewName(name string) *Condition {
        return &Condition{Name: name}
 }
diff -r c176563afbbb -r 28809f93995e pkgtools/pkglint/files/mklexer.go
--- a/pkgtools/pkglint/files/mklexer.go Sat Nov 30 19:59:34 2019 +0000
+++ b/pkgtools/pkglint/files/mklexer.go Sat Nov 30 20:35:11 2019 +0000
@@ -8,6 +8,15 @@
 
 // MkLexer splits a text into a sequence of variable uses
 // and plain text.
+//
+// The actual parsing algorithm in devel/bmake/files/var.c differs from
+// pkglint's parser in many ways and produces different results in
+// almost all edge cases. See devel/bmake/files/var.c:/'\\\\'/.
+//
+// The pkglint parser had been built from scratch using naive assumptions
+// about how bmake parses these expressions. These assumptions do not hold
+// a strict test, but luckily the pkgsrc package developers don't explore
+// these edge cases anyway.
 type MkLexer struct {
        lexer *textproc.Lexer
        line  *Line
@@ -153,10 +162,12 @@
 // This is used for the :L and :? modifiers since they accept arbitrary
 // text as the "variable name" and effectively interpret it as the variable
 // value instead.
+//
+// See devel/bmake/files/var.c:/^VarGetPattern/
 func (p *MkLexer) varUseText(closing byte) string {
        lexer := p.lexer
        start := lexer.Mark()
-       re := regcomp(regex.Pattern(condStr(closing == '}', `^([^$:}]|\$\$)+`, `^([^$:)]|\$\$)+`)))
+       re := regcomp(regex.Pattern(condStr(closing == '}', `^([^$:\\}]|\$\$|\\.)+`, `^([^$:\\)]|\$\$|\\.)+`)))
        for p.VarUse() != nil || lexer.SkipRegexp(re) {
        }
        return lexer.Since(start)
@@ -213,34 +224,14 @@
                }
 
                if hasPrefix(mod, "ts") {
-                       // See devel/bmake/files/var.c:/case 't'
-                       sep := mod[2:] + p.varUseText(closing)
-                       switch {
-                       case sep == "":
-                               lexer.SkipString(":")
-                       case len(sep) == 1:
-                               break
-                       case matches(sep, `^\\\d+`):
-                               break
-                       default:
-                               if p.line != nil {
-                                       p.line.Warnf("Invalid separator %q for :ts modifier of %q.", sep, varname)
-                                       p.line.Explain(
-                                               "The separator for the :ts modifier must be either a single character",
-                                               "or an escape sequence like \\t or \\n or an octal or decimal escape",
-                                               "sequence; see the bmake man page for further details.")
-                               }
-                       }
-                       return lexer.Since(mark)
+                       return p.varUseModifierSeparator(mod, closing, lexer, varname, mark)
                }
 
-       case '=', 'D', 'M', 'N', 'U':
-               lexer.Skip(1)
-               re := regcomp(regex.Pattern(condStr(closing == '}', `^([^$:\\}]|\$\$|\\.)+`, `^([^$:\\)]|\$\$|\\.)+`)))
-               for p.VarUse() != nil || lexer.SkipRegexp(re) {
-               }
-               arg := lexer.Since(mark)
-               return strings.Replace(arg, "\\:", ":", -1)
+       case 'D', 'U':
+               return p.varUseText(closing)
+
+       case 'M', 'N':
+               return p.varUseModifierMatch(closing)
 
        case 'C', 'S':
                if ok, _, _, _, _ := p.varUseModifierSubst(closing); ok {
@@ -268,10 +259,7 @@
 
        lexer.Reset(mark)
 
-       re := regcomp(regex.Pattern(condStr(closing == '}', `^([^:$}]|\$\$)+`, `^([^:$)]|\$\$)+`)))
-       for p.VarUse() != nil || lexer.SkipRegexp(re) {
-       }
-       modifier := lexer.Since(mark)
+       modifier := p.varUseText(closing)
 
        // ${SOURCES:%.c=%.o} or ${:!uname -a!:[2]}
        if contains(modifier, "=") || (hasPrefix(modifier, "!") && hasSuffix(modifier, "!")) {
@@ -285,6 +273,79 @@
        return ""
 }
 
+// varUseModifierSeparator parses the :ts modifier.
+//
+// The API of this method is tricky.
+// It is only extracted from varUseModifier to make the latter smaller.
+func (p *MkLexer) varUseModifierSeparator(
+       mod string, closing byte, lexer *textproc.Lexer, varname string,
+       mark textproc.LexerMark) string {
+
+       // See devel/bmake/files/var.c:/case 't'
+       sep := mod[2:] + p.varUseText(closing)
+       switch {
+       case sep == "":
+               lexer.SkipString(":")
+       case len(sep) == 1:
+               break
+       case matches(sep, `^\\\d+`):
+               break
+       default:
+               if p.line != nil {
+                       p.line.Warnf("Invalid separator %q for :ts modifier of %q.", sep, varname)
+                       p.line.Explain(
+                               "The separator for the :ts modifier must be either a single character",
+                               "or an escape sequence like \\t or \\n or an octal or decimal escape",
+                               "sequence; see the bmake man page for further details.")
+               }
+       }
+       return lexer.Since(mark)
+}
+
+// varUseModifierMatch parses an :M or :N pattern.
+//
+// See devel/bmake/files/var.c:/^ApplyModifiers/, case 'M'.
+func (p *MkLexer) varUseModifierMatch(closing byte) string {
+       lexer := p.lexer
+       mark := lexer.Mark()
+       lexer.Skip(1)
+       opening := byte(condInt(closing == '}', '{', '('))
+       _ = opening
+
+       nest := 1
+       seenBackslash := false
+       for !lexer.EOF() {
+               ch := lexer.PeekByte()



Home | Main Index | Thread Index | Old Index