pkgsrc-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[pkgsrc/trunk]: pkgsrc/pkgtools/pkglint pkgtools/pkglint: update to 20.1.2



details:   https://anonhg.NetBSD.org/pkgsrc/rev/4b4b2d6ade75
branches:  trunk
changeset: 427231:4b4b2d6ade75
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Mon Apr 13 19:46:44 2020 +0000

description:
pkgtools/pkglint: update to 20.1.2

Changes since 20.1.1:

Ensure that relative paths to other packages have the canonical form
../../category/package.

Add notes about pathname patters that mention ${WRKSRC} even though they
are already defined to be relative to WRKSRC.

Fix check for redundantly added categories. The check had previously
reported that the included file would be redundant, which was wrong.
It's always the including file that provides the redundancy.

diffstat:

 pkgtools/pkglint/Makefile                     |    5 +-
 pkgtools/pkglint/files/mklinechecker.go       |   15 +++
 pkgtools/pkglint/files/package.go             |   33 -------
 pkgtools/pkglint/files/package_test.go        |   68 ----------------
 pkgtools/pkglint/files/redundantscope.go      |   45 ++++++++++
 pkgtools/pkglint/files/redundantscope_test.go |  110 ++++++++++++++++++++++++++
 pkgtools/pkglint/files/vardefs.go             |   42 ++++-----
 pkgtools/pkglint/files/vartype.go             |    8 +
 pkgtools/pkglint/files/vartypecheck.go        |   84 ++++++++++++-------
 pkgtools/pkglint/files/vartypecheck_test.go   |   74 +++++++++++++++++-
 10 files changed, 323 insertions(+), 161 deletions(-)

diffs (truncated from 696 to 300 lines):

diff -r 0155f4521070 -r 4b4b2d6ade75 pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Mon Apr 13 19:19:37 2020 +0000
+++ b/pkgtools/pkglint/Makefile Mon Apr 13 19:46:44 2020 +0000
@@ -1,7 +1,6 @@
-# $NetBSD: Makefile,v 1.640 2020/04/12 11:01:44 bsiegert Exp $
+# $NetBSD: Makefile,v 1.641 2020/04/13 19:46:44 rillig Exp $
 
-PKGNAME=       pkglint-20.1.1
-PKGREVISION=   1
+PKGNAME=       pkglint-20.1.2
 CATEGORIES=    pkgtools
 DISTNAME=      tools
 MASTER_SITES=  ${MASTER_SITE_GITHUB:=golang/}
diff -r 0155f4521070 -r 4b4b2d6ade75 pkgtools/pkglint/files/mklinechecker.go
--- a/pkgtools/pkglint/files/mklinechecker.go   Mon Apr 13 19:19:37 2020 +0000
+++ b/pkgtools/pkglint/files/mklinechecker.go   Mon Apr 13 19:46:44 2020 +0000
@@ -382,6 +382,21 @@
        mkline := ck.MkLine
        makefile := pkgdir.JoinNoClean("Makefile")
        ck.CheckRelativePath(makefile, makefile.AsRelPath(), true)
+
+       if hasSuffix(pkgdir.String(), "/") {
+               mkline.Errorf("Relative package directories like %q must not end with a slash.", pkgdir.String())
+               mkline.Explain(
+                       "This causes problems with bulk builds, at least with limited builds,",
+                       "as the trailing slash in a package directory name causes pbulk-scan",
+                       "to fail with \"Invalid path from master\" and leads to a hung scan phase.")
+       } else if pkgdir.AsPath() != pkgdir.AsPath().Clean() {
+               mkline.Errorf("Relative package directories like %q must be canonical.",
+                       pkgdir.String())
+               mkline.Explain(
+                       "The canonical form of a package path is \"../../category/package\".")
+       }
+
+       // This strips any trailing slash.
        pkgdir = mkline.ResolveVarsInRelativePath(pkgdir, ck.MkLines.pkg)
 
        if !matches(pkgdir.String(), `^\.\./\.\./([^./][^/]*/[^./][^/]*)$`) && !containsVarUse(pkgdir.String()) {
diff -r 0155f4521070 -r 4b4b2d6ade75 pkgtools/pkglint/files/package.go
--- a/pkgtools/pkglint/files/package.go Mon Apr 13 19:19:37 2020 +0000
+++ b/pkgtools/pkglint/files/package.go Mon Apr 13 19:46:44 2020 +0000
@@ -711,7 +711,6 @@
                return G.CheckGlobal || !G.Pkgsrc.IsInfra(mkline.Filename())
        }
        pkg.redundant.Check(allLines) // Updates the variables in the scope
-       pkg.checkCategories()
        pkg.checkGnuConfigureUseLanguages()
        pkg.checkUseLanguagesCompilerMk(allLines)
 
@@ -1047,38 +1046,6 @@
                seeGuide("Package components, Makefile", "components.Makefile"))
 }
 
