pkgsrc-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[pkgsrc/trunk]: pkgsrc/pkgtools/pkglint pkglint: Update to 5.5



details:   https://anonhg.NetBSD.org/pkgsrc/rev/1c2dad29ef29
branches:  trunk
changeset: 373737:1c2dad29ef29
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Wed Jan 10 00:39:52 2018 +0000

description:
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:

 pkgtools/pkglint/Makefile                   |    4 +-
 pkgtools/pkglint/files/mklines.go           |    3 +-
 pkgtools/pkglint/files/package.go           |   44 ++++++++--
 pkgtools/pkglint/files/shell.go             |   34 +++++--
 pkgtools/pkglint/files/shell_test.go        |   26 ++++-
 pkgtools/pkglint/files/substcontext.go      |  114 +++++++++++++++++++++------
 pkgtools/pkglint/files/substcontext_test.go |  110 +++++++++++++++++++++++++-
 pkgtools/pkglint/files/util.go              |    8 +-
 8 files changed, 277 insertions(+), 66 deletions(-)

diffs (truncated from 571 to 300 lines):

diff -r ecf7057782fe -r 1c2dad29ef29 pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Wed Jan 10 00:19:48 2018 +0000
+++ b/pkgtools/pkglint/Makefile Wed Jan 10 00:39:52 2018 +0000
@@ -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 -r ecf7057782fe -r 1c2dad29ef29 pkgtools/pkglint/files/mklines.go
--- a/pkgtools/pkglint/files/mklines.go Wed Jan 10 00:19:48 2018 +0000
+++ b/pkgtools/pkglint/files/mklines.go Wed Jan 10 00:39:52 2018 +0000
@@ -102,7 +102,7 @@
 
        CheckLineRcsid(mklines.lines[0], `#\s+`, "# ")
 
-       var substcontext SubstContext
+       substcontext := NewSubstContext()
        var varalign VaralignBlock
        indentation := &mklines.indentation
        indentation.Push(0)
@@ -131,6 +131,7 @@
 
                case mkline.IsCond():
                        ck.checkCond(mklines.forVars, indentation)
+                       substcontext.Conditional(mkline)
 
                case mkline.IsDependency():
                        ck.checkDependencyRule(allowedTargets)
diff -r ecf7057782fe -r 1c2dad29ef29 pkgtools/pkglint/files/package.go
--- a/pkgtools/pkglint/files/package.go Wed Jan 10 00:19:48 2018 +0000
+++ b/pkgtools/pkglint/files/package.go Wed Jan 10 00:39:52 2018 +0000
@@ -15,16 +15,17 @@
 
 // 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 @@
 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 @@
        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 @@
                                NewMkLines(lines).DetermineUsedVariables()
                        }
                }
+               if hasPrefix(path.Base(fname), "PLIST") {
+                       pkg.loadPlistDirs(fname)
+               }
        }
 
        for _, fname := range files {
@@ -884,3 +889,20 @@
                }
        }
 }
+
+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 -r ecf7057782fe -r 1c2dad29ef29 pkgtools/pkglint/files/shell.go
--- a/pkgtools/pkglint/files/shell.go   Wed Jan 10 00:19:48 2018 +0000
+++ b/pkgtools/pkglint/files/shell.go   Wed Jan 10 00:39:52 2018 +0000
@@ -628,18 +628,28 @@
        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 -r ecf7057782fe -r 1c2dad29ef29 pkgtools/pkglint/files/shell_test.go
--- a/pkgtools/pkglint/files/shell_test.go      Wed Jan 10 00:19:48 2018 +0000
+++ b/pkgtools/pkglint/files/shell_test.go      Wed Jan 10 00:39:52 2018 +0000
@@ -220,12 +220,24 @@
                "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 @@
        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 @@
        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 -r ecf7057782fe -r 1c2dad29ef29 pkgtools/pkglint/files/substcontext.go
--- a/pkgtools/pkglint/files/substcontext.go    Wed Jan 10 00:19:48 2018 +0000
+++ b/pkgtools/pkglint/files/substcontext.go    Wed Jan 10 00:39:52 2018 +0000
@@ -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 @@
 
        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":



Home | Main Index | Thread Index | Old Index