Source-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 20.1.14



details:   https://anonhg.NetBSD.org/pkgsrc/rev/3a0e0d36bcec
branches:  trunk
changeset: 433654:3a0e0d36bcec
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Sat Jun 06 20:42:56 2020 +0000

description:
pkgtools/pkglint: update to 20.1.14

Changes since 20.1.13:

Packages that don't define DISTNAME probably don't download any files
and thus may not need a distinfo file.  (There are several other
conditions involved in this, though.)

When reporting wrong distinfo hashes, always report them in the order in
which they appear in the distinfo file, not by hashmap order.

Fix panic when parsing the Makefile line "./=value", which according to
bmake is a variable assignment.  This is not used in practice though.

Disallow a leading hyphen in package option names.  There are only very
few packages that wrongly use these option selectors in
PKG_SUGGESTED_OPTIONS.

Distinguish between a tool dependency (USE_TOOLS) and a plain tool name
(TOOLS_NOOP, TOOLS_BROKEN, TOOLS_FAIL).  Allow packages to add arbitrary
tools to these lists.

diffstat:

 pkgtools/pkglint/Makefile                   |    4 +-
 pkgtools/pkglint/files/buildlink3.go        |    5 +
 pkgtools/pkglint/files/buildlink3_test.go   |    6 +-
 pkgtools/pkglint/files/distinfo.go          |   10 +-
 pkgtools/pkglint/files/distinfo_test.go     |    4 +-
 pkgtools/pkglint/files/licenses.go          |   12 +-
 pkgtools/pkglint/files/licenses_test.go     |   14 ++
 pkgtools/pkglint/files/mklineparser.go      |   21 +--
 pkgtools/pkglint/files/mklineparser_test.go |    8 +-
 pkgtools/pkglint/files/mktokenslexer.go     |   23 ++++
 pkgtools/pkglint/files/package.go           |   70 ++++++++----
 pkgtools/pkglint/files/package_test.go      |   47 ++++++--
 pkgtools/pkglint/files/pkglint_test.go      |    1 -
 pkgtools/pkglint/files/util.go              |    8 +-
 pkgtools/pkglint/files/vardefs.go           |  149 ++++++++++++++-------------
 pkgtools/pkglint/files/vartype.go           |    6 +-
 pkgtools/pkglint/files/vartype_test.go      |    2 -
 pkgtools/pkglint/files/vartypecheck.go      |   47 ++++++--
 pkgtools/pkglint/files/vartypecheck_test.go |   59 +++++++---
 19 files changed, 319 insertions(+), 177 deletions(-)

diffs (truncated from 1096 to 300 lines):

diff -r 3f9bb4cd3428 -r 3a0e0d36bcec pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Sat Jun 06 19:09:37 2020 +0000
+++ b/pkgtools/pkglint/Makefile Sat Jun 06 20:42:56 2020 +0000
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.652 2020/06/02 17:52:26 rillig Exp $
+# $NetBSD: Makefile,v 1.653 2020/06/06 20:42:56 rillig Exp $
 
-PKGNAME=       pkglint-20.1.13
+PKGNAME=       pkglint-20.1.14
 CATEGORIES=    pkgtools
 DISTNAME=      tools
 MASTER_SITES=  ${MASTER_SITE_GITHUB:=golang/}
diff -r 3f9bb4cd3428 -r 3a0e0d36bcec pkgtools/pkglint/files/buildlink3.go
--- a/pkgtools/pkglint/files/buildlink3.go      Sat Jun 06 19:09:37 2020 +0000
+++ b/pkgtools/pkglint/files/buildlink3.go      Sat Jun 06 20:42:56 2020 +0000
@@ -276,6 +276,11 @@
                        mkline.Warnf("Only buildlink variables for %q, not %q may be set in this file.", pkgbase, varparam)
                }
        }
