diff options
author | rillig <rillig@pkgsrc.org> | 2018-01-10 00:39:52 +0000 |
---|---|---|
committer | rillig <rillig@pkgsrc.org> | 2018-01-10 00:39:52 +0000 |
commit | d0c98098c9447da7f53a01f9a2342294382ea88f (patch) | |
tree | 39a9627a3673e0bd75ff0b47967bd5f7bab023f0 /pkgtools | |
parent | 8475adda0dc468654094742f76f9909006c0d76c (diff) | |
download | pkgsrc-d0c98098c9447da7f53a01f9a2342294382ea88f.tar.gz |
pkglint: Update to 5.5
Changes since 5.4.26:
SUBST blocks are now checked correctly even if they contain conditionals
like .if ... .elif ... .endif.
AUTO_MKDIRS is only suggested for those directories that actually appear
in the PLIST since other directories are not affected by this variable.
Diffstat (limited to 'pkgtools')
-rw-r--r-- | pkgtools/pkglint/Makefile | 4 | ||||
-rw-r--r-- | pkgtools/pkglint/files/mklines.go | 3 | ||||
-rw-r--r-- | pkgtools/pkglint/files/package.go | 44 | ||||
-rw-r--r-- | pkgtools/pkglint/files/shell.go | 34 | ||||
-rw-r--r-- | pkgtools/pkglint/files/shell_test.go | 26 | ||||
-rw-r--r-- | pkgtools/pkglint/files/substcontext.go | 114 | ||||
-rw-r--r-- | pkgtools/pkglint/files/substcontext_test.go | 110 | ||||
-rw-r--r-- | pkgtools/pkglint/files/util.go | 8 |
8 files changed, 277 insertions, 66 deletions
diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile index 7fb845dee02..c28d8e2f2f5 100644 --- a/pkgtools/pkglint/Makefile +++ b/pkgtools/pkglint/Makefile @@ -1,6 +1,6 @@ -# $NetBSD: Makefile,v 1.523 2018/01/07 17:08:15 rillig Exp $ +# $NetBSD: Makefile,v 1.524 2018/01/10 00:39:52 rillig Exp $ -PKGNAME= pkglint-5.4.26 +PKGNAME= pkglint-5.5 DISTFILES= # none CATEGORIES= pkgtools diff --git a/pkgtools/pkglint/files/mklines.go b/pkgtools/pkglint/files/mklines.go index fb301ef712c..a418b3487f0 100644 --- a/pkgtools/pkglint/files/mklines.go +++ b/pkgtools/pkglint/files/mklines.go @@ -102,7 +102,7 @@ func (mklines *MkLines) Check() { CheckLineRcsid(mklines.lines[0], `#\s+`, "# ") - var substcontext SubstContext + substcontext := NewSubstContext() var varalign VaralignBlock indentation := &mklines.indentation indentation.Push(0) @@ -131,6 +131,7 @@ func (mklines *MkLines) Check() { case mkline.IsCond(): ck.checkCond(mklines.forVars, indentation) + substcontext.Conditional(mkline) case mkline.IsDependency(): ck.checkDependencyRule(allowedTargets) diff --git a/pkgtools/pkglint/files/package.go b/pkgtools/pkglint/files/package.go index 609f9d0fac5..7f7c1e2ded0 100644 --- a/pkgtools/pkglint/files/package.go +++ b/pkgtools/pkglint/files/package.go @@ -15,16 +15,17 @@ const rePkgname = `^([\w\-.+]+)-(\d(?:\w|\.\d)*)$` // Package contains data for the pkgsrc package that is currently checked. type Package struct { - Pkgpath string // e.g. "category/pkgdir" - Pkgdir string // PKGDIR from the package Makefile - Filesdir string // FILESDIR from the package Makefile - Patchdir string // PATCHDIR from the package Makefile - DistinfoFile string // DISTINFO_FILE from the package Makefile - EffectivePkgname string // PKGNAME or DISTNAME from the package Makefile, including nb13 - EffectivePkgbase string // The effective PKGNAME without the version - EffectivePkgversion string // The version part of the effective PKGNAME, excluding nb13 - EffectivePkgnameLine MkLine // The origin of the three effective_* values - SeenBsdPrefsMk bool // Has bsd.prefs.mk already been included? + Pkgpath string // e.g. "category/pkgdir" + Pkgdir string // PKGDIR from the package Makefile + Filesdir string // FILESDIR from the package Makefile + Patchdir string // PATCHDIR from the package Makefile + DistinfoFile string // DISTINFO_FILE from the package Makefile + EffectivePkgname string // PKGNAME or DISTNAME from the package Makefile, including nb13 + EffectivePkgbase string // The effective PKGNAME without the version + EffectivePkgversion string // The version part of the effective PKGNAME, excluding nb13 + EffectivePkgnameLine MkLine // The origin of the three effective_* values + SeenBsdPrefsMk bool // Has bsd.prefs.mk already been included? + PlistDirs map[string]bool // Directories mentioned in the PLIST files vardef map[string]MkLine // (varname, varcanon) => line varuse map[string]MkLine // (varname, varcanon) => line @@ -41,6 +42,7 @@ type Package struct { func NewPackage(pkgpath string) *Package { pkg := &Package{ Pkgpath: pkgpath, + PlistDirs: make(map[string]bool), vardef: make(map[string]MkLine), varuse: make(map[string]MkLine), bl3: make(map[string]Line), @@ -178,7 +180,7 @@ func checkdirPackage(pkgpath string) { haveDistinfo := false havePatches := false - // Determine the used variables before checking any of the Makefile fragments. + // Determine the used variables and PLIST directories before checking any of the Makefile fragments. for _, fname := range files { if (hasPrefix(path.Base(fname), "Makefile.") || hasSuffix(fname, ".mk")) && !matches(fname, `patch-`) && @@ -188,6 +190,9 @@ func checkdirPackage(pkgpath string) { NewMkLines(lines).DetermineUsedVariables() } } + if hasPrefix(path.Base(fname), "PLIST") { + pkg.loadPlistDirs(fname) + } } for _, fname := range files { @@ -884,3 +889,20 @@ func (pkg *Package) CheckInclude(mkline MkLine, indentation *Indentation) { } } } + +func (pkg *Package) loadPlistDirs(plistFilename string) { + lines, err := readLines(plistFilename, false) + if err != nil { + return + } + + for _, line := range lines { + text := line.Text + // Keep in sync with PlistChecker.collectFilesAndDirs + if !contains(text, "$") && !contains(text, "@") { + for dir := path.Dir(text); dir != "."; dir = path.Dir(dir) { + pkg.PlistDirs[dir] = true + } + } + } +} diff --git a/pkgtools/pkglint/files/shell.go b/pkgtools/pkglint/files/shell.go index ac6b1450930..90fb2a74d3d 100644 --- a/pkgtools/pkglint/files/shell.go +++ b/pkgtools/pkglint/files/shell.go @@ -628,18 +628,28 @@ func (scc *SimpleCommandChecker) checkAutoMkdirs() { for _, arg := range scc.strcmd.Args { if !contains(arg, "$$") && !matches(arg, `\$\{[_.]*[a-z]`) { if m, dirname := match1(arg, `^(?:\$\{DESTDIR\})?\$\{PREFIX(?:|:Q)\}/(.*)`); m { - scc.shline.mkline.Notef("You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= %s\" instead of %q.", dirname, cmdname) - Explain( - "Many packages include a list of all needed directories in their", - "PLIST file. In such a case, you can just set AUTO_MKDIRS=yes and", - "be done. The pkgsrc infrastructure will then create all directories", - "in advance.", - "", - "To create directories that are not mentioned in the PLIST file, it", - "is easier to just list them in INSTALLATION_DIRS than to execute the", - "commands explicitly. That way, you don't have to think about which", - "of the many INSTALL_*_DIR variables is appropriate, since", - "INSTALLATION_DIRS takes care of that.") + if G.Pkg != nil && G.Pkg.PlistDirs[dirname] { + scc.shline.mkline.Notef("You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= %s\" instead of %q.", dirname, cmdname) + Explain( + "Many packages include a list of all needed directories in their", + "PLIST file. In such a case, you can just set AUTO_MKDIRS=yes and", + "be done. The pkgsrc infrastructure will then create all directories", + "in advance.", + "", + "To create directories that are not mentioned in the PLIST file, it", + "is easier to just list them in INSTALLATION_DIRS than to execute the", + "commands explicitly. That way, you don't have to think about which", + "of the many INSTALL_*_DIR variables is appropriate, since", + "INSTALLATION_DIRS takes care of that.") + } else { + scc.shline.mkline.Notef("You can use \"INSTALLATION_DIRS+= %s\" instead of %q.", dirname, cmdname) + Explain( + "To create directories during installation, it is easier to just", + "list them in INSTALLATION_DIRS than to execute the commands", + "explicitly. That way, you don't have to think about which", + "of the many INSTALL_*_DIR variables is appropriate, since", + "INSTALLATION_DIRS takes care of that.") + } } } } diff --git a/pkgtools/pkglint/files/shell_test.go b/pkgtools/pkglint/files/shell_test.go index 09d10cf947e..c9395b08478 100644 --- a/pkgtools/pkglint/files/shell_test.go +++ b/pkgtools/pkglint/files/shell_test.go @@ -220,12 +220,24 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine(c *check.C) { "WARN: fname:1: The shell command \"cp\" should not be hidden.", "WARN: fname:1: Unknown shell command \"cp\".") + G.Pkg = NewPackage("category/pkgbase") + G.Pkg.PlistDirs["share/pkgbase"] = true + + // A directory that is found in the PLIST. shline.CheckShellCommandLine("${RUN} ${INSTALL_DATA_DIR} share/pkgbase ${PREFIX}/share/pkgbase") s.CheckOutputLines( "NOTE: fname:1: You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= share/pkgbase\" instead of \"${INSTALL_DATA_DIR}\".", "WARN: fname:1: The INSTALL_*_DIR commands can only handle one directory at a time.") + // A directory that is not found in the PLIST. + shline.CheckShellCommandLine("${RUN} ${INSTALL_DATA_DIR} ${PREFIX}/share/other") + + s.CheckOutputLines( + "NOTE: fname:1: You can use \"INSTALLATION_DIRS+= share/other\" instead of \"${INSTALL_DATA_DIR}\".") + + G.Pkg = nil + // See PR 46570, item "1. It does not" shline.CheckShellCommandLine("for x in 1 2 3; do echo \"$$x\" || exit 1; done") @@ -514,21 +526,21 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine__install_dir(c *check.C) { shline.CheckShellCommandLine(shline.mkline.Shellcmd()) s.CheckOutputLines( - "NOTE: Makefile:85: You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= dir1\" instead of \"${INSTALL_DATA_DIR}\".", - "NOTE: Makefile:85: You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= dir2\" instead of \"${INSTALL_DATA_DIR}\".", + "NOTE: Makefile:85: You can use \"INSTALLATION_DIRS+= dir1\" instead of \"${INSTALL_DATA_DIR}\".", + "NOTE: Makefile:85: You can use \"INSTALLATION_DIRS+= dir2\" instead of \"${INSTALL_DATA_DIR}\".", "WARN: Makefile:85: The INSTALL_*_DIR commands can only handle one directory at a time.") shline.CheckShellCommandLine("${INSTALL_DATA_DIR} -d -m 0755 ${DESTDIR}${PREFIX}/share/examples/gdchart") // No warning about multiple directories, since 0755 is an option, not an argument. s.CheckOutputLines( - "NOTE: Makefile:85: You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= share/examples/gdchart\" instead of \"${INSTALL_DATA_DIR}\".") + "NOTE: Makefile:85: You can use \"INSTALLATION_DIRS+= share/examples/gdchart\" instead of \"${INSTALL_DATA_DIR}\".") shline.CheckShellCommandLine("${INSTALL_DATA_DIR} -d -m 0755 ${DESTDIR}${PREFIX}/dir1 ${PREFIX}/dir2") s.CheckOutputLines( - "NOTE: Makefile:85: You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= dir1\" instead of \"${INSTALL_DATA_DIR}\".", - "NOTE: Makefile:85: You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= dir2\" instead of \"${INSTALL_DATA_DIR}\".", + "NOTE: Makefile:85: You can use \"INSTALLATION_DIRS+= dir1\" instead of \"${INSTALL_DATA_DIR}\".", + "NOTE: Makefile:85: You can use \"INSTALLATION_DIRS+= dir2\" instead of \"${INSTALL_DATA_DIR}\".", "WARN: Makefile:85: The INSTALL_*_DIR commands can only handle one directory at a time.") } @@ -539,8 +551,8 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine__install_option_d(c *check. shline.CheckShellCommandLine(shline.mkline.Shellcmd()) s.CheckOutputLines( - "NOTE: Makefile:85: You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= dir1\" instead of \"${INSTALL} -d\".", - "NOTE: Makefile:85: You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= dir2\" instead of \"${INSTALL} -d\".") + "NOTE: Makefile:85: You can use \"INSTALLATION_DIRS+= dir1\" instead of \"${INSTALL} -d\".", + "NOTE: Makefile:85: You can use \"INSTALLATION_DIRS+= dir2\" instead of \"${INSTALL} -d\".") } func (s *Suite) Test_ShellLine__shell_comment_with_line_continuation(c *check.C) { diff --git a/pkgtools/pkglint/files/substcontext.go b/pkgtools/pkglint/files/substcontext.go index 9a4bc01a815..1fab268301f 100644 --- a/pkgtools/pkglint/files/substcontext.go +++ b/pkgtools/pkglint/files/substcontext.go @@ -1,21 +1,55 @@ package main +import "netbsd.org/pkglint/trace" + // SubstContext records the state of a block of variable assignments // that make up a SUBST class (see `mk/subst.mk`). type SubstContext struct { - id string - stage string - message string - files []string - sed []string - vars []string - filterCmd string + id string + stage string + message string + curr *SubstContextStats + inAllBranches SubstContextStats + filterCmd string +} + +func NewSubstContext() *SubstContext { + return &SubstContext{curr: &SubstContextStats{}} +} + +type SubstContextStats struct { + seenFiles bool + seenSed bool + seenVars bool + seenTransform bool + prev *SubstContextStats +} + +func (st *SubstContextStats) Copy() *SubstContextStats { + return &SubstContextStats{st.seenFiles, st.seenSed, st.seenVars, st.seenTransform, st} +} + +func (st *SubstContextStats) And(other *SubstContextStats) { + st.seenFiles = st.seenFiles && other.seenFiles + st.seenSed = st.seenSed && other.seenSed + st.seenVars = st.seenVars && other.seenVars + st.seenTransform = st.seenTransform && other.seenTransform +} + +func (st *SubstContextStats) Or(other SubstContextStats) { + st.seenFiles = st.seenFiles || other.seenFiles + st.seenSed = st.seenSed || other.seenSed + st.seenVars = st.seenVars || other.seenVars + st.seenTransform = st.seenTransform || other.seenTransform } func (ctx *SubstContext) Varassign(mkline MkLine) { if !G.opts.WarnExtra { return } + if trace.Tracing { + trace.Stepf("SubstContext.Varassign %#v %v#", ctx.curr, ctx.inAllBranches) + } varname := mkline.Varname() op := mkline.Op() @@ -62,27 +96,59 @@ func (ctx *SubstContext) Varassign(mkline MkLine) { switch varbase { case "SUBST_STAGE": - ctx.dup(mkline, &ctx.stage, varname, value) + ctx.dupString(mkline, &ctx.stage, varname, value) case "SUBST_MESSAGE": - ctx.dup(mkline, &ctx.message, varname, value) + ctx.dupString(mkline, &ctx.message, varname, value) case "SUBST_FILES": - ctx.duplist(mkline, &ctx.files, varname, op, value) + ctx.dupBool(mkline, &ctx.curr.seenFiles, varname, op, value) case "SUBST_SED": - ctx.duplist(mkline, &ctx.sed, varname, op, value) - case "SUBST_FILTER_CMD": - ctx.dup(mkline, &ctx.filterCmd, varname, value) + ctx.dupBool(mkline, &ctx.curr.seenSed, varname, op, value) + ctx.curr.seenTransform = true case "SUBST_VARS": - ctx.duplist(mkline, &ctx.vars, varname, op, value) + ctx.dupBool(mkline, &ctx.curr.seenVars, varname, op, value) + ctx.curr.seenTransform = true + case "SUBST_FILTER_CMD": + ctx.dupString(mkline, &ctx.filterCmd, varname, value) + ctx.curr.seenTransform = true default: mkline.Warnf("Foreign variable %q in SUBST block.", varname) } } +func (ctx *SubstContext) Conditional(mkline MkLine) { + if ctx.id == "" || !G.opts.WarnExtra { + return + } + + if trace.Tracing { + trace.Stepf("+ SubstContext.Conditional %#v %v#", ctx.curr, ctx.inAllBranches) + } + dir := mkline.Directive() + if dir == "if" { + ctx.inAllBranches = SubstContextStats{true, true, true, true, nil} + } + if dir == "elif" || dir == "else" || dir == "endif" { + if ctx.curr.prev != nil { // Don't crash on malformed input + ctx.inAllBranches.And(ctx.curr) + ctx.curr = ctx.curr.prev + } + } + if dir == "if" || dir == "elif" || dir == "else" { + ctx.curr = ctx.curr.Copy() + } + if dir == "endif" { + ctx.curr.Or(ctx.inAllBranches) + } + if trace.Tracing { + trace.Stepf("- SubstContext.Conditional %#v %v#", ctx.curr, ctx.inAllBranches) + } +} + func (ctx *SubstContext) IsComplete() bool { return ctx.id != "" && ctx.stage != "" && - len(ctx.files) != 0 && - (len(ctx.sed) != 0 || len(ctx.vars) != 0 || ctx.filterCmd != "") + ctx.curr.seenFiles && + ctx.curr.seenTransform } func (ctx *SubstContext) Finish(mkline MkLine) { @@ -92,19 +158,17 @@ func (ctx *SubstContext) Finish(mkline MkLine) { if ctx.stage == "" { mkline.Warnf("Incomplete SUBST block: %s missing.", ctx.varname("SUBST_STAGE")) } - if len(ctx.files) == 0 { + if !ctx.curr.seenFiles { mkline.Warnf("Incomplete SUBST block: %s missing.", ctx.varname("SUBST_FILES")) } - if len(ctx.sed) == 0 && len(ctx.vars) == 0 && ctx.filterCmd == "" { + if !ctx.curr.seenTransform { mkline.Warnf("Incomplete SUBST block: %s, %s or %s missing.", ctx.varname("SUBST_SED"), ctx.varname("SUBST_VARS"), ctx.varname("SUBST_FILTER_CMD")) } ctx.id = "" ctx.stage = "" ctx.message = "" - ctx.files = nil - ctx.sed = nil - ctx.vars = nil + ctx.curr = &SubstContextStats{} ctx.filterCmd = "" } @@ -116,16 +180,16 @@ func (ctx *SubstContext) varname(varbase string) string { } } -func (ctx *SubstContext) dup(mkline MkLine, pstr *string, varname, value string) { +func (ctx *SubstContext) dupString(mkline MkLine, pstr *string, varname, value string) { if *pstr != "" { mkline.Warnf("Duplicate definition of %q.", varname) } *pstr = value } -func (ctx *SubstContext) duplist(mkline MkLine, plist *[]string, varname string, op MkOperator, value string) { - if len(*plist) > 0 && op != opAssignAppend { +func (ctx *SubstContext) dupBool(mkline MkLine, flag *bool, varname string, op MkOperator, value string) { + if *flag && op != opAssignAppend { mkline.Warnf("All but the first %q lines should use the \"+=\" operator.", varname) } - *plist = append(*plist, value) + *flag = true } diff --git a/pkgtools/pkglint/files/substcontext_test.go b/pkgtools/pkglint/files/substcontext_test.go index 1a33cbc9a6d..6ddeedea2cb 100644 --- a/pkgtools/pkglint/files/substcontext_test.go +++ b/pkgtools/pkglint/files/substcontext_test.go @@ -1,13 +1,14 @@ package main import ( - check "gopkg.in/check.v1" + "fmt" + "gopkg.in/check.v1" ) func (s *Suite) Test_SubstContext__incomplete(c *check.C) { s.Init(c) G.opts.WarnExtra = true - ctx := new(SubstContext) + ctx := NewSubstContext() ctx.Varassign(newSubstLine(10, "PKGNAME=pkgname-1.0")) @@ -34,7 +35,7 @@ func (s *Suite) Test_SubstContext__incomplete(c *check.C) { func (s *Suite) Test_SubstContext__complete(c *check.C) { s.Init(c) G.opts.WarnExtra = true - ctx := new(SubstContext) + ctx := NewSubstContext() ctx.Varassign(newSubstLine(10, "PKGNAME=pkgname-1.0")) ctx.Varassign(newSubstLine(11, "SUBST_CLASSES+=p")) @@ -55,7 +56,7 @@ func (s *Suite) Test_SubstContext__complete(c *check.C) { func (s *Suite) Test_SubstContext__OPSYSVARS(c *check.C) { s.Init(c) G.opts.WarnExtra = true - ctx := new(SubstContext) + ctx := NewSubstContext() ctx.Varassign(newSubstLine(11, "SUBST_CLASSES.SunOS+=prefix")) ctx.Varassign(newSubstLine(12, "SUBST_CLASSES.NetBSD+=prefix")) @@ -73,7 +74,7 @@ func (s *Suite) Test_SubstContext__OPSYSVARS(c *check.C) { func (s *Suite) Test_SubstContext__no_class(c *check.C) { s.Init(c) s.UseCommandLine("-Wextra") - ctx := new(SubstContext) + ctx := NewSubstContext() ctx.Varassign(newSubstLine(10, "UNRELATED=anything")) ctx.Varassign(newSubstLine(11, "SUBST_FILES.repl+=Makefile.in")) @@ -85,6 +86,105 @@ func (s *Suite) Test_SubstContext__no_class(c *check.C) { "WARN: Makefile:13: Incomplete SUBST block: SUBST_STAGE.repl missing.") } +func (s *Suite) Test_SubstContext__conditionals(c *check.C) { + s.Init(c) + s.UseCommandLine("-Wextra") + + simulateSubstLines( + "10: SUBST_CLASSES+= os", + "11: SUBST_STAGE.os= post-configure", + "12: SUBST_MESSAGE.os= Guessing operating system", + "13: SUBST_FILES.os= guess-os.h", + "14: .if ${OPSYS} == NetBSD", + "15: SUBST_FILTER_CMD.os= ${SED} -e s,@OPSYS@,NetBSD,", + "16: .elif ${OPSYS} == Darwin", + "17: SUBST_SED.os= -e s,@OPSYS@,Darwin1,", + "18: SUBST_SED.os= -e s,@OPSYS@,Darwin2,", + "19: .elif ${OPSYS} == Linux", + "18: SUBST_SED.os= -e s,@OPSYS@,Linux,", + "19: .else", + "20: SUBST_VARS.os= OPSYS", + "21: .endif", + "22: ") + + // All the other lines are correctly determined as being alternatives + // to each other. And since every branch contains some transformation + // (SED, VARS, FILTER_CMD), everything is fine. + s.CheckOutputLines( + "WARN: Makefile:18: All but the first \"SUBST_SED.os\" lines should use the \"+=\" operator.") +} + +func (s *Suite) Test_SubstContext__one_conditional_missing_transformation(c *check.C) { + s.Init(c) + s.UseCommandLine("-Wextra") + + simulateSubstLines( + "10: SUBST_CLASSES+= os", + "11: SUBST_STAGE.os= post-configure", + "12: SUBST_MESSAGE.os= Guessing operating system", + "13: SUBST_FILES.os= guess-os.h", + "14: .if ${OPSYS} == NetBSD", + "15: SUBST_FILES.os= -e s,@OpSYS@,NetBSD,", // A simple typo, this should be SUBST_SED. + "16: .elif ${OPSYS} == Darwin", + "17: SUBST_SED.os= -e s,@OPSYS@,Darwin1,", + "18: SUBST_SED.os= -e s,@OPSYS@,Darwin2,", + "19: .else", + "20: SUBST_VARS.os= OPSYS", + "21: .endif", + "22: ") + + s.CheckOutputLines( + "WARN: Makefile:15: All but the first \"SUBST_FILES.os\" lines should use the \"+=\" operator.", + "WARN: Makefile:18: All but the first \"SUBST_SED.os\" lines should use the \"+=\" operator.", + "WARN: Makefile:22: Incomplete SUBST block: SUBST_SED.os, SUBST_VARS.os or SUBST_FILTER_CMD.os missing.") +} + +func (s *Suite) Test_SubstContext__nested_conditionals(c *check.C) { + s.Init(c) + s.UseCommandLine("-Wextra") + + simulateSubstLines( + "10: SUBST_CLASSES+= os", + "11: SUBST_STAGE.os= post-configure", + "12: SUBST_MESSAGE.os= Guessing operating system", + "14: .if ${OPSYS} == NetBSD", + "13: SUBST_FILES.os= guess-netbsd.h", + "15: . if ${ARCH} == i386", + "16: SUBST_FILTER_CMD.os= ${SED} -e s,@OPSYS,NetBSD-i386,", + "17: . elif ${ARCH} == x86_64", + "18: SUBST_VARS.os= OPSYS", + "19: . else", + "20: SUBST_SED.os= -e s,@OPSYS,NetBSD-unknown", + "21: . endif", + "22: .else", + "23: SUBST_SED.os= -e s,@OPSYS@,unknown,", + "24: .endif", + "25: ") + + // The branch in line 23 omits SUBST_FILES. + s.CheckOutputLines( + "WARN: Makefile:25: Incomplete SUBST block: SUBST_FILES.os missing.") +} + +func simulateSubstLines(texts ...string) { + ctx := NewSubstContext() + for _, lineText := range texts { + var lineno int + fmt.Sscanf(lineText[0:4], "%d: ", &lineno) + text := lineText[4:] + line := newSubstLine(lineno, text) + + switch { + case text == "": + ctx.Finish(line) + case hasPrefix(text, "."): + ctx.Conditional(line) + default: + ctx.Varassign(line) + } + } +} + func newSubstLine(lineno int, text string) MkLine { return NewMkLine(NewLine("Makefile", lineno, text, nil)) } diff --git a/pkgtools/pkglint/files/util.go b/pkgtools/pkglint/files/util.go index aaf8f19bfae..e8658a0a33a 100644 --- a/pkgtools/pkglint/files/util.go +++ b/pkgtools/pkglint/files/util.go @@ -256,9 +256,11 @@ func dirglob(dirname string) []string { if err != nil { return nil } - fnames := make([]string, len(fis)) - for i, fi := range fis { - fnames[i] = dirname + "/" + fi.Name() + var fnames []string + for _, fi := range fis { + if !(isIgnoredFilename(fi.Name())) { + fnames = append(fnames, dirname+"/"+fi.Name()) + } } return fnames } |