pkgsrc-Changes archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

CVS commit: pkgsrc/pkgtools/pkglint



Module Name:    pkgsrc
Committed By:   rillig
Date:           Sun Mar 22 17:43:15 UTC 2020

Modified Files:
        pkgsrc/pkgtools/pkglint: Makefile
        pkgsrc/pkgtools/pkglint/files: distinfo_test.go package.go
            package_test.go patches.go patches_test.go path.go path_test.go
            pkglint_test.go pkgsrc_test.go plist.go plist_test.go util_test.go

Log Message:
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.


To generate a diff of this commit:
cvs rdiff -u -r1.636 -r1.637 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.37 -r1.38 pkgsrc/pkgtools/pkglint/files/distinfo_test.go \
    pkgsrc/pkgtools/pkglint/files/patches.go
cvs rdiff -u -r1.84 -r1.85 pkgsrc/pkgtools/pkglint/files/package.go
cvs rdiff -u -r1.71 -r1.72 pkgsrc/pkgtools/pkglint/files/package_test.go
cvs rdiff -u -r1.36 -r1.37 pkgsrc/pkgtools/pkglint/files/patches_test.go
cvs rdiff -u -r1.9 -r1.10 pkgsrc/pkgtools/pkglint/files/path.go
cvs rdiff -u -r1.10 -r1.11 pkgsrc/pkgtools/pkglint/files/path_test.go
cvs rdiff -u -r1.61 -r1.62 pkgsrc/pkgtools/pkglint/files/pkglint_test.go
cvs rdiff -u -r1.46 -r1.47 pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go
cvs rdiff -u -r1.55 -r1.56 pkgsrc/pkgtools/pkglint/files/plist.go
cvs rdiff -u -r1.47 -r1.48 pkgsrc/pkgtools/pkglint/files/plist_test.go
cvs rdiff -u -r1.51 -r1.52 pkgsrc/pkgtools/pkglint/files/util_test.go

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: pkgsrc/pkgtools/pkglint/Makefile
diff -u pkgsrc/pkgtools/pkglint/Makefile:1.636 pkgsrc/pkgtools/pkglint/Makefile:1.637
--- pkgsrc/pkgtools/pkglint/Makefile:1.636      Sat Mar 21 16:57:20 2020
+++ pkgsrc/pkgtools/pkglint/Makefile    Sun Mar 22 17:43:15 2020
@@ -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/}

Index: pkgsrc/pkgtools/pkglint/files/distinfo_test.go
diff -u pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.37 pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.38
--- pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.37 Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/distinfo_test.go      Sun Mar 22 17:43:15 2020
@@ -185,6 +185,8 @@ func (s *Suite) Test_distinfoLinesChecke
                "",
                ".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_distinfoLinesChecke
                "",
                ".include \"../../lang/php/ext.mk\"",
                ".include \"../../mk/bsd.pkg.mk\"")
+       t.CreateFileLines("archivers/php-zlib/DESCR",
+               "Description")
 
        G.Check(t.File("archivers/php-zlib"))
 
