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 May 23 08:51:08 UTC 2020

Modified Files:
        pkgsrc/pkgtools/pkglint: Makefile
        pkgsrc/pkgtools/pkglint/files: check_test.go options_test.go package.go
            package_test.go plist.go plist_test.go vartypecheck.go

Log Message:
pkgtools/pkglint: update to 20.1.8

Changes since 20.1.7:

There are about 300 cases where a package defines a PLIST conditional in
PLIST_VARS but none of its PLIST files actually uses that condition.
These cases get a warning.


To generate a diff of this commit:
cvs rdiff -u -r1.646 -r1.647 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.69 -r1.70 pkgsrc/pkgtools/pkglint/files/check_test.go
cvs rdiff -u -r1.23 -r1.24 pkgsrc/pkgtools/pkglint/files/options_test.go
cvs rdiff -u -r1.89 -r1.90 pkgsrc/pkgtools/pkglint/files/package.go
cvs rdiff -u -r1.76 -r1.77 pkgsrc/pkgtools/pkglint/files/package_test.go
cvs rdiff -u -r1.57 -r1.58 pkgsrc/pkgtools/pkglint/files/plist.go
cvs rdiff -u -r1.48 -r1.49 pkgsrc/pkgtools/pkglint/files/plist_test.go
cvs rdiff -u -r1.87 -r1.88 pkgsrc/pkgtools/pkglint/files/vartypecheck.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.646 pkgsrc/pkgtools/pkglint/Makefile:1.647
--- pkgsrc/pkgtools/pkglint/Makefile:1.646      Mon May 18 19:11:16 2020
+++ pkgsrc/pkgtools/pkglint/Makefile    Sat May 23 08:51:07 2020
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.646 2020/05/18 19:11:16 rillig Exp $
+# $NetBSD: Makefile,v 1.647 2020/05/23 08:51:07 rillig Exp $
 
-PKGNAME=       pkglint-20.1.7
+PKGNAME=       pkglint-20.1.8
 CATEGORIES=    pkgtools
 DISTNAME=      tools
 MASTER_SITES=  ${MASTER_SITE_GITHUB:=golang/}

