pkgtools/pkglint: update to 21.4.2

Changes since 21.4.1:

When checking a package, check for naming collision with other packages
from the same category, on case-insensitive file systems.  For packages
from pkgsrc-wip, additionally perform the same check for the main
category of the package, to prepare for importing the package.

8 files changed, 164 insertions, 42 deletions

diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile
index 1310d6f680d..acadf80318c 100644
--- a/pkgtools/pkglint/Makefile
+++ b/pkgtools/pkglint/Makefile
@@ -1,7 +1,6 @@
-# $NetBSD: Makefile,v 1.708 2022/01/09 20:10:37 bsiegert Exp $
+# $NetBSD: Makefile,v 1.709 2022/01/16 19:14:51 rillig Exp $
 
-PKGNAME=	pkglint-21.4.1
-PKGREVISION=	1
+PKGNAME=	pkglint-21.4.2
 CATEGORIES=	pkgtools
 DISTNAME=	tools
 MASTER_SITES=	${MASTER_SITE_GITHUB:=golang/}
diff --git a/pkgtools/pkglint/files/category.go b/pkgtools/pkglint/files/category.go
index 1d5f9aae33a..1ddc3da5dca 100644
--- a/pkgtools/pkglint/files/category.go
+++ b/pkgtools/pkglint/files/category.go
@@ -5,7 +5,7 @@ import (
 	"strings"
 )
 
