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



details:   https://anonhg.NetBSD.org/pkgsrc/rev/8a6ce8a06ec9
branches:  trunk
changeset: 424765:8a6ce8a06ec9
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Sun Mar 15 11:31:24 2020 +0000

description:
pkgtools/pkglint: update to 19.4.11

Changes since 19.4.10:

The use of PKG_OPTIONS and PKG_BUILD_OPTIONS in buildlink3.mk and other
files is checked for common mistakes.

Checking the indentation of a continuation line no longer crashes in edge
cases.

diffstat:

 pkgtools/pkglint/Makefile                      |   4 +-
 pkgtools/pkglint/files/buildlink3.go           |  34 +++++++++++++++-
 pkgtools/pkglint/files/buildlink3_test.go      |  54 ++++++++++++++++++++++++++
 pkgtools/pkglint/files/check_test.go           |  26 +++++++-----
 pkgtools/pkglint/files/mkcondchecker.go        |  15 +++++-
 pkgtools/pkglint/files/mkcondchecker_test.go   |  52 ++++++++++++++++++++----
 pkgtools/pkglint/files/mkvarusechecker.go      |  24 +++++++++++
 pkgtools/pkglint/files/mkvarusechecker_test.go |  39 ++++++++++++++++++
 pkgtools/pkglint/files/options.go              |  18 +++++++-
 pkgtools/pkglint/files/options_test.go         |  24 +++++-----
 pkgtools/pkglint/files/package.go              |  38 +++++++++++++-----
 pkgtools/pkglint/files/package_test.go         |  40 +++++++++++++++++++
 pkgtools/pkglint/files/pkglint.go              |  12 ++++-
 pkgtools/pkglint/files/util_test.go            |   6 ++
 pkgtools/pkglint/files/varalignblock.go        |   5 +-
 pkgtools/pkglint/files/varalignblock_test.go   |  24 +++++++++++
 16 files changed, 357 insertions(+), 58 deletions(-)

diffs (truncated from 796 to 300 lines):

diff -r a99e6199f60b -r 8a6ce8a06ec9 pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Sun Mar 15 11:16:04 2020 +0000
+++ b/pkgtools/pkglint/Makefile Sun Mar 15 11:31:24 2020 +0000
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.633 2020/03/07 23:35:35 rillig Exp $
+# $NetBSD: Makefile,v 1.634 2020/03/15 11:31:24 rillig Exp $
 
-PKGNAME=       pkglint-19.4.10
+PKGNAME=       pkglint-19.4.11
 CATEGORIES=    pkgtools
 DISTNAME=      tools
 MASTER_SITES=  ${MASTER_SITE_GITHUB:=golang/}
diff -r a99e6199f60b -r 8a6ce8a06ec9 pkgtools/pkglint/files/buildlink3.go
--- a/pkgtools/pkglint/files/buildlink3.go      Sun Mar 15 11:16:04 2020 +0000
+++ b/pkgtools/pkglint/files/buildlink3.go      Sun Mar 15 11:31:24 2020 +0000
@@ -13,8 +13,8 @@
        abi, api         *DependencyPattern
 }
 
-func CheckLinesBuildlink3Mk(mklines *MkLines) {
-       (&Buildlink3Checker{mklines: mklines}).Check()
+func NewBuildlink3Checker(mklines *MkLines) *Buildlink3Checker {
+       return &Buildlink3Checker{mklines: mklines}
 }
 
 func (ck *Buildlink3Checker) Check() {
@@ -182,6 +182,10 @@
                case mkline.IsDirective() && mkline.Directive() == "endif":
                        indentLevel--
                }
+
+               mkline.ForEachUsed(func(varUse *MkVarUse, time VucTime) {
+                       ck.checkVarUse(varUse, mkline)
+               })
        }
 
        if indentLevel > 0 {
@@ -195,6 +199,32 @@
        return true
 }
 
