pkgsrc-Changes archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
CVS commit: pkgsrc/pkgtools/pkglint
Module Name: pkgsrc
Committed By: rillig
Date: Sat Jan 26 16:31:33 UTC 2019
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile PLIST
pkgsrc/pkgtools/pkglint/files: autofix.go autofix_test.go buildlink3.go
buildlink3_test.go category.go check_test.go distinfo.go
distinfo_test.go files_test.go licenses.go licenses_test.go line.go
linechecker.go linechecker_test.go lines.go lines_test.go
logging_test.go mkline.go mkline_test.go mklinechecker.go
mklinechecker_test.go mklines.go mklines_test.go mkparser.go
mkparser_test.go mkshparser.go mkshparser_test.go mktypes_test.go
options.go options_test.go package.go package_test.go patches.go
patches_test.go pkglint.go pkglint_test.go pkgsrc.go pkgsrc_test.go
plist.go plist_test.go shell.go shell_test.go shtokenizer_test.go
shtypes.go substcontext.go substcontext_test.go util.go
util_test.go vardefs.go vartype.go vartype_test.go vartypecheck.go
vartypecheck_test.go
Added Files:
pkgsrc/pkgtools/pkglint/files: linelexer.go linelexer_test.go
pkgsrc/pkgtools/pkglint/files/cmd/pkglint: main.go main_test.go
Removed Files:
pkgsrc/pkgtools/pkglint/files: expecter.go expecter_test.go
pkgsrc/pkgtools/pkglint/files/cmd/pkglint: pkglint.go pkglint_test.go
Log Message:
pkgtools/pkglint: update to 5.6.12
Changes since 5.6.11:
* In buildlink3.mk files, print the paths relative
to the line, not to the pkgsrc root.
* When explaining that a variable cannot be set/used because of wrong
permissions, list the permissions. This provides more transparency
than just stating that the desired action is not allowed.
* When pkglint checks a pkgsrc-wip package, don't warn about malformed
lines in doc/CHANGES-* since pkgsrc-wip users typically cannot do
anything about these errors.
* In profiling mode, not only the code coverage and the performance
statistics are dumped, the whole heap is also dumped to see which
parts of pkglint consume the most heap memory. Pkglint now needs
less heap memory than before, which mainly affects full scans.
* The checks for absolute pathnames have gone. They were of questionable
value since pkglint has failed to give proper advice on how to fix
them properly, at least for the last 12 years.
* The check that pkgsrc-wip packages should only use exact CVS Ids
(the unexpanded variant) has been disabled again. It occurred about
16000 times but even fixing it wouldn't improve anything since it
was mostly a formatting issue without any practical consequences.
* Warn about trailing variable modifiers like in ${VAR:S,from,to,extra}.
* Properly parse ${VAR:!command!}.
* Suggest to replace SUBST_SED with SUBST_VARS where possible, even
with complicated shell quoting. Pkglint can autofix most of these
overly verbose cases.
* Load builtin.mk whenever the corresponding buildlink3.mk file is
included. This fixes several warnings about undefined variables
(especially for packages using OpenSSL).
* Parse .for lines like bmake does since 2015, splitting words like
in brk_string.
* Optionally show a warning even if it cannot be autofixed by pkglint.
This is useful for the SUBST_VARS replacement since even when
pkglint cannot automatically replace the code, there are still cases
where it can warn at least.
* As always, several refactorings.
To generate a diff of this commit:
cvs rdiff -u -r1.565 -r1.566 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.9 -r1.10 pkgsrc/pkgtools/pkglint/PLIST
cvs rdiff -u -r1.15 -r1.16 pkgsrc/pkgtools/pkglint/files/autofix.go
cvs rdiff -u -r1.16 -r1.17 pkgsrc/pkgtools/pkglint/files/autofix_test.go \
pkgsrc/pkgtools/pkglint/files/category.go
cvs rdiff -u -r1.18 -r1.19 pkgsrc/pkgtools/pkglint/files/buildlink3.go
cvs rdiff -u -r1.25 -r1.26 pkgsrc/pkgtools/pkglint/files/buildlink3_test.go \
pkgsrc/pkgtools/pkglint/files/vartype.go
cvs rdiff -u -r1.32 -r1.33 pkgsrc/pkgtools/pkglint/files/check_test.go \
pkgsrc/pkgtools/pkglint/files/shell.go
cvs rdiff -u -r1.26 -r1.27 pkgsrc/pkgtools/pkglint/files/distinfo.go
cvs rdiff -u -r1.23 -r1.24 pkgsrc/pkgtools/pkglint/files/distinfo_test.go \
pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
cvs rdiff -u -r1.15 -r0 pkgsrc/pkgtools/pkglint/files/expecter.go
cvs rdiff -u -r1.2 -r0 pkgsrc/pkgtools/pkglint/files/expecter_test.go
cvs rdiff -u -r1.22 -r1.23 pkgsrc/pkgtools/pkglint/files/files_test.go \
pkgsrc/pkgtools/pkglint/files/mkparser.go
cvs rdiff -u -r1.19 -r1.20 pkgsrc/pkgtools/pkglint/files/licenses.go
cvs rdiff -u -r1.20 -r1.21 pkgsrc/pkgtools/pkglint/files/licenses_test.go \
pkgsrc/pkgtools/pkglint/files/mkparser_test.go \
pkgsrc/pkgtools/pkglint/files/substcontext.go \
pkgsrc/pkgtools/pkglint/files/substcontext_test.go
cvs rdiff -u -r1.31 -r1.32 pkgsrc/pkgtools/pkglint/files/line.go \
pkgsrc/pkgtools/pkglint/files/pkglint_test.go \
pkgsrc/pkgtools/pkglint/files/plist_test.go
cvs rdiff -u -r1.12 -r1.13 pkgsrc/pkgtools/pkglint/files/linechecker.go
cvs rdiff -u -r1.13 -r1.14 pkgsrc/pkgtools/pkglint/files/linechecker_test.go \
pkgsrc/pkgtools/pkglint/files/shtokenizer_test.go \
pkgsrc/pkgtools/pkglint/files/shtypes.go
cvs rdiff -u -r0 -r1.1 pkgsrc/pkgtools/pkglint/files/linelexer.go \
pkgsrc/pkgtools/pkglint/files/linelexer_test.go
cvs rdiff -u -r1.4 -r1.5 pkgsrc/pkgtools/pkglint/files/lines.go
cvs rdiff -u -r1.6 -r1.7 pkgsrc/pkgtools/pkglint/files/lines_test.go \
pkgsrc/pkgtools/pkglint/files/mktypes_test.go
cvs rdiff -u -r1.10 -r1.11 pkgsrc/pkgtools/pkglint/files/logging_test.go \
pkgsrc/pkgtools/pkglint/files/mkshparser_test.go \
pkgsrc/pkgtools/pkglint/files/options.go
cvs rdiff -u -r1.44 -r1.45 pkgsrc/pkgtools/pkglint/files/mkline.go
cvs rdiff -u -r1.49 -r1.50 pkgsrc/pkgtools/pkglint/files/mkline_test.go
cvs rdiff -u -r1.27 -r1.28 pkgsrc/pkgtools/pkglint/files/mklinechecker.go \
pkgsrc/pkgtools/pkglint/files/patches.go \
pkgsrc/pkgtools/pkglint/files/patches_test.go
cvs rdiff -u -r1.39 -r1.40 pkgsrc/pkgtools/pkglint/files/mklines.go
cvs rdiff -u -r1.34 -r1.35 pkgsrc/pkgtools/pkglint/files/mklines_test.go
cvs rdiff -u -r1.11 -r1.12 pkgsrc/pkgtools/pkglint/files/mkshparser.go
cvs rdiff -u -r1.9 -r1.10 pkgsrc/pkgtools/pkglint/files/options_test.go
cvs rdiff -u -r1.43 -r1.44 pkgsrc/pkgtools/pkglint/files/package.go
cvs rdiff -u -r1.37 -r1.38 pkgsrc/pkgtools/pkglint/files/package_test.go \
pkgsrc/pkgtools/pkglint/files/shell_test.go
cvs rdiff -u -r1.45 -r1.46 pkgsrc/pkgtools/pkglint/files/pkglint.go
cvs rdiff -u -r1.17 -r1.18 pkgsrc/pkgtools/pkglint/files/pkgsrc.go
cvs rdiff -u -r1.14 -r1.15 pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go \
pkgsrc/pkgtools/pkglint/files/vartype_test.go
cvs rdiff -u -r1.35 -r1.36 pkgsrc/pkgtools/pkglint/files/plist.go \
pkgsrc/pkgtools/pkglint/files/util.go
cvs rdiff -u -r1.21 -r1.22 pkgsrc/pkgtools/pkglint/files/util_test.go
cvs rdiff -u -r1.54 -r1.55 pkgsrc/pkgtools/pkglint/files/vardefs.go
cvs rdiff -u -r1.47 -r1.48 pkgsrc/pkgtools/pkglint/files/vartypecheck.go
cvs rdiff -u -r1.40 -r1.41 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
cvs rdiff -u -r0 -r1.1 pkgsrc/pkgtools/pkglint/files/cmd/pkglint/main.go \
pkgsrc/pkgtools/pkglint/files/cmd/pkglint/main_test.go
cvs rdiff -u -r1.2 -r0 pkgsrc/pkgtools/pkglint/files/cmd/pkglint/pkglint.go
cvs rdiff -u -r1.1 -r0 \
pkgsrc/pkgtools/pkglint/files/cmd/pkglint/pkglint_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.565 pkgsrc/pkgtools/pkglint/Makefile:1.566
--- pkgsrc/pkgtools/pkglint/Makefile:1.565 Thu Jan 24 10:00:42 2019
+++ pkgsrc/pkgtools/pkglint/Makefile Sat Jan 26 16:31:33 2019
@@ -1,7 +1,6 @@
-# $NetBSD: Makefile,v 1.565 2019/01/24 10:00:42 bsiegert Exp $
+# $NetBSD: Makefile,v 1.566 2019/01/26 16:31:33 rillig Exp $
-PKGNAME= pkglint-5.6.11
-PKGREVISION= 1
+PKGNAME= pkglint-5.6.12
CATEGORIES= pkgtools
DISTNAME= tools
MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/}
Index: pkgsrc/pkgtools/pkglint/PLIST
diff -u pkgsrc/pkgtools/pkglint/PLIST:1.9 pkgsrc/pkgtools/pkglint/PLIST:1.10
--- pkgsrc/pkgtools/pkglint/PLIST:1.9 Sun Jan 13 19:55:52 2019
+++ pkgsrc/pkgtools/pkglint/PLIST Sat Jan 26 16:31:33 2019
@@ -1,4 +1,4 @@
-@comment $NetBSD: PLIST,v 1.9 2019/01/13 19:55:52 rillig Exp $
+@comment $NetBSD: PLIST,v 1.10 2019/01/26 16:31:33 rillig Exp $
bin/pkglint
gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint.a
gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint/getopt.a
@@ -18,12 +18,10 @@ gopkg/src/netbsd.org/pkglint/buildlink3_
gopkg/src/netbsd.org/pkglint/category.go
gopkg/src/netbsd.org/pkglint/category_test.go
gopkg/src/netbsd.org/pkglint/check_test.go
-gopkg/src/netbsd.org/pkglint/cmd/pkglint/pkglint.go
-gopkg/src/netbsd.org/pkglint/cmd/pkglint/pkglint_test.go
+gopkg/src/netbsd.org/pkglint/cmd/pkglint/main.go
+gopkg/src/netbsd.org/pkglint/cmd/pkglint/main_test.go
gopkg/src/netbsd.org/pkglint/distinfo.go
gopkg/src/netbsd.org/pkglint/distinfo_test.go
-gopkg/src/netbsd.org/pkglint/expecter.go
-gopkg/src/netbsd.org/pkglint/expecter_test.go
gopkg/src/netbsd.org/pkglint/files.go
gopkg/src/netbsd.org/pkglint/files_test.go
gopkg/src/netbsd.org/pkglint/fuzzer_test.go
@@ -44,6 +42,8 @@ gopkg/src/netbsd.org/pkglint/line.go
gopkg/src/netbsd.org/pkglint/line_test.go
gopkg/src/netbsd.org/pkglint/linechecker.go
gopkg/src/netbsd.org/pkglint/linechecker_test.go
+gopkg/src/netbsd.org/pkglint/linelexer.go
+gopkg/src/netbsd.org/pkglint/linelexer_test.go
gopkg/src/netbsd.org/pkglint/lines.go
gopkg/src/netbsd.org/pkglint/lines_test.go
gopkg/src/netbsd.org/pkglint/logging.go
Index: pkgsrc/pkgtools/pkglint/files/autofix.go
diff -u pkgsrc/pkgtools/pkglint/files/autofix.go:1.15 pkgsrc/pkgtools/pkglint/files/autofix.go:1.16
--- pkgsrc/pkgtools/pkglint/files/autofix.go:1.15 Mon Dec 17 00:15:39 2018
+++ pkgsrc/pkgtools/pkglint/files/autofix.go Sat Jan 26 16:31:33 2019
@@ -30,6 +30,7 @@ type autofixShortTerm struct {
diagFormat string // Is logged only if it couldn't be fixed automatically
diagArgs []interface{} //
explanation []string // Is printed together with the diagnostic
+ anyway bool // Print the diagnostic even if it cannot be autofixed
}
type autofixAction struct {
@@ -229,6 +230,12 @@ func (fix *Autofix) Delete() {
}
}
+// Anyway has the effect of showing the diagnostic even when nothing can
+// be fixed automatically.
+func (fix *Autofix) Anyway() {
+ fix.anyway = true
+}
+
// Apply does the actual work.
// Depending on the pkglint mode, it either:
//
@@ -255,7 +262,7 @@ func (fix *Autofix) Apply() {
fix.autofixShortTerm = autofixShortTerm{}
}
- if !G.Logger.Relevant(fix.diagFormat) || len(fix.actions) == 0 {
+ if !G.Logger.Relevant(fix.diagFormat) || !fix.anyway && len(fix.actions) == 0 {
reset()
return
}
Index: pkgsrc/pkgtools/pkglint/files/autofix_test.go
diff -u pkgsrc/pkgtools/pkglint/files/autofix_test.go:1.16 pkgsrc/pkgtools/pkglint/files/autofix_test.go:1.17
--- pkgsrc/pkgtools/pkglint/files/autofix_test.go:1.16 Sun Jan 13 19:55:52 2019
+++ pkgsrc/pkgtools/pkglint/files/autofix_test.go Sat Jan 26 16:31:33 2019
@@ -547,9 +547,53 @@ func (s *Suite) Test_Autofix_Delete__com
"+\tbelow")
}
+// Demonstrates that without the --show-autofix option, diagnostics are
+// shown even when they cannot be autofixed.
+//
+// This is typical when an autofix is provided for simple scenarios,
+// but the code actually found is a little more complicated.
+func (s *Suite) Test_Autofix__show_unfixable_diagnostics_in_default_mode(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("--source")
+ lines := t.NewLines("Makefile",
+ "line1",
+ "line2",
+ "line3")
+
+ lines.Lines[0].Warnf("This warning is shown since the --show-autofix option is not given.")
+
+ fix := lines.Lines[1].Autofix()
+ fix.Warnf("This warning cannot be fixed and is therefore not shown.")
+ fix.Replace("XXX", "TODO")
+ fix.Apply()
+
+ fix.Warnf("This warning cannot be fixed automatically but should be shown anyway.")
+ fix.Replace("XXX", "TODO")
+ fix.Anyway()
+ fix.Apply()
+
+ // If this warning should ever appear it is probably because fix.anyway is not reset properly.
+ fix.Warnf("This warning cannot be fixed and is therefore not shown.")
+ fix.Replace("XXX", "TODO")
+ fix.Apply()
+
+ lines.Lines[2].Warnf("This warning is also shown.")
+
+ t.CheckOutputLines(
+ ">\tline1",
+ "WARN: Makefile:1: This warning is shown since the --show-autofix option is not given.",
+ "",
+ ">\tline2",
+ "WARN: Makefile:2: This warning cannot be fixed automatically but should be shown anyway.",
+ "",
+ ">\tline3",
+ "WARN: Makefile:3: This warning is also shown.")
+}
+
// Demonstrates that the --show-autofix option only shows those diagnostics
// that would be fixed.
-func (s *Suite) Test_Autofix__suppress_unfixable_warnings(c *check.C) {
+func (s *Suite) Test_Autofix__suppress_unfixable_warnings_with_show_autofix(c *check.C) {
t := s.Init(c)
t.SetUpCommandLine("--show-autofix", "--source")
Index: pkgsrc/pkgtools/pkglint/files/category.go
diff -u pkgsrc/pkgtools/pkglint/files/category.go:1.16 pkgsrc/pkgtools/pkglint/files/category.go:1.17
--- pkgsrc/pkgtools/pkglint/files/category.go:1.16 Mon Dec 17 00:15:39 2018
+++ pkgsrc/pkgtools/pkglint/files/category.go Sat Jan 26 16:31:33 2019
@@ -14,13 +14,13 @@ func CheckdirCategory(dir string) {
mklines.Check()
- exp := NewMkExpecter(mklines)
- for exp.AdvanceIfPrefix("#") {
+ mlex := NewMkLinesLexer(mklines)
+ for mlex.SkipPrefix("#") {
}
- exp.ExpectEmptyLine()
+ mlex.SkipEmptyOrNote()
- if exp.AdvanceIf(func(mkline MkLine) bool { return mkline.IsVarassign() && mkline.Varname() == "COMMENT" }) {
- mkline := exp.PreviousMkLine()
+ if mlex.SkipIf(func(mkline MkLine) bool { return mkline.IsVarassign() && mkline.Varname() == "COMMENT" }) {
+ mkline := mlex.PreviousMkLine()
lex := textproc.NewLexer(mkline.Value())
valid := textproc.NewByteSet("--- '(),/0-9A-Za-z")
@@ -40,9 +40,9 @@ func CheckdirCategory(dir string) {
}
} else {
- exp.CurrentLine().Errorf("COMMENT= line expected.")
+ mlex.CurrentLine().Errorf("COMMENT= line expected.")
}
- exp.ExpectEmptyLine()
+ mlex.SkipEmptyOrNote()
type subdir struct {
name string
@@ -57,11 +57,11 @@ func CheckdirCategory(dir string) {
var mSubdirs []subdir
seen := make(map[string]MkLine)
- for !exp.EOF() {
- mkline := exp.CurrentMkLine()
+ for !mlex.EOF() {
+ mkline := mlex.CurrentMkLine()
if (mkline.IsVarassign() || mkline.IsCommentedVarassign()) && mkline.Varname() == "SUBDIR" {
- exp.Advance()
+ mlex.Skip()
name := mkline.Value()
if mkline.IsCommentedVarassign() && mkline.VarassignComment() == "" {
@@ -113,7 +113,7 @@ func CheckdirCategory(dir string) {
if len(mRest) > 0 {
line = mRest[0].line.Line
} else {
- line = exp.CurrentLine()
+ line = mlex.CurrentLine()
}
fix := line.Autofix()
@@ -141,10 +141,10 @@ func CheckdirCategory(dir string) {
// the pkgsrc-wip category Makefile defines its own targets for
// generating indexes and READMEs. Just skip them.
if !G.Wip {
- exp.ExpectEmptyLine()
- exp.ExpectText(".include \"../mk/misc/category.mk\"")
- if !exp.EOF() {
- exp.CurrentLine().Errorf("The file should end here.")
+ mlex.SkipEmptyOrNote()
+ mlex.SkipContainsOrWarn(".include \"../mk/misc/category.mk\"")
+ if !mlex.EOF() {
+ mlex.CurrentLine().Errorf("The file should end here.")
}
}
Index: pkgsrc/pkgtools/pkglint/files/buildlink3.go
diff -u pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.18 pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.19
--- pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.18 Sun Jan 13 19:55:52 2019
+++ pkgsrc/pkgtools/pkglint/files/buildlink3.go Sat Jan 26 16:31:33 2019
@@ -25,66 +25,66 @@ func (ck *Buildlink3Checker) Check() {
mklines.Check()
- exp := NewMkExpecter(mklines)
+ llex := NewMkLinesLexer(mklines)
- for exp.AdvanceIf(MkLine.IsComment) {
- line := exp.PreviousLine()
+ for llex.SkipIf(MkLine.IsComment) {
+ line := llex.PreviousLine()
// See pkgtools/createbuildlink/files/createbuildlink
if hasPrefix(line.Text, "# XXX This file was created automatically") {
line.Errorf("This comment indicates unfinished work (url2pkg).")
}
}
- exp.ExpectEmptyLine()
+ llex.SkipEmptyOrNote()
- if exp.AdvanceIfMatches(`^BUILDLINK_DEPMETHOD\.([^\t ]+)\?=.*$`) {
- exp.PreviousLine().Warnf("This line belongs inside the .ifdef block.")
- for exp.AdvanceIfEquals("") {
+ if llex.SkipRegexp(`^BUILDLINK_DEPMETHOD\.([^\t ]+)\?=.*$`) {
+ llex.PreviousLine().Warnf("This line belongs inside the .ifdef block.")
+ for llex.SkipString("") {
}
}
- if !ck.checkFirstParagraph(exp) {
+ if !ck.checkFirstParagraph(llex) {
return
}
- if !ck.checkSecondParagraph(exp) {
+ if !ck.checkSecondParagraph(llex) {
return
}
- if !ck.checkMainPart(exp) {
+ if !ck.checkMainPart(llex) {
return
}
// Fourth paragraph: Cleanup, corresponding to the first paragraph.
- if !exp.ExpectText("BUILDLINK_TREE+=\t-" + ck.pkgbase) {
+ if !llex.SkipContainsOrWarn("BUILDLINK_TREE+=\t-" + ck.pkgbase) {
return
}
- if !exp.EOF() {
- exp.CurrentLine().Warnf("The file should end here.")
+ if !llex.EOF() {
+ llex.CurrentLine().Warnf("The file should end here.")
}
if G.Pkg != nil {
- // TODO: Commenting this line doesn't make any test fail, but it should.
G.Pkg.checkLinesBuildlink3Inclusion(mklines)
}
mklines.SaveAutofixChanges()
}
-func (ck *Buildlink3Checker) checkFirstParagraph(exp *MkExpecter) bool {
+func (ck *Buildlink3Checker) checkFirstParagraph(mlex *MkLinesLexer) bool {
// First paragraph: Introduction of the package identifier
- if !exp.AdvanceIfMatches(`^BUILDLINK_TREE\+=[\t ]*([^\t ]+)$`) {
- exp.CurrentLine().Warnf("Expected a BUILDLINK_TREE line.")
+ m := mlex.NextRegexp(`^BUILDLINK_TREE\+=[\t ]*([^\t ]+)$`)
+ if m == nil {
+ mlex.CurrentLine().Warnf("Expected a BUILDLINK_TREE line.")
return false
}
- pkgbase := exp.Group(1)
- pkgbaseLine := exp.PreviousMkLine()
+ pkgbase := m[1]
+ pkgbaseLine := mlex.PreviousMkLine()
if containsVarRef(pkgbase) {
ck.checkVaruseInPkgbase(pkgbase, pkgbaseLine)
}
- exp.ExpectEmptyLine()
+ mlex.SkipEmptyOrNote()
ck.pkgbase = pkgbase
ck.pkgbaseLine = pkgbaseLine
return true
@@ -92,19 +92,20 @@ func (ck *Buildlink3Checker) checkFirstP
// checkSecondParagraph checks the multiple inclusion protection and
// introduces the uppercase package identifier.
-func (ck *Buildlink3Checker) checkSecondParagraph(exp *MkExpecter) bool {
+func (ck *Buildlink3Checker) checkSecondParagraph(mlex *MkLinesLexer) bool {
pkgbase := ck.pkgbase
pkgbaseLine := ck.pkgbaseLine
- if !exp.AdvanceIfMatches(`^\.if !defined\(([^\t ]+)_BUILDLINK3_MK\)$`) {
+ m := mlex.NextRegexp(`^\.if !defined\(([^\t ]+)_BUILDLINK3_MK\)$`)
+ if m == nil {
return false
}
- pkgupperLine, pkgupper := exp.PreviousMkLine(), exp.Group(1)
+ pkgupperLine, pkgupper := mlex.PreviousMkLine(), m[1]
- if !exp.ExpectText(pkgupper + "_BUILDLINK3_MK:=") {
+ if !mlex.SkipContainsOrWarn(pkgupper + "_BUILDLINK3_MK:=") {
return false
}
- exp.ExpectEmptyLine()
+ mlex.SkipEmptyOrNote()
// See pkgtools/createbuildlink/files/createbuildlink, keyword PKGUPPER
ucPkgbase := strings.ToUpper(strings.Replace(pkgbase, "-", "_", -1))
@@ -123,19 +124,19 @@ func (ck *Buildlink3Checker) checkSecond
}
// Third paragraph: Package information.
-func (ck *Buildlink3Checker) checkMainPart(exp *MkExpecter) bool {
+func (ck *Buildlink3Checker) checkMainPart(mlex *MkLinesLexer) bool {
pkgbase := ck.pkgbase
// The first .if is from the second paragraph.
indentLevel := 1
- for !exp.EOF() && indentLevel > 0 {
- mkline := exp.CurrentMkLine()
- exp.Advance()
+ for !mlex.EOF() && indentLevel > 0 {
+ mkline := mlex.CurrentMkLine()
+ mlex.Skip()
switch {
case mkline.IsVarassign():
- ck.checkVarassign(exp, mkline, pkgbase)
+ ck.checkVarassign(mlex, mkline, pkgbase)
case mkline.IsDirective() && mkline.Directive() == "if":
indentLevel++
@@ -150,13 +151,13 @@ func (ck *Buildlink3Checker) checkMainPa
}
if ck.apiLine == nil {
- exp.CurrentLine().Warnf("Definition of BUILDLINK_API_DEPENDS is missing.")
+ mlex.CurrentLine().Warnf("Definition of BUILDLINK_API_DEPENDS is missing.")
}
- exp.ExpectEmptyLine()
+ mlex.SkipEmptyOrNote()
return true
}
-func (ck *Buildlink3Checker) checkVarassign(exp *MkExpecter, mkline MkLine, pkgbase string) {
+func (ck *Buildlink3Checker) checkVarassign(mlex *MkLinesLexer, mkline MkLine, pkgbase string) {
varname, value := mkline.Varname(), mkline.Value()
doCheck := false
@@ -204,28 +205,30 @@ func (ck *Buildlink3Checker) checkVarass
}
func (ck *Buildlink3Checker) checkVaruseInPkgbase(pkgbase string, pkgbaseLine MkLine) {
- checkSpecificVar := func(varuse, simple string) bool {
- if contains(pkgbase, varuse) {
- pkgbaseLine.Warnf("Please use %q instead of %q (also in other variables in this file).", simple, varuse)
- return true
+ for _, token := range pkgbaseLine.ValueTokens() {
+ if token.Varuse == nil {
+ continue
+ }
+
+ replacement := ""
+ switch token.Varuse.varname {
+ case "PYPKGPREFIX":
+ replacement = "py"
+ case "RUBY_BASE", "RUBY_PKGPREFIX":
+ replacement = "ruby"
+ case "PHP_PKG_PREFIX":
+ replacement = "php"
+ }
+
+ if replacement != "" {
+ pkgbaseLine.Warnf("Please use %q instead of %q (also in other variables in this file).",
+ replacement, token.Text)
+ } else {
+ pkgbaseLine.Warnf(
+ "Please replace %q with a simple string (also in other variables in this file).",
+ token.Text)
}
- return false
- }
-
- warned := checkSpecificVar("${PYPKGPREFIX}", "py") ||
- checkSpecificVar("${RUBY_BASE}", "ruby") ||
- checkSpecificVar("${RUBY_PKGPREFIX}", "ruby") ||
- checkSpecificVar("${PHP_PKG_PREFIX}", "php")
-
- if !warned {
- // TODO: Replace regex with proper VarUse.
- if m, varuse := match1(pkgbase, `(\$\{\w+\})`); m {
- pkgbaseLine.Warnf("Please replace %q with a simple string (also in other variables in this file).", varuse)
- warned = true
- }
- }
- if warned {
G.Explain(
"The identifiers in the BUILDLINK_TREE variable should be plain",
"strings that do not refer to any variable.",
Index: pkgsrc/pkgtools/pkglint/files/buildlink3_test.go
diff -u pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.25 pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.26
--- pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.25 Sun Jan 13 19:55:52 2019
+++ pkgsrc/pkgtools/pkglint/files/buildlink3_test.go Sat Jan 26 16:31:33 2019
@@ -2,6 +2,32 @@ package pkglint
import "gopkg.in/check.v1"
+// This test ensures that CheckLinesBuildlink3Mk really checks for
+// buildlink3.mk files that are included by the buildlink3.mk file
+// but not by the package.
+func (s *Suite) Test_CheckLinesBuildlink3Mk__package(c *check.C) {
+ t := s.Init(c)
+
+ t.CreateFileLines("category/dependency1/buildlink3.mk",
+ MkRcsID)
+ t.CreateFileLines("category/dependency2/buildlink3.mk",
+ MkRcsID)
+ t.SetUpPackage("category/package",
+ "PKGNAME=\tpackage-1.0",
+ "",
+ ".include \"../../category/dependency1/buildlink3.mk\"")
+
+ t.CreateFileDummyBuildlink3("category/package/buildlink3.mk",
+ ".include \"../../category/dependency2/buildlink3.mk\"")
+
+ G.Check(t.File("category/package"))
+
+ t.CheckOutputLines(
+ "WARN: ~/category/package/buildlink3.mk:12: " +
+ "../../category/dependency2/buildlink3.mk is included " +
+ "by this file but not by the package.")
+}
+
func (s *Suite) Test_CheckLinesBuildlink3Mk__unfinished_url2pkg(c *check.C) {
t := s.Init(c)
@@ -355,7 +381,21 @@ func (s *Suite) Test_CheckLinesBuildlink
t := s.Init(c)
t.SetUpVartypes()
- mklines := t.NewMkLines("buildlink3.mk",
+ mklinesPhp := t.NewMkLines("x11/php-wxwidgets/buildlink3.mk",
+ MkRcsID,
+ "",
+ "BUILDLINK_TREE+=\t${PHP_PKG_PREFIX}-wxWidgets",
+ "",
+ ".if !defined(PHP_WXWIDGETS_BUILDLINK3_MK)",
+ "PHP_WXWIDGETS_BUILDLINK3_MK:=",
+ "",
+ "BUILDLINK_API_DEPENDS.${PHP_PKG_PREFIX}-wxWidgets+=\t${PHP_PKG_PREFIX}-wxWidgets>=2.6.1.0",
+ "BUILDLINK_ABI_DEPENDS.${PHP_PKG_PREFIX}-wxWidgets+=\t${PHP_PKG_PREFIX}-wxWidgets>=2.8.10.1nb26",
+ "",
+ ".endif",
+ "",
+ "BUILDLINK_TREE+=\t-${PHP_PKG_PREFIX}-wxWidgets")
+ mklinesPy := t.NewMkLines("x11/py-wxwidgets/buildlink3.mk",
MkRcsID,
"",
"BUILDLINK_TREE+=\t${PYPKGPREFIX}-wxWidgets",
@@ -369,11 +409,45 @@ func (s *Suite) Test_CheckLinesBuildlink
".endif",
"",
"BUILDLINK_TREE+=\t-${PYPKGPREFIX}-wxWidgets")
+ mklinesRuby1 := t.NewMkLines("x11/ruby1-wxwidgets/buildlink3.mk",
+ MkRcsID,
+ "",
+ "BUILDLINK_TREE+=\t${RUBY_BASE}-wxWidgets",
+ "",
+ ".if !defined(RUBY_WXWIDGETS_BUILDLINK3_MK)",
+ "RUBY_WXWIDGETS_BUILDLINK3_MK:=",
+ "",
+ "BUILDLINK_API_DEPENDS.${RUBY_BASE}-wxWidgets+=\t${RUBY_BASE}-wxWidgets>=2.6.1.0",
+ "BUILDLINK_ABI_DEPENDS.${RUBY_BASE}-wxWidgets+=\t${RUBY_BASE}-wxWidgets>=2.8.10.1nb26",
+ "",
+ ".endif",
+ "",
+ "BUILDLINK_TREE+=\t-${RUBY_BASE}-wxWidgets")
+ mklinesRuby2 := t.NewMkLines("x11/ruby2-wxwidgets/buildlink3.mk",
+ MkRcsID,
+ "",
+ "BUILDLINK_TREE+=\t${RUBY_PKGPREFIX}-wxWidgets",
+ "",
+ ".if !defined(RUBY_WXWIDGETS_BUILDLINK3_MK)",
+ "RUBY_WXWIDGETS_BUILDLINK3_MK:=",
+ "",
+ "BUILDLINK_API_DEPENDS.${RUBY_PKGPREFIX}-wxWidgets+=\t${RUBY_PKGPREFIX}-wxWidgets>=2.6.1.0",
+ "BUILDLINK_ABI_DEPENDS.${RUBY_PKGPREFIX}-wxWidgets+=\t${RUBY_PKGPREFIX}-wxWidgets>=2.8.10.1nb26",
+ "",
+ ".endif",
+ "",
+ "BUILDLINK_TREE+=\t-${RUBY_PKGPREFIX}-wxWidgets")
- CheckLinesBuildlink3Mk(mklines)
+ CheckLinesBuildlink3Mk(mklinesPhp)
+ CheckLinesBuildlink3Mk(mklinesPy)
+ CheckLinesBuildlink3Mk(mklinesRuby1)
+ CheckLinesBuildlink3Mk(mklinesRuby2)
t.CheckOutputLines(
- "WARN: buildlink3.mk:3: Please use \"py\" instead of \"${PYPKGPREFIX}\" (also in other variables in this file).")
+ "WARN: x11/php-wxwidgets/buildlink3.mk:3: Please use \"php\" instead of \"${PHP_PKG_PREFIX}\" (also in other variables in this file).",
+ "WARN: x11/py-wxwidgets/buildlink3.mk:3: Please use \"py\" instead of \"${PYPKGPREFIX}\" (also in other variables in this file).",
+ "WARN: x11/ruby1-wxwidgets/buildlink3.mk:3: Please use \"ruby\" instead of \"${RUBY_BASE}\" (also in other variables in this file).",
+ "WARN: x11/ruby2-wxwidgets/buildlink3.mk:3: Please use \"ruby\" instead of \"${RUBY_PKGPREFIX}\" (also in other variables in this file).")
}
func (s *Suite) Test_CheckLinesBuildlink3Mk__PKGBASE_with_unknown_variable(c *check.C) {
@@ -399,30 +473,22 @@ func (s *Suite) Test_CheckLinesBuildlink
t.CheckOutputLines(
"WARN: buildlink3.mk:3: LICENSE may not be used in any file; it is a write-only variable.",
- // FIXME: License is not a list type, although it can be appended to.
- "WARN: buildlink3.mk:3: The list variable LICENSE should not be embedded in a word.",
+ "WARN: buildlink3.mk:3: The variable LICENSE should be quoted as part of a shell word.",
"WARN: buildlink3.mk:8: LICENSE should not be evaluated at load time.",
"WARN: buildlink3.mk:8: LICENSE may not be used in any file; it is a write-only variable.",
- // FIXME: License is not a list type, although it can be appended to.
- "WARN: buildlink3.mk:8: The list variable LICENSE should not be embedded in a word.",
"WARN: buildlink3.mk:8: LICENSE should not be evaluated indirectly at load time.",
"WARN: buildlink3.mk:8: LICENSE may not be used in any file; it is a write-only variable.",
- // FIXME: License is not a list type, although it can be appended to.
- "WARN: buildlink3.mk:8: The list variable LICENSE should not be embedded in a word.",
+ "WARN: buildlink3.mk:8: The variable LICENSE should be quoted as part of a shell word.",
"WARN: buildlink3.mk:9: LICENSE should not be evaluated at load time.",
"WARN: buildlink3.mk:9: LICENSE may not be used in any file; it is a write-only variable.",
- // FIXME: License is not a list type, although it can be appended to.
- "WARN: buildlink3.mk:9: The list variable LICENSE should not be embedded in a word.",
"WARN: buildlink3.mk:9: LICENSE should not be evaluated indirectly at load time.",
"WARN: buildlink3.mk:9: LICENSE may not be used in any file; it is a write-only variable.",
- // FIXME: License is not a list type, although it can be appended to.
- "WARN: buildlink3.mk:9: The list variable LICENSE should not be embedded in a word.",
+ "WARN: buildlink3.mk:9: The variable LICENSE should be quoted as part of a shell word.",
"WARN: buildlink3.mk:13: LICENSE may not be used in any file; it is a write-only variable.",
- // FIXME: License is not a list type, although it can be appended to.
- "WARN: buildlink3.mk:13: The list variable LICENSE should not be embedded in a word.",
+ "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/vartype.go
diff -u pkgsrc/pkgtools/pkglint/files/vartype.go:1.25 pkgsrc/pkgtools/pkglint/files/vartype.go:1.26
--- pkgsrc/pkgtools/pkglint/files/vartype.go:1.25 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/vartype.go Sat Jan 26 16:31:33 2019
@@ -1,9 +1,6 @@
package pkglint
-import (
- "path"
- "strings"
-)
+import "path"
// Vartype is a combination of a data type and a permission specification.
// See vardefs.go for examples, and vartypecheck.go for the implementation.
@@ -56,24 +53,22 @@ func (perms ACLPermissions) String() str
if perms == 0 {
return "none"
}
- result := "" +
- ifelseStr(perms.Contains(aclpSet), "set, ", "") +
- ifelseStr(perms.Contains(aclpSetDefault), "set-default, ", "") +
- ifelseStr(perms.Contains(aclpAppend), "append, ", "") +
- ifelseStr(perms.Contains(aclpUseLoadtime), "use-loadtime, ", "") +
- ifelseStr(perms.Contains(aclpUse), "use, ", "") +
- ifelseStr(perms.Contains(aclpUnknown), "unknown, ", "")
- return strings.TrimRight(result, ", ")
+ return joinSkipEmpty(", ",
+ ifelseStr(perms.Contains(aclpSet), "set", ""),
+ ifelseStr(perms.Contains(aclpSetDefault), "set-default", ""),
+ ifelseStr(perms.Contains(aclpAppend), "append", ""),
+ ifelseStr(perms.Contains(aclpUseLoadtime), "use-loadtime", ""),
+ ifelseStr(perms.Contains(aclpUse), "use", ""),
+ ifelseStr(perms.Contains(aclpUnknown), "unknown", ""))
}
func (perms ACLPermissions) HumanString() string {
- result := "" +
- ifelseStr(perms.Contains(aclpSet), "set, ", "") +
- ifelseStr(perms.Contains(aclpSetDefault), "given a default value, ", "") +
- ifelseStr(perms.Contains(aclpAppend), "appended to, ", "") +
- ifelseStr(perms.Contains(aclpUseLoadtime), "used at load time, ", "") +
- ifelseStr(perms.Contains(aclpUse), "used, ", "")
- return strings.TrimRight(result, ", ")
+ return joinSkipEmptyOxford("or",
+ ifelseStr(perms.Contains(aclpSet), "set", ""),
+ ifelseStr(perms.Contains(aclpSetDefault), "given a default value", ""),
+ ifelseStr(perms.Contains(aclpAppend), "appended to", ""),
+ ifelseStr(perms.Contains(aclpUseLoadtime), "used at load time", ""),
+ ifelseStr(perms.Contains(aclpUse), "used", ""))
}
func (vt *Vartype) EffectivePermissions(basename string) ACLPermissions {
@@ -104,25 +99,34 @@ func (vt *Vartype) AllowedFiles(perms AC
files = append(files, aclEntry.glob)
}
}
- return strings.Join(files, ", ")
+ return joinSkipEmptyCambridge("or", files...)
}
// IsConsideredList returns whether the type is considered a list.
-// This distinction between "real lists" and "considered a list" makes
-// the implementation of checklineMkVartype easier.
+//
+// FIXME: Explain why this method is necessary. IsList is clear, and MayBeAppendedTo also,
+// but this in-between state needs a decent explanation.
+// Probably MkLineChecker.checkVartype needs to be revisited completely.
func (vt *Vartype) IsConsideredList() bool {
if vt.kindOfList == lkShell {
return true
}
switch vt.basicType {
- case BtAwkCommand, BtSedCommands, BtShellCommand, BtShellCommands, BtLicense, BtConfFiles:
+ case BtAwkCommand, BtSedCommands, BtShellCommand, BtShellCommands, BtConfFiles:
return true
}
return false
}
func (vt *Vartype) MayBeAppendedTo() bool {
- return vt.kindOfList != lkNone || vt.IsConsideredList() || vt.basicType == BtComment
+ if vt.kindOfList != lkNone || vt.IsConsideredList() {
+ return true
+ }
+ switch vt.basicType {
+ case BtComment, BtLicense:
+ return true
+ }
+ return false
}
func (vt *Vartype) String() string {
Index: pkgsrc/pkgtools/pkglint/files/check_test.go
diff -u pkgsrc/pkgtools/pkglint/files/check_test.go:1.32 pkgsrc/pkgtools/pkglint/files/check_test.go:1.33
--- pkgsrc/pkgtools/pkglint/files/check_test.go:1.32 Sun Jan 13 19:55:52 2019
+++ pkgsrc/pkgtools/pkglint/files/check_test.go Sat Jan 26 16:31:33 2019
@@ -284,7 +284,12 @@ func (t *Tester) SetUpCategory(name stri
// After calling this method, individual files can be overwritten as necessary.
// Then, G.Pkgsrc.LoadInfrastructure should be called to load all the files.
func (t *Tester) SetUpPackage(pkgpath string, makefileLines ...string) string {
+
category := path.Dir(pkgpath)
+ if category == "wip" {
+ // To avoid boilerplate CATEGORIES definitions for wip packages.
+ category = "local"
+ }
t.SetUpPkgsrc()
t.SetUpVartypes()
@@ -295,18 +300,22 @@ func (t *Tester) SetUpPackage(pkgpath st
t.CreateFileLines(pkgpath+"/PLIST",
PlistRcsID,
"bin/program")
+
+ // This distinfo file contains dummy hashes since pkglint cannot check the
+ // distfiles hashes anyway. It can only check the hashes for the patches.
t.CreateFileLines(pkgpath+"/distinfo",
RcsID,
"",
- "SHA1 (distfile-1.0.tar.gz) = 12341234...",
- "RMD160 (distfile-1.0.tar.gz) = 12341234...",
- "SHA512 (distfile-1.0.tar.gz) = 12341234...",
+ "SHA1 (distfile-1.0.tar.gz) = 12341234",
+ "RMD160 (distfile-1.0.tar.gz) = 12341234",
+ "SHA512 (distfile-1.0.tar.gz) = 12341234",
"Size (distfile-1.0.tar.gz) = 12341234")
mlines := []string{
MkRcsID,
"",
"DISTNAME=\tdistname-1.0",
+ "#PKGNAME=\tpackage-1.0",
"CATEGORIES=\t" + category,
"MASTER_SITES=\t# none",
"",
@@ -380,7 +389,7 @@ func (t *Tester) CreateFileDummyPatch(re
"+new")
}
-func (t *Tester) CreateFileDummyBuildlink3(relativeFileName string) {
+func (t *Tester) CreateFileDummyBuildlink3(relativeFileName string, customLines ...string) {
dir := path.Dir(relativeFileName)
lower := path.Base(dir)
upper := strings.ToUpper(lower)
@@ -395,20 +404,27 @@ func (t *Tester) CreateFileDummyBuildlin
return msg
}
- t.CreateFileLines(relativeFileName,
+ var lines []string
+ lines = append(lines,
MkRcsID,
- sprintf(""),
+ "",
sprintf("BUILDLINK_TREE+=\t%s", lower),
- sprintf(""),
+ "",
sprintf(".if !defined(%s_BUILDLINK3_MK)", upper),
sprintf("%s_BUILDLINK3_MK:=", upper),
- sprintf(""),
+ "",
aligned("BUILDLINK_API_DEPENDS.%s+=", lower)+sprintf("%s>=0", lower),
aligned("BUILDLINK_PKGSRCDIR.%s?=", lower)+sprintf("../../%s", dir),
aligned("BUILDLINK_DEPMETHOD.%s?=", lower)+"build",
+ "")
+ lines = append(lines, customLines...)
+ lines = append(lines,
+ "",
sprintf(".endif # %s_BUILDLINK3_MK", upper),
- sprintf(""),
+ "",
sprintf("BUILDLINK_TREE+=\t-%s", lower))
+
+ t.CreateFileLines(relativeFileName, lines...)
}
// File returns the absolute path to the given file in the
Index: pkgsrc/pkgtools/pkglint/files/shell.go
diff -u pkgsrc/pkgtools/pkglint/files/shell.go:1.32 pkgsrc/pkgtools/pkglint/files/shell.go:1.33
--- pkgsrc/pkgtools/pkglint/files/shell.go:1.32 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/shell.go Sat Jan 26 16:31:33 2019
@@ -24,7 +24,7 @@ func NewShellLine(mkline MkLine) *ShellL
}
var shellCommandsType = &Vartype{lkNone, BtShellCommands, []ACLEntry{{"*", aclpAllRuntime}}, false}
-var shellWordVuc = &VarUseContext{shellCommandsType, vucTimeUnknown, vucQuotPlain, false}
+var shellWordVuc = &VarUseContext{shellCommandsType, vucTimeUnknown, VucQuotPlain, false}
func (shline *ShellLine) CheckWord(token string, checkQuoting bool, time ToolTime) {
if trace.Tracing {
@@ -621,9 +621,6 @@ func (scc *SimpleCommandChecker) checkRe
cmdname := scc.strcmd.Name
isSubst := false
for _, arg := range scc.strcmd.Args {
- if !isSubst {
- LineChecker{scc.shline.mkline.Line}.CheckAbsolutePathname(arg)
- }
if G.Testing && isSubst && !matches(arg, `"^[\"\'].*[\"\']$`) {
scc.shline.mkline.Warnf("Substitution commands like %q should always be quoted.", arg)
G.Explain(
Index: pkgsrc/pkgtools/pkglint/files/distinfo.go
diff -u pkgsrc/pkgtools/pkglint/files/distinfo.go:1.26 pkgsrc/pkgtools/pkglint/files/distinfo.go:1.27
--- pkgsrc/pkgtools/pkglint/files/distinfo.go:1.26 Fri Dec 21 08:05:24 2018
+++ pkgsrc/pkgtools/pkglint/files/distinfo.go Sat Jan 26 16:31:33 2019
@@ -3,6 +3,7 @@ package pkglint
import (
"bytes"
"crypto/sha1"
+ "encoding/hex"
"io/ioutil"
"path"
"strings"
@@ -144,7 +145,6 @@ func (ck *distinfoLinesChecker) checkUnr
patchName := file.Name()
if file.Mode().IsRegular() && !ck.patches[patchName] && hasPrefix(patchName, "patch-") {
ck.distinfoLines.Errorf("patch %q is not recorded. Run %q.",
- // FIXME: Relative paths must not be "../dependency" but the full "../../category/dependency" instead.
cleanpath(relpath(path.Dir(ck.distinfoLines.FileName), G.Pkg.File(ck.patchdir+"/"+patchName))),
bmake("makepatchsum"))
}
@@ -153,21 +153,45 @@ func (ck *distinfoLinesChecker) checkUnr
// Inter-package check for differing distfile checksums.
func (ck *distinfoLinesChecker) checkGlobalDistfileMismatch(line Line, filename, alg, hash string) {
- hashes := G.Pkgsrc.Hashes
// Intentionally checking the filename instead of ck.isPatch.
// Missing the few distfiles that actually start with patch-*
// is more convenient than having lots of false positive mismatches.
+ if hasPrefix(filename, "patch-") {
+ return
+ }
+
+ hashes := G.Pkgsrc.Hashes
+ if hashes == nil {
+ return
+ }
+
+ // The Size hash is not encoded in hex, therefore it would trigger wrong error messages below.
+ // Since the Size hash is targeted towards humans and not really useful for detecting duplicates,
+ // omitting the check here is ok. Any mismatches will be reliably detected because the other
+ // hashes will be different, too.
+ if alg == "Size" {
+ return
+ }
+
if hashes != nil && !hasPrefix(filename, "patch-") {
key := alg + ":" + filename
otherHash := hashes[key]
+
+ // See https://github.com/golang/go/issues/29802
+ hashBytes := make([]byte, hex.DecodedLen(len(hash)))
+ _, err := hex.Decode(hashBytes, []byte(hash))
+ if err != nil {
+ line.Errorf("The %s hash for %s contains a non-hex character.", alg, filename)
+ }
+
if otherHash != nil {
- if otherHash.hash != hash {
+ if !bytes.Equal(otherHash.hash, hashBytes) {
line.Errorf("The %s hash for %s is %s, which conflicts with %s in %s.",
- alg, filename, hash, otherHash.hash, line.RefTo(otherHash.line))
+ alg, filename, hash, hex.EncodeToString(otherHash.hash), line.RefToLocation(otherHash.location))
}
} else {
- hashes[key] = &Hash{hash, line}
+ hashes[key] = &Hash{hashBytes, line.Location}
}
}
}
Index: pkgsrc/pkgtools/pkglint/files/distinfo_test.go
diff -u pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.23 pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.24
--- pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.23 Sun Jan 13 19:55:52 2019
+++ pkgsrc/pkgtools/pkglint/files/distinfo_test.go Sat Jan 26 16:31:33 2019
@@ -55,7 +55,8 @@ func (s *Suite) Test_distinfoLinesChecke
RcsID,
"",
"SHA512 (distfile-1.0.tar.gz) = 1234567822222222",
- "SHA512 (distfile-1.1.tar.gz) = 1111111111111111")
+ "SHA512 (distfile-1.1.tar.gz) = 1111111111111111",
+ "SHA512 (encoding-error.tar.gz) = 12345678abcdefgh")
t.CreateFileLines("Makefile",
MkRcsID,
"",
@@ -75,14 +76,26 @@ func (s *Suite) Test_distinfoLinesChecke
G.Main("pkglint", "-r", "-Wall", "-Call", t.File("."))
t.CheckOutputLines(
- "ERROR: ~/category/package1/distinfo:4: Expected SHA1, RMD160, SHA512, Size checksums for \"distfile-1.0.tar.gz\", got SHA512.",
- "ERROR: ~/category/package1/distinfo:EOF: Expected SHA1, RMD160, SHA512, Size checksums for \"distfile-1.1.tar.gz\", got SHA512.",
+ "ERROR: ~/category/package1/distinfo:4: "+
+ "Expected SHA1, RMD160, SHA512, Size checksums for \"distfile-1.0.tar.gz\", got SHA512.",
+ "ERROR: ~/category/package1/distinfo:EOF: "+
+ "Expected SHA1, RMD160, SHA512, Size checksums for \"distfile-1.1.tar.gz\", got SHA512.",
"ERROR: ~/category/package2/distinfo:3: The SHA512 hash for distfile-1.0.tar.gz is 1234567822222222, "+
"which conflicts with 1234567811111111 in ../package1/distinfo:3.",
- "ERROR: ~/category/package2/distinfo:4: Expected SHA1, RMD160, SHA512, Size checksums for \"distfile-1.0.tar.gz\", got SHA512.",
- "ERROR: ~/category/package2/distinfo:EOF: Expected SHA1, RMD160, SHA512, Size checksums for \"distfile-1.1.tar.gz\", got SHA512.",
+ "ERROR: ~/category/package2/distinfo:4: "+
+ "Expected SHA1, RMD160, SHA512, Size checksums for \"distfile-1.0.tar.gz\", got SHA512.",
+ "ERROR: ~/category/package2/distinfo:5: "+
+ "Expected SHA1, RMD160, SHA512, Size checksums for \"distfile-1.1.tar.gz\", got SHA512.",
+ "ERROR: ~/category/package2/distinfo:5: "+
+ "The SHA512 hash for encoding-error.tar.gz contains a non-hex character.",
+ "ERROR: ~/category/package2/distinfo:EOF: "+
+ "Expected SHA1, RMD160, SHA512, Size checksums for \"encoding-error.tar.gz\", got SHA512.",
"WARN: ~/licenses/gnu-gpl-v2: This license seems to be unused.",
- "5 errors and 1 warning found.")
+ "7 errors and 1 warning found.")
+
+ // Ensure that hex.DecodeString does not waste memory here.
+ t.Check(len(G.Pkgsrc.Hashes["SHA512:distfile-1.0.tar.gz"].hash), equals, 8)
+ t.Check(cap(G.Pkgsrc.Hashes["SHA512:distfile-1.0.tar.gz"].hash), equals, 8)
}
func (s *Suite) Test_CheckLinesDistinfo__uncommitted_patch(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.23 pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.24
--- pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.23 Sun Jan 13 19:55:52 2019
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go Sat Jan 26 16:31:33 2019
@@ -257,7 +257,8 @@ func (s *Suite) Test_MkLineChecker_check
mklines.Check()
t.CheckOutputLines(
- "WARN: filename.mk:2: The variable DISTNAME may not be appended to (only set, given a default value) in this file.",
+ "WARN: filename.mk:2: The variable DISTNAME may not be appended to "+
+ "(only set, or given a default value) in this file.",
"WARN: filename.mk:2: The \"+=\" operator should only be used with lists, not with DISTNAME.")
}
@@ -427,7 +428,7 @@ func (s *Suite) Test_MkLineChecker_check
MkRcsID,
"COMMENT=\t${GAMES_USER}",
"COMMENT:=\t${PKGBASE}",
- "PYPKGPREFIX=${PKGBASE}")
+ "PYPKGPREFIX=\t${PKGBASE}")
G.Pkgsrc.loadDefaultBuildDefs()
G.Pkgsrc.UserDefinedVars.Define("GAMES_USER", mklines.mklines[0])
@@ -436,8 +437,85 @@ func (s *Suite) Test_MkLineChecker_check
t.CheckOutputLines(
"WARN: options.mk:3: PKGBASE should not be evaluated at load time.",
"WARN: options.mk:4: The variable PYPKGPREFIX may not be set in this file; it would be ok in pyversion.mk.",
+ "WARN: options.mk:4: PKGBASE should not be evaluated indirectly at load time.")
+}
+
+func (s *Suite) Test_MkLineChecker_checkVarusePermissions__explain(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("-Wall", "--explain")
+ t.SetUpVartypes()
+ mklines := t.NewMkLines("options.mk",
+ MkRcsID,
+ "COMMENT=\t${GAMES_USER}",
+ "COMMENT:=\t${PKGBASE}",
+ "PYPKGPREFIX=\t${PKGBASE}")
+ G.Pkgsrc.loadDefaultBuildDefs()
+ G.Pkgsrc.UserDefinedVars.Define("GAMES_USER", mklines.mklines[0])
+
+ mklines.Check()
+
+ t.CheckOutputLines(
+ "WARN: options.mk:3: PKGBASE should not be evaluated at load time.",
+ "",
+ "\tMany variables, especially lists of something, get their values",
+ "\tincrementally. Therefore it is generally unsafe to rely on their",
+ "\tvalue until it is clear that it will never change again. This point",
+ "\tis reached when the whole package Makefile is loaded and execution",
+ "\tof the shell commands starts; in some cases earlier.",
+ "",
+ "\tAdditionally, when using the \":=\" operator, each $$ is replaced with",
+ "\ta single $, so variables that have references to shell variables or",
+ "\tregular expressions are modified in a subtle way.",
+ "",
+ "WARN: options.mk:4: The variable PYPKGPREFIX may not be set in this file; it would be ok in pyversion.mk.",
+ "",
+ "\tThe allowed actions for a variable are determined based on the file",
+ "\tname in which the variable is used or defined. The rules for",
+ "\tPYPKGPREFIX are:",
+ "",
+ "\t* in pyversion.mk, it may be set",
+ "\t* in any file, it may be used at load time, or used",
+ "",
+ "\tIf these rules seem to be incorrect, please ask on the",
+ "\ttech-pkg%NetBSD.org@localhost mailing list.",
+ "",
"WARN: options.mk:4: PKGBASE should not be evaluated indirectly at load time.",
- "NOTE: options.mk:4: This variable value should be aligned to column 17.")
+ "",
+ "\tThe variable on the left-hand side may be evaluated at load time,",
+ "\tbut the variable on the right-hand side may not. Because of the",
+ "\tassignment in this line, the variable might be used indirectly at",
+ "\tload time, before it is guaranteed to be properly initialized.",
+ "")
+}
+
+func (s *Suite) Test_MkLineChecker_explainPermissions(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("-Wall", "--explain")
+ t.SetUpVartypes()
+
+ mkline := t.NewMkLine("buildlink3.mk", 123, "AUTO_MKDIRS=\tyes")
+
+ MkLineChecker{mkline}.Check()
+
+ t.CheckOutputLines(
+ "WARN: buildlink3.mk:123: The variable AUTO_MKDIRS may not be set in this file; "+
+ "it would be ok in Makefile, Makefile.* or *.mk.",
+ "",
+ "\tThe allowed actions for a variable are determined based on the file",
+ "\tname in which the variable is used or defined. The rules for",
+ "\tAUTO_MKDIRS are:",
+ "",
+ "\t* in Makefile, it may be set, or used",
+ "\t* in buildlink3.mk, it may not be accessed at all",
+ "\t* in builtin.mk, it may not be accessed at all",
+ "\t* in Makefile.*, it may be set, given a default value, or used",
+ "\t* in *.mk, it may be set, given a default value, or used",
+ "",
+ "\tIf these rules seem to be incorrect, please ask on the",
+ "\ttech-pkg%NetBSD.org@localhost mailing list.",
+ "")
}
func (s *Suite) Test_MkLineChecker_checkVarusePermissions__load_time(c *check.C) {
@@ -826,7 +904,7 @@ func (s *Suite) Test_MkLineChecker_Check
G.Check(pkg)
t.CheckOutputLines(
- "NOTE: ~/category/package/Makefile:5: The :Q operator isn't necessary for ${HOMEPAGE} here.")
+ "NOTE: ~/category/package/Makefile:6: The :Q operator isn't necessary for ${HOMEPAGE} here.")
}
// The ${VARNAME:=suffix} expression should only be used with lists.
@@ -888,7 +966,7 @@ func (s *Suite) Test_MkLineChecker_Check
guessed: true,
},
time: vucTimeRun,
- quoting: vucQuotPlain,
+ quoting: VucQuotPlain,
IsWordPart: false,
})
@@ -1029,11 +1107,13 @@ func (s *Suite) Test_MkLineChecker_check
"WARN: ~/module.mk:6: SITES_distfile.tar.gz is defined but not used.",
"WARN: ~/module.mk:6: SITES_* is deprecated. Please use SITES.* instead.",
"WARN: ~/module.mk:7: The variable PYTHON_VERSIONS_ACCEPTED may not be set "+
- "(only given a default value, appended to) in this file; it would be ok in Makefile, Makefile.common, options.mk.",
+ "(only given a default value, or appended to) in this file; "+
+ "it would be ok in Makefile, Makefile.common or options.mk.",
"WARN: ~/module.mk:7: Invalid version number \"-13\".",
"ERROR: ~/module.mk:7: All values for PYTHON_VERSIONS_ACCEPTED must be positive integers.",
"WARN: ~/module.mk:8: The variable PYTHON_VERSIONS_ACCEPTED may not be set "+
- "(only given a default value, appended to) in this file; it would be ok in Makefile, Makefile.common, options.mk.",
+ "(only given a default value, or appended to) in this file; "+
+ "it would be ok in Makefile, Makefile.common or options.mk.",
"WARN: ~/module.mk:8: The values for PYTHON_VERSIONS_ACCEPTED should be in decreasing order.")
}
Index: pkgsrc/pkgtools/pkglint/files/files_test.go
diff -u pkgsrc/pkgtools/pkglint/files/files_test.go:1.22 pkgsrc/pkgtools/pkglint/files/files_test.go:1.23
--- pkgsrc/pkgtools/pkglint/files/files_test.go:1.22 Sun Jan 13 19:55:52 2019
+++ pkgsrc/pkgtools/pkglint/files/files_test.go Sat Jan 26 16:31:33 2019
@@ -139,8 +139,8 @@ func (s *Suite) Test_convertToLogicalLin
c.Check(lines.Len(), equals, 1)
c.Check(lines.Lines[0].String(), equals, "filename:1: last line\\")
t.CheckOutputLines(
- // FIXME: linebreak is missing before ERROR.
- ">\tlast line\\ERROR: filename:1: File must end with a newline.")
+ ">\tlast line\\",
+ "ERROR: filename:1: File must end with a newline.")
}
func (s *Suite) Test_matchContinuationLine(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/mkparser.go
diff -u pkgsrc/pkgtools/pkglint/files/mkparser.go:1.22 pkgsrc/pkgtools/pkglint/files/mkparser.go:1.23
--- pkgsrc/pkgtools/pkglint/files/mkparser.go:1.22 Sun Jan 13 19:55:52 2019
+++ pkgsrc/pkgtools/pkglint/files/mkparser.go Sat Jan 26 16:31:33 2019
@@ -25,7 +25,8 @@ func (p *MkParser) Rest() string {
// NewMkParser creates a new parser for the given text.
// If emitWarnings is false, line may be nil.
//
-// TODO: Document what exactly text is. Is it the form taken from the file, or is it after unescaping "\#" to #?
+// The text argument is assumed to be after unescaping the # character,
+// which means the # is a normal character and does not introduce a Makefile comment.
//
// TODO: Remove the emitWarnings argument in order to separate parsing from checking.
func NewMkParser(line Line, text string, emitWarnings bool) *MkParser {
@@ -46,12 +47,6 @@ func (p *MkParser) MkTokens() []*MkToken
var tokens []*MkToken
for !p.EOF() {
- // FIXME: Aren't the comments already gone at this stage?
- if lexer.SkipByte('#') {
- lexer.Skip(len(lexer.Rest()))
- continue
- }
-
mark := lexer.Mark()
if varuse := p.VarUse(); varuse != nil {
tokens = append(tokens, &MkToken{Text: lexer.Since(mark), Varuse: varuse})
@@ -91,23 +86,23 @@ func (p *MkParser) VarUse() *MkVarUse {
varnameMark := lexer.Mark()
varname := p.Varname()
- if varname != "" {
- modifiers := p.VarUseModifiers(varname, closing)
- if lexer.SkipByte(closing) {
- if usingRoundParen && p.EmitWarnings {
- parenVaruse := lexer.Since(mark)
- edit := []byte(parenVaruse)
- edit[1] = '{'
- edit[len(edit)-1] = '}'
- bracesVaruse := string(edit)
-
- fix := p.Line.Autofix()
- fix.Warnf("Please use curly braces {} instead of round parentheses () for %s.", varname)
- fix.Replace(parenVaruse, bracesVaruse)
- fix.Apply()
- }
- return &MkVarUse{varname, modifiers}
+
+ modifiers := p.VarUseModifiers(varname, closing)
+ if lexer.SkipByte(closing) {
+ if usingRoundParen && p.EmitWarnings {
+ parenVaruse := lexer.Since(mark)
+ edit := []byte(parenVaruse)
+ edit[1] = '{'
+ edit[len(edit)-1] = '}'
+ bracesVaruse := string(edit)
+
+ fix := p.Line.Autofix()
+ fix.Warnf("Please use curly braces {} instead of round parentheses () for %s.", varname)
+ fix.Replace(parenVaruse, bracesVaruse)
+ fix.Apply()
}
+
+ return &MkVarUse{varname, modifiers}
}
// This code path parses ${arbitrary text :L} and ${expression :? true-branch : false-branch }.
@@ -175,7 +170,6 @@ func (p *MkParser) VarUseModifiers(varna
// The :S and :C modifiers may be chained without using the : as separator.
mayOmitColon := false
-loop:
for lexer.SkipByte(':') || mayOmitColon {
mayOmitColon = false
modifierMark := lexer.Mark()
@@ -215,7 +209,7 @@ loop:
case lexer.SkipRegexp(G.res.Compile(`^\\\d+`)):
break
default:
- break loop
+ continue
}
appendModifier(lexer.Since(modifierMark))
continue
@@ -264,15 +258,21 @@ loop:
lexer.Reset(modifierMark)
- // FIXME: Why skip over unknown modifiers here? This accepts :S,a,b,c,d,e,f but shouldn't.
re := G.res.Compile(regex.Pattern(ifelseStr(closing == '}', `^([^:$}]|\$\$)+`, `^([^:$)]|\$\$)+`)))
for p.VarUse() != nil || lexer.SkipRegexp(re) {
}
+ modifier := lexer.Since(modifierMark)
- if suffixSubst := lexer.Since(modifierMark); contains(suffixSubst, "=") {
- appendModifier(suffixSubst)
+ // ${SOURCES:%.c=%.o} or ${:!uname -a:[2]}
+ if contains(modifier, "=") || (hasPrefix(modifier, "!") && hasSuffix(modifier, "!")) {
+ appendModifier(modifier)
continue
}
+
+ if p.EmitWarnings && modifier != "" {
+ p.Line.Warnf("Invalid variable modifier %q for %q.", modifier, varname)
+ }
+
}
return modifiers
}
@@ -315,7 +315,7 @@ func (p *MkParser) varUseModifierSubst(l
return false
}
- lexer.SkipRegexp(G.res.Compile(`^[1gW]`)) // FIXME: Multiple modifiers may be mentioned
+ lexer.NextBytesFunc(func(b byte) bool { return b == '1' || b == 'g' || b == 'W' })
return true
}
@@ -506,8 +506,8 @@ func (p *MkParser) mkCondFunc() *mkCond
}
// TODO: Consider suggesting ${VAR} instead of !empty(VAR) since it is shorter and
- // avoids unnecessary negation, which makes the expression less confusing.
- // This applies especially to the ${VAR:Mpattern} form.
+ // avoids unnecessary negation, which makes the expression less confusing.
+ // This applies especially to the ${VAR:Mpattern} form.
case "commands", "exists", "make", "target":
argMark := lexer.Mark()
@@ -534,6 +534,28 @@ func (p *MkParser) Varname() string {
}
func (p *MkParser) PkgbasePattern() string {
+
+ // isVersion returns true for "1.2", "[0-9]*", "${PKGVERSION}", "${PKGNAME:C/^.*-//}",
+ // but not for "client", "${PKGNAME}", "[a-z]".
+ isVersion := func(s string) bool {
+ lexer := textproc.NewLexer(s)
+
+ lexer.SkipByte('[')
+ if lexer.NextByteSet(textproc.Digit) != -1 {
+ return true
+ }
+
+ lookaheadParser := NewMkParser(nil, lexer.Rest(), false)
+ varUse := lookaheadParser.VarUse()
+ if varUse != nil {
+ if contains(varUse.varname, "VER") || len(varUse.modifiers) > 0 {
+ return true
+ }
+ }
+
+ return false
+ }
+
lexer := p.lexer
start := lexer.Mark()
@@ -544,18 +566,7 @@ func (p *MkParser) PkgbasePattern() stri
continue
}
- lookahead := lexer.Copy()
- if !lookahead.SkipByte('-') {
- break
- }
-
- if lookahead.SkipRegexp(G.res.Compile(`^\d`)) ||
- // TODO: Replace regex with proper VarUse.
- lookahead.SkipRegexp(G.res.Compile(`^\$\{\w*VER\w*\}`)) ||
- lookahead.SkipByte('[') {
-
- // The parser is looking at a hyphen followed by a version number.
- // This means the pkgbase stops before the hyphen.
+ if lexer.PeekByte() != '-' || isVersion(lexer.Rest()[1:]) {
break
}
@@ -646,8 +657,7 @@ func (p *MkParser) Dependency() *Depende
if lexer.SkipByte('-') && lexer.Rest() != "" {
versionMark := lexer.Mark()
- // FIXME: Use VarUse.
- for lexer.SkipRegexp(G.res.Compile(`^(\$\{\w+\}|[\w\[\]*_.\-])`)) {
+ for p.VarUse() != nil || lexer.SkipRegexp(G.res.Compile(`^[\w\[\]*_.\-]+`)) {
}
if !lexer.SkipString("{,nb*}") {
Index: pkgsrc/pkgtools/pkglint/files/licenses.go
diff -u pkgsrc/pkgtools/pkglint/files/licenses.go:1.19 pkgsrc/pkgtools/pkglint/files/licenses.go:1.20
--- pkgsrc/pkgtools/pkglint/files/licenses.go:1.19 Mon Dec 17 00:15:39 2018
+++ pkgsrc/pkgtools/pkglint/files/licenses.go Sat Jan 26 16:31:33 2019
@@ -32,7 +32,7 @@ func (lc *LicenseChecker) checkName(lice
if licenseFile == "" {
licenseFile = G.Pkgsrc.File("licenses/" + license)
if G.Pkgsrc.UsedLicenses != nil {
- G.Pkgsrc.UsedLicenses[license] = true
+ G.Pkgsrc.UsedLicenses[intern(license)] = true
}
}
Index: pkgsrc/pkgtools/pkglint/files/licenses_test.go
diff -u pkgsrc/pkgtools/pkglint/files/licenses_test.go:1.20 pkgsrc/pkgtools/pkglint/files/licenses_test.go:1.21
--- pkgsrc/pkgtools/pkglint/files/licenses_test.go:1.20 Sun Jan 13 19:55:52 2019
+++ pkgsrc/pkgtools/pkglint/files/licenses_test.go Sat Jan 26 16:31:33 2019
@@ -57,9 +57,9 @@ func (s *Suite) Test_LicenseChecker_chec
G.Main("pkglint", t.File("category/package"))
- // FIXME: It should be allowed to place a license file directly into
- // the package directory.
+ // There is no warning about the unusual file name in the package directory.
+ // If it were not mentioned in LICENSE_FILE, the file named my-license
+ // would be warned about.
t.CheckOutputLines(
- "WARN: ~/category/package/my-license: Unexpected file found.",
- "0 errors and 1 warning found.")
+ "Looks fine.")
}
Index: pkgsrc/pkgtools/pkglint/files/mkparser_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.20 pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.21
--- pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.20 Sun Jan 13 19:55:52 2019
+++ pkgsrc/pkgtools/pkglint/files/mkparser_test.go Sat Jan 26 16:31:33 2019
@@ -66,12 +66,13 @@ func (s *Suite) Test_MkParser_MkTokens(c
varuse("MASTER_SITES", "=subdir/")},
"")
- // FIXME: Text must match modifiers.
testRest("${VAR:S,a,b,c,d,e,f}",
[]*MkToken{{
Text: "${VAR:S,a,b,c,d,e,f}",
Varuse: NewMkVarUse("VAR", "S,a,b,")}},
"")
+ t.CheckOutputLines(
+ "WARN: Test_MkParser_MkTokens.mk:1: Invalid variable modifier \"c,d,e,f\" for \"VAR\".")
testRest("Text${VAR:Mmodifier}${VAR2}more text${VAR3}", []*MkToken{
literal("Text"),
@@ -267,6 +268,16 @@ func (s *Suite) Test_MkParser_VarUse(c *
test("${${PKGBASE} ${PKGVERSION}:L}",
varuse("${PKGBASE} ${PKGVERSION}", "L"))
+ // The variable name is optional; the variable with the empty name always
+ // evaluates to the empty string. Bmake actively prevents this variable from
+ // ever being defined. Therefore the :U branch is always taken, and this
+ // in turn is used to implement the variables from the .for loops.
+ test("${:U}",
+ varuse("", "U"))
+
+ test("${:Ufixed value}",
+ varuse("", "Ufixed value"))
+
// This complicated expression returns the major.minor.patch version
// of the package given in ${d}.
//
@@ -522,11 +533,51 @@ func (s *Suite) Test_MkParser_VarUse__pa
"COMMENT=${P1} ${P2}) ${P3:Q} ${BRACES} $(A.$(B.${C}))")
}
+func (s *Suite) Test_MkParser_VarUseModifiers(c *check.C) {
+ t := s.Init(c)
+
+ varUse := NewMkVarUse
+ test := func(text string, varUse *MkVarUse, rest string, diagnostics ...string) {
+ mkline := t.NewMkLine("Makefile", 20, "\t"+text)
+ p := NewMkParser(mkline.Line, mkline.ShellCommand(), true)
+
+ actual := p.VarUse()
+
+ t.Check(actual, deepEquals, varUse)
+ t.Check(p.Rest(), equals, rest)
+ if len(diagnostics) > 0 {
+ t.CheckOutputLines(diagnostics...)
+ } else {
+ t.CheckOutputEmpty()
+ }
+ }
+
+ // The !command! modifier is used so seldom that pkglint does not
+ // check whether the command is actually valid.
+ // At least not while parsing the modifier since at this point it might
+ // be still unknown which of the commands can be used and which cannot.
+ test("${VAR:!command!}", varUse("VAR", "!command!"), "")
+
+ test("${VAR:!command}", varUse("VAR"), "",
+ "WARN: Makefile:20: Invalid variable modifier \"!command\" for \"VAR\".")
+
+ test("${VAR:command!}", varUse("VAR"), "",
+ "WARN: Makefile:20: Invalid variable modifier \"command!\" for \"VAR\".")
+
+ // The :L modifier makes the variable value "echo hello", and the :[1]
+ // modifier extracts the "echo".
+ test("${echo hello:L:[1]}", varUse("echo hello", "L", "[1]"), "")
+
+ // bmake ignores the :[3] modifier, and the :L modifier just returns the
+ // variable name, in this case BUILD_DIRS.
+ test("${BUILD_DIRS:[3]:L}", varUse("BUILD_DIRS", "[3]", "L"), "")
+}
+
func (s *Suite) Test_MkParser_varUseModifierSubst(c *check.C) {
t := s.Init(c)
varUse := NewMkVarUse
- test := func(text string, varUse *MkVarUse, rest string) {
+ test := func(text string, varUse *MkVarUse, rest string, diagnostics ...string) {
mkline := t.NewMkLine("Makefile", 20, "\t"+text)
p := NewMkParser(mkline.Line, mkline.ShellCommand(), true)
@@ -534,16 +585,36 @@ func (s *Suite) Test_MkParser_varUseModi
t.Check(actual, deepEquals, varUse)
t.Check(p.Rest(), equals, rest)
- t.CheckOutputEmpty()
+ if len(diagnostics) > 0 {
+ t.CheckOutputLines(diagnostics...)
+ } else {
+ t.CheckOutputEmpty()
+ }
}
- test("${VAR:S", nil, "${VAR:S") // Just for code coverage.
- test("${VAR:S}", varUse("VAR"), "") // FIXME: should not consume anything.
- test("${VAR:S,}", varUse("VAR"), "") // FIXME: should not consume anything.
- test("${VAR:S,from,to}", varUse("VAR"), "") // FIXME: should not consume anything.
+ test("${VAR:S", nil, "${VAR:S",
+ "WARN: Makefile:20: Invalid variable modifier \"S\" for \"VAR\".")
+
+ test("${VAR:S}", varUse("VAR"), "",
+ "WARN: Makefile:20: Invalid variable modifier \"S\" for \"VAR\".")
+
+ test("${VAR:S,}", varUse("VAR"), "",
+ "WARN: Makefile:20: Invalid variable modifier \"S,\" for \"VAR\".")
+
+ test("${VAR:S,from,to}", varUse("VAR"), "",
+ "WARN: Makefile:20: Invalid variable modifier \"S,from,to\" for \"VAR\".")
+
test("${VAR:S,from,to,}", varUse("VAR", "S,from,to,"), "")
+
test("${VAR:S,^from$,to,}", varUse("VAR", "S,^from$,to,"), "")
+
test("${VAR:S,@F@,${F},}", varUse("VAR", "S,@F@,${F},"), "")
+
+ test("${VAR:S,from,to,1}", varUse("VAR", "S,from,to,1"), "")
+ test("${VAR:S,from,to,g}", varUse("VAR", "S,from,to,g"), "")
+ test("${VAR:S,from,to,W}", varUse("VAR", "S,from,to,W"), "")
+
+ test("${VAR:S,from,to,1gW}", varUse("VAR", "S,from,to,1gW"), "")
}
func (s *Suite) Test_MkParser_varUseModifierAt(c *check.C) {
@@ -565,9 +636,12 @@ func (s *Suite) Test_MkParser_varUseModi
}
}
- test("${VAR:@", nil, "${VAR:@") // Just for code coverage.
+ test("${VAR:@", nil, "${VAR:@",
+ "WARN: Makefile:20: Invalid variable modifier \"@\" for \"VAR\".")
+
test("${VAR:@i@${i}}", varUse("VAR", "@i@${i}"), "",
"WARN: Makefile:20: Modifier ${VAR:@i@...@} is missing the final \"@\".")
+
test("${VAR:@i@${i}@}", varUse("VAR", "@i@${i}@"), "")
}
@@ -586,6 +660,27 @@ func (s *Suite) Test_MkParser_PkgbasePat
testRest("${PHP_PKG_PREFIX}-pdo-5.*", "${PHP_PKG_PREFIX}-pdo", "-5.*")
testRest("${PYPKGPREFIX}-metakit-[0-9]*", "${PYPKGPREFIX}-metakit", "-[0-9]*")
+ testRest("pkgbase-[0-9]*", "pkgbase", "-[0-9]*")
+
+ testRest("pkgbase-client-[0-9]*", "pkgbase-client", "-[0-9]*")
+
+ testRest("pkgbase-${VARIANT}-[0-9]*", "pkgbase-${VARIANT}", "-[0-9]*")
+
+ testRest("pkgbase-${VERSION}-[0-9]*", "pkgbase", "-${VERSION}-[0-9]*")
+
+ // This PKGNAME pattern is the one from bsd.pkg.mk.
+ // The pattern assumes that the version number does not contain a hyphen,
+ // which feels a bit too simple.
+ //
+ // Since variable substitutions are more common for version numbers
+ // than for parts of the package name, pkglint treats the PKGNAME
+ // as a version number.
+ testRest("pkgbase-${PKGNAME:C/^.*-//}-[0-9]*", "pkgbase", "-${PKGNAME:C/^.*-//}-[0-9]*")
+
+ // Using the [a-z] pattern in the package base is not seen in the wild.
+ // Therefore this rather strange parsing result is ok.
+ testRest("pkgbase-[a-z]-1.0", "pkgbase-", "[a-z]-1.0")
+
// This is a valid dependency pattern, but it's more complicated
// than the patterns pkglint can handle as of January 2019.
//
Index: pkgsrc/pkgtools/pkglint/files/substcontext.go
diff -u pkgsrc/pkgtools/pkglint/files/substcontext.go:1.20 pkgsrc/pkgtools/pkglint/files/substcontext.go:1.21
--- pkgsrc/pkgtools/pkglint/files/substcontext.go:1.20 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/substcontext.go Sat Jan 26 16:31:33 2019
@@ -1,6 +1,9 @@
package pkglint
-import "netbsd.org/pkglint/textproc"
+import (
+ "netbsd.org/pkglint/textproc"
+ "strings"
+)
// SubstContext records the state of a block of variable assignments
// that make up a SUBST class (see `mk/subst.mk`).
@@ -244,7 +247,7 @@ func (ctx *SubstContext) suggestSubstVar
tokens, _ := splitIntoShellTokens(mkline.Line, mkline.Value())
for _, token := range tokens {
- parser := NewMkParser(nil, token, false)
+ parser := NewMkParser(nil, mkline.UnquoteShell(token), false)
lexer := parser.lexer
if !lexer.SkipByte('s') {
continue
@@ -280,11 +283,19 @@ func (ctx *SubstContext) suggestSubstVar
continue
}
- mkline.Notef("The substitution command %q can be replaced with \"SUBST_VARS.%s+= %s\".", token, ctx.id, varname)
- mkline.Explain(
+ fix := mkline.Autofix()
+ fix.Notef("The substitution command %q can be replaced with \"SUBST_VARS.%s+= %s\".", token, ctx.id, varname)
+ fix.Explain(
"Replacing @VAR@ with ${VAR} is such a typical pattern that pkgsrc has built-in support for it,",
"requiring only the variable name instead of the full sed command.")
+ if mkline.VarassignComment() == "" && len(tokens) == 2 && tokens[0] == "-e" {
+ // TODO: Extract the alignment computation somewhere else, so that it is generally available.
+ alignBefore := tabWidth(mkline.ValueAlign())
+ alignAfter := tabWidth(sprintf("SUBST_VARS.%s+=\t", ctx.id))
+ tabs := strings.Repeat("\t", imax((alignAfter-alignBefore)/8, 0))
+ fix.Replace(mkline.Text, sprintf("SUBST_VARS.%s+=\t%s%s", ctx.id, tabs, varname))
+ }
+ fix.Anyway()
+ fix.Apply()
}
-
- // TODO: Autofix
}
Index: pkgsrc/pkgtools/pkglint/files/substcontext_test.go
diff -u pkgsrc/pkgtools/pkglint/files/substcontext_test.go:1.20 pkgsrc/pkgtools/pkglint/files/substcontext_test.go:1.21
--- pkgsrc/pkgtools/pkglint/files/substcontext_test.go:1.20 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/substcontext_test.go Sat Jan 26 16:31:33 2019
@@ -377,7 +377,7 @@ func (s *Suite) Test_SubstContext_sugges
"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, even when the -e is missing.
+ "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.
@@ -388,15 +388,56 @@ func (s *Suite) Test_SubstContext_sugges
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\".",
+ "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\".",
- // TODO: Handle the quotes in line 10
- // TODO: Handle the quotes in line 11
- // TODO: Handle the quotes in line 12
- "NOTE: subst.mk:13: Please always use \"-e\" in sed commands, even if there is only one substitution.")
+ "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\".")
+
+ t.SetUpCommandLine("--show-autofix")
+
+ mklines.Check()
+
+ 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: subst.mk:13: The substitution command \"s,'@SH@','${SH}',\" "+
+ "can be replaced with \"SUBST_VARS.test+= SH\".")
}
// simulateSubstLines only tests some of the inner workings of SubstContext.
Index: pkgsrc/pkgtools/pkglint/files/line.go
diff -u pkgsrc/pkgtools/pkglint/files/line.go:1.31 pkgsrc/pkgtools/pkglint/files/line.go:1.32
--- pkgsrc/pkgtools/pkglint/files/line.go:1.31 Fri Dec 21 08:05:24 2018
+++ pkgsrc/pkgtools/pkglint/files/line.go Sat Jan 26 16:31:33 2019
@@ -29,6 +29,29 @@ type RawLine struct {
func (rline *RawLine) String() string { return sprintf("%d:%s", rline.Lineno, rline.textnl) }
+type Location struct {
+ Filename string // uses / as directory separator on all platforms
+ firstLine int32 // zero means the whole file, -1 means EOF
+ lastLine int32 // usually the same as firstLine, may differ in Makefiles
+}
+
+func NewLocation(filename string, firstLine, lastLine int) Location {
+ return Location{filename, int32(firstLine), int32(lastLine)}
+}
+
+func (loc *Location) Linenos() string {
+ switch {
+ case loc.firstLine == -1:
+ return "EOF"
+ case loc.firstLine == 0:
+ return ""
+ case loc.firstLine == loc.lastLine:
+ return strconv.Itoa(int(loc.firstLine))
+ default:
+ return sprintf("%d--%d", loc.firstLine, loc.lastLine)
+ }
+}
+
// Line represents a line of text from a file.
// It aliases a pointer type to reduces the number of *Line occurrences in the code.
// Using a type alias is more efficient than an interface type, I guess.
@@ -36,14 +59,15 @@ 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
- lastLine int32 // usually the same as firstLine, may differ in Makefiles
- // without the trailing newline character;
- // in Makefiles, also contains the text from the continuation lines
- Text string
+ // But first find out where and why pkglint needs so much memory (200 MB for a full recursive run over pkgsrc + wip).
+ Location
+ Basename string // the last component of the Filename
+
+ // the text of the line, without the trailing newline character;
+ // in Makefiles, also contains the text from the continuation lines,
+ // joined by single spaces
+ Text string
+
raw []*RawLine // contains the original text including trailing newline
autofix *Autofix // any changes that pkglint would like to apply to the line
Once
@@ -58,7 +82,7 @@ func NewLine(filename string, lineno int
// NewLineMulti is for logical Makefile lines that end with backslash.
func NewLineMulti(filename string, firstLine, lastLine int, text string, rawLines []*RawLine) Line {
- return &LineImpl{filename, path.Base(filename), int32(firstLine), int32(lastLine), text, rawLines, nil, Once{}}
+ return &LineImpl{NewLocation(filename, firstLine, lastLine), path.Base(filename), text, rawLines, nil, Once{}}
}
// NewLineEOF creates a dummy line for logging, with the "line number" EOF.
@@ -71,19 +95,6 @@ func NewLineWhole(filename string) Line
return NewLineMulti(filename, 0, 0, "", nil)
}
-func (line *LineImpl) Linenos() string {
- switch {
- case line.firstLine == -1:
- return "EOF"
- case line.firstLine == 0:
- return ""
- case line.firstLine == line.lastLine:
- return strconv.Itoa(int(line.firstLine))
- default:
- return sprintf("%d--%d", line.firstLine, line.lastLine)
- }
-}
-
// RefTo returns a reference to another line,
// which can be in the same file or in a different file.
func (line *LineImpl) RefTo(other Line) string {
@@ -93,6 +104,13 @@ func (line *LineImpl) RefTo(other Line)
return "line " + other.Linenos()
}
+func (line *LineImpl) RefToLocation(other Location) string {
+ if line.Filename != other.Filename {
+ return cleanpath(relpath(path.Dir(line.Filename), other.Filename)) + ":" + other.Linenos()
+ }
+ return "line " + other.Linenos()
+}
+
// PathToFile returns the relative path from this line to the given file path.
// This is typically used for arguments in diagnostics, which should always be
// relative to the line with which the diagnostic is associated.
@@ -109,9 +127,12 @@ func (line *LineImpl) showSource(out *Se
return
}
- write := func(prefix, line string) {
+ writeLine := func(prefix, line string) {
out.Write(prefix)
out.Write(escapePrintable(line))
+ if !hasSuffix(line, "\n") {
+ out.Write("\n")
+ }
}
printDiff := func(rawLines []*RawLine) {
@@ -125,24 +146,24 @@ func (line *LineImpl) showSource(out *Se
for _, rawLine := range rawLines {
if rawLine.textnl != rawLine.orignl {
if rawLine.orignl != "" {
- write("-\t", rawLine.orignl)
+ writeLine("-\t", rawLine.orignl)
}
if rawLine.textnl != "" {
- write("+\t", rawLine.textnl)
+ writeLine("+\t", rawLine.textnl)
}
} else {
- write(prefix, rawLine.orignl)
+ writeLine(prefix, rawLine.orignl)
}
}
}
if line.autofix != nil {
for _, before := range line.autofix.linesBefore {
- write("+\t", before)
+ writeLine("+\t", before)
}
printDiff(line.raw)
for _, after := range line.autofix.linesAfter {
- write("+\t", after)
+ writeLine("+\t", after)
}
} else {
printDiff(line.raw)
Index: pkgsrc/pkgtools/pkglint/files/pkglint_test.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.31 pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.32
--- pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.31 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/pkglint_test.go Sat Jan 26 16:31:33 2019
@@ -1,6 +1,7 @@
package pkglint
import (
+ "errors"
"gopkg.in/check.v1"
"io/ioutil"
"os"
@@ -56,7 +57,6 @@ func (s *Suite) Test_Pkglint_Main__help(
" Flags for -W, --warning:",
" all all of the following",
" none none of the following",
- " absname warn about use of absolute filenames (disabled)",
" directcmd warn about use of direct command names instead of Make variables (enabled)",
" extra enable some extra warnings (disabled)",
" order warn if Makefile entries are unordered (enabled)",
@@ -1132,6 +1132,17 @@ func (s *Suite) Test_Pkglint_checkExecut
// See the "Too late" comment in Pkglint.checkExecutable.
t.CheckOutputEmpty()
}
+
+func (s *Suite) Test_Pkglint_AssertNil(c *check.C) {
+ t := s.Init(c)
+
+ G.AssertNil(nil, "this is not an error")
+
+ t.ExpectPanic(
+ func() { G.AssertNil(errors.New("unexpected error"), "Oops") },
+ "Pkglint internal error: Oops: unexpected error")
+}
+
func (s *Suite) Test_Main(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/plist_test.go
diff -u pkgsrc/pkgtools/pkglint/files/plist_test.go:1.31 pkgsrc/pkgtools/pkglint/files/plist_test.go:1.32
--- pkgsrc/pkgtools/pkglint/files/plist_test.go:1.31 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/plist_test.go Sat Jan 26 16:31:33 2019
@@ -188,6 +188,11 @@ func (s *Suite) Test_PlistChecker_checkL
"${PLIST.empty}",
"",
"$prefix/bin",
+
+ // This line does not count as a PLIST condition since it has
+ // a :Q modifier, which does not work in PLISTs. Therefore the
+ // ${PLIST.man:Q} is considered part of the filename.
+ "${PLIST.man:Q}man/cat3/strlcpy.3",
"<<<<<<<<< merge conflict")
CheckLinesPlist(lines)
@@ -197,7 +202,7 @@ func (s *Suite) Test_PlistChecker_checkL
"WARN: PLIST:4: \"bin/arm-linux-only\" should be sorted before \"bin/conditional-program\".",
"WARN: PLIST:10: PLISTs should not contain empty lines.",
"WARN: PLIST:11: PLISTs should not contain empty lines.",
- "WARN: PLIST:13: Invalid line type: <<<<<<<<< merge conflict")
+ "WARN: PLIST:14: Invalid line type: <<<<<<<<< merge conflict")
}
func (s *Suite) Test_PlistChecker_checkPathMan__gz(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/linechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/linechecker.go:1.12 pkgsrc/pkgtools/pkglint/files/linechecker.go:1.13
--- pkgsrc/pkgtools/pkglint/files/linechecker.go:1.12 Fri Dec 21 08:05:24 2018
+++ pkgsrc/pkgtools/pkglint/files/linechecker.go Sat Jan 26 16:31:33 2019
@@ -9,37 +9,6 @@ type LineChecker struct {
line Line
}
-// CheckAbsolutePathname checks whether any absolute pathnames occur in the line.
-//
-// XXX: Is this check really useful? It had been added 10 years ago because some
-// style guide said that "absolute pathnames should be avoided", but there was no
-// evidence for that.
-func (ck LineChecker) CheckAbsolutePathname(text string) {
- if trace.Tracing {
- defer trace.Call1(text)()
- }
-
- // XXX: The following code only checks the first absolute pathname per line.
- // The remaining pathnames are ignored. This is probably harmless in practice
- // since it doesn't occur often.
-
- // In the GNU coding standards, DESTDIR is defined as a (usually
- // empty) prefix that can be used to install files to a different
- // location from what they have been built for. Therefore
- // everything following it is considered an absolute pathname.
- //
- // Another context where absolute pathnames usually appear is in
- // assignments like "bindir=/bin".
- if m, path := match1(text, `(?:^|[\t ]|\$[{(]DESTDIR[)}]|[\w_]+[\t ]*=[\t ]*)(/(?:[^"' \t\\]|"[^"*]"|'[^']*')*)`); m {
- if matches(path, `^/\w`) {
-
- // XXX: Why is the "before" text from the above regular expression
- // not passed on to this method?
- ck.CheckWordAbsolutePathname(path)
- }
- }
-}
-
func (ck LineChecker) CheckLength(maxLength int) {
if len(ck.line.Text) <= maxLength {
return
@@ -85,62 +54,3 @@ func (ck LineChecker) CheckTrailingWhite
fix.Apply()
}
}
-
-// CheckWordAbsolutePathname checks the given word (which is often part of a
-// shell command) for absolute pathnames.
-//
-// XXX: Is this check really useful? It had been added 10 years ago because some
-// style guide said that "absolute pathnames should be avoided", but there was no
-// evidence for that.
-func (ck LineChecker) CheckWordAbsolutePathname(word string) {
- if trace.Tracing {
- defer trace.Call1(word)()
- }
-
- if !G.Opts.WarnAbsname {
- return
- }
-
- switch {
- case matches(word, `^/dev/(?:null|tty|zero)$`):
- // These are defined by POSIX.
-
- case matches(word, `^/dev/(?:stdin|stdout|stderr)$`):
- ck.line.Warnf("The %q file is not portable.", word)
- G.Explain(
- "The special files /dev/{stdin,stdout,stderr}, although present",
- "on Linux systems, are not available on other systems, and POSIX",
- "explicitly mentions them as examples of system-specific filenames.",
- "",
- "See https://unix.stackexchange.com/q/36403.";)
-
- case word == "/bin/sh":
- // This is usually correct, although on Solaris, it's pretty feature-crippled.
-
- case matches(word, `/s\W`):
- // Probably a sed(1) command, such as /find/s,replace,with,
-
- case matches(word, `^/(?:[a-z]|\$[({])`):
- // Absolute paths probably start with a lowercase letter.
- ck.line.Warnf("Found absolute pathname: %s", word)
- if contains(ck.line.Text, "DESTDIR") {
- G.Explain(
- "Absolute pathnames are often an indicator for unportable code.",
- "As pkgsrc aims to be a portable system,",
- "absolute pathnames should be avoided whenever possible.",
- "",
- "A special variable in this context is ${DESTDIR},",
- "which is used in GNU projects to specify a different directory",
- "for installation than what the programs see later when they are executed.",
- "Usually it is empty, so if anything after that variable starts with a slash,",
- "it is considered an absolute pathname.")
- } else {
- G.Explain(
- "Absolute pathnames are often an indicator for unportable code.",
- "As pkgsrc aims to be a portable system,",
- "absolute pathnames should be avoided whenever possible.")
-
- // TODO: Explain how to actually fix this warning properly.
- }
- }
-}
Index: pkgsrc/pkgtools/pkglint/files/linechecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/linechecker_test.go:1.13 pkgsrc/pkgtools/pkglint/files/linechecker_test.go:1.14
--- pkgsrc/pkgtools/pkglint/files/linechecker_test.go:1.13 Sun Jan 13 19:55:52 2019
+++ pkgsrc/pkgtools/pkglint/files/linechecker_test.go Sat Jan 26 16:31:33 2019
@@ -4,82 +4,6 @@ import (
"gopkg.in/check.v1"
)
-func (s *Suite) Test_LineChecker_CheckAbsolutePathname(c *check.C) {
- t := s.Init(c)
-
- t.SetUpCommandLine("-Wabsname", "--explain")
- mklines := t.NewMkLines("Makefile",
- MkRcsID,
- "\tbindir=/bin",
- "\tbindir=/../lib",
- "\tcat /dev/null",
- "\tcat /dev/tty",
- "\tcat /dev/zero",
- "\tcat /dev/stdin",
- "\tcat /dev/stdout",
- "\tcat /dev/stderr",
- "\tprintf '#! /bin/sh\\nexit 0'",
- "\tprogram=$$bindir/program",
- "\tbindir=${PREFIX}/bin",
- "\tbindir=${DESTDIR}${PREFIX}/bin",
- "\tbindir=${DESTDIR}/bin",
-
- // This is not a filename at all, but certainly looks like one.
- // Nevertheless, pkglint doesn't fall into the trap.
- "\tsed -e /usr/s/usr/var/g")
-
- mklines.ForEach(func(mkline MkLine) {
- if !mkline.IsComment() {
- LineChecker{mkline.Line}.CheckAbsolutePathname(mkline.ShellCommand())
- }
- })
-
- t.CheckOutputLines(
- "WARN: Makefile:2: Found absolute pathname: /bin",
- "",
- "\tAbsolute pathnames are often an indicator for unportable code. As",
- "\tpkgsrc aims to be a portable system, absolute pathnames should be",
- "\tavoided whenever possible.",
- "",
- "WARN: Makefile:7: The \"/dev/stdin\" file is not portable.",
- "",
- "\tThe special files /dev/{stdin,stdout,stderr}, although present on",
- "\tLinux systems, are not available on other systems, and POSIX",
- "\texplicitly mentions them as examples of system-specific filenames.",
- "",
- "\tSee https://unix.stackexchange.com/q/36403.";,
- "",
- "WARN: Makefile:8: The \"/dev/stdout\" file is not portable.",
- "WARN: Makefile:9: The \"/dev/stderr\" file is not portable.",
- "WARN: Makefile:14: Found absolute pathname: /bin",
- "",
- "\tAbsolute pathnames are often an indicator for unportable code. As",
- "\tpkgsrc aims to be a portable system, absolute pathnames should be",
- "\tavoided whenever possible.",
- "",
- "\tA special variable in this context is ${DESTDIR}, which is used in",
- "\tGNU projects to specify a different directory for installation than",
- "\twhat the programs see later when they are executed. Usually it is",
- "\tempty, so if anything after that variable starts with a slash, it is",
- "\tconsidered an absolute pathname.",
- "")
-}
-
-// It is unclear whether pkglint should check for absolute pathnames by default.
-// It might be useful, but all the code surrounding this check was added for
-// theoretical reasons instead of a practical bug. Therefore the code is still
-// there, it is just not enabled by default.
-func (s *Suite) Test_LineChecker_CheckAbsolutePathname__disabled_by_default(c *check.C) {
- t := s.Init(c)
-
- t.SetUpCommandLine( /* none, which means -Wall is suppressed */ )
- line := t.NewLine("Makefile", 1, "# dummy")
-
- LineChecker{line}.CheckAbsolutePathname("bindir=/bin")
-
- t.CheckOutputEmpty()
-}
-
func (s *Suite) Test_LineChecker_CheckLength(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/shtokenizer_test.go
diff -u pkgsrc/pkgtools/pkglint/files/shtokenizer_test.go:1.13 pkgsrc/pkgtools/pkglint/files/shtokenizer_test.go:1.14
--- pkgsrc/pkgtools/pkglint/files/shtokenizer_test.go:1.13 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/shtokenizer_test.go Sat Jan 26 16:31:33 2019
@@ -10,12 +10,12 @@ func (s *Suite) Test_ShTokenizer_ShAtom(
// testRest ensures that the given string is parsed to the expected
// atoms, and returns the remaining text.
- testRest := func(s string, expected ...*ShAtom) string {
+ testRest := func(s string, expectedAtoms ...*ShAtom) string {
p := NewShTokenizer(dummyLine, s, false)
q := shqPlain
- for _, exp := range expected {
- c.Check(p.ShAtom(q), deepEquals, exp)
- q = exp.Quoting
+ for _, expectedAtom := range expectedAtoms {
+ c.Check(p.ShAtom(q), deepEquals, expectedAtom)
+ q = expectedAtom.Quoting
}
return p.Rest()
}
Index: pkgsrc/pkgtools/pkglint/files/shtypes.go
diff -u pkgsrc/pkgtools/pkglint/files/shtypes.go:1.13 pkgsrc/pkgtools/pkglint/files/shtypes.go:1.14
--- pkgsrc/pkgtools/pkglint/files/shtypes.go:1.13 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/shtypes.go Sat Jan 26 16:31:33 2019
@@ -100,18 +100,18 @@ func (q ShQuoting) String() string {
}[q]
}
-func (q ShQuoting) ToVarUseContext() vucQuoting {
+func (q ShQuoting) ToVarUseContext() VucQuoting {
switch q {
case shqPlain:
- return vucQuotPlain
+ return VucQuotPlain
case shqDquot:
- return vucQuotDquot
+ return VucQuotDquot
case shqSquot:
- return vucQuotSquot
+ return VucQuotSquot
case shqBackt:
- return vucQuotBackt
+ return VucQuotBackt
}
- return vucQuotUnknown
+ return VucQuotUnknown
}
// ShToken is an operator or a keyword or some text intermingled with variables.
Index: pkgsrc/pkgtools/pkglint/files/lines.go
diff -u pkgsrc/pkgtools/pkglint/files/lines.go:1.4 pkgsrc/pkgtools/pkglint/files/lines.go:1.5
--- pkgsrc/pkgtools/pkglint/files/lines.go:1.4 Mon Dec 17 00:15:39 2018
+++ pkgsrc/pkgtools/pkglint/files/lines.go Sat Jan 26 16:31:33 2019
@@ -31,8 +31,8 @@ func (ls *LinesImpl) Warnf(format string
NewLineWhole(ls.FileName).Warnf(format, args...)
}
-func (ls *LinesImpl) SaveAutofixChanges() {
- SaveAutofixChanges(ls)
+func (ls *LinesImpl) SaveAutofixChanges() bool {
+ return SaveAutofixChanges(ls)
}
func (ls *LinesImpl) CheckRcsID(index int, prefixRe regex.Pattern, suggestedPrefix string) bool {
@@ -43,13 +43,13 @@ func (ls *LinesImpl) CheckRcsID(index in
line := ls.Lines[index]
if m, expanded := match1(line.Text, `^`+prefixRe+`\$`+`NetBSD(:[^\$]+)?\$$`); m {
- if G.Wip && expanded != "" {
+ if G.Testing && G.Wip && expanded != "" {
fix := line.Autofix()
fix.Notef("Expected exactly %q.", suggestedPrefix+"$"+"NetBSD$")
fix.Explain(
- "Several files in pkgsrc must contain the CVS Id, so that their",
+ "Most files in pkgsrc contain the CVS Id, so that their",
"current version can be traced back later from a binary package.",
- "This is to ensure reproducible builds, for example for finding bugs.",
+ "This is to ensure reproducible builds and for reliably locating bugs.",
"",
"These CVS Ids are specific to the CVS version control system,",
"and pkgsrc-wip uses Git instead.",
Index: pkgsrc/pkgtools/pkglint/files/lines_test.go
diff -u pkgsrc/pkgtools/pkglint/files/lines_test.go:1.6 pkgsrc/pkgtools/pkglint/files/lines_test.go:1.7
--- pkgsrc/pkgtools/pkglint/files/lines_test.go:1.6 Sun Jan 13 19:55:52 2019
+++ pkgsrc/pkgtools/pkglint/files/lines_test.go Sat Jan 26 16:31:33 2019
@@ -31,8 +31,7 @@ func (s *Suite) Test_Lines_CheckRcsID__w
t := s.Init(c)
t.SetUpPkgsrc()
- t.SetUpPackage("wip/package",
- "CATEGORIES=\tchinese")
+ t.SetUpPackage("wip/package")
t.CreateFileLines("wip/package/file1.mk",
"# $"+"NetBSD: dummy $")
t.CreateFileLines("wip/package/file2.mk",
Index: pkgsrc/pkgtools/pkglint/files/mktypes_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mktypes_test.go:1.6 pkgsrc/pkgtools/pkglint/files/mktypes_test.go:1.7
--- pkgsrc/pkgtools/pkglint/files/mktypes_test.go:1.6 Mon Dec 17 00:15:39 2018
+++ pkgsrc/pkgtools/pkglint/files/mktypes_test.go Sat Jan 26 16:31:33 2019
@@ -51,6 +51,3 @@ func (s *Suite) Test_MkVarUseModifier_Ma
c.Check(to, equals, "\\:")
c.Check(options, equals, "")
}
-
-// TODO: Add test for :L in the middle of a MkVarUse.
-// TODO: Add test for :L at the end of a MkVarUse.
Index: pkgsrc/pkgtools/pkglint/files/logging_test.go
diff -u pkgsrc/pkgtools/pkglint/files/logging_test.go:1.10 pkgsrc/pkgtools/pkglint/files/logging_test.go:1.11
--- pkgsrc/pkgtools/pkglint/files/logging_test.go:1.10 Sun Jan 13 19:55:52 2019
+++ pkgsrc/pkgtools/pkglint/files/logging_test.go Sat Jan 26 16:31:33 2019
@@ -186,7 +186,7 @@ func (s *Suite) Test__show_source_separa
// output lines.
//
// TODO: Giving the diagnostics again would be useful, but the warning and
-// error counters should not be affected, as well as the exitcode.
+// error counters should not be affected, as well as the exitcode.
func (s *Suite) Test__show_source_separator_autofix(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/mkshparser_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkshparser_test.go:1.10 pkgsrc/pkgtools/pkglint/files/mkshparser_test.go:1.11
--- pkgsrc/pkgtools/pkglint/files/mkshparser_test.go:1.10 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/mkshparser_test.go Sat Jan 26 16:31:33 2019
@@ -288,8 +288,12 @@ func (s *ShSuite) Test_ShellParser__comp
func (s *ShSuite) Test_ShellParser__term(c *check.C) {
b := s.init(c)
- // TODO
- _ = b
+ s.test("echo1 ; echo2 ;",
+ b.List().
+ AddCommand(b.SimpleCommand("echo1")).
+ AddSemicolon().
+ AddCommand(b.SimpleCommand("echo2")).
+ AddSemicolon())
}
func (s *ShSuite) Test_ShellParser__for_clause(c *check.C) {
@@ -423,8 +427,16 @@ func (s *ShSuite) Test_ShellParser__unti
func (s *ShSuite) Test_ShellParser__function_definition(c *check.C) {
b := s.init(c)
- // TODO
- _ = b
+ s.test("fn() { simple-command; }",
+ b.List().AddCommand(b.Function(
+ "fn",
+ b.Brace(b.List().
+ AddCommand(b.SimpleCommand("simple-command")).
+ AddSemicolon()).
+ Compound)))
+
+ // For some reason the POSIX grammar does not allow function bodies that consist of
+ // a single command without braces or parentheses.
}
func (s *ShSuite) Test_ShellParser__brace_group(c *check.C) {
@@ -522,10 +534,8 @@ func (s *ShSuite) Test_ShellParser__io_r
}
func (s *ShSuite) Test_ShellParser__io_here(c *check.C) {
- b := s.init(c)
-
- // TODO
- _ = b
+ // In pkgsrc Makefiles, the IO here-documents cannot be used since all the text
+ // is joined into a single line. Therefore there are no tests here.
}
func (s *ShSuite) init(c *check.C) *MkShBuilder {
@@ -607,6 +617,48 @@ func (s *ShSuite) Test_ShellLexer_Lex__r
c.Check(lexer.Lex(&llval), equals, 0)
}
+func (s *ShSuite) Test_ShellLexer_Lex__keywords(c *check.C) {
+ b := s.init(c)
+
+ testErr := func(program, error, remaining string) {
+ tokens, rest := splitIntoShellTokens(dummyLine, program)
+ s.c.Check(rest, equals, "")
+
+ lexer := ShellLexer{
+ current: "",
+ remaining: tokens,
+ atCommandStart: true,
+ error: ""}
+ parser := shyyParserImpl{}
+
+ succeeded := parser.Parse(&lexer)
+
+ c.Check(succeeded, equals, 1)
+ c.Check(lexer.error, equals, error)
+ c.Check(joinSkipEmpty(" ", append([]string{lexer.current}, lexer.remaining...)...), equals, remaining)
+ }
+
+ s.test(
+ "echo if then elif else fi for in do done while until case esac",
+ b.List().AddCommand(b.SimpleCommand("echo",
+ "if", "then", "elif", "else", "fi",
+ "for", "in", "do", "done", "while", "until", "case", "esac")))
+
+ testErr(
+ "echo ;; remaining",
+ "syntax error",
+ ";; remaining")
+
+ testErr(
+ "echo (subshell-command args)",
+ "syntax error",
+ "subshell-command args )")
+
+ testErr("while :; do :; done if cond; then :; fi",
+ "syntax error",
+ "if cond ; then : ; fi")
+}
+
type MkShBuilder struct {
}
Index: pkgsrc/pkgtools/pkglint/files/options.go
diff -u pkgsrc/pkgtools/pkglint/files/options.go:1.10 pkgsrc/pkgtools/pkglint/files/options.go:1.11
--- pkgsrc/pkgtools/pkglint/files/options.go:1.10 Fri Dec 21 08:05:24 2018
+++ pkgsrc/pkgtools/pkglint/files/options.go Sat Jan 26 16:31:33 2019
@@ -7,11 +7,11 @@ func CheckLinesOptionsMk(mklines MkLines
mklines.Check()
- exp := NewMkExpecter(mklines)
- exp.AdvanceWhile(func(mkline MkLine) bool { return mkline.IsComment() || mkline.IsEmpty() })
+ mlex := NewMkLinesLexer(mklines)
+ mlex.SkipWhile(func(mkline MkLine) bool { return mkline.IsComment() || mkline.IsEmpty() })
- if exp.EOF() || !(exp.CurrentMkLine().IsVarassign() && exp.CurrentMkLine().Varname() == "PKG_OPTIONS_VAR") {
- exp.CurrentLine().Warnf("Expected definition of PKG_OPTIONS_VAR.")
+ if mlex.EOF() || !(mlex.CurrentMkLine().IsVarassign() && mlex.CurrentMkLine().Varname() == "PKG_OPTIONS_VAR") {
+ mlex.CurrentLine().Warnf("Expected definition of PKG_OPTIONS_VAR.")
G.Explain(
"The input variables in an options.mk file should always be",
"mentioned in the same order: PKG_OPTIONS_VAR,",
@@ -19,15 +19,15 @@ func CheckLinesOptionsMk(mklines MkLines
"This way, the options.mk files have the same structure and are easy to understand.")
return
}
- exp.Advance()
+ mlex.Skip()
declaredOptions := make(map[string]MkLine)
handledOptions := make(map[string]MkLine)
var optionsInDeclarationOrder []string
loop:
- for ; !exp.EOF(); exp.Advance() {
- mkline := exp.CurrentMkLine()
+ for ; !mlex.EOF(); mlex.Skip() {
+ mkline := mlex.CurrentMkLine()
switch {
case mkline.IsComment():
break
@@ -50,12 +50,12 @@ loop:
case mkline.IsInclude():
if mkline.IncludedFile() == "../../mk/bsd.options.mk" {
- exp.Advance()
+ mlex.Skip()
break loop
}
default:
- exp.CurrentLine().Warnf("Expected inclusion of \"../../mk/bsd.options.mk\".")
+ mlex.CurrentLine().Warnf("Expected inclusion of \"../../mk/bsd.options.mk\".")
G.Explain(
"After defining the input variables (PKG_OPTIONS_VAR, etc.),",
"bsd.options.mk should be included to do the actual processing.",
@@ -65,8 +65,8 @@ loop:
}
}
- for ; !exp.EOF(); exp.Advance() {
- mkline := exp.CurrentMkLine()
+ for ; !mlex.EOF(); mlex.Skip() {
+ mkline := mlex.CurrentMkLine()
if mkline.IsDirective() && (mkline.Directive() == "if" || mkline.Directive() == "elif") {
cond := mkline.Cond()
if cond == nil {
@@ -101,14 +101,14 @@ loop:
declared := declaredOptions[option]
handled := handledOptions[option]
- if declared != nil && handled == nil {
+ if handled == nil {
declared.Warnf("Option %q should be handled below in an .if block.", option)
G.Explain(
"If an option is not processed in this file, it may either be a",
"typo, or the option does not have any effect.")
}
- if declared == nil && handled != nil {
+ if declared == nil {
handled.Warnf("Option %q is handled but not added to PKG_SUPPORTED_OPTIONS.", option)
G.Explain(
"This block of code will never be run since PKG_OPTIONS cannot",
Index: pkgsrc/pkgtools/pkglint/files/mkline.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline.go:1.44 pkgsrc/pkgtools/pkglint/files/mkline.go:1.45
--- pkgsrc/pkgtools/pkglint/files/mkline.go:1.44 Sun Jan 13 19:55:52 2019
+++ pkgsrc/pkgtools/pkglint/files/mkline.go Sat Jan 26 16:31:33 2019
@@ -334,27 +334,27 @@ func (mkline *MkLineImpl) SetConditional
mkline.data = include
}
-// Tokenize extracts variable uses and other text from the string.
+// Tokenize extracts variable uses and other text from the given text.
//
-// TODO: Check this paragraph for correctness.
-// Either:
-// The given s must have exactly the format from the file, i.e. an escaped
-// comment is written as \#.
-// Or:
-// The given s must have the format after parsing comments, i.e. the trailing
-// comment is already removed, and a # does not introduce another comment.
+// When used in IsVarassign lines, the given text must have the format
+// after stripping the end-of-line comment. Such text is available from
+// Value. A shell comment is therefore marked by a simple #, not an escaped
+// \# like in Makefiles.
+//
+// When used in IsShellCommand lines, # does not mark a Makefile comment
+// and may thus still appear in the text. Therefore, # marks a shell comment.
//
// Example:
// input: ${PREFIX}/bin abc
// output: [MkToken("${PREFIX}", MkVarUse("PREFIX")), MkToken("/bin abc")]
//
// See ValueTokens, which is the tokenized version of Value.
-func (mkline *MkLineImpl) Tokenize(s string, warn bool) []*MkToken {
+func (mkline *MkLineImpl) Tokenize(text string, warn bool) []*MkToken {
if trace.Tracing {
- defer trace.Call(mkline, s)()
+ defer trace.Call(mkline, text)()
}
- p := NewMkParser(mkline.Line, s, true)
+ p := NewMkParser(mkline.Line, text, true)
tokens := p.MkTokens()
if warn && p.Rest() != "" {
mkline.Warnf("Internal pkglint error in MkLine.Tokenize at %q.", p.Rest())
@@ -673,15 +673,8 @@ func (mkline *MkLineImpl) VariableNeedsQ
}
}
- // In .for loops, the :Q operator is always misplaced, since
- // the items are broken up at whitespace, not as shell words
- // like in all other parts of make(1).
- if vuc.quoting == vucQuotFor {
- return no
- }
-
// A shell word may appear as part of a shell word, for example COMPILER_RPATH_FLAG.
- if vuc.IsWordPart && vuc.quoting == vucQuotPlain {
+ if vuc.IsWordPart && vuc.quoting == VucQuotPlain {
if vartype.kindOfList == lkNone && vartype.basicType == BtShellWord {
return no
}
@@ -697,7 +690,7 @@ func (mkline *MkLineImpl) VariableNeedsQ
// Both of these can be correct, depending on the situation:
// 1. echo ${PERL5:Q}
// 2. xargs ${PERL5}
- if !vuc.IsWordPart && vuc.quoting == vucQuotPlain {
+ if !vuc.IsWordPart && vuc.quoting == VucQuotPlain {
if wantList && haveList {
return unknown
}
@@ -707,14 +700,14 @@ func (mkline *MkLineImpl) VariableNeedsQ
// special characters, so they can safely be used inside any quotes.
if tool := G.ToolByVarname(varname); tool != nil {
switch vuc.quoting {
- case vucQuotPlain:
+ case VucQuotPlain:
if !vuc.IsWordPart {
return no
}
// XXX: Should there be a return here? It looks as if it could have been forgotten.
- case vucQuotBackt:
+ case VucQuotBackt:
return no
- case vucQuotDquot, vucQuotSquot:
+ case VucQuotDquot, VucQuotSquot:
return unknown
}
}
@@ -723,7 +716,7 @@ func (mkline *MkLineImpl) VariableNeedsQ
//
// An exception is in the case of backticks, because the whole backticks expression
// is parsed as a single shell word by pkglint. (XXX: This comment may be outdated.)
- if vuc.IsWordPart && vucVartype.IsShell() && vuc.quoting != vucQuotBackt {
+ if vuc.IsWordPart && vucVartype.IsShell() && vuc.quoting != VucQuotBackt {
return yes
}
@@ -815,6 +808,47 @@ func (mkline *MkLineImpl) DetermineUsedV
return varnames
}
+func (mkline *MkLineImpl) UnquoteShell(str string) string {
+ var sb strings.Builder
+ n := len(str)
+
+outer:
+ for i := 0; i < n; i++ {
+ switch str[i] {
+ case '"':
+ for i++; i < n; i++ {
+ switch str[i] {
+ case '"':
+ continue outer
+ case '\\':
+ i++
+ if i < n {
+ sb.WriteByte(str[i])
+ }
+ default:
+ sb.WriteByte(str[i])
+ }
+ }
+
+ case '\'':
+ for i++; i < n && str[i] != '\''; i++ {
+ sb.WriteByte(str[i])
+ }
+
+ case '\\':
+ i++
+ if i < n {
+ sb.WriteByte(str[i])
+ }
+
+ default:
+ sb.WriteByte(str[i])
+ }
+ }
+
+ return sb.String()
+}
+
type MkOperator uint8
const (
@@ -864,7 +898,7 @@ func (op MkOperator) String() string {
type VarUseContext struct {
vartype *Vartype
time vucTime
- quoting vucQuoting
+ quoting VucQuoting
IsWordPart bool // Example: LOCALBASE=${LOCALBASE}
}
@@ -892,29 +926,22 @@ const (
func (t vucTime) String() string { return [...]string{"unknown", "parse", "run"}[t] }
-// The quoting context in which the variable is used.
+// VucQuoting describes in what level of quoting the variable is used.
// Depending on this context, the modifiers :Q or :M can be allowed or not.
//
// The shell tokenizer knows multi-level quoting modes (see ShQuoting),
// but for deciding whether :Q is necessary or not, a single level is enough.
-type vucQuoting uint8
+type VucQuoting uint8
const (
- vucQuotUnknown vucQuoting = iota
- vucQuotPlain // Example: echo LOCALBASE=${LOCALBASE}
- vucQuotDquot // Example: echo "The version is ${PKGVERSION}."
- vucQuotSquot // Example: echo 'The version is ${PKGVERSION}.'
- vucQuotBackt // Example: echo `sed 1q ${WRKSRC}/README`
-
- // The .for loop in Makefiles. This is the only place where
- // variables are split on whitespace. Everywhere else (:Q, :M)
- // they are split like in the shell.
- //
- // Example: .for f in ${EXAMPLE_FILES}
- vucQuotFor
+ VucQuotUnknown VucQuoting = iota
+ VucQuotPlain // Example: echo LOCALBASE=${LOCALBASE}
+ VucQuotDquot // Example: echo "The version is ${PKGVERSION}."
+ VucQuotSquot // Example: echo 'The version is ${PKGVERSION}.'
+ VucQuotBackt // Example: echo `sed 1q ${WRKSRC}/README`
)
-func (q vucQuoting) String() string {
+func (q VucQuoting) String() string {
return [...]string{"unknown", "plain", "dquot", "squot", "backt", "mk-for"}[q]
}
Index: pkgsrc/pkgtools/pkglint/files/mkline_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.49 pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.50
--- pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.49 Sun Jan 13 19:55:52 2019
+++ pkgsrc/pkgtools/pkglint/files/mkline_test.go Sat Jan 26 16:31:33 2019
@@ -229,7 +229,7 @@ func (s *Suite) Test_VarUseContext_Strin
t.SetUpVartypes()
vartype := G.Pkgsrc.VariableType("PKGNAME")
- vuc := VarUseContext{vartype, vucTimeUnknown, vucQuotBackt, false}
+ vuc := VarUseContext{vartype, vucTimeUnknown, VucQuotBackt, false}
c.Check(vuc.String(), equals, "(Pkgname time:unknown quoting:backt wordpart:false)")
}
@@ -320,7 +320,7 @@ func (s *Suite) Test_MkLine_VariableNeed
mkline := t.NewMkLine("filename", 1, "PKGNAME:= ${UNKNOWN}")
t.SetUpVartypes()
- vuc := VarUseContext{G.Pkgsrc.VariableType("PKGNAME"), vucTimeParse, vucQuotUnknown, false}
+ vuc := VarUseContext{G.Pkgsrc.VariableType("PKGNAME"), vucTimeParse, VucQuotUnknown, false}
nq := mkline.VariableNeedsQuoting("UNKNOWN", nil, &vuc)
c.Check(nq, equals, unknown)
@@ -333,7 +333,7 @@ func (s *Suite) Test_MkLine_VariableNeed
t.SetUpMasterSite("MASTER_SITE_SOURCEFORGE", "http://downloads.sourceforge.net/sourceforge/";)
mkline := t.NewMkLine("Makefile", 95, "MASTER_SITES=\t${HOMEPAGE}")
- vuc := VarUseContext{G.Pkgsrc.vartypes["MASTER_SITES"], vucTimeRun, vucQuotPlain, false}
+ vuc := VarUseContext{G.Pkgsrc.vartypes["MASTER_SITES"], vucTimeRun, VucQuotPlain, false}
nq := mkline.VariableNeedsQuoting("HOMEPAGE", G.Pkgsrc.vartypes["HOMEPAGE"], &vuc)
c.Check(nq, equals, no)
@@ -804,8 +804,12 @@ func (s *Suite) Test_MkLine_ValueSplit(c
c.Check(split, deepEquals, expected)
}
- test("#empty",
- []string(nil)...)
+ test("Platform-independent C# compiler #5",
+ "Platform-independent C# compiler #5")
+
+ // This warning refers to the #5 since it starts a word, but not to the C#.
+ t.CheckOutputLines(
+ "WARN: Makefile:1: The # character starts a Makefile comment.")
test("/bin",
"/bin")
@@ -1171,6 +1175,35 @@ func (s *Suite) Test_MkLine_DetermineUse
"x"})
}
+func (s *Suite) Test_MkLine_UnquoteShell(c *check.C) {
+ t := s.Init(c)
+
+ test := func(input, output string) {
+ unquoted := (*MkLineImpl).UnquoteShell(nil, input)
+ t.Check(unquoted, equals, output)
+ }
+
+ test("", "")
+ test("plain", "plain")
+ test("plain words", "plain words")
+ test("\"dquot\"", "dquot")
+ test("\"dquot \\\"escaped\\\\\"", "dquot \"escaped\\")
+ test("'squot \\\"escaped\\\\'", "squot \\\"escaped\\\\")
+ test("'squot,''squot'", "squot,squot")
+ test("\"dquot,\"'squot'", "dquot,squot")
+ test("\"'\",'\"'", "',\"")
+ test("\\\" \\\\", "\" \\")
+
+ // UnquoteShell does not parse shell variable expansions or even subshells.
+ // It therefore must cope with unexpected input and make the best out of it.
+
+ test("\\", "")
+ test("\"\\", "")
+ test("'", "")
+ test("\"$(\"", "$(")
+ test("`", "`")
+}
+
func (s *Suite) Test_matchMkDirective(c *check.C) {
test := func(input, expectedIndent, expectedDirective, expectedArgs, expectedComment string) {
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.27 pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.28
--- pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.27 Sun Jan 13 19:55:52 2019
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker.go Sat Jan 26 16:31:33 2019
@@ -220,7 +220,7 @@ func (ck MkLineChecker) checkDirectiveFo
// running pkglint over the whole pkgsrc tree did not produce any different result
// whether guessed was true or false.
forLoopType := Vartype{lkShell, BtUnknown, []ACLEntry{{"*", aclpAllRead}}, false}
- forLoopContext := VarUseContext{&forLoopType, vucTimeParse, vucQuotFor, false}
+ forLoopContext := VarUseContext{&forLoopType, vucTimeParse, VucQuotPlain, false}
for _, itemsVar := range mkline.DetermineUsedVariables() {
ck.CheckVaruse(&MkVarUse{itemsVar, nil}, &forLoopContext)
}
@@ -349,13 +349,42 @@ func (ck MkLineChecker) checkVarassignLe
mkline.Warnf("The variable %s may not be %s by any package.",
varname, needed.HumanString())
}
- G.Explain(
- "The allowed actions for a variable are determined based on the file",
- "name in which the variable is used or defined.",
- // FIXME: List the rules in this very explanation.
- "The exact rules are hard-coded into pkglint.",
- "If they seem to be incorrect, please ask on the tech-pkg%NetBSD.org@localhost mailing list.")
+ ck.explainPermissions(varname, vartype)
+ }
+}
+
+func (ck MkLineChecker) explainPermissions(varname string, vartype *Vartype) {
+ if !G.Logger.Opts.Explain {
+ return
+ }
+
+ var expl []string
+ expl = append(expl,
+ "The allowed actions for a variable are determined based on the file",
+ "name in which the variable is used or defined.",
+ sprintf("The rules for %s are:", varname),
+ "")
+
+ for _, rule := range vartype.aclEntries {
+ perms := rule.permissions.HumanString()
+
+ files := rule.glob
+ if files == "*" {
+ files = "any file"
+ }
+
+ if perms != "" {
+ expl = append(expl, sprintf("* in %s, it may be %s", files, perms))
+ } else {
+ expl = append(expl, sprintf("* in %s, it may not be accessed at all", files))
+ }
}
+
+ expl = append(expl,
+ "",
+ "If these rules seem to be incorrect, please ask on the tech-pkg%NetBSD.org@localhost mailing list.")
+
+ G.Explain(expl...)
}
// CheckVaruse checks a single use of a variable in a specific context.
@@ -393,7 +422,7 @@ func (ck MkLineChecker) CheckVaruse(varu
needsQuoting := mkline.VariableNeedsQuoting(varname, vartype, vuc)
- if G.Opts.WarnQuoting && vuc.quoting != vucQuotUnknown && needsQuoting != unknown {
+ if G.Opts.WarnQuoting && vuc.quoting != VucQuotUnknown && needsQuoting != unknown {
// FIXME: Why "Shellword" when there's no indication that this is actually a shell type?
// It's for splitting the value into tokens, taking "double" and 'single' quotes into account.
ck.CheckVaruseShellword(varname, vartype, vuc, varuse.Mod(), needsQuoting)
@@ -558,12 +587,8 @@ func (ck MkLineChecker) checkVarusePermi
} else {
mkline.Warnf("%s may not be used in any file; it is a write-only variable.", varname)
}
- G.Explain(
- "The allowed actions for a variable are determined based on the file",
- "name in which the variable is used or defined.",
- // FIXME: List the rules in this very explanation.
- "The exact rules are hard-coded into pkglint.",
- "If they seem to be incorrect, please ask on the tech-pkg%NetBSD.org@localhost mailing list.")
+
+ ck.explainPermissions(varname, vartype)
}
}
@@ -689,7 +714,7 @@ func (ck MkLineChecker) CheckVaruseShell
}
} else if mod != correctMod {
- if vuc.quoting == vucQuotPlain {
+ if vuc.quoting == VucQuotPlain {
fix := mkline.Autofix()
fix.Warnf("Please use ${%s%s} instead of ${%s%s}.", varname, correctMod, varname, mod)
fix.Explain(
@@ -712,7 +737,7 @@ func (ck MkLineChecker) CheckVaruseShell
seeGuide("Echoing a string exactly as-is", "echo-literal"))
}
- } else if vuc.quoting != vucQuotPlain {
+ } else if vuc.quoting != VucQuotPlain {
mkline.Warnf("Please move ${%s%s} outside of any quoting characters.", varname, mod)
mkline.Explain(
"The :Q modifier only works reliably when it is used outside of any",
@@ -908,7 +933,7 @@ func (ck MkLineChecker) checkTextVarUse(
spaceLeft := i-1 < 0 || matches(tokens[i-1].Text, `[\t ]$`)
spaceRight := i+1 >= len(tokens) || matches(tokens[i+1].Text, `^[\t ]`)
isWordPart := !(spaceLeft && spaceRight)
- vuc := VarUseContext{vartype, time, vucQuotPlain, isWordPart}
+ vuc := VarUseContext{vartype, time, VucQuotPlain, isWordPart}
ck.CheckVaruse(token.Varuse, &vuc)
}
}
@@ -1160,7 +1185,7 @@ func (ck MkLineChecker) checkDirectiveCo
checkVarUse := func(varuse *MkVarUse) {
var vartype *Vartype // TODO: Insert a better type guess here.
- vuc := VarUseContext{vartype, vucTimeParse, vucQuotPlain, false}
+ vuc := VarUseContext{vartype, vucTimeParse, VucQuotPlain, false}
ck.CheckVaruse(varuse, &vuc)
}
Index: pkgsrc/pkgtools/pkglint/files/patches.go
diff -u pkgsrc/pkgtools/pkglint/files/patches.go:1.27 pkgsrc/pkgtools/pkglint/files/patches.go:1.28
--- pkgsrc/pkgtools/pkglint/files/patches.go:1.27 Fri Dec 21 08:05:24 2018
+++ pkgsrc/pkgtools/pkglint/files/patches.go Sat Jan 26 16:31:33 2019
@@ -12,12 +12,12 @@ func CheckLinesPatch(lines Lines) {
defer trace.Call1(lines.FileName)()
}
- (&PatchChecker{lines, NewExpecter(lines), false, false}).Check()
+ (&PatchChecker{lines, NewLinesLexer(lines), false, false}).Check()
}
type PatchChecker struct {
lines Lines
- exp *Expecter
+ llex *LinesLexer
seenDocumentation bool
previousLineEmpty bool
}
@@ -34,53 +34,53 @@ func (ck *PatchChecker) Check() {
}
if ck.lines.CheckRcsID(0, ``, "") {
- ck.exp.Advance()
+ ck.llex.Skip()
}
- if ck.exp.EOF() {
+ if ck.llex.EOF() {
ck.lines.Lines[0].Errorf("Patch files must not be empty.")
return
}
- ck.previousLineEmpty = ck.exp.ExpectEmptyLine()
+ ck.previousLineEmpty = ck.llex.SkipEmptyOrNote()
patchedFiles := 0
- for !ck.exp.EOF() {
- line := ck.exp.CurrentLine()
- if ck.exp.AdvanceIfMatches(rePatchUniFileDel) {
- if ck.exp.AdvanceIfMatches(rePatchUniFileAdd) {
+ for !ck.llex.EOF() {
+ line := ck.llex.CurrentLine()
+ if ck.llex.SkipRegexp(rePatchUniFileDel) {
+ if m := ck.llex.NextRegexp(rePatchUniFileAdd); m != nil {
ck.checkBeginDiff(line, patchedFiles)
- ck.checkUnifiedDiff(ck.exp.Group(1))
+ ck.checkUnifiedDiff(m[1])
patchedFiles++
continue
}
- ck.exp.StepBack()
+ ck.llex.Undo()
}
- if ck.exp.AdvanceIfMatches(rePatchUniFileAdd) {
- patchedFile := ck.exp.Group(1)
- if ck.exp.AdvanceIfMatches(rePatchUniFileDel) {
+ if m := ck.llex.NextRegexp(rePatchUniFileAdd); m != nil {
+ patchedFile := m[1]
+ if ck.llex.SkipRegexp(rePatchUniFileDel) {
ck.checkBeginDiff(line, patchedFiles)
- ck.exp.PreviousLine().Warnf("Unified diff headers should be first ---, then +++.")
+ ck.llex.PreviousLine().Warnf("Unified diff headers should be first ---, then +++.")
ck.checkUnifiedDiff(patchedFile)
patchedFiles++
continue
}
- ck.exp.StepBack()
+ ck.llex.Undo()
}
- if ck.exp.AdvanceIfMatches(`^\*\*\*[\t ]([^\t ]+)(.*)$`) {
- if ck.exp.AdvanceIfMatches(`^---[\t ]([^\t ]+)(.*)$`) {
+ if ck.llex.SkipRegexp(`^\*\*\*[\t ]([^\t ]+)(.*)$`) {
+ if ck.llex.SkipRegexp(`^---[\t ]([^\t ]+)(.*)$`) {
ck.checkBeginDiff(line, patchedFiles)
line.Warnf("Please use unified diffs (diff -u) for patches.")
return
}
- ck.exp.StepBack()
+ ck.llex.Undo()
}
- ck.exp.Advance()
+ ck.llex.Skip()
ck.previousLineEmpty = ck.isEmptyLine(line.Text)
if !ck.previousLineEmpty {
ck.seenDocumentation = true
@@ -115,20 +115,26 @@ func (ck *PatchChecker) checkUnifiedDiff
}
hasHunks := false
- for ck.exp.AdvanceIfMatches(rePatchUniHunk) {
- text := ck.exp.m[0]
+ for {
+ m := ck.llex.NextRegexp(rePatchUniHunk)
+ if m == nil {
+ break
+ }
+
+ text := m[0]
hasHunks = true
- linesToDel := toInt(ck.exp.Group(2), 1)
- linesToAdd := toInt(ck.exp.Group(4), 1)
+ linesToDel := toInt(m[2], 1)
+ linesToAdd := toInt(m[4], 1)
if trace.Tracing {
trace.Stepf("hunk -%d +%d", linesToDel, linesToAdd)
}
+
ck.checktextUniHunkCr()
ck.checktextRcsid(text)
- for !ck.exp.EOF() && (linesToDel > 0 || linesToAdd > 0 || hasPrefix(ck.exp.CurrentLine().Text, "\\")) {
- line := ck.exp.CurrentLine()
- ck.exp.Advance()
+ for !ck.llex.EOF() && (linesToDel > 0 || linesToAdd > 0 || hasPrefix(ck.llex.CurrentLine().Text, "\\")) {
+ line := ck.llex.CurrentLine()
+ ck.llex.Skip()
text := line.Text
switch {
@@ -150,6 +156,7 @@ func (ck *PatchChecker) checkUnifiedDiff
case hasPrefix(text, "+"):
linesToAdd--
+ ck.checktextRcsid(text)
ck.checklineAdded(text[1:], patchedFileType)
case hasPrefix(text, "\\"):
@@ -166,18 +173,18 @@ func (ck *PatchChecker) checkUnifiedDiff
// been lost during transmission. There is no way to detect
// this by looking only at the patch file.
if linesToAdd != linesToDel {
- line := ck.exp.PreviousLine()
+ line := ck.llex.PreviousLine()
line.Warnf("Premature end of patch hunk (expected %d lines to be deleted and %d lines to be added).",
linesToDel, linesToAdd)
}
}
if !hasHunks {
- ck.exp.CurrentLine().Errorf("No patch hunks for %q.", patchedFile)
+ ck.llex.CurrentLine().Errorf("No patch hunks for %q.", patchedFile)
}
- if !ck.exp.EOF() {
- line := ck.exp.CurrentLine()
+ if !ck.llex.EOF() {
+ line := ck.llex.CurrentLine()
if !ck.isEmptyLine(line.Text) && !matches(line.Text, rePatchUniFileDel) {
line.Warnf("Empty line or end of file expected.")
G.Explain(
@@ -228,10 +235,10 @@ func (ck *PatchChecker) checklineContext
defer trace.Call2(text, patchedFileType.String())()
}
+ ck.checktextRcsid(text)
+
if G.Opts.WarnExtra {
ck.checklineAdded(text, patchedFileType)
- } else {
- ck.checktextRcsid(text)
}
}
@@ -240,16 +247,8 @@ func (ck *PatchChecker) checklineAdded(a
defer trace.Call2(addedText, patchedFileType.String())()
}
- ck.checktextRcsid(addedText)
-
- line := ck.exp.PreviousLine()
+ line := ck.llex.PreviousLine()
switch patchedFileType {
- case ftShell, ftIgnore:
- break
- case ftMakefile:
- ck.checklineOtherAbsolutePathname(line, addedText)
- case ftSource:
- ck.checklineSourceAbsolutePathname(line, addedText)
case ftConfigure:
if hasSuffix(addedText, ": Avoid regenerating within pkgsrc") {
line.Errorf("This code must not be included in patches.")
@@ -259,8 +258,6 @@ func (ck *PatchChecker) checklineAdded(a
"For more details, look for \"configure-scripts-override\" in",
"mk/configure/gnu-configure.mk.")
}
- default:
- ck.checklineOtherAbsolutePathname(line, addedText)
}
}
@@ -269,7 +266,7 @@ func (ck *PatchChecker) checktextUniHunk
defer trace.Call0()()
}
- line := ck.exp.PreviousLine()
+ line := ck.llex.PreviousLine()
if hasSuffix(line.Text, "\r") {
// This code has been introduced around 2006.
// As of 2018, this might be fixed by now.
@@ -288,9 +285,9 @@ func (ck *PatchChecker) checktextRcsid(t
}
if m, tagname := match1(text, `\$(Author|Date|Header|Id|Locker|Log|Name|RCSfile|Revision|Source|State|NetBSD)(?::[^\$]*)?\$`); m {
if matches(text, rePatchUniHunk) {
- ck.exp.PreviousLine().Warnf("Found RCS tag \"$%s$\". Please remove it.", tagname)
+ ck.llex.PreviousLine().Warnf("Found RCS tag \"$%s$\". Please remove it.", tagname)
} else {
- ck.exp.PreviousLine().Warnf("Found RCS tag \"$%s$\". Please remove it by reducing the number of context lines using pkgdiff or \"diff -U[210]\".", tagname)
+ ck.llex.PreviousLine().Warnf("Found RCS tag \"$%s$\". Please remove it by reducing the number of context lines using pkgdiff or \"diff -U[210]\".", tagname)
}
}
}
@@ -308,27 +305,14 @@ func (ck *PatchChecker) isEmptyLine(text
type FileType uint8
-// TODO: Is this type really useful? It is mainly used for warning about absolute filenames,
-// and that check is questionable in itself.
-
const (
- ftSource FileType = iota
- ftShell
- ftMakefile
- ftText
- ftConfigure
- ftIgnore
+ ftConfigure FileType = iota
ftUnknown
)
func (ft FileType) String() string {
return [...]string{
- "source code",
- "shell code",
- "Makefile",
- "text file",
"configure file",
- "ignored",
"unknown",
}[ft]
}
@@ -341,82 +325,10 @@ func guessFileType(filename string) (fil
basename := path.Base(filename)
basename = strings.TrimSuffix(basename, ".in") // doesn't influence the content type
- ext := strings.ToLower(strings.TrimLeft(path.Ext(basename), "."))
switch {
- case matches(basename, `^I?[Mm]akefile|\.ma?k$`):
- return ftMakefile
case basename == "configure" || basename == "configure.ac":
return ftConfigure
}
-
- switch ext {
- case "m4", "sh":
- return ftShell
- case "c", "cc", "cpp", "cxx", "el", "h", "hh", "hpp", "l", "pl", "pm", "py", "s", "t", "y":
- return ftSource
- case "conf", "html", "info", "man", "po", "tex", "texi", "texinfo", "txt", "xml":
- return ftText
- case "":
- return ftUnknown
- }
-
- if trace.Tracing {
- trace.Step1("Unknown file type for %q", filename)
- }
return ftUnknown
}
-
-// Looks for strings like "/dev/cd0" appearing in source code
-func (ck *PatchChecker) checklineSourceAbsolutePathname(line Line, text string) {
- if !strings.ContainsAny(text, "\"'") {
- return
- }
- if matched, before, _, str := match3(text, `^(.*)(["'])(/\w[^"']*)["']`); matched {
- if trace.Tracing {
- trace.Step2("checklineSourceAbsolutePathname: before=%q, str=%q", before, str)
- }
-
- switch {
- case matches(before, `[A-Z_][\t ]*$`):
- // ok; C example: const char *echo_cmd = PREFIX "/bin/echo";
-
- case matches(before, `\+[\t ]*$`):
- // ok; Python example: libdir = prefix + '/lib'
-
- default:
- LineChecker{line}.CheckWordAbsolutePathname(str)
- }
- }
-}
-
-func (ck *PatchChecker) checklineOtherAbsolutePathname(line Line, text string) {
- if trace.Tracing {
- defer trace.Call1(text)()
- }
-
- if hasPrefix(text, "#") && !hasPrefix(text, "#!") {
- // Don't warn for absolute pathnames in comments, except for shell interpreters.
-
- } else if m, before, dir, _ := match3(text, `^(.*?)((?:/[\w.]+)*/(?:bin|dev|etc|home|lib|mnt|opt|proc|sbin|tmp|usr|var)\b[\w./\-]*)(.*)$`); m {
- switch {
- case matches(before, `[\w).@}]$`) && !matches(before, `DESTDIR.$`):
- // Example: $prefix/bin
- // Example: $(prefix)/bin
- // Example: ../bin
- // Example: @prefix@/bin
- // Example: ${prefix}/bin
-
- case matches(before, `\+[\t ]*["']$`):
- // Example: prefix + '/lib'
-
- // XXX new: case matches(before, `\bs.$`): // Example: sed -e s,/usr,@PREFIX@,
-
- default:
- if trace.Tracing {
- trace.Step1("before=%q", before)
- }
- LineChecker{line}.CheckWordAbsolutePathname(dir)
- }
- }
-}
Index: pkgsrc/pkgtools/pkglint/files/patches_test.go
diff -u pkgsrc/pkgtools/pkglint/files/patches_test.go:1.27 pkgsrc/pkgtools/pkglint/files/patches_test.go:1.28
--- pkgsrc/pkgtools/pkglint/files/patches_test.go:1.27 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/patches_test.go Sat Jan 26 16:31:33 2019
@@ -145,50 +145,6 @@ func (s *Suite) Test_CheckLinesPatch__gi
"ERROR: patch-aa:5: Each patch must be documented.")
}
-func (s *Suite) Test_PatchChecker_checklineSourceAbsolutePathname(c *check.C) {
- t := s.Init(c)
-
- lines := t.NewLines("patch-aa",
- RcsID,
- "",
- "Documentation",
- "",
- "--- code.c.orig",
- "+++ code.c",
- "@@ -0,0 +1,3 @@",
- "+const char abspath[] = PREFIX \"/bin/program\";",
- "+val abspath = libdir + \"/libiconv.so.1.0\"",
- "+const char abspath[] = \"/dev/scd0\";")
-
- CheckLinesPatch(lines)
-
- t.CheckOutputLines(
- "WARN: patch-aa:10: Found absolute pathname: /dev/scd0")
-}
-
-func (s *Suite) Test_PatchChecker_checklineOtherAbsolutePathname(c *check.C) {
- t := s.Init(c)
-
- lines := t.NewLines("patch-aa",
- RcsID,
- "",
- "Documentation",
- "",
- "--- file.unknown.orig",
- "+++ file.unknown",
- "@@ -0,0 +1,5 @@",
- "+abspath=\"@prefix@/bin/program\"",
- "+abspath=\"${DESTDIR}/bin/\"",
- "+abspath=\"${PREFIX}/bin/\"",
- "+abspath = $prefix + '/bin/program'",
- "+abspath=\"$prefix/bin/program\"")
-
- CheckLinesPatch(lines)
-
- t.CheckOutputLines(
- "WARN: patch-aa:9: Found absolute pathname: /bin/")
-}
-
// The output of BSD Make typically contains "*** Error code".
// In some really good patches, this output is included in the patch comment,
// to document why the patch is necessary.
@@ -351,49 +307,6 @@ func (s *Suite) Test_CheckLinesPatch__on
"WARN: patch-context:5: Please use unified diffs (diff -u) for patches.")
}
-// TODO: Maybe this should only be checked if the patch changes
-// an absolute path to a relative one, because otherwise these
-// absolute paths may be intentional.
-func (s *Suite) Test_CheckLinesPatch__Makefile_with_absolute_pathnames(c *check.C) {
- t := s.Init(c)
-
- t.SetUpCommandLine("-Wabsname", "-Wno-extra")
- lines := t.NewLines("patch-unified",
- RcsID,
- "",
- "Documentation for the patch",
- "",
- "--- Makefile.orig",
- "+++ Makefile",
- "@@ -1,3 +1,7 @@",
- " \t/bin/cp context before",
- "-\t/bin/cp deleted",
- "+\t/bin/cp added",
- "+#\t/bin/cp added comment",
- "+# added comment",
- "+\t${DESTDIR}/bin/cp added",
- "+\t${prefix}/bin/cp added",
- " \t/bin/cp context after")
-
- CheckLinesPatch(lines)
-
- t.CheckOutputLines(
- "WARN: patch-unified:10: Found absolute pathname: /bin/cp",
- "WARN: patch-unified:13: Found absolute pathname: /bin/cp")
-
- // With extra warnings turned on, absolute paths in the context lines
- // are also checked, to detect absolute paths that might be overlooked.
- G.Opts.WarnExtra = true
-
- CheckLinesPatch(lines)
-
- t.CheckOutputLines(
- "WARN: patch-unified:8: Found absolute pathname: /bin/cp",
- "WARN: patch-unified:10: Found absolute pathname: /bin/cp",
- "WARN: patch-unified:13: Found absolute pathname: /bin/cp",
- "WARN: patch-unified:15: Found absolute pathname: /bin/cp")
-}
-
func (s *Suite) Test_CheckLinesPatch__no_newline_with_text_following(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/mklines.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines.go:1.39 pkgsrc/pkgtools/pkglint/files/mklines.go:1.40
--- pkgsrc/pkgtools/pkglint/files/mklines.go:1.39 Sun Jan 13 19:55:52 2019
+++ pkgsrc/pkgtools/pkglint/files/mklines.go Sat Jan 26 16:31:33 2019
@@ -171,13 +171,12 @@ func (mklines *MkLinesImpl) checkAll() {
mklines.indentation.CheckFinish(mklines.lines.FileName)
}
- // TODO: Extract this code so that it is clearly visible in the stack trace.
if trace.Tracing {
trace.Stepf("Starting main checking loop")
}
mklines.ForEachEnd(lineAction, atEnd)
- substContext.Finish(NewMkLine(mklines.lines.EOFLine())) // TODO: mklines.EOFLine()
+ substContext.Finish(mklines.EOFLine())
varalign.Finish()
CheckLinesTrailingEmptyLines(mklines.lines)
@@ -350,8 +349,8 @@ func (mklines *MkLinesImpl) collectDocum
relevant := true
// TODO: Correctly interpret declarations like "package-settable variables:" and
- // TODO: "user-settable variables", as well as "default: ...", "allowed: ...",
- // TODO: "list of" and other types.
+ // "user-settable variables", as well as "default: ...", "allowed: ...",
+ // "list of" and other types.
finish := func() {
if commentLines >= 3 && relevant {
@@ -481,6 +480,10 @@ func (mklines *MkLinesImpl) SaveAutofixC
mklines.lines.SaveAutofixChanges()
}
+func (mklines *MkLinesImpl) EOFLine() MkLine {
+ return NewMkLine(mklines.lines.EOFLine())
+}
+
// VaralignBlock checks that all variable assignments from a paragraph
// use the same indentation depth for their values.
// It also checks that the indentation uses tabs instead of spaces.
Index: pkgsrc/pkgtools/pkglint/files/mklines_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.34 pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.35
--- pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.34 Sun Jan 13 19:55:52 2019
+++ pkgsrc/pkgtools/pkglint/files/mklines_test.go Sat Jan 26 16:31:33 2019
@@ -339,8 +339,8 @@ func (s *Suite) Test_MkLines_collectDefi
// FIXME: the below warning is wrong; it's ok to have SUBST blocks in all files,
// maybe except buildlink3.mk.
"WARN: determine-defined-variables.mk:12: The variable SUBST_VARS.subst may not be set " +
- "(only given a default value, appended to) in this file; " +
- "it would be ok in Makefile, Makefile.common, options.mk.")
+ "(only given a default value, or appended to) in this file; " +
+ "it would be ok in Makefile, Makefile.common or options.mk.")
}
func (s *Suite) Test_MkLines_collectDefinedVariables__BUILTIN_FIND_FILES_VAR(c *check.C) {
@@ -954,6 +954,26 @@ func (s *Suite) Test_MkLines_CheckRedund
t.CheckOutputEmpty()
}
+func (s *Suite) Test_MkLines_CheckRedundantAssignments__included_OPSYS_variable(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package",
+ ".include \"../../category/dependency/buildlink3.mk\"",
+ "CONFIGURE_ARGS+=\tone",
+ "CONFIGURE_ARGS=\ttwo",
+ "CONFIGURE_ARGS+=\tthree")
+ t.SetUpPackage("category/dependency")
+ t.CreateFileDummyBuildlink3("category/dependency/buildlink3.mk")
+ t.CreateFileLines("category/dependency/builtin.mk",
+ MkRcsID,
+ "CONFIGURE_ARGS.Darwin+=\tdarwin")
+
+ G.Check(t.File("category/package"))
+
+ t.CheckOutputLines(
+ "WARN: ~/category/package/Makefile:21: Variable CONFIGURE_ARGS is overwritten in line 22.")
+}
+
func (s *Suite) Test_MkLines_Check__PLIST_VARS(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/mkshparser.go
diff -u pkgsrc/pkgtools/pkglint/files/mkshparser.go:1.11 pkgsrc/pkgtools/pkglint/files/mkshparser.go:1.12
--- pkgsrc/pkgtools/pkglint/files/mkshparser.go:1.11 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/mkshparser.go Sat Jan 26 16:31:33 2019
@@ -192,7 +192,9 @@ func (lex *ShellLexer) Lex(lval *shyySym
case "do":
return tkDO
case "done":
- // TODO: add test that ensures "lex.atCommandStart = false" is required here.
+ // lex.atCommandStart must stay true here because further "done" or "fi"
+ // may follow directly, without any semicolon.
+ // Ideally lex.atCommandStart would be a tri-state variable: yes, no, partly.
return tkDONE
case "in":
lex.atCommandStart = false
@@ -204,7 +206,7 @@ func (lex *ShellLexer) Lex(lval *shyySym
case "{":
return tkLBRACE
case "}":
- // TODO: add test that ensures "lex.atCommandStart = false" is required here.
+ // See the comment at the "done" case above.
return tkRBRACE
case "!":
return tkEXCLAM
Index: pkgsrc/pkgtools/pkglint/files/options_test.go
diff -u pkgsrc/pkgtools/pkglint/files/options_test.go:1.9 pkgsrc/pkgtools/pkglint/files/options_test.go:1.10
--- pkgsrc/pkgtools/pkglint/files/options_test.go:1.9 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/options_test.go Sat Jan 26 16:31:33 2019
@@ -68,6 +68,69 @@ func (s *Suite) Test_CheckLinesOptionsMk
"Option \"undeclared\" is handled but not added to PKG_SUPPORTED_OPTIONS.")
}
+// This test is provided for code coverage. Similarities to existing files are purely coincidental.
+func (s *Suite) Test_CheckLinesOptionsMk__edge_cases(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("-Wall,no-space")
+ t.SetUpVartypes()
+ t.SetUpOption("option1", "Description for option1")
+ t.CreateFileLines("mk/compiler.mk",
+ MkRcsID)
+ t.CreateFileLines("mk/bsd.options.mk",
+ MkRcsID)
+ t.DisableTracing()
+
+ mklines := t.SetUpFileMkLines("category/package/options.mk",
+ MkRcsID)
+
+ CheckLinesOptionsMk(mklines)
+
+ t.CheckOutputLines(
+ "WARN: ~/category/package/options.mk:EOF: Expected definition of PKG_OPTIONS_VAR.")
+
+ mklines = t.SetUpFileMkLines("category/package/options.mk",
+ MkRcsID,
+ "PKG_SUPPORTED_OPTIONS=\toption1")
+
+ CheckLinesOptionsMk(mklines)
+
+ t.CheckOutputLines(
+ "WARN: ~/category/package/options.mk:2: Expected definition of PKG_OPTIONS_VAR.")
+
+ mklines = t.SetUpFileMkLines("category/package/options.mk",
+ MkRcsID,
+ "PKG_OPTIONS_VAR=\tPKG_OPTIONS.pkgbase",
+ "PKG_SUPPORTED_OPTIONS=\toption1",
+ ".include \"../../mk/compiler.mk\"")
+
+ CheckLinesOptionsMk(mklines)
+
+ t.CheckOutputLines(
+ "WARN: ~/category/package/options.mk:3: " +
+ "Option \"option1\" should be handled below in an .if block.")
+
+ mklines = t.SetUpFileMkLines("category/package/options.mk",
+ MkRcsID,
+ "PKG_OPTIONS_VAR=\tPKG_OPTIONS.pkgbase",
+ "PKG_SUPPORTED_OPTIONS=\toption1",
+ ".include \"../../mk/bsd.options.mk\"",
+ "",
+ ".if !empty(PKG_OPTIONS:O:u:Moption1) "+
+ "|| !empty(PKG_OPTIONS:Noption1) "+
+ "|| !empty(PKG_OPTIONS:O) "+
+ "|| !empty(X11_TYPE) "+
+ "|| !empty(PKG_OPTIONS:M${X11_TYPE})",
+ ".endif")
+
+ CheckLinesOptionsMk(mklines)
+
+ // Although technically this option is handled by the :Noption1 modifier,
+ // this is so unusual that the warning is justified.
+ t.CheckOutputLines(
+ "WARN: ~/category/package/options.mk:3: Option \"option1\" should be handled below in an .if block.")
+}
+
// If there is no .include line after the declaration of the package-settable
// variables, the whole analysis stops.
//
Index: pkgsrc/pkgtools/pkglint/files/package.go
diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.43 pkgsrc/pkgtools/pkglint/files/package.go:1.44
--- pkgsrc/pkgtools/pkglint/files/package.go:1.43 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/package.go Sat Jan 26 16:31:33 2019
@@ -109,7 +109,7 @@ func (pkg *Package) checkPossibleDowngra
changeVersion := replaceAll(change.Version, `nb\d+$`, "")
if pkgver.Compare(pkgversion, changeVersion) < 0 {
mkline.Warnf("The package is being downgraded from %s (see %s) to %s.",
- change.Version, mkline.Line.RefTo(change.Line), pkgversion)
+ change.Version, mkline.Line.RefToLocation(change.Location), pkgversion)
G.Explain(
"The files in doc/CHANGES-*, in which all version changes are",
"recorded, have a higher version number than what the package says.",
@@ -134,10 +134,10 @@ func (pkg *Package) checkLinesBuildlink3
for _, mkline := range mklines.mklines {
if mkline.IsInclude() {
includedFile := mkline.IncludedFile()
- if m, bl3 := match1(includedFile, `^\.\./\.\./(.*)/buildlink3\.mk`); m {
- includedFiles[bl3] = mkline
- if pkg.bl3[bl3] == nil {
- mkline.Warnf("%s/buildlink3.mk is included by this file but not by the package.", bl3)
+ if matches(includedFile, `^\.\./\.\./.*/buildlink3\.mk`) {
+ includedFiles[includedFile] = mkline
+ if pkg.bl3[includedFile] == nil {
+ mkline.Warnf("%s is included by this file but not by the package.", includedFile)
}
}
}
@@ -146,7 +146,7 @@ func (pkg *Package) checkLinesBuildlink3
if trace.Tracing {
for packageBl3 := range pkg.bl3 {
if includedFiles[packageBl3] == nil {
- trace.Step1("%s/buildlink3.mk is included by the package but not by the buildlink3.mk file.", packageBl3)
+ trace.Step1("%s is included by the package but not by the buildlink3.mk file.", packageBl3)
}
}
}
@@ -314,12 +314,7 @@ func (pkg *Package) readMakefile(filenam
// For every included buildlink3.mk, include the corresponding builtin.mk
// automatically since the pkgsrc infrastructure does the same.
- //
- // Disabled for now since it increases the running time by about 20%
- // and produces many new warnings, which must be evaluated first.
- //
- // TODO: Remove the G.Testing below.
- if G.Testing && path.Base(filename) == "buildlink3.mk" {
+ if path.Base(filename) == "buildlink3.mk" {
builtin := path.Join(path.Dir(filename), "builtin.mk")
if fileExists(builtin) {
pkg.readMakefile(builtin, mainLines, allLines, "")
@@ -364,10 +359,10 @@ func (pkg *Package) findIncludedFile(mkl
if includedFile != "" {
if mkline.Basename != "buildlink3.mk" {
- if m, bl3File := match1(includedFile, `^\.\./\.\./(.*)/buildlink3\.mk$`); m {
- pkg.bl3[bl3File] = mkline
+ if matches(includedFile, `^\.\./\.\./(.*)/buildlink3\.mk$`) {
+ pkg.bl3[includedFile] = mkline
if trace.Tracing {
- trace.Step1("Buildlink3 file in package: %q", bl3File)
+ trace.Step1("Buildlink3 file in package: %q", includedFile)
}
}
}
Index: pkgsrc/pkgtools/pkglint/files/package_test.go
diff -u pkgsrc/pkgtools/pkglint/files/package_test.go:1.37 pkgsrc/pkgtools/pkglint/files/package_test.go:1.38
--- pkgsrc/pkgtools/pkglint/files/package_test.go:1.37 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/package_test.go Sat Jan 26 16:31:33 2019
@@ -19,7 +19,8 @@ func (s *Suite) Test_Package_checkLinesB
t.CheckOutputLines(
"WARN: category/package/buildlink3.mk:3: " +
- "category/dependency/buildlink3.mk is included by this file but not by the package.")
+ "../../category/dependency/buildlink3.mk is included by this file " +
+ "but not by the package.")
}
func (s *Suite) Test_Package_checkLinesBuildlink3Inclusion__package_but_not_file(c *check.C) {
@@ -39,7 +40,7 @@ func (s *Suite) Test_Package_checkLinesB
// for building other packages. These cannot be flagged as warnings.
t.CheckOutputLines(
"TRACE: + (*Package).checkLinesBuildlink3Inclusion()",
- "TRACE: 1 ../../category/dependency/buildlink3.mk/buildlink3.mk "+
+ "TRACE: 1 ../../category/dependency/buildlink3.mk "+
"is included by the package but not by the buildlink3.mk file.",
"TRACE: - (*Package).checkLinesBuildlink3Inclusion()")
}
@@ -361,7 +362,7 @@ func (s *Suite) Test_Package_determineEf
G.Check(pkg)
t.CheckOutputLines(
- "NOTE: ~/category/package/Makefile:20: " +
+ "NOTE: ~/category/package/Makefile:4: " +
"This assignment is probably redundant since PKGNAME is ${DISTNAME} by default.")
}
@@ -714,9 +715,9 @@ func (s *Suite) Test_Package_checkUpdate
"WARN: category/pkg1/../../doc/TODO:3: Invalid line format \"\".",
"WARN: category/pkg1/../../doc/TODO:4: Invalid line format \"\\tO wrong bullet\".",
"WARN: category/pkg1/../../doc/TODO:5: Invalid package name \"package-without-version\".",
- "NOTE: category/pkg1/Makefile:20: The update request to 1.0 from doc/TODO has been done.",
- "WARN: category/pkg2/Makefile:20: This package should be updated to 2.0 ([nice new features]).",
- "NOTE: category/pkg3/Makefile:20: This package is newer than the update request to 3.0 ([security update]).",
+ "NOTE: category/pkg1/Makefile:4: The update request to 1.0 from doc/TODO has been done.",
+ "WARN: category/pkg2/Makefile:4: This package should be updated to 2.0 ([nice new features]).",
+ "NOTE: category/pkg3/Makefile:4: This package is newer than the update request to 3.0 ([security update]).",
"0 errors and 4 warnings found.",
"(Run \"pkglint -e\" to show explanations.)")
}
@@ -901,6 +902,29 @@ func (s *Suite) Test_Package_readMakefil
"WARN: ~/category/package/Makefile:20: Invalid relative path \"../package/extra.mk\".")
}
+// When a buildlink3.mk file is included, the corresponding builtin.mk
+// file is included by the pkgsrc infrastructure. Therefore all variables
+// declared in the builtin.mk file become known in the package.
+func (s *Suite) Test_Package_readMakefile__builtin_mk(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpTool("echo", "ECHO", AtRunTime)
+ t.SetUpPackage("category/package",
+ ".include \"../../category/lib1/buildlink3.mk\"",
+ "",
+ "show-var-from-builtin: .PHONY",
+ "\techo ${VAR_FROM_BUILTIN} ${OTHER_VAR}")
+ t.CreateFileDummyBuildlink3("category/lib1/buildlink3.mk")
+ t.CreateFileLines("category/lib1/builtin.mk",
+ MkRcsID,
+ "VAR_FROM_BUILTIN=\t# defined")
+
+ G.Check(t.File("category/package"))
+
+ t.CheckOutputLines(
+ "WARN: ~/category/package/Makefile:23: OTHER_VAR is used but not defined.")
+}
+
func (s *Suite) Test_Package_checkLocallyModified(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/shell_test.go
diff -u pkgsrc/pkgtools/pkglint/files/shell_test.go:1.37 pkgsrc/pkgtools/pkglint/files/shell_test.go:1.38
--- pkgsrc/pkgtools/pkglint/files/shell_test.go:1.37 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/shell_test.go Sat Jan 26 16:31:33 2019
@@ -177,23 +177,23 @@ func (s *Suite) Test_ShellLine_CheckShel
t.SetUpTool("echo", "", AtRunTime)
t.SetUpVartypes()
- test("echo ${PKGNAME:Q}") // vucQuotPlain
+ test("echo ${PKGNAME:Q}") // VucQuotPlain
t.CheckOutputLines(
"WARN: filename:1: PKGNAME may not be used in this file; "+
- "it would be ok in Makefile, Makefile.*, *.mk.",
+ "it would be ok in Makefile, Makefile.* or *.mk.",
"NOTE: filename:1: The :Q operator isn't necessary for ${PKGNAME} here.")
- test("echo \"${CFLAGS:Q}\"") // vucQuotDquot
+ test("echo \"${CFLAGS:Q}\"") // VucQuotDquot
t.CheckOutputLines(
"WARN: filename:1: The :Q modifier should not be used inside double quotes.",
"WARN: filename:1: CFLAGS may not be used in this file; "+
- "it would be ok in Makefile, Makefile.common, options.mk, *.mk.",
+ "it would be ok in Makefile, Makefile.common, options.mk or *.mk.",
"WARN: filename:1: Please use ${CFLAGS:M*:Q} instead of ${CFLAGS:Q} "+
"and make sure the variable appears outside of any quoting characters.")
- test("echo '${COMMENT:Q}'") // vucQuotSquot
+ test("echo '${COMMENT:Q}'") // VucQuotSquot
t.CheckOutputLines(
"WARN: filename:1: COMMENT may not be used in any file; it is a write-only variable.",
@@ -262,7 +262,8 @@ func (s *Suite) Test_ShellLine_CheckShel
// TODO: Why is TOOLS_PATH.msgfmt not recognized?
// At least, the warning should be more specific, mentioning USE_TOOLS.
t.CheckOutputLines(
- "WARN: filename:1: WRKSRC may not be used in this file; it would be ok in Makefile, Makefile.*, *.mk.",
+ "WARN: filename:1: WRKSRC may not be used in this file; "+
+ "it would be ok in Makefile, Makefile.* or *.mk.",
"WARN: filename:1: Unknown shell command \"[\".",
"WARN: filename:1: Unknown shell command \"${TOOLS_PATH.msgfmt}\".")
@@ -1032,8 +1033,7 @@ func (s *Suite) Test_ShellLine_CheckShel
mklines.Check()
t.CheckOutputLines(
- "ERROR: Makefile:3: The Solaris /bin/sh cannot handle \"cd\" inside conditionals.",
- "WARN: Makefile:3: Found absolute pathname: /bin")
+ "ERROR: Makefile:3: The Solaris /bin/sh cannot handle \"cd\" inside conditionals.")
}
func (s *Suite) Test_ShellLine_CheckShellCommand__negated_pipe(c *check.C) {
@@ -1050,8 +1050,7 @@ func (s *Suite) Test_ShellLine_CheckShel
mklines.Check()
t.CheckOutputLines(
- "WARN: Makefile:3: The Solaris /bin/sh does not support negation of shell commands.",
- "WARN: Makefile:3: Found absolute pathname: /etc/passwd")
+ "WARN: Makefile:3: The Solaris /bin/sh does not support negation of shell commands.")
}
func (s *Suite) Test_ShellLine_CheckShellCommand__subshell(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/pkglint.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint.go:1.45 pkgsrc/pkgtools/pkglint/files/pkglint.go:1.46
--- pkgsrc/pkgtools/pkglint/files/pkglint.go:1.45 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/pkglint.go Sat Jan 26 16:31:33 2019
@@ -11,6 +11,7 @@ import (
"path"
"path/filepath"
"runtime"
+ "runtime/debug"
"runtime/pprof"
"strings"
)
@@ -38,12 +39,14 @@ type Pkglint struct {
loaded *histogram.Histogram
res regex.Registry
fileCache *FileCache
+ interner StringInterner
}
func NewPkglint() Pkglint {
return Pkglint{
res: regex.NewRegistry(),
- fileCache: NewFileCache(200)}
+ fileCache: NewFileCache(200),
+ interner: NewStringInterner()}
}
type CmdOpts struct {
@@ -73,7 +76,6 @@ type CmdOpts struct {
// could be contrasted by a future --ignore option, in order to suppress
// individual checks.
- WarnAbsname,
WarnDirectcmd,
WarnExtra,
WarnOrder,
@@ -98,8 +100,8 @@ type CmdOpts struct {
}
type Hash struct {
- hash string
- line Line // TODO: Maybe a Location object would already be enough.
+ hash []byte
+ location Location
}
type pkglintFatal struct{}
@@ -147,6 +149,21 @@ func (pkglint *Pkglint) Main(argv ...str
}
if pkglint.Opts.Profiling {
+
+ defer func() {
+ pkglint.fileCache.table = nil
+ pkglint.fileCache.mapping = nil
+ runtime.GC()
+
+ fd, err := os.Create("pkglint.heapdump")
+ G.AssertNil(err, "heapDump.create")
+
+ debug.WriteHeapDump(fd.Fd())
+
+ err = fd.Close()
+ G.AssertNil(err, "heapDump.close")
+ }()
+
f, err := os.Create("pkglint.pprof")
if err != nil {
dummyLine.Fatalf("Cannot create profiling file: %s", err)
@@ -176,20 +193,22 @@ func (pkglint *Pkglint) Main(argv ...str
pkglint.Todo = []string{"."}
}
- firstArg := pkglint.Todo[0]
- if fileExists(firstArg) {
- firstArg = path.Dir(firstArg)
+ firstDir := pkglint.Todo[0]
+ if fileExists(firstDir) {
+ firstDir = path.Dir(firstDir)
}
- relTopdir := findPkgsrcTopdir(firstArg)
+
+ relTopdir := findPkgsrcTopdir(firstDir)
if relTopdir == "" {
// If the first argument to pkglint is not inside a pkgsrc tree,
// pkglint doesn't know where to load the infrastructure files from,
// and these are needed for virtually every single check.
// Therefore, the only sensible thing to do is to quit immediately.
- dummyLine.Fatalf("%q must be inside a pkgsrc tree.", firstArg)
+ dummyLine.Fatalf("%q must be inside a pkgsrc tree.", firstDir)
}
- pkglint.Pkgsrc = NewPkgsrc(firstArg + "/" + relTopdir)
+ pkglint.Pkgsrc = NewPkgsrc(firstDir + "/" + relTopdir)
+ pkglint.Wip = matches(pkglint.Pkgsrc.ToRel(firstDir), `^wip(/|$)`) // Same as in Pkglint.Check.
pkglint.Pkgsrc.LoadInfrastructure()
currentUser, err := user.Current()
@@ -250,7 +269,6 @@ func (pkglint *Pkglint) ParseCommandLine
check.AddFlagVar("patches", &gopts.CheckPatches, true, "check patches")
check.AddFlagVar("PLIST", &gopts.CheckPlist, true, "check PLIST files")
- warn.AddFlagVar("absname", &gopts.WarnAbsname, false, "warn about use of absolute filenames")
warn.AddFlagVar("directcmd", &gopts.WarnDirectcmd, true, "warn about use of direct command names instead of Make variables")
warn.AddFlagVar("extra", &gopts.WarnExtra, false, "enable some extra warnings")
warn.AddFlagVar("order", &gopts.WarnOrder, true, "warn if Makefile entries are unordered")
@@ -490,6 +508,9 @@ func (pkglint *Pkglint) Assertf(cond boo
// AssertNil ensures that the given error is nil.
//
+// Contrary to other diagnostics, the format should not end in a period
+// since it is followed by the error.
+//
// Other than Assertf, this method does not require any comparison operator in the calling code.
// This makes it possible to get 100% branch coverage for cases that "really can never fail".
func (pkglint *Pkglint) AssertNil(err error, format string, args ...interface{}) {
@@ -578,13 +599,14 @@ func CheckLinesDescr(lines Lines) {
}
}
}
+
CheckLinesTrailingEmptyLines(lines)
if maxLines := 24; lines.Len() > maxLines {
line := lines.Lines[maxLines]
line.Warnf("File too long (should be no more than %d lines).", maxLines)
- G.Explain(
+ line.Explain(
"The DESCR file should fit on a traditional terminal of 80x25 characters.",
"It is also intended to give a _brief_ summary about the package's contents.")
}
@@ -763,6 +785,9 @@ func (pkglint *Pkglint) checkReg(filenam
NewLineWhole(filename).Warnf("Only packages in regress/ may have spec files.")
}
+ case pkglint.matchesLicenseFile(basename):
+ break
+
default:
NewLineWhole(filename).Warnf("Unexpected file found.")
if pkglint.Opts.CheckExtra {
@@ -771,6 +796,15 @@ func (pkglint *Pkglint) checkReg(filenam
}
}
+func (pkglint *Pkglint) matchesLicenseFile(basename string) bool {
+ if pkglint.Pkg == nil {
+ return false
+ }
+
+ licenseFile, _ := pkglint.Pkg.vars.Value("LICENSE_FILE")
+ return basename == path.Base(licenseFile)
+}
+
func (pkglint *Pkglint) checkExecutable(filename string, mode os.FileMode) {
if mode.Perm()&0111 == 0 {
// Not executable at all.
Index: pkgsrc/pkgtools/pkglint/files/pkgsrc.go
diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.17 pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.18
--- pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.17 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/pkgsrc.go Sat Jan 26 16:31:33 2019
@@ -40,6 +40,10 @@ type Pkgsrc struct {
Deprecated map[string]string //
vartypes map[string]*Vartype // varcanon => type
+ // TODO: The Hashes and UsedLicenses are modified after the initialization.
+ // This contradicts the expectation that all pkgsrc data is constant.
+ // These two fields should probably be moved to the Pkglint type.
+
Hashes map[string]*Hash // Maps "alg:filename" => hash (inter-package check).
UsedLicenses map[string]bool // Maps "license name" => true (inter-package check).
}
@@ -416,7 +420,7 @@ func (src *Pkgsrc) parseSuggestedUpdates
if state == 3 {
if m, pkgname, comment := match2(text, `^\to[\t ]([^\t ]+)(?:[\t ]*(.+))?$`); m {
if m, pkgbase, pkgversion := match2(pkgname, rePkgname); m {
- updates = append(updates, SuggestedUpdate{line, pkgbase, pkgversion, comment})
+ updates = append(updates, SuggestedUpdate{line.Location, intern(pkgbase), intern(pkgversion), intern(comment)})
} else {
line.Warnf("Invalid package name %q.", pkgname)
}
@@ -435,13 +439,29 @@ func (src *Pkgsrc) loadSuggestedUpdates(
func (src *Pkgsrc) loadDocChangesFromFile(filename string) []*Change {
+ warn := !G.Wip
+
parseChange := func(line Line) *Change {
- text := line.Text
- if !hasPrefix(text, "\t") {
+ lex := textproc.NewLexer(line.Text)
+
+ space := lex.NextHspace()
+ if space == "" {
+ return nil
+ }
+
+ if space != "\t" {
+ if warn {
+ line.Warnf("Package changes should be indented using a single tab, not %q.", space)
+ line.Explain(
+ "To avoid this formatting mistake in the future, just run",
+ sprintf("%q", bmake("cce")),
+ "after committing the update to the package.")
+ }
+
return nil
}
- f := strings.Fields(text)
+ f := strings.Fields(lex.Rest())
n := len(f)
if n != 4 && n != 6 {
return nil
@@ -453,53 +473,97 @@ func (src *Pkgsrc) loadDocChangesFromFil
}
author, date = author[1:], date[:len(date)-1]
+ newChange := func(version string) *Change {
+ return &Change{
+ Location: line.Location,
+ Action: intern(action),
+ Pkgpath: intern(pkgpath),
+ Version: intern(version),
+ Author: intern(author),
+ Date: intern(date),
+ }
+ }
+
switch {
case action == "Added" && f[2] == "version" && n == 6:
- return &Change{line, action, pkgpath, f[3], author, date}
+ return newChange(f[3])
+
case (action == "Updated" || action == "Downgraded") && f[2] == "to" && n == 6:
- return &Change{line, action, pkgpath, f[3], author, date}
+ return newChange(f[3])
+
case action == "Removed" && (n == 6 && f[2] == "successor" || n == 4):
- return &Change{line, action, pkgpath, "", author, date}
+ return newChange("")
+
case (action == "Renamed" || action == "Moved") && f[2] == "to" && n == 6:
- return &Change{line, action, pkgpath, "", author, date}
+ return newChange("")
}
+
+ line.Warnf("Unknown doc/CHANGES line: %s", line.Text)
+ line.Explain(
+ "See mk/misc/developer.mk for the rules.")
+
return nil
}
+ // Each date in the file should be from the same year as the filename says.
+ // This check has been added in 2018.
+ // For years earlier than 2018 pkglint doesn't care because it's not a big issue anyway.
year := ""
- if m, yyyy := match1(filename, `-(\d+)$`); m && yyyy >= "2018" { // TODO: Why 2018?
+ if m, yyyy := match1(filename, `-(\d+)$`); m && yyyy >= "2018" {
year = yyyy
}
+ infra := false
lines := Load(filename, MustSucceed|NotEmpty)
var changes []*Change
for _, line := range lines.Lines {
- if change := parseChange(line); change != nil {
- changes = append(changes, change)
- if year != "" && change.Date[0:4] != year {
- line.Warnf("Year %s for %s does not match the filename %s.", change.Date[0:4], change.Pkgpath, filename)
- }
- if len(changes) >= 2 && year != "" {
- if prev := changes[len(changes)-2]; change.Date < prev.Date {
- line.Warnf("Date %s for %s is earlier than %s for %s.", change.Date, change.Pkgpath, prev.Date, prev.Pkgpath)
- G.Explain(
- "The entries in doc/CHANGES should be in chronological order, and",
- "all dates are assumed to be in the UTC timezone, to prevent time",
- "warps.",
- "",
- "To fix this, determine which of the involved dates are correct",
- "and which aren't.",
- "",
- "To prevent this kind of mistakes in the future, make sure that",
- sprintf("your system time is correct and run %q to commit", bmake("cce")),
- "the changes entry.")
- }
+
+ if hasPrefix(line.Text, "\tmk/") {
+ infra = true
+ }
+ if infra {
+ if hasSuffix(line.Text, "]") {
+ infra = false
+ }
+ continue
+ }
+
+ change := parseChange(line)
+ if change == nil {
+ continue
+ }
+
+ changes = append(changes, change)
+
+ if !warn {
+ continue
+ }
+
+ if year != "" && change.Date[0:4] != year {
+ line.Warnf("Year %s for %s does not match the filename %s.",
+ change.Date[0:4], change.Pkgpath, filename)
+ }
+
+ if len(changes) >= 2 && year != "" {
+ if prev := changes[len(changes)-2]; change.Date < prev.Date {
+ line.Warnf("Date %s for %s is earlier than %s in %s.",
+ change.Date, change.Pkgpath, prev.Date, line.RefToLocation(prev.Location))
+ line.Explain(
+ "The entries in doc/CHANGES should be in chronological order, and",
+ "all dates are assumed to be in the UTC timezone, to prevent time",
+ "warps.",
+ "",
+ "To fix this, determine which of the involved dates are correct",
+ "and which aren't.",
+ "",
+ "To prevent this kind of mistakes in the future,",
+ "make sure that your system time is correct and run",
+ sprintf("%q", bmake("cce")),
+ "to commit the changes entry.")
}
- } else if lex := textproc.NewLexer(line.Text); lex.SkipByte('\t') && lex.TestByteSet(textproc.Upper) {
- line.Warnf("Unknown doc/CHANGES line: %s", line.Text)
- G.Explain("See mk/misc/developer.mk for the rules.")
}
}
+
return changes
}
@@ -916,17 +980,17 @@ func (src *Pkgsrc) guessVariableType(var
// Change describes a modification to a single package, from the doc/CHANGES-* files.
type Change struct {
- Line Line
- Action string
- Pkgpath string
- Version string
- Author string
- Date string
+ Location Location
+ Action string
+ Pkgpath string
+ Version string
+ Author string
+ Date string
}
// SuggestedUpdate describes a desired package update, from the doc/TODO file.
type SuggestedUpdate struct {
- Line Line
+ Line Location
Pkgname string
Version string
Comment string
Index: pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go
diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.14 pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.15
--- pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.14 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go Sat Jan 26 16:31:33 2019
@@ -44,8 +44,8 @@ func (s *Suite) Test_Pkgsrc_parseSuggest
todo := G.Pkgsrc.parseSuggestedUpdates(lines)
c.Check(todo, check.DeepEquals, []SuggestedUpdate{
- {lines.Lines[5], "CSP", "0.34", ""},
- {lines.Lines[6], "freeciv-client", "2.5.0", "(urgent)"}})
+ {lines.Lines[5].Location, "CSP", "0.34", ""},
+ {lines.Lines[6].Location, "freeciv-client", "2.5.0", "(urgent)"}})
}
func (s *Suite) Test_Pkgsrc_checkToplevelUnusedLicenses(c *check.C) {
@@ -164,6 +164,19 @@ func (s *Suite) Test_Pkgsrc_loadTools__B
// does not contain anything at mklinechecker.go:/UserDefinedVars/.
}
+func (s *Suite) Test_Pkgsrc_loadDocChanges__not_found(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPkgsrc()
+ t.Remove("doc/CHANGES-2018")
+ t.Remove("doc/TODO")
+ t.Remove("doc")
+
+ t.ExpectFatal(
+ G.Pkgsrc.loadDocChanges,
+ "FATAL: ~/doc: Cannot be read for loading the package changes.")
+}
+
func (s *Suite) Test_Pkgsrc_loadDocChangesFromFile(c *check.C) {
t := s.Init(c)
@@ -184,54 +197,106 @@ func (s *Suite) Test_Pkgsrc_loadDocChang
changes := G.Pkgsrc.loadDocChangesFromFile(t.File("doc/CHANGES-2018"))
c.Assert(len(changes), equals, 7)
- c.Check(*changes[0], equals, Change{changes[0].Line,
+ c.Check(*changes[0], equals, Change{changes[0].Location,
"Added", "category/package", "1.0", "author1", "2015-01-01"})
- c.Check(*changes[1], equals, Change{changes[1].Line,
+ c.Check(*changes[1], equals, Change{changes[1].Location,
"Updated", "category/package", "1.5", "author2", "2018-01-02"})
- c.Check(*changes[2], equals, Change{changes[2].Line,
+ c.Check(*changes[2], equals, Change{changes[2].Location,
"Renamed", "category/package", "", "author3", "2018-01-03"})
- c.Check(*changes[3], equals, Change{changes[3].Line,
+ c.Check(*changes[3], equals, Change{changes[3].Location,
"Moved", "category/package", "", "author4", "2018-01-04"})
- c.Check(*changes[4], equals, Change{changes[4].Line,
+ c.Check(*changes[4], equals, Change{changes[4].Location,
"Removed", "category/package", "", "author5", "2018-01-09"})
- c.Check(*changes[5], equals, Change{changes[5].Line,
+ c.Check(*changes[5], equals, Change{changes[5].Location,
"Removed", "category/package", "", "author6", "2018-01-06"})
- c.Check(*changes[6], equals, Change{changes[6].Line,
+ c.Check(*changes[6], equals, Change{changes[6].Location,
"Downgraded", "category/package", "1.2", "author7", "2018-01-07"})
t.CheckOutputLines(
"WARN: ~/doc/CHANGES-2018:1: Year 2015 for category/package does not match the filename ~/doc/CHANGES-2018.",
- "WARN: ~/doc/CHANGES-2018:6: Date 2018-01-06 for category/package is earlier than 2018-01-09 for category/package.",
+ "WARN: ~/doc/CHANGES-2018:6: Date 2018-01-06 for category/package is earlier than 2018-01-09 in line 5.",
"WARN: ~/doc/CHANGES-2018:12: Unknown doc/CHANGES line: \tAdded another [new package]")
}
-func (s *Suite) Test_Pkgsrc_loadDocChanges__not_found(c *check.C) {
+func (s *Suite) Test_Pkgsrc_loadDocChangesFromFile__not_found(c *check.C) {
t := s.Init(c)
- t.SetUpPkgsrc()
- t.Remove("doc/CHANGES-2018")
- t.Remove("doc/TODO")
- t.Remove("doc")
-
t.ExpectFatal(
- G.Pkgsrc.loadDocChanges,
- "FATAL: ~/doc: Cannot be read for loading the package changes.")
+ func() { G.Pkgsrc.loadDocChangesFromFile(t.File("doc/CHANGES-2018")) },
+ "FATAL: ~/doc/CHANGES-2018: Cannot be read.")
}
-func (s *Suite) Test_Pkgsrc_loadDocChangesFromFile__not_found(c *check.C) {
+// Since package authors for pkgsrc-wip cannot necessarily commit to
+// main pkgsrc, don't warn about unsorted doc/CHANGES lines.
+// Only pkgsrc main committers can fix these.
+func (s *Suite) Test_Pkgsrc_loadDocChangesFromFile__wip_suppresses_warnings(c *check.C) {
t := s.Init(c)
- t.ExpectFatal(
- func() { G.Pkgsrc.loadDocChangesFromFile(t.File("doc/CHANGES-2018")) },
- "FATAL: ~/doc/CHANGES-2018: Cannot be read.")
+ t.SetUpPackage("wip/package")
+ t.CreateFileLines("doc/CHANGES-2018",
+ RcsID,
+ "",
+ "Changes to the packages collection and infrastructure in 2018:",
+ "",
+ "\tUpdated sysutils/checkperms to 1.10 [rillig 2018-01-05]",
+ "\tUpdated sysutils/checkperms to 1.11 [rillig 2018-01-01]")
+
+ G.Main("pkglint", t.File("wip/package"))
+
+ t.CheckOutputLines(
+ "Looks fine.")
+}
+
+func (s *Suite) Test_Pkgsrc_loadDocChangesFromFile__wrong_indentation(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package")
+ t.CreateFileLines("doc/CHANGES-2018",
+ RcsID,
+ "",
+ "Changes to the packages collection and infrastructure in 2018:",
+ "",
+ " Updated sysutils/checkperms to 1.10 [rillig 2018-01-05]",
+ " \tUpdated sysutils/checkperms to 1.11 [rillig 2018-01-01]")
+
+ G.Main("pkglint", t.File("category/package"))
+
+ t.CheckOutputLines(
+ "WARN: ~/doc/CHANGES-2018:5: Package changes should be indented using a single tab, not \" \".",
+ "WARN: ~/doc/CHANGES-2018:6: Package changes should be indented using a single tab, not \" \\t\".",
+ "0 errors and 2 warnings found.",
+ "(Run \"pkglint -e\" to show explanations.)")
+}
+
+// Once or twice in a decade, changes to the pkgsrc infrastructure are also
+// documented in doc/CHANGES. These entries typically span multiple lines.
+func (s *Suite) Test_Pkgsrc_loadDocChangesFromFile__infrastructure(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package")
+ t.CreateFileLines("doc/CHANGES-2018",
+ RcsID,
+ "",
+ "Changes to the packages collection and infrastructure in 2018:",
+ "",
+ "\tmk/bsd.pkg.mk: Added new framework for handling packages",
+ "\t\twith multiple MASTER_SITES while fetching the main",
+ "\t\tdistfile directly from GitHub [rillig 2018-01-01]",
+ "\tmk/bsd.pkg.mk: Another infrastructure change [rillig 2018-01-02]")
+
+ G.Main("pkglint", t.File("category/package"))
+
+ // For pkglint's purpose, the infrastructure entries are simply ignored
+ // since they do not belong to a single package.
+ t.CheckOutputLines(
+ "Looks fine.")
}
-func (s *Suite) Test_Pkgsrc_loadDocChangesFromFile__wip(c *check.C) {
+func (s *Suite) Test_Pkgsrc_parseSuggestedUpdates__wip(c *check.C) {
t := s.Init(c)
pkg := t.SetUpPackage("wip/package",
- "DISTNAME=\tpackage-1.11",
- "CATEGORIES=\tlocal") // To avoid "Invalid category wip".
+ "DISTNAME=\tpackage-1.11")
t.CreateFileLines("wip/TODO",
RcsID,
"",
Index: pkgsrc/pkgtools/pkglint/files/vartype_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartype_test.go:1.14 pkgsrc/pkgtools/pkglint/files/vartype_test.go:1.15
--- pkgsrc/pkgtools/pkglint/files/vartype_test.go:1.14 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/vartype_test.go Sat Jan 26 16:31:33 2019
@@ -55,7 +55,7 @@ func (s *Suite) Test_ACLPermissions_Huma
equals, "") // Doesn't happen in practice
c.Check(aclpAll.HumanString(),
- equals, "set, given a default value, appended to, used at load time, used")
+ equals, "set, given a default value, appended to, used at load time, or used")
c.Check(aclpUnknown.HumanString(),
equals, "") // Doesn't happen in practice
Index: pkgsrc/pkgtools/pkglint/files/plist.go
diff -u pkgsrc/pkgtools/pkglint/files/plist.go:1.35 pkgsrc/pkgtools/pkglint/files/plist.go:1.36
--- pkgsrc/pkgtools/pkglint/files/plist.go:1.35 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/plist.go Sat Jan 26 16:31:33 2019
@@ -44,8 +44,8 @@ type PlistChecker struct {
type PlistLine struct {
Line
- condition string // e.g. PLIST.docs
- text string // Line.Text without any conditions of the form ${PLIST.cond}
+ conditions []string // e.g. PLIST.docs
+ text string // Line.Text without any conditions of the form ${PLIST.cond}
}
func (ck *PlistChecker) Check(plainLines Lines) {
@@ -79,14 +79,19 @@ func (ck *PlistChecker) Check(plainLines
func (ck *PlistChecker) NewLines(lines Lines) []*PlistLine {
plines := make([]*PlistLine, lines.Len())
for i, line := range lines.Lines {
- condition, text := "", line.Text
- if hasPrefix(text, "${PLIST.") {
- if m, cond, rest := match2(text, `^(?:\$\{(PLIST\.[\w-.]+)\})+(.*)`); m {
- condition, text = cond, rest
+ var conditions []string
+ text := line.Text
+
+ for hasPrefix(text, "${PLIST.") /* just for performance */ {
+ if m, cond, rest := match2(text, `^(?:\$\{(PLIST\.[\w-.]+)\})(.*)`); m {
+ conditions = append(conditions, cond)
+ text = rest
+ } else {
+ break
}
- // TODO: Support multiple conditions per line.
}
- plines[i] = &PlistLine{line, condition, text}
+
+ plines[i] = &PlistLine{line, conditions, text}
}
return plines
}
@@ -94,12 +99,13 @@ func (ck *PlistChecker) NewLines(lines L
var plistLineStart = textproc.NewByteSet("$0-9A-Za-z")
func (ck *PlistChecker) collectFilesAndDirs(plines []*PlistLine) {
+
for _, pline := range plines {
if text := pline.text; len(text) > 0 {
first := text[0]
switch {
case plistLineStart.Contains(first):
- if prev := ck.allFiles[text]; prev == nil || pline.condition < prev.condition {
+ if prev := ck.allFiles[text]; prev == nil || stringSliceLess(pline.conditions, prev.conditions) {
ck.allFiles[text] = pline
}
for dir := path.Dir(text); dir != "."; dir = path.Dir(dir) {
@@ -224,7 +230,7 @@ func (ck *PlistChecker) checkDuplicate(p
}
prev := ck.allFiles[text]
- if prev == pline || prev.condition != "" {
+ if prev == pline || len(prev.conditions) > 0 {
return
}
@@ -510,8 +516,8 @@ func (s *plistLineSorter) Sort() {
sort.SliceStable(s.middle, func(i, j int) bool {
mi := s.middle[i]
mj := s.middle[j]
- less := mi.text < mj.text || (mi.text == mj.text &&
- mi.condition < mj.condition)
+ less := mi.text < mj.text ||
+ (mi.text == mj.text && stringSliceLess(mi.conditions, mj.conditions))
if (i < j) != less {
s.changed = true
}
Index: pkgsrc/pkgtools/pkglint/files/util.go
diff -u pkgsrc/pkgtools/pkglint/files/util.go:1.35 pkgsrc/pkgtools/pkglint/files/util.go:1.36
--- pkgsrc/pkgtools/pkglint/files/util.go:1.35 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/util.go Sat Jan 26 16:31:33 2019
@@ -66,6 +66,15 @@ func replaceAllFunc(s string, re regex.P
return G.res.Compile(re).ReplaceAllStringFunc(s, repl)
}
+// intern returns an independent copy of the given string.
+//
+// It should be called when only a small substring of a large string
+// is needed for the rest of the program's lifetime.
+//
+// All strings allocated here will stay in memory forever,
+// therefore it should only be used for long-lived strings.
+func intern(str string) string { return G.interner.Intern(str) }
+
// trimHspace returns str, with leading and trailing space (U+0020)
// and tab (U+0009) removed.
//
@@ -340,11 +349,22 @@ func mkopSubst(s string, left bool, from
// relpath returns the relative path from `from` to `to`.
func relpath(from, to string) string {
+
+ // From "dir" to "dir/subdir/...".
if hasPrefix(to, from) && len(to) > len(from)+1 && to[len(from)] == '/' {
return path.Clean(to[len(from)+1:])
}
- // TODO: avoid these filesystem calls as they require IO.
+ // Take a shortcut for the most common variant in a complete pkgsrc scan,
+ // which is to resolve the relative path from a package to the pkgsrc root.
+ // This avoids unnecessary calls to the filesystem API.
+ if to == "." {
+ fromParts := strings.FieldsFunc(from, func(r rune) bool { return r == '/' })
+ if len(fromParts) == 3 && !hasPrefix(fromParts[0], ".") && !hasPrefix(fromParts[1], ".") && fromParts[2] == "." {
+ return "../.."
+ }
+ }
+
absFrom := abspath(from)
absTo := abspath(to)
rel, err := filepath.Rel(absFrom, absTo)
@@ -697,16 +717,12 @@ func (s *RedundantScope) Handle(mkline M
switch op {
case opAssign:
- if s.OnOverwrite != nil {
- s.OnOverwrite(existing.mkline, mkline)
- }
+ s.OnOverwrite(existing.mkline, mkline)
existing.value = value
case opAssignAppend:
existing.value += " " + value
case opAssignDefault:
- if s.OnIgnore != nil {
- s.OnIgnore(existing.mkline, mkline)
- }
+ s.OnIgnore(existing.mkline, mkline)
case opAssignShell, opAssignEval:
s.vars[varname] = nil // Won't be checked further.
}
@@ -856,17 +872,24 @@ func seeGuide(sectionName, sectionID str
sectionName, sectionID)
}
+// wrap performs automatic word wrapping on the given lines.
+//
+// Empty lines, indented lines and lines starting with "*" are kept as-is.
func wrap(max int, lines ...string) []string {
var wrapped []string
- var buf strings.Builder
+ var sb strings.Builder
nonSpace := textproc.Space.Inverse()
for _, line := range lines {
+
if line == "" || isHspace(line[0]) || line[0] == '*' {
- if buf.Len() > 0 {
- wrapped = append(wrapped, buf.String())
- buf.Reset()
+
+ // Finish current paragraph.
+ if sb.Len() > 0 {
+ wrapped = append(wrapped, sb.String())
+ sb.Reset()
}
+
wrapped = append(wrapped, line)
continue
}
@@ -877,25 +900,23 @@ func wrap(max int, lines ...string) []st
space := lexer.NextBytesSet(textproc.Space)
word := lexer.NextBytesSet(nonSpace)
- if bol && space == "" && buf.Len() > 0 {
+ if bol && sb.Len() > 0 {
space = " "
}
- if buf.Len() > 0 && buf.Len()+len(space)+len(word) > max {
- wrapped = append(wrapped, buf.String())
- buf.Reset()
- if hasPrefix(space, " ") {
- space = ""
- }
+ if sb.Len() > 0 && sb.Len()+len(space)+len(word) > max {
+ wrapped = append(wrapped, sb.String())
+ sb.Reset()
+ space = ""
}
- buf.WriteString(space)
- buf.WriteString(word)
+ sb.WriteString(space)
+ sb.WriteString(word)
}
}
- if buf.Len() > 0 {
- wrapped = append(wrapped, buf.String())
+ if sb.Len() > 0 {
+ wrapped = append(wrapped, sb.String())
}
return wrapped
@@ -929,3 +950,96 @@ func escapePrintable(s string) string {
}
return escaped.String()
}
+
+func stringSliceLess(a, b []string) bool {
+ limit := len(a)
+ if len(b) < limit {
+ limit = len(b)
+ }
+
+ for i := 0; i < limit; i++ {
+ if a[i] != b[i] {
+ return a[i] < b[i]
+ }
+ }
+
+ return len(a) < len(b)
+}
+
+func joinSkipEmpty(sep string, elements ...string) string {
+ var nonempty []string
+ for _, element := range elements {
+ if element != "" {
+ nonempty = append(nonempty, element)
+ }
+ }
+ return strings.Join(nonempty, sep)
+}
+
+func joinSkipEmptyCambridge(conn string, elements ...string) string {
+ var nonempty []string
+ for _, element := range elements {
+ if element != "" {
+ nonempty = append(nonempty, element)
+ }
+ }
+
+ var sb strings.Builder
+ for i, element := range nonempty {
+ if i > 0 {
+ if i == len(nonempty)-1 {
+ sb.WriteRune(' ')
+ sb.WriteString(conn)
+ sb.WriteRune(' ')
+ } else {
+ sb.WriteString(", ")
+ }
+ }
+ sb.WriteString(element)
+ }
+
+ return sb.String()
+}
+
+func joinSkipEmptyOxford(conn string, elements ...string) string {
+ var nonempty []string
+ for _, element := range elements {
+ if element != "" {
+ nonempty = append(nonempty, element)
+ }
+ }
+
+ if lastIndex := len(nonempty) - 1; lastIndex >= 1 {
+ nonempty[lastIndex] = conn + " " + nonempty[lastIndex]
+ }
+
+ return strings.Join(nonempty, ", ")
+}
+
+// StringInterner collects commonly used strings to avoid wasting heap memory
+// by duplicated strings.
+type StringInterner struct {
+ strs map[string]string
+}
+
+func NewStringInterner() StringInterner {
+ return StringInterner{make(map[string]string)}
+}
+
+func (si *StringInterner) Intern(str string) string {
+ interned, found := si.strs[str]
+ if found {
+ return interned
+ }
+
+ // Ensure that the original string is never stored directly in the map
+ // since it might be a substring of a very large string. The interned
+ // strings must be completely independent of anything from the outside,
+ // so that the large source string can be freed afterwards.
+ var sb strings.Builder
+ sb.WriteString(str)
+ key := sb.String()
+
+ si.strs[key] = key
+ return key
+}
Index: pkgsrc/pkgtools/pkglint/files/util_test.go
diff -u pkgsrc/pkgtools/pkglint/files/util_test.go:1.21 pkgsrc/pkgtools/pkglint/files/util_test.go:1.22
--- pkgsrc/pkgtools/pkglint/files/util_test.go:1.21 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/util_test.go Sat Jan 26 16:31:33 2019
@@ -111,10 +111,17 @@ func (s *Suite) Test_cleanpath(c *check.
// Relpath is called so often that handling the most common calls
// without file system IO makes sense.
func (s *Suite) Test_relpath__quick(c *check.C) {
- c.Check(relpath("some/dir", "some/dir/../.."), equals, "../..")
- c.Check(relpath("some/dir", "some/dir/./././../.."), equals, "../..")
- c.Check(relpath("some/dir", "some/dir/"), equals, ".")
- c.Check(relpath("some/dir", "some/directory"), equals, "../directory")
+
+ test := func(from, to, result string) {
+ c.Check(relpath(from, to), equals, result)
+ }
+
+ test("some/dir", "some/dir/../..", "../..")
+ test("some/dir", "some/dir/./././../..", "../..")
+ test("some/dir", "some/dir/", ".")
+ test("some/dir", "some/directory", "../directory")
+
+ test("category/package/.", ".", "../..")
}
// This is not really an internal error but won't happen in practice anyway.
@@ -547,7 +554,10 @@ func (s *Suite) Test_wrap(c *check.C) {
"with",
"linebreaks.",
"",
- "Sentence one. Sentence two.")
+ "Sentence one. Sentence two.",
+ "",
+ "A\tB\tC\tD",
+ "E\tveryVeryVeryVeryVeryVeryVeryVeryLong")
expected := []string{
"See the pkgsrc",
@@ -573,7 +583,10 @@ func (s *Suite) Test_wrap(c *check.C) {
"linebreaks.",
"",
"Sentence one.",
- "Sentence two."}
+ "Sentence two.",
+ "",
+ "A\tB\tC\tD E",
+ "veryVeryVeryVeryVeryVeryVeryVeryLong"}
c.Check(wrapped, deepEquals, expected)
}
@@ -585,3 +598,77 @@ func (s *Suite) Test_escapePrintable(c *
c.Check(escapePrintable("Bad \xFF character"), equals, "Bad <\\xFF> character")
c.Check(escapePrintable("Unicode \uFFFD replacement"), equals, "Unicode <U+FFFD> replacement")
}
+
+func (s *Suite) Test_stringSliceLess(c *check.C) {
+ var elements = [][][]string{
+ {nil, {}},
+ {{"a"}},
+ {{"a", "a"}},
+ {{"a", "b"}},
+ {{"b"}},
+ {{"b", "a"}}}
+
+ test := func(i int, iElement []string, j int, jElement []string) {
+ actual := stringSliceLess(iElement, jElement)
+ expected := i < j
+ if actual != expected {
+ c.Check(
+ []interface{}{i, iElement, j, jElement, actual},
+ check.DeepEquals,
+ []interface{}{i, iElement, j, jElement, expected})
+ }
+ }
+
+ for i, iElements := range elements {
+ for j, jElements := range elements {
+ for _, iElement := range iElements {
+ for _, jElement := range jElements {
+ test(i, iElement, j, jElement)
+ }
+ }
+ }
+ }
+}
+
+func (s *Suite) Test_joinSkipEmpty(c *check.C) {
+ t := s.Init(c)
+
+ t.Check(
+ joinSkipEmpty(", ", "", "one", "", "", "two", "", "three"),
+ deepEquals,
+ "one, two, three")
+}
+
+func (s *Suite) Test_joinSkipEmptyCambridge(c *check.C) {
+ t := s.Init(c)
+
+ t.Check(
+ joinSkipEmptyCambridge("and", "", "one", "", "", "two", "", "three"),
+ deepEquals,
+ "one, two and three")
+
+ t.Check(
+ joinSkipEmptyCambridge("and", "", "one", "", ""),
+ deepEquals,
+ "one")
+}
+
+func (s *Suite) Test_joinSkipEmptyOxford(c *check.C) {
+ t := s.Init(c)
+
+ t.Check(
+ joinSkipEmptyOxford("and", "", "one", "", "", "two", "", "three"),
+ deepEquals,
+ "one, two, and three")
+}
+
+func (s *Suite) Test_StringInterner(c *check.C) {
+ t := s.Init(c)
+
+ si := NewStringInterner()
+
+ t.Check(si.Intern(""), equals, "")
+ t.Check(si.Intern("Hello"), equals, "Hello")
+ t.Check(si.Intern("Hello, world"), equals, "Hello, world")
+ t.Check(si.Intern("Hello, world"[0:5]), equals, "Hello")
+}
Index: pkgsrc/pkgtools/pkglint/files/vardefs.go
diff -u pkgsrc/pkgtools/pkglint/files/vardefs.go:1.54 pkgsrc/pkgtools/pkglint/files/vardefs.go:1.55
--- pkgsrc/pkgtools/pkglint/files/vardefs.go:1.54 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/vardefs.go Sat Jan 26 16:31:33 2019
@@ -94,7 +94,7 @@ func (src *Pkgsrc) InitVartypes() {
words := mkline.ValueFields(mkline.Args())
if len(words) > 2 && words[0] == "_version_" {
for _, word := range words[2:] {
- languages[word] = true
+ languages[intern(word)] = true
}
}
}
@@ -130,7 +130,7 @@ func (src *Pkgsrc) InitVartypes() {
words, _ := splitIntoMkWords(mkline.Line, mkline.Value())
for _, word := range words {
if !contains(word, "$") {
- values[word] = true
+ values[intern(word)] = true
}
}
}
@@ -970,9 +970,14 @@ func (src *Pkgsrc) InitVartypes() {
sys("PGPKGSRCDIR", lkNone, BtPathname)
sys("PHASE_MSG", lkNone, BtShellCommand)
usr("PHP_VERSION_REQD", lkNone, BtVersion)
+ acl("PHP_PKG_PREFIX", lkNone, enumFromDirs("lang", `^php(\d+)$`, "php$1", "php56 php71 php72 php73"), ""+
+ "special:phpversion.mk: set; "+
+ "*: use-loadtime, use")
sys("PKGBASE", lkNone, BtIdentifier)
- acl("PKGCONFIG_FILE.*", lkShell, BtPathname, "builtin.mk: set, append; pkgconfig-builtin.mk: use-loadtime")
- acl("PKGCONFIG_OVERRIDE", lkShell, BtPathmask, "Makefile: set, append; Makefile.common: append")
+ acl("PKGCONFIG_FILE.*", lkShell, BtPathname,
+ "builtin.mk: set, append; special:pkgconfig-builtin.mk: use-loadtime")
+ acl("PKGCONFIG_OVERRIDE", lkShell, BtPathmask,
+ "Makefile: set, append; Makefile.common: append")
pkg("PKGCONFIG_OVERRIDE_STAGE", lkNone, BtStage)
pkg("PKGDIR", lkNone, BtRelativePkgDir)
sys("PKGDIRMODE", lkNone, BtFileMode)
@@ -1068,7 +1073,7 @@ func (src *Pkgsrc) InitVartypes() {
sysload("PTHREAD_TYPE", lkNone, BtIdentifier) // Or "native" or "none".
pkg("PY_PATCHPLIST", lkNone, BtYes)
acl("PYPKGPREFIX", lkNone, enumFromDirs("lang", `^python(\d+)$`, "py$1", "py27 py36"), ""+
- "pyversion.mk: set; "+
+ "special:pyversion.mk: set; "+
"*: use-loadtime, use")
pkg("PYTHON_FOR_BUILD_ONLY", lkNone, enum("yes no test tool YES")) // See lang/python/pyversion.mk
pkglist("REPLACE_PYTHON", lkShell, BtPathmask)
@@ -1104,7 +1109,13 @@ func (src *Pkgsrc) InitVartypes() {
pkg("RESTRICTED", lkNone, BtMessage)
usr("ROOT_USER", lkNone, BtUserGroupName)
usr("ROOT_GROUP", lkNone, BtUserGroupName)
+ acl("RUBY_BASE", lkNone, enumFromDirs("lang", `^ruby(\d+)$`, "ruby$1", "ruby22 ruby23 ruby24 ruby25"), ""+
+ "special:rubyversion.mk: set; "+
+ "*: use-loadtime, use")
usr("RUBY_VERSION_REQD", lkNone, BtVersion)
+ acl("RUBY_PKGPREFIX", lkNone, enumFromDirs("lang", `^ruby(\d+)$`, "ruby$1", "ruby22 ruby23 ruby24 ruby25"), ""+
+ "special:rubyversion.mk: set, default, use; "+
+ "*: use-loadtime, use")
sys("RUN", lkNone, BtShellCommand)
sys("RUN_LDCONFIG", lkNone, BtYesNo)
acl("SCRIPTS_ENV", lkShell, BtShellWord, "Makefile, Makefile.common: append")
@@ -1268,14 +1279,19 @@ func parseACLEntries(varname string, acl
case "*",
"Makefile", "Makefile.common", "Makefile.*",
"buildlink3.mk", "builtin.mk", "options.mk", "hacks.mk", "*.mk",
- "bsd.options.mk", "pkgconfig-builtin.mk", "pyversion.mk":
+ "bsd.options.mk":
break
default:
- G.Assertf(false, "Invalid ACL glob %q for %q.", glob, varname)
+ withoutSpecial := strings.TrimPrefix(glob, "special:")
+ if withoutSpecial == glob {
+ G.Assertf(false, "Invalid ACL glob %q for %q.", glob, varname)
+ } else {
+ glob = withoutSpecial
+ }
}
for _, prev := range result {
matched, err := path.Match(prev.glob, glob)
- G.AssertNil(err, "Invalid ACL pattern %q for %q.", glob, varname)
+ G.AssertNil(err, "Invalid ACL pattern %q for %q", glob, varname)
G.Assertf(!matched, "Unreachable ACL pattern %q for %q.", glob, varname)
}
result = append(result, ACLEntry{glob, permissions})
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.47 pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.48
--- pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.47 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck.go Sat Jan 26 16:31:33 2019
@@ -172,6 +172,7 @@ func (cv *VartypeCheck) Category() {
if cv.Value != "wip" && fileExists(G.Pkgsrc.File(cv.Value+"/Makefile")) {
return
}
+
switch cv.Value {
case
"chinese", "crosspkgtools",
@@ -811,8 +812,6 @@ func (cv *VartypeCheck) PathMask() {
if !matches(cv.ValueNoVar, `^[#%*+\-./0-9?@A-Z\[\]_a-z~]*$`) {
cv.Warnf("%q is not a valid pathname mask.", cv.Value)
}
-
- LineChecker{cv.MkLine.Line}.CheckAbsolutePathname(cv.Value)
}
// Pathname checks for pathnames.
@@ -828,8 +827,6 @@ func (cv *VartypeCheck) Pathname() {
if !matches(cv.ValueNoVar, `^[#\-0-9A-Za-z._~+%/]*$`) {
cv.Warnf("%q is not a valid pathname.", cv.Value)
}
-
- LineChecker{cv.MkLine.Line}.CheckAbsolutePathname(cv.Value)
}
func (cv *VartypeCheck) Perl5Packlist() {
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.40 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.41
--- pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.40 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go Sat Jan 26 16:31:33 2019
@@ -166,7 +166,6 @@ func (s *Suite) Test_VartypeCheck_ConfFi
vt.Output(
"WARN: filename:1: Values for CONF_FILES should always be pairs of paths.",
"WARN: filename:3: Values for CONF_FILES should always be pairs of paths.",
- "WARN: filename:5: Found absolute pathname: /etc/bootrc",
"WARN: filename:5: The destination file \"/etc/bootrc\" should start with a variable reference.")
}
@@ -804,7 +803,6 @@ func (s *Suite) Test_VartypeCheck_PathMa
"src/*/*")
vt.Output(
- "WARN: filename:1: Found absolute pathname: /home/user/*",
"WARN: filename:2: \"src/*&*\" is not a valid pathname mask.")
vt.Op(opUseMatch)
@@ -826,9 +824,9 @@ func (s *Suite) Test_VartypeCheck_Pathna
vt.Values(
"anything")
+ // FIXME: Warn about the absolute pathname in line 4.
vt.Output(
- "WARN: filename:1: \"${PREFIX}/*\" is not a valid pathname.",
- "WARN: filename:4: Found absolute pathname: /bin")
+ "WARN: filename:1: \"${PREFIX}/*\" is not a valid pathname.")
}
func (s *Suite) Test_VartypeCheck_Perl5Packlist(c *check.C) {
Added files:
Index: pkgsrc/pkgtools/pkglint/files/linelexer.go
diff -u /dev/null pkgsrc/pkgtools/pkglint/files/linelexer.go:1.1
--- /dev/null Sat Jan 26 16:31:34 2019
+++ pkgsrc/pkgtools/pkglint/files/linelexer.go Sat Jan 26 16:31:33 2019
@@ -0,0 +1,139 @@
+package pkglint
+
+import (
+ "netbsd.org/pkglint/regex"
+ "strings"
+)
+
+// LinesLexer records the state when checking a list of lines from top to bottom.
+type LinesLexer struct {
+ lines Lines
+ index int
+}
+
+func NewLinesLexer(lines Lines) *LinesLexer {
+ return &LinesLexer{lines, 0}
+}
+
+func (llex *LinesLexer) CurrentLine() Line {
+ if llex.index < llex.lines.Len() {
+ return llex.lines.Lines[llex.index]
+ }
+ return NewLineEOF(llex.lines.FileName)
+}
+
+func (llex *LinesLexer) PreviousLine() Line {
+ return llex.lines.Lines[llex.index-1]
+}
+
+func (llex *LinesLexer) EOF() bool {
+ return !(llex.index < llex.lines.Len())
+}
+
+// Skip skips the current line and returns true.
+func (llex *LinesLexer) Skip() bool {
+ if llex.EOF() {
+ return false
+ }
+ llex.index++
+ return true
+}
+
+func (llex *LinesLexer) Undo() {
+ llex.index--
+}
+
+func (llex *LinesLexer) NextRegexp(re regex.Pattern) []string {
+ if trace.Tracing {
+ defer trace.Call(llex.CurrentLine().Text, re)()
+ }
+
+ if !llex.EOF() {
+ if m := G.res.Match(llex.lines.Lines[llex.index].Text, re); m != nil {
+ llex.index++
+ return m
+ }
+ }
+ return nil
+}
+
+func (llex *LinesLexer) SkipRegexp(re regex.Pattern) bool {
+ return llex.NextRegexp(re) != nil
+}
+
+func (llex *LinesLexer) SkipPrefix(prefix string) bool {
+ if trace.Tracing {
+ defer trace.Call2(llex.CurrentLine().Text, prefix)()
+ }
+
+ return !llex.EOF() && strings.HasPrefix(llex.lines.Lines[llex.index].Text, prefix) && llex.Skip()
+}
+
+func (llex *LinesLexer) SkipString(text string) bool {
+ if trace.Tracing {
+ defer trace.Call2(llex.CurrentLine().Text, text)()
+ }
+
+ return !llex.EOF() && llex.lines.Lines[llex.index].Text == text && llex.Skip()
+}
+
+func (llex *LinesLexer) SkipEmptyOrNote() bool {
+ if llex.SkipString("") {
+ return true
+ }
+
+ if G.Opts.WarnSpace {
+ if llex.index == 0 {
+ fix := llex.CurrentLine().Autofix()
+ fix.Notef("Empty line expected before this line.")
+ fix.InsertBefore("")
+ fix.Apply()
+ } else {
+ fix := llex.PreviousLine().Autofix()
+ fix.Notef("Empty line expected after this line.")
+ fix.InsertAfter("")
+ fix.Apply()
+ }
+ }
+ return false
+}
+
+func (llex *LinesLexer) SkipContainsOrWarn(text string) bool {
+ result := llex.SkipString(text)
+ if !result {
+ llex.CurrentLine().Warnf("This line should contain the following text: %s", text)
+ }
+ return result
+}
+
+// MkLinesLexer records the state when checking a list of Makefile lines from top to bottom.
+type MkLinesLexer struct {
+ mklines MkLines
+ LinesLexer
+}
+
+func NewMkLinesLexer(mklines MkLines) *MkLinesLexer {
+ return &MkLinesLexer{mklines, *NewLinesLexer(mklines.lines)}
+}
+
+func (mlex *MkLinesLexer) PreviousMkLine() MkLine {
+ return mlex.mklines.mklines[mlex.index-1]
+}
+
+func (mlex *MkLinesLexer) CurrentMkLine() MkLine {
+ return mlex.mklines.mklines[mlex.index]
+}
+
+func (mlex *MkLinesLexer) SkipWhile(pred func(mkline MkLine) bool) {
+ if trace.Tracing {
+ defer trace.Call(mlex.CurrentMkLine().Text)()
+ }
+
+ for !mlex.EOF() && pred(mlex.CurrentMkLine()) {
+ mlex.Skip()
+ }
+}
+
+func (mlex *MkLinesLexer) SkipIf(pred func(mkline MkLine) bool) bool {
+ return !mlex.EOF() && pred(mlex.CurrentMkLine()) && mlex.Skip()
+}
Index: pkgsrc/pkgtools/pkglint/files/linelexer_test.go
diff -u /dev/null pkgsrc/pkgtools/pkglint/files/linelexer_test.go:1.1
--- /dev/null Sat Jan 26 16:31:34 2019
+++ pkgsrc/pkgtools/pkglint/files/linelexer_test.go Sat Jan 26 16:31:33 2019
@@ -0,0 +1,36 @@
+package pkglint
+
+import (
+ "gopkg.in/check.v1"
+)
+
+func (s *Suite) Test_LinesLexer_SkipEmptyOrNote__beginning_of_file(c *check.C) {
+ t := s.Init(c)
+
+ lines := t.NewLines("file.txt",
+ "line 1",
+ "line 2")
+ llex := NewLinesLexer(lines)
+
+ llex.SkipEmptyOrNote()
+
+ t.CheckOutputLines(
+ "NOTE: file.txt:1: Empty line expected before this line.")
+}
+
+func (s *Suite) Test_LinesLexer_SkipEmptyOrNote__end_of_file(c *check.C) {
+ t := s.Init(c)
+
+ lines := t.NewLines("file.txt",
+ "line 1",
+ "line 2")
+ llex := NewLinesLexer(lines)
+
+ for llex.Skip() {
+ }
+
+ llex.SkipEmptyOrNote()
+
+ t.CheckOutputLines(
+ "NOTE: file.txt:2: Empty line expected after this line.")
+}
Index: pkgsrc/pkgtools/pkglint/files/cmd/pkglint/main.go
diff -u /dev/null pkgsrc/pkgtools/pkglint/files/cmd/pkglint/main.go:1.1
--- /dev/null Sat Jan 26 16:31:34 2019
+++ pkgsrc/pkgtools/pkglint/files/cmd/pkglint/main.go Sat Jan 26 16:31:33 2019
@@ -0,0 +1,12 @@
+package main
+
+import (
+ "netbsd.org/pkglint"
+ "os"
+)
+
+var exit = os.Exit
+
+func main() {
+ exit(pkglint.Main())
+}
Index: pkgsrc/pkgtools/pkglint/files/cmd/pkglint/main_test.go
diff -u /dev/null pkgsrc/pkgtools/pkglint/files/cmd/pkglint/main_test.go:1.1
--- /dev/null Sat Jan 26 16:31:34 2019
+++ pkgsrc/pkgtools/pkglint/files/cmd/pkglint/main_test.go Sat Jan 26 16:31:33 2019
@@ -0,0 +1,48 @@
+package main
+
+import (
+ "io/ioutil"
+ "os"
+ "testing"
+
+ "gopkg.in/check.v1"
+)
+
+type Suite struct{}
+
+func Test(t *testing.T) {
+ check.Suite(new(Suite))
+ check.TestingT(t)
+}
+
+// This test goes into great lengths to bring the code coverage to 100%.
+// Without this test, the main function would be a trivial one-liner.
+// To make that one-liner testable, the call to os.Exit must be mockable.
+func (s *Suite) Test_main(c *check.C) {
+ tmpdir := c.MkDir()
+ out, err := os.Create(tmpdir + "/out")
+ c.Assert(err, check.IsNil)
+
+ prevStdout := os.Stdout
+ prevArgs := os.Args
+ prevExit := exit
+ defer func() {
+ os.Stdout = prevStdout
+ os.Args = prevArgs
+ exit = prevExit
+ }()
+
+ os.Stdout = out
+ os.Args = []string{"pkglint", "--version"}
+ exit = func(code int) { c.Check(code, check.Equals, 0) }
+
+ main()
+
+ err = out.Close()
+ c.Assert(err, check.IsNil)
+
+ output, err := ioutil.ReadFile(out.Name())
+ c.Assert(err, check.IsNil)
+
+ c.Check(string(output), check.Matches, `^(@VERSION@|\d+(\.\d+)+)\n$`)
+}
Home |
Main Index |
Thread Index |
Old Index