diff options
Diffstat (limited to 'pkgtools/pkglint')
25 files changed, 287 insertions, 112 deletions
diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile index 4066ebcaeda..05506a7adbf 100644 --- a/pkgtools/pkglint/Makefile +++ b/pkgtools/pkglint/Makefile @@ -1,7 +1,6 @@ -# $NetBSD: Makefile,v 1.517 2017/07/27 11:21:25 wiz Exp $ +# $NetBSD: Makefile,v 1.518 2017/10/08 22:31:13 rillig Exp $ -PKGNAME= pkglint-5.4.20 -PKGREVISION= 3 +PKGNAME= pkglint-5.4.21 DISTFILES= # none CATEGORIES= pkgtools diff --git a/pkgtools/pkglint/files/distinfo.go b/pkgtools/pkglint/files/distinfo.go index e962acac271..49c7af9fa0e 100644 --- a/pkgtools/pkglint/files/distinfo.go +++ b/pkgtools/pkglint/files/distinfo.go @@ -6,6 +6,7 @@ import ( "fmt" "io/ioutil" "netbsd.org/pkglint/line" + "netbsd.org/pkglint/linechecks" "netbsd.org/pkglint/trace" "strings" ) @@ -54,7 +55,7 @@ type distinfoLinesChecker struct { } func (ck *distinfoLinesChecker) checkLines(lines []line.Line) { - LineChecker{lines[0]}.CheckRcsid(``, "") + linechecks.CheckRcsid(lines[0], ``, "") if 1 < len(lines) && lines[1].Text() != "" { lines[1].Notef("Empty line expected.") } diff --git a/pkgtools/pkglint/files/getopt/getopt.go b/pkgtools/pkglint/files/getopt/getopt.go index b1deb30bd29..eda37a3cf13 100644 --- a/pkgtools/pkglint/files/getopt/getopt.go +++ b/pkgtools/pkglint/files/getopt/getopt.go @@ -1,7 +1,7 @@ +// Package getopt provides a parser for command line options, +// supporting multi-value options such as -Wall,no-extra. package getopt -// Self-written getopt to support multi-argument options. - import ( "fmt" "io" diff --git a/pkgtools/pkglint/files/globaldata.go b/pkgtools/pkglint/files/globaldata.go index 71494a81e2b..bddc1b575f0 100644 --- a/pkgtools/pkglint/files/globaldata.go +++ b/pkgtools/pkglint/files/globaldata.go @@ -67,7 +67,7 @@ func (gd *GlobalData) Initialize() { gd.loadDeprecatedVars() } -func (gd *GlobalData) Latest(category string, re regex.RegexPattern, repl string) string { +func (gd *GlobalData) Latest(category string, re regex.Pattern, repl string) string { key := category + "/" + string(re) + " => " + repl if latest, found := gd.latest[key]; found { return latest diff --git a/pkgtools/pkglint/files/line.go b/pkgtools/pkglint/files/line.go index 84c1de9ae4d..23eb9f59c19 100644 --- a/pkgtools/pkglint/files/line.go +++ b/pkgtools/pkglint/files/line.go @@ -17,6 +17,7 @@ import ( "fmt" "io" "netbsd.org/pkglint/line" + "netbsd.org/pkglint/linechecks" "netbsd.org/pkglint/regex" "path" "strconv" @@ -205,7 +206,7 @@ func (line *LineImpl) AutofixReplace(from, to string) bool { return false } -func (line *LineImpl) AutofixReplaceRegexp(from regex.RegexPattern, to string) bool { +func (line *LineImpl) AutofixReplaceRegexp(from regex.Pattern, to string) bool { for _, rawLine := range line.raw { if rawLine.Lineno != 0 { if replaced := regex.Compile(from).ReplaceAllString(rawLine.textnl, to); replaced != rawLine.textnl { @@ -242,4 +243,5 @@ func (line *LineImpl) AutofixMark(reason string) { func init() { line.NewLineEOF = NewLineEOF + linechecks.Explain = Explain } diff --git a/pkgtools/pkglint/files/line/line.go b/pkgtools/pkglint/files/line/line.go index 4b006942571..5d9db0e7ac8 100644 --- a/pkgtools/pkglint/files/line/line.go +++ b/pkgtools/pkglint/files/line/line.go @@ -21,7 +21,7 @@ type Line interface { ReferenceFrom(Line) string AutofixReplace(from, to string) bool - AutofixReplaceRegexp(from regex.RegexPattern, to string) bool + AutofixReplaceRegexp(from regex.Pattern, to string) bool AutofixInsertBefore(text string) bool AutofixDelete() bool AutofixMark(reason string) diff --git a/pkgtools/pkglint/files/linechecks/linechecker.go b/pkgtools/pkglint/files/linechecks/linechecker.go new file mode 100755 index 00000000000..eb5bb9c40bc --- /dev/null +++ b/pkgtools/pkglint/files/linechecks/linechecker.go @@ -0,0 +1,109 @@ +package linechecks + +import ( + "fmt" + "netbsd.org/pkglint/line" + "netbsd.org/pkglint/regex" + "netbsd.org/pkglint/trace" + "strings" +) + +var Explain func(...string) + +func CheckAbsolutePathname(line line.Line, text string) { + if trace.Tracing { + defer trace.Call1(text)() + } + + // In the GNU coding standards, DESTDIR is defined as a (usually + // empty) prefix that can be used to install files to a different + // location from what they have been built for. Therefore + // everything following it is considered an absolute pathname. + // + // Another context where absolute pathnames usually appear is in + // assignments like "bindir=/bin". + if m, path := regex.Match1(text, `(?:^|\$[{(]DESTDIR[)}]|[\w_]+\s*=\s*)(/(?:[^"'\s]|"[^"*]"|'[^']*')*)`); m { + if regex.Matches(path, `^/\w`) { + CheckwordAbsolutePathname(line, path) + } + } +} + +func CheckLength(line line.Line, maxlength int) { + if len(line.Text()) > maxlength { + line.Warnf("Line too long (should be no more than %d characters).", maxlength) + Explain( + "Back in the old time, terminals with 80x25 characters were common.", + "And this is still the default size of many terminal emulators.", + "Moderately short lines also make reading easier.") + } +} + +func CheckValidCharacters(line line.Line, reChar regex.Pattern) { + rest := regex.Compile(reChar).ReplaceAllString(line.Text(), "") + if rest != "" { + uni := "" + for _, c := range rest { + uni += fmt.Sprintf(" %U", c) + } + line.Warnf("Line contains invalid characters (%s).", uni[1:]) + } +} + +func CheckTrailingWhitespace(line line.Line) { + if strings.HasSuffix(line.Text(), " ") || strings.HasSuffix(line.Text(), "\t") { + if !line.AutofixReplaceRegexp(`\s+\n$`, "\n") { + line.Notef("Trailing white-space.") + Explain( + "When a line ends with some white-space, that space is in most cases", + "irrelevant and can be removed.") + } + } +} + +func CheckRcsid(line line.Line, prefixRe regex.Pattern, suggestedPrefix string) bool { + if trace.Tracing { + defer trace.Call(prefixRe, suggestedPrefix)() + } + + if regex.Matches(line.Text(), `^`+prefixRe+`\$`+`NetBSD(?::[^\$]+)?\$$`) { + return true + } + + if !line.AutofixInsertBefore(suggestedPrefix + "$" + "NetBSD$") { + line.Errorf("Expected %q.", suggestedPrefix+"$"+"NetBSD$") + Explain( + "Several files in pkgsrc must contain the CVS Id, so that their", + "current version can be traced back later from a binary package.", + "This is to ensure reproducible builds, for example for finding bugs.") + } + return false +} + +func CheckwordAbsolutePathname(line line.Line, word string) { + if trace.Tracing { + defer trace.Call1(word)() + } + + switch { + case regex.Matches(word, `^/dev/(?:null|tty|zero)$`): + // These are defined by POSIX. + case word == "/bin/sh": + // This is usually correct, although on Solaris, it's pretty feature-crippled. + case regex.Matches(word, `^/s\W`): + // Probably a sed(1) command + case regex.Matches(word, `^/(?:[a-z]|\$[({])`): + // Absolute paths probably start with a lowercase letter. + line.Warnf("Found absolute pathname: %s", word) + Explain( + "Absolute pathnames are often an indicator for unportable code. As", + "pkgsrc aims to be a portable system, absolute pathnames should be", + "avoided whenever possible.", + "", + "A special variable in this context is ${DESTDIR}, which is used in", + "GNU projects to specify a different directory for installation than", + "what the programs see later when they are executed. Usually it is", + "empty, so if anything after that variable starts with a slash, it is", + "considered an absolute pathname.") + } +} diff --git a/pkgtools/pkglint/files/linechecks/linechecker_test.go b/pkgtools/pkglint/files/linechecks/linechecker_test.go new file mode 100755 index 00000000000..3324c057c62 --- /dev/null +++ b/pkgtools/pkglint/files/linechecks/linechecker_test.go @@ -0,0 +1,76 @@ +package linechecks + +// Note: These tests are currently not run, since the dependencies +// between the Go packages are not yet resolved properly. + +import ( + "gopkg.in/check.v1" + "netbsd.org/pkglint/line" +) + +type Suite struct { + c *check.C +} + +func (s *Suite) SetUpTest(c *check.C) { + Explain = func(...string) {} +} + +func (s *Suite) TearDownTest(c *check.C) { + Explain = nil +} + +func (s *Suite) Init(c *check.C) { + s.c = c +} + +func (s *Suite) CheckOutputLines(lines ...string) { + panic("Not yet implemented") +} + +func (s *Suite) NewLines(...string) []line.Line { + panic("Not yet implemented") +} + +func NewLine(string, int, string, []string) line.Line { + panic("Not yet implemented") +} + +func (s *Suite) Test_LineChecker_CheckAbsolutePathname(c *check.C) { + line := NewLine("Makefile", 1, "# dummy", nil) + + CheckAbsolutePathname(line, "bindir=/bin") + CheckAbsolutePathname(line, "bindir=/../lib") + + s.CheckOutputLines( + "WARN: Makefile:1: Found absolute pathname: /bin") +} + +func (s *Suite) Test_LineChecker_CheckTrailingWhitespace(c *check.C) { + s.Init(c) + line := NewLine("Makefile", 32, "The line must go on ", nil) + + CheckTrailingWhitespace(line) + + s.CheckOutputLines( + "NOTE: Makefile:32: Trailing white-space.") +} + +func (s *Suite) Test_LineChecker_CheckRcsid(c *check.C) { + s.Init(c) + lines := s.NewLines("fname", + "$"+"NetBSD: dummy $", + "$"+"NetBSD$", + "$"+"Id: dummy $", + "$"+"Id$", + "$"+"FreeBSD$") + + for _, line := range lines { + CheckRcsid(line, ``, "") + } + + s.CheckOutputLines( + "ERROR: fname:3: Expected \"$"+"NetBSD$\".", + "ERROR: fname:4: Expected \"$"+"NetBSD$\".", + "ERROR: fname:5: Expected \"$"+"NetBSD$\".") +} diff --git a/pkgtools/pkglint/files/mkline_test.go b/pkgtools/pkglint/files/mkline_test.go index 7473f960552..8c25a6069be 100644 --- a/pkgtools/pkglint/files/mkline_test.go +++ b/pkgtools/pkglint/files/mkline_test.go @@ -297,7 +297,6 @@ func (s *Suite) Test_MkLines_Check__extra(c *check.C) { s.CheckOutputLines( "WARN: options.mk:3: The values for PYTHON_VERSIONS_ACCEPTED should be in decreasing order.", - "NOTE: options.mk:4: Please .include \"../../meta-pkgs/kde3/kde3.mk\" instead of this line.", "NOTE: options.mk:5: Please use \"# empty\", \"# none\" or \"yes\" instead of \"# defined\".", "WARN: options.mk:7: Please include \"../../mk/bsd.prefs.mk\" before using \"?=\".", "WARN: options.mk:10: Building the package should take place entirely inside ${WRKSRC}, not \"${WRKSRC}/..\".", diff --git a/pkgtools/pkglint/files/mklinechecker.go b/pkgtools/pkglint/files/mklinechecker.go index 61a641da7b6..9f4564e5484 100644 --- a/pkgtools/pkglint/files/mklinechecker.go +++ b/pkgtools/pkglint/files/mklinechecker.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "netbsd.org/pkglint/linechecks" "netbsd.org/pkglint/regex" "netbsd.org/pkglint/trace" "os" @@ -17,8 +18,8 @@ type MkLineChecker struct { func (ck MkLineChecker) Check() { mkline := ck.MkLine - LineChecker{mkline}.CheckTrailingWhitespace() - LineChecker{mkline}.CheckValidCharacters(`[\t -~]`) + linechecks.CheckTrailingWhitespace(mkline) + linechecks.CheckValidCharacters(mkline, `[\t -~]`) switch { case mkline.IsVarassign(): @@ -787,14 +788,6 @@ func (ck MkLineChecker) checkVarassignSpecific() { } } - if varname == "CONFIGURE_ARGS" && contains(value, "=${PREFIX}/share/kde") { - mkline.Notef("Please .include \"../../meta-pkgs/kde3/kde3.mk\" instead of this line.") - Explain( - "That file does many things automatically and consistently that this", - "package also does. When using kde3.mk, you can probably also leave", - "out some explicit dependencies.") - } - if varname == "PYTHON_VERSIONS_ACCEPTED" { ck.checkVarassignPythonVersions(varname, value) } @@ -1050,7 +1043,7 @@ func (ck MkLineChecker) checkCompareVarStr(varname, op, value string) { } } -func (ck MkLineChecker) CheckValidCharactersInValue(reValid regex.RegexPattern) { +func (ck MkLineChecker) CheckValidCharactersInValue(reValid regex.Pattern) { mkline := ck.MkLine rest := regex.Compile(reValid).ReplaceAllString(mkline.Value(), "") if rest != "" { diff --git a/pkgtools/pkglint/files/mklines.go b/pkgtools/pkglint/files/mklines.go index dd9f1487f6b..84bfca6988d 100644 --- a/pkgtools/pkglint/files/mklines.go +++ b/pkgtools/pkglint/files/mklines.go @@ -2,6 +2,7 @@ package main import ( "netbsd.org/pkglint/line" + "netbsd.org/pkglint/linechecks" "netbsd.org/pkglint/trace" "path" "strings" @@ -101,7 +102,7 @@ func (mklines *MkLines) Check() { // In the second pass, the actual checks are done. - LineChecker{mklines.lines[0]}.CheckRcsid(`#\s+`, "# ") + linechecks.CheckRcsid(mklines.lines[0], `#\s+`, "# ") var substcontext SubstContext var varalign VaralignBlock diff --git a/pkgtools/pkglint/files/mkparser.go b/pkgtools/pkglint/files/mkparser.go index 8a5e9a12088..9269a5e8789 100644 --- a/pkgtools/pkglint/files/mkparser.go +++ b/pkgtools/pkglint/files/mkparser.go @@ -74,7 +74,7 @@ func (p *MkParser) VarUse() *MkVarUse { } } - for p.VarUse() != nil || repl.AdvanceRegexp(regex.RegexPattern(`^([^$:`+closing+`]|\$\$)+`)) { + for p.VarUse() != nil || repl.AdvanceRegexp(regex.Pattern(`^([^$:`+closing+`]|\$\$)+`)) { } rest := p.Rest() if hasPrefix(rest, ":L") || hasPrefix(rest, ":?") { @@ -133,7 +133,7 @@ func (p *MkParser) VarUseModifiers(varname, closing string) []string { case '=', 'D', 'M', 'N', 'U': if repl.AdvanceRegexp(`^[=DMNU]`) { - for p.VarUse() != nil || repl.AdvanceRegexp(regex.RegexPattern(`^([^$:`+closing+`]|\$\$)+`)) { + for p.VarUse() != nil || repl.AdvanceRegexp(regex.Pattern(`^([^$:`+closing+`]|\$\$)+`)) { } modifiers = append(modifiers, repl.Since(modifierMark)) continue @@ -143,7 +143,7 @@ func (p *MkParser) VarUseModifiers(varname, closing string) []string { if repl.AdvanceRegexp(`^[CS]([%,/:;@^|])`) { separator := repl.Group(1) repl.AdvanceStr("^") - re := regex.RegexPattern(`^([^\` + separator + `$` + closing + `\\]|\$\$|\\.)+`) + re := regex.Pattern(`^([^\` + separator + `$` + closing + `\\]|\$\$|\\.)+`) for p.VarUse() != nil || repl.AdvanceRegexp(re) { } repl.AdvanceStr("$") @@ -162,7 +162,7 @@ func (p *MkParser) VarUseModifiers(varname, closing string) []string { case '@': if repl.AdvanceRegexp(`^@([\w.]+)@`) { loopvar := repl.Group(1) - for p.VarUse() != nil || repl.AdvanceRegexp(regex.RegexPattern(`^([^$:@`+closing+`\\]|\$\$|\\.)+`)) { + for p.VarUse() != nil || repl.AdvanceRegexp(regex.Pattern(`^([^$:@`+closing+`\\]|\$\$|\\.)+`)) { } if !repl.AdvanceStr("@") && p.EmitWarnings { p.Line.Warnf("Modifier ${%s:@%s@...@} is missing the final \"@\".", varname, loopvar) @@ -179,7 +179,7 @@ func (p *MkParser) VarUseModifiers(varname, closing string) []string { case '?': repl.AdvanceStr("?") - re := regex.RegexPattern(`^([^$:` + closing + `]|\$\$)+`) + re := regex.Pattern(`^([^$:` + closing + `]|\$\$)+`) for p.VarUse() != nil || repl.AdvanceRegexp(re) { } if repl.AdvanceStr(":") { @@ -191,7 +191,7 @@ func (p *MkParser) VarUseModifiers(varname, closing string) []string { } repl.Reset(modifierMark) - for p.VarUse() != nil || repl.AdvanceRegexp(regex.RegexPattern(`^([^:$`+closing+`]|\$\$)+`)) { + for p.VarUse() != nil || repl.AdvanceRegexp(regex.Pattern(`^([^:$`+closing+`]|\$\$)+`)) { } if suffixSubst := repl.Since(modifierMark); contains(suffixSubst, "=") { modifiers = append(modifiers, suffixSubst) diff --git a/pkgtools/pkglint/files/mkshtypes.go b/pkgtools/pkglint/files/mkshtypes.go index 203837e0d66..38d77dd54f4 100644 --- a/pkgtools/pkglint/files/mkshtypes.go +++ b/pkgtools/pkglint/files/mkshtypes.go @@ -26,7 +26,7 @@ func (list *MkShList) AddSeparator(separator MkShSeparator) *MkShList { type MkShAndOr struct { Pipes []*MkShPipeline - Ops []string // Either "&&" or "||" + Ops []string // Each element is either "&&" or "||" } func NewMkShAndOr(pipeline *MkShPipeline) *MkShAndOr { @@ -147,7 +147,7 @@ func (c *StrCommand) HasOption(opt string) bool { return false } -func (c *StrCommand) AnyArgMatches(pattern regex.RegexPattern) bool { +func (c *StrCommand) AnyArgMatches(pattern regex.Pattern) bool { for _, arg := range c.Args { if matches(arg, pattern) { return true diff --git a/pkgtools/pkglint/files/package.go b/pkgtools/pkglint/files/package.go index a55d1d4a5cf..b4b7dce1837 100644 --- a/pkgtools/pkglint/files/package.go +++ b/pkgtools/pkglint/files/package.go @@ -503,7 +503,7 @@ func (pkg *Package) pkgnameFromDistname(pkgname, distname string) string { defer trace.Call(str, smod, trace.Ref(result))() } qsep := regexp.QuoteMeta(smod[1:2]) - if m, left, from, right, to, flags := regex.Match5(smod, regex.RegexPattern(`^S`+qsep+`(\^?)([^:]*?)(\$?)`+qsep+`([^:]*)`+qsep+`([1g]*)$`)); m { + if m, left, from, right, to, flags := regex.Match5(smod, regex.Pattern(`^S`+qsep+`(\^?)([^:]*?)(\$?)`+qsep+`([^:]*)`+qsep+`([1g]*)$`)); m { result := mkopSubst(str, left != "", from, right != "", to, flags) if trace.Tracing { trace.Stepf("subst %q %q => %q", str, smod, result) diff --git a/pkgtools/pkglint/files/patches.go b/pkgtools/pkglint/files/patches.go index 9c8dfbb254b..86ce2a23377 100644 --- a/pkgtools/pkglint/files/patches.go +++ b/pkgtools/pkglint/files/patches.go @@ -4,6 +4,7 @@ package main import ( "netbsd.org/pkglint/line" + "netbsd.org/pkglint/linechecks" "netbsd.org/pkglint/textproc" "netbsd.org/pkglint/trace" "path" @@ -36,7 +37,7 @@ func (ck *PatchChecker) Check() { defer trace.Call0()() } - if (LineChecker{ck.lines[0]}).CheckRcsid(``, "") { + if linechecks.CheckRcsid(ck.lines[0], ``, "") { ck.exp.Advance() } ck.previousLineEmpty = ck.exp.ExpectEmptyLine(G.opts.WarnSpace) @@ -318,34 +319,6 @@ func guessFileType(line line.Line, fname string) (fileType FileType) { return ftUnknown } -func checkwordAbsolutePathname(line line.Line, word string) { - if trace.Tracing { - defer trace.Call1(word)() - } - - switch { - case matches(word, `^/dev/(?:null|tty|zero)$`): - // These are defined by POSIX. - case word == "/bin/sh": - // This is usually correct, although on Solaris, it's pretty feature-crippled. - case matches(word, `^/s\W`): - // Probably a sed(1) command - case matches(word, `^/(?:[a-z]|\$[({])`): - // Absolute paths probably start with a lowercase letter. - line.Warnf("Found absolute pathname: %s", word) - Explain( - "Absolute pathnames are often an indicator for unportable code. As", - "pkgsrc aims to be a portable system, absolute pathnames should be", - "avoided whenever possible.", - "", - "A special variable in this context is ${DESTDIR}, which is used in", - "GNU projects to specify a different directory for installation than", - "what the programs see later when they are executed. Usually it is", - "empty, so if anything after that variable starts with a slash, it is", - "considered an absolute pathname.") - } -} - // Looks for strings like "/dev/cd0" appearing in source code func checklineSourceAbsolutePathname(line line.Line, text string) { if !strings.ContainsAny(text, "\"'") { @@ -364,7 +337,7 @@ func checklineSourceAbsolutePathname(line line.Line, text string) { // ok; Python example: libdir = prefix + '/lib' default: - checkwordAbsolutePathname(line, str) + linechecks.CheckwordAbsolutePathname(line, str) } } } @@ -389,7 +362,7 @@ func checklineOtherAbsolutePathname(line line.Line, text string) { if trace.Tracing { trace.Step1("before=%q", before) } - checkwordAbsolutePathname(line, path) + linechecks.CheckwordAbsolutePathname(line, path) } } } diff --git a/pkgtools/pkglint/files/pkglint.go b/pkgtools/pkglint/files/pkglint.go index d9329d46f43..5d4e88263ed 100644 --- a/pkgtools/pkglint/files/pkglint.go +++ b/pkgtools/pkglint/files/pkglint.go @@ -6,6 +6,7 @@ import ( "netbsd.org/pkglint/getopt" "netbsd.org/pkglint/histogram" "netbsd.org/pkglint/line" + "netbsd.org/pkglint/linechecks" "netbsd.org/pkglint/regex" "netbsd.org/pkglint/trace" "os" @@ -279,9 +280,9 @@ func ChecklinesDescr(lines []line.Line) { } for _, line := range lines { - LineChecker{line}.CheckLength(80) - LineChecker{line}.CheckTrailingWhitespace() - LineChecker{line}.CheckValidCharacters(`[\t -~]`) + linechecks.CheckLength(line, 80) + linechecks.CheckTrailingWhitespace(line) + linechecks.CheckValidCharacters(line, `[\t -~]`) if contains(line.Text(), "${") { line.Notef("Variables are not expanded in the DESCR file.") } @@ -326,11 +327,11 @@ func ChecklinesMessage(lines []line.Line) { line.Warnf("Expected a line of exactly 75 \"=\" characters.") explainMessage() } - LineChecker{lines[1]}.CheckRcsid(``, "") + linechecks.CheckRcsid(lines[1], ``, "") for _, line := range lines { - LineChecker{line}.CheckLength(80) - LineChecker{line}.CheckTrailingWhitespace() - LineChecker{line}.CheckValidCharacters(`[\t -~]`) + linechecks.CheckLength(line, 80) + linechecks.CheckTrailingWhitespace(line) + linechecks.CheckValidCharacters(line, `[\t -~]`) } if lastLine := lines[len(lines)-1]; lastLine.Text() != hline { lastLine.Warnf("Expected a line of exactly 75 \"=\" characters.") diff --git a/pkgtools/pkglint/files/plist.go b/pkgtools/pkglint/files/plist.go index 2104dae910f..33590285026 100644 --- a/pkgtools/pkglint/files/plist.go +++ b/pkgtools/pkglint/files/plist.go @@ -2,6 +2,7 @@ package main import ( "netbsd.org/pkglint/line" + "netbsd.org/pkglint/linechecks" "netbsd.org/pkglint/regex" "netbsd.org/pkglint/trace" "path" @@ -14,7 +15,7 @@ func ChecklinesPlist(lines []line.Line) { defer trace.Call1(lines[0].Filename())() } - LineChecker{lines[0]}.CheckRcsid(`@comment `, "@comment ") + linechecks.CheckRcsid(lines[0], `@comment `, "@comment ") if len(lines) == 1 { lines[0].Warnf("PLIST files shouldn't be empty.") @@ -33,14 +34,16 @@ func ChecklinesPlist(lines []line.Line) { ck := &PlistChecker{ make(map[string]*PlistLine), make(map[string]*PlistLine), - ""} + "", + false} ck.Check(lines) } type PlistChecker struct { - allFiles map[string]*PlistLine - allDirs map[string]*PlistLine - lastFname string + allFiles map[string]*PlistLine + allDirs map[string]*PlistLine + lastFname string + warnedAboutIconThemes bool } type PlistLine struct { @@ -355,19 +358,27 @@ func (ck *PlistChecker) checkpathShare(pline *PlistLine) { "warning is harmless.") } - case hasPrefix(text, "share/icons/hicolor/") && G.Pkg != nil && G.Pkg.Pkgpath != "graphics/hicolor-icon-theme": - f := "../../graphics/hicolor-icon-theme/buildlink3.mk" - if G.Pkg.included[f] == nil { - line.Errorf("Packages that install hicolor icons must include %q in the Makefile.", f) + case hasPrefix(text, "share/icons/") && G.Pkg != nil: + if hasPrefix(text, "share/icons/hicolor/") && G.Pkg.Pkgpath != "graphics/hicolor-icon-theme" { + f := "../../graphics/hicolor-icon-theme/buildlink3.mk" + if G.Pkg.included[f] == nil { + line.Errorf("Packages that install hicolor icons must include %q in the Makefile.", f) + } } - case hasPrefix(text, "share/icons/gnome") && G.Pkg != nil && G.Pkg.Pkgpath != "graphics/gnome-icon-theme": - f := "../../graphics/gnome-icon-theme/buildlink3.mk" - if G.Pkg.included[f] == nil { - line.Errorf("The package Makefile must include %q.", f) - Explain( - "Packages that install GNOME icons must maintain the icon theme", - "cache.") + if hasPrefix(text, "share/icons/gnome") && G.Pkg.Pkgpath != "graphics/gnome-icon-theme" { + f := "../../graphics/gnome-icon-theme/buildlink3.mk" + if G.Pkg.included[f] == nil { + line.Errorf("The package Makefile must include %q.", f) + Explain( + "Packages that install GNOME icons must maintain the icon theme", + "cache.") + } + } + + if !ck.warnedAboutIconThemes && contains(text[12:], "/") && G.Pkg.vardef["ICON_THEMES"] == nil { + line.Warnf("Packages that install icon theme files should set ICON_THEMES.") + ck.warnedAboutIconThemes = true } case hasPrefix(text, "share/doc/html/"): diff --git a/pkgtools/pkglint/files/plist_test.go b/pkgtools/pkglint/files/plist_test.go index 98f18463716..088b5bf990a 100644 --- a/pkgtools/pkglint/files/plist_test.go +++ b/pkgtools/pkglint/files/plist_test.go @@ -44,6 +44,7 @@ func (s *Suite) Test_ChecklinesPlist(c *check.C) { "WARN: PLIST:12: Please remove this line. It is no longer necessary.", "WARN: PLIST:13: Manual page missing for sbin/clockctl.", "ERROR: PLIST:14: The package Makefile must include \"../../graphics/gnome-icon-theme/buildlink3.mk\".", + "WARN: PLIST:14: Packages that install icon theme files should set ICON_THEMES.", "ERROR: PLIST:16: Duplicate filename \"share/tzinfo\", already appeared in line 15.") } @@ -125,7 +126,7 @@ func (s *Suite) Test_PlistLineSorter_Sort(c *check.C) { "lib/before.la", "lib/after.la", "@exec echo \"after lib/after.la\"") - ck := &PlistChecker{nil, nil, ""} + ck := &PlistChecker{nil, nil, "", false} plines := ck.NewLines(lines) NewPlistLineSorter(plines).Sort() diff --git a/pkgtools/pkglint/files/regex/regex.go b/pkgtools/pkglint/files/regex/regex.go index c7addc854b9..a6b107f0f1b 100644 --- a/pkgtools/pkglint/files/regex/regex.go +++ b/pkgtools/pkglint/files/regex/regex.go @@ -1,3 +1,7 @@ +// Package regex provides a registry of precompiled regular expressions +// to allow reusing them without the syntactic overhead of declaring +// pattern variables everywhere in the code. +// The registry is not thread-safe, but the precompiled patterns are. package regex import ( @@ -8,22 +12,22 @@ import ( "time" ) -type RegexPattern string +type Pattern string var ( Profiling bool ) var ( - res map[RegexPattern]*regexp.Regexp + res map[Pattern]*regexp.Regexp rematch *histogram.Histogram renomatch *histogram.Histogram retime *histogram.Histogram ) -func Compile(re RegexPattern) *regexp.Regexp { +func Compile(re Pattern) *regexp.Regexp { if res == nil { - res = make(map[RegexPattern]*regexp.Regexp) + res = make(map[Pattern]*regexp.Regexp) } cre := res[re] if cre == nil { @@ -33,7 +37,7 @@ func Compile(re RegexPattern) *regexp.Regexp { return cre } -func Match(s string, re RegexPattern) []string { +func Match(s string, re Pattern) []string { if !Profiling { return Compile(re).FindStringSubmatch(s) } @@ -61,7 +65,7 @@ func Match(s string, re RegexPattern) []string { return m } -func Matches(s string, re RegexPattern) bool { +func Matches(s string, re Pattern) bool { matches := Compile(re).MatchString(s) if Profiling { if matches { @@ -73,42 +77,42 @@ func Matches(s string, re RegexPattern) bool { return matches } -func Match1(s string, re RegexPattern) (matched bool, m1 string) { +func Match1(s string, re Pattern) (matched bool, m1 string) { if m := matchn(s, re, 1); m != nil { return true, m[1] } return } -func Match2(s string, re RegexPattern) (matched bool, m1, m2 string) { +func Match2(s string, re Pattern) (matched bool, m1, m2 string) { if m := matchn(s, re, 2); m != nil { return true, m[1], m[2] } return } -func Match3(s string, re RegexPattern) (matched bool, m1, m2, m3 string) { +func Match3(s string, re Pattern) (matched bool, m1, m2, m3 string) { if m := matchn(s, re, 3); m != nil { return true, m[1], m[2], m[3] } return } -func Match4(s string, re RegexPattern) (matched bool, m1, m2, m3, m4 string) { +func Match4(s string, re Pattern) (matched bool, m1, m2, m3, m4 string) { if m := matchn(s, re, 4); m != nil { return true, m[1], m[2], m[3], m[4] } return } -func Match5(s string, re RegexPattern) (matched bool, m1, m2, m3, m4, m5 string) { +func Match5(s string, re Pattern) (matched bool, m1, m2, m3, m4, m5 string) { if m := matchn(s, re, 5); m != nil { return true, m[1], m[2], m[3], m[4], m[5] } return } -func ReplaceFirst(s string, re RegexPattern, replacement string) ([]string, string) { +func ReplaceFirst(s string, re Pattern, replacement string) ([]string, string) { if m := Compile(re).FindStringSubmatchIndex(s); m != nil { replaced := s[:m[0]] + replacement + s[m[1]:] mm := make([]string, len(m)/2) @@ -128,7 +132,7 @@ func PrintStats() { } } -func matchn(s string, re RegexPattern, n int) []string { +func matchn(s string, re Pattern, n int) []string { if m := Match(s, re); m != nil { if len(m) != 1+n { panic(fmt.Sprintf("expected match%d, got match%d for %q", len(m)-1, n, re)) diff --git a/pkgtools/pkglint/files/shell.go b/pkgtools/pkglint/files/shell.go index 0668b8bbbdf..61f6aa534ee 100644 --- a/pkgtools/pkglint/files/shell.go +++ b/pkgtools/pkglint/files/shell.go @@ -4,6 +4,7 @@ package main import ( "netbsd.org/pkglint/line" + "netbsd.org/pkglint/linechecks" "netbsd.org/pkglint/textproc" "netbsd.org/pkglint/trace" "path" @@ -596,7 +597,7 @@ func (scc *SimpleCommandChecker) checkAbsolutePathnames() { isSubst := false for _, arg := range scc.strcmd.Args { if !isSubst { - LineChecker{scc.shline.mkline}.CheckAbsolutePathname(arg) + linechecks.CheckAbsolutePathname(scc.shline.mkline, arg) } if false && isSubst && !matches(arg, `"^[\"\'].*[\"\']$`) { scc.shline.mkline.Warnf("Substitution commands like %q should always be quoted.", arg) diff --git a/pkgtools/pkglint/files/textproc/expecter.go b/pkgtools/pkglint/files/textproc/expecter.go index cd29d59590c..672b25d78f2 100644 --- a/pkgtools/pkglint/files/textproc/expecter.go +++ b/pkgtools/pkglint/files/textproc/expecter.go @@ -52,7 +52,7 @@ func (exp *Expecter) StepBack() { exp.index-- } -func (exp *Expecter) AdvanceIfMatches(re regex.RegexPattern) bool { +func (exp *Expecter) AdvanceIfMatches(re regex.Pattern) bool { if trace.Tracing { defer trace.Call(exp.CurrentLine().Text(), re)() } diff --git a/pkgtools/pkglint/files/textproc/prefixreplacer.go b/pkgtools/pkglint/files/textproc/prefixreplacer.go index 535a78dea62..4cea506cec2 100644 --- a/pkgtools/pkglint/files/textproc/prefixreplacer.go +++ b/pkgtools/pkglint/files/textproc/prefixreplacer.go @@ -11,6 +11,8 @@ var Testing bool type PrefixReplacerMark string +// PrefixReplacer parses an arbitrary string into its components by repeatedly +// stripping off a prefix matched by a literal string or a regular expression. type PrefixReplacer struct { rest string s string @@ -80,7 +82,7 @@ func (pr *PrefixReplacer) AdvanceHspace() bool { return false } -func (pr *PrefixReplacer) AdvanceRegexp(re regex.RegexPattern) bool { +func (pr *PrefixReplacer) AdvanceRegexp(re regex.Pattern) bool { pr.m = nil pr.s = "" if !strings.HasPrefix(string(re), "^") { diff --git a/pkgtools/pkglint/files/util.go b/pkgtools/pkglint/files/util.go index fc1538b0219..32feacc2b29 100644 --- a/pkgtools/pkglint/files/util.go +++ b/pkgtools/pkglint/files/util.go @@ -25,19 +25,19 @@ func hasPrefix(s, prefix string) bool { func hasSuffix(s, suffix string) bool { return strings.HasSuffix(s, suffix) } -func matches(s string, re regex.RegexPattern) bool { +func matches(s string, re regex.Pattern) bool { return regex.Matches(s, re) } -func match1(s string, re regex.RegexPattern) (matched bool, m1 string) { +func match1(s string, re regex.Pattern) (matched bool, m1 string) { return regex.Match1(s, re) } -func match2(s string, re regex.RegexPattern) (matched bool, m1, m2 string) { +func match2(s string, re regex.Pattern) (matched bool, m1, m2 string) { return regex.Match2(s, re) } -func match3(s string, re regex.RegexPattern) (matched bool, m1, m2, m3 string) { +func match3(s string, re regex.Pattern) (matched bool, m1, m2, m3 string) { return regex.Match3(s, re) } -func match4(s string, re regex.RegexPattern) (matched bool, m1, m2, m3, m4 string) { +func match4(s string, re regex.Pattern) (matched bool, m1, m2, m3, m4 string) { return regex.Match4(s, re) } @@ -55,7 +55,7 @@ func imax(a, b int) int { return b } -func mustMatch(s string, re regex.RegexPattern) []string { +func mustMatch(s string, re regex.Pattern) []string { if m := regex.Match(s, re); m != nil { return m } @@ -261,7 +261,7 @@ func mkopSubst(s string, left bool, from string, right bool, to string, flags st if trace.Tracing { defer trace.Call(s, left, from, right, to, flags)() } - re := regex.RegexPattern(ifelseStr(left, "^", "") + regexp.QuoteMeta(from) + ifelseStr(right, "$", "")) + re := regex.Pattern(ifelseStr(left, "^", "") + regexp.QuoteMeta(from) + ifelseStr(right, "$", "")) done := false gflag := contains(flags, "g") return regex.Compile(re).ReplaceAllStringFunc(s, func(match string) string { @@ -319,7 +319,7 @@ func containsVarRef(s string) bool { return contains(s, "${") } -func reReplaceRepeatedly(from string, re regex.RegexPattern, to string) string { +func reReplaceRepeatedly(from string, re regex.Pattern, to string) string { replaced := regex.Compile(re).ReplaceAllString(from, to) if replaced != from { return reReplaceRepeatedly(replaced, re, to) diff --git a/pkgtools/pkglint/files/vardefs.go b/pkgtools/pkglint/files/vardefs.go index fe1996a957a..05b306d6ded 100644 --- a/pkgtools/pkglint/files/vardefs.go +++ b/pkgtools/pkglint/files/vardefs.go @@ -585,6 +585,7 @@ func (gd *GlobalData) InitVartypes() { acl("HAS_CONFIGURE", lkNone, BtYes, "Makefile, Makefile.common: set") pkglist("HEADER_TEMPLATES", lkShell, BtPathname) pkg("HOMEPAGE", lkNone, BtHomepage) + pkg("ICON_THEMES", lkNone, BtYes) acl("IGNORE_PKG.*", lkNone, BtYes, "*: set, use-loadtime") sys("IMAKE", lkNone, BtShellCommand) acl("INCOMPAT_CURSES", lkSpace, BtMachinePlatformPattern, "Makefile: set, append") diff --git a/pkgtools/pkglint/files/vartypecheck.go b/pkgtools/pkglint/files/vartypecheck.go index 4bb5802ef61..a61752b03ca 100644 --- a/pkgtools/pkglint/files/vartypecheck.go +++ b/pkgtools/pkglint/files/vartypecheck.go @@ -2,6 +2,7 @@ package main import ( "netbsd.org/pkglint/line" + "netbsd.org/pkglint/linechecks" "netbsd.org/pkglint/regex" "netbsd.org/pkglint/trace" "path" @@ -676,7 +677,7 @@ func (cv *VartypeCheck) Pathmask() { if !matches(cv.ValueNoVar, `^[#\-0-9A-Za-z._~+%*?/\[\]]*`) { cv.Line.Warnf("%q is not a valid pathname mask.", cv.Value) } - LineChecker{cv.Line}.CheckAbsolutePathname(cv.Value) + linechecks.CheckAbsolutePathname(cv.Line, cv.Value) } // Like Filename, but including slashes @@ -688,7 +689,7 @@ func (cv *VartypeCheck) Pathname() { if !matches(cv.ValueNoVar, `^[#\-0-9A-Za-z._~+%/]*$`) { cv.Line.Warnf("%q is not a valid pathname.", cv.Value) } - LineChecker{cv.Line}.CheckAbsolutePathname(cv.Value) + linechecks.CheckAbsolutePathname(cv.Line, cv.Value) } func (cv *VartypeCheck) Perl5Packlist() { |