+
+       if varname == "pkgbase" && value != ck.pkgbase {
+               mkline.Errorf("A buildlink3.mk file must only query its own PKG_BUILD_OPTIONS.%s, not PKG_BUILD_OPTIONS.%s.",
+                       ck.pkgbase, value)
+       }
 }
 
 func (ck *Buildlink3Checker) checkVaruseInPkgbase(pkgbaseLine *MkLine) {
diff -r 3f9bb4cd3428 -r 3a0e0d36bcec pkgtools/pkglint/files/buildlink3_test.go
--- a/pkgtools/pkglint/files/buildlink3_test.go Sat Jun 06 19:09:37 2020 +0000
+++ b/pkgtools/pkglint/files/buildlink3_test.go Sat Jun 06 20:42:56 2020 +0000
@@ -732,10 +732,12 @@
 
        G.Check(t.File("category/package/buildlink3.mk"))
 
-       // TODO: Warn about PKG_BUILD_OPTIONS.package since that variable is not defined.
        t.CheckOutputLines(
                "WARN: ~/category/package/buildlink3.mk:15: "+
                        "PKG_BUILD_OPTIONS is used but not defined.",
+               "ERROR: ~/category/package/buildlink3.mk:12: "+
+                       "A buildlink3.mk file must only query its own PKG_BUILD_OPTIONS.package, "+
+                       "not PKG_BUILD_OPTIONS.unrelated.",
                "WARN: ~/category/package/buildlink3.mk:21: "+
                        "Wrong PKG_BUILD_OPTIONS, expected \"package\" instead of \"unrelated\".")
 }
@@ -743,7 +745,7 @@
 // As of October 2018, pkglint parses package dependencies a little
 // different than the pkg_* tools.
 // In all but two cases this works, this is one of the exceptions.
-// The "{totem,totem-xine}" cannot be parsed, therefore the check skipped.
+// The "{totem,totem-xine}" cannot be parsed, therefore the check is skipped.
 func (s *Suite) Test_Buildlink3Checker_checkVarassign__abi_api_versions_brace(c *check.C) {
        t := s.Init(c)
 
diff -r 3f9bb4cd3428 -r 3a0e0d36bcec pkgtools/pkglint/files/distinfo.go
--- a/pkgtools/pkglint/files/distinfo.go        Sat Jun 06 19:09:37 2020 +0000
+++ b/pkgtools/pkglint/files/distinfo.go        Sat Jun 06 20:42:56 2020 +0000
@@ -198,12 +198,12 @@
        for _, alg := range algorithms {
                missing[alg] = true
        }
-       seen := map[string]distinfoHash{}
+       seen := map[string]bool{}
 
        for _, hash := range info.hashes {
                alg := hash.algorithm
                if missing[alg] {
-                       seen[alg] = hash
+                       seen[alg] = true
                        delete(missing, alg)
                }
        }
@@ -275,7 +275,11 @@
                }
        }
 
