From 486dfa6de7f99794eead938495a83f0c194cd0e0 Mon Sep 17 00:00:00 2001
From: Roland Illig <roland.illig@gmx.de>
Date: Fri, 19 Jan 2024 18:32:44 +0100
Subject: [PATCH] Warn about duplicate DESCR files

Suggested by gdt@.

https://mail-index.netbsd.org/pkgsrc-users/2024/01/19/msg038803.html
---
 v23/distinfo_test.go |  5 ++++-
 v23/pkglint.go       | 35 ++++++++++++++++++++++++++++++++++-
 v23/pkglint_test.go  |  2 ++
 v23/pkgsrc_test.go   |  4 +++-
 4 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/v23/distinfo_test.go b/v23/distinfo_test.go
index 09435c89..014d635a 100644
--- a/v23/distinfo_test.go
+++ b/v23/distinfo_test.go
@@ -721,6 +721,9 @@ func (s *Suite) Test_distinfoLinesChecker_checkGlobalDistfileMismatch(c *check.C
 		"ERROR: ~/category/package1/distinfo:5: "+
 			"Expected BLAKE2s, SHA512, Size checksums for \"patch-4.2.tar.gz\", got SHA512.",
 
+		"WARN: ~/category/package2/DESCR: DESCR file is the same "+
+			"as \"../../category/package1/DESCR\".",
+
 		"ERROR: ~/category/package2/distinfo:3: "+
 			"Expected BLAKE2s, SHA512, Size checksums for \"distfile-1.0.tar.gz\", got SHA512.",
 		"ERROR: ~/category/package2/distinfo:3: "+
@@ -734,7 +737,7 @@ func (s *Suite) Test_distinfoLinesChecker_checkGlobalDistfileMismatch(c *check.C
 			"The SHA512 hash for encoding-error.tar.gz contains a non-hex character.",
 
 		"WARN: ~/licenses/gnu-gpl-v2: This license seems to be unused.",
-		"8 errors and 1 warning found.",
+		"8 errors and 2 warnings found.",
 		t.Shquote("(Run \"pkglint -e -r -Wall -Call %s\" to show explanations.)", "."))
 
 	// Ensure that hex.DecodeString does not waste memory here.
diff --git a/v23/pkglint.go b/v23/pkglint.go
index 7f20b5df..debce11c 100644
--- a/v23/pkglint.go
+++ b/v23/pkglint.go
@@ -1,6 +1,7 @@
 package pkglint
 
 import (
+	"crypto/sha1"
 	"fmt"
 	"github.com/rillig/pkglint/v23/getopt"
 	"github.com/rillig/pkglint/v23/histogram"
@@ -635,6 +636,7 @@ func (p *Pkglint) checkReg(filename CurrPath, basename RelPath, depth int, pkg *
 	case basename.HasPrefixText("DESCR"):
 		if lines := Load(filename, NotEmpty|LogErrors); lines != nil {
 			CheckLinesDescr(lines)
+			G.InterPackage.CheckDuplicateDescr(filename)
 		}
 
 	case basename == "distinfo":
@@ -864,17 +866,21 @@ func (p *Pkglint) Abs(filename CurrPath) CurrPath {
 	return filename.Clean()
 }
 
+// InterPackage collects data from the inter-package analysis.
+// It is most useful when running pkglint on a complete pkgsrc installation.
 type InterPackage struct {
 	hashes       map[string]*Hash    // Maps "alg:filename" => hash (inter-package check).
 	usedLicenses map[string]struct{} // Maps "license name" => true (inter-package check).
 	bl3Names     map[string]Location // Maps buildlink3 identifiers to their first occurrence.
+	descr        map[[sha1.Size]byte][]CurrPath
 }
 
 func (ip *InterPackage) Enable() {
 	*ip = InterPackage{
 		make(map[string]*Hash),
 		make(map[string]struct{}),
-		make(map[string]Location)}
+		make(map[string]Location),
+		make(map[[sha1.Size]byte][]CurrPath)}
 
 	// This is the only license that is added by an infrastructure file,
 	// mk/djbware.mk. The correct way to handle this situation would be
@@ -921,3 +927,30 @@ func (ip *InterPackage) Bl3(name string, loc *Location) *Location {
 	ip.bl3Names[name] = *loc
 	return nil
 }
+
+func (ip *InterPackage) CheckDuplicateDescr(filename CurrPath) {
+	descr := ip.descr
+	if descr == nil {
+		return
+	}
+	b, err := os.ReadFile(filename.String())
+	if err != nil {
+		return
+	}
+	h := sha1.Sum(b)
+	existing := descr[h]
+	descr[h] = append(existing, filename)
+	if len(existing) == 0 {
+		return
+	}
+	line := NewLineWhole(filename)
+	line.Warnf("DESCR file is the same as %q.",
+		line.Rel(existing[len(existing)-1]))
+	line.Explain(
+		"Each DESCR file should be unique,",
+		"to help the user choose the most appropriate package.",
+		"",
+		"If two packages really need the exact same DESCR file,",
+		"create a single DESCR file for both",
+		"and refer to it via DESCR_SRC.")
+}
diff --git a/v23/pkglint_test.go b/v23/pkglint_test.go
index 091b8ec4..46bda49f 100644
--- a/v23/pkglint_test.go
+++ b/v23/pkglint_test.go
@@ -1511,6 +1511,8 @@ func (s *Suite) Test_InterPackage_Bl3__same_identifier(c *check.C) {
 			"can be replaced with the simpler \"S,^,,\".",
 		"NOTE: category/package1/Makefile:4: The modifier \"@v@${v}@\" "+
 			"can be replaced with the simpler \"=\".",
+		"WARN: category/package2/DESCR: DESCR file is "+
+			"the same as \"../../category/package1/DESCR\".",
 		"NOTE: category/package2/Makefile:4: The modifier \"@v@${v}@\" "+
 			"can be replaced with the simpler \"S,^,,\".",
 		"NOTE: category/package2/Makefile:4: The modifier \"@v@${v}@\" "+
diff --git a/v23/pkgsrc_test.go b/v23/pkgsrc_test.go
index 5a0b5be0..286cfe7f 100644
--- a/v23/pkgsrc_test.go
+++ b/v23/pkgsrc_test.go
@@ -834,10 +834,12 @@ func (s *Suite) Test_Pkgsrc_checkToplevelUnusedLicenses(c *check.C) {
 	t.Main("-r", "-Cglobal", ".")
 
 	t.CheckOutputLines(
+		"WARN: ~/category/package2/DESCR: DESCR file is the same "+
+			"as \"../../category/package/DESCR\".",
 		"ERROR: ~/category/package2/Makefile:11: License file ../../licenses/missing does not exist.",
 		"WARN: ~/licenses/gnu-gpl-v2: This license seems to be unused.", // Added by Tester.SetUpPkgsrc
 		"WARN: ~/licenses/gnu-gpl-v3: This license seems to be unused.",
-		"1 error and 2 warnings found.",
+		"1 error and 3 warnings found.",
 		t.Shquote("(Run \"pkglint -e -r -Cglobal %s\" to show explanations.)", "."))
 }