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:           Fri May  8 19:50:04 UTC 2020

Modified Files:
        pkgsrc/pkgtools/pkglint: Makefile
        pkgsrc/pkgtools/pkglint/files: buildlink3.go check_test.go
            distinfo_test.go mkline.go package.go package_test.go patches.go
            patches_test.go pkglint_test.go pkgsrc.go pkgsrc_test.go vardefs.go
            vartypecheck.go

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


To generate a diff of this commit:
cvs rdiff -u -r1.642 -r1.643 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.34 -r1.35 pkgsrc/pkgtools/pkglint/files/buildlink3.go
cvs rdiff -u -r1.68 -r1.69 pkgsrc/pkgtools/pkglint/files/check_test.go
cvs rdiff -u -r1.39 -r1.40 pkgsrc/pkgtools/pkglint/files/distinfo_test.go \
    pkgsrc/pkgtools/pkglint/files/patches.go
cvs rdiff -u -r1.77 -r1.78 pkgsrc/pkgtools/pkglint/files/mkline.go
cvs rdiff -u -r1.88 -r1.89 pkgsrc/pkgtools/pkglint/files/package.go
cvs rdiff -u -r1.75 -r1.76 pkgsrc/pkgtools/pkglint/files/package_test.go
cvs rdiff -u -r1.38 -r1.39 pkgsrc/pkgtools/pkglint/files/patches_test.go
cvs rdiff -u -r1.63 -r1.64 pkgsrc/pkgtools/pkglint/files/pkglint_test.go
cvs rdiff -u -r1.53 -r1.54 pkgsrc/pkgtools/pkglint/files/pkgsrc.go
cvs rdiff -u -r1.47 -r1.48 pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go
cvs rdiff -u -r1.94 -r1.95 pkgsrc/pkgtools/pkglint/files/vardefs.go
cvs rdiff -u -r1.85 -r1.86 pkgsrc/pkgtools/pkglint/files/vartypecheck.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.642 pkgsrc/pkgtools/pkglint/Makefile:1.643
--- pkgsrc/pkgtools/pkglint/Makefile:1.642      Thu Apr 30 21:15:03 2020
+++ pkgsrc/pkgtools/pkglint/Makefile    Fri May  8 19:50:04 2020
@@ -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/}

Index: pkgsrc/pkgtools/pkglint/files/buildlink3.go
diff -u pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.34 pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.35
--- pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.34    Wed Mar 18 08:24:49 2020
+++ pkgsrc/pkgtools/pkglint/files/buildlink3.go Fri May  8 19:50:04 2020
@@ -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() {

Index: pkgsrc/pkgtools/pkglint/files/check_test.go
diff -u pkgsrc/pkgtools/pkglint/files/check_test.go:1.68 pkgsrc/pkgtools/pkglint/files/check_test.go:1.69
--- pkgsrc/pkgtools/pkglint/files/check_test.go:1.68    Wed Mar 18 08:24:49 2020
+++ pkgsrc/pkgtools/pkglint/files/check_test.go Fri May  8 19:50:04 2020
@@ -568,13 +568,15 @@ func (t *Tester) CreateFileDummyPatch(fi
        // 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")

Index: pkgsrc/pkgtools/pkglint/files/distinfo_test.go
diff -u pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.39 pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.40
--- pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.39 Thu Mar 26 07:02:44 2020
+++ pkgsrc/pkgtools/pkglint/files/distinfo_test.go      Fri May  8 19:50:04 2020
@@ -113,7 +113,7 @@ func (s *Suite) Test_distinfoLinesChecke
 
        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_distinfoLinesChecke
        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_distinfoLinesChecke
                "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_distinfoLinesChecke
                        "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_distinfoLinesChecke
        t.SetUpFileLines("distinfo",
                CvsID,
                "",
-               "SHA1 (patch-aa) = ebbf34b0641bcb508f17d5a27f2bf2a536d810ac")
+               "SHA1 (patch-aa) = 9a93207561abfef7e7550598c5a08f2c3226995b")
        t.FinishSetUp()
 
        G.checkdirPackage(".")
@@ -797,7 +797,7 @@ func (s *Suite) Test_distinfoLinesChecke
        t.SetUpFileLines("distinfo",
                CvsID,
                "",
-               "SHA1 (patch-aa) = ebbf34b0641bcb508f17d5a27f2bf2a536d810ac")
+               "SHA1 (patch-aa) = 9a93207561abfef7e7550598c5a08f2c3226995b")
        t.FinishSetUp()
 
        G.checkdirPackage(".")
@@ -830,7 +830,7 @@ func (s *Suite) Test_distinfoLinesChecke
 
        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\" "+
Index: pkgsrc/pkgtools/pkglint/files/patches.go
diff -u pkgsrc/pkgtools/pkglint/files/patches.go:1.39 pkgsrc/pkgtools/pkglint/files/patches.go:1.40
--- pkgsrc/pkgtools/pkglint/files/patches.go:1.39       Thu Apr 30 21:15:03 2020
+++ pkgsrc/pkgtools/pkglint/files/patches.go    Fri May  8 19:50:04 2020
@@ -32,14 +32,15 @@ func (ck *PatchChecker) Check(pkg *Packa
 
        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 *Packa
                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 *Packa
 
                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 *Packa
                }
        }
 
-       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(t
        }
 }
 
+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: ") ||

