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.9



details:   https://anonhg.NetBSD.org/pkgsrc/rev/e82dde8a7838
branches:  trunk
changeset: 323237:e82dde8a7838
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Mon May 06 20:27:17 2019 +0000

description:
pkgtools/pkglint: update to 5.7.9

Changes since 5.7.8:

* Buildlink3.mk files are checked for typos in the identifier that is
  used for BUILDLINK_TREE, to detect copy-and-paste mistakes.

* Having a rationale is recommended for some variables, especially those
  that make a package fail to build or crash at runtime. This check is
  only active when -Wextra is given, since it is still actively debated
  whether such a check is actually useful.

* Files called Makefile.php can easily be mistaken to be PHP files.
  Therefore the recommended naming convention is to have auxiliary files
  called *.mk. There are already many more files called *.mk than those
  being called Makefile.*.

* The check for unquoted sed substitution commands has been made more
  detailed, but since it is completely disabled, there's nothing to see
  for now.

* The definitions for MASTER_SITE_* are loaded directly from the pkgsrc
  infrastructure instead of hard-coding them in pkglint.

diffstat:

 pkgtools/pkglint/Makefile                     |    4 +-
 pkgtools/pkglint/files/buildlink3.go          |   25 +++++
 pkgtools/pkglint/files/buildlink3_test.go     |   53 ++++++++++++
 pkgtools/pkglint/files/check_test.go          |   19 ++++-
 pkgtools/pkglint/files/distinfo.go            |   11 +-
 pkgtools/pkglint/files/distinfo_test.go       |    4 +-
 pkgtools/pkglint/files/licenses.go            |    4 +-
 pkgtools/pkglint/files/mklinechecker.go       |    3 +
 pkgtools/pkglint/files/mklinechecker_test.go  |    8 +
 pkgtools/pkglint/files/mklines_test.go        |    1 +
 pkgtools/pkglint/files/package.go             |   26 +++++-
 pkgtools/pkglint/files/package_test.go        |   29 ++++++
 pkgtools/pkglint/files/pkglint.go             |   59 +++++++++++++-
 pkgtools/pkglint/files/pkglint_test.go        |   21 ++++
 pkgtools/pkglint/files/pkgsrc.go              |   10 +-
 pkgtools/pkglint/files/redundantscope_test.go |  112 +++++++++++++++++++++++++-
 pkgtools/pkglint/files/shell.go               |   71 +++++++++++++---
 pkgtools/pkglint/files/shell_test.go          |   52 +++++++----
 pkgtools/pkglint/files/toplevel.go            |    3 +-
 pkgtools/pkglint/files/util.go                |   16 +++
 pkgtools/pkglint/files/vardefs.go             |   54 +++---------
 21 files changed, 486 insertions(+), 99 deletions(-)

diffs (truncated from 920 to 300 lines):

diff -r fb0484ce012e -r e82dde8a7838 pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Mon May 06 17:40:16 2019 +0000
+++ b/pkgtools/pkglint/Makefile Mon May 06 20:27:17 2019 +0000
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.578 2019/04/28 18:13:53 rillig Exp $
+# $NetBSD: Makefile,v 1.579 2019/05/06 20:27:17 rillig Exp $
 
-PKGNAME=       pkglint-5.7.8
+PKGNAME=       pkglint-5.7.9
 CATEGORIES=    pkgtools
 DISTNAME=      tools
 MASTER_SITES=  ${MASTER_SITE_GITHUB:=golang/}
diff -r fb0484ce012e -r e82dde8a7838 pkgtools/pkglint/files/buildlink3.go
--- a/pkgtools/pkglint/files/buildlink3.go      Mon May 06 17:40:16 2019 +0000
+++ b/pkgtools/pkglint/files/buildlink3.go      Mon May 06 20:27:17 2019 +0000
@@ -2,6 +2,7 @@
 
 import (
        "netbsd.org/pkglint/pkgver"
+       "path"
        "strings"
 )
 
@@ -84,12 +85,36 @@
        if containsVarRef(pkgbase) {
                ck.checkVaruseInPkgbase(pkgbase, pkgbaseLine)
        }
+
+       ck.checkUniquePkgbase(pkgbase, pkgbaseLine)
+
        mlex.SkipEmptyOrNote()
        ck.pkgbase = pkgbase
        ck.pkgbaseLine = pkgbaseLine
        return true
 }
 
