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.6



details:   https://anonhg.NetBSD.org/pkgsrc/rev/30a70f57d8f7
branches:  trunk
changeset: 332731:30a70f57d8f7
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Tue Apr 23 21:20:49 2019 +0000

description:
pkgtools/pkglint: update to 5.7.6

Changes since 5.7.5:

* The explanation for distfile hashes is only given when the distfiles
  actually need to be downloaded. If they are already there, no
  explanation is necessary.

* Makefile lines that are commented and have line continuations are
  properly parsed. This affects the autofix for variable value
  realignment.

* Variable permissions are not checked in hacks.mk since pkgsrc
  developers who know about hacks.mk probably know what they are doing.
  From hacks.mk files, builtin.mk files may be included directly, for
  the same reason.

* Expressions of the form !empty(PKGPATH:Mpattern), when PKGPATH is not
  a list variable and pattern has no wildcards, can be written in a
  simpler form, and pkglint autofixes this. For example the above
  expression is transformed into ${PKGPATH} == pattern. This
  transformation reduces the amount of double negations (!empty) in the
  code.

* Duplicate warnings about invalid relative ../package have been merged.

* TOOLS_ALIASES are properly resolved. The line USE_TOOLS=ggrep makes
  the tools grep, egrep and fgrep known to pkglint, in the same way as
  in the pkgsrc infrastructure.

* The diagnostics for missing or unnecessary distinfo files have been
  improved to provide some guidance.

* Packages that use MESSAGE_SRC to build the message from multiple files
  no longer produce a warning for malformed message files. These files
  are simply skipped.

diffstat:

 pkgtools/pkglint/Makefile                       |    4 +-
 pkgtools/pkglint/files/autofix_test.go          |   23 +-
 pkgtools/pkglint/files/check_test.go            |   17 +-
 pkgtools/pkglint/files/distinfo.go              |   47 ++--
 pkgtools/pkglint/files/distinfo_test.go         |   76 +++++--
 pkgtools/pkglint/files/files.go                 |    6 +-
 pkgtools/pkglint/files/files_test.go            |   10 +-
 pkgtools/pkglint/files/mklinechecker.go         |  216 +++++++++++++++++-----
 pkgtools/pkglint/files/mklinechecker_test.go    |  226 +++++++++++++++++++++--
 pkgtools/pkglint/files/mklines.go               |   38 +++-
 pkgtools/pkglint/files/mklines_varalign_test.go |    4 +-
 pkgtools/pkglint/files/mkparser.go              |   18 +-
 pkgtools/pkglint/files/mkparser_test.go         |    8 +-
 pkgtools/pkglint/files/package.go               |   41 ++--
 pkgtools/pkglint/files/package_test.go          |   10 +-
 pkgtools/pkglint/files/pkglint.go               |   12 +-
 pkgtools/pkglint/files/pkglint_test.go          |   37 ++-
 pkgtools/pkglint/files/pkgsrc.go                |    6 +-
 pkgtools/pkglint/files/shell.go                 |    6 +-
 pkgtools/pkglint/files/tools.go                 |   85 +++++++-
 pkgtools/pkglint/files/tools_test.go            |  135 +++++++++++---
 pkgtools/pkglint/files/vardefs.go               |    9 +-
 22 files changed, 785 insertions(+), 249 deletions(-)

diffs (truncated from 1804 to 300 lines):

diff -r 381230051221 -r 30a70f57d8f7 pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Tue Apr 23 20:17:12 2019 +0000
+++ b/pkgtools/pkglint/Makefile Tue Apr 23 21:20:49 2019 +0000
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.575 2019/04/20 17:43:24 rillig Exp $
+# $NetBSD: Makefile,v 1.576 2019/04/23 21:20:49 rillig Exp $
 
-PKGNAME=       pkglint-5.7.5
+PKGNAME=       pkglint-5.7.6
 CATEGORIES=    pkgtools
 DISTNAME=      tools
 MASTER_SITES=  ${MASTER_SITE_GITHUB:=golang/}
diff -r 381230051221 -r 30a70f57d8f7 pkgtools/pkglint/files/autofix_test.go
--- a/pkgtools/pkglint/files/autofix_test.go    Tue Apr 23 20:17:12 2019 +0000
+++ b/pkgtools/pkglint/files/autofix_test.go    Tue Apr 23 21:20:49 2019 +0000
@@ -897,11 +897,24 @@
        fix.Anyway()
        fix.Apply()
 
