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 5.7.2



details:   https://anonhg.NetBSD.org/pkgsrc/rev/4a5fe740bbb8
branches:  trunk
changeset: 331005:4a5fe740bbb8
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Sun Mar 10 19:01:50 2019 +0000

description:
pkgtools/pkglint: update to 5.7.2

Changes since 5.7.1:

* Fixed detection of GNU_CONFIGURE=yes combined with USE_LANGUAGES
  missing c. This combination tends to fail in the configure phase.

* When the distinfo doesn't contain all hashes for the downloaded
  distfiles (typically SHA512 is missing) and the distfiles are actually
  downloaded to ${PKGSRCDIR}/distfiles, pkglint can now add the missing
  hashes. It only does this if there is at least one existing hash
  and if all existing hashes are correct.

* The check for redundant variables has been improved considerably.
  Before there were several situations in which pkglint didn't get the
  redundant variable definitions right because its internal model only
  mimicked reality. The model has been improved and so have the
  diagnostics.

* Pkglint only warns about wrong permissions (for defining or using
  a variable) when it knows the type of the variable and the permissions
  for the current file. Before, it had also warned if the permissions
  for the current file were not explicitly defined.

* CFLAGS and LDFLAGS may be appended in buildlink3.mk files. This
  had been disallowed before, for no apparent reason. There are several
  places in pkgsrc where especially CFLAGS.${OPSYS} is appended to.

* Cleaned up internal handling of relative paths. Previously pkglint
  sometimes resolved relative paths using the wrong base directory,
  which led to all kinds of wrong warnings and strange behavior.

* Fixed lots of edge cases when parsing Makefile lines. These cases
  don't occur often but experience tells that the most fundamental code
  must be as correct as possible (see the handling of relative paths
  above).

* Lots of refactoring and housekeeping, as always.

diffstat:

 pkgtools/pkglint/Makefile                     |     6 +-
 pkgtools/pkglint/PLIST                        |     6 +-
 pkgtools/pkglint/files/autofix.go             |     6 +-
 pkgtools/pkglint/files/autofix_test.go        |     1 -
 pkgtools/pkglint/files/buildlink3_test.go     |    17 +-
 pkgtools/pkglint/files/category_test.go       |     7 +-
 pkgtools/pkglint/files/check_test.go          |   124 ++-
 pkgtools/pkglint/files/distinfo.go            |   376 +++++-
 pkgtools/pkglint/files/distinfo_test.go       |   443 +++++++-
 pkgtools/pkglint/files/line.go                |     7 +-
 pkgtools/pkglint/files/lines.go               |     1 +
 pkgtools/pkglint/files/logging_test.go        |     4 +-
 pkgtools/pkglint/files/mkline.go              |   353 ++++--
 pkgtools/pkglint/files/mkline_test.go         |   564 ++++++++++-
 pkgtools/pkglint/files/mklinechecker.go       |   115 +-
 pkgtools/pkglint/files/mklinechecker_test.go  |   437 +++++++-
 pkgtools/pkglint/files/mklines.go             |    46 +-
 pkgtools/pkglint/files/mklines_test.go        |   265 +----
 pkgtools/pkglint/files/mkparser.go            |    72 +-
 pkgtools/pkglint/files/mkparser_test.go       |    26 +
 pkgtools/pkglint/files/mktokenslexer.go       |    89 +
 pkgtools/pkglint/files/mktokenslexer_test.go  |   291 +++++
 pkgtools/pkglint/files/package.go             |   256 +++-
 pkgtools/pkglint/files/package_test.go        |   277 +++++-
 pkgtools/pkglint/files/pkglint.1              |     2 +-
 pkgtools/pkglint/files/pkglint.go             |     4 +-
 pkgtools/pkglint/files/pkglint_test.go        |    19 +-
 pkgtools/pkglint/files/pkgsrc.go              |     9 +
 pkgtools/pkglint/files/pkgsrc_test.go         |    23 +
 pkgtools/pkglint/files/plist.go               |     4 +-
 pkgtools/pkglint/files/plist_test.go          |    28 +
 pkgtools/pkglint/files/redundantscope.go      |   279 +++++
 pkgtools/pkglint/files/redundantscope_test.go |  1284 +++++++++++++++++++++++++
 pkgtools/pkglint/files/shell.go               |     8 -
 pkgtools/pkglint/files/shell_test.go          |   100 +-
 pkgtools/pkglint/files/substcontext.go        |    14 +-
 pkgtools/pkglint/files/substcontext_test.go   |    52 +-
 pkgtools/pkglint/files/textproc/lexer.go      |     3 +
 pkgtools/pkglint/files/trace/tracing.go       |     2 +
 pkgtools/pkglint/files/util.go                |   259 +---
 pkgtools/pkglint/files/util_test.go           |   130 +-
 pkgtools/pkglint/files/var.go                 |   278 +++++-
 pkgtools/pkglint/files/var_test.go            |   330 ++++++-
 pkgtools/pkglint/files/vardefs.go             |    80 +-
 pkgtools/pkglint/files/vartype.go             |     4 +
 pkgtools/pkglint/files/vartypecheck.go        |     2 +-
 pkgtools/pkglint/files/vartypecheck_test.go   |   360 +++---
 47 files changed, 5721 insertions(+), 1342 deletions(-)

