pkgsrc-Changes archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
CVS commit: pkgsrc/pkgtools/pkglint
Module Name: pkgsrc
Committed By: rillig
Date: Thu Feb 21 22:49:04 UTC 2019
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile
pkgsrc/pkgtools/pkglint/files: alternatives_test.go autofix.go
autofix_test.go buildlink3.go buildlink3_test.go category.go
category_test.go check_test.go distinfo.go distinfo_test.go
files.go licenses.go line.go line_test.go linelexer.go
linelexer_test.go lines_test.go logging.go logging_test.go
mkline.go mkline_test.go mklinechecker.go mklinechecker_test.go
mklines.go mklines_test.go mkparser.go mkparser_test.go mktypes.go
options.go options_test.go package.go package_test.go patches.go
pkglint.0 pkglint.1 pkglint.go pkglint_test.go pkgsrc.go
pkgsrc_test.go plist.go plist_test.go shell_test.go
substcontext_test.go toplevel.go util.go util_test.go
vartypecheck.go vartypecheck_test.go
pkgsrc/pkgtools/pkglint/files/getopt: getopt.go
pkgsrc/pkgtools/pkglint/files/textproc: lexer.go lexer_test.go
pkgsrc/pkgtools/pkglint/files/trace: tracing.go
Log Message:
pkgtools/pkglint: update to 5.7.0
Changes since 5.6.12:
* Many of the -C and -W command line options have been removed since
they are not used in practice. The -Wall and -Call options continue
to work though; these are the only options mentioned in the pkgsrc
guide.
* When a PLIST file contains redundant libtool libraries (.la and the
corresponding .so), there is only a single warning per file.
* Warnings about the package COMMENT are now strictly ordered from left
to right.
* The hashes for all distfiles must now contain the SHA512 hash. This
hash has been added to many distfiles in 2015. It's time now to
enforce it on all other distfiles as well.
* Makefile fragments that are included inside an .elif exists(...)
are not reported as missing.
* The check for redundant variables and accidentally overwritten
variables has been improved. Now the warning occurs at the later
definition. This especially applies to cases where a file is included
and after that, some of its variables are overridden. Variables in
unrelated files are no longer marked as redundant.
* When a package contains multiple definitions of a single variable
(typical for Makefile.common), the later definition overrides the
earlier definition. That way, the location of DISTINFO_FILE and
PATCHDIR is resolved correctly.
To generate a diff of this commit:
cvs rdiff -u -r1.566 -r1.567 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.12 -r1.13 pkgsrc/pkgtools/pkglint/files/alternatives_test.go
cvs rdiff -u -r1.16 -r1.17 pkgsrc/pkgtools/pkglint/files/autofix.go \
pkgsrc/pkgtools/pkglint/files/line_test.go \
pkgsrc/pkgtools/pkglint/files/toplevel.go
cvs rdiff -u -r1.17 -r1.18 pkgsrc/pkgtools/pkglint/files/autofix_test.go \
pkgsrc/pkgtools/pkglint/files/category.go
cvs rdiff -u -r1.19 -r1.20 pkgsrc/pkgtools/pkglint/files/buildlink3.go \
pkgsrc/pkgtools/pkglint/files/logging.go
cvs rdiff -u -r1.26 -r1.27 pkgsrc/pkgtools/pkglint/files/buildlink3_test.go
cvs rdiff -u -r1.18 -r1.19 pkgsrc/pkgtools/pkglint/files/category_test.go \
pkgsrc/pkgtools/pkglint/files/pkgsrc.go
cvs rdiff -u -r1.33 -r1.34 pkgsrc/pkgtools/pkglint/files/check_test.go
cvs rdiff -u -r1.27 -r1.28 pkgsrc/pkgtools/pkglint/files/distinfo.go
cvs rdiff -u -r1.24 -r1.25 pkgsrc/pkgtools/pkglint/files/distinfo_test.go \
pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
cvs rdiff -u -r1.23 -r1.24 pkgsrc/pkgtools/pkglint/files/files.go \
pkgsrc/pkgtools/pkglint/files/mkparser.go
cvs rdiff -u -r1.20 -r1.21 pkgsrc/pkgtools/pkglint/files/licenses.go
cvs rdiff -u -r1.32 -r1.33 pkgsrc/pkgtools/pkglint/files/line.go \
pkgsrc/pkgtools/pkglint/files/pkglint_test.go \
pkgsrc/pkgtools/pkglint/files/plist_test.go
cvs rdiff -u -r1.1 -r1.2 pkgsrc/pkgtools/pkglint/files/linelexer.go \
pkgsrc/pkgtools/pkglint/files/linelexer_test.go
cvs rdiff -u -r1.7 -r1.8 pkgsrc/pkgtools/pkglint/files/lines_test.go
cvs rdiff -u -r1.11 -r1.12 pkgsrc/pkgtools/pkglint/files/logging_test.go \
pkgsrc/pkgtools/pkglint/files/options.go
cvs rdiff -u -r1.45 -r1.46 pkgsrc/pkgtools/pkglint/files/mkline.go
cvs rdiff -u -r1.50 -r1.51 pkgsrc/pkgtools/pkglint/files/mkline_test.go
cvs rdiff -u -r1.28 -r1.29 pkgsrc/pkgtools/pkglint/files/mklinechecker.go \
pkgsrc/pkgtools/pkglint/files/patches.go
cvs rdiff -u -r1.40 -r1.41 pkgsrc/pkgtools/pkglint/files/mklines.go
cvs rdiff -u -r1.35 -r1.36 pkgsrc/pkgtools/pkglint/files/mklines_test.go
cvs rdiff -u -r1.21 -r1.22 pkgsrc/pkgtools/pkglint/files/mkparser_test.go \
pkgsrc/pkgtools/pkglint/files/substcontext_test.go
cvs rdiff -u -r1.10 -r1.11 pkgsrc/pkgtools/pkglint/files/mktypes.go \
pkgsrc/pkgtools/pkglint/files/options_test.go
cvs rdiff -u -r1.44 -r1.45 pkgsrc/pkgtools/pkglint/files/package.go
cvs rdiff -u -r1.38 -r1.39 pkgsrc/pkgtools/pkglint/files/package_test.go \
pkgsrc/pkgtools/pkglint/files/pkglint.0 \
pkgsrc/pkgtools/pkglint/files/shell_test.go
cvs rdiff -u -r1.52 -r1.53 pkgsrc/pkgtools/pkglint/files/pkglint.1
cvs rdiff -u -r1.46 -r1.47 pkgsrc/pkgtools/pkglint/files/pkglint.go
cvs rdiff -u -r1.15 -r1.16 pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go
cvs rdiff -u -r1.36 -r1.37 pkgsrc/pkgtools/pkglint/files/plist.go \
pkgsrc/pkgtools/pkglint/files/util.go
cvs rdiff -u -r1.22 -r1.23 pkgsrc/pkgtools/pkglint/files/util_test.go
cvs rdiff -u -r1.48 -r1.49 pkgsrc/pkgtools/pkglint/files/vartypecheck.go
cvs rdiff -u -r1.41 -r1.42 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
cvs rdiff -u -r1.7 -r1.8 pkgsrc/pkgtools/pkglint/files/getopt/getopt.go
cvs rdiff -u -r1.3 -r1.4 pkgsrc/pkgtools/pkglint/files/textproc/lexer.go \
pkgsrc/pkgtools/pkglint/files/textproc/lexer_test.go
cvs rdiff -u -r1.5 -r1.6 pkgsrc/pkgtools/pkglint/files/trace/tracing.go
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: pkgsrc/pkgtools/pkglint/Makefile
diff -u pkgsrc/pkgtools/pkglint/Makefile:1.566 pkgsrc/pkgtools/pkglint/Makefile:1.567
--- pkgsrc/pkgtools/pkglint/Makefile:1.566 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/Makefile Thu Feb 21 22:49:03 2019
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.566 2019/01/26 16:31:33 rillig Exp $
+# $NetBSD: Makefile,v 1.567 2019/02/21 22:49:03 rillig Exp $
-PKGNAME= pkglint-5.6.12
+PKGNAME= pkglint-5.7.0
CATEGORIES= pkgtools
DISTNAME= tools
MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/}
Index: pkgsrc/pkgtools/pkglint/files/alternatives_test.go
diff -u pkgsrc/pkgtools/pkglint/files/alternatives_test.go:1.12 pkgsrc/pkgtools/pkglint/files/alternatives_test.go:1.13
--- pkgsrc/pkgtools/pkglint/files/alternatives_test.go:1.12 Sun Jan 13 19:55:52 2019
+++ pkgsrc/pkgtools/pkglint/files/alternatives_test.go Thu Feb 21 22:49:03 2019
@@ -59,3 +59,23 @@ func (s *Suite) Test_CheckFileAlternativ
t.CheckOutputLines(
"ERROR: ALTERNATIVES: Must not be empty.")
}
+
+func (s *Suite) Test_CheckFileAlternatives__ALTERNATIVES_SRC(c *check.C) {
+ t := s.Init(c)
+
+ // It's a strange situation, having an ALTERNATIVES file defined by
+ // the package but then referring to another package's file by means
+ // of ALTERNATIVES_SRC. As of February 2019 I don't remember if I
+ // really had this case in mind when I initially wrote the code in
+ // CheckFileAlternatives.
+ t.SetUpPackage("category/package",
+ "ALTERNATIVES_SRC=\talts")
+ t.CreateFileLines("category/package/ALTERNATIVES",
+ "bin/pgm @PREFIX@/bin/gnu-program",
+ "bin/pgm @PREFIX@/bin/nb-program")
+ G.Pkgsrc.LoadInfrastructure()
+
+ G.Check(t.File("category/package"))
+
+ t.CheckOutputEmpty()
+}
Index: pkgsrc/pkgtools/pkglint/files/autofix.go
diff -u pkgsrc/pkgtools/pkglint/files/autofix.go:1.16 pkgsrc/pkgtools/pkglint/files/autofix.go:1.17
--- pkgsrc/pkgtools/pkglint/files/autofix.go:1.16 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/autofix.go Thu Feb 21 22:49:03 2019
@@ -96,15 +96,13 @@ func (fix *Autofix) ReplaceAfter(prefix,
}
for _, rawLine := range fix.line.raw {
- if rawLine.Lineno != 0 {
- replaced := strings.Replace(rawLine.textnl, prefix+from, prefix+to, 1)
- if replaced != rawLine.textnl {
- if G.Logger.IsAutofix() {
- rawLine.textnl = replaced
- }
- fix.Describef(rawLine.Lineno, "Replacing %q with %q.", from, to)
- return
+ replaced := strings.Replace(rawLine.textnl, prefix+from, prefix+to, 1)
+ if replaced != rawLine.textnl {
+ if G.Logger.IsAutofix() {
+ rawLine.textnl = replaced
}
+ fix.Describef(rawLine.Lineno, "Replacing %q with %q.", from, to)
+ return
}
}
}
@@ -122,26 +120,24 @@ func (fix *Autofix) ReplaceRegex(from re
done := 0
for _, rawLine := range fix.line.raw {
- if rawLine.Lineno != 0 {
- var froms []string // The strings that have actually changed
+ var froms []string // The strings that have actually changed
- replace := func(fromText string) string {
- if howOften >= 0 && done >= howOften {
- return fromText
- }
- froms = append(froms, fromText)
- done++
- return toText
+ replace := func(fromText string) string {
+ if howOften >= 0 && done >= howOften {
+ return fromText
}
+ froms = append(froms, fromText)
+ done++
+ return toText
+ }
- replaced := replaceAllFunc(rawLine.textnl, from, replace)
- if replaced != rawLine.textnl {
- if G.Logger.IsAutofix() {
- rawLine.textnl = replaced
- }
- for _, fromText := range froms {
- fix.Describef(rawLine.Lineno, "Replacing %q with %q.", fromText, toText)
- }
+ replaced := replaceAllFunc(rawLine.textnl, from, replace)
+ if replaced != rawLine.textnl {
+ if G.Logger.IsAutofix() {
+ rawLine.textnl = replaced
+ }
+ for _, fromText := range froms {
+ fix.Describef(rawLine.Lineno, "Replacing %q with %q.", fromText, toText)
}
}
}
@@ -231,9 +227,10 @@ func (fix *Autofix) Delete() {
}
// Anyway has the effect of showing the diagnostic even when nothing can
-// be fixed automatically.
+// be fixed automatically, but only if neither --show-autofix nor
+// --autofix mode is given.
func (fix *Autofix) Anyway() {
- fix.anyway = true
+ fix.anyway = !G.Logger.IsAutofix()
}
// Apply does the actual work.
@@ -262,21 +259,25 @@ func (fix *Autofix) Apply() {
fix.autofixShortTerm = autofixShortTerm{}
}
- if !G.Logger.Relevant(fix.diagFormat) || !fix.anyway && len(fix.actions) == 0 {
+ if !(G.Logger.Relevant(fix.diagFormat) && (len(fix.actions) > 0 || fix.anyway)) {
reset()
return
}
- logDiagnostic := (G.Logger.Opts.ShowAutofix || !G.Logger.Opts.Autofix) &&
- fix.diagFormat != SilentAutofixFormat
+ logDiagnostic := true
+ switch {
+ case fix.diagFormat == SilentAutofixFormat:
+ logDiagnostic = false
+ case G.Logger.Opts.Autofix && !G.Logger.Opts.ShowAutofix:
+ logDiagnostic = false
+ }
+
logFix := G.Logger.IsAutofix()
if logDiagnostic {
msg := sprintf(fix.diagFormat, fix.diagArgs...)
- if !logFix {
- if fix.diagFormat == AutofixFormat || G.Logger.FirstTime(line.Filename, line.Linenos(), msg) {
- line.showSource(G.out)
- }
+ if !logFix && G.Logger.FirstTime(line.Filename, line.Linenos(), msg) {
+ line.showSource(G.out)
}
G.Logf(fix.level, line.Filename, line.Linenos(), fix.diagFormat, msg)
}
@@ -299,7 +300,7 @@ func (fix *Autofix) Apply() {
G.Explain(fix.explanation...)
}
if G.Logger.Opts.ShowSource {
- if !G.Logger.Opts.Explain || !logDiagnostic || len(fix.explanation) == 0 {
+ if !(G.Logger.Opts.Explain && logDiagnostic && len(fix.explanation) > 0) {
G.out.Separate()
}
}
@@ -327,8 +328,8 @@ func (fix *Autofix) Realign(mkline MkLin
{
// Parsing the continuation marker as variable value is cheating but works well.
text := strings.TrimSuffix(mkline.raw[0].orignl, "\n")
- m, _, _, _, _, valueAlign, value, _, _ := MatchVarassign(text)
- if m && value != "\\" {
+ _, _, _, _, _, valueAlign, value, _, _ := MatchVarassign(text)
+ if value != "\\" {
oldWidth = tabWidth(valueAlign)
}
}
@@ -336,7 +337,7 @@ func (fix *Autofix) Realign(mkline MkLin
for _, rawLine := range fix.line.raw[1:] {
_, comment, space := match2(rawLine.textnl, `^(#?)([ \t]*)`)
width := tabWidth(comment + space)
- if (oldWidth == 0 || width < oldWidth) && width >= 8 && rawLine.textnl != "\n" {
+ if (oldWidth == 0 || width < oldWidth) && width >= 8 {
oldWidth = width
}
if !matches(space, `^\t* {0,7}$`) {
Index: pkgsrc/pkgtools/pkglint/files/line_test.go
diff -u pkgsrc/pkgtools/pkglint/files/line_test.go:1.16 pkgsrc/pkgtools/pkglint/files/line_test.go:1.17
--- pkgsrc/pkgtools/pkglint/files/line_test.go:1.16 Mon Dec 17 00:15:39 2018
+++ pkgsrc/pkgtools/pkglint/files/line_test.go Thu Feb 21 22:49:03 2019
@@ -12,6 +12,23 @@ func (s *Suite) Test_RawLine_String(c *c
c.Check(line.raw[0].String(), equals, "123:text\n")
}
+func (s *Suite) Test_NewLine__assertion(c *check.C) {
+ t := s.Init(c)
+
+ t.ExpectPanic(
+ func() { NewLine("filename", 123, "text", nil) },
+ "Pkglint internal error: use NewLineMulti for creating a Line with no RawLine attached to it")
+}
+
+func (s *Suite) Test_Line_IsMultiline(c *check.C) {
+ t := s.Init(c)
+
+ t.Check(t.NewLine("filename", 123, "text").IsMultiline(), equals, false)
+ t.Check(NewLineEOF("filename").IsMultiline(), equals, false)
+
+ t.Check(NewLineMulti("filename", 123, 125, "text", nil).IsMultiline(), equals, 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) {
Index: pkgsrc/pkgtools/pkglint/files/toplevel.go
diff -u pkgsrc/pkgtools/pkglint/files/toplevel.go:1.16 pkgsrc/pkgtools/pkglint/files/toplevel.go:1.17
--- pkgsrc/pkgtools/pkglint/files/toplevel.go:1.16 Mon Dec 17 00:15:39 2018
+++ pkgsrc/pkgtools/pkglint/files/toplevel.go Thu Feb 21 22:49:03 2019
@@ -29,8 +29,8 @@ func CheckdirToplevel(dir string) {
if G.Opts.Recursive {
if G.Opts.CheckGlobal {
- G.Pkgsrc.UsedLicenses = make(map[string]bool)
- G.Pkgsrc.Hashes = make(map[string]*Hash)
+ G.UsedLicenses = make(map[string]bool)
+ G.Hashes = make(map[string]*Hash)
}
G.Todo = append(append([]string(nil), ctx.subdirs...), G.Todo...)
}
Index: pkgsrc/pkgtools/pkglint/files/autofix_test.go
diff -u pkgsrc/pkgtools/pkglint/files/autofix_test.go:1.17 pkgsrc/pkgtools/pkglint/files/autofix_test.go:1.18
--- pkgsrc/pkgtools/pkglint/files/autofix_test.go:1.17 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/autofix_test.go Thu Feb 21 22:49:03 2019
@@ -210,6 +210,35 @@ func (s *Suite) Test_Autofix_ReplaceRege
"+\tYXXXX")
}
+// When an autofix replaces text, it does not touch those
+// lines that have been inserted before since these are
+// usually already correct.
+func (s *Suite) Test_Autofix_ReplaceAfter__after_inserting_a_line(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("--show-autofix")
+ line := t.NewLine("filename", 5, "initial text")
+
+ fix := line.Autofix()
+ fix.Notef("Inserting a line.")
+ fix.InsertBefore("line before")
+ fix.InsertAfter("line after")
+ fix.Apply()
+
+ fix.Notef("Replacing text.")
+ fix.Replace("l", "L")
+ fix.ReplaceRegex(`i`, "I", 1)
+ fix.Apply()
+
+ 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.",
+ "NOTE: filename:5: Replacing text.",
+ "AUTOFIX: filename:5: Replacing \"l\" with \"L\".",
+ "AUTOFIX: filename:5: Replacing \"i\" with \"I\".")
+}
+
func (s *Suite) Test_SaveAutofixChanges(c *check.C) {
t := s.Init(c)
@@ -627,6 +656,35 @@ func (s *Suite) Test_Autofix__suppress_u
"+\tTODO")
}
+// With the default command line options, this warning is printed.
+// With the --show-autofix option this warning is NOT printed since it
+// cannot be fixed automatically.
+func (s *Suite) Test_Autofix_Anyway__options(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("--show-autofix")
+
+ line := t.NewLine("filename", 3, "")
+ fix := line.Autofix()
+
+ fix.Warnf("This autofix doesn't match.")
+ fix.Replace("doesn't match", "")
+ fix.Anyway()
+ fix.Apply()
+
+ t.CheckOutputEmpty()
+
+ t.SetUpCommandLine()
+
+ fix.Warnf("This autofix doesn't match.")
+ fix.Replace("doesn't match", "")
+ fix.Anyway()
+ fix.Apply()
+
+ t.CheckOutputLines(
+ "WARN: filename:3: This autofix doesn't match.")
+}
+
// If an Autofix doesn't do anything, it must not log any diagnostics.
func (s *Suite) Test_Autofix__noop_replace(c *check.C) {
t := s.Init(c)
@@ -809,6 +867,210 @@ func (s *Suite) Test_Autofix_Apply__expl
"NOTE: README.txt:123: A note without fix.")
}
+func (s *Suite) Test_Autofix_Anyway__autofix_option(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("--autofix")
+ line := t.NewLine("filename", 5, "text")
+
+ fix := line.Autofix()
+ fix.Notef("This line is quite short.")
+ fix.Replace("not found", "needle")
+ fix.Anyway()
+ fix.Apply()
+
+ // The replacement text is not found, therefore the note should not be logged.
+ // Because of fix.Anyway, the note should be logged anyway.
+ // But because of the --autofix option, the note should not be logged.
+ // Therefore, in the end nothing is shown in this case.
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_Autofix_Anyway__autofix_and_show_autofix_options(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("--autofix", "--show-autofix")
+ line := t.NewLine("filename", 5, "text")
+
+ fix := line.Autofix()
+ fix.Notef("This line is quite short.")
+ fix.Replace("not found", "needle")
+ fix.Anyway()
+ fix.Apply()
+
+ // The replacement text is not found, therefore the note should not be logged.
+ // Because of fix.Anyway, the note should be logged anyway.
+ // But because of the --autofix option, the note should not be logged.
+ // Even if the --show-autofix option is explicitly given, the note should not be logged.
+ // Therefore, in the end nothing is shown in this case.
+ t.CheckOutputEmpty()
+}
+
+// The --autofix option normally suppresses the diagnostics and just logs
+// the actual fixes. Adding the --show-autofix option logs both.
+func (s *Suite) Test_Autofix_Apply__autofix_and_show_autofix_options(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("--autofix", "--show-autofix")
+ line := t.NewLine("filename", 5, "text")
+
+ fix := line.Autofix()
+ fix.Notef("This line is quite short.")
+ fix.Replace("text", "replacement")
+ fix.Apply()
+
+ t.CheckOutputLines(
+ "NOTE: filename:5: This line is quite short.",
+ "AUTOFIX: filename:5: Replacing \"text\" with \"replacement\".")
+}
+
+// Ensures that without explanations, the separator between the individual
+// diagnostics are generated.
+func (s *Suite) Test_Autofix_Apply__source_without_explain(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("--source", "--explain", "--show-autofix")
+ line := t.NewLine("filename", 5, "text")
+
+ fix := line.Autofix()
+ fix.Notef("This line is quite short.")
+ fix.Replace("text", "replacement")
+ fix.Apply()
+
+ fix.Warnf("Follow-up warning, separated.")
+ fix.Replace("replacement", "text again")
+ fix.Apply()
+
+ t.CheckOutputLines(
+ "NOTE: filename:5: This line is quite short.",
+ "AUTOFIX: filename:5: Replacing \"text\" with \"replacement\".",
+ "-\ttext",
+ "+\treplacement",
+ "",
+ "WARN: filename:5: Follow-up warning, separated.",
+ "AUTOFIX: filename:5: Replacing \"replacement\" with \"text again\".",
+ "-\ttext",
+ "+\ttext again")
+}
+
+func (s *Suite) Test_Autofix_Realign__wrong_line_type(c *check.C) {
+ t := s.Init(c)
+
+ mklines := t.NewMkLines("file.mk",
+ MkRcsID,
+ ".if \\",
+ "${PKGSRC_RUN_TESTS}")
+
+ mkline := mklines.mklines[1]
+ fix := mkline.Autofix()
+
+ t.ExpectPanic(
+ func() { fix.Realign(mkline, 16) },
+ "Pkglint internal error: Line must be a variable assignment.")
+}
+
+func (s *Suite) Test_Autofix_Realign__short_continuation_line(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("--autofix")
+ mklines := t.SetUpFileMkLines("file.mk",
+ MkRcsID,
+ "BUILD_DIRS= \\",
+ "\tdir \\",
+ "")
+ mkline := mklines.mklines[1]
+ fix := mkline.Autofix()
+ fix.Warnf("Line should be realigned.")
+
+ // In this case realigning has no effect since the oldWidth == 8,
+ // which counts as "sufficiently intentional not to be modified".
+ fix.Realign(mkline, 16)
+
+ mklines.SaveAutofixChanges()
+
+ t.CheckOutputEmpty()
+ t.CheckFileLines("file.mk",
+ MkRcsID,
+ "BUILD_DIRS= \\",
+ "\tdir \\",
+ "")
+}
+
+func (s *Suite) Test_Autofix_Realign__multiline_indented_with_spaces(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("--autofix")
+ mklines := t.SetUpFileMkLines("file.mk",
+ MkRcsID,
+ "BUILD_DIRS= \\",
+ "\t dir1 \\",
+ "\t\tdir2 \\",
+ " ") // Trailing whitespace is not fixed by Autofix.Realign.
+
+ mkline := mklines.mklines[1]
+
+ fix := mkline.Autofix()
+ fix.Warnf("Warning.")
+ fix.Realign(mkline, 16)
+ fix.Apply()
+
+ mklines.SaveAutofixChanges()
+
+ t.CheckOutputLines(
+ "AUTOFIX: ~/file.mk:3: Replacing indentation \"\\t \" with \"\\t\\t\".")
+ t.CheckFileLines("file.mk",
+ MkRcsID,
+ "BUILD_DIRS= \\",
+ "\t\tdir1 \\",
+ "\t\tdir2 \\",
+ " ")
+}
+
+// Just for branch coverage.
+func (s *Suite) Test_Autofix_setDiag__no_testing_mode(c *check.C) {
+ t := s.Init(c)
+
+ line := t.NewLine("file.mk", 123, "text")
+
+ G.Testing = false
+
+ fix := line.Autofix()
+ fix.Notef("Note.")
+ fix.Replace("from", "to")
+ fix.Apply()
+
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_Autofix_setDiag__bad_call_sequence(c *check.C) {
+ t := s.Init(c)
+
+ line := t.NewLine("file.mk", 123, "text")
+ fix := line.Autofix()
+ fix.Notef("Note.")
+
+ t.ExpectPanic(
+ func() { fix.Notef("Note 2.") },
+ "Pkglint internal error: Autofix can only have a single diagnostic.")
+
+ fix.level = nil // To cover the second assertion.
+ t.ExpectPanic(
+ func() { fix.Notef("Note 2.") },
+ "Pkglint internal error: Autofix can only have a single diagnostic.")
+}
+
+func (s *Suite) Test_Autofix_assertRealLine(c *check.C) {
+ t := s.Init(c)
+
+ line := NewLineEOF("filename.mk")
+ fix := line.Autofix()
+ fix.Warnf("Warning.")
+
+ t.ExpectPanic(
+ func() { fix.Replace("from", "to") },
+ "Pkglint internal error: Cannot autofix this line since it is not a real line.")
+}
+
func (s *Suite) Test_SaveAutofixChanges__file_removed(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/category.go
diff -u pkgsrc/pkgtools/pkglint/files/category.go:1.17 pkgsrc/pkgtools/pkglint/files/category.go:1.18
--- pkgsrc/pkgtools/pkglint/files/category.go:1.17 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/category.go Thu Feb 21 22:49:03 2019
@@ -123,7 +123,7 @@ func CheckdirCategory(dir string) {
}
fRest = fRest[1:]
- } else if len(mRest) > 0 && (len(fRest) == 0 || mRest[0].name < fRest[0]) {
+ } else if len(fRest) == 0 || mRest[0].name < fRest[0] {
if !fCheck[mRest[0].name] {
fix := mRest[0].line.Autofix()
fix.Errorf("%q exists in the Makefile but not in the file system.", mRest[0].name)
Index: pkgsrc/pkgtools/pkglint/files/buildlink3.go
diff -u pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.19 pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.20
--- pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.19 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/buildlink3.go Thu Feb 21 22:49:03 2019
@@ -205,7 +205,8 @@ func (ck *Buildlink3Checker) checkVarass
}
func (ck *Buildlink3Checker) checkVaruseInPkgbase(pkgbase string, pkgbaseLine MkLine) {
- for _, token := range pkgbaseLine.ValueTokens() {
+ tokens, _ := pkgbaseLine.ValueTokens()
+ for _, token := range tokens {
if token.Varuse == nil {
continue
}
Index: pkgsrc/pkgtools/pkglint/files/logging.go
diff -u pkgsrc/pkgtools/pkglint/files/logging.go:1.19 pkgsrc/pkgtools/pkglint/files/logging.go:1.20
--- pkgsrc/pkgtools/pkglint/files/logging.go:1.19 Fri Dec 21 08:05:24 2018
+++ pkgsrc/pkgtools/pkglint/files/logging.go Thu Feb 21 22:49:03 2019
@@ -4,6 +4,7 @@ import (
"bytes"
"io"
"netbsd.org/pkglint/histogram"
+ "netbsd.org/pkglint/textproc"
"path"
)
@@ -51,6 +52,7 @@ var (
var dummyLine = NewLineMulti("", 0, 0, "", nil)
+// IsAutofix returns whether one of the --show-autofix or --autofix options is active.
func (l *Logger) IsAutofix() bool { return l.Opts.Autofix || l.Opts.ShowAutofix }
// Relevant decides and remembers whether the given diagnostic is relevant and should be logged.
@@ -186,8 +188,17 @@ func (l *Logger) Logf(level *LogLevel, f
return
}
- if G.Testing && format != AutofixFormat && !hasSuffix(format, ": %s") && !hasSuffix(format, ". %s") {
- G.Assertf(hasSuffix(format, "."), "Diagnostic format %q must end in a period.", format)
+ if G.Testing && format != AutofixFormat {
+ if textproc.Alpha.Contains(format[0]) {
+ G.Assertf(
+ textproc.Upper.Contains(format[0]),
+ "Diagnostic %q must start with an uppercase letter.",
+ format)
+ }
+
+ if !hasSuffix(format, ": %s") && !hasSuffix(format, ". %s") {
+ G.Assertf(hasSuffix(format, "."), "Diagnostic format %q must end in a period.", format)
+ }
}
if filename != "" {
Index: pkgsrc/pkgtools/pkglint/files/buildlink3_test.go
diff -u pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.26 pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.27
--- pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.26 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/buildlink3_test.go Thu Feb 21 22:49:03 2019
@@ -493,6 +493,192 @@ func (s *Suite) Test_CheckLinesBuildlink
"WARN: buildlink3.mk:3: Please replace \"${LICENSE}\" with a simple string (also in other variables in this file).")
}
+func (s *Suite) Test_Buildlink3Checker_checkMainPart__if_else_endif(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package",
+ "PKGNAME=\tpackage-1.0")
+ t.CreateFileDummyBuildlink3("category/package/buildlink3.mk",
+ ".if ${X11_TYPE} == modular",
+ ".else",
+ ".endif")
+
+ G.Check(t.File("category/package"))
+
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_Buildlink3Checker_checkVarassign__dependencies_with_path(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package",
+ "PKGNAME=\tpackage-1.0")
+ t.CreateFileDummyBuildlink3("category/package/buildlink3.mk",
+ "BUILDLINK_ABI_DEPENDS.package+=\tpackage>=1.0:../../category/package",
+ "BUILDLINK_API_DEPENDS.package+=\tpackage>=1.5:../../category/package")
+
+ G.Check(t.File("category/package"))
+
+ // Since these dependencies are malformed, they are not processed further.
+ // Doing that would reveal that the ABI version should be higher than the API version.
+ t.CheckOutputLines(
+ "WARN: ~/category/package/buildlink3.mk:12: "+
+ "Invalid dependency pattern \"package>=1.0:../../category/package\".",
+ "WARN: ~/category/package/buildlink3.mk:13: "+
+ "Invalid dependency pattern \"package>=1.5:../../category/package\".")
+}
+
+func (s *Suite) Test_Buildlink3Checker_checkVarassign__abi_without_api(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package",
+ "PKGNAME=\tpackage-1.0")
+ // t.CreateFileDummyBuildlink3() cannot be used here since it always adds an API line.
+ t.CreateFileLines("category/package/buildlink3.mk",
+ MkRcsID,
+ "",
+ "BUILDLINK_TREE+=\tpackage",
+ "",
+ ".if !defined(PACKAGE_BUILDLINK3_MK)",
+ "PACKAGE_BUILDLINK3_MK:=",
+ "",
+ "BUILDLINK_PKGSRCDIR.package?=\t../../category/package",
+ "BUILDLINK_DEPMETHOD.package?=\tbuild",
+ "BUILDLINK_ABI_DEPENDS.package+=\tpackage>=1.0",
+ "",
+ ".endif # PACKAGE_BUILDLINK3_MK",
+ "",
+ "BUILDLINK_TREE+=\t-package")
+
+ G.Check(t.File("category/package"))
+
+ // Since only ABI is given but not API, the check for ABI >= API cannot be done.
+ t.CheckOutputLines(
+ "WARN: ~/category/package/buildlink3.mk:13: Definition of BUILDLINK_API_DEPENDS is missing.")
+}
+
+func (s *Suite) Test_Buildlink3Checker_checkVarassign__abi_and_api_with_variables(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package",
+ "PKGNAME=\tpackage-1.0")
+ t.CreateFileDummyBuildlink3("category/package/buildlink3.mk",
+ "BUILDLINK_ABI_DEPENDS.package+=\tpackage>=${ABI_VERSION}",
+ "BUILDLINK_API_DEPENDS.package+=\tpackage>=${API_VERSION}",
+ "",
+ "ABI_VERSION=\t1.0",
+ "API_VERSION=\t1.5")
+
+ G.Check(t.File("category/package"))
+
+ // Since the versions contain variable references, pkglint refuses to compare them.
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_Buildlink3Checker_checkVarassign__api_with_variable(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package",
+ "PKGNAME=\tpackage-1.0")
+ t.CreateFileDummyBuildlink3("category/package/buildlink3.mk",
+ "BUILDLINK_ABI_DEPENDS.package+=\tpackage>=1.0",
+ "BUILDLINK_API_DEPENDS.package+=\tpackage>=${API_VERSION}",
+ "",
+ "API_VERSION=\t1.5")
+
+ G.Check(t.File("category/package"))
+
+ // Since the versions contain variable references, pkglint refuses to compare them.
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_Buildlink3Checker_checkVarassign__abi_and_api_with_pattern(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package",
+ "PKGNAME=\tpackage-1.0")
+ t.CreateFileDummyBuildlink3("category/package/buildlink3.mk",
+ "BUILDLINK_ABI_DEPENDS.package+=\tpackage-1.*",
+ "BUILDLINK_API_DEPENDS.package+=\tpackage-2.*")
+
+ G.Check(t.File("category/package"))
+
+ // Since the versions do not contain lower bounds (they are package-1.*
+ // instead of package>=1), pkglint refuses to compare them.
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_Buildlink3Checker_checkVarassign__api_with_pattern(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package",
+ "PKGNAME=\tpackage-1.0")
+ t.CreateFileDummyBuildlink3("category/package/buildlink3.mk",
+ "BUILDLINK_ABI_DEPENDS.package+=\tpackage>=1",
+ "BUILDLINK_API_DEPENDS.package+=\tpackage-1.*")
+
+ G.Check(t.File("category/package"))
+
+ // Since the API version does not contain lower bounds (it is package-1.*
+ // instead of package>=1), pkglint refuses to compare the versions.
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_Buildlink3Checker_checkVarassign__other_variables(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package",
+ "PKGNAME=\tpackage-1.0")
+ t.CreateFileDummyBuildlink3("category/package/buildlink3.mk",
+ "BUILDLINK_TREE+=\tmistake", // Wrong, but doesn't happen in practice.
+ "",
+ "LDFLAGS.NetBSD+=\t-ldl",
+ "",
+ "BUILDLINK_DEPMETHOD.other+=\tbuild",
+ "",
+ "BUILDLINK_API_DEPENDS.other+=\tother>=3")
+
+ G.Check(t.File("category/package"))
+
+ // FIXME: Why is appending to LDFLAGS forbidden? It sounds useful.
+ t.CheckOutputLines(
+ "WARN: ~/category/package/buildlink3.mk:14: "+
+ "The variable LDFLAGS.NetBSD may not be appended to in this file; "+
+ "it would be ok in Makefile, Makefile.common, options.mk or *.mk.",
+ "WARN: ~/category/package/buildlink3.mk:16: "+
+ "Only buildlink variables for \"package\", "+
+ "not \"other\" may be set in this file.")
+}
+
+// Just for branch coverage.
+func (s *Suite) Test_Buildlink3Checker_Check__no_tracing(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package")
+ t.CreateFileDummyBuildlink3("category/package/buildlink3.mk")
+ t.DisableTracing()
+
+ G.Check(t.File("category/package/buildlink3.mk"))
+
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_Buildlink3Checker_checkSecondParagraph__missing_mkbase(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package",
+ "DISTNAME=\t# empty",
+ "PKGNAME=\t# empty, to force mkbase to be empty")
+ t.CreateFileDummyBuildlink3("category/package/buildlink3.mk")
+
+ G.Check(t.File("category/package"))
+
+ // There is no warning from buildlink3.mk about mismatched package names
+ // since that is only a follow-up error of being unable to parse the pkgbase.
+ t.CheckOutputLines(
+ "WARN: ~/category/package/Makefile:4: \"\" is not a valid package name.")
+}
+
// Since the buildlink3 checker does not use MkLines.ForEach, it has to keep
// track of the nesting depth of .if directives.
func (s *Suite) Test_Buildlink3Checker_checkMainPart__nested_if(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/category_test.go
diff -u pkgsrc/pkgtools/pkglint/files/category_test.go:1.18 pkgsrc/pkgtools/pkglint/files/category_test.go:1.19
--- pkgsrc/pkgtools/pkglint/files/category_test.go:1.18 Sun Jan 13 19:55:52 2019
+++ pkgsrc/pkgtools/pkglint/files/category_test.go Thu Feb 21 22:49:03 2019
@@ -130,6 +130,89 @@ func (s *Suite) Test_CheckdirCategory__s
"ERROR: ~/category/Makefile:11: \"commented-mk-only\" exists in the Makefile but not in the file system.")
}
+func (s *Suite) Test_CheckdirCategory__only_in_Makefile(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPkgsrc()
+ t.SetUpVartypes()
+ t.CreateFileLines("mk/misc/category.mk")
+ t.CreateFileLines("category/both/Makefile")
+ t.CreateFileLines("category/Makefile",
+ MkRcsID,
+ "",
+ "COMMENT=\tCategory comment",
+ "",
+ "SUBDIR+=\tabove-only-in-makefile",
+ "SUBDIR+=\tboth",
+ "SUBDIR+=\tonly-in-makefile",
+ "",
+ ".include \"../mk/misc/category.mk\"")
+
+ CheckdirCategory(t.File("category"))
+
+ t.CheckOutputLines(
+ "ERROR: ~/category/Makefile:5: \"above-only-in-makefile\" exists in the Makefile "+
+ "but not in the file system.",
+ "ERROR: ~/category/Makefile:7: \"only-in-makefile\" exists in the Makefile "+
+ "but not in the file system.")
+}
+
+func (s *Suite) Test_CheckdirCategory__only_in_file_system(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPkgsrc()
+ t.SetUpVartypes()
+ t.CreateFileLines("mk/misc/category.mk")
+ t.CreateFileLines("category/above-only-in-fs/Makefile")
+ t.CreateFileLines("category/both/Makefile")
+ t.CreateFileLines("category/only-in-fs/Makefile")
+ t.CreateFileLines("category/Makefile",
+ MkRcsID,
+ "",
+ "COMMENT=\tCategory comment",
+ "",
+ "SUBDIR+=\tboth",
+ "",
+ ".include \"../mk/misc/category.mk\"")
+
+ CheckdirCategory(t.File("category"))
+
+ t.CheckOutputLines(
+ "ERROR: ~/category/Makefile:5: \"above-only-in-fs\" exists in the file system "+
+ "but not in the Makefile.",
+ "ERROR: ~/category/Makefile:6: \"only-in-fs\" exists in the file system "+
+ "but not in the Makefile.")
+}
+
+func (s *Suite) Test_CheckdirCategory__recursive(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("-r")
+ t.SetUpPkgsrc()
+ t.SetUpVartypes()
+ t.CreateFileLines("mk/misc/category.mk")
+ t.CreateFileLines("category/commented/Makefile")
+ t.CreateFileLines("category/package/Makefile")
+ t.CreateFileLines("category/Makefile",
+ MkRcsID,
+ "",
+ "COMMENT=\tCategory comment",
+ "",
+ "#SUBDIR+=\tcommented\t# reason",
+ "SUBDIR+=\tpackage",
+ "",
+ ".include \"../mk/misc/category.mk\"")
+ t.Chdir("category")
+
+ CheckdirCategory(".")
+
+ t.CheckOutputEmpty()
+ t.Check(
+ G.Todo,
+ deepEquals,
+ []string{"./package"})
+}
+
// Ensures that a directory in the file system can be added at the very
// end of the SUBDIR list. This case takes a different code path than
// an addition in the middle.
@@ -187,6 +270,61 @@ func (s *Suite) Test_CheckdirCategory__i
"NOTE: ~/category/Makefile:5: This variable value should be aligned with tabs, not spaces, to column 17.")
}
+func (s *Suite) Test_CheckdirCategory__comment_at_the_top(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPkgsrc()
+ t.SetUpVartypes()
+ t.CreateFileLines("mk/misc/category.mk")
+ t.CreateFileLines("category/package/Makefile")
+ t.CreateFileLines("category/Makefile",
+ MkRcsID,
+ "",
+ "# This category collects all programs that don't fit anywhere else.",
+ "",
+ "COMMENT=\tCategory comment",
+ "",
+ "SUBDIR+=\tpackage",
+ "",
+ ".include \"../mk/misc/category.mk\"")
+
+ CheckdirCategory(t.File("category"))
+
+ // FIXME: Wow. These are quite a few warnings and errors, just because there is
+ // an additional comment above the COMMENT definition.
+ t.CheckOutputLines(
+ "ERROR: ~/category/Makefile:3: COMMENT= line expected.",
+ "NOTE: ~/category/Makefile:2: Empty line expected after 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:2: Empty line expected after this line.",
+ "WARN: ~/category/Makefile:3: This line should contain the following text: .include \"../mk/misc/category.mk\"",
+ "ERROR: ~/category/Makefile:3: The file should end here.")
+}
+
+func (s *Suite) Test_CheckdirCategory__unexpected_EOF_while_reading_SUBDIR(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPkgsrc()
+ t.SetUpVartypes()
+ t.CreateFileLines("mk/misc/category.mk")
+ t.CreateFileLines("category/package/Makefile")
+ t.CreateFileLines("category/Makefile",
+ MkRcsID,
+ "",
+ "COMMENT=\tCategory comment",
+ "",
+ "SUBDIR+=\tpackage")
+
+ CheckdirCategory(t.File("category"))
+
+ // Doesn't happen in practice since categories are created very seldom.
+ t.CheckOutputLines(
+ "NOTE: ~/category/Makefile:5: Empty line expected after this line.",
+ "WARN: ~/category/Makefile:EOF: This line should contain the following text: "+
+ ".include \"../mk/misc/category.mk\"")
+}
+
func (s *Suite) Test_CheckdirCategory__no_Makefile(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/pkgsrc.go
diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.18 pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.19
--- pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.18 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/pkgsrc.go Thu Feb 21 22:49:03 2019
@@ -34,18 +34,15 @@ type Pkgsrc struct {
LastChange map[string]*Change //
listVersions map[string][]string // See ListVersions
- // Variables that may be overridden by the pkgsrc user. Used for checking BUILD_DEFS.
+ // Variables that may be overridden by the pkgsrc user.
+ // They are typically defined in mk/defaults/mk.conf.
+ //
+ // Whenever a package uses such a variable, it must add the variable name
+ // to BUILD_DEFS.
UserDefinedVars Scope
Deprecated map[string]string //
vartypes map[string]*Vartype // varcanon => type
-
- // TODO: The Hashes and UsedLicenses are modified after the initialization.
- // This contradicts the expectation that all pkgsrc data is constant.
- // These two fields should probably be moved to the Pkglint type.
-
- Hashes map[string]*Hash // Maps "alg:filename" => hash (inter-package check).
- UsedLicenses map[string]bool // Maps "license name" => true (inter-package check).
}
func NewPkgsrc(dir string) *Pkgsrc {
@@ -62,9 +59,7 @@ func NewPkgsrc(dir string) *Pkgsrc {
make(map[string][]string),
NewScope(),
make(map[string]string),
- make(map[string]*Vartype),
- nil, // Only initialized when pkglint is run for a whole pkgsrc installation
- nil}
+ make(map[string]*Vartype)}
return &src
}
@@ -222,7 +217,7 @@ func (src *Pkgsrc) ListVersions(category
// written without dots, which leads to ambiguities:
//
// databases/postgresql: 94 < 95 < 96 < 10 < 11
- // lang/go: go19 < go110 < go111 < go2
+ // lang/go: 19 < 110 < 111 < 2
keys := make(map[string]int)
for _, name := range names {
if m, pkgbase, versionStr := match2(name, `^(\D+)(\d+)$`); m {
@@ -240,9 +235,7 @@ func (src *Pkgsrc) ListVersions(category
}
sort.SliceStable(names, func(i, j int) bool {
- // TODO: Check if this Less implementation is really transitive.
- // examples: ps ps5 ps10 ps96 pq px
- if keyI, keyJ := keys[names[i]], keys[names[j]]; keyI != 0 && keyJ != 0 {
+ if keyI, keyJ := keys[names[i]], keys[names[j]]; keyI != keyJ {
return keyI < keyJ
}
return naturalLess(names[i], names[j])
@@ -258,7 +251,7 @@ func (src *Pkgsrc) ListVersions(category
}
func (src *Pkgsrc) checkToplevelUnusedLicenses() {
- usedLicenses := src.UsedLicenses
+ usedLicenses := G.UsedLicenses
if usedLicenses == nil {
return
}
@@ -827,6 +820,9 @@ func (src *Pkgsrc) addBuildDefs(varnames
}
}
+// IsBuildDef returns whether the given variable is automatically added
+// to BUILD_DEFS by the pkgsrc infrastructure. In such a case, the
+// package doesn't need to add the variable to BUILD_DEFS itself.
func (src *Pkgsrc) IsBuildDef(varname string) bool {
return src.buildDefs[varname]
}
Index: pkgsrc/pkgtools/pkglint/files/check_test.go
diff -u pkgsrc/pkgtools/pkglint/files/check_test.go:1.33 pkgsrc/pkgtools/pkglint/files/check_test.go:1.34
--- pkgsrc/pkgtools/pkglint/files/check_test.go:1.33 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/check_test.go Thu Feb 21 22:49:03 2019
@@ -301,6 +301,15 @@ func (t *Tester) SetUpPackage(pkgpath st
PlistRcsID,
"bin/program")
+ // Because the package Makefile includes this file, the check for the
+ // correct ordering of variables is skipped. As of February 2019, the
+ // SetupPackage function does not insert the custom variables in the
+ // correct position. To prevent the tests from having to mention the
+ // unrelated warnings about the variable order, that check is suppressed
+ // here.
+ t.CreateFileLines(pkgpath+"/suppress-varorder.mk",
+ MkRcsID)
+
// This distinfo file contains dummy hashes since pkglint cannot check the
// distfiles hashes anyway. It can only check the hashes for the patches.
t.CreateFileLines(pkgpath+"/distinfo",
@@ -323,7 +332,8 @@ func (t *Tester) SetUpPackage(pkgpath st
"HOMEPAGE=\t# none",
"COMMENT=\tDummy package",
"LICENSE=\t2-clause-bsd",
- ""}
+ "",
+ ".include \"suppress-varorder.mk\""}
for len(mlines) < 19 {
mlines = append(mlines, "# empty")
}
@@ -665,9 +675,6 @@ func (t *Tester) CheckOutputLines(expect
//
// This is useful when stepping through the code, especially
// in combination with SetUpCommandLine("--debug").
-//
-// In JetBrains GoLand, the tracing output is suppressed after the first
-// failed check, see https://youtrack.jetbrains.com/issue/GO-6154.
func (t *Tester) EnableTracing() {
G.out = NewSeparatorWriter(io.MultiWriter(os.Stdout, &t.stdout))
trace.Out = os.Stdout
@@ -677,8 +684,9 @@ func (t *Tester) EnableTracing() {
// EnableTracingToLog enables the tracing and writes the tracing output
// to the test log that can be examined with Tester.Output.
func (t *Tester) EnableTracingToLog() {
- t.EnableTracing()
+ G.out = NewSeparatorWriter(&t.stdout)
trace.Out = &t.stdout
+ trace.Tracing = true
}
// EnableSilentTracing enables tracing mode but discards any tracing output.
Index: pkgsrc/pkgtools/pkglint/files/distinfo.go
diff -u pkgsrc/pkgtools/pkglint/files/distinfo.go:1.27 pkgsrc/pkgtools/pkglint/files/distinfo.go:1.28
--- pkgsrc/pkgtools/pkglint/files/distinfo.go:1.27 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/distinfo.go Thu Feb 21 22:49:03 2019
@@ -124,7 +124,7 @@ func (ck *distinfoLinesChecker) checkAlg
"",
"If the patches directory looks wrong, pkglint needs to be improved.")
- case algorithms != "SHA1, RMD160, Size" && algorithms != "SHA1, RMD160, SHA512, Size":
+ case algorithms != "SHA1, RMD160, SHA512, Size":
line.Errorf("Expected SHA1, RMD160, SHA512, Size checksums for %q, got %s.", filename, algorithms)
}
}
@@ -144,7 +144,7 @@ func (ck *distinfoLinesChecker) checkUnr
for _, file := range patchFiles {
patchName := file.Name()
if file.Mode().IsRegular() && !ck.patches[patchName] && hasPrefix(patchName, "patch-") {
- ck.distinfoLines.Errorf("patch %q is not recorded. Run %q.",
+ ck.distinfoLines.Errorf("Patch %q is not recorded. Run %q.",
cleanpath(relpath(path.Dir(ck.distinfoLines.FileName), G.Pkg.File(ck.patchdir+"/"+patchName))),
bmake("makepatchsum"))
}
@@ -161,7 +161,7 @@ func (ck *distinfoLinesChecker) checkGlo
return
}
- hashes := G.Pkgsrc.Hashes
+ hashes := G.Hashes
if hashes == nil {
return
}
@@ -174,25 +174,23 @@ func (ck *distinfoLinesChecker) checkGlo
return
}
- if hashes != nil && !hasPrefix(filename, "patch-") {
- key := alg + ":" + filename
- otherHash := hashes[key]
-
- // See https://github.com/golang/go/issues/29802
- hashBytes := make([]byte, hex.DecodedLen(len(hash)))
- _, err := hex.Decode(hashBytes, []byte(hash))
- if err != nil {
- line.Errorf("The %s hash for %s contains a non-hex character.", alg, filename)
- }
-
- if otherHash != nil {
- if !bytes.Equal(otherHash.hash, hashBytes) {
- line.Errorf("The %s hash for %s is %s, which conflicts with %s in %s.",
- alg, filename, hash, hex.EncodeToString(otherHash.hash), line.RefToLocation(otherHash.location))
- }
- } else {
- hashes[key] = &Hash{hashBytes, line.Location}
+ key := alg + ":" + filename
+ otherHash := hashes[key]
+
+ // See https://github.com/golang/go/issues/29802
+ hashBytes := make([]byte, hex.DecodedLen(len(hash)))
+ _, err := hex.Decode(hashBytes, []byte(hash))
+ if err != nil {
+ line.Errorf("The %s hash for %s contains a non-hex character.", alg, filename)
+ }
+
+ if otherHash != nil {
+ if !bytes.Equal(otherHash.hash, hashBytes) {
+ line.Errorf("The %s hash for %s is %s, which conflicts with %s in %s.",
+ alg, filename, hash, hex.EncodeToString(otherHash.hash), line.RefToLocation(otherHash.location))
}
+ } else {
+ hashes[key] = &Hash{hashBytes, line.Location}
}
}
@@ -244,16 +242,3 @@ func computePatchSha1Hex(patchFilename s
}
return sprintf("%x", hasher.Sum(nil)), nil
}
-
-func AutofixDistinfo(oldSha1, newSha1 string) {
- distinfoFilename := G.Pkg.File(G.Pkg.DistinfoFile)
- if lines := Load(distinfoFilename, NotEmpty|LogErrors); lines != nil {
- for _, line := range lines.Lines {
- fix := line.Autofix()
- fix.Warnf(SilentAutofixFormat)
- fix.Replace(oldSha1, newSha1)
- fix.Apply()
- }
- SaveAutofixChanges(lines)
- }
-}
Index: pkgsrc/pkgtools/pkglint/files/distinfo_test.go
diff -u pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.24 pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.25
--- pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.24 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/distinfo_test.go Thu Feb 21 22:49:03 2019
@@ -34,6 +34,65 @@ func (s *Suite) Test_CheckLinesDistinfo(
"WARN: distinfo:9: Patch file \"patch-nonexistent\" does not exist in directory \"patches\".")
}
+func (s *Suite) Test_CheckLinesDistinfo__nonexistent_distfile_called_patch(c *check.C) {
+ t := s.Init(c)
+
+ t.Chdir("category/package")
+ lines := t.SetUpFileLines("distinfo",
+ RcsID,
+ "",
+ "MD5 (patch-5.3.tar.gz) = 12345678901234567890123456789012",
+ "SHA1 (patch-5.3.tar.gz) = 1234567890123456789012345678901234567890")
+ G.Pkg = NewPackage(".")
+
+ CheckLinesDistinfo(lines)
+
+ // Even though the filename starts with "patch-" and therefore looks like
+ // a patch, it is a normal distfile because it has other hash algorithms
+ // than exactly SHA1.
+ t.CheckOutputLines(
+ "ERROR: distinfo:EOF: Expected SHA1, RMD160, SHA512, Size checksums " +
+ "for \"patch-5.3.tar.gz\", got MD5, SHA1.")
+}
+
+func (s *Suite) Test_CheckLinesDistinfo__wrong_distfile_algorithms(c *check.C) {
+ t := s.Init(c)
+
+ t.Chdir("category/package")
+ lines := t.SetUpFileLines("distinfo",
+ RcsID,
+ "",
+ "MD5 (distfile.tar.gz) = 12345678901234567890123456789012",
+ "SHA1 (distfile.tar.gz) = 1234567890123456789012345678901234567890")
+
+ CheckLinesDistinfo(lines)
+
+ t.CheckOutputLines(
+ "ERROR: distinfo:EOF: Expected SHA1, RMD160, SHA512, Size checksums " +
+ "for \"distfile.tar.gz\", got MD5, SHA1.")
+}
+
+func (s *Suite) Test_CheckLinesDistinfo__wrong_patch_algorithms(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package")
+ t.Chdir("category/package")
+ t.CreateFileDummyPatch("patches/patch-aa")
+ t.SetUpFileLines("distinfo",
+ RcsID,
+ "",
+ "MD5 (patch-aa) = 12345678901234567890123456789012",
+ "SHA1 (patch-aa) = 1234567890123456789012345678901234567890")
+
+ G.Check(".")
+
+ t.CheckOutputLines(
+ "ERROR: distinfo:4: SHA1 hash of patches/patch-aa differs "+
+ "(distinfo has 1234567890123456789012345678901234567890, "+
+ "patch file has ebbf34b0641bcb508f17d5a27f2bf2a536d810ac).",
+ "ERROR: distinfo:EOF: Expected SHA1 hash for patch-aa, got MD5, SHA1.")
+}
+
// When checking the complete pkgsrc tree, pkglint has all information it needs
// to check whether different packages use the same distfile but require
// different hashes for it.
@@ -94,8 +153,8 @@ func (s *Suite) Test_distinfoLinesChecke
"7 errors and 1 warning found.")
// Ensure that hex.DecodeString does not waste memory here.
- t.Check(len(G.Pkgsrc.Hashes["SHA512:distfile-1.0.tar.gz"].hash), equals, 8)
- t.Check(cap(G.Pkgsrc.Hashes["SHA512:distfile-1.0.tar.gz"].hash), equals, 8)
+ t.Check(len(G.Hashes["SHA512:distfile-1.0.tar.gz"].hash), equals, 8)
+ t.Check(cap(G.Hashes["SHA512:distfile-1.0.tar.gz"].hash), equals, 8)
}
func (s *Suite) Test_CheckLinesDistinfo__uncommitted_patch(c *check.C) {
@@ -136,8 +195,8 @@ func (s *Suite) Test_CheckLinesDistinfo_
G.checkdirPackage(".")
t.CheckOutputLines(
- "ERROR: distinfo: patch \"patches/patch-aa\" is not recorded. Run \""+confMake+" makepatchsum\".",
- "ERROR: distinfo: patch \"patches/patch-src-Makefile\" is not recorded. Run \""+confMake+" makepatchsum\".")
+ "ERROR: distinfo: Patch \"patches/patch-aa\" is not recorded. Run \""+confMake+" makepatchsum\".",
+ "ERROR: distinfo: Patch \"patches/patch-src-Makefile\" is not recorded. Run \""+confMake+" makepatchsum\".")
}
// The distinfo file and the patches are usually placed in the package
@@ -167,7 +226,7 @@ func (s *Suite) Test_CheckLinesDistinfo_
"(distinfo has ..., patch file has ebbf34b0641bcb508f17d5a27f2bf2a536d810ac).",
"WARN: ../../other/common/distinfo:4: Patch file \"patch-only-in-distinfo\" "+
"does not exist in directory \"../../devel/patches/patches\".",
- "ERROR: ../../other/common/distinfo: patch \"../../devel/patches/patches/patch-only-in-patches\" "+
+ "ERROR: ../../other/common/distinfo: Patch \"../../devel/patches/patches/patch-only-in-patches\" "+
"is not recorded. Run \""+confMake+" makepatchsum\".")
}
@@ -197,7 +256,7 @@ func (s *Suite) Test_CheckLinesDistinfo_
"(distinfo has ..., patch file has ebbf34b0641bcb508f17d5a27f2bf2a536d810ac).",
"WARN: ../../other/common/distinfo:4: Patch file \"patch-only-in-distinfo\" "+
"does not exist in directory \"patches\".",
- "ERROR: ../../other/common/distinfo: patch \"patches/patch-only-in-patches\" "+
+ "ERROR: ../../other/common/distinfo: Patch \"patches/patch-only-in-patches\" "+
"is not recorded. Run \""+confMake+" makepatchsum\".")
}
@@ -281,6 +340,26 @@ func (s *Suite) Test_CheckLinesDistinfo_
t.CheckOutputEmpty()
}
+func (s *Suite) Test_distinfoLinesChecker_checkUncommittedPatch(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package")
+ t.Chdir("category/package")
+ t.CreateFileDummyPatch("patches/patch-aa")
+ t.CreateFileLines("CVS/Entries",
+ "/distinfo/...")
+ t.CreateFileLines("patches/CVS/Entries",
+ "/patch-aa/...")
+ t.SetUpFileLines("distinfo",
+ RcsID,
+ "",
+ "SHA1 (patch-aa) = ebbf34b0641bcb508f17d5a27f2bf2a536d810ac")
+
+ G.checkdirPackage(".")
+
+ t.CheckOutputEmpty()
+}
+
func (s *Suite) Test_distinfoLinesChecker_checkPatchSha1(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.24 pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.25
--- pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.24 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go Thu Feb 21 22:49:03 2019
@@ -34,18 +34,22 @@ func (s *Suite) Test_MkLineChecker_Check
t.SetUpVartypes()
t.CreateFileLines("mk/bsd.prefs.mk")
+ t.CreateFileLines("mk/bsd.fast.prefs.mk")
mklines := t.SetUpFileMkLines("category/package/buildlink3.mk",
- ".include \"../../mk/bsd.prefs.mk\"")
+ MkRcsID,
+ ".include \"../../mk/bsd.prefs.mk\"",
+ ".include \"../../mk/bsd.fast.prefs.mk\"")
+
// If the buildlink3.mk file doesn't actually exist, resolving the
// relative path fails since that depends on the actual file system,
// not on syntactical paths; see os.Stat in CheckRelativePath.
//
// TODO: Refactor relpath to be independent of a filesystem.
- MkLineChecker{mklines.mklines[0]}.Check()
+ mklines.Check()
t.CheckOutputLines(
- "NOTE: ~/category/package/buildlink3.mk:1: For efficiency reasons, " +
+ "NOTE: ~/category/package/buildlink3.mk:2: For efficiency reasons, " +
"please include bsd.fast.prefs.mk instead of bsd.prefs.mk.")
}
@@ -130,7 +134,7 @@ func (s *Suite) Test_MkLineChecker_check
"",
".for var in a b c",
".endfor",
- ".undef var")
+ ".undef var unrelated")
mklines.Check()
@@ -176,6 +180,67 @@ func (s *Suite) Test_MkLineChecker_check
"ERROR: filename.mk:12: Invalid variable name \"${VAR}\".")
}
+func (s *Suite) Test_MkLineChecker_checkDirectiveEnd__ending_comments(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpVartypes()
+ mklines := t.NewMkLines("opsys.mk",
+ MkRcsID,
+ "",
+ ".for i in 1 2 3 4 5",
+ ". if ${OPSYS} == NetBSD",
+ ". if ${MACHINE_ARCH} == x86_64",
+ ". if ${OS_VERSION:M8.*}",
+ ". endif # MACHINE_ARCH", // Wrong, should be OS_VERSION.
+ ". endif # OS_VERSION", // Wrong, should be MACHINE_ARCH.
+ ". endif # OPSYS", // Correct.
+ ".endfor # j", // Wrong, should be i.
+ "",
+ ".if ${PKG_OPTIONS:Moption}",
+ ".endif # option", // Correct.
+ "",
+ ".if ${PKG_OPTIONS:Moption}",
+ ".endif # opti", // This typo goes unnoticed since "opti" is a substring of the condition.
+ "",
+ ".if ${OPSYS} == NetBSD",
+ ".elif ${OPSYS} == FreeBSD",
+ ".endif # NetBSD", // Wrong, should be FreeBSD from the .elif.
+ "",
+ ".for ii in 1 2",
+ ". for jj in 1 2",
+ ". endfor # ii", // Note: a simple "i" would not generate a warning because it is found in the word "in".
+ ".endfor # ii")
+
+ // See MkLineChecker.checkDirective
+ mklines.Check()
+
+ t.CheckOutputLines(
+ "WARN: opsys.mk:7: Comment \"MACHINE_ARCH\" does not match condition \"${OS_VERSION:M8.*}\".",
+ "WARN: opsys.mk:8: Comment \"OS_VERSION\" does not match condition \"${MACHINE_ARCH} == x86_64\".",
+ "WARN: opsys.mk:10: Comment \"j\" does not match loop \"i in 1 2 3 4 5\".",
+ "WARN: opsys.mk:12: Unknown option \"option\".",
+ "WARN: opsys.mk:20: Comment \"NetBSD\" does not match condition \"${OPSYS} == FreeBSD\".",
+ "WARN: opsys.mk:24: Comment \"ii\" does not match loop \"jj in 1 2\".")
+}
+
+func (s *Suite) Test_MkLineChecker_checkDirectiveFor(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPkgsrc()
+ t.CreateFileLines("mk/file.mk",
+ MkRcsID,
+ ".for i = 1 2 3", // The "=" should rather be "in".
+ ".endfor",
+ "",
+ ".for _i_ in 1 2 3", // Underscores are only allowed in infrastructure files.
+ ".endfor")
+
+ G.Check(t.File("mk/file.mk"))
+
+ // Pkglint doesn't care about trivial syntax errors, bmake will already catch these.
+ t.CheckOutputEmpty()
+}
+
func (s *Suite) Test_MkLineChecker_checkDependencyRule(c *check.C) {
t := s.Init(c)
@@ -200,7 +265,6 @@ func (s *Suite) Test_MkLineChecker_check
func (s *Suite) Test_MkLineChecker_checkVartype__simple_type(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Wtypes")
t.SetUpVartypes()
// Since COMMENT is defined in vardefs.go its type is certain instead of guessed.
@@ -229,21 +293,6 @@ func (s *Suite) Test_MkLineChecker_check
t.CheckOutputEmpty()
}
-// The command line option -Wno-types can be used to suppress the type checks.
-// Suppressing it is rarely needed and comes from Feb 12 2005 when this feature was introduced.
-// Since then the type system has matured and proven effective.
-func (s *Suite) Test_MkLineChecker_checkVartype__skip(c *check.C) {
- t := s.Init(c)
-
- t.SetUpCommandLine("-Wno-types")
- t.SetUpVartypes()
- mkline := t.NewMkLine("filename", 1, "DISTNAME=invalid:::distname")
-
- MkLineChecker{mkline}.Check()
-
- t.CheckOutputEmpty()
-}
-
func (s *Suite) Test_MkLineChecker_checkVartype__append_to_non_list(c *check.C) {
t := s.Init(c)
@@ -279,7 +328,6 @@ func (s *Suite) Test_MkLineChecker_check
func (s *Suite) Test_MkLineChecker_checkDirectiveCond(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Wtypes")
t.SetUpVartypes()
test := func(cond string, output ...string) {
@@ -296,20 +344,28 @@ func (s *Suite) Test_MkLineChecker_check
"{ ccache ccc clang distcc f2c gcc hp icc ido "+
"mipspro mipspro-ucode pcc sunpro xlc } for PKGSRC_COMPILER.")
- test(".elif ${A} != ${B}")
+ test(".elif ${A} != ${B}",
+ "WARN: filename:1: A is used but not defined.",
+ "WARN: filename:1: B is used but not defined.")
test(".if ${HOMEPAGE} == \"mailto:someone%example.org@localhost\"";,
- "WARN: filename:1: \"mailto:someone%example.org@localhost\"; is not a valid URL.")
+ "WARN: filename:1: \"mailto:someone%example.org@localhost\"; is not a valid URL.",
+ "WARN: filename:1: HOMEPAGE should not be evaluated at load time.",
+ "WARN: filename:1: HOMEPAGE may not be used in any file; it is a write-only variable.")
test(".if !empty(PKGSRC_RUN_TEST:M[Y][eE][sS])",
"WARN: filename:1: PKGSRC_RUN_TEST should be matched "+
"against \"[yY][eE][sS]\" or \"[nN][oO]\", not \"[Y][eE][sS]\".")
- test(".if !empty(IS_BUILTIN.Xfixes:M[yY][eE][sS])")
+ test(".if !empty(IS_BUILTIN.Xfixes:M[yY][eE][sS])",
+ "WARN: filename:1: IS_BUILTIN.Xfixes should not be evaluated at load time.",
+ "WARN: filename:1: IS_BUILTIN.Xfixes may not be used in this file; it would be ok in builtin.mk.")
test(".if !empty(${IS_BUILTIN.Xfixes:M[yY][eE][sS]})",
"WARN: filename:1: The empty() function takes a variable name as parameter, "+
- "not a variable expression.")
+ "not a variable expression.",
+ "WARN: filename:1: IS_BUILTIN.Xfixes should not be evaluated at load time.",
+ "WARN: filename:1: IS_BUILTIN.Xfixes may not be used in this file; it would be ok in builtin.mk.")
test(".if ${PKGSRC_COMPILER} == \"msvc\"",
"WARN: filename:1: \"msvc\" is not valid for PKGSRC_COMPILER. "+
@@ -317,7 +373,9 @@ func (s *Suite) Test_MkLineChecker_check
"WARN: filename:1: Use ${PKGSRC_COMPILER:Mmsvc} instead of the == operator.")
test(".if ${PKG_LIBTOOL:Mlibtool}",
- "NOTE: filename:1: PKG_LIBTOOL should be compared using == instead of matching against \":Mlibtool\".")
+ "NOTE: filename:1: PKG_LIBTOOL should be compared using == instead of matching against \":Mlibtool\".",
+ "WARN: filename:1: PKG_LIBTOOL should not be evaluated at load time.",
+ "WARN: filename:1: PKG_LIBTOOL may not be used in any file; it is a write-only variable.")
test(".if ${MACHINE_PLATFORM:MUnknownOS-*-*} || ${MACHINE_ARCH:Mx86}",
"WARN: filename:1: "+
@@ -336,7 +394,8 @@ func (s *Suite) Test_MkLineChecker_check
"NOTE: filename:1: MACHINE_ARCH should be compared using == instead of matching against \":Mx86\".")
test(".if ${MASTER_SITES:Mftp://*} == \"ftp://netbsd.org/\"";,
- nil...)
+ "WARN: filename:1: MASTER_SITES should not be evaluated at load time.",
+ "WARN: filename:1: MASTER_SITES may not be used in any file; it is a write-only variable.")
// The only interesting line from the below tracing output is the one
// containing "checkCompareVarStr".
@@ -350,6 +409,10 @@ func (s *Suite) Test_MkLineChecker_check
"TRACE: 1 2 + (*Pkgsrc).VariableType(\"VAR\")",
"TRACE: 1 2 3 No type definition found for \"VAR\".",
"TRACE: 1 2 - (*Pkgsrc).VariableType(\"VAR\", \"=>\", (*pkglint.Vartype)(nil))",
+ "WARN: filename:1: VAR is used but not defined.",
+ "TRACE: 1 2 + MkLineChecker.checkVarusePermissions(\"VAR\", (no-type time:parse quoting:plain wordpart:false))",
+ "TRACE: 1 2 3 No type definition found for \"VAR\".",
+ "TRACE: 1 2 - MkLineChecker.checkVarusePermissions(\"VAR\", (no-type time:parse quoting:plain wordpart:false))",
"TRACE: 1 2 + (*MkLineImpl).VariableNeedsQuoting(\"VAR\", (*pkglint.Vartype)(nil), (no-type time:parse quoting:plain wordpart:false))",
"TRACE: 1 2 - (*MkLineImpl).VariableNeedsQuoting(\"VAR\", (*pkglint.Vartype)(nil), (no-type time:parse quoting:plain wordpart:false), \"=>\", unknown)",
"TRACE: 1 - MkLineChecker.CheckVaruse(filename:1, ${VAR:Mpattern1:Mpattern2}, (no-type time:parse quoting:plain wordpart:false))",
@@ -375,19 +438,37 @@ func (s *Suite) Test_MkLineChecker_check
func (s *Suite) Test_MkLineChecker_checkVarassignLeftPermissions(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Wall,no-space")
t.SetUpVartypes()
+ t.SetUpTool("awk", "AWK", AtRunTime)
mklines := t.NewMkLines("options.mk",
MkRcsID,
- "PKG_DEVELOPER?= yes",
- "BUILD_DEFS?= VARBASE")
+ "PKG_DEVELOPER?=\tyes",
+ "BUILD_DEFS?=\tVARBASE",
+ "USE_TOOLS:=\t${USE_TOOLS:Nunwanted-tool}",
+ "USE_TOOLS:=\t${MY_TOOLS}",
+ "USE_TOOLS:=\tawk")
mklines.Check()
t.CheckOutputLines(
"WARN: options.mk:2: The variable PKG_DEVELOPER may not be given a default value by any package.",
"WARN: options.mk:2: Please include \"../../mk/bsd.prefs.mk\" before using \"?=\".",
- "WARN: options.mk:3: The variable BUILD_DEFS may not be given a default value (only appended to) in this file.")
+ "WARN: options.mk:3: The variable BUILD_DEFS may not be given a default value (only appended to) in this file.",
+ "WARN: options.mk:5: The variable USE_TOOLS may not be set (only appended to) in this file.",
+ "WARN: options.mk:5: MY_TOOLS is used but not defined.",
+ "WARN: options.mk:6: The variable USE_TOOLS may not be set (only appended to) in this file.")
+}
+
+func (s *Suite) Test_MkLineChecker_checkVarassignLeftPermissions__no_tracing(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpVartypes()
+ t.DisableTracing() // Just to reach branch coverage for unknown permissions.
+ mklines := t.NewMkLines("options.mk",
+ MkRcsID,
+ "COMMENT=\tShort package description")
+
+ mklines.Check()
}
// Don't check the permissions for infrastructure files since they have their own rules.
@@ -581,6 +662,51 @@ func (s *Suite) Test_MkLineChecker_check
"WARN: any.mk:2: PKGREVISION may not be used in any file; it is a write-only variable.")
}
+func (s *Suite) Test_MkLineChecker_checkVarusePermissions__indirectly(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpVartypes()
+ mklines := t.NewMkLines("file.mk",
+ MkRcsID,
+ "IGNORE_PKG.package=\t${ONLY_FOR_UNPRIVILEGED}")
+
+ mklines.Check()
+
+ t.CheckOutputLines(
+ "WARN: file.mk:2: IGNORE_PKG.package should be set to YES or yes.",
+ "WARN: file.mk:2: ONLY_FOR_UNPRIVILEGED should not be evaluated indirectly at load time.")
+}
+
+func (s *Suite) Test_MkLineChecker_warnVaruseToolLoadTime(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpVartypes()
+ t.SetUpTool("nowhere", "NOWHERE", Nowhere)
+ t.SetUpTool("after-prefs", "AFTER_PREFS", AfterPrefsMk)
+ t.SetUpTool("at-runtime", "AT_RUNTIME", AtRunTime)
+ mklines := t.NewMkLines("Makefile",
+ MkRcsID,
+ ".if ${NOWHERE} && ${AFTER_PREFS} && ${AT_RUNTIME} && ${MK_TOOL}",
+ ".endif",
+ "",
+ "TOOLS_CREATE+=\t\tmk-tool",
+ "_TOOLS_VARNAME.mk-tool=\tMK_TOOL")
+
+ mklines.Check()
+
+ t.CheckOutputLines(
+ "WARN: Makefile:2: To use the tool ${NOWHERE} at load time, "+
+ "it has to be added to USE_TOOLS before including bsd.prefs.mk.",
+ "WARN: Makefile:2: To use the tool ${AFTER_PREFS} at load time, "+
+ "bsd.prefs.mk has to be included before.",
+ "WARN: Makefile:2: The tool ${AT_RUNTIME} cannot be used at load time.",
+ "WARN: Makefile:2: To use the tool ${MK_TOOL} at load time, "+
+ "bsd.prefs.mk has to be included before.",
+ "WARN: Makefile:6: Variable names starting with an underscore "+
+ "(_TOOLS_VARNAME.mk-tool) are reserved for internal pkgsrc use.",
+ "WARN: Makefile:6: _TOOLS_VARNAME.mk-tool is defined but not used.")
+}
+
func (s *Suite) Test_MkLineChecker_Check__warn_varuse_LOCALBASE(c *check.C) {
t := s.Init(c)
@@ -764,6 +890,17 @@ func (s *Suite) Test_MkLineChecker_check
"WARN: Makefile:2: Compiler flag \"%s\\\\\\\"\" should start with a hyphen.")
}
+func (s *Suite) Test_MkLineChecker_checkDirectiveIndentation(c *check.C) {
+ t := s.Init(c)
+
+ mkline := t.NewMkLine("filename.mk", 123, ".if 0")
+
+ // Calling this method is only useful in the context of a whole file.
+ MkLineChecker{mkline}.checkDirectiveIndentation(4)
+
+ t.CheckOutputEmpty()
+}
+
func (s *Suite) Test_MkLineChecker_checkDirectiveIndentation__autofix(c *check.C) {
t := s.Init(c)
@@ -1021,7 +1158,64 @@ func (s *Suite) Test_MkLineChecker_Check
"WARN: ~/options.mk:2: The user-defined variable VARBASE is used but not added to BUILD_DEFS.")
}
-func (s *Suite) Test_MkLineChecker_CheckVaruse__complicated_range(c *check.C) {
+// The LOCALBASE variable may be defined and used in the infrastructure.
+// It is always equivalent to PREFIX and only exists for historic reasons.
+func (s *Suite) Test_MkLineChecker_CheckVaruse__LOCALBASE_in_infrastructure(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPkgsrc()
+ t.CreateFileLines("mk/infra.mk",
+ MkRcsID,
+ "LOCALBASE?=\t${PREFIX}",
+ "DEFAULT_PREFIX=\t${LOCALBASE}")
+ G.Pkgsrc.LoadInfrastructure()
+
+ G.Check(t.File("mk/infra.mk"))
+
+ // No warnings about LOCALBASE being used; in packages LOCALBASE is deprecated.
+ t.CheckOutputLines(
+ "WARN: ~/mk/infra.mk:2: PREFIX should not be evaluated indirectly at load time.")
+}
+
+func (s *Suite) Test_MkLineChecker_CheckVaruse__user_defined_variable_and_BUILD_DEFS(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPkgsrc()
+ t.CreateFileLines("mk/defaults/mk.conf",
+ "VARBASE?=\t${PREFIX}/var",
+ "PYTHON_VER?=\t36")
+ G.Pkgsrc.LoadInfrastructure()
+ mklines := t.NewMkLines("file.mk",
+ MkRcsID,
+ "BUILD_DEFS+=\tPYTHON_VER",
+ "\t: ${VARBASE}",
+ "\t: ${VARBASE}",
+ "\t: ${PYTHON_VER}")
+
+ mklines.Check()
+
+ t.CheckOutputLines(
+ "WARN: file.mk:3: The user-defined variable VARBASE is used but not added to BUILD_DEFS.")
+}
+
+func (s *Suite) Test_MkLineChecker_checkVaruseModifiersSuffix(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpVartypes()
+ mklines := t.NewMkLines("file.mk",
+ MkRcsID,
+ "\t: ${HOMEPAGE:=subdir/:Q}", // wrong
+ "\t: ${BUILD_DIRS:=subdir/}", // correct
+ "\t: ${BIN_PROGRAMS:=.exe}") // unknown since BIN_PROGRAMS doesn't have a type
+
+ mklines.Check()
+
+ t.CheckOutputLines(
+ "WARN: file.mk:2: The :from=to modifier should only be used with lists, not with HOMEPAGE.",
+ "WARN: file.mk:4: BIN_PROGRAMS is used but not defined.")
+}
+
+func (s *Suite) Test_MkLineChecker_checkVaruseModifiersRange(c *check.C) {
t := s.Init(c)
t.SetUpCommandLine("--show-autofix", "--source")
@@ -1039,6 +1233,24 @@ func (s *Suite) Test_MkLineChecker_Check
"Replacing \":C/^/_asdf_/1:M_asdf_*:S/^_asdf_//\" with \":[1]\".",
"-\tCC:=\t${CC:C/^/_asdf_/1:M_asdf_*:S/^_asdf_//}",
"+\tCC:=\t${CC:[1]}")
+
+ // Now go through all the "almost" cases, to reach full branch coverage.
+ mklines := t.NewMkLines("gcc.mk",
+ MkRcsID,
+ "\t: ${CC:M1:M2:M3}",
+ "\t: ${CC:C/^begin//:M2:M3}", // M1 pattern not exactly ^
+ "\t: ${CC:C/^/_asdf_/g:M2:M3}", // M1 options != "1"
+ "\t: ${CC:C/^/....../g:M2:M3}", // M1 replacement doesn't match \w+
+ "\t: ${CC:C/^/_asdf_/1:O:M3}", // M2 is not a match modifier
+ "\t: ${CC:C/^/_asdf_/1:N2:M3}", // M2 is :N instead of :M
+ "\t: ${CC:C/^/_asdf_/1:M_asdf_:M3}", // M2 pattern is missing the * at the end
+ "\t: ${CC:C/^/_asdf_/1:Mother:M3}", // M2 pattern differs from the M1 pattern
+ "\t: ${CC:C/^/_asdf_/1:M_asdf_*:M3}", // M3 ist not a substitution modifier
+ "\t: ${CC:C/^/_asdf_/1:M_asdf_*:S,from,to,}", // M3 pattern differs from the M1 pattern
+ "\t: ${CC:C/^/_asdf_/1:M_asdf_*:S,^_asdf_,to,}", // M3 replacement is not empty
+ "\t: ${CC:C/^/_asdf_/1:M_asdf_*:S,^_asdf_,,g}") // M3 modifier has options
+
+ mklines.Check()
}
func (s *Suite) Test_MkLineChecker_CheckVaruse__deprecated_PKG_DEBUG(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/files.go
diff -u pkgsrc/pkgtools/pkglint/files/files.go:1.23 pkgsrc/pkgtools/pkglint/files/files.go:1.24
--- pkgsrc/pkgtools/pkglint/files/files.go:1.23 Mon Dec 17 00:15:39 2018
+++ pkgsrc/pkgtools/pkglint/files/files.go Thu Feb 21 22:49:03 2019
@@ -102,7 +102,7 @@ func nextLogicalLine(filename string, ra
func matchContinuationLine(textnl string) (leadingWhitespace, text, trailingWhitespace, cont string) {
j := len(textnl)
- if j > 0 && textnl[j-1] == '\n' {
+ if textnl[j-1] == '\n' {
j--
}
Index: pkgsrc/pkgtools/pkglint/files/mkparser.go
diff -u pkgsrc/pkgtools/pkglint/files/mkparser.go:1.23 pkgsrc/pkgtools/pkglint/files/mkparser.go:1.24
--- pkgsrc/pkgtools/pkglint/files/mkparser.go:1.23 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/mkparser.go Thu Feb 21 22:49:03 2019
@@ -27,8 +27,6 @@ func (p *MkParser) Rest() string {
//
// The text argument is assumed to be after unescaping the # character,
// which means the # is a normal character and does not introduce a Makefile comment.
-//
-// TODO: Remove the emitWarnings argument in order to separate parsing from checking.
func NewMkParser(line Line, text string, emitWarnings bool) *MkParser {
G.Assertf((line != nil) == emitWarnings, "line must be given iff emitWarnings is set")
return &MkParser{line, textproc.NewLexer(text), emitWarnings}
@@ -441,7 +439,7 @@ func (p *MkParser) mkCondAtom() MkCond {
m := lexer.NextRegexp(G.res.Compile(`^[\t ]*(<|<=|==|!=|>=|>)[\t ]*`))
if m == nil {
- return &mkCond{Not: &mkCond{Empty: lhs}} // See devel/bmake/files/cond.c:/\* For \.if \$/
+ return &mkCond{Var: lhs} // See devel/bmake/files/cond.c:/\* For \.if \$/
}
op := m[1]
@@ -697,6 +695,7 @@ type mkCond struct {
Defined string
Empty *MkVarUse
+ Var *MkVarUse
CompareVarNum *MkCondCompareVarNum
CompareVarStr *MkCondCompareVarStr
CompareVarVar *MkCondCompareVarVar
@@ -730,6 +729,7 @@ type MkCondCallback struct {
CompareVarStr func(varuse *MkVarUse, op string, str string)
CompareVarVar func(left *MkVarUse, op string, right *MkVarUse)
Call func(name string, arg string)
+ Var func(varuse *MkVarUse)
VarUse func(varuse *MkVarUse)
}
@@ -764,6 +764,14 @@ func (w *MkCondWalker) Walk(cond MkCond,
callback.VarUse(&MkVarUse{cond.Defined, nil})
}
+ case cond.Var != nil:
+ if callback.Var != nil {
+ callback.Var(cond.Var)
+ }
+ if callback.VarUse != nil {
+ callback.VarUse(cond.Var)
+ }
+
case cond.Empty != nil:
if callback.Empty != nil {
callback.Empty(cond.Empty)
Index: pkgsrc/pkgtools/pkglint/files/licenses.go
diff -u pkgsrc/pkgtools/pkglint/files/licenses.go:1.20 pkgsrc/pkgtools/pkglint/files/licenses.go:1.21
--- pkgsrc/pkgtools/pkglint/files/licenses.go:1.20 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/licenses.go Thu Feb 21 22:49:03 2019
@@ -31,8 +31,8 @@ func (lc *LicenseChecker) checkName(lice
}
if licenseFile == "" {
licenseFile = G.Pkgsrc.File("licenses/" + license)
- if G.Pkgsrc.UsedLicenses != nil {
- G.Pkgsrc.UsedLicenses[intern(license)] = true
+ if G.UsedLicenses != nil {
+ G.UsedLicenses[intern(license)] = true
}
}
Index: pkgsrc/pkgtools/pkglint/files/line.go
diff -u pkgsrc/pkgtools/pkglint/files/line.go:1.32 pkgsrc/pkgtools/pkglint/files/line.go:1.33
--- pkgsrc/pkgtools/pkglint/files/line.go:1.32 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/line.go Thu Feb 21 22:49:03 2019
@@ -19,9 +19,16 @@ import (
)
type RawLine struct {
- Lineno int // Counting starts at 1; 0 means inserted by Autofix
- orignl string // The line as read in from the file, including newline
- textnl string // The line as modified by Autofix, including newline
+ 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.
@@ -145,9 +152,7 @@ func (line *LineImpl) showSource(out *Se
for _, rawLine := range rawLines {
if rawLine.textnl != rawLine.orignl {
- if rawLine.orignl != "" {
- writeLine("-\t", rawLine.orignl)
- }
+ writeLine("-\t", rawLine.orignl)
if rawLine.textnl != "" {
writeLine("+\t", rawLine.textnl)
}
Index: pkgsrc/pkgtools/pkglint/files/pkglint_test.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.32 pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.33
--- pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.32 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/pkglint_test.go Thu Feb 21 22:49:03 2019
@@ -38,35 +38,19 @@ func (s *Suite) Test_Pkglint_Main__help(
" -W, --warning=warning,... enable or disable groups of warnings",
"",
" Flags for -C, --check:",
- " all all of the following",
- " none none of the following",
- " ALTERNATIVES check ALTERNATIVES files (enabled)",
- " bl3 check buildlink3.mk files (enabled)",
- " DESCR check DESCR file (enabled)",
- " distinfo check distinfo file (enabled)",
- " extra check various additional files (disabled)",
- " global inter-package checks (disabled)",
- " INSTALL check INSTALL and DEINSTALL scripts (enabled)",
- " Makefile check Makefiles (enabled)",
- " MESSAGE check MESSAGE file (enabled)",
- " mk check other .mk files (enabled)",
- " options check options.mk files (enabled)",
- " patches check patches (enabled)",
- " PLIST check PLIST files (enabled)",
+ " all all of the following",
+ " none none of the following",
+ " extra check various additional files (disabled)",
+ " global inter-package checks (disabled)",
"",
" Flags for -W, --warning:",
- " all all of the following",
- " none none of the following",
- " directcmd warn about use of direct command names instead of Make variables (enabled)",
- " extra enable some extra warnings (disabled)",
- " order warn if Makefile entries are unordered (enabled)",
- " perm warn about unforeseen variable definition and use (disabled)",
- " plist-depr warn about deprecated paths in PLISTs (disabled)",
- " plist-sort warn about unsorted entries in PLISTs (disabled)",
- " quoting warn about quoting issues (disabled)",
- " space warn about inconsistent use of whitespace (disabled)",
- " style warn about stylistic issues (disabled)",
- " types do some simple type checking in Makefiles (enabled)",
+ " all all of the following",
+ " none none of the following",
+ " extra enable some extra warnings (disabled)",
+ " perm warn about unforeseen variable definition and use (disabled)",
+ " quoting warn about quoting issues (disabled)",
+ " space warn about inconsistent use of whitespace (disabled)",
+ " style warn about stylistic issues (disabled)",
"",
" (Prefix a flag with \"no-\" to disable it.)")
}
Index: pkgsrc/pkgtools/pkglint/files/plist_test.go
diff -u pkgsrc/pkgtools/pkglint/files/plist_test.go:1.32 pkgsrc/pkgtools/pkglint/files/plist_test.go:1.33
--- pkgsrc/pkgtools/pkglint/files/plist_test.go:1.32 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/plist_test.go Thu Feb 21 22:49:03 2019
@@ -48,6 +48,23 @@ func (s *Suite) Test_CheckLinesPlist(c *
"ERROR: PLIST:18: Duplicate filename \"share/tzinfo\", already appeared in line 17.")
}
+// When a PLIST contains multiple libtool libraries, USE_LIBTOOL needs only
+// be defined once in the package Makefile. Therefore, a single warning is enough.
+func (s *Suite) Test_CheckLinesPlist__multiple_libtool_libraries(c *check.C) {
+ t := s.Init(c)
+
+ G.Pkg = NewPackage(t.File("category/pkgbase"))
+ lines := t.NewLines("PLIST",
+ PlistRcsID,
+ "lib/libc.la",
+ "lib/libm.la")
+
+ CheckLinesPlist(lines)
+
+ t.CheckOutputLines(
+ "WARN: PLIST:2: Packages that install libtool libraries should define USE_LIBTOOL.")
+}
+
func (s *Suite) Test_CheckLinesPlist__empty(c *check.C) {
t := s.Init(c)
@@ -92,7 +109,6 @@ func (s *Suite) Test_CheckLinesPlist__co
func (s *Suite) Test_CheckLinesPlist__sorting(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Wplist-sort")
lines := t.NewLines("PLIST",
PlistRcsID,
"@comment Do not remove",
@@ -471,7 +487,7 @@ func (s *Suite) Test_PlistChecker_checkP
CheckLinesPlist(lines)
t.CheckOutputLines(
- "WARN: ~/PLIST:2: perllocal.pod files should not be in the PLIST.",
+ "WARN: ~/PLIST:2: The perllocal.pod file should not be in the PLIST.",
"WARN: ~/PLIST:3: CVS files should not be in the PLIST.",
"WARN: ~/PLIST:4: .orig files should not be in the PLIST.")
}
@@ -560,7 +576,7 @@ func (s *Suite) Test_PlistLine_CheckTrai
CheckLinesPlist(lines)
t.CheckOutputLines(
- "ERROR: ~/PLIST:2: pkgsrc does not support filenames ending in whitespace.")
+ "ERROR: ~/PLIST:2: Pkgsrc does not support filenames ending in whitespace.")
}
func (s *Suite) Test_PlistLine_CheckDirective(c *check.C) {
@@ -580,7 +596,7 @@ func (s *Suite) Test_PlistLine_CheckDire
t.CheckOutputLines(
"WARN: ~/PLIST:2: Please remove this line. It is no longer necessary.",
- "ERROR: ~/PLIST:3: ldconfig must be used with \"||/usr/bin/true\".",
+ "ERROR: ~/PLIST:3: The ldconfig command must be used with \"||/usr/bin/true\".",
"WARN: ~/PLIST:5: @dirrm is obsolete. Please remove this line.",
"WARN: ~/PLIST:6: Invalid number of arguments for imake-man, should be 3.",
"WARN: ~/PLIST:7: IMAKE_MANNEWSUFFIX is not meant to appear in PLISTs.",
Index: pkgsrc/pkgtools/pkglint/files/linelexer.go
diff -u pkgsrc/pkgtools/pkglint/files/linelexer.go:1.1 pkgsrc/pkgtools/pkglint/files/linelexer.go:1.2
--- pkgsrc/pkgtools/pkglint/files/linelexer.go:1.1 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/linelexer.go Thu Feb 21 22:49:03 2019
@@ -1,9 +1,6 @@
package pkglint
-import (
- "netbsd.org/pkglint/regex"
- "strings"
-)
+import "netbsd.org/pkglint/regex"
// LinesLexer records the state when checking a list of lines from top to bottom.
type LinesLexer struct {
@@ -66,7 +63,11 @@ func (llex *LinesLexer) SkipPrefix(prefi
defer trace.Call2(llex.CurrentLine().Text, prefix)()
}
- return !llex.EOF() && strings.HasPrefix(llex.lines.Lines[llex.index].Text, prefix) && llex.Skip()
+ if !llex.EOF() && hasPrefix(llex.lines.Lines[llex.index].Text, prefix) {
+ llex.Skip()
+ return true
+ }
+ return false
}
func (llex *LinesLexer) SkipString(text string) bool {
@@ -74,7 +75,11 @@ func (llex *LinesLexer) SkipString(text
defer trace.Call2(llex.CurrentLine().Text, text)()
}
- return !llex.EOF() && llex.lines.Lines[llex.index].Text == text && llex.Skip()
+ if !llex.EOF() && llex.lines.Lines[llex.index].Text == text {
+ llex.Skip()
+ return true
+ }
+ return false
}
func (llex *LinesLexer) SkipEmptyOrNote() bool {
@@ -135,5 +140,9 @@ func (mlex *MkLinesLexer) SkipWhile(pred
}
func (mlex *MkLinesLexer) SkipIf(pred func(mkline MkLine) bool) bool {
- return !mlex.EOF() && pred(mlex.CurrentMkLine()) && mlex.Skip()
+ if !mlex.EOF() && pred(mlex.CurrentMkLine()) {
+ mlex.Skip()
+ return true
+ }
+ return false
}
Index: pkgsrc/pkgtools/pkglint/files/linelexer_test.go
diff -u pkgsrc/pkgtools/pkglint/files/linelexer_test.go:1.1 pkgsrc/pkgtools/pkglint/files/linelexer_test.go:1.2
--- pkgsrc/pkgtools/pkglint/files/linelexer_test.go:1.1 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/linelexer_test.go Thu Feb 21 22:49:03 2019
@@ -34,3 +34,18 @@ func (s *Suite) Test_LinesLexer_SkipEmpt
t.CheckOutputLines(
"NOTE: file.txt:2: Empty line expected after this line.")
}
+
+func (s *Suite) Test_LinesLexer_SkipPrefix(c *check.C) {
+ t := s.Init(c)
+
+ lines := t.NewLines("file.txt",
+ "line 1",
+ "line 2")
+ llex := NewLinesLexer(lines)
+
+ t.Check(llex.SkipPrefix("line 1"), equals, true)
+ t.Check(llex.SkipPrefix("line 1"), equals, false)
+ t.Check(llex.SkipPrefix("line 2"), equals, true)
+ t.Check(llex.SkipPrefix("line 2"), equals, false)
+ t.Check(llex.SkipPrefix(""), equals, false)
+}
Index: pkgsrc/pkgtools/pkglint/files/lines_test.go
diff -u pkgsrc/pkgtools/pkglint/files/lines_test.go:1.7 pkgsrc/pkgtools/pkglint/files/lines_test.go:1.8
--- pkgsrc/pkgtools/pkglint/files/lines_test.go:1.7 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/lines_test.go Thu Feb 21 22:49:03 2019
@@ -60,4 +60,15 @@ func (s *Suite) Test_Lines_CheckRcsID__w
"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.")
+
+ // In production mode, this error is disabled since it doesn't provide
+ // enough benefit compared to the work it would produce.
+ //
+ // To make it useful, the majority of pkgsrc-wip packages would first
+ // have to follow this style.
+ G.Testing = false
+
+ G.Check(t.File("wip/package"))
+
+ t.CheckOutputEmpty()
}
Index: pkgsrc/pkgtools/pkglint/files/logging_test.go
diff -u pkgsrc/pkgtools/pkglint/files/logging_test.go:1.11 pkgsrc/pkgtools/pkglint/files/logging_test.go:1.12
--- pkgsrc/pkgtools/pkglint/files/logging_test.go:1.11 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/logging_test.go Thu Feb 21 22:49:03 2019
@@ -2,6 +2,7 @@ package pkglint
import (
"gopkg.in/check.v1"
+ "netbsd.org/pkglint/histogram"
"strings"
)
@@ -52,6 +53,67 @@ func (s *Suite) Test_Logger_Logf__mixed_
"ERROR: filename:3: Logf output 3.\n")
}
+func (s *Suite) Test_Logger_Logf__production(c *check.C) {
+ var sw strings.Builder
+ logger := Logger{out: NewSeparatorWriter(&sw)}
+
+ // In production mode, the checks for the diagnostic messages are
+ // turned off, for performance reasons. The unit tests provide
+ // enough coverage.
+ G.Testing = false
+ logger.Logf(Error, "filename", "3", "diagnostic", "message")
+
+ c.Check(sw.String(), equals, ""+
+ "ERROR: filename:3: message\n")
+}
+
+func (s *Suite) Test_Logger_Logf__profiling(c *check.C) {
+ t := s.Init(c)
+
+ line := t.NewLine("filename", 123, "text")
+
+ G.Opts.Profiling = true
+ G.Logger.histo = histogram.New()
+ line.Warnf("Warning.")
+
+ G.Logger.histo.PrintStats(G.out.out, "loghisto", -1)
+
+ t.CheckOutputLines(
+ "WARN: filename:123: Warning.",
+ "loghisto 1 Warning.")
+}
+
+func (s *Suite) Test_Logger_Logf__profiling_autofix(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("--show-autofix", "--source", "--explain")
+ line := t.NewLine("filename", 123, "text")
+
+ G.Opts.Profiling = true
+ G.Logger.histo = histogram.New()
+
+ fix := line.Autofix()
+ fix.Notef("Autofix note.")
+ fix.Explain(
+ "Autofix explanation.")
+ fix.Replace("text", "replacement")
+ fix.Apply()
+
+ // The AUTOFIX line is not counted in the histogram although
+ // it uses the same code path as the other messages.
+ G.Logger.histo.PrintStats(G.out.out, "loghisto", -1)
+
+ t.CheckOutputLines(
+ "NOTE: filename:123: Autofix note.",
+ "AUTOFIX: filename:123: Replacing \"text\" with \"replacement\".",
+ "-\ttext",
+ "+\treplacement",
+ "",
+ "\tAutofix explanation.",
+ "",
+ "loghisto 1 Autofix note.")
+}
+
// Diag filters duplicate messages, unlike Logf.
func (s *Suite) Test_Logger_Diag__duplicates(c *check.C) {
t := s.Init(c)
@@ -686,14 +748,17 @@ func (s *Suite) Test_Logger_Diag__source
G.Main("pkglint", "--source", "-Wall", t.File("category/package1"), t.File("category/package2"))
t.CheckOutputLines(
- "ERROR: ~/category/package1/distinfo: patch \"../dependency/patches/patch-aa\" "+
- "is not recorded. Run \""+confMake+" makepatchsum\".",
+ "ERROR: ~/category/package1/distinfo: "+
+ "Patch \"../dependency/patches/patch-aa\" is not recorded. "+
+ "Run \""+confMake+" makepatchsum\".",
"",
">\t--- old file",
- "ERROR: ~/category/dependency/patches/patch-aa:3: Each patch must be documented.",
+ "ERROR: ~/category/dependency/patches/patch-aa:3: "+
+ "Each patch must be documented.",
"",
- "ERROR: ~/category/package2/distinfo: patch \"../dependency/patches/patch-aa\" "+
- "is not recorded. Run \""+confMake+" makepatchsum\".",
+ "ERROR: ~/category/package2/distinfo: "+
+ "Patch \"../dependency/patches/patch-aa\" is not recorded. "+
+ "Run \""+confMake+" makepatchsum\".",
"",
"3 errors and 0 warnings found.",
"(Run \"pkglint -e\" to show explanations.)")
Index: pkgsrc/pkgtools/pkglint/files/options.go
diff -u pkgsrc/pkgtools/pkglint/files/options.go:1.11 pkgsrc/pkgtools/pkglint/files/options.go:1.12
--- pkgsrc/pkgtools/pkglint/files/options.go:1.11 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/options.go Thu Feb 21 22:49:03 2019
@@ -73,18 +73,20 @@ loop:
continue
}
- cond.Walk(&MkCondCallback{
- Empty: func(varuse *MkVarUse) {
- if varuse.varname == "PKG_OPTIONS" && len(varuse.modifiers) == 1 {
- if m, positive, pattern := varuse.modifiers[0].MatchMatch(); m && positive {
- option := pattern
- if !containsVarRef(option) {
- handledOptions[option] = mkline
- optionsInDeclarationOrder = append(optionsInDeclarationOrder, option)
- }
+ recordUsedOption := func(varuse *MkVarUse) {
+ if varuse.varname == "PKG_OPTIONS" && len(varuse.modifiers) == 1 {
+ if m, positive, pattern := varuse.modifiers[0].MatchMatch(); m && positive {
+ option := pattern
+ if !containsVarRef(option) {
+ handledOptions[option] = mkline
+ optionsInDeclarationOrder = append(optionsInDeclarationOrder, option)
}
}
- }})
+ }
+ }
+ cond.Walk(&MkCondCallback{
+ Empty: recordUsedOption,
+ Var: recordUsedOption})
if cond.Empty != nil && mkline.HasElseBranch() {
mkline.Notef("The positive branch of the .if/.else should be the one where the option is set.")
@@ -101,14 +103,14 @@ loop:
declared := declaredOptions[option]
handled := handledOptions[option]
- if handled == nil {
+ switch {
+ case handled == nil:
declared.Warnf("Option %q should be handled below in an .if block.", option)
G.Explain(
"If an option is not processed in this file, it may either be a",
"typo, or the option does not have any effect.")
- }
- if declared == nil {
+ case declared == nil:
handled.Warnf("Option %q is handled but not added to PKG_SUPPORTED_OPTIONS.", option)
G.Explain(
"This block of code will never be run since PKG_OPTIONS cannot",
@@ -117,5 +119,5 @@ loop:
}
}
- SaveAutofixChanges(mklines.lines)
+ mklines.SaveAutofixChanges()
}
Index: pkgsrc/pkgtools/pkglint/files/mkline.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline.go:1.45 pkgsrc/pkgtools/pkglint/files/mkline.go:1.46
--- pkgsrc/pkgtools/pkglint/files/mkline.go:1.45 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/mkline.go Thu Feb 21 22:49:03 2019
@@ -354,10 +354,18 @@ func (mkline *MkLineImpl) Tokenize(text
defer trace.Call(mkline, text)()
}
- p := NewMkParser(mkline.Line, text, true)
- tokens := p.MkTokens()
- if warn && p.Rest() != "" {
- mkline.Warnf("Internal pkglint error in MkLine.Tokenize at %q.", p.Rest())
+ var tokens []*MkToken
+ var rest string
+ if (mkline.IsVarassign() || mkline.IsCommentedVarassign()) && text == mkline.Value() {
+ tokens, rest = mkline.ValueTokens()
+ } else {
+ p := NewMkParser(mkline.Line, text, true)
+ tokens = p.MkTokens()
+ rest = p.Rest()
+ }
+
+ if warn && rest != "" {
+ mkline.Warnf("Internal pkglint error in MkLine.Tokenize at %q.", rest)
}
return tokens
}
@@ -458,21 +466,21 @@ func (mkline *MkLineImpl) ValueFields(va
return split
}
-func (mkline *MkLineImpl) ValueTokens() []*MkToken {
+func (mkline *MkLineImpl) ValueTokens() ([]*MkToken, string) {
value := mkline.Value()
if value == "" {
- return nil
+ return nil, ""
}
assign := mkline.data.(mkLineAssign)
if assign.valueMk != nil || assign.valueMkRest != "" {
- return assign.valueMk
+ return assign.valueMk, assign.valueMkRest
}
p := NewMkParser(mkline.Line, value, true)
assign.valueMk = p.MkTokens()
assign.valueMkRest = p.Rest()
- return assign.valueMk
+ return assign.valueMk, assign.valueMkRest
}
// Fields applies to variable assignments and .for loops.
@@ -528,22 +536,23 @@ func (mkline *MkLineImpl) ResolveVarsInR
} else {
basedir = path.Dir(mkline.Filename)
}
- pkgsrcdir := relpath(basedir, G.Pkgsrc.File("."))
-
- if G.Testing {
- // Relative pkgsrc paths usually only contain two or three levels.
- // A possible reason for reaching this assertion is:
- // Tests that access the file system must create their lines
- // using t.SetUpFileMkLines, not using t.NewMkLines.
- G.Assertf(!contains(pkgsrcdir, "../../../../.."),
- "Relative path %q for %q is too deep below the pkgsrc root %q.",
- pkgsrcdir, basedir, G.Pkgsrc.File("."))
- }
tmp := relativePath
- tmp = strings.Replace(tmp, "${PKGSRCDIR}", pkgsrcdir, -1)
- tmp = strings.Replace(tmp, "${.CURDIR}", ".", -1)
- tmp = strings.Replace(tmp, "${.PARSEDIR}", ".", -1)
+ if contains(tmp, "PKGSRCDIR") {
+ pkgsrcdir := relpath(basedir, G.Pkgsrc.File("."))
+
+ if G.Testing {
+ // Relative pkgsrc paths usually only contain two or three levels.
+ // A possible reason for reaching this assertion is a pkglint unit test
+ // that uses t.NewMkLines instead of the correct t.SetUpFileMkLines.
+ G.Assertf(!contains(pkgsrcdir, "../../../../.."),
+ "Relative path %q for %q is too deep below the pkgsrc root %q.",
+ pkgsrcdir, basedir, G.Pkgsrc.File("."))
+ }
+ tmp = strings.Replace(tmp, "${PKGSRCDIR}", pkgsrcdir, -1)
+ }
+ tmp = strings.Replace(tmp, "${.CURDIR}", ".", -1) // TODO: Replace with the "typical" os.Getwd().
+ tmp = strings.Replace(tmp, "${.PARSEDIR}", ".", -1) // FIXME
replaceLatest := func(varuse, category string, pattern regex.Pattern, replacement string) {
if contains(tmp, varuse) {
@@ -551,11 +560,16 @@ func (mkline *MkLineImpl) ResolveVarsInR
tmp = strings.Replace(tmp, varuse, latest, -1)
}
}
+
+ // These variables are only used in pkgsrc packages, therefore they
+ // are replaced with the fixed "../.." regardless of where the text appears.
replaceLatest("${LUA_PKGSRCDIR}", "lang", `^lua[0-9]+$`, "../../lang/$0")
replaceLatest("${PHPPKGSRCDIR}", "lang", `^php[0-9]+$`, "../../lang/$0")
- replaceLatest("${SUSE_DIR_PREFIX}", "emulators", `^(suse[0-9]+)_base$`, "$1")
replaceLatest("${PYPKGSRCDIR}", "lang", `^python[0-9]+$`, "../../lang/$0")
+
replaceLatest("${PYPACKAGE}", "lang", `^python[0-9]+$`, "$0")
+ replaceLatest("${SUSE_DIR_PREFIX}", "emulators", `^(suse[0-9]+)_base$`, "$1")
+
if G.Pkg != nil {
// XXX: Even if these variables are defined indirectly,
// pkglint should be able to resolve them properly.
@@ -668,7 +682,7 @@ func (mkline *MkLineImpl) VariableNeedsQ
}
return no
}
- if vartype.kindOfList == lkShell && !vuc.IsWordPart {
+ if !vuc.IsWordPart {
return no
}
}
@@ -690,10 +704,8 @@ func (mkline *MkLineImpl) VariableNeedsQ
// Both of these can be correct, depending on the situation:
// 1. echo ${PERL5:Q}
// 2. xargs ${PERL5}
- if !vuc.IsWordPart && vuc.quoting == VucQuotPlain {
- if wantList && haveList {
- return unknown
- }
+ if !vuc.IsWordPart && wantList && haveList {
+ return unknown
}
// Pkglint assumes that the tool definitions don't include very
@@ -737,7 +749,7 @@ func (mkline *MkLineImpl) VariableNeedsQ
// Bad: LDADD+= -l${LIBS}
// Good: LDADD+= ${LIBS:S,^,-l,}
- if wantList && haveList && vuc.IsWordPart {
+ if wantList {
return yes
}
@@ -1139,17 +1151,6 @@ func (ind *Indentation) TrackAfter(mklin
ind.top().depth += 2
}
- if cond != nil {
- ind.RememberUsedVariables(cond)
-
- cond.Walk(&MkCondCallback{
- Call: func(name string, arg string) {
- if name == "exists" {
- ind.AddCheckedFile(arg)
- }
- }})
- }
-
case "for", "ifdef", "ifndef":
ind.top().depth += 2
@@ -1169,6 +1170,23 @@ func (ind *Indentation) TrackAfter(mklin
}
}
+ switch directive {
+ case "if", "elif":
+ cond := mkline.Cond()
+ if cond == nil {
+ break
+ }
+
+ ind.RememberUsedVariables(cond)
+
+ cond.Walk(&MkCondCallback{
+ Call: func(name string, arg string) {
+ if name == "exists" {
+ ind.AddCheckedFile(arg)
+ }
+ }})
+ }
+
if trace.Tracing {
trace.Stepf("Indentation after line %s: %s", mkline.Linenos(), ind)
}
Index: pkgsrc/pkgtools/pkglint/files/mkline_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.50 pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.51
--- pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.50 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/mkline_test.go Thu Feb 21 22:49:03 2019
@@ -17,6 +17,20 @@ func (s *Suite) Test_NewMkLine__varassig
c.Check(mkline.VarassignComment(), equals, "# varassign comment")
}
+func (s *Suite) Test_NewMkLine__varassign_space_around_operator(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("--show-autofix", "--source")
+ t.NewMkLine("test.mk", 101,
+ "pkgbase = package")
+
+ t.CheckOutputLines(
+ "NOTE: test.mk:101: Unnecessary space after variable name \"pkgbase\".",
+ "AUTOFIX: test.mk:101: Replacing \"pkgbase =\" with \"pkgbase=\".",
+ "-\tpkgbase = package",
+ "+\tpkgbase= package")
+}
+
func (s *Suite) Test_NewMkLine__shellcmd(c *check.C) {
t := s.Init(c)
@@ -453,6 +467,25 @@ func (s *Suite) Test_MkLine_VariableNeed
t.CheckOutputEmpty() // Don't suggest to use ${HOMEPAGE:Q}.
}
+func (s *Suite) Test_MkLine_VariableNeedsQuoting__MASTER_SITES_and_HOMEPAGE(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpVartypes()
+ mklines := t.NewMkLines("Makefile",
+ MkRcsID,
+ "MASTER_SITES=\t${HOMEPAGE}",
+ "MASTER_SITES=\t${PATH}", // Some nonsense just for branch coverage.
+ "HOMEPAGE=\t${MASTER_SITES}",
+ "HOMEPAGE=\t${BUILD_DIRS}") // Some nonsense just for branch coverage.
+
+ mklines.Check()
+
+ t.CheckOutputLines(
+ "WARN: Makefile:3: Please use ${PATH:Q} instead of ${PATH}.",
+ "WARN: Makefile:4: HOMEPAGE should not be defined in terms of MASTER_SITEs.",
+ "WARN: Makefile:5: Please use ${BUILD_DIRS:Q} instead of ${BUILD_DIRS}.")
+}
+
// Before November 2018, pkglint did not parse $$(subshell) commands very well.
// As a side effect, it sometimes issued wrong warnings about the :Q modifier.
//
@@ -743,6 +776,13 @@ func (s *Suite) Test_MkLine_VariableNeed
"WARN: ~/Makefile:6: The variable PATH may not be set by any package.",
"WARN: ~/Makefile:6: PREFIX should not be evaluated at load time.",
"WARN: ~/Makefile:6: PATH should not be evaluated at load time.")
+
+ // Just for branch coverage.
+ trace.Tracing = false
+ MkLineChecker{mklines.mklines[2]}.Check()
+
+ t.CheckOutputLines(
+ "WARN: ~/Makefile:3: GO_SRCPATH is defined but not used.")
}
func (s *Suite) Test_MkLine__shell_varuse_in_backt_dquot(c *check.C) {
@@ -832,6 +872,17 @@ func (s *Suite) Test_MkLine_ValueSplit(c
"",
"${VAR2}two",
"words")
+
+}
+
+func (s *Suite) Test_MkLine_ValueSplit__invalid_argument(c *check.C) {
+ t := s.Init(c)
+
+ mkline := t.NewMkLine("filename.mk", 123, "VAR=\tvalue")
+
+ t.ExpectPanic(
+ func() { mkline.ValueSplit("value", "") },
+ "Pkglint internal error: Separator must not be empty; use ValueFields to split on whitespace")
}
func (s *Suite) Test_MkLine_Fields__varassign(c *check.C) {
@@ -934,8 +985,8 @@ func (s *Suite) Test_MkLine_ValueTokens(
testTokens := func(value string, expected ...*MkToken) {
mkline := t.NewMkLine("Makefile", 1, "PATH=\t"+value)
- split := mkline.ValueTokens()
- c.Check(split, deepEquals, expected)
+ tokens, _ := mkline.ValueTokens()
+ c.Check(tokens, deepEquals, expected)
}
testTokens("#empty",
@@ -957,14 +1008,46 @@ func (s *Suite) Test_MkLine_ValueTokens_
t := s.Init(c)
mkline := t.NewMkLine("Makefile", 1, "PATH=\tvalue ${UNFINISHED")
- split := mkline.ValueTokens()
+ tokens, rest := mkline.ValueTokens()
+
+ c.Check(tokens, deepEquals, []*MkToken{{"value ", nil}})
+ c.Check(rest, equals, "${UNFINISHED")
+
+ tokens2, rest2 := mkline.ValueTokens() // This time the slice is taken from the cache.
+
+ // In Go, it's not possible to compare slices for reference equality.
+ c.Check(tokens2, deepEquals, tokens)
+ c.Check(rest2, equals, rest)
+}
+
+func (s *Suite) Test_MkLine_ValueTokens__caching_parse_error(c *check.C) {
+ t := s.Init(c)
- c.Check(split, deepEquals, []*MkToken{{"value ", nil}})
+ mkline := t.NewMkLine("Makefile", 1, "PATH=\t${UNFINISHED")
+ tokens, rest := mkline.ValueTokens()
- split2 := mkline.ValueTokens() // This time the slice is taken from the cache.
+ c.Check(tokens, check.IsNil)
+ c.Check(rest, equals, "${UNFINISHED")
+
+ tokens2, rest2 := mkline.ValueTokens() // This time the slice is taken from the cache.
// In Go, it's not possible to compare slices for reference equality.
- c.Check(split2, deepEquals, split)
+ c.Check(tokens2, deepEquals, tokens)
+ c.Check(rest2, equals, rest)
+}
+
+func (s *Suite) Test_MkLine_ValueTokens__warnings(c *check.C) {
+ t := s.Init(c)
+
+ mklines := t.NewMkLines("Makefile",
+ MkRcsID,
+ "ROUND=\t$(ROUND)")
+
+ mklines.mklines[1].ValueTokens()
+ mklines.Check()
+
+ t.CheckOutputLines(
+ "WARN: Makefile:2: Please use curly braces {} instead of round parentheses () for ROUND.")
}
func (s *Suite) Test_MkLine_ResolveVarsInRelativePath(c *check.C) {
@@ -983,6 +1066,7 @@ func (s *Suite) Test_MkLine_ResolveVarsI
}
test("", ".")
+ test("${PKGSRCDIR}", ".")
test("${LUA_PKGSRCDIR}", "../../lang/lua53")
test("${PHPPKGSRCDIR}", "../../lang/php72")
test("${SUSE_DIR_PREFIX}", "suse100")
@@ -995,6 +1079,10 @@ func (s *Suite) Test_MkLine_ResolveVarsI
test("${FILESDIR}", "files")
test("${PKGDIR}", ".")
+
+ // Just for branch coverage.
+ G.Testing = false
+ test("${PKGSRCDIR}", "../..")
}
func (s *Suite) Test_MkLine_ResolveVarsInRelativePath__directory_depth(c *check.C) {
@@ -1128,6 +1216,44 @@ func (s *Suite) Test_Indentation_Remembe
c.Check(ind.Varnames(), deepEquals, []string{"PKGREVISION"})
}
+func (s *Suite) Test_Indentation_TrackAfter__checked_files(c *check.C) {
+ t := s.Init(c)
+
+ mklines := t.NewMkLines("file.mk",
+ MkRcsID,
+ "",
+ ".if make(other.mk)",
+ ". include \"other.mk\"",
+ ".endif",
+ "",
+ ".if exists(checked.mk)",
+ ". include \"checked.mk\"",
+ ".elif exists(other-checked.mk)",
+ ". include \"other-checked.mk\"",
+ ".endif")
+
+ mklines.Check()
+
+ t.CheckOutputLines(
+ "ERROR: file.mk:4: Relative path \"other.mk\" does not exist.")
+}
+
+func (s *Suite) Test_Indentation_TrackAfter__lonely_else(c *check.C) {
+ t := s.Init(c)
+
+ mklines := t.NewMkLines("file.mk",
+ MkRcsID,
+ "",
+ ".else")
+
+ mklines.Check()
+
+ // Surprisingly, pkglint doesn't report an error about this trivial bug.
+ // This will be caught by bmake, though. Therefore the only purpose of
+ // this test is the branch coverage in the "top.mkline != nil" case.
+ t.CheckOutputEmpty()
+}
+
func (s *Suite) Test_MkLine_DetermineUsedVariables(c *check.C) {
t := s.Init(c)
@@ -1214,8 +1340,58 @@ func (s *Suite) Test_matchMkDirective(c
[]interface{}{true, expectedIndent, expectedDirective, expectedArgs, expectedComment})
}
- test(".if ${VAR} == value", "", "if", "${VAR} == value", "")
- test(".\tendif # comment", "\t", "endif", "", "comment")
- test(".if ${VAR} == \"#\"", "", "if", "${VAR} == \"", "\"")
- test(".if ${VAR:[#]}", "", "if", "${VAR:[#]}", "")
+ test(".if ${VAR} == value",
+ "", "if", "${VAR} == value", "")
+
+ test(".\tendif # comment",
+ "\t", "endif", "", "comment")
+
+ test(".if ${VAR} == \"#\"",
+ "", "if", "${VAR} == \"", "\"")
+
+ test(".if ${VAR:[#]}",
+ "", "if", "${VAR:[#]}", "")
+
+ test(".if ${VAR} == \\",
+ "", "if", "${VAR} == \\", "")
+}
+
+func (s *Suite) Test_MatchMkInclude(c *check.C) {
+ t := s.Init(c)
+
+ test := func(input, expectedIndent, expectedDirective, expectedFilename string) {
+ m, indent, directive, args := MatchMkInclude(input)
+ c.Check(
+ []interface{}{m, indent, directive, args},
+ deepEquals,
+ []interface{}{true, expectedIndent, expectedDirective, expectedFilename})
+ }
+
+ testFail := func(input string) {
+ m, _, _, _ := MatchMkInclude(input)
+ if m {
+ c.Errorf("Text %q unexpectedly matched.", input)
+ }
+ }
+
+ testFail("")
+ testFail(".")
+ testFail(".include")
+ testFail(".include \"")
+ testFail(".include \"other.mk")
+ testFail(".include \"other.mk\" \"")
+
+ test(".include \"other.mk\"",
+ "", "include", "other.mk")
+
+ test(".include \"other.mk\"\t",
+ "", "include", "other.mk")
+
+ test(".include \"other.mk\"\t#",
+ "", "include", "other.mk")
+
+ test(".include \"other.mk\"\t# comment",
+ "", "include", "other.mk")
+
+ t.CheckOutputEmpty()
}
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.28 pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.29
--- pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.28 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker.go Thu Feb 21 22:49:03 2019
@@ -4,6 +4,7 @@ import (
"netbsd.org/pkglint/regex"
"os"
"path"
+ "path/filepath"
"strconv"
"strings"
)
@@ -298,9 +299,6 @@ func (ck MkLineChecker) checkVarassignLe
op := mkline.Op()
vartype := G.Pkgsrc.VariableType(varname)
if vartype == nil {
- if trace.Tracing {
- trace.Step1("No type definition found for %q.", varname)
- }
return
}
@@ -308,7 +306,7 @@ func (ck MkLineChecker) checkVarassignLe
// E.g. USE_TOOLS:= ${USE_TOOLS:Nunwanted-tool}
if op == opAssignEval && perms&aclpAppend != 0 {
- tokens := mkline.ValueTokens()
+ tokens, _ := mkline.ValueTokens()
if len(tokens) == 1 && tokens[0].Varuse != nil && tokens[0].Varuse.varname == varname {
return
}
@@ -563,15 +561,8 @@ func (ck MkLineChecker) checkVarusePermi
if !tool.UsableAtLoadTime(G.Mk.Tools.SeenPrefs) {
ck.warnVaruseToolLoadTime(varname, tool)
}
-
} else {
- // Might the variable be used indirectly at load time, for example
- // by assigning it to another variable which then gets evaluated?
- isIndirect := vuc.time != vucTimeParse && // Otherwise it would be directly.
- // The context might be used at load time somewhere.
- vuc.vartype != nil && vuc.vartype.Union().Contains(aclpUseLoadtime)
-
- ck.warnVaruseLoadTime(varname, isIndirect)
+ ck.warnVaruseLoadTime(varname, indirectly)
}
}
@@ -1051,10 +1042,6 @@ func (ck MkLineChecker) checkVartype(var
defer trace.Call(varname, op, value, comment)()
}
- if !G.Opts.WarnTypes {
- return
- }
-
mkline := ck.MkLine
vartype := G.Pkgsrc.VariableType(varname)
@@ -1191,6 +1178,7 @@ func (ck MkLineChecker) checkDirectiveCo
cond.Walk(&MkCondCallback{
Empty: ck.checkDirectiveCondEmpty,
+ Var: ck.checkDirectiveCondEmpty,
CompareVarStr: checkCompareVarStr,
VarUse: checkVarUse})
}
@@ -1298,11 +1286,11 @@ func (ck MkLineChecker) CheckRelativePat
}
abs := resolvedPath
- if !hasPrefix(abs, "/") {
+ if !filepath.IsAbs(abs) {
abs = path.Dir(mkline.Filename) + "/" + abs
}
if _, err := os.Stat(abs); err != nil {
- if mustExist {
+ if mustExist && !(G.Mk != nil && G.Mk.indentation.IsCheckedFile(resolvedPath)) {
mkline.Errorf("Relative path %q does not exist.", resolvedPath)
}
return
Index: pkgsrc/pkgtools/pkglint/files/patches.go
diff -u pkgsrc/pkgtools/pkglint/files/patches.go:1.28 pkgsrc/pkgtools/pkglint/files/patches.go:1.29
--- pkgsrc/pkgtools/pkglint/files/patches.go:1.28 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/patches.go Thu Feb 21 22:49:03 2019
@@ -98,7 +98,7 @@ func (ck *PatchChecker) Check() {
if SaveAutofixChanges(ck.lines) && G.Pkg != nil && err == nil {
sha1After, err := computePatchSha1Hex(ck.lines.FileName)
if err == nil {
- AutofixDistinfo(sha1Before, sha1After)
+ G.Pkg.AutofixDistinfo(sha1Before, sha1After)
}
}
}
Index: pkgsrc/pkgtools/pkglint/files/mklines.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines.go:1.40 pkgsrc/pkgtools/pkglint/files/mklines.go:1.41
--- pkgsrc/pkgtools/pkglint/files/mklines.go:1.40 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/mklines.go Thu Feb 21 22:49:03 2019
@@ -405,18 +405,15 @@ func (mklines *MkLinesImpl) CheckRedunda
scope := NewRedundantScope()
isRelevant := func(old, new MkLine) bool {
- if old.Basename != "Makefile" && new.Basename == "Makefile" {
- return false
- }
if new.Op() == opAssignEval {
return false
}
return true
}
- scope.OnIgnore = func(old, new MkLine) {
+ scope.OnRedundant = func(old, new MkLine) {
if isRelevant(old, new) && old.Value() == new.Value() {
- old.Notef("Definition of %s is redundant because of %s.", new.Varname(), old.RefTo(new))
+ new.Notef("Definition of %s is redundant because of %s.", old.Varname(), new.RefTo(old))
}
}
Index: pkgsrc/pkgtools/pkglint/files/mklines_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.35 pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.36
--- pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.35 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/mklines_test.go Thu Feb 21 22:49:03 2019
@@ -511,43 +511,6 @@ func (s *Suite) Test_MkLines_Check__inde
"NOTE: ~/module.mk:7: This directive should be indented by 2 spaces.")
}
-func (s *Suite) Test_MkLines_Check__endif_comment(c *check.C) {
- t := s.Init(c)
-
- t.SetUpVartypes()
- mklines := t.NewMkLines("opsys.mk",
- MkRcsID,
- "",
- ".for i in 1 2 3 4 5",
- ". if ${OPSYS} == NetBSD",
- ". if ${MACHINE_ARCH} == x86_64",
- ". if ${OS_VERSION:M8.*}",
- ". endif # MACHINE_ARCH", // Wrong, should be OS_VERSION.
- ". endif # OS_VERSION", // Wrong, should be MACHINE_ARCH.
- ". endif # OPSYS", // Correct.
- ".endfor # j", // Wrong, should be i.
- "",
- ".if ${PKG_OPTIONS:Moption}",
- ".endif # option", // Correct.
- "",
- ".if ${PKG_OPTIONS:Moption}",
- ".endif # opti", // This typo goes unnoticed since "opti" is a substring of the condition.
- "",
- ".if ${OPSYS} == NetBSD",
- ".elif ${OPSYS} == FreeBSD",
- ".endif # NetBSD") // Wrong, should be FreeBSD from the .elif.
-
- // See MkLineChecker.checkDirective
- mklines.Check()
-
- t.CheckOutputLines(
- "WARN: opsys.mk:7: Comment \"MACHINE_ARCH\" does not match condition \"${OS_VERSION:M8.*}\".",
- "WARN: opsys.mk:8: Comment \"OS_VERSION\" does not match condition \"${MACHINE_ARCH} == x86_64\".",
- "WARN: opsys.mk:10: Comment \"j\" does not match loop \"i in 1 2 3 4 5\".",
- "WARN: opsys.mk:12: Unknown option \"option\".",
- "WARN: opsys.mk:20: Comment \"NetBSD\" does not match condition \"${OPSYS} == FreeBSD\".")
-}
-
func (s *Suite) Test_MkLines_Check__unfinished_directives(c *check.C) {
t := s.Init(c)
@@ -747,25 +710,22 @@ func (s *Suite) Test_MkLines_CheckRedund
"OVERRIDE=\tprevious value",
"REDUNDANT=\tredundant")
including := t.NewMkLines("including.mk",
+ ".include \"included.mk\"",
"OVERRIDE=\toverridden value",
"REDUNDANT=\tredundant")
var allLines []Line
+ allLines = append(allLines, including.lines.Lines[:1]...)
allLines = append(allLines, included.lines.Lines...)
- allLines = append(allLines, including.lines.Lines...)
+ allLines = append(allLines, including.lines.Lines[1:]...)
mklines := NewMkLines(NewLines(included.lines.FileName, allLines))
// XXX: The warnings from here are not in the same order as the other warnings.
// XXX: There may be some warnings for the same file separated by warnings for other files.
mklines.CheckRedundantAssignments()
- // No warning for VAR=... in Makefile since it makes sense to have common files
- // with default values for variables, overriding some of them in each package.
t.CheckOutputLines(
- // FIXME: The below warning is wrong because overwriting in a different file is ok.
- "WARN: included.mk:1: Variable OVERRIDE is overwritten in including.mk:1.",
- // FIXME: It's the other way round: including.mk:2 is redundant because of included.mk:2.
- "NOTE: included.mk:2: Definition of REDUNDANT is redundant because of including.mk:2.")
+ "NOTE: including.mk:3: Definition of REDUNDANT is redundant because of included.mk:2.")
}
func (s *Suite) Test_MkLines_CheckRedundantAssignments__override_in_Makefile(c *check.C) {
@@ -774,13 +734,15 @@ func (s *Suite) Test_MkLines_CheckRedund
"VAR=\tvalue ${OTHER}",
"VAR?=\tvalue ${OTHER}",
"VAR=\tnew value")
- makefile := t.NewMkLines("Makefile",
+ including := t.NewMkLines("Makefile",
+ ".include \"module.mk\"",
"VAR=\tthe package may overwrite variables from other files")
var allLines []Line
+ allLines = append(allLines, including.lines.Lines[:1]...)
allLines = append(allLines, included.lines.Lines...)
- allLines = append(allLines, makefile.lines.Lines...)
- mklines := NewMkLines(NewLines(included.lines.FileName, allLines))
+ allLines = append(allLines, including.lines.Lines[1:]...)
+ mklines := NewMkLines(NewLines(including.lines.FileName, allLines))
// XXX: The warnings from here are not in the same order as the other warnings.
// XXX: There may be some warnings for the same file separated by warnings for other files.
@@ -789,7 +751,7 @@ func (s *Suite) Test_MkLines_CheckRedund
// No warning for VAR=... in Makefile since it makes sense to have common files
// with default values for variables, overriding some of them in each package.
t.CheckOutputLines(
- "NOTE: module.mk:1: Definition of VAR is redundant because of line 2.",
+ "NOTE: module.mk:2: Definition of VAR is redundant because of line 1.",
"WARN: module.mk:1: Variable VAR is overwritten in line 3.")
}
@@ -826,7 +788,7 @@ func (s *Suite) Test_MkLines_CheckRedund
mklines.CheckRedundantAssignments()
t.CheckOutputLines(
- "NOTE: module.mk:1: Definition of VAR is redundant because of line 2.")
+ "NOTE: module.mk:2: Definition of VAR is redundant because of line 1.")
}
func (s *Suite) Test_MkLines_CheckRedundantAssignments__conditional_overwrite(c *check.C) {
@@ -869,7 +831,7 @@ func (s *Suite) Test_MkLines_CheckRedund
t.CheckOutputLines(
"WARN: module.mk:1: Variable OTHER is overwritten in line 3.",
- "NOTE: module.mk:2: Definition of VAR is redundant because of line 4.")
+ "NOTE: module.mk:4: Definition of VAR is redundant because of line 2.")
}
func (s *Suite) Test_MkLines_CheckRedundantAssignments__overwrite_different_value_used_between(c *check.C) {
@@ -894,8 +856,7 @@ func (s *Suite) Test_MkLines_CheckRedund
t.CheckOutputLines(
"WARN: module.mk:1: Variable OTHER is overwritten in line 4.",
- // FIXME: It's the other way round: line 6 is redundant because of line 2.
- "NOTE: module.mk:2: Definition of VAR is redundant because of line 6.")
+ "NOTE: module.mk:6: Definition of VAR is redundant because of line 2.")
}
func (s *Suite) Test_MkLines_CheckRedundantAssignments__procedure_call(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/mkparser_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.21 pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.22
--- pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.21 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/mkparser_test.go Thu Feb 21 22:49:03 2019
@@ -375,12 +375,8 @@ func (s *Suite) Test_MkParser_MkCond(c *
}
varuse := NewMkVarUse
- // TODO: Add tests for &&, ||, !.
-
- // TODO: Add test for !empty(VAR:M}).
-
test("${OPSYS:MNetBSD}",
- &mkCond{Not: &mkCond{Empty: varuse("OPSYS", "MNetBSD")}})
+ &mkCond{Var: varuse("OPSYS", "MNetBSD")})
test("defined(VARNAME)",
&mkCond{Defined: "VARNAME"})
@@ -443,8 +439,8 @@ func (s *Suite) Test_MkParser_MkCond(c *
test("${MACHINE_ARCH:Mi386} || ${MACHINE_OPSYS:MNetBSD}",
&mkCond{Or: []*mkCond{
- {Not: &mkCond{Empty: varuse("MACHINE_ARCH", "Mi386")}},
- {Not: &mkCond{Empty: varuse("MACHINE_OPSYS", "MNetBSD")}}}})
+ {Var: varuse("MACHINE_ARCH", "Mi386")},
+ {Var: varuse("MACHINE_OPSYS", "MNetBSD")}}})
// Exotic cases
@@ -485,7 +481,7 @@ func (s *Suite) Test_MkParser_MkCond(c *
&mkCond{Defined: "VARNAME"})
test("${\"${PKG_OPTIONS:Moption}\":?--enable-option:--disable-option}",
- &mkCond{Not: &mkCond{Empty: varuse("\"${PKG_OPTIONS:Moption}\"", "?--enable-option:--disable-option")}})
+ &mkCond{Var: varuse("\"${PKG_OPTIONS:Moption}\"", "?--enable-option:--disable-option")})
// Errors
@@ -494,7 +490,7 @@ func (s *Suite) Test_MkParser_MkCond(c *
"|| defined(PKG_OPTIONS:Msamplerate)")
testRest("${LEFT} &&",
- &mkCond{Not: &mkCond{Empty: varuse("LEFT")}},
+ &mkCond{Var: varuse("LEFT")},
"&&")
testRest("\"unfinished string literal",
@@ -821,6 +817,9 @@ func (s *Suite) Test_MkCondWalker_Walk(c
Call: func(name string, arg string) {
addEvent("call", name, arg)
},
+ Var: func(varuse *MkVarUse) {
+ addEvent("var", varuseStr(varuse))
+ },
VarUse: func(varuse *MkVarUse) {
addEvent("varUse", varuseStr(varuse))
}})
@@ -836,6 +835,6 @@ func (s *Suite) Test_MkCondWalker_Walk(c
" defined VAR",
" varUse VAR",
" call exists, file.mk",
- " empty NONEMPTY",
+ " var NONEMPTY",
" varUse NONEMPTY"})
}
Index: pkgsrc/pkgtools/pkglint/files/substcontext_test.go
diff -u pkgsrc/pkgtools/pkglint/files/substcontext_test.go:1.21 pkgsrc/pkgtools/pkglint/files/substcontext_test.go:1.22
--- pkgsrc/pkgtools/pkglint/files/substcontext_test.go:1.21 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/substcontext_test.go Thu Feb 21 22:49:03 2019
@@ -370,18 +370,19 @@ func (s *Suite) Test_SubstContext_sugges
"SUBST_CLASSES+=\t\ttest",
"SUBST_STAGE.test=\tpre-configure",
"SUBST_FILES.test=\tfilename",
- "SUBST_SED.test+=\t-e s,@SH@,${SH},g", // Can be replaced.
- "SUBST_SED.test+=\t-e s,@SH@,${SH:Q},g", // Can be replaced, with or without the :Q modifier.
- "SUBST_SED.test+=\t-e s,@SH@,${SH:T},g", // Cannot be replaced because of the :T modifier.
- "SUBST_SED.test+=\t-e s,@SH@,${SH},", // Can be replaced, even without the g option.
- "SUBST_SED.test+=\t-e 's,@SH@,${SH},'", // Can be replaced, whether in single quotes or not.
- "SUBST_SED.test+=\t-e \"s,@SH@,${SH},\"", // Can be replaced, whether in double quotes or not.
- "SUBST_SED.test+=\t-e s,'@SH@','${SH}',", // Can be replaced, even when the quoting changes midways.
- "SUBST_SED.test+=\ts,'@SH@','${SH}',", // Can be replaced manually, even when the -e is missing.
- "SUBST_SED.test+=\t-e s,@SH@,${PKGNAME},", // Cannot be replaced since the variable name differs.
- "SUBST_SED.test+=\t-e s,@SH@,'\"'${SH:Q}'\"',g", // Cannot be replaced since the double quotes are added.
- "SUBST_SED.test+=\t-e s", // Just to get 100% code coverage.
- "SUBST_SED.test+=\t-e s,@SH@,${SH:Q}", // Just to get 100% code coverage.
+ "SUBST_SED.test+=\t-e s,@SH@,${SH},g", // Can be replaced.
+ "SUBST_SED.test+=\t-e s,@SH@,${SH:Q},g", // Can be replaced, with or without the :Q modifier.
+ "SUBST_SED.test+=\t-e s,@SH@,${SH:T},g", // Cannot be replaced because of the :T modifier.
+ "SUBST_SED.test+=\t-e s,@SH@,${SH},", // Can be replaced, even without the g option.
+ "SUBST_SED.test+=\t-e 's,@SH@,${SH},'", // Can be replaced, whether in single quotes or not.
+ "SUBST_SED.test+=\t-e \"s,@SH@,${SH},\"", // Can be replaced, whether in double quotes or not.
+ "SUBST_SED.test+=\t-e s,'@SH@','${SH}',", // Can be replaced, even when the quoting changes midways.
+ "SUBST_SED.test+=\ts,'@SH@','${SH}',", // Can be replaced manually, even when the -e is missing.
+ "SUBST_SED.test+=\t-e s,@SH@,${PKGNAME},", // Cannot be replaced since the variable name differs.
+ "SUBST_SED.test+=\t-e s,@SH@,'\"'${SH:Q}'\"',g", // Cannot be replaced since the double quotes are added.
+ "SUBST_SED.test+=\t-e s", // Just to get 100% code coverage.
+ "SUBST_SED.test+=\t-e s,@SH@,${SH:Q}", // Just to get 100% code coverage.
+ "SUBST_SED.test+=\t-e s,@SH@,${SH:Q}, # comment", // This is not fixed automatically.
"# end")
mklines.Check()
@@ -405,6 +406,8 @@ func (s *Suite) Test_SubstContext_sugges
"NOTE: subst.mk:13: Please always use \"-e\" in sed commands, "+
"even if there is only one substitution.",
"NOTE: subst.mk:13: The substitution command \"s,'@SH@','${SH}',\" "+
+ "can be replaced with \"SUBST_VARS.test+= SH\".",
+ "NOTE: subst.mk:18: The substitution command \"s,@SH@,${SH:Q},\" "+
"can be replaced with \"SUBST_VARS.test+= SH\".")
t.SetUpCommandLine("--show-autofix")
@@ -435,9 +438,7 @@ func (s *Suite) Test_SubstContext_sugges
"NOTE: subst.mk:12: The substitution command \"s,'@SH@','${SH}',\" "+
"can be replaced with \"SUBST_VARS.test+= SH\".",
"AUTOFIX: subst.mk:12: Replacing \"SUBST_SED.test+=\\t-e s,'@SH@','${SH}',\" "+
- "with \"SUBST_VARS.test+=\\tSH\".",
- "NOTE: subst.mk:13: The substitution command \"s,'@SH@','${SH}',\" "+
- "can be replaced with \"SUBST_VARS.test+= SH\".")
+ "with \"SUBST_VARS.test+=\\tSH\".")
}
// simulateSubstLines only tests some of the inner workings of SubstContext.
Index: pkgsrc/pkgtools/pkglint/files/mktypes.go
diff -u pkgsrc/pkgtools/pkglint/files/mktypes.go:1.10 pkgsrc/pkgtools/pkglint/files/mktypes.go:1.11
--- pkgsrc/pkgtools/pkglint/files/mktypes.go:1.10 Sun Jan 13 19:55:53 2019
+++ pkgsrc/pkgtools/pkglint/files/mktypes.go Thu Feb 21 22:49:03 2019
@@ -133,9 +133,8 @@ func (vu *MkVarUse) Mod() string {
return mod.String()
}
-// IsExpression returns whether the varname is interpreted as a variable
-// name (the usual case) or as an expression (rare, only the modifiers
-// "?:" and "L" do this).
+// IsExpression returns whether the varname is interpreted as an expression
+// instead of a variable name (rare, only the modifiers :? and :L do this).
func (vu *MkVarUse) IsExpression() bool {
if len(vu.modifiers) == 0 {
return false
Index: pkgsrc/pkgtools/pkglint/files/options_test.go
diff -u pkgsrc/pkgtools/pkglint/files/options_test.go:1.10 pkgsrc/pkgtools/pkglint/files/options_test.go:1.11
--- pkgsrc/pkgtools/pkglint/files/options_test.go:1.10 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/options_test.go Thu Feb 21 22:49:03 2019
@@ -192,3 +192,44 @@ func (s *Suite) Test_CheckLinesOptionsMk
t.CheckOutputLines(
"WARN: ~/category/package/options.mk:13: Invalid condition, unrecognized part: \"${OPSYS} == 'Darwin'\".")
}
+
+func (s *Suite) Test_CheckLinesOptionsMk__PLIST_VARS_based_on_PKG_SUPPORTED_OPTIONS(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpOption("one", "")
+ t.SetUpOption("two", "")
+ t.SetUpOption("three", "")
+ t.SetUpPackage("category/package")
+ t.CreateFileLines("mk/bsd.options.mk")
+ t.SetUpFileMkLines("category/package/options.mk",
+ MkRcsID,
+ "",
+ "PKG_OPTIONS_VAR=\tPKG_OPTIONS.package",
+ "PKG_SUPPORTED_OPTIONS+=\tone",
+ "PKG_SUPPORTED_OPTIONS+=\ttwo",
+ "PKG_SUPPORTED_OPTIONS+=\tthree",
+ "",
+ ".include \"../../mk/bsd.options.mk\"",
+ "",
+ "PLIST_VARS+=\t${PKG_SUPPORTED_OPTIONS}",
+ "",
+ ".if ${PKG_OPTIONS:Mone}",
+ "PLIST.one=\tyes",
+ ".endif",
+ "",
+ ".if ${PKG_OPTIONS:Mthree}",
+ "PLIST.three=\tyes",
+ ".endif")
+ t.Chdir("category/package")
+
+ G.Check(".")
+
+ // Even though PLIST_VARS is defined indirectly by referencing
+ // PKG_SUPPORTED_OPTIONS and that variable is defined in several
+ // lines, pkglint gets all the facts correct and knows that
+ // only PLIST.two is missing.
+ t.CheckOutputLines(
+ "WARN: options.mk:10: "+
+ "\"two\" is added to PLIST_VARS, but PLIST.two is not defined in this file.",
+ "WARN: options.mk:5: Option \"two\" should be handled below in an .if block.")
+}
Index: pkgsrc/pkgtools/pkglint/files/package.go
diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.44 pkgsrc/pkgtools/pkglint/files/package.go:1.45
--- pkgsrc/pkgtools/pkglint/files/package.go:1.44 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/package.go Thu Feb 21 22:49:03 2019
@@ -185,10 +185,10 @@ func (pkg *Package) loadPackageMakefile(
allLines.collectUsedVariables()
allLines.CheckRedundantAssignments()
- pkg.Pkgdir, _ = pkg.vars.Value("PKGDIR")
- pkg.DistinfoFile, _ = pkg.vars.Value("DISTINFO_FILE")
- pkg.Filesdir, _ = pkg.vars.Value("FILESDIR")
- pkg.Patchdir, _ = pkg.vars.Value("PATCHDIR")
+ pkg.Pkgdir = pkg.vars.LastValue("PKGDIR")
+ pkg.DistinfoFile = pkg.vars.LastValue("DISTINFO_FILE")
+ pkg.Filesdir = pkg.vars.LastValue("FILESDIR")
+ pkg.Patchdir = pkg.vars.LastValue("PATCHDIR")
// See lang/php/ext.mk
if varIsDefinedSimilar("PHPEXT_MK") {
@@ -438,6 +438,11 @@ func (pkg *Package) checkGnuConfigureUse
vars := pkg.vars
if gnuLine := vars.FirstDefinition("GNU_CONFIGURE"); gnuLine != nil {
+
+ // FIXME: Instead of using the first definition here, a better approach
+ // is probably to use all the definitions except those from mk/compiler.mk.
+ // In real pkgsrc, the last definition is typically from mk/compiler.mk
+ // and only contains c++.
if useLine := vars.FirstDefinition("USE_LANGUAGES"); useLine != nil {
if matches(useLine.VarassignComment(), `(?-i)\b(?:c|empty|none)\b`) {
@@ -460,7 +465,7 @@ func (pkg *Package) checkGnuConfigureUse
// It is only used inside pkgsrc to mark changes that are
// independent from the upstream package.
func (pkg *Package) nbPart() string {
- pkgrevision, _ := pkg.vars.Value("PKGREVISION")
+ pkgrevision := pkg.vars.LastValue("PKGREVISION")
if rev, err := strconv.Atoi(pkgrevision); err == nil {
return "nb" + strconv.Itoa(rev)
}
@@ -591,7 +596,7 @@ func (pkg *Package) CheckVarorder(mkline
defer trace.Call0()()
}
- if !G.Opts.WarnOrder || pkg.seenMakefileCommon {
+ if pkg.seenMakefileCommon {
return
}
@@ -666,12 +671,10 @@ func (pkg *Package) CheckVarorder(mkline
variable("TOOL_DEPENDS", many),
variable("DEPENDS", many))}
- firstRelevant := -1
- lastRelevant := -1
+ relevantLines := (func() []MkLine {
+ firstRelevant := -1
+ lastRelevant := -1
- // TODO: understand and explain this code.
- // It is much longer and much more complicated than it should be.
- skip := func() bool {
relevantVars := make(map[string]bool)
for _, section := range sections {
for _, variable := range section.vars {
@@ -693,7 +696,7 @@ func (pkg *Package) CheckVarorder(mkline
trace.Stepf("Skipping varorder because of line %s.",
mklines.mklines[firstIrrelevant].Linenos())
}
- return true
+ return nil
}
lastRelevant = i
} else {
@@ -713,9 +716,13 @@ func (pkg *Package) CheckVarorder(mkline
}
if firstRelevant == -1 {
- return true
+ return nil
}
- interesting := mklines.mklines[firstRelevant : lastRelevant+1]
+ return mklines.mklines[firstRelevant : lastRelevant+1]
+ })()
+
+ skip := func() bool {
+ interesting := relevantLines
varcanon := func() string {
for len(interesting) > 0 && interesting[0].IsComment() {
@@ -760,7 +767,7 @@ func (pkg *Package) CheckVarorder(mkline
return len(interesting) == 0
}
- if skip() {
+ if len(relevantLines) == 0 || skip() {
return
}
@@ -768,7 +775,7 @@ func (pkg *Package) CheckVarorder(mkline
for _, section := range sections {
for _, variable := range section.vars {
found := false
- for _, mkline := range mklines.mklines[firstRelevant : lastRelevant+1] {
+ for _, mkline := range relevantLines {
if mkline.IsVarassign() || mkline.IsCommentedVarassign() {
if mkline.Varcanon() == variable.varname {
canonical = append(canonical, mkline.Varname())
@@ -791,7 +798,7 @@ func (pkg *Package) CheckVarorder(mkline
// TODO: This leads to very long and complicated warnings.
// Those parts that are correct should not be mentioned,
// except if they are helpful for locating the mistakes.
- mkline := mklines.mklines[firstRelevant]
+ mkline := relevantLines[0]
mkline.Warnf("The canonical order of the variables is %s.", strings.Join(canonical, ", "))
G.Explain(
"In simple package Makefiles, some common variables should be",
@@ -812,8 +819,8 @@ func (pkg *Package) checkLocallyModified
defer trace.Call(filename)()
}
- owner, _ := pkg.vars.Value("OWNER")
- maintainer, _ := pkg.vars.Value("MAINTAINER")
+ owner := pkg.vars.LastValue("OWNER")
+ maintainer := pkg.vars.LastValue("MAINTAINER")
if maintainer == "pkgsrc-users%NetBSD.org@localhost" {
maintainer = ""
}
@@ -898,6 +905,19 @@ func (pkg *Package) loadPlistDirs(plistF
}
}
+func (pkg *Package) AutofixDistinfo(oldSha1, newSha1 string) {
+ distinfoFilename := pkg.File(pkg.DistinfoFile)
+ if lines := Load(distinfoFilename, NotEmpty|LogErrors); lines != nil {
+ for _, line := range lines.Lines {
+ fix := line.Autofix()
+ fix.Warnf(SilentAutofixFormat)
+ fix.Replace(oldSha1, newSha1)
+ fix.Apply()
+ }
+ lines.SaveAutofixChanges()
+ }
+}
+
type PlistContent struct {
Dirs map[string]bool
Files map[string]bool
Index: pkgsrc/pkgtools/pkglint/files/package_test.go
diff -u pkgsrc/pkgtools/pkglint/files/package_test.go:1.38 pkgsrc/pkgtools/pkglint/files/package_test.go:1.39
--- pkgsrc/pkgtools/pkglint/files/package_test.go:1.38 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/package_test.go Thu Feb 21 22:49:03 2019
@@ -72,7 +72,6 @@ func (s *Suite) Test_Package_pkgnameFrom
func (s *Suite) Test_Package_CheckVarorder(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Worder")
pkg := NewPackage(t.File("x11/9term"))
pkg.CheckVarorder(t.NewMkLines("Makefile",
@@ -106,7 +105,6 @@ func (s *Suite) Test_Package_CheckVarord
func (s *Suite) Test_Package_CheckVarorder__comments_do_not_crash(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Worder")
pkg := NewPackage(t.File("x11/9term"))
pkg.CheckVarorder(t.NewMkLines("Makefile",
@@ -129,8 +127,6 @@ func (s *Suite) Test_Package_CheckVarord
func (s *Suite) Test_Package_CheckVarorder__comments_are_ignored(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Worder")
-
pkg := NewPackage(t.File("x11/9term"))
pkg.CheckVarorder(t.NewMkLines("Makefile",
@@ -150,8 +146,6 @@ func (s *Suite) Test_Package_CheckVarord
func (s *Suite) Test_Package_CheckVarorder__skip_if_there_are_directives(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Worder")
-
pkg := NewPackage(t.File("category/package"))
pkg.CheckVarorder(t.NewMkLines("Makefile",
@@ -175,7 +169,6 @@ func (s *Suite) Test_Package_CheckVarord
func (s *Suite) Test_Package_CheckVarorder__GITHUB_PROJECT_at_the_top(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Worder")
pkg := NewPackage(t.File("x11/9term"))
pkg.CheckVarorder(t.NewMkLines("Makefile",
@@ -196,7 +189,6 @@ func (s *Suite) Test_Package_CheckVarord
func (s *Suite) Test_Package_CheckVarorder__GITHUB_PROJECT_at_the_bottom(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Worder")
pkg := NewPackage(t.File("x11/9term"))
pkg.CheckVarorder(t.NewMkLines("Makefile",
@@ -217,8 +209,6 @@ func (s *Suite) Test_Package_CheckVarord
func (s *Suite) Test_Package_CheckVarorder__license(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Worder")
-
t.CreateFileLines("mk/bsd.pkg.mk", "# dummy")
t.CreateFileLines("x11/Makefile", MkRcsID)
t.CreateFileLines("x11/9term/PLIST", PlistRcsID, "bin/9term")
@@ -226,10 +216,10 @@ func (s *Suite) Test_Package_CheckVarord
t.CreateFileLines("x11/9term/Makefile",
MkRcsID,
"",
- "DISTNAME=9term-1.0",
- "CATEGORIES=x11",
+ "DISTNAME=\t9term-1.0",
+ "CATEGORIES=\tx11",
"",
- "COMMENT=Terminal",
+ "COMMENT=\tTerminal",
"",
".include \"../../mk/bsd.pkg.mk\"")
@@ -246,7 +236,6 @@ func (s *Suite) Test_Package_CheckVarord
func (s *Suite) Test_Package_CheckVarorder__MASTER_SITES(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Worder")
pkg := NewPackage(t.File("category/package"))
pkg.CheckVarorder(t.NewMkLines("Makefile",
@@ -267,7 +256,6 @@ func (s *Suite) Test_Package_CheckVarord
func (s *Suite) Test_Package_CheckVarorder__diagnostics(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Worder")
t.SetUpVartypes()
pkg := NewPackage(t.File("category/package"))
@@ -354,7 +342,6 @@ func (s *Suite) Test_Package_determineEf
func (s *Suite) Test_Package_determineEffectivePkgVars__same(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Wall,no-order")
pkg := t.SetUpPackage("category/package",
"DISTNAME=\tdistname-1.0",
"PKGNAME=\tdistname-1.0")
@@ -369,7 +356,6 @@ func (s *Suite) Test_Package_determineEf
func (s *Suite) Test_Package_determineEffectivePkgVars__invalid_DISTNAME(c *check.C) {
t := s.Init(c)
- t.SetUpCommandLine("-Wall,no-order")
pkg := t.SetUpPackage("category/package",
"DISTNAME=\tpkgname-version")
@@ -541,10 +527,10 @@ func (s *Suite) Test_Package_loadPackage
// A file including itself does not lead to an endless loop while parsing
// but may still produce unexpected warnings, such as redundant definitions.
t.CheckOutputLines(
- "NOTE: ~/category/package/Makefile:3: Definition of PKGNAME is redundant "+
- "because of ../../category/package/Makefile:3.",
- "NOTE: ~/category/package/Makefile:4: Definition of DISTNAME is redundant "+
- "because of ../../category/package/Makefile:4.")
+ "NOTE: ~/category/package/Makefile:3: "+
+ "Definition of PKGNAME is redundant because of ../../category/package/Makefile:3.",
+ "NOTE: ~/category/package/Makefile:4: "+
+ "Definition of DISTNAME is redundant because of ../../category/package/Makefile:4.")
}
func (s *Suite) Test_Package_loadPackageMakefile__PECL_VERSION(c *check.C) {
@@ -632,9 +618,8 @@ func (s *Suite) Test_Package__include_af
G.checkdirPackage(t.File("category/package"))
- // FIXME: This error message should not appear at all because of the .if exists before.
- t.CheckOutputLines(
- "ERROR: ~/category/package/Makefile:21: Relative path \"options.mk\" does not exist.")
+ // No error message at all because of the .if exists before.
+ t.CheckOutputEmpty()
}
// See https://github.com/rillig/pkglint/issues/1
@@ -657,11 +642,8 @@ func (s *Suite) Test_Package_readMakefil
func (s *Suite) Test_Package__redundant_master_sites(c *check.C) {
t := s.Init(c)
- t.SetUpVartypes()
+ t.SetUpPkgsrc()
t.SetUpMasterSite("MASTER_SITE_R_CRAN", "http://cran.r-project.org/src/";)
- t.CreateFileLines("mk/bsd.pkg.mk")
- t.CreateFileLines("licenses/gnu-gpl-v2",
- "The licenses for most software are designed to take away ...")
t.CreateFileLines("math/R/Makefile.extension",
MkRcsID,
"",
@@ -680,13 +662,20 @@ func (s *Suite) Test_Package__redundant_
"",
".include \"../../math/R/Makefile.extension\"",
".include \"../../mk/bsd.pkg.mk\"")
+ G.Pkgsrc.LoadInfrastructure()
// See Package.checkfilePackageMakefile
- // See Scope.uncond
G.checkdirPackage(t.File("math/R-date"))
+ // The definition in Makefile:6 is redundant because the same definition
+ // occurs later in Makefile.extension:4. Usually the later definition gets
+ // the note. In this case though, it would be wrong to mark the
+ // definition in Makefile.extension as redundant because that definition is
+ // probably used by other packages as well. Therefore in this case the roles
+ // of the two lines are swapped; see RedundantScope.Handle, the ".includes" line.
t.CheckOutputLines(
- "NOTE: ~/math/R-date/Makefile:6: Definition of MASTER_SITES is redundant because of ../../math/R/Makefile.extension:4.")
+ "NOTE: ~/math/R-date/Makefile:6: " +
+ "Definition of MASTER_SITES is redundant because of ../../math/R/Makefile.extension:4.")
}
func (s *Suite) Test_Package_checkUpdate(c *check.C) {
@@ -709,7 +698,7 @@ func (s *Suite) Test_Package_checkUpdate
"\t"+"o package3-3.0 [security update]")
t.Chdir(".")
- G.Main("pkglint", "-Wall,no-space,no-order", "category/pkg1", "category/pkg2", "category/pkg3")
+ G.Main("pkglint", "-Wall,no-space", "category/pkg1", "category/pkg2", "category/pkg3")
t.CheckOutputLines(
"WARN: category/pkg1/../../doc/TODO:3: Invalid line format \"\".",
@@ -928,8 +917,6 @@ func (s *Suite) Test_Package_readMakefil
func (s *Suite) Test_Package_checkLocallyModified(c *check.C) {
t := s.Init(c)
- // no-order since SetUpPackage doesn't place OWNER correctly.
- t.SetUpCommandLine("-Wall,no-order")
G.Username = "example-user"
t.CreateFileLines("category/package/CVS/Entries",
"/Makefile//modified//")
@@ -988,3 +975,68 @@ func (s *Suite) Test_Package_checkLocall
t.CheckOutputEmpty()
}
+
+// In practice the distinfo file can always be autofixed since it has
+// just been read successfully and the corresponding patch file could
+// also be autofixed right before.
+func (s *Suite) Test_Package_AutofixDistinfo__missing_file(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPkgsrc()
+ G.Pkg = NewPackage(t.File("category/package"))
+
+ G.Pkg.AutofixDistinfo("old", "new")
+
+ t.CheckOutputLines(
+ "ERROR: ~/category/package/distinfo: Cannot be read.")
+}
+
+func (s *Suite) Test_Package__using_common_Makefile_overriding_DISTINFO_FILE(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("security/pinentry")
+ t.CreateFileLines("security/pinentry/Makefile.common",
+ MkRcsID,
+ "DISTINFO_FILE=\t${.CURDIR}/../../security/pinentry/distinfo")
+ t.SetUpPackage("security/pinentry-fltk",
+ ".include \"../../security/pinentry/Makefile.common\"",
+ "DISTINFO_FILE=\t${.CURDIR}/distinfo")
+ t.CreateFileDummyPatch("security/pinentry-fltk/patches/patch-aa")
+ t.CreateFileLines("security/pinentry-fltk/distinfo",
+ RcsID,
+ "",
+ "SHA1 (patch-aa) = ebbf34b0641bcb508f17d5a27f2bf2a536d810ac")
+ G.Pkgsrc.LoadInfrastructure()
+
+ G.Check(t.File("security/pinentry"))
+
+ t.CheckOutputEmpty()
+
+ G.Check(t.File("security/pinentry-fltk"))
+
+ // The DISTINFO_FILE definition from pinentry-fltk overrides
+ // the one from pinentry since it appears later.
+ // Therefore the patch is searched for at the right location.
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_Package__redundant_variable_in_unrelated_files(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("databases/py-trytond-ldap-authentication",
+ ".include \"../../devel/py-trytond/Makefile.common\"",
+ ".include \"../../lang/python/egg.mk\"")
+ t.CreateFileLines("devel/py-trytond/Makefile.common",
+ MkRcsID,
+ "PY_PATCHPLIST=\tyes")
+ t.CreateFileLines("lang/python/egg.mk",
+ MkRcsID,
+ "PY_PATCHPLIST=\tyes")
+ G.Pkgsrc.LoadInfrastructure()
+
+ G.Check(t.File("databases/py-trytond-ldap-authentication"))
+
+ // Since egg.mk and Makefile.common are unrelated, the definition of
+ // PY_PATCHPLIST is not redundant in these files.
+ t.CheckOutputEmpty()
+}
Index: pkgsrc/pkgtools/pkglint/files/pkglint.0
diff -u pkgsrc/pkgtools/pkglint/files/pkglint.0:1.38 pkgsrc/pkgtools/pkglint/files/pkglint.0:1.39
--- pkgsrc/pkgtools/pkglint/files/pkglint.0:1.38 Sat Jan 13 23:56:14 2018
+++ pkgsrc/pkgtools/pkglint/files/pkglint.0 Thu Feb 21 22:49:03 2019
@@ -1,7 +1,7 @@
-PKGLINT(1) NetBSD General Commands Manual PKGLINT(1)
+PKGLINT(1) General Commands Manual PKGLINT(1)
NNAAMMEE
- ppkkgglliinntt -- static analyzer for pkgsrc packages
+ ppkkgglliinntt - static analyzer for pkgsrc packages
SSYYNNOOPPSSIISS
ppkkgglliinntt [--ooppttiioonnss] [_d_i_r _._._.]
@@ -10,6 +10,7 @@ DDEESSCCRRIIPPTTIIOONN
ppkkgglliinntt attempts to detect features of the named pkgsrc packages that are
likely to be bugs, or that are simply deprecated.
+
OOppttiioonnss
--CC{{[[nnoo--]]cchheecckk,,......}} Enable or disable specific checks. For a list of
checks, see below.
@@ -62,78 +63,39 @@ DDEESSCCRRIIPPTTIIOONN
nnoonnee Disable all checks.
- [[nnoo--]]AALLTTEERRNNAATTIIVVEESS Check the alternatives file.
-
- [[nnoo--]]DDEESSCCRR Check the DESCR file.
-
- [[nnoo--]]IINNSSTTAALLLL Check the INSTALL and DEINSTALL scripts.
-
- [[nnoo--]]MMaakkeeffiillee Check the package Makefile, including all included
- files.
-
- [[nnoo--]]MMEESSSSAAGGEE Check MESSAGE files.
-
- [[nnoo--]]PPLLIISSTT Check PLIST files.
-
- [[nnoo--]]bbll33 Check buildlink3 Makefiles.
-
- [[nnoo--]]ddiissttiinnffoo Check the distinfo file.
-
[[nnoo--]]eexxttrraa Check remaining files in the package directory.
- [[nnoo--]]mmkk Check Makefile fragments besides buildlink3.
-
- [[nnoo--]]ppaattcchheess Check the pkgsrc specific patch files.
+ [[nnoo--]]gglloobbaall Check inter-package consistency for distfile hashes
+ and used licenses.
WWaarrnniinnggss
aallll Enable all warnings.
nnoonnee Disable all warnings.
- [[nnoo--]]aabbssnnaammee Warn if a file contains an absolute pathname.
-
- [[nnoo--]]ddiirreeccttccmmdd Warn if a system command name is used instead of a
- variable (e.g. sed instead of ${SED}).
-
[[nnoo--]]eexxttrraa Emit some additional warnings that are not enabled by
default.
- [[nnoo--]]oorrddeerr Warn if Makefile variables are not in the preferred
- order.
-
[[nnoo--]]ppeerrmm Warn if a variable is used or modified outside its
specified scope.
- [[nnoo--]]pplliisstt--ddeepprr Warn if deprecated pathnames are used in _P_L_I_S_T files.
- This warning is disabled by default.
-
- [[nnoo--]]pplliisstt--ssoorrtt Warn if items of a PLIST file are not sorted alpha-
- betically. This warning is disabled by default.
-
[[nnoo--]]qquuoottiinngg Warn for possibly invalid quoting of make variables
in shell programs and shell variables themselves.
- [[nnoo--]]ssppaaccee Emit notes for inconsistent use of white-space.
+ [[nnoo--]]ssppaaccee Emit notes for inconsistent use of whitespace.
[[nnoo--]]ssttyyllee Warn for stylistic issues that don't affect the build
process.
- [[nnoo--]]ttyyppeess Warn for some _M_a_k_e_f_i_l_e variables if their assigned
- values do not match their type.
-
- [[nnoo--]]vvaarroorrddeerr Warn if the variables in a package _M_a_k_e_f_i_l_es are not
- ordered in the way it is described the pkgsrc guide.
-
OOtthheerr aarrgguummeennttss
- _d_i_r _._._. The pkgsrc directories to be checked. If omit-
- ted, the current directory is checked.
+ _d_i_r _._._. The pkgsrc directories to be checked. If
+ omitted, the current directory is checked.
FFIILLEESS
- pkgsrc/mk/* Files from the pkgsrc infrastructure.
+ _p_k_g_s_r_c_/_m_k_/_* Files from the pkgsrc infrastructure.
EEXXAAMMPPLLEESS
- ppkkgglliinntt --CCnnoonnee,,ppaattcchheess ..
- Checks the patches of the package in the current directory.
+ ppkkgglliinntt .. Checks the package in the current directory.
ppkkgglliinntt --WWaallll //uussrr//ppkkggssrrcc//ddeevveell
Checks the category Makefile and reports any warnings it can
@@ -155,4 +117,4 @@ BBUUGGSS
If you don't understand the messages, feel free to ask the author or on
the <pkgsrc-users%pkgsrc.org@localhost> mailing list.
-NetBSD 7.0.2 January 14, 2018 NetBSD 7.0.2
+NetBSD 8.0 January 14, 2018 NetBSD 8.0
Index: pkgsrc/pkgtools/pkglint/files/shell_test.go
diff -u pkgsrc/pkgtools/pkglint/files/shell_test.go:1.38 pkgsrc/pkgtools/pkglint/files/shell_test.go:1.39
--- pkgsrc/pkgtools/pkglint/files/shell_test.go:1.38 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/shell_test.go Thu Feb 21 22:49:03 2019
@@ -1148,7 +1148,7 @@ func (s *Suite) Test_SimpleCommandChecke
// FIXME: In PERL5:Q and PERL6:Q, the :Q is wrong.
t.CheckOutputLines(
"WARN: Makefile:3: PERL5_VARS_CMD is defined but not used.",
- "WARN: Makefile:4: The \"perl6\" tool is used but not added to USE_TOOLS.")
+ "WARN: Makefile:4: The \"${PERL6:Q}\" tool is used but not added to USE_TOOLS.")
}
// The package Makefile and other .mk files in a package directory
@@ -1189,6 +1189,26 @@ func (s *Suite) Test_SimpleCommandChecke
"WARN: file.mk:3: A shell comment should not contain semicolons.")
}
+// This test ensures that the command line options to INSTALL_*_DIR are properly
+// parsed and do not lead to "can only handle one directory at a time" warnings.
+func (s *Suite) Test_SimpleCommandChecker_checkInstallMulti(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpVartypes()
+ mklines := t.NewMkLines("install.mk",
+ MkRcsID,
+ "",
+ "do-install:",
+ "\t${INSTALL_PROGRAM_DIR} -m 0555 -g ${APACHE_GROUP} -o ${APACHE_USER} \\",
+ "\t\t${DESTDIR}${PREFIX}/lib/apache-modules")
+
+ mklines.Check()
+
+ t.CheckOutputLines(
+ "NOTE: install.mk:4--5: You can use \"INSTALLATION_DIRS+= lib/apache-modules\" " +
+ "instead of \"${INSTALL_PROGRAM_DIR}\".")
+}
+
func (s *Suite) Test_SimpleCommandChecker_checkPaxPe(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/pkglint.1
diff -u pkgsrc/pkgtools/pkglint/files/pkglint.1:1.52 pkgsrc/pkgtools/pkglint/files/pkglint.1:1.53
--- pkgsrc/pkgtools/pkglint/files/pkglint.1:1.52 Sat Jan 13 23:56:14 2018
+++ pkgsrc/pkgtools/pkglint/files/pkglint.1 Thu Feb 21 22:49:03 2019
@@ -1,4 +1,4 @@
-.\" $NetBSD: pkglint.1,v 1.52 2018/01/13 23:56:14 rillig Exp $
+.\" $NetBSD: pkglint.1,v 1.53 2019/02/21 22:49:03 rillig Exp $
.\" From FreeBSD: portlint.1,v 1.8 1997/11/25 14:53:14 itojun Exp
.\"
.\" Copyright (c) 1997 by Jun-ichiro Itoh <itojun%itojun.org@localhost>.
@@ -6,7 +6,7 @@
.\"
.\" Roland Illig <roland.illig%gmx.de@localhost>, 2004, 2005.
.\" Thomas Klausner <wiz%NetBSD.org@localhost>, 2012.
-.\" Roland Illig <rillig%NetBSD.org@localhost>, 2015-2018.
+.\" Roland Illig <rillig%NetBSD.org@localhost>, 2015-2019.
.\"
.Dd January 14, 2018
.Dt PKGLINT 1
@@ -88,28 +88,10 @@ For a list of warnings, see below.
Enable all checks.
.It Cm none
Disable all checks.
-.It Cm [no-]ALTERNATIVES
-Check the alternatives file.
-.It Cm [no-]DESCR
-Check the DESCR file.
-.It Cm [no-]INSTALL
-Check the INSTALL and DEINSTALL scripts.
-.It Cm [no-]Makefile
-Check the package Makefile, including all included files.
-.It Cm [no-]MESSAGE
-Check MESSAGE files.
-.It Cm [no-]PLIST
-Check PLIST files.
-.It Cm [no-]bl3
-Check buildlink3 Makefiles.
-.It Cm [no-]distinfo
-Check the distinfo file.
.It Cm [no-]extra
Check remaining files in the package directory.
-.It Cm [no-]mk
-Check Makefile fragments besides buildlink3.
-.It Cm [no-]patches
-Check the pkgsrc specific patch files.
+.It Cm [no-]global
+Check inter-package consistency for distfile hashes and used licenses.
.El
.\" =======================================================================
.Ss Warnings
@@ -118,41 +100,17 @@ Check the pkgsrc specific patch files.
Enable all warnings.
.It Cm none
Disable all warnings.
-.It Cm [no-]absname
-Warn if a file contains an absolute pathname.
-.It Cm [no-]directcmd
-Warn if a system command name is used instead of a variable (e.g. sed
-instead of ${SED}).
.It Cm [no-]extra
Emit some additional warnings that are not enabled by default.
-.It Cm [no-]order
-Warn if Makefile variables are not in the preferred order.
.It Cm [no-]perm
Warn if a variable is used or modified outside its specified scope.
-.It Cm [no-]plist-depr
-Warn if deprecated pathnames are used in
-.Pa PLIST
-files.
-This warning is disabled by default.
-.It Cm [no-]plist-sort
-Warn if items of a PLIST file are not sorted alphabetically.
-This warning is disabled by default.
.It Cm [no-]quoting
Warn for possibly invalid quoting of make variables in shell programs
and shell variables themselves.
.It Cm [no-]space
-Emit notes for inconsistent use of white-space.
+Emit notes for inconsistent use of whitespace.
.It Cm [no-]style
Warn for stylistic issues that don't affect the build process.
-.It Cm [no-]types
-Warn for some
-.Pa Makefile
-variables if their assigned values do not match
-their type.
-.It Cm [no-]varorder
-Warn if the variables in a package
-.Pa Makefile Ns
-s are not ordered in the way it is described the pkgsrc guide.
.El
.\" =======================================================================
.Ss Other arguments
@@ -168,8 +126,8 @@ Files from the pkgsrc infrastructure.
.El
.Sh EXAMPLES
.Bl -tag -width Fl
-.It Ic pkglint \-Cnone,patches \&.
-Checks the patches of the package in the current directory.
+.It Ic pkglint \&.
+Checks the package in the current directory.
.It Ic pkglint \-Wall /usr/pkgsrc/devel
Checks the category Makefile and reports any warnings it can find.
.El
Index: pkgsrc/pkgtools/pkglint/files/pkglint.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint.go:1.46 pkgsrc/pkgtools/pkglint/files/pkglint.go:1.47
--- pkgsrc/pkgtools/pkglint/files/pkglint.go:1.46 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/pkglint.go Thu Feb 21 22:49:03 2019
@@ -40,6 +40,9 @@ type Pkglint struct {
res regex.Registry
fileCache *FileCache
interner StringInterner
+
+ Hashes map[string]*Hash // Maps "alg:filename" => hash (inter-package check).
+ UsedLicenses map[string]bool // Maps "license name" => true (inter-package check).
}
func NewPkglint() Pkglint {
@@ -50,24 +53,8 @@ func NewPkglint() Pkglint {
}
type CmdOpts struct {
- // TODO: Are these Check* options really necessary?
- //
- // They had been introduced in order to make pkglint more flexible,
- // but without any actual need.
-
- CheckAlternatives,
- CheckBuildlink3,
- CheckDescr,
- CheckDistinfo,
CheckExtra,
- CheckGlobal,
- CheckInstall,
- CheckMakefile,
- CheckMessage,
- CheckMk,
- CheckOptions,
- CheckPatches,
- CheckPlist bool
+ CheckGlobal bool
// TODO: Are these Warn* options really all necessary?
//
@@ -76,16 +63,11 @@ type CmdOpts struct {
// could be contrasted by a future --ignore option, in order to suppress
// individual checks.
- WarnDirectcmd,
WarnExtra,
- WarnOrder,
WarnPerm,
- WarnPlistDepr,
- WarnPlistSort,
WarnQuoting,
WarnSpace,
- WarnStyle,
- WarnTypes bool
+ WarnStyle bool
Profiling,
ShowHelp,
@@ -255,30 +237,14 @@ func (pkglint *Pkglint) ParseCommandLine
opts.AddFlagVar('V', "version", &gopts.ShowVersion, false, "show the version number of pkglint")
warn := opts.AddFlagGroup('W', "warning", "warning,...", "enable or disable groups of warnings")
- check.AddFlagVar("ALTERNATIVES", &gopts.CheckAlternatives, true, "check ALTERNATIVES files")
- check.AddFlagVar("bl3", &gopts.CheckBuildlink3, true, "check buildlink3.mk files")
- check.AddFlagVar("DESCR", &gopts.CheckDescr, true, "check DESCR file")
- check.AddFlagVar("distinfo", &gopts.CheckDistinfo, true, "check distinfo file")
check.AddFlagVar("extra", &gopts.CheckExtra, false, "check various additional files")
check.AddFlagVar("global", &gopts.CheckGlobal, false, "inter-package checks")
- check.AddFlagVar("INSTALL", &gopts.CheckInstall, true, "check INSTALL and DEINSTALL scripts")
- check.AddFlagVar("Makefile", &gopts.CheckMakefile, true, "check Makefiles")
- check.AddFlagVar("MESSAGE", &gopts.CheckMessage, true, "check MESSAGE file")
- check.AddFlagVar("mk", &gopts.CheckMk, true, "check other .mk files")
- check.AddFlagVar("options", &gopts.CheckOptions, true, "check options.mk files")
- check.AddFlagVar("patches", &gopts.CheckPatches, true, "check patches")
- check.AddFlagVar("PLIST", &gopts.CheckPlist, true, "check PLIST files")
- warn.AddFlagVar("directcmd", &gopts.WarnDirectcmd, true, "warn about use of direct command names instead of Make variables")
warn.AddFlagVar("extra", &gopts.WarnExtra, false, "enable some extra warnings")
- warn.AddFlagVar("order", &gopts.WarnOrder, true, "warn if Makefile entries are unordered")
warn.AddFlagVar("perm", &gopts.WarnPerm, false, "warn about unforeseen variable definition and use")
- warn.AddFlagVar("plist-depr", &gopts.WarnPlistDepr, false, "warn about deprecated paths in PLISTs")
- warn.AddFlagVar("plist-sort", &gopts.WarnPlistSort, false, "warn about unsorted entries in PLISTs")
warn.AddFlagVar("quoting", &gopts.WarnQuoting, false, "warn about quoting issues")
warn.AddFlagVar("space", &gopts.WarnSpace, false, "warn about inconsistent use of whitespace")
warn.AddFlagVar("style", &gopts.WarnStyle, false, "warn about stylistic issues")
- warn.AddFlagVar("types", &gopts.WarnTypes, true, "do some simple type checking in Makefiles")
remainingArgs, err := opts.Parse(args)
if err != nil {
@@ -471,9 +437,7 @@ func (pkglint *Pkglint) checkdirPackage(
case path.Base(filename) == "Makefile":
pkglint.checkExecutable(filename, st.Mode())
- if pkglint.Opts.CheckMakefile {
- pkg.checkfilePackageMakefile(filename, mklines)
- }
+ pkg.checkfilePackageMakefile(filename, mklines)
default:
pkglint.checkDirent(filename, st.Mode())
@@ -487,7 +451,7 @@ func (pkglint *Pkglint) checkdirPackage(
pkg.checkLocallyModified(filename)
}
- if pkg.Pkgdir == "." && pkglint.Opts.CheckDistinfo && pkglint.Opts.CheckPatches {
+ if pkg.Pkgdir == "." {
if havePatches && !haveDistinfo {
// TODO: Add Line.RefTo to make the context clear.
NewLineWhole(pkg.File(pkg.DistinfoFile)).Warnf("File not found. Please run %q.", bmake("makepatchsum"))
@@ -543,12 +507,12 @@ func resolveVariableRefs(text string) (r
if !visited[varname] {
visited[varname] = true
if G.Pkg != nil {
- if value, ok := G.Pkg.vars.Value(varname); ok {
+ if value, ok := G.Pkg.vars.LastValueFound(varname); ok {
return value
}
}
if G.Mk != nil {
- if value, ok := G.Mk.vars.Value(varname); ok {
+ if value, ok := G.Mk.vars.LastValueFound(varname); ok {
return value
}
}
@@ -700,55 +664,39 @@ func (pkglint *Pkglint) checkReg(filenam
switch {
case basename == "ALTERNATIVES":
- if pkglint.Opts.CheckAlternatives {
- CheckFileAlternatives(filename)
- }
+ CheckFileAlternatives(filename)
case basename == "buildlink3.mk":
- if pkglint.Opts.CheckBuildlink3 {
- if mklines := LoadMk(filename, NotEmpty|LogErrors); mklines != nil {
- CheckLinesBuildlink3Mk(mklines)
- }
+ if mklines := LoadMk(filename, NotEmpty|LogErrors); mklines != nil {
+ CheckLinesBuildlink3Mk(mklines)
}
case hasPrefix(basename, "DESCR"):
- if pkglint.Opts.CheckDescr {
- if lines := Load(filename, NotEmpty|LogErrors); lines != nil {
- CheckLinesDescr(lines)
- }
+ if lines := Load(filename, NotEmpty|LogErrors); lines != nil {
+ CheckLinesDescr(lines)
}
case basename == "distinfo":
- if pkglint.Opts.CheckDistinfo {
- if lines := Load(filename, NotEmpty|LogErrors); lines != nil {
- CheckLinesDistinfo(lines)
- }
+ if lines := Load(filename, NotEmpty|LogErrors); lines != nil {
+ CheckLinesDistinfo(lines)
}
case basename == "DEINSTALL" || basename == "INSTALL":
- if pkglint.Opts.CheckInstall {
- CheckFileOther(filename)
- }
+ CheckFileOther(filename)
case hasPrefix(basename, "MESSAGE"):
- if pkglint.Opts.CheckMessage {
- if lines := Load(filename, NotEmpty|LogErrors); lines != nil {
- CheckLinesMessage(lines)
- }
+ if lines := Load(filename, NotEmpty|LogErrors); lines != nil {
+ CheckLinesMessage(lines)
}
case basename == "options.mk":
- if pkglint.Opts.CheckOptions {
- if mklines := LoadMk(filename, NotEmpty|LogErrors); mklines != nil {
- CheckLinesOptionsMk(mklines)
- }
+ if mklines := LoadMk(filename, NotEmpty|LogErrors); mklines != nil {
+ CheckLinesOptionsMk(mklines)
}
case matches(basename, `^patch-[-\w.~+]*\w$`):
- if pkglint.Opts.CheckPatches {
- if lines := Load(filename, NotEmpty|LogErrors); lines != nil {
- CheckLinesPatch(lines)
- }
+ if lines := Load(filename, NotEmpty|LogErrors); lines != nil {
+ CheckLinesPatch(lines)
}
case matches(filename, `(?:^|/)patches/manual[^/]*$`):
@@ -760,17 +708,13 @@ func (pkglint *Pkglint) checkReg(filenam
NewLineWhole(filename).Warnf("Patch files should be named \"patch-\", followed by letters, '-', '_', '.', and digits only.")
case (hasPrefix(basename, "Makefile") || hasSuffix(basename, ".mk")) &&
- !contains(filename, "files/") &&
- !contains(filename, "patches/"):
- if pkglint.Opts.CheckMk {
- CheckFileMk(filename)
- }
+ !(hasPrefix(filename, "files/") || contains(filename, "/files/")) &&
+ !(hasPrefix(filename, "patches/") || contains(filename, "/patches/")):
+ CheckFileMk(filename)
case hasPrefix(basename, "PLIST"):
- if pkglint.Opts.CheckPlist {
- if lines := Load(filename, NotEmpty|LogErrors); lines != nil {
- CheckLinesPlist(lines)
- }
+ if lines := Load(filename, NotEmpty|LogErrors); lines != nil {
+ CheckLinesPlist(lines)
}
case hasPrefix(basename, "CHANGES-"):
@@ -801,7 +745,7 @@ func (pkglint *Pkglint) matchesLicenseFi
return false
}
- licenseFile, _ := pkglint.Pkg.vars.Value("LICENSE_FILE")
+ licenseFile := pkglint.Pkg.vars.LastValue("LICENSE_FILE")
return basename == path.Base(licenseFile)
}
@@ -857,9 +801,9 @@ func CheckLinesTrailingEmptyLines(lines
// to USE_TOOLS in the current scope (file or package).
func (pkglint *Pkglint) Tool(command string, time ToolTime) (tool *Tool, usable bool) {
varname := ""
- // TODO: Replace regex with proper VarUse.
- if m, toolVarname := match1(command, `^\$\{(\w+)\}$`); m {
- varname = toolVarname
+ p := NewMkParser(nil, command, false)
+ if varUse := p.VarUse(); varUse != nil && p.EOF() {
+ varname = varUse.varname
}
tools := pkglint.tools()
Index: pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go
diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.15 pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.16
--- pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.15 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go Thu Feb 21 22:49:03 2019
@@ -435,6 +435,50 @@ func (s *Suite) Test_Pkgsrc_ListVersions
"postgresql11"})
}
+func (s *Suite) Test_Pkgsrc_ListVersions__ensure_transitive(c *check.C) {
+ names := []string{
+ "base",
+ "base0",
+ "base000",
+ "base-client",
+ "base1",
+ "base01",
+ "base-client1",
+ "base5",
+ "base10"}
+
+ keys := make(map[string]int)
+ for _, name := range names {
+ if m, _, versionStr := match2(name, `^(\D+)(\d+)$`); m {
+ keys[name] = toInt(versionStr, 0)
+ }
+ }
+
+ less := func(a, b string) bool {
+ if keyI, keyJ := keys[a], keys[b]; keyI != keyJ {
+ return keyI < keyJ
+ }
+ return naturalLess(a, b)
+ }
+
+ test := func(i int, j int) {
+ actual := less(names[i], names[j])
+ expected := i < j
+ if actual != expected {
+ c.Check(
+ []interface{}{names[i], ifelseStr(actual, "<", "!<"), names[j]},
+ check.DeepEquals,
+ []interface{}{names[i], ifelseStr(expected, "<", "!<"), names[j]})
+ }
+ }
+
+ for i := range names {
+ for j := range names {
+ test(i, j)
+ }
+ }
+}
+
func (s *Suite) Test_Pkgsrc_ListVersions__numeric_multiple_numbers(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/plist.go
diff -u pkgsrc/pkgtools/pkglint/files/plist.go:1.36 pkgsrc/pkgtools/pkglint/files/plist.go:1.37
--- pkgsrc/pkgtools/pkglint/files/plist.go:1.36 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/plist.go Thu Feb 21 22:49:03 2019
@@ -65,13 +65,9 @@ func (ck *PlistChecker) Check(plainLines
}
CheckLinesTrailingEmptyLines(plainLines)
- if G.Opts.WarnPlistSort {
- sorter := NewPlistLineSorter(plines)
- sorter.Sort()
- if !sorter.autofixed {
- SaveAutofixChanges(plainLines)
- }
- } else {
+ sorter := NewPlistLineSorter(plines)
+ sorter.Sort()
+ if !sorter.autofixed {
SaveAutofixChanges(plainLines)
}
}
@@ -112,8 +108,6 @@ func (ck *PlistChecker) collectFilesAndD
ck.allDirs[dir] = pline
}
case first == '@':
- // TODO: Check if this directive is still used,
- // or if it has been removed during a pkg_install re-implementation.
if m, dirname := match1(text, `^@exec \$\{MKDIR\} %D/(.*)$`); m {
for dir := dirname; dir != "."; dir = path.Dir(dir) {
ck.allDirs[dir] = pline
@@ -199,7 +193,7 @@ func (ck *PlistChecker) checkPath(pline
pline.Warnf(".orig files should not be in the PLIST.")
}
if hasSuffix(text, "/perllocal.pod") {
- pline.Warnf("perllocal.pod files should not be in the PLIST.")
+ pline.Warnf("The perllocal.pod file should not be in the PLIST.")
G.Explain(
"This file is handled automatically by the INSTALL/DEINSTALL scripts",
"since its contents depends on more than one package.")
@@ -210,7 +204,7 @@ func (ck *PlistChecker) checkPath(pline
}
func (ck *PlistChecker) checkSorted(pline *PlistLine) {
- if text := pline.text; G.Opts.WarnPlistSort && hasAlnumPrefix(text) && !containsVarRef(text) {
+ if text := pline.text; hasAlnumPrefix(text) && !containsVarRef(text) {
if ck.lastFname != "" {
if ck.lastFname > text && !G.Logger.Opts.Autofix {
pline.Warnf("%q should be sorted before %q.", text, ck.lastFname)
@@ -292,7 +286,7 @@ func (ck *PlistChecker) checkPathLib(pli
switch ext := path.Ext(basename); ext {
case ".la":
- if G.Pkg != nil && !G.Pkg.vars.Defined("USE_LIBTOOL") {
+ if G.Pkg != nil && !G.Pkg.vars.Defined("USE_LIBTOOL") && ck.once.FirstTime("USE_LIBTOOL") {
pline.Warnf("Packages that install libtool libraries should define USE_LIBTOOL.")
}
}
@@ -378,9 +372,7 @@ func (ck *PlistChecker) checkPathShare(p
}
case hasPrefix(text, "share/doc/html/"):
- if G.Opts.WarnPlistDepr {
- pline.Warnf("Use of \"share/doc/html\" is deprecated. Use \"share/doc/${PKGBASE}\" instead.")
- }
+ pline.Warnf("Use of \"share/doc/html\" is deprecated. Use \"share/doc/${PKGBASE}\" instead.")
case G.Pkg != nil && G.Pkg.EffectivePkgbase != "" && (hasPrefix(text, "share/doc/"+G.Pkg.EffectivePkgbase+"/") ||
hasPrefix(text, "share/examples/"+G.Pkg.EffectivePkgbase+"/")):
@@ -398,7 +390,7 @@ func (ck *PlistChecker) checkPathShare(p
func (pline *PlistLine) CheckTrailingWhitespace() {
if hasSuffix(pline.text, " ") || hasSuffix(pline.text, "\t") {
- pline.Errorf("pkgsrc does not support filenames ending in whitespace.")
+ pline.Errorf("Pkgsrc does not support filenames ending in whitespace.")
G.Explain(
"Each character in the PLIST is relevant, even trailing whitespace.")
}
@@ -420,7 +412,7 @@ func (pline *PlistLine) CheckDirective(c
case "exec", "unexec":
switch {
case contains(arg, "ldconfig") && !contains(arg, "/usr/bin/true"):
- pline.Errorf("ldconfig must be used with \"||/usr/bin/true\".")
+ pline.Errorf("The ldconfig command must be used with \"||/usr/bin/true\".")
}
case "comment":
Index: pkgsrc/pkgtools/pkglint/files/util.go
diff -u pkgsrc/pkgtools/pkglint/files/util.go:1.36 pkgsrc/pkgtools/pkglint/files/util.go:1.37
--- pkgsrc/pkgtools/pkglint/files/util.go:1.36 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/util.go Thu Feb 21 22:49:03 2019
@@ -347,7 +347,8 @@ func mkopSubst(s string, left bool, from
})
}
-// relpath returns the relative path from `from` to `to`.
+// relpath returns the relative path from the directory "from"
+// to the filesystem entry "to".
func relpath(from, to string) string {
// From "dir" to "dir/subdir/...".
@@ -454,30 +455,56 @@ func (o *Once) check(key uint64) bool {
// Scope remembers which variables are defined and which are used
// in a certain scope, such as a package or a file.
type Scope struct {
- defined map[string]MkLine
+ firstDef map[string]MkLine // TODO: Can this be removed?
+ lastDef map[string]MkLine
+ value map[string]string
used map[string]MkLine
fallback map[string]string
}
func NewScope() Scope {
- return Scope{make(map[string]MkLine), make(map[string]MkLine), make(map[string]string)}
+ return Scope{
+ make(map[string]MkLine),
+ make(map[string]MkLine),
+ make(map[string]string),
+ make(map[string]MkLine),
+ make(map[string]string)}
}
// Define marks the variable and its canonicalized form as defined.
func (s *Scope) Define(varname string, mkline MkLine) {
- if s.defined[varname] == nil {
- s.defined[varname] = mkline
- if trace.Tracing {
- trace.Step2("Defining %q in %s", varname, mkline.String())
+ def := func(name string) {
+ if s.firstDef[name] == nil {
+ s.firstDef[name] = mkline
+ if trace.Tracing {
+ trace.Step2("Defining %q for the first time in %s", name, mkline.String())
+ }
+ } else if trace.Tracing {
+ trace.Step2("Defining %q in %s", name, mkline.String())
+ }
+
+ s.lastDef[name] = mkline
+
+ // In most cases the defining lines are indeed variable assignments.
+ // Exceptions are comments that only document the variable but still mark
+ // it as defined so that it doesn't produce the "used but not defined" warning.
+ if mkline.IsVarassign() || mkline.IsCommentedVarassign() {
+
+ switch mkline.Op() {
+ case opAssign, opAssignEval, opAssignShell:
+ s.value[name] = mkline.Value()
+ case opAssignAppend:
+ s.value[name] += " " + mkline.Value()
+ case opAssignDefault:
+ // No change to the value.
+ }
}
}
+ def(varname)
varcanon := varnameCanon(varname)
- if varcanon != varname && s.defined[varcanon] == nil {
- s.defined[varcanon] = mkline
- if trace.Tracing {
- trace.Step2("Defining %q in %s", varcanon, mkline.String())
- }
+ if varcanon != varname {
+ def(varcanon)
}
}
@@ -509,12 +536,12 @@ func (s *Scope) Use(varname string, line
// Even if Defined returns true, FirstDefinition doesn't necessarily return true
// since the latter ignores the default definitions from vardefs.go, keyword dummyVardefMkline.
func (s *Scope) Defined(varname string) bool {
- return s.defined[varname] != nil
+ return s.firstDef[varname] != nil
}
// DefinedSimilar tests whether the variable or its canonicalized form is defined.
func (s *Scope) DefinedSimilar(varname string) bool {
- if s.defined[varname] != nil {
+ if s.firstDef[varname] != nil {
if trace.Tracing {
trace.Step1("Variable %q is defined", varname)
}
@@ -522,7 +549,7 @@ func (s *Scope) DefinedSimilar(varname s
}
varcanon := varnameCanon(varname)
- if s.defined[varcanon] != nil {
+ if s.firstDef[varcanon] != nil {
if trace.Tracing {
trace.Step2("Variable %q (similar to %q) is defined", varcanon, varname)
}
@@ -549,7 +576,25 @@ func (s *Scope) UsedSimilar(varname stri
//
// Having multiple definitions is typical in the branches of "if" statements.
func (s *Scope) FirstDefinition(varname string) MkLine {
- mkline := s.defined[varname]
+ mkline := s.firstDef[varname]
+ if mkline != nil && mkline.IsVarassign() {
+ lastLine := s.LastDefinition(varname)
+ if lastLine != mkline {
+ mkline.Notef("FirstDefinition differs from LastDefinition in %s.", mkline.RefTo(lastLine))
+ }
+ return mkline
+ }
+ return nil // See NewPackage and G.Pkgsrc.UserDefinedVars
+}
+
+// LastDefinition returns the line in which the variable has been defined last.
+//
+// Having multiple definitions is typical in the branches of "if" statements.
+//
+// Another typical case involves two files: the included file defines a default
+// value, and the including file later overrides that value.
+func (s *Scope) LastDefinition(varname string) MkLine {
+ mkline := s.lastDef[varname]
if mkline != nil && mkline.IsVarassign() {
return mkline
}
@@ -560,8 +605,23 @@ func (s *Scope) FirstUse(varname string)
return s.used[varname]
}
-func (s *Scope) Value(varname string) (value string, found bool) {
- mkline := s.FirstDefinition(varname)
+// LastValue returns the value from the last variable definition.
+//
+// If an empty string is returned this can mean either that the
+// variable value is indeed the empty string or that the variable
+// was not found. To distinguish these cases, call LastValueFound instead.
+func (s *Scope) LastValue(varname string) string {
+ value, _ := s.LastValueFound(varname)
+ return value
+}
+
+func (s *Scope) LastValueFound(varname string) (value string, found bool) {
+ value, found = s.value[varname]
+ if found {
+ return
+ }
+
+ mkline := s.LastDefinition(varname)
if mkline != nil {
return mkline.Value(), true
}
@@ -573,13 +633,14 @@ func (s *Scope) Value(varname string) (v
func (s *Scope) DefineAll(other Scope) {
var varnames []string
- for varname := range other.defined {
+ for varname := range other.firstDef {
varnames = append(varnames, varname)
}
sort.Strings(varnames)
for _, varname := range varnames {
- s.Define(varname, other.defined[varname])
+ s.Define(varname, other.firstDef[varname])
+ s.Define(varname, other.lastDef[varname])
}
}
@@ -661,22 +722,26 @@ func naturalLess(str1, str2 string) bool
return len1 < len2
}
-// RedundantScope checks for redundant variable definitions and accidentally
-// overwriting variables. It tries to be as correct as possible by not flagging
-// anything that is defined conditionally. There may be some edge cases though
-// like defining PKGNAME, then evaluating it using :=, then defining it again.
-// This pattern is so error-prone that it should not appear in pkgsrc at all,
-// thus pkglint doesn't even expect it. (Well, except for the PKGNAME case,
-// but that's deep in the infrastructure and only affects the "nb13" extension.)
+// RedundantScope checks for redundant variable definitions and for variables
+// that are accidentally overwritten. It tries to be as correct as possible
+// by not flagging anything that is defined conditionally.
+//
+// There may be some edge cases though like defining PKGNAME, then evaluating
+// it using :=, then defining it again. This pattern is so error-prone that
+// it should not appear in pkgsrc at all, thus pkglint doesn't even expect it.
+// (Well, except for the PKGNAME case, but that's deep in the infrastructure
+// and only affects the "nb13" extension.)
type RedundantScope struct {
vars map[string]*redundantScopeVarinfo
dirLevel int // The number of enclosing directives (.if, .for).
- OnIgnore func(old, new MkLine)
+ includePath includePath
+ OnRedundant func(old, new MkLine)
OnOverwrite func(old, new MkLine)
}
type redundantScopeVarinfo struct {
- mkline MkLine
- value string
+ mkline MkLine
+ includePath includePath
+ value string
}
func NewRedundantScope() *RedundantScope {
@@ -684,10 +749,19 @@ func NewRedundantScope() *RedundantScope
}
func (s *RedundantScope) Handle(mkline MkLine) {
+ if mkline.firstLine == 1 {
+ s.includePath.push(mkline.Location.Filename)
+ } else {
+ s.includePath.popUntil(mkline.Location.Filename)
+ }
+
switch {
case mkline.IsVarassign():
varname := mkline.Varname()
if s.dirLevel != 0 {
+ // Since the variable is defined or assigned conditionally,
+ // it becomes too complicated for pkglint to check all possible
+ // code paths. Therefore ignore the variable from now on.
s.vars[varname] = nil
break
}
@@ -707,7 +781,7 @@ func (s *RedundantScope) Handle(mkline M
if op == opAssignAppend {
value = " " + value
}
- s.vars[varname] = &redundantScopeVarinfo{mkline, value}
+ s.vars[varname] = &redundantScopeVarinfo{mkline, s.includePath.copy(), value}
}
} else if existing != nil {
@@ -717,12 +791,24 @@ func (s *RedundantScope) Handle(mkline M
switch op {
case opAssign:
- s.OnOverwrite(existing.mkline, mkline)
+ if s.includePath.includes(existing.includePath) {
+ // This is the usual pattern of including a file and
+ // then overwriting some of them. Although technically
+ // this overwrites the previous definition, it is not
+ // worth a warning since this is used a lot and
+ // intentionally.
+ } else {
+ s.OnOverwrite(existing.mkline, mkline)
+ }
existing.value = value
case opAssignAppend:
existing.value += " " + value
case opAssignDefault:
- s.OnIgnore(existing.mkline, mkline)
+ if existing.includePath.includes(s.includePath) {
+ s.OnRedundant(mkline, existing.mkline)
+ } else if s.includePath.includes(existing.includePath) || s.includePath.equals(existing.includePath) {
+ s.OnRedundant(existing.mkline, mkline)
+ }
case opAssignShell, opAssignEval:
s.vars[varname] = nil // Won't be checked further.
}
@@ -738,6 +824,46 @@ func (s *RedundantScope) Handle(mkline M
}
}
+type includePath struct {
+ files []string
+}
+
+func (p *includePath) push(filename string) {
+ p.files = append(p.files, filename)
+}
+
+func (p *includePath) popUntil(filename string) {
+ for p.files[len(p.files)-1] != filename {
+ p.files = p.files[:len(p.files)-1]
+ }
+}
+
+func (p *includePath) includes(other includePath) bool {
+ for i, filename := range p.files {
+ if i < len(other.files) && other.files[i] == filename {
+ continue
+ }
+ return false
+ }
+ return len(p.files) < len(other.files)
+}
+
+func (p *includePath) equals(other includePath) bool {
+ if len(p.files) != len(other.files) {
+ return false
+ }
+ for i, filename := range p.files {
+ if other.files[i] != filename {
+ return false
+ }
+ }
+ return true
+}
+
+func (p *includePath) copy() includePath {
+ return includePath{append([]string(nil), p.files...)}
+}
+
// IsPrefs returns whether the given file, when included, loads the user
// preferences.
func IsPrefs(filename string) bool {
Index: pkgsrc/pkgtools/pkglint/files/util_test.go
diff -u pkgsrc/pkgtools/pkglint/files/util_test.go:1.22 pkgsrc/pkgtools/pkglint/files/util_test.go:1.23
--- pkgsrc/pkgtools/pkglint/files/util_test.go:1.22 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/util_test.go Thu Feb 21 22:49:03 2019
@@ -322,6 +322,24 @@ func (s *Suite) Test_isLocallyModified(c
c.Check(isLocallyModified(t.File("unmodified")), equals, false)
}
+func (s *Suite) Test_Scope_Define(c *check.C) {
+ t := s.Init(c)
+
+ scope := NewScope()
+ scope.Define("BUILD_DIRS", t.NewMkLine("file.mk", 121, "BUILD_DIRS=\tone two three"))
+
+ c.Check(scope.LastValue("BUILD_DIRS"), equals, "one two three")
+
+ scope.Define("BUILD_DIRS", t.NewMkLine("file.mk", 123, "BUILD_DIRS+=\tfour"))
+
+ c.Check(scope.LastValue("BUILD_DIRS"), equals, "one two three four")
+
+ // Later default assignments do not have an effect.
+ scope.Define("BUILD_DIRS", t.NewMkLine("file.mk", 123, "BUILD_DIRS?=\tdefault"))
+
+ c.Check(scope.LastValue("BUILD_DIRS"), equals, "one two three four")
+}
+
func (s *Suite) Test_Scope_Defined(c *check.C) {
t := s.Init(c)
@@ -360,7 +378,8 @@ func (s *Suite) Test_Scope_DefineAll(c *
dst := NewScope()
dst.DefineAll(src)
- c.Check(dst.defined, check.HasLen, 0)
+ c.Check(dst.firstDef, check.HasLen, 0)
+ c.Check(dst.lastDef, check.HasLen, 0)
c.Check(dst.used, check.HasLen, 0)
src.Define("VAR", t.NewMkLine("file.mk", 1, "VAR=value"))
@@ -373,7 +392,7 @@ func (s *Suite) Test_Scope_FirstDefiniti
t := s.Init(c)
mkline1 := t.NewMkLine("fname.mk", 3, "VAR=\tvalue")
- mkline2 := t.NewMkLine("fname.mk", 3, ".if ${VAR::=value}")
+ mkline2 := t.NewMkLine("fname.mk", 3, ".if ${SNEAKY::=value}")
scope := NewScope()
scope.Define("VAR", mkline1)
@@ -388,6 +407,25 @@ func (s *Suite) Test_Scope_FirstDefiniti
t.Check(scope.FirstDefinition("SNEAKY"), check.IsNil)
}
+func (s *Suite) Test_Scope_LastValue(c *check.C) {
+ t := s.Init(c)
+
+ mklines := t.NewMkLines("file.mk",
+ MkRcsID,
+ "VAR=\tfirst",
+ "VAR=\tsecond",
+ ".if 1",
+ "VAR=\tthird (conditional)",
+ ".endif")
+
+ mklines.Check()
+
+ t.Check(mklines.vars.LastValue("VAR"), equals, "third (conditional)")
+
+ t.CheckOutputLines(
+ "WARN: file.mk:2: VAR is defined but not used.")
+}
+
func (s *Suite) Test_Scope__no_tracing(c *check.C) {
t := s.Init(c)
@@ -672,3 +710,28 @@ func (s *Suite) Test_StringInterner(c *c
t.Check(si.Intern("Hello, world"), equals, "Hello, world")
t.Check(si.Intern("Hello, world"[0:5]), equals, "Hello")
}
+
+func (s *Suite) Test_includePath_includes(c *check.C) {
+ t := s.Init(c)
+
+ path := func(locations ...string) includePath {
+ return includePath{locations}
+ }
+
+ var (
+ m = path("Makefile")
+ mc = path("Makefile", "Makefile.common")
+ mco = path("Makefile", "Makefile.common", "other.mk")
+ mo = path("Makefile", "other.mk")
+ )
+
+ t.Check(m.includes(m), equals, false)
+
+ t.Check(m.includes(mc), equals, true)
+ t.Check(m.includes(mco), equals, true)
+ t.Check(mc.includes(mco), equals, true)
+
+ t.Check(mc.includes(m), equals, false)
+ t.Check(mc.includes(mo), equals, false)
+ t.Check(mo.includes(mc), equals, false)
+}
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.48 pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.49
--- pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.48 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck.go Thu Feb 21 22:49:03 2019
@@ -232,14 +232,6 @@ func (cv *VartypeCheck) Comment() {
cv.Warnf("COMMENT should not begin with %q.", first)
}
- if m, isA := match1(value, `\b(is an?)\b`); m {
- cv.Warnf("COMMENT should not contain %q.", isA)
- G.Explain(
- "The words \"package is a\" are redundant.",
- "Since every package comment could start with them,",
- "it is better to remove this redundancy in all cases.")
- }
-
if G.Pkg != nil && G.Pkg.EffectivePkgbase != "" {
pkgbase := G.Pkg.EffectivePkgbase
if hasPrefix(strings.ToLower(value), strings.ToLower(pkgbase+" ")) {
@@ -255,6 +247,14 @@ func (cv *VartypeCheck) Comment() {
cv.Warnf("COMMENT should start with a capital letter.")
}
+ if m, isA := match1(value, `\b(is an?)\b`); m {
+ cv.Warnf("COMMENT should not contain %q.", isA)
+ G.Explain(
+ "The words \"package is a\" are redundant.",
+ "Since every package comment could start with them,",
+ "it is better to remove this redundancy in all cases.")
+ }
+
if hasSuffix(value, ".") {
cv.Warnf("COMMENT should not end with a period.")
}
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.41 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.42
--- pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.41 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go Thu Feb 21 22:49:03 2019
@@ -134,6 +134,7 @@ func (s *Suite) Test_VartypeCheck_Commen
"Package is an awesome package",
"The Big New Package is a great package",
"Converter converts between measurement units",
+ "Converter is a unit converter",
"\"Official\" office suite",
"'SQL injection fuzzer")
@@ -148,7 +149,9 @@ func (s *Suite) Test_VartypeCheck_Commen
"WARN: filename:7: COMMENT should not contain \"is a\".",
"WARN: filename:8: COMMENT should not contain \"is an\".",
"WARN: filename:9: COMMENT should not contain \"is a\".",
- "WARN: filename:10: COMMENT should not start with the package name.")
+ "WARN: filename:10: COMMENT should not start with the package name.",
+ "WARN: filename:11: COMMENT should not start with the package name.",
+ "WARN: filename:11: COMMENT should not contain \"is a\".")
}
func (s *Suite) Test_VartypeCheck_ConfFiles(c *check.C) {
@@ -516,7 +519,8 @@ func (s *Suite) Test_VartypeCheck_Homepa
vt.Output(
"WARN: filename:4: HOMEPAGE should not be defined in terms of MASTER_SITEs.")
- delete(G.Pkg.vars.defined, "MASTER_SITES")
+ delete(G.Pkg.vars.firstDef, "MASTER_SITES")
+ delete(G.Pkg.vars.lastDef, "MASTER_SITES")
G.Pkg.vars.Define("MASTER_SITES", t.NewMkLine(G.Pkg.File("Makefile"), 5,
"MASTER_SITES=\thttps://cdn.NetBSD.org/pub/pkgsrc/distfiles/";))
@@ -527,7 +531,8 @@ func (s *Suite) Test_VartypeCheck_Homepa
"WARN: filename:5: HOMEPAGE should not be defined in terms of MASTER_SITEs. " +
"Use https://cdn.NetBSD.org/pub/pkgsrc/distfiles/ directly.")
- delete(G.Pkg.vars.defined, "MASTER_SITES")
+ delete(G.Pkg.vars.firstDef, "MASTER_SITES")
+ delete(G.Pkg.vars.lastDef, "MASTER_SITES")
G.Pkg.vars.Define("MASTER_SITES", t.NewMkLine(G.Pkg.File("Makefile"), 5,
"MASTER_SITES=\t${MASTER_SITE_GITHUB}"))
Index: pkgsrc/pkgtools/pkglint/files/getopt/getopt.go
diff -u pkgsrc/pkgtools/pkglint/files/getopt/getopt.go:1.7 pkgsrc/pkgtools/pkglint/files/getopt/getopt.go:1.8
--- pkgsrc/pkgtools/pkglint/files/getopt/getopt.go:1.7 Sun Dec 2 01:57:48 2018
+++ pkgsrc/pkgtools/pkglint/files/getopt/getopt.go Thu Feb 21 22:49:04 2019
@@ -34,18 +34,55 @@ func (o *Options) AddFlagGroup(shortName
return grp
}
+// AddFlagVar adds a boolean flag to the options.
+//
+// Example:
+// var verbose bool
+//
+// opts := NewOptions()
+// opts.AddFlagVar('v', "verbose", &verbose, false, "Enable verbose output")
+//
+// This option can be used in the following ways:
+// -v
+// --verbose
+// --verbose=on (or yes, 1, true, enabled)
+// --verbose=off (or no, 0, false, disabled)
func (o *Options) AddFlagVar(shortName rune, longName string, pflag *bool, defval bool, description string) {
*pflag = defval
opt := option{shortName, longName, "", description, pflag}
o.options = append(o.options, &opt)
}
+// AddStrVar adds a string option to the options.
+//
+// Example:
+// var outputFilename string
+//
+// opts := NewOptions()
+// opts.AddStrVar('o', "output", &outputFilename, "", "Write the output to the given file")
+//
+// This option can be used in the following ways:
+// -o output.txt
+// --output output.txt
+// --output=output.txt
func (o *Options) AddStrVar(shortName rune, longName string, pstr *string, defval string, description string) {
*pstr = defval
opt := option{shortName, longName, "", description, pstr}
o.options = append(o.options, &opt)
}
+// AddStrList adds a string option to the options that can be used multiple times.
+//
+// Example:
+// var includes []string
+//
+// opts := NewOptions()
+// opts.AddStrList('i', "include", &includes, nil, "Include the files matching the pattern")
+//
+// This option can be used in the following ways:
+// -i "*.txt" -i "*.docx"
+// --include "*.txt" --include "*.md"
+// --include="*.txt" --include="*.md"
func (o *Options) AddStrList(shortName rune, longName string, plist *[]string, description string) {
*plist = []string{}
opt := option{shortName, longName, "", description, plist}
Index: pkgsrc/pkgtools/pkglint/files/textproc/lexer.go
diff -u pkgsrc/pkgtools/pkglint/files/textproc/lexer.go:1.3 pkgsrc/pkgtools/pkglint/files/textproc/lexer.go:1.4
--- pkgsrc/pkgtools/pkglint/files/textproc/lexer.go:1.3 Mon Dec 17 00:15:39 2018
+++ pkgsrc/pkgtools/pkglint/files/textproc/lexer.go Thu Feb 21 22:49:04 2019
@@ -264,6 +264,7 @@ func (bs *ByteSet) Contains(b byte) bool
var (
Alnum = NewByteSet("A-Za-z0-9") // Alphanumerical, without underscore
AlnumU = NewByteSet("A-Za-z0-9_") // Alphanumerical, including underscore
+ Alpha = NewByteSet("A-Za-z") // Alphabetical, without underscore
Digit = NewByteSet("0-9") // The digits zero to nine
Upper = NewByteSet("A-Z") // The uppercase letters from A to Z
Lower = NewByteSet("a-z") // The lowercase letters from a to z
Index: pkgsrc/pkgtools/pkglint/files/textproc/lexer_test.go
diff -u pkgsrc/pkgtools/pkglint/files/textproc/lexer_test.go:1.3 pkgsrc/pkgtools/pkglint/files/textproc/lexer_test.go:1.4
--- pkgsrc/pkgtools/pkglint/files/textproc/lexer_test.go:1.3 Mon Dec 17 00:15:39 2018
+++ pkgsrc/pkgtools/pkglint/files/textproc/lexer_test.go Thu Feb 21 22:49:04 2019
@@ -391,6 +391,18 @@ func (s *Suite) Test__XPrint(c *check.C)
c.Check(set.Contains(0xA0), equals, false)
}
+func (s *Suite) Test__Alpha(c *check.C) {
+ set := Alpha
+
+ c.Check(set.Contains(0x00), equals, false)
+ c.Check(set.Contains('@'), equals, false)
+ c.Check(set.Contains('A'), equals, true)
+ c.Check(set.Contains('Z'), equals, true)
+ c.Check(set.Contains('`'), equals, false)
+ c.Check(set.Contains('a'), equals, true)
+ c.Check(set.Contains('z'), equals, true)
+}
+
func (s *Suite) Test__test_names(c *check.C) {
ck := intqa.NewTestNameChecker(c)
ck.AllowCamelCaseDescriptions(
Index: pkgsrc/pkgtools/pkglint/files/trace/tracing.go
diff -u pkgsrc/pkgtools/pkglint/files/trace/tracing.go:1.5 pkgsrc/pkgtools/pkglint/files/trace/tracing.go:1.6
--- pkgsrc/pkgtools/pkglint/files/trace/tracing.go:1.5 Fri Dec 21 08:05:24 2018
+++ pkgsrc/pkgtools/pkglint/files/trace/tracing.go Thu Feb 21 22:49:04 2019
@@ -37,14 +37,32 @@ func (t *Tracer) Step2(format string, ar
t.Stepf(format, arg0, arg1)
}
+// Call0 is used to trace a no-arguments function call.
+//
+// Usage:
+// if trace.Tracing {
+// defer trace.Call0()()
+// }
func (t *Tracer) Call0() func() {
return t.traceCall()
}
+// Call1 is used to trace a function call with a single string argument.
+//
+// Usage:
+// if trace.Tracing {
+// defer trace.Call1(str1)()
+// }
func (t *Tracer) Call1(arg1 string) func() {
return t.traceCall(arg1)
}
+// Call2 is used to trace a function call with 2 string arguments.
+//
+// Usage:
+// if trace.Tracing {
+// defer trace.Call2(str1, str2)()
+// }
func (t *Tracer) Call2(arg1, arg2 string) func() {
return t.traceCall(arg1, arg2)
}
@@ -96,19 +114,19 @@ func (t *Tracer) traceCall(args ...inter
panic("Internal pkglint error: calls to trace.Call must only occur in tracing mode")
}
- funcname := "?"
+ functionName := "?"
if pc, _, _, ok := runtime.Caller(2); ok {
if fn := runtime.FuncForPC(pc); fn != nil {
- funcname = strings.TrimPrefix(fn.Name(), "netbsd.org/pkglint.")
+ functionName = strings.TrimPrefix(fn.Name(), "netbsd.org/pkglint.")
}
}
indent := t.traceIndent()
- _, _ = fmt.Fprintf(t.Out, "TRACE: %s+ %s(%s)\n", indent, funcname, argsStr(withoutResults(args)))
+ _, _ = fmt.Fprintf(t.Out, "TRACE: %s+ %s(%s)\n", indent, functionName, argsStr(withoutResults(args)))
t.depth++
return func() {
t.depth--
- _, _ = fmt.Fprintf(t.Out, "TRACE: %s- %s(%s)\n", indent, funcname, argsStr(withResults(args)))
+ _, _ = fmt.Fprintf(t.Out, "TRACE: %s- %s(%s)\n", indent, functionName, argsStr(withResults(args)))
}
}
Home |
Main Index |
Thread Index |
Old Index