+func (ck *Buildlink3Checker) checkUniquePkgbase(pkgbase string, mkline MkLine) {
+       prev := G.InterPackage.Bl3(pkgbase, &mkline.Location)
+       if prev == nil {
+               return
+       }
+
+       base, name := trimCommon(pkgbase, path.Base(path.Dir(mkline.Filename)))
+       if base == "" && matches(name, `^(\d*|-cvs|-fossil|-git|-hg|-svn|-devel|-snapshot)$`) {
+               return
+       }
+
+       mkline.Errorf("Duplicate package identifier %q already appeared in %s.",
+               pkgbase, mkline.RefToLocation(*prev))
+       mkline.Explain(
+               "Each buildlink3.mk file must have a unique identifier.",
+               "These identifiers are used for multiple-inclusion guards,",
+               "and using the same identifier for different packages",
+               "(often by copy-and-paste) may change the dependencies",
+               "of a package in subtle and unexpected ways.")
+}
+
 // checkSecondParagraph checks the multiple inclusion protection and
 // introduces the uppercase package identifier.
 func (ck *Buildlink3Checker) checkSecondParagraph(mlex *MkLinesLexer) bool {
diff -r fb0484ce012e -r e82dde8a7838 pkgtools/pkglint/files/buildlink3_test.go
--- a/pkgtools/pkglint/files/buildlink3_test.go Mon May 06 17:40:16 2019 +0000
+++ b/pkgtools/pkglint/files/buildlink3_test.go Mon May 06 20:27:17 2019 +0000
@@ -520,6 +520,59 @@
                        "(also in other variables in this file).")
 }
 
+func (s *Suite) Test_Buildlink3Checker_checkUniquePkgbase(c *check.C) {
+       t := s.Init(c)
+
+       G.InterPackage.Enable()
+
+       test := func(pkgbase, pkgpath string, diagnostics ...string) {
+               mkline := t.NewMkLine(t.File(pkgpath+"/buildlink3.mk"), 123, "")
+
+               (*Buildlink3Checker).checkUniquePkgbase(nil, pkgbase, mkline)
+
+               t.CheckOutput(diagnostics)
+       }
+
+       // From now on, the pkgbase "php" may only be used for "php\d+".
+       test("php", "lang/php56",
+               nil...)
+
+       // No warning since "php" is a valid buildlink3 basename for "php56".
+       test("php", "lang/php72",
+               nil...)
+
+       // But this is a clear typo.
+       test("php", "security/pgp",
+               "ERROR: ~/security/pgp/buildlink3.mk:123: "+
+                       "Duplicate package identifier \"php\" already appeared "+
+                       "in ../../lang/php56/buildlink3.mk:123.")
+
+       // This combination is not allowed because the names "php" and "php-pcre"
+       // differ too much. The only allowed inexact match is that the pkgname
+       // has one more number than the pkgbase, no matter at which position.
+       test("php", "textproc/php-pcre",
+               "ERROR: ~/textproc/php-pcre/buildlink3.mk:123: "+
+                       "Duplicate package identifier \"php\" already appeared "+
+                       "in ../../lang/php56/buildlink3.mk:123.")
+
+       test("ruby-module", "net/ruby24-module",
+               nil...)
+
+       test("ruby-module", "net/ruby26-module",
+               nil...)
+
+       test("ruby-module", "net/ruby26-module12",
+               "ERROR: ~/net/ruby26-module12/buildlink3.mk:123: "+
+                       "Duplicate package identifier \"ruby-module\" already appeared "+
+                       "in ../../net/ruby24-module/buildlink3.mk:123.")
+
+       test("package", "devel/package",
+               nil...)
+
+       test("package", "wip/package",
+               nil...)
+}
+
 func (s *Suite) Test_Buildlink3Checker_checkMainPart__if_else_endif(c *check.C) {
        t := s.Init(c)
 
diff -r fb0484ce012e -r e82dde8a7838 pkgtools/pkglint/files/check_test.go
--- a/pkgtools/pkglint/files/check_test.go      Mon May 06 17:40:16 2019 +0000
+++ b/pkgtools/pkglint/files/check_test.go      Mon May 06 20:27:17 2019 +0000
@@ -177,6 +177,13 @@
 }
 
 func (t *Tester) SetUpMasterSite(varname string, urls ...string) {
+       if !G.Pkgsrc.vartypes.DefinedExact(varname) {
+               G.Pkgsrc.vartypes.DefineParse(varname, BtFetchURL,
+                       List|SystemProvided,
+                       "buildlink3.mk: none",
+                       "*: use")
+       }
+
        for _, url := range urls {
                G.Pkgsrc.registerMasterSite(varname, url)
        }
@@ -272,6 +279,8 @@
        // The various MASTER_SITE_* variables for use in the
        // MASTER_SITES are defined in this file.
        //
+       // To define a MASTER_SITE for a pkglint test, call t.SetUpMasterSite.
+       //
        // See Pkgsrc.loadMasterSites.
        t.CreateFileLines("mk/fetch/sites.mk",
                MkRcsID)
@@ -497,6 +506,14 @@
        return path.Clean(t.tmpdir + "/" + relativeFileName)
 }
 