diffs (truncated from 9683 to 300 lines):

diff -r 34843b330ff9 -r 4a5fe740bbb8 pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Sun Mar 10 18:46:28 2019 +0000
+++ b/pkgtools/pkglint/Makefile Sun Mar 10 19:01:50 2019 +0000
@@ -1,7 +1,6 @@
-# $NetBSD: Makefile,v 1.569 2019/03/09 10:05:10 bsiegert Exp $
+# $NetBSD: Makefile,v 1.570 2019/03/10 19:01:50 rillig Exp $
 
-PKGNAME=       pkglint-5.7.1
-PKGREVISION=   1
+PKGNAME=       pkglint-5.7.2
 CATEGORIES=    pkgtools
 DISTNAME=      tools
 MASTER_SITES=  ${MASTER_SITE_GITHUB:=golang/}
@@ -80,4 +79,5 @@
 BUILDLINK_DEPMETHOD.go-check=  full
 
 .include "../../devel/go-check/buildlink3.mk"
+.include "../../security/go-crypto/buildlink3.mk"
 .include "../../mk/bsd.pkg.mk"
diff -r 34843b330ff9 -r 4a5fe740bbb8 pkgtools/pkglint/PLIST
--- a/pkgtools/pkglint/PLIST    Sun Mar 10 18:46:28 2019 +0000
+++ b/pkgtools/pkglint/PLIST    Sun Mar 10 19:01:50 2019 +0000
@@ -1,4 +1,4 @@
-@comment $NetBSD: PLIST,v 1.10 2019/01/26 16:31:33 rillig Exp $
+@comment $NetBSD: PLIST,v 1.11 2019/03/10 19:01:50 rillig Exp $
 bin/pkglint
 gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint.a
 gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint/getopt.a
@@ -63,6 +63,8 @@
 gopkg/src/netbsd.org/pkglint/mkshtypes_test.go
 gopkg/src/netbsd.org/pkglint/mkshwalker.go
 gopkg/src/netbsd.org/pkglint/mkshwalker_test.go
+gopkg/src/netbsd.org/pkglint/mktokenslexer.go
+gopkg/src/netbsd.org/pkglint/mktokenslexer_test.go
 gopkg/src/netbsd.org/pkglint/mktypes.go
 gopkg/src/netbsd.org/pkglint/mktypes_test.go
 gopkg/src/netbsd.org/pkglint/options.go
@@ -81,6 +83,8 @@
 gopkg/src/netbsd.org/pkglint/pkgver/vercmp_test.go
 gopkg/src/netbsd.org/pkglint/plist.go
 gopkg/src/netbsd.org/pkglint/plist_test.go
