pkgsrc-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 5.7.3



details:   https://anonhg.NetBSD.org/pkgsrc/rev/9227ad281d5e
branches:  trunk
changeset: 393743:9227ad281d5e
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Sun Mar 24 13:58:38 2019 +0000

description:
pkgtools/pkglint: update to 5.7.3

Changes since 5.7.2:

PLIST files are checked for non-ASCII characters. Even though pkgsrc
sets up the environment with LC_ALL=C, there are still some cases
of encoding errors. The case discussed on the tech-pkg mailing list
was lang/go112.

The checks for variable permissions ("may not be set in this file")
have been reworked completely. Many of the variable permissions had
different rules for Makefile and Makefile.common. These different
rules tried to prevent accidental overwriting of variables. Starting
in July 2018, pkglint got a check for redundant variables that is
far more accurate than the previous variable permissions. Therefore
these fine-grained permissions are no longer necessary. This removes
a few hundred wrong warnings about insufficient permissions.

The check that adds missing SHA512 hashes to distinfo files has been
fixed to work correctly in DIST_SUBDIR cases.

Improved the checks regarding tools that are used by a package but
not added to USE_TOOLS. For example, the "make" tool is always
available, as are all tools that are added to TOOLS_CREATE.

Lots of small improvements, as always.

diffstat:

 pkgtools/pkglint/Makefile                       |     5 +-
 pkgtools/pkglint/files/buildlink3_test.go       |    42 +-
 pkgtools/pkglint/files/check_test.go            |    30 +-
 pkgtools/pkglint/files/cmd/pkglint/main_test.go |     2 +-
 pkgtools/pkglint/files/distinfo.go              |    39 +-
 pkgtools/pkglint/files/distinfo_test.go         |    20 +-
 pkgtools/pkglint/files/mkline.go                |     2 -
 pkgtools/pkglint/files/mkline_test.go           |   117 +-
 pkgtools/pkglint/files/mklinechecker.go         |   213 +-
 pkgtools/pkglint/files/mklinechecker_test.go    |   485 +++-
 pkgtools/pkglint/files/mklines.go               |     6 +-
 pkgtools/pkglint/files/mklines_test.go          |    16 +-
 pkgtools/pkglint/files/mklines_varalign_test.go |    12 +-
 pkgtools/pkglint/files/mkparser_test.go         |    18 +-
 pkgtools/pkglint/files/mkshparser_test.go       |    26 +-
 pkgtools/pkglint/files/mktypes.go               |    19 +-
 pkgtools/pkglint/files/mktypes_test.go          |    22 +
 pkgtools/pkglint/files/options.go               |    13 +-
 pkgtools/pkglint/files/package.go               |   177 +-
 pkgtools/pkglint/files/package_test.go          |    92 +-
 pkgtools/pkglint/files/pkglint.go               |   104 +-
 pkgtools/pkglint/files/pkglint_test.go          |     6 +-
 pkgtools/pkglint/files/pkgsrc.go                |    60 +-
 pkgtools/pkglint/files/pkgsrc_test.go           |     4 +-
 pkgtools/pkglint/files/plist.go                 |    78 +-
 pkgtools/pkglint/files/plist_test.go            |   117 +-
 pkgtools/pkglint/files/redundantscope.go        |    20 +-
 pkgtools/pkglint/files/redundantscope_test.go   |  1059 +++++---
 pkgtools/pkglint/files/shell.go                 |   128 +-
 pkgtools/pkglint/files/shell_test.go            |   309 +-
 pkgtools/pkglint/files/tools.go                 |     2 +-
 pkgtools/pkglint/files/tools_test.go            |    48 +-
 pkgtools/pkglint/files/util.go                  |    40 +-
 pkgtools/pkglint/files/util_test.go             |    37 +-
 pkgtools/pkglint/files/vardefs.go               |  2608 +++++++++++++---------
 pkgtools/pkglint/files/vardefs_test.go          |    44 +-
 pkgtools/pkglint/files/vartype.go               |    79 +-
 pkgtools/pkglint/files/vartype_test.go          |   112 +-
 pkgtools/pkglint/files/vartypecheck.go          |    19 +-
 pkgtools/pkglint/files/vartypecheck_test.go     |    44 +-
 40 files changed, 3811 insertions(+), 2463 deletions(-)