Index: pkgsrc/pkgtools/pkglint/files/mkline.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline.go:1.77 pkgsrc/pkgtools/pkglint/files/mkline.go:1.78
--- pkgsrc/pkgtools/pkglint/files/mkline.go:1.77        Thu Mar 26 07:02:44 2020
+++ pkgsrc/pkgtools/pkglint/files/mkline.go     Fri May  8 19:50:04 2020
@@ -293,14 +293,15 @@ func (mkline *MkLine) Args() string { re
 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"
 }

Index: pkgsrc/pkgtools/pkglint/files/package.go
diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.88 pkgsrc/pkgtools/pkglint/files/package.go:1.89
--- pkgsrc/pkgtools/pkglint/files/package.go:1.88       Thu Apr 30 21:15:03 2020
+++ pkgsrc/pkgtools/pkglint/files/package.go    Fri May  8 19:50:04 2020
@@ -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.
 //

Index: pkgsrc/pkgtools/pkglint/files/package_test.go
diff -u pkgsrc/pkgtools/pkglint/files/package_test.go:1.75 pkgsrc/pkgtools/pkglint/files/package_test.go:1.76
--- pkgsrc/pkgtools/pkglint/files/package_test.go:1.75  Thu Apr 30 21:15:03 2020
+++ pkgsrc/pkgtools/pkglint/files/package_test.go       Fri May  8 19:50:04 2020
@@ -143,7 +143,7 @@ func (s *Suite) Test_Package__using_comm
        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_checkfilePa
        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_checkOwnerM
        t.CreateFileLines("category/package/distinfo",
                CvsID,
                "",
-               "SHA1 (patch-aa) = ebbf34b0641bcb508f17d5a27f2bf2a536d810ac")
+               "SHA1 (patch-aa) = 9a93207561abfef7e7550598c5a08f2c3226995b")
        t.FinishSetUp()
 
        G.Check(pkg)

Index: pkgsrc/pkgtools/pkglint/files/patches_test.go
diff -u pkgsrc/pkgtools/pkglint/files/patches_test.go:1.38 pkgsrc/pkgtools/pkglint/files/patches_test.go:1.39
--- pkgsrc/pkgtools/pkglint/files/patches_test.go:1.38  Thu Apr 30 21:15:03 2020
+++ pkgsrc/pkgtools/pkglint/files/patches_test.go       Fri May  8 19:50:04 2020
@@ -15,8 +15,8 @@ func (s *Suite) Test_CheckLinesPatch__wi
                "* 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
 
        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__wi
        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__er
                "",
                "*** 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__wr
                "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__on
                "",
                "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_checkA
                        "",
                        "Demonstrates absolute paths.",
                        "",
-                       "--- before",
-                       "+++ after",
+                       "--- file.orig",
+                       "+++ file",
                        "@@ -1,0 +1,1 @@",
                        "+"+addedLine)
 
@@ -940,6 +940,90 @@ func (s *Suite) Test_PatchChecker_checkt
                "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) {

Index: pkgsrc/pkgtools/pkglint/files/pkglint_test.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.63 pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.64
--- pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.63  Mon Mar 23 19:55:08 2020
+++ pkgsrc/pkgtools/pkglint/files/pkglint_test.go       Fri May  8 19:50:04 2020
@@ -215,8 +215,8 @@ func (s *Suite) Test_Pkglint_Main__compl
                "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__compl
                "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__r
        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_checkRegCvs
        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",

Index: pkgsrc/pkgtools/pkglint/files/pkgsrc.go
diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.53 pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.54
--- pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.53        Wed Mar 18 08:42:48 2020
+++ pkgsrc/pkgtools/pkglint/files/pkgsrc.go     Fri May  8 19:50:04 2020
@@ -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) loadDocChangesFromFil
                year = yyyy
        }
 
+       latest := make(map[PkgsrcPath]*Change)
+
        infra := false
        lines := Load(filename, MustSucceed|NotEmpty)
        var changes []*Change
@@ -225,34 +228,87 @@ func (src *Pkgsrc) loadDocChangesFromFil
                        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))
-               }
-
-               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.")
-                       }
-               }
+               src.checkChangeVersion(change, latest, line)
+               src.checkChangeDate(filename, year, change, line, changes)
        }
 
        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
+       }
+}
+
+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 {
        lex := textproc.NewLexer(line.Text)
 

Index: pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go
diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.47 pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.48
--- pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.47   Sun Mar 22 17:43:15 2020
+++ pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go        Fri May  8 19:50:04 2020
@@ -351,6 +351,49 @@ func (s *Suite) Test_Pkgsrc_loadDocChang
                "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)
 

Index: pkgsrc/pkgtools/pkglint/files/vardefs.go
diff -u pkgsrc/pkgtools/pkglint/files/vardefs.go:1.94 pkgsrc/pkgtools/pkglint/files/vardefs.go:1.95
--- pkgsrc/pkgtools/pkglint/files/vardefs.go:1.94       Thu Apr 30 21:15:03 2020
+++ pkgsrc/pkgtools/pkglint/files/vardefs.go    Fri May  8 19:50:04 2020
@@ -287,7 +287,7 @@ func (reg *VarTypeRegistry) compilerLang
                                }
                        }
 
-                       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 {

Index: pkgsrc/pkgtools/pkglint/files/vartypecheck.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.85 pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.86
--- pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.85  Thu Apr 30 21:15:03 2020
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck.go       Fri May  8 19:50:04 2020
@@ -373,7 +373,11 @@ func (cv *VartypeCheck) DependencyPatter
                        "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) DependencyPatter
                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) DependencyPatter
                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() {



Home | Main Index | Thread Index | Old Index