pkgsrc-Changes-HG archive

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

[pkgsrc/trunk]: pkgsrc/pkgtools/pkglint pkgtools/pkglint: update to 19.4.2



details:   https://anonhg.NetBSD.org/pkgsrc/rev/248a7c1927c6
branches:  trunk
changeset: 347028:248a7c1927c6
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Mon Jan 06 20:38:42 2020 +0000

description:
pkgtools/pkglint: update to 19.4.2

Changes since 19.4.1:

Fixed variable resolution. Before, variables that had not been defined
in an included file could still end up in the scope of the package,
which made many of the pkglint checks unreliable.

Each condition that is used in the PLIST should be defined somewhere in
the package Makefile or its included files.

When loading a package Makefile, hacks.mk is loaded implicitly at the
end, just as the pkgsrc infrastructure does in mk/bsd.hacks.mk.

diffstat:

 pkgtools/pkglint/Makefile                    |   4 +-
 pkgtools/pkglint/files/check_test.go         |  13 ++-
 pkgtools/pkglint/files/files.go              |   2 +-
 pkgtools/pkglint/files/mkline.go             |   6 +-
 pkgtools/pkglint/files/mklinechecker_test.go |   8 +-
 pkgtools/pkglint/files/mklineparser.go       |   6 +-
 pkgtools/pkglint/files/mklineparser_test.go  |   6 +-
 pkgtools/pkglint/files/mklines.go            |  30 +++++++-
 pkgtools/pkglint/files/mklines_test.go       |   1 +
 pkgtools/pkglint/files/package.go            |   8 ++-
 pkgtools/pkglint/files/plist.go              |  42 +++++++++++-
 pkgtools/pkglint/files/plist_test.go         |  96 ++++++++++++++++++++++++++++
 pkgtools/pkglint/files/util.go               |   6 +
 pkgtools/pkglint/files/util_test.go          |  56 ++++++++++++++++
 pkgtools/pkglint/files/vartypecheck_test.go  |  22 +++---
 15 files changed, 271 insertions(+), 35 deletions(-)

diffs (truncated from 609 to 300 lines):

diff -r 91c95f4e1b08 -r 248a7c1927c6 pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Mon Jan 06 19:17:04 2020 +0000
+++ b/pkgtools/pkglint/Makefile Mon Jan 06 20:38:42 2020 +0000
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.622 2020/01/04 19:53:13 rillig Exp $
+# $NetBSD: Makefile,v 1.623 2020/01/06 20:38:42 rillig Exp $
 
-PKGNAME=       pkglint-19.4.1
+PKGNAME=       pkglint-19.4.2
 CATEGORIES=    pkgtools
 DISTNAME=      tools
 MASTER_SITES=  ${MASTER_SITE_GITHUB:=golang/}
diff -r 91c95f4e1b08 -r 248a7c1927c6 pkgtools/pkglint/files/check_test.go
--- a/pkgtools/pkglint/files/check_test.go      Mon Jan 06 19:17:04 2020 +0000
+++ b/pkgtools/pkglint/files/check_test.go      Mon Jan 06 20:38:42 2020 +0000
@@ -319,7 +319,7 @@
 
        var load func(filename CurrPath)
        load = func(filename CurrPath) {
-               mklines := NewMkLines(Load(filename, MustSucceed), nil)
+               mklines := NewMkLines(Load(filename, MustSucceed), nil, nil)
                for _, mkline := range mklines.mklines {
                        lines = append(lines, mkline.Line)
 
@@ -333,7 +333,7 @@
 
        // This assumes that the test files do not contain parse errors.
        // Otherwise the diagnostics would appear twice.
-       return NewMkLines(NewLines(t.File(filename), lines), nil)
+       return NewMkLines(NewLines(t.File(filename), lines), nil, nil)
 }
 
 // SetUpPkgsrc sets up a minimal but complete pkgsrc installation in the
@@ -444,6 +444,9 @@
 // After calling this method, individual files can be overwritten as necessary.
 // At the end of the setup phase, t.FinishSetUp() must be called to load all
 // the files.
+//
+// If the package path does not really matter for this test,
+// just use "category/package".
 func (t *Tester) SetUpPackage(pkgpath RelPath, makefileLines ...string) CurrPath {
        assertf(
                matches(pkgpath.String(), `^[^/]+/[^/]+$`),
@@ -750,7 +753,7 @@
                        }
                }
 
-               mklines := NewMkLines(NewLines(NewCurrPath(relFilename.AsPath()), lines), nil)
+               mklines := NewMkLines(NewLines(NewCurrPath(relFilename.AsPath()), lines), nil, nil)
                assertf(files[filename] == nil, "MkLines with name %q already exists.", filename)
                files[filename] = mklines
                return mklines
@@ -1054,7 +1057,7 @@
 // This can lead to strange error messages such as "Relative path %s does
 // not exist." because an intermediate directory in the path does not exist.
 //
-// If the filename is irrelevant for the particular test, take filename.mk.
+// If the filename is irrelevant for the particular test, just use filename.mk.
 func (t *Tester) NewMkLines(filename CurrPath, lines ...string) *MkLines {
        return t.NewMkLinesPkg(filename, nil, lines...)
 }
@@ -1070,7 +1073,7 @@
                rawText.WriteString(line)
                rawText.WriteString("\n")
        }
