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 Oct 26 11:43:36 UTC 2019
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile
pkgsrc/pkgtools/pkglint/files: check_test.go mkline.go mklinechecker.go
mklinechecker_test.go mklineparser.go mklineparser_test.go
mklines.go mklines_test.go package_test.go shell_test.go util.go
vardefs_test.go
Log Message:
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.
To generate a diff of this commit:
cvs rdiff -u -r1.602 -r1.603 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.50 -r1.51 pkgsrc/pkgtools/pkglint/files/check_test.go
cvs rdiff -u -r1.60 -r1.61 pkgsrc/pkgtools/pkglint/files/mkline.go
cvs rdiff -u -r1.48 -r1.49 pkgsrc/pkgtools/pkglint/files/mklinechecker.go
cvs rdiff -u -r1.44 -r1.45 \
pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
cvs rdiff -u -r1.1 -r1.2 pkgsrc/pkgtools/pkglint/files/mklineparser.go \
pkgsrc/pkgtools/pkglint/files/mklineparser_test.go
cvs rdiff -u -r1.56 -r1.57 pkgsrc/pkgtools/pkglint/files/mklines.go
cvs rdiff -u -r1.49 -r1.50 pkgsrc/pkgtools/pkglint/files/mklines_test.go
cvs rdiff -u -r1.54 -r1.55 pkgsrc/pkgtools/pkglint/files/package_test.go \
pkgsrc/pkgtools/pkglint/files/util.go
cvs rdiff -u -r1.53 -r1.54 pkgsrc/pkgtools/pkglint/files/shell_test.go
cvs rdiff -u -r1.21 -r1.22 pkgsrc/pkgtools/pkglint/files/vardefs_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.602 pkgsrc/pkgtools/pkglint/Makefile:1.603
--- pkgsrc/pkgtools/pkglint/Makefile:1.602 Sat Oct 26 09:51:47 2019
+++ pkgsrc/pkgtools/pkglint/Makefile Sat Oct 26 11:43:36 2019
@@ -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/}
Index: pkgsrc/pkgtools/pkglint/files/check_test.go
diff -u pkgsrc/pkgtools/pkglint/files/check_test.go:1.50 pkgsrc/pkgtools/pkglint/files/check_test.go:1.51
--- pkgsrc/pkgtools/pkglint/files/check_test.go:1.50 Sat Oct 26 09:51:48 2019
+++ pkgsrc/pkgtools/pkglint/files/check_test.go Sat Oct 26 11:43:36 2019
@@ -403,8 +403,14 @@ func (t *Tester) SetUpPackage(pkgpath st
"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:
Index: pkgsrc/pkgtools/pkglint/files/mkline.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline.go:1.60 pkgsrc/pkgtools/pkglint/files/mkline.go:1.61
--- pkgsrc/pkgtools/pkglint/files/mkline.go:1.60 Sat Oct 26 09:51:48 2019
+++ pkgsrc/pkgtools/pkglint/files/mkline.go Sat Oct 26 11:43:36 2019
@@ -76,6 +76,8 @@ func (mkline *MkLine) String() string {
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
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.48 pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.49
--- pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.48 Sat Oct 26 09:51:48 2019
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker.go Sat Oct 26 11:43:36 2019
@@ -131,8 +131,7 @@ func (ck MkLineChecker) checkInclude() {
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 @@ func (ck MkLineChecker) checkVarassignLe
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 @@ func (ck MkLineChecker) checkVarassignLe
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,",
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.44 pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.45
--- pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.44 Sat Oct 26 09:51:48 2019
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go Sat Oct 26 11:43:36 2019
@@ -150,15 +150,15 @@ func (s *Suite) Test_MkLineChecker_check
// 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 @@ func (s *Suite) Test_MkLineChecker_check
"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)
Index: pkgsrc/pkgtools/pkglint/files/mklineparser.go
diff -u pkgsrc/pkgtools/pkglint/files/mklineparser.go:1.1 pkgsrc/pkgtools/pkglint/files/mklineparser.go:1.2
--- pkgsrc/pkgtools/pkglint/files/mklineparser.go:1.1 Sat Oct 26 09:51:48 2019
+++ pkgsrc/pkgtools/pkglint/files/mklineparser.go Sat Oct 26 11:43:36 2019
@@ -355,7 +355,7 @@ func (MkLineParser) split(line *Line, te
}
}
- 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 @@ type mkLineSplitResult struct {
tokens []*MkToken
spaceBeforeComment string
hasComment bool
+ hasRationale bool // filled in later, by MkLines.collectRationale
comment string
}
Index: pkgsrc/pkgtools/pkglint/files/mklineparser_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklineparser_test.go:1.1 pkgsrc/pkgtools/pkglint/files/mklineparser_test.go:1.2
--- pkgsrc/pkgtools/pkglint/files/mklineparser_test.go:1.1 Sat Oct 26 09:51:48 2019
+++ pkgsrc/pkgtools/pkglint/files/mklineparser_test.go Sat Oct 26 11:43:36 2019
@@ -1007,6 +1007,7 @@ func (s *Suite) Test_MkLineParser_split_
"EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d")),
"",
false,
+ false,
"",
},
Index: pkgsrc/pkgtools/pkglint/files/mklines.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines.go:1.56 pkgsrc/pkgtools/pkglint/files/mklines.go:1.57
--- pkgsrc/pkgtools/pkglint/files/mklines.go:1.56 Sat Oct 26 09:51:48 2019
+++ pkgsrc/pkgtools/pkglint/files/mklines.go Sat Oct 26 11:43:36 2019
@@ -86,6 +86,7 @@ func (mklines *MkLines) Check() {
// 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 @@ func (mklines *MkLines) collectElse() {
// 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) {
Index: pkgsrc/pkgtools/pkglint/files/mklines_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.49 pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.50
--- pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.49 Tue Oct 1 21:37:59 2019
+++ pkgsrc/pkgtools/pkglint/files/mklines_test.go Sat Oct 26 11:43:36 2019
@@ -501,6 +501,77 @@ func (s *Suite) Test_MkLines_ExpandLoopV
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)
Index: pkgsrc/pkgtools/pkglint/files/package_test.go
diff -u pkgsrc/pkgtools/pkglint/files/package_test.go:1.54 pkgsrc/pkgtools/pkglint/files/package_test.go:1.55
--- pkgsrc/pkgtools/pkglint/files/package_test.go:1.54 Tue Oct 1 21:37:59 2019
+++ pkgsrc/pkgtools/pkglint/files/package_test.go Sat Oct 26 11:43:36 2019
@@ -2797,12 +2797,12 @@ func (s *Suite) Test_Package_parse__incl
"~/category/package/Makefile:12: ",
"~/category/package/Makefile:13: .include \"suppress-varorder.mk\"",
"~/category/package/suppress-varorder.mk:1: "+MkCvsID,
- "~/category/package/Makefile:14: # empty",
- "~/category/package/Makefile:15: # empty",
- "~/category/package/Makefile:16: # empty",
- "~/category/package/Makefile:17: # empty",
- "~/category/package/Makefile:18: # empty",
- "~/category/package/Makefile:19: # empty",
+ "~/category/package/Makefile:14: ",
+ "~/category/package/Makefile:15: # filler",
+ "~/category/package/Makefile:16: # filler",
+ "~/category/package/Makefile:17: # filler",
+ "~/category/package/Makefile:18: # filler",
+ "~/category/package/Makefile:19: ",
"~/category/package/Makefile:20: .include \"../../mk/dlopen.buildlink3.mk\"",
"~/category/package/../../mk/dlopen.buildlink3.mk:1: .include \"dlopen.builtin.mk\"",
"~/mk/dlopen.builtin.mk:1: .include \"pthread.builtin.mk\"",
Index: pkgsrc/pkgtools/pkglint/files/util.go
diff -u pkgsrc/pkgtools/pkglint/files/util.go:1.54 pkgsrc/pkgtools/pkglint/files/util.go:1.55
--- pkgsrc/pkgtools/pkglint/files/util.go:1.54 Sat Oct 26 09:51:48 2019
+++ pkgsrc/pkgtools/pkglint/files/util.go Sat Oct 26 11:43:36 2019
@@ -82,6 +82,14 @@ func containsStr(slice []string, s strin
return false
}
+func mapStr(slice []string, fn func(s string) string) []string {
+ result := make([]string, len(slice))
+ for i, str := range slice {
+ result[i] = fn(str)
+ }
+ return result
+}
+
// intern returns an independent copy of the given string.
//
// It should be called when only a small substring of a large string
Index: pkgsrc/pkgtools/pkglint/files/shell_test.go
diff -u pkgsrc/pkgtools/pkglint/files/shell_test.go:1.53 pkgsrc/pkgtools/pkglint/files/shell_test.go:1.54
--- pkgsrc/pkgtools/pkglint/files/shell_test.go:1.53 Tue Oct 1 21:37:59 2019
+++ pkgsrc/pkgtools/pkglint/files/shell_test.go Sat Oct 26 11:43:36 2019
@@ -1474,7 +1474,7 @@ func (s *Suite) Test_SimpleCommandChecke
t := s.Init(c)
t.SetUpPackage("category/package",
- "AUTO_MKDIRS=\tyes",
+ "AUTO_MKDIRS=\t\tyes",
"INSTALLATION_DIRS+=\tshare/redundant",
"INSTALLATION_DIRS+=\tnot/redundant ${EGDIR}")
t.CreateFileLines("category/package/PLIST",
Index: pkgsrc/pkgtools/pkglint/files/vardefs_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vardefs_test.go:1.21 pkgsrc/pkgtools/pkglint/files/vardefs_test.go:1.22
--- pkgsrc/pkgtools/pkglint/files/vardefs_test.go:1.21 Thu Sep 12 21:15:48 2019
+++ pkgsrc/pkgtools/pkglint/files/vardefs_test.go Sat Oct 26 11:43:36 2019
@@ -186,7 +186,9 @@ func (s *Suite) Test_VarTypeRegistry_Ini
G.Check(pkg)
// No warning about a missing :Q modifier.
- t.CheckOutputEmpty()
+ t.CheckOutputLines(
+ "WARN: ~/category/package/Makefile:20: " +
+ "Setting variable BROKEN_ON_PLATFORM should have a rationale.")
}
func (s *Suite) Test_VarTypeRegistry_Init__no_tracing(c *check.C) {
Home |
Main Index |
Thread Index |
Old Index