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 Jun  6 20:42:56 UTC 2020

Modified Files:
        pkgsrc/pkgtools/pkglint: Makefile
        pkgsrc/pkgtools/pkglint/files: buildlink3.go buildlink3_test.go
            distinfo.go distinfo_test.go licenses.go licenses_test.go
            mklineparser.go mklineparser_test.go mktokenslexer.go package.go
            package_test.go pkglint_test.go util.go vardefs.go vartype.go
            vartype_test.go vartypecheck.go vartypecheck_test.go

Log Message:
pkgtools/pkglint: update to 20.1.14

Changes since 20.1.13:

Packages that don't define DISTNAME probably don't download any files
and thus may not need a distinfo file.  (There are several other
conditions involved in this, though.)

When reporting wrong distinfo hashes, always report them in the order in
which they appear in the distinfo file, not by hashmap order.

Fix panic when parsing the Makefile line "./=value", which according to
bmake is a variable assignment.  This is not used in practice though.

Disallow a leading hyphen in package option names.  There are only very
few packages that wrongly use these option selectors in
PKG_SUGGESTED_OPTIONS.

Distinguish between a tool dependency (USE_TOOLS) and a plain tool name
(TOOLS_NOOP, TOOLS_BROKEN, TOOLS_FAIL).  Allow packages to add arbitrary
tools to these lists.


To generate a diff of this commit:
cvs rdiff -u -r1.652 -r1.653 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.36 -r1.37 pkgsrc/pkgtools/pkglint/files/buildlink3.go
cvs rdiff -u -r1.44 -r1.45 pkgsrc/pkgtools/pkglint/files/buildlink3_test.go
cvs rdiff -u -r1.45 -r1.46 pkgsrc/pkgtools/pkglint/files/distinfo.go
cvs rdiff -u -r1.41 -r1.42 pkgsrc/pkgtools/pkglint/files/distinfo_test.go
cvs rdiff -u -r1.32 -r1.33 pkgsrc/pkgtools/pkglint/files/licenses.go
cvs rdiff -u -r1.27 -r1.28 pkgsrc/pkgtools/pkglint/files/licenses_test.go
cvs rdiff -u -r1.13 -r1.14 pkgsrc/pkgtools/pkglint/files/mklineparser.go
cvs rdiff -u -r1.11 -r1.12 pkgsrc/pkgtools/pkglint/files/mklineparser_test.go
cvs rdiff -u -r1.3 -r1.4 pkgsrc/pkgtools/pkglint/files/mktokenslexer.go
cvs rdiff -u -r1.91 -r1.92 pkgsrc/pkgtools/pkglint/files/package.go
cvs rdiff -u -r1.77 -r1.78 pkgsrc/pkgtools/pkglint/files/package_test.go
cvs rdiff -u -r1.65 -r1.66 pkgsrc/pkgtools/pkglint/files/pkglint_test.go
cvs rdiff -u -r1.78 -r1.79 pkgsrc/pkgtools/pkglint/files/util.go
cvs rdiff -u -r1.97 -r1.98 pkgsrc/pkgtools/pkglint/files/vardefs.go
cvs rdiff -u -r1.50 -r1.51 pkgsrc/pkgtools/pkglint/files/vartype.go
cvs rdiff -u -r1.25 -r1.26 pkgsrc/pkgtools/pkglint/files/vartype_test.go
cvs rdiff -u -r1.90 -r1.91 pkgsrc/pkgtools/pkglint/files/vartypecheck.go
cvs rdiff -u -r1.81 -r1.82 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.652 pkgsrc/pkgtools/pkglint/Makefile:1.653
--- pkgsrc/pkgtools/pkglint/Makefile:1.652      Tue Jun  2 17:52:26 2020
+++ pkgsrc/pkgtools/pkglint/Makefile    Sat Jun  6 20:42:56 2020
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.652 2020/06/02 17:52:26 rillig Exp $
+# $NetBSD: Makefile,v 1.653 2020/06/06 20:42:56 rillig Exp $
 
-PKGNAME=       pkglint-20.1.13
+PKGNAME=       pkglint-20.1.14
 CATEGORIES=    pkgtools
 DISTNAME=      tools
 MASTER_SITES=  ${MASTER_SITE_GITHUB:=golang/}

Index: pkgsrc/pkgtools/pkglint/files/buildlink3.go
diff -u pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.36 pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.37
--- pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.36    Mon Jun  1 20:49:54 2020
+++ pkgsrc/pkgtools/pkglint/files/buildlink3.go Sat Jun  6 20:42:56 2020
@@ -276,6 +276,11 @@ func (ck *Buildlink3Checker) checkVarass
                        mkline.Warnf("Only buildlink variables for %q, not %q may be set in this file.", pkgbase, varparam)
                }
        }
+
+       if varname == "pkgbase" && value != ck.pkgbase {
+               mkline.Errorf("A buildlink3.mk file must only query its own PKG_BUILD_OPTIONS.%s, not PKG_BUILD_OPTIONS.%s.",
+                       ck.pkgbase, value)
+       }
 }
 
 func (ck *Buildlink3Checker) checkVaruseInPkgbase(pkgbaseLine *MkLine) {

Index: pkgsrc/pkgtools/pkglint/files/buildlink3_test.go
diff -u pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.44 pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.45
--- pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.44       Mon Jun  1 20:49:54 2020
+++ pkgsrc/pkgtools/pkglint/files/buildlink3_test.go    Sat Jun  6 20:42:56 2020
@@ -732,10 +732,12 @@ func (s *Suite) Test_Buildlink3Checker_c
 
        G.Check(t.File("category/package/buildlink3.mk"))
 
-       // TODO: Warn about PKG_BUILD_OPTIONS.package since that variable is not defined.
        t.CheckOutputLines(
                "WARN: ~/category/package/buildlink3.mk:15: "+
                        "PKG_BUILD_OPTIONS is used but not defined.",
+               "ERROR: ~/category/package/buildlink3.mk:12: "+
+                       "A buildlink3.mk file must only query its own PKG_BUILD_OPTIONS.package, "+
+                       "not PKG_BUILD_OPTIONS.unrelated.",
                "WARN: ~/category/package/buildlink3.mk:21: "+
                        "Wrong PKG_BUILD_OPTIONS, expected \"package\" instead of \"unrelated\".")
 }
@@ -743,7 +745,7 @@ func (s *Suite) Test_Buildlink3Checker_c
 // As of October 2018, pkglint parses package dependencies a little
 // different than the pkg_* tools.
 // In all but two cases this works, this is one of the exceptions.
-// The "{totem,totem-xine}" cannot be parsed, therefore the check skipped.
+// The "{totem,totem-xine}" cannot be parsed, therefore the check is skipped.
 func (s *Suite) Test_Buildlink3Checker_checkVarassign__abi_api_versions_brace(c *check.C) {
        t := s.Init(c)
 

Index: pkgsrc/pkgtools/pkglint/files/distinfo.go
diff -u pkgsrc/pkgtools/pkglint/files/distinfo.go:1.45 pkgsrc/pkgtools/pkglint/files/distinfo.go:1.46
--- pkgsrc/pkgtools/pkglint/files/distinfo.go:1.45      Mon Jun  1 20:49:54 2020
+++ pkgsrc/pkgtools/pkglint/files/distinfo.go   Sat Jun  6 20:42:56 2020
@@ -198,12 +198,12 @@ func (ck *distinfoLinesChecker) checkAlg
        for _, alg := range algorithms {
                missing[alg] = true
        }
-       seen := map[string]distinfoHash{}
+       seen := map[string]bool{}
 
        for _, hash := range info.hashes {
                alg := hash.algorithm
                if missing[alg] {
-                       seen[alg] = hash
+                       seen[alg] = true
                        delete(missing, alg)
                }
        }
@@ -275,7 +275,11 @@ func (ck *distinfoLinesChecker) checkAlg
                }
        }
 