-func CheckdirCategory(dir CurrPath) {
+func CheckdirCategory(dir CurrPath, recurse bool) {
 	if trace.Tracing {
 		defer trace.Call(dir)()
 	}
@@ -48,6 +48,7 @@ func CheckdirCategory(dir CurrPath) {
 
 	var fSubdirs []RelPath
 	var mSubdirs []subdir
+	var recurseInto []CurrPath
 
 	for _, subdir := range getSubdirs(dir) {
 		if dir.JoinNoClean(subdir).JoinNoClean("Makefile").IsFile() {
@@ -96,6 +97,9 @@ func CheckdirCategory(dir CurrPath) {
 			}
 
 			mSubdirs = append(mSubdirs, subdir{sub, mkline})
+			if recurse && !mkline.IsCommentedVarassign() {
+				recurseInto = append(recurseInto, dir.JoinNoClean(sub))
+			}
 
 		} else {
 			if !mkline.IsEmpty() {
@@ -168,13 +172,30 @@ func CheckdirCategory(dir CurrPath) {
 
 	mklines.SaveAutofixChanges()
 
-	if G.Recursive {
-		var recurseInto []CurrPath
-		for _, msub := range mSubdirs {
-			if !msub.line.IsCommentedVarassign() {
-				recurseInto = append(recurseInto, dir.JoinNoClean(msub.name))
+	G.Todo.PushFront(recurseInto...)
+}
+
+func CheckPackageDirCollision(dir CurrPath, pkgdir RelPath) {
+	mklines := LoadMk(dir.JoinNoClean("Makefile").CleanDot(), nil, 0)
+	if mklines == nil {
+		return
+	}
+
+	lowerPkgdir := strings.ToLower(pkgdir.String())
+	mklines.ForEach(func(mkline *MkLine) {
+		if mkline.IsVarassignMaybeCommented() && mkline.Varname() == "SUBDIR" {
+			value := NewPath(mkline.Value())
+			if value.IsAbs() {
+				return
+			}
+			sub := NewRelPath(value)
+			lowerSub := strings.ToLower(sub.String())
+			if lowerSub == lowerPkgdir && sub != pkgdir {
+				// TODO: Merge duplicate code from CheckdirCategory.
+				mkline.Errorf("On case-insensitive file systems, "+
+					"%q is the same as %q.",
+					sub, pkgdir)
 			}
 		}
-		G.Todo.PushFront(recurseInto...)
-	}
+	})
 }
diff --git a/pkgtools/pkglint/files/category_test.go b/pkgtools/pkglint/files/category_test.go
index 58e4437b6e8..50ef4236f71 100644
--- a/pkgtools/pkglint/files/category_test.go
+++ b/pkgtools/pkglint/files/category_test.go
@@ -14,7 +14,7 @@ func (s *Suite) Test_CheckdirCategory__totally_broken(c *check.C) {
 		"",
 		".include \"../mk/category.mk\"")
 
-	CheckdirCategory(t.File("archivers"))
+	CheckdirCategory(t.File("archivers"), false)
 
 	t.CheckOutputLines(
 		"ERROR: ~/archivers/Makefile:1: Expected \"# $"+"NetBSD$\".",
@@ -53,7 +53,7 @@ func (s *Suite) Test_CheckdirCategory__invalid_comment(c *check.C) {
 	t.CreateFileLines("mk/misc/category.mk",
 		"# dummy")
 
-	CheckdirCategory(t.File("archivers"))
+	CheckdirCategory(t.File("archivers"), true)
 
 	t.CheckOutputLines(
 		"WARN: ~/archivers/Makefile:3: COMMENT contains invalid characters (\\ $ $ $ $ \").")
@@ -118,7 +118,7 @@ func (s *Suite) Test_CheckdirCategory__subdirs(c *check.C) {
 		".include \"../mk/misc/category.mk\"")
 	t.FinishSetUp()
 
-	CheckdirCategory(t.File("category"))
+	CheckdirCategory(t.File("category"), false)
 
 	t.CheckOutputLines(
 		"ERROR: ~/category/Makefile:7: \"duplicate\" must only appear once, already seen in line 5.",
@@ -148,7 +148,7 @@ func (s *Suite) Test_CheckdirCategory__only_in_Makefile(c *check.C) {
 		".include \"../mk/misc/category.mk\"")
 	t.FinishSetUp()
 
-	CheckdirCategory(t.File("category"))
+	CheckdirCategory(t.File("category"), false)
 
 	t.CheckOutputLines(
 		"ERROR: ~/category/Makefile:5: "+
@@ -175,7 +175,7 @@ func (s *Suite) Test_CheckdirCategory__only_in_file_system(c *check.C) {
 		".include \"../mk/misc/category.mk\"")
 	t.FinishSetUp()
 
-	CheckdirCategory(t.File("category"))
+	CheckdirCategory(t.File("category"), false)
 
 	t.CheckOutputLines(
 		"ERROR: ~/category/Makefile:5: Package \"above-only-in-fs\" must be listed here.",
@@ -208,7 +208,7 @@ func (s *Suite) Test_CheckdirCategory__recursive(c *check.C) {
 	// but close enough for this test.
 	t.CheckDeepEquals(G.Todo.entries, []CurrPath{"."})
 
-	CheckdirCategory(".")
+	CheckdirCategory(".", true)
 
 	t.CheckOutputEmpty()
 	t.CheckDeepEquals(G.Todo.entries, []CurrPath{"./package", "."})
@@ -240,7 +240,7 @@ func (s *Suite) Test_CheckdirCategory__subdirs_file_system_at_the_bottom(c *chec
 		".include \"../mk/misc/category.mk\"")
 	t.FinishSetUp()
 
-	CheckdirCategory(t.File("category"))
+	CheckdirCategory(t.File("category"), false)
 
 	t.CheckOutputLines(
 		"ERROR: ~/category/Makefile:6: Package \"zzz-fs-only\" must be listed here.",
@@ -265,7 +265,7 @@ func (s *Suite) Test_CheckdirCategory__indentation(c *check.C) {
 		".include \"../mk/misc/category.mk\"")
 	t.FinishSetUp()
 
-	CheckdirCategory(t.File("category"))
+	CheckdirCategory(t.File("category"), false)
 
 	t.CheckOutputLines(
 		"NOTE: ~/category/Makefile:5: This variable value should be aligned " +
@@ -290,7 +290,7 @@ func (s *Suite) Test_CheckdirCategory__comment_at_the_top(c *check.C) {
 		".include \"../mk/misc/category.mk\"")
 	t.FinishSetUp()
 
-	CheckdirCategory(t.File("category"))
+	CheckdirCategory(t.File("category"), false)
 
 	// These are quite a few warnings and errors, just because there is
 	// an additional comment above the COMMENT definition.
@@ -321,7 +321,7 @@ func (s *Suite) Test_CheckdirCategory__unexpected_EOF_while_reading_SUBDIR(c *ch
 		"SUBDIR+=\tpackage")
 	t.FinishSetUp()
 
-	CheckdirCategory(t.File("category"))
+	CheckdirCategory(t.File("category"), false)
 
 	// Doesn't happen in practice since categories are created very seldom.
 	t.CheckOutputLines(
@@ -466,3 +466,48 @@ func (s *Suite) Test_CheckdirCategory__case_insensitive_file_system(c *check.C)
 		"ERROR: Makefile:7: On case-insensitive file systems, "+
 			"\"package\" is the same as \"PACKAGE\" from line 5.")
 }
+
+func (s *Suite) Test_CheckPackageDirCollision__main(c *check.C) {
+	t := s.Init(c)
+
+	// on case-insensitive filesystems, the packages 'Package' and 'package'
+	// overwrite 'PACKAGE'.
+	t.SetUpPackage("category/PACKAGE")
+	t.SetUpPackage("category/Package")
+	t.SetUpPackage("category/package")
+	t.CreateFileLines("mk/misc/category.mk")
+	t.Chdir("category/package")
+	t.FinishSetUp()
+
+	G.Check(".")
+
+	t.CheckOutputLines(
+		"ERROR: ../Makefile:5: On case-insensitive file systems, "+
+			"\"PACKAGE\" is the same as \"package\".",
+		"ERROR: ../Makefile:6: On case-insensitive file systems, "+
+			"\"Package\" is the same as \"package\".")
+}
+
+func (s *Suite) Test_CheckPackageDirCollision__wip(c *check.C) {
+	t := s.Init(c)
+
+	// on case-insensitive filesystems, the package 'Package' overwrites
+	// 'PACKAGE'.
+	t.SetUpPackage("category/PACKAGE")
+	t.SetUpPackage("category/Package")
+	t.SetUpPackage("wip/package",
+		"CATEGORIES=\tcategory")
+	t.CreateFileLines("mk/misc/category.mk")
+	t.Chdir("wip/package")
+	t.FinishSetUp()
+
+	G.Check(".")
+
+	t.CheckOutputLines(
+		"ERROR: ../../category/Makefile:5: "+
+			"On case-insensitive file systems, "+
+			"\"PACKAGE\" is the same as \"package\".",
+		"ERROR: ../../category/Makefile:6: "+
+			"On case-insensitive file systems, "+
+			"\"Package\" is the same as \"package\".")
+}
diff --git a/pkgtools/pkglint/files/check_test.go b/pkgtools/pkglint/files/check_test.go
index 32640b671c2..79ee97acad2 100644
--- a/pkgtools/pkglint/files/check_test.go
+++ b/pkgtools/pkglint/files/check_test.go
@@ -9,6 +9,7 @@ import (
 	"netbsd.org/pkglint/regex"
 	"os"
 	"regexp"
+	"sort"
 	"strconv"
 	"strings"
 	"testing"
@@ -413,16 +414,42 @@ func (t *Tester) SetUpPkgsrc() {
 	t.seenSetupPkgsrc++
 }
 
-// SetUpCategory makes the given category valid by creating a dummy Makefile.
+// SetUpCategory makes the given category valid by creating a realistic
+// Makefile.
 // After that, it can be mentioned in the CATEGORIES variable of a package.
-func (t *Tester) SetUpCategory(name RelPath) {
-	assert(G.Pkgsrc.Rel(t.File(name)).Count() == 1)
+func (t *Tester) SetUpCategory(name RelPath, pkgdir RelPath) {
+	categoryDir := G.Pkgsrc.Rel(t.File(name))
+	assert(categoryDir.Count() == 1)
+
+	categoryMakefile := name.JoinNoClean("Makefile")
+	text, err := t.File(categoryMakefile).ReadString()
+	oldLines := strings.Split(text, "\n")
+	if err != nil {
+		oldLines = []string{
+			MkCvsID,
+			"",
+			"COMMENT=\tComment for the category",
+			"",
+			// The SUBDIRs will be added here later.
+			"",
+			".include \"../mk/misc/category.mk\""}
+	} else {
+		oldLines = oldLines[:len(oldLines)-1]
+	}
 
-	makefile := name.JoinNoClean("Makefile")
-	if !t.File(makefile).IsFile() {
-		t.CreateFileLines(makefile,
-			MkCvsID)
+	subdirs := NewStringSet()
+	if pkgdir != "" {
+		subdirs.Add("SUBDIR+=\t" + pkgdir.String())
 	}
+	subdirs.AddAll(oldLines[4 : len(oldLines)-2])
+	sort.Strings(subdirs.Elements)
+
+	var newLines []string
+	newLines = append(newLines, oldLines[:4]...)
+	newLines = append(newLines, subdirs.Elements...)
+	newLines = append(newLines, oldLines[len(oldLines)-2:]...)
+
+	t.CreateFileLines(categoryMakefile, newLines...)
 }
 
 // SetUpPackage sets up all files for a package (including the pkgsrc
@@ -457,7 +484,7 @@ func (t *Tester) SetUpPackage(pkgpath RelPath, makefileLines ...string) CurrPath
 	}
 
 	t.SetUpPkgsrc()
-	t.SetUpCategory(category)
+	t.SetUpCategory(category, distname)
 
 	t.CreateFileLines(pkgpath.JoinNoClean("DESCR"),
 		"Package description")
diff --git a/pkgtools/pkglint/files/mkassignchecker_test.go b/pkgtools/pkglint/files/mkassignchecker_test.go
index 9a8a9278eec..825d5aa761c 100644
--- a/pkgtools/pkglint/files/mkassignchecker_test.go
+++ b/pkgtools/pkglint/files/mkassignchecker_test.go
@@ -858,7 +858,7 @@ func (s *Suite) Test_MkAssignChecker_checkRightCategory__regress(c *check.C) {
 		"CATEGORIES=\tregress")
 	t.SetUpPackage("regress/misc-package",
 		"CATEGORIES=\tmisc")
-	t.SetUpCategory("misc")
+	t.SetUpCategory("misc", "")
 	t.FinishSetUp()
 	t.Chdir(".")
 
diff --git a/pkgtools/pkglint/files/pkglint.go b/pkgtools/pkglint/files/pkglint.go
index 0e093f43ed8..689668e4e39 100644
--- a/pkgtools/pkglint/files/pkglint.go
+++ b/pkgtools/pkglint/files/pkglint.go
@@ -334,7 +334,7 @@ func (p *Pkglint) checkMode(dirent CurrPath, mode os.FileMode) {
 	case "../..":
 		p.checkdirPackage(dir)
 	case "..":
-		CheckdirCategory(dir)
+		CheckdirCategory(dir, G.Recursive)
 	case ".":
 		CheckdirToplevel(dir)
 	default:
@@ -351,6 +351,36 @@ func (p *Pkglint) checkdirPackage(dir CurrPath) {
 
 	pkg := NewPackage(dir)
 	pkg.Check()
+
+	pkgBasedir := p.Abs(dir).Base()
+	CheckPackageDirCollision(pkg.File(".."), pkgBasedir)
+	p.checkWipPackageDirCollision(pkg, pkgBasedir)
+}
+
+// checkWipPackageDirCollision checks that the package directory of a
+// pkgsrc-wip package doesn't create a collision on a case-insensitive
+// file system when it will be imported into main pkgsrc.
+func (p *Pkglint) checkWipPackageDirCollision(pkg *Package, pkgBasedir RelPath) {
+	if !p.Wip {
+		return
+	}
+	categories := pkg.vars.FirstDefinition("CATEGORIES")
+	if categories == nil {
+		return
+	}
+	fields := categories.Fields()
+	if len(fields) == 0 {
+		return
+	}
+	path := NewPath(fields[0])
+	if path.IsAbs() {
+		return
+	}
+	categoryDir := pkg.File("../..").JoinClean(NewRelPath(path))
+	if !categoryDir.IsDir() {
+		return
+	}
+	CheckPackageDirCollision(categoryDir, pkgBasedir)
 }
 
 // Returns the pkgsrc top-level directory, relative to the given directory.
diff --git a/pkgtools/pkglint/files/redundantscope_test.go b/pkgtools/pkglint/files/redundantscope_test.go
index b9a0c815d12..10d27e7f481 100644
--- a/pkgtools/pkglint/files/redundantscope_test.go
+++ b/pkgtools/pkglint/files/redundantscope_test.go
@@ -1716,7 +1716,7 @@ func (s *Suite) Test_RedundantScope_checkAppendUnique__eval_assignment(c *check.
 func (s *Suite) Test_RedundantScope_checkAppendUnique__not_redundant(c *check.C) {
 	t := s.Init(c)
 
-	t.SetUpCategory("perl")
+	t.SetUpCategory("perl", "")
 	t.SetUpPackage("category/package",
 		"CATEGORIES:=\tcategory",
 		".include \"included1.mk\"",
diff --git a/pkgtools/pkglint/files/util.go b/pkgtools/pkglint/files/util.go
index ca24e48f3c3..1cbf90852aa 100644
--- a/pkgtools/pkglint/files/util.go
+++ b/pkgtools/pkglint/files/util.go
@@ -105,25 +105,25 @@ func invalidCharacters(s string, valid *textproc.ByteSet) string {
 	var unis strings.Builder
 
 	for _, r := range s {
-		switch {
-		case r == rune(byte(r)) && valid.Contains(byte(r)):
+		if r == rune(byte(r)) && valid.Contains(byte(r)) {
 			continue
-		case '!' <= r && r <= '~':
+		}
+		if unis.Len() > 0 {
 			unis.WriteByte(' ')
+		}
+		switch {
+		case '!' <= r && r <= '~':
 			unis.WriteByte(byte(r))
 		case r == ' ':
-			unis.WriteString(" space")
+			unis.WriteString("space")
 		case r == '\t':
-			unis.WriteString(" tab")
+			unis.WriteString("tab")
 		default:
-			_, _ = fmt.Fprintf(&unis, " %U", r)
+			_, _ = fmt.Fprintf(&unis, "%U", r)
 		}
 	}
 
-	if unis.Len() == 0 {
-		return ""
-	}
-	return unis.String()[1:]
+	return unis.String()
 }
 
 // intern returns an independent copy of the given string.