+gopkg/src/netbsd.org/pkglint/redundantscope.go
+gopkg/src/netbsd.org/pkglint/redundantscope_test.go
 gopkg/src/netbsd.org/pkglint/regex/regex.go
 gopkg/src/netbsd.org/pkglint/shell.go
 gopkg/src/netbsd.org/pkglint/shell.y
diff -r 34843b330ff9 -r 4a5fe740bbb8 pkgtools/pkglint/files/autofix.go
--- a/pkgtools/pkglint/files/autofix.go Sun Mar 10 18:46:28 2019 +0000
+++ b/pkgtools/pkglint/files/autofix.go Sun Mar 10 19:01:50 2019 +0000
@@ -328,9 +328,9 @@
        {
                // Parsing the continuation marker as variable value is cheating but works well.
                text := strings.TrimSuffix(mkline.raw[0].orignl, "\n")
-               _, _, _, _, _, valueAlign, value, _, _ := MatchVarassign(text)
-               if value != "\\" {
-                       oldWidth = tabWidth(valueAlign)
+               _, a := MatchVarassign(text)
+               if a.value != "\\" {
+                       oldWidth = tabWidth(a.valueAlign)
                }
        }
 
diff -r 34843b330ff9 -r 4a5fe740bbb8 pkgtools/pkglint/files/autofix_test.go
--- a/pkgtools/pkglint/files/autofix_test.go    Sun Mar 10 18:46:28 2019 +0000
+++ b/pkgtools/pkglint/files/autofix_test.go    Sun Mar 10 19:01:50 2019 +0000
@@ -161,7 +161,6 @@
        fix.Apply()
 
        t.CheckOutputLines(
-               "",
                "AUTOFIX: ~/Makefile:2: Replacing \"X\" with \"Y\".",
                "-\tline2",
                "+\tYXXe2")
diff -r 34843b330ff9 -r 4a5fe740bbb8 pkgtools/pkglint/files/buildlink3_test.go
--- a/pkgtools/pkglint/files/buildlink3_test.go Sun Mar 10 18:46:28 2019 +0000
+++ b/pkgtools/pkglint/files/buildlink3_test.go Sun Mar 10 19:01:50 2019 +0000
@@ -474,22 +474,13 @@
        t.CheckOutputLines(
                "WARN: buildlink3.mk:3: LICENSE may not be used in any file; it is a write-only variable.",
                "WARN: buildlink3.mk:3: The variable LICENSE should be quoted as part of a shell word.",
-
                "WARN: buildlink3.mk:8: LICENSE should not be evaluated at load time.",
-               "WARN: buildlink3.mk:8: LICENSE may not be used in any file; it is a write-only variable.",
                "WARN: buildlink3.mk:8: LICENSE should not be evaluated indirectly at load time.",
-               "WARN: buildlink3.mk:8: LICENSE may not be used in any file; it is a write-only variable.",
                "WARN: buildlink3.mk:8: The variable LICENSE should be quoted as part of a shell word.",
-
                "WARN: buildlink3.mk:9: LICENSE should not be evaluated at load time.",
-               "WARN: buildlink3.mk:9: LICENSE may not be used in any file; it is a write-only variable.",
                "WARN: buildlink3.mk:9: LICENSE should not be evaluated indirectly at load time.",
-               "WARN: buildlink3.mk:9: LICENSE may not be used in any file; it is a write-only variable.",
                "WARN: buildlink3.mk:9: The variable LICENSE should be quoted as part of a shell word.",
-
-               "WARN: buildlink3.mk:13: LICENSE may not be used in any file; it is a write-only variable.",
                "WARN: buildlink3.mk:13: The variable LICENSE should be quoted as part of a shell word.",
-
                "WARN: buildlink3.mk:3: Please replace \"${LICENSE}\" with a simple string (also in other variables in this file).")
 }
 
@@ -640,13 +631,9 @@
 
        G.Check(t.File("category/package"))
 