Index: pkgsrc/pkgtools/pkglint/files/patches.go
diff -u pkgsrc/pkgtools/pkglint/files/patches.go:1.37 pkgsrc/pkgtools/pkglint/files/patches.go:1.38
--- pkgsrc/pkgtools/pkglint/files/patches.go:1.37       Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/patches.go    Sun Mar 22 17:43:15 2020
@@ -136,6 +136,7 @@ func (ck *PatchChecker) checkUnifiedDiff
                                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(a
                "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") {

Index: pkgsrc/pkgtools/pkglint/files/package.go
diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.84 pkgsrc/pkgtools/pkglint/files/package.go:1.85
--- pkgsrc/pkgtools/pkglint/files/package.go:1.84       Wed Mar 18 08:24:49 2020
+++ pkgsrc/pkgtools/pkglint/files/package.go    Sun Mar 22 17:43:15 2020
@@ -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(plistF
                "",
                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(plistF
        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 []Cu
                                "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) {

Index: pkgsrc/pkgtools/pkglint/files/package_test.go
diff -u pkgsrc/pkgtools/pkglint/files/package_test.go:1.71 pkgsrc/pkgtools/pkglint/files/package_test.go:1.72
--- pkgsrc/pkgtools/pkglint/files/package_test.go:1.71  Wed Mar 18 08:24:49 2020
+++ pkgsrc/pkgtools/pkglint/files/package_test.go       Sun Mar 22 17:43:15 2020
@@ -20,6 +20,8 @@ func (s *Suite) Test_Package__varuse_at_
                "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_
                "",
                ".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_f
                "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_loadPackage
        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__patc
                "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_CheckVarord
 
        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,

Index: pkgsrc/pkgtools/pkglint/files/patches_test.go
diff -u pkgsrc/pkgtools/pkglint/files/patches_test.go:1.36 pkgsrc/pkgtools/pkglint/files/patches_test.go:1.37
--- pkgsrc/pkgtools/pkglint/files/patches_test.go:1.36  Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/patches_test.go       Sun Mar 22 17:43:15 2020
@@ -596,6 +596,26 @@ func (s *Suite) Test_PatchChecker_Check_
        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)
 

Index: pkgsrc/pkgtools/pkglint/files/path.go
diff -u pkgsrc/pkgtools/pkglint/files/path.go:1.9 pkgsrc/pkgtools/pkglint/files/path.go:1.10
--- pkgsrc/pkgtools/pkglint/files/path.go:1.9   Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/path.go       Sun Mar 22 17:43:15 2020
@@ -94,6 +94,25 @@ func (p Path) HasPrefixPath(prefix Path)
                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.

Index: pkgsrc/pkgtools/pkglint/files/path_test.go
diff -u pkgsrc/pkgtools/pkglint/files/path_test.go:1.10 pkgsrc/pkgtools/pkglint/files/path_test.go:1.11
--- pkgsrc/pkgtools/pkglint/files/path_test.go:1.10     Sat Feb 15 13:48:40 2020
+++ pkgsrc/pkgtools/pkglint/files/path_test.go  Sun Mar 22 17:43:15 2020
@@ -487,8 +487,8 @@ func (s *Suite) Test_Path_IsAbs(c *check
        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 *c
 
        test("/", true)
        test("./", false)
-       test("C:/", runtime.GOOS == "windows")
+       test("C:/", true)
 }
 
 func (s *Suite) Test_CurrPath_HasPrefixPath(c *check.C) {

Index: pkgsrc/pkgtools/pkglint/files/pkglint_test.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.61 pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.62
--- pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.61  Wed Mar 18 08:24:49 2020
+++ pkgsrc/pkgtools/pkglint/files/pkglint_test.go       Sun Mar 22 17:43:15 2020
@@ -186,6 +186,9 @@ func (s *Suite) Test_Pkglint_Main__compl
                "",
                ".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__
 // 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_checkdirPac
                "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_checkdirPac
        // 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__r
                "",
                "COMMENT=\tComment",
                "LICENSE=\t2-clause-bsd")
+       t.CreateFileLines("category/package/DESCR",
+               "Description")
        t.CreateFileLines("category/package/PLIST",
                PlistCvsID,
                "bin/program")

Index: pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go
diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.46 pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.47
--- pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.46   Wed Mar 18 08:42:49 2020
+++ pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go        Sun Mar 22 17:43:15 2020
@@ -1092,6 +1092,18 @@ func (s *Suite) Test_Pkgsrc_ListVersions
        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)

Index: pkgsrc/pkgtools/pkglint/files/plist.go
diff -u pkgsrc/pkgtools/pkglint/files/plist.go:1.55 pkgsrc/pkgtools/pkglint/files/plist.go:1.56
--- pkgsrc/pkgtools/pkglint/files/plist.go:1.55 Wed Mar 18 08:24:49 2020
+++ pkgsrc/pkgtools/pkglint/files/plist.go      Sun Mar 22 17:43:15 2020
@@ -695,17 +695,56 @@ func (s *plistLineSorter) Sort() {
        s.autofixed = SaveAutofixChanges(NewLines(lines[0].Filename(), lines))
 }
 
-type PlistRank uint8
+type PlistRank struct {
+       Rank  int
+       Opsys string
+       Arch  string
+       Rest  string
+}
 
-const (
-       Plain PlistRank = iota
-       Common
-       CommonEnd
-       Opsys
-       Arch
-       OpsysArch
-       EmulOpsysArch
-)
+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})

Index: pkgsrc/pkgtools/pkglint/files/plist_test.go
diff -u pkgsrc/pkgtools/pkglint/files/plist_test.go:1.47 pkgsrc/pkgtools/pkglint/files/plist_test.go:1.48
--- pkgsrc/pkgtools/pkglint/files/plist_test.go:1.47    Wed Mar 18 08:24:49 2020
+++ pkgsrc/pkgtools/pkglint/files/plist_test.go Sun Mar 22 17:43:15 2020
@@ -777,8 +777,29 @@ func (s *Suite) Test_PlistChecker_checkD
        //
        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_Sor
                "@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 *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))
                }
        }
 

Index: pkgsrc/pkgtools/pkglint/files/util_test.go
diff -u pkgsrc/pkgtools/pkglint/files/util_test.go:1.51 pkgsrc/pkgtools/pkglint/files/util_test.go:1.52
--- pkgsrc/pkgtools/pkglint/files/util_test.go:1.51     Wed Mar 18 08:24:49 2020
+++ pkgsrc/pkgtools/pkglint/files/util_test.go  Sun Mar 22 17:43:15 2020
@@ -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
        }
 
        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
                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))
-       }
 }



Home | Main Index | Thread Index | Old Index