Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[pkgsrc/trunk]: pkgsrc/pkgtools/pkglint pkgtools/pkglint: update to 20.1.8
details: https://anonhg.NetBSD.org/pkgsrc/rev/bcf67e52d36a
branches: trunk
changeset: 432551:bcf67e52d36a
user: rillig <rillig%pkgsrc.org@localhost>
date: Sat May 23 08:51:07 2020 +0000
description:
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.
diffstat:
pkgtools/pkglint/Makefile | 4 +-
pkgtools/pkglint/files/check_test.go | 5 ++++
pkgtools/pkglint/files/options_test.go | 13 +++++++++-
pkgtools/pkglint/files/package.go | 11 ++++++--
pkgtools/pkglint/files/package_test.go | 19 ++++++++++++++++
pkgtools/pkglint/files/plist.go | 1 +
pkgtools/pkglint/files/plist_test.go | 39 ++++++++++++++++++++-------------
pkgtools/pkglint/files/vartypecheck.go | 23 ++++++++++++++-----
8 files changed, 87 insertions(+), 28 deletions(-)
diffs (284 lines):
diff -r ea3ed10a2e53 -r bcf67e52d36a pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Sat May 23 08:49:08 2020 +0000
+++ b/pkgtools/pkglint/Makefile Sat May 23 08:51:07 2020 +0000
@@ -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/}
diff -r ea3ed10a2e53 -r bcf67e52d36a pkgtools/pkglint/files/check_test.go
--- a/pkgtools/pkglint/files/check_test.go Sat May 23 08:49:08 2020 +0000
+++ b/pkgtools/pkglint/files/check_test.go Sat May 23 08:51:07 2020 +0000
@@ -450,6 +450,11 @@
//
// 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(), `^[^/]+/[^/]+$`),
diff -r ea3ed10a2e53 -r bcf67e52d36a pkgtools/pkglint/files/options_test.go
--- a/pkgtools/pkglint/files/options_test.go Sat May 23 08:49:08 2020 +0000
+++ b/pkgtools/pkglint/files/options_test.go Sat May 23 08:51:07 2020 +0000
@@ -524,6 +524,10 @@
"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 @@
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 @@
"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(".")
diff -r ea3ed10a2e53 -r bcf67e52d36a pkgtools/pkglint/files/package.go
--- a/pkgtools/pkglint/files/package.go Sat May 23 08:49:08 2020 +0000
+++ b/pkgtools/pkglint/files/package.go Sat May 23 08:51:07 2020 +0000
@@ -543,6 +543,9 @@
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 @@
// 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)}
}
diff -r ea3ed10a2e53 -r bcf67e52d36a pkgtools/pkglint/files/package_test.go
--- a/pkgtools/pkglint/files/package_test.go Sat May 23 08:49:08 2020 +0000
+++ b/pkgtools/pkglint/files/package_test.go Sat May 23 08:51:07 2020 +0000
@@ -1839,6 +1839,25 @@
"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)
diff -r ea3ed10a2e53 -r bcf67e52d36a pkgtools/pkglint/files/plist.go
--- a/pkgtools/pkglint/files/plist.go Sat May 23 08:49:08 2020 +0000
+++ b/pkgtools/pkglint/files/plist.go Sat May 23 08:51:07 2020 +0000
@@ -529,6 +529,7 @@
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}
}
diff -r ea3ed10a2e53 -r bcf67e52d36a pkgtools/pkglint/files/plist_test.go
--- a/pkgtools/pkglint/files/plist_test.go Sat May 23 08:49:08 2020 +0000
+++ b/pkgtools/pkglint/files/plist_test.go Sat May 23 08:51:07 2020 +0000
@@ -1084,22 +1084,27 @@
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 @@
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_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,
diff -r ea3ed10a2e53 -r bcf67e52d36a pkgtools/pkglint/files/vartypecheck.go
--- a/pkgtools/pkglint/files/vartypecheck.go Sat May 23 08:49:08 2020 +0000
+++ b/pkgtools/pkglint/files/vartypecheck.go Sat May 23 08:51:07 2020 +0000
@@ -1091,21 +1091,23 @@
}
}
-// 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 @@
}
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