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/2934223fde8a
branches: trunk
changeset: 413024:2934223fde8a
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 01c7f4002651 -r 2934223fde8a 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 01c7f4002651 -r 2934223fde8a 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 01c7f4002651 -r 2934223fde8a 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 01c7f4002651 -r 2934223fde8a 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 01c7f4002651 -r 2934223fde8a 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 01c7f4002651 -r 2934223fde8a 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