-func (pkg *Package) checkCategories() {
-       categories := pkg.redundant.vars["CATEGORIES"]
-       if categories == nil || !categories.vari.IsConstant() {
-               return
-       }
-
-       // XXX: Decide what exactly this map means.
-       //  Is it "this category has been seen somewhere",
-       //  or is it "this category has definitely been added"?
-       seen := map[string]*MkLine{}
-       for _, mkline := range categories.vari.WriteLocations() {
-               switch mkline.Op() {
-               case opAssignDefault:
-                       for _, category := range mkline.ValueFields(mkline.Value()) {
-                               if seen[category] == nil {
-                                       seen[category] = mkline
-                               }
-                       }
-               default:
-                       for _, category := range mkline.ValueFields(mkline.Value()) {
-                               if seen[category] != nil {
-                                       mkline.Notef("Category %q is already added in %s.",
-                                               category, mkline.RelMkLine(seen[category]))
-                               }
-                               if seen[category] == nil {
-                                       seen[category] = mkline
-                               }
-                       }
-               }
-       }
-}
-
 func (pkg *Package) checkGnuConfigureUseLanguages() {
        s := pkg.redundant
 
diff -r 0155f4521070 -r 4b4b2d6ade75 pkgtools/pkglint/files/package_test.go
--- a/pkgtools/pkglint/files/package_test.go    Mon Apr 13 19:19:37 2020 +0000
+++ b/pkgtools/pkglint/files/package_test.go    Mon Apr 13 19:46:44 2020 +0000
@@ -2282,74 +2282,6 @@
                        "CATEGORIES, empty line, MAINTAINER, COMMENT, LICENSE, empty line, DEPENDS.")
 }
 
-func (s *Suite) Test_Package_checkCategories__redundant(c *check.C) {
-       t := s.Init(c)
-
-       t.SetUpPackage("category/package",
-               "CATEGORIES=\tcategory perl5",
-               ".include \"included.mk\"")
-       t.CreateFileLines("category/package/included.mk",
-               MkCvsID,
-               "CATEGORIES+=\tperl5 python",
-               "CATEGORIES+=\tpython",
-               "CATEGORIES?=\tcategory japanese")
-       t.Chdir("category/package")
-       t.FinishSetUp()
-
-       G.Check(".")
-
-       t.CheckOutputLines(
-               // TODO: Warn in the including file, not in the included file, just as in RedundantScope.
-               "NOTE: included.mk:2: Category \"perl5\" is already added in Makefile:5.",
-               "NOTE: included.mk:3: Category \"python\" is already added in line 2.")
-}
-
-func (s *Suite) Test_Package_checkCategories__redundant_but_not_constant(c *check.C) {
-       t := s.Init(c)
-
-       t.SetUpPackage("category/package",
-               "CATEGORIES=\tcategory",
-               ".include \"included.mk\"")
-       t.CreateFileLines("category/package/included.mk",
-               MkCvsID,
-               "CATEGORIES+=\tperl5 python",
-               "CATEGORIES+=\tpython",
-               "CATEGORIES?=\tcategory japanese",
-               "",
-               ".if 1",
-               "CATEGORIES+=\tchinese",
-               ".endif")
-       t.Chdir("category/package")
-       t.FinishSetUp()
-
-       G.Check(".")
-
-       // No diagnostics at all, because CATEGORIES is not constant,
-       // as "chinese" may or may not be added.
-       t.CheckOutputEmpty()
-}
-
-// The := assignment operator is equivalent to the simple = operator
-// if its right-hand side does not contain references to any variables.
-func (s *Suite) Test_Package_checkCategories__eval_assignment(c *check.C) {
-       t := s.Init(c)
-
-       t.SetUpPackage("category/package",
-               "CATEGORIES:=\tcategory",
-               ".include \"included.mk\"")
-       t.CreateFileLines("category/package/included.mk",
-               MkCvsID,
-               "CATEGORIES+=\tcategory")
-       t.Chdir("category/package")
-       t.FinishSetUp()
-
-       G.Check(".")
-
-       t.CheckOutputLines(
-               "NOTE: included.mk:2: " +
-                       "Category \"category\" is already added in Makefile:5.")
-}
-
 func (s *Suite) Test_Package_checkGnuConfigureUseLanguages__no_C(c *check.C) {
        t := s.Init(c)
 
diff -r 0155f4521070 -r 4b4b2d6ade75 pkgtools/pkglint/files/redundantscope.go
--- a/pkgtools/pkglint/files/redundantscope.go  Mon Apr 13 19:19:37 2020 +0000
+++ b/pkgtools/pkglint/files/redundantscope.go  Mon Apr 13 19:46:44 2020 +0000
@@ -152,6 +152,51 @@
                                s.onRedundant(prevWrites[len(prevWrites)-1], mkline)
                        }
                }
