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:           Sat Nov  2 16:37:48 UTC 2019

Modified Files:
        pkgsrc/pkgtools/pkglint: Makefile
        pkgsrc/pkgtools/pkglint/files: check_test.go linelexer.go lines.go
            mkline.go mkline_test.go mklinechecker.go mklinechecker_test.go
            mklines.go options.go options_test.go package.go package_test.go
            patches.go patches_test.go pkglint.go util.go vardefs.go

Log Message:
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.


To generate a diff of this commit:
cvs rdiff -u -r1.604 -r1.605 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.52 -r1.53 pkgsrc/pkgtools/pkglint/files/check_test.go
cvs rdiff -u -r1.5 -r1.6 pkgsrc/pkgtools/pkglint/files/linelexer.go
cvs rdiff -u -r1.8 -r1.9 pkgsrc/pkgtools/pkglint/files/lines.go
cvs rdiff -u -r1.61 -r1.62 pkgsrc/pkgtools/pkglint/files/mkline.go
cvs rdiff -u -r1.69 -r1.70 pkgsrc/pkgtools/pkglint/files/mkline_test.go
cvs rdiff -u -r1.50 -r1.51 pkgsrc/pkgtools/pkglint/files/mklinechecker.go
cvs rdiff -u -r1.45 -r1.46 \
    pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
cvs rdiff -u -r1.57 -r1.58 pkgsrc/pkgtools/pkglint/files/mklines.go
cvs rdiff -u -r1.18 -r1.19 pkgsrc/pkgtools/pkglint/files/options.go
cvs rdiff -u -r1.16 -r1.17 pkgsrc/pkgtools/pkglint/files/options_test.go
cvs rdiff -u -r1.66 -r1.67 pkgsrc/pkgtools/pkglint/files/package.go
cvs rdiff -u -r1.56 -r1.57 pkgsrc/pkgtools/pkglint/files/package_test.go \
    pkgsrc/pkgtools/pkglint/files/util.go
cvs rdiff -u -r1.31 -r1.32 pkgsrc/pkgtools/pkglint/files/patches.go
cvs rdiff -u -r1.30 -r1.31 pkgsrc/pkgtools/pkglint/files/patches_test.go
cvs rdiff -u -r1.62 -r1.63 pkgsrc/pkgtools/pkglint/files/pkglint.go
cvs rdiff -u -r1.74 -r1.75 pkgsrc/pkgtools/pkglint/files/vardefs.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.604 pkgsrc/pkgtools/pkglint/Makefile:1.605
--- pkgsrc/pkgtools/pkglint/Makefile:1.604      Fri Nov  1 19:56:52 2019
+++ pkgsrc/pkgtools/pkglint/Makefile    Sat Nov  2 16:37:48 2019
@@ -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/}

Index: pkgsrc/pkgtools/pkglint/files/check_test.go
diff -u pkgsrc/pkgtools/pkglint/files/check_test.go:1.52 pkgsrc/pkgtools/pkglint/files/check_test.go:1.53
--- pkgsrc/pkgtools/pkglint/files/check_test.go:1.52    Fri Nov  1 19:56:53 2019
+++ pkgsrc/pkgtools/pkglint/files/check_test.go Sat Nov  2 16:37:48 2019
@@ -194,6 +194,8 @@ func (t *Tester) SetUpMasterSite(varname
 }
 
 // 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
 }

