pkgsrc-Changes archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
CVS commit: pkgsrc/pkgtools/pkglint
Module Name: pkgsrc
Committed By: rillig
Date: Sat Dec 14 18:04:16 UTC 2019
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile
pkgsrc/pkgtools/pkglint/files: mkcondchecker_test.go substcontext.go
substcontext_test.go vardefs.go vartype.go vartypecheck.go
vartypecheck_test.go
Log Message:
pkgtools/pkglint: update to 19.3.18
Changes since 19.3.17:
The SUBST check has been completely rewritten. It can handle several
SUBST classes at the same time now. This reduces the number of wrong
warnings.
To generate a diff of this commit:
cvs rdiff -u -r1.618 -r1.619 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.2 -r1.3 pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go
cvs rdiff -u -r1.33 -r1.34 pkgsrc/pkgtools/pkglint/files/substcontext.go
cvs rdiff -u -r1.32 -r1.33 pkgsrc/pkgtools/pkglint/files/substcontext_test.go
cvs rdiff -u -r1.83 -r1.84 pkgsrc/pkgtools/pkglint/files/vardefs.go
cvs rdiff -u -r1.43 -r1.44 pkgsrc/pkgtools/pkglint/files/vartype.go
cvs rdiff -u -r1.73 -r1.74 pkgsrc/pkgtools/pkglint/files/vartypecheck.go
cvs rdiff -u -r1.66 -r1.67 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: pkgsrc/pkgtools/pkglint/Makefile
diff -u pkgsrc/pkgtools/pkglint/Makefile:1.618 pkgsrc/pkgtools/pkglint/Makefile:1.619
--- pkgsrc/pkgtools/pkglint/Makefile:1.618 Fri Dec 13 07:44:02 2019
+++ pkgsrc/pkgtools/pkglint/Makefile Sat Dec 14 18:04:15 2019
@@ -1,7 +1,6 @@
-# $NetBSD: Makefile,v 1.618 2019/12/13 07:44:02 bsiegert Exp $
+# $NetBSD: Makefile,v 1.619 2019/12/14 18:04:15 rillig Exp $
-PKGNAME= pkglint-19.3.17
-PKGREVISION= 1
+PKGNAME= pkglint-19.3.18
CATEGORIES= pkgtools
DISTNAME= tools
MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/}
Index: pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go:1.2 pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go:1.3
--- pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go:1.2 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go Sat Dec 14 18:04:15 2019
@@ -73,8 +73,7 @@ func (s *Suite) Test_MkCondChecker_Check
test(".if ${MACHINE_PLATFORM:MUnknownOS-*-*} || ${MACHINE_ARCH:Mx86}",
"WARN: filename.mk:4: "+
"The pattern \"UnknownOS\" cannot match any of "+
- "{ AIX BSDOS Bitrig Cygwin Darwin DragonFly FreeBSD FreeMiNT GNUkFreeBSD HPUX Haiku "+
- "IRIX Interix Linux Minix MirBSD NetBSD OSF1 OpenBSD QNX SCO_SV SunOS UnixWare "+
+ "{ Cygwin DragonFly FreeBSD Linux NetBSD SunOS "+
"} for the operating system part of MACHINE_PLATFORM.",
"WARN: filename.mk:4: "+
"The pattern \"x86\" cannot match any of "+
Index: pkgsrc/pkgtools/pkglint/files/substcontext.go
diff -u pkgsrc/pkgtools/pkglint/files/substcontext.go:1.33 pkgsrc/pkgtools/pkglint/files/substcontext.go:1.34
--- pkgsrc/pkgtools/pkglint/files/substcontext.go:1.33 Fri Dec 13 01:39:23 2019
+++ pkgsrc/pkgtools/pkglint/files/substcontext.go Sat Dec 14 18:04:15 2019
@@ -3,30 +3,25 @@ package pkglint
import "netbsd.org/pkglint/textproc"
// SubstContext records the state of a block of variable assignments
-// that make up a SUBST class (see `mk/subst.mk`).
+// that make up a SUBST class.
+//
+// See mk/subst.mk.
type SubstContext struct {
- queuedIds []string
- id string
- doneIds map[string]bool
+ active *substBlock
- foreignAllowed map[string]struct{}
- foreign []*MkLine
-
- conds []*substCond
+ scopes []*substScope
once Once
}
func NewSubstContext() *SubstContext {
- ctx := SubstContext{}
- ctx.reset()
- return &ctx
+ return &SubstContext{scopes: []*substScope{newSubstScope()}}
}
func (ctx *SubstContext) Process(mkline *MkLine) {
switch {
case mkline.IsEmpty():
- ctx.finishClass(mkline)
+ ctx.emptyLine()
case mkline.IsVarassign():
ctx.varassign(mkline)
case mkline.IsDirective():
@@ -35,8 +30,20 @@ func (ctx *SubstContext) Process(mkline
}
func (ctx *SubstContext) Finish(diag Diagnoser) {
- ctx.finishClass(diag)
- ctx.finishFile(diag)
+ // Prevent panics on unbalanced conditionals.
+ for len(ctx.scopes) > 1 {
+ ctx.leave(diag)
+ }
+
+ for _, scope := range ctx.scopes {
+ scope.finish(diag)
+ }
+}
+
+func (ctx *SubstContext) emptyLine() {
+ for _, scope := range ctx.scopes {
+ scope.emptyLine()
+ }
}
func (ctx *SubstContext) varassign(mkline *MkLine) {
@@ -47,8 +54,8 @@ func (ctx *SubstContext) varassign(mklin
}
if ctx.isForeign(mkline.Varcanon()) {
- if ctx.isActive() {
- ctx.rememberForeign(mkline)
+ if ctx.isActive() && !ctx.block().seenEmpty {
+ ctx.block().rememberForeign(mkline)
}
return
}
@@ -59,115 +66,341 @@ func (ctx *SubstContext) varassign(mklin
}
}
- if hasPrefix(mkline.Varname(), "SUBST_") && !ctx.isActiveId(mkline.Varparam()) {
+ if hasPrefix(mkline.Varname(), "SUBST_") && mkline.Varparam() != ctx.activeId() {
if !ctx.varassignDifferentClass(mkline) {
return
}
}
+ block := ctx.block()
switch varcanon {
case "SUBST_STAGE.*":
- ctx.varassignStage(mkline)
+ block.varassignStage(mkline)
case "SUBST_MESSAGE.*":
- ctx.varassignMessages(mkline)
+ block.varassignMessages(mkline)
case "SUBST_FILES.*":
- ctx.varassignFiles(mkline)
+ block.varassignFiles(mkline)
case "SUBST_SED.*":
- ctx.varassignSed(mkline)
+ block.varassignSed(mkline)
case "SUBST_VARS.*":
- ctx.varassignVars(mkline)
+ block.varassignVars(mkline)
case "SUBST_FILTER_CMD.*":
- ctx.varassignFilterCmd(mkline)
+ block.varassignFilterCmd(mkline)
}
}
func (ctx *SubstContext) varassignClasses(mkline *MkLine) {
- classes := mkline.ValueFields(mkline.WithoutMakeVariables(mkline.Value()))
- if len(classes) == 0 {
+ ids := mkline.ValueFields(mkline.WithoutMakeVariables(mkline.Value()))
+ if len(ids) == 0 {
return
}
- if len(classes) > 1 {
+ if len(ids) > 1 {
mkline.Notef("Please add only one class at a time to SUBST_CLASSES.")
mkline.Explain(
"This way, each substitution class forms a block in the package Makefile,",
"and to delete this block, it is not necessary to look anywhere else.")
}
- for _, class := range classes {
- ctx.queue(class)
- }
- id := classes[0]
- if ctx.isActive() && !ctx.isActiveId(id) {
- id := ctx.activeId() // since ctx.condEndif may reset it
+ ctx.prepareSubstClasses(mkline)
+ ctx.deactivate(mkline)
- for ctx.isConditional() {
- // This will be confusing for the outer SUBST block,
- // but since that block is assumed to be finished,
- // this doesn't matter.
- ctx.condEndif(mkline)
- }
-
- complete := ctx.isComplete() // since ctx.finishClass will reset it
- ctx.finishClass(mkline)
- if !complete {
- mkline.Warnf("Subst block %q should be finished before adding the next class to SUBST_CLASSES.", id)
+ for _, id := range ids {
+ if ctx.lookup(id) == nil {
+ ctx.scopes[len(ctx.scopes)-1].define(id)
+ } else if mkline.Varparam() == "" {
+ mkline.Errorf("Duplicate SUBST class %q.", id)
}
}
+}
- ctx.setActiveId(id)
-
- return
+func (ctx *SubstContext) prepareSubstClasses(mkline *MkLine) {
+ for _, scope := range ctx.scopes {
+ scope.prepareSubstClasses(mkline)
+ }
}
// varassignOutsideBlock handles variable assignments of SUBST variables that
// appear without a directly corresponding SUBST block.
func (ctx *SubstContext) varassignOutsideBlock(mkline *MkLine) (continueWithNewId bool) {
- varparam := mkline.Varparam()
+ id := mkline.Varparam()
- if ctx.isListCanon(mkline.Varcanon()) && ctx.isDone(varparam) {
+ if id != "" && ctx.isListCanon(mkline.Varcanon()) && ctx.isDone(id) {
if mkline.Op() != opAssignAppend {
mkline.Warnf("Late additions to a SUBST variable should use the += operator.")
}
return
}
- if containsWord(mkline.Rationale(), varparam) {
- return
+
+ return ctx.activate(mkline, ctx.lookup(id) == nil)
+}
+
+func (ctx *SubstContext) varassignDifferentClass(mkline *MkLine) (ok bool) {
+ varname := mkline.Varname()
+ unknown := ctx.lookup(mkline.Varparam()) == nil
+ if unknown && ctx.isActive() && !ctx.block().isComplete() {
+ mkline.Warnf("Variable %q does not match SUBST class %q.",
+ varname, ctx.activeId())
+ if !ctx.block().seenEmpty {
+ return false
+ }
+ }
+
+ return ctx.activate(mkline, unknown)
+}
+
+func (ctx *SubstContext) directive(mkline *MkLine) {
+ dir := mkline.Directive()
+ switch dir {
+ case "if":
+ ctx.enter()
+ case "elif":
+ ctx.nextBranch(mkline, false)
+ case "else":
+ ctx.nextBranch(mkline, true)
+ case "endif":
+ ctx.leave(mkline)
+ }
+}
+
+func (ctx *SubstContext) enter() {
+ for _, scope := range ctx.scopes {
+ scope.enter()
+ }
+ ctx.scopes = append(ctx.scopes, newSubstScope())
+}
+
+func (ctx *SubstContext) nextBranch(diag Diagnoser, isElse bool) {
+ if ctx.isActive() && !ctx.block().isConditional() {
+ ctx.deactivate(diag)
+ }
+
+ for _, scope := range ctx.scopes {
+ scope.nextBranch(diag, isElse)
+ }
+}
+
+func (ctx *SubstContext) leave(diag Diagnoser) {
+ ctx.deactivate(diag)
+
+ for _, scope := range ctx.scopes {
+ scope.leave(diag)
}
- if ctx.start(varparam) {
+ if len(ctx.scopes) > 1 {
+ ctx.scopes = ctx.scopes[:len(ctx.scopes)-1]
+ }
+}
+
+func (ctx *SubstContext) activate(mkline *MkLine, deactivate bool) bool {
+ id := mkline.Varparam()
+ if id == "" {
+ mkline.Errorf("Invalid SUBST class %q in variable name.", id)
+ return false
+ }
+
+ if deactivate {
+ ctx.deactivate(mkline)
+ }
+
+ if block := ctx.lookup(id); block != nil {
+ ctx.active = block
return true
}
- if ctx.once.FirstTime(varparam) {
+ if ctx.once.FirstTime(id) && !containsWord(mkline.Rationale(), id) {
mkline.Warnf("Before defining %s, the SUBST class "+
"should be declared using \"SUBST_CLASSES+= %s\".",
- mkline.Varname(), varparam)
+ mkline.Varname(), id)
}
- return
+ return false
}
-func (ctx *SubstContext) varassignDifferentClass(mkline *MkLine) (ok bool) {
- varname := mkline.Varname()
- varparam := mkline.Varparam()
+func (ctx *SubstContext) deactivate(diag Diagnoser) {
+ if !ctx.isActive() {
+ return
+ }
- if !ctx.isComplete() {
- mkline.Warnf("Variable %q does not match SUBST class %q.", varname, ctx.activeId())
+ block := ctx.block()
+ if !block.isConditional() {
+ block.finish(diag)
+ }
+ ctx.active = nil
+}
+
+func (*SubstContext) isForeign(varcanon string) bool {
+ switch varcanon {
+ case
+ "SUBST_STAGE.*",
+ "SUBST_MESSAGE.*",
+ "SUBST_FILES.*",
+ "SUBST_SED.*",
+ "SUBST_VARS.*",
+ "SUBST_FILTER_CMD.*":
return false
}
+ return true
+}
+
+func (*SubstContext) isListCanon(varcanon string) bool {
+ switch varcanon {
+ case
+ "SUBST_FILES.*",
+ "SUBST_SED.*",
+ "SUBST_VARS.*":
+ return true
+ }
+ return false
+}
- ctx.finishClass(mkline)
+func (ctx *SubstContext) block() *substBlock {
+ assertNotNil(ctx.active)
+ return ctx.active
+}
- ctx.start(varparam)
- return true
+func (ctx *SubstContext) lookup(id string) *substBlock {
+ for i := len(ctx.scopes) - 1; i >= 0; i-- {
+ if def := ctx.scopes[i].def(id); def != nil {
+ return def
+ }
+ }
+ return nil
+}
+
+func (ctx *SubstContext) isDone(id string) bool {
+ for _, scope := range ctx.scopes {
+ if scope.isDone(id) {
+ return true
+ }
+ }
+ return false
+}
+
+func (ctx *SubstContext) isActive() bool { return ctx.active != nil }
+
+func (ctx *SubstContext) activeId() string {
+ assert(ctx.isActive())
+ return ctx.active.id
+}
+
+type substScope struct {
+ defs []*substBlock
+}
+
+func newSubstScope() *substScope {
+ return &substScope{nil}
+}
+
+func (s *substScope) def(id string) *substBlock {
+ for _, def := range s.defs {
+ if def.id == id {
+ return def
+ }
+ }
+ return nil
+}
+
+func (s *substScope) define(id string) {
+ assert(s.def(id) == nil)
+ s.defs = append(s.defs, newSubstBlock(id))
+}
+
+func (s *substScope) isDone(id string) bool {
+ def := s.def(id)
+ return def != nil && def.done
}
-func (ctx *SubstContext) varassignStage(mkline *MkLine) {
- if ctx.isConditional() {
+func (s *substScope) emptyLine() {
+ for _, block := range s.defs {
+ block.seenEmpty = true
+ }
+}
+
+// finish brings all blocks that are defined in the current scope
+// to an end.
+func (s *substScope) finish(diag Diagnoser) {
+ for _, block := range s.defs {
+ block.finish(diag)
+ }
+}
+
+func (s *substScope) prepareSubstClasses(diag Diagnoser) {
+ for _, block := range s.defs {
+ if block.hasStarted() && !block.isComplete() {
+ diag.Warnf("Subst block %q should be finished "+
+ "before adding the next class to SUBST_CLASSES.",
+ block.id)
+ }
+ }
+}
+
+func (s *substScope) enter() {
+ for _, block := range s.defs {
+ block.enter()
+ }
+}
+
+func (s *substScope) nextBranch(diag Diagnoser, isElse bool) {
+ for _, block := range s.defs {
+ if block.isConditional() {
+ block.nextBranch(isElse)
+ } else {
+ block.finish(diag)
+ }
+ }
+}
+
+func (s *substScope) leave(diag Diagnoser) {
+ for _, block := range s.defs {
+ if block.isConditional() {
+ block.leave()
+ } else {
+ block.finish(diag)
+ }
+ }
+}
+
+type substBlock struct {
+ id string
+
+ // Records which of the SUBST variables have been seen either
+ // directly or in a conditional branch.
+ conds []*substCond
+
+ // In the paragraph of a SUBST block, there should be only variables
+ // that actually belong to the SUBST block.
+ //
+ // In addition, variables that are mentioned in SUBST_VARS may also
+ // be defined there because they closely relate to the SUBST block.
+ foreignAllowed map[string]struct{}
+ foreign []*MkLine
+
+ // Whether there has been an empty line between the SUBST_CLASSES
+ // line and the current line.
+ //
+ // Before the empty line, variables that don't obviously belong to
+ // this SUBST block generate warnings since they may be typos,
+ // such as for a different SUBST block.
+ seenEmpty bool
+
+ // Whether the SUBST_CLASSES has already gone out of scope.
+ //
+ // XXX: When it is out of scope, it should also be unreachable
+ // by any pkglint code. There's something inconsistent here.
+ done bool
+}
+
+func newSubstBlock(id string) *substBlock {
+ assert(id != "")
+ return &substBlock{id: id, conds: []*substCond{newSubstCond()}}
+}
+
+func (b *substBlock) varassignStage(mkline *MkLine) {
+ if b.isConditional() {
mkline.Warnf("%s should not be defined conditionally.", mkline.Varname())
}
- ctx.dupString(mkline, ssStage)
+ b.dupString(mkline, ssStage)
value := mkline.Value()
if value == "pre-patch" || value == "post-patch" {
@@ -196,84 +429,55 @@ func (ctx *SubstContext) varassignStage(
}
}
-func (ctx *SubstContext) varassignMessages(mkline *MkLine) {
+func (b *substBlock) varassignMessages(mkline *MkLine) {
varname := mkline.Varname()
- if ctx.isConditional() {
+ if b.isConditional() {
mkline.Warnf("%s should not be defined conditionally.", varname)
}
- ctx.dupString(mkline, ssMessage)
+ b.dupString(mkline, ssMessage)
}
-func (ctx *SubstContext) varassignFiles(mkline *MkLine) {
- ctx.dupList(mkline, ssFiles, ssNone)
+func (b *substBlock) varassignFiles(mkline *MkLine) {
+ b.dupList(mkline, ssFiles, ssNone)
}
-func (ctx *SubstContext) varassignSed(mkline *MkLine) {
- ctx.dupList(mkline, ssSed, ssNone)
- ctx.seen().set(ssTransform)
+func (b *substBlock) varassignSed(mkline *MkLine) {
+ b.dupList(mkline, ssSed, ssNone)
+ b.addSeen(ssTransform)
- ctx.suggestSubstVars(mkline)
+ b.suggestSubstVars(mkline)
}
-func (ctx *SubstContext) varassignVars(mkline *MkLine) {
- ctx.dupList(mkline, ssVars, ssVarsAutofix)
- ctx.seen().set(ssTransform)
+func (b *substBlock) varassignVars(mkline *MkLine) {
+ b.dupList(mkline, ssVars, ssVarsAutofix)
+ b.addSeen(ssTransform)
for _, substVar := range mkline.Fields() {
- ctx.allowVar(substVar)
+ b.allowVar(substVar)
}
}
-func (ctx *SubstContext) varassignFilterCmd(mkline *MkLine) {
- ctx.dupString(mkline, ssFilterCmd)
- ctx.seen().set(ssTransform)
+func (b *substBlock) varassignFilterCmd(mkline *MkLine) {
+ b.dupString(mkline, ssFilterCmd)
+ b.addSeen(ssTransform)
}
-func (ctx *SubstContext) dupList(mkline *MkLine, part substSeen, autofixPart substSeen) {
- if ctx.seenInBranch(part) && mkline.Op() != opAssignAppend {
- ctx.fixOperatorAppend(mkline, ctx.seenInBranch(autofixPart))
- }
- ctx.seen().set(part)
-}
-
-func (ctx *SubstContext) dupString(mkline *MkLine, part substSeen) {
- if ctx.seenInBranch(part) {
- mkline.Warnf("Duplicate definition of %q.", mkline.Varname())
- }
- ctx.seen().set(part)
-}
-
-func (ctx *SubstContext) fixOperatorAppend(mkline *MkLine, dueToAutofix bool) {
- before := mkline.ValueAlign()
- after := alignWith(mkline.Varname()+"+=", before)
-
- fix := mkline.Autofix()
- if dueToAutofix {
- fix.Notef(SilentAutofixFormat)
- } else {
- fix.Warnf("All but the first assignment to %q should use the \"+=\" operator.",
- mkline.Varname())
- }
- fix.Replace(before, after)
- fix.Apply()
-}
-
-func (ctx *SubstContext) suggestSubstVars(mkline *MkLine) {
+func (b *substBlock) suggestSubstVars(mkline *MkLine) {
tokens, _ := splitIntoShellTokens(mkline.Line, mkline.Value())
for _, token := range tokens {
- varname := ctx.extractVarname(mkline.UnquoteShell(token, false))
+ varname := b.extractVarname(mkline.UnquoteShell(token, false))
if varname == "" {
continue
}
- id := ctx.activeId()
+ id := b.id
varop := sprintf("SUBST_VARS.%s%s%s",
id,
condStr(hasSuffix(id, "+"), " ", ""),
- condStr(ctx.seenInBranch(ssVars), "+=", "="))
+ condStr(b.hasSeen(ssVars), "+=", "="))
fix := mkline.Autofix()
fix.Notef("The substitution command %q can be replaced with \"%s %s\".",
@@ -291,13 +495,43 @@ func (ctx *SubstContext) suggestSubstVar
// assignment operators on them. It's probably not worth the
// effort, though.
- ctx.seen().set(ssVars | ssVarsAutofix)
+ b.addSeen(ssVars)
+ b.addSeen(ssVarsAutofix)
+ }
+}
+
+func (b *substBlock) dupString(mkline *MkLine, part substSeen) {
+ if b.hasSeen(part) {
+ mkline.Warnf("Duplicate definition of %q.", mkline.Varname())
+ }
+ b.addSeen(part)
+}
+
+func (b *substBlock) dupList(mkline *MkLine, part substSeen, autofixPart substSeen) {
+ if b.hasSeenAnywhere(part) && mkline.Op() != opAssignAppend {
+ b.fixOperatorAppend(mkline, b.hasSeen(autofixPart))
+ }
+ b.addSeen(part)
+}
+
+func (b *substBlock) fixOperatorAppend(mkline *MkLine, dueToAutofix bool) {
+ before := mkline.ValueAlign()
+ after := alignWith(mkline.Varname()+"+=", before)
+
+ fix := mkline.Autofix()
+ if dueToAutofix {
+ fix.Notef(SilentAutofixFormat)
+ } else {
+ fix.Warnf("All but the first assignment to %q should use the \"+=\" operator.",
+ mkline.Varname())
}
+ fix.Replace(before, after)
+ fix.Apply()
}
// extractVarname extracts the variable name from a sed command of the form
// s,@VARNAME@,${VARNAME}, and some related variants thereof.
-func (*SubstContext) extractVarname(token string) string {
+func (*substBlock) extractVarname(token string) string {
parser := NewMkLexer(token, nil)
lexer := parser.lexer
if !lexer.SkipByte('s') {
@@ -337,235 +571,111 @@ func (*SubstContext) extractVarname(toke
return varname
}
-func (*SubstContext) isForeign(varcanon string) bool {
- switch varcanon {
- case
- "SUBST_STAGE.*",
- "SUBST_MESSAGE.*",
- "SUBST_FILES.*",
- "SUBST_SED.*",
- "SUBST_VARS.*",
- "SUBST_FILTER_CMD.*":
- return false
- }
- return true
+func (b *substBlock) isComplete() bool {
+ return b.allSeen().hasAll(ssStage | ssFiles | ssTransform)
}
-func (*SubstContext) isListCanon(varcanon string) bool {
- switch varcanon {
- case
- "SUBST_FILES.*",
- "SUBST_SED.*",
- "SUBST_VARS.*":
- return true
+func (b *substBlock) hasSeen(part substSeen) bool {
+ for _, cond := range b.conds {
+ if cond.hasSeen(part) {
+ return true
+ }
}
return false
}
-func (ctx *SubstContext) directive(mkline *MkLine) {
- dir := mkline.Directive()
- switch dir {
- case "if":
- ctx.condIf()
-
- case "elif", "else":
- ctx.condElse(mkline, dir)
-
- case "endif":
- ctx.condEndif(mkline)
+func (b *substBlock) hasSeenAnywhere(part substSeen) bool {
+ for _, cond := range b.conds {
+ if cond.hasSeenAnywhere(part) {
+ return true
+ }
}
+ return false
}
-func (ctx *SubstContext) condIf() {
- top := substCond{total: ssAll}
- ctx.conds = append(ctx.conds, &top)
+func (b *substBlock) allSeen() substSeen {
+ all := ssNone
+ for _, cond := range b.conds {
+ all.addAll(cond.curr)
+ }
+ return all
}
-func (ctx *SubstContext) condElse(mkline *MkLine, dir string) {
- top := ctx.cond()
- top.total.retain(top.curr)
- if !ctx.isConditional() {
- ctx.finishClass(mkline)
- }
- top.curr = ssNone
- top.seenElse = dir == "else"
+func (b *substBlock) addSeen(part substSeen) {
+ b.conds[len(b.conds)-1].addSeen(part)
}
-func (ctx *SubstContext) condEndif(diag Diagnoser) {
- top := ctx.cond()
- top.total.retain(top.curr)
- if !ctx.isConditional() {
- ctx.finishClass(diag)
- }
- if !top.seenElse {
- top.total = ssNone
- }
- if len(ctx.conds) > 1 {
- ctx.conds = ctx.conds[:len(ctx.conds)-1]
- }
- ctx.seen().union(top.total)
+func (b *substBlock) rememberForeign(mkline *MkLine) {
+ b.foreign = append(b.foreign, mkline)
}
-func (ctx *SubstContext) finishClass(diag Diagnoser) {
- if !ctx.isActive() {
- return
- }
+// isConditional returns whether the current line is at a deeper conditional
+// level than the corresponding SUBST_CLASSES line.
+func (b *substBlock) isConditional() bool { return len(b.conds) > 1 }
- if ctx.seen().get(ssAll) {
- ctx.checkBlockComplete(diag)
- ctx.checkForeignVariables()
- } else {
- ctx.markAsNotDone()
+func (b *substBlock) allowVar(varname string) {
+ if b.foreignAllowed == nil {
+ b.foreignAllowed = map[string]struct{}{}
}
-
- ctx.reset()
+ b.foreignAllowed[varname] = struct{}{}
}
-func (ctx *SubstContext) checkBlockComplete(diag Diagnoser) {
- id := ctx.activeId()
- seen := ctx.seen()
- if !seen.get(ssStage) {
- diag.Warnf("Incomplete SUBST block: SUBST_STAGE.%s missing.", id)
- }
- if !seen.get(ssFiles) {
- diag.Warnf("Incomplete SUBST block: SUBST_FILES.%s missing.", id)
- }
- if !seen.get(ssTransform) {
- diag.Warnf("Incomplete SUBST block: SUBST_SED.%[1]s, SUBST_VARS.%[1]s or SUBST_FILTER_CMD.%[1]s missing.", id)
- }
-}
+func (b *substBlock) enter() { b.conds = append(b.conds, newSubstCond()) }
-func (ctx *SubstContext) checkForeignVariables() {
- ctx.forEachForeignVar(func(mkline *MkLine) {
- mkline.Warnf("Foreign variable %q in SUBST block.", mkline.Varname())
- })
+func (b *substBlock) nextBranch(isElse bool) {
+ cond := b.conds[len(b.conds)-1]
+ cond.leaveBranch()
+ cond.enterBranch(isElse)
}
-func (ctx *SubstContext) finishFile(diag Diagnoser) {
- for _, id := range ctx.queuedIds {
- if id != "" && !ctx.isDone(id) {
- ctx.warnUndefinedBlock(diag, id)
- }
- }
-}
+func (b *substBlock) leave() {
+ assert(b.isConditional())
-func (*SubstContext) warnUndefinedBlock(diag Diagnoser, id string) {
- diag.Warnf("Missing SUBST block for %q.", id)
- diag.Explain(
- "After adding a SUBST class to SUBST_CLASSES,",
- "the remaining SUBST variables should be defined in the same file.",
- "",
- "See mk/subst.mk for the comprehensive documentation.")
+ n := len(b.conds)
+ b.conds[n-2].leaveLevel(b.conds[n-1])
+ b.conds = b.conds[:n-1]
}
-// In the paragraph of a SUBST block, there should be only variables
-// that actually belong to the SUBST block.
-//
-// In addition, variables that are mentioned in SUBST_VARS may also
-// be defined there because they closely relate to the SUBST block.
+func (b *substBlock) finish(diag Diagnoser) {
+ assert(len(b.conds) == 1)
-func (ctx *SubstContext) allowVar(varname string) {
- if ctx.foreignAllowed == nil {
- ctx.foreignAllowed = make(map[string]struct{})
+ if b.done {
+ return
}
- ctx.foreignAllowed[varname] = struct{}{}
-}
+ b.done = true
-func (ctx *SubstContext) rememberForeign(mkline *MkLine) {
- ctx.foreign = append(ctx.foreign, mkline)
-}
-
-// forEachForeignVar performs the given action for each variable that
-// is defined in the SUBST block and is not mentioned in SUBST_VARS.
-func (ctx *SubstContext) forEachForeignVar(action func(*MkLine)) {
- for _, foreign := range ctx.foreign {
- if _, ok := ctx.foreignAllowed[foreign.Varname()]; !ok {
- action(foreign)
- }
+ if !b.hasStarted() {
+ diag.Warnf("Missing SUBST block for %q.", b.id)
+ return
}
-}
-
-func (ctx *SubstContext) reset() {
- ctx.id = ""
- ctx.foreignAllowed = nil
- ctx.foreign = nil
- ctx.conds = []*substCond{{seenElse: true}}
-}
-
-func (ctx *SubstContext) isActive() bool { return ctx.id != "" }
-
-func (ctx *SubstContext) isActiveId(id string) bool { return ctx.id == id }
-
-func (ctx *SubstContext) activeId() string {
- assert(ctx.isActive())
- return ctx.id
-}
-
-func (ctx *SubstContext) setActiveId(id string) {
- ctx.id = id
- ctx.cond().top = true
- ctx.markAsDone(id)
-}
-func (ctx *SubstContext) queue(id string) {
- ctx.queuedIds = append(ctx.queuedIds, id)
-}
-
-func (ctx *SubstContext) start(id string) bool {
- for i, queuedId := range ctx.queuedIds {
- if queuedId == id {
- ctx.queuedIds[i] = ""
- ctx.setActiveId(id)
- return true
- }
+ if !b.hasSeen(ssStage) {
+ diag.Warnf("Incomplete SUBST block: SUBST_STAGE.%s missing.", b.id)
}
- return false
-}
-
-func (ctx *SubstContext) markAsDone(id string) {
- if ctx.doneIds == nil {
- ctx.doneIds = map[string]bool{}
+ if !b.hasSeen(ssFiles) {
+ diag.Warnf("Incomplete SUBST block: SUBST_FILES.%s missing.", b.id)
+ }
+ if !b.hasSeen(ssTransform) {
+ diag.Warnf(
+ "Incomplete SUBST block: SUBST_SED.%[1]s, "+
+ "SUBST_VARS.%[1]s or SUBST_FILTER_CMD.%[1]s missing.",
+ b.id)
}
- ctx.doneIds[id] = true
-}
-
-func (ctx *SubstContext) markAsNotDone() {
- ctx.doneIds[ctx.id] = false
-}
-
-func (ctx *SubstContext) isDone(varparam string) bool {
- return ctx.doneIds[varparam]
-}
-
-func (ctx *SubstContext) isComplete() bool {
- return ctx.seen().hasAll(ssStage | ssFiles | ssTransform)
-}
-
-// isConditional returns whether the current line is at a deeper conditional
-// level than the assignment where the corresponding class ID is added to
-// SUBST_CLASSES.
-//
-// TODO: Adjust the implementation to this description.
-func (ctx *SubstContext) isConditional() bool {
- return !ctx.cond().top
-}
-// cond returns information about the parts of the SUBST block that
-// have already been seen in the current leaf branch of the conditionals.
-func (ctx *SubstContext) seen() *substSeen {
- return &ctx.cond().curr
+ b.checkForeignVariables()
}
-// cond returns information about the current branch of conditionals.
-func (ctx *SubstContext) cond() *substCond {
- return ctx.conds[len(ctx.conds)-1]
+func (b *substBlock) checkForeignVariables() {
+ for _, mkline := range b.foreign {
+ if _, ok := b.foreignAllowed[mkline.Varname()]; !ok {
+ mkline.Warnf("Foreign variable %q in SUBST block.", mkline.Varname())
+ }
+ }
}
-// Returns true if the given flag from substSeen has been seen
-// somewhere in the conditional path of the current line.
-func (ctx *SubstContext) seenInBranch(part substSeen) bool {
- for _, cond := range ctx.conds {
- if cond.curr.get(part) {
+func (b *substBlock) hasStarted() bool {
+ for _, cond := range b.conds {
+ if cond.currAny != ssNone {
return true
}
}
@@ -573,23 +683,16 @@ func (ctx *SubstContext) seenInBranch(pa
}
type substCond struct {
- // Tells whether a SUBST block has started at this conditional level.
- // All variable assignments that belong to this class must happen at
- // this conditional level or below it.
- //
- // TODO: For Test_SubstContext_Directive__conditional_complete,
- // this needs to be changed to the set of classes that have been
- // added to SUBST_CLASSES at this level.
- top bool
-
// Collects the parts of the SUBST block that have been defined in all
// branches that have been parsed completely.
- total substSeen
+ total substSeen
+ totalAny substSeen
// Collects the parts of the SUBST block that are defined in the current
// branch of the conditional. At the end of the branch, they are merged
// into the total.
- curr substSeen
+ curr substSeen
+ currAny substSeen
// Marks whether the current conditional statement has
// an .else branch. If it doesn't, this means that all variables
@@ -597,6 +700,40 @@ type substCond struct {
seenElse bool
}
+func newSubstCond() *substCond { return &substCond{total: ssAll} }
+
+func (c *substCond) enterBranch(isElse bool) {
+ c.curr = ssNone
+ c.currAny = ssNone
+ c.seenElse = isElse
+}
+
+func (c *substCond) leaveBranch() {
+ c.total.retainAll(c.curr)
+ c.totalAny.addAll(c.currAny)
+ c.curr = ssNone
+ c.currAny = ssNone
+}
+
+func (c *substCond) leaveLevel(child *substCond) {
+ child.leaveBranch()
+ if child.seenElse {
+ c.curr.addAll(child.total)
+ }
+ c.currAny.addAll(child.totalAny)
+}
+
+func (c *substCond) hasSeen(part substSeen) bool { return c.curr.has(part) }
+
+func (c *substCond) hasSeenAnywhere(part substSeen) bool {
+ return c.currAny.has(part)
+}
+
+func (c *substCond) addSeen(part substSeen) {
+ c.curr.set(part)
+ c.currAny.set(part)
+}
+
// substSeen contains all variables that depend on a particular SUBST
// class ID. These variables can be set in conditional branches, and
// pkglint keeps track whether they are set in all branches or only
@@ -617,8 +754,16 @@ const (
ssNone substSeen = 0
)
-func (s *substSeen) set(part substSeen) { *s |= part }
-func (s *substSeen) get(part substSeen) bool { return *s&part != 0 }
-func (s *substSeen) hasAll(other substSeen) bool { return *s&other == other }
-func (s *substSeen) union(other substSeen) { *s |= other }
-func (s *substSeen) retain(other substSeen) { *s &= other }
+func (s *substSeen) set(part substSeen) {
+ assert(part&(part-1) == 0)
+ *s |= part
+}
+
+func (s *substSeen) has(part substSeen) bool {
+ assert(part&(part-1) == 0)
+ return *s&part != 0
+}
+
+func (s substSeen) hasAll(other substSeen) bool { return s&other == other }
+func (s *substSeen) addAll(other substSeen) { *s |= other }
+func (s *substSeen) retainAll(other substSeen) { *s &= other }
Index: pkgsrc/pkgtools/pkglint/files/substcontext_test.go
diff -u pkgsrc/pkgtools/pkglint/files/substcontext_test.go:1.32 pkgsrc/pkgtools/pkglint/files/substcontext_test.go:1.33
--- pkgsrc/pkgtools/pkglint/files/substcontext_test.go:1.32 Fri Dec 13 01:39:23 2019
+++ pkgsrc/pkgtools/pkglint/files/substcontext_test.go Sat Dec 14 18:04:15 2019
@@ -5,6 +5,7 @@ import "gopkg.in/check.v1"
func (t *Tester) NewSubstAutofixTest(lines ...string) func(bool) {
return func(autofix bool) {
mklines := t.NewMkLines("filename.mk", lines...)
+ mklines.collectRationale()
ctx := NewSubstContext()
mklines.ForEach(ctx.Process)
@@ -18,111 +19,13 @@ func (t *Tester) RunSubst(lines ...strin
assert(lines[len(lines)-1] != "")
mklines := t.NewMkLines("filename.mk", lines...)
+ mklines.collectRationale()
ctx := NewSubstContext()
mklines.ForEach(ctx.Process)
ctx.Finish(mklines.EOFLine())
}
-func (s *Suite) Test_SubstContext__OPSYSVARS(c *check.C) {
- t := s.Init(c)
-
- ctx := NewSubstContext()
-
- // SUBST_CLASSES is added to OPSYSVARS in mk/bsd.pkg.mk.
- ctx.varassign(t.NewMkLine("filename.mk", 11, "SUBST_CLASSES.SunOS+=prefix"))
- ctx.varassign(t.NewMkLine("filename.mk", 12, "SUBST_CLASSES.NetBSD+=prefix"))
- ctx.varassign(t.NewMkLine("filename.mk", 13, "SUBST_FILES.prefix=Makefile"))
- ctx.varassign(t.NewMkLine("filename.mk", 14, "SUBST_SED.prefix=s,@PREFIX@,${PREFIX},g"))
- ctx.varassign(t.NewMkLine("filename.mk", 15, "SUBST_STAGE.prefix=post-configure"))
-
- t.CheckEquals(ctx.isComplete(), true)
-
- ctx.Finish(t.NewMkLine("filename.mk", 15, ""))
-
- t.CheckOutputLines(
- "NOTE: filename.mk:14: The substitution command \"s,@PREFIX@,${PREFIX},g\" " +
- "can be replaced with \"SUBST_VARS.prefix= PREFIX\".")
-}
-
-func (s *Suite) Test_SubstContext__no_class(c *check.C) {
- t := s.Init(c)
-
- t.RunSubst(
- "UNRELATED=anything",
- "SUBST_FILES.repl+=Makefile.in",
- "SUBST_SED.repl+=-e s,from,to,g")
-
- t.CheckOutputLines(
- "WARN: filename.mk:2: Before defining SUBST_FILES.repl, " +
- "the SUBST class should be declared using \"SUBST_CLASSES+= repl\".")
-}
-
-func (s *Suite) Test_SubstContext__multiple_classes_in_one_line(c *check.C) {
- t := s.Init(c)
-
- t.RunSubst(
- "SUBST_CLASSES+= one two",
- "SUBST_STAGE.one= post-configure",
- "SUBST_FILES.one= one.txt",
- "SUBST_SED.one= s,one,1,g",
- "SUBST_STAGE.two= post-configure",
- "SUBST_FILES.two= two.txt")
-
- t.CheckOutputLines(
- "NOTE: filename.mk:1: Please add only one class at a time to SUBST_CLASSES.",
- "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_SED.two, SUBST_VARS.two or SUBST_FILTER_CMD.two missing.")
-}
-
-func (s *Suite) Test_SubstContext__multiple_classes_in_one_line_multiple_blocks(c *check.C) {
- t := s.Init(c)
-
- t.RunSubst(
- "SUBST_CLASSES+= one two",
- "SUBST_STAGE.one= post-configure",
- "SUBST_FILES.one= one.txt",
- "SUBST_SED.one= s,one,1,g",
- "",
- "SUBST_STAGE.two= post-configure",
- "SUBST_FILES.two= two.txt",
- "",
- "SUBST_STAGE.three= post-configure",
- "",
- "SUBST_VARS.four= PREFIX",
- "",
- "SUBST_VARS.three= PREFIX")
-
- t.CheckOutputLines(
- "NOTE: filename.mk:1: Please add only one class at a time to SUBST_CLASSES.",
- "WARN: filename.mk:8: Incomplete SUBST block: "+
- "SUBST_SED.two, SUBST_VARS.two or SUBST_FILTER_CMD.two missing.",
- "WARN: filename.mk:9: Before defining SUBST_STAGE.three, "+
- "the SUBST class should be declared using \"SUBST_CLASSES+= three\".",
- "WARN: filename.mk:11: Before defining SUBST_VARS.four, "+
- "the SUBST class should be declared using \"SUBST_CLASSES+= four\".")
-}
-
-func (s *Suite) Test_SubstContext__multiple_classes_in_one_block(c *check.C) {
- t := s.Init(c)
-
- t.RunSubst(
- "SUBST_CLASSES+= one",
- "SUBST_STAGE.one= post-configure",
- "SUBST_STAGE.one= post-configure",
- "SUBST_FILES.one= one.txt",
- "SUBST_CLASSES+= two", // The block "one" is not finished yet.
- "SUBST_SED.one= s,one,1,g",
- "SUBST_STAGE.two= post-configure",
- "SUBST_FILES.two= two.txt",
- "SUBST_SED.two= s,two,2,g")
-
- t.CheckOutputLines(
- "WARN: filename.mk:3: Duplicate definition of \"SUBST_STAGE.one\".",
- "WARN: filename.mk:5: Incomplete SUBST block: SUBST_SED.one, SUBST_VARS.one or SUBST_FILTER_CMD.one missing.",
- "WARN: filename.mk:5: Subst block \"one\" should be finished before adding the next class to SUBST_CLASSES.",
- "WARN: filename.mk:6: Variable \"SUBST_SED.one\" does not match SUBST class \"two\".")
-}
-
// This is a strange example that probably won't occur in practice.
//
// Continuing a SUBST class in one of the branches and starting
@@ -147,26 +50,7 @@ func (s *Suite) Test_SubstContext__parti
t.CheckOutputEmpty()
}
-func (s *Suite) Test_SubstContext__files_missing(c *check.C) {
- t := s.Init(c)
-
- t.RunSubst(
- "SUBST_CLASSES+= one",
- "SUBST_STAGE.one= pre-configure",
- "SUBST_CLASSES+= two",
- "SUBST_STAGE.two= pre-configure",
- "SUBST_FILES.two= two.txt",
- "SUBST_SED.two= s,two,2,g")
-
- t.CheckOutputLines(
- "WARN: filename.mk:3: Incomplete SUBST block: SUBST_FILES.one missing.",
- "WARN: filename.mk:3: Incomplete SUBST block: "+
- "SUBST_SED.one, SUBST_VARS.one or SUBST_FILTER_CMD.one missing.",
- "WARN: filename.mk:3: Subst block \"one\" should be finished "+
- "before adding the next class to SUBST_CLASSES.")
-}
-
-func (s *Suite) Test_SubstContext__directives(c *check.C) {
+func (s *Suite) Test_SubstContext__conditionals(c *check.C) {
t := s.Init(c)
t.RunSubst(
@@ -193,88 +77,75 @@ func (s *Suite) Test_SubstContext__direc
"to \"SUBST_SED.os\" should use the \"+=\" operator.")
}
-func (s *Suite) Test_SubstContext__adjacent(c *check.C) {
- t := s.Init(c)
-
- t.RunSubst(
- "SUBST_CLASSES+=\t1",
- "SUBST_STAGE.1=\tpre-configure",
- "SUBST_FILES.1=\tfile1",
- "SUBST_SED.1=\t-e s,subst1,repl1,",
- "SUBST_CLASSES+=\t2",
- "SUBST_SED.1+=\t-e s,subst1b,repl1b,", // Misplaced
- "SUBST_STAGE.2=\tpre-configure",
- "SUBST_FILES.2=\tfile2",
- "SUBST_SED.2=\t-e s,subst2,repl2,")
-
- t.CheckOutputLines(
- "WARN: filename.mk:6: Variable \"SUBST_SED.1\" does not match SUBST class \"2\".")
-}
-
-func (s *Suite) Test_SubstContext__do_patch(c *check.C) {
- t := s.Init(c)
-
- t.RunSubst(
- "SUBST_CLASSES+=\tos",
- "SUBST_STAGE.os=\tdo-patch",
- "SUBST_FILES.os=\tguess-os.h",
- "SUBST_SED.os=\t-e s,@OPSYS@,Darwin,")
-
- // No warning, since there is nothing to fix automatically.
- // This case doesn't occur in practice anyway.
- t.CheckOutputEmpty()
-}
-
-// Variables mentioned in SUBST_VARS are not considered "foreign"
-// in the block and may be mixed with the other SUBST variables.
-func (s *Suite) Test_SubstContext__SUBST_VARS_defined_in_block(c *check.C) {
+func (s *Suite) Test_SubstContext_varassign__no_class(c *check.C) {
t := s.Init(c)
t.RunSubst(
- "SUBST_CLASSES+=\tos",
- "SUBST_STAGE.os=\tpre-configure",
- "SUBST_FILES.os=\tguess-os.h",
- "SUBST_VARS.os=\tTODAY1",
- "TODAY1!=\tdate",
- "TODAY2!=\tdate")
+ "UNRELATED=anything",
+ "SUBST_FILES.repl+=Makefile.in",
+ "SUBST_SED.repl+=-e s,from,to,g")
t.CheckOutputLines(
- "WARN: filename.mk:6: Foreign variable \"TODAY2\" in SUBST block.")
+ "WARN: filename.mk:2: Before defining SUBST_FILES.repl, " +
+ "the SUBST class should be declared using \"SUBST_CLASSES+= repl\".")
}
-// Variables mentioned in SUBST_VARS may appear in the same paragraph,
-// or alternatively anywhere else in the file.
-func (s *Suite) Test_SubstContext__SUBST_VARS_in_next_paragraph(c *check.C) {
+func (s *Suite) Test_SubstContext_varassign__multiple_classes_in_one_line_multiple_blocks(c *check.C) {
t := s.Init(c)
t.RunSubst(
- "SUBST_CLASSES+=\tos",
- "SUBST_STAGE.os=\tpre-configure",
- "SUBST_FILES.os=\tguess-os.h",
- "SUBST_VARS.os=\tTODAY1",
+ "SUBST_CLASSES+= one two",
+ "SUBST_STAGE.one= post-configure",
+ "SUBST_FILES.one= one.txt",
+ "SUBST_SED.one= s,one,1,g",
"",
- "TODAY1!=\tdate",
- "TODAY2!=\tdate")
+ "SUBST_STAGE.two= post-configure",
+ "SUBST_FILES.two= two.txt",
+ "",
+ "SUBST_STAGE.three= post-configure",
+ "",
+ "SUBST_VARS.four= PREFIX",
+ "",
+ "SUBST_VARS.three= PREFIX")
- t.CheckOutputEmpty()
+ t.CheckOutputLines(
+ "NOTE: filename.mk:1: Please add only one class at a time to SUBST_CLASSES.",
+ "WARN: filename.mk:9: Variable \"SUBST_STAGE.three\" "+
+ "does not match SUBST class \"two\".",
+ "WARN: filename.mk:9: Incomplete SUBST block: "+
+ "SUBST_SED.two, SUBST_VARS.two or SUBST_FILTER_CMD.two missing.",
+ "WARN: filename.mk:9: Before defining SUBST_STAGE.three, "+
+ "the SUBST class should be declared using \"SUBST_CLASSES+= three\".",
+ "WARN: filename.mk:11: Before defining SUBST_VARS.four, "+
+ "the SUBST class should be declared using \"SUBST_CLASSES+= four\".")
}
-func (s *Suite) Test_SubstContext__multiple_SUBST_VARS(c *check.C) {
+func (s *Suite) Test_SubstContext_varassign__multiple_classes_in_one_block(c *check.C) {
t := s.Init(c)
t.RunSubst(
- "SUBST_CLASSES+=\tos",
- "SUBST_STAGE.os=\tpre-configure",
- "SUBST_FILES.os=\tguess-os.h",
- "SUBST_VARS.os=\tPREFIX VARBASE")
+ "SUBST_CLASSES+= one",
+ "SUBST_STAGE.one= post-configure",
+ "SUBST_STAGE.one= post-configure",
+ "SUBST_FILES.one= one.txt",
+ "SUBST_CLASSES+= two", // The block "one" is not finished yet.
+ "SUBST_SED.one= s,one,1,g",
+ "SUBST_STAGE.two= post-configure",
+ "SUBST_FILES.two= two.txt",
+ "SUBST_SED.two= s,two,2,g")
- t.CheckOutputEmpty()
+ t.CheckOutputLines(
+ "WARN: filename.mk:3: Duplicate definition of \"SUBST_STAGE.one\".",
+ "WARN: filename.mk:5: Subst block \"one\" should be finished "+
+ "before adding the next class to SUBST_CLASSES.",
+ "WARN: filename.mk:5: Incomplete SUBST block: SUBST_SED.one, SUBST_VARS.one or SUBST_FILTER_CMD.one missing.",
+ "WARN: filename.mk:6: Late additions to a SUBST variable should use the += operator.")
}
-// As of May 2019, pkglint does not check the order of the variables in
+// As of December 2019, pkglint does not check the order of the variables in
// a SUBST block. Enforcing this order, or at least suggesting it, would
// make pkgsrc packages more uniform, which is a good idea, but not urgent.
-func (s *Suite) Test_SubstContext__unusual_variable_order(c *check.C) {
+func (s *Suite) Test_SubstContext_varassign__unusual_order(c *check.C) {
t := s.Init(c)
t.RunSubst(
@@ -287,41 +158,7 @@ func (s *Suite) Test_SubstContext__unusu
t.CheckOutputEmpty()
}
-func (s *Suite) Test_SubstContext__completely_conditional_then(c *check.C) {
- t := s.Init(c)
-
- t.RunSubst(
- ".if ${OPSYS} == Linux",
- "SUBST_CLASSES+=\tid",
- "SUBST_STAGE.id=\tpre-configure",
- "SUBST_SED.id=\t-e sahara",
- ".else",
- ".endif")
-
- // The block already ends at the .else, not at the end of the file,
- // since that is the scope where the SUBST id is defined.
- t.CheckOutputLines(
- "WARN: filename.mk:5: Incomplete SUBST block: SUBST_FILES.id missing.")
-}
-
-func (s *Suite) Test_SubstContext__completely_conditional_else(c *check.C) {
- t := s.Init(c)
-
- t.RunSubst(
- ".if ${OPSYS} == Linux",
- ".else",
- "SUBST_CLASSES+=\tid",
- "SUBST_STAGE.id=\tpre-configure",
- "SUBST_SED.id=\t-e sahara",
- ".endif")
-
- // The block already ends at the .endif, not at the end of the file,
- // since that is the scope where the SUBST id is defined.
- t.CheckOutputLines(
- "WARN: filename.mk:6: Incomplete SUBST block: SUBST_FILES.id missing.")
-}
-
-func (s *Suite) Test_SubstContext__SUBST_CLASSES_in_separate_paragraph(c *check.C) {
+func (s *Suite) Test_SubstContext_varassign__blocks_in_separate_paragraphs(c *check.C) {
t := s.Init(c)
t.RunSubst(
@@ -344,7 +181,7 @@ func (s *Suite) Test_SubstContext__SUBST
"WARN: filename.mk:EOF: Missing SUBST block for \"4\".")
}
-func (s *Suite) Test_SubstContext__wrong_class(c *check.C) {
+func (s *Suite) Test_SubstContext_varassign__typo_in_id(c *check.C) {
t := s.Init(c)
t.RunSubst(
@@ -358,33 +195,9 @@ func (s *Suite) Test_SubstContext__wrong
t.CheckOutputLines(
"NOTE: filename.mk:1: Please add only one class at a time to SUBST_CLASSES.",
- "WARN: filename.mk:2: Variable \"SUBST_STAGE.x\" does not match SUBST class \"1\".",
- "WARN: filename.mk:3: Variable \"SUBST_FILES.x\" does not match SUBST class \"1\".",
- "WARN: filename.mk:4: Variable \"SUBST_VARS.x\" does not match SUBST class \"1\".",
- // XXX: This line could change to 2, since that is already in the queue.
- "WARN: filename.mk:5: Variable \"SUBST_STAGE.2\" does not match SUBST class \"1\".",
- "WARN: filename.mk:6: Variable \"SUBST_FILES.2\" does not match SUBST class \"1\".",
- "WARN: filename.mk:7: Variable \"SUBST_VARS.2\" does not match SUBST class \"1\".",
- "WARN: filename.mk:EOF: Missing SUBST block for \"1\".",
- "WARN: filename.mk:EOF: Missing SUBST block for \"2\".")
-}
-
-func (s *Suite) Test_SubstContext_varassign__late_addition(c *check.C) {
- t := s.Init(c)
-
- t.RunSubst(
- "SUBST_CLASSES+=\tid",
- "SUBST_STAGE.id=\tpost-configure",
- "SUBST_FILES.id=\tfiles",
- "SUBST_VARS.id=\tPREFIX",
- "",
- ".if ${OPSYS} == NetBSD",
- "SUBST_VARS.id=\tOPSYS",
- ".endif")
-
- t.CheckOutputLines(
- "WARN: filename.mk:7: Late additions to a SUBST variable " +
- "should use the += operator.")
+ "WARN: filename.mk:2: Before defining SUBST_STAGE.x, "+
+ "the SUBST class should be declared using \"SUBST_CLASSES+= x\".",
+ "WARN: filename.mk:EOF: Missing SUBST block for \"1\".")
}
func (s *Suite) Test_SubstContext_varassign__late_addition_to_unknown_class(c *check.C) {
@@ -403,31 +216,156 @@ func (s *Suite) Test_SubstContext_varass
"the SUBST class should be declared using \"SUBST_CLASSES+= id\".")
}
-func (s *Suite) Test_SubstContext_varassignClasses__none(c *check.C) {
+func (s *Suite) Test_SubstContext_varassign__rationale(c *check.C) {
t := s.Init(c)
t.RunSubst(
- "SUBST_CLASSES+=\t# none")
+ "# Adjust setup.py",
+ "SUBST_CLASSES+= setup",
+ "SUBST_STAGE.setup= post-configure",
+ "SUBST_FILES.setup= setup.py",
+ "SUBST_VARS.setup= VAR")
+ // The rationale in line 1 is supposed to suppress warnings,
+ // not add new ones.
t.CheckOutputEmpty()
}
-func (s *Suite) Test_SubstContext_varassignClasses__indirect(c *check.C) {
+func (s *Suite) Test_SubstContext_varassign__interleaved(c *check.C) {
t := s.Init(c)
- t.SetUpVartypes()
- mklines := t.NewMkLines("filename.mk",
- MkCvsID,
- "SUBST_CLASSES+=\t${VAR}")
-
- mklines.Check()
-
+ t.RunSubst(
+ "SUBST_CLASSES+= 1 2 3",
+ "SUBST_STAGE.1= post-configure",
+ "SUBST_STAGE.2= post-configure",
+ "SUBST_STAGE.3= post-configure",
+ "SUBST_FILES.1= setup.py",
+ "SUBST_FILES.2= setup.py",
+ "SUBST_FILES.3= setup.py",
+ "SUBST_VARS.1= VAR",
+ "SUBST_VARS.2= VAR",
+ "SUBST_VARS.3= VAR")
+
+ // The above does not follow the common pattern of defining
+ // each block on its own.
+ // It technically works but is not easy to read for humans.
+ t.CheckOutputLines(
+ "NOTE: filename.mk:1: " +
+ "Please add only one class at a time to SUBST_CLASSES.")
+}
+
+func (s *Suite) Test_SubstContext_varassignClasses__OPSYSVARS(c *check.C) {
+ t := s.Init(c)
+
+ ctx := NewSubstContext()
+
+ // SUBST_CLASSES is added to OPSYSVARS in mk/bsd.pkg.mk.
+ ctx.varassign(t.NewMkLine("filename.mk", 11, "SUBST_CLASSES.SunOS+=prefix"))
+ ctx.varassign(t.NewMkLine("filename.mk", 12, "SUBST_CLASSES.NetBSD+=prefix"))
+ ctx.varassign(t.NewMkLine("filename.mk", 13, "SUBST_FILES.prefix=Makefile"))
+ ctx.varassign(t.NewMkLine("filename.mk", 14, "SUBST_SED.prefix=s,@PREFIX@,${PREFIX},g"))
+ ctx.varassign(t.NewMkLine("filename.mk", 15, "SUBST_STAGE.prefix=post-configure"))
+
+ t.CheckEquals(ctx.block().isComplete(), true)
+
+ ctx.Finish(t.NewMkLine("filename.mk", 15, ""))
+
+ t.CheckOutputLines(
+ "NOTE: filename.mk:14: The substitution command \"s,@PREFIX@,${PREFIX},g\" " +
+ "can be replaced with \"SUBST_VARS.prefix= PREFIX\".")
+}
+
+func (s *Suite) Test_SubstContext_varassignClasses__duplicate_id(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= id",
+ "SUBST_CLASSES+= id")
+
+ t.CheckOutputLines(
+ "ERROR: filename.mk:2: Duplicate SUBST class \"id\".",
+ "WARN: filename.mk:EOF: Missing SUBST block for \"id\".")
+}
+
+func (s *Suite) Test_SubstContext_varassignClasses__multiple_classes_in_one_line(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= one two",
+ "SUBST_STAGE.one= post-configure",
+ "SUBST_FILES.one= one.txt",
+ "SUBST_SED.one= s,one,1,g",
+ "SUBST_STAGE.two= post-configure",
+ "SUBST_FILES.two= two.txt")
+
+ t.CheckOutputLines(
+ "NOTE: filename.mk:1: Please add only one class at a time to SUBST_CLASSES.",
+ "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_SED.two, SUBST_VARS.two or SUBST_FILTER_CMD.two missing.")
+}
+
+func (s *Suite) Test_SubstContext_varassignClasses__none(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+=\t# none")
+
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_SubstContext_varassignClasses__indirect(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpVartypes()
+ mklines := t.NewMkLines("filename.mk",
+ MkCvsID,
+ "SUBST_CLASSES+=\t${VAR}")
+
+ mklines.Check()
+
t.CheckOutputLines(
"ERROR: filename.mk:2: Identifiers for SUBST_CLASSES "+
"must not refer to other variables.",
"WARN: filename.mk:2: VAR is used but not defined.")
}
+func (s *Suite) Test_SubstContext_varassignOutsideBlock__assign(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+=\t1",
+ "SUBST_STAGE.1=\tpre-configure",
+ "SUBST_FILES.1=\tfile1",
+ "SUBST_SED.1=\t-e s,subst1,repl1,",
+ "SUBST_CLASSES+=\t2",
+ "SUBST_SED.1=\t-e s,subst1b,repl1b,", // Misplaced
+ "SUBST_STAGE.2=\tpre-configure",
+ "SUBST_FILES.2=\tfile2",
+ "SUBST_SED.2=\t-e s,subst2,repl2,")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:6: Late additions to a SUBST variable " +
+ "should use the += operator.")
+}
+
+func (s *Suite) Test_SubstContext_varassignOutsideBlock__append(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+=\t1",
+ "SUBST_STAGE.1=\tpre-configure",
+ "SUBST_FILES.1=\tfile1",
+ "SUBST_SED.1=\t-e s,subst1,repl1,",
+ "SUBST_CLASSES+=\t2",
+ "SUBST_SED.1+=\t-e s,subst1b,repl1b,", // Misplaced
+ "SUBST_STAGE.2=\tpre-configure",
+ "SUBST_FILES.2=\tfile2",
+ "SUBST_SED.2=\t-e s,subst2,repl2,")
+
+ // The SUBST_SED.1 line is misplaced. It uses the += operator,
+ // which makes it unclear whether this is a typo or not.
+ t.CheckOutputEmpty()
+}
+
// The rationale for the stray SUBST variables has to be specific.
//
// For example, in the following snippet from mail/dkim-milter/options.mk
@@ -460,19 +398,432 @@ func (s *Suite) Test_SubstContext_varass
ctx := NewSubstContext()
mklines.collectRationale()
- mklines.ForEach(ctx.Process)
+ mklines.ForEach(ctx.Process)
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:2: Before defining SUBST_VARS.one, "+
+ "the SUBST class should be declared using \"SUBST_CLASSES+= one\".",
+ // In filename.mk:5 there is a proper rationale, thus no warning.
+ "WARN: filename.mk:8: Before defining SUBST_VARS.def, "+
+ "the SUBST class should be declared using \"SUBST_CLASSES+= def\".",
+ "WARN: filename.mk:11: Before defining SUBST_SED.libs, "+
+ "the SUBST class should be declared using \"SUBST_CLASSES+= libs\".")
+}
+
+// Unbalanced conditionals must not lead to a panic.
+func (s *Suite) Test_SubstContext_directive__before_SUBST_CLASSES(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ ".if 0",
+ ".endif",
+ "SUBST_CLASSES+=\tos",
+ ".elif 0")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:4: Missing SUBST block for \"os\".")
+}
+
+func (s *Suite) Test_SubstContext_directive__conditional_blocks_complete(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_CLASSES+= nb",
+ "SUBST_STAGE.nb= post-configure",
+ "SUBST_FILES.nb= guess-netbsd.h",
+ "SUBST_VARS.nb= HAVE_NETBSD",
+ ".else",
+ "SUBST_CLASSES+= os",
+ "SUBST_STAGE.os= post-configure",
+ "SUBST_FILES.os= guess-netbsd.h",
+ "SUBST_VARS.os= HAVE_OTHER",
+ ".endif")
+
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_SubstContext_directive__conditional_blocks_incomplete(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_CLASSES+= nb",
+ "SUBST_STAGE.nb= post-configure",
+ "SUBST_VARS.nb= HAVE_NETBSD",
+ ".else",
+ "SUBST_CLASSES+= os",
+ "SUBST_STAGE.os= post-configure",
+ "SUBST_FILES.os= guess-netbsd.h",
+ ".endif")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:5: Incomplete SUBST block: SUBST_FILES.nb missing.",
+ "WARN: filename.mk:6: Subst block \"nb\" should be finished "+
+ "before adding the next class to SUBST_CLASSES.",
+ "WARN: filename.mk:9: Incomplete SUBST block: "+
+ "SUBST_SED.os, SUBST_VARS.os or SUBST_FILTER_CMD.os missing.")
+}
+
+func (s *Suite) Test_SubstContext_directive__conditional_complete(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= id",
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_STAGE.id=\t\tpost-configure",
+ "SUBST_MESSAGE.id=\tpost-configure",
+ "SUBST_FILES.id=\t\tguess-netbsd.h",
+ "SUBST_SED.id=\t\t-e s,from,to,",
+ "SUBST_VARS.id=\t\tHAVE_OTHER",
+ "SUBST_FILTER_CMD.id=\tHAVE_OTHER",
+ ".else",
+ "SUBST_STAGE.id=\t\tpost-configure",
+ "SUBST_MESSAGE.id=\tpost-configure",
+ "SUBST_FILES.id=\t\tguess-netbsd.h",
+ "SUBST_SED.id=\t\t-e s,from,to,",
+ "SUBST_VARS.id=\t\tHAVE_OTHER",
+ "SUBST_FILTER_CMD.id=\tHAVE_OTHER",
+ ".endif")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:3: SUBST_STAGE.id should not be defined conditionally.",
+ "WARN: filename.mk:4: SUBST_MESSAGE.id should not be defined conditionally.",
+ "WARN: filename.mk:10: SUBST_STAGE.id should not be defined conditionally.",
+ "WARN: filename.mk:11: SUBST_MESSAGE.id should not be defined conditionally.")
+}
+
+func (s *Suite) Test_SubstContext_directive__conditionally_overwritten_filter(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= id",
+ "SUBST_STAGE.id=\t\tpost-configure",
+ "SUBST_MESSAGE.id=\tpost-configure",
+ "SUBST_FILES.id=\t\tguess-netbsd.h",
+ "SUBST_FILTER_CMD.id=\tHAVE_OTHER",
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_FILTER_CMD.id=\tHAVE_OTHER",
+ ".endif")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:7: Duplicate definition of \"SUBST_FILTER_CMD.id\".")
+}
+
+// Hopefully nobody will ever trigger this case in real pkgsrc.
+// It's plain confusing to a casual reader to nest a complete
+// SUBST block into another SUBST block.
+func (s *Suite) Test_SubstContext_directive__conditionally_nested_block(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= outer",
+ "SUBST_STAGE.outer= post-configure",
+ "SUBST_FILES.outer= outer.txt",
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_CLASSES+= inner",
+ "SUBST_STAGE.inner= post-configure",
+ "SUBST_FILES.inner= inner.txt",
+ "SUBST_VARS.inner= INNER",
+ ".endif",
+ "SUBST_VARS.outer= OUTER")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:5: Subst block \"outer\" should be finished " +
+ "before adding the next class to SUBST_CLASSES.")
+}
+
+// It's completely valid to have several SUBST blocks in a single paragraph.
+// As soon as a SUBST_CLASSES line appears, pkglint assumes that all previous
+// SUBST blocks are finished. That's exactly the case here.
+func (s *Suite) Test_SubstContext_directive__conditionally_following_block(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= outer",
+ "SUBST_STAGE.outer= post-configure",
+ "SUBST_FILES.outer= outer.txt",
+ "SUBST_VARS.outer= OUTER",
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_CLASSES+= middle",
+ "SUBST_STAGE.middle= post-configure",
+ "SUBST_FILES.middle= inner.txt",
+ "SUBST_VARS.middle= INNER",
+ ". if ${MACHINE_ARCH} == amd64",
+ "SUBST_CLASSES+= inner",
+ "SUBST_STAGE.inner= post-configure",
+ "SUBST_FILES.inner= inner.txt",
+ "SUBST_VARS.inner= INNER",
+ ". endif",
+ ".endif")
+
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_SubstContext_directive__two_blocks_in_condition(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_CLASSES+= a",
+ "SUBST_STAGE.a= post-configure",
+ "SUBST_FILES.a= outer.txt",
+ "SUBST_VARS.a= OUTER",
+ "SUBST_CLASSES+= b",
+ "SUBST_STAGE.b= post-configure",
+ "SUBST_FILES.b= inner.txt",
+ "SUBST_VARS.b= INNER",
+ ".endif")
+
+ // Up to 2019-12-12, pkglint wrongly warned in filename.mk:6:
+ // Subst block "a" should be finished before adding
+ // the next class to SUBST_CLASSES.
+ // The warning was wrong since block "a" has all required fields set.
+ // The warning was caused by an inconsistent check whether the current
+ // block had any conditional variables.
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_SubstContext_directive__nested_conditional_incomplete_block(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= outer",
+ "SUBST_STAGE.outer= post-configure",
+ "SUBST_FILES.outer= outer.txt",
+ "SUBST_VARS.outer= OUTER",
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_CLASSES+= inner1",
+ "SUBST_STAGE.inner1= post-configure",
+ "SUBST_VARS.inner1= INNER",
+ "SUBST_CLASSES+= inner2",
+ "SUBST_STAGE.inner2= post-configure",
+ "SUBST_FILES.inner2= inner.txt",
+ "SUBST_VARS.inner2= INNER",
+ ".endif")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:9: Subst block \"inner1\" should be finished "+
+ "before adding the next class to SUBST_CLASSES.",
+ "WARN: filename.mk:9: Incomplete SUBST block: SUBST_FILES.inner1 missing.")
+}
+
+func (s *Suite) Test_SubstContext_leave__details_in_then_branch(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= os",
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_VARS.os= OPSYS",
+ "SUBST_SED.os= -e s,@OPSYS@,NetBSD,",
+ "SUBST_STAGE.os= post-configure",
+ "SUBST_MESSAGE.os= Guessing operating system",
+ "SUBST_FILES.os= guess-os.h",
+ ".endif")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:5: SUBST_STAGE.os should not be defined conditionally.",
+ "WARN: filename.mk:6: SUBST_MESSAGE.os should not be defined conditionally.",
+ "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_STAGE.os missing.",
+ "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_FILES.os missing.",
+ "WARN: filename.mk:EOF: Incomplete SUBST block: "+
+ "SUBST_SED.os, SUBST_VARS.os or SUBST_FILTER_CMD.os missing.")
+}
+
+func (s *Suite) Test_SubstContext_leave__details_in_else_branch(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= os",
+ ".if ${OPSYS} == NetBSD",
+ ".else",
+ "SUBST_VARS.os= OPSYS",
+ "SUBST_SED.os= -e s,@OPSYS@,NetBSD,",
+ "SUBST_STAGE.os= post-configure",
+ "SUBST_MESSAGE.os= Guessing operating system",
+ "SUBST_FILES.os= guess-os.h",
+ ".endif")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:6: SUBST_STAGE.os should not be defined conditionally.",
+ "WARN: filename.mk:7: SUBST_MESSAGE.os should not be defined conditionally.",
+ "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_STAGE.os missing.",
+ "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_FILES.os missing.",
+ "WARN: filename.mk:EOF: Incomplete SUBST block: "+
+ "SUBST_SED.os, SUBST_VARS.os or SUBST_FILTER_CMD.os missing.")
+}
+
+func (s *Suite) Test_SubstContext_leave__empty_conditional_at_end(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= os",
+ "SUBST_VARS.os= OPSYS",
+ "SUBST_SED.os= -e s,@OPSYS@,NetBSD,",
+ "SUBST_STAGE.os= post-configure",
+ "SUBST_MESSAGE.os= Guessing operating system",
+ "SUBST_FILES.os= guess-os.h",
+ ".if ${OPSYS} == NetBSD",
+ ".else",
+ ".endif")
+
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_SubstContext_leave__missing_transformation_in_one_branch(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= os",
+ "SUBST_STAGE.os= post-configure",
+ "SUBST_MESSAGE.os= Guessing operating system",
+ "SUBST_FILES.os= guess-os.h",
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_FILES.os= -e s,@OpSYS@,NetBSD,", // A simple typo, this should be SUBST_SED.
+ ".elif ${OPSYS} == Darwin",
+ "SUBST_SED.os= -e s,@OPSYS@,Darwin1,",
+ "SUBST_SED.os= -e s,@OPSYS@,Darwin2,",
+ ".else",
+ "SUBST_VARS.os= OPSYS",
+ ".endif")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:6: All but the first assignment "+
+ "to \"SUBST_FILES.os\" should use the \"+=\" operator.",
+ "WARN: filename.mk:9: All but the first assignment "+
+ "to \"SUBST_SED.os\" should use the \"+=\" operator.",
+ "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_SED.os, "+
+ "SUBST_VARS.os or SUBST_FILTER_CMD.os missing.")
+}
+
+func (s *Suite) Test_SubstContext_leave__nested_conditionals(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= os",
+ "SUBST_STAGE.os= post-configure",
+ "SUBST_MESSAGE.os= Guessing operating system",
+ ".if ${OPSYS} == NetBSD",
+ "SUBST_FILES.os= guess-netbsd.h",
+ ". if ${ARCH} == i386",
+ "SUBST_FILTER_CMD.os= ${SED} -e s,@OPSYS,NetBSD-i386,",
+ ". elif ${ARCH} == x86_64",
+ "SUBST_VARS.os= OPSYS",
+ ". else",
+ "SUBST_SED.os= -e s,@OPSYS,NetBSD-unknown",
+ ". endif",
+ ".else",
+ // This branch omits SUBST_FILES.
+ "SUBST_SED.os= -e s,@OPSYS@,unknown,",
+ ".endif")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_FILES.os missing.")
+}
+
+// With every .if directive, a new scope is created, to properly
+// keep track of the conditional level at which the SUBST classes
+// are declared.
+//
+// The scopes are even created when there is no SUBST variable
+// anywhere close. The conditionals must be tracked for determining
+// the end of the scope for the SUBST_CLASSES IDs.
+func (s *Suite) Test_substScope__conditionals(c *check.C) {
+ t := s.Init(c)
+
+ ctx := NewSubstContext()
+
+ line := func(text string) {
+ mkline := t.NewMkLine("filename.mk", 123, text)
+ ctx.Process(mkline)
+ }
+ verifyScopes := func(n int) {
+ t.CheckEquals(len(ctx.scopes), n)
+ }
+
+ verifyScopes(1)
+
+ line(".if 1")
+ verifyScopes(2)
+
+ line(". if 1")
+ verifyScopes(3)
+
+ line(". if 1")
+ verifyScopes(4)
+
+ line(". elif 1")
+ verifyScopes(4)
+
+ line(". else")
+ verifyScopes(4)
+
+ line(". endif")
+ verifyScopes(3)
+
+ line(". endif")
+ verifyScopes(2)
+
+ line(".endif")
+ verifyScopes(1)
+
+ // An unbalanced .endif must not lead to a panic.
+ line(".endif")
+ verifyScopes(1)
+
+ ctx.Finish(NewLineEOF("filename.mk"))
+}
+
+func (s *Suite) Test_substScope_prepareSubstClasses(c *check.C) {
+ t := s.Init(c)
+ t.RunSubst(
+ "SUBST_CLASSES+= 1",
+ "SUBST_STAGE.1= post-configure",
+ ".if 0")
+
+ // There's no need to warn about unbalanced conditionals
+ // since that is already done by MkLines.Check.
t.CheckOutputLines(
- "WARN: filename.mk:2: Before defining SUBST_VARS.one, "+
- "the SUBST class should be declared using \"SUBST_CLASSES+= one\".",
- // In filename.mk:5 there is a proper rationale, thus no warning.
- "WARN: filename.mk:8: Before defining SUBST_VARS.def, "+
- "the SUBST class should be declared using \"SUBST_CLASSES+= def\".",
- "WARN: filename.mk:11: Before defining SUBST_SED.libs, "+
- "the SUBST class should be declared using \"SUBST_CLASSES+= libs\".")
+ "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_FILES.1 missing.",
+ "WARN: filename.mk:EOF: Incomplete SUBST block: "+
+ "SUBST_SED.1, SUBST_VARS.1 or SUBST_FILTER_CMD.1 missing.")
+}
+
+func (s *Suite) Test_substScope_prepareSubstClasses__nested(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= 1",
+ "SUBST_STAGE.1= post-configure",
+ ".if 0",
+ ".if 0",
+ "SUBST_CLASSES+= 2")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:5: Subst block \"1\" should be finished "+
+ "before adding the next class to SUBST_CLASSES.",
+ "WARN: filename.mk:EOF: Missing SUBST block for \"2\".",
+ "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_FILES.1 missing.",
+ "WARN: filename.mk:EOF: Incomplete SUBST block: "+
+ "SUBST_SED.1, SUBST_VARS.1 or SUBST_FILTER_CMD.1 missing.")
+}
+
+func (s *Suite) Test_substBlock_varassignStage__do_patch(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+=\tos",
+ "SUBST_STAGE.os=\tdo-patch",
+ "SUBST_FILES.os=\tguess-os.h",
+ "SUBST_SED.os=\t-e s,@OPSYS@,Darwin,")
+
+ // No warning, since there is nothing to fix automatically.
+ // This case doesn't occur in practice anyway.
+ t.CheckOutputEmpty()
}
-func (s *Suite) Test_SubstContext_varassignStage__pre_patch(c *check.C) {
+func (s *Suite) Test_substBlock_varassignStage__pre_patch(c *check.C) {
t := s.Init(c)
t.Chdir(".")
@@ -495,7 +846,7 @@ func (s *Suite) Test_SubstContext_varass
"SUBST_SED.os= -e s,@OPSYS@,Darwin,")
}
-func (s *Suite) Test_SubstContext_varassignStage__post_patch(c *check.C) {
+func (s *Suite) Test_substBlock_varassignStage__post_patch(c *check.C) {
t := s.Init(c)
t.Chdir(".")
@@ -518,12 +869,31 @@ func (s *Suite) Test_SubstContext_varass
"SUBST_SED.os= -e s,@OPSYS@,Darwin,")
}
+func (s *Suite) Test_substBlock_varassignStage__empty_class(c *check.C) {
+ t := s.Init(c)
+
+ t.Chdir(".")
+
+ t.RunSubst(
+ "SUBST_CLASSES+= id",
+ "",
+ "SUBST_STAGE.= post-patch",
+ "SUBST_STAGE.id= post-configure",
+ "SUBST_FILES.id= files",
+ "SUBST_VARS.id= VAR",
+ "SUBST_VARS.= VAR")
+
+ t.CheckOutputLines(
+ "ERROR: filename.mk:3: Invalid SUBST class \"\" in variable name.",
+ "ERROR: filename.mk:7: Invalid SUBST class \"\" in variable name.")
+}
+
// As of December 2019, pkglint does not use token positions internally.
// Instead it only does simple string replacement when autofixing things.
// To avoid damaging anything, replacements are only done if they are
// unambiguous. This is not the case here, since line 4 contains the
// string "pre-patch" twice.
-func (s *Suite) Test_SubstContext_varassignStage__ambiguous_replacement(c *check.C) {
+func (s *Suite) Test_substBlock_varassignStage__ambiguous_replacement(c *check.C) {
t := s.Init(c)
t.Chdir(".")
@@ -541,7 +911,7 @@ func (s *Suite) Test_SubstContext_varass
t.CheckEquals(t.File("filename.mk").IsFile(), false)
}
-func (s *Suite) Test_SubstContext_varassignStage__with_NO_CONFIGURE(c *check.C) {
+func (s *Suite) Test_substBlock_varassignStage__with_NO_CONFIGURE(c *check.C) {
t := s.Init(c)
pkg := t.SetUpPackage("category/package",
@@ -572,7 +942,7 @@ func (s *Suite) Test_SubstContext_varass
"when NO_CONFIGURE is set (in line 35).")
}
-func (s *Suite) Test_SubstContext_varassignStage__without_NO_CONFIGURE(c *check.C) {
+func (s *Suite) Test_substBlock_varassignStage__without_NO_CONFIGURE(c *check.C) {
t := s.Init(c)
pkg := t.SetUpPackage("category/package",
@@ -589,7 +959,7 @@ func (s *Suite) Test_SubstContext_varass
// Before 2019-12-12, pkglint wrongly warned about variables that were
// not obviously SUBST variables, even if they were used later in SUBST_VARS.
-func (s *Suite) Test_SubstContext_varassignVars__var_before_SUBST_VARS(c *check.C) {
+func (s *Suite) Test_substBlock_varassignVars__var_before_SUBST_VARS(c *check.C) {
t := s.Init(c)
t.RunSubst(
@@ -612,23 +982,19 @@ func (s *Suite) Test_SubstContext_varass
"WARN: filename.mk:4: Foreign variable \"FOREIGN\" in SUBST block.")
}
-func (s *Suite) Test_SubstContext_dupList__conditional_before_unconditional(c *check.C) {
+func (s *Suite) Test_substBlock_varassignVars(c *check.C) {
t := s.Init(c)
t.RunSubst(
- "SUBST_CLASSES+= os",
- "SUBST_STAGE.os= post-configure",
- ".if 1",
- "SUBST_FILES.os= conditional",
- ".endif",
- "SUBST_FILES.os= unconditional",
- "SUBST_VARS.os= OPSYS")
+ "SUBST_CLASSES+=\tos",
+ "SUBST_STAGE.os=\tpre-configure",
+ "SUBST_FILES.os=\tguess-os.h",
+ "SUBST_VARS.os=\tPREFIX VARBASE")
- // TODO: Warn that the conditional line is overwritten.
t.CheckOutputEmpty()
}
-func (s *Suite) Test_SubstContext_suggestSubstVars(c *check.C) {
+func (s *Suite) Test_substBlock_suggestSubstVars(c *check.C) {
t := s.Init(c)
t.Chdir(".")
@@ -752,7 +1118,7 @@ func (s *Suite) Test_SubstContext_sugges
// If the SUBST_CLASS identifier ends with a plus, the generated code must
// use the correct assignment operator and be nicely formatted.
-func (s *Suite) Test_SubstContext_suggestSubstVars__plus(c *check.C) {
+func (s *Suite) Test_substBlock_suggestSubstVars__plus(c *check.C) {
t := s.Init(c)
t.Chdir(".")
@@ -786,7 +1152,7 @@ func (s *Suite) Test_SubstContext_sugges
// The last of the SUBST_SED variables is 15 characters wide. When SUBST_SED
// is replaced with SUBST_VARS, this becomes 16 characters and therefore
// requires the whole paragraph to be indented by one more tab.
-func (s *Suite) Test_SubstContext_suggestSubstVars__autofix_realign_paragraph(c *check.C) {
+func (s *Suite) Test_substBlock_suggestSubstVars__autofix_realign_paragraph(c *check.C) {
t := s.Init(c)
t.Chdir(".")
@@ -818,7 +1184,7 @@ func (s *Suite) Test_SubstContext_sugges
"SUBST_VARS.pfx+= PREFIX")
}
-func (s *Suite) Test_SubstContext_suggestSubstVars__autofix_plus_sed(c *check.C) {
+func (s *Suite) Test_substBlock_suggestSubstVars__autofix_plus_sed(c *check.C) {
t := s.Init(c)
t.Chdir(".")
@@ -849,7 +1215,7 @@ func (s *Suite) Test_SubstContext_sugges
"SUBST_SED.pfx+= -e s,@PREFIX@,other,g")
}
-func (s *Suite) Test_SubstContext_suggestSubstVars__autofix_plus_vars(c *check.C) {
+func (s *Suite) Test_substBlock_suggestSubstVars__autofix_plus_vars(c *check.C) {
t := s.Init(c)
t.Chdir(".")
@@ -881,7 +1247,7 @@ func (s *Suite) Test_SubstContext_sugges
"SUBST_VARS.id+= PKGMANDIR")
}
-func (s *Suite) Test_SubstContext_suggestSubstVars__autofix_indentation(c *check.C) {
+func (s *Suite) Test_substBlock_suggestSubstVars__autofix_indentation(c *check.C) {
t := s.Init(c)
t.Chdir(".")
@@ -911,7 +1277,7 @@ func (s *Suite) Test_SubstContext_sugges
"SUBST_VARS.fix-paths= PREFIX")
}
-func (s *Suite) Test_SubstContext_suggestSubstVars__conditional(c *check.C) {
+func (s *Suite) Test_substBlock_suggestSubstVars__conditional(c *check.C) {
t := s.Init(c)
t.Chdir(".")
@@ -938,389 +1304,118 @@ func (s *Suite) Test_SubstContext_sugges
"with \"SUBST_VARS.id+=\\tVAR2\".")
t.CheckFileLinesDetab("filename.mk",
- "SUBST_CLASSES+= id",
- "SUBST_STAGE.id= pre-configure",
- "SUBST_FILES.id= files",
- "SUBST_VARS.id= VAR",
- ".if 1",
- "SUBST_VARS.id+= VAR2",
- ".endif")
-}
-
-func (s *Suite) Test_SubstContext_extractVarname(c *check.C) {
- t := s.Init(c)
-
- test := func(input, expected string) {
- t.CheckEquals((*SubstContext).extractVarname(nil, input), expected)
- }
-
- // A simple variable name.
- test("s,@VAR@,${VAR},", "VAR")
-
- // A parameterized variable name.
- test("s,@VAR.param@,${VAR.param},", "VAR.param")
-
- // Only substitution commands can be replaced with SUBST_VARS.
- test("/pattern/d", "")
-
- // An incomplete substitution command.
- test("s", "")
-
- // Wrong placeholder character, only @ works.
- test("s,!VAR!,${VAR},", "")
-
- // The placeholder must have exactly 1 @ on each side.
- test("s,@@VAR@@,${VAR},", "")
-
- // Malformed because the comma is the separator.
- test("s,@VAR,VAR@,${VAR},", "")
-
- // The replacement pattern is not a simple variable name enclosed in @.
- test("s,@VAR!VAR@,${VAR},", "")
-
- // The replacement may only contain the :Q modifier.
- test("s,@VAR@,${VAR:Mpattern},", "")
-
- // The :Q modifier is allowed in the replacement.
- test("s,@VAR@,${VAR:Q},", "VAR")
-
- // The replacement may contain the :Q modifier only once.
- test("s,@VAR@,${VAR:Q:Q},", "")
-
- // The replacement must be a plain variable expression, without prefix.
- test("s,@VAR@,prefix${VAR},", "")
-
- // The replacement must be a plain variable expression, without suffix.
- test("s,@VAR@,${VAR}suffix,", "")
-}
-
-func (s *Suite) Test_SubstContext_directive__before_SUBST_CLASSES(c *check.C) {
- t := s.Init(c)
-
- t.RunSubst(
- ".if 0",
- ".endif",
- "SUBST_CLASSES+=\tos",
- ".elif 0") // Just for branch coverage.
-
- t.CheckOutputLines(
- "WARN: filename.mk:EOF: Missing SUBST block for \"os\".")
-}
-
-func (s *Suite) Test_SubstContext_directive__conditional_blocks_complete(c *check.C) {
- t := s.Init(c)
-
- t.RunSubst(
- ".if ${OPSYS} == NetBSD",
- "SUBST_CLASSES+= nb",
- "SUBST_STAGE.nb= post-configure",
- "SUBST_FILES.nb= guess-netbsd.h",
- "SUBST_VARS.nb= HAVE_NETBSD",
- ".else",
- "SUBST_CLASSES+= os",
- "SUBST_STAGE.os= post-configure",
- "SUBST_FILES.os= guess-netbsd.h",
- "SUBST_VARS.os= HAVE_OTHER",
- ".endif")
-
- t.CheckOutputEmpty()
-}
-
-func (s *Suite) Test_SubstContext_directive__conditional_blocks_incomplete(c *check.C) {
- t := s.Init(c)
-
- t.RunSubst(
- ".if ${OPSYS} == NetBSD",
- "SUBST_CLASSES+= nb",
- "SUBST_STAGE.nb= post-configure",
- "SUBST_VARS.nb= HAVE_NETBSD",
- ".else",
- "SUBST_CLASSES+= os",
- "SUBST_STAGE.os= post-configure",
- "SUBST_FILES.os= guess-netbsd.h",
- ".endif")
-
- t.CheckOutputLines(
- "WARN: filename.mk:5: Incomplete SUBST block: SUBST_FILES.nb missing.",
- "WARN: filename.mk:9: Incomplete SUBST block: "+
- "SUBST_SED.os, SUBST_VARS.os or SUBST_FILTER_CMD.os missing.")
-}
-
-func (s *Suite) Test_SubstContext_directive__conditional_complete(c *check.C) {
- t := s.Init(c)
-
- t.RunSubst(
- "SUBST_CLASSES+= id",
- ".if ${OPSYS} == NetBSD",
- "SUBST_STAGE.id=\t\tpost-configure",
- "SUBST_MESSAGE.id=\tpost-configure",
- "SUBST_FILES.id=\t\tguess-netbsd.h",
- "SUBST_SED.id=\t\t-e s,from,to,",
- "SUBST_VARS.id=\t\tHAVE_OTHER",
- "SUBST_FILTER_CMD.id=\tHAVE_OTHER",
- ".else",
- "SUBST_STAGE.id=\t\tpost-configure",
- "SUBST_MESSAGE.id=\tpost-configure",
- "SUBST_FILES.id=\t\tguess-netbsd.h",
- "SUBST_SED.id=\t\t-e s,from,to,",
- "SUBST_VARS.id=\t\tHAVE_OTHER",
- "SUBST_FILTER_CMD.id=\tHAVE_OTHER",
- ".endif")
-
- t.CheckOutputLines(
- "WARN: filename.mk:3: SUBST_STAGE.id should not be defined conditionally.",
- "WARN: filename.mk:4: SUBST_MESSAGE.id should not be defined conditionally.",
- "WARN: filename.mk:10: SUBST_STAGE.id should not be defined conditionally.",
- "WARN: filename.mk:11: SUBST_MESSAGE.id should not be defined conditionally.")
-}
-
-func (s *Suite) Test_SubstContext_directive__conditionally_overwritten_filter(c *check.C) {
- t := s.Init(c)
-
- t.RunSubst(
- "SUBST_CLASSES+= id",
- "SUBST_STAGE.id=\t\tpost-configure",
- "SUBST_MESSAGE.id=\tpost-configure",
- "SUBST_FILES.id=\t\tguess-netbsd.h",
- "SUBST_FILTER_CMD.id=\tHAVE_OTHER",
- ".if ${OPSYS} == NetBSD",
- "SUBST_FILTER_CMD.id=\tHAVE_OTHER",
- ".endif")
-
- t.CheckOutputLines(
- "WARN: filename.mk:7: Duplicate definition of \"SUBST_FILTER_CMD.id\".")
-}
-
-// Hopefully nobody will ever trigger this case in real pkgsrc.
-// It's plain confusing to a casual reader to nest a complete
-// SUBST block into another SUBST block.
-// That's why pkglint doesn't cover this case correctly.
-func (s *Suite) Test_SubstContext_directive__conditionally_nested_block(c *check.C) {
- t := s.Init(c)
-
- t.RunSubst(
- "SUBST_CLASSES+= outer",
- "SUBST_STAGE.outer= post-configure",
- "SUBST_FILES.outer= outer.txt",
- ".if ${OPSYS} == NetBSD",
- "SUBST_CLASSES+= inner",
- "SUBST_STAGE.inner= post-configure",
- "SUBST_FILES.inner= inner.txt",
- "SUBST_VARS.inner= INNER",
- ".endif",
- "SUBST_VARS.outer= OUTER")
-
- t.CheckOutputLines(
- "WARN: filename.mk:5: Incomplete SUBST block: "+
- "SUBST_SED.outer, SUBST_VARS.outer or SUBST_FILTER_CMD.outer missing.",
- "WARN: filename.mk:5: Subst block \"outer\" should be finished "+
- "before adding the next class to SUBST_CLASSES.",
- "WARN: filename.mk:10: "+
- "Late additions to a SUBST variable should use the += operator.")
-}
-
-// It's completely valid to have several SUBST blocks in a single paragraph.
-// As soon as a SUBST_CLASSES line appears, pkglint assumes that all previous
-// SUBST blocks are finished. That's exactly the case here.
-func (s *Suite) Test_SubstContext_directive__conditionally_following_block(c *check.C) {
- t := s.Init(c)
-
- t.RunSubst(
- "SUBST_CLASSES+= outer",
- "SUBST_STAGE.outer= post-configure",
- "SUBST_FILES.outer= outer.txt",
- "SUBST_VARS.outer= OUTER",
- ".if ${OPSYS} == NetBSD",
- "SUBST_CLASSES+= middle",
- "SUBST_STAGE.middle= post-configure",
- "SUBST_FILES.middle= inner.txt",
- "SUBST_VARS.middle= INNER",
- ". if ${MACHINE_ARCH} == amd64",
- "SUBST_CLASSES+= inner",
- "SUBST_STAGE.inner= post-configure",
- "SUBST_FILES.inner= inner.txt",
- "SUBST_VARS.inner= INNER",
- ". endif",
+ "SUBST_CLASSES+= id",
+ "SUBST_STAGE.id= pre-configure",
+ "SUBST_FILES.id= files",
+ "SUBST_VARS.id= VAR",
+ ".if 1",
+ "SUBST_VARS.id+= VAR2",
".endif")
-
- t.CheckOutputEmpty()
}
-func (s *Suite) Test_SubstContext_directive__two_blocks_in_condition(c *check.C) {
+func (s *Suite) Test_substBlock_dupList__late_addition(c *check.C) {
t := s.Init(c)
t.RunSubst(
+ "SUBST_CLASSES+=\tid",
+ "SUBST_STAGE.id=\tpost-configure",
+ "SUBST_FILES.id=\tfiles",
+ "SUBST_VARS.id=\tPREFIX",
+ "",
".if ${OPSYS} == NetBSD",
- "SUBST_CLASSES+= a",
- "SUBST_STAGE.a= post-configure",
- "SUBST_FILES.a= outer.txt",
- "SUBST_VARS.a= OUTER",
- "SUBST_CLASSES+= b",
- "SUBST_STAGE.b= post-configure",
- "SUBST_FILES.b= inner.txt",
- "SUBST_VARS.b= INNER",
+ "SUBST_VARS.id=\tOPSYS",
".endif")
- // Up to 2019-12-12, pkglint wrongly warned in filename.mk:6:
- // Subst block "a" should be finished before adding
- // the next class to SUBST_CLASSES.
- // The warning was wrong since block "a" has all required fields set.
- // The warning was caused by an inconsistent check whether the current
- // block had any conditional variables.
- t.CheckOutputEmpty()
+ t.CheckOutputLines(
+ "WARN: filename.mk:7: All but the first assignment " +
+ "to \"SUBST_VARS.id\" should use the \"+=\" operator.")
}
-func (s *Suite) Test_SubstContext_directive__nested_conditional_incomplete_block(c *check.C) {
+func (s *Suite) Test_substBlock_dupList__conditional_before_unconditional(c *check.C) {
t := s.Init(c)
t.RunSubst(
- "SUBST_CLASSES+= outer",
- "SUBST_STAGE.outer= post-configure",
- "SUBST_FILES.outer= outer.txt",
- "SUBST_VARS.outer= OUTER",
- ".if ${OPSYS} == NetBSD",
- "SUBST_CLASSES+= inner1",
- "SUBST_STAGE.inner1= post-configure",
- "SUBST_VARS.inner1= INNER",
- "SUBST_CLASSES+= inner2",
- "SUBST_STAGE.inner2= post-configure",
- "SUBST_FILES.inner2= inner.txt",
- "SUBST_VARS.inner2= INNER",
- ".endif")
+ "SUBST_CLASSES+= os",
+ "SUBST_STAGE.os= post-configure",
+ ".if 1",
+ "SUBST_FILES.os= conditional",
+ ".endif",
+ "SUBST_FILES.os= unconditional",
+ "SUBST_VARS.os= OPSYS")
t.CheckOutputLines(
- "WARN: filename.mk:9: Incomplete SUBST block: SUBST_FILES.inner1 missing.",
- "WARN: filename.mk:9: Subst block \"inner1\" should be finished "+
- "before adding the next class to SUBST_CLASSES.")
+ "WARN: filename.mk:6: All but the first assignment " +
+ "to \"SUBST_FILES.os\" should use the \"+=\" operator.")
}
-func (s *Suite) Test_SubstContext_finishClass__details_in_then_branch(c *check.C) {
+func (s *Suite) Test_substBlock_extractVarname(c *check.C) {
t := s.Init(c)
- t.RunSubst(
- "SUBST_CLASSES+= os",
- ".if ${OPSYS} == NetBSD",
- "SUBST_VARS.os= OPSYS",
- "SUBST_SED.os= -e s,@OPSYS@,NetBSD,",
- "SUBST_STAGE.os= post-configure",
- "SUBST_MESSAGE.os= Guessing operating system",
- "SUBST_FILES.os= guess-os.h",
- ".endif")
+ test := func(input, expected string) {
+ t.CheckEquals((*substBlock).extractVarname(nil, input), expected)
+ }
- t.CheckOutputLines(
- "WARN: filename.mk:5: SUBST_STAGE.os should not be defined conditionally.",
- "WARN: filename.mk:6: SUBST_MESSAGE.os should not be defined conditionally.",
- "WARN: filename.mk:EOF: Missing SUBST block for \"os\".")
-}
+ // A simple variable name.
+ test("s,@VAR@,${VAR},", "VAR")
-func (s *Suite) Test_SubstContext_finishClass__details_in_else_branch(c *check.C) {
- t := s.Init(c)
+ // A parameterized variable name.
+ test("s,@VAR.param@,${VAR.param},", "VAR.param")
- t.RunSubst(
- "SUBST_CLASSES+= os",
- ".if ${OPSYS} == NetBSD",
- ".else",
- "SUBST_VARS.os= OPSYS",
- "SUBST_SED.os= -e s,@OPSYS@,NetBSD,",
- "SUBST_STAGE.os= post-configure",
- "SUBST_MESSAGE.os= Guessing operating system",
- "SUBST_FILES.os= guess-os.h",
- ".endif")
+ // Only substitution commands can be replaced with SUBST_VARS.
+ test("/pattern/d", "")
- t.CheckOutputLines(
- "WARN: filename.mk:6: SUBST_STAGE.os should not be defined conditionally.",
- "WARN: filename.mk:7: SUBST_MESSAGE.os should not be defined conditionally.",
- "WARN: filename.mk:EOF: Missing SUBST block for \"os\".")
-}
+ // An incomplete substitution command.
+ test("s", "")
-func (s *Suite) Test_SubstContext_finishClass__empty_conditional_at_end(c *check.C) {
- t := s.Init(c)
+ // Wrong placeholder character, only @ works.
+ test("s,!VAR!,${VAR},", "")
- t.RunSubst(
- "SUBST_CLASSES+= os",
- "SUBST_VARS.os= OPSYS",
- "SUBST_SED.os= -e s,@OPSYS@,NetBSD,",
- "SUBST_STAGE.os= post-configure",
- "SUBST_MESSAGE.os= Guessing operating system",
- "SUBST_FILES.os= guess-os.h",
- ".if ${OPSYS} == NetBSD",
- ".else",
- ".endif")
+ // The placeholder must have exactly 1 @ on each side.
+ test("s,@@VAR@@,${VAR},", "")
- t.CheckOutputEmpty()
-}
+ // Malformed because the comma is the separator.
+ test("s,@VAR,VAR@,${VAR},", "")
-func (s *Suite) Test_SubstContext_finishClass__missing_transformation_in_one_branch(c *check.C) {
- t := s.Init(c)
+ // The replacement pattern is not a simple variable name enclosed in @.
+ test("s,@VAR!VAR@,${VAR},", "")
- t.RunSubst(
- "SUBST_CLASSES+= os",
- "SUBST_STAGE.os= post-configure",
- "SUBST_MESSAGE.os= Guessing operating system",
- "SUBST_FILES.os= guess-os.h",
- ".if ${OPSYS} == NetBSD",
- "SUBST_FILES.os= -e s,@OpSYS@,NetBSD,", // A simple typo, this should be SUBST_SED.
- ".elif ${OPSYS} == Darwin",
- "SUBST_SED.os= -e s,@OPSYS@,Darwin1,",
- "SUBST_SED.os= -e s,@OPSYS@,Darwin2,",
- ".else",
- "SUBST_VARS.os= OPSYS",
- ".endif")
+ // The replacement may only contain the :Q modifier.
+ test("s,@VAR@,${VAR:Mpattern},", "")
- t.CheckOutputLines(
- "WARN: filename.mk:6: All but the first assignment "+
- "to \"SUBST_FILES.os\" should use the \"+=\" operator.",
- "WARN: filename.mk:9: All but the first assignment "+
- "to \"SUBST_SED.os\" should use the \"+=\" operator.",
- "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_SED.os, "+
- "SUBST_VARS.os or SUBST_FILTER_CMD.os missing.")
-}
+ // The :Q modifier is allowed in the replacement.
+ test("s,@VAR@,${VAR:Q},", "VAR")
-func (s *Suite) Test_SubstContext_finishClass__nested_conditionals(c *check.C) {
- t := s.Init(c)
+ // The replacement may contain the :Q modifier only once.
+ test("s,@VAR@,${VAR:Q:Q},", "")
- t.RunSubst(
- "SUBST_CLASSES+= os",
- "SUBST_STAGE.os= post-configure",
- "SUBST_MESSAGE.os= Guessing operating system",
- ".if ${OPSYS} == NetBSD",
- "SUBST_FILES.os= guess-netbsd.h",
- ". if ${ARCH} == i386",
- "SUBST_FILTER_CMD.os= ${SED} -e s,@OPSYS,NetBSD-i386,",
- ". elif ${ARCH} == x86_64",
- "SUBST_VARS.os= OPSYS",
- ". else",
- "SUBST_SED.os= -e s,@OPSYS,NetBSD-unknown",
- ". endif",
- ".else",
- // This branch omits SUBST_FILES.
- "SUBST_SED.os= -e s,@OPSYS@,unknown,",
- ".endif")
+ // The replacement must be a plain variable expression, without prefix.
+ test("s,@VAR@,prefix${VAR},", "")
- t.CheckOutputLines(
- "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_FILES.os missing.")
+ // The replacement must be a plain variable expression, without suffix.
+ test("s,@VAR@,${VAR}suffix,", "")
}
-func (s *Suite) Test_SubstContext_isComplete__incomplete(c *check.C) {
+func (s *Suite) Test_substBlock_isComplete__incomplete(c *check.C) {
t := s.Init(c)
ctx := NewSubstContext()
ctx.varassign(t.NewMkLine("filename.mk", 10, "PKGNAME=pkgname-1.0"))
- t.CheckEquals(ctx.id, "")
+ t.CheckEquals(ctx.isActive(), false)
ctx.varassign(t.NewMkLine("filename.mk", 11, "SUBST_CLASSES+=interp"))
- t.CheckEquals(ctx.id, "interp")
+ t.CheckEquals(ctx.isActive(), false)
ctx.varassign(t.NewMkLine("filename.mk", 12, "SUBST_FILES.interp=Makefile"))
- t.CheckEquals(ctx.isComplete(), false)
+ t.CheckEquals(ctx.activeId(), "interp")
+ t.CheckEquals(ctx.block().isComplete(), false)
ctx.varassign(t.NewMkLine("filename.mk", 13, "SUBST_SED.interp=s,@PREFIX@,${PREFIX},g"))
- t.CheckEquals(ctx.isComplete(), false)
+ t.CheckEquals(ctx.block().isComplete(), false)
ctx.Finish(t.NewMkLine("filename.mk", 14, ""))
@@ -1330,7 +1425,7 @@ func (s *Suite) Test_SubstContext_isComp
"WARN: filename.mk:14: Incomplete SUBST block: SUBST_STAGE.interp missing.")
}
-func (s *Suite) Test_SubstContext_isComplete__complete(c *check.C) {
+func (s *Suite) Test_substBlock_isComplete__complete(c *check.C) {
t := s.Init(c)
ctx := NewSubstContext()
@@ -1340,11 +1435,11 @@ func (s *Suite) Test_SubstContext_isComp
ctx.varassign(t.NewMkLine("filename.mk", 12, "SUBST_FILES.p=Makefile"))
ctx.varassign(t.NewMkLine("filename.mk", 13, "SUBST_SED.p=s,@PREFIX@,${PREFIX},g"))
- t.CheckEquals(ctx.isComplete(), false)
+ t.CheckEquals(ctx.block().isComplete(), false)
ctx.varassign(t.NewMkLine("filename.mk", 14, "SUBST_STAGE.p=post-configure"))
- t.CheckEquals(ctx.isComplete(), true)
+ t.CheckEquals(ctx.block().isComplete(), true)
ctx.Finish(t.NewMkLine("filename.mk", 15, ""))
@@ -1352,3 +1447,106 @@ func (s *Suite) Test_SubstContext_isComp
"NOTE: filename.mk:13: The substitution command \"s,@PREFIX@,${PREFIX},g\" " +
"can be replaced with \"SUBST_VARS.p= PREFIX\".")
}
+
+func (s *Suite) Test_substBlock_finish__conditional_inside_then(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ ".if ${OPSYS} == Linux",
+ "SUBST_CLASSES+=\tid",
+ "SUBST_STAGE.id=\tpre-configure",
+ "SUBST_SED.id=\t-e sahara",
+ ".else",
+ ".endif")
+
+ // The block already ends at the .else, not at the end of the file,
+ // since that is the scope where the SUBST id is defined.
+ t.CheckOutputLines(
+ "WARN: filename.mk:5: Incomplete SUBST block: SUBST_FILES.id missing.")
+}
+
+func (s *Suite) Test_substBlock_finish__conditional_inside_else(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ ".if ${OPSYS} == Linux",
+ ".else",
+ "SUBST_CLASSES+=\tid",
+ "SUBST_STAGE.id=\tpre-configure",
+ "SUBST_SED.id=\t-e sahara",
+ ".endif")
+
+ // The block already ends at the .endif, not at the end of the file,
+ // since that is the scope where the SUBST id is defined.
+ t.CheckOutputLines(
+ "WARN: filename.mk:6: Incomplete SUBST block: SUBST_FILES.id missing.")
+}
+
+func (s *Suite) Test_substBlock_finish__files_missing(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= one",
+ "SUBST_STAGE.one= pre-configure",
+ "SUBST_CLASSES+= two",
+ "SUBST_STAGE.two= pre-configure",
+ "SUBST_FILES.two= two.txt",
+ "SUBST_SED.two= s,two,2,g")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:3: Subst block \"one\" should be finished "+
+ "before adding the next class to SUBST_CLASSES.",
+ "WARN: filename.mk:3: Incomplete SUBST block: SUBST_FILES.one missing.",
+ "WARN: filename.mk:3: Incomplete SUBST block: "+
+ "SUBST_SED.one, SUBST_VARS.one or SUBST_FILTER_CMD.one missing.")
+}
+
+// Variables mentioned in SUBST_VARS may appear in the same paragraph,
+// or alternatively anywhere else in the file.
+func (s *Suite) Test_substBlock_checkForeignVariables__in_next_paragraph(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+=\tos",
+ "SUBST_STAGE.os=\tpre-configure",
+ "SUBST_FILES.os=\tguess-os.h",
+ "SUBST_VARS.os=\tTODAY1",
+ "",
+ "TODAY1!=\tdate",
+ "TODAY2!=\tdate")
+
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_substBlock_checkForeignVariables__mixed_separate(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= 1",
+ "SUBST_STAGE.1= post-configure",
+ "SUBST_FILES.1= files",
+ "",
+ "SUBST_VARS.1= VAR",
+ "USE_TOOLS+= gmake")
+
+ // The USE_TOOLS is not in the SUBST block anymore since there is
+ // an empty line between SUBST_CLASSES and SUBST_VARS.
+ t.CheckOutputEmpty()
+}
+
+// Variables mentioned in SUBST_VARS are not considered "foreign"
+// in the block and may be mixed with the other SUBST variables.
+func (s *Suite) Test_substBlock_checkForeignVariables__in_block(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+=\tos",
+ "SUBST_STAGE.os=\tpre-configure",
+ "SUBST_FILES.os=\tguess-os.h",
+ "SUBST_VARS.os=\tTODAY1",
+ "TODAY1!=\tdate",
+ "TODAY2!=\tdate")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:6: Foreign variable \"TODAY2\" in SUBST block.")
+}
Index: pkgsrc/pkgtools/pkglint/files/vardefs.go
diff -u pkgsrc/pkgtools/pkglint/files/vardefs.go:1.83 pkgsrc/pkgtools/pkglint/files/vardefs.go:1.84
--- pkgsrc/pkgtools/pkglint/files/vardefs.go:1.83 Fri Dec 13 01:39:23 2019
+++ pkgsrc/pkgtools/pkglint/files/vardefs.go Sat Dec 14 18:04:16 2019
@@ -518,7 +518,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
"openjdk8 oracle-jdk8 openjdk7 sun-jdk7 jdk16 jdk15 kaffe",
"_PKG_JVMS.*")
- platforms := reg.enumFromFiles(src,
+ operatingSystems := reg.enumFromFiles(src,
"mk/platform",
`(.*)\.mk$`, "$1",
"Cygwin DragonFly FreeBSD Linux NetBSD SunOS")
@@ -1362,7 +1362,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkglistrat("ONLY_FOR_COMPILER", compilers)
reg.pkglistrat("ONLY_FOR_PLATFORM", BtMachinePlatformPattern)
reg.pkgrat("ONLY_FOR_UNPRIVILEGED", BtYesNo)
- reg.sysload("OPSYS", platforms, DefinedIfInScope|NonemptyIfDefined)
+ reg.sysload("OPSYS", operatingSystems, DefinedIfInScope|NonemptyIfDefined)
reg.pkglistbl3("OPSYSVARS", BtVariableName)
reg.pkg("OSVERSION_SPECIFIC", BtYes)
reg.sysload("OS_VARIANT", BtIdentifierDirect, DefinedIfInScope)
Index: pkgsrc/pkgtools/pkglint/files/vartype.go
diff -u pkgsrc/pkgtools/pkglint/files/vartype.go:1.43 pkgsrc/pkgtools/pkglint/files/vartype.go:1.44
--- pkgsrc/pkgtools/pkglint/files/vartype.go:1.43 Fri Dec 13 01:39:23 2019
+++ pkgsrc/pkgtools/pkglint/files/vartype.go Sat Dec 14 18:04:16 2019
@@ -467,7 +467,6 @@ var (
BtYesNo = &BasicType{"YesNo", (*VartypeCheck).YesNo}
BtYesNoIndirectly = &BasicType{"YesNoIndirectly", (*VartypeCheck).YesNoIndirectly}
- BtMachineOpsys = enumFromValues(machineOpsysValues)
BtMachineArch = enumFromValues(machineArchValues)
BtMachineGnuArch = enumFromValues(machineGnuArchValues)
BtEmulOpsys = enumFromValues(emulOpsysValues)
@@ -490,10 +489,6 @@ func init() {
// TODO: Move these values to VarTypeRegistry.Init and read them from the
// pkgsrc infrastructure files, as far as possible.
const (
- machineOpsysValues = "" + // See mk/platform
- "AIX BSDOS Bitrig Cygwin Darwin DragonFly FreeBSD FreeMiNT GNUkFreeBSD " +
- "HPUX Haiku IRIX Interix Linux Minix MirBSD NetBSD OSF1 OpenBSD QNX SCO_SV SunOS UnixWare"
-
// See mk/emulator/emulator-vars.mk.
emulOpsysValues = "" +
"bitrig bsdos cygwin darwin dragonfly freebsd " +
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.73 pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.74
--- pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.73 Fri Dec 13 01:39:23 2019
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck.go Sat Dec 14 18:04:16 2019
@@ -799,8 +799,14 @@ func (cv *VartypeCheck) MachinePlatformP
}
if m, opsysPattern, versionPattern, archPattern := match3(pattern, reTriple); m {
+ // This is cheated, but the dynamically loaded enums are not
+ // provided in a type registry where they could be looked up
+ // by a name. The following line therefore assumes that OPSYS
+ // is an operating system name, which sounds like a safe bet.
+ btOpsys := G.Pkgsrc.vartypes.types["OPSYS"].basicType
+
opsysCv := cv.WithVarnameValueMatch("the operating system part of "+cv.Varname, opsysPattern)
- BtMachineOpsys.checker(opsysCv)
+ btOpsys.checker(opsysCv)
versionCv := cv.WithVarnameValueMatch("the version part of "+cv.Varname, versionPattern)
versionCv.Version()
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.66 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.67
--- pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.66 Fri Dec 13 01:39:23 2019
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go Sat Dec 14 18:04:16 2019
@@ -1176,8 +1176,7 @@ func (s *Suite) Test_VartypeCheck_Machin
vt.Output(
"WARN: filename.mk:1: \"linux-i386\" is not a valid platform pattern.",
"WARN: filename.mk:2: The pattern \"nextbsd\" cannot match any of "+
- "{ AIX BSDOS Bitrig Cygwin Darwin DragonFly FreeBSD FreeMiNT GNUkFreeBSD HPUX Haiku "+
- "IRIX Interix Linux Minix MirBSD NetBSD OSF1 OpenBSD QNX SCO_SV SunOS UnixWare "+
+ "{ Cygwin DragonFly FreeBSD Linux NetBSD SunOS "+
"} for the operating system part of MACHINE_PLATFORM.",
"WARN: filename.mk:2: The pattern \"8087\" cannot match any of "+
"{ aarch64 aarch64eb alpha amd64 arc arm arm26 arm32 "+
@@ -1191,8 +1190,7 @@ func (s *Suite) Test_VartypeCheck_Machin
"rs6000 s390 sh3eb sh3el sparc sparc64 vax x86_64 "+
"} for the hardware architecture part of MACHINE_PLATFORM.",
"WARN: filename.mk:3: The pattern \"netbsd\" cannot match any of "+
- "{ AIX BSDOS Bitrig Cygwin Darwin DragonFly FreeBSD FreeMiNT GNUkFreeBSD HPUX Haiku "+
- "IRIX Interix Linux Minix MirBSD NetBSD OSF1 OpenBSD QNX SCO_SV SunOS UnixWare "+
+ "{ Cygwin DragonFly FreeBSD Linux NetBSD SunOS "+
"} for the operating system part of MACHINE_PLATFORM.",
"WARN: filename.mk:3: The pattern \"l*\" cannot match any of "+
"{ aarch64 aarch64eb alpha amd64 arc arm arm26 arm32 "+
@@ -1227,8 +1225,7 @@ func (s *Suite) Test_VartypeCheck_Machin
vt.Output(
"WARN: filename.mk:1: \"linux-i386\" is not a valid platform pattern.",
"WARN: filename.mk:2: The pattern \"nextbsd\" cannot match any of "+
- "{ AIX BSDOS Bitrig Cygwin Darwin DragonFly FreeBSD FreeMiNT GNUkFreeBSD HPUX Haiku "+
- "IRIX Interix Linux Minix MirBSD NetBSD OSF1 OpenBSD QNX SCO_SV SunOS UnixWare "+
+ "{ Cygwin DragonFly FreeBSD Linux NetBSD SunOS "+
"} for the operating system part of ONLY_FOR_PLATFORM.",
"WARN: filename.mk:2: The pattern \"8087\" cannot match any of "+
"{ aarch64 aarch64eb alpha amd64 arc arm arm26 arm32 "+
@@ -1242,8 +1239,7 @@ func (s *Suite) Test_VartypeCheck_Machin
"rs6000 s390 sh3eb sh3el sparc sparc64 vax x86_64 "+
"} for the hardware architecture part of ONLY_FOR_PLATFORM.",
"WARN: filename.mk:3: The pattern \"netbsd\" cannot match any of "+
- "{ AIX BSDOS Bitrig Cygwin Darwin DragonFly FreeBSD FreeMiNT GNUkFreeBSD HPUX Haiku "+
- "IRIX Interix Linux Minix MirBSD NetBSD OSF1 OpenBSD QNX SCO_SV SunOS UnixWare "+
+ "{ Cygwin DragonFly FreeBSD Linux NetBSD SunOS "+
"} for the operating system part of ONLY_FOR_PLATFORM.",
"WARN: filename.mk:3: The pattern \"l*\" cannot match any of "+
"{ aarch64 aarch64eb alpha amd64 arc arm arm26 arm32 "+
Home |
Main Index |
Thread Index |
Old Index