Received: by mail.netbsd.org (Postfix, from userid 605) id A711084DA9; Tue, 9 Oct 2018 19:12:30 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by mail.netbsd.org (Postfix) with ESMTP id B553B84D7D for ; Tue, 9 Oct 2018 19:12:29 +0000 (UTC) X-Virus-Scanned: amavisd-new at netbsd.org Received: from mail.netbsd.org ([127.0.0.1]) by localhost (mail.netbsd.org [127.0.0.1]) (amavisd-new, port 10025) with ESMTP id oAp0vlM9ylog for ; Tue, 9 Oct 2018 19:12:14 +0000 (UTC) Received: from cvs.NetBSD.org (ivanova.NetBSD.org [IPv6:2001:470:a085:999:28c:faff:fe03:5984]) by mail.netbsd.org (Postfix) with ESMTP id 6C73D84D3B for ; Tue, 9 Oct 2018 19:12:14 +0000 (UTC) Received: by cvs.NetBSD.org (Postfix, from userid 500) id 655D2FBEE; Tue, 9 Oct 2018 19:12:14 +0000 (UTC) Content-Transfer-Encoding: 7bit Content-Type: multipart/mixed; boundary="_----------=_1539112334210460" MIME-Version: 1.0 Date: Tue, 9 Oct 2018 19:12:14 +0000 From: "Roland Illig" Subject: CVS commit: pkgsrc/pkgtools/pkglint To: pkgsrc-changes@NetBSD.org Reply-To: rillig@netbsd.org X-Mailer: log_accum Message-Id: <20181009191214.655D2FBEE@cvs.NetBSD.org> Sender: pkgsrc-changes-owner@NetBSD.org List-Id: pkgsrc-changes.NetBSD.org Precedence: bulk List-Unsubscribe: This is a multi-part message in MIME format. --_----------=_1539112334210460 Content-Disposition: inline Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII" Module Name: pkgsrc Committed By: rillig Date: Tue Oct 9 19:12:13 UTC 2018 Modified Files: pkgsrc/pkgtools/pkglint: Makefile pkgsrc/pkgtools/pkglint/files: alternatives_test.go autofix.go buildlink3_test.go check_test.go files.go mkline.go mkline_test.go mklinechecker.go mklinechecker_test.go mklines.go mklines_test.go mkparser.go mkparser_test.go options.go options_test.go package.go package_test.go patches_test.go pkglint.go pkglint_test.go pkgsrc.go pkgsrc_test.go plist.go shell.go shell_test.go shtokenizer_test.go substcontext.go tools.go tools_test.go util.go util_test.go vardefs.go vardefs_test.go vartype.go vartype_test.go vartypecheck.go vartypecheck_test.go pkgsrc/pkgtools/pkglint/files/getopt: getopt.go getopt_test.go pkgsrc/pkgtools/pkglint/files/licenses: licenses.go licenses_test.go pkgsrc/pkgtools/pkglint/files/textproc: prefixreplacer.go pkgsrc/pkgtools/pkglint/files/trace: tracing_test.go Log Message: pkgtools/pkglint: update to 5.6.4 Changes since 5.6.3: * Allow += for COMMENT * Sync variable type definitions with reality * Fix check for "used but not defined" variables. This check had been broken since pkgtools/pkglint/files/pkglint.pl r1.776 from 2008-10-18 (3cd071958e63), which missed its 10-year anniversary by just 9 days. After fixing this check, pkglint produces about 800 new warnings spread all over pkgsrc, most of which are real typos. * Detect used variables also in .if and .elif conditions. This is closely related to the above fix and reduces the number of "defined but not used" variables, while at the same time producing new warnings because these variables are used at load time, where some of these variables are not yet defined. * Detect variables for which pkglint doesn't know the exact data type by scanning all files under mk/ at startup. Currently there are about 470 of these variables. No "used but not defined" warnings are issued for these variables anymore. * To speed up pkglint when checking the whole pkgsrc tree at once, the most often needed files are cached to reduce IO load. The checks for USE_TOOLS are optimized now since they were a major bottleneck. Together with other performance improvements this makes pkglint about 50% faster when checking the whole pkgsrc tree including pkgsrc-wip. To generate a diff of this commit: cvs rdiff -u -r1.549 -r1.550 pkgsrc/pkgtools/pkglint/Makefile cvs rdiff -u -r1.5 -r1.6 pkgsrc/pkgtools/pkglint/files/alternatives_test.go \ pkgsrc/pkgtools/pkglint/files/options.go \ pkgsrc/pkgtools/pkglint/files/options_test.go \ pkgsrc/pkgtools/pkglint/files/tools.go \ pkgsrc/pkgtools/pkglint/files/tools_test.go cvs rdiff -u -r1.10 -r1.11 pkgsrc/pkgtools/pkglint/files/autofix.go \ pkgsrc/pkgtools/pkglint/files/pkgsrc.go cvs rdiff -u -r1.18 -r1.19 pkgsrc/pkgtools/pkglint/files/buildlink3_test.go cvs rdiff -u -r1.26 -r1.27 pkgsrc/pkgtools/pkglint/files/check_test.go \ pkgsrc/pkgtools/pkglint/files/shell.go cvs rdiff -u -r1.19 -r1.20 pkgsrc/pkgtools/pkglint/files/files.go \ pkgsrc/pkgtools/pkglint/files/mklinechecker.go cvs rdiff -u -r1.38 -r1.39 pkgsrc/pkgtools/pkglint/files/mkline.go \ pkgsrc/pkgtools/pkglint/files/pkglint.go cvs rdiff -u -r1.42 -r1.43 pkgsrc/pkgtools/pkglint/files/mkline_test.go cvs rdiff -u -r1.15 -r1.16 \ pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go \ pkgsrc/pkgtools/pkglint/files/mkparser_test.go cvs rdiff -u -r1.32 -r1.33 pkgsrc/pkgtools/pkglint/files/mklines.go \ pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go cvs rdiff -u -r1.28 -r1.29 pkgsrc/pkgtools/pkglint/files/mklines_test.go cvs rdiff -u -r1.17 -r1.18 pkgsrc/pkgtools/pkglint/files/mkparser.go \ pkgsrc/pkgtools/pkglint/files/vartype.go cvs rdiff -u -r1.36 -r1.37 pkgsrc/pkgtools/pkglint/files/package.go cvs rdiff -u -r1.30 -r1.31 pkgsrc/pkgtools/pkglint/files/package_test.go cvs rdiff -u -r1.21 -r1.22 pkgsrc/pkgtools/pkglint/files/patches_test.go cvs rdiff -u -r1.25 -r1.26 pkgsrc/pkgtools/pkglint/files/pkglint_test.go cvs rdiff -u -r1.8 -r1.9 pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go \ pkgsrc/pkgtools/pkglint/files/shtokenizer_test.go \ pkgsrc/pkgtools/pkglint/files/vartype_test.go cvs rdiff -u -r1.29 -r1.30 pkgsrc/pkgtools/pkglint/files/plist.go \ pkgsrc/pkgtools/pkglint/files/util.go cvs rdiff -u -r1.31 -r1.32 pkgsrc/pkgtools/pkglint/files/shell_test.go cvs rdiff -u -r1.13 -r1.14 pkgsrc/pkgtools/pkglint/files/substcontext.go cvs rdiff -u -r1.14 -r1.15 pkgsrc/pkgtools/pkglint/files/util_test.go cvs rdiff -u -r1.46 -r1.47 pkgsrc/pkgtools/pkglint/files/vardefs.go cvs rdiff -u -r1.4 -r1.5 pkgsrc/pkgtools/pkglint/files/vardefs_test.go cvs rdiff -u -r1.40 -r1.41 pkgsrc/pkgtools/pkglint/files/vartypecheck.go cvs rdiff -u -r1.4 -r1.5 pkgsrc/pkgtools/pkglint/files/getopt/getopt.go cvs rdiff -u -r1.6 -r1.7 pkgsrc/pkgtools/pkglint/files/getopt/getopt_test.go cvs rdiff -u -r1.3 -r1.4 pkgsrc/pkgtools/pkglint/files/licenses/licenses.go cvs rdiff -u -r1.2 -r1.3 \ pkgsrc/pkgtools/pkglint/files/licenses/licenses_test.go cvs rdiff -u -r1.6 -r1.7 \ pkgsrc/pkgtools/pkglint/files/textproc/prefixreplacer.go cvs rdiff -u -r1.1 -r1.2 pkgsrc/pkgtools/pkglint/files/trace/tracing_test.go Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. --_----------=_1539112334210460 Content-Disposition: inline Content-Length: 177562 Content-Transfer-Encoding: binary Content-Type: text/x-diff; charset=us-ascii Modified files: Index: pkgsrc/pkgtools/pkglint/Makefile diff -u pkgsrc/pkgtools/pkglint/Makefile:1.549 pkgsrc/pkgtools/pkglint/Makefile:1.550 --- pkgsrc/pkgtools/pkglint/Makefile:1.549 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/Makefile Tue Oct 9 19:12:13 2018 @@ -1,6 +1,6 @@ -# $NetBSD: Makefile,v 1.549 2018/10/03 22:27:53 rillig Exp $ +# $NetBSD: Makefile,v 1.550 2018/10/09 19:12:13 rillig Exp $ -PKGNAME= pkglint-5.6.3 +PKGNAME= pkglint-5.6.4 DISTFILES= # none CATEGORIES= pkgtools Index: pkgsrc/pkgtools/pkglint/files/alternatives_test.go diff -u pkgsrc/pkgtools/pkglint/files/alternatives_test.go:1.5 pkgsrc/pkgtools/pkglint/files/alternatives_test.go:1.6 --- pkgsrc/pkgtools/pkglint/files/alternatives_test.go:1.5 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/alternatives_test.go Tue Oct 9 19:12:13 2018 @@ -21,11 +21,7 @@ func (s *Suite) Test_CheckfileAlternativ G.CheckDirent(".") - // TODO: Remove redundant diagnostics. t.CheckOutputLines( - "NOTE: ALTERNATIVES:1: @PREFIX@/ can be omitted from the file name.", - "NOTE: ALTERNATIVES:2: @PREFIX@/ can be omitted from the file name.", - "ERROR: ALTERNATIVES:5: Invalid ALTERNATIVES line \"invalid\".", "ERROR: ALTERNATIVES:1: Alternative implementation \"@PREFIX@/sbin/sendmail.postfix@POSTFIXVER@\" must appear in the PLIST as \"sbin/sendmail.postfix${POSTFIXVER}\".", "NOTE: ALTERNATIVES:1: @PREFIX@/ can be omitted from the file name.", "NOTE: ALTERNATIVES:2: @PREFIX@/ can be omitted from the file name.", Index: pkgsrc/pkgtools/pkglint/files/options.go diff -u pkgsrc/pkgtools/pkglint/files/options.go:1.5 pkgsrc/pkgtools/pkglint/files/options.go:1.6 --- pkgsrc/pkgtools/pkglint/files/options.go:1.5 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/options.go Tue Oct 9 19:12:13 2018 @@ -37,7 +37,7 @@ loop: case mkline.IsVarassign(): switch mkline.Varcanon() { case "PKG_SUPPORTED_OPTIONS", "PKG_OPTIONS_GROUP.*", "PKG_OPTIONS_SET.*": - for _, option := range splitOnSpace(mkline.Value()) { + for _, option := range fields(mkline.Value()) { if !containsVarRef(option) { declaredOptions[option] = mkline optionsInDeclarationOrder = append(optionsInDeclarationOrder, option) @@ -68,7 +68,7 @@ loop: for ; !exp.EOF(); exp.Advance() { mkline := exp.CurrentMkLine() if mkline.IsDirective() && (mkline.Directive() == "if" || mkline.Directive() == "elif") { - cond := NewMkParser(mkline.Line, mkline.Args(), false).MkCond() + cond := mkline.Cond() if cond == nil { continue } Index: pkgsrc/pkgtools/pkglint/files/options_test.go diff -u pkgsrc/pkgtools/pkglint/files/options_test.go:1.5 pkgsrc/pkgtools/pkglint/files/options_test.go:1.6 --- pkgsrc/pkgtools/pkglint/files/options_test.go:1.5 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/options_test.go Tue Oct 9 19:12:13 2018 @@ -54,6 +54,7 @@ func (s *Suite) Test_ChecklinesOptionsMk ChecklinesOptionsMk(mklines) t.CheckOutputLines( + "WARN: ~/category/package/options.mk:6: l is used but not defined.", "WARN: ~/category/package/options.mk:18: Unknown option \"undeclared\".", "NOTE: ~/category/package/options.mk:21: The positive branch of the .if/.else should be the one where the option is set.", "WARN: ~/category/package/options.mk:6: Option \"mc-charset\" should be handled below in an .if block.", Index: pkgsrc/pkgtools/pkglint/files/tools.go diff -u pkgsrc/pkgtools/pkglint/files/tools.go:1.5 pkgsrc/pkgtools/pkglint/files/tools.go:1.6 --- pkgsrc/pkgtools/pkglint/files/tools.go:1.5 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/tools.go Tue Oct 9 19:12:13 2018 @@ -1,8 +1,8 @@ package main import ( + "fmt" "netbsd.org/pkglint/trace" - "path" "sort" "strings" ) @@ -12,6 +12,10 @@ import ( // pkgsrc. // // See `mk/tools/`. +// +// TODO: MustUseVarForm does not really depend on the tool but only depends +// on where the tool is used (load time, run time). This had already been +// modeled wrong in pkglint 4, more than 10 years ago. type Tool struct { Name string // e.g. "sed", "gzip" Varname string // e.g. "SED", "GZIP_CMD" @@ -19,11 +23,9 @@ type Tool struct { Validity Validity } -func (tool *Tool) SetValidity(validity Validity, traceName string) { - if trace.Tracing && validity != tool.Validity { - trace.Stepf("%s: Setting validity of %q to %s", traceName, tool.Name, validity) - } - tool.Validity = validity +func (tool *Tool) String() string { + return fmt.Sprintf("%s:%s:%s:%s", + tool.Name, tool.Varname, ifelseStr(tool.MustUseVarForm, "var", ""), tool.Validity) } // UsableAtLoadTime means that the tool may be used by its variable @@ -82,19 +84,22 @@ type Tools struct { TraceName string // Only for the trace log byName map[string]*Tool // "sed" => tool byVarname map[string]*Tool // "GREP_CMD" => tool - SeenPrefs bool // Determines the effect of adding the tool to USE_TOOLS + fallback *Tools + SeenPrefs bool // Determines the effect of adding the tool to USE_TOOLS } -func NewTools(traceName string) Tools { - return Tools{ +func NewTools(traceName string) *Tools { + return &Tools{ traceName, make(map[string]*Tool), make(map[string]*Tool), + nil, false} } // Define registers the tool by its name and the corresponding -// variable name (if nonempty). +// variable name (if nonempty). Depending on the given mkline, +// it may be added to USE_TOOLS automatically. // // After this tool is added to USE_TOOLS, it may be used by this name // (e.g. "awk") or by its variable (e.g. ${AWK}). @@ -103,33 +108,28 @@ func (tr *Tools) Define(name, varname st trace.Stepf("Tools.Define for %s: %q %q in %s", tr.TraceName, name, varname, mkline) } - tool := tr.def(name, varname, mkline) - if varname != "" { - tool.Varname = varname + if !tr.IsValidToolName(name) { + mkline.Errorf("Invalid tool name %q.", name) } - return tool + + validity := tr.validity(mkline.Basename, false) + return tr.defTool(name, varname, false, validity) } -func (tr *Tools) def(name, varname string, mkline MkLine) *Tool { - if mkline != nil && !tr.IsValidToolName(name) { - mkline.Errorf("Invalid tool name %q.", name) - } +func (tr *Tools) defTool(name, varname string, mustUseVarForm bool, validity Validity) *Tool { + fresh := &Tool{name, varname, mustUseVarForm, validity} - validity := Nowhere - if mkline != nil { - if IsPrefs(mkline.Filename) { - validity = AfterPrefsMk - } else if path.Base(mkline.Filename) == "bsd.pkg.mk" { - validity = AtRunTime - } + tool := tr.byName[name] + if tool == nil { + tool = fresh + tr.byName[name] = tool + } else { + tr.merge(tool, fresh) } - tool := &Tool{name, varname, false, validity} - if name != "" { - if existing := tr.byName[name]; existing != nil { - tool = existing - } else { - tr.byName[name] = tool + if tr.fallback != nil { + if fallback := tr.fallback.ByName(name); fallback != nil { + tr.merge(tool, fallback) } } @@ -142,6 +142,18 @@ func (tr *Tools) def(name, varname strin return tool } +func (tr *Tools) merge(target, source *Tool) { + if target.Varname == "" && source.Varname != "" { + target.Varname = source.Varname + } + if !target.MustUseVarForm && source.MustUseVarForm { + target.MustUseVarForm = true + } + if source.Validity > target.Validity { + target.Validity = source.Validity + } +} + func (tr *Tools) Trace() { if trace.Tracing { defer trace.Call1(tr.TraceName)() @@ -158,17 +170,23 @@ func (tr *Tools) Trace() { for _, toolname := range keys { trace.Stepf("tool %+v", tr.byName[toolname]) } + + if tr.fallback != nil { + tr.fallback.Trace() + } } // ParseToolLine updates the tool definitions according to the given // line from a Makefile. -func (tr *Tools) ParseToolLine(mkline MkLine) { - tr.ParseToolLineCreate(mkline, false) -} - -// ParseToolLineCreate updates the tool definitions according to the given -// line from a Makefile, registering the tools if necessary. -func (tr *Tools) ParseToolLineCreate(mkline MkLine, createIfAbsent bool) { +// +// If fromInfrastructure is true, the tool is defined even when it is only +// added to USE_TOOLS (which normally doesn't define anything). This way, +// pkglint also finds those tools whose definitions are too difficult to +// parse from the code. +// +// If addToUseTools is true, a USE_TOOLS line makes a tool immediately +// usable. This should only be done if the current line is unconditional. +func (tr *Tools) ParseToolLine(mkline MkLine, fromInfrastructure bool, addToUseTools bool) { switch { case mkline.IsVarassign(): @@ -194,13 +212,13 @@ func (tr *Tools) ParseToolLineCreate(mkl case "_TOOLS.*": if !containsVarRef(varparam) { tr.Define(varparam, "", mkline) - for _, tool := range splitOnSpace(value) { + for _, tool := range fields(value) { tr.Define(tool, "", mkline) } } case "USE_TOOLS": - tr.parseUseTools(mkline, createIfAbsent) + tr.parseUseTools(mkline, fromInfrastructure, addToUseTools) } case mkline.IsInclude(): @@ -214,61 +232,68 @@ func (tr *Tools) ParseToolLineCreate(mkl // It determines the validity of the tool, i.e. in which places it may be used. // // If createIfAbsent is true and the tools is unknown, it is registered. -func (tr *Tools) parseUseTools(mkline MkLine, createIfAbsent bool) { +// This can be done only in the pkgsrc infrastructure files, where the +// actual definition is assumed to be in some other file. In packages +// though, this assumption cannot be made and pkglint needs to be strict. +func (tr *Tools) parseUseTools(mkline MkLine, createIfAbsent bool, addToUseTools bool) { value := mkline.Value() if containsVarRef(value) { return } - deps := splitOnSpace(value) + deps := fields(value) // See mk/tools/autoconf.mk:/^\.if !defined/ if matches(value, `\bautoconf213\b`) { - for _, name := range [...]string{"autoconf-2.13", "autoheader-2.13", "autoreconf-2.13", "autoscan-2.13", "autoupdate-2.13", "ifnames-2.13"} { - if createIfAbsent { - tr.Define(name, "", mkline) - } - deps = append(deps, name) - } + deps = append(deps, "autoconf-2.13", "autoheader-2.13", "autoreconf-2.13", "autoscan-2.13", "autoupdate-2.13", "ifnames-2.13") } if matches(value, `\bautoconf\b`) { - for _, name := range [...]string{"autoheader", "autom4te", "autoreconf", "autoscan", "autoupdate", "ifnames"} { - if createIfAbsent { - tr.Define(name, "", mkline) - } - deps = append(deps, name) - } + deps = append(deps, "autoheader", "autom4te", "autoreconf", "autoscan", "autoupdate", "ifnames") } + validity := tr.validity(mkline.Basename, addToUseTools) for _, dep := range deps { name := strings.Split(dep, ":")[0] - tool := tr.ByName(name) - if tool == nil && createIfAbsent { - tr.Define(name, "", mkline) - } - if tool != nil { - validity := tr.validity(mkline.Filename) - if validity > tool.Validity { - tool.SetValidity(validity, tr.TraceName) - } + if createIfAbsent || tr.ByName(name) != nil { + tr.defTool(name, "", false, validity) } } } -func (tr *Tools) validity(fileName string) Validity { - basename := path.Base(fileName) - if basename == "Makefile" && tr.SeenPrefs { - return AtRunTime - } - if basename == "bsd.prefs.mk" || basename == "Makefile" { +func (tr *Tools) validity(basename string, useTools bool) Validity { + switch { + case IsPrefs(basename): // IsPrefs is not 100% accurate here, but good enough + return AfterPrefsMk + case basename == "Makefile" && !tr.SeenPrefs: return AfterPrefsMk + case useTools, basename == "bsd.pkg.mk": + return AtRunTime + default: + return Nowhere } - return AtRunTime } -func (tr *Tools) ByVarname(varname string) (tool *Tool) { return tr.byVarname[varname] } +func (tr *Tools) ByVarname(varname string) *Tool { + tool := tr.byVarname[varname] + if tool == nil && tr.fallback != nil { + fallback := tr.fallback.ByVarname(varname) + if fallback != nil { + return tr.defTool(fallback.Name, fallback.Varname, fallback.MustUseVarForm, fallback.Validity) + } + } + return tool +} -func (tr *Tools) ByName(name string) (tool *Tool) { return tr.byName[name] } +func (tr *Tools) ByName(name string) *Tool { + tool := tr.byName[name] + if tool == nil && tr.fallback != nil { + fallback := tr.fallback.ByName(name) + if fallback != nil { + return tr.defTool(fallback.Name, fallback.Varname, fallback.MustUseVarForm, fallback.Validity) + } + } + return tool +} func (tr *Tools) Usable(tool *Tool, time ToolTime) bool { if time == LoadTime { @@ -278,40 +303,9 @@ func (tr *Tools) Usable(tool *Tool, time } } -func (tr *Tools) AddAll(other Tools) { - if trace.Tracing && len(other.byName) != 0 { - defer trace.Call(other.TraceName+" to "+tr.TraceName, len(other.byName))() - } - - // Same as the code below, just a little faster. - if !trace.Tracing { - for _, otherTool := range other.byName { - tool := tr.def(otherTool.Name, otherTool.Varname, nil) - tool.MustUseVarForm = tool.MustUseVarForm || otherTool.MustUseVarForm - if otherTool.Validity > tool.Validity { - tool.SetValidity(otherTool.Validity, tr.TraceName) - } - } - return - } - - var names []string - for name := range other.byName { - names = append(names, name) - } - sort.Strings(names) - - for _, name := range names { - otherTool := other.byName[name] - if trace.Tracing { - trace.Stepf("Tools.AddAll %+v", *otherTool) - } - tool := tr.def(otherTool.Name, otherTool.Varname, nil) - tool.MustUseVarForm = tool.MustUseVarForm || otherTool.MustUseVarForm - if otherTool.Validity > tool.Validity { - tool.SetValidity(otherTool.Validity, tr.TraceName) - } - } +func (tr *Tools) Fallback(other *Tools) { + G.Assertf(tr.fallback == nil, "Tools.Fallback must only be called once.") + tr.fallback = other } func (tr *Tools) IsValidToolName(name string) bool { Index: pkgsrc/pkgtools/pkglint/files/tools_test.go diff -u pkgsrc/pkgtools/pkglint/files/tools_test.go:1.5 pkgsrc/pkgtools/pkglint/files/tools_test.go:1.6 --- pkgsrc/pkgtools/pkglint/files/tools_test.go:1.5 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/tools_test.go Tue Oct 9 19:12:13 2018 @@ -2,10 +2,37 @@ package main import "gopkg.in/check.v1" +func (s *Suite) Test_Tool_UsableAtLoadTime(c *check.C) { + + nowhere := Tool{"nowhere", "", false, Nowhere} + c.Check(nowhere.UsableAtLoadTime(false), equals, false) + c.Check(nowhere.UsableAtLoadTime(true), equals, false) + + load := Tool{"load", "", false, AfterPrefsMk} + c.Check(load.UsableAtLoadTime(false), equals, false) + c.Check(load.UsableAtLoadTime(true), equals, true) + + run := Tool{"run", "", false, AtRunTime} + c.Check(run.UsableAtLoadTime(false), equals, false) + c.Check(run.UsableAtLoadTime(true), equals, false) +} + +func (s *Suite) Test_Tool_UsableAtRunTime(c *check.C) { + + nowhere := Tool{"nowhere", "", false, Nowhere} + c.Check(nowhere.UsableAtRunTime(), equals, false) + + load := Tool{"load", "", false, AfterPrefsMk} + c.Check(load.UsableAtRunTime(), equals, true) + + run := Tool{"run", "", false, AtRunTime} + c.Check(run.UsableAtRunTime(), equals, true) +} + func (s *Suite) Test_Tools_ParseToolLine(c *check.C) { t := s.Init(c) - t.SetupToolUsable("tool1", "") + t.SetupTool("tool1", "", Nowhere) t.SetupVartypes() t.CreateFileLines("Makefile", MkRcsID, @@ -18,7 +45,7 @@ func (s *Suite) Test_Tools_ParseToolLine t.CheckOutputEmpty() } -func (s *Suite) Test_Tools_def__invalid_tool_name(c *check.C) { +func (s *Suite) Test_Tools_Define__invalid_tool_name(c *check.C) { t := s.Init(c) reg := NewTools("") @@ -65,7 +92,8 @@ func (s *Suite) Test_Tools__USE_TOOLS_pr t.CheckOutputLines( "WARN: ~/module.mk:5: Unknown shell command \"${AWK}\".", - "0 errors and 1 warning found.", + "WARN: ~/module.mk:5: AWK is used but not defined.", + "0 errors and 2 warnings found.", "(Run \"pkglint -e\" to show explanations.)") } @@ -89,9 +117,9 @@ func (s *Suite) Test_Tools__load_from_in tools := NewTools("") - tools.ParseToolLine(t.NewMkLine("create.mk", 2, "TOOLS_CREATE+= load")) - tools.ParseToolLine(t.NewMkLine("create.mk", 3, "TOOLS_CREATE+= run")) - tools.ParseToolLine(t.NewMkLine("create.mk", 4, "TOOLS_CREATE+= nowhere")) + tools.ParseToolLine(t.NewMkLine("create.mk", 2, "TOOLS_CREATE+= load"), true, false) + tools.ParseToolLine(t.NewMkLine("create.mk", 3, "TOOLS_CREATE+= run"), true, false) + tools.ParseToolLine(t.NewMkLine("create.mk", 4, "TOOLS_CREATE+= nowhere"), true, false) // The references to the tools are stable, // the lookup methods always return the same objects. @@ -106,9 +134,9 @@ func (s *Suite) Test_Tools__load_from_in c.Check(nowhere, deepEquals, &Tool{"nowhere", "", false, Nowhere}) // The name RUN_CMD avoids conflicts with RUN. - tools.ParseToolLine(t.NewMkLine("varnames.mk", 2, "_TOOLS_VARNAME.load= LOAD")) - tools.ParseToolLine(t.NewMkLine("varnames.mk", 3, "_TOOLS_VARNAME.run= RUN_CMD")) - tools.ParseToolLine(t.NewMkLine("varnames.mk", 4, "_TOOLS_VARNAME.nowhere= NOWHERE")) + tools.ParseToolLine(t.NewMkLine("varnames.mk", 2, "_TOOLS_VARNAME.load= LOAD"), true, false) + tools.ParseToolLine(t.NewMkLine("varnames.mk", 3, "_TOOLS_VARNAME.run= RUN_CMD"), true, false) + tools.ParseToolLine(t.NewMkLine("varnames.mk", 4, "_TOOLS_VARNAME.nowhere= NOWHERE"), true, false) // At this point the tools can be found by their variable names, too. // They still may not be used. @@ -128,7 +156,7 @@ func (s *Suite) Test_Tools__load_from_in c.Check(nowhere.UsableAtLoadTime(true), equals, false) c.Check(nowhere.UsableAtRunTime(), equals, false) - tools.ParseToolLine(t.NewMkLine("bsd.prefs.mk", 2, "USE_TOOLS+= load")) + tools.ParseToolLine(t.NewMkLine("bsd.prefs.mk", 2, "USE_TOOLS+= load"), true, true) // Tools that are added to USE_TOOLS in bsd.prefs.mk may be used afterwards. // By variable name, they may be used both at load time as well as run time. @@ -138,7 +166,7 @@ func (s *Suite) Test_Tools__load_from_in c.Check(load.UsableAtLoadTime(true), equals, true) c.Check(load.UsableAtRunTime(), equals, true) - tools.ParseToolLine(t.NewMkLine("bsd.pkg.mk", 2, "USE_TOOLS+= run")) + tools.ParseToolLine(t.NewMkLine("bsd.pkg.mk", 2, "USE_TOOLS+= run"), true, true) // Tools that are added to USE_TOOLS in bsd.pkg.mk may be used afterwards at run time. // The {pre,do,post}-* targets may use both forms (${CAT} and cat). @@ -174,7 +202,7 @@ func (s *Suite) Test_Tools__package_Make G.Pkgsrc.LoadInfrastructure() tools := NewTools("") - tools.AddAll(G.Pkgsrc.Tools) + tools.Fallback(G.Pkgsrc.Tools) load := tools.ByName("load") run := tools.ByName("run") @@ -190,7 +218,7 @@ func (s *Suite) Test_Tools__package_Make // All other files must not use the tools at load time. // For them, seenPrefs can be though of as being true from the beginning. - tools.ParseToolLine(t.NewMkLine("Makefile", 2, "USE_TOOLS+= pkg-before-prefs")) + tools.ParseToolLine(t.NewMkLine("Makefile", 2, "USE_TOOLS+= pkg-before-prefs"), false, true) c.Check(before.Validity, equals, AfterPrefsMk) c.Check(before.UsableAtLoadTime(false), equals, false) @@ -199,11 +227,11 @@ func (s *Suite) Test_Tools__package_Make c.Check(tools.SeenPrefs, equals, false) - tools.ParseToolLine(t.NewMkLine("Makefile", 3, ".include \"../../mk/bsd.prefs.mk\"")) + tools.ParseToolLine(t.NewMkLine("Makefile", 3, ".include \"../../mk/bsd.prefs.mk\""), false, true) c.Check(tools.SeenPrefs, equals, true) - tools.ParseToolLine(t.NewMkLine("Makefile", 4, "USE_TOOLS+= pkg-after-prefs")) + tools.ParseToolLine(t.NewMkLine("Makefile", 4, "USE_TOOLS+= pkg-after-prefs"), false, true) c.Check(after.Validity, equals, AtRunTime) c.Check(after.UsableAtLoadTime(false), equals, false) @@ -327,19 +355,19 @@ func (s *Suite) Test_Tools__tools_having t.CheckOutputLines( "TRACE: + (*Tools).Trace(\"Pkgsrc\")", - "TRACE: 1 tool &{Name:awk Varname:AWK MustUseVarForm:false Validity:AfterPrefsMk}", - "TRACE: 1 tool &{Name:echo Varname:ECHO MustUseVarForm:true Validity:AfterPrefsMk}", - "TRACE: 1 tool &{Name:echo -n Varname:ECHO_N MustUseVarForm:true Validity:AfterPrefsMk}", - "TRACE: 1 tool &{Name:false Varname:FALSE MustUseVarForm:true Validity:Nowhere}", - "TRACE: 1 tool &{Name:gawk Varname:AWK MustUseVarForm:false Validity:Nowhere}", - "TRACE: 1 tool &{Name:gsed Varname:SED MustUseVarForm:false Validity:Nowhere}", - "TRACE: 1 tool &{Name:sed Varname:SED MustUseVarForm:false Validity:AfterPrefsMk}", - "TRACE: 1 tool &{Name:test Varname:TEST MustUseVarForm:true Validity:AfterPrefsMk}", - "TRACE: 1 tool &{Name:true Varname:TRUE MustUseVarForm:true Validity:AfterPrefsMk}", + "TRACE: 1 tool awk:AWK::AfterPrefsMk", + "TRACE: 1 tool echo:ECHO:var:AfterPrefsMk", + "TRACE: 1 tool echo -n:ECHO_N:var:AfterPrefsMk", + "TRACE: 1 tool false:FALSE:var:AtRunTime", + "TRACE: 1 tool gawk:AWK::Nowhere", + "TRACE: 1 tool gsed:SED::Nowhere", + "TRACE: 1 tool sed:SED::AfterPrefsMk", + "TRACE: 1 tool test:TEST:var:AfterPrefsMk", + "TRACE: 1 tool true:TRUE:var:AfterPrefsMk", "TRACE: - (*Tools).Trace(\"Pkgsrc\")") tools := NewTools("module.mk") - tools.AddAll(G.Pkgsrc.Tools) + tools.Fallback(G.Pkgsrc.Tools) t.EnableTracingToLog() tools.Trace() @@ -347,15 +375,17 @@ func (s *Suite) Test_Tools__tools_having t.CheckOutputLines( "TRACE: + (*Tools).Trace(\"module.mk\")", - "TRACE: 1 tool &{Name:awk Varname:AWK MustUseVarForm:false Validity:AfterPrefsMk}", - "TRACE: 1 tool &{Name:echo Varname:ECHO MustUseVarForm:true Validity:AfterPrefsMk}", - "TRACE: 1 tool &{Name:echo -n Varname:ECHO_N MustUseVarForm:true Validity:AfterPrefsMk}", - "TRACE: 1 tool &{Name:false Varname:FALSE MustUseVarForm:true Validity:Nowhere}", - "TRACE: 1 tool &{Name:gawk Varname:AWK MustUseVarForm:false Validity:Nowhere}", - "TRACE: 1 tool &{Name:gsed Varname:SED MustUseVarForm:false Validity:Nowhere}", - "TRACE: 1 tool &{Name:sed Varname:SED MustUseVarForm:false Validity:AfterPrefsMk}", - "TRACE: 1 tool &{Name:test Varname:TEST MustUseVarForm:true Validity:AfterPrefsMk}", - "TRACE: 1 tool &{Name:true Varname:TRUE MustUseVarForm:true Validity:AfterPrefsMk}", + "TRACE: 1 + (*Tools).Trace(\"Pkgsrc\")", + "TRACE: 1 2 tool awk:AWK::AfterPrefsMk", + "TRACE: 1 2 tool echo:ECHO:var:AfterPrefsMk", + "TRACE: 1 2 tool echo -n:ECHO_N:var:AfterPrefsMk", + "TRACE: 1 2 tool false:FALSE:var:AtRunTime", + "TRACE: 1 2 tool gawk:AWK::Nowhere", + "TRACE: 1 2 tool gsed:SED::Nowhere", + "TRACE: 1 2 tool sed:SED::AfterPrefsMk", + "TRACE: 1 2 tool test:TEST:var:AfterPrefsMk", + "TRACE: 1 2 tool true:TRUE:var:AfterPrefsMk", + "TRACE: 1 - (*Tools).Trace(\"Pkgsrc\")", "TRACE: - (*Tools).Trace(\"module.mk\")") } @@ -387,52 +417,102 @@ func (s *Suite) Test_Tools__var(c *check t.CheckOutputEmpty() } -// Demonstrates how the Tools type handles tool that share the same +// Demonstrates how the Tools type handles tools that share the same // variable name. Of these tools, the GNU variant is preferred. // +// In this realistic variant, the non-GNU tool is defined in bsd.prefs.mk +// and the GNU tool is only defined but not made available. +// // See also Pkglint.Tool. -func (s *Suite) Test_Tools_AddAll__tools_having_the_same_variable_name(c *check.C) { +func (s *Suite) Test_Tools_Fallback__tools_having_the_same_variable_name_realistic(c *check.C) { nonGnu := NewTools("non-gnu") - nonGnu.Define("sed", "SED", dummyMkLine).SetValidity(AfterPrefsMk, "") + nonGnu.defTool("sed", "SED", false, AfterPrefsMk) gnu := NewTools("gnu") - gnu.Define("gsed", "SED", dummyMkLine) + gnu.defTool("gsed", "SED", false, Nowhere) local1 := NewTools("local") - local1.AddAll(nonGnu) - local1.AddAll(gnu) + local1.defTool("sed", "SED", false, AfterPrefsMk) + local1.Fallback(gnu) c.Check(local1.ByName("sed").Validity, equals, AfterPrefsMk) c.Check(local1.ByName("gsed").Validity, equals, Nowhere) local1.Trace() local2 := NewTools("local") - local2.AddAll(gnu) - local2.AddAll(nonGnu) + local2.defTool("gsed", "SED", false, Nowhere) + local2.Fallback(nonGnu) c.Check(local2.ByName("sed").Validity, equals, AfterPrefsMk) c.Check(local2.ByName("gsed").Validity, equals, Nowhere) local2.Trace() - nonGnu.ByName("sed").Validity = Nowhere - gnu.ByName("gsed").Validity = AfterPrefsMk + // No matter in which order the tool definitions are encountered, + // the non-GNU version is always chosen since the GNU version is + // not available at all. + c.Check(local1.ByVarname("SED").String(), equals, "sed:SED::AfterPrefsMk") + c.Check(local2.ByVarname("SED").String(), equals, "sed:SED::AfterPrefsMk") +} - local3 := NewTools("local") - local3.AddAll(nonGnu) - local3.AddAll(gnu) - - c.Check(local3.ByName("sed").Validity, equals, Nowhere) - c.Check(local3.ByName("gsed").Validity, equals, AfterPrefsMk) - local3.Trace() - - local4 := NewTools("local") - local4.AddAll(gnu) - local4.AddAll(nonGnu) - - c.Check(local4.ByName("sed").Validity, equals, Nowhere) - c.Check(local4.ByName("gsed").Validity, equals, AfterPrefsMk) - local4.Trace() +// Demonstrates how the Tools type handles tools that share the same +// variable name. Of these tools, the GNU variant is preferred. +// +// In this unrealistic variant, the GNU tool is defined in bsd.prefs.mk +// and the non-GNU tool is only defined but not made available. +// +// See also Pkglint.Tool. +func (s *Suite) Test_Tools_Fallback__tools_having_the_same_variable_name_unrealistic(c *check.C) { + nonGnu := NewTools("non-gnu") + nonGnu.defTool("sed", "SED", false, Nowhere) + + gnu := NewTools("gnu") + gnu.defTool("gsed", "SED", false, AfterPrefsMk) + + local1 := NewTools("local") + local1.defTool("sed", "SED", false, Nowhere) + local1.Fallback(gnu) + + c.Check(local1.ByName("sed").Validity, equals, Nowhere) + c.Check(local1.ByName("gsed").Validity, equals, AfterPrefsMk) + local1.Trace() + + local2 := NewTools("local") + local2.defTool("gsed", "SED", false, AfterPrefsMk) + local2.Fallback(nonGnu) + + c.Check(local2.ByName("sed").Validity, equals, Nowhere) + c.Check(local2.ByName("gsed").Validity, equals, AfterPrefsMk) + local2.Trace() + + // FIXME: Must both be gsed:SED::AfterPrefsMk + c.Check(local1.ByVarname("SED").String(), equals, "sed:SED::Nowhere") + c.Check(local2.ByVarname("SED").String(), equals, "sed:SED::Nowhere") +} + +// The cmake tool is included conditionally. The condition is so simple that +// pkglint could parse it but it depends on the particular package. +// This is something that pkglint cannot do right now, since the global tools +// are loaded once for all packages. +// +// Therefore there is a workaround for USE_CMAKE. +// +// See mk/tools/cmake.mk. +func (s *Suite) Test_Tools__cmake(c *check.C) { + t := s.Init(c) + + t.SetupCommandLine("-Wall") + t.SetupPackage("category/package", + "USE_CMAKE=\tyes", + "", + "do-test:", + "\tcd ${WRKSRC} && cmake") + t.CreateFileLines("mk/tools/defaults.mk", + ".if defined(USE_CMAKE)", + "USE_TOOLS+=\tcmake cpack", + ".endif") + G.Pkgsrc.LoadInfrastructure() + + G.CheckDirent(t.File("category/package")) - c.Check(local1, deepEquals, local2) - c.Check(local4, deepEquals, local4) + t.CheckOutputEmpty() } Index: pkgsrc/pkgtools/pkglint/files/autofix.go diff -u pkgsrc/pkgtools/pkglint/files/autofix.go:1.10 pkgsrc/pkgtools/pkglint/files/autofix.go:1.11 --- pkgsrc/pkgtools/pkglint/files/autofix.go:1.10 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/autofix.go Tue Oct 9 19:12:13 2018 @@ -128,7 +128,8 @@ func (fix *Autofix) Realign(mkline MkLin { // Interpreting the continuation marker as variable value // is cheating, but works well. - m, _, _, _, _, valueAlign, value, _, _ := MatchVarassign(mkline.raw[0].orignl) + text := strings.TrimSuffix(mkline.raw[0].orignl, "\n") + m, _, _, _, _, valueAlign, value, _, _ := MatchVarassign(text) if m && value != "\\" { oldWidth = tabWidth(valueAlign) } Index: pkgsrc/pkgtools/pkglint/files/pkgsrc.go diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.10 pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.11 --- pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.10 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/pkgsrc.go Tue Oct 9 19:12:13 2018 @@ -4,6 +4,8 @@ import ( "io/ioutil" "netbsd.org/pkglint/regex" "netbsd.org/pkglint/trace" + "os" + "path/filepath" "sort" "strconv" "strings" @@ -21,7 +23,7 @@ type Pkgsrc struct { // within the bsd.pkg.mk file. buildDefs map[string]bool - Tools Tools + Tools *Tools MasterSiteURLToVar map[string]string // "https://github.com/" => "MASTER_SITE_GITHUB" MasterSiteVarToURL map[string]string // "MASTER_SITE_GITHUB" => "https://github.com/" @@ -62,13 +64,30 @@ func NewPkgsrc(dir string) *Pkgsrc { // Some user-defined variables do not influence the binary // package at all and therefore do not have to be added to // BUILD_DEFS; therefore they are marked as "already added". - src.AddBuildDefs("DISTDIR", "FETCH_CMD", "FETCH_OUTPUT_ARGS") + src.AddBuildDefs( + "DISTDIR", + "FETCH_CMD", + "FETCH_OUTPUT_ARGS", + "FETCH_USING", + "PKGSRC_RUN_TEST") + + // The following variables are used so often that not every + // package should need to add it to BUILD_DEFS manually. + src.AddBuildDefs( + "PKGSRC_COMPILER", + "PKGSRC_USE_SSP", + "UNPRIVILEGED", + "USE_CROSS_COMPILE") + + // The following variables are so obscure that they are + // probably not used in practice. + src.AddBuildDefs( + "MANINSTALL") // The following variables are added to _BUILD_DEFS by the pkgsrc // infrastructure and thus don't need to be added by the package again. // To regenerate the below list: // grep -hr '^_BUILD_DEFS+=' mk/ | tr ' \t' '\n\n' | sed -e 's,.*=,,' -e '/^_/d' -e '/^$/d' -e 's,.*,"&"\,,' | sort -u - src.AddBuildDefs("PKG_HACKS") src.AddBuildDefs( "ABI", "BUILTIN_PKGS", @@ -129,6 +148,7 @@ func (src *Pkgsrc) LoadInfrastructure() src.loadUserDefinedVars() src.loadTools() src.initDeprecatedVars() + src.loadUntypedVars() } // Latest returns the latest package matching the given pattern. @@ -221,45 +241,38 @@ func (src *Pkgsrc) loadTools() { } } - // TODO: parse bsd.prefs.mk instead of hardcoding this. - toolDefs := []struct { - Name string - Varname string + // TODO: parse bsd.prefs.mk and bsd.pkg.mk instead of hardcoding this. + toolDefs := [...]struct { + Name string + Varname string + Validity Validity }{ - {"echo", "ECHO"}, - {"echo -n", "ECHO_N"}, - {"false", "FALSE"}, - {"test", "TEST"}, - {"true", "TRUE"}} + {"echo", "ECHO", AfterPrefsMk}, + {"echo -n", "ECHO_N", AfterPrefsMk}, + {"false", "FALSE", AtRunTime}, // from bsd.pkg.mk + {"test", "TEST", AfterPrefsMk}, + {"true", "TRUE", AfterPrefsMk}} for _, toolDef := range toolDefs { - tool := tools.Define(toolDef.Name, toolDef.Varname, dummyMkLine) - tool.MustUseVarForm = true - if toolDef.Name != "false" { - tool.SetValidity(AfterPrefsMk, tools.TraceName) - } + tools.defTool(toolDef.Name, toolDef.Varname, true, toolDef.Validity) } for _, basename := range toolFiles { mklines := G.Pkgsrc.LoadMk("mk/tools/"+basename, MustSucceed|NotEmpty) - for _, mkline := range mklines.mklines { - tools.ParseToolLineCreate(mkline, true) - } + mklines.ForEach(func(mkline MkLine) { + tools.ParseToolLine(mkline, true, !mklines.indentation.IsConditional()) + }) } for _, relativeName := range [...]string{"mk/bsd.prefs.mk", "mk/bsd.pkg.mk"} { mklines := G.Pkgsrc.LoadMk(relativeName, MustSucceed|NotEmpty) - for _, mkline := range mklines.mklines { + mklines.ForEach(func(mkline MkLine) { if mkline.IsVarassign() { - switch mkline.Varname() { + varname := mkline.Varname() + switch varname { case "USE_TOOLS": - // Since this line is in the pkgsrc infrastructure, each tool mentioned - // in USE_TOOLS is trusted to be also defined somewhere in the actual - // list of available tools. - // - // This assumption does not work for processing USE_TOOLS in packages, though. - tools.ParseToolLineCreate(mkline, true) + tools.ParseToolLine(mkline, true, !mklines.indentation.IsConditional()) case "_BUILD_DEFS": for _, bdvar := range mkline.ValueSplit(mkline.Value(), "") { @@ -267,7 +280,7 @@ func (src *Pkgsrc) loadTools() { } } } - } + }) } if trace.Tracing { @@ -275,6 +288,56 @@ func (src *Pkgsrc) loadTools() { } } +// loadUntypedVars scans all pkgsrc infrastructure files in mk/ +// to find variable definitions that are not yet covered in +// Pkgsrc.InitVartypes. +// +// Even if pkglint cannot guess the type of each variable, +// at least prevent the "used but not defined" warnings. +func (src *Pkgsrc) loadUntypedVars() { + + // Setting guessed to false prevents the vartype.guessed case in MkLineChecker.CheckVaruse. + unknownType := &Vartype{lkNone, BtUnknown, []ACLEntry{{"*", aclpAll}}, false} + + handleLine := func(mkline MkLine) { + if mkline.IsVarassign() { + varcanon := mkline.Varcanon() + + switch { + case + src.vartypes[varcanon] != nil, // Already defined + src.Tools.ByVarname(varcanon) != nil, // Already known as a tool + hasPrefix(varcanon, "_"), // Skip internal variables + contains(varcanon, "$"), // Indirect or parameterized + hasSuffix(varcanon, "_MK"): // Multiple-inclusion guard + + default: + if trace.Tracing { + trace.Stepf("Untyped variable %q in %s", varcanon, mkline) + } + src.vartypes[varcanon] = unknownType + } + } + } + + handleMkFile := func(path string) { + mklines := LoadMk(path, 0) + if mklines != nil { + mklines.ForEach(handleLine) + } + } + + handleFile := func(pathName string, info os.FileInfo, err error) error { + baseName := info.Name() + if hasSuffix(baseName, ".mk") || baseName == "mk.conf" { + handleMkFile(filepath.ToSlash(pathName)) + } + return nil + } + + _ = filepath.Walk(src.File("mk"), handleFile) +} + func (src *Pkgsrc) parseSuggestedUpdates(lines []Line) []SuggestedUpdate { var updates []SuggestedUpdate state := 0 @@ -633,7 +696,7 @@ func (src *Pkgsrc) loadMasterSites() { if mkline.IsVarassign() { varname := mkline.Varname() if hasPrefix(varname, "MASTER_SITE_") && varname != "MASTER_SITE_BACKUP" { - for _, url := range splitOnSpace(mkline.Value()) { + for _, url := range fields(mkline.Value()) { if matches(url, `^(?:http://|https://|ftp://)`) { if nameToURL[varname] == "" { nameToURL[varname] = url @@ -641,6 +704,8 @@ func (src *Pkgsrc) loadMasterSites() { urlToName[url] = varname } } + // TODO: register variable type, to avoid redundant + // definitions in vardefs.go. } } } Index: pkgsrc/pkgtools/pkglint/files/buildlink3_test.go diff -u pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.18 pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.19 --- pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.18 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/buildlink3_test.go Tue Oct 9 19:12:13 2018 @@ -343,6 +343,7 @@ func (s *Suite) Test_ChecklinesBuildlink // No warning about the indentation of the .include lines. t.CheckOutputLines( + "WARN: ~/buildlink3.mk:3: VAAPI_AVAILABLE is used but not defined.", "ERROR: ~/buildlink3.mk:11: \"multimedia/libva\" does not exist.", "ERROR: ~/buildlink3.mk:11: There is no package in \"multimedia/libva\".", "ERROR: ~/buildlink3.mk:13: \"x11/libX11/buildlink3.mk\" does not exist.", Index: pkgsrc/pkgtools/pkglint/files/check_test.go diff -u pkgsrc/pkgtools/pkglint/files/check_test.go:1.26 pkgsrc/pkgtools/pkglint/files/check_test.go:1.27 --- pkgsrc/pkgtools/pkglint/files/check_test.go:1.26 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/check_test.go Tue Oct 9 19:12:13 2018 @@ -161,17 +161,8 @@ func (t *Tester) SetupOption(name, descr G.Pkgsrc.PkgOptions[name] = description } -func (t *Tester) SetupTool(name, varname string) *Tool { - tools := G.Pkgsrc.Tools - return tools.Define(name, varname, dummyMkLine) -} - -// SetupToolUsable registers a tool and immediately makes it usable, -// as if the tool were predefined globally in pkgsrc. -func (t *Tester) SetupToolUsable(name, varname string) *Tool { - tool := t.SetupTool(name, varname) - tool.SetValidity(AtRunTime, G.Pkgsrc.Tools.TraceName) - return tool +func (t *Tester) SetupTool(name, varname string, validity Validity) *Tool { + return G.Pkgsrc.Tools.defTool(name, varname, false, validity) } // SetupFileLines creates a temporary file and writes the given lines to it. @@ -247,8 +238,9 @@ func (t *Tester) SetupCategory(name stri } } -// SetupPackage sets up all files for a package so that it does not produce -// any warnings. +// SetupPackage sets up all files for a package (including the pkgsrc +// infrastructure) so that it does not produce any warnings. After calling +// this method, individual files can be overwritten as necessary. // // The given makefileLines start in line 20. Except if they are variable // definitions for already existing variables, then they replace that line. Index: pkgsrc/pkgtools/pkglint/files/shell.go diff -u pkgsrc/pkgtools/pkglint/files/shell.go:1.26 pkgsrc/pkgtools/pkglint/files/shell.go:1.27 --- pkgsrc/pkgtools/pkglint/files/shell.go:1.26 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/shell.go Tue Oct 9 19:12:13 2018 @@ -6,7 +6,6 @@ import ( "netbsd.org/pkglint/textproc" "netbsd.org/pkglint/trace" "path" - "strings" ) const ( @@ -165,7 +164,7 @@ outer: } } - if strings.TrimSpace(parser.Rest()) != "" { + if trimHspace(parser.Rest()) != "" { line.Warnf("Pkglint parse error in ShellLine.CheckWord at %q (quoting=%s), rest: %s", token, quoting, parser.Rest()) } } @@ -283,7 +282,7 @@ func (shline *ShellLine) CheckShellComma "to understand, since all the complexity of using sed and mv is", "hidden behind the scenes.", "", - "Run \"bmake help topic=subst\" for more information.") + "Run \""+confMake+" help topic=subst\" for more information.") if contains(shelltext, "#") { Explain( "When migrating to the SUBST framework, pay attention to \"#\"", @@ -304,7 +303,7 @@ func (shline *ShellLine) CheckShellComma } repl := G.NewPrefixReplacer(shelltext) - repl.AdvanceRegexp(`^\s+`) + repl.SkipHspace() if repl.AdvanceRegexp(`^[-@]+`) { shline.checkHiddenAndSuppress(repl.Group(0), repl.Rest()) } @@ -452,6 +451,8 @@ func (scc *SimpleCommandChecker) checkCo } shellword := scc.strcmd.Name + scc.shline.checkInstallCommand(shellword) + switch { case shellword == "${RUN}" || shellword == "": case scc.handleForbiddenCommand(): @@ -487,11 +488,10 @@ func (scc *SimpleCommandChecker) handleT scc.shline.mkline.Warnf("The %q tool is used but not added to USE_TOOLS.", command) } - if tool != nil && !containsVarRef(command) && tool.MustUseVarForm { + if tool != nil && tool.MustUseVarForm && !containsVarRef(command) { scc.shline.mkline.Warnf("Please use \"${%s}\" instead of %q.", tool.Varname, command) } - scc.shline.checkCommandUse(command) return tool != nil } @@ -527,12 +527,12 @@ func (scc *SimpleCommandChecker) handleC if tool.Validity == Nowhere { scc.shline.mkline.Warnf("The %q tool is used but not added to USE_TOOLS.", tool.Name) } - scc.shline.checkCommandUse(shellword) + scc.shline.checkInstallCommand(shellword) return true } if vartype := G.Pkgsrc.VariableType(varname); vartype != nil && vartype.basicType.name == "ShellCommand" { - scc.shline.checkCommandUse(shellword) + scc.shline.checkInstallCommand(shellword) return true } @@ -917,7 +917,7 @@ func (spc *ShellProgramChecker) checkSet } // Some shell commands should not be used in the install phase. -func (shline *ShellLine) checkCommandUse(shellcmd string) { +func (shline *ShellLine) checkInstallCommand(shellcmd string) { if trace.Tracing { defer trace.Call()() } Index: pkgsrc/pkgtools/pkglint/files/files.go diff -u pkgsrc/pkgtools/pkglint/files/files.go:1.19 pkgsrc/pkgtools/pkglint/files/files.go:1.20 --- pkgsrc/pkgtools/pkglint/files/files.go:1.19 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/files.go Tue Oct 9 19:12:13 2018 @@ -48,7 +48,9 @@ func Load(fileName string, options LoadO } result := convertToLogicalLines(fileName, rawText, options&Makefile != 0) - G.fileCache.Put(fileName, options, result) + if hasSuffix(fileName, ".mk") { + G.fileCache.Put(fileName, options, result) + } return result } Index: pkgsrc/pkgtools/pkglint/files/mklinechecker.go diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.19 pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.20 --- pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.19 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/mklinechecker.go Tue Oct 9 19:12:13 2018 @@ -79,7 +79,7 @@ func (ck MkLineChecker) checkInclude() { "Makefile.common.") case IsPrefs(includefile): - if path.Base(mkline.Filename) == "buildlink3.mk" && includefile == "../../mk/bsd.prefs.mk" { + if mkline.Basename == "buildlink3.mk" && includefile == "../../mk/bsd.prefs.mk" { mkline.Notef("For efficiency reasons, please include bsd.fast.prefs.mk instead of bsd.prefs.mk.") } @@ -112,7 +112,11 @@ func (ck MkLineChecker) checkDirective(f needsArgument := false switch directive { - case "if", "ifdef", "ifndef", "elif", "for", "undef": + case + "if", "ifdef", "ifndef", "elif", + "for", "undef", + "error", "warning", "info", + "export", "export-env", "unexport", "unexport-env": needsArgument = true } @@ -136,8 +140,8 @@ func (ck MkLineChecker) checkDirective(f } else if directive == "for" { ck.checkDirectiveFor(forVars, ind) - } else if directive == "undef" && args != "" { - for _, varname := range splitOnSpace(args) { + } else if directive == "undef" { + for _, varname := range fields(args) { if forVars[varname] { mkline.Notef("Using \".undef\" after a \".for\" loop is unnecessary.") } @@ -170,7 +174,7 @@ func (ck MkLineChecker) checkDirectiveFo args := mkline.Args() if m, vars, values := match2(args, `^(\S+(?:\s*\S+)*?)\s+in\s+(.*)$`); m { - for _, forvar := range splitOnSpace(vars) { + for _, forvar := range fields(vars) { indentation.AddVar(forvar) if !G.Infrastructure && hasPrefix(forvar, "_") { mkline.Warnf("Variable names starting with an underscore (%s) are reserved for internal pkgsrc use.", forvar) @@ -189,7 +193,7 @@ func (ck MkLineChecker) checkDirectiveFo // Check if any of the value's types is not guessed. guessed := true - for _, value := range splitOnSpace(values) { + for _, value := range fields(values) { if m, vname := match1(value, `^\$\{(.*)\}`); m { vartype := G.Pkgsrc.VariableType(vname) if vartype != nil && !vartype.guessed { @@ -200,7 +204,7 @@ func (ck MkLineChecker) checkDirectiveFo forLoopType := &Vartype{lkSpace, BtUnknown, []ACLEntry{{"*", aclpAllRead}}, guessed} forLoopContext := &VarUseContext{forLoopType, vucTimeParse, vucQuotFor, false} - for _, forLoopVar := range mkline.ExtractUsedVariables(values) { + for _, forLoopVar := range mkline.DetermineUsedVariables() { ck.CheckVaruse(&MkVarUse{forLoopVar, nil}, forLoopContext) } } @@ -222,8 +226,8 @@ func (ck MkLineChecker) checkDirectiveIn func (ck MkLineChecker) checkDependencyRule(allowedTargets map[string]bool) { mkline := ck.MkLine - targets := splitOnSpace(mkline.Targets()) - sources := splitOnSpace(mkline.Sources()) + targets := fields(mkline.Targets()) + sources := fields(mkline.Sources()) for _, source := range sources { if source == ".PHONY" { @@ -283,7 +287,7 @@ func (ck MkLineChecker) checkVarassignPe return } - perms := vartype.EffectivePermissions(mkline.Filename) + perms := vartype.EffectivePermissions(mkline.Basename) // E.g. USE_TOOLS:= ${USE_TOOLS:Nunwanted-tool} if op == opAssignEval && perms&aclpAppend != 0 { @@ -351,14 +355,15 @@ func (ck MkLineChecker) CheckVaruse(varu case !G.opts.WarnExtra: case vartype != nil && !vartype.guessed: // Well-known variables are probably defined by the infrastructure. - case varIsUsed(varname): + case varIsDefinedSimilar(varname): case containsVarRef(varname): + case G.Mk != nil && !G.Mk.FirstTime("used but not defined: "+varname): default: mkline.Warnf("%s is used but not defined.", varname) } if hasPrefix(varuse.Mod(), ":=") && vartype != nil && !vartype.IsConsideredList() { - mkline.Warnf("The :from=to modifier should only be used with lists.") + mkline.Warnf("The :from=to modifier should only be used with lists, not with %s.", varuse.varname) Explain( "Instead of:", "\tMASTER_SITES=\t${HOMEPAGE:=repository/}", @@ -423,37 +428,37 @@ func (ck MkLineChecker) checkVarusePermi } mkline := ck.MkLine - perms := vartype.EffectivePermissions(mkline.Filename) + perms := vartype.EffectivePermissions(mkline.Basename) - isLoadTime := false // Will the variable be used at load time? + warnLoadTime := false // Will the variable be used at load time? // Might the variable be used indirectly at load time, for example // by assigning it to another variable which then gets evaluated? - isIndirect := false + warnIndirect := false switch { case vuc.vartype != nil && vuc.vartype.guessed: - // Don't warn about unknown variables. + // Don't warn about unknown variables. case vuc.time == vucTimeParse && !perms.Contains(aclpUseLoadtime): - isLoadTime = true + warnLoadTime = true case vuc.vartype != nil && vuc.vartype.Union().Contains(aclpUseLoadtime) && !perms.Contains(aclpUseLoadtime): - isLoadTime = true - isIndirect = true + warnLoadTime = true + warnIndirect = true } - if isLoadTime { + if warnLoadTime { if tool := G.ToolByVarname(varname, LoadTime); tool != nil { ck.checkVaruseToolLoadTime(varname, tool) } else { - ck.checkVaruseLoadTime(varname, isIndirect) + ck.warnVaruseLoadTime(varname, warnIndirect) } } if !perms.Contains(aclpUseLoadtime) && !perms.Contains(aclpUse) { needed := aclpUse - if isLoadTime { + if warnLoadTime { needed = aclpUseLoadtime } alternativeFiles := vartype.AllowedFiles(needed) @@ -482,7 +487,7 @@ func (ck MkLineChecker) checkVaruseToolL return } - if path.Base(ck.MkLine.Filename) == "Makefile" { + if ck.MkLine.Basename == "Makefile" { pkgsrcTool := G.Pkgsrc.Tools.ByName(tool.Name) if pkgsrcTool != nil && pkgsrcTool.Validity == Nowhere { // The tool must have been added too late to USE_TOOLS, @@ -506,7 +511,7 @@ func (ck MkLineChecker) checkVaruseToolL "only be used at run time, except in the package Makefile itself.") } -func (ck MkLineChecker) checkVaruseLoadTime(varname string, isIndirect bool) { +func (ck MkLineChecker) warnVaruseLoadTime(varname string, isIndirect bool) { mkline := ck.MkLine if !isIndirect { @@ -646,7 +651,7 @@ func (ck MkLineChecker) checkVarassignPy } mkline := ck.MkLine - strversions := splitOnSpace(value) + strversions := fields(value) intversions := make([]int, len(strversions)) for i, strversion := range strversions { iver, err := strconv.Atoi(strversion) @@ -692,11 +697,13 @@ func (ck MkLineChecker) checkVarassign() trace.Step1("%s might be unused unless it is an argument to a procedure file.", varname) } - } else if !varIsUsed(varname) { + } else if !varIsUsedSimilar(varname) { if vartypes := G.Pkgsrc.vartypes; vartypes[varname] != nil || vartypes[varcanon] != nil { // Ok } else if deprecated := G.Pkgsrc.Deprecated; deprecated[varname] != "" || deprecated[varcanon] != "" { // Ok + } else if G.Mk != nil && !G.Mk.FirstTime("defined but not used: "+varname) { + // Skip } else { mkline.Warnf("%s is defined but not used.", varname) } @@ -870,7 +877,7 @@ func (ck MkLineChecker) CheckVartype(var if op == opAssignAppend { if vartype != nil && !vartype.MayBeAppendedTo() { - mkline.Warnf("The \"+=\" operator should only be used with lists.") + mkline.Warnf("The \"+=\" operator should only be used with lists, not with %s.", varname) } } @@ -892,7 +899,7 @@ func (ck MkLineChecker) CheckVartype(var break case vartype.kindOfList == lkSpace: - for _, word := range splitOnSpace(value) { + for _, word := range fields(value) { ck.CheckVartypePrimitive(varname, vartype.basicType, op, word, comment, vartype.guessed) } @@ -971,7 +978,7 @@ func (ck MkLineChecker) checkDirectiveCo defer trace.Call1(mkline.Args())() } - p := NewMkParser(mkline.Line, mkline.Args(), false) + p := NewMkParser(mkline.Line, mkline.Args(), false) // XXX: Why false? cond := p.MkCond() if !p.EOF() { mkline.Warnf("Invalid condition, unrecognized part: %q.", p.Rest()) @@ -1028,13 +1035,16 @@ func (ck MkLineChecker) checkDirectiveCo } } + checkVarUse := func(varuse *MkVarUse) { + var vartype *Vartype // TODO: Insert a better type guess here. + vuc := &VarUseContext{vartype, vucTimeParse, vucQuotPlain, false} + ck.CheckVaruse(varuse, vuc) + } + NewMkCondWalker().Walk(cond, &MkCondCallback{ Empty: checkEmpty, - CompareVarStr: checkCompareVarStr}) - - if G.Mk != nil { - G.Mk.indentation.RememberUsedVariables(cond) - } + CompareVarStr: checkCompareVarStr, + VarUse: checkVarUse}) } func (ck MkLineChecker) checkCompareVarStr(varname, op, value string) { Index: pkgsrc/pkgtools/pkglint/files/mkline.go diff -u pkgsrc/pkgtools/pkglint/files/mkline.go:1.38 pkgsrc/pkgtools/pkglint/files/mkline.go:1.39 --- pkgsrc/pkgtools/pkglint/files/mkline.go:1.38 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/mkline.go Tue Oct 9 19:12:13 2018 @@ -19,7 +19,8 @@ type MkLineImpl struct { Line data interface{} // One of the following mkLine* types } -type mkLineAssign struct { +type mkLineAssign = *mkLineAssignImpl +type mkLineAssignImpl struct { commented bool // Whether the whole variable assignment is commented out varname string // e.g. "HOMEPAGE", "SUBST_SED.perl" varcanon string // e.g. "HOMEPAGE", "SUBST_SED.*" @@ -34,14 +35,17 @@ type mkLineShell struct { } type mkLineComment struct{} type mkLineEmpty struct{} -type mkLineDirective struct { +type mkLineDirective = *mkLineDirectiveImpl +type mkLineDirectiveImpl struct { indent string directive string args string comment string elseLine MkLine // (filled in later) + cond MkCond // (filled in later, as needed) } -type mkLineInclude struct { +type mkLineInclude = *mkLineIncludeImpl +type mkLineIncludeImpl struct { mustExist bool sys bool indent string @@ -56,7 +60,7 @@ type mkLineDependency struct { func NewMkLine(line Line) *MkLineImpl { text := line.Text - if hasPrefix(text, " ") { + if hasPrefix(text, " ") && line.Basename != "bsd.buildlink3.mk" { line.Warnf("Makefile lines should not start with space characters.") Explain( "If you want this line to contain a shell program, use a tab", @@ -89,7 +93,7 @@ func NewMkLine(line Line) *MkLineImpl { value = strings.Replace(value, "\\#", "#", -1) varparam := varnameParam(varname) - return &MkLineImpl{line, mkLineAssign{ + return &MkLineImpl{line, &mkLineAssignImpl{ commented, varname, varnameCanon(varname), @@ -105,7 +109,7 @@ func NewMkLine(line Line) *MkLineImpl { return &MkLineImpl{line, mkLineShell{shellcmd}} } - trimmedText := strings.TrimSpace(text) + trimmedText := trimHspace(text) if strings.HasPrefix(trimmedText, "#") { return &MkLineImpl{line, mkLineComment{}} } @@ -115,15 +119,15 @@ func NewMkLine(line Line) *MkLineImpl { } if m, indent, directive, args, comment := matchMkDirective(text); m { - return &MkLineImpl{line, mkLineDirective{indent, directive, args, comment, nil}} + return &MkLineImpl{line, &mkLineDirectiveImpl{indent, directive, args, comment, nil, nil}} } if m, indent, directive, includefile := MatchMkInclude(text); m { - return &MkLineImpl{line, mkLineInclude{directive == "include", false, indent, includefile, ""}} + return &MkLineImpl{line, &mkLineIncludeImpl{directive == "include", false, indent, includefile, ""}} } if m, indent, directive, includefile := match3(text, `^\.(\s*)(s?include)\s+<([^>]+)>\s*(?:#.*)?$`); m { - return &MkLineImpl{line, mkLineInclude{directive == "include", true, indent, includefile, ""}} + return &MkLineImpl{line, &mkLineIncludeImpl{directive == "include", true, indent, includefile, ""}} } if m, targets, whitespace, sources := match3(text, `^([^\s:]+(?:\s*[^\s:]+)*)(\s*):\s*([^#]*?)(?:\s*#.*)?$`); m { @@ -247,6 +251,19 @@ func (mkline *MkLineImpl) Directive() st // Args returns the arguments from an .if, .ifdef, .ifndef, .elif, .for, .undef. func (mkline *MkLineImpl) Args() string { return mkline.data.(mkLineDirective).args } +// Cond applies to an .if or .elif line and returns the parsed condition. +// +// If a parse error occurs, it is silently swallowed, returning a +// best-effort part of the condition, or even nil. +func (mkline *MkLineImpl) Cond() MkCond { + cond := mkline.data.(mkLineDirective).cond + if cond == nil { + cond = NewMkParser(mkline.Line, mkline.Args(), false).MkCond() + mkline.data.(mkLineDirective).cond = cond + } + return cond +} + // DirectiveComment is the trailing end-of-line comment, typically at a deeply nested .endif or .endfor. func (mkline *MkLineImpl) DirectiveComment() string { return mkline.data.(mkLineDirective).comment } func (mkline *MkLineImpl) HasElseBranch() bool { return mkline.data.(mkLineDirective).elseLine != nil } @@ -262,9 +279,9 @@ func (mkline *MkLineImpl) IncludeFile() func (mkline *MkLineImpl) Targets() string { return mkline.data.(mkLineDependency).targets } func (mkline *MkLineImpl) Sources() string { return mkline.data.(mkLineDependency).sources } -// ConditionalVars is a space-separated list of those variable names -// on which the inclusion depends. It is initialized later, -// step by step, when parsing other lines +// ConditionalVars applies to .include lines and is a space-separated +// list of those variable names on which the inclusion depends. +// It is initialized later, step by step, when parsing other lines. func (mkline *MkLineImpl) ConditionalVars() string { return mkline.data.(mkLineInclude).conditionalVars } func (mkline *MkLineImpl) SetConditionalVars(varnames string) { include := mkline.data.(mkLineInclude) @@ -306,7 +323,7 @@ func (mkline *MkLineImpl) ValueSplit(val if token.Varuse == nil && contains(token.Text, separator) { var subs []string if separator == "" { - subs = splitOnSpace(token.Text) + subs = fields(token.Text) } else { subs = strings.Split(token.Text, separator) } @@ -324,15 +341,13 @@ func (mkline *MkLineImpl) ValueTokens() } func (mkline *MkLineImpl) WithoutMakeVariables(value string) string { - valueNovar := value - for { - // TODO: properly parse nested variables - replaced := replaceFirst(valueNovar, `\$\{[^{}]*\}`, "") - if replaced == valueNovar { - return valueNovar + valueNovar := "" + for _, token := range NewMkParser(nil, value, false).MkTokens() { + if token.Varuse == nil { + valueNovar += token.Text } - valueNovar = replaced } + return valueNovar } func (mkline *MkLineImpl) ResolveVarsInRelativePath(relativePath string, adjustDepth bool) string { @@ -386,21 +401,8 @@ func (mkline *MkLineImpl) ResolveVarsInR func (ind *Indentation) RememberUsedVariables(cond MkCond) { NewMkCondWalker().Walk(cond, &MkCondCallback{ - Defined: func(varname string) { - ind.AddVar(varname) - }, - Empty: func(varuse *MkVarUse) { - ind.AddVar(varuse.varname) - }, - CompareVarNum: func(varuse *MkVarUse, op string, num string) { - ind.AddVar(varuse.varname) - }, - CompareVarStr: func(varuse *MkVarUse, op string, str string) { + VarUse: func(varuse *MkVarUse) { ind.AddVar(varuse.varname) - }, - CompareVarVar: func(left *MkVarUse, op string, right *MkVarUse) { - ind.AddVar(left.varname) - ind.AddVar(right.varname) }}) } @@ -426,15 +428,20 @@ func matchMkDirective(text string) (m bo indentEnd := i directiveStart := i - for i < n && 'a' <= text[i] && text[i] <= 'z' { + for i < n && ('a' <= text[i] && text[i] <= 'z' || text[i] == '-') { i++ } directiveEnd := i directive = text[directiveStart:directiveEnd] switch directive { - case "if", "ifdef", "ifndef", "else", "elif", "endif", "for", "endfor", "undef": + case "if", "else", "elif", "endif", + "ifdef", "ifndef", + "for", "endfor", "undef", + "error", "warning", "info", + "export", "export-env", "unexport", "unexport-env": break default: + // Intentionally not supported are: ifmake ifnmake elifdef elifndef elifmake elifnmake. return } @@ -483,7 +490,7 @@ func (mkline *MkLineImpl) VariableNeedsQ defer trace.Call(varname, vartype, vuc, trace.Result(&needsQuoting))() } - if vartype == nil || vuc.vartype == nil { + if vartype == nil || vuc.vartype == nil || vartype.basicType == BtUnknown { return nqDontKnow } @@ -581,67 +588,63 @@ func (mkline *MkLineImpl) VariableNeedsQ return nqDontKnow } -// TODO: merge with determineUsedVariables -func (mkline *MkLineImpl) ExtractUsedVariables(text string) []string { - re := G.res.Compile(`^(?:[^\$]+|\$[\$*<>?@]|\$\{([.0-9A-Z_a-z]+)(?::(?:[^\${}]|\$[^{])+)?\})`) - rest := text - var result []string - for { - m := re.FindStringSubmatchIndex(rest) - if m == nil { - break - } - varname := rest[negToZero(m[2]):negToZero(m[3])] - rest = rest[:m[0]] + rest[m[1]:] - if varname != "" { - result = append(result, varname) - } - } +func (mkline *MkLineImpl) DetermineUsedVariables() []string { + var varnames []string - if trace.Tracing && rest != "" { - trace.Step1("extractUsedVariables: rest=%q", rest) + add := func(varname string) { + varnames = append(varnames, varname) } - return result -} -func (mkline *MkLineImpl) DetermineUsedVariables() (varnames []string) { - rest := mkline.Text + var searchIn func(text string) - if strings.HasPrefix(rest, "#") { - return + searchInVarUse := func(varuse *MkVarUse) { + varname := varuse.varname + if !varuse.IsExpression() { + add(varname) + } + searchIn(varname) + for _, mod := range varuse.modifiers { + searchIn(mod) + } } - for { - p1 := strings.Index(rest, "${") - p2 := strings.Index(rest, "$(") - p3 := strings.Index(rest, "defined(") - p4 := strings.Index(rest, "empty(") - if p1 == -1 && p2 == -1 && p3 == -1 && p4 == -1 { + searchIn = func(text string) { + if !contains(text, "$") { return } - min := -1 - if min == -1 || (p1 != -1 && p1 < min) { - min = p1 - } - if min == -1 || (p2 != -1 && p2 < min) { - min = p2 - } - if min == -1 || (p3 != -1 && p3 < min) { - min = p3 - } - if min == -1 || (p4 != -1 && p4 < min) { - min = p4 - } - rest = rest[min:] - m := G.res.Compile(`(?:\$\{|\$\(|defined\(|empty\()([*+\-.0-9A-Z_a-z]+)[:})]`).FindStringSubmatchIndex(rest) - if m == nil { - return + for _, token := range NewMkParser(nil, text, false).MkTokens() { + if token.Varuse != nil { + searchInVarUse(token.Varuse) + } } - varname := rest[m[2]:m[3]] - varnames = append(varnames, varname) - rest = rest[:m[0]] + rest[m[1]:] } + + switch { + + case mkline.IsVarassign(): + searchIn(mkline.Value()) + + case mkline.IsDirective() && mkline.Directive() == "for": + searchIn(mkline.Args()) + + case mkline.IsDirective() && mkline.Cond() != nil: + NewMkCondWalker().Walk( + mkline.Cond(), + &MkCondCallback{VarUse: searchInVarUse}) + + case mkline.IsShellCommand(): + searchIn(mkline.ShellCommand()) + + case mkline.IsDependency(): + searchIn(mkline.Targets()) + searchIn(mkline.Sources()) + + case mkline.IsInclude(): + searchIn(mkline.IncludeFile()) + } + + return varnames } type MkOperator uint8 @@ -775,7 +778,7 @@ func (ind *Indentation) String() string s += " (" + strings.Join(level.conditionalVars, " ") + ")" } } - return "[" + strings.TrimSpace(s) + "]" + return "[" + trimHspace(s) + "]" } type indentationLevel struct { @@ -897,12 +900,9 @@ func (ind *Indentation) TrackBefore(mkli trace.Stepf("Indentation before line %s: %s", mkline.Linenos(), ind) } - directive := mkline.Directive() - args := mkline.Args() - - switch directive { + switch mkline.Directive() { case "for", "if", "ifdef", "ifndef": - ind.Push(mkline, ind.top().depth, args) + ind.Push(mkline, ind.top().depth, mkline.Args()) } } @@ -918,32 +918,26 @@ func (ind *Indentation) TrackAfter(mklin case "if": // For multiple-inclusion guards, the indentation stays at the same level. guard := false - if hasPrefix(args, "!defined") && hasSuffix(args, "_MK)") { - if hasPrefix(args, "!defined(") && hasSuffix(args, ")") { - varname := args[9 : len(args)-1] - if varname != "" && isalnum(varname) { - ind.AddVar(varname) - guard = true - } + if hasPrefix(args, "!defined(") && hasSuffix(args, "_MK)") { + varname := args[9 : len(args)-1] + if varname != "" && isalnum(varname) { + ind.AddVar(varname) + guard = true } } if !guard { ind.top().depth += 2 } - // Note: adding the used variables for arbitrary conditions - // happens in MkLineChecker.checkDirectiveCond for performance reasons. + if cond := mkline.Cond(); cond != nil { + ind.RememberUsedVariables(cond) - if contains(args, "exists") { - cond := NewMkParser(mkline.Line, args, false).MkCond() - if cond != nil { - NewMkCondWalker().Walk(cond, &MkCondCallback{ - Call: func(name string, arg string) { - if name == "exists" { - ind.AddCheckedFile(arg) - } - }}) - } + NewMkCondWalker().Walk(cond, &MkCondCallback{ + Call: func(name string, arg string) { + if name == "exists" { + ind.AddCheckedFile(arg) + } + }}) } case "for", "ifdef", "ifndef": @@ -986,13 +980,26 @@ func MatchVarassign(text string) (m, com for ; i < n; i++ { b := text[i] switch { + + // As of go1.11.1 (October 2018), the Go compiler doesn't emit good + // code for these kinds of comparisons. + // See https://github.com/golang/go/issues/17372. case 'A' <= b && b <= 'Z', 'a' <= b && b <= 'z', b == '_', '0' <= b && b <= '9', - '$' <= b && b <= '.' && (b == '$' || b == '*' || b == '+' || b == '-' || b == '.'), - b == '[', - b == '{', b == '}': + '*' <= b && b <= '.' && (b == '*' || b == '+' || b == '-' || b == '.'), + b == '[': // For the tool of the same name, e.g. "TOOLS_PATH.[". + continue + + case b == '$': + parser := NewMkParser(nil, text[i:], false) + varuse := parser.VarUse() + if varuse == nil { + return + } + varuseLen := len(text[i:]) - len(parser.Rest()) + i += varuseLen - 1 continue } break @@ -1055,7 +1062,7 @@ func MatchVarassign(text string) (m, com spaceAfterVarname = text[varnameEnd:opStart] op = text[opStart:opEnd] valueAlign = text[0:valueStart] - value = strings.TrimSpace(string(valuebuf[:j])) + value = trimHspace(string(valuebuf[:j])) spaceAfterValue = text[valueEnd:commentStart] comment = text[commentStart:commentEnd] return @@ -1069,12 +1076,12 @@ func MatchMkInclude(text string) (m bool } if repl.AdvanceStr("include") || repl.AdvanceStr("sinclude") { directive = repl.Str() - repl.AdvanceHspace() + repl.SkipHspace() if repl.AdvanceByte('"') { if repl.AdvanceBytesFunc(func(c byte) bool { return c != '"' }) { filename = repl.Str() if repl.AdvanceByte('"') { - repl.AdvanceHspace() + repl.SkipHspace() if repl.EOF() || repl.PeekByte() == '#' { m = true return Index: pkgsrc/pkgtools/pkglint/files/pkglint.go diff -u pkgsrc/pkgtools/pkglint/files/pkglint.go:1.38 pkgsrc/pkgtools/pkglint/files/pkglint.go:1.39 --- pkgsrc/pkgtools/pkglint/files/pkglint.go:1.38 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/pkglint.go Tue Oct 9 19:12:13 2018 @@ -59,7 +59,7 @@ type Pkglint struct { func NewPkglint() Pkglint { return Pkglint{ res: regex.NewRegistry(), - fileCache: NewFileCache(100)} + fileCache: NewFileCache(200)} } type CmdOpts struct { @@ -546,7 +546,7 @@ func (pkglint *Pkglint) Checkfile(fname return } - pkglint.checkExecutable(st) + pkglint.checkExecutable(fname, st) pkglint.checkMode(fname, st.Mode()) } @@ -575,7 +575,11 @@ func (pkglint *Pkglint) checkMode(fname case basename == "ALTERNATIVES": if pkglint.opts.CheckAlternatives { - CheckfileAlternatives(fname, nil) + var plistFiles map[string]bool + if G.Pkg != nil { + plistFiles = G.Pkg.PlistFiles + } + CheckfileAlternatives(fname, plistFiles) } case basename == "buildlink3.mk": @@ -665,9 +669,20 @@ func (pkglint *Pkglint) checkMode(fname } } -func (pkglint *Pkglint) checkExecutable(st os.FileInfo) { - fname := st.Name() - if st.Mode().IsRegular() && st.Mode().Perm()&0111 != 0 && !isCommitted(fname) { +func (pkglint *Pkglint) checkExecutable(fname string, st os.FileInfo) { + switch { + case !st.Mode().IsRegular(): + // Directories and other entries may be executable. + + case st.Mode().Perm()&0111 == 0: + // Good. + + case isCommitted(fname): + // Too late to be fixed by the package developer, since + // CVS remembers the executable bit in the repo file. + // At this point, it can only be reset by the CVS admins. + + default: line := NewLine(fname, 0, "", nil) fix := line.Autofix() fix.Warnf("Should not be executable.") @@ -709,33 +724,13 @@ func (pkglint *Pkglint) Tool(command str varname = toolVarname } - if G.Mk != nil { - tools := G.Mk.Tools - if t := tools.ByName(command); t != nil { - if tools.Usable(t, time) { - return t, true - } - tool = t - } - - if t := tools.ByVarname(varname); t != nil { - if tools.Usable(t, time) { - return t, true - } - if tool == nil { - tool = t - } - } - } + tools := pkglint.tools() - tools := G.Pkgsrc.Tools if t := tools.ByName(command); t != nil { if tools.Usable(t, time) { return t, true } - if tool == nil { - tool = t - } + tool = t } if t := tools.ByVarname(varname); t != nil { @@ -746,7 +741,6 @@ func (pkglint *Pkglint) Tool(command str tool = t } } - return } @@ -756,27 +750,13 @@ func (pkglint *Pkglint) Tool(command str // current package. It is not guaranteed to be usable; that must be // checked by the calling code. func (pkglint *Pkglint) ToolByVarname(varname string, time ToolTime) *Tool { + return pkglint.tools().ByVarname(varname) +} - var tool *Tool +func (pkglint *Pkglint) tools() *Tools { if G.Mk != nil { - tools := G.Mk.Tools - if t := tools.ByVarname(varname); t != nil { - if tools.Usable(t, time) { - return t - } - tool = t - } - } - - tools := G.Pkgsrc.Tools - if t := tools.ByVarname(varname); t != nil { - if tools.Usable(t, time) { - return t - } - if tool == nil { - tool = t - } + return G.Mk.Tools + } else { + return G.Pkgsrc.Tools } - - return tool } Index: pkgsrc/pkgtools/pkglint/files/mkline_test.go diff -u pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.42 pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.43 --- pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.42 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/mkline_test.go Tue Oct 9 19:12:13 2018 @@ -218,6 +218,18 @@ func (s *Suite) Test_NewMkLine__autofix_ "pkgbase := pkglint") } +func (s *Suite) Test_MkLine_Cond(c *check.C) { + t := s.Init(c) + + mkline := t.NewMkLine("Makefile", 2, ".if ${VAR} == Value") + + cond := mkline.Cond() + + c.Check(cond.CompareVarStr.Var.varname, equals, "VAR") + c.Check(cond.CompareVarStr.Str, equals, "Value") + c.Check(mkline.Cond(), equals, cond) +} + // Guessing the variable type works for both plain and parameterized variable names. func (s *Suite) Test_Pkgsrc_VariableType__varparam(c *check.C) { t := s.Init(c) @@ -277,11 +289,48 @@ func (s *Suite) Test_NewMkLine__leading_ t := s.Init(c) _ = t.NewMkLine("rubyversion.mk", 427, " _RUBYVER=\t2.15") + _ = t.NewMkLine("bsd.buildlink3.mk", 132, " ok:=yes") + // In mk/buildlink3/bsd.buildlink3.mk, the leading space is really helpful, + // therefore no warnings for that file. t.CheckOutputLines( "WARN: rubyversion.mk:427: Makefile lines should not start with space characters.") } +// Exotic test cases from the pkgsrc infrastructure. +// Hopefully, pkgsrc packages don't need such complicated code. +func (s *Suite) Test_NewMkLine__infrastructure(c *check.C) { + t := s.Init(c) + + mklines := t.NewMkLines("infra.mk", + MkRcsID, + " USE_BUILTIN.${_pkg_:S/^-//}:=no", + ".error \"Something went wrong\"", + ".export WRKDIR", + ".export", + ".unexport-env WRKDIR", + "", + ".ifmake target1", // Luckily, this is not used in the wild. + ".elifnmake target2", // Neither is this. + ".endif") + + c.Check(mklines.mklines[1].Varcanon(), equals, "USE_BUILTIN.*") + c.Check(mklines.mklines[2].Directive(), equals, "error") + c.Check(mklines.mklines[3].Directive(), equals, "export") + + t.CheckOutputLines( + "WARN: infra.mk:2: Makefile lines should not start with space characters.", + "ERROR: infra.mk:8: Unknown Makefile line format: \".ifmake target1\".", + "ERROR: infra.mk:9: Unknown Makefile line format: \".elifnmake target2\".") + + mklines.Check() + + t.CheckOutputLines( + "WARN: infra.mk:2: USE_BUILTIN.${_pkg_:S/^-//} is defined but not used.", + "ERROR: infra.mk:5: \".export\" requires arguments.", + "ERROR: infra.mk:10: Unmatched .endif.") +} + func (s *Suite) Test_MkLines_Check__extra(c *check.C) { t := s.Init(c) @@ -387,8 +436,8 @@ func (s *Suite) Test_MkLine_VariableNeed t.SetupCommandLine("-Wall") t.SetupVartypes() - t.SetupToolUsable("find", "FIND") - t.SetupToolUsable("sort", "SORT") + t.SetupTool("find", "FIND", AtRunTime) + t.SetupTool("sort", "SORT", AtRunTime) G.Pkg = NewPackage(t.File("category/pkgbase")) G.Mk = t.NewMkLines("Makefile", MkRcsID, @@ -425,8 +474,8 @@ func (s *Suite) Test_MkLine_VariableNeed t := s.Init(c) t.SetupCommandLine("-Wall") - t.SetupToolUsable("perl", "PERL5") - t.SetupToolUsable("bash", "BASH") + t.SetupTool("perl", "PERL5", AtRunTime) + t.SetupTool("bash", "BASH", AtRunTime) t.SetupVartypes() mklines := t.NewMkLines("Makefile", MkRcsID, @@ -465,8 +514,8 @@ func (s *Suite) Test_MkLine_VariableNeed t.SetupCommandLine("-Wall") t.SetupVartypes() - t.SetupToolUsable("awk", "AWK") - t.SetupToolUsable("echo", "ECHO") + t.SetupTool("awk", "AWK", AtRunTime) + t.SetupTool("echo", "ECHO", AtRunTime) G.Mk = t.NewMkLines("xpi.mk", MkRcsID, "\t id=$$(${AWK} '{print}' < ${WRKSRC}/idfile) && echo \"$$id\"", @@ -546,8 +595,8 @@ func (s *Suite) Test_MkLine_VariableNeed t := s.Init(c) t.SetupCommandLine("-Wall") - t.SetupToolUsable("echo", "ECHO") - t.SetupToolUsable("sh", "SH") + t.SetupTool("echo", "ECHO", AtRunTime) + t.SetupTool("sh", "SH", AtRunTime) t.SetupVartypes() G.Mk = t.NewMkLines("x11/labltk/Makefile", MkRcsID, @@ -615,7 +664,9 @@ func (s *Suite) Test_MkLine_VariableNeed G.Mk.Check() - t.CheckOutputEmpty() // Don't warn about missing :Q modifiers. + // Don't warn about missing :Q modifiers. + t.CheckOutputLines( + "WARN: x11/eterm/Makefile:2: PIXMAP_FILES is used but not defined.") } func (s *Suite) Test_MkLine_VariableNeedsQuoting__PKGNAME_and_URL_list_in_URL_list(c *check.C) { @@ -638,7 +689,7 @@ func (s *Suite) Test_MkLine_VariableNeed t.SetupCommandLine("-Wall") t.SetupVartypes() - t.SetupToolUsable("tar", "TAR") + t.SetupTool("tar", "TAR", AtRunTime) mklines := t.NewMkLines("Makefile", MkRcsID, "", @@ -659,7 +710,7 @@ func (s *Suite) Test_MkLine_VariableNeed t.SetupCommandLine("-Wall") t.SetupVartypes() - t.SetupToolUsable("cat", "CAT") + t.SetupTool("cat", "CAT", AtRunTime) mklines := t.NewMkLines("Makefile", MkRcsID, "", @@ -734,7 +785,7 @@ func (s *Suite) Test_MkLine_VariableNeed t.SetupCommandLine("-Wall,no-space") t.SetupVartypes() - t.SetupToolUsable("bash", "BASH") + t.SetupTool("bash", "BASH", AtRunTime) mklines := t.SetupFileMkLines("Makefile", MkRcsID, @@ -767,7 +818,10 @@ func (s *Suite) Test_MkLine_VariableNeed t.CheckOutputLines( "WARN: ~/Makefile:4: The variable LINKER_RPATH_FLAG may not be set by any package.", "WARN: ~/Makefile:4: Please use ${LINKER_RPATH_FLAG:S/-rpath/& /:Q} instead of ${LINKER_RPATH_FLAG:S/-rpath/& /}.", - "WARN: ~/Makefile:4: LINKER_RPATH_FLAG should not be evaluated at load time.") + "WARN: ~/Makefile:4: LINKER_RPATH_FLAG should not be evaluated at load time.", + "WARN: ~/Makefile:6: The variable PATH may not be set by any package.", + "WARN: ~/Makefile:6: PREFIX should not be evaluated at load time.", + "WARN: ~/Makefile:6: PATH should not be evaluated at load time.") } func (s *Suite) Test_ShellLine_CheckWord__PKGMANDIR(c *check.C) { @@ -800,8 +854,9 @@ func (s *Suite) Test_MkLines_Check__VERS mklines.Check() t.CheckOutputLines( - "WARN: geography/viking/Makefile:2: " + - "The list variable MASTER_SITE_SOURCEFORGE should not be embedded in a word.") + "WARN: geography/viking/Makefile:2: "+ + "The list variable MASTER_SITE_SOURCEFORGE should not be embedded in a word.", + "WARN: geography/viking/Makefile:2: VERSION is used but not defined.") } func (s *Suite) Test_MkLines_Check__shell_command_as_wordpart_in_ENV_list(c *check.C) { @@ -825,6 +880,7 @@ func (s *Suite) Test_MkLine__shell_varus t.SetupCommandLine("-Wall") t.SetupVartypes() + t.SetupTool("grep", "GREP", AtRunTime) mklines := t.NewMkLines("x11/motif/Makefile", MkRcsID, "post-patch:", @@ -833,8 +889,7 @@ func (s *Suite) Test_MkLine__shell_varus mklines.Check() // Just ensure that there are no parse errors. - t.CheckOutputLines( - "WARN: x11/motif/Makefile:3: Unknown shell command \"${GREP}\".") + t.CheckOutputEmpty() } // See PR 46570, Ctrl+F "3. In lang/perl5". @@ -1050,25 +1105,57 @@ func (s *Suite) Test_Indentation_Remembe mkline := t.NewMkLine("Makefile", 123, ".if ${PKGREVISION} > 0") ind := NewIndentation() - cond := NewMkParser(mkline.Line, mkline.Args(), false) - ind.RememberUsedVariables(cond.MkCond()) + ind.RememberUsedVariables(mkline.Cond()) t.CheckOutputEmpty() c.Check(ind.Varnames(), equals, "PKGREVISION") } -func (s *Suite) Test_MkLine_ExtractUsedVariables(c *check.C) { +func (s *Suite) Test_MkLine_DetermineUsedVariables(c *check.C) { t := s.Init(c) - mkline := t.NewMkLine("Makefile", 123, "") - - nestedVarnames := mkline.ExtractUsedVariables("${VAR.${param}}") - - // ExtractUsedVariables is very old code, should be more advanced. - c.Check(nestedVarnames, check.IsNil) - - plainVarnames := mkline.ExtractUsedVariables("${VAR}and${VAR2}") + mklines := t.NewMkLines("Makefile", + MkRcsID, + "VAR=\t${VALUE} # ${varassign.comment}", + ".if ${OPSYS:M${endianness}} == ${Hello:L} # ${if.comment}", + ".for var in one ${two} three # ${for.comment}", + "# ${empty.comment}", + "${TARGETS}: ${SOURCES} # ${dependency.comment}", + ".include \"${OTHER_FILE}\"", + "", + "\t"+ + "${VAR.${param}}"+ + "${VAR}and${VAR2}"+ + "${VAR:M${pattern}}"+ + "$(ROUND_PARENTHESES)"+ + "$$shellvar"+ + "$<$@$x") + + var varnames []string + for _, mkline := range mklines.mklines { + varnames = append(varnames, mkline.DetermineUsedVariables()...) + } - c.Check(plainVarnames, deepEquals, []string{"VAR", "VAR2"}) + c.Check(varnames, deepEquals, []string{ + "VALUE", + "OPSYS", + "endianness", + // "Hello" is not a variable name, the :L modifier makes it an expression. + "two", + "TARGETS", + "SOURCES", + "OTHER_FILE", + + "VAR.${param}", + "param", + "VAR", + "VAR2", + "VAR", + "pattern", + "ROUND_PARENTHESES", + // Shell variables are ignored here. + "<", + "@", + "x"}) } Index: pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.15 pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.16 --- pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.15 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go Tue Oct 9 19:12:13 2018 @@ -176,13 +176,19 @@ func (s *Suite) Test_MkLineChecker_Check func (s *Suite) Test_MkLineChecker_CheckVartype__append_to_non_list(c *check.C) { t := s.Init(c) + t.SetupCommandLine("-Wall") t.SetupVartypes() - mkline := t.NewMkLine("fname", 1, "DISTNAME+=suffix") + mklines := t.NewMkLines("fname.mk", + MkRcsID, + "DISTNAME+=\tsuffix", + "COMMENT=\tComment for", + "COMMENT+=\tthe package") - MkLineChecker{mkline}.Check() + mklines.Check() t.CheckOutputLines( - "WARN: fname:1: The \"+=\" operator should only be used with lists.") + "WARN: fname.mk:2: The variable DISTNAME may not be appended to (only set, given a default value) in this file.", + "WARN: fname.mk:2: The \"+=\" operator should only be used with lists, not with DISTNAME.") } // Pkglint once interpreted all lists as consisting of shell tokens, @@ -347,11 +353,17 @@ func (s *Suite) Test_MkLineChecker_check t.SetupVartypes() mklines := t.NewMkLines("options.mk", MkRcsID, - "WRKSRC:=${.CURDIR}") + "WRKSRC:=${.CURDIR}", + ".if ${PKG_SYSCONFDIR.gdm} != \"etc\"", + ".endif") mklines.Check() - // Don't warn that ".CURDIR should not be evaluated at load time." + // Evaluating PKG_SYSCONFDIR.* at load time is probably ok, though + // pkglint cannot prove anything here. + // + // Evaluating .CURDIR at load time is ok since it is defined from + // the beginning. t.CheckOutputLines( "NOTE: options.mk:2: This variable value should be aligned to column 17.") } @@ -555,8 +567,10 @@ func (s *Suite) Test_MkLineChecker_Check mklines.Check() - // FIXME: There should be some notes and warnings; prevented by the PERL5 case in VariableNeedsQuoting. - t.CheckOutputEmpty() + // FIXME: There should be some notes and warnings about missing :M*; + // these are currently prevented by the PERL5 case in VariableNeedsQuoting. + t.CheckOutputLines( + "WARN: ~/options.mk:4: ADA_FLAGS is used but not defined.") } func (s *Suite) Test_MkLineChecker_CheckVaruseShellword__mstar_not_needed(c *check.C) { @@ -611,7 +625,7 @@ func (s *Suite) Test_MkLineChecker_Check mklines.Check() t.CheckOutputLines( - "WARN: ~/options.mk:2: The :from=to modifier should only be used with lists.") + "WARN: ~/options.mk:2: The :from=to modifier should only be used with lists, not with WRKDIR.") } func (s *Suite) Test_MkLineChecker_CheckVaruse__for(c *check.C) { @@ -631,6 +645,27 @@ func (s *Suite) Test_MkLineChecker_Check t.CheckOutputEmpty() } +func (s *Suite) Test_MkLineChecker_CheckVaruse__defined_in_infrastructure(c *check.C) { + t := s.Init(c) + + t.SetupCommandLine("-Wall") + t.SetupPkgsrc() + t.SetupVartypes() + t.CreateFileLines("mk/deeply/nested/infra.mk", + MkRcsID, + "INFRA_VAR?=\tvalue") + G.Pkgsrc.LoadInfrastructure() + mklines := t.SetupFileMkLines("category/package/module.mk", + MkRcsID, + "do-fetch:", + "\t: ${INFRA_VAR} ${UNDEFINED}") + + mklines.Check() + + t.CheckOutputLines( + "WARN: ~/category/package/module.mk:3: UNDEFINED is used but not defined.") +} + func (s *Suite) Test_MkLineChecker_CheckVaruse__build_defs(c *check.C) { t := s.Init(c) @@ -668,8 +703,9 @@ func (s *Suite) Test_MkLineChecker_check "_TOOLS_VARNAME.sed= SED", "DIST_SUBDIR= ${PKGNAME}", "WRKSRC= ${PKGNAME}", - "SITES_distfile.tar.gz= ${MASTER_SITES_GITHUB:=user/}", - // TODO: The first of the below assignments should be flagged as redundant by RedundantScope. + "SITES_distfile.tar.gz= ${MASTER_SITE_GITHUB:=user/}", + // TODO: The first of the below assignments should be flagged as redundant by RedundantScope; + // that check is currently only implemented for package Makefiles, not for other files. "PYTHON_VERSIONS_ACCEPTED= -13", "PYTHON_VERSIONS_ACCEPTED= 27 36") Index: pkgsrc/pkgtools/pkglint/files/mkparser_test.go diff -u pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.15 pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.16 --- pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.15 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/mkparser_test.go Tue Oct 9 19:12:13 2018 @@ -1,7 +1,9 @@ package main import ( + "fmt" "gopkg.in/check.v1" + "strings" ) func (s *Suite) Test_MkParser_MkTokens(c *check.C) { @@ -241,7 +243,7 @@ func (s *Suite) Test_MkParser_MkCond(c * " || defined(PKG_OPTIONS:Msamplerate)") checkRest("${LEFT} &&", &mkCond{Not: &mkCond{Empty: varuse("LEFT")}}, - " &&") + "&&") checkRest("\"unfinished string literal", nil, "\"unfinished string literal") @@ -270,3 +272,61 @@ func (s *Suite) Test_MkParser__varuse_pa MkRcsID, "COMMENT=${P1} ${P2}) ${P3:Q} ${BRACES}") } + +func (s *Suite) Test_MkCondWalker_Walk(c *check.C) { + t := s.Init(c) + + mkline := t.NewMkLine("Makefile", 4, ""+ + ".if ${VAR:Mmatch} == ${OTHER} || "+ + "${STR} == Str || "+ + "${NUM} == 3 && "+ + "defined(VAR) && "+ + "!exists(file.mk) && "+ + "(((${NONEMPTY})))") + var events []string + + varuseStr := func(varuse *MkVarUse) string { + return strings.Join(append([]string{varuse.varname}, varuse.modifiers...), ":") + } + + addEvent := func(name string, args ...string) { + events = append(events, fmt.Sprintf("%14s %s", name, strings.Join(args, ", "))) + } + + NewMkCondWalker().Walk(mkline.Cond(), &MkCondCallback{ + Defined: func(varname string) { + addEvent("defined", varname) + }, + Empty: func(varuse *MkVarUse) { + addEvent("empty", varuseStr(varuse)) + }, + CompareVarNum: func(varuse *MkVarUse, op string, num string) { + addEvent("compareVarNum", varuseStr(varuse), num) + }, + CompareVarStr: func(varuse *MkVarUse, op string, str string) { + addEvent("compareVarStr", varuseStr(varuse), str) + }, + CompareVarVar: func(left *MkVarUse, op string, right *MkVarUse) { + addEvent("compareVarVar", varuseStr(left), varuseStr(right)) + }, + Call: func(name string, arg string) { + addEvent("call", name, arg) + }, + VarUse: func(varuse *MkVarUse) { + addEvent("varUse", varuseStr(varuse)) + }}) + + c.Check(events, deepEquals, []string{ + " compareVarVar VAR:Mmatch, OTHER", + " varUse VAR:Mmatch", + " varUse OTHER", + " compareVarStr STR, Str", + " varUse STR", + " compareVarNum NUM, 3", + " varUse NUM", + " defined VAR", + " varUse VAR", + " call exists, file.mk", + " empty NONEMPTY", + " varUse NONEMPTY"}) +} Index: pkgsrc/pkgtools/pkglint/files/mklines.go diff -u pkgsrc/pkgtools/pkglint/files/mklines.go:1.32 pkgsrc/pkgtools/pkglint/files/mklines.go:1.33 --- pkgsrc/pkgtools/pkglint/files/mklines.go:1.32 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/mklines.go Tue Oct 9 19:12:13 2018 @@ -2,7 +2,6 @@ package main import ( "netbsd.org/pkglint/trace" - "path" "strings" ) @@ -17,7 +16,7 @@ type MkLines struct { plistVarAdded map[string]MkLine // Identifiers that are added to PLIST_VARS. plistVarSet map[string]MkLine // Identifiers for which PLIST.${id} is defined. plistVarSkip bool // True if any of the PLIST_VARS identifiers refers to a variable. - Tools Tools // Tools defined in file scope. + Tools *Tools // Tools defined in file scope. indentation *Indentation // Indentation depth of preprocessing directives; only available during MkLines.ForEach. Once } @@ -34,7 +33,7 @@ func NewMkLines(lines []Line) *MkLines { } tools := NewTools(traceName) - tools.AddAll(G.Pkgsrc.Tools) + tools.Fallback(G.Pkgsrc.Tools) return &MkLines{ mklines, @@ -66,15 +65,6 @@ func (mklines *MkLines) Check() { G.Mk = mklines defer func() { G.Mk = nil }() - allowedTargets := make(map[string]bool) - prefixes := [...]string{"pre", "do", "post"} - actions := [...]string{"fetch", "extract", "patch", "tools", "wrapper", "configure", "build", "test", "install", "package", "clean"} - for _, prefix := range prefixes { - for _, action := range actions { - allowedTargets[prefix+"-"+action] = true - } - } - // In the first pass, all additions to BUILD_DEFS and USE_TOOLS // are collected to make the order of the definitions irrelevant. mklines.DetermineUsedVariables() @@ -83,12 +73,28 @@ func (mklines *MkLines) Check() { mklines.collectElse() // In the second pass, the actual checks are done. + mklines.checkAll() + + SaveAutofixChanges(mklines.lines) +} + +func (mklines *MkLines) checkAll() { + allowedTargets := func() map[string]bool { + targets := make(map[string]bool) + prefixes := [...]string{"pre", "do", "post"} + actions := [...]string{"fetch", "extract", "patch", "tools", "wrapper", "configure", "build", "test", "install", "package", "clean"} + for _, prefix := range prefixes { + for _, action := range actions { + targets[prefix+"-"+action] = true + } + } + return targets + }() CheckLineRcsid(mklines.lines[0], `#\s+`, "# ") - substcontext := NewSubstContext() + substContext := NewSubstContext() var varalign VaralignBlock - lastMkline := mklines.mklines[len(mklines.mklines)-1] isHacksMk := mklines.lines[0].Basename == "hacks.mk" lineAction := func(mkline MkLine) bool { @@ -99,16 +105,16 @@ func (mklines *MkLines) Check() { ck := MkLineChecker{mkline} ck.Check() varalign.Check(mkline) - mklines.Tools.ParseToolLine(mkline) + mklines.Tools.ParseToolLine(mkline, false, false) switch { case mkline.IsEmpty(): - substcontext.Finish(mkline) + substContext.Finish(mkline) case mkline.IsVarassign(): mklines.target = "" mkline.Tokenize(mkline.Value(), true) // Just for the side-effect of the warnings. - substcontext.Varassign(mkline) + substContext.Varassign(mkline) switch mkline.Varcanon() { case "PLIST_VARS": @@ -134,7 +140,7 @@ func (mklines *MkLines) Check() { case mkline.IsDirective(): ck.checkDirective(mklines.forVars, mklines.indentation) - substcontext.Directive(mkline) + substContext.Directive(mkline) case mkline.IsDependency(): ck.checkDependencyRule(allowedTargets) @@ -160,16 +166,15 @@ func (mklines *MkLines) Check() { } mklines.ForEachEnd(lineAction, atEnd) - substcontext.Finish(lastMkline) + substContext.Finish(NewMkLine(NewLineEOF(mklines.lines[0].Filename))) varalign.Finish() ChecklinesTrailingEmptyLines(mklines.lines) - - SaveAutofixChanges(mklines.lines) } // ForEach calls the action for each line, until the action returns false. -// It keeps track of the indentation and all conditional variables. +// It keeps track of the indentation (see MkLines.indentation) +// and all conditional variables (see Indentation.IsConditional). func (mklines *MkLines) ForEach(action func(mkline MkLine)) { mklines.ForEachEnd( func(mkline MkLine) bool { action(mkline); return true }, @@ -201,7 +206,7 @@ func (mklines *MkLines) DetermineDefined } for _, mkline := range mklines.mklines { - mklines.Tools.ParseToolLine(mkline) + mklines.Tools.ParseToolLine(mkline, false, true) if !mkline.IsVarassign() { continue @@ -211,14 +216,24 @@ func (mklines *MkLines) DetermineDefined varcanon := mkline.Varcanon() switch varcanon { - case "BUILD_DEFS", "PKG_GROUPS_VARS", "PKG_USERS_VARS": - for _, varname := range splitOnSpace(mkline.Value()) { + case + "BUILD_DEFS", + "PKG_GROUPS_VARS", + "PKG_USERS_VARS": + for _, varname := range fields(mkline.Value()) { mklines.buildDefs[varname] = true if trace.Tracing { trace.Step1("%q is added to BUILD_DEFS.", varname) } } + case + "BUILTIN_FIND_FILES_VAR", + "BUILTIN_FIND_HEADERS_VAR": + for _, varname := range fields(mkline.Value()) { + mklines.vars.Define(varname, mkline) + } + case "PLIST_VARS": ids := mkline.ValueSplit(resolveVariableRefs(mkline.Value()), "") for _, id := range ids { @@ -234,7 +249,7 @@ func (mklines *MkLines) DetermineDefined } case "SUBST_VARS.*": - for _, svar := range splitOnSpace(mkline.Value()) { + for _, svar := range fields(mkline.Value()) { mklines.UseVar(mkline, varnameCanon(svar)) if trace.Tracing { trace.Step1("varuse %s", svar) @@ -242,7 +257,7 @@ func (mklines *MkLines) DetermineDefined } case "OPSYSVARS": - for _, osvar := range splitOnSpace(mkline.Value()) { + for _, osvar := range fields(mkline.Value()) { mklines.UseVar(mkline, osvar+".*") defineVar(mkline, osvar) } @@ -282,11 +297,11 @@ func (mklines *MkLines) collectElse() { func (mklines *MkLines) DetermineUsedVariables() { for _, mkline := range mklines.mklines { - varnames := mkline.DetermineUsedVariables() - for _, varname := range varnames { + for _, varname := range mkline.DetermineUsedVariables() { mklines.UseVar(mkline, varname) } } + mklines.determineDocumentedVariables() } @@ -312,7 +327,7 @@ func (mklines *MkLines) determineDocumen text := mkline.Text switch { case hasPrefix(text, "#"): - words := splitOnSpace(text) + words := fields(text) if len(words) <= 1 { break } @@ -346,7 +361,7 @@ func (mklines *MkLines) determineDocumen func (mklines *MkLines) CheckRedundantVariables() { scope := NewRedundantScope() isRelevant := func(old, new MkLine) bool { - if path.Base(old.Filename) != "Makefile" && path.Base(new.Filename) == "Makefile" { + if old.Basename != "Makefile" && new.Basename == "Makefile" { return false } if new.Op() == opAssignEval { @@ -447,7 +462,8 @@ func (va *VaralignBlock) Check(mkline Mk if mkline.IsMultiline() { // Interpreting the continuation marker as variable value // is cheating, but works well. - m, _, _, _, _, _, value, _, _ := MatchVarassign(mkline.raw[0].orignl) + text := strings.TrimSuffix(mkline.raw[0].orignl, "\n") + m, _, _, _, _, _, value, _, _ := MatchVarassign(text) continuation = m && value == "\\" } Index: pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.32 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.33 --- pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.32 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go Tue Oct 9 19:12:13 2018 @@ -907,9 +907,9 @@ func (s *Suite) Test_VartypeCheck_Tool(c t := s.Init(c) vt := NewVartypeCheckTester(t, (*VartypeCheck).Tool) - t.SetupToolUsable("tool1", "") - t.SetupToolUsable("tool2", "") - t.SetupToolUsable("tool3", "") + t.SetupTool("tool1", "", AtRunTime) + t.SetupTool("tool2", "", AtRunTime) + t.SetupTool("tool3", "", AtRunTime) vt.Varname("USE_TOOLS") vt.Op(opAssignAppend) Index: pkgsrc/pkgtools/pkglint/files/mklines_test.go diff -u pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.28 pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.29 --- pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.28 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/mklines_test.go Tue Oct 9 19:12:13 2018 @@ -86,9 +86,10 @@ func (s *Suite) Test_MkLines__for_loop_m t := s.Init(c) t.SetupCommandLine("-Wall") - t.SetupToolUsable("echo", "ECHO") - t.SetupToolUsable("find", "FIND") - t.SetupToolUsable("pax", "PAX") + t.SetupVartypes() + t.SetupTool("echo", "ECHO", AtRunTime) + t.SetupTool("find", "FIND", AtRunTime) + t.SetupTool("pax", "PAX", AtRunTime) mklines := t.NewMkLines("Makefile", // From audio/squeezeboxserver MkRcsID, "", @@ -102,6 +103,7 @@ func (s *Suite) Test_MkLines__for_loop_m t.CheckOutputLines( "WARN: Makefile:3: Variable names starting with an underscore (_list_) are reserved for internal pkgsrc use.", "WARN: Makefile:3: Variable names starting with an underscore (_dir_) are reserved for internal pkgsrc use.", + "WARN: Makefile:3: SBS_COPY is used but not defined.", "WARN: Makefile:4: The exitcode of \"${FIND}\" at the left of the | operator is ignored.") } @@ -129,6 +131,7 @@ func (s *Suite) Test_MkLines__varuse_sh_ t.SetupCommandLine("-Wall") t.SetupVartypes() + t.SetupTool("sed", "SED", AfterPrefsMk) mklines := t.NewMkLines("lang/qore/module.mk", MkRcsID, "qore-version=\tqore --short-version | ${SED} -e s/-.*//", @@ -209,6 +212,7 @@ func (s *Suite) Test_MkLines__indirect_v t := s.Init(c) t.SetupCommandLine("-Wall") + t.SetupTool("echo", "ECHO", AfterPrefsMk) mklines := t.NewMkLines("net/uucp/Makefile", MkRcsID, "", @@ -220,14 +224,18 @@ func (s *Suite) Test_MkLines__indirect_v mklines.Check() // No warning about UUCP_${var} being used but not defined. - t.CheckOutputLines( - "WARN: net/uucp/Makefile:5: Unknown shell command \"${ECHO}\".") + // Normally, parameterized variables use a dot instead of an + // underscore as separator. This is one of the other cases, + // and pkglint just doesn't warn about dynamic variable names + // like UUCP_${var} or SITES_${distfile}. + t.CheckOutputEmpty() } func (s *Suite) Test_MkLines_Check__list_variable_as_part_of_word(c *check.C) { t := s.Init(c) t.SetupCommandLine("-Wall") + t.SetupVartypes() mklines := t.NewMkLines("converters/chef/Makefile", MkRcsID, "\tcd ${WRKSRC} && tr '\\r' '\\n' < ${DISTDIR}/${DIST_SUBDIR}/${DISTFILES} > chef.l") @@ -346,9 +354,6 @@ func (s *Suite) Test_MkLines_DetermineDe // The SUV variable is used implicitly by the SUBST framework, therefore no warning. // The OSV.NetBSD variable is used implicitly via the OSV variable, therefore no warning. t.CheckOutputLines( - // FIXME: For most lists, using the := operator to exclude an item is ok. - "WARN: determine-defined-variables.mk:4: USE_TOOLS should not be evaluated at load time.", - "WARN: determine-defined-variables.mk:4: USE_TOOLS may not be used in any file; it is a write-only variable.", // FIXME: the below warning is wrong; it's ok to have SUBST blocks in all files, maybe except buildlink3.mk. "WARN: determine-defined-variables.mk:12: The variable SUBST_VARS.subst may not be set (only given a default value, appended to) in this file; it would be ok in Makefile, Makefile.common, options.mk.", // FIXME: the below warning is wrong; variables mentioned in SUBST_VARS should be allowed in that block. @@ -356,6 +361,31 @@ func (s *Suite) Test_MkLines_DetermineDe "WARN: determine-defined-variables.mk:16: Unknown shell command \"unknown-command\".") } +func (s *Suite) Test_MkLines_DetermineDefinedVariables__BUILTIN_FIND_FILES_VAR(c *check.C) { + t := s.Init(c) + + t.SetupCommandLine("-Wall,no-space") + t.SetupPackage("category/package") + t.CreateFileLines("mk/buildlink3/bsd.builtin.mk", + MkRcsID) + mklines := t.SetupFileMkLines("category/package/builtin.mk", + MkRcsID, + "", + "BUILTIN_FIND_FILES_VAR:= H_XFT2", + "BUILTIN_FIND_FILES.H_XFT2= ${X11BASE}/include/X11/Xft/Xft.h", + "", + ".include \"../../mk/buildlink3/bsd.builtin.mk\"", + "", + ".if ${H_XFT2:N__nonexistent__} && ${H_UNDEF:N__nonexistent__}", + ".endif") + G.Pkgsrc.LoadInfrastructure() + + mklines.Check() + + t.CheckOutputLines( + "WARN: ~/category/package/builtin.mk:8: H_UNDEF is used but not defined.") +} + func (s *Suite) Test_MkLines_DetermineUsedVariables__simple(c *check.C) { t := s.Init(c) @@ -382,8 +412,8 @@ func (s *Suite) Test_MkLines_DetermineUs c.Check(len(mklines.vars.used), equals, 3) c.Check(mklines.vars.FirstUse("inner"), equals, mkline) - c.Check(mklines.vars.FirstUse("outer."), equals, mkline) // XXX: why the . at the end? c.Check(mklines.vars.FirstUse("outer.*"), equals, mkline) + c.Check(mklines.vars.FirstUse("outer.${inner}"), equals, mkline) } func (s *Suite) Test_MkLines__private_tool_undefined(c *check.C) { @@ -424,6 +454,7 @@ func (s *Suite) Test_MkLines_Check__inde t := s.Init(c) t.SetupCommandLine("-Wall") + t.SetupVartypes() mklines := t.NewMkLines("options.mk", MkRcsID, ". if !defined(GUARD_MK)", @@ -445,14 +476,19 @@ func (s *Suite) Test_MkLines_Check__inde t.CheckOutputLines( "NOTE: options.mk:2: This directive should be indented by 0 spaces.", + "WARN: options.mk:2: GUARD_MK is used but not defined.", "NOTE: options.mk:3: This directive should be indented by 0 spaces.", "NOTE: options.mk:4: This directive should be indented by 2 spaces.", + "WARN: options.mk:4: FILES is used but not defined.", "NOTE: options.mk:5: This directive should be indented by 4 spaces.", + "WARN: options.mk:5: GUARD2_MK is used but not defined.", "NOTE: options.mk:6: This directive should be indented by 4 spaces.", "NOTE: options.mk:7: This directive should be indented by 4 spaces.", "NOTE: options.mk:8: This directive should be indented by 2 spaces.", "NOTE: options.mk:9: This directive should be indented by 2 spaces.", + "WARN: options.mk:9: COND1 is used but not defined.", "NOTE: options.mk:10: This directive should be indented by 2 spaces.", + "WARN: options.mk:10: COND2 is used but not defined.", "NOTE: options.mk:11: This directive should be indented by 2 spaces.", "ERROR: options.mk:11: \".else\" does not take arguments. If you meant \"else if\", use \".elif\".", "NOTE: options.mk:12: This directive should be indented by 2 spaces.", @@ -466,17 +502,18 @@ func (s *Suite) Test_MkLines_Check__endi t := s.Init(c) t.SetupCommandLine("-Wall") + t.SetupVartypes() mklines := t.NewMkLines("opsys.mk", MkRcsID, "", ".for i in 1 2 3 4 5", ". if ${OPSYS} == NetBSD", - ". if ${ARCH} == x86_64", + ". if ${MACHINE_ARCH} == x86_64", ". if ${OS_VERSION:M8.*}", - ". endif # ARCH", // Wrong, should be OS_VERSION. - ". endif # OS_VERSION", // Wrong, should be ARCH. - ". endif # OPSYS", // Correct. - ".endfor # j", // Wrong, should be i. + ". endif # MACHINE_ARCH", // Wrong, should be OS_VERSION. + ". endif # OS_VERSION", // Wrong, should be MACHINE_ARCH. + ". endif # OPSYS", // Correct. + ".endfor # j", // Wrong, should be i. "", ".if ${PKG_OPTIONS:Moption}", ".endif # option", @@ -492,9 +529,10 @@ func (s *Suite) Test_MkLines_Check__endi mklines.Check() t.CheckOutputLines( - "WARN: opsys.mk:7: Comment \"ARCH\" does not match condition \"${OS_VERSION:M8.*}\".", - "WARN: opsys.mk:8: Comment \"OS_VERSION\" does not match condition \"${ARCH} == x86_64\".", + "WARN: opsys.mk:7: Comment \"MACHINE_ARCH\" does not match condition \"${OS_VERSION:M8.*}\".", + "WARN: opsys.mk:8: Comment \"OS_VERSION\" does not match condition \"${MACHINE_ARCH} == x86_64\".", "WARN: opsys.mk:10: Comment \"j\" does not match loop \"i in 1 2 3 4 5\".", + "WARN: opsys.mk:12: Unknown option \"option\".", "WARN: opsys.mk:20: Comment \"NetBSD\" does not match condition \"${OPSYS} == FreeBSD\".") } @@ -502,12 +540,13 @@ func (s *Suite) Test_MkLines_Check__unba t := s.Init(c) t.SetupCommandLine("-Wall") + t.SetupVartypes() mklines := t.NewMkLines("opsys.mk", MkRcsID, "", ".for i in 1 2 3 4 5", ". if ${OPSYS} == NetBSD", - ". if ${ARCH} == x86_64", + ". if ${MACHINE_ARCH} == x86_64", ". if ${OS_VERSION:M8.*}") mklines.Check() @@ -516,6 +555,24 @@ func (s *Suite) Test_MkLines_Check__unba "ERROR: opsys.mk:6: Directive indentation is not 0, but 8.") } +func (s *Suite) Test_MkLines_Check__incomplete_subst_at_end(c *check.C) { + t := s.Init(c) + + t.SetupCommandLine("-Wall") + t.SetupVartypes() + mklines := t.NewMkLines("subst.mk", + MkRcsID, + "", + "SUBST_CLASSES+=\tclass") + + mklines.Check() + + t.CheckOutputLines( + "WARN: subst.mk:EOF: Incomplete SUBST block: SUBST_STAGE.class missing.", + "WARN: subst.mk:EOF: Incomplete SUBST block: SUBST_FILES.class missing.", + "WARN: subst.mk:EOF: Incomplete SUBST block: SUBST_SED.class, SUBST_VARS.class or SUBST_FILTER_CMD.class missing.") +} + // Demonstrates how to define your own make(1) targets for creating // files in the current directory. The pkgsrc-wip category Makefile // does this, while all other categories don't need any custom code. @@ -524,7 +581,7 @@ func (s *Suite) Test_MkLines__wip_catego t.SetupCommandLine("-Wall", "--explain") t.SetupVartypes() - t.SetupToolUsable("rm", "RM") + t.SetupTool("rm", "RM", AtRunTime) t.CreateFileLines("mk/misc/category.mk") mklines := t.SetupFileMkLines("wip/Makefile", MkRcsID, @@ -566,7 +623,7 @@ func (s *Suite) Test_MkLines_determineDo t.SetupCommandLine("-Wall") t.SetupVartypes() - t.SetupToolUsable("rm", "RM") + t.SetupTool("rm", "RM", AtRunTime) mklines := t.NewMkLines("Makefile", MkRcsID, "#", @@ -896,3 +953,39 @@ func (s *Suite) Test_MkLines_Check__hack // No warning about including bsd.prefs.mk before using the ?= operator. t.CheckOutputEmpty() } + +func (s *Suite) Test_MkLines_ForEach__conditional_variables(c *check.C) { + t := s.Init(c) + + t.SetupCommandLine("-Wall,no-space") + t.SetupVartypes() + mklines := t.NewMkLines("conditional.mk", + MkRcsID, + "", + ".if defined(PKG_DEVELOPER)", + "DEVELOPER=\tyes", + ".endif", + "", + ".if ${USE_TOOLS:Mgettext}", + "USES_GETTEXT=\tyes", + ".endif") + + seenDeveloper := false + seenUsesGettext := false + + mklines.ForEach(func(mkline MkLine) { + if mkline.IsVarassign() { + switch mkline.Varname() { + case "DEVELOPER": + c.Check(mklines.indentation.IsConditional(), equals, true) + seenDeveloper = true + case "USES_GETTEXT": + c.Check(mklines.indentation.IsConditional(), equals, true) + seenUsesGettext = true + } + } + }) + + c.Check(seenDeveloper, equals, true) + c.Check(seenUsesGettext, equals, true) +} Index: pkgsrc/pkgtools/pkglint/files/mkparser.go diff -u pkgsrc/pkgtools/pkglint/files/mkparser.go:1.17 pkgsrc/pkgtools/pkglint/files/mkparser.go:1.18 --- pkgsrc/pkgtools/pkglint/files/mkparser.go:1.17 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/mkparser.go Tue Oct 9 19:12:13 2018 @@ -217,7 +217,8 @@ func (p *MkParser) MkCond() MkCond { ands := []MkCond{and} for { mark := p.repl.Mark() - if !p.repl.AdvanceRegexp(`^\s*\|\|\s*`) { + p.repl.SkipHspace() + if !p.repl.AdvanceStr("||") { break } next := p.mkCondAnd() @@ -265,7 +266,7 @@ func (p *MkParser) mkCondAtom() MkCond { repl := p.repl mark := repl.Mark() - repl.SkipSpace() + repl.SkipHspace() switch { case repl.AdvanceStr("!"): cond := p.mkCondAtom() @@ -275,25 +276,25 @@ func (p *MkParser) mkCondAtom() MkCond { case repl.AdvanceStr("("): cond := p.MkCond() if cond != nil { - repl.SkipSpace() + repl.SkipHspace() if repl.AdvanceStr(")") { return cond } } - case repl.AdvanceRegexp(`^defined\s*\(`): + case repl.HasPrefix("defined") && repl.AdvanceRegexp(`^defined\s*\(`): if varname := p.Varname(); varname != "" { if repl.AdvanceStr(")") { return &mkCond{Defined: varname} } } - case repl.AdvanceRegexp(`^empty\s*\(`): + case repl.HasPrefix("empty") && repl.AdvanceRegexp(`^empty\s*\(`): if varname := p.Varname(); varname != "" { modifiers := p.VarUseModifiers(varname, ")") if repl.AdvanceStr(")") { return &mkCond{Empty: &MkVarUse{varname, modifiers}} } } - case repl.AdvanceRegexp(`^(commands|exists|make|target)\s*\(`): + case uint(repl.PeekByte()-'a') <= 'z'-'a' && repl.AdvanceRegexp(`^(commands|exists|make|target)\s*\(`): funcname := repl.Group(1) argMark := repl.Mark() for p.VarUse() != nil || repl.AdvanceRegexp(`^[^$)]+`) { @@ -402,6 +403,7 @@ type MkCondCallback struct { CompareVarStr func(varuse *MkVarUse, op string, str string) CompareVarVar func(left *MkVarUse, op string, right *MkVarUse) Call func(name string, arg string) + VarUse func(varuse *MkVarUse) } type MkCondWalker struct{} @@ -425,25 +427,42 @@ func (w *MkCondWalker) Walk(cond MkCond, if callback.Defined != nil { callback.Defined(cond.Defined) } + if callback.VarUse != nil { + callback.VarUse(&MkVarUse{cond.Defined, nil}) + } case cond.Empty != nil: if callback.Empty != nil { callback.Empty(cond.Empty) } + if callback.VarUse != nil { + callback.VarUse(cond.Empty) + } case cond.CompareVarVar != nil: if callback.CompareVarVar != nil { cvv := cond.CompareVarVar callback.CompareVarVar(cvv.Left, cvv.Op, cvv.Right) } + if callback.VarUse != nil { + cvv := cond.CompareVarVar + callback.VarUse(cvv.Left) + callback.VarUse(cvv.Right) + } case cond.CompareVarStr != nil: if callback.CompareVarStr != nil { cvs := cond.CompareVarStr callback.CompareVarStr(cvs.Var, cvs.Op, cvs.Str) } + if callback.VarUse != nil { + callback.VarUse(cond.CompareVarStr.Var) + } case cond.CompareVarNum != nil: if callback.CompareVarNum != nil { cvn := cond.CompareVarNum callback.CompareVarNum(cvn.Var, cvn.Op, cvn.Num) } + if callback.VarUse != nil { + callback.VarUse(cond.CompareVarNum.Var) + } case cond.Call != nil: if callback.Call != nil { call := cond.Call Index: pkgsrc/pkgtools/pkglint/files/vartype.go diff -u pkgsrc/pkgtools/pkglint/files/vartype.go:1.17 pkgsrc/pkgtools/pkglint/files/vartype.go:1.18 --- pkgsrc/pkgtools/pkglint/files/vartype.go:1.17 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/vartype.go Tue Oct 9 19:12:13 2018 @@ -70,9 +70,9 @@ func (perms ACLPermissions) HumanString( return strings.TrimRight(result, ", ") } -func (vt *Vartype) EffectivePermissions(fname string) ACLPermissions { +func (vt *Vartype) EffectivePermissions(basename string) ACLPermissions { for _, aclEntry := range vt.aclEntries { - if m, _ := path.Match(aclEntry.glob, path.Base(fname)); m { + if m, _ := path.Match(aclEntry.glob, basename); m { return aclEntry.permissions } } @@ -118,7 +118,7 @@ func (vt *Vartype) IsConsideredList() bo } func (vt *Vartype) MayBeAppendedTo() bool { - return vt.kindOfList != lkNone || vt.IsConsideredList() + return vt.kindOfList != lkNone || vt.IsConsideredList() || vt.basicType == BtComment } func (vt *Vartype) String() string { Index: pkgsrc/pkgtools/pkglint/files/package.go diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.36 pkgsrc/pkgtools/pkglint/files/package.go:1.37 --- pkgsrc/pkgtools/pkglint/files/package.go:1.36 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/package.go Tue Oct 9 19:12:13 2018 @@ -169,12 +169,14 @@ func (pkglint *Pkglint) checkdirPackage( if pkg.DistinfoFile != pkg.vars.fallback["DISTINFO_FILE"] { files = append(files, pkg.File(pkg.DistinfoFile)) } + haveDistinfo := false havePatches := false // Determine the used variables and PLIST directories before checking any of the Makefile fragments. for _, fname := range files { - if (hasPrefix(path.Base(fname), "Makefile.") || hasSuffix(fname, ".mk")) && + basename := path.Base(fname) + if (hasPrefix(basename, "Makefile.") || hasSuffix(fname, ".mk")) && !matches(fname, `patch-`) && !contains(fname, pkg.Pkgdir+"/") && !contains(fname, pkg.Filesdir+"/") { @@ -182,7 +184,7 @@ func (pkglint *Pkglint) checkdirPackage( mklines.DetermineUsedVariables() } } - if hasPrefix(path.Base(fname), "PLIST") { + if hasPrefix(basename, "PLIST") { pkg.loadPlistDirs(fname) } } @@ -194,9 +196,10 @@ func (pkglint *Pkglint) checkdirPackage( } continue } - if fname == pkg.File("Makefile") { + + if path.Base(fname) == "Makefile" { if st, err := os.Lstat(fname); err == nil { - pkglint.checkExecutable(st) + pkglint.checkExecutable(fname, st) } if G.opts.CheckMakefile { pkg.checkfilePackageMakefile(fname, lines) @@ -217,14 +220,6 @@ func (pkglint *Pkglint) checkdirPackage( NewLineWhole(pkg.File(pkg.DistinfoFile)).Warnf("File not found. Please run \"%s makepatchsum\".", confMake) } } - - if G.opts.CheckAlternatives { - for _, fname := range files { - if path.Base(fname) == "ALTERNATIVES" { - CheckfileAlternatives(fname, pkg.PlistFiles) - } - } - } } func (pkg *Package) loadPackageMakefile() *MkLines { @@ -246,6 +241,11 @@ func (pkg *Package) loadPackageMakefile( } } + if pkg.vars.Defined("USE_CMAKE") { + mainLines.Tools.defTool("cmake", "", false, AtRunTime) + mainLines.Tools.defTool("cpack", "", false, AtRunTime) + } + allLines.DetermineUsedVariables() allLines.CheckRedundantVariables() @@ -255,11 +255,11 @@ func (pkg *Package) loadPackageMakefile( pkg.Patchdir, _ = pkg.vars.Value("PATCHDIR") // See lang/php/ext.mk - if varIsDefined("PHPEXT_MK") { - if !varIsDefined("USE_PHP_EXT_PATCHES") { + if varIsDefinedSimilar("PHPEXT_MK") { + if !varIsDefinedSimilar("USE_PHP_EXT_PATCHES") { pkg.Patchdir = "patches" } - if varIsDefined("PECL_VERSION") { + if varIsDefinedSimilar("PECL_VERSION") { pkg.DistinfoFile = "distinfo" } else { pkg.IgnoreMissingPatches = true @@ -315,7 +315,7 @@ func (pkg *Package) readMakefile(fname s } if includeFile != "" { - if path.Base(fname) != "buildlink3.mk" { + if mkline.Basename != "buildlink3.mk" { if m, bl3File := match1(includeFile, `^\.\./\.\./(.*)/buildlink3\.mk$`); m { pkg.bl3[bl3File] = mkline.Line if trace.Tracing { @@ -333,7 +333,7 @@ func (pkg *Package) readMakefile(fname s mkline.ExplainRelativeDirs() } - if path.Base(fname) == "Makefile" && !hasPrefix(incDir, "../../mk/") && incBase != "buildlink3.mk" && incBase != "builtin.mk" && incBase != "options.mk" { + if mkline.Basename == "Makefile" && !hasPrefix(incDir, "../../mk/") && incBase != "buildlink3.mk" && incBase != "builtin.mk" && incBase != "options.mk" { if trace.Tracing { trace.Step1("Including %q sets seenMakefileCommon.", includeFile) } Index: pkgsrc/pkgtools/pkglint/files/package_test.go diff -u pkgsrc/pkgtools/pkglint/files/package_test.go:1.30 pkgsrc/pkgtools/pkglint/files/package_test.go:1.31 --- pkgsrc/pkgtools/pkglint/files/package_test.go:1.30 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/package_test.go Tue Oct 9 19:12:13 2018 @@ -395,7 +395,7 @@ func (s *Suite) Test_Package_loadPackage t.CheckOutputLines( "Whole Makefile (with all included files) follows:", - "~/category/package/Makefile:1: # $NetBSD: package_test.go,v 1.30 2018/10/03 22:27:53 rillig Exp $", + "~/category/package/Makefile:1: "+MkRcsID, "~/category/package/Makefile:2: ", "~/category/package/Makefile:3: CATEGORIES=category", "~/category/package/Makefile:4: ", @@ -489,7 +489,7 @@ func (s *Suite) Test_Package__varuse_at_ t := s.Init(c) t.SetupPkgsrc() - t.SetupToolUsable("printf", "") + t.SetupTool("printf", "", AtRunTime) t.CreateFileLines("licenses/2-clause-bsd", "# dummy") t.CreateFileLines("misc/Makefile") @@ -1054,7 +1054,7 @@ func (s *Suite) Test_Pkglint_checkdirPac // Pkglint cannot currently resolve the location of DISTINFO_FILE completely // because the variable \"rv\" comes from a .for loop. // - // TODO: resolve variables in simple .for loops like the above. + // TODO: iterate over variables in simple .for loops like the above. G.CheckDirent(pkg) t.CheckOutputEmpty() Index: pkgsrc/pkgtools/pkglint/files/patches_test.go diff -u pkgsrc/pkgtools/pkglint/files/patches_test.go:1.21 pkgsrc/pkgtools/pkglint/files/patches_test.go:1.22 --- pkgsrc/pkgtools/pkglint/files/patches_test.go:1.21 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/patches_test.go Tue Oct 9 19:12:13 2018 @@ -686,14 +686,14 @@ func (s *Suite) Test_PatchChecker_checkt " $"+"Id$", "-old line", "+new line", - " $Author: rillig $") + " $"+"Author: authorship $") ChecklinesPatch(lines) t.CheckOutputLines( - "WARN: ~/patch-aa:7: Found RCS tag \"$Id: patches_test.go,v 1.21 2018/10/03 22:27:53 rillig Exp $\". Please remove it.", - "WARN: ~/patch-aa:8: Found RCS tag \"$Id: patches_test.go,v 1.21 2018/10/03 22:27:53 rillig Exp $\". Please remove it by reducing the number of context lines using pkgdiff or \"diff -U[210]\".", - "WARN: ~/patch-aa:11: Found RCS tag \"$Author: rillig $\". Please remove it by reducing the number of context lines using pkgdiff or \"diff -U[210]\".") + "WARN: ~/patch-aa:7: Found RCS tag \"$"+"Id$\". Please remove it.", + "WARN: ~/patch-aa:8: Found RCS tag \"$"+"Id$\". Please remove it by reducing the number of context lines using pkgdiff or \"diff -U[210]\".", + "WARN: ~/patch-aa:11: Found RCS tag \"$"+"Author$\". Please remove it by reducing the number of context lines using pkgdiff or \"diff -U[210]\".") } func (s *Suite) Test_FileType_String(c *check.C) { Index: pkgsrc/pkgtools/pkglint/files/pkglint_test.go diff -u pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.25 pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.26 --- pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.25 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/pkglint_test.go Tue Oct 9 19:12:13 2018 @@ -2,6 +2,7 @@ package main import ( "io/ioutil" + "path" "strings" "time" @@ -259,7 +260,7 @@ func (s *Suite) Test_Pkglint__coverage(c cmdline := os.Getenv("PKGLINT_TESTCMDLINE") if cmdline != "" { G.logOut, G.logErr, trace.Out = NewSeparatorWriter(os.Stdout), NewSeparatorWriter(os.Stderr), os.Stdout - G.Main(append([]string{"pkglint"}, splitOnSpace(cmdline)...)...) + G.Main(append([]string{"pkglint"}, fields(cmdline)...)...) } } @@ -578,9 +579,9 @@ func (s *Suite) Test_Pkglint_Tool__looku loadTimeTool, loadTimeUsable := G.Tool("tool", LoadTime) runTimeTool, runTimeUsable := G.Tool("tool", RunTime) - c.Check(loadTimeTool, equals, global) + c.Check(*loadTimeTool, equals, *global) c.Check(loadTimeUsable, equals, false) - c.Check(runTimeTool, equals, global) + c.Check(*runTimeTool, equals, *global) c.Check(runTimeUsable, equals, false) } @@ -607,16 +608,14 @@ func (s *Suite) Test_Pkglint_Tool__looku t := s.Init(c) G.Mk = t.NewMkLines("Makefile", MkRcsID) - global := G.Pkgsrc.Tools.Define("tool", "TOOL", dummyMkLine) - - global.Validity = Nowhere + G.Pkgsrc.Tools.defTool("tool", "TOOL", false, Nowhere) loadTimeTool, loadTimeUsable := G.Tool("${TOOL}", LoadTime) runTimeTool, runTimeUsable := G.Tool("${TOOL}", RunTime) - c.Check(loadTimeTool, equals, global) + c.Check(loadTimeTool.String(), equals, "tool:TOOL::Nowhere") c.Check(loadTimeUsable, equals, false) - c.Check(runTimeTool, equals, global) + c.Check(runTimeTool.String(), equals, "tool:TOOL::Nowhere") c.Check(runTimeUsable, equals, false) } @@ -624,16 +623,14 @@ func (s *Suite) Test_Pkglint_Tool__looku t := s.Init(c) G.Mk = t.NewMkLines("Makefile", MkRcsID) - global := G.Pkgsrc.Tools.Define("tool", "TOOL", dummyMkLine) - - global.Validity = AtRunTime + G.Pkgsrc.Tools.defTool("tool", "TOOL", false, AtRunTime) loadTimeTool, loadTimeUsable := G.Tool("${TOOL}", LoadTime) runTimeTool, runTimeUsable := G.Tool("${TOOL}", RunTime) - c.Check(loadTimeTool, equals, global) + c.Check(loadTimeTool.String(), equals, "tool:TOOL::AtRunTime") c.Check(loadTimeUsable, equals, false) - c.Check(runTimeTool, equals, global) + c.Check(runTimeTool.String(), equals, "tool:TOOL::AtRunTime") c.Check(runTimeUsable, equals, true) } @@ -651,16 +648,14 @@ func (s *Suite) Test_Pkglint_ToolByVarna c.Check(G.ToolByVarname("TOOL", RunTime), equals, local) } -func (s *Suite) Test_Pkglint_ToolByVarname__fallback(c *check.C) { +func (s *Suite) Test_Pkglint_ToolByVarname(c *check.C) { t := s.Init(c) G.Mk = t.NewMkLines("Makefile", MkRcsID) - global := G.Pkgsrc.Tools.Define("tool", "TOOL", dummyMkLine) + G.Pkgsrc.Tools.defTool("tool", "TOOL", false, AtRunTime) - global.Validity = AtRunTime - - c.Check(G.ToolByVarname("TOOL", LoadTime), equals, global) - c.Check(G.ToolByVarname("TOOL", RunTime), equals, global) + c.Check(G.ToolByVarname("TOOL", LoadTime).String(), equals, "tool:TOOL::AtRunTime") + c.Check(G.ToolByVarname("TOOL", RunTime).String(), equals, "tool:TOOL::AtRunTime") } func (s *Suite) Test_CheckfileExtra(c *check.C) { @@ -775,13 +770,13 @@ func (s *Suite) Test_Pkglint_Checkfile__ "", "SHA1 (patch-README) = b9101ebf0bca8ce243ed6433b65555fa6a5ecd52") - // Copy category/package to wip/package. + // Copy category/package/** to wip/package. for _, basename := range []string{"files/README", "patches/patch-README", "Makefile", "PLIST", "README", "TODO", "distinfo"} { src := "category/package/" + basename dst := "wip/package/" + basename text, err := ioutil.ReadFile(t.File(src)) c.Check(err, check.IsNil) - t.CreateFileLines(dst, strings.TrimSpace(string(text))) + t.CreateFileLines(dst, strings.TrimSuffix(string(text), "\n")) } t.SetupPkgsrc() @@ -887,18 +882,21 @@ func (s *Suite) Test_CheckfileMk__enoent func (s *Suite) Test_Pkglint_checkExecutable(c *check.C) { t := s.Init(c) - G.checkExecutable(ExecutableFileInfo{t.File("fname.mk")}) + fileName := t.File("file.mk") + fileInfo := ExecutableFileInfo{path.Base(fileName)} + + G.checkExecutable(fileName, fileInfo) t.CheckOutputLines( - "WARN: ~/fname.mk: Should not be executable.") + "WARN: ~/file.mk: Should not be executable.") t.SetupCommandLine("--autofix") - G.checkExecutable(ExecutableFileInfo{t.File("fname.mk")}) + G.checkExecutable(fileName, fileInfo) // FIXME: The error message "Cannot clear executable bits" is swallowed. t.CheckOutputLines( - "AUTOFIX: ~/fname.mk: Clearing executable bits") + "AUTOFIX: ~/file.mk: Clearing executable bits") } func (s *Suite) Test_main(c *check.C) { Index: pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.8 pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.9 --- pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.8 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go Tue Oct 9 19:12:13 2018 @@ -74,7 +74,9 @@ func (s *Suite) Test_Pkgsrc_loadTools(c t.CreateFileLines("mk/tools/flex.mk", "# empty") t.CreateFileLines("mk/tools/gettext.mk", - "USE_TOOLS+=msgfmt", + ".if ${USE_TOOLS:Mgettext}", // This conditional prevents msgfmt from + "USE_TOOLS+=msgfmt", // being added to the default USE_TOOLS. + ".endif", "TOOLS_CREATE+=msgfmt") t.CreateFileLines("mk/tools/strip.mk", ".if defined(_INSTALL_UNSTRIPPED) || !defined(TOOLS_PLATFORM.strip)", @@ -102,20 +104,20 @@ func (s *Suite) Test_Pkgsrc_loadTools(c t.CheckOutputLines( "TRACE: + (*Tools).Trace(\"Pkgsrc\")", - "TRACE: 1 tool &{Name:bzcat Varname: MustUseVarForm:false Validity:Nowhere}", - "TRACE: 1 tool &{Name:bzip2 Varname: MustUseVarForm:false Validity:Nowhere}", - "TRACE: 1 tool &{Name:chown Varname:CHOWN MustUseVarForm:false Validity:Nowhere}", - "TRACE: 1 tool &{Name:echo Varname:ECHO MustUseVarForm:true Validity:AfterPrefsMk}", - "TRACE: 1 tool &{Name:echo -n Varname:ECHO_N MustUseVarForm:true Validity:AfterPrefsMk}", - "TRACE: 1 tool &{Name:false Varname:FALSE MustUseVarForm:true Validity:Nowhere}", - "TRACE: 1 tool &{Name:gawk Varname:AWK MustUseVarForm:false Validity:Nowhere}", - "TRACE: 1 tool &{Name:m4 Varname: MustUseVarForm:false Validity:AfterPrefsMk}", - "TRACE: 1 tool &{Name:msgfmt Varname: MustUseVarForm:false Validity:Nowhere}", - "TRACE: 1 tool &{Name:mv Varname:MV MustUseVarForm:false Validity:AtRunTime}", - "TRACE: 1 tool &{Name:pwd Varname:PWD MustUseVarForm:false Validity:AfterPrefsMk}", - "TRACE: 1 tool &{Name:strip Varname: MustUseVarForm:false Validity:Nowhere}", - "TRACE: 1 tool &{Name:test Varname:TEST MustUseVarForm:true Validity:AfterPrefsMk}", - "TRACE: 1 tool &{Name:true Varname:TRUE MustUseVarForm:true Validity:AfterPrefsMk}", + "TRACE: 1 tool bzcat:::Nowhere", + "TRACE: 1 tool bzip2:::Nowhere", + "TRACE: 1 tool chown:CHOWN::Nowhere", + "TRACE: 1 tool echo:ECHO:var:AfterPrefsMk", + "TRACE: 1 tool echo -n:ECHO_N:var:AfterPrefsMk", + "TRACE: 1 tool false:FALSE:var:AtRunTime", + "TRACE: 1 tool gawk:AWK::Nowhere", + "TRACE: 1 tool m4:::AfterPrefsMk", + "TRACE: 1 tool msgfmt:::Nowhere", + "TRACE: 1 tool mv:MV::AtRunTime", + "TRACE: 1 tool pwd:PWD::AfterPrefsMk", + "TRACE: 1 tool strip:::Nowhere", + "TRACE: 1 tool test:TEST:var:AfterPrefsMk", + "TRACE: 1 tool true:TRUE:var:AfterPrefsMk", "TRACE: - (*Tools).Trace(\"Pkgsrc\")") } @@ -124,7 +126,7 @@ func (s *Suite) Test_Pkgsrc_loadTools__B t := s.Init(c) t.SetupCommandLine("-Wall") - t.SetupToolUsable("echo", "ECHO") + t.SetupTool("echo", "ECHO", AtRunTime) pkg := t.SetupPackage("category/package", "pre-configure:", "\t@${ECHO} ${PKG_SYSCONFDIR} ${VARBASE}") Index: pkgsrc/pkgtools/pkglint/files/shtokenizer_test.go diff -u pkgsrc/pkgtools/pkglint/files/shtokenizer_test.go:1.8 pkgsrc/pkgtools/pkglint/files/shtokenizer_test.go:1.9 --- pkgsrc/pkgtools/pkglint/files/shtokenizer_test.go:1.8 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/shtokenizer_test.go Tue Oct 9 19:12:13 2018 @@ -507,7 +507,7 @@ func (s *Suite) Test_ShTokenizer__exampl "\t"+"\"`'`y", // Covers shAtomDquotBackt: return nil - // FIXME: Pkglint must parse unescpaed dollar in the same way, everywhere. + // FIXME: Pkglint must parse unescaped dollar in the same way, everywhere. "\t"+"\"`$|", // Covers shAtomDquotBacktDquot: return nil Index: pkgsrc/pkgtools/pkglint/files/vartype_test.go diff -u pkgsrc/pkgtools/pkglint/files/vartype_test.go:1.8 pkgsrc/pkgtools/pkglint/files/vartype_test.go:1.9 --- pkgsrc/pkgtools/pkglint/files/vartype_test.go:1.8 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/vartype_test.go Tue Oct 9 19:12:13 2018 @@ -9,17 +9,16 @@ func (s *Suite) Test_Vartype_EffectivePe t.SetupVartypes() - if t := G.Pkgsrc.vartypes["PREFIX"]; c.Check(t, check.NotNil) { - c.Check(t.basicType.name, equals, "Pathname") - c.Check(t.aclEntries, check.DeepEquals, []ACLEntry{{glob: "*", permissions: aclpUse}}) - c.Check(t.EffectivePermissions("Makefile"), equals, aclpUse) + if typ := G.Pkgsrc.vartypes["PREFIX"]; c.Check(typ, check.NotNil) { + c.Check(typ.basicType.name, equals, "Pathname") + c.Check(typ.aclEntries, check.DeepEquals, []ACLEntry{{glob: "*", permissions: aclpUse}}) + c.Check(typ.EffectivePermissions("Makefile"), equals, aclpUse) } - if t := G.Pkgsrc.vartypes["EXTRACT_OPTS"]; c.Check(t, check.NotNil) { - c.Check(t.basicType.name, equals, "ShellWord") - c.Check(t.EffectivePermissions("Makefile"), equals, aclpAppend|aclpSet) - c.Check(t.EffectivePermissions("../Makefile"), equals, aclpAppend|aclpSet) - c.Check(t.EffectivePermissions("options.mk"), equals, aclpUnknown) + if typ := G.Pkgsrc.vartypes["EXTRACT_OPTS"]; c.Check(typ, check.NotNil) { + c.Check(typ.basicType.name, equals, "ShellWord") + c.Check(typ.EffectivePermissions("Makefile"), equals, aclpAppend|aclpSet) + c.Check(typ.EffectivePermissions("options.mk"), equals, aclpUnknown) } } Index: pkgsrc/pkgtools/pkglint/files/plist.go diff -u pkgsrc/pkgtools/pkglint/files/plist.go:1.29 pkgsrc/pkgtools/pkglint/files/plist.go:1.30 --- pkgsrc/pkgtools/pkglint/files/plist.go:1.29 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/plist.go Tue Oct 9 19:12:13 2018 @@ -53,8 +53,8 @@ func (ck *PlistChecker) Check(plainLines plines := ck.NewLines(plainLines) ck.collectFilesAndDirs(plines) - if fname := plines[0].Filename; path.Base(fname) == "PLIST.common_end" { - commonLines := Load(strings.TrimSuffix(fname, "_end"), NotEmpty) + if plines[0].Basename == "PLIST.common_end" { + commonLines := Load(strings.TrimSuffix(plines[0].Filename, "_end"), NotEmpty) if commonLines != nil { ck.collectFilesAndDirs(ck.NewLines(commonLines)) } @@ -429,7 +429,7 @@ func (pline *PlistLine) CheckDirective(c "command in the PLIST") case "imake-man": - args := splitOnSpace(arg) + args := fields(arg) switch { case len(args) != 3: pline.Warnf("Invalid number of arguments for imake-man.") Index: pkgsrc/pkgtools/pkglint/files/util.go diff -u pkgsrc/pkgtools/pkglint/files/util.go:1.29 pkgsrc/pkgtools/pkglint/files/util.go:1.30 --- pkgsrc/pkgtools/pkglint/files/util.go:1.29 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/util.go Tue Oct 9 19:12:13 2018 @@ -13,7 +13,6 @@ import ( "strconv" "strings" "time" - "unicode" ) type YesNoUnknown uint8 @@ -38,6 +37,9 @@ func hasPrefix(s, prefix string) bool { func hasSuffix(s, suffix string) bool { return strings.HasSuffix(s, suffix) } +func fields(s string) []string { + return strings.Fields(s) +} func matches(s string, re regex.Pattern) bool { return G.res.Matches(s, re) } @@ -56,10 +58,6 @@ func match4(s string, re regex.Pattern) func match5(s string, re regex.Pattern) (matched bool, m1, m2, m3, m4, m5 string) { return G.res.Match5(s, re) } -func replaceFirst(s string, re regex.Pattern, repl string) string { - m, replaced := G.res.ReplaceFirst(s, re, repl) - return ifelseStr(m != nil, replaced, s) -} func replaceAll(s string, re regex.Pattern, repl string) string { return G.res.Compile(re).ReplaceAllString(s, repl) } @@ -67,6 +65,22 @@ func replaceAllFunc(s string, re regex.P return G.res.Compile(re).ReplaceAllStringFunc(s, repl) } +// trimHspace returns str, with leading and trailing space (U+0020) +// and tab (U+0009) removed. +// +// It is simpler and faster than strings.TrimSpace. +func trimHspace(str string) string { + start := 0 + end := len(str) + for start < end && (str[start] == ' ' || str[start] == '\t') { + start++ + } + for start < end && (str[end-1] == ' ' || str[end-1] == '\t') { + end-- + } + return str[start:end] +} + func ifelseStr(cond bool, a, b string) string { if cond { return a @@ -261,47 +275,20 @@ func defineVar(mkline MkLine, varname st } } -// varIsDefined tests whether the variable (or its canonicalized form) +// varIsDefinedSimilar tests whether the variable (or its canonicalized form) // is defined in the current package or in the current file. -// TODO: rename to similar -func varIsDefined(varname string) bool { - return G.Mk != nil && G.Mk.vars.DefinedSimilar(varname) || +func varIsDefinedSimilar(varname string) bool { + return G.Mk != nil && (G.Mk.vars.DefinedSimilar(varname) || G.Mk.forVars[varname]) || G.Pkg != nil && G.Pkg.vars.DefinedSimilar(varname) } -// varIsUsed tests whether the variable (or its canonicalized form) +// varIsUsedSimilar tests whether the variable (or its canonicalized form) // is used in the current package or in the current file. -// TODO: rename to similar -func varIsUsed(varname string) bool { +func varIsUsedSimilar(varname string) bool { return G.Mk != nil && G.Mk.vars.UsedSimilar(varname) || G.Pkg != nil && G.Pkg.vars.UsedSimilar(varname) } -func splitOnSpace(s string) []string { - i := 0 - n := len(s) - - for i < n && unicode.IsSpace(rune(s[i])) { - i++ - } - - var parts []string - for i < n { - start := i - for i < n && !unicode.IsSpace(rune(s[i])) { - i++ - } - if start != i { - parts = append(parts, s[start:i]) - } - for i < n && unicode.IsSpace(rune(s[i])) { - i++ - } - } - - return parts -} - func fileExists(fname string) bool { st, err := os.Stat(fname) return err == nil && st.Mode().IsRegular() @@ -312,14 +299,6 @@ func dirExists(fname string) bool { return err == nil && st.Mode().IsDir() } -// Useful in combination with regex.Find*Index -func negToZero(i int) int { - if i >= 0 { - return i - } - return 0 -} - func toInt(s string, def int) int { if n, err := strconv.Atoi(s); err == nil { return n @@ -729,13 +708,16 @@ func (s *RedundantScope) Handle(mkline M } } +// IsPrefs returns whether the given file, when included, loads the user +// preferences. func IsPrefs(fileName string) bool { switch path.Base(fileName) { - case "bsd.prefs.mk", - "bsd.fast.prefs.mk", - "bsd.builtin.mk", // mk/buildlink3/bsd.builtin.mk - "pkgconfig-builtin.mk", - "bsd.options.mk": + case // See https://github.com/golang/go/issues/28057 + "bsd.prefs.mk", // in mk/ + "bsd.fast.prefs.mk", // in mk/ + "bsd.builtin.mk", // in mk/buildlink3/ + "pkgconfig-builtin.mk", // in mk/buildlink3/ + "bsd.options.mk": // in mk/ return true } return false @@ -760,10 +742,10 @@ type FileCache struct { } type fileCacheEntry struct { - count int - fileName string - options LoadOptions - lines []Line + count int + key string + options LoadOptions + lines []Line } func NewFileCache(size int) *FileCache { @@ -780,32 +762,52 @@ func (c *FileCache) Put(fileName string, entry := c.mapping[key] if entry == nil { if len(c.table) == cap(c.table) { - sort.Slice(c.table, func(i, j int) bool { return c.table[j].count < c.table[i].count }) - minCount := c.table[len(c.table)-1].count - newLen := len(c.table) - for newLen > 0 && c.table[newLen-1].count == minCount { - delete(c.mapping, c.key(c.table[newLen-1].fileName)) - newLen-- - } - c.table = c.table[0:newLen] - - // To avoid files from getting stuck in the cache. - for _, e := range c.table { - e.count /= 2 - } + c.removeOldEntries() } - entry = &fileCacheEntry{0, fileName, options, lines} + entry = new(fileCacheEntry) c.table = append(c.table, entry) c.mapping[key] = entry } - entry.count = 0 - entry.fileName = fileName + entry.count = 1 + entry.key = key entry.options = options entry.lines = lines } +func (c *FileCache) removeOldEntries() { + printStats := func() bool { return false }() + + sort.Slice(c.table, func(i, j int) bool { return c.table[j].count < c.table[i].count }) + + if printStats { + for _, e := range c.table { + G.logOut.Printf("FileCache %q with count %d.\n", e.key, e.count) + } + } + + minCount := c.table[len(c.table)-1].count + newLen := len(c.table) + for newLen > 0 && c.table[newLen-1].count == minCount { + e := c.table[newLen-1] + if printStats { + G.logOut.Printf("FileCache.Evict %q with count %d.\n", e.key, e.count) + } + delete(c.mapping, e.key) + newLen-- + } + c.table = c.table[0:newLen] + + // To avoid files getting stuck in the cache. + for _, e := range c.table { + if printStats { + G.logOut.Printf("FileCache.Halve %q with count %d.\n", e.key, e.count) + } + e.count /= 2 + } +} + func (c *FileCache) Get(fileName string, options LoadOptions) []Line { key := c.key(fileName) entry, found := c.mapping[key] Index: pkgsrc/pkgtools/pkglint/files/shell_test.go diff -u pkgsrc/pkgtools/pkglint/files/shell_test.go:1.31 pkgsrc/pkgtools/pkglint/files/shell_test.go:1.32 --- pkgsrc/pkgtools/pkglint/files/shell_test.go:1.31 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/shell_test.go Tue Oct 9 19:12:13 2018 @@ -137,10 +137,10 @@ func (s *Suite) Test_ShellLine_CheckShel t.SetupCommandLine("-Wall") t.SetupVartypes() - t.SetupToolUsable("awk", "AWK") - t.SetupToolUsable("cp", "CP") - t.SetupToolUsable("mkdir", "MKDIR") // This is actually "mkdir -p". - t.SetupToolUsable("unzip", "UNZIP_CMD") + t.SetupTool("awk", "AWK", AtRunTime) + t.SetupTool("cp", "CP", AtRunTime) + t.SetupTool("mkdir", "MKDIR", AtRunTime) // This is actually "mkdir -p". + t.SetupTool("unzip", "UNZIP_CMD", AtRunTime) checkShellCommandLine := func(shellCommand string) { G.Mk = t.NewMkLines("fname", @@ -164,7 +164,7 @@ func (s *Suite) Test_ShellLine_CheckShel "WARN: fname:1: Unknown shell command \"echo\".", "WARN: fname:1: Unknown shell command \"echo\".") - t.SetupToolUsable("echo", "") + t.SetupTool("echo", "", AtRunTime) t.SetupVartypes() checkShellCommandLine("echo ${PKGNAME:Q}") // vucQuotPlain @@ -323,7 +323,7 @@ func (s *Suite) Test_ShellLine_CheckShel t.SetupCommandLine("-Wall") t.SetupVartypes() - t.SetupToolUsable("echo", "") + t.SetupTool("echo", "", AtRunTime) G.Mk = t.NewMkLines("Makefile", "\techo ${PKGNAME:Q}") shline := NewShellLine(G.Mk.mklines[0]) @@ -339,7 +339,7 @@ func (s *Suite) Test_ShellLine_CheckShel t.SetupCommandLine("-Wall", "--show-autofix") t.SetupVartypes() - t.SetupToolUsable("echo", "") + t.SetupTool("echo", "", AtRunTime) G.Mk = t.NewMkLines("Makefile", "\techo ${PKGNAME:Q}") shline := NewShellLine(G.Mk.mklines[0]) @@ -356,11 +356,11 @@ func (s *Suite) Test_ShellProgramChecker t.SetupCommandLine("-Wall") t.SetupVartypes() - t.SetupToolUsable("cat", "") - t.SetupToolUsable("echo", "") - t.SetupToolUsable("printf", "") - t.SetupToolUsable("sed", "") - t.SetupToolUsable("right-side", "") + t.SetupTool("cat", "", AtRunTime) + t.SetupTool("echo", "", AtRunTime) + t.SetupTool("printf", "", AtRunTime) + t.SetupTool("sed", "", AtRunTime) + t.SetupTool("right-side", "", AtRunTime) G.Mk = t.NewMkLines("Makefile", "\t echo | right-side", "\t sed s,s,s, | right-side", @@ -394,7 +394,7 @@ func (s *Suite) Test_ShellLine_CheckShel t.SetupCommandLine("-Wall", "--autofix") t.SetupVartypes() - t.SetupToolUsable("echo", "") + t.SetupTool("echo", "", AtRunTime) G.Mk = t.NewMkLines("Makefile", "\techo ${PKGNAME:Q}") shline := NewShellLine(G.Mk.mklines[0]) @@ -438,7 +438,7 @@ func (s *Suite) Test_ShellLine_CheckShel t := s.Init(c) t.SetupVartypes() - t.SetupToolUsable("pax", "") + t.SetupTool("pax", "", AtRunTime) G.Mk = t.NewMkLines("fname", "# dummy") shline := NewShellLine(G.Mk.mklines[0]) @@ -614,7 +614,7 @@ func (s *Suite) Test_ShellLine_variableN t.SetupCommandLine("-Wall") t.SetupVartypes() - t.SetupToolUsable("cp", "") + t.SetupTool("cp", "", AtRunTime) mklines := t.NewMkLines("fname.mk", MkRcsID, "", @@ -636,7 +636,7 @@ func (s *Suite) Test_ShellLine_CheckShel t := s.Init(c) t.SetupCommandLine("-Wall") - echo := t.SetupToolUsable("echo", "ECHO") + echo := t.SetupTool("echo", "ECHO", AtRunTime) echo.MustUseVarForm = true G.Mk = t.NewMkLines("fname", "# dummy") @@ -679,7 +679,7 @@ func (s *Suite) Test_ShellLine_CheckShel "WARN: Makefile:3: Please use the RCD_SCRIPTS mechanism to install rc.d scripts automatically to ${RCD_SCRIPTS_EXAMPLEDIR}.") } -func (s *Suite) Test_ShellLine_checkCommandUse(c *check.C) { +func (s *Suite) Test_ShellLine_checkInstallCommand(c *check.C) { t := s.Init(c) G.Mk = t.NewMkLines("fname", @@ -688,12 +688,12 @@ func (s *Suite) Test_ShellLine_checkComm shline := t.NewShellLine("fname", 1, "\tdummy") - shline.checkCommandUse("sed") + shline.checkInstallCommand("sed") t.CheckOutputLines( "WARN: fname:1: The shell command \"sed\" should not be used in the install phase.") - shline.checkCommandUse("cp") + shline.checkInstallCommand("cp") t.CheckOutputLines( "WARN: fname:1: ${CP} should not be used to install files.") @@ -950,8 +950,8 @@ func (s *Suite) Test_SimpleCommandChecke func (s *Suite) Test_SimpleCommandChecker_handleCommandVariable(c *check.C) { t := s.Init(c) - t.SetupToolUsable("perl", "PERL5") - t.SetupTool("perl6", "PERL6") + t.SetupTool("perl", "PERL5", AtRunTime) + t.SetupTool("perl6", "PERL6", Nowhere) mklines := t.NewMkLines("Makefile", MkRcsID, "", @@ -960,13 +960,10 @@ func (s *Suite) Test_SimpleCommandChecke mklines.Check() - // FIXME: Warn about using _PERL5_VARS because it starts with an underscore. - // FIXME: Omit the redundant PERL5_VARS_CMD warning in line 4. // FIXME: In PERL5:Q and PERL6:Q, the :Q is wrong. t.CheckOutputLines( "WARN: Makefile:3: PERL5_VARS_CMD is defined but not used.", - "WARN: Makefile:4: The \"perl6\" tool is used but not added to USE_TOOLS.", - "WARN: Makefile:4: PERL5_VARS_CMD is defined but not used.") + "WARN: Makefile:4: The \"perl6\" tool is used but not added to USE_TOOLS.") } // The package Makefile and other .mk files in a package directory @@ -1088,9 +1085,9 @@ func (s *Suite) Test_ShellProgramChecker t := s.Init(c) t.SetupCommandLine("-Wall") - t.SetupToolUsable("echo", "") - t.SetupToolUsable("rm", "") - t.SetupToolUsable("touch", "") + t.SetupTool("echo", "", AtRunTime) + t.SetupTool("rm", "", AtRunTime) + t.SetupTool("touch", "", AtRunTime) mklines := t.NewMkLines("Makefile", MkRcsID, "pre-configure:", @@ -1108,8 +1105,8 @@ func (s *Suite) Test_ShellProgramChecker t := s.Init(c) t.SetupCommandLine("-Wall") - t.SetupToolUsable("echo", "") - t.SetupToolUsable("touch", "") + t.SetupTool("echo", "", AtRunTime) + t.SetupTool("touch", "", AtRunTime) mklines := t.NewMkLines("Makefile", MkRcsID, "pre-configure:", @@ -1127,12 +1124,12 @@ func (s *Suite) Test_ShellProgramChecker t.SetupCommandLine("-Wall") t.SetupVartypes() - t.SetupToolUsable("echo", "") - t.SetupToolUsable("grep", "GREP") - t.SetupToolUsable("sed", "") - t.SetupToolUsable("touch", "") - t.SetupToolUsable("tr", "tr") - t.SetupToolUsable("true", "TRUE") + t.SetupTool("echo", "", AtRunTime) + t.SetupTool("grep", "GREP", AtRunTime) + t.SetupTool("sed", "", AtRunTime) + t.SetupTool("touch", "", AtRunTime) + t.SetupTool("tr", "tr", AtRunTime) + t.SetupTool("true", "TRUE", AtRunTime) mklines := t.NewMkLines("Makefile", MkRcsID, "pre-configure:", Index: pkgsrc/pkgtools/pkglint/files/substcontext.go diff -u pkgsrc/pkgtools/pkglint/files/substcontext.go:1.13 pkgsrc/pkgtools/pkglint/files/substcontext.go:1.14 --- pkgsrc/pkgtools/pkglint/files/substcontext.go:1.13 Wed Sep 5 17:56:22 2018 +++ pkgsrc/pkgtools/pkglint/files/substcontext.go Tue Oct 9 19:12:13 2018 @@ -54,7 +54,7 @@ func (ctx *SubstContext) Varassign(mklin op := mkline.Op() value := mkline.Value() if varcanon == "SUBST_CLASSES" || varcanon == "SUBST_CLASSES.*" { - classes := splitOnSpace(value) + classes := fields(value) if len(classes) > 1 { mkline.Warnf("Please add only one class at a time to SUBST_CLASSES.") } Index: pkgsrc/pkgtools/pkglint/files/util_test.go diff -u pkgsrc/pkgtools/pkglint/files/util_test.go:1.14 pkgsrc/pkgtools/pkglint/files/util_test.go:1.15 --- pkgsrc/pkgtools/pkglint/files/util_test.go:1.14 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/util_test.go Tue Oct 9 19:12:13 2018 @@ -220,13 +220,6 @@ func emptyToNil(slice []string) []string return slice } -func (s *Suite) Test_splitOnSpace(c *check.C) { - c.Check(splitOnSpace("aaaaa"), deepEquals, []string{"aaaaa"}) - c.Check(splitOnSpace(" a b\tc\n"), deepEquals, []string{"a", "b", "c"}) - c.Check(splitOnSpace(" "), check.IsNil) - c.Check(splitOnSpace(""), check.IsNil) -} - func (s *Suite) Test_isLocallyModified(c *check.C) { t := s.Init(c) Index: pkgsrc/pkgtools/pkglint/files/vardefs.go diff -u pkgsrc/pkgtools/pkglint/files/vardefs.go:1.46 pkgsrc/pkgtools/pkglint/files/vardefs.go:1.47 --- pkgsrc/pkgtools/pkglint/files/vardefs.go:1.46 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/vardefs.go Tue Oct 9 19:12:13 2018 @@ -41,6 +41,14 @@ func (src *Pkgsrc) InitVartypes() { "Makefile.*, *.mk: default, set, use") } + // pkgload is the same as pkg, except that the variable may be accessed at load time. + pkgload := func(varname string, kindOfList KindOfList, checker *BasicType) { + acl(varname, kindOfList, checker, ""+ + "Makefile: set, use, use-loadtime; "+ + "buildlink3.mk, builtin.mk:; "+ + "Makefile.*, *.mk: default, set, use, use-loadtime") + } + // A package-defined list may be appended to in all Makefiles except buildlink3.mk and builtin.mk. // Simple assignment (instead of appending) is only allowed in Makefile and Makefile.common. pkglist := func(varname string, kindOfList KindOfList, checker *BasicType) { @@ -57,9 +65,17 @@ func (src *Pkgsrc) InitVartypes() { sys := func(varname string, kindOfList KindOfList, checker *BasicType) { acl(varname, kindOfList, checker, "buildlink3.mk:; *: use") } + usr := func(varname string, kindOfList KindOfList, checker *BasicType) { acl(varname, kindOfList, checker, "buildlink3.mk:; *: use-loadtime, use") } + + // sysload defines a system-provided variable that may already be used + // at load time. + sysload := func(varname string, kindOfList KindOfList, checker *BasicType) { + acl(varname, kindOfList, checker, "*: use-loadtime, use") + } + bl3list := func(varname string, kindOfList KindOfList, checker *BasicType) { acl(varname, kindOfList, checker, "buildlink3.mk, builtin.mk: append") } @@ -74,7 +90,7 @@ func (src *Pkgsrc) InitVartypes() { if mklines != nil { for _, mkline := range mklines.mklines { if mkline.IsDirective() && mkline.Directive() == "for" { - words := splitOnSpace(mkline.Args()) + words := fields(mkline.Args()) if len(words) > 2 && words[0] == "_version_" { for _, word := range words[2:] { languages[word] = true @@ -83,7 +99,7 @@ func (src *Pkgsrc) InitVartypes() { } } } - for _, language := range splitOnSpace("ada c c99 c++ c++11 fortran fortran77 java objc obj-c++") { + for _, language := range [...]string{"ada", "c", "c99", "c++", "c++11", "fortran", "fortran77", "java", "objc", "obj-c++"} { languages[language] = true } @@ -186,6 +202,7 @@ func (src *Pkgsrc) InitVartypes() { usr("PKGSRC_USE_FORTIFY", lkNone, BtYesNo) usr("PKGSRC_USE_RELRO", lkNone, BtYesNo) usr("PKGSRC_USE_SSP", lkNone, enum("no yes strong all")) + usr("PREFER.*", lkNone, enum("pkgsrc native")) usr("PREFER_PKGSRC", lkShell, BtIdentifier) usr("PREFER_NATIVE", lkShell, BtIdentifier) usr("PREFER_NATIVE_PTHREADS", lkNone, BtYesNo) @@ -193,7 +210,7 @@ func (src *Pkgsrc) InitVartypes() { usr("LOCALBASE", lkNone, BtPathname) usr("CROSSBASE", lkNone, BtPathname) usr("VARBASE", lkNone, BtPathname) - usr("X11_TYPE", lkNone, enum("modular native")) + acl("X11_TYPE", lkNone, enum("modular native"), "*: use-loadtime, use") usr("X11BASE", lkNone, BtPathname) usr("MOTIFBASE", lkNone, BtPathname) usr("PKGINFODIR", lkNone, BtPathname) @@ -469,6 +486,7 @@ func (src *Pkgsrc) InitVartypes() { acl("BDB_DEFAULT", lkNone, enum("db1 db2 db3 db4 db5 db6"), "") sys("BDB_LIBS", lkShell, BtLdFlag) sys("BDB_TYPE", lkNone, enum("db1 db2 db3 db4 db5 db6")) + sys("BIGENDIANPLATFORMS", lkSpace, BtMachinePlatformPattern) sys("BINGRP", lkNone, BtUserGroupName) sys("BINMODE", lkNone, BtFileMode) sys("BINOWN", lkNone, BtUserGroupName) @@ -476,11 +494,11 @@ func (src *Pkgsrc) InitVartypes() { pkg("BOOTSTRAP_PKG", lkNone, BtYesNo) acl("BROKEN", lkNone, BtMessage, "") pkg("BROKEN_GETTEXT_DETECTION", lkNone, BtYesNo) - pkglist("BROKEN_EXCEPT_ON_PLATFORM", lkShell, BtMachinePlatformPattern) + pkglist("BROKEN_EXCEPT_ON_PLATFORM", lkSpace, BtMachinePlatformPattern) pkglist("BROKEN_ON_PLATFORM", lkSpace, BtMachinePlatformPattern) sys("BSD_MAKE_ENV", lkShell, BtShellWord) - acl("BUILDLINK_ABI_DEPENDS.*", lkSpace, BtDependency, "builtin.mk: append, use-loadtime; *: append") - acl("BUILDLINK_API_DEPENDS.*", lkSpace, BtDependency, "builtin.mk: append, use-loadtime; *: append") + acl("BUILDLINK_ABI_DEPENDS.*", lkSpace, BtDependency, "buildlink3.mk, builtin.mk: append, use-loadtime; *: append") + acl("BUILDLINK_API_DEPENDS.*", lkSpace, BtDependency, "buildlink3.mk, builtin.mk: append, use-loadtime; *: append") acl("BUILDLINK_AUTO_DIRS.*", lkNone, BtYesNo, "buildlink3.mk: append") acl("BUILDLINK_CONTENTS_FILTER", lkNone, BtShellCommand, "") sys("BUILDLINK_CFLAGS", lkShell, BtCFlag) @@ -510,6 +528,7 @@ func (src *Pkgsrc) InitVartypes() { acl("BUILDLINK_TRANSFORM", lkShell, BtWrapperTransform, "*: append") acl("BUILDLINK_TRANSFORM.*", lkShell, BtWrapperTransform, "*: append") acl("BUILDLINK_TREE", lkShell, BtIdentifier, "buildlink3.mk: append") + acl("BUILDLINK_X11_DIR", lkNone, BtPathname, "*: use") acl("BUILD_DEFS", lkShell, BtVariableName, "Makefile, Makefile.common, options.mk: append") pkglist("BUILD_DEFS_EFFECTS", lkShell, BtVariableName) acl("BUILD_DEPENDS", lkSpace, BtDependencyWithPath, "Makefile, Makefile.common, *.mk: append") @@ -533,12 +552,12 @@ func (src *Pkgsrc) InitVartypes() { sys("BUILTIN_X11_TYPE", lkNone, BtUnknown) sys("BUILTIN_X11_VERSION", lkNone, BtUnknown) acl("CATEGORIES", lkShell, BtCategory, "Makefile: set, append; Makefile.common: set, default, append") - sys("CC_VERSION", lkNone, BtMessage) - sys("CC", lkNone, BtShellCommand) + sysload("CC_VERSION", lkNone, BtMessage) + sysload("CC", lkNone, BtShellCommand) pkglist("CFLAGS", lkShell, BtCFlag) // may also be changed by the user pkglist("CFLAGS.*", lkShell, BtCFlag) // may also be changed by the user acl("CHECK_BUILTIN", lkNone, BtYesNo, "builtin.mk: default; Makefile: set") - acl("CHECK_BUILTIN.*", lkNone, BtYesNo, "Makefile, options.mk, buildlink3.mk: set; builtin.mk: default; *: use-loadtime") + acl("CHECK_BUILTIN.*", lkNone, BtYesNo, "Makefile, options.mk, buildlink3.mk: set; builtin.mk: default, use-loadtime; *: use-loadtime") acl("CHECK_FILES_SKIP", lkShell, BtBasicRegularExpression, "Makefile, Makefile.common: append") pkg("CHECK_FILES_SUPPORTED", lkNone, BtYesNo) usr("CHECK_HEADERS", lkNone, BtYesNo) @@ -559,7 +578,14 @@ func (src *Pkgsrc) InitVartypes() { pkg("CMAKE_ARG_PATH", lkNone, BtPathname) pkglist("CMAKE_ARGS", lkShell, BtShellWord) pkglist("CMAKE_ARGS.*", lkShell, BtShellWord) + pkglist("CMAKE_DEPENDENCIES_REWRITE", lkShell, BtPathmask) // Relative to WRKSRC + pkglist("CMAKE_MODULE_PATH_OVERRIDE", lkShell, BtPathmask) // Relative to WRKSRC + pkg("CMAKE_PKGSRC_BUILD_FLAGS", lkNone, BtYesNo) + pkglist("CMAKE_PREFIX_PATH", lkShell, BtPathmask) + pkglist("CMAKE_USE_GNU_INSTALL_DIRS", lkNone, BtYesNo) + pkg("CMAKE_INSTALL_PREFIX", lkNone, BtPathname) // The default is ${PREFIX}. acl("COMMENT", lkNone, BtComment, "Makefile, Makefile.common: set, append") + sys("COMPILE.*", lkNone, BtShellCommand) acl("COMPILER_RPATH_FLAG", lkNone, enum("-Wl,-rpath"), "*: use") pkglist("CONFIGURE_ARGS", lkShell, BtShellWord) pkglist("CONFIGURE_ARGS.*", lkShell, BtShellWord) @@ -586,6 +612,7 @@ func (src *Pkgsrc) InitVartypes() { pkglist("CXXFLAGS", lkShell, BtCFlag) pkglist("CXXFLAGS.*", lkShell, BtCFlag) pkglist("CWRAPPERS_APPEND.*", lkShell, BtShellWord) + sys("DEFAULT_DISTFILES", lkShell, BtFetchURL) // From mk/fetch/bsd.fetch-vars.mk. acl("DEINSTALL_FILE", lkNone, BtPathname, "Makefile: set") acl("DEINSTALL_SRC", lkShell, BtPathname, "Makefile: set; Makefile.common: default, set") acl("DEINSTALL_TEMPLATES", lkShell, BtPathname, "Makefile: set, append; Makefile.common: set, default, append") @@ -650,8 +677,8 @@ func (src *Pkgsrc) InitVartypes() { sys("EMUL_OPSYS", lkNone, enum("darwin freebsd hpux irix linux osf1 solaris sunos none")) pkg("EMUL_PKG_FMT", lkNone, enum("plain rpm")) usr("EMUL_PLATFORM", lkNone, BtEmulPlatform) - pkg("EMUL_PLATFORMS", lkShell, BtEmulPlatform) - usr("EMUL_PREFER", lkShell, BtEmulPlatform) + pkg("EMUL_PLATFORMS", lkSpace, BtEmulPlatform) + usr("EMUL_PREFER", lkSpace, BtEmulPlatform) pkg("EMUL_REQD", lkSpace, BtDependency) usr("EMUL_TYPE.*", lkNone, enum("native builtin suse suse-9.1 suse-9.x suse-10.0 suse-10.x")) sys("ERROR_CAT", lkNone, BtShellCommand) @@ -688,6 +715,8 @@ func (src *Pkgsrc) InitVartypes() { acl("FONTS_DIRS.*", lkShell, BtPathname, "Makefile: set, append, use; Makefile.common: append, use") sys("GAMEDATAMODE", lkNone, BtFileMode) sys("GAMES_GROUP", lkNone, BtUserGroupName) + sys("GAMEDATA_PERMS", lkShell, BtPerms) + sys("GAMEDIR_PERMS", lkShell, BtPerms) sys("GAMEMODE", lkNone, BtFileMode) sys("GAMES_USER", lkNone, BtUserGroupName) pkglist("GCC_REQD", lkShell, BtVersion) @@ -756,6 +785,7 @@ func (src *Pkgsrc) InitVartypes() { sys("LD", lkNone, BtShellCommand) pkglist("LDFLAGS", lkShell, BtLdFlag) pkglist("LDFLAGS.*", lkShell, BtLdFlag) + sysload("LIBABISUFFIX", lkNone, BtIdentifier) // Can also be empty. sys("LIBGRP", lkNone, BtUserGroupName) sys("LIBMODE", lkNone, BtFileMode) sys("LIBOWN", lkNone, BtUserGroupName) @@ -768,16 +798,19 @@ func (src *Pkgsrc) InitVartypes() { acl("LICENCE", lkNone, BtLicense, "Makefile, Makefile.common, options.mk: set, append") acl("LICENSE", lkNone, BtLicense, "Makefile, Makefile.common, options.mk: set, append") pkg("LICENSE_FILE", lkNone, BtPathname) + sys("LINK.*", lkNone, BtShellCommand) sys("LINKER_RPATH_FLAG", lkNone, BtShellWord) + sys("LITTLEENDIANPLATFORMS", lkSpace, BtMachinePlatformPattern) sys("LOWER_OPSYS", lkNone, BtIdentifier) sys("LOWER_VENDOR", lkNone, BtIdentifier) + sys("LP64PLATFORMS", lkSpace, BtMachinePlatformPattern) acl("LTCONFIG_OVERRIDE", lkShell, BtPathmask, "Makefile: set, append; Makefile.common: append") - sys("MACHINE_ARCH", lkNone, enumMachineArch) - sys("MACHINE_GNU_ARCH", lkNone, enumMachineGnuArch) - sys("MACHINE_GNU_PLATFORM", lkNone, BtMachineGnuPlatform) - sys("MACHINE_PLATFORM", lkNone, BtMachinePlatform) + sysload("MACHINE_ARCH", lkNone, enumMachineArch) + sysload("MACHINE_GNU_ARCH", lkNone, enumMachineGnuArch) + sysload("MACHINE_GNU_PLATFORM", lkNone, BtMachineGnuPlatform) + sysload("MACHINE_PLATFORM", lkNone, BtMachinePlatform) acl("MAINTAINER", lkNone, BtMailAddress, "Makefile: set; Makefile.common: default") - sys("MAKE", lkNone, BtShellCommand) + sysload("MAKE", lkNone, BtShellCommand) pkglist("MAKEFLAGS", lkShell, BtShellWord) acl("MAKEVARS", lkShell, BtVariableName, "buildlink3.mk, builtin.mk, hacks.mk: append") pkglist("MAKE_DIRS", lkShell, BtPathname) @@ -798,6 +831,7 @@ func (src *Pkgsrc) InitVartypes() { pkglist("MASTER_SITES", lkShell, BtFetchURL) sys("MASTER_SITE_APACHE", lkShell, BtFetchURL) sys("MASTER_SITE_BACKUP", lkShell, BtFetchURL) + sys("MASTER_SITE_CRATESIO", lkShell, BtFetchURL) sys("MASTER_SITE_CYGWIN", lkShell, BtFetchURL) sys("MASTER_SITE_DEBIAN", lkShell, BtFetchURL) sys("MASTER_SITE_FREEBSD", lkShell, BtFetchURL) @@ -816,9 +850,12 @@ func (src *Pkgsrc) InitVartypes() { sys("MASTER_SITE_MOZILLA_ESR", lkShell, BtFetchURL) sys("MASTER_SITE_MYSQL", lkShell, BtFetchURL) sys("MASTER_SITE_NETLIB", lkShell, BtFetchURL) + sys("MASTER_SITE_OPENBSD", lkShell, BtFetchURL) sys("MASTER_SITE_OPENOFFICE", lkShell, BtFetchURL) sys("MASTER_SITE_OSDN", lkShell, BtFetchURL) sys("MASTER_SITE_PERL_CPAN", lkShell, BtFetchURL) + sys("MASTER_SITE_PGSQL", lkShell, BtFetchURL) + sys("MASTER_SITE_PYPI", lkShell, BtFetchURL) sys("MASTER_SITE_R_CRAN", lkShell, BtFetchURL) sys("MASTER_SITE_RUBYGEMS", lkShell, BtFetchURL) sys("MASTER_SITE_SOURCEFORGE", lkShell, BtFetchURL) @@ -827,12 +864,14 @@ func (src *Pkgsrc) InitVartypes() { sys("MASTER_SITE_TEX_CTAN", lkShell, BtFetchURL) sys("MASTER_SITE_XCONTRIB", lkShell, BtFetchURL) sys("MASTER_SITE_XEMACS", lkShell, BtFetchURL) + sys("MASTER_SITE_XORG", lkShell, BtFetchURL) pkglist("MESSAGE_SRC", lkShell, BtPathname) acl("MESSAGE_SUBST", lkShell, BtShellWord, "Makefile, Makefile.common, options.mk: append") pkg("META_PACKAGE", lkNone, BtYes) sys("MISSING_FEATURES", lkShell, BtIdentifier) acl("MYSQL_VERSIONS_ACCEPTED", lkShell, mysqlVersions, "Makefile: set") usr("MYSQL_VERSION_DEFAULT", lkNone, BtVersion) + sys("NATIVE_CC", lkNone, BtShellCommand) // See mk/platform/tools.NetBSD.mk (and some others). sys("NM", lkNone, BtShellCommand) sys("NONBINMODE", lkNone, BtFileMode) pkg("NOT_FOR_COMPILER", lkShell, compilers) @@ -852,13 +891,15 @@ func (src *Pkgsrc) InitVartypes() { acl("NO_PKGTOOLS_REQD_CHECK", lkNone, BtYes, "Makefile: set") acl("NO_SRC_ON_CDROM", lkNone, BtRestricted, "Makefile, Makefile.common: set") acl("NO_SRC_ON_FTP", lkNone, BtRestricted, "Makefile, Makefile.common: set") + sysload("OBJECT_FMT", lkNone, enum("COFF ECOFF ELF SOM XCOFF Mach-O PE a.out")) pkglist("ONLY_FOR_COMPILER", lkShell, compilers) pkglist("ONLY_FOR_PLATFORM", lkSpace, BtMachinePlatformPattern) pkg("ONLY_FOR_UNPRIVILEGED", lkNone, BtYesNo) - sys("OPSYS", lkNone, BtIdentifier) + sysload("OPSYS", lkNone, BtIdentifier) acl("OPSYSVARS", lkShell, BtVariableName, "Makefile, Makefile.common: append") acl("OSVERSION_SPECIFIC", lkNone, BtYes, "Makefile, Makefile.common: set") - sys("OS_VERSION", lkNone, BtVersion) + sysload("OS_VERSION", lkNone, BtVersion) + sysload("OSX_VERSION", lkNone, BtVersion) // See mk/platform/Darwin.mk. pkg("OVERRIDE_DIRDEPTH*", lkNone, BtInteger) pkg("OVERRIDE_GNU_CONFIG_SCRIPTS", lkNone, BtYes) acl("OWNER", lkNone, BtMailAddress, "Makefile: set; Makefile.common: default") @@ -874,9 +915,27 @@ func (src *Pkgsrc) InitVartypes() { acl("PATCH_DIST_STRIP*", lkNone, BtShellWord, "buildlink3.mk, builtin.mk:; Makefile, Makefile.common, *.mk: set") acl("PATCH_SITES", lkShell, BtFetchURL, "Makefile, Makefile.common, options.mk: set") acl("PATCH_STRIP", lkNone, BtShellWord, "") + sys("PATH", lkNone, BtPathlist) // From the PATH environment variable. + sys("PAXCTL", lkNone, BtShellCommand) // See mk/pax.mk. acl("PERL5_PACKLIST", lkShell, BtPerl5Packlist, "Makefile: set; options.mk: set, append") acl("PERL5_PACKLIST_DIR", lkNone, BtPathname, "") pkg("PERL5_REQD", lkShell, BtVersion) + sys("PERL5_INSTALLARCHLIB", lkNone, BtPathname) // See lang/perl5/vars.mk + sys("PERL5_INSTALLSCRIPT", lkNone, BtPathname) + sys("PERL5_INSTALLVENDORBIN", lkNone, BtPathname) + sys("PERL5_INSTALLVENDORSCRIPT", lkNone, BtPathname) + sys("PERL5_INSTALLVENDORARCH", lkNone, BtPathname) + sys("PERL5_INSTALLVENDORLIB", lkNone, BtPathname) + sys("PERL5_INSTALLVENDORMAN1DIR", lkNone, BtPathname) + sys("PERL5_INSTALLVENDORMAN3DIR", lkNone, BtPathname) + sys("PERL5_SUB_INSTALLARCHLIB", lkNone, BtPrefixPathname) // See lang/perl5/vars.mk + sys("PERL5_SUB_INSTALLSCRIPT", lkNone, BtPrefixPathname) + sys("PERL5_SUB_INSTALLVENDORBIN", lkNone, BtPrefixPathname) + sys("PERL5_SUB_INSTALLVENDORSCRIPT", lkNone, BtPrefixPathname) + sys("PERL5_SUB_INSTALLVENDORARCH", lkNone, BtPrefixPathname) + sys("PERL5_SUB_INSTALLVENDORLIB", lkNone, BtPrefixPathname) + sys("PERL5_SUB_INSTALLVENDORMAN1DIR", lkNone, BtPrefixPathname) + sys("PERL5_SUB_INSTALLVENDORMAN3DIR", lkNone, BtPrefixPathname) pkg("PERL5_USE_PACKLIST", lkNone, BtYesNo) sys("PGSQL_PREFIX", lkNone, BtPathname) acl("PGSQL_VERSIONS_ACCEPTED", lkShell, pgsqlVersions, "") @@ -895,18 +954,21 @@ func (src *Pkgsrc) InitVartypes() { sys("PKGLOCALEDIR", lkNone, BtPathname) pkg("PKGNAME", lkNone, BtPkgName) sys("PKGNAME_NOREV", lkNone, BtPkgName) - sys("PKGPATH", lkNone, BtPathname) + sysload("PKGPATH", lkNone, BtPathname) acl("PKGREPOSITORY", lkNone, BtUnknown, "") acl("PKGREVISION", lkNone, BtPkgRevision, "Makefile: set") sys("PKGSRCDIR", lkNone, BtPathname) acl("PKGSRCTOP", lkNone, BtYes, "Makefile: set") + sys("PKGSRC_SETENV", lkNone, BtShellCommand) acl("PKGTOOLS_ENV", lkShell, BtShellWord, "") sys("PKGVERSION", lkNone, BtVersion) + sys("PKGVERSION_NOREV", lkNone, BtVersion) // Without the nb* part. sys("PKGWILDCARD", lkNone, BtFilemask) sys("PKG_ADMIN", lkNone, BtShellCommand) sys("PKG_APACHE", lkNone, enum("apache24")) pkg("PKG_APACHE_ACCEPTED", lkShell, enum("apache24")) usr("PKG_APACHE_DEFAULT", lkNone, enum("apache24")) + sysload("PKG_BUILD_OPTIONS.*", lkShell, BtOption) usr("PKG_CONFIG", lkNone, BtYes) // ^^ No, this is not the popular command from GNOME, but the setting // whether the pkgsrc user wants configuration files automatically @@ -918,6 +980,7 @@ func (src *Pkgsrc) InitVartypes() { sys("PKG_DELETE", lkNone, BtShellCommand) acl("PKG_DESTDIR_SUPPORT", lkShell, enum("destdir user-destdir"), "Makefile, Makefile.common: set") pkglist("PKG_FAIL_REASON", lkShell, BtShellWord) + sysload("PKG_FORMAT", lkNone, BtIdentifier) acl("PKG_GECOS.*", lkNone, BtMessage, "Makefile: set") acl("PKG_GID.*", lkNone, BtInteger, "Makefile: set") acl("PKG_GROUPS", lkShell, BtShellWord, "Makefile: set, append") @@ -950,7 +1013,10 @@ func (src *Pkgsrc) InitVartypes() { acl("PKG_SUGGESTED_OPTIONS", lkShell, BtOption, "Makefile, Makefile.common, options.mk: set, append") acl("PKG_SUGGESTED_OPTIONS.*", lkShell, BtOption, "Makefile, Makefile.common, options.mk: set, append") acl("PKG_SUPPORTED_OPTIONS", lkShell, BtOption, "Makefile: set, append; Makefile.common: set; options.mk: set, append, use") - pkg("PKG_SYSCONFDIR*", lkNone, BtPathname) + acl("PKG_SYSCONFDIR*", lkNone, BtPathname, ""+ + "Makefile: set, use, use-loadtime; "+ + "buildlink3.mk, builtin.mk: use-loadtime; "+ + "Makefile.*, *.mk: default, set, use, use-loadtime") pkglist("PKG_SYSCONFDIR_PERMS", lkShell, BtPerms) sys("PKG_SYSCONFBASEDIR", lkNone, BtPathname) pkg("PKG_SYSCONFSUBDIR", lkNone, BtPathname) @@ -959,7 +1025,7 @@ func (src *Pkgsrc) InitVartypes() { acl("PKG_USERS", lkShell, BtShellWord, "Makefile: set, append") pkglist("PKG_USERS_VARS", lkShell, BtVariableName) acl("PKG_USE_KERBEROS", lkNone, BtYes, "Makefile, Makefile.common: set") - pkg("PLIST.*", lkNone, BtYes) + pkgload("PLIST.*", lkNone, BtYes) pkglist("PLIST_VARS", lkShell, BtIdentifier) pkglist("PLIST_SRC", lkShell, BtRelativePkgPath) pkglist("PLIST_SUBST", lkShell, BtShellWord) @@ -974,7 +1040,7 @@ func (src *Pkgsrc) InitVartypes() { sys("PTHREAD_LDFLAGS", lkShell, BtLdFlag) sys("PTHREAD_LIBS", lkShell, BtLdFlag) acl("PTHREAD_OPTS", lkShell, enum("native optional require"), "Makefile: set, append; Makefile.common, buildlink3.mk: append") - sys("PTHREAD_TYPE", lkNone, BtIdentifier) // Or "native" or "none". + sysload("PTHREAD_TYPE", lkNone, BtIdentifier) // Or "native" or "none". pkg("PY_PATCHPLIST", lkNone, BtYes) acl("PYPKGPREFIX", lkNone, enum("py27 py34 py35 py36"), "pyversion.mk: set; *: use-loadtime, use") pkg("PYTHON_FOR_BUILD_ONLY", lkNone, BtYes) @@ -988,6 +1054,8 @@ func (src *Pkgsrc) InitVartypes() { pkglist("RCD_SCRIPTS", lkShell, BtFilename) acl("RCD_SCRIPT_SRC.*", lkNone, BtPathname, "Makefile: set") acl("RCD_SCRIPT_WRK.*", lkNone, BtPathname, "Makefile: set") + usr("REAL_ROOT_USER", lkNone, BtUserGroupName) + usr("REAL_ROOT_GROUP", lkNone, BtUserGroupName) acl("REPLACE.*", lkNone, BtUnknown, "Makefile: set") pkglist("REPLACE_AWK", lkShell, BtPathmask) pkglist("REPLACE_BASH", lkShell, BtPathmask) @@ -1013,7 +1081,8 @@ func (src *Pkgsrc) InitVartypes() { sys("RUN", lkNone, BtShellCommand) sys("RUN_LDCONFIG", lkNone, BtYesNo) acl("SCRIPTS_ENV", lkShell, BtShellWord, "Makefile, Makefile.common: append") - usr("SETUID_ROOT_PERMS", lkShell, BtShellWord) + usr("SETGID_GAMES_PERMS", lkShell, BtPerms) + usr("SETUID_ROOT_PERMS", lkShell, BtPerms) pkg("SET_LIBDIR", lkNone, BtYes) sys("SHAREGRP", lkNone, BtUserGroupName) sys("SHAREMODE", lkNone, BtFileMode) @@ -1022,6 +1091,7 @@ func (src *Pkgsrc) InitVartypes() { acl("SHLIB_HANDLING", lkNone, enum("YES NO no"), "") acl("SHLIBTOOL", lkNone, BtShellCommand, "Makefile: use") acl("SHLIBTOOL_OVERRIDE", lkShell, BtPathmask, "Makefile: set, append; Makefile.common: append") + sysload("SHLIB_TYPE", lkNone, enum("COFF ECOFF ELF SOM XCOFF Mach-O PE PEwin a.out aixlib dylib none")) acl("SITES.*", lkShell, BtFetchURL, "Makefile, Makefile.common, options.mk: set, append, use") usr("SMF_PREFIS", lkNone, BtPathname) pkg("SMF_SRCDIR", lkNone, BtPathname) @@ -1058,7 +1128,7 @@ func (src *Pkgsrc) InitVartypes() { sys("TOOLS_GNU_MISSING", lkShell, BtTool) sys("TOOLS_NOOP", lkShell, BtTool) sys("TOOLS_PATH.*", lkNone, BtPathname) - sys("TOOLS_PLATFORM.*", lkNone, BtShellCommand) + sysload("TOOLS_PLATFORM.*", lkNone, BtShellCommand) sys("TOUCH_FLAGS", lkShell, BtShellWord) pkglist("UAC_REQD_EXECS", lkShell, BtPrefixPathname) acl("UNLIMIT_RESOURCES", lkShell, enum("cputime datasize memorysize stacksize"), "Makefile: set, append; Makefile.common: append") @@ -1067,8 +1137,9 @@ func (src *Pkgsrc) InitVartypes() { pkglist("UNWRAP_FILES", lkShell, BtPathmask) usr("UPDATE_TARGET", lkShell, BtIdentifier) pkg("USERGROUP_PHASE", lkNone, enum("configure build pre-install")) + usr("USER_ADDITIONAL_PKGS", lkShell, BtPkgPath) pkg("USE_BSD_MAKEFILE", lkNone, BtYes) - acl("USE_BUILTIN.*", lkNone, BtYesNoIndirectly, "builtin.mk: set") + acl("USE_BUILTIN.*", lkNone, BtYesNoIndirectly, "buildlink3.mk: use-loadtime; builtin.mk: set, use, use-loadtime; options.mk: use-loadtime") pkg("USE_CMAKE", lkNone, BtYes) usr("USE_DESTDIR", lkNone, BtYes) pkglist("USE_FEATURES", lkShell, BtIdentifier) @@ -1088,17 +1159,19 @@ func (src *Pkgsrc) InitVartypes() { pkg("USE_PKGINSTALL", lkNone, BtYes) pkg("USE_PKGLOCALEDIR", lkNone, BtYesNo) usr("USE_PKGSRC_GCC", lkNone, BtYes) - acl("USE_TOOLS", lkShell, BtTool, "*: append") - acl("USE_TOOLS.*", lkShell, BtTool, "*: append") + acl("USE_TOOLS", lkShell, BtTool, "*: append, use-loadtime") + acl("USE_TOOLS.*", lkShell, BtTool, "*: append, use-loadtime") pkg("USE_X11", lkNone, BtYes) sys("WARNINGS", lkShell, BtShellWord) sys("WARNING_MSG", lkNone, BtShellCommand) sys("WARNING_CAT", lkNone, BtShellCommand) + sysload("WRAPPER_DIR", lkNone, BtPathname) acl("WRAPPER_REORDER_CMDS", lkShell, BtWrapperReorder, "Makefile, Makefile.common, buildlink3.mk: append") pkg("WRAPPER_SHELL", lkNone, BtShellCommand) acl("WRAPPER_TRANSFORM_CMDS", lkShell, BtWrapperTransform, "Makefile, Makefile.common, buildlink3.mk: append") sys("WRKDIR", lkNone, BtPathname) pkg("WRKSRC", lkNone, BtWrkdirSubdirectory) + pkglist("X11_LDFLAGS", lkShell, BtLdFlag) sys("X11_PKGSRCDIR.*", lkNone, BtPathname) usr("XAW_TYPE", lkNone, enum("3d neXtaw standard xpm")) acl("XMKMF_FLAGS", lkShell, BtShellWord, "") @@ -1115,7 +1188,7 @@ func (src *Pkgsrc) InitVartypes() { func enum(values string) *BasicType { valueMap := make(map[string]bool) - for _, value := range splitOnSpace(values) { + for _, value := range fields(values) { valueMap[value] = true } name := "enum: " + values + " " // See IsEnum Index: pkgsrc/pkgtools/pkglint/files/vardefs_test.go diff -u pkgsrc/pkgtools/pkglint/files/vardefs_test.go:1.4 pkgsrc/pkgtools/pkglint/files/vardefs_test.go:1.5 --- pkgsrc/pkgtools/pkglint/files/vardefs_test.go:1.4 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/vardefs_test.go Tue Oct 9 19:12:13 2018 @@ -61,3 +61,17 @@ func (s *Suite) Test_parseACLEntries(c * func() { parseACLEntries("VARNAME", "*.mk: use; buildlink3.mk: append") }, "FATAL: Ineffective ACL glob \"buildlink3.mk\" for \"VARNAME\".") } + +func (s *Suite) Test_Pkgsrc_InitVartypes__LP64PLATFORMS(c *check.C) { + t := s.Init(c) + + t.SetupCommandLine("-Wall") + pkg := t.SetupPackage("category/package", + "BROKEN_ON_PLATFORM=\t${LP64PLATFORMS}") + + G.CheckDirent(pkg) + + // No warning about a missing :Q operator. + // All PLATFORM variables must be either lkNone or lkSpace. + t.CheckOutputEmpty() +} Index: pkgsrc/pkgtools/pkglint/files/vartypecheck.go diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.40 pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.41 --- pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.40 Wed Oct 3 22:27:53 2018 +++ pkgsrc/pkgtools/pkglint/files/vartypecheck.go Tue Oct 9 19:12:13 2018 @@ -24,12 +24,10 @@ type VartypeCheck struct { // fields except the value. This is typically used when checking parts // of composite types. func NewVartypeCheckValue(vc *VartypeCheck, value string) *VartypeCheck { - valueNoVar := vc.MkLine.WithoutMakeVariables(value) - - copy := *vc - copy.Value = value - copy.ValueNoVar = valueNoVar - return © + newVc := *vc + newVc.Value = value + newVc.ValueNoVar = vc.MkLine.WithoutMakeVariables(value) + return &newVc } const ( @@ -152,7 +150,7 @@ func (cv *VartypeCheck) CFlag() { } } -// The single-line description of the package. +// Comment checks for the single-line description of the package. func (cv *VartypeCheck) Comment() { line, value := cv.Line, cv.Value @@ -179,7 +177,7 @@ func (cv *VartypeCheck) Comment() { "provide additional information instead.") } } - if matches(value, `^[a-z]`) { + if matches(value, `^[a-z]`) && cv.Op == opAssign { line.Warnf("COMMENT should start with a capital letter.") } if hasSuffix(value, ".") { @@ -763,7 +761,7 @@ func (cv *VartypeCheck) PkgRevision() { if !matches(cv.Value, `^[1-9]\d*$`) { cv.Line.Warnf("%s must be a positive integer number.", cv.Varname) } - if path.Base(cv.Line.Filename) != "Makefile" { + if cv.Line.Basename != "Makefile" { cv.Line.Errorf("%s only makes sense directly in the package Makefile.", cv.Varname) Explain( "Usually, different packages using the same Makefile.common have", Index: pkgsrc/pkgtools/pkglint/files/getopt/getopt.go diff -u pkgsrc/pkgtools/pkglint/files/getopt/getopt.go:1.4 pkgsrc/pkgtools/pkglint/files/getopt/getopt.go:1.5 --- pkgsrc/pkgtools/pkglint/files/getopt/getopt.go:1.4 Sat Jan 27 18:50:37 2018 +++ pkgsrc/pkgtools/pkglint/files/getopt/getopt.go Tue Oct 9 19:12:13 2018 @@ -18,6 +18,14 @@ func NewOptions() *Options { return new(Options) } +// AddFlagGroup adds an option that takes multiple flag values. +// +// Example: +// var extra bool +// +// opts := NewOptions() +// warnings := opts.AddFlagGroup('W', "warnings", "warning,...", "Enable the given warnings") +// warnings.AddFlagVar("extra", &extra, false, "Print extra warnings") func (o *Options) AddFlagGroup(shortName rune, longName, argDescription, description string) *FlagGroup { grp := new(FlagGroup) opt := &option{shortName, longName, argDescription, description, grp} @@ -37,6 +45,8 @@ func (o *Options) AddStrList(shortName r o.options = append(o.options, opt) } +// Parse extracts the command line options from the given arguments. +// args[0] is the program name, as in os.Args. func (o *Options) Parse(args []string) (remainingArgs []string, err error) { var skip int for i := 1; i < len(args) && err == nil; i++ { @@ -108,20 +118,24 @@ func (o *Options) handleLongOption(args } return 0, nil case *[]string: - if argval != nil { + switch { + case argval != nil: *data = append(*data, *argval) return 0, nil - } else if i+1 < len(args) { + case i+1 < len(args): *data = append(*data, args[i+1]) return 1, nil - } else { + default: return 0, optErr("option requires an argument: --" + opt.longName) } case *FlagGroup: - if argval == nil { - return 1, data.parse("--"+opt.longName+"=", args[i+1]) - } else { + switch { + case argval != nil: return 0, data.parse("--"+opt.longName+"=", *argval) + case i+1 < len(args): + return 1, data.parse("--"+opt.longName+"=", args[i+1]) + default: + return 0, optErr("option requires an argument: --" + opt.longName) } } panic("getopt: unknown option type") @@ -139,23 +153,25 @@ optchar: case *[]string: argarg := optchars[ai+utf8.RuneLen(optchar):] - if argarg != "" { + switch { + case argarg != "": *data = append(*data, argarg) return 0, nil - } else if i+1 < len(args) { + case i+1 < len(args): *data = append(*data, args[i+1]) return 1, nil - } else { + default: return 0, optErr("option requires an argument: -" + string([]rune{optchar})) } case *FlagGroup: argarg := optchars[ai+utf8.RuneLen(optchar):] - if argarg != "" { + switch { + case argarg != "": return 0, data.parse(string([]rune{'-', optchar}), argarg) - } else if i+1 < len(args) { + case i+1 < len(args): return 1, data.parse(string([]rune{'-', optchar}), args[i+1]) - } else { + default: return 0, optErr("option requires an argument: -" + string([]rune{optchar})) } } Index: pkgsrc/pkgtools/pkglint/files/getopt/getopt_test.go diff -u pkgsrc/pkgtools/pkglint/files/getopt/getopt_test.go:1.6 pkgsrc/pkgtools/pkglint/files/getopt/getopt_test.go:1.7 --- pkgsrc/pkgtools/pkglint/files/getopt/getopt_test.go:1.6 Wed Oct 3 22:27:54 2018 +++ pkgsrc/pkgtools/pkglint/files/getopt/getopt_test.go Tue Oct 9 19:12:13 2018 @@ -2,6 +2,7 @@ package getopt import ( "gopkg.in/check.v1" + "strings" "testing" ) @@ -177,3 +178,100 @@ func (s *Suite) Test_Options_Parse__long c.Check(iflag, check.Equals, true) c.Check(err, check.ErrorMatches, `^progname: invalid argument for option --jflag$`) } + +func (s *Suite) Test_Options_handleLongOption__flag_group_without_argument(c *check.C) { + var extra bool + + opts := NewOptions() + group := opts.AddFlagGroup('W', "warnings", "warning,...", "Print selected warnings") + group.AddFlagVar("extra", &extra, false, "Print extra warnings") + + args, err := opts.Parse([]string{"progname", "--warnings"}) + + c.Check(args, check.IsNil) + c.Check(err.Error(), check.Equals, "progname: option requires an argument: --warnings") + c.Check(extra, check.Equals, false) +} + +func (s *Suite) Test_Options_handleLongOption__flag_group_separate_argument(c *check.C) { + var extra bool + + opts := NewOptions() + group := opts.AddFlagGroup('W', "warnings", "warning,...", "Print selected warnings") + group.AddFlagVar("extra", &extra, false, "Print extra warnings") + + args, err := opts.Parse([]string{"progname", "--warnings", "extra,unknown"}) + + c.Check(args, check.IsNil) + c.Check(err.Error(), check.Equals, "progname: unknown option: --warnings=unknown") + c.Check(extra, check.Equals, true) +} + +func (s *Suite) Test_Options_handleLongOption__flag_group_negated(c *check.C) { + var extra bool + + opts := NewOptions() + group := opts.AddFlagGroup('W', "warnings", "warning,...", "Print selected warnings") + group.AddFlagVar("extra", &extra, true, "Print extra warnings") + + args, err := opts.Parse([]string{"progname", "--warnings", "all,no-extra"}) + + c.Check(args, check.IsNil) + c.Check(err, check.IsNil) + c.Check(extra, check.Equals, false) +} + +func (s *Suite) Test_Options_handleLongOption__internal_error(c *check.C) { + var extra bool + + opts := NewOptions() + group := opts.AddFlagGroup('W', "warnings", "warning,...", "Print selected warnings") + group.AddFlagVar("extra", &extra, false, "Print extra warnings") + opts.options[0].data = "unexpected value" + + c.Check( + func() { _, _ = opts.Parse([]string{"progname", "--warnings"}) }, + check.Panics, + "getopt: unknown option type") +} + +func (s *Suite) Test_Options_parseShortOptions__flag_group_separate_argument(c *check.C) { + var extra bool + + opts := NewOptions() + group := opts.AddFlagGroup('W', "warnings", "warning,...", "Print selected warnings") + group.AddFlagVar("extra", &extra, false, "Print extra warnings") + + args, err := opts.Parse([]string{"progname", "-W", "extra,unknown"}) + + c.Check(args, check.IsNil) + c.Check(err.Error(), check.Equals, "progname: unknown option: -Wunknown") + c.Check(extra, check.Equals, true) +} + +func (s *Suite) Test_Options_Help(c *check.C) { + var verbose, basic, extra bool + + opts := NewOptions() + opts.AddFlagVar('v', "verbose", &verbose, false, "Print a detailed log") + group := opts.AddFlagGroup('W', "warnings", "warning,...", "Print selected warnings") + group.AddFlagVar("basic", &basic, true, "Print basic warnings") + group.AddFlagVar("extra", &extra, false, "Print extra warnings") + + var out strings.Builder + opts.Help(&out, "progname [options] args") + + c.Check(out.String(), check.Equals, ""+ + "usage: progname [options] args\n"+ + "\n"+ + " -v, --verbose Print a detailed log\n"+ + " -W, --warnings=warning,... Print selected warnings\n"+ + "\n"+ + " Flags for -W, --warnings:\n"+ + " all all of the following\n"+ + " none none of the following\n"+ + " basic Print basic warnings (enabled)\n"+ + " extra Print extra warnings (disabled)\n"+ + "\n"+ + " (Prefix a flag with \"no-\" to disable it.)\n") +} Index: pkgsrc/pkgtools/pkglint/files/licenses/licenses.go diff -u pkgsrc/pkgtools/pkglint/files/licenses/licenses.go:1.3 pkgsrc/pkgtools/pkglint/files/licenses/licenses.go:1.4 --- pkgsrc/pkgtools/pkglint/files/licenses/licenses.go:1.3 Wed Oct 3 22:27:54 2018 +++ pkgsrc/pkgtools/pkglint/files/licenses/licenses.go Tue Oct 9 19:12:13 2018 @@ -20,7 +20,7 @@ type Condition struct { func Parse(licenses string, res *regex.Registry) *Condition { lexer := &licenseLexer{repl: textproc.NewPrefixReplacer(licenses, res)} result := liyyNewParser().Parse(lexer) - if result == 0 { + if result == 0 && lexer.repl.EOF() { return lexer.result } return nil @@ -64,7 +64,7 @@ type licenseLexer struct { func (lexer *licenseLexer) Lex(llval *liyySymType) int { repl := lexer.repl - repl.AdvanceHspace() + repl.SkipHspace() switch { case repl.EOF(): return 0 Index: pkgsrc/pkgtools/pkglint/files/licenses/licenses_test.go diff -u pkgsrc/pkgtools/pkglint/files/licenses/licenses_test.go:1.2 pkgsrc/pkgtools/pkglint/files/licenses/licenses_test.go:1.3 --- pkgsrc/pkgtools/pkglint/files/licenses/licenses_test.go:1.2 Wed Oct 3 22:27:54 2018 +++ pkgsrc/pkgtools/pkglint/files/licenses/licenses_test.go Tue Oct 9 19:12:13 2018 @@ -51,6 +51,8 @@ func (s *Suite) Test_Parse(c *check.C) { c.Check(Parse("a AND b OR c AND d", &res).String(), check.Equals, "a MIXED b MIXED c MIXED d") c.Check(Parse("AND artistic", &res), check.IsNil) + + c.Check(Parse("invalid/character", &res), check.IsNil) } func (s *Suite) Test_Condition_String(c *check.C) { @@ -81,6 +83,32 @@ func (s *Suite) Test_Condition_String(c c.Check(mixed.String(), check.Equals, "a MIXED b MIXED c") } +func (s *Suite) Test_Walk(c *check.C) { + res := regex.NewRegistry() + + condition := Parse("(a OR b) AND (c AND d)", &res) + + var out []string + condition.Walk(func(condition *Condition) { + switch { + case condition.Name != "": + out = append(out, condition.Name) + case condition.Paren != nil: + out = append(out, "()") + case condition.And && condition.Or: + out = append(out, "MIXED") + case condition.And: + out = append(out, "AND") + case condition.Or: + out = append(out, "OR") + default: + panic("unexpected") + } + }) + + c.Check(out, check.DeepEquals, []string{"a", "b", "OR", "()", "c", "d", "AND", "()", "AND"}) +} + func NewName(name string) *Condition { return &Condition{Name: name} } Index: pkgsrc/pkgtools/pkglint/files/textproc/prefixreplacer.go diff -u pkgsrc/pkgtools/pkglint/files/textproc/prefixreplacer.go:1.6 pkgsrc/pkgtools/pkglint/files/textproc/prefixreplacer.go:1.7 --- pkgsrc/pkgtools/pkglint/files/textproc/prefixreplacer.go:1.6 Wed Oct 3 22:27:54 2018 +++ pkgsrc/pkgtools/pkglint/files/textproc/prefixreplacer.go Tue Oct 9 19:12:13 2018 @@ -80,11 +80,7 @@ func (pr *PrefixReplacer) AdvanceBytesFu // AdvanceHspace advances over as many spaces and tabs as possible. func (pr *PrefixReplacer) AdvanceHspace() bool { - i := 0 - rest := pr.rest - for i < len(rest) && (rest[i] == ' ' || rest[i] == '\t') { - i++ - } + i := initialHspace(pr.rest) if i != 0 { pr.s = pr.rest[:i] pr.rest = pr.rest[i:] @@ -134,8 +130,8 @@ func (pr *PrefixReplacer) Skip(n int) { pr.rest = pr.rest[n:] } -func (pr *PrefixReplacer) SkipSpace() { - pr.rest = strings.TrimLeft(pr.rest, " \t") +func (pr *PrefixReplacer) SkipHspace() { + pr.rest = pr.rest[initialHspace(pr.rest):] } // Since returns the substring between the mark and the current position. @@ -156,3 +152,11 @@ func (pr *PrefixReplacer) HasPrefix(str func (pr *PrefixReplacer) HasPrefixRegexp(re regex.Pattern) bool { return pr.res.Matches(pr.rest, re) } + +func initialHspace(s string) int { + i := 0 + for i < len(s) && (s[i] == ' ' || s[i] == '\t') { + i++ + } + return i +} Index: pkgsrc/pkgtools/pkglint/files/trace/tracing_test.go diff -u pkgsrc/pkgtools/pkglint/files/trace/tracing_test.go:1.1 pkgsrc/pkgtools/pkglint/files/trace/tracing_test.go:1.2 --- pkgsrc/pkgtools/pkglint/files/trace/tracing_test.go:1.1 Wed Sep 5 17:56:22 2018 +++ pkgsrc/pkgtools/pkglint/files/trace/tracing_test.go Tue Oct 9 19:12:13 2018 @@ -65,6 +65,52 @@ func (s *Suite) Test_Call__argumentsAndR "TRACE: - netbsd.org/pkglint/trace.argumentsAndResultWrong(\"arg0\", 1234, \"\")\n") } +func (s *Suite) Test__fixed_argument_variants(c *check.C) { + + output := s.captureTracingOutput(func() { + defer Call0()() + defer Call1("x")() + defer Call2("x", "y")() + Step1("step %s", "a") + Step2("step %s, %s", "a", "b") + }) + + c.Check(output, check.Equals, ""+ + "TRACE: + netbsd.org/pkglint/trace.(*Suite).Test__fixed_argument_variants.func1()\n"+ + "TRACE: 1 + netbsd.org/pkglint/trace.(*Suite).Test__fixed_argument_variants.func1(\"x\")\n"+ + "TRACE: 1 2 + netbsd.org/pkglint/trace.(*Suite).Test__fixed_argument_variants.func1(\"x\", \"y\")\n"+ + "TRACE: 1 2 3 step a\n"+ + "TRACE: 1 2 3 step a, b\n"+ + "TRACE: 1 2 - netbsd.org/pkglint/trace.(*Suite).Test__fixed_argument_variants.func1(\"x\", \"y\")\n"+ + "TRACE: 1 - netbsd.org/pkglint/trace.(*Suite).Test__fixed_argument_variants.func1(\"x\")\n"+ + "TRACE: - netbsd.org/pkglint/trace.(*Suite).Test__fixed_argument_variants.func1()\n") +} + +func (s *Suite) Test__stringer_arg(c *check.C) { + + output := s.captureTracingOutput(func() { + defer Call(str{}, &str{})() + }) + + c.Check(output, check.Equals, ""+ + "TRACE: + netbsd.org/pkglint/trace.(*Suite).Test__stringer_arg.func1(It's a string, It's a string)\n"+ + "TRACE: - netbsd.org/pkglint/trace.(*Suite).Test__stringer_arg.func1(It's a string, It's a string)\n") +} + +func (s *Suite) Test_traceCall__panic(c *check.C) { + c.Check( + func() { traceCall() }, + check.Panics, + "Internal pkglint error: calls to trace.Call must only occur in tracing mode") +} + +func (s *Suite) Test_Result__panic(c *check.C) { + c.Check( + func() { Result("invalid argument") }, + check.Panics, + "Result must be called with a pointer to the result, not \"invalid argument\".") +} + func (s *Suite) captureTracingOutput(action func()) string { out := bytes.Buffer{} Out = &out @@ -76,3 +122,9 @@ func (s *Suite) captureTracingOutput(act Out = nil return out.String() } + +type str struct{} + +func (str) String() string { + return "It's a string" +} --_----------=_1539112334210460--