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: Sun Dec 2 23:12:43 UTC 2018
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile
pkgsrc/pkgtools/pkglint/files: autofix.go buildlink3_test.go
category_test.go licenses.go line.go lines.go lines_test.go
mkline.go mkline_test.go mklinechecker.go mklinechecker_test.go
mklines.go mklines_test.go mklines_varalign_test.go package.go
package_test.go pkglint.go pkgsrc.go substcontext.go tools_test.go
vardefs.go vartype.go vartypecheck_test.go
Log Message:
pkgtools/pkglint: update to 5.6.8
Changes since 5.6.7:
In pkgsrc-wip, if the first line of a file contains an expanded CVS Id,
it is not an error but only a note that it should be an unexpanded CVS
Id. The autofix for this no longer inserts a new line but replaces the
existing line.
Several refactorings and small improvements to the existing diagnostics.
To generate a diff of this commit:
cvs rdiff -u -r1.559 -r1.560 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.13 -r1.14 pkgsrc/pkgtools/pkglint/files/autofix.go \
pkgsrc/pkgtools/pkglint/files/pkgsrc.go
cvs rdiff -u -r1.21 -r1.22 pkgsrc/pkgtools/pkglint/files/buildlink3_test.go \
pkgsrc/pkgtools/pkglint/files/vartype.go
cvs rdiff -u -r1.14 -r1.15 pkgsrc/pkgtools/pkglint/files/category_test.go
cvs rdiff -u -r1.17 -r1.18 pkgsrc/pkgtools/pkglint/files/licenses.go
cvs rdiff -u -r1.28 -r1.29 pkgsrc/pkgtools/pkglint/files/line.go
cvs rdiff -u -r1.2 -r1.3 pkgsrc/pkgtools/pkglint/files/lines.go
cvs rdiff -u -r1.1 -r1.2 pkgsrc/pkgtools/pkglint/files/lines_test.go
cvs rdiff -u -r1.41 -r1.42 pkgsrc/pkgtools/pkglint/files/mkline.go \
pkgsrc/pkgtools/pkglint/files/pkglint.go
cvs rdiff -u -r1.45 -r1.46 pkgsrc/pkgtools/pkglint/files/mkline_test.go
cvs rdiff -u -r1.23 -r1.24 pkgsrc/pkgtools/pkglint/files/mklinechecker.go
cvs rdiff -u -r1.19 -r1.20 \
pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
cvs rdiff -u -r1.35 -r1.36 pkgsrc/pkgtools/pkglint/files/mklines.go
cvs rdiff -u -r1.31 -r1.32 pkgsrc/pkgtools/pkglint/files/mklines_test.go
cvs rdiff -u -r1.5 -r1.6 \
pkgsrc/pkgtools/pkglint/files/mklines_varalign_test.go
cvs rdiff -u -r1.39 -r1.40 pkgsrc/pkgtools/pkglint/files/package.go
cvs rdiff -u -r1.33 -r1.34 pkgsrc/pkgtools/pkglint/files/package_test.go
cvs rdiff -u -r1.16 -r1.17 pkgsrc/pkgtools/pkglint/files/substcontext.go
cvs rdiff -u -r1.8 -r1.9 pkgsrc/pkgtools/pkglint/files/tools_test.go
cvs rdiff -u -r1.50 -r1.51 pkgsrc/pkgtools/pkglint/files/vardefs.go
cvs rdiff -u -r1.36 -r1.37 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: pkgsrc/pkgtools/pkglint/Makefile
diff -u pkgsrc/pkgtools/pkglint/Makefile:1.559 pkgsrc/pkgtools/pkglint/Makefile:1.560
--- pkgsrc/pkgtools/pkglint/Makefile:1.559 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/Makefile Sun Dec 2 23:12:43 2018
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.559 2018/12/02 01:57:48 rillig Exp $
+# $NetBSD: Makefile,v 1.560 2018/12/02 23:12:43 rillig Exp $
-PKGNAME= pkglint-5.6.7
+PKGNAME= pkglint-5.6.8
CATEGORIES= pkgtools
DISTNAME= tools
MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/}
Index: pkgsrc/pkgtools/pkglint/files/autofix.go
diff -u pkgsrc/pkgtools/pkglint/files/autofix.go:1.13 pkgsrc/pkgtools/pkglint/files/autofix.go:1.14
--- pkgsrc/pkgtools/pkglint/files/autofix.go:1.13 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/autofix.go Sun Dec 2 23:12:43 2018
@@ -303,6 +303,10 @@ func (fix *Autofix) Apply() {
}
func (fix *Autofix) Realign(mkline MkLine, newWidth int) {
+
+ // XXX: Check whether this method can be implemented as Custom fix.
+ // This complicated code should not be in the Autofix type.
+
fix.assertRealLine()
G.Assertf(mkline.IsMultiline(), "Line must be a multiline.")
G.Assertf(mkline.IsVarassign() || mkline.IsCommentedVarassign(), "Line must be a variable assignment.")
@@ -315,8 +319,7 @@ func (fix *Autofix) Realign(mkline MkLin
oldWidth := 0 // The minimum required indentation in the original lines.
{
- // Parsing the continuation marker as variable value
- // is cheating but works well.
+ // Parsing the continuation marker as variable value is cheating but works well.
text := strings.TrimSuffix(mkline.raw[0].orignl, "\n")
m, _, _, _, _, valueAlign, value, _, _ := MatchVarassign(text)
if m && value != "\\" {
Index: pkgsrc/pkgtools/pkglint/files/pkgsrc.go
diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.13 pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.14
--- pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.13 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/pkgsrc.go Sun Dec 2 23:12:43 2018
@@ -323,8 +323,8 @@ func (src *Pkgsrc) loadTools() {
tools.ParseToolLine(mkline, true, !mklines.indentation.IsConditional())
case "_BUILD_DEFS":
- for _, bdvar := range mkline.ValueFields(mkline.Value()) {
- src.AddBuildDefs(bdvar)
+ for _, buildDefsVar := range mkline.Fields() {
+ src.AddBuildDefs(buildDefsVar)
}
}
}
Index: pkgsrc/pkgtools/pkglint/files/buildlink3_test.go
diff -u pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.21 pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.22
--- pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.21 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/buildlink3_test.go Sun Dec 2 23:12:43 2018
@@ -8,7 +8,7 @@ func (s *Suite) Test_ChecklinesBuildlink
t.SetupVartypes()
t.CreateFileLines("x11/Xbae/Makefile")
t.CreateFileLines("mk/motif.buildlink3.mk")
- mklines := t.SetupFileMkLines("buildlink3.mk",
+ mklines := t.SetupFileMkLines("category/package/buildlink3.mk",
MkRcsID,
"# XXX This file was created automatically using createbuildlink-@PKGVERSION@",
"",
@@ -30,7 +30,7 @@ func (s *Suite) Test_ChecklinesBuildlink
ChecklinesBuildlink3Mk(mklines)
t.CheckOutputLines(
- "ERROR: ~/buildlink3.mk:2: This comment indicates unfinished work (url2pkg).")
+ "ERROR: ~/category/package/buildlink3.mk:2: This comment indicates unfinished work (url2pkg).")
}
// Before version 5.3, pkglint wrongly warned here.
Index: pkgsrc/pkgtools/pkglint/files/vartype.go
diff -u pkgsrc/pkgtools/pkglint/files/vartype.go:1.21 pkgsrc/pkgtools/pkglint/files/vartype.go:1.22
--- pkgsrc/pkgtools/pkglint/files/vartype.go:1.21 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/vartype.go Sun Dec 2 23:12:43 2018
@@ -16,6 +16,7 @@ type Vartype struct {
type KindOfList uint8
+// TODO: Remove lkSpace. Since 2015 the .for variables are split on shell words, like everywhere else.
const (
lkNone KindOfList = iota // Plain data type
lkSpace // List entries are separated by whitespace; used in .for loops.
@@ -203,6 +204,10 @@ func (bt *BasicType) AllowedEnums() stri
return bt.name[6 : len(bt.name)-1]
}
+// TODO: Try to implement BasicType.PossibleChars()
+// TODO: Try to implement BasicType.CanBeEmpty()
+// TODO: Try to implement BasicType.PossibleWords() / PossibleValues()
+
var (
BtAwkCommand = &BasicType{"AwkCommand", (*VartypeCheck).AwkCommand}
BtBasicRegularExpression = &BasicType{"BasicRegularExpression", (*VartypeCheck).BasicRegularExpression}
Index: pkgsrc/pkgtools/pkglint/files/category_test.go
diff -u pkgsrc/pkgtools/pkglint/files/category_test.go:1.14 pkgsrc/pkgtools/pkglint/files/category_test.go:1.15
--- pkgsrc/pkgtools/pkglint/files/category_test.go:1.14 Wed Nov 7 20:58:22 2018
+++ pkgsrc/pkgtools/pkglint/files/category_test.go Sun Dec 2 23:12:43 2018
@@ -23,7 +23,7 @@ func (s *Suite) Test_CheckdirCategory__t
"NOTE: ~/archivers/Makefile:2: This variable value should be aligned to column 17.",
"NOTE: ~/archivers/Makefile:3: This variable value should be aligned with tabs, not spaces, to column 17.",
"NOTE: ~/archivers/Makefile:4: This variable value should be aligned to column 17.",
- "ERROR: ~/archivers/Makefile:6: \"../mk/category.mk\" does not exist.",
+ "ERROR: ~/archivers/Makefile:6: Relative path \"../mk/category.mk\" does not exist.",
"NOTE: ~/archivers/Makefile:1: Empty line expected after this line.",
"ERROR: ~/archivers/Makefile:2: COMMENT= line expected.",
"NOTE: ~/archivers/Makefile:1: Empty line expected after this line.", // XXX: Duplicate.
Index: pkgsrc/pkgtools/pkglint/files/licenses.go
diff -u pkgsrc/pkgtools/pkglint/files/licenses.go:1.17 pkgsrc/pkgtools/pkglint/files/licenses.go:1.18
--- pkgsrc/pkgtools/pkglint/files/licenses.go:1.17 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/licenses.go Sun Dec 2 23:12:43 2018
@@ -26,7 +26,7 @@ func (lc *LicenseChecker) checkName(lice
licenseFile := ""
if G.Pkg != nil {
if mkline := G.Pkg.vars.FirstDefinition("LICENSE_FILE"); mkline != nil {
- licenseFile = G.Pkg.File(mkline.ResolveVarsInRelativePath(mkline.Value(), false))
+ licenseFile = G.Pkg.File(mkline.ResolveVarsInRelativePath(mkline.Value()))
}
}
if licenseFile == "" {
Index: pkgsrc/pkgtools/pkglint/files/line.go
diff -u pkgsrc/pkgtools/pkglint/files/line.go:1.28 pkgsrc/pkgtools/pkglint/files/line.go:1.29
--- pkgsrc/pkgtools/pkglint/files/line.go:1.28 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/line.go Sun Dec 2 23:12:43 2018
@@ -35,6 +35,8 @@ func (rline *RawLine) String() string {
type Line = *LineImpl
type LineImpl struct {
+ // TODO: Consider storing pointers to the Filename and Basename instead of strings to save memory.
+ // But first find out where and why pkglint needs so much memory (200 MB for a full recursive run over pkgsrc + wip).
Filename string // uses / as directory separator on all platforms
Basename string // the last component of the Filename
firstLine int32 // zero means the whole file, -1 means EOF
Index: pkgsrc/pkgtools/pkglint/files/lines.go
diff -u pkgsrc/pkgtools/pkglint/files/lines.go:1.2 pkgsrc/pkgtools/pkglint/files/lines.go:1.3
--- pkgsrc/pkgtools/pkglint/files/lines.go:1.2 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/lines.go Sun Dec 2 23:12:43 2018
@@ -45,7 +45,7 @@ func (ls *LinesImpl) CheckRcsID(index in
if G.Wip && expanded != "" {
fix := line.Autofix()
- fix.Errorf("Expected exactly %q.", suggestedPrefix+"$"+"NetBSD$")
+ fix.Notef("Expected exactly %q.", suggestedPrefix+"$"+"NetBSD$")
fix.Explain(
"Several files in pkgsrc must contain the CVS Id, so that their",
"current version can be traced back later from a binary package.",
@@ -59,7 +59,7 @@ func (ls *LinesImpl) CheckRcsID(index in
"",
"To preserve the history of the CVS Id, should that ever be needed,",
"remove the leading $.")
- fix.InsertBefore(suggestedPrefix + "$" + "NetBSD$")
+ fix.ReplaceRegex(`.*`, suggestedPrefix+"$"+"NetBSD$", 1)
fix.Apply()
}
Index: pkgsrc/pkgtools/pkglint/files/lines_test.go
diff -u pkgsrc/pkgtools/pkglint/files/lines_test.go:1.1 pkgsrc/pkgtools/pkglint/files/lines_test.go:1.2
--- pkgsrc/pkgtools/pkglint/files/lines_test.go:1.1 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/lines_test.go Sun Dec 2 23:12:43 2018
@@ -47,8 +47,18 @@ func (s *Suite) Test_Lines_CheckRcsID__w
G.CheckDirent(t.File("wip/package"))
t.CheckOutputLines(
- "ERROR: ~/wip/package/file1.mk:1: Expected exactly \"# $"+"NetBSD$\".",
+ "NOTE: ~/wip/package/file1.mk:1: Expected exactly \"# $"+"NetBSD$\".",
"ERROR: ~/wip/package/file3.mk:1: Expected \"# $"+"NetBSD$\".",
"ERROR: ~/wip/package/file4.mk:1: Expected \"# $"+"NetBSD$\".",
"ERROR: ~/wip/package/file5.mk:1: Expected \"# $"+"NetBSD$\".")
+
+ G.Logger.Opts.Autofix = true
+
+ G.CheckDirent(t.File("wip/package"))
+
+ t.CheckOutputLines(
+ "AUTOFIX: ~/wip/package/file1.mk:1: Replacing \"# $NetBSD: lines_test.go,v 1.2 2018/12/02 23:12:43 rillig Exp $\" with \"# $NetBSD: lines_test.go,v 1.2 2018/12/02 23:12:43 rillig Exp
$\".",
+ "AUTOFIX: ~/wip/package/file3.mk:1: Inserting a line \"# $NetBSD: lines_test.go,v 1.2 2018/12/02 23:12:43 rillig Exp $\" before this line.",
+ "AUTOFIX: ~/wip/package/file4.mk:1: Inserting a line \"# $NetBSD: lines_test.go,v 1.2 2018/12/02 23:12:43 rillig Exp $\" before this line.",
+ "AUTOFIX: ~/wip/package/file5.mk:1: Inserting a line \"# $NetBSD: lines_test.go,v 1.2 2018/12/02 23:12:43 rillig Exp $\" before this line.")
}
Index: pkgsrc/pkgtools/pkglint/files/mkline.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline.go:1.41 pkgsrc/pkgtools/pkglint/files/mkline.go:1.42
--- pkgsrc/pkgtools/pkglint/files/mkline.go:1.41 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/mkline.go Sun Dec 2 23:12:43 2018
@@ -31,6 +31,7 @@ type mkLineAssignImpl struct {
value string // The trimmed value
valueMk []*MkToken // The value, sent through splitIntoMkWords
valueMkRest string // nonempty in case of parse errors
+ fields []string // The value, space-separated according to shell quoting rules
comment string
}
type mkLineShell struct {
@@ -43,9 +44,10 @@ type mkLineDirectiveImpl struct {
indent string // the space between the leading "." and the directive
directive string // "if", "else", "for", etc.
args string
- comment string // mainly interesting for .endif and .endfor
- elseLine MkLine // for .if (filled in later)
- cond MkCond // for .if and .elif (filled in later as needed)
+ comment string // mainly interesting for .endif and .endfor
+ elseLine MkLine // for .if (filled in later)
+ cond MkCond // for .if and .elif (filled in on first access)
+ fields []string // the arguments for the .for loop (filled in on first access)
}
type mkLineInclude = *mkLineIncludeImpl // See https://github.com/golang/go/issues/28045
type mkLineIncludeImpl struct {
@@ -110,6 +112,7 @@ func NewMkLine(line Line) *MkLineImpl {
strings.Replace(value, "\\#", "#", -1),
nil,
"",
+ nil,
comment}}
}
@@ -128,7 +131,7 @@ func NewMkLine(line Line) *MkLineImpl {
}
if m, indent, directive, args, comment := matchMkDirective(text); m {
- return &MkLineImpl{line, &mkLineDirectiveImpl{indent, directive, args, comment, nil, nil}}
+ return &MkLineImpl{line, &mkLineDirectiveImpl{indent, directive, args, comment, nil, nil, nil}}
}
if m, indent, directive, includedFile := MatchMkInclude(text); m {
@@ -472,6 +475,41 @@ func (mkline *MkLineImpl) ValueTokens()
return assign.valueMk
}
+// Fields applies to variable assignments and .for loops.
+// For variable assignments, it returns the right-hand side, properly split into words.
+// For .for loops, it returns all arguments (including variable names), properly split into words.
+func (mkline *MkLineImpl) Fields() []string {
+ if mkline.IsVarassign() {
+ value := mkline.Value()
+ if value == "" {
+ return nil
+ }
+
+ assign := mkline.data.(mkLineAssign)
+ if assign.fields != nil {
+ return assign.fields
+ }
+
+ assign.fields = mkline.ValueFields(value)
+ return assign.fields
+ }
+
+ // For .for loops.
+ args := mkline.Args()
+ if args == "" {
+ return nil
+ }
+
+ directive := mkline.data.(mkLineDirective)
+ if directive.fields != nil {
+ return directive.fields
+ }
+
+ directive.fields = mkline.ValueFields(args)
+ return directive.fields
+
+}
+
func (mkline *MkLineImpl) WithoutMakeVariables(value string) string {
valueNovar := ""
for _, token := range NewMkParser(nil, value, false).MkTokens() {
@@ -482,7 +520,7 @@ func (mkline *MkLineImpl) WithoutMakeVar
return valueNovar
}
-func (mkline *MkLineImpl) ResolveVarsInRelativePath(relativePath string, adjustDepth bool) string {
+func (mkline *MkLineImpl) ResolveVarsInRelativePath(relativePath string) string {
var basedir string
if G.Pkg != nil {
@@ -526,13 +564,6 @@ func (mkline *MkLineImpl) ResolveVarsInR
tmp = strings.Replace(tmp, "${PKGDIR}", G.Pkg.Pkgdir, -1)
}
- // TODO: What is this good for, and in which cases does it make a difference?
- if adjustDepth {
- if hasPrefix(tmp, "../../") && !hasPrefix(tmp[6:], ".") {
- tmp = pkgsrcdir + "/" + tmp[6:]
- }
- }
-
tmp = cleanpath(tmp)
if trace.Tracing && relativePath != tmp {
Index: pkgsrc/pkgtools/pkglint/files/pkglint.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint.go:1.41 pkgsrc/pkgtools/pkglint/files/pkglint.go:1.42
--- pkgsrc/pkgtools/pkglint/files/pkglint.go:1.41 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/pkglint.go Sun Dec 2 23:12:43 2018
@@ -359,7 +359,7 @@ func (pkglint *Pkglint) checkdirPackage(
!contains(filename, pkg.Pkgdir+"/") &&
!contains(filename, pkg.Filesdir+"/") {
if fragmentMklines := LoadMk(filename, MustSucceed); fragmentMklines != nil {
- fragmentMklines.DetermineUsedVariables()
+ fragmentMklines.collectUsedVariables()
}
}
if hasPrefix(basename, "PLIST") {
@@ -422,6 +422,8 @@ func findPkgsrcTopdir(filename string) s
}
func resolveVariableRefs(text string) (resolved string) {
+ // TODO: How does this fit into the Scope type, which is newer than this function?
+
if !contains(text, "${") {
return text
}
Index: pkgsrc/pkgtools/pkglint/files/mkline_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.45 pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.46
--- pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.45 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/mkline_test.go Sun Dec 2 23:12:43 2018
@@ -395,7 +395,7 @@ func (s *Suite) Test_MkLine_VariableNeed
MkRcsID,
"GENERATE_PLIST= cd ${DESTDIR}${PREFIX}; ${FIND} * \\( -type f -or -type l \\) | ${SORT};")
- G.Mk.DetermineDefinedVariables()
+ G.Mk.collectDefinedVariables()
MkLineChecker{G.Mk.mklines[1]}.Check()
t.CheckOutputLines(
@@ -916,7 +916,7 @@ func (s *Suite) Test_MkLine_ResolveVarsI
mkline := mklines.mklines[0]
checkResolve := func(before string, after string) {
- c.Check(mkline.ResolveVarsInRelativePath(before, false), equals, after)
+ c.Check(mkline.ResolveVarsInRelativePath(before), equals, after)
}
checkResolve("", ".")
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.23 pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.24
--- pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.23 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker.go Sun Dec 2 23:12:43 2018
@@ -162,7 +162,7 @@ func (ck MkLineChecker) checkDirective(f
ck.checkDirectiveFor(forVars, ind)
case directive == "undef":
- for _, varname := range fields(args) {
+ for _, varname := range mkline.Fields() {
if forVars[varname] {
mkline.Notef("Using \".undef\" after a \".for\" loop is unnecessary.")
}
@@ -203,8 +203,8 @@ func (ck MkLineChecker) checkDirectiveFo
if matches(forvar, `^[_a-z][_a-z0-9]*$`) {
// Fine.
- } else if strings.IndexFunc(forvar, func(r rune) bool { return 'A' <= r && r <= 'Z' }) != -1 {
- mkline.Warnf(".for variable names should not contain uppercase letters.")
+ } else if matches(forvar, `^[A-Z_a-z][0-9A-Z_a-z]*$`) {
+ mkline.Warnf("The variable name %q in the .for loop should not contain uppercase letters.", forvar)
} else {
mkline.Errorf("Invalid variable name %q.", forvar)
}
@@ -764,18 +764,18 @@ func (ck MkLineChecker) checkVaruseDepre
}
}
-func (ck MkLineChecker) checkVarassignDecreasingVersions(varname, value string) {
+func (ck MkLineChecker) checkVarassignDecreasingVersions() {
if trace.Tracing {
- defer trace.Call2(varname, value)()
+ defer trace.Call0()()
}
mkline := ck.MkLine
- strVersions := fields(value)
+ strVersions := mkline.Fields()
intVersions := make([]int, len(strVersions))
for i, strVersion := range strVersions {
iver, err := strconv.Atoi(strVersion)
if err != nil || !(iver > 0) {
- mkline.Errorf("All values for %s must be positive integers.", varname)
+ mkline.Errorf("All values for %s must be positive integers.", mkline.Varname())
return
}
intVersions[i] = iver
@@ -783,7 +783,7 @@ func (ck MkLineChecker) checkVarassignDe
for i, ver := range intVersions {
if i > 0 && ver >= intVersions[i-1] {
- mkline.Warnf("The values for %s should be in decreasing order.", varname)
+ mkline.Warnf("The values for %s should be in decreasing order.", mkline.Varname())
G.Explain(
"If they aren't, it may be possible that needless versions of",
"packages are installed.")
@@ -945,7 +945,7 @@ func (ck MkLineChecker) checkVarassignSp
}
if varname == "PYTHON_VERSIONS_ACCEPTED" {
- ck.checkVarassignDecreasingVersions(varname, value)
+ ck.checkVarassignDecreasingVersions()
}
if mkline.VarassignComment() == "# defined" && !hasSuffix(varname, "_MK") && !hasSuffix(varname, "_COMMON") {
@@ -971,8 +971,8 @@ func (ck MkLineChecker) checkVarassignSp
}
if varname == "PKG_SKIP_REASON" && G.Mk.indentation.DependsOn("OPSYS") {
- mkline.Notef("Consider defining NOT_FOR_PLATFORM instead of " +
- "setting PKG_SKIP_REASON depending on ${OPSYS}.")
+ mkline.Notef("Consider setting NOT_FOR_PLATFORM instead of " +
+ "PKG_SKIP_REASON depending on ${OPSYS}.")
}
}
@@ -1151,54 +1151,27 @@ func (ck MkLineChecker) checkDirectiveCo
return
}
- checkEmpty := func(varuse *MkVarUse) {
- varname := varuse.varname
- if matches(varname, `^\$.*:[MN]`) {
- mkline.Warnf("The empty() function takes a variable name as parameter, not a variable expression.")
- G.Explain(
- "Instead of empty(${VARNAME:Mpattern}), you should write either",
- "of the following:",
- "",
- "\tempty(VARNAME:Mpattern)",
- "\t${VARNAME:Mpattern} == \"\"",
- "",
- "Instead of !empty(${VARNAME:Mpattern}), you should write either",
- "of the following:",
- "",
- "\t!empty(VARNAME:Mpattern)",
- "\t${VARNAME:Mpattern}")
- }
-
- modifiers := varuse.modifiers
- for _, modifier := range modifiers {
- if m, positive, pattern := modifier.MatchMatch(); m && (positive || len(modifiers) == 1) {
- ck.checkVartype(varname, opUseMatch, pattern, "")
-
- vartype := G.Pkgsrc.VariableType(varname)
- if matches(pattern, `^[\w-/]+$`) && vartype != nil && !vartype.IsConsideredList() {
- mkline.Notef("%s should be compared using == instead of the :M or :N modifier without wildcards.", varname)
- G.Explain(
- "This variable has a single value, not a list of values. Therefore",
- "it feels strange to apply list operators like :M and :N onto it.",
- "A more direct approach is to use the == and != operators.",
- "",
- "An entirely different case is when the pattern contains wildcards",
- "like ^, *, $. In such a case, using the :M or :N modifiers is",
- "useful and preferred.")
- }
- }
- }
- }
-
checkCompareVarStr := func(varuse *MkVarUse, op string, value string) {
varname := varuse.varname
varmods := varuse.modifiers
- if len(varmods) == 0 {
+ switch len(varmods) {
+ case 0:
ck.checkCompareVarStr(varname, op, value)
- } else if len(varmods) == 1 {
+
+ case 1:
if m, _, _ := varmods[0].MatchMatch(); m && value != "" {
ck.checkVartype(varname, opUseMatch, value, "")
}
+
+ default:
+ // This case covers ${VAR:Mfilter:O:u} or similar uses in conditions.
+ // To check these properly, pkglint first needs to know the most common modifiers and how they interact.
+ // As of November 2018, the modifiers are not modeled.
+ // The following tracing statement makes it easy to discover these cases,
+ // in order to decide whether checking them is worthwhile.
+ if trace.Tracing {
+ trace.Stepf("checkCompareVarStr ${%s%s} %s %s", varuse.varname, varuse.Mod(), op, value)
+ }
}
}
@@ -1209,11 +1182,52 @@ func (ck MkLineChecker) checkDirectiveCo
}
cond.Walk(&MkCondCallback{
- Empty: checkEmpty,
+ Empty: ck.checkDirectiveCondEmpty,
CompareVarStr: checkCompareVarStr,
VarUse: checkVarUse})
}
+// checkDirectiveCondEmpty checks a condition of the form empty(...) in an .if directive.
+func (ck MkLineChecker) checkDirectiveCondEmpty(varuse *MkVarUse) {
+ varname := varuse.varname
+ if matches(varname, `^\$.*:[MN]`) {
+ ck.MkLine.Warnf("The empty() function takes a variable name as parameter, not a variable expression.")
+ G.Explain(
+ "Instead of empty(${VARNAME:Mpattern}), you should write either",
+ "of the following:",
+ "",
+ "\tempty(VARNAME:Mpattern)",
+ "\t${VARNAME:Mpattern} == \"\"",
+ "",
+ "Instead of !empty(${VARNAME:Mpattern}), you should write either",
+ "of the following:",
+ "",
+ "\t!empty(VARNAME:Mpattern)",
+ "\t${VARNAME:Mpattern}")
+ }
+
+ modifiers := varuse.modifiers
+ for _, modifier := range modifiers {
+ if m, positive, pattern := modifier.MatchMatch(); m && (positive || len(modifiers) == 1) {
+ ck.checkVartype(varname, opUseMatch, pattern, "")
+
+ vartype := G.Pkgsrc.VariableType(varname)
+ if matches(pattern, `^[\w-/]+$`) && vartype != nil && !vartype.IsConsideredList() {
+ ck.MkLine.Notef("%s should be compared using %s instead of matching against %q.",
+ varname, ifelseStr(positive, "==", "!="), ":"+modifier.Text)
+ G.Explain(
+ "This variable has a single value, not a list of values.",
+ "Therefore it feels strange to apply list operators like :M and :N onto it.",
+ "A more direct approach is to use the == and != operators.",
+ "",
+ "An entirely different case is when the pattern contains wildcards like ^, *, $.",
+ "In such a case, using the :M or :N modifiers is useful and preferred.")
+ }
+ }
+ }
+
+}
+
func (ck MkLineChecker) checkCompareVarStr(varname, op, value string) {
ck.checkVartype(varname, opUseCompare, value, "")
@@ -1226,6 +1240,17 @@ func (ck MkLineChecker) checkCompareVarS
}
}
+// CheckRelativePkgdir checks a reference from one pkgsrc package to another.
+// These references should always have the form ../../category/package.
+//
+// When used in DEPENDS or similar variables, these directories could theoretically
+// also be relative to the pkgsrc root, which would save a few keystrokes.
+// This, however, is not implemented in pkgsrc and suggestions regarding this topic
+// have not been made in the last two decades on the public mailing lists.
+// While being a bit redundant, the current scheme works well.
+//
+// When used in .include directives, the relative package directories must be written
+// with the leading ../.. anyway, so the benefit might not be too big at all.
func (ck MkLineChecker) CheckRelativePkgdir(pkgdir string) {
if trace.Tracing {
defer trace.Call1(pkgdir)()
@@ -1233,8 +1258,9 @@ func (ck MkLineChecker) CheckRelativePkg
mkline := ck.MkLine
ck.CheckRelativePath(pkgdir, true)
- pkgdir = mkline.ResolveVarsInRelativePath(pkgdir, false)
+ pkgdir = mkline.ResolveVarsInRelativePath(pkgdir)
+ // XXX: Is the leading "./" realistic?
if m, otherpkgpath := match1(pkgdir, `^(?:\./)?\.\./\.\./([^/]+/[^/]+)$`); m {
if !fileExists(G.Pkgsrc.File(otherpkgpath + "/Makefile")) {
mkline.Errorf("There is no package in %q.", otherpkgpath)
@@ -1249,6 +1275,8 @@ func (ck MkLineChecker) CheckRelativePkg
}
}
+// CheckRelativePath checks a relative path that leads to the directory of another package
+// or to a subdirectory thereof or a file within there.
func (ck MkLineChecker) CheckRelativePath(relativePath string, mustExist bool) {
if trace.Tracing {
defer trace.Call(relativePath, mustExist)()
@@ -1259,7 +1287,7 @@ func (ck MkLineChecker) CheckRelativePat
mkline.Errorf("A main pkgsrc package must not depend on a pkgsrc-wip package.")
}
- resolvedPath := mkline.ResolveVarsInRelativePath(relativePath, true)
+ resolvedPath := mkline.ResolveVarsInRelativePath(relativePath)
if containsVarRef(resolvedPath) {
return
}
@@ -1270,7 +1298,7 @@ func (ck MkLineChecker) CheckRelativePat
}
if _, err := os.Stat(abs); err != nil {
if mustExist {
- mkline.Errorf("%q does not exist.", resolvedPath)
+ mkline.Errorf("Relative path %q does not exist.", resolvedPath)
}
return
}
@@ -1286,5 +1314,6 @@ func (ck MkLineChecker) CheckRelativePat
// For category Makefiles.
default:
mkline.Warnf("Invalid relative path %q.", relativePath)
+ // TODO: Explain this warning.
}
}
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.19 pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.20
--- pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.19 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go Sun Dec 2 23:12:43 2018
@@ -23,14 +23,17 @@ func (s *Suite) Test_MkLineChecker_Check
t.CreateFileLines("mk/bsd.prefs.mk")
mklines := t.SetupFileMkLines("category/package/buildlink3.mk",
".include \"../../mk/bsd.prefs.mk\"")
- // If the buildlink3.mk file is not actually created, resolving the
+ // If the buildlink3.mk file doesn't actually exist, resolving the
// relative path fails since that depends on the actual file system,
// not on syntactical paths; see os.Stat in CheckRelativePath.
+ //
+ // TODO: Refactor relpath to be independent of a filesystem.
MkLineChecker{mklines.mklines[0]}.Check()
t.CheckOutputLines(
- "NOTE: ~/category/package/buildlink3.mk:1: For efficiency reasons, please include bsd.fast.prefs.mk instead of bsd.prefs.mk.")
+ "NOTE: ~/category/package/buildlink3.mk:1: For efficiency reasons, " +
+ "please include bsd.fast.prefs.mk instead of bsd.prefs.mk.")
}
func (s *Suite) Test_MkLineChecker_checkInclude(c *check.C) {
@@ -53,12 +56,16 @@ func (s *Suite) Test_MkLineChecker_check
mklines.Check()
t.CheckOutputLines(
- "ERROR: ~/category/package/filename.mk:3: ../../pkgtools/x11-links/buildlink3.mk must not be included directly. "+
+ "ERROR: ~/category/package/filename.mk:3: "+
+ "../../pkgtools/x11-links/buildlink3.mk must not be included directly. "+
"Include \"../../mk/x11.buildlink3.mk\" instead.",
- "ERROR: ~/category/package/filename.mk:4: ../../graphics/jpeg/buildlink3.mk must not be included directly. "+
+ "ERROR: ~/category/package/filename.mk:4: "+
+ "../../graphics/jpeg/buildlink3.mk must not be included directly. "+
"Include \"../../mk/jpeg.buildlink3.mk\" instead.",
- "WARN: ~/category/package/filename.mk:5: Please write \"USE_TOOLS+= intltool\" instead of this line.",
- "ERROR: ~/category/package/filename.mk:6: ../../devel/intltool/builtin.mk must not be included directly. "+
+ "WARN: ~/category/package/filename.mk:5: "+
+ "Please write \"USE_TOOLS+= intltool\" instead of this line.",
+ "ERROR: ~/category/package/filename.mk:6: "+
+ "../../devel/intltool/builtin.mk must not be included directly. "+
"Include \"../../devel/intltool/buildlink3.mk\" instead.")
}
@@ -70,10 +77,24 @@ func (s *Suite) Test_MkLineChecker_check
MkLineChecker{mkline}.checkInclude()
t.CheckOutputLines(
- "ERROR: ~/Makefile:2: \"other/package/Makefile\" does not exist.",
+ "ERROR: ~/Makefile:2: Relative path \"../../other/package/Makefile\" does not exist.",
"ERROR: ~/Makefile:2: Other Makefiles must not be included directly.")
}
+func (s *Suite) Test_MkLineChecker_checkInclude__Makefile_exists(c *check.C) {
+ t := s.Init(c)
+
+ t.CreateFileLines("other/existing/Makefile")
+ t.SetupPackage("category/package",
+ ".include \"../../other/existing/Makefile\"",
+ ".include \"../../other/not-found/Makefile\"")
+
+ G.checkdirPackage(t.File("category/package"))
+
+ t.CheckOutputLines(
+ "ERROR: ~/category/package/Makefile:20: Cannot read \"../../other/existing/Makefile\".")
+}
+
func (s *Suite) Test_MkLineChecker_checkDirective(c *check.C) {
t := s.Init(c)
@@ -96,26 +117,50 @@ func (s *Suite) Test_MkLineChecker_check
"",
".for var in a b c",
".endfor",
- ".undef var",
+ ".undef var")
+
+ mklines.Check()
+
+ t.CheckOutputLines(
+ "ERROR: category/package/filename.mk:3: \".for\" requires arguments.",
+ "ERROR: category/package/filename.mk:6: \".if\" requires arguments.",
+ "ERROR: category/package/filename.mk:7: \".else\" does not take arguments. "+
+ "If you meant \"else if\", use \".elif\".",
+ "ERROR: category/package/filename.mk:8: \".endif\" does not take arguments.",
+ "WARN: category/package/filename.mk:10: The \".ifdef\" directive is deprecated. "+
+ "Please use \".if defined(FNAME_MK)\" instead.",
+ "WARN: category/package/filename.mk:12: The \".ifndef\" directive is deprecated. "+
+ "Please use \".if !defined(FNAME_MK)\" instead.",
+ "NOTE: category/package/filename.mk:17: Using \".undef\" after a \".for\" loop is unnecessary.")
+}
+
+func (s *Suite) Test_MkLineChecker_checkDirective__for_loop_varname(c *check.C) {
+ t := s.Init(c)
+
+ t.SetupVartypes()
+
+ mklines := t.NewMkLines("filename.mk",
+ MkRcsID,
+ "",
+ ".for VAR in a b c", // Should be lowercase.
+ ".endfor",
"",
- ".for VAR in a b c",
+ ".for _var_ in a b c", // Should be written without underscores.
".endfor",
"",
- ".for $ in a b c",
+ ".for .var. in a b c", // Should be written without dots.
+ ".endfor",
+ "",
+ ".for ${VAR} in a b c", // The variable name really must be an identifier.
".endfor")
mklines.Check()
t.CheckOutputLines(
- "ERROR: category/package/filename.mk:3: \".for\" requires arguments.",
- "ERROR: category/package/filename.mk:6: \".if\" requires arguments.",
- "ERROR: category/package/filename.mk:7: \".else\" does not take arguments. If you meant \"else if\", use \".elif\".",
- "ERROR: category/package/filename.mk:8: \".endif\" does not take arguments.",
- "WARN: category/package/filename.mk:10: The \".ifdef\" directive is deprecated. Please use \".if defined(FNAME_MK)\" instead.",
- "WARN: category/package/filename.mk:12: The \".ifndef\" directive is deprecated. Please use \".if !defined(FNAME_MK)\" instead.",
- "NOTE: category/package/filename.mk:17: Using \".undef\" after a \".for\" loop is unnecessary.",
- "WARN: category/package/filename.mk:19: .for variable names should not contain uppercase letters.",
- "ERROR: category/package/filename.mk:22: Invalid variable name \"$\".")
+ "WARN: filename.mk:3: The variable name \"VAR\" in the .for loop should not contain uppercase letters.",
+ "WARN: filename.mk:6: Variable names starting with an underscore (_var_) are reserved for internal pkgsrc use.",
+ "ERROR: filename.mk:9: Invalid variable name \".var.\".",
+ "ERROR: filename.mk:12: Invalid variable name \"${VAR}\".")
}
func (s *Suite) Test_MkLineChecker_checkDependencyRule(c *check.C) {
@@ -145,10 +190,7 @@ func (s *Suite) Test_MkLineChecker_check
t.SetupCommandLine("-Wtypes")
t.SetupVartypes()
- vartype1 := G.Pkgsrc.vartypes["COMMENT"]
- c.Assert(vartype1, check.NotNil)
- c.Check(vartype1.guessed, equals, false)
-
+ // Since COMMENT is defined in vardefs.go its type is certain instead of guessed.
vartype := G.Pkgsrc.VariableType("COMMENT")
c.Assert(vartype, check.NotNil)
@@ -174,6 +216,9 @@ func (s *Suite) Test_MkLineChecker_check
t.CheckOutputEmpty()
}
+// The command line option -Wno-types can be used to suppress the type checks.
+// Suppressing it is rarely needed and comes from Feb 12 2005 when this feature was introduced.
+// Since then the type system has matured and proven effective.
func (s *Suite) Test_MkLineChecker_checkVartype__skip(c *check.C) {
t := s.Init(c)
@@ -217,7 +262,7 @@ func (s *Suite) Test_MkLineChecker_check
t.CheckOutputEmpty()
}
-func (s *Suite) Test_MkLineChecker_Check__conditions(c *check.C) {
+func (s *Suite) Test_MkLineChecker_checkDirectiveCond(c *check.C) {
t := s.Init(c)
t.SetupCommandLine("-Wtypes")
@@ -243,34 +288,22 @@ func (s *Suite) Test_MkLineChecker_Check
"WARN: filename:1: \"mailto:someone%example.org@localhost\"; is not a valid URL.")
testCond(".if !empty(PKGSRC_RUN_TEST:M[Y][eE][sS])",
- "WARN: filename:1: PKGSRC_RUN_TEST should be matched against \"[yY][eE][sS]\" or \"[nN][oO]\", not \"[Y][eE][sS]\".")
+ "WARN: filename:1: PKGSRC_RUN_TEST should be matched "+
+ "against \"[yY][eE][sS]\" or \"[nN][oO]\", not \"[Y][eE][sS]\".")
testCond(".if !empty(IS_BUILTIN.Xfixes:M[yY][eE][sS])")
testCond(".if !empty(${IS_BUILTIN.Xfixes:M[yY][eE][sS]})",
- "WARN: filename:1: The empty() function takes a variable name as parameter, not a variable expression.")
+ "WARN: filename:1: The empty() function takes a variable name as parameter, "+
+ "not a variable expression.")
- testCond(".if ${EMUL_PLATFORM} == \"linux-x386\"",
- "WARN: filename:1: "+
- "\"x386\" is not valid for the hardware architecture part of EMUL_PLATFORM. "+
- "Use one of "+
- "{ aarch64 aarch64eb alpha amd64 arc arm arm26 arm32 cobalt coldfire convex "+
- "dreamcast earm earmeb earmhf earmhfeb earmv4 earmv4eb earmv5 earmv5eb earmv6 earmv6eb "+
- "earmv6hf earmv6hfeb earmv7 earmv7eb earmv7hf earmv7hfeb evbarm hpcmips hpcsh hppa hppa64 "+
- "i386 i586 i686 ia64 m68000 m68k m88k mips mips64 mips64eb mips64el mipseb mipsel mipsn32 "+
- "mlrisc ns32k pc532 pmax powerpc powerpc64 rs6000 s390 sh3eb sh3el sparc sparc64 vax x86_64 "+
- "} instead.")
+ testCond(".if ${PKGSRC_COMPILER} == \"msvc\"",
+ "WARN: filename:1: \"msvc\" is not valid for PKGSRC_COMPILER. "+
+ "Use one of { ccache ccc clang distcc f2c gcc hp icc ido mipspro mipspro-ucode pcc sunpro xlc } instead.",
+ "WARN: filename:1: Use ${PKGSRC_COMPILER:Mmsvc} instead of the == operator.")
- testCond(".if ${EMUL_PLATFORM:Mlinux-x386}",
- "WARN: filename:1: "+
- "The pattern \"x386\" cannot match any of { aarch64 aarch64eb alpha amd64 arc arm arm26 "+
- "arm32 cobalt coldfire convex dreamcast earm earmeb earmhf earmhfeb earmv4 earmv4eb "+
- "earmv5 earmv5eb earmv6 earmv6eb earmv6hf earmv6hfeb earmv7 earmv7eb earmv7hf "+
- "earmv7hfeb evbarm hpcmips hpcsh hppa hppa64 i386 i586 i686 ia64 m68000 m68k m88k "+
- "mips mips64 mips64eb mips64el mipseb mipsel mipsn32 mlrisc ns32k pc532 pmax powerpc powerpc64 "+
- "rs6000 s390 sh3eb sh3el sparc sparc64 vax x86_64 } "+
- "for the hardware architecture part of EMUL_PLATFORM.",
- "NOTE: filename:1: EMUL_PLATFORM should be compared using == instead of the :M or :N modifier without wildcards.")
+ testCond(".if ${PKG_LIBTOOL:Mlibtool}",
+ "NOTE: filename:1: PKG_LIBTOOL should be compared using == instead of matching against \":Mlibtool\".")
testCond(".if ${MACHINE_PLATFORM:MUnknownOS-*-*} || ${MACHINE_ARCH:Mx86}",
"WARN: filename:1: "+
@@ -286,7 +319,7 @@ func (s *Suite) Test_MkLineChecker_Check
"m68000 m68k m88k mips mips64 mips64eb mips64el mipseb mipsel mipsn32 mlrisc ns32k pc532 pmax "+
"powerpc powerpc64 rs6000 s390 sh3eb sh3el sparc sparc64 vax x86_64 "+
"} for MACHINE_ARCH.",
- "NOTE: filename:1: MACHINE_ARCH should be compared using == instead of the :M or :N modifier without wildcards.")
+ "NOTE: filename:1: MACHINE_ARCH should be compared using == instead of matching against \":Mx86\".")
testCond(".if ${MASTER_SITES:Mftp://*} == \"ftp://netbsd.org/\"";)
}
@@ -386,11 +419,10 @@ func (s *Suite) Test_MkLineChecker_check
mklines.Check()
- // Evaluating PKG_SYSCONFDIR.* at load time is probably ok, though
- // pkglint cannot prove anything here.
+ // Evaluating PKG_SYSCONFDIR.* at load time is probably ok,
+ // though pkglint cannot prove anything here.
//
- // Evaluating .CURDIR at load time is ok since it is defined from
- // the beginning.
+ // Evaluating .CURDIR at load time is definitely ok since it is defined from the beginning.
t.CheckOutputLines(
"NOTE: options.mk:2: This variable value should be aligned to column 17.")
}
@@ -438,7 +470,7 @@ func (s *Suite) Test_MkLineChecker_check
"WARN: any.mk:2: PKGREVISION may not be used in any file; it is a write-only variable.")
}
-func (s *Suite) Test_MkLineChecker__warn_varuse_LOCALBASE(c *check.C) {
+func (s *Suite) Test_MkLineChecker_Check__warn_varuse_LOCALBASE(c *check.C) {
t := s.Init(c)
t.SetupVartypes()
@@ -453,14 +485,22 @@ func (s *Suite) Test_MkLineChecker__warn
func (s *Suite) Test_MkLineChecker_CheckRelativePkgdir(c *check.C) {
t := s.Init(c)
- mklines := t.SetupFileMkLines("Makefile",
+ t.CreateFileLines("other/package/Makefile")
+ // Must be in the filesystem because of directory references.
+ mklines := t.SetupFileMkLines("category/package/Makefile",
"# dummy")
+ ck := MkLineChecker{mklines.mklines[0]}
- MkLineChecker{mklines.mklines[0]}.CheckRelativePkgdir("../pkgbase")
-
- t.CheckOutputLines(
- "ERROR: ~/Makefile:1: \"../pkgbase\" does not exist.",
- "WARN: ~/Makefile:1: \"../pkgbase\" is not a valid relative package directory.")
+ ck.CheckRelativePkgdir("../pkgbase")
+ ck.CheckRelativePkgdir("../../other/package")
+ ck.CheckRelativePkgdir("../../other/does-not-exist")
+
+ // FIXME: The diagnostics for does-not-exist are redundant.
+ t.CheckOutputLines(
+ "ERROR: ~/category/package/Makefile:1: Relative path \"../pkgbase\" does not exist.",
+ "WARN: ~/category/package/Makefile:1: \"../pkgbase\" is not a valid relative package directory.",
+ "ERROR: ~/category/package/Makefile:1: Relative path \"../../other/does-not-exist\" does not exist.",
+ "ERROR: ~/category/package/Makefile:1: There is no package in \"other/does-not-exist\".")
}
// PR pkg/46570, item 2
@@ -476,39 +516,89 @@ func (s *Suite) Test_MkLineChecker__uncl
t.CheckOutputLines(
"WARN: Makefile:2: Unclosed Make variable starting at \"${EGDIR/apparmor.d $...\".",
"WARN: Makefile:2: EGDIRS is defined but not used.",
+
+ // XXX: This warning is redundant because of the "Unclosed" warning above.
"WARN: Makefile:2: Pkglint parse error in MkLine.Tokenize at "+
"\"${EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d\".")
}
-func (s *Suite) Test_MkLineChecker__Varuse_Modifier_L(c *check.C) {
+func (s *Suite) Test_MkLineChecker_Check__varuse_modifier_L(c *check.C) {
t := s.Init(c)
t.SetupVartypes()
- G.Mk = t.NewMkLines("x11/xkeyboard-config/Makefile",
- "FILES_SUBST+=XKBCOMP_SYMLINK=${${XKBBASE}/xkbcomp:L:Q}")
+ mklines := t.NewMkLines("x11/xkeyboard-config/Makefile",
+ "FILES_SUBST+=XKBCOMP_SYMLINK=${${XKBBASE}/xkbcomp:L:Q}",
+ "FILES_SUBST+=XKBCOMP_SYMLINK=${${XKBBASE}/xkbcomp:Q}")
- MkLineChecker{G.Mk.mklines[0]}.Check()
+ MkLineChecker{mklines.mklines[0]}.Check()
+ MkLineChecker{mklines.mklines[1]}.Check()
- // Don't warn that ${XKBBASE}/xkbcomp is used but not defined.
- t.CheckOutputEmpty()
+ // In line 1, don't warn that ${XKBBASE}/xkbcomp is used but not defined.
+ // This is because the :L modifier interprets everything before as an expression
+ // instead of a variable name.
+ //
+ // In line 2 the :L modifier is missing, therefore ${XKBBASE}/xkbcomp is the
+ // name of another variable, and that variable is not known. Only XKBBASE is known.
+ //
+ // FIXME: The below warnings are wrong because the MkParser does not recognize the
+ // slash as part of a variable name. Because of that, parsing stops before the $.
+ // The warning "Unclosed Make variable" wrongly assumes that any parse error from
+ // a variable use is because of unclosed braces, which it isn't in this case.h
+ t.CheckOutputLines(
+ "WARN: x11/xkeyboard-config/Makefile:2: Unclosed Make variable starting at \"${${XKBBASE}/xkbcomp...\".",
+ "WARN: x11/xkeyboard-config/Makefile:2: Unclosed Make variable starting at \"${${XKBBASE}/xkbcomp...\".")
}
func (s *Suite) Test_MkLineChecker_checkDirectiveCond__comparison_with_shell_command(c *check.C) {
t := s.Init(c)
t.SetupVartypes()
- G.Mk = t.NewMkLines("security/openssl/Makefile",
+ mklines := t.NewMkLines("security/openssl/Makefile",
MkRcsID,
".if ${PKGSRC_COMPILER} == \"gcc\" && ${CC} == \"cc\"",
".endif")
- G.Mk.Check()
+ mklines.Check()
// Don't warn about unknown shell command "cc".
t.CheckOutputLines(
"WARN: security/openssl/Makefile:2: Use ${PKGSRC_COMPILER:Mgcc} instead of the == operator.")
}
+func (s *Suite) Test_MkLineChecker_checkDirectiveCondEmpty(c *check.C) {
+ t := s.Init(c)
+
+ t.SetupVartypes()
+ mkline := t.NewMkLine("module.mk", 123, ".if ${PKGPATH} == \"category/package\"")
+ ck := MkLineChecker{mkline}
+
+ // FIXME: checkDirectiveCondEmpty cannot know whether it is empty(...) or !empty(...).
+ // It must know that to generate the proper diagnostics.
+
+ ck.checkDirectiveCondEmpty(NewMkVarUse("PKGPATH", "Mpattern"))
+
+ // When the pattern contains placeholders, it cannot be converted to == or !=.
+ ck.checkDirectiveCondEmpty(NewMkVarUse("PKGPATH", "Mpa*n"))
+
+ ck.checkDirectiveCondEmpty(NewMkVarUse("PKGPATH", "tl", "Mpattern"))
+
+ ck.checkDirectiveCondEmpty(NewMkVarUse("PKGPATH", "Ncategory/package"))
+
+ // ${PKGPATH:None:Ntwo} is a short variant of ${PKGPATH} != "one" && ${PKGPATH} != "two",
+ // therefore no note is logged in this case.
+ ck.checkDirectiveCondEmpty(NewMkVarUse("PKGPATH", "None", "Ntwo"))
+
+ // Note: this combination doesn't make sense since the patterns "one" and "two" don't overlap.
+ ck.checkDirectiveCondEmpty(NewMkVarUse("PKGPATH", "Mone", "Mtwo"))
+
+ t.CheckOutputLines(
+ "NOTE: module.mk:123: PKGPATH should be compared using == instead of matching against \":Mpattern\".",
+ "NOTE: module.mk:123: PKGPATH should be compared using == instead of matching against \":Mpattern\".",
+ "NOTE: module.mk:123: PKGPATH should be compared using != instead of matching against \":Ncategory/package\".",
+ "NOTE: module.mk:123: PKGPATH should be compared using == instead of matching against \":Mone\".",
+ "NOTE: module.mk:123: PKGPATH should be compared using == instead of matching against \":Mtwo\".")
+}
+
func (s *Suite) Test_MkLineChecker_checkDirectiveCond__comparing_PKGSRC_COMPILER_with_eqeq(c *check.C) {
t := s.Init(c)
@@ -538,7 +628,8 @@ func (s *Suite) Test_MkLineChecker_check
c.Check(words, deepEquals, []string{"`pkg-config pidgin --cflags`"})
c.Check(rest, equals, "")
- MkLineChecker{G.Mk.mklines[1]}.checkVartype("CFLAGS", opAssignAppend, "`pkg-config pidgin --cflags`", "")
+ ck := MkLineChecker{G.Mk.mklines[1]}
+ ck.checkVartype("CFLAGS", opAssignAppend, "`pkg-config pidgin --cflags`", "")
// No warning about "`pkg-config" being an unknown CFlag.
t.CheckOutputEmpty()
@@ -740,6 +831,10 @@ func (s *Suite) Test_MkLineChecker_Check
t.CheckOutputEmpty()
}
+// Any variable that is defined in the pkgsrc infrastructure in mk/**/*.mk is
+// considered defined, and no "used but not defined" warning is logged for it.
+//
+// See Pkgsrc.loadUntypedVars.
func (s *Suite) Test_MkLineChecker_CheckVaruse__defined_in_infrastructure(c *check.C) {
t := s.Init(c)
@@ -796,8 +891,10 @@ func (s *Suite) Test_MkLineChecker_Check
// FIXME: The check is called two times, even though it only produces a single NOTE.
t.CheckOutputLines(
- "NOTE: mk/compiler/gcc.mk:150: The modifier \":C/^/_asdf_/1:M_asdf_*:S/^_asdf_//\" can be written as \":[1]\".",
- "AUTOFIX: mk/compiler/gcc.mk:150: Replacing \":C/^/_asdf_/1:M_asdf_*:S/^_asdf_//\" with \":[1]\".",
+ "NOTE: mk/compiler/gcc.mk:150: "+
+ "The modifier \":C/^/_asdf_/1:M_asdf_*:S/^_asdf_//\" can be written as \":[1]\".",
+ "AUTOFIX: mk/compiler/gcc.mk:150: "+
+ "Replacing \":C/^/_asdf_/1:M_asdf_*:S/^_asdf_//\" with \":[1]\".",
"-\tCC:=\t${CC:C/^/_asdf_/1:M_asdf_*:S/^_asdf_//}",
"+\tCC:=\t${CC:[1]}")
}
@@ -823,7 +920,6 @@ func (s *Suite) Test_MkLineChecker_check
t.SetupPkgsrc()
G.Pkgsrc.LoadInfrastructure()
-
t.SetupCommandLine("-Wall,no-space")
t.SetupVartypes()
mklines := t.SetupFileMkLines("module.mk",
@@ -840,6 +936,7 @@ func (s *Suite) Test_MkLineChecker_check
mklines.Check()
+ // TODO: Split this test into several, one for each topic.
t.CheckOutputLines(
"WARN: ~/module.mk:2: Please use the RCD_SCRIPTS mechanism to install rc.d scripts automatically to ${RCD_SCRIPTS_EXAMPLEDIR}.",
"WARN: ~/module.mk:3: _TOOLS_VARNAME.sed is defined but not used.",
@@ -932,5 +1029,6 @@ func (s *Suite) Test_MkLineChecker_Check
"ERROR: ~/category/package/module.mk:2: A main pkgsrc package must not depend on a pkgsrc-wip package.",
"ERROR: ~/category/package/module.mk:3: A main pkgsrc package must not depend on a pkgsrc-wip package.",
"WARN: ~/category/package/module.mk:5: LATEST_PYTHON is used but not defined.",
+ // TODO: This warning is unspecific, there is also a pkglint warning "should be ../../category/package".
"WARN: ~/category/package/module.mk:11: Invalid relative path \"../package/module.mk\".")
}
Index: pkgsrc/pkgtools/pkglint/files/mklines.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines.go:1.35 pkgsrc/pkgtools/pkglint/files/mklines.go:1.36
--- pkgsrc/pkgtools/pkglint/files/mklines.go:1.35 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/mklines.go Sun Dec 2 23:12:43 2018
@@ -10,16 +10,19 @@ type MkLines = *MkLinesImpl
type MkLinesImpl struct {
mklines []MkLine
lines Lines
- forVars map[string]bool // The variables currently used in .for loops
- target string // Current make(1) target
- vars Scope
+ target string // Current make(1) target; only available during checkAll
+ vars Scope //
buildDefs map[string]bool // Variables that are registered in BUILD_DEFS, to ensure that all user-defined variables are added to it.
plistVarAdded map[string]MkLine // Identifiers that are added to PLIST_VARS.
plistVarSet map[string]MkLine // Identifiers for which PLIST.${id} is defined.
plistVarSkip bool // True if any of the PLIST_VARS identifiers refers to a variable.
Tools *Tools // Tools defined in file scope.
indentation *Indentation // Indentation depth of preprocessing directives; only available during MkLines.ForEach.
+ forVars map[string]bool // The variables currently used in .for loops; only available during MkLines.checkAll.
Once
+
+ // TODO: Consider extracting plistVarAdded, plistVarSet, plistVarSkip into an own type.
+ // TODO: Describe where each of the above fields is valid.
}
func NewMkLines(lines Lines) MkLines {
@@ -34,7 +37,6 @@ func NewMkLines(lines Lines) MkLines {
return &MkLinesImpl{
mklines,
lines,
- make(map[string]bool),
"",
NewScope(),
make(map[string]bool),
@@ -43,9 +45,35 @@ func NewMkLines(lines Lines) MkLines {
false,
tools,
nil,
+ make(map[string]bool),
Once{}}
}
+// TODO: Consider defining an interface MkLinesChecker (different name, though, since this one confuses even me)
+// that checks a single topic, like:
+//
+// * PlistVars
+// * ForLoops
+// * MakeTargets
+// * Tools
+// * Indentation
+// * LoadTimeVarUse
+// * Subst
+// * VarAlign
+//
+// These could be run in parallel to get the diagnostics strictly from top to bottom.
+// Some of the checkers will probably depend on one another.
+//
+// The driving code for these checkers could look like:
+//
+// ck.Init
+// ck.BeforeLine
+// ck.Line
+// ck.AfterLine
+// ck.Finish
+
+// UseVar remembers that the given variable is used in the given line.
+// This controls the "defined but not used" warning.
func (mklines *MkLinesImpl) UseVar(mkline MkLine, varname string) {
mklines.vars.Use(varname, mkline)
if G.Pkg != nil {
@@ -63,8 +91,8 @@ func (mklines *MkLinesImpl) Check() {
// In the first pass, all additions to BUILD_DEFS and USE_TOOLS
// are collected to make the order of the definitions irrelevant.
- mklines.DetermineUsedVariables()
- mklines.DetermineDefinedVariables()
+ mklines.collectUsedVariables()
+ mklines.collectDefinedVariables()
mklines.collectPlistVars()
mklines.collectElse()
@@ -75,17 +103,19 @@ func (mklines *MkLinesImpl) Check() {
}
func (mklines *MkLinesImpl) checkAll() {
- allowedTargets := func() map[string]bool {
- targets := make(map[string]bool)
- prefixes := [...]string{"pre", "do", "post"}
- actions := [...]string{"fetch", "extract", "patch", "tools", "wrapper", "configure", "build", "test", "install", "package", "clean"}
- for _, prefix := range prefixes {
- for _, action := range actions {
- targets[prefix+"-"+action] = true
- }
- }
- return targets
- }()
+ allowedTargets := map[string]bool{
+ "pre-fetch": true, "do-fetch": true, "post-fetch": true,
+ "pre-extract": true, "do-extract": true, "post-extract": true,
+ "pre-patch": true, "do-patch": true, "post-patch": true,
+ "pre-tools": true, "do-tools": true, "post-tools": true,
+ "pre-wrapper": true, "do-wrapper": true, "post-wrapper": true,
+ "pre-configure": true, "do-configure": true, "post-configure": true,
+ "pre-build": true, "do-build": true, "post-build": true,
+ "pre-test": true, "do-test": true, "post-test": true,
+ "pre-install": true, "do-install": true, "post-install": true,
+ "pre-package": true, "do-package": true, "post-package": true,
+ "pre-clean": true, "do-clean": true, "post-clean": true}
+ G.Assertf(len(allowedTargets) == 33, "Error in allowedTargets initialization")
mklines.lines.CheckRcsID(0, `#[\t ]+`, "# ")
@@ -95,12 +125,14 @@ func (mklines *MkLinesImpl) checkAll() {
lineAction := func(mkline MkLine) bool {
if isHacksMk {
+ // Needs to be set here because it is reset in MkLines.ForEach.
mklines.Tools.SeenPrefs = true
}
ck := MkLineChecker{mkline}
ck.Check()
- varalign.Check(mkline)
+
+ varalign.Process(mkline)
mklines.Tools.ParseToolLine(mkline, false, false)
switch {
@@ -112,26 +144,12 @@ func (mklines *MkLinesImpl) checkAll() {
mkline.Tokenize(mkline.Value(), true) // Just for the side-effect of the warnings.
substContext.Varassign(mkline)
- switch mkline.Varcanon() {
- case "PLIST_VARS":
- ids := mkline.ValueFields(resolveVariableRefs(mkline.Value()))
- for _, id := range ids {
- if !mklines.plistVarSkip && mklines.plistVarSet[id] == nil {
- mkline.Warnf("%q is added to PLIST_VARS, but PLIST.%s is not defined in this file.", id, id)
- }
- }
-
- case "PLIST.*":
- id := mkline.Varparam()
- if !mklines.plistVarSkip && mklines.plistVarAdded[id] == nil {
- mkline.Warnf("PLIST.%s is defined, but %q is not added to PLIST_VARS in this file.", id, id)
- }
- }
+ mklines.checkVarassignPlist(mkline)
case mkline.IsInclude():
mklines.target = ""
if G.Pkg != nil {
- G.Pkg.CheckInclude(mkline, mklines.indentation)
+ G.Pkg.checkIncludeConditionally(mkline, mklines.indentation)
}
case mkline.IsDirective():
@@ -165,6 +183,23 @@ func (mklines *MkLinesImpl) checkAll() {
ChecklinesTrailingEmptyLines(mklines.lines)
}
+func (mklines *MkLinesImpl) checkVarassignPlist(mkline MkLine) {
+ switch mkline.Varcanon() {
+ case "PLIST_VARS":
+ for _, id := range mkline.ValueFields(resolveVariableRefs(mkline.Value())) {
+ if !mklines.plistVarSkip && mklines.plistVarSet[id] == nil {
+ mkline.Warnf("%q is added to PLIST_VARS, but PLIST.%s is not defined in this file.", id, id)
+ }
+ }
+
+ case "PLIST.*":
+ id := mkline.Varparam()
+ if !mklines.plistVarSkip && mklines.plistVarAdded[id] == nil {
+ mkline.Warnf("PLIST.%s is defined, but %q is not added to PLIST_VARS in this file.", id, id)
+ }
+ }
+}
+
// ForEach calls the action for each line, until the action returns false.
// It keeps track of the indentation (see MkLines.indentation)
// and all conditional variables (see Indentation.IsConditional).
@@ -199,7 +234,7 @@ func (mklines *MkLinesImpl) ForEachEnd(a
mklines.indentation = nil
}
-func (mklines *MkLinesImpl) DetermineDefinedVariables() {
+func (mklines *MkLinesImpl) collectDefinedVariables() {
if trace.Tracing {
defer trace.Call0()()
}
@@ -217,9 +252,9 @@ func (mklines *MkLinesImpl) DetermineDef
switch varcanon {
case
"BUILD_DEFS",
- "PKG_GROUPS_VARS",
- "PKG_USERS_VARS":
- for _, varname := range fields(mkline.Value()) {
+ "PKG_GROUPS_VARS", // see mk/misc/unprivileged.mk
+ "PKG_USERS_VARS": // see mk/misc/unprivileged.mk
+ for _, varname := range mkline.Fields() {
mklines.buildDefs[varname] = true
if trace.Tracing {
trace.Step1("%q is added to BUILD_DEFS.", varname)
@@ -229,16 +264,16 @@ func (mklines *MkLinesImpl) DetermineDef
case
"BUILTIN_FIND_FILES_VAR",
"BUILTIN_FIND_HEADERS_VAR":
- for _, varname := range fields(mkline.Value()) {
+ for _, varname := range mkline.Fields() {
mklines.vars.Define(varname, mkline)
}
case "PLIST_VARS":
- ids := mkline.ValueFields(resolveVariableRefs(mkline.Value()))
- for _, id := range ids {
+ for _, id := range mkline.ValueFields(resolveVariableRefs(mkline.Value())) {
if trace.Tracing {
trace.Step1("PLIST.%s is added to PLIST_VARS.", id)
}
+
if containsVarRef(id) {
mklines.UseVar(mkline, "PLIST.*")
mklines.plistVarSkip = true
@@ -248,29 +283,29 @@ func (mklines *MkLinesImpl) DetermineDef
}
case "SUBST_VARS.*":
- for _, svar := range fields(mkline.Value()) {
- mklines.UseVar(mkline, varnameCanon(svar))
+ for _, substVar := range mkline.Fields() {
+ mklines.UseVar(mkline, varnameCanon(substVar))
if trace.Tracing {
- trace.Step1("varuse %s", svar)
+ trace.Step1("varuse %s", substVar)
}
}
case "OPSYSVARS":
- for _, osvar := range fields(mkline.Value()) {
- mklines.UseVar(mkline, osvar+".*")
- defineVar(mkline, osvar)
+ for _, opsysVar := range mkline.Fields() {
+ mklines.UseVar(mkline, opsysVar+".*")
+ defineVar(mkline, opsysVar)
}
}
}
}
func (mklines *MkLinesImpl) collectPlistVars() {
+ // TODO: The PLIST_VARS code above looks very similar.
for _, mkline := range mklines.mklines {
if mkline.IsVarassign() {
switch mkline.Varcanon() {
case "PLIST_VARS":
- ids := mkline.ValueFields(resolveVariableRefs(mkline.Value()))
- for _, id := range ids {
+ for _, id := range mkline.ValueFields(resolveVariableRefs(mkline.Value())) {
if containsVarRef(id) {
mklines.plistVarSkip = true
} else {
@@ -292,20 +327,24 @@ func (mklines *MkLinesImpl) collectPlist
func (mklines *MkLinesImpl) collectElse() {
// Make a dry-run over the lines, which sets data.elseLine (in mkline.go) as a side-effect.
mklines.ForEach(func(mkline MkLine) {})
+ // TODO: Check whether this ForEach is redundant because it is already run somewhere else.
}
-func (mklines *MkLinesImpl) DetermineUsedVariables() {
+func (mklines *MkLinesImpl) collectUsedVariables() {
for _, mkline := range mklines.mklines {
for _, varname := range mkline.DetermineUsedVariables() {
mklines.UseVar(mkline, varname)
}
}
- mklines.determineDocumentedVariables()
+ mklines.collectDocumentedVariables()
}
-// Loosely based on mk/help/help.awk, revision 1.28
-func (mklines *MkLinesImpl) determineDocumentedVariables() {
+// collectDocumentedVariables collects the variables that are mentioned in the human-readable
+// documentation of the Makefile fragments from the pkgsrc infrastructure.
+//
+// Loosely based on mk/help/help.awk, revision 1.28, but much simpler.
+func (mklines *MkLinesImpl) collectDocumentedVariables() {
scope := NewScope()
commentLines := 0
relevant := true
@@ -357,8 +396,9 @@ func (mklines *MkLinesImpl) determineDoc
finish()
}
-func (mklines *MkLinesImpl) CheckRedundantVariables() {
+func (mklines *MkLinesImpl) CheckRedundantAssignments() {
scope := NewRedundantScope()
+
isRelevant := func(old, new MkLine) bool {
if old.Basename != "Makefile" && new.Basename == "Makefile" {
return false
@@ -368,11 +408,13 @@ func (mklines *MkLinesImpl) CheckRedunda
}
return true
}
+
scope.OnIgnore = func(old, new MkLine) {
if isRelevant(old, new) && old.Value() == new.Value() {
old.Notef("Definition of %s is redundant because of %s.", new.Varname(), old.RefTo(new))
}
}
+
scope.OnOverwrite = func(old, new MkLine) {
if isRelevant(old, new) {
old.Warnf("Variable %s is overwritten in %s.", new.Varname(), old.RefTo(new))
@@ -448,7 +490,7 @@ type varalignBlockInfo struct {
continuation bool // A continuation line with no value in the first line.
}
-func (va *VaralignBlock) Check(mkline MkLine) {
+func (va *VaralignBlock) Process(mkline MkLine) {
switch {
case !G.Opts.WarnSpace:
return
@@ -457,21 +499,20 @@ func (va *VaralignBlock) Check(mkline Mk
va.Finish()
return
- case mkline.IsCommentedVarassign():
- break
+ case mkline.IsVarassign(), mkline.IsCommentedVarassign():
+ va.processVarassign(mkline)
- case mkline.IsComment():
+ case mkline.IsComment(), mkline.IsDirective():
return
- case mkline.IsDirective():
- return
-
- case !mkline.IsVarassign():
+ default:
trace.Stepf("Skipping")
va.skip = true
return
}
+}
+func (va *VaralignBlock) processVarassign(mkline MkLine) {
switch {
case mkline.Op() == opAssignEval && matches(mkline.Varname(), `^[a-z]`):
// Arguments to procedures do not take part in block alignment.
@@ -494,8 +535,7 @@ func (va *VaralignBlock) Check(mkline Mk
continuation := false
if mkline.IsMultiline() {
- // Interpreting the continuation marker as variable value
- // is cheating but works well.
+ // Parsing the continuation marker as variable value is cheating but works well.
text := strings.TrimSuffix(mkline.raw[0].orignl, "\n")
m, _, _, _, _, _, value, _, _ := MatchVarassign(text)
continuation = m && value == "\\"
@@ -581,14 +621,14 @@ func (va *VaralignBlock) optimalWidth(in
if trace.Tracing {
trace.Stepf("Indentation including whitespace is between %d and %d.",
minTotalWidth, maxTotalWidth)
- trace.Stepf("Minimum required indentation is %d + 1.",
- minVarnameOpWidth)
+ trace.Stepf("Minimum required indentation is %d + 1.", minVarnameOpWidth)
if outlier != 0 {
trace.Stepf("The outlier is at indentation %d.", outlier)
}
}
if minTotalWidth > minVarnameOpWidth && minTotalWidth == maxTotalWidth && minTotalWidth%8 == 0 {
+ // The whole paragraph is already indented to the same width.
return minTotalWidth
}
@@ -643,15 +683,17 @@ func (va *VaralignBlock) realignInitialL
if wrongColumn {
fix.Explain(
- "Normally, all variable values in a block should start at the same",
- "column. There are some exceptions to this rule:",
+ "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 using tabs.",
+ "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 using tabs.",
"",
- "Variable definitions that span multiple lines are not checked for",
- "alignment at all.",
+ "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.")
Index: pkgsrc/pkgtools/pkglint/files/mklines_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.31 pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.32
--- pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.31 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/mklines_test.go Sun Dec 2 23:12:43 2018
@@ -40,6 +40,8 @@ func (s *Suite) Test_MkLines_Check__auto
func (s *Suite) Test_MkLines_Check__unusual_target(c *check.C) {
t := s.Init(c)
+ t.SetupVartypes()
+ t.SetupTool("cc", "CC", AtRunTime)
mklines := t.NewMkLines("Makefile",
MkRcsID,
"",
@@ -48,13 +50,8 @@ func (s *Suite) Test_MkLines_Check__unus
mklines.Check()
- // FIXME: .TARGET is always defined.
- // FIXME: .IMPSRC is always defined.
t.CheckOutputLines(
- "WARN: Makefile:3: Unusual target \"echo\".",
- "WARN: Makefile:4: Unknown shell command \"cc\".",
- "WARN: Makefile:4: .TARGET is used but not defined.",
- "WARN: Makefile:4: .IMPSRC is used but not defined.")
+ "WARN: Makefile:3: Unusual target \"echo\".")
}
func (s *Suite) Test_MkLines__quoting_LDFLAGS_for_GNU_configure(c *check.C) {
@@ -84,6 +81,8 @@ func (s *Suite) Test_MkLines__for_loop_m
mklines := t.NewMkLines("Makefile", // From audio/squeezeboxserver
MkRcsID,
"",
+ "SBS_COPY=\tsource target",
+ "",
".for _list_ _dir_ in ${SBS_COPY}",
"\tcd ${WRKSRC} && ${FIND} ${${_list_}} -type f ! -name '*.orig' 2>/dev/null "+
"| pax -rw -pm ${DESTDIR}${PREFIX}/${${_dir_}}",
@@ -92,10 +91,11 @@ func (s *Suite) Test_MkLines__for_loop_m
mklines.Check()
t.CheckOutputLines(
- "WARN: Makefile:3: Variable names starting with an underscore (_list_) are reserved for internal pkgsrc use.",
- "WARN: Makefile:3: Variable names starting with an underscore (_dir_) are reserved for internal pkgsrc use.",
- "WARN: Makefile:3: SBS_COPY is used but not defined.",
- "WARN: Makefile:4: The exitcode of \"${FIND}\" at the left of the | operator is ignored.")
+ "WARN: Makefile:5: Variable names starting with an underscore (_list_) "+
+ "are reserved for internal pkgsrc use.",
+ "WARN: Makefile:5: Variable names starting with an underscore (_dir_) "+
+ "are reserved for internal pkgsrc use.",
+ "WARN: Makefile:6: The exitcode of \"${FIND}\" at the left of the | operator is ignored.")
}
func (s *Suite) Test_MkLines__comparing_YesNo_variable_to_string(c *check.C) {
@@ -113,7 +113,8 @@ func (s *Suite) Test_MkLines__comparing_
t.CheckOutputLines(
"WARN: databases/gdbm_compat/builtin.mk:2: " +
- "USE_BUILTIN.gdbm should be matched against \"[yY][eE][sS]\" or \"[nN][oO]\", not compared with \"no\".")
+ "USE_BUILTIN.gdbm should be matched against \"[yY][eE][sS]\" or \"[nN][oO]\", " +
+ "not compared with \"no\".")
}
func (s *Suite) Test_MkLines__varuse_sh_modifier(c *check.C) {
@@ -132,6 +133,7 @@ func (s *Suite) Test_MkLines__varuse_sh_
vars3 := mklines.mklines[2].DetermineUsedVariables()
+ // qore-version, despite its unusual name, is a pretty normal Make variable.
c.Check(vars3, deepEquals, []string{"qore-version"})
mklines.Check()
@@ -161,10 +163,27 @@ func (s *Suite) Test_MkLines__varuse_par
"WARN: converters/wv2/Makefile:2: ICONV_TYPE is used but not defined.")
}
-// Even very complicated shell commands are parsed correctly.
-// Since the variable is defined in this same Makefile, it is
-// assumed to be a known shell command and therefore does not need
-// USE_TOOLS or a similar declaration.
+// When an ODE runtime loop is used to expand variables to shell commands,
+// pkglint only understands that there is a variable that is executed as
+// shell command.
+//
+// In this example, GCONF_SCHEMAS is a list of filenames, but pkglint doesn't know this
+// because there is no built-in rule saying *_SCHEMAS are filenames.
+// If the variable name had been GCONF_SCHEMA_FILES, pkglint would know.
+//
+// As of November 2018, pkglint sees GCONF_SCHEMAS as being the shell command.
+// It doesn't expand the @s@ loop to see what really happens.
+//
+// If it did that, it could notice that GCONF_SCHEMAS expands to a single shell command,
+// and in that command INSTALL_DATA is used as the command for the first time,
+// and as a regular command line argument in all other times.
+// This combination is strange enough to warrant a warning.
+//
+// The bug here is the missing semicolon just before the @}.
+//
+// Pkglint could offer to either add the missing semicolon.
+// Or, if it knows what INSTALL_DATA does, it could simply say that INSTALL_DATA
+// can handle multiple files in a single invocation.
func (s *Suite) Test_MkLines__loop_modifier(c *check.C) {
t := s.Init(c)
@@ -173,8 +192,8 @@ func (s *Suite) Test_MkLines__loop_modif
MkRcsID,
"GCONF_SCHEMAS=\tapps_xchat_url_handler.schemas",
"post-install:",
- "\t${GCONF_SCHEMAS:@.s.@"+
- "${INSTALL_DATA} ${WRKSRC}/src/common/dbus/${.s.} ${DESTDIR}${GCONF_SCHEMAS_DIR}/@}")
+ "\t${GCONF_SCHEMAS:@s@"+
+ "${INSTALL_DATA} ${WRKSRC}/src/common/dbus/${s} ${DESTDIR}${GCONF_SCHEMAS_DIR}/@}")
mklines.Check()
@@ -182,7 +201,6 @@ func (s *Suite) Test_MkLines__loop_modif
t.CheckOutputEmpty()
}
-// PR 46570
func (s *Suite) Test_MkLines__PKG_SKIP_REASON_depending_on_OPSYS(c *check.C) {
t := s.Init(c)
@@ -197,7 +215,7 @@ func (s *Suite) Test_MkLines__PKG_SKIP_R
mklines.Check()
t.CheckOutputLines(
- "NOTE: Makefile:4: Consider defining NOT_FOR_PLATFORM instead of setting PKG_SKIP_REASON depending on ${OPSYS}.")
+ "NOTE: Makefile:4: Consider setting NOT_FOR_PLATFORM instead of PKG_SKIP_REASON depending on ${OPSYS}.")
}
// PR 46570, item "15. net/uucp/Makefile has a make loop"
@@ -311,7 +329,7 @@ func (s *Suite) Test_MkLines_CheckForUse
c.Check(G.autofixAvailable, equals, true)
}
-func (s *Suite) Test_MkLines_DetermineDefinedVariables(c *check.C) {
+func (s *Suite) Test_MkLines_collectDefinedVariables(c *check.C) {
t := s.Init(c)
t.SetupCommandLine("-Wall,no-space")
@@ -350,7 +368,7 @@ func (s *Suite) Test_MkLines_DetermineDe
"WARN: determine-defined-variables.mk:16: Unknown shell command \"unknown-command\".")
}
-func (s *Suite) Test_MkLines_DetermineDefinedVariables__BUILTIN_FIND_FILES_VAR(c *check.C) {
+func (s *Suite) Test_MkLines_collectDefinedVariables__BUILTIN_FIND_FILES_VAR(c *check.C) {
t := s.Init(c)
t.SetupCommandLine("-Wall,no-space")
@@ -375,7 +393,7 @@ func (s *Suite) Test_MkLines_DetermineDe
"WARN: ~/category/package/builtin.mk:8: H_UNDEF is used but not defined.")
}
-func (s *Suite) Test_MkLines_DetermineUsedVariables__simple(c *check.C) {
+func (s *Suite) Test_MkLines_collectUsedVariables__simple(c *check.C) {
t := s.Init(c)
mklines := t.NewMkLines("filename",
@@ -383,13 +401,13 @@ func (s *Suite) Test_MkLines_DetermineUs
mkline := mklines.mklines[0]
G.Mk = mklines
- mklines.DetermineUsedVariables()
+ mklines.collectUsedVariables()
c.Check(len(mklines.vars.used), equals, 1)
c.Check(mklines.vars.FirstUse("VAR"), equals, mkline)
}
-func (s *Suite) Test_MkLines_DetermineUsedVariables__nested(c *check.C) {
+func (s *Suite) Test_MkLines_collectUsedVariables__nested(c *check.C) {
t := s.Init(c)
mklines := t.NewMkLines("filename.mk",
@@ -403,7 +421,7 @@ func (s *Suite) Test_MkLines_DetermineUs
shellMkline := mklines.mklines[5]
G.Mk = mklines
- mklines.DetermineUsedVariables()
+ mklines.collectUsedVariables()
c.Check(len(mklines.vars.used), equals, 5)
c.Check(mklines.vars.FirstUse("lparam"), equals, assignMkline)
@@ -638,7 +656,7 @@ func (s *Suite) Test_MkLines__wip_catego
"")
}
-func (s *Suite) Test_MkLines_determineDocumentedVariables(c *check.C) {
+func (s *Suite) Test_MkLines_collectDocumentedVariables(c *check.C) {
t := s.Init(c)
t.SetupVartypes()
@@ -670,7 +688,7 @@ func (s *Suite) Test_MkLines_determineDo
// The variables that appear in the documentation are marked as
// used, to prevent the "defined but not used" warnings.
- mklines.determineDocumentedVariables()
+ mklines.collectDocumentedVariables()
var varnames []string
for varname, mkline := range mklines.vars.used {
@@ -725,7 +743,7 @@ func (s *Suite) Test_MkLines__unknown_op
"WARN: options.mk:4: Unknown option \"unknown\".")
}
-func (s *Suite) Test_MkLines_CheckRedundantVariables(c *check.C) {
+func (s *Suite) Test_MkLines_CheckRedundantAssignments(c *check.C) {
t := s.Init(c)
included := t.NewMkLines("module.mk",
"VAR=\tvalue ${OTHER}",
@@ -738,68 +756,68 @@ func (s *Suite) Test_MkLines_CheckRedund
// XXX: The warnings from here are not in the same order as the other warnings.
// XXX: There may be some warnings for the same file separated by warnings for other files.
- mklines.CheckRedundantVariables()
+ mklines.CheckRedundantAssignments()
t.CheckOutputLines(
"NOTE: module.mk:1: Definition of VAR is redundant because of line 2.",
"WARN: module.mk:1: Variable VAR is overwritten in line 3.")
}
-func (s *Suite) Test_MkLines_CheckRedundantVariables__different_value(c *check.C) {
+func (s *Suite) Test_MkLines_CheckRedundantAssignments__different_value(c *check.C) {
t := s.Init(c)
mklines := t.NewMkLines("module.mk",
"VAR=\tvalue ${OTHER}",
"VAR?=\tdifferent value")
- mklines.CheckRedundantVariables()
+ mklines.CheckRedundantAssignments()
t.CheckOutputEmpty()
}
-func (s *Suite) Test_MkLines_CheckRedundantVariables__overwrite_same_value(c *check.C) {
+func (s *Suite) Test_MkLines_CheckRedundantAssignments__overwrite_same_value(c *check.C) {
t := s.Init(c)
mklines := t.NewMkLines("module.mk",
"VAR=\tvalue ${OTHER}",
"VAR=\tvalue ${OTHER}")
- mklines.CheckRedundantVariables()
+ mklines.CheckRedundantAssignments()
t.CheckOutputLines(
"NOTE: module.mk:1: Definition of VAR is redundant because of line 2.")
}
-func (s *Suite) Test_MkLines_CheckRedundantVariables__procedure_call(c *check.C) {
+func (s *Suite) Test_MkLines_CheckRedundantAssignments__procedure_call(c *check.C) {
t := s.Init(c)
mklines := t.NewMkLines("mk/pthread.buildlink3.mk",
"CHECK_BUILTIN.pthread:=\tyes",
".include \"../../mk/pthread.builtin.mk\"",
"CHECK_BUILTIN.pthread:=\tno")
- mklines.CheckRedundantVariables()
+ mklines.CheckRedundantAssignments()
t.CheckOutputEmpty()
}
-func (s *Suite) Test_MkLines_CheckRedundantVariables__shell_and_eval(c *check.C) {
+func (s *Suite) Test_MkLines_CheckRedundantAssignments__shell_and_eval(c *check.C) {
t := s.Init(c)
mklines := t.NewMkLines("module.mk",
"VAR:=\tvalue ${OTHER}",
"VAR!=\tvalue ${OTHER}")
- mklines.CheckRedundantVariables()
+ mklines.CheckRedundantAssignments()
// Combining := and != is too complicated to be analyzed by pkglint,
// therefore no warning.
t.CheckOutputEmpty()
}
-func (s *Suite) Test_MkLines_CheckRedundantVariables__shell_and_eval_literal(c *check.C) {
+func (s *Suite) Test_MkLines_CheckRedundantAssignments__shell_and_eval_literal(c *check.C) {
t := s.Init(c)
mklines := t.NewMkLines("module.mk",
"VAR:=\tvalue",
"VAR!=\tvalue")
- mklines.CheckRedundantVariables()
+ mklines.CheckRedundantAssignments()
// Even when := is used with a literal value (which is usually
// only done for procedure calls), the shell evaluation can have
@@ -816,8 +834,9 @@ func (s *Suite) Test_MkLines_Check__PLIS
t.SetupOption("both", "")
t.SetupOption("only-added", "")
t.SetupOption("only-defined", "")
+ t.CreateFileLines("mk/bsd.options.mk")
- mklines := t.SetupFileMkLines("options.mk",
+ mklines := t.SetupFileMkLines("category/package/options.mk",
MkRcsID,
"",
"PKG_OPTIONS_VAR= PKG_OPTIONS.pkg",
@@ -839,9 +858,8 @@ func (s *Suite) Test_MkLines_Check__PLIS
mklines.Check()
t.CheckOutputLines(
- "ERROR: ~/options.mk:7: \"mk/bsd.options.mk\" does not exist.", // Not relevant for this test.
- "WARN: ~/options.mk:9: \"only-added\" is added to PLIST_VARS, but PLIST.only-added is not defined in this file.",
- "WARN: ~/options.mk:16: PLIST.only-defined is defined, but \"only-defined\" is not added to PLIST_VARS in this file.")
+ "WARN: ~/category/package/options.mk:9: \"only-added\" is added to PLIST_VARS, but PLIST.only-added is not defined in this file.",
+ "WARN: ~/category/package/options.mk:16: PLIST.only-defined is defined, but \"only-defined\" is not added to PLIST_VARS in this file.")
}
func (s *Suite) Test_MkLines_Check__PLIST_VARS_indirect(c *check.C) {
@@ -874,7 +892,7 @@ func (s *Suite) Test_MkLines_Check__PLIS
t.CheckOutputEmpty()
}
-func (s *Suite) Test_MkLines_Check__if_else(c *check.C) {
+func (s *Suite) Test_MkLines_collectElse(c *check.C) {
t := s.Init(c)
t.SetupCommandLine("-Wno-space")
@@ -1090,7 +1108,28 @@ func (s *Suite) Test_MkLines_ForEach__co
c.Check(seenUsesGettext, equals, true)
}
-func (s *Suite) Test_VaralignBlock_Check__autofix(c *check.C) {
+// At 2018-12-02, pkglint had resolved ${MY_PLIST_VARS} into a single word,
+// whereas the correct behavior is to resolve it into two words.
+// It had produced warnings about mismatched PLIST_VARS IDs.
+func (s *Suite) Test_MkLines_checkVarassignPlist__indirect(c *check.C) {
+ t := s.Init(c)
+
+ t.SetupVartypes()
+ mklines := t.SetupFileMkLines("plist.mk",
+ MkRcsID,
+ "",
+ "MY_PLIST_VARS=\tvar1 var2",
+ "PLIST_VARS+=\t${MY_PLIST_VARS}",
+ "",
+ "PLIST.var1=\tyes",
+ "PLIST.var2=\tyes")
+
+ mklines.Check()
+
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_VaralignBlock_Process__autofix(c *check.C) {
t := s.Init(c)
t.SetupCommandLine("-Wspace", "--show-autofix")
@@ -1112,7 +1151,7 @@ func (s *Suite) Test_VaralignBlock_Check
var varalign VaralignBlock
for _, line := range mklines.mklines {
- varalign.Check(line)
+ varalign.Process(line)
}
varalign.Finish()
@@ -1133,7 +1172,7 @@ func (s *Suite) Test_VaralignBlock_Check
// When the lines of a paragraph are inconsistently aligned,
// they are realigned to the minimum required width.
-func (s *Suite) Test_VaralignBlock_Check__reduce_indentation(c *check.C) {
+func (s *Suite) Test_VaralignBlock_Process__reduce_indentation(c *check.C) {
t := s.Init(c)
mklines := t.NewMkLines("file.mk",
@@ -1147,7 +1186,7 @@ func (s *Suite) Test_VaralignBlock_Check
var varalign VaralignBlock
for _, mkline := range mklines.mklines {
- varalign.Check(mkline)
+ varalign.Process(mkline)
}
varalign.Finish()
@@ -1160,7 +1199,7 @@ func (s *Suite) Test_VaralignBlock_Check
// For every variable assignment, there is at least one space or tab between the variable
// name and the value. Even if it is the longest line, and even if the value would start
// exactly at a tab stop.
-func (s *Suite) Test_VaralignBlock_Check__longest_line_no_space(c *check.C) {
+func (s *Suite) Test_VaralignBlock_Process__longest_line_no_space(c *check.C) {
t := s.Init(c)
t.SetupCommandLine("-Wspace")
@@ -1172,7 +1211,7 @@ func (s *Suite) Test_VaralignBlock_Check
var varalign VaralignBlock
for _, mkline := range mklines.mklines {
- varalign.Check(mkline)
+ varalign.Process(mkline)
}
varalign.Finish()
@@ -1183,7 +1222,7 @@ func (s *Suite) Test_VaralignBlock_Check
"NOTE: file.mk:4: This variable value should be aligned to column 33.")
}
-func (s *Suite) Test_VaralignBlock_Check__only_spaces(c *check.C) {
+func (s *Suite) Test_VaralignBlock_Process__only_spaces(c *check.C) {
t := s.Init(c)
t.SetupCommandLine("-Wspace")
@@ -1195,7 +1234,7 @@ func (s *Suite) Test_VaralignBlock_Check
var varalign VaralignBlock
for _, mkline := range mklines.mklines {
- varalign.Check(mkline)
+ varalign.Process(mkline)
}
varalign.Finish()
Index: pkgsrc/pkgtools/pkglint/files/mklines_varalign_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines_varalign_test.go:1.5 pkgsrc/pkgtools/pkglint/files/mklines_varalign_test.go:1.6
--- pkgsrc/pkgtools/pkglint/files/mklines_varalign_test.go:1.5 Wed Nov 7 20:58:23 2018
+++ pkgsrc/pkgtools/pkglint/files/mklines_varalign_test.go Sun Dec 2 23:12:43 2018
@@ -61,7 +61,7 @@ func (vt *VaralignTester) run(autofix bo
var varalign VaralignBlock
for _, mkline := range mklines.mklines {
- varalign.Check(mkline)
+ varalign.Process(mkline)
}
varalign.Finish()
Index: pkgsrc/pkgtools/pkglint/files/package.go
diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.39 pkgsrc/pkgtools/pkglint/files/package.go:1.40
--- pkgsrc/pkgtools/pkglint/files/package.go:1.39 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/package.go Sun Dec 2 23:12:43 2018
@@ -160,8 +160,8 @@ func (pkg *Package) loadPackageMakefile(
mainLines.Tools.def("cpack", "", false, AtRunTime)
}
- allLines.DetermineUsedVariables()
- allLines.CheckRedundantVariables()
+ allLines.collectUsedVariables()
+ allLines.CheckRedundantAssignments()
pkg.Pkgdir, _ = pkg.vars.Value("PKGDIR")
pkg.DistinfoFile, _ = pkg.vars.Value("DISTINFO_FILE")
@@ -217,7 +217,7 @@ func (pkg *Package) readMakefile(filenam
var includedFile, incDir, incBase string
if mkline.IsInclude() {
- includedFile = resolveVariableRefs(mkline.ResolveVarsInRelativePath(mkline.IncludedFile(), true))
+ includedFile = resolveVariableRefs(mkline.ResolveVarsInRelativePath(mkline.IncludedFile()))
if containsVarRef(includedFile) {
if !contains(filename, "/mk/") {
mkline.Notef("Skipping include file %q. This may result in false warnings.", includedFile)
@@ -753,7 +753,7 @@ func (pkg *Package) checkLocallyModified
}
}
-func (pkg *Package) CheckInclude(mkline MkLine, indentation *Indentation) {
+func (pkg *Package) checkIncludeConditionally(mkline MkLine, indentation *Indentation) {
conditionalVars := mkline.ConditionalVars()
if len(conditionalVars) == 0 {
conditionalVars = indentation.Varnames()
Index: pkgsrc/pkgtools/pkglint/files/package_test.go
diff -u pkgsrc/pkgtools/pkglint/files/package_test.go:1.33 pkgsrc/pkgtools/pkglint/files/package_test.go:1.34
--- pkgsrc/pkgtools/pkglint/files/package_test.go:1.33 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/package_test.go Sun Dec 2 23:12:43 2018
@@ -531,7 +531,7 @@ func (s *Suite) Test_Package_loadPackage
G.CheckDirent(pkg)
}
-func (s *Suite) Test_Package_CheckInclude__conditional_and_unconditional_include(c *check.C) {
+func (s *Suite) Test_Package_checkIncludeConditionally__conditional_and_unconditional_include(c *check.C) {
t := s.Init(c)
t.SetupVartypes()
@@ -618,7 +618,7 @@ func (s *Suite) Test_Package__include_af
"WARN: distinfo: File not found. Please run \""+confMake+" makesum\" or define NO_CHECKSUM=yes in the package Makefile.",
"ERROR: Makefile: Each package must define its LICENSE.",
"WARN: Makefile: No COMMENT given.",
- "ERROR: Makefile:4: \"options.mk\" does not exist.")
+ "ERROR: Makefile:4: Relative path \"options.mk\" does not exist.")
}
// See https://github.com/rillig/pkglint/issues/1
Index: pkgsrc/pkgtools/pkglint/files/substcontext.go
diff -u pkgsrc/pkgtools/pkglint/files/substcontext.go:1.16 pkgsrc/pkgtools/pkglint/files/substcontext.go:1.17
--- pkgsrc/pkgtools/pkglint/files/substcontext.go:1.16 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/substcontext.go Sun Dec 2 23:12:43 2018
@@ -157,7 +157,7 @@ func (ctx *SubstContext) Varassign(mklin
case "SUBST_VARS.*":
ctx.dupBool(mkline, &ctx.curr.seenVars, varname, op, value)
ctx.curr.seenTransform = true
- for _, substVar := range mkline.ValueFields(value) {
+ for _, substVar := range mkline.Fields() {
if ctx.vars == nil {
ctx.vars = make(map[string]bool)
}
Index: pkgsrc/pkgtools/pkglint/files/tools_test.go
diff -u pkgsrc/pkgtools/pkglint/files/tools_test.go:1.8 pkgsrc/pkgtools/pkglint/files/tools_test.go:1.9
--- pkgsrc/pkgtools/pkglint/files/tools_test.go:1.8 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/tools_test.go Sun Dec 2 23:12:43 2018
@@ -255,7 +255,7 @@ func (s *Suite) Test_Tools__builtin_mk(c
// Tools that are defined by pkgsrc as load-time tools
// may be used in any file at load time.
- mklines := t.SetupFileMkLines("builtin.mk",
+ mklines := t.SetupFileMkLines("category/package/builtin.mk",
MkRcsID,
"",
"VAR!= ${ECHO} 'too early'",
@@ -275,12 +275,12 @@ func (s *Suite) Test_Tools__builtin_mk(c
mklines.Check()
t.CheckOutputLines(
- "WARN: ~/builtin.mk:3: To use the tool ${ECHO} at load time, bsd.prefs.mk has to be included before.",
- "WARN: ~/builtin.mk:4: To use the tool ${LOAD} at load time, bsd.prefs.mk has to be included before.",
- "WARN: ~/builtin.mk:5: The tool ${RUN_CMD} cannot be used at load time.",
- "WARN: ~/builtin.mk:6: The tool ${NOWHERE} cannot be used at load time.",
- "WARN: ~/builtin.mk:12: The tool ${RUN_CMD} cannot be used at load time.",
- "WARN: ~/builtin.mk:13: The tool ${NOWHERE} cannot be used at load time.")
+ "WARN: ~/category/package/builtin.mk:3: To use the tool ${ECHO} at load time, bsd.prefs.mk has to be included before.",
+ "WARN: ~/category/package/builtin.mk:4: To use the tool ${LOAD} at load time, bsd.prefs.mk has to be included before.",
+ "WARN: ~/category/package/builtin.mk:5: The tool ${RUN_CMD} cannot be used at load time.",
+ "WARN: ~/category/package/builtin.mk:6: The tool ${NOWHERE} cannot be used at load time.",
+ "WARN: ~/category/package/builtin.mk:12: The tool ${RUN_CMD} cannot be used at load time.",
+ "WARN: ~/category/package/builtin.mk:13: The tool ${NOWHERE} cannot be used at load time.")
}
func (s *Suite) Test_Tools__implicit_definition_in_bsd_pkg_mk(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/vardefs.go
diff -u pkgsrc/pkgtools/pkglint/files/vardefs.go:1.50 pkgsrc/pkgtools/pkglint/files/vardefs.go:1.51
--- pkgsrc/pkgtools/pkglint/files/vardefs.go:1.50 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/vardefs.go Sun Dec 2 23:12:43 2018
@@ -141,7 +141,7 @@ func (src *Pkgsrc) InitVartypes() {
if len(values) > 0 {
joined := keysJoined(values)
if trace.Tracing {
- trace.Stepf("Enum from %s in: %s", strings.Join(varcanons, " "), filename, joined)
+ trace.Stepf("Enum from %s in %s with values: %s", strings.Join(varcanons, " "), filename, joined)
}
return enum(joined)
}
@@ -487,6 +487,7 @@ func (src *Pkgsrc) InitVartypes() {
// some other variables, sorted alphabetically
acl(".CURDIR", lkNone, BtPathname, "buildlink3.mk:; *: use, use-loadtime")
+ acl(".IMPSRC", lkShell, BtPathname, "buildlink3.mk:; *: use, use-loadtime")
acl(".TARGET", lkNone, BtPathname, "buildlink3.mk:; *: use, use-loadtime")
acl("@", lkNone, BtPathname, "buildlink3.mk:; *: use, use-loadtime")
acl("ALL_ENV", lkShell, BtShellWord, "")
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.36 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.37
--- pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.36 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go Sun Dec 2 23:12:43 2018
@@ -216,7 +216,7 @@ func (s *Suite) Test_VartypeCheck_Depend
vt.Output(
"WARN: ~/category/package/filename.mk:1: Invalid dependency pattern with path \"Perl\".",
"WARN: ~/category/package/filename.mk:2: Dependencies should have the form \"../../category/package\".",
- "ERROR: ~/category/package/filename.mk:3: \"../../lang/perl5\" does not exist.",
+ "ERROR: ~/category/package/filename.mk:3: Relative path \"../../lang/perl5\" does not exist.",
"ERROR: ~/category/package/filename.mk:3: There is no package in \"lang/perl5\".",
"WARN: ~/category/package/filename.mk:3: Please use USE_TOOLS+=perl:run instead of this dependency.",
"WARN: ~/category/package/filename.mk:4: Invalid dependency pattern \"broken0.12.1\".",
@@ -305,7 +305,7 @@ func (s *Suite) Test_VartypeCheck_Enum__
mklines.Check()
t.CheckOutputLines(
- "NOTE: module.mk:3: MACHINE_ARCH should be compared using == instead of the :M or :N modifier without wildcards.",
+ "NOTE: module.mk:3: MACHINE_ARCH should be compared using == instead of matching against \":Mi386\".",
"WARN: module.mk:5: Use ${PKGSRC_COMPILER:Mclang} instead of the == operator.")
}
@@ -524,7 +524,7 @@ func (s *Suite) Test_VartypeCheck_Licens
G.Mk = t.NewMkLines("perl5.mk",
MkRcsID,
"PERL5_LICENSE= gnu-gpl-v2 OR artistic")
- G.Mk.DetermineDefinedVariables()
+ G.Mk.collectDefinedVariables()
vt := NewVartypeCheckTester(t, (*VartypeCheck).License)
@@ -753,9 +753,9 @@ func (s *Suite) Test_VartypeCheck_PkgPat
"../../invalid/relative")
vt.Output(
- "ERROR: filename:3: \"../../invalid\" does not exist.",
+ "ERROR: filename:3: Relative path \"../../invalid\" does not exist.",
"WARN: filename:3: \"../../invalid\" is not a valid relative package directory.",
- "ERROR: filename:4: \"../../../../invalid/relative\" does not exist.",
+ "ERROR: filename:4: Relative path \"../../../../invalid/relative\" does not exist.",
"WARN: filename:4: \"../../../../invalid/relative\" is not a valid relative package directory.")
}
@@ -862,9 +862,9 @@ func (s *Suite) Test_VartypeCheck_Relati
"../../invalid/relative")
vt.Output(
- "ERROR: filename:1: \"category/other-package\" does not exist.",
- "ERROR: filename:4: \"invalid\" does not exist.",
- "ERROR: filename:5: \"../../invalid/relative\" does not exist.")
+ "ERROR: filename:1: Relative path \"category/other-package\" does not exist.",
+ "ERROR: filename:4: Relative path \"invalid\" does not exist.",
+ "ERROR: filename:5: Relative path \"../../invalid/relative\" does not exist.")
}
func (s *Suite) Test_VartypeCheck_Restricted(c *check.C) {
Home |
Main Index |
Thread Index |
Old Index