pkgsrc-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[pkgsrc/trunk]: pkgsrc/pkgtools/pkglint/files pkgtools/pkglint: update to 5.7.22
details: https://anonhg.NetBSD.org/pkgsrc/rev/88d0d1960566
branches: trunk
changeset: 400252:88d0d1960566
user: rillig <rillig%pkgsrc.org@localhost>
date: Sun Aug 25 21:44:37 2019 +0000
description:
pkgtools/pkglint: update to 5.7.22
Changes since 5.7.21:
* The files from wip/mk do not belong to the main pkgsrc infrastructure.
Therefore, when loading the package Makefile to look for defined but
unused variables, parsing doesn't stop in these files.
diffstat:
pkgtools/pkglint/files/check_test.go | 4 +-
pkgtools/pkglint/files/mkline.go | 15 +-
pkgtools/pkglint/files/mkline_test.go | 9 +-
pkgtools/pkglint/files/mklines.go | 9 +-
pkgtools/pkglint/files/package.go | 101 +++++++----
pkgtools/pkglint/files/package_test.go | 232 +++++++++++++++++++++++++-
pkgtools/pkglint/files/pkglint.go | 3 -
pkgtools/pkglint/files/varalignblock.go | 2 +-
pkgtools/pkglint/files/varalignblock_test.go | 4 +-
9 files changed, 314 insertions(+), 65 deletions(-)
diffs (truncated from 680 to 300 lines):
diff -r 5ce91e51db0a -r 88d0d1960566 pkgtools/pkglint/files/check_test.go
--- a/pkgtools/pkglint/files/check_test.go Sun Aug 25 20:30:11 2019 +0000
+++ b/pkgtools/pkglint/files/check_test.go Sun Aug 25 21:44:37 2019 +0000
@@ -233,8 +233,7 @@
lines = append(lines, mkline.Line)
if mkline.IsInclude() {
- included := cleanpath(path.Dir(filename) + "/" + mkline.IncludedFile())
- load(included)
+ load(mkline.IncludedFileFull())
}
}
}
@@ -681,6 +680,7 @@
}
// Main runs the pkglint main program with the given command line arguments.
+// Other than in the other tests, the -Wall option is not added implicitly.
//
// Arguments that name existing files or directories in the temporary test
// directory are transformed to their actual paths.
diff -r 5ce91e51db0a -r 88d0d1960566 pkgtools/pkglint/files/mkline.go
--- a/pkgtools/pkglint/files/mkline.go Sun Aug 25 20:30:11 2019 +0000
+++ b/pkgtools/pkglint/files/mkline.go Sun Aug 25 21:44:37 2019 +0000
@@ -421,12 +421,17 @@
func (mkline *MkLine) IncludedFile() string { return mkline.data.(*mkLineInclude).includedFile }
+func (mkline *MkLine) IncludedFileFull() string {
+ return cleanpath(path.Join(path.Dir(mkline.Filename), mkline.IncludedFile()))
+}
+
func (mkline *MkLine) Targets() string { return mkline.data.(mkLineDependency).targets }
func (mkline *MkLine) Sources() string { return mkline.data.(mkLineDependency).sources }
-// ConditionalVars applies to .include lines and is a space-separated
-// list of those variable names on which the inclusion depends.
+// ConditionalVars applies to .include lines and contains the
+// variable names on which the inclusion depends.
+//
// It is initialized later, step by step, when parsing other lines.
func (mkline *MkLine) ConditionalVars() []string {
return mkline.data.(*mkLineInclude).conditionalVars
@@ -1367,16 +1372,16 @@
//
// Variables named *_MK are excluded since they are usually not interesting.
func (ind *Indentation) Varnames() []string {
- var varnames []string
+ varnames := NewStringSet()
for _, level := range ind.levels {
for _, levelVarname := range level.conditionalVars {
// multiple-inclusion guard must be filtered out earlier.
assert(!hasSuffix(levelVarname, "_MK"))
- varnames = append(varnames, levelVarname)
+ varnames.Add(levelVarname)
}
}
- return varnames
+ return varnames.Elements
}
// Args returns the arguments of the innermost .if, .elif or .for.
diff -r 5ce91e51db0a -r 88d0d1960566 pkgtools/pkglint/files/mkline_test.go
--- a/pkgtools/pkglint/files/mkline_test.go Sun Aug 25 20:30:11 2019 +0000
+++ b/pkgtools/pkglint/files/mkline_test.go Sun Aug 25 21:44:37 2019 +0000
@@ -1931,12 +1931,11 @@
G.Check(t.File("category/package"))
- // TODO: It feels wrong that OPSYS is mentioned twice here.
- // Why only twice and not three times?
t.CheckOutputLines(
- "WARN: ~/category/package/buildlink3.mk:14: " +
- "\"../../category/other/buildlink3.mk\" is included conditionally here " +
- "(depending on OPSYS, OPSYS) and unconditionally in Makefile:20.")
+ "WARN: ~/category/package/Makefile:20: " +
+ "\"../../category/other/buildlink3.mk\" is included " +
+ "unconditionally here and " +
+ "conditionally in buildlink3.mk:14 (depending on OPSYS).")
}
func (s *Suite) Test_MkLine_ForEachUsed(c *check.C) {
diff -r 5ce91e51db0a -r 88d0d1960566 pkgtools/pkglint/files/mklines.go
--- a/pkgtools/pkglint/files/mklines.go Sun Aug 25 20:30:11 2019 +0000
+++ b/pkgtools/pkglint/files/mklines.go Sun Aug 25 21:44:37 2019 +0000
@@ -90,6 +90,9 @@
mklines.collectVariables()
mklines.collectPlistVars()
mklines.collectElse()
+ if G.Pkg != nil {
+ G.Pkg.collectConditionalIncludes(mklines)
+ }
// In the second pass, the actual checks are done.
mklines.checkAll()
@@ -237,7 +240,7 @@
// ForEachEnd calls the action for each line, until the action returns false.
// It keeps track of the indentation and all conditional variables.
// At the end, atEnd is called with the last line as its argument.
-func (mklines *MkLines) ForEachEnd(action func(mkline *MkLine) bool, atEnd func(lastMkline *MkLine)) {
+func (mklines *MkLines) ForEachEnd(action func(mkline *MkLine) bool, atEnd func(lastMkline *MkLine)) bool {
// XXX: To avoid looping over the lines multiple times, it would
// be nice to have an interface LinesChecker that checks a single topic.
@@ -247,9 +250,11 @@
mklines.indentation = NewIndentation()
mklines.Tools.SeenPrefs = false
+ result := true
for _, mkline := range mklines.mklines {
mklines.indentation.TrackBefore(mkline)
if !action(mkline) {
+ result = false
break
}
mklines.indentation.TrackAfter(mkline)
@@ -259,6 +264,8 @@
atEnd(mklines.mklines[len(mklines.mklines)-1])
}
mklines.indentation = nil
+
+ return result
}
// ExpandLoopVar searches the surrounding .for loops for the given
diff -r 5ce91e51db0a -r 88d0d1960566 pkgtools/pkglint/files/package.go
--- a/pkgtools/pkglint/files/package.go Sun Aug 25 20:30:11 2019 +0000
+++ b/pkgtools/pkglint/files/package.go Sun Aug 25 21:44:37 2019 +0000
@@ -23,7 +23,7 @@
Filesdir string // FILESDIR from the package Makefile
Patchdir string // PATCHDIR from the package Makefile
DistinfoFile string // DISTINFO_FILE from the package Makefile
- EffectivePkgname string // PKGNAME or DISTNAME from the package Makefile, including nb13
+ EffectivePkgname string // PKGNAME or DISTNAME from the package Makefile, including nb13, can be empty
EffectivePkgbase string // EffectivePkgname without the version
EffectivePkgversion string // The version part of the effective PKGNAME, excluding nb13
EffectivePkgnameLine *MkLine // The origin of the three Effective* values
@@ -56,6 +56,8 @@
unconditionalIncludes map[string]*MkLine
IgnoreMissingPatches bool // In distinfo, don't warn about patches that cannot be found.
+
+ Once Once
}
func NewPackage(dir string) *Package {
@@ -112,6 +114,13 @@
return relpath(pkg.dir, filename)
}
+// Returns whether the given file (relative to the package directory)
+// is included somewhere in the package, either directly or indirectly.
+func (pkg *Package) Includes(filename string) bool {
+ return pkg.unconditionalIncludes[filename] != nil ||
+ pkg.conditionalIncludes[filename] != nil
+}
+
func (pkg *Package) checkPossibleDowngrade() {
if trace.Tracing {
defer trace.Call0()()
@@ -160,9 +169,13 @@
}
}
-// checkLinesBuildlink3Inclusion checks whether the package Makefile and
-// the corresponding buildlink3.mk agree for all included buildlink3.mk
-// files whether they are included conditionally or unconditionally.
+// checkLinesBuildlink3Inclusion checks whether the package Makefile includes
+// at least those buildlink3.mk files that are included by the buildlink3.mk
+// file of the package.
+//
+// The other direction is not checked since it is perfectly fine for a package
+// to have more dependencies than are needed for buildlink the package.
+// (This might be worth re-checking though.)
func (pkg *Package) checkLinesBuildlink3Inclusion(mklines *MkLines) {
if trace.Tracing {
defer trace.Call0()()
@@ -173,7 +186,7 @@
for _, mkline := range mklines.mklines {
if mkline.IsInclude() {
includedFile := mkline.IncludedFile()
- if matches(includedFile, `^\.\./\.\./.*/buildlink3\.mk`) {
+ if hasSuffix(includedFile, "/buildlink3.mk") {
includedFiles[includedFile] = mkline
if pkg.bl3[includedFile] == nil {
mkline.Warnf("%s is included by this file but not by the package.", includedFile)
@@ -210,6 +223,7 @@
// Determine the used variables and PLIST directories before checking any of the Makefile fragments.
// TODO: Why is this code necessary? What effect does it have?
+ pkg.collectConditionalIncludes(mklines)
for _, filename := range files {
basename := path.Base(filename)
if (hasPrefix(basename, "Makefile.") || hasSuffix(filename, ".mk")) &&
@@ -218,6 +232,7 @@
!contains(filename, pkg.Filesdir+"/") {
fragmentMklines := LoadMk(filename, MustSucceed)
fragmentMklines.collectUsedVariables()
+ pkg.collectConditionalIncludes(fragmentMklines)
}
if hasPrefix(basename, "PLIST") {
pkg.loadPlistDirs(filename)
@@ -382,21 +397,30 @@
return mainLines, allLines
}
+func (pkg *Package) collectConditionalIncludes(mklines *MkLines) {
+ mklines.ForEach(func(mkline *MkLine) {
+ if mkline.IsInclude() {
+ mkline.SetConditionalVars(mklines.indentation.Varnames())
+
+ key := pkg.Rel(mkline.IncludedFileFull())
+ if mklines.indentation.IsConditional() {
+ pkg.conditionalIncludes[key] = mkline
+ } else {
+ pkg.unconditionalIncludes[key] = mkline
+ }
+ }
+ })
+}
+
// TODO: What is allLines used for, is it still necessary? Would it be better as a field in Package?
func (pkg *Package) parse(mklines *MkLines, allLines *MkLines, includingFileForUsedCheck string) bool {
if trace.Tracing {
defer trace.Call1(mklines.lines.Filename)()
}
- result := true
-
- lineAction := func(mkline *MkLine) bool {
- result = pkg.parseLine(mklines, mkline, allLines)
- return result
- }
-
- atEnd := func(mkline *MkLine) {}
- mklines.ForEachEnd(lineAction, atEnd)
+ result := mklines.ForEachEnd(
+ func(mkline *MkLine) bool { return pkg.parseLine(mklines, mkline, allLines) },
+ func(mkline *MkLine) {})
if includingFileForUsedCheck != "" {
mklines.CheckUsedBy(G.Pkgsrc.ToRel(includingFileForUsedCheck))
@@ -540,15 +564,11 @@
return false
}
- if !contains(includingFile, "/mk/") {
- return true
+ if contains(includingFile, "/mk/") && !hasPrefix(G.Pkgsrc.ToRel(includingFile), "wip/mk") {
+ return hasSuffix(includingFile, "buildlink3.mk") && hasSuffix(includedFile, "builtin.mk")
}
- if hasSuffix(includingFile, "buildlink3.mk") && hasSuffix(includedFile, "builtin.mk") {
- return true
- }
-
- return false
+ return true
}
func (pkg *Package) collectSeenInclude(mkline *MkLine, includedFile string) {
@@ -588,7 +608,7 @@
}
if mkline.Basename != "buildlink3.mk" {
- if matches(includedFile, `^\.\./\.\./(.*)/buildlink3\.mk$`) {
+ if hasSuffix(includedFile, "/buildlink3.mk") {
pkg.bl3[includedFile] = mkline
if trace.Tracing {
trace.Step1("Buildlink3 file in package: %q", includedFile)
@@ -666,10 +686,19 @@
pkg.checkUpdate()
+ // TODO: Maybe later collect the conditional includes from allLines
+ // instead of mklines. This will lead to about 6000 new warnings
+ // though.
+ // pkg.collectConditionalIncludes(allLines)
+
allLines.collectVariables() // To get the tool definitions
mklines.Tools = allLines.Tools // TODO: also copy the other collected data
mklines.Check()
+ if false && pkg.EffectivePkgname != "" && pkg.Includes("../../lang/python/extension.mk") {
+ pkg.EffectivePkgnameLine.Warnf("The PKGNAME of Python extensions should start with ${PYPKGPREFIX}.")
+ }
+
pkg.CheckVarorder(mklines)
SaveAutofixChanges(mklines.lines)
@@ -1232,27 +1261,34 @@
}
func (pkg *Package) checkIncludeConditionally(mkline *MkLine, indentation *Indentation) {
- mkline.SetConditionalVars(indentation.Varnames())
+ if IsPrefs(mkline.IncludedFile()) {
+ return
+ }
Home |
Main Index |
Thread Index |
Old Index