summaryrefslogtreecommitdiff
path: root/pkgtools
diff options
context:
space:
mode:
authorrillig <rillig@pkgsrc.org>2019-11-02 16:37:48 +0000
committerrillig <rillig@pkgsrc.org>2019-11-02 16:37:48 +0000
commit07ba12c27807d9693e0a0b09337e9fcc64fcec16 (patch)
tree8e6bdffe92424c81feb1bd01db63d59b76d1c2ab /pkgtools
parentae5822e08c7145dbbe0737137c02aae49fc0ce3c (diff)
downloadpkgsrc-07ba12c27807d9693e0a0b09337e9fcc64fcec16.tar.gz
pkgtools/pkglint: update to 19.3.5
Changes since 19.3.4: Variable uses in parentheses (such as $(VAR) instead of ${VAR}) are treated the same. The ones in parentheses had less support before. Improved the checks for options.mk files, adding support for options that are defined using .for loops and those referring to other variables. Packages that set DISTFILES to an empty list no longer require a distinfo file. Patches whose filename contains the word CVE may patch more than one target file.
Diffstat (limited to 'pkgtools')
-rw-r--r--pkgtools/pkglint/Makefile4
-rw-r--r--pkgtools/pkglint/files/check_test.go2
-rw-r--r--pkgtools/pkglint/files/linelexer.go10
-rw-r--r--pkgtools/pkglint/files/lines.go10
-rw-r--r--pkgtools/pkglint/files/mkline.go8
-rw-r--r--pkgtools/pkglint/files/mkline_test.go4
-rw-r--r--pkgtools/pkglint/files/mklinechecker.go2
-rw-r--r--pkgtools/pkglint/files/mklinechecker_test.go8
-rw-r--r--pkgtools/pkglint/files/mklines.go4
-rwxr-xr-xpkgtools/pkglint/files/options.go159
-rwxr-xr-xpkgtools/pkglint/files/options_test.go224
-rw-r--r--pkgtools/pkglint/files/package.go7
-rw-r--r--pkgtools/pkglint/files/package_test.go42
-rw-r--r--pkgtools/pkglint/files/patches.go6
-rw-r--r--pkgtools/pkglint/files/patches_test.go25
-rw-r--r--pkgtools/pkglint/files/pkglint.go4
-rw-r--r--pkgtools/pkglint/files/util.go2
-rw-r--r--pkgtools/pkglint/files/vardefs.go2
18 files changed, 405 insertions, 118 deletions
diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile
index f34bbdb4784..be6f812be13 100644
--- a/pkgtools/pkglint/Makefile
+++ b/pkgtools/pkglint/Makefile
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.604 2019/11/01 19:56:52 rillig Exp $
+# $NetBSD: Makefile,v 1.605 2019/11/02 16:37:48 rillig Exp $
-PKGNAME= pkglint-19.3.4
+PKGNAME= pkglint-19.3.5
CATEGORIES= pkgtools
DISTNAME= tools
MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/}
diff --git a/pkgtools/pkglint/files/check_test.go b/pkgtools/pkglint/files/check_test.go
index 289b86d8110..e2e6181cde9 100644
--- a/pkgtools/pkglint/files/check_test.go
+++ b/pkgtools/pkglint/files/check_test.go
@@ -194,6 +194,8 @@ func (t *Tester) SetUpMasterSite(varname string, urls ...string) {
}
// SetUpOption pretends that the given package option is defined in mk/defaults/options.description.
+//
+// In tests, the description may be left empty.
func (t *Tester) SetUpOption(name, description string) {
G.Pkgsrc.PkgOptions[name] = description
}
diff --git a/pkgtools/pkglint/files/linelexer.go b/pkgtools/pkglint/files/linelexer.go
index cc9bda070d0..9c3f10caa02 100644
--- a/pkgtools/pkglint/files/linelexer.go
+++ b/pkgtools/pkglint/files/linelexer.go
@@ -131,16 +131,6 @@ func (mlex *MkLinesLexer) CurrentMkLine() *MkLine {
return mlex.mklines.mklines[mlex.index]
}
-func (mlex *MkLinesLexer) SkipWhile(pred func(mkline *MkLine) bool) {
- if trace.Tracing {
- defer trace.Call(mlex.CurrentMkLine().Text)()
- }
-
- for !mlex.EOF() && pred(mlex.CurrentMkLine()) {
- mlex.Skip()
- }
-}
-
func (mlex *MkLinesLexer) SkipIf(pred func(mkline *MkLine) bool) bool {
if !mlex.EOF() && pred(mlex.CurrentMkLine()) {
mlex.Skip()
diff --git a/pkgtools/pkglint/files/lines.go b/pkgtools/pkglint/files/lines.go
index 7d76fb318ea..339c2f6e48a 100644
--- a/pkgtools/pkglint/files/lines.go
+++ b/pkgtools/pkglint/files/lines.go
@@ -21,13 +21,9 @@ func (ls *Lines) LastLine() *Line { return ls.Lines[ls.Len()-1] }
func (ls *Lines) EOFLine() *Line { return NewLineMulti(ls.Filename, -1, -1, "", nil) }
-func (ls *Lines) Errorf(format string, args ...interface{}) {
- NewLineWhole(ls.Filename).Errorf(format, args...)
-}
-
-func (ls *Lines) Warnf(format string, args ...interface{}) {
- NewLineWhole(ls.Filename).Warnf(format, args...)
-}
+// Whole returns a virtual line that can be used for issuing diagnostics
+// and explanations, but not for text replacements.
+func (ls *Lines) Whole() *Line { return NewLineWhole(ls.Filename) }
func (ls *Lines) SaveAutofixChanges() bool {
return SaveAutofixChanges(ls)
diff --git a/pkgtools/pkglint/files/mkline.go b/pkgtools/pkglint/files/mkline.go
index 5d85d5f19c0..f2df0a3ddb5 100644
--- a/pkgtools/pkglint/files/mkline.go
+++ b/pkgtools/pkglint/files/mkline.go
@@ -279,6 +279,8 @@ func (mkline *MkLine) MustExist() bool { return mkline.data.(*mkLineInclude).mus
func (mkline *MkLine) IncludedFile() string { return mkline.data.(*mkLineInclude).includedFile }
+// IncludedFileFull returns the path to the included file, relative to the
+// current working directory.
func (mkline *MkLine) IncludedFileFull() string {
return cleanpath(path.Join(path.Dir(mkline.Filename), mkline.IncludedFile()))
}
@@ -325,7 +327,11 @@ func (mkline *MkLine) Tokenize(text string, warn bool) []*MkToken {
if mkline.IsVarassignMaybeCommented() && text == mkline.Value() {
tokens, rest = mkline.ValueTokens()
} else {
- p := NewMkParser(mkline.Line, text)
+ var line *Line
+ if warn {
+ line = mkline.Line
+ }
+ p := NewMkParser(line, text)
tokens = p.MkTokens()
rest = p.Rest()
}
diff --git a/pkgtools/pkglint/files/mkline_test.go b/pkgtools/pkglint/files/mkline_test.go
index 544f768ed99..89f40f79bfa 100644
--- a/pkgtools/pkglint/files/mkline_test.go
+++ b/pkgtools/pkglint/files/mkline_test.go
@@ -1587,9 +1587,7 @@ func (s *Suite) Test_MkLine_UnquoteShell(c *check.C) {
test("\"\\", "")
test("'", "")
- test("\"$(\"", "$(\"",
- "WARN: filename.mk:1: Missing closing \")\" for \"\\\"\".",
- "WARN: filename.mk:1: Invalid part \"\\\"\" after variable name \"\".")
+ test("\"$(\"", "$(\"")
test("`", "`")
diff --git a/pkgtools/pkglint/files/mklinechecker.go b/pkgtools/pkglint/files/mklinechecker.go
index df63a57fa7c..bf86857e790 100644
--- a/pkgtools/pkglint/files/mklinechecker.go
+++ b/pkgtools/pkglint/files/mklinechecker.go
@@ -1588,7 +1588,7 @@ func (ck MkLineChecker) simplifyCondition(varuse *MkVarUse, fromEmpty bool, notE
pattern +
condStr(fromEmpty, ")", "}")
- quote := condStr(matches(pattern, `[^\-/0-9@A-Za-z]`), "\"", "")
+ quote := condStr(matches(pattern, `[^\-/0-9@A-Z_a-z]`), "\"", "")
to := "${" + varname + "} " + op + " " + quote + pattern + quote
return from, to
}
diff --git a/pkgtools/pkglint/files/mklinechecker_test.go b/pkgtools/pkglint/files/mklinechecker_test.go
index d51ac81585d..cc61459ac06 100644
--- a/pkgtools/pkglint/files/mklinechecker_test.go
+++ b/pkgtools/pkglint/files/mklinechecker_test.go
@@ -2125,6 +2125,14 @@ func (s *Suite) Test_MkLineChecker_checkDirectiveCondEmpty(c *check.C) {
"NOTE: module.mk:2: PKGPATH should be compared using == instead of matching against \":Mtwo\".",
".if ${PKGPATH:Mone:Mtwo}")
+
+ test(
+ ".if ${MACHINE_ARCH:Mx86_64}",
+
+ "NOTE: module.mk:2: MACHINE_ARCH should be compared using == instead of matching against \":Mx86_64\".",
+ "AUTOFIX: module.mk:2: Replacing \"${MACHINE_ARCH:Mx86_64}\" with \"${MACHINE_ARCH} == x86_64\".",
+
+ ".if ${MACHINE_ARCH} == x86_64")
}
func (s *Suite) Test_MkLineChecker_checkDirectiveCond__comparing_PKGSRC_COMPILER_with_eqeq(c *check.C) {
diff --git a/pkgtools/pkglint/files/mklines.go b/pkgtools/pkglint/files/mklines.go
index f91d65cec86..eca17c26969 100644
--- a/pkgtools/pkglint/files/mklines.go
+++ b/pkgtools/pkglint/files/mklines.go
@@ -70,6 +70,10 @@ func NewMkLines(lines *Lines) *MkLines {
// ck.AfterLine
// ck.Finish
+// Whole returns a virtual line that can be used for issuing diagnostics
+// and explanations, but not for text replacements.
+func (mklines *MkLines) Whole() *Line { return mklines.lines.Whole() }
+
// UseVar remembers that the given variable is used in the given line.
// This controls the "defined but not used" warning.
func (mklines *MkLines) UseVar(mkline *MkLine, varname string, time VucTime) {
diff --git a/pkgtools/pkglint/files/options.go b/pkgtools/pkglint/files/options.go
index 2100fadcbc0..9777fa8ff9b 100755
--- a/pkgtools/pkglint/files/options.go
+++ b/pkgtools/pkglint/files/options.go
@@ -3,7 +3,9 @@ package pkglint
func CheckLinesOptionsMk(mklines *MkLines) {
ck := OptionsLinesChecker{
mklines,
+ false,
make(map[string]*MkLine),
+ false,
make(map[string]*MkLine),
nil}
@@ -16,7 +18,9 @@ func CheckLinesOptionsMk(mklines *MkLines) {
type OptionsLinesChecker struct {
mklines *MkLines
+ declaredArbitrary bool
declaredOptions map[string]*MkLine
+ handledArbitrary bool
handledOptions map[string]*MkLine
optionsInDeclarationOrder []string
}
@@ -26,94 +30,91 @@ func (ck *OptionsLinesChecker) Check() {
mklines.Check()
- mlex := NewMkLinesLexer(mklines)
- mlex.SkipWhile(func(mkline *MkLine) bool { return mkline.IsComment() || mkline.IsEmpty() })
-
- if !ck.lookingAtPkgOptionsVar(mlex) {
- return
- }
- mlex.Skip()
-
- upper := true
- for !mlex.EOF() && upper {
- upper = ck.handleUpperLine(mlex.CurrentMkLine())
- mlex.Skip()
- }
-
- i := 0
- mklines.ForEach(func(mkline *MkLine) {
- if i >= mlex.index {
- ck.handleLowerLine(mkline)
- }
- i++
- })
+ ck.collect()
ck.checkOptionsMismatch()
mklines.SaveAutofixChanges()
}
-func (ck *OptionsLinesChecker) lookingAtPkgOptionsVar(mlex *MkLinesLexer) bool {
- if !mlex.EOF() {
- mkline := mlex.CurrentMkLine()
- if mkline.IsVarassign() && mkline.Varname() == "PKG_OPTIONS_VAR" {
- return true
+func (ck *OptionsLinesChecker) collect() {
+ seenPkgOptionsVar := false
+ seenInclude := false
+
+ ck.mklines.ForEach(func(mkline *MkLine) {
+ if mkline.IsEmpty() || mkline.IsComment() {
+ return
+ }
+
+ if !seenInclude {
+ if !seenPkgOptionsVar && mkline.IsVarassign() && mkline.Varname() == "PKG_OPTIONS_VAR" {
+ seenPkgOptionsVar = true
+ }
+ seenInclude = mkline.IsInclude() && mkline.IncludedFile() == "../../mk/bsd.options.mk"
+ }
+
+ if !seenInclude {
+ ck.handleUpperLine(mkline, seenPkgOptionsVar)
+ } else {
+ ck.handleLowerLine(mkline)
}
+ })
+
+ if !seenPkgOptionsVar {
+ ck.mklines.Whole().Errorf("Each options.mk file must define PKG_OPTIONS_VAR.")
}
- line := mlex.CurrentLine()
- line.Warnf("Expected definition of PKG_OPTIONS_VAR.")
- line.Explain(
- "The input variables in an options.mk file should always be",
- "mentioned in the same order: PKG_OPTIONS_VAR,",
- "PKG_SUPPORTED_OPTIONS, PKG_SUGGESTED_OPTIONS.",
- "This way, the options.mk files have the same structure and are easy to understand.")
- return false
+ if !seenInclude {
+ file := ck.mklines.Whole()
+ file.Errorf("Each options.mk file must .include \"../../mk/bsd.options.mk\".")
+ file.Explain(
+ "After defining the input variables (PKG_OPTIONS_VAR, etc.),",
+ "bsd.options.mk must be included to do the actual processing.")
+ }
}
// handleUpperLine checks a line from the upper part of an options.mk file,
// before bsd.options.mk is included.
-func (ck *OptionsLinesChecker) handleUpperLine(mkline *MkLine) bool {
- switch {
- case mkline.IsComment():
- break
- case mkline.IsEmpty():
- break
-
- case mkline.IsVarassign():
- switch mkline.Varcanon() {
- case "PKG_SUPPORTED_OPTIONS",
- "PKG_SUPPORTED_OPTIONS.*",
- "PKG_OPTIONS_GROUP.*",
- "PKG_OPTIONS_SET.*":
- for _, option := range mkline.ValueFields(mkline.Value()) {
- if !containsVarRef(option) {
- ck.declaredOptions[option] = mkline
- ck.optionsInDeclarationOrder = append(ck.optionsInDeclarationOrder, option)
- }
- }
+func (ck *OptionsLinesChecker) handleUpperLine(mkline *MkLine, seenPkgOptionsVar bool) {
+
+ declare := func(option string) {
+ if containsVarRef(option) {
+ ck.declaredArbitrary = true
+ } else {
+ ck.declaredOptions[option] = mkline
+ ck.optionsInDeclarationOrder = append(ck.optionsInDeclarationOrder, option)
}
+ }
- case mkline.IsDirective():
- // The conditionals are typically for OPSYS and MACHINE_ARCH.
+ if !mkline.IsVarassign() {
+ return
+ }
- case mkline.IsInclude():
- if mkline.IncludedFile() == "../../mk/bsd.options.mk" {
- return false
+ switch mkline.Varcanon() {
+ case "PKG_SUPPORTED_OPTIONS",
+ "PKG_SUPPORTED_OPTIONS.*",
+ "PKG_OPTIONS_GROUP.*",
+ "PKG_OPTIONS_SET.*":
+ if !seenPkgOptionsVar {
+ ck.warnVarorder(mkline)
}
- default:
- line := mkline
- line.Warnf("Expected inclusion of \"../../mk/bsd.options.mk\".")
- line.Explain(
- "After defining the input variables (PKG_OPTIONS_VAR, etc.),",
- "bsd.options.mk should be included to do the actual processing.",
- "No other actions should take place in this part of the file",
- "in order to have the same structure in all options.mk files.")
- return false
+ for _, option := range mkline.ValueFields(mkline.Value()) {
+ if optionVarUse := ToVarUse(option); optionVarUse != nil {
+ forVars := ck.mklines.ExpandLoopVar(optionVarUse.varname)
+ for _, option := range forVars {
+ declare(option)
+ }
+ if len(forVars) == 0 {
+ for _, option := range mkline.ValueFields(resolveVariableRefs(ck.mklines, option)) {
+ declare(option)
+ }
+ }
+ } else {
+ declare(option)
+ }
+ }
}
-
- return true
}
func (ck *OptionsLinesChecker) handleLowerLine(mkline *MkLine) {
@@ -138,6 +139,7 @@ func (ck *OptionsLinesChecker) handleLowerCondition(mkline *MkLine, cond *MkCond
recordOption := func(option string) {
if containsVarRef(option) {
+ ck.handledArbitrary = true
return
}
@@ -164,11 +166,16 @@ func (ck *OptionsLinesChecker) handleLowerCondition(mkline *MkLine, cond *MkCond
recordOption(pattern)
} else {
+ matched := false
for declaredOption := range ck.declaredOptions {
if pathMatches(pattern, declaredOption) {
+ matched = true
recordOption(declaredOption)
}
}
+ if !matched {
+ ck.handledArbitrary = true
+ }
}
}
@@ -199,13 +206,13 @@ func (ck *OptionsLinesChecker) checkOptionsMismatch() {
handled := ck.handledOptions[option]
switch {
- case handled == nil:
+ case handled == nil && !ck.handledArbitrary:
declared.Warnf("Option %q should be handled below in an .if block.", option)
declared.Explain(
"If an option is not processed in this file, it may either be a",
"typo, or the option does not have any effect.")
- case declared == nil:
+ case declared == nil && !ck.declaredArbitrary:
handled.Warnf("Option %q is handled but not added to PKG_SUPPORTED_OPTIONS.", option)
handled.Explain(
"This block of code will never be run since PKG_OPTIONS cannot",
@@ -214,3 +221,13 @@ func (ck *OptionsLinesChecker) checkOptionsMismatch() {
}
}
}
+
+func (ck *OptionsLinesChecker) warnVarorder(mkline *MkLine) {
+ mkline.Warnf("Expected definition of PKG_OPTIONS_VAR.")
+ mkline.Explain(
+ "The input variables in an options.mk file should always be",
+ "mentioned in the same order: PKG_OPTIONS_VAR,",
+ "PKG_SUPPORTED_OPTIONS, PKG_SUGGESTED_OPTIONS.",
+ "",
+ "This way, the options.mk files have the same structure and are easy to understand.")
+}
diff --git a/pkgtools/pkglint/files/options_test.go b/pkgtools/pkglint/files/options_test.go
index 61986f2af9d..978888d1de8 100755
--- a/pkgtools/pkglint/files/options_test.go
+++ b/pkgtools/pkglint/files/options_test.go
@@ -2,6 +2,107 @@ package pkglint
import "gopkg.in/check.v1"
+func (s *Suite) Test_CheckLinesOptionsMk__literal(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpOption("declared", "")
+ t.SetUpOption("both", "")
+ t.SetUpOption("handled", "")
+ t.SetUpPackage("category/package",
+ ".include \"../../mk/bsd.options.mk\"")
+ t.CreateFileLines("mk/bsd.options.mk",
+ MkCvsID)
+ mklines := t.SetUpFileMkLines("category/package/options.mk",
+ MkCvsID,
+ "",
+ "PKG_OPTIONS_VAR=\tPKG_OPTIONS.package",
+ "PKG_SUPPORTED_OPTIONS=\tdeclared both",
+ "",
+ ".include \"../../mk/bsd.options.mk\"",
+ "",
+ ".if ${PKG_OPTIONS:Mboth}",
+ ".endif",
+ "",
+ ".if ${PKG_OPTIONS:Mhandled}",
+ ".endif")
+ t.Chdir("category/package")
+ t.FinishSetUp()
+
+ CheckLinesOptionsMk(mklines)
+
+ t.CheckOutputLines(
+ "WARN: ~/category/package/options.mk:4: "+
+ "Option \"declared\" should be handled below in an .if block.",
+ "WARN: ~/category/package/options.mk:11: "+
+ "Option \"handled\" is handled but not added to PKG_SUPPORTED_OPTIONS.")
+}
+
+func (s *Suite) Test_CheckLinesOptionsMk__literal_in_for_loop(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpOption("declared", "")
+ t.SetUpOption("both", "")
+ t.SetUpOption("handled", "")
+ t.SetUpPackage("category/package",
+ ".include \"../../mk/bsd.options.mk\"")
+ t.CreateFileLines("mk/bsd.options.mk",
+ MkCvsID)
+ mklines := t.SetUpFileMkLines("category/package/options.mk",
+ MkCvsID,
+ "",
+ "PKG_OPTIONS_VAR=\tPKG_OPTIONS.package",
+ ".for declared_option in declared both",
+ "PKG_SUPPORTED_OPTIONS=\t${declared_option}",
+ ".endfor",
+ "",
+ ".include \"../../mk/bsd.options.mk\"",
+ "",
+ ".for handled_option in both handled",
+ ". if ${PKG_OPTIONS:M${handled_option}}",
+ ". endif",
+ ".endfor")
+ t.Chdir("category/package")
+ t.FinishSetUp()
+
+ CheckLinesOptionsMk(mklines)
+
+ t.CheckOutputLines(
+ "WARN: ~/category/package/options.mk:5: "+
+ "Option \"declared\" should be handled below in an .if block.",
+ "WARN: ~/category/package/options.mk:11: "+
+ "Option \"handled\" is handled but not added to PKG_SUPPORTED_OPTIONS.")
+}
+
+// Before version 19.3.5, pkglint warned when bsd.prefs.mk was
+// included in the top half of the file.
+func (s *Suite) Test_CheckLinesOptionsMk__prefs(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpOption("option", "")
+ t.SetUpPackage("category/package",
+ ".include \"../../mk/bsd.options.mk\"")
+ t.CreateFileLines("mk/bsd.options.mk",
+ MkCvsID)
+ mklines := t.SetUpFileMkLines("category/package/options.mk",
+ MkCvsID,
+ "",
+ ".include \"../../mk/bsd.prefs.mk\"",
+ "",
+ "PKG_OPTIONS_VAR=\tPKG_OPTIONS.package",
+ "PKG_SUPPORTED_OPTIONS=\toption",
+ "",
+ ".include \"../../mk/bsd.options.mk\"",
+ "",
+ ".if ${PKG_OPTIONS:Moption}",
+ ".endif")
+ t.Chdir("category/package")
+ t.FinishSetUp()
+
+ CheckLinesOptionsMk(mklines)
+
+ t.CheckOutputEmpty()
+}
+
func (s *Suite) Test_CheckLinesOptionsMk(c *check.C) {
t := s.Init(c)
@@ -63,9 +164,10 @@ func (s *Suite) Test_CheckLinesOptionsMk(c *check.C) {
"The positive branch of the .if/.else should be the one where the option is set.",
// TODO: The diagnostics should appear in the correct order.
"WARN: ~/category/package/options.mk:6: "+
- "Option \"mc-charset\" should be handled below in an .if block.",
- "WARN: ~/category/package/options.mk:18: "+
- "Option \"undeclared\" is handled but not added to PKG_SUPPORTED_OPTIONS.")
+ "Option \"mc-charset\" should be handled below in an .if block.")
+ // TODO: There is no warning for the option "undeclared" since
+ // the option lang-${l} sets declaredArbitrary. This in turn
+ // disables possible wrong warnings, but a few too many.
}
// This test is provided for code coverage. Similarities to existing files are purely coincidental.
@@ -87,7 +189,8 @@ func (s *Suite) Test_CheckLinesOptionsMk__edge_cases(c *check.C) {
CheckLinesOptionsMk(mklines)
t.CheckOutputLines(
- "WARN: ~/category/package/options.mk:EOF: Expected definition of PKG_OPTIONS_VAR.")
+ "ERROR: ~/category/package/options.mk: Each options.mk file must define PKG_OPTIONS_VAR.",
+ "ERROR: ~/category/package/options.mk: Each options.mk file must .include \"../../mk/bsd.options.mk\".")
mklines = t.SetUpFileMkLines("category/package/options.mk",
MkCvsID,
@@ -96,7 +199,14 @@ func (s *Suite) Test_CheckLinesOptionsMk__edge_cases(c *check.C) {
CheckLinesOptionsMk(mklines)
t.CheckOutputLines(
- "WARN: ~/category/package/options.mk:2: Expected definition of PKG_OPTIONS_VAR.")
+ "WARN: ~/category/package/options.mk:2: "+
+ "Expected definition of PKG_OPTIONS_VAR.",
+ "ERROR: ~/category/package/options.mk: "+
+ "Each options.mk file must define PKG_OPTIONS_VAR.",
+ "ERROR: ~/category/package/options.mk: "+
+ "Each options.mk file must .include \"../../mk/bsd.options.mk\".",
+ "WARN: ~/category/package/options.mk:2: "+
+ "Option \"option1\" should be handled below in an .if block.")
mklines = t.SetUpFileMkLines("category/package/options.mk",
MkCvsID,
@@ -107,7 +217,9 @@ func (s *Suite) Test_CheckLinesOptionsMk__edge_cases(c *check.C) {
CheckLinesOptionsMk(mklines)
t.CheckOutputLines(
- "WARN: ~/category/package/options.mk:3: " +
+ "ERROR: ~/category/package/options.mk: "+
+ "Each options.mk file must .include \"../../mk/bsd.options.mk\".",
+ "WARN: ~/category/package/options.mk:3: "+
"Option \"option1\" should be handled below in an .if block.")
mklines = t.SetUpFileMkLines("category/package/options.mk",
@@ -155,7 +267,8 @@ func (s *Suite) Test_CheckLinesOptionsMk__unexpected_line(c *check.C) {
CheckLinesOptionsMk(mklines)
t.CheckOutputLines(
- "WARN: ~/category/package/options.mk:5: Expected inclusion of \"../../mk/bsd.options.mk\".")
+ "ERROR: ~/category/package/options.mk: " +
+ "Each options.mk file must .include \"../../mk/bsd.options.mk\".")
}
func (s *Suite) Test_CheckLinesOptionsMk__malformed_condition(c *check.C) {
@@ -382,10 +495,17 @@ func (s *Suite) Test_CheckLinesOptionsMk__combined_option_handling_coverage(c *c
G.Check(".")
- // The warning appears because the pattern "opt-[" is malformed
- // and therefore doesn't match the option.
- t.CheckOutputLines(
- "WARN: options.mk:4: Option \"opt-variant\" should be handled below in an .if block.")
+ // The pattern "opt-[" does not match any of the declared options
+ // since the pattern is malformed and pkglint does not distinguish
+ // between invalid and non-matching patterns.
+ //
+ // The pattern "other-*" also doesn't match.
+ //
+ // Since the patterns don't match any of the variables from
+ // PKG_SUPPORTED_OPTIONS, pkglint cannot analyze all possible cases
+ // and therefore suppresses all warnings about options that are
+ // declared but not handled.
+ t.CheckOutputEmpty()
}
func (s *Suite) Test_CheckLinesOptionsMk__options_in_for_loop(c *check.C) {
@@ -485,3 +605,85 @@ func (s *Suite) Test_CheckLinesOptionsMk__partly_indirect(c *check.C) {
t.CheckOutputEmpty()
}
+
+func (s *Suite) Test_CheckLinesOptionsMk__indirect_supported_options_parentheses(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpOption("indirect", "")
+ t.SetUpOption("direct", "")
+ t.SetUpVartypes()
+ t.CreateFileLines("mk/bsd.options.mk",
+ MkCvsID)
+ mklines := t.SetUpFileMkLines("category/package/options.mk",
+ MkCvsID,
+ "",
+ "PKG_OPTIONS_VAR=\tPKG_OPTIONS.package",
+ "OPTIONS=\t\tindirect",
+ "PKG_SUPPORTED_OPTIONS=\t$(OPTIONS) direct",
+ "",
+ ".include \"../../mk/bsd.options.mk\"",
+ "",
+ ".for option in ${OPTIONS}",
+ ". if ${PKG_OPTIONS:M${option}}",
+ ". endif",
+ ".endfor")
+
+ CheckLinesOptionsMk(mklines)
+
+ t.CheckOutputLines(
+ "WARN: ~/category/package/options.mk:5: "+
+ "Please use curly braces {} instead of round parentheses () for OPTIONS.",
+ "WARN: ~/category/package/options.mk:5: "+
+ "Option \"direct\" should be handled below in an .if block.")
+}
+
+func (s *Suite) Test_CheckLinesOptionsMk__handled_but_not_supported(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpOption("option", "")
+ t.SetUpVartypes()
+ t.CreateFileLines("mk/bsd.options.mk",
+ MkCvsID)
+ mklines := t.SetUpFileMkLines("category/package/options.mk",
+ MkCvsID,
+ "",
+ "PKG_OPTIONS_VAR=\tPKG_OPTIONS.package",
+ "PKG_SUPPORTED_OPTIONS=\t# none",
+ "",
+ ".include \"../../mk/bsd.options.mk\"",
+ "",
+ ".if ${PKG_OPTIONS:Moption}",
+ ".endif")
+
+ CheckLinesOptionsMk(mklines)
+
+ t.CheckOutputLines(
+ "WARN: ~/category/package/options.mk:8: " +
+ "Option \"option\" is handled but not added to PKG_SUPPORTED_OPTIONS.")
+}
+
+func (s *Suite) Test_CheckLinesOptionsMk__supported_but_not_checked(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpOption("option", "")
+ t.SetUpVartypes()
+ t.CreateFileLines("mk/bsd.options.mk",
+ MkCvsID)
+ mklines := t.SetUpFileMkLines("category/package/options.mk",
+ MkCvsID,
+ "",
+ "PKG_OPTIONS_VAR=\tPKG_OPTIONS.package",
+ "PKG_SUPPORTED_OPTIONS=\toption",
+ "",
+ ".include \"../../mk/bsd.options.mk\"",
+ "",
+ ".if ${PKG_OPTIONS:Mopt${:Uion}}",
+ ".endif")
+
+ CheckLinesOptionsMk(mklines)
+
+ // Pkglint does not expand the ${:Uion}, therefore it doesn't know that
+ // the option is indeed handled. Because of this uncertainty, pkglint
+ // does not issue any warnings about possibly unhandled options at all.
+ t.CheckOutputEmpty()
+}
diff --git a/pkgtools/pkglint/files/package.go b/pkgtools/pkglint/files/package.go
index bdfd012dc4e..8e50d3e6ca8 100644
--- a/pkgtools/pkglint/files/package.go
+++ b/pkgtools/pkglint/files/package.go
@@ -627,9 +627,12 @@ func (pkg *Package) checkfilePackageMakefile(filename string, mklines *MkLines,
vars := pkg.vars
pkg.checkPlist()
- if (vars.Defined("NO_CHECKSUM") || vars.Defined("META_PACKAGE")) &&
- isEmptyDir(pkg.File(pkg.Patchdir)) {
+ want := !vars.Defined("NO_CHECKSUM")
+ want = want && !vars.Defined("META_PACKAGE")
+ want = want && !(vars.Defined("DISTFILES") && vars.LastValue("DISTFILES") == "")
+ want = want || !isEmptyDir(pkg.File(pkg.Patchdir))
+ if !want {
distinfoFile := pkg.File(pkg.DistinfoFile)
if fileExists(distinfoFile) {
NewLineWhole(distinfoFile).Warnf("This file should not exist since NO_CHECKSUM or META_PACKAGE is set.")
diff --git a/pkgtools/pkglint/files/package_test.go b/pkgtools/pkglint/files/package_test.go
index b6a8fd6091e..abb70719ea0 100644
--- a/pkgtools/pkglint/files/package_test.go
+++ b/pkgtools/pkglint/files/package_test.go
@@ -1263,12 +1263,18 @@ func (s *Suite) Test_Package_checkIncludeConditionally__conditional_and_uncondit
".if ${OPSYS} == \"Linux\"",
".include \"../../sysutils/coreutils/buildlink3.mk\"",
".endif")
+ t.CreateFileLines("mk/bsd.options.mk", "")
t.CreateFileLines("devel/zlib/buildlink3.mk", "")
t.CreateFileLines("sysutils/coreutils/buildlink3.mk", "")
t.CreateFileLines("category/package/options.mk",
MkCvsID,
"",
+ "PKG_OPTIONS_VAR=\tPKG_OPTIONS.package",
+ "PKG_SUPPORTED_OPTIONS=\tzlib",
+ "",
+ ".include \"../../mk/bsd.options.mk\"",
+ "",
".if !empty(PKG_OPTIONS:Mzlib)",
". include \"../../devel/zlib/buildlink3.mk\"",
".endif",
@@ -1281,11 +1287,10 @@ func (s *Suite) Test_Package_checkIncludeConditionally__conditional_and_uncondit
t.CheckOutputLines(
"WARN: Makefile:20: \"../../devel/zlib/buildlink3.mk\" is included "+
"unconditionally here "+
- "and conditionally in options.mk:4 (depending on PKG_OPTIONS).",
+ "and conditionally in options.mk:9 (depending on PKG_OPTIONS).",
"WARN: Makefile:22: \"../../sysutils/coreutils/buildlink3.mk\" is included "+
"conditionally here (depending on OPSYS) and "+
- "unconditionally in options.mk:6.",
- "WARN: options.mk:3: Expected definition of PKG_OPTIONS_VAR.")
+ "unconditionally in options.mk:11.")
}
func (s *Suite) Test_Package_checkIncludeConditionally__explain_PKG_OPTIONS_in_Makefile(c *check.C) {
@@ -1861,6 +1866,37 @@ func (s *Suite) Test_Package_checkfilePackageMakefile__files_Makefile(c *check.C
"ERROR: ~/category/package/Makefile: Each package must define its LICENSE.")
}
+// Until version 19.3.5, pkglint warned that this package would need a
+// distinfo file.
+func (s *Suite) Test_Package_checkfilePackageMakefile__no_distfiles(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package",
+ "DISTFILES=\t# none")
+ t.FinishSetUp()
+
+ G.Check(t.File("category/package"))
+
+ t.CheckOutputLines(
+ "WARN: ~/category/package/distinfo: " +
+ "This file should not exist since NO_CHECKSUM or META_PACKAGE is set.")
+}
+
+func (s *Suite) Test_Package_checkfilePackageMakefile__distfiles(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package",
+ "DISTFILES=\tpackage-1.0.tar.gz")
+ t.Remove("category/package/distinfo")
+ t.FinishSetUp()
+
+ G.Check(t.File("category/package"))
+
+ t.CheckOutputLines(
+ "WARN: ~/category/package/distinfo: " +
+ "A package that downloads files should have a distinfo file.")
+}
+
func (s *Suite) Test_Package_checkGnuConfigureUseLanguages__no_C(c *check.C) {
t := s.Init(c)
diff --git a/pkgtools/pkglint/files/patches.go b/pkgtools/pkglint/files/patches.go
index ad2532e4f3f..df64453d1d8 100644
--- a/pkgtools/pkglint/files/patches.go
+++ b/pkgtools/pkglint/files/patches.go
@@ -79,10 +79,10 @@ func (ck *PatchChecker) Check() {
}
}
- if patchedFiles > 1 {
- ck.lines.Warnf("Contains patches for %d files, should be only one.", patchedFiles)
+ if patchedFiles > 1 && !matches(ck.lines.Filename, `\bCVE\b`) {
+ ck.lines.Whole().Warnf("Contains patches for %d files, should be only one.", patchedFiles)
} else if patchedFiles == 0 {
- ck.lines.Errorf("Contains no patch.")
+ ck.lines.Whole().Errorf("Contains no patch.")
}
CheckLinesTrailingEmptyLines(ck.lines)
diff --git a/pkgtools/pkglint/files/patches_test.go b/pkgtools/pkglint/files/patches_test.go
index e9713b7f246..1b9afcf5930 100644
--- a/pkgtools/pkglint/files/patches_test.go
+++ b/pkgtools/pkglint/files/patches_test.go
@@ -255,6 +255,31 @@ func (s *Suite) Test_CheckLinesPatch__two_patched_files(c *check.C) {
"WARN: patch-aa: Contains patches for 2 files, should be only one.")
}
+func (s *Suite) Test_CheckLinesPatch__two_patched_files_for_CVE(c *check.C) {
+ t := s.Init(c)
+
+ lines := t.NewLines("patch-CVE-2019-0001",
+ CvsID,
+ "",
+ "Patches that are provided by upstream for a specific topic don't",
+ "need to be split artificially.",
+ "",
+ "--- oldfile",
+ "+++ newfile",
+ "@@ -1 +1 @@",
+ "-old",
+ "+new",
+ "--- oldfile2",
+ "+++ newfile2",
+ "@@ -1 +1 @@",
+ "-old",
+ "+new")
+
+ CheckLinesPatch(lines)
+
+ t.CheckOutputEmpty()
+}
+
// The patch headers are only recognized as such if they appear directly below each other.
func (s *Suite) Test_CheckLinesPatch__documentation_that_looks_like_patch_lines(c *check.C) {
t := s.Init(c)
diff --git a/pkgtools/pkglint/files/pkglint.go b/pkgtools/pkglint/files/pkglint.go
index 05efef29622..db3f063bbc6 100644
--- a/pkgtools/pkglint/files/pkglint.go
+++ b/pkgtools/pkglint/files/pkglint.go
@@ -438,7 +438,7 @@ func findPkgsrcTopdir(dirname string) string {
func resolveVariableRefs(mklines *MkLines, text string) (resolved string) {
// TODO: How does this fit into the Scope type, which is newer than this function?
- if !contains(text, "${") {
+ if !containsVarRef(text) {
return text
}
@@ -497,7 +497,7 @@ func CheckLinesDescr(lines *Lines) {
ck.CheckTrailingWhitespace()
ck.CheckValidCharacters()
- if contains(line.Text, "${") {
+ if containsVarRef(line.Text) {
for _, token := range NewMkParser(nil, line.Text).MkTokens() {
if token.Varuse != nil && G.Pkgsrc.VariableType(nil, token.Varuse.varname) != nil {
line.Notef("Variables are not expanded in the DESCR file.")
diff --git a/pkgtools/pkglint/files/util.go b/pkgtools/pkglint/files/util.go
index 70594ac6751..329fa368fe3 100644
--- a/pkgtools/pkglint/files/util.go
+++ b/pkgtools/pkglint/files/util.go
@@ -609,7 +609,7 @@ func pathContainsDir(haystack, needle string) bool {
}
func containsVarRef(s string) bool {
- return contains(s, "${")
+ return contains(s, "${") || contains(s, "$(")
}
func hasAlnumPrefix(s string) bool { return s != "" && textproc.AlnumU.Contains(s[0]) }
diff --git a/pkgtools/pkglint/files/vardefs.go b/pkgtools/pkglint/files/vardefs.go
index 4c98824323e..69858caad6b 100644
--- a/pkgtools/pkglint/files/vardefs.go
+++ b/pkgtools/pkglint/files/vardefs.go
@@ -1407,7 +1407,7 @@ func (reg *VarTypeRegistry) Init(src *Pkgsrc) {
reg.acllist("PKG_HACKS", BtIdentifier,
PackageSettable,
"*: none")
- reg.sys("PKG_INFO", BtShellCommand)
+ reg.sysload("PKG_INFO", BtShellCommand)
reg.sys("PKG_JAVA_HOME", BtPathname)
reg.sys("PKG_JVM", jvms)
reg.pkglistrat("PKG_JVMS_ACCEPTED", jvms)