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.8
details: https://anonhg.NetBSD.org/pkgsrc/rev/bc03635944ad
branches: trunk
changeset: 322820:bc03635944ad
user: rillig <rillig%pkgsrc.org@localhost>
date: Sun Apr 28 18:13:53 2019 +0000
description:
pkgtools/pkglint: update to 5.7.8
Changes since 5.7.7:
Warn about definitions of NOT_FOR_* and ONLY_FOR_* which are missing a
rationale. When maintaining such packages it helps a lot to know why the
package cannot be built on a particular platform or with a particular
compiler or Python version.
diffstat:
pkgtools/pkglint/Makefile | 4 +-
pkgtools/pkglint/files/logging.go | 5 +-
pkgtools/pkglint/files/mklinechecker.go | 58 ++++++++++++++++++++
pkgtools/pkglint/files/mklinechecker_test.go | 42 ++++++++++++-
pkgtools/pkglint/files/pkgsrc_test.go | 11 +++-
pkgtools/pkglint/files/vardefs.go | 81 +++++++++++++++++++--------
pkgtools/pkglint/files/vartype.go | 6 ++
7 files changed, 170 insertions(+), 37 deletions(-)
diffs (truncated from 424 to 300 lines):
diff -r 63571a55c0d7 -r bc03635944ad pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Sun Apr 28 17:11:53 2019 +0000
+++ b/pkgtools/pkglint/Makefile Sun Apr 28 18:13:53 2019 +0000
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.577 2019/04/27 19:33:56 rillig Exp $
+# $NetBSD: Makefile,v 1.578 2019/04/28 18:13:53 rillig Exp $
-PKGNAME= pkglint-5.7.7
+PKGNAME= pkglint-5.7.8
CATEGORIES= pkgtools
DISTNAME= tools
MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/}
diff -r 63571a55c0d7 -r bc03635944ad pkgtools/pkglint/files/logging.go
--- a/pkgtools/pkglint/files/logging.go Sun Apr 28 17:11:53 2019 +0000
+++ b/pkgtools/pkglint/files/logging.go Sun Apr 28 18:13:53 2019 +0000
@@ -236,9 +236,8 @@
// Errorf logs a technical error on the error output.
//
-// location must be either an empty string or a slash-separated filename,
-// such as the one in Location.Filename. It may be followed by the usual
-// ":123" for line numbers.
+// location must be a slash-separated filename, such as the one in
+// Location.Filename. It may be followed by the usual ":123" for line numbers.
//
// For diagnostics, use Logf instead.
func (l *Logger) Errorf(location string, format string, args ...interface{}) {
diff -r 63571a55c0d7 -r bc03635944ad pkgtools/pkglint/files/mklinechecker.go
--- a/pkgtools/pkglint/files/mklinechecker.go Sun Apr 28 17:11:53 2019 +0000
+++ b/pkgtools/pkglint/files/mklinechecker.go Sun Apr 28 18:13:53 2019 +0000
@@ -402,6 +402,63 @@
ck.MkLine.Explain(expl...)
}
+func (ck MkLineChecker) checkVarassignLeftRationale() {
+
+ isRationale := func(mkline MkLine) bool {
+ if mkline.IsVarassign() || mkline.IsCommentedVarassign() {
+ return mkline.VarassignComment() != ""
+ }
+ return mkline.IsComment() && !hasPrefix(mkline.Text, "# $")
+ }
+
+ needsRationale := func(mkline MkLine) bool {
+ if !mkline.IsVarassign() && !mkline.IsCommentedVarassign() {
+ return false
+ }
+ vartype := G.Pkgsrc.VariableType(ck.MkLines, mkline.Varname())
+ return vartype != nil && vartype.NeedsRationale()
+ }
+
+ mkline := ck.MkLine
+ if !needsRationale(mkline) {
+ return
+ }
+
+ if mkline.VarassignComment() != "" {
+ return
+ }
+
+ // Check whether there is a comment directly above.
+ for i, other := range ck.MkLines.mklines {
+ if other == mkline && i > 0 {
+ aboveIndex := i - 1
+ for aboveIndex > 0 && needsRationale(ck.MkLines.mklines[aboveIndex]) {
+ aboveIndex--
+ }
+
+ if isRationale(ck.MkLines.mklines[aboveIndex]) {
+ return
+ }
+ }
+ }
+
+ mkline.Warnf("Setting variable %s should have a rationale.", mkline.Varname())
+ mkline.Explain(
+ "Since this variable prevents the package from being built in some situations,",
+ "the reasons for this restriction should be documented.",
+ "Otherwise it becomes too difficult to check whether these restrictions still apply",
+ "when the package is updated by someone else later.",
+ "",
+ "To add the rationale, put it in a comment at the end of this line,",
+ "or in a separate comment in the line above.",
+ "The rationale should try to answer these questions:",
+ "",
+ "* which specific aspects of the package are affected?",
+ "* if it's a dependency, is the dependency too old or too new?",
+ "* in which situations does a crash occur, if any?",
+ "* has it been reported upstream?")
+}
+
// CheckVaruse checks a single use of a variable in a specific context.
func (ck MkLineChecker) CheckVaruse(varuse *MkVarUse, vuc *VarUseContext) {
mkline := ck.MkLine
@@ -932,6 +989,7 @@
if !ck.checkVarassignLeftUserSettable() {
ck.checkVarassignLeftPermissions()
}
+ ck.checkVarassignLeftRationale()
ck.checkTextVarUse(
ck.MkLine.Varname(),
diff -r 63571a55c0d7 -r bc03635944ad pkgtools/pkglint/files/mklinechecker_test.go
--- a/pkgtools/pkglint/files/mklinechecker_test.go Sun Apr 28 17:11:53 2019 +0000
+++ b/pkgtools/pkglint/files/mklinechecker_test.go Sun Apr 28 18:13:53 2019 +0000
@@ -736,6 +736,38 @@
t.CheckOutputEmpty()
}
+func (s *Suite) Test_MkLineChecker_checkVarassignLeftRationale(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpVartypes()
+ mklines := t.NewMkLines("filename.mk",
+ MkRcsID,
+ "ONLY_FOR_PLATFORM=\t*-*-*", // The CVS Id above is not a rationale.
+ "NOT_FOR_PLATFORM=\t*-*-*", // Neither does this line have a rationale.
+ "",
+ "ONLY_FOR_PLATFORM+=\t*-*-* # rationale",
+ "",
+ "# rationale in the line above",
+ "ONLY_FOR_PLATFORM+=\t*-*-*",
+ "",
+ "#VAR=\tvalue", // This comment is not a rationale.
+ "ONLY_FOR_PLATFORM+=\t*-*-*", // Needs a rationale.
+ "",
+ "# rationale",
+ "BROKEN_ON_PLATFORM+=\t*-*-*",
+ "BROKEN_ON_PLATFORM+=\t*-*-*", // The rationale applies to this line, too.
+ "",
+ "PKGNAME=\tpackage-1.0", // Does not need a rationale.
+ "UNKNOWN=\t${UNKNOWN}") // Unknown type, does not need a rationale.
+
+ mklines.Check()
+
+ t.CheckOutputLines(
+ "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.")
+}
+
func (s *Suite) Test_MkLineChecker_checkVarassignOpShell(c *check.C) {
t := s.Init(c)
@@ -1225,11 +1257,11 @@
t.SetUpVartypes()
mklines := t.NewMkLines("Makefile",
MkRcsID,
- "PYTHON_VERSIONS_ACCEPTED=\t36 __future__",
- "PYTHON_VERSIONS_ACCEPTED=\t36 -13",
- "PYTHON_VERSIONS_ACCEPTED=\t36 ${PKGVERSION_NOREV}",
- "PYTHON_VERSIONS_ACCEPTED=\t36 37",
- "PYTHON_VERSIONS_ACCEPTED=\t37 36 27 25")
+ "PYTHON_VERSIONS_ACCEPTED=\t36 __future__ # rationale",
+ "PYTHON_VERSIONS_ACCEPTED=\t36 -13 # rationale",
+ "PYTHON_VERSIONS_ACCEPTED=\t36 ${PKGVERSION_NOREV} # rationale",
+ "PYTHON_VERSIONS_ACCEPTED=\t36 37 # rationale",
+ "PYTHON_VERSIONS_ACCEPTED=\t37 36 27 25 # rationale")
// TODO: All but the last of the above assignments should be flagged as
// redundant by RedundantScope; as of March 2019, that check is only
diff -r 63571a55c0d7 -r bc03635944ad pkgtools/pkglint/files/pkgsrc_test.go
--- a/pkgtools/pkglint/files/pkgsrc_test.go Sun Apr 28 17:11:53 2019 +0000
+++ b/pkgtools/pkglint/files/pkgsrc_test.go Sun Apr 28 18:13:53 2019 +0000
@@ -199,6 +199,12 @@
t.CreateFileLines("mk/bsd.pkg.mk",
MkRcsID,
"_BUILD_DEFS+=\tPKG_SYSCONFBASEDIR PKG_SYSCONFDIR")
+ t.CreateFileLines("mk/defaults/mk.conf",
+ MkRcsID,
+ "",
+ "VARBASE=\t\t/var/pkg",
+ "PKG_SYSCONFBASEDIR=\t/usr/pkg/etc",
+ "PKG_SYSCONFDIR=\t/usr/pkg/etc")
t.FinishSetUp()
G.Check(pkg)
@@ -206,8 +212,9 @@
c.Check(G.Pkgsrc.IsBuildDef("PKG_SYSCONFDIR"), equals, true)
c.Check(G.Pkgsrc.IsBuildDef("VARBASE"), equals, false)
- // FIXME: There should be a warning for VARBASE, but G.Pkgsrc.UserDefinedVars
- // does not contain anything at mklinechecker.go:/UserDefinedVars/.
+ t.CheckOutputLines(
+ "WARN: ~/category/package/Makefile:21: " +
+ "The user-defined variable VARBASE is used but not added to BUILD_DEFS.")
}
func (s *Suite) Test_Pkgsrc_loadDocChanges__not_found(c *check.C) {
diff -r 63571a55c0d7 -r bc03635944ad pkgtools/pkglint/files/vardefs.go
--- a/pkgtools/pkglint/files/vardefs.go Sun Apr 28 17:11:53 2019 +0000
+++ b/pkgtools/pkglint/files/vardefs.go Sun Apr 28 18:13:53 2019 +0000
@@ -124,6 +124,14 @@
"Makefile, Makefile.*, *.mk: default, set, use")
}
+ // Like pkg, but always needs a rationale.
+ pkgrat := func(varname string, basicType *BasicType) {
+ acl(varname, basicType,
+ PackageSettable|NeedsRationale,
+ "buildlink3.mk, builtin.mk: none",
+ "Makefile, Makefile.*, *.mk: default, set, use")
+ }
+
// pkgload is the same as pkg, except that the variable may be accessed at load time.
pkgload := func(varname string, basicType *BasicType) {
acl(varname, basicType,
@@ -145,6 +153,14 @@
"Makefile, Makefile.*, *.mk: default, set, append, use")
}
+ // Like pkglist, but always needs a rationale.
+ pkglistrat := func(varname string, basicType *BasicType) {
+ acllist(varname, basicType,
+ List|PackageSettable|NeedsRationale,
+ "buildlink3.mk, builtin.mk: none",
+ "Makefile, Makefile.*, *.mk: default, set, append, use")
+ }
+
// pkgappend declares a variable that may use the += operator,
// even though it is not a list where each item can be interpreted
// on its own.
@@ -168,6 +184,14 @@
"Makefile, Makefile.*, *.mk: default, set, append, use")
}
+ // Like pkgappend, but always needs a rationale.
+ pkgappendrat := func(varname string, basicType *BasicType) {
+ acl(varname, basicType,
+ PackageSettable|NeedsRationale,
+ "buildlink3.mk, builtin.mk: none",
+ "Makefile, Makefile.*, *.mk: default, set, append, use")
+ }
+
// Some package-defined variables may be modified in buildlink3.mk files.
// These variables are typically related to compiling and linking files
// from C and related languages.
@@ -184,6 +208,13 @@
"Makefile, Makefile.*, *.mk: default, set, append, use")
}
+ // Like pkglistbl3, but always needs a rationale.
+ pkglistbl3rat := func(varname string, basicType *BasicType) {
+ acl(varname, basicType,
+ List|PackageSettable|NeedsRationale,
+ "Makefile, Makefile.*, *.mk: default, set, append, use")
+ }
+
// sys declares a user-defined or system-defined variable that must not
// be modified by packages.
//
@@ -776,10 +807,10 @@
pkglist("BOOTSTRAP_DEPENDS", BtDependencyWithPath)
pkg("BOOTSTRAP_PKG", BtYesNo)
// BROKEN should better be a list of messages instead of a simple string.
- pkgappend("BROKEN", BtMessage)
+ pkgappendrat("BROKEN", BtMessage)
pkg("BROKEN_GETTEXT_DETECTION", BtYesNo)
- pkglist("BROKEN_EXCEPT_ON_PLATFORM", BtMachinePlatformPattern)
- pkglist("BROKEN_ON_PLATFORM", BtMachinePlatformPattern)
+ pkglistrat("BROKEN_EXCEPT_ON_PLATFORM", BtMachinePlatformPattern)
+ pkglistrat("BROKEN_ON_PLATFORM", BtMachinePlatformPattern)
syslist("BSD_MAKE_ENV", BtShellWord)
// TODO: Align the permissions of the various BUILDLINK_*.* variables with each other.
acllist("BUILDLINK_ABI_DEPENDS.*", BtDependency,
@@ -1026,7 +1057,7 @@
pkglist("EMACS_VERSIONS_ACCEPTED", emacsVersions)
sys("EMACS_VERSION_MAJOR", BtInteger)
sys("EMACS_VERSION_MINOR", BtInteger)
- pkglist("EMACS_VERSION_REQD", emacsVersions)
+ pkglistrat("EMACS_VERSION_REQD", emacsVersions)
sys("EMULDIR", BtPathname)
sys("EMULSUBDIR", BtPathname)
sys("OPSYS_EMULDIR", BtPathname)
@@ -1070,17 +1101,17 @@
pkglist("FILES_SUBST", BtShellWord)
syslist("FILES_SUBST_SED", BtShellWord)
pkglist("FIX_RPATH", BtVariableName)
- pkglist("FLEX_REQD", BtVersion)
+ pkglistrat("FLEX_REQD", BtVersion)
pkglist("FONTS_DIRS.*", BtPathname)
syslist("GAMEDATA_PERMS", BtPerms)
syslist("GAMEDIR_PERMS", BtPerms)
- pkglistbl3("GCC_REQD", BtGccReqd)
+ pkglistbl3rat("GCC_REQD", BtGccReqd)
pkgappend("GENERATE_PLIST", BtShellCommands)
pkg("GITHUB_PROJECT", BtIdentifier)
pkg("GITHUB_TAG", BtIdentifier)
pkg("GITHUB_RELEASE", BtFileName)
pkg("GITHUB_TYPE", enum("tag release"))
- pkg("GMAKE_REQD", BtVersion)
+ pkgrat("GMAKE_REQD", BtVersion)
// Some packages need to set GNU_ARCH.i386 to either i486 or i586.
pkg("GNU_ARCH.*", BtIdentifier)
// GNU_CONFIGURE needs to be tested in some buildlink3.mk files,
@@ -1104,7 +1135,7 @@
PackageSettable,
"*: set, use-loadtime")
sys("IMAKE", BtShellCommand)
- pkglistbl3("INCOMPAT_CURSES", BtMachinePlatformPattern)
Home |
Main Index |
Thread Index |
Old Index