Source-Changes-HG archive

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

[pkgsrc/trunk]: pkgsrc/pkgtools/pkglint pkgtools/pkglint: update to 20.1.4



details:   https://anonhg.NetBSD.org/pkgsrc/rev/16a815febc2e
branches:  trunk
changeset: 431061:16a815febc2e
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Fri May 08 19:50:04 2020 +0000

description:
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.

diffstat:

 pkgtools/pkglint/Makefile               |    4 +-
 pkgtools/pkglint/files/buildlink3.go    |    2 -
 pkgtools/pkglint/files/check_test.go    |    4 +-
 pkgtools/pkglint/files/distinfo_test.go |   14 +-
 pkgtools/pkglint/files/mkline.go        |    5 +-
 pkgtools/pkglint/files/package.go       |    3 +-
 pkgtools/pkglint/files/package_test.go  |    6 +-
 pkgtools/pkglint/files/patches.go       |   64 ++++++++++++++---
 pkgtools/pkglint/files/patches_test.go  |  114 +++++++++++++++++++++++++++----
 pkgtools/pkglint/files/pkglint_test.go  |   10 +-
 pkgtools/pkglint/files/pkgsrc.go        |  102 ++++++++++++++++++++++------
 pkgtools/pkglint/files/pkgsrc_test.go   |   43 ++++++++++++
 pkgtools/pkglint/files/vardefs.go       |    2 +-
 pkgtools/pkglint/files/vartypecheck.go  |   51 +++++--------
 14 files changed, 319 insertions(+), 105 deletions(-)

diffs (truncated from 782 to 300 lines):

diff -r 3d49b7f747b5 -r 16a815febc2e pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Fri May 08 18:21:08 2020 +0000
+++ b/pkgtools/pkglint/Makefile Fri May 08 19:50:04 2020 +0000
@@ -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 -r 3d49b7f747b5 -r 16a815febc2e pkgtools/pkglint/files/buildlink3.go
--- a/pkgtools/pkglint/files/buildlink3.go      Fri May 08 18:21:08 2020 +0000
+++ b/pkgtools/pkglint/files/buildlink3.go      Fri May 08 19:50:04 2020 +0000
@@ -330,8 +330,6 @@
 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 -r 3d49b7f747b5 -r 16a815febc2e pkgtools/pkglint/files/check_test.go
--- a/pkgtools/pkglint/files/check_test.go      Fri May 08 18:21:08 2020 +0000
+++ b/pkgtools/pkglint/files/check_test.go      Fri May 08 19:50:04 2020 +0000
@@ -568,13 +568,15 @@
        // 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 -r 3d49b7f747b5 -r 16a815febc2e pkgtools/pkglint/files/distinfo_test.go
--- a/pkgtools/pkglint/files/distinfo_test.go   Fri May 08 18:21:08 2020 +0000
+++ b/pkgtools/pkglint/files/distinfo_test.go   Fri May 08 19:50:04 2020 +0000
@@ -113,7 +113,7 @@
 
        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 @@
        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 @@
                "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 @@
                        "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 @@
        t.SetUpFileLines("distinfo",
                CvsID,
                "",
-               "SHA1 (patch-aa) = ebbf34b0641bcb508f17d5a27f2bf2a536d810ac")
+               "SHA1 (patch-aa) = 9a93207561abfef7e7550598c5a08f2c3226995b")
        t.FinishSetUp()
 
        G.checkdirPackage(".")
@@ -797,7 +797,7 @@
        t.SetUpFileLines("distinfo",
                CvsID,
                "",
-               "SHA1 (patch-aa) = ebbf34b0641bcb508f17d5a27f2bf2a536d810ac")
+               "SHA1 (patch-aa) = 9a93207561abfef7e7550598c5a08f2c3226995b")
        t.FinishSetUp()
 
        G.checkdirPackage(".")
@@ -830,7 +830,7 @@
 
        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 -r 3d49b7f747b5 -r 16a815febc2e pkgtools/pkglint/files/mkline.go
--- a/pkgtools/pkglint/files/mkline.go  Fri May 08 18:21:08 2020 +0000
+++ b/pkgtools/pkglint/files/mkline.go  Fri May 08 19:50:04 2020 +0000
@@ -293,14 +293,15 @@
 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 -r 3d49b7f747b5 -r 16a815febc2e pkgtools/pkglint/files/package.go
--- a/pkgtools/pkglint/files/package.go Fri May 08 18:21:08 2020 +0000
+++ b/pkgtools/pkglint/files/package.go Fri May 08 19:50:04 2020 +0000
@@ -8,7 +8,8 @@
 )
 
 // 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 -r 3d49b7f747b5 -r 16a815febc2e pkgtools/pkglint/files/package_test.go
--- a/pkgtools/pkglint/files/package_test.go    Fri May 08 18:21:08 2020 +0000
+++ b/pkgtools/pkglint/files/package_test.go    Fri May 08 19:50:04 2020 +0000
@@ -143,7 +143,7 @@
        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 @@
        t.CreateFileLines("category/package/distinfo",
                CvsID,
                "",
-               "SHA1 (patch-aa) = ebbf34b0641bcb508f17d5a27f2bf2a536d810ac")
+               "SHA1 (patch-aa) = 9a93207561abfef7e7550598c5a08f2c3226995b")
 
        t.FinishSetUp()
 
@@ -3105,7 +3105,7 @@
        t.CreateFileLines("category/package/distinfo",
                CvsID,
                "",
-               "SHA1 (patch-aa) = ebbf34b0641bcb508f17d5a27f2bf2a536d810ac")
+               "SHA1 (patch-aa) = 9a93207561abfef7e7550598c5a08f2c3226995b")
        t.FinishSetUp()
 
        G.Check(pkg)
diff -r 3d49b7f747b5 -r 16a815febc2e pkgtools/pkglint/files/patches.go
--- a/pkgtools/pkglint/files/patches.go Fri May 08 18:21:08 2020 +0000
+++ b/pkgtools/pkglint/files/patches.go Fri May 08 19:50:04 2020 +0000
@@ -32,14 +32,15 @@
 
        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 @@
                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 @@
 
                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 @@
                }
        }
 
-       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) 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 -r 3d49b7f747b5 -r 16a815febc2e pkgtools/pkglint/files/patches_test.go
--- a/pkgtools/pkglint/files/patches_test.go    Fri May 08 18:21:08 2020 +0000
+++ b/pkgtools/pkglint/files/patches_test.go    Fri May 08 19:50:04 2020 +0000
@@ -15,8 +15,8 @@
                "* 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",



Home | Main Index | Thread Index | Old Index