+func (ck *Buildlink3Checker) checkVarUse(varUse *MkVarUse, mkline *MkLine) {
+       varname := varUse.varname
+       if varname == "PKG_OPTIONS" {
+               mkline.Errorf("PKG_OPTIONS is not available in buildlink3.mk files.")
+               mkline.Explain(
+                       "The buildlink3.mk file of a package is only ever included",
+                       "by other packages, never by the package itself.",
+                       "Therefore it does not make sense to use the variable PKG_OPTIONS",
+                       "in this place since it contains the package options of a random",
+                       "package that happens to include this file.",
+                       "",
+                       "To access the options of this package, see mk/pkg-build-options.mk.")
+       }
+
+       if varnameBase(varname) == "PKG_BUILD_OPTIONS" {
+               param := varnameParam(varname)
+               if param != "" && param != ck.pkgbase {
+                       mkline.Warnf("Wrong PKG_BUILD_OPTIONS, expected %q instead of %q.",
+                               ck.pkgbase, param)
+                       mkline.Explain(
+                               "The variable parameter for PKG_BUILD_OPTIONS must correspond",
+                               "to the value of \"pkgbase\" above.")
+               }
+       }
+}
+
 func (ck *Buildlink3Checker) checkVarassign(mkline *MkLine, pkgbase string) {
        varname, value := mkline.Varname(), mkline.Value()
        doCheck := false
diff -r a99e6199f60b -r 8a6ce8a06ec9 pkgtools/pkglint/files/buildlink3_test.go
--- a/pkgtools/pkglint/files/buildlink3_test.go Sun Mar 15 11:16:04 2020 +0000
+++ b/pkgtools/pkglint/files/buildlink3_test.go Sun Mar 15 11:31:24 2020 +0000
@@ -2,6 +2,10 @@
 
 import "gopkg.in/check.v1"
 
+func CheckLinesBuildlink3Mk(mklines *MkLines) {
+       NewBuildlink3Checker(mklines).Check()
+}
+
 // This test ensures that CheckLinesBuildlink3Mk really checks for
 // buildlink3.mk files that are included by the buildlink3.mk file
 // but not by the package.
@@ -473,6 +477,25 @@
                "WARN: buildlink3.mk:10: Invalid dependency pattern \"hs-X11!=1.6.1.2nb2\".")
 }
 
+func (s *Suite) Test_CheckLinesBuildlink3Mk__PKG_OPTIONS(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpOption("option", "")
+       t.SetUpPkgsrc()
+       t.SetUpPackage("category/package")
+       t.CreateFileBuildlink3("category/package/buildlink3.mk",
+               ".include \"../../mk/bsd.fast.prefs.mk\"",
+               ".if ${PKG_OPTIONS:Moption}",
+               ".endif")
+       t.FinishSetUp()
+
+       G.Check(t.File("category/package/buildlink3.mk"))
+
+       t.CheckOutputLines(
+               "ERROR: ~/category/package/buildlink3.mk:13: " +
+                       "PKG_OPTIONS is not available in buildlink3.mk files.")
+}
+
 // Just for branch coverage.
 func (s *Suite) Test_Buildlink3Checker_Check__no_tracing(c *check.C) {
        t := s.Init(c)
@@ -661,6 +684,37 @@
                "WARN: ~/category/package/buildlink3.mk:14: The file should end here.")
 }
 
+func (s *Suite) Test_Buildlink3Checker_checkVarUse__PKG_BUILD_OPTIONS(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpOption("option", "")
+       t.SetUpPkgsrc()
+       t.CreateFileLines("mk/pkg-build-options.mk")
+       t.SetUpPackage("category/package")
+       t.CreateFileBuildlink3("category/package/buildlink3.mk",
+               "pkgbase := unrelated",
+               ".include \"../../mk/pkg-build-options.mk\"",
+               "",
+               ".if ${PKG_BUILD_OPTIONS:Moption}", // missing variable parameter
+               ".endif",
+               "",
+               ".if ${PKG_BUILD_OPTIONS.package:Moption}", // wrong variable parameter
+               ".endif",
+               "",
+               ".if ${PKG_BUILD_OPTIONS.unrelated:Moption}", // corresponds to pkgbase above
+               ".endif")
+       t.FinishSetUp()
+
+       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.",
+               "WARN: ~/category/package/buildlink3.mk:21: "+
+                       "Wrong PKG_BUILD_OPTIONS, expected \"package\" instead of \"unrelated\".")
+}
+
 // 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.