Index: pkgsrc/pkgtools/pkglint/files/linelexer.go
diff -u pkgsrc/pkgtools/pkglint/files/linelexer.go:1.5 pkgsrc/pkgtools/pkglint/files/linelexer.go:1.6
--- pkgsrc/pkgtools/pkglint/files/linelexer.go:1.5      Mon Jul  1 22:25:52 2019
+++ pkgsrc/pkgtools/pkglint/files/linelexer.go  Sat Nov  2 16:37:48 2019
@@ -131,16 +131,6 @@ func (mlex *MkLinesLexer) CurrentMkLine(
        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()

Index: pkgsrc/pkgtools/pkglint/files/lines.go
diff -u pkgsrc/pkgtools/pkglint/files/lines.go:1.8 pkgsrc/pkgtools/pkglint/files/lines.go:1.9
--- pkgsrc/pkgtools/pkglint/files/lines.go:1.8  Sun Sep  8 22:47:47 2019
+++ pkgsrc/pkgtools/pkglint/files/lines.go      Sat Nov  2 16:37:48 2019
@@ -21,13 +21,9 @@ func (ls *Lines) LastLine() *Line { retu
 
 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)

Index: pkgsrc/pkgtools/pkglint/files/mkline.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline.go:1.61 pkgsrc/pkgtools/pkglint/files/mkline.go:1.62
--- pkgsrc/pkgtools/pkglint/files/mkline.go:1.61        Sat Oct 26 11:43:36 2019
+++ pkgsrc/pkgtools/pkglint/files/mkline.go     Sat Nov  2 16:37:48 2019
@@ -279,6 +279,8 @@ func (mkline *MkLine) MustExist() bool {
 
 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 stri
        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()
        }

Index: pkgsrc/pkgtools/pkglint/files/mkline_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.69 pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.70
--- pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.69   Sat Oct 26 09:51:48 2019
+++ pkgsrc/pkgtools/pkglint/files/mkline_test.go        Sat Nov  2 16:37:48 2019
@@ -1587,9 +1587,7 @@ func (s *Suite) Test_MkLine_UnquoteShell
        test("\"\\", "")
        test("'", "")
 
-       test("\"$(\"", "$(\"",
-               "WARN: filename.mk:1: Missing closing \")\" for \"\\\"\".",
-               "WARN: filename.mk:1: Invalid part \"\\\"\" after variable name \"\".")
+       test("\"$(\"", "$(\"")
 
        test("`", "`")
 

Index: pkgsrc/pkgtools/pkglint/files/mklinechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.50 pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.51
--- pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.50 Fri Nov  1 19:56:53 2019
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker.go      Sat Nov  2 16:37:48 2019
@@ -1588,7 +1588,7 @@ func (ck MkLineChecker) simplifyConditio
                        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
        }

Index: pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.45 pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.46
--- pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.45    Sat Oct 26 11:43:36 2019
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go Sat Nov  2 16:37:48 2019
@@ -2125,6 +2125,14 @@ func (s *Suite) Test_MkLineChecker_check
                "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) {

Index: pkgsrc/pkgtools/pkglint/files/mklines.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines.go:1.57 pkgsrc/pkgtools/pkglint/files/mklines.go:1.58
--- pkgsrc/pkgtools/pkglint/files/mklines.go:1.57       Sat Oct 26 11:43:36 2019
+++ pkgsrc/pkgtools/pkglint/files/mklines.go    Sat Nov  2 16:37:48 2019
@@ -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) {

Index: pkgsrc/pkgtools/pkglint/files/options.go
diff -u pkgsrc/pkgtools/pkglint/files/options.go:1.18 pkgsrc/pkgtools/pkglint/files/options.go:1.19
--- pkgsrc/pkgtools/pkglint/files/options.go:1.18       Fri Nov  1 19:56:53 2019
+++ pkgsrc/pkgtools/pkglint/files/options.go    Sat Nov  2 16:37:48 2019
@@ -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 *MkLine
 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) handleLow
 
        recordOption := func(option string) {
                if containsVarRef(option) {
+                       ck.handledArbitrary = true
                        return
                }
 
@@ -164,11 +166,16 @@ func (ck *OptionsLinesChecker) handleLow
                        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) checkOpti
                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) checkOpti
                }
        }
 }
+
+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.")
+}

Index: pkgsrc/pkgtools/pkglint/files/options_test.go
diff -u pkgsrc/pkgtools/pkglint/files/options_test.go:1.16 pkgsrc/pkgtools/pkglint/files/options_test.go:1.17
--- pkgsrc/pkgtools/pkglint/files/options_test.go:1.16  Fri Nov  1 19:56:53 2019
+++ pkgsrc/pkgtools/pkglint/files/options_test.go       Sat Nov  2 16:37:48 2019
@@ -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
                        "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
        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
        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
        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
        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
 
        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
 
        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()