-       // The replacement text is not found, therefore the note should not be logged.
-       // Because of fix.Anyway, the note should be logged anyway.
-       // But because of the --autofix option, the note should not be logged.
-       // Even if the --show-autofix option is explicitly given, the note should not be logged.
-       // Therefore, in the end nothing is shown in this case.
+       // The text to be replaced is not found. Because nothing is fixed here,
+       // there's no need to log anything.
+       //
+       // But fix.Anyway is set, therefore the diagnostic should be logged even
+       // though it cannot be fixed automatically. This comes handy in situations
+       // where simple cases can be fixed automatically and more complex cases
+       // (often involving special characters that need to be escaped properly)
+       // should nevertheless result in a diagnostics.
+       //
+       // The --autofix option is set, which means that the diagnostics don't
+       // get logged, only the actual fixes do.
+       //
+       // But then there's also the --show-autofix option, which logs the
+       // corresponding diagnostic for each autofix that actually changes
+       // something. But this autofix doesn't change anything, therefore even
+       // the --show-autofix doesn't have an effect.
+       //
+       // Therefore, in the end nothing is logged in this case.
        t.CheckOutputEmpty()
 }
 
diff -r 381230051221 -r 30a70f57d8f7 pkgtools/pkglint/files/check_test.go
--- a/pkgtools/pkglint/files/check_test.go      Tue Apr 23 20:17:12 2019 +0000
+++ b/pkgtools/pkglint/files/check_test.go      Tue Apr 23 21:20:49 2019 +0000
@@ -187,7 +187,7 @@
 }
 
 func (t *Tester) SetUpTool(name, varname string, validity Validity) *Tool {
-       return G.Pkgsrc.Tools.def(name, varname, false, validity)
+       return G.Pkgsrc.Tools.def(name, varname, false, validity, nil)
 }
 
 // SetUpFileLines creates a temporary file and writes the given lines to it.
@@ -646,6 +646,9 @@
 }
 
 // Main runs the pkglint main program with the given command line arguments.
+//
+// Arguments that name existing files or directories in the temporary test
+// directory are transformed to their actual paths.
 func (t *Tester) Main(args ...string) int {
        if t.seenFinish && !t.seenMain {
                t.Errorf("Calling t.FinishSetup() before t.Main() is redundant " +
@@ -659,7 +662,17 @@
        G.warnings = 0
        G.logged = Once{}
 
-       argv := append([]string{"pkglint"}, args...)
+       argv := []string{"pkglint"}
+       for _, arg := range args {
+               fileArg := t.File(arg)
+               _, err := os.Lstat(fileArg)
+               if err == nil {
+                       argv = append(argv, fileArg)
+               } else {
+                       argv = append(argv, arg)
+               }
+       }
+
        return G.Main(argv...)
 }
 
diff -r 381230051221 -r 30a70f57d8f7 pkgtools/pkglint/files/distinfo.go
--- a/pkgtools/pkglint/files/distinfo.go        Tue Apr 23 20:17:12 2019 +0000
+++ b/pkgtools/pkglint/files/distinfo.go        Tue Apr 23 21:20:49 2019 +0000
@@ -195,31 +195,32 @@
 
        distdir := G.Pkgsrc.File("distfiles")
 
-       // It's a rare situation that the explanation is generated
-       // this far from the corresponding diagnostic.
-       // This explanation only makes sense when there are some
-       // hashes missing that can be automatically added by pkglint.
-       line.Explain(
-               "To add the missing lines to the distinfo file, run",
-               sprintf("\t%s", bmake("distinfo")),
-               "for each variant of the package until all distfiles are downloaded to",
-               "${PKGSRCDIR}/distfiles.",
-               "",
-               "The variants are typically selected by setting EMUL_PLATFORM",
-               "or similar variables in the command line.",
-               "",
-               "After that, run",
-               sprintf("%q", "cvs update -C distinfo"),
-               "to revert the distinfo file to the previous state, since the above",
-               "commands have removed some of the entries.",
-               "",
-               "After downloading all possible distfiles, run",
-               sprintf("%q,", "pkglint --autofix"),
-               "which will find the downloaded distfiles and add the missing",
-               "hashes to the distinfo file.")
-
        distfile := cleanpath(distdir + "/" + info.filename())
        if !fileExists(distfile) {
+
+               // It's a rare situation that the explanation is generated
+               // this far from the corresponding diagnostic.
+               // This explanation only makes sense when there are some
+               // hashes missing that can be automatically added by pkglint.
+               line.Explain(
+                       "To add the missing lines to the distinfo file, run",
+                       sprintf("\t%s", bmake("distinfo")),
+                       "for each variant of the package until all distfiles are downloaded to",
+                       "${PKGSRCDIR}/distfiles.",
+                       "",
+                       "The variants are typically selected by setting EMUL_PLATFORM",
+                       "or similar variables in the command line.",
+                       "",
+                       "After that, run",
+                       sprintf("%q", "cvs update -C distinfo"),
+                       "to revert the distinfo file to the previous state, since the above",
+                       "commands have removed some of the entries.",
+                       "",
+                       "After downloading all possible distfiles, run",
+                       sprintf("%q,", "pkglint --autofix"),
+                       "which will find the downloaded distfiles and add the missing",
+                       "hashes to the distinfo file.")
+
                return
        }
 
diff -r 381230051221 -r 30a70f57d8f7 pkgtools/pkglint/files/distinfo_test.go
--- a/pkgtools/pkglint/files/distinfo_test.go   Tue Apr 23 20:17:12 2019 +0000
+++ b/pkgtools/pkglint/files/distinfo_test.go   Tue Apr 23 21:20:49 2019 +0000
@@ -501,10 +501,10 @@
                "ERROR: ~/category/package/distinfo:5: Patch patch-nonexistent does not exist.")
 }
 
