kpriv/pkgsrc
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

pkgtools/pkglint: update to 19.4.8

Browse source 
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.
This commit is contained in:
rillig 2020-02-15 13:48:40 +00:00
parent 90b78de3c5
commit 77cc481aa0
57 changed files with 949 additions and 821 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
pkgtools/pkglint/Makefile
View file

@ -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/}

135
pkgtools/pkglint/files/autofix.go
View file

@ -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 = "SilentAutofixFormat"
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, from string, to string) {
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 int, textIndex int, from string, to strin
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
}
replaced := rawLine.textnl[:textIndex] + to + rawLine.textnl[textIndex+len(from):]
text := fix.texts[rawIndex]
assert(textIndex < len(text))
assert(hasPrefix(text[textIndex:], from))
rawLine.textnl = replaced
replaced := text[:textIndex] + to + text[textIndex+len(from):]
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 int, textIndex int, from string, to strin
// 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(showAutofix, autofix bool)) {
// 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) (autofixed bool) {
// 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) (autofixed bool) {
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
changed[filename] = true
}
chlines = append(chlines, fix.linesBefore...)
for _, raw := range line.raw {
chlines = append(chlines, raw.textnl)
}
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 {

119
pkgtools/pkglint/files/autofix_test.go
View file

@ -19,8 +19,8 @@ func (s *Suite) Test_Autofix__default_also_updates_line(c *check.C) {
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_autofix_modifies_line(c *check.C) {
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_autofix_modifies_line(c *check.C) {
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_autofix_modifies_line(c *check.C) {
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_fixes(c *check.C) {
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_fixes(c *check.C) {
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_fixes(c *check.C) {
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__silent_with_diagnostic(c *check.C) {
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__silent_with_diagnostic(c *check.C) {
"\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_ReplaceAfter__after_inserting_a_line(c *check.C) {
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_ReplaceAfter__after_inserting_a_line(c *check.C) {
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__only(c *check.C) {
"# 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__only(c *check.C) {
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_InsertBefore(c *check.C) {
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(c *check.C) {
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__combined_with_insert(c *check.C) {
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_memory(c *check.C) {
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 *check.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_after_replacing(c *check.C) {
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 *check.C) {
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 *check.C) {
"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__no_changes_necessary(c *check.C) {
// 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()
}

2
pkgtools/pkglint/files/buildlink3.go
View file

@ -100,7 +100,7 @@ func (ck *Buildlink3Checker) checkUniquePkgbase(pkgbase string, mkline *MkLine)
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

2
pkgtools/pkglint/files/buildlink3_test.go
View file

@ -443,7 +443,7 @@ func (s *Suite) Test_CheckLinesBuildlink3Mk__missing_endif(c *check.C) {
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) {

4
pkgtools/pkglint/files/category.go
View file

@ -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() {

14
pkgtools/pkglint/files/category_test.go
View file

@ -24,14 +24,14 @@ func (s *Suite) Test_CheckdirCategory__totally_broken(c *check.C) {
"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__subdirs_file_system_at_the_bottom(c *chec
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__comment_at_the_top(c *check.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__unexpected_EOF_while_reading_SUBDIR(c *ch
// 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\"")
}

41
pkgtools/pkglint/files/check_test.go
View file

@ -456,7 +456,7 @@ func (t *Tester) SetUpPackage(pkgpath RelPath, makefileLines ...string) CurrPath
"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(filename RelPath, lines ...string) CurrPath {
}
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(filename RelPath, customLines ...string) {
// 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 RelPath) {
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) ExpectDiagnosticsAutofix(action func(autofix bool), diagnostics
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.

4
pkgtools/pkglint/files/distinfo.go
View file

@ -281,12 +281,12 @@ func (ck *distinfoLinesChecker) checkAlgorithmsDistfile(info distinfoFileInfo) {
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()
}

16
pkgtools/pkglint/files/distinfo_test.go
View file

@ -27,7 +27,7 @@ func (s *Suite) Test_CheckLinesDistinfo__parse_errors(c *check.C) {
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_distinfoLinesChecker_parse__empty_file(c *check.C) {
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_distinfoLinesChecker_parse__commented_first_line(c *check.C
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_distinfoLinesChecker_parse__completely_empty_file(c *check.
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_distinfoLinesChecker_checkAlgorithmsDistfile__add_missing_h
"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_distinfoLinesChecker_checkAlgorithmsDistfile__add_missing_h
"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_distinfoLinesChecker_checkAlgorithmsDistfile__bottom_algori
"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) {

28
pkgtools/pkglint/files/files.go
View file

@ -31,7 +31,7 @@ func Load(filename CurrPath, options LoadOptions) *Lines {
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 LoadOptions) *Lines {
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 LoadOptions) *Lines {
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 CurrPath, rawText string, joinBackslashLines
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 CurrPath, rawText string, joinBackslashLines
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, rawLines []*RawLine, index int) (*Line,
}
}
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) {

39
pkgtools/pkglint/files/homepage.go
View file

@ -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) checkReachable() {
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) checkReachable() {
}
}
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

134
pkgtools/pkglint/files/homepage_test.go
View file

@ -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_Check(c *check.C) {
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_checkFtp(c *check.C) {
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_migrate(c *check.C) {
"")
// 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_checkBadUrls(c *check.C) {
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_checkReachable(c *check.C) {
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_checkReachable(c *check.C) {
<-shutdown
}()
vt.Varname("HOMEPAGE")
vt.Values(
"http://localhost:28780/status/200",
"http://localhost:28780/status/301?location=/redirect301",
"http://localhost:28780/status/302?location=/redirect302",
"http://localhost:28780/status/307?location=/redirect307",
"http://localhost:28780/status/404",
"http://localhost:28780/status/500")
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...)
}
vt.Output(
"WARN: filename.mk:2: Homepage "+
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\".",
"WARN: filename.mk:3: Homepage "+
"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\".",
"WARN: filename.mk:4: Homepage "+
"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\".",
"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\" "+
"redirects to \"http://localhost:28780/redirect307\".")
test(
"http://localhost:28780/status/404",
"WARN: filename.mk:1: Homepage \"http://localhost:28780/status/404\" "+
"returns HTTP status \"404 Not Found\".")
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")
test(
"http://localhost:28780/%invalid",
"ERROR: filename.mk:1: Invalid URL \"http://localhost:28780/%invalid\".")
vt.Output(
"ERROR: filename.mk:21: Invalid URL \"http://localhost:28780/%invalid\".")
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:.*)$`)
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/")
// 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(
"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_isReachableOnline(c *check.C) {
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_isReachableOnline(c *check.C) {
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_isReachableOnline(c *check.C) {
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_hasAnySuffix(c *check.C) {
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_classifyNetworkError(c *check.C) {
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")
}

143
pkgtools/pkglint/files/line.go
View file

@ -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
// zero means the whole file, -1 means EOF, normal lines start with 1
lineno int
}
func NewLocation(filename CurrPath, firstLine, lastLine int) Location {
return Location{filename, int32(firstLine), int32(lastLine)}
func NewLocation(filename CurrPath, lineno int) Location {
return Location{filename, lineno}
}
func (loc *Location) String() string {
return loc.Filename.String() + ":" + loc.Linenos()
}
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.Pattern) (found bool, expanded bool) {
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, args ...interface{}) {
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
}

55
pkgtools/pkglint/files/line_test.go
View file

@ -4,54 +4,37 @@ import (
"gopkg.in/check.v1"
)
func (s *Suite) Test_RawLine_String(c *check.C) {
t := s.Init(c)
line := t.NewLine("filename", 123, "text")
t.CheckEquals(line.raw[0].String(), "123:text\n")
}
func (s *Suite) Test_NewLine__assertion(c *check.C) {
t := s.Init(c)
t.ExpectAssert(func() { NewLine("filename", 123, "text", nil) })
}
func (s *Suite) Test_Line_RelLine__assertion(c *check.C) {
t := s.Init(c)
line := t.NewLine("filename", 123, "")
whole := NewLineWhole("filename")
t.ExpectAssert(func() { line.RelLine(whole) })
}
func (s *Suite) Test_Line_RelLocation__assertion(c *check.C) {
t := s.Init(c)
line := t.NewLine("filename", 123, "")
whole := NewLineWhole("filename")
t.ExpectAssert(func() { line.RelLocation(whole.Location) })
}
func (s *Suite) Test_Line_IsMultiline(c *check.C) {
t := s.Init(c)
t.CheckEquals(t.NewLine("filename", 123, "text").IsMultiline(), false)
t.CheckEquals(NewLineEOF("filename").IsMultiline(), false)
t.CheckEquals(NewLineMulti("filename", 123, 125, "text", nil).IsMultiline(), true)
}
// 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) {
t := s.Init(c)
line := t.NewLine("filename", 123, "")
t.EnableTracingToLog()
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) {

2
pkgtools/pkglint/files/linechecker.go
View file

@ -42,7 +42,7 @@ func (ck LineChecker) CheckTrailingWhitespace() {
// 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

4
pkgtools/pkglint/files/lines.go
View file

@ -18,7 +18,7 @@ func (ls *Lines) Len() int { return len(ls.Lines) }
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, prefixRe regex.Pattern, suggestedPrefix s
"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

6
pkgtools/pkglint/files/lines_test.go
View file

@ -59,9 +59,9 @@ func (s *Suite) Test_Lines_CheckCvsID__wip(c *check.C) {
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.

8
pkgtools/pkglint/files/lineslexer.go
View file

@ -94,15 +94,15 @@ func (llex *LinesLexer) SkipEmptyOrNote() bool {
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()
}

4
pkgtools/pkglint/files/lineslexer_test.go
View file

@ -30,7 +30,7 @@ func (s *Suite) Test_LinesLexer_SkipEmptyOrNote__beginning_of_file(c *check.C) {
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_SkipEmptyOrNote__end_of_file(c *check.C) {
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) {

79
pkgtools/pkglint/files/logging.go
View file

@ -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 *LogLevel, format string, args ...interf
}
}
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 *LogLevel, format string, args ...interf
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, filename CurrPath, lineno, format, msg st
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, filename CurrPath, lineno, format, msg st
} 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, filename CurrPath, lineno, format, msg st
}
}
// TechFatalf logs a technical error on the error output and quits pkglint.
//
// For diagnostics, use Logf instead.
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)
if trace.Tracing {
trace.Stepf("TechFatalf: %s%s", loc, msg)
}
panic(pkglintFatal{})
}
// 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...)
locationStr := ""
if !location.IsEmpty() {
locationStr = location.String() + ": "
}
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))
all := sprintf("ERROR: %s%s\n", loc, msg)
esc := escapePrintable(all)
l.err.Write(esc)
}
func (l *Logger) ShowSummary(args []string) {

118
pkgtools/pkglint/files/logging_test.go
View file

@ -218,17 +218,17 @@ func (s *Suite) Test_Logger_Diag__show_source(c *check.C) {
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__separator_show_autofix_with_explanation
"")
}
// 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__separator_autofix(c *check.C) {
"+\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__profiling(c *check.C) {
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__profiling_autofix(c *check.C) {
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__wording(c *check.C) {
"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__gcc_format(c *check.C) {
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) {

18
pkgtools/pkglint/files/mkassignchecker.go
View file

@ -111,7 +111,7 @@ func (ck *MkAssignChecker) checkVarassignLeftBsdPrefs() {
return
}
if !G.Opts.WarnExtra ||
if !G.WarnExtra ||
G.Infrastructure ||
mkline.Op() != opAssignDefault ||
ck.MkLines.Tools.SeenPrefs {
@ -203,7 +203,7 @@ func (ck *MkAssignChecker) checkVarassignLeftUserSettable() bool {
//
// See checkPermissions.
func (ck *MkAssignChecker) checkVarassignLeftPermissions() {
if !G.Opts.WarnPerm {
if !G.WarnPerm {
return
}
if G.Infrastructure {
@ -274,7 +274,7 @@ func (ck *MkAssignChecker) checkVarassignLeftPermissions() {
}
func (ck *MkAssignChecker) checkVarassignLeftRationale() {
if !G.Opts.WarnExtra {
if !G.WarnExtra {
return
}
@ -384,21 +384,21 @@ func (ck *MkAssignChecker) checkVarassignRightCategory() {
}
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()
}

19
pkgtools/pkglint/files/mkassignchecker_test.go
View file

@ -783,6 +783,25 @@ func (s *Suite) Test_MkAssignChecker_checkVarassignRightCategory__wrong_in_packa
"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)

2
pkgtools/pkglint/files/mkcondchecker_test.go
View file

@ -233,7 +233,7 @@ func (s *Suite) Test_MkCondChecker_checkNotEmpty(c *check.C) {
}
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}.")
}

10
pkgtools/pkglint/files/mkline.go
View file

@ -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 { return mkline.data.(*mkLineAssign).var
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() RelPath { return mkline.data.(*mkLineInclud
// 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) ResolveVarsInRelativePath(relativePath PackagePath, pkg *P
if pkg != nil {
basedir = pkg.File(".")
} else {
basedir = mkline.Filename.DirNoClean()
basedir = mkline.Filename().Dir()
}
tmp := relativePath

29
pkgtools/pkglint/files/mklinechecker.go
View file

@ -43,8 +43,9 @@ func (ck MkLineChecker) checkEmptyContinuation() {
}
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) checkShellCommand() {
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) checkShellCommand() {
"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) checkIncludeBuiltin() {
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) checkDirectiveIndentation(expectedDepth int) {
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) CheckRelativePath(pp PackagePath, rel RelPath, mustExist
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) CheckRelativePath(pp PackagePath, rel RelPath, mustExist
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.

2
pkgtools/pkglint/files/mklineparser.go
View file

@ -170,7 +170,7 @@ func (p MkLineParser) fixSpaceAfterVarname(line *Line, a *mkLineAssign) {
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)

4
pkgtools/pkglint/files/mklines.go
View file

@ -567,7 +567,7 @@ func (mklines *MkLines) CheckUsedBy(relativeName PkgsrcPath) {
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(relativeName PkgsrcPath) {
"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()
}

8
pkgtools/pkglint/files/mklines_test.go
View file

@ -1222,7 +1222,7 @@ func (s *Suite) Test_MkLines_CheckUsedBy__show_autofix(c *check.C) {
"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__show_autofix(c *check.C) {
"#"),
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__show_autofix(c *check.C) {
"# 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)
}

2
pkgtools/pkglint/files/mkshparser_test.go
View file

@ -1006,7 +1006,7 @@ func (b *MkShBuilder) Redirected(cmd *MkShCommand, redirects ...*MkShRedirection
}
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())

6
pkgtools/pkglint/files/mkvarusechecker.go
View file

@ -42,7 +42,7 @@ func (ck *MkVarUseChecker) checkUndefined() {
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(time VucTime) {
//
// 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) warnToolLoadTime(varname string, tool *Tool) {
// 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
}

28
pkgtools/pkglint/files/package.go
View file

@ -143,7 +143,7 @@ func (pkg *Package) load() ([]CurrPath, *MkLines, *MkLines) {
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() (*MkLines, *MkLines) {
// 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 *MkLines, allLines *MkLines, includingFileForU
// 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 *MkLines, mkline *MkLine, allLines *MkLine
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(mkline *MkLine, includingFilename CurrPa
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) checkfilePackageMakefile(filename CurrPath, mklines *MkLines
//
// 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) checkfilePackageMakefile(filename CurrPath, mklines *MkLines
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) checkGnuConfigureUseLanguages() {
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) checkUseLanguagesCompilerMk(mklines *MkLines) {
}
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) checkPossibleDowngrade() {
"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 CurrPath, mode os.FileMode) {
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 CurrPath, mode os.FileMode) {
switch {
case basename == "files",
basename == "patches",
dirent.DirNoClean().Base() == "files",
dirent.Dir().Base() == "files",
isEmptyDir(dirent):
break
@ -1453,7 +1453,7 @@ func (pkg *Package) checkIncludeConditionally(mkline *MkLine, indentation *Inden
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) checkIncludeConditionally(mkline *MkLine, indentation *Inden
} 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
}

12
pkgtools/pkglint/files/package_test.go
View file

@ -3127,12 +3127,10 @@ func (s *Suite) Test_Package_checkLinesBuildlink3Inclusion__ocaml(c *check.C) {
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 *check.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))
}

4
pkgtools/pkglint/files/patches.go
View file

@ -168,7 +168,7 @@ func (ck *PatchChecker) checkUnifiedDiff(patchedFile Path) {
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(line *Line, patchedFiles int) {
if !ck.previousLineEmpty {
fix := line.Autofix()
fix.Notef("Empty line expected.")
fix.InsertBefore("")
fix.InsertAbove("")
fix.Apply()
}
}

8
pkgtools/pkglint/files/patches_test.go
View file

@ -56,8 +56,8 @@ func (s *Suite) Test_CheckLinesPatch__without_empty_line__autofix(c *check.C) {
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_comment_and_no_empty_lines(c *check.C)
// 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__missing_CVS_Id(c *check.C) {
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.")
}

30
pkgtools/pkglint/files/path.go
View file

@ -25,11 +25,9 @@ func (p Path) GoString() string { return sprintf("%q", string(p)) }
// 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 Path(p) }
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 { return NewPath(string(p)) }
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 RelPath) PkgsrcPath {
// 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, string) {
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() }

63
pkgtools/pkglint/files/path_test.go
View file

@ -57,28 +57,11 @@ func (s *Suite) Test_Path_IsEmpty(c *check.C) {
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 *check.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_AsRelPath(c *check.C) {
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 *check.C) {
}
func (s *Suite) Test_RelPath_DirClean(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.DirClean(), dir)
}
test("./dir/../dir///./file", "dir")
}
func (s *Suite) Test_RelPath_DirNoClean(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")

230
pkgtools/pkglint/files/pkglint.go
View file

@ -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 io.Writer) Pkglint {
// 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.Writer, stderr io.Writer, args []string)
}
}()
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() func() {
}
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() func() {
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() func() {
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() {
}
}
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(args []string) int {
//
// 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 CurrPath) {
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 CurrPath, mode os.FileMode) {
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 CurrPath, mode os.FileMode) {
}
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 CurrPath, mode os.FileMode) {
switch pkgsrcdir {
case "../..":
pkglint.checkdirPackage(dir)
p.checkdirPackage(dir)
case "..":
CheckdirCategory(dir)
case ".":
@ -350,8 +344,8 @@ func (pkglint *Pkglint) checkMode(dirent CurrPath, mode os.FileMode) {
}
// 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 *Package) {
// 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 *Package) {
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 *Package) {
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 *Package) {
// 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(filename CurrPath, basename string, depth int,
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(filename CurrPath, basename string, depth int,
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(filename CurrPath, basename string, depth int,
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(filename CurrPath, basename string, depth int,
}
}
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(filename CurrPath) {
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 *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 *MkLines, command string, time ToolTime) (t
// 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(filename CurrPath) map[string]CvsEntry {
}
}
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()
}

10
pkgtools/pkglint/files/pkglint_test.go
View file

@ -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__autofix_exitcode(c *check.C) {
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__autofix(c *check.C) {
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,

28
pkgtools/pkglint/files/pkgsrc.go
View file

@ -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, warn bool) *Change {
}
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) checkRemovedAfterLastFreeze() {
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 PkgsrcPath) CurrPath {
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) bool {
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

18
pkgtools/pkglint/files/pkgsrc_test.go
View file

@ -510,18 +510,20 @@ func (s *Suite) Test_Pkgsrc_checkRemovedAfterLastFreeze__check_global(c *check.C
// 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 *check.C) {
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 *check.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 *check.C) {
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 *check.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)

10
pkgtools/pkglint/files/plist.go
View file

@ -126,7 +126,7 @@ func (ck *PlistChecker) collectPath(rel RelPath, pline *PlistLine) {
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(pline *PlistLine) {
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 *PlistLine) {
} 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))
}

6
pkgtools/pkglint/files/redundantscope.go
View file

@ -46,10 +46,10 @@ func (s *RedundantScope) checkLine(mklines *MkLines, mkline *MkLine) {
}
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())
}
}

2
pkgtools/pkglint/files/redundantscope_test.go
View file

@ -1445,7 +1445,7 @@ func (s *Suite) Test_RedundantScope__is_relevant_for_infrastructure(c *check.C)
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)

27
pkgtools/pkglint/files/shell.go
View file

@ -60,7 +60,7 @@ func (scc *SimpleCommandChecker) checkCommandStart() {
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) checkPipeExitcode(pipeline *MkShPipeline) {
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) CheckShellCommand(shellcmd string, pSetE *bool, time
}
}
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) checkShVarUsePlain(atom *ShAtom, checkQuoting bool)
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) checkMultiLineComment() {
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) checkMultiLineComment() {
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.",

6
pkgtools/pkglint/files/toplevel.go
View file

@ -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...)
}
}

3
pkgtools/pkglint/files/toplevel_test.go
View file

@ -59,7 +59,8 @@ func (s *Suite) Test_CheckdirToplevel__recursive(c *check.C) {
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.)", "."))
}

70
pkgtools/pkglint/files/util.go
View file

@ -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 *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 *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) *MkLine {
// 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) string {
func (s *Scope) LastValueFound(varname string) (value string, found bool) {
value, found = s.value[varname]
if found {
return
if !found {
value, found = s.fallback[varname]
}
mkline := s.LastDefinition(varname)
if mkline != nil && mkline.Op() != opAssignShell {
return mkline.Value(), true
}
if fallback, ok := s.fallback[varname]; ok {
return fallback, true
}
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 CurrPath, options LoadOptions) *Lines {
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 (b *bag) add(key interface{}, count int) {
b.entries = append(b.entries, bagEntry{key, count})
}
func (mi *bag) add(key interface{}, value int) {
mi.slice = append(mi.slice, struct {
key interface{}
value int
}{key, value})
}
func (b *bag) len() int { return len(b.entries) }
func (mi *bag) len() int { return len(mi.slice) }
type bagEntry struct {
key interface{}
count int
}

13
pkgtools/pkglint/files/util_test.go
View file

@ -554,12 +554,14 @@ func (s *Suite) Test_Scope_Define(c *check.C) {
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 *check.C) {
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.C) {
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)

2
pkgtools/pkglint/files/var.go
View file

@ -185,7 +185,7 @@ func (v *Var) Write(mkline *MkLine, conditional bool, conditionVarnames ...strin
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)
}

8
pkgtools/pkglint/files/varalignblock.go
View file

@ -156,7 +156,7 @@ func (va *VaralignBlock) Process(mkline *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) processVarassign(mkline *MkLine) {
}
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})

28
pkgtools/pkglint/files/varalignblock_test.go
View file

@ -3308,7 +3308,7 @@ func (s *Suite) Test_VaralignSplitter_split(c *check.C) {
// 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_alignValueSingle(c *check.C) {
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_alignValueInitial(c *check.C) {
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_alignValueMultiFollow(c *check.C) {
// 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_alignValueMultiFollow(c *check.C) {
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_alignValue(c *check.C) {
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_alignValue(c *check.C) {
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_alignValue(c *check.C) {
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_alignContinuation(c *check.C) {
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,

9
pkgtools/pkglint/files/vardefs.go
View file

@ -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) enumFromFiles(
}
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)

1
pkgtools/pkglint/files/vartype.go
View file

@ -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}

29
pkgtools/pkglint/files/vartypecheck.go
View file

@ -46,8 +46,8 @@ func (cv *VartypeCheck) Explain(explanation ...string) { cv.MkLine.E
//
// 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)

64
pkgtools/pkglint/files/vartypecheck_test.go
View file

@ -358,6 +358,20 @@ func (s *Suite) Test_VartypeCheck_ConfFiles(c *check.C) {
"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_Pathname(c *check.C) {
"${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:1: The pathname \"${PREFIX}/*\" contains the invalid character \"*\".")
"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: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_RelativePkgPath(c *check.C) {
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.")
}