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: Fri Dec 13 01:39:24 UTC 2019
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile PLIST
pkgsrc/pkgtools/pkglint/files: buildlink3_test.go check_test.go line.go
mkassignchecker.go mklexer.go mklexer_test.go mkline.go
mklineparser.go mklineparser_test.go mklines.go mklines_test.go
mkshparser.go mkshparser_test.go mktypes.go mktypes_test.go
package_test.go path.go pkglint.1 pkgsrc.go pkgsrc_test.go
redundantscope_test.go shell.go shell_test.go shtokenizer.go
shtokenizer_test.go shtypes_test.go substcontext.go
substcontext_test.go util.go util_test.go varalignblock.go
varalignblock_test.go vardefs.go vargroups_test.go vartype.go
vartype_test.go vartypecheck.go vartypecheck_test.go
pkgsrc/pkgtools/pkglint/files/intqa: qa_test.go
pkgsrc/pkgtools/pkglint/files/textproc: lexer.go
Added Files:
pkgsrc/pkgtools/pkglint/files: mkalign.go mkalign_test.go
Log Message:
pkgtools/pkglint: update to 19.3.17
Changes since 19.3.16:
Pkglint now handles SUBST blocks correctly, even those in which some of
the variables are defined conditionally. It correctly reports those that
are missing in at least one of the possible branches.
PKG_JVM is no longer marked as deprecated. It was once package-settable.
Since 2002 it is system-provided, and the package-settable counterpart
is PKG_JVM_DEFAULT. This does not fit into pkglint's simple model of
deprecating variables since the variable name is still valid, it should
just not be defined by packages anymore.
The alignment of variable assignments has been fixed in some edge cases.
In continuation lines where the backslash is beyond column 72, the
whitespace before the continuation backslash is fixed to a single space.
To generate a diff of this commit:
cvs rdiff -u -r1.616 -r1.617 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.20 -r1.21 pkgsrc/pkgtools/pkglint/PLIST
cvs rdiff -u -r1.38 -r1.39 pkgsrc/pkgtools/pkglint/files/buildlink3_test.go
cvs rdiff -u -r1.59 -r1.60 pkgsrc/pkgtools/pkglint/files/check_test.go
cvs rdiff -u -r1.42 -r1.43 pkgsrc/pkgtools/pkglint/files/line.go \
pkgsrc/pkgtools/pkglint/files/util_test.go \
pkgsrc/pkgtools/pkglint/files/vartype.go
cvs rdiff -u -r0 -r1.1 pkgsrc/pkgtools/pkglint/files/mkalign.go \
pkgsrc/pkgtools/pkglint/files/mkalign_test.go
cvs rdiff -u -r1.2 -r1.3 pkgsrc/pkgtools/pkglint/files/mkassignchecker.go
cvs rdiff -u -r1.5 -r1.6 pkgsrc/pkgtools/pkglint/files/mklexer.go \
pkgsrc/pkgtools/pkglint/files/vargroups_test.go
cvs rdiff -u -r1.4 -r1.5 pkgsrc/pkgtools/pkglint/files/mklexer_test.go
cvs rdiff -u -r1.70 -r1.71 pkgsrc/pkgtools/pkglint/files/mkline.go
cvs rdiff -u -r1.8 -r1.9 pkgsrc/pkgtools/pkglint/files/mklineparser.go \
pkgsrc/pkgtools/pkglint/files/mklineparser_test.go
cvs rdiff -u -r1.64 -r1.65 pkgsrc/pkgtools/pkglint/files/mklines.go \
pkgsrc/pkgtools/pkglint/files/package_test.go
cvs rdiff -u -r1.56 -r1.57 pkgsrc/pkgtools/pkglint/files/mklines_test.go
cvs rdiff -u -r1.18 -r1.19 pkgsrc/pkgtools/pkglint/files/mkshparser.go
cvs rdiff -u -r1.22 -r1.23 pkgsrc/pkgtools/pkglint/files/mkshparser_test.go \
pkgsrc/pkgtools/pkglint/files/mktypes.go
cvs rdiff -u -r1.20 -r1.21 pkgsrc/pkgtools/pkglint/files/mktypes_test.go
cvs rdiff -u -r1.6 -r1.7 pkgsrc/pkgtools/pkglint/files/path.go
cvs rdiff -u -r1.62 -r1.63 pkgsrc/pkgtools/pkglint/files/pkglint.1 \
pkgsrc/pkgtools/pkglint/files/shell_test.go
cvs rdiff -u -r1.47 -r1.48 pkgsrc/pkgtools/pkglint/files/pkgsrc.go
cvs rdiff -u -r1.40 -r1.41 pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go
cvs rdiff -u -r1.10 -r1.11 \
pkgsrc/pkgtools/pkglint/files/redundantscope_test.go \
pkgsrc/pkgtools/pkglint/files/shtypes_test.go
cvs rdiff -u -r1.54 -r1.55 pkgsrc/pkgtools/pkglint/files/shell.go
cvs rdiff -u -r1.23 -r1.24 pkgsrc/pkgtools/pkglint/files/shtokenizer.go \
pkgsrc/pkgtools/pkglint/files/shtokenizer_test.go \
pkgsrc/pkgtools/pkglint/files/vartype_test.go
cvs rdiff -u -r1.32 -r1.33 pkgsrc/pkgtools/pkglint/files/substcontext.go
cvs rdiff -u -r1.31 -r1.32 pkgsrc/pkgtools/pkglint/files/substcontext_test.go
cvs rdiff -u -r1.65 -r1.66 pkgsrc/pkgtools/pkglint/files/util.go \
pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
cvs rdiff -u -r1.11 -r1.12 pkgsrc/pkgtools/pkglint/files/varalignblock.go
cvs rdiff -u -r1.7 -r1.8 pkgsrc/pkgtools/pkglint/files/varalignblock_test.go
cvs rdiff -u -r1.82 -r1.83 pkgsrc/pkgtools/pkglint/files/vardefs.go
cvs rdiff -u -r1.72 -r1.73 pkgsrc/pkgtools/pkglint/files/vartypecheck.go
cvs rdiff -u -r1.3 -r1.4 pkgsrc/pkgtools/pkglint/files/intqa/qa_test.go
cvs rdiff -u -r1.8 -r1.9 pkgsrc/pkgtools/pkglint/files/textproc/lexer.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.616 pkgsrc/pkgtools/pkglint/Makefile:1.617
--- pkgsrc/pkgtools/pkglint/Makefile:1.616 Mon Dec 9 20:38:15 2019
+++ pkgsrc/pkgtools/pkglint/Makefile Fri Dec 13 01:39:23 2019
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.616 2019/12/09 20:38:15 rillig Exp $
+# $NetBSD: Makefile,v 1.617 2019/12/13 01:39:23 rillig Exp $
-PKGNAME= pkglint-19.3.16
+PKGNAME= pkglint-19.3.17
CATEGORIES= pkgtools
DISTNAME= tools
MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/}
Index: pkgsrc/pkgtools/pkglint/PLIST
diff -u pkgsrc/pkgtools/pkglint/PLIST:1.20 pkgsrc/pkgtools/pkglint/PLIST:1.21
--- pkgsrc/pkgtools/pkglint/PLIST:1.20 Sun Dec 8 00:06:38 2019
+++ pkgsrc/pkgtools/pkglint/PLIST Fri Dec 13 01:39:23 2019
@@ -1,4 +1,4 @@
-@comment $NetBSD: PLIST,v 1.20 2019/12/08 00:06:38 rillig Exp $
+@comment $NetBSD: PLIST,v 1.21 2019/12/13 01:39:23 rillig Exp $
bin/pkglint
gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint.a
gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint/getopt.a
@@ -49,6 +49,8 @@ gopkg/src/netbsd.org/pkglint/lineslexer.
gopkg/src/netbsd.org/pkglint/lineslexer_test.go
gopkg/src/netbsd.org/pkglint/logging.go
gopkg/src/netbsd.org/pkglint/logging_test.go
+gopkg/src/netbsd.org/pkglint/mkalign.go
+gopkg/src/netbsd.org/pkglint/mkalign_test.go
gopkg/src/netbsd.org/pkglint/mkassignchecker.go
gopkg/src/netbsd.org/pkglint/mkassignchecker_test.go
gopkg/src/netbsd.org/pkglint/mkcondchecker.go
Index: pkgsrc/pkgtools/pkglint/files/buildlink3_test.go
diff -u pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.38 pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.39
--- pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.38 Sun Dec 8 00:06:38 2019
+++ pkgsrc/pkgtools/pkglint/files/buildlink3_test.go Fri Dec 13 01:39:23 2019
@@ -856,9 +856,13 @@ func (s *Suite) Test_Buildlink3Checker_c
CheckLinesBuildlink3Mk(mklines)
t.CheckOutputLines(
+ "ERROR: x11/php-wxwidgets/buildlink3.mk:3: "+
+ "Identifiers for BUILDLINK_TREE must not refer to other variables.",
"WARN: x11/php-wxwidgets/buildlink3.mk:8: "+
"To use PHP_PKG_PREFIX at load time, "+
".include \"../../mk/bsd.fast.prefs.mk\" first.",
+ "ERROR: x11/php-wxwidgets/buildlink3.mk:13: "+
+ "Identifiers for BUILDLINK_TREE must not refer to other variables.",
"WARN: x11/php-wxwidgets/buildlink3.mk:3: "+
"Please use \"php\" instead of \"${PHP_PKG_PREFIX}\" "+
"(also in other variables in this file).")
@@ -888,9 +892,13 @@ func (s *Suite) Test_Buildlink3Checker_c
CheckLinesBuildlink3Mk(mklines)
t.CheckOutputLines(
+ "ERROR: x11/py-wxwidgets/buildlink3.mk:3: "+
+ "Identifiers for BUILDLINK_TREE must not refer to other variables.",
"WARN: x11/py-wxwidgets/buildlink3.mk:8: "+
"To use PYPKGPREFIX at load time, "+
".include \"../../mk/bsd.fast.prefs.mk\" first.",
+ "ERROR: x11/py-wxwidgets/buildlink3.mk:13: "+
+ "Identifiers for BUILDLINK_TREE must not refer to other variables.",
"WARN: x11/py-wxwidgets/buildlink3.mk:3: "+
"Please use \"py\" instead of \"${PYPKGPREFIX}\" "+
"(also in other variables in this file).")
@@ -920,9 +928,13 @@ func (s *Suite) Test_Buildlink3Checker_c
CheckLinesBuildlink3Mk(mklines)
t.CheckOutputLines(
+ "ERROR: x11/ruby1-wxwidgets/buildlink3.mk:3: "+
+ "Identifiers for BUILDLINK_TREE must not refer to other variables.",
"WARN: x11/ruby1-wxwidgets/buildlink3.mk:8: "+
"To use RUBY_BASE at load time, "+
".include \"../../mk/bsd.fast.prefs.mk\" first.",
+ "ERROR: x11/ruby1-wxwidgets/buildlink3.mk:13: "+
+ "Identifiers for BUILDLINK_TREE must not refer to other variables.",
"WARN: x11/ruby1-wxwidgets/buildlink3.mk:3: "+
"Please use \"ruby\" instead of \"${RUBY_BASE}\" "+
"(also in other variables in this file).")
@@ -952,9 +964,13 @@ func (s *Suite) Test_Buildlink3Checker_c
CheckLinesBuildlink3Mk(mklines)
t.CheckOutputLines(
+ "ERROR: x11/ruby2-wxwidgets/buildlink3.mk:3: "+
+ "Identifiers for BUILDLINK_TREE must not refer to other variables.",
"WARN: x11/ruby2-wxwidgets/buildlink3.mk:8: "+
"To use RUBY_PKGPREFIX at load time, "+
".include \"../../mk/bsd.fast.prefs.mk\" first.",
+ "ERROR: x11/ruby2-wxwidgets/buildlink3.mk:13: "+
+ "Identifiers for BUILDLINK_TREE must not refer to other variables.",
"WARN: x11/ruby2-wxwidgets/buildlink3.mk:3: "+
"Please use \"ruby\" instead of \"${RUBY_PKGPREFIX}\" "+
"(also in other variables in this file).")
@@ -984,6 +1000,7 @@ func (s *Suite) Test_Buildlink3Checker_c
CheckLinesBuildlink3Mk(mklines)
t.CheckOutputLines(
+ "ERROR: buildlink3.mk:3: Identifiers for BUILDLINK_TREE must not refer to other variables.",
"WARN: buildlink3.mk:3: LICENSE should not be used in this file; "+
"it would be ok in Makefile, Makefile.* or *.mk, but not buildlink3.mk or builtin.mk.",
"WARN: buildlink3.mk:3: The variable LICENSE should be quoted as part of a shell word.",
@@ -991,6 +1008,7 @@ func (s *Suite) Test_Buildlink3Checker_c
"WARN: buildlink3.mk:8: The variable LICENSE should be quoted as part of a shell word.",
"WARN: buildlink3.mk:9: The variable LICENSE should be quoted as part of a shell word.",
"WARN: buildlink3.mk:9: The variable LICENSE should be quoted as part of a shell word.",
+ "ERROR: buildlink3.mk:13: Identifiers for BUILDLINK_TREE must not refer to other variables.",
"WARN: buildlink3.mk:13: The variable LICENSE should be quoted as part of a shell word.",
"WARN: buildlink3.mk:3: Please replace \"${LICENSE}\" with a simple string "+
"(also in other variables in this file).")
Index: pkgsrc/pkgtools/pkglint/files/check_test.go
diff -u pkgsrc/pkgtools/pkglint/files/check_test.go:1.59 pkgsrc/pkgtools/pkglint/files/check_test.go:1.60
--- pkgsrc/pkgtools/pkglint/files/check_test.go:1.59 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/check_test.go Fri Dec 13 01:39:23 2019
@@ -9,6 +9,7 @@ import (
"netbsd.org/pkglint/regex"
"os"
"regexp"
+ "strconv"
"strings"
"testing"
@@ -139,10 +140,13 @@ func Test__qa(t *testing.T) {
ck.Configure("tools.go", "*", "*", -intqa.EMissingTest) // TODO
ck.Configure("util.go", "*", "*", -intqa.EMissingTest) // TODO
ck.Configure("var.go", "*", "*", -intqa.EMissingTest) // TODO
- ck.Configure("varalignblock.go", "*", "*", -intqa.EMissingTest) // TODO
- ck.Configure("vardefs.go", "*", "*", -intqa.EMissingTest) // TODO
- ck.Configure("vargroups.go", "*", "*", -intqa.EMissingTest) // TODO
- ck.Configure("vartype.go", "*", "*", -intqa.EMissingTest) // TODO
+
+ ck.Configure("varalignblock.go", "*", "*", -intqa.EMissingTest) // TODO
+ ck.Configure("varalignblock.go", "varalignLine", "*", +intqa.EMissingTest) // TODO: remove as redundant
+
+ ck.Configure("vardefs.go", "*", "*", -intqa.EMissingTest) // TODO
+ ck.Configure("vargroups.go", "*", "*", -intqa.EMissingTest) // TODO
+ ck.Configure("vartype.go", "*", "*", -intqa.EMissingTest) // TODO
// For now, don't require tests for all the test code.
// Having good coverage for the main code is more important.
@@ -165,6 +169,7 @@ func Test__qa(t *testing.T) {
// The Suite type is used for testing all parts of pkglint.
// Therefore its test methods may be everywhere.
ck.Configure("*.go", "Suite", "*", -intqa.EMethodsSameFile)
+ ck.Configure("*.go", "Tester", "*", -intqa.EMethodsSameFile)
ck.Check()
}
@@ -858,10 +863,20 @@ func (t *Tester) CheckEquals(actual inte
return t.c.Check(actual, check.Equals, expected)
}
+func (t *Tester) CheckEqualsf(actual interface{}, expected interface{}, format string, args ...interface{}) bool {
+ return t.c.Check(actual, check.Equals, expected,
+ check.Commentf(format, args...))
+}
+
func (t *Tester) CheckDeepEquals(actual interface{}, expected interface{}) bool {
return t.c.Check(actual, check.DeepEquals, expected)
}
+func (t *Tester) CheckDeepEqualsf(actual interface{}, expected interface{}, format string, args ...interface{}) bool {
+ return t.c.Check(actual, check.DeepEquals, expected,
+ check.Commentf(format, args...))
+}
+
// InternalErrorf reports a consistency error in the tests.
func (t *Tester) InternalErrorf(format string, args ...interface{}) {
// It is not possible to panic here since check.v1 would then
@@ -1097,7 +1112,6 @@ func (t *Tester) CheckOutputLines(expect
func (t *Tester) CheckOutputLinesMatching(pattern regex.Pattern, expectedLines ...string) {
output := t.Output()
var actualLines []string
- actualLines = append(actualLines)
for _, line := range strings.Split(strings.TrimSuffix(output, "\n"), "\n") {
if matches(line, pattern) {
actualLines = append(actualLines, line)
@@ -1302,6 +1316,22 @@ func (t *Tester) CheckFileLinesDetab(fil
t.CheckDeepEquals(detabbedLines, lines)
}
+// CheckDotColumns verifies that each appearance of "..34" is indeed
+// right-aligned at column 34, taking tabs into account.
+// Columns are zero-based.
+func (t *Tester) CheckDotColumns(lines ...string) {
+ for index, line := range lines {
+ ms := regcomp(`\.\.(\d+)`).FindAllStringSubmatchIndex(line, -1)
+ for _, m := range ms {
+ prefix := line[:m[1]]
+ width := tabWidth(prefix)
+ num, err := strconv.Atoi(line[m[2]:m[3]])
+ assertNil(err, "")
+ t.CheckEqualsf(num, width, "lines[%d]", index)
+ }
+ }
+}
+
// Use marks all passed functions as used for the Go compiler.
//
// This means that the test cases that follow do not have to use each of them,
@@ -1341,3 +1371,12 @@ func (t *Tester) ReportUncheckedOutput()
_, _ = fmt.Fprintf(&msg, "\n")
_, _ = os.Stderr.WriteString(msg.String())
}
+
+// SplitStringsBool unpacks the given varargs into a string slice and a bool.
+func (t *Tester) SplitStringsBool(data []interface{}) ([]string, bool) {
+ var strs []string
+ for _, text := range data[:len(data)-1] {
+ strs = append(strs, text.(string))
+ }
+ return strs, data[len(data)-1].(bool)
+}
Index: pkgsrc/pkgtools/pkglint/files/line.go
diff -u pkgsrc/pkgtools/pkglint/files/line.go:1.42 pkgsrc/pkgtools/pkglint/files/line.go:1.43
--- pkgsrc/pkgtools/pkglint/files/line.go:1.42 Sun Dec 8 00:06:38 2019
+++ pkgsrc/pkgtools/pkglint/files/line.go Fri Dec 13 01:39:23 2019
@@ -16,6 +16,7 @@ package pkglint
import (
"netbsd.org/pkglint/regex"
"strconv"
+ "strings"
)
type RawLine struct {
@@ -36,6 +37,13 @@ type RawLine struct {
func (rline *RawLine) String() string { return sprintf("%d:%s", rline.Lineno, rline.textnl) }
+func (rline *RawLine) text() string {
+ // TODO: use this method everywhere
+ // TODO: add orig()
+ // TODO: export these two functions
+ return strings.TrimSuffix(rline.textnl, "\n")
+}
+
type Location struct {
Filename CurrPath
firstLine int32 // zero means the whole file, -1 means EOF
Index: pkgsrc/pkgtools/pkglint/files/util_test.go
diff -u pkgsrc/pkgtools/pkglint/files/util_test.go:1.42 pkgsrc/pkgtools/pkglint/files/util_test.go:1.43
--- pkgsrc/pkgtools/pkglint/files/util_test.go:1.42 Mon Dec 9 20:38:16 2019
+++ pkgsrc/pkgtools/pkglint/files/util_test.go Fri Dec 13 01:39:23 2019
@@ -221,19 +221,26 @@ func (s *Suite) Test_alignWith(c *check.
t := s.Init(c)
test := func(str, other, expected string) {
- t.CheckEquals(alignWith(str, other), expected)
+ aligned := alignWith(str, other)
+ t.CheckEquals(aligned, expected)
+ t.CheckEquals(hasPrefix(aligned, str), true)
+
+ // It would be unusual to call this function with a string
+ // that itself ends with space.
+ t.CheckEquals(rtrimHspace(aligned), str)
}
- // At least one tab is _always_ added.
- test("", "", "\t")
+ // The needed alignment may be empty.
+ // In some contexts like the value of a variable assignment, this
+ // should not happen. In other contexts it's ok.
+ test("", "", "")
test("VAR=", "1234567", "VAR= ")
test("VAR=", "12345678", "VAR=\t")
test("VAR=", "123456789", "VAR=\t ")
- // At least one tab is added in any case,
- // even if the other string is shorter.
- test("1234567890=", "V=", "1234567890=\t")
+ // If the other string is shorter, no extra tab is added.
+ test("1234567890=", "V=", "1234567890=")
}
func (s *Suite) Test_indent(c *check.C) {
@@ -252,6 +259,9 @@ func (s *Suite) Test_indent(c *check.C)
test(15, "\t ")
test(16, "\t\t")
test(72, "\t\t\t\t\t\t\t\t\t")
+ test(79, "\t\t\t\t\t\t\t\t\t ")
+ test(80, "\t\t\t\t\t\t\t\t\t\t")
+ test(87, "\t\t\t\t\t\t\t\t\t\t ")
}
func (s *Suite) Test_alignmentAfter(c *check.C) {
@@ -312,44 +322,6 @@ func (s *Suite) Test_varnameParam(c *che
t.CheckEquals(varnameParam(".CURDIR"), "")
}
-func (s *Suite) Test_mkopSubst__middle(c *check.C) {
- t := s.Init(c)
-
- t.CheckEquals(mkopSubst("pkgname", false, "kgna", false, "ri", ""), "prime")
- t.CheckEquals(mkopSubst("pkgname", false, "pkgname", false, "replacement", ""), "replacement")
- t.CheckEquals(mkopSubst("aaaaaaa", false, "a", false, "b", ""), "baaaaaa")
-}
-
-func (s *Suite) Test_mkopSubst__left(c *check.C) {
- t := s.Init(c)
-
- t.CheckEquals(mkopSubst("pkgname", true, "kgna", false, "ri", ""), "pkgname")
- t.CheckEquals(mkopSubst("pkgname", true, "pkgname", false, "replacement", ""), "replacement")
-}
-
-func (s *Suite) Test_mkopSubst__right(c *check.C) {
- t := s.Init(c)
-
- t.CheckEquals(mkopSubst("pkgname", false, "kgna", true, "ri", ""), "pkgname")
- t.CheckEquals(mkopSubst("pkgname", false, "pkgname", true, "replacement", ""), "replacement")
-}
-
-func (s *Suite) Test_mkopSubst__left_and_right(c *check.C) {
- t := s.Init(c)
-
- t.CheckEquals(mkopSubst("pkgname", true, "kgna", true, "ri", ""), "pkgname")
- t.CheckEquals(mkopSubst("pkgname", false, "pkgname", false, "replacement", ""), "replacement")
-}
-
-func (s *Suite) Test_mkopSubst__gflag(c *check.C) {
- t := s.Init(c)
-
- t.CheckEquals(mkopSubst("aaaaa", false, "a", false, "b", "g"), "bbbbb")
- t.CheckEquals(mkopSubst("aaaaa", true, "a", false, "b", "g"), "baaaa")
- t.CheckEquals(mkopSubst("aaaaa", false, "a", true, "b", "g"), "aaaab")
- t.CheckEquals(mkopSubst("aaaaa", true, "a", true, "b", "g"), "aaaaa")
-}
-
func (s *Suite) Test__regex_ReplaceFirst(c *check.C) {
t := s.Init(c)
@@ -520,18 +492,33 @@ func (s *Suite) Test_Scope_Define(c *che
t := s.Init(c)
scope := NewScope()
- scope.Define("BUILD_DIRS", t.NewMkLine("file.mk", 121, "BUILD_DIRS=\tone two three"))
- t.CheckEquals(scope.LastValue("BUILD_DIRS"), "one two three")
+ test := func(line string, ok bool, value string) {
+ scope.Define("BUILD_DIRS", t.NewMkLine("file.mk", 123, line))
+
+ actualValue, actualFound := scope.LastValueFound("BUILD_DIRS")
+
+ t.CheckEquals(actualValue, value)
+ t.CheckEquals(actualFound, ok)
+ }
+
+ test("BUILD_DIRS?=\tdefault",
+ true, "default")
- scope.Define("BUILD_DIRS", t.NewMkLine("file.mk", 123, "BUILD_DIRS+=\tfour"))
+ test(
+ "BUILD_DIRS=\tone two three",
+ true, "one two three")
- t.CheckEquals(scope.LastValue("BUILD_DIRS"), "one two three four")
+ test(
+ "BUILD_DIRS+=\tfour",
+ true, "one two three four")
// Later default assignments do not have an effect.
- scope.Define("BUILD_DIRS", t.NewMkLine("file.mk", 123, "BUILD_DIRS?=\tdefault"))
+ test("BUILD_DIRS?=\tdefault",
+ true, "one two three four")
- t.CheckEquals(scope.LastValue("BUILD_DIRS"), "one two three four")
+ test("BUILD_DIRS!=\techo dynamic",
+ false, "")
}
func (s *Suite) Test_Scope_Mentioned(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/vartype.go
diff -u pkgsrc/pkgtools/pkglint/files/vartype.go:1.42 pkgsrc/pkgtools/pkglint/files/vartype.go:1.43
--- pkgsrc/pkgtools/pkglint/files/vartype.go:1.42 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/vartype.go Fri Dec 13 01:39:23 2019
@@ -351,7 +351,8 @@ func (bt *BasicType) NeedsQ() bool {
BtEmulPlatform,
BtFileMode,
BtFilename,
- BtIdentifier,
+ BtIdentifierDirect,
+ BtIdentifierIndirect,
BtInteger,
BtMachineGnuPlatform,
BtMachinePlatform,
@@ -419,7 +420,8 @@ var (
BtFileMode = &BasicType{"FileMode", (*VartypeCheck).FileMode}
BtGccReqd = &BasicType{"GccReqd", (*VartypeCheck).GccReqd}
BtHomepage = &BasicType{"Homepage", (*VartypeCheck).Homepage}
- BtIdentifier = &BasicType{"Identifier", (*VartypeCheck).Identifier}
+ BtIdentifierDirect = &BasicType{"Identifier", (*VartypeCheck).IdentifierDirect}
+ BtIdentifierIndirect = &BasicType{"Identifier", (*VartypeCheck).IdentifierIndirect}
BtInteger = &BasicType{"Integer", (*VartypeCheck).Integer}
BtLdFlag = &BasicType{"LdFlag", (*VartypeCheck).LdFlag}
BtLicense = &BasicType{"License", (*VartypeCheck).License}
@@ -472,7 +474,6 @@ var (
BtEmulArch = enumFromValues(machineArchValues) // Just a wild guess.
BtMachineGnuPlatformOpsys = BtEmulOpsys
- btCond = &BasicType{".if condition", nil /* never called */}
btForLoop = &BasicType{".for loop", nil /* never called */}
)
Index: pkgsrc/pkgtools/pkglint/files/mkassignchecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mkassignchecker.go:1.2 pkgsrc/pkgtools/pkglint/files/mkassignchecker.go:1.3
--- pkgsrc/pkgtools/pkglint/files/mkassignchecker.go:1.2 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/mkassignchecker.go Fri Dec 13 01:39:23 2019
@@ -547,7 +547,7 @@ func (ck *MkAssignChecker) checkVarassig
}
mkline := ck.MkLine
- atoms := NewShTokenizer(mkline.Line, mkline.Value(), false).ShAtoms()
+ atoms := NewShTokenizer(mkline.Line, mkline.Value()).ShAtoms()
for i, atom := range atoms {
if varuse := atom.VarUse(); varuse != nil {
wordPart := isWordPart(atoms, i)
Index: pkgsrc/pkgtools/pkglint/files/mklexer.go
diff -u pkgsrc/pkgtools/pkglint/files/mklexer.go:1.5 pkgsrc/pkgtools/pkglint/files/mklexer.go:1.6
--- pkgsrc/pkgtools/pkglint/files/mklexer.go:1.5 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/mklexer.go Fri Dec 13 01:39:23 2019
@@ -39,23 +39,30 @@ func (p *MkLexer) MkTokens() ([]*MkToken
var tokens []*MkToken
for !lexer.EOF() {
- mark := lexer.Mark()
- if varuse := p.VarUse(); varuse != nil {
- tokens = append(tokens, &MkToken{Text: lexer.Since(mark), Varuse: varuse})
- continue
+ token := p.MkToken()
+ if token == nil {
+ break
}
+ tokens = append(tokens, token)
+ }
+ return tokens, lexer.Rest()
+}
- for lexer.NextBytesFunc(func(b byte) bool { return b != '$' }) != "" || lexer.SkipString("$$") {
- }
- text := lexer.Since(mark)
- if text != "" {
- tokens = append(tokens, &MkToken{Text: text})
- continue
- }
+func (p *MkLexer) MkToken() *MkToken {
+ lexer := p.lexer
- break
+ mark := lexer.Mark()
+ if varuse := p.VarUse(); varuse != nil {
+ return &MkToken{Text: lexer.Since(mark), Varuse: varuse}
}
- return tokens, lexer.Rest()
+
+ for lexer.NextBytesFunc(func(b byte) bool { return b != '$' }) != "" || lexer.SkipString("$$") {
+ }
+ text := lexer.Since(mark)
+ if text != "" {
+ return &MkToken{Text: text}
+ }
+ return nil
}
// VarUse parses a variable expression like ${VAR}, $@, ${VAR:Mpattern:Ox}.
Index: pkgsrc/pkgtools/pkglint/files/vargroups_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vargroups_test.go:1.5 pkgsrc/pkgtools/pkglint/files/vargroups_test.go:1.6
--- pkgsrc/pkgtools/pkglint/files/vargroups_test.go:1.5 Sun Dec 8 00:06:38 2019
+++ pkgsrc/pkgtools/pkglint/files/vargroups_test.go Fri Dec 13 01:39:23 2019
@@ -69,7 +69,7 @@ func (s *Suite) Test_VargroupsChecker__v
t.CheckOutputLines(
"WARN: Makefile:7: VAR.param is defined but not used.",
- // FIXME: Hmmm, that's going to be complicated to get right.
+ // TODO: Hmmm, that's going to be complicated to get right.
"WARN: Makefile:7: Variable VAR.param is defined but not mentioned in the _VARGROUPS section.")
}
Index: pkgsrc/pkgtools/pkglint/files/mklexer_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklexer_test.go:1.4 pkgsrc/pkgtools/pkglint/files/mklexer_test.go:1.5
--- pkgsrc/pkgtools/pkglint/files/mklexer_test.go:1.4 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/mklexer_test.go Fri Dec 13 01:39:23 2019
@@ -101,31 +101,52 @@ func (s *Suite) Test_MkLexer_MkTokens(c
"")
}
+func (s *Suite) Test_MkLexer_MkToken(c *check.C) {
+ t := s.Init(c)
+
+ test := func(input string, expectedToken *MkToken, expectedRest string, diagnostics ...string) {
+ lexer := NewMkLexer(input, t.NewLine("Test_MkLexer_VarUse.mk", 1, ""))
+ actualToken := lexer.MkToken()
+ rest := lexer.Rest()
+
+ t.CheckDeepEquals(actualToken, expectedToken)
+ t.CheckEquals(rest, expectedRest)
+ t.CheckOutput(diagnostics)
+ }
+
+ test("${VARIABLE}rest",
+ &MkToken{"${VARIABLE}", NewMkVarUse("VARIABLE")}, "rest")
+
+ test("$@rest",
+ &MkToken{"$@", NewMkVarUse("@")}, "rest")
+
+ test("text$$",
+ &MkToken{"text$$", nil}, "")
+
+ test("text$$${REST}",
+ &MkToken{"text$$", nil}, "${REST}")
+
+ test("",
+ nil, "")
+}
+
func (s *Suite) Test_MkLexer_VarUse(c *check.C) {
t := s.Init(c)
b := NewMkTokenBuilder()
varuse := b.VaruseToken
varuseText := b.VaruseTextToken
- // FIXME: This function does much more than necessary to test VarUse.
- testRest := func(input string, expectedTokens []*MkToken, expectedRest string, diagnostics ...string) {
- line := t.NewLines("Test_MkLexer_VarUse.mk", input).Lines[0]
- p := NewMkLexer(input, line)
-
- actualTokens, rest := p.MkTokens()
+ testRest := func(input string, expectedToken *MkToken, expectedRest string, diagnostics ...string) {
+ lexer := NewMkLexer(input, t.NewLine("Test_MkLexer_VarUse.mk", 1, ""))
+ actualToken := lexer.MkToken()
+ rest := lexer.Rest()
- t.CheckDeepEquals(actualTokens, expectedTokens)
- for i, expectedToken := range expectedTokens {
- if i < len(actualTokens) {
- t.CheckDeepEquals(*actualTokens[i], *expectedToken)
- t.CheckDeepEquals(actualTokens[i].Varuse, expectedToken.Varuse)
- }
- }
+ t.CheckDeepEquals(actualToken, expectedToken)
t.CheckEquals(rest, expectedRest)
t.CheckOutput(diagnostics)
}
test := func(input string, expectedToken *MkToken, diagnostics ...string) {
- testRest(input, b.Tokens(expectedToken), "", diagnostics...)
+ testRest(input, expectedToken, "", diagnostics...)
}
t.Use(testRest, test, varuse, varuseText)
@@ -597,7 +618,7 @@ func (s *Suite) Test_MkLexer_varUseModif
mod := p.varUseModifier("VAR", '}')
t.CheckEquals(mod, "")
- // FIXME: The "S," has just disappeared.
+ // XXX: The "S," has just disappeared.
t.CheckEquals(p.Rest(), "}")
t.CheckOutputLines(
Index: pkgsrc/pkgtools/pkglint/files/mkline.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline.go:1.70 pkgsrc/pkgtools/pkglint/files/mkline.go:1.71
--- pkgsrc/pkgtools/pkglint/files/mkline.go:1.70 Mon Dec 9 20:38:16 2019
+++ pkgsrc/pkgtools/pkglint/files/mkline.go Fri Dec 13 01:39:23 2019
@@ -75,7 +75,9 @@ func (mkline *MkLine) String() string {
func (mkline *MkLine) HasComment() bool { return mkline.splitResult.hasComment }
-func (mkline *MkLine) HasRationale() bool { return mkline.splitResult.hasRationale }
+func (mkline *MkLine) HasRationale() bool { return mkline.splitResult.rationale != "" }
+
+func (mkline *MkLine) Rationale() string { return mkline.splitResult.rationale }
// Comment returns the comment after the first unescaped #.
//
Index: pkgsrc/pkgtools/pkglint/files/mklineparser.go
diff -u pkgsrc/pkgtools/pkglint/files/mklineparser.go:1.8 pkgsrc/pkgtools/pkglint/files/mklineparser.go:1.9
--- pkgsrc/pkgtools/pkglint/files/mklineparser.go:1.8 Mon Dec 9 20:38:16 2019
+++ pkgsrc/pkgtools/pkglint/files/mklineparser.go Fri Dec 13 01:39:23 2019
@@ -372,7 +372,7 @@ func (MkLineParser) split(diag Autofixer
}
}
- return mkLineSplitResult{mainTrimmed, tokens, spaceBeforeComment, hasComment, false, comment}
+ return mkLineSplitResult{mainTrimmed, tokens, spaceBeforeComment, hasComment, "", comment}
}
// unescapeComment takes a Makefile line, as written in a file, and splits
@@ -463,6 +463,6 @@ type mkLineSplitResult struct {
tokens []*MkToken
spaceBeforeComment string
hasComment bool
- hasRationale bool // filled in later, by MkLines.collectRationale
+ rationale string // 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.8 pkgsrc/pkgtools/pkglint/files/mklineparser_test.go:1.9
--- pkgsrc/pkgtools/pkglint/files/mklineparser_test.go:1.8 Mon Dec 9 20:38:16 2019
+++ pkgsrc/pkgtools/pkglint/files/mklineparser_test.go Fri Dec 13 01:39:23 2019
@@ -1041,7 +1041,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.64 pkgsrc/pkgtools/pkglint/files/mklines.go:1.65
--- pkgsrc/pkgtools/pkglint/files/mklines.go:1.64 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/mklines.go Fri Dec 13 01:39:23 2019
@@ -103,11 +103,24 @@ func (mklines *MkLines) collectRationale
return mkline.IsComment() && !mkline.IsCommentedVarassign()
}
- rationale := false
+ var rat strings.Builder
for _, mkline := range mklines.mklines {
- rationale = rationale || isRealComment(mkline) && isUseful(mkline)
- mkline.splitResult.hasRationale = rationale || isUseful(mkline)
- rationale = rationale && !mkline.IsEmpty()
+ if isRealComment(mkline) && isUseful(mkline) {
+ rat.WriteString(mkline.Comment())
+ rat.WriteString("\n")
+ }
+
+ var lineRat strings.Builder
+ lineRat.WriteString(rat.String())
+ if isUseful(mkline) {
+ lineRat.WriteString(mkline.Comment())
+ lineRat.WriteString("\n")
+ }
+
+ mkline.splitResult.rationale = lineRat.String()
+ if mkline.IsEmpty() {
+ rat.Reset()
+ }
}
}
Index: pkgsrc/pkgtools/pkglint/files/package_test.go
diff -u pkgsrc/pkgtools/pkglint/files/package_test.go:1.64 pkgsrc/pkgtools/pkglint/files/package_test.go:1.65
--- pkgsrc/pkgtools/pkglint/files/package_test.go:1.64 Mon Dec 9 20:38:16 2019
+++ pkgsrc/pkgtools/pkglint/files/package_test.go Fri Dec 13 01:39:23 2019
@@ -1484,7 +1484,6 @@ func (s *Suite) Test_Package_checkfilePa
func (s *Suite) Test_Package_checkfilePackageMakefile__prefs_indirect(c *check.C) {
t := s.Init(c)
- // FIXME: remove t.SetUpOption("option", "An example option")
t.SetUpPackage("category/package",
".if ${OPSYS} == NetBSD", // 20: OPSYS is not yet defined here.
".endif", // 21
Index: pkgsrc/pkgtools/pkglint/files/mklines_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.56 pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.57
--- pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.56 Mon Dec 9 20:38:16 2019
+++ pkgsrc/pkgtools/pkglint/files/mklines_test.go Fri Dec 13 01:39:23 2019
@@ -411,9 +411,7 @@ func (s *Suite) Test_MkLines_Check__inco
mklines.Check()
t.CheckOutputLines(
- "WARN: subst.mk:EOF: Incomplete SUBST block: SUBST_STAGE.class missing.",
- "WARN: subst.mk:EOF: Incomplete SUBST block: SUBST_FILES.class missing.",
- "WARN: subst.mk:EOF: Incomplete SUBST block: SUBST_SED.class, SUBST_VARS.class or SUBST_FILTER_CMD.class missing.")
+ "WARN: subst.mk:EOF: Missing SUBST block for \"class\".")
}
func (s *Suite) Test_MkLines_collectRationale(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/mkshparser.go
diff -u pkgsrc/pkgtools/pkglint/files/mkshparser.go:1.18 pkgsrc/pkgtools/pkglint/files/mkshparser.go:1.19
--- pkgsrc/pkgtools/pkglint/files/mkshparser.go:1.18 Wed Nov 27 22:10:07 2019
+++ pkgsrc/pkgtools/pkglint/files/mkshparser.go Fri Dec 13 01:39:23 2019
@@ -238,7 +238,7 @@ func (lex *ShellLexer) Lex(lval *shyySym
lex.atCommandStart = false
case lex.atCommandStart && matches(token, `^[A-Za-z_]\w*=`):
ttype = tkASSIGNMENT_WORD
- p := NewShTokenizer(nil, token, false)
+ p := NewShTokenizer(nil, token)
lval.Word = p.ShToken()
case hasPrefix(token, "#"):
// This doesn't work for multiline shell programs.
@@ -246,7 +246,7 @@ func (lex *ShellLexer) Lex(lval *shyySym
return 0
default:
ttype = tkWORD
- p := NewShTokenizer(nil, token, false)
+ p := NewShTokenizer(nil, token)
lval.Word = p.ShToken()
lex.atCommandStart = false
Index: pkgsrc/pkgtools/pkglint/files/mkshparser_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkshparser_test.go:1.22 pkgsrc/pkgtools/pkglint/files/mkshparser_test.go:1.23
--- pkgsrc/pkgtools/pkglint/files/mkshparser_test.go:1.22 Mon Dec 2 23:32:09 2019
+++ pkgsrc/pkgtools/pkglint/files/mkshparser_test.go Fri Dec 13 01:39:23 2019
@@ -1007,7 +1007,7 @@ func (b *MkShBuilder) Redirected(cmd *Mk
func (b *MkShBuilder) Token(mktext string) *ShToken {
line := NewLine("MkShBuilder.Token.mk", 1, "", &RawLine{1, "\n", "\n"})
- tokenizer := NewShTokenizer(line, mktext, false)
+ tokenizer := NewShTokenizer(line, mktext)
token := tokenizer.ShToken()
assertf(tokenizer.parser.EOF(), "Invalid token: %q", tokenizer.parser.Rest())
return token
Index: pkgsrc/pkgtools/pkglint/files/mktypes.go
diff -u pkgsrc/pkgtools/pkglint/files/mktypes.go:1.22 pkgsrc/pkgtools/pkglint/files/mktypes.go:1.23
--- pkgsrc/pkgtools/pkglint/files/mktypes.go:1.22 Sun Dec 8 00:06:38 2019
+++ pkgsrc/pkgtools/pkglint/files/mktypes.go Fri Dec 13 01:39:23 2019
@@ -1,6 +1,10 @@
package pkglint
-import "strings"
+import (
+ "netbsd.org/pkglint/regex"
+ "regexp"
+ "strings"
+)
// MkToken represents a contiguous string from a Makefile.
// It is either a literal string or a variable use.
@@ -84,11 +88,31 @@ func (m MkVarUseModifier) Subst(str stri
return "", false
}
- result := mkopSubst(str, leftAnchor, from, rightAnchor, to, options)
- if trace.Tracing && result != str {
+ ok, result := m.EvalSubst(str, leftAnchor, from, rightAnchor, to, options)
+ if trace.Tracing && ok && result != str {
trace.Stepf("Subst: %q %q => %q", str, m.Text, result)
}
- return result, true
+ return result, ok
+}
+
+// mkopSubst evaluates make(1)'s :S substitution operator.
+// It does not resolve any variables.
+func (MkVarUseModifier) EvalSubst(s string, left bool, from string, right bool, to string, flags string) (ok bool, result string) {
+
+ if containsVarRefLong(from) || containsVarRefLong(to) {
+ return false, ""
+ }
+
+ re := regex.Pattern(condStr(left, "^", "") + regexp.QuoteMeta(from) + condStr(right, "$", ""))
+ done := false
+ gflag := contains(flags, "g")
+ return true, replaceAllFunc(s, re, func(match string) string {
+ if gflag || !done {
+ done = !gflag
+ return to
+ }
+ return match
+ })
}
// MatchMatch tries to match the modifier to a :M or a :N pattern matching.
Index: pkgsrc/pkgtools/pkglint/files/mktypes_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mktypes_test.go:1.20 pkgsrc/pkgtools/pkglint/files/mktypes_test.go:1.21
--- pkgsrc/pkgtools/pkglint/files/mktypes_test.go:1.20 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/mktypes_test.go Fri Dec 13 01:39:23 2019
@@ -155,6 +155,48 @@ func (s *Suite) Test_MkVarUseModifier_Su
t.CheckEquals(result, "The target")
}
+func (s *Suite) Test_MkVarUseModifier_EvalSubst(c *check.C) {
+ t := s.Init(c)
+
+ test := func(s string, left bool, from string, right bool, to string, flags string, ok bool, result string) {
+ mod := MkVarUseModifier{}
+
+ actualOk, actual := mod.EvalSubst(s, left, from, right, to, flags)
+
+ t.CheckEquals(actualOk, ok)
+ t.CheckEquals(actual, result)
+ }
+
+ // Replace anywhere
+ test("pkgname", false, "kgna", false, "ri", "", true, "prime")
+ test("pkgname", false, "pkgname", false, "replacement", "", true, "replacement")
+ test("aaaaaaa", false, "a", false, "b", "", true, "baaaaaa")
+
+ // Anchored at the beginning
+ test("pkgname", true, "kgna", false, "ri", "", true, "pkgname")
+ test("pkgname", true, "pkgname", false, "replacement", "", true, "replacement")
+
+ // Anchored at the end
+ test("pkgname", false, "kgna", true, "ri", "", true, "pkgname")
+ test("pkgname", false, "pkgname", true, "replacement", "", true, "replacement")
+
+ // Anchored at both sides
+ test("pkgname", true, "kgna", true, "ri", "", true, "pkgname")
+ test("pkgname", false, "pkgname", false, "replacement", "", true, "replacement")
+
+ // Replace all
+ test("aaaaa", false, "a", false, "b", "g", true, "bbbbb")
+ test("aaaaa", true, "a", false, "b", "g", true, "baaaa")
+ test("aaaaa", false, "a", true, "b", "g", true, "aaaab")
+ test("aaaaa", true, "a", true, "b", "g", true, "aaaaa")
+
+ // Replacements using variables are trickier to get right.
+ test("anything", false, "${VAR}", false, "replacement", "", false, "")
+ test("anything", false, "pattern", false, "${VAR}", "", false, "")
+ test("echo $$$$", false, "$$", false, "dollar", "", true, "echo dollar$$")
+ test("echo $$$$", false, "$$", false, "dollar", "g", true, "echo dollardollar")
+}
+
func (s *Suite) Test_MkVarUseModifier_MatchMatch(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/path.go
diff -u pkgsrc/pkgtools/pkglint/files/path.go:1.6 pkgsrc/pkgtools/pkglint/files/path.go:1.7
--- pkgsrc/pkgtools/pkglint/files/path.go:1.6 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/path.go Fri Dec 13 01:39:23 2019
@@ -116,7 +116,7 @@ func (p Path) ContainsText(contained str
// ContainsPath returns whether the sub path is part of the path.
// The basic unit of comparison is a path component, not a character.
//
-// Note that the paths used in pkglint may contains seemingly unnecessary
+// Note that the paths used in pkglint may contain seemingly unnecessary
// components, like "../../wip/mk/../../devel/gettext-lib". To ignore these
// components, use ContainsPathCanonical instead.
func (p Path) ContainsPath(sub Path) bool {
Index: pkgsrc/pkgtools/pkglint/files/pkglint.1
diff -u pkgsrc/pkgtools/pkglint/files/pkglint.1:1.62 pkgsrc/pkgtools/pkglint/files/pkglint.1:1.63
--- pkgsrc/pkgtools/pkglint/files/pkglint.1:1.62 Mon Dec 9 20:38:16 2019
+++ pkgsrc/pkgtools/pkglint/files/pkglint.1 Fri Dec 13 01:39:23 2019
@@ -1,4 +1,4 @@
-.\" $NetBSD: pkglint.1,v 1.62 2019/12/09 20:38:16 rillig Exp $
+.\" $NetBSD: pkglint.1,v 1.63 2019/12/13 01:39:23 rillig Exp $
.\" From FreeBSD: portlint.1,v 1.8 1997/11/25 14:53:14 itojun Exp
.\"
.\" Copyright (c) 1997 by Jun-ichiro Itoh <itojun%itojun.org@localhost>.
Index: pkgsrc/pkgtools/pkglint/files/shell_test.go
diff -u pkgsrc/pkgtools/pkglint/files/shell_test.go:1.62 pkgsrc/pkgtools/pkglint/files/shell_test.go:1.63
--- pkgsrc/pkgtools/pkglint/files/shell_test.go:1.62 Mon Dec 9 20:38:16 2019
+++ pkgsrc/pkgtools/pkglint/files/shell_test.go Fri Dec 13 01:39:23 2019
@@ -130,22 +130,30 @@ func (s *Suite) Test_SimpleCommandChecke
func (s *Suite) Test_SimpleCommandChecker_handleCommandVariable(c *check.C) {
t := s.Init(c)
- t.SetUpTool("perl", "PERL5", AtRunTime)
- t.SetUpTool("perl6", "PERL6", Nowhere)
+ t.SetUpTool("runtime", "RUNTIME", AtRunTime)
+ t.SetUpTool("nowhere", "NOWHERE", Nowhere)
mklines := t.NewMkLines("Makefile",
MkCvsID,
"",
- "PERL5_VARS_CMD=\t${PERL5:Q}",
- "PERL5_VARS_CMD=\t${PERL6:Q}",
+ "RUNTIME_Q_CMD=\t${RUNTIME:Q}",
+ "NOWHERE_Q_CMD=\t${NOWHERE:Q}",
+ "RUNTIME_CMD=\t${RUNTIME}",
+ "NOWHERE_CMD=\t${NOWHERE}",
"",
"pre-configure:",
- "\t${PERL5_VARS_CMD} -e 'print 12345'")
+ "\t: ${RUNTIME_Q_CMD} ${NOWHERE_Q_CMD}",
+ "\t: ${RUNTIME_CMD} ${NOWHERE_CMD}")
mklines.Check()
- // FIXME: In PERL5:Q and PERL6:Q, the :Q is wrong.
+ // A tool that appears as the name of a shell command is exactly
+ // intended to be used without quotes, so that its possible
+ // command line options are treated as separate arguments.
+ //
+ // TODO: Add a warning that in lines 3 and 4, the :Q is wrong.
t.CheckOutputLines(
- "WARN: Makefile:4: The \"${PERL6:Q}\" tool is used but not added to USE_TOOLS.")
+ "WARN: Makefile:4: The \"${NOWHERE:Q}\" tool is used but not added to USE_TOOLS.",
+ "WARN: Makefile:6: The \"${NOWHERE}\" tool is used but not added to USE_TOOLS.")
}
func (s *Suite) Test_SimpleCommandChecker_handleCommandVariable__parameterized(c *check.C) {
@@ -1485,7 +1493,7 @@ func (s *Suite) Test_ShellLineChecker_Ch
ck.CheckWord(ck.mkline.ShellCommand(), false, RunTime)
- // FIXME: Should be parsed correctly. Make passes the dollar through (probably),
+ // XXX: Should be parsed correctly. Make passes the dollar through (probably),
// and the shell parser should complain about the unfinished string literal.
t.CheckOutputLines(
"WARN: filename.mk:1: Internal pkglint error in ShTokenizer.ShAtom at \"$\" (quoting=s).",
@@ -1649,7 +1657,7 @@ func (s *Suite) Test_ShellLineChecker_un
test := func(input string, expectedOutput string, expectedRest string, diagnostics ...string) {
ck := t.NewShellLineChecker("# dummy")
- tok := NewShTokenizer(nil, input, false)
+ tok := NewShTokenizer(nil, input)
atoms := tok.ShAtoms()
// Set up the correct quoting mode for the test by skipping
@@ -1765,7 +1773,7 @@ func (s *Suite) Test_ShellLineChecker_ch
mklines.Check()
- // FIXME: It is inconsistent that the check for unquoted shell
+ // XXX: It is inconsistent that the check for unquoted shell
// variables is enabled for CONFIGURE_ARGS (where shell variables
// don't make sense at all) but not for real shell commands.
t.CheckOutputLines(
Index: pkgsrc/pkgtools/pkglint/files/pkgsrc.go
diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.47 pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.48
--- pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.47 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/pkgsrc.go Fri Dec 13 01:39:23 2019
@@ -494,7 +494,6 @@ func (src *Pkgsrc) initDeprecatedVars()
"NO_WRKSUBDIR": "Use WRKSRC=${WRKDIR} instead.",
"PATCH_SITE_SUBDIR": "Use some form of PATCHES_SITES instead.",
"PATCH_SUM_FILE": "Use DISTINFO_FILE instead.",
- "PKG_JVM": "Use PKG_DEFAULT_JVM instead.",
"USE_BUILDLINK2": "You can just remove it.",
"USE_BUILDLINK3": "You can just remove it.",
"USE_CANNA": "Use the PKG_OPTIONS framework instead.",
@@ -601,9 +600,6 @@ func (src *Pkgsrc) initDeprecatedVars()
"_PKG_DEBUG": "Use RUN (with more error checking) instead.",
"LICENCE": "Use LICENSE instead.",
- // November 2007
- // USE_NCURSES: Include "../../devel/ncurses/buildlink3.mk" instead.
-
// December 2007
"INSTALLATION_DIRS_FROM_PLIST": "Use AUTO_MKDIRS instead.",
Index: pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go
diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.40 pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.41
--- pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.40 Sun Dec 8 00:06:38 2019
+++ pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go Fri Dec 13 01:39:23 2019
@@ -790,9 +790,7 @@ func (s *Suite) Test_Pkgsrc_initDeprecat
"WARN: Makefile:2: Definition of USE_PERL5 is deprecated. "+
"Use USE_TOOLS+=perl or USE_TOOLS+=perl:run instead.",
"WARN: Makefile:3: Definition of SUBST_POSTCMD.class is deprecated. "+
- "Has been removed, as it seemed unused.",
- "WARN: Makefile:4: Use of \"PKG_JVM\" is deprecated. "+
- "Use PKG_DEFAULT_JVM instead.")
+ "Has been removed, as it seemed unused.")
}
func (s *Suite) Test_Pkgsrc_loadUntypedVars(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/redundantscope_test.go
diff -u pkgsrc/pkgtools/pkglint/files/redundantscope_test.go:1.10 pkgsrc/pkgtools/pkglint/files/redundantscope_test.go:1.11
--- pkgsrc/pkgtools/pkglint/files/redundantscope_test.go:1.10 Sun Dec 8 00:06:38 2019
+++ pkgsrc/pkgtools/pkglint/files/redundantscope_test.go Fri Dec 13 01:39:23 2019
@@ -840,7 +840,7 @@ func (s *Suite) Test_RedundantScope__bra
t.CheckOutputEmpty()
}
-// FIXME: Continue the systematic redundancy tests.
+// TODO: Continue the systematic redundancy tests.
//
// Tests where the variables are defined in a .for loop that might not be
// evaluated at all.
Index: pkgsrc/pkgtools/pkglint/files/shtypes_test.go
diff -u pkgsrc/pkgtools/pkglint/files/shtypes_test.go:1.10 pkgsrc/pkgtools/pkglint/files/shtypes_test.go:1.11
--- pkgsrc/pkgtools/pkglint/files/shtypes_test.go:1.10 Wed Nov 27 22:10:07 2019
+++ pkgsrc/pkgtools/pkglint/files/shtypes_test.go Fri Dec 13 01:39:23 2019
@@ -18,7 +18,7 @@ func (s *Suite) Test_ShAtom_String(c *ch
t := s.Init(c)
line := t.NewLine("filename.mk", 1, "")
- tokenizer := NewShTokenizer(line, "${ECHO} \"hello, world\"", false)
+ tokenizer := NewShTokenizer(line, "${ECHO} \"hello, world\"")
atoms := tokenizer.ShAtoms()
@@ -47,7 +47,7 @@ func (s *Suite) Test_ShToken_String(c *c
t := s.Init(c)
line := t.NewLine("filename.mk", 1, "")
- tokenizer := NewShTokenizer(line, "${ECHO} \"hello, world\"", false)
+ tokenizer := NewShTokenizer(line, "${ECHO} \"hello, world\"")
t.CheckEquals(tokenizer.ShToken().String(), "ShToken([varuse(\"ECHO\")])")
t.CheckEquals(tokenizer.ShToken().String(), "ShToken([ShAtom(text, \"\\\"\", d) ShAtom(text, \"hello, world\", d) \"\\\"\"])")
Index: pkgsrc/pkgtools/pkglint/files/shell.go
diff -u pkgsrc/pkgtools/pkglint/files/shell.go:1.54 pkgsrc/pkgtools/pkglint/files/shell.go:1.55
--- pkgsrc/pkgtools/pkglint/files/shell.go:1.54 Mon Dec 9 20:38:16 2019
+++ pkgsrc/pkgtools/pkglint/files/shell.go Fri Dec 13 01:39:23 2019
@@ -793,7 +793,7 @@ func (ck *ShellLineChecker) CheckWord(to
}
func (ck *ShellLineChecker) checkWordQuoting(token string, checkQuoting bool, time ToolTime) {
- tok := NewShTokenizer(ck.mkline.Line, token, true)
+ tok := NewShTokenizer(ck.mkline.Line, token)
atoms := tok.ShAtoms()
quoting := shqPlain
@@ -1092,7 +1092,7 @@ func splitIntoShellTokens(line *Line, te
// TODO: Check whether this function is used correctly by all callers.
// It may be better to use a proper shell parser instead of this tokenizer.
- p := NewShTokenizer(line, text, false)
+ p := NewShTokenizer(line, text)
for {
token := p.ShToken()
if token == nil {
Index: pkgsrc/pkgtools/pkglint/files/shtokenizer.go
diff -u pkgsrc/pkgtools/pkglint/files/shtokenizer.go:1.23 pkgsrc/pkgtools/pkglint/files/shtokenizer.go:1.24
--- pkgsrc/pkgtools/pkglint/files/shtokenizer.go:1.23 Sun Dec 8 00:06:38 2019
+++ pkgsrc/pkgtools/pkglint/files/shtokenizer.go Fri Dec 13 01:39:23 2019
@@ -7,14 +7,8 @@ type ShTokenizer struct {
inWord bool
}
-func NewShTokenizer(diag Autofixer, text string, emitWarnings bool) *ShTokenizer {
- // TODO: Switching to NewMkParser is nontrivial since emitWarnings must equal (line != nil).
- // assert((line != nil) == emitWarnings)
- if diag != nil {
- emitWarnings = true
- }
- mklex := NewMkLexer(text, diag)
- return &ShTokenizer{mklex, false}
+func NewShTokenizer(diag Autofixer, text string) *ShTokenizer {
+ return &ShTokenizer{NewMkLexer(text, diag), false}
}
// ShAtom parses a basic building block of a shell program.
Index: pkgsrc/pkgtools/pkglint/files/shtokenizer_test.go
diff -u pkgsrc/pkgtools/pkglint/files/shtokenizer_test.go:1.23 pkgsrc/pkgtools/pkglint/files/shtokenizer_test.go:1.24
--- pkgsrc/pkgtools/pkglint/files/shtokenizer_test.go:1.23 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/shtokenizer_test.go Fri Dec 13 01:39:23 2019
@@ -96,7 +96,7 @@ func (s *Suite) Test_ShTokenizer__fuzzin
defer fuzzer.CheckOk()
for i := 0; i < 1000; i++ {
- tokenizer := NewShTokenizer(line, fuzzer.Generate(50), false)
+ tokenizer := NewShTokenizer(line, fuzzer.Generate(50))
tokenizer.ShAtoms()
t.Output() // Discard the output, only react on panics.
}
@@ -110,7 +110,7 @@ func (s *Suite) Test_ShTokenizer_ShAtom(
// atoms, and returns the remaining text.
testRest := func(s string, expectedAtoms []*ShAtom, expectedRest string) {
line := t.NewLine("filename.mk", 1, "")
- p := NewShTokenizer(line, s, false)
+ p := NewShTokenizer(line, s)
actualAtoms := p.ShAtoms()
@@ -540,7 +540,7 @@ func (s *Suite) Test_ShTokenizer_ShAtom_
test := func(input, expectedOutput string) {
line := t.NewLine("filename.mk", 1, "")
- p := NewShTokenizer(line, input, false)
+ p := NewShTokenizer(line, input)
q := shqPlain
result := ""
for {
@@ -580,7 +580,7 @@ func (s *Suite) Test_ShTokenizer_ShAtom_
t := s.Init(c)
line := t.NewLine("filename.mk", 123, "\ttoken")
- tok := NewShTokenizer(line, line.Text, true)
+ tok := NewShTokenizer(line, line.Text)
t.ExpectPanicMatches(
func() { tok.ShAtom(^ShQuoting(0)) },
// Normalize the panic message, for Go < 12 if I remember correctly.
@@ -591,7 +591,7 @@ func (s *Suite) Test_ShTokenizer_shVarUs
t := s.Init(c)
test := func(input string, output *ShAtom, rest string) {
- tok := NewShTokenizer(nil, input, false)
+ tok := NewShTokenizer(nil, input)
actual := tok.shVarUse(shqPlain)
t.CheckDeepEquals(actual, output)
@@ -645,7 +645,7 @@ func (s *Suite) Test_ShTokenizer_ShToken
// tokens, and returns the remaining text.
testRest := func(str string, expected ...string) string {
line := t.NewLine("testRest.mk", 1, "")
- p := NewShTokenizer(line, str, false)
+ p := NewShTokenizer(line, str)
for _, exp := range expected {
t.CheckEquals(p.ShToken().MkText, exp)
}
@@ -654,7 +654,7 @@ func (s *Suite) Test_ShTokenizer_ShToken
test := func(str string, expected ...string) {
line := t.NewLine("test.mk", 1, "")
- p := NewShTokenizer(line, str, false)
+ p := NewShTokenizer(line, str)
for _, exp := range expected {
t.CheckEquals(p.ShToken().MkText, exp)
}
@@ -664,7 +664,7 @@ func (s *Suite) Test_ShTokenizer_ShToken
testNil := func(str string) {
line := t.NewLine("testNil.mk", 1, "")
- p := NewShTokenizer(line, str, false)
+ p := NewShTokenizer(line, str)
c.Check(p.ShToken(), check.IsNil)
t.CheckEquals(p.Rest(), "")
t.CheckOutputEmpty()
Index: pkgsrc/pkgtools/pkglint/files/vartype_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartype_test.go:1.23 pkgsrc/pkgtools/pkglint/files/vartype_test.go:1.24
--- pkgsrc/pkgtools/pkglint/files/vartype_test.go:1.23 Sun Dec 8 00:06:38 2019
+++ pkgsrc/pkgtools/pkglint/files/vartype_test.go Fri Dec 13 01:39:23 2019
@@ -209,7 +209,7 @@ func (s *Suite) Test_BasicType_NeedsQ(c
// Typically safe, seldom used in practice.
test("DISTFILES", false)
- // XXX: BtIdentifier is used for several other purposes
+ test("SUBST_CLASSES", false)
test("PLIST_VARS", false)
test("MAKE_JOBS", false) // XXX: What if MAKE_JOBS is undefined?
Index: pkgsrc/pkgtools/pkglint/files/substcontext.go
diff -u pkgsrc/pkgtools/pkglint/files/substcontext.go:1.32 pkgsrc/pkgtools/pkglint/files/substcontext.go:1.33
--- pkgsrc/pkgtools/pkglint/files/substcontext.go:1.32 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/substcontext.go Fri Dec 13 01:39:23 2019
@@ -5,247 +5,259 @@ import "netbsd.org/pkglint/textproc"
// SubstContext records the state of a block of variable assignments
// that make up a SUBST class (see `mk/subst.mk`).
type SubstContext struct {
- id string
- stage string
- message string
- curr *SubstContextStats
- inAllBranches SubstContextStats
- filterCmd string
- vars map[string]bool
-}
-
-func NewSubstContext() *SubstContext {
- return &SubstContext{curr: &SubstContextStats{}}
-}
+ queuedIds []string
+ id string
+ doneIds map[string]bool
-type SubstContextStats struct {
- seenFiles bool
- seenSed bool
- seenVars bool
- seenTransform bool
- prev *SubstContextStats
-}
+ foreignAllowed map[string]struct{}
+ foreign []*MkLine
-func (st *SubstContextStats) Copy() *SubstContextStats {
- return &SubstContextStats{st.seenFiles, st.seenSed, st.seenVars, st.seenTransform, st}
-}
+ conds []*substCond
-func (st *SubstContextStats) And(other *SubstContextStats) {
- st.seenFiles = st.seenFiles && other.seenFiles
- st.seenSed = st.seenSed && other.seenSed
- st.seenVars = st.seenVars && other.seenVars
- st.seenTransform = st.seenTransform && other.seenTransform
+ once Once
}
-func (st *SubstContextStats) Or(other SubstContextStats) {
- st.seenFiles = st.seenFiles || other.seenFiles
- st.seenSed = st.seenSed || other.seenSed
- st.seenVars = st.seenVars || other.seenVars
- st.seenTransform = st.seenTransform || other.seenTransform
+func NewSubstContext() *SubstContext {
+ ctx := SubstContext{}
+ ctx.reset()
+ return &ctx
}
func (ctx *SubstContext) Process(mkline *MkLine) {
switch {
case mkline.IsEmpty():
- ctx.Finish(mkline)
+ ctx.finishClass(mkline)
case mkline.IsVarassign():
- ctx.Varassign(mkline)
+ ctx.varassign(mkline)
case mkline.IsDirective():
- ctx.Directive(mkline)
+ ctx.directive(mkline)
}
}
-func (ctx *SubstContext) Varassign(mkline *MkLine) {
- if trace.Tracing {
- trace.Stepf("SubstContext.Varassign curr=%v all=%v", ctx.curr, ctx.inAllBranches)
- }
+func (ctx *SubstContext) Finish(diag Diagnoser) {
+ ctx.finishClass(diag)
+ ctx.finishFile(diag)
+}
- varname := mkline.Varname()
+func (ctx *SubstContext) varassign(mkline *MkLine) {
varcanon := mkline.Varcanon()
- varparam := mkline.Varparam()
- op := mkline.Op()
- value := mkline.Value()
if varcanon == "SUBST_CLASSES" || varcanon == "SUBST_CLASSES.*" {
- classes := mkline.ValueFields(value)
- if len(classes) > 1 {
- mkline.Warnf("Please add only one class at a time to SUBST_CLASSES.")
- }
- if ctx.id != "" && ctx.id != classes[0] {
- complete := ctx.IsComplete()
- id := ctx.id
- ctx.Finish(mkline)
- if !complete {
- mkline.Warnf("Subst block %q should be finished before adding the next class to SUBST_CLASSES.", id)
- }
- }
- ctx.id = classes[0]
+ ctx.varassignClasses(mkline)
return
}
- foreign := true
- switch varcanon {
- case
- "SUBST_STAGE.*",
- "SUBST_MESSAGE.*",
- "SUBST_FILES.*",
- "SUBST_SED.*",
- "SUBST_VARS.*",
- "SUBST_FILTER_CMD.*":
- foreign = false
- }
-
- if foreign && ctx.vars[varname] {
- foreign = false
+ if ctx.isForeign(mkline.Varcanon()) {
+ if ctx.isActive() {
+ ctx.rememberForeign(mkline)
+ }
+ return
}
- if foreign {
- if ctx.id != "" {
- mkline.Warnf("Foreign variable %q in SUBST block.", varname)
+ if !ctx.isActive() {
+ if !ctx.varassignOutsideBlock(mkline) {
+ return
}
- return
}
- if ctx.id == "" {
- mkline.Warnf("SUBST_CLASSES should come before the definition of %q.", varname)
- ctx.id = varparam
- }
-
- if hasPrefix(varname, "SUBST_") && varparam != ctx.id {
- if ctx.IsComplete() {
- // XXX: This code sometimes produces weird warnings. See
- // meta-pkgs/xorg/Makefile.common 1.41 for an example.
- ctx.Finish(mkline)
-
- // The following assignment prevents an additional warning,
- // but from a technically viewpoint, it is incorrect.
- ctx.id = varparam
- } else {
- mkline.Warnf("Variable %q does not match SUBST class %q.", varname, ctx.id)
+ if hasPrefix(mkline.Varname(), "SUBST_") && !ctx.isActiveId(mkline.Varparam()) {
+ if !ctx.varassignDifferentClass(mkline) {
return
}
}
switch varcanon {
case "SUBST_STAGE.*":
- ctx.dupString(mkline, &ctx.stage, varname, value)
- if value == "pre-patch" || value == "post-patch" {
- fix := mkline.Autofix()
- fix.Warnf("Substitutions should not happen in the patch phase.")
- fix.Explain(
- "Performing substitutions during post-patch breaks tools such as",
- "mkpatches, making it very difficult to regenerate correct patches",
- "after making changes, and often leading to substituted string",
- "replacements being committed.",
- "",
- "Instead of pre-patch, use post-extract.",
- "Instead of post-patch, use pre-configure.")
- fix.Replace("pre-patch", "post-extract")
- fix.Replace("post-patch", "pre-configure")
- fix.Apply()
- // XXX: Add test that has "SUBST_STAGE.id=pre-patch # or rather post-patch?"
- }
-
- if G.Pkg != nil && (value == "pre-configure" || value == "post-configure") {
- if noConfigureLine := G.Pkg.vars.FirstDefinition("NO_CONFIGURE"); noConfigureLine != nil {
- mkline.Warnf("SUBST_STAGE %s has no effect when NO_CONFIGURE is set (in %s).",
- value, mkline.RelMkLine(noConfigureLine))
- mkline.Explain(
- "To fix this properly, remove the definition of NO_CONFIGURE.")
- }
- }
-
+ ctx.varassignStage(mkline)
case "SUBST_MESSAGE.*":
- ctx.dupString(mkline, &ctx.message, varname, value)
-
+ ctx.varassignMessages(mkline)
case "SUBST_FILES.*":
- ctx.dupBool(mkline, &ctx.curr.seenFiles, varname, op, value)
-
+ ctx.varassignFiles(mkline)
case "SUBST_SED.*":
- ctx.dupBool(mkline, &ctx.curr.seenSed, varname, op, value)
- ctx.curr.seenTransform = true
-
- ctx.suggestSubstVars(mkline)
-
+ ctx.varassignSed(mkline)
case "SUBST_VARS.*":
- ctx.dupBool(mkline, &ctx.curr.seenVars, varname, op, value)
- ctx.curr.seenTransform = true
- for _, substVar := range mkline.Fields() {
- if ctx.vars == nil {
- ctx.vars = make(map[string]bool)
- }
- ctx.vars[substVar] = true
- }
-
+ ctx.varassignVars(mkline)
case "SUBST_FILTER_CMD.*":
- ctx.dupString(mkline, &ctx.filterCmd, varname, value)
- ctx.curr.seenTransform = true
+ ctx.varassignFilterCmd(mkline)
}
}
-func (ctx *SubstContext) Directive(mkline *MkLine) {
- if ctx.id == "" {
+func (ctx *SubstContext) varassignClasses(mkline *MkLine) {
+ classes := mkline.ValueFields(mkline.WithoutMakeVariables(mkline.Value()))
+ if len(classes) == 0 {
return
}
- if trace.Tracing {
- trace.Stepf("+ SubstContext.Directive %v %v", ctx.curr, ctx.inAllBranches)
- }
- dir := mkline.Directive()
- if dir == "if" {
- ctx.inAllBranches = SubstContextStats{true, true, true, true, nil}
+ if len(classes) > 1 {
+ mkline.Notef("Please add only one class at a time to SUBST_CLASSES.")
+ mkline.Explain(
+ "This way, each substitution class forms a block in the package Makefile,",
+ "and to delete this block, it is not necessary to look anywhere else.")
+ }
+ for _, class := range classes {
+ ctx.queue(class)
+ }
+
+ id := classes[0]
+ if ctx.isActive() && !ctx.isActiveId(id) {
+ id := ctx.activeId() // since ctx.condEndif may reset it
+
+ for ctx.isConditional() {
+ // This will be confusing for the outer SUBST block,
+ // but since that block is assumed to be finished,
+ // this doesn't matter.
+ ctx.condEndif(mkline)
+ }
+
+ complete := ctx.isComplete() // since ctx.finishClass will reset it
+ ctx.finishClass(mkline)
+ if !complete {
+ mkline.Warnf("Subst block %q should be finished before adding the next class to SUBST_CLASSES.", id)
+ }
}
- if dir == "elif" || dir == "else" || dir == "endif" {
- if ctx.curr.prev != nil { // Don't crash on malformed input
- ctx.inAllBranches.And(ctx.curr)
- ctx.curr = ctx.curr.prev
+
+ ctx.setActiveId(id)
+
+ return
+}
+
+// varassignOutsideBlock handles variable assignments of SUBST variables that
+// appear without a directly corresponding SUBST block.
+func (ctx *SubstContext) varassignOutsideBlock(mkline *MkLine) (continueWithNewId bool) {
+ varparam := mkline.Varparam()
+
+ if ctx.isListCanon(mkline.Varcanon()) && ctx.isDone(varparam) {
+ if mkline.Op() != opAssignAppend {
+ mkline.Warnf("Late additions to a SUBST variable should use the += operator.")
}
+ return
}
- if dir == "if" || dir == "elif" || dir == "else" {
- ctx.curr = ctx.curr.Copy()
+ if containsWord(mkline.Rationale(), varparam) {
+ return
}
- if dir == "endif" {
- ctx.curr.Or(ctx.inAllBranches)
+
+ if ctx.start(varparam) {
+ return true
}
- if trace.Tracing {
- trace.Stepf("- SubstContext.Directive %v %v", ctx.curr, ctx.inAllBranches)
+
+ if ctx.once.FirstTime(varparam) {
+ mkline.Warnf("Before defining %s, the SUBST class "+
+ "should be declared using \"SUBST_CLASSES+= %s\".",
+ mkline.Varname(), varparam)
}
+ return
}
-func (ctx *SubstContext) IsComplete() bool {
- return ctx.stage != "" && ctx.curr.seenFiles && ctx.curr.seenTransform
+func (ctx *SubstContext) varassignDifferentClass(mkline *MkLine) (ok bool) {
+ varname := mkline.Varname()
+ varparam := mkline.Varparam()
+
+ if !ctx.isComplete() {
+ mkline.Warnf("Variable %q does not match SUBST class %q.", varname, ctx.activeId())
+ return false
+ }
+
+ ctx.finishClass(mkline)
+
+ ctx.start(varparam)
+ return true
}
-func (ctx *SubstContext) Finish(mkline *MkLine) {
- if ctx.id == "" {
- return
+func (ctx *SubstContext) varassignStage(mkline *MkLine) {
+ if ctx.isConditional() {
+ mkline.Warnf("%s should not be defined conditionally.", mkline.Varname())
}
- id := ctx.id
- if ctx.stage == "" {
- mkline.Warnf("Incomplete SUBST block: SUBST_STAGE.%s missing.", id)
+ ctx.dupString(mkline, ssStage)
+
+ value := mkline.Value()
+ if value == "pre-patch" || value == "post-patch" {
+ fix := mkline.Autofix()
+ fix.Warnf("Substitutions should not happen in the patch phase.")
+ fix.Explain(
+ "Performing substitutions during post-patch breaks tools such as",
+ "mkpatches, making it very difficult to regenerate correct patches",
+ "after making changes, and often leading to substituted string",
+ "replacements being committed.",
+ "",
+ "Instead of pre-patch, use post-extract.",
+ "Instead of post-patch, use pre-configure.")
+ fix.Replace("pre-patch", "post-extract")
+ fix.Replace("post-patch", "pre-configure")
+ fix.Apply()
}
- if !ctx.curr.seenFiles {
- mkline.Warnf("Incomplete SUBST block: SUBST_FILES.%s missing.", id)
+
+ if G.Pkg != nil && (value == "pre-configure" || value == "post-configure") {
+ if noConfigureLine := G.Pkg.vars.FirstDefinition("NO_CONFIGURE"); noConfigureLine != nil {
+ mkline.Warnf("SUBST_STAGE %s has no effect when NO_CONFIGURE is set (in %s).",
+ value, mkline.RelMkLine(noConfigureLine))
+ mkline.Explain(
+ "To fix this properly, remove the definition of NO_CONFIGURE.")
+ }
}
- if !ctx.curr.seenTransform {
- mkline.Warnf("Incomplete SUBST block: SUBST_SED.%[1]s, SUBST_VARS.%[1]s or SUBST_FILTER_CMD.%[1]s missing.", id)
+}
+
+func (ctx *SubstContext) varassignMessages(mkline *MkLine) {
+ varname := mkline.Varname()
+
+ if ctx.isConditional() {
+ mkline.Warnf("%s should not be defined conditionally.", varname)
}
- *ctx = *NewSubstContext()
+ ctx.dupString(mkline, ssMessage)
+}
+
+func (ctx *SubstContext) varassignFiles(mkline *MkLine) {
+ ctx.dupList(mkline, ssFiles, ssNone)
}
-func (*SubstContext) dupString(mkline *MkLine, pstr *string, varname, value string) {
- if *pstr != "" {
- mkline.Warnf("Duplicate definition of %q.", varname)
+func (ctx *SubstContext) varassignSed(mkline *MkLine) {
+ ctx.dupList(mkline, ssSed, ssNone)
+ ctx.seen().set(ssTransform)
+
+ ctx.suggestSubstVars(mkline)
+}
+
+func (ctx *SubstContext) varassignVars(mkline *MkLine) {
+ ctx.dupList(mkline, ssVars, ssVarsAutofix)
+ ctx.seen().set(ssTransform)
+
+ for _, substVar := range mkline.Fields() {
+ ctx.allowVar(substVar)
}
- *pstr = value
}
-func (*SubstContext) dupBool(mkline *MkLine, flag *bool, varname string, op MkOperator, value string) {
- if *flag && op != opAssignAppend {
- mkline.Warnf("All but the first %q lines should use the \"+=\" operator.", varname)
+func (ctx *SubstContext) varassignFilterCmd(mkline *MkLine) {
+ ctx.dupString(mkline, ssFilterCmd)
+ ctx.seen().set(ssTransform)
+}
+
+func (ctx *SubstContext) dupList(mkline *MkLine, part substSeen, autofixPart substSeen) {
+ if ctx.seenInBranch(part) && mkline.Op() != opAssignAppend {
+ ctx.fixOperatorAppend(mkline, ctx.seenInBranch(autofixPart))
}
- *flag = true
+ ctx.seen().set(part)
+}
+
+func (ctx *SubstContext) dupString(mkline *MkLine, part substSeen) {
+ if ctx.seenInBranch(part) {
+ mkline.Warnf("Duplicate definition of %q.", mkline.Varname())
+ }
+ ctx.seen().set(part)
+}
+
+func (ctx *SubstContext) fixOperatorAppend(mkline *MkLine, dueToAutofix bool) {
+ before := mkline.ValueAlign()
+ after := alignWith(mkline.Varname()+"+=", before)
+
+ fix := mkline.Autofix()
+ if dueToAutofix {
+ fix.Notef(SilentAutofixFormat)
+ } else {
+ fix.Warnf("All but the first assignment to %q should use the \"+=\" operator.",
+ mkline.Varname())
+ }
+ fix.Replace(before, after)
+ fix.Apply()
}
func (ctx *SubstContext) suggestSubstVars(mkline *MkLine) {
@@ -257,10 +269,11 @@ func (ctx *SubstContext) suggestSubstVar
continue
}
+ id := ctx.activeId()
varop := sprintf("SUBST_VARS.%s%s%s",
- ctx.id,
- condStr(hasSuffix(ctx.id, "+"), " ", ""),
- condStr(ctx.curr.seenVars, "+=", "="))
+ id,
+ condStr(hasSuffix(id, "+"), " ", ""),
+ condStr(ctx.seenInBranch(ssVars), "+=", "="))
fix := mkline.Autofix()
fix.Notef("The substitution command %q can be replaced with \"%s %s\".",
@@ -273,7 +286,12 @@ func (ctx *SubstContext) suggestSubstVar
}
fix.Apply()
- ctx.curr.seenVars = true
+ // At this point the number of SUBST_SED assignments is one
+ // less than before. Therefore it is possible to adjust the
+ // assignment operators on them. It's probably not worth the
+ // effort, though.
+
+ ctx.seen().set(ssVars | ssVarsAutofix)
}
}
@@ -318,3 +336,289 @@ func (*SubstContext) extractVarname(toke
return varname
}
+
+func (*SubstContext) isForeign(varcanon string) bool {
+ switch varcanon {
+ case
+ "SUBST_STAGE.*",
+ "SUBST_MESSAGE.*",
+ "SUBST_FILES.*",
+ "SUBST_SED.*",
+ "SUBST_VARS.*",
+ "SUBST_FILTER_CMD.*":
+ return false
+ }
+ return true
+}
+
+func (*SubstContext) isListCanon(varcanon string) bool {
+ switch varcanon {
+ case
+ "SUBST_FILES.*",
+ "SUBST_SED.*",
+ "SUBST_VARS.*":
+ return true
+ }
+ return false
+}
+
+func (ctx *SubstContext) directive(mkline *MkLine) {
+ dir := mkline.Directive()
+ switch dir {
+ case "if":
+ ctx.condIf()
+
+ case "elif", "else":
+ ctx.condElse(mkline, dir)
+
+ case "endif":
+ ctx.condEndif(mkline)
+ }
+}
+
+func (ctx *SubstContext) condIf() {
+ top := substCond{total: ssAll}
+ ctx.conds = append(ctx.conds, &top)
+}
+
+func (ctx *SubstContext) condElse(mkline *MkLine, dir string) {
+ top := ctx.cond()
+ top.total.retain(top.curr)
+ if !ctx.isConditional() {
+ ctx.finishClass(mkline)
+ }
+ top.curr = ssNone
+ top.seenElse = dir == "else"
+}
+
+func (ctx *SubstContext) condEndif(diag Diagnoser) {
+ top := ctx.cond()
+ top.total.retain(top.curr)
+ if !ctx.isConditional() {
+ ctx.finishClass(diag)
+ }
+ if !top.seenElse {
+ top.total = ssNone
+ }
+ if len(ctx.conds) > 1 {
+ ctx.conds = ctx.conds[:len(ctx.conds)-1]
+ }
+ ctx.seen().union(top.total)
+}
+
+func (ctx *SubstContext) finishClass(diag Diagnoser) {
+ if !ctx.isActive() {
+ return
+ }
+
+ if ctx.seen().get(ssAll) {
+ ctx.checkBlockComplete(diag)
+ ctx.checkForeignVariables()
+ } else {
+ ctx.markAsNotDone()
+ }
+
+ ctx.reset()
+}
+
+func (ctx *SubstContext) checkBlockComplete(diag Diagnoser) {
+ id := ctx.activeId()
+ seen := ctx.seen()
+ if !seen.get(ssStage) {
+ diag.Warnf("Incomplete SUBST block: SUBST_STAGE.%s missing.", id)
+ }
+ if !seen.get(ssFiles) {
+ diag.Warnf("Incomplete SUBST block: SUBST_FILES.%s missing.", id)
+ }
+ if !seen.get(ssTransform) {
+ diag.Warnf("Incomplete SUBST block: SUBST_SED.%[1]s, SUBST_VARS.%[1]s or SUBST_FILTER_CMD.%[1]s missing.", id)
+ }
+}
+
+func (ctx *SubstContext) checkForeignVariables() {
+ ctx.forEachForeignVar(func(mkline *MkLine) {
+ mkline.Warnf("Foreign variable %q in SUBST block.", mkline.Varname())
+ })
+}
+
+func (ctx *SubstContext) finishFile(diag Diagnoser) {
+ for _, id := range ctx.queuedIds {
+ if id != "" && !ctx.isDone(id) {
+ ctx.warnUndefinedBlock(diag, id)
+ }
+ }
+}
+
+func (*SubstContext) warnUndefinedBlock(diag Diagnoser, id string) {
+ diag.Warnf("Missing SUBST block for %q.", id)
+ diag.Explain(
+ "After adding a SUBST class to SUBST_CLASSES,",
+ "the remaining SUBST variables should be defined in the same file.",
+ "",
+ "See mk/subst.mk for the comprehensive documentation.")
+}
+
+// In the paragraph of a SUBST block, there should be only variables
+// that actually belong to the SUBST block.
+//
+// In addition, variables that are mentioned in SUBST_VARS may also
+// be defined there because they closely relate to the SUBST block.
+
+func (ctx *SubstContext) allowVar(varname string) {
+ if ctx.foreignAllowed == nil {
+ ctx.foreignAllowed = make(map[string]struct{})
+ }
+ ctx.foreignAllowed[varname] = struct{}{}
+}
+
+func (ctx *SubstContext) rememberForeign(mkline *MkLine) {
+ ctx.foreign = append(ctx.foreign, mkline)
+}
+
+// forEachForeignVar performs the given action for each variable that
+// is defined in the SUBST block and is not mentioned in SUBST_VARS.
+func (ctx *SubstContext) forEachForeignVar(action func(*MkLine)) {
+ for _, foreign := range ctx.foreign {
+ if _, ok := ctx.foreignAllowed[foreign.Varname()]; !ok {
+ action(foreign)
+ }
+ }
+}
+
+func (ctx *SubstContext) reset() {
+ ctx.id = ""
+ ctx.foreignAllowed = nil
+ ctx.foreign = nil
+ ctx.conds = []*substCond{{seenElse: true}}
+}
+
+func (ctx *SubstContext) isActive() bool { return ctx.id != "" }
+
+func (ctx *SubstContext) isActiveId(id string) bool { return ctx.id == id }
+
+func (ctx *SubstContext) activeId() string {
+ assert(ctx.isActive())
+ return ctx.id
+}
+
+func (ctx *SubstContext) setActiveId(id string) {
+ ctx.id = id
+ ctx.cond().top = true
+ ctx.markAsDone(id)
+}
+
+func (ctx *SubstContext) queue(id string) {
+ ctx.queuedIds = append(ctx.queuedIds, id)
+}
+
+func (ctx *SubstContext) start(id string) bool {
+ for i, queuedId := range ctx.queuedIds {
+ if queuedId == id {
+ ctx.queuedIds[i] = ""
+ ctx.setActiveId(id)
+ return true
+ }
+ }
+ return false
+}
+
+func (ctx *SubstContext) markAsDone(id string) {
+ if ctx.doneIds == nil {
+ ctx.doneIds = map[string]bool{}
+ }
+ ctx.doneIds[id] = true
+}
+
+func (ctx *SubstContext) markAsNotDone() {
+ ctx.doneIds[ctx.id] = false
+}
+
+func (ctx *SubstContext) isDone(varparam string) bool {
+ return ctx.doneIds[varparam]
+}
+
+func (ctx *SubstContext) isComplete() bool {
+ return ctx.seen().hasAll(ssStage | ssFiles | ssTransform)
+}
+
+// isConditional returns whether the current line is at a deeper conditional
+// level than the assignment where the corresponding class ID is added to
+// SUBST_CLASSES.
+//
+// TODO: Adjust the implementation to this description.
+func (ctx *SubstContext) isConditional() bool {
+ return !ctx.cond().top
+}
+
+// cond returns information about the parts of the SUBST block that
+// have already been seen in the current leaf branch of the conditionals.
+func (ctx *SubstContext) seen() *substSeen {
+ return &ctx.cond().curr
+}
+
+// cond returns information about the current branch of conditionals.
+func (ctx *SubstContext) cond() *substCond {
+ return ctx.conds[len(ctx.conds)-1]
+}
+
+// Returns true if the given flag from substSeen has been seen
+// somewhere in the conditional path of the current line.
+func (ctx *SubstContext) seenInBranch(part substSeen) bool {
+ for _, cond := range ctx.conds {
+ if cond.curr.get(part) {
+ return true
+ }
+ }
+ return false
+}
+
+type substCond struct {
+ // Tells whether a SUBST block has started at this conditional level.
+ // All variable assignments that belong to this class must happen at
+ // this conditional level or below it.
+ //
+ // TODO: For Test_SubstContext_Directive__conditional_complete,
+ // this needs to be changed to the set of classes that have been
+ // added to SUBST_CLASSES at this level.
+ top bool
+
+ // Collects the parts of the SUBST block that have been defined in all
+ // branches that have been parsed completely.
+ total substSeen
+
+ // Collects the parts of the SUBST block that are defined in the current
+ // branch of the conditional. At the end of the branch, they are merged
+ // into the total.
+ curr substSeen
+
+ // Marks whether the current conditional statement has
+ // an .else branch. If it doesn't, this means that all variables
+ // are potentially unset in that branch.
+ seenElse bool
+}
+
+// substSeen contains all variables that depend on a particular SUBST
+// class ID. These variables can be set in conditional branches, and
+// pkglint keeps track whether they are set in all branches or only
+// in some of them.
+type substSeen uint8
+
+const (
+ ssStage substSeen = 1 << iota
+ ssMessage
+ ssFiles
+ ssSed
+ ssVars
+ ssVarsAutofix
+ ssFilterCmd
+ ssTransform
+
+ ssAll substSeen = 1<<iota - 1
+ ssNone substSeen = 0
+)
+
+func (s *substSeen) set(part substSeen) { *s |= part }
+func (s *substSeen) get(part substSeen) bool { return *s&part != 0 }
+func (s *substSeen) hasAll(other substSeen) bool { return *s&other == other }
+func (s *substSeen) union(other substSeen) { *s |= other }
+func (s *substSeen) retain(other substSeen) { *s &= other }
Index: pkgsrc/pkgtools/pkglint/files/substcontext_test.go
diff -u pkgsrc/pkgtools/pkglint/files/substcontext_test.go:1.31 pkgsrc/pkgtools/pkglint/files/substcontext_test.go:1.32
--- pkgsrc/pkgtools/pkglint/files/substcontext_test.go:1.31 Mon Dec 2 23:32:09 2019
+++ pkgsrc/pkgtools/pkglint/files/substcontext_test.go Fri Dec 13 01:39:23 2019
@@ -1,346 +1,547 @@
package pkglint
-import (
- "fmt"
- "gopkg.in/check.v1"
-)
+import "gopkg.in/check.v1"
-func (s *Suite) Test_SubstContext__incomplete(c *check.C) {
- t := s.Init(c)
-
- t.SetUpCommandLine("-Wextra")
- ctx := NewSubstContext()
-
- ctx.Varassign(t.NewMkLine("Makefile", 10, "PKGNAME=pkgname-1.0"))
+func (t *Tester) NewSubstAutofixTest(lines ...string) func(bool) {
+ return func(autofix bool) {
+ mklines := t.NewMkLines("filename.mk", lines...)
+ ctx := NewSubstContext()
- t.CheckEquals(ctx.id, "")
-
- ctx.Varassign(t.NewMkLine("Makefile", 11, "SUBST_CLASSES+=interp"))
-
- t.CheckEquals(ctx.id, "interp")
+ mklines.ForEach(ctx.Process)
+ ctx.Finish(mklines.EOFLine())
- ctx.Varassign(t.NewMkLine("Makefile", 12, "SUBST_FILES.interp=Makefile"))
-
- t.CheckEquals(ctx.IsComplete(), false)
-
- ctx.Varassign(t.NewMkLine("Makefile", 13, "SUBST_SED.interp=s,@PREFIX@,${PREFIX},g"))
-
- t.CheckEquals(ctx.IsComplete(), false)
-
- ctx.Finish(t.NewMkLine("Makefile", 14, ""))
-
- t.CheckOutputLines(
- "NOTE: Makefile:13: The substitution command \"s,@PREFIX@,${PREFIX},g\" "+
- "can be replaced with \"SUBST_VARS.interp= PREFIX\".",
- "WARN: Makefile:14: Incomplete SUBST block: SUBST_STAGE.interp missing.")
+ mklines.SaveAutofixChanges()
+ }
}
-func (s *Suite) Test_SubstContext__complete(c *check.C) {
- t := s.Init(c)
+func (t *Tester) RunSubst(lines ...string) {
+ assert(lines[len(lines)-1] != "")
- t.SetUpCommandLine("-Wextra")
+ mklines := t.NewMkLines("filename.mk", lines...)
ctx := NewSubstContext()
- ctx.Varassign(t.NewMkLine("Makefile", 10, "PKGNAME=pkgname-1.0"))
- ctx.Varassign(t.NewMkLine("Makefile", 11, "SUBST_CLASSES+=p"))
- ctx.Varassign(t.NewMkLine("Makefile", 12, "SUBST_FILES.p=Makefile"))
- ctx.Varassign(t.NewMkLine("Makefile", 13, "SUBST_SED.p=s,@PREFIX@,${PREFIX},g"))
-
- t.CheckEquals(ctx.IsComplete(), false)
-
- ctx.Varassign(t.NewMkLine("Makefile", 14, "SUBST_STAGE.p=post-configure"))
-
- t.CheckEquals(ctx.IsComplete(), true)
-
- ctx.Finish(t.NewMkLine("Makefile", 15, ""))
-
- t.CheckOutputLines(
- "NOTE: Makefile:13: The substitution command \"s,@PREFIX@,${PREFIX},g\" " +
- "can be replaced with \"SUBST_VARS.p= PREFIX\".")
+ mklines.ForEach(ctx.Process)
+ ctx.Finish(mklines.EOFLine())
}
func (s *Suite) Test_SubstContext__OPSYSVARS(c *check.C) {
t := s.Init(c)
- G.Opts.WarnExtra = true
ctx := NewSubstContext()
// SUBST_CLASSES is added to OPSYSVARS in mk/bsd.pkg.mk.
- ctx.Varassign(t.NewMkLine("Makefile", 11, "SUBST_CLASSES.SunOS+=prefix"))
- ctx.Varassign(t.NewMkLine("Makefile", 12, "SUBST_CLASSES.NetBSD+=prefix"))
- ctx.Varassign(t.NewMkLine("Makefile", 13, "SUBST_FILES.prefix=Makefile"))
- ctx.Varassign(t.NewMkLine("Makefile", 14, "SUBST_SED.prefix=s,@PREFIX@,${PREFIX},g"))
- ctx.Varassign(t.NewMkLine("Makefile", 15, "SUBST_STAGE.prefix=post-configure"))
+ ctx.varassign(t.NewMkLine("filename.mk", 11, "SUBST_CLASSES.SunOS+=prefix"))
+ ctx.varassign(t.NewMkLine("filename.mk", 12, "SUBST_CLASSES.NetBSD+=prefix"))
+ ctx.varassign(t.NewMkLine("filename.mk", 13, "SUBST_FILES.prefix=Makefile"))
+ ctx.varassign(t.NewMkLine("filename.mk", 14, "SUBST_SED.prefix=s,@PREFIX@,${PREFIX},g"))
+ ctx.varassign(t.NewMkLine("filename.mk", 15, "SUBST_STAGE.prefix=post-configure"))
- t.CheckEquals(ctx.IsComplete(), true)
+ t.CheckEquals(ctx.isComplete(), true)
- ctx.Finish(t.NewMkLine("Makefile", 15, ""))
+ ctx.Finish(t.NewMkLine("filename.mk", 15, ""))
t.CheckOutputLines(
- "NOTE: Makefile:14: The substitution command \"s,@PREFIX@,${PREFIX},g\" " +
+ "NOTE: filename.mk:14: The substitution command \"s,@PREFIX@,${PREFIX},g\" " +
"can be replaced with \"SUBST_VARS.prefix= PREFIX\".")
}
func (s *Suite) Test_SubstContext__no_class(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Wextra")
- ctx := NewSubstContext()
-
- ctx.Varassign(t.NewMkLine("Makefile", 10, "UNRELATED=anything"))
- ctx.Varassign(t.NewMkLine("Makefile", 11, "SUBST_FILES.repl+=Makefile.in"))
- ctx.Varassign(t.NewMkLine("Makefile", 12, "SUBST_SED.repl+=-e s,from,to,g"))
- ctx.Finish(t.NewMkLine("Makefile", 13, ""))
+ t.RunSubst(
+ "UNRELATED=anything",
+ "SUBST_FILES.repl+=Makefile.in",
+ "SUBST_SED.repl+=-e s,from,to,g")
t.CheckOutputLines(
- "WARN: Makefile:11: SUBST_CLASSES should come before the definition of \"SUBST_FILES.repl\".",
- "WARN: Makefile:13: Incomplete SUBST block: SUBST_STAGE.repl missing.")
+ "WARN: filename.mk:2: Before defining SUBST_FILES.repl, " +
+ "the SUBST class should be declared using \"SUBST_CLASSES+= repl\".")
}
func (s *Suite) Test_SubstContext__multiple_classes_in_one_line(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Wextra")
+ t.RunSubst(
+ "SUBST_CLASSES+= one two",
+ "SUBST_STAGE.one= post-configure",
+ "SUBST_FILES.one= one.txt",
+ "SUBST_SED.one= s,one,1,g",
+ "SUBST_STAGE.two= post-configure",
+ "SUBST_FILES.two= two.txt")
+
+ t.CheckOutputLines(
+ "NOTE: filename.mk:1: Please add only one class at a time to SUBST_CLASSES.",
+ "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_SED.two, SUBST_VARS.two or SUBST_FILTER_CMD.two missing.")
+}
+
+func (s *Suite) Test_SubstContext__multiple_classes_in_one_line_multiple_blocks(c *check.C) {
+ t := s.Init(c)
- simulateSubstLines(t,
- "10: SUBST_CLASSES+= one two",
- "11: SUBST_STAGE.one= post-configure",
- "12: SUBST_FILES.one= one.txt",
- "13: SUBST_SED.one= s,one,1,g",
- "14: SUBST_STAGE.two= post-configure",
- "15: SUBST_FILES.two= two.txt")
+ t.RunSubst(
+ "SUBST_CLASSES+= one two",
+ "SUBST_STAGE.one= post-configure",
+ "SUBST_FILES.one= one.txt",
+ "SUBST_SED.one= s,one,1,g",
+ "",
+ "SUBST_STAGE.two= post-configure",
+ "SUBST_FILES.two= two.txt",
+ "",
+ "SUBST_STAGE.three= post-configure",
+ "",
+ "SUBST_VARS.four= PREFIX",
+ "",
+ "SUBST_VARS.three= PREFIX")
t.CheckOutputLines(
- "WARN: Makefile:10: Please add only one class at a time to SUBST_CLASSES.",
- "WARN: Makefile:16: Incomplete SUBST block: SUBST_SED.two, SUBST_VARS.two or SUBST_FILTER_CMD.two missing.")
+ "NOTE: filename.mk:1: Please add only one class at a time to SUBST_CLASSES.",
+ "WARN: filename.mk:8: Incomplete SUBST block: "+
+ "SUBST_SED.two, SUBST_VARS.two or SUBST_FILTER_CMD.two missing.",
+ "WARN: filename.mk:9: Before defining SUBST_STAGE.three, "+
+ "the SUBST class should be declared using \"SUBST_CLASSES+= three\".",
+ "WARN: filename.mk:11: Before defining SUBST_VARS.four, "+
+ "the SUBST class should be declared using \"SUBST_CLASSES+= four\".")
}
func (s *Suite) Test_SubstContext__multiple_classes_in_one_block(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Wextra")
+ t.RunSubst(
+ "SUBST_CLASSES+= one",
+ "SUBST_STAGE.one= post-configure",
+ "SUBST_STAGE.one= post-configure",
+ "SUBST_FILES.one= one.txt",
+ "SUBST_CLASSES+= two", // The block "one" is not finished yet.
+ "SUBST_SED.one= s,one,1,g",
+ "SUBST_STAGE.two= post-configure",
+ "SUBST_FILES.two= two.txt",
+ "SUBST_SED.two= s,two,2,g")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:3: Duplicate definition of \"SUBST_STAGE.one\".",
+ "WARN: filename.mk:5: Incomplete SUBST block: SUBST_SED.one, SUBST_VARS.one or SUBST_FILTER_CMD.one missing.",
+ "WARN: filename.mk:5: Subst block \"one\" should be finished before adding the next class to SUBST_CLASSES.",
+ "WARN: filename.mk:6: Variable \"SUBST_SED.one\" does not match SUBST class \"two\".")
+}
+
+// This is a strange example that probably won't occur in practice.
+//
+// Continuing a SUBST class in one of the branches and starting
+// a fresh one in the other seems far-fetched.
+func (s *Suite) Test_SubstContext__partially_continued_class_in_conditional(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= outer",
+ "SUBST_STAGE.outer= post-configure",
+ "SUBST_FILES.outer= files",
+ "SUBST_VARS.outer= OUTER.first",
+ ".if ${:Ualways}",
+ "SUBST_VARS.outer+= OUTER.second",
+ ".else",
+ "SUBST_CLASSES+= inner",
+ "SUBST_STAGE.inner= post-configure",
+ "SUBST_FILES.inner= files",
+ "SUBST_VARS.inner= INNER",
+ ".endif")
- simulateSubstLines(t,
- "10: SUBST_CLASSES+= one",
- "11: SUBST_STAGE.one= post-configure",
- "12: SUBST_STAGE.one= post-configure",
- "13: SUBST_FILES.one= one.txt",
- "14: SUBST_CLASSES+= two", // The block "one" is not finished yet.
- "15: SUBST_SED.one= s,one,1,g",
- "16: SUBST_STAGE.two= post-configure",
- "17: SUBST_FILES.two= two.txt",
- "18: SUBST_SED.two= s,two,2,g")
-
- t.CheckOutputLines(
- "WARN: Makefile:12: Duplicate definition of \"SUBST_STAGE.one\".",
- "WARN: Makefile:14: Incomplete SUBST block: SUBST_SED.one, SUBST_VARS.one or SUBST_FILTER_CMD.one missing.",
- "WARN: Makefile:14: Subst block \"one\" should be finished before adding the next class to SUBST_CLASSES.",
- "WARN: Makefile:15: Variable \"SUBST_SED.one\" does not match SUBST class \"two\".")
+ t.CheckOutputEmpty()
}
func (s *Suite) Test_SubstContext__files_missing(c *check.C) {
t := s.Init(c)
- simulateSubstLines(t,
- "10: SUBST_CLASSES+= one",
- "11: SUBST_STAGE.one= pre-configure",
- "12: SUBST_CLASSES+= two",
- "13: SUBST_STAGE.two= pre-configure",
- "14: SUBST_FILES.two= two.txt",
- "15: SUBST_SED.two= s,two,2,g")
+ t.RunSubst(
+ "SUBST_CLASSES+= one",
+ "SUBST_STAGE.one= pre-configure",
+ "SUBST_CLASSES+= two",
+ "SUBST_STAGE.two= pre-configure",
+ "SUBST_FILES.two= two.txt",
+ "SUBST_SED.two= s,two,2,g")
t.CheckOutputLines(
- "WARN: Makefile:12: Incomplete SUBST block: SUBST_FILES.one missing.",
- "WARN: Makefile:12: Incomplete SUBST block: "+
+ "WARN: filename.mk:3: Incomplete SUBST block: SUBST_FILES.one missing.",
+ "WARN: filename.mk:3: Incomplete SUBST block: "+
"SUBST_SED.one, SUBST_VARS.one or SUBST_FILTER_CMD.one missing.",
- "WARN: Makefile:12: Subst block \"one\" should be finished "+
+ "WARN: filename.mk:3: Subst block \"one\" should be finished "+
"before adding the next class to SUBST_CLASSES.")
}
func (s *Suite) Test_SubstContext__directives(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Wextra")
-
- simulateSubstLines(t,
- "10: SUBST_CLASSES+= os",
- "11: SUBST_STAGE.os= post-configure",
- "12: SUBST_MESSAGE.os= Guessing operating system",
- "13: SUBST_FILES.os= guess-os.h",
- "14: .if ${OPSYS} == NetBSD",
- "15: SUBST_FILTER_CMD.os= ${SED} -e s,@OPSYS@,NetBSD,",
- "16: .elif ${OPSYS} == Darwin",
- "17: SUBST_SED.os= -e s,@OPSYS@,Darwin1,",
- "18: SUBST_SED.os= -e s,@OPSYS@,Darwin2,",
- "19: .elif ${OPSYS} == Linux",
- "20: SUBST_SED.os= -e s,@OPSYS@,Linux,",
- "21: .else",
- "22: SUBST_VARS.os= OPSYS",
- "23: .endif")
+ t.RunSubst(
+ "SUBST_CLASSES+= os",
+ "SUBST_STAGE.os= post-configure",
+ "SUBST_MESSAGE.os= Guessing operating system",
+ "SUBST_FILES.os= guess-os.h",
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_FILTER_CMD.os= ${SED} -e s,@OPSYS@,NetBSD,",
+ ".elif ${OPSYS} == Darwin",
+ "SUBST_SED.os= -e s,@OPSYS@,Darwin1,",
+ "SUBST_SED.os= -e s,@OPSYS@,Darwin2,",
+ ".elif ${OPSYS} == Linux",
+ "SUBST_SED.os= -e s,@OPSYS@,Linux,",
+ ".else",
+ "SUBST_VARS.os= OPSYS",
+ ".endif")
// All the other lines are correctly determined as being alternatives
// to each other. And since every branch contains some transformation
// (SED, VARS, FILTER_CMD), everything is fine.
t.CheckOutputLines(
- "WARN: Makefile:18: All but the first \"SUBST_SED.os\" lines should use the \"+=\" operator.")
+ "WARN: filename.mk:9: All but the first assignment " +
+ "to \"SUBST_SED.os\" should use the \"+=\" operator.")
}
-func (s *Suite) Test_SubstContext__directives_around_everything_then(c *check.C) {
+func (s *Suite) Test_SubstContext__adjacent(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Wextra")
+ t.RunSubst(
+ "SUBST_CLASSES+=\t1",
+ "SUBST_STAGE.1=\tpre-configure",
+ "SUBST_FILES.1=\tfile1",
+ "SUBST_SED.1=\t-e s,subst1,repl1,",
+ "SUBST_CLASSES+=\t2",
+ "SUBST_SED.1+=\t-e s,subst1b,repl1b,", // Misplaced
+ "SUBST_STAGE.2=\tpre-configure",
+ "SUBST_FILES.2=\tfile2",
+ "SUBST_SED.2=\t-e s,subst2,repl2,")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:6: Variable \"SUBST_SED.1\" does not match SUBST class \"2\".")
+}
+
+func (s *Suite) Test_SubstContext__do_patch(c *check.C) {
+ t := s.Init(c)
- simulateSubstLines(t,
- "10: SUBST_CLASSES+= os",
- "11: .if ${OPSYS} == NetBSD",
- "12: SUBST_VARS.os= OPSYS",
- "13: SUBST_SED.os= -e s,@OPSYS@,NetBSD,",
- "14: SUBST_STAGE.os= post-configure",
- "15: SUBST_MESSAGE.os= Guessing operating system",
- "16: SUBST_FILES.os= guess-os.h",
- "17: .endif")
+ t.RunSubst(
+ "SUBST_CLASSES+=\tos",
+ "SUBST_STAGE.os=\tdo-patch",
+ "SUBST_FILES.os=\tguess-os.h",
+ "SUBST_SED.os=\t-e s,@OPSYS@,Darwin,")
- // TODO: The SUBST variables are not guaranteed to be defined in all cases.
+ // No warning, since there is nothing to fix automatically.
+ // This case doesn't occur in practice anyway.
t.CheckOutputEmpty()
}
-func (s *Suite) Test_SubstContext__directives_around_everything_else(c *check.C) {
+// Variables mentioned in SUBST_VARS are not considered "foreign"
+// in the block and may be mixed with the other SUBST variables.
+func (s *Suite) Test_SubstContext__SUBST_VARS_defined_in_block(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Wextra")
-
- simulateSubstLines(t,
- "10: SUBST_CLASSES+= os",
- "11: .if ${OPSYS} == NetBSD",
- "12: .else",
- "13: SUBST_VARS.os= OPSYS",
- "14: SUBST_SED.os= -e s,@OPSYS@,NetBSD,",
- "15: SUBST_STAGE.os= post-configure",
- "16: SUBST_MESSAGE.os= Guessing operating system",
- "17: SUBST_FILES.os= guess-os.h",
- "18: .endif")
+ t.RunSubst(
+ "SUBST_CLASSES+=\tos",
+ "SUBST_STAGE.os=\tpre-configure",
+ "SUBST_FILES.os=\tguess-os.h",
+ "SUBST_VARS.os=\tTODAY1",
+ "TODAY1!=\tdate",
+ "TODAY2!=\tdate")
- // FIXME: The warnings must be the same as in the "then" test case.
t.CheckOutputLines(
- "WARN: Makefile:19: Incomplete SUBST block: SUBST_FILES.os missing.",
- "WARN: Makefile:19: Incomplete SUBST block: SUBST_SED.os, SUBST_VARS.os or "+
- "SUBST_FILTER_CMD.os missing.")
+ "WARN: filename.mk:6: Foreign variable \"TODAY2\" in SUBST block.")
}
-func (s *Suite) Test_SubstContext__empty_directive(c *check.C) {
+// Variables mentioned in SUBST_VARS may appear in the same paragraph,
+// or alternatively anywhere else in the file.
+func (s *Suite) Test_SubstContext__SUBST_VARS_in_next_paragraph(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Wextra")
+ t.RunSubst(
+ "SUBST_CLASSES+=\tos",
+ "SUBST_STAGE.os=\tpre-configure",
+ "SUBST_FILES.os=\tguess-os.h",
+ "SUBST_VARS.os=\tTODAY1",
+ "",
+ "TODAY1!=\tdate",
+ "TODAY2!=\tdate")
+
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_SubstContext__multiple_SUBST_VARS(c *check.C) {
+ t := s.Init(c)
- simulateSubstLines(t,
- "10: SUBST_CLASSES+= os",
- "11: SUBST_VARS.os= OPSYS",
- "12: SUBST_SED.os= -e s,@OPSYS@,NetBSD,",
- "13: SUBST_STAGE.os= post-configure",
- "14: SUBST_MESSAGE.os= Guessing operating system",
- "15: SUBST_FILES.os= guess-os.h",
- "16: .if ${OPSYS} == NetBSD",
- "17: .else",
- "18: .endif")
+ t.RunSubst(
+ "SUBST_CLASSES+=\tos",
+ "SUBST_STAGE.os=\tpre-configure",
+ "SUBST_FILES.os=\tguess-os.h",
+ "SUBST_VARS.os=\tPREFIX VARBASE")
t.CheckOutputEmpty()
}
-func (s *Suite) Test_SubstContext__missing_transformation_in_one_branch(c *check.C) {
+// As of May 2019, pkglint does not check the order of the variables in
+// a SUBST block. Enforcing this order, or at least suggesting it, would
+// make pkgsrc packages more uniform, which is a good idea, but not urgent.
+func (s *Suite) Test_SubstContext__unusual_variable_order(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Wextra")
+ t.RunSubst(
+ "SUBST_CLASSES+=\t\tid",
+ "SUBST_SED.id=\t\t-e /deleteme/d",
+ "SUBST_FILES.id=\t\tfile",
+ "SUBST_MESSAGE.id=\tMessage",
+ "SUBST_STAGE.id=\t\tpre-configure")
- simulateSubstLines(t,
- "10: SUBST_CLASSES+= os",
- "11: SUBST_STAGE.os= post-configure",
- "12: SUBST_MESSAGE.os= Guessing operating system",
- "13: SUBST_FILES.os= guess-os.h",
- "14: .if ${OPSYS} == NetBSD",
- "15: SUBST_FILES.os= -e s,@OpSYS@,NetBSD,", // A simple typo, this should be SUBST_SED.
- "16: .elif ${OPSYS} == Darwin",
- "17: SUBST_SED.os= -e s,@OPSYS@,Darwin1,",
- "18: SUBST_SED.os= -e s,@OPSYS@,Darwin2,",
- "19: .else",
- "20: SUBST_VARS.os= OPSYS",
- "21: .endif")
+ t.CheckOutputEmpty()
+}
+func (s *Suite) Test_SubstContext__completely_conditional_then(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ ".if ${OPSYS} == Linux",
+ "SUBST_CLASSES+=\tid",
+ "SUBST_STAGE.id=\tpre-configure",
+ "SUBST_SED.id=\t-e sahara",
+ ".else",
+ ".endif")
+
+ // The block already ends at the .else, not at the end of the file,
+ // since that is the scope where the SUBST id is defined.
t.CheckOutputLines(
- "WARN: Makefile:15: All but the first \"SUBST_FILES.os\" lines should use the \"+=\" operator.",
- "WARN: Makefile:18: All but the first \"SUBST_SED.os\" lines should use the \"+=\" operator.",
- "WARN: Makefile:22: Incomplete SUBST block: SUBST_SED.os, SUBST_VARS.os or SUBST_FILTER_CMD.os missing.")
+ "WARN: filename.mk:5: Incomplete SUBST block: SUBST_FILES.id missing.")
}
-func (s *Suite) Test_SubstContext__nested_conditionals(c *check.C) {
+func (s *Suite) Test_SubstContext__completely_conditional_else(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Wextra")
+ t.RunSubst(
+ ".if ${OPSYS} == Linux",
+ ".else",
+ "SUBST_CLASSES+=\tid",
+ "SUBST_STAGE.id=\tpre-configure",
+ "SUBST_SED.id=\t-e sahara",
+ ".endif")
+
+ // The block already ends at the .endif, not at the end of the file,
+ // since that is the scope where the SUBST id is defined.
+ t.CheckOutputLines(
+ "WARN: filename.mk:6: Incomplete SUBST block: SUBST_FILES.id missing.")
+}
+
+func (s *Suite) Test_SubstContext__SUBST_CLASSES_in_separate_paragraph(c *check.C) {
+ t := s.Init(c)
- simulateSubstLines(t,
- "10: SUBST_CLASSES+= os",
- "11: SUBST_STAGE.os= post-configure",
- "12: SUBST_MESSAGE.os= Guessing operating system",
- "13: .if ${OPSYS} == NetBSD",
- "14: SUBST_FILES.os= guess-netbsd.h",
- "15: . if ${ARCH} == i386",
- "16: SUBST_FILTER_CMD.os= ${SED} -e s,@OPSYS,NetBSD-i386,",
- "17: . elif ${ARCH} == x86_64",
- "18: SUBST_VARS.os= OPSYS",
- "19: . else",
- "20: SUBST_SED.os= -e s,@OPSYS,NetBSD-unknown",
- "21: . endif",
- "22: .else",
- "23: SUBST_SED.os= -e s,@OPSYS@,unknown,",
- "24: .endif")
+ t.RunSubst(
+ "SUBST_CLASSES+= 1 2 3 4",
+ "",
+ "SUBST_STAGE.1= post-configure",
+ "SUBST_FILES.1= files",
+ "SUBST_VARS.1= VAR1",
+ "",
+ "SUBST_STAGE.2= post-configure",
+ "SUBST_FILES.2= files",
+ "SUBST_VARS.2= VAR1",
+ "",
+ "SUBST_STAGE.3= post-configure",
+ "SUBST_FILES.3= files",
+ "SUBST_VARS.3= VAR1")
- // The branch in line 23 omits SUBST_FILES.
t.CheckOutputLines(
- "WARN: Makefile:25: Incomplete SUBST block: SUBST_FILES.os missing.")
+ "NOTE: filename.mk:1: Please add only one class at a time to SUBST_CLASSES.",
+ "WARN: filename.mk:EOF: Missing SUBST block for \"4\".")
}
-func (s *Suite) Test_SubstContext__pre_patch(c *check.C) {
+func (s *Suite) Test_SubstContext__wrong_class(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Wextra", "--show-autofix")
- t.SetUpVartypes()
+ t.RunSubst(
+ "SUBST_CLASSES+= 1 2",
+ "SUBST_STAGE.x= post-configure",
+ "SUBST_FILES.x= files",
+ "SUBST_VARS.x= VAR1",
+ "SUBST_STAGE.2= post-configure",
+ "SUBST_FILES.2= files",
+ "SUBST_VARS.2= VAR1")
- mklines := t.NewMkLines("os.mk",
- MkCvsID,
+ t.CheckOutputLines(
+ "NOTE: filename.mk:1: Please add only one class at a time to SUBST_CLASSES.",
+ "WARN: filename.mk:2: Variable \"SUBST_STAGE.x\" does not match SUBST class \"1\".",
+ "WARN: filename.mk:3: Variable \"SUBST_FILES.x\" does not match SUBST class \"1\".",
+ "WARN: filename.mk:4: Variable \"SUBST_VARS.x\" does not match SUBST class \"1\".",
+ // XXX: This line could change to 2, since that is already in the queue.
+ "WARN: filename.mk:5: Variable \"SUBST_STAGE.2\" does not match SUBST class \"1\".",
+ "WARN: filename.mk:6: Variable \"SUBST_FILES.2\" does not match SUBST class \"1\".",
+ "WARN: filename.mk:7: Variable \"SUBST_VARS.2\" does not match SUBST class \"1\".",
+ "WARN: filename.mk:EOF: Missing SUBST block for \"1\".",
+ "WARN: filename.mk:EOF: Missing SUBST block for \"2\".")
+}
+
+func (s *Suite) Test_SubstContext_varassign__late_addition(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+=\tid",
+ "SUBST_STAGE.id=\tpost-configure",
+ "SUBST_FILES.id=\tfiles",
+ "SUBST_VARS.id=\tPREFIX",
"",
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_VARS.id=\tOPSYS",
+ ".endif")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:7: Late additions to a SUBST variable " +
+ "should use the += operator.")
+}
+
+func (s *Suite) Test_SubstContext_varassign__late_addition_to_unknown_class(c *check.C) {
+ t := s.Init(c)
+
+ mklines := t.NewMkLines("filename.mk",
+ "SUBST_VARS.id=\tOPSYS",
+ "")
+ ctx := NewSubstContext()
+ mklines.collectRationale()
+
+ mklines.ForEach(ctx.Process)
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:1: Before defining SUBST_VARS.id, " +
+ "the SUBST class should be declared using \"SUBST_CLASSES+= id\".")
+}
+
+func (s *Suite) Test_SubstContext_varassignClasses__none(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+=\t# none")
+
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_SubstContext_varassignClasses__indirect(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpVartypes()
+ mklines := t.NewMkLines("filename.mk",
+ MkCvsID,
+ "SUBST_CLASSES+=\t${VAR}")
+
+ mklines.Check()
+
+ t.CheckOutputLines(
+ "ERROR: filename.mk:2: Identifiers for SUBST_CLASSES "+
+ "must not refer to other variables.",
+ "WARN: filename.mk:2: VAR is used but not defined.")
+}
+
+// The rationale for the stray SUBST variables has to be specific.
+//
+// For example, in the following snippet from mail/dkim-milter/options.mk
+// revision 1.9, there is a comment, but that is not a rationale and also
+// not related to the SUBST_CLASS variable at all:
+// ### IPv6 support.
+// .if !empty(PKG_OPTIONS:Minet6)
+// SUBST_SED.libs+= -e 's|@INET6@||g'
+// .endif
+func (s *Suite) Test_SubstContext_varassignOutsideBlock__rationale(c *check.C) {
+ t := s.Init(c)
+
+ mklines := t.NewMkLines("filename.mk",
+ // The rationale is too unspecific since it doesn't refer to the
+ // "one" class.
+ "# I know what I'm doing.",
+ "SUBST_VARS.one=\tOPSYS",
+ "",
+ // The subst class "two" appears in the rationale.
+ "# The two class is defined somewhere else.",
+ "SUBST_VARS.two=\tOPSYS",
+ "",
+ // The word "defined" doesn't match the subst class "def".
+ "# This subst class is defined somewhere else.",
+ "SUBST_VARS.def=\tOPSYS",
+ "",
+ "# Rationale that is completely irrelevant.",
+ "SUBST_SED.libs+=\t-e sahara",
+ "")
+ ctx := NewSubstContext()
+ mklines.collectRationale()
+
+ mklines.ForEach(ctx.Process)
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:2: Before defining SUBST_VARS.one, "+
+ "the SUBST class should be declared using \"SUBST_CLASSES+= one\".",
+ // In filename.mk:5 there is a proper rationale, thus no warning.
+ "WARN: filename.mk:8: Before defining SUBST_VARS.def, "+
+ "the SUBST class should be declared using \"SUBST_CLASSES+= def\".",
+ "WARN: filename.mk:11: Before defining SUBST_SED.libs, "+
+ "the SUBST class should be declared using \"SUBST_CLASSES+= libs\".")
+}
+
+func (s *Suite) Test_SubstContext_varassignStage__pre_patch(c *check.C) {
+ t := s.Init(c)
+
+ t.Chdir(".")
+
+ doTest := t.NewSubstAutofixTest(
"SUBST_CLASSES+=\tos",
"SUBST_STAGE.os=\tpre-patch",
"SUBST_FILES.os=\tguess-os.h",
"SUBST_SED.os=\t-e s,@OPSYS@,Darwin,")
- mklines.Check()
-
- t.CheckOutputLines(
- "WARN: os.mk:4: Substitutions should not happen in the patch phase.",
- "AUTOFIX: os.mk:4: Replacing \"pre-patch\" with \"post-extract\".")
+ t.ExpectDiagnosticsAutofix(
+ doTest,
+ "WARN: filename.mk:2: Substitutions should not happen in the patch phase.",
+ "AUTOFIX: filename.mk:2: Replacing \"pre-patch\" with \"post-extract\".")
+
+ t.CheckFileLinesDetab("filename.mk",
+ "SUBST_CLASSES+= os",
+ "SUBST_STAGE.os= post-extract",
+ "SUBST_FILES.os= guess-os.h",
+ "SUBST_SED.os= -e s,@OPSYS@,Darwin,")
}
-func (s *Suite) Test_SubstContext__post_patch(c *check.C) {
+func (s *Suite) Test_SubstContext_varassignStage__post_patch(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Wextra", "--show-autofix")
- t.SetUpVartypes()
+ t.Chdir(".")
- mklines := t.NewMkLines("os.mk",
- MkCvsID,
- "",
+ doTest := t.NewSubstAutofixTest(
"SUBST_CLASSES+=\tos",
"SUBST_STAGE.os=\tpost-patch",
"SUBST_FILES.os=\tguess-os.h",
"SUBST_SED.os=\t-e s,@OPSYS@,Darwin,")
- mklines.Check()
+ t.ExpectDiagnosticsAutofix(
+ doTest,
+ "WARN: filename.mk:2: Substitutions should not happen in the patch phase.",
+ "AUTOFIX: filename.mk:2: Replacing \"post-patch\" with \"pre-configure\".")
+
+ t.CheckFileLinesDetab("filename.mk",
+ "SUBST_CLASSES+= os",
+ "SUBST_STAGE.os= pre-configure",
+ "SUBST_FILES.os= guess-os.h",
+ "SUBST_SED.os= -e s,@OPSYS@,Darwin,")
+}
+
+// As of December 2019, pkglint does not use token positions internally.
+// Instead it only does simple string replacement when autofixing things.
+// To avoid damaging anything, replacements are only done if they are
+// unambiguous. This is not the case here, since line 4 contains the
+// string "pre-patch" twice.
+func (s *Suite) Test_SubstContext_varassignStage__ambiguous_replacement(c *check.C) {
+ t := s.Init(c)
- t.CheckOutputLines(
- "WARN: os.mk:4: Substitutions should not happen in the patch phase.",
- "AUTOFIX: os.mk:4: Replacing \"post-patch\" with \"pre-configure\".")
+ t.Chdir(".")
+
+ doTest := t.NewSubstAutofixTest(
+ "SUBST_CLASSES+= pre-patch",
+ "SUBST_STAGE.pre-patch= pre-patch",
+ "SUBST_FILES.pre-patch= files",
+ "SUBST_VARS.pre-patch= VARNAME")
+
+ t.ExpectDiagnosticsAutofix(
+ doTest,
+ "WARN: filename.mk:2: Substitutions should not happen in the patch phase.")
+
+ t.CheckEquals(t.File("filename.mk").IsFile(), false)
}
-func (s *Suite) Test_SubstContext__with_NO_CONFIGURE(c *check.C) {
+func (s *Suite) Test_SubstContext_varassignStage__with_NO_CONFIGURE(c *check.C) {
t := s.Init(c)
pkg := t.SetUpPackage("category/package",
@@ -371,7 +572,7 @@ func (s *Suite) Test_SubstContext__with_
"when NO_CONFIGURE is set (in line 35).")
}
-func (s *Suite) Test_SubstContext__without_NO_CONFIGURE(c *check.C) {
+func (s *Suite) Test_SubstContext_varassignStage__without_NO_CONFIGURE(c *check.C) {
t := s.Init(c)
pkg := t.SetUpPackage("category/package",
@@ -386,251 +587,167 @@ func (s *Suite) Test_SubstContext__witho
t.CheckOutputEmpty()
}
-func (s *Suite) Test_SubstContext__adjacent(c *check.C) {
+// Before 2019-12-12, pkglint wrongly warned about variables that were
+// not obviously SUBST variables, even if they were used later in SUBST_VARS.
+func (s *Suite) Test_SubstContext_varassignVars__var_before_SUBST_VARS(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Wextra")
- t.SetUpVartypes()
-
- mklines := t.NewMkLines("os.mk",
- MkCvsID,
+ t.RunSubst(
+ "SUBST_CLASSES+= id",
+ "SUBST_STAGE.id= post-configure",
+ "SUBST_FILES.id= files",
+ "FOREIGN= not mentioned in SUBST_VARS",
+ "VAR= ok",
+ "SUBST_VARS.id= VAR",
"",
- "SUBST_CLASSES+=\t1",
- "SUBST_STAGE.1=\tpre-configure",
- "SUBST_FILES.1=\tfile1",
- "SUBST_SED.1=\t-e s,subst1,repl1,",
- "SUBST_CLASSES+=\t2",
- "SUBST_SED.1+=\t-e s,subst1b,repl1b,", // Misplaced
- "SUBST_STAGE.2=\tpre-configure",
- "SUBST_FILES.2=\tfile2",
- "SUBST_SED.2=\t-e s,subst2,repl2,")
-
- mklines.Check()
+ // This second block makes sure that the list of foreign variables
+ // is properly reset at the end of a SUBST block.
+ // If it weren't, there would be additional warnings.
+ "SUBST_CLASSES+= 2",
+ "SUBST_STAGE.2= post-configure",
+ "SUBST_FILES.2= files",
+ "SUBST_VARS.2= OTHER")
t.CheckOutputLines(
- "WARN: os.mk:8: Variable \"SUBST_SED.1\" does not match SUBST class \"2\".")
+ "WARN: filename.mk:4: Foreign variable \"FOREIGN\" in SUBST block.")
}
-func (s *Suite) Test_SubstContext__do_patch(c *check.C) {
+func (s *Suite) Test_SubstContext_dupList__conditional_before_unconditional(c *check.C) {
t := s.Init(c)
- t.SetUpVartypes()
-
- mklines := t.NewMkLines("os.mk",
- MkCvsID,
- "",
- "SUBST_CLASSES+=\tos",
- "SUBST_STAGE.os=\tdo-patch",
- "SUBST_FILES.os=\tguess-os.h",
- "SUBST_SED.os=\t-e s,@OPSYS@,Darwin,")
-
- mklines.Check()
+ t.RunSubst(
+ "SUBST_CLASSES+= os",
+ "SUBST_STAGE.os= post-configure",
+ ".if 1",
+ "SUBST_FILES.os= conditional",
+ ".endif",
+ "SUBST_FILES.os= unconditional",
+ "SUBST_VARS.os= OPSYS")
- // No warning, since there is nothing to fix automatically.
- // This case also doesn't occur in practice.
+ // TODO: Warn that the conditional line is overwritten.
t.CheckOutputEmpty()
}
-// Variables mentioned in SUBST_VARS are not considered "foreign"
-// in the block and may be mixed with the other SUBST variables.
-func (s *Suite) Test_SubstContext__SUBST_VARS_defined_in_block(c *check.C) {
+func (s *Suite) Test_SubstContext_suggestSubstVars(c *check.C) {
t := s.Init(c)
- t.SetUpVartypes()
-
- mklines := t.NewMkLines("os.mk",
- MkCvsID,
- "",
- "SUBST_CLASSES+=\tos",
- "SUBST_STAGE.os=\tpre-configure",
- "SUBST_FILES.os=\tguess-os.h",
- "SUBST_VARS.os=\tTODAY1",
- "TODAY1!=\tdate",
- "TODAY2!=\tdate")
-
- mklines.Check()
-
- t.CheckOutputLines(
- "WARN: os.mk:8: TODAY2 is defined but not used.",
- "WARN: os.mk:8: Foreign variable \"TODAY2\" in SUBST block.")
-}
+ t.Chdir(".")
-// Variables mentioned in SUBST_VARS may appear in the same paragraph,
-// or alternatively anywhere else in the file.
-func (s *Suite) Test_SubstContext__SUBST_VARS_in_next_paragraph(c *check.C) {
- t := s.Init(c)
+ test := func(line string, diagnostics ...string) {
+ doTest := t.NewSubstAutofixTest(
+ "SUBST_CLASSES+=\t\ttest",
+ "SUBST_STAGE.test=\tpre-configure",
+ "SUBST_FILES.test=\tfilename",
+ line)
+
+ t.ExpectDiagnosticsAutofix(
+ doTest,
+ diagnostics...)
+ }
- t.SetUpVartypes()
+ // Can be replaced.
+ test(
+ "SUBST_SED.test+=\t-e s,@SH@,${SH},g",
- mklines := t.NewMkLines("os.mk",
- MkCvsID,
- "",
- "SUBST_CLASSES+=\tos",
- "SUBST_STAGE.os=\tpre-configure",
- "SUBST_FILES.os=\tguess-os.h",
- "SUBST_VARS.os=\tTODAY1",
- "",
- "TODAY1!=\tdate",
- "TODAY2!=\tdate")
+ "NOTE: filename.mk:4: The substitution command \"s,@SH@,${SH},g\" "+
+ "can be replaced with \"SUBST_VARS.test= SH\".",
+ "AUTOFIX: filename.mk:4: Replacing \"SUBST_SED.test+=\\t-e s,@SH@,${SH},g\" "+
+ "with \"SUBST_VARS.test=\\tSH\".")
- mklines.Check()
+ // Can be replaced, with or without the :Q modifier.
+ test(
+ "SUBST_SED.test+=\t-e s,@SH@,${SH:Q},g",
- t.CheckOutputLines(
- "WARN: os.mk:9: TODAY2 is defined but not used.")
-}
+ "NOTE: filename.mk:4: The substitution command \"s,@SH@,${SH:Q},g\" "+
+ "can be replaced with \"SUBST_VARS.test= SH\".",
+ "AUTOFIX: filename.mk:4: Replacing \"SUBST_SED.test+=\\t-e s,@SH@,${SH:Q},g\" "+
+ "with \"SUBST_VARS.test=\\tSH\".")
-func (s *Suite) Test_SubstContext__multiple_SUBST_VARS(c *check.C) {
- t := s.Init(c)
+ // Cannot be replaced because of the :T modifier.
+ test(
+ "SUBST_SED.test+=\t-e s,@SH@,${SH:T},g",
- t.SetUpCommandLine("-Wextra")
- t.SetUpVartypes()
+ nil...)
- mklines := t.NewMkLines("os.mk",
- MkCvsID,
- "",
- "SUBST_CLASSES+=\tos",
- "SUBST_STAGE.os=\tpre-configure",
- "SUBST_FILES.os=\tguess-os.h",
- "SUBST_VARS.os=\tPREFIX VARBASE")
+ // Can be replaced, even without the g option.
+ test(
+ "SUBST_SED.test+=\t-e s,@SH@,${SH},",
- mklines.Check()
+ "NOTE: filename.mk:4: The substitution command \"s,@SH@,${SH},\" "+
+ "can be replaced with \"SUBST_VARS.test= SH\".",
+ "AUTOFIX: filename.mk:4: Replacing \"SUBST_SED.test+=\\t-e s,@SH@,${SH},\" "+
+ "with \"SUBST_VARS.test=\\tSH\".")
- t.CheckOutputEmpty()
-}
+ // Can be replaced, whether in single quotes or not.
+ test(
+ "SUBST_SED.test+=\t-e 's,@SH@,${SH},'",
-// As of May 2019, pkglint does not check the order of the variables in
-// a SUBST block. Enforcing this order, or at least suggesting it, would
-// make pkgsrc packages more uniform, which is a good idea, but not urgent.
-func (s *Suite) Test_SubstContext__unusual_variable_order(c *check.C) {
- t := s.Init(c)
+ "NOTE: filename.mk:4: The substitution command \"'s,@SH@,${SH},'\" "+
+ "can be replaced with \"SUBST_VARS.test= SH\".",
+ "AUTOFIX: filename.mk:4: Replacing \"SUBST_SED.test+=\\t-e 's,@SH@,${SH},'\" "+
+ "with \"SUBST_VARS.test=\\tSH\".")
- t.SetUpVartypes()
+ // Can be replaced, whether in double quotes or not.
+ test(
+ "SUBST_SED.test+=\t-e \"s,@SH@,${SH},\"",
- mklines := t.NewMkLines("subst.mk",
- MkCvsID,
- "",
- "SUBST_CLASSES+=\t\tid",
- "SUBST_SED.id=\t\t-e /deleteme/d",
- "SUBST_FILES.id=\t\tfile",
- "SUBST_MESSAGE.id=\tMessage",
- "SUBST_STAGE.id=\t\tpre-configure")
+ "NOTE: filename.mk:4: The substitution command \"\\\"s,@SH@,${SH},\\\"\" "+
+ "can be replaced with \"SUBST_VARS.test= SH\".",
+ "AUTOFIX: filename.mk:4: Replacing \"SUBST_SED.test+=\\t-e \\\"s,@SH@,${SH},\\\"\" "+
+ "with \"SUBST_VARS.test=\\tSH\".")
- mklines.Check()
+ // Can be replaced, even when the quoting changes midways.
+ test(
+ "SUBST_SED.test+=\t-e s,'@SH@','${SH}',",
- t.CheckOutputEmpty()
-}
+ "NOTE: filename.mk:4: The substitution command \"s,'@SH@','${SH}',\" "+
+ "can be replaced with \"SUBST_VARS.test= SH\".",
+ "AUTOFIX: filename.mk:4: Replacing \"SUBST_SED.test+=\\t-e s,'@SH@','${SH}',\" "+
+ "with \"SUBST_VARS.test=\\tSH\".")
-// Since the SUBST_CLASSES definition starts the SUBST block, all
-// directives above it are ignored by the SUBST context.
-func (s *Suite) Test_SubstContext_Directive__before_SUBST_CLASSES(c *check.C) {
- t := s.Init(c)
+ // Can be replaced manually, even when the -e is missing.
+ test(
+ "SUBST_SED.test+=\ts,'@SH@','${SH}',",
+ "NOTE: filename.mk:4: The substitution command \"s,'@SH@','${SH}',\" "+
+ "can be replaced with \"SUBST_VARS.test= SH\".")
- t.SetUpVartypes()
- t.DisableTracing() // Just for branch coverage.
+ // Cannot be replaced since the variable name differs.
+ test(
+ "SUBST_SED.test+=\t-e s,@SH@,${PKGNAME},",
- mklines := t.NewMkLines("os.mk",
- MkCvsID,
- "",
- ".if 0",
- ".endif",
- "SUBST_CLASSES+=\tos",
- ".elif 0") // Just for branch coverage.
+ nil...)
- mklines.Check()
+ // Cannot be replaced since the double quotes are added.
+ test(
+ "SUBST_SED.test+=\t-e s,@SH@,'\"'${SH:Q}'\"',g",
- t.CheckOutputLines(
- "WARN: os.mk:EOF: Incomplete SUBST block: SUBST_STAGE.os missing.",
- "WARN: os.mk:EOF: Incomplete SUBST block: SUBST_FILES.os missing.",
- "WARN: os.mk:EOF: Incomplete SUBST block: "+
- "SUBST_SED.os, SUBST_VARS.os or SUBST_FILTER_CMD.os missing.")
-}
+ nil...)
-func (s *Suite) Test_SubstContext_suggestSubstVars(c *check.C) {
- t := s.Init(c)
+ // Just to get 100% code coverage.
+ test(
+ "SUBST_SED.test+=\t-e s",
- t.SetUpVartypes()
- t.SetUpTool("sh", "SH", AtRunTime)
+ nil...)
- mklines := t.NewMkLines("subst.mk",
- MkCvsID,
- "",
- "SUBST_CLASSES+=\t\ttest",
- "SUBST_STAGE.test=\tpre-configure",
- "SUBST_FILES.test=\tfilename",
- "SUBST_SED.test+=\t-e s,@SH@,${SH},g", // Can be replaced.
- "SUBST_SED.test+=\t-e s,@SH@,${SH:Q},g", // Can be replaced, with or without the :Q modifier.
- "SUBST_SED.test+=\t-e s,@SH@,${SH:T},g", // Cannot be replaced because of the :T modifier.
- "SUBST_SED.test+=\t-e s,@SH@,${SH},", // Can be replaced, even without the g option.
- "SUBST_SED.test+=\t-e 's,@SH@,${SH},'", // Can be replaced, whether in single quotes or not.
- "SUBST_SED.test+=\t-e \"s,@SH@,${SH},\"", // Can be replaced, whether in double quotes or not.
- "SUBST_SED.test+=\t-e s,'@SH@','${SH}',", // Can be replaced, even when the quoting changes midways.
- "SUBST_SED.test+=\ts,'@SH@','${SH}',", // Can be replaced manually, even when the -e is missing.
- "SUBST_SED.test+=\t-e s,@SH@,${PKGNAME},", // Cannot be replaced since the variable name differs.
- "SUBST_SED.test+=\t-e s,@SH@,'\"'${SH:Q}'\"',g", // Cannot be replaced since the double quotes are added.
- "SUBST_SED.test+=\t-e s", // Just to get 100% code coverage.
- "SUBST_SED.test+=\t-e s,@SH@,${SH:Q}", // Just to get 100% code coverage.
- "SUBST_SED.test+=\t-e s,@SH@,${SH:Q}, # comment", // Just a note; not fixed because of the comment.
- "SUBST_SED.test+=\t-n s,@SH@,${SH:Q},", // Just a note; not fixed because of the -n.
- "# end")
+ // Just to get 100% code coverage.
+ test(
+ "SUBST_SED.test+=\t-e s,@SH@,${SH:Q}",
- mklines.Check()
+ nil...)
- t.CheckOutputLines(
- "WARN: subst.mk:6: Please use ${SH:Q} instead of ${SH}.",
- "NOTE: subst.mk:6: The substitution command \"s,@SH@,${SH},g\" "+
- "can be replaced with \"SUBST_VARS.test= SH\".",
- "NOTE: subst.mk:7: The substitution command \"s,@SH@,${SH:Q},g\" "+
- "can be replaced with \"SUBST_VARS.test+= SH\".",
- "WARN: subst.mk:8: Please use ${SH:T:Q} instead of ${SH:T}.",
- "WARN: subst.mk:9: Please use ${SH:Q} instead of ${SH}.",
- "NOTE: subst.mk:9: The substitution command \"s,@SH@,${SH},\" "+
- "can be replaced with \"SUBST_VARS.test+= SH\".",
- "NOTE: subst.mk:10: The substitution command \"'s,@SH@,${SH},'\" "+
- "can be replaced with \"SUBST_VARS.test+= SH\".",
- "NOTE: subst.mk:11: The substitution command \"\\\"s,@SH@,${SH},\\\"\" "+
- "can be replaced with \"SUBST_VARS.test+= SH\".",
- "NOTE: subst.mk:12: The substitution command \"s,'@SH@','${SH}',\" "+
- "can be replaced with \"SUBST_VARS.test+= SH\".",
- "NOTE: subst.mk:13: Please always use \"-e\" in sed commands, "+
- "even if there is only one substitution.",
- "NOTE: subst.mk:13: The substitution command \"s,'@SH@','${SH}',\" "+
- "can be replaced with \"SUBST_VARS.test+= SH\".",
- "NOTE: subst.mk:18: The substitution command \"s,@SH@,${SH:Q},\" "+
- "can be replaced with \"SUBST_VARS.test+= SH\".",
- "NOTE: subst.mk:19: Please always use \"-e\" in sed commands, "+
- "even if there is only one substitution.",
- "NOTE: subst.mk:19: The substitution command \"s,@SH@,${SH:Q},\" "+
- "can be replaced with \"SUBST_VARS.test+= SH\".")
+ // Just a note; not fixed because of the comment.
+ test(
+ "SUBST_SED.test+=\t-e s,@SH@,${SH:Q}, # comment",
- t.SetUpCommandLine("--show-autofix")
+ "NOTE: filename.mk:4: The substitution command \"s,@SH@,${SH:Q},\" "+
+ "can be replaced with \"SUBST_VARS.test= SH\".")
- mklines.Check()
+ // Just a note; not fixed because of the -n.
+ test(
+ "SUBST_SED.test+=\t-n s,@SH@,${SH:Q},",
- t.CheckOutputLines(
- "NOTE: subst.mk:6: The substitution command \"s,@SH@,${SH},g\" "+
- "can be replaced with \"SUBST_VARS.test= SH\".",
- "AUTOFIX: subst.mk:6: Replacing \"SUBST_SED.test+=\\t-e s,@SH@,${SH},g\" "+
- "with \"SUBST_VARS.test=\\tSH\".",
- "NOTE: subst.mk:7: The substitution command \"s,@SH@,${SH:Q},g\" "+
- "can be replaced with \"SUBST_VARS.test+= SH\".",
- "AUTOFIX: subst.mk:7: Replacing \"SUBST_SED.test+=\\t-e s,@SH@,${SH:Q},g\" "+
- "with \"SUBST_VARS.test+=\\tSH\".",
- "NOTE: subst.mk:9: The substitution command \"s,@SH@,${SH},\" "+
- "can be replaced with \"SUBST_VARS.test+= SH\".",
- "AUTOFIX: subst.mk:9: Replacing \"SUBST_SED.test+=\\t-e s,@SH@,${SH},\" "+
- "with \"SUBST_VARS.test+=\\tSH\".",
- "NOTE: subst.mk:10: The substitution command \"'s,@SH@,${SH},'\" "+
- "can be replaced with \"SUBST_VARS.test+= SH\".",
- "AUTOFIX: subst.mk:10: Replacing \"SUBST_SED.test+=\\t-e 's,@SH@,${SH},'\" "+
- "with \"SUBST_VARS.test+=\\tSH\".",
- "NOTE: subst.mk:11: The substitution command \"\\\"s,@SH@,${SH},\\\"\" "+
- "can be replaced with \"SUBST_VARS.test+= SH\".",
- "AUTOFIX: subst.mk:11: Replacing \"SUBST_SED.test+=\\t-e \\\"s,@SH@,${SH},\\\"\" "+
- "with \"SUBST_VARS.test+=\\tSH\".",
- "NOTE: subst.mk:12: The substitution command \"s,'@SH@','${SH}',\" "+
- "can be replaced with \"SUBST_VARS.test+= SH\".",
- "AUTOFIX: subst.mk:12: Replacing \"SUBST_SED.test+=\\t-e s,'@SH@','${SH}',\" "+
- "with \"SUBST_VARS.test+=\\tSH\".")
+ "NOTE: filename.mk:4: The substitution command \"s,@SH@,${SH:Q},\" "+
+ "can be replaced with \"SUBST_VARS.test= SH\".")
}
// If the SUBST_CLASS identifier ends with a plus, the generated code must
@@ -638,39 +755,32 @@ func (s *Suite) Test_SubstContext_sugges
func (s *Suite) Test_SubstContext_suggestSubstVars__plus(c *check.C) {
t := s.Init(c)
- t.SetUpVartypes()
- t.SetUpTool("sh", "SH", AtRunTime)
+ t.Chdir(".")
- mklines := t.NewMkLines("subst.mk",
- MkCvsID,
- "",
+ doTest := t.NewSubstAutofixTest(
"SUBST_CLASSES+=\t\tgtk+",
"SUBST_STAGE.gtk+ =\tpre-configure",
"SUBST_FILES.gtk+ =\tfilename",
"SUBST_SED.gtk+ +=\t-e s,@SH@,${SH:Q},g",
"SUBST_SED.gtk+ +=\t-e s,@SH@,${SH:Q},g")
- mklines.Check()
-
- t.CheckOutputLines(
- "NOTE: subst.mk:6: The substitution command \"s,@SH@,${SH:Q},g\" "+
+ t.ExpectDiagnosticsAutofix(
+ doTest,
+ "NOTE: filename.mk:4: The substitution command \"s,@SH@,${SH:Q},g\" "+
"can be replaced with \"SUBST_VARS.gtk+ = SH\".",
- "NOTE: subst.mk:7: The substitution command \"s,@SH@,${SH:Q},g\" "+
- "can be replaced with \"SUBST_VARS.gtk+ += SH\".")
-
- t.SetUpCommandLine("--show-autofix")
-
- mklines.Check()
-
- t.CheckOutputLines(
- "NOTE: subst.mk:6: The substitution command \"s,@SH@,${SH:Q},g\" "+
- "can be replaced with \"SUBST_VARS.gtk+ = SH\".",
- "AUTOFIX: subst.mk:6: Replacing \"SUBST_SED.gtk+ +=\\t-e s,@SH@,${SH:Q},g\" "+
- "with \"SUBST_VARS.gtk+ =\\tSH\".",
- "NOTE: subst.mk:7: The substitution command \"s,@SH@,${SH:Q},g\" "+
+ "NOTE: filename.mk:5: The substitution command \"s,@SH@,${SH:Q},g\" "+
"can be replaced with \"SUBST_VARS.gtk+ += SH\".",
- "AUTOFIX: subst.mk:7: Replacing \"SUBST_SED.gtk+ +=\\t-e s,@SH@,${SH:Q},g\" "+
+ "AUTOFIX: filename.mk:4: Replacing \"SUBST_SED.gtk+ +=\\t-e s,@SH@,${SH:Q},g\" "+
+ "with \"SUBST_VARS.gtk+ =\\tSH\".",
+ "AUTOFIX: filename.mk:5: Replacing \"SUBST_SED.gtk+ +=\\t-e s,@SH@,${SH:Q},g\" "+
"with \"SUBST_VARS.gtk+ +=\\tSH\".")
+
+ t.CheckFileLinesDetab("filename.mk",
+ "SUBST_CLASSES+= gtk+",
+ "SUBST_STAGE.gtk+ = pre-configure",
+ "SUBST_FILES.gtk+ = filename",
+ "SUBST_VARS.gtk+ = SH",
+ "SUBST_VARS.gtk+ += SH")
}
// The last of the SUBST_SED variables is 15 characters wide. When SUBST_SED
@@ -679,39 +789,28 @@ func (s *Suite) Test_SubstContext_sugges
func (s *Suite) Test_SubstContext_suggestSubstVars__autofix_realign_paragraph(c *check.C) {
t := s.Init(c)
- t.SetUpVartypes()
t.Chdir(".")
- mklines := t.SetUpFileMkLines("subst.mk",
- MkCvsID,
- "",
+ doTest := t.NewSubstAutofixTest(
"SUBST_CLASSES+=\t\tpfx",
"SUBST_STAGE.pfx=\tpre-configure",
"SUBST_FILES.pfx=\tfilename",
"SUBST_SED.pfx=\t\t-e s,@PREFIX@,${PREFIX},g",
"SUBST_SED.pfx+=\t\t-e s,@PREFIX@,${PREFIX},g")
- mklines.Check()
+ t.ExpectDiagnosticsAutofix(
+ doTest,
- t.CheckOutputLines(
- "NOTE: subst.mk:6: The substitution command \"s,@PREFIX@,${PREFIX},g\" "+
+ "NOTE: filename.mk:4: The substitution command \"s,@PREFIX@,${PREFIX},g\" "+
"can be replaced with \"SUBST_VARS.pfx= PREFIX\".",
- "NOTE: subst.mk:7: The substitution command \"s,@PREFIX@,${PREFIX},g\" "+
- "can be replaced with \"SUBST_VARS.pfx+= PREFIX\".")
-
- t.SetUpCommandLine("--autofix")
-
- mklines.Check()
-
- t.CheckOutputLines(
- "AUTOFIX: subst.mk:6: Replacing \"SUBST_SED.pfx=\\t\\t-e s,@PREFIX@,${PREFIX},g\" "+
+ "NOTE: filename.mk:5: The substitution command \"s,@PREFIX@,${PREFIX},g\" "+
+ "can be replaced with \"SUBST_VARS.pfx+= PREFIX\".",
+ "AUTOFIX: filename.mk:4: Replacing \"SUBST_SED.pfx=\\t\\t-e s,@PREFIX@,${PREFIX},g\" "+
"with \"SUBST_VARS.pfx=\\t\\tPREFIX\".",
- "AUTOFIX: subst.mk:7: Replacing \"SUBST_SED.pfx+=\\t\\t-e s,@PREFIX@,${PREFIX},g\" "+
+ "AUTOFIX: filename.mk:5: Replacing \"SUBST_SED.pfx+=\\t\\t-e s,@PREFIX@,${PREFIX},g\" "+
"with \"SUBST_VARS.pfx+=\\tPREFIX\".")
- t.CheckFileLinesDetab("subst.mk",
- MkCvsID,
- "",
+ t.CheckFileLinesDetab("filename.mk",
"SUBST_CLASSES+= pfx",
"SUBST_STAGE.pfx= pre-configure",
"SUBST_FILES.pfx= filename",
@@ -722,103 +821,89 @@ func (s *Suite) Test_SubstContext_sugges
func (s *Suite) Test_SubstContext_suggestSubstVars__autofix_plus_sed(c *check.C) {
t := s.Init(c)
- t.SetUpVartypes()
t.Chdir(".")
- mklines := t.SetUpFileMkLines("subst.mk",
- MkCvsID,
- "",
+ doTest := t.NewSubstAutofixTest(
"SUBST_CLASSES+=\t\tpfx",
"SUBST_STAGE.pfx=\tpre-configure",
"SUBST_FILES.pfx=\tfilename",
"SUBST_SED.pfx=\t\t-e s,@PREFIX@,${PREFIX},g",
"SUBST_SED.pfx+=\t\t-e s,@PREFIX@,other,g")
- mklines.Check()
-
- t.CheckOutputLines(
- "NOTE: subst.mk:6: The substitution command \"s,@PREFIX@,${PREFIX},g\" " +
- "can be replaced with \"SUBST_VARS.pfx= PREFIX\".")
-
- t.SetUpCommandLine("-Wall", "--autofix")
+ t.ExpectDiagnosticsAutofix(
+ doTest,
- mklines.Check()
-
- t.CheckOutputLines(
- "AUTOFIX: subst.mk:6: Replacing \"SUBST_SED.pfx=\\t\\t-e s,@PREFIX@,${PREFIX},g\" " +
+ "NOTE: filename.mk:4: The substitution command \"s,@PREFIX@,${PREFIX},g\" "+
+ "can be replaced with \"SUBST_VARS.pfx= PREFIX\".",
+ "AUTOFIX: filename.mk:4: "+
+ "Replacing \"SUBST_SED.pfx=\\t\\t-e s,@PREFIX@,${PREFIX},g\" "+
"with \"SUBST_VARS.pfx=\\t\\tPREFIX\".")
- t.CheckFileLinesDetab("subst.mk",
- MkCvsID,
- "",
+ t.CheckFileLinesDetab("filename.mk",
"SUBST_CLASSES+= pfx",
"SUBST_STAGE.pfx= pre-configure",
"SUBST_FILES.pfx= filename",
"SUBST_VARS.pfx= PREFIX",
- // TODO: If this subst class is used nowhere else, pkglint could
- // replace this += with a simple =.
+ // Since the SUBST_SED that was previously here used the = operator,
+ // this += might be replaced with a simple =.
"SUBST_SED.pfx+= -e s,@PREFIX@,other,g")
}
func (s *Suite) Test_SubstContext_suggestSubstVars__autofix_plus_vars(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Wall", "--autofix")
- t.SetUpVartypes()
t.Chdir(".")
- mklines := t.SetUpFileMkLines("subst.mk",
- MkCvsID,
- "",
+ doTest := t.NewSubstAutofixTest(
"SUBST_CLASSES+=\tid",
"SUBST_STAGE.id=\tpre-configure",
"SUBST_FILES.id=\tfilename",
"SUBST_SED.id=\t-e s,@PREFIX@,${PREFIX},g",
"SUBST_VARS.id=\tPKGMANDIR")
- mklines.Check()
+ t.ExpectDiagnosticsAutofix(
+ doTest,
- t.CheckOutputLines(
- "AUTOFIX: subst.mk:6: Replacing \"SUBST_SED.id=\\t-e s,@PREFIX@,${PREFIX},g\" " +
- "with \"SUBST_VARS.id=\\tPREFIX\".")
+ "NOTE: filename.mk:4: The substitution command \"s,@PREFIX@,${PREFIX},g\" "+
+ "can be replaced with \"SUBST_VARS.id= PREFIX\".",
+ "AUTOFIX: filename.mk:4: "+
+ "Replacing \"SUBST_SED.id=\\t-e s,@PREFIX@,${PREFIX},g\" "+
+ "with \"SUBST_VARS.id=\\tPREFIX\".",
+ "AUTOFIX: filename.mk:5: "+
+ "Replacing \"SUBST_VARS.id=\\t\" "+
+ "with \"SUBST_VARS.id+=\\t\".")
- t.CheckFileLinesDetab("subst.mk",
- MkCvsID,
- "",
+ t.CheckFileLinesDetab("filename.mk",
"SUBST_CLASSES+= id",
"SUBST_STAGE.id= pre-configure",
"SUBST_FILES.id= filename",
"SUBST_VARS.id= PREFIX",
- // FIXME: This must be += instead of = since the previous line already uses =.
- // Luckily the check for redundant assignments catches this already.
- "SUBST_VARS.id= PKGMANDIR")
+ "SUBST_VARS.id+= PKGMANDIR")
}
func (s *Suite) Test_SubstContext_suggestSubstVars__autofix_indentation(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Wall", "--autofix")
- t.SetUpVartypes()
t.Chdir(".")
- mklines := t.SetUpFileMkLines("subst.mk",
- MkCvsID,
- "",
+ doTest := t.NewSubstAutofixTest(
"SUBST_CLASSES+=\t\t\tfix-paths",
"SUBST_STAGE.fix-paths=\t\tpre-configure",
"SUBST_MESSAGE.fix-paths=\tMessage",
"SUBST_FILES.fix-paths=\t\tfilename",
"SUBST_SED.fix-paths=\t\t-e s,@PREFIX@,${PREFIX},g")
- mklines.Check()
+ t.ExpectDiagnosticsAutofix(
+ doTest,
- t.CheckOutputLines(
- "AUTOFIX: subst.mk:7: Replacing \"SUBST_SED.fix-paths=\\t\\t-e s,@PREFIX@,${PREFIX},g\" " +
+ "NOTE: filename.mk:5: "+
+ "The substitution command \"s,@PREFIX@,${PREFIX},g\" "+
+ "can be replaced with \"SUBST_VARS.fix-paths= PREFIX\".",
+ "AUTOFIX: filename.mk:5: Replacing "+
+ "\"SUBST_SED.fix-paths=\\t\\t-e s,@PREFIX@,${PREFIX},g\" "+
"with \"SUBST_VARS.fix-paths=\\t\\tPREFIX\".")
- t.CheckFileLinesDetab("subst.mk",
- MkCvsID,
- "",
+ t.CheckFileLinesDetab("filename.mk",
"SUBST_CLASSES+= fix-paths",
"SUBST_STAGE.fix-paths= pre-configure",
"SUBST_MESSAGE.fix-paths= Message",
@@ -826,6 +911,42 @@ func (s *Suite) Test_SubstContext_sugges
"SUBST_VARS.fix-paths= PREFIX")
}
+func (s *Suite) Test_SubstContext_suggestSubstVars__conditional(c *check.C) {
+ t := s.Init(c)
+
+ t.Chdir(".")
+
+ doTest := t.NewSubstAutofixTest(
+ "SUBST_CLASSES+= id",
+ "SUBST_STAGE.id= pre-configure",
+ "SUBST_FILES.id= files",
+ "SUBST_SED.id= -e s,@VAR@,${VAR},",
+ ".if 1",
+ "SUBST_SED.id+= -e s,@VAR2@,${VAR2},",
+ ".endif")
+
+ t.ExpectDiagnosticsAutofix(
+ doTest,
+
+ "NOTE: filename.mk:4: The substitution command \"s,@VAR@,${VAR},\" "+
+ "can be replaced with \"SUBST_VARS.id= VAR\".",
+ "NOTE: filename.mk:6: The substitution command \"s,@VAR2@,${VAR2},\" "+
+ "can be replaced with \"SUBST_VARS.id+= VAR2\".",
+ "AUTOFIX: filename.mk:4: Replacing \"SUBST_SED.id= -e s,@VAR@,${VAR},\" "+
+ "with \"SUBST_VARS.id=\\tVAR\".",
+ "AUTOFIX: filename.mk:6: Replacing \"SUBST_SED.id+= -e s,@VAR2@,${VAR2},\" "+
+ "with \"SUBST_VARS.id+=\\tVAR2\".")
+
+ t.CheckFileLinesDetab("filename.mk",
+ "SUBST_CLASSES+= id",
+ "SUBST_STAGE.id= pre-configure",
+ "SUBST_FILES.id= files",
+ "SUBST_VARS.id= VAR",
+ ".if 1",
+ "SUBST_VARS.id+= VAR2",
+ ".endif")
+}
+
func (s *Suite) Test_SubstContext_extractVarname(c *check.C) {
t := s.Init(c)
@@ -873,34 +994,361 @@ func (s *Suite) Test_SubstContext_extrac
test("s,@VAR@,${VAR}suffix,", "")
}
-// simulateSubstLines only tests some of the inner workings of SubstContext.
-// It is not realistic for all cases. If in doubt, use MkLines.Check.
-func simulateSubstLines(t *Tester, texts ...string) {
+func (s *Suite) Test_SubstContext_directive__before_SUBST_CLASSES(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ ".if 0",
+ ".endif",
+ "SUBST_CLASSES+=\tos",
+ ".elif 0") // Just for branch coverage.
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:EOF: Missing SUBST block for \"os\".")
+}
+
+func (s *Suite) Test_SubstContext_directive__conditional_blocks_complete(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_CLASSES+= nb",
+ "SUBST_STAGE.nb= post-configure",
+ "SUBST_FILES.nb= guess-netbsd.h",
+ "SUBST_VARS.nb= HAVE_NETBSD",
+ ".else",
+ "SUBST_CLASSES+= os",
+ "SUBST_STAGE.os= post-configure",
+ "SUBST_FILES.os= guess-netbsd.h",
+ "SUBST_VARS.os= HAVE_OTHER",
+ ".endif")
+
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_SubstContext_directive__conditional_blocks_incomplete(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_CLASSES+= nb",
+ "SUBST_STAGE.nb= post-configure",
+ "SUBST_VARS.nb= HAVE_NETBSD",
+ ".else",
+ "SUBST_CLASSES+= os",
+ "SUBST_STAGE.os= post-configure",
+ "SUBST_FILES.os= guess-netbsd.h",
+ ".endif")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:5: Incomplete SUBST block: SUBST_FILES.nb missing.",
+ "WARN: filename.mk:9: Incomplete SUBST block: "+
+ "SUBST_SED.os, SUBST_VARS.os or SUBST_FILTER_CMD.os missing.")
+}
+
+func (s *Suite) Test_SubstContext_directive__conditional_complete(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= id",
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_STAGE.id=\t\tpost-configure",
+ "SUBST_MESSAGE.id=\tpost-configure",
+ "SUBST_FILES.id=\t\tguess-netbsd.h",
+ "SUBST_SED.id=\t\t-e s,from,to,",
+ "SUBST_VARS.id=\t\tHAVE_OTHER",
+ "SUBST_FILTER_CMD.id=\tHAVE_OTHER",
+ ".else",
+ "SUBST_STAGE.id=\t\tpost-configure",
+ "SUBST_MESSAGE.id=\tpost-configure",
+ "SUBST_FILES.id=\t\tguess-netbsd.h",
+ "SUBST_SED.id=\t\t-e s,from,to,",
+ "SUBST_VARS.id=\t\tHAVE_OTHER",
+ "SUBST_FILTER_CMD.id=\tHAVE_OTHER",
+ ".endif")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:3: SUBST_STAGE.id should not be defined conditionally.",
+ "WARN: filename.mk:4: SUBST_MESSAGE.id should not be defined conditionally.",
+ "WARN: filename.mk:10: SUBST_STAGE.id should not be defined conditionally.",
+ "WARN: filename.mk:11: SUBST_MESSAGE.id should not be defined conditionally.")
+}
+
+func (s *Suite) Test_SubstContext_directive__conditionally_overwritten_filter(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= id",
+ "SUBST_STAGE.id=\t\tpost-configure",
+ "SUBST_MESSAGE.id=\tpost-configure",
+ "SUBST_FILES.id=\t\tguess-netbsd.h",
+ "SUBST_FILTER_CMD.id=\tHAVE_OTHER",
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_FILTER_CMD.id=\tHAVE_OTHER",
+ ".endif")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:7: Duplicate definition of \"SUBST_FILTER_CMD.id\".")
+}
+
+// Hopefully nobody will ever trigger this case in real pkgsrc.
+// It's plain confusing to a casual reader to nest a complete
+// SUBST block into another SUBST block.
+// That's why pkglint doesn't cover this case correctly.
+func (s *Suite) Test_SubstContext_directive__conditionally_nested_block(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= outer",
+ "SUBST_STAGE.outer= post-configure",
+ "SUBST_FILES.outer= outer.txt",
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_CLASSES+= inner",
+ "SUBST_STAGE.inner= post-configure",
+ "SUBST_FILES.inner= inner.txt",
+ "SUBST_VARS.inner= INNER",
+ ".endif",
+ "SUBST_VARS.outer= OUTER")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:5: Incomplete SUBST block: "+
+ "SUBST_SED.outer, SUBST_VARS.outer or SUBST_FILTER_CMD.outer missing.",
+ "WARN: filename.mk:5: Subst block \"outer\" should be finished "+
+ "before adding the next class to SUBST_CLASSES.",
+ "WARN: filename.mk:10: "+
+ "Late additions to a SUBST variable should use the += operator.")
+}
+
+// It's completely valid to have several SUBST blocks in a single paragraph.
+// As soon as a SUBST_CLASSES line appears, pkglint assumes that all previous
+// SUBST blocks are finished. That's exactly the case here.
+func (s *Suite) Test_SubstContext_directive__conditionally_following_block(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= outer",
+ "SUBST_STAGE.outer= post-configure",
+ "SUBST_FILES.outer= outer.txt",
+ "SUBST_VARS.outer= OUTER",
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_CLASSES+= middle",
+ "SUBST_STAGE.middle= post-configure",
+ "SUBST_FILES.middle= inner.txt",
+ "SUBST_VARS.middle= INNER",
+ ". if ${MACHINE_ARCH} == amd64",
+ "SUBST_CLASSES+= inner",
+ "SUBST_STAGE.inner= post-configure",
+ "SUBST_FILES.inner= inner.txt",
+ "SUBST_VARS.inner= INNER",
+ ". endif",
+ ".endif")
+
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_SubstContext_directive__two_blocks_in_condition(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_CLASSES+= a",
+ "SUBST_STAGE.a= post-configure",
+ "SUBST_FILES.a= outer.txt",
+ "SUBST_VARS.a= OUTER",
+ "SUBST_CLASSES+= b",
+ "SUBST_STAGE.b= post-configure",
+ "SUBST_FILES.b= inner.txt",
+ "SUBST_VARS.b= INNER",
+ ".endif")
+
+ // Up to 2019-12-12, pkglint wrongly warned in filename.mk:6:
+ // Subst block "a" should be finished before adding
+ // the next class to SUBST_CLASSES.
+ // The warning was wrong since block "a" has all required fields set.
+ // The warning was caused by an inconsistent check whether the current
+ // block had any conditional variables.
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_SubstContext_directive__nested_conditional_incomplete_block(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= outer",
+ "SUBST_STAGE.outer= post-configure",
+ "SUBST_FILES.outer= outer.txt",
+ "SUBST_VARS.outer= OUTER",
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_CLASSES+= inner1",
+ "SUBST_STAGE.inner1= post-configure",
+ "SUBST_VARS.inner1= INNER",
+ "SUBST_CLASSES+= inner2",
+ "SUBST_STAGE.inner2= post-configure",
+ "SUBST_FILES.inner2= inner.txt",
+ "SUBST_VARS.inner2= INNER",
+ ".endif")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:9: Incomplete SUBST block: SUBST_FILES.inner1 missing.",
+ "WARN: filename.mk:9: Subst block \"inner1\" should be finished "+
+ "before adding the next class to SUBST_CLASSES.")
+}
+
+func (s *Suite) Test_SubstContext_finishClass__details_in_then_branch(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= os",
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_VARS.os= OPSYS",
+ "SUBST_SED.os= -e s,@OPSYS@,NetBSD,",
+ "SUBST_STAGE.os= post-configure",
+ "SUBST_MESSAGE.os= Guessing operating system",
+ "SUBST_FILES.os= guess-os.h",
+ ".endif")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:5: SUBST_STAGE.os should not be defined conditionally.",
+ "WARN: filename.mk:6: SUBST_MESSAGE.os should not be defined conditionally.",
+ "WARN: filename.mk:EOF: Missing SUBST block for \"os\".")
+}
+
+func (s *Suite) Test_SubstContext_finishClass__details_in_else_branch(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= os",
+ ".if ${OPSYS} == NetBSD",
+ ".else",
+ "SUBST_VARS.os= OPSYS",
+ "SUBST_SED.os= -e s,@OPSYS@,NetBSD,",
+ "SUBST_STAGE.os= post-configure",
+ "SUBST_MESSAGE.os= Guessing operating system",
+ "SUBST_FILES.os= guess-os.h",
+ ".endif")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:6: SUBST_STAGE.os should not be defined conditionally.",
+ "WARN: filename.mk:7: SUBST_MESSAGE.os should not be defined conditionally.",
+ "WARN: filename.mk:EOF: Missing SUBST block for \"os\".")
+}
+
+func (s *Suite) Test_SubstContext_finishClass__empty_conditional_at_end(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= os",
+ "SUBST_VARS.os= OPSYS",
+ "SUBST_SED.os= -e s,@OPSYS@,NetBSD,",
+ "SUBST_STAGE.os= post-configure",
+ "SUBST_MESSAGE.os= Guessing operating system",
+ "SUBST_FILES.os= guess-os.h",
+ ".if ${OPSYS} == NetBSD",
+ ".else",
+ ".endif")
+
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_SubstContext_finishClass__missing_transformation_in_one_branch(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= os",
+ "SUBST_STAGE.os= post-configure",
+ "SUBST_MESSAGE.os= Guessing operating system",
+ "SUBST_FILES.os= guess-os.h",
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_FILES.os= -e s,@OpSYS@,NetBSD,", // A simple typo, this should be SUBST_SED.
+ ".elif ${OPSYS} == Darwin",
+ "SUBST_SED.os= -e s,@OPSYS@,Darwin1,",
+ "SUBST_SED.os= -e s,@OPSYS@,Darwin2,",
+ ".else",
+ "SUBST_VARS.os= OPSYS",
+ ".endif")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:6: All but the first assignment "+
+ "to \"SUBST_FILES.os\" should use the \"+=\" operator.",
+ "WARN: filename.mk:9: All but the first assignment "+
+ "to \"SUBST_SED.os\" should use the \"+=\" operator.",
+ "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_SED.os, "+
+ "SUBST_VARS.os or SUBST_FILTER_CMD.os missing.")
+}
+
+func (s *Suite) Test_SubstContext_finishClass__nested_conditionals(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= os",
+ "SUBST_STAGE.os= post-configure",
+ "SUBST_MESSAGE.os= Guessing operating system",
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_FILES.os= guess-netbsd.h",
+ ". if ${ARCH} == i386",
+ "SUBST_FILTER_CMD.os= ${SED} -e s,@OPSYS,NetBSD-i386,",
+ ". elif ${ARCH} == x86_64",
+ "SUBST_VARS.os= OPSYS",
+ ". else",
+ "SUBST_SED.os= -e s,@OPSYS,NetBSD-unknown",
+ ". endif",
+ ".else",
+ // This branch omits SUBST_FILES.
+ "SUBST_SED.os= -e s,@OPSYS@,unknown,",
+ ".endif")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_FILES.os missing.")
+}
+
+func (s *Suite) Test_SubstContext_isComplete__incomplete(c *check.C) {
+ t := s.Init(c)
+
ctx := NewSubstContext()
- lineno := 0
- for _, lineText := range texts {
- var curr int
- _, err := fmt.Sscanf(lineText[0:4], "%d: ", &curr)
- assertNil(err, "")
-
- if lineno != 0 {
- t.CheckEquals(curr, lineno)
- }
-
- text := lineText[4:]
- line := t.NewMkLine("Makefile", curr, text)
-
- switch {
- case text == "":
- ctx.Finish(line)
- case hasPrefix(text, "."):
- ctx.Directive(line)
- default:
- ctx.Varassign(line)
- }
- lineno = curr + 1
- }
+ ctx.varassign(t.NewMkLine("filename.mk", 10, "PKGNAME=pkgname-1.0"))
- ctx.Finish(t.NewMkLine("Makefile", lineno, ""))
+ t.CheckEquals(ctx.id, "")
+
+ ctx.varassign(t.NewMkLine("filename.mk", 11, "SUBST_CLASSES+=interp"))
+
+ t.CheckEquals(ctx.id, "interp")
+
+ ctx.varassign(t.NewMkLine("filename.mk", 12, "SUBST_FILES.interp=Makefile"))
+
+ t.CheckEquals(ctx.isComplete(), false)
+
+ ctx.varassign(t.NewMkLine("filename.mk", 13, "SUBST_SED.interp=s,@PREFIX@,${PREFIX},g"))
+
+ t.CheckEquals(ctx.isComplete(), false)
+
+ ctx.Finish(t.NewMkLine("filename.mk", 14, ""))
+
+ t.CheckOutputLines(
+ "NOTE: filename.mk:13: The substitution command \"s,@PREFIX@,${PREFIX},g\" "+
+ "can be replaced with \"SUBST_VARS.interp= PREFIX\".",
+ "WARN: filename.mk:14: Incomplete SUBST block: SUBST_STAGE.interp missing.")
+}
+
+func (s *Suite) Test_SubstContext_isComplete__complete(c *check.C) {
+ t := s.Init(c)
+
+ ctx := NewSubstContext()
+
+ ctx.varassign(t.NewMkLine("filename.mk", 10, "PKGNAME=pkgname-1.0"))
+ ctx.varassign(t.NewMkLine("filename.mk", 11, "SUBST_CLASSES+=p"))
+ ctx.varassign(t.NewMkLine("filename.mk", 12, "SUBST_FILES.p=Makefile"))
+ ctx.varassign(t.NewMkLine("filename.mk", 13, "SUBST_SED.p=s,@PREFIX@,${PREFIX},g"))
+
+ t.CheckEquals(ctx.isComplete(), false)
+
+ ctx.varassign(t.NewMkLine("filename.mk", 14, "SUBST_STAGE.p=post-configure"))
+
+ t.CheckEquals(ctx.isComplete(), true)
+
+ ctx.Finish(t.NewMkLine("filename.mk", 15, ""))
+
+ t.CheckOutputLines(
+ "NOTE: filename.mk:13: The substitution command \"s,@PREFIX@,${PREFIX},g\" " +
+ "can be replaced with \"SUBST_VARS.p= PREFIX\".")
}
Index: pkgsrc/pkgtools/pkglint/files/util.go
diff -u pkgsrc/pkgtools/pkglint/files/util.go:1.65 pkgsrc/pkgtools/pkglint/files/util.go:1.66
--- pkgsrc/pkgtools/pkglint/files/util.go:1.65 Mon Dec 9 20:38:16 2019
+++ pkgsrc/pkgtools/pkglint/files/util.go Fri Dec 13 01:39:23 2019
@@ -71,6 +71,11 @@ func replaceAllFunc(s string, re regex.P
return G.res.Compile(re).ReplaceAllStringFunc(s, repl)
}
+func containsWord(s, word string) bool {
+ return strings.Contains(s, word) &&
+ matches(s, regex.Pattern(`\b`+regexp.QuoteMeta(word)+`\b`))
+}
+
func containsStr(slice []string, s string) bool {
for _, str := range slice {
if s == str {
@@ -207,6 +212,13 @@ func imax(a, b int) int {
return b
}
+func imin(a, b int) int {
+ if a < b {
+ return a
+ }
+ return b
+}
+
// assertNil ensures that the given error is nil.
//
// Contrary to other diagnostics, the format should not end in a period
@@ -386,19 +398,35 @@ func detab(s string) string {
// alignWith extends str with as many tabs and spaces as needed to reach
// the same screen width as the other string.
func alignWith(str, other string) string {
+ return str + alignmentTo(str, other)
+}
+
+// alignmentTo returns the whitespace that is necessary to
+// bring str to the same width as other.
+func alignmentTo(str, other string) string {
strWidth := tabWidth(str)
otherWidth := tabWidth(other)
+ return alignmentToWidths(strWidth, otherWidth)
+}
+
+func alignmentToWidths(strWidth, otherWidth int) string {
if otherWidth <= strWidth {
- return str + "\t"
+ return ""
}
if strWidth&-8 != otherWidth&-8 {
strWidth &= -8
}
- alignment := indent(otherWidth - strWidth)
- return str + alignment
+ return indent(otherWidth - strWidth)
}
func indent(width int) string {
+ const tabsAndSpaces = "\t\t\t\t\t\t\t\t\t "
+ middle := len(tabsAndSpaces) - 7
+ if width <= 8*middle+7 {
+ start := middle - width>>3
+ end := middle + width&7
+ return tabsAndSpaces[start:end]
+ }
return strings.Repeat("\t", width>>3) + " "[:width&7]
}
@@ -452,32 +480,28 @@ func toInt(s string, def int) int {
return def
}
-// mkopSubst evaluates make(1)'s :S substitution operator.
-// It does not resolve any variables.
-// FIXME: Move this function to the MkVarUseModifier type.
-// FIXME: Clearly signal that substituting is not possible if either
-// of the strings contains a variable reference.
-func mkopSubst(s string, left bool, from string, right bool, to string, flags string) string {
- re := regex.Pattern(condStr(left, "^", "") + regexp.QuoteMeta(from) + condStr(right, "$", ""))
- done := false
- gflag := contains(flags, "g")
- return replaceAllFunc(s, re, func(match string) string {
- if gflag || !done {
- done = !gflag
- return to
+func containsVarRef(s string) bool {
+ if !contains(s, "$") {
+ return false
+ }
+ lex := NewMkLexer(s, nil)
+ tokens, _ := lex.MkTokens()
+ for _, token := range tokens {
+ if token.Varuse != nil {
+ return true
}
- return match
- })
+ }
+ return false
}
-func containsVarRef(s string) bool {
+func containsVarRefLong(s string) bool {
if !contains(s, "$") {
return false
}
lex := NewMkLexer(s, nil)
tokens, _ := lex.MkTokens()
for _, token := range tokens {
- if token.Varuse != nil {
+ if token.Varuse != nil && len(token.Text) > 2 {
return true
}
}
@@ -594,7 +618,7 @@ func (s *Scope) Define(varname string, m
case opAssignDefault:
// No change to the value.
case opAssignShell:
- s.value[name] = mkline.Value() // FIXME: Really?
+ delete(s.value, name)
default:
s.value[name] = mkline.Value()
}
@@ -759,7 +783,8 @@ func (s *Scope) FirstUse(varname string)
//
// If an empty string is returned this can mean either that the
// variable value is indeed the empty string or that the variable
-// was not found. To distinguish these cases, call LastValueFound instead.
+// was not found, or that the variable value cannot be determined
+// reliably. To distinguish these cases, call LastValueFound instead.
func (s *Scope) LastValue(varname string) string {
value, _ := s.LastValueFound(varname)
return value
@@ -772,7 +797,7 @@ func (s *Scope) LastValueFound(varname s
}
mkline := s.LastDefinition(varname)
- if mkline != nil {
+ if mkline != nil && mkline.Op() != opAssignShell {
return mkline.Value(), true
}
if fallback, ok := s.fallback[varname]; ok {
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.65 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.66
--- pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.65 Mon Dec 9 20:38:16 2019
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go Fri Dec 13 01:39:23 2019
@@ -968,9 +968,41 @@ func (s *Suite) Test_VartypeCheck_Homepa
"WARN: filename.mk:41: HOMEPAGE should not be defined in terms of MASTER_SITEs.")
}
-func (s *Suite) Test_VartypeCheck_Identifier(c *check.C) {
+func (s *Suite) Test_VartypeCheck_IdentifierDirect(c *check.C) {
t := s.Init(c)
- vt := NewVartypeCheckTester(t, BtIdentifier)
+ vt := NewVartypeCheckTester(t, BtIdentifierDirect)
+
+ vt.Varname("PKGBASE")
+ vt.Values(
+ "${OTHER_VAR}",
+ "identifiers cannot contain spaces",
+ "id/cannot/contain/slashes",
+ "id-${OTHER_VAR}",
+ "")
+
+ vt.Output(
+ "ERROR: filename.mk:1: Identifiers for PKGBASE "+
+ "must not refer to other variables.",
+ "WARN: filename.mk:2: Invalid identifier \"identifiers cannot contain spaces\".",
+ "WARN: filename.mk:3: Invalid identifier \"id/cannot/contain/slashes\".",
+ "ERROR: filename.mk:4: Identifiers for PKGBASE "+
+ "must not refer to other variables.",
+ "WARN: filename.mk:5: Invalid identifier \"\".")
+
+ vt.Op(opUseMatch)
+ vt.Values(
+ "[A-Z]",
+ "[A-Z.]",
+ "${PKG_OPTIONS:Moption}",
+ "A*B")
+
+ vt.Output(
+ "WARN: filename.mk:12: Invalid identifier pattern \"[A-Z.]\" for PKGBASE.")
+}
+
+func (s *Suite) Test_VartypeCheck_IdentifierIndirect(c *check.C) {
+ t := s.Init(c)
+ vt := NewVartypeCheckTester(t, BtIdentifierIndirect)
vt.Varname("MYSQL_CHARSET")
vt.Values(
Index: pkgsrc/pkgtools/pkglint/files/varalignblock.go
diff -u pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.11 pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.12
--- pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.11 Sun Dec 8 00:06:38 2019
+++ pkgsrc/pkgtools/pkglint/files/varalignblock.go Fri Dec 13 01:39:23 2019
@@ -141,30 +141,8 @@ import (
// dürfen die weiteren Fortsetzungszeilen weiter eingerückt sein als die erste.
// Ihre Einrückung besteht aus Tabs, gefolgt von 0 bis 7 Leerzeichen.
type VaralignBlock struct {
- infos []*varalignLine
- skip bool
-}
-
-type varalignLine struct {
- mkline *MkLine
- rawIndex int
-
- // Is true for multilines that don't have a value in their first
- // physical line.
- //
- // The follow-up lines of these lines may be indented with as few
- // as a single tab. Example:
- // VAR= \
- // value1 \
- // value2
- // In all other lines, the indentation must be at least the indentation
- // of the first value found.
- multiEmpty bool
-
- // Whether the line is so long that it may use a single tab as indentation.
- long bool
-
- varalignParts
+ mkinfos []*varalignMkLine
+ skip bool
}
func (va *VaralignBlock) Process(mkline *MkLine) {
@@ -206,6 +184,7 @@ func (va *VaralignBlock) processVarassig
}
follow := false
+ var infos []*varalignLine
for i, raw := range mkline.raw {
parts := NewVaralignSplitter().split(strings.TrimSuffix(raw.textnl, "\n"), i == 0)
info := varalignLine{mkline, i, follow, false, parts}
@@ -215,89 +194,52 @@ func (va *VaralignBlock) processVarassig
info.multiEmpty = true
}
- va.infos = append(va.infos, &info)
+ infos = append(infos, &info)
}
+ va.mkinfos = append(va.mkinfos, &varalignMkLine{infos})
}
func (va *VaralignBlock) Finish() {
- infos := va.infos
+ mkinfos := va.mkinfos
skip := va.skip
*va = VaralignBlock{} // overwrites infos and skip
- if len(infos) == 0 || skip {
+ if len(mkinfos) == 0 || skip {
return
}
if trace.Tracing {
- defer trace.Call(infos[0].mkline.Line)()
+ defer trace.Call()()
}
- newWidth := va.optimalWidth(infos)
- va.adjustLong(newWidth, infos)
- rightMargin := 0
-
- // When the indentation of the initial line of a multiline is
- // changed, all its follow-up lines are shifted by the same
- // amount and in the same direction. Typical examples are
- // SUBST_SED, shell programs and AWK programs like in
- // GENERATE_PLIST.
- indentDiffSet := false
- // The amount by which the follow-up lines are shifted.
- // Positive values mean shifting to the right, negative values
- // mean shifting to the left.
- indentDiff := 0
+ newWidth := va.optimalWidth(mkinfos)
+ va.adjustLong(newWidth, mkinfos)
- for i, info := range infos {
- if info.rawIndex == 0 {
- indentDiffSet = false
- indentDiff = 0
- restIndex := i + condInt(info.value != "", 0, 1)
- rightMargin = va.rightMargin(infos[restIndex:])
- }
-
- va.checkRightMargin(info, newWidth, rightMargin)
-
- if newWidth > 0 || info.rawIndex > 0 {
- va.realign(info, newWidth, &indentDiffSet, &indentDiff)
- }
- }
-}
+ for _, mkinfo := range mkinfos {
-// rightMargin calculates the column in which the continuation backslashes
-// should be placed.
-func (*VaralignBlock) rightMargin(infos []*varalignLine) int {
- var columns []int
- for _, info := range infos {
- if info.isContinuation() {
- space := info.spaceBeforeContinuation()
- if space != "" && space != " " {
- columns = append(columns, info.continuationColumn())
- }
- }
- }
+ // When the indentation of the initial line of a multiline is
+ // changed, all its follow-up lines are shifted by the same
+ // amount and in the same direction. Typical examples are
+ // SUBST_SED, shell programs and AWK programs like in
+ // GENERATE_PLIST.
+ indentDiffSet := false
- sort.Ints(columns)
+ // The amount by which the follow-up lines are shifted.
+ // Positive values mean shifting to the right, negative values
+ // mean shifting to the left.
+ indentDiff := 0
- for i := len(columns) - 2; i >= 0; i-- {
- if columns[i] == columns[i+1] {
- return columns[i]
- }
- }
+ _, rightMargin := mkinfo.rightMargin()
+ for _, info := range mkinfo.infos {
- if len(columns) <= 1 {
- return 0
- }
+ // TODO: move below va.realign
+ info.alignContinuation(newWidth, rightMargin)
- var min int
- for _, info := range infos {
- if info.isContinuation() {
- mainWidth := info.uptoCommentWidth()
- if mainWidth > min {
- min = mainWidth
+ if newWidth > 0 || info.rawIndex > 0 {
+ va.realign(info, newWidth, &indentDiffSet, &indentDiff)
}
}
}
- return min&-8 + 8
}
// optimalWidth computes the desired screen width for the variable assignment
@@ -306,18 +248,23 @@ func (*VaralignBlock) rightMargin(infos
// There may be a single line sticking out from the others (called outlier).
// This is to prevent a single SITES.* variable from forcing the rest of the
// paragraph to be indented too far to the right.
-func (*VaralignBlock) optimalWidth(infos []*varalignLine) int {
+func (*VaralignBlock) optimalWidth(mkinfos []*varalignMkLine) int {
- var widths mklineInts
- for _, info := range infos {
- if !info.multiEmpty && info.rawIndex == 0 {
- widths.append(info.mkline, info.varnameOpWidth())
+ var widths bag
+ for _, mkinfo := range mkinfos {
+ for _, info := range mkinfo.infos {
+ if !info.multiEmpty && info.rawIndex == 0 {
+ widths.Add(info.fixer, info.varnameOpWidth())
+ }
}
}
widths.sortDesc()
longest := widths.opt(0)
- longestLine := widths.optLine(0)
+ var longestLine *MkLine
+ if len(widths.slice) > 0 {
+ longestLine = widths.key(0).(*MkLine)
+ }
secondLongest := widths.opt(1)
haveOutlier := secondLongest != 0 &&
@@ -329,12 +276,14 @@ func (*VaralignBlock) optimalWidth(infos
outlier := condInt(haveOutlier, longest, 0)
// Widths of the current indentation (including whitespace)
- var spaceWidths mklineInts
- for _, info := range infos {
- if info.multiEmpty || info.rawIndex > 0 || outlier > 0 && info.varnameOpWidth() == outlier {
- continue
+ var spaceWidths bag
+ for _, mkinfo := range mkinfos {
+ for _, info := range mkinfo.infos {
+ if info.multiEmpty || info.rawIndex > 0 || outlier > 0 && info.varnameOpWidth() == outlier {
+ continue
+ }
+ spaceWidths.Add(nil, info.varnameOpSpaceWidth())
}
- spaceWidths.append(info.mkline, info.varnameOpSpaceWidth())
}
spaceWidths.sortDesc()
@@ -366,60 +315,33 @@ func (*VaralignBlock) optimalWidth(infos
// adjustLong allows any follow-up line to start either in column 8 or at
// least in column newWidth. But only if there is at least one continuation
// line that starts in column 8 and needs the full width up to column 72.
-func (va *VaralignBlock) adjustLong(newWidth int, infos []*varalignLine) {
+func (va *VaralignBlock) adjustLong(newWidth int, mkinfos []*varalignMkLine) {
anyLong := false
- for i, info := range infos {
- if info.rawIndex == 0 {
- anyLong = false
- for _, follow := range infos[i+1:] {
- if follow.rawIndex == 0 {
- break
- }
- if !follow.multiEmpty && follow.spaceBeforeValue == "\t" && follow.varnameOpSpaceWidth() < newWidth && follow.widthAlignedAt(newWidth) > 72 {
- anyLong = true
- break
+ for _, mkinfo := range mkinfos {
+ infos := mkinfo.infos
+ for i, info := range infos {
+ if info.rawIndex == 0 {
+ anyLong = false
+ for _, follow := range infos[i+1:] {
+ if follow.rawIndex == 0 {
+ break
+ }
+ if !follow.multiEmpty && follow.spaceBeforeValue == "\t" && follow.varnameOpSpaceWidth() < newWidth && follow.widthAlignedAt(newWidth) > 72 {
+ anyLong = true
+ break
+ }
}
}
- }
-
- info.long = anyLong && info.varnameOpSpaceWidth() == 8
- }
-}
-
-func (va *VaralignBlock) checkRightMargin(info *varalignLine, newWidth int, rightMargin int) {
- if !info.isContinuation() {
- return
- }
-
- oldSpace := info.spaceBeforeContinuation()
- if oldSpace == " " || oldSpace == "\t" {
- return
- }
- column := info.continuationColumn()
- if column == 72 || column == rightMargin || column <= newWidth {
- return
- }
-
- newSpace := " "
- fix := info.mkline.Autofix()
- if oldSpace == "" || rightMargin == 0 || info.uptoCommentWidth() >= rightMargin {
- fix.Notef("The continuation backslash should be preceded by a single space or tab.")
- } else {
- newSpace = alignmentAfter(info.uptoComment(), rightMargin)
- fix.Notef(
- "The continuation backslash should be preceded by a single space or tab, "+
- "or be in column %d, not %d.",
- rightMargin+1, column+1)
+ info.long = anyLong && info.varnameOpSpaceWidth() == 8
+ }
}
- fix.ReplaceAt(info.rawIndex, info.continuationIndex()-len(oldSpace), oldSpace, newSpace)
- fix.Apply()
}
func (va *VaralignBlock) realign(info *varalignLine, newWidth int, indentDiffSet *bool, indentDiff *int) {
if info.multiEmpty {
if info.rawIndex == 0 {
- va.realignMultiEmptyInitial(info, newWidth)
+ info.alignValueMultiEmptyInitial(newWidth)
} else {
va.realignMultiEmptyFollow(info, newWidth, indentDiffSet, indentDiff)
}
@@ -427,48 +349,12 @@ func (va *VaralignBlock) realign(info *v
va.realignMultiInitial(info, newWidth, indentDiffSet, indentDiff)
} else if info.rawIndex > 0 {
assert(*indentDiffSet)
- va.realignMultiFollow(info, newWidth, *indentDiff)
+ info.alignValueMultiFollow(newWidth, *indentDiff)
} else {
- va.realignSingle(info, newWidth)
+ info.alignValueSingle(newWidth)
}
}
-func (*VaralignBlock) realignMultiEmptyInitial(info *varalignLine, newWidth int) {
- leadingComment := info.leadingComment
- varnameOp := info.varnameOp
- oldSpace := info.spaceBeforeValue
-
- // Indent the outlier and any other lines that stick out
- // with a space instead of a tab to keep the line short.
- newSpace := " "
- if info.varnameOpSpaceWidth() <= newWidth {
- newSpace = alignmentAfter(leadingComment+varnameOp, newWidth)
- }
-
- if newSpace == oldSpace {
- return
- }
-
- if newSpace == " " {
- return // This case is handled by checkRightMargin.
- }
-
- hasSpace := strings.IndexByte(oldSpace, ' ') != -1
- oldColumn := info.varnameOpSpaceWidth()
- column := tabWidthSlice(leadingComment, varnameOp, newSpace)
-
- fix := info.mkline.Autofix()
- if hasSpace && column != oldColumn {
- fix.Notef("This variable value should be aligned with tabs, not spaces, to column %d.", column+1)
- } else if column != oldColumn {
- fix.Notef("This variable value should be aligned to column %d.", column+1) // TODO: to column %d instead of %d.
- } else {
- fix.Notef("Variable values should be aligned with tabs, not spaces.")
- }
- fix.ReplaceAt(info.rawIndex, info.spaceBeforeValueIndex(), oldSpace, newSpace)
- fix.Apply()
-}
-
func (va *VaralignBlock) realignMultiEmptyFollow(info *varalignLine, newWidth int, indentDiffSet *bool, indentDiff *int) {
oldSpace := info.spaceBeforeValue
oldWidth := tabWidth(oldSpace)
@@ -481,141 +367,14 @@ func (va *VaralignBlock) realignMultiEmp
}
}
- newSpace := indent(imax(oldWidth+*indentDiff, 8))
- if newSpace == oldSpace {
- return
- }
-
- // Below a continuation marker, there may be a completely empty line.
- // This is confusing to the human readers, but technically allowed.
- if info.varalignParts.isEmpty() {
- return
- }
-
- fix := info.mkline.Autofix()
- fix.Notef("This continuation line should be indented with %q.", newSpace)
- fix.ReplaceAt(info.rawIndex, info.spaceBeforeValueIndex(), oldSpace, newSpace)
- fix.Apply()
+ info.alignValueMultiEmptyFollow(imax(oldWidth+*indentDiff, 8))
}
func (va *VaralignBlock) realignMultiInitial(info *varalignLine, newWidth int, indentDiffSet *bool, indentDiff *int) {
- leadingComment := info.leadingComment
- varnameOp := info.varnameOp
- oldSpace := info.spaceBeforeValue
-
*indentDiffSet = true
- oldWidth := info.varnameOpSpaceWidth()
- *indentDiff = newWidth - oldWidth
-
- newSpace := alignmentAfter(leadingComment+varnameOp, newWidth)
- if newSpace == oldSpace {
- return
- }
-
- hasSpace := strings.IndexByte(oldSpace, ' ') != -1
- width := tabWidthSlice(leadingComment, varnameOp, newSpace)
-
- fix := info.mkline.Autofix()
- if hasSpace && width != oldWidth {
- fix.Notef("This variable value should be aligned with tabs, not spaces, to column %d.", width+1)
- } else if width != oldWidth {
- fix.Notef("This variable value should be aligned to column %d.", width+1)
- } else {
- fix.Notef("Variable values should be aligned with tabs, not spaces.")
- }
- fix.ReplaceAt(info.rawIndex, info.spaceBeforeValueIndex(), oldSpace, newSpace)
- fix.Apply()
-}
-
-func (va *VaralignBlock) realignMultiFollow(info *varalignLine, newWidth int, indentDiff int) {
- oldSpace := info.spaceBeforeValue
- newSpace := indent(tabWidth(oldSpace) + indentDiff)
- if tabWidth(newSpace) < newWidth {
- newSpace = indent(newWidth)
- }
- if newSpace == oldSpace || info.long {
- return
- }
-
- fix := info.mkline.Autofix()
- fix.Notef("This continuation line should be indented with %q.", indent(newWidth))
- modified, replaced := fix.ReplaceAt(info.rawIndex, info.spaceBeforeValueIndex(), oldSpace, newSpace)
- assert(modified)
- if info.continuation != "" && info.continuationColumn() == 72 {
- orig := strings.TrimSuffix(replaced, "\n")
- base := rtrimHspace(strings.TrimSuffix(orig, "\\"))
- spaceIndex := len(base)
- oldSuffix := orig[spaceIndex:]
- newSuffix := " \\"
- if tabWidth(base) < 72 {
- newSuffix = alignmentAfter(base, 72) + "\\"
- }
- if newSuffix != oldSuffix {
- fix.ReplaceAt(info.rawIndex, spaceIndex, oldSuffix, newSuffix)
- }
- }
- fix.Apply()
-}
-
-func (va *VaralignBlock) realignSingle(info *varalignLine, newWidth int) {
- leadingComment := info.leadingComment
- varnameOp := info.varnameOp
- oldSpace := info.spaceBeforeValue
+ *indentDiff = newWidth - info.varnameOpSpaceWidth()
- newSpace := ""
- for tabWidthSlice(leadingComment, varnameOp, newSpace) < newWidth {
- newSpace += "\t"
- }
-
- // Indent the outlier with a space instead of a tab to keep the line short.
- if newSpace == "" && info.isCanonicalInitial(newWidth) {
- return
- }
- if newSpace == "" {
- newSpace = " "
- }
-
- if newSpace == oldSpace {
- return
- }
-
- hasSpace := strings.IndexByte(oldSpace, ' ') != -1
- oldColumn := tabWidthSlice(leadingComment, varnameOp, oldSpace)
- column := tabWidthSlice(leadingComment, varnameOp, newSpace)
-
- fix := info.mkline.Autofix()
- if newSpace == " " {
- fix.Notef("This outlier variable value should be aligned with a single space.")
- va.explainWrongColumn(fix)
- } else if hasSpace && column != oldColumn {
- fix.Notef("This variable value should be aligned with tabs, not spaces, to column %d.", column+1)
- va.explainWrongColumn(fix)
- } else if column != oldColumn {
- fix.Notef("This variable value should be aligned to column %d.", column+1)
- va.explainWrongColumn(fix)
- } else {
- fix.Notef("Variable values should be aligned with tabs, not spaces.")
- }
- fix.ReplaceAt(info.rawIndex, info.spaceBeforeValueIndex(), oldSpace, newSpace)
- fix.Apply()
-}
-
-func (va *VaralignBlock) explainWrongColumn(fix *Autofix) {
- fix.Explain(
- "Normally, all variable values in a block should start at the same column.",
- "This provides orientation, especially for sequences",
- "of variables that often appear in the same order.",
- "For these it suffices to look at the variable values only.",
- "",
- "There are some exceptions to this rule:",
- "",
- "Definitions for long variable names may be indented with a single space instead of tabs,",
- "but only if they appear in a block that is otherwise indented with tabs.",
- "",
- "Variable definitions that span multiple lines are not checked for alignment at all.",
- "",
- "When the block contains something else than variable definitions",
- "and directives like .if or .for, it is not checked at all.")
+ info.alignValueMultiInitial(newWidth)
}
// VaralignSplitter parses the text of a raw line into those parts that
@@ -713,6 +472,278 @@ func (VaralignSplitter) parseValue(lexer
return value, space, continuation
}
+type varalignMkLine struct {
+ infos []*varalignLine
+}
+
+// rightMargin calculates the column in which the continuation backslashes
+// should be placed.
+// In addition it returns whether the right margin is already in its
+// canonical form.
+func (l *varalignMkLine) rightMargin() (ok bool, margin int) {
+ restIndex := condInt(l.infos[0].value == "", 1, 0)
+ infos := l.infos[restIndex:]
+
+ var columns []int
+ for _, info := range infos {
+ if info.isContinuation() {
+ space := info.spaceBeforeContinuation()
+ if space != "" && space != " " {
+ columns = append(columns, info.continuationColumn())
+ }
+ }
+ }
+
+ if len(columns) <= 1 {
+ return false, 0
+ }
+
+ sort.Ints(columns)
+
+ for i := len(columns) - 2; i >= 0; i-- {
+ col := columns[i]
+ if col == columns[i+1] {
+ ok := columns[0] == columns[len(columns)-1] && col <= 72
+ return ok, imin(col, 72)
+ }
+ }
+
+ var min int
+ for _, info := range infos {
+ if info.isContinuation() {
+ mainWidth := info.uptoValueWidth()
+ if mainWidth > min {
+ min = mainWidth
+ }
+ }
+ }
+ return false, min&-8 + 8
+}
+
+type varalignLine struct {
+ fixer Autofixer
+ rawIndex int
+
+ // Is true for multilines that don't have a value in their first
+ // physical line.
+ //
+ // The follow-up lines of these lines may be indented with as few
+ // as a single tab. Example:
+ // VAR= \
+ // value1 \
+ // value2
+ // In all other lines, the indentation must be at least the indentation
+ // of the first value found.
+ multiEmpty bool
+
+ // Whether the line is so long that it may use a single tab as indentation.
+ long bool
+
+ varalignParts
+}
+
+func (info *varalignLine) alignValueSingle(newWidth int) {
+ leadingComment := info.leadingComment
+ varnameOp := info.varnameOp
+
+ oldSpace := info.spaceBeforeValue
+ newSpace := alignmentToWidths(tabWidthSlice(leadingComment, varnameOp), newWidth)
+
+ // Indent the outlier with a space instead of a tab to keep the line short.
+ if newSpace == "" && info.isCanonicalInitial(newWidth) {
+ return
+ }
+ if newSpace == "" {
+ newSpace = " "
+ }
+
+ if newSpace == oldSpace {
+ return
+ }
+
+ hasSpace := strings.IndexByte(oldSpace, ' ') != -1
+ oldColumn := tabWidthSlice(leadingComment, varnameOp, oldSpace)
+ column := tabWidthSlice(leadingComment, varnameOp, newSpace)
+
+ fix := info.fixer.Autofix()
+ if newSpace == " " {
+ fix.Notef("This outlier variable value should be aligned with a single space.")
+ info.explainWrongColumn(fix)
+ } else if hasSpace && column != oldColumn {
+ fix.Notef("This variable value should be aligned with tabs, not spaces, to column %d.", column+1)
+ info.explainWrongColumn(fix)
+ } else if column != oldColumn {
+ fix.Notef("This variable value should be aligned to column %d.", column+1)
+ info.explainWrongColumn(fix)
+ } else {
+ fix.Notef("Variable values should be aligned with tabs, not spaces.")
+ }
+ fix.ReplaceAt(info.rawIndex, info.spaceBeforeValueIndex(), oldSpace, newSpace)
+ fix.Apply()
+}
+
+func (info *varalignLine) alignValueMultiEmptyInitial(newWidth int) {
+ leadingComment := info.leadingComment
+ varnameOp := info.varnameOp
+ oldSpace := info.spaceBeforeValue
+
+ // Indent the outlier and any other lines that stick out
+ // with a space instead of a tab to keep the line short.
+ newSpace := " "
+ if info.varnameOpSpaceWidth() <= newWidth {
+ newSpace = alignmentAfter(leadingComment+varnameOp, newWidth)
+ }
+
+ if newSpace == oldSpace {
+ return
+ }
+
+ if newSpace == " " {
+ return // This case is handled by checkRightMargin.
+ }
+
+ hasSpace := strings.IndexByte(oldSpace, ' ') != -1
+ oldColumn := info.varnameOpSpaceWidth()
+ column := tabWidthSlice(leadingComment, varnameOp, newSpace)
+
+ fix := info.fixer.Autofix()
+ if hasSpace && column != oldColumn {
+ fix.Notef("This variable value should be aligned with tabs, not spaces, to column %d.", column+1)
+ } else if column != oldColumn {
+ fix.Notef("This variable value should be aligned to column %d.", column+1) // TODO: to column %d instead of %d.
+ } else {
+ fix.Notef("Variable values should be aligned with tabs, not spaces.")
+ }
+ fix.ReplaceAt(info.rawIndex, info.spaceBeforeValueIndex(), oldSpace, newSpace)
+ fix.Apply()
+}
+
+func (info *varalignLine) alignValueMultiInitial(column int) {
+ leadingComment := info.leadingComment
+ varnameOp := info.varnameOp
+ oldSpace := info.spaceBeforeValue
+
+ newSpace := alignmentAfter(leadingComment+varnameOp, column)
+ if newSpace == oldSpace {
+ return
+ }
+
+ oldWidth := info.varnameOpSpaceWidth()
+ width := tabWidthSlice(leadingComment, varnameOp, newSpace)
+
+ fix := info.fixer.Autofix()
+ if width != oldWidth && contains(oldSpace, " ") {
+ fix.Notef("This variable value should be aligned with tabs, not spaces, to column %d.", width+1)
+ } else if width != oldWidth {
+ fix.Notef("This variable value should be aligned to column %d.", width+1)
+ } else {
+ fix.Notef("Variable values should be aligned with tabs, not spaces.")
+ }
+ fix.ReplaceAt(info.rawIndex, info.spaceBeforeValueIndex(), oldSpace, newSpace)
+ fix.Apply()
+}
+
+func (info *varalignLine) alignValueMultiEmptyFollow(column int) {
+ oldSpace := info.spaceBeforeValue
+ newSpace := indent(column)
+ if newSpace == oldSpace {
+ return
+ }
+
+ // Below a continuation marker, there may be a completely empty line.
+ // This is confusing to the human readers, but technically allowed.
+ if info.varalignParts.isEmpty() {
+ return
+ }
+
+ fix := info.fixer.Autofix()
+ fix.Notef("This continuation line should be indented with %q.", newSpace)
+ fix.ReplaceAt(info.rawIndex, info.spaceBeforeValueIndex(), oldSpace, newSpace)
+ fix.Apply()
+
+ // TODO: Merge with alignValueMultiFollow
+}
+
+func (info *varalignLine) alignValueMultiFollow(column, indentDiff int) {
+ oldSpace := info.spaceBeforeValue
+ newWidth := imax(column, tabWidth(oldSpace)+indentDiff)
+ newSpace := indent(newWidth)
+ if newSpace == oldSpace || info.long || info.isTooLongFor(newWidth) {
+ return
+ }
+
+ fix := info.fixer.Autofix()
+ fix.Notef("This continuation line should be indented with %q.", newSpace)
+ modified, replaced := fix.ReplaceAt(info.rawIndex, info.spaceBeforeValueIndex(), oldSpace, newSpace)
+ assert(modified)
+ if info.continuation != "" && info.continuationColumn() == 72 {
+ orig := strings.TrimSuffix(replaced, "\n")
+ base := rtrimHspace(strings.TrimSuffix(orig, "\\"))
+ spaceIndex := len(base)
+ oldSuffix := orig[spaceIndex:]
+ newSuffix := " \\"
+ if tabWidth(base) < 72 {
+ newSuffix = alignmentAfter(base, 72) + "\\"
+ }
+ if newSuffix != oldSuffix {
+ fix.ReplaceAt(info.rawIndex, spaceIndex, oldSuffix, newSuffix)
+ }
+ }
+ fix.Apply()
+}
+
+func (info *varalignLine) alignContinuation(valueColumn, rightMarginColumn int) {
+ if !info.isContinuation() {
+ return
+ }
+
+ oldSpace := info.spaceBeforeContinuation()
+ if oldSpace == " " || oldSpace == "\t" {
+ return
+ }
+
+ column := info.continuationColumn()
+ if column == 72 || column == rightMarginColumn || column <= valueColumn {
+ return
+ }
+
+ newSpace := " "
+ fix := info.fixer.Autofix()
+ if oldSpace == "" || rightMarginColumn == 0 {
+ fix.Notef("The continuation backslash should be preceded by a single space or tab.")
+ } else if info.isTooLongFor(valueColumn) {
+ fix.Notef("The continuation backslash should be preceded by a single space.")
+ } else if info.uptoValueWidth() >= rightMarginColumn {
+ fix.Notef("The continuation backslash should be preceded by a single space or tab.")
+ } else {
+ newSpace = alignmentAfter(info.uptoValue(), rightMarginColumn)
+ fix.Notef(
+ "The continuation backslash should be in column %d, not %d.",
+ rightMarginColumn+1, column+1)
+ }
+ index := info.continuationIndex() - len(oldSpace)
+ fix.ReplaceAt(info.rawIndex, index, oldSpace, newSpace)
+ fix.Apply()
+}
+
+func (*varalignLine) explainWrongColumn(fix *Autofix) {
+ fix.Explain(
+ "Normally, all variable values in a block should start at the same column.",
+ "This provides orientation, especially for sequences",
+ "of variables that often appear in the same order.",
+ "For these it suffices to look at the variable values only.",
+ "",
+ "There are some exceptions to this rule:",
+ "",
+ "Definitions for long variable names may be indented with a single space instead of tabs,",
+ "but only if they appear in a block that is otherwise indented with tabs.",
+ "",
+ "Variable definitions that span multiple lines are not checked for alignment at all.",
+ "",
+ "When the block contains something else than variable definitions",
+ "and directives like .if or .for, it is not checked at all.")
+}
+
type varalignParts struct {
leadingComment string // either the # or some rarely used U+0020 spaces
varnameOp string // empty iff it is a follow-up line
@@ -722,6 +753,13 @@ type varalignParts struct {
continuation string // either a single backslash or empty
}
+func (p *varalignParts) String() string {
+ return p.leadingComment +
+ p.varnameOp + p.spaceBeforeValue +
+ p.value + p.spaceAfterValue +
+ p.continuation
+}
+
// continuation returns whether this line ends with a backslash.
func (p *varalignParts) isContinuation() bool {
return p.continuation != ""
@@ -758,13 +796,13 @@ func (p *varalignParts) spaceBeforeConti
return p.spaceAfterValue
}
-func (p *varalignParts) uptoCommentWidth() int {
+func (p *varalignParts) uptoValueWidth() int {
return tabWidth(rtrimHspace(p.leadingComment +
p.varnameOp + p.spaceBeforeValue +
p.value))
}
-func (p *varalignParts) uptoComment() string {
+func (p *varalignParts) uptoValue() string {
return rtrimHspace(p.leadingComment +
p.varnameOp + p.spaceBeforeValue +
p.value)
@@ -826,39 +864,44 @@ func (p *varalignParts) widthAlignedAt(v
p.value+p.spaceAfterValue+p.continuation)
}
-type mklineInts struct {
+func (p *varalignParts) isTooLongFor(valueColumn int) bool {
+ column := tabWidthAppend(valueColumn, p.value)
+ if p.isContinuation() {
+ column += 2
+ }
+ return column > 73
+}
+
+type bag struct {
slice []struct {
- mkline *MkLine
- value int
+ key interface{}
+ value int
}
}
-func (mi mklineInts) sortDesc() {
+func (mi *bag) sortDesc() {
less := func(i, j int) bool { return mi.slice[j].value < mi.slice[i].value }
sort.SliceStable(mi.slice, less)
}
-func (mi mklineInts) opt(index int) int {
+func (mi *bag) opt(index int) int {
if uint(index) < uint(len(mi.slice)) {
return mi.slice[index].value
}
return 0
}
-func (mi mklineInts) optLine(index int) *MkLine {
- if uint(index) < uint(len(mi.slice)) {
- return mi.slice[index].mkline
- }
- return nil
+func (mi *bag) key(index int) interface{} {
+ return mi.slice[index].key
}
-func (mi *mklineInts) append(mkline *MkLine, value int) {
+func (mi *bag) Add(key interface{}, value int) {
mi.slice = append(mi.slice, struct {
- mkline *MkLine
- value int
- }{mkline, value})
+ key interface{}
+ value int
+ }{key, value})
}
-func (mi mklineInts) min() int { return mi.opt(0) }
+func (mi *bag) min() int { return mi.opt(0) }
-func (mi mklineInts) max() int { return mi.opt(len(mi.slice) - 1) }
+func (mi *bag) max() int { return mi.opt(len(mi.slice) - 1) }
Index: pkgsrc/pkgtools/pkglint/files/varalignblock_test.go
diff -u pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.7 pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.8
--- pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.7 Sun Nov 17 01:26:25 2019
+++ pkgsrc/pkgtools/pkglint/files/varalignblock_test.go Fri Dec 13 01:39:23 2019
@@ -30,7 +30,10 @@ func NewVaralignTester(s *Suite, c *chec
}
// Input remembers the input lines that are checked and possibly realigned.
-func (vt *VaralignTester) Input(lines ...string) { vt.input = lines }
+func (vt *VaralignTester) Input(lines ...string) {
+ vt.tester.CheckDotColumns(lines...)
+ vt.input = lines
+}
// InputDetab validates the input lines after replacing tabs with spaces.
//
@@ -92,19 +95,21 @@ func (vt *VaralignTester) run(autofix bo
varalign.Process(mkline)
}
- infos := varalign.infos // since they are overwritten by Finish
+ mkinfos := varalign.mkinfos // since they are overwritten by Finish
varalign.Finish()
if len(vt.internals) > 0 {
var actual []string
- for _, info := range infos {
- var minWidth, curWidth, continuation string
- minWidth = condStr(info.rawIndex == 0, sprintf("%02d", info.varnameOpWidth()), " ")
- curWidth = sprintf(" %02d", info.varnameOpSpaceWidth())
- if info.isContinuation() {
- continuation = sprintf(" %02d", info.continuationColumn())
+ for _, mkinfo := range mkinfos {
+ for _, info := range mkinfo.infos {
+ var minWidth, curWidth, continuation string
+ minWidth = condStr(info.rawIndex == 0, sprintf("%02d", info.varnameOpWidth()), " ")
+ curWidth = sprintf(" %02d", info.varnameOpSpaceWidth())
+ if info.isContinuation() {
+ continuation = sprintf(" %02d", info.continuationColumn())
+ }
+ actual = append(actual, minWidth+curWidth+continuation)
}
- actual = append(actual, minWidth+curWidth+continuation)
}
t.CheckDeepEquals(actual, vt.internals)
}
@@ -1448,7 +1453,7 @@ func (s *Suite) Test_VaralignBlock__cont
vt.Diagnostics(
"NOTE: Makefile:1: This variable value should be aligned to column 17.",
"NOTE: Makefile:3: This variable value should be aligned with tabs, not spaces, to column 17.",
- "NOTE: Makefile:4: This continuation line should be indented with \"\\t\\t\".",
+ "NOTE: Makefile:4: This continuation line should be indented with \"\\t\\t \".",
"NOTE: Makefile:5: This continuation line should be indented with \"\\t\\t\".")
vt.Autofixes(
"AUTOFIX: Makefile:1: Replacing \"\\t\" with \"\\t\\t\".",
@@ -2250,8 +2255,7 @@ func (s *Suite) Test_VaralignBlock__mixe
"05 08 15",
" 17")
vt.Diagnostics(
- // FIXME: This diagnostic doesn't match the autofix.
- "NOTE: Makefile:3: This continuation line should be indented with \"\\t\".")
+ "NOTE: Makefile:3: This continuation line should be indented with \"\\t\\t \".")
vt.Autofixes(
"AUTOFIX: Makefile:3: Replacing \" \\t \\t \" with \"\\t\\t \".")
vt.Fixed(
@@ -2471,14 +2475,11 @@ func (s *Suite) Test_VaralignBlock__alig
t := s.Init(c)
test := func(data ...interface{}) {
- var lineTexts []string
- for _, text := range data[:len(data)-1] {
- lineTexts = append(lineTexts, text.(string))
- }
- expected := data[len(data)-1].(bool)
+ lines, expected := t.SplitStringsBool(data)
+ t.CheckDotColumns(lines...)
mklines := t.NewMkLines("filename.mk",
- lineTexts...)
+ lines...)
assert(len(mklines.mklines) == 1)
var varalign VaralignBlock
@@ -2489,7 +2490,7 @@ func (s *Suite) Test_VaralignBlock__alig
if expected {
t.CheckEquals(output, "")
} else if output == "" {
- t.Check(output, check.Not(check.Equals), "")
+ t.Check(output, check.Not(check.Equals), "output should not be empty")
}
}
@@ -2535,7 +2536,7 @@ func (s *Suite) Test_VaralignBlock__alig
// appear in a rectangular shape in the source code.
test(
"VAR.param=\tvalue \\",
- "\t10........20........30........40........50........60...4",
+ "\t10........20........30........40........50........60..64",
false)
// The second line is indented with a single tab because otherwise
@@ -2543,7 +2544,7 @@ func (s *Suite) Test_VaralignBlock__alig
// use the smaller indentation.
test(
"VAR.param=\tvalue \\",
- "\t10........20........30........40........50........60....5",
+ "\t10........20........30........40........50........60...65",
true)
// Having the continuation line in column 0 looks even more confusing.
@@ -2759,8 +2760,7 @@ func (s *Suite) Test_VaralignBlock__init
" 24 72",
" 24")
vt.Diagnostics(
- "NOTE: Makefile:1: The continuation backslash should be preceded " +
- "by a single space or tab, or be in column 73, not 81.")
+ "NOTE: Makefile:1: The continuation backslash should be in column 73, not 81.")
vt.Autofixes(
"AUTOFIX: Makefile:1: Replacing \"\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\\t\".")
vt.Fixed(
@@ -2782,8 +2782,7 @@ func (s *Suite) Test_VaralignBlock__long
" 08 17",
" 08")
vt.Diagnostics(
- "NOTE: Makefile:1: The continuation backslash should be preceded "+
- "by a single space or tab, or be in column 57, not 66.",
+ "NOTE: Makefile:1: The continuation backslash should be in column 57, not 66.",
"NOTE: Makefile:2: This continuation line should be indented with \"\\t\\t\\t\\t\\t\\t\".",
"NOTE: Makefile:3: This continuation line should be indented with \"\\t\\t\\t\\t\\t\\t\".")
vt.Autofixes(
@@ -2793,6 +2792,8 @@ func (s *Suite) Test_VaralignBlock__long
vt.Fixed(
"VAR= value \\",
// FIXME: The backslash should be aligned properly.
+ // It is not replaced because alignContinuation is called before fixAlign,
+ // which is the wrong order.
" value \\",
" value")
vt.Run()
@@ -2803,31 +2804,83 @@ func (s *Suite) Test_VaralignBlock__long
func (s *Suite) Test_VaralignBlock__long_lines_2(c *check.C) {
vt := NewVaralignTester(s, c)
vt.Input(
- "INSTALLATION_DIRS=\t_____________________________________________________________________ \\"+
- "\t\t\t\t __________________________________________________________\t\t \\",
- "\t\t\t__________________________________________________________\t\t \t\\",
- "\t\t\t__________________________________________________________\t\t \t\\",
- "\t\t\t_________________________")
+ "INSTALLATION_DIRS=\t......32......40......48......56......64......72......80......88..... \\"+
+ "\t\t\t\t ....136.....144.....152.....160.....168.....176.....184...\t\t \\",
+ "\t\t\t......32......40......48......56......64......72......80..\t\t \t\\",
+ "\t\t\t......32......40......48......56......64......72......80..\t\t \t\\",
+ "\t\t\t......32......40......48.")
vt.InputDetab(
- "INSTALLATION_DIRS= _____________________________________________________________________ \\
__________________________________________________________ \\",
- " __________________________________________________________ \\",
- " __________________________________________________________ \\",
- " _________________________")
+ "INSTALLATION_DIRS= ......32......40......48......56......64......72......80......88..... \\"+
+ " ....136.....144.....152.....160.....168.....176.....184... \\",
+ " ......32......40......48......56......64......72......80.. \\",
+ " ......32......40......48......56......64......72......80.. \\",
+ " ......32......40......48.")
vt.Internals(
"18 24 201",
" 24 104",
" 24 104",
" 24")
vt.Diagnostics(
- "NOTE: Makefile:1: The continuation backslash should be preceded by a single space or tab.")
+ "NOTE: Makefile:1: The continuation backslash should be preceded by a single space.",
+ "NOTE: Makefile:2: The continuation backslash should be preceded by a single space.",
+ "NOTE: Makefile:3: The continuation backslash should be preceded by a single space.")
+ vt.Autofixes(
+ "AUTOFIX: Makefile:1: Replacing \"\\t\\t \" with \" \".",
+ "AUTOFIX: Makefile:2: Replacing \"\\t\\t \\t\" with \" \".",
+ "AUTOFIX: Makefile:3: Replacing \"\\t\\t \\t\" with \" \".")
+ vt.Fixed(
+ "INSTALLATION_DIRS= ......32......40......48......56......64......72......80......88..... \\"+
+ " ....136.....144.....152.....160.....168.....176.....184... \\",
+ " ......32......40......48......56......64......72......80.. \\",
+ " ......32......40......48......56......64......72......80.. \\",
+ " ......32......40......48.")
+ vt.Run()
+}
+
+// Each MkLine has its own right margin.
+// As of December 2019, it is ok when these are not aligned per paragraph.
+func (s *Suite) Test_VaralignBlock__right_margin_in_adjacent_lines(c *check.C) {
+ vt := NewVaralignTester(s, c)
+ vt.Input(
+ "VAR1=\t\\",
+ "\t......16\t\t\\",
+ "\t......16\t\t\\",
+ "\t......16",
+ "VAR2=\t\\",
+ "\t......16\t\t\t\\",
+ "\t......16\t\t\t\\",
+ "\t......16")
+ vt.InputDetab(
+ "VAR1= \\",
+ " ......16 \\",
+ " ......16 \\",
+ " ......16",
+ "VAR2= \\",
+ " ......16 \\",
+ " ......16 \\",
+ " ......16")
+ vt.Internals(
+ "05 08 08",
+ " 08 32",
+ " 08 32",
+ " 08",
+ "05 08 08",
+ " 08 40",
+ " 08 40",
+ " 08")
+ vt.Diagnostics(
+ nil...)
vt.Autofixes(
- "AUTOFIX: Makefile:1: Replacing \"\\t\\t \" with \" \".")
+ nil...)
vt.Fixed(
- "INSTALLATION_DIRS= _____________________________________________________________________ \\"+
- " __________________________________________________________ \\",
- " __________________________________________________________ \\",
- " __________________________________________________________ \\",
- " _________________________")
+ "VAR1= \\",
+ " ......16 \\",
+ " ......16 \\",
+ " ......16",
+ "VAR2= \\",
+ " ......16 \\",
+ " ......16 \\",
+ " ......16")
vt.Run()
}
@@ -3004,42 +3057,33 @@ func (s *Suite) Test_VaralignBlock_proce
vt.Run()
}
-func (s *Suite) Test_VaralignBlock_realignMultiEmptyInitial(c *check.C) {
- t := s.Init(c)
-
- mklines := t.NewMkLines("filename.mk",
- MkCvsID,
- "VAR=\t${VAR}",
- "LONG_VARIABLE_NAME= \t \\",
- "\t${LONG_VARIABLE_NAME}")
-
- mklines.Check()
-
- t.CheckOutputLines(
- "NOTE: filename.mk:3: The continuation backslash should be preceded by a single space or tab.")
-}
-
-func (s *Suite) Test_VaralignBlock_realignMultiEmptyInitial__spaces(c *check.C) {
+func (s *Suite) Test_VaralignBlock_Finish__continuation_beyond_right_margin(c *check.C) {
vt := NewVaralignTester(s, c)
+
vt.Input(
- "VAR= \\",
- "\tvalue",
- // This line is necessary to trigger the realignment; see VaralignBlock.Finish.
- "VAR= value")
- vt.Internals(
- "04 08 08",
- " 08",
- "04 05")
- vt.Diagnostics(
- "NOTE: Makefile:1: Variable values should be aligned with tabs, not spaces.",
- "NOTE: Makefile:3: This variable value should be aligned with tabs, not spaces, to column 9.")
- vt.Autofixes(
- "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".",
- "AUTOFIX: Makefile:3: Replacing \" \" with \"\\t\".")
- vt.Fixed(
- "VAR= \\",
- " value",
- "VAR= value")
+ "VAR....8......16..=\t\t......40......48.\t\t\t\t\\", // column 80
+ "\t\t\t......32......40......48......56......64..\t\\", // column 72
+ "\t\t\t...29")
+ vt.InputDetab(
+ "VAR....8......16..= ......40......48. \\",
+ " ......32......40......48......56......64.. \\",
+ " ...29")
+ vt.Diagnostics(
+ // XXX: In this case, it would also help to reduce the indentation
+ // of the variable value.
+ "NOTE: Makefile:1: The continuation backslash should be "+
+ "in column 73, not 81.",
+ // Line 2 is not indented to column 32
+ // since that would make the line longer than 72 columns.
+ "NOTE: Makefile:3: This continuation line should be "+
+ "indented with \"\\t\\t\\t\\t\".")
+ vt.Autofixes(
+ "AUTOFIX: Makefile:1: Replacing \"\\t\\t\\t\\t\" with \"\\t\\t\\t\".",
+ "AUTOFIX: Makefile:3: Replacing \"\\t\\t\\t\" with \"\\t\\t\\t\\t\".")
+ vt.Fixed(
+ "VAR....8......16..= ......40......48. \\",
+ " ......32......40......48......56......64.. \\",
+ " ...29")
vt.Run()
}
@@ -3111,119 +3155,6 @@ func (s *Suite) Test_VaralignBlock_reali
vt.Run()
}
-func (s *Suite) Test_VaralignBlock_realignMultiFollow__unindent_long_lines(c *check.C) {
- vt := NewVaralignTester(s, c)
- vt.Input(
- "SHORT=\tvalue",
- "PROGRAM_AWK=\t\t\t\t--------50--------60--------70 \\",
- "\t\t\t\t\t\t\t\t\t3 \\",
- "\t\t\t\t\t\t\t\t\t74 \\",
- "\t\t\t\t\t\t\t\t\t-75 \t\t\t \\",
- "\t\t\t\t\t\t\t\t\t--76 \\",
- "\t\t\t\t\t\t\t\t66 \\",
- "\t\t\t\t\t\t\t\t1")
- vt.InputDetab(
- "SHORT= value",
- "PROGRAM_AWK= --------50--------60--------70 \\",
- " 3 \\",
- " 74 \\",
- " -75 \\",
- " --76 \\",
- " 66 \\",
- " 1")
- vt.Internals(
- "06 08",
- "12 40 71",
- " 72 89",
- " 72 89",
- " 72 98",
- " 72 77",
- " 64 67",
- " 64")
- vt.Diagnostics(
- "NOTE: Makefile:1: This variable value should be aligned to column 17.",
- "NOTE: Makefile:2: This variable value should be aligned to column 17.",
- "NOTE: Makefile:3: This continuation line should be indented with \"\\t\\t\".",
- "NOTE: Makefile:4: This continuation line should be indented with \"\\t\\t\".",
- "NOTE: Makefile:5: The continuation backslash should be preceded by a single space or tab, or be in column 90, not 99.",
- "NOTE: Makefile:5: This continuation line should be indented with \"\\t\\t\".",
- "NOTE: Makefile:6: This continuation line should be indented with \"\\t\\t\".",
- "NOTE: Makefile:7: This continuation line should be indented with \"\\t\\t\".",
- "NOTE: Makefile:8: This continuation line should be indented with \"\\t\\t\".")
- vt.Autofixes(
- "AUTOFIX: Makefile:1: Replacing \"\\t\" with \"\\t\\t\".",
- "AUTOFIX: Makefile:2: Replacing \"\\t\\t\\t\\t\" with \"\\t\".",
- "AUTOFIX: Makefile:3: Replacing \"\\t\\t\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\\t\".",
- "AUTOFIX: Makefile:4: Replacing \"\\t\\t\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\\t\".",
- "AUTOFIX: Makefile:5: Replacing \" \\t\\t\\t \" with \"\\t\\t \".",
- "AUTOFIX: Makefile:5: Replacing \"\\t\\t\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\\t\".",
- "AUTOFIX: Makefile:6: Replacing \"\\t\\t\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\\t\".",
- "AUTOFIX: Makefile:7: Replacing \"\\t\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\".",
- "AUTOFIX: Makefile:8: Replacing \"\\t\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\".")
- vt.Fixed(
- // After shifting the lines to the left, none of the lines is
- // considered "long" anymore, therefore the backslashes are not
- // kept in column 72. Nevertheless they look unorganized right now.
- "SHORT= value",
- "PROGRAM_AWK= --------50--------60--------70 \\",
- // FIXME: only use a single space before the backslash.
- " 3 \\",
- // FIXME: only use a single space before the backslash.
- " 74 \\",
- // FIXME: only use a single space before the backslash.
- " -75 \\",
- " --76 \\",
- " 66 \\",
- " 1")
- vt.Run()
-}
-
-func (s *Suite) Test_VaralignBlock_realignMultiFollow__unindent_long_initial_line(c *check.C) {
- vt := NewVaralignTester(s, c)
- vt.Input(
- "VAR-----10!=\t\t----30--------40--------50-----6\t\t\t\\",
- "\t\t --------30--------40-\t\t\t\t\\",
- "\t\t --------30--------40--------50--------60-------8\t\\",
- "\t\t ----5\t\t\t\t\t\t\\",
- "\t\t-7")
- vt.InputDetab(
- "VAR-----10!= ----30--------40--------50-----6 \\",
- " --------30--------40- \\",
- " --------30--------40--------50--------60-------8 \\",
- " ----5 \\",
- " -7")
- vt.Internals(
- "12 24 80",
- " 20 72",
- " 20 72",
- " 20 72",
- " 16")
- vt.Diagnostics(
- "NOTE: Makefile:1: The continuation backslash should be preceded by a single space or tab, or be in column 73, not 81.",
- "NOTE: Makefile:2: This continuation line should be indented with \"\\t\\t\\t\".",
- "NOTE: Makefile:3: This continuation line should be indented with \"\\t\\t\\t\".",
- "NOTE: Makefile:4: This continuation line should be indented with \"\\t\\t\\t\".",
- "NOTE: Makefile:5: This continuation line should be indented with \"\\t\\t\\t\".")
- vt.Autofixes(
- // FIXME: Mention the continuation backslash in the replacement.
- "AUTOFIX: Makefile:1: Replacing \"\\t\\t\\t\" with \"\\t\\t\".",
- "AUTOFIX: Makefile:2: Replacing \"\\t\\t \" with \"\\t\\t\\t\".",
- "AUTOFIX: Makefile:3: Replacing \"\\t\\t \" with \"\\t\\t\\t\".",
- "AUTOFIX: Makefile:3: Replacing \"\\t\\\\\" with \" \\\\\".",
- "AUTOFIX: Makefile:4: Replacing \"\\t\\t \" with \"\\t\\t\\t\".",
- "AUTOFIX: Makefile:5: Replacing \"\\t\\t\" with \"\\t\\t\\t\".")
- vt.Fixed(
- "VAR-----10!= ----30--------40--------50-----6 \\",
- // FIXME: Preserve the original relative indentation.
- " --------30--------40- \\",
- // FIXME: Preserve the original relative indentation.
- " --------30--------40--------50--------60-------8 \\",
- // FIXME: Preserve the original relative indentation.
- " ----5 \\",
- " -7")
- vt.Run()
-}
-
func (s *Suite) Test_VaralignSplitter_split(c *check.C) {
t := s.Init(c)
@@ -3446,6 +3377,700 @@ func (s *Suite) Test_VaralignSplitter_sp
func() { test("VAR=\tvalue\n", true, varalignParts{}) })
}
+func (s *Suite) Test_varalignMkLine_rightMargin(c *check.C) {
+ t := s.Init(c)
+
+ test := func(common bool, margin int, lines ...string) {
+ t.CheckDotColumns(lines...)
+ mklines := t.NewMkLines("filename.mk",
+ lines...)
+
+ var block VaralignBlock
+ mklines.ForEach(func(mkline *MkLine) {
+ block.Process(mkline)
+ })
+
+ t.Check(block.mkinfos, check.HasLen, 1)
+ for _, mkinfo := range block.mkinfos {
+ actualCommon, actualMargin := mkinfo.rightMargin()
+ t.CheckDeepEquals(
+ []interface{}{actualCommon, actualMargin},
+ []interface{}{common, margin})
+ }
+ }
+
+ // Lines without continuation don't have a right margin.
+ test(false, 0,
+ "VAR=\t...13")
+
+ // Lines with a needless continuation also don't have a right margin.
+ // The backslash in the first line is ignored completely since it
+ // is commonly aligned at a different position than the other
+ // backslashes. Either with a single space (which wouldn't matter
+ // anyway) or with some tabs to the column of the variable value.
+ test(false, 0,
+ "VAR=\t\\",
+ "\tvalue")
+
+ test(false, 0,
+ "VAR=\t\t\t\\",
+ "\tvalue")
+
+ // Single spaces are ignored since they do not explicitly express
+ // the intention to draw a common right margin.
+ test(false, 0,
+ "VAR= \\",
+ "\t......16........26 \\",
+ "\tvalue")
+
+ // Single tabs take part in the right margin since they are already
+ // aligned at a multiple of 8. Pkglint therefore assumes that it is
+ // intended to have a common right margin.
+ //
+ // There is no common margin, and the minimum necessary margin is 32.
+ test(false, 32,
+ "VAR=\t\\",
+ "\t\\",
+ "\t......16........26\t\\",
+ "\tvalue")
+
+ // The first line is ignored since it is empty, which leaves only
+ // a single remaining line. That is not enough to decide whether
+ // a right margin is really intended (or is it?), therefore no
+ // right margin is assumed.
+ test(false, 0,
+ "VAR=\t\\",
+ "\t......16........26\t\\",
+ "\tvalue")
+
+ // In long lines, the backslash is usually separated by a single
+ // space and is therefore ignored.
+ // All remaining backslashes (there is only 1) are aligned in the
+ // same column.
+ //
+ // XXX: Why is 1 relevant backslash not enough?
+ test(false, 0,
+ "VAR=\t\t\\",
+ "\t......16......24......32......40 \\",
+ "\tvalue")
+
+ // Again, the first line is ignored, and each remaining line has
+ // the backslash in a different position. This means no common
+ // margin.
+ test(false, 0,
+ "VAR=\t\t\\",
+ "\t\t\\",
+ "\t......16......24......32......40 \\",
+ "\tvalue")
+
+ // When there are at least 2 relevant backslashes, they produce a
+ // right margin.
+ test(true, 16,
+ "VAR=\t...13\t\\",
+ "\t\t\\",
+ "\t......16......24......32......40 \\",
+ "\tvalue")
+
+ // If a long line uses a tab (and thereby becomes longer than
+ // strictly necessary), that is a sign that the line is not
+ // thought to be overly long, therefore it is probably desired
+ // to align all lines in that column.
+ test(false, 48,
+ "VAR=\t...13\t\\",
+ "\t......16......24......32......40\t\\",
+ "\t...13")
+
+ // If the relevant backslashes are in different columns, there is
+ // no common right margin.
+ test(false, 16,
+ "VAR=\t...13\t\\", // column 16
+ "\t...13\t\t\t\t\\", // column 40
+ "\t...13")
+
+ // It doesn't matter whether the backslash is aligned using spaces
+ // or tabs, as they are visually the same.
+ test(false, 16,
+ "VAR= ...13 \\", // column 16
+ "\t...13\t\t\t\t\\", // column 40
+ "\t...13")
+
+ // The common right margin is determined by starting from the right
+ // and searching until there are at least 2 lines having the same
+ // right margin.
+ test(false, 40,
+ "VAR=\t\\", // column 16
+ "\tv\t\t\t\t\t\\", // column 48
+ "\tv\t\t\t\\", // column 32
+ "\tv\t\t\t\t\\", // column 40
+ "\tv\t\t\t\t\t\t\\", // column 56
+ "\tv\t\t\t\t\\", // column 40, for the second time
+ "\tv\t\t\\", // column 24
+ "\tv")
+
+ // All backslashes are in different columns.
+ // Therefore, suggest the minimum possible column for the backslashes.
+ test(false, 16,
+ "VAR=\t\\", // column 16
+ "\tv\t\t\t\t\t\\", // column 48
+ "\tv\t\t\t\\", // column 32
+ "\tv\t\t\t\t\\", // column 40
+ "\tv\t\t\t\t\t\t\\", // column 56
+ "\tv\t\t\\", // column 24
+ "\tv")
+
+ // The decision of choosing column 24 may feel somewhat arbitrary,
+ // but this whole situation is artificially constructed anyway.
+ //
+ // The intention of choosing the largest column with at least two
+ // backslashes is simply that this will fix most practical occurrences
+ // where most of the backslashes are already aligned in that column,
+ // and only a few are in column 64.
+ test(false, 24,
+ "VAR=\t\\", // column 16
+ "\tv\t\t\t\\", // column 32
+ "\tv\t\t\t\t\\", // column 40
+ "\t......16......24......32\t\t\\", // column 48
+ "\tv\t\t\\", // column 24
+ "\tv\t\t\\", // column 24, appears twice
+ "\tv")
+
+ // The reasonable maximum value for the right margin is 72 since
+ // that column is the last that is still visible on an 80x25 display.
+ test(false, 72,
+ "VAR=\t\\", // column 16
+ "\tv\t\t\t\t\t\t\t\t\t\t\t\\", // column 96
+ "\tv\t\t\t\t\t\t\t\t\t\t\t\\", // column 96
+ "\tv\t\t\t\t\t\t\t\t\t\t\t\\", // column 96
+ "\tv\t\t\t\t\t\t\t\t\t\t\t\\", // column 96
+ "\tv\t\t\t\t\\", // column 40
+ "\tv\t\t\\", // column 24
+ "\tv\t\t\\", // column 24, appears twice
+ "\tv")
+
+ // The continuation backslash in the first line is too far to the right.
+ test(false, 72,
+ "VAR....8......16..=\t\t......40......48.\t\t\t\t\\", // column 80
+ "\t\t\t......32......40......48......56......64..\t\\", // column 72
+ "\t\t\t...29")
+}
+
+func (s *Suite) Test_varalignLine_alignValueSingle(c *check.C) {
+ t := s.Init(c)
+
+ test := func(before string, column int, after string, diagnostics ...string) {
+
+ doTest := func(autofix bool) {
+ t.CheckDotColumns(before)
+ mkline := t.NewMkLine("filename.mk", 123, before)
+ parts := NewVaralignSplitter().split(before, true)
+ info := &varalignLine{mkline, 0, false, false, parts}
+
+ info.alignValueSingle(column)
+
+ t.CheckEqualsf(
+ mkline.raw[0].text(),
+ condStr(autofix, after, before),
+ "Line.raw.text, autofix=%v", autofix)
+
+ // As of 2019-12-11, the info fields are not updated
+ // accordingly, but they should.
+ // TODO: update info accordingly
+ t.CheckEqualsf(info.String(), before,
+ "info.String, autofix=%v", autofix)
+ }
+
+ t.ExpectDiagnosticsAutofix(doTest, diagnostics...)
+ }
+
+ // The variable value is already in column 8, thus nothing to fix.
+ test(
+ "VAR=\tvalue",
+ 8,
+
+ "VAR=\tvalue",
+ nil...)
+
+ // Aligned to the wrong column, using only tabs.
+ test(
+ "VAR=\tvalue",
+ 16,
+
+ "VAR=\t\tvalue",
+ "NOTE: filename.mk:123: This variable value "+
+ "should be aligned to column 17.",
+ "AUTOFIX: filename.mk:123: Replacing \"\\t\" with \"\\t\\t\".")
+
+ // Aligned to the wrong column, using a mixture of tabs and spaces.
+ test(
+ "VAR=\t value",
+ 16,
+
+ "VAR=\t\tvalue",
+ "NOTE: filename.mk:123: This variable value "+
+ "should be aligned with tabs, not spaces, to column 17.",
+ "AUTOFIX: filename.mk:123: Replacing \"\\t \" with \"\\t\\t\".")
+
+ // Correct column, but using spaces for indentation.
+ test(
+ "VAR= \t \tvalue",
+ 16,
+
+ "VAR=\t\tvalue",
+ "NOTE: filename.mk:123: Variable values "+
+ "should be aligned with tabs, not spaces.",
+ "AUTOFIX: filename.mk:123: Replacing \" \\t \\t\" with \"\\t\\t\".")
+
+ // If the value is indented more than necessary, the redundant
+ // indentation is dropped.
+ test(
+ "VAR=\t\t\t\t\tvalue",
+ 8,
+
+ "VAR=\tvalue",
+ "NOTE: filename.mk:123: "+
+ "This variable value should be aligned to column 9.",
+ "AUTOFIX: filename.mk:123: Replacing \"\\t\\t\\t\\t\\t\" with \"\\t\".")
+
+ // An outlier should use a single space, to be as far to the
+ // left as possible.
+ //
+ // XXX: Why is this line not considered an outlier?
+ // info.isCanonicalInitial returns true for it.
+ test(
+ "VAR....8......16......24......32=\t\t\t\t\tvalue",
+ 8,
+
+ "VAR....8......16......24......32=\t\t\t\t\tvalue",
+ nil...)
+
+ // An outlier should use a single space, to be as far to the
+ // left as possible.
+ test(
+ "VAR....8......16......24......32=\t\t\t \t\tvalue",
+ 8,
+
+ "VAR....8......16......24......32= value",
+ "NOTE: filename.mk:123: This outlier variable value "+
+ "should be aligned with a single space.",
+ "AUTOFIX: filename.mk:123: Replacing \"\\t\\t\\t \\t\\t\" with \" \".")
+}
+
+func (s *Suite) Test_varalignLine_alignValueMultiEmptyInitial(c *check.C) {
+ t := s.Init(c)
+
+ mklines := t.NewMkLines("filename.mk",
+ MkCvsID,
+ "VAR=\t${VAR}",
+ "LONG_VARIABLE_NAME= \t \\",
+ "\t${LONG_VARIABLE_NAME}")
+
+ mklines.Check()
+
+ t.CheckOutputLines(
+ "NOTE: filename.mk:3: The continuation backslash should be preceded by a single space or tab.")
+}
+
+func (s *Suite) Test_varalignLine_alignValueMultiEmptyInitial__spaces(c *check.C) {
+ vt := NewVaralignTester(s, c)
+ vt.Input(
+ "VAR= \\",
+ "\tvalue",
+ // This line is necessary to trigger the realignment; see VaralignBlock.Finish.
+ "VAR= value")
+ vt.Internals(
+ "04 08 08",
+ " 08",
+ "04 05")
+ vt.Diagnostics(
+ "NOTE: Makefile:1: Variable values should be aligned with tabs, not spaces.",
+ "NOTE: Makefile:3: This variable value should be aligned with tabs, not spaces, to column 9.")
+ vt.Autofixes(
+ "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".",
+ "AUTOFIX: Makefile:3: Replacing \" \" with \"\\t\".")
+ vt.Fixed(
+ "VAR= \\",
+ " value",
+ "VAR= value")
+ vt.Run()
+}
+
+func (s *Suite) Test_varalignLine_alignValueMultiInitial(c *check.C) {
+ t := s.Init(c)
+
+ test := func(before string, column int, after string, diagnostics ...string) {
+ t.CheckDotColumns(before)
+
+ doTest := func(autofix bool) {
+ mklines := t.NewMkLines("filename.mk",
+ before,
+ "\t...13")
+ assert(len(mklines.mklines) == 1)
+ mkline := mklines.mklines[0]
+
+ text := mkline.raw[0].text()
+ parts := NewVaralignSplitter().split(text, true)
+ info := &varalignLine{mkline, 0, false, false, parts}
+
+ info.alignValueMultiInitial(column)
+
+ t.CheckEqualsf(
+ mkline.raw[0].text(),
+ condStr(autofix, after, before),
+ "Line.raw.text, autofix=%v", autofix)
+
+ // As of 2019-12-11, the info fields are not updated
+ // accordingly, but they should.
+ // TODO: update info accordingly
+ t.CheckEqualsf(info.String(), before,
+ "info.String, autofix=%v", autofix)
+ }
+
+ t.ExpectDiagnosticsAutofix(doTest, diagnostics...)
+ }
+
+ // The value is already in column 8, thus nothing to do.
+ test(
+ "VAR=\tvalue \\",
+ 8,
+
+ "VAR=\tvalue \\",
+ nil...)
+
+ test(
+ "VAR=\tvalue \\",
+ 16,
+
+ "VAR=\t\tvalue \\",
+ "NOTE: filename.mk:1: This variable value should be aligned to column 17.",
+ "AUTOFIX: filename.mk:1: Replacing \"\\t\" with \"\\t\\t\".")
+
+ // The column is already correct,
+ // but the alignment should be done with tabs, not spaces.
+ test(
+ "VAR= \t \tvalue \\",
+ 16,
+
+ "VAR=\t\tvalue \\",
+ "NOTE: filename.mk:1: Variable values should be aligned with tabs, not spaces.",
+ "AUTOFIX: filename.mk:1: Replacing \" \\t \\t\" with \"\\t\\t\".")
+
+ // Both the column and the use of spaces in the alignment
+ // need to be fixed.
+ test(
+ "VAR= \t value \\",
+ 16,
+
+ "VAR=\t\tvalue \\",
+ "NOTE: filename.mk:1: This variable value should be aligned with tabs, not spaces, to column 17.",
+ "AUTOFIX: filename.mk:1: Replacing \" \\t \" with \"\\t\\t\".")
+}
+
+func (s *Suite) Test_varalignLine_alignValueMultiEmptyFollow(c *check.C) {
+ t := s.Init(c)
+
+ test := func(diagnostics ...string) {
+ // FIXME
+ t.CheckOutput(diagnostics)
+ }
+
+ test()
+}
+
+func (s *Suite) Test_varalignLine_alignValueMultiFollow__unindent_long_lines(c *check.C) {
+ vt := NewVaralignTester(s, c)
+ vt.Input(
+ "SHORT=\tvalue",
+ "PROGRAM_AWK=\t\t\t\t--------50--------60--------70 \\",
+ "\t\t\t\t\t\t\t\t\t3 \\",
+ "\t\t\t\t\t\t\t\t\t74 \\",
+ "\t\t\t\t\t\t\t\t\t-75 \t\t\t \\",
+ "\t\t\t\t\t\t\t\t\t--76 \\",
+ "\t\t\t\t\t\t\t\t66 \\",
+ "\t\t\t\t\t\t\t\t1")
+ vt.InputDetab(
+ "SHORT= value",
+ "PROGRAM_AWK= --------50--------60--------70 \\",
+ " 3 \\",
+ " 74 \\",
+ " -75 \\",
+ " --76 \\",
+ " 66 \\",
+ " 1")
+ vt.Internals(
+ "06 08",
+ "12 40 71",
+ " 72 89",
+ " 72 89",
+ " 72 98",
+ " 72 77",
+ " 64 67",
+ " 64")
+ vt.Diagnostics(
+ "NOTE: Makefile:1: This variable value should be aligned to column 17.",
+ "NOTE: Makefile:2: This variable value should be aligned to column 17.",
+ // XXX: Wrong order; should be strictly from left to right.
+ "NOTE: Makefile:3: The continuation backslash should be preceded by a single space or tab.",
+ "NOTE: Makefile:3: This continuation line should be indented with \"\\t\\t\\t\\t\\t\\t\".",
+ "NOTE: Makefile:4: The continuation backslash should be preceded by a single space or tab.",
+ "NOTE: Makefile:4: This continuation line should be indented with \"\\t\\t\\t\\t\\t\\t\".",
+ "NOTE: Makefile:5: The continuation backslash should be preceded by a single space or tab.",
+ "NOTE: Makefile:5: This continuation line should be indented with \"\\t\\t\\t\\t\\t\\t\".",
+ "NOTE: Makefile:6: This continuation line should be indented with \"\\t\\t\\t\\t\\t\\t\".",
+ "NOTE: Makefile:7: This continuation line should be indented with \"\\t\\t\\t\\t\\t\".",
+ "NOTE: Makefile:8: This continuation line should be indented with \"\\t\\t\\t\\t\\t\".")
+ vt.Autofixes(
+ "AUTOFIX: Makefile:1: Replacing \"\\t\" with \"\\t\\t\".",
+ "AUTOFIX: Makefile:2: Replacing \"\\t\\t\\t\\t\" with \"\\t\".",
+ "AUTOFIX: Makefile:3: Replacing \" \" with \" \".",
+ "AUTOFIX: Makefile:3: Replacing \"\\t\\t\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\\t\".",
+ "AUTOFIX: Makefile:4: Replacing \" \" with \" \".",
+ "AUTOFIX: Makefile:4: Replacing \"\\t\\t\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\\t\".",
+ "AUTOFIX: Makefile:5: Replacing \" \\t\\t\\t \" with \" \".",
+ "AUTOFIX: Makefile:5: Replacing \"\\t\\t\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\\t\".",
+ "AUTOFIX: Makefile:6: Replacing \"\\t\\t\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\\t\".",
+ "AUTOFIX: Makefile:7: Replacing \"\\t\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\".",
+ "AUTOFIX: Makefile:8: Replacing \"\\t\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\".")
+ vt.Fixed(
+ // After shifting the lines to the left, none of the lines is
+ // considered "long" anymore, therefore the backslashes are not
+ // kept in column 72. Nevertheless they look unorganized right now.
+ "SHORT= value",
+ "PROGRAM_AWK= --------50--------60--------70 \\",
+ " 3 \\",
+ " 74 \\",
+ " -75 \\",
+ " --76 \\",
+ " 66 \\",
+ " 1")
+ vt.Run()
+}
+
+func (s *Suite) Test_varalignLine_alignValueMultiFollow__unindent_long_initial_line(c *check.C) {
+ vt := NewVaralignTester(s, c)
+ vt.Input(
+ "VAR-----10!=\t\t----30--------40--------50-----6\t\t\t\\",
+ "\t\t --------30--------40-\t\t\t\t\\",
+ "\t\t --------30--------40--------50--------60-------8\t\\",
+ "\t\t ----5\t\t\t\t\t\t\\",
+ "\t\t-7")
+ vt.InputDetab(
+ "VAR-----10!= ----30--------40--------50-----6 \\",
+ " --------30--------40- \\",
+ " --------30--------40--------50--------60-------8 \\",
+ " ----5 \\",
+ " -7")
+ vt.Internals(
+ "12 24 80",
+ " 20 72",
+ " 20 72",
+ " 20 72",
+ " 16")
+ vt.Diagnostics(
+ "NOTE: Makefile:1: The continuation backslash should be in column 73, not 81.",
+ "NOTE: Makefile:2: This continuation line should be indented with \"\\t\\t\\t\".",
+ "NOTE: Makefile:4: This continuation line should be indented with \"\\t\\t\\t\".",
+ "NOTE: Makefile:5: This continuation line should be indented with \"\\t\\t\\t\".")
+ vt.Autofixes(
+ // FIXME: Mention the continuation backslash in the replacement.
+ "AUTOFIX: Makefile:1: Replacing \"\\t\\t\\t\" with \"\\t\\t\".",
+ "AUTOFIX: Makefile:2: Replacing \"\\t\\t \" with \"\\t\\t\\t\".",
+ "AUTOFIX: Makefile:4: Replacing \"\\t\\t \" with \"\\t\\t\\t\".",
+ "AUTOFIX: Makefile:5: Replacing \"\\t\\t\" with \"\\t\\t\\t\".")
+ vt.Fixed(
+ "VAR-----10!= ----30--------40--------50-----6 \\",
+ // FIXME: Preserve the original relative indentation.
+ " --------30--------40- \\",
+ // FIXME: Preserve the original relative indentation.
+ " --------30--------40--------50--------60-------8 \\",
+ // FIXME: Preserve the original relative indentation.
+ " ----5 \\",
+ " -7")
+ vt.Run()
+}
+
+func (s *Suite) Test_varalignLine_alignContinuation(c *check.C) {
+ t := s.Init(c)
+
+ lines := func(lines ...string) []string { return lines }
+ test := func(before []string, rawIndex, valueColumn, rightMarginColumn int, after string, diagnostics ...string) {
+ t.CheckDotColumns(before...)
+
+ doTest := func(autofix bool) {
+ mklines := t.NewMkLines("filename.mk", before...)
+ assert(len(mklines.mklines) == 1)
+ mkline := mklines.mklines[0]
+
+ text := mkline.raw[rawIndex].text()
+ parts := NewVaralignSplitter().split(text, rawIndex == 0)
+ info := &varalignLine{mkline, rawIndex, false, false, parts}
+
+ info.alignContinuation(valueColumn, rightMarginColumn)
+
+ t.CheckEqualsf(
+ mkline.raw[rawIndex].text(),
+ condStr(autofix, after, before[rawIndex]),
+ "Line.raw.text, autofix=%v", autofix)
+
+ // As of 2019-12-11, the info fields are not updated
+ // accordingly, but they should.
+ // TODO: update info accordingly
+ t.CheckEqualsf(info.String(), before[rawIndex],
+ "info.String, autofix=%v", autofix)
+ }
+
+ t.ExpectDiagnosticsAutofix(doTest, diagnostics...)
+ }
+
+ // In this line, there is no continuation backslash,
+ // thus nothing to align.
+ test(
+ lines(
+ "VAR=\t...13"),
+ 0, 32, 48,
+
+ "VAR=\t...13",
+ nil...)
+
+ // The continuation backslash in line 1 is already canonical,
+ // independently of the alignment column for the variable values.
+ //
+ // XXX: Maybe later enforce the continuation backslash to be
+ // aligned at newWidth.
+ test(
+ lines(
+ "VAR=\t...13 \\",
+ "\t...13"),
+ 0, 32, 48,
+
+ "VAR=\t...13 \\",
+ nil...)
+
+ // In line 2, there is no continuation backslash,
+ // thus nothing to align.
+ test(
+ lines(
+ "VAR=\t...13 \\",
+ "\t...13"),
+ 1, 32, 48,
+
+ "\t...13",
+ nil...)
+
+ // A single tab before the continuation backslash is always
+ // considered canonical, thus nothing to align.
+ //
+ // XXX: Why? It would be better to force the tab to the valueColumn.
+ test(
+ lines(
+ "VAR=\t...13\t\\",
+ "\t...13"),
+ 0, 32, 48,
+
+ "VAR=\t...13\t\\",
+ nil...)
+
+ // Column 24 is left of the valueAlign,
+ // therefore there is nothing to align.
+ //
+ // XXX: Why? It would be better to force the tab to the valueColumn.
+ test(
+ lines(
+ "VAR=\t...13\t\t\\",
+ "\t...13"),
+ 0, 32, 48,
+
+ "VAR=\t...13\t\t\\",
+ nil...)
+
+ // Column 32 is the valueColumn, therefore there is nothing to align.
+ test(
+ lines(
+ "VAR=\t...13\t\t\t\\",
+ "\t...13"),
+ 0, 32, 48,
+
+ "VAR=\t...13\t\t\t\\",
+ nil...)
+
+ // Column 40 is somewhere between the valueColumn and the right margin
+ // of the MkLine and is thus arbitrary.
+ // It is aligned to the right margin.
+ test(
+ lines(
+ "VAR=\t...13\t\t\t\t\\",
+ "\t...13"),
+ 0, 32, 48,
+
+ "VAR=\t...13\t\t\t\t\t\\",
+ "NOTE: filename.mk:1: "+
+ "The continuation backslash should be in column 49, not 41.",
+ "AUTOFIX: filename.mk:1: Replacing \"\\t\\t\\t\\t\" "+
+ "with \"\\t\\t\\t\\t\\t\".")
+
+ // Column 48 is already at the right margin, thus nothing to align.
+ test(
+ lines(
+ "VAR=\t......16......24......32\t\t\\",
+ "\t...13"),
+ 0, 32, 48,
+
+ "VAR=\t......16......24......32\t\t\\",
+ nil...)
+
+ // Column 56 is right of the right margin.
+ // It is reduced to the right margin.
+ test(
+ lines(
+ "VAR=\t......16......24\t\t\t\t\\",
+ "\t...13"),
+ 0, 32, 48,
+
+ "VAR=\t......16......24\t\t\t\\",
+ "NOTE: filename.mk:1: "+
+ "The continuation backslash should be in column 49, not 57.",
+ "AUTOFIX: filename.mk:1: Replacing \"\\t\\t\\t\\t\" "+
+ "with \"\\t\\t\\t\".")
+
+ // Column 72 is the "natural" column for continuation backslashes,
+ // therefore there is nothing to align.
+ test(
+ lines(
+ "VAR=\t...13\t\t\t\t\t\t\t\t\\",
+ "\t...13"),
+ 0, 32, 48,
+
+ "VAR=\t...13\t\t\t\t\t\t\t\t\\",
+ nil...)
+
+ // If the value itself forces the continuation backslash to be beyond
+ // column 72, the continuation backslash should only be separated by
+ // a single space, to keep it as close to the text as possible.
+ test(
+ lines(
+ "VAR=\t...13\t\t\t\t\t\t\t\t...77\t\t\t\\",
+ "\t...13"),
+ 0, 32, 48,
+
+ "VAR=\t...13\t\t\t\t\t\t\t\t...77 \\",
+ "NOTE: filename.mk:1: The continuation backslash should be "+
+ "preceded by a single space.",
+ "AUTOFIX: filename.mk:1: Replacing \"\\t\\t\\t\" with \" \".")
+}
+
+func (s *Suite) Test_varalignLine_explainWrongColumn(c *check.C) {
+ t := s.Init(c)
+
+ mkline := t.NewMkLine("filename.mk", 123, "")
+ fix := mkline.Autofix()
+ fix.Notef("Note.")
+ (*varalignLine)(nil).explainWrongColumn(fix)
+ fix.Apply()
+
+ t.CheckOutputLines(
+ "NOTE: filename.mk:123: Note.")
+ t.CheckEquals(G.Logger.explanationsAvailable, true)
+}
+
// This constellation doesn't occur in practice because the code in
// VaralignBlock.processVarassign skips it, see INCLUSION_GUARD_MK.
func (s *Suite) Test_varalignParts_isEmptyContinuation__edge_case(c *check.C) {
@@ -3538,3 +4163,25 @@ func (s *Suite) Test_varalignParts_isCan
test("#", " ", false)
test("#", "\t", true)
}
+
+func (s *Suite) Test_varalignParts_isTooLongFor(c *check.C) {
+ t := s.Init(c)
+
+ test := func(valueColumn int, value, continuation string, tooLong bool) {
+ t.CheckDotColumns(indent(valueColumn) + value)
+
+ parts := varalignParts{value: value, continuation: continuation}
+ t.CheckEquals(parts.isTooLongFor(valueColumn), tooLong)
+ }
+
+ // In lines without continuation backslash,
+ // the value may go up to the continuation backslash.
+ test(64, ".......73", "", false)
+ test(64, "........74", "", true)
+
+ // In lines with continuation backslash,
+ // the value including a space and the continuation backslash
+ // may go up to column 73.
+ test(64, ".....71", "\\", false)
+ test(64, "......72", "\\", true)
+}
Index: pkgsrc/pkgtools/pkglint/files/vardefs.go
diff -u pkgsrc/pkgtools/pkglint/files/vardefs.go:1.82 pkgsrc/pkgtools/pkglint/files/vardefs.go:1.83
--- pkgsrc/pkgtools/pkglint/files/vardefs.go:1.82 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/vardefs.go Fri Dec 13 01:39:23 2019
@@ -560,8 +560,8 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.usr("PKGSRC_USE_SSP", enum("no yes strong all"))
reg.usr("PKGSRC_USE_STACK_CHECK", enum("no yes"))
reg.usr("PREFER.*", enum("pkgsrc native"))
- reg.usrlist("PREFER_PKGSRC", BtIdentifier)
- reg.usrlist("PREFER_NATIVE", BtIdentifier)
+ reg.usrlist("PREFER_PKGSRC", BtIdentifierDirect)
+ reg.usrlist("PREFER_NATIVE", BtIdentifierDirect)
reg.usr("PREFER_NATIVE_PTHREADS", BtYesNo)
reg.usr("WRKOBJDIR", BtPathname)
reg.usr("LOCALBASE", BtPathname)
@@ -596,7 +596,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.usr("PACKAGES", BtPathname)
reg.usr("PASSIVE_FETCH", BtYes)
reg.usr("PATCH_FUZZ_FACTOR", enum("none -F0 -F1 -F2 -F3"))
- reg.usrlist("ACCEPTABLE_LICENSES", BtIdentifier)
+ reg.usrlist("ACCEPTABLE_LICENSES", BtIdentifierIndirect)
reg.usr("SPECIFIC_PKGS", BtYes)
reg.usrlist("SITE_SPECIFIC_PKGS", BtPkgpath)
reg.usrlist("HOST_SPECIFIC_PKGS", BtPkgpath)
@@ -638,7 +638,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.usr("CLAMAV_GROUP", BtUserGroupName)
reg.usr("CLAMAV_USER", BtUserGroupName)
reg.usr("CLAMAV_DBDIR", BtPathname)
- reg.usr("CONSERVER_DEFAULTHOST", BtIdentifier)
+ reg.usr("CONSERVER_DEFAULTHOST", BtIdentifierIndirect)
reg.usr("CONSERVER_DEFAULTPORT", BtInteger)
reg.usr("CUPS_GROUP", BtUserGroupName)
reg.usr("CUPS_USER", BtUserGroupName)
@@ -653,7 +653,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.usr("DEFANG_GROUP", BtUserGroupName)
reg.usr("DEFANG_USER", BtUserGroupName)
reg.usr("DEFANG_SPOOLDIR", BtPathname)
- reg.usr("DEFAULT_IRC_SERVER", BtIdentifier)
+ reg.usr("DEFAULT_IRC_SERVER", BtIdentifierIndirect)
reg.usr("DEFAULT_SERIAL_DEVICE", BtPathname)
reg.usr("DIALER_GROUP", BtUserGroupName)
reg.usr("DJBDNS_AXFR_USER", BtUserGroupName)
@@ -687,7 +687,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.usr("GAMEDATAMODE", BtFileMode)
reg.usr("GAMEGRP", BtUserGroupName)
reg.usr("GAMEOWN", BtUserGroupName)
- reg.usr("GRUB_NETWORK_CARDS", BtIdentifier)
+ reg.usr("GRUB_NETWORK_CARDS", BtIdentifierIndirect)
reg.usr("GRUB_PRESET_COMMAND", enum("bootp dhcp rarp"))
reg.usrlist("GRUB_SCAN_ARGS", BtShellWord)
reg.usr("HASKELL_COMPILER", enum("ghc"))
@@ -706,7 +706,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.usr("IRCD_HYBRID_NICLEN", BtInteger)
reg.usr("IRCD_HYBRID_TOPICLEN", BtInteger)
reg.usr("IRCD_HYBRID_SYSLOG_EVENTS", BtUnknown)
- reg.usr("IRCD_HYBRID_SYSLOG_FACILITY", BtIdentifier)
+ reg.usr("IRCD_HYBRID_SYSLOG_FACILITY", BtIdentifierIndirect)
reg.usr("IRCD_HYBRID_MAXCONN", BtInteger)
reg.usr("IRCD_HYBRID_IRC_USER", BtUserGroupName)
reg.usr("IRCD_HYBRID_IRC_GROUP", BtUserGroupName)
@@ -721,7 +721,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.usr("KERMIT_SUID_UUCP", BtYes)
reg.usr("KJS_USE_PCRE", BtYes)
reg.usr("KNEWS_DOMAIN_FILE", BtPathname)
- reg.usr("KNEWS_DOMAIN_NAME", BtIdentifier)
+ reg.usr("KNEWS_DOMAIN_NAME", BtIdentifierIndirect)
reg.usr("LIBDVDCSS_HOMEPAGE", BtHomepage)
reg.usrlist("LIBDVDCSS_MASTER_SITES", BtFetchURL)
reg.usr("LIBUSB_TYPE", enum("compat native"))
@@ -729,14 +729,14 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.usr("LEAFNODE_DATA_DIR", BtPathname)
reg.usr("LEAFNODE_USER", BtUserGroupName)
reg.usr("LEAFNODE_GROUP", BtUserGroupName)
- reg.usrlist("LINUX_LOCALES", BtIdentifier)
- reg.usr("MAILAGENT_DOMAIN", BtIdentifier)
+ reg.usrlist("LINUX_LOCALES", BtIdentifierIndirect)
+ reg.usr("MAILAGENT_DOMAIN", BtIdentifierIndirect)
reg.usr("MAILAGENT_EMAIL", BtMailAddress)
- reg.usr("MAILAGENT_FQDN", BtIdentifier)
+ reg.usr("MAILAGENT_FQDN", BtIdentifierIndirect)
reg.usr("MAILAGENT_ORGANIZATION", BtUnknown)
reg.usr("MAJORDOMO_HOMEDIR", BtPathname)
reg.usrlist("MAKEINFO_ARGS", BtShellWord)
- reg.usr("MECAB_CHARSET", BtIdentifier)
+ reg.usr("MECAB_CHARSET", BtIdentifierIndirect)
reg.usr("MEDIATOMB_GROUP", BtUserGroupName)
reg.usr("MEDIATOMB_USER", BtUserGroupName)
reg.usr("MIREDO_USER", BtUserGroupName)
@@ -752,16 +752,16 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.usr("MYSQL_USER", BtUserGroupName)
reg.usr("MYSQL_GROUP", BtUserGroupName)
reg.usr("MYSQL_DATADIR", BtPathname)
- reg.usr("MYSQL_CHARSET", BtIdentifier)
- reg.usrlist("MYSQL_EXTRA_CHARSET", BtIdentifier)
+ reg.usr("MYSQL_CHARSET", BtIdentifierIndirect)
+ reg.usrlist("MYSQL_EXTRA_CHARSET", BtIdentifierIndirect)
reg.usr("NAGIOS_GROUP", BtUserGroupName)
reg.usr("NAGIOS_USER", BtUserGroupName)
reg.usr("NAGIOSCMD_GROUP", BtUserGroupName)
reg.usr("NAGIOSDIR", BtPathname)
reg.usr("NBPAX_PROGRAM_PREFIX", BtUnknown)
- reg.usr("NMH_EDITOR", BtIdentifier)
+ reg.usr("NMH_EDITOR", BtIdentifierIndirect)
reg.usr("NMH_MTA", enum("smtp sendmail"))
- reg.usr("NMH_PAGER", BtIdentifier)
+ reg.usr("NMH_PAGER", BtIdentifierIndirect)
reg.usr("NS_PREFERRED", enum("communicator navigator mozilla"))
reg.usr("NULLMAILER_USER", BtUserGroupName)
reg.usr("NULLMAILER_GROUP", BtUserGroupName)
@@ -798,7 +798,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.usr("QMAIL_QFILTER_TMPDIR", BtPathname)
reg.usr("QMAIL_QUEUE_DIR", BtPathname)
reg.usr("QMAIL_QUEUE_EXTRA", BtMailAddress)
- reg.usr("QPOPPER_FAC", BtIdentifier)
+ reg.usr("QPOPPER_FAC", BtIdentifierIndirect)
reg.usr("QPOPPER_USER", BtUserGroupName)
reg.usr("QPOPPER_SPOOL_DIR", BtPathname)
reg.usr("RASMOL_DEPTH", enum("8 16 32"))
@@ -821,7 +821,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.usr("SSYNC_PAWD", enum("pawd pwd"))
reg.usr("SUSE_PREFER", enum("13.1 12.1 10.0")) // TODO: extract
reg.usr("TEXMFSITE", BtPathname)
- reg.usr("THTTPD_LOG_FACILITY", BtIdentifier)
+ reg.usr("THTTPD_LOG_FACILITY", BtIdentifierIndirect)
reg.usr("UCSPI_SSL_USER", BtUserGroupName)
reg.usr("UCSPI_SSL_GROUP", BtUserGroupName)
reg.usr("UNPRIVILEGED", BtYesNo)
@@ -834,10 +834,10 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.usr("WCALC_HTMLPATH", BtPathname) // URL path
reg.usr("WCALC_CGIDIR", BtPrefixPathname)
reg.usr("WCALC_CGIPATH", BtPathname) // URL path
- reg.usrlist("WDM_MANAGERS", BtIdentifier)
+ reg.usrlist("WDM_MANAGERS", BtIdentifierIndirect)
reg.usr("X10_PORT", BtPathname)
reg.usrpkg("XAW_TYPE", enum("standard 3d xpm neXtaw"))
- reg.usr("XLOCK_DEFAULT_MODE", BtIdentifier)
+ reg.usr("XLOCK_DEFAULT_MODE", BtIdentifierIndirect)
reg.usr("ZSH_STATIC", BtYes)
// some other variables, sorted alphabetically
@@ -905,7 +905,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
"buildlink3.mk: set")
reg.syslist("BUILDLINK_CPPFLAGS", BtCFlag)
reg.bl3list("BUILDLINK_CPPFLAGS.*", BtCFlag)
- reg.acllist("BUILDLINK_DEPENDS", BtIdentifier,
+ reg.acllist("BUILDLINK_DEPENDS", BtIdentifierDirect,
PackageSettable,
"buildlink3.mk: append")
reg.acllist("BUILDLINK_DEPMETHOD.*", BtBuildlinkDepmethod,
@@ -957,7 +957,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.acllist("BUILDLINK_RPATHDIRS.*", BtPathname,
PackageSettable,
"buildlink3.mk: append")
- reg.acllist("BUILDLINK_TARGETS", BtIdentifier,
+ reg.acllist("BUILDLINK_TARGETS", BtIdentifierDirect,
PackageSettable,
"Makefile, Makefile.*, *.mk: append")
reg.acl("BUILDLINK_FNAME_TRANSFORM.*", BtSedCommands,
@@ -969,7 +969,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.acllist("BUILDLINK_TRANSFORM.*", BtWrapperTransform,
PackageSettable,
"*: append")
- reg.acllist("BUILDLINK_TREE", BtIdentifier,
+ reg.acllist("BUILDLINK_TREE", BtIdentifierDirect,
PackageSettable,
"buildlink3.mk: append")
reg.acl("BUILDLINK_X11_DIR", BtPathname,
@@ -984,10 +984,10 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkglist("BUILD_ENV", BtShellWord)
reg.sys("BUILD_MAKE_CMD", BtShellCommand)
reg.pkglist("BUILD_MAKE_FLAGS", BtShellWord)
- reg.pkglist("BUILD_TARGET", BtIdentifier)
- reg.pkglist("BUILD_TARGET.*", BtIdentifier)
+ reg.pkglist("BUILD_TARGET", BtIdentifierIndirect)
+ reg.pkglist("BUILD_TARGET.*", BtIdentifierIndirect)
reg.pkg("BUILD_USES_MSGFMT", BtYes)
- reg.acl("BUILTIN_PKG", BtIdentifier,
+ reg.acl("BUILTIN_PKG", BtIdentifierDirect,
PackageSettable,
"builtin.mk: set, use, use-loadtime",
"Makefile, Makefile.*, *.mk: use, use-loadtime")
@@ -1084,7 +1084,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.sys("DELAYED_ERROR_MSG", BtShellCommand)
reg.sys("DELAYED_WARNING_MSG", BtShellCommand)
reg.pkglistbl3("DEPENDS", BtDependencyWithPath)
- reg.usrlist("DEPENDS_TARGET", BtIdentifier)
+ reg.usrlist("DEPENDS_TARGET", BtIdentifierDirect)
reg.pkglist("DESCR_SRC", BtPathname)
reg.sys("DESTDIR", BtPathname)
reg.pkg("DESTDIR_VARNAME", BtVariableName)
@@ -1095,12 +1095,12 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkg("DISTNAME", BtFilename)
reg.pkg("DIST_SUBDIR", BtPathname)
reg.pkglist("DJB_BUILD_ARGS", BtShellWord)
- reg.pkglist("DJB_BUILD_TARGETS", BtIdentifier)
+ reg.pkglist("DJB_BUILD_TARGETS", BtIdentifierIndirect)
reg.pkgappend("DJB_CONFIG_CMDS", BtShellCommands)
reg.pkglist("DJB_CONFIG_DIRS", BtWrksrcSubdirectory)
reg.pkg("DJB_CONFIG_HOME", BtFilename)
reg.pkg("DJB_CONFIG_PREFIX", BtPathname)
- reg.pkglist("DJB_INSTALL_TARGETS", BtIdentifier)
+ reg.pkglist("DJB_INSTALL_TARGETS", BtIdentifierIndirect)
reg.pkg("DJB_MAKE_TARGETS", BtYesNo)
reg.pkg("DJB_RESTRICTED", BtYesNo)
reg.pkg("DJB_SLASHPACKAGE", BtYesNo)
@@ -1126,8 +1126,8 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.sys("EMACS_FLAVOR", enum("emacs xemacs"))
reg.sys("EMACS_INFOPREFIX", BtPathname)
reg.sys("EMACS_LISPPREFIX", BtPathname)
- reg.pkglistbl3("EMACS_MODULES", BtIdentifier)
- reg.sys("EMACS_PKGNAME_PREFIX", BtIdentifier) // Or the empty string.
+ reg.pkglistbl3("EMACS_MODULES", BtIdentifierIndirect)
+ reg.sys("EMACS_PKGNAME_PREFIX", BtIdentifierIndirect) // Or the empty string.
reg.pkglist("EMACS_VERSIONS_ACCEPTED", emacsVersions)
reg.sys("EMACS_VERSION_MAJOR", BtInteger)
reg.sys("EMACS_VERSION_MINOR", BtInteger)
@@ -1137,9 +1137,9 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.sys("OPSYS_EMULDIR", BtPathname)
reg.sys("EMULSUBDIRSLASH", BtPathname)
reg.sys("EMUL_ARCH", enum("arm i386 m68k none ns32k sparc vax x86_64"))
- reg.sys("EMUL_DISTRO", BtIdentifier)
+ reg.sys("EMUL_DISTRO", BtIdentifierIndirect)
reg.sys("EMUL_IS_NATIVE", BtYes)
- reg.pkglist("EMUL_MODULES.*", BtIdentifier)
+ reg.pkglist("EMUL_MODULES.*", BtIdentifierIndirect)
reg.sys("EMUL_OPSYS", enum("darwin freebsd hpux irix linux osf1 solaris sunos none"))
reg.pkg("EMUL_PKG_FMT", enum("plain rpm"))
reg.usr("EMUL_PLATFORM", BtEmulPlatform)
@@ -1181,13 +1181,13 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.syslist("GAMEDIR_PERMS", BtPerms)
reg.pkglistbl3rat("GCC_REQD", BtGccReqd)
reg.pkgappend("GENERATE_PLIST", BtShellCommands)
- reg.pkg("GITHUB_PROJECT", BtIdentifier)
- reg.pkg("GITHUB_TAG", BtIdentifier)
+ reg.pkg("GITHUB_PROJECT", BtIdentifierIndirect)
+ reg.pkg("GITHUB_TAG", BtIdentifierIndirect)
reg.pkg("GITHUB_RELEASE", BtFilename)
reg.pkg("GITHUB_TYPE", enum("tag release"))
reg.pkgrat("GMAKE_REQD", BtVersion)
// Some packages need to set GNU_ARCH.i386 to either i486 or i586.
- reg.pkg("GNU_ARCH.*", BtIdentifier)
+ reg.pkg("GNU_ARCH.*", BtIdentifierDirect)
// GNU_CONFIGURE needs to be tested in some buildlink3.mk files,
// such as lang/vala.
reg.acl("GNU_CONFIGURE", BtYes,
@@ -1234,7 +1234,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.syslist("INSTALL_SCRIPTS_ENV", BtShellWord)
reg.sys("INSTALL_SCRIPT_DIR", BtShellCommand)
reg.pkglist("INSTALL_SRC", BtPathname)
- reg.pkglist("INSTALL_TARGET", BtIdentifier)
+ reg.pkglist("INSTALL_TARGET", BtIdentifierIndirect)
reg.pkglist("INSTALL_TEMPLATES", BtPathname)
reg.pkgload("INSTALL_UNSTRIPPED", BtYesNo)
reg.pkglist("INTERACTIVE_STAGE", enum("fetch extract configure build test install"))
@@ -1256,11 +1256,11 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.sys("KRB5BASE", BtPathname)
reg.pkglist("KRB5_ACCEPTED", enum("heimdal mit-krb5"))
reg.usr("KRB5_DEFAULT", enum("heimdal mit-krb5"))
- reg.sys("KRB5_TYPE", BtIdentifier)
+ reg.sys("KRB5_TYPE", BtIdentifierIndirect)
reg.sys("LD", BtShellCommand)
- reg.pkglistbl3("LDFLAGS", BtLdFlag) // May also be changed by the user.
- reg.pkglistbl3("LDFLAGS.*", BtLdFlag) // May also be changed by the user.
- reg.sysload("LIBABISUFFIX", BtIdentifier) // Can also be empty.
+ reg.pkglistbl3("LDFLAGS", BtLdFlag) // May also be changed by the user.
+ reg.pkglistbl3("LDFLAGS.*", BtLdFlag) // May also be changed by the user.
+ reg.sysload("LIBABISUFFIX", BtIdentifierIndirect) // Can also be empty.
reg.sys("LIBGRP", BtUserGroupName)
reg.sys("LIBMODE", BtFileMode)
reg.sys("LIBOWN", BtUserGroupName)
@@ -1276,8 +1276,8 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.sys("LINK.*", BtShellCommand)
reg.sys("LINKER_RPATH_FLAG", BtShellWord)
reg.syslist("LITTLEENDIANPLATFORMS", BtMachinePlatformPattern)
- reg.sysload("LOWER_OPSYS", BtIdentifier, NonemptyIfDefined)
- reg.sysload("LOWER_VENDOR", BtIdentifier, NonemptyIfDefined)
+ reg.sysload("LOWER_OPSYS", BtIdentifierDirect, NonemptyIfDefined)
+ reg.sysload("LOWER_VENDOR", BtIdentifierDirect, NonemptyIfDefined)
reg.sysloadlist("LP64PLATFORMS", BtMachinePlatformPattern, DefinedIfInScope|NonemptyIfDefined)
reg.pkglist("LTCONFIG_OVERRIDE", BtPathPattern)
@@ -1335,7 +1335,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkglist("MESSAGE_SRC", BtPathname)
reg.pkglist("MESSAGE_SUBST", BtShellWord)
reg.pkg("META_PACKAGE", BtYes)
- reg.syslist("MISSING_FEATURES", BtIdentifier)
+ reg.syslist("MISSING_FEATURES", BtIdentifierDirect)
reg.pkglist("MYSQL_VERSIONS_ACCEPTED", mysqlVersions)
reg.usr("MYSQL_VERSION_DEFAULT", BtVersion)
reg.sys("NATIVE_CC", BtShellCommand) // See mk/platform/tools.NetBSD.mk (and some others).
@@ -1365,7 +1365,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.sysload("OPSYS", platforms, DefinedIfInScope|NonemptyIfDefined)
reg.pkglistbl3("OPSYSVARS", BtVariableName)
reg.pkg("OSVERSION_SPECIFIC", BtYes)
- reg.sysload("OS_VARIANT", BtIdentifier, DefinedIfInScope)
+ reg.sysload("OS_VARIANT", BtIdentifierDirect, DefinedIfInScope)
reg.sysload("OS_VERSION", BtVersion)
reg.sysload("OSX_VERSION", BtVersion) // See mk/platform/Darwin.mk.
reg.pkg("OVERRIDE_DIRDEPTH*", BtInteger)
@@ -1425,7 +1425,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
SystemProvided,
"special:phpversion.mk: set",
"*: use, use-loadtime")
- reg.sys("PKGBASE", BtIdentifier)
+ reg.sys("PKGBASE", BtIdentifierDirect)
// Despite its name, this is actually a list of filenames.
reg.acllist("PKGCONFIG_FILE.*", BtPathname,
PackageSettable,
@@ -1473,7 +1473,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.sys("PKG_DELETE", BtShellCommand)
reg.pkglist("PKG_DESTDIR_SUPPORT", enum("destdir user-destdir"))
reg.pkglistone("PKG_FAIL_REASON", BtShellWord)
- reg.sysload("PKG_FORMAT", BtIdentifier)
+ reg.sysload("PKG_FORMAT", BtIdentifierDirect)
reg.pkg("PKG_GECOS.*", BtMessage)
reg.pkg("PKG_GID.*", BtInteger)
reg.pkglist("PKG_GROUPS", BtShellWord)
@@ -1485,18 +1485,21 @@ func (reg *VarTypeRegistry) Init(src *Pk
//
// TODO: Is it possible that a package includes the hacks.mk file from
// one of its dependencies?
- reg.acllist("PKG_HACKS", BtIdentifier,
+ reg.acllist("PKG_HACKS", BtIdentifierDirect,
PackageSettable,
"*: none")
reg.sysload("PKG_INFO", BtShellCommand, DefinedIfInScope|NonemptyIfDefined)
reg.sys("PKG_JAVA_HOME", BtPathname)
- // FIXME: Add definition for PKG_DEFAULT_JVM.
- reg.sys("PKG_JVM", jvms) // deprecated
+ reg.sysload("PKG_JVM", jvms)
reg.pkglistrat("PKG_JVMS_ACCEPTED", jvms)
reg.sys("PKG_LIBTOOL", BtPathname)
// begin PKG_OPTIONS section
//
+ // Most identifiers for the groups are given as literal strings.
+ // In rare cases (audio/speex), ${MACHINE_ARCH} is used for selecting a group,
+ // but not for defining it.
+ //
// TODO: force the pkgsrc packages to only define options in the
// options.mk file. Most packages already do this, but some still
// define them in the Makefile or Makefile.common.
@@ -1507,9 +1510,9 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkgloadlist("PKG_OPTIONS_GROUP.*", BtOption)
reg.pkgloadlist("PKG_OPTIONS_LEGACY_OPTS", BtUnknown)
reg.pkgloadlist("PKG_OPTIONS_LEGACY_VARS", BtUnknown)
- reg.pkgloadlist("PKG_OPTIONS_NONEMPTY_SETS", BtIdentifier)
- reg.pkgloadlist("PKG_OPTIONS_OPTIONAL_GROUPS", BtIdentifier)
- reg.pkgloadlist("PKG_OPTIONS_REQUIRED_GROUPS", BtIdentifier)
+ reg.pkgloadlist("PKG_OPTIONS_NONEMPTY_SETS", BtIdentifierIndirect)
+ reg.pkgloadlist("PKG_OPTIONS_OPTIONAL_GROUPS", BtIdentifierIndirect)
+ reg.pkgloadlist("PKG_OPTIONS_REQUIRED_GROUPS", BtIdentifierIndirect)
reg.pkgloadlist("PKG_OPTIONS_SET.*", BtOption)
reg.pkgload("PKG_OPTIONS_VAR", BtPkgOptionsVar)
reg.pkgloadlist("PKG_SUGGESTED_OPTIONS", BtOption)
@@ -1533,13 +1536,13 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkglist("PKG_SYSCONFDIR_PERMS", BtPerms)
reg.sys("PKG_SYSCONFBASEDIR", BtPathname)
reg.pkg("PKG_SYSCONFSUBDIR", BtPathname)
- reg.pkg("PKG_SYSCONFVAR", BtIdentifier)
+ reg.pkg("PKG_SYSCONFVAR", BtIdentifierDirect)
reg.pkg("PKG_UID", BtInteger)
reg.pkglist("PKG_USERS", BtShellWord)
reg.pkglist("PKG_USERS_VARS", BtVariableName)
reg.pkg("PKG_USE_KERBEROS", BtYes)
reg.pkgload("PLIST.*", BtYes)
- reg.pkgloadlist("PLIST_VARS", BtIdentifier)
+ reg.pkgloadlist("PLIST_VARS", BtIdentifierIndirect)
reg.pkglist("PLIST_SRC", BtRelativePkgPath)
reg.pkglist("PLIST_SUBST", BtShellWord)
reg.pkg("PLIST_TYPE", enum("dynamic static"))
@@ -1562,7 +1565,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.syslist("PTHREAD_LDFLAGS", BtLdFlag)
reg.syslist("PTHREAD_LIBS", BtLdFlag)
reg.pkglistbl3("PTHREAD_OPTS", enum("native optional require"))
- reg.sysload("PTHREAD_TYPE", BtIdentifier) // Or "native" or "none".
+ reg.sysload("PTHREAD_TYPE", BtIdentifierDirect) // Or "native" or "none".
reg.pkg("PY_PATCHPLIST", BtYes)
reg.acl("PYPKGPREFIX",
reg.enumFromDirs(src, "lang", `^python(\d+)$`, "py$1", "py27 py36"),
@@ -1599,7 +1602,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkglist("REPLACE_BASH", BtPathPattern)
reg.pkglist("REPLACE_CSH", BtPathPattern)
reg.pkglist("REPLACE_FILES.*", BtPathPattern)
- reg.pkglist("REPLACE_INTERPRETER", BtIdentifier)
+ reg.pkglist("REPLACE_INTERPRETER", BtIdentifierIndirect)
reg.pkglist("REPLACE_KSH", BtPathPattern)
reg.pkglist("REPLACE_LOCALEDIR_PATTERNS", BtFilePattern)
reg.pkglist("REPLACE_LUA", BtPathPattern)
@@ -1648,7 +1651,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkg("SMF_SRCDIR", BtPathname)
reg.pkg("SMF_NAME", BtFilename)
reg.pkg("SMF_MANIFEST", BtPathname)
- reg.pkglist("SMF_INSTANCES", BtIdentifier)
+ reg.pkglist("SMF_INSTANCES", BtIdentifierIndirect)
reg.pkglist("SMF_METHODS", BtFilename)
reg.pkg("SMF_METHOD_SRC.*", BtPathname)
reg.pkg("SMF_METHOD_SHELL", BtShellCommand)
@@ -1661,8 +1664,8 @@ func (reg *VarTypeRegistry) Init(src *Pk
PackageSettable,
"Makefile: append")
- reg.pkglistbl3("SUBST_CLASSES", BtIdentifier)
- reg.pkglistbl3("SUBST_CLASSES.*", BtIdentifier) // OPSYS-specific
+ reg.pkglistbl3("SUBST_CLASSES", BtIdentifierDirect)
+ reg.pkglistbl3("SUBST_CLASSES.*", BtIdentifierDirect) // OPSYS-specific
reg.pkglistbl3("SUBST_FILES.*", BtPathPattern)
reg.pkgbl3("SUBST_FILTER_CMD.*", BtShellCommand)
reg.pkgbl3("SUBST_MESSAGE.*", BtMessage)
@@ -1674,7 +1677,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkglist("TEST_DEPENDS", BtDependencyWithPath)
reg.pkglist("TEST_DIRS", BtWrksrcSubdirectory)
reg.pkglist("TEST_ENV", BtShellWord)
- reg.pkglist("TEST_TARGET", BtIdentifier)
+ reg.pkglist("TEST_TARGET", BtIdentifierIndirect)
reg.pkglistrat("TEXINFO_REQD", BtVersion)
reg.pkglistbl3("TOOL_DEPENDS", BtDependencyWithPath)
reg.syslist("TOOLS_ALIASES", BtFilename)
@@ -1694,7 +1697,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.usr("UNPRIVILEGED_USER", BtUserGroupName)
reg.usr("UNPRIVILEGED_GROUP", BtUserGroupName)
reg.pkglist("UNWRAP_FILES", BtPathPattern)
- reg.cmdline("UPDATE_TARGET", BtIdentifier, List)
+ reg.cmdline("UPDATE_TARGET", BtIdentifierDirect, List)
reg.pkg("USERGROUP_PHASE", enum("configure build pre-install"))
reg.usrlist("USER_ADDITIONAL_PKGS", BtPkgpath)
reg.pkg("USE_BSD_MAKEFILE", BtYes)
@@ -1709,7 +1712,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkg("USE_CMAKE", BtYes)
reg.usr("USE_DESTDIR", BtYes)
- reg.pkglist("USE_FEATURES", BtIdentifier)
+ reg.pkglist("USE_FEATURES", BtIdentifierDirect)
reg.pkg("USE_GAMESGROUP", BtYesNo)
reg.pkg("USE_GCC_RUNTIME", BtYesNo)
reg.pkg("USE_GNU_CONFIGURE_HOST", BtYesNo)
@@ -1768,7 +1771,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkglist("XMKMF_FLAGS", BtShellWord)
reg.pkglist("_WRAP_EXTRA_ARGS.*", BtShellWord)
- reg.infralist("_VARGROUPS", BtIdentifier)
+ reg.infralist("_VARGROUPS", BtIdentifierDirect)
reg.infralist("_USER_VARS.*", BtVariableName)
reg.infralist("_PKG_VARS.*", BtVariableName)
reg.infralist("_SYS_VARS.*", BtVariableName)
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.72 pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.73
--- pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.72 Mon Dec 9 20:38:16 2019
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck.go Fri Dec 13 01:39:23 2019
@@ -676,7 +676,7 @@ func (cv *VartypeCheck) Homepage() {
// Identifier checks for valid identifiers in various contexts, limiting the
// valid characters to A-Za-z0-9_.
-func (cv *VartypeCheck) Identifier() {
+func (cv *VartypeCheck) IdentifierDirect() {
if cv.Op == opUseMatch {
if cv.Value == cv.ValueNoVar && !matches(cv.Value, `^[\w*\-?\[\]]+$`) {
cv.Warnf("Invalid identifier pattern %q for %s.", cv.Value, cv.Varname)
@@ -685,8 +685,8 @@ func (cv *VartypeCheck) Identifier() {
}
if cv.Value != cv.ValueNoVar {
- // TODO: Activate this warning again, or document why that is not useful.
- // line.logWarning("Identifiers should be given directly.")
+ cv.Errorf("Identifiers for %s must not refer to other variables.", cv.Varname)
+ return
}
switch {
@@ -701,6 +701,14 @@ func (cv *VartypeCheck) Identifier() {
}
}
+// Identifier checks for valid identifiers in various contexts, limiting the
+// valid characters to A-Za-z0-9_.
+func (cv *VartypeCheck) IdentifierIndirect() {
+ if cv.Value == cv.ValueNoVar {
+ cv.IdentifierDirect()
+ }
+}
+
func (cv *VartypeCheck) Integer() {
if !matches(cv.Value, `^\d+$`) {
cv.Warnf("Invalid integer %q.", cv.Value)
Index: pkgsrc/pkgtools/pkglint/files/intqa/qa_test.go
diff -u pkgsrc/pkgtools/pkglint/files/intqa/qa_test.go:1.3 pkgsrc/pkgtools/pkglint/files/intqa/qa_test.go:1.4
--- pkgsrc/pkgtools/pkglint/files/intqa/qa_test.go:1.3 Sun Dec 8 00:06:38 2019
+++ pkgsrc/pkgtools/pkglint/files/intqa/qa_test.go Fri Dec 13 01:39:23 2019
@@ -722,6 +722,9 @@ func (s *Suite) Test_sortedKeys(c *check
func (s *Suite) Test_Value_Method(c *check.C) {
_ = s.Init(c)
+ // To make it appear actually used to golangci-lint.
+ _ = Value{}
+
// Just for code coverage of checkTestFile, to have a piece of code
// that lives in the same file as its test.
}
Index: pkgsrc/pkgtools/pkglint/files/textproc/lexer.go
diff -u pkgsrc/pkgtools/pkglint/files/textproc/lexer.go:1.8 pkgsrc/pkgtools/pkglint/files/textproc/lexer.go:1.9
--- pkgsrc/pkgtools/pkglint/files/textproc/lexer.go:1.8 Sat Nov 30 20:35:11 2019
+++ pkgsrc/pkgtools/pkglint/files/textproc/lexer.go Fri Dec 13 01:39:23 2019
@@ -140,6 +140,8 @@ func (l *Lexer) NextByte() byte {
// NextBytesFunc chops off the longest prefix (possibly empty) consisting
// solely of bytes for which fn returns true.
+//
+// TODO: SkipBytesFunc
func (l *Lexer) NextBytesFunc(fn func(b byte) bool) string {
i := 0
rest := l.rest
Added files:
Index: pkgsrc/pkgtools/pkglint/files/mkalign.go
diff -u /dev/null pkgsrc/pkgtools/pkglint/files/mkalign.go:1.1
--- /dev/null Fri Dec 13 01:39:24 2019
+++ pkgsrc/pkgtools/pkglint/files/mkalign.go Fri Dec 13 01:39:23 2019
@@ -0,0 +1,233 @@
+package pkglint
+
+type MkAlignFile struct {
+ Paras []*MkAlignPara
+}
+
+func (f *MkAlignFile) AlignParas() {
+ // #. Ein einzelner Absatz, der einen Tab weniger eingerückt ist als die übrigen,
+ // darf auf die Einrückung der anderen Absätze angeglichen werden,
+ // sofern der Absatz dadurch nicht zu breit wird.
+ panic("implement me")
+}
+
+type MkAlignPara struct {
+}
+
+func (p *MkAlignPara) IsAligned() bool {
+ // #. Das Ausrichten mit mehr als 1 Tab ist erlaubt, wenn die Ausrichtung einheitlich ist.
+
+ // #. Wenn VarOp über die Ausrichtung hinausragt (Ausreißer),
+ // darf zwischen VarOp und Wert statt der Ausrichtung 1 Leerzeichen sein.
+
+ // #. Das Verhältnis zwischen Tab-Zeilen und hinausragenden Zeilen muss ausgewogen sein.
+ // Nicht zu viele hinausragende Zeilen. (Noch zu definieren.)
+ // Möglicher Ansatz: Anteil der Leerfläche?
+
+ panic("implement me")
+}
+
+func (p *MkAlignPara) IsOutlier(line *MkAlignLine) bool {
+ // #. Wenn VarOp über die Ausrichtung hinausragt (Ausreißer),
+ // darf zwischen VarOp und Wert statt der Ausrichtung 1 Leerzeichen sein.
+ panic("implement me")
+}
+
+// ValueAlignment returns the column at which all values of the paragraph
+// are aligned, or false if they aren't.
+//
+// #. Die Werte aller Zeilen sind mit Tabs an einer gemeinsamen vertikalen Linie
+// (Ausrichtung) ausgerichtet.
+func (p *MkAlignPara) ValueAlignment() (bool, int) {
+ panic("implement me")
+}
+
+func (p *MkAlignPara) MinValueAlignment() int {
+ // #. Die minimale Ausrichtung ergibt sich aus der maximalen Breite von # und VarOp
+ // aller Zeilen, gerundet zum nächsten Tabstopp.
+ // Dabei zählen auch Zeilen mit, die rechts von VarOp komplett leer sind.
+ panic("implement me")
+}
+
+func (p *MkAlignPara) MaxValueAlignment() int {
+ // #. Die maximale Ausrichtung ergibt sich aus der maximalen Breite von Wert
+ // und Kommentar, abgezogen vom maximalen rechten Rand (in Spalte 73).
+ panic("implement me")
+}
+
+func (p *MkAlignPara) MayAlignValuesTo(column int) bool {
+ panic("implement me")
+}
+
+func (p *MkAlignPara) AlignValuesTo(column int) {
+ // #. Beim Umformatieren darf die Zeilenbreite die 73 Zeichen nicht überschreiten,
+ // damit am rechten Rand eindeutig ist, wo jede Zeile aufhört.
+ // Zeilen, die bereits vorher breiter waren, dürfen ruhig noch breiter werden.
+
+ panic("implement me")
+}
+
+type MkAlignMkLine struct {
+}
+
+func (l *MkAlignMkLine) RightMargin() int {
+ // #. Jede MkZeile hat für alle ihre Zeilen einen gemeinsamen rechten Rand.
+
+ // #. Um den gemeinsamen rechten Rand zu bestimmen, werden alle Zeilen ignoriert,
+ // in denen die Fortsetzung durch 1 Leerzeichen abgetrennt ist.
+
+ // #. Einzelne Fortsetzungen dürfen über den rechten Rand hinausragen.
+ // Die Fortsetzung wird dann durch 1 Leerzeichen abgetrennt.
+
+ panic("implement me")
+}
+
+func (l *MkAlignMkLine) IsCanonical() bool {
+ // #. Eine leere Erstzeile mit 1 fortgesetzer Zeile ist nur zulässig,
+ // wenn die kombinierte Zeile breiter als 73 Zeichen wäre.
+ // Sonst werden die beiden Zeilen kombiniert.
+
+ // ### Mehrzeilig, fortgesetzte Zeilen
+ //
+ // #. Nach einer leeren Erstzeile ist die erste fortgesetzte Zeile an der
+ // Ausrichtung aller Zeilen eingerückt, wenn die Erstzeile über die
+ // Ausrichtung ragt und der Platz aller Zeilen es zulässt, andernfalls
+ // mit 1 Tab.
+ //
+ // #. Bei mehrzeiligen einrückbaren Werten (AWK, Shell, Listen aus Tupeln)
+ // dürfen die weiteren Fortsetzungszeilen weiter eingerückt sein als die erste.
+ // Ihre Einrückung besteht aus Tabs, gefolgt von 0 bis 7 Leerzeichen.
+
+ // In the continuation lines, each follow-up line is indented with at least
+ // one tab, to avoid confusing them with regular single-lines. This is
+ // especially true for CONFIGURE_ENV, since the environment variables are
+ // typically uppercase as well.
+
+ // MULTI_LINE= \
+ // The value starts in the second line.
+ //
+ // The backslash in the first line is usually aligned to the other variables
+ // in the same paragraph. If the variable name is longer than the indentation
+ // of the paragraph, it may be indented with a single space.
+ //
+ // In multi-line shell commands or AWK programs, the backslash is
+ // often indented to column 73, as are the backslashes from the follow-up
+ // lines, to act as a visual guideline.
+
+ // Continuation lines may or may not have their value in the first line.
+
+ // In general, all values should be aligned using tabs.
+ // As an exception, a single very long line (called an outlier) may be
+ // aligned with a single space.
+ // A typical example is a SITES.very-long-file-name.tar.gz variable
+ // between HOMEPAGE and DISTFILES.
+
+ panic("implement me")
+}
+
+func (l *MkAlignMkLine) HasCanonicalRightMargin() bool {
+ // #. Die Fortsetzungen jeder MkZeile sind entweder alle durch je 1 Leerzeichen abgetrennt,
+ // oder alle Fortsetzungen sind am rechten Rand.
+
+ // #. Einzelne Fortsetzungen dürfen über den rechten Rand hinausragen.
+ // Die Fortsetzung wird dann durch 1 Leerzeichen abgetrennt.
+
+ // #. Die Fortsetzung der Erstzeile mit Wert ist durch 1 Leerzeichen abgetrennt,
+ // wenn sie rechts von der Ausrichtung steht,
+ // andernfalls durch Tabs an der Ausrichtung.
+
+ panic("implement me")
+}
+
+func (l *MkAlignMkLine) CurrentValueAlign() int {
+ // The indentation of the first value of the variable determines the minimum
+ // indentation for the remaining continuation lines.
+ //
+ // To allow long variable
+ // values to be indented as little as possible, the follow-up lines only need
+ // to be indented by a single tab, even if the other
+ // variables in the paragraph are aligned further to the right.
+ //
+ // If the
+ // indentation is not a single tab, it must match the indentation of the
+ // other lines in the paragraph.
+ panic("implement me")
+}
+
+type MkAlignLine struct {
+ Comment string
+ VarOp string
+ SpaceBeforeValue string
+ Value string
+ SpaceAfterValue string
+ Continuation string
+}
+
+func (l *MkAlignLine) HasCanonicalRightMargin(valueAlignColumn int) bool {
+ panic("implement me")
+}
+
+func (l *MkAlignLine) IsCanonicalSingle() bool {
+ panic("implement me")
+}
+
+func (l *MkAlignLine) IsCanonicalLeadEmpty() bool {
+ // SHELL_CMD= \
+ // if ${PKG_ADMIN} pmatch ${PKGNAME} ${dependency}; then \
+ // ${ECHO} yes; \
+ // else \
+ // ${ECHO} no; \
+ // fi
+ //
+ //
+ panic("implement me")
+}
+
+func (l *MkAlignLine) IsCanonicalLeadValue() bool {
+ // MULTI_LINE= The value starts in the first line \
+ // and continues in the second line.
+ //
+ // In lists or plain text, like in the example above, all values are
+ // aligned in the same column. Some variables also contain code, and in
+ // these variables, the line containing the first word defines how deep
+ // the follow-up lines must be indented at least.
+
+ panic("implement me")
+}
+
+func (l *MkAlignLine) IsCanonicalFollowLead() bool {
+ // SHELL_CMD= \
+ // --> if ${PKG_ADMIN} pmatch ${PKGNAME} ${dependency}; then \
+ // ${ECHO} yes; \
+ // else \
+ // ${ECHO} no; \
+ // fi
+ //
+ // MULTI_LINE= The value starts in the first line \
+ // and continues in the second line.
+ //
+ // In lists or plain text, like in the example above, all values are
+ // aligned in the same column. Some variables also contain code, and in
+ // these variables, the line containing the first word defines how deep
+ // the follow-up lines must be indented at least.
+
+ panic("implement me")
+}
+
+func (l *MkAlignLine) IsCanonicalFollow() bool {
+ // In the continuation lines, each follow-up line is indented with at least
+ // one tab, to avoid confusing them with regular single-lines. This is
+ // especially true for CONFIGURE_ENV, since the environment variables are
+ // typically uppercase as well.
+
+ // SHELL_CMD= \
+ // if ${PKG_ADMIN} pmatch ${PKGNAME} ${dependency}; then \
+ // ${ECHO} yes; \
+ // else \
+ // ${ECHO} no; \
+ // fi
+ //
+ //
+
+ panic("implement me")
+}
Index: pkgsrc/pkgtools/pkglint/files/mkalign_test.go
diff -u /dev/null pkgsrc/pkgtools/pkglint/files/mkalign_test.go:1.1
--- /dev/null Fri Dec 13 01:39:24 2019
+++ pkgsrc/pkgtools/pkglint/files/mkalign_test.go Fri Dec 13 01:39:23 2019
@@ -0,0 +1,236 @@
+package pkglint
+
+import "gopkg.in/check.v1"
+
+func (s *Suite) Test_MkAlignFile_AlignParas(c *check.C) {
+ t := s.Init(c)
+
+ test := func() {
+ // TODO
+ t.CheckEquals(true, true)
+ }
+
+ test()
+}
+
+func (s *Suite) Test_MkAlignPara_IsAligned(c *check.C) {
+ t := s.Init(c)
+
+ test := func() {
+ // TODO
+ t.CheckEquals(true, true)
+ }
+
+ test()
+}
+
+func (s *Suite) Test_MkAlignPara_IsOutlier(c *check.C) {
+ t := s.Init(c)
+
+ test := func() {
+ // TODO
+ t.CheckEquals(true, true)
+ }
+
+ test()
+}
+
+func (s *Suite) Test_MkAlignPara_ValueAlignment(c *check.C) {
+ t := s.Init(c)
+
+ test := func() {
+ // TODO
+ t.CheckEquals(true, true)
+ }
+
+ test()
+}
+
+func (s *Suite) Test_MkAlignPara_MinValueAlignment(c *check.C) {
+ t := s.Init(c)
+
+ test := func() {
+ // TODO
+ t.CheckEquals(true, true)
+ }
+
+ test()
+}
+
+func (s *Suite) Test_MkAlignPara_MaxValueAlignment(c *check.C) {
+ t := s.Init(c)
+
+ test := func() {
+ // TODO
+ t.CheckEquals(true, true)
+ }
+
+ test()
+}
+
+func (s *Suite) Test_MkAlignPara_MayAlignValuesTo(c *check.C) {
+ t := s.Init(c)
+
+ test := func() {
+ // TODO
+ t.CheckEquals(true, true)
+ }
+
+ test()
+}
+
+func (s *Suite) Test_MkAlignPara_AlignValuesTo(c *check.C) {
+ t := s.Init(c)
+
+ test := func() {
+ // TODO
+ t.CheckEquals(true, true)
+ }
+
+ test()
+}
+
+func (s *Suite) Test_MkAlignMkLine_RightMargin(c *check.C) {
+ t := s.Init(c)
+
+ test := func() {
+ // TODO
+ t.CheckEquals(true, true)
+ }
+
+ test()
+}
+
+func (s *Suite) Test_MkAlignMkLine_IsCanonical(c *check.C) {
+ t := s.Init(c)
+
+ test := func() {
+ // TODO
+ t.CheckEquals(true, true)
+ }
+
+ test()
+}
+
+func (s *Suite) Test_MkAlignMkLine_HasCanonicalRightMargin(c *check.C) {
+ t := s.Init(c)
+
+ test := func() {
+ // TODO
+ t.CheckEquals(true, true)
+ }
+
+ test()
+}
+
+func (s *Suite) Test_MkAlignMkLine_CurrentValueAlign(c *check.C) {
+ t := s.Init(c)
+
+ test := func() {
+ // TODO
+ t.CheckEquals(true, true)
+ }
+
+ test()
+}
+
+func (s *Suite) Test_MkAlignLine_HasCanonicalRightMargin(c *check.C) {
+ t := s.Init(c)
+
+ test := func() {
+ // TODO
+ t.CheckEquals(true, true)
+ }
+
+ test()
+}
+
+func (s *Suite) Test_MkAlignLine_IsCanonicalSingle(c *check.C) {
+ t := s.Init(c)
+
+ test := func() {
+ // TODO
+ t.CheckEquals(true, true)
+ }
+
+ test()
+}
+
+func (s *Suite) Test_MkAlignLine_IsCanonicalLeadEmpty(c *check.C) {
+ t := s.Init(c)
+
+ test := func() {
+ // TODO
+ t.CheckEquals(true, true)
+ }
+
+ test()
+}
+
+func (s *Suite) Test_MkAlignLine_IsCanonicalLeadValue(c *check.C) {
+ t := s.Init(c)
+
+ test := func() {
+ // TODO
+ t.CheckEquals(true, true)
+ }
+
+ test()
+}
+
+func (s *Suite) Test_MkAlignLine_IsCanonicalFollowLead(c *check.C) {
+ t := s.Init(c)
+
+ test := func() {
+ // TODO
+ t.CheckEquals(true, true)
+ }
+
+ test()
+}
+
+func (s *Suite) Test_MkAlignLine_IsCanonicalFollow(c *check.C) {
+ t := s.Init(c)
+
+ test := func(line string, minAlign int, isCanonical bool) {
+ t.CheckDotColumns(line)
+ // TODO
+ t.CheckEquals(true, true)
+ }
+
+ test("\tvalue", 0, true)
+ test("\tvalue", 8, true)
+ test("\tvalue", 9, false)
+
+ // TODO: Why should pkglint care about the right margin?
+ // If there is an existing right margin, it should be kept as-is,
+ // but otherwise, why not let the pkgsrc developers fix this
+ // themselves?
+
+ // This line is 63 characters wide.
+ // It should be indented with one more tab.
+ //
+ // After indenting it, it is 71 characters wide,
+ // which is exactly the maximum right border
+ // for lines without a continuation backslash.
+ test("\tv\t\t\t\t\t\t.....63", 16, false)
+
+ // This line is 64 characters wide.
+ // It should be indented with one more tab.
+ //
+ // After indenting it, it is 72 characters wide,
+ // which is beyond the maximum right border
+ // for lines without a continuation backslash.
+ // Therefore it counts as canonical.
+ test("\tv\t\t\t\t\t\t......64", 16, true)
+
+ // This line already already overflows the right margin.
+ // On an 80-column display it is not decidable whether this line
+ // continues to the right or whether it stops there.
+ // Therefore it doesn't hurt to make the line even longer.
+ //
+ // Splitting this line into several shorter lines would require
+ // too much knowledge, therefore this task is left to the pkgsrc
+ // developers.
+ test("\tv\t\t\t\t\t\t\t\t.....79", 16, false)
+}
Home |
Main Index |
Thread Index |
Old Index