Index: pkgsrc/pkgtools/pkglint/files/check_test.go
diff -u pkgsrc/pkgtools/pkglint/files/check_test.go:1.69 pkgsrc/pkgtools/pkglint/files/check_test.go:1.70
--- pkgsrc/pkgtools/pkglint/files/check_test.go:1.69    Fri May  8 19:50:04 2020
+++ pkgsrc/pkgtools/pkglint/files/check_test.go Sat May 23 08:51:07 2020
@@ -450,6 +450,11 @@ func (t *Tester) SetUpCategory(name RelP
 //
 // If the package path does not really matter for this test,
 // just use "category/package".
+//
+// To get short pathnames in the diagnostics, t.Chdir is often called
+// afterwards, if the test only sets up a single package.
+// In that case, the returned path is often not used since passing it
+// to Pkglint.Check would generate the long pathnames in the diagnostics.
 func (t *Tester) SetUpPackage(pkgpath RelPath, makefileLines ...string) CurrPath {
        assertf(
                matches(pkgpath.String(), `^[^/]+/[^/]+$`),

Index: pkgsrc/pkgtools/pkglint/files/options_test.go
diff -u pkgsrc/pkgtools/pkglint/files/options_test.go:1.23 pkgsrc/pkgtools/pkglint/files/options_test.go:1.24
--- pkgsrc/pkgtools/pkglint/files/options_test.go:1.23  Sun Mar 15 11:31:24 2020
+++ pkgsrc/pkgtools/pkglint/files/options_test.go       Sat May 23 08:51:07 2020
@@ -524,6 +524,10 @@ func (s *Suite) Test_CheckLinesOptionsMk
                "WARN: options.mk:4: Option \"other\" should be handled below in an .if block.")
 }
 
+// In this scenario, the evaluation of the PKG_OPTIONS takes place in a
+// loop over the PLIST_VARS, which is quite indirect, compared to a
+// direct ${PKG_OPTIONS:Mnetbsd}. In most practical cases, the identifiers
+// in PLIST_VARS are literals, which still allows that they are analyzed.
 func (s *Suite) Test_CheckLinesOptionsMk__indirect(c *check.C) {
        t := s.Init(c)
 
@@ -533,7 +537,8 @@ func (s *Suite) Test_CheckLinesOptionsMk
        t.CreateFileLines("mk/bsd.options.mk")
        t.SetUpPackage("category/package",
                ".include \"options.mk\"")
-       t.CreateFileLines("category/package/options.mk",
+       t.Chdir("category/package")
+       t.CreateFileLines("options.mk",
                MkCvsID,
                "",
                "PKG_OPTIONS_VAR=\tPKG_OPTIONS.package",
@@ -556,8 +561,12 @@ func (s *Suite) Test_CheckLinesOptionsMk
                "PLIST.${option}=\tyes",
                ".  endif",
                ".endfor")
+       t.CreateFileLines("PLIST",
+               PlistCvsID,
+               "${PLIST.generic}bin/generic",
+               "${PLIST.netbsd}bin/netbsd",
+               "${PLIST.os}bin/os")
        t.FinishSetUp()
-       t.Chdir("category/package")
 
        G.Check(".")
 

Index: pkgsrc/pkgtools/pkglint/files/package.go
diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.89 pkgsrc/pkgtools/pkglint/files/package.go:1.90
--- pkgsrc/pkgtools/pkglint/files/package.go:1.89       Fri May  8 19:50:04 2020
+++ pkgsrc/pkgtools/pkglint/files/package.go    Sat May 23 08:51:07 2020
@@ -543,6 +543,9 @@ func (pkg *Package) loadPlistDirs(plistF
                        rank := NewPlistRank(plistLine.Basename)
                        pkg.PlistLines.Add(plistLine, rank)
                }
+               for _, cond := range plistLine.conditions {
+                       pkg.Plist.Conditions[strings.TrimPrefix(cond, "PLIST.")] = true
+               }
        }
 }
 
@@ -1619,12 +1622,14 @@ func (pkg *Package) Includes(filename Pa
 // 2. Ensure that the entries mentioned in the ALTERNATIVES file
 // also appear in the PLIST files.
 type PlistContent struct {
-       Dirs  map[RelPath]*PlistLine
-       Files map[RelPath]*PlistLine
+       Dirs       map[RelPath]*PlistLine
+       Files      map[RelPath]*PlistLine
+       Conditions map[string]bool // each ${PLIST.id} sets ["id"] = true.
 }
 
 func NewPlistContent() PlistContent {
        return PlistContent{
                make(map[RelPath]*PlistLine),
-               make(map[RelPath]*PlistLine)}
+               make(map[RelPath]*PlistLine),
+               make(map[string]bool)}
 }

Index: pkgsrc/pkgtools/pkglint/files/package_test.go
diff -u pkgsrc/pkgtools/pkglint/files/package_test.go:1.76 pkgsrc/pkgtools/pkglint/files/package_test.go:1.77
--- pkgsrc/pkgtools/pkglint/files/package_test.go:1.76  Fri May  8 19:50:04 2020
+++ pkgsrc/pkgtools/pkglint/files/package_test.go       Sat May 23 08:51:07 2020
@@ -1839,6 +1839,25 @@ func (s *Suite) Test_Package_checkPlist_
                "WARN: ~/category/p5-Packlist/Makefile:20: This package should not have a PLIST file.")
 }
 