-// When there is at least one correct hash for a distfile, running
-// pkglint --autofix adds the missing hashes, provided the distfile has been
-// downloaded to pkgsrc/distfiles, which is the standard distfiles location.
-func (s *Suite) Test_distinfoLinesChecker_checkAlgorithmsDistfile__add_missing_hashes(c *check.C) {
+// When there is at least one correct hash for a distfile and the distfile
+// has already been downloaded to pkgsrc/distfiles, which is the standard
+// distfiles location, running pkglint --autofix adds the missing hashes.
+func (s *Suite) Test_distinfoLinesChecker_checkAlgorithmsDistfile__add_missing_hashes_for_existing_distfile(c *check.C) {
        t := s.Init(c)
 
        t.SetUpCommandLine("-Wall", "--explain")
@@ -527,23 +527,6 @@
                "ERROR: ~/category/package/distinfo:3: "+
                        "Expected SHA1, RMD160, SHA512, Size checksums for \"package-1.0.txt\", "+
                        "got RMD160, Size, CRC32.",
-               "",
-               "\tTo add the missing lines to the distinfo file, run",
-               "\t\t"+confMake+" distinfo",
-               "\tfor each variant of the package until all distfiles are downloaded",
-               "\tto ${PKGSRCDIR}/distfiles.",
-               "",
-               "\tThe variants are typically selected by setting EMUL_PLATFORM or",
-               "\tsimilar variables in the command line.",
-               "",
-               "\tAfter that, run \"cvs update -C distinfo\" to revert the distinfo file",
-               "\tto the previous state, since the above commands have removed some of",
-               "\tthe entries.",
-               "",
-               "\tAfter downloading all possible distfiles, run \"pkglint --autofix\",",
-               "\twhich will find the downloaded distfiles and add the missing hashes",
-               "\tto the distinfo file.",
-               "",
                "ERROR: ~/category/package/distinfo:3: Missing SHA1 hash for package-1.0.txt.",
                "ERROR: ~/category/package/distinfo:3: Missing SHA512 hash for package-1.0.txt.")
 
@@ -583,6 +566,57 @@
                        "got SHA1, RMD160, SHA512, Size, CRC32.")
 }
 
