diff options
author | rillig <rillig@pkgsrc.org> | 2016-11-14 01:08:23 +0000 |
---|---|---|
committer | rillig <rillig@pkgsrc.org> | 2016-11-14 01:08:23 +0000 |
commit | 2b293cbe11a78a5172d06b5de6b4704a26074ba0 (patch) | |
tree | d39beeae1debe4d067248559b79d54a91caa8d31 | |
parent | 3f80878731edc50e19680fb483c31d9482aad0be (diff) | |
download | pkgsrc-2b293cbe11a78a5172d06b5de6b4704a26074ba0.tar.gz |
Updated pkglint to 5.4.11.
Changes since 5.4.10:
* Replaced regular expression with hand-written matching code, since
it is 30 times as fast.
* Reduced number of syscalls by remembering os.Lstat results and
CVS/Entries.
* Reduced number of syscalls by querying the current user only once.
* Added warning for comparing ${PKGSRC_COMPILER} == "clang", which
should rather be ${PKGSRC_COMPILER:Mclang}.
* Added variable definitions for NOT_PAX_ASLR_SAFE and NOT_PAX_MPROTECT_SAFE.
-rw-r--r-- | pkgtools/pkglint/Makefile | 4 | ||||
-rw-r--r-- | pkgtools/pkglint/files/globaldata.go | 2 | ||||
-rw-r--r-- | pkgtools/pkglint/files/globalvars.go | 19 | ||||
-rw-r--r-- | pkgtools/pkglint/files/logging.go | 2 | ||||
-rw-r--r-- | pkgtools/pkglint/files/main.go | 7 | ||||
-rw-r--r-- | pkgtools/pkglint/files/mkline.go | 10 | ||||
-rw-r--r-- | pkgtools/pkglint/files/mkline_test.go | 16 | ||||
-rw-r--r-- | pkgtools/pkglint/files/package.go | 21 | ||||
-rw-r--r-- | pkgtools/pkglint/files/pkglint.go | 63 | ||||
-rw-r--r-- | pkgtools/pkglint/files/plist_test.go | 2 | ||||
-rw-r--r-- | pkgtools/pkglint/files/util.go | 33 | ||||
-rw-r--r-- | pkgtools/pkglint/files/util_test.go | 58 | ||||
-rw-r--r-- | pkgtools/pkglint/files/vardefs.go | 2 |
13 files changed, 197 insertions, 42 deletions
diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile index 9690a1b2ea7..971120b84ad 100644 --- a/pkgtools/pkglint/Makefile +++ b/pkgtools/pkglint/Makefile @@ -1,6 +1,6 @@ -# $NetBSD: Makefile,v 1.499 2016/11/01 21:40:25 rillig Exp $ +# $NetBSD: Makefile,v 1.500 2016/11/14 01:08:23 rillig Exp $ -PKGNAME= pkglint-5.4.10 +PKGNAME= pkglint-5.4.11 DISTFILES= # none CATEGORIES= pkgtools diff --git a/pkgtools/pkglint/files/globaldata.go b/pkgtools/pkglint/files/globaldata.go index 6a7ae1e8cc8..4fb0c405867 100644 --- a/pkgtools/pkglint/files/globaldata.go +++ b/pkgtools/pkglint/files/globaldata.go @@ -114,7 +114,7 @@ func (gd *GlobalData) loadTools() { fname := G.globalData.Pkgsrcdir + "/mk/tools/bsd.tools.mk" lines := LoadExistingLines(fname, true) for _, line := range lines { - if m, _, _, includefile := match3(line.Text, reMkInclude); m { + if m, _, _, includefile := MatchMkInclude(line.Text); m { if !contains(includefile, "/") { toolFiles = append(toolFiles, includefile) } diff --git a/pkgtools/pkglint/files/globalvars.go b/pkgtools/pkglint/files/globalvars.go index cc94335eb86..a76adb85dfd 100644 --- a/pkgtools/pkglint/files/globalvars.go +++ b/pkgtools/pkglint/files/globalvars.go @@ -11,12 +11,15 @@ type GlobalVars struct { Pkg *Package // The package that is currently checked. Mk *MkLines // The Makefile (or fragment) that is currently checked. - Todo []string // The files or directories that still need to be checked. - CurrentDir string // The currently checked directory, relative to the cwd - CurPkgsrcdir string // The pkgsrc directory, relative to currentDir - Wip bool // Is the currently checked directory from pkgsrc-wip? - Infrastructure bool // Is the currently checked item from the pkgsrc infrastructure? - Testing bool // Is pkglint in self-testing mode (only during development)? + Todo []string // The files or directories that still need to be checked. + CurrentDir string // The currently checked directory, relative to the cwd + CurPkgsrcdir string // The pkgsrc directory, relative to currentDir + Wip bool // Is the currently checked directory from pkgsrc-wip? + Infrastructure bool // Is the currently checked item from the pkgsrc infrastructure? + Testing bool // Is pkglint in self-testing mode (only during development)? + CurrentUsername string // For checking against OWNER and MAINTAINER + CvsEntriesDir string // Cached to avoid I/O + CvsEntriesLines []*Line Hash map[string]*Hash // Maps "alg:fname" => hash (inter-package check). UsedLicenses map[string]bool // Maps "license name" => true (inter-package check). @@ -32,10 +35,10 @@ type GlobalVars struct { logErr io.Writer debugOut io.Writer - res map[string]*regexp.Regexp + res map[string]*regexp.Regexp // Compiled regular expressions rematch *Histogram renomatch *Histogram - retime *Histogram + retime *Histogram // Total time taken by matching a regular expression loghisto *Histogram } diff --git a/pkgtools/pkglint/files/logging.go b/pkgtools/pkglint/files/logging.go index d5aca5c8fdf..9c753208bcd 100644 --- a/pkgtools/pkglint/files/logging.go +++ b/pkgtools/pkglint/files/logging.go @@ -105,7 +105,7 @@ func Explain(explanation ...string) { print(fmt.Sprintf("Long explanation line (%d): %s\n", l, s)) } if m, before := match1(s, `(.+)\. [^ ]`); m { - if !matches(before, `\d$`) { + if !matches(before, `\d$|e\.g`) { print(fmt.Sprintf("Short space after period: %s\n", s)) } } diff --git a/pkgtools/pkglint/files/main.go b/pkgtools/pkglint/files/main.go index 05d8cb950e6..05c1a5ab992 100644 --- a/pkgtools/pkglint/files/main.go +++ b/pkgtools/pkglint/files/main.go @@ -4,6 +4,7 @@ import ( "fmt" "io" "os" + "os/user" "path/filepath" "runtime/pprof" ) @@ -61,6 +62,12 @@ func (pkglint *Pkglint) Main(args ...string) (exitcode int) { G.globalData.Initialize() + currentUser, err := user.Current() + if err == nil { + // On Windows, this is `Computername\Username`. + G.CurrentUsername = regcomp(`^.*\\`).ReplaceAllString(currentUser.Username, "") + } + for len(G.Todo) != 0 { item := G.Todo[0] G.Todo = G.Todo[1:] diff --git a/pkgtools/pkglint/files/mkline.go b/pkgtools/pkglint/files/mkline.go index 61d9ed23bfc..06d0b5f46fc 100644 --- a/pkgtools/pkglint/files/mkline.go +++ b/pkgtools/pkglint/files/mkline.go @@ -118,7 +118,7 @@ func NewMkLine(line *Line) (mkline *MkLine) { return } - if m, indent, directive, includefile := match3(text, reMkInclude); m { + if m, indent, directive, includefile := MatchMkInclude(text); m { mkline.xtype = 6 mkline.data = mkLineInclude{directive == "include", indent, includefile, ""} return @@ -1196,6 +1196,14 @@ func (mkline *MkLine) CheckCond() { value := node.args[2].(string) if len(varmods) == 0 { mkline.CheckVartype(varname, opUse, value, "") + if varname == "PKGSRC_COMPILER" { + op := node.args[1].(string) + mkline.Line.Warnf("Use ${PKGSRC_COMPILER:%s%s} instead of the %s operator.", ifelseStr(op == "==", "M", "N"), value, op) + Explain( + "The PKGSRC_COMPILER can be a list of chained compilers, e.g. \"ccache", + "distcc clang\". Therefore, comparing it using == or != leads to", + "wrong results in these cases.") + } } else if len(varmods) == 1 && matches(varmods[0], `^[MN]`) && value != "" { mkline.CheckVartype(varname, opUseMatch, value, "") } diff --git a/pkgtools/pkglint/files/mkline_test.go b/pkgtools/pkglint/files/mkline_test.go index f55df6f807f..9ff4c0d33a4 100644 --- a/pkgtools/pkglint/files/mkline_test.go +++ b/pkgtools/pkglint/files/mkline_test.go @@ -760,7 +760,21 @@ func (s *Suite) Test_MkLine_CheckCond_comparison_with_shell_command(c *check.C) G.Mk.Check() - c.Check(s.Output(), equals, "") // Don’t warn about unknown shell command "cc". + // Don’t warn about unknown shell command "cc". + c.Check(s.Output(), equals, "WARN: security/openssl/Makefile:2: Use ${PKGSRC_COMPILER:Mgcc} instead of the == operator.\n") +} + +func (s *Suite) Test_MkLine_CheckCond_comparing_PKGSRC_COMPILER_with_eqeq(c *check.C) { + s.UseCommandLine(c, "-Wall") + G.globalData.InitVartypes() + G.Mk = s.NewMkLines("audio/pulseaudio/Makefile", + mkrcsid, + ".if ${OPSYS} == \"Darwin\" && ${PKGSRC_COMPILER} == \"clang\"", + ".endif") + + G.Mk.Check() + + c.Check(s.Output(), equals, "WARN: audio/pulseaudio/Makefile:2: Use ${PKGSRC_COMPILER:Mclang} instead of the == operator.\n") } func (s *Suite) Test_MkLine_Pkgmandir(c *check.C) { diff --git a/pkgtools/pkglint/files/package.go b/pkgtools/pkglint/files/package.go index 57bdb4f19fe..8128777fe53 100644 --- a/pkgtools/pkglint/files/package.go +++ b/pkgtools/pkglint/files/package.go @@ -2,7 +2,6 @@ package main import ( "fmt" - "os/user" "path" "regexp" "strconv" @@ -329,11 +328,13 @@ func (pkg *Package) readMakefile(fname string, mainLines *MkLines, allLines *MkL // current file and in the current working directory. // Pkglint doesn’t have an include dir list, like make(1) does. if !fileExists(dirname + "/" + includeFile) { - dirname = G.CurrentDir - } - if !fileExists(dirname + "/" + includeFile) { - line.Error1("Cannot read %q.", dirname+"/"+includeFile) - return false + if dirname != G.CurrentDir { // Prevent unnecessary syscalls + dirname = G.CurrentDir + if !fileExists(dirname + "/" + includeFile) { + line.Error1("Cannot read %q.", dirname+"/"+includeFile) + return false + } + } } if G.opts.Debug { @@ -808,13 +809,7 @@ func (pkg *Package) checkLocallyModified(fname string) { return } - user, err := user.Current() - if err != nil || user.Username == "" { - return - } - // On Windows, this is `Computername\Username`. - username := regcomp(`^.*\\`).ReplaceAllString(user.Username, "") - + username := G.CurrentUsername if G.opts.Debug { traceStep("user=%q owner=%q maintainer=%q", username, owner, maintainer) } diff --git a/pkgtools/pkglint/files/pkglint.go b/pkgtools/pkglint/files/pkglint.go index 9cac45d13db..59fdbd98842 100644 --- a/pkgtools/pkglint/files/pkglint.go +++ b/pkgtools/pkglint/files/pkglint.go @@ -7,8 +7,7 @@ import ( ) const ( - reMkInclude = `^\.(\s*)(s?include)\s+\"([^\"]+)\"\s*(?:#.*)?$` - rePkgname = `^([\w\-.+]+)-(\d(?:\w|\.\d)*)$` + rePkgname = `^([\w\-.+]+)-(\d(?:\w|\.\d)*)$` ) // Returns the pkgsrc top-level directory, relative to the given file or directory. @@ -386,6 +385,66 @@ func MatchVarassign(text string) (m bool, varname, spaceAfterVarname, op, valueA return } +func MatchMkInclude(text string) (bool, string, string, string) { + goto start +fail: + return false, "", "", "" + +start: + len := len(text) + if len == 0 || text[0] != '.' { + goto fail + } + + i := 1 + + spaceStart := i + for i < len && (text[i] == ' ' || text[i] == '\t') { + i++ + } + spaceEnd := i + + directiveStart := i + if i < len && text[i] == 's' { + i++ + } + if !(i+7 < len && text[i] == 'i' && text[i+1] == 'n' && text[i+2] == 'c' && text[i+3] == 'l' && text[i+4] == 'u' && text[i+5] == 'd' && text[i+6] == 'e') { + goto fail + } + i += 7 + directiveEnd := i + + for i < len && (text[i] == ' ' || text[i] == '\t') { + i++ + } + if !(i < len && text[i] == '"') { + goto fail + } + i++ + + pathStart := i + for i < len && text[i] != '"' { + i++ + } + pathEnd := i + if !(pathStart < pathEnd) { + goto fail + } + + if !(i < len && text[i] == '"') { + goto fail + } + i++ + + for i < len && (text[i] == ' ' || text[i] == '\t') { + i++ + } + if !(i == len || i < len && text[i] == '#') { + goto fail + } + return true, text[spaceStart:spaceEnd], text[directiveStart:directiveEnd], text[pathStart:pathEnd] +} + type DependencyPattern struct { pkgbase string // "freeciv-client", "{gcc48,gcc48-libs}", "${EMACS_REQD}" lowerOp string // ">=", ">" diff --git a/pkgtools/pkglint/files/plist_test.go b/pkgtools/pkglint/files/plist_test.go index 119bb5d3a42..b0d7da1c022 100644 --- a/pkgtools/pkglint/files/plist_test.go +++ b/pkgtools/pkglint/files/plist_test.go @@ -221,7 +221,7 @@ func (s *Suite) Test_PlistChecker__autofix(c *check.C) { "AUTOFIX: ~/PLIST: Has been auto-fixed. Please re-run pkglint.\n") c.Check(len(lines), equals, len(fixedLines)) c.Check(s.LoadTmpFile(c, "PLIST"), equals, ""+ - "@comment $NetBSD: plist_test.go,v 1.9 2016/11/01 21:40:25 rillig Exp $\n"+ + "@comment $"+"NetBSD$\n"+ "${PLIST.xen}lib/libvirt/connection-driver/libvirt_driver_libxl.la\n"+ "${PLIST.hal}lib/libvirt/connection-driver/libvirt_driver_nodedev.la\n"+ "lib/libvirt/connection-driver/libvirt_driver_storage.la\n"+ diff --git a/pkgtools/pkglint/files/util.go b/pkgtools/pkglint/files/util.go index 347f4f05d6f..5b8cf08d8ac 100644 --- a/pkgtools/pkglint/files/util.go +++ b/pkgtools/pkglint/files/util.go @@ -71,13 +71,10 @@ func getSubdirs(fname string) []string { // Checks whether a file is already committed to the CVS repository. func isCommitted(fname string) bool { - basename := path.Base(fname) - lines, err := readLines(path.Dir(fname)+"/CVS/Entries", false) - if err != nil { - return false - } + lines := loadCvsEntries(fname) + needle := "/" + path.Base(fname) + "/" for _, line := range lines { - if hasPrefix(line.Text, "/"+basename+"/") { + if hasPrefix(line.Text, needle) { return true } } @@ -85,13 +82,10 @@ func isCommitted(fname string) bool { } func isLocallyModified(fname string) bool { - basename := path.Base(fname) - lines, err := readLines(path.Dir(fname)+"/CVS/Entries", false) - if err != nil { - return false - } + lines := loadCvsEntries(fname) + needle := "/" + path.Base(fname) + "/" for _, line := range lines { - if hasPrefix(line.Text, "/"+basename+"/") { + if hasPrefix(line.Text, needle) { cvsModTime, err := time.Parse(time.ANSIC, strings.Split(line.Text, "/")[3]) if err != nil { return false @@ -112,6 +106,21 @@ func isLocallyModified(fname string) bool { return false } +func loadCvsEntries(fname string) []*Line { + dir := path.Dir(fname) + if dir == G.CvsEntriesDir { + return G.CvsEntriesLines + } + + lines, err := readLines(dir+"/CVS/Entries", false) + if err != nil { + return nil + } + G.CvsEntriesDir = dir + G.CvsEntriesLines = lines + return lines +} + // Returns the number of columns that a string occupies when printed with // a tabulator size of 8. func tabLength(s string) int { diff --git a/pkgtools/pkglint/files/util_test.go b/pkgtools/pkglint/files/util_test.go index 9a091441e0e..ea2fa730943 100644 --- a/pkgtools/pkglint/files/util_test.go +++ b/pkgtools/pkglint/files/util_test.go @@ -2,6 +2,7 @@ package main import ( check "gopkg.in/check.v1" + "testing" ) func (s *Suite) Test_MkopSubst__middle(c *check.C) { @@ -86,3 +87,60 @@ func (s *Suite) Test_PrefixReplacer_Since(c *check.C) { repl.AdvanceRegexp(`^\w+`) c.Check(repl.Since(mark), equals, "hello") } + +const reMkIncludeBenchmark = `^\.(\s*)(s?include)\s+\"([^\"]+)\"\s*(?:#.*)?$` +const reMkIncludeBenchmarkPositive = `^\.(\s*)(s?include)\s+\"(.+)\"\s*(?:#.*)?$` + +func Benchmark_match3_buildlink3(b *testing.B) { + for i := 0; i < b.N; i++ { + match3(".include \"../../category/package/buildlink3.mk\"", reMkIncludeBenchmark) + } +} + +func Benchmark_match3_bsd_pkg_mk(b *testing.B) { + for i := 0; i < b.N; i++ { + match3(".include \"../../mk/bsd.pkg.mk\"", reMkIncludeBenchmark) + } +} + +func Benchmark_match3_samedir(b *testing.B) { + for i := 0; i < b.N; i++ { + match3(".include \"options.mk\"", reMkIncludeBenchmark) + } +} + +func Benchmark_match3_bsd_pkg_mk_comment(b *testing.B) { + for i := 0; i < b.N; i++ { + match3(".include \"../../mk/bsd.pkg.mk\" # infrastructure ", reMkIncludeBenchmark) + } +} + +func Benchmark_match3_buildlink3_positive(b *testing.B) { + for i := 0; i < b.N; i++ { + match3(".include \"../../category/package/buildlink3.mk\"", reMkIncludeBenchmarkPositive) + } +} + +func Benchmark_match3_bsd_pkg_mk_positive(b *testing.B) { + for i := 0; i < b.N; i++ { + match3(".include \"../../mk/bsd.pkg.mk\"", reMkIncludeBenchmarkPositive) + } +} + +func Benchmark_match3_samedir_positive(b *testing.B) { + for i := 0; i < b.N; i++ { + match3(".include \"options.mk\"", reMkIncludeBenchmarkPositive) + } +} + +func Benchmark_match3_bsd_pkg_mk_comment_positive(b *testing.B) { + for i := 0; i < b.N; i++ { + match3(".include \"../../mk/bsd.pkg.mk\" # infrastructure ", reMkIncludeBenchmarkPositive) + } +} + +func Benchmark_match3_explicit(b *testing.B) { + for i := 0; i < b.N; i++ { + MatchMkInclude(".include \"../../mk/bsd.pkg.mk\" # infrastructure ") + } +} diff --git a/pkgtools/pkglint/files/vardefs.go b/pkgtools/pkglint/files/vardefs.go index 2bf228b6dad..db16e601a58 100644 --- a/pkgtools/pkglint/files/vardefs.go +++ b/pkgtools/pkglint/files/vardefs.go @@ -504,6 +504,8 @@ func (gd *GlobalData) InitVartypes() { pkglist("NOT_FOR_BULK_PLATFORM", lkSpace, BtMachinePlatformPattern) pkglist("NOT_FOR_PLATFORM", lkSpace, BtMachinePlatformPattern) pkg("NOT_FOR_UNPRIVILEGED", lkNone, BtYesNo) + pkglist("NOT_PAX_ASLR_SAFE", lkShell, BtPathmask) + pkglist("NOT_PAX_MPROTECT_SAFE", lkShell, BtPathmask) acl("NO_BIN_ON_CDROM", lkNone, BtRestricted, "Makefile, Makefile.common: set") acl("NO_BIN_ON_FTP", lkNone, BtRestricted, "Makefile, Makefile.common: set") acl("NO_BUILD", lkNone, BtYes, "Makefile, Makefile.common: set; Makefile.*: default, set") |