+// Copy copies a file inside the temporary directory.
+func (t *Tester) Copy(relativeSrc, relativeDst string) {
+       data, err := ioutil.ReadFile(t.File(relativeSrc))
+       G.AssertNil(err, "Copy.Read")
+       err = ioutil.WriteFile(t.File(relativeDst), data, 0777)
+       G.AssertNil(err, "Copy.Write")
+}
+
 // Chdir changes the current working directory to the given subdirectory
 // of the temporary directory, creating it if necessary.
 //
@@ -552,7 +569,7 @@
 //              "VAR= env")))
 //
 //  mklines := get("including.mk")
-//  module := get("module.mk")
+//  module := get("subdir/module.mk")
 //
 // The filenames passed to the include function are all relative to the
 // same location, but that location is irrelevant in practice. The generated
diff -r fb0484ce012e -r e82dde8a7838 pkgtools/pkglint/files/distinfo.go
--- a/pkgtools/pkglint/files/distinfo.go        Mon May 06 17:40:16 2019 +0000
+++ b/pkgtools/pkglint/files/distinfo.go        Mon May 06 20:27:17 2019 +0000
@@ -323,8 +323,8 @@
 
 // Inter-package check for differing distfile checksums.
 func (ck *distinfoLinesChecker) checkGlobalDistfileMismatch(info distinfoHash) {
-       hashes := G.Hashes
-       if hashes == nil {
+
+       if !G.InterPackage.Enabled() {
                return
        }
 
@@ -348,23 +348,20 @@
                return
        }
 
-       key := alg + ":" + filename
-       otherHash := hashes[key]
-
        // See https://github.com/golang/go/issues/29802
        hashBytes := make([]byte, hex.DecodedLen(len(hash)))
        _, err := hex.Decode(hashBytes, []byte(hash))
        if err != nil {
                line.Errorf("The %s hash for %s contains a non-hex character.", alg, filename)
+               return
        }
 
+       otherHash := G.InterPackage.Hash(alg, filename, hashBytes, &line.Location)
        if otherHash != nil {
                if !bytes.Equal(otherHash.hash, hashBytes) {
                        line.Errorf("The %s hash for %s is %s, which conflicts with %s in %s.",
                                alg, filename, hash, hex.EncodeToString(otherHash.hash), line.RefToLocation(otherHash.location))
                }
-       } else {
-               hashes[key] = &Hash{hashBytes, line.Location}
        }
 }
 
diff -r fb0484ce012e -r e82dde8a7838 pkgtools/pkglint/files/distinfo_test.go
--- a/pkgtools/pkglint/files/distinfo_test.go   Mon May 06 17:40:16 2019 +0000
+++ b/pkgtools/pkglint/files/distinfo_test.go   Mon May 06 20:27:17 2019 +0000
@@ -204,8 +204,8 @@
                "(Run \"pkglint -e\" to show explanations.)")
 
        // Ensure that hex.DecodeString does not waste memory here.