-       return NewMkLines(convertToLogicalLines(filename, rawText.String(), true), pkg)
+       return NewMkLines(convertToLogicalLines(filename, rawText.String(), true), pkg, nil)
 }
 
 // Returns and consumes the output from both stdout and stderr.
diff -r 91c95f4e1b08 -r 248a7c1927c6 pkgtools/pkglint/files/files.go
--- a/pkgtools/pkglint/files/files.go   Mon Jan 06 19:17:04 2020 +0000
+++ b/pkgtools/pkglint/files/files.go   Mon Jan 06 20:38:42 2020 +0000
@@ -19,7 +19,7 @@
        if lines == nil {
                return nil
        }
-       return NewMkLines(lines, pkg)
+       return NewMkLines(lines, pkg, nil)
 }
 
 func Load(filename CurrPath, options LoadOptions) *Lines {
diff -r 91c95f4e1b08 -r 248a7c1927c6 pkgtools/pkglint/files/mkline.go
--- a/pkgtools/pkglint/files/mkline.go  Mon Jan 06 19:17:04 2020 +0000
+++ b/pkgtools/pkglint/files/mkline.go  Mon Jan 06 20:38:42 2020 +0000
@@ -30,7 +30,6 @@
        varparam          string // e.g. "", "perl"
        spaceAfterVarname string
        op                MkOperator //
-       valueAlign        string     // The text up to and including the assignment operator, e.g. VARNAME+=\t
        value             string     // The trimmed value
        valueMk           []*MkToken // The value, sent through splitIntoMkWords
        valueMkRest       string     // nonempty in case of parse errors
@@ -211,7 +210,10 @@
 
 // ValueAlign applies to variable assignments and returns all the text
 // before the variable value, e.g. "VARNAME+=\t".
-func (mkline *MkLine) ValueAlign() string { return mkline.data.(*mkLineAssign).valueAlign }
+func (mkline *MkLine) ValueAlign() string {
+       parts := NewVaralignSplitter().split(mkline.Line.raw[0].Text(), true)
+       return parts.leadingComment + parts.varnameOp + parts.spaceBeforeValue
+}
 
 func (mkline *MkLine) Value() string { return mkline.data.(*mkLineAssign).value }
 
diff -r 91c95f4e1b08 -r 248a7c1927c6 pkgtools/pkglint/files/mklinechecker_test.go
--- a/pkgtools/pkglint/files/mklinechecker_test.go      Mon Jan 06 19:17:04 2020 +0000
+++ b/pkgtools/pkglint/files/mklinechecker_test.go      Mon Jan 06 20:38:42 2020 +0000
@@ -425,7 +425,11 @@
        // The purpose of this "nonexistent" diagnostic is only to show that
        // hacks.mk is indeed parsed and checked.
        t.CheckOutputLines(
-               "ERROR: ~/category/package/hacks.mk:2: " +
+               // XXX: The diagnostics should be the same since they
+               //  describe the same problem.
+               "ERROR: ~/category/package/hacks.mk:2: "+
+                       "Cannot read \"../../category/package/nonexistent.mk\".",
+               "ERROR: ~/category/package/hacks.mk:2: "+
                        "Relative path \"../../category/package/nonexistent.mk\" does not exist.")
 }
 