-       for alg, hash := range seen {
+       for _, hash := range info.hashes {
+               alg := hash.algorithm
+               if !seen[alg] {
+                       continue
+               }
                computed := compute(alg)
 
                if computed != hash.hash {

Index: pkgsrc/pkgtools/pkglint/files/distinfo_test.go
diff -u pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.41 pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.42
--- pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.41 Sun May 24 19:12:29 2020
+++ pkgsrc/pkgtools/pkglint/files/distinfo_test.go      Sat Jun  6 20:42:56 2020
@@ -149,7 +149,7 @@ func (s *Suite) Test_distinfoLinesChecke
 // PHP modules that are not PECL use the distinfo file from lang/php* but
 // their own patches directory. Therefore the distinfo file refers to missing
 // patches. Since this strange situation is caused by the pkgsrc
-// infrastructure, there is nothing a package author can do about.
+// infrastructure, there is nothing a package author can do.
 //
 // XXX: Re-check the documentation for this test.
 func (s *Suite) Test_distinfoLinesChecker_check__missing_php_patches(c *check.C) {
@@ -191,6 +191,8 @@ func (s *Suite) Test_distinfoLinesChecke
 
        G.Check(t.File("archivers/php-bz2"))
 
+       t.CheckOutputEmpty()
+
        t.CreateFileLines("archivers/php-zlib/Makefile",
                MkCvsID,
                "",

Index: pkgsrc/pkgtools/pkglint/files/licenses.go
diff -u pkgsrc/pkgtools/pkglint/files/licenses.go:1.32 pkgsrc/pkgtools/pkglint/files/licenses.go:1.33
--- pkgsrc/pkgtools/pkglint/files/licenses.go:1.32      Sat Jan 18 21:56:09 2020
+++ pkgsrc/pkgtools/pkglint/files/licenses.go   Sat Jun  6 20:42:56 2020
@@ -28,10 +28,14 @@ func (lc *LicenseChecker) checkName(lice
        pkg := lc.MkLines.pkg
        if pkg != nil {
                if mkline := pkg.vars.FirstDefinition("LICENSE_FILE"); mkline != nil {
-                       // TODO: Not every path is relative to the package directory.
-                       rel := NewPackagePathString(mkline.Value())
-                       relResolved := mkline.ResolveVarsInRelativePath(rel, lc.MkLines.pkg)
-                       licenseFile = pkg.File(relResolved)
+                       value := mkline.Value()
+                       if NewPath(value).IsAbs() {
+                               mkline.Errorf("LICENSE_FILE must not be an absolute path.")
+                       } else {
+                               rel := NewPackagePathString(value)
+                               relResolved := mkline.ResolveVarsInRelativePath(rel, lc.MkLines.pkg)
+                               licenseFile = pkg.File(relResolved)
+                       }
                }
        }
        if licenseFile.IsEmpty() {

Index: pkgsrc/pkgtools/pkglint/files/licenses_test.go
diff -u pkgsrc/pkgtools/pkglint/files/licenses_test.go:1.27 pkgsrc/pkgtools/pkglint/files/licenses_test.go:1.28
--- pkgsrc/pkgtools/pkglint/files/licenses_test.go:1.27 Sun Dec  8 00:06:38 2019
+++ pkgsrc/pkgtools/pkglint/files/licenses_test.go      Sat Jun  6 20:42:56 2020
@@ -68,3 +68,17 @@ func (s *Suite) Test_LicenseChecker_chec
        t.CheckOutputLines(
                "Looks fine.")
 }
+
+func (s *Suite) Test_LicenseChecker_checkName__LICENSE_FILE_absolute(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpPkgsrc()
+       t.SetUpPackage("category/package",
+               "LICENSE_FILE=\t/usr/license")
+
+       t.Main("category/package")
+
+       t.CheckOutputLines(
+               "ERROR: ~/category/package/Makefile:20: LICENSE_FILE must not be an absolute path.",
+               "1 error found.")
+}

Index: pkgsrc/pkgtools/pkglint/files/mklineparser.go
diff -u pkgsrc/pkgtools/pkglint/files/mklineparser.go:1.13 pkgsrc/pkgtools/pkglint/files/mklineparser.go:1.14
--- pkgsrc/pkgtools/pkglint/files/mklineparser.go:1.13  Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/mklineparser.go       Sat Jun  6 20:42:56 2020
@@ -100,16 +100,9 @@ func (p MkLineParser) MatchVarassign(lin
        for !commented && lexer.SkipByte(' ') {
        }
 
-       varnameStart := lexer.Mark()
-       // TODO: duplicated code in MkLexer.Varname
-       for lexer.NextBytesSet(VarbaseBytes) != "" || lexer.NextVarUse() != nil {
-       }
-       if lexer.SkipByte('.') || hasPrefix(splitResult.main, "SITES_") {
-               for lexer.NextBytesSet(VarparamBytes) != "" || lexer.NextVarUse() != nil {
-               }
-       }
-
-       varname := lexer.Since(varnameStart)
+       mkLexer := NewMkLexer(lexer.Rest(), nil)
+       varname := mkLexer.Varname()
+       lexer.SkipMixed(len(lexer.Rest()) - len(mkLexer.Rest()))
 
        if varname == "" {
                return false, nil
@@ -215,13 +208,11 @@ func (p MkLineParser) parseCommentOrEmpt
 }
 
 func (p MkLineParser) parseDirective(line *Line, splitResult mkLineSplitResult) *MkLine {
-       text := line.Text
-       if !hasPrefix(text, ".") { // TODO: use lexer instead
+       lexer := textproc.NewLexer(splitResult.main)
+       if !lexer.SkipByte('.') {
                return nil
        }
 
-       lexer := textproc.NewLexer(splitResult.main[1:])
-
        indent := lexer.NextHspace()
        directive := lexer.NextBytesSet(LowerDash)
        switch directive {
@@ -296,7 +287,7 @@ func (p MkLineParser) parseMergeConflict
 // contain the shell commands to be associated with make targets. These cannot
 // have comments.
 //
-// If line is given, it is used for logging parse errors and warnings
+// If diag is given, it is used for logging parse errors and warnings
 // about round parentheses instead of curly braces, as well as ambiguous
 // variables of the form $v instead of ${v}.
 //

Index: pkgsrc/pkgtools/pkglint/files/mklineparser_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklineparser_test.go:1.11 pkgsrc/pkgtools/pkglint/files/mklineparser_test.go:1.12
--- pkgsrc/pkgtools/pkglint/files/mklineparser_test.go:1.11     Mon Jan  6 20:38:42 2020
+++ pkgsrc/pkgtools/pkglint/files/mklineparser_test.go  Sat Jun  6 20:42:56 2020
@@ -214,7 +214,9 @@ func (s *Suite) Test_MkLineParser_MatchV
                text := line.Text
 
                t.CheckOutputEmpty()
-               valueAlign := NewMkLineParser().Parse(line).ValueAlign()
+               mkline := NewMkLineParser().Parse(line)
+               assert(mkline.IsVarassignMaybeCommented())
+               valueAlign := mkline.ValueAlign()
                _ = t.Output()
 
                parser := NewMkLineParser()
@@ -274,6 +276,10 @@ func (s *Suite) Test_MkLineParser_MatchV
        test(" VAR=value", false, "VAR", "", "=", " VAR=", "value", "", "")
        test("VAR=value #comment", false, "VAR", "", "=", "VAR=", "value", " ", "#comment")
        test("NFILES=${FILES:[#]}", false, "NFILES", "", "=", "NFILES=", "${FILES:[#]}", "", "")
+       test(".VARNAME=value", false, ".VARNAME", "", "=", ".VARNAME=", "value", "", "")
+       test(".VAR.param=value", false, ".VAR.param", "", "=", ".VAR.param=", "value", "", "")
+       testInvalid("./=value", nil...)
+       testInvalid("#./=value", nil...)
 
        // To humans, the base variable name seems to be SITES_, being parameterized
        // with distfile-1.0.tar.gz. For pkglint though, the base variable name is

Index: pkgsrc/pkgtools/pkglint/files/mktokenslexer.go
diff -u pkgsrc/pkgtools/pkglint/files/mktokenslexer.go:1.3 pkgsrc/pkgtools/pkglint/files/mktokenslexer.go:1.4
--- pkgsrc/pkgtools/pkglint/files/mktokenslexer.go:1.3  Sat Nov 30 20:35:11 2019
+++ pkgsrc/pkgtools/pkglint/files/mktokenslexer.go      Sat Jun  6 20:42:56 2020
@@ -46,6 +46,29 @@ func (m *MkTokensLexer) Rest() string {
        return sb.String()
 }
 
+// Skip skips the next n bytes from the plain text.
+// If there is a variable use in the next n bytes, it panics; see SkipMixed.
+func (m *MkTokensLexer) Skip(n int) bool {
+       return m.Lexer.Skip(n)
+}
+
+// SkipMixed skips the next n bytes, be they in plain text or in variable uses.
+// It is only used in very special situations.
+func (m *MkTokensLexer) SkipMixed(n int) bool {
+       result := n > 0
+       for n > 0 {
+               use := m.NextVarUse()
+               if use != nil {
+                       n -= len(use.Text)
+               } else {
+                       skip := imin(len(m.Lexer.Rest()), n)
+                       assert(m.Lexer.Skip(skip))
+                       n -= skip
+               }
+       }
+       return result
+}
+
 // NextVarUse returns the next varuse token, unless there is some plain text
 // before it. In that case or at EOF, it returns nil.
 func (m *MkTokensLexer) NextVarUse() *MkToken {

Index: pkgsrc/pkgtools/pkglint/files/package.go
diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.91 pkgsrc/pkgtools/pkglint/files/package.go:1.92
--- pkgsrc/pkgtools/pkglint/files/package.go:1.91       Mon Jun  1 20:49:54 2020
+++ pkgsrc/pkgtools/pkglint/files/package.go    Sat Jun  6 20:42:56 2020
@@ -660,33 +660,12 @@ func (pkg *Package) checkfilePackageMake
                defer trace.Call(filename)()
        }
 
-       vars := pkg.vars
        pkg.checkPlist()
 
-       want := !vars.IsDefined("NO_CHECKSUM")
-       want = want && !vars.IsDefined("META_PACKAGE")
-       want = want && !(vars.IsDefined("DISTFILES") && vars.LastValue("DISTFILES") == "")
-       want = want || !isEmptyDir(pkg.File(pkg.Patchdir))
-
-       if !want {
-               distinfoFile := pkg.File(pkg.DistinfoFile)
-               if distinfoFile.IsFile() {
-                       NewLineWhole(distinfoFile).Warnf("This file should not exist since NO_CHECKSUM or META_PACKAGE is set.")
-               }
-       } else {
-               distinfoFile := pkg.File(pkg.DistinfoFile)
-               if !containsVarUse(distinfoFile.String()) && !distinfoFile.IsFile() {
-                       line := NewLineWhole(distinfoFile)
-                       line.Warnf("A package that downloads files should have a distinfo file.")
-                       line.Explain(
-                               sprintf("To generate the distinfo file, run %q.", bmake("makesum")),
-                               "",
-                               "To mark the package as not needing a distinfo file, set",
-                               "NO_CHECKSUM=yes in the package Makefile.")
-               }
-       }
+       pkg.checkDistinfoExists()
 
        // TODO: There are other REPLACE_* variables which are probably also affected by NO_CONFIGURE.
+       vars := pkg.vars
        if noConfigureLine := vars.FirstDefinition("NO_CONFIGURE"); noConfigureLine != nil {
                if replacePerlLine := vars.FirstDefinition("REPLACE_PERL"); replacePerlLine != nil {
                        replacePerlLine.Warnf("REPLACE_PERL is ignored when NO_CONFIGURE is set (in %s).",
@@ -766,6 +745,51 @@ func (pkg *Package) checkfilePackageMake
        SaveAutofixChanges(mklines.lines)
 }
 
+func (pkg *Package) checkDistinfoExists() {
+       vars := pkg.vars
+
+       want := pkg.wantDistinfo(vars)
+
+       if !want {
+               distinfoFile := pkg.File(pkg.DistinfoFile)
+               if distinfoFile.IsFile() {
+                       NewLineWhole(distinfoFile).Warnf("This file should not exist.")
+               }
+       } else {
+               distinfoFile := pkg.File(pkg.DistinfoFile)
+               if !containsVarUse(distinfoFile.String()) && !distinfoFile.IsFile() {
+                       line := NewLineWhole(distinfoFile)
+                       line.Warnf("A package that downloads files should have a distinfo file.")
+                       line.Explain(
+                               sprintf("To generate the distinfo file, run %q.", bmake("makesum")),
+                               "",
+                               "To mark the package as not needing a distinfo file, set",
+                               "NO_CHECKSUM=yes in the package Makefile.")
+               }
+       }
+}
+
+func (pkg *Package) wantDistinfo(vars Scope) bool {
+       switch {
+       case vars.IsDefined("DISTINFO_FILE"):
+               return true
+       case vars.IsDefined("DISTFILES") && vars.LastValue("DISTFILES") != "":
+               return true
+       case vars.IsDefined("DISTFILES"):
+               break
+       case vars.IsDefined("NO_CHECKSUM"):
+               break
+       case vars.IsDefined("META_PACKAGE"):
+               break // see Test_Package_checkfilePackageMakefile__META_PACKAGE_with_patch
+       case !vars.IsDefined("DISTNAME"):
+               break
+       default:
+               return true
+       }
+
+       return !isEmptyDir(pkg.File(pkg.Patchdir))
+}
+
 // checkPlist checks whether the package needs a PLIST file,
 // or whether that file should be omitted since it is autogenerated.
 func (pkg *Package) checkPlist() {

Index: pkgsrc/pkgtools/pkglint/files/package_test.go
diff -u pkgsrc/pkgtools/pkglint/files/package_test.go:1.77 pkgsrc/pkgtools/pkglint/files/package_test.go:1.78
--- pkgsrc/pkgtools/pkglint/files/package_test.go:1.77  Sat May 23 08:51:07 2020
+++ pkgsrc/pkgtools/pkglint/files/package_test.go       Sat Jun  6 20:42:56 2020
@@ -283,6 +283,7 @@ func (s *Suite) Test_Package__distinfo_f
                MkCvsID,
                ".include \"../../multimedia/gst-base/Makefile.common\"",
                ".include \"../../mk/bsd.pkg.mk\"")
+       t.CreateFileDummyPatch("x11/gst-x11/patches/patch-aa")
        t.CreateFileLines("multimedia/gst-base/Makefile.common",
                MkCvsID,
                ".include \"plugins.mk\"")
@@ -301,8 +302,9 @@ func (s *Suite) Test_Package__distinfo_f
                "WARN: x11/gst-x11/Makefile: This package should have a PLIST file.",
                "ERROR: x11/gst-x11/Makefile: Each package must define its LICENSE.",
                "WARN: x11/gst-x11/Makefile: Each package should define a COMMENT.",
-               "WARN: x11/gst-x11/../../multimedia/gst-base/distinfo:3: "+
-                       "Patch file \"patch-aa\" does not exist in directory \"../../x11/gst-x11/patches\".",
+               "ERROR: x11/gst-x11/../../multimedia/gst-base/distinfo:3: "+
+                       "SHA1 hash of ../../x11/gst-x11/patches/patch-aa differs "+
+                       "(distinfo has 1234, patch file has 9a93207561abfef7e7550598c5a08f2c3226995b).",
                "ERROR: x11/gst-x11/Makefile: Each package must have a DESCR file.")
 }
 
@@ -570,6 +572,7 @@ func (s *Suite) Test_Package_loadPackage
        t.CreateFileLines("category/package/Makefile",
                MkCvsID,
                "",
+               "DISTNAME=\tpackage-1.0",
                "CATEGORIES=\tcategory",
                "",
                "COMMENT=\tComment",
@@ -584,10 +587,11 @@ func (s *Suite) Test_Package_loadPackage
                "Whole Makefile (with all included files) follows:",
                "~/category/package/Makefile:1: "+MkCvsID,
                "~/category/package/Makefile:2: ",
-               "~/category/package/Makefile:3: CATEGORIES=\tcategory",
-               "~/category/package/Makefile:4: ",
-               "~/category/package/Makefile:5: COMMENT=\tComment",
-               "~/category/package/Makefile:6: LICENSE=\t2-clause-bsd")
+               "~/category/package/Makefile:3: DISTNAME=\tpackage-1.0",
+               "~/category/package/Makefile:4: CATEGORIES=\tcategory",
+               "~/category/package/Makefile:5: ",
+               "~/category/package/Makefile:6: COMMENT=\tComment",
+               "~/category/package/Makefile:7: LICENSE=\t2-clause-bsd")
 }
 
 func (s *Suite) Test_Package_loadPackageMakefile(c *check.C) {
@@ -1517,8 +1521,7 @@ func (s *Suite) Test_Package_checkfilePa
 
        t.CheckOutputLines(
                "WARN: ~/category/package/Makefile:20: This package should not have a PLIST file.",
-               "WARN: ~/category/package/distinfo: "+
-                       "This file should not exist since NO_CHECKSUM or META_PACKAGE is set.")
+               "WARN: ~/category/package/distinfo: This file should not exist.")
 }
 
 func (s *Suite) Test_Package_checkfilePackageMakefile__META_PACKAGE_with_patch(c *check.C) {
@@ -1538,8 +1541,11 @@ func (s *Suite) Test_Package_checkfilePa
        G.Check(pkg)
 
        // At first it may sound strange to have a META_PACKAGE with patches.
-       // As of June 2019, there are 21 meta packages having a patches
-       // directory, being referred to by PATCHDIR.
+       // As of June 2019, there are 21 meta packages that have a patches
+       // directory.
+       // These patches are not used by the meta package itself.
+       // They are just stored there in the "most obvious location",
+       // to be used by the related packages.
        t.CheckOutputEmpty()
 }
 
@@ -1627,8 +1633,7 @@ func (s *Suite) Test_Package_checkfilePa
        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.")
+               "WARN: ~/category/package/distinfo: This file should not exist.")
 }
 
 func (s *Suite) Test_Package_checkfilePackageMakefile__distfiles(c *check.C) {
@@ -1646,6 +1651,21 @@ func (s *Suite) Test_Package_checkfilePa
                        "A package that downloads files should have a distinfo file.")
 }
 
+func (s *Suite) Test_Package_checkfilePackageMakefile__no_distname(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpPackage("category/package",
+               "#DISTNAME=\t#undefined",
+               "PKGNAME=\tpackage-1.0")
+       t.Remove("category/package/distinfo")
+       t.Chdir("category/package")
+       t.FinishSetUp()
+
+       G.Check(".")
+
+       t.CheckOutputEmpty()
+}
+
 // The fonts/t1lib package has split the options handling between the
 // package Makefile and options.mk. Make sure that this situation is
 // handled correctly by pkglint.
@@ -1797,8 +1817,7 @@ func (s *Suite) Test_Package_checkPlist_
 
        t.CheckOutputLines(
                "WARN: ~/category/package/Makefile:20: This package should not have a PLIST file.",
-               "WARN: ~/category/package/distinfo: This file should not exist "+
-                       "since NO_CHECKSUM or META_PACKAGE is set.")
+               "WARN: ~/category/package/distinfo: This file should not exist.")
 }
 
 func (s *Suite) Test_Package_checkPlist__Perl5_packlist(c *check.C) {

Index: pkgsrc/pkgtools/pkglint/files/pkglint_test.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.65 pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.66
--- pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.65  Sun May 17 07:07:19 2020
+++ pkgsrc/pkgtools/pkglint/files/pkglint_test.go       Sat Jun  6 20:42:56 2020
@@ -542,7 +542,6 @@ func (s *Suite) Test_Pkglint_checkdirPac
 
        t.CheckOutputLines(
                "WARN: Makefile: This package should have a PLIST file.",
-               "WARN: distinfo: A package that downloads files should have a distinfo file.",
                "ERROR: Makefile: Each package must define its LICENSE.",
                "WARN: Makefile: Each package should define a COMMENT.",
                "ERROR: Makefile: Each package must have a DESCR file.")

Index: pkgsrc/pkgtools/pkglint/files/util.go
diff -u pkgsrc/pkgtools/pkglint/files/util.go:1.78 pkgsrc/pkgtools/pkglint/files/util.go:1.79
--- pkgsrc/pkgtools/pkglint/files/util.go:1.78  Mon Jun  1 20:49:54 2020
+++ pkgsrc/pkgtools/pkglint/files/util.go       Sat Jun  6 20:42:56 2020
@@ -1408,6 +1408,10 @@ type LazyStringBuilder struct {
        buf      []byte
 }
 
+func NewLazyStringBuilder(expected string) LazyStringBuilder {
+       return LazyStringBuilder{expected: expected}
+}
+
 func (b *LazyStringBuilder) Write(p []byte) (n int, err error) {
        for _, c := range p {
                b.WriteByte(c)
@@ -1415,10 +1419,6 @@ func (b *LazyStringBuilder) Write(p []by
        return len(p), nil
 }
 
-func NewLazyStringBuilder(expected string) LazyStringBuilder {
-       return LazyStringBuilder{expected: expected}
-}
-
 func (b *LazyStringBuilder) Len() int {
        return b.len
 }

Index: pkgsrc/pkgtools/pkglint/files/vardefs.go
diff -u pkgsrc/pkgtools/pkglint/files/vardefs.go:1.97 pkgsrc/pkgtools/pkglint/files/vardefs.go:1.98
--- pkgsrc/pkgtools/pkglint/files/vardefs.go:1.97       Mon Jun  1 20:49:54 2020
+++ pkgsrc/pkgtools/pkglint/files/vardefs.go    Sat Jun  6 20:42:56 2020
@@ -75,6 +75,8 @@ func (reg *VarTypeRegistry) Define(varna
        }
 }
 
+// DefineName registers a variable type given an ACL name.
+// The available ACL names are listed in Init.
 func (reg *VarTypeRegistry) DefineName(varname string, basicType *BasicType, options vartypeOptions, aclName string) {
        aclEntries := reg.cache[aclName]
        assertNotNil(aclEntries)
@@ -241,15 +243,16 @@ func (reg *VarTypeRegistry) usrpkg(varna
        reg.DefineName(varname, basicType, PackageSettable|UserSettable, "usrpkg")
 }
 
-// sysload declares a system-provided variable that may already be used at load time.
+// sysloadbl3 declares a system-provided variable that may already be used at load time.
 //
-// TODO: For some of these variables, bsd.prefs.mk has to be included first.
-func (reg *VarTypeRegistry) sysload(varname string, basicType *BasicType, options ...vartypeOptions) {
-       reg.DefineName(varname, basicType, reg.options(SystemProvided, options), "sysload")
+// For most of these variables, bsd.prefs.mk has to be included before they can be used.
+// For those that are defined earlier, see AlwaysInScope.
+func (reg *VarTypeRegistry) sysloadbl3(varname string, basicType *BasicType, options ...vartypeOptions) {
+       reg.DefineName(varname, basicType, reg.options(SystemProvided, options), "sysloadbl3")
 }
 
-func (reg *VarTypeRegistry) sysloadlist(varname string, basicType *BasicType, options ...vartypeOptions) {
-       reg.DefineName(varname, basicType, reg.options(List|SystemProvided, options), "sysload")
+func (reg *VarTypeRegistry) sysloadbl3list(varname string, basicType *BasicType, options ...vartypeOptions) {
+       reg.DefineName(varname, basicType, reg.options(List|SystemProvided, options), "sysloadbl3")
 }
 
 // bl3list declares a list variable that is defined by buildlink3.mk and
@@ -457,6 +460,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
                "Makefile, Makefile.*, *.mk: default, set, use")
        reg.compile("pkglistbl3",
                "Makefile, Makefile.*, *.mk: default, set, append, use")
+
        reg.compile("sys",
                "buildlink3.mk: none",
                "*: use")
@@ -465,6 +469,9 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.compile("syslist",
                "buildlink3.mk: none",
                "*: use")
+       reg.compile("sysloadbl3",
+               "*: use, use-loadtime")
+
        reg.compile("usr",
                // TODO: why is builtin.mk missing here?
                "buildlink3.mk: none",
@@ -478,14 +485,15 @@ func (reg *VarTypeRegistry) Init(src *Pk
                "buildlink3.mk, builtin.mk: none",
                "Makefile.*, *.mk: default, set, use, use-loadtime",
                "*: use, use-loadtime")
-       reg.compile("sysload",
-               "*: use, use-loadtime")
+
        reg.compile("bl3list",
                "buildlink3.mk, builtin.mk: append",
                "*: use")
+
        reg.compile("cmdline",
                "buildlink3.mk, builtin.mk: none",
                "*: use, use-loadtime")
+
        reg.compile("infralist",
                "*: set, append, use")
 
@@ -565,14 +573,10 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.usr("CROSSBASE", BtPathname)
        reg.usr("VARBASE", BtPathname)
 
-       // X11_TYPE and X11BASE may be used in buildlink3.mk as well, which the
-       // standard sysload doesn't allow.
-       reg.acl("X11_TYPE", enum("modular native"),
-               UserSettable|DefinedIfInScope|NonemptyIfDefined,
-               "*: use, use-loadtime")
-       reg.acl("X11BASE", BtPathname,
-               UserSettable,
-               "*: use, use-loadtime")
+       reg.sysloadbl3("X11_TYPE", enum("modular native"),
+               UserSettable|DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("X11BASE", BtPathname,
+               UserSettable)
 
        reg.usr("MOTIFBASE", BtPathname)
        reg.usr("PKGINFODIR", BtPathname)
@@ -848,10 +852,10 @@ func (reg *VarTypeRegistry) Init(src *Pk
        // TODO: Determine DefinedIfInScope automatically.
        // TODO: Determine NonemptyIfDefined automatically.
 
-       reg.sysload(".newline", BtMessage, AlwaysInScope|DefinedIfInScope|NonemptyIfDefined)
-       reg.sysloadlist(".ALLSRC", BtPathname, AlwaysInScope)
-       reg.sysload(".CURDIR", BtPathname, AlwaysInScope|DefinedIfInScope|NonemptyIfDefined)
-       reg.sysload(".IMPSRC", BtPathname)
+       reg.sysloadbl3(".newline", BtMessage, AlwaysInScope|DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3list(".ALLSRC", BtPathname, AlwaysInScope)
+       reg.sysloadbl3(".CURDIR", BtPathname, AlwaysInScope|DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3(".IMPSRC", BtPathname)
        reg.sys(".TARGET", BtPathname)
        reg.sys("@", BtPathname)
        reg.pkglistbl3("ALL_ENV", BtShellWord)
@@ -872,9 +876,9 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.syslist("BDB_LIBS", BtLdFlag)
        reg.sys("BDB_TYPE", enum("db1 db2 db3 db4 db5 db6"))
        reg.syslist("BIGENDIANPLATFORMS", BtMachinePlatformPattern)
-       reg.sysload("BINGRP", BtUserGroupName, DefinedIfInScope|NonemptyIfDefined)
-       reg.sysload("BINMODE", BtFileMode, DefinedIfInScope|NonemptyIfDefined)
-       reg.sysload("BINOWN", BtUserGroupName, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("BINGRP", BtUserGroupName, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("BINMODE", BtFileMode, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("BINOWN", BtUserGroupName, DefinedIfInScope|NonemptyIfDefined)
        reg.pkglist("BOOTSTRAP_DEPENDS", BtDependencyWithPath)
        reg.pkg("BOOTSTRAP_PKG", BtYesNo)
        reg.pkglistone("BROKEN", BtShellWord)
@@ -1008,8 +1012,8 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.sys("BUILTIN_X11_TYPE", BtUnknown)
        reg.sys("BUILTIN_X11_VERSION", BtUnknown)
        reg.DefineName("CATEGORIES", BtCategory, List|PackageSettable|Unique, "pkglist")
-       reg.sysload("CC_VERSION", BtMessage, DefinedIfInScope|NonemptyIfDefined)
-       reg.sysload("CC", BtShellCommand)
+       reg.sysloadbl3("CC_VERSION", BtMessage, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("CC", BtShellCommand)
        reg.pkglistbl3("CFLAGS", BtCFlag)   // may also be changed by the user
        reg.pkglistbl3("CFLAGS.*", BtCFlag) // may also be changed by the user
        reg.acl("CHECK_BUILTIN", BtYesNo,
@@ -1106,9 +1110,9 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.acllist("DL_LIBS", BtLdFlag,
                PackageSettable,
                "*: append, use")
-       reg.sysload("DOCOWN", BtUserGroupName, DefinedIfInScope|NonemptyIfDefined)
-       reg.sysload("DOCGRP", BtUserGroupName, DefinedIfInScope|NonemptyIfDefined)
-       reg.sysload("DOCMODE", BtFileMode, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("DOCOWN", BtUserGroupName, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("DOCGRP", BtUserGroupName, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("DOCMODE", BtFileMode, DefinedIfInScope|NonemptyIfDefined)
        reg.sys("DOWNLOADED_DISTFILE", BtPathname)
        reg.sys("DO_NADA", BtShellCommand)
        reg.pkg("DYNAMIC_SITES_CMD", BtShellCommand)
@@ -1255,9 +1259,9 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.usr("KRB5_DEFAULT", enum("heimdal mit-krb5"))
        reg.sys("KRB5_TYPE", BtIdentifierIndirect)
        reg.sys("LD", BtShellCommand)
-       reg.pkglistbl3("LDFLAGS", BtLdFlag)               // May also be changed by the user.
-       reg.pkglistbl3("LDFLAGS.*", BtLdFlag)             // May also be changed by the user.
-       reg.sysload("LIBABISUFFIX", BtIdentifierIndirect) // Can also be empty.
+       reg.pkglistbl3("LDFLAGS", BtLdFlag)                  // May also be changed by the user.
+       reg.pkglistbl3("LDFLAGS.*", BtLdFlag)                // May also be changed by the user.
+       reg.sysloadbl3("LIBABISUFFIX", BtIdentifierIndirect) // Can also be empty.
        reg.sys("LIBGRP", BtUserGroupName)
        reg.sys("LIBMODE", BtFileMode)
        reg.sys("LIBOWN", BtUserGroupName)
@@ -1273,24 +1277,24 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.sys("LINK.*", BtShellCommand)
        reg.sys("LINKER_RPATH_FLAG", BtShellWord)
        reg.syslist("LITTLEENDIANPLATFORMS", BtMachinePlatformPattern)
-       reg.sysload("LOWER_OPSYS", BtIdentifierDirect, NonemptyIfDefined)
-       reg.sysload("LOWER_VENDOR", BtIdentifierDirect, NonemptyIfDefined)
-       reg.sysloadlist("LP64PLATFORMS", BtMachinePlatformPattern, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("LOWER_OPSYS", BtIdentifierDirect, NonemptyIfDefined)
+       reg.sysloadbl3("LOWER_VENDOR", BtIdentifierDirect, NonemptyIfDefined)
+       reg.sysloadbl3list("LP64PLATFORMS", BtMachinePlatformPattern, DefinedIfInScope|NonemptyIfDefined)
 
        // See devel/bmake/files/main.c:/Var_Set."MACHINE_ARCH"/.
-       reg.sysload("MACHINE_ARCH", BtMachineArch, AlwaysInScope|DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("MACHINE_ARCH", BtMachineArch, AlwaysInScope|DefinedIfInScope|NonemptyIfDefined)
 
        // From mk/endian.mk, determined by a shell program that compiles
        // a C program. That's just too much for pkglint to analyze.
-       reg.sysload("MACHINE_ENDIAN", enum("big little unknown"), DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("MACHINE_ENDIAN", enum("big little unknown"), DefinedIfInScope|NonemptyIfDefined)
 
-       reg.sysload("MACHINE_GNU_ARCH", BtMachineGnuArch, DefinedIfInScope|NonemptyIfDefined)
-       reg.sysload("MACHINE_GNU_PLATFORM", BtMachineGnuPlatform, DefinedIfInScope|NonemptyIfDefined)
-       reg.sysload("MACHINE_PLATFORM", BtMachinePlatform, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("MACHINE_GNU_ARCH", BtMachineGnuArch, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("MACHINE_GNU_PLATFORM", BtMachineGnuPlatform, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("MACHINE_PLATFORM", BtMachinePlatform, DefinedIfInScope|NonemptyIfDefined)
        reg.pkg("MAINTAINER", BtMailAddress)
 
        // See devel/bmake/files/main.c:/Var_Set."MAKE"/.
-       reg.sysload("MAKE", BtShellCommand, AlwaysInScope|DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("MAKE", BtShellCommand, AlwaysInScope|DefinedIfInScope|NonemptyIfDefined)
 
        // System-provided, but packages may extend them.
        // TODO: This needs a special declaration since the very first
@@ -1310,9 +1314,9 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.pkg("MAKE_PROGRAM", BtShellCommand)
        reg.pkg("MANCOMPRESSED", BtYesNo)
        reg.pkg("MANCOMPRESSED_IF_MANZ", BtYes)
-       reg.sysload("MANGRP", BtUserGroupName, DefinedIfInScope|NonemptyIfDefined)
-       reg.sysload("MANMODE", BtFileMode, DefinedIfInScope|NonemptyIfDefined)
-       reg.sysload("MANOWN", BtUserGroupName, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("MANGRP", BtUserGroupName, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("MANMODE", BtFileMode, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("MANOWN", BtUserGroupName, DefinedIfInScope|NonemptyIfDefined)
        reg.pkglist("MASTER_SITES", BtFetchURL)
 
        for _, filename := range []PkgsrcPath{"mk/fetch/sites.mk", "mk/fetch/fetch.mk"} {
@@ -1354,15 +1358,15 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.pkg("NO_PKGTOOLS_REQD_CHECK", BtYes)
        reg.pkg("NO_SRC_ON_CDROM", BtRestricted)
        reg.pkg("NO_SRC_ON_FTP", BtRestricted)
-       reg.sysload("OBJECT_FMT", enum("COFF ECOFF ELF SOM XCOFF Mach-O PE a.out"))
+       reg.sysloadbl3("OBJECT_FMT", enum("COFF ECOFF ELF SOM XCOFF Mach-O PE a.out"))
        reg.pkglistrat("ONLY_FOR_COMPILER", compilers)
        reg.pkglistrat("ONLY_FOR_PLATFORM", BtMachinePlatformPattern)
-       reg.sysload("OPSYS", operatingSystems, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("OPSYS", operatingSystems, DefinedIfInScope|NonemptyIfDefined)
        reg.pkglistbl3("OPSYSVARS", BtVariableName)
        reg.pkg("OSVERSION_SPECIFIC", BtYes)
-       reg.sysload("OS_VARIANT", BtIdentifierDirect, DefinedIfInScope)
-       reg.sysload("OS_VERSION", BtVersion)
-       reg.sysload("OSX_VERSION", BtVersion) // See mk/platform/Darwin.mk.
+       reg.sysloadbl3("OS_VARIANT", BtIdentifierDirect, DefinedIfInScope)
+       reg.sysloadbl3("OS_VERSION", BtVersion)
+       reg.sysloadbl3("OSX_VERSION", BtVersion) // See mk/platform/Darwin.mk.
        reg.pkg("OVERRIDE_DIRDEPTH*", BtInteger)
        reg.pkg("OVERRIDE_GNU_CONFIG_SCRIPTS", BtYes)
        reg.pkg("OWNER", BtMailAddress)
@@ -1380,7 +1384,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.pkg("PATCH_STRIP", BtShellWord)
 
        // From the PATH environment variable.
-       reg.sysload("PATH", BtPathlist, AlwaysInScope|DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("PATH", BtPathlist, AlwaysInScope|DefinedIfInScope|NonemptyIfDefined)
 
        reg.sys("PAXCTL", BtShellCommand) // See mk/pax.mk.
        reg.pkglist("PERL5_PACKLIST", BtPerl5Packlist)
@@ -1433,7 +1437,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.sys("PKGLOCALEDIR", BtPathname)
        reg.pkg("PKGNAME", BtPkgname)
        reg.sys("PKGNAME_NOREV", BtPkgname)
-       reg.sysload("PKGPATH", BtPkgpath, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("PKGPATH", BtPkgpath, DefinedIfInScope|NonemptyIfDefined)
        reg.sys("PKGREPOSITORY", BtUnknown)
        // This variable is special in that it really only makes sense to
        // be set in a package Makefile.
@@ -1441,7 +1445,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.acl("PKGREVISION", BtPkgrevision,
                PackageSettable|NonemptyIfDefined,
                "Makefile: set")
-       reg.sysload("PKGSRCDIR", BtPathname, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("PKGSRCDIR", BtPathname, DefinedIfInScope|NonemptyIfDefined)
        // This definition is only valid in the top-level Makefile,
        // not in category or package Makefiles.
        reg.acl("PKGSRCTOP", BtYes,
@@ -1452,11 +1456,11 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.sys("PKGVERSION", BtVersion)
        reg.sys("PKGVERSION_NOREV", BtVersion) // Without the nb* part.
        reg.sys("PKGWILDCARD", BtFilePattern)
-       reg.sysload("PKG_ADMIN", BtShellCommand)
+       reg.sysloadbl3("PKG_ADMIN", BtShellCommand)
        reg.sys("PKG_APACHE", enum("apache24"), DefinedIfInScope|NonemptyIfDefined)
        reg.pkglistrat("PKG_APACHE_ACCEPTED", enum("apache24"))
        reg.usr("PKG_APACHE_DEFAULT", enum("apache24"))
-       reg.sysloadlist("PKG_BUILD_OPTIONS.*", BtOption)
+       reg.sysloadbl3list("PKG_BUILD_OPTIONS.*", BtOption)
        reg.usr("PKG_CONFIG", BtYes)
        // ^^ No, this is not the popular command from GNOME, but the setting
        // whether the pkgsrc user wants configuration files automatically
@@ -1468,7 +1472,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.sys("PKG_DELETE", BtShellCommand)
        reg.pkglist("PKG_DESTDIR_SUPPORT", enum("destdir user-destdir"))
        reg.pkglistone("PKG_FAIL_REASON", BtShellWord)
-       reg.sysload("PKG_FORMAT", BtIdentifierDirect)
+       reg.sysloadbl3("PKG_FORMAT", BtIdentifierDirect)
        reg.pkg("PKG_GECOS.*", BtMessage)
        reg.pkg("PKG_GID.*", BtInteger)
        reg.pkglist("PKG_GROUPS", BtShellWord)
@@ -1483,9 +1487,9 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.acllist("PKG_HACKS", BtIdentifierDirect,
                PackageSettable,
                "*: none")
-       reg.sysload("PKG_INFO", BtShellCommand, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("PKG_INFO", BtShellCommand, DefinedIfInScope|NonemptyIfDefined)
        reg.sys("PKG_JAVA_HOME", BtPathname)
-       reg.sysload("PKG_JVM", jvms)
+       reg.sysloadbl3("PKG_JVM", jvms)
        reg.pkglistrat("PKG_JVMS_ACCEPTED", jvms)
        reg.sys("PKG_LIBTOOL", BtPathname)
 
@@ -1498,7 +1502,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
        // TODO: Consider forcing the pkgsrc packages to only define options in the
        //  options.mk file. Most packages already do this, but some still
        //  define them in the Makefile or Makefile.common.
-       reg.sysloadlist("PKG_OPTIONS", BtOption, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3list("PKG_OPTIONS", BtOption, DefinedIfInScope|NonemptyIfDefined)
        reg.usrlist("PKG_OPTIONS.*", BtOption)
        reg.pkgloadlist("PKG_LEGACY_OPTIONS", BtOption)
        reg.pkgloadlist("PKG_OPTIONS_DEPRECATED_WARNINGS", BtShellWord)
@@ -1560,7 +1564,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.syslist("PTHREAD_LDFLAGS", BtLdFlag)
        reg.syslist("PTHREAD_LIBS", BtLdFlag)
        reg.pkglistbl3("PTHREAD_OPTS", enum("native optional require"))
-       reg.sysload("PTHREAD_TYPE", BtIdentifierDirect) // Or "native" or "none".
+       reg.sysloadbl3("PTHREAD_TYPE", BtIdentifierDirect) // Or "native" or "none".
        reg.pkg("PY_PATCHPLIST", BtYes)
        reg.acl("PYPKGPREFIX",
                reg.enumFromDirs(src, "lang", `^python(\d+)$`, "py$1", "py27 py36"),
@@ -1582,7 +1586,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
        //  to "files".
        reg.pkg("RCD_SCRIPT_SRC.*", BtPathname)
        reg.pkg("RCD_SCRIPT_WRK.*", BtPathname)
-       reg.sysload("READLINE_TYPE", enum("editline none readline"),
+       reg.sysloadbl3("READLINE_TYPE", enum("editline none readline"),
                DefinedIfInScope|NonemptyIfDefined)
        reg.usr("REAL_ROOT_USER", BtUserGroupName)
        reg.usr("REAL_ROOT_GROUP", BtUserGroupName)
@@ -1633,13 +1637,13 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.usrlist("SETGID_GAMES_PERMS", BtPerms)
        reg.usrlist("SETUID_ROOT_PERMS", BtPerms)
        reg.pkg("SET_LIBDIR", BtYes)
-       reg.sysload("SHAREGRP", BtUserGroupName, DefinedIfInScope|NonemptyIfDefined)
-       reg.sysload("SHAREMODE", BtFileMode, DefinedIfInScope|NonemptyIfDefined)
-       reg.sysload("SHAREOWN", BtUserGroupName, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("SHAREGRP", BtUserGroupName, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("SHAREMODE", BtFileMode, DefinedIfInScope|NonemptyIfDefined)
+       reg.sysloadbl3("SHAREOWN", BtUserGroupName, DefinedIfInScope|NonemptyIfDefined)
        reg.sys("SHCOMMENT", BtShellCommand, DefinedIfInScope|NonemptyIfDefined)
        reg.sys("SHLIBTOOL", BtShellCommand)
        reg.pkglist("SHLIBTOOL_OVERRIDE", BtWrksrcPathPattern)
-       reg.sysload("SHLIB_TYPE",
+       reg.sysloadbl3("SHLIB_TYPE",
                enum("COFF ECOFF ELF SOM XCOFF Mach-O PE PEwin a.out aixlib dylib none"),
                DefinedIfInScope|NonemptyIfDefined)
        reg.pkglist("SITES.*", BtFetchURL)
@@ -1682,15 +1686,16 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.pkglistrat("TEXINFO_REQD", BtVersion)
        reg.pkglistbl3("TOOL_DEPENDS", BtDependencyWithPath)
        reg.syslist("TOOLS_ALIASES", BtFilename)
-       reg.syslist("TOOLS_BROKEN", BtTool)
+       reg.pkglistbl3("TOOLS_BROKEN", BtToolName)
        reg.sys("TOOLS_CMD.*", BtPathname)
-       reg.pkglist("TOOLS_CREATE", BtTool)
+       reg.pkglist("TOOLS_CREATE", BtToolName)
        reg.pkglist("TOOLS_DEPENDS.*", BtDependencyWithPath)
-       reg.syslist("TOOLS_GNU_MISSING", BtTool)
-       reg.syslist("TOOLS_NOOP", BtTool)
+       reg.pkglistbl3("TOOLS_FAIL", BtToolName)
+       reg.syslist("TOOLS_GNU_MISSING", BtToolName)
+       reg.pkglistbl3("TOOLS_NOOP", BtToolName)
        reg.sys("TOOLS_PATH.*", BtPathname)
-       reg.sysload("TOOLS_PLATFORM.*", BtShellCommand)
-       reg.sysload("TOOLS_SHELL", BtShellCommand)
+       reg.sysloadbl3("TOOLS_PLATFORM.*", BtShellCommand)
+       reg.sysloadbl3("TOOLS_SHELL", BtShellCommand)
        reg.syslist("TOUCH_FLAGS", BtShellWord)
        reg.pkglist("UAC_REQD_EXECS", BtPrefixPathname)
        reg.pkglistbl3("UNLIMIT_RESOURCES",
@@ -1746,13 +1751,13 @@ func (reg *VarTypeRegistry) Init(src *Pk
        //
        // The use-loadtime is only for devel/ncurses/Makefile.common, which
        // removes tbl from USE_TOOLS.
-       reg.acllist("USE_TOOLS", BtTool,
+       reg.acllist("USE_TOOLS", BtToolDependency,
                PackageSettable,
                "special:Makefile.common: set, append, use, use-loadtime",
                "buildlink3.mk: append",
                "builtin.mk: append, use-loadtime",
                "*: set, append, use")
-       reg.acllist("USE_TOOLS.*", BtTool, // OPSYS-specific
+       reg.acllist("USE_TOOLS.*", BtToolDependency, // OPSYS-specific
                PackageSettable,
                "buildlink3.mk, builtin.mk: append",
                "*: set, append, use")
@@ -1761,7 +1766,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
        reg.syslist("WARNINGS", BtShellWord)
        reg.sys("WARNING_MSG", BtShellCommand)
        reg.sys("WARNING_CAT", BtShellCommand)
-       reg.sysload("WRAPPER_DIR", BtPathname)
+       reg.sysloadbl3("WRAPPER_DIR", BtPathname)
        reg.pkglistbl3("WRAPPER_REORDER_CMDS", BtWrapperReorder)
        reg.pkg("WRAPPER_SHELL", BtShellCommand)
        reg.pkglist("WRAPPER_TRANSFORM_CMDS", BtWrapperTransform)

Index: pkgsrc/pkgtools/pkglint/files/vartype.go
diff -u pkgsrc/pkgtools/pkglint/files/vartype.go:1.50 pkgsrc/pkgtools/pkglint/files/vartype.go:1.51
--- pkgsrc/pkgtools/pkglint/files/vartype.go:1.50       Mon Jun  1 20:49:54 2020
+++ pkgsrc/pkgtools/pkglint/files/vartype.go    Sat Jun  6 20:42:56 2020
@@ -378,7 +378,8 @@ func (bt *BasicType) NeedsQ() bool {
                BtRelativePkgDir,
                BtRelativePkgPath,
                BtStage,
-               BtTool, // Sometimes contains a colon, but that should be ok.
+               BtToolDependency, // ok since the [ tool is usually not mentioned.
+               BtToolName,       // ok since the [ tool is usually not mentioned.
                BtUserGroupName,
                BtVersion,
                BtWrkdirSubdirectory,
@@ -462,7 +463,8 @@ var (
        BtShellCommands          = &BasicType{"ShellCommands", nil} // see func init below
        BtShellWord              = &BasicType{"ShellWord", nil}     // see func init below
        BtStage                  = &BasicType{"Stage", (*VartypeCheck).Stage}
-       BtTool                   = &BasicType{"Tool", (*VartypeCheck).Tool}
+       BtToolDependency         = &BasicType{"ToolDependency", (*VartypeCheck).ToolDependency}
+       BtToolName               = &BasicType{"ToolName", (*VartypeCheck).ToolName}
        BtUnknown                = &BasicType{"Unknown", (*VartypeCheck).Unknown}
        BtURL                    = &BasicType{"URL", (*VartypeCheck).URL}
        BtUserGroupName          = &BasicType{"UserGroupName", (*VartypeCheck).UserGroupName}

Index: pkgsrc/pkgtools/pkglint/files/vartype_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartype_test.go:1.25 pkgsrc/pkgtools/pkglint/files/vartype_test.go:1.26
--- pkgsrc/pkgtools/pkglint/files/vartype_test.go:1.25  Mon Dec 30 16:27:13 2019
+++ pkgsrc/pkgtools/pkglint/files/vartype_test.go       Sat Jun  6 20:42:56 2020
@@ -136,7 +136,6 @@ func (s *Suite) Test_Vartype_Alternative
                rules(
                        "buildlink3.mk: none",
                        "*: set"),
-               // TODO: should be "buildlink3.mk only".
                "*, but not buildlink3.mk")
 
        // If there are both positive and negative cases, preserve all the
@@ -158,7 +157,6 @@ func (s *Suite) Test_Vartype_Alternative
                        "builtin.mk: set",
                        "Makefile: none",
                        "*.mk: append"),
-               // TODO: should be "builtin.mk only".
                "builtin.mk, but not buildlink3.mk, Makefile or *.mk")
 }
 

Index: pkgsrc/pkgtools/pkglint/files/vartypecheck.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.90 pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.91
--- pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.90  Mon Jun  1 20:49:54 2020
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck.go       Sat Jun  6 20:42:56 2020
@@ -864,7 +864,13 @@ func (cv *VartypeCheck) Message() {
        }
 }
 
-// Option checks whether a single package option from options.mk conforms to the naming conventions.
+// Option checks whether a single package option from an options.mk file
+// conforms to the naming conventions.
+//
+// This check only handles option names, as in PKG_SUPPORTED_OPTIONS or
+// PKG_SUGGESTED_OPTIONS. It does not handle option selections, which may
+// pre prefixed with a hyphen to disable the option. Option selections
+// are all user-settable and therefore are out of scope for pkglint.
 func (cv *VartypeCheck) Option() {
        value := cv.Value
 
@@ -872,14 +878,11 @@ func (cv *VartypeCheck) Option() {
                return
        }
 
-       validName := regex.Pattern(`^-?([a-z][-0-9a-z_+]*)$`)
+       validName := regex.Pattern(`^([a-z][-0-9a-z_+]*)$`)
        if cv.Op == opUseMatch {
-               validName = `^-?([a-z][*+\-0-9?\[\]_a-z]*)$`
+               validName = `^([a-z][*+\-0-9?\[\]_a-z]*)$`
        }
 
-       // TODO: Distinguish between:
-       //  - a bare option name (must start with a letter),
-       //  - an option selection (may have a leading hyphen).
        m, optname := match1(value, validName)
        if !m {
                cv.Errorf("Invalid option name %q. Option names must start with a lowercase letter and be all-lowercase.", value)
@@ -1352,14 +1355,9 @@ func (cv *VartypeCheck) Stage() {
        }
 }
 
-// Tool checks for tool names like "awk", "m4:pkgsrc", "digest:bootstrap".
-//
-// TODO: Distinguish between Tool and ToolDependency.
-func (cv *VartypeCheck) Tool() {
-       if cv.Varname == "TOOLS_NOOP" && cv.Op == opAssignAppend {
-               // no warning for package-defined tool definitions
-
-       } else if m, toolname, tooldep := match2(cv.Value, `^([-\w]+|\[)(?::(\w+))?$`); m {
+// ToolDependency checks for tool dependencies like "awk", "m4:pkgsrc", "digest:bootstrap".
+func (cv *VartypeCheck) ToolDependency() {
+       if m, toolname, tooldep := match2(cv.Value, `^([-\w]+|\[)(?::(\w+))?$`); m {
                if tool, _ := G.Tool(cv.MkLines, toolname, RunTime); tool == nil {
                        cv.Errorf("Unknown tool %q.", toolname)
                }
@@ -1377,6 +1375,27 @@ func (cv *VartypeCheck) Tool() {
        }
 }
 
+// ToolName checks for a tool name without any trailing ":pkgsrc" or ":run".
+func (cv *VartypeCheck) ToolName() {
+       name := cv.Value
+       nameNoVar := cv.ValueNoVar
+
+       if contains(nameNoVar, ":") {
+               cv.Errorf("%s accepts only plain tool names, without any colon.", cv.Varname)
+               return
+       }
+       if name != nameNoVar {
+               return
+       }
+
+       if !matches(name, `^([-\w]+|\[)$`) {
+               cv.Errorf("Invalid tool name %q.", name)
+               cv.Explain(
+                       "Tool names must consist of letters, digits, underscores and hyphens only.")
+               return
+       }
+}
+
 // Unknown doesn't check for anything.
 //
 // Used for:

Index: pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.81 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.82
--- pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.81     Mon Jun  1 20:49:54 2020
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go  Sat Jun  6 20:42:56 2020
@@ -1363,18 +1363,21 @@ func (s *Suite) Test_VartypeCheck_Option
        G.Pkgsrc.PkgOptions["documented"] = "Option description"
        G.Pkgsrc.PkgOptions["undocumented"] = ""
 
-       vt.Varname("PKG_OPTIONS.pkgbase")
+       vt.Varname("PKG_SUPPORTED_OPTIONS")
        vt.Values(
                "documented",
                "undocumented",
                "unknown",
                "underscore_is_deprecated",
-               "UPPER")
+               "UPPER",
+               "-invalid")
 
        vt.Output(
                "WARN: filename.mk:3: Unknown option \"unknown\".",
                "WARN: filename.mk:4: Use of the underscore character in option names is deprecated.",
                "ERROR: filename.mk:5: Invalid option name \"UPPER\". "+
+                       "Option names must start with a lowercase letter and be all-lowercase.",
+               "ERROR: filename.mk:6: Invalid option name \"-invalid\". "+
                        "Option names must start with a lowercase letter and be all-lowercase.")
 }
 
@@ -1931,9 +1934,9 @@ func (s *Suite) Test_VartypeCheck_Stage(
                        "Use one of {pre,do,post}-{extract,patch,configure,build,test,install}.")
 }
 
-func (s *Suite) Test_VartypeCheck_Tool(c *check.C) {
+func (s *Suite) Test_VartypeCheck_ToolDependency(c *check.C) {
        t := s.Init(c)
-       vt := NewVartypeCheckTester(t, BtTool)
+       vt := NewVartypeCheckTester(t, BtToolDependency)
 
        t.SetUpTool("tool1", "", AtRunTime)
        t.SetUpTool("tool2", "", AtRunTime)
@@ -1964,19 +1967,6 @@ func (s *Suite) Test_VartypeCheck_Tool(c
                "ERROR: filename.mk:12: Invalid tool dependency \"unknown\". " +
                        "Use one of \"bootstrap\", \"build\", \"pkgsrc\", \"run\" or \"test\".")
 
-       vt.Varname("TOOLS_NOOP")
-       vt.Op(opAssignAppend)
-       vt.Values(
-               "gmake:run")
-
-       vt.Varname("TOOLS_NOOP")
-       vt.Op(opAssign) // TODO: In a Makefile, this should be equivalent to opAssignAppend.
-       vt.Values(
-               "gmake:run")
-
-       vt.Output(
-               "ERROR: filename.mk:31: Unknown tool \"gmake\".")
-
        vt.Varname("USE_TOOLS")
        vt.Op(opUseMatch)
        vt.Values(
@@ -1998,6 +1988,41 @@ func (s *Suite) Test_VartypeCheck_Tool(c
        vt.OutputEmpty()
 }
 
+func (s *Suite) Test_VartypeCheck_ToolName(c *check.C) {
+       t := s.Init(c)
+       vt := NewVartypeCheckTester(t, BtToolName)
+
+       t.SetUpTool("tool1", "", AtRunTime)
+       t.SetUpTool("tool2", "", AtRunTime)
+       t.SetUpTool("tool3", "", AtRunTime)
+
+       vt.Varname("TOOLS_BROKEN")
+       vt.Op(opAssignAppend)
+       vt.Values(
+               "tool1",
+               "tool3:anything",
+               "${t}",
+               "mal:formed:tool",
+               "unknown")
+
+       vt.Output(
+               "ERROR: filename.mk:2: TOOLS_BROKEN accepts only plain tool names, "+
+                       "without any colon.",
+               "ERROR: filename.mk:4: TOOLS_BROKEN accepts only plain tool names, "+
+                       "without any colon.")
+
+       vt.Varname("TOOLS_NOOP")
+       vt.Op(opUseMatch)
+       vt.Values(
+               "tool1",
+               "tool1\\:build",
+               "${t}\\:build")
+
+       vt.Output(
+               "ERROR: filename.mk:12: TOOLS_NOOP accepts only plain tool names, without any colon.",
+               "ERROR: filename.mk:13: TOOLS_NOOP accepts only plain tool names, without any colon.")
+}
+
 func (s *Suite) Test_VartypeCheck_Unknown(c *check.C) {
        t := s.Init(c)
        vt := NewVartypeCheckTester(t, BtUnknown)



Home | Main Index | Thread Index | Old Index