diff options
author | rillig <rillig@pkgsrc.org> | 2020-06-01 20:49:54 +0000 |
---|---|---|
committer | rillig <rillig@pkgsrc.org> | 2020-06-01 20:49:54 +0000 |
commit | 05d76c3982a4aaf6ced2f7744f7043f9f4e6e4ac (patch) | |
tree | 119ec105dd83c8cf4e64fa8b9a0cb9858163627a | |
parent | 67f2e52979de3b4cd3050e4b462bddf771386c34 (diff) | |
download | pkgsrc-05d76c3982a4aaf6ced2f7744f7043f9f4e6e4ac.tar.gz |
pkgtools/pkglint: update to 20.1.12
Changes since 20.1.11:
The file bsd.pkg.mk must only ever be included by package Makefiles
directly, not by other Makefile fragments. Seen in www/w3m.
The variable BUILDLINK_PREFIX.* should only be used for packages that
have actually been included by the package. This catches the use of
BUILDLINK_PREFIX.libiconv, which should have been iconv instead.
Allow comments before line 3 in buildlink3.mk files. This is necessary
for mariadb55-client since its buildlink identifier is mysql-client,
which is so non-obvious that it needs to be documented.
26 files changed, 256 insertions, 107 deletions
diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile index d744a5ce368..c733d057e08 100644 --- a/pkgtools/pkglint/Makefile +++ b/pkgtools/pkglint/Makefile @@ -1,6 +1,6 @@ -# $NetBSD: Makefile,v 1.650 2020/05/29 20:13:17 rillig Exp $ +# $NetBSD: Makefile,v 1.651 2020/06/01 20:49:54 rillig Exp $ -PKGNAME= pkglint-20.1.11 +PKGNAME= pkglint-20.1.12 CATEGORIES= pkgtools DISTNAME= tools MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/} diff --git a/pkgtools/pkglint/files/buildlink3.go b/pkgtools/pkglint/files/buildlink3.go index 761107bc27e..46e424ec013 100644 --- a/pkgtools/pkglint/files/buildlink3.go +++ b/pkgtools/pkglint/files/buildlink3.go @@ -72,6 +72,9 @@ func (ck *Buildlink3Checker) Check() { func (ck *Buildlink3Checker) checkFirstParagraph(mlex *MkLinesLexer) bool { + for mlex.SkipPrefix("#") { + } + // First paragraph: Introduction of the package identifier m := mlex.NextRegexp(`^BUILDLINK_TREE\+=[\t ]*([^\t ]+)$`) if m == nil { @@ -104,7 +107,7 @@ func (ck *Buildlink3Checker) checkUniquePkgbase(pkgbase string, mkline *MkLine) } dirname := G.Pkgsrc.Rel(mkline.Filename().Dir()).Base() - base, name := trimCommon(pkgbase, dirname) + base, name := trimCommon(pkgbase, dirname.String()) if base == "" && matches(name, `^(\d*|-cvs|-fossil|-git|-hg|-svn|-devel|-snapshot)$`) { return } @@ -315,6 +318,7 @@ func (ck *Buildlink3Checker) checkVaruseInPkgbase(pkgbaseLine *MkLine) { type Buildlink3Data struct { id Buildlink3ID + prefix Path pkgsrcdir PackagePath apiDepends *DependencyPattern apiDependsLine *MkLine @@ -331,43 +335,50 @@ type Buildlink3ID string func LoadBuildlink3Data(mklines *MkLines) *Buildlink3Data { var data Buildlink3Data + mklines.ForEach(func(mkline *MkLine) { - if mkline.IsVarassign() { - varname := mkline.Varname() - varbase := varnameBase(varname) - varid := Buildlink3ID(varnameParam(varname)) - - if varname == "BUILDLINK_TREE" { - value := mkline.Value() - if !hasPrefix(value, "-") { - data.id = Buildlink3ID(mkline.Value()) - } - } + if !mkline.IsVarassign() { + return + } - if varbase == "BUILDLINK_API_DEPENDS" && varid == data.id { - p := NewMkParser(nil, mkline.Value()) - dep := p.DependencyPattern() - if dep != nil && p.EOF() { - data.apiDepends = dep - data.apiDependsLine = mkline - } + varname := mkline.Varname() + varbase := varnameBase(varname) + varid := Buildlink3ID(varnameParam(varname)) + + if varname == "BUILDLINK_TREE" { + value := mkline.Value() + if !hasPrefix(value, "-") { + data.id = Buildlink3ID(mkline.Value()) } + } - if varbase == "BUILDLINK_ABI_DEPENDS" && varid == data.id { - p := NewMkParser(nil, mkline.Value()) - dep := p.DependencyPattern() - if dep != nil && p.EOF() { - data.abiDepends = dep - data.abiDependsLine = mkline - } + if varbase == "BUILDLINK_API_DEPENDS" && varid == data.id { + p := NewMkParser(nil, mkline.Value()) + dep := p.DependencyPattern() + if dep != nil && p.EOF() { + data.apiDepends = dep + data.apiDependsLine = mkline } + } - if varbase == "BUILDLINK_PKGSRCDIR" && varid == data.id { - data.pkgsrcdir = NewPackagePathString(mkline.Value()) + if varbase == "BUILDLINK_ABI_DEPENDS" && varid == data.id { + p := NewMkParser(nil, mkline.Value()) + dep := p.DependencyPattern() + if dep != nil && p.EOF() { + data.abiDepends = dep + data.abiDependsLine = mkline } } + + if varbase == "BUILDLINK_PREFIX" && varid == data.id { + data.prefix = NewPath(mkline.Value()) + } + if varbase == "BUILDLINK_PKGSRCDIR" && varid == data.id { + data.pkgsrcdir = NewPackagePathString(mkline.Value()) + } }) - if data.id != "" && !data.pkgsrcdir.IsEmpty() && data.apiDepends != nil && data.abiDepends != nil { + + if data.id != "" { return &data } return nil diff --git a/pkgtools/pkglint/files/buildlink3_test.go b/pkgtools/pkglint/files/buildlink3_test.go index aaf306236e1..0e071e84db5 100644 --- a/pkgtools/pkglint/files/buildlink3_test.go +++ b/pkgtools/pkglint/files/buildlink3_test.go @@ -506,6 +506,35 @@ func (s *Suite) Test_Buildlink3Checker_Check__no_tracing(c *check.C) { t.CheckOutputEmpty() } +func (s *Suite) Test_Buildlink3Checker_checkFirstParagraph__comment_before_tree(c *check.C) { + t := s.Init(c) + + t.SetUpPkgsrc() + t.SetUpPackage("category/package") + t.CreateFileLines("category/package/buildlink3.mk", + MkCvsID, + "", + "# comment", + "BUILDLINK_TREE+=\tpackage", + "", + ".if !defined(PACKAGE_BUILDLINK3_MK)", + "PACKAGE_BUILDLINK3_MK:=", + "", + "BUILDLINK_API_DEPENDS.package+=\tpackage>=0", + "BUILDLINK_PKGSRCDIR.package?=\t../../category/package", + "BUILDLINK_DEPMETHOD.package?=\tbuild", + "", + ".endif # PACKAGE_BUILDLINK3_MK", + "", + "BUILDLINK_TREE+=\t-package") + t.FinishSetUp() + + G.Check(t.File("category/package/buildlink3.mk")) + + // No warning in line 3. Comments are ok there. + t.CheckOutputEmpty() +} + func (s *Suite) Test_Buildlink3Checker_checkUniquePkgbase(c *check.C) { t := s.Init(c) diff --git a/pkgtools/pkglint/files/check_test.go b/pkgtools/pkglint/files/check_test.go index fad25f4c7eb..5537fdab85b 100644 --- a/pkgtools/pkglint/files/check_test.go +++ b/pkgtools/pkglint/files/check_test.go @@ -498,7 +498,7 @@ func (t *Tester) SetUpPackage(pkgpath RelPath, makefileLines ...string) CurrPath mlines := []string{ MkCvsID, "", - "DISTNAME=\t" + distname + "-1.0", + "DISTNAME=\t" + distname.String() + "-1.0", "#PKGNAME=\tpackage-1.0", "CATEGORIES=\t" + category.String(), "MASTER_SITES=\t# none", @@ -589,7 +589,7 @@ func (t *Tester) CreateFileDummyPatch(filename RelPath) { func (t *Tester) CreateFileBuildlink3(filename RelPath, customLines ...string) { lower := filename.Dir().Base() - t.CreateFileBuildlink3Id(filename, lower, customLines...) + t.CreateFileBuildlink3Id(filename, lower.String(), customLines...) } func (t *Tester) CreateFileBuildlink3Id(filename RelPath, id string, customLines ...string) { @@ -739,7 +739,7 @@ func (t *Tester) SetUpHierarchy() ( fromDir := including.Dir().Clean() to := basedir.Rel(included.AsPath()) if fromDir == to.Dir() { - return NewRelPathString(to.Base()) + return to.Base() } else { return fromDir.Rel(basedir).JoinNoClean(to).CleanDot() } @@ -1001,9 +1001,9 @@ func (t *Tester) NewLine(filename CurrPath, lineno int, text string) *Line { func (t *Tester) NewMkLine(filename CurrPath, lineno int, text string) *MkLine { basename := filename.Base() assertf( - hasSuffix(basename, ".mk") || + basename.HasSuffixText(".mk") || basename == "Makefile" || - hasPrefix(basename, "Makefile.") || + basename.HasPrefixText("Makefile.") || basename == "mk.conf", "filename %q must be realistic, otherwise the variable permissions are wrong", filename) @@ -1053,7 +1053,7 @@ func (t *Tester) NewMkLines(filename CurrPath, lines ...string) *MkLines { func (t *Tester) NewMkLinesPkg(filename CurrPath, pkg *Package, lines ...string) *MkLines { basename := filename.Base() assertf( - hasSuffix(basename, ".mk") || basename == "Makefile" || hasPrefix(basename, "Makefile."), + basename.HasSuffixText(".mk") || basename == "Makefile" || basename.HasPrefixText("Makefile."), "filename %q must be realistic, otherwise the variable permissions are wrong", filename) var rawText strings.Builder diff --git a/pkgtools/pkglint/files/distinfo.go b/pkgtools/pkglint/files/distinfo.go index 875c9dadc42..09961ba22d5 100644 --- a/pkgtools/pkglint/files/distinfo.go +++ b/pkgtools/pkglint/files/distinfo.go @@ -35,7 +35,7 @@ func CheckLinesDistinfo(pkg *Package, lines *Lines) { ck.checkUnrecordedPatches() if pkg != nil { - pkg.distinfoDistfiles = make(map[string]bool) + pkg.distinfoDistfiles = make(map[RelPath]bool) for path := range ck.infos { pkg.distinfoDistfiles[path.Base()] = true } diff --git a/pkgtools/pkglint/files/line.go b/pkgtools/pkglint/files/line.go index aaf1334d179..5bf145b9c96 100644 --- a/pkgtools/pkglint/files/line.go +++ b/pkgtools/pkglint/files/line.go @@ -56,7 +56,7 @@ type Line struct { // TODO: Consider storing pointers to the Filename and Basename instead of strings to save memory. // But first find out where and why pkglint needs so much memory (200 MB for a full recursive run over pkgsrc + wip). Location Location - Basename string // the last component of the Filename + Basename RelPath // the last component of the Filename // the text of the line, without the trailing newline character; // in Makefiles, also contains the text from the continuation lines, diff --git a/pkgtools/pkglint/files/lines.go b/pkgtools/pkglint/files/lines.go index 1a86012d5a3..a1487b559bd 100644 --- a/pkgtools/pkglint/files/lines.go +++ b/pkgtools/pkglint/files/lines.go @@ -6,7 +6,7 @@ import ( type Lines struct { Filename CurrPath - BaseName string // TODO: consider converting to Path + BaseName RelPath Lines []*Line } diff --git a/pkgtools/pkglint/files/mkassignchecker.go b/pkgtools/pkglint/files/mkassignchecker.go index 547fdcd087e..417c1b295c2 100644 --- a/pkgtools/pkglint/files/mkassignchecker.go +++ b/pkgtools/pkglint/files/mkassignchecker.go @@ -387,7 +387,7 @@ func (ck *MkAssignChecker) checkVarassignRightCategory() { primary := categories[0] dir := G.Pkgsrc.Rel(mkline.Filename()).Dir().Dir().Base() - if primary == dir || dir == "wip" || dir == "regress" { + if primary == dir.String() || dir == "wip" || dir == "regress" { return } @@ -397,7 +397,7 @@ func (ck *MkAssignChecker) checkVarassignRightCategory() { "The primary category of a package should be its location in the", "pkgsrc directory tree, to make it easy to find the package.", "All other categories may be added after this primary category.") - if len(categories) > 1 && categories[1] == dir { + if len(categories) > 1 && categories[1] == dir.String() { fix.Replace(primary+" "+categories[1], categories[1]+" "+primary) } fix.Apply() diff --git a/pkgtools/pkglint/files/mkline.go b/pkgtools/pkglint/files/mkline.go index 57ff4a23e16..abbf253defe 100644 --- a/pkgtools/pkglint/files/mkline.go +++ b/pkgtools/pkglint/files/mkline.go @@ -270,6 +270,10 @@ func (mkline *MkLine) FirstLineContainsValue() bool { func (mkline *MkLine) ShellCommand() string { return mkline.data.(mkLineShell).command } +// Indent returns the whitespace between the dot and the directive. +// +// For the following example line it returns two spaces: +// . include "other.mk" func (mkline *MkLine) Indent() string { if mkline.IsDirective() { return mkline.data.(*mkLineDirective).indent diff --git a/pkgtools/pkglint/files/mklinechecker.go b/pkgtools/pkglint/files/mklinechecker.go index 1aac66a1b04..5c2fbd24699 100644 --- a/pkgtools/pkglint/files/mklinechecker.go +++ b/pkgtools/pkglint/files/mklinechecker.go @@ -245,7 +245,6 @@ func (ck MkLineChecker) checkInclude() { if trace.Tracing { trace.Stepf("includingFile=%s includedFile=%s", mkline.Filename(), includedFile) } - // TODO: Not every path is relative to the package directory. ck.CheckRelativePath(NewPackagePath(includedFile), includedFile, mustExist) switch { @@ -257,7 +256,10 @@ func (ck MkLineChecker) checkInclude() { "module.mk or similar.", "After that, both this one and the other package should include the newly created file.") - case mkline.Basename == "buildlink3.mk" && includedFile.Base() == "bsd.prefs.mk": + case mkline.Basename != "Makefile" && includedFile.HasBase("bsd.pkg.mk"): + mkline.Errorf("The file bsd.pkg.mk must only be included by package Makefiles, not by other Makefile fragments.") + + case mkline.Basename == "buildlink3.mk" && includedFile.HasBase("bsd.prefs.mk"): fix := mkline.Autofix() fix.Notef("For efficiency reasons, please include bsd.fast.prefs.mk instead of bsd.prefs.mk.") fix.Replace("bsd.prefs.mk", "bsd.fast.prefs.mk") diff --git a/pkgtools/pkglint/files/mklinechecker_test.go b/pkgtools/pkglint/files/mklinechecker_test.go index ea9311ed8c5..d5861b03f31 100644 --- a/pkgtools/pkglint/files/mklinechecker_test.go +++ b/pkgtools/pkglint/files/mklinechecker_test.go @@ -369,17 +369,22 @@ func (s *Suite) Test_MkLineChecker_checkInclude(c *check.C) { t.CreateFileLines("graphics/jpeg/buildlink3.mk") t.CreateFileLines("devel/intltool/buildlink3.mk") t.CreateFileLines("devel/intltool/builtin.mk") + t.CreateFileLines("mk/bsd.pkg.mk") mklines := t.SetUpFileMkLines("category/package/filename.mk", MkCvsID, "", ".include \"../../pkgtools/x11-links/buildlink3.mk\"", ".include \"../../graphics/jpeg/buildlink3.mk\"", ".include \"../../devel/intltool/buildlink3.mk\"", - ".include \"../../devel/intltool/builtin.mk\"") + ".include \"../../devel/intltool/builtin.mk\"", + ".include \"/absolute\"", + ".include \"../../mk/bsd.pkg.mk\"") mklines.Check() t.CheckOutputLines( + "ERROR: ~/category/package/filename.mk:7: "+ + "Unknown Makefile line format: \".include \\\"/absolute\\\"\".", "ERROR: ~/category/package/filename.mk:3: "+ "\"../../pkgtools/x11-links/buildlink3.mk\" must not be included directly. "+ "Include \"../../mk/x11.buildlink3.mk\" instead.", @@ -390,7 +395,10 @@ func (s *Suite) Test_MkLineChecker_checkInclude(c *check.C) { "Please write \"USE_TOOLS+= intltool\" instead of this line.", "ERROR: ~/category/package/filename.mk:6: "+ "\"../../devel/intltool/builtin.mk\" must not be included directly. "+ - "Include \"../../devel/intltool/buildlink3.mk\" instead.") + "Include \"../../devel/intltool/buildlink3.mk\" instead.", + "ERROR: ~/category/package/filename.mk:8: "+ + "The file bsd.pkg.mk must only be included by package Makefiles, "+ + "not by other Makefile fragments.") } func (s *Suite) Test_MkLineChecker_checkInclude__Makefile(c *check.C) { diff --git a/pkgtools/pkglint/files/mkvarusechecker.go b/pkgtools/pkglint/files/mkvarusechecker.go index d76376b53c6..0aeeb3df624 100644 --- a/pkgtools/pkglint/files/mkvarusechecker.go +++ b/pkgtools/pkglint/files/mkvarusechecker.go @@ -148,6 +148,48 @@ func (ck *MkVarUseChecker) checkVarname(time VucTime) { fix.ReplaceAfter("${", "LOCALBASE", "PREFIX") fix.Apply() } + + ck.checkVarnameBuildlink(varname) +} + +func (ck *MkVarUseChecker) checkVarnameBuildlink(varname string) { + pkg := ck.MkLines.pkg + if pkg == nil { + return + } + + if !hasPrefix(varname, "BUILDLINK_PREFIX.") { + return + } + + basename := ck.MkLine.Basename + if basename == "buildlink3.mk" || basename == "builtin.mk" { + return + } + + varparam := varnameParam(varname) + id := Buildlink3ID(varparam) + if pkg.bl3Data[id] != nil || containsVarUse(varparam) { + return + } + + // Several packages contain Makefile fragments that are more related + // to the buildlink3.mk file than to the package Makefile. + // These may use the buildlink identifier from the package itself. + bl3 := LoadMk(pkg.File("buildlink3.mk"), pkg, 0) + if bl3 != nil { + bl3Data := LoadBuildlink3Data(bl3) + if bl3Data != nil && bl3Data.id == id { + return + } + } + + if id == "mysql-client" && pkg.Includes("../../mk/mysql.buildlink3.mk") != nil { + return + } + + ck.MkLine.Warnf("Buildlink identifier %q is not known in this package.", + varparam) } // checkPermissions checks the permissions when a variable is used, diff --git a/pkgtools/pkglint/files/mkvarusechecker_test.go b/pkgtools/pkglint/files/mkvarusechecker_test.go index ac063a982d9..ddf68c1723c 100644 --- a/pkgtools/pkglint/files/mkvarusechecker_test.go +++ b/pkgtools/pkglint/files/mkvarusechecker_test.go @@ -379,6 +379,25 @@ func (s *Suite) Test_MkVarUseChecker_checkVarname(c *check.C) { "AUTOFIX: filename.mk:3: Replacing \"LOCALBASE\" with \"PREFIX\".") } +func (s *Suite) Test_MkVarUseChecker_checkVarnameBuildlink(c *check.C) { + t := s.Init(c) + + t.SetUpPackage("category/library") + t.CreateFileBuildlink3Id("category/library/buildlink3.mk", "lib") + t.SetUpPackage("category/package", + "CONFIGURE_ARGS+=\t--with-library=${BUILDLINK_PREFIX.library}", + "CONFIGURE_ARGS+=\t--with-lib=${BUILDLINK_PREFIX.lib}", + "", + ".include \"../../category/library/buildlink3.mk\"") + t.Chdir("category/package") + t.FinishSetUp() + + G.Check(".") + + t.CheckOutputLines( + "WARN: Makefile:20: Buildlink identifier \"library\" is not known in this package.") +} + func (s *Suite) Test_MkVarUseChecker_checkPermissions(c *check.C) { t := s.Init(c) diff --git a/pkgtools/pkglint/files/package.go b/pkgtools/pkglint/files/package.go index b381a0d921e..6c364f74f29 100644 --- a/pkgtools/pkglint/files/package.go +++ b/pkgtools/pkglint/files/package.go @@ -90,7 +90,7 @@ type Package struct { // Contains the basenames of the distfiles that are mentioned in distinfo, // for example "package-1.0.tar.gz", even if that file is in a DIST_SUBDIR. - distinfoDistfiles map[string]bool + distinfoDistfiles map[RelPath]bool } func NewPackage(dir CurrPath) *Package { @@ -161,11 +161,11 @@ func (pkg *Package) load() ([]CurrPath, *MkLines, *MkLines) { files = append(files, pkg.File(pkg.DistinfoFile)) } - isRelevantMk := func(filename CurrPath, basename string) bool { - if !hasPrefix(basename, "Makefile.") && !filename.HasSuffixText(".mk") { + isRelevantMk := func(filename CurrPath, basename RelPath) bool { + if !hasPrefix(basename.String(), "Makefile.") && !filename.HasSuffixText(".mk") { return false } - if filename.Dir().Base() == "patches" { + if filename.Dir().HasBase("patches") { return false } if pkg.Pkgdir == "." { @@ -184,7 +184,7 @@ func (pkg *Package) load() ([]CurrPath, *MkLines, *MkLines) { pkg.collectConditionalIncludes(fragmentMklines) pkg.loadBuildlink3Pkgbase(filename, fragmentMklines) } - if hasPrefix(basename, "PLIST") { + if basename.HasPrefixText("PLIST") { pkg.loadPlistDirs(filename) } } @@ -1351,7 +1351,7 @@ func (pkg *Package) checkDirent(dirent CurrPath, mode os.FileMode) { case mode.IsRegular(): G.checkReg(dirent, basename, G.Pkgsrc.Rel(dirent).Count(), pkg) - case hasPrefix(basename, "work"): + case basename.HasPrefixText("work"): if G.Import { NewLineWhole(dirent).Errorf("Must be cleaned up before committing the package.") } @@ -1361,7 +1361,7 @@ func (pkg *Package) checkDirent(dirent CurrPath, mode os.FileMode) { switch { case basename == "files", basename == "patches", - dirent.Dir().Base() == "files", + dirent.Dir().HasBase("files"), isEmptyDir(dirent): break @@ -1445,10 +1445,10 @@ func (pkg *Package) checkFreeze(filename CurrPath) { // TODO: Move to MkLinesChecker. func (*Package) checkFileMakefileExt(filename CurrPath) { base := filename.Base() - if !hasPrefix(base, "Makefile.") || base == "Makefile.common" { + if !base.HasPrefixText("Makefile.") || base == "Makefile.common" { return } - ext := strings.TrimPrefix(base, "Makefile.") + ext := strings.TrimPrefix(base.String(), "Makefile.") line := NewLineWhole(filename) line.Notef("Consider renaming %q to %q.", base, ext+".mk") @@ -1566,7 +1566,7 @@ func (pkg *Package) checkIncludeConditionally(mkline *MkLine, indentation *Inden // already done with *_MK variables. } -func (pkg *Package) matchesLicenseFile(basename string) bool { +func (pkg *Package) matchesLicenseFile(basename RelPath) bool { licenseFile := NewPath(pkg.vars.LastValue("LICENSE_FILE")) return basename == licenseFile.Base() } diff --git a/pkgtools/pkglint/files/patches.go b/pkgtools/pkglint/files/patches.go index 7cef0e9225f..66115d5176d 100644 --- a/pkgtools/pkglint/files/patches.go +++ b/pkgtools/pkglint/files/patches.go @@ -365,7 +365,7 @@ func (ck *PatchChecker) checktextCvsID(text string) { } func (ck *PatchChecker) checkCanonicalPatchName(patched Path) { - patch := ck.lines.BaseName + patch := ck.lines.BaseName.String() if matches(patch, `^patch-[a-z][a-z]$`) { // This naming scheme is only accepted for historic reasons. // It has has absolutely no benefit. @@ -386,7 +386,7 @@ func (ck *PatchChecker) checkCanonicalPatchName(patched Path) { if patchNorm == patchedNorm { return } - if hasSuffix(patchedNorm, patchNorm) && patchNorm == normalize(patched.Base()) { + if hasSuffix(patchedNorm, patchNorm) && patchNorm == normalize(patched.Base().String()) { return } diff --git a/pkgtools/pkglint/files/path.go b/pkgtools/pkglint/files/path.go index 745e6562ff8..4ec9993e03a 100644 --- a/pkgtools/pkglint/files/path.go +++ b/pkgtools/pkglint/files/path.go @@ -42,7 +42,7 @@ func (p Path) Dir() Path { return NewPath(s[:end]) } -func (p Path) Base() string { return path.Base(string(p)) } +func (p Path) Base() RelPath { return NewRelPathString(path.Base(string(p))) } func (p Path) Split() (dir Path, base string) { strDir, strBase := path.Split(string(p)) @@ -157,7 +157,7 @@ func (p Path) HasSuffixPath(suffix Path) bool { (len(p) == len(suffix) || p[len(p)-len(suffix)-1] == '/') } -func (p Path) HasBase(base string) bool { return p.Base() == base } +func (p Path) HasBase(base string) bool { return p.Base().String() == base } func (p Path) TrimSuffix(suffix string) Path { return Path(strings.TrimSuffix(string(p), suffix)) @@ -253,7 +253,7 @@ func (p CurrPath) Dir() CurrPath { return CurrPath(p.AsPath().Dir()) } -func (p CurrPath) Base() string { return p.AsPath().Base() } +func (p CurrPath) Base() RelPath { return p.AsPath().Base() } func (p CurrPath) Split() (dir CurrPath, base string) { pathDir, pathBase := p.AsPath().Split() @@ -395,7 +395,7 @@ func (p PkgsrcPath) Dir() PkgsrcPath { return NewPkgsrcPath(p.AsPath().Dir()) } -func (p PkgsrcPath) Base() string { return p.AsPath().Base() } +func (p PkgsrcPath) Base() RelPath { return p.AsPath().Base() } func (p PkgsrcPath) Count() int { return p.AsPath().Count() } @@ -484,7 +484,7 @@ func (p RelPath) Dir() RelPath { return RelPath(p.AsPath().Dir()) } -func (p RelPath) Base() string { return p.AsPath().Base() } +func (p RelPath) Base() RelPath { return p.AsPath().Base() } func (p RelPath) HasBase(base string) bool { return p.AsPath().HasBase(base) } diff --git a/pkgtools/pkglint/files/path_test.go b/pkgtools/pkglint/files/path_test.go index e5dda345daa..4f2928a7d06 100644 --- a/pkgtools/pkglint/files/path_test.go +++ b/pkgtools/pkglint/files/path_test.go @@ -77,7 +77,7 @@ func (s *Suite) Test_Path_Dir(c *check.C) { func (s *Suite) Test_Path_Base(c *check.C) { t := s.Init(c) - test := func(p Path, base string) { + test := func(p Path, base RelPath) { t.CheckEquals(p.Base(), base) } @@ -615,7 +615,7 @@ func (s *Suite) Test_CurrPath_Dir(c *check.C) { func (s *Suite) Test_CurrPath_Base(c *check.C) { t := s.Init(c) - test := func(curr CurrPath, base string) { + test := func(curr CurrPath, base RelPath) { t.CheckEquals(curr.Base(), base) } @@ -1104,7 +1104,7 @@ func (s *Suite) Test_PkgsrcPath_Dir(c *check.C) { func (s *Suite) Test_PkgsrcPath_Base(c *check.C) { t := s.Init(c) - test := func(pp PkgsrcPath, base string) { + test := func(pp PkgsrcPath, base RelPath) { t.CheckEquals(pp.Base(), base) } @@ -1347,7 +1347,7 @@ func (s *Suite) Test_RelPath_Dir(c *check.C) { func (s *Suite) Test_RelPath_Base(c *check.C) { t := s.Init(c) - test := func(rel RelPath, base string) { + test := func(rel RelPath, base RelPath) { t.CheckEquals(rel.Base(), base) } diff --git a/pkgtools/pkglint/files/pkglint.go b/pkgtools/pkglint/files/pkglint.go index 41a964b10a3..757d6e922d8 100644 --- a/pkgtools/pkglint/files/pkglint.go +++ b/pkgtools/pkglint/files/pkglint.go @@ -43,7 +43,7 @@ type Pkglint struct { Username string // For checking against OWNER and MAINTAINER cvsEntriesDir CurrPath // Cached to avoid I/O - cvsEntries map[string]CvsEntry + cvsEntries map[RelPath]CvsEntry Logger Logger @@ -308,7 +308,6 @@ func (p *Pkglint) checkMode(dirent CurrPath, mode os.FileMode) { dir = dirent.Dir() } - basename := dirent.Base() pkgsrcRel := p.Pkgsrc.Rel(dirent) p.Wip = pkgsrcRel.HasPrefixPath("wip") @@ -323,7 +322,7 @@ func (p *Pkglint) checkMode(dirent CurrPath, mode os.FileMode) { if isReg { p.checkExecutable(dirent, mode) - p.checkReg(dirent, basename, pkgsrcRel.Count(), nil) + p.checkReg(dirent, dirent.Base(), pkgsrcRel.Count(), nil) return } @@ -549,10 +548,10 @@ func CheckFileMk(filename CurrPath, pkg *Package) { // checkReg checks the given regular file. // depth is 3 for files in the package directory, and 4 or more for files // deeper in the directory hierarchy, such as in files/ or patches/. -func (p *Pkglint) checkReg(filename CurrPath, basename string, depth int, pkg *Package) { +func (p *Pkglint) checkReg(filename CurrPath, basename RelPath, depth int, pkg *Package) { if depth == 3 && !p.Wip { - if contains(basename, "TODO") { + if basename.ContainsText("TODO") { NewLineWhole(filename).Errorf("Packages in main pkgsrc must not have a %s file.", basename) // TODO: Add a convincing explanation. return @@ -560,10 +559,10 @@ func (p *Pkglint) checkReg(filename CurrPath, basename string, depth int, pkg *P } switch { - case hasSuffix(basename, "~"), - hasSuffix(basename, ".orig"), - hasSuffix(basename, ".rej"), - contains(basename, "TODO") && depth == 3: + case basename.HasSuffixText("~"), + basename.HasSuffixText(".orig"), + basename.HasSuffixText(".rej"), + basename.ContainsText("TODO") && depth == 3: if p.Import { NewLineWhole(filename).Errorf("Must be cleaned up before committing the package.") } @@ -584,7 +583,7 @@ func (p *Pkglint) checkReg(filename CurrPath, basename string, depth int, pkg *P case p.Wip && basename == "COMMIT_MSG": // https://mail-index.netbsd.org/pkgsrc-users/2020/05/10/msg031174.html - case hasPrefix(basename, "DESCR"): + case basename.HasPrefixText("DESCR"): if lines := Load(filename, NotEmpty|LogErrors); lines != nil { CheckLinesDescr(lines) } @@ -597,7 +596,7 @@ func (p *Pkglint) checkReg(filename CurrPath, basename string, depth int, pkg *P case basename == "DEINSTALL" || basename == "INSTALL": CheckFileOther(filename) - case hasPrefix(basename, "MESSAGE"): + case basename.HasPrefixText("MESSAGE"): if lines := Load(filename, NotEmpty|LogErrors); lines != nil { CheckLinesMessage(lines, pkg) } @@ -611,36 +610,36 @@ func (p *Pkglint) checkReg(filename CurrPath, basename string, depth int, pkg *P CheckLinesOptionsMk(mklines, buildlinkID) } - case matches(basename, `^patch-[-\w.~+]*\w$`): + case matches(basename.String(), `^patch-[-\w.~+]*\w$`): if lines := Load(filename, NotEmpty|LogErrors); lines != nil { CheckLinesPatch(lines, pkg) } - case filename.Dir().Base() == "patches" && matches(filename.Base(), `^manual[^/]*$`): + case filename.Dir().HasBase("patches") && filename.Base().HasPrefixText("manual"): if trace.Tracing { trace.Stepf("Unchecked file %q.", filename) } - case filename.Dir().Base() == "patches": + case filename.Dir().HasBase("patches"): NewLineWhole(filename).Warnf("Patch files should be named \"patch-\", followed by letters, '-', '_', '.', and digits only.") - case (hasPrefix(basename, "Makefile") || hasSuffix(basename, ".mk")) && + case (basename.HasPrefixText("Makefile") || basename.HasSuffixText(".mk")) && !G.Pkgsrc.Rel(filename).AsPath().ContainsPath("files"): CheckFileMk(filename, pkg) - case hasPrefix(basename, "PLIST"): + case basename.HasPrefixText("PLIST"): if lines := Load(filename, NotEmpty|LogErrors); lines != nil { CheckLinesPlist(pkg, lines) } - case contains(basename, "README"): + case basename.ContainsText("README"): break - case hasPrefix(basename, "CHANGES-"): + case basename.HasPrefixText("CHANGES-"): // This only checks the file but doesn't register the changes globally. _ = p.Pkgsrc.loadDocChangesFromFile(filename) - case filename.Dir().Base() == "files": + case filename.Dir().HasBase("files"): // Skip files directly in the files/ directory, but not those further down. case basename == "spec": @@ -674,7 +673,7 @@ func (p *Pkglint) checkRegCvsSubst(filename CurrPath) { "For more information, see", "https://www.gnu.org/software/trans-coord/manual/cvs/html_node/Substitution-modes.html.", "", - sprintf("To fix this, run \"cvs admin -kkv %s\"", shquote(filename.Base()))) + sprintf("To fix this, run \"cvs admin -kkv %s\"", shquote(filename.Base().String()))) } func (p *Pkglint) checkExecutable(filename CurrPath, mode os.FileMode) { @@ -757,13 +756,13 @@ func (p *Pkglint) tools(mklines *MkLines) *Tools { } } -func (p *Pkglint) loadCvsEntries(filename CurrPath) map[string]CvsEntry { +func (p *Pkglint) loadCvsEntries(filename CurrPath) map[RelPath]CvsEntry { dir := filename.Dir().Clean() if dir == p.cvsEntriesDir { return p.cvsEntries } - var entries map[string]CvsEntry + var entries map[RelPath]CvsEntry handle := func(line *Line, add bool, text string) { if !hasPrefix(text, "/") { @@ -776,16 +775,17 @@ func (p *Pkglint) loadCvsEntries(filename CurrPath) map[string]CvsEntry { return } + key := NewRelPathString(fields[1]) if add { - entries[fields[1]] = CvsEntry{fields[1], fields[2], fields[3], fields[4], fields[5]} + entries[key] = CvsEntry{key, fields[2], fields[3], fields[4], fields[5]} } else { - delete(entries, fields[1]) + delete(entries, key) } } lines := Load(dir.JoinNoClean("CVS/Entries"), 0) if lines != nil { - entries = make(map[string]CvsEntry) + entries = make(map[RelPath]CvsEntry) for _, line := range lines.Lines { handle(line, true, line.Text) } diff --git a/pkgtools/pkglint/files/pkgsrc.go b/pkgtools/pkglint/files/pkgsrc.go index 76a8ab3fc9b..55fbdde60f9 100644 --- a/pkgtools/pkglint/files/pkgsrc.go +++ b/pkgtools/pkglint/files/pkgsrc.go @@ -186,7 +186,7 @@ func (src *Pkgsrc) loadDocChangesFromFile(filename CurrPath) []*Change { // This check has been added in 2018. // For years earlier than 2018 pkglint doesn't care because it's not a big issue anyway. year := "" - if _, yyyy := match1(filename.Base(), `-(\d\d\d\d)$`); yyyy >= "2018" { + if _, yyyy := match1(filename.Base().String(), `-(\d\d\d\d)$`); yyyy >= "2018" { year = yyyy } @@ -532,7 +532,7 @@ func (src *Pkgsrc) loadToolsPlatform() { var systems []string scopes := make(map[string]*RedundantScope) for _, mkFile := range src.File("mk/tools").ReadPaths() { - m, opsys := match1(mkFile.Base(), `^tools\.(.+)\.mk$`) + m, opsys := match1(mkFile.Base().String(), `^tools\.(.+)\.mk$`) if !m { continue } diff --git a/pkgtools/pkglint/files/plist.go b/pkgtools/pkglint/files/plist.go index 3650145cf29..b1de3d75de5 100644 --- a/pkgtools/pkglint/files/plist.go +++ b/pkgtools/pkglint/files/plist.go @@ -170,7 +170,7 @@ func (ck *PlistChecker) checkPath(pline *PlistLine, rel RelPath) { ck.checkSorted(pline) ck.checkDuplicate(pline) - if contains(rel.Base(), "${IMAKE_MANNEWSUFFIX}") { + if rel.Base().ContainsText("${IMAKE_MANNEWSUFFIX}") { pline.warnImakeMannewsuffix() } @@ -342,7 +342,7 @@ func (ck *PlistChecker) checkPathLib(pline *PlistLine, rel RelPath) { } basename := rel.Base() - if contains(basename, ".a") || contains(basename, ".so") { + if basename.ContainsText(".a") || basename.ContainsText(".so") { la := replaceAll(pline.text, `(\.a|\.so[0-9.]*)$`, ".la") if la != pline.text { laLine := ck.allFiles[NewRelPathString(la)] @@ -362,7 +362,7 @@ func (ck *PlistChecker) checkPathLib(pline *PlistLine, rel RelPath) { pline.Errorf("Only the libiconv package may install lib/charset.alias.") } - if hasSuffix(basename, ".la") && !pkg.vars.IsDefined("USE_LIBTOOL") { + if basename.HasSuffixText(".la") && !pkg.vars.IsDefined("USE_LIBTOOL") { if ck.once.FirstTime("USE_LIBTOOL") { pline.Warnf("Packages that install libtool libraries should define USE_LIBTOOL.") } @@ -710,7 +710,7 @@ type PlistRank struct { var defaultPlistRank = &PlistRank{0, "", "", ""} -func NewPlistRank(basename string) *PlistRank { +func NewPlistRank(basename RelPath) *PlistRank { isOpsys := func(s string) bool { return G.Pkgsrc.VariableType(nil, "OPSYS").basicType.HasEnum(s) } @@ -733,7 +733,7 @@ func NewPlistRank(basename string) *PlistRank { return &PlistRank{2, "", "", ""} } - parts := strings.Split(basename[6:], "-") + parts := strings.Split(basename.String()[6:], "-") rank := PlistRank{3, "", "", ""} if isOpsys(parts[0]) { rank.Opsys = parts[0] diff --git a/pkgtools/pkglint/files/tools.go b/pkgtools/pkglint/files/tools.go index 5ec09453f09..1c9b97eabac 100644 --- a/pkgtools/pkglint/files/tools.go +++ b/pkgtools/pkglint/files/tools.go @@ -154,7 +154,7 @@ func (tr *Tools) Define(name, varname string, mkline *MkLine) *Tool { return nil } - validity := tr.validity(mkline.Basename, false) + validity := tr.validity(mkline.Basename.String(), false) return tr.def(name, varname, false, validity, nil) } @@ -318,7 +318,7 @@ func (tr *Tools) parseUseTools(mkline *MkLine, createIfAbsent bool, addToUseTool return } - validity := tr.validity(mkline.Basename, addToUseTools) + validity := tr.validity(mkline.Basename.String(), addToUseTools) for _, dep := range mkline.ValueFields(value) { name := strings.Split(dep, ":")[0] if createIfAbsent || tr.ByName(name) != nil { diff --git a/pkgtools/pkglint/files/util.go b/pkgtools/pkglint/files/util.go index 8c96eef38fb..7934f1712ac 100644 --- a/pkgtools/pkglint/files/util.go +++ b/pkgtools/pkglint/files/util.go @@ -382,7 +382,7 @@ func isLocallyModified(filename CurrPath) bool { // // See http://cvsman.com/cvs-1.12.12/cvs_19.php. type CvsEntry struct { - Name string + Name RelPath Revision string Timestamp string Options string diff --git a/pkgtools/pkglint/files/vardefs.go b/pkgtools/pkglint/files/vardefs.go index cc0b69994f1..d723e300299 100644 --- a/pkgtools/pkglint/files/vardefs.go +++ b/pkgtools/pkglint/files/vardefs.go @@ -402,8 +402,8 @@ func (reg *VarTypeRegistry) enumFromFiles( var relevant []string for _, filename := range src.File(basedir).ReadPaths() { basename := filename.Base() - if matches(basename, re) { - relevant = append(relevant, replaceAll(basename, re, repl)) + if matches(basename.String(), re) { + relevant = append(relevant, replaceAll(basename.String(), re, repl)) } } if len(relevant) == 0 { diff --git a/pkgtools/pkglint/files/vartype.go b/pkgtools/pkglint/files/vartype.go index 8300a6e1bc3..af3639ff613 100644 --- a/pkgtools/pkglint/files/vartype.go +++ b/pkgtools/pkglint/files/vartype.go @@ -209,9 +209,9 @@ func (vt *Vartype) IsDefinedIfInScope() bool { return vt.options&DefinedIfInS func (vt *Vartype) IsNonemptyIfDefined() bool { return vt.options&NonemptyIfDefined != 0 } func (vt *Vartype) IsUnique() bool { return vt.options&Unique != 0 } -func (vt *Vartype) EffectivePermissions(basename string) ACLPermissions { +func (vt *Vartype) EffectivePermissions(basename RelPath) ACLPermissions { for _, aclEntry := range vt.aclEntries { - if aclEntry.matcher.matches(basename) { + if aclEntry.matcher.matches(basename.String()) { return aclEntry.permissions } } diff --git a/pkgtools/pkglint/files/vartypecheck.go b/pkgtools/pkglint/files/vartypecheck.go index a2e11fdcb0c..1de64d0fe09 100644 --- a/pkgtools/pkglint/files/vartypecheck.go +++ b/pkgtools/pkglint/files/vartypecheck.go @@ -394,7 +394,7 @@ func (cv *VartypeCheck) DependencyPattern() { return } defpat := depends(data) - if defpat.LowerOp != deppat.LowerOp { + if defpat == nil || defpat.LowerOp != deppat.LowerOp { return } if pkgver.Compare(deppat.Lower, defpat.Lower) < 0 { diff --git a/pkgtools/pkglint/files/vartypecheck_test.go b/pkgtools/pkglint/files/vartypecheck_test.go index cce97c099a1..45c59e46867 100644 --- a/pkgtools/pkglint/files/vartypecheck_test.go +++ b/pkgtools/pkglint/files/vartypecheck_test.go @@ -507,6 +507,40 @@ func (s *Suite) Test_VartypeCheck_DependencyPattern__smaller_version(c *check.C) "version 1.4abi from ../../category/lib/buildlink3.mk:13.") } +func (s *Suite) Test_VartypeCheck_DependencyPattern__API_ABI(c *check.C) { + t := s.Init(c) + + t.SetUpPackage("category/package", + ".include \"../../category/lib/buildlink3.mk\"", + "BUILDLINK_API_DEPENDS.lib+=\tlib>=1.0pkg") + t.SetUpPackage("category/lib") + t.CreateFileBuildlink3("category/lib/buildlink3.mk", + "BUILDLINK_ABI_DEPENDS.lib+=\tlib>=1.4abi") + t.Chdir("category/package") + t.FinishSetUp() + + G.checkdirPackage(".") + + t.CheckOutputEmpty() +} + +func (s *Suite) Test_VartypeCheck_DependencyPattern__ABI_API(c *check.C) { + t := s.Init(c) + + t.SetUpPackage("category/package", + ".include \"../../category/lib/buildlink3.mk\"", + "BUILDLINK_ABI_DEPENDS.lib+=\tlib>=1.1pkg") + t.SetUpPackage("category/lib") + t.CreateFileBuildlink3("category/lib/buildlink3.mk", + "BUILDLINK_API_DEPENDS.lib+=\tlib>=1.3api") + t.Chdir("category/package") + t.FinishSetUp() + + G.checkdirPackage(".") + + t.CheckOutputEmpty() +} + func (s *Suite) Test_VartypeCheck_DependencyWithPath(c *check.C) { t := s.Init(c) |