-       // FIXME: Why is appending to LDFLAGS forbidden? It sounds useful.
        t.CheckOutputLines(
-               "WARN: ~/category/package/buildlink3.mk:14: "+
-                       "The variable LDFLAGS.NetBSD may not be appended to in this file; "+
-                       "it would be ok in Makefile, Makefile.common, options.mk or *.mk.",
-               "WARN: ~/category/package/buildlink3.mk:16: "+
-                       "Only buildlink variables for \"package\", "+
+               "WARN: ~/category/package/buildlink3.mk:16: " +
+                       "Only buildlink variables for \"package\", " +
                        "not \"other\" may be set in this file.")
 }
 
diff -r 34843b330ff9 -r 4a5fe740bbb8 pkgtools/pkglint/files/category_test.go
--- a/pkgtools/pkglint/files/category_test.go   Sun Mar 10 18:46:28 2019 +0000
+++ b/pkgtools/pkglint/files/category_test.go   Sun Mar 10 19:01:50 2019 +0000
@@ -290,8 +290,11 @@
 
        CheckdirCategory(t.File("category"))
 
-       // FIXME: Wow. These are quite a few warnings and errors, just because there is
-       //  an additional comment above the COMMENT definition.
+       // These are quite a few warnings and errors, just because there is
+       // an additional comment above the COMMENT definition.
+       // On the other hand, the category Makefiles are so simple and their
+       // structure has been fixed for at least 20 years, therefore this case
+       // is rather exotic anyway.
        t.CheckOutputLines(
                "ERROR: ~/category/Makefile:3: COMMENT= line expected.",
                "NOTE: ~/category/Makefile:2: Empty line expected after this line.",
diff -r 34843b330ff9 -r 4a5fe740bbb8 pkgtools/pkglint/files/check_test.go
--- a/pkgtools/pkglint/files/check_test.go      Sun Mar 10 18:46:28 2019 +0000
+++ b/pkgtools/pkglint/files/check_test.go      Sun Mar 10 19:01:50 2019 +0000
@@ -92,7 +92,6 @@
                _, _ = fmt.Fprintf(os.Stderr, "Cannot chdir back to previous dir: %s", err)
        }
 
-       G = Pkglint{} // unusable because of missing Logger.out and Logger.err
        if out := t.Output(); out != "" {
                var msg strings.Builder
                msg.WriteString("\n")
@@ -106,8 +105,11 @@
                _, _ = fmt.Fprintf(&msg, "\n")
                _, _ = os.Stderr.WriteString(msg.String())
        }
+
        t.tmpdir = ""
        t.DisableTracing()
+
+       G = Pkglint{} // unusable because of missing Logger.out and Logger.err
 }
 
 var _ = check.Suite(new(Suite))
@@ -196,6 +198,36 @@
        return LoadMk(filename, MustSucceed)
 }
 
+// LoadMkInclude loads the given Makefile fragment and all the files it includes,
+// merging all the lines into a single MkLines object.
+//
+// This is useful for testing code related to Package.readMakefile.
+func (t *Tester) LoadMkInclude(relativeFileName string) MkLines {
+       var lines []Line
+
+       // TODO: Include files with multiple-inclusion guard only once.
+       // TODO: Include files without multiple-inclusion guard as often as needed.
+       // TODO: Set an upper limit, to prevent denial of service.
+
+       var load func(filename string)
+       load = func(filename string) {
+               for _, mkline := range NewMkLines(Load(filename, MustSucceed)).mklines {
+                       lines = append(lines, mkline.Line)
+
+                       if mkline.IsInclude() {
+                               included := cleanpath(path.Dir(filename) + "/" + mkline.IncludedFile())
+                               load(included)
+                       }
+               }
+       }
+
+       load(t.File(relativeFileName))
+
+       // This assumes that the test files do not contain parse errors.
+       // Otherwise the diagnostics would appear twice.
+       return NewMkLines(NewLines(t.File(relativeFileName), lines))
+}
+
 // SetUpPkgsrc sets up a minimal but complete pkgsrc installation in the
 // temporary folder, so that pkglint runs without any errors.
 // Individual files may be overwritten by calling other SetUp* methods.
