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