summaryrefslogtreecommitdiff
path: root/pkgtools/pkglint/files
diff options
context:
space:
mode:
Diffstat (limited to 'pkgtools/pkglint/files')
-rw-r--r--pkgtools/pkglint/files/autofix.go14
-rw-r--r--pkgtools/pkglint/files/autofix_test.go8
-rw-r--r--pkgtools/pkglint/files/check_test.go117
-rw-r--r--pkgtools/pkglint/files/line.go10
-rw-r--r--pkgtools/pkglint/files/logging_test.go77
-rw-r--r--pkgtools/pkglint/files/mklinechecker.go10
-rw-r--r--pkgtools/pkglint/files/mklinechecker_test.go78
-rw-r--r--pkgtools/pkglint/files/mklines_test.go2
-rw-r--r--pkgtools/pkglint/files/mkshparser_test.go2
-rw-r--r--pkgtools/pkglint/files/package.go4
-rw-r--r--pkgtools/pkglint/files/pkglint.go37
-rw-r--r--pkgtools/pkglint/files/pkglint_test.go6
-rw-r--r--pkgtools/pkglint/files/pkgsrc_test.go3
-rw-r--r--pkgtools/pkglint/files/plist.go2
-rw-r--r--pkgtools/pkglint/files/vardefs.go2
-rw-r--r--pkgtools/pkglint/files/vartypecheck.go90
-rw-r--r--pkgtools/pkglint/files/vartypecheck_test.go72
17 files changed, 394 insertions, 140 deletions
diff --git a/pkgtools/pkglint/files/autofix.go b/pkgtools/pkglint/files/autofix.go
index 31e08fd5f17..dd4349c8b6e 100644
--- a/pkgtools/pkglint/files/autofix.go
+++ b/pkgtools/pkglint/files/autofix.go
@@ -279,9 +279,9 @@ func (fix *Autofix) Apply() {
if logDiagnostic {
msg := sprintf(fix.diagFormat, fix.diagArgs...)
if !logFix && G.Logger.FirstTime(line.Filename, line.Linenos(), msg) {
- line.showSource(G.out)
+ line.showSource(G.Logger.out)
}
- G.Logf(fix.level, line.Filename, line.Linenos(), fix.diagFormat, msg)
+ G.Logger.Logf(fix.level, line.Filename, line.Linenos(), fix.diagFormat, msg)
}
if logFix {
@@ -290,20 +290,20 @@ func (fix *Autofix) Apply() {
if action.lineno != 0 {
lineno = strconv.Itoa(action.lineno)
}
- G.Logf(AutofixLogLevel, line.Filename, lineno, AutofixFormat, action.description)
+ G.Logger.Logf(AutofixLogLevel, line.Filename, lineno, AutofixFormat, action.description)
}
}
if logDiagnostic || logFix {
if logFix {
- line.showSource(G.out)
+ line.showSource(G.Logger.out)
}
if logDiagnostic && len(fix.explanation) > 0 {
line.Explain(fix.explanation...)
}
if G.Logger.Opts.ShowSource {
if !(G.Logger.Opts.Explain && logDiagnostic && len(fix.explanation) > 0) {
- G.out.Separate()
+ G.Logger.out.Separate()
}
}
}
@@ -394,7 +394,7 @@ func (fix *Autofix) skip() bool {
fix.diagFormat != "",
"Autofix: The diagnostic must be given before the action.")
// This check is necessary for the --only command line option.
- return !G.shallBeLogged(fix.diagFormat)
+ return !G.Logger.shallBeLogged(fix.diagFormat)
}
func (fix *Autofix) assertRealLine() {
@@ -414,7 +414,7 @@ func SaveAutofixChanges(lines Lines) (autofixed bool) {
if !G.Logger.Opts.Autofix {
for _, line := range lines.Lines {
if line.autofix != nil && line.autofix.modified {
- G.autofixAvailable = true
+ G.Logger.autofixAvailable = true
if G.Logger.Opts.ShowAutofix {
// Only in this case can the loaded lines be modified.
G.fileCache.Evict(line.Filename)
diff --git a/pkgtools/pkglint/files/autofix_test.go b/pkgtools/pkglint/files/autofix_test.go
index 19f09c4bf40..5d3e3fefd7a 100644
--- a/pkgtools/pkglint/files/autofix_test.go
+++ b/pkgtools/pkglint/files/autofix_test.go
@@ -360,7 +360,7 @@ func (s *Suite) Test_Autofix_Explain__without_explain_option(c *check.C) {
t.CheckOutputLines(
"WARN: Makefile:74: Please write row instead of line.")
- c.Check(G.explanationsAvailable, equals, true)
+ c.Check(G.Logger.explanationsAvailable, equals, true)
}
func (s *Suite) Test_Autofix_Explain__default(c *check.C) {
@@ -380,7 +380,7 @@ func (s *Suite) Test_Autofix_Explain__default(c *check.C) {
"",
"\tExplanation",
"")
- c.Check(G.explanationsAvailable, equals, true)
+ c.Check(G.Logger.explanationsAvailable, equals, true)
}
func (s *Suite) Test_Autofix_Explain__show_autofix(c *check.C) {
@@ -401,7 +401,7 @@ func (s *Suite) Test_Autofix_Explain__show_autofix(c *check.C) {
"",
"\tExplanation",
"")
- c.Check(G.explanationsAvailable, equals, true)
+ c.Check(G.Logger.explanationsAvailable, equals, true)
}
func (s *Suite) Test_Autofix_Explain__autofix(c *check.C) {
@@ -418,7 +418,7 @@ func (s *Suite) Test_Autofix_Explain__autofix(c *check.C) {
t.CheckOutputLines(
"AUTOFIX: Makefile:74: Replacing \"line\" with \"row\".")
- c.Check(G.explanationsAvailable, equals, false) // Not necessary.
+ c.Check(G.Logger.explanationsAvailable, equals, false) // Not necessary.
}
func (s *Suite) Test_Autofix_Explain__SilentAutofixFormat(c *check.C) {
diff --git a/pkgtools/pkglint/files/check_test.go b/pkgtools/pkglint/files/check_test.go
index 421c2579435..792ca448242 100644
--- a/pkgtools/pkglint/files/check_test.go
+++ b/pkgtools/pkglint/files/check_test.go
@@ -62,8 +62,8 @@ func (s *Suite) SetUpTest(c *check.C) {
G = NewPkglint()
G.Testing = true
- G.out = NewSeparatorWriter(&t.stdout)
- G.err = NewSeparatorWriter(&t.stderr)
+ G.Logger.out = NewSeparatorWriter(&t.stdout)
+ G.Logger.err = NewSeparatorWriter(&t.stderr)
trace.Out = &t.stdout
// XXX: Maybe the tests can run a bit faster when they don't
@@ -114,7 +114,7 @@ func (s *Suite) TearDownTest(c *check.C) {
t.tmpdir = ""
t.DisableTracing()
- G = Pkglint{} // unusable because of missing Logger.out and Logger.err
+ G = unusablePkglint()
}
var _ = check.Suite(new(Suite))
@@ -675,9 +675,9 @@ func (t *Tester) Main(args ...string) int {
t.seenMain = true
// Reset the logger, for tests where t.Main is called multiple times.
- G.errors = 0
- G.warnings = 0
- G.logged = Once{}
+ G.Logger.errors = 0
+ G.Logger.warnings = 0
+ G.Logger.logged = Once{}
argv := []string{"pkglint"}
for _, arg := range args {
@@ -857,20 +857,16 @@ func (t *Tester) Output() string {
t.stdout.Reset()
t.stderr.Reset()
- G.Logger.logged = Once{}
- if G.Logger.out != nil { // Necessary because Main resets the G variable.
- G.Logger.out.state = 0 // Prevent an empty line at the beginning of the next output.
- G.Logger.err.state = 0
+ if G.Usable() {
+ G.Logger.logged = Once{}
+ if G.Logger.out != nil { // Necessary because Main resets the G variable.
+ G.Logger.out.state = 0 // Prevent an empty line at the beginning of the next output.
+ G.Logger.err.state = 0
+ }
}
G.Assertf(t.tmpdir != "", "Tester must be initialized before checking the output.")
- output := stdout + stderr
- // TODO: The explanations are wrapped. Because of this it can happen
- // that t.tmpdir is spread among multiple lines if that directory
- // name contains spaces, which is common on Windows. A temporary
- // workaround is to set TMP=/path/without/spaces.
- output = strings.Replace(output, t.tmpdir, "~", -1)
- return output
+ return strings.Replace(stdout+stderr, t.tmpdir, "~", -1)
}
// CheckOutputEmpty ensures that the output up to now is empty.
@@ -881,15 +877,90 @@ func (t *Tester) CheckOutputEmpty() {
}
// CheckOutputLines checks that the output up to now equals the given lines.
+//
// After the comparison, the output buffers are cleared so that later
// calls only check against the newly added output.
//
-// See CheckOutputEmpty.
+// See CheckOutputEmpty, CheckOutputLinesIgnoreSpace.
func (t *Tester) CheckOutputLines(expectedLines ...string) {
G.Assertf(len(expectedLines) > 0, "To check empty lines, use CheckLinesEmpty instead.")
t.CheckOutput(expectedLines)
}
+// CheckOutputLinesIgnoreSpace checks that the output up to now equals the given lines.
+// During comparison, each run of whitespace (space, tab, newline) is normalized so that
+// different line breaks are ignored. This is useful for testing line-wrapped explanations.
+//
+// After the comparison, the output buffers are cleared so that later
+// calls only check against the newly added output.
+//
+// See CheckOutputEmpty, CheckOutputLines.
+func (t *Tester) CheckOutputLinesIgnoreSpace(expectedLines ...string) {
+ G.Assertf(len(expectedLines) > 0, "To check empty lines, use CheckLinesEmpty instead.")
+ G.Assertf(t.tmpdir != "", "Tester must be initialized before checking the output.")
+
+ rawOutput := t.stdout.String() + t.stderr.String()
+ _ = t.Output() // Just to consume the output
+
+ actual, expected := t.compareOutputIgnoreSpace(rawOutput, expectedLines, t.tmpdir)
+ t.Check(actual, deepEquals, expected)
+}
+
+func (t *Tester) compareOutputIgnoreSpace(rawOutput string, expectedLines []string, tmpdir string) ([]string, []string) {
+ whitespace := regexp.MustCompile(`\s+`)
+
+ // Replace all occurrences of tmpdir in the raw output with a tilde,
+ // also covering cases where tmpdir is wrapped into multiple lines.
+ output := func() string {
+ var tmpdirPattern strings.Builder
+ for i, part := range whitespace.Split(tmpdir, -1) {
+ if i > 0 {
+ tmpdirPattern.WriteString("\\s+")
+ }
+ tmpdirPattern.WriteString(regexp.QuoteMeta(part))
+ }
+
+ return regexp.MustCompile(tmpdirPattern.String()).ReplaceAllString(rawOutput, "~")
+ }()
+
+ normSpace := func(s string) string {
+ return whitespace.ReplaceAllString(s, " ")
+ }
+ if normSpace(output) == normSpace(strings.Join(expectedLines, "\n")) {
+ return nil, nil
+ }
+
+ actualLines := strings.Split(output, "\n")
+ actualLines = actualLines[:len(actualLines)-1]
+
+ return emptyToNil(actualLines), emptyToNil(expectedLines)
+}
+
+func (s *Suite) Test_Tester_compareOutputIgnoreSpace(c *check.C) {
+ t := s.Init(c)
+
+ lines := func(lines ...string) []string { return lines }
+ test := func(rawOutput string, expectedLines []string, tmpdir string, eq bool) {
+ actual, expected := t.compareOutputIgnoreSpace(rawOutput, expectedLines, tmpdir)
+ t.Check(actual == nil && expected == nil, equals, eq)
+ }
+
+ test("", lines(), "/tmp", true)
+
+ // The expectedLines are missing a space at the end.
+ test(" \t\noutput\n\t ", lines("\toutput"), "/tmp", false)
+
+ test(" \t\noutput\n\t ", lines("\toutput\n"), "/tmp", true)
+
+ test("/tmp/\n\t \nspace", lines("~"), "/tmp/\t\t\t \n\n\nspace", true)
+
+ // The rawOutput contains more spaces than the tmpdir.
+ test("/tmp/\n\t \nspace", lines("~"), "/tmp/space", false)
+
+ // The tmpdir contains more spaces than the rawOutput.
+ test("/tmp/space", lines("~"), "/tmp/ \t\nspace", false)
+}
+
// CheckOutputMatches checks that the output up to now matches the given lines.
// Each line may either be an exact string or a regular expression.
// By convention, regular expressions are written in backticks.
@@ -955,7 +1026,7 @@ func (t *Tester) CheckOutput(expectedLines []string) {
// This is useful when stepping through the code, especially
// in combination with SetUpCommandLine("--debug").
func (t *Tester) EnableTracing() {
- G.out = NewSeparatorWriter(io.MultiWriter(os.Stdout, &t.stdout))
+ G.Logger.out = NewSeparatorWriter(io.MultiWriter(os.Stdout, &t.stdout))
trace.Out = os.Stdout
trace.Tracing = true
}
@@ -963,7 +1034,7 @@ 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() {
- G.out = NewSeparatorWriter(&t.stdout)
+ G.Logger.out = NewSeparatorWriter(&t.stdout)
trace.Out = &t.stdout
trace.Tracing = true
}
@@ -975,7 +1046,7 @@ func (t *Tester) EnableTracingToLog() {
// It is used to check all calls to trace.Result, since the compiler
// cannot check them.
func (t *Tester) EnableSilentTracing() {
- G.out = NewSeparatorWriter(&t.stdout)
+ G.Logger.out = NewSeparatorWriter(&t.stdout)
trace.Out = ioutil.Discard
trace.Tracing = true
}
@@ -984,7 +1055,9 @@ func (t *Tester) EnableSilentTracing() {
// The diagnostics go to the in-memory buffer again,
// ready to be checked with CheckOutputLines.
func (t *Tester) DisableTracing() {
- G.out = NewSeparatorWriter(&t.stdout)
+ if G.Usable() {
+ G.Logger.out = NewSeparatorWriter(&t.stdout)
+ }
trace.Tracing = false
trace.Out = nil
}
diff --git a/pkgtools/pkglint/files/line.go b/pkgtools/pkglint/files/line.go
index 376f44bc715..2b744343c0f 100644
--- a/pkgtools/pkglint/files/line.go
+++ b/pkgtools/pkglint/files/line.go
@@ -176,22 +176,22 @@ func (line *LineImpl) Fatalf(format string, args ...interface{}) {
if trace.Tracing {
trace.Stepf("Fatalf: %q, %v", format, args)
}
- G.Diag(line, Fatal, format, args...)
+ G.Logger.Diag(line, Fatal, format, args...)
}
func (line *LineImpl) Errorf(format string, args ...interface{}) {
- G.Diag(line, Error, format, args...)
+ G.Logger.Diag(line, Error, format, args...)
}
func (line *LineImpl) Warnf(format string, args ...interface{}) {
- G.Diag(line, Warn, format, args...)
+ G.Logger.Diag(line, Warn, format, args...)
}
func (line *LineImpl) Notef(format string, args ...interface{}) {
- G.Diag(line, Note, format, args...)
+ G.Logger.Diag(line, Note, format, args...)
}
-func (line *LineImpl) Explain(explanation ...string) { G.Explain(explanation...) }
+func (line *LineImpl) Explain(explanation ...string) { G.Logger.Explain(explanation...) }
func (line *LineImpl) String() string {
return sprintf("%s:%s: %s", line.Filename, line.Linenos(), line.Text)
diff --git a/pkgtools/pkglint/files/logging_test.go b/pkgtools/pkglint/files/logging_test.go
index a96f9754854..f730fef8bdf 100644
--- a/pkgtools/pkglint/files/logging_test.go
+++ b/pkgtools/pkglint/files/logging_test.go
@@ -76,7 +76,7 @@ func (s *Suite) Test_Logger_Logf__profiling(c *check.C) {
G.Logger.histo = histogram.New()
line.Warnf("Warning.")
- G.Logger.histo.PrintStats(G.out.out, "loghisto", -1)
+ G.Logger.histo.PrintStats(G.Logger.out.out, "loghisto", -1)
t.CheckOutputLines(
"WARN: filename:123: Warning.",
@@ -101,7 +101,7 @@ func (s *Suite) Test_Logger_Logf__profiling_autofix(c *check.C) {
// 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)
+ G.Logger.histo.PrintStats(G.Logger.out.out, "loghisto", -1)
t.CheckOutputLines(
"NOTE: filename:123: Autofix note.",
@@ -409,6 +409,35 @@ func (s *Suite) Test_Logger_Explain__empty_lines(c *check.C) {
"")
}
+// In an explanation, it can happen that the pkgsrc directory is mentioned.
+// While pkgsrc does not support either PKGSRCDIR or PREFIX or really any
+// other directory name to contain spaces, during pkglint development this
+// may happen because the pkgsrc root is in the temporary directory.
+//
+// In this situation, the ~ placeholder must still be properly substituted.
+func (s *Suite) Test_Logger_Explain__line_wrapped_temporary_directory(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("--explain")
+ filename := t.File("filename.mk")
+ mkline := t.NewMkLine(filename, 123, "")
+
+ mkline.Notef("Just a note to get the below explanation.")
+ G.Logger.Explain(
+ sprintf("%[1]s %[1]s %[1]s %[1]s %[1]s %[1]q", filename))
+
+ t.CheckOutputLinesIgnoreSpace(
+ "NOTE: ~/filename.mk:123: Just a note to get the below explanation.",
+ "",
+ "\t~/filename.mk",
+ "\t~/filename.mk",
+ "\t~/filename.mk",
+ "\t~/filename.mk",
+ "\t~/filename.mk",
+ "\t\"~/filename.mk\"",
+ "")
+}
+
func (s *Suite) Test_Logger_ShowSummary__explanations_with_only(c *check.C) {
t := s.Init(c)
@@ -418,21 +447,21 @@ func (s *Suite) Test_Logger_ShowSummary__explanations_with_only(c *check.C) {
// Neither the warning nor the corresponding explanation are logged.
line.Warnf("Filtered warning.")
line.Explain("Explanation for the above warning.")
- G.ShowSummary()
+ G.Logger.ShowSummary()
// Since the above warning is filtered out by the --only option,
// adding --explain to the options would not show any explanation.
// Therefore, "Run \"pkglint -e\"" is not advertised in this case,
// but see below.
- c.Check(G.explanationsAvailable, equals, false)
+ c.Check(G.Logger.explanationsAvailable, equals, false)
t.CheckOutputLines(
"Looks fine.")
line.Warnf("This warning is interesting.")
line.Explain("This explanation is available.")
- G.ShowSummary()
+ G.Logger.ShowSummary()
- c.Check(G.explanationsAvailable, equals, true)
+ c.Check(G.Logger.explanationsAvailable, equals, true)
t.CheckOutputLines(
"WARN: Makefile:27: This warning is interesting.",
"0 errors and 1 warning found.",
@@ -647,10 +676,11 @@ func (s *Suite) Test_Logger_Logf__gcc_format(c *check.C) {
t.SetUpCommandLine("--gcc-output-format")
- G.Logf(Note, "filename", "123", "Both filename and line number.", "Both filename and line number.")
- G.Logf(Note, "", "123", "No filename, only line number.", "No filename, only line number.")
- G.Logf(Note, "filename", "", "Filename without line number.", "Filename without line number.")
- G.Logf(Note, "", "", "Neither filename nor line number.", "Neither filename nor line number.")
+ logger := &G.Logger
+ logger.Logf(Note, "filename", "123", "Both filename and line number.", "Both filename and line number.")
+ logger.Logf(Note, "", "123", "No filename, only line number.", "No filename, only line number.")
+ logger.Logf(Note, "filename", "", "Filename without line number.", "Filename without line number.")
+ logger.Logf(Note, "", "", "Neither filename nor line number.", "Neither filename nor line number.")
t.CheckOutputLines(
"filename:123: note: Both filename and line number.",
@@ -664,10 +694,11 @@ func (s *Suite) Test_Logger_Logf__traditional_format(c *check.C) {
t.SetUpCommandLine("--gcc-output-format=no")
- G.Logf(Note, "filename", "123", "Both filename and line number.", "Both filename and line number.")
- G.Logf(Note, "", "123", "No filename, only line number.", "No filename, only line number.")
- G.Logf(Note, "filename", "", "Filename without line number.", "Filename without line number.")
- G.Logf(Note, "", "", "Neither filename nor line number.", "Neither filename nor line number.")
+ logger := &G.Logger
+ logger.Logf(Note, "filename", "123", "Both filename and line number.", "Both filename and line number.")
+ logger.Logf(Note, "", "123", "No filename, only line number.", "No filename, only line number.")
+ logger.Logf(Note, "filename", "", "Filename without line number.", "Filename without line number.")
+ logger.Logf(Note, "", "", "Neither filename nor line number.", "Neither filename nor line number.")
t.CheckOutputLines(
"NOTE: filename:123: Both filename and line number.",
@@ -681,7 +712,7 @@ func (s *Suite) Test_Logger_Errorf__gcc_format(c *check.C) {
t.SetUpCommandLine("--gcc-output-format")
- G.Errorf("filename", "Cannot be opened for %s.", "reading")
+ G.Logger.Errorf("filename", "Cannot be opened for %s.", "reading")
t.CheckOutputLines(
"filename: error: Cannot be opened for reading.")
@@ -693,8 +724,8 @@ func (s *Suite) Test_Logger_Logf__strange_characters(c *check.C) {
t.SetUpCommandLine("--gcc-output-format", "--source", "--explain")
- G.Logf(Note, "filename", "123", "Format.", "Unicode \U0001F645 and ANSI \x1B are never logged.")
- G.Explain(
+ G.Logger.Logf(Note, "filename", "123", "Format.", "Unicode \U0001F645 and ANSI \x1B are never logged.")
+ G.Logger.Explain(
"Even a \u0007 in the explanation is silent.")
t.CheckOutputLines(
@@ -780,17 +811,17 @@ func (s *Suite) Test_Logger_shallBeLogged(c *check.C) {
t.SetUpCommandLine( /* none */ )
- c.Check(G.shallBeLogged("Options should not contain whitespace."), equals, true)
+ c.Check(G.Logger.shallBeLogged("Options should not contain whitespace."), equals, true)
t.SetUpCommandLine("--only", "whitespace")
- c.Check(G.shallBeLogged("Options should not contain whitespace."), equals, true)
- c.Check(G.shallBeLogged("Options should not contain space."), equals, false)
+ c.Check(G.Logger.shallBeLogged("Options should not contain whitespace."), equals, true)
+ c.Check(G.Logger.shallBeLogged("Options should not contain space."), equals, false)
t.SetUpCommandLine( /* none again */ )
- c.Check(G.shallBeLogged("Options should not contain whitespace."), equals, true)
- c.Check(G.shallBeLogged("Options should not contain space."), equals, true)
+ c.Check(G.Logger.shallBeLogged("Options should not contain whitespace."), equals, true)
+ c.Check(G.Logger.shallBeLogged("Options should not contain space."), equals, true)
}
// Even if verbose logging is disabled, the "Replacing" diagnostics
@@ -816,7 +847,7 @@ func (s *Suite) Test_Logger_Logf__panic(c *check.C) {
t := s.Init(c)
t.ExpectPanic(
- func() { G.Logf(Error, "filename", "13", "No period", "No period") },
+ func() { G.Logger.Logf(Error, "filename", "13", "No period", "No period") },
"Pkglint internal error: Diagnostic format \"No period\" must end in a period.")
}
diff --git a/pkgtools/pkglint/files/mklinechecker.go b/pkgtools/pkglint/files/mklinechecker.go
index 3f8d4475cd3..2e8a258b6ff 100644
--- a/pkgtools/pkglint/files/mklinechecker.go
+++ b/pkgtools/pkglint/files/mklinechecker.go
@@ -609,8 +609,9 @@ func (ck MkLineChecker) checkVaruseModifiersRange(varuse *MkVarUse) {
}
}
-// checkVarusePermissions checks the permissions for the right-hand side
-// of a variable assignment line.
+// checkVarusePermissions checks the permissions when a variable is used,
+// be it in a variable assignment, in a shell command, a conditional, or
+// somewhere else.
//
// See checkVarassignLeftPermissions.
func (ck MkLineChecker) checkVarusePermissions(varname string, vartype *Vartype, vuc *VarUseContext) {
@@ -712,6 +713,11 @@ func (ck MkLineChecker) checkVarusePermissions(varname string, vartype *Vartype,
// Anyway, there must be a warning now since the requested use is not
// allowed. The only remaining question is about how detailed the
// warning will be.
+ ck.warnVarusePermissions(varname, vartype, directly, indirectly)
+}
+
+func (ck MkLineChecker) warnVarusePermissions(varname string, vartype *Vartype, directly, indirectly bool) {
+ mkline := ck.MkLine
anyPerms := vartype.Union()
if !anyPerms.Contains(aclpUse) && !anyPerms.Contains(aclpUseLoadtime) {
diff --git a/pkgtools/pkglint/files/mklinechecker_test.go b/pkgtools/pkglint/files/mklinechecker_test.go
index fd2fa3ea2c7..e31ec5a4810 100644
--- a/pkgtools/pkglint/files/mklinechecker_test.go
+++ b/pkgtools/pkglint/files/mklinechecker_test.go
@@ -529,6 +529,40 @@ func (s *Suite) Test_MkLineChecker_checkVarassign__URL_with_shell_special_charac
t.CheckOutputEmpty()
}
+func (s *Suite) Test_MkLineChecker_checkVarassign__list(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpMasterSite("MASTER_SITE_GITHUB", "https://github.com/")
+ t.SetUpVartypes()
+ t.SetUpCommandLine("-Wall", "--explain")
+ mklines := t.NewMkLines("filename.mk",
+ MkRcsID,
+ "SITES.distfile=\t-${MASTER_SITE_GITHUB:=project/}")
+
+ mklines.Check()
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:2: The list variable MASTER_SITE_GITHUB should not be embedded in a word.",
+ "",
+ "\tWhen a list variable has multiple elements, this expression expands",
+ "\tto something unexpected:",
+ "",
+ "\tExample: ${MASTER_SITE_SOURCEFORGE}directory/ expands to",
+ "",
+ "\t\thttps://mirror1.sf.net/ https://mirror2.sf.net/directory/",
+ "",
+ "\tThe first URL is missing the directory. To fix this, write",
+ "\t\t${MASTER_SITE_SOURCEFORGE:=directory/}.",
+ "",
+ "\tExample: -l${LIBS} expands to",
+ "",
+ "\t\t-llib1 lib2",
+ "",
+ "\tThe second library is missing the -l. To fix this, write",
+ "\t${LIBS:S,^,-l,}.",
+ "")
+}
+
func (s *Suite) Test_MkLineChecker_checkDirectiveCond(c *check.C) {
t := s.Init(c)
@@ -1481,7 +1515,8 @@ func (s *Suite) Test_MkLineChecker_checkDirectiveCond__compare_pattern_with_empt
// TODO: There should be a warning about "<>" containing invalid
// characters for a path. See VartypeCheck.Pathname
t.CheckOutputLines(
- "WARN: filename.mk:5: \"*\" is not a valid pathname.")
+ "WARN: filename.mk:5: The pathname pattern \"<>\" contains the invalid characters \"<>\".",
+ "WARN: filename.mk:5: The pathname \"*\" contains the invalid character \"*\".")
}
func (s *Suite) Test_MkLineChecker_checkDirectiveCondEmpty(c *check.C) {
@@ -1522,8 +1557,10 @@ func (s *Suite) Test_MkLineChecker_checkDirectiveCondEmpty(c *check.C) {
test(
".if ${PKGPATH:Mpattern}",
+
"NOTE: module.mk:2: PKGPATH should be compared using == instead of matching against \":Mpattern\".",
"AUTOFIX: module.mk:2: Replacing \"${PKGPATH:Mpattern}\" with \"${PKGPATH} == pattern\".",
+
".if ${PKGPATH} == pattern")
// When the pattern contains placeholders, it cannot be converted to == or !=.
@@ -1534,13 +1571,17 @@ func (s *Suite) Test_MkLineChecker_checkDirectiveCondEmpty(c *check.C) {
// The :tl modifier prevents the autofix.
test(
".if ${PKGPATH:tl:Mpattern}",
+
"NOTE: module.mk:2: PKGPATH should be compared using == instead of matching against \":Mpattern\".",
+
".if ${PKGPATH:tl:Mpattern}")
test(
".if ${PKGPATH:Ncategory/package}",
+
"NOTE: module.mk:2: PKGPATH should be compared using != instead of matching against \":Ncategory/package\".",
"AUTOFIX: module.mk:2: Replacing \"${PKGPATH:Ncategory/package}\" with \"${PKGPATH} != category/package\".",
+
".if ${PKGPATH} != category/package")
// ${PKGPATH:None:Ntwo} is a short variant of ${PKGPATH} != "one" &&
@@ -1552,26 +1593,34 @@ func (s *Suite) Test_MkLineChecker_checkDirectiveCondEmpty(c *check.C) {
// Note: this combination doesn't make sense since the patterns "one" and "two" don't overlap.
test(".if ${PKGPATH:Mone:Mtwo}",
+
"NOTE: module.mk:2: PKGPATH should be compared using == instead of matching against \":Mone\".",
"NOTE: module.mk:2: PKGPATH should be compared using == instead of matching against \":Mtwo\".",
+
".if ${PKGPATH:Mone:Mtwo}")
test(".if !empty(PKGPATH:Mpattern)",
+
"NOTE: module.mk:2: PKGPATH should be compared using == instead of matching against \":Mpattern\".",
"AUTOFIX: module.mk:2: Replacing \"!empty(PKGPATH:Mpattern)\" with \"${PKGPATH} == pattern\".",
+
".if ${PKGPATH} == pattern")
test(".if empty(PKGPATH:Mpattern)",
+
"NOTE: module.mk:2: PKGPATH should be compared using != instead of matching against \":Mpattern\".",
"AUTOFIX: module.mk:2: Replacing \"empty(PKGPATH:Mpattern)\" with \"${PKGPATH} != pattern\".",
+
".if ${PKGPATH} != pattern")
test(".if !!empty(PKGPATH:Mpattern)",
+
// TODO: When taking all the ! into account, this is actually a
// test for emptiness, therefore the diagnostics should suggest
// the != operator instead of ==.
"NOTE: module.mk:2: PKGPATH should be compared using == instead of matching against \":Mpattern\".",
"AUTOFIX: module.mk:2: Replacing \"!empty(PKGPATH:Mpattern)\" with \"(${PKGPATH} == pattern)\".",
+
// TODO: This condition could be simplified even more.
// Luckily the !! pattern doesn't occur in practice.
".if !(${PKGPATH} == pattern)")
@@ -1582,31 +1631,46 @@ func (s *Suite) Test_MkLineChecker_checkDirectiveCondEmpty(c *check.C) {
test(
".if ${PKGPATH:Mpattern}",
+
"NOTE: module.mk:2: PKGPATH should be compared using == instead of matching against \":Mpattern\".",
"AUTOFIX: module.mk:2: Replacing \"${PKGPATH:Mpattern}\" with \"${PKGPATH} == pattern\".",
+
".if ${PKGPATH} == pattern")
test(
".if !${PKGPATH:Mpattern}",
+
"NOTE: module.mk:2: PKGPATH should be compared using != instead of matching against \":Mpattern\".",
"AUTOFIX: module.mk:2: Replacing \"!${PKGPATH:Mpattern}\" with \"${PKGPATH} != pattern\".",
+
".if ${PKGPATH} != pattern")
// This pattern with spaces doesn't make sense at all in the :M
// modifier since it can never match.
+ // Or can it, if the PKGPATH contains quotes?
+ // How exactly does bmake apply the matching here, are both values unquoted?
test(
".if ${PKGPATH:Mpattern with spaces}",
- nil...)
+
+ "WARN: module.mk:2: The pathname pattern \"pattern with spaces\" contains the invalid characters \" \".",
+
+ ".if ${PKGPATH:Mpattern with spaces}")
// TODO: ".if ${PKGPATH} == \"pattern with spaces\"")
test(
".if ${PKGPATH:M'pattern with spaces'}",
- nil...)
+
+ "WARN: module.mk:2: The pathname pattern \"'pattern with spaces'\" contains the invalid characters \"' '\".",
+
+ ".if ${PKGPATH:M'pattern with spaces'}")
// TODO: ".if ${PKGPATH} == 'pattern with spaces'")
test(
".if ${PKGPATH:M&&}",
- nil...)
+
+ "WARN: module.mk:2: The pathname pattern \"&&\" contains the invalid characters \"&&\".",
+
+ ".if ${PKGPATH:M&&}")
// TODO: ".if ${PKGPATH} == '&&'")
// If PKGPATH is "", the condition is false.
@@ -1623,8 +1687,10 @@ func (s *Suite) Test_MkLineChecker_checkDirectiveCondEmpty(c *check.C) {
// has been included, like everywhere else.
test(
".if ${PKGPATH:Nnegative-pattern}",
+
"NOTE: module.mk:2: PKGPATH should be compared using != instead of matching against \":Nnegative-pattern\".",
"AUTOFIX: module.mk:2: Replacing \"${PKGPATH:Nnegative-pattern}\" with \"${PKGPATH} != negative-pattern\".",
+
".if ${PKGPATH} != negative-pattern")
// Since UNKNOWN is not a well-known system-provided variable that is
@@ -1636,17 +1702,21 @@ func (s *Suite) Test_MkLineChecker_checkDirectiveCondEmpty(c *check.C) {
test(
".if ${PKGPATH:Mpath1} || ${PKGPATH:Mpath2}",
+
"NOTE: module.mk:2: PKGPATH should be compared using == instead of matching against \":Mpath1\".",
"NOTE: module.mk:2: PKGPATH should be compared using == instead of matching against \":Mpath2\".",
"AUTOFIX: module.mk:2: Replacing \"${PKGPATH:Mpath1}\" with \"(${PKGPATH} == path1)\".",
"AUTOFIX: module.mk:2: Replacing \"${PKGPATH:Mpath2}\" with \"(${PKGPATH} == path2)\".",
+
// TODO: remove the redundant parentheses
".if (${PKGPATH} == path1) || (${PKGPATH} == path2)")
test(
".if (((((${PKGPATH:Mpath})))))",
+
"NOTE: module.mk:2: PKGPATH should be compared using == instead of matching against \":Mpath\".",
"AUTOFIX: module.mk:2: Replacing \"${PKGPATH:Mpath}\" with \"${PKGPATH} == path\".",
+
".if (((((${PKGPATH} == path)))))")
}
diff --git a/pkgtools/pkglint/files/mklines_test.go b/pkgtools/pkglint/files/mklines_test.go
index 83c6a5a095a..6ed3201fe1b 100644
--- a/pkgtools/pkglint/files/mklines_test.go
+++ b/pkgtools/pkglint/files/mklines_test.go
@@ -301,7 +301,7 @@ func (s *Suite) Test_MkLines_CheckForUsedComment(c *check.C) {
// TODO: What if there is an introductory comment first? That should stay at the top of the file.
// TODO: What if the "used by" comments appear in the second paragraph, preceded by only comments and empty lines?
- c.Check(G.autofixAvailable, equals, true)
+ c.Check(G.Logger.autofixAvailable, equals, true)
}
func (s *Suite) Test_MkLines_collectDefinedVariables(c *check.C) {
diff --git a/pkgtools/pkglint/files/mkshparser_test.go b/pkgtools/pkglint/files/mkshparser_test.go
index da64bc2d4b8..dcaf95913c9 100644
--- a/pkgtools/pkglint/files/mkshparser_test.go
+++ b/pkgtools/pkglint/files/mkshparser_test.go
@@ -61,7 +61,7 @@ func (s *ShSuite) SetUpTest(c *check.C) {
}
func (s *ShSuite) TearDownTest(c *check.C) {
- G = Pkglint{} // Make it unusable
+ G = unusablePkglint()
}
func (s *ShSuite) Test_ShellParser__program(c *check.C) {
diff --git a/pkgtools/pkglint/files/package.go b/pkgtools/pkglint/files/package.go
index 82b98fbec4b..c12421f1b8e 100644
--- a/pkgtools/pkglint/files/package.go
+++ b/pkgtools/pkglint/files/package.go
@@ -285,9 +285,9 @@ func (pkg *Package) loadPackageMakefile() (MkLines, MkLines) {
// a single string. Maybe it makes sense to print the file inclusion hierarchy
// to quickly see files that cannot be included because of unresolved variables.
if G.Opts.DumpMakefile {
- G.out.WriteLine("Whole Makefile (with all included files) follows:")
+ G.Logger.out.WriteLine("Whole Makefile (with all included files) follows:")
for _, line := range allLines.lines.Lines {
- G.out.WriteLine(line.String())
+ G.Logger.out.WriteLine(line.String())
}
}
diff --git a/pkgtools/pkglint/files/pkglint.go b/pkgtools/pkglint/files/pkglint.go
index 2661fcf82ea..370657b56bd 100644
--- a/pkgtools/pkglint/files/pkglint.go
+++ b/pkgtools/pkglint/files/pkglint.go
@@ -33,7 +33,7 @@ type Pkglint struct {
cvsEntriesDir string // Cached to avoid I/O
cvsEntriesLines Lines
- Logger
+ Logger Logger
loaded *histogram.Histogram
res regex.Registry
@@ -59,6 +59,11 @@ func NewPkglint() Pkglint {
interner: NewStringInterner()}
}
+// unusablePkglint returns a pkglint object that crashes as early as possible.
+// This is to ensure that tests are properly initialized and shut down.
+func unusablePkglint() Pkglint { return Pkglint{} }
+func (pkglint *Pkglint) Usable() bool { return pkglint != nil }
+
type InterPackage struct {
hashes map[string]*Hash // Maps "alg:filename" => hash (inter-package check).
usedLicenses map[string]struct{} // Maps "license name" => true (inter-package check).
@@ -155,13 +160,13 @@ var (
)
func Main() int {
- G.out = NewSeparatorWriter(os.Stdout)
- G.err = NewSeparatorWriter(os.Stderr)
+ G.Logger.out = NewSeparatorWriter(os.Stdout)
+ G.Logger.err = NewSeparatorWriter(os.Stderr)
trace.Out = os.Stdout
exitCode := G.Main(os.Args...)
if G.Opts.Profiling {
- G = Pkglint{} // Free all memory.
- runtime.GC() // For detecting possible memory leaks; see qa-pkglint.
+ G = unusablePkglint() // Free all memory.
+ runtime.GC() // For detecting possible memory leaks; see qa-pkglint.
}
return exitCode
}
@@ -216,14 +221,14 @@ func (pkglint *Pkglint) Main(argv ...string) (exitCode int) {
defer pprof.StopCPUProfile()
pkglint.res.Profiling()
- pkglint.histo = histogram.New()
+ pkglint.Logger.histo = histogram.New()
pkglint.loaded = histogram.New()
defer func() {
- pkglint.out.Write("")
- pkglint.histo.PrintStats(pkglint.out.out, "loghisto", -1)
- pkglint.res.PrintStats(pkglint.out.out)
- pkglint.loaded.PrintStats(pkglint.out.out, "loaded", 10)
- pkglint.out.WriteLine(sprintf("fileCache: %d hits, %d misses", pkglint.fileCache.hits, pkglint.fileCache.misses))
+ pkglint.Logger.out.Write("")
+ pkglint.Logger.histo.PrintStats(pkglint.Logger.out.out, "loghisto", -1)
+ pkglint.res.PrintStats(pkglint.Logger.out.out)
+ pkglint.loaded.PrintStats(pkglint.Logger.out.out, "loaded", 10)
+ pkglint.Logger.out.WriteLine(sprintf("fileCache: %d hits, %d misses", pkglint.fileCache.hits, pkglint.fileCache.misses))
}()
}
@@ -266,8 +271,8 @@ func (pkglint *Pkglint) Main(argv ...string) (exitCode int) {
pkglint.Pkgsrc.checkToplevelUnusedLicenses()
- pkglint.ShowSummary()
- if pkglint.errors != 0 {
+ pkglint.Logger.ShowSummary()
+ if pkglint.Logger.errors != 0 {
return 1
}
return 0
@@ -307,7 +312,7 @@ func (pkglint *Pkglint) ParseCommandLine(args []string) int {
remainingArgs, err := opts.Parse(args)
if err != nil {
- errOut := pkglint.err.out
+ errOut := pkglint.Logger.err.out
_, _ = fmt.Fprintln(errOut, err)
_, _ = fmt.Fprintln(errOut, "")
opts.Help(errOut, "pkglint [options] dir...")
@@ -316,12 +321,12 @@ func (pkglint *Pkglint) ParseCommandLine(args []string) int {
gopts.args = remainingArgs
if gopts.ShowHelp {
- opts.Help(pkglint.out.out, "pkglint [options] dir...")
+ opts.Help(pkglint.Logger.out.out, "pkglint [options] dir...")
return 0
}
if pkglint.Opts.ShowVersion {
- _, _ = fmt.Fprintf(pkglint.out.out, "%s\n", confVersion)
+ _, _ = fmt.Fprintf(pkglint.Logger.out.out, "%s\n", confVersion)
return 0
}
diff --git a/pkgtools/pkglint/files/pkglint_test.go b/pkgtools/pkglint/files/pkglint_test.go
index fdb22511997..5bf8a58bd00 100644
--- a/pkgtools/pkglint/files/pkglint_test.go
+++ b/pkgtools/pkglint/files/pkglint_test.go
@@ -109,7 +109,7 @@ func (s *Suite) Test_Pkglint_Main__panic(c *check.C) {
pkg := t.SetUpPackage("category/package")
- G.out = nil // Force an error that cannot happen in practice.
+ G.Logger.out = nil // Force an error that cannot happen in practice.
c.Check(
func() { t.Main(pkg) },
@@ -280,8 +280,8 @@ func (s *Suite) Test_Pkglint__realistic(c *check.C) {
cmdline := os.Getenv("PKGLINT_TESTCMDLINE")
if cmdline != "" {
- G.out = NewSeparatorWriter(os.Stdout)
- G.err = NewSeparatorWriter(os.Stderr)
+ G.Logger.out = NewSeparatorWriter(os.Stdout)
+ G.Logger.err = NewSeparatorWriter(os.Stderr)
trace.Out = os.Stdout
t.Main(strings.Fields(cmdline)...)
}
diff --git a/pkgtools/pkglint/files/pkgsrc_test.go b/pkgtools/pkglint/files/pkgsrc_test.go
index 741b0473050..e61c672de77 100644
--- a/pkgtools/pkglint/files/pkgsrc_test.go
+++ b/pkgtools/pkglint/files/pkgsrc_test.go
@@ -738,5 +738,6 @@ func (s *Suite) Test_Pkgsrc_guessVariableType__SKIP(c *check.C) {
// There is no warning for the += operator in line 3 since the variable type
// (although guessed) is a list of things, and lists may be appended to.
t.CheckOutputLines(
- "WARN: filename.mk:2: \"\\\"bad*pathname\\\"\" is not a valid pathname mask.")
+ "WARN: filename.mk:2: The pathname pattern \"\\\"bad*pathname\\\"\" " +
+ "contains the invalid characters \"\\\"\\\"\".")
}
diff --git a/pkgtools/pkglint/files/plist.go b/pkgtools/pkglint/files/plist.go
index 1654079e9fb..09c17fc2737 100644
--- a/pkgtools/pkglint/files/plist.go
+++ b/pkgtools/pkglint/files/plist.go
@@ -536,7 +536,7 @@ func (s *plistLineSorter) Sort() {
return
}
- if !G.shallBeLogged("%q should be sorted before %q.") {
+ if !G.Logger.shallBeLogged("%q should be sorted before %q.") {
return
}
if len(s.middle) == 0 {
diff --git a/pkgtools/pkglint/files/vardefs.go b/pkgtools/pkglint/files/vardefs.go
index 2c46b1705af..5d35ddeffa8 100644
--- a/pkgtools/pkglint/files/vardefs.go
+++ b/pkgtools/pkglint/files/vardefs.go
@@ -1281,7 +1281,7 @@ func (reg *VarTypeRegistry) Init(src *Pkgsrc) {
pkg("OVERRIDE_DIRDEPTH*", BtInteger)
pkg("OVERRIDE_GNU_CONFIG_SCRIPTS", BtYes)
pkg("OWNER", BtMailAddress)
- pkglist("OWN_DIRS", BtPathname)
+ pkglist("OWN_DIRS", BtPathmask)
pkglist("OWN_DIRS_PERMS", BtPerms)
sys("PAMBASE", BtPathname)
usr("PAM_DEFAULT", enum("linux-pam openpam solaris-pam"))
diff --git a/pkgtools/pkglint/files/vartypecheck.go b/pkgtools/pkglint/files/vartypecheck.go
index 7fafc10ba5b..cc962597e00 100644
--- a/pkgtools/pkglint/files/vartypecheck.go
+++ b/pkgtools/pkglint/files/vartypecheck.go
@@ -1,6 +1,7 @@
package pkglint
import (
+ "netbsd.org/pkglint/regex"
"path"
"sort"
"strings"
@@ -512,11 +513,29 @@ func (cv *VartypeCheck) FetchURL() {
}
}
- // TODO: Replace the regular expression by accessing the MkVarUse.
- if m, name, subdir := match2(cv.Value, `\$\{(MASTER_SITE_[^:]*).*:=(.*)\}$`); m {
+ tokens := cv.MkLine.Tokenize(cv.Value, false)
+ for _, token := range tokens {
+ varUse := token.Varuse
+ if varUse == nil {
+ continue
+ }
+
+ name := varUse.varname
+ if !hasPrefix(name, "MASTER_SITE_") {
+ continue
+ }
+
if G.Pkgsrc.MasterSiteVarToURL[name] == "" {
- cv.Errorf("The site %s does not exist.", name)
+ if !(G.Pkg != nil && G.Pkg.vars.Defined(name)) {
+ cv.Errorf("The site %s does not exist.", name)
+ }
}
+
+ if len(varUse.modifiers) != 1 || !hasPrefix(varUse.modifiers[0].Text, "=") {
+ continue
+ }
+
+ subdir := varUse.modifiers[0].Text[1:]
if !hasSuffix(subdir, "/") {
cv.Errorf("The subdirectory in %s must end with a slash.", name)
}
@@ -527,28 +546,38 @@ func (cv *VartypeCheck) FetchURL() {
//
// See http://www.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap03.html#tag_03_169
func (cv *VartypeCheck) Filename() {
- switch {
- case cv.Op == opUseMatch:
- break
- case contains(cv.ValueNoVar, "/"):
- cv.Warnf("A filename should not contain a slash.")
- case !matches(cv.ValueNoVar, `^[-0-9@A-Za-z.,_~+%]*$`):
- cv.Warnf("%q is not a valid filename.", cv.Value)
+ valid := regex.Pattern(ifelseStr(
+ cv.Op == opUseMatch,
+ `[%*+,\-.0-9?@A-Z\[\]_a-z~]`,
+ `[%+,\-.0-9@A-Z_a-z~]`))
+
+ invalid := replaceAll(cv.ValueNoVar, valid, "")
+ if invalid == "" {
+ return
}
+
+ cv.Warnf(
+ ifelseStr(cv.Op == opUseMatch,
+ "The filename pattern %q contains the invalid character%s %q.",
+ "The filename %q contains the invalid character%s %q."),
+ cv.Value,
+ ifelseStr(len(invalid) > 1, "s", ""),
+ invalid)
}
func (cv *VartypeCheck) FileMask() {
// TODO: Decide whether to call this a "mask" or a "pattern", and use only that word everywhere.
- switch {
- case cv.Op == opUseMatch:
- break
- case contains(cv.ValueNoVar, "/"):
- cv.Warnf("A filename mask should not contain a slash.")
- case !matches(cv.ValueNoVar, `^[#%*+\-./0-9?@A-Z\[\]_a-z~]*$`):
- cv.Warnf("%q is not a valid filename mask.", cv.Value)
+ invalid := replaceAll(cv.ValueNoVar, `[%*+,\-.0-9?@A-Z\[\]_a-z~]`, "")
+ if invalid == "" {
+ return
}
+
+ cv.Warnf("The filename pattern %q contains the invalid character%s %q.",
+ cv.Value,
+ ifelseStr(len(invalid) > 1, "s", ""),
+ invalid)
}
func (cv *VartypeCheck) FileMode() {
@@ -819,13 +848,15 @@ func (cv *VartypeCheck) Pathlist() {
//
// See FileMask.
func (cv *VartypeCheck) PathMask() {
- if cv.Op == opUseMatch {
+ invalid := replaceAll(cv.ValueNoVar, `[%*+,\-./0-9?@A-Z\[\]_a-z~]`, "")
+ if invalid == "" {
return
}
- if !matches(cv.ValueNoVar, `^[#%*+\-./0-9?@A-Z\[\]_a-z~]*$`) {
- cv.Warnf("%q is not a valid pathname mask.", cv.Value)
- }
+ cv.Warnf("The pathname pattern %q contains the invalid character%s %q.",
+ cv.Value,
+ ifelseStr(len(invalid) > 1, "s", ""),
+ invalid)
}
// Pathname checks for pathnames.
@@ -834,13 +865,22 @@ func (cv *VartypeCheck) PathMask() {
//
// See http://www.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap03.html#tag_03_266
func (cv *VartypeCheck) Pathname() {
- if cv.Op == opUseMatch {
+ valid := regex.Pattern(ifelseStr(
+ cv.Op == opUseMatch,
+ `[%*+,\-./0-9?@A-Z\[\]_a-z~]`,
+ `[%+,\-./0-9@A-Z_a-z~]`))
+ invalid := replaceAll(cv.ValueNoVar, valid, "")
+ if invalid == "" {
return
}
- if !matches(cv.ValueNoVar, `^[#\-0-9A-Za-z._~+%/]*$`) {
- cv.Warnf("%q is not a valid pathname.", cv.Value)
- }
+ cv.Warnf(
+ ifelseStr(cv.Op == opUseMatch,
+ "The pathname pattern %q contains the invalid character%s %q.",
+ "The pathname %q contains the invalid character%s %q."),
+ cv.Value,
+ ifelseStr(len(invalid) > 1, "s", ""),
+ invalid)
}
func (cv *VartypeCheck) Perl5Packlist() {
diff --git a/pkgtools/pkglint/files/vartypecheck_test.go b/pkgtools/pkglint/files/vartypecheck_test.go
index 071f7bd2505..47e7e94a6f3 100644
--- a/pkgtools/pkglint/files/vartypecheck_test.go
+++ b/pkgtools/pkglint/files/vartypecheck_test.go
@@ -452,17 +452,27 @@ func (s *Suite) Test_VartypeCheck_Enum__use_match(c *check.C) {
func (s *Suite) Test_VartypeCheck_FetchURL(c *check.C) {
t := s.Init(c)
+
+ t.SetUpPackage("category/own-master-site",
+ "MASTER_SITE_OWN=\thttps://example.org/")
+ t.FinishSetUp()
+
vt := NewVartypeCheckTester(t, (*VartypeCheck).FetchURL)
t.SetUpMasterSite("MASTER_SITE_GNU", "http://ftp.gnu.org/pub/gnu/")
t.SetUpMasterSite("MASTER_SITE_GITHUB", "https://github.com/")
+ G.Pkg = NewPackage(t.File("category/own-master-site"))
+ G.Pkg.load()
+
vt.Varname("MASTER_SITES")
vt.Values(
"https://github.com/example/project/",
"http://ftp.gnu.org/pub/gnu/bison", // Missing a slash at the end
"${MASTER_SITE_GNU:=bison}",
- "${MASTER_SITE_INVALID:=subdir/}")
+ "${MASTER_SITE_INVALID:=subdir/}",
+ "${MASTER_SITE_OWN}",
+ "${MASTER_SITE_OWN:=subdir/}")
vt.Output(
"WARN: filename.mk:1: Please use ${MASTER_SITE_GITHUB:=example/} "+
@@ -486,6 +496,14 @@ func (s *Suite) Test_VartypeCheck_FetchURL(c *check.C) {
vt.Output(
"WARN: filename.mk:23: \"http://example.org/download?filename=<distfile>;version=<version>\" is not a valid URL.")
+
+ vt.Values(
+ "${MASTER_SITE_GITHUB:S,^,-,:=project/archive/${DISTFILE}}")
+
+ // No warning since there is more than a single := modifier.
+ // In this case, because of the hyphen that is added at the beginning,
+ // the URL is not required to end with a slash anymore.
+ vt.OutputEmpty()
}
func (s *Suite) Test_VartypeCheck_Filename(c *check.C) {
@@ -497,8 +515,8 @@ func (s *Suite) Test_VartypeCheck_Filename(c *check.C) {
"OS/2-manual.txt")
vt.Output(
- "WARN: filename.mk:1: \"Filename with spaces.docx\" is not a valid filename.",
- "WARN: filename.mk:2: A filename should not contain a slash.")
+ "WARN: filename.mk:1: The filename \"Filename with spaces.docx\" contains the invalid characters \" \".",
+ "WARN: filename.mk:2: The filename \"OS/2-manual.txt\" contains the invalid character \"/\".")
vt.Op(opUseMatch)
vt.Values(
@@ -506,7 +524,9 @@ func (s *Suite) Test_VartypeCheck_Filename(c *check.C) {
// There's no guarantee that a filename only contains [A-Za-z0-9.].
// Therefore there are no useful checks in this situation.
- vt.OutputEmpty()
+ vt.Output(
+ "WARN: filename.mk:11: The filename pattern \"Filename with spaces.docx\" " +
+ "contains the invalid characters \" \".")
}
func (s *Suite) Test_VartypeCheck_FileMask(c *check.C) {
@@ -518,16 +538,21 @@ func (s *Suite) Test_VartypeCheck_FileMask(c *check.C) {
"OS/2-manual.txt")
vt.Output(
- "WARN: filename.mk:1: \"FileMask with spaces.docx\" is not a valid filename mask.",
- "WARN: filename.mk:2: A filename mask should not contain a slash.")
+ "WARN: filename.mk:1: The filename pattern \"FileMask with spaces.docx\" "+
+ "contains the invalid characters \" \".",
+ "WARN: filename.mk:2: The filename pattern \"OS/2-manual.txt\" "+
+ "contains the invalid character \"/\".")
vt.Op(opUseMatch)
vt.Values(
"FileMask with spaces.docx")
// There's no guarantee that a filename only contains [A-Za-z0-9.].
- // Therefore there are no useful checks in this situation.
- vt.OutputEmpty()
+ // Therefore it might be necessary to allow all characters here.
+ // TODO: Investigate whether this restriction is useful in practice.
+ vt.Output(
+ "WARN: filename.mk:11: The filename pattern \"FileMask with spaces.docx\" " +
+ "contains the invalid characters \" \".")
}
func (s *Suite) Test_VartypeCheck_FileMode(c *check.C) {
@@ -894,7 +919,7 @@ func (s *Suite) Test_VartypeCheck_PathMask(c *check.C) {
"src/*/*")
vt.Output(
- "WARN: filename.mk:2: \"src/*&*\" is not a valid pathname mask.")
+ "WARN: filename.mk:2: The pathname pattern \"src/*&*\" contains the invalid character \"&\".")
vt.Op(opUseMatch)
vt.Values("any")
@@ -916,7 +941,7 @@ func (s *Suite) Test_VartypeCheck_Pathname(c *check.C) {
"anything")
vt.Output(
- "WARN: filename.mk:1: \"${PREFIX}/*\" is not a valid pathname.")
+ "WARN: filename.mk:1: The pathname \"${PREFIX}/*\" contains the invalid character \"*\".")
}
func (s *Suite) Test_VartypeCheck_Perl5Packlist(c *check.C) {
@@ -1230,8 +1255,13 @@ func (s *Suite) Test_VartypeCheck_URL(c *check.C) {
"https://www.netbsd.org/",
"https://www.example.org",
"ftp://example.org/pub/",
- "gopher://example.org/",
+ "gopher://example.org/")
+ vt.Output(
+ "WARN: filename.mk:4: Please write NetBSD.org instead of www.netbsd.org.",
+ "NOTE: filename.mk:5: For consistency, please add a trailing slash to \"https://www.example.org\".")
+
+ vt.Values(
"",
"ftp://example.org/<",
"gopher://example.org/<",
@@ -1243,17 +1273,15 @@ func (s *Suite) Test_VartypeCheck_URL(c *check.C) {
"string with spaces")
vt.Output(
- "WARN: filename.mk:4: Please write NetBSD.org instead of www.netbsd.org.",
- "NOTE: filename.mk:5: For consistency, please add a trailing slash to \"https://www.example.org\".",
- "WARN: filename.mk:8: \"\" is not a valid URL.",
- "WARN: filename.mk:9: \"ftp://example.org/<\" is not a valid URL.",
- "WARN: filename.mk:10: \"gopher://example.org/<\" is not a valid URL.",
- "WARN: filename.mk:11: \"http://example.org/<\" is not a valid URL.",
- "WARN: filename.mk:12: \"https://example.org/<\" is not a valid URL.",
- "WARN: filename.mk:13: \"https://www.example.org/path with spaces\" is not a valid URL.",
- "WARN: filename.mk:14: \"httpxs://www.example.org\" is not a valid URL. Only ftp, gopher, http, and https URLs are allowed here.",
- "WARN: filename.mk:15: \"mailto:someone@example.org\" is not a valid URL.",
- "WARN: filename.mk:16: \"string with spaces\" is not a valid URL.")
+ "WARN: filename.mk:11: \"\" is not a valid URL.",
+ "WARN: filename.mk:12: \"ftp://example.org/<\" is not a valid URL.",
+ "WARN: filename.mk:13: \"gopher://example.org/<\" is not a valid URL.",
+ "WARN: filename.mk:14: \"http://example.org/<\" is not a valid URL.",
+ "WARN: filename.mk:15: \"https://example.org/<\" is not a valid URL.",
+ "WARN: filename.mk:16: \"https://www.example.org/path with spaces\" is not a valid URL.",
+ "WARN: filename.mk:17: \"httpxs://www.example.org\" is not a valid URL. Only ftp, gopher, http, and https URLs are allowed here.",
+ "WARN: filename.mk:18: \"mailto:someone@example.org\" is not a valid URL.",
+ "WARN: filename.mk:19: \"string with spaces\" is not a valid URL.")
// Yes, even in 2019, some pkgsrc-wip packages really use a gopher HOMEPAGE.
vt.Values(
generated by cgit v1.2.3 (git 2.39.1) at 2024-12-20 07:41:00 +0000