+}

Index: pkgsrc/pkgtools/pkglint/files/package.go
diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.66 pkgsrc/pkgtools/pkglint/files/package.go:1.67
--- pkgsrc/pkgtools/pkglint/files/package.go:1.66       Fri Nov  1 19:56:53 2019
+++ pkgsrc/pkgtools/pkglint/files/package.go    Sat Nov  2 16:37:48 2019
@@ -627,9 +627,12 @@ func (pkg *Package) checkfilePackageMake
        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.")

Index: pkgsrc/pkgtools/pkglint/files/package_test.go
diff -u pkgsrc/pkgtools/pkglint/files/package_test.go:1.56 pkgsrc/pkgtools/pkglint/files/package_test.go:1.57
--- pkgsrc/pkgtools/pkglint/files/package_test.go:1.56  Fri Nov  1 19:56:53 2019
+++ pkgsrc/pkgtools/pkglint/files/package_test.go       Sat Nov  2 16:37:48 2019
@@ -1263,12 +1263,18 @@ func (s *Suite) Test_Package_checkInclud
                ".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_checkInclud
        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_checkfilePa
                "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)
 
Index: pkgsrc/pkgtools/pkglint/files/util.go
diff -u pkgsrc/pkgtools/pkglint/files/util.go:1.56 pkgsrc/pkgtools/pkglint/files/util.go:1.57
--- pkgsrc/pkgtools/pkglint/files/util.go:1.56  Fri Nov  1 19:56:53 2019
+++ pkgsrc/pkgtools/pkglint/files/util.go       Sat Nov  2 16:37:48 2019
@@ -609,7 +609,7 @@ func pathContainsDir(haystack, needle st
 }
 
 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]) }

Index: pkgsrc/pkgtools/pkglint/files/patches.go
diff -u pkgsrc/pkgtools/pkglint/files/patches.go:1.31 pkgsrc/pkgtools/pkglint/files/patches.go:1.32
--- pkgsrc/pkgtools/pkglint/files/patches.go:1.31       Sun Jun 30 20:56:19 2019
+++ pkgsrc/pkgtools/pkglint/files/patches.go    Sat Nov  2 16:37:48 2019
@@ -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)

Index: pkgsrc/pkgtools/pkglint/files/patches_test.go
diff -u pkgsrc/pkgtools/pkglint/files/patches_test.go:1.30 pkgsrc/pkgtools/pkglint/files/patches_test.go:1.31
--- pkgsrc/pkgtools/pkglint/files/patches_test.go:1.30  Sun Jun 30 20:56:19 2019
+++ pkgsrc/pkgtools/pkglint/files/patches_test.go       Sat Nov  2 16:37:48 2019
@@ -255,6 +255,31 @@ func (s *Suite) Test_CheckLinesPatch__tw
                "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)

Index: pkgsrc/pkgtools/pkglint/files/pkglint.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint.go:1.62 pkgsrc/pkgtools/pkglint/files/pkglint.go:1.63
--- pkgsrc/pkgtools/pkglint/files/pkglint.go:1.62       Fri Nov  1 19:56:53 2019
+++ pkgsrc/pkgtools/pkglint/files/pkglint.go    Sat Nov  2 16:37:48 2019
@@ -438,7 +438,7 @@ func findPkgsrcTopdir(dirname string) st
 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.")

Index: pkgsrc/pkgtools/pkglint/files/vardefs.go
diff -u pkgsrc/pkgtools/pkglint/files/vardefs.go:1.74 pkgsrc/pkgtools/pkglint/files/vardefs.go:1.75
--- pkgsrc/pkgtools/pkglint/files/vardefs.go:1.74       Fri Nov  1 19:56:53 2019
+++ pkgsrc/pkgtools/pkglint/files/vardefs.go    Sat Nov  2 16:37:48 2019
@@ -1407,7 +1407,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
        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)



Home | Main Index | Thread Index | Old Index