pkgsrc-Changes archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
CVS commit: pkgsrc/pkgtools/pkglint
Module Name: pkgsrc
Committed By: rillig
Date: Sun Mar 15 11:31:24 UTC 2020
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile
pkgsrc/pkgtools/pkglint/files: buildlink3.go buildlink3_test.go
check_test.go mkcondchecker.go mkcondchecker_test.go
mkvarusechecker.go mkvarusechecker_test.go options.go
options_test.go package.go package_test.go pkglint.go util_test.go
varalignblock.go varalignblock_test.go
Log Message:
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.
To generate a diff of this commit:
cvs rdiff -u -r1.633 -r1.634 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.32 -r1.33 pkgsrc/pkgtools/pkglint/files/buildlink3.go
cvs rdiff -u -r1.41 -r1.42 pkgsrc/pkgtools/pkglint/files/buildlink3_test.go
cvs rdiff -u -r1.66 -r1.67 pkgsrc/pkgtools/pkglint/files/check_test.go
cvs rdiff -u -r1.5 -r1.6 pkgsrc/pkgtools/pkglint/files/mkcondchecker.go
cvs rdiff -u -r1.6 -r1.7 pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go
cvs rdiff -u -r1.7 -r1.8 pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go \
pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go
cvs rdiff -u -r1.21 -r1.22 pkgsrc/pkgtools/pkglint/files/options.go
cvs rdiff -u -r1.22 -r1.23 pkgsrc/pkgtools/pkglint/files/options_test.go
cvs rdiff -u -r1.82 -r1.83 pkgsrc/pkgtools/pkglint/files/package.go
cvs rdiff -u -r1.69 -r1.70 pkgsrc/pkgtools/pkglint/files/package_test.go
cvs rdiff -u -r1.75 -r1.76 pkgsrc/pkgtools/pkglint/files/pkglint.go
cvs rdiff -u -r1.49 -r1.50 pkgsrc/pkgtools/pkglint/files/util_test.go
cvs rdiff -u -r1.18 -r1.19 pkgsrc/pkgtools/pkglint/files/varalignblock.go
cvs rdiff -u -r1.15 -r1.16 \
pkgsrc/pkgtools/pkglint/files/varalignblock_test.go
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: pkgsrc/pkgtools/pkglint/Makefile
diff -u pkgsrc/pkgtools/pkglint/Makefile:1.633 pkgsrc/pkgtools/pkglint/Makefile:1.634
--- pkgsrc/pkgtools/pkglint/Makefile:1.633 Sat Mar 7 23:35:35 2020
+++ pkgsrc/pkgtools/pkglint/Makefile Sun Mar 15 11:31:24 2020
@@ -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/}
Index: pkgsrc/pkgtools/pkglint/files/buildlink3.go
diff -u pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.32 pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.33
--- pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.32 Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/buildlink3.go Sun Mar 15 11:31:24 2020
@@ -13,8 +13,8 @@ type Buildlink3Checker struct {
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 @@ func (ck *Buildlink3Checker) checkMainPa
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 @@ func (ck *Buildlink3Checker) checkMainPa
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
Index: pkgsrc/pkgtools/pkglint/files/buildlink3_test.go
diff -u pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.41 pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.42
--- pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.41 Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/buildlink3_test.go Sun Mar 15 11:31:24 2020
@@ -2,6 +2,10 @@ package pkglint
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 @@ func (s *Suite) Test_CheckLinesBuildlink
"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 @@ func (s *Suite) Test_Buildlink3Checker_c
"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.
Index: pkgsrc/pkgtools/pkglint/files/check_test.go
diff -u pkgsrc/pkgtools/pkglint/files/check_test.go:1.66 pkgsrc/pkgtools/pkglint/files/check_test.go:1.67
--- pkgsrc/pkgtools/pkglint/files/check_test.go:1.66 Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/check_test.go Sun Mar 15 11:31:24 2020
@@ -581,15 +581,19 @@ func (t *Tester) CreateFileDummyPatch(fi
}
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 @@ func (t *Tester) CreateFileBuildlink3(fi
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...)
}
Index: pkgsrc/pkgtools/pkglint/files/mkcondchecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mkcondchecker.go:1.5 pkgsrc/pkgtools/pkglint/files/mkcondchecker.go:1.6
--- pkgsrc/pkgtools/pkglint/files/mkcondchecker.go:1.5 Sat Mar 7 23:35:35 2020
+++ pkgsrc/pkgtools/pkglint/files/mkcondchecker.go Sun Mar 15 11:31:24 2020
@@ -82,12 +82,21 @@ func (ck *MkCondChecker) checkNotEmpty(n
// 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),
Index: pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go:1.6 pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go:1.7
--- pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go:1.6 Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go Sun Mar 15 11:31:24 2020
@@ -219,7 +219,7 @@ func (s *Suite) Test_MkCondChecker_Check
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 @@ func (s *Suite) Test_MkCondChecker_check
}
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 @@ func (s *Suite) Test_MkCondChecker_simpl
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 @@ func (s *Suite) Test_MkCondChecker_simpl
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 @@ func (s *Suite) Test_MkCondChecker_simpl
// 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.")
+ testAfterPrefs(
".if ${PREFS_DEFINED:M&&}",
".if ${PREFS_DEFINED:M&&}",
nil...)
- testBeforeAndAfterPrefs(
+ testBeforePrefs(
+ ".if ${PREFS:M&&}",
+ ".if ${PREFS:M&&}",
+
+ // TODO: Warn that the :U is missing.
+ "WARN: filename.mk:3: To use PREFS at load time, .include \"../../mk/bsd.prefs.mk\" first.")
+ testAfterPrefs(
".if ${PREFS:M&&}",
".if ${PREFS:M&&}",
@@ -947,7 +969,19 @@ func (s *Suite) Test_MkCondChecker_simpl
// The characters <=> may be used unescaped in :M and :N patterns
// but not in .if conditions. There they must be enclosed in quotes.
- testBeforeAndAfterPrefs(
+ testBeforePrefs(
+ ".if ${PREFS_DEFINED:M<=>}",
+ ".if ${PREFS_DEFINED:U} == \"<=>\"",
+
+ "NOTE: filename.mk:3: PREFS_DEFINED can be "+
+ "compared using the simpler \"${PREFS_DEFINED:U} == \"<=>\"\" "+
+ "instead of matching against \":M<=>\".",
+ "WARN: filename.mk:3: To use PREFS_DEFINED at load time, "+
+ ".include \"../../mk/bsd.prefs.mk\" first.",
+ "AUTOFIX: filename.mk:3: "+
+ "Replacing \"${PREFS_DEFINED:M<=>}\" "+
+ "with \"${PREFS_DEFINED:U} == \\\"<=>\\\"\".")
+ testAfterPrefs(
".if ${PREFS_DEFINED:M<=>}",
".if ${PREFS_DEFINED} == \"<=>\"",
Index: pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go:1.7 pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go:1.8
--- pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go:1.7 Sat Mar 7 23:35:35 2020
+++ pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go Sun Mar 15 11:31:24 2020
@@ -33,6 +33,7 @@ func (ck *MkVarUseChecker) Check(vuc *Va
ck.checkToolsPlatform()
ck.checkBuildDefs()
ck.checkDeprecated()
+ ck.checkPkgBuildOptions()
NewMkLineChecker(ck.MkLines, ck.MkLine).
checkTextVarUse(ck.use.varname, ck.vartype, vuc.time)
@@ -738,3 +739,26 @@ func (ck *MkVarUseChecker) checkDeprecat
ck.MkLine.Warnf("Use of %q is deprecated. %s", varname, instead)
}
+
+func (ck *MkVarUseChecker) checkPkgBuildOptions() {
+ pkg := ck.MkLines.pkg
+ if pkg == nil {
+ return
+ }
+ varname := ck.use.varname
+ if !hasPrefix(varname, "PKG_BUILD_OPTIONS.") {
+ return
+ }
+ param := varnameParam(varname)
+ if pkg.seenPkgbase.Seen(param) {
+ return
+ }
+
+ ck.MkLine.Warnf("The PKG_BUILD_OPTIONS for %q are not available to this package.",
+ param)
+ ck.MkLine.Explain(
+ "The variable parameter for PKG_BUILD_OPTIONS must correspond",
+ "to the value of \"pkgbase\" above.",
+ "",
+ "For more information, see mk/pkg-build-options.mk.")
+}
Index: pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go:1.7 pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go:1.8
--- pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go:1.7 Sat Mar 7 23:35:35 2020
+++ pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go Sun Mar 15 11:31:24 2020
@@ -1325,3 +1325,42 @@ func (s *Suite) Test_MkVarUseChecker_che
"WARN: filename.mk:3: Use of \"USE_CROSSBASE\" is deprecated. "+
"Has been removed.")
}
+
+// This test demonstrates some typos that an inexperienced pkgsrc developer
+// might make. This scenario is not intended to be realistic.
+func (s *Suite) Test_MkVarUseChecker_checkPkgBuildOptions(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpOption("option", "")
+ t.SetUpPackage("category/package",
+ ".include \"../../category/lib/buildlink3.mk\"")
+ t.SetUpPackage("category/lib")
+ t.CreateFileLines("mk/pkg-build-options.mk")
+ t.CreateFileBuildlink3("category/package/buildlink3.mk",
+ ".include \"../../mk/bsd.fast.prefs.mk\"",
+ "",
+ ".if ${PKG_BUILD_OPTIONS.lib:Moption}", // Too early
+ ".endif",
+ "",
+ ".if ${PKG_BUILD_OPTIONS.unrelated:Moption}",
+ ".include \"../../category/lib/buildlink3.mk\"",
+ ".endif",
+ "",
+ ".if ${PKG_BUILD_OPTIONS.lib:Moption}", // Only defined conditionally
+ ".endif")
+ t.CreateFileBuildlink3("category/lib/buildlink3.mk",
+ "pkgbase := lib",
+ ".include \"../../mk/pkg-build-options.mk\"")
+ t.Chdir("category/package")
+ t.FinishSetUp()
+
+ G.Check(".")
+
+ t.CheckOutputLines(
+ "WARN: Makefile:20: \"../../category/lib/buildlink3.mk\" is included unconditionally here "+
+ "and conditionally in buildlink3.mk:18 (depending on PKG_BUILD_OPTIONS.unrelated).",
+ "WARN: buildlink3.mk:17: The PKG_BUILD_OPTIONS for \"unrelated\" are not available to this package.",
+ "WARN: buildlink3.mk:14: Wrong PKG_BUILD_OPTIONS, expected \"package\" instead of \"lib\".",
+ "WARN: buildlink3.mk:17: Wrong PKG_BUILD_OPTIONS, expected \"package\" instead of \"unrelated\".",
+ "WARN: buildlink3.mk:21: Wrong PKG_BUILD_OPTIONS, expected \"package\" instead of \"lib\".")
+}
Index: pkgsrc/pkgtools/pkglint/files/options.go
diff -u pkgsrc/pkgtools/pkglint/files/options.go:1.21 pkgsrc/pkgtools/pkglint/files/options.go:1.22
--- pkgsrc/pkgtools/pkglint/files/options.go:1.21 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/options.go Sun Mar 15 11:31:24 2020
@@ -1,8 +1,9 @@
package pkglint
-func CheckLinesOptionsMk(mklines *MkLines) {
+func CheckLinesOptionsMk(mklines *MkLines, buildlinkID string) {
ck := OptionsLinesChecker{
mklines,
+ buildlinkID,
false,
make(map[string]*MkLine),
false,
@@ -16,7 +17,8 @@ func CheckLinesOptionsMk(mklines *MkLine
//
// See mk/bsd.options.mk for a detailed description.
type OptionsLinesChecker struct {
- mklines *MkLines
+ mklines *MkLines
+ buildlinkID string
declaredArbitrary bool
declaredOptions map[string]*MkLine
@@ -49,6 +51,18 @@ func (ck *OptionsLinesChecker) collect()
if !seenInclude {
if !seenPkgOptionsVar && mkline.IsVarassign() && mkline.Varname() == "PKG_OPTIONS_VAR" {
seenPkgOptionsVar = true
+ buildlinkID := ck.buildlinkID
+ optionsID := varnameParam(mkline.Value())
+ if buildlinkID != "" && optionsID != "" && buildlinkID != optionsID {
+ mkline.Warnf("The buildlink3 identifier %q should be the same as the options identifier %q.",
+ buildlinkID, optionsID)
+ mkline.Explain(
+ "Having different identifiers refer to the same package",
+ "is confusing for the pkgsrc user.",
+ "The pkgsrc infrastructure doesn't care though",
+ "if the identifiers in PKG_OPTIONS.* and PKG_BUILD_OPTIONS.*",
+ "are the same or not.")
+ }
}
seenInclude = mkline.IsInclude() && mkline.IncludedFile() == "../../mk/bsd.options.mk"
}
Index: pkgsrc/pkgtools/pkglint/files/options_test.go
diff -u pkgsrc/pkgtools/pkglint/files/options_test.go:1.22 pkgsrc/pkgtools/pkglint/files/options_test.go:1.23
--- pkgsrc/pkgtools/pkglint/files/options_test.go:1.22 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/options_test.go Sun Mar 15 11:31:24 2020
@@ -88,7 +88,7 @@ func (s *Suite) Test_CheckLinesOptionsMk
t.Chdir("category/package")
t.FinishSetUp()
- CheckLinesOptionsMk(mklines)
+ CheckLinesOptionsMk(mklines, "")
t.CheckOutputLines(
"WARN: ~/category/package/options.mk:4: "+
@@ -124,7 +124,7 @@ func (s *Suite) Test_CheckLinesOptionsMk
t.Chdir("category/package")
t.FinishSetUp()
- CheckLinesOptionsMk(mklines)
+ CheckLinesOptionsMk(mklines, "")
t.CheckOutputLines(
"WARN: ~/category/package/options.mk:5: "+
@@ -158,7 +158,7 @@ func (s *Suite) Test_CheckLinesOptionsMk
t.Chdir("category/package")
t.FinishSetUp()
- CheckLinesOptionsMk(mklines)
+ CheckLinesOptionsMk(mklines, "")
t.CheckOutputEmpty()
}
@@ -184,7 +184,7 @@ func (s *Suite) Test_CheckLinesOptionsMk
t.Chdir("category/package")
t.FinishSetUp()
- CheckLinesOptionsMk(mklines)
+ CheckLinesOptionsMk(mklines, "")
t.CheckOutputLines(
"WARN: ~/category/package/options.mk:3: " +
@@ -201,7 +201,7 @@ func (s *Suite) Test_CheckLinesOptionsMk
t.Chdir("category/package")
t.FinishSetUp()
- CheckLinesOptionsMk(mklines)
+ CheckLinesOptionsMk(mklines, "")
t.CheckOutputLines(
"ERROR: ~/category/package/options.mk: "+
@@ -240,7 +240,7 @@ func (s *Suite) Test_CheckLinesOptionsMk
".endif")
t.FinishSetUp()
- CheckLinesOptionsMk(mklines)
+ CheckLinesOptionsMk(mklines, "")
t.CheckOutputLines(
// This warning comes from VarTypeCheck.PkgOption
@@ -300,7 +300,7 @@ func (s *Suite) Test_CheckLinesOptionsMk
".elif !empty(PKG_OPTIONS:Msqlite)",
".endif")
- CheckLinesOptionsMk(mklines)
+ CheckLinesOptionsMk(mklines, "")
t.CheckOutputLines(
"WARN: ~/category/package/options.mk:6: l is used but not defined.",
@@ -335,7 +335,7 @@ func (s *Suite) Test_CheckLinesOptionsMk
"pre-configure:",
"\techo \"In the pre-configure stage.\"")
- CheckLinesOptionsMk(mklines)
+ CheckLinesOptionsMk(mklines, "")
t.CheckOutputLines(
"WARN: ~/category/package/options.mk:6: "+
@@ -375,7 +375,7 @@ func (s *Suite) Test_CheckLinesOptionsMk
".if ${OPSYS} == 'Darwin'",
".endif")
- CheckLinesOptionsMk(mklines)
+ CheckLinesOptionsMk(mklines, "")
t.CheckOutputLines(
"WARN: category/package/options.mk:15: Invalid condition, unrecognized part: \"${OPSYS} == 'Darwin'\".")
@@ -618,7 +618,7 @@ func (s *Suite) Test_CheckLinesOptionsMk
". endif",
".endfor")
- CheckLinesOptionsMk(mklines)
+ CheckLinesOptionsMk(mklines, "")
t.CheckOutputLines(
"WARN: ~/category/package/options.mk:5: "+
@@ -645,7 +645,7 @@ func (s *Suite) Test_CheckLinesOptionsMk
".if ${PKG_OPTIONS:Moption}",
".endif")
- CheckLinesOptionsMk(mklines)
+ CheckLinesOptionsMk(mklines, "")
t.CheckOutputLines(
"WARN: ~/category/package/options.mk:8: " +
@@ -670,7 +670,7 @@ func (s *Suite) Test_CheckLinesOptionsMk
".if ${PKG_OPTIONS:Mopt${:Uion}}",
".endif")
- CheckLinesOptionsMk(mklines)
+ CheckLinesOptionsMk(mklines, "")
// Pkglint does not expand the ${:Uion}, therefore it doesn't know that
// the option is indeed handled. Because of this uncertainty, pkglint
Index: pkgsrc/pkgtools/pkglint/files/package.go
diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.82 pkgsrc/pkgtools/pkglint/files/package.go:1.83
--- pkgsrc/pkgtools/pkglint/files/package.go:1.82 Sat Mar 7 23:35:35 2020
+++ pkgsrc/pkgtools/pkglint/files/package.go Sun Mar 15 11:31:24 2020
@@ -16,17 +16,21 @@ const rePkgname = `^([\w\-.+]+)-(\d[.0-9
// This is necessary because variables in Makefiles may be used before they are defined,
// and such dependencies often span multiple files that are included indirectly.
type Package struct {
- dir CurrPath // The directory of the package, for resolving files
- Pkgpath PkgsrcPath // e.g. "category/pkgdir"
- Pkgdir PackagePath // PKGDIR from the package Makefile
- Filesdir PackagePath // FILESDIR from the package Makefile
- Patchdir PackagePath // PATCHDIR from the package Makefile
- DistinfoFile PackagePath // DISTINFO_FILE from the package Makefile
- EffectivePkgname string // PKGNAME or DISTNAME from the package Makefile, including nb13, can be empty
- EffectivePkgbase string // EffectivePkgname without the version
- EffectivePkgversion string // The version part of the effective PKGNAME, excluding nb13
- EffectivePkgnameLine *MkLine // The origin of the three Effective* values
- Plist PlistContent // Files and directories mentioned in the PLIST files
+ dir CurrPath // The directory of the package, for resolving files
+ Pkgpath PkgsrcPath // e.g. "category/pkgdir"
+
+ EffectivePkgname string // PKGNAME or DISTNAME from the package Makefile, including nb13, can be empty
+ EffectivePkgbase string // EffectivePkgname without the version
+ EffectivePkgversion string // The version part of the effective PKGNAME, excluding nb13
+ EffectivePkgnameLine *MkLine // The origin of the three Effective* values
+ buildlinkID string
+ optionsID string
+
+ Pkgdir PackagePath // PKGDIR from the package Makefile
+ Filesdir PackagePath // FILESDIR from the package Makefile
+ Patchdir PackagePath // PATCHDIR from the package Makefile
+ DistinfoFile PackagePath // DISTINFO_FILE from the package Makefile
+ Plist PlistContent // Files and directories mentioned in the PLIST files
vars Scope
redundant *RedundantScope
@@ -55,6 +59,9 @@ type Package struct {
// TODO: Be more precise about the purpose of this field.
seenInclude bool
+ // The identifiers for which PKG_BUILD_OPTIONS is defined.
+ seenPkgbase Once
+
// During both load() and check(), tells whether bsd.prefs.mk has
// already been loaded directly or indirectly.
//
@@ -160,6 +167,11 @@ func (pkg *Package) load() ([]CurrPath,
if isRelevantMk(filename, basename) {
fragmentMklines := LoadMk(filename, pkg, MustSucceed)
pkg.collectConditionalIncludes(fragmentMklines)
+ fragmentMklines.ForEach(func(mkline *MkLine) {
+ if mkline.IsVarassign() && mkline.Varname() == "pkgbase" {
+ pkg.seenPkgbase.FirstTime(mkline.Value())
+ }
+ })
}
if hasPrefix(basename, "PLIST") {
pkg.loadPlistDirs(filename)
@@ -311,6 +323,10 @@ func (pkg *Package) parseLine(mklines *M
}
pkg.vars.Define(varname, mkline)
}
+
+ if varname == "pkgbase" {
+ pkg.seenPkgbase.FirstTime(mkline.Value())
+ }
}
return true
}
Index: pkgsrc/pkgtools/pkglint/files/package_test.go
diff -u pkgsrc/pkgtools/pkglint/files/package_test.go:1.69 pkgsrc/pkgtools/pkglint/files/package_test.go:1.70
--- pkgsrc/pkgtools/pkglint/files/package_test.go:1.69 Mon Feb 17 20:22:21 2020
+++ pkgsrc/pkgtools/pkglint/files/package_test.go Sun Mar 15 11:31:24 2020
@@ -320,6 +320,46 @@ func (s *Suite) Test_Package__case_insen
t.CheckOutputEmpty()
}
+// This package has several identifiers that all differ:
+// - it lives in the directory "package"
+// - the package name is "pkgname"
+// - it downloads "distname-1.0.tar.gz"
+// (in some places the distname is used as the package name)
+// - in options.mk its name is "optid"
+// - in buildlink3.mk its name is "bl3id"
+// All these identifiers should ideally be the same.
+// For historic reasons, the package directory and the package name
+// may differ.
+func (s *Suite) Test_Package__different_package_identifiers(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package",
+ "DISTNAME=\tdistname-1.0",
+ "PKGNAME=\tpkgname-1.0")
+ t.CreateFileLines("mk/bsd.options.mk")
+ t.CreateFileLines("category/package/options.mk",
+ MkCvsID,
+ "",
+ "PKG_OPTIONS_VAR=\tPKG_OPTIONS.optid",
+ "PKG_SUPPORTED_OPTIONS=\t# none",
+ "",
+ ".include \"../../mk/bsd.options.mk\"",
+ "",
+ "# Nothing to do here")
+ t.CreateFileBuildlink3Id("category/package/buildlink3.mk", "bl3id")
+ t.Chdir("category/package")
+ t.FinishSetUp()
+
+ G.checkdirPackage(".")
+
+ t.CheckOutputLines(
+ "ERROR: buildlink3.mk:3: Package name mismatch "+
+ "between \"bl3id\" in this file and \"pkgname\" from Makefile:4.",
+ "WARN: options.mk:3: The buildlink3 identifier \"bl3id\" "+
+ "should be the same as the options identifier \"optid\".")
+
+}
+
func (s *Suite) Test_NewPackage(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/pkglint.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint.go:1.75 pkgsrc/pkgtools/pkglint/files/pkglint.go:1.76
--- pkgsrc/pkgtools/pkglint/files/pkglint.go:1.75 Sat Mar 7 23:35:35 2020
+++ pkgsrc/pkgtools/pkglint/files/pkglint.go Sun Mar 15 11:31:24 2020
@@ -571,7 +571,11 @@ func (p *Pkglint) checkReg(filename Curr
case basename == "buildlink3.mk":
if mklines := LoadMk(filename, pkg, NotEmpty|LogErrors); mklines != nil {
- CheckLinesBuildlink3Mk(mklines)
+ ck := NewBuildlink3Checker(mklines)
+ ck.Check()
+ if pkg != nil {
+ pkg.buildlinkID = ck.pkgbase
+ }
}
case hasPrefix(basename, "DESCR"):
@@ -594,7 +598,11 @@ func (p *Pkglint) checkReg(filename Curr
case basename == "options.mk":
if mklines := LoadMk(filename, pkg, NotEmpty|LogErrors); mklines != nil {
- CheckLinesOptionsMk(mklines)
+ buildlinkID := ""
+ if pkg != nil {
+ buildlinkID = pkg.buildlinkID
+ }
+ CheckLinesOptionsMk(mklines, buildlinkID)
}
case matches(basename, `^patch-[-\w.~+]*\w$`):
Index: pkgsrc/pkgtools/pkglint/files/util_test.go
diff -u pkgsrc/pkgtools/pkglint/files/util_test.go:1.49 pkgsrc/pkgtools/pkglint/files/util_test.go:1.50
--- pkgsrc/pkgtools/pkglint/files/util_test.go:1.49 Sat Mar 7 23:35:35 2020
+++ pkgsrc/pkgtools/pkglint/files/util_test.go Sun Mar 15 11:31:24 2020
@@ -304,6 +304,12 @@ func (s *Suite) Test_alignWith(c *check.
test("1234567890=", "V=", "1234567890=")
}
+func (s *Suite) Test_alignmentToWidths(c *check.C) {
+ t := s.Init(c)
+
+ t.CheckEquals(alignmentToWidths(8, 72), "\t\t\t\t\t\t\t\t")
+}
+
func (s *Suite) Test_indent(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/varalignblock.go
diff -u pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.18 pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.19
--- pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.18 Sat Mar 7 23:35:35 2020
+++ pkgsrc/pkgtools/pkglint/files/varalignblock.go Sun Mar 15 11:31:24 2020
@@ -656,9 +656,6 @@ func (info *varalignLine) alignContinuat
fix.Notef("The continuation backslash should be preceded by a single space.")
} else {
newSpace = alignmentToWidths(info.uptoValueWidth(), rightMarginColumn)
- if newSpace == "" {
- newSpace = " "
- }
fix.Notef(
"The continuation backslash should be in column %d, not %d.",
rightMarginColumn+1, column+1)
@@ -824,7 +821,7 @@ func (p *varalignParts) uptoValueWidth()
if p.value != "" {
return p.spaceAfterValueColumn()
} else {
- return p.varnameOpColumn()
+ return p.spaceBeforeValueColumn()
}
}
Index: pkgsrc/pkgtools/pkglint/files/varalignblock_test.go
diff -u pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.15 pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.16
--- pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.15 Sat Mar 7 23:35:35 2020
+++ pkgsrc/pkgtools/pkglint/files/varalignblock_test.go Sun Mar 15 11:31:24 2020
@@ -2063,6 +2063,30 @@ func (s *Suite) Test_VaralignBlock__var_
vt.Run()
}
+func (s *Suite) Test_VaralignBlock__var8_tabs80_cont_tab_value_tabs72_cont_tab_value_tabs72_cont_tab_value9(c *check.C) {
+ vt := NewVaralignTester(s, c)
+ vt.Input(
+ "VAR...7=\t\t\t\t\t\t\t\t\t\\",
+ "\t.\t\t\t\t\t\t\t\t\\",
+ "\t.\t\t\t\t\t\t\t\t\\",
+ "\t.")
+ vt.InputDetab(
+ "VAR...7= \\",
+ " . \\",
+ " . \\",
+ " .")
+ vt.Diagnostics(
+ "NOTE: Makefile:1: The continuation backslash should be in column 73, not 81.")
+ vt.Autofixes(
+ "AUTOFIX: Makefile:1: Replacing \"\\t\\t\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\\t\\t\\t\".")
+ vt.Fixed(
+ "VAR...7= \\",
+ " . \\",
+ " . \\",
+ " .")
+ vt.Run()
+}
+
// Up to 2020-01-27, pkglint removed all spaces before the backslash,
// which was against the rule of having at least one space.
func (s *Suite) Test_VaralignBlock__right_margin(c *check.C) {
Home |
Main Index |
Thread Index |
Old Index