-       t.Check(len(G.Hashes["SHA512:distfile-1.0.tar.gz"].hash), equals, 8)
-       t.Check(cap(G.Hashes["SHA512:distfile-1.0.tar.gz"].hash), equals, 8)
+       t.Check(len(G.InterPackage.hashes["SHA512:distfile-1.0.tar.gz"].hash), equals, 8)
+       t.Check(cap(G.InterPackage.hashes["SHA512:distfile-1.0.tar.gz"].hash), equals, 8)
 }
 
 func (s *Suite) Test_distinfoLinesChecker_checkAlgorithms__missing_patch_with_distfile_checksums(c *check.C) {
diff -r fb0484ce012e -r e82dde8a7838 pkgtools/pkglint/files/licenses.go
--- a/pkgtools/pkglint/files/licenses.go        Mon May 06 17:40:16 2019 +0000
+++ b/pkgtools/pkglint/files/licenses.go        Mon May 06 20:27:17 2019 +0000
@@ -32,9 +32,7 @@
        }
        if licenseFile == "" {
                licenseFile = G.Pkgsrc.File("licenses/" + license)
-               if G.UsedLicenses != nil {
-                       G.UsedLicenses[intern(license)] = true
-               }
+               G.InterPackage.UseLicense(license)
        }
 
        if !fileExists(licenseFile) {
diff -r fb0484ce012e -r e82dde8a7838 pkgtools/pkglint/files/mklinechecker.go
--- a/pkgtools/pkglint/files/mklinechecker.go   Mon May 06 17:40:16 2019 +0000
+++ b/pkgtools/pkglint/files/mklinechecker.go   Mon May 06 20:27:17 2019 +0000
@@ -403,6 +403,9 @@
 }
 
 func (ck MkLineChecker) checkVarassignLeftRationale() {
+       if !G.Opts.WarnExtra {
+               return
+       }
 
        isRationale := func(mkline MkLine) bool {
                if mkline.IsVarassign() || mkline.IsCommentedVarassign() {
diff -r fb0484ce012e -r e82dde8a7838 pkgtools/pkglint/files/mklinechecker_test.go
--- a/pkgtools/pkglint/files/mklinechecker_test.go      Mon May 06 17:40:16 2019 +0000
+++ b/pkgtools/pkglint/files/mklinechecker_test.go      Mon May 06 20:27:17 2019 +0000
@@ -766,6 +766,13 @@
                "WARN: filename.mk:2: Setting variable ONLY_FOR_PLATFORM should have a rationale.",
                "WARN: filename.mk:3: Setting variable NOT_FOR_PLATFORM should have a rationale.",
                "WARN: filename.mk:11: Setting variable ONLY_FOR_PLATFORM should have a rationale.")
+
+       // This check is only enabled when -Wextra is given.
+       t.SetUpCommandLine()
+
+       mklines.Check()
+
+       t.CheckOutputEmpty()
 }
 
 func (s *Suite) Test_MkLineChecker_checkVarassignOpShell(c *check.C) {
@@ -2150,6 +2157,7 @@
        t := s.Init(c)
 
        t.SetUpPkgsrc()
+       t.SetUpMasterSite("MASTER_SITE_GITHUB", "https://download.github.com/";)
        t.SetUpCommandLine("-Wall,no-space")
        mklines := t.SetUpFileMkLines("module.mk",
                MkRcsID,
diff -r fb0484ce012e -r e82dde8a7838 pkgtools/pkglint/files/mklines_test.go
--- a/pkgtools/pkglint/files/mklines_test.go    Mon May 06 17:40:16 2019 +0000
+++ b/pkgtools/pkglint/files/mklines_test.go    Mon May 06 20:27:17 2019 +0000
@@ -903,6 +903,7 @@
        t := s.Init(c)
 
        t.SetUpVartypes()
+       t.SetUpMasterSite("MASTER_SITE_SOURCEFORGE", "https://download.sf.net/";)
        mklines := t.NewMkLines("geography/viking/Makefile",
                MkRcsID,
                "MASTER_SITES=\t${MASTER_SITE_SOURCEFORGE:=viking/}${VERSION}/")
diff -r fb0484ce012e -r e82dde8a7838 pkgtools/pkglint/files/package.go
--- a/pkgtools/pkglint/files/package.go Mon May 06 17:40:16 2019 +0000
+++ b/pkgtools/pkglint/files/package.go Mon May 06 20:27:17 2019 +0000
@@ -218,11 +218,11 @@
        return files, mklines, allLines
 }
 
-func (pkg *Package) check(files []string, mklines, allLines MkLines) {
+func (pkg *Package) check(filenames []string, mklines, allLines MkLines) {
        haveDistinfo := false



Home | Main Index | Thread Index | Old Index