pkgsrc-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 19.3.3
details: https://anonhg.NetBSD.org/pkgsrc/rev/29507b490a2d
branches: trunk
changeset: 403382:29507b490a2d
user: rillig <rillig%pkgsrc.org@localhost>
date: Sat Oct 26 11:43:36 2019 +0000
description:
pkgtools/pkglint: update to 19.3.3
Changes since 19.3.2:
The rationale for variables like BROKEN, GCC_REQD and for direct
inclusion of builtin.mk files may span multiple lines, and it may end
with an empty comment line.
diffstat:
pkgtools/pkglint/Makefile | 4 +-
pkgtools/pkglint/files/check_test.go | 10 +++-
pkgtools/pkglint/files/mkline.go | 2 +
pkgtools/pkglint/files/mklinechecker.go | 25 +---------
pkgtools/pkglint/files/mklinechecker_test.go | 38 +++++++++++---
pkgtools/pkglint/files/mklineparser.go | 3 +-
pkgtools/pkglint/files/mklineparser_test.go | 1 +
pkgtools/pkglint/files/mklines.go | 20 +++++++
pkgtools/pkglint/files/mklines_test.go | 71 ++++++++++++++++++++++++++++
pkgtools/pkglint/files/package_test.go | 12 ++--
pkgtools/pkglint/files/shell_test.go | 2 +-
pkgtools/pkglint/files/util.go | 8 +++
pkgtools/pkglint/files/vardefs_test.go | 4 +-
13 files changed, 155 insertions(+), 45 deletions(-)
diffs (truncated from 365 to 300 lines):
diff -r f5ad1e1e4661 -r 29507b490a2d pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Sat Oct 26 11:32:27 2019 +0000
+++ b/pkgtools/pkglint/Makefile Sat Oct 26 11:43:36 2019 +0000
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.602 2019/10/26 09:51:47 rillig Exp $
+# $NetBSD: Makefile,v 1.603 2019/10/26 11:43:36 rillig Exp $
-PKGNAME= pkglint-19.3.2
+PKGNAME= pkglint-19.3.3
CATEGORIES= pkgtools
DISTNAME= tools
MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/}
diff -r f5ad1e1e4661 -r 29507b490a2d pkgtools/pkglint/files/check_test.go
--- a/pkgtools/pkglint/files/check_test.go Sat Oct 26 11:32:27 2019 +0000
+++ b/pkgtools/pkglint/files/check_test.go Sat Oct 26 11:43:36 2019 +0000
@@ -403,8 +403,14 @@
"LICENSE=\t2-clause-bsd",
"",
".include \"suppress-varorder.mk\""}
- for len(mlines) < 19 {
- mlines = append(mlines, "# empty")
+ if len(mlines) < 19 {
+ mlines = append(mlines, "")
+ }
+ for len(mlines) < 18 {
+ mlines = append(mlines, "# filler")
+ }
+ if len(mlines) < 19 {
+ mlines = append(mlines, "")
}
line:
diff -r f5ad1e1e4661 -r 29507b490a2d pkgtools/pkglint/files/mkline.go
--- a/pkgtools/pkglint/files/mkline.go Sat Oct 26 11:32:27 2019 +0000
+++ b/pkgtools/pkglint/files/mkline.go Sat Oct 26 11:43:36 2019 +0000
@@ -76,6 +76,8 @@
func (mkline *MkLine) HasComment() bool { return mkline.splitResult.hasComment }
+func (mkline *MkLine) HasRationale() bool { return mkline.splitResult.hasRationale }
+
// Comment returns the comment after the first unescaped #.
//
// A special case are variable assignments. If these are commented out
diff -r f5ad1e1e4661 -r 29507b490a2d pkgtools/pkglint/files/mklinechecker.go
--- a/pkgtools/pkglint/files/mklinechecker.go Sat Oct 26 11:32:27 2019 +0000
+++ b/pkgtools/pkglint/files/mklinechecker.go Sat Oct 26 11:43:36 2019 +0000
@@ -131,8 +131,7 @@
mkline.Warnf("Please write \"USE_TOOLS+= intltool\" instead of this line.")
case hasSuffix(includedFile, "/builtin.mk"):
- // TODO: mkline.HasRationale
- if mkline.Basename != "hacks.mk" && !mkline.HasComment() {
+ if mkline.Basename != "hacks.mk" && !mkline.HasRationale() {
fix := mkline.Autofix()
fix.Errorf("%s must not be included directly. Include \"%s/buildlink3.mk\" instead.", includedFile, path.Dir(includedFile))
fix.Replace("builtin.mk", "buildlink3.mk")
@@ -438,12 +437,6 @@
return
}
- isRationale := func(mkline *MkLine) bool {
- return mkline.IsComment() &&
- !hasPrefix(mkline.Text, "# $") &&
- !mkline.IsCommentedVarassign()
- }
-
needsRationale := func(mkline *MkLine) bool {
if !mkline.IsVarassignMaybeCommented() {
return false
@@ -457,24 +450,10 @@
return
}
- if mkline.HasComment() {
+ if mkline.HasRationale() {
return
}
- // Check whether there is a comment directly above.
- for i, other := range ck.MkLines.mklines {
- if other == mkline && i > 0 {
- aboveIndex := i - 1
- for aboveIndex > 0 && needsRationale(ck.MkLines.mklines[aboveIndex]) {
- aboveIndex--
- }
-
- if isRationale(ck.MkLines.mklines[aboveIndex]) {
- return
- }
- }
- }
-
mkline.Warnf("Setting variable %s should have a rationale.", mkline.Varname())
mkline.Explain(
"Since this variable prevents the package from being built in some situations,",
diff -r f5ad1e1e4661 -r 29507b490a2d pkgtools/pkglint/files/mklinechecker_test.go
--- a/pkgtools/pkglint/files/mklinechecker_test.go Sat Oct 26 11:32:27 2019 +0000
+++ b/pkgtools/pkglint/files/mklinechecker_test.go Sat Oct 26 11:43:36 2019 +0000
@@ -150,15 +150,15 @@
// the expected reading order of human readers.
t.SetUpPackage("category/package",
- "ASSIGN_DIFF=\tpkg", // assignment, differs from default value
- "ASSIGN_DIFF2=\treally # ok", // ok because of the rationale in the comment
- "ASSIGN_SAME=\tdefault", // assignment, same value as default
- "DEFAULT_DIFF?=\tpkg", // default, differs from default value
- "DEFAULT_SAME?=\tdefault", // same value as default
- "FETCH_USING=\tcurl", // both user-settable and package-settable
- "APPEND_DIRS+=\tdir3", // appending requires a separate diagnostic
- "COMMENTED_SAME?=\tdefault", // commented default, same value as default
- "COMMENTED_DIFF?=\tpkg") // commented default, differs from default value
+ "ASSIGN_DIFF=\t\tpkg", // assignment, differs from default value
+ "ASSIGN_DIFF2=\t\treally # ok", // ok because of the rationale in the comment
+ "ASSIGN_SAME=\t\tdefault", // assignment, same value as default
+ "DEFAULT_DIFF?=\t\tpkg", // default, differs from default value
+ "DEFAULT_SAME?=\t\tdefault", // same value as default
+ "FETCH_USING=\t\tcurl", // both user-settable and package-settable
+ "APPEND_DIRS+=\t\tdir3", // appending requires a separate diagnostic
+ "COMMENTED_SAME?=\tdefault", // commented default, same value as default
+ "COMMENTED_DIFF?=\tpkg") // commented default, differs from default value
t.CreateFileLines("mk/defaults/mk.conf",
MkCvsID,
"ASSIGN_DIFF?=default",
@@ -413,6 +413,26 @@
"Include \"../../category/package/buildlink3.mk\" instead.")
}
+func (s *Suite) Test_MkLineChecker_checkInclude__builtin_mk_rationale(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package",
+ "# I have good reasons for including this file directly.",
+ ".include \"../../category/package/builtin.mk\"",
+ "",
+ ".include \"../../category/package/builtin.mk\"")
+ t.CreateFileLines("category/package/builtin.mk",
+ MkCvsID)
+ t.FinishSetUp()
+
+ G.checkdirPackage(t.File("category/package"))
+
+ t.CheckOutputLines(
+ "ERROR: ~/category/package/Makefile:23: " +
+ "../../category/package/builtin.mk must not be included directly. " +
+ "Include \"../../category/package/buildlink3.mk\" instead.")
+}
+
func (s *Suite) Test_MkLineChecker__permissions_in_hacks_mk(c *check.C) {
t := s.Init(c)
diff -r f5ad1e1e4661 -r 29507b490a2d pkgtools/pkglint/files/mklineparser.go
--- a/pkgtools/pkglint/files/mklineparser.go Sat Oct 26 11:32:27 2019 +0000
+++ b/pkgtools/pkglint/files/mklineparser.go Sat Oct 26 11:43:36 2019 +0000
@@ -355,7 +355,7 @@
}
}
- return mkLineSplitResult{mainTrimmed, tokens, spaceBeforeComment, hasComment, comment}
+ return mkLineSplitResult{mainTrimmed, tokens, spaceBeforeComment, hasComment, false, comment}
}
// unescapeComment takes a Makefile line, as written in a file, and splits
@@ -446,5 +446,6 @@
tokens []*MkToken
spaceBeforeComment string
hasComment bool
+ hasRationale bool // filled in later, by MkLines.collectRationale
comment string
}
diff -r f5ad1e1e4661 -r 29507b490a2d pkgtools/pkglint/files/mklineparser_test.go
--- a/pkgtools/pkglint/files/mklineparser_test.go Sat Oct 26 11:32:27 2019 +0000
+++ b/pkgtools/pkglint/files/mklineparser_test.go Sat Oct 26 11:43:36 2019 +0000
@@ -1007,6 +1007,7 @@
"EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d")),
"",
false,
+ false,
"",
},
diff -r f5ad1e1e4661 -r 29507b490a2d pkgtools/pkglint/files/mklines.go
--- a/pkgtools/pkglint/files/mklines.go Sat Oct 26 11:32:27 2019 +0000
+++ b/pkgtools/pkglint/files/mklines.go Sat Oct 26 11:43:36 2019 +0000
@@ -86,6 +86,7 @@
// In the first pass, all additions to BUILD_DEFS and USE_TOOLS
// are collected to make the order of the definitions irrelevant.
+ mklines.collectRationale()
mklines.collectUsedVariables()
mklines.collectVariables()
mklines.collectPlistVars()
@@ -403,6 +404,25 @@
// TODO: Check whether this ForEach is redundant because it is already run somewhere else.
}
+func (mklines *MkLines) collectRationale() {
+
+ useful := func(mkline *MkLine) bool {
+ comment := trimHspace(mkline.Comment())
+ return comment != "" && !hasPrefix(comment, "$")
+ }
+
+ realComment := func(mkline *MkLine) bool {
+ return mkline.IsComment() && !mkline.IsCommentedVarassign()
+ }
+
+ rationale := false
+ for _, mkline := range mklines.mklines {
+ rationale = rationale || realComment(mkline) && useful(mkline)
+ mkline.splitResult.hasRationale = rationale || useful(mkline)
+ rationale = rationale && !mkline.IsEmpty()
+ }
+}
+
func (mklines *MkLines) collectUsedVariables() {
for _, mkline := range mklines.mklines {
mkline.ForEachUsed(func(varUse *MkVarUse, time VucTime) {
diff -r f5ad1e1e4661 -r 29507b490a2d pkgtools/pkglint/files/mklines_test.go
--- a/pkgtools/pkglint/files/mklines_test.go Sat Oct 26 11:32:27 2019 +0000
+++ b/pkgtools/pkglint/files/mklines_test.go Sat Oct 26 11:43:36 2019 +0000
@@ -501,6 +501,77 @@
t.Check(values, check.HasLen, 0)
}
+func (s *Suite) Test_MkLines_collectRationale(c *check.C) {
+ t := s.Init(c)
+
+ test := func(specs ...string) {
+ lines := mapStr(specs, func(s string) string { return s[4:] })
+ mklines := t.NewMkLines("filename.mk", lines...)
+ mklines.collectRationale()
+ var actual []string
+ mklines.ForEach(func(mkline *MkLine) {
+ actual = append(actual, condStr(mkline.HasRationale(), "R ", "- ")+mkline.Text)
+ })
+ t.CheckDeepEquals(actual, specs)
+ }
+
+ // An uncommented line does not have a rationale.
+ test(
+ "- VAR=value")
+
+ // The rationale can be given at the end of the line.
+ // This is useful for short explanations or remarks.
+ test(
+ "R VAR=value # rationale")
+
+ // A rationale can be given above a line. This is useful for
+ // explanations that don't fit into a single line.
+ test(
+ "R # rationale",
+ "R VAR=value")
+
+ // A commented variable assignment is just that, commented code.
+ // It does not count as a rationale.
+ test(
+ "- #VAR=value",
+ "- VAR=value")
+
+ // An empty line ends the rationale. The empty line does have a
+ // rationale itself, but that is useless since pkglint doesn't
+ // check empty lines for rationales.
+ test(
+ "R # rationale",
+ "R ",
+ "- VAR=value")
+
+ // A rationale covers all lines that follow, until the next empty
+ // line.
+ test(
+ "R # rationale",
+ "R NOT_FOR_PLATFORM+=\tLinux-*-*",
+ "R NOT_FOR_PLATFORM+=\tNetBSD-*-*",
+ "R NOT_FOR_PLATFORM+=\tCygwin-*-*")
+
+ // Large comment blocks often end with an empty comment line.
+ test(
+ "R # rationale",
+ "R #",
+ "R NOT_FOR_PLATFORM+=\tLinux-*-*",
+ "R NOT_FOR_PLATFORM+=\tNetBSD-*-*",
+ "R NOT_FOR_PLATFORM+=\tCygwin-*-*")
+
+ // The CVS Id is not a rationale.
+ test(
+ "- "+MkCvsID,
+ "- VAR=\tvalue")
+
+ // A rationale at the end of a line applies only to that line.
+ test(
+ "- VAR=\tvalue",
+ "R VAR=\tvalue # rationale",
+ "- VAR=\tvalue")
+}
+
func (s *Suite) Test_MkLines_collectVariables(c *check.C) {
t := s.Init(c)
diff -r f5ad1e1e4661 -r 29507b490a2d pkgtools/pkglint/files/package_test.go
Home |
Main Index |
Thread Index |
Old Index