pkgtools/pkglint: update to 20.1.4

Changes since 20.1.3:

For patches that patch a single file, the filename of the patch should
correspond to the patched file. There are a few different naming schemes
in action, therefore the check is relatively loose. Patches that are
called patch-[a-z][a-z] continue to be allowed for historic reasons.
Patches that are called patch-CVE-* are also allowed.

The entries in doc/CHANGES-* are checked for consistency. For example,
it doesn't make sense to add a package twice or "update" a package from
version 1.0 to version 1.0. All version numbers in these entries must
be valid pkgsrc versions, i.e. start with a digit and only use
characters from -.0-9A-Z_a-z.

14 files changed, 318 insertions, 104 deletions

diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile
index 3bea1707805..ad7b4e4cab3 100644
--- a/pkgtools/pkglint/Makefile
+++ b/pkgtools/pkglint/Makefile
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.642 2020/04/30 21:15:03 rillig Exp $
+# $NetBSD: Makefile,v 1.643 2020/05/08 19:50:04 rillig Exp $
 
-PKGNAME=	pkglint-20.1.3
+PKGNAME=	pkglint-20.1.4
 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 9b63c6eb7f6..761107bc27e 100644
--- a/pkgtools/pkglint/files/buildlink3.go
+++ b/pkgtools/pkglint/files/buildlink3.go
@@ -330,8 +330,6 @@ type Buildlink3Data struct {
 type Buildlink3ID string
 
 func LoadBuildlink3Data(mklines *MkLines) *Buildlink3Data {
-	assert(mklines.lines.BaseName == "buildlink3.mk")
-
 	var data Buildlink3Data
 	mklines.ForEach(func(mkline *MkLine) {
 		if mkline.IsVarassign() {
diff --git a/pkgtools/pkglint/files/check_test.go b/pkgtools/pkglint/files/check_test.go
index 66e78929c48..2f54dd29ead 100644
--- a/pkgtools/pkglint/files/check_test.go
+++ b/pkgtools/pkglint/files/check_test.go
@@ -568,13 +568,15 @@ func (t *Tester) CreateFileDummyPatch(filename RelPath) {
 	// Patch files only make sense in category/package/patches directories.
 	assert(G.Pkgsrc.Rel(t.File(filename)).Count() == 4)
 
+	patchedFile := replaceAll(filename.String(), `.*?\bpatches/patch-`, "")
+
 	t.CreateFileLines(filename,
 		CvsID,
 		"",
 		"Documentation",
 		"",
 		"--- oldfile",
-		"+++ newfile",
+		"+++ "+patchedFile,
 		"@@ -1 +1 @@",
 		"-old",
 		"+new")
diff --git a/pkgtools/pkglint/files/distinfo_test.go b/pkgtools/pkglint/files/distinfo_test.go
index c4825bd5f26..dc28b34ce62 100644
--- a/pkgtools/pkglint/files/distinfo_test.go
+++ b/pkgtools/pkglint/files/distinfo_test.go
@@ -113,7 +113,7 @@ func (s *Suite) Test_distinfoLinesChecker_check__distinfo_and_patches_in_separat
 
 	t.CheckOutputLines(
 		"ERROR: ../../other/common/distinfo:3: SHA1 hash of patches/patch-aa differs "+
-			"(distinfo has ..., patch file has ebbf34b0641bcb508f17d5a27f2bf2a536d810ac).",
+			"(distinfo has ..., patch file has 9a93207561abfef7e7550598c5a08f2c3226995b).",
 		"WARN: ../../other/common/distinfo:4: Patch file \"patch-only-in-distinfo\" "+
 			"does not exist in directory \"patches\".",
 		"ERROR: ../../other/common/distinfo: Patch \"patches/patch-only-in-patches\" "+
@@ -176,7 +176,7 @@ func (s *Suite) Test_distinfoLinesChecker_check__missing_php_patches(c *check.C)
 	t.CreateFileLines("lang/php72/distinfo",
 		CvsID,
 		"",
-		"SHA1 (patch-php72) = ebbf34b0641bcb508f17d5a27f2bf2a536d810ac")
+		"SHA1 (patch-php72) = 9bd4352244636c587db21abfbb99bb34ded1e333")
 
 	t.CreateFileLines("archivers/php-bz2/Makefile",
 		MkCvsID,
@@ -324,7 +324,7 @@ func (s *Suite) Test_distinfoLinesChecker_checkAlgorithms__wrong_patch_algorithm
 		"ERROR: distinfo:3: Expected SHA1 hash for patch-aa, got MD5, SHA1.",
 		"ERROR: distinfo:4: SHA1 hash of patches/patch-aa differs "+
 			"(distinfo has 1234567890123456789012345678901234567890, "+
-			"patch file has ebbf34b0641bcb508f17d5a27f2bf2a536d810ac).")
+			"patch file has 9a93207561abfef7e7550598c5a08f2c3226995b).")
 }
 
 func (s *Suite) Test_distinfoLinesChecker_checkAlgorithms__missing_patch_with_distfile_checksums(c *check.C) {
@@ -375,7 +375,7 @@ func (s *Suite) Test_distinfoLinesChecker_checkAlgorithms__existing_patch_with_d
 			"Expected SHA1 hash for patch-aa, got SHA1, RMD160, SHA512, Size.",
 		"ERROR: ~/category/package/distinfo:3: "+
 			"SHA1 hash of patches/patch-aa differs (distinfo has ..., "+
-			"patch file has ebbf34b0641bcb508f17d5a27f2bf2a536d810ac).")
+			"patch file has 9a93207561abfef7e7550598c5a08f2c3226995b).")
 }
 
 func (s *Suite) Test_distinfoLinesChecker_checkAlgorithms__missing_patch_with_wrong_algorithms(c *check.C) {
@@ -775,7 +775,7 @@ func (s *Suite) Test_distinfoLinesChecker_checkUncommittedPatch__bad(c *check.C)
 	t.SetUpFileLines("distinfo",
 		CvsID,
 		"",
-		"SHA1 (patch-aa) = ebbf34b0641bcb508f17d5a27f2bf2a536d810ac")
+		"SHA1 (patch-aa) = 9a93207561abfef7e7550598c5a08f2c3226995b")
 	t.FinishSetUp()
 
 	G.checkdirPackage(".")
@@ -797,7 +797,7 @@ func (s *Suite) Test_distinfoLinesChecker_checkUncommittedPatch__good(c *check.C
 	t.SetUpFileLines("distinfo",
 		CvsID,
 		"",
-		"SHA1 (patch-aa) = ebbf34b0641bcb508f17d5a27f2bf2a536d810ac")
+		"SHA1 (patch-aa) = 9a93207561abfef7e7550598c5a08f2c3226995b")
 	t.FinishSetUp()
 
 	G.checkdirPackage(".")
@@ -830,7 +830,7 @@ func (s *Suite) Test_distinfoLinesChecker_checkPatchSha1__relative_path_in_disti
 
 	t.CheckOutputLines(
 		"ERROR: ../../other/common/distinfo:3: SHA1 hash of ../../devel/patches/patches/patch-aa differs "+
-			"(distinfo has ..., patch file has ebbf34b0641bcb508f17d5a27f2bf2a536d810ac).",
+			"(distinfo has ..., patch file has 9a93207561abfef7e7550598c5a08f2c3226995b).",
 		"WARN: ../../other/common/distinfo:4: Patch file \"patch-only-in-distinfo\" "+
 			"does not exist in directory \"../../devel/patches/patches\".",
 		"ERROR: ../../other/common/distinfo: Patch \"../../devel/patches/patches/patch-only-in-patches\" "+
diff --git a/pkgtools/pkglint/files/mkline.go b/pkgtools/pkglint/files/mkline.go
index 4588fa49270..2e024d0566f 100644
--- a/pkgtools/pkglint/files/mkline.go
+++ b/pkgtools/pkglint/files/mkline.go
@@ -293,14 +293,15 @@ func (mkline *MkLine) Args() string { return mkline.data.(*mkLineDirective).args
 func (mkline *MkLine) Cond() *MkCond {
 	cond := mkline.data.(*mkLineDirective).cond
 	if cond == nil {
-		assert(mkline.HasCond())
+		assert(mkline.NeedsCond())
 		cond = NewMkParser(mkline.Line, mkline.Args()).MkCond()
 		mkline.data.(*mkLineDirective).cond = cond
 	}
 	return cond
 }
 
-func (mkline *MkLine) HasCond() bool {
+// NeedsCond returns whether the directive requires a condition as argument.
+func (mkline *MkLine) NeedsCond() bool {
 	directive := mkline.Directive()
 	return directive == "if" || directive == "elif"
 }
diff --git a/pkgtools/pkglint/files/package.go b/pkgtools/pkglint/files/package.go
index cbd18d941b9..30effb85120 100644
--- a/pkgtools/pkglint/files/package.go
+++ b/pkgtools/pkglint/files/package.go
@@ -8,7 +8,8 @@ import (
 )
 
 // TODO: What about package names that refer to other variables?
-const rePkgname = `^([\w\-.+]+)-(\d[.0-9A-Z_a-z]*)$`
+// TODO: Allow a hyphen in the middle of a version number.
+const rePkgname = `^([\w\-.+]+)-([0-9][.0-9A-Z_a-z]*)$`
 
 // Package is the pkgsrc package that is currently checked.
 //
diff --git a/pkgtools/pkglint/files/package_test.go b/pkgtools/pkglint/files/package_test.go
index b772c1049d8..0ed424afc13 100644
--- a/pkgtools/pkglint/files/package_test.go
+++ b/pkgtools/pkglint/files/package_test.go
@@ -143,7 +143,7 @@ func (s *Suite) Test_Package__using_common_Makefile_overriding_DISTINFO_FILE(c *
 	t.CreateFileLines("security/pinentry-fltk/distinfo",
 		CvsID,
 		"",
-		"SHA1 (patch-aa) = ebbf34b0641bcb508f17d5a27f2bf2a536d810ac")
+		"SHA1 (patch-aa) = 9a93207561abfef7e7550598c5a08f2c3226995b")
 	t.FinishSetUp()
 
 	G.Check(t.File("security/pinentry"))
@@ -1531,7 +1531,7 @@ func (s *Suite) Test_Package_checkfilePackageMakefile__META_PACKAGE_with_patch(c
 	t.CreateFileLines("category/package/distinfo",
 		CvsID,
 		"",
-		"SHA1 (patch-aa) = ebbf34b0641bcb508f17d5a27f2bf2a536d810ac")
+		"SHA1 (patch-aa) = 9a93207561abfef7e7550598c5a08f2c3226995b")
 
 	t.FinishSetUp()
 
@@ -3105,7 +3105,7 @@ func (s *Suite) Test_Package_checkOwnerMaintainer__directory(c *check.C) {
 	t.CreateFileLines("category/package/distinfo",
 		CvsID,
 		"",
-		"SHA1 (patch-aa) = ebbf34b0641bcb508f17d5a27f2bf2a536d810ac")
+		"SHA1 (patch-aa) = 9a93207561abfef7e7550598c5a08f2c3226995b")
 	t.FinishSetUp()
 
 	G.Check(pkg)
diff --git a/pkgtools/pkglint/files/patches.go b/pkgtools/pkglint/files/patches.go
index 34147872aaa..736f22d9c76 100644
--- a/pkgtools/pkglint/files/patches.go
+++ b/pkgtools/pkglint/files/patches.go
@@ -32,14 +32,15 @@ func (ck *PatchChecker) Check(pkg *Package) {
 
 	ck.previousLineEmpty = ck.llex.SkipEmptyOrNote()
 
-	patchedFiles := 0
+	var patchedFiles []Path
 	for !ck.llex.EOF() {
 		line := ck.llex.CurrentLine()
 		if ck.llex.SkipRegexp(rePatchUniFileDel) {
 			if m := ck.llex.NextRegexp(rePatchUniFileAdd); m != nil {
-				ck.checkBeginDiff(line, patchedFiles)
-				ck.checkUnifiedDiff(NewPath(m[1]))
-				patchedFiles++
+				patchedFile := NewPath(m[1])
+				ck.checkBeginDiff(line, len(patchedFiles))
+				ck.checkUnifiedDiff(patchedFile)
+				patchedFiles = append(patchedFiles, patchedFile)
 				continue
 			}
 
@@ -49,10 +50,10 @@ func (ck *PatchChecker) Check(pkg *Package) {
 		if m := ck.llex.NextRegexp(rePatchUniFileAdd); m != nil {
 			patchedFile := NewPath(m[1])
 			if ck.llex.SkipRegexp(rePatchUniFileDel) {
-				ck.checkBeginDiff(line, patchedFiles)
+				ck.checkBeginDiff(line, len(patchedFiles))
 				ck.llex.PreviousLine().Warnf("Unified diff headers should be first ---, then +++.")
 				ck.checkUnifiedDiff(patchedFile)
-				patchedFiles++
+				patchedFiles = append(patchedFiles, patchedFile)
 				continue
 			}
 
@@ -61,7 +62,7 @@ func (ck *PatchChecker) Check(pkg *Package) {
 
 		if ck.llex.SkipRegexp(`^\*\*\*[\t ]([^\t ]+)(.*)$`) {
 			if ck.llex.SkipRegexp(`^---[\t ]([^\t ]+)(.*)$`) {
-				ck.checkBeginDiff(line, patchedFiles)
+				ck.checkBeginDiff(line, len(patchedFiles))
 				line.Warnf("Please use unified diffs (diff -u) for patches.")
 				return
 			}
@@ -76,11 +77,16 @@ func (ck *PatchChecker) Check(pkg *Package) {
 		}
 	}
 
-	if patchedFiles > 1 && !matches(ck.lines.Filename.String(), `\bCVE\b`) {
-		ck.lines.Whole().Warnf("Contains patches for %d files, should be only one.", patchedFiles)
-	} else if patchedFiles == 0 {
+	nPatched := len(patchedFiles)
+	if nPatched > 1 && !matches(ck.lines.Filename.String(), `\bCVE\b`) {
+		ck.lines.Whole().Warnf("Contains patches for %d files, should be only one.", nPatched)
+	}
+	if nPatched == 0 {
 		ck.lines.Whole().Errorf("Contains no patch.")
 	}
+	if len(patchedFiles) == 1 {
+		ck.checkCanonicalPatchName(patchedFiles[0])
+	}
 
 	CheckLinesTrailingEmptyLines(ck.lines)
 	sha1Before := computePatchSha1Hex(ck.lines)
@@ -339,10 +345,46 @@ func (ck *PatchChecker) checktextCvsID(text string) {
 	}
 }
 
+func (ck *PatchChecker) checkCanonicalPatchName(patched Path) {
+	patch := ck.lines.BaseName
+	if matches(patch, `^patch-[a-z][a-z]$`) {
+		// This naming scheme is only accepted for historic reasons.
+		// It has has absolutely no benefit.
+		return
+	}
+	if matches(patch, `^patch-[A-Z]+-[0-9]+`) {
+		return
+	}
+
+	// The patch name only needs to correspond very roughly to the patched file.
+	// There are varying schemes in use that transform a filename to a patch name.
+	normalize := func(s string) string {
+		return strings.ToLower(replaceAll(s, `[^A-Za-z0-9]+`, "*"))
+	}
+
+	patchedNorm := normalize(patched.Clean().String())
+	patchNorm := normalize(strings.TrimPrefix(patch, "patch-"))
+	if patchNorm == patchedNorm {
+		return
+	}
+	if hasSuffix(patchedNorm, patchNorm) && patchNorm == normalize(patched.Base()) {
+		return
+	}
+
+	// See pkgtools/pkgdiff/files/mkpatches, function patch_name.
+	canon1 := replaceAll(patched.Clean().String(), `_`, "__")
+	canon2 := replaceAll(canon1, `[/\s]`, "_")
+	canonicalName := "patch-" + canon2
+
+	ck.lines.Whole().Warnf(
+		"The patch file should be named %q to match the patched file %q.",
+		canonicalName, patched.String())
+}
+
 // isEmptyLine tests whether a line provides essentially no interesting content.
 // The focus here is on human-generated content that is intended for other human readers.
 // Therefore text that is typical for patch generators is considered empty as well.
-func (ck *PatchChecker) isEmptyLine(text string) bool {
+func (*PatchChecker) isEmptyLine(text string) bool {
 	return text == "" ||
 		hasPrefix(text, "index ") ||
 		hasPrefix(text, "Index: ") ||
diff --git a/pkgtools/pkglint/files/patches_test.go b/pkgtools/pkglint/files/patches_test.go
index 542338e73a4..6be95daba67 100644
--- a/pkgtools/pkglint/files/patches_test.go
+++ b/pkgtools/pkglint/files/patches_test.go
@@ -15,8 +15,8 @@ func (s *Suite) Test_CheckLinesPatch__with_comment(c *check.C) {
 		"* either why it is specific to pkgsrc",
 		"* or where it has been reported upstream",
 		"",
-		"--- file.orig",
-		"+++ file",
+		"--- WithComment.orig",
+		"+++ WithComment",
 		"@@ -5,3 +5,3 @@",
 		" context before",
 		"-old line",
@@ -84,8 +84,8 @@ func (s *Suite) Test_CheckLinesPatch__no_comment_and_no_empty_lines(c *check.C)
 
 	patchLines := t.SetUpFileLines("patch-WithoutEmptyLines",
 		CvsID,
-		"--- file.orig",
-		"+++ file",
+		"--- WithoutEmptyLines.orig",
+		"+++ WithoutEmptyLines",
 		"@@ -1,1 +1,1 @@",
 		"-old line",
 		"+new line")
@@ -109,8 +109,8 @@ func (s *Suite) Test_CheckLinesPatch__without_comment(c *check.C) {
 	lines := t.NewLines("patch-WithoutComment",
 		CvsID,
 		"",
-		"--- file.orig",
-		"+++ file",
+		"--- WithoutComment.orig",
+		"+++ WithoutComment",
 		"@@ -5,3 +5,3 @@",
 		" context before",
 		"-old line",
@@ -134,8 +134,8 @@ func (s *Suite) Test_CheckLinesPatch__error_code(c *check.C) {
 		"",
 		"*** Error code 1", // Looks like a context diff but isn't.
 		"",
-		"--- file.orig",
-		"+++ file",
+		"--- ErrorCode.orig",
+		"+++ ErrorCode",
 		"@@ -5,3 +5,3 @@",
 		" context before",
 		"-old line",
@@ -156,8 +156,8 @@ func (s *Suite) Test_CheckLinesPatch__wrong_header_order(c *check.C) {
 		"Text",
 		"Text",
 		"",
-		"+++ file",      // Wrong order
-		"--- file.orig", // Wrong order
+		"+++ WrongOrder",      // Wrong order
+		"--- WrongOrder.orig", // Wrong order
 		"@@ -5,3 +5,3 @@",
 		" context before",
 		"-old line",
@@ -282,13 +282,13 @@ func (s *Suite) Test_CheckLinesPatch__only_unified_header_but_no_content(c *chec
 		"",
 		"Documentation for the patch",
 		"",
-		"--- file.orig",
-		"+++ file")
+		"--- unified.orig",
+		"+++ unified")
 
 	CheckLinesPatch(lines, nil)
 
 	t.CheckOutputLines(
-		"ERROR: patch-unified:EOF: No patch hunks for \"file\".")
+		"ERROR: patch-unified:EOF: No patch hunks for \"unified\".")
 }
 
 func (s *Suite) Test_CheckLinesPatch__only_context_header_but_no_content(c *check.C) {
@@ -763,8 +763,8 @@ func (s *Suite) Test_PatchChecker_checkAddedAbsPath(c *check.C) {
 			"",
 			"Demonstrates absolute paths.",
 			"",
-			"--- before",
-			"+++ after",
+			"--- file.orig",
+			"+++ file",
 			"@@ -1,0 +1,1 @@",
 			"+"+addedLine)
 
@@ -940,6 +940,90 @@ func (s *Suite) Test_PatchChecker_checktextCvsID(c *check.C) {
 		"WARN: ~/patch-aa:11: Found CVS tag \"$"+"Author$\". Please remove it by reducing the number of context lines using pkgdiff or \"diff -U[210]\".")
 }
 
+func (s *Suite) Test_PatchChecker_checkCanonicalPatchName(c *check.C) {
+	t := s.Init(c)
+
+	test := func(patchName CurrPath, patchedFile Path, diagnostics ...string) {
+		ck := PatchChecker{lines: t.NewLines(patchName)}
+
+		ck.checkCanonicalPatchName(patchedFile)
+
+		t.CheckOutput(diagnostics)
+	}
+
+	test(
+		"patch-aa",
+		"any-file.c",
+		nil...)
+
+	test(
+		"patch-src_main.c",
+		"src/main.c",
+		nil...)
+
+	// By converting the ".c" to "_c", file managers that only inspect
+	// the file extension don't get confused.
+	test(
+		"patch-src_main_c",
+		"src/main.c",
+		nil...)
+
+	// CVE patches may patch anything.
+	// They may even patch more than one file.
+	// Having the source clearly named in the patch file is more important
+	// than having a patch name that corresponds to the patched file.
+	test(
+		"patch-CVE-2020-0001",
+		"src/anything.c",
+		nil...)
+
+	// Same for Xen Security Advisories.
+	test(
+		"patch-XSA-0001",
+		"src/anything.c",
+		nil...)
+
+	test(
+		"patch-file_underscore.py",
+		"file_underscore.py",
+		nil...)
+
+	test(
+		"patch-one.py",
+		"two.py",
+		"WARN: patch-one.py: The patch file should be named \"patch-two.py\" "+
+			"to match the patched file \"two.py\".")
+
+	// Don't suggest patch-._file as the patch name since that is unusual.
+	test(
+		"patch-file",
+		"./file",
+		nil...)
+
+	// This is usually ok, assuming that the same file does not occur
+	// in other directories as well.
+	test(
+		"patch-file",
+		"./src/subdir/file",
+		nil...)
+
+	// It's not enough if the patch name is an arbitrary suffix of the
+	// patched file. The only allowed abbreviation is the basename.
+	test(
+		"patch-c",
+		"./src/subdir/file.c",
+		"WARN: patch-c: The patch file should be named \"patch-src_subdir_file.c\" "+
+			"to match the patched file \"./src/subdir/file.c\".")
+
+	// Allow existing patches to differ in case.
+	// Most packages won't have files that could conflict on a
+	// case-insensitive filesystem anyway.
+	test(
+		"patch-upper",
+		"./src/UPPER",
+		nil...)
+}
+
 // Autogenerated "comments" from Git or other tools don't count as real
 // comments since they don't convey any intention of a human developer.
 func (s *Suite) Test_PatchChecker_isEmptyLine(c *check.C) {
diff --git a/pkgtools/pkglint/files/pkglint_test.go b/pkgtools/pkglint/files/pkglint_test.go
index 4b26de215cd..9e6d588c42d 100644
--- a/pkgtools/pkglint/files/pkglint_test.go
+++ b/pkgtools/pkglint/files/pkglint_test.go
@@ -215,8 +215,8 @@ func (s *Suite) Test_Pkglint_Main__complete_package(c *check.C) {
 		"removed lines. The hunk headers says that one line is to be",
 		"removed, but in fact, there is no deletion line below it.",
 		"",
-		"--- a/checkperms.c",
-		"+++ b/checkperms.c",
+		"--- checkperms.c",
+		"+++ checkperms.c",
 		"@@ -1,1 +1,3 @@", // at line 1, delete 1 line; at line 1, add 3 lines
 		"+// Header 1",
 		"+// Header 2",
@@ -243,7 +243,7 @@ func (s *Suite) Test_Pkglint_Main__complete_package(c *check.C) {
 		"ERROR: ~/sysutils/checkperms/Makefile:4: Invalid category \"tools\".",
 		"ERROR: ~/sysutils/checkperms/TODO: Packages in main pkgsrc must not have a TODO file.",
 		"ERROR: ~/sysutils/checkperms/distinfo:7: SHA1 hash of patches/patch-checkperms.c differs "+
-			"(distinfo has asdfasdf, patch file has e775969de639ec703866c0336c4c8e0fdd96309c).",
+			"(distinfo has asdfasdf, patch file has bcfb79696cb6bf4d2222a6d78a530e11bf1c0cea).",
 		"WARN: ~/sysutils/checkperms/patches/patch-checkperms.c:12: Premature end of patch hunk "+
 			"(expected 1 lines to be deleted and 0 lines to be added).",
 		"3 errors, 2 warnings and 1 note found.",
@@ -1069,7 +1069,7 @@ func (s *Suite) Test_Pkglint_checkReg__readme_and_todo(c *check.C) {
 	t.CreateFileLines("category/package/distinfo",
 		CvsID,
 		"",
-		"SHA1 (patch-README) = ebbf34b0641bcb508f17d5a27f2bf2a536d810ac")
+		"SHA1 (patch-README) = 87686be2a11c9d610ff09e029db92efddd96e4f9")
 
 	// Copy category/package/** to wip/package.
 	// TODO: Extract into Tester.CopyAll.
@@ -1204,7 +1204,7 @@ func (s *Suite) Test_Pkglint_checkRegCvsSubst__full_package(c *check.C) {
 	t.CreateFileLines("distinfo",
 		CvsID,
 		"",
-		"SHA1 (patch-any) = ebbf34b0641bcb508f17d5a27f2bf2a536d810ac")
+		"SHA1 (patch-any) = c3bc5923e225e5eafb8bb1f55e2142317c19800c")
 	t.CreateFileLines("CVS/Entries",
 		"/Makefile/1.1/modified/-ko/")
 	t.CreateFileLines("patches/CVS/Entries",
diff --git a/pkgtools/pkglint/files/pkgsrc.go b/pkgtools/pkglint/files/pkgsrc.go
index 6bbb281a4f0..2f2040dc18a 100644
--- a/pkgtools/pkglint/files/pkgsrc.go
+++ b/pkgtools/pkglint/files/pkgsrc.go
@@ -1,6 +1,7 @@
 package pkglint
 
 import (
+	"netbsd.org/pkglint/pkgver"
 	"netbsd.org/pkglint/regex"
 	"netbsd.org/pkglint/textproc"
 	"os"
@@ -189,6 +190,8 @@ func (src *Pkgsrc) loadDocChangesFromFile(filename CurrPath) []*Change {
 		year = yyyy
 	}
 
+	latest := make(map[PkgsrcPath]*Change)
+
 	infra := false
 	lines := Load(filename, MustSucceed|NotEmpty)
 	var changes []*Change
@@ -225,32 +228,85 @@ func (src *Pkgsrc) loadDocChangesFromFile(filename CurrPath) []*Change {
 			continue
 		}
 
-		if year != "" && change.Date[0:4] != year {
-			line.Warnf("Year %q for %s does not match the filename %s.",
-				change.Date[0:4], change.Pkgpath.String(), line.Rel(filename))
-		}
+		src.checkChangeVersion(change, latest, line)
+		src.checkChangeDate(filename, year, change, line, changes)
+	}
 
-		if len(changes) >= 2 && year != "" {
-			if prev := changes[len(changes)-2]; change.Date < prev.Date {
-				line.Warnf("Date %q for %s is earlier than %q in %s.",
-					change.Date, change.Pkgpath.String(), prev.Date, line.RelLocation(prev.Location))
-				line.Explain(
-					"The entries in doc/CHANGES should be in chronological order, and",
-					"all dates are assumed to be in the UTC timezone, to prevent time",
-					"warps.",
-					"",
-					"To fix this, determine which of the involved dates are correct",
-					"and which aren't.",
-					"",
-					"To prevent this kind of mistakes in the future,",
-					"make sure that your system time is correct and run",
-					sprintf("%q", bmake("cce")),
-					"to commit the changes entry.")
-			}
+	return changes
+}
+
+func (src *Pkgsrc) checkChangeVersion(change *Change, latest map[PkgsrcPath]*Change, line *Line) {
+	switch change.Action {
+
+	case Added:
+		src.checkChangeVersionNumber(change, line)
+		existing := latest[change.Pkgpath]
+		if existing != nil && existing.Version() == change.Version() {
+			line.Warnf("Package %q was already added in %s.",
+				change.Pkgpath.String(), line.RelLocation(existing.Location))
+		}
+		latest[change.Pkgpath] = change
+
+	case Updated:
+		src.checkChangeVersionNumber(change, line)
+		existing := latest[change.Pkgpath]
+		if existing != nil && pkgver.Compare(change.Version(), existing.Version()) <= 0 {
+			line.Warnf("Updating %q from %s in %s to %s should increase the version number.",
+				change.Pkgpath.String(), existing.Version(), line.RelLocation(existing.Location), change.Version())
 		}
+		latest[change.Pkgpath] = change
+
+	case Downgraded:
+		src.checkChangeVersionNumber(change, line)
+		existing := latest[change.Pkgpath]
+		if existing != nil && pkgver.Compare(change.Version(), existing.Version()) >= 0 {
+			line.Warnf("Downgrading %q from %s in %s to %s should decrease the version number.",
+				change.Pkgpath.String(), existing.Version(), line.RelLocation(existing.Location), change.Version())
+		}
+		latest[change.Pkgpath] = change
+
+	case Renamed, Moved, Removed:
+		latest[change.Pkgpath] = nil
 	}
+}
 
-	return changes
+func (src *Pkgsrc) checkChangeVersionNumber(change *Change, line *Line) {
+	version := change.Version()
+
+	switch {
+	case !textproc.NewLexer(version).TestByteSet(textproc.Digit):
+		line.Warnf("Version number %q should start with a digit.", version)
+
+	// See rePkgname for the regular expression.
+	case !matches(version, `^([0-9][.\-0-9A-Z_a-z]*)$`):
+		line.Warnf("Malformed version number %q.", version)
+	}
+}
+
+func (src *Pkgsrc) checkChangeDate(filename CurrPath, year string, change *Change, line *Line, changes []*Change) {
+	if year != "" && change.Date[0:4] != year {
+		line.Warnf("Year %q for %s does not match the filename %s.",
+			change.Date[0:4], change.Pkgpath.String(), line.Rel(filename))
+	}
+
+	if len(changes) >= 2 && year != "" {
+		if prev := changes[len(changes)-2]; change.Date < prev.Date {
+			line.Warnf("Date %q for %s is earlier than %q in %s.",
+				change.Date, change.Pkgpath.String(), prev.Date, line.RelLocation(prev.Location))
+			line.Explain(
+				"The entries in doc/CHANGES should be in chronological order, and",
+				"all dates are assumed to be in the UTC timezone, to prevent time",
+				"warps.",
+				"",
+				"To fix this, determine which of the involved dates are correct",
+				"and which aren't.",
+				"",
+				"To prevent this kind of mistakes in the future,",
+				"make sure that your system time is correct and run",
+				sprintf("%q", bmake("cce")),
+				"to commit the changes entry.")
+		}
+	}
 }
 
 func (*Pkgsrc) parseDocChange(line *Line, warn bool) *Change {
diff --git a/pkgtools/pkglint/files/pkgsrc_test.go b/pkgtools/pkglint/files/pkgsrc_test.go
index 0eb9fda3e4d..b9979801968 100644
--- a/pkgtools/pkglint/files/pkgsrc_test.go
+++ b/pkgtools/pkglint/files/pkgsrc_test.go
@@ -351,6 +351,49 @@ func (s *Suite) Test_Pkgsrc_loadDocChangesFromFile__old(c *check.C) {
 		"WARN: ~/doc/CHANGES-2018:6: Invalid doc/CHANGES line: \tUpdated pkgpath to 1.0 [author d]")
 }
 
+func (s *Suite) Test_Pkgsrc_checkChangeVersion(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpCommandLine("-Cglobal", "-Wall")
+	t.CreateFileLines("doc/CHANGES-2020",
+		"\tAdded category/package version 1.0 [author1 2020-01-01]",
+		"\tAdded category/package version 1.0 [author1 2020-01-01]",
+		"\tUpdated category/package to 0.9 [author1 2020-01-01]",
+		"\tDowngraded category/package to 1.0 [author1 2020-01-01]",
+		"\tRenamed category/package to category/renamed [author1 2020-01-01]",
+		"\tMoved category/package to other/renamed [author1 2020-01-01]")
+	t.Chdir("doc")
+
+	G.Pkgsrc.loadDocChangesFromFile("CHANGES-2020")
+
+	t.CheckOutputLines(
+		"WARN: CHANGES-2020:2: Package \"category/package\" was already added in line 1.",
+		"WARN: CHANGES-2020:3: Updating \"category/package\" from 1.0 in line 2 to 0.9 should increase the version number.",
+		"WARN: CHANGES-2020:4: Downgrading \"category/package\" from 0.9 in line 3 to 1.0 should decrease the version number.")
+}
+
+func (s *Suite) Test_Pkgsrc_checkChangeVersionNumber(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpCommandLine("-Cglobal", "-Wall")
+	t.CreateFileLines("doc/CHANGES-2020",
+		"\tAdded category/package version v1 [author1 2020-01-01]",
+		"\tUpdated category/package to v2 [author1 2020-01-01]",
+		"\tDowngraded category/package to v2 [author1 2020-01-01]",
+		"\tUpdated category/package to 2020/03 [author1 2020-01-01]")
+	t.Chdir("doc")
+
+	G.Pkgsrc.loadDocChangesFromFile("CHANGES-2020")
+
+	t.CheckOutputLines(
+		"WARN: CHANGES-2020:1: Version number \"v1\" should start with a digit.",
+		"WARN: CHANGES-2020:2: Version number \"v2\" should start with a digit.",
+		"WARN: CHANGES-2020:3: Version number \"v2\" should start with a digit.",
+		"WARN: CHANGES-2020:3: Downgrading \"category/package\" from v2 in line 2 "+
+			"to v2 should decrease the version number.",
+		"WARN: CHANGES-2020:4: Malformed version number \"2020/03\".")
+}
+
 func (s *Suite) Test_Pkgsrc_parseDocChange(c *check.C) {
 	t := s.Init(c)
 
diff --git a/pkgtools/pkglint/files/vardefs.go b/pkgtools/pkglint/files/vardefs.go
index 2423e7939a9..71dc5d741df 100644
--- a/pkgtools/pkglint/files/vardefs.go
+++ b/pkgtools/pkglint/files/vardefs.go
@@ -287,7 +287,7 @@ func (reg *VarTypeRegistry) compilerLanguages(src *Pkgsrc) *BasicType {
 				}
 			}
 
-			if mkline.IsDirective() && mkline.HasCond() && mkline.Cond() != nil {
+			if mkline.IsDirective() && mkline.NeedsCond() && mkline.Cond() != nil {
 				mkline.Cond().Walk(&MkCondCallback{
 					VarUse: func(varuse *MkVarUse) {
 						if varuse.varname == "USE_LANGUAGES" && len(varuse.modifiers) == 1 {
diff --git a/pkgtools/pkglint/files/vartypecheck.go b/pkgtools/pkglint/files/vartypecheck.go
index d0009d11a17..06d195b00a0 100644
--- a/pkgtools/pkglint/files/vartypecheck.go
+++ b/pkgtools/pkglint/files/vartypecheck.go
@@ -373,7 +373,11 @@ func (cv *VartypeCheck) DependencyPattern() {
 			"must be omitted, so the full package name becomes \"foo2.0-2.1.x\".")
 	}
 
-	checkBuildlinkApiDepends := func() {
+	checkDepends := func(
+		prefix string,
+		depends func(data *Buildlink3Data) *DependencyPattern,
+		dependsLine func(data *Buildlink3Data) *MkLine,
+	) {
 		if deppat.LowerOp == "" {
 			return
 		}
@@ -381,7 +385,7 @@ func (cv *VartypeCheck) DependencyPattern() {
 		if pkg == nil {
 			return
 		}
-		if !hasPrefix(cv.Varname, "BUILDLINK_API_DEPENDS.") {
+		if !hasPrefix(cv.Varname, prefix) {
 			return
 		}
 		bl3id := Buildlink3ID(varnameParam(cv.Varname))
@@ -389,42 +393,25 @@ func (cv *VartypeCheck) DependencyPattern() {
 		if data == nil {
 			return
 		}
-		if data.apiDepends.LowerOp != deppat.LowerOp {
+		defpat := depends(data)
+		if defpat.LowerOp != deppat.LowerOp {
 			return
 		}
-		if pkgver.Compare(deppat.Lower, data.apiDepends.Lower) < 0 {
+		if pkgver.Compare(deppat.Lower, defpat.Lower) < 0 {
 			cv.Warnf("Version %s is smaller than the default version %s from %s.",
-				deppat.Lower, data.apiDepends.Lower, cv.MkLine.RelMkLine(data.apiDependsLine))
+				deppat.Lower, defpat.Lower, cv.MkLine.RelMkLine(dependsLine(data)))
 		}
 	}
 
-	checkBuildlinkAbiDepends := func() {
-		if deppat.LowerOp == "" {
-			return
-		}
-		pkg := cv.MkLines.pkg
-		if pkg == nil {
-			return
-		}
-		if !hasPrefix(cv.Varname, "BUILDLINK_ABI_DEPENDS.") {
-			return
-		}
-		bl3id := Buildlink3ID(varnameParam(cv.Varname))
-		data := pkg.bl3Data[bl3id]
-		if data == nil {
-			return
-		}
-		if data.abiDepends.LowerOp != deppat.LowerOp {
-			return
-		}
-		if pkgver.Compare(deppat.Lower, data.abiDepends.Lower) < 0 {
-			cv.Warnf("Version %s is smaller than the default version %s from %s.",
-				deppat.Lower, data.abiDepends.Lower, cv.MkLine.RelMkLine(data.abiDependsLine))
-		}
-	}
-
-	checkBuildlinkApiDepends()
-	checkBuildlinkAbiDepends()
+	checkDepends(
+		"BUILDLINK_API_DEPENDS.",
+		func(data *Buildlink3Data) *DependencyPattern { return data.apiDepends },
+		func(data *Buildlink3Data) *MkLine { return data.apiDependsLine },
+	)
+	checkDepends(
+		"BUILDLINK_ABI_DEPENDS.",
+		func(data *Buildlink3Data) *DependencyPattern { return data.abiDepends },
+		func(data *Buildlink3Data) *MkLine { return data.abiDependsLine })
 }
 
 func (cv *VartypeCheck) DependencyWithPath() {