pkgsrc-Changes archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
CVS commit: pkgsrc/pkgtools/pkglint
Module Name: pkgsrc
Committed By: rillig
Date: Sun Mar 4 20:34:33 UTC 2018
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile
pkgsrc/pkgtools/pkglint/files: autofix.go check_test.go globaldata.go
line.go logging.go logging_test.go mkline.go mklinechecker.go
mklinechecker_test.go mklines.go mklines_test.go package.go
package_test.go pkglint.go pkglint_test.go plist.go plist_test.go
shell.go shell_test.go shtokenizer.go util.go vardefs.go
vartypecheck.go vartypecheck_test.go
Added Files:
pkgsrc/pkgtools/pkglint/files: codewalk.md
Removed Files:
pkgsrc/pkgtools/pkglint/files: globalvars.go
Log Message:
pkgtools/pkglint: update to 5.5.6
Changes since 5.5.5:
* Only offer explanations if an explainable diagnostic has actually
been logged.
* Clean up code.
* Improve a few diagnostics.
* In any Makefile, treat documented variables as used. This prevents
warning about defined but unused variables.
* Add support for some variables not previously known to pkglint.
To generate a diff of this commit:
cvs rdiff -u -r1.530 -r1.531 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.4 -r1.5 pkgsrc/pkgtools/pkglint/files/autofix.go
cvs rdiff -u -r1.17 -r1.18 pkgsrc/pkgtools/pkglint/files/check_test.go \
pkgsrc/pkgtools/pkglint/files/mklines_test.go
cvs rdiff -u -r0 -r1.1 pkgsrc/pkgtools/pkglint/files/codewalk.md
cvs rdiff -u -r1.25 -r1.26 pkgsrc/pkgtools/pkglint/files/globaldata.go \
pkgsrc/pkgtools/pkglint/files/package.go
cvs rdiff -u -r1.9 -r0 pkgsrc/pkgtools/pkglint/files/globalvars.go
cvs rdiff -u -r1.20 -r1.21 pkgsrc/pkgtools/pkglint/files/line.go \
pkgsrc/pkgtools/pkglint/files/mklines.go
cvs rdiff -u -r1.10 -r1.11 pkgsrc/pkgtools/pkglint/files/logging.go
cvs rdiff -u -r1.2 -r1.3 pkgsrc/pkgtools/pkglint/files/logging_test.go
cvs rdiff -u -r1.28 -r1.29 pkgsrc/pkgtools/pkglint/files/mkline.go
cvs rdiff -u -r1.9 -r1.10 pkgsrc/pkgtools/pkglint/files/mklinechecker.go
cvs rdiff -u -r1.7 -r1.8 pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go \
pkgsrc/pkgtools/pkglint/files/shtokenizer.go
cvs rdiff -u -r1.18 -r1.19 pkgsrc/pkgtools/pkglint/files/package_test.go
cvs rdiff -u -r1.27 -r1.28 pkgsrc/pkgtools/pkglint/files/pkglint.go
cvs rdiff -u -r1.15 -r1.16 pkgsrc/pkgtools/pkglint/files/pkglint_test.go
cvs rdiff -u -r1.22 -r1.23 pkgsrc/pkgtools/pkglint/files/plist.go
cvs rdiff -u -r1.21 -r1.22 pkgsrc/pkgtools/pkglint/files/plist_test.go \
pkgsrc/pkgtools/pkglint/files/shell.go \
pkgsrc/pkgtools/pkglint/files/shell_test.go \
pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
cvs rdiff -u -r1.19 -r1.20 pkgsrc/pkgtools/pkglint/files/util.go
cvs rdiff -u -r1.36 -r1.37 pkgsrc/pkgtools/pkglint/files/vardefs.go
cvs rdiff -u -r1.29 -r1.30 pkgsrc/pkgtools/pkglint/files/vartypecheck.go
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: pkgsrc/pkgtools/pkglint/Makefile
diff -u pkgsrc/pkgtools/pkglint/Makefile:1.530 pkgsrc/pkgtools/pkglint/Makefile:1.531
--- pkgsrc/pkgtools/pkglint/Makefile:1.530 Sun Mar 4 15:52:18 2018
+++ pkgsrc/pkgtools/pkglint/Makefile Sun Mar 4 20:34:32 2018
@@ -1,7 +1,6 @@
-# $NetBSD: Makefile,v 1.530 2018/03/04 15:52:18 bsiegert Exp $
+# $NetBSD: Makefile,v 1.531 2018/03/04 20:34:32 rillig Exp $
-PKGNAME= pkglint-5.5.5
-PKGREVISION= 1
+PKGNAME= pkglint-5.5.6
DISTFILES= # none
CATEGORIES= pkgtools
Index: pkgsrc/pkgtools/pkglint/files/autofix.go
diff -u pkgsrc/pkgtools/pkglint/files/autofix.go:1.4 pkgsrc/pkgtools/pkglint/files/autofix.go:1.5
--- pkgsrc/pkgtools/pkglint/files/autofix.go:1.4 Mon Feb 19 12:40:38 2018
+++ pkgsrc/pkgtools/pkglint/files/autofix.go Sun Mar 4 20:34:33 2018
@@ -227,7 +227,8 @@ func (fix *Autofix) Apply() {
return
}
- if shallBeLogged(fix.diagFormat) {
+ G.explainNext = shallBeLogged(fix.diagFormat)
+ if G.explainNext {
logDiagnostic := fix.level != nil && fix.diagFormat != "Silent-Magic-Diagnostic" &&
!(G.opts.Autofix && !G.opts.PrintAutofix) && len(fix.actions) > 0
if logDiagnostic {
Index: pkgsrc/pkgtools/pkglint/files/check_test.go
diff -u pkgsrc/pkgtools/pkglint/files/check_test.go:1.17 pkgsrc/pkgtools/pkglint/files/check_test.go:1.18
--- pkgsrc/pkgtools/pkglint/files/check_test.go:1.17 Mon Feb 19 12:40:38 2018
+++ pkgsrc/pkgtools/pkglint/files/check_test.go Sun Mar 4 20:34:33 2018
@@ -45,7 +45,7 @@ func (s *Suite) SetUpTest(c *check.C) {
t := &Tester{checkC: c}
s.Tester = t
- G = GlobalVars{Testing: true}
+ G = Pkglint{Testing: true}
textproc.Testing = true
G.logOut = NewSeparatorWriter(&t.stdout)
G.logErr = NewSeparatorWriter(&t.stderr)
@@ -62,7 +62,7 @@ func (s *Suite) TearDownTest(c *check.C)
t := s.Tester
t.checkC = nil // No longer usable; see https://github.com/go-check/check/issues/22
- G = GlobalVars{}
+ G = Pkglint{} // unusable because of missing logOut and logErr
textproc.Testing = false
if out := t.Output(); out != "" {
fmt.Fprintf(os.Stderr, "Unchecked output in %q; check with: t.CheckOutputLines(%v)",
@@ -96,7 +96,7 @@ func (t *Tester) c() *check.C {
// SetupCommandLine simulates a command line for the remainder of the test.
// See Pkglint.ParseCommandLine.
func (t *Tester) SetupCommandLine(args ...string) {
- exitcode := new(Pkglint).ParseCommandLine(append([]string{"pkglint"}, args...))
+ exitcode := G.ParseCommandLine(append([]string{"pkglint"}, args...))
if exitcode != nil && *exitcode != 0 {
t.CheckOutputEmpty()
t.c().Fatalf("Cannot parse command line: %#v", args)
Index: pkgsrc/pkgtools/pkglint/files/mklines_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.17 pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.18
--- pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.17 Mon Feb 19 12:40:38 2018
+++ pkgsrc/pkgtools/pkglint/files/mklines_test.go Sun Mar 4 20:34:33 2018
@@ -1,7 +1,9 @@
package main
import (
+ "fmt"
"gopkg.in/check.v1"
+ "sort"
)
func (s *Suite) Test_MkLines_Check__autofix_conditional_indentation(c *check.C) {
@@ -162,6 +164,10 @@ func (s *Suite) Test_MkLines__varuse_par
t.CheckOutputEmpty()
}
+// Even very complicated shell commands are parsed correctly.
+// Since the variable is defined in this same Makefile, it is
+// assumed to be a known shell command and therefore does not need
+// USE_TOOLS or a similar declaration.
func (s *Suite) Test_MkLines__loop_modifier(c *check.C) {
t := s.Init(c)
@@ -176,11 +182,8 @@ func (s *Suite) Test_MkLines__loop_modif
mklines.Check()
- t.CheckOutputLines(
- // No warning about missing @ at the end
- "WARN: chat/xchat/Makefile:4: " +
- "Unknown shell command \"${GCONF_SCHEMAS:@.s.@" +
- "${INSTALL_DATA} ${WRKSRC}/src/common/dbus/${.s.} ${DESTDIR}${GCONF_SCHEMAS_DIR}/@}\".")
+ // Earlier versions of pkglint warned about a missing @ at the end.
+ t.CheckOutputEmpty()
}
// PR 46570
@@ -320,8 +323,8 @@ func (s *Suite) Test_MkLines_DetermineUs
mklines.DetermineUsedVariables()
- c.Check(len(mklines.varuse), equals, 1)
- c.Check(mklines.varuse["VAR"], equals, mkline)
+ c.Check(len(mklines.vars.used), equals, 1)
+ c.Check(mklines.vars.FirstUse("VAR"), equals, mkline)
}
func (s *Suite) Test_MkLines_DetermineUsedVariables__nested(c *check.C) {
@@ -334,10 +337,10 @@ func (s *Suite) Test_MkLines_DetermineUs
mklines.DetermineUsedVariables()
- c.Check(len(mklines.varuse), equals, 3)
- c.Check(mklines.varuse["inner"], equals, mkline)
- c.Check(mklines.varuse["outer."], equals, mkline)
- c.Check(mklines.varuse["outer.*"], equals, mkline)
+ 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)
}
func (s *Suite) Test_MkLines_PrivateTool_Undefined(c *check.C) {
@@ -443,3 +446,50 @@ func (s *Suite) Test_MkLines_wip_categor
t.CheckOutputLines(
"ERROR: Makefile:14: \"/mk/misc/category.mk\" does not exist.")
}
+
+func (s *Suite) Test_MkLines_ExtractDocumentedVariables(c *check.C) {
+ t := s.Init(c)
+
+ t.SetupCommandLine("-Wall")
+ G.globalData.InitVartypes()
+ t.SetupTool(&Tool{Name: "rm", Varname: "RM", Predefined: true})
+ mklines := t.NewMkLines("Makefile",
+ MkRcsID,
+ "#",
+ "# User-settable variables:",
+ "#",
+ "# PKG_DEBUG_LEVEL",
+ "#\tHow verbose should pkgsrc be when running shell commands?",
+ "#",
+ "#\t* 0:\tdon't show most shell ...",
+ "",
+ "# PKG_VERBOSE",
+ "#\tWhen this variable is defined, pkgsrc gets a bit more verbose",
+ "#\t(i.e. \"-v\" option is passed to some commands ...",
+ "",
+ "# VARIABLE",
+ "#\tA paragraph of a single line is not enough to be recognized as \"relevant\".",
+ "",
+ "# VARBASE1.<param1>",
+ "# VARBASE2.*",
+ "# VARBASE3.${id}")
+
+ // The variables that appear in the documentation are marked as
+ // used, to prevent the "defined but not used" warnings.
+ mklines.determineDocumentedVariables()
+
+ var varnames []string
+ for varname, mkline := range mklines.vars.used {
+ varnames = append(varnames, fmt.Sprintf("%s (line %s)", varname, mkline.Linenos()))
+ }
+ sort.Strings(varnames)
+
+ expected := []string{
+ "PKG_DEBUG_LEVEL (line 5)",
+ "PKG_VERBOSE (line 10)",
+ "VARBASE1.* (line 17)",
+ "VARBASE2.* (line 18)",
+ "VARBASE3.${id} (line 19)",
+ "VARBASE3.* (line 19)"}
+ c.Check(varnames, deepEquals, expected)
+}
Index: pkgsrc/pkgtools/pkglint/files/globaldata.go
diff -u pkgsrc/pkgtools/pkglint/files/globaldata.go:1.25 pkgsrc/pkgtools/pkglint/files/globaldata.go:1.26
--- pkgsrc/pkgtools/pkglint/files/globaldata.go:1.25 Mon Feb 19 12:40:38 2018
+++ pkgsrc/pkgtools/pkglint/files/globaldata.go Sun Mar 4 20:34:33 2018
@@ -44,6 +44,9 @@ type SuggestedUpdate struct {
Comment string
}
+// Initialize reads the pkgsrc infrastructure files to
+// extract information like the tools, packages to update,
+// user-defined variables.
func (gd *GlobalData) Initialize() {
firstArg := G.Todo[0]
if fileExists(firstArg) {
Index: pkgsrc/pkgtools/pkglint/files/package.go
diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.25 pkgsrc/pkgtools/pkglint/files/package.go:1.26
--- pkgsrc/pkgtools/pkglint/files/package.go:1.25 Mon Feb 19 12:40:38 2018
+++ pkgsrc/pkgtools/pkglint/files/package.go Sun Mar 4 20:34:33 2018
@@ -26,13 +26,12 @@ type Package struct {
SeenBsdPrefsMk bool // Has bsd.prefs.mk already been included?
PlistDirs map[string]bool // Directories mentioned in the PLIST files
- vardef map[string]MkLine // (varname, varcanon) => line
- varuse map[string]MkLine // (varname, varcanon) => line
- bl3 map[string]Line // buildlink3.mk name => line; contains only buildlink3.mk files that are directly included.
- plistSubstCond map[string]bool // varname => true; list of all variables that are used as conditionals (@comment or nothing) in PLISTs.
- included map[string]Line // fname => line
- seenMakefileCommon bool // Does the package have any .includes?
- loadTimeTools map[string]bool // true=ok, false=not ok, absent=not mentioned in USE_TOOLS.
+ vars Scope
+ bl3 map[string]Line // buildlink3.mk name => line; contains only buildlink3.mk files that are directly included.
+ plistSubstCond map[string]bool // varname => true; list of all variables that are used as conditionals (@comment or nothing) in PLISTs.
+ included map[string]Line // fname => line
+ seenMakefileCommon bool // Does the package have any .includes?
+ loadTimeTools map[string]bool // true=ok, false=not ok, absent=not mentioned in USE_TOOLS.
conditionalIncludes map[string]MkLine
unconditionalIncludes map[string]MkLine
once Once
@@ -42,8 +41,7 @@ func NewPackage(pkgpath string) *Package
pkg := &Package{
Pkgpath: pkgpath,
PlistDirs: make(map[string]bool),
- vardef: make(map[string]MkLine),
- varuse: make(map[string]MkLine),
+ vars: NewScope(),
bl3: make(map[string]Line),
plistSubstCond: make(map[string]bool),
included: make(map[string]Line),
@@ -52,21 +50,11 @@ func NewPackage(pkgpath string) *Package
unconditionalIncludes: make(map[string]MkLine),
}
for varname, line := range G.globalData.UserDefinedVars {
- pkg.vardef[varname] = line
+ pkg.vars.Define(varname, line)
}
return pkg
}
-func (pkg *Package) defineVar(mkline MkLine, varname string) {
- if pkg.vardef[varname] == nil {
- pkg.vardef[varname] = mkline
- }
- varcanon := varnameCanon(varname)
- if pkg.vardef[varcanon] == nil {
- pkg.vardef[varcanon] = mkline
- }
-}
-
func (pkg *Package) varValue(varname string) (string, bool) {
switch varname {
case "KRB5_TYPE":
@@ -74,7 +62,7 @@ func (pkg *Package) varValue(varname str
case "PGSQL_VERSION":
return "95", true
}
- if mkline := pkg.vardef[varname]; mkline != nil {
+ if mkline := pkg.vars.FirstDefinition(varname); mkline != nil {
return mkline.Value(), true
}
return "", false
@@ -150,7 +138,7 @@ func (pkg *Package) checklinesBuildlink3
}
}
-func checkdirPackage(pkgpath string) {
+func (pkglint *Pkglint) checkdirPackage(pkgpath string) {
if trace.Tracing {
defer trace.Call1(pkgpath)()
}
@@ -203,7 +191,7 @@ func checkdirPackage(pkgpath string) {
pkg.checkfilePackageMakefile(fname, lines)
}
} else {
- Checkfile(fname)
+ pkglint.Checkfile(fname)
}
if contains(fname, "/patches/patch-") {
havePatches = true
@@ -359,11 +347,11 @@ func (pkg *Package) readMakefile(fname s
if mkline.IsVarassign() {
varname, op, value := mkline.Varname(), mkline.Op(), mkline.Value()
- if op != opAssignDefault || G.Pkg.vardef[varname] == nil {
+ if op != opAssignDefault || !G.Pkg.vars.Defined(varname) {
if trace.Tracing {
trace.Stepf("varassign(%q, %q, %q)", varname, op, value)
}
- G.Pkg.vardef[varname] = mkline
+ G.Pkg.vars.Define(varname, mkline)
}
}
}
@@ -380,16 +368,16 @@ func (pkg *Package) checkfilePackageMake
defer trace.Call1(fname)()
}
- vardef := pkg.vardef
- if vardef["PLIST_SRC"] == nil &&
- vardef["GENERATE_PLIST"] == nil &&
- vardef["META_PACKAGE"] == nil &&
+ vars := pkg.vars
+ if !vars.Defined("PLIST_SRC") &&
+ !vars.Defined("GENERATE_PLIST") &&
+ !vars.Defined("META_PACKAGE") &&
!fileExists(G.CurrentDir+"/"+pkg.Pkgdir+"/PLIST") &&
!fileExists(G.CurrentDir+"/"+pkg.Pkgdir+"/PLIST.common") {
NewLineWhole(fname).Warnf("Neither PLIST nor PLIST.common exist, and PLIST_SRC is unset. Are you sure PLIST handling is ok?")
}
- if (vardef["NO_CHECKSUM"] != nil || vardef["META_PACKAGE"] != nil) && isEmptyDir(G.CurrentDir+"/"+pkg.Patchdir) {
+ if (vars.Defined("NO_CHECKSUM") || vars.Defined("META_PACKAGE")) && isEmptyDir(G.CurrentDir+"/"+pkg.Patchdir) {
if distinfoFile := G.CurrentDir + "/" + pkg.DistinfoFile; fileExists(distinfoFile) {
NewLineWhole(distinfoFile).Warnf("This file should not exist if NO_CHECKSUM or META_PACKAGE is set.")
}
@@ -399,15 +387,15 @@ func (pkg *Package) checkfilePackageMake
}
}
- if perlLine, noconfLine := vardef["REPLACE_PERL"], vardef["NO_CONFIGURE"]; perlLine != nil && noconfLine != nil {
+ if perlLine, noconfLine := vars.FirstDefinition("REPLACE_PERL"), vars.FirstDefinition("NO_CONFIGURE"); perlLine != nil && noconfLine != nil {
perlLine.Warnf("REPLACE_PERL is ignored when NO_CONFIGURE is set (in %s)", noconfLine.ReferenceFrom(perlLine.Line))
}
- if vardef["LICENSE"] == nil && vardef["META_PACKAGE"] == nil && pkg.once.FirstTime("LICENSE") {
+ if !vars.Defined("LICENSE") && !vars.Defined("META_PACKAGE") && pkg.once.FirstTime("LICENSE") {
NewLineWhole(fname).Errorf("Each package must define its LICENSE.")
}
- if gnuLine, useLine := vardef["GNU_CONFIGURE"], vardef["USE_LANGUAGES"]; gnuLine != nil && useLine != nil {
+ if gnuLine, useLine := vars.FirstDefinition("GNU_CONFIGURE"), vars.FirstDefinition("USE_LANGUAGES"); gnuLine != nil && useLine != nil {
if matches(useLine.VarassignComment(), `(?-i)\b(?:c|empty|none)\b`) {
// Don't emit a warning, since the comment
// probably contains a statement that C is
@@ -425,11 +413,11 @@ func (pkg *Package) checkfilePackageMake
pkg.determineEffectivePkgVars()
pkg.checkPossibleDowngrade()
- if vardef["COMMENT"] == nil {
+ if !vars.Defined("COMMENT") {
NewLineWhole(fname).Warnf("No COMMENT given.")
}
- if imake, x11 := vardef["USE_IMAKE"], vardef["USE_X11"]; imake != nil && x11 != nil {
+ if imake, x11 := vars.FirstDefinition("USE_IMAKE"), vars.FirstDefinition("USE_X11"); imake != nil && x11 != nil {
if !hasSuffix(x11.Filename, "/mk/x11.buildlink3.mk") {
imake.Notef("USE_IMAKE makes USE_X11 in %s superfluous.", x11.ReferenceFrom(imake.Line))
}
@@ -442,7 +430,7 @@ func (pkg *Package) checkfilePackageMake
}
func (pkg *Package) getNbpart() string {
- line := pkg.vardef["PKGREVISION"]
+ line := pkg.vars.FirstDefinition("PKGREVISION")
if line == nil {
return ""
}
@@ -454,8 +442,8 @@ func (pkg *Package) getNbpart() string {
}
func (pkg *Package) determineEffectivePkgVars() {
- distnameLine := pkg.vardef["DISTNAME"]
- pkgnameLine := pkg.vardef["PKGNAME"]
+ distnameLine := pkg.vars.FirstDefinition("DISTNAME")
+ pkgnameLine := pkg.vars.FirstDefinition("PKGNAME")
distname := ""
if distnameLine != nil {
@@ -543,7 +531,7 @@ func (pkg *Package) pkgnameFromDistname(
}
func (pkg *Package) expandVariableWithDefault(varname, defaultValue string) string {
- mkline := G.Pkg.vardef[varname]
+ mkline := G.Pkg.vars.FirstDefinition(varname)
if mkline == nil {
return defaultValue
}
@@ -834,8 +822,8 @@ func (pkg *Package) checkLocallyModified
defer trace.Call(fname)()
}
- ownerLine := pkg.vardef["OWNER"]
- maintainerLine := pkg.vardef["MAINTAINER"]
+ ownerLine := pkg.vars.FirstDefinition("OWNER")
+ maintainerLine := pkg.vars.FirstDefinition("MAINTAINER")
owner := ""
maintainer := ""
if ownerLine != nil && !containsVarRef(ownerLine.Value()) {
Index: pkgsrc/pkgtools/pkglint/files/line.go
diff -u pkgsrc/pkgtools/pkglint/files/line.go:1.20 pkgsrc/pkgtools/pkglint/files/line.go:1.21
--- pkgsrc/pkgtools/pkglint/files/line.go:1.20 Sat Jan 27 18:50:36 2018
+++ pkgsrc/pkgtools/pkglint/files/line.go Sun Mar 4 20:34:33 2018
@@ -123,7 +123,8 @@ func (line *LineImpl) log(level *LogLeve
// These are logged by Autofix.Apply.
return
}
- if !shallBeLogged(format) {
+ G.explainNext = shallBeLogged(format)
+ if !G.explainNext {
return
}
Index: pkgsrc/pkgtools/pkglint/files/mklines.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines.go:1.20 pkgsrc/pkgtools/pkglint/files/mklines.go:1.21
--- pkgsrc/pkgtools/pkglint/files/mklines.go:1.20 Sun Jan 28 23:21:16 2018
+++ pkgsrc/pkgtools/pkglint/files/mklines.go Sun Mar 4 20:34:33 2018
@@ -10,14 +10,13 @@ import (
type MkLines struct {
mklines []MkLine
lines []Line
- forVars map[string]bool // The variables currently used in .for loops
- target string // Current make(1) target
- vardef map[string]MkLine // varname => line; for all variables that are defined in the current file
- varuse map[string]MkLine // varname => line; for all variables that are used in the current file
- buildDefs map[string]bool // Variables that are registered in BUILD_DEFS, to ensure that all user-defined variables are added to it.
- plistVars map[string]bool // Variables that are registered in PLIST_VARS, to ensure that all user-defined variables are added to it.
- tools map[string]bool // Set of tools that are declared to be used.
- toolRegistry ToolRegistry // Tools defined in file scope.
+ forVars map[string]bool // The variables currently used in .for loops
+ target string // Current make(1) target
+ vars Scope
+ buildDefs map[string]bool // Variables that are registered in BUILD_DEFS, to ensure that all user-defined variables are added to it.
+ plistVars map[string]bool // Variables that are registered in PLIST_VARS, to ensure that all user-defined variables are added to it.
+ tools map[string]bool // Set of tools that are declared to be used.
+ toolRegistry ToolRegistry // Tools defined in file scope.
SeenBsdPrefsMk bool
indentation Indentation // Indentation depth of preprocessing directives
}
@@ -39,8 +38,7 @@ func NewMkLines(lines []Line) *MkLines {
lines,
make(map[string]bool),
"",
- make(map[string]MkLine),
- make(map[string]MkLine),
+ NewScope(),
make(map[string]bool),
make(map[string]bool),
tools,
@@ -49,28 +47,15 @@ func NewMkLines(lines []Line) *MkLines {
Indentation{}}
}
-func (mklines *MkLines) DefineVar(mkline MkLine, varname string) {
- if mklines.vardef[varname] == nil {
- mklines.vardef[varname] = mkline
- }
- varcanon := varnameCanon(varname)
- if mklines.vardef[varcanon] == nil {
- mklines.vardef[varcanon] = mkline
- }
-}
-
func (mklines *MkLines) UseVar(mkline MkLine, varname string) {
- varcanon := varnameCanon(varname)
- mklines.varuse[varname] = mkline
- mklines.varuse[varcanon] = mkline
+ mklines.vars.Use(varname, mkline)
if G.Pkg != nil {
- G.Pkg.varuse[varname] = mkline
- G.Pkg.varuse[varcanon] = mkline
+ G.Pkg.vars.Use(varname, mkline)
}
}
func (mklines *MkLines) VarValue(varname string) (value string, found bool) {
- if mkline := mklines.vardef[varname]; mkline != nil {
+ if mkline := mklines.vars.FirstDefinition(varname); mkline != nil {
return mkline.Value(), true
}
return "", false
@@ -117,6 +102,7 @@ func (mklines *MkLines) Check() {
case mkline.IsVarassign():
mklines.target = ""
+ mkline.Tokenize(mkline.Value())
substcontext.Varassign(mkline)
case mkline.IsInclude():
@@ -136,6 +122,9 @@ func (mklines *MkLines) Check() {
case mkline.IsDependency():
ck.checkDependencyRule(allowedTargets)
mklines.target = mkline.Targets()
+
+ case mkline.IsShellcmd():
+ mkline.Tokenize(mkline.Shellcmd())
}
}
lastMkline := mklines.mklines[len(mklines.mklines)-1]
@@ -222,6 +211,57 @@ func (mklines *MkLines) DetermineUsedVar
mklines.UseVar(mkline, varname)
}
}
+ mklines.determineDocumentedVariables()
+}
+
+// Loosely based on mk/help/help.awk, revision 1.28
+func (mklines *MkLines) determineDocumentedVariables() {
+ scope := NewScope()
+ commentLines := 0
+ relevant := true
+
+ finish := func() {
+ if commentLines >= 3 && relevant {
+ for varname, mkline := range scope.used {
+ mklines.vars.Use(varname, mkline)
+ }
+ }
+
+ scope = NewScope()
+ commentLines = 0
+ relevant = true
+ }
+
+ for _, mkline := range mklines.mklines {
+ text := mkline.Text
+ words := splitOnSpace(text)
+
+ if 1 < len(words) && words[0] == "#" {
+ commentLines++
+
+ parser := NewMkParser(mkline.Line, words[1], false)
+ varname := parser.Varname()
+ if hasSuffix(varname, ".") && parser.repl.AdvanceRegexp(`^<\w+>`) {
+ varname += "*"
+ }
+ parser.repl.AdvanceStr(":")
+
+ varbase := varnameBase(varname)
+ if varbase == strings.ToUpper(varbase) && matches(varbase, `[A-Z]`) && parser.EOF() {
+ scope.Use(varname, mkline)
+ }
+
+ if 1 < len(words) && words[1] == "Copyright" {
+ relevant = false
+ }
+ }
+
+ if text == "" {
+ finish()
+ }
+ }
+
+ finish()
}
func (mklines *MkLines) setSeenBsdPrefsMk() {
Index: pkgsrc/pkgtools/pkglint/files/logging.go
diff -u pkgsrc/pkgtools/pkglint/files/logging.go:1.10 pkgsrc/pkgtools/pkglint/files/logging.go:1.11
--- pkgsrc/pkgtools/pkglint/files/logging.go:1.10 Sat Jan 27 18:50:36 2018
+++ pkgsrc/pkgtools/pkglint/files/logging.go Sun Mar 4 20:34:33 2018
@@ -101,23 +101,6 @@ func logs(level *LogLevel, fname, lineno
}
func Explain(explanation ...string) {
- if G.opts.Explain {
- complete := strings.Join(explanation, "\n")
- if G.explanationsGiven[complete] {
- return
- }
- if G.explanationsGiven == nil {
- G.explanationsGiven = make(map[string]bool)
- }
- G.explanationsGiven[complete] = true
-
- G.logOut.WriteLine("")
- for _, explanationLine := range explanation {
- G.logOut.WriteLine("\t" + explanationLine)
- }
- G.logOut.WriteLine("")
- }
-
if G.Testing {
for _, s := range explanation {
if l := tabWidth(s); l > 68 && contains(s, " ") {
@@ -131,7 +114,30 @@ func Explain(explanation ...string) {
}
}
}
+
+ if !G.explainNext {
+ return
+ }
G.explanationsAvailable = true
+ if !G.opts.Explain {
+ return
+ }
+
+ complete := strings.Join(explanation, "\n")
+ if G.explanationsGiven[complete] {
+ return
+ }
+ if G.explanationsGiven == nil {
+ G.explanationsGiven = make(map[string]bool)
+ }
+ G.explanationsGiven[complete] = true
+
+ G.logOut.WriteLine("")
+ for _, explanationLine := range explanation {
+ G.logOut.WriteLine("\t" + explanationLine)
+ }
+ G.logOut.WriteLine("")
+
}
type pkglintFatal struct{}
Index: pkgsrc/pkgtools/pkglint/files/logging_test.go
diff -u pkgsrc/pkgtools/pkglint/files/logging_test.go:1.2 pkgsrc/pkgtools/pkglint/files/logging_test.go:1.3
--- pkgsrc/pkgtools/pkglint/files/logging_test.go:1.2 Sat Jan 27 18:50:36 2018
+++ pkgsrc/pkgtools/pkglint/files/logging_test.go Sun Mar 4 20:34:33 2018
@@ -135,3 +135,47 @@ func (s *Suite) Test_Line_log_only(c *ch
"-\tThe old song",
"+\tThe new2 song")
}
+
+func (s *Suite) Test_collect_explanations_with_only(c *check.C) {
+ t := s.Init(c)
+
+ t.SetupCommandLine("--only", "interesting")
+ line := t.NewLine("Makefile", 27, "The old song")
+
+ line.Warnf("Filtered warning.") // Is not logged.
+ Explain("Explanation for the above warning.") // Neither would this explanation be logged.
+ G.PrintSummary()
+
+ c.Check(G.explanationsAvailable, equals, false)
+ t.CheckOutputLines(
+ "Looks fine.") // "pkglint -e" is not advertised since the above explanation is not relevant.
+
+ line.Warnf("What an interesting line.")
+ Explain("This explanation is available.")
+ G.PrintSummary()
+
+ c.Check(G.explanationsAvailable, equals, true)
+ t.CheckOutputLines(
+ "WARN: Makefile:27: What an interesting line.",
+ "0 errors and 1 warning found.",
+ "(Run \"pkglint -e\" to show explanations.)")
+}
+
+func (s *Suite) Test_explain_with_only(c *check.C) {
+ t := s.Init(c)
+
+ t.SetupCommandLine("--only", "interesting", "--explain")
+ line := t.NewLine("Makefile", 27, "The old song")
+
+ line.Warnf("Filtered warning.") // Is not logged.
+ Explain("Explanation for the above warning.") // Neither is this explanation logged.
+
+ line.Warnf("What an interesting line.")
+ Explain("This explanation is logged.")
+
+ t.CheckOutputLines(
+ "WARN: Makefile:27: What an interesting line.",
+ "",
+ "\tThis explanation is logged.",
+ "")
+}
Index: pkgsrc/pkgtools/pkglint/files/mkline.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline.go:1.28 pkgsrc/pkgtools/pkglint/files/mkline.go:1.29
--- pkgsrc/pkgtools/pkglint/files/mkline.go:1.28 Mon Feb 19 12:40:38 2018
+++ pkgsrc/pkgtools/pkglint/files/mkline.go Sun Mar 4 20:34:33 2018
@@ -9,6 +9,10 @@ import (
"strings"
)
+// MkLine is a line from a Makefile fragment.
+// There are several types of lines.
+// The most common types in pkgsrc are variable assignments,
+// shell commands and preprocessor instructions.
type MkLine = *MkLineImpl
type MkLineImpl struct {
@@ -47,13 +51,11 @@ type mkLineDependency struct {
sources string
}
-func NewMkLine(line Line) (mkline *MkLineImpl) {
- mkline = &MkLineImpl{Line: line}
-
+func NewMkLine(line Line) *MkLineImpl {
text := line.Text
if hasPrefix(text, " ") {
- mkline.Warnf("Makefile lines should not start with space characters.")
+ line.Warnf("Makefile lines should not start with space characters.")
Explain(
"If you want this line to contain a shell program, use a tab",
"character for indentation. Otherwise please remove the leading",
@@ -85,7 +87,7 @@ func NewMkLine(line Line) (mkline *MkLin
value = strings.Replace(value, "\\#", "#", -1)
varparam := varnameParam(varname)
- mkline.data = mkLineAssign{
+ return &MkLineImpl{line, mkLineAssign{
commented,
varname,
varnameCanon(varname),
@@ -93,58 +95,48 @@ func NewMkLine(line Line) (mkline *MkLin
NewMkOperator(op),
valueAlign,
value,
- comment}
- mkline.Tokenize(value)
- return
+ comment}}
}
if hasPrefix(text, "\t") {
shellcmd := text[1:]
- mkline.data = mkLineShell{shellcmd}
- mkline.Tokenize(shellcmd)
- return
+ return &MkLineImpl{line, mkLineShell{shellcmd}}
}
trimmedText := strings.TrimSpace(text)
if strings.HasPrefix(trimmedText, "#") {
- mkline.data = mkLineComment{}
- return
+ return &MkLineImpl{line, mkLineComment{}}
}
if trimmedText == "" {
- mkline.data = mkLineEmpty{}
- return
+ return &MkLineImpl{line, mkLineEmpty{}}
}
if m, indent, directive, args := matchMkCond(text); m {
- mkline.data = mkLineConditional{indent, directive, args}
- return
+ return &MkLineImpl{line, mkLineConditional{indent, directive, args}}
}
if m, indent, directive, includefile := MatchMkInclude(text); m {
- mkline.data = mkLineInclude{directive == "include", false, indent, includefile, ""}
- return
+ return &MkLineImpl{line, mkLineInclude{directive == "include", false, indent, includefile, ""}}
}
if m, indent, directive, includefile := match3(text, `^\.(\s*)(s?include)\s+<([^>]+)>\s*(?:#.*)?$`); m {
- mkline.data = mkLineInclude{directive == "include", true, indent, includefile, ""}
- return
+ return &MkLineImpl{line, mkLineInclude{directive == "include", true, indent, includefile, ""}}
}
if m, targets, whitespace, sources := match3(text, `^([^\s:]+(?:\s*[^\s:]+)*)(\s*):\s*([^#]*?)(?:\s*#.*)?$`); m {
- mkline.data = mkLineDependency{targets, sources}
if whitespace != "" {
line.Warnf("Space before colon in dependency line.")
}
- return
+ return &MkLineImpl{line, mkLineDependency{targets, sources}}
}
if matches(text, `^(<<<<<<<|=======|>>>>>>>)`) {
- return
+ return &MkLineImpl{line, nil}
}
line.Errorf("Unknown Makefile line format.")
- return mkline
+ return &MkLineImpl{line, nil}
}
func (mkline *MkLineImpl) String() string {
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.9 pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.10
--- pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.9 Mon Feb 19 12:40:38 2018
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker.go Sun Mar 4 20:34:33 2018
@@ -454,7 +454,7 @@ func (ck MkLineChecker) CheckVarusePermi
}
func (ck MkLineChecker) WarnVaruseLocalbase() {
- ck.MkLine.Warnf("The LOCALBASE variable should not be used by packages.")
+ ck.MkLine.Warnf("Please use PREFIX instead of LOCALBASE.")
Explain(
// from jlam via private mail.
"Currently, LOCALBASE is typically used in these cases:",
@@ -465,6 +465,7 @@ func (ck MkLineChecker) WarnVaruseLocalb
"Example for (1):",
"",
" STRLIST= ${LOCALBASE}/bin/strlist",
+ "",
" do-build:",
" cd ${WRKSRC} && ${STRLIST} *.str",
"",
@@ -472,6 +473,7 @@ func (ck MkLineChecker) WarnVaruseLocalb
"",
" EVAL_PREFIX= STRLIST_PREFIX=strlist",
" STRLIST= ${STRLIST_PREFIX}/bin/strlist",
+ "",
" do-build:",
" cd ${WRKSRC} && ${STRLIST} *.str",
"",
@@ -515,7 +517,7 @@ func (ck MkLineChecker) CheckVaruseShell
//
// When doing checks outside a package, the :M* operator is needed for safety.
needMstar := matches(varname, `^(?:.*_)?(?:CFLAGS||CPPFLAGS|CXXFLAGS|FFLAGS|LDFLAGS|LIBS)$`) &&
- (G.Pkg == nil || G.Pkg.vardef["GNU_CONFIGURE"] != nil)
+ (G.Pkg == nil || G.Pkg.vars.Defined("GNU_CONFIGURE"))
strippedMod := mod
if m, stripped := match1(mod, `(.*?)(?::M\*)?(?::Q)?$`); m {
@@ -678,7 +680,7 @@ func (ck MkLineChecker) checkVarassign()
// The variables mentioned in EVAL_PREFIX will later be
// defined by find-prefix.mk. Therefore, they are marked
// as known in the current file.
- G.Mk.vardef[evalVarname] = mkline
+ G.Mk.vars.Define(evalVarname, mkline)
}
}
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.7 pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.8
--- pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.7 Mon Feb 19 12:40:38 2018
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go Sun Mar 4 20:34:33 2018
@@ -206,7 +206,7 @@ func (s *Suite) Test_MkLineChecker_WarnV
MkLineChecker{mkline}.WarnVaruseLocalbase()
t.CheckOutputLines(
- "WARN: options.mk:56: The LOCALBASE variable should not be used by packages.")
+ "WARN: options.mk:56: Please use PREFIX instead of LOCALBASE.")
}
func (s *Suite) Test_MkLineChecker_CheckRelativePkgdir(c *check.C) {
@@ -225,16 +225,17 @@ func (s *Suite) Test_MkLineChecker_Check
func (s *Suite) Test_MkLineChecker__unclosed_varuse(c *check.C) {
t := s.Init(c)
- mkline := t.NewMkLine("Makefile", 93, "EGDIRS=${EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d")
+ mklines := t.NewMkLines("Makefile",
+ MkRcsID,
+ "EGDIRS=${EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d")
- MkLineChecker{mkline}.checkVarassign()
+ mklines.Check()
t.CheckOutputLines(
- "WARN: Makefile:93: Pkglint parse error in MkLine.Tokenize at "+
- "\"${EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d\".",
- "WARN: Makefile:93: Pkglint parse error in ShTokenizer.ShAtom at "+
- "\"${EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d\" (quoting=plain)",
- "WARN: Makefile:93: EGDIRS is defined but not used. Spelling mistake?")
+ "WARN: Makefile:2: Unclosed Make variable starting at \"${EGDIR/apparmor.d $...\"",
+ "WARN: Makefile:2: EGDIRS is defined but not used. Spelling mistake?",
+ "WARN: Makefile:2: Pkglint parse error in MkLine.Tokenize at "+
+ "\"${EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d\".")
}
func (s *Suite) Test_MkLineChecker__Varuse_Modifier_L(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/shtokenizer.go
diff -u pkgsrc/pkgtools/pkglint/files/shtokenizer.go:1.7 pkgsrc/pkgtools/pkglint/files/shtokenizer.go:1.8
--- pkgsrc/pkgtools/pkglint/files/shtokenizer.go:1.7 Mon Jan 1 18:04:15 2018
+++ pkgsrc/pkgtools/pkglint/files/shtokenizer.go Sun Mar 4 20:34:33 2018
@@ -56,7 +56,11 @@ func (p *ShTokenizer) ShAtom(quoting ShQ
if atom == nil {
repl.Reset(mark)
- p.parser.Line.Warnf("Pkglint parse error in ShTokenizer.ShAtom at %q (quoting=%s)", repl.Rest(), quoting)
+ if hasPrefix(repl.Rest(), "${") {
+ p.parser.Line.Warnf("Unclosed Make variable starting at %q", shorten(repl.Rest(), 20))
+ } else {
+ p.parser.Line.Warnf("Pkglint parse error in ShTokenizer.ShAtom at %q (quoting=%s)", repl.Rest(), quoting)
+ }
}
return atom
}
Index: pkgsrc/pkgtools/pkglint/files/package_test.go
diff -u pkgsrc/pkgtools/pkglint/files/package_test.go:1.18 pkgsrc/pkgtools/pkglint/files/package_test.go:1.19
--- pkgsrc/pkgtools/pkglint/files/package_test.go:1.18 Mon Feb 19 12:40:38 2018
+++ pkgsrc/pkgtools/pkglint/files/package_test.go Sun Mar 4 20:34:33 2018
@@ -6,7 +6,7 @@ func (s *Suite) Test_Package_pkgnameFrom
t := s.Init(c)
pkg := NewPackage("dummy")
- pkg.vardef["PKGNAME"] = t.NewMkLine("Makefile", 5, "PKGNAME=dummy")
+ pkg.vars.Define("PKGNAME", t.NewMkLine("Makefile", 5, "PKGNAME=dummy"))
c.Check(pkg.pkgnameFromDistname("pkgname-1.0", "whatever"), equals, "pkgname-1.0")
c.Check(pkg.pkgnameFromDistname("${DISTNAME}", "distname-1.0"), equals, "distname-1.0")
@@ -162,7 +162,7 @@ func (s *Suite) Test_Package_varorder_li
G.globalData.Pkgsrcdir = t.TmpDir()
G.CurrentDir = t.TmpDir()
- (&Pkglint{}).CheckDirent(t.TmpDir() + "/x11/9term")
+ G.CheckDirent(t.TmpDir() + "/x11/9term")
// Since the error is grave enough, the warning about the correct position is suppressed.
t.CheckOutputLines(
@@ -249,11 +249,12 @@ func (s *Suite) Test_Package_getNbpart(c
t := s.Init(c)
pkg := NewPackage("category/pkgbase")
- pkg.vardef["PKGREVISION"] = t.NewMkLine("Makefile", 1, "PKGREVISION=14")
+ pkg.vars.Define("PKGREVISION", t.NewMkLine("Makefile", 1, "PKGREVISION=14"))
c.Check(pkg.getNbpart(), equals, "nb14")
- pkg.vardef["PKGREVISION"] = t.NewMkLine("Makefile", 1, "PKGREVISION=asdf")
+ pkg.vars = NewScope()
+ pkg.vars.Define("PKGREVISION", t.NewMkLine("Makefile", 1, "PKGREVISION=asdf"))
c.Check(pkg.getNbpart(), equals, "")
}
@@ -266,9 +267,9 @@ func (s *Suite) Test_Package_determineEf
distnameLine := t.NewMkLine("Makefile", 4, "DISTNAME=distname-1.0")
pkgrevisionLine := t.NewMkLine("Makefile", 5, "PKGREVISION=13")
- pkg.defineVar(pkgnameLine, pkgnameLine.Varname())
- pkg.defineVar(distnameLine, distnameLine.Varname())
- pkg.defineVar(pkgrevisionLine, pkgrevisionLine.Varname())
+ pkg.vars.Define(pkgnameLine.Varname(), pkgnameLine)
+ pkg.vars.Define(distnameLine.Varname(), distnameLine)
+ pkg.vars.Define(pkgrevisionLine.Varname(), pkgrevisionLine)
pkg.determineEffectivePkgVars()
@@ -311,11 +312,11 @@ func (s *Suite) Test_checkdirPackage(c *
MkRcsID)
G.CurrentDir = t.TmpDir()
- checkdirPackage(t.TmpDir())
+ G.checkdirPackage(t.TmpDir())
t.CheckOutputLines(
"WARN: ~/Makefile: Neither PLIST nor PLIST.common exist, and PLIST_SRC is unset. Are you sure PLIST handling is ok?",
- "WARN: ~/distinfo: File not found. Please run \"@BMAKE@ makesum\".",
+ "WARN: ~/distinfo: File not found. Please run \""+confMake+" makesum\".",
"ERROR: ~/Makefile: Each package must define its LICENSE.",
"WARN: ~/Makefile: No COMMENT given.")
}
@@ -330,7 +331,7 @@ func (s *Suite) Test_checkdirPackage__me
G.CurrentDir = t.TmpDir()
G.globalData.InitVartypes()
- checkdirPackage(t.TmpDir())
+ G.checkdirPackage(t.TmpDir())
t.CheckOutputLines(
"WARN: ~/Makefile: No COMMENT given.") // No error about missing LICENSE.
@@ -391,7 +392,7 @@ func (s *Suite) Test_Package__varuse_at_
t.CreateFileLines("category/pkgbase/distinfo",
RcsID)
- (&Pkglint{}).Main("pkglint", "-q", "-Wperm", t.TmpDir()+"/category/pkgbase")
+ G.Main("pkglint", "-q", "-Wperm", t.TmpDir()+"/category/pkgbase")
t.CheckOutputLines(
"WARN: ~/category/pkgbase/Makefile:8: To use the tool \"FALSE\" at load time, bsd.prefs.mk has to be included before.",
@@ -459,7 +460,7 @@ func (s *Suite) Test_Package_conditional
G.CurPkgsrcdir = "../.."
G.Pkg = pkg
- checkdirPackage("category/package")
+ G.checkdirPackage("category/package")
t.CheckOutputLines(
"WARN: ~/category/package/options.mk:3: Unknown option \"zlib\".",
Index: pkgsrc/pkgtools/pkglint/files/pkglint.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint.go:1.27 pkgsrc/pkgtools/pkglint/files/pkglint.go:1.28
--- pkgsrc/pkgtools/pkglint/files/pkglint.go:1.27 Mon Feb 19 12:40:38 2018
+++ pkgsrc/pkgtools/pkglint/files/pkglint.go Sun Mar 4 20:34:33 2018
@@ -17,15 +17,107 @@ import (
const confMake = "@BMAKE@"
const confVersion = "@VERSION@"
+// Pkglint contains all global variables of this Go package.
+// The rest of the global state is in the other packages:
+// regex.Profiling (not thread-local)
+// regex.res (and related variables; not thread-safe)
+// textproc.Testing (not thread-local; harmless)
+// tracing.Tracing (not thread-safe)
+// tracing.Out (not thread-safe)
+// tracing.traceDepth (not thread-safe)
+type Pkglint struct {
+ opts CmdOpts //
+ globalData GlobalData //
+ Pkg *Package // The package that is currently checked.
+ Mk *MkLines // The Makefile (or fragment) that is currently checked.
+
+ Todo []string // The files or directories that still need to be checked.
+ CurrentDir string // The currently checked directory, relative to the cwd
+ CurPkgsrcdir string // The pkgsrc directory, relative to currentDir
+ Wip bool // Is the currently checked directory from pkgsrc-wip?
+ Infrastructure bool // Is the currently checked item from the pkgsrc infrastructure?
+ Testing bool // Is pkglint in self-testing mode (only during development)?
+ CurrentUsername string // For checking against OWNER and MAINTAINER
+ CvsEntriesDir string // Cached to avoid I/O
+ CvsEntriesLines []Line
+
+ Hash map[string]*Hash // Maps "alg:fname" => hash (inter-package check).
+ UsedLicenses map[string]bool // Maps "license name" => true (inter-package check).
+
+ errors int
+ warnings int
+ explainNext bool
+ logged map[string]bool
+ explanationsAvailable bool
+ explanationsGiven map[string]bool
+ autofixAvailable bool
+ logOut *SeparatorWriter
+ logErr *SeparatorWriter
+
+ loghisto *histogram.Histogram
+}
+
+type CmdOpts struct {
+ CheckAlternatives,
+ CheckBuildlink3,
+ CheckDescr,
+ CheckDistinfo,
+ CheckExtra,
+ CheckGlobal,
+ CheckInstall,
+ CheckMakefile,
+ CheckMessage,
+ CheckMk,
+ CheckPatches,
+ CheckPlist bool
+
+ WarnAbsname,
+ WarnDirectcmd,
+ WarnExtra,
+ WarnOrder,
+ WarnPerm,
+ WarnPlistDepr,
+ WarnPlistSort,
+ WarnQuoting,
+ WarnSpace,
+ WarnStyle,
+ WarnTypes bool
+
+ Explain,
+ Autofix,
+ GccOutput,
+ PrintHelp,
+ DumpMakefile,
+ Import,
+ LogVerbose,
+ Profiling,
+ Quiet,
+ Recursive,
+ PrintAutofix,
+ PrintSource,
+ PrintVersion bool
+
+ LogOnly []string
+
+ args []string
+}
+
+type Hash struct {
+ hash string
+ line Line
+}
+
+// G is the abbreviation for "global state";
+// it is the only global variable in this Go package
+var G Pkglint
+
func main() {
G.logOut = NewSeparatorWriter(os.Stdout)
G.logErr = NewSeparatorWriter(os.Stderr)
trace.Out = os.Stdout
- os.Exit(new(Pkglint).Main(os.Args...))
+ os.Exit(G.Main(os.Args...))
}
-type Pkglint struct{}
-
// Main runs the main program with the given arguments.
// args[0] is the program name.
func (pkglint *Pkglint) Main(args ...string) (exitcode int) {
@@ -43,7 +135,7 @@ func (pkglint *Pkglint) Main(args ...str
return *exitcode
}
- if G.opts.Profiling {
+ if pkglint.opts.Profiling {
f, err := os.Create("pkglint.pprof")
if err != nil {
dummyLine.Fatalf("Cannot create profiling file: %s", err)
@@ -52,45 +144,45 @@ func (pkglint *Pkglint) Main(args ...str
defer pprof.StopCPUProfile()
regex.Profiling = true
- G.loghisto = histogram.New()
+ pkglint.loghisto = histogram.New()
defer func() {
- G.logOut.Write("")
- G.loghisto.PrintStats("loghisto", G.logOut.out, 0)
+ pkglint.logOut.Write("")
+ pkglint.loghisto.PrintStats("loghisto", pkglint.logOut.out, 0)
regex.PrintStats()
}()
}
- for _, arg := range G.opts.args {
- G.Todo = append(G.Todo, filepath.ToSlash(arg))
+ for _, arg := range pkglint.opts.args {
+ pkglint.Todo = append(pkglint.Todo, filepath.ToSlash(arg))
}
- if len(G.Todo) == 0 {
- G.Todo = []string{"."}
+ if len(pkglint.Todo) == 0 {
+ pkglint.Todo = []string{"."}
}
- G.globalData.Initialize()
+ pkglint.globalData.Initialize()
currentUser, err := user.Current()
if err == nil {
// On Windows, this is `Computername\Username`.
- G.CurrentUsername = regex.Compile(`^.*\\`).ReplaceAllString(currentUser.Username, "")
+ pkglint.CurrentUsername = regex.Compile(`^.*\\`).ReplaceAllString(currentUser.Username, "")
}
- for len(G.Todo) != 0 {
- item := G.Todo[0]
- G.Todo = G.Todo[1:]
+ for len(pkglint.Todo) != 0 {
+ item := pkglint.Todo[0]
+ pkglint.Todo = pkglint.Todo[1:]
pkglint.CheckDirent(item)
}
checkToplevelUnusedLicenses()
pkglint.PrintSummary()
- if G.errors != 0 {
+ if pkglint.errors != 0 {
return 1
}
return 0
}
func (pkglint *Pkglint) ParseCommandLine(args []string) *int {
- gopts := &G.opts
+ gopts := &pkglint.opts
opts := getopt.NewOptions()
check := opts.AddFlagGroup('C', "check", "check,...", "enable or disable specific checks")
@@ -138,21 +230,21 @@ func (pkglint *Pkglint) ParseCommandLine
remainingArgs, err := opts.Parse(args)
if err != nil {
- fmt.Fprintf(G.logErr.out, "%s\n\n", err)
- opts.Help(G.logErr.out, "pkglint [options] dir...")
+ fmt.Fprintf(pkglint.logErr.out, "%s\n\n", err)
+ opts.Help(pkglint.logErr.out, "pkglint [options] dir...")
exitcode := 1
return &exitcode
}
gopts.args = remainingArgs
if gopts.PrintHelp {
- opts.Help(G.logOut.out, "pkglint [options] dir...")
+ opts.Help(pkglint.logOut.out, "pkglint [options] dir...")
exitcode := 0
return &exitcode
}
- if G.opts.PrintVersion {
- fmt.Fprintf(G.logOut.out, "%s\n", confVersion)
+ if pkglint.opts.PrintVersion {
+ fmt.Fprintf(pkglint.logOut.out, "%s\n", confVersion)
exitcode := 0
return &exitcode
}
@@ -161,22 +253,22 @@ func (pkglint *Pkglint) ParseCommandLine
}
func (pkglint *Pkglint) PrintSummary() {
- if !G.opts.Quiet && !G.opts.Autofix {
- if G.errors != 0 || G.warnings != 0 {
- G.logOut.Printf("%d %s and %d %s found.\n",
- G.errors, ifelseStr(G.errors == 1, "error", "errors"),
- G.warnings, ifelseStr(G.warnings == 1, "warning", "warnings"))
+ if !pkglint.opts.Quiet && !pkglint.opts.Autofix {
+ if pkglint.errors != 0 || pkglint.warnings != 0 {
+ pkglint.logOut.Printf("%d %s and %d %s found.\n",
+ pkglint.errors, ifelseStr(pkglint.errors == 1, "error", "errors"),
+ pkglint.warnings, ifelseStr(pkglint.warnings == 1, "warning", "warnings"))
} else {
- G.logOut.WriteLine("Looks fine.")
+ pkglint.logOut.WriteLine("Looks fine.")
}
- if G.explanationsAvailable && !G.opts.Explain {
- G.logOut.WriteLine("(Run \"pkglint -e\" to show explanations.)")
+ if pkglint.explanationsAvailable && !pkglint.opts.Explain {
+ pkglint.logOut.WriteLine("(Run \"pkglint -e\" to show explanations.)")
}
- if G.autofixAvailable && !G.opts.PrintAutofix {
- G.logOut.WriteLine("(Run \"pkglint -fs\" to show what can be fixed automatically.)")
+ if pkglint.autofixAvailable && !pkglint.opts.PrintAutofix {
+ pkglint.logOut.WriteLine("(Run \"pkglint -fs\" to show what can be fixed automatically.)")
}
- if G.autofixAvailable && !G.opts.Autofix {
- G.logOut.WriteLine("(Run \"pkglint -F\" to automatically fix some issues.)")
+ if pkglint.autofixAvailable && !pkglint.opts.Autofix {
+ pkglint.logOut.WriteLine("(Run \"pkglint -F\" to automatically fix some issues.)")
}
}
}
@@ -194,13 +286,14 @@ func (pkglint *Pkglint) CheckDirent(fnam
isDir := st.Mode().IsDir()
isReg := st.Mode().IsRegular()
- G.CurrentDir = ifelseStr(isReg, path.Dir(fname), fname)
- absCurrentDir := abspath(G.CurrentDir)
- G.Wip = !G.opts.Import && matches(absCurrentDir, `/wip/|/wip$`)
- G.Infrastructure = matches(absCurrentDir, `/mk/|/mk$`)
- G.CurPkgsrcdir = findPkgsrcTopdir(G.CurrentDir)
- if G.CurPkgsrcdir == "" {
- NewLineWhole(fname).Errorf("Cannot determine the pkgsrc root directory for %q.", G.CurrentDir)
+ currentDir := ifelseStr(isReg, path.Dir(fname), fname)
+ pkglint.CurrentDir = currentDir
+ absCurrentDir := abspath(currentDir)
+ pkglint.Wip = !pkglint.opts.Import && matches(absCurrentDir, `/wip/|/wip$`)
+ pkglint.Infrastructure = matches(absCurrentDir, `/mk/|/mk$`)
+ pkglint.CurPkgsrcdir = findPkgsrcTopdir(currentDir)
+ if pkglint.CurPkgsrcdir == "" {
+ NewLineWhole(fname).Errorf("Cannot determine the pkgsrc root directory for %q.", currentDir)
return
}
@@ -208,13 +301,13 @@ func (pkglint *Pkglint) CheckDirent(fnam
case isDir && isEmptyDir(fname):
return
case isReg:
- Checkfile(fname)
+ pkglint.Checkfile(fname)
return
}
- switch G.CurPkgsrcdir {
+ switch pkglint.CurPkgsrcdir {
case "../..":
- checkdirPackage(relpath(G.globalData.Pkgsrcdir, G.CurrentDir))
+ pkglint.checkdirPackage(relpath(pkglint.globalData.Pkgsrcdir, currentDir))
case "..":
CheckdirCategory()
case ".":
@@ -365,14 +458,14 @@ func CheckfileMk(fname string) {
SaveAutofixChanges(lines)
}
-func Checkfile(fname string) {
+func (pkglint *Pkglint) Checkfile(fname string) {
if trace.Tracing {
defer trace.Call1(fname)()
}
basename := path.Base(fname)
if hasPrefix(basename, "work") || hasSuffix(basename, "~") || hasSuffix(basename, ".orig") || hasSuffix(basename, ".rej") {
- if G.opts.Import {
+ if pkglint.opts.Import {
NewLineWhole(fname).Errorf("Must be cleaned up before committing the package.")
}
return
@@ -414,45 +507,45 @@ func Checkfile(fname string) {
NewLineWhole(fname).Errorf("Only files and directories are allowed in pkgsrc.")
case basename == "ALTERNATIVES":
- if G.opts.CheckAlternatives {
+ if pkglint.opts.CheckAlternatives {
CheckfileExtra(fname)
}
case basename == "buildlink3.mk":
- if G.opts.CheckBuildlink3 {
+ if pkglint.opts.CheckBuildlink3 {
if lines := LoadNonemptyLines(fname, true); lines != nil {
ChecklinesBuildlink3Mk(NewMkLines(lines))
}
}
case hasPrefix(basename, "DESCR"):
- if G.opts.CheckDescr {
+ if pkglint.opts.CheckDescr {
if lines := LoadNonemptyLines(fname, false); lines != nil {
ChecklinesDescr(lines)
}
}
case basename == "distinfo":
- if G.opts.CheckDistinfo {
+ if pkglint.opts.CheckDistinfo {
if lines := LoadNonemptyLines(fname, false); lines != nil {
ChecklinesDistinfo(lines)
}
}
case basename == "DEINSTALL" || basename == "INSTALL":
- if G.opts.CheckInstall {
+ if pkglint.opts.CheckInstall {
CheckfileExtra(fname)
}
case hasPrefix(basename, "MESSAGE"):
- if G.opts.CheckMessage {
+ if pkglint.opts.CheckMessage {
if lines := LoadNonemptyLines(fname, false); lines != nil {
ChecklinesMessage(lines)
}
}
case matches(basename, `^patch-[-A-Za-z0-9_.~+]*[A-Za-z0-9_]$`):
- if G.opts.CheckPatches {
+ if pkglint.opts.CheckPatches {
if lines := LoadNonemptyLines(fname, false); lines != nil {
ChecklinesPatch(lines)
}
@@ -467,12 +560,12 @@ func Checkfile(fname string) {
NewLineWhole(fname).Warnf("Patch files should be named \"patch-\", followed by letters, '-', '_', '.', and digits only.")
case matches(basename, `^(?:.*\.mk|Makefile.*)$`) && !matches(fname, `files/`) && !matches(fname, `patches/`):
- if G.opts.CheckMk {
+ if pkglint.opts.CheckMk {
CheckfileMk(fname)
}
case hasPrefix(basename, "PLIST"):
- if G.opts.CheckPlist {
+ if pkglint.opts.CheckPlist {
if lines := LoadNonemptyLines(fname, false); lines != nil {
ChecklinesPlist(lines)
}
@@ -483,7 +576,7 @@ func Checkfile(fname string) {
case hasPrefix(basename, "CHANGES-"):
// This only checks the file, but doesn't register the changes globally.
- _ = G.globalData.loadDocChangesFromFile(fname)
+ _ = pkglint.globalData.loadDocChangesFromFile(fname)
case matches(fname, `(?:^|/)files/[^/]*$`):
// Skip
@@ -493,7 +586,7 @@ func Checkfile(fname string) {
default:
NewLineWhole(fname).Warnf("Unexpected file found.")
- if G.opts.CheckExtra {
+ if pkglint.opts.CheckExtra {
CheckfileExtra(fname)
}
}
Index: pkgsrc/pkgtools/pkglint/files/pkglint_test.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.15 pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.16
--- pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.15 Mon Feb 19 12:40:38 2018
+++ pkgsrc/pkgtools/pkglint/files/pkglint_test.go Sun Mar 4 20:34:33 2018
@@ -11,7 +11,7 @@ import (
func (s *Suite) Test_Pkglint_Main_help(c *check.C) {
t := s.Init(c)
- exitcode := new(Pkglint).Main("pkglint", "-h")
+ exitcode := G.Main("pkglint", "-h")
c.Check(exitcode, equals, 0)
c.Check(t.Output(), check.Matches, `^\Qusage: pkglint [options] dir...\E\n(?s).+`)
@@ -20,7 +20,7 @@ func (s *Suite) Test_Pkglint_Main_help(c
func (s *Suite) Test_Pkglint_Main_version(c *check.C) {
t := s.Init(c)
- exitcode := new(Pkglint).Main("pkglint", "--version")
+ exitcode := G.Main("pkglint", "--version")
c.Check(exitcode, equals, 0)
t.CheckOutputLines(
@@ -30,7 +30,7 @@ func (s *Suite) Test_Pkglint_Main_versio
func (s *Suite) Test_Pkglint_Main_no_args(c *check.C) {
t := s.Init(c)
- exitcode := new(Pkglint).Main("pkglint")
+ exitcode := G.Main("pkglint")
c.Check(exitcode, equals, 1)
t.CheckOutputLines(
@@ -40,20 +40,20 @@ func (s *Suite) Test_Pkglint_Main_no_arg
func (s *Suite) Test_Pkglint_Main__only(c *check.C) {
t := s.Init(c)
- exitcode := new(Pkglint).ParseCommandLine([]string{"pkglint", "-Wall", "-o", ":Q", "--version"})
+ exitcode := G.ParseCommandLine([]string{"pkglint", "-Wall", "-o", ":Q", "--version"})
if c.Check(exitcode, check.NotNil) {
c.Check(*exitcode, equals, 0)
}
c.Check(G.opts.LogOnly, deepEquals, []string{":Q"})
t.CheckOutputLines(
- "@VERSION@")
+ confVersion)
}
func (s *Suite) Test_Pkglint_Main__unknown_option(c *check.C) {
t := s.Init(c)
- exitcode := new(Pkglint).Main("pkglint", "--unknown-option")
+ exitcode := G.Main("pkglint", "--unknown-option")
c.Check(exitcode, equals, 1)
t.CheckOutputLines(
@@ -242,14 +242,14 @@ func (s *Suite) Test_Pkglint_Main__compl
"Size (checkperms-1.12.tar.gz) = 6621 bytes",
"SHA1 (patch-checkperms.c) = asdfasdf") // Invalid SHA-1 checksum
- new(Pkglint).Main("pkglint", "-Wall", "-Call", t.TempFilename("sysutils/checkperms"))
+ G.Main("pkglint", "-Wall", "-Call", t.TempFilename("sysutils/checkperms"))
t.CheckOutputLines(
"WARN: ~/sysutils/checkperms/Makefile:3: This package should be updated to 1.13 ([supports more file formats]).",
"ERROR: ~/sysutils/checkperms/Makefile:4: Invalid category \"tools\".",
"ERROR: ~/sysutils/checkperms/distinfo:7: SHA1 hash of patches/patch-checkperms.c differs "+
"(distinfo has asdfasdf, patch file has e775969de639ec703866c0336c4c8e0fdd96309c). "+
- "Run \"@BMAKE@ makepatchsum\".",
+ "Run \""+confMake+" makepatchsum\".",
"WARN: ~/sysutils/checkperms/patches/patch-checkperms.c:12: Premature end of patch hunk "+
"(expected 1 lines to be deleted and 0 lines to be added)",
"2 errors and 2 warnings found.",
@@ -266,7 +266,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
- new(Pkglint).Main(append([]string{"pkglint"}, splitOnSpace(cmdline)...)...)
+ G.Main(append([]string{"pkglint"}, splitOnSpace(cmdline)...)...)
}
}
@@ -275,7 +275,7 @@ func (s *Suite) Test_Pkglint_CheckDirent
t.SetupFileLines("empty")
- new(Pkglint).CheckDirent(t.TmpDir())
+ G.CheckDirent(t.TmpDir())
t.CheckOutputLines(
"ERROR: ~: Cannot determine the pkgsrc root directory for \"~\".")
@@ -289,24 +289,23 @@ func (s *Suite) Test_Pkglint_CheckDirent
t.SetupFileLines("category/Makefile")
t.SetupFileLines("Makefile")
G.globalData.Pkgsrcdir = t.TmpDir()
- pkglint := new(Pkglint)
- pkglint.CheckDirent(t.TmpDir())
+ G.CheckDirent(t.TmpDir())
t.CheckOutputLines(
"ERROR: ~/Makefile: Must not be empty.")
- pkglint.CheckDirent(t.TempFilename("category"))
+ G.CheckDirent(t.TempFilename("category"))
t.CheckOutputLines(
"ERROR: ~/category/Makefile: Must not be empty.")
- pkglint.CheckDirent(t.TempFilename("category/package"))
+ G.CheckDirent(t.TempFilename("category/package"))
t.CheckOutputLines(
"ERROR: ~/category/package/Makefile: Must not be empty.")
- pkglint.CheckDirent(t.TempFilename("category/package/nonexistent"))
+ G.CheckDirent(t.TempFilename("category/package/nonexistent"))
t.CheckOutputLines(
"ERROR: ~/category/package/nonexistent: No such file or directory.")
@@ -317,7 +316,7 @@ func (s *Suite) Test_resolveVariableRefs
mkline := t.NewMkLine("fname", 1, "GCC_VERSION=${GCC_VERSION}")
G.Pkg = NewPackage(".")
- G.Pkg.vardef["GCC_VERSION"] = mkline
+ G.Pkg.vars.Define("GCC_VERSION", mkline)
resolved := resolveVariableRefs("gcc-${GCC_VERSION}")
@@ -340,12 +339,15 @@ func (s *Suite) Test_resolveVariableRefs
c.Check(resolved, equals, "you got it")
}
+// Usually, a dot in a variable name means a parameterized form.
+// In this case, it is part of a version number. Resolving these
+// variables from the scope works nevertheless.
func (s *Suite) Test_resolveVariableRefs__special_chars(c *check.C) {
t := s.Init(c)
mkline := t.NewMkLine("fname", 10, "_=x11")
G.Pkg = NewPackage("category/pkg")
- G.Pkg.vardef["GST_PLUGINS0.10_TYPE"] = mkline
+ G.Pkg.vars.Define("GST_PLUGINS0.10_TYPE", mkline)
resolved := resolveVariableRefs("gst-plugins0.10-${GST_PLUGINS0.10_TYPE}/distinfo")
Index: pkgsrc/pkgtools/pkglint/files/plist.go
diff -u pkgsrc/pkgtools/pkglint/files/plist.go:1.22 pkgsrc/pkgtools/pkglint/files/plist.go:1.23
--- pkgsrc/pkgtools/pkglint/files/plist.go:1.22 Mon Feb 19 12:40:38 2018
+++ pkgsrc/pkgtools/pkglint/files/plist.go Sun Mar 4 20:34:33 2018
@@ -182,7 +182,7 @@ func (ck *PlistChecker) checkpath(pline
ck.checkpathShare(pline)
}
- if contains(text, "${PKGLOCALEDIR}") && G.Pkg != nil && G.Pkg.vardef["USE_PKGLOCALEDIR"] == nil {
+ if contains(text, "${PKGLOCALEDIR}") && G.Pkg != nil && !G.Pkg.vars.Defined("USE_PKGLOCALEDIR") {
line.Warnf("PLIST contains ${PKGLOCALEDIR}, but USE_PKGLOCALEDIR was not found.")
}
@@ -258,7 +258,7 @@ func (ck *PlistChecker) checkpathInfo(pl
return
}
- if G.Pkg != nil && G.Pkg.vardef["INFO_FILES"] == nil {
+ if G.Pkg != nil && !G.Pkg.vars.Defined("INFO_FILES") {
pline.line.Warnf("Packages that install info files should set INFO_FILES.")
}
}
@@ -279,11 +279,8 @@ func (ck *PlistChecker) checkpathLib(pli
switch ext := path.Ext(basename); ext {
case ".a", ".la", ".so":
- if G.opts.WarnExtra && dirname == "lib" && !hasPrefix(basename, "lib") {
- pline.line.Warnf("Library filename %q should start with \"lib\".", basename)
- }
if ext == "la" {
- if G.Pkg != nil && G.Pkg.vardef["USE_LIBTOOL"] == nil {
+ if G.Pkg != nil && !G.Pkg.vars.Defined("USE_LIBTOOL") {
pline.line.Warnf("Packages that install libtool libraries should define USE_LIBTOOL.")
}
}
@@ -360,7 +357,7 @@ func (ck *PlistChecker) checkpathShare(p
}
}
- if contains(text[12:], "/") && G.Pkg.vardef["ICON_THEMES"] == nil && ck.once.FirstTime("ICON_THEMES") {
+ if contains(text[12:], "/") && !G.Pkg.vars.Defined("ICON_THEMES") && ck.once.FirstTime("ICON_THEMES") {
line.Warnf("Packages that install icon theme files should set ICON_THEMES.")
}
Index: pkgsrc/pkgtools/pkglint/files/plist_test.go
diff -u pkgsrc/pkgtools/pkglint/files/plist_test.go:1.21 pkgsrc/pkgtools/pkglint/files/plist_test.go:1.22
--- pkgsrc/pkgtools/pkglint/files/plist_test.go:1.21 Mon Feb 19 12:40:38 2018
+++ pkgsrc/pkgtools/pkglint/files/plist_test.go Sun Mar 4 20:34:33 2018
@@ -36,7 +36,6 @@ func (s *Suite) Test_ChecklinesPlist(c *
"Please use the CONF_FILES framework, which is described in mk/pkginstall/bsd.pkginstall.mk.",
"ERROR: PLIST:4: RCD_SCRIPTS must not be registered in the PLIST. Please use the RCD_SCRIPTS framework.",
"ERROR: PLIST:6: \"info/dir\" must not be listed. Use install-info to add/remove an entry.",
- "WARN: PLIST:7: Library filename \"c.so\" should start with \"lib\".",
"WARN: PLIST:8: Redundant library found. The libtool library is in line 9.",
"WARN: PLIST:9: \"lib/libc.la\" should be sorted before \"lib/libc.so.6\".",
"WARN: PLIST:10: Preformatted manual page without unformatted one.",
Index: pkgsrc/pkgtools/pkglint/files/shell.go
diff -u pkgsrc/pkgtools/pkglint/files/shell.go:1.21 pkgsrc/pkgtools/pkglint/files/shell.go:1.22
--- pkgsrc/pkgtools/pkglint/files/shell.go:1.21 Mon Feb 19 12:40:38 2018
+++ pkgsrc/pkgtools/pkglint/files/shell.go Sun Mar 4 20:34:33 2018
@@ -457,7 +457,7 @@ func (scc *SimpleCommandChecker) checkCo
case scc.handleCommandVariable():
case matches(shellword, `^(?::|break|cd|continue|eval|exec|exit|export|read|set|shift|umask|unset)$`):
case hasPrefix(shellword, "./"): // All commands from the current directory are fine.
- case hasPrefix(shellword, "${PKGSRCDIR"): // With or without the :Q modifier
+ case matches(shellword, `\$\{(PKGSRCDIR|PREFIX)(:Q)?\}`):
case scc.handleComment():
default:
if G.opts.WarnExtra && !(G.Mk != nil && G.Mk.indentation.DependsOn("OPSYS")) {
@@ -520,7 +520,9 @@ func (scc *SimpleCommandChecker) handleC
}
shellword := scc.strcmd.Name
- if m, varname := match1(shellword, `^\$\{([\w_]+)\}$`); m {
+ parser := NewMkParser(scc.shline.mkline.Line, shellword, false)
+ if varuse := parser.VarUse(); varuse != nil && parser.EOF() {
+ varname := varuse.varname
if tool := G.globalData.Tools.byVarname[varname]; tool != nil {
if !G.Mk.tools[tool.Name] {
@@ -537,7 +539,10 @@ func (scc *SimpleCommandChecker) handleC
// When the package author has explicitly defined a command
// variable, assume it to be valid.
- if G.Pkg != nil && G.Pkg.vardef[varname] != nil {
+ if G.Mk != nil && G.Mk.vars.DefinedSimilar(varname) {
+ return true
+ }
+ if G.Pkg != nil && G.Pkg.vars.DefinedSimilar(varname) {
return true
}
}
Index: pkgsrc/pkgtools/pkglint/files/shell_test.go
diff -u pkgsrc/pkgtools/pkglint/files/shell_test.go:1.21 pkgsrc/pkgtools/pkglint/files/shell_test.go:1.22
--- pkgsrc/pkgtools/pkglint/files/shell_test.go:1.21 Mon Feb 19 12:40:38 2018
+++ pkgsrc/pkgtools/pkglint/files/shell_test.go Sun Mar 4 20:34:33 2018
@@ -109,6 +109,7 @@ func (s *Suite) Test_ShellLine_CheckShel
t := s.Init(c)
t.SetupCommandLine("-Wall")
+ G.globalData.InitVartypes()
checkShellCommandLine := func(shellCommand string) {
G.Mk = t.NewMkLines("fname",
@@ -122,7 +123,7 @@ func (s *Suite) Test_ShellLine_CheckShel
t.CheckOutputEmpty()
- checkShellCommandLine("uname=`uname`; echo $$uname; echo")
+ checkShellCommandLine("uname=`uname`; echo $$uname; echo; ${PREFIX}/bin/command")
t.CheckOutputLines(
"WARN: fname:1: Unknown shell command \"uname\".",
@@ -466,6 +467,7 @@ func (s *Suite) Test_ShellLine_CheckShel
text := "\tfor f in *.pl; do ${SED} s,@PREFIX@,${PREFIX}, < $f > $f.tmp && ${MV} $f.tmp $f; done"
shline := t.NewShellLine("Makefile", 3, text)
+ shline.mkline.Tokenize(shline.mkline.Shellcmd())
shline.CheckShellCommandLine(text)
t.CheckOutputLines(
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.21 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.22
--- pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.21 Sat Jan 27 18:50:36 2018
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go Sun Mar 4 20:34:33 2018
@@ -80,13 +80,20 @@ func (s *Suite) Test_VartypeCheck_CFlag(
func (s *Suite) Test_VartypeCheck_Comment(c *check.C) {
t := s.Init(c)
+ G.Pkg = NewPackage("category/converter")
+ G.Pkg.EffectivePkgbase = "converter"
+
runVartypeChecks(t, "COMMENT", opAssign, (*VartypeCheck).Comment,
"Versatile Programming Language",
"TODO: Short description of the package",
"A great package.",
"some packages need a very very long comment to explain their basic usefulness",
"\"Quoting the comment is wrong\"",
- "'Quoting the comment is wrong'")
+ "'Quoting the comment is wrong'",
+ "Package is a great package",
+ "Package is an awesome package",
+ "The Big New Package is a great package",
+ "Converter converts between measurement units")
t.CheckOutputLines(
"ERROR: fname:2: COMMENT must be set.",
@@ -95,7 +102,11 @@ func (s *Suite) Test_VartypeCheck_Commen
"WARN: fname:4: COMMENT should start with a capital letter.",
"WARN: fname:4: COMMENT should not be longer than 70 characters.",
"WARN: fname:5: COMMENT should not be enclosed in quotes.",
- "WARN: fname:6: COMMENT should not be enclosed in quotes.")
+ "WARN: fname:6: COMMENT should not be enclosed in quotes.",
+ "WARN: fname:7: COMMENT should not contain \"is a\".",
+ "WARN: fname:8: COMMENT should not contain \"is an\".",
+ "WARN: fname:9: COMMENT should not contain \"is a\".",
+ "WARN: fname:10: COMMENT should not start with the package name.")
}
func (s *Suite) Test_VartypeCheck_ConfFiles(c *check.C) {
@@ -616,6 +627,7 @@ func runVartypeChecks(t *Tester, varname
}
for i, value := range values {
mkline := t.NewMkLine("fname", i+1, varname+op.String()+value)
+ mkline.Tokenize(mkline.Value())
valueNovar := mkline.WithoutMakeVariables(mkline.Value())
vc := &VartypeCheck{mkline, mkline.Line, mkline.Varname(), mkline.Op(), mkline.Value(), valueNovar, "", false}
checker(vc)
Index: pkgsrc/pkgtools/pkglint/files/util.go
diff -u pkgsrc/pkgtools/pkglint/files/util.go:1.19 pkgsrc/pkgtools/pkglint/files/util.go:1.20
--- pkgsrc/pkgtools/pkglint/files/util.go:1.19 Sat Jan 27 18:50:36 2018
+++ pkgsrc/pkgtools/pkglint/files/util.go Sun Mar 4 20:34:33 2018
@@ -182,6 +182,18 @@ func detab(s string) string {
return detabbed
}
+func shorten(s string, maxChars int) string {
+ chars := 0
+ for i := range s {
+ if chars >= maxChars {
+ return s[:i] + "..."
+ break
+ }
+ chars++
+ }
+ return s
+}
+
func varnameBase(varname string) string {
dot := strings.IndexByte(varname, '.')
if dot != -1 {
@@ -204,34 +216,30 @@ func varnameParam(varname string) string
return ""
}
+// defineVar marks a variable as defined in both the current package and the current file.
func defineVar(mkline MkLine, varname string) {
if G.Mk != nil {
- G.Mk.DefineVar(mkline, varname)
+ G.Mk.vars.Define(varname, mkline)
}
if G.Pkg != nil {
- G.Pkg.defineVar(mkline, varname)
+ G.Pkg.vars.Define(varname, mkline)
}
}
+
+// varIsDefined 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 {
- varcanon := varnameCanon(varname)
- if G.Mk != nil && (G.Mk.vardef[varname] != nil || G.Mk.vardef[varcanon] != nil) {
- return true
- }
- if G.Pkg != nil && (G.Pkg.vardef[varname] != nil || G.Pkg.vardef[varcanon] != nil) {
- return true
- }
- return false
+ return G.Mk != nil && G.Mk.vars.DefinedSimilar(varname) ||
+ G.Pkg != nil && G.Pkg.vars.DefinedSimilar(varname)
}
+// varIsUsed 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 {
- varcanon := varnameCanon(varname)
- if G.Mk != nil && (G.Mk.varuse[varname] != nil || G.Mk.varuse[varcanon] != nil) {
- return true
- }
- if G.Pkg != nil && (G.Pkg.varuse[varname] != nil || G.Pkg.varuse[varcanon] != nil) {
- return true
- }
- return false
+ return G.Mk != nil && G.Mk.vars.UsedSimilar(varname) ||
+ G.Pkg != nil && G.Pkg.vars.UsedSimilar(varname)
}
func splitOnSpace(s string) []string {
@@ -372,3 +380,94 @@ func (o *Once) FirstTime(what string) bo
o.seen[what] = true
return true
}
+
+// Scope remembers which variables are defined and which are used
+// in a certain scope, such as a package or a file.
+type Scope struct {
+ defined map[string]MkLine
+ used map[string]MkLine
+}
+
+func NewScope() Scope {
+ return Scope{make(map[string]MkLine), make(map[string]MkLine)}
+}
+
+// Define marks the variable and its canonicalized form as defined.
+func (s *Scope) Define(varname string, line MkLine) {
+ if s.defined[varname] == nil {
+ s.defined[varname] = line
+ if trace.Tracing {
+ trace.Step2("Defining %q in line %s", varname, line.Linenos())
+ }
+ }
+ varcanon := varnameCanon(varname)
+ if varcanon != varname && s.defined[varcanon] == nil {
+ s.defined[varcanon] = line
+ if trace.Tracing {
+ trace.Step2("Defining %q in line %s", varcanon, line.Linenos())
+ }
+ }
+}
+
+// Use marks the variable and its canonicalized form as used.
+func (s *Scope) Use(varname string, line MkLine) {
+ if s.used[varname] == nil {
+ s.used[varname] = line
+ if trace.Tracing {
+ trace.Step2("Using %q in line %s", varname, line.Linenos())
+ }
+ }
+ varcanon := varnameCanon(varname)
+ if varcanon != varname && s.used[varcanon] == nil {
+ s.used[varcanon] = line
+ if trace.Tracing {
+ trace.Step2("Using %q in line %s", varcanon, line.Linenos())
+ }
+ }
+}
+
+// Defined tests whether the variable is defined.
+// It does NOT test the canonicalized variable name.
+func (s *Scope) Defined(varname string) bool {
+ return s.defined[varname] != nil
+}
+
+// DefinedSimilar tests whether the variable or its canonicalized form is defined.
+func (s *Scope) DefinedSimilar(varname string) bool {
+ if s.defined[varname] != nil {
+ if trace.Tracing {
+ trace.Step1("Variable %q is defined", varname)
+ }
+ return true
+ }
+ varcanon := varnameCanon(varname)
+ if s.defined[varcanon] != nil {
+ if trace.Tracing {
+ trace.Step2("Variable %q (similar to %q) is defined", varcanon, varname)
+ }
+ return true
+ }
+ return false
+}
+
+// Used tests whether the variable is used.
+// It does NOT test the canonicalized variable name.
+func (s *Scope) Used(varname string) bool {
+ return s.used[varname] != nil
+}
+
+// UsedSimilar tests whether the variable or its canonicalized form is used.
+func (s *Scope) UsedSimilar(varname string) bool {
+ if s.used[varname] != nil {
+ return true
+ }
+ return s.used[varnameCanon(varname)] != nil
+}
+
+func (s *Scope) FirstDefinition(varname string) MkLine {
+ return s.defined[varname]
+}
+
+func (s *Scope) FirstUse(varname string) MkLine {
+ return s.used[varname]
+}
Index: pkgsrc/pkgtools/pkglint/files/vardefs.go
diff -u pkgsrc/pkgtools/pkglint/files/vardefs.go:1.36 pkgsrc/pkgtools/pkglint/files/vardefs.go:1.37
--- pkgsrc/pkgtools/pkglint/files/vardefs.go:1.36 Mon Feb 19 12:40:38 2018
+++ pkgsrc/pkgtools/pkglint/files/vardefs.go Sun Mar 4 20:34:33 2018
@@ -362,6 +362,7 @@ func (gd *GlobalData) InitVartypes() {
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_AUTO_DIRS.*", lkNone, BtYesNo, "buildlink3.mk: append")
acl("BUILDLINK_CONTENTS_FILTER", lkNone, BtShellCommand, "")
sys("BUILDLINK_CFLAGS", lkShell, BtCFlag)
bl3list("BUILDLINK_CFLAGS.*", lkShell, BtCFlag)
@@ -579,6 +580,7 @@ func (gd *GlobalData) InitVartypes() {
pkg("GITHUB_TYPE", lkNone, enum("tag release"))
pkg("GMAKE_REQD", lkNone, BtVersion)
acl("GNU_ARCH", lkNone, enum("mips"), "")
+ acl("GNU_ARCH.*", lkNone, BtIdentifier, "buildlink3.mk:; *: set, use")
acl("GNU_CONFIGURE", lkNone, BtYes, "Makefile, Makefile.common: set")
acl("GNU_CONFIGURE_INFODIR", lkNone, BtPathname, "Makefile, Makefile.common: set")
acl("GNU_CONFIGURE_LIBDIR", lkNone, BtPathname, "Makefile, Makefile.common: set")
@@ -969,6 +971,7 @@ func (gd *GlobalData) InitVartypes() {
acl("USE_TOOLS", lkShell, BtTool, "*: append")
acl("USE_TOOLS.*", lkShell, BtTool, "*: append")
pkg("USE_X11", lkNone, BtYes)
+ sys("WARNINGS", lkShell, BtShellWord)
sys("WARNING_MSG", lkNone, BtShellCommand)
sys("WARNING_CAT", lkNone, BtShellCommand)
acl("WRAPPER_REORDER_CMDS", lkShell, BtWrapperReorder, "Makefile, Makefile.common, buildlink3.mk: append")
@@ -980,6 +983,14 @@ func (gd *GlobalData) InitVartypes() {
usr("XAW_TYPE", lkNone, enum("3d neXtaw standard xpm"))
acl("XMKMF_FLAGS", lkShell, BtShellWord, "")
pkglist("_WRAP_EXTRA_ARGS.*", lkShell, BtShellWord)
+
+ // Only for infrastructure files; see mk/misc/show.mk
+ acl("_VARGROUPS", lkSpace, BtIdentifier, "*: append")
+ acl("_USER_VARS.*", lkSpace, BtIdentifier, "*: append")
+ acl("_PKG_VARS.*", lkSpace, BtIdentifier, "*: append")
+ acl("_SYS_VARS.*", lkSpace, BtIdentifier, "*: append")
+ acl("_DEF_VARS.*", lkSpace, BtIdentifier, "*: append")
+ acl("_USE_VARS.*", lkSpace, BtIdentifier, "*: append")
}
func enum(values string) *BasicType {
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.29 pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.30
--- pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.29 Mon Feb 19 12:40:38 2018
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck.go Sun Mar 4 20:34:33 2018
@@ -193,6 +193,23 @@ func (cv *VartypeCheck) Comment() {
if m, first := match1(value, `^(?i)(a|an)\s`); m {
line.Warnf("COMMENT should not begin with %q.", first)
}
+ if m, isA := match1(value, ` (is a|is an) `); m {
+ line.Warnf("COMMENT should not contain %q.", isA)
+ Explain(
+ "The words \"package is a\" are redundant. Since every package comment",
+ "could start with them, it is better to remove this redundancy in all",
+ "cases.")
+ }
+ if G.Pkg != nil && G.Pkg.EffectivePkgbase != "" {
+ pkgbase := G.Pkg.EffectivePkgbase
+ if strings.HasPrefix(strings.ToLower(value), strings.ToLower(pkgbase+" ")) {
+ line.Warnf("COMMENT should not start with the package name.")
+ Explain(
+ "The COMMENT is usually displayed together with the package name.",
+ "Therefore it does not need to repeat the package name but should",
+ "provide additional information instead.")
+ }
+ }
if matches(value, `^[a-z]`) {
line.Warnf("COMMENT should start with a capital letter.")
}
Added files:
Index: pkgsrc/pkgtools/pkglint/files/codewalk.md
diff -u /dev/null pkgsrc/pkgtools/pkglint/files/codewalk.md:1.1
--- /dev/null Sun Mar 4 20:34:33 2018
+++ pkgsrc/pkgtools/pkglint/files/codewalk.md Sun Mar 4 20:34:33 2018
@@ -0,0 +1,212 @@
+# The pkglint tour
+
+> Note: I wish there were a tool for nicely rendering the below codewalk blocks.
+> If you know of such a tool, please tell me.
+> `godoc` codewalks don't count since [they are only supported for the Go distribution
+> itself](https://github.com/golang/go/issues/14369).
+>
+> See also:
+>
+> * [github/markup/#172](https://github.com/github/markup/issues/172#issuecomment-33241601) (closed)
+> * [MarkdownHelper](https://github.com/BurdetteLamar/MarkdownHelper)
+
+## The entry points
+
+### Running pkglint
+
+```codewalk
+file pkglint.go
+start ^func main
+end ^\}
+```
+
+When running pkglint, the `G` variable is set up first.
+It contains the whole global state of pkglint.
+(Except for some of the subpackages, which have to be initialized separately.)
+All the interesting code is in the `Pkglint` type.
+This code structure makes it easy to test the code.
+
+### Testing pkglint
+
+Very similar code is used to set up the test and tear it down again:
+
+```codewalk
+file check_test.go
+start ^func .* SetUpTest
+end ^\}
+```
+
+```codewalk
+file check_test.go
+start ^func .* TearDownTest
+end ^\}
+```
+
+## First contact: checking a single DESCR file
+
+To learn how pkglint works internally, it is a good idea to start with
+a very simple example.
+Since the `DESCR` files have a very simple structure (they only contain
+text for human consumption), they are the ideal target.
+Let's trace an invocation of the command `pkglint DESCR` down to where
+the actual checks happen.
+
+```codewalk
+file pkglint.go
+start func main
+```
+
+```codewalk
+file pkglint.go
+start ^[\t]if exitcode
+end ^\t\}$
+```
+
+Since there are no command line options starting with a hyphen, we can
+skip the command line parsing for this example.
+
+```codewalk
+file pkglint.go
+start ^[\t]for _, arg
+end ^\}
+```
+
+The argument `DESCR` is saved in the `TODO` list, and then the pkgsrc
+infrastructure data is loaded by `Initialize`.
+This must happen in this order because pkglint needs to determine the
+pkgsrc root directory, just in case there are two or more pkgsrc trees
+in the local system.
+The path of the pkgsrc directory is determined from the first command
+line argument, which in this file is `DESCR`. From there, the pkgsrc
+root is usually reachable via `../../`, and this is what pkglint tries.
+
+After the initialization of everything,
+all items from the TODO list are worked off and handed over to `CheckDirent`.
+
+```codewalk
+file pkglint.go
+start func CheckDirent
+```
+
+Since `DESCR` is a regular file, the next method to call is `Checkfile`.
+
+```codewalk
+file pkglint.go
+start func Checkfile
+```
+
+```codewalk
+file pkglint.go
+start /basename, "DESCR"/
+end ^$
+```
+
+When compared to the code blocks around this one, it looks strange that
+this one uses `hasPrefix` and the others use a direct string comparison.
+But indeed, there are a few packages that actually have `DESCR.common`
+files. So everything's fine here.
+
+At this point, the file is loaded and converted to lines.
+For DESCR files, this is very simple, so there's no need to dive into that.
+The actual checks usually work on `Line` objects instead of files
+because the lines offer nice methods for logging the diagnostics
+and for automatically fixing the text (in pkglint's `--autofix` mode).
+
+```codewalk
+file pkglint.go
+start func ChecklinesDescr
+end ^\}
+```
+
+Now we are where the actual action takes place.
+The code looks straight-forward here.
+First, each line is checked on its own,
+and the final check is for too long files.
+
+The call to `SaveAutofixChanges` at the end looks a bit strange
+since none of the visible checks fixes anything.
+The autofix feature must be hidden in one of the line checks,
+and indeed, the code for `CheckLineTrailingWhitespace` says:
+
+```codewalk
+file linechecker.go
+start ^func CheckLineTrailingWhitespace
+end ^\}
+```
+
+This code is a typical example for using the autofix feature.
+Some more details are described at the `Autofix` type itself:
+
+```codewalk
+file linechecker.go
+start /^type Autofix/ upwhile /^\/\//
+end /^type Autofix/
+```
+
+The journey ends here, and it hasn't been that difficult.
+If that was too easy, have a look at the complex cases here:
+
+```codewalk
+file mkline.go
+start /^func .* VariableNeedsQuoting
+```
+
+## Basic ingredients
+
+Pkglint checks packages, and a package consists of several different files.
+All pkgsrc files are text files, which are organized in lines.
+Most pkglint diagnostics refer to a specific line,
+therefore the `Line` type is responsible for producing the diagnostics.
+
+### Line
+
+Most checks in pkgsrc only need to look at a single line.
+Lines that are independent of the file type are implemented in the `Line` type.
+This type contains the methods `Errorf`, `Warnf` and `Notef` to produce diagnostics
+of the following form:
+
+```text
+WARN: Makefile:3: COMMENT should not start with "A" or "An".
+```
+
+The definition for the `Line` type is:
+
+```codewalk
+file line.go
+start ^type Line =
+```
+
+```codewalk
+file line.go
+start ^type LineImpl struct
+end ^\}
+```
+
+### MkLine
+
+```codewalk
+file mkline.go
+start ^type MkLine =
+```
+
+```codewalk
+file mkline.go
+start ^type MkLineImpl struct
+end ^\}
+```
+
+Most of the pkgsrc infrastructure is written in Makefiles.
+In these, there may be line continuations (the ones ending in backslash).
+Plus, they may contain Make variables of the form `${VARNAME}` or `${VARNAME:Modifiers}`,
+and these are handled specially.
+
+### ShellLine
+
+The instructions for building and installing packages are written in Shell.
+The `ShellLine` type provides methods for checking shell commands and their individual parts.
+
+```codewalk
+file shell.go
+start ^type ShellLine struct
+end ^\}
+```
Home |
Main Index |
Thread Index |
Old Index