+
+       case opAssignAppend:
+               s.checkAppendUnique(mkline, info)
+       }
+}
+
+// checkAppendUnique checks whether a redundant value is appended to a
+// variable that doesn't need repeated values, such as CATEGORIES.
+func (s *RedundantScope) checkAppendUnique(mkline *MkLine, info *redundantScopeVarinfo) {
+       if !info.vari.IsConstant() {
+               return
+       }
+
+       vartype := G.Pkgsrc.VariableType(nil, info.vari.Name)
+       if !(vartype != nil && vartype.IsUnique()) {
+               return
+       }
+
+       checkRedundantAppend := func(redundant *MkLine, because *MkLine) {
+               reds := redundant.ValueFieldsLiteral()
+               becs := because.ValueFieldsLiteral()
+               for _, red := range reds {
+                       for _, bec := range becs {
+                               if red != bec {
+                                       continue
+                               }
+                               if redundant == mkline {
+                                       redundant.Notef("Appending %q to %s is redundant because it is already added in %s.",
+                                               red, info.vari.Name, redundant.RelMkLine(because))
+                               } else {
+                                       redundant.Notef("Adding %q to %s is redundant because it will later be appended in %s.",
+                                               red, info.vari.Name, redundant.RelMkLine(because))
+                               }
+                       }
+               }
+       }
+
+       if s.includePath.includesOrEqualsAll(info.includePaths) {
+               for _, prev := range info.vari.WriteLocations() {
+                       checkRedundantAppend(mkline, prev)
+               }
+       } else if s.includePath.includedByOrEqualsAll(info.includePaths) {
+               for _, prev := range info.vari.WriteLocations() {
+                       checkRedundantAppend(prev, mkline)
+               }
        }
 }
 
diff -r 0155f4521070 -r 4b4b2d6ade75 pkgtools/pkglint/files/redundantscope_test.go
--- a/pkgtools/pkglint/files/redundantscope_test.go     Mon Apr 13 19:19:37 2020 +0000
+++ b/pkgtools/pkglint/files/redundantscope_test.go     Mon Apr 13 19:46:44 2020 +0000
@@ -1584,6 +1584,116 @@
                        "Definition of PKG_OPTIONS is redundant because of line 1.")
 }
 
+func (s *Suite) Test_RedundantScope_checkAppendUnique__redundant_before_including(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpPackage("category/package",
+               "CATEGORIES=\tcategory perl5",
+               ".include \"included.mk\"")
+       t.CreateFileLines("category/package/included.mk",
+               MkCvsID,
+               "CATEGORIES+=\tperl5 python",
+               "CATEGORIES+=\tpython",
+               "CATEGORIES?=\tcategory japanese")
+       t.Chdir("category/package")
+       t.FinishSetUp()
+
+       G.Check(".")
+
+       // The second line sounds a bit strange since it references a line
+       // further down in the file. It's correct though.
+       t.CheckOutputLines(
+               "NOTE: Makefile:5: Adding \"perl5\" to CATEGORIES is redundant "+
+                       "because it will later be appended in included.mk:2.",
+               "NOTE: included.mk:2: Adding \"python\" to CATEGORIES is redundant "+
+                       "because it will later be appended in line 3.")
+}
+
+func (s *Suite) Test_RedundantScope_checkAppendUnique__redundant_after_including(c *check.C) {
+       t := s.Init(c)
+
+       // The assignment to CATEGORIES must be commented out in this test.
+       // The redundancy check only works if either _all_ previous variable
+       // assignments happen in included files or if _all_ previous variable
+       // assignments happen in including files.
+       //
+       // See Tester.SetUpPackage for the magic that is involved in defining
+       // a package during testing. That magic is also the reason for having
+       // both included1.mk and included2.mk.
+       t.SetUpPackage("category/package",
+               "#CATEGORIES=\tcategory",
+               ".include \"included1.mk\"")
+       t.CreateFileLines("category/package/included1.mk",
+               MkCvsID,
+               ".include \"included2.mk\"",
+               "CATEGORIES+=\tcategory perl5 python japanese")
+       t.CreateFileLines("category/package/included2.mk",
+               MkCvsID,
+               "CATEGORIES+=\tcategory perl5 japanese chinese")
+       t.Chdir("category/package")
+       t.FinishSetUp()
+
+       G.Check(".")
+
+       t.CheckOutputLines(
+               "NOTE: included1.mk:3: Appending \"category\" to CATEGORIES is redundant "+
+                       "because it is already added in included2.mk:2.",
+               "NOTE: included1.mk:3: Appending \"perl5\" to CATEGORIES is redundant "+
+                       "because it is already added in included2.mk:2.",
+               "NOTE: included1.mk:3: Appending \"japanese\" to CATEGORIES is redundant "+
+                       "because it is already added in included2.mk:2.")
+}
+
+func (s *Suite) Test_RedundantScope_checkAppendUnique__redundant_and_later_conditional(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpPackage("category/package",
+               "CATEGORIES=\tcategory",
+               ".include \"included.mk\"")
+       t.CreateFileLines("category/package/included.mk",
+               MkCvsID,
+               "CATEGORIES+=\tperl5 python",
+               "CATEGORIES+=\tpython",
+               "CATEGORIES?=\tcategory japanese",
+               "",



Home | Main Index | Thread Index | Old Index