@@ -257,6 +289,8 @@
        // used at load time by packages.
        t.CreateFileLines("mk/bsd.prefs.mk",
                MkRcsID)
+       t.CreateFileLines("mk/bsd.fast.prefs.mk",
+               MkRcsID)
 
        // Category Makefiles require this file for the common definitions.
        t.CreateFileLines("mk/misc/category.mk")
@@ -486,6 +520,69 @@
        G.fileCache.Evict(filename)
 }
 
+// SetUpHierarchy provides a function for creating hierarchies of MkLines
+// that include each other.
+// The hierarchy is created only in memory, nothing is written to disk.
+//
+//  include, get := t.SetUpHierarchy()
+//
+//  include("including.mk",
+//      include("other.mk",
+//          "VAR= other"),
+//      include("module.mk",
+//          "VAR= module",
+//          include("version.mk",
+//              "VAR= version"),
+//          include("env.mk",
+//              "VAR= env")))
+//
+//  mklines := get("including.mk")
+//  module := get("module.mk")
+func (t *Tester) SetUpHierarchy() (
+       include func(filename string, args ...interface{}) MkLines,
+       get func(string) MkLines) {
+
+       files := map[string]MkLines{}
+
+       // FIXME: Define where the filename is relative to: to the file, or to the current directory.
+       include = func(filename string, args ...interface{}) MkLines {
+               var lines []Line
+               lineno := 1
+
+               addLine := func(text string) {
+                       lines = append(lines, t.NewLine(filename, lineno, text))
+                       lineno++
+               }
+
+               for _, arg := range args {
+                       switch arg := arg.(type) {
+                       case string:
+                               addLine(arg)
+                       case MkLines:
+                               text := sprintf(".include %q", arg.lines.FileName)
+                               addLine(text)
+                               lines = append(lines, arg.lines.Lines...)
+                       default:
+                               panic("invalid type")
+                       }
+               }
+
+               mklines := NewMkLines(NewLines(filename, lines))
+               // FIXME: This filename must be relative to the including file.
+               G.Assertf(files[filename] == nil, "MkLines with name %q already exist.", filename)
+               // FIXME: This filename must be relative to the base directory.
+               files[filename] = mklines
+               return mklines
+       }
+
+       get = func(filename string) MkLines {
+               G.Assertf(files[filename] != nil, "MkLines with name %q doesn't exist.", filename)
+               return files[filename]
+       }
+
+       return
+}
+
 // Check delegates a check to the check.Check function.
 // Thereby, there is no need to distinguish between c.Check and t.Check
 // in the test code.
@@ -584,6 +681,11 @@
 
 // NewMkLine creates an in-memory line in the Makefile format with the given text.
 func (t *Tester) NewMkLine(filename string, lineno int, text string) MkLine {
+       basename := path.Base(filename)
+       G.Assertf(
+               hasSuffix(basename, ".mk") || basename == "Makefile" || hasPrefix(basename, "Makefile."),
+               "filename %q must be realistic, otherwise the variable permissions are wrong", filename)
+
        return NewMkLine(t.NewLine(filename, lineno, text))
 }
 
@@ -616,6 +718,11 @@
 // No actual file is created for the lines;
 // see SetUpFileMkLines for loading Makefile fragments with line continuations.
 func (t *Tester) NewMkLines(filename string, lines ...string) MkLines {
+       basename := path.Base(filename)
+       G.Assertf(
+               hasSuffix(basename, ".mk") || basename == "Makefile" || hasPrefix(basename, "Makefile."),
+               "filename %q must be realistic, otherwise the variable permissions are wrong", filename)
+
        var rawText strings.Builder
        for _, line := range lines {
                rawText.WriteString(line)
@@ -633,13 +740,18 @@
        t.stdout.Reset()
        t.stderr.Reset()
        G.Logger.logged = Once{}
+       if G.Logger.out != nil { // Necessary because Main resets the G variable.



Home | Main Index | Thread Index | Old Index