@@ -495,7 +499,7 @@
                ".endif",
                ".endfor",
                ".endif")
-       mklines := NewMkLines(lines, nil)
+       mklines := NewMkLines(lines, nil, nil)
 
        mklines.Check()
 
diff -r 91c95f4e1b08 -r 248a7c1927c6 pkgtools/pkglint/files/mklineparser.go
--- a/pkgtools/pkglint/files/mklineparser.go    Mon Jan 06 19:17:04 2020 +0000
+++ b/pkgtools/pkglint/files/mklineparser.go    Mon Jan 06 20:38:42 2020 +0000
@@ -5,6 +5,8 @@
        "strings"
 )
 
+// MkLineParser parses a line of text into the syntactical parts of a
+// line in Makefiles.
 type MkLineParser struct{}
 
 func NewMkLineParser() MkLineParser { return MkLineParser{} }
@@ -147,7 +149,6 @@
                varparam:          varnameParam(varname),
                spaceAfterVarname: spaceAfterVarname,
                op:                op,
-               valueAlign:        valueAlign,
                value:             value,
                valueMk:           nil, // filled in lazily
                valueMkRest:       "",  // filled in lazily
@@ -169,7 +170,8 @@
                break
 
        default:
-               before := a.valueAlign
+               parts := NewVaralignSplitter().split(line.raw[0].Text(), true)
+               before := parts.leadingComment + parts.varnameOp + parts.spaceBeforeValue
                after := alignWith(varname+op.String(), before)
 
                fix := line.Autofix()
diff -r 91c95f4e1b08 -r 248a7c1927c6 pkgtools/pkglint/files/mklineparser_test.go
--- a/pkgtools/pkglint/files/mklineparser_test.go       Mon Jan 06 19:17:04 2020 +0000
+++ b/pkgtools/pkglint/files/mklineparser_test.go       Mon Jan 06 20:38:42 2020 +0000
@@ -213,6 +213,10 @@
        testLine := func(line *Line, commented bool, varname, spaceAfterVarname, op, align, value, spaceAfterValue, comment string, diagnostics ...string) {
                text := line.Text
 
+               t.CheckOutputEmpty()
+               valueAlign := NewMkLineParser().Parse(line).ValueAlign()
+               _ = t.Output()
+
                parser := NewMkLineParser()
                splitResult := parser.split(nil, text, true)
                m, actual := parser.MatchVarassign(line, text, &splitResult)
@@ -225,13 +229,13 @@
                        varparam:          varnameParam(varname),
                        spaceAfterVarname: spaceAfterVarname,
                        op:                NewMkOperator(op),
-                       valueAlign:        align,
                        value:             value,
                        valueMk:           nil,
                        valueMkRest:       "",
                        fields:            nil,
                }
                t.CheckDeepEquals(*actual, expected)
+               t.CheckEquals(valueAlign, align)
                t.CheckEquals(splitResult.spaceBeforeComment, spaceAfterValue)
                t.CheckEquals(splitResult.hasComment, comment != "")
                t.CheckEquals(condStr(splitResult.hasComment, "#", "")+splitResult.comment, comment)
diff -r 91c95f4e1b08 -r 248a7c1927c6 pkgtools/pkglint/files/mklines.go
--- a/pkgtools/pkglint/files/mklines.go Mon Jan 06 19:17:04 2020 +0000
+++ b/pkgtools/pkglint/files/mklines.go Mon Jan 06 20:38:42 2020 +0000
@@ -6,7 +6,24 @@
 type MkLines struct {
        mklines []*MkLine
        lines   *Lines
-       pkg     *Package
+
+       // The package that provides further context for cross-checks,
+       // such as the conditionally included files.
+       //
+       // This package should be used mostly as a read-only storage of context
+       // information. To keep the code understandable, only few things should
+       // be changed, if at all. This is exactly the reason that the
+       // extraScope has been moved to a separate variable.
+       //
+       // XXX: Maybe split this field into two: pkg and pkgForModification.
+       pkg *Package
+
+       // The extra scope in which all variable assignments are recorded.
+       // In most cases this is nil.
+       //
+       // When loading the package Makefile with all its included files,
+       // it is set to pkg.vars.
+       extraScope *Scope
 
        allVars       Scope              // The variables after loading the complete file
        buildDefs     map[string]bool    // Variables that are registered in BUILD_DEFS, to ensure that all user-defined variables are added to it.
@@ -36,7 +53,7 @@
        postLine func(mkline *MkLine)
 }
 