+func (s *Suite) Test_Package_checkPlist__unused_PLIST_variable(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpPackage("category/package",
+               "PLIST_VARS+=\tused unused",
+               "PLIST.used=\tyes",
+               "PLIST.unused=\tyes")
+       t.CreateFileLines("category/package/PLIST",
+               PlistCvsID,
+               "${PLIST.used}bin/a")
+       t.Chdir("category/package")
+       t.FinishSetUp()
+
+       G.Check(".")
+
+       t.CheckOutputLines(
+               "WARN: Makefile:20: PLIST identifier \"unused\" is not used in any PLIST file.")
+}
+
 func (s *Suite) Test_Package_CheckVarorder__only_required_variables(c *check.C) {
        t := s.Init(c)
 

Index: pkgsrc/pkgtools/pkglint/files/plist.go
diff -u pkgsrc/pkgtools/pkglint/files/plist.go:1.57 pkgsrc/pkgtools/pkglint/files/plist.go:1.58
--- pkgsrc/pkgtools/pkglint/files/plist.go:1.57 Thu Apr 30 21:15:03 2020
+++ pkgsrc/pkgtools/pkglint/files/plist.go      Sat May 23 08:51:08 2020
@@ -529,6 +529,7 @@ func (ck *PlistChecker) checkOmf(plines 
 
 type PlistLine struct {
        *Line
+       // XXX: Why "PLIST.docs" and not simply "docs"?
        conditions []string // e.g. PLIST.docs
        text       string   // Line.Text without any conditions of the form ${PLIST.cond}
 }

Index: pkgsrc/pkgtools/pkglint/files/plist_test.go
diff -u pkgsrc/pkgtools/pkglint/files/plist_test.go:1.48 pkgsrc/pkgtools/pkglint/files/plist_test.go:1.49
--- pkgsrc/pkgtools/pkglint/files/plist_test.go:1.48    Sun Mar 22 17:43:15 2020
+++ pkgsrc/pkgtools/pkglint/files/plist_test.go Sat May 23 08:51:08 2020
@@ -1084,22 +1084,27 @@ func (s *Suite) Test_PlistChecker_checkP
 func (s *Suite) Test_PlistChecker_checkPathCond(c *check.C) {
        t := s.Init(c)
 
-       pkg := t.SetUpPackage("category/package",
+       t.SetUpPackage("category/package",
                "PLIST_VARS+=\tmk-undefined mk-yes both",
                "PLIST.mk-yes=\tyes",
                "PLIST.both=\tyes")
-       t.CreateFileLines("category/package/PLIST",
+       t.Chdir("category/package")
+       t.CreateFileLines("PLIST",
                PlistCvsID,
                "${PLIST.both}${PLIST.plist}bin/program")
        t.FinishSetUp()
 
-       G.Check(pkg)
+       G.Check(".")
 
        t.CheckOutputLines(
-               "WARN: ~/category/package/Makefile:20: "+
+               "WARN: Makefile:20: "+
+                       "PLIST identifier \"mk-undefined\" is not used in any PLIST file.",
+               "WARN: Makefile:20: "+
+                       "PLIST identifier \"mk-yes\" is not used in any PLIST file.",
+               "WARN: Makefile:20: "+
                        "\"mk-undefined\" is added to PLIST_VARS, "+
                        "but PLIST.mk-undefined is not defined in this file.",
-               "WARN: ~/category/package/PLIST:2: "+
+               "WARN: PLIST:2: "+
                        "Condition \"plist\" should be added to PLIST_VARS "+
                        "in the package Makefile.")
 }
@@ -1120,14 +1125,17 @@ func (s *Suite) Test_PlistChecker_checkC
        G.Check(pkg)
 
        t.CheckOutputLines(
-               "WARN: ~/category/package/PLIST:2: " +
-                       "Condition \"plist\" should be added to PLIST_VARS " +
+               "WARN: ~/category/package/Makefile:20: "+
+                       "PLIST identifier \"mk-yes\" is not used in any PLIST file.",
+               "WARN: ~/category/package/PLIST:2: "+
+                       "Condition \"plist\" should be added to PLIST_VARS "+
                        "in the package Makefile.")
 }
 
 // Because of the unresolvable variable in the package Makefile,
 // pkglint cannot be absolutely sure about the possible PLIST
-// conditions. Therefore all such warnings are suppressed.
+// conditions. Even though ${PLIST.plist} is missing the corresponding
+// PLIST_VARS+=plist in the Makefile, there is no warning about this.
 //
 // As of January 2020, this case typically occurs when PLIST_VARS
 // is defined based on PKG_SUPPORTED_OPTIONS. Expanding that variable
@@ -1137,27 +1145,28 @@ func (s *Suite) Test_PlistChecker_checkC
 func (s *Suite) Test_PlistChecker_checkCond__unresolvable_variable(c *check.C) {
        t := s.Init(c)
 
-       pkg := t.SetUpPackage("category/package",
+       t.SetUpPackage("category/package",
                "PLIST_VARS+=\tmk-only ${UNRESOLVABLE}",
                "PLIST.mk-only=\tyes")
-       t.CreateFileLines("category/package/PLIST",
+       t.Chdir("category/package")
+       t.CreateFileLines("PLIST",
                PlistCvsID,
                "${PLIST.plist}bin/program")
        t.FinishSetUp()
 
-       G.Check(pkg)
+       G.Check(".")
 
        t.CheckOutputLines(
-               "WARN: ~/category/package/Makefile:20: " +
+               "WARN: Makefile:20: "+
+                       "PLIST identifier \"mk-only\" is not used in any PLIST file.",
+               "WARN: Makefile:20: "+
                        "UNRESOLVABLE is used but not defined.")
 }
 
 func (s *Suite) Test_PlistChecker_checkCond__hacks_mk(c *check.C) {
        t := s.Init(c)
 
-       t.SetUpPackage("category/package",
-               "PLIST_VARS+=\tmk", // To get past the mkline == nil condition.
-               "PLIST.mk=\tyes")
+       t.SetUpPackage("category/package")
        t.Chdir("category/package")
        t.CreateFileLines("hacks.mk",
                MkCvsID,

Index: pkgsrc/pkgtools/pkglint/files/vartypecheck.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.87 pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.88
--- pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.87  Mon May 18 19:11:16 2020
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck.go       Sat May 23 08:51:08 2020
@@ -1091,21 +1091,23 @@ func (cv *VartypeCheck) Pkgrevision() {
        }
 }
 
-// PlistIdentifier checks for valid identifiers in PLIST_VARS.
+// PlistIdentifier checks for valid condition identifiers in PLIST_VARS.
+//
 // These identifiers are interpreted as regular expressions by
 // mk/plist/plist-subst.mk, therefore they are limited to very
 // few characters.
 func (cv *VartypeCheck) PlistIdentifier() {
-       if cv.Value != cv.ValueNoVar {
+       cond := cv.Value
+       if cond != cv.ValueNoVar {
                return
        }
 
        if cv.Op == opUseMatch {
                invalidPatternChars := textproc.NewByteSet("A-Za-z0-9---_*?[]")
-               invalid := invalidCharacters(cv.Value, invalidPatternChars)
+               invalid := invalidCharacters(cond, invalidPatternChars)
                if invalid != "" {
                        cv.Warnf("PLIST identifier pattern %q contains invalid characters (%s).",
-                               cv.Value, invalid)
+                               cond, invalid)
                        cv.Explain(
                                "PLIST identifiers must consist of [A-Za-z0-9-_] only.",
                                "In patterns, the characters *?[] are allowed additionally.")
@@ -1114,12 +1116,21 @@ func (cv *VartypeCheck) PlistIdentifier(
        }
 
        invalidChars := textproc.NewByteSet("A-Za-z0-9---_")
-       invalid := invalidCharacters(cv.Value, invalidChars)
+       invalid := invalidCharacters(cond, invalidChars)
        if invalid != "" {
                cv.Errorf("PLIST identifier %q contains invalid characters (%s).",
-                       cv.Value, invalid)
+                       cond, invalid)
                cv.Explain(
                        "PLIST identifiers must consist of [A-Za-z0-9-_] only.")
+               return
+       }
+
+       if cv.MkLines.pkg != nil && !cv.MkLines.pkg.Plist.Conditions[cond] {
+               cv.Warnf("PLIST identifier %q is not used in any PLIST file.", cond)
+               cv.Explain(
+                       "For every identifier that is added to PLIST_VARS,",
+                       "there should be a corresponding ${PLIST.identifier}",
+                       "in any of the PLIST files of the package.")
        }
 }
 



Home | Main Index | Thread Index | Old Index