+// When some of the hashes for a distfile are missing, pkglint can calculate
+// them. In order to do this, the distfile needs to be downloaded first. This
+// often requires manual work, otherwise it would have been done already.
+//
+// Since the distfile has not been downloaded in this test case, pkglint can
+// only explain how to download the distfile.
+func (s *Suite) Test_distinfoLinesChecker_checkAlgorithmsDistfile__add_missing_hashes_for_nonexistent_distfile(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpCommandLine("-Wall", "--explain")
+       t.SetUpPackage("category/package")
+       t.CreateFileLines("category/package/distinfo",
+               RcsID,
+               "",
+               "RMD160 (package-1.0.txt) = 1a88147a0344137404c63f3b695366eab869a98a",
+               "Size (package-1.0.txt) = 13 bytes",
+               "CRC32 (package-1.0.txt) = asdf")
+       t.FinishSetUp()
+
+       G.Check(t.File("category/package"))
+
+       t.CheckOutputLines(
+               "ERROR: ~/category/package/distinfo:3: "+
+                       "Expected SHA1, RMD160, SHA512, Size checksums for \"package-1.0.txt\", "+
+                       "got RMD160, Size, CRC32.",
+               "",
+               "\tTo add the missing lines to the distinfo file, run",
+               "\t\t"+confMake+" distinfo",
+               "\tfor each variant of the package until all distfiles are downloaded",
+               "\tto ${PKGSRCDIR}/distfiles.",
+               "",
+               "\tThe variants are typically selected by setting EMUL_PLATFORM or",
+               "\tsimilar variables in the command line.",
+               "",
+               "\tAfter that, run \"cvs update -C distinfo\" to revert the distinfo file",
+               "\tto the previous state, since the above commands have removed some of",
+               "\tthe entries.",
+               "",
+               "\tAfter downloading all possible distfiles, run \"pkglint --autofix\",",
+               "\twhich will find the downloaded distfiles and add the missing hashes",
+               "\tto the distinfo file.",
+               "")
+
+       t.SetUpCommandLine("-Wall", "--autofix", "--show-autofix", "--source")
+
+       G.Check(t.File("category/package"))
+
+       // Since the distfile does not exist, pkglint cannot fix anything.
+       t.CheckOutputEmpty()
+}
+
 func (s *Suite) Test_distinfoLinesChecker_checkAlgorithmsDistfile__wrong_distfile_hash(c *check.C) {
        t := s.Init(c)
 
diff -r 381230051221 -r 30a70f57d8f7 pkgtools/pkglint/files/files.go
--- a/pkgtools/pkglint/files/files.go   Tue Apr 23 20:17:12 2019 +0000
+++ b/pkgtools/pkglint/files/files.go   Tue Apr 23 21:20:49 2019 +0000
@@ -2,6 +2,7 @@
 
 import (
        "io/ioutil"
+       "netbsd.org/pkglint/textproc"
        "path"
        "strings"
 )
@@ -74,6 +75,7 @@
        firstlineno := rawLines[index].Lineno
        var lineRawLines []*RawLine
        interestingRawLines := rawLines[index:]
+       trim := ""
 
        for i, rawLine := range interestingRawLines {
                indent, rawText, outdent, cont := matchContinuationLine(rawLine.textnl)
@@ -81,12 +83,14 @@
                if text.Len() == 0 {
                        text.WriteString(indent)
                }
-               text.WriteString(rawText)
+               text.WriteString(strings.TrimPrefix(rawText, trim))
+
                lineRawLines = append(lineRawLines, rawLine)
 
                if cont != "" && i != len(interestingRawLines)-1 {
                        text.WriteString(" ")
                        index++
+                       trim = textproc.NewLexer(rawText).NextString("#")
                } else {
                        text.WriteString(outdent)
                        text.WriteString(cont)
diff -r 381230051221 -r 30a70f57d8f7 pkgtools/pkglint/files/files_test.go
--- a/pkgtools/pkglint/files/files_test.go      Tue Apr 23 20:17:12 2019 +0000
+++ b/pkgtools/pkglint/files/files_test.go      Tue Apr 23 21:20:49 2019 +0000
@@ -98,7 +98,7 @@
        t.CheckOutputEmpty()
 }
 
-func (s *Suite) Test_convertToLogicalLines__commented_multi(c *check.C) {
+func (s *Suite) Test_nextLogicalLine__commented_multi(c *check.C) {
        t := s.Init(c)
 
        mklines := t.SetUpFileMkLines("filename.mk",
@@ -107,11 +107,9 @@
                "#\tcontinuation 2")
        mkline := mklines.mklines[0]
 
-       // FIXME: It is more pragmatic to strip the leading comments from the
-       //  continuation lines as well, so that the variable value is "continuation 1 continuation 2".
-       //  See nextLogicalLine.
-       t.Check(mkline.Value(), equals, "")



Home | Main Index | Thread Index | Old Index