pkgsrc-Changes archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
CVS commit: pkgsrc/pkgtools/pkglint
Module Name: pkgsrc
Committed By: rillig
Date: Sun Jan 26 17:12:37 UTC 2020
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile PLIST
pkgsrc/pkgtools/pkglint/files: logging.go mkassignchecker.go mkline.go
mklinechecker.go mklinechecker_test.go mklines_test.go pkglint.go
pkglint_test.go substcontext.go util.go vartypecheck.go
vartypecheck_test.go
pkgsrc/pkgtools/pkglint/files/getopt: getopt.go getopt_test.go
Added Files:
pkgsrc/pkgtools/pkglint/files: homepage.go homepage_test.go
Log Message:
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.
To generate a diff of this commit:
cvs rdiff -u -r1.627 -r1.628 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.25 -r1.26 pkgsrc/pkgtools/pkglint/PLIST
cvs rdiff -u -r0 -r1.1 pkgsrc/pkgtools/pkglint/files/homepage.go \
pkgsrc/pkgtools/pkglint/files/homepage_test.go
cvs rdiff -u -r1.38 -r1.39 pkgsrc/pkgtools/pkglint/files/logging.go
cvs rdiff -u -r1.5 -r1.6 pkgsrc/pkgtools/pkglint/files/mkassignchecker.go
cvs rdiff -u -r1.74 -r1.75 pkgsrc/pkgtools/pkglint/files/mkline.go \
pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
cvs rdiff -u -r1.62 -r1.63 pkgsrc/pkgtools/pkglint/files/mklinechecker.go
cvs rdiff -u -r1.58 -r1.59 \
pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go \
pkgsrc/pkgtools/pkglint/files/pkglint_test.go
cvs rdiff -u -r1.60 -r1.61 pkgsrc/pkgtools/pkglint/files/mklines_test.go
cvs rdiff -u -r1.72 -r1.73 pkgsrc/pkgtools/pkglint/files/pkglint.go
cvs rdiff -u -r1.36 -r1.37 pkgsrc/pkgtools/pkglint/files/substcontext.go
cvs rdiff -u -r1.71 -r1.72 pkgsrc/pkgtools/pkglint/files/util.go
cvs rdiff -u -r1.80 -r1.81 pkgsrc/pkgtools/pkglint/files/vartypecheck.go
cvs rdiff -u -r1.8 -r1.9 pkgsrc/pkgtools/pkglint/files/getopt/getopt.go
cvs rdiff -u -r1.13 -r1.14 \
pkgsrc/pkgtools/pkglint/files/getopt/getopt_test.go
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: pkgsrc/pkgtools/pkglint/Makefile
diff -u pkgsrc/pkgtools/pkglint/Makefile:1.627 pkgsrc/pkgtools/pkglint/Makefile:1.628
--- pkgsrc/pkgtools/pkglint/Makefile:1.627 Thu Jan 23 21:56:50 2020
+++ pkgsrc/pkgtools/pkglint/Makefile Sun Jan 26 17:12:36 2020
@@ -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/}
Index: pkgsrc/pkgtools/pkglint/PLIST
diff -u pkgsrc/pkgtools/pkglint/PLIST:1.25 pkgsrc/pkgtools/pkglint/PLIST:1.26
--- pkgsrc/pkgtools/pkglint/PLIST:1.25 Mon Jan 6 21:40:40 2020
+++ pkgsrc/pkgtools/pkglint/PLIST Sun Jan 26 17:12:36 2020
@@ -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/geto
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
Index: pkgsrc/pkgtools/pkglint/files/logging.go
diff -u pkgsrc/pkgtools/pkglint/files/logging.go:1.38 pkgsrc/pkgtools/pkglint/files/logging.go:1.39
--- pkgsrc/pkgtools/pkglint/files/logging.go:1.38 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/logging.go Sun Jan 26 17:12:36 2020
@@ -48,6 +48,8 @@ type LoggerOpts struct {
ShowSource,
GccOutput,
Quiet bool
+
+ Only []string
}
type LogLevel struct {
@@ -107,7 +109,10 @@ func (l *Logger) Diag(line *Line, level
if G.Testing {
for _, arg := range args {
switch arg.(type) {
- case int, string, error:
+ case int, string:
+ case error:
+ // TODO: errors do not belong in diagnostics,
+ // they belong in normal error messages.
default:
// All paths in diagnostics must be relative to the line.
// To achieve that, call line.Rel(currPath).
@@ -174,11 +179,11 @@ func (l *Logger) Relevant(format string)
//
// It only inspects the --only arguments; duplicates are handled in Logger.Logf.
func (l *Logger) shallBeLogged(format string) bool {
- if len(G.Opts.LogOnly) == 0 {
+ if len(l.Opts.Only) == 0 {
return true
}
- for _, substr := range G.Opts.LogOnly {
+ for _, substr := range l.Opts.Only {
if contains(format, substr) {
return true
}
Index: pkgsrc/pkgtools/pkglint/files/mkassignchecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mkassignchecker.go:1.5 pkgsrc/pkgtools/pkglint/files/mkassignchecker.go:1.6
--- pkgsrc/pkgtools/pkglint/files/mkassignchecker.go:1.5 Sat Jan 18 21:56:09 2020
+++ pkgsrc/pkgtools/pkglint/files/mkassignchecker.go Sun Jan 26 17:12:36 2020
@@ -284,7 +284,7 @@ func (ck *MkAssignChecker) checkVarassig
return
}
- if mkline.Rationale() != "" {
+ if mkline.HasRationale() {
return
}
Index: pkgsrc/pkgtools/pkglint/files/mkline.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline.go:1.74 pkgsrc/pkgtools/pkglint/files/mkline.go:1.75
--- pkgsrc/pkgtools/pkglint/files/mkline.go:1.74 Sat Jan 18 21:56:09 2020
+++ pkgsrc/pkgtools/pkglint/files/mkline.go Sun Jan 26 17:12:36 2020
@@ -4,6 +4,7 @@ import (
"fmt"
"netbsd.org/pkglint/regex"
"netbsd.org/pkglint/textproc"
+ "regexp"
"strings"
)
@@ -74,12 +75,40 @@ func (mkline *MkLine) String() string {
func (mkline *MkLine) HasComment() bool { return mkline.splitResult.hasComment }
-// Rationale returns the comments that are close enough to this line.
+// HasRationale returns true if the comments that are close enough to
+// this line contain a rationale for suppressing a diagnostic.
//
// These comments are used to suppress pkglint warnings,
// such as for BROKEN, NOT_FOR_PLATFORMS, MAKE_JOBS_SAFE,
// and HOMEPAGE using http instead of https.
-func (mkline *MkLine) Rationale() string { return mkline.splitResult.rationale }
+//
+// To qualify as a rationale, the comment must contain any of the given
+// keywords. If no keywords are given, any comment qualifies.
+func (mkline *MkLine) HasRationale(keywords ...string) bool {
+ rationale := mkline.splitResult.rationale
+ if rationale == "" {
+ return false
+ }
+ if len(keywords) == 0 {
+ return true
+ }
+
+ // Avoid expensive regular expression search.
+ rationaleContains := func(keyword string) bool {
+ return contains(rationale, keyword)
+ }
+ if !anyStr(keywords, rationaleContains) {
+ return false
+ }
+
+ for _, keyword := range keywords {
+ pattern := regex.Pattern(`\b` + regexp.QuoteMeta(keyword) + `\b`)
+ if matches(rationale, pattern) {
+ return true
+ }
+ }
+ return false
+}
// Comment returns the comment after the first unescaped #.
//
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.74 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.75
--- pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.74 Thu Jan 23 21:56:50 2020
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go Sun Jan 26 17:12:36 2020
@@ -1,8 +1,6 @@
package pkglint
-import (
- "gopkg.in/check.v1"
-)
+import "gopkg.in/check.v1"
func (s *Suite) Test_VartypeCheck_Errorf(c *check.C) {
t := s.Init(c)
@@ -144,7 +142,7 @@ func (s *Suite) Test_VartypeCheck_AwkCom
vt.Output(
"WARN: filename.mk:1: $0 is ambiguous. "+
"Use ${0} if you mean a Make variable or $$0 if you mean a shell variable.",
- "WARN: filename.mk:3: $0 is ambiguous. "+
+ "WARN: filename.mk:11: $0 is ambiguous. "+
"Use ${0} if you mean a Make variable or $$0 if you mean a shell variable.")
}
@@ -919,96 +917,10 @@ func (s *Suite) Test_VartypeCheck_Homepa
"${MASTER_SITES}")
vt.Output(
- "WARN: filename.mk:1: HOMEPAGE should use https instead of http.",
+ "WARN: filename.mk:1: HOMEPAGE should migrate from http to https.",
"WARN: filename.mk:3: HOMEPAGE should not be defined in terms of MASTER_SITEs.")
- pkg := NewPackage(t.File("category/package"))
- vt.Package(pkg)
-
- vt.Values(
- "${MASTER_SITES}")
-
- // When this assignment occurs while checking a package, but the package
- // doesn't define MASTER_SITES, that variable cannot be expanded, which means
- // the warning cannot refer to its value.
- vt.Output(
- "WARN: filename.mk:11: HOMEPAGE should not be defined in terms of MASTER_SITEs.")
-
- delete(pkg.vars.firstDef, "MASTER_SITES")
- delete(pkg.vars.lastDef, "MASTER_SITES")
- pkg.vars.Define("MASTER_SITES", t.NewMkLine(pkg.File("Makefile"), 5,
- "MASTER_SITES=\thttps://cdn.NetBSD.org/pub/pkgsrc/distfiles/";))
-
- vt.Values(
- "${MASTER_SITES}")
-
- vt.Output(
- "WARN: filename.mk:21: HOMEPAGE should not be defined in terms of MASTER_SITEs. " +
- "Use https://cdn.NetBSD.org/pub/pkgsrc/distfiles/ directly.")
-
- delete(pkg.vars.firstDef, "MASTER_SITES")
- delete(pkg.vars.lastDef, "MASTER_SITES")
- pkg.vars.Define("MASTER_SITES", t.NewMkLine(pkg.File("Makefile"), 5,
- "MASTER_SITES=\t${MASTER_SITE_GITHUB}"))
-
- vt.Values(
- "${MASTER_SITES}")
-
- // When MASTER_SITES itself makes use of another variable, pkglint doesn't
- // resolve that variable and just outputs the simple variant of this warning.
- vt.Output(
- "WARN: filename.mk:31: HOMEPAGE should not be defined in terms of MASTER_SITEs.")
-
- delete(pkg.vars.firstDef, "MASTER_SITES")
- delete(pkg.vars.lastDef, "MASTER_SITES")
- pkg.vars.Define("MASTER_SITES", t.NewMkLine(pkg.File("Makefile"), 5,
- "MASTER_SITES=\t# none"))
-
- vt.Values(
- "${MASTER_SITES}")
-
- // When MASTER_SITES is empty, pkglint cannot extract the first of the URLs
- // for using it in the HOMEPAGE.
- vt.Output(
- "WARN: filename.mk:41: HOMEPAGE should not be defined in terms of MASTER_SITEs.")
-}
-
-func (s *Suite) Test_VartypeCheck_Homepage__http(c *check.C) {
- t := s.Init(c)
- vt := NewVartypeCheckTester(t, BtHomepage)
-
- vt.Varname("HOMEPAGE")
- vt.Values(
- "http://www.gnustep.org/";,
- "http://www.pkgsrc.org/";,
- "http://project.sourceforge.net/";,
- "http://sf.net/p/project/";,
- "http://example.org/ # doesn't support https",
- "http://example.org/ # only supports http",
- "http://asf.net/";)
-
- vt.Output(
- "WARN: filename.mk:2: HOMEPAGE should use https instead of http.",
- "WARN: filename.mk:3: HOMEPAGE should use https instead of http.",
- "WARN: filename.mk:4: HOMEPAGE should use https instead of http.",
- "WARN: filename.mk:7: HOMEPAGE should use https instead of http.")
-
- t.SetUpCommandLine("--autofix")
- vt.Values(
- "http://www.gnustep.org/";,
- "http://www.pkgsrc.org/";,
- "http://project.sourceforge.net/";,
- "http://sf.net/p/project/";,
- "http://example.org/ # doesn't support https",
- "http://example.org/ # only supports http",
- "http://asf.net/";)
-
- // www.gnustep.org does not support https at all.
- // www.pkgsrc.org is not in the (short) list of known https domains,
- // therefore pkglint does not dare to change it automatically.
- vt.Output(
- "AUTOFIX: filename.mk:13: Replacing \"http://project.sourceforge.net\"; with \"https://project.sourceforge.io\".";,
- "AUTOFIX: filename.mk:14: Replacing \"http\" with \"https\".")
+ // For more tests, see HomepageChecker.
}
func (s *Suite) Test_VartypeCheck_IdentifierDirect(c *check.C) {
@@ -2380,6 +2292,8 @@ func (vt *VartypeCheckTester) Values(val
mklines.ForEach(func(mkline *MkLine) { test(mklines, mkline, value) })
}
+
+ vt.nextSection()
}
// Output checks that the output from all previous steps is
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.62 pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.63
--- pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.62 Sat Jan 18 21:56:09 2020
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker.go Sun Jan 26 17:12:36 2020
@@ -268,16 +268,30 @@ func (ck MkLineChecker) checkInclude() {
case includedFile.HasSuffixPath("intltool/buildlink3.mk"):
mkline.Warnf("Please write \"USE_TOOLS+= intltool\" instead of this line.")
+ }
- case includedFile != "builtin.mk" && includedFile.HasSuffixPath("builtin.mk"):
- if mkline.Basename != "hacks.mk" && mkline.Rationale() == "" {
- fix := mkline.Autofix()
- fix.Errorf("%q must not be included directly. Include %q instead.",
- includedFile, includedFile.DirNoClean().JoinNoClean("buildlink3.mk"))
- fix.Replace("builtin.mk", "buildlink3.mk")
- fix.Apply()
- }
+ ck.checkIncludeBuiltin()
+}
+
+func (ck MkLineChecker) checkIncludeBuiltin() {
+ mkline := ck.MkLine
+
+ includedFile := mkline.IncludedFile()
+ switch {
+ case includedFile == "builtin.mk",
+ !includedFile.HasSuffixPath("builtin.mk"),
+ mkline.Basename == "hacks.mk",
+ mkline.HasRationale("builtin", "include", "included", "including"):
+ return
}
+
+ includeInstead := includedFile.DirNoClean().JoinNoClean("buildlink3.mk")
+
+ fix := mkline.Autofix()
+ fix.Errorf("%q must not be included directly. Include %q instead.",
+ includedFile, includeInstead)
+ fix.Replace("builtin.mk", "buildlink3.mk")
+ fix.Apply()
}
func (ck MkLineChecker) checkDirectiveIndentation(expectedDepth int) {
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.58 pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.59
--- pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.58 Sat Jan 18 21:56:09 2020
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go Sun Jan 26 17:12:36 2020
@@ -433,25 +433,8 @@ func (s *Suite) Test_MkLineChecker_check
"Relative path \"../../category/package/nonexistent.mk\" does not exist.")
}
-func (s *Suite) Test_MkLineChecker_checkInclude__builtin_mk(c *check.C) {
- t := s.Init(c)
-
- t.SetUpPackage("category/package",
- ".include \"../../category/package/builtin.mk\"",
- ".include \"../../category/package/builtin.mk\" # ok")
- t.CreateFileLines("category/package/builtin.mk",
- MkCvsID)
- t.FinishSetUp()
-
- G.checkdirPackage(t.File("category/package"))
-
- t.CheckOutputLines(
- "ERROR: ~/category/package/Makefile:20: " +
- "\"../../category/package/builtin.mk\" must not be included directly. " +
- "Include \"../../category/package/buildlink3.mk\" instead.")
-}
-
-func (s *Suite) Test_MkLineChecker_checkInclude__buildlink3_mk_includes_builtin_mk(c *check.C) {
+// A buildlink3.mk file may include its corresponding builtin.mk file directly.
+func (s *Suite) Test_MkLineChecker_checkIncludeBuiltin__buildlink3_mk(c *check.C) {
t := s.Init(c)
t.SetUpPkgsrc()
@@ -467,14 +450,15 @@ func (s *Suite) Test_MkLineChecker_check
t.CheckOutputEmpty()
}
-func (s *Suite) Test_MkLineChecker_checkInclude__builtin_mk_rationale(c *check.C) {
+func (s *Suite) Test_MkLineChecker_checkIncludeBuiltin__rationale(c *check.C) {
t := s.Init(c)
t.SetUpPackage("category/package",
"# I have good reasons for including this file directly.",
".include \"../../category/package/builtin.mk\"",
"",
- ".include \"../../category/package/builtin.mk\"")
+ ".include \"../../category/package/builtin.mk\"",
+ ".include \"../../category/package/builtin.mk\" # intentionally included directly")
t.CreateFileLines("category/package/builtin.mk",
MkCvsID)
t.FinishSetUp()
Index: pkgsrc/pkgtools/pkglint/files/pkglint_test.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.58 pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.59
--- pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.58 Sat Jan 11 15:47:58 2020
+++ pkgsrc/pkgtools/pkglint/files/pkglint_test.go Sun Jan 26 17:12:36 2020
@@ -58,6 +58,7 @@ func (s *Suite) Test_Pkglint_Main__help(
" -h, --help show a detailed usage message",
" -I, --dumpmakefile dump the Makefile after parsing",
" -i, --import prepare the import of a wip package",
+ " -n, --network enable checks that need network access",
" -o, --only only log diagnostics containing the given text",
" -p, --profiling profile the executing program",
" -q, --quiet don't show a summary line when finishing",
@@ -356,7 +357,7 @@ func (s *Suite) Test_Pkglint_ParseComman
if exitcode != -1 {
t.CheckEquals(exitcode, 0)
}
- t.CheckDeepEquals(G.Opts.LogOnly, []string{":Q"})
+ t.CheckDeepEquals(G.Logger.Opts.Only, []string{":Q"})
t.CheckOutputLines(
confVersion)
}
Index: pkgsrc/pkgtools/pkglint/files/mklines_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.60 pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.61
--- pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.60 Sat Jan 18 21:56:09 2020
+++ pkgsrc/pkgtools/pkglint/files/mklines_test.go Sun Jan 26 17:12:36 2020
@@ -423,7 +423,7 @@ func (s *Suite) Test_MkLines_collectRati
mklines.collectRationale()
var actual []string
mklines.ForEach(func(mkline *MkLine) {
- actual = append(actual, condStr(mkline.Rationale() != "", "R ", "- ")+mkline.Text)
+ actual = append(actual, condStr(mkline.HasRationale(), "R ", "- ")+mkline.Text)
})
t.CheckDeepEquals(actual, specs)
}
Index: pkgsrc/pkgtools/pkglint/files/pkglint.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint.go:1.72 pkgsrc/pkgtools/pkglint/files/pkglint.go:1.73
--- pkgsrc/pkgtools/pkglint/files/pkglint.go:1.72 Sat Jan 11 15:47:58 2020
+++ pkgsrc/pkgtools/pkglint/files/pkglint.go Sun Jan 26 17:12:36 2020
@@ -78,11 +78,10 @@ type CmdOpts struct {
ShowHelp,
DumpMakefile,
Import,
+ Network,
Recursive,
ShowVersion bool
- LogOnly []string
-
args []string
}
@@ -235,7 +234,8 @@ func (pkglint *Pkglint) ParseCommandLine
opts.AddFlagVar('h', "help", &gopts.ShowHelp, false, "show a detailed usage message")
opts.AddFlagVar('I', "dumpmakefile", &gopts.DumpMakefile, false, "dump the Makefile after parsing")
opts.AddFlagVar('i', "import", &gopts.Import, false, "prepare the import of a wip package")
- opts.AddStrList('o', "only", &gopts.LogOnly, "only log diagnostics containing the given text")
+ opts.AddFlagVar('n', "network", &gopts.Network, false, "enable checks that need network access")
+ opts.AddStrList('o', "only", &lopts.Only, "only log diagnostics containing the given text")
opts.AddFlagVar('p', "profiling", &gopts.Profiling, false, "profile the executing program")
opts.AddFlagVar('q', "quiet", &lopts.Quiet, false, "don't show a summary line when finishing")
opts.AddFlagVar('r', "recursive", &gopts.Recursive, false, "check subdirectories, too")
Index: pkgsrc/pkgtools/pkglint/files/substcontext.go
diff -u pkgsrc/pkgtools/pkglint/files/substcontext.go:1.36 pkgsrc/pkgtools/pkglint/files/substcontext.go:1.37
--- pkgsrc/pkgtools/pkglint/files/substcontext.go:1.36 Sat Jan 4 19:53:14 2020
+++ pkgsrc/pkgtools/pkglint/files/substcontext.go Sun Jan 26 17:12:36 2020
@@ -197,7 +197,7 @@ func (ctx *SubstContext) activate(mkline
return true
}
- if ctx.once.FirstTime(id) && !containsWord(mkline.Rationale(), id) {
+ if ctx.once.FirstTime(id) && !mkline.HasRationale(id) {
mkline.Warnf("Before defining %s, the SUBST class "+
"should be declared using \"SUBST_CLASSES+= %s\".",
mkline.Varname(), id)
Index: pkgsrc/pkgtools/pkglint/files/util.go
diff -u pkgsrc/pkgtools/pkglint/files/util.go:1.71 pkgsrc/pkgtools/pkglint/files/util.go:1.72
--- pkgsrc/pkgtools/pkglint/files/util.go:1.71 Sat Jan 11 15:47:58 2020
+++ pkgsrc/pkgtools/pkglint/files/util.go Sun Jan 26 17:12:36 2020
@@ -71,11 +71,6 @@ func replaceAllFunc(s string, re regex.P
return G.res.Compile(re).ReplaceAllStringFunc(s, repl)
}
-func containsWord(s, word string) bool {
- return strings.Contains(s, word) &&
- matches(s, regex.Pattern(`\b`+regexp.QuoteMeta(word)+`\b`))
-}
-
func containsStr(slice []string, s string) bool {
for _, str := range slice {
if s == str {
@@ -93,6 +88,15 @@ func mapStr(slice []string, fn func(s st
return result
}
+func anyStr(slice []string, fn func(s string) bool) bool {
+ for _, str := range slice {
+ if fn(str) {
+ return true
+ }
+ }
+ return false
+}
+
func filterStr(slice []string, fn func(s string) bool) []string {
result := make([]string, 0, len(slice))
for _, str := range slice {
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.80 pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.81
--- pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.80 Thu Jan 23 21:56:50 2020
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck.go Sun Jan 26 17:12:36 2020
@@ -632,117 +632,9 @@ func (cv *VartypeCheck) GccReqd() {
func (cv *VartypeCheck) Homepage() {
cv.URL()
- cv.homepageBasedOnMasterSites()
- cv.homepageHttp()
-}
-
-func (cv *VartypeCheck) homepageBasedOnMasterSites() {
- m, wrong, sitename, subdir := match3(cv.Value, `^(\$\{(MASTER_SITE\w+)(?::=([\w\-/]+))?\})`)
- if !m {
- return
- }
-
- baseURL := G.Pkgsrc.MasterSiteVarToURL[sitename]
- if sitename == "MASTER_SITES" && cv.MkLines.pkg != nil {
- mkline := cv.MkLines.pkg.vars.FirstDefinition("MASTER_SITES")
- if mkline != nil {
- if !containsVarUse(mkline.Value()) {
- masterSites := cv.MkLine.ValueFields(mkline.Value())
- if len(masterSites) > 0 {
- baseURL = masterSites[0]
- }
- }
- }
- }
-
- fixedURL := baseURL + subdir
-
- fix := cv.Autofix()
- if baseURL != "" {
- 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 (cv *VartypeCheck) homepageHttp() {
- m, host := match1(cv.Value, `http://([A-Za-z0-9-.]+)`)
- if !m {
- return
- }
-
- rationale := cv.MkLine.Rationale()
- if containsWord(rationale, "http") || containsWord(rationale, "https") {
- return
- }
-
- hasAnySuffix := func(s string, suffixes ...string) bool {
- for _, suffix := range suffixes {
- if hasSuffix(s, suffix) {
- dotIndex := len(s) - len(suffix)
- if dotIndex == 0 || s[dotIndex-1] == '.' {
- return true
- }
- }
- }
- return false
- }
-
- // Don't warn about sites that don't support https at all.
- if hasAnySuffix(host,
- "www.gnustep.org", // 2020-01-18
- "aspell.net", // 2020-01-18
- ) {
- return
- }
-
- supportsHttps := hasAnySuffix(host,
- "apache.org",
- "archive.org",
- "ctan.org",
- "freedesktop.org",
- "github.com",
- "github.io",
- "gnome.org",
- "gnu.org",
- "kde.org",
- "kldp.net",
- "linuxfoundation.org",
- "NetBSD.org",
- "nongnu.org",
- "sf.net",
- "sourceforge.net",
- "tryton.org",
- "tug.org")
-
- fix := cv.Autofix()
- fix.Warnf("HOMEPAGE should use https instead of http.")
- if supportsHttps {
- if hasAnySuffix(host, "sourceforge.net") {
- // See https://sourceforge.net/p/forge/documentation/Custom%20VHOSTs/
- fix.Replace("http://"+host, "https://"+replaceAll(host, `\.net`, ".io"))
- } else {
- fix.Replace("http", "https")
- }
- }
- 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()
+ ck := NewHomepageChecker(cv.Value, cv.ValueNoVar, cv.MkLine, cv.MkLines)
+ ck.Check()
}
// Identifier checks for valid identifiers in various contexts, limiting the
Index: pkgsrc/pkgtools/pkglint/files/getopt/getopt.go
diff -u pkgsrc/pkgtools/pkglint/files/getopt/getopt.go:1.8 pkgsrc/pkgtools/pkglint/files/getopt/getopt.go:1.9
--- pkgsrc/pkgtools/pkglint/files/getopt/getopt.go:1.8 Thu Feb 21 22:49:04 2019
+++ pkgsrc/pkgtools/pkglint/files/getopt/getopt.go Sun Jan 26 17:12:37 2020
@@ -106,7 +106,7 @@ func (o *Options) Parse(args []string) (
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 @@ func (o *Options) Help(out io.Writer, ge
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()
Index: pkgsrc/pkgtools/pkglint/files/getopt/getopt_test.go
diff -u pkgsrc/pkgtools/pkglint/files/getopt/getopt_test.go:1.13 pkgsrc/pkgtools/pkglint/files/getopt/getopt_test.go:1.14
--- pkgsrc/pkgtools/pkglint/files/getopt/getopt_test.go:1.13 Wed Nov 27 22:10:07 2019
+++ pkgsrc/pkgtools/pkglint/files/getopt/getopt_test.go Sun Jan 26 17:12:37 2020
@@ -248,6 +248,33 @@ func (s *Suite) Test_Options_Parse__long
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 @@ func (s *Suite) Test_Options_Help__with_
" (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)
Added files:
Index: pkgsrc/pkgtools/pkglint/files/homepage.go
diff -u /dev/null pkgsrc/pkgtools/pkglint/files/homepage.go:1.1
--- /dev/null Sun Jan 26 17:12:37 2020
+++ pkgsrc/pkgtools/pkglint/files/homepage.go Sun Jan 26 17:12:36 2020
@@ -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",
+ "gnome.org",
+ "gnu.org",
+ "kde.org",
+ "kldp.net",
+ "linuxfoundation.org",
+ "NetBSD.org",
+ "nongnu.org",
+ "tryton.org",
+ "tug.org") {
+ return port == "", "http", "https"
+ }
+
+ if scheme == "http" && host == "sf.net" {
+ return port == "", "http://sf.net";, "https://sourceforge.net";
+ }
+
+ from := scheme
+ to := "https"
+ toReachable := unknown
+
+ // SourceForge projects use either http://project.sourceforge.net or
+ // https://project.sourceforge.io (not net).
+ if m, project := match1(host, `^([\w-]+)\.(?:sf|sourceforge)\.net$`); m {
+ if scheme == "http" {
+ from = scheme + "://" + host
+ // See https://sourceforge.net/p/forge/documentation/Custom%20VHOSTs
+ to = "https://"; + project + ".sourceforge.io"
+ } else {
+ from = "sourceforge.net"
+ to = "sourceforge.io"
+
+ // Roll back wrong https SourceForge homepages generated by:
+ // https://mail-index.netbsd.org/pkgsrc-changes/2020/01/18/msg205146.html
+ if port == "" && G.Opts.Network {
+ _, migrated := replaceOnce(url, from, to)
+ if ck.isReachable(migrated) == no {
+ ok, httpOnly := replaceOnce(url, "https://";, "http://";)
+ if ok && ck.isReachable(httpOnly) == yes && ck.isReachable(url) == no {
+ from = "https"
+ to = "http"
+ toReachable = yes
+ }
+ }
+ }
+ }
+ }
+
+ if from == to {
+ return false, "", ""
+ }
+
+ shouldAutofix := toReachable == yes
+ if port == "" && G.Opts.Network && toReachable == unknown {
+ _, migrated := replaceOnce(url, from, to)
+ shouldAutofix = ck.isReachable(migrated) == yes
+ }
+ return shouldAutofix, from, to
+}
+
+func (ck *HomepageChecker) checkBadUrls() {
+ m, host := match1(ck.Value, `https?://([A-Za-z0-9-.]+)`)
+ if !m {
+ return
+ }
+
+ if !ck.hasAnySuffix(host,
+ ".dl.sourceforge.net",
+ "downloads.sourceforge.net") {
+ return
+ }
+
+ mkline := ck.MkLine
+ mkline.Warnf("A direct download URL is not 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.")
+}
+
+func (ck *HomepageChecker) checkReachable() {
+ mkline := ck.MkLine
+ url := ck.Value
+
+ if !G.Opts.Network || url != ck.ValueNoVar {
+ return
+ }
+ if !matches(url, `^https?://[A-Za-z0-9-.]+(?::[0-9]+)?/[!-~]*$`) {
+ return
+ }
+
+ var client http.Client
+ client.Timeout = 3 * time.Second
+ client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
+ return http.ErrUseLastResponse
+ }
+
+ request, err := http.NewRequest("GET", url, nil)
+ if err != nil {
+ mkline.Errorf("Invalid URL %q.", url)
+ return
+ }
+
+ response, err := client.Do(request)
+ if err != nil {
+ networkError := ck.classifyNetworkError(err)
+ mkline.Warnf("Homepage %q cannot be checked: %s", url, networkError)
+ return
+ }
+ defer func() { _ = response.Body.Close() }()
+
+ location, err := response.Location()
+ if err == nil {
+ mkline.Warnf("Homepage %q redirects to %q.", url, location.String())
+ return
+ }
+
+ if response.StatusCode != 200 {
+ mkline.Warnf("Homepage %q returns HTTP status %q.", url, response.Status)
+ return
+ }
+}
+
+func (*HomepageChecker) isReachable(url string) YesNoUnknown {
+ switch {
+ case !G.Opts.Network,
+ containsVarRefLong(url),
+ !matches(url, `^https?://[A-Za-z0-9-.]+(?::[0-9]+)?/[!-~]*$`):
+ return unknown
+ }
+
+ var client http.Client
+ client.Timeout = 3 * time.Second
+ client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
+ return http.ErrUseLastResponse
+ }
+
+ request, err := http.NewRequest("GET", url, nil)
+ if err != nil {
+ return no
+ }
+ response, err := client.Do(request)
+ if err != nil {
+ return no
+ }
+ _ = response.Body.Close()
+ if response.StatusCode != 200 {
+ return no
+ }
+ return yes
+}
+
+func (*HomepageChecker) hasAnySuffix(s string, suffixes ...string) bool {
+ for _, suffix := range suffixes {
+ if hasSuffix(s, suffix) {
+ dotIndex := len(s) - len(suffix)
+ if dotIndex == 0 || s[dotIndex-1] == '.' || suffix[0] == '.' {
+ return true
+ }
+ }
+ }
+ return false
+}
+
+func (*HomepageChecker) classifyNetworkError(err error) string {
+ cause := err
+ for {
+ // Unwrap was added in Go 1.13.
+ // See https://github.com/golang/go/issues/36781
+ if unwrap, ok := cause.(interface{ Unwrap() error }); ok {
+ cause = unwrap.Unwrap()
+ continue
+ }
+ break
+ }
+
+ // DNSError.IsNotFound was added in Go 1.13.
+ // See https://github.com/golang/go/issues/28635
+ if cause, ok := cause.(*net.DNSError); ok && cause.Err == "no such host" {
+ return "name not found"
+ }
+
+ if cause, ok := cause.(syscall.Errno); ok {
+ if cause == 10061 || cause == syscall.ECONNREFUSED {
+ return "connection refused"
+ }
+ }
+
+ if cause, ok := cause.(net.Error); ok && cause.Timeout() {
+ return "timeout"
+ }
+
+ return sprintf("unknown network error: %s", err)
+}
Index: pkgsrc/pkgtools/pkglint/files/homepage_test.go
diff -u /dev/null pkgsrc/pkgtools/pkglint/files/homepage_test.go:1.1
--- /dev/null Sun Jan 26 17:12:37 2020
+++ pkgsrc/pkgtools/pkglint/files/homepage_test.go Sun Jan 26 17:12:36 2020
@@ -0,0 +1,399 @@
+package pkglint
+
+import (
+ "context"
+ "errors"
+ "gopkg.in/check.v1"
+ "net"
+ "net/http"
+ "strconv"
+ "syscall"
+ "time"
+)
+
+func (s *Suite) Test_NewHomepageChecker(c *check.C) {
+ t := s.Init(c)
+
+ mklines := t.NewMkLines("filename.mk",
+ "HOMEPAGE=\t# none")
+ mkline := mklines.mklines[0]
+
+ ck := NewHomepageChecker("value", "valueNoVar", mkline, mklines)
+
+ t.CheckEquals(ck.Value, "value")
+ t.CheckEquals(ck.ValueNoVar, "valueNoVar")
+}
+
+func (s *Suite) Test_HomepageChecker_Check(c *check.C) {
+ t := s.Init(c)
+
+ mklines := t.NewMkLines("filename.mk",
+ "HOMEPAGE=\tftp://example.org/";)
+ mkline := mklines.mklines[0]
+ value := mkline.Value()
+
+ ck := NewHomepageChecker(value, value, mkline, mklines)
+
+ ck.Check()
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:1: An FTP URL does not represent a user-friendly homepage.")
+}
+
+func (s *Suite) Test_HomepageChecker_checkBasedOnMasterSites(c *check.C) {
+ t := s.Init(c)
+ vt := NewVartypeCheckTester(t, BtHomepage)
+
+ vt.Varname("HOMEPAGE")
+ vt.Values(
+ "${MASTER_SITES}")
+
+ vt.Output(
+ "WARN: filename.mk:1: HOMEPAGE should not be defined in terms of MASTER_SITEs.")
+
+ pkg := NewPackage(t.File("category/package"))
+ vt.Package(pkg)
+
+ vt.Values(
+ "${MASTER_SITES}")
+
+ // When this assignment occurs while checking a package, but the package
+ // doesn't define MASTER_SITES, that variable cannot be expanded, which means
+ // the warning cannot suggest a replacement value.
+ vt.Output(
+ "WARN: filename.mk:11: HOMEPAGE should not be defined in terms of MASTER_SITEs.")
+
+ delete(pkg.vars.firstDef, "MASTER_SITES")
+ delete(pkg.vars.lastDef, "MASTER_SITES")
+ pkg.vars.Define("MASTER_SITES", t.NewMkLine(pkg.File("Makefile"), 5,
+ "MASTER_SITES=\thttps://cdn.NetBSD.org/pub/pkgsrc/distfiles/";))
+
+ vt.Values(
+ "${MASTER_SITES}")
+
+ vt.Output(
+ "WARN: filename.mk:21: HOMEPAGE should not be defined in terms of MASTER_SITEs. " +
+ "Use https://cdn.NetBSD.org/pub/pkgsrc/distfiles/ directly.")
+
+ delete(pkg.vars.firstDef, "MASTER_SITES")
+ delete(pkg.vars.lastDef, "MASTER_SITES")
+ pkg.vars.Define("MASTER_SITES", t.NewMkLine(pkg.File("Makefile"), 5,
+ "MASTER_SITES=\t${MASTER_SITE_GITHUB}"))
+
+ vt.Values(
+ "${MASTER_SITES}")
+
+ // When MASTER_SITES itself makes use of another variable, pkglint doesn't
+ // resolve that variable and just outputs the simple variant of this warning.
+ vt.Output(
+ "WARN: filename.mk:31: HOMEPAGE should not be defined in terms of MASTER_SITEs.")
+
+ delete(pkg.vars.firstDef, "MASTER_SITES")
+ delete(pkg.vars.lastDef, "MASTER_SITES")
+ pkg.vars.Define("MASTER_SITES", t.NewMkLine(pkg.File("Makefile"), 5,
+ "MASTER_SITES=\t# none"))
+
+ vt.Values(
+ "${MASTER_SITES}")
+
+ // When MASTER_SITES is empty, pkglint cannot extract the first of the URLs
+ // for using it in the HOMEPAGE.
+ vt.Output(
+ "WARN: filename.mk:41: HOMEPAGE should not be defined in terms of MASTER_SITEs.")
+}
+
+func (s *Suite) Test_HomepageChecker_checkFtp(c *check.C) {
+ t := s.Init(c)
+ vt := NewVartypeCheckTester(t, BtHomepage)
+
+ vt.Varname("HOMEPAGE")
+ vt.Values(
+ "ftp://example.org/";,
+ "ftp://example.org/ # no HTTP homepage available")
+
+ vt.Output(
+ "WARN: filename.mk:1: " +
+ "An FTP URL does not represent a user-friendly homepage.")
+}
+
+func (s *Suite) Test_HomepageChecker_checkHttp(c *check.C) {
+ t := s.Init(c)
+ vt := NewVartypeCheckTester(t, BtHomepage)
+
+ vt.Varname("HOMEPAGE")
+ vt.Values(
+ "http://www.gnustep.org/";,
+ "http://www.pkgsrc.org/";,
+ "http://project.sourceforge.net/";,
+ "http://sf.net/p/project/";,
+ "http://sourceforge.net/p/project/";,
+ "http://example.org/ # doesn't support https",
+ "http://example.org/ # only supports http",
+ "http://asf.net/";)
+
+ vt.Output(
+ "WARN: filename.mk:2: HOMEPAGE should migrate from http to https.",
+ "WARN: filename.mk:3: HOMEPAGE should migrate "+
+ "from http://project.sourceforge.net "+
+ "to https://project.sourceforge.io.";,
+ "WARN: filename.mk:4: HOMEPAGE should migrate "+
+ "from http://sf.net to https://sourceforge.net.";,
+ "WARN: filename.mk:5: HOMEPAGE should migrate from http to https.",
+ "WARN: filename.mk:8: HOMEPAGE should migrate from http to https.")
+
+ t.SetUpCommandLine("--autofix")
+ vt.Values(
+ "http://www.gnustep.org/";,
+ "http://www.pkgsrc.org/";,
+ "http://project.sourceforge.net/";,
+ "http://sf.net/p/project/";,
+ "http://sourceforge.net/p/project/";,
+ "http://example.org/ # doesn't support https",
+ "http://example.org/ # only supports http",
+ "http://kde.org/";,
+ "http://asf.net/";)
+
+ // www.gnustep.org does not support https at all.
+ // www.pkgsrc.org is not in the (short) list of known https domains,
+ // therefore pkglint does not dare to change it automatically.
+ vt.Output(
+ "AUTOFIX: filename.mk:14: Replacing \"http://sf.net\"; "+
+ "with \"https://sourceforge.net\".";,
+ "AUTOFIX: filename.mk:18: Replacing \"http\" with \"https\".")
+}
+
+func (s *Suite) Test_HomepageChecker_toHttps(c *check.C) {
+ t := s.Init(c)
+
+ test := func(url string, shouldAutofix bool, from, to string) {
+ toHttps := (*HomepageChecker).toHttps
+ actualShouldAutofix, actualFrom, actualTo := toHttps(nil, url)
+ t.CheckDeepEquals(
+ []interface{}{actualShouldAutofix, actualFrom, actualTo},
+ []interface{}{shouldAutofix, from, to})
+ }
+
+ test("http://localhost/";, false, "http", "https")
+
+ test(
+ "http://project.sourceforge.net/";,
+ false,
+ "http://project.sourceforge.net";,
+ "https://project.sourceforge.io";)
+
+ // To clean up the wrong autofix from 2020-01-18:
+ // https://mail-index.netbsd.org/pkgsrc-changes/2020/01/18/msg205146.html
+ test(
+ "https://project.sourceforge.net/";,
+ false,
+ "sourceforge.net",
+ "sourceforge.io")
+
+ test(
+ "http://godoc.org/${GO_SRCPATH}";,
+ false,
+ "http",
+ "https")
+}
+
+func (s *Suite) Test_HomepageChecker_checkBadUrls(c *check.C) {
+ t := s.Init(c)
+ vt := NewVartypeCheckTester(t, BtHomepage)
+
+ vt.Varname("HOMEPAGE")
+ vt.Values(
+ "http://garr.dl.sourceforge.net/project/name/dir/subdir/";,
+ "https://downloads.sourceforge.net/project/name/dir/subdir/";)
+
+ vt.Output(
+ "WARN: filename.mk:1: A direct download URL is not a user-friendly homepage.",
+ "WARN: filename.mk:2: A direct download URL is not a user-friendly homepage.")
+}
+
+func (s *Suite) Test_HomepageChecker_checkReachable(c *check.C) {
+ t := s.Init(c)
+ vt := NewVartypeCheckTester(t, BtHomepage)
+
+ t.SetUpCommandLine("--network")
+
+ mux := http.NewServeMux()
+ mux.HandleFunc("/status/", func(writer http.ResponseWriter, request *http.Request) {
+ location := request.URL.Query().Get("location")
+ if location != "" {
+ writer.Header().Set("Location", location)
+ }
+
+ status, err := strconv.Atoi(request.URL.Path[len("/status/"):])
+ assertNil(err, "")
+ writer.WriteHeader(status)
+ })
+ mux.HandleFunc("/timeout", func(http.ResponseWriter, *http.Request) {
+ time.Sleep(5 * time.Second)
+ })
+
+ // 28780 = 256 * 'p' + 'l'
+ srv := http.Server{Addr: "localhost:28780", Handler: mux}
+ listener, err := net.Listen("tcp", srv.Addr)
+ assertNil(err, "")
+ shutdown := make(chan bool)
+
+ go func() {
+ err = srv.Serve(listener)
+ assertf(err == http.ErrServerClosed, "%s", err)
+ shutdown <- true
+ }()
+
+ defer func() {
+ err := srv.Shutdown(context.Background())
+ assertNil(err, "")
+ <-shutdown
+ }()
+
+ vt.Varname("HOMEPAGE")
+ vt.Values(
+ "http://localhost:28780/status/200";,
+ "http://localhost:28780/status/301?location=/redirect301";,
+ "http://localhost:28780/status/302?location=/redirect302";,
+ "http://localhost:28780/status/307?location=/redirect307";,
+ "http://localhost:28780/status/404";,
+ "http://localhost:28780/status/500";)
+
+ vt.Output(
+ "WARN: filename.mk:1: HOMEPAGE should migrate from http to https.",
+ "WARN: filename.mk:2: HOMEPAGE should migrate from http to https.",
+ "WARN: filename.mk:2: Homepage "+
+ "\"http://localhost:28780/status/301?location=/redirect301\"; "+
+ "redirects to \"http://localhost:28780/redirect301\".";,
+ "WARN: filename.mk:3: HOMEPAGE should migrate from http to https.",
+ "WARN: filename.mk:3: Homepage "+
+ "\"http://localhost:28780/status/302?location=/redirect302\"; "+
+ "redirects to \"http://localhost:28780/redirect302\".";,
+ "WARN: filename.mk:4: HOMEPAGE should migrate from http to https.",
+ "WARN: filename.mk:4: Homepage "+
+ "\"http://localhost:28780/status/307?location=/redirect307\"; "+
+ "redirects to \"http://localhost:28780/redirect307\".";,
+ "WARN: filename.mk:5: HOMEPAGE should migrate from http to https.",
+ "WARN: filename.mk:5: Homepage \"http://localhost:28780/status/404\"; "+
+ "returns HTTP status \"404 Not Found\".",
+ "WARN: filename.mk:6: HOMEPAGE should migrate from http to https.",
+ "WARN: filename.mk:6: Homepage \"http://localhost:28780/status/500\"; "+
+ "returns HTTP status \"500 Internal Server Error\".")
+
+ vt.Values(
+ "http://localhost:28780/timeout";)
+
+ vt.Output(
+ "WARN: filename.mk:11: HOMEPAGE should migrate from http to https.",
+ "WARN: filename.mk:11: Homepage \"http://localhost:28780/timeout\"; "+
+ "cannot be checked: timeout")
+
+ vt.Values(
+ "http://localhost:28780/%invalid";)
+
+ vt.Output(
+ "WARN: filename.mk:21: HOMEPAGE should migrate from http to https.",
+ "ERROR: filename.mk:21: Invalid URL \"http://localhost:28780/%invalid\".";)
+
+ vt.Values(
+ "http://localhost:28781/";)
+
+ // The "unknown network error" is for compatibility with Go < 1.13.
+ t.CheckOutputMatches(
+ "WARN: filename.mk:31: HOMEPAGE should migrate from http to https.",
+ `^WARN: filename\.mk:31: Homepage "http://localhost:28781/"; `+
+ `cannot be checked: (connection refused|unknown network error:.*)$`)
+
+ vt.Values(
+ "https://no-such-name.example.org/";)
+
+ // The "unknown network error" is for compatibility with Go < 1.13.
+ t.CheckOutputMatches(
+ `^WARN: filename\.mk:41: Homepage "https://no-such-name.example.org/"; ` +
+ `cannot be checked: (name not found|unknown network error:.*)$`)
+}
+
+func (s *Suite) Test_HomepageChecker_isReachable(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("--network")
+
+ mux := http.NewServeMux()
+ mux.HandleFunc("/status/", func(writer http.ResponseWriter, request *http.Request) {
+ location := request.URL.Query().Get("location")
+ if location != "" {
+ writer.Header().Set("Location", location)
+ }
+
+ status, err := strconv.Atoi(request.URL.Path[len("/status/"):])
+ assertNil(err, "")
+ writer.WriteHeader(status)
+ })
+ mux.HandleFunc("/timeout", func(http.ResponseWriter, *http.Request) {
+ time.Sleep(5 * time.Second)
+ })
+ mux.HandleFunc("/ok/", func(http.ResponseWriter, *http.Request) {})
+
+ // 28780 = 256 * 'p' + 'l'
+ srv := http.Server{Addr: "localhost:28780", Handler: mux}
+ listener, err := net.Listen("tcp", srv.Addr)
+ assertNil(err, "")
+ shutdown := make(chan bool)
+
+ go func() {
+ err := srv.Serve(listener)
+ assertf(err == http.ErrServerClosed, "%s", err)
+ shutdown <- true
+ }()
+
+ defer func() {
+ err := srv.Shutdown(context.Background())
+ assertNil(err, "")
+ <-shutdown
+ }()
+
+ test := func(url string, reachable YesNoUnknown) {
+ actual := (*HomepageChecker).isReachable(nil, url)
+
+ t.CheckEquals(actual, reachable)
+ }
+
+ test("http://localhost:28780/status/200";, yes)
+ test("http://localhost:28780/status/301?location=/";, no)
+ test("http://localhost:28780/status/404";, no)
+ test("http://localhost:28780/status/500";, no)
+ test("http://localhost:28780/timeout";, no)
+ test("http://localhost:28780/ok/${VAR}";, unknown)
+ test("http://localhost:28780/ invalid", unknown)
+ test("http://localhost:28780/%invalid";, no)
+ test("http://localhost:28781/";, no)
+}
+
+func (s *Suite) Test_HomepageChecker_hasAnySuffix(c *check.C) {
+ t := s.Init(c)
+
+ test := func(s string, hasAnySuffix bool, suffixes ...string) {
+ actual := (*HomepageChecker).hasAnySuffix(nil, s, suffixes...)
+
+ t.CheckEquals(actual, hasAnySuffix)
+ }
+
+ test("example.org", true, "org")
+ test("example.com", false, "org")
+ test("example.org", true, "example.org")
+ test("example.org", false, ".example.org")
+ test("example.org", true, ".org")
+}
+
+func (s *Suite) Test_HomepageChecker_classifyNetworkError(c *check.C) {
+ t := s.Init(c)
+
+ test := func(err error, expectedClass string) {
+ actual := (*HomepageChecker).classifyNetworkError(nil, err)
+
+ t.CheckEquals(actual, expectedClass)
+ }
+
+ test(syscall.Errno(10061), "connection refused")
+ test(syscall.ECONNREFUSED, "connection refused")
+ test(errors.New("unknown"), "unknown network error: unknown")
+}
Home |
Main Index |
Thread Index |
Old Index