diff options
Diffstat (limited to 'pkgtools/pkglint')
22 files changed, 240 insertions, 244 deletions
diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile index 0f70a880576..c04fe41b2ac 100644 --- a/pkgtools/pkglint/Makefile +++ b/pkgtools/pkglint/Makefile @@ -1,6 +1,6 @@ -# $NetBSD: Makefile,v 1.470 2015/12/05 08:54:08 rillig Exp $ +# $NetBSD: Makefile,v 1.471 2015/12/05 15:16:29 rillig Exp $ -PKGNAME= pkglint-5.2 +PKGNAME= pkglint-5.2.1 DISTFILES= # none CATEGORIES= pkgtools diff --git a/pkgtools/pkglint/files/check_test.go b/pkgtools/pkglint/files/check_test.go index 25b10d644a6..104e2c4f6bf 100644 --- a/pkgtools/pkglint/files/check_test.go +++ b/pkgtools/pkglint/files/check_test.go @@ -50,10 +50,6 @@ func (s *Suite) UseCommandLine(c *check.C, args ...string) { } } -func (s *Suite) DummyLine() *Line { - return NewLine("fname", "1", "dummy", nil) -} - func (s *Suite) CreateTmpFile(c *check.C, fname, content string) { if s.tmpdir == "" { s.tmpdir = filepath.ToSlash(c.MkDir()) diff --git a/pkgtools/pkglint/files/deprecated_test.go b/pkgtools/pkglint/files/deprecated_test.go index 37687c31624..a3415a79307 100644 --- a/pkgtools/pkglint/files/deprecated_test.go +++ b/pkgtools/pkglint/files/deprecated_test.go @@ -7,8 +7,8 @@ import ( func (s *Suite) TestDeprecated(c *check.C) { G.globalData.deprecated = getDeprecatedVars() - line := NewLine("Makefile", "5", "dummy", nil) - checklineMkVarassign(line, "USE_PERL5", "=", "yes", "") + line := NewLine("Makefile", "5", "USE_PERL5=\tyes", nil) + NewMkLine(line).checkVarassign() c.Check(s.Output(), equals, "WARN: Makefile:5: Definition of USE_PERL5 is deprecated. Use USE_TOOLS+=perl or USE_TOOLS+=perl:run instead.\n") } diff --git a/pkgtools/pkglint/files/distinfo.go b/pkgtools/pkglint/files/distinfo.go index 7e54085d245..c9630552c1c 100644 --- a/pkgtools/pkglint/files/distinfo.go +++ b/pkgtools/pkglint/files/distinfo.go @@ -115,7 +115,7 @@ func (ck *distinfoLinesChecker) checkUnrecordedPatches() { for _, file := range files { patch := file.Name() - if !ck.patches[patch] { + if file.Mode().IsRegular() && !ck.patches[patch] { errorf(ck.distinfoFilename, noLines, "patch %q is not recorded. Run \"%s makepatchsum\".", ck.patchdir+"/"+patch, confMake) } } diff --git a/pkgtools/pkglint/files/distinfo_test.go b/pkgtools/pkglint/files/distinfo_test.go index 8b55439d188..c01fb45ba27 100644 --- a/pkgtools/pkglint/files/distinfo_test.go +++ b/pkgtools/pkglint/files/distinfo_test.go @@ -61,6 +61,7 @@ func (s *Suite) TestChecklinesDistinfo_UncommittedPatch(c *check.C) { } func (s *Suite) TestChecklinesDistinfo_UnrecordedPatches(c *check.C) { + s.CreateTmpFile(c, "patches/CVS/Entries", "") s.CreateTmpFile(c, "patches/patch-aa", "") s.CreateTmpFile(c, "patches/patch-src-Makefile", "") G.currentDir = s.tmpdir diff --git a/pkgtools/pkglint/files/line.go b/pkgtools/pkglint/files/line.go index d5d53398698..f54f53d5e6e 100644 --- a/pkgtools/pkglint/files/line.go +++ b/pkgtools/pkglint/files/line.go @@ -56,9 +56,9 @@ func (ln *Line) printSource(out io.Writer) { } } -func (ln *Line) fatalf(format string, args ...interface{}) bool { +func (ln *Line) fatalf(format string, args ...interface{}) { ln.printSource(G.logErr) - return fatalf(ln.fname, ln.lines, format, args...) + fatalf(ln.fname, ln.lines, format, args...) } func (ln *Line) errorf(format string, args ...interface{}) bool { ln.printSource(G.logOut) diff --git a/pkgtools/pkglint/files/line_test.go b/pkgtools/pkglint/files/line_test.go index 01bd9714609..e589b06cc09 100644 --- a/pkgtools/pkglint/files/line_test.go +++ b/pkgtools/pkglint/files/line_test.go @@ -4,7 +4,7 @@ import ( check "gopkg.in/check.v1" ) -func (s *Suite) TestLineAppendPrepend(c *check.C) { +func (s *Suite) TestLineModify(c *check.C) { line := NewLine("fname", "1", "dummy", []*RawLine{{1, "original\n"}}) c.Check(line.changed, equals, false) diff --git a/pkgtools/pkglint/files/logging.go b/pkgtools/pkglint/files/logging.go index bd76f6c343a..48db8890f61 100644 --- a/pkgtools/pkglint/files/logging.go +++ b/pkgtools/pkglint/files/logging.go @@ -13,16 +13,16 @@ type LogLevel struct { } var ( - llFatal = LogLevel{"FATAL", "fatal"} - llError = LogLevel{"ERROR", "error"} - llWarn = LogLevel{"WARN", "warning"} - llNote = LogLevel{"NOTE", "note"} - llDebug = LogLevel{"DEBUG", "debug"} + llFatal = &LogLevel{"FATAL", "fatal"} + llError = &LogLevel{"ERROR", "error"} + llWarn = &LogLevel{"WARN", "warning"} + llNote = &LogLevel{"NOTE", "note"} + llDebug = &LogLevel{"DEBUG", "debug"} ) var dummyLine = NewLine(noFile, noLines, "", nil) -func logMessage(level LogLevel, fname, lineno, message string) { +func logf(out io.Writer, level *LogLevel, fname, lineno, format string, args ...interface{}) bool { if fname != noFile { fname = cleanpath(fname) } @@ -43,40 +43,28 @@ func logMessage(level LogLevel, fname, lineno, message string) { text += sep + level.gccName + ":" sep = " " } - text += sep + message + "\n" - if level != llFatal { - io.WriteString(G.logOut, text) - } else { - io.WriteString(G.logErr, text) - } + text += sep + sprintf(format, args...) + "\n" + io.WriteString(out, text) + return true } -func fatalf(fname, lineno, format string, args ...interface{}) bool { - message := sprintf(format, args...) - logMessage(llFatal, fname, lineno, message) +func fatalf(fname, lineno, format string, args ...interface{}) { + logf(G.logErr, llFatal, fname, lineno, format, args...) panic(pkglintFatal{}) } func errorf(fname, lineno, format string, args ...interface{}) bool { - message := sprintf(format, args...) - logMessage(llError, fname, lineno, message) G.errors++ - return true + return logf(G.logOut, llError, fname, lineno, format, args...) } func warnf(fname, lineno, format string, args ...interface{}) bool { - message := sprintf(format, args...) - logMessage(llWarn, fname, lineno, message) G.warnings++ - return true + return logf(G.logOut, llWarn, fname, lineno, format, args...) } func notef(fname, lineno, format string, args ...interface{}) bool { - message := sprintf(format, args...) - logMessage(llNote, fname, lineno, message) - return true + return logf(G.logOut, llNote, fname, lineno, format, args...) } func debugf(fname, lineno, format string, args ...interface{}) bool { - message := sprintf(format, args...) - logMessage(llDebug, fname, lineno, message) - return true + return logf(G.logOut, llDebug, fname, lineno, format, args...) } type pkglintFatal struct{} diff --git a/pkgtools/pkglint/files/makefiles.go b/pkgtools/pkglint/files/makefiles.go index faf95c41b71..854fbffb9bb 100644 --- a/pkgtools/pkglint/files/makefiles.go +++ b/pkgtools/pkglint/files/makefiles.go @@ -168,6 +168,10 @@ func resolveVarsInRelativePath(relpath string, adjustDepth bool) string { func parselineMk(line *Line) { defer tracecall("parselineMk", line.text)() + if len(line.extra) != 0 { + return + } + text := line.text if m, varname, op, value, comment := matchVarassign(text); m { @@ -245,50 +249,6 @@ func ParselinesMk(lines []*Line) { } } -func checklineMkText(line *Line, text string) { - defer tracecall("checklineMkText", text)() - - if m, varname := match1(text, `^(?:[^#]*[^\$])?\$(\w+)`); m { - line.warnf("$%s is ambiguous. Use ${%s} if you mean a Makefile variable or $$%s if you mean a shell variable.", varname, varname, varname) - } - - if line.lines == "1" { - checklineRcsid(line, `# `, "# ") - } - - if contains(text, "${WRKSRC}/../") { - line.warnf("Using \"${WRKSRC}/..\" is conceptually wrong. Please use a combination of WRKSRC, CONFIGURE_DIRS and BUILD_DIRS instead.") - line.explain( - "You should define WRKSRC such that all of CONFIGURE_DIRS, BUILD_DIRS", - "and INSTALL_DIRS are subdirectories of it.") - } - - // Note: A simple -R is not detected, as the rate of false positives is too high. - if m, flag := match1(text, `\b(-Wl,--rpath,|-Wl,-rpath-link,|-Wl,-rpath,|-Wl,-R)\b`); m { - line.warnf("Please use ${COMPILER_RPATH_FLAG} instead of %q.", flag) - } - - rest := text - for { - m, r := replaceFirst(rest, `(?:^|[^$])\$\{([-A-Z0-9a-z_]+)(\.[\-0-9A-Z_a-z]+)?(?::[^\}]+)?\}`, "") - if m == nil { - break - } - rest = r - - varbase, varext := m[1], m[2] - varname := varbase + varext - varcanon := varnameCanon(varname) - instead := G.globalData.deprecated[varname] - if instead == "" { - instead = G.globalData.deprecated[varcanon] - } - if instead != "" { - line.warnf("Use of %q is deprecated. %s", varname, instead) - } - } -} - func ChecklinesMk(lines []*Line) { defer tracecall("ChecklinesMk", lines[0].fname)() @@ -368,9 +328,10 @@ func ChecklinesMk(lines []*Line) { } else if line.extra["is_comment"] != nil { // No further checks. - } else if m, varname, op, value, comment := matchVarassign(text); m { - ChecklineMkVaralign(line) - checklineMkVarassign(line, varname, op, value, comment) + } else if m, varname, op, value, _ := matchVarassign(text); m { + ml := NewMkLine(line) + ml.checkVaralign() + ml.checkVarassign() substcontext.Varassign(line, varname, op, value) } else if hasPrefix(text, "\t") { @@ -450,7 +411,7 @@ func ChecklinesMk(lines []*Line) { } } else if directive == "if" || directive == "elif" { - checklineMkIf(line, args) + NewMkLine(line).checkIf() } else if directive == "ifdef" || directive == "ifndef" { if matches(args, `\s`) { @@ -497,7 +458,7 @@ func ChecklinesMk(lines []*Line) { vucExtentWord, } for _, fvar := range extractUsedVariables(line, values) { - checklineMkVaruse(line, fvar, "", forLoopContext) + NewMkLine(line).checkVaruse(fvar, "", forLoopContext) } } diff --git a/pkgtools/pkglint/files/mkcond.go b/pkgtools/pkglint/files/mkcond.go index 4d4b869c196..0907105f686 100644 --- a/pkgtools/pkglint/files/mkcond.go +++ b/pkgtools/pkglint/files/mkcond.go @@ -28,29 +28,3 @@ func parseMkCond(line *Line, cond string) *Tree { } return NewTree("unknown", cond) } - -func checklineMkIf(line *Line, condition string) { - defer tracecall("checklineMkCond", condition)() - - tree := parseMkCond(line, condition) - - { - var pvarname, ppattern *string - if tree.Match(NewTree("not", NewTree("empty", NewTree("match", &pvarname, &ppattern)))) { - vartype := getVariableType(line, *pvarname) - if vartype != nil && vartype.checker.IsEnum() { - if !matches(*ppattern, `[\$\[*]`) && !vartype.checker.HasEnum(*ppattern) { - line.warnf("Invalid :M value %q. Only { %s } are allowed.", *ppattern, vartype.checker.AllowedEnums()) - } - } - return - } - } - - { - var pop, pvarname, pvalue *string - if tree.Match(NewTree("compareVarStr", &pvarname, &pop, &pvalue)) { - checklineMkVartype(line, *pvarname, "use", *pvalue, "") - } - } -} diff --git a/pkgtools/pkglint/files/mkcond_test.go b/pkgtools/pkglint/files/mkcond_test.go index 1af382a7b26..2272e939c43 100644 --- a/pkgtools/pkglint/files/mkcond_test.go +++ b/pkgtools/pkglint/files/mkcond_test.go @@ -23,19 +23,18 @@ func (s *Suite) TestParseMkCond_Compare(c *check.C) { func (s *Suite) TestChecklineMkCondition(c *check.C) { s.UseCommandLine(c, "-Wtypes") G.globalData.InitVartypes() - line := NewLine("fname", "1", "", nil) - checklineMkIf(line, "!empty(PKGSRC_COMPILER:Mmycc)") + NewMkLine(NewLine("fname", "1", ".if !empty(PKGSRC_COMPILER:Mmycc)", nil)).checkIf() c.Check(s.Stdout(), equals, "WARN: fname:1: Invalid :M value \"mycc\". "+ "Only { ccache ccc clang distcc f2c gcc hp icc ido gcc mipspro "+ "mipspro-ucode pcc sunpro xlc } are allowed.\n") - checklineMkIf(line, "${A} != ${B}") + NewMkLine(NewLine("fname", "1", ".elif ${A} != ${B}", nil)).checkIf() c.Check(s.Stdout(), equals, "") // Unknown condition types are silently ignored - checklineMkIf(line, "${HOMEPAGE} == \"mailto:someone@example.org\"") + NewMkLine(NewLine("fname", "1", ".if ${HOMEPAGE} == \"mailto:someone@example.org\"", nil)).checkIf() c.Check(s.Output(), equals, "WARN: fname:1: \"mailto:someone@example.org\" is not a valid URL.\n") } diff --git a/pkgtools/pkglint/files/mkline.go b/pkgtools/pkglint/files/mkline.go index b30ec6ff56d..e49bb35dc37 100644 --- a/pkgtools/pkglint/files/mkline.go +++ b/pkgtools/pkglint/files/mkline.go @@ -7,33 +7,41 @@ import ( "strings" ) -func checklineMkVardef(line *Line, varname, op string) { - defer tracecall("checklineMkVardef", varname, op)() +type MkLine struct { + line *Line +} + +func NewMkLine(line *Line) *MkLine { + parselineMk(line) + return &MkLine{line} +} - defineVar(line, varname) +func (ml *MkLine) checkVardef(varname, op string) { + defer tracecall("MkLine.checkVardef", varname, op)() + defineVar(ml.line, varname) + ml.checkVardefPermissions(varname, op) +} + +func (ml *MkLine) checkVardefPermissions(varname, op string) { if !G.opts.WarnPerm { return } + line := ml.line perms := getVariablePermissions(line, varname) var needed string switch op { - case "=": - needed = "s" - case "!=": + case "=", "!=", ":=": needed = "s" case "?=": needed = "d" case "+=": needed = "a" - case ":=": - needed = "s" } if !contains(perms, needed) { - // XXX: line.warnf("Permission %q requested for %s, but only { %s } is allowed.", - line.warnf("Permission [%s] requested for %s, but only [%s] is allowed.", + line.warnf("Permission %q requested for %s, but only { %s } are allowed.", ReadableVartypePermissions(needed), varname, ReadableVartypePermissions(perms)) line.explain( "Pkglint restricts the allowed actions on variables based on the filename.", @@ -52,9 +60,10 @@ func checklineMkVardef(line *Line, varname, op string) { } } -func checklineMkVaruse(line *Line, varname string, mod string, vuc *VarUseContext) { - defer tracecall("checklineMkVaruse", line, varname, mod, *vuc)() +func (ml *MkLine) checkVaruse(varname string, mod string, vuc *VarUseContext) { + defer tracecall("MkLine.checkVaruse", ml.line, varname, mod, *vuc)() + line := ml.line vartype := getVariableType(line, varname) if G.opts.WarnExtra && (vartype == nil || vartype.guessed == guGuessed) && @@ -63,22 +72,20 @@ func checklineMkVaruse(line *Line, varname string, mod string, vuc *VarUseContex line.warnf("%s is used but not defined. Spelling mistake?", varname) } - if G.opts.WarnPerm { - checklineMkVarusePerm(line, varname, vuc) - } + ml.checkVarusePermissions(varname, vuc) if varname == "LOCALBASE" && !G.isInfrastructure { - checklineMkVaruseLocalbase(line) + ml.warnVaruseLocalbase() } needsQuoting := variableNeedsQuoting(line, varname, vuc) if vuc.shellword == vucQuotFor { - checklineMkVaruseFor(line, varname, vartype, needsQuoting) + ml.checkVaruseFor(varname, vartype, needsQuoting) } if G.opts.WarnQuoting && vuc.shellword != vucQuotUnknown && needsQuoting != nqDontKnow { - checklineMkVaruseShellword(line, varname, vartype, vuc, mod, needsQuoting) + ml.checkVaruseShellword(varname, vartype, vuc, mod, needsQuoting) } if G.globalData.userDefinedVars[varname] != nil && !G.globalData.systemBuildDefs[varname] && !G.mkContext.buildDefs[varname] { @@ -92,7 +99,12 @@ func checklineMkVaruse(line *Line, varname string, mod string, vuc *VarUseContex } } -func checklineMkVarusePerm(line *Line, varname string, vuc *VarUseContext) { +func (ml *MkLine) checkVarusePermissions(varname string, vuc *VarUseContext) { + if !G.opts.WarnPerm { + return + } + + line := ml.line perms := getVariablePermissions(line, varname) isLoadTime := false // Will the variable be used at load time? @@ -141,7 +153,8 @@ func checklineMkVarusePerm(line *Line, varname string, vuc *VarUseContext) { } } -func checklineMkVaruseLocalbase(line *Line) { +func (ml *MkLine) warnVaruseLocalbase() { + line := ml.line line.warnf("The LOCALBASE variable should not be used by packages.") line.explain( // from jlam via private mail. @@ -172,7 +185,7 @@ func checklineMkVaruseLocalbase(line *Line) { " CONFIGURE_ENV+= --with-datafiles=${PREFIX}/share/battalion") } -func checklineMkVaruseFor(line *Line, varname string, vartype *Vartype, needsQuoting NeedsQuoting) { +func (ml *MkLine) checkVaruseFor(varname string, vartype *Vartype, needsQuoting NeedsQuoting) { switch { case vartype == nil: // Cannot check anything here. @@ -184,8 +197,8 @@ func checklineMkVaruseFor(line *Line, varname string, vartype *Vartype, needsQuo // Fine, this variable is not supposed to contain special characters. default: - line.warnf("The variable %s should not be used in .for loops.", varname) - line.explain( + ml.line.warnf("The variable %s should not be used in .for loops.", varname) + ml.line.explain( "The .for loop splits its argument at sequences of white-space, as", "opposed to all other places in make(1), which act like the shell.", "Therefore only variables that are specifically designed to match this", @@ -193,7 +206,7 @@ func checklineMkVaruseFor(line *Line, varname string, vartype *Vartype, needsQuo } } -func checklineMkVaruseShellword(line *Line, varname string, vartype *Vartype, vuc *VarUseContext, mod string, needsQuoting NeedsQuoting) { +func (ml *MkLine) checkVaruseShellword(varname string, vartype *Vartype, vuc *VarUseContext, mod string, needsQuoting NeedsQuoting) { // In GNU configure scripts, a few variables need to be // passed through the :M* operator before they reach the @@ -209,6 +222,7 @@ func checklineMkVaruseShellword(line *Line, varname string, vartype *Vartype, vu } correctMod := strippedMod + ifelseStr(needMstar, ":M*:Q", ":Q") + line := ml.line if mod == ":M*:Q" && !needMstar { line.notef("The :M* modifier is not needed here.") @@ -250,15 +264,15 @@ func checklineMkVaruseShellword(line *Line, varname string, vartype *Vartype, vu } } -func checklineMkDecreasingOrder(line *Line, varname, value string) { - defer tracecall("checklineMkDecreasingOrder", varname, value)() +func (ml *MkLine) checkDecreasingOrder(varname, value string) { + defer tracecall("MkLine.checkDecreasingOrder", varname, value)() strversions := splitOnSpace(value) intversions := make([]int, len(strversions)) for i, strversion := range strversions { iver, err := strconv.Atoi(strversion) if err != nil || !(iver > 0) { - line.errorf("All values for %s must be positive integers.", varname) + ml.line.errorf("All values for %s must be positive integers.", varname) return } intversions[i] = iver @@ -266,21 +280,26 @@ func checklineMkDecreasingOrder(line *Line, varname, value string) { for i, ver := range intversions { if i > 0 && ver >= intversions[i-1] { - line.warnf("The values for %s should be in decreasing order.", varname) - line.explain( + ml.line.warnf("The values for %s should be in decreasing order.", varname) + ml.line.explain( "If they aren't, it may be possible that needless versions of packages", "are installed.") } } } -func checklineMkVarassign(line *Line, varname, op, value, comment string) { - defer tracecall("checklineMkVarassign", varname, op, value)() +func (ml *MkLine) checkVarassign() { + defer tracecall("MkLine.checkVarassign")() + line := ml.line + varname := line.extra["varname"].(string) + op := line.extra["op"].(string) + value := line.extra["value"].(string) + comment := line.extra["comment"].(string) varbase := varnameBase(varname) varcanon := varnameCanon(varname) - checklineMkVardef(line, varname, op) + ml.checkVardef(varname, op) if G.opts.WarnExtra && op == "?=" && G.pkgContext != nil && !G.pkgContext.seenBsdPrefsMk { switch varbase { @@ -300,8 +319,8 @@ func checklineMkVarassign(line *Line, varname, op, value, comment string) { } } - checklineMkText(line, value) - checklineMkVartype(line, varname, op, value, comment) + ml.checkText(value) + ml.checkVartype(varname, op, value, comment) // If the variable is not used and is untyped, it may be a spelling mistake. if op == ":=" && varname == strings.ToLower(varname) { @@ -355,7 +374,7 @@ func checklineMkVarassign(line *Line, varname, op, value, comment string) { } if varname == "PYTHON_VERSIONS_ACCEPTED" { - checklineMkDecreasingOrder(line, varname, value) + ml.checkDecreasingOrder(varname, value) } if comment == "# defined" && !matches(varname, `.*(?:_MK|_COMMON)$`) { @@ -420,7 +439,7 @@ func checklineMkVarassign(line *Line, varname, op, value, comment string) { vucQuotUnknown, vucExtentUnknown} for _, usedVar := range usedVars { - checklineMkVaruse(line, usedVar, "", vuc) + ml.checkVaruse(usedVar, "", vuc) } } @@ -479,13 +498,14 @@ const reVarnamePlural = "^(?:" + "|TOOLS_NOOP" + ")$" -func checklineMkVartype(line *Line, varname, op, value, comment string) { - defer tracecall("checklineMkVartype", varname, op, value, comment)() +func (ml *MkLine) checkVartype(varname, op, value, comment string) { + defer tracecall("MkLine.checkVartype", varname, op, value, comment)() if !G.opts.WarnTypes { return } + line := ml.line varbase := varnameBase(varname) vartype := getVariableType(line, varname) @@ -508,7 +528,7 @@ func checklineMkVartype(line *Line, varname, op, value, comment string) { _ = G.opts.DebugMisc && line.debugf("Use of !=: %q", value) case vartype.kindOfList == lkNone: - checklineMkVartypePrimitive(line, varname, vartype.checker, op, value, comment, vartype.isConsideredList(), vartype.guessed) + ml.checkVartypePrimitive(varname, vartype.checker, op, value, comment, vartype.isConsideredList(), vartype.guessed) default: var words []string @@ -519,7 +539,7 @@ func checklineMkVartype(line *Line, varname, op, value, comment string) { } for _, word := range words { - checklineMkVartypePrimitive(line, varname, vartype.checker, op, word, comment, true, vartype.guessed) + ml.checkVartypePrimitive(varname, vartype.checker, op, word, comment, true, vartype.guessed) if vartype.kindOfList != lkSpace { checklineMkShellword(line, word, true) } @@ -530,23 +550,23 @@ func checklineMkVartype(line *Line, varname, op, value, comment string) { // The `op` parameter is one of `=`, `+=`, `:=`, `!=`, `?=`, `use`, `pp-use`, ``. // For some variables (like BuildlinkDepth), the operator influences the valid values. // The `comment` parameter comes from a variable assignment, when a part of the line is commented out. -func checklineMkVartypePrimitive(line *Line, varname string, checker *VarChecker, op, value, comment string, isList bool, guessed Guessed) { - defer tracecall("checklineMkVartypePrimitive", varname, op, value, comment, isList, guessed)() +func (ml *MkLine) checkVartypePrimitive(varname string, checker *VarChecker, op, value, comment string, isList bool, guessed Guessed) { + defer tracecall("MkLine.checkVartypePrimitive", varname, op, value, comment, isList, guessed)() - ctx := &VartypeCheck{line, varname, op, value, "", comment, isList, guessed == guGuessed} - ctx.valueNovar = withoutMakeVariables(line, value, isList) + ctx := &VartypeCheck{ml.line, varname, op, value, "", comment, isList, guessed == guGuessed} + ctx.valueNovar = ml.withoutMakeVariables(value, isList) checker.checker(ctx) } -func withoutMakeVariables(line *Line, value string, qModifierAllowed bool) string { +func (ml *MkLine) withoutMakeVariables(value string, qModifierAllowed bool) string { valueNovar := value for { var m []string if m, valueNovar = replaceFirst(valueNovar, `\$\{([^{}]*)\}`, ""); m != nil { varuse := m[1] if !qModifierAllowed && hasSuffix(varuse, ":Q") { - line.warnf("The :Q operator should only be used in lists and shell commands.") + ml.line.warnf("The :Q operator should only be used in lists and shell commands.") } } else { return valueNovar @@ -554,8 +574,8 @@ func withoutMakeVariables(line *Line, value string, qModifierAllowed bool) strin } } -func ChecklineMkVaralign(line *Line) { - text := line.text +func (ml *MkLine) checkVaralign() { + text := ml.line.text if m := regcomp(reVarassign).FindStringSubmatchIndex(text); m != nil { varname := text[m[2]:m[3]] space1 := text[m[3]:m[4]] @@ -563,14 +583,104 @@ func ChecklineMkVaralign(line *Line) { align := text[m[5]:m[6]] if G.opts.WarnSpace && align != " " && strings.Trim(align, "\t") != "" { - line.notef("Alignment of variable values should be done with tabs, not spaces.") + ml.line.notef("Alignment of variable values should be done with tabs, not spaces.") prefix := varname + space1 + op alignedWidth := tabLength(prefix + align) tabs := "" for tabLength(prefix+tabs) < alignedWidth { tabs += "\t" } - line.replace(prefix+align, prefix+tabs) + ml.line.replace(prefix+align, prefix+tabs) + } + } +} + +func (ml *MkLine) checkText(text string) { + defer tracecall("MkLine.checkText", text)() + + line := ml.line + if m, varname := match1(text, `^(?:[^#]*[^\$])?\$(\w+)`); m { + line.warnf("$%s is ambiguous. Use ${%s} if you mean a Makefile variable or $$%s if you mean a shell variable.", varname, varname, varname) + } + + if line.lines == "1" { + checklineRcsid(line, `# `, "# ") + } + + if contains(text, "${WRKSRC}/../") { + line.warnf("Using \"${WRKSRC}/..\" is conceptually wrong. Please use a combination of WRKSRC, CONFIGURE_DIRS and BUILD_DIRS instead.") + line.explain( + "You should define WRKSRC such that all of CONFIGURE_DIRS, BUILD_DIRS", + "and INSTALL_DIRS are subdirectories of it.") + } + + // Note: A simple -R is not detected, as the rate of false positives is too high. + if m, flag := match1(text, `\b(-Wl,--rpath,|-Wl,-rpath-link,|-Wl,-rpath,|-Wl,-R)\b`); m { + line.warnf("Please use ${COMPILER_RPATH_FLAG} instead of %q.", flag) + } + + rest := text + for { + m, r := replaceFirst(rest, `(?:^|[^$])\$\{([-A-Z0-9a-z_]+)(\.[\-0-9A-Z_a-z]+)?(?::[^\}]+)?\}`, "") + if m == nil { + break + } + rest = r + + varbase, varext := m[1], m[2] + varname := varbase + varext + varcanon := varnameCanon(varname) + instead := G.globalData.deprecated[varname] + if instead == "" { + instead = G.globalData.deprecated[varcanon] + } + if instead != "" { + line.warnf("Use of %q is deprecated. %s", varname, instead) + } + } +} + +func (ml *MkLine) checkAbsolutePathname(text string) { + defer tracecall("MkLine.checkAbsolutePathname", text)() + + // In the GNU coding standards, DESTDIR is defined as a (usually + // empty) prefix that can be used to install files to a different + // location from what they have been built for. Therefore + // everything following it is considered an absolute pathname. + // + // Another context where absolute pathnames usually appear is in + // assignments like "bindir=/bin". + if m, path := match1(text, `(?:^|\$[{(]DESTDIR[)}]|[\w_]+\s*=\s*)(/(?:[^"'\s]|"[^"*]"|'[^']*')*)`); m { + if matches(path, `^/\w`) { + checkwordAbsolutePathname(ml.line, path) + } + } +} + +func (ml *MkLine) checkIf() { + defer tracecall("MkLine.checkIf")() + + line := ml.line + condition := line.extra["args"].(string) + tree := parseMkCond(line, condition) + + { + var pvarname, ppattern *string + if tree.Match(NewTree("not", NewTree("empty", NewTree("match", &pvarname, &ppattern)))) { + vartype := getVariableType(line, *pvarname) + if vartype != nil && vartype.checker.IsEnum() { + if !matches(*ppattern, `[\$\[*]`) && !vartype.checker.HasEnum(*ppattern) { + line.warnf("Invalid :M value %q. Only { %s } are allowed.", *ppattern, vartype.checker.AllowedEnums()) + } + } + return + } + } + + { + var pop, pvarname, pvalue *string + if tree.Match(NewTree("compareVarStr", &pvarname, &pop, &pvalue)) { + NewMkLine(line).checkVartype(*pvarname, "use", *pvalue, "") } } } diff --git a/pkgtools/pkglint/files/mkline_test.go b/pkgtools/pkglint/files/mkline_test.go index c4d0657910c..1605e7537b6 100644 --- a/pkgtools/pkglint/files/mkline_test.go +++ b/pkgtools/pkglint/files/mkline_test.go @@ -7,29 +7,29 @@ import ( func (s *Suite) TestChecklineMkVartype_SimpleType(c *check.C) { s.UseCommandLine(c, "-Wtypes", "-Dunchecked") G.globalData.InitVartypes() - line := NewLine("fname", "1", "dummy", nil) + ml := NewMkLine(NewLine("fname", "1", "COMMENT=\tA nice package", nil)) vartype1 := G.globalData.vartypes["COMMENT"] c.Assert(vartype1, check.NotNil) c.Check(vartype1.guessed, equals, guNotGuessed) - vartype := getVariableType(line, "COMMENT") + vartype := getVariableType(ml.line, "COMMENT") c.Assert(vartype, check.NotNil) c.Check(vartype.checker.name, equals, "Comment") c.Check(vartype.guessed, equals, guNotGuessed) c.Check(vartype.kindOfList, equals, lkNone) - checklineMkVartype(line, "COMMENT", "=", "A nice package", "") + ml.checkVartype("COMMENT", "=", "A nice package", "") c.Check(s.Stdout(), equals, "WARN: fname:1: COMMENT should not begin with \"A\".\n") } func (s *Suite) TestChecklineMkVartype(c *check.C) { - line := NewLine("fname", "1", "dummy", nil) G.globalData.InitVartypes() + ml := NewMkLine(NewLine("fname", "1", "DISTNAME=gcc-${GCC_VERSION}", nil)) - checklineMkVartype(line, "DISTNAME", "=", "gcc-${GCC_VERSION}", "") + ml.checkVartype("DISTNAME", "=", "gcc-${GCC_VERSION}", "") } func (s *Suite) TestChecklineMkVaralign(c *check.C) { @@ -44,7 +44,7 @@ func (s *Suite) TestChecklineMkVaralign(c *check.C) { "VAR=\tvalue") // Already aligned with tabs only, left unchanged. for _, line := range lines { - ChecklineMkVaralign(line) + NewMkLine(line).checkVaralign() } c.Check(lines[0].changed, equals, true) @@ -76,3 +76,12 @@ func (s *Suite) TestChecklineMkVaralign(c *check.C) { "NOTE: file.mk:6: Autofix: replacing \"VAR= \\t\" with \"VAR=\\t\\t\".\n") c.Check(tabLength("VAR= \t"), equals, 16) } + +func (s *Suite) TestMkLine_CheckAbsolutePathname(c *check.C) { + ml := NewMkLine(NewLine("Makefile", "1", "# dummy", nil)) + + ml.checkAbsolutePathname("bindir=/bin") + ml.checkAbsolutePathname("bindir=/../lib") + + c.Check(s.Output(), equals, "WARN: Makefile:1: Found absolute pathname: /bin\n") +} diff --git a/pkgtools/pkglint/files/patches.go b/pkgtools/pkglint/files/patches.go index b6c620e9b67..98811531e14 100644 --- a/pkgtools/pkglint/files/patches.go +++ b/pkgtools/pkglint/files/patches.go @@ -524,11 +524,11 @@ func (ctx *CheckPatchContext) checkAddedContents() { switch *ctx.currentFiletype { case ftShell: case ftMakefile: - // This check is not as accurate as the similar one in checklineMkShelltext. + // This check is not as accurate as the similar one in MkLine.checkShelltext. shellwords, _ := splitIntoShellwords(line, addedText) for _, shellword := range shellwords { if !hasPrefix(shellword, "#") { - checklineMkAbsolutePathname(line, shellword) + NewMkLine(line).checkAbsolutePathname(shellword) } } case ftSource: diff --git a/pkgtools/pkglint/files/pkglint.go b/pkgtools/pkglint/files/pkglint.go index 020d79c80d2..8c024c1ebb2 100644 --- a/pkgtools/pkglint/files/pkglint.go +++ b/pkgtools/pkglint/files/pkglint.go @@ -281,42 +281,18 @@ func checklineTrailingWhitespace(line *Line) { func checklineRcsid(line *Line, prefixRe, suggestedPrefix string) bool { defer tracecall("checklineRcsid", prefixRe, suggestedPrefix)() - repartRcsid := "NetBSD" - if G.isWip { - repartRcsid = "(?:NetBSD|Id)" - } - - if !matches(line.text, `^`+prefixRe+`\$`+repartRcsid+`(?::[^\$]+)?\$$`) { + if !matches(line.text, `^`+prefixRe+`\$NetBSD(?::[^\$]+)?\$$`) { line.errorf("Expected %q.", suggestedPrefix+"$"+"NetBSD$") line.explain( "Several files in pkgsrc must contain the CVS Id, so that their current", "version can be traced back later from a binary package. This is to", - "ensure reproducible builds, for example for finding bugs.", - "", - "Please insert the text from the above error message (without the quotes)", - "at this position in the file.") + "ensure reproducible builds, for example for finding bugs.") + line.insertBefore(suggestedPrefix + "$" + "NetBSD$") return false } return true } -func checklineMkAbsolutePathname(line *Line, text string) { - defer tracecall("checklineMkAbsolutePathname", text)() - - // In the GNU coding standards, DESTDIR is defined as a (usually - // empty) prefix that can be used to install files to a different - // location from what they have been built for. Therefore - // everything following it is considered an absolute pathname. - // - // Another context where absolute pathnames usually appear is in - // assignments like "bindir=/bin". - if m, path := match1(text, `(?:^|\$[{(]DESTDIR[)}]|[\w_]+\s*=\s*)(/(?:[^"'\s]|"[^"*]"|'[^']*')*)`); m { - if matches(path, `^/\w`) { - checkwordAbsolutePathname(line, path) - } - } -} - func checklineRelativePath(line *Line, path string, mustExist bool) { if !G.isWip && contains(path, "/wip/") { line.errorf("A main pkgsrc package must not depend on a pkgsrc-wip package.") diff --git a/pkgtools/pkglint/files/pkglint_test.go b/pkgtools/pkglint/files/pkglint_test.go index 84f24727f86..8c111e6053e 100644 --- a/pkgtools/pkglint/files/pkglint_test.go +++ b/pkgtools/pkglint/files/pkglint_test.go @@ -99,24 +99,6 @@ func (s *Suite) TestChecklineRcsid(c *check.C) { "ERROR: fname:3: Expected \"$"+"NetBSD$\".\n"+ "ERROR: fname:4: Expected \"$"+"NetBSD$\".\n"+ "ERROR: fname:5: Expected \"$"+"NetBSD$\".\n") - - G.isWip = true - - for _, line := range lines { - checklineRcsid(line, ``, "") - } - - c.Check(s.Output(), equals, ""+ - "ERROR: fname:5: Expected \"$"+"NetBSD$\".\n") -} - -func (s *Suite) TestChecklineMkAbsolutePathname(c *check.C) { - line := NewLine("Makefile", "1", "dummy", nil) - - checklineMkAbsolutePathname(line, "bindir=/bin") - checklineMkAbsolutePathname(line, "bindir=/../lib") - - c.Check(s.Output(), equals, "WARN: Makefile:1: Found absolute pathname: /bin\n") } func (s *Suite) TestMatchVarassign(c *check.C) { diff --git a/pkgtools/pkglint/files/shell.go b/pkgtools/pkglint/files/shell.go index e2157b034f7..45bce010e21 100644 --- a/pkgtools/pkglint/files/shell.go +++ b/pkgtools/pkglint/files/shell.go @@ -15,7 +15,7 @@ func checklineMkShellcmdUse(line *Line, shellcmd string) { NewMkShellLine(line).checkCommandUse(shellcmd) } func checklineMkShellcmd(line *Line, shellcmd string) { - checklineMkText(line, shellcmd) + NewMkLine(line).checkText(shellcmd) NewMkShellLine(line).checklineMkShelltext(shellcmd) } @@ -104,7 +104,7 @@ func (msline *MkShellLine) checklineMkShellword(shellword string, checkQuoting b line := msline.line if m, varname, mod := match2(shellword, `^\$\{(`+reVarnameDirect+`)(:[^{}]+)?\}$`); m { - checklineMkVaruse(line, varname, mod, shellwordVuc) + NewMkLine(line).checkVaruse(varname, mod, shellwordVuc) return } @@ -223,7 +223,7 @@ outer: vucstate = vucQuotBackt } vuc := &VarUseContext{vucTimeUnknown, shellcommandContextType, vucstate, vucExtentWordpart} - checklineMkVaruse(line, varname, mod, vuc) + NewMkLine(line).checkVaruse(varname, mod, vuc) } // The syntax of the variable modifiers can get quite @@ -410,7 +410,7 @@ func (msline *MkShellLine) checklineMkShelltext(shelltext string) { st.checkCommandStart() st.checkConditionalCd() if state != scstPaxS && state != scstSedE && state != scstCaseLabel { - checklineMkAbsolutePathname(line, shellword) + NewMkLine(line).checkAbsolutePathname(shellword) } st.checkAutoMkdirs() st.checkInstallMulti() diff --git a/pkgtools/pkglint/files/shell_test.go b/pkgtools/pkglint/files/shell_test.go index 013b6d52c9a..489136e59d0 100644 --- a/pkgtools/pkglint/files/shell_test.go +++ b/pkgtools/pkglint/files/shell_test.go @@ -21,7 +21,7 @@ func (s *Suite) TestSplitIntoShellwords_LineContinuation(c *check.C) { func (s *Suite) TestChecklineMkShelltext(c *check.C) { s.UseCommandLine(c, "-Wall") G.mkContext = newMkContext() - msline := NewMkShellLine(NewLine("fname", "1", "dummy", nil)) + msline := NewMkShellLine(NewLine("fname", "1", "# dummy", nil)) msline.checklineMkShelltext("@# Comment") @@ -87,7 +87,7 @@ func (s *Suite) TestChecklineMkShelltext(c *check.C) { func (s *Suite) TestChecklineMkShellword(c *check.C) { s.UseCommandLine(c, "-Wall") G.globalData.InitVartypes() - line := NewLine("fname", "1", "dummy", nil) + line := NewLine("fname", "1", "# dummy", nil) c.Check(matches("${list}", `^`+reVarnameDirect+`$`), equals, false) @@ -106,7 +106,7 @@ func (s *Suite) TestShelltextContext_CheckCommandStart(c *check.C) { G.globalData.vartools = map[string]string{"echo": "ECHO"} G.globalData.toolsVarRequired = map[string]bool{"echo": true} G.mkContext = newMkContext() - line := NewLine("fname", "3", "dummy", nil) + line := NewLine("fname", "3", "# dummy", nil) checklineMkShellcmd(line, "echo \"hello, world\"") @@ -117,26 +117,26 @@ func (s *Suite) TestShelltextContext_CheckCommandStart(c *check.C) { func (s *Suite) TestMkShellLine_checklineMkShelltext(c *check.C) { - shline := NewMkShellLine(s.DummyLine()) + shline := NewMkShellLine(NewLine("Makefile", "3", "# dummy", nil)) shline.checklineMkShelltext("for f in *.pl; do ${SED} s,@PREFIX@,${PREFIX}, < $f > $f.tmp && ${MV} $f.tmp $f; done") - c.Check(s.Output(), equals, "NOTE: fname:1: Please use the SUBST framework instead of ${SED} and ${MV}.\n") + c.Check(s.Output(), equals, "NOTE: Makefile:3: Please use the SUBST framework instead of ${SED} and ${MV}.\n") shline.checklineMkShelltext("install -c manpage.1 ${PREFIX}/man/man1/manpage.1") - c.Check(s.Output(), equals, "WARN: fname:1: Please use ${PKGMANDIR} instead of \"man\".\n") + c.Check(s.Output(), equals, "WARN: Makefile:3: Please use ${PKGMANDIR} instead of \"man\".\n") shline.checklineMkShelltext("cp init-script ${PREFIX}/etc/rc.d/service") - c.Check(s.Output(), equals, "WARN: fname:1: Please use the RCD_SCRIPTS mechanism to install rc.d scripts automatically to ${RCD_SCRIPTS_EXAMPLEDIR}.\n") + c.Check(s.Output(), equals, "WARN: Makefile:3: Please use the RCD_SCRIPTS mechanism to install rc.d scripts automatically to ${RCD_SCRIPTS_EXAMPLEDIR}.\n") } func (s *Suite) TestMkShellLine_checkCommandUse(c *check.C) { G.mkContext = newMkContext() G.mkContext.target = "do-install" - shline := NewMkShellLine(s.DummyLine()) + shline := NewMkShellLine(NewLine("fname", "1", "dummy", nil)) shline.checkCommandUse("sed") diff --git a/pkgtools/pkglint/files/vars_test.go b/pkgtools/pkglint/files/vars_test.go index ca1277e3319..19c5b1c5910 100644 --- a/pkgtools/pkglint/files/vars_test.go +++ b/pkgtools/pkglint/files/vars_test.go @@ -5,7 +5,7 @@ import ( ) func (s *Suite) TestVariableNeedsQuoting(c *check.C) { - line := s.DummyLine() + line := NewLine("fname", "1", "dummy", nil) G.globalData.InitVartypes() pkgnameType := G.globalData.vartypes["PKGNAME"] diff --git a/pkgtools/pkglint/files/vartypecheck.go b/pkgtools/pkglint/files/vartypecheck.go index dace8974ef4..88b16d1111a 100644 --- a/pkgtools/pkglint/files/vartypecheck.go +++ b/pkgtools/pkglint/files/vartypecheck.go @@ -213,7 +213,7 @@ func (cv *VartypeCheck) EmulPlatform() { } func (cv *VartypeCheck) FetchURL() { - checklineMkVartypePrimitive(cv.line, cv.varname, CheckvarURL, cv.op, cv.value, cv.comment, cv.listContext, cv.guessed) + NewMkLine(cv.line).checkVartypePrimitive(cv.varname, CheckvarURL, cv.op, cv.value, cv.comment, cv.listContext, cv.guessed) for siteUrl, siteName := range G.globalData.masterSiteUrls { if hasPrefix(cv.value, siteUrl) { @@ -362,7 +362,7 @@ func (cv *VartypeCheck) Option() { // The PATH environment variable func (cv *VartypeCheck) Pathlist() { if !contains(cv.value, ":") && cv.guessed == guGuessed { - checklineMkVartypePrimitive(cv.line, cv.varname, CheckvarPathname, cv.op, cv.value, cv.comment, cv.listContext, cv.guessed) + NewMkLine(cv.line).checkVartypePrimitive(cv.varname, CheckvarPathname, cv.op, cv.value, cv.comment, cv.listContext, cv.guessed) return } @@ -387,7 +387,7 @@ func (cv *VartypeCheck) Pathmask() { if !matches(cv.valueNovar, `^[#\-0-9A-Za-z._~+%*?/\[\]]*`) { cv.line.warnf("%q is not a valid pathname mask.", cv.value) } - checklineMkAbsolutePathname(cv.line, cv.value) + NewMkLine(cv.line).checkAbsolutePathname(cv.value) } // Like Filename, but including slashes @@ -396,7 +396,7 @@ func (cv *VartypeCheck) Pathname() { if !matches(cv.valueNovar, `^[#\-0-9A-Za-z._~+%/]*$`) { cv.line.warnf("%q is not a valid pathname.", cv.value) } - checklineMkAbsolutePathname(cv.line, cv.value) + NewMkLine(cv.line).checkAbsolutePathname(cv.value) } func (cv *VartypeCheck) Perl5Packlist() { @@ -412,7 +412,7 @@ func (cv *VartypeCheck) PkgName() { } func (cv *VartypeCheck) PkgOptionsVar() { - checklineMkVartypePrimitive(cv.line, cv.varname, CheckvarVarname, cv.op, cv.value, cv.comment, false, cv.guessed) + NewMkLine(cv.line).checkVartypePrimitive(cv.varname, CheckvarVarname, cv.op, cv.value, cv.comment, false, cv.guessed) if matches(cv.value, `\$\{PKGBASE[:\}]`) { cv.line.errorf("PKGBASE must not be used in PKG_OPTIONS_VAR.") cv.line.explain( @@ -553,7 +553,7 @@ func (cv *VartypeCheck) SedCommands() { } checklineMkShellword(line, words[i-1], true) checklineMkShellword(line, words[i], true) - checklineMkVartypePrimitive(line, cv.varname, CheckvarSedCommand, cv.op, words[i], cv.comment, cv.listContext, cv.guessed) + NewMkLine(line).checkVartypePrimitive(cv.varname, CheckvarSedCommand, cv.op, words[i], cv.comment, cv.listContext, cv.guessed) } else { line.errorf("The -e option to sed requires an argument.") } @@ -698,7 +698,7 @@ func (cv *VartypeCheck) WrapperTransform() { } func (cv *VartypeCheck) WrkdirSubdirectory() { - checklineMkVartypePrimitive(cv.line, cv.varname, CheckvarPathname, cv.op, cv.value, cv.comment, cv.listContext, cv.guessed) + NewMkLine(cv.line).checkVartypePrimitive(cv.varname, CheckvarPathname, cv.op, cv.value, cv.comment, cv.listContext, cv.guessed) } // A directory relative to ${WRKSRC}, for use in CONFIGURE_DIRS and similar variables. diff --git a/pkgtools/pkglint/files/vartypecheck_test.go b/pkgtools/pkglint/files/vartypecheck_test.go index e640ac84de7..c0023ff6606 100644 --- a/pkgtools/pkglint/files/vartypecheck_test.go +++ b/pkgtools/pkglint/files/vartypecheck_test.go @@ -262,7 +262,7 @@ func (s *Suite) TestVartypeCheck_Yes(c *check.C) { } func newVartypeCheck(varname, op, value string) *VartypeCheck { - line := NewLine("fname", "1", "dummy", nil) - valueNovar := withoutMakeVariables(line, value, true) + line := NewLine("fname", "1", varname+op+value, nil) + valueNovar := NewMkLine(line).withoutMakeVariables(value, true) return &VartypeCheck{line, varname, op, value, valueNovar, "", true, guNotGuessed} } diff --git a/pkgtools/pkglint/files/varusecontext_test.go b/pkgtools/pkglint/files/varusecontext_test.go index 2c8d3f1c524..931bdd45455 100644 --- a/pkgtools/pkglint/files/varusecontext_test.go +++ b/pkgtools/pkglint/files/varusecontext_test.go @@ -6,7 +6,7 @@ import ( func (s *Suite) TestVarUseContext_ToString(c *check.C) { G.globalData.InitVartypes() - vartype := getVariableType(s.DummyLine(), "PKGNAME") + vartype := getVariableType(NewLine("fname", "1", "dummy", nil), "PKGNAME") vuc := &VarUseContext{vucTimeUnknown, vartype, vucQuotBackt, vucExtentWord} c.Check(vuc.String(), equals, "(unknown PkgName backt word)") |