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:           Mon Apr 13 19:46:45 UTC 2020

Modified Files:
        pkgsrc/pkgtools/pkglint: Makefile
        pkgsrc/pkgtools/pkglint/files: mklinechecker.go package.go
            package_test.go redundantscope.go redundantscope_test.go vardefs.go
            vartype.go vartypecheck.go vartypecheck_test.go

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


To generate a diff of this commit:
cvs rdiff -u -r1.640 -r1.641 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.64 -r1.65 pkgsrc/pkgtools/pkglint/files/mklinechecker.go
cvs rdiff -u -r1.86 -r1.87 pkgsrc/pkgtools/pkglint/files/package.go
cvs rdiff -u -r1.73 -r1.74 pkgsrc/pkgtools/pkglint/files/package_test.go
cvs rdiff -u -r1.12 -r1.13 pkgsrc/pkgtools/pkglint/files/redundantscope.go
cvs rdiff -u -r1.14 -r1.15 \
    pkgsrc/pkgtools/pkglint/files/redundantscope_test.go
cvs rdiff -u -r1.92 -r1.93 pkgsrc/pkgtools/pkglint/files/vardefs.go
cvs rdiff -u -r1.48 -r1.49 pkgsrc/pkgtools/pkglint/files/vartype.go
cvs rdiff -u -r1.83 -r1.84 pkgsrc/pkgtools/pkglint/files/vartypecheck.go
cvs rdiff -u -r1.78 -r1.79 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: pkgsrc/pkgtools/pkglint/Makefile
diff -u pkgsrc/pkgtools/pkglint/Makefile:1.640 pkgsrc/pkgtools/pkglint/Makefile:1.641
--- pkgsrc/pkgtools/pkglint/Makefile:1.640      Sun Apr 12 11:01:44 2020
+++ pkgsrc/pkgtools/pkglint/Makefile    Mon Apr 13 19:46:44 2020
@@ -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/}

Index: pkgsrc/pkgtools/pkglint/files/mklinechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.64 pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.65
--- pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.64 Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker.go      Mon Apr 13 19:46:44 2020
@@ -382,6 +382,21 @@ func (ck MkLineChecker) CheckRelativePkg
        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()) {

Index: pkgsrc/pkgtools/pkglint/files/package.go
diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.86 pkgsrc/pkgtools/pkglint/files/package.go:1.87
--- pkgsrc/pkgtools/pkglint/files/package.go:1.86       Thu Mar 26 07:02:44 2020
+++ pkgsrc/pkgtools/pkglint/files/package.go    Mon Apr 13 19:46:44 2020
@@ -711,7 +711,6 @@ func (pkg *Package) checkfilePackageMake
                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 @@ func (pkg *Package) CheckVarorder(mkline
                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
 

Index: pkgsrc/pkgtools/pkglint/files/package_test.go
diff -u pkgsrc/pkgtools/pkglint/files/package_test.go:1.73 pkgsrc/pkgtools/pkglint/files/package_test.go:1.74
--- pkgsrc/pkgtools/pkglint/files/package_test.go:1.73  Thu Mar 26 07:02:44 2020
+++ pkgsrc/pkgtools/pkglint/files/package_test.go       Mon Apr 13 19:46:44 2020
@@ -2282,74 +2282,6 @@ func (s *Suite) Test_Package_CheckVarord
                        "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)
 

Index: pkgsrc/pkgtools/pkglint/files/redundantscope.go
diff -u pkgsrc/pkgtools/pkglint/files/redundantscope.go:1.12 pkgsrc/pkgtools/pkglint/files/redundantscope.go:1.13
--- pkgsrc/pkgtools/pkglint/files/redundantscope.go:1.12        Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/redundantscope.go     Mon Apr 13 19:46:44 2020
@@ -152,6 +152,51 @@ func (s *RedundantScope) handleVarassign
                                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)
+               }
        }
 }
 

Index: pkgsrc/pkgtools/pkglint/files/redundantscope_test.go
diff -u pkgsrc/pkgtools/pkglint/files/redundantscope_test.go:1.14 pkgsrc/pkgtools/pkglint/files/redundantscope_test.go:1.15
--- pkgsrc/pkgtools/pkglint/files/redundantscope_test.go:1.14   Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/redundantscope_test.go        Mon Apr 13 19:46:44 2020
@@ -1584,6 +1584,116 @@ func (s *Suite) Test_RedundantScope_hand
                        "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",
