From 6f7cdbf7da7dae7a59c8c0407992d93f826d3cf1 Mon Sep 17 00:00:00 2001 From: rillig Date: Sun, 22 Mar 2020 17:43:15 +0000 Subject: pkgtools/pkglint: update to 19.4.13 Changes since 19.4.12: Files that are mentioned redundantly in PLIST files generate an error. Missing DESCR files generate an error. Hard-coded /usr/pkg in patches generates an error. --- pkgtools/pkglint/Makefile | 5 +- pkgtools/pkglint/files/distinfo_test.go | 4 ++ pkgtools/pkglint/files/package.go | 27 +++++++++- pkgtools/pkglint/files/package_test.go | 25 ++++++++- pkgtools/pkglint/files/patches.go | 13 +++++ pkgtools/pkglint/files/patches_test.go | 20 ++++++++ pkgtools/pkglint/files/path.go | 21 +++++++- pkgtools/pkglint/files/path_test.go | 6 +-- pkgtools/pkglint/files/pkglint_test.go | 13 +++-- pkgtools/pkglint/files/pkgsrc_test.go | 12 +++++ pkgtools/pkglint/files/plist.go | 91 ++++++++++++++++++++++++++------- pkgtools/pkglint/files/plist_test.go | 87 +++++++++++++++++++++++++------ pkgtools/pkglint/files/util_test.go | 60 +++++++++++----------- 13 files changed, 308 insertions(+), 76 deletions(-) (limited to 'pkgtools') diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile index 75741a2873d..e26bd77b960 100644 --- a/pkgtools/pkglint/Makefile +++ b/pkgtools/pkglint/Makefile @@ -1,7 +1,6 @@ -# $NetBSD: Makefile,v 1.636 2020/03/21 16:57:20 bsiegert Exp $ +# $NetBSD: Makefile,v 1.637 2020/03/22 17:43:15 rillig Exp $ -PKGNAME= pkglint-19.4.12 -PKGREVISION= 1 +PKGNAME= pkglint-19.4.13 CATEGORIES= pkgtools DISTNAME= tools MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/} diff --git a/pkgtools/pkglint/files/distinfo_test.go b/pkgtools/pkglint/files/distinfo_test.go index b5ecc647cf6..f590e4bd120 100644 --- a/pkgtools/pkglint/files/distinfo_test.go +++ b/pkgtools/pkglint/files/distinfo_test.go @@ -185,6 +185,8 @@ func (s *Suite) Test_distinfoLinesChecker_check__missing_php_patches(c *check.C) "", ".include \"../../lang/php/ext.mk\"", ".include \"../../mk/bsd.pkg.mk\"") + t.CreateFileLines("archivers/php-bz2/DESCR", + "Description") t.FinishSetUp() G.Check(t.File("archivers/php-bz2")) @@ -194,6 +196,8 @@ func (s *Suite) Test_distinfoLinesChecker_check__missing_php_patches(c *check.C) "", ".include \"../../lang/php/ext.mk\"", ".include \"../../mk/bsd.pkg.mk\"") + t.CreateFileLines("archivers/php-zlib/DESCR", + "Description") G.Check(t.File("archivers/php-zlib")) diff --git a/pkgtools/pkglint/files/package.go b/pkgtools/pkglint/files/package.go index b9d8bded8f8..d1625fea55c 100644 --- a/pkgtools/pkglint/files/package.go +++ b/pkgtools/pkglint/files/package.go @@ -31,6 +31,7 @@ type Package struct { Patchdir PackagePath // PATCHDIR from the package Makefile DistinfoFile PackagePath // DISTINFO_FILE from the package Makefile Plist PlistContent // Files and directories mentioned in the PLIST files + PlistLines *PlistLines vars Scope redundant *RedundantScope @@ -104,6 +105,7 @@ func NewPackage(dir CurrPath) *Package { Patchdir: "patches", // TODO: Redundant, see the vars.Fallback below. DistinfoFile: "${PKGDIR}/distinfo", // TODO: Redundant, see the vars.Fallback below. Plist: NewPlistContent(), + PlistLines: NewPlistLines(), vars: NewScope(), bl3: make(map[PackagePath]*MkLine), bl3Data: make(map[Buildlink3ID]*Buildlink3Data), @@ -520,7 +522,7 @@ func (pkg *Package) loadPlistDirs(plistFilename CurrPath) { "", Once{}, false} - ck.Load(lines) + plistLines := ck.Load(lines) for filename, pline := range ck.allFiles { pkg.Plist.Files[filename] = pline @@ -528,6 +530,12 @@ func (pkg *Package) loadPlistDirs(plistFilename CurrPath) { for dirname, pline := range ck.allDirs { pkg.Plist.Dirs[dirname] = pline } + for _, plistLine := range plistLines { + if plistLine.HasPath() { + rank := NewPlistRank(plistLine.Basename) + pkg.PlistLines.Add(plistLine, rank) + } + } } func (pkg *Package) check(filenames []CurrPath, mklines, allLines *MkLines) { @@ -577,7 +585,24 @@ func (pkg *Package) check(filenames []CurrPath, mklines, allLines *MkLines) { "To generate a distinfo file for the existing patches, run", sprintf("%q.", bmake("makepatchsum"))) } + + pkg.checkDescr(filenames, mklines) + } +} + +func (pkg *Package) checkDescr(filenames []CurrPath, mklines *MkLines) { + if mklines == nil { + return + } + for _, filename := range filenames { + if filename.HasBase("DESCR") { + return + } + } + if pkg.vars.IsDefined("DESCR_SRC") { + return } + mklines.Whole().Errorf("Each package must have a DESCR file.") } func (pkg *Package) checkfilePackageMakefile(filename CurrPath, mklines *MkLines, allLines *MkLines) { diff --git a/pkgtools/pkglint/files/package_test.go b/pkgtools/pkglint/files/package_test.go index 473f7da1a89..1cb6e2ab983 100644 --- a/pkgtools/pkglint/files/package_test.go +++ b/pkgtools/pkglint/files/package_test.go @@ -20,6 +20,8 @@ func (s *Suite) Test_Package__varuse_at_load_time(c *check.C) { "TOOLS_CREATE+=nice", "TOOLS_CREATE+=true", "_TOOLS_VARNAME.nice=NICE") + t.CreateFileLines("category/pkgbase/DESCR", + "Description") t.CreateFileLines("category/pkgbase/Makefile", MkCvsID, @@ -247,6 +249,8 @@ func (s *Suite) Test_Package__redundant_master_sites(c *check.C) { "", ".include \"../../math/R/Makefile.extension\"", ".include \"../../mk/bsd.pkg.mk\"") + t.CreateFileLines("math/R-date/DESCR", + "Description") t.FinishSetUp() // See Package.checkfilePackageMakefile @@ -297,7 +301,9 @@ func (s *Suite) Test_Package__distinfo_from_other_package(c *check.C) { "WARN: x11/gst-x11/Makefile: This package should have a PLIST file.", "ERROR: x11/gst-x11/Makefile: Each package must define its LICENSE.", "WARN: x11/gst-x11/Makefile: Each package should define a COMMENT.", - "WARN: x11/gst-x11/../../multimedia/gst-base/distinfo:3: Patch file \"patch-aa\" does not exist in directory \"../../x11/gst-x11/patches\".") + "WARN: x11/gst-x11/../../multimedia/gst-base/distinfo:3: "+ + "Patch file \"patch-aa\" does not exist in directory \"../../x11/gst-x11/patches\".", + "ERROR: x11/gst-x11/Makefile: Each package must have a DESCR file.") } func (s *Suite) Test_Package__case_insensitive(c *check.C) { @@ -549,6 +555,8 @@ func (s *Suite) Test_Package_loadPackageMakefile__dump(c *check.C) { t.SetUpCommandLine("--dumpmakefile") t.SetUpPkgsrc() t.CreateFileLines("category/Makefile") + t.CreateFileLines("category/package/DESCR", + "Description") t.CreateFileLines("category/package/PLIST", PlistCvsID, "bin/program") @@ -1322,6 +1330,20 @@ func (s *Suite) Test_Package_check__patches_Makefile(c *check.C) { "1 warning found.") } +func (s *Suite) Test_Package_checkDescr__DESCR_SRC(c *check.C) { + t := s.Init(c) + + t.SetUpPackage("other/package") + t.SetUpPackage("category/package", + "DESCR_SRC=\t../../other/package/DESCR") + t.Remove("category/package/DESCR") + t.FinishSetUp() + + G.Check(t.File("category/package")) + + t.CheckOutputEmpty() +} + func (s *Suite) Test_Package_checkfilePackageMakefile__GNU_CONFIGURE(c *check.C) { t := s.Init(c) @@ -1923,6 +1945,7 @@ func (s *Suite) Test_Package_CheckVarorder__license(c *check.C) { t.CreateFileLines("mk/bsd.pkg.mk", "# dummy") t.CreateFileLines("x11/Makefile", MkCvsID) + t.CreateFileLines("x11/9term/DESCR", "Terminal") t.CreateFileLines("x11/9term/PLIST", PlistCvsID, "bin/9term") t.CreateFileLines("x11/9term/Makefile", MkCvsID, diff --git a/pkgtools/pkglint/files/patches.go b/pkgtools/pkglint/files/patches.go index 2364f2c6d4e..31c71a3d57a 100644 --- a/pkgtools/pkglint/files/patches.go +++ b/pkgtools/pkglint/files/patches.go @@ -136,6 +136,7 @@ func (ck *PatchChecker) checkUnifiedDiff(patchedFile Path) { linesToAdd-- ck.checktextCvsID(text) ck.checkConfigure(text[1:], isConfigure) + ck.checkAddedLine(text[1:]) case hasPrefix(text, "\\"): // \ No newline at end of file (or a translation of that message) @@ -222,6 +223,18 @@ func (ck *PatchChecker) checkConfigure(addedText string, isConfigure bool) { "mk/configure/gnu-configure.mk.") } +func (ck *PatchChecker) checkAddedLine(addedText string) { + if !matches(addedText, `/usr/pkg\b`) { + return + } + + line := ck.llex.PreviousLine() + line.Errorf("Patches must not hard-code the pkgsrc PREFIX.") + line.Explain( + "Instead of hard-coding /usr/pkg, packages should use the PREFIX variable.", + "The usual way of doing this is to use the SUBST framework in mk/subst.mk.") +} + func (ck *PatchChecker) checktextUniHunkCr() { line := ck.llex.PreviousLine() if !hasSuffix(line.Text, "\r") { diff --git a/pkgtools/pkglint/files/patches_test.go b/pkgtools/pkglint/files/patches_test.go index 47e040c8faa..23977c9e97f 100644 --- a/pkgtools/pkglint/files/patches_test.go +++ b/pkgtools/pkglint/files/patches_test.go @@ -596,6 +596,26 @@ func (s *Suite) Test_PatchChecker_Check__absolute_path(c *check.C) { t.CheckOutputEmpty() } +func (s *Suite) Test_PatchChecker_Check__add_hardcoded_usr_pkg(c *check.C) { + t := s.Init(c) + + lines := t.SetUpFileLines("patch-aa", + CvsID, + "", + "This patch wrongly contains the hard-coded PREFIX.", + "", + "--- Makefile", + "+++ Makefile", + "@@ -1,1 +1,1 @@", + "- prefix := @prefix@", + "+ prefix := /usr/pkg") + + CheckLinesPatch(lines, nil) + + t.CheckOutputLines( + "ERROR: ~/patch-aa:9: Patches must not hard-code the pkgsrc PREFIX.") +} + func (s *Suite) Test_PatchChecker_checkUnifiedDiff__lines_at_end(c *check.C) { t := s.Init(c) diff --git a/pkgtools/pkglint/files/path.go b/pkgtools/pkglint/files/path.go index b30b2799274..745e6562ff8 100644 --- a/pkgtools/pkglint/files/path.go +++ b/pkgtools/pkglint/files/path.go @@ -94,6 +94,25 @@ func (p Path) HasPrefixPath(prefix Path) bool { return !p.IsAbs() } + si := 0 + pi := 0 + for { + for si < len(p) && (p[si] == '.' || p[si] == '/') { + si++ + } + for pi < len(prefix) && (prefix[pi] == '.' || prefix[pi] == '/') { + pi++ + } + if si >= len(p) || pi >= len(prefix) { + break + } + if p[si] != prefix[pi] { + return false + } + si++ + pi++ + } + parts := p.Parts() prefixParts := prefix.Parts() if len(prefixParts) > len(parts) { @@ -192,7 +211,7 @@ func (p Path) CleanPath() Path { } func (p Path) IsAbs() bool { - return p.HasPrefixText("/") || filepath.IsAbs(filepath.FromSlash(string(p))) + return len(p) > 0 && (p[0] == '/' || len(p) > 2 && p[1] == ':' && p[2] == '/') } // Rel returns the relative path from this path to the other. diff --git a/pkgtools/pkglint/files/path_test.go b/pkgtools/pkglint/files/path_test.go index f8f18893f6e..e631ad1d932 100644 --- a/pkgtools/pkglint/files/path_test.go +++ b/pkgtools/pkglint/files/path_test.go @@ -487,8 +487,8 @@ func (s *Suite) Test_Path_IsAbs(c *check.C) { test(".", false) test("a/b", false) test("/a", true) - test("C:/", runtime.GOOS == "windows") - test("c:/", runtime.GOOS == "windows") + test("C:/", true) + test("c:/", true) } func (s *Suite) Test_Path_Rel(c *check.C) { @@ -651,7 +651,7 @@ func (s *Suite) Test_CurrPath_IsAbs(c *check.C) { test("/", true) test("./", false) - test("C:/", runtime.GOOS == "windows") + test("C:/", true) } func (s *Suite) Test_CurrPath_HasPrefixPath(c *check.C) { diff --git a/pkgtools/pkglint/files/pkglint_test.go b/pkgtools/pkglint/files/pkglint_test.go index 57d2102ca60..217f2b32507 100644 --- a/pkgtools/pkglint/files/pkglint_test.go +++ b/pkgtools/pkglint/files/pkglint_test.go @@ -186,6 +186,9 @@ func (s *Suite) Test_Pkglint_Main__complete_package(c *check.C) { "", ".include \"../../mk/bsd.pkg.mk\"") + t.CreateFileLines("sysutils/checkperms/DESCR", + "Description") + t.CreateFileLines("sysutils/checkperms/MESSAGE", "===========================================================================", CvsID, @@ -528,7 +531,7 @@ func (s *Suite) Test_Pkglint_checkMode__neither_file_nor_directory(c *check.C) { // A package that is very incomplete may produce lots of warnings. // This case is unrealistic since most packages are either generated by url2pkg // or copied from an existing working package. -func (s *Suite) Test_Pkglint_checkdirPackage(c *check.C) { +func (s *Suite) Test_Pkglint_checkdirPackage__incomplete_package(c *check.C) { t := s.Init(c) t.Chdir("category/package") @@ -541,7 +544,8 @@ func (s *Suite) Test_Pkglint_checkdirPackage(c *check.C) { "WARN: Makefile: This package should have a PLIST file.", "WARN: distinfo: A package that downloads files should have a distinfo file.", "ERROR: Makefile: Each package must define its LICENSE.", - "WARN: Makefile: Each package should define a COMMENT.") + "WARN: Makefile: Each package should define a COMMENT.", + "ERROR: Makefile: Each package must have a DESCR file.") } func (s *Suite) Test_Pkglint_checkdirPackage__PKGDIR(c *check.C) { @@ -609,7 +613,8 @@ func (s *Suite) Test_Pkglint_checkdirPackage__meta_package_without_license(c *ch // No error about missing LICENSE since meta-packages don't need a license. // They are so simple that there is no reason to have any license. t.CheckOutputLines( - "WARN: Makefile: Each package should define a COMMENT.") + "WARN: Makefile: Each package should define a COMMENT.", + "ERROR: Makefile: Each package must have a DESCR file.") } func (s *Suite) Test_Pkglint_checkdirPackage__filename_with_variable(c *check.C) { @@ -1029,6 +1034,8 @@ func (s *Suite) Test_Pkglint_checkReg__readme_and_todo(c *check.C) { "", "COMMENT=\tComment", "LICENSE=\t2-clause-bsd") + t.CreateFileLines("category/package/DESCR", + "Description") t.CreateFileLines("category/package/PLIST", PlistCvsID, "bin/program") diff --git a/pkgtools/pkglint/files/pkgsrc_test.go b/pkgtools/pkglint/files/pkgsrc_test.go index 16fcc68eb0d..0eb9fda3e4d 100644 --- a/pkgtools/pkglint/files/pkgsrc_test.go +++ b/pkgtools/pkglint/files/pkgsrc_test.go @@ -1092,6 +1092,18 @@ func (s *Suite) Test_Pkgsrc_ListVersions__error_is_cached(c *check.C) { t.CheckOutputEmpty() // No repeated error message } +func (s *Suite) Test_Pkgsrc_ListVersions__empty_directories_are_ignored(c *check.C) { + t := s.Init(c) + + t.CreateFileLines("lang/lang123/Makefile") + t.CreateFileLines("lang/lang124/empty/empty/empty/empty/CVS/Entries") + + versions := G.Pkgsrc.ListVersions("lang", `^lang[0-9]+$`, "lang/$0", true) + + // lang/lang124 is not mentioned since it is "essentially empty". + t.CheckDeepEquals(versions, []string{"lang/lang123"}) +} + // See PR 46570, Ctrl+F "3. In lang/perl5". func (s *Suite) Test_Pkgsrc_VariableType(c *check.C) { t := s.Init(c) diff --git a/pkgtools/pkglint/files/plist.go b/pkgtools/pkglint/files/plist.go index 6d5a1f5d9fa..6a097ebb905 100644 --- a/pkgtools/pkglint/files/plist.go +++ b/pkgtools/pkglint/files/plist.go @@ -695,17 +695,56 @@ func (s *plistLineSorter) Sort() { s.autofixed = SaveAutofixChanges(NewLines(lines[0].Filename(), lines)) } -type PlistRank uint8 - -const ( - Plain PlistRank = iota - Common - CommonEnd - Opsys - Arch - OpsysArch - EmulOpsysArch -) +type PlistRank struct { + Rank int + Opsys string + Arch string + Rest string +} + +var defaultPlistRank = &PlistRank{0, "", "", ""} + +func NewPlistRank(basename string) *PlistRank { + isOpsys := func(s string) bool { + return G.Pkgsrc.VariableType(nil, "OPSYS").basicType.HasEnum(s) + } + isArch := func(s string) bool { + return G.Pkgsrc.VariableType(nil, "MACHINE_ARCH").basicType.HasEnum(s) + } + isEmulOpsys := func(s string) bool { + return G.Pkgsrc.VariableType(nil, "EMUL_OPSYS").basicType.HasEnum(s) + } + isEmulArch := func(s string) bool { + return G.Pkgsrc.VariableType(nil, "EMUL_ARCH").basicType.HasEnum(s) + } + + switch basename { + case "PLIST": + return defaultPlistRank + case "PLIST.common": + return &PlistRank{1, "", "", ""} + case "PLIST.common_end": + return &PlistRank{2, "", "", ""} + } + + parts := strings.Split(basename[6:], "-") + rank := PlistRank{3, "", "", ""} + if isOpsys(parts[0]) { + rank.Opsys = parts[0] + parts = parts[1:] + } + if len(parts) > 0 && isArch(parts[0]) { + rank.Arch = parts[0] + parts = parts[1:] + } + if len(parts) >= 2 && isEmulOpsys(parts[0]) && isEmulArch(parts[1]) { + rank.Opsys = parts[0] + rank.Arch = parts[1] + parts = parts[2:] + } + rank.Rest = strings.Join(parts, "-") + return &rank +} // The ranks among the files are: // PLIST @@ -715,10 +754,20 @@ const ( // -> { PLIST.OPSYS.ARCH, PLIST.EMUL_PLATFORM } // Files are a later level must not mention files that are already // mentioned at an earlier level. -func (r PlistRank) Dominates(other PlistRank) bool { - return r <= other && - !(r == Opsys && other == Arch) && - !(r == OpsysArch && other == EmulOpsysArch) +func (r *PlistRank) MoreGeneric(other *PlistRank) bool { + if r.Rank != 3 && other.Rank != 3 { + return r.Rank < other.Rank + } + if r.Opsys != "" && r.Opsys != other.Opsys { + return false + } + if r.Arch != "" && r.Arch != other.Arch { + return false + } + if r.Rest != "" && r.Rest != other.Rest { + return false + } + return *r != *other } type PlistLines struct { @@ -731,15 +780,21 @@ func NewPlistLines() *PlistLines { type plistLineData struct { line *PlistLine - rank PlistRank + rank *PlistRank } -func (pl *PlistLines) Add(line *PlistLine, rank PlistRank) { +func (pl *PlistLines) Add(line *PlistLine, rank *PlistRank) { path := line.Path() for _, existing := range pl.all[path] { - if existing.rank.Dominates(rank) { + switch { + case existing.rank == rank: + break + case existing.rank.MoreGeneric(rank): line.Errorf("Path %s is already listed in %s.", path, line.RelLine(existing.line.Line)) + case rank.MoreGeneric(existing.rank): + existing.line.Errorf("Path %s is already listed in %s.", + path, existing.line.RelLine(line.Line)) } } pl.all[path] = append(pl.all[path], &plistLineData{line, rank}) diff --git a/pkgtools/pkglint/files/plist_test.go b/pkgtools/pkglint/files/plist_test.go index 210d8a5db34..55758a0fb10 100644 --- a/pkgtools/pkglint/files/plist_test.go +++ b/pkgtools/pkglint/files/plist_test.go @@ -777,8 +777,29 @@ func (s *Suite) Test_PlistChecker_checkDuplicate__OPSYS(c *check.C) { // G.Check(".") - // TODO: Warn that bin/program is duplicate, but not bin/os-specific. - t.CheckOutputEmpty() + // For example, bin/program is duplicate, but not bin/os-specific. + t.CheckOutputLines( + "ERROR: PLIST.Linux:2: Path bin/common is already listed in PLIST:2.", + "ERROR: PLIST.Linux:3: Path bin/common_end is already listed in PLIST:3.", + "ERROR: PLIST.Linux:4: Path bin/conditional is already listed in PLIST:4.", + "ERROR: PLIST.Linux:6: Path bin/plist is already listed in PLIST:5.", + "ERROR: PLIST.NetBSD:2: Path bin/common is already listed in PLIST:2.", + "ERROR: PLIST.NetBSD:3: Path bin/common_end is already listed in PLIST:3.", + "ERROR: PLIST.NetBSD:4: Path bin/conditional is already listed in PLIST:4.", + "ERROR: PLIST.NetBSD:6: Path bin/plist is already listed in PLIST:5.", + "ERROR: PLIST.common:2: Path bin/common is already listed in PLIST:2.", + "ERROR: PLIST.Linux:2: Path bin/common is already listed in PLIST.common:2.", + "ERROR: PLIST.NetBSD:2: Path bin/common is already listed in PLIST.common:2.", + "ERROR: PLIST.common:3: Path bin/conditional is already listed in PLIST:4.", + "ERROR: PLIST.Linux:4: Path bin/conditional is already listed in PLIST.common:3.", + "ERROR: PLIST.NetBSD:4: Path bin/conditional is already listed in PLIST.common:3.", + "ERROR: PLIST.common_end:2: Path bin/common_end is already listed in PLIST:3.", + "ERROR: PLIST.Linux:3: Path bin/common_end is already listed in PLIST.common_end:2.", + "ERROR: PLIST.NetBSD:3: Path bin/common_end is already listed in PLIST.common_end:2.", + "ERROR: PLIST.common_end:3: Path bin/conditional is already listed in PLIST:4.", + "ERROR: PLIST.Linux:4: Path bin/conditional is already listed in PLIST.common_end:3.", + "ERROR: PLIST.NetBSD:4: Path bin/conditional is already listed in PLIST.common_end:3.", + "ERROR: PLIST.common_end:3: Path bin/conditional is already listed in PLIST.common:3.") } func (s *Suite) Test_PlistChecker_checkPathBin(c *check.C) { @@ -1400,21 +1421,57 @@ func (s *Suite) Test_plistLineSorter_Sort(c *check.C) { "@exec echo \"after lib/after.la\"") // The footer starts here } -func (s *Suite) Test_PlistRank_Dominates(c *check.C) { +func (s *Suite) Test_NewPlistRank(c *check.C) { + t := s.Init(c) + + t.SetUpVartypes() + + t.CheckDeepEquals(NewPlistRank("PLIST"), &PlistRank{0, "", "", ""}) + t.CheckDeepEquals(NewPlistRank("PLIST.common"), &PlistRank{1, "", "", ""}) + t.CheckDeepEquals(NewPlistRank("PLIST.common_end"), &PlistRank{2, "", "", ""}) + t.CheckDeepEquals(NewPlistRank("PLIST.NetBSD"), &PlistRank{3, "NetBSD", "", ""}) + t.CheckDeepEquals(NewPlistRank("PLIST.NetBSD-opt"), &PlistRank{3, "NetBSD", "", "opt"}) + t.CheckDeepEquals(NewPlistRank("PLIST.x86_64"), &PlistRank{3, "", "x86_64", ""}) + t.CheckDeepEquals(NewPlistRank("PLIST.NetBSD-x86_64"), &PlistRank{3, "NetBSD", "x86_64", ""}) + t.CheckDeepEquals(NewPlistRank("PLIST.linux-x86_64"), &PlistRank{3, "linux", "x86_64", ""}) + t.CheckDeepEquals(NewPlistRank("PLIST.solaris-sparc"), &PlistRank{3, "solaris", "sparc", ""}) + t.CheckDeepEquals(NewPlistRank("PLIST.other"), &PlistRank{3, "", "", "other"}) + + // To list all current PLIST filenames: + // cd $pkgsrcdir && find . -name 'PLIST*' -printf '%f\n' | sort | uniq -c | sort -nr +} + +func (s *Suite) Test_PlistRank_MoreGeneric(c *check.C) { + t := s.Init(c) + + t.SetUpVartypes() + plain := NewPlistRank("PLIST") + common := NewPlistRank("PLIST.common") + commonEnd := NewPlistRank("PLIST.common_end") + netbsd := NewPlistRank("PLIST.NetBSD") + netbsdRest := NewPlistRank("PLIST.NetBSD-rest") + x86 := NewPlistRank("PLIST.x86_64") + netbsdX86 := NewPlistRank("PLIST.NetBSD-x86_64") + linuxX86 := NewPlistRank("PLIST.Linux-x86_64") + emulLinuxX86 := NewPlistRank("PLIST.linux-x86_64") + var rel relation - rel.add(Plain, Common) - rel.add(Common, CommonEnd) - rel.add(CommonEnd, Opsys) - rel.add(CommonEnd, Arch) - rel.add(Opsys, OpsysArch) - rel.add(Opsys, EmulOpsysArch) - rel.add(Arch, OpsysArch) - rel.add(Arch, EmulOpsysArch) - rel.reflexive = true + rel.add(plain, common) + rel.add(common, commonEnd) + rel.add(commonEnd, netbsd) + rel.add(commonEnd, x86) + rel.add(commonEnd, linuxX86) + rel.add(netbsd, netbsdX86) + rel.add(netbsd, netbsdRest) + rel.add(x86, netbsdX86) + rel.add(x86, linuxX86) + rel.add(x86, emulLinuxX86) + rel.reflexive = false rel.transitive = true rel.antisymmetric = true + rel.onError = func(s string) { c.Error(s) } - rel.check(func(a, b int) bool { return PlistRank(a).Dominates(PlistRank(b)) }) + rel.check(func(a, b interface{}) bool { return a.(*PlistRank).MoreGeneric(b.(*PlistRank)) }) } func (s *Suite) Test_NewPlistLines(c *check.C) { @@ -1438,12 +1495,12 @@ func (s *Suite) Test_PlistLines_Add(c *check.C) { for _, line := range plistLines { if line.HasPath() { - lines.Add(line, Plain) + lines.Add(line, NewPlistRank(line.Basename)) } } for _, line := range plistCommonLines { if line.HasPath() { - lines.Add(line, Common) + lines.Add(line, NewPlistRank(line.Basename)) } } diff --git a/pkgtools/pkglint/files/util_test.go b/pkgtools/pkglint/files/util_test.go index 201ba90a446..b0558ee394d 100644 --- a/pkgtools/pkglint/files/util_test.go +++ b/pkgtools/pkglint/files/util_test.go @@ -1269,33 +1269,32 @@ func (s *Suite) Test_interval(c *check.C) { } type relation struct { - pairs []struct{ a, b int } + idx map[interface{}]int + elements []interface{} + pairs []struct{ a, b interface{} } reflexive bool transitive bool antisymmetric bool + onError func(s string) } func (r *relation) add(a interface{}, b interface{}) { - ia := int(reflect.ValueOf(a).Uint()) - ib := int(reflect.ValueOf(b).Uint()) - r.pairs = append(r.pairs, struct{ a, b int }{ia, ib}) -} - -func (r *relation) size() int { - max := 0 - for _, pair := range r.pairs { - if pair.a > max { - max = pair.a - } - if pair.b > max { - max = pair.b - } + if r.idx == nil { + r.idx = make(map[interface{}]int) } - return max + 1 + if _, ok := r.idx[a]; !ok { + r.idx[a] = len(r.idx) + r.elements = append(r.elements, a) + } + if _, ok := r.idx[b]; !ok { + r.idx[b] = len(r.idx) + r.elements = append(r.elements, b) + } + r.pairs = append(r.pairs, struct{ a, b interface{} }{a, b}) } -func (r *relation) check(actual func(int, int) bool) { - n := r.size() +func (r *relation) check(actual func(interface{}, interface{}) bool) { + n := len(r.idx) rel := make([][]bool, n) for i := 0; i < n; i++ { rel[i] = make([]bool, n) @@ -1308,7 +1307,7 @@ func (r *relation) check(actual func(int, int) bool) { } for _, pair := range r.pairs { - rel[pair.a][pair.b] = true + rel[r.idx[pair.a]][r.idx[pair.b]] = true } if r.transitive { @@ -1334,26 +1333,25 @@ func (r *relation) check(actual func(int, int) bool) { for i := 0; i < n; i++ { for j := 0; j < n; j++ { if i != j && rel[i][j] && rel[j][i] { - panic(sprintf( + r.onError(sprintf( "the antisymmetric relation must not contain "+ - "both (%[1]d, %[2]d) and (%[2]d, %[1]d)", - i, j)) + "both (%#[1]v, %#[2]v) and (%#[2]v, %#[1]v)", + r.elements[i], r.elements[j])) } } } } - actualRel := make([][]bool, n) for i := 0; i < n; i++ { - actualRel[i] = make([]bool, n) for j := 0; j < n; j++ { - actualRel[i][j] = actual(i, j) + ei := r.elements[i] + ej := r.elements[j] + actualRel := actual(ei, ej) + if actualRel != rel[i][j] { + _ = actual(ei, ej) + r.onError(sprintf("expected %#v <=> %#v to be %v, was %v", + ei, ej, rel[i][j], actualRel)) + } } } - - if sprintf("%#v", rel) != sprintf("%#v", actualRel) { - // The line breaks at these positions make the two relations - // visually comparable in the output. - panic(sprintf("the relation must be\n%#v, not \n%#v", rel, actualRel)) - } } -- cgit v1.2.3