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



details:   https://anonhg.NetBSD.org/pkgsrc/rev/89b54b24ea8f
branches:  trunk
changeset: 410110:89b54b24ea8f
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Sun Jan 26 17:12:36 2020 +0000

description:
pkgtools/pkglint: update to 19.4.6

Changes since 19.4.5:

Added the --network option, which allows pkglint to use HTTP calls for
determining whether the homepage of a package is reachable.

Added migration from http URLs to the corresponding https URLs. This is
only done if the https URL is indeed reachable or well-known to support
https.

Added migration from https SourceForge URLs back to http URLs since a
previous pkglint run had migrated URLs to non-working https URLs. See
https://mail-index.netbsd.org/pkgsrc-changes/2020/01/18/msg205146.html.

Added a warning for HOMEPAGE that uses ftp:// since that is not
user-friendly. In the same way, download-only host names on SourceForge
are not suitable as a homepage since they usually only generate a 404.

diffstat:

 pkgtools/pkglint/Makefile                    |    4 +-
 pkgtools/pkglint/PLIST                       |    4 +-
 pkgtools/pkglint/files/getopt/getopt.go      |   20 +-
 pkgtools/pkglint/files/getopt/getopt_test.go |   44 ++
 pkgtools/pkglint/files/homepage.go           |  361 ++++++++++++++++++++++++
 pkgtools/pkglint/files/homepage_test.go      |  399 +++++++++++++++++++++++++++
 pkgtools/pkglint/files/logging.go            |   11 +-
 pkgtools/pkglint/files/mkassignchecker.go    |    2 +-
 pkgtools/pkglint/files/mkline.go             |   33 ++-
 pkgtools/pkglint/files/mklinechecker.go      |   30 +-
 pkgtools/pkglint/files/mklinechecker_test.go |   26 +-
 pkgtools/pkglint/files/mklines_test.go       |    2 +-
 pkgtools/pkglint/files/pkglint.go            |    6 +-
 pkgtools/pkglint/files/pkglint_test.go       |    3 +-
 pkgtools/pkglint/files/substcontext.go       |    2 +-
 pkgtools/pkglint/files/util.go               |   14 +-
 pkgtools/pkglint/files/vartypecheck.go       |  112 +-------
 pkgtools/pkglint/files/vartypecheck_test.go  |   98 +------
 18 files changed, 913 insertions(+), 258 deletions(-)

diffs (truncated from 1446 to 300 lines):

diff -r efd482a5c30c -r 89b54b24ea8f pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Sun Jan 26 14:11:02 2020 +0000
+++ b/pkgtools/pkglint/Makefile Sun Jan 26 17:12:36 2020 +0000
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.627 2020/01/23 21:56:50 rillig Exp $
+# $NetBSD: Makefile,v 1.628 2020/01/26 17:12:36 rillig Exp $
 
-PKGNAME=       pkglint-19.4.5
+PKGNAME=       pkglint-19.4.6
 CATEGORIES=    pkgtools
 DISTNAME=      tools
 MASTER_SITES=  ${MASTER_SITE_GITHUB:=golang/}
diff -r efd482a5c30c -r 89b54b24ea8f pkgtools/pkglint/PLIST
--- a/pkgtools/pkglint/PLIST    Sun Jan 26 14:11:02 2020 +0000
+++ b/pkgtools/pkglint/PLIST    Sun Jan 26 17:12:36 2020 +0000
@@ -1,4 +1,4 @@
-@comment $NetBSD: PLIST,v 1.25 2020/01/06 21:40:40 ryoon Exp $
+@comment $NetBSD: PLIST,v 1.26 2020/01/26 17:12:36 rillig Exp $
 bin/pkglint
 gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint.a
 gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint/getopt.a
@@ -29,6 +29,8 @@
 gopkg/src/netbsd.org/pkglint/getopt/getopt_test.go
 gopkg/src/netbsd.org/pkglint/histogram/histogram.go
 gopkg/src/netbsd.org/pkglint/histogram/histogram_test.go
+gopkg/src/netbsd.org/pkglint/homepage.go
+gopkg/src/netbsd.org/pkglint/homepage_test.go
 gopkg/src/netbsd.org/pkglint/intqa/qa.go
 gopkg/src/netbsd.org/pkglint/intqa/qa_test.go
 gopkg/src/netbsd.org/pkglint/licenses.go
diff -r efd482a5c30c -r 89b54b24ea8f pkgtools/pkglint/files/getopt/getopt.go
--- a/pkgtools/pkglint/files/getopt/getopt.go   Sun Jan 26 14:11:02 2020 +0000
+++ b/pkgtools/pkglint/files/getopt/getopt.go   Sun Jan 26 17:12:36 2020 +0000
@@ -106,7 +106,7 @@
                        skip, err = o.parseLongOption(args, i, arg[2:])
                        i += skip
 
-               case strings.HasPrefix(arg, "-"):
+               case strings.HasPrefix(arg, "-") && len(arg) > 1:
                        skip, err = o.parseShortOptions(args, i, arg[1:])
                        i += skip
 