diffs (truncated from 9543 to 300 lines):

diff -r 4fee6faf36ea -r 9227ad281d5e pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Sun Mar 24 13:04:05 2019 +0000
+++ b/pkgtools/pkglint/Makefile Sun Mar 24 13:58:38 2019 +0000
@@ -1,7 +1,6 @@
-# $NetBSD: Makefile,v 1.571 2019/03/16 08:35:48 bsiegert Exp $
+# $NetBSD: Makefile,v 1.572 2019/03/24 13:58:38 rillig Exp $
 
-PKGNAME=       pkglint-5.7.2
-PKGREVISION=   1
+PKGNAME=       pkglint-5.7.3
 CATEGORIES=    pkgtools
 DISTNAME=      tools
 MASTER_SITES=  ${MASTER_SITE_GITHUB:=golang/}
diff -r 4fee6faf36ea -r 9227ad281d5e pkgtools/pkglint/files/buildlink3_test.go
--- a/pkgtools/pkglint/files/buildlink3_test.go Sun Mar 24 13:04:05 2019 +0000
+++ b/pkgtools/pkglint/files/buildlink3_test.go Sun Mar 24 13:58:38 2019 +0000
@@ -130,6 +130,38 @@
        t.CheckOutputEmpty()
 }
 
+func (s *Suite) Test_CheckLinesBuildlink3Mk__name_mismatch__Perl(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpPackage("x11/p5-gtk2",
+               "DISTNAME=\tGtk2-1.0",
+               "PKGNAME=\t${DISTNAME:C:Gtk2:p5-gtk2:}")
+       t.CreateFileLines("x11/p5-gtk2/buildlink3.mk",
+               MkRcsID,
+               "",
+               "BUILDLINK_TREE+=\tp5-gtk2",
+               "",
+               ".if !defined(P5_GTK2_BUILDLINK3_MK)",
+               "P5_GTK2_BUILDLINK3_MK:=",
+               "",
+               "BUILDLINK_API_DEPENDS.p5-gtk2+=\tp5-gtk2>=1.0",
+               "BUILDLINK_ABI_DEPENDS.p5-gtk2+=\tp5-gtk2>=1.0",
+               "",
+               ".endif\t# P5_GTK2_BUILDLINK3_MK",
+               "",
+               "BUILDLINK_TREE+=\t-p5-gtk2")
+
+       G.Check(t.File("x11/p5-gtk2"))
+
+       // Up to 2019-03-17, pkglint wrongly complained about a mismatch
+       // between the package name from buildlink3.mk (p5-gtk2) and the
+       // one from the package Makefile (Gtk2).
+       //
+       // Pkglint had taken this information from the DISTNAME variable,
+       // ignoring the fact that PKGNAME was also defined.
+       t.CheckOutputEmpty()
+}
+
 func (s *Suite) Test_CheckLinesBuildlink3Mk__name_mismatch_multiple_inclusion(c *check.C) {
        t := s.Init(c)
 
@@ -472,16 +504,14 @@
        CheckLinesBuildlink3Mk(mklines)
 
        t.CheckOutputLines(
-               "WARN: buildlink3.mk:3: LICENSE may not be used in any file; it is a write-only variable.",
+               "WARN: buildlink3.mk:3: LICENSE should not be used in this file; "+
+                       "it would be ok in Makefile, Makefile.* or *.mk, but not buildlink3.mk or builtin.mk.",
                "WARN: buildlink3.mk:3: The variable LICENSE should be quoted as part of a shell word.",
-               "WARN: buildlink3.mk:8: LICENSE should not be evaluated at load time.",
-               "WARN: buildlink3.mk:8: LICENSE should not be evaluated indirectly at load time.",
                "WARN: buildlink3.mk:8: The variable LICENSE should be quoted as part of a shell word.",
-               "WARN: buildlink3.mk:9: LICENSE should not be evaluated at load time.",
-               "WARN: buildlink3.mk:9: LICENSE should not be evaluated indirectly at load time.",
                "WARN: buildlink3.mk:9: The variable LICENSE should be quoted as part of a shell word.",
                "WARN: buildlink3.mk:13: The variable LICENSE should be quoted as part of a shell word.",
-               "WARN: buildlink3.mk:3: Please replace \"${LICENSE}\" with a simple string (also in other variables in this file).")
+               "WARN: buildlink3.mk:3: Please replace \"${LICENSE}\" with a simple string "+
+                       "(also in other variables in this file).")
 }
 
 func (s *Suite) Test_Buildlink3Checker_checkMainPart__if_else_endif(c *check.C) {
diff -r 4fee6faf36ea -r 9227ad281d5e pkgtools/pkglint/files/check_test.go
--- a/pkgtools/pkglint/files/check_test.go      Sun Mar 24 13:04:05 2019 +0000
+++ b/pkgtools/pkglint/files/check_test.go      Sun Mar 24 13:58:38 2019 +0000
@@ -162,7 +162,7 @@
 //
 // See SetUpTool for registering tools like echo, awk, perl.
 func (t *Tester) SetUpVartypes() {
-       G.Pkgsrc.InitVartypes()
+       G.Pkgsrc.vartypes.Init(G.Pkgsrc)
 }
 
 func (t *Tester) SetUpMasterSite(varname string, urls ...string) {
@@ -505,11 +505,13 @@
                t.c.Fatalf("Chdir must only be called once per test; already in %q.", t.relCwd)
        }
 
-       _ = os.MkdirAll(t.File(relativeDirName), 0700)
-       if err := os.Chdir(t.File(relativeDirName)); err != nil {
+       absDirName := t.File(relativeDirName)
+       _ = os.MkdirAll(absDirName, 0700)
+       if err := os.Chdir(absDirName); err != nil {
                t.c.Fatalf("Cannot chdir: %s", err)
        }
        t.relCwd = relativeDirName
+       G.cwd = absDirName
 }
 
 // Remove removes the file from the temporary directory. The file must exist.
@@ -689,8 +691,8 @@
        return NewMkLine(t.NewLine(filename, lineno, text))
 }
 