-func NewMkLines(lines *Lines, pkg *Package) *MkLines {
+func NewMkLines(lines *Lines, pkg *Package, extraScope *Scope) *MkLines {
        mklines := make([]*MkLine, lines.Len())
        for i, line := range lines.Lines {
                mklines[i] = NewMkLineParser().Parse(line)
@@ -49,6 +66,7 @@
                mklines,
                lines,
                pkg,
+               extraScope,
                NewScope(),
                make(map[string]bool),
                make(map[string]*MkLine),
@@ -154,8 +172,8 @@
 // This controls the "defined but not used" warning.
 func (mklines *MkLines) UseVar(mkline *MkLine, varname string, time VucTime) {
        mklines.allVars.Use(varname, mkline, time)
-       if mklines.pkg != nil {
-               mklines.pkg.vars.Use(varname, mkline, time)
+       if mklines.extraScope != nil {
+               mklines.extraScope.Use(varname, mkline, time)
        }
 }
 
@@ -344,8 +362,8 @@
 // defineVar marks a variable as defined in both the current package and the current file.
 func (mklines *MkLines) defineVar(mkline *MkLine, varname string) {
        mklines.allVars.Define(varname, mkline)
-       if mklines.pkg != nil {
-               mklines.pkg.vars.Define(varname, mkline)
+       if mklines.extraScope != nil {
+               mklines.extraScope.Define(varname, mkline)
        }
 }
 
diff -r 91c95f4e1b08 -r 248a7c1927c6 pkgtools/pkglint/files/mklines_test.go
--- a/pkgtools/pkglint/files/mklines_test.go    Mon Jan 06 19:17:04 2020 +0000
+++ b/pkgtools/pkglint/files/mklines_test.go    Mon Jan 06 20:38:42 2020 +0000
@@ -15,6 +15,7 @@
                MkCvsID,
                "GNU_CONFIGURE=\tyes",
                "CONFIGURE_ENV+=\tX_LIBS=${X11_LDFLAGS:Q}")
+       mklines.extraScope = &pkg.vars
 
        mklines.Check()
 
diff -r 91c95f4e1b08 -r 248a7c1927c6 pkgtools/pkglint/files/package.go
--- a/pkgtools/pkglint/files/package.go Mon Jan 06 19:17:04 2020 +0000
+++ b/pkgtools/pkglint/files/package.go Mon Jan 06 20:38:42 2020 +0000
@@ -181,11 +181,17 @@
                return nil, nil
        }
 
-       allLines := NewMkLines(NewLines("", nil), pkg)
+       allLines := NewMkLines(NewLines("", nil), pkg, &pkg.vars)
        if !pkg.parse(mainLines, allLines, "", true) {
                return nil, nil
        }
 
+       // See mk/bsd.hacks.mk, which is included by mk/bsd.pkg.mk.
+       hacks := LoadMk(pkg.File("${PKGDIR}/hacks.mk"), pkg, NotEmpty)
+       if hacks != nil {
+               _ = pkg.parse(hacks, allLines, "", false)
+       }
+
        // TODO: Is this still necessary? This code is 20 years old and was introduced
        //  when pkglint loaded the package Makefile including all included files into
        //  a single string. Maybe it makes sense to print the file inclusion hierarchy
diff -r 91c95f4e1b08 -r 248a7c1927c6 pkgtools/pkglint/files/plist.go
--- a/pkgtools/pkglint/files/plist.go   Mon Jan 06 19:17:04 2020 +0000
+++ b/pkgtools/pkglint/files/plist.go   Mon Jan 06 20:38:42 2020 +0000
@@ -206,6 +206,7 @@
        }
 
        ck.checkPathMisc(rel, pline)
+       ck.checkPathCond(pline)
 }
 



Home | Main Index | Thread Index | Old Index