@@ -282,13 +282,19 @@
        finishTable()
 
        for _, opt := range o.options {
-               if opt.argsName == "" {
-                       rowf("  -%c, --%s\t %s",
-                               opt.shortName, opt.longName, opt.description)
-               } else {
-                       rowf("  -%c, --%s=%s\t %s",
-                               opt.shortName, opt.longName, opt.argsName, opt.description)
+               name := ""
+               sep := ""
+               if opt.shortName != 0 {
+                       name = "-" + string(opt.shortName)
+                       sep = ", "
                }
+               if opt.longName != "" {
+                       name += sep + "--" + opt.longName
+                       if opt.argsName != "" {
+                               name += "=" + opt.argsName
+                       }
+               }
+               rowf("  %s\t %s", name, opt.description)
        }
        finishTable()
 
diff -r efd482a5c30c -r 89b54b24ea8f pkgtools/pkglint/files/getopt/getopt_test.go
--- a/pkgtools/pkglint/files/getopt/getopt_test.go      Sun Jan 26 14:11:02 2020 +0000
+++ b/pkgtools/pkglint/files/getopt/getopt_test.go      Sun Jan 26 17:12:36 2020 +0000
@@ -248,6 +248,33 @@
        c.Check(unfinished, check.Equals, "")
 }
 
+// From an implementation standpoint, it would be a likely bug to interpret
+// the "--" as the long name of the option, and that would set the flag
+// to true.
+func (s *Suite) Test_Options_Parse__only_short(c *check.C) {
+       var onlyShort bool
+       opts := NewOptions()
+       opts.AddFlagVar('s', "", &onlyShort, false, "only short")
+
+       args, err := opts.Parse([]string{"program", "--", "arg"})
+
+       c.Check(err, check.IsNil)
+       c.Check(args, check.DeepEquals, []string{"arg"})
+       c.Check(onlyShort, check.Equals, false)
+}
+
+func (s *Suite) Test_Options_Parse__only_long(c *check.C) {
+       var onlyLong bool
+       opts := NewOptions()
+       opts.AddFlagVar(0, "long", &onlyLong, false, "only long")
+
+       args, err := opts.Parse([]string{"program", "-", "arg"})
+
+       c.Check(err, check.IsNil)
+       c.Check(args, check.DeepEquals, []string{"-", "arg"})
+       c.Check(onlyLong, check.Equals, false)
+}
+
 func (s *Suite) Test_Options_handleLongOption__string(c *check.C) {
        var extra bool
 
@@ -409,6 +436,23 @@
                "  (Prefix a flag with \"no-\" to disable it.)\n")
 }
 