-       for alg, hash := range seen {
+       for _, hash := range info.hashes {
+               alg := hash.algorithm
+               if !seen[alg] {
+                       continue
+               }
                computed := compute(alg)
 
                if computed != hash.hash {
diff -r 3f9bb4cd3428 -r 3a0e0d36bcec pkgtools/pkglint/files/distinfo_test.go
--- a/pkgtools/pkglint/files/distinfo_test.go   Sat Jun 06 19:09:37 2020 +0000
+++ b/pkgtools/pkglint/files/distinfo_test.go   Sat Jun 06 20:42:56 2020 +0000
@@ -149,7 +149,7 @@
 // PHP modules that are not PECL use the distinfo file from lang/php* but
 // their own patches directory. Therefore the distinfo file refers to missing
 // patches. Since this strange situation is caused by the pkgsrc
-// infrastructure, there is nothing a package author can do about.
+// infrastructure, there is nothing a package author can do.
 //
 // XXX: Re-check the documentation for this test.
 func (s *Suite) Test_distinfoLinesChecker_check__missing_php_patches(c *check.C) {
@@ -191,6 +191,8 @@
 
        G.Check(t.File("archivers/php-bz2"))
 
+       t.CheckOutputEmpty()
+
        t.CreateFileLines("archivers/php-zlib/Makefile",
                MkCvsID,
                "",
diff -r 3f9bb4cd3428 -r 3a0e0d36bcec pkgtools/pkglint/files/licenses.go
--- a/pkgtools/pkglint/files/licenses.go        Sat Jun 06 19:09:37 2020 +0000
+++ b/pkgtools/pkglint/files/licenses.go        Sat Jun 06 20:42:56 2020 +0000
@@ -28,10 +28,14 @@
        pkg := lc.MkLines.pkg
        if pkg != nil {
                if mkline := pkg.vars.FirstDefinition("LICENSE_FILE"); mkline != nil {
-                       // TODO: Not every path is relative to the package directory.
-                       rel := NewPackagePathString(mkline.Value())
-                       relResolved := mkline.ResolveVarsInRelativePath(rel, lc.MkLines.pkg)
-                       licenseFile = pkg.File(relResolved)
+                       value := mkline.Value()
+                       if NewPath(value).IsAbs() {
+                               mkline.Errorf("LICENSE_FILE must not be an absolute path.")
+                       } else {
+                               rel := NewPackagePathString(value)
+                               relResolved := mkline.ResolveVarsInRelativePath(rel, lc.MkLines.pkg)
+                               licenseFile = pkg.File(relResolved)
+                       }
                }
        }
        if licenseFile.IsEmpty() {
diff -r 3f9bb4cd3428 -r 3a0e0d36bcec pkgtools/pkglint/files/licenses_test.go
--- a/pkgtools/pkglint/files/licenses_test.go   Sat Jun 06 19:09:37 2020 +0000
+++ b/pkgtools/pkglint/files/licenses_test.go   Sat Jun 06 20:42:56 2020 +0000
@@ -68,3 +68,17 @@
        t.CheckOutputLines(
                "Looks fine.")
 }
+
+func (s *Suite) Test_LicenseChecker_checkName__LICENSE_FILE_absolute(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpPkgsrc()
+       t.SetUpPackage("category/package",
+               "LICENSE_FILE=\t/usr/license")
+
+       t.Main("category/package")
+
+       t.CheckOutputLines(
+               "ERROR: ~/category/package/Makefile:20: LICENSE_FILE must not be an absolute path.",
+               "1 error found.")
+}
diff -r 3f9bb4cd3428 -r 3a0e0d36bcec pkgtools/pkglint/files/mklineparser.go
--- a/pkgtools/pkglint/files/mklineparser.go    Sat Jun 06 19:09:37 2020 +0000
+++ b/pkgtools/pkglint/files/mklineparser.go    Sat Jun 06 20:42:56 2020 +0000
@@ -100,16 +100,9 @@
        for !commented && lexer.SkipByte(' ') {
        }
 
-       varnameStart := lexer.Mark()
-       // TODO: duplicated code in MkLexer.Varname
-       for lexer.NextBytesSet(VarbaseBytes) != "" || lexer.NextVarUse() != nil {
-       }
-       if lexer.SkipByte('.') || hasPrefix(splitResult.main, "SITES_") {
-               for lexer.NextBytesSet(VarparamBytes) != "" || lexer.NextVarUse() != nil {
-               }
-       }
-
-       varname := lexer.Since(varnameStart)
+       mkLexer := NewMkLexer(lexer.Rest(), nil)
+       varname := mkLexer.Varname()
+       lexer.SkipMixed(len(lexer.Rest()) - len(mkLexer.Rest()))
 
        if varname == "" {
                return false, nil
@@ -215,13 +208,11 @@
 }
 
 func (p MkLineParser) parseDirective(line *Line, splitResult mkLineSplitResult) *MkLine {
-       text := line.Text
-       if !hasPrefix(text, ".") { // TODO: use lexer instead
+       lexer := textproc.NewLexer(splitResult.main)
+       if !lexer.SkipByte('.') {
                return nil
        }
 
-       lexer := textproc.NewLexer(splitResult.main[1:])
-
        indent := lexer.NextHspace()
        directive := lexer.NextBytesSet(LowerDash)
        switch directive {
@@ -296,7 +287,7 @@
 // contain the shell commands to be associated with make targets. These cannot
 // have comments.
 //
-// If line is given, it is used for logging parse errors and warnings
+// If diag is given, it is used for logging parse errors and warnings
 // about round parentheses instead of curly braces, as well as ambiguous
 // variables of the form $v instead of ${v}.
 //
diff -r 3f9bb4cd3428 -r 3a0e0d36bcec pkgtools/pkglint/files/mklineparser_test.go
--- a/pkgtools/pkglint/files/mklineparser_test.go       Sat Jun 06 19:09:37 2020 +0000
+++ b/pkgtools/pkglint/files/mklineparser_test.go       Sat Jun 06 20:42:56 2020 +0000
@@ -214,7 +214,9 @@
                text := line.Text
 
                t.CheckOutputEmpty()
-               valueAlign := NewMkLineParser().Parse(line).ValueAlign()
+               mkline := NewMkLineParser().Parse(line)
+               assert(mkline.IsVarassignMaybeCommented())
+               valueAlign := mkline.ValueAlign()
                _ = t.Output()
 
                parser := NewMkLineParser()
@@ -274,6 +276,10 @@
        test(" VAR=value", false, "VAR", "", "=", " VAR=", "value", "", "")
        test("VAR=value #comment", false, "VAR", "", "=", "VAR=", "value", " ", "#comment")
        test("NFILES=${FILES:[#]}", false, "NFILES", "", "=", "NFILES=", "${FILES:[#]}", "", "")
+       test(".VARNAME=value", false, ".VARNAME", "", "=", ".VARNAME=", "value", "", "")
+       test(".VAR.param=value", false, ".VAR.param", "", "=", ".VAR.param=", "value", "", "")
+       testInvalid("./=value", nil...)
+       testInvalid("#./=value", nil...)
 
        // To humans, the base variable name seems to be SITES_, being parameterized
        // with distfile-1.0.tar.gz. For pkglint though, the base variable name is
diff -r 3f9bb4cd3428 -r 3a0e0d36bcec pkgtools/pkglint/files/mktokenslexer.go
--- a/pkgtools/pkglint/files/mktokenslexer.go   Sat Jun 06 19:09:37 2020 +0000
+++ b/pkgtools/pkglint/files/mktokenslexer.go   Sat Jun 06 20:42:56 2020 +0000
@@ -46,6 +46,29 @@
        return sb.String()
 }
 
+// Skip skips the next n bytes from the plain text.
+// If there is a variable use in the next n bytes, it panics; see SkipMixed.
+func (m *MkTokensLexer) Skip(n int) bool {
+       return m.Lexer.Skip(n)
+}
+
+// SkipMixed skips the next n bytes, be they in plain text or in variable uses.
+// It is only used in very special situations.
+func (m *MkTokensLexer) SkipMixed(n int) bool {
+       result := n > 0
+       for n > 0 {
+               use := m.NextVarUse()
+               if use != nil {
+                       n -= len(use.Text)
+               } else {
+                       skip := imin(len(m.Lexer.Rest()), n)
+                       assert(m.Lexer.Skip(skip))
+                       n -= skip
+               }
+       }
+       return result
+}
+
 // NextVarUse returns the next varuse token, unless there is some plain text
 // before it. In that case or at EOF, it returns nil.
 func (m *MkTokensLexer) NextVarUse() *MkToken {
diff -r 3f9bb4cd3428 -r 3a0e0d36bcec pkgtools/pkglint/files/package.go
--- a/pkgtools/pkglint/files/package.go Sat Jun 06 19:09:37 2020 +0000
+++ b/pkgtools/pkglint/files/package.go Sat Jun 06 20:42:56 2020 +0000
@@ -660,33 +660,12 @@
                defer trace.Call(filename)()
        }
 
-       vars := pkg.vars
        pkg.checkPlist()
 
-       want := !vars.IsDefined("NO_CHECKSUM")
-       want = want && !vars.IsDefined("META_PACKAGE")
-       want = want && !(vars.IsDefined("DISTFILES") && vars.LastValue("DISTFILES") == "")
-       want = want || !isEmptyDir(pkg.File(pkg.Patchdir))
-
-       if !want {
-               distinfoFile := pkg.File(pkg.DistinfoFile)
-               if distinfoFile.IsFile() {
-                       NewLineWhole(distinfoFile).Warnf("This file should not exist since NO_CHECKSUM or META_PACKAGE is set.")
-               }
-       } else {
-               distinfoFile := pkg.File(pkg.DistinfoFile)
-               if !containsVarUse(distinfoFile.String()) && !distinfoFile.IsFile() {
-                       line := NewLineWhole(distinfoFile)
-                       line.Warnf("A package that downloads files should have a distinfo file.")
-                       line.Explain(
-                               sprintf("To generate the distinfo file, run %q.", bmake("makesum")),
-                               "",
-                               "To mark the package as not needing a distinfo file, set",
-                               "NO_CHECKSUM=yes in the package Makefile.")
-               }
-       }
+       pkg.checkDistinfoExists()
 
        // TODO: There are other REPLACE_* variables which are probably also affected by NO_CONFIGURE.
+       vars := pkg.vars
        if noConfigureLine := vars.FirstDefinition("NO_CONFIGURE"); noConfigureLine != nil {
                if replacePerlLine := vars.FirstDefinition("REPLACE_PERL"); replacePerlLine != nil {
                        replacePerlLine.Warnf("REPLACE_PERL is ignored when NO_CONFIGURE is set (in %s).",
@@ -766,6 +745,51 @@
        SaveAutofixChanges(mklines.lines)
 }
 
+func (pkg *Package) checkDistinfoExists() {
+       vars := pkg.vars
+



Home | Main Index | Thread Index | Old Index