-func (t *Tester) NewShellLine(filename string, lineno int, text string) *ShellLine {
-       return NewShellLine(t.NewMkLine(filename, lineno, text))
+func (t *Tester) NewShellLineChecker(filename string, lineno int, text string) *ShellLineChecker {
+       return NewShellLineChecker(t.NewMkLine(filename, lineno, text))
 }
 
 // NewLines returns a list of simple lines that belong together.
@@ -759,11 +761,7 @@
 //
 // See CheckOutputLines.
 func (t *Tester) CheckOutputEmpty() {
-       output := t.Output()
-
-       actualLines := strings.Split(output, "\n")
-       actualLines = actualLines[:len(actualLines)-1]
-       t.Check(emptyToNil(actualLines), deepEquals, emptyToNil(nil))
+       t.CheckOutput(nil)
 }
 
 // CheckOutputLines checks that the output up to now equals the given lines.
@@ -773,7 +771,19 @@
 // See CheckOutputEmpty.
 func (t *Tester) CheckOutputLines(expectedLines ...string) {
        G.Assertf(len(expectedLines) > 0, "To check empty lines, use CheckLinesEmpty instead.")
+       t.CheckOutput(expectedLines)
+}
 
+// CheckOutput checks that the output up to now equals the given lines.
+// After the comparison, the output buffers are cleared so that later
+// calls only check against the newly added output.
+//
+// The expectedLines can be either empty or non-empty.
+//
+// When the output is always empty, use CheckOutputEmpty instead.
+// When the output always contain some lines, use CheckOutputLines instead.
+// This variant should only be used when the expectedLines are generated dynamically.
+func (t *Tester) CheckOutput(expectedLines []string) {
        output := t.Output()
        actualLines := strings.Split(output, "\n")
        actualLines = actualLines[:len(actualLines)-1]
diff -r 4fee6faf36ea -r 9227ad281d5e pkgtools/pkglint/files/cmd/pkglint/main_test.go
--- a/pkgtools/pkglint/files/cmd/pkglint/main_test.go   Sun Mar 24 13:04:05 2019 +0000
+++ b/pkgtools/pkglint/files/cmd/pkglint/main_test.go   Sun Mar 24 13:58:38 2019 +0000
@@ -44,5 +44,5 @@
        output, err := ioutil.ReadFile(out.Name())
        c.Assert(err, check.IsNil)
 
-       c.Check(string(output), check.Matches, `^(@VERSION@|\d+(\.\d+)+)\n$`)
+       c.Check(string(output), check.Matches, `^(@VERSION@|\d+(\.\d+)+(nb\d+)?)\n$`)
 }
diff -r 4fee6faf36ea -r 9227ad281d5e pkgtools/pkglint/files/distinfo.go
--- a/pkgtools/pkglint/files/distinfo.go        Sun Mar 24 13:04:05 2019 +0000
+++ b/pkgtools/pkglint/files/distinfo.go        Sun Mar 24 13:58:38 2019 +0000
@@ -13,15 +13,15 @@
        "strings"
 )
 