diff -r a99e6199f60b -r 8a6ce8a06ec9 pkgtools/pkglint/files/check_test.go
--- a/pkgtools/pkglint/files/check_test.go      Sun Mar 15 11:16:04 2020 +0000
+++ b/pkgtools/pkglint/files/check_test.go      Sun Mar 15 11:31:24 2020 +0000
@@ -581,15 +581,19 @@
 }
 
 func (t *Tester) CreateFileBuildlink3(filename RelPath, customLines ...string) {
+       lower := filename.Dir().Base()
+       t.CreateFileBuildlink3Id(filename, lower, customLines...)
+}
+
+func (t *Tester) CreateFileBuildlink3Id(filename RelPath, id string, customLines ...string) {
        // Buildlink3.mk files only make sense in category/package directories.
        assert(G.Pkgsrc.Rel(t.File(filename)).Count() == 3)
 
        dir := filename.Dir().Clean()
-       lower := dir.Base()
        // see pkgtools/createbuildlink/files/createbuildlink, "package specific variables"
-       upper := strings.Replace(strings.ToUpper(lower), "-", "_", -1)
+       upperID := strings.Replace(strings.ToUpper(id), "-", "_", -1)
 
-       width := tabWidthSlice("BUILDLINK_API_DEPENDS.", lower, "+=\t")
+       width := tabWidthSlice("BUILDLINK_API_DEPENDS.", id, "+=\t")
 
        aligned := func(format string, args ...interface{}) string {
                msg := sprintf(format, args...)
@@ -603,21 +607,21 @@
        lines = append(lines,
                MkCvsID,
                "",
-               sprintf("BUILDLINK_TREE+=\t%s", lower),
+               sprintf("BUILDLINK_TREE+=\t%s", id),
                "",
-               sprintf(".if !defined(%s_BUILDLINK3_MK)", upper),
-               sprintf("%s_BUILDLINK3_MK:=", upper),
+               sprintf(".if !defined(%s_BUILDLINK3_MK)", upperID),
+               sprintf("%s_BUILDLINK3_MK:=", upperID),
                "",
-               aligned("BUILDLINK_API_DEPENDS.%s+=", lower)+sprintf("%s>=0", lower),
-               aligned("BUILDLINK_PKGSRCDIR.%s?=", lower)+sprintf("../../%s", dir),
-               aligned("BUILDLINK_DEPMETHOD.%s?=", lower)+"build",
+               aligned("BUILDLINK_API_DEPENDS.%s+=", id)+sprintf("%s>=0", id),
+               aligned("BUILDLINK_PKGSRCDIR.%s?=", id)+sprintf("../../%s", dir),
+               aligned("BUILDLINK_DEPMETHOD.%s?=", id)+"build",
                "")
        lines = append(lines, customLines...)
        lines = append(lines,
                "",
-               sprintf(".endif # %s_BUILDLINK3_MK", upper),
+               sprintf(".endif # %s_BUILDLINK3_MK", upperID),
                "",
-               sprintf("BUILDLINK_TREE+=\t-%s", lower))
+               sprintf("BUILDLINK_TREE+=\t-%s", id))
 
        t.CreateFileLines(filename, lines...)
 }
diff -r a99e6199f60b -r 8a6ce8a06ec9 pkgtools/pkglint/files/mkcondchecker.go
--- a/pkgtools/pkglint/files/mkcondchecker.go   Sun Mar 15 11:16:04 2020 +0000
+++ b/pkgtools/pkglint/files/mkcondchecker.go   Sun Mar 15 11:31:24 2020 +0000
@@ -82,12 +82,21 @@
        // This applies especially to the ${VAR:Mpattern} form.
        //
        // See MkCondChecker.simplify.
