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