-func CheckLinesDistinfo(lines Lines) {
+func CheckLinesDistinfo(pkg *Package, lines Lines) {
        if trace.Tracing {
                defer trace.Call1(lines.FileName)()
        }
 
        filename := lines.FileName
        patchdir := "patches"
-       if G.Pkg != nil && dirExists(G.Pkg.File(G.Pkg.Patchdir)) {
-               patchdir = G.Pkg.Patchdir
+       if pkg != nil && dirExists(pkg.File(pkg.Patchdir)) {
+               patchdir = pkg.Patchdir
        }
        if trace.Tracing {
                trace.Step1("patchdir=%q", patchdir)
@@ -29,7 +29,7 @@
 
        distinfoIsCommitted := isCommitted(filename)
        ck := distinfoLinesChecker{
-               lines, patchdir, distinfoIsCommitted,
+               pkg, lines, patchdir, distinfoIsCommitted,
                nil, make(map[string]distinfoFileInfo)}
        ck.parse()
        ck.check()
@@ -40,8 +40,9 @@
 }
 
 type distinfoLinesChecker struct {
+       pkg                 *Package
        lines               Lines
-       patchdir            string // Relative to G.Pkg
+       patchdir            string // Relative to pkg
        distinfoIsCommitted bool
 
        filenames []string // For keeping the order from top to bottom
@@ -64,9 +65,9 @@
                switch {
                case !hasPrefix(prevFilename, "patch-"):
                        return no
-               case G.Pkg == nil:
+               case ck.pkg == nil:
                        return unknown
-               case fileExists(G.Pkg.File(ck.patchdir + "/" + prevFilename)):
+               case fileExists(ck.pkg.File(ck.patchdir + "/" + prevFilename)):
                        return yes
                default:
                        return no
@@ -146,12 +147,12 @@
        // At this point, the file is either a missing patch file or a distfile.
 
        case hasPrefix(filename, "patch-") && algorithms == "SHA1":
-               if G.Pkg.IgnoreMissingPatches {
+               if ck.pkg.IgnoreMissingPatches {
                        break
                }
 
                line.Warnf("Patch file %q does not exist in directory %q.",
-                       filename, line.PathToFile(G.Pkg.File(ck.patchdir)))
+                       filename, line.PathToFile(ck.pkg.File(ck.patchdir)))
                G.Explain(
                        "If the patches directory looks correct, the patch may have been",
                        "removed without updating the distinfo file.",
@@ -193,10 +194,6 @@
        }
 
        distdir := G.Pkgsrc.File("distfiles")
-       distSubdir := ""
-       if G.Pkg != nil {
-               distSubdir = G.Pkg.vars.LastValue("DIST_SUBDIR")
-       }
 
        // It's a rare situation that the explanation is generated
        // this far from the corresponding diagnostic.
@@ -206,7 +203,7 @@
                "To add the missing lines to the distinfo file, run",
                sprintf("\t%s", bmake("distinfo")),
                "for each variant of the package until all distfiles are downloaded to",
-               sprintf("%q.", cleanpath("${PKGSRCDIR}/distfiles/"+distSubdir)),
+               "${PKGSRCDIR}/distfiles.",
                "",
                "The variants are typically selected by setting EMUL_PLATFORM",
                "or similar variables in the command line.",
@@ -221,7 +218,7 @@
                "which will find the downloaded distfiles and add the missing",
                "hashes to the distinfo file.")
 
-       distfile := cleanpath(distdir + "/" + distSubdir + "/" + info.filename())
+       distfile := cleanpath(distdir + "/" + info.filename())
        if !fileExists(distfile) {
                return
        }
@@ -301,10 +298,10 @@
 }
 
 func (ck *distinfoLinesChecker) checkUnrecordedPatches() {
-       if G.Pkg == nil {
+       if ck.pkg == nil {
                return
        }
-       patchFiles, err := ioutil.ReadDir(G.Pkg.File(ck.patchdir))
+       patchFiles, err := ioutil.ReadDir(ck.pkg.File(ck.patchdir))
        if err != nil {
                if trace.Tracing {
                        trace.Stepf("Cannot read patchdir %q: %s", ck.patchdir, err)
@@ -317,7 +314,7 @@
                if file.Mode().IsRegular() && ck.infos[patchName].isPatch != yes && hasPrefix(patchName, "patch-") {
                        line := NewLineWhole(ck.lines.FileName)
                        line.Errorf("Patch %q is not recorded. Run %q.",
-                               line.PathToFile(G.Pkg.File(ck.patchdir+"/"+patchName)),
+                               line.PathToFile(ck.pkg.File(ck.patchdir+"/"+patchName)),
                                bmake("makepatchsum"))
                }
        }
@@ -377,7 +374,7 @@
        line := info.line
 
        patchFileName := ck.patchdir + "/" + patchName
-       resolvedPatchFileName := G.Pkg.File(patchFileName)
+       resolvedPatchFileName := ck.pkg.File(patchFileName)
        if ck.distinfoIsCommitted && !isCommitted(resolvedPatchFileName) {
                line.Warnf("%s is registered in distinfo but not added to CVS.", line.PathToFile(resolvedPatchFileName))
        }
@@ -387,7 +384,7 @@
 }
 
 func (ck *distinfoLinesChecker) checkPatchSha1(line Line, patchFileName, distinfoSha1Hex string) {
-       fileSha1Hex, err := computePatchSha1Hex(G.Pkg.File(patchFileName))
+       fileSha1Hex, err := computePatchSha1Hex(ck.pkg.File(patchFileName))
        if err != nil {
                line.Errorf("Patch %s does not exist.", patchFileName)
                return
@@ -395,7 +392,7 @@
        if distinfoSha1Hex != fileSha1Hex {
                fix := line.Autofix()
                fix.Errorf("SHA1 hash of %s differs (distinfo has %s, patch file has %s).",



Home | Main Index | Thread Index | Old Index