+func (s *Suite) Test_Options_Help__partial(c *check.C) {
+       var onlyShort, onlyLong bool
+
+       opts := NewOptions()
+       opts.AddFlagVar('s', "", &onlyShort, false, "Only short option")
+       opts.AddFlagVar(0, "long", &onlyLong, false, "Only long option")
+
+       var out strings.Builder
+       opts.Help(&out, "progname [options] args")
+
+       c.Check(out.String(), check.Equals, ""+
+               "usage: progname [options] args\n"+
+               "\n"+
+               "  -s       Only short option\n"+
+               "  --long   Only long option\n")
+}
+
 func (s *Suite) Test__qa(c *check.C) {
        ck := intqa.NewQAChecker(c.Errorf)
        ck.Configure("*", "*", "*", -intqa.EMissingTest)
diff -r efd482a5c30c -r 89b54b24ea8f pkgtools/pkglint/files/homepage.go
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/pkgtools/pkglint/files/homepage.go        Sun Jan 26 17:12:36 2020 +0000
@@ -0,0 +1,361 @@
+package pkglint
+
+import (
+       "net"
+       "net/http"
+       "syscall"
+       "time"
+)
+
+// HomepageChecker runs the checks for a HOMEPAGE definition.
+//
+// When pkglint is in network mode (which has to be enabled explicitly using
+// --network), it checks whether the homepage is actually reachable.
+//
+// The homepage URLs should use https as far as possible.
+// To achieve this goal, the HomepageChecker can migrate homepages
+// from less preferred URLs to preferred URLs.
+//
+// For most sites, the list of possible URLs is:
+//  - https://$rest (preferred)
+//  - http://$rest (less preferred)
+//
+// For SourceForge, it's a little more complicated:
+//  - https://$project.sourceforge.io/$path
+//  - http://$project.sourceforge.net/$path
+//  - http://$project.sourceforge.io/$path (not officially supported)
+//  - https://$project.sourceforge.net/$path (not officially supported)
+//  - https://sourceforge.net/projects/$project/
+//  - http://sourceforge.net/projects/$project/
+//  - https://sf.net/projects/$project/
+//  - http://sf.net/projects/$project/
+//  - https://sf.net/p/$project/
+//  - http://sf.net/p/$project/
+//
+// TODO: implement complete homepage migration for SourceForge.
+// TODO: allow to suppress the automatic migration for SourceForge,
+//  even if it is not about https vs. http.
+type HomepageChecker struct {
+       Value      string
+       ValueNoVar string
+       MkLine     *MkLine
+       MkLines    *MkLines
+}
+
+func NewHomepageChecker(value string, valueNoVar string, mkline *MkLine, mklines *MkLines) *HomepageChecker {
+       return &HomepageChecker{value, valueNoVar, mkline, mklines}
+}
+
+func (ck *HomepageChecker) Check() {
+       ck.checkBasedOnMasterSites()
+       ck.checkFtp()
+       ck.checkHttp()
+       ck.checkBadUrls()
+       ck.checkReachable()
+}
+
+func (ck *HomepageChecker) checkBasedOnMasterSites() {
+       m, wrong, sitename, subdir := match3(ck.Value, `^(\$\{(MASTER_SITE\w+)(?::=([\w\-/]+))?\})`)
+       if !m {
+               return
+       }
+
+       baseURL := G.Pkgsrc.MasterSiteVarToURL[sitename]
+       if sitename == "MASTER_SITES" && ck.MkLines.pkg != nil {
+               mkline := ck.MkLines.pkg.vars.FirstDefinition("MASTER_SITES")
+               if mkline != nil {
+                       if !containsVarUse(mkline.Value()) {
+                               masterSites := ck.MkLine.ValueFields(mkline.Value())
+                               if len(masterSites) > 0 {
+                                       baseURL = masterSites[0]
+                               }
+                       }
+               }
+       }
+
+       fixedURL := baseURL + subdir
+
+       fix := ck.MkLine.Autofix()
+       if baseURL != "" {
+               // TODO: Don't suggest any of checkBadUrls.
+               fix.Warnf("HOMEPAGE should not be defined in terms of MASTER_SITEs. Use %s directly.", fixedURL)
+       } else {
+               fix.Warnf("HOMEPAGE should not be defined in terms of MASTER_SITEs.")
+       }
+       fix.Explain(
+               "The HOMEPAGE is a single URL, while MASTER_SITES is a list of URLs.",
+               "As long as this list has exactly one element, this works, but as",
+               "soon as another site is added, the HOMEPAGE would not be a valid",
+               "URL anymore.",
+               "",
+               "Defining MASTER_SITES=${HOMEPAGE} is ok, though.")
+       if baseURL != "" {
+               fix.Replace(wrong, fixedURL)
+       }
+       fix.Apply()
+}
+
+func (ck *HomepageChecker) checkFtp() {
+       if !hasPrefix(ck.Value, "ftp://";) {
+               return
+       }
+
+       mkline := ck.MkLine
+       if mkline.HasRationale("ftp", "FTP", "http", "https", "HTTP") {
+               return
+       }
+
+       mkline.Warnf("An FTP URL does not represent a user-friendly homepage.")
+       mkline.Explain(
+               "This homepage URL has probably been generated by url2pkg",
+               "and not been reviewed by the package author.",
+               "",
+               "In most cases there exists a more welcoming URL,",
+               "which is usually served via HTTP.")
+}
+
+func (ck *HomepageChecker) checkHttp() {
+       if ck.MkLine.HasRationale("http", "https") {
+               return
+       }
+
+       shouldAutofix, from, to := ck.toHttps(ck.Value)
+       if from == "" {
+               return
+       }
+
+       fix := ck.MkLine.Autofix()
+       fix.Warnf("HOMEPAGE should migrate from %s to %s.", from, to)
+       if shouldAutofix {
+               fix.Replace(from, to)
+       }
+       fix.Explain(
+               "To provide secure communication by default,",
+               "the HOMEPAGE URL should use the https protocol if available.",
+               "",
+               "If the HOMEPAGE really does not support https,",
+               "add a comment near the HOMEPAGE variable stating this clearly.")
+       fix.Apply()
+}
+
+// toHttps checks whether the homepage should be migrated from http to https
+// and which part of the homepage URL needs to be modified for that.
+//
+// If for some reason the https URL should not be reachable but the
+// corresponding http URL is, the homepage is changed back to http.
+func (ck *HomepageChecker) toHttps(url string) (bool, string, string) {
+       m, scheme, host, port := match3(url, `(https?)://([A-Za-z0-9-.]+)(:[0-9]+)?`)
+       if !m {
+               return false, "", ""
+       }
+
+       if ck.hasAnySuffix(host,
+               "www.gnustep.org",           // 2020-01-18
+               "aspell.net",                // 2020-01-18
+               "downloads.sourceforge.net", // gets another warning already
+               ".dl.sourceforge.net",       // gets another warning already
+       ) {
+               return false, "", ""
+       }
+
+       if scheme == "http" && ck.hasAnySuffix(host,
+               "apache.org",
+               "archive.org",
+               "ctan.org",
+               "freedesktop.org",
+               "github.com",
+               "github.io",



Home | Main Index | Thread Index | Old Index