Updated pkglint to 5.4.10.

Changes since 5.4.9:

* Check for mismatch between conditional and unconditional includes
  of other files (mostly depending on PKG_OPTIONS or OPSYS)
* Check that PLIST files contain "man" instead of "${PKGMANDIR}"
This commit is contained in:
rillig 2016-11-01 21:40:25 +00:00
parent b29215dde2
commit 95724ea46c
10 changed files with 280 additions and 49 deletions

View file

@ -1,7 +1,6 @@
# $NetBSD: Makefile,v 1.498 2016/10/29 08:59:48 bsiegert Exp $
# $NetBSD: Makefile,v 1.499 2016/11/01 21:40:25 rillig Exp $
PKGNAME= pkglint-5.4.9.1
PKGREVISION= 1
PKGNAME= pkglint-5.4.10
DISTFILES= # none
CATEGORIES= pkgtools

View file

@ -34,9 +34,10 @@ type mkLineConditional struct {
args string
}
type mkLineInclude struct {
mustexist bool
indent string
includeFile string
mustexist bool
indent string
includeFile string
conditionVars string // (filled in later)
}
type mkLineDependency struct {
targets string
@ -119,13 +120,13 @@ func NewMkLine(line *Line) (mkline *MkLine) {
if m, indent, directive, includefile := match3(text, reMkInclude); m {
mkline.xtype = 6
mkline.data = mkLineInclude{directive == "include", indent, includefile}
mkline.data = mkLineInclude{directive == "include", indent, includefile, ""}
return
}
if m, indent, directive, includefile := match3(text, `^\.(\s*)(s?include)\s+<([^>]+)>\s*(?:#.*)?$`); m {
mkline.xtype = 7
mkline.data = mkLineInclude{directive == "include", indent, includefile}
mkline.data = mkLineInclude{directive == "include", indent, includefile, ""}
return
}
@ -208,7 +209,7 @@ func (mkline *MkLine) checkInclude() {
}
if mkline.Indent() != "" {
mkline.checkDirectiveIndentation()
mkline.checkDirectiveIndentation(G.Mk.indentation.Depth())
}
includefile := mkline.Includefile()
@ -260,7 +261,7 @@ func (mkline *MkLine) checkCond(forVars map[string]bool) {
indentation := &G.Mk.indentation
switch directive {
case "endif", "endfor", "elif", "else":
case "endif", "endfor":
if indentation.Len() > 1 {
indentation.Pop()
} else {
@ -268,12 +269,15 @@ func (mkline *MkLine) checkCond(forVars map[string]bool) {
}
}
mkline.checkDirectiveIndentation()
expectedDepth := indentation.Depth()
if directive == "elif" || directive == "else" {
expectedDepth = indentation.depth[len(indentation.depth)-2]
}
mkline.checkDirectiveIndentation(expectedDepth)
if directive == "if" && matches(args, `^!defined\([\w]+_MK\)$`) {
indentation.Push(indentation.Depth())
} else if matches(directive, `^(?:if|ifdef|ifndef|for|elif|else)$`) {
} else if matches(directive, `^(?:if|ifdef|ifndef|for)$`) {
indentation.Push(indentation.Depth() + 2)
}
@ -340,15 +344,14 @@ func (mkline *MkLine) checkCond(forVars map[string]bool) {
}
}
func (mkline *MkLine) checkDirectiveIndentation() {
func (mkline *MkLine) checkDirectiveIndentation(expectedDepth int) {
if G.Mk == nil {
return
}
indent := mkline.Indent()
indentation := G.Mk.indentation
if expected := strings.Repeat(" ", indentation.Depth()); indent != expected {
if expected := strings.Repeat(" ", expectedDepth); indent != expected {
if G.opts.WarnSpace && !mkline.Line.AutofixReplace("."+indent, "."+expected) {
mkline.Line.Notef("This directive should be indented by %d spaces.", indentation.Depth())
mkline.Line.Notef("This directive should be indented by %d spaces.", expectedDepth)
}
}
}
@ -1671,8 +1674,8 @@ func (vuc *VarUseContext) String() string {
}
type Indentation struct {
depth []int // Number of space characters; always a multiple of 2
conditionVars []map[string]bool // Variables on which the current path depends
depth []int // Number of space characters; always a multiple of 2
conditionVars [][]string // Variables on which the current path depends
}
func (ind *Indentation) Len() int {
@ -1696,17 +1699,46 @@ func (ind *Indentation) Push(indent int) {
func (ind *Indentation) AddVar(varname string) {
level := ind.Len() - 1
if ind.conditionVars[level] == nil {
ind.conditionVars[level] = make(map[string]bool)
if hasSuffix(varname, "_MK") {
return
}
ind.conditionVars[level][varname] = true
for _, existingVarname := range ind.conditionVars[level] {
if varname == existingVarname {
return
}
}
ind.conditionVars[level] = append(ind.conditionVars[level], varname)
}
func (ind *Indentation) DependsOn(varname string) bool {
for _, levelVarnames := range ind.conditionVars {
for _, levelVarname := range levelVarnames {
if varname == levelVarname {
return true
}
}
}
return false
}
func (ind *Indentation) IsConditional() bool {
for _, vars := range ind.conditionVars {
if vars[varname] {
if len(vars) > 0 {
return true
}
}
return false
}
func (ind *Indentation) Varnames() string {
sep := ""
varnames := ""
for _, levelVarnames := range ind.conditionVars {
for _, levelVarname := range levelVarnames {
varnames += sep + levelVarname
sep = ", "
}
}
return varnames
}

View file

@ -858,3 +858,43 @@ func (s *Suite) Test_MkLine_CheckVartype_CFLAGS(c *check.C) {
"WARN: Makefile:2: Unknown compiler flag \"-bs\".\n"+
"WARN: Makefile:2: Compiler flag \"%s\\\\\\\"\" should start with a hyphen.\n")
}
func (s *Suite) Test_Indentation(c *check.C) {
ind := &Indentation{}
ind.Push(0)
c.Check(ind.Depth(), equals, 0)
c.Check(ind.DependsOn("VARNAME"), equals, false)
ind.Push(2)
c.Check(ind.Depth(), equals, 2)
ind.AddVar("LEVEL1.VAR1")
c.Check(ind.Varnames(), equals, "LEVEL1.VAR1")
ind.AddVar("LEVEL1.VAR2")
c.Check(ind.Varnames(), equals, "LEVEL1.VAR1, LEVEL1.VAR2")
c.Check(ind.DependsOn("LEVEL1.VAR1"), equals, true)
c.Check(ind.DependsOn("OTHER_VAR"), equals, false)
ind.Push(2)
ind.AddVar("LEVEL2.VAR")
c.Check(ind.Varnames(), equals, "LEVEL1.VAR1, LEVEL1.VAR2, LEVEL2.VAR")
ind.Pop()
c.Check(ind.Varnames(), equals, "LEVEL1.VAR1, LEVEL1.VAR2")
c.Check(ind.IsConditional(), equals, true)
ind.Pop()
c.Check(ind.Varnames(), equals, "")
c.Check(ind.IsConditional(), equals, false)
}

View file

@ -123,6 +123,9 @@ func (mklines *MkLines) Check() {
case "bsd.prefs.mk", "bsd.fast.prefs.mk", "bsd.builtin.mk":
mklines.setSeenBsdPrefsMk()
}
if G.Pkg != nil {
G.Pkg.CheckInclude(mkline, indentation)
}
case mkline.IsCond():
mkline.checkCond(mklines.forVars)

View file

@ -413,3 +413,44 @@ func (s *Suite) Test_MkLines_PrivateTool_Defined(c *check.C) {
c.Check(s.Output(), equals, "")
}
func (s *Suite) Test_MkLines_Check_indentation(c *check.C) {
s.UseCommandLine(c, "-Wall")
mklines := s.NewMkLines("options.mk",
mkrcsid,
". if !defined(GUARD_MK)",
". if ${OPSYS} == ${OPSYS}",
". for i in ${FILES}",
". if !defined(GUARD2_MK)",
". else",
". endif",
". endfor",
". if ${COND1}",
". elif ${COND2}",
". else ${COND3}",
". endif",
". endif",
". endif",
". endif")
mklines.Check()
c.Check(s.Output(), equals, ""+
"NOTE: options.mk:2: This directive should be indented by 0 spaces.\n"+
"NOTE: options.mk:3: This directive should be indented by 0 spaces.\n"+
"NOTE: options.mk:4: This directive should be indented by 2 spaces.\n"+
"NOTE: options.mk:5: This directive should be indented by 4 spaces.\n"+
"NOTE: options.mk:6: This directive should be indented by 4 spaces.\n"+
"NOTE: options.mk:7: This directive should be indented by 4 spaces.\n"+
"NOTE: options.mk:8: This directive should be indented by 2 spaces.\n"+
"NOTE: options.mk:9: This directive should be indented by 2 spaces.\n"+
"NOTE: options.mk:10: This directive should be indented by 2 spaces.\n"+
"NOTE: options.mk:11: This directive should be indented by 2 spaces.\n"+
"ERROR: options.mk:11: \".else\" does not take arguments.\n"+
"NOTE: options.mk:11: If you meant \"else if\", use \".elif\".\n"+
"NOTE: options.mk:12: This directive should be indented by 2 spaces.\n"+
"NOTE: options.mk:13: This directive should be indented by 0 spaces.\n"+
"NOTE: options.mk:14: This directive should be indented by 0 spaces.\n"+
"ERROR: options.mk:15: Unmatched .endif.\n"+
"NOTE: options.mk:15: This directive should be indented by 0 spaces.\n")
}

View file

@ -22,24 +22,28 @@ type Package struct {
EffectivePkgnameLine *MkLine // The origin of the three effective_* values
SeenBsdPrefsMk bool // Has bsd.prefs.mk already been included?
vardef map[string]*MkLine // (varname, varcanon) => line
varuse map[string]*MkLine // (varname, varcanon) => line
bl3 map[string]*Line // buildlink3.mk name => line; contains only buildlink3.mk files that are directly included.
plistSubstCond map[string]bool // varname => true; list of all variables that are used as conditionals (@comment or nothing) in PLISTs.
included map[string]*Line // fname => line
seenMakefileCommon bool // Does the package have any .includes?
loadTimeTools map[string]bool // true=ok, false=not ok, absent=not mentioned in USE_TOOLS.
vardef map[string]*MkLine // (varname, varcanon) => line
varuse map[string]*MkLine // (varname, varcanon) => line
bl3 map[string]*Line // buildlink3.mk name => line; contains only buildlink3.mk files that are directly included.
plistSubstCond map[string]bool // varname => true; list of all variables that are used as conditionals (@comment or nothing) in PLISTs.
included map[string]*Line // fname => line
seenMakefileCommon bool // Does the package have any .includes?
loadTimeTools map[string]bool // true=ok, false=not ok, absent=not mentioned in USE_TOOLS.
conditionalIncludes map[string]*MkLine
unconditionalIncludes map[string]*MkLine
}
func NewPackage(pkgpath string) *Package {
pkg := &Package{
Pkgpath: pkgpath,
vardef: make(map[string]*MkLine),
varuse: make(map[string]*MkLine),
bl3: make(map[string]*Line),
plistSubstCond: make(map[string]bool),
included: make(map[string]*Line),
loadTimeTools: make(map[string]bool),
Pkgpath: pkgpath,
vardef: make(map[string]*MkLine),
varuse: make(map[string]*MkLine),
bl3: make(map[string]*Line),
plistSubstCond: make(map[string]bool),
included: make(map[string]*Line),
loadTimeTools: make(map[string]bool),
conditionalIncludes: make(map[string]*MkLine),
unconditionalIncludes: make(map[string]*MkLine),
}
for varname, line := range G.globalData.UserDefinedVars {
pkg.vardef[varname] = line
@ -834,3 +838,30 @@ func (pkg *Package) checkLocallyModified(fname string) {
}
}
}
func (pkg *Package) CheckInclude(mkline *MkLine, indentation *Indentation) {
if includeLine := mkline.data.(mkLineInclude); includeLine.conditionVars == "" {
includeLine.conditionVars = indentation.Varnames()
mkline.data = includeLine
}
if path.Dir(abspath(mkline.Line.Fname)) == abspath(G.CurrentDir) {
includefile := mkline.Includefile()
if indentation.IsConditional() {
pkg.conditionalIncludes[includefile] = mkline
if other := pkg.unconditionalIncludes[includefile]; other != nil {
dependingOn := mkline.data.(mkLineInclude).conditionVars
mkline.Line.Warnf("%q is included conditionally here (depending on %s) and unconditionally in %s.",
cleanpath(includefile), dependingOn, other.Line.ReferenceFrom(mkline.Line))
}
} else {
pkg.unconditionalIncludes[includefile] = mkline
if other := pkg.conditionalIncludes[includefile]; other != nil {
dependingOn := other.data.(mkLineInclude).conditionVars
mkline.Line.Warnf("%q is included unconditionally here and conditionally in %s (depending on %s).",
cleanpath(includefile), other.Line.ReferenceFrom(mkline.Line), dependingOn)
}
}
}
}

View file

@ -203,3 +203,47 @@ func (s *Suite) Test_Package_loadPackageMakefile(c *check.C) {
c.Check(s.Output(), equals, "")
}
func (s *Suite) Test_Package_conditionalAndUnconditionalInclude(c *check.C) {
G.globalData.InitVartypes()
s.CreateTmpFileLines(c, "category/package/Makefile",
mkrcsid,
"",
"COMMENT\t=Description",
"LICENSE\t= gnu-gpl-v2",
".include \"../../devel/zlib/buildlink3.mk\"",
".if ${OPSYS} == \"Linux\"",
".include \"../../sysutils/coreutils/buildlink3.mk\"",
".endif",
".include \"../../mk/bsd.pkg.mk\"")
s.CreateTmpFileLines(c, "category/package/options.mk",
mkrcsid,
"",
".if !empty(PKG_OPTIONS:Mzlib)",
". include \"../../devel/zlib/buildlink3.mk\"",
".endif",
".include \"../../sysutils/coreutils/buildlink3.mk\"")
s.CreateTmpFileLines(c, "category/package/PLIST",
"@comment $"+"NetBSD$",
"bin/program")
s.CreateTmpFileLines(c, "category/package/distinfo",
"$"+"NetBSD$")
s.CreateTmpFileLines(c, "devel/zlib/buildlink3.mk", "")
s.CreateTmpFileLines(c, "licenses/gnu-gpl-v2", "")
s.CreateTmpFileLines(c, "mk/bsd.pkg.mk", "")
s.CreateTmpFileLines(c, "sysutils/coreutils/buildlink3.mk", "")
pkg := NewPackage("category/package")
G.globalData.Pkgsrcdir = s.tmpdir
G.CurrentDir = s.tmpdir + "/category/package"
G.CurPkgsrcdir = "../.."
G.Pkg = pkg
checkdirPackage("category/package")
c.Check(s.Output(), equals, ""+
"WARN: ~/category/package/options.mk:3: Unknown option \"zlib\".\n"+
"WARN: ~/category/package/options.mk:4: \"../../devel/zlib/buildlink3.mk\" is included conditionally here (depending on PKG_OPTIONS) and unconditionally in Makefile:5.\n"+
"WARN: ~/category/package/options.mk:6: \"../../sysutils/coreutils/buildlink3.mk\" is included unconditionally here and conditionally in Makefile:7 (depending on OPSYS).\n")
}

View file

@ -316,18 +316,17 @@ func MatchVarassign(text string) (m bool, varname, spaceAfterVarname, op, valueA
varnameStart := i
for ; i < n; i++ {
b := text[i]
mask := uint(0)
switch b & 0xE0 {
case 0x20:
mask = 0x03ff6c10
case 0x40:
mask = 0x8ffffffe
case 0x60:
mask = 0x2ffffffe
}
if (mask>>(b&0x1F))&1 == 0 {
break
switch {
case 'A' <= b && b <= 'Z',
'a' <= b && b <= 'z',
b == '_',
'0' <= b && b <= '9',
'$' <= b && b <= '.' && (b == '$' || b == '*' || b == '+' || b == '-' || b == '.'),
b == '[',
b == '{', b == '}':
continue
}
break
}
varnameEnd := i

View file

@ -138,6 +138,14 @@ func (ck *PlistChecker) checkpath(pline *PlistLine) {
if contains(basename, "${IMAKE_MANNEWSUFFIX}") {
pline.warnImakeMannewsuffix()
}
if hasPrefix(text, "${PKGMANDIR}/") {
if !line.AutofixReplace("${PKGMANDIR}/", "man/") {
line.Note0("PLIST files should mention \"man/\" instead of \"${PKGMANDIR}\".")
Explain2(
"The pkgsrc infrastructure takes care of replacing the correct value",
"when generating the actual PLIST for the package.")
}
}
topdir := ""
if firstSlash := strings.IndexByte(text, '/'); firstSlash != -1 {

View file

@ -168,6 +168,16 @@ func (s *Suite) Test_PlistChecker_checkpathMan_gz(c *check.C) {
c.Check(s.Output(), equals, "NOTE: PLIST:2: The .gz extension is unnecessary for manual pages.\n")
}
func (s *Suite) TestPlistChecker_checkpath__PKGMANDIR(c *check.C) {
lines := s.NewLines("PLIST",
"@comment $"+"NetBSD$",
"${PKGMANDIR}/man1/sh.1")
ChecklinesPlist(lines)
c.Check(s.Output(), equals, "NOTE: PLIST:2: PLIST files should mention \"man/\" instead of \"${PKGMANDIR}\".\n")
}
func (s *Suite) Test_PlistChecker__autofix(c *check.C) {
s.UseCommandLine(c, "-Wall")
@ -177,6 +187,7 @@ func (s *Suite) Test_PlistChecker__autofix(c *check.C) {
"${PLIST.hal}lib/libvirt/connection-driver/libvirt_driver_nodedev.la",
"${PLIST.xen}lib/libvirt/connection-driver/libvirt_driver_libxl.la",
"lib/libvirt/lock-driver/lockd.la",
"${PKGMANDIR}/man1/sh.1",
"share/augeas/lenses/virtlockd.aug",
"share/doc/${PKGNAME}/html/32favicon.png",
"share/doc/${PKGNAME}/html/404.html",
@ -196,7 +207,8 @@ func (s *Suite) Test_PlistChecker__autofix(c *check.C) {
c.Check(s.Output(), equals, ""+
"WARN: ~/PLIST:3: \"lib/libvirt/connection-driver/libvirt_driver_nodedev.la\" should be sorted before \"lib/libvirt/connection-driver/libvirt_driver_storage.la\".\n"+
"WARN: ~/PLIST:4: \"lib/libvirt/connection-driver/libvirt_driver_libxl.la\" should be sorted before \"lib/libvirt/connection-driver/libvirt_driver_nodedev.la\".\n")
"WARN: ~/PLIST:4: \"lib/libvirt/connection-driver/libvirt_driver_libxl.la\" should be sorted before \"lib/libvirt/connection-driver/libvirt_driver_nodedev.la\".\n"+
"NOTE: ~/PLIST:6: PLIST files should mention \"man/\" instead of \"${PKGMANDIR}\".\n")
s.UseCommandLine(c, "-Wall", "--autofix")
ChecklinesPlist(lines)
@ -204,7 +216,29 @@ func (s *Suite) Test_PlistChecker__autofix(c *check.C) {
fixedLines := LoadExistingLines(fname, false)
c.Check(s.Output(), equals, ""+
"AUTOFIX: ~/PLIST:6: Replacing \"${PKGMANDIR}/\" with \"man/\".\n"+
"AUTOFIX: ~/PLIST:1: Sorting the whole file.\n"+
"AUTOFIX: ~/PLIST: Has been auto-fixed. Please re-run pkglint.\n")
c.Check(len(lines), equals, len(fixedLines))
c.Check(s.LoadTmpFile(c, "PLIST"), equals, ""+
"@comment $NetBSD: plist_test.go,v 1.9 2016/11/01 21:40:25 rillig Exp $\n"+
"${PLIST.xen}lib/libvirt/connection-driver/libvirt_driver_libxl.la\n"+
"${PLIST.hal}lib/libvirt/connection-driver/libvirt_driver_nodedev.la\n"+
"lib/libvirt/connection-driver/libvirt_driver_storage.la\n"+
"lib/libvirt/lock-driver/lockd.la\n"+
"man/man1/sh.1\n"+
"share/augeas/lenses/virtlockd.aug\n"+
"share/doc/${PKGNAME}/html/32favicon.png\n"+
"share/doc/${PKGNAME}/html/404.html\n"+
"share/doc/${PKGNAME}/html/acl.html\n"+
"share/doc/${PKGNAME}/html/aclpolkit.html\n"+
"share/doc/${PKGNAME}/html/windows.html\n"+
"share/examples/libvirt/libvirt.conf\n"+
"share/locale/zh_CN/LC_MESSAGES/libvirt.mo\n"+
"share/locale/zh_TW/LC_MESSAGES/libvirt.mo\n"+
"share/locale/zu/LC_MESSAGES/libvirt.mo\n"+
"@pkgdir share/examples/libvirt/nwfilter\n"+
"@pkgdir etc/libvirt/qemu/networks/autostart\n"+
"@pkgdir etc/logrotate.d\n"+
"@pkgdir etc/sasl2\n")
}