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 Feb 15 13:48:40 UTC 2020
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile
pkgsrc/pkgtools/pkglint/files: autofix.go autofix_test.go buildlink3.go
buildlink3_test.go category.go category_test.go check_test.go
distinfo.go distinfo_test.go files.go homepage.go homepage_test.go
line.go line_test.go linechecker.go lines.go lines_test.go
lineslexer.go lineslexer_test.go logging.go logging_test.go
mkassignchecker.go mkassignchecker_test.go mkcondchecker_test.go
mkline.go mklinechecker.go mklineparser.go mklines.go
mklines_test.go mkshparser_test.go mkvarusechecker.go package.go
package_test.go patches.go patches_test.go path.go path_test.go
pkglint.go pkglint_test.go pkgsrc.go pkgsrc_test.go plist.go
redundantscope.go redundantscope_test.go shell.go toplevel.go
toplevel_test.go util.go util_test.go var.go varalignblock.go
varalignblock_test.go vardefs.go vartype.go vartypecheck.go
vartypecheck_test.go
Log Message:
pkgtools/pkglint: update to 19.4.8
Changes since 19.4.7:
The diagnostic for homepages using FTP is simpler now.
When running pkglint recursively on the top-level directory, the
inter-package checks (distfile hashes, unused licenses) are enabled
implicitly. This way, the only effect of the -Cglobal option is now
whether the pkgsrc infrastructure files are checked as well.
The check for removed packages that have not been recorded in
doc/CHANGES prints the correct lines when pkglint is run with the
--source option.
Fatal technical errors are no longer treated as diagnostics since they
are none. That was an early conceptual mistake, but since these fatal
error didn't happen often, it didn't matter.
In diagnostics, when referring to other lines, the previously used words
before/after have been replaced with above/below to avoid any confusion
whether space or time is meant.
In CONF_FILES, spaces and quotes are allowed.
See https://gnats.netbsd.org/42191.
Fixed unintended side-effects when running pkglint --autofix --only.
Before, all fixes were applied to the file, whether or not they matched
the --only option.
Fixed resolution of relative paths of the form ../../category/package
when they appeared in an infrastructure file.
Lots of refactorings and housekeeping, as usual.
To generate a diff of this commit:
cvs rdiff -u -r1.630 -r1.631 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.37 -r1.38 pkgsrc/pkgtools/pkglint/files/autofix.go
cvs rdiff -u -r1.39 -r1.40 pkgsrc/pkgtools/pkglint/files/autofix_test.go
cvs rdiff -u -r1.31 -r1.32 pkgsrc/pkgtools/pkglint/files/buildlink3.go \
pkgsrc/pkgtools/pkglint/files/category.go \
pkgsrc/pkgtools/pkglint/files/category_test.go \
pkgsrc/pkgtools/pkglint/files/files.go
cvs rdiff -u -r1.40 -r1.41 pkgsrc/pkgtools/pkglint/files/buildlink3_test.go \
pkgsrc/pkgtools/pkglint/files/logging.go
cvs rdiff -u -r1.65 -r1.66 pkgsrc/pkgtools/pkglint/files/check_test.go
cvs rdiff -u -r1.41 -r1.42 pkgsrc/pkgtools/pkglint/files/distinfo.go
cvs rdiff -u -r1.36 -r1.37 pkgsrc/pkgtools/pkglint/files/distinfo_test.go \
pkgsrc/pkgtools/pkglint/files/patches.go
cvs rdiff -u -r1.3 -r1.4 pkgsrc/pkgtools/pkglint/files/homepage.go \
pkgsrc/pkgtools/pkglint/files/mkassignchecker_test.go
cvs rdiff -u -r1.2 -r1.3 pkgsrc/pkgtools/pkglint/files/homepage_test.go \
pkgsrc/pkgtools/pkglint/files/lineslexer.go
cvs rdiff -u -r1.45 -r1.46 pkgsrc/pkgtools/pkglint/files/line.go
cvs rdiff -u -r1.20 -r1.21 pkgsrc/pkgtools/pkglint/files/line_test.go
cvs rdiff -u -r1.17 -r1.18 pkgsrc/pkgtools/pkglint/files/linechecker.go
cvs rdiff -u -r1.12 -r1.13 pkgsrc/pkgtools/pkglint/files/lines.go \
pkgsrc/pkgtools/pkglint/files/mklineparser.go \
pkgsrc/pkgtools/pkglint/files/varalignblock_test.go
cvs rdiff -u -r1.11 -r1.12 pkgsrc/pkgtools/pkglint/files/lines_test.go \
pkgsrc/pkgtools/pkglint/files/redundantscope.go
cvs rdiff -u -r1.1 -r1.2 pkgsrc/pkgtools/pkglint/files/lineslexer_test.go
cvs rdiff -u -r1.26 -r1.27 pkgsrc/pkgtools/pkglint/files/logging_test.go
cvs rdiff -u -r1.6 -r1.7 pkgsrc/pkgtools/pkglint/files/mkassignchecker.go
cvs rdiff -u -r1.5 -r1.6 pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go \
pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go
cvs rdiff -u -r1.75 -r1.76 pkgsrc/pkgtools/pkglint/files/mkline.go
cvs rdiff -u -r1.63 -r1.64 pkgsrc/pkgtools/pkglint/files/mklinechecker.go
cvs rdiff -u -r1.67 -r1.68 pkgsrc/pkgtools/pkglint/files/mklines.go \
pkgsrc/pkgtools/pkglint/files/package_test.go
cvs rdiff -u -r1.61 -r1.62 pkgsrc/pkgtools/pkglint/files/mklines_test.go
cvs rdiff -u -r1.23 -r1.24 pkgsrc/pkgtools/pkglint/files/mkshparser_test.go
cvs rdiff -u -r1.79 -r1.80 pkgsrc/pkgtools/pkglint/files/package.go
cvs rdiff -u -r1.35 -r1.36 pkgsrc/pkgtools/pkglint/files/patches_test.go
cvs rdiff -u -r1.8 -r1.9 pkgsrc/pkgtools/pkglint/files/path.go
cvs rdiff -u -r1.9 -r1.10 pkgsrc/pkgtools/pkglint/files/path_test.go
cvs rdiff -u -r1.73 -r1.74 pkgsrc/pkgtools/pkglint/files/pkglint.go
cvs rdiff -u -r1.59 -r1.60 pkgsrc/pkgtools/pkglint/files/pkglint_test.go
cvs rdiff -u -r1.49 -r1.50 pkgsrc/pkgtools/pkglint/files/pkgsrc.go
cvs rdiff -u -r1.42 -r1.43 pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go
cvs rdiff -u -r1.52 -r1.53 pkgsrc/pkgtools/pkglint/files/plist.go
cvs rdiff -u -r1.13 -r1.14 \
pkgsrc/pkgtools/pkglint/files/redundantscope_test.go
cvs rdiff -u -r1.57 -r1.58 pkgsrc/pkgtools/pkglint/files/shell.go
cvs rdiff -u -r1.28 -r1.29 pkgsrc/pkgtools/pkglint/files/toplevel.go
cvs rdiff -u -r1.22 -r1.23 pkgsrc/pkgtools/pkglint/files/toplevel_test.go
cvs rdiff -u -r1.72 -r1.73 pkgsrc/pkgtools/pkglint/files/util.go
cvs rdiff -u -r1.47 -r1.48 pkgsrc/pkgtools/pkglint/files/util_test.go
cvs rdiff -u -r1.7 -r1.8 pkgsrc/pkgtools/pkglint/files/var.go
cvs rdiff -u -r1.16 -r1.17 pkgsrc/pkgtools/pkglint/files/varalignblock.go
cvs rdiff -u -r1.88 -r1.89 pkgsrc/pkgtools/pkglint/files/vardefs.go
cvs rdiff -u -r1.46 -r1.47 pkgsrc/pkgtools/pkglint/files/vartype.go
cvs rdiff -u -r1.81 -r1.82 pkgsrc/pkgtools/pkglint/files/vartypecheck.go
cvs rdiff -u -r1.76 -r1.77 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: pkgsrc/pkgtools/pkglint/Makefile
diff -u pkgsrc/pkgtools/pkglint/Makefile:1.630 pkgsrc/pkgtools/pkglint/Makefile:1.631
--- pkgsrc/pkgtools/pkglint/Makefile:1.630 Wed Feb 5 04:09:00 2020
+++ pkgsrc/pkgtools/pkglint/Makefile Sat Feb 15 13:48:40 2020
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.630 2020/02/05 04:09:00 rillig Exp $
+# $NetBSD: Makefile,v 1.631 2020/02/15 13:48:40 rillig Exp $
-PKGNAME= pkglint-19.4.7
+PKGNAME= pkglint-19.4.8
CATEGORIES= pkgtools
DISTNAME= tools
MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/}
Index: pkgsrc/pkgtools/pkglint/files/autofix.go
diff -u pkgsrc/pkgtools/pkglint/files/autofix.go:1.37 pkgsrc/pkgtools/pkglint/files/autofix.go:1.38
--- pkgsrc/pkgtools/pkglint/files/autofix.go:1.37 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/autofix.go Sat Feb 15 13:48:40 2020
@@ -17,9 +17,10 @@ type Autofixer interface {
// The modifications are kept in memory only,
// until they are written to disk by SaveAutofixChanges.
type Autofix struct {
- line *Line
- linesBefore []string // Newly inserted lines, including \n
- linesAfter []string // Newly inserted lines, including \n
+ line *Line
+ above []string // Newly inserted lines, including \n
+ texts []string // Modified lines, including \n
+ below []string // Newly inserted lines, including \n
// Whether an actual fix has been applied to the text of the raw lines
modified bool
@@ -61,7 +62,11 @@ const SilentAutofixFormat = "SilentAutof
const autofixFormat = "AutofixFormat"
func NewAutofix(line *Line) *Autofix {
- return &Autofix{line: line}
+ texts := make([]string, len(line.raw))
+ for i, rawLine := range line.raw {
+ texts[i] = rawLine.orignl
+ }
+ return &Autofix{line, nil, texts, nil, false, autofixShortTerm{}}
}
// Errorf remembers the error for logging it later when Apply is called.
@@ -108,31 +113,33 @@ func (fix *Autofix) ReplaceAfter(prefix,
prefixTo := prefix + to
n := 0
- for _, rawLine := range fix.line.raw {
- n += strings.Count(rawLine.textnl, prefixFrom)
+ for _, text := range fix.texts {
+ n += strings.Count(text, prefixFrom)
}
if n != 1 {
return
}
- for _, rawLine := range fix.line.raw {
- ok, replaced := replaceOnce(rawLine.textnl, prefixFrom, prefixTo)
- if ok {
- if G.Logger.IsAutofix() {
- rawLine.textnl = replaced
-
- // Fix the parsed text as well.
- // This is only approximate and won't work in some edge cases
- // that involve escaped comments or replacements across line breaks.
- //
- // TODO: Do this properly by parsing the whole line again,
- // and ideally everything that depends on the parsed line.
- // This probably requires a generic notification mechanism.
- _, fix.line.Text = replaceOnce(fix.line.Text, prefixFrom, prefixTo)
- }
- fix.Describef(rawLine.Lineno, "Replacing %q with %q.", from, to)
- return
+ for rawIndex, text := range fix.texts {
+ ok, replaced := replaceOnce(text, prefixFrom, prefixTo)
+ if !ok {
+ continue
+ }
+
+ if G.Logger.IsAutofix() {
+ fix.texts[rawIndex] = replaced
+
+ // Fix the parsed text as well.
+ // This is only approximate and won't work in some edge cases
+ // that involve escaped comments or replacements across line breaks.
+ //
+ // TODO: Do this properly by parsing the whole line again,
+ // and ideally everything that depends on the parsed line.
+ // This probably requires a generic notification mechanism.
+ _, fix.line.Text = replaceOnce(fix.line.Text, prefixFrom, prefixTo)
}
+ fix.Describef(rawIndex, "Replacing %q with %q.", from, to)
+ return
}
}
@@ -142,13 +149,17 @@ func (fix *Autofix) ReplaceAt(rawIndex i
assert(from != to)
fix.assertRealLine()
- rawLine := fix.line.raw[rawIndex]
- assert(textIndex < len(rawLine.textnl))
- assert(hasPrefix(rawLine.textnl[textIndex:], from))
+ if fix.skip() {
+ return
+ }
+
+ text := fix.texts[rawIndex]
+ assert(textIndex < len(text))
+ assert(hasPrefix(text[textIndex:], from))
- replaced := rawLine.textnl[:textIndex] + to + rawLine.textnl[textIndex+len(from):]
+ replaced := text[:textIndex] + to + text[textIndex+len(from):]
- rawLine.textnl = replaced
+ fix.texts[rawIndex] = replaced
// Fix the parsed text as well.
// This is only approximate and won't work in some edge cases
@@ -159,38 +170,35 @@ func (fix *Autofix) ReplaceAt(rawIndex i
// This probably requires a generic notification mechanism.
_, fix.line.Text = replaceOnce(fix.line.Text, from, to)
- if fix.skip() {
- return
- }
- fix.Describef(rawLine.Lineno, "Replacing %q with %q.", from, to)
+ fix.Describef(rawIndex, "Replacing %q with %q.", from, to)
}
-// InsertBefore prepends a line before the current line.
+// InsertAbove prepends a line above the current line.
// The newline is added internally.
-func (fix *Autofix) InsertBefore(text string) {
+func (fix *Autofix) InsertAbove(text string) {
fix.assertRealLine()
if fix.skip() {
return
}
- fix.linesBefore = append(fix.linesBefore, text+"\n")
- fix.Describef(fix.line.raw[0].Lineno, "Inserting a line %q before this line.", text)
+ fix.above = append(fix.above, text+"\n")
+ fix.Describef(0, "Inserting a line %q above this line.", text)
}
-// InsertAfter appends a line after the current line.
+// InsertBelow appends a line below the current line.
// The newline is added internally.
-func (fix *Autofix) InsertAfter(text string) {
+func (fix *Autofix) InsertBelow(text string) {
fix.assertRealLine()
if fix.skip() {
return
}
- fix.linesAfter = append(fix.linesAfter, text+"\n")
- fix.Describef(fix.line.raw[len(fix.line.raw)-1].Lineno, "Inserting a line %q after this line.", text)
+ fix.below = append(fix.below, text+"\n")
+ fix.Describef(len(fix.line.raw)-1, "Inserting a line %q below this line.", text)
}
// Delete removes the current line completely.
-// It can be combined with InsertAfter or InsertBefore to
+// It can be combined with InsertBelow or InsertAbove to
// replace the complete line with some different text.
func (fix *Autofix) Delete() {
fix.assertRealLine()
@@ -198,9 +206,9 @@ func (fix *Autofix) Delete() {
return
}
- for _, line := range fix.line.raw {
- line.textnl = ""
- fix.Describef(line.Lineno, "Deleting this line.")
+ for rawIndex := range fix.texts {
+ fix.texts[rawIndex] = ""
+ fix.Describef(rawIndex, "Deleting this line.")
}
}
@@ -210,7 +218,7 @@ func (fix *Autofix) Delete() {
// The fixer function must check whether it can actually fix something,
// and if so, call Describef to describe the actual fix.
//
-// If autofix is false, the the fix should be applied, as far as only
+// If autofix is false, the fix should be applied, as far as only
// in-memory data structures are effected, and these are not written
// back to disk. No externally observable modification must be done.
// For example, changing the text of Line.raw is appropriate,
@@ -237,8 +245,10 @@ func (fix *Autofix) Custom(fixer func(sh
// Describef can be called from within an Autofix.Custom call to remember a
// description of the actual fix for logging it later when Apply is called.
// Describef may be called multiple times before calling Apply.
-func (fix *Autofix) Describef(lineno int, format string, args ...interface{}) {
- fix.actions = append(fix.actions, autofixAction{sprintf(format, args...), lineno})
+func (fix *Autofix) Describef(rawIndex int, format string, args ...interface{}) {
+ msg := sprintf(format, args...)
+ lineno := fix.line.Location.Lineno(rawIndex)
+ fix.actions = append(fix.actions, autofixAction{msg, lineno})
}
// Apply does the actual work that has been prepared by previous calls to
@@ -297,10 +307,10 @@ func (fix *Autofix) Apply() {
if logDiagnostic {
linenos := fix.affectedLinenos()
msg := sprintf(fix.diagFormat, fix.diagArgs...)
- if !logFix && G.Logger.FirstTime(line.Filename, linenos, msg) {
+ if !logFix && G.Logger.FirstTime(line.Filename(), linenos, msg) {
G.Logger.writeSource(line)
}
- G.Logger.Logf(fix.level, line.Filename, linenos, fix.diagFormat, msg)
+ G.Logger.Logf(fix.level, line.Filename(), linenos, fix.diagFormat, msg)
}
if logFix {
@@ -309,7 +319,7 @@ func (fix *Autofix) Apply() {
if action.lineno != 0 {
lineno = strconv.Itoa(action.lineno)
}
- G.Logger.Logf(AutofixLogLevel, line.Filename, lineno, autofixFormat, action.description)
+ G.Logger.Logf(AutofixLogLevel, line.Filename(), lineno, autofixFormat, action.description)
}
G.Logger.writeSource(line)
}
@@ -375,7 +385,7 @@ func (fix *Autofix) skip() bool {
func (fix *Autofix) assertRealLine() {
// Some Line objects do not correspond to real lines of a file.
// These cannot be fixed since they are neither part of Lines nor of MkLines.
- assert(fix.line.firstLine >= 1)
+ assert(fix.line.Location.lineno >= 1)
}
// SaveAutofixChanges writes the given lines back into their files,
@@ -392,11 +402,11 @@ func SaveAutofixChanges(lines *Lines) (a
// Fast lane for the case that nothing is written back to disk.
if !G.Logger.Opts.Autofix {
for _, line := range lines.Lines {
- if line.autofix != nil && line.autofix.modified {
+ if line.fix != nil && line.fix.modified {
G.Logger.autofixAvailable = true
if G.Logger.Opts.ShowAutofix {
// Only in this case can the loaded lines be modified.
- G.fileCache.Evict(line.Filename)
+ G.fileCache.Evict(line.Filename())
}
}
}
@@ -420,22 +430,21 @@ func SaveAutofixChanges(lines *Lines) (a
changes := make(map[CurrPath][]string)
changed := make(map[CurrPath]bool)
for _, line := range lines.Lines {
- chlines := changes[line.Filename]
- if fix := line.autofix; fix != nil {
+ filename := line.Filename()
+ chlines := changes[filename]
+ if fix := line.fix; fix != nil {
if fix.modified {
- changed[line.Filename] = true
- }
- chlines = append(chlines, fix.linesBefore...)
- for _, raw := range line.raw {
- chlines = append(chlines, raw.textnl)
+ changed[filename] = true
}
- chlines = append(chlines, fix.linesAfter...)
+ chlines = append(chlines, fix.above...)
+ chlines = append(chlines, fix.texts...)
+ chlines = append(chlines, fix.below...)
} else {
for _, raw := range line.raw {
- chlines = append(chlines, raw.textnl)
+ chlines = append(chlines, raw.orignl)
}
}
- changes[line.Filename] = chlines
+ changes[filename] = chlines
}
for filename := range changed {
Index: pkgsrc/pkgtools/pkglint/files/autofix_test.go
diff -u pkgsrc/pkgtools/pkglint/files/autofix_test.go:1.39 pkgsrc/pkgtools/pkglint/files/autofix_test.go:1.40
--- pkgsrc/pkgtools/pkglint/files/autofix_test.go:1.39 Thu Jan 23 21:56:50 2020
+++ pkgsrc/pkgtools/pkglint/files/autofix_test.go Sat Feb 15 13:48:40 2020
@@ -19,8 +19,8 @@ func (s *Suite) Test_Autofix__default_al
fix := line.Autofix()
fix.Warnf("Row should be replaced with line.")
fix.Replace("row", "line")
- fix.InsertBefore("above")
- fix.InsertAfter("below")
+ fix.InsertAbove("above")
+ fix.InsertBelow("below")
fix.Delete()
fix.Apply()
@@ -46,8 +46,8 @@ func (s *Suite) Test_Autofix__show_autof
fix := line.Autofix()
fix.Warnf("Row should be replaced with line.")
fix.ReplaceAfter("", "# row", "# line")
- fix.InsertBefore("above")
- fix.InsertAfter("below")
+ fix.InsertAbove("above")
+ fix.InsertBelow("below")
fix.Delete()
fix.Apply()
@@ -57,8 +57,8 @@ func (s *Suite) Test_Autofix__show_autof
t.CheckOutputLines(
"WARN: ~/Makefile:1--2: Row should be replaced with line.",
"AUTOFIX: ~/Makefile:1: Replacing \"# row\" with \"# line\".",
- "AUTOFIX: ~/Makefile:1: Inserting a line \"above\" before this line.",
- "AUTOFIX: ~/Makefile:2: Inserting a line \"below\" after this line.",
+ "AUTOFIX: ~/Makefile:1: Inserting a line \"above\" above this line.",
+ "AUTOFIX: ~/Makefile:2: Inserting a line \"below\" below this line.",
"AUTOFIX: ~/Makefile:1: Deleting this line.",
"AUTOFIX: ~/Makefile:2: Deleting this line.",
"+\tabove",
@@ -71,12 +71,20 @@ func (s *Suite) Test_Autofix__show_autof
func (s *Suite) Test_Autofix__multiple_fixes(c *check.C) {
t := s.Init(c)
+ newRawLines := func(texts ...string) []*RawLine {
+ var rawLines []*RawLine
+ for _, text := range texts {
+ rawLines = append(rawLines, &RawLine{text})
+ }
+ return rawLines
+ }
+
t.SetUpCommandLine("--show-autofix", "--explain")
line := t.NewLine("filename", 1, "original")
- c.Check(line.autofix, check.IsNil)
- t.CheckDeepEquals(line.raw, t.NewRawLines(1, "original\n"))
+ c.Check(line.fix, check.IsNil)
+ t.CheckDeepEquals(line.raw, newRawLines("original\n"))
{
fix := line.Autofix()
@@ -85,8 +93,9 @@ func (s *Suite) Test_Autofix__multiple_f
fix.Apply()
}
- c.Check(line.autofix, check.NotNil)
- t.CheckDeepEquals(line.raw, t.NewRawLines(1, "original\n", "lriginao\n"))
+ c.Check(line.fix, check.NotNil)
+ t.CheckDeepEquals(line.raw, newRawLines("original\n"))
+ t.CheckDeepEquals(line.fix.texts, []string{"lriginao\n"})
t.CheckOutputLines(
"AUTOFIX: filename:1: Replacing \"original\" with \"lriginao\".")
@@ -97,9 +106,10 @@ func (s *Suite) Test_Autofix__multiple_f
fix.Apply()
}
- c.Check(line.autofix, check.NotNil)
- t.CheckDeepEquals(line.raw, t.NewRawLines(1, "original\n", "lruginao\n"))
- t.CheckEquals(line.raw[0].textnl, "lruginao\n")
+ c.Check(line.fix, check.NotNil)
+ t.CheckDeepEquals(line.raw, newRawLines("original\n"))
+ t.CheckDeepEquals(line.fix.texts, []string{"lruginao\n"})
+ t.CheckEquals(line.RawText(0), "lruginao")
t.CheckOutputLines(
"AUTOFIX: filename:1: Replacing \"ig\" with \"ug\".")
@@ -110,13 +120,13 @@ func (s *Suite) Test_Autofix__multiple_f
fix.Apply()
}
- c.Check(line.autofix, check.NotNil)
- t.CheckDeepEquals(line.raw, t.NewRawLines(1, "original\n", "middle\n"))
- t.CheckEquals(line.raw[0].textnl, "middle\n")
+ c.Check(line.fix, check.NotNil)
+ t.CheckDeepEquals(line.raw, newRawLines("original\n"))
+ t.CheckDeepEquals(line.fix.texts, []string{"middle\n"})
t.CheckOutputLines(
"AUTOFIX: filename:1: Replacing \"lruginao\" with \"middle\".")
- t.CheckEquals(line.raw[0].textnl, "middle\n")
+ t.CheckDeepEquals(line.fix.texts, []string{"middle\n"})
t.CheckOutputEmpty()
{
@@ -485,14 +495,14 @@ func (s *Suite) Test_Autofix_Explain__si
fix := line.Autofix()
fix.Warnf(SilentAutofixFormat)
- fix.InsertBefore("before")
+ fix.InsertAbove("above")
fix.Apply()
fix.Notef("This diagnostic is necessary for the following explanation.")
fix.Explain(
"When inserting multiple lines, Apply must be called in-between.",
"Otherwise the changes are not described to the human reader.")
- fix.InsertAfter("after")
+ fix.InsertBelow("below")
fix.Apply()
t.CheckOutputLines(
@@ -501,7 +511,7 @@ func (s *Suite) Test_Autofix_Explain__si
"\tWhen inserting multiple lines, Apply must be called in-between.",
"\tOtherwise the changes are not described to the human reader.",
"")
- t.CheckEquals(fix.RawText(), "before\nText\nafter\n")
+ t.CheckEquals(fix.RawText(), "above\nText\nbelow\n")
}
func (s *Suite) Test_Autofix_Replace(c *check.C) {
@@ -589,8 +599,8 @@ func (s *Suite) Test_Autofix_ReplaceAfte
fix := line.Autofix()
fix.Notef("Inserting a line.")
- fix.InsertBefore("line before")
- fix.InsertAfter("line after")
+ fix.InsertAbove("line above")
+ fix.InsertBelow("line below")
fix.Apply()
fix.Notef("Replacing text.")
@@ -600,8 +610,8 @@ func (s *Suite) Test_Autofix_ReplaceAfte
t.CheckOutputLines(
"NOTE: filename:5: Inserting a line.",
- "AUTOFIX: filename:5: Inserting a line \"line before\" before this line.",
- "AUTOFIX: filename:5: Inserting a line \"line after\" after this line.",
+ "AUTOFIX: filename:5: Inserting a line \"line above\" above this line.",
+ "AUTOFIX: filename:5: Inserting a line \"line below\" below this line.",
"NOTE: filename:5: Replacing text.",
"AUTOFIX: filename:5: Replacing \"l\" with \"L\".",
"AUTOFIX: filename:5: Replacing \"i\" with \"I\".")
@@ -754,10 +764,8 @@ func (s *Suite) Test_Autofix_ReplaceAt__
"# comment")
mklines.ForEach(func(mkline *MkLine) {
- // FIXME
- // The modifications from this replacement are not supposed
- // to be saved to the file.
- // They should only be applied to the in-memory copy.
+ // The modifications from this replacement are not saved to the file.
+ // They are only applied to the in-memory copy.
fix := mkline.Autofix()
fix.Warnf("Warning.")
fix.ReplaceAt(0, 0, "# ", "COMMENT=\t")
@@ -775,12 +783,10 @@ func (s *Suite) Test_Autofix_ReplaceAt__
t.CheckOutputLines(
"AUTOFIX: ~/filename.mk:1: Replacing \"comment\" with \"remark\".")
t.CheckFileLines("filename.mk",
- // FIXME: The "COMMENT=" must not appear here since it
- // was supposed to be ignored by the --only option.
- "COMMENT=\tremark")
+ "# remark")
}
-func (s *Suite) Test_Autofix_InsertBefore(c *check.C) {
+func (s *Suite) Test_Autofix_InsertAbove(c *check.C) {
t := s.Init(c)
t.SetUpCommandLine("--show-autofix", "--source")
@@ -788,17 +794,17 @@ func (s *Suite) Test_Autofix_InsertBefor
fix := line.Autofix()
fix.Warnf("Dummy.")
- fix.InsertBefore("inserted")
+ fix.InsertAbove("inserted")
fix.Apply()
t.CheckOutputLines(
"WARN: Makefile:30: Dummy.",
- "AUTOFIX: Makefile:30: Inserting a line \"inserted\" before this line.",
+ "AUTOFIX: Makefile:30: Inserting a line \"inserted\" above this line.",
"+\tinserted",
">\toriginal")
}
-func (s *Suite) Test_Autofix_InsertAfter(c *check.C) {
+func (s *Suite) Test_Autofix_InsertBelow(c *check.C) {
t := s.Init(c)
doTest := func(bool) {
@@ -806,14 +812,14 @@ func (s *Suite) Test_Autofix_InsertAfter
fix := line.Autofix()
fix.Errorf("Error.")
- fix.InsertAfter("after")
+ fix.InsertBelow("below")
fix.Apply()
}
t.ExpectDiagnosticsAutofix(
doTest,
"ERROR: DESCR:1: Error.",
- "AUTOFIX: DESCR:1: Inserting a line \"after\" after this line.")
+ "AUTOFIX: DESCR:1: Inserting a line \"below\" below this line.")
}
func (s *Suite) Test_Autofix_Delete(c *check.C) {
@@ -866,15 +872,15 @@ func (s *Suite) Test_Autofix_Delete__com
fix := line.Autofix()
fix.Warnf("This line should be replaced completely.")
fix.Delete()
- fix.InsertAfter("below")
- fix.InsertBefore("above")
+ fix.InsertBelow("below")
+ fix.InsertAbove("above")
fix.Apply()
t.CheckOutputLines(
"WARN: Makefile:30: This line should be replaced completely.",
"AUTOFIX: Makefile:30: Deleting this line.",
- "AUTOFIX: Makefile:30: Inserting a line \"below\" after this line.",
- "AUTOFIX: Makefile:30: Inserting a line \"above\" before this line.",
+ "AUTOFIX: Makefile:30: Inserting a line \"below\" below this line.",
+ "AUTOFIX: Makefile:30: Inserting a line \"above\" above this line.",
"+\tabove",
"-\tto be deleted",
"+\tbelow")
@@ -898,7 +904,7 @@ func (s *Suite) Test_Autofix_Custom__in_
fix := line.Autofix()
fix.Warnf("Please write in ALL-UPPERCASE.")
fix.Custom(func(showAutofix, autofix bool) {
- fix.Describef(int(line.firstLine), "Converting to uppercase")
+ fix.Describef(0, "Converting to uppercase")
if showAutofix || autofix {
line.Text = strings.ToUpper(line.Text)
}
@@ -933,17 +939,16 @@ func (s *Suite) Test_Autofix_Describef(c
t := s.Init(c)
doTest := func(bool) {
- line := t.NewLine("DESCR", 1, "Description of the package")
+ line := t.NewLine("DESCR", 123, "Description of the package")
fix := line.Autofix()
fix.Errorf("Error.")
fix.Custom(func(showAutofix, autofix bool) {
- fix.Describef(123, "Masking.")
- raw := line.raw[0]
- raw.textnl = replaceAll(raw.Text(), `\p{L}`, "*") + "\n"
+ fix.Describef(0, "Masking.")
+ fix.texts[0] = replaceAll(fix.texts[0], `\p{L}`, "*")
})
fix.Apply()
- t.CheckEquals(line.raw[0].Text(), "*********** ** *** *******")
+ t.CheckEquals(line.RawText(0), "*********** ** *** *******")
}
t.ExpectDiagnosticsAutofix(
@@ -1196,7 +1201,7 @@ func (s *Suite) Test_Autofix_Apply__text
t.CheckOutputLines(
"AUTOFIX: filename.mk:123: Replacing \"value\" with \"new value\".")
- t.CheckEquals(mkline.raw[0].textnl, "VAR=\tnew value\n")
+ t.CheckEquals(mkline.fix.texts[0], "VAR=\tnew value\n")
t.CheckEquals(mkline.raw[0].orignl, "VAR=\tvalue\n")
t.CheckEquals(mkline.Text, "VAR=\tnew value")
// TODO: should be updated as well.
@@ -1295,8 +1300,8 @@ func (s *Suite) Test_Autofix_skip(c *che
fix.Replace("111", "___")
fix.ReplaceAfter(" ", "222", "___")
fix.ReplaceAt(0, 0, "VAR", "NEW")
- fix.InsertBefore("before")
- fix.InsertAfter("after")
+ fix.InsertAbove("above")
+ fix.InsertBelow("below")
fix.Delete()
fix.Custom(func(showAutofix, autofix bool) {})
@@ -1309,7 +1314,7 @@ func (s *Suite) Test_Autofix_skip(c *che
"VAR=\t111 222 333 444 555 \\",
"666")
t.CheckEquals(fix.RawText(), ""+
- "NEW=\t111 222 333 444 555 \\\n"+
+ "VAR=\t111 222 333 444 555 \\\n"+
"666\n")
}
@@ -1450,14 +1455,14 @@ func (s *Suite) Test_SaveAutofixChanges_
// or --autofix options are enabled.
func (fix *Autofix) RawText() string {
var text strings.Builder
- for _, lineBefore := range fix.linesBefore {
- text.WriteString(lineBefore)
+ for _, above := range fix.above {
+ text.WriteString(above)
}
- for _, raw := range fix.line.raw {
- text.WriteString(raw.textnl)
+ for _, modified := range fix.texts {
+ text.WriteString(modified)
}
- for _, lineAfter := range fix.linesAfter {
- text.WriteString(lineAfter)
+ for _, below := range fix.below {
+ text.WriteString(below)
}
return text.String()
}
Index: pkgsrc/pkgtools/pkglint/files/buildlink3.go
diff -u pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.31 pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.32
--- pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.31 Sat Jan 18 21:56:09 2020
+++ pkgsrc/pkgtools/pkglint/files/buildlink3.go Sat Feb 15 13:48:40 2020
@@ -100,7 +100,7 @@ func (ck *Buildlink3Checker) checkUnique
return
}
- dirname := G.Pkgsrc.Rel(mkline.Filename.DirNoClean()).Base()
+ dirname := G.Pkgsrc.Rel(mkline.Filename().Dir()).Base()
base, name := trimCommon(pkgbase, dirname)
if base == "" && matches(name, `^(\d*|-cvs|-fossil|-git|-hg|-svn|-devel|-snapshot)$`) {
return
Index: pkgsrc/pkgtools/pkglint/files/category.go
diff -u pkgsrc/pkgtools/pkglint/files/category.go:1.31 pkgsrc/pkgtools/pkglint/files/category.go:1.32
--- pkgsrc/pkgtools/pkglint/files/category.go:1.31 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/category.go Sat Feb 15 13:48:40 2020
@@ -115,7 +115,7 @@ func CheckdirCategory(dir CurrPath) {
fix := line.Autofix()
fix.Errorf("%q exists in the file system but not in the Makefile.", fCurrent)
- fix.InsertBefore("SUBDIR+=\t" + fCurrent.String())
+ fix.InsertAbove("SUBDIR+=\t" + fCurrent.String())
fix.Apply()
}
fRest = fRest[1:]
@@ -147,7 +147,7 @@ func CheckdirCategory(dir CurrPath) {
mklines.SaveAutofixChanges()
- if G.Opts.Recursive {
+ if G.Recursive {
var recurseInto []CurrPath
for _, msub := range mSubdirs {
if !msub.line.IsCommentedVarassign() {
Index: pkgsrc/pkgtools/pkglint/files/category_test.go
diff -u pkgsrc/pkgtools/pkglint/files/category_test.go:1.31 pkgsrc/pkgtools/pkglint/files/category_test.go:1.32
--- pkgsrc/pkgtools/pkglint/files/category_test.go:1.31 Sat Jan 11 15:47:58 2020
+++ pkgsrc/pkgtools/pkglint/files/category_test.go Sat Feb 15 13:48:40 2020
@@ -24,14 +24,14 @@ func (s *Suite) Test_CheckdirCategory__t
"NOTE: ~/archivers/Makefile:3: This variable value should be aligned with tabs, not spaces, to column 17 instead of 10.",
"NOTE: ~/archivers/Makefile:4: This variable value should be aligned to column 17 instead of 9.",
"ERROR: ~/archivers/Makefile:6: Relative path \"../mk/category.mk\" does not exist.",
- "NOTE: ~/archivers/Makefile:2: Empty line expected before this line.",
+ "NOTE: ~/archivers/Makefile:2: Empty line expected above this line.",
"ERROR: ~/archivers/Makefile:2: COMMENT= line expected.",
- "NOTE: ~/archivers/Makefile:2: Empty line expected before this line.",
+ "NOTE: ~/archivers/Makefile:2: Empty line expected above this line.",
"WARN: ~/archivers/Makefile:3: \"aaaaa\" should come before \"pkg1\".",
"ERROR: ~/archivers/Makefile:4: SUBDIR+= line or empty line expected.",
"ERROR: ~/archivers/Makefile:2: \"pkg1\" exists in the Makefile but not in the file system.",
"ERROR: ~/archivers/Makefile:3: \"aaaaa\" exists in the Makefile but not in the file system.",
- "NOTE: ~/archivers/Makefile:4: Empty line expected before this line.",
+ "NOTE: ~/archivers/Makefile:4: Empty line expected above this line.",
"WARN: ~/archivers/Makefile:4: This line should contain the following text: .include \"../mk/misc/category.mk\"",
"ERROR: ~/archivers/Makefile:4: The file must end here.")
}
@@ -246,7 +246,7 @@ func (s *Suite) Test_CheckdirCategory__s
t.CheckOutputLines(
"ERROR: ~/category/Makefile:6: \"zzz-fs-only\" exists in the file system but not in the Makefile.",
- "AUTOFIX: ~/category/Makefile:6: Inserting a line \"SUBDIR+=\\tzzz-fs-only\" before this line.")
+ "AUTOFIX: ~/category/Makefile:6: Inserting a line \"SUBDIR+=\\tzzz-fs-only\" above this line.")
}
func (s *Suite) Test_CheckdirCategory__indentation(c *check.C) {
@@ -301,10 +301,10 @@ func (s *Suite) Test_CheckdirCategory__c
// is rather exotic anyway.
t.CheckOutputLines(
"ERROR: ~/category/Makefile:3: COMMENT= line expected.",
- "NOTE: ~/category/Makefile:3: Empty line expected before this line.",
+ "NOTE: ~/category/Makefile:3: Empty line expected above this line.",
"ERROR: ~/category/Makefile:3: SUBDIR+= line or empty line expected.",
"ERROR: ~/category/Makefile:3: \"package\" exists in the file system but not in the Makefile.",
- "NOTE: ~/category/Makefile:3: Empty line expected before this line.",
+ "NOTE: ~/category/Makefile:3: Empty line expected above this line.",
"WARN: ~/category/Makefile:3: This line should contain the following text: .include \"../mk/misc/category.mk\"",
"ERROR: ~/category/Makefile:3: The file must end here.")
}
@@ -327,7 +327,7 @@ func (s *Suite) Test_CheckdirCategory__u
// Doesn't happen in practice since categories are created very seldom.
t.CheckOutputLines(
- "NOTE: ~/category/Makefile:5: Empty line expected after this line.",
+ "NOTE: ~/category/Makefile:5: Empty line expected below this line.",
"WARN: ~/category/Makefile:EOF: This line should contain the following text: "+
".include \"../mk/misc/category.mk\"")
}
Index: pkgsrc/pkgtools/pkglint/files/files.go
diff -u pkgsrc/pkgtools/pkglint/files/files.go:1.31 pkgsrc/pkgtools/pkglint/files/files.go:1.32
--- pkgsrc/pkgtools/pkglint/files/files.go:1.31 Mon Jan 6 20:38:42 2020
+++ pkgsrc/pkgtools/pkglint/files/files.go Sat Feb 15 13:48:40 2020
@@ -31,7 +31,7 @@ func Load(filename CurrPath, options Loa
if err != nil {
switch {
case options&MustSucceed != 0:
- NewLineWhole(filename).Fatalf("Cannot be read.")
+ G.Logger.TechFatalf(filename, "Cannot be read.")
case options&LogErrors != 0:
NewLineWhole(filename).Errorf("Cannot be read.")
}
@@ -41,14 +41,14 @@ func Load(filename CurrPath, options Loa
if rawText == "" && options&NotEmpty != 0 {
switch {
case options&MustSucceed != 0:
- NewLineWhole(filename).Fatalf("Must not be empty.")
+ G.Logger.TechFatalf(filename, "Must not be empty.")
case options&LogErrors != 0:
NewLineWhole(filename).Errorf("Must not be empty.")
}
return nil
}
- if G.Opts.Profiling {
+ if G.Profiling {
G.loaded.Add(filename.Clean().String(), 1)
}
@@ -61,9 +61,9 @@ func Load(filename CurrPath, options Loa
func convertToLogicalLines(filename CurrPath, rawText string, joinBackslashLines bool) *Lines {
var rawLines []*RawLine
- for lineno, rawLine := range strings.SplitAfter(rawText, "\n") {
+ for _, rawLine := range strings.SplitAfter(rawText, "\n") {
if rawLine != "" {
- rawLines = append(rawLines, &RawLine{1 + lineno, rawLine, rawLine})
+ rawLines = append(rawLines, &RawLine{rawLine})
}
}
@@ -75,9 +75,9 @@ func convertToLogicalLines(filename Curr
lineno = nextLineno
}
} else {
- for _, rawLine := range rawLines {
- text := rawLine.Text()
- logline := NewLine(filename, rawLine.Lineno, text, rawLine)
+ for rawIndex, rawLine := range rawLines {
+ text := rawLine.Orig()
+ logline := NewLine(filename, rawIndex+1, text, rawLine)
loglines = append(loglines, logline)
}
}
@@ -92,20 +92,20 @@ func convertToLogicalLines(filename Curr
func nextLogicalLine(filename CurrPath, rawLines []*RawLine, index int) (*Line, int) {
{ // Handle the common case efficiently
rawLine := rawLines[index]
- text := rawLine.Text()
+ text := rawLine.Orig()
if !hasSuffix(text, "\\") {
- return NewLine(filename, rawLine.Lineno, text, rawLines[index]), index + 1
+ return NewLine(filename, index+1, text, rawLines[index]), index + 1
}
}
var text strings.Builder
- firstlineno := rawLines[index].Lineno
+ firstLineno := index + 1
var lineRawLines []*RawLine
interestingRawLines := rawLines[index:]
trim := ""
for i, rawLine := range interestingRawLines {
- indent, rawText, outdent, cont := matchContinuationLine(rawLine.Text())
+ indent, rawText, outdent, cont := matchContinuationLine(rawLine.Orig())
if text.Len() == 0 {
text.WriteString(indent)
@@ -125,9 +125,7 @@ func nextLogicalLine(filename CurrPath,
}
}
- lastlineno := rawLines[index].Lineno
-
- return NewLineMulti(filename, firstlineno, lastlineno, text.String(), lineRawLines), index + 1
+ return NewLineMulti(filename, firstLineno, text.String(), lineRawLines), index + 1
}
func matchContinuationLine(text string) (leadingWhitespace, result, trailingWhitespace, cont string) {
Index: pkgsrc/pkgtools/pkglint/files/buildlink3_test.go
diff -u pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.40 pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.41
--- pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.40 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/buildlink3_test.go Sat Feb 15 13:48:40 2020
@@ -443,7 +443,7 @@ func (s *Suite) Test_CheckLinesBuildlink
t.CheckOutputLines(
"ERROR: buildlink3.mk:EOF: .if from line 5 must be closed.",
- "NOTE: buildlink3.mk:6: Empty line expected after this line.")
+ "NOTE: buildlink3.mk:6: Empty line expected below this line.")
}
func (s *Suite) Test_CheckLinesBuildlink3Mk__invalid_dependency_patterns(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/logging.go
diff -u pkgsrc/pkgtools/pkglint/files/logging.go:1.40 pkgsrc/pkgtools/pkglint/files/logging.go:1.41
--- pkgsrc/pkgtools/pkglint/files/logging.go:1.40 Wed Feb 5 04:09:00 2020
+++ pkgsrc/pkgtools/pkglint/files/logging.go Sat Feb 15 13:48:40 2020
@@ -58,10 +58,6 @@ type LogLevel struct {
}
var (
- // FIXME: By definition there cannot be fatal diagnostics.
- // Having these was a misconception from the beginning,
- // and they must be re-classified as fatal technical errors.
- Fatal = &LogLevel{"FATAL", "fatal"}
Error = &LogLevel{"ERROR", "error"}
Warn = &LogLevel{"WARN", "warning"}
Note = &LogLevel{"NOTE", "note"}
@@ -124,7 +120,7 @@ func (l *Logger) Diag(line *Line, level
}
}
- if l.IsAutofix() && level != Fatal {
+ if l.IsAutofix() {
// In these two cases, the only interesting diagnostics are those that can
// be fixed automatically. These are logged by Autofix.Apply.
l.suppressExpl = true
@@ -135,7 +131,7 @@ func (l *Logger) Diag(line *Line, level
return
}
- filename := line.Filename
+ filename := line.Filename()
linenos := line.Linenos()
msg := sprintf(format, args...)
if !l.FirstTime(filename, linenos, msg) {
@@ -209,13 +205,13 @@ func (l *Logger) writeSource(line *Line)
if !l.IsAutofix() {
l.out.Separate()
}
- if l.IsAutofix() && line.autofix != nil {
- for _, before := range line.autofix.linesBefore {
- l.writeLine("+\t", before)
+ if l.IsAutofix() {
+ for _, above := range line.fix.above {
+ l.writeLine("+\t", above)
}
l.writeDiff(line)
- for _, after := range line.autofix.linesAfter {
- l.writeLine("+\t", after)
+ for _, below := range line.fix.below {
+ l.writeLine("+\t", below)
}
} else {
l.writeDiff(line)
@@ -226,24 +222,25 @@ func (l *Logger) writeSource(line *Line)
}
func (l *Logger) writeDiff(line *Line) {
- showAsChanged := func(rawLine *RawLine) bool {
- return l.IsAutofix() && rawLine.textnl != rawLine.orignl
+ showAsChanged := func(rawIndex int, rawLine *RawLine) bool {
+ return l.IsAutofix() &&
+ line.fix.texts[rawIndex] != rawLine.orignl
}
rawLines := line.raw
prefix := ">\t"
- for _, rawLine := range rawLines {
- if showAsChanged(rawLine) {
+ for rawIndex, rawLine := range rawLines {
+ if showAsChanged(rawIndex, rawLine) {
prefix = "\t" // Make it look like an actual diff
}
}
- for _, rawLine := range rawLines {
- if showAsChanged(rawLine) {
+ for rawIndex, rawLine := range rawLines {
+ if showAsChanged(rawIndex, rawLine) {
l.writeLine("-\t", rawLine.orignl)
- if rawLine.textnl != "" {
- l.writeLine("+\t", rawLine.textnl)
+ if line.fix.texts[rawIndex] != "" {
+ l.writeLine("+\t", line.fix.texts[rawIndex])
}
} else {
l.writeLine(prefix, rawLine.orignl)
@@ -298,15 +295,10 @@ func (l *Logger) Logf(level *LogLevel, f
if !filename.IsEmpty() {
filename = filename.CleanPath()
}
- if G.Opts.Profiling && format != autofixFormat && level != Fatal {
+ if G.Profiling && format != autofixFormat {
l.histo.Add(format, 1)
}
- out := l.out
- if level == Fatal {
- out = l.err
- }
-
filenameSep := condStr(!filename.IsEmpty(), ": ", "")
effLineno := condStr(!filename.IsEmpty(), lineno, "")
linenoSep := condStr(effLineno != "", ":", "")
@@ -316,11 +308,9 @@ func (l *Logger) Logf(level *LogLevel, f
} else {
diag = sprintf("%s%s%s%s%s: %s\n", level.TraditionalName, filenameSep, filename, linenoSep, effLineno, msg)
}
- out.Write(escapePrintable(diag))
+ l.out.Write(escapePrintable(diag))
switch level {
- case Fatal:
- panic(pkglintFatal{})
case Error:
l.errors++
case Warn:
@@ -330,24 +320,31 @@ func (l *Logger) Logf(level *LogLevel, f
}
}
-// TechErrorf logs a technical error on the error output.
+// TechFatalf logs a technical error on the error output and quits pkglint.
//
// For diagnostics, use Logf instead.
-func (l *Logger) TechErrorf(location CurrPath, format string, args ...interface{}) {
+func (l *Logger) TechFatalf(location CurrPath, format string, args ...interface{}) {
+ loc := location.String() + condStr(location.IsEmpty(), "", ": ")
msg := sprintf(format, args...)
+ all := sprintf("FATAL: %s%s\n", loc, msg)
+ esc := escapePrintable(all)
+ l.err.Write(esc)
- locationStr := ""
- if !location.IsEmpty() {
- locationStr = location.String() + ": "
+ if trace.Tracing {
+ trace.Stepf("TechFatalf: %s%s", loc, msg)
}
+ panic(pkglintFatal{})
+}
- var diag string
- if l.Opts.GccOutput {
- diag = sprintf("%s%s: %s\n", locationStr, Error.GccName, msg)
- } else {
- diag = sprintf("%s: %s%s\n", Error.TraditionalName, locationStr, msg)
- }
- l.err.Write(escapePrintable(diag))
+// TechErrorf logs a technical error on the error output.
+//
+// For diagnostics, use Logf instead.
+func (l *Logger) TechErrorf(location CurrPath, format string, args ...interface{}) {
+ loc := location.String() + condStr(location.IsEmpty(), "", ": ")
+ msg := sprintf(format, args...)
+ all := sprintf("ERROR: %s%s\n", loc, msg)
+ esc := escapePrintable(all)
+ l.err.Write(esc)
}
func (l *Logger) ShowSummary(args []string) {
Index: pkgsrc/pkgtools/pkglint/files/check_test.go
diff -u pkgsrc/pkgtools/pkglint/files/check_test.go:1.65 pkgsrc/pkgtools/pkglint/files/check_test.go:1.66
--- pkgsrc/pkgtools/pkglint/files/check_test.go:1.65 Sat Jan 18 21:56:09 2020
+++ pkgsrc/pkgtools/pkglint/files/check_test.go Sat Feb 15 13:48:40 2020
@@ -456,7 +456,7 @@ func (t *Tester) SetUpPackage(pkgpath Re
"pkgpath %q must have the form \"category/package\"", pkgpath)
distname := pkgpath.Base()
- category := pkgpath.DirNoClean()
+ category := pkgpath.Dir()
if category == "wip" {
// To avoid boilerplate CATEGORIES definitions for wip packages.
category = "local"
@@ -551,7 +551,7 @@ func (t *Tester) CreateFileLines(filenam
}
abs := t.File(filename)
- err := os.MkdirAll(abs.DirNoClean().String(), 0777)
+ err := os.MkdirAll(abs.Dir().String(), 0777)
t.c.Assert(err, check.IsNil)
err = abs.WriteString(content.String())
@@ -584,7 +584,7 @@ func (t *Tester) CreateFileBuildlink3(fi
// Buildlink3.mk files only make sense in category/package directories.
assert(G.Pkgsrc.Rel(t.File(filename)).Count() == 3)
- dir := filename.DirClean()
+ dir := filename.Dir().Clean()
lower := dir.Base()
// see pkgtools/createbuildlink/files/createbuildlink, "package specific variables"
upper := strings.Replace(strings.ToUpper(lower), "-", "_", -1)
@@ -642,7 +642,7 @@ func (t *Tester) Copy(source, target Rel
data, err := absSource.ReadString()
assertNil(err, "Copy.Read")
- err = os.MkdirAll(absTarget.DirClean().String(), 0777)
+ err = os.MkdirAll(absTarget.Dir().Clean().String(), 0777)
assertNil(err, "Copy.MkdirAll")
err = absTarget.WriteString(data)
assertNil(err, "Copy.Write")
@@ -724,9 +724,9 @@ func (t *Tester) SetUpHierarchy() (
//
// This is the same mechanism that is used in Pkgsrc.Relpath.
includePath := func(including, included RelPath) RelPath {
- fromDir := including.DirClean()
+ fromDir := including.Dir().Clean()
to := basedir.Rel(included.AsPath())
- if fromDir == to.DirNoClean() {
+ if fromDir == to.Dir() {
return NewRelPathString(to.Base())
} else {
return fromDir.Rel(basedir).JoinNoClean(to).CleanDot()
@@ -979,37 +979,10 @@ func (t *Tester) ExpectDiagnosticsAutofi
t.CheckOutput(diagnostics)
}
-// NewRawLines creates lines from line numbers and raw text, including newlines.
-//
-// Arguments are sequences of either (lineno, orignl) or (lineno, orignl, textnl).
-//
-// Specifying textnl is only useful when simulating a line that has already been
-// modified by Autofix.
-func (t *Tester) NewRawLines(args ...interface{}) []*RawLine {
- rawlines := make([]*RawLine, len(args)/2)
- j := 0
- for i := 0; i < len(args); i += 2 {
- lineno := args[i].(int)
- orignl := args[i+1].(string)
- textnl := orignl
- if i+2 < len(args) {
- if s, ok := args[i+2].(string); ok {
- textnl = s
- i++
- }
- }
- rawlines[j] = &RawLine{lineno, orignl, textnl}
- j++
- }
- return rawlines[:j]
-}
-
// NewLine creates an in-memory line with the given text.
// This line does not correspond to any line in a file.
func (t *Tester) NewLine(filename CurrPath, lineno int, text string) *Line {
- textnl := text + "\n"
- rawLine := RawLine{lineno, textnl, textnl}
- return NewLine(filename, lineno, text, &rawLine)
+ return NewLine(filename, lineno, text, &RawLine{text + "\n"})
}
// NewMkLine creates an in-memory line in the Makefile format with the given text.
Index: pkgsrc/pkgtools/pkglint/files/distinfo.go
diff -u pkgsrc/pkgtools/pkglint/files/distinfo.go:1.41 pkgsrc/pkgtools/pkglint/files/distinfo.go:1.42
--- pkgsrc/pkgtools/pkglint/files/distinfo.go:1.41 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/distinfo.go Sat Feb 15 13:48:40 2020
@@ -281,12 +281,12 @@ func (ck *distinfoLinesChecker) checkAlg
if insertion == nil {
fix := line.Autofix()
fix.Errorf("Missing %s hash for %s.", alg, info.filename())
- fix.InsertBefore(sprintf("%s (%s) = %s", alg, info.filename(), computed))
+ fix.InsertAbove(sprintf("%s (%s) = %s", alg, info.filename(), computed))
fix.Apply()
} else {
fix := insertion.Autofix()
fix.Errorf("Missing %s hash for %s.", alg, info.filename())
- fix.InsertAfter(sprintf("%s (%s) = %s", alg, info.filename(), computed))
+ fix.InsertBelow(sprintf("%s (%s) = %s", alg, info.filename(), computed))
fix.Apply()
}
Index: pkgsrc/pkgtools/pkglint/files/distinfo_test.go
diff -u pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.36 pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.37
--- pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.36 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/distinfo_test.go Sat Feb 15 13:48:40 2020
@@ -27,7 +27,7 @@ func (s *Suite) Test_CheckLinesDistinfo_
t.CheckOutputLines(
"ERROR: distinfo:1: Expected \"$"+"NetBSD$\".",
- "NOTE: distinfo:1: Empty line expected before this line.",
+ "NOTE: distinfo:1: Empty line expected above this line.",
"ERROR: distinfo:1: Invalid line: should be the CVS ID",
"ERROR: distinfo:2: Invalid line: should be empty",
"ERROR: distinfo:8: Invalid line: Another invalid line",
@@ -58,7 +58,7 @@ func (s *Suite) Test_distinfoLinesChecke
CheckLinesDistinfo(nil, lines)
t.CheckOutputLines(
- "NOTE: ~/distinfo:1: Empty line expected after this line.")
+ "NOTE: ~/distinfo:1: Empty line expected below this line.")
}
func (s *Suite) Test_distinfoLinesChecker_parse__commented_first_line(c *check.C) {
@@ -73,7 +73,7 @@ func (s *Suite) Test_distinfoLinesChecke
t.CheckOutputLines(
"ERROR: ~/distinfo:1: Expected \""+CvsID+"\".",
- "NOTE: ~/distinfo:1: Empty line expected before this line.",
+ "NOTE: ~/distinfo:1: Empty line expected above this line.",
"ERROR: ~/distinfo:1: Invalid line: "+PlistCvsID)
}
@@ -86,7 +86,7 @@ func (s *Suite) Test_distinfoLinesChecke
CheckLinesDistinfo(nil, lines)
t.CheckOutputLines(
- "NOTE: ~/distinfo:EOF: Empty line expected before this line.")
+ "NOTE: ~/distinfo:EOF: Empty line expected above this line.")
}
// When the distinfo file and the patches are placed in the same package,
@@ -400,7 +400,7 @@ func (s *Suite) Test_distinfoLinesChecke
"AUTOFIX: ~/category/package/distinfo:3: "+
"Inserting a line \"SHA1 (package-1.0.txt) "+
"= cd50d19784897085a8d0e3e413f8612b097c03f1\" "+
- "before this line.",
+ "above this line.",
"+\tSHA1 (package-1.0.txt) = cd50d19784897085a8d0e3e413f8612b097c03f1",
">\tRMD160 (package-1.0.txt) = 1a88147a0344137404c63f3b695366eab869a98a",
"",
@@ -408,7 +408,7 @@ func (s *Suite) Test_distinfoLinesChecke
"AUTOFIX: ~/category/package/distinfo:3: "+
"Inserting a line \"SHA512 (package-1.0.txt) "+
"= f65f341b35981fda842b09b2c8af9bcdb7602a4c2e6fa1f7d41f0974d3e3122f"+
- "268fc79d5a4af66358f5133885cd1c165c916f80ab25e5d8d95db46f803c782c\" after this line.",
+ "268fc79d5a4af66358f5133885cd1c165c916f80ab25e5d8d95db46f803c782c\" below this line.",
"+\tSHA1 (package-1.0.txt) = cd50d19784897085a8d0e3e413f8612b097c03f1",
">\tRMD160 (package-1.0.txt) = 1a88147a0344137404c63f3b695366eab869a98a",
"+\tSHA512 (package-1.0.txt) = f65f341b35981fda842b09b2c8af9bcdb7602a4c2e6fa1f7d41f0974d3e3122f"+
@@ -572,9 +572,9 @@ func (s *Suite) Test_distinfoLinesChecke
"AUTOFIX: ~/category/package/distinfo:4: "+
"Inserting a line \"SHA512 (package-1.0.txt) = f65f341b35981fda842b"+
"09b2c8af9bcdb7602a4c2e6fa1f7d41f0974d3e3122f268fc79d5a4af66358f513"+
- "3885cd1c165c916f80ab25e5d8d95db46f803c782c\" after this line.",
+ "3885cd1c165c916f80ab25e5d8d95db46f803c782c\" below this line.",
"AUTOFIX: ~/category/package/distinfo:4: "+
- "Inserting a line \"Size (package-1.0.txt) = 13 bytes\" after this line.")
+ "Inserting a line \"Size (package-1.0.txt) = 13 bytes\" below this line.")
}
func (s *Suite) Test_distinfoLinesChecker_checkAlgorithmsDistfile__algorithms_in_wrong_order(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/patches.go
diff -u pkgsrc/pkgtools/pkglint/files/patches.go:1.36 pkgsrc/pkgtools/pkglint/files/patches.go:1.37
--- pkgsrc/pkgtools/pkglint/files/patches.go:1.36 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/patches.go Sat Feb 15 13:48:40 2020
@@ -168,7 +168,7 @@ func (ck *PatchChecker) checkUnifiedDiff
line.Explain(
"This line is not part of the patch anymore, although it may look so.",
"To make this situation clear, there should be an",
- "empty line before this line.",
+ "empty line above this line.",
"If the line doesn't contain useful information, it should be removed.")
}
}
@@ -200,7 +200,7 @@ func (ck *PatchChecker) checkBeginDiff(l
if !ck.previousLineEmpty {
fix := line.Autofix()
fix.Notef("Empty line expected.")
- fix.InsertBefore("")
+ fix.InsertAbove("")
fix.Apply()
}
}
Index: pkgsrc/pkgtools/pkglint/files/homepage.go
diff -u pkgsrc/pkgtools/pkglint/files/homepage.go:1.3 pkgsrc/pkgtools/pkglint/files/homepage.go:1.4
--- pkgsrc/pkgtools/pkglint/files/homepage.go:1.3 Wed Feb 5 04:09:00 2020
+++ pkgsrc/pkgtools/pkglint/files/homepage.go Sat Feb 15 13:48:40 2020
@@ -43,10 +43,11 @@ type HomepageChecker struct {
// Can be mocked for the tests.
isReachable func(url string) YesNoUnknown
+ Timeout time.Duration
}
func NewHomepageChecker(value string, valueNoVar string, mkline *MkLine, mklines *MkLines) *HomepageChecker {
- ck := HomepageChecker{value, valueNoVar, mkline, mklines, nil}
+ ck := HomepageChecker{value, valueNoVar, mkline, mklines, nil, 3 * time.Second}
ck.isReachable = ck.isReachableOnline
return &ck
}
@@ -110,7 +111,7 @@ func (ck *HomepageChecker) checkFtp() {
return
}
- mkline.Warnf("An FTP URL does not represent a user-friendly homepage.")
+ mkline.Warnf("An FTP URL is not a user-friendly homepage.")
mkline.Explain(
"This homepage URL has probably been generated by url2pkg",
"and not been reviewed by the package author.",
@@ -132,7 +133,7 @@ func (ck *HomepageChecker) checkHttp() {
fix := ck.MkLine.Autofix()
fix.Warnf("HOMEPAGE should migrate from %s to %s.", from, to)
fix.Replace(from, to)
- if from == "http" && to == "https" {
+ if from == "http" {
fix.Explain(
"To provide secure communication by default,",
"the HOMEPAGE URL should use the https protocol if available.",
@@ -252,25 +253,25 @@ func (ck *HomepageChecker) checkReachabl
mkline := ck.MkLine
url := ck.Value
- if !G.Opts.Network || url != ck.ValueNoVar {
+ if !G.Network || url != ck.ValueNoVar {
return
}
if !matches(url, `^https?://[A-Za-z0-9-.]+(?::[0-9]+)?/[!-~]*$`) {
return
}
- var client http.Client
- client.Timeout = 3 * time.Second
- client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
- return http.ErrUseLastResponse
- }
-
request, err := http.NewRequest("GET", url, nil)
if err != nil {
mkline.Errorf("Invalid URL %q.", url)
return
}
+ client := http.Client{
+ Timeout: ck.Timeout,
+ CheckRedirect: func(*http.Request, []*http.Request) error {
+ return http.ErrUseLastResponse
+ },
+ }
response, err := client.Do(request)
if err != nil {
networkError := ck.classifyNetworkError(err)
@@ -291,28 +292,30 @@ func (ck *HomepageChecker) checkReachabl
}
}
-func (*HomepageChecker) isReachableOnline(url string) YesNoUnknown {
+func (ck *HomepageChecker) isReachableOnline(url string) YesNoUnknown {
switch {
- case !G.Opts.Network,
+ case !G.Network,
containsVarRefLong(url),
!matches(url, `^https?://[A-Za-z0-9-.]+(?::[0-9]+)?/[!-~]*$`):
return unknown
}
- var client http.Client
- client.Timeout = 3 * time.Second
- client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
- return http.ErrUseLastResponse
- }
-
request, err := http.NewRequest("GET", url, nil)
if err != nil {
return no
}
+
+ client := http.Client{
+ Timeout: ck.Timeout,
+ CheckRedirect: func(req *http.Request, via []*http.Request) error {
+ return http.ErrUseLastResponse
+ },
+ }
response, err := client.Do(request)
if err != nil {
return no
}
+
_ = response.Body.Close()
if response.StatusCode != 200 {
return no
Index: pkgsrc/pkgtools/pkglint/files/mkassignchecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkassignchecker_test.go:1.3 pkgsrc/pkgtools/pkglint/files/mkassignchecker_test.go:1.4
--- pkgsrc/pkgtools/pkglint/files/mkassignchecker_test.go:1.3 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/mkassignchecker_test.go Sat Feb 15 13:48:40 2020
@@ -783,6 +783,25 @@ func (s *Suite) Test_MkAssignChecker_che
"WARN: Makefile:5: The primary category should be \"obscure\", not \"perl5\".")
}
+// Allow any primary category in "packages" from regress/*.
+// These packages won't be installed in a regular pkgsrc installation anyway.
+func (s *Suite) Test_MkAssignChecker_checkVarassignRightCategory__regress(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("regress/regress-package",
+ "CATEGORIES=\tregress")
+ t.SetUpPackage("regress/misc-package",
+ "CATEGORIES=\tmisc")
+ t.SetUpCategory("misc")
+ t.FinishSetUp()
+ t.Chdir(".")
+
+ G.Check("regress/regress-package")
+ G.Check("regress/misc-package")
+
+ t.CheckOutputEmpty()
+}
+
func (s *Suite) Test_MkAssignChecker_checkVarassignRightCategory__append(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/homepage_test.go
diff -u pkgsrc/pkgtools/pkglint/files/homepage_test.go:1.2 pkgsrc/pkgtools/pkglint/files/homepage_test.go:1.3
--- pkgsrc/pkgtools/pkglint/files/homepage_test.go:1.2 Wed Feb 5 04:09:00 2020
+++ pkgsrc/pkgtools/pkglint/files/homepage_test.go Sat Feb 15 13:48:40 2020
@@ -6,6 +6,7 @@ import (
"gopkg.in/check.v1"
"net"
"net/http"
+ "netbsd.org/pkglint/regex"
"strconv"
"syscall"
"time"
@@ -37,7 +38,7 @@ func (s *Suite) Test_HomepageChecker_Che
ck.Check()
t.CheckOutputLines(
- "WARN: filename.mk:1: An FTP URL does not represent a user-friendly homepage.")
+ "WARN: filename.mk:1: An FTP URL is not a user-friendly homepage.")
}
func (s *Suite) Test_HomepageChecker_checkBasedOnMasterSites(c *check.C) {
@@ -113,7 +114,7 @@ func (s *Suite) Test_HomepageChecker_che
vt.Output(
"WARN: filename.mk:1: " +
- "An FTP URL does not represent a user-friendly homepage.")
+ "An FTP URL is not a user-friendly homepage.")
}
func (s *Suite) Test_HomepageChecker_checkHttp(c *check.C) {
@@ -263,7 +264,7 @@ func (s *Suite) Test_HomepageChecker_mig
"")
// Since the URL contains a variable, it cannot be resolved.
- // Therefore it is skipped without any HTTP request being sent.
+ // Therefore it is skipped without sending any HTTP request.
test(
"http://godoc.org/${GO_SRCPATH}";,
false,
@@ -287,7 +288,6 @@ func (s *Suite) Test_HomepageChecker_che
func (s *Suite) Test_HomepageChecker_checkReachable(c *check.C) {
t := s.Init(c)
- vt := NewVartypeCheckTester(t, BtHomepage)
t.SetUpCommandLine("--network")
@@ -303,7 +303,7 @@ func (s *Suite) Test_HomepageChecker_che
writer.WriteHeader(status)
})
mux.HandleFunc("/timeout", func(http.ResponseWriter, *http.Request) {
- time.Sleep(5 * time.Second)
+ time.Sleep(2 * time.Second)
})
// 28780 = 256 * 'p' + 'l'
@@ -324,64 +324,87 @@ func (s *Suite) Test_HomepageChecker_che
<-shutdown
}()
- vt.Varname("HOMEPAGE")
- vt.Values(
+ test := func(url string, diagnostics ...string) {
+ mklines := t.NewMkLines("filename.mk",
+ "HOMEPAGE=\t"+url)
+ mkline := mklines.mklines[0]
+ ck := NewHomepageChecker(url, mkline.WithoutMakeVariables(url), mkline, mklines)
+ ck.Timeout = 1 * time.Second
+ ck.checkReachable()
+ t.CheckOutput(diagnostics)
+ }
+ testMatches := func(url string, diagnostics ...regex.Pattern) {
+ mklines := t.NewMkLines("filename.mk",
+ "HOMEPAGE=\t"+url)
+ ck := NewHomepageChecker(url, url, mklines.mklines[0], mklines)
+ ck.Timeout = 1 * time.Second
+ ck.checkReachable()
+ t.CheckOutputMatches(diagnostics...)
+ }
+
+ test(
"http://localhost:28780/status/200";,
+ nil...)
+
+ test(
"http://localhost:28780/status/301?location=/redirect301";,
+ "WARN: filename.mk:1: Homepage "+
+ "\"http://localhost:28780/status/301?location=/redirect301\"; "+
+ "redirects to \"http://localhost:28780/redirect301\".";)
+
+ test(
"http://localhost:28780/status/302?location=/redirect302";,
+ "WARN: filename.mk:1: Homepage "+
+ "\"http://localhost:28780/status/302?location=/redirect302\"; "+
+ "redirects to \"http://localhost:28780/redirect302\".";)
+
+ test(
"http://localhost:28780/status/307?location=/redirect307";,
+ "WARN: filename.mk:1: Homepage "+
+ "\"http://localhost:28780/status/307?location=/redirect307\"; "+
+ "redirects to \"http://localhost:28780/redirect307\".";)
+
+ test(
"http://localhost:28780/status/404";,
- "http://localhost:28780/status/500";)
+ "WARN: filename.mk:1: Homepage \"http://localhost:28780/status/404\"; "+
+ "returns HTTP status \"404 Not Found\".")
- vt.Output(
- "WARN: filename.mk:2: Homepage "+
- "\"http://localhost:28780/status/301?location=/redirect301\"; "+
- "redirects to \"http://localhost:28780/redirect301\".";,
- "WARN: filename.mk:3: Homepage "+
- "\"http://localhost:28780/status/302?location=/redirect302\"; "+
- "redirects to \"http://localhost:28780/redirect302\".";,
- "WARN: filename.mk:4: Homepage "+
- "\"http://localhost:28780/status/307?location=/redirect307\"; "+
- "redirects to \"http://localhost:28780/redirect307\".";,
- "WARN: filename.mk:5: Homepage \"http://localhost:28780/status/404\"; "+
- "returns HTTP status \"404 Not Found\".",
- "WARN: filename.mk:6: Homepage \"http://localhost:28780/status/500\"; "+
+ test(
+ "http://localhost:28780/status/500";,
+ "WARN: filename.mk:1: Homepage \"http://localhost:28780/status/500\"; "+
"returns HTTP status \"500 Internal Server Error\".")
- vt.Values(
- "http://localhost:28780/timeout";)
-
- vt.Output(
- "WARN: filename.mk:11: Homepage \"http://localhost:28780/timeout\"; " +
+ test(
+ "http://localhost:28780/timeout";,
+ "WARN: filename.mk:1: Homepage \"http://localhost:28780/timeout\"; "+
"cannot be checked: timeout")
- vt.Values(
- "http://localhost:28780/%invalid";)
-
- vt.Output(
- "ERROR: filename.mk:21: Invalid URL \"http://localhost:28780/%invalid\".";)
-
- vt.Values(
- "http://localhost:28781/";)
-
- // The "unknown network error" is for compatibility with Go < 1.13.
- t.CheckOutputMatches(
- `^WARN: filename\.mk:31: Homepage "http://localhost:28781/"; ` +
- `cannot be checked: (connection refused|unknown network error:.*)$`)
-
- vt.Values(
- "https://no-such-name.example.org/";)
+ test(
+ "http://localhost:28780/%invalid";,
+ "ERROR: filename.mk:1: Invalid URL \"http://localhost:28780/%invalid\".";)
- // The "unknown network error" is for compatibility with Go < 1.13.
- t.CheckOutputMatches(
- `^WARN: filename\.mk:41: Homepage "https://no-such-name.example.org/"; ` +
+ testMatches(
+ "http://localhost:28781/";,
+ // The "unknown network error" is for compatibility with Go < 1.13.
+ `^WARN: filename\.mk:1: Homepage "http://localhost:28781/"; `+
+ `cannot be checked: (connection refused|timeout|unknown network error:.*)$`)
+
+ testMatches(
+ "https://no-such-name.example.org/";,
+ // The "unknown network error" is for compatibility with Go < 1.13.
+ `^WARN: filename\.mk:1: Homepage "https://no-such-name.example.org/"; `+
`cannot be checked: (name not found|unknown network error:.*)$`)
- vt.Values(
- "https://!!!invalid/";)
+ // Syntactically invalid URLs are silently skipped since VartypeCheck.URL
+ // already warns about them.
+ test(
+ "https://!!!invalid/";,
+ nil...)
- t.CheckOutputLines(
- "WARN: filename.mk:51: \"https://!!!invalid/\"; is not a valid URL.")
+ // URLs with variables are skipped since they cannot be resolved in this form.
+ test(
+ "https://${SERVER}/";,
+ nil...)
}
func (s *Suite) Test_HomepageChecker_isReachableOnline(c *check.C) {
@@ -401,7 +424,7 @@ func (s *Suite) Test_HomepageChecker_isR
writer.WriteHeader(status)
})
mux.HandleFunc("/timeout", func(http.ResponseWriter, *http.Request) {
- time.Sleep(5 * time.Second)
+ time.Sleep(1 * time.Second)
})
mux.HandleFunc("/ok/", func(http.ResponseWriter, *http.Request) {})
@@ -425,6 +448,7 @@ func (s *Suite) Test_HomepageChecker_isR
test := func(url string, reachable YesNoUnknown) {
ck := NewHomepageChecker(url, url, nil, nil)
+ ck.Timeout = 500 * time.Millisecond
actual := ck.isReachableOnline(url)
t.CheckEquals(actual, reachable)
@@ -444,8 +468,8 @@ func (s *Suite) Test_HomepageChecker_isR
func (s *Suite) Test_HomepageChecker_hasAnySuffix(c *check.C) {
t := s.Init(c)
- test := func(s string, hasAnySuffix bool, suffixes ...string) {
- actual := (*HomepageChecker).hasAnySuffix(nil, s, suffixes...)
+ test := func(s string, hasAnySuffix bool, suffix string) {
+ actual := (*HomepageChecker).hasAnySuffix(nil, s, suffix)
t.CheckEquals(actual, hasAnySuffix)
}
@@ -455,6 +479,7 @@ func (s *Suite) Test_HomepageChecker_has
test("example.org", true, "example.org")
test("example.org", false, ".example.org")
test("example.org", true, ".org")
+ test("borg", false, "org")
}
func (s *Suite) Test_HomepageChecker_classifyNetworkError(c *check.C) {
@@ -468,5 +493,8 @@ func (s *Suite) Test_HomepageChecker_cla
test(syscall.Errno(10061), "connection refused")
test(syscall.ECONNREFUSED, "connection refused")
+ test(syscall.ECONNRESET, "unknown network error: connection reset by peer")
test(errors.New("unknown"), "unknown network error: unknown")
+ test(&net.AddrError{"msg", "addr"}, "unknown network error: address addr: msg")
+ test(&net.DNSError{Err: "msg", Name: "name"}, "unknown network error: lookup name: msg")
}
Index: pkgsrc/pkgtools/pkglint/files/lineslexer.go
diff -u pkgsrc/pkgtools/pkglint/files/lineslexer.go:1.2 pkgsrc/pkgtools/pkglint/files/lineslexer.go:1.3
--- pkgsrc/pkgtools/pkglint/files/lineslexer.go:1.2 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/lineslexer.go Sat Feb 15 13:48:40 2020
@@ -94,15 +94,15 @@ func (llex *LinesLexer) SkipEmptyOrNote(
if llex.index < llex.lines.Len() || llex.lines.Len() == 0 {
fix := llex.CurrentLine().Autofix()
- fix.Notef("Empty line expected before this line.")
+ fix.Notef("Empty line expected above this line.")
if !llex.EOF() {
- fix.InsertBefore("")
+ fix.InsertAbove("")
}
fix.Apply()
} else {
fix := llex.PreviousLine().Autofix()
- fix.Notef("Empty line expected after this line.")
- fix.InsertAfter("")
+ fix.Notef("Empty line expected below this line.")
+ fix.InsertBelow("")
fix.Apply()
}
Index: pkgsrc/pkgtools/pkglint/files/line.go
diff -u pkgsrc/pkgtools/pkglint/files/line.go:1.45 pkgsrc/pkgtools/pkglint/files/line.go:1.46
--- pkgsrc/pkgtools/pkglint/files/line.go:1.45 Wed Feb 5 04:09:00 2020
+++ pkgsrc/pkgtools/pkglint/files/line.go Sat Feb 15 13:48:40 2020
@@ -20,25 +20,10 @@ import (
)
type RawLine struct {
- Lineno int // Counting starts at 1
-
// The line as read in from the file, including newline;
// can never be empty. Only in the very last line of each file,
// the trailing newline might be missing.
orignl string
-
- // The line as modified by Autofix, including newline;
- // empty string for deleted lines.
- textnl string
-
- // XXX: Since only Autofix needs to distinguish between orignl and textnl,
- // one of these fields should probably be moved there.
-}
-
-func (rline *RawLine) String() string { return sprintf("%d:%s", rline.Lineno, rline.textnl) }
-
-func (rline *RawLine) Text() string {
- return strings.TrimSuffix(rline.textnl, "\n")
}
func (rline *RawLine) Orig() string {
@@ -46,41 +31,31 @@ func (rline *RawLine) Orig() string {
}
type Location struct {
- Filename CurrPath
- firstLine int32 // zero means the whole file, -1 means EOF
- lastLine int32 // usually the same as firstLine, may differ in Makefiles
-}
+ Filename CurrPath
-func NewLocation(filename CurrPath, firstLine, lastLine int) Location {
- return Location{filename, int32(firstLine), int32(lastLine)}
+ // zero means the whole file, -1 means EOF, normal lines start with 1
+ lineno int
}
-func (loc *Location) String() string {
- return loc.Filename.String() + ":" + loc.Linenos()
+func NewLocation(filename CurrPath, lineno int) Location {
+ return Location{filename, lineno}
}
-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)
- }
+func (loc *Location) Lineno(rawIndex int) int {
+ return loc.lineno + rawIndex
}
+// File resolves the given path relative to the directory of this
+// location.
func (loc *Location) File(rel RelPath) CurrPath {
- return loc.Filename.DirNoClean().JoinNoClean(rel)
+ return loc.Filename.Dir().JoinNoClean(rel)
}
// Line represents a line of text from a file.
type Line 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).
- Location
+ Location Location
Basename string // the last component of the Filename
// the text of the line, without the trailing newline character;
@@ -88,55 +63,95 @@ type Line struct {
// 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 Once
+ raw []*RawLine // contains the original text including trailing newline
+ fix *Autofix // any changes that pkglint would like to apply to the line
+ once Once
// XXX: Filename and Basename could be replaced with a pointer to a Lines object.
}
func NewLine(filename CurrPath, lineno int, text string, rawLine *RawLine) *Line {
assert(rawLine != nil) // Use NewLineMulti for creating a Line with no RawLine attached to it.
- return NewLineMulti(filename, lineno, lineno, text, []*RawLine{rawLine})
+ return NewLineMulti(filename, lineno, text, []*RawLine{rawLine})
}
// NewLineMulti is for logical Makefile lines that end with backslash.
-func NewLineMulti(filename CurrPath, firstLine, lastLine int, text string, rawLines []*RawLine) *Line {
- return &Line{NewLocation(filename, firstLine, lastLine), filename.Base(), text, rawLines, nil, Once{}}
+func NewLineMulti(filename CurrPath, firstLine int, text string, rawLines []*RawLine) *Line {
+ return &Line{NewLocation(filename, firstLine), filename.Base(), text, rawLines, nil, Once{}}
}
// NewLineEOF creates a dummy line for logging, with the "line number" EOF.
func NewLineEOF(filename CurrPath) *Line {
- return NewLineMulti(filename, -1, 0, "", nil)
+ return NewLineMulti(filename, -1, "", nil)
}
// NewLineWhole creates a dummy line for logging messages that affect a file as a whole.
func NewLineWhole(filename CurrPath) *Line {
- return NewLineMulti(filename, 0, 0, "", nil)
+ return NewLineMulti(filename, 0, "", nil)
+}
+
+func (line *Line) Filename() CurrPath { return line.Location.Filename }
+
+// File resolves the given path relative to the directory where this line
+// appears in.
+func (line *Line) File(rel RelPath) CurrPath { return line.Location.File(rel) }
+
+func (line *Line) Linenos() string {
+ first := line.Location.lineno
+ if first == -1 {
+ return "EOF"
+ }
+ if first == 0 {
+ return ""
+ }
+
+ n := len(line.raw)
+ if n == 1 {
+ return strconv.Itoa(first)
+ }
+ return sprintf("%d--%d", first, first+n-1)
}
// RelLine returns a reference to another line,
// which can be in the same file or in a different file.
func (line *Line) RelLine(other *Line) string {
- return line.RelLocation(other.Location)
+ assert(other.Location.Lineno(0) >= 1)
+
+ if line.Filename() != other.Filename() {
+ return line.Rel(other.Filename()).String() + ":" + other.Linenos()
+ }
+ return "line " + other.Linenos()
}
func (line *Line) RelLocation(other Location) string {
- if line.Filename != other.Filename {
- return line.Rel(other.Filename).String() + ":" + other.Linenos()
+ lineno := other.Lineno(0)
+ assert(lineno >= 1)
+
+ if line.Filename() != other.Filename {
+ return sprintf("%s:%d", line.Rel(other.Filename).String(), lineno)
}
- return "line " + other.Linenos()
+ return sprintf("line %d", lineno)
}
// Rel 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.
func (line *Line) Rel(other CurrPath) RelPath {
- return G.Pkgsrc.Relpath(line.Filename.DirNoClean(), other)
+ return G.Pkgsrc.Relpath(line.Filename().Dir(), other)
}
-func (line *Line) IsMultiline() bool {
- return line.firstLine > 0 && line.firstLine != line.lastLine
+func (line *Line) IsMultiline() bool { return len(line.raw) > 1 }
+
+// RawText returns the raw text from the given physical line,
+// excluding \n, including any previous autofixes.
+func (line *Line) RawText(rawIndex int) string {
+ var textnl string
+ if line.fix != nil {
+ textnl = line.fix.texts[rawIndex]
+ } else {
+ textnl = line.raw[rawIndex].orignl
+ }
+ return strings.TrimSuffix(textnl, "\n")
}
func (line *Line) IsCvsID(prefixRe regex.Pattern) (found bool, expanded bool) {
@@ -144,16 +159,6 @@ func (line *Line) IsCvsID(prefixRe regex
return m, exp != ""
}
-// FIXME: By definition there cannot be fatal diagnostics.
-// Having these was a misconception from the beginning,
-// and they must be re-classified as fatal technical errors.
-func (line *Line) Fatalf(format string, args ...interface{}) {
- if trace.Tracing {
- trace.Stepf("Fatalf: %q, %v", format, args)
- }
- G.Logger.Diag(line, Fatal, format, args...)
-}
-
func (line *Line) Errorf(format string, args ...interface{}) {
G.Logger.Diag(line, Error, format, args...)
}
@@ -169,7 +174,7 @@ func (line *Line) Notef(format string, a
func (line *Line) Explain(explanation ...string) { G.Logger.Explain(explanation...) }
func (line *Line) String() string {
- return sprintf("%s:%s: %s", line.Filename, line.Linenos(), line.Text)
+ return sprintf("%s:%s: %s", line.Filename(), line.Linenos(), line.Text)
}
// Autofix returns the autofix instance belonging to the line.
@@ -188,19 +193,19 @@ func (line *Line) String() string {
//
// fix.Replace("from", "to")
// fix.ReplaceAfter("prefix", "from", "to")
-// fix.InsertBefore("new line")
-// fix.InsertAfter("new line")
+// fix.InsertAbove("new line")
+// fix.InsertBelow("new line")
// fix.Delete()
// fix.Custom(func(showAutofix, autofix bool) {})
//
// fix.Apply()
func (line *Line) Autofix() *Autofix {
- if line.autofix == nil {
- line.autofix = NewAutofix(line)
+ if line.fix == nil {
+ line.fix = NewAutofix(line)
} else {
// This assertion fails if an Autofix is reused before
// its Apply method is called.
- assert(line.autofix.autofixShortTerm.diagFormat == "")
+ assert(line.fix.autofixShortTerm.diagFormat == "")
}
- return line.autofix
+ return line.fix
}
Index: pkgsrc/pkgtools/pkglint/files/line_test.go
diff -u pkgsrc/pkgtools/pkglint/files/line_test.go:1.20 pkgsrc/pkgtools/pkglint/files/line_test.go:1.21
--- pkgsrc/pkgtools/pkglint/files/line_test.go:1.20 Wed Nov 27 22:10:07 2019
+++ pkgsrc/pkgtools/pkglint/files/line_test.go Sat Feb 15 13:48:40 2020
@@ -4,54 +4,37 @@ import (
"gopkg.in/check.v1"
)
-func (s *Suite) Test_RawLine_String(c *check.C) {
+func (s *Suite) Test_NewLine__assertion(c *check.C) {
t := s.Init(c)
- line := t.NewLine("filename", 123, "text")
-
- t.CheckEquals(line.raw[0].String(), "123:text\n")
+ t.ExpectAssert(func() { NewLine("filename", 123, "text", nil) })
}
-func (s *Suite) Test_NewLine__assertion(c *check.C) {
+func (s *Suite) Test_Line_RelLine__assertion(c *check.C) {
t := s.Init(c)
- t.ExpectAssert(func() { NewLine("filename", 123, "text", nil) })
+ line := t.NewLine("filename", 123, "")
+ whole := NewLineWhole("filename")
+
+ t.ExpectAssert(func() { line.RelLine(whole) })
}
-func (s *Suite) Test_Line_IsMultiline(c *check.C) {
+func (s *Suite) Test_Line_RelLocation__assertion(c *check.C) {
t := s.Init(c)
- t.CheckEquals(t.NewLine("filename", 123, "text").IsMultiline(), false)
- t.CheckEquals(NewLineEOF("filename").IsMultiline(), false)
+ line := t.NewLine("filename", 123, "")
+ whole := NewLineWhole("filename")
- t.CheckEquals(NewLineMulti("filename", 123, 125, "text", nil).IsMultiline(), true)
+ t.ExpectAssert(func() { line.RelLocation(whole.Location) })
}
-// In case of a fatal error, pkglint quits in a controlled manner,
-// and the trace log shows where the fatal error happened.
-func (s *Suite) Test_Line_Fatalf__trace(c *check.C) {
+func (s *Suite) Test_Line_IsMultiline(c *check.C) {
t := s.Init(c)
- line := t.NewLine("filename", 123, "")
- t.EnableTracingToLog()
+ t.CheckEquals(t.NewLine("filename", 123, "text").IsMultiline(), false)
+ t.CheckEquals(NewLineEOF("filename").IsMultiline(), false)
- inner := func() {
- defer trace.Call0()()
- line.Fatalf("Cannot continue because of %q and %q.", "reason 1", "reason 2")
- }
- outer := func() {
- defer trace.Call0()()
- inner()
- }
-
- t.ExpectFatal(
- outer,
- "TRACE: + (*Suite).Test_Line_Fatalf__trace.func2()",
- "TRACE: 1 + (*Suite).Test_Line_Fatalf__trace.func1()",
- "TRACE: 1 2 Fatalf: \"Cannot continue because of %q and %q.\", [reason 1 reason 2]",
- "TRACE: 1 - (*Suite).Test_Line_Fatalf__trace.func1()",
- "TRACE: - (*Suite).Test_Line_Fatalf__trace.func2()",
- "FATAL: filename:123: Cannot continue because of \"reason 1\" and \"reason 2\".")
+ t.CheckEquals(NewLineMulti("filename", 123, "text", []*RawLine{nil, nil, nil}).IsMultiline(), true)
}
func (s *Suite) Test_Line_Autofix__reuse_incomplete(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/linechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/linechecker.go:1.17 pkgsrc/pkgtools/pkglint/files/linechecker.go:1.18
--- pkgsrc/pkgtools/pkglint/files/linechecker.go:1.17 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/linechecker.go Sat Feb 15 13:48:40 2020
@@ -42,7 +42,7 @@ func (ck LineChecker) CheckTrailingWhite
// be Markdown files in pkgsrc, this code has to be adjusted.
rawIndex := len(ck.line.raw) - 1
- text := ck.line.raw[rawIndex].Text()
+ text := ck.line.RawText(rawIndex)
trimmedLen := len(rtrimHspace(text))
if trimmedLen == len(text) {
return
Index: pkgsrc/pkgtools/pkglint/files/lines.go
diff -u pkgsrc/pkgtools/pkglint/files/lines.go:1.12 pkgsrc/pkgtools/pkglint/files/lines.go:1.13
--- pkgsrc/pkgtools/pkglint/files/lines.go:1.12 Mon Dec 30 16:27:13 2019
+++ pkgsrc/pkgtools/pkglint/files/lines.go Sat Feb 15 13:48:40 2020
@@ -18,7 +18,7 @@ func (ls *Lines) Len() int { return len(
func (ls *Lines) LastLine() *Line { return ls.Lines[ls.Len()-1] }
-func (ls *Lines) EOFLine() *Line { return NewLineMulti(ls.Filename, -1, -1, "", nil) }
+func (ls *Lines) EOFLine() *Line { return NewLineMulti(ls.Filename, -1, "", nil) }
// Whole returns a virtual line that can be used for issuing diagnostics
// and explanations, but not for text replacements.
@@ -71,7 +71,7 @@ func (ls *Lines) CheckCvsID(index int, p
"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.")
- fix.InsertBefore(suggestedPrefix + "$" + "NetBSD$")
+ fix.InsertAbove(suggestedPrefix + "$" + "NetBSD$")
fix.Apply()
return false
Index: pkgsrc/pkgtools/pkglint/files/mklineparser.go
diff -u pkgsrc/pkgtools/pkglint/files/mklineparser.go:1.12 pkgsrc/pkgtools/pkglint/files/mklineparser.go:1.13
--- pkgsrc/pkgtools/pkglint/files/mklineparser.go:1.12 Mon Jan 6 20:38:42 2020
+++ pkgsrc/pkgtools/pkglint/files/mklineparser.go Sat Feb 15 13:48:40 2020
@@ -170,7 +170,7 @@ func (p MkLineParser) fixSpaceAfterVarna
break
default:
- parts := NewVaralignSplitter().split(line.raw[0].Text(), true)
+ parts := NewVaralignSplitter().split(line.RawText(0), true)
before := parts.leadingComment + parts.varnameOp + parts.spaceBeforeValue
after := alignWith(varname+op.String(), before)
Index: pkgsrc/pkgtools/pkglint/files/varalignblock_test.go
diff -u pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.12 pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.13
--- pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.12 Wed Feb 5 04:09:00 2020
+++ pkgsrc/pkgtools/pkglint/files/varalignblock_test.go Sat Feb 15 13:48:40 2020
@@ -3308,7 +3308,7 @@ func (s *Suite) Test_VaralignSplitter_sp
// as usual.
//
// 2. It is a continuation of the value, and therefore the value ends
- // here; everything after this line is part of the trailing comment.
+ // here; everything below this line is part of the trailing comment.
//
// The character that follows the comment character decides which
// interpretation is used. A space makes the comment a trailing
@@ -3566,7 +3566,7 @@ func (s *Suite) Test_varalignLine_alignV
info.alignValueSingle(column)
t.CheckEqualsf(
- mkline.raw[0].Text(), after,
+ mkline.RawText(0), after,
"Line.raw.text, autofix=%v", autofix)
// As of 2019-12-11, the info fields are not updated
@@ -3724,14 +3724,14 @@ func (s *Suite) Test_varalignLine_alignV
assert(len(mklines.mklines) == 1)
mkline := mklines.mklines[0]
- text := mkline.raw[0].Text()
+ text := mkline.RawText(0)
parts := NewVaralignSplitter().split(text, true)
info := &varalignLine{mkline, 0, false, parts}
info.alignValueInitial(column)
t.CheckEqualsf(
- mkline.raw[0].Text(), after,
+ mkline.RawText(0), after,
"Line.raw.text, autofix=%v", autofix)
t.CheckEqualsf(info.String(), after,
@@ -3831,7 +3831,7 @@ func (s *Suite) Test_varalignLine_alignV
// newLine creates a line consisting of either 2 or 3 physical lines.
// The given text ends up in the raw line with index 1.
- newLine := func(text string, column, indentDiff int) (*varalignLine, *RawLine) {
+ newLine := func(text string, column, indentDiff int) (*varalignLine, *MkLine) {
t.CheckDotColumns("", text)
leading := alignWith("VAR=", indent(column)) + "value \\"
@@ -3845,18 +3845,18 @@ func (s *Suite) Test_varalignLine_alignV
mkline := mklines.mklines[0]
parts := NewVaralignSplitter().split(text, false)
- return &varalignLine{mkline, 1, false, parts}, mkline.raw[1]
+ return &varalignLine{mkline, 1, false, parts}, mkline
}
test := func(before string, column, indentDiff int, after string, diagnostics ...string) {
doTest := func(autofix bool) {
- info, raw := newLine(before, column, indentDiff)
+ info, mkline := newLine(before, column, indentDiff)
width := imax(column, info.valueColumn()+indentDiff)
info.alignValueMultiFollow(width)
- t.CheckEquals(raw.Text(), after)
+ t.CheckEquals(mkline.RawText(1), after)
}
t.ExpectDiagnosticsAutofix(
@@ -4110,14 +4110,14 @@ func (s *Suite) Test_varalignLine_alignV
assert(len(mklines.mklines) == 1)
mkline := mklines.mklines[0]
- text := mkline.raw[0].Text()
+ text := mkline.RawText(0)
parts := NewVaralignSplitter().split(text, true)
info := &varalignLine{mkline, 0, false, parts}
info.alignValue(column)
t.CheckEqualsf(
- mkline.raw[0].Text(), after,
+ mkline.RawText(0), after,
"Line.raw.text, autofix=%v", autofix)
t.CheckEqualsf(info.String(), after,
@@ -4137,7 +4137,7 @@ func (s *Suite) Test_varalignLine_alignV
assert(len(mklines.mklines) == 1)
mkline := mklines.mklines[0]
- text := mkline.raw[0].Text()
+ text := mkline.RawText(0)
parts := NewVaralignSplitter().split(text, true)
info := &varalignLine{mkline, 0, false, parts}
@@ -4145,7 +4145,7 @@ func (s *Suite) Test_varalignLine_alignV
func() { info.alignValue(column) })
t.CheckEqualsf(
- mkline.raw[0].Text(), before,
+ mkline.RawText(0), before,
"Line.raw.text, autofix=%v", autofix)
t.CheckEqualsf(info.String(), before,
@@ -4224,14 +4224,14 @@ func (s *Suite) Test_varalignLine_alignC
assert(len(mklines.mklines) == 1)
mkline := mklines.mklines[0]
- text := mkline.raw[rawIndex].Text()
+ text := mkline.RawText(rawIndex)
parts := NewVaralignSplitter().split(text, rawIndex == 0)
info := &varalignLine{mkline, rawIndex, false, parts}
info.alignContinuation(valueColumn, rightMarginColumn)
t.CheckEqualsf(
- mkline.raw[rawIndex].Text(), after,
+ mkline.RawText(rawIndex), after,
"Line.raw.text, autofix=%v", autofix)
t.CheckEqualsf(info.String(), after,
Index: pkgsrc/pkgtools/pkglint/files/lines_test.go
diff -u pkgsrc/pkgtools/pkglint/files/lines_test.go:1.11 pkgsrc/pkgtools/pkglint/files/lines_test.go:1.12
--- pkgsrc/pkgtools/pkglint/files/lines_test.go:1.11 Sun Sep 8 22:47:47 2019
+++ pkgsrc/pkgtools/pkglint/files/lines_test.go Sat Feb 15 13:48:40 2020
@@ -59,9 +59,9 @@ func (s *Suite) Test_Lines_CheckCvsID__w
t.CheckOutputLines(
"AUTOFIX: ~/wip/package/file1.mk:1: Replacing \"# $"+"NetBSD: dummy $\" with \"# $"+"NetBSD$\".",
- "AUTOFIX: ~/wip/package/file3.mk:1: Inserting a line \"# $"+"NetBSD$\" before this line.",
- "AUTOFIX: ~/wip/package/file4.mk:1: Inserting a line \"# $"+"NetBSD$\" before this line.",
- "AUTOFIX: ~/wip/package/file5.mk:1: Inserting a line \"# $"+"NetBSD$\" before this line.")
+ "AUTOFIX: ~/wip/package/file3.mk:1: Inserting a line \"# $"+"NetBSD$\" above this line.",
+ "AUTOFIX: ~/wip/package/file4.mk:1: Inserting a line \"# $"+"NetBSD$\" above this line.",
+ "AUTOFIX: ~/wip/package/file5.mk:1: Inserting a line \"# $"+"NetBSD$\" above this line.")
// In production mode, this error is disabled since it doesn't provide
// enough benefit compared to the work it would produce.
Index: pkgsrc/pkgtools/pkglint/files/redundantscope.go
diff -u pkgsrc/pkgtools/pkglint/files/redundantscope.go:1.11 pkgsrc/pkgtools/pkglint/files/redundantscope.go:1.12
--- pkgsrc/pkgtools/pkglint/files/redundantscope.go:1.11 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/redundantscope.go Sat Feb 15 13:48:40 2020
@@ -46,10 +46,10 @@ func (s *RedundantScope) checkLine(mklin
}
func (s *RedundantScope) updateIncludePath(mkline *MkLine) {
- if mkline.firstLine == 1 {
- s.includePath.push(mkline.Location.Filename)
+ if mkline.Location.lineno == 1 {
+ s.includePath.push(mkline.Filename())
} else {
- s.includePath.popUntil(mkline.Location.Filename)
+ s.includePath.popUntil(mkline.Filename())
}
}
Index: pkgsrc/pkgtools/pkglint/files/lineslexer_test.go
diff -u pkgsrc/pkgtools/pkglint/files/lineslexer_test.go:1.1 pkgsrc/pkgtools/pkglint/files/lineslexer_test.go:1.2
--- pkgsrc/pkgtools/pkglint/files/lineslexer_test.go:1.1 Sun Dec 8 00:06:38 2019
+++ pkgsrc/pkgtools/pkglint/files/lineslexer_test.go Sat Feb 15 13:48:40 2020
@@ -30,7 +30,7 @@ func (s *Suite) Test_LinesLexer_SkipEmpt
llex.SkipEmptyOrNote()
t.CheckOutputLines(
- "NOTE: file.txt:1: Empty line expected before this line.")
+ "NOTE: file.txt:1: Empty line expected above this line.")
}
func (s *Suite) Test_LinesLexer_SkipEmptyOrNote__end_of_file(c *check.C) {
@@ -47,7 +47,7 @@ func (s *Suite) Test_LinesLexer_SkipEmpt
llex.SkipEmptyOrNote()
t.CheckOutputLines(
- "NOTE: file.txt:2: Empty line expected after this line.")
+ "NOTE: file.txt:2: Empty line expected below this line.")
}
func (s *Suite) Test_MkLinesLexer_SkipIf(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/logging_test.go
diff -u pkgsrc/pkgtools/pkglint/files/logging_test.go:1.26 pkgsrc/pkgtools/pkglint/files/logging_test.go:1.27
--- pkgsrc/pkgtools/pkglint/files/logging_test.go:1.26 Mon Dec 30 16:27:13 2019
+++ pkgsrc/pkgtools/pkglint/files/logging_test.go Sat Feb 15 13:48:40 2020
@@ -218,17 +218,17 @@ func (s *Suite) Test_Logger_Diag__show_s
fix := line.Autofix()
fix.Notef("Diagnostics can show the differences in autofix mode.")
- fix.InsertBefore("new line before")
- fix.InsertAfter("new line after")
+ fix.InsertAbove("new line above")
+ fix.InsertBelow("new line below")
fix.Apply()
t.CheckOutputLines(
"NOTE: filename:123: Diagnostics can show the differences in autofix mode.",
- "AUTOFIX: filename:123: Inserting a line \"new line before\" before this line.",
- "AUTOFIX: filename:123: Inserting a line \"new line after\" after this line.",
- "+\tnew line before",
+ "AUTOFIX: filename:123: Inserting a line \"new line above\" above this line.",
+ "AUTOFIX: filename:123: Inserting a line \"new line below\" below this line.",
+ "+\tnew line above",
">\ttext",
- "+\tnew line after")
+ "+\tnew line below")
}
func (s *Suite) Test_Logger_Diag__show_source_with_whole_file(c *check.C) {
@@ -538,25 +538,17 @@ func (s *Suite) Test_Logger_writeSource_
"")
}
+// Fatal errors are not specific to a single line, therefore they only
+// take a filename as argument.
+// The --show-autofix and --source options have no effect on fatal errors.
func (s *Suite) Test_Logger_writeSource__fatal_with_show_autofix(c *check.C) {
t := s.Init(c)
t.SetUpCommandLine("--source", "--show-autofix")
- lines := t.SetUpFileLines("DESCR",
- "The first line")
- // In the unusual constellation where a fatal error occurs with both
- // --source and --show-autofix, and the line has not had any autofix,
- // the cited source code is shown above the diagnostic. This is
- // different from the usual order in --show-autofix mode, which is to
- // show the diagnostic first and then its effects.
- //
- // This inconsistency does not matter though since it is extremely
- // rare.
t.ExpectFatal(
- func() { lines.Lines[0].Fatalf("Fatal.") },
- ">\tThe first line",
- "FATAL: ~/DESCR:1: Fatal.")
+ func() { G.Logger.TechFatalf("DESCR", "Fatal.") },
+ "FATAL: DESCR: Fatal.")
}
// See Test__show_source_separator_show_autofix for the ordering of the
@@ -598,6 +590,58 @@ func (s *Suite) Test_Logger_writeSource_
"+\tThe bronze medal line")
}
+func (s *Suite) Test_Logger_writeSource__first_warn_then_autofix(c *check.C) {
+ t := s.Init(c)
+
+ test := func(diagnostics ...string) {
+ lines := t.SetUpFileLines("DESCR",
+ "The first line",
+ "The second line")
+ line := lines.Lines[0]
+
+ line.Warnf("Warning.")
+ fix := line.Autofix()
+ fix.Warnf("Autofix.")
+ fix.Replace("first", "upper")
+ fix.Apply()
+
+ fix = lines.Lines[1].Autofix()
+ fix.Warnf("Autofix.")
+ fix.Replace("second", "last")
+ fix.Apply()
+
+ t.CheckOutput(diagnostics)
+ }
+
+ t.SetUpCommandLine("--source")
+
+ // The warning reports the unmodified source text of the affected line.
+ // Later, the autofix modifies that same line, but the modification is
+ // not reported.
+ // Luckily, this behavior is consistent with the one in line 2, which
+ // also only reports the original source text.
+ test(
+ ">\tThe first line",
+ "WARN: ~/DESCR:1: Warning.",
+ "WARN: ~/DESCR:1: Autofix.",
+ "",
+ ">\tThe second line",
+ "WARN: ~/DESCR:2: Autofix.")
+
+ t.SetUpCommandLine("--source", "--show-autofix")
+
+ test(
+ "WARN: ~/DESCR:1: Autofix.",
+ "AUTOFIX: ~/DESCR:1: Replacing \"first\" with \"upper\".",
+ "-\tThe first line",
+ "+\tThe upper line",
+ "",
+ "WARN: ~/DESCR:2: Autofix.",
+ "AUTOFIX: ~/DESCR:2: Replacing \"second\" with \"last\".",
+ "-\tThe second line",
+ "+\tThe last line")
+}
+
// Calling Logf without further preparation just logs the message.
// Suppressing duplicate messages or filtering messages happens
// in other methods of the Logger, namely Relevant, FirstTime, Diag.
@@ -670,7 +714,7 @@ func (s *Suite) Test_Logger_Logf__profil
line := t.NewLine("filename", 123, "text")
- G.Opts.Profiling = true
+ G.Profiling = true
G.Logger.histo = histogram.New()
line.Warnf("Warning.")
@@ -687,7 +731,7 @@ func (s *Suite) Test_Logger_Logf__profil
t.SetUpCommandLine("--show-autofix", "--source", "--explain")
line := t.NewLine("filename", 123, "text")
- G.Opts.Profiling = true
+ G.Profiling = true
G.Logger.histo = histogram.New()
fix := line.Autofix()
@@ -859,6 +903,36 @@ func (s *Suite) Test_Logger_Logf__wordin
"NOTE: filename:13: This should.")
}
+// In case of a fatal error, pkglint quits in a controlled manner,
+// and the trace log shows where the fatal error happened.
+func (s *Suite) Test_Logger_TechFatalf__trace(c *check.C) {
+ t := s.Init(c)
+
+ t.EnableTracingToLog()
+
+ inner := func() {
+ defer trace.Call0()()
+ G.Logger.TechFatalf(
+ "filename",
+ "Cannot continue because of %q and %q.", "reason 1", "reason 2")
+ }
+ outer := func() {
+ defer trace.Call0()()
+ inner()
+ }
+
+ t.ExpectFatal(
+ outer,
+ "TRACE: + (*Suite).Test_Logger_TechFatalf__trace.func2()",
+ "TRACE: 1 + (*Suite).Test_Logger_TechFatalf__trace.func1()",
+ "TRACE: 1 2 TechFatalf: filename: Cannot continue because of \"reason 1\" and \"reason 2\".",
+ "TRACE: 1 - (*Suite).Test_Logger_TechFatalf__trace.func1()",
+ "TRACE: - (*Suite).Test_Logger_TechFatalf__trace.func2()",
+ "FATAL: filename: Cannot continue because of \"reason 1\" and \"reason 2\".")
+}
+
+// Technical errors are not diagnostics.
+// Therefore --gcc-output-format has no effect on them.
func (s *Suite) Test_Logger_TechErrorf__gcc_format(c *check.C) {
t := s.Init(c)
@@ -867,7 +941,7 @@ func (s *Suite) Test_Logger_TechErrorf__
G.Logger.TechErrorf("filename", "Cannot be opened for %s.", "reading")
t.CheckOutputLines(
- "filename: error: Cannot be opened for reading.")
+ "ERROR: filename: Cannot be opened for reading.")
}
func (s *Suite) Test_Logger_ShowSummary__explanations_with_only(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/mkassignchecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mkassignchecker.go:1.6 pkgsrc/pkgtools/pkglint/files/mkassignchecker.go:1.7
--- pkgsrc/pkgtools/pkglint/files/mkassignchecker.go:1.6 Sun Jan 26 17:12:36 2020
+++ pkgsrc/pkgtools/pkglint/files/mkassignchecker.go Sat Feb 15 13:48:40 2020
@@ -111,7 +111,7 @@ func (ck *MkAssignChecker) checkVarassig
return
}
- if !G.Opts.WarnExtra ||
+ if !G.WarnExtra ||
G.Infrastructure ||
mkline.Op() != opAssignDefault ||
ck.MkLines.Tools.SeenPrefs {
@@ -203,7 +203,7 @@ func (ck *MkAssignChecker) checkVarassig
//
// See checkPermissions.
func (ck *MkAssignChecker) checkVarassignLeftPermissions() {
- if !G.Opts.WarnPerm {
+ if !G.WarnPerm {
return
}
if G.Infrastructure {
@@ -274,7 +274,7 @@ func (ck *MkAssignChecker) checkVarassig
}
func (ck *MkAssignChecker) checkVarassignLeftRationale() {
- if !G.Opts.WarnExtra {
+ if !G.WarnExtra {
return
}
@@ -384,21 +384,21 @@ func (ck *MkAssignChecker) checkVarassig
}
categories := mkline.ValueFields(mkline.Value())
- actual := categories[0]
- expected := G.Pkgsrc.Rel(mkline.Filename).DirNoClean().DirNoClean().Base()
+ primary := categories[0]
+ dir := G.Pkgsrc.Rel(mkline.Filename()).Dir().Dir().Base()
- if expected == "wip" || actual == expected {
+ if primary == dir || dir == "wip" || dir == "regress" {
return
}
fix := mkline.Autofix()
- fix.Warnf("The primary category should be %q, not %q.", expected, actual)
+ fix.Warnf("The primary category should be %q, not %q.", dir, primary)
fix.Explain(
"The primary category of a package should be its location in the",
"pkgsrc directory tree, to make it easy to find the package.",
"All other categories may be added after this primary category.")
- if len(categories) > 1 && categories[1] == expected {
- fix.Replace(categories[0]+" "+categories[1], categories[1]+" "+categories[0])
+ if len(categories) > 1 && categories[1] == dir {
+ fix.Replace(primary+" "+categories[1], categories[1]+" "+primary)
}
fix.Apply()
}
Index: pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go:1.5 pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go:1.6
--- pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go:1.5 Sat Jan 18 21:56:09 2020
+++ pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go Sat Feb 15 13:48:40 2020
@@ -233,7 +233,7 @@ func (s *Suite) Test_MkCondChecker_check
}
test("!empty(VAR)",
- // FIXME: Add a :U modifier if VAR might be undefined.
+ // TODO: Add a :U modifier if VAR might be undefined.
"NOTE: filename.mk:1: !empty(VAR) can be replaced with the simpler ${VAR}.")
}
Index: pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go:1.5 pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go:1.6
--- pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go:1.5 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go Sat Feb 15 13:48:40 2020
@@ -42,7 +42,7 @@ func (ck *MkVarUseChecker) checkUndefine
varname := varuse.varname
switch {
- case !G.Opts.WarnExtra,
+ case !G.WarnExtra,
// Well-known variables are probably defined by the infrastructure.
vartype != nil && !vartype.IsGuessed(),
// TODO: At load time, check ck.MkLines.loadVars instead of allVars.
@@ -153,7 +153,7 @@ func (ck *MkVarUseChecker) checkVarname(
//
// See checkVarassignLeftPermissions.
func (ck *MkVarUseChecker) checkPermissions(vuc *VarUseContext) {
- if !G.Opts.WarnPerm {
+ if !G.WarnPerm {
return
}
if G.Infrastructure {
@@ -448,7 +448,7 @@ func (ck *MkVarUseChecker) warnToolLoadT
// checkVarUseWords checks whether a variable use of the form ${VAR}
// or ${VAR:modifiers} is allowed in a certain context.
func (ck *MkVarUseChecker) checkQuoting(vuc *VarUseContext) {
- if !G.Opts.WarnQuoting || vuc.quoting == VucQuotUnknown {
+ if !G.WarnQuoting || vuc.quoting == VucQuotUnknown {
return
}
Index: pkgsrc/pkgtools/pkglint/files/mkline.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline.go:1.75 pkgsrc/pkgtools/pkglint/files/mkline.go:1.76
--- pkgsrc/pkgtools/pkglint/files/mkline.go:1.75 Sun Jan 26 17:12:36 2020
+++ pkgsrc/pkgtools/pkglint/files/mkline.go Sat Feb 15 13:48:40 2020
@@ -70,7 +70,7 @@ type mkLineDependency struct {
// String returns the filename and line numbers.
func (mkline *MkLine) String() string {
- return sprintf("%s:%s", mkline.Filename, mkline.Linenos())
+ return sprintf("%s:%s", mkline.Filename(), mkline.Linenos())
}
func (mkline *MkLine) HasComment() bool { return mkline.splitResult.hasComment }
@@ -241,9 +241,9 @@ func (mkline *MkLine) Varparam() string
func (mkline *MkLine) Op() MkOperator { return mkline.data.(*mkLineAssign).op }
// ValueAlign applies to variable assignments and returns all the text
-// before the variable value, e.g. "VARNAME+=\t".
+// to the left of the variable value, e.g. "VARNAME+=\t".
func (mkline *MkLine) ValueAlign() string {
- parts := NewVaralignSplitter().split(mkline.Line.raw[0].Text(), true)
+ parts := NewVaralignSplitter().split(mkline.Line.RawText(0), true)
return parts.leadingComment + parts.varnameOp + parts.spaceBeforeValue
}
@@ -316,7 +316,7 @@ func (mkline *MkLine) IncludedFile() Rel
// IncludedFileFull returns the path to the included file.
func (mkline *MkLine) IncludedFileFull() CurrPath {
- dir := mkline.Filename.DirNoClean()
+ dir := mkline.Filename().Dir()
joined := dir.JoinNoClean(mkline.IncludedFile())
return joined.CleanPath()
}
@@ -606,7 +606,7 @@ func (mkline *MkLine) ResolveVarsInRelat
if pkg != nil {
basedir = pkg.File(".")
} else {
- basedir = mkline.Filename.DirNoClean()
+ basedir = mkline.Filename().Dir()
}
tmp := relativePath
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.63 pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.64
--- pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.63 Sun Jan 26 17:12:36 2020
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker.go Sat Feb 15 13:48:40 2020
@@ -43,8 +43,9 @@ func (ck MkLineChecker) checkEmptyContin
}
line := ck.MkLine.Line
- if line.raw[len(line.raw)-1].Orig() == "" {
- lastLine := NewLine(line.Filename, int(line.lastLine), "", line.raw[len(line.raw)-1])
+ lastRawIndex := len(line.raw) - 1
+ if line.raw[lastRawIndex].Orig() == "" {
+ lastLine := NewLine(line.Filename(), line.Location.Lineno(lastRawIndex), "", line.raw[lastRawIndex])
lastLine.Warnf("This line looks empty but continues the previous line.")
lastLine.Explain(
"This line should be indented like other continuation lines,",
@@ -190,7 +191,7 @@ func (ck MkLineChecker) checkShellComman
shellCommand := mkline.ShellCommand()
if hasPrefix(mkline.Text, "\t\t") {
- lexer := textproc.NewLexer(mkline.raw[0].Text())
+ lexer := textproc.NewLexer(mkline.RawText(0))
tabs := lexer.NextBytesFunc(func(b byte) bool { return b == '\t' })
fix := mkline.Autofix()
@@ -201,8 +202,8 @@ func (ck MkLineChecker) checkShellComman
"there is no need to indent some of the commands,",
"or to use more horizontal space than necessary.")
- for i, raw := range mkline.Line.raw {
- if hasPrefix(raw.Text(), tabs) {
+ for i := range mkline.raw {
+ if hasPrefix(mkline.RawText(i), tabs) {
fix.ReplaceAt(i, 0, tabs, "\t")
}
}
@@ -234,7 +235,7 @@ func (ck MkLineChecker) checkInclude() {
includedFile := mkline.IncludedFile()
mustExist := mkline.MustExist()
if trace.Tracing {
- trace.Stepf("includingFile=%s includedFile=%s", mkline.Filename, includedFile)
+ trace.Stepf("includingFile=%s includedFile=%s", mkline.Filename(), includedFile)
}
// TODO: Not every path is relative to the package directory.
ck.CheckRelativePath(NewPackagePath(includedFile), includedFile, mustExist)
@@ -285,7 +286,7 @@ func (ck MkLineChecker) checkIncludeBuil
return
}
- includeInstead := includedFile.DirNoClean().JoinNoClean("buildlink3.mk")
+ includeInstead := includedFile.Dir().JoinNoClean("buildlink3.mk")
fix := mkline.Autofix()
fix.Errorf("%q must not be included directly. Include %q instead.",
@@ -298,9 +299,9 @@ func (ck MkLineChecker) checkDirectiveIn
mkline := ck.MkLine
indent := mkline.Indent()
if expected := strings.Repeat(" ", expectedDepth); indent != expected {
- fix := mkline.Line.Autofix()
+ fix := mkline.Autofix()
fix.Notef("This directive should be indented by %d spaces.", expectedDepth)
- if hasPrefix(mkline.Line.raw[0].Text(), "."+indent) {
+ if hasPrefix(mkline.RawText(0), "."+indent) {
fix.ReplaceAt(0, 0, "."+indent, "."+expected)
}
fix.Apply()
@@ -325,10 +326,12 @@ func (ck MkLineChecker) CheckRelativePat
return
}
- resolvedRel := resolvedPath.AsRelPath()
- abs := mkline.Filename.DirNoClean().JoinNoClean(resolvedRel)
+ abs := G.Pkgsrc.FilePkg(resolvedPath)
+ if abs.IsEmpty() {
+ abs = mkline.File(resolvedPath.AsRelPath())
+ }
if !abs.Exists() {
- pkgsrcPath := G.Pkgsrc.Rel(ck.MkLine.File(resolvedRel))
+ pkgsrcPath := G.Pkgsrc.Rel(ck.MkLine.File(resolvedPath.AsRelPath()))
if mustExist && !ck.MkLines.indentation.HasExists(pkgsrcPath) {
mkline.Errorf("Relative path %q does not exist.", rel)
}
@@ -345,7 +348,7 @@ func (ck MkLineChecker) CheckRelativePat
case matches(resolvedPath.String(), `^\.\./\.\./[^./][^/]*/[^/]`):
// From a package to another package.
- case resolvedPath.HasPrefixPath("../mk") && G.Pkgsrc.Rel(mkline.Filename).Count() == 2:
+ case resolvedPath.HasPrefixPath("../mk") && G.Pkgsrc.Rel(mkline.Filename()).Count() == 2:
// For category Makefiles.
// TODO: Or from a pkgsrc wip package to wip/mk.
Index: pkgsrc/pkgtools/pkglint/files/mklines.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines.go:1.67 pkgsrc/pkgtools/pkglint/files/mklines.go:1.68
--- pkgsrc/pkgtools/pkglint/files/mklines.go:1.67 Mon Jan 6 20:38:42 2020
+++ pkgsrc/pkgtools/pkglint/files/mklines.go Sat Feb 15 13:48:40 2020
@@ -567,7 +567,7 @@ func (mklines *MkLines) CheckUsedBy(rela
if paras[0].to > 1 {
fix := prevLine.Autofix()
fix.Notef(SilentAutofixFormat)
- fix.InsertAfter("")
+ fix.InsertBelow("")
fix.Apply()
}
}
@@ -589,7 +589,7 @@ func (mklines *MkLines) CheckUsedBy(rela
"that file should have a clearly defined and documented purpose,",
"and the filename should reflect that purpose.",
"Typical names are module.mk, plugin.mk or version.mk.")
- fix.InsertAfter(expected)
+ fix.InsertBelow(expected)
fix.Apply()
}
Index: pkgsrc/pkgtools/pkglint/files/package_test.go
diff -u pkgsrc/pkgtools/pkglint/files/package_test.go:1.67 pkgsrc/pkgtools/pkglint/files/package_test.go:1.68
--- pkgsrc/pkgtools/pkglint/files/package_test.go:1.67 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/package_test.go Sat Feb 15 13:48:40 2020
@@ -3127,12 +3127,10 @@ func (s *Suite) Test_Package_checkLinesB
G.Check("mk/ocaml.mk")
G.checkdirPackage("x11/ocaml-graphics")
- t.CheckOutputLines(
- // This error is only reported if the file is checked on its own.
- // If it is checked as part of a package, both bmake and pkglint
- // use the package path as the fallback search path.
- "ERROR: mk/ocaml.mk:2: Relative path " +
- "\"../../lang/ocaml/buildlink3.mk\" does not exist.")
+ // Up to 2020-02-15, pkglint reported a missing relative path in
+ // mk/ocaml.mk:2 since resolving relative paths had not used the
+ // correct base directory.
+ t.CheckOutputEmpty()
}
// Just for code coverage.
@@ -3538,5 +3536,5 @@ func (s *Suite) Test_Package_Includes(c
// See Package.collectConditionalIncludes and Indentation.IsConditional.
t.CheckEquals(
pkg.conditionalIncludes["never.mk"].Location,
- NewLocation(t.File("category/package/Makefile"), 22, 22))
+ NewLocation(t.File("category/package/Makefile"), 22))
}
Index: pkgsrc/pkgtools/pkglint/files/mklines_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.61 pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.62
--- pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.61 Sun Jan 26 17:12:36 2020
+++ pkgsrc/pkgtools/pkglint/files/mklines_test.go Sat Feb 15 13:48:40 2020
@@ -1222,7 +1222,7 @@ func (s *Suite) Test_MkLines_CheckUsedBy
"VARNAME=\tvalue"),
diagnostics(
"WARN: Makefile.common:1: Please add a line \"# used by category/package\" here.",
- "AUTOFIX: Makefile.common:1: Inserting a line \"# used by category/package\" after this line."))
+ "AUTOFIX: Makefile.common:1: Inserting a line \"# used by category/package\" below this line."))
// The "used by" comments may either start in line 2 or in line 3.
test(
@@ -1233,7 +1233,7 @@ func (s *Suite) Test_MkLines_CheckUsedBy
"#"),
diagnostics(
"WARN: Makefile.common:1: Please add a line \"# used by category/package\" here.",
- "AUTOFIX: Makefile.common:1: Inserting a line \"# used by category/package\" after this line."))
+ "AUTOFIX: Makefile.common:1: Inserting a line \"# used by category/package\" below this line."))
// TODO: What if there is an introductory comment first? That should stay at the top of the file.
// TODO: What if the "used by" comments appear in the second paragraph, preceded by only comments and empty lines?
@@ -1248,9 +1248,9 @@ func (s *Suite) Test_MkLines_CheckUsedBy
"# that spans",
"# several lines"),
diagnostics(
- "AUTOFIX: Makefile.common:4: Inserting a line \"\" after this line.",
+ "AUTOFIX: Makefile.common:4: Inserting a line \"\" below this line.",
"WARN: Makefile.common:4: Please add a line \"# used by category/package\" here.",
- "AUTOFIX: Makefile.common:4: Inserting a line \"# used by category/package\" after this line."))
+ "AUTOFIX: Makefile.common:4: Inserting a line \"# used by category/package\" below this line."))
t.CheckEquals(G.Logger.autofixAvailable, true)
}
Index: pkgsrc/pkgtools/pkglint/files/mkshparser_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkshparser_test.go:1.23 pkgsrc/pkgtools/pkglint/files/mkshparser_test.go:1.24
--- pkgsrc/pkgtools/pkglint/files/mkshparser_test.go:1.23 Fri Dec 13 01:39:23 2019
+++ pkgsrc/pkgtools/pkglint/files/mkshparser_test.go Sat Feb 15 13:48:40 2020
@@ -1006,7 +1006,7 @@ func (b *MkShBuilder) Redirected(cmd *Mk
}
func (b *MkShBuilder) Token(mktext string) *ShToken {
- line := NewLine("MkShBuilder.Token.mk", 1, "", &RawLine{1, "\n", "\n"})
+ line := NewLine("MkShBuilder.Token.mk", 1, "", &RawLine{"\n"})
tokenizer := NewShTokenizer(line, mktext)
token := tokenizer.ShToken()
assertf(tokenizer.parser.EOF(), "Invalid token: %q", tokenizer.parser.Rest())
Index: pkgsrc/pkgtools/pkglint/files/package.go
diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.79 pkgsrc/pkgtools/pkglint/files/package.go:1.80
--- pkgsrc/pkgtools/pkglint/files/package.go:1.79 Sat Jan 18 21:56:09 2020
+++ pkgsrc/pkgtools/pkglint/files/package.go Sat Feb 15 13:48:40 2020
@@ -143,7 +143,7 @@ func (pkg *Package) load() ([]CurrPath,
if !hasPrefix(basename, "Makefile.") && !filename.HasSuffixText(".mk") {
return false
}
- if filename.DirNoClean().Base() == "patches" {
+ if filename.Dir().Base() == "patches" {
return false
}
if pkg.Pkgdir == "." {
@@ -197,7 +197,7 @@ func (pkg *Package) loadPackageMakefile(
// when pkglint loaded the package Makefile including all included files into
// a single string. Maybe it makes sense to print the file inclusion hierarchy
// to quickly see files that cannot be included because of unresolved variables.
- if G.Opts.DumpMakefile {
+ if G.DumpMakefile {
G.Logger.out.WriteLine("Whole Makefile (with all included files) follows:")
for _, line := range allLines.lines.Lines {
G.Logger.out.WriteLine(line.String())
@@ -260,7 +260,7 @@ func (pkg *Package) parse(mklines *MkLin
// automatically since the pkgsrc infrastructure does the same.
filename := mklines.lines.Filename
if filename.Base() == "buildlink3.mk" {
- builtin := filename.DirNoClean().JoinNoClean("builtin.mk").CleanPath()
+ builtin := filename.Dir().JoinNoClean("builtin.mk").CleanPath()
builtinRel := G.Pkgsrc.Relpath(pkg.dir, builtin)
if pkg.included.FirstTime(builtinRel.String()) && builtin.IsFile() {
builtinMkLines := LoadMk(builtin, pkg, MustSucceed|LogErrors)
@@ -276,7 +276,7 @@ func (pkg *Package) parseLine(mklines *M
allLines.lines.Lines = append(allLines.lines.Lines, mkline.Line)
if mkline.IsInclude() {
- includingFile := mkline.Filename
+ includingFile := mkline.Filename()
includedFile := mkline.IncludedFile()
includedMkLines, skip := pkg.loadIncluded(mkline, includingFile)
@@ -402,7 +402,7 @@ func (pkg *Package) resolveIncludedFile(
if containsVarUse(includedText) {
if trace.Tracing && !includingFilename.ContainsPath("mk") {
trace.Stepf("%s:%s: Skipping unresolvable include file %q.",
- mkline.Filename, mkline.Linenos(), includedFile)
+ mkline.Filename(), mkline.Linenos(), includedFile)
}
return ""
}
@@ -604,7 +604,7 @@ func (pkg *Package) checkfilePackageMake
//
// If the RedundantScope is applied also to individual files,
// it would have to be added here.
- return G.Opts.CheckGlobal || !G.Pkgsrc.IsInfra(mkline.Filename)
+ return G.CheckGlobal || !G.Pkgsrc.IsInfra(mkline.Filename())
}
pkg.redundant.Check(allLines) // Updates the variables in the scope
pkg.checkCategories()
@@ -620,7 +620,7 @@ func (pkg *Package) checkfilePackageMake
if imake := vars.FirstDefinition("USE_IMAKE"); imake != nil {
if x11 := vars.FirstDefinition("USE_X11"); x11 != nil {
- if !x11.Filename.HasSuffixPath("mk/x11.buildlink3.mk") {
+ if !x11.Filename().HasSuffixPath("mk/x11.buildlink3.mk") {
imake.Notef("USE_IMAKE makes USE_X11 in %s redundant.", imake.RelMkLine(x11))
}
}
@@ -991,7 +991,7 @@ func (pkg *Package) checkGnuConfigureUse
var wrongLines []*MkLine
for _, mkline := range useLanguages.vari.WriteLocations() {
- if G.Pkgsrc.IsInfra(mkline.Line.Filename) {
+ if G.Pkgsrc.IsInfra(mkline.Filename()) {
continue
}
@@ -1035,7 +1035,7 @@ func (pkg *Package) checkUseLanguagesCom
}
if mkline.Basename == "compiler.mk" {
- if G.Pkgsrc.Relpath(pkg.dir, mkline.Filename) == "../../mk/compiler.mk" {
+ if G.Pkgsrc.Relpath(pkg.dir, mkline.Filename()) == "../../mk/compiler.mk" {
return
}
}
@@ -1205,7 +1205,7 @@ func (pkg *Package) checkPossibleDowngra
"This is unusual, since packages are typically upgraded instead of",
"downgraded.")
- case cmp > 0 && !isLocallyModified(mkline.Filename):
+ case cmp > 0 && !isLocallyModified(mkline.Filename()):
mkline.Notef("Package version %q is greater than the latest %q from %s.",
pkgversion, change.Version(), mkline.Line.RelLocation(change.Location))
mkline.Explain(
@@ -1276,7 +1276,7 @@ func (pkg *Package) checkDirent(dirent C
G.checkReg(dirent, basename, G.Pkgsrc.Rel(dirent).Count(), pkg)
case hasPrefix(basename, "work"):
- if G.Opts.Import {
+ if G.Import {
NewLineWhole(dirent).Errorf("Must be cleaned up before committing the package.")
}
return
@@ -1285,7 +1285,7 @@ func (pkg *Package) checkDirent(dirent C
switch {
case basename == "files",
basename == "patches",
- dirent.DirNoClean().Base() == "files",
+ dirent.Dir().Base() == "files",
isEmptyDir(dirent):
break
@@ -1453,7 +1453,7 @@ func (pkg *Package) checkIncludeConditio
if indentation.IsConditional() {
if other := pkg.unconditionalIncludes[key]; other != nil {
- if !pkg.Once.FirstTimeSlice("checkIncludeConditionally", mkline.Location.String(), other.Location.String()) {
+ if !pkg.Once.FirstTimeSlice("checkIncludeConditionally", mkline.String(), other.String()) {
return
}
@@ -1469,7 +1469,7 @@ func (pkg *Package) checkIncludeConditio
} else {
if other := pkg.conditionalIncludes[key]; other != nil {
- if !pkg.Once.FirstTimeSlice("checkIncludeConditionally", other.Location.String(), mkline.Location.String()) {
+ if !pkg.Once.FirstTimeSlice("checkIncludeConditionally", other.String(), mkline.String()) {
return
}
Index: pkgsrc/pkgtools/pkglint/files/patches_test.go
diff -u pkgsrc/pkgtools/pkglint/files/patches_test.go:1.35 pkgsrc/pkgtools/pkglint/files/patches_test.go:1.36
--- pkgsrc/pkgtools/pkglint/files/patches_test.go:1.35 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/patches_test.go Sat Feb 15 13:48:40 2020
@@ -56,8 +56,8 @@ func (s *Suite) Test_CheckLinesPatch__wi
CheckLinesPatch(patchLines, pkg)
t.CheckOutputLines(
- "AUTOFIX: patch-WithoutEmptyLines:2: Inserting a line \"\" before this line.",
- "AUTOFIX: patch-WithoutEmptyLines:3: Inserting a line \"\" before this line.",
+ "AUTOFIX: patch-WithoutEmptyLines:2: Inserting a line \"\" above this line.",
+ "AUTOFIX: patch-WithoutEmptyLines:3: Inserting a line \"\" above this line.",
"AUTOFIX: distinfo:3: Replacing \"49abd735b7e706ea9ed6671628bb54e91f7f5ffb\" "+
"with \"4938fc8c0b483dc2e33e741b0da883d199e78164\".")
@@ -98,7 +98,7 @@ func (s *Suite) Test_CheckLinesPatch__no
// the same. Outside of the testing environment, this duplicate
// diagnostic is suppressed; see LogVerbose.
t.CheckOutputLines(
- "NOTE: ~/patch-WithoutEmptyLines:2: Empty line expected before this line.",
+ "NOTE: ~/patch-WithoutEmptyLines:2: Empty line expected above this line.",
"ERROR: ~/patch-WithoutEmptyLines:2: Each patch must be documented.",
"NOTE: ~/patch-WithoutEmptyLines:2: Empty line expected.")
}
@@ -534,7 +534,7 @@ func (s *Suite) Test_PatchChecker_Check_
t.CheckOutputLines(
sprintf("ERROR: ~/patch-aa:1: Expected %q.", CvsID),
- "NOTE: ~/patch-aa:1: Empty line expected before this line.",
+ "NOTE: ~/patch-aa:1: Empty line expected above this line.",
"ERROR: ~/patch-aa: Contains no patch.")
}
Index: pkgsrc/pkgtools/pkglint/files/path.go
diff -u pkgsrc/pkgtools/pkglint/files/path.go:1.8 pkgsrc/pkgtools/pkglint/files/path.go:1.9
--- pkgsrc/pkgtools/pkglint/files/path.go:1.8 Sat Jan 18 21:56:09 2020
+++ pkgsrc/pkgtools/pkglint/files/path.go Sat Feb 15 13:48:40 2020
@@ -25,11 +25,9 @@ func (p Path) GoString() string { return
// which is usually a sign of an uninitialized variable.
func (p Path) IsEmpty() bool { return p == "" }
-func (p Path) DirClean() Path { return Path(path.Dir(string(p))) }
-
// Returns the directory of the path, with only minimal cleaning.
// Only redundant dots and slashes are removed, and only at the end.
-func (p Path) DirNoClean() Path {
+func (p Path) Dir() Path {
s := p.String()
end := len(s)
for end > 0 && s[end-1] != '/' {
@@ -232,12 +230,8 @@ func (p CurrPath) AsPath() Path { return
func (p CurrPath) IsEmpty() bool { return p.AsPath().IsEmpty() }
-func (p CurrPath) DirClean() CurrPath {
- return CurrPath(p.AsPath().DirClean())
-}
-
-func (p CurrPath) DirNoClean() CurrPath {
- return CurrPath(p.AsPath().DirNoClean())
+func (p CurrPath) Dir() CurrPath {
+ return CurrPath(p.AsPath().Dir())
}
func (p CurrPath) Base() string { return p.AsPath().Base() }
@@ -378,12 +372,8 @@ func (p PkgsrcPath) AsPath() Path { retu
func (p PkgsrcPath) AsRelPath() RelPath { return RelPath(p) }
-func (p PkgsrcPath) DirClean() PkgsrcPath {
- return NewPkgsrcPath(p.AsPath().DirClean())
-}
-
-func (p PkgsrcPath) DirNoClean() PkgsrcPath {
- return NewPkgsrcPath(p.AsPath().DirNoClean())
+func (p PkgsrcPath) Dir() PkgsrcPath {
+ return NewPkgsrcPath(p.AsPath().Dir())
}
func (p PkgsrcPath) Base() string { return p.AsPath().Base() }
@@ -401,6 +391,10 @@ func (p PkgsrcPath) JoinNoClean(other Re
// PackagePath is a path relative to the package directory. It is used
// for the PATCHDIR and PKGDIR variables, as well as dependencies and
// conflicts on other packages.
+//
+// It can have two forms:
+// - patches (further down)
+// - ../../category/package/* (up to the pkgsrc root, then down again)
type PackagePath string
func NewPackagePath(p RelPath) PackagePath {
@@ -467,10 +461,8 @@ func (p RelPath) Split() (RelPath, strin
return NewRelPath(dir), base
}
-func (p RelPath) DirClean() RelPath { return RelPath(p.AsPath().DirClean()) }
-
-func (p RelPath) DirNoClean() RelPath {
- return RelPath(p.AsPath().DirNoClean())
+func (p RelPath) Dir() RelPath {
+ return RelPath(p.AsPath().Dir())
}
func (p RelPath) Base() string { return p.AsPath().Base() }
Index: pkgsrc/pkgtools/pkglint/files/path_test.go
diff -u pkgsrc/pkgtools/pkglint/files/path_test.go:1.9 pkgsrc/pkgtools/pkglint/files/path_test.go:1.10
--- pkgsrc/pkgtools/pkglint/files/path_test.go:1.9 Sat Jan 18 21:56:09 2020
+++ pkgsrc/pkgtools/pkglint/files/path_test.go Sat Feb 15 13:48:40 2020
@@ -57,28 +57,11 @@ func (s *Suite) Test_Path_IsEmpty(c *che
test("/", false)
}
-func (s *Suite) Test_Path_DirClean(c *check.C) {
+func (s *Suite) Test_Path_Dir(c *check.C) {
t := s.Init(c)
test := func(p, dir Path) {
- t.CheckEquals(p.DirClean(), dir)
- }
-
- test("", ".")
- test("././././", ".")
- test("/root", "/")
- test("filename", ".")
- test("dir/filename", "dir")
- test("dir/filename\\with\\backslash", "dir")
-
- test("././././dir/filename", "dir")
-}
-
-func (s *Suite) Test_Path_DirNoClean(c *check.C) {
- t := s.Init(c)
-
- test := func(p, dir Path) {
- t.CheckEquals(p.DirNoClean(), dir)
+ t.CheckEquals(p.Dir(), dir)
}
test("", ".")
@@ -617,21 +600,11 @@ func (s *Suite) Test_CurrPath_IsEmpty(c
test("/", false)
}
-func (s *Suite) Test_CurrPath_DirClean(c *check.C) {
+func (s *Suite) Test_CurrPath_Dir(c *check.C) {
t := s.Init(c)
test := func(curr, dir CurrPath) {
- t.CheckEquals(curr.DirClean(), dir)
- }
-
- test("./dir/../dir///./file", "dir")
-}
-
-func (s *Suite) Test_CurrPath_DirNoClean(c *check.C) {
- t := s.Init(c)
-
- test := func(curr, dir CurrPath) {
- t.CheckEquals(curr.DirNoClean(), dir)
+ t.CheckEquals(curr.Dir(), dir)
}
test("./dir/../dir///./file", "./dir/../dir")
@@ -1116,21 +1089,11 @@ func (s *Suite) Test_PkgsrcPath_AsRelPat
t.CheckEquals(rel.String(), "./category/package/Makefile")
}
-func (s *Suite) Test_PkgsrcPath_DirClean(c *check.C) {
+func (s *Suite) Test_PkgsrcPath_Dir(c *check.C) {
t := s.Init(c)
test := func(pp, cleaned PkgsrcPath) {
- t.CheckEquals(pp.DirClean(), cleaned)
- }
-
- test("./dir/../dir/base///.", "dir/base")
-}
-
-func (s *Suite) Test_PkgsrcPath_DirNoClean(c *check.C) {
- t := s.Init(c)
-
- test := func(pp, cleaned PkgsrcPath) {
- t.CheckEquals(pp.DirNoClean(), cleaned)
+ t.CheckEquals(pp.Dir(), cleaned)
}
test("./dir/../dir/base///.", "./dir/../dir/base")
@@ -1369,21 +1332,11 @@ func (s *Suite) Test_RelPath_Split(c *ch
}
-func (s *Suite) Test_RelPath_DirClean(c *check.C) {
- t := s.Init(c)
-
- test := func(rel RelPath, dir RelPath) {
- t.CheckEquals(rel.DirClean(), dir)
- }
-
- test("./dir/../dir///./file", "dir")
-}
-
-func (s *Suite) Test_RelPath_DirNoClean(c *check.C) {
+func (s *Suite) Test_RelPath_Dir(c *check.C) {
t := s.Init(c)
test := func(rel RelPath, dir RelPath) {
- t.CheckEquals(rel.DirNoClean(), dir)
+ t.CheckEquals(rel.Dir(), dir)
}
test("./dir/../dir///./file", "./dir/../dir")
Index: pkgsrc/pkgtools/pkglint/files/pkglint.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint.go:1.73 pkgsrc/pkgtools/pkglint/files/pkglint.go:1.74
--- pkgsrc/pkgtools/pkglint/files/pkglint.go:1.73 Sun Jan 26 17:12:36 2020
+++ pkgsrc/pkgtools/pkglint/files/pkglint.go Sat Feb 15 13:48:40 2020
@@ -20,8 +20,19 @@ const confVersion = "@VERSION@"
// Pkglint is a container for all global variables of this Go package.
type Pkglint struct {
- Opts CmdOpts // Command line options.
- Pkgsrc Pkgsrc // Global data, mostly extracted from mk/*.
+ CheckGlobal bool
+
+ WarnExtra,
+ WarnPerm,
+ WarnQuoting bool
+
+ Profiling,
+ DumpMakefile,
+ Import,
+ Network,
+ Recursive bool
+
+ Pkgsrc Pkgsrc // Global data, mostly extracted from mk/*.
Todo CurrPathQueue // The files or directories that still need to be checked.
@@ -67,24 +78,6 @@ func NewPkglint(stdout io.Writer, stderr
// This is to ensure that tests are properly initialized and shut down.
func unusablePkglint() Pkglint { return Pkglint{} }
-type CmdOpts struct {
- CheckGlobal bool
-
- WarnExtra,
- WarnPerm,
- WarnQuoting bool
-
- Profiling,
- ShowHelp,
- DumpMakefile,
- Import,
- Network,
- Recursive,
- ShowVersion bool
-
- args []string
-}
-
type Hash struct {
hash []byte
location Location
@@ -107,7 +100,7 @@ var (
// One of these options is trace.Tracing, which is connected to --debug.
//
// It also discards the -Wall option that is used by default in other tests.
-func (pkglint *Pkglint) Main(stdout io.Writer, stderr io.Writer, args []string) (exitCode int) {
+func (p *Pkglint) Main(stdout io.Writer, stderr io.Writer, args []string) (exitCode int) {
G.Logger.out = NewSeparatorWriter(stdout)
G.Logger.err = NewSeparatorWriter(stderr)
trace.Out = stdout
@@ -119,30 +112,30 @@ func (pkglint *Pkglint) Main(stdout io.W
}
}()
- if exitcode := pkglint.ParseCommandLine(args); exitcode != -1 {
+ if exitcode := p.ParseCommandLine(args); exitcode != -1 {
return exitcode
}
- if pkglint.Opts.Profiling {
- defer pkglint.setUpProfiling()()
+ if p.Profiling {
+ defer p.setUpProfiling()()
}
- pkglint.prepareMainLoop()
+ p.prepareMainLoop()
- for !pkglint.Todo.IsEmpty() {
- pkglint.Check(pkglint.Todo.Pop())
+ for !p.Todo.IsEmpty() {
+ p.Check(p.Todo.Pop())
}
- pkglint.Pkgsrc.checkToplevelUnusedLicenses()
+ p.Pkgsrc.checkToplevelUnusedLicenses()
- pkglint.Logger.ShowSummary(args)
- if pkglint.Logger.errors != 0 {
+ p.Logger.ShowSummary(args)
+ if p.Logger.errors != 0 {
return 1
}
return 0
}
-func (pkglint *Pkglint) setUpProfiling() func() {
+func (p *Pkglint) setUpProfiling() func() {
var cleanups []func()
atExit := func(cleanup func()) {
@@ -150,8 +143,8 @@ func (pkglint *Pkglint) setUpProfiling()
}
atExit(func() {
- pkglint.fileCache.table = nil
- pkglint.fileCache.mapping = nil
+ p.fileCache.table = nil
+ p.fileCache.mapping = nil
runtime.GC()
fd, err := os.Create("pkglint.heapdump")
@@ -168,7 +161,7 @@ func (pkglint *Pkglint) setUpProfiling()
f, err := os.Create("pkglint.pprof")
if err != nil {
- pkglint.Logger.TechErrorf("pkglint.pprof", "Cannot create profiling file: %s", err)
+ p.Logger.TechErrorf("pkglint.pprof", "Cannot create profiling file: %s", err)
panic(pkglintFatal{})
}
atExit(func() { assertNil(f.Close(), "") })
@@ -177,15 +170,15 @@ func (pkglint *Pkglint) setUpProfiling()
assertNil(err, "Cannot start profiling")
atExit(pprof.StopCPUProfile)
- pkglint.res.Profiling()
- pkglint.Logger.histo = histogram.New()
- pkglint.loaded = histogram.New()
+ p.res.Profiling()
+ p.Logger.histo = histogram.New()
+ p.loaded = histogram.New()
atExit(func() {
- pkglint.Logger.out.Write("")
- pkglint.Logger.histo.PrintStats(pkglint.Logger.out.out, "loghisto", -1)
- pkglint.res.PrintStats(pkglint.Logger.out.out)
- pkglint.loaded.PrintStats(pkglint.Logger.out.out, "loaded", 10)
- pkglint.Logger.out.WriteLine(sprintf("fileCache: %d hits, %d misses", pkglint.fileCache.hits, pkglint.fileCache.misses))
+ p.Logger.out.Write("")
+ p.Logger.histo.PrintStats(p.Logger.out.out, "loghisto", -1)
+ p.res.PrintStats(p.Logger.out.out)
+ p.loaded.PrintStats(p.Logger.out.out, "loaded", 10)
+ p.Logger.out.WriteLine(sprintf("fileCache: %d hits, %d misses", p.fileCache.hits, p.fileCache.misses))
})
return func() {
@@ -195,85 +188,86 @@ func (pkglint *Pkglint) setUpProfiling()
}
}
-func (pkglint *Pkglint) prepareMainLoop() {
- firstDir := pkglint.Todo.Front()
+func (p *Pkglint) prepareMainLoop() {
+ firstDir := p.Todo.Front()
if firstDir.IsFile() {
- firstDir = firstDir.DirNoClean()
+ firstDir = firstDir.Dir()
}
- relTopdir := pkglint.findPkgsrcTopdir(firstDir)
+ relTopdir := p.findPkgsrcTopdir(firstDir)
if relTopdir.IsEmpty() {
// 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.
- NewLineWhole(firstDir).Fatalf("Must be inside a pkgsrc tree.")
+ // Since virtually every single check needs these files,
+ // the only sensible thing to do is to quit immediately.
+ G.Logger.TechFatalf(firstDir, "Must be inside a pkgsrc tree.")
}
- pkglint.Pkgsrc = NewPkgsrc(firstDir.JoinNoClean(relTopdir))
- pkglint.Wip = pkglint.Pkgsrc.IsWip(firstDir) // See Pkglint.checkMode.
- pkglint.Pkgsrc.LoadInfrastructure()
+ p.Pkgsrc = NewPkgsrc(firstDir.JoinNoClean(relTopdir))
+ p.Wip = p.Pkgsrc.IsWip(firstDir) // See Pkglint.checkMode.
+ p.Pkgsrc.LoadInfrastructure()
currentUser, err := user.Current()
assertNil(err, "user.Current")
// On Windows, this is `Computername\Username`.
- pkglint.Username = replaceAll(currentUser.Username, `^.*\\`, "")
+ p.Username = replaceAll(currentUser.Username, `^.*\\`, "")
}
-func (pkglint *Pkglint) ParseCommandLine(args []string) int {
- gopts := &pkglint.Opts
- lopts := &pkglint.Logger.Opts
+func (p *Pkglint) ParseCommandLine(args []string) int {
+ lopts := &p.Logger.Opts
opts := getopt.NewOptions()
+ var showHelp bool
+ var showVersion bool
+
check := opts.AddFlagGroup('C', "check", "check,...", "enable or disable specific checks")
opts.AddFlagVar('d', "debug", &trace.Tracing, false, "log verbose call traces for debugging")
opts.AddFlagVar('e', "explain", &lopts.Explain, false, "explain the diagnostics or give further help")
opts.AddFlagVar('f', "show-autofix", &lopts.ShowAutofix, false, "show what pkglint can fix automatically")
opts.AddFlagVar('F', "autofix", &lopts.Autofix, false, "try to automatically fix some errors")
opts.AddFlagVar('g', "gcc-output-format", &lopts.GccOutput, false, "mimic the gcc output format")
- opts.AddFlagVar('h', "help", &gopts.ShowHelp, false, "show a detailed usage message")
- opts.AddFlagVar('I', "dumpmakefile", &gopts.DumpMakefile, false, "dump the Makefile after parsing")
- opts.AddFlagVar('i', "import", &gopts.Import, false, "prepare the import of a wip package")
- opts.AddFlagVar('n', "network", &gopts.Network, false, "enable checks that need network access")
+ opts.AddFlagVar('h', "help", &showHelp, false, "show a detailed usage message")
+ opts.AddFlagVar('I', "dumpmakefile", &p.DumpMakefile, false, "dump the Makefile after parsing")
+ opts.AddFlagVar('i', "import", &p.Import, false, "prepare the import of a wip package")
+ opts.AddFlagVar('n', "network", &p.Network, false, "enable checks that need network access")
opts.AddStrList('o', "only", &lopts.Only, "only log diagnostics containing the given text")
- opts.AddFlagVar('p', "profiling", &gopts.Profiling, false, "profile the executing program")
+ opts.AddFlagVar('p', "profiling", &p.Profiling, false, "profile the executing program")
opts.AddFlagVar('q', "quiet", &lopts.Quiet, false, "don't show a summary line when finishing")
- opts.AddFlagVar('r', "recursive", &gopts.Recursive, false, "check subdirectories, too")
+ opts.AddFlagVar('r', "recursive", &p.Recursive, false, "check subdirectories, too")
opts.AddFlagVar('s', "source", &lopts.ShowSource, false, "show the source lines together with diagnostics")
- opts.AddFlagVar('V', "version", &gopts.ShowVersion, false, "show the version number of pkglint")
+ opts.AddFlagVar('V', "version", &showVersion, false, "show the version number of pkglint")
warn := opts.AddFlagGroup('W', "warning", "warning,...", "enable or disable groups of warnings")
- check.AddFlagVar("global", &gopts.CheckGlobal, false, "inter-package checks")
+ check.AddFlagVar("global", &p.CheckGlobal, false, "inter-package checks")
- warn.AddFlagVar("extra", &gopts.WarnExtra, false, "enable some extra warnings")
- warn.AddFlagVar("perm", &gopts.WarnPerm, false, "warn about unforeseen variable definition and use")
- warn.AddFlagVar("quoting", &gopts.WarnQuoting, false, "warn about quoting issues")
+ warn.AddFlagVar("extra", &p.WarnExtra, false, "enable some extra warnings")
+ warn.AddFlagVar("perm", &p.WarnPerm, false, "warn about unforeseen variable definition and use")
+ warn.AddFlagVar("quoting", &p.WarnQuoting, false, "warn about quoting issues")
remainingArgs, err := opts.Parse(args)
if err != nil {
- errOut := pkglint.Logger.err.out
+ errOut := p.Logger.err.out
_, _ = fmt.Fprintln(errOut, err)
_, _ = fmt.Fprintln(errOut, "")
opts.Help(errOut, "pkglint [options] dir...")
return 1
}
- gopts.args = remainingArgs
- if gopts.ShowHelp {
- opts.Help(pkglint.Logger.out.out, "pkglint [options] dir...")
+ if showHelp {
+ opts.Help(p.Logger.out.out, "pkglint [options] dir...")
return 0
}
- if pkglint.Opts.ShowVersion {
- _, _ = fmt.Fprintf(pkglint.Logger.out.out, "%s\n", confVersion)
+ if showVersion {
+ _, _ = fmt.Fprintf(p.Logger.out.out, "%s\n", confVersion)
return 0
}
- for _, arg := range pkglint.Opts.args {
- pkglint.Todo.Push(NewCurrPathSlash(arg))
+ for _, arg := range remainingArgs {
+ p.Todo.Push(NewCurrPathSlash(arg))
}
- if pkglint.Todo.IsEmpty() {
- pkglint.Todo.Push(".")
+ if p.Todo.IsEmpty() {
+ p.Todo.Push(".")
}
return -1
@@ -286,7 +280,7 @@ func (pkglint *Pkglint) ParseCommandLine
//
// It sets up all the global state (infrastructure, wip) for accurately
// classifying the entry.
-func (pkglint *Pkglint) Check(dirent CurrPath) {
+func (p *Pkglint) Check(dirent CurrPath) {
if trace.Tracing {
defer trace.Call(dirent)()
}
@@ -297,10 +291,10 @@ func (pkglint *Pkglint) Check(dirent Cur
return
}
- pkglint.checkMode(dirent, st.Mode())
+ p.checkMode(dirent, st.Mode())
}
-func (pkglint *Pkglint) checkMode(dirent CurrPath, mode os.FileMode) {
+func (p *Pkglint) checkMode(dirent CurrPath, mode os.FileMode) {
// TODO: merge duplicate code in Package.checkDirent
isDir := mode.IsDir()
isReg := mode.IsRegular()
@@ -311,15 +305,15 @@ func (pkglint *Pkglint) checkMode(dirent
dir := dirent
if !isDir {
- dir = dirent.DirNoClean()
+ dir = dirent.Dir()
}
basename := dirent.Base()
- pkgsrcRel := pkglint.Pkgsrc.Rel(dirent)
+ pkgsrcRel := p.Pkgsrc.Rel(dirent)
- pkglint.Wip = pkgsrcRel.HasPrefixPath("wip")
- pkglint.Infrastructure = pkgsrcRel.HasPrefixPath("mk")
- pkgsrcdir := pkglint.findPkgsrcTopdir(dir)
+ p.Wip = pkgsrcRel.HasPrefixPath("wip")
+ p.Infrastructure = pkgsrcRel.HasPrefixPath("mk")
+ pkgsrcdir := p.findPkgsrcTopdir(dir)
if pkgsrcdir.IsEmpty() {
G.Logger.TechErrorf("",
"Cannot determine the pkgsrc root directory for %q.",
@@ -328,8 +322,8 @@ func (pkglint *Pkglint) checkMode(dirent
}
if isReg {
- pkglint.checkExecutable(dirent, mode)
- pkglint.checkReg(dirent, basename, pkgsrcRel.Count(), nil)
+ p.checkExecutable(dirent, mode)
+ p.checkReg(dirent, basename, pkgsrcRel.Count(), nil)
return
}
@@ -339,7 +333,7 @@ func (pkglint *Pkglint) checkMode(dirent
switch pkgsrcdir {
case "../..":
- pkglint.checkdirPackage(dir)
+ p.checkdirPackage(dir)
case "..":
CheckdirCategory(dir)
case ".":
@@ -350,8 +344,8 @@ func (pkglint *Pkglint) checkMode(dirent
}
// checkdirPackage checks a complete pkgsrc package, including each
-// of the files individually, and also when seen in combination.
-func (pkglint *Pkglint) checkdirPackage(dir CurrPath) {
+// of the files individually, and when seen in combination.
+func (p *Pkglint) checkdirPackage(dir CurrPath) {
if trace.Tracing {
defer trace.Call(dir)()
}
@@ -477,7 +471,7 @@ func CheckLinesMessage(lines *Lines, pkg
// For now, skip all checks when the MESSAGE may be built from multiple
// files.
//
- // If the need arises, some of the checks may be activated again, but
+ // If the need arises, some of these checks may be activated again, but
// that requires more sophisticated code.
if pkg != nil && pkg.vars.IsDefined("MESSAGE_SRC") {
return
@@ -503,7 +497,7 @@ func CheckLinesMessage(lines *Lines, pkg
fix := line.Autofix()
fix.Warnf("Expected a line of exactly 75 \"=\" characters.")
fix.Explain(explanation()...)
- fix.InsertBefore(hline)
+ fix.InsertAbove(hline)
fix.Apply()
lines.CheckCvsID(0, ``, "")
} else {
@@ -519,7 +513,7 @@ func CheckLinesMessage(lines *Lines, pkg
fix := lastLine.Autofix()
fix.Warnf("Expected a line of exactly 75 \"=\" characters.")
fix.Explain(explanation()...)
- fix.InsertAfter(hline)
+ fix.InsertBelow(hline)
fix.Apply()
}
CheckLinesTrailingEmptyLines(lines)
@@ -548,9 +542,9 @@ func CheckFileMk(filename CurrPath, pkg
// checkReg checks the given regular file.
// depth is 3 for files in the package directory, and 4 or more for files
// deeper in the directory hierarchy, such as in files/ or patches/.
-func (pkglint *Pkglint) checkReg(filename CurrPath, basename string, depth int, pkg *Package) {
+func (p *Pkglint) checkReg(filename CurrPath, basename string, depth int, pkg *Package) {
- if depth == 3 && !pkglint.Wip {
+ if depth == 3 && !p.Wip {
if contains(basename, "TODO") {
NewLineWhole(filename).Errorf("Packages in main pkgsrc must not have a %s file.", basename)
// TODO: Add a convincing explanation.
@@ -563,13 +557,13 @@ func (pkglint *Pkglint) checkReg(filenam
hasSuffix(basename, ".orig"),
hasSuffix(basename, ".rej"),
contains(basename, "TODO") && depth == 3:
- if pkglint.Opts.Import {
+ if p.Import {
NewLineWhole(filename).Errorf("Must be cleaned up before committing the package.")
}
return
}
- pkglint.checkRegCvsSubst(filename)
+ p.checkRegCvsSubst(filename)
switch {
case basename == "ALTERNATIVES":
@@ -608,12 +602,12 @@ func (pkglint *Pkglint) checkReg(filenam
CheckLinesPatch(lines, pkg)
}
- case filename.DirNoClean().Base() == "patches" && matches(filename.Base(), `^manual[^/]*$`):
+ case filename.Dir().Base() == "patches" && matches(filename.Base(), `^manual[^/]*$`):
if trace.Tracing {
trace.Stepf("Unchecked file %q.", filename)
}
- case filename.DirNoClean().Base() == "patches":
+ case filename.Dir().Base() == "patches":
NewLineWhole(filename).Warnf("Patch files should be named \"patch-\", followed by letters, '-', '_', '.', and digits only.")
case (hasPrefix(basename, "Makefile") || hasSuffix(basename, ".mk")) &&
@@ -630,13 +624,13 @@ func (pkglint *Pkglint) checkReg(filenam
case hasPrefix(basename, "CHANGES-"):
// This only checks the file but doesn't register the changes globally.
- _ = pkglint.Pkgsrc.loadDocChangesFromFile(filename)
+ _ = p.Pkgsrc.loadDocChangesFromFile(filename)
- case filename.DirNoClean().Base() == "files":
+ case filename.Dir().Base() == "files":
// Skip files directly in the files/ directory, but not those further down.
case basename == "spec":
- if !pkglint.Pkgsrc.Rel(filename).HasPrefixPath("regress") {
+ if !p.Pkgsrc.Rel(filename).HasPrefixPath("regress") {
NewLineWhole(filename).Warnf("Only packages in regress/ may have spec files.")
}
@@ -648,7 +642,7 @@ func (pkglint *Pkglint) checkReg(filenam
}
}
-func (pkglint *Pkglint) checkRegCvsSubst(filename CurrPath) {
+func (p *Pkglint) checkRegCvsSubst(filename CurrPath) {
entries := G.loadCvsEntries(filename)
entry, found := entries[filename.Base()]
if !found || entry.Options == "" {
@@ -669,7 +663,7 @@ func (pkglint *Pkglint) checkRegCvsSubst
sprintf("To fix this, run \"cvs admin -kkv %s\"", shquote(filename.Base())))
}
-func (pkglint *Pkglint) checkExecutable(filename CurrPath, mode os.FileMode) {
+func (p *Pkglint) checkExecutable(filename CurrPath, mode os.FileMode) {
if mode.Perm()&0111 == 0 {
// Not executable at all.
return
@@ -719,8 +713,8 @@ func CheckLinesTrailingEmptyLines(lines
// The command can be "sed" or "gsed" or "${SED}".
// If a tool is returned, usable tells whether that tool has been added
// to USE_TOOLS in the current scope (file or package).
-func (pkglint *Pkglint) Tool(mklines *MkLines, command string, time ToolTime) (tool *Tool, usable bool) {
- tools := pkglint.tools(mklines)
+func (p *Pkglint) Tool(mklines *MkLines, command string, time ToolTime) (tool *Tool, usable bool) {
+ tools := p.tools(mklines)
if varUse := ToVarUse(command); varUse != nil {
tool = tools.ByVarname(varUse.varname)
@@ -737,22 +731,22 @@ func (pkglint *Pkglint) Tool(mklines *Mk
// It is not guaranteed to be usable (added to USE_TOOLS), only defined;
// that must be checked by the calling code,
// see Tool.UsableAtLoadTime and Tool.UsableAtRunTime.
-func (pkglint *Pkglint) ToolByVarname(mklines *MkLines, varname string) *Tool {
- return pkglint.tools(mklines).ByVarname(varname)
+func (p *Pkglint) ToolByVarname(mklines *MkLines, varname string) *Tool {
+ return p.tools(mklines).ByVarname(varname)
}
-func (pkglint *Pkglint) tools(mklines *MkLines) *Tools {
+func (p *Pkglint) tools(mklines *MkLines) *Tools {
if mklines != nil {
return mklines.Tools
} else {
- return pkglint.Pkgsrc.Tools
+ return p.Pkgsrc.Tools
}
}
-func (pkglint *Pkglint) loadCvsEntries(filename CurrPath) map[string]CvsEntry {
- dir := filename.DirClean()
- if dir == pkglint.cvsEntriesDir {
- return pkglint.cvsEntries
+func (p *Pkglint) loadCvsEntries(filename CurrPath) map[string]CvsEntry {
+ dir := filename.Dir().Clean()
+ if dir == p.cvsEntriesDir {
+ return p.cvsEntries
}
var entries map[string]CvsEntry
@@ -795,14 +789,14 @@ func (pkglint *Pkglint) loadCvsEntries(f
}
}
- pkglint.cvsEntriesDir = dir
- pkglint.cvsEntries = entries
+ p.cvsEntriesDir = dir
+ p.cvsEntries = entries
return entries
}
-func (pkglint *Pkglint) Abs(filename CurrPath) CurrPath {
+func (p *Pkglint) Abs(filename CurrPath) CurrPath {
if !filename.IsAbs() {
- return pkglint.cwd.JoinNoClean(NewRelPath(filename.AsPath())).Clean()
+ return p.cwd.JoinNoClean(NewRelPath(filename.AsPath())).Clean()
}
return filename.Clean()
}
Index: pkgsrc/pkgtools/pkglint/files/pkglint_test.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.59 pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.60
--- pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.59 Sun Jan 26 17:12:36 2020
+++ pkgsrc/pkgtools/pkglint/files/pkglint_test.go Sat Feb 15 13:48:40 2020
@@ -9,7 +9,7 @@ import (
"strings"
)
-func (pkglint *Pkglint) isUsable() bool { return pkglint.fileCache != nil }
+func (p *Pkglint) isUsable() bool { return p.fileCache != nil }
func (s *Suite) Test_Pkglint_Main(c *check.C) {
t := s.Init(c)
@@ -259,7 +259,7 @@ func (s *Suite) Test_Pkglint_Main__autof
exitcode := t.Main("-Wall", "--autofix", "filename.mk")
t.CheckOutputLines(
- "AUTOFIX: ~/filename.mk:1: Inserting a line \"" + MkCvsID + "\" before this line.")
+ "AUTOFIX: ~/filename.mk:1: Inserting a line \"" + MkCvsID + "\" above this line.")
t.CheckEquals(exitcode, 0)
}
@@ -872,10 +872,10 @@ func (s *Suite) Test_CheckLinesMessage__
t.CheckOutputLines(
"AUTOFIX: ~/MESSAGE:1: Inserting a line \"=============================="+
- "=============================================\" before this line.",
- "AUTOFIX: ~/MESSAGE:1: Inserting a line \"$"+"NetBSD$\" before this line.",
+ "=============================================\" above this line.",
+ "AUTOFIX: ~/MESSAGE:1: Inserting a line \"$"+"NetBSD$\" above this line.",
"AUTOFIX: ~/MESSAGE:5: Inserting a line \"=============================="+
- "=============================================\" after this line.")
+ "=============================================\" below this line.")
t.CheckFileLines("MESSAGE",
"===========================================================================",
CvsID,
Index: pkgsrc/pkgtools/pkglint/files/pkgsrc.go
diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.49 pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.50
--- pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.49 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/pkgsrc.go Sat Feb 15 13:48:40 2020
@@ -151,7 +151,7 @@ func (src *Pkgsrc) loadDocChanges() {
docDir := src.File("doc")
files := src.ReadDir("doc")
if len(files) == 0 {
- NewLineWhole(docDir).Fatalf("Cannot be read for loading the package changes.")
+ G.Logger.TechFatalf(docDir, "Cannot be read for loading the package changes.")
}
var filenames []RelPath
@@ -179,7 +179,7 @@ func (src *Pkgsrc) loadDocChanges() {
func (src *Pkgsrc) loadDocChangesFromFile(filename CurrPath) []*Change {
- warn := G.Opts.CheckGlobal && !G.Wip
+ warn := G.CheckGlobal && !G.Wip
// Each date in the file should be from the same year as the filename says.
// This check has been added in 2018.
@@ -330,7 +330,7 @@ func (*Pkgsrc) parseDocChange(line *Line
}
func (src *Pkgsrc) checkRemovedAfterLastFreeze() {
- if src.LastFreezeStart == "" || G.Wip || !G.Opts.CheckGlobal {
+ if src.LastFreezeStart == "" || G.Wip || !G.CheckGlobal {
return
}
@@ -347,10 +347,10 @@ func (src *Pkgsrc) checkRemovedAfterLast
sort.Slice(wrong, func(i, j int) bool { return wrong[i].IsAbove(wrong[j]) })
for _, change := range wrong {
- // It's a bit cheated to construct a Line from only a Location,
- // without the wrong text. That's only because I'm too lazy loading
- // the file again, and the original text is not lying around anywhere.
- line := NewLineMulti(change.Location.Filename, int(change.Location.firstLine), int(change.Location.lastLine), "", nil)
+ // The original line of the change is not available anymore.
+ // Therefore it is necessary to load the whole file again.
+ lines := Load(change.Location.Filename, MustSucceed)
+ line := lines.Lines[change.Location.lineno-1]
line.Errorf("Package %s must either exist or be marked as removed.", change.Pkgpath.String())
}
}
@@ -425,7 +425,7 @@ func (src *Pkgsrc) loadTools() {
}
}
if len(toolFiles) <= 1 {
- NewLineWhole(toc).Fatalf("Too few tool files.")
+ G.Logger.TechFatalf(toc, "Too few tool files.")
}
}
@@ -1120,6 +1120,16 @@ func (src *Pkgsrc) File(relativeName Pkg
return src.topdir.JoinNoClean(cleaned).CleanDot()
}
+// FilePkg resolves a package-relative path to the real file that it represents.
+// If the given path does not start with "../../", the result is empty.
+func (src *Pkgsrc) FilePkg(rel PackagePath) CurrPath {
+ parts := rel.AsPath().Parts()
+ if len(parts) >= 4 && parts[0] == ".." && parts[1] == ".." && parts[2] != ".." {
+ return src.File(NewPkgsrcPath(NewPath(strings.Join(parts[2:], "/"))))
+ }
+ return ""
+}
+
// Rel returns the path of `filename`, relative to the pkgsrc top directory.
//
// Example:
@@ -1177,7 +1187,7 @@ func (ch *Change) IsAbove(other *Change)
if ch.Date != other.Date {
return ch.Date < other.Date
}
- return ch.Location.firstLine < other.Location.firstLine
+ return ch.Location.lineno < other.Location.lineno
}
type ChangeAction uint8
Index: pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go
diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.42 pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.43
--- pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.42 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go Sat Feb 15 13:48:40 2020
@@ -510,18 +510,20 @@ func (s *Suite) Test_Pkgsrc_checkRemoved
// And for finding the removal reliably, it doesn't matter how long ago
// the last package change was.
- // The empty lines in the following output demonstrate the cheating
- // by creating fake lines from Change.Location.
t.CheckOutputLines(
+ ">\t\tUpdated category/updated-before to 1.0 [updater 2019-04-01]",
"ERROR: ~/doc/CHANGES-2019:3: Package category/updated-before "+
"must either exist or be marked as removed.",
"",
+ ">\t\tUpdated category/updated-after to 1.0 [updater 2019-07-01]",
"ERROR: ~/doc/CHANGES-2019:6: Package category/updated-after "+
"must either exist or be marked as removed.",
"",
+ ">\t\tAdded category/added-after version 1.0 [updater 2019-07-01]",
"ERROR: ~/doc/CHANGES-2019:7: Package category/added-after "+
"must either exist or be marked as removed.",
"",
+ ">\t\tDowngraded category/downgraded to 1.0 [author 2019-07-03]",
"ERROR: ~/doc/CHANGES-2019:9: Package category/downgraded "+
"must either exist or be marked as removed.")
}
@@ -1408,7 +1410,7 @@ func (s *Suite) Test_Pkgsrc_File(c *chec
func (s *Suite) Test_Change_Version(c *check.C) {
t := s.Init(c)
- loc := Location{"doc/CHANGES-2019", 5, 5}
+ loc := Location{"doc/CHANGES-2019", 5}
added := Change{loc, Added, "category/path", "1.0", "author", "2019-01-01"}
updated := Change{loc, Updated, "category/path", "1.0", "author", "2019-01-01"}
downgraded := Change{loc, Downgraded, "category/path", "1.0", "author", "2019-01-01"}
@@ -1423,7 +1425,7 @@ func (s *Suite) Test_Change_Version(c *c
func (s *Suite) Test_Change_Target(c *check.C) {
t := s.Init(c)
- loc := Location{"doc/CHANGES-2019", 5, 5}
+ loc := Location{"doc/CHANGES-2019", 5}
renamed := Change{loc, Renamed, "category/path", "category/other", "author", "2019-01-01"}
moved := Change{loc, Moved, "category/path", "category/other", "author", "2019-01-01"}
downgraded := Change{loc, Downgraded, "category/path", "1.0", "author", "2019-01-01"}
@@ -1436,7 +1438,7 @@ func (s *Suite) Test_Change_Target(c *ch
func (s *Suite) Test_Change_Successor(c *check.C) {
t := s.Init(c)
- loc := Location{"doc/CHANGES-2019", 5, 5}
+ loc := Location{"doc/CHANGES-2019", 5}
removed := Change{loc, Removed, "category/path", "", "author", "2019-01-01"}
removedSucc := Change{loc, Removed, "category/path", "category/successor", "author", "2019-01-01"}
downgraded := Change{loc, Downgraded, "category/path", "1.0", "author", "2019-01-01"}
@@ -1450,9 +1452,9 @@ func (s *Suite) Test_Change_IsAbove(c *c
t := s.Init(c)
var changes = []*Change{
- {Location{"", 1, 1}, 0, "", "", "", "2011-07-01"},
- {Location{"", 2, 2}, 0, "", "", "", "2011-07-01"},
- {Location{"", 1, 1}, 0, "", "", "", "2011-07-02"}}
+ {Location{"", 1}, 0, "", "", "", "2011-07-01"},
+ {Location{"", 2}, 0, "", "", "", "2011-07-01"},
+ {Location{"", 1}, 0, "", "", "", "2011-07-02"}}
test := func(i int, chi *Change, j int, chj *Change) {
actual := chi.IsAbove(chj)
Index: pkgsrc/pkgtools/pkglint/files/plist.go
diff -u pkgsrc/pkgtools/pkglint/files/plist.go:1.52 pkgsrc/pkgtools/pkglint/files/plist.go:1.53
--- pkgsrc/pkgtools/pkglint/files/plist.go:1.52 Mon Jan 6 20:38:42 2020
+++ pkgsrc/pkgtools/pkglint/files/plist.go Sat Feb 15 13:48:40 2020
@@ -126,7 +126,7 @@ func (ck *PlistChecker) collectPath(rel
if prev := ck.allFiles[rel]; prev == nil || stringSliceLess(pline.conditions, prev.conditions) {
ck.allFiles[rel] = pline
}
- for dir := rel.DirNoClean(); dir != "."; dir = dir.DirNoClean() {
+ for dir := rel.Dir(); dir != "."; dir = dir.Dir() {
ck.allDirs[dir] = pline
}
}
@@ -136,7 +136,7 @@ func (ck *PlistChecker) collectDirective
if !m || NewPath(dirname).IsAbs() {
return
}
- for dir := NewRelPathString(dirname); dir != "."; dir = dir.DirNoClean() {
+ for dir := NewRelPathString(dirname); dir != "."; dir = dir.Dir() {
ck.allDirs[dir] = pline
}
}
@@ -155,7 +155,7 @@ func (ck *PlistChecker) checkLine(pline
} else if m, cmd, arg := match2(text, `^@([a-z-]+)[\t ]*(.*)`); m {
pline.CheckDirective(cmd, arg)
- if cmd == "comment" && pline.firstLine > 1 {
+ if cmd == "comment" && pline.Location.lineno > 1 {
ck.nonAsciiAllowed = true
}
@@ -647,7 +647,7 @@ func (s *plistLineSorter) Sort() {
fix := firstLine.Autofix()
fix.Notef(SilentAutofixFormat)
- fix.Describef(int(firstLine.firstLine), "Sorting the whole file.")
+ fix.Describef(0, "Sorting the whole file.")
fix.Apply()
var lines []*Line
@@ -661,5 +661,5 @@ func (s *plistLineSorter) Sort() {
lines = append(lines, pline.Line)
}
- s.autofixed = SaveAutofixChanges(NewLines(lines[0].Filename, lines))
+ s.autofixed = SaveAutofixChanges(NewLines(lines[0].Filename(), lines))
}
Index: pkgsrc/pkgtools/pkglint/files/redundantscope_test.go
diff -u pkgsrc/pkgtools/pkglint/files/redundantscope_test.go:1.13 pkgsrc/pkgtools/pkglint/files/redundantscope_test.go:1.14
--- pkgsrc/pkgtools/pkglint/files/redundantscope_test.go:1.13 Wed Feb 5 04:09:00 2020
+++ pkgsrc/pkgtools/pkglint/files/redundantscope_test.go Sat Feb 15 13:48:40 2020
@@ -1445,7 +1445,7 @@ func (s *Suite) Test_RedundantScope__is_
scope := NewRedundantScope()
scope.IsRelevant = func(mkline *MkLine) bool {
// See checkfilePackageMakefile.
- return G.Opts.CheckGlobal || !G.Pkgsrc.IsInfra(mkline.Filename)
+ return G.CheckGlobal || !G.Pkgsrc.IsInfra(mkline.Filename())
}
scope.Check(mklines)
Index: pkgsrc/pkgtools/pkglint/files/shell.go
diff -u pkgsrc/pkgtools/pkglint/files/shell.go:1.57 pkgsrc/pkgtools/pkglint/files/shell.go:1.58
--- pkgsrc/pkgtools/pkglint/files/shell.go:1.57 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/shell.go Sat Feb 15 13:48:40 2020
@@ -60,7 +60,7 @@ func (scc *SimpleCommandChecker) checkCo
case matches(shellword, `\$\{(PKGSRCDIR|PREFIX)(:Q)?\}`):
break
default:
- if G.Opts.WarnExtra && !scc.mklines.indentation.DependsOn("OPSYS") {
+ if G.WarnExtra && !scc.mklines.indentation.DependsOn("OPSYS") {
scc.mkline.Warnf("Unknown shell command %q.", shellword)
scc.mkline.Explain(
"To make the package portable to all platforms that pkgsrc supports,",
@@ -577,7 +577,7 @@ func (ck *ShellLineChecker) checkPipeExi
return false, ""
}
- if G.Opts.WarnExtra && len(pipeline.Cmds) > 1 {
+ if G.WarnExtra && len(pipeline.Cmds) > 1 {
if canFail, cmd := canFail(); canFail {
if cmd != "" {
ck.Warnf("The exitcode of %q at the left of the | operator is ignored.", cmd)
@@ -744,7 +744,7 @@ func (ck *ShellLineChecker) CheckShellCo
}
}
walker.Callback.AndOr = func(andor *MkShAndOr) {
- if G.Opts.WarnExtra && !*pSetE && walker.Current().Index != 0 {
+ if G.WarnExtra && !*pSetE && walker.Current().Index != 0 {
ck.checkSetE(walker.Parent(1).(*MkShList), walker.Current().Index)
}
}
@@ -922,7 +922,7 @@ func (ck *ShellLineChecker) checkShVarUs
if shVarname == "@" {
ck.Warnf("The $@ shell variable should only be used in double quotes.")
- } else if G.Opts.WarnQuoting && checkQuoting && ck.variableNeedsQuoting(shVarname) {
+ } else if G.WarnQuoting && checkQuoting && ck.variableNeedsQuoting(shVarname) {
ck.Warnf("Unquoted shell variable %q.", shVarname)
ck.Explain(
"When a shell variable contains whitespace, it is expanded (split into multiple words)",
@@ -1022,8 +1022,8 @@ func (ck *ShellLineChecker) checkMultiLi
return
}
- for _, line := range mkline.raw[:len(mkline.raw)-1] {
- text := strings.TrimSuffix(line.Text(), "\\")
+ for rawIndex, rawLine := range mkline.raw[:len(mkline.raw)-1] {
+ text := strings.TrimSuffix(mkline.RawText(rawIndex), "\\")
tokens, rest := splitIntoShellTokens(nil, text)
if rest != "" {
return
@@ -1031,18 +1031,23 @@ func (ck *ShellLineChecker) checkMultiLi
for _, token := range tokens {
if hasPrefix(token, "#") {
- ck.warnMultiLineComment(line)
+ ck.warnMultiLineComment(rawIndex, rawLine)
return
}
}
}
}
-func (ck *ShellLineChecker) warnMultiLineComment(raw *RawLine) {
- line := NewLine(ck.mkline.Filename, raw.Lineno, raw.Text(), raw)
+func (ck *ShellLineChecker) warnMultiLineComment(rawIndex int, raw *RawLine) {
+ line := ck.mkline.Line
+ singleLine := NewLine(
+ line.Filename(),
+ line.Location.Lineno(rawIndex),
+ line.RawText(rawIndex),
+ raw)
- line.Warnf("The shell comment does not stop at the end of this line.")
- line.Explain(
+ singleLine.Warnf("The shell comment does not stop at the end of this line.")
+ singleLine.Explain(
"When a shell command is spread out on multiple lines that are",
"continued with a backslash, they will nevertheless be converted to",
"a single line before the shell sees them.",
Index: pkgsrc/pkgtools/pkglint/files/toplevel.go
diff -u pkgsrc/pkgtools/pkglint/files/toplevel.go:1.28 pkgsrc/pkgtools/pkglint/files/toplevel.go:1.29
--- pkgsrc/pkgtools/pkglint/files/toplevel.go:1.28 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/toplevel.go Sat Feb 15 13:48:40 2020
@@ -27,10 +27,8 @@ func CheckdirToplevel(dir CurrPath) {
mklines.Check()
- if G.Opts.Recursive {
- if G.Opts.CheckGlobal {
- G.InterPackage.Enable()
- }
+ if G.Recursive {
+ G.InterPackage.Enable()
G.Todo.PushFront(ctx.subdirs...)
}
}
Index: pkgsrc/pkgtools/pkglint/files/toplevel_test.go
diff -u pkgsrc/pkgtools/pkglint/files/toplevel_test.go:1.22 pkgsrc/pkgtools/pkglint/files/toplevel_test.go:1.23
--- pkgsrc/pkgtools/pkglint/files/toplevel_test.go:1.22 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/toplevel_test.go Sat Feb 15 13:48:40 2020
@@ -59,7 +59,8 @@ func (s *Suite) Test_CheckdirToplevel__r
t.CheckOutputLines(
"WARN: ~/category/package/Makefile:20: UNKNOWN is defined but not used.",
- "1 warning found.",
+ "WARN: ~/licenses/gnu-gpl-v2: This license seems to be unused.",
+ "2 warnings found.",
t.Shquote("(Run \"pkglint -e -Wall -r %s\" to show explanations.)", "."))
}
Index: pkgsrc/pkgtools/pkglint/files/util.go
diff -u pkgsrc/pkgtools/pkglint/files/util.go:1.72 pkgsrc/pkgtools/pkglint/files/util.go:1.73
--- pkgsrc/pkgtools/pkglint/files/util.go:1.72 Sun Jan 26 17:12:36 2020
+++ pkgsrc/pkgtools/pkglint/files/util.go Sat Feb 15 13:48:40 2020
@@ -325,7 +325,7 @@ func isEmptyDir(filename CurrPath) bool
func getSubdirs(filename CurrPath) []RelPath {
dirents, err := filename.ReadDir()
if err != nil {
- NewLineWhole(filename).Fatalf("Cannot be read: %s", err)
+ G.Logger.TechFatalf(filename, "Cannot be read: %s", err)
}
var subdirs []RelPath
@@ -658,7 +658,7 @@ func (s *Scope) def(name string, mkline
// In most cases the defining lines are indeed variable assignments.
// Exceptions are comments from documentation sections, which still mark
- // it as defined so that it doesn't produce the "used but not defined" warning;
+ // the variable as defined so that it doesn't produce the "used but not defined" warning;
// see MkLines.collectDocumentedVariables.
if !mkline.IsVarassign() {
return
@@ -669,12 +669,13 @@ func (s *Scope) def(name string, mkline
value := mkline.Value()
if trace.Tracing {
trace.Stepf("Scope.Define.append %s: %s = %q + %q",
- &mkline.Location, name, s.value[name], value)
+ mkline.String(), name, s.value[name], value)
}
s.value[name] += " " + value
case opAssignDefault:
- // No change to the value.
- // FIXME: If there is no value yet, set it.
+ if _, set := s.value[name]; !set {
+ s.value[name] = mkline.Value()
+ }
case opAssignShell:
delete(s.value, name)
default:
@@ -831,7 +832,7 @@ func (s *Scope) FirstUse(varname string)
// LastValue returns the value from the last variable definition.
//
-// If an empty string is returned this can mean either that the
+// If an empty string is returned, this can mean either that the
// variable value is indeed the empty string or that the variable
// was not found, or that the variable value cannot be determined
// reliably. To distinguish these cases, call LastValueFound instead.
@@ -842,18 +843,10 @@ func (s *Scope) LastValue(varname string
func (s *Scope) LastValueFound(varname string) (value string, found bool) {
value, found = s.value[varname]
- if found {
- return
- }
-
- mkline := s.LastDefinition(varname)
- if mkline != nil && mkline.Op() != opAssignShell {
- return mkline.Value(), true
- }
- if fallback, ok := s.fallback[varname]; ok {
- return fallback, true
+ if !found {
+ value, found = s.fallback[varname]
}
- return "", false
+ return
}
func (s *Scope) DefineAll(other Scope) {
@@ -932,7 +925,7 @@ func naturalLess(str1, str2 string) bool
if nr1, nr2 := str1[nonZero1:idx1], str2[nonZero2:idx2]; nr1 != nr2 {
return nr1 < nr2
}
- // Otherwise, the one with less zeros is less.
+ // Otherwise, the one with fewer zeros is less.
// Because everything up to the number is equal, comparing the index
// after the zeros is sufficient.
if nonZero1 != nonZero2 {
@@ -1060,7 +1053,7 @@ func (c *FileCache) Get(filename CurrPat
lines := make([]*Line, entry.lines.Len())
for i, line := range entry.lines.Lines {
- lines[i] = NewLineMulti(filename, int(line.firstLine), int(line.lastLine), line.Text, line.raw)
+ lines[i] = NewLineMulti(filename, line.Location.lineno, line.Text, line.raw)
}
return NewLines(filename, lines)
}
@@ -1466,33 +1459,34 @@ func (i *optInt) set(value int) {
}
type bag struct {
- slice []struct {
- key interface{}
- value int
- }
+ // Wrapping the slice in an extra struct avoids 'receiver might be nil'
+ // warnings.
+
+ entries []bagEntry
}
-func (mi *bag) sortDesc() {
- less := func(i, j int) bool { return mi.slice[j].value < mi.slice[i].value }
- sort.SliceStable(mi.slice, less)
+func (b *bag) sortDesc() {
+ es := b.entries
+ less := func(i, j int) bool { return es[j].count < es[i].count }
+ sort.SliceStable(es, less)
}
-func (mi *bag) opt(index int) int {
- if uint(index) < uint(len(mi.slice)) {
- return mi.slice[index].value
+func (b *bag) opt(index int) int {
+ if uint(index) < uint(len(b.entries)) {
+ return b.entries[index].count
}
return 0
}
-func (mi *bag) key(index int) interface{} {
- return mi.slice[index].key
-}
+func (b *bag) key(index int) interface{} { return b.entries[index].key }
-func (mi *bag) add(key interface{}, value int) {
- mi.slice = append(mi.slice, struct {
- key interface{}
- value int
- }{key, value})
+func (b *bag) add(key interface{}, count int) {
+ b.entries = append(b.entries, bagEntry{key, count})
}
-func (mi *bag) len() int { return len(mi.slice) }
+func (b *bag) len() int { return len(b.entries) }
+
+type bagEntry struct {
+ key interface{}
+ count int
+}
Index: pkgsrc/pkgtools/pkglint/files/util_test.go
diff -u pkgsrc/pkgtools/pkglint/files/util_test.go:1.47 pkgsrc/pkgtools/pkglint/files/util_test.go:1.48
--- pkgsrc/pkgtools/pkglint/files/util_test.go:1.47 Mon Jan 6 20:38:42 2020
+++ pkgsrc/pkgtools/pkglint/files/util_test.go Sat Feb 15 13:48:40 2020
@@ -554,12 +554,14 @@ func (s *Suite) Test_Scope_Define(c *che
scope := NewScope()
test := func(line string, ok bool, value string) {
- scope.Define("BUILD_DIRS", t.NewMkLine("file.mk", 123, line))
+ mkline := t.NewMkLine("file.mk", 123, line)
+ scope.Define("BUILD_DIRS", mkline)
actualValue, actualFound := scope.LastValueFound("BUILD_DIRS")
t.CheckEquals(actualValue, value)
t.CheckEquals(actualFound, ok)
+ t.CheckEquals(scope.value["BUILD_DIRS"], value)
}
test("BUILD_DIRS?=\tdefault",
@@ -579,6 +581,11 @@ func (s *Suite) Test_Scope_Define(c *che
test("BUILD_DIRS!=\techo dynamic",
false, "")
+
+ // FIXME: This is not correct. The shell assignment sets the variable,
+ // after which all further default assignments are ignored.
+ test("BUILD_DIRS?=\tdefault after shell assignment",
+ true, "default after shell assignment")
}
func (s *Suite) Test_Scope_Mentioned(c *check.C) {
@@ -777,13 +784,13 @@ func (s *Suite) Test_FileCache(c *check.
linesFromCache := cache.Get("Makefile", 0)
t.CheckEquals(linesFromCache.Filename, NewCurrPath("Makefile"))
c.Check(linesFromCache.Lines, check.HasLen, 2)
- t.CheckEquals(linesFromCache.Lines[0].Filename, NewCurrPath("Makefile"))
+ t.CheckEquals(linesFromCache.Lines[0].Filename(), NewCurrPath("Makefile"))
// Cache keys are normalized using path.Clean.
linesFromCache2 := cache.Get("./Makefile", 0)
t.CheckEquals(linesFromCache2.Filename, NewCurrPath("./Makefile"))
c.Check(linesFromCache2.Lines, check.HasLen, 2)
- t.CheckEquals(linesFromCache2.Lines[0].Filename, NewCurrPath("./Makefile"))
+ t.CheckEquals(linesFromCache2.Lines[0].Filename(), NewCurrPath("./Makefile"))
cache.Put("file1.mk", 0, lines)
cache.Put("file2.mk", 0, lines)
Index: pkgsrc/pkgtools/pkglint/files/var.go
diff -u pkgsrc/pkgtools/pkglint/files/var.go:1.7 pkgsrc/pkgtools/pkglint/files/var.go:1.8
--- pkgsrc/pkgtools/pkglint/files/var.go:1.7 Sun Dec 8 00:06:38 2019
+++ pkgsrc/pkgtools/pkglint/files/var.go Sat Feb 15 13:48:40 2020
@@ -185,7 +185,7 @@ func (v *Var) Write(mkline *MkLine, cond
v.refs.AddAll(conditionVarnames)
v.update(mkline, &v.valueInfra)
- if !G.Pkgsrc.IsInfra(mkline.Line.Filename) {
+ if !G.Pkgsrc.IsInfra(mkline.Line.Filename()) {
v.update(mkline, &v.value)
}
Index: pkgsrc/pkgtools/pkglint/files/varalignblock.go
diff -u pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.16 pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.17
--- pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.16 Wed Feb 5 04:09:00 2020
+++ pkgsrc/pkgtools/pkglint/files/varalignblock.go Sat Feb 15 13:48:40 2020
@@ -156,7 +156,7 @@ func (va *VaralignBlock) Process(mkline
case mkline.IsComment(), mkline.IsDirective():
default:
- trace.Stepf("Skipping varalign block because of line %s", &mkline.Location)
+ trace.Stepf("Skipping varalign block because of line %s", mkline.String())
va.skip = true
}
}
@@ -184,9 +184,9 @@ func (va *VaralignBlock) processVarassig
}
var infos []*varalignLine
- for i, raw := range mkline.raw {
- parts := NewVaralignSplitter().split(raw.Text(), i == 0)
- info := varalignLine{mkline, i, false, parts}
+ for rawIndex := range mkline.raw {
+ parts := NewVaralignSplitter().split(mkline.RawText(rawIndex), rawIndex == 0)
+ info := varalignLine{mkline, rawIndex, false, parts}
infos = append(infos, &info)
}
va.mkinfos = append(va.mkinfos, &varalignMkLine{infos})
Index: pkgsrc/pkgtools/pkglint/files/vardefs.go
diff -u pkgsrc/pkgtools/pkglint/files/vardefs.go:1.88 pkgsrc/pkgtools/pkglint/files/vardefs.go:1.89
--- pkgsrc/pkgtools/pkglint/files/vardefs.go:1.88 Sat Jan 18 21:56:09 2020
+++ pkgsrc/pkgtools/pkglint/files/vardefs.go Sat Feb 15 13:48:40 2020
@@ -418,7 +418,8 @@ func (reg *VarTypeRegistry) enumFrom(
}
if !G.Testing {
- mklines.Whole().Fatalf(
+ G.Logger.TechFatalf(
+ mklines.lines.Filename,
"Must contain at least 1 variable definition for %s.",
joinSkipEmptyCambridge("or", varcanons...))
}
@@ -442,7 +443,8 @@ func (reg *VarTypeRegistry) enumFromDirs
versions := src.ListVersions(category, re, repl, false)
if len(versions) == 0 {
if !G.Testing {
- NewLineWhole(src.File(category)).Fatalf(
+ G.Logger.TechFatalf(
+ src.File(category),
"Must contain at least 1 subdirectory matching %q.", re)
}
return enum(defval)
@@ -469,7 +471,8 @@ func (reg *VarTypeRegistry) enumFromFile
}
if len(relevant) == 0 {
if !G.Testing {
- NewLineWhole(src.File(basedir)).Fatalf(
+ G.Logger.TechFatalf(
+ src.File(basedir),
"Must contain at least 1 file matching %q.", re)
}
return enum(defval)
Index: pkgsrc/pkgtools/pkglint/files/vartype.go
diff -u pkgsrc/pkgtools/pkglint/files/vartype.go:1.46 pkgsrc/pkgtools/pkglint/files/vartype.go:1.47
--- pkgsrc/pkgtools/pkglint/files/vartype.go:1.46 Sat Jan 18 21:56:09 2020
+++ pkgsrc/pkgtools/pkglint/files/vartype.go Sat Feb 15 13:48:40 2020
@@ -435,6 +435,7 @@ var (
BtPathlist = &BasicType{"Pathlist", (*VartypeCheck).Pathlist}
BtPathPattern = &BasicType{"PathPattern", (*VartypeCheck).PathPattern}
BtPathname = &BasicType{"Pathname", (*VartypeCheck).Pathname}
+ BtPathnameSpace = &BasicType{"PathnameSpace", (*VartypeCheck).PathnameSpace}
BtPerl5Packlist = &BasicType{"Perl5Packlist", (*VartypeCheck).Perl5Packlist}
BtPerms = &BasicType{"Perms", (*VartypeCheck).Perms}
BtPkgname = &BasicType{"Pkgname", (*VartypeCheck).Pkgname}
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.81 pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.82
--- pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.81 Sun Jan 26 17:12:36 2020
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck.go Sat Feb 15 13:48:40 2020
@@ -46,8 +46,8 @@ func (cv *VartypeCheck) Explain(explanat
//
// fix.Replace("from", "to")
// fix.ReplaceAfter("prefix", "from", "to")
-// fix.InsertBefore("new line")
-// fix.InsertAfter("new line")
+// fix.InsertAbove("new line")
+// fix.InsertBelow("new line")
// fix.Delete()
// fix.Custom(func(showAutofix, autofix bool) {})
//
@@ -276,9 +276,9 @@ func (cv *VartypeCheck) ConfFiles() {
}
for i, word := range words {
- cv.WithValue(word).Pathname()
+ cv.WithValue(word).PathnameSpace()
- if i%2 == 1 && !hasPrefix(word, "${") {
+ if i%2 == 1 && !matches(word, `["']*\$\{`) {
cv.Warnf("The destination file %q should start with a variable reference.", word)
cv.Explain(
"Since pkgsrc can be installed in different locations, the",
@@ -931,6 +931,27 @@ func (cv *VartypeCheck) Pathname() {
invalid)
}
+// Like Pathname, but may contain spaces as well.
+// Because the spaces must be quoted, backslashes and quotes are allowed as well.
+func (cv *VartypeCheck) PathnameSpace() {
+ valid := regex.Pattern(condStr(
+ cv.Op == opUseMatch,
+ `[ "%'*+,\-./0-9?@A-Z\[\\\]_a-z~]`,
+ `[ "%'+,\-./0-9@A-Z\\_a-z~]`))
+ invalid := replaceAll(cv.ValueNoVar, valid, "")
+ if invalid == "" {
+ return
+ }
+
+ cv.Warnf(
+ condStr(cv.Op == opUseMatch,
+ "The pathname pattern %q contains the invalid character%s %q.",
+ "The pathname %q contains the invalid character%s %q."),
+ cv.Value,
+ condStr(len(invalid) > 1, "s", ""),
+ invalid)
+}
+
func (cv *VartypeCheck) Perl5Packlist() {
if cv.Value != cv.ValueNoVar {
cv.Warnf("%s should not depend on other variables.", cv.Varname)
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.76 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.77
--- pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.76 Wed Feb 5 04:09:00 2020
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go Sat Feb 15 13:48:40 2020
@@ -358,6 +358,20 @@ func (s *Suite) Test_VartypeCheck_ConfFi
"WARN: filename.mk:1: Values for CONF_FILES should always be pairs of paths.",
"WARN: filename.mk:3: Values for CONF_FILES should always be pairs of paths.",
"WARN: filename.mk:5: The destination file \"/etc/bootrc\" should start with a variable reference.")
+
+ // See pkgsrc/regress/conf-files-spaces.
+ vt.Values(
+ "back\\ slash.conf ${PKG_SYSCONFDIR}/back\\ slash.conf",
+ "\"d quot.conf\" \"${PKG_SYSCONFDIR}/d quot.conf\"",
+ "'s quot.conf' '${PKG_SYSCONFDIR}/''s quot.conf'")
+ vt.OutputEmpty()
+
+ vt.Values(
+ "\\*.conf ${PKG_SYSCONFDIR}/\\*.conf")
+ vt.Output(
+ "WARN: filename.mk:21: The pathname \"\\\\*.conf\" contains the invalid character \"*\".",
+ "WARN: filename.mk:21: The pathname \"${PKG_SYSCONFDIR}/\\\\*.conf\" contains the invalid character \"*\".")
+
}
// See Test_MkParser_DependencyPattern.
@@ -1302,13 +1316,53 @@ func (s *Suite) Test_VartypeCheck_Pathna
"${PREFIX}/*",
"${PREFIX}/share/locale",
"share/locale",
- "/bin")
+ "/bin",
+ "/path with spaces")
+ vt.Output(
+ "WARN: filename.mk:1: The pathname \"${PREFIX}/*\" "+
+ "contains the invalid character \"*\".",
+ "WARN: filename.mk:5: The pathname \"/path with spaces\" "+
+ "contains the invalid characters \" \".")
+
vt.Op(opUseMatch)
vt.Values(
- "anything")
+ "anything",
+ "/path with *spaces")
+ vt.Output(
+ "WARN: filename.mk:12: The pathname pattern \"/path with *spaces\" " +
+ "contains the invalid characters \" \".")
+}
+
+func (s *Suite) Test_VartypeCheck_PathnameSpace(c *check.C) {
+ // Invent a variable name since this data type is only used as part
+ // of CONF_FILES.
+ G.Pkgsrc.vartypes.DefineParse("CONFIG_FILE", BtPathnameSpace,
+ NoVartypeOptions, "*.mk: set, use")
+ vt := NewVartypeCheckTester(s.Init(c), BtPathnameSpace)
+ vt.Varname("CONFIG_FILE")
+ vt.Values(
+ "${PREFIX}/*",
+ "${PREFIX}/share/locale",
+ "share/locale",
+ "/bin",
+ "/path with spaces")
+ vt.Output(
+ "WARN: filename.mk:1: The pathname \"${PREFIX}/*\" " +
+ "contains the invalid character \"*\".")
+
+ vt.Op(opUseMatch)
+ vt.Values(
+ "anything",
+ "/path with *spaces&",
+ "/path with spaces and ;several, other &characters.",
+ )
vt.Output(
- "WARN: filename.mk:1: The pathname \"${PREFIX}/*\" contains the invalid character \"*\".")
+ "WARN: filename.mk:12: The pathname pattern \"/path with *spaces&\" "+
+ "contains the invalid character \"&\".",
+ "WARN: filename.mk:13: The pathname pattern "+
+ "\"/path with spaces and ;several, other &characters.\" "+
+ "contains the invalid characters \";&\".")
}
func (s *Suite) Test_VartypeCheck_Perl5Packlist(c *check.C) {
@@ -1611,8 +1665,6 @@ func (s *Suite) Test_VartypeCheck_Relati
vt.Output(
"ERROR: ../../mk/infra.mk:1: Relative path \"../package\" does not exist.",
- // FIXME: This directory _does_ exist.
- "ERROR: ../../mk/infra.mk:2: Relative path \"../../category/other-package\" does not exist.",
"ERROR: ../../mk/infra.mk:3: Relative path \"../../missing/package\" does not exist.",
"ERROR: ../../mk/infra.mk:4: Relative path \"../../category/missing\" does not exist.")
}
Home |
Main Index |
Thread Index |
Old Index