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/44af4c564169
branches: trunk
changeset: 410647:44af4c564169
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 1c9f3c7b610b -r 44af4c564169 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 1c9f3c7b610b -r 44af4c564169 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 1c9f3c7b610b -r 44af4c564169 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 1c9f3c7b610b -r 44af4c564169 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 1c9f3c7b610b -r 44af4c564169 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