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:           Wed Jan 10 00:39:52 UTC 2018

Modified Files:
        pkgsrc/pkgtools/pkglint: Makefile
        pkgsrc/pkgtools/pkglint/files: mklines.go package.go shell.go
            shell_test.go substcontext.go substcontext_test.go util.go

Log Message:
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.


To generate a diff of this commit:
cvs rdiff -u -r1.523 -r1.524 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.16 -r1.17 pkgsrc/pkgtools/pkglint/files/mklines.go
cvs rdiff -u -r1.21 -r1.22 pkgsrc/pkgtools/pkglint/files/package.go
cvs rdiff -u -r1.19 -r1.20 pkgsrc/pkgtools/pkglint/files/shell.go
cvs rdiff -u -r1.17 -r1.18 pkgsrc/pkgtools/pkglint/files/shell_test.go \
    pkgsrc/pkgtools/pkglint/files/util.go
cvs rdiff -u -r1.9 -r1.10 pkgsrc/pkgtools/pkglint/files/substcontext.go
cvs rdiff -u -r1.8 -r1.9 pkgsrc/pkgtools/pkglint/files/substcontext_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.523 pkgsrc/pkgtools/pkglint/Makefile:1.524
--- pkgsrc/pkgtools/pkglint/Makefile:1.523      Sun Jan  7 17:08:15 2018
+++ pkgsrc/pkgtools/pkglint/Makefile    Wed Jan 10 00:39:52 2018
@@ -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
 

Index: pkgsrc/pkgtools/pkglint/files/mklines.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines.go:1.16 pkgsrc/pkgtools/pkglint/files/mklines.go:1.17
--- pkgsrc/pkgtools/pkglint/files/mklines.go:1.16       Sun Jan  7 17:08:15 2018
+++ pkgsrc/pkgtools/pkglint/files/mklines.go    Wed Jan 10 00:39:52 2018
@@ -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)

Index: pkgsrc/pkgtools/pkglint/files/package.go
diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.21 pkgsrc/pkgtools/pkglint/files/package.go:1.22
--- pkgsrc/pkgtools/pkglint/files/package.go:1.21       Sun Jan  7 01:13:21 2018
+++ pkgsrc/pkgtools/pkglint/files/package.go    Wed Jan 10 00:39:52 2018
@@ -15,16 +15,17 @@ const rePkgname = `^([\w\-.+]+)-(\d(?:\w
 
 // 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 
                }
        }
 }
+
+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
+                       }
+               }
+       }
+}

Index: pkgsrc/pkgtools/pkglint/files/shell.go
diff -u pkgsrc/pkgtools/pkglint/files/shell.go:1.19 pkgsrc/pkgtools/pkglint/files/shell.go:1.20
--- pkgsrc/pkgtools/pkglint/files/shell.go:1.19 Sun Jan  7 01:13:21 2018
+++ pkgsrc/pkgtools/pkglint/files/shell.go      Wed Jan 10 00:39:52 2018
@@ -628,18 +628,28 @@ func (scc *SimpleCommandChecker) checkAu
        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.")
+                               }
                        }
                }
        }

Index: pkgsrc/pkgtools/pkglint/files/shell_test.go
diff -u pkgsrc/pkgtools/pkglint/files/shell_test.go:1.17 pkgsrc/pkgtools/pkglint/files/shell_test.go:1.18
--- pkgsrc/pkgtools/pkglint/files/shell_test.go:1.17    Sun Jan  7 01:13:21 2018
+++ pkgsrc/pkgtools/pkglint/files/shell_test.go Wed Jan 10 00:39:52 2018
@@ -220,12 +220,24 @@ func (s *Suite) Test_ShellLine_CheckShel
                "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_CheckShel
        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_CheckShel
        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) {
Index: pkgsrc/pkgtools/pkglint/files/util.go
diff -u pkgsrc/pkgtools/pkglint/files/util.go:1.17 pkgsrc/pkgtools/pkglint/files/util.go:1.18
--- pkgsrc/pkgtools/pkglint/files/util.go:1.17  Sun Jan  7 01:13:21 2018
+++ pkgsrc/pkgtools/pkglint/files/util.go       Wed Jan 10 00:39:52 2018
@@ -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
 }

Index: pkgsrc/pkgtools/pkglint/files/substcontext.go
diff -u pkgsrc/pkgtools/pkglint/files/substcontext.go:1.9 pkgsrc/pkgtools/pkglint/files/substcontext.go:1.10
--- pkgsrc/pkgtools/pkglint/files/substcontext.go:1.9   Sun Jan 29 14:27:48 2017
+++ pkgsrc/pkgtools/pkglint/files/substcontext.go       Wed Jan 10 00:39:52 2018
@@ -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(mklin
 
        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 M
        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
        }
 }
 
-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
 }

Index: pkgsrc/pkgtools/pkglint/files/substcontext_test.go
diff -u pkgsrc/pkgtools/pkglint/files/substcontext_test.go:1.8 pkgsrc/pkgtools/pkglint/files/substcontext_test.go:1.9
--- pkgsrc/pkgtools/pkglint/files/substcontext_test.go:1.8      Sun Jan 29 14:27:48 2017
+++ pkgsrc/pkgtools/pkglint/files/substcontext_test.go  Wed Jan 10 00:39:52 2018
@@ -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__incom
 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__compl
 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__OPSYS
 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_cl
                "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))
 }



Home | Main Index | Thread Index | Old Index