-       if !G.Experimental {
+       if !hasPrefix(not.Empty.varname, "PKG_BUILD_OPTIONS.") {
                return
        }
 
-       ck.MkLine.Notef("!empty(%s%s) can be replaced with the simpler %s.",
-               not.Empty.varname, not.Empty.Mod(), not.Empty.String())
+       fix := ck.MkLine.Autofix()
+       from := sprintf("!empty(%s%s)", not.Empty.varname, not.Empty.Mod())
+       to := not.Empty.String()
+       fix.Notef("%s can be replaced with %s.", from, to)
+       fix.Explain(
+               "Besides being simpler to read, the expression will also fail",
+               "quickly with a \"Malformed conditional\" error from bmake",
+               "if it should ever be undefined at this point.",
+               "This catches typos and other programming mistakes.")
+       fix.Replace(from, to)
+       fix.Apply()
 }
 
 // checkEmpty checks a condition of the form empty(VAR),
diff -r a99e6199f60b -r 8a6ce8a06ec9 pkgtools/pkglint/files/mkcondchecker_test.go
--- a/pkgtools/pkglint/files/mkcondchecker_test.go      Sun Mar 15 11:16:04 2020 +0000
+++ b/pkgtools/pkglint/files/mkcondchecker_test.go      Sun Mar 15 11:31:24 2020 +0000
@@ -219,7 +219,7 @@
 func (s *Suite) Test_MkCondChecker_checkNotEmpty(c *check.C) {
        t := s.Init(c)
 
-       G.Experimental = true
+       t.SetUpVartypes()
 
        test := func(cond string, diagnostics ...string) {
                mklines := t.NewMkLines("filename.mk",
@@ -233,8 +233,17 @@
        }
 
        test("!empty(VAR)",
-               // TODO: Add a :U modifier if VAR might be undefined.
-               "NOTE: filename.mk:1: !empty(VAR) can be replaced with the simpler ${VAR}.")
+
+               // Only a few variables are suggested to use the simpler form,
+               // because of the side-effect when the variable is undefined.
+               // VAR is not one of these variables.
+               nil...)
+
+       test(
+               "!empty(PKG_BUILD_OPTIONS.package:Moption)",
+
+               "NOTE: filename.mk:1: !empty(PKG_BUILD_OPTIONS.package:Moption) "+
+                       "can be replaced with ${PKG_BUILD_OPTIONS.package:Moption}.")
 }
 
 func (s *Suite) Test_MkCondChecker_checkEmpty(c *check.C) {
@@ -395,13 +404,14 @@
        t.Chdir("category/package")
 
        // The Anything type suppresses the warnings from type checking.
-       // BtUnknown would not work, see Pkgsrc.VariableType.
+       // BtUnknown would not work here, see Pkgsrc.VariableType.
        btAnything := &BasicType{"Anything", func(cv *VartypeCheck) {}}
 
        // For simplifying the expressions, it is necessary to know whether
        // a variable can be undefined. Undefined variables need the
-       // :U modifier, otherwise bmake will complain about "malformed
-       // conditions", which is not entirely precise since the expression
+       // :U modifier or must be enclosed in double quotes, otherwise
+       // bmake will complain about a "Malformed conditional". That error
+       // message is not entirely precise since the expression
        // is syntactically valid, it's just the evaluation that fails.
        //
        // Some variables such as MACHINE_ARCH are in scope from the very
@@ -474,6 +484,7 @@
                doTest(true, before, after, diagnostics...)
        }
        testBeforeAndAfterPrefs := func(before, after string, diagnostics ...string) {
+               doTest(false, before, after, diagnostics...)
                doTest(true, before, after, diagnostics...)
        }
 
@@ -919,13 +930,24 @@
        // replaced automatically, see mkCondLiteralChars.
        // TODO: Add tests for all characters that are special in string literals or patterns.
        // TODO: Then, extend the set of characters for which the expressions are simplified.
-       testBeforeAndAfterPrefs(
+       testBeforePrefs(
+               ".if ${PREFS_DEFINED:M&&}",
+               ".if ${PREFS_DEFINED:M&&}",
+
+               "WARN: filename.mk:3: To use PREFS_DEFINED at load time, .include \"../../mk/bsd.prefs.mk\" first.")



Home | Main Index | Thread Index | Old Index