+               "",
+               ".if 1",
+               "CATEGORIES+=\tchinese",
+               ".endif")
+       t.Chdir("category/package")
+       t.FinishSetUp()
+
+       G.Check(".")
+
+       // Even though the "chinese" category is conditional, pkglint can
+       // diagnose that everything that happens before that conditional
+       // assignment adds to the constant value of the variable.
+       // Therefore it flags the duplicate category "python".
+       t.CheckOutputLines(
+               "NOTE: included.mk:2: Adding \"python\" to CATEGORIES is redundant " +
+                       "because it will later be appended in line 3.")
+}
+
+// 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_RedundantScope_checkAppendUnique__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: Makefile:5: Adding \"category\" to CATEGORIES is redundant " +
+                       "because it will later be appended in included.mk:2.")
+}
+
 func (s *Suite) Test_includePath_includes(c *check.C) {
        t := s.Init(c)
 

Index: pkgsrc/pkgtools/pkglint/files/vardefs.go
diff -u pkgsrc/pkgtools/pkglint/files/vardefs.go:1.92 pkgsrc/pkgtools/pkglint/files/vardefs.go:1.93
--- pkgsrc/pkgtools/pkglint/files/vardefs.go:1.92       Thu Mar 26 07:02:44 2020
+++ pkgsrc/pkgtools/pkglint/files/vardefs.go    Mon Apr 13 19:46:44 2020
@@ -861,7 +861,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.sys("AR", BtShellCommand, AlwaysInScope|DefinedIfInScope|NonemptyIfDefined)
        reg.sys("AS", BtShellCommand, AlwaysInScope|DefinedIfInScope|NonemptyIfDefined)
        reg.pkglist("AUTOCONF_REQD", BtVersion)
-       reg.pkglist("AUTOMAKE_OVERRIDE", BtPathPattern)
+       reg.pkglist("AUTOMAKE_OVERRIDE", BtYesNo)
        reg.pkglist("AUTOMAKE_REQD", BtVersion)
        reg.pkg("AUTO_MKDIRS", BtYesNo)
        reg.usr("BATCH", BtYes)
@@ -1007,7 +1007,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
                "builtin.mk: set")
        reg.sys("BUILTIN_X11_TYPE", BtUnknown)
        reg.sys("BUILTIN_X11_VERSION", BtUnknown)
-       reg.pkglist("CATEGORIES", BtCategory)
+       reg.DefineName("CATEGORIES", BtCategory, List|PackageSettable|Unique, "pkglist")
        reg.sysload("CC_VERSION", BtMessage, DefinedIfInScope|NonemptyIfDefined)
        reg.sysload("CC", BtShellCommand)
        reg.pkglistbl3("CFLAGS", BtCFlag)   // may also be changed by the user
@@ -1041,8 +1041,8 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.pkg("CMAKE_ARG_PATH", BtPathname)
        reg.pkglist("CMAKE_ARGS", BtShellWord)
        reg.pkglist("CMAKE_ARGS.*", BtShellWord)
-       reg.pkglist("CMAKE_DEPENDENCIES_REWRITE", BtPathPattern) // Relative to WRKSRC
-       reg.pkglist("CMAKE_MODULE_PATH_OVERRIDE", BtPathPattern) // Relative to WRKSRC
+       reg.pkglist("CMAKE_DEPENDENCIES_REWRITE", BtWrksrcPathPattern)
+       reg.pkglist("CMAKE_MODULE_PATH_OVERRIDE", BtWrksrcPathPattern)
        reg.pkg("CMAKE_PKGSRC_BUILD_FLAGS", BtYesNo)
        reg.pkglist("CMAKE_PREFIX_PATH", BtPathPattern)
        reg.pkg("CMAKE_USE_GNU_INSTALL_DIRS", BtYesNo)
@@ -1059,11 +1059,10 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.pkg("CONFIGURE_HAS_LIBDIR", BtYesNo)
        reg.pkg("CONFIGURE_HAS_MANDIR", BtYesNo)
        reg.pkg("CONFIGURE_SCRIPT", BtPathname)
-       reg.pkglist("CONFIG_GUESS_OVERRIDE", BtPathPattern)
-       reg.pkglist("CONFIG_STATUS_OVERRIDE", BtPathPattern)
+       reg.pkglist("CONFIG_GUESS_OVERRIDE", BtWrksrcPathPattern)
        reg.pkg("CONFIG_SHELL", BtShellCommand)
        reg.cmdline("CONFIG_SHELL_FLAGS", BtShellWord, List)
-       reg.pkglist("CONFIG_SUB_OVERRIDE", BtPathPattern)
+       reg.pkglist("CONFIG_SUB_OVERRIDE", BtWrksrcPathPattern)
        reg.pkglist("CONFLICTS", BtDependencyPattern)
        reg.pkgappend("CONF_FILES", BtConfFiles)
        reg.pkg("CONF_FILES_MODE", enum("0644 0640 0600 0400"))
@@ -1151,7 +1150,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.sys("EXTRACT_CMD", BtShellCommand)
        reg.pkg("EXTRACT_DIR", BtPathname)
        reg.pkg("EXTRACT_DIR.*", BtPathname)
-       reg.pkglist("EXTRACT_ELEMENTS", BtPathPattern)
+       reg.pkglist("EXTRACT_ELEMENTS", BtPathPattern) // TODO: No slashes allowed
        reg.pkglist("EXTRACT_ENV", BtShellWord)
        reg.pkglist("EXTRACT_ONLY", BtPathname)
        reg.pkglist("EXTRACT_OPTS", BtShellWord)
@@ -1266,7 +1265,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.pkglist("LIBS", BtLdFlag)
        reg.pkglist("LIBS.*", BtLdFlag)
        reg.sys("LIBTOOL", BtShellCommand)
-       reg.pkglist("LIBTOOL_OVERRIDE", BtPathPattern)
+       reg.pkglist("LIBTOOL_OVERRIDE", BtWrksrcPathPattern)
        reg.pkglistrat("LIBTOOL_REQD", BtVersion)
        reg.pkgappend("LICENCE", BtLicense)
        reg.pkgappend("LICENSE", BtLicense)
@@ -1277,7 +1276,6 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.sysload("LOWER_OPSYS", BtIdentifierDirect, NonemptyIfDefined)
        reg.sysload("LOWER_VENDOR", BtIdentifierDirect, NonemptyIfDefined)
        reg.sysloadlist("LP64PLATFORMS", BtMachinePlatformPattern, DefinedIfInScope|NonemptyIfDefined)
-       reg.pkglist("LTCONFIG_OVERRIDE", BtPathPattern)
 
        // See devel/bmake/files/main.c:/Var_Set."MACHINE_ARCH"/.
        reg.sysload("MACHINE_ARCH", BtMachineArch, AlwaysInScope|DefinedIfInScope|NonemptyIfDefined)
@@ -1429,7 +1427,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
                PackageSettable,
                "builtin.mk: set, append",
                "special:pkgconfig-builtin.mk: use-loadtime")
-       reg.pkglist("PKGCONFIG_OVERRIDE", BtPathPattern)
+       reg.pkglist("PKGCONFIG_OVERRIDE", BtWrksrcPathPattern)
        reg.pkg("PKGCONFIG_OVERRIDE_STAGE", BtStage)
        reg.pkg("PKGDIR", BtRelativePkgDir)
        reg.sys("PKGDIRMODE", BtFileMode)
@@ -1596,17 +1594,17 @@ func (reg *VarTypeRegistry) Init(src *Pk
        // is a plain string.
        reg.pkg("REPLACE.*", BtUnknown)
 
-       reg.pkglist("REPLACE_AWK", BtPathPattern)
-       reg.pkglist("REPLACE_BASH", BtPathPattern)
-       reg.pkglist("REPLACE_CSH", BtPathPattern)
-       reg.pkglist("REPLACE_FILES.*", BtPathPattern)
+       reg.pkglist("REPLACE_AWK", BtWrksrcPathPattern)
+       reg.pkglist("REPLACE_BASH", BtWrksrcPathPattern)
+       reg.pkglist("REPLACE_CSH", BtWrksrcPathPattern)
+       reg.pkglist("REPLACE_FILES.*", BtWrksrcPathPattern)
        reg.pkglist("REPLACE_INTERPRETER", BtIdentifierIndirect)
-       reg.pkglist("REPLACE_KSH", BtPathPattern)
+       reg.pkglist("REPLACE_KSH", BtWrksrcPathPattern)
        reg.pkglist("REPLACE_LOCALEDIR_PATTERNS", BtFilePattern)
-       reg.pkglist("REPLACE_LUA", BtPathPattern)
-       reg.pkglist("REPLACE_PERL", BtPathPattern)
-       reg.pkglist("REPLACE_PYTHON", BtPathPattern)
-       reg.pkglist("REPLACE_SH", BtPathPattern)
+       reg.pkglist("REPLACE_LUA", BtWrksrcPathPattern)
+       reg.pkglist("REPLACE_PERL", BtWrksrcPathPattern)
+       reg.pkglist("REPLACE_PYTHON", BtWrksrcPathPattern)
+       reg.pkglist("REPLACE_SH", BtWrksrcPathPattern)
        reg.pkglist("REQD_DIRS", BtPathname)
        reg.pkglist("REQD_DIRS_PERMS", BtPerms)
        reg.pkglist("REQD_FILES", BtPathname)
@@ -1640,7 +1638,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.sysload("SHAREOWN", BtUserGroupName, DefinedIfInScope|NonemptyIfDefined)
        reg.sys("SHCOMMENT", BtShellCommand, DefinedIfInScope|NonemptyIfDefined)
        reg.sys("SHLIBTOOL", BtShellCommand)
-       reg.pkglist("SHLIBTOOL_OVERRIDE", BtPathPattern)
+       reg.pkglist("SHLIBTOOL_OVERRIDE", BtWrksrcPathPattern)
        reg.sysload("SHLIB_TYPE",
                enum("COFF ECOFF ELF SOM XCOFF Mach-O PE PEwin a.out aixlib dylib none"),
                DefinedIfInScope|NonemptyIfDefined)
@@ -1664,7 +1662,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
 
        reg.pkglistbl3("SUBST_CLASSES", BtIdentifierDirect)
        reg.pkglistbl3("SUBST_CLASSES.*", BtIdentifierDirect) // OPSYS-specific
-       reg.pkglistbl3("SUBST_FILES.*", BtPathPattern)
+       reg.pkglistbl3("SUBST_FILES.*", BtWrksrcPathPattern)
        reg.pkgbl3("SUBST_FILTER_CMD.*", BtShellCommand)
        reg.pkgbl3("SUBST_MESSAGE.*", BtMessage)
        reg.pkgappendbl3("SUBST_SED.*", BtSedCommands)

Index: pkgsrc/pkgtools/pkglint/files/vartype.go
diff -u pkgsrc/pkgtools/pkglint/files/vartype.go:1.48 pkgsrc/pkgtools/pkglint/files/vartype.go:1.49
--- pkgsrc/pkgtools/pkglint/files/vartype.go:1.48       Sat Mar  7 23:35:35 2020
+++ pkgsrc/pkgtools/pkglint/files/vartype.go    Mon Apr 13 19:46:44 2020
@@ -134,6 +134,12 @@ const (
        //  X11_TYPE (user-settable)
        NonemptyIfDefined
 
+       // Unique is true if it doesn't make sense to append the same
+       // value more than once to the variable.
+       //
+       // A typical example is CATEGORIES.
+       Unique
+
        NoVartypeOptions = 0
 )
 
@@ -201,6 +207,7 @@ func (vt *Vartype) IsOnePerLine() bool  
 func (vt *Vartype) IsAlwaysInScope() bool       { return vt.options&AlwaysInScope != 0 }
 func (vt *Vartype) IsDefinedIfInScope() bool    { return vt.options&DefinedIfInScope != 0 }
 func (vt *Vartype) IsNonemptyIfDefined() bool   { return vt.options&NonemptyIfDefined != 0 }
+func (vt *Vartype) IsUnique() bool              { return vt.options&Unique != 0 }
 
 func (vt *Vartype) EffectivePermissions(basename string) ACLPermissions {
        for _, aclEntry := range vt.aclEntries {
@@ -465,6 +472,7 @@ var (
        BtWrapperReorder         = &BasicType{"WrapperReorder", (*VartypeCheck).WrapperReorder}
        BtWrapperTransform       = &BasicType{"WrapperTransform", (*VartypeCheck).WrapperTransform}
        BtWrkdirSubdirectory     = &BasicType{"WrkdirSubdirectory", (*VartypeCheck).WrkdirSubdirectory}
+       BtWrksrcPathPattern      = &BasicType{"WrksrcPathPattern", (*VartypeCheck).WrksrcPathPattern}
        BtWrksrcSubdirectory     = &BasicType{"WrksrcSubdirectory", (*VartypeCheck).WrksrcSubdirectory}
        BtYes                    = &BasicType{"Yes", (*VartypeCheck).Yes}
        BtYesNo                  = &BasicType{"YesNo", (*VartypeCheck).YesNo}

Index: pkgsrc/pkgtools/pkglint/files/vartypecheck.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.83 pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.84
--- pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.83  Wed Mar 18 08:24:49 2020
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck.go       Mon Apr 13 19:46:44 2020
@@ -436,43 +436,46 @@ func (cv *VartypeCheck) DependencyWithPa
        }
 
        parts := cv.MkLine.ValueSplit(value, ":")
-       if len(parts) == 2 {
-               pattern := parts[0]
-               packagePath := NewPackagePathString(parts[1])
-               relPath := packagePath.AsRelPath()
-               pathParts := relPath.Parts()
-               pkg := pathParts[len(pathParts)-1]
-
-               if len(pathParts) >= 2 && pathParts[0] == ".." && pathParts[1] != ".." {
-                       cv.Warnf("Dependency paths should have the form \"../../category/package\".")
-                       cv.MkLine.ExplainRelativeDirs()
-               }
+       if len(parts) != 2 {
+               cv.Warnf("Invalid dependency pattern with path %q.", value)
+               cv.Explain(
+                       "Examples for valid dependency patterns with path are:",
+                       "  package-[0-9]*:../../category/package",
+                       "  package>=3.41:../../category/package",
+                       "  package-2.718{,nb*}:../../category/package")
+               return
+       }
 
-               if !containsVarUse(packagePath.String()) {
-                       ck := MkLineChecker{cv.MkLines, cv.MkLine}
-                       ck.CheckRelativePkgdir(relPath, packagePath)
-               }
+       pattern := parts[0]
+       dependencyDir := NewPath(parts[1])
+       if dependencyDir.IsAbs() {
+               cv.Errorf("Dependency paths like %q must be relative.", parts[1])
+               return
+       }
 
-               switch pkg {
-               case "gettext":
-                       cv.Warnf("Please use USE_TOOLS+=msgfmt instead of this dependency.")
-               case "perl5":
-                       cv.Warnf("Please use USE_TOOLS+=perl:run instead of this dependency.")
-               case "gmake":
-                       cv.Warnf("Please use USE_TOOLS+=gmake instead of this dependency.")
-               }
+       pathParts := dependencyDir.Parts()
+       if len(pathParts) >= 2 && pathParts[0] == ".." && pathParts[1] != ".." {
+               cv.Warnf("Dependency paths should have the form \"../../category/package\".")
+               cv.MkLine.ExplainRelativeDirs()
+       }
 
-               cv.WithValue(pattern).DependencyPattern()
+       if !containsVarUse(parts[1]) {
+               ck := MkLineChecker{cv.MkLines, cv.MkLine}
+               rel := NewRelPath(dependencyDir)
+               ck.CheckRelativePkgdir(rel, NewPackagePath(rel))
+       }
 
-               return
+       pkg := pathParts[len(pathParts)-1]
+       switch pkg {
+       case "gettext":
+               cv.Warnf("Please use USE_TOOLS+=msgfmt instead of this dependency.")
+       case "perl5":
+               cv.Warnf("Please use USE_TOOLS+=perl:run instead of this dependency.")
+       case "gmake":
+               cv.Warnf("Please use USE_TOOLS+=gmake instead of this dependency.")
        }
 
-       cv.Warnf("Invalid dependency pattern with path %q.", value)
-       cv.Explain(
-               "Examples for valid dependency patterns with path are:",
-               "  package-[0-9]*:../../category/package",
-               "  package>=3.41:../../category/package",
-               "  package-2.718{,nb*}:../../category/package")
+       cv.WithValue(pattern).DependencyPattern()
 }
 
 func (cv *VartypeCheck) DistSuffix() {
@@ -1522,6 +1525,21 @@ func (cv *VartypeCheck) WrkdirSubdirecto
        cv.Pathname()
 }
 
+// WrksrcPathPattern is a shell pattern for existing pathnames, possibly including slashes.
+func (cv *VartypeCheck) WrksrcPathPattern() {
+       cv.PathPattern()
+
+       if hasPrefix(cv.Value, "${WRKSRC}/") {
+               fix := cv.Autofix()
+               fix.Notef("The pathname patterns in %s don't need to mention ${WRKSRC}.", cv.Varname)
+               fix.Explain(
+                       "These pathname patters are interpreted relative to ${WRKSRC} by definition.",
+                       "Therefore that part can be left out.")
+               fix.Replace("${WRKSRC}/", "")
+               fix.Apply()
+       }
+}
+
 // WrksrcSubdirectory checks a directory relative to ${WRKSRC},
 // for use in CONFIGURE_DIRS and similar variables.
 func (cv *VartypeCheck) WrksrcSubdirectory() {

Index: pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.78 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.79
--- pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.78     Wed Mar 18 08:24:49 2020
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go  Mon Apr 13 19:46:44 2020
@@ -582,9 +582,55 @@ func (s *Suite) Test_VartypeCheck_Depend
                "gettext-[0-9]*:files/../../../databases/py-sqlite3")
 
        vt.Output(
-               "WARN: ~/category/package/filename.mk:31: " +
-                       "\"files/../../../databases/py-sqlite3\" is " +
+               "ERROR: ~/category/package/filename.mk:31: "+
+                       "Relative package directories like "+
+                       "\"files/../../../databases/py-sqlite3\" must be canonical.",
+               "WARN: ~/category/package/filename.mk:31: "+
+                       "\"files/../../../databases/py-sqlite3\" is "+
                        "not a valid relative package directory.")
+
+       // The path has a trailing slash.
+       // https://mail-index.netbsd.org/pkgsrc-changes/2020/03/26/msg209490.html
+       vt.Values(
+               "py-sqlite3-[0-9]*:../../databases/py-sqlite3/",
+               "py-sqlite3-[0-9]*:../../././databases/py-sqlite3")
+
+       vt.Output(
+               "ERROR: ~/category/package/filename.mk:41: "+
+                       "Relative package directories like "+
+                       "\"../../databases/py-sqlite3/\" must not end with a slash.",
+               "ERROR: ~/category/package/filename.mk:42: "+
+                       "Relative package directories like "+
+                       "\"../../././databases/py-sqlite3\" must be canonical.")
+
+       vt.Values("py-sqlite3>=0:/usr/pkg")
+
+       vt.Output(
+               "ERROR: ~/category/package/filename.mk:51: " +
+                       "Dependency paths like \"/usr/pkg\" must be relative.")
+
+       vt.Values(
+               "py-sqlite3>=0:../package/../../category/package")
+
+       // These warnings are quite redundant. It's an edge case anyway.
+       vt.Output(
+               "WARN: ~/category/package/filename.mk:61: "+
+                       "Dependency paths should have the form \"../../category/package\".",
+               "WARN: ~/category/package/filename.mk:61: "+
+                       "References to other packages should look like \"../../category/package\", not \"../package\".",
+               "ERROR: ~/category/package/filename.mk:61: "+
+                       "Relative package directories like "+
+                       "\"../package/../../category/package\" must be canonical.",
+               "WARN: ~/category/package/filename.mk:61: "+
+                       "\"../package/../../category/package\" is not a valid relative package directory.")
+
+       // The "empty" field after the colon is not even counted as a field.
+       vt.Values(
+               "py-sqlite3>=0:")
+
+       vt.Output(
+               "WARN: ~/category/package/filename.mk:71: " +
+                       "Invalid dependency pattern with path \"py-sqlite3>=0:\".")
 }
 
 func (s *Suite) Test_VartypeCheck_DistSuffix(c *check.C) {
@@ -2130,6 +2176,30 @@ func (s *Suite) Test_VartypeCheck_Wrkdir
                        "contains the invalid character \" \".")
 }
 
+func (s *Suite) Test_VartypeCheck_WrksrcPathPattern(c *check.C) {
+       t := s.Init(c)
+       vt := NewVartypeCheckTester(t, BtWrksrcPathPattern)
+
+       vt.Varname("SUBST_FILES.class")
+       vt.Op(opAssign)
+       vt.Values(
+               "relative/*.sh",
+               "${WRKSRC}/relative/*.sh")
+
+       vt.Output(
+               "NOTE: filename.mk:2: The pathname patterns in SUBST_FILES.class " +
+                       "don't need to mention ${WRKSRC}.")
+
+       t.SetUpCommandLine("--autofix")
+
+       vt.Values(
+               "relative/*.sh",
+               "${WRKSRC}/relative/*.sh")
+
+       vt.Output(
+               "AUTOFIX: filename.mk:12: Replacing \"${WRKSRC}/\" with \"\".")
+}
+
 func (s *Suite) Test_VartypeCheck_WrksrcSubdirectory(c *check.C) {
        vt := NewVartypeCheckTester(s.Init(c), BtWrksrcSubdirectory)
 



Home | Main Index | Thread Index | Old Index