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:           Fri Nov  1 19:56:53 UTC 2019

Modified Files:
        pkgsrc/pkgtools/pkglint: Makefile
        pkgsrc/pkgtools/pkglint/files: category.go category_test.go
            check_test.go mklinechecker.go mktypes.go mktypes_test.go
            options.go options_test.go package.go package_test.go pkglint.go
            toplevel.go util.go vardefs.go

Log Message:
pkgtools/pkglint: update to 19.3.4

Changes since 19.3.3:

In cases where the conditions for including buildlink3.mk files differ
between the package itself and its own buildlink3.mk file, explain how
to determine PKG_OPTIONS for dependencies.

Don't issue wrong warnings in options.mk files when the options are
handled in a .for loop.


To generate a diff of this commit:
cvs rdiff -u -r1.603 -r1.604 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.23 -r1.24 pkgsrc/pkgtools/pkglint/files/category.go
cvs rdiff -u -r1.25 -r1.26 pkgsrc/pkgtools/pkglint/files/category_test.go
cvs rdiff -u -r1.51 -r1.52 pkgsrc/pkgtools/pkglint/files/check_test.go
cvs rdiff -u -r1.49 -r1.50 pkgsrc/pkgtools/pkglint/files/mklinechecker.go
cvs rdiff -u -r1.18 -r1.19 pkgsrc/pkgtools/pkglint/files/mktypes.go
cvs rdiff -u -r1.15 -r1.16 pkgsrc/pkgtools/pkglint/files/mktypes_test.go \
    pkgsrc/pkgtools/pkglint/files/options_test.go
cvs rdiff -u -r1.17 -r1.18 pkgsrc/pkgtools/pkglint/files/options.go
cvs rdiff -u -r1.65 -r1.66 pkgsrc/pkgtools/pkglint/files/package.go
cvs rdiff -u -r1.55 -r1.56 pkgsrc/pkgtools/pkglint/files/package_test.go \
    pkgsrc/pkgtools/pkglint/files/util.go
cvs rdiff -u -r1.61 -r1.62 pkgsrc/pkgtools/pkglint/files/pkglint.go
cvs rdiff -u -r1.22 -r1.23 pkgsrc/pkgtools/pkglint/files/toplevel.go
cvs rdiff -u -r1.73 -r1.74 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.603 pkgsrc/pkgtools/pkglint/Makefile:1.604
--- pkgsrc/pkgtools/pkglint/Makefile:1.603      Sat Oct 26 11:43:36 2019
+++ pkgsrc/pkgtools/pkglint/Makefile    Fri Nov  1 19:56:52 2019
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.603 2019/10/26 11:43:36 rillig Exp $
+# $NetBSD: Makefile,v 1.604 2019/11/01 19:56:52 rillig Exp $
 
-PKGNAME=       pkglint-19.3.3
+PKGNAME=       pkglint-19.3.4
 CATEGORIES=    pkgtools
 DISTNAME=      tools
 MASTER_SITES=  ${MASTER_SITE_GITHUB:=golang/}

Index: pkgsrc/pkgtools/pkglint/files/category.go
diff -u pkgsrc/pkgtools/pkglint/files/category.go:1.23 pkgsrc/pkgtools/pkglint/files/category.go:1.24
--- pkgsrc/pkgtools/pkglint/files/category.go:1.23      Sat Oct 26 09:51:48 2019
+++ pkgsrc/pkgtools/pkglint/files/category.go   Fri Nov  1 19:56:53 2019
@@ -161,6 +161,6 @@ func CheckdirCategory(dir string) {
                                recurseInto = append(recurseInto, joinPath(dir, msub.name))
                        }
                }
-               G.Todo = append(recurseInto, G.Todo...)
+               G.Todo.PushFront(recurseInto...)
        }
 }

Index: pkgsrc/pkgtools/pkglint/files/category_test.go
diff -u pkgsrc/pkgtools/pkglint/files/category_test.go:1.25 pkgsrc/pkgtools/pkglint/files/category_test.go:1.26
--- pkgsrc/pkgtools/pkglint/files/category_test.go:1.25 Sun Sep  8 22:47:47 2019
+++ pkgsrc/pkgtools/pkglint/files/category_test.go      Fri Nov  1 19:56:53 2019
@@ -208,12 +208,12 @@ func (s *Suite) Test_CheckdirCategory__r
        // It is only removed in Pkglint.Main, therefore it stays there even
        // after the call to CheckdirCategory. This is a bit unrealistic,
        // but close enough for this test.
-       t.CheckDeepEquals(G.Todo, []string{"."})
+       t.CheckDeepEquals(G.Todo.entries, []string{"."})
 
        CheckdirCategory(".")
 
        t.CheckOutputEmpty()
-       t.CheckDeepEquals(G.Todo, []string{"./package", "."})
+       t.CheckDeepEquals(G.Todo.entries, []string{"./package", "."})
 }
 
 // Ensures that a directory in the file system can be added at the very

Index: pkgsrc/pkgtools/pkglint/files/check_test.go
diff -u pkgsrc/pkgtools/pkglint/files/check_test.go:1.51 pkgsrc/pkgtools/pkglint/files/check_test.go:1.52
--- pkgsrc/pkgtools/pkglint/files/check_test.go:1.51    Sat Oct 26 11:43:36 2019
+++ pkgsrc/pkgtools/pkglint/files/check_test.go Fri Nov  1 19:56:53 2019
@@ -69,6 +69,7 @@ func (s *Suite) SetUpTest(c *check.C) {
 
        t.c = c
        t.SetUpCommandLine("-Wall")    // To catch duplicate warnings
+       G.Todo.Pop()                   // The "." was inserted by default.
        t.seenSetUpCommandLine = false // This default call doesn't count.
 
        // To improve code coverage and ensure that trace.Result works
@@ -693,6 +694,8 @@ func (t *Tester) FinishSetUp() {
 //
 // Arguments that name existing files or directories in the temporary test
 // directory are transformed to their actual paths.
+//
+// Does not work in combination with SetUpOption.
 func (t *Tester) Main(args ...string) int {
        if t.seenFinish && !t.seenMain {
                t.Errorf("Calling t.FinishSetup() before t.Main() is redundant " +

Index: pkgsrc/pkgtools/pkglint/files/mklinechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.49 pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.50
--- pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.49 Sat Oct 26 11:43:36 2019
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker.go      Fri Nov  1 19:56:53 2019
@@ -437,16 +437,9 @@ func (ck MkLineChecker) checkVarassignLe
                return
        }
 
-       needsRationale := func(mkline *MkLine) bool {
-               if !mkline.IsVarassignMaybeCommented() {
-                       return false
-               }
-               vartype := G.Pkgsrc.VariableType(ck.MkLines, mkline.Varname())
-               return vartype != nil && vartype.NeedsRationale()
-       }
-
        mkline := ck.MkLine
-       if !needsRationale(mkline) {
+       vartype := G.Pkgsrc.VariableType(ck.MkLines, mkline.Varname())
+       if vartype == nil || !vartype.NeedsRationale() {
                return
        }
 

Index: pkgsrc/pkgtools/pkglint/files/mktypes.go
diff -u pkgsrc/pkgtools/pkglint/files/mktypes.go:1.18 pkgsrc/pkgtools/pkglint/files/mktypes.go:1.19
--- pkgsrc/pkgtools/pkglint/files/mktypes.go:1.18       Thu Sep 12 21:15:48 2019
+++ pkgsrc/pkgtools/pkglint/files/mktypes.go    Fri Nov  1 19:56:53 2019
@@ -89,13 +89,15 @@ func (m MkVarUseModifier) Subst(str stri
 
 // MatchMatch tries to match the modifier to a :M or a :N pattern matching.
 // Examples:
-//  :Mpattern   => true, true, "pattern"
-//  :Npattern   => true, false, "pattern"
+//  :Mpattern   => true,  true,  "pattern", true
+//  :M*         => true,  true,  "*",       false
+//  :M${VAR}    => true,  true,  "${VAR}",  false
+//  :Npattern   => true,  false, "pattern", true
 //  :X          => false
 func (m MkVarUseModifier) MatchMatch() (ok bool, positive bool, pattern string, exact bool) {
        if hasPrefix(m.Text, "M") || hasPrefix(m.Text, "N") {
                // See devel/bmake/files/str.c:^Str_Match
-               exact := !strings.ContainsAny(m.Text[1:], "*?[\\")
+               exact := !strings.ContainsAny(m.Text[1:], "*?[\\$")
                return true, m.Text[0] == 'M', m.Text[1:], exact
        }
        return false, false, "", false

Index: pkgsrc/pkgtools/pkglint/files/mktypes_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mktypes_test.go:1.15 pkgsrc/pkgtools/pkglint/files/mktypes_test.go:1.16
--- pkgsrc/pkgtools/pkglint/files/mktypes_test.go:1.15  Sat Oct 26 09:51:48 2019
+++ pkgsrc/pkgtools/pkglint/files/mktypes_test.go       Fri Nov  1 19:56:53 2019
@@ -39,6 +39,16 @@ func (MkTokenBuilder) VarUse(varname str
        return &MkVarUse{varname, mods}
 }
 
+// AddCommand adds a command directly to a list of commands,
+// creating all the intermediate nodes for the syntactic representation.
+// As soon as that representation is replaced with a semantic representation,
+// this method should no longer be necessary.
+func (list *MkShList) AddCommand(command *MkShCommand) *MkShList {
+       pipeline := NewMkShPipeline(false, []*MkShCommand{command})
+       andOr := NewMkShAndOr(pipeline)
+       return list.AddAndOr(andOr)
+}
+
 func (s *Suite) Test_MkVarUse_Mod(c *check.C) {
        t := s.Init(c)
 
@@ -53,14 +63,29 @@ func (s *Suite) Test_MkVarUse_Mod(c *che
        test("${PATH:ts::Q}", ":ts::Q")
 }
 
-// AddCommand adds a command directly to a list of commands,
-// creating all the intermediate nodes for the syntactic representation.
-// As soon as that representation is replaced with a semantic representation,
-// this method should no longer be necessary.
-func (list *MkShList) AddCommand(command *MkShCommand) *MkShList {
-       pipeline := NewMkShPipeline(false, []*MkShCommand{command})
-       andOr := NewMkShAndOr(pipeline)
-       return list.AddAndOr(andOr)
+func (s *Suite) Test_MkVarUseModifier_MatchMatch(c *check.C) {
+       t := s.Init(c)
+
+       testFail := func(modifier string) {
+               mod := MkVarUseModifier{modifier}
+               ok, _, _, _ := mod.MatchMatch()
+               t.CheckEquals(ok, false)
+       }
+       test := func(modifier string, positive bool, pattern string, exact bool) {
+               mod := MkVarUseModifier{modifier}
+               actualOk, actualPositive, actualPattern, actualExact := mod.MatchMatch()
+               t.CheckDeepEquals(
+                       []interface{}{actualOk, actualPositive, actualPattern, actualExact},
+                       []interface{}{true, positive, pattern, exact})
+       }
+
+       testFail("")
+       testFail("X")
+
+       test("Mpattern", true, "pattern", true)
+       test("M*", true, "*", false)
+       test("M${VAR}", true, "${VAR}", false)
+       test("Npattern", false, "pattern", true)
 }
 
 func (s *Suite) Test_MkVarUseModifier_ChangesWords(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/options_test.go
diff -u pkgsrc/pkgtools/pkglint/files/options_test.go:1.15 pkgsrc/pkgtools/pkglint/files/options_test.go:1.16
--- pkgsrc/pkgtools/pkglint/files/options_test.go:1.15  Wed Aug 21 16:45:17 2019
+++ pkgsrc/pkgtools/pkglint/files/options_test.go       Fri Nov  1 19:56:53 2019
@@ -387,3 +387,101 @@ func (s *Suite) Test_CheckLinesOptionsMk
        t.CheckOutputLines(
                "WARN: options.mk:4: Option \"opt-variant\" should be handled below in an .if block.")
 }
+
+func (s *Suite) Test_CheckLinesOptionsMk__options_in_for_loop(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpOption("idea", "")
+       t.SetUpOption("md2", "")
+       t.SetUpOption("md5", "")
+       t.SetUpOption("other", "")
+       t.CreateFileLines("mk/bsd.options.mk")
+       t.SetUpPackage("category/package",
+               ".include \"options.mk\"")
+       t.CreateFileLines("category/package/options.mk",
+               MkCvsID,
+               "",
+               "PKG_OPTIONS_VAR=\tPKG_OPTIONS.package",
+               "PKG_SUPPORTED_OPTIONS=\tidea md2 md5 other",
+               "",
+               ".include \"../../mk/bsd.options.mk\"",
+               "",
+               ".for alg in idea md2 md5",
+               ".  if ${PKG_OPTIONS:M${alg}}",
+               ".  endif",
+               ".endfor")
+       t.FinishSetUp()
+       t.Chdir("category/package")
+
+       G.Check(".")
+
+       t.CheckOutputLines(
+               "WARN: options.mk:4: Option \"other\" should be handled below in an .if block.")
+}
+
+func (s *Suite) Test_CheckLinesOptionsMk__indirect(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpOption("generic", "")
+       t.SetUpOption("netbsd", "")
+       t.SetUpOption("os", "")
+       t.CreateFileLines("mk/bsd.options.mk")
+       t.SetUpPackage("category/package",
+               ".include \"options.mk\"")
+       t.CreateFileLines("category/package/options.mk",
+               MkCvsID,
+               "",
+               "PKG_OPTIONS_VAR=\tPKG_OPTIONS.package",
+               "PKG_SUPPORTED_OPTIONS=\tgeneric",
+               "PKG_SUGGESTED_OPTIONS=\tgeneric",
+               "",
+               "PKG_SUPPORTED_OPTIONS.FreeBSD=\tos",
+               "PKG_SUGGESTED_OPTIONS.FreeBSD=\tos",
+               "",
+               "PKG_SUPPORTED_OPTIONS.NetBSD+=\tnetbsd os",
+               "PKG_SUGGESTED_OPTIONS.NetBSD+=\tnetbsd os",
+               "",
+               ".include \"../../mk/bsd.options.mk\"",
+               "",
+               "PLIST_VARS+=\tgeneric netbsd os",
+               "",
+               ".for option in ${PLIST_VARS}",
+               ".  if ${PKG_OPTIONS:M${option}}",
+               "CONFIGURE_ARGS+=\t--enable-${option:S/-/_/}",
+               "PLIST.${option}=\tyes",
+               ".  endif",
+               ".endfor")
+       t.FinishSetUp()
+       t.Chdir("category/package")
+
+       G.Check(".")
+
+       t.CheckOutputEmpty()
+}
+
+// An unrealistic scenario, but necessary for code coverage.
+func (s *Suite) Test_CheckLinesOptionsMk__partly_indirect(c *check.C) {
+       t := s.Init(c)
+
+       t.CreateFileLines("mk/bsd.options.mk")
+       t.SetUpPackage("category/package",
+               ".include \"options.mk\"")
+       t.CreateFileLines("category/package/options.mk",
+               MkCvsID,
+               "",
+               "PKG_OPTIONS_VAR=\tPKG_OPTIONS.package",
+               "PKG_SUPPORTED_OPTIONS=\tgeneric-${OPSYS}",
+               "",
+               ".include \"../../mk/bsd.options.mk\"",
+               "",
+               ".for option in generic-${OPSYS}",
+               ".  if ${PKG_OPTIONS:M${option}}",
+               ".  endif",
+               ".endfor")
+       t.FinishSetUp()
+       t.Chdir("category/package")
+
+       G.Check(".")
+
+       t.CheckOutputEmpty()
+}

Index: pkgsrc/pkgtools/pkglint/files/options.go
diff -u pkgsrc/pkgtools/pkglint/files/options.go:1.17 pkgsrc/pkgtools/pkglint/files/options.go:1.18
--- pkgsrc/pkgtools/pkglint/files/options.go:1.17       Wed Aug 21 16:45:17 2019
+++ pkgsrc/pkgtools/pkglint/files/options.go    Fri Nov  1 19:56:53 2019
@@ -1,7 +1,5 @@
 package pkglint
 
-import "path"
-
 func CheckLinesOptionsMk(mklines *MkLines) {
        ck := OptionsLinesChecker{
                mklines,
@@ -42,10 +40,13 @@ func (ck *OptionsLinesChecker) Check() {
                mlex.Skip()
        }
 
-       for !mlex.EOF() {
-               ck.handleLowerLine(mlex.CurrentMkLine())
-               mlex.Skip()
-       }
+       i := 0
+       mklines.ForEach(func(mkline *MkLine) {
+               if i >= mlex.index {
+                       ck.handleLowerLine(mkline)
+               }
+               i++
+       })
 
        ck.checkOptionsMismatch()
 
@@ -81,7 +82,10 @@ func (ck *OptionsLinesChecker) handleUpp
 
        case mkline.IsVarassign():
                switch mkline.Varcanon() {
-               case "PKG_SUPPORTED_OPTIONS", "PKG_OPTIONS_GROUP.*", "PKG_OPTIONS_SET.*":
+               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
@@ -113,48 +117,64 @@ func (ck *OptionsLinesChecker) handleUpp
 }
 
 func (ck *OptionsLinesChecker) handleLowerLine(mkline *MkLine) {
-       if mkline.IsDirective() {
-               directive := mkline.Directive()
-               if directive == "if" || directive == "elif" {
-                       cond := mkline.Cond()
-                       if cond != nil {
-                               ck.handleLowerCondition(mkline, cond)
-                       }
-               }
+       if !mkline.IsDirective() {
+               return
+       }
+
+       directive := mkline.Directive()
+       if directive != "if" && directive != "elif" {
+               return
+       }
+
+       cond := mkline.Cond()
+       if cond == nil {
+               return
        }
+
+       ck.handleLowerCondition(mkline, cond)
 }
 
 func (ck *OptionsLinesChecker) handleLowerCondition(mkline *MkLine, cond *MkCond) {
 
-       recordUsedOption := func(varuse *MkVarUse) {
-               if varuse.varname != "PKG_OPTIONS" || len(varuse.modifiers) != 1 {
+       recordOption := func(option string) {
+               if containsVarRef(option) {
                        return
                }
 
-               m, positive, pattern, exact := varuse.modifiers[0].MatchMatch()
-               if !m || !positive || containsVarRef(pattern) {
+               ck.handledOptions[option] = mkline
+               ck.optionsInDeclarationOrder = append(ck.optionsInDeclarationOrder, option)
+       }
+
+       recordVarUse := func(varuse *MkVarUse) {
+               if varuse.varname != "PKG_OPTIONS" || len(varuse.modifiers) != 1 {
                        return
                }
 
-               if exact {
-                       option := pattern
-                       ck.handledOptions[option] = mkline
-                       ck.optionsInDeclarationOrder = append(ck.optionsInDeclarationOrder, option)
+               m, positive, pattern, exact := varuse.modifiers[0].MatchMatch()
+               if !m || !positive {
                        return
                }
 
-               for declaredOption := range ck.declaredOptions {
-                       matched, err := path.Match(pattern, declaredOption)
-                       if err == nil && matched {
-                               ck.handledOptions[declaredOption] = mkline
-                               ck.optionsInDeclarationOrder = append(ck.optionsInDeclarationOrder, declaredOption)
+               if optionVarUse := ToVarUse(pattern); optionVarUse != nil {
+                       for _, option := range ck.mklines.ExpandLoopVar(optionVarUse.varname) {
+                               recordOption(option)
+                       }
+
+               } else if exact {
+                       recordOption(pattern)
+
+               } else {
+                       for declaredOption := range ck.declaredOptions {
+                               if pathMatches(pattern, declaredOption) {
+                                       recordOption(declaredOption)
+                               }
                        }
                }
        }
 
        cond.Walk(&MkCondCallback{
-               Empty: recordUsedOption,
-               Var:   recordUsedOption})
+               Empty: recordVarUse,
+               Var:   recordVarUse})
 
        if cond.Empty != nil && cond.Empty.varname == "PKG_OPTIONS" && mkline.HasElseBranch() {
                mkline.Notef("The positive branch of the .if/.else should be the one where the option is set.")

Index: pkgsrc/pkgtools/pkglint/files/package.go
diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.65 pkgsrc/pkgtools/pkglint/files/package.go:1.66
--- pkgsrc/pkgtools/pkglint/files/package.go:1.65       Sat Oct 26 09:51:48 2019
+++ pkgsrc/pkgtools/pkglint/files/package.go    Fri Nov  1 19:56:53 2019
@@ -1272,6 +1272,18 @@ func (pkg *Package) checkIncludeConditio
 
        key := pkg.Rel(mkline.IncludedFileFull())
 
+       explainPkgOptions := func(uncond *MkLine, cond *MkLine) {
+               if uncond.Basename == "buildlink3.mk" && containsStr(cond.ConditionalVars(), "PKG_OPTIONS") {
+                       mkline.Explain(
+                               "When including a dependent file, the conditions in the",
+                               "buildlink3.mk file should be the same as in options.mk",
+                               "or the Makefile.",
+                               "",
+                               "To find out the PKG_OPTIONS of this package at build time,",
+                               "have a look at mk/pkg-build-options.mk.")
+               }
+       }
+
        if indentation.IsConditional() {
                if other := pkg.unconditionalIncludes[key]; other != nil {
                        if !pkg.Once.FirstTimeSlice("checkIncludeConditionally", mkline.Location.String(), other.Location.String()) {
@@ -1282,6 +1294,8 @@ func (pkg *Package) checkIncludeConditio
                                "%q is included conditionally here (depending on %s) "+
                                        "and unconditionally in %s.",
                                cleanpath(mkline.IncludedFile()), strings.Join(mkline.ConditionalVars(), ", "), mkline.RefTo(other))
+
+                       explainPkgOptions(other, mkline)
                }
 
        } else {
@@ -1295,15 +1309,7 @@ func (pkg *Package) checkIncludeConditio
                                        "and conditionally in %s (depending on %s).",
                                cleanpath(mkline.IncludedFile()), mkline.RefTo(other), strings.Join(other.ConditionalVars(), ", "))
 
-                       if mkline.Basename == "buildlink3.mk" && containsStr(other.ConditionalVars(), "PKG_OPTIONS") {
-                               mkline.Explain(
-                                       "When including a dependent file, the conditions in the",
-                                       "buildlink3.mk file should be the same as in options.mk",
-                                       "or the Makefile.",
-                                       "",
-                                       "To find out the PKG_OPTIONS of this package at build time,",
-                                       "have a look at mk/pkg-build-options.mk.")
-                       }
+                       explainPkgOptions(mkline, other)
                }
        }
 

Index: pkgsrc/pkgtools/pkglint/files/package_test.go
diff -u pkgsrc/pkgtools/pkglint/files/package_test.go:1.55 pkgsrc/pkgtools/pkglint/files/package_test.go:1.56
--- pkgsrc/pkgtools/pkglint/files/package_test.go:1.55  Sat Oct 26 11:43:36 2019
+++ pkgsrc/pkgtools/pkglint/files/package_test.go       Fri Nov  1 19:56:53 2019
@@ -1288,6 +1288,111 @@ func (s *Suite) Test_Package_checkInclud
                "WARN: options.mk:3: Expected definition of PKG_OPTIONS_VAR.")
 }
 
+func (s *Suite) Test_Package_checkIncludeConditionally__explain_PKG_OPTIONS_in_Makefile(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpCommandLine("-Wall", "--explain")
+       t.SetUpOption("zlib", "use zlib compression")
+
+       t.CreateFileLines("mk/bsd.options.mk",
+               MkCvsID)
+       t.CreateFileLines("devel/zlib/buildlink3.mk",
+               MkCvsID)
+       t.SetUpPackage("category/package",
+               "PKG_OPTIONS_VAR=\tPKG_OPTIONS.package",
+               "PKG_SUPPORTED_OPTIONS=\tzlib",
+               "",
+               ".include \"../../mk/bsd.options.mk\"",
+               "",
+               ".if ${PKG_OPTIONS:Mzlib}",
+               ".include \"../../devel/zlib/buildlink3.mk\"",
+               ".endif")
+       t.CreateFileDummyBuildlink3("category/package/buildlink3.mk",
+               ".include \"../../devel/zlib/buildlink3.mk\"")
+       t.Chdir("category/package")
+       t.FinishSetUp()
+
+       G.checkdirPackage(".")
+
+       t.CheckOutputLines(
+               "WARN: Makefile:26: "+
+                       "\"../../devel/zlib/buildlink3.mk\" is included conditionally here "+
+                       "(depending on PKG_OPTIONS) and unconditionally in buildlink3.mk:12.",
+               "",
+               "\tWhen including a dependent file, the conditions in the buildlink3.mk",
+               "\tfile should be the same as in options.mk or the Makefile.",
+               "",
+               "\tTo find out the PKG_OPTIONS of this package at build time, have a",
+               "\tlook at mk/pkg-build-options.mk.",
+               "")
+}
+
+func (s *Suite) Test_Package_checkIncludeConditionally__no_explanation(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpCommandLine("-Wall", "--explain")
+       t.CreateFileLines("devel/zlib/buildlink3.mk",
+               MkCvsID)
+       t.SetUpPackage("category/package",
+               ".if ${OPSYS} == Linux",
+               ".include \"../../devel/zlib/buildlink3.mk\"",
+               ".endif")
+       t.CreateFileDummyBuildlink3("category/package/buildlink3.mk",
+               ".include \"../../devel/zlib/buildlink3.mk\"")
+       t.Chdir("category/package")
+       t.FinishSetUp()
+
+       G.checkdirPackage(".")
+
+       t.CheckOutputLines(
+               "WARN: Makefile:21: " +
+                       "\"../../devel/zlib/buildlink3.mk\" is included conditionally here " +
+                       "(depending on OPSYS) and unconditionally in buildlink3.mk:12.")
+}
+
+func (s *Suite) Test_Package_checkIncludeConditionally__explain_PKG_OPTIONS_in_options_mk(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpCommandLine("-Wall", "--explain")
+       t.SetUpOption("zlib", "use zlib compression")
+
+       t.CreateFileLines("mk/bsd.options.mk",
+               MkCvsID)
+       t.CreateFileLines("devel/zlib/buildlink3.mk",
+               MkCvsID)
+       t.SetUpPackage("category/package",
+               ".include \"options.mk\"")
+       t.CreateFileLines("category/package/options.mk",
+               MkCvsID,
+               "",
+               "PKG_OPTIONS_VAR=\tPKG_OPTIONS.package",
+               "PKG_SUPPORTED_OPTIONS=\tzlib",
+               "",
+               ".include \"../../mk/bsd.options.mk\"",
+               "",
+               ".if ${PKG_OPTIONS:Mzlib}",
+               ".include \"../../devel/zlib/buildlink3.mk\"",
+               ".endif")
+       t.CreateFileDummyBuildlink3("category/package/buildlink3.mk",
+               ".include \"../../devel/zlib/buildlink3.mk\"")
+       t.Chdir("category/package")
+       t.FinishSetUp()
+
+       G.checkdirPackage(".")
+
+       t.CheckOutputLines(
+               "WARN: buildlink3.mk:12: "+
+                       "\"../../devel/zlib/buildlink3.mk\" is included unconditionally here "+
+                       "and conditionally in options.mk:9 (depending on PKG_OPTIONS).",
+               "",
+               "\tWhen including a dependent file, the conditions in the buildlink3.mk",
+               "\tfile should be the same as in options.mk or the Makefile.",
+               "",
+               "\tTo find out the PKG_OPTIONS of this package at build time, have a",
+               "\tlook at mk/pkg-build-options.mk.",
+               "")
+}
+
 func (s *Suite) Test_Package_checkIncludeConditionally__unconditionally_first(c *check.C) {
        t := s.Init(c)
 
Index: pkgsrc/pkgtools/pkglint/files/util.go
diff -u pkgsrc/pkgtools/pkglint/files/util.go:1.55 pkgsrc/pkgtools/pkglint/files/util.go:1.56
--- pkgsrc/pkgtools/pkglint/files/util.go:1.55  Sat Oct 26 11:43:36 2019
+++ pkgsrc/pkgtools/pkglint/files/util.go       Fri Nov  1 19:56:53 2019
@@ -1384,3 +1384,34 @@ func shquote(s string) string {
        }
        return "'" + strings.Replace(s, "'", "'\\''", -1) + "'"
 }
+
+func pathMatches(pattern, s string) bool {
+       matched, err := path.Match(pattern, s)
+       return err == nil && matched
+}
+
+type StringQueue struct {
+       entries []string
+}
+
+func (q *StringQueue) PushFront(entries ...string) {
+       q.entries = append(append([]string(nil), entries...), q.entries...)
+}
+
+func (q *StringQueue) Push(entries ...string) {
+       q.entries = append(q.entries, entries...)
+}
+
+func (q *StringQueue) Empty() bool {
+       return len(q.entries) == 0
+}
+
+func (q *StringQueue) Front() string {
+       return q.entries[0]
+}
+
+func (q *StringQueue) Pop() string {
+       front := q.entries[0]
+       q.entries = q.entries[1:]
+       return front
+}

Index: pkgsrc/pkgtools/pkglint/files/pkglint.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint.go:1.61 pkgsrc/pkgtools/pkglint/files/pkglint.go:1.62
--- pkgsrc/pkgtools/pkglint/files/pkglint.go:1.61       Tue Oct  1 21:37:59 2019
+++ pkgsrc/pkgtools/pkglint/files/pkglint.go    Fri Nov  1 19:56:53 2019
@@ -26,12 +26,12 @@ type Pkglint struct {
        Pkgsrc Pkgsrc   // Global data, mostly extracted from mk/*.
        Pkg    *Package // The package that is currently checked, or nil.
 
-       Todo           []string // The files or directories that still need to be checked.
-       Wip            bool     // Is the currently checked file or package from pkgsrc-wip?
-       Infrastructure bool     // Is the currently checked file from the pkgsrc infrastructure?
-       Testing        bool     // Is pkglint in self-testing mode (only during development)?
-       Experimental   bool     // For experimental features, only enabled individually in tests
-       Username       string   // For checking against OWNER and MAINTAINER
+       Todo           StringQueue // The files or directories that still need to be checked.
+       Wip            bool        // Is the currently checked file or package from pkgsrc-wip?
+       Infrastructure bool        // Is the currently checked file from the pkgsrc infrastructure?
+       Testing        bool        // Is pkglint in self-testing mode (only during development)?
+       Experimental   bool        // For experimental features, only enabled individually in tests
+       Username       string      // For checking against OWNER and MAINTAINER
 
        cvsEntriesDir string // Cached to avoid I/O
        cvsEntries    map[string]CvsEntry
@@ -190,10 +190,8 @@ func (pkglint *Pkglint) Main(stdout io.W
 
        pkglint.prepareMainLoop()
 
-       for len(pkglint.Todo) > 0 {
-               item := pkglint.Todo[0]
-               pkglint.Todo = pkglint.Todo[1:]
-               pkglint.Check(item)
+       for !pkglint.Todo.Empty() {
+               pkglint.Check(pkglint.Todo.Pop())
        }
 
        pkglint.Pkgsrc.checkToplevelUnusedLicenses()
@@ -258,7 +256,7 @@ func (pkglint *Pkglint) setUpProfiling()
 }
 
 func (pkglint *Pkglint) prepareMainLoop() {
-       firstDir := pkglint.Todo[0]
+       firstDir := pkglint.Todo.Front()
        if fileExists(firstDir) {
                firstDir = path.Dir(firstDir)
        }
@@ -333,12 +331,11 @@ func (pkglint *Pkglint) ParseCommandLine
                return 0
        }
 
-       pkglint.Todo = nil
        for _, arg := range pkglint.Opts.args {
-               pkglint.Todo = append(pkglint.Todo, filepath.ToSlash(arg))
+               pkglint.Todo.Push(filepath.ToSlash(arg))
        }
-       if len(pkglint.Todo) == 0 {
-               pkglint.Todo = []string{"."}
+       if pkglint.Todo.Empty() {
+               pkglint.Todo.Push(".")
        }
 
        return -1

Index: pkgsrc/pkgtools/pkglint/files/toplevel.go
diff -u pkgsrc/pkgtools/pkglint/files/toplevel.go:1.22 pkgsrc/pkgtools/pkglint/files/toplevel.go:1.23
--- pkgsrc/pkgtools/pkglint/files/toplevel.go:1.22      Sat Oct 26 09:51:48 2019
+++ pkgsrc/pkgtools/pkglint/files/toplevel.go   Fri Nov  1 19:56:53 2019
@@ -31,7 +31,7 @@ func CheckdirToplevel(dir string) {
                if G.Opts.CheckGlobal {
                        G.InterPackage.Enable()
                }
-               G.Todo = append(append([]string(nil), ctx.subdirs...), G.Todo...)
+               G.Todo.PushFront(ctx.subdirs...)
        }
 }
 

Index: pkgsrc/pkgtools/pkglint/files/vardefs.go
diff -u pkgsrc/pkgtools/pkglint/files/vardefs.go:1.73 pkgsrc/pkgtools/pkglint/files/vardefs.go:1.74
--- pkgsrc/pkgtools/pkglint/files/vardefs.go:1.73       Tue Oct  1 21:37:59 2019
+++ pkgsrc/pkgtools/pkglint/files/vardefs.go    Fri Nov  1 19:56:53 2019
@@ -156,6 +156,17 @@ func (reg *VarTypeRegistry) pkglistrat(v
                "Makefile, Makefile.*, *.mk: default, set, append, use")
 }
 
+// A package-defined load-time list may be used or defined or appended to in
+// all Makefiles except buildlink3.mk and builtin.mk. Simple assignment
+// (instead of appending) is also allowed. If this leads to an unconditional
+// assignment overriding a previous value, the redundancy check will catch it.
+func (reg *VarTypeRegistry) pkgloadlist(varname string, basicType *BasicType) {
+       reg.acllist(varname, basicType,
+               List|PackageSettable,
+               "buildlink3.mk, builtin.mk: none",
+               "Makefile, Makefile.*, *.mk: default, set, append, use, use-loadtime")
+}
+
 // pkgappend declares a variable that may use the += operator,
 // even though it is not a list where each item can be interpreted
 // on its own.
@@ -328,7 +339,7 @@ func (reg *VarTypeRegistry) compilerLang
                                        VarUse: func(varuse *MkVarUse) {
                                                if varuse.varname == "USE_LANGUAGES" && len(varuse.modifiers) == 1 {
                                                        ok, _, pattern, exact := varuse.modifiers[0].MatchMatch()
-                                                       if ok && exact && !containsVarRef(pattern) {
+                                                       if ok && exact {
                                                                languages[intern(pattern)] = true
                                                        }
                                                }
@@ -1409,28 +1420,27 @@ func (reg *VarTypeRegistry) Init(src *Pk
        //  define them in the Makefile or Makefile.common.
        reg.sysloadlist("PKG_OPTIONS", BtOption)
        reg.usrlist("PKG_OPTIONS.*", BtOption)
-       opt := reg.pkg
-       optlist := reg.pkglist
-       optlist("PKG_LEGACY_OPTIONS", BtOption)
-       optlist("PKG_OPTIONS_DEPRECATED_WARNINGS", BtShellWord)
-       optlist("PKG_OPTIONS_GROUP.*", BtOption)
-       optlist("PKG_OPTIONS_LEGACY_OPTS", BtUnknown)
-       optlist("PKG_OPTIONS_LEGACY_VARS", BtUnknown)
-       optlist("PKG_OPTIONS_NONEMPTY_SETS", BtIdentifier)
-       optlist("PKG_OPTIONS_OPTIONAL_GROUPS", BtIdentifier)
-       optlist("PKG_OPTIONS_REQUIRED_GROUPS", BtIdentifier)
-       optlist("PKG_OPTIONS_SET.*", BtOption)
-       opt("PKG_OPTIONS_VAR", BtPkgOptionsVar)
-       reg.pkglist("PKG_SKIP_REASON", BtShellWord)
-       optlist("PKG_SUGGESTED_OPTIONS", BtOption)
-       optlist("PKG_SUGGESTED_OPTIONS.*", BtOption)
-       optlist("PKG_SUPPORTED_OPTIONS", BtOption)
+       reg.pkgloadlist("PKG_LEGACY_OPTIONS", BtOption)
+       reg.pkgloadlist("PKG_OPTIONS_DEPRECATED_WARNINGS", BtShellWord)
+       reg.pkgloadlist("PKG_OPTIONS_GROUP.*", BtOption)
+       reg.pkgloadlist("PKG_OPTIONS_LEGACY_OPTS", BtUnknown)
+       reg.pkgloadlist("PKG_OPTIONS_LEGACY_VARS", BtUnknown)
+       reg.pkgloadlist("PKG_OPTIONS_NONEMPTY_SETS", BtIdentifier)
+       reg.pkgloadlist("PKG_OPTIONS_OPTIONAL_GROUPS", BtIdentifier)
+       reg.pkgloadlist("PKG_OPTIONS_REQUIRED_GROUPS", BtIdentifier)
+       reg.pkgloadlist("PKG_OPTIONS_SET.*", BtOption)
+       reg.pkgload("PKG_OPTIONS_VAR", BtPkgOptionsVar)
+       reg.pkgloadlist("PKG_SUGGESTED_OPTIONS", BtOption)
+       reg.pkgloadlist("PKG_SUGGESTED_OPTIONS.*", BtOption)
+       reg.pkgloadlist("PKG_SUPPORTED_OPTIONS", BtOption)
+       reg.pkgloadlist("PKG_SUPPORTED_OPTIONS.*", BtOption)
        // end PKG_OPTIONS section
 
        reg.pkg("PKG_PRESERVE", BtYes)
        reg.pkg("PKG_SHELL", BtPathname)
        reg.pkg("PKG_SHELL.*", BtPathname)
        reg.sys("PKG_SHLIBTOOL", BtPathname)
+       reg.pkglist("PKG_SKIP_REASON", BtShellWord)
        // The special exception for buildlink3.mk is only here because
        // of textproc/xmlcatmgr.
        reg.acl("PKG_SYSCONFDIR*", BtPathname,
@@ -1447,7 +1457,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.pkglist("PKG_USERS_VARS", BtVariableName)
        reg.pkg("PKG_USE_KERBEROS", BtYes)
        reg.pkgload("PLIST.*", BtYes)
-       reg.pkglist("PLIST_VARS", BtIdentifier)
+       reg.pkgloadlist("PLIST_VARS", BtIdentifier)
        reg.pkglist("PLIST_SRC", BtRelativePkgPath)
        reg.pkglist("PLIST_SUBST", BtShellWord)
        reg.pkg("PLIST_TYPE", enum("dynamic static"))



Home | Main Index | Thread Index | Old Index