Received: by mail.netbsd.org (Postfix, from userid 605) id 77B1984DB3; Mon, 2 Dec 2019 23:32:22 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by mail.netbsd.org (Postfix) with ESMTP id ED06B84CE7 for ; Mon, 2 Dec 2019 23:32:21 +0000 (UTC) X-Virus-Scanned: amavisd-new at netbsd.org Received: from mail.netbsd.org ([127.0.0.1]) by localhost (mail.netbsd.org [127.0.0.1]) (amavisd-new, port 10025) with ESMTP id z9RrXcpOzUZo for ; Mon, 2 Dec 2019 23:32:11 +0000 (UTC) Received: from cvs.NetBSD.org (ivanova.NetBSD.org [IPv6:2001:470:a085:999:28c:faff:fe03:5984]) by mail.netbsd.org (Postfix) with ESMTP id 1149184CE3 for ; Mon, 2 Dec 2019 23:32:11 +0000 (UTC) Received: by cvs.NetBSD.org (Postfix, from userid 500) id 0E2BBFA97; Mon, 2 Dec 2019 23:32:11 +0000 (UTC) Content-Transfer-Encoding: 7bit Content-Type: multipart/mixed; boundary="_----------=_157532953163210" MIME-Version: 1.0 Date: Mon, 2 Dec 2019 23:32:11 +0000 From: "Roland Illig" Subject: CVS commit: pkgsrc/pkgtools/pkglint To: pkgsrc-changes@NetBSD.org Reply-To: rillig@netbsd.org X-Mailer: log_accum Message-Id: <20191202233211.0E2BBFA97@cvs.NetBSD.org> Sender: pkgsrc-changes-owner@NetBSD.org List-Id: pkgsrc-changes.NetBSD.org Precedence: bulk List-Unsubscribe: This is a multi-part message in MIME format. --_----------=_157532953163210 Content-Disposition: inline Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII" Module Name: pkgsrc Committed By: rillig Date: Mon Dec 2 23:32:10 UTC 2019 Modified Files: pkgsrc/pkgtools/pkglint: Makefile PLIST pkgsrc/pkgtools/pkglint/files: alternatives.go autofix.go buildlink3.go buildlink3_test.go category.go category_test.go check_test.go distinfo.go distinfo_test.go files.go files_test.go licenses.go line.go linelexer.go linelexer_test.go lines.go logging.go mklexer.go mkline.go mkline_test.go mklinechecker.go mklinechecker_test.go mklineparser_test.go mklines.go mklines_test.go mkparser.go mkshparser_test.go options_test.go package.go package_test.go patches.go path.go path_test.go pkglint.0 pkglint.1 pkglint.go pkglint_test.go pkgsrc.go pkgsrc_test.go plist.go redundantscope.go redundantscope_test.go shell.go shell_test.go shtokenizer.go substcontext_test.go tools_test.go toplevel.go util.go util_test.go varalignblock.go vardefs.go vardefs_test.go vartype.go vartypecheck.go vartypecheck_test.go pkgsrc/pkgtools/pkglint/files/histogram: histogram.go pkgsrc/pkgtools/pkglint/files/intqa: qa.go qa_test.go pkgsrc/pkgtools/pkglint/files/textproc: lexer_bench_test.go pkgsrc/pkgtools/pkglint/files/trace: tracing_test.go Removed Files: pkgsrc/pkgtools/pkglint/files: testnames_test.go Log Message: pkgtools/pkglint: update to 19.3.13 Changes since 19.3.12: The command line option -Wspace has been removed. Warnings and notes about whitespace are now generated by default and cannot be switched off. This is to ensure a consistent visual appearance of the package Makefiles. Shell programs that are indented unnecessarily deep generate a note by default now. Before, the option -Wall was necessary to get these notes. The check for unintended comments in multi-line shell programs is now enabled again. It had been disabled some time ago as byproduct of a bug fix in the shell parser. The check for unique buildlink3 package identifiers now also works if pkglint is run from a package directory instead of the pkgsrc root directory. To generate a diff of this commit: cvs rdiff -u -r1.612 -r1.613 pkgsrc/pkgtools/pkglint/Makefile cvs rdiff -u -r1.18 -r1.19 pkgsrc/pkgtools/pkglint/PLIST cvs rdiff -u -r1.17 -r1.18 pkgsrc/pkgtools/pkglint/files/alternatives.go cvs rdiff -u -r1.32 -r1.33 pkgsrc/pkgtools/pkglint/files/autofix.go cvs rdiff -u -r1.26 -r1.27 pkgsrc/pkgtools/pkglint/files/buildlink3.go \ pkgsrc/pkgtools/pkglint/files/category.go cvs rdiff -u -r1.36 -r1.37 pkgsrc/pkgtools/pkglint/files/buildlink3_test.go cvs rdiff -u -r1.27 -r1.28 pkgsrc/pkgtools/pkglint/files/category_test.go cvs rdiff -u -r1.56 -r1.57 pkgsrc/pkgtools/pkglint/files/check_test.go cvs rdiff -u -r1.38 -r1.39 pkgsrc/pkgtools/pkglint/files/distinfo.go \ pkgsrc/pkgtools/pkglint/files/mkparser.go \ pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go \ pkgsrc/pkgtools/pkglint/files/util_test.go cvs rdiff -u -r1.34 -r1.35 pkgsrc/pkgtools/pkglint/files/distinfo_test.go cvs rdiff -u -r1.28 -r1.29 pkgsrc/pkgtools/pkglint/files/files.go \ pkgsrc/pkgtools/pkglint/files/licenses.go cvs rdiff -u -r1.29 -r1.30 pkgsrc/pkgtools/pkglint/files/files_test.go cvs rdiff -u -r1.40 -r1.41 pkgsrc/pkgtools/pkglint/files/line.go cvs rdiff -u -r1.7 -r1.8 pkgsrc/pkgtools/pkglint/files/linelexer.go cvs rdiff -u -r1.4 -r1.5 pkgsrc/pkgtools/pkglint/files/linelexer_test.go \ pkgsrc/pkgtools/pkglint/files/mklineparser_test.go cvs rdiff -u -r1.10 -r1.11 pkgsrc/pkgtools/pkglint/files/lines.go cvs rdiff -u -r1.31 -r1.32 pkgsrc/pkgtools/pkglint/files/logging.go cvs rdiff -u -r1.2 -r1.3 pkgsrc/pkgtools/pkglint/files/mklexer.go cvs rdiff -u -r1.66 -r1.67 pkgsrc/pkgtools/pkglint/files/mkline.go cvs rdiff -u -r1.73 -r1.74 pkgsrc/pkgtools/pkglint/files/mkline_test.go cvs rdiff -u -r1.55 -r1.56 pkgsrc/pkgtools/pkglint/files/mklinechecker.go cvs rdiff -u -r1.50 -r1.51 \ pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go \ pkgsrc/pkgtools/pkglint/files/shell.go cvs rdiff -u -r1.61 -r1.62 pkgsrc/pkgtools/pkglint/files/mklines.go \ pkgsrc/pkgtools/pkglint/files/util.go \ pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go cvs rdiff -u -r1.52 -r1.53 pkgsrc/pkgtools/pkglint/files/mklines_test.go \ pkgsrc/pkgtools/pkglint/files/pkglint_test.go cvs rdiff -u -r1.21 -r1.22 pkgsrc/pkgtools/pkglint/files/mkshparser_test.go \ pkgsrc/pkgtools/pkglint/files/shtokenizer.go cvs rdiff -u -r1.19 -r1.20 pkgsrc/pkgtools/pkglint/files/options_test.go cvs rdiff -u -r1.71 -r1.72 pkgsrc/pkgtools/pkglint/files/package.go cvs rdiff -u -r1.60 -r1.61 pkgsrc/pkgtools/pkglint/files/package_test.go cvs rdiff -u -r1.33 -r1.34 pkgsrc/pkgtools/pkglint/files/patches.go cvs rdiff -u -r1.3 -r1.4 pkgsrc/pkgtools/pkglint/files/path.go \ pkgsrc/pkgtools/pkglint/files/path_test.go cvs rdiff -u -r1.41 -r1.42 pkgsrc/pkgtools/pkglint/files/pkglint.0 cvs rdiff -u -r1.58 -r1.59 pkgsrc/pkgtools/pkglint/files/pkglint.1 \ pkgsrc/pkgtools/pkglint/files/shell_test.go cvs rdiff -u -r1.67 -r1.68 pkgsrc/pkgtools/pkglint/files/pkglint.go cvs rdiff -u -r1.44 -r1.45 pkgsrc/pkgtools/pkglint/files/pkgsrc.go cvs rdiff -u -r1.45 -r1.46 pkgsrc/pkgtools/pkglint/files/plist.go cvs rdiff -u -r1.8 -r1.9 pkgsrc/pkgtools/pkglint/files/redundantscope.go \ pkgsrc/pkgtools/pkglint/files/redundantscope_test.go cvs rdiff -u -r1.30 -r1.31 pkgsrc/pkgtools/pkglint/files/substcontext_test.go cvs rdiff -u -r1.10 -r0 pkgsrc/pkgtools/pkglint/files/testnames_test.go cvs rdiff -u -r1.22 -r1.23 pkgsrc/pkgtools/pkglint/files/tools_test.go cvs rdiff -u -r1.25 -r1.26 pkgsrc/pkgtools/pkglint/files/toplevel.go \ pkgsrc/pkgtools/pkglint/files/vardefs_test.go cvs rdiff -u -r1.9 -r1.10 pkgsrc/pkgtools/pkglint/files/varalignblock.go cvs rdiff -u -r1.79 -r1.80 pkgsrc/pkgtools/pkglint/files/vardefs.go cvs rdiff -u -r1.39 -r1.40 pkgsrc/pkgtools/pkglint/files/vartype.go cvs rdiff -u -r1.68 -r1.69 pkgsrc/pkgtools/pkglint/files/vartypecheck.go cvs rdiff -u -r1.4 -r1.5 pkgsrc/pkgtools/pkglint/files/histogram/histogram.go cvs rdiff -u -r1.1 -r1.2 pkgsrc/pkgtools/pkglint/files/intqa/qa.go \ pkgsrc/pkgtools/pkglint/files/intqa/qa_test.go cvs rdiff -u -r1.1 -r1.2 \ pkgsrc/pkgtools/pkglint/files/textproc/lexer_bench_test.go cvs rdiff -u -r1.6 -r1.7 pkgsrc/pkgtools/pkglint/files/trace/tracing_test.go Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. --_----------=_157532953163210 Content-Disposition: inline Content-Length: 220909 Content-Transfer-Encoding: binary Content-Type: text/x-diff; charset=utf-8 Modified files: Index: pkgsrc/pkgtools/pkglint/Makefile diff -u pkgsrc/pkgtools/pkglint/Makefile:1.612 pkgsrc/pkgtools/pkglint/Makefile:1.613 --- pkgsrc/pkgtools/pkglint/Makefile:1.612 Sat Nov 30 20:35:11 2019 +++ pkgsrc/pkgtools/pkglint/Makefile Mon Dec 2 23:32:09 2019 @@ -1,6 +1,6 @@ -# $NetBSD: Makefile,v 1.612 2019/11/30 20:35:11 rillig Exp $ +# $NetBSD: Makefile,v 1.613 2019/12/02 23:32:09 rillig Exp $ -PKGNAME= pkglint-19.3.12 +PKGNAME= pkglint-19.3.13 CATEGORIES= pkgtools DISTNAME= tools MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/} Index: pkgsrc/pkgtools/pkglint/PLIST diff -u pkgsrc/pkgtools/pkglint/PLIST:1.18 pkgsrc/pkgtools/pkglint/PLIST:1.19 --- pkgsrc/pkgtools/pkglint/PLIST:1.18 Wed Nov 27 22:10:06 2019 +++ pkgsrc/pkgtools/pkglint/PLIST Mon Dec 2 23:32:09 2019 @@ -1,4 +1,4 @@ -@comment $NetBSD: PLIST,v 1.18 2019/11/27 22:10:06 rillig Exp $ +@comment $NetBSD: PLIST,v 1.19 2019/12/02 23:32:09 rillig Exp $ bin/pkglint gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint.a gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint/getopt.a @@ -105,7 +105,6 @@ gopkg/src/netbsd.org/pkglint/shtypes.go gopkg/src/netbsd.org/pkglint/shtypes_test.go gopkg/src/netbsd.org/pkglint/substcontext.go gopkg/src/netbsd.org/pkglint/substcontext_test.go -gopkg/src/netbsd.org/pkglint/testnames_test.go gopkg/src/netbsd.org/pkglint/textproc/lexer.go gopkg/src/netbsd.org/pkglint/textproc/lexer_bench_test.go gopkg/src/netbsd.org/pkglint/textproc/lexer_test.go Index: pkgsrc/pkgtools/pkglint/files/alternatives.go diff -u pkgsrc/pkgtools/pkglint/files/alternatives.go:1.17 pkgsrc/pkgtools/pkglint/files/alternatives.go:1.18 --- pkgsrc/pkgtools/pkglint/files/alternatives.go:1.17 Sat Nov 23 23:35:56 2019 +++ pkgsrc/pkgtools/pkglint/files/alternatives.go Mon Dec 2 23:32:09 2019 @@ -5,7 +5,7 @@ import ( "strings" ) -func CheckFileAlternatives(filename Path) { +func CheckFileAlternatives(filename CurrPath) { lines := Load(filename, NotEmpty|LogErrors) if lines == nil { return Index: pkgsrc/pkgtools/pkglint/files/autofix.go diff -u pkgsrc/pkgtools/pkglint/files/autofix.go:1.32 pkgsrc/pkgtools/pkglint/files/autofix.go:1.33 --- pkgsrc/pkgtools/pkglint/files/autofix.go:1.32 Sat Nov 30 20:35:11 2019 +++ pkgsrc/pkgtools/pkglint/files/autofix.go Mon Dec 2 23:32:09 2019 @@ -443,12 +443,12 @@ func SaveAutofixChanges(lines *Lines) (a if G.Testing { abs := G.Abs(lines.Filename) - absTmp := G.Abs(NewPathSlash(os.TempDir())) + absTmp := G.Abs(NewCurrPathSlash(os.TempDir())) assertf(abs.HasPrefixPath(absTmp), "%q must be inside %q", abs, absTmp) } - changes := make(map[Path][]string) - changed := make(map[Path]bool) + changes := make(map[CurrPath][]string) + changed := make(map[CurrPath]bool) for _, line := range lines.Lines { chlines := changes[line.Filename] if fix := line.autofix; fix != nil { Index: pkgsrc/pkgtools/pkglint/files/buildlink3.go diff -u pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.26 pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.27 --- pkgsrc/pkgtools/pkglint/files/buildlink3.go:1.26 Sat Nov 23 23:35:56 2019 +++ pkgsrc/pkgtools/pkglint/files/buildlink3.go Mon Dec 2 23:32:09 2019 @@ -99,7 +99,8 @@ func (ck *Buildlink3Checker) checkUnique return } - base, name := trimCommon(pkgbase, mkline.Filename.Dir().Base()) + dirname := G.Pkgsrc.ToRel(mkline.Filename.DirNoClean()).Base() + base, name := trimCommon(pkgbase, dirname) if base == "" && matches(name, `^(\d*|-cvs|-fossil|-git|-hg|-svn|-devel|-snapshot)$`) { return } Index: pkgsrc/pkgtools/pkglint/files/category.go diff -u pkgsrc/pkgtools/pkglint/files/category.go:1.26 pkgsrc/pkgtools/pkglint/files/category.go:1.27 --- pkgsrc/pkgtools/pkglint/files/category.go:1.26 Sat Nov 30 20:35:11 2019 +++ pkgsrc/pkgtools/pkglint/files/category.go Mon Dec 2 23:32:09 2019 @@ -6,7 +6,7 @@ import ( "strings" ) -func CheckdirCategory(dir Path) { +func CheckdirCategory(dir CurrPath) { if trace.Tracing { defer trace.Call(dir)() } @@ -64,7 +64,7 @@ func CheckdirCategory(dir Path) { for !mlex.EOF() { mkline := mlex.CurrentMkLine() - if (mkline.IsVarassignMaybeCommented()) && mkline.Varname() == "SUBDIR" { + if mkline.IsVarassignMaybeCommented() && mkline.Varname() == "SUBDIR" { mlex.Skip() name := mkline.Value() // TODO: Maybe NewPath here already @@ -155,7 +155,7 @@ func CheckdirCategory(dir Path) { mklines.SaveAutofixChanges() if G.Opts.Recursive { - var recurseInto []Path + var recurseInto []CurrPath for _, msub := range mSubdirs { if !msub.line.IsCommentedVarassign() { recurseInto = append(recurseInto, dir.JoinNoClean(msub.name)) Index: pkgsrc/pkgtools/pkglint/files/buildlink3_test.go diff -u pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.36 pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.37 --- pkgsrc/pkgtools/pkglint/files/buildlink3_test.go:1.36 Sat Nov 23 23:35:56 2019 +++ pkgsrc/pkgtools/pkglint/files/buildlink3_test.go Mon Dec 2 23:32:09 2019 @@ -599,7 +599,7 @@ func (s *Suite) Test_Buildlink3Checker_c G.InterPackage.Enable() - test := func(pkgbase string, pkgpath Path, diagnostics ...string) { + test := func(pkgbase string, pkgpath RelPath, diagnostics ...string) { mkline := t.NewMkLine(t.File(pkgpath+"/buildlink3.mk"), 123, "") (*Buildlink3Checker).checkUniquePkgbase(nil, pkgbase, mkline) @@ -611,7 +611,7 @@ func (s *Suite) Test_Buildlink3Checker_c test("php", "lang/php56", nil...) - // No warning since "php" is a valid buildlink3 basename for "php56". + // No warning since "php" is a valid buildlink3 basename for "php72". test("php", "lang/php72", nil...) @@ -647,6 +647,30 @@ func (s *Suite) Test_Buildlink3Checker_c nil...) } +func (s *Suite) Test_Buildlink3Checker_checkUniquePkgbase__chdir(c *check.C) { + t := s.Init(c) + + G.InterPackage.Enable() + t.Chdir("lang/php56") + + test := func(pkgbase string, path CurrPath, diagnostics ...string) { + mkline := t.NewMkLine(path.JoinNoClean("buildlink3.mk"), 123, "") + + (*Buildlink3Checker).checkUniquePkgbase(nil, pkgbase, mkline) + + t.CheckOutput(diagnostics) + } + + test("php", "../../lang/php72", + nil...) + + // Using the identifier "php" is ok in the current directory since + // its relative path from the pkgsrc root is "lang/php56", which + // starts with "php" as well. + test("php", ".", + nil...) +} + func (s *Suite) Test_Buildlink3Checker_checkSecondParagraph__missing_mkbase(c *check.C) { t := s.Init(c) Index: pkgsrc/pkgtools/pkglint/files/category_test.go diff -u pkgsrc/pkgtools/pkglint/files/category_test.go:1.27 pkgsrc/pkgtools/pkglint/files/category_test.go:1.28 --- pkgsrc/pkgtools/pkglint/files/category_test.go:1.27 Sat Nov 23 23:35:56 2019 +++ pkgsrc/pkgtools/pkglint/files/category_test.go Mon Dec 2 23:32:09 2019 @@ -208,12 +208,12 @@ func (s *Suite) Test_CheckdirCategory__r // It is only removed in Pkglint.Main, therefore it stays there even // after the call to CheckdirCategory. This is a bit unrealistic, // but close enough for this test. - t.CheckDeepEquals(G.Todo.entries, []Path{"."}) + t.CheckDeepEquals(G.Todo.entries, []CurrPath{"."}) CheckdirCategory(".") t.CheckOutputEmpty() - t.CheckDeepEquals(G.Todo.entries, []Path{"./package", "."}) + t.CheckDeepEquals(G.Todo.entries, []CurrPath{"./package", "."}) } // Ensures that a directory in the file system can be added at the very Index: pkgsrc/pkgtools/pkglint/files/check_test.go diff -u pkgsrc/pkgtools/pkglint/files/check_test.go:1.56 pkgsrc/pkgtools/pkglint/files/check_test.go:1.57 --- pkgsrc/pkgtools/pkglint/files/check_test.go:1.56 Wed Nov 27 22:10:07 2019 +++ pkgsrc/pkgtools/pkglint/files/check_test.go Mon Dec 2 23:32:09 2019 @@ -5,6 +5,7 @@ import ( "fmt" "io" "io/ioutil" + "netbsd.org/pkglint/intqa" "netbsd.org/pkglint/regex" "os" "regexp" @@ -74,7 +75,7 @@ func (s *Suite) SetUpTest(c *check.C) { prevdir, err := os.Getwd() assertNil(err, "Cannot get current working directory: %s", err) - t.prevdir = prevdir + t.prevdir = NewCurrPathString(prevdir) // No longer usable; see https://github.com/go-check/check/issues/22 t.c = nil @@ -84,7 +85,7 @@ func (s *Suite) TearDownTest(c *check.C) t := s.Tester t.c = nil // No longer usable; see https://github.com/go-check/check/issues/22 - err := os.Chdir(t.prevdir) + err := os.Chdir(t.prevdir.String()) assertNil(err, "Cannot chdir back to previous dir: %s", err) if t.seenSetupPkgsrc > 0 && !t.seenFinish && !t.seenMain { @@ -98,6 +99,19 @@ func (s *Suite) TearDownTest(c *check.C) G = unusablePkglint() } +// Ensures that all test names follow a common naming scheme: +// +// Test_${Type}_${Method}__${description_using_underscores} +func (s *Suite) Test__qa(c *check.C) { + ck := intqa.NewQAChecker(c.Errorf) + ck.Configure("*", "*", "*", -intqa.EMissingTest) + ck.Configure("path.go", "*", "*", +intqa.EMissingTest) + ck.Configure("*yacc.go", "*", "*", intqa.ENone) + ck.Configure("*", "*", "", -intqa.EMissingTest) + ck.Configure("*.go", "Suite", "*", -intqa.EMethodsSameFile) + ck.Check() +} + var _ = check.Suite(new(Suite)) func Test(t *testing.T) { check.TestingT(t) } @@ -113,9 +127,9 @@ type Tester struct { stdout bytes.Buffer stderr bytes.Buffer - tmpdir Path - prevdir string // The current working directory before the test started - relCwd Path // See Tester.Chdir + tmpdir CurrPath + prevdir CurrPath // The current working directory before the test started + cwd RelPath // relative to tmpdir; see Tester.Chdir seenSetUpCommandLine bool seenSetupPkgsrc int @@ -193,33 +207,33 @@ func (t *Tester) SetUpTool(name, varname // The file is then read in, without interpreting line continuations. // // See SetUpFileMkLines for loading a Makefile fragment. -func (t *Tester) SetUpFileLines(relativeFileName Path, lines ...string) *Lines { - filename := t.CreateFileLines(relativeFileName, lines...) - return Load(filename, MustSucceed) +func (t *Tester) SetUpFileLines(filename RelPath, lines ...string) *Lines { + abs := t.CreateFileLines(filename, lines...) + return Load(abs, MustSucceed) } // SetUpFileLines creates a temporary file and writes the given lines to it. // The file is then read in, handling line continuations for Makefiles. // // See SetUpFileLines for loading an ordinary file. -func (t *Tester) SetUpFileMkLines(relativeFileName Path, lines ...string) *MkLines { - filename := t.CreateFileLines(relativeFileName, lines...) - return LoadMk(filename, MustSucceed) +func (t *Tester) SetUpFileMkLines(filename RelPath, lines ...string) *MkLines { + abs := t.CreateFileLines(filename, lines...) + return LoadMk(abs, MustSucceed) } // LoadMkInclude loads the given Makefile fragment and all the files it includes, // merging all the lines into a single MkLines object. // // This is useful for testing code related to Package.readMakefile. -func (t *Tester) LoadMkInclude(relativeFileName Path) *MkLines { +func (t *Tester) LoadMkInclude(filename RelPath) *MkLines { var lines []*Line // TODO: Include files with multiple-inclusion guard only once. // TODO: Include files without multiple-inclusion guard as often as needed. // TODO: Set an upper limit, to prevent denial of service. - var load func(filename Path) - load = func(filename Path) { + var load func(filename CurrPath) + load = func(filename CurrPath) { for _, mkline := range NewMkLines(Load(filename, MustSucceed)).mklines { lines = append(lines, mkline.Line) @@ -229,11 +243,11 @@ func (t *Tester) LoadMkInclude(relativeF } } - load(t.File(relativeFileName)) + load(t.File(filename)) // This assumes that the test files do not contain parse errors. // Otherwise the diagnostics would appear twice. - return NewMkLines(NewLines(t.File(relativeFileName), lines)) + return NewMkLines(NewLines(t.File(filename), lines)) } // SetUpPkgsrc sets up a minimal but complete pkgsrc installation in the @@ -319,8 +333,8 @@ func (t *Tester) SetUpPkgsrc() { // SetUpCategory makes the given category valid by creating a dummy Makefile. // After that, it can be mentioned in the CATEGORIES variable of a package. -func (t *Tester) SetUpCategory(name Path) { - assert(name.Count() == 1) +func (t *Tester) SetUpCategory(name RelPath) { + assert(G.Pkgsrc.ToRel(t.File(name)).Count() == 1) makefile := name.JoinNoClean("Makefile") if !t.File(makefile).IsFile() { @@ -340,11 +354,13 @@ func (t *Tester) SetUpCategory(name Path // After calling this method, individual files can be overwritten as necessary. // At the end of the setup phase, t.FinishSetUp() must be called to load all // the files. -func (t *Tester) SetUpPackage(pkgpath Path, makefileLines ...string) Path { - assertf(matches(pkgpath.String(), `^[^/]+/[^/]+$`), "pkgpath %q must have the form \"category/package\"", pkgpath) +func (t *Tester) SetUpPackage(pkgpath RelPath, makefileLines ...string) CurrPath { + assertf( + matches(pkgpath.String(), `^[^/]+/[^/]+$`), + "pkgpath %q must have the form \"category/package\"", pkgpath) distname := pkgpath.Base() - category := pkgpath.Dir() + category := pkgpath.DirNoClean() if category == "wip" { // To avoid boilerplate CATEGORIES definitions for wip packages. category = "local" @@ -431,29 +447,32 @@ line: // given lines to it. // // It returns the full path to the created file. -func (t *Tester) CreateFileLines(relativeFileName Path, lines ...string) (filename Path) { +func (t *Tester) CreateFileLines(filename RelPath, lines ...string) CurrPath { var content strings.Builder for _, line := range lines { content.WriteString(line) content.WriteString("\n") } - filename = t.File(relativeFileName) - err := os.MkdirAll(filename.Dir().String(), 0777) + abs := t.File(filename) + err := os.MkdirAll(abs.DirNoClean().String(), 0777) t.c.Assert(err, check.IsNil) - err = filename.WriteString(content.String()) + err = abs.WriteString(content.String()) t.c.Assert(err, check.IsNil) - G.fileCache.Evict(filename) + G.fileCache.Evict(abs) - return filename + return abs } // CreateFileDummyPatch creates a patch file with the given name in the // temporary directory. -func (t *Tester) CreateFileDummyPatch(relativeFileName Path) { - t.CreateFileLines(relativeFileName, +func (t *Tester) CreateFileDummyPatch(filename RelPath) { + // Patch files only make sense in category/package/patches directories. + assert(G.Pkgsrc.ToRel(t.File(filename)).Count() == 4) + + t.CreateFileLines(filename, CvsID, "", "Documentation", @@ -465,9 +484,11 @@ func (t *Tester) CreateFileDummyPatch(re "+new") } -func (t *Tester) CreateFileDummyBuildlink3(relativeFileName Path, customLines ...string) { - assert(relativeFileName.Count() == 3) - dir := relativeFileName.Dir() +func (t *Tester) CreateFileDummyBuildlink3(filename RelPath, customLines ...string) { + // Buildlink3.mk files only make sense in category/package directories. + assert(G.Pkgsrc.ToRel(t.File(filename)).Count() == 3) + + dir := filename.DirClean() lower := dir.Base() // see pkgtools/createbuildlink/files/createbuildlink, "package specific variables" upper := strings.Replace(strings.ToUpper(lower), "-", "_", -1) @@ -502,32 +523,33 @@ func (t *Tester) CreateFileDummyBuildlin "", sprintf("BUILDLINK_TREE+=\t-%s", lower)) - t.CreateFileLines(relativeFileName, lines...) + t.CreateFileLines(filename, lines...) } // File returns the absolute path to the given file in the // temporary directory. It doesn't check whether that file exists. // Calls to Tester.Chdir change the base directory for the relative filename. -func (t *Tester) File(relativeFileName Path) Path { - if t.tmpdir == "" { - t.tmpdir = NewPathSlash(t.c.MkDir()) +func (t *Tester) File(filename RelPath) CurrPath { + if t.tmpdir.IsEmpty() { + t.tmpdir = NewCurrPathSlash(t.c.MkDir()) } - if t.relCwd != "" { - return relativeFileName.Clean() + if t.cwd != "" { + return NewCurrPath(filename.Clean().AsPath()) } - return t.tmpdir.JoinClean(relativeFileName) + return t.tmpdir.JoinClean(filename.AsPath()) } // Copy copies a file inside the temporary directory. -func (t *Tester) Copy(relativeSrc, relativeDst Path) { - src := t.File(relativeSrc) - dst := t.File(relativeDst) +func (t *Tester) Copy(source, target RelPath) { + absSource := t.File(source) + absTarget := t.File(target) - data, err := src.ReadString() + data, err := absSource.ReadString() assertNil(err, "Copy.Read") - err = os.MkdirAll(dst.Dir().String(), 0777) + // FIXME: consider DirNoClean + err = os.MkdirAll(absTarget.DirClean().String(), 0777) assertNil(err, "Copy.MkdirAll") - err = dst.WriteString(data) + err = absTarget.WriteString(data) assertNil(err, "Copy.Write") } @@ -543,29 +565,30 @@ func (t *Tester) Copy(relativeSrc, relat // // As long as this method is not called in a test, the current working // directory is indeterminate. -func (t *Tester) Chdir(relativeDirName Path) { - if t.relCwd != "" { +func (t *Tester) Chdir(dirname RelPath) { + if t.cwd != "" { // When multiple calls of Chdir are mixed with calls to CreateFileLines, // the resulting Lines and MkLines variables will use relative filenames, // and these will point to different areas in the file system. This is // usually not indented and therefore prevented. - t.c.Fatalf("Chdir must only be called once per test; already in %q.", t.relCwd) + t.c.Fatalf("Chdir must only be called once per test; already in %q.", t.cwd) } - absDirName := t.File(relativeDirName) + absDirName := t.File(dirname) assertNil(os.MkdirAll(absDirName.String(), 0700), "MkDirAll") assertNil(os.Chdir(absDirName.String()), "Chdir") - t.relCwd = relativeDirName + t.cwd = dirname G.cwd = absDirName + G.Pkgsrc.topdir = NewCurrPath(absDirName.Rel(G.Pkgsrc.topdir)) } // Remove removes the file or directory from the temporary directory. // The file or directory must exist. -func (t *Tester) Remove(relativeFileName Path) { - filename := t.File(relativeFileName) - err := os.Remove(filename.String()) +func (t *Tester) Remove(filename RelPath) { + abs := t.File(filename) + err := os.Remove(abs.String()) t.c.Assert(err, check.IsNil) - G.fileCache.Evict(filename) + G.fileCache.Evict(abs) } // SetUpHierarchy provides a function for creating hierarchies of MkLines @@ -588,22 +611,42 @@ func (t *Tester) Remove(relativeFileName // module := get("subdir/module.mk") // // The filenames passed to the include function are all relative to the -// same location, but that location is irrelevant in practice. The generated -// .include lines take the relative paths into account. For example, when -// subdir/module.mk includes subdir/version.mk, the include line is just: +// same location, which is typically the pkgsrc root or a package directory, +// but the code doesn't care. +// +// The generated .include lines take the relative paths into account. +// For example, when subdir/module.mk includes subdir/version.mk, +// the include line is just: // .include "version.mk" func (t *Tester) SetUpHierarchy() ( - include func(filename Path, args ...interface{}) *MkLines, - get func(Path) *MkLines) { + include func(filename RelPath, args ...interface{}) *MkLines, + get func(path RelPath) *MkLines) { - files := map[Path]*MkLines{} + files := map[RelPath]*MkLines{} + basedir := NewPath(".") - include = func(filename Path, args ...interface{}) *MkLines { + // includePath returns the path to be used in an .include. + // + // This is the same mechanism that is used in Pkgsrc.Relpath. + includePath := func(including, included Path) Path { + // FIXME: consider DirNoClean + fromDir := including.DirClean() + to := basedir.Rel(included) + // FIXME: consider DirNoClean + if fromDir == to.DirClean() { + return NewPath(to.Base()) + } else { + return fromDir.Rel(basedir).JoinNoClean(to).CleanDot() + } + } + + include = func(filename RelPath, args ...interface{}) *MkLines { var lines []*Line lineno := 1 + relFilename := basedir.Rel(filename.AsPath()) addLine := func(text string) { - lines = append(lines, t.NewLine(filename, lineno, text)) + lines = append(lines, t.NewLine(NewCurrPath(relFilename), lineno, text)) lineno++ } @@ -612,24 +655,21 @@ func (t *Tester) SetUpHierarchy() ( case string: addLine(arg) case *MkLines: - fromDir := G.Pkgsrc.File(filename.Dir()) - to := G.Pkgsrc.File(arg.lines.Filename) - rel := G.Pkgsrc.Relpath(fromDir, to) - text := sprintf(".include %q", rel) - addLine(text) + rel := includePath(relFilename, arg.lines.Filename.AsPath()) + addLine(sprintf(".include %q", rel)) lines = append(lines, arg.lines.Lines...) default: panic("invalid type") } } - mklines := NewMkLines(NewLines(filename, lines)) + mklines := NewMkLines(NewLines(NewCurrPath(relFilename), lines)) assertf(files[filename] == nil, "MkLines with name %q already exists.", filename) files[filename] = mklines return mklines } - get = func(filename Path) *MkLines { + get = func(filename RelPath) *MkLines { assertf(files[filename] != nil, "MkLines with name %q doesn't exist.", filename) return files[filename] } @@ -707,7 +747,7 @@ func (t *Tester) Main(args ...string) in argv := []string{"pkglint"} for _, arg := range args { - fileArg := t.File(NewPath(arg)) + fileArg := t.File(NewRelPathString(arg)) if fileArg.Exists() { argv = append(argv, fileArg.String()) } else { @@ -851,14 +891,14 @@ func (t *Tester) NewRawLines(args ...int // NewLine creates an in-memory line with the given text. // This line does not correspond to any line in a file. -func (t *Tester) NewLine(filename Path, lineno int, text string) *Line { +func (t *Tester) NewLine(filename CurrPath, lineno int, text string) *Line { textnl := text + "\n" rawLine := RawLine{lineno, textnl, textnl} return NewLine(filename, lineno, text, &rawLine) } // NewMkLine creates an in-memory line in the Makefile format with the given text. -func (t *Tester) NewMkLine(filename Path, lineno int, text string) *MkLine { +func (t *Tester) NewMkLine(filename CurrPath, lineno int, text string) *MkLine { basename := filename.Base() assertf( hasSuffix(basename, ".mk") || @@ -878,14 +918,14 @@ func (t *Tester) NewShellLineChecker(tex // NewLines returns a list of simple lines that belong together. // // To work with line continuations like in Makefiles, use SetUpFileMkLines. -func (t *Tester) NewLines(filename Path, lines ...string) *Lines { +func (t *Tester) NewLines(filename CurrPath, lines ...string) *Lines { return t.NewLinesAt(filename, 1, lines...) } // NewLinesAt returns a list of simple lines that belong together. // // To work with line continuations like in Makefiles, use SetUpFileMkLines. -func (t *Tester) NewLinesAt(filename Path, firstLine int, texts ...string) *Lines { +func (t *Tester) NewLinesAt(filename CurrPath, firstLine int, texts ...string) *Lines { lines := make([]*Line, len(texts)) for i, text := range texts { lines[i] = t.NewLine(filename, i+firstLine, text) @@ -899,7 +939,7 @@ func (t *Tester) NewLinesAt(filename Pat // // No actual file is created for the lines; // see SetUpFileMkLines for loading Makefile fragments with line continuations. -func (t *Tester) NewMkLines(filename Path, lines ...string) *MkLines { +func (t *Tester) NewMkLines(filename CurrPath, lines ...string) *MkLines { basename := filename.Base() assertf( hasSuffix(basename, ".mk") || basename == "Makefile" || hasPrefix(basename, "Makefile."), @@ -980,7 +1020,7 @@ func (t *Tester) CheckOutputLinesMatchin // See CheckOutputEmpty, CheckOutputLines. func (t *Tester) CheckOutputLinesIgnoreSpace(expectedLines ...string) { assertf(len(expectedLines) > 0, "To check empty lines, use CheckOutputEmpty instead.") - assertf(t.tmpdir != "", "Tester must be initialized before checking the output.") + assertf(!t.tmpdir.IsEmpty(), "Tester must be initialized before checking the output.") rawOutput := t.stdout.String() + t.stderr.String() _ = t.Output() // Just to consume the output @@ -989,7 +1029,7 @@ func (t *Tester) CheckOutputLinesIgnoreS t.CheckDeepEquals(actual, expected) } -func (t *Tester) compareOutputIgnoreSpace(rawOutput string, expectedLines []string, tmpdir Path) ([]string, []string) { +func (t *Tester) compareOutputIgnoreSpace(rawOutput string, expectedLines []string, tmpdir CurrPath) ([]string, []string) { whitespace := regexp.MustCompile(`\s+`) // Replace all occurrences of tmpdir in the raw output with a tilde, @@ -1023,7 +1063,7 @@ func (s *Suite) Test_Tester_compareOutpu t := s.Init(c) lines := func(lines ...string) []string { return lines } - test := func(rawOutput string, expectedLines []string, tmpdir Path, eq bool) { + test := func(rawOutput string, expectedLines []string, tmpdir CurrPath, eq bool) { actual, expected := t.compareOutputIgnoreSpace(rawOutput, expectedLines, tmpdir) t.CheckEquals(actual == nil && expected == nil, eq) } @@ -1143,8 +1183,8 @@ func (t *Tester) DisableTracing() { // CheckFileLines loads the lines from the temporary file and checks that // they equal the given lines. -func (t *Tester) CheckFileLines(relativeFileName Path, lines ...string) { - content, err := t.File(relativeFileName).ReadString() +func (t *Tester) CheckFileLines(filename RelPath, lines ...string) { + content, err := t.File(filename).ReadString() t.c.Assert(err, check.IsNil) actualLines := strings.Split(content, "\n") actualLines = actualLines[:len(actualLines)-1] @@ -1155,8 +1195,8 @@ func (t *Tester) CheckFileLines(relative // that they equal the given lines. The loaded file may use tabs or spaces // for indentation, while the lines in the code use spaces exclusively, // in order to make the depth of the indentation clearly visible in the test code. -func (t *Tester) CheckFileLinesDetab(relativeFileName Path, lines ...string) { - actualLines := Load(t.File(relativeFileName), MustSucceed) +func (t *Tester) CheckFileLinesDetab(filename RelPath, lines ...string) { + actualLines := Load(t.File(filename), MustSucceed) var detabbedLines []string for _, line := range actualLines.Lines { Index: pkgsrc/pkgtools/pkglint/files/distinfo.go diff -u pkgsrc/pkgtools/pkglint/files/distinfo.go:1.38 pkgsrc/pkgtools/pkglint/files/distinfo.go:1.39 --- pkgsrc/pkgtools/pkglint/files/distinfo.go:1.38 Sat Nov 30 20:35:11 2019 +++ pkgsrc/pkgtools/pkglint/files/distinfo.go Mon Dec 2 23:32:09 2019 @@ -17,7 +17,7 @@ func CheckLinesDistinfo(pkg *Package, li } filename := lines.Filename - patchdir := NewPath("patches") + patchdir := NewPackagePath("patches") if pkg != nil && pkg.File(pkg.Patchdir).IsDir() { patchdir = pkg.Patchdir } @@ -40,7 +40,7 @@ func CheckLinesDistinfo(pkg *Package, li type distinfoLinesChecker struct { pkg *Package lines *Lines - patchdir Path // Relative to pkg + patchdir PackagePath distinfoIsCommitted bool filenames []Path // For keeping the order from top to bottom @@ -89,7 +89,7 @@ func (ck *distinfoLinesChecker) parse() continue } - if prevFilename != "" && filename != prevFilename { + if !prevFilename.IsEmpty() && filename != prevFilename { finishGroup() } prevFilename = filename @@ -97,7 +97,7 @@ func (ck *distinfoLinesChecker) parse() hashes = append(hashes, distinfoHash{line, filename, alg, hash}) } - if prevFilename != "" { + if !prevFilename.IsEmpty() { finishGroup() } } @@ -194,7 +194,7 @@ func (ck *distinfoLinesChecker) checkAlg distdir := G.Pkgsrc.File("distfiles") - distfile := cleanpath(distdir.JoinNoClean(info.filename())) + distfile := distdir.JoinNoClean(info.filename()).CleanPath() if !distfile.IsFile() { // It's a rare situation that the explanation is generated @@ -380,7 +380,7 @@ func (ck *distinfoLinesChecker) checkUnc } } -func (ck *distinfoLinesChecker) checkPatchSha1(line *Line, patchFileName Path, distinfoSha1Hex string) { +func (ck *distinfoLinesChecker) checkPatchSha1(line *Line, patchFileName PackagePath, distinfoSha1Hex string) { lines := Load(ck.pkg.File(patchFileName), 0) if lines == nil { line.Errorf("Patch %s does not exist.", patchFileName) Index: pkgsrc/pkgtools/pkglint/files/mkparser.go diff -u pkgsrc/pkgtools/pkglint/files/mkparser.go:1.38 pkgsrc/pkgtools/pkglint/files/mkparser.go:1.39 --- pkgsrc/pkgtools/pkglint/files/mkparser.go:1.38 Sat Nov 30 20:35:11 2019 +++ pkgsrc/pkgtools/pkglint/files/mkparser.go Mon Dec 2 23:32:09 2019 @@ -48,7 +48,7 @@ func (p *MkParser) MkCond() *MkCond { for { mark := p.lexer.Mark() p.lexer.SkipHspace() - if !(p.lexer.SkipString("||")) { + if !p.lexer.SkipString("||") { break } next := p.mkCondAnd() Index: pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.38 pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.39 --- pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.38 Sat Nov 30 20:35:11 2019 +++ pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go Mon Dec 2 23:32:09 2019 @@ -1132,6 +1132,7 @@ func (s *Suite) Test_Pkgsrc_checkTopleve t.CreateFileLines("category/Makefile", MkCvsID, + "", "COMMENT=\tExample category", "", "SUBDIR+=\tpackage", @@ -1176,15 +1177,19 @@ func (s *Suite) Test_Pkgsrc_ReadDir(c *c func (s *Suite) Test_Pkgsrc_Relpath(c *check.C) { t := s.Init(c) - t.Chdir(".") t.CheckEquals(G.Pkgsrc.topdir, t.tmpdir) + t.Chdir(".") + t.CheckEquals(G.Pkgsrc.topdir, NewCurrPath(".")) - test := func(from, to Path, result Path) { + test := func(from, to CurrPath, result Path) { t.CheckEquals(G.Pkgsrc.Relpath(from, to), result) } // TODO: add tests going from each of (top, cat, pkg, pkgsub) to the others + test("category", "other/package", "../other/package") + test("category/package", "other", "../../other") + test("some/dir", "some/directory", "../../some/directory") test("some/directory", "some/dir", "../../some/dir") @@ -1211,7 +1216,7 @@ func (s *Suite) Test_Pkgsrc_Relpath(c *c "x11/frameworkintegration/../../meta-pkgs/kde/kf5.mk", "meta-pkgs/kde/kf5.mk") - volume := NewPathSlash(filepath.VolumeName(t.tmpdir.String())) + volume := NewCurrPathSlash(filepath.VolumeName(t.tmpdir.String())) G.Pkgsrc.topdir = volume.JoinNoClean("usr/pkgsrc") // Taken from Test_MkLineChecker_CheckRelativePath__wip_mk @@ -1241,7 +1246,7 @@ func (s *Suite) Test_Pkgsrc_Relpath(c *c test("some/dir", ".", "../..") test("some/dir/.", ".", "../..") - chdir := func(path Path) { + chdir := func(path CurrPath) { // See Tester.Chdir; a direct Chdir works here since this test // neither loads lines nor processes them. assertNil(os.Chdir(path.String()), "Chdir %s", path) @@ -1297,7 +1302,7 @@ func (s *Suite) Test_Pkgsrc_File(c *chec G.Pkgsrc.topdir = "$pkgsrcdir" - test := func(rel, resolved Path) { + test := func(rel PkgsrcPath, resolved CurrPath) { t.CheckEquals(G.Pkgsrc.File(rel), resolved) } @@ -1337,8 +1342,8 @@ func (s *Suite) Test_Change_Target(c *ch moved := Change{loc, Moved, "category/path", "category/other", "author", "2019-01-01"} downgraded := Change{loc, Downgraded, "category/path", "1.0", "author", "2019-01-01"} - t.CheckEquals(renamed.Target(), NewPath("category/other")) - t.CheckEquals(moved.Target(), NewPath("category/other")) + t.CheckEquals(renamed.Target(), NewPkgsrcPath("category/other")) + t.CheckEquals(moved.Target(), NewPkgsrcPath("category/other")) t.ExpectAssert(func() { downgraded.Target() }) } Index: pkgsrc/pkgtools/pkglint/files/util_test.go diff -u pkgsrc/pkgtools/pkglint/files/util_test.go:1.38 pkgsrc/pkgtools/pkglint/files/util_test.go:1.39 --- pkgsrc/pkgtools/pkglint/files/util_test.go:1.38 Sat Nov 30 20:35:11 2019 +++ pkgsrc/pkgtools/pkglint/files/util_test.go Mon Dec 2 23:32:09 2019 @@ -337,56 +337,6 @@ func (s *Suite) Test__regex_ReplaceFirst t.CheckEquals(rest, "X+c+d") } -func (s *Suite) Test_cleanpath(c *check.C) { - t := s.Init(c) - - test := func(from, to Path) { - t.CheckEquals(cleanpath(from), to) - } - - test("simple/path", "simple/path") - test("/absolute/path", "/absolute/path") - - // Single dot components are removed, unless it's the only component of the path. - test("./././.", ".") - test("./././", ".") - test("dir/multi/././/file", "dir/multi/file") - test("dir/", "dir") - - test("dir/", "dir") - - // Components like aa/bb/../.. are removed, but not in the initial part of the path, - // and only if they are not followed by another "..". - test("dir/../dir/../dir/../dir/subdir/../../Makefile", "dir/../dir/../dir/../Makefile") - test("111/222/../../333/444/../../555/666/../../777/888/9", "111/222/../../777/888/9") - test("1/2/3/../../4/5/6/../../7/8/9/../../../../10", "1/2/3/../../4/7/8/9/../../../../10") - test("cat/pkg.v1/../../cat/pkg.v2/Makefile", "cat/pkg.v1/../../cat/pkg.v2/Makefile") - test("aa/../../../../../a/b/c/d", "aa/../../../../../a/b/c/d") - test("aa/bb/../../../../a/b/c/d", "aa/bb/../../../../a/b/c/d") - test("aa/bb/cc/../../../a/b/c/d", "aa/bb/cc/../../../a/b/c/d") - test("aa/bb/cc/dd/../../a/b/c/d", "aa/bb/a/b/c/d") - test("aa/bb/cc/dd/ee/../a/b/c/d", "aa/bb/cc/dd/ee/../a/b/c/d") - test("../../../../../a/b/c/d", "../../../../../a/b/c/d") - test("aa/../../../../a/b/c/d", "aa/../../../../a/b/c/d") - test("aa/bb/../../../a/b/c/d", "aa/bb/../../../a/b/c/d") - test("aa/bb/cc/../../a/b/c/d", "aa/bb/cc/../../a/b/c/d") - test("aa/bb/cc/dd/../a/b/c/d", "aa/bb/cc/dd/../a/b/c/d") - test("aa/../cc/../../a/b/c/d", "aa/../cc/../../a/b/c/d") - - // The initial 2 components of the path are typically category/package, when - // pkglint is called from the pkgsrc top-level directory. - // This path serves as the context and therefore is always kept. - test("aa/bb/../../cc/dd/../../ee/ff", "aa/bb/../../ee/ff") - test("aa/bb/../../cc/dd/../..", "aa/bb/../..") - test("aa/bb/cc/dd/../..", "aa/bb") - test("aa/bb/../../cc/dd/../../ee/ff/buildlink3.mk", "aa/bb/../../ee/ff/buildlink3.mk") - test("./aa/bb/../../cc/dd/../../ee/ff/buildlink3.mk", "aa/bb/../../ee/ff/buildlink3.mk") - - test("../.", "..") - test("../././././././.", "..") - test(".././././././././", "..") -} - const reMkIncludeBenchmark = `^\.([\t ]*)(s?include)[\t ]+\"([^\"]+)\"[\t ]*(?:#.*)?$` const reMkIncludeBenchmarkPositive = `^\.([\t ]*)(s?include)[\t ]+\"(.+)\"[\t ]*(?:#.*)?$` @@ -705,15 +655,15 @@ func (s *Suite) Test_FileCache(c *check. c.Check(cache.Get("Makefile", MustSucceed|LogErrors), check.IsNil) // Wrong LoadOptions. linesFromCache := cache.Get("Makefile", 0) - t.CheckEquals(linesFromCache.Filename, NewPath("Makefile")) + t.CheckEquals(linesFromCache.Filename, NewCurrPath("Makefile")) c.Check(linesFromCache.Lines, check.HasLen, 2) - t.CheckEquals(linesFromCache.Lines[0].Filename, NewPath("Makefile")) + t.CheckEquals(linesFromCache.Lines[0].Filename, NewCurrPath("Makefile")) // Cache keys are normalized using path.Clean. linesFromCache2 := cache.Get("./Makefile", 0) - t.CheckEquals(linesFromCache2.Filename, NewPath("./Makefile")) + t.CheckEquals(linesFromCache2.Filename, NewCurrPath("./Makefile")) c.Check(linesFromCache2.Lines, check.HasLen, 2) - t.CheckEquals(linesFromCache2.Lines[0].Filename, NewPath("./Makefile")) + t.CheckEquals(linesFromCache2.Lines[0].Filename, NewCurrPath("./Makefile")) cache.Put("file1.mk", 0, lines) cache.Put("file2.mk", 0, lines) @@ -1096,6 +1046,23 @@ func (s *Suite) Test_LazyStringBuilder_W t.CheckDeepEquals(sb.buf, []byte{'w', 'o', 'l', 'f'}) } +func (s *Suite) Test_LazyStringBuilder_writeToBuf__assertion(c *check.C) { + t := s.Init(c) + + sb := NewLazyStringBuilder("0123456789abcdef0123456789abcdef") + sb.WriteString("0123456789abcdef0123456789abcdeX") + + t.CheckEquals(cap(sb.buf), 32) + + sb.Reset("0123456789abcdef") + sb.WriteString("01234567") + + // Intentionally violate the invariant of the LazyStringBuilder that + // as long as sb.usingBuf is false, sb.len is at most len(sb.expected). + sb.expected = "" + t.ExpectAssert(func() { sb.writeToBuf('x') }) +} + func (s *Suite) Test_LazyStringBuilder_Reset(c *check.C) { t := s.Init(c) Index: pkgsrc/pkgtools/pkglint/files/distinfo_test.go diff -u pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.34 pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.35 --- pkgsrc/pkgtools/pkglint/files/distinfo_test.go:1.34 Sun Nov 17 01:26:25 2019 +++ pkgsrc/pkgtools/pkglint/files/distinfo_test.go Mon Dec 2 23:32:09 2019 @@ -116,7 +116,6 @@ func (s *Suite) Test_distinfoLinesChecke t := s.Init(c) t.SetUpPkgsrc() - t.SetUpCommandLine("-Wall,no-space") t.CreateFileLines("licenses/unknown-license") t.CreateFileLines("lang/php/ext.mk", MkCvsID, @@ -142,7 +141,7 @@ func (s *Suite) Test_distinfoLinesChecke t.CreateFileLines("archivers/php-bz2/Makefile", MkCvsID, "", - "USE_PHP_EXT_PATCHES= yes", + "USE_PHP_EXT_PATCHES=\tyes", "", ".include \"../../lang/php/ext.mk\"", ".include \"../../mk/bsd.pkg.mk\"") Index: pkgsrc/pkgtools/pkglint/files/files.go diff -u pkgsrc/pkgtools/pkglint/files/files.go:1.28 pkgsrc/pkgtools/pkglint/files/files.go:1.29 --- pkgsrc/pkgtools/pkglint/files/files.go:1.28 Sat Nov 23 23:35:56 2019 +++ pkgsrc/pkgtools/pkglint/files/files.go Mon Dec 2 23:32:09 2019 @@ -14,7 +14,7 @@ const ( LogErrors // ) -func LoadMk(filename Path, options LoadOptions) *MkLines { +func LoadMk(filename CurrPath, options LoadOptions) *MkLines { lines := Load(filename, options|Makefile) if lines == nil { return nil @@ -22,7 +22,7 @@ func LoadMk(filename Path, options LoadO return NewMkLines(lines) } -func Load(filename Path, options LoadOptions) *Lines { +func Load(filename CurrPath, options LoadOptions) *Lines { if fromCache := G.fileCache.Get(filename, options); fromCache != nil { return fromCache } @@ -59,7 +59,7 @@ func Load(filename Path, options LoadOpt return result } -func convertToLogicalLines(filename Path, rawText string, joinBackslashLines bool) *Lines { +func convertToLogicalLines(filename CurrPath, rawText string, joinBackslashLines bool) *Lines { var rawLines []*RawLine for lineno, rawLine := range strings.SplitAfter(rawText, "\n") { if rawLine != "" { @@ -89,7 +89,7 @@ func convertToLogicalLines(filename Path return NewLines(filename, loglines) } -func nextLogicalLine(filename Path, rawLines []*RawLine, index int) (*Line, int) { +func nextLogicalLine(filename CurrPath, rawLines []*RawLine, index int) (*Line, int) { { // Handle the common case efficiently rawLine := rawLines[index] textnl := rawLine.textnl Index: pkgsrc/pkgtools/pkglint/files/licenses.go diff -u pkgsrc/pkgtools/pkglint/files/licenses.go:1.28 pkgsrc/pkgtools/pkglint/files/licenses.go:1.29 --- pkgsrc/pkgtools/pkglint/files/licenses.go:1.28 Wed Nov 27 22:10:07 2019 +++ pkgsrc/pkgtools/pkglint/files/licenses.go Mon Dec 2 23:32:09 2019 @@ -24,13 +24,14 @@ func (lc *LicenseChecker) Check(value st } func (lc *LicenseChecker) checkName(license string) { - licenseFile := NewPath("") + licenseFile := NewCurrPath("") if G.Pkg != nil { if mkline := G.Pkg.vars.FirstDefinition("LICENSE_FILE"); mkline != nil { - licenseFile = G.Pkg.File(mkline.ResolveVarsInRelativePath(NewPath(mkline.Value()))) + rel := mkline.ResolveVarsInRelativePath(NewRelPathString(mkline.Value())) + licenseFile = G.Pkg.File(NewPackagePath(rel.AsPath())) } } - if licenseFile == "" { + if licenseFile.IsEmpty() { licenseFile = G.Pkgsrc.File("licenses").JoinNoClean(NewPath(license)) G.InterPackage.UseLicense(license) } Index: pkgsrc/pkgtools/pkglint/files/files_test.go diff -u pkgsrc/pkgtools/pkglint/files/files_test.go:1.29 pkgsrc/pkgtools/pkglint/files/files_test.go:1.30 --- pkgsrc/pkgtools/pkglint/files/files_test.go:1.29 Sun Nov 17 01:26:25 2019 +++ pkgsrc/pkgtools/pkglint/files/files_test.go Mon Dec 2 23:32:09 2019 @@ -77,6 +77,28 @@ func (s *Suite) Test_convertToLogicalLin t.CheckEquals(lines.Lines[1].String(), "filename:3: second line") } +// This test demonstrates that pkglint deviates from bmake. +// Bmake keeps all the trailing whitespace from the line and replaces the +// backslash plus any indentation with a single space. This results in: +// "\tprintf '%s\\n' 'none none space space tab\t tab'" +// This is another of the edge cases probably no-one relies on. +func (s *Suite) Test_convertToLogicalLines__continuation_spacing(c *check.C) { + t := s.Init(c) + + rawText := "" + + "\tprintf '%s\\n' 'none\\\n" + + "none\\\n" + + "space \\\n" + + " space \\\n" + + "tab\t\\\n" + + "\ttab'\n" + + lines := convertToLogicalLines("filename", rawText, true) + + t.CheckEquals(lines.Lines[0].Text, + "\tprintf '%s\\n' 'none none space space tab tab'") +} + func (s *Suite) Test_convertToLogicalLines__continuation_in_last_line(c *check.C) { t := s.Init(c) Index: pkgsrc/pkgtools/pkglint/files/line.go diff -u pkgsrc/pkgtools/pkglint/files/line.go:1.40 pkgsrc/pkgtools/pkglint/files/line.go:1.41 --- pkgsrc/pkgtools/pkglint/files/line.go:1.40 Wed Nov 27 22:10:07 2019 +++ pkgsrc/pkgtools/pkglint/files/line.go Mon Dec 2 23:32:09 2019 @@ -37,7 +37,7 @@ type RawLine struct { func (rline *RawLine) String() string { return sprintf("%d:%s", rline.Lineno, rline.textnl) } type Location struct { - Filename Path + Filename CurrPath firstLine int32 // zero means the whole file, -1 means EOF lastLine int32 // usually the same as firstLine, may differ in Makefiles } @@ -46,7 +46,7 @@ func (loc *Location) String() string { return loc.Filename.String() + ":" + loc.Linenos() } -func NewLocation(filename Path, firstLine, lastLine int) Location { +func NewLocation(filename CurrPath, firstLine, lastLine int) Location { return Location{filename, int32(firstLine), int32(lastLine)} } @@ -82,23 +82,23 @@ type Line struct { // XXX: Filename and Basename could be replaced with a pointer to a Lines object. } -func NewLine(filename Path, lineno int, text string, rawLine *RawLine) *Line { +func NewLine(filename CurrPath, lineno int, text string, rawLine *RawLine) *Line { assert(rawLine != nil) // Use NewLineMulti for creating a Line with no RawLine attached to it. return NewLineMulti(filename, lineno, lineno, text, []*RawLine{rawLine}) } // NewLineMulti is for logical Makefile lines that end with backslash. -func NewLineMulti(filename Path, firstLine, lastLine int, text string, rawLines []*RawLine) *Line { +func NewLineMulti(filename CurrPath, firstLine, lastLine int, text string, rawLines []*RawLine) *Line { return &Line{NewLocation(filename, firstLine, lastLine), filename.Base(), text, rawLines, nil, Once{}} } // NewLineEOF creates a dummy line for logging, with the "line number" EOF. -func NewLineEOF(filename Path) *Line { +func NewLineEOF(filename CurrPath) *Line { return NewLineMulti(filename, -1, 0, "", nil) } // NewLineWhole creates a dummy line for logging messages that affect a file as a whole. -func NewLineWhole(filename Path) *Line { +func NewLineWhole(filename CurrPath) *Line { return NewLineMulti(filename, 0, 0, "", nil) } @@ -118,8 +118,9 @@ func (line *Line) RefToLocation(other Lo // PathToFile returns the relative path from this line to the given file path. // This is typically used for arguments in diagnostics, which should always be // relative to the line with which the diagnostic is associated. -func (line *Line) PathToFile(filePath Path) Path { - return G.Pkgsrc.Relpath(line.Filename.Dir(), filePath) +func (line *Line) PathToFile(filePath CurrPath) Path { + // FIXME: consider DirNoClean + return G.Pkgsrc.Relpath(line.Filename.DirClean(), filePath) } func (line *Line) IsMultiline() bool { Index: pkgsrc/pkgtools/pkglint/files/linelexer.go diff -u pkgsrc/pkgtools/pkglint/files/linelexer.go:1.7 pkgsrc/pkgtools/pkglint/files/linelexer.go:1.8 --- pkgsrc/pkgtools/pkglint/files/linelexer.go:1.7 Sat Nov 23 23:35:56 2019 +++ pkgsrc/pkgtools/pkglint/files/linelexer.go Mon Dec 2 23:32:09 2019 @@ -92,19 +92,18 @@ func (llex *LinesLexer) SkipEmptyOrNote( return true } - if G.Opts.WarnSpace { - if llex.index == 0 { - fix := llex.CurrentLine().Autofix() - fix.Notef("Empty line expected before this line.") - fix.InsertBefore("") - fix.Apply() - } else { - fix := llex.PreviousLine().Autofix() - fix.Notef("Empty line expected after this line.") - fix.InsertAfter("") - fix.Apply() - } + if llex.index == 0 { + fix := llex.CurrentLine().Autofix() + fix.Notef("Empty line expected before this line.") + fix.InsertBefore("") + fix.Apply() + } else { + fix := llex.PreviousLine().Autofix() + fix.Notef("Empty line expected after this line.") + fix.InsertAfter("") + fix.Apply() } + return false } Index: pkgsrc/pkgtools/pkglint/files/linelexer_test.go diff -u pkgsrc/pkgtools/pkglint/files/linelexer_test.go:1.4 pkgsrc/pkgtools/pkglint/files/linelexer_test.go:1.5 --- pkgsrc/pkgtools/pkglint/files/linelexer_test.go:1.4 Sun Nov 17 01:26:25 2019 +++ pkgsrc/pkgtools/pkglint/files/linelexer_test.go Mon Dec 2 23:32:09 2019 @@ -49,3 +49,18 @@ func (s *Suite) Test_LinesLexer_SkipEmpt t.CheckOutputLines( "NOTE: file.txt:2: Empty line expected after this line.") } + +func (s *Suite) Test_MkLinesLexer_SkipIf(c *check.C) { + t := s.Init(c) + + mklines := t.NewMkLines("filename.mk", + "# comment", + "VAR=\tnot a comment") + mlex := NewMkLinesLexer(mklines) + + t.CheckEquals(mlex.SkipIf((*MkLine).IsComment), true) + t.CheckEquals(mlex.SkipIf((*MkLine).IsComment), false) + t.CheckEquals(mlex.SkipIf((*MkLine).IsVarassign), true) + t.CheckEquals(mlex.SkipIf((*MkLine).IsVarassign), false) + t.CheckEquals(mlex.EOF(), true) +} Index: pkgsrc/pkgtools/pkglint/files/mklineparser_test.go diff -u pkgsrc/pkgtools/pkglint/files/mklineparser_test.go:1.4 pkgsrc/pkgtools/pkglint/files/mklineparser_test.go:1.5 --- pkgsrc/pkgtools/pkglint/files/mklineparser_test.go:1.4 Sat Nov 23 23:35:56 2019 +++ pkgsrc/pkgtools/pkglint/files/mklineparser_test.go Mon Dec 2 23:32:09 2019 @@ -163,7 +163,6 @@ func (s *Suite) Test_MkLineParser_parseV func (s *Suite) Test_MkLineParser_parseVarassign__autofix_space_after_varname(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wspace") filename := t.CreateFileLines("Makefile", MkCvsID, "VARNAME +=\t${VARNAME}", @@ -178,9 +177,12 @@ func (s *Suite) Test_MkLineParser_parseV "NOTE: ~/Makefile:2: Unnecessary space after variable name \"VARNAME\".", // The assignment operators other than = and += cannot lead to ambiguities. - "NOTE: ~/Makefile:5: Unnecessary space after variable name \"VARNAME+\".") + "NOTE: ~/Makefile:5: Unnecessary space after variable name \"VARNAME+\".", - t.SetUpCommandLine("-Wspace", "--autofix") + "WARN: ~/Makefile:5: "+ + "Please include \"../../mk/bsd.prefs.mk\" before using \"?=\".") + + t.SetUpCommandLine("-Wall", "--autofix") CheckFileMk(filename) Index: pkgsrc/pkgtools/pkglint/files/lines.go diff -u pkgsrc/pkgtools/pkglint/files/lines.go:1.10 pkgsrc/pkgtools/pkglint/files/lines.go:1.11 --- pkgsrc/pkgtools/pkglint/files/lines.go:1.10 Sat Nov 23 23:35:56 2019 +++ pkgsrc/pkgtools/pkglint/files/lines.go Mon Dec 2 23:32:09 2019 @@ -5,12 +5,12 @@ import ( ) type Lines struct { - Filename Path + Filename CurrPath BaseName string // TODO: consider converting to Path Lines []*Line } -func NewLines(filename Path, lines []*Line) *Lines { +func NewLines(filename CurrPath, lines []*Line) *Lines { return &Lines{filename, filename.Base(), lines} } Index: pkgsrc/pkgtools/pkglint/files/logging.go diff -u pkgsrc/pkgtools/pkglint/files/logging.go:1.31 pkgsrc/pkgtools/pkglint/files/logging.go:1.32 --- pkgsrc/pkgtools/pkglint/files/logging.go:1.31 Wed Nov 27 22:10:07 2019 +++ pkgsrc/pkgtools/pkglint/files/logging.go Mon Dec 2 23:32:09 2019 @@ -57,7 +57,6 @@ var ( // that an explanation is available. func (l *Logger) Explain(explanation ...string) { if l.suppressExpl { - l.suppressExpl = false return } @@ -123,7 +122,7 @@ func (l *Logger) Diag(line *Line, level l.Logf(level, filename, linenos, format, msg) } -func (l *Logger) FirstTime(filename Path, linenos, msg string) bool { +func (l *Logger) FirstTime(filename CurrPath, linenos, msg string) bool { if l.verbose { return true } @@ -227,7 +226,7 @@ func (l *Logger) showSource(line *Line) // IsAutofix returns whether one of the --show-autofix or --autofix options is active. func (l *Logger) IsAutofix() bool { return l.Opts.Autofix || l.Opts.ShowAutofix } -func (l *Logger) Logf(level *LogLevel, filename Path, lineno, format, msg string) { +func (l *Logger) Logf(level *LogLevel, filename CurrPath, lineno, format, msg string) { if l.suppressDiag { l.suppressDiag = false return @@ -246,8 +245,8 @@ func (l *Logger) Logf(level *LogLevel, f } } - if filename != "" { - filename = cleanpath(filename) + if !filename.IsEmpty() { + filename = filename.CleanPath() } if G.Opts.Profiling && format != AutofixFormat && level != Fatal { l.histo.Add(format, 1) @@ -258,8 +257,8 @@ func (l *Logger) Logf(level *LogLevel, f out = l.err } - filenameSep := condStr(filename != "", ": ", "") - effLineno := condStr(filename != "", lineno, "") + filenameSep := condStr(!filename.IsEmpty(), ": ", "") + effLineno := condStr(!filename.IsEmpty(), lineno, "") linenoSep := condStr(effLineno != "", ":", "") var diag string if l.Opts.GccOutput { @@ -287,7 +286,7 @@ func (l *Logger) Logf(level *LogLevel, f // Location.Filename. It may be followed by the usual ":123" for line numbers. // // For diagnostics, use Logf instead. -func (l *Logger) TechErrorf(location Path, format string, args ...interface{}) { +func (l *Logger) TechErrorf(location CurrPath, format string, args ...interface{}) { msg := sprintf(format, args...) var diag string if l.Opts.GccOutput { Index: pkgsrc/pkgtools/pkglint/files/mklexer.go diff -u pkgsrc/pkgtools/pkglint/files/mklexer.go:1.2 pkgsrc/pkgtools/pkglint/files/mklexer.go:1.3 --- pkgsrc/pkgtools/pkglint/files/mklexer.go:1.2 Sat Nov 30 20:35:11 2019 +++ pkgsrc/pkgtools/pkglint/files/mklexer.go Mon Dec 2 23:32:09 2019 @@ -262,7 +262,7 @@ func (p *MkLexer) varUseModifier(varname modifier := p.varUseText(closing) // ${SOURCES:%.c=%.o} or ${:!uname -a!:[2]} - if contains(modifier, "=") || (hasPrefix(modifier, "!") && hasSuffix(modifier, "!")) { + if contains(modifier, "=") || hasPrefix(modifier, "!") && hasSuffix(modifier, "!") { return modifier } Index: pkgsrc/pkgtools/pkglint/files/mkline.go diff -u pkgsrc/pkgtools/pkglint/files/mkline.go:1.66 pkgsrc/pkgtools/pkglint/files/mkline.go:1.67 --- pkgsrc/pkgtools/pkglint/files/mkline.go:1.66 Sat Nov 30 20:35:11 2019 +++ pkgsrc/pkgtools/pkglint/files/mkline.go Mon Dec 2 23:32:09 2019 @@ -278,10 +278,11 @@ func (mkline *MkLine) MustExist() bool { func (mkline *MkLine) IncludedFile() Path { return mkline.data.(*mkLineInclude).includedFile } -// IncludedFileFull returns the path to the included file, relative to the -// current working directory. -func (mkline *MkLine) IncludedFileFull() Path { - return cleanpath(mkline.Filename.Dir().JoinClean(mkline.IncludedFile())) // FIXME: JoinNoClean? +// IncludedFileFull returns the path to the included file. +func (mkline *MkLine) IncludedFileFull() CurrPath { + // FIXME: consider DirNoClean + // FIXME: consider JoinNoClean + return mkline.Filename.DirClean().JoinClean(mkline.IncludedFile()).CleanPath() } func (mkline *MkLine) Targets() string { return mkline.data.(mkLineDependency).targets } @@ -553,16 +554,17 @@ func (*MkLine) WithoutMakeVariables(valu return valueNovar.String() } -func (mkline *MkLine) ResolveVarsInRelativePath(relativePath Path) Path { +func (mkline *MkLine) ResolveVarsInRelativePath(relativePath RelPath) RelPath { if !containsVarRef(relativePath.String()) { - return cleanpath(relativePath) + return relativePath.CleanPath() } - var basedir Path + var basedir CurrPath if G.Pkg != nil { basedir = G.Pkg.File(".") } else { - basedir = mkline.Filename.Dir() + // FIXME: consider DirNoClean + basedir = mkline.Filename.DirClean() } tmp := relativePath @@ -596,7 +598,7 @@ func (mkline *MkLine) ResolveVarsInRelat // TODO: Add test that suggests ${.PARSEDIR} in .include to be omitted. tmp = tmp.Replace("${.PARSEDIR}", ".") - replaceLatest := func(varuse string, category Path, pattern regex.Pattern, replacement string) { + replaceLatest := func(varuse string, category PkgsrcPath, pattern regex.Pattern, replacement string) { if tmp.ContainsText(varuse) { latest := G.Pkgsrc.Latest(category, pattern, replacement) tmp = tmp.Replace(varuse, latest) @@ -620,7 +622,7 @@ func (mkline *MkLine) ResolveVarsInRelat tmp = tmp.Replace("${PKGDIR}", G.Pkg.Pkgdir.String()) } - tmp = cleanpath(tmp) + tmp = tmp.CleanPath() if trace.Tracing && relativePath != tmp { trace.Stepf("resolveVarsInRelativePath: %q => %q", relativePath, tmp) @@ -1053,7 +1055,7 @@ type indentationLevel struct { // pkglint will happily accept .include "fname" in both the then and // the else branch. This is ok since the primary job of this file list // is to prevent wrong pkglint warnings about missing files. - checkedFiles []Path + checkedFiles []RelPath // whether the line is a multiple-inclusion guard guard bool @@ -1158,14 +1160,16 @@ func (ind *Indentation) Args() string { return ind.top().args } -func (ind *Indentation) AddCheckedFile(filename Path) { +func (ind *Indentation) AddCheckedFile(filename RelPath) { top := ind.top() top.checkedFiles = append(top.checkedFiles, filename) } // HasExists returns whether the given filename has been tested in an // exists(filename) condition and thus may or may not exist. -func (ind *Indentation) HasExists(filename Path) bool { +// +// FIXME: Replace RelPath with PkgsrcPath, to make the filenames reliable. +func (ind *Indentation) HasExists(filename RelPath) bool { for _, level := range ind.levels { for _, levelFilename := range level.checkedFiles { if filename == levelFilename { @@ -1240,13 +1244,13 @@ func (ind *Indentation) TrackAfter(mklin cond.Walk(&MkCondCallback{ Call: func(name string, arg string) { if name == "exists" { - ind.AddCheckedFile(NewPath(arg)) + ind.AddCheckedFile(NewRelPathString(arg)) } }}) } } -func (ind *Indentation) CheckFinish(filename Path) { +func (ind *Indentation) CheckFinish(filename CurrPath) { if ind.IsEmpty() { return } @@ -1291,7 +1295,7 @@ func MatchMkInclude(text string) (m bool // here. But since these usually don't contain double quotes, it has // worked fine up to now. filename = NewPath(lexer.NextBytesFunc(func(c byte) bool { return c != '"' })) - if filename != "" && lexer.SkipByte('"') { + if !filename.IsEmpty() && lexer.SkipByte('"') { lexer.NextHspace() if lexer.EOF() { m = true Index: pkgsrc/pkgtools/pkglint/files/mkline_test.go diff -u pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.73 pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.74 --- pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.73 Wed Nov 27 22:10:07 2019 +++ pkgsrc/pkgtools/pkglint/files/mkline_test.go Mon Dec 2 23:32:09 2019 @@ -505,7 +505,7 @@ func (s *Suite) Test_MkLine_ResolveVarsI MkCvsID) mkline := mklines.mklines[0] - test := func(before Path, after Path) { + test := func(before RelPath, after RelPath) { t.CheckEquals(mkline.ResolveVarsInRelativePath(before), after) } @@ -962,16 +962,15 @@ func (s *Suite) Test_MkLine_VariableNeed func (s *Suite) Test_MkLine_VariableNeedsQuoting__shellword_part(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wall,no-space") t.SetUpVartypes() mklines := t.SetUpFileMkLines("Makefile", MkCvsID, "", - "SUBST_CLASSES+= class", - "SUBST_STAGE.class= pre-configure", - "SUBST_FILES.class= files", - "SUBST_SED.class=-e s:@LINKER_RPATH_FLAG@:${LINKER_RPATH_FLAG}:g") + "SUBST_CLASSES+=\t\tclass", + "SUBST_STAGE.class=\tpre-configure", + "SUBST_FILES.class=\tfiles", + "SUBST_SED.class=\t-e s:@LINKER_RPATH_FLAG@:${LINKER_RPATH_FLAG}:g") mklines.Check() @@ -984,14 +983,13 @@ func (s *Suite) Test_MkLine_VariableNeed func (s *Suite) Test_MkLine_VariableNeedsQuoting__tool_in_shell_command(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wall,no-space") t.SetUpVartypes() t.SetUpTool("bash", "BASH", AtRunTime) mklines := t.SetUpFileMkLines("Makefile", MkCvsID, "", - "CONFIG_SHELL= ${BASH}") + "CONFIG_SHELL=\t${BASH}") mklines.Check() @@ -1048,17 +1046,17 @@ func (s *Suite) Test_MkLine_VariableNeed func (s *Suite) Test_MkLine_VariableNeedsQuoting__uncovered_cases(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wall,no-space", "--explain") + t.SetUpCommandLine("-Wall", "--explain") t.SetUpVartypes() mklines := t.SetUpFileMkLines("Makefile", MkCvsID, "", - "GO_SRCPATH= ${HOMEPAGE:S,https://,,}", - "LINKER_RPATH_FLAG:= ${LINKER_RPATH_FLAG:S/-rpath/& /}", - "HOMEPAGE= http://godoc.org/${GO_SRCPATH}", - "PATH:= ${PREFIX}/cross/bin:${PATH}", - "NO_SRC_ON_FTP= ${RESTRICTED}") + "GO_SRCPATH=\t\t${HOMEPAGE:S,https://,,}", + "LINKER_RPATH_FLAG:=\t${LINKER_RPATH_FLAG:S/-rpath/& /}", + "HOMEPAGE=\t\thttp://godoc.org/${GO_SRCPATH}", + "PATH:=\t\t\t${PREFIX}/cross/bin:${PATH}", + "NO_SRC_ON_FTP=\t\t${RESTRICTED}") mklines.Check() Index: pkgsrc/pkgtools/pkglint/files/mklinechecker.go diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.55 pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.56 --- pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.55 Wed Nov 27 22:10:07 2019 +++ pkgsrc/pkgtools/pkglint/files/mklinechecker.go Mon Dec 2 23:32:09 2019 @@ -1085,10 +1085,10 @@ func (ck MkLineChecker) checkVarassignRi categories := mkline.ValueFields(mkline.Value()) actual := categories[0] - expected := mkline.Filename.Dir().Dir().Base() - if expected == "." { - expected = G.Pkgsrc.ToRel(mkline.Filename).Dir().Dir().Base() - } + // FIXME: consider DirNoClean + // FIXME: consider DirNoClean + expected := G.Pkgsrc.ToRel(mkline.Filename).DirClean().DirClean().Base() + if expected == "wip" || actual == expected { return } @@ -1258,7 +1258,7 @@ func (ck MkLineChecker) checkShellComman mkline := ck.MkLine shellCommand := mkline.ShellCommand() - if G.Opts.WarnSpace && hasPrefix(mkline.Text, "\t\t") { + if hasPrefix(mkline.Text, "\t\t") { lexer := textproc.NewLexer(mkline.raw[0].textnl) tabs := lexer.NextBytesFunc(func(b byte) bool { return b == '\t' }) @@ -1305,7 +1305,7 @@ func (ck MkLineChecker) checkInclude() { if trace.Tracing { trace.Stepf("includingFile=%s includedFile=%s", mkline.Filename, includedFile) } - ck.CheckRelativePath(includedFile, mustExist) + ck.CheckRelativePath(NewRelPath(includedFile), mustExist) switch { case includedFile.HasBase("Makefile"): @@ -1342,7 +1342,9 @@ func (ck MkLineChecker) checkInclude() { case includedFile != "builtin.mk" && includedFile.HasSuffixPath("builtin.mk"): if mkline.Basename != "hacks.mk" && !mkline.HasRationale() { fix := mkline.Autofix() - fix.Errorf("%s must not be included directly. Include \"%s/buildlink3.mk\" instead.", includedFile, includedFile.Dir()) + // FIXME: Use %q instead of %s. + // FIXME: consider DirNoClean + fix.Errorf("%s must not be included directly. Include \"%s/buildlink3.mk\" instead.", includedFile, includedFile.DirClean()) fix.Replace("builtin.mk", "buildlink3.mk") fix.Apply() } @@ -1350,9 +1352,6 @@ func (ck MkLineChecker) checkInclude() { } func (ck MkLineChecker) checkDirectiveIndentation(expectedDepth int) { - if !G.Opts.WarnSpace { - return - } mkline := ck.MkLine indent := mkline.Indent() if expected := strings.Repeat(" ", expectedDepth); indent != expected { @@ -1365,7 +1364,7 @@ func (ck MkLineChecker) checkDirectiveIn // CheckRelativePath checks a relative path that leads to the directory of another package // or to a subdirectory thereof or a file within there. -func (ck MkLineChecker) CheckRelativePath(relativePath Path, mustExist bool) { +func (ck MkLineChecker) CheckRelativePath(relativePath RelPath, mustExist bool) { if trace.Tracing { defer trace.Call(relativePath, mustExist)() } @@ -1380,12 +1379,13 @@ func (ck MkLineChecker) CheckRelativePat return } - if resolvedPath.IsAbs() { + if resolvedPath.AsPath().IsAbs() { mkline.Errorf("The path %q must be relative.", resolvedPath) return } - abs := mkline.Filename.Dir().JoinNoClean(resolvedPath) + // FIXME: consider DirNoClean + abs := mkline.Filename.DirClean().JoinNoClean(resolvedPath.AsPath()) if !abs.Exists() { if mustExist && !ck.MkLines.indentation.HasExists(resolvedPath) { mkline.Errorf("Relative path %q does not exist.", resolvedPath) @@ -1428,13 +1428,13 @@ func (ck MkLineChecker) CheckRelativePat // // When used in .include directives, the relative package directories must be written // with the leading ../.. anyway, so the benefit might not be too big at all. -func (ck MkLineChecker) CheckRelativePkgdir(pkgdir Path) { +func (ck MkLineChecker) CheckRelativePkgdir(pkgdir RelPath) { if trace.Tracing { defer trace.Call(pkgdir)() } mkline := ck.MkLine - ck.CheckRelativePath(pkgdir+"/Makefile", true) + ck.CheckRelativePath(pkgdir.JoinNoClean("Makefile"), true) pkgdir = mkline.ResolveVarsInRelativePath(pkgdir) if !matches(pkgdir.String(), `^\.\./\.\./([^./][^/]*/[^./][^/]*)$`) && !containsVarRef(pkgdir.String()) { Index: pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.50 pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.51 --- pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.50 Wed Nov 27 22:10:07 2019 +++ pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go Mon Dec 2 23:32:09 2019 @@ -780,14 +780,13 @@ func (s *Suite) Test_MkLineChecker_Check t.SetUpPkgsrc() t.CreateFileLines("mk/defaults/mk.conf", "VARBASE?= /usr/pkg/var") - t.SetUpCommandLine("-Wall,no-space") t.FinishSetUp() mklines := t.SetUpFileMkLines("options.mk", MkCvsID, - "COMMENT= ${VARBASE} ${X11_TYPE}", - "PKG_FAIL_REASON+= ${VARBASE} ${X11_TYPE}", - "BUILD_DEFS+= X11_TYPE") + "COMMENT=\t\t${VARBASE} ${X11_TYPE}", + "PKG_FAIL_REASON+=\t${VARBASE} ${X11_TYPE}", + "BUILD_DEFS+=\t\tX11_TYPE") mklines.Check() @@ -1462,18 +1461,17 @@ func (s *Suite) Test_MkLineChecker_check func (s *Suite) Test_MkLineChecker_checkVarUseQuoting__mstar(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wall,no-space") t.SetUpVartypes() mklines := t.SetUpFileMkLines("options.mk", MkCvsID, - "CONFIGURE_ARGS+= CFLAGS=${CFLAGS:Q}", - "CONFIGURE_ARGS+= CFLAGS=${CFLAGS:M*:Q}", - "CONFIGURE_ARGS+= ADA_FLAGS=${ADA_FLAGS:Q}", - "CONFIGURE_ARGS+= ADA_FLAGS=${ADA_FLAGS:M*:Q}", - "CONFIGURE_ENV+= CFLAGS=${CFLAGS:Q}", - "CONFIGURE_ENV+= CFLAGS=${CFLAGS:M*:Q}", - "CONFIGURE_ENV+= ADA_FLAGS=${ADA_FLAGS:Q}", - "CONFIGURE_ENV+= ADA_FLAGS=${ADA_FLAGS:M*:Q}") + "CONFIGURE_ARGS+=\tCFLAGS=${CFLAGS:Q}", + "CONFIGURE_ARGS+=\tCFLAGS=${CFLAGS:M*:Q}", + "CONFIGURE_ARGS+=\tADA_FLAGS=${ADA_FLAGS:Q}", + "CONFIGURE_ARGS+=\tADA_FLAGS=${ADA_FLAGS:M*:Q}", + "CONFIGURE_ENV+=\t\tCFLAGS=${CFLAGS:Q}", + "CONFIGURE_ENV+=\t\tCFLAGS=${CFLAGS:M*:Q}", + "CONFIGURE_ENV+=\t\tADA_FLAGS=${ADA_FLAGS:Q}", + "CONFIGURE_ENV+=\t\tADA_FLAGS=${ADA_FLAGS:M*:Q}") mklines.Check() @@ -1486,7 +1484,6 @@ func (s *Suite) Test_MkLineChecker_check func (s *Suite) Test_MkLineChecker_checkVarUseQuoting__mstar_not_needed(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wall,no-space") pkg := t.SetUpPackage("category/package", "MAKE_FLAGS+=\tCFLAGS=${CFLAGS:M*:Q}", "MAKE_FLAGS+=\tLFLAGS=${LDFLAGS:M*:Q}") @@ -1668,11 +1665,10 @@ func (s *Suite) Test_MkLineChecker_check t.SetUpPkgsrc() - t.SetUpCommandLine("-Wall,no-space") mklines := t.SetUpFileMkLines("module.mk", MkCvsID, - "CFLAGS+= -Wl,--rpath,${PREFIX}/lib", - "PKG_FAIL_REASON+= \"Group ${GAMEGRP} doesn't exist.\"") + "CFLAGS+=\t\t-Wl,--rpath,${PREFIX}/lib", + "PKG_FAIL_REASON+=\t\"Group ${GAMEGRP} doesn't exist.\"") t.FinishSetUp() mklines.Check() @@ -1985,17 +1981,17 @@ func (s *Suite) Test_MkLineChecker_check t.SetUpPkgsrc() t.SetUpMasterSite("MASTER_SITE_GITHUB", "https://download.github.com/") - t.SetUpCommandLine("-Wall,no-space") + mklines := t.SetUpFileMkLines("module.mk", MkCvsID, - "EGDIR= ${PREFIX}/etc/rc.d", - "RPMIGNOREPATH+= ${PREFIX}/etc/rc.d", - "_TOOLS_VARNAME.sed= SED", - "DIST_SUBDIR= ${PKGNAME}", - "WRKSRC= ${PKGNAME}", - "SITES_distfile.tar.gz= ${MASTER_SITE_GITHUB:=user/}", - "MASTER_SITES= https://cdn.example.org/${PKGNAME}/", - "MASTER_SITES= https://cdn.example.org/distname-${PKGVERSION}/") + "EGDIR=\t\t\t${PREFIX}/etc/rc.d", + "RPMIGNOREPATH+=\t\t${PREFIX}/etc/rc.d", + "_TOOLS_VARNAME.sed=\tSED", + "DIST_SUBDIR=\t\t${PKGNAME}", + "WRKSRC=\t\t\t${PKGNAME}", + "SITES_distfile.tar.gz=\t${MASTER_SITE_GITHUB:=user/}", + "MASTER_SITES=\t\thttps://cdn.example.org/${PKGNAME}/", + "MASTER_SITES=\t\thttps://cdn.example.org/distname-${PKGVERSION}/") t.FinishSetUp() mklines.Check() @@ -2282,7 +2278,7 @@ func (s *Suite) Test_MkLineChecker_check func (s *Suite) Test_MkLineChecker_checkDirectiveIndentation__autofix(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("--autofix", "-Wspace") + t.SetUpCommandLine("--autofix") lines := t.SetUpFileLines("filename.mk", MkCvsID, ".if defined(A)", @@ -2435,7 +2431,7 @@ func (s *Suite) Test_MkLineChecker_Check t.CreateFileLines("other/package/Makefile") - test := func(relativePkgdir Path, diagnostics ...string) { + test := func(relativePkgdir RelPath, diagnostics ...string) { // Must be in the filesystem because of directory references. mklines := t.SetUpFileMkLines("category/package/Makefile", "# dummy") Index: pkgsrc/pkgtools/pkglint/files/shell.go diff -u pkgsrc/pkgtools/pkglint/files/shell.go:1.50 pkgsrc/pkgtools/pkglint/files/shell.go:1.51 --- pkgsrc/pkgtools/pkglint/files/shell.go:1.50 Wed Nov 27 22:10:07 2019 +++ pkgsrc/pkgtools/pkglint/files/shell.go Mon Dec 2 23:32:09 2019 @@ -57,8 +57,6 @@ func (scc *SimpleCommandChecker) checkCo break case matches(shellword, `\$\{(PKGSRCDIR|PREFIX)(:Q)?\}`): break - case scc.handleComment(): - break default: if G.Opts.WarnExtra && !scc.MkLines.indentation.DependsOn("OPSYS") { scc.mkline.Warnf("Unknown shell command %q.", shellword) @@ -145,43 +143,6 @@ func (scc *SimpleCommandChecker) handleS return false } -func (scc *SimpleCommandChecker) handleComment() bool { - if trace.Tracing { - defer trace.Call0()() - } - - shellword := scc.strcmd.Name - if trace.Tracing { - defer trace.Call1(shellword)() - } - - if !hasPrefix(shellword, "#") { - return false - } - - if !scc.mkline.IsMultiline() { - return false - } - - scc.mkline.Warnf("A shell comment does not stop at the end of line.") - scc.Explain( - "When a shell command is split into multiple lines that are", - "continued with a backslash, they will nevertheless be converted to", - "a single line before the shell sees them.", - "", - "This means that even if it looks as if the comment only spanned", - "one line in the Makefile, in fact it spans until the end of the whole", - "shell command.", - "", - "To insert a comment into shell code, you can write it like this:", - "", - "\t"+"${SHCOMMENT} \"The following command might fail; this is ok.\"", - "", - "Note that any special characters in the comment are still", - "interpreted by the shell.") - return true -} - func (scc *SimpleCommandChecker) checkRegexReplace() { if trace.Tracing { defer trace.Call0()() @@ -575,6 +536,8 @@ func NewShellLineChecker(mklines *MkLine return &ShellLineChecker{mklines, mkline, true} } +// CheckShellCommands checks for a list of shell commands, of which each one +// is terminated with a semicolon. These are used in GENERATE_PLIST. func (ck *ShellLineChecker) CheckShellCommands(shellcmds string, time ToolTime) { setE := true ck.CheckShellCommand(shellcmds, &setE, time) @@ -630,6 +593,7 @@ func (ck *ShellLineChecker) CheckShellCo } ck.CheckShellCommand(lexer.Rest(), &setE, RunTime) + ck.checkMultiLineComment() } func (ck *ShellLineChecker) checkHiddenAndSuppress(hiddenAndSuppress, rest string) { @@ -1029,6 +993,56 @@ func (ck *ShellLineChecker) checkInstall } } +func (ck *ShellLineChecker) checkMultiLineComment() { + mkline := ck.mkline + if !mkline.IsMultiline() || !contains(mkline.Text, "#") { + return + } + + for _, line := range mkline.raw[:len(mkline.raw)-1] { + text := strings.TrimSuffix(line.textnl, "\\\n") + + tokens, rest := splitIntoShellTokens(nil, text) + if rest != "" { + return + } + + for _, token := range tokens { + if hasPrefix(token, "#") { + ck.warnMultiLineComment(line) + return + } + } + } +} + +func (ck *ShellLineChecker) warnMultiLineComment(raw *RawLine) { + text := strings.TrimSuffix(raw.textnl, "\n") + line := NewLine(ck.mkline.Filename, raw.Lineno, text, raw) + + line.Warnf("The shell comment does not stop at the end of this line.") + line.Explain( + "When a shell command is spread out on multiple lines that are", + "continued with a backslash, they will nevertheless be converted to", + "a single line before the shell sees them.", + "", + "This means that even if it looks as if the comment only spanned", + "one line in the Makefile, in fact it spans until the end of the whole", + "shell command.", + "", + "To insert a comment into shell code, you can write it like this:", + "", + "\t${SHCOMMENT} \"The following command might fail; this is ok.\"", + "", + "Note that any special characters in the comment are still", + "interpreted by the shell.", + "", + "If that is not possible, you can apply the :D modifier to the", + "variable with the empty name, which is guaranteed to be undefined:", + "", + "\t${:D this is commented out}") +} + func (ck *ShellLineChecker) Errorf(format string, args ...interface{}) { ck.mkline.Errorf(format, args...) } Index: pkgsrc/pkgtools/pkglint/files/mklines.go diff -u pkgsrc/pkgtools/pkglint/files/mklines.go:1.61 pkgsrc/pkgtools/pkglint/files/mklines.go:1.62 --- pkgsrc/pkgtools/pkglint/files/mklines.go:1.61 Wed Nov 27 22:10:07 2019 +++ pkgsrc/pkgtools/pkglint/files/mklines.go Mon Dec 2 23:32:09 2019 @@ -440,7 +440,7 @@ func (mklines *MkLines) checkVarassignPl // CheckUsedBy checks that this file (a Makefile.common) has the given // relativeName in one of the "# used by" comments at the beginning of the file. -func (mklines *MkLines) CheckUsedBy(relativeName Path) { +func (mklines *MkLines) CheckUsedBy(relativeName PkgsrcPath) { lines := mklines.lines if lines.Len() < 3 { return Index: pkgsrc/pkgtools/pkglint/files/util.go diff -u pkgsrc/pkgtools/pkglint/files/util.go:1.61 pkgsrc/pkgtools/pkglint/files/util.go:1.62 --- pkgsrc/pkgtools/pkglint/files/util.go:1.61 Sat Nov 30 20:35:11 2019 +++ pkgsrc/pkgtools/pkglint/files/util.go Mon Dec 2 23:32:09 2019 @@ -121,6 +121,7 @@ func rtrimHspace(str string) string { return str[:end] } +// trimCommon returns the middle portion of the given strings that differs. func trimCommon(a, b string) (string, string) { // trim common prefix for len(a) > 0 && len(b) > 0 && a[0] == b[0] { @@ -238,7 +239,7 @@ func assertf(cond bool, format string, a } } -func isEmptyDir(filename Path) bool { +func isEmptyDir(filename CurrPath) bool { if filename.HasSuffixPath("CVS") { return true } @@ -261,7 +262,7 @@ func isEmptyDir(filename Path) bool { return true } -func getSubdirs(filename Path) []Path { +func getSubdirs(filename CurrPath) []Path { dirents, err := filename.ReadDir() if err != nil { NewLineWhole(filename).Fatalf("Cannot be read: %s", err) @@ -285,22 +286,8 @@ func isIgnoredFilename(filename string) return hasPrefix(filename, ".#") } -func dirglob(dirname Path) []Path { - infos, err := dirname.ReadDir() - if err != nil { - return nil - } - var filenames []Path - for _, info := range infos { - if !(isIgnoredFilename(info.Name())) { - filenames = append(filenames, cleanpath(dirname.JoinNoClean(NewPath(info.Name())))) - } - } - return filenames -} - // Checks whether a file is already committed to the CVS repository. -func isCommitted(filename Path) bool { +func isCommitted(filename CurrPath) bool { entries := G.loadCvsEntries(filename) _, found := entries[filename.Base()] return found @@ -311,7 +298,7 @@ func isCommitted(filename Path) bool { // // There is no corresponding test for Git (as used by pkgsrc-wip) since that // is more difficult to implement than simply reading a CVS/Entries file. -func isLocallyModified(filename Path) bool { +func isLocallyModified(filename CurrPath) bool { entries := G.loadCvsEntries(filename) entry, found := entries[filename.Base()] if !found { @@ -457,44 +444,6 @@ func mkopSubst(s string, left bool, from }) } -// Differs from path.Clean in that only "../../" is replaced, not "../". -// Also, the initial directory is always kept. -// This is to provide the package path as context in deeply nested .include chains. -func cleanpath(filename Path) Path { - parts := make([]string, 0, 5) - lex := textproc.NewLexer(filename.String()) - for lex.SkipString("./") { - } - - for !lex.EOF() { - part := lex.NextBytesFunc(func(b byte) bool { return b != '/' }) - parts = append(parts, part) - if lex.SkipByte('/') { - for lex.SkipByte('/') || lex.SkipString("./") { - } - } - } - - for len(parts) > 1 && parts[len(parts)-1] == "." { - parts = parts[:len(parts)-1] - } - - for i := 2; i+3 < len(parts); /* nothing */ { - if parts[i] != ".." && parts[i+1] != ".." && parts[i+2] == ".." && parts[i+3] == ".." { - if i+4 == len(parts) || parts[i+4] != ".." { - parts = append(parts[:i], parts[i+4:]...) - continue - } - } - i++ - } - - if len(parts) == 0 { - return "." - } - return NewPath(strings.Join(parts, "/")) -} - func containsVarRef(s string) bool { return contains(s, "${") || contains(s, "$(") } @@ -926,7 +875,7 @@ func NewFileCache(size int) *FileCache { 0} } -func (c *FileCache) Put(filename Path, options LoadOptions, lines *Lines) { +func (c *FileCache) Put(filename CurrPath, options LoadOptions, lines *Lines) { key := c.key(filename) entry := c.mapping[key] @@ -980,7 +929,7 @@ func (c *FileCache) removeOldEntries() { } } -func (c *FileCache) Get(filename Path, options LoadOptions) *Lines { +func (c *FileCache) Get(filename CurrPath, options LoadOptions) *Lines { key := c.key(filename) entry, found := c.mapping[key] if found && entry.options == options { @@ -997,7 +946,7 @@ func (c *FileCache) Get(filename Path, o return nil } -func (c *FileCache) Evict(filename Path) { +func (c *FileCache) Evict(filename CurrPath) { key := c.key(filename) entry, found := c.mapping[key] if !found { @@ -1015,7 +964,7 @@ func (c *FileCache) Evict(filename Path) } } -func (c *FileCache) key(filename Path) string { return filename.Clean().String() } +func (c *FileCache) key(filename CurrPath) string { return filename.Clean().String() } func bmakeHelp(topic string) string { return bmake("help topic=" + topic) } @@ -1265,27 +1214,27 @@ func pathMatches(pattern, s string) bool return err == nil && matched } -type PathQueue struct { - entries []Path +type CurrPathQueue struct { + entries []CurrPath } -func (q *PathQueue) PushFront(entries ...Path) { - q.entries = append(append([]Path(nil), entries...), q.entries...) +func (q *CurrPathQueue) PushFront(entries ...CurrPath) { + q.entries = append(append([]CurrPath(nil), entries...), q.entries...) } -func (q *PathQueue) Push(entries ...Path) { +func (q *CurrPathQueue) Push(entries ...CurrPath) { q.entries = append(q.entries, entries...) } -func (q *PathQueue) IsEmpty() bool { +func (q *CurrPathQueue) IsEmpty() bool { return len(q.entries) == 0 } -func (q *PathQueue) Front() Path { +func (q *CurrPathQueue) Front() CurrPath { return q.entries[0] } -func (q *PathQueue) Pop() Path { +func (q *CurrPathQueue) Pop() CurrPath { front := q.entries[0] q.entries = q.entries[1:] return front @@ -1294,7 +1243,7 @@ func (q *PathQueue) Pop() Path { // LazyStringBuilder builds a string that is most probably equal to an // already existing string. In that case, it avoids any memory allocations. type LazyStringBuilder struct { - Expected string + expected string len int usingBuf bool buf []byte @@ -1308,7 +1257,7 @@ func (b *LazyStringBuilder) Write(p []by } func NewLazyStringBuilder(expected string) LazyStringBuilder { - return LazyStringBuilder{Expected: expected} + return LazyStringBuilder{expected: expected} } func (b *LazyStringBuilder) Len() int { @@ -1316,7 +1265,7 @@ func (b *LazyStringBuilder) Len() int { } func (b *LazyStringBuilder) WriteString(s string) { - if !b.usingBuf && b.len+len(s) <= len(b.Expected) && hasPrefix(b.Expected[b.len:], s) { + if !b.usingBuf && b.len+len(s) <= len(b.expected) && hasPrefix(b.expected[b.len:], s) { b.len += len(s) return } @@ -1326,7 +1275,7 @@ func (b *LazyStringBuilder) WriteString( } func (b *LazyStringBuilder) WriteByte(c byte) { - if !b.usingBuf && b.len < len(b.Expected) && b.Expected[b.len] == c { + if !b.usingBuf && b.len < len(b.expected) && b.expected[b.len] == c { b.len++ return } @@ -1337,9 +1286,9 @@ func (b *LazyStringBuilder) writeToBuf(c if !b.usingBuf { if cap(b.buf) >= b.len { b.buf = b.buf[:b.len] - assert(copy(b.buf, b.Expected) == b.len) + assert(copy(b.buf, b.expected) == b.len) } else { - b.buf = []byte(b.Expected)[:b.len] + b.buf = []byte(b.expected)[:b.len] } b.usingBuf = true } @@ -1349,7 +1298,7 @@ func (b *LazyStringBuilder) writeToBuf(c } func (b *LazyStringBuilder) Reset(expected string) { - b.Expected = expected + b.expected = expected b.usingBuf = false b.len = 0 } @@ -1358,5 +1307,5 @@ func (b *LazyStringBuilder) String() str if b.usingBuf { return string(b.buf[:b.len]) } - return b.Expected[:b.len] + return b.expected[:b.len] } Index: pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.61 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.62 --- pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.61 Wed Nov 27 22:10:07 2019 +++ pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go Mon Dec 2 23:32:09 2019 @@ -341,6 +341,7 @@ func (s *Suite) Test_VartypeCheck_Depend vt := NewVartypeCheckTester(t, BtDependencyWithPath) t.CreateFileLines("category/package/Makefile") + t.CreateFileLines("category/package/files/dummy") t.CreateFileLines("databases/py-sqlite3/Makefile") t.CreateFileLines("devel/gettext/Makefile") t.CreateFileLines("devel/gmake/Makefile") @@ -404,6 +405,14 @@ func (s *Suite) Test_VartypeCheck_Depend "Invalid dependency pattern \"${PYPKGPREFIX}-sqlite3\".", "WARN: ~/category/package/filename.mk:22: "+ "Invalid dependency pattern \"${PYPKGPREFIX}-sqlite3\".") + + vt.Values( + "gettext-[0-9]*:files/../../../databases/py-sqlite3") + + vt.Output( + "WARN: ~/category/package/filename.mk:31: " + + "\"files/../../../databases/py-sqlite3\" is " + + "not a valid relative package directory.") } func (s *Suite) Test_VartypeCheck_DistSuffix(c *check.C) { @@ -1544,6 +1553,16 @@ func (s *Suite) Test_VartypeCheck_Tool(c "${t}\\:build") vt.OutputEmpty() + + vt.Op(opAssignAppend) + vt.Values( + "tool1:bootstrap", + "tool1:build", + "tool1:pkgsrc", + "tool1:run", + "tool1:test") + + vt.OutputEmpty() } func (s *Suite) Test_VartypeCheck_URL(c *check.C) { @@ -1771,6 +1790,24 @@ func (s *Suite) Test_VartypeCheck_YesNo( vt.Output( "WARN: filename.mk:3: PKG_DEVELOPER should be set to YES, yes, NO, or no.", "WARN: filename.mk:4: PKG_DEVELOPER should be set to YES, yes, NO, or no.") + + vt.Op(opUseMatch) + vt.Values( + "yes", + "[Yy]es", + "[Yy][Ee][Ss]", + "[yY][eE][sS]", + "[Nn]o", + "[Nn][Oo]", + "[nN][oO]") + + vt.Output( + "WARN: filename.mk:11: PKG_DEVELOPER should be matched against "+ + "\"[yY][eE][sS]\" or \"[nN][oO]\", not \"yes\".", + "WARN: filename.mk:12: PKG_DEVELOPER should be matched against "+ + "\"[yY][eE][sS]\" or \"[nN][oO]\", not \"[Yy]es\".", + "WARN: filename.mk:15: PKG_DEVELOPER should be matched against "+ + "\"[yY][eE][sS]\" or \"[nN][oO]\", not \"[Nn]o\".") } func (s *Suite) Test_VartypeCheck_YesNoIndirectly(c *check.C) { @@ -1793,7 +1830,7 @@ func (s *Suite) Test_VartypeCheck_YesNoI type VartypeCheckTester struct { tester *Tester basicType *BasicType - filename Path + filename CurrPath lineno int varname string op MkOperator @@ -1821,7 +1858,7 @@ func (vt *VartypeCheckTester) Varname(va vt.nextSection() } -func (vt *VartypeCheckTester) File(filename Path) { +func (vt *VartypeCheckTester) File(filename CurrPath) { vt.filename = filename vt.lineno = 1 } Index: pkgsrc/pkgtools/pkglint/files/mklines_test.go diff -u pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.52 pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.53 --- pkgsrc/pkgtools/pkglint/files/mklines_test.go:1.52 Sat Nov 23 23:35:56 2019 +++ pkgsrc/pkgtools/pkglint/files/mklines_test.go Mon Dec 2 23:32:09 2019 @@ -557,25 +557,24 @@ func (s *Suite) Test_MkLines_collectDocu func (s *Suite) Test_MkLines_collectVariables(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wall,no-space") t.SetUpPkgsrc() t.CreateFileLines("mk/tools/defaults.mk", "USE_TOOLS+= autoconf autoconf213") mklines := t.NewMkLines("determine-defined-variables.mk", MkCvsID, "", - "USE_TOOLS+= autoconf213 autoconf", + "USE_TOOLS+=\t\tautoconf213 autoconf", "", - "OPSYSVARS+= OSV", - "OSV.NetBSD= NetBSD-specific value", + "OPSYSVARS+=\t\tOSV", + "OSV.NetBSD=\t\tNetBSD-specific value", "", - "SUBST_CLASSES+= subst", - "SUBST_STAGE.subst= pre-configure", - "SUBST_FILES.subst= file", - "SUBST_VARS.subst= SUV", - "SUV= value for substitution", + "SUBST_CLASSES+=\t\tsubst", + "SUBST_STAGE.subst=\tpre-configure", + "SUBST_FILES.subst=\tfile", + "SUBST_VARS.subst=\tSUV", + "SUV=\t\t\tvalue for substitution", "", - "#BUILD_DEFS+= VARBASE", + "#BUILD_DEFS+=\t\tVARBASE", "", "pre-configure:", "\t${RUN} autoreconf; autoheader-2.13", @@ -594,15 +593,14 @@ func (s *Suite) Test_MkLines_collectVari func (s *Suite) Test_MkLines_collectVariables__BUILTIN_FIND_FILES_VAR(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wall,no-space") t.SetUpPackage("category/package") t.CreateFileLines("mk/buildlink3/bsd.builtin.mk", MkCvsID) mklines := t.SetUpFileMkLines("category/package/builtin.mk", MkCvsID, "", - "BUILTIN_FIND_FILES_VAR:= H_XFT2", - "BUILTIN_FIND_FILES.H_XFT2= ${X11BASE}/include/X11/Xft/Xft.h", + "BUILTIN_FIND_FILES_VAR:=\tH_XFT2", + "BUILTIN_FIND_FILES.H_XFT2=\t${X11BASE}/include/X11/Xft/Xft.h", "", ".include \"../../mk/buildlink3/bsd.builtin.mk\"", "", @@ -637,7 +635,6 @@ func (s *Suite) Test_MkLines_collectVari func (s *Suite) Test_MkLines_ForEach__conditional_variables(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wall,no-space") t.SetUpVartypes() mklines := t.NewMkLines("conditional.mk", MkCvsID, @@ -673,7 +670,6 @@ func (s *Suite) Test_MkLines_ForEach__co func (s *Suite) Test_MkLines_collectElse(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wno-space") t.SetUpVartypes() mklines := t.NewMkLines("module.mk", @@ -763,7 +759,6 @@ func (s *Suite) Test_MkLines_checkAll__u func (s *Suite) Test_MkLines_checkAll__PLIST_VARS(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wno-space") t.SetUpVartypes() t.SetUpOption("both", "") t.SetUpOption("only-added", "") @@ -773,20 +768,20 @@ func (s *Suite) Test_MkLines_checkAll__P mklines := t.SetUpFileMkLines("category/package/options.mk", MkCvsID, "", - "PKG_OPTIONS_VAR= PKG_OPTIONS.pkg", - "PKG_SUPPORTED_OPTIONS= both only-added only-defined", - "PKG_SUGGESTED_OPTIONS= # none", + "PKG_OPTIONS_VAR=\tPKG_OPTIONS.pkg", + "PKG_SUPPORTED_OPTIONS=\tboth only-added only-defined", + "PKG_SUGGESTED_OPTIONS=\t# none", "", ".include \"../../mk/bsd.options.mk\"", "", - "PLIST_VARS+= both only-added", + "PLIST_VARS+=\t\tboth only-added", "", ".if !empty(PKG_OPTIONS:Mboth)", - "PLIST.both= yes", + "PLIST.both=\t\tyes", ".endif", "", ".if !empty(PKG_OPTIONS:Monly-defined)", - "PLIST.only-defined= yes", + "PLIST.only-defined=\tyes", ".endif") mklines.Check() @@ -799,7 +794,6 @@ func (s *Suite) Test_MkLines_checkAll__P func (s *Suite) Test_MkLines_checkAll__PLIST_VARS_indirect(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wno-space") t.SetUpVartypes() t.SetUpOption("option1", "") t.SetUpOption("option2", "") @@ -807,18 +801,18 @@ func (s *Suite) Test_MkLines_checkAll__P mklines := t.SetUpFileMkLines("module.mk", MkCvsID, "", - "MY_PLIST_VARS= option1 option2", - "PLIST_VARS+= ${MY_PLIST_VARS}", + "MY_PLIST_VARS=\toption1 option2", + "PLIST_VARS+=\t${MY_PLIST_VARS}", ".for option in option3", - "PLIST_VARS+= ${option}", + "PLIST_VARS+=\t${option}", ".endfor", "", ".if 0", - "PLIST.option1= yes", + "PLIST.option1=\tyes", ".endif", "", ".if 1", - "PLIST.option2= yes", + "PLIST.option2=\tyes", ".endif") mklines.Check() @@ -835,7 +829,6 @@ func (s *Suite) Test_MkLines_checkAll__P func (s *Suite) Test_MkLines_checkAll__PLIST_VARS_indirect_2(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wno-space") t.SetUpVartypes() t.SetUpOption("a", "") t.SetUpOption("b", "") @@ -844,12 +837,12 @@ func (s *Suite) Test_MkLines_checkAll__P mklines := t.NewMkLines("module.mk", MkCvsID, "", - "PKG_SUPPORTED_OPTIONS= a b c", - "PLIST_VARS+= ${PKG_SUPPORTED_OPTIONS:S,a,,g}", + "PKG_SUPPORTED_OPTIONS=\ta b c", + "PLIST_VARS+=\t\t${PKG_SUPPORTED_OPTIONS:S,a,,g}", "", - "PLIST_VARS+= only-added", + "PLIST_VARS+=\t\tonly-added", "", - "PLIST.only-defined= yes") + "PLIST.only-defined=\tyes") mklines.Check() @@ -862,21 +855,20 @@ func (s *Suite) Test_MkLines_checkAll__P func (s *Suite) Test_MkLines_checkAll__defined_and_used_variables(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wno-space") t.SetUpVartypes() mklines := t.NewMkLines("module.mk", MkCvsID, "", ".for lang in de fr", - "PLIST_VARS+= ${lang}", + "PLIST_VARS+=\t${lang}", ".endif", "", ".for language in de fr", - "PLIST.${language}= yes", + "PLIST.${language}=\tyes", ".endif", "", - "PLIST.other= yes") + "PLIST.other=\tyes") mklines.Check() @@ -889,12 +881,11 @@ func (s *Suite) Test_MkLines_checkAll__d func (s *Suite) Test_MkLines_checkAll__hacks_mk(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wall,no-space") t.SetUpVartypes() mklines := t.NewMkLines("hacks.mk", MkCvsID, "", - "PKGNAME?= pkgbase-1.0") + "PKGNAME?=\tpkgbase-1.0") mklines.Check() @@ -1026,13 +1017,13 @@ func (s *Suite) Test_MkLines_checkAll__e "", ".for word in ${PKG_FAIL_REASON}", "CONFIGURE_ARGS+=\t--sharedir=${PREFIX}/share/kde", - "COMMENT=\t# defined", + "COMMENT=\t\t# defined", ".endfor", - "GAMES_USER?=pkggames", - "GAMES_GROUP?=pkggames", - "PLIST_SUBST+= CONDITIONAL=${CONDITIONAL}", - "CONDITIONAL=\"@comment\"", - "BUILD_DIRS=\t${WRKSRC}/../build") + "GAMES_USER?=\t\tpkggames", + "GAMES_GROUP?=\t\tpkggames", + "PLIST_SUBST+=\t\tCONDITIONAL=${CONDITIONAL}", + "CONDITIONAL=\t\t\"@comment\"", + "BUILD_DIRS=\t\t${WRKSRC}/../build") mklines.Check() @@ -1069,7 +1060,7 @@ func (s *Suite) Test_MkLines_CheckUsedBy t.SetUpCommandLine("--show-autofix") - test := func(pkgpath Path, lines []string, diagnostics []string) { + test := func(pkgpath PkgsrcPath, lines []string, diagnostics []string) { mklines := t.NewMkLines("Makefile.common", lines...) mklines.CheckUsedBy(pkgpath) @@ -1155,7 +1146,7 @@ func (s *Suite) Test_MkLines_CheckUsedBy func (s *Suite) Test_MkLines_CheckUsedBy(c *check.C) { t := s.Init(c) - test := func(pkgpath Path, lines []string, diagnostics []string) { + test := func(pkgpath PkgsrcPath, lines []string, diagnostics []string) { mklines := t.NewMkLines("Makefile.common", lines...) mklines.CheckUsedBy(pkgpath) Index: pkgsrc/pkgtools/pkglint/files/pkglint_test.go diff -u pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.52 pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.53 --- pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.52 Sat Nov 30 20:35:11 2019 +++ pkgsrc/pkgtools/pkglint/files/pkglint_test.go Mon Dec 2 23:32:09 2019 @@ -77,7 +77,6 @@ func (s *Suite) Test_Pkglint_Main__help( " extra enable some extra warnings (disabled)", " perm warn about unforeseen variable definition and use (disabled)", " quoting warn about quoting issues (disabled)", - " space warn about inconsistent use of whitespace (disabled)", "", " (Prefix a flag with \"no-\" to disable it.)") } @@ -307,7 +306,7 @@ func (s *Suite) Test_Pkglint_Main__profi // Pkglint always writes the profiling data into the current directory. // TODO: Make the location of the profiling log a mandatory parameter. - t.CheckEquals(NewPath("pkglint.pprof").IsFile(), true) + t.CheckEquals(NewCurrPath("pkglint.pprof").IsFile(), true) err := os.Remove("pkglint.pprof") c.Check(err, check.IsNil) @@ -494,7 +493,7 @@ func (s *Suite) Test_Pkglint_Check(c *ch func (s *Suite) Test_Pkglint_Check__invalid_files_before_import(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Call", "-Wall,no-space", "--import") + t.SetUpCommandLine("-Call", "-Wall", "--import") pkg := t.SetUpPackage("category/package") t.CreateFileLines("category/package/work/log") t.CreateFileLines("category/package/Makefile~") @@ -640,7 +639,6 @@ func (s *Suite) Test_Pkglint_checkdirPac func (s *Suite) Test_Pkglint_checkdirPackage__ALTERNATIVES(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wall,no-space") pkg := t.SetUpPackage("category/package") t.CreateFileLines("category/package/ALTERNATIVES", "bin/wrapper bin/wrapper-impl") @@ -929,7 +927,7 @@ func (s *Suite) Test_Pkglint_checkReg__i func (s *Suite) Test_Pkglint_checkReg__other(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Call", "-Wall,no-space") + t.SetUpCommandLine("-Call", "-Wall") pkg := t.SetUpPackage("category/package") t.CreateFileLines("category/package/INSTALL", "#! /bin/sh") @@ -953,10 +951,10 @@ func (s *Suite) Test_Pkglint_checkReg__r t.CreateFileDummyPatch("category/package/patches/patch-README") t.CreateFileLines("category/package/Makefile", MkCvsID, - "CATEGORIES=category", + "CATEGORIES=\tcategory", "", - "COMMENT=Comment", - "LICENSE=2-clause-bsd") + "COMMENT=\tComment", + "LICENSE=\t2-clause-bsd") t.CreateFileLines("category/package/PLIST", PlistCvsID, "bin/program") Index: pkgsrc/pkgtools/pkglint/files/mkshparser_test.go diff -u pkgsrc/pkgtools/pkglint/files/mkshparser_test.go:1.21 pkgsrc/pkgtools/pkglint/files/mkshparser_test.go:1.22 --- pkgsrc/pkgtools/pkglint/files/mkshparser_test.go:1.21 Wed Nov 27 22:10:07 2019 +++ pkgsrc/pkgtools/pkglint/files/mkshparser_test.go Mon Dec 2 23:32:09 2019 @@ -713,7 +713,7 @@ func (s *ShSuite) Test_parseShellProgram func (s *ShSuite) init(c *check.C) *MkShBuilder { s.c = c - tmpdir := NewPath("The ShSuite tests don't need a temporary directory") + tmpdir := NewCurrPath("The ShSuite tests don't need a temporary directory") s.t = &Tester{c: c, testName: c.TestName(), tmpdir: tmpdir} G = NewPkglint(&s.t.stdout, &s.t.stderr) return NewMkShBuilder() Index: pkgsrc/pkgtools/pkglint/files/shtokenizer.go diff -u pkgsrc/pkgtools/pkglint/files/shtokenizer.go:1.21 pkgsrc/pkgtools/pkglint/files/shtokenizer.go:1.22 --- pkgsrc/pkgtools/pkglint/files/shtokenizer.go:1.21 Wed Nov 27 22:10:07 2019 +++ pkgsrc/pkgtools/pkglint/files/shtokenizer.go Mon Dec 2 23:32:09 2019 @@ -4,6 +4,7 @@ import "netbsd.org/pkglint/textproc" type ShTokenizer struct { parser *MkLexer + inWord bool } func NewShTokenizer(line *Line, text string, emitWarnings bool) *ShTokenizer { @@ -13,7 +14,7 @@ func NewShTokenizer(line *Line, text str emitWarnings = true } mklex := NewMkLexer(text, line) - return &ShTokenizer{mklex} + return &ShTokenizer{mklex, false} } // ShAtom parses a basic building block of a shell program. @@ -81,6 +82,8 @@ func (p *ShTokenizer) shAtomPlain() *ShA if op := p.shOperator(q); op != nil { return op } + inWord := p.inWord + p.inWord = false lexer := p.parser.lexer mark := lexer.Mark() switch { @@ -92,7 +95,7 @@ func (p *ShTokenizer) shAtomPlain() *ShA return &ShAtom{shtText, lexer.Since(mark), shqSquot, nil} case lexer.SkipByte('`'): return &ShAtom{shtText, lexer.Since(mark), shqBackt, nil} - case lexer.PeekByte() == '#': + case lexer.PeekByte() == '#' && !inWord: rest := lexer.Rest() lexer.Skip(len(rest)) return &ShAtom{shtComment, rest, q, nil} @@ -293,6 +296,7 @@ func (p *ShTokenizer) shAtomDquotBacktSq // ${var:=default} func (p *ShTokenizer) shAtomInternal(q ShQuoting, dquot, squot bool) *ShAtom { if shVarUse := p.shVarUse(q); shVarUse != nil { + p.inWord = true return shVarUse } @@ -332,6 +336,7 @@ loop: } if token := lexer.Since(mark); token != "" { + p.inWord = true return &ShAtom{shtText, token, q, nil} } return nil Index: pkgsrc/pkgtools/pkglint/files/options_test.go diff -u pkgsrc/pkgtools/pkglint/files/options_test.go:1.19 pkgsrc/pkgtools/pkglint/files/options_test.go:1.20 --- pkgsrc/pkgtools/pkglint/files/options_test.go:1.19 Sun Nov 17 01:26:25 2019 +++ pkgsrc/pkgtools/pkglint/files/options_test.go Mon Dec 2 23:32:09 2019 @@ -252,7 +252,6 @@ func (s *Suite) Test_CheckLinesOptionsMk func (s *Suite) Test_CheckLinesOptionsMk(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wall,no-space") t.SetUpVartypes() t.SetUpOption("mc-charset", "") t.SetUpOption("mysql", "") @@ -268,17 +267,17 @@ func (s *Suite) Test_CheckLinesOptionsMk mklines := t.SetUpFileMkLines("category/package/options.mk", MkCvsID, "", - "PKG_OPTIONS_VAR= PKG_OPTIONS.mc", - "PKG_OPTIONS_REQUIRED_GROUPS= screen", - "PKG_OPTIONS_GROUP.screen= ncurses slang", - "PKG_SUPPORTED_OPTIONS= mc-charset x11 lang-${l} negative", - "PKG_SUGGESTED_OPTIONS= mc-charset slang", - "PKG_OPTIONS_NONEMPTY_SETS+= db", - "PKG_OPTIONS_SET.db= mysql sqlite", + "PKG_OPTIONS_VAR=\t\tPKG_OPTIONS.mc", + "PKG_OPTIONS_REQUIRED_GROUPS=\tscreen", + "PKG_OPTIONS_GROUP.screen=\tncurses slang", + "PKG_SUPPORTED_OPTIONS=\t\tmc-charset x11 lang-${l} negative", + "PKG_SUGGESTED_OPTIONS=\t\tmc-charset slang", + "PKG_OPTIONS_NONEMPTY_SETS+=\tdb", + "PKG_OPTIONS_SET.db=\t\tmysql sqlite", "", ".include \"../../mk/bsd.options.mk\"", "", - "PKGNAME?= default-pkgname-1.", + "PKGNAME?=\tdefault-pkgname-1.", "", ".if !empty(PKG_OPTIONS:Mx11)", ".endif", @@ -323,7 +322,6 @@ func (s *Suite) Test_CheckLinesOptionsMk func (s *Suite) Test_CheckLinesOptionsMk__unexpected_line(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wno-space") t.SetUpVartypes() t.CreateFileLines("mk/bsd.options.mk", @@ -332,7 +330,7 @@ func (s *Suite) Test_CheckLinesOptionsMk mklines := t.SetUpFileMkLines("category/package/options.mk", MkCvsID, "", - "PKG_OPTIONS_VAR= PKG_OPTIONS.mc", + "PKG_OPTIONS_VAR=\tPKG_OPTIONS.mc", "", "pre-configure:", "\techo \"In the pre-configure stage.\"") @@ -340,14 +338,15 @@ func (s *Suite) Test_CheckLinesOptionsMk CheckLinesOptionsMk(mklines) t.CheckOutputLines( - "ERROR: ~/category/package/options.mk: " + + "WARN: ~/category/package/options.mk:6: "+ + "Unknown shell command \"echo\".", + "ERROR: ~/category/package/options.mk: "+ "Each options.mk file must .include \"../../mk/bsd.options.mk\".") } func (s *Suite) Test_CheckLinesOptionsMk__malformed_condition(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wno-space") t.SetUpVartypes() t.SetUpOption("mc-charset", "") t.SetUpOption("ncurses", "") @@ -360,9 +359,9 @@ func (s *Suite) Test_CheckLinesOptionsMk mklines := t.SetUpFileMkLines("category/package/options.mk", MkCvsID, "", - "PKG_OPTIONS_VAR= PKG_OPTIONS.mc", - "PKG_SUPPORTED_OPTIONS= # none", - "PKG_SUGGESTED_OPTIONS= # none", + "PKG_OPTIONS_VAR=\t\tPKG_OPTIONS.mc", + "PKG_SUPPORTED_OPTIONS=\t\t# none", + "PKG_SUGGESTED_OPTIONS=\t\t# none", "", "# Comments and conditionals are allowed at this point.", ".if ${OPSYS} == NetBSD", Index: pkgsrc/pkgtools/pkglint/files/package.go diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.71 pkgsrc/pkgtools/pkglint/files/package.go:1.72 --- pkgsrc/pkgtools/pkglint/files/package.go:1.71 Sat Nov 30 20:35:11 2019 +++ pkgsrc/pkgtools/pkglint/files/package.go Mon Dec 2 23:32:09 2019 @@ -16,12 +16,12 @@ const rePkgname = `^([\w\-.+]+)-(\d[.0-9 // This is necessary because variables in Makefiles may be used before they are defined, // and such dependencies often span multiple files that are included indirectly. type Package struct { - dir Path // The directory of the package, for resolving files - Pkgpath Path // e.g. "category/pkgdir" - Pkgdir Path // PKGDIR from the package Makefile - Filesdir Path // FILESDIR from the package Makefile - Patchdir Path // PATCHDIR from the package Makefile - DistinfoFile Path // DISTINFO_FILE from the package Makefile + dir CurrPath // The directory of the package, for resolving files + Pkgpath PkgsrcPath // e.g. "category/pkgdir" + Pkgdir PackagePath // PKGDIR from the package Makefile + Filesdir PackagePath // FILESDIR from the package Makefile + Patchdir PackagePath // PATCHDIR from the package Makefile + DistinfoFile PackagePath // DISTINFO_FILE from the package Makefile EffectivePkgname string // PKGNAME or DISTNAME from the package Makefile, including nb13, can be empty EffectivePkgbase string // EffectivePkgname without the version EffectivePkgversion string // The version part of the effective PKGNAME, excluding nb13 @@ -59,7 +59,7 @@ type Package struct { Once Once } -func NewPackage(dir Path) *Package { +func NewPackage(dir CurrPath) *Package { pkgpath := G.Pkgsrc.ToRel(dir) // Package directory must be two subdirectories below the pkgsrc root. @@ -99,7 +99,7 @@ func NewPackage(dir Path) *Package { return &pkg } -func (pkg *Package) load() ([]Path, *MkLines, *MkLines) { +func (pkg *Package) load() ([]CurrPath, *MkLines, *MkLines) { // Load the package Makefile and all included files, // to collect all used and defined variables and similar data. mklines, allLines := pkg.loadPackageMakefile() @@ -107,26 +107,27 @@ func (pkg *Package) load() ([]Path, *MkL return nil, nil, nil } - files := dirglob(pkg.File(".")) + files := pkg.File(".").ReadPaths() if pkg.Pkgdir != "." { - files = append(files, dirglob(pkg.File(pkg.Pkgdir))...) + files = append(files, pkg.File(pkg.Pkgdir).ReadPaths()...) } - files = append(files, dirglob(pkg.File(pkg.Patchdir))...) - if pkg.DistinfoFile != NewPath(pkg.vars.fallback["DISTINFO_FILE"]) { + files = append(files, pkg.File(pkg.Patchdir).ReadPaths()...) + if pkg.DistinfoFile != NewPackagePath(NewPath(pkg.vars.fallback["DISTINFO_FILE"])) { files = append(files, pkg.File(pkg.DistinfoFile)) } - isRelevantMk := func(filename Path, basename string) bool { + isRelevantMk := func(filename CurrPath, basename string) bool { if !hasPrefix(basename, "Makefile.") && !filename.HasSuffixText(".mk") { return false } - if filename.Dir().Base() == "patches" { + // FIXME: consider DirNoClean + if filename.DirClean().Base() == "patches" { return false } if pkg.Pkgdir == "." { return true } - return !filename.ContainsPath(pkg.Pkgdir) + return !filename.ContainsPath(pkg.Pkgdir.AsPath()) } // Determine the used variables and PLIST directories before checking any of the Makefile fragments. @@ -181,10 +182,10 @@ func (pkg *Package) loadPackageMakefile( allLines.collectUsedVariables() - pkg.Pkgdir = NewPath(pkg.vars.LastValue("PKGDIR")) - pkg.DistinfoFile = NewPath(pkg.vars.LastValue("DISTINFO_FILE")) - pkg.Filesdir = NewPath(pkg.vars.LastValue("FILESDIR")) - pkg.Patchdir = NewPath(pkg.vars.LastValue("PATCHDIR")) + pkg.Pkgdir = NewPackagePath(NewPath(pkg.vars.LastValue("PKGDIR"))) + pkg.DistinfoFile = NewPackagePath(NewPath(pkg.vars.LastValue("DISTINFO_FILE"))) + pkg.Filesdir = NewPackagePath(NewPath(pkg.vars.LastValue("FILESDIR"))) + pkg.Patchdir = NewPackagePath(NewPath(pkg.vars.LastValue("PATCHDIR"))) // See lang/php/ext.mk if pkg.vars.IsDefinedSimilar("PHPEXT_MK") { @@ -212,7 +213,7 @@ func (pkg *Package) loadPackageMakefile( } // TODO: What is allLines used for, is it still necessary? Would it be better as a field in Package? -func (pkg *Package) parse(mklines *MkLines, allLines *MkLines, includingFileForUsedCheck Path) bool { +func (pkg *Package) parse(mklines *MkLines, allLines *MkLines, includingFileForUsedCheck CurrPath) bool { if trace.Tracing { defer trace.Call(mklines.lines.Filename)() } @@ -229,7 +230,8 @@ func (pkg *Package) parse(mklines *MkLin // automatically since the pkgsrc infrastructure does the same. filename := mklines.lines.Filename if filename.Base() == "buildlink3.mk" { - builtin := cleanpath(filename.Dir().JoinNoClean("builtin.mk")) + // FIXME: consider DirNoClean + builtin := filename.DirClean().JoinNoClean("builtin.mk").CleanPath() builtinRel := G.Pkgsrc.Relpath(pkg.dir, builtin) if pkg.included.FirstTime(builtinRel.String()) && builtin.IsFile() { builtinMkLines := LoadMk(builtin, MustSucceed|LogErrors) @@ -250,16 +252,16 @@ func (pkg *Package) parseLine(mklines *M includedMkLines, skip := pkg.loadIncluded(mkline, includingFile) if includedMkLines == nil { - if skip || mklines.indentation.HasExists(includedFile) { + if skip || mklines.indentation.HasExists(NewRelPath(includedFile)) { return true // See https://github.com/rillig/pkglint/issues/1 } mkline.Errorf("Cannot read %q.", includedFile) return false } - filenameForUsedCheck := NewPath("") + filenameForUsedCheck := NewCurrPath("") dir, base := includedFile.Split() - if dir != "" && base == "Makefile.common" && dir != "../../"+pkg.Pkgpath+"/" { + if dir != "" && base == "Makefile.common" && dir.String() != "../../"+pkg.Pkgpath.String()+"/" { filenameForUsedCheck = includingFile } if !pkg.parse(includedMkLines, allLines, filenameForUsedCheck) { @@ -287,15 +289,17 @@ func (pkg *Package) parseLine(mklines *M // the included file is not processed further for whatever reason. But if // skip is false, the file could not be read and an appropriate error message // has already been logged. -func (pkg *Package) loadIncluded(mkline *MkLine, includingFile Path) (includedMklines *MkLines, skip bool) { +func (pkg *Package) loadIncluded(mkline *MkLine, includingFile CurrPath) (includedMklines *MkLines, skip bool) { includedFile := pkg.resolveIncludedFile(mkline, includingFile) - if includedFile == "" { + if includedFile.IsEmpty() { return nil, true } - dirname, _ := includingFile.Split() // TODO: .Dir? - dirname = cleanpath(dirname) + // TODO: .Dir? Add test before changing this. + // pkglint -Wall x11/kde-runtime4 + dirname, _ := includingFile.Split() + dirname = dirname.CleanPath() fullIncluded := dirname.JoinNoClean(includedFile) relIncludedFile := G.Pkgsrc.Relpath(pkg.dir, fullIncluded) @@ -355,11 +359,11 @@ func (pkg *Package) loadIncluded(mkline // resolveIncludedFile resolves Makefile variables such as ${PKGPATH} to // their actual values. -func (pkg *Package) resolveIncludedFile(mkline *MkLine, includingFilename Path) Path { +func (pkg *Package) resolveIncludedFile(mkline *MkLine, includingFilename CurrPath) Path { - // TODO: resolveVariableRefs uses G.Pkg implicitly. It should be made explicit. + // FIXME: resolveVariableRefs uses G.Pkg implicitly. It should be made explicit. // TODO: Try to combine resolveVariableRefs and ResolveVarsInRelativePath. - resolved := mkline.ResolveVarsInRelativePath(mkline.IncludedFile()) + resolved := mkline.ResolveVarsInRelativePath(NewRelPath(mkline.IncludedFile())) includedText := resolveVariableRefs(nil /* XXX: or maybe some mklines? */, resolved.String()) includedFile := NewPath(includedText) if containsVarRef(includedText) { @@ -386,7 +390,7 @@ func (pkg *Package) resolveIncludedFile( // // The includingFile is relative to the current working directory, // the includedFile is taken directly from the .include directive. -func (*Package) shouldDiveInto(includingFile, includedFile Path) bool { +func (*Package) shouldDiveInto(includingFile CurrPath, includedFile Path) bool { if includedFile.HasSuffixPath("bsd.pkg.mk") || IsPrefs(includedFile) { return false @@ -436,7 +440,7 @@ func (pkg *Package) collectConditionalIn }) } -func (pkg *Package) loadPlistDirs(plistFilename Path) { +func (pkg *Package) loadPlistDirs(plistFilename CurrPath) { lines := Load(plistFilename, MustSucceed) ck := PlistChecker{ pkg, @@ -455,7 +459,7 @@ func (pkg *Package) loadPlistDirs(plistF } } -func (pkg *Package) check(filenames []Path, mklines, allLines *MkLines) { +func (pkg *Package) check(filenames []CurrPath, mklines, allLines *MkLines) { haveDistinfo := false havePatches := false @@ -505,7 +509,7 @@ func (pkg *Package) check(filenames []Pa } } -func (pkg *Package) checkfilePackageMakefile(filename Path, mklines *MkLines, allLines *MkLines) { +func (pkg *Package) checkfilePackageMakefile(filename CurrPath, mklines *MkLines, allLines *MkLines) { if trace.Tracing { defer trace.Call(filename)() } @@ -607,7 +611,7 @@ func (pkg *Package) checkPlist() { needsPlist, line := pkg.needsPlist() hasPlist := pkg.File(pkg.Pkgdir.JoinNoClean("PLIST")).IsFile() || - pkg.File(pkg.Pkgdir.JoinNoClean("/PLIST.common")).IsFile() + pkg.File(pkg.Pkgdir.JoinNoClean("PLIST.common")).IsFile() if needsPlist && !hasPlist { line.Warnf("This package should have a PLIST file.") @@ -731,7 +735,7 @@ func (pkg *Package) CheckVarorder(mkline {"TOOL_DEPENDS", many}, {"DEPENDS", many}} - relevantLines := (func() []*MkLine { + relevantLines := func() []*MkLine { firstRelevant := -1 lastRelevant := -1 @@ -779,7 +783,7 @@ func (pkg *Package) CheckVarorder(mkline return nil } return mklines.mklines[firstRelevant : lastRelevant+1] - })() + }() // If there are foreign variables, skip the whole check. // The check is only intended for the most simple packages. @@ -1199,7 +1203,7 @@ func (pkg *Package) checkUpdate() { // checkDirent checks a directory entry based on its filename and its mode // (regular file, directory, symlink). -func (pkg *Package) checkDirent(dirent Path, mode os.FileMode) { +func (pkg *Package) checkDirent(dirent CurrPath, mode os.FileMode) { // TODO: merge duplicate code in Pkglint.checkMode basename := dirent.Base() @@ -1219,7 +1223,8 @@ func (pkg *Package) checkDirent(dirent P switch { case basename == "files", basename == "patches", - dirent.Dir().Base() == "files", + // FIXME: consider DirNoClean + dirent.DirClean().Base() == "files", isEmptyDir(dirent): break @@ -1241,7 +1246,7 @@ func (pkg *Package) checkDirent(dirent P // // Pkglint assumes that the local username is the same as the NetBSD // username, which fits most scenarios. -func (pkg *Package) checkOwnerMaintainer(filename Path) { +func (pkg *Package) checkOwnerMaintainer(filename CurrPath) { if trace.Tracing { defer trace.Call(filename)() } @@ -1283,7 +1288,7 @@ func (pkg *Package) checkOwnerMaintainer "keyword \"maintainer\", for more information.") } -func (pkg *Package) checkFreeze(filename Path) { +func (pkg *Package) checkFreeze(filename CurrPath) { freezeStart := G.Pkgsrc.LastFreezeStart if freezeStart == "" || G.Pkgsrc.LastFreezeEnd != "" { return @@ -1300,7 +1305,7 @@ func (pkg *Package) checkFreeze(filename "See https://www.NetBSD.org/developers/pkgsrc/ for the exact rules.") } -func (pkg *Package) checkFileMakefileExt(filename Path) { +func (pkg *Package) checkFileMakefileExt(filename CurrPath) { base := filename.Base() if !hasPrefix(base, "Makefile.") || base == "Makefile.common" { return @@ -1392,7 +1397,7 @@ func (pkg *Package) checkIncludeConditio mkline.Warnf( "%q is included conditionally here%s "+ "and unconditionally in %s.", - cleanpath(mkline.IncludedFile()), + mkline.IncludedFile().CleanPath(), dependingOn(mkline.ConditionalVars()), mkline.RefTo(other)) @@ -1408,7 +1413,7 @@ func (pkg *Package) checkIncludeConditio mkline.Warnf( "%q is included unconditionally here "+ "and conditionally in %s%s.", - cleanpath(mkline.IncludedFile()), + mkline.IncludedFile().CleanPath(), mkline.RefTo(other), dependingOn(other.ConditionalVars())) @@ -1438,10 +1443,10 @@ func (pkg *Package) AutofixDistinfo(oldS // File returns the (possibly absolute) path to relativeFileName, // as resolved from the package's directory. // Variables that are known in the package are resolved, e.g. ${PKGDIR}. -func (pkg *Package) File(relativeFileName Path) Path { - joined := pkg.dir.JoinNoClean(relativeFileName) +func (pkg *Package) File(relativeFileName PackagePath) CurrPath { + joined := pkg.dir.JoinNoClean(relativeFileName.AsPath()) resolved := resolveVariableRefs(nil /* XXX: or maybe some mklines? */, joined.String()) - return cleanpath(NewPath(resolved)) + return NewCurrPathString(resolved).CleanPath() } // Rel returns the path by which the given filename (as seen from the @@ -1450,7 +1455,7 @@ func (pkg *Package) File(relativeFileNam // // Example: // NewPackage("category/package").Rel("other/package") == "../../other/package" -func (pkg *Package) Rel(filename Path) Path { +func (pkg *Package) Rel(filename CurrPath) Path { return G.Pkgsrc.Relpath(pkg.dir, filename) } Index: pkgsrc/pkgtools/pkglint/files/package_test.go diff -u pkgsrc/pkgtools/pkglint/files/package_test.go:1.60 pkgsrc/pkgtools/pkglint/files/package_test.go:1.61 --- pkgsrc/pkgtools/pkglint/files/package_test.go:1.60 Wed Nov 27 22:10:07 2019 +++ pkgsrc/pkgtools/pkglint/files/package_test.go Mon Dec 2 23:32:09 2019 @@ -24,20 +24,20 @@ func (s *Suite) Test_Package__varuse_at_ t.CreateFileLines("category/pkgbase/Makefile", MkCvsID, "", - "PKGNAME= loadtime-vartest-1.0", - "CATEGORIES= category", + "PKGNAME=\tloadtime-vartest-1.0", + "CATEGORIES=\tcategory", + "", + "COMMENT=\tDemonstrate variable values during parsing", + "LICENSE=\t2-clause-bsd", "", - "COMMENT= Demonstrate variable values during parsing", - "LICENSE= 2-clause-bsd", + "PLIST_SRC=\t# none", + "NO_CHECKSUM=\tyes", + "NO_CONFIGURE=\tyes", "", - "PLIST_SRC= # none", - "NO_CHECKSUM= yes", - "NO_CONFIGURE= yes", - "", - "USE_TOOLS+= echo false", - "FALSE_BEFORE!= echo false=${FALSE:Q}", // false= - "NICE_BEFORE!= echo nice=${NICE:Q}", // nice= - "TRUE_BEFORE!= echo true=${TRUE:Q}", // true= + "USE_TOOLS+=\techo false", + "FALSE_BEFORE!=\techo false=${FALSE:Q}", // false= + "NICE_BEFORE!=\techo nice=${NICE:Q}", // nice= + "TRUE_BEFORE!=\techo true=${TRUE:Q}", // true= // // All three variables above are empty since the tool // variables are initialized by bsd.prefs.mk. The variables @@ -55,7 +55,7 @@ func (s *Suite) Test_Package__varuse_at_ // run to set up the correct PATH. // "", - "USE_TOOLS+= nice", + "USE_TOOLS+=\tnice", // // The "nice" tool will only be available as ${NICE} after bsd.pkg.mk // has been included. Even including bsd.prefs.mk another time does @@ -64,9 +64,9 @@ func (s *Suite) Test_Package__varuse_at_ "", ".include \"../../mk/bsd.prefs.mk\"", // Has no effect. "", - "FALSE_AFTER!= echo false=${FALSE:Q}", // false=false - "NICE_AFTER!= echo nice=${NICE:Q}", // nice= - "TRUE_AFTER!= echo true=${TRUE:Q}", // true=true + "FALSE_AFTER!=\techo false=${FALSE:Q}", // false=false + "NICE_AFTER!=\techo nice=${NICE:Q}", // nice= + "TRUE_AFTER!=\techo true=${TRUE:Q}", // true=true "", "do-build:", "\t${RUN} printf 'before: %-20s %-20s %-20s\\n' ${FALSE_BEFORE} ${NICE_BEFORE} ${TRUE_BEFORE}", @@ -75,7 +75,7 @@ func (s *Suite) Test_Package__varuse_at_ "", ".include \"../../mk/bsd.pkg.mk\"") - t.SetUpCommandLine("-q", "-Wall,no-space") + t.SetUpCommandLine("-q", "-Wall") t.FinishSetUp() G.Check(t.File("category/pkgbase")) @@ -273,7 +273,6 @@ func (s *Suite) Test_Package__redundant_ func (s *Suite) Test_Package__distinfo_from_other_package(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wall,no-space") t.SetUpPkgsrc() t.Chdir(".") t.CreateFileLines("x11/gst-x11/Makefile", @@ -496,7 +495,7 @@ func (s *Suite) Test_Package_loadPackage t.CreateFileLines("category/package/Makefile", MkCvsID, "", - "CATEGORIES=category", + "CATEGORIES=\tcategory", "", "COMMENT=\tComment", "LICENSE=\t2-clause-bsd") @@ -510,7 +509,7 @@ func (s *Suite) Test_Package_loadPackage "Whole Makefile (with all included files) follows:", "~/category/package/Makefile:1: "+MkCvsID, "~/category/package/Makefile:2: ", - "~/category/package/Makefile:3: CATEGORIES=category", + "~/category/package/Makefile:3: CATEGORIES=\tcategory", "~/category/package/Makefile:4: ", "~/category/package/Makefile:5: COMMENT=\tComment", "~/category/package/Makefile:6: LICENSE=\t2-clause-bsd") @@ -827,7 +826,6 @@ func (s *Suite) Test_Package_parse__none func (s *Suite) Test_Package_parse__skipping(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wall,no-space") pkg := t.SetUpPackage("category/package", ".include \"${MYSQL_PKGSRCDIR:S/-client$/-server/}/buildlink3.mk\"") t.FinishSetUp() @@ -845,7 +843,7 @@ func (s *Suite) Test_Package_parse__skip output := t.Output() var relevant []string for _, line := range strings.Split(output, "\n") { - if contains(line, "Skipping") { + if contains(line, "Skipping unresolvable") { relevant = append(relevant, line) } } @@ -1011,6 +1009,33 @@ func (s *Suite) Test_Package_parse__fall "The path to the included file should be \"pthread.builtin.mk\".") } +func (s *Suite) Test_Package_loadIncluded__nested_inclusion(c *check.C) { + t := s.Init(c) + + t.SetUpPackage("x11/kde-runtime4", + ".include \"../../x11/kde-libs4/buildlink3.mk\"") + t.SetUpPackage("x11/kde-libs4") + t.CreateFileDummyBuildlink3("x11/kde-libs4/buildlink3.mk", + ".include \"../../databases/openldap/buildlink3.mk\"") + t.SetUpPackage("databases/openldap") + t.CreateFileDummyBuildlink3("databases/openldap/buildlink3.mk", + "VAR=\tvalue", + "VAR=\tvalue") // Provoke a warning in this file. + t.FinishSetUp() + // Without this line, the current directory is an absolute directory, + // and the pkgsrc top directory is as well. One of them prevents the + // verbose paths from being generated. + t.Chdir(".") + + G.Check("x11/kde-runtime4") + + // The first part of the path must be "x11/kde-runtime4" to easily + // identify the package by which all other files are included. + t.CheckOutputLines( + "NOTE: x11/kde-runtime4/../../databases/openldap/buildlink3.mk:13: " + + "Definition of VAR is redundant because of line 12.") +} + // Just for code coverage. func (s *Suite) Test_Package_resolveIncludedFile__no_tracing(c *check.C) { t := s.Init(c) @@ -1065,7 +1090,7 @@ func (s *Suite) Test_Package_shouldDiveI t := s.Init(c) t.Chdir("category/package") - test := func(including, included Path, expected bool) { + test := func(including CurrPath, included Path, expected bool) { actual := (*Package)(nil).shouldDiveInto(including, included) t.CheckEquals(actual, expected) } @@ -1232,7 +1257,7 @@ func (s *Suite) Test_Package_check__patc func (s *Suite) Test_Package_checkfilePackageMakefile__GNU_CONFIGURE(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wall,no-space") + t.SetUpCommandLine("-Wall") pkg := t.SetUpPackage("category/package", "GNU_CONFIGURE=\tyes", "USE_LANGUAGES=\t#") @@ -1249,7 +1274,6 @@ func (s *Suite) Test_Package_checkfilePa func (s *Suite) Test_Package_checkfilePackageMakefile__GNU_CONFIGURE_ok(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wall,no-space") pkg := t.SetUpPackage("category/package", "GNU_CONFIGURE=\tyes", "USE_LANGUAGES=\t# none, really") @@ -1263,7 +1287,6 @@ func (s *Suite) Test_Package_checkfilePa func (s *Suite) Test_Package_checkfilePackageMakefile__REPLACE_PERL(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wall,no-space") pkg := t.SetUpPackage("category/package", "REPLACE_PERL=\t*.pl", "NO_CONFIGURE=\tyes") @@ -2522,11 +2545,11 @@ func (s *Suite) Test_Package_checkUpdate // The package names intentionally differ from the package directories // to ensure that the check uses the package name. t.SetUpPackage("category/pkg1", - "PKGNAME= package1-1.0") + "PKGNAME=\tpackage1-1.0") t.SetUpPackage("category/pkg2", - "PKGNAME= package2-1.0") + "PKGNAME=\tpackage2-1.0") t.SetUpPackage("category/pkg3", - "PKGNAME= package3-5.0") + "PKGNAME=\tpackage3-5.0") t.CreateFileLines("doc/TODO", CvsID, "Suggested package updates", @@ -2541,7 +2564,7 @@ func (s *Suite) Test_Package_checkUpdate "\t"+"o package3-3.0 [security update]") t.Chdir(".") - t.Main("-Wall,no-space", "category/pkg1", "category/pkg2", "category/pkg3") + t.Main("-Wall", "category/pkg1", "category/pkg2", "category/pkg3") t.CheckOutputLines( "NOTE: category/pkg1/Makefile:4: The update request to 1.0 from ../../doc/TODO:6 has been done.", @@ -2556,7 +2579,7 @@ func (s *Suite) Test_Package_checkUpdate func (s *Suite) Test_Package_checkDirent__errors(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Call", "-Wall,no-space") + t.SetUpCommandLine("-Call", "-Wall") t.SetUpPkgsrc() t.CreateFileLines("category/package/files/subdir/file") t.CreateFileLines("category/package/files/subdir/subsub/file") @@ -2576,12 +2599,13 @@ func (s *Suite) Test_Package_checkDirent func (s *Suite) Test_Package_checkDirent__file_selection(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Call", "-Wall,no-space") + t.SetUpCommandLine("-Call", "-Wall") t.SetUpPkgsrc() t.CreateFileLines("doc/CHANGES-2018", CvsID) t.CreateFileLines("category/package/buildlink3.mk", - MkCvsID) + MkCvsID, + "") t.CreateFileLines("category/package/unexpected.txt", CvsID) t.FinishSetUp() @@ -2592,6 +2616,7 @@ func (s *Suite) Test_Package_checkDirent pkg.checkDirent(t.File("category/package/unexpected.txt"), 0444) t.CheckOutputLines( + "NOTE: ~/category/package/buildlink3.mk:2: Trailing empty lines.", "WARN: ~/category/package/buildlink3.mk:EOF: Expected a BUILDLINK_TREE line.", "WARN: ~/category/package/unexpected.txt: Unexpected file found.") } Index: pkgsrc/pkgtools/pkglint/files/patches.go diff -u pkgsrc/pkgtools/pkglint/files/patches.go:1.33 pkgsrc/pkgtools/pkglint/files/patches.go:1.34 --- pkgsrc/pkgtools/pkglint/files/patches.go:1.33 Sat Nov 23 23:35:56 2019 +++ pkgsrc/pkgtools/pkglint/files/patches.go Mon Dec 2 23:32:09 2019 @@ -200,7 +200,7 @@ func (ck *PatchChecker) checkBeginDiff(l "be mentioned in this file, to prevent duplicate work.") } - if G.Opts.WarnSpace && !ck.previousLineEmpty { + if !ck.previousLineEmpty { fix := line.Autofix() fix.Notef("Empty line expected.") fix.InsertBefore("") Index: pkgsrc/pkgtools/pkglint/files/path.go diff -u pkgsrc/pkgtools/pkglint/files/path.go:1.3 pkgsrc/pkgtools/pkglint/files/path.go:1.4 --- pkgsrc/pkgtools/pkglint/files/path.go:1.3 Sat Nov 30 20:35:11 2019 +++ pkgsrc/pkgtools/pkglint/files/path.go Mon Dec 2 23:32:09 2019 @@ -8,22 +8,14 @@ import ( "strings" ) -// Path is a slash-separated path in the filesystem. +// Path is a slash-separated path. +// It may or may not resolve to an existing file. // It may be absolute or relative. // Some paths may contain placeholders like @VAR@ or ${VAR}. -// The base directory of relative paths depends on the context -// in which the path is used. -// -// TODO: Consider adding several more specialized types of path: -// TODO: CurrPath, relative to the current working directory -// TODO: PkgsrcPath, relative to the pkgsrc root -// TODO: PackagePath, relative to the package directory -// TODO: RelativePath, relative to some other basedir +// The base directory of relative paths is unspecified. type Path string -func NewPath(name string) Path { return Path(name) } - -func NewPathSlash(name string) Path { return Path(filepath.ToSlash(name)) } +func NewPath(p string) Path { return Path(p) } func (p Path) String() string { return string(p) } @@ -33,7 +25,24 @@ func (p Path) GoString() string { return // which is usually a sign of an uninitialized variable. func (p Path) IsEmpty() bool { return p == "" } -func (p Path) Dir() Path { return Path(path.Dir(string(p))) } +func (p Path) DirClean() Path { return Path(path.Dir(string(p))) } + +// Returns the directory of the path, with only minimal cleaning. +// Only redundant dots and slashes are removed, and only at the end. +func (p Path) DirNoClean() Path { + s := p.String() + end := len(s) + for end > 0 && s[end-1] != '/' { + end-- + } + for end > 1 && s[end-1] == '/' || end > 2 && hasPrefix(s[end-2:], "/.") { + end-- + } + if end == 0 { + return "." + } + return NewPath(s[:end]) +} func (p Path) Base() string { return path.Base(string(p)) } @@ -48,7 +57,7 @@ func (p Path) Split() (dir Path, base st // Absolute paths have an empty string as its first part. // All other parts are nonempty. func (p Path) Parts() []string { - if p == "" { + if p.IsEmpty() { return nil } @@ -120,11 +129,6 @@ func (p Path) ContainsPath(sub Path) boo return sub == "." } -func (p Path) ContainsPathCanonical(sub Path) bool { - cleaned := cleanpath(p) - return cleaned.ContainsPath(sub) -} - func (p Path) HasSuffixText(suffix string) bool { return hasSuffix(string(p), suffix) } @@ -159,16 +163,30 @@ func (p Path) Clean() Path { return NewP // CleanDot returns the path with single dots removed and double slashes // collapsed. func (p Path) CleanDot() Path { - if !p.ContainsText(".") { + if !p.ContainsText(".") && !p.ContainsText("//") { return p } - var parts []string - for i, part := range p.Parts() { - if !(part == "." || i > 0 && part == "") { // See Parts - parts = append(parts, part) + parts := p.Parts() + return NewPath(strings.Join(parts, "/")) +} + +// Differs from path.Clean in that only "../../" is replaced, not "../". +// Also, the initial directory is always kept. +// This is to provide the package path as context in deeply nested .include chains. +func (p Path) CleanPath() Path { + parts := p.Parts() + + for i := 2; i+3 < len(parts); /* nothing */ { + if parts[i] != ".." && parts[i+1] != ".." && parts[i+2] == ".." && parts[i+3] == ".." { + if i+4 == len(parts) || parts[i+4] != ".." { + parts = append(parts[:i], parts[i+4:]...) + continue + } } + i++ } + if len(parts) == 0 { return "." } @@ -188,44 +206,262 @@ func (p Path) Rel(other Path) Path { return NewPath(filepath.ToSlash(rel)) } -func (p Path) Rename(newName Path) error { +// CurrPath is a path that is either absolute or relative to the current +// working directory. It is used in command line arguments and for +// loading files from the file system, and later in the diagnostics. +type CurrPath string + +func NewCurrPath(p Path) CurrPath { return CurrPath(p) } + +func NewCurrPathString(p string) CurrPath { return CurrPath(p) } + +func NewCurrPathSlash(p string) CurrPath { + return CurrPath(filepath.ToSlash(p)) +} + +func (p CurrPath) GoString() string { return p.AsPath().GoString() } + +func (p CurrPath) String() string { return string(p) } + +func (p CurrPath) AsPath() Path { return Path(p) } + +func (p CurrPath) IsEmpty() bool { return p.AsPath().IsEmpty() } + +func (p CurrPath) DirClean() CurrPath { + return CurrPath(p.AsPath().DirClean()) +} + +func (p CurrPath) DirNoClean() CurrPath { + return CurrPath(p.AsPath().DirNoClean()) +} + +func (p CurrPath) Base() string { return p.AsPath().Base() } + +func (p CurrPath) Split() (dir CurrPath, base string) { + pathDir, pathBase := p.AsPath().Split() + return NewCurrPath(pathDir), pathBase +} + +func (p CurrPath) Parts() []string { return p.AsPath().Parts() } + +func (p CurrPath) IsAbs() bool { return p.AsPath().IsAbs() } + +func (p CurrPath) HasPrefixPath(prefix CurrPath) bool { + return p.AsPath().HasPrefixPath(prefix.AsPath()) +} + +func (p CurrPath) ContainsPath(sub Path) bool { + return p.AsPath().ContainsPath(sub) +} + +func (p CurrPath) ContainsText(text string) bool { + return p.AsPath().ContainsText(text) +} + +func (p CurrPath) HasSuffixPath(suffix Path) bool { + return p.AsPath().HasSuffixPath(suffix) +} + +func (p CurrPath) HasSuffixText(suffix string) bool { + return p.AsPath().HasSuffixText(suffix) +} + +func (p CurrPath) HasBase(base string) bool { + return p.AsPath().HasBase(base) +} + +func (p CurrPath) TrimSuffix(suffix string) CurrPath { + return NewCurrPath(p.AsPath().TrimSuffix(suffix)) +} + +func (p CurrPath) ReplaceSuffix(from string, to string) CurrPath { + trimmed := p.TrimSuffix(from) + assert(trimmed != p) + return NewCurrPathString(trimmed.String() + to) +} + +func (p CurrPath) Clean() CurrPath { return CurrPath(p.AsPath().Clean()) } + +func (p CurrPath) CleanDot() CurrPath { + return NewCurrPath(p.AsPath().CleanDot()) +} + +func (p CurrPath) CleanPath() CurrPath { + return CurrPath(p.AsPath().CleanPath()) +} + +func (p CurrPath) JoinNoClean(other Path) CurrPath { + return CurrPath(p.AsPath().JoinNoClean(other)) +} + +func (p CurrPath) JoinClean(other Path) CurrPath { + return NewCurrPath(p.AsPath().JoinClean(other)) +} + +func (p CurrPath) Rel(rel CurrPath) Path { + return p.AsPath().Rel(rel.AsPath()) +} + +func (p CurrPath) Rename(newName CurrPath) error { return os.Rename(string(p), string(newName)) } -func (p Path) Lstat() (os.FileInfo, error) { return os.Lstat(string(p)) } +func (p CurrPath) Lstat() (os.FileInfo, error) { return os.Lstat(string(p)) } -func (p Path) Stat() (os.FileInfo, error) { return os.Stat(string(p)) } +func (p CurrPath) Stat() (os.FileInfo, error) { return os.Stat(string(p)) } -func (p Path) Exists() bool { +func (p CurrPath) Exists() bool { _, err := p.Lstat() return err == nil } -func (p Path) IsFile() bool { +func (p CurrPath) IsFile() bool { info, err := p.Lstat() return err == nil && info.Mode().IsRegular() } -func (p Path) IsDir() bool { +func (p CurrPath) IsDir() bool { info, err := p.Lstat() return err == nil && info.IsDir() } -func (p Path) Chmod(mode os.FileMode) error { +func (p CurrPath) Chmod(mode os.FileMode) error { return os.Chmod(string(p), mode) } -func (p Path) ReadDir() ([]os.FileInfo, error) { +func (p CurrPath) ReadDir() ([]os.FileInfo, error) { return ioutil.ReadDir(string(p)) } -func (p Path) Open() (*os.File, error) { return os.Open(string(p)) } +func (p CurrPath) ReadPaths() []CurrPath { + infos, err := p.ReadDir() + if err != nil { + return nil + } + var filenames []CurrPath + for _, info := range infos { + if !isIgnoredFilename(info.Name()) { + joined := p.JoinNoClean(NewPath(info.Name())).CleanPath() + filenames = append(filenames, joined) + } + } + return filenames +} -func (p Path) ReadString() (string, error) { +func (p CurrPath) Open() (*os.File, error) { return os.Open(string(p)) } + +func (p CurrPath) ReadString() (string, error) { bytes, err := ioutil.ReadFile(string(p)) return string(bytes), err } -func (p Path) WriteString(s string) error { +func (p CurrPath) WriteString(s string) error { return ioutil.WriteFile(string(p), []byte(s), 0666) } + +// PkgsrcPath is a path relative to the pkgsrc root. +type PkgsrcPath string + +func NewPkgsrcPath(p Path) PkgsrcPath { return PkgsrcPath(p) } + +func (p PkgsrcPath) String() string { return string(p) } + +func (p PkgsrcPath) AsPath() Path { return NewPath(string(p)) } + +func (p PkgsrcPath) DirClean() PkgsrcPath { + return NewPkgsrcPath(p.AsPath().DirClean()) +} + +func (p PkgsrcPath) DirNoClean() PkgsrcPath { + return NewPkgsrcPath(p.AsPath().DirNoClean()) +} + +func (p PkgsrcPath) Base() string { return p.AsPath().Base() } + +func (p PkgsrcPath) Count() int { return p.AsPath().Count() } + +func (p PkgsrcPath) HasPrefixPath(prefix Path) bool { + return p.AsPath().HasPrefixPath(prefix) +} + +func (p PkgsrcPath) JoinNoClean(other Path) PkgsrcPath { + return NewPkgsrcPath(p.AsPath().JoinNoClean(other)) +} + +func (p PkgsrcPath) JoinRel(other RelPath) PkgsrcPath { + return p.JoinNoClean(other.AsPath()) +} + +// PackagePath is a path relative to the package directory. It is used +// for the PATCHDIR and PKGDIR variables, as well as dependencies and +// conflicts on other packages. +type PackagePath string + +func NewPackagePath(p Path) PackagePath { return PackagePath(p) } + +func (p PackagePath) AsPath() Path { return Path(p) } + +func (p PackagePath) String() string { return p.AsPath().String() } + +// TODO: try RelPath instead of Path +func (p PackagePath) JoinNoClean(other Path) PackagePath { + return NewPackagePath(p.AsPath().JoinNoClean(other)) +} + +func (p PackagePath) IsEmpty() bool { return p.AsPath().IsEmpty() } + +// RelPath is a path that is relative to some base directory that is not +// further specified. +type RelPath string + +func NewRelPath(p Path) RelPath { return RelPath(p) } + +func NewRelPathString(p string) RelPath { return RelPath(p) } + +func (p RelPath) AsPath() Path { return NewPath(string(p)) } + +func (p RelPath) String() string { return p.AsPath().String() } + +func (p RelPath) DirClean() RelPath { return RelPath(p.AsPath().DirClean()) } + +func (p RelPath) DirNoClean() RelPath { + return RelPath(p.AsPath().DirNoClean()) +} + +func (p RelPath) Base() string { return p.AsPath().Base() } + +func (p RelPath) HasBase(base string) bool { return p.AsPath().HasBase(base) } + +func (p RelPath) Parts() []string { return p.AsPath().Parts() } + +func (p RelPath) Count() int { return p.AsPath().Count() } + +func (p RelPath) Clean() RelPath { return NewRelPath(p.AsPath().Clean()) } + +func (p RelPath) CleanPath() RelPath { + return RelPath(p.AsPath().CleanPath()) +} + +func (p RelPath) JoinNoClean(other Path) RelPath { + return RelPath(p.AsPath().JoinNoClean(other)) +} + +func (p RelPath) Replace(from string, to string) RelPath { + return RelPath(p.AsPath().Replace(from, to)) +} + +func (p RelPath) HasPrefixPath(prefix Path) bool { + return p.AsPath().HasPrefixPath(prefix) +} + +func (p RelPath) ContainsPath(sub Path) bool { + return p.AsPath().ContainsPath(sub) +} + +func (p RelPath) ContainsText(text string) bool { + return p.AsPath().ContainsText(text) +} + +func (p RelPath) HasSuffixPath(suffix Path) bool { + return p.AsPath().HasSuffixPath(suffix) +} Index: pkgsrc/pkgtools/pkglint/files/path_test.go diff -u pkgsrc/pkgtools/pkglint/files/path_test.go:1.3 pkgsrc/pkgtools/pkglint/files/path_test.go:1.4 --- pkgsrc/pkgtools/pkglint/files/path_test.go:1.3 Sat Nov 30 20:35:11 2019 +++ pkgsrc/pkgtools/pkglint/files/path_test.go Mon Dec 2 23:32:09 2019 @@ -16,17 +16,6 @@ func (s *Suite) Test_NewPath(c *check.C) c.Check(NewPath("\\"), check.Not(check.Equals), NewPath("/")) } -func (s *Suite) Test_NewPathSlash(c *check.C) { - t := s.Init(c) - - t.CheckEquals(NewPathSlash("filename"), NewPathSlash("filename")) - t.CheckEquals(NewPathSlash("\\"), NewPathSlash("\\")) - - t.CheckEquals( - NewPathSlash("\\"), - NewPathSlash(condStr(runtime.GOOS == "windows", "/", "\\"))) -} - func (s *Suite) Test_Path_String(c *check.C) { t := s.Init(c) @@ -60,11 +49,29 @@ func (s *Suite) Test_Path_IsEmpty(c *che test("/", false) } -func (s *Suite) Test_Path_Dir(c *check.C) { +func (s *Suite) Test_Path_DirClean(c *check.C) { t := s.Init(c) test := func(p, dir Path) { - t.CheckEquals(p.Dir(), dir) + t.CheckEquals(p.DirClean(), dir) + } + + test("", ".") + test("././././", ".") + test("/root", "/") + test("filename", ".") + test("dir/filename", "dir") + test("dir/filename\\with\\backslash", "dir") + + // TODO: I didn't expect that Dir would return the cleaned path. + test("././././dir/filename", "dir") +} + +func (s *Suite) Test_Path_DirNoClean(c *check.C) { + t := s.Init(c) + + test := func(p, dir Path) { + t.CheckEquals(p.DirNoClean(), dir) } test("", ".") @@ -294,32 +301,6 @@ func (s *Suite) Test_Path_ContainsPath(c test("aa/bb/cc", "c", false) } -func (s *Suite) Test_Path_ContainsPathCanonical(c *check.C) { - t := s.Init(c) - - test := func(p, sub Path, contains bool) { - t.CheckEquals(p.ContainsPathCanonical(sub), contains) - } - - test("", "", false) - test(".", "", false) - test("filename", "", false) - test("filename", "filename", true) - test("a/b/c", "a", true) - test("a/b/c", "b", true) - test("a/b/c", "c", true) - test("a/b/c", "a/b", true) - test("a/b/c", "b/c", true) - test("a/b/c", "a/b/c", true) - test("aa/b/c", "a", false) - test("a/bb/c", "b", false) - test("a/bb/c", "b/c", false) - test("mk/fetch/fetch.mk", "mk", true) - test("category/package/../../wip/mk", "mk", true) - test("category/package/../../wip/mk/..", "mk", true) // FIXME - test("category/package/../../wip/mk/../..", "mk", false) -} - func (s *Suite) Test_Path_HasSuffixText(c *check.C) { t := s.Init(c) @@ -444,6 +425,61 @@ func (s *Suite) Test_Path_CleanDot(c *ch test("/absolute", "/absolute") test("/usr/pkgsrc/wip/package", "/usr/pkgsrc/wip/package") test("/usr/pkgsrc/wip/package/../mk/git-package.mk", "/usr/pkgsrc/wip/package/../mk/git-package.mk") + test("a//b", "a/b") +} + +func (s *Suite) Test_Path_CleanPath(c *check.C) { + t := s.Init(c) + + test := func(from, to Path) { + t.CheckEquals(from.CleanPath(), to) + } + + test("simple/path", "simple/path") + test("/absolute/path", "/absolute/path") + + // Single dot components are removed, unless it's the only component of the path. + test("./././.", ".") + test("./././", ".") + test("dir/multi/././/file", "dir/multi/file") + test("dir/", "dir") + + test("dir/", "dir") + + // Components like aa/bb/../.. are removed, but not in the initial part of the path, + // and only if they are not followed by another "..". + test("dir/../dir/../dir/../dir/subdir/../../Makefile", "dir/../dir/../dir/../Makefile") + test("111/222/../../333/444/../../555/666/../../777/888/9", "111/222/../../777/888/9") + test("1/2/3/../../4/5/6/../../7/8/9/../../../../10", "1/2/3/../../4/7/8/9/../../../../10") + test("cat/pkg.v1/../../cat/pkg.v2/Makefile", "cat/pkg.v1/../../cat/pkg.v2/Makefile") + test("aa/../../../../../a/b/c/d", "aa/../../../../../a/b/c/d") + test("aa/bb/../../../../a/b/c/d", "aa/bb/../../../../a/b/c/d") + test("aa/bb/cc/../../../a/b/c/d", "aa/bb/cc/../../../a/b/c/d") + test("aa/bb/cc/dd/../../a/b/c/d", "aa/bb/a/b/c/d") + test("aa/bb/cc/dd/ee/../a/b/c/d", "aa/bb/cc/dd/ee/../a/b/c/d") + test("../../../../../a/b/c/d", "../../../../../a/b/c/d") + test("aa/../../../../a/b/c/d", "aa/../../../../a/b/c/d") + test("aa/bb/../../../a/b/c/d", "aa/bb/../../../a/b/c/d") + test("aa/bb/cc/../../a/b/c/d", "aa/bb/cc/../../a/b/c/d") + test("aa/bb/cc/dd/../a/b/c/d", "aa/bb/cc/dd/../a/b/c/d") + test("aa/../cc/../../a/b/c/d", "aa/../cc/../../a/b/c/d") + + // The initial 2 components of the path are typically category/package, when + // pkglint is called from the pkgsrc top-level directory. + // This path serves as the context and therefore is always kept. + test("aa/bb/../../cc/dd/../../ee/ff", "aa/bb/../../ee/ff") + test("aa/bb/../../cc/dd/../..", "aa/bb/../..") + test("aa/bb/cc/dd/../..", "aa/bb") + test("aa/bb/../../cc/dd/../../ee/ff/buildlink3.mk", "aa/bb/../../ee/ff/buildlink3.mk") + test("./aa/bb/../../cc/dd/../../ee/ff/buildlink3.mk", "aa/bb/../../ee/ff/buildlink3.mk") + + test("../.", "..") + test("../././././././.", "..") + test(".././././././././", "..") + + test( + "x11/kde-runtime4/../../misc/kdepimlibs4/../../databases/openldap-client/buildlink3.mk", + "x11/kde-runtime4/../../databases/openldap-client/buildlink3.mk") } func (s *Suite) Test_Path_IsAbs(c *check.C) { @@ -473,13 +509,332 @@ func (s *Suite) Test_Path_Rel(c *check.C t.CheckEquals(abc.Rel(base), NewPath("../../../.")) } -func (s *Suite) Test_Path_Rename(c *check.C) { +func (s *Suite) Test_NewCurrPath(c *check.C) { + t := s.Init(c) + + curr := NewCurrPath("dir/.///file") + + t.CheckEquals(curr.String(), "dir/.///file") +} + +func (s *Suite) Test_NewCurrPathString(c *check.C) { + t := s.Init(c) + + curr := NewCurrPathString("dir/.///file") + + t.CheckEquals(curr.String(), "dir/.///file") +} + +func (s *Suite) Test_NewCurrPathSlash(c *check.C) { + t := s.Init(c) + + test := func(path, curr string) { + t.CheckEquals(NewCurrPathSlash(path).String(), curr) + } + testWindows := func(path, currWindows, currOther string) { + t.CheckEquals( + NewCurrPathSlash(path).String(), + condStr(runtime.GOOS == "windows", currWindows, currOther)) + } + + test("filename", "filename") + test("dir/.///file", "dir/.///file") + + testWindows("\\", "/", "\\") +} + +func (s *Suite) Test_NewCurrPathSlash__windows(c *check.C) { + t := s.Init(c) + + if runtime.GOOS != "windows" { + return + } + + curr := NewCurrPathSlash("dir\\.\\\\\\file") + + t.CheckEquals(curr.String(), "dir/.///file") +} + +func (s *Suite) Test_CurrPath_GoString(c *check.C) { + t := s.Init(c) + + // Tabs in filenames are rare, probably typos. + curr := NewCurrPath("dir/file\t") + + t.CheckEquals(curr.GoString(), "\"dir/file\\t\"") +} + +func (s *Suite) Test_CurrPath_String(c *check.C) { + t := s.Init(c) + + // Tabs in filenames are rare, probably typos. + curr := NewCurrPath("dir/file\t") + + t.CheckEquals(curr.String(), "dir/file\t") +} + +func (s *Suite) Test_CurrPath_AsPath(c *check.C) { + t := s.Init(c) + + // Tabs in filenames are rare, probably typos. + curr := NewCurrPath("dir/file\t") + + t.CheckEquals(curr.AsPath(), NewPath("dir/file\t")) +} + +func (s *Suite) Test_CurrPath_IsEmpty(c *check.C) { + t := s.Init(c) + + test := func(curr CurrPath, isEmpty bool) { + t.CheckEquals(curr.IsEmpty(), isEmpty) + } + + test("", true) + test(".", false) + test("/", false) +} + +func (s *Suite) Test_CurrPath_DirClean(c *check.C) { + t := s.Init(c) + + test := func(curr, dir CurrPath) { + t.CheckEquals(curr.DirClean(), dir) + } + + test("./dir/../dir///./file", "dir") +} + +func (s *Suite) Test_CurrPath_DirNoClean(c *check.C) { + t := s.Init(c) + + test := func(curr, dir CurrPath) { + t.CheckEquals(curr.DirNoClean(), dir) + } + + test("./dir/../dir///./file", "./dir/../dir") +} + +func (s *Suite) Test_CurrPath_Base(c *check.C) { + t := s.Init(c) + + test := func(curr CurrPath, base string) { + t.CheckEquals(curr.Base(), base) + } + + test("dir/file", "file") +} + +func (s *Suite) Test_CurrPath_Split(c *check.C) { + t := s.Init(c) + + test := func(curr, dir CurrPath, base string) { + actualDir, actualBase := curr.Split() + t.CheckEquals(actualDir, dir) + t.CheckEquals(actualBase, base) + } + + test("dir/file", "dir/", "file") +} + +func (s *Suite) Test_CurrPath_Parts(c *check.C) { + t := s.Init(c) + + test := func(curr CurrPath, parts ...string) { + t.CheckDeepEquals(curr.Parts(), parts) + } + + test("dir/file", "dir", "file") +} + +func (s *Suite) Test_CurrPath_IsAbs(c *check.C) { + t := s.Init(c) + + test := func(curr CurrPath, isAbs bool) { + t.CheckDeepEquals(curr.IsAbs(), isAbs) + } + + test("/", true) + test("./", false) + test("C:/", runtime.GOOS == "windows") +} + +func (s *Suite) Test_CurrPath_HasPrefixPath(c *check.C) { + t := s.Init(c) + + test := func(curr, prefix CurrPath, hasPrefix bool) { + t.CheckEquals(curr.HasPrefixPath(prefix), hasPrefix) + } + + test("dir/file", "dir", true) + test("dir/file", "file", false) + test("dir", ".", true) +} + +func (s *Suite) Test_CurrPath_ContainsPath(c *check.C) { + t := s.Init(c) + + test := func(curr CurrPath, sub Path, hasPrefix bool) { + t.CheckEquals(curr.ContainsPath(sub), hasPrefix) + } + + test("dir/file", "dir", true) + test("dir/file", "file", true) + test("dir/file", "fi", false) + test("dir", ".", true) +} + +func (s *Suite) Test_CurrPath_ContainsText(c *check.C) { + t := s.Init(c) + + test := func(curr CurrPath, sub string, hasPrefix bool) { + t.CheckEquals(curr.ContainsText(sub), hasPrefix) + } + + test("dir/file", "dir", true) + test("dir/file", "r/f", true) +} + +func (s *Suite) Test_CurrPath_HasSuffixPath(c *check.C) { + t := s.Init(c) + + test := func(curr CurrPath, suffix Path, hasPrefix bool) { + t.CheckEquals(curr.HasSuffixPath(suffix), hasPrefix) + } + + test("dir/file", "dir", false) + test("dir/file", "file", true) + test("dir/file", "le", false) + + // In contrast to HasPrefixPath, it doesn't really make sense to + // ask whether a path ends with the current directory. + test("dir", ".", false) +} + +func (s *Suite) Test_CurrPath_HasSuffixText(c *check.C) { + t := s.Init(c) + + test := func(curr CurrPath, suffix string, hasPrefix bool) { + t.CheckEquals(curr.HasSuffixText(suffix), hasPrefix) + } + + test("dir/file", "dir", false) + test("dir/file", "file", true) + test("dir/file", "le", true) +} + +func (s *Suite) Test_CurrPath_HasBase(c *check.C) { + t := s.Init(c) + + test := func(curr CurrPath, base string, hasPrefix bool) { + t.CheckEquals(curr.HasBase(base), hasPrefix) + } + + test("dir/file", "dir", false) + test("dir/file", "file", true) + test("dir/file", "le", false) +} + +func (s *Suite) Test_CurrPath_TrimSuffix(c *check.C) { + t := s.Init(c) + + test := func(curr CurrPath, suffix string, trimmed CurrPath) { + t.CheckEquals(curr.TrimSuffix(suffix), trimmed) + } + + test("dir/file", "dir", "dir/file") + test("dir/file", "file", "dir/") + test("dir/file", "le", "dir/fi") +} + +func (s *Suite) Test_CurrPath_ReplaceSuffix(c *check.C) { + t := s.Init(c) + + test := func(curr CurrPath, from, to string, replaced CurrPath) { + t.CheckEquals(curr.ReplaceSuffix(from, to), replaced) + } + + test("dir/file", "file", "subdir", "dir/subdir") + + // The path must actually end with the suffix, otherwise there is + // the risk of creating unintended paths. + t.ExpectAssert( + func() { test("dir/file", "no-match", "anything", "dir/file") }) +} + +func (s *Suite) Test_CurrPath_Clean(c *check.C) { + t := s.Init(c) + + test := func(curr, cleaned CurrPath) { + t.CheckEquals(curr.Clean(), cleaned) + } + + test("dir/file", "dir/file") + test("dir/.////../file", "file") +} + +func (s *Suite) Test_CurrPath_CleanDot(c *check.C) { + t := s.Init(c) + + test := func(curr, cleaned CurrPath) { + t.CheckEquals(curr.CleanDot(), cleaned) + } + + test("dir/file", "dir/file") + test("dir/.////../file", "dir/../file") +} + +func (s *Suite) Test_CurrPath_CleanPath(c *check.C) { + t := s.Init(c) + + test := func(curr, cleaned CurrPath) { + t.CheckEquals(curr.CleanPath(), cleaned) + } + + test("a/b/../../c/d/../../e/../f", "a/b/../../e/../f") +} + +func (s *Suite) Test_CurrPath_JoinNoClean(c *check.C) { + t := s.Init(c) + + test := func(curr CurrPath, other Path, joined CurrPath) { + t.CheckEquals(curr.JoinNoClean(other), joined) + } + + test("", "", "/") + test(".", "file", "./file") + test("dir", "subdir/file", "dir/subdir/file") +} + +func (s *Suite) Test_CurrPath_JoinClean(c *check.C) { + t := s.Init(c) + + test := func(curr CurrPath, other Path, joined CurrPath) { + t.CheckEquals(curr.JoinClean(other), joined) + } + + test("", "", "") + test(".", "./////file", "file") + test("dir/./.", "../subdir/file", "subdir/file") +} + +func (s *Suite) Test_CurrPath_Rel(c *check.C) { + t := s.Init(c) + + test := func(curr, suffix CurrPath, rel Path) { + t.CheckEquals(curr.Rel(suffix), rel) + } + + test("dir/subdir", "dir", "..") + test("dir/subdir", "file", "../../file") +} + +func (s *Suite) Test_CurrPath_Rename(c *check.C) { t := s.Init(c) f := t.CreateFileLines("filename.old", "line 1") t.CheckEquals(f.IsFile(), true) - dst := NewPath(f.TrimSuffix(".old").String() + ".new") + dst := f.ReplaceSuffix(".old", ".new") err := f.Rename(dst) @@ -489,10 +844,10 @@ func (s *Suite) Test_Path_Rename(c *chec "line 1") } -func (s *Suite) Test_Path_Lstat(c *check.C) { +func (s *Suite) Test_CurrPath_Lstat(c *check.C) { t := s.Init(c) - testDir := func(f Path, isDir bool) { + testDir := func(f CurrPath, isDir bool) { st, err := f.Lstat() assertNil(err, "Lstat") t.CheckEquals(st.Mode()&os.ModeDir != 0, isDir) @@ -505,10 +860,10 @@ func (s *Suite) Test_Path_Lstat(c *check testDir(t.File("file"), false) } -func (s *Suite) Test_Path_Stat(c *check.C) { +func (s *Suite) Test_CurrPath_Stat(c *check.C) { t := s.Init(c) - testDir := func(f Path, isDir bool) { + testDir := func(f CurrPath, isDir bool) { st, err := f.Stat() assertNil(err, "Stat") t.CheckEquals(st.Mode()&os.ModeDir != 0, isDir) @@ -521,10 +876,10 @@ func (s *Suite) Test_Path_Stat(c *check. testDir(t.File("file"), false) } -func (s *Suite) Test_Path_Exists(c *check.C) { +func (s *Suite) Test_CurrPath_Exists(c *check.C) { t := s.Init(c) - test := func(f Path, exists bool) { + test := func(f CurrPath, exists bool) { t.CheckEquals(f.Exists(), exists) } @@ -536,7 +891,7 @@ func (s *Suite) Test_Path_Exists(c *chec test(t.File("enoent"), false) } -func (s *Suite) Test_Path_IsFile(c *check.C) { +func (s *Suite) Test_CurrPath_IsFile(c *check.C) { t := s.Init(c) t.CreateFileLines("dir/file") @@ -547,7 +902,7 @@ func (s *Suite) Test_Path_IsFile(c *chec t.CheckEquals(t.File("dir/file").IsFile(), true) } -func (s *Suite) Test_Path_IsDir(c *check.C) { +func (s *Suite) Test_CurrPath_IsDir(c *check.C) { t := s.Init(c) t.CreateFileLines("dir/file") @@ -558,10 +913,10 @@ func (s *Suite) Test_Path_IsDir(c *check t.CheckEquals(t.File("dir/file").IsDir(), false) } -func (s *Suite) Test_Path_Chmod(c *check.C) { +func (s *Suite) Test_CurrPath_Chmod(c *check.C) { t := s.Init(c) - testWritable := func(f Path, writable bool) { + testWritable := func(f CurrPath, writable bool) { lstat, err := f.Lstat() assertNil(err, "Lstat") t.CheckEquals(lstat.Mode().Perm()&0200 != 0, writable) @@ -576,7 +931,7 @@ func (s *Suite) Test_Path_Chmod(c *check testWritable(f, false) } -func (s *Suite) Test_Path_ReadDir(c *check.C) { +func (s *Suite) Test_CurrPath_ReadDir(c *check.C) { t := s.Init(c) t.CreateFileLines("subdir/file") @@ -595,7 +950,23 @@ func (s *Suite) Test_Path_ReadDir(c *che t.CheckDeepEquals(names, []string{".git", "CVS", "file", "subdir"}) } -func (s *Suite) Test_Path_Open(c *check.C) { +func (s *Suite) Test_CurrPath_ReadPaths(c *check.C) { + t := s.Init(c) + + t.CreateFileLines("dir/subdir/file") + t.CreateFileLines("dir/CVS/Entries") + t.CreateFileLines("dir/file") + + p := t.File("dir") + + paths := p.ReadPaths() + + t.CheckDeepEquals(paths, []CurrPath{ + t.File("dir/file"), + t.File("dir/subdir")}) +} + +func (s *Suite) Test_CurrPath_Open(c *check.C) { t := s.Init(c) t.CreateFileLines("filename", @@ -613,7 +984,7 @@ func (s *Suite) Test_Path_Open(c *check. t.CheckEquals(sb.String(), "line 1\nline 2\n") } -func (s *Suite) Test_Path_ReadString(c *check.C) { +func (s *Suite) Test_CurrPath_ReadString(c *check.C) { t := s.Init(c) t.CreateFileLines("filename", @@ -626,7 +997,7 @@ func (s *Suite) Test_Path_ReadString(c * t.CheckEquals(text, "line 1\nline 2\n") } -func (s *Suite) Test_Path_WriteString(c *check.C) { +func (s *Suite) Test_CurrPath_WriteString(c *check.C) { t := s.Init(c) err := t.File("filename").WriteString("line 1\nline 2\n") @@ -636,3 +1007,331 @@ func (s *Suite) Test_Path_WriteString(c "line 1", "line 2") } + +func (s *Suite) Test_NewPkgsrcPath(c *check.C) { + t := s.Init(c) + + p := NewPkgsrcPath("category/package") + + t.CheckEquals(p.AsPath(), NewPath("category/package")) +} + +func (s *Suite) Test_PkgsrcPath_String(c *check.C) { + t := s.Init(c) + + p := NewPkgsrcPath("any string..././") + + str := p.String() + + // No normalization takes place because it is typically not needed. + t.CheckEquals(str, "any string..././") +} + +func (s *Suite) Test_PkgsrcPath_AsPath(c *check.C) { + t := s.Init(c) + + pp := NewPkgsrcPath("./category/package/Makefile") + + p := pp.AsPath() + + t.CheckEquals(p.String(), "./category/package/Makefile") +} + +func (s *Suite) Test_PkgsrcPath_DirClean(c *check.C) { + t := s.Init(c) + + pp := NewPkgsrcPath("./dir/../dir/base///.") + + dir := pp.DirClean() + + t.CheckEquals(dir, NewPkgsrcPath("dir/base")) +} + +func (s *Suite) Test_PkgsrcPath_DirNoClean(c *check.C) { + t := s.Init(c) + + pp := NewPkgsrcPath("./dir/../dir/base///.") + + dir := pp.DirNoClean() + + t.CheckEquals(dir, NewPkgsrcPath("./dir/../dir/base")) +} + +func (s *Suite) Test_PkgsrcPath_Base(c *check.C) { + t := s.Init(c) + + pp := NewPkgsrcPath("dir/base///.") + + base := pp.Base() + + t.CheckEquals(base, ".") +} + +func (s *Suite) Test_PkgsrcPath_Count(c *check.C) { + t := s.Init(c) + + pp := NewPkgsrcPath("./...////dir") + + count := pp.Count() + + t.CheckEquals(count, 2) +} + +func (s *Suite) Test_PkgsrcPath_HasPrefixPath(c *check.C) { + t := s.Init(c) + + pp := NewPkgsrcPath("./././///prefix/suffix") + + hasPrefixPath := pp.HasPrefixPath("prefix") + + t.CheckEquals(hasPrefixPath, true) +} + +func (s *Suite) Test_PkgsrcPath_JoinNoClean(c *check.C) { + t := s.Init(c) + + pp := NewPkgsrcPath("base///.") + + joined := pp.JoinNoClean("./../rel") + + t.CheckEquals(joined, NewPkgsrcPath("base///././../rel")) +} + +func (s *Suite) Test_PkgsrcPath_JoinRel(c *check.C) { + t := s.Init(c) + + pp := NewPkgsrcPath("base///.") + + joined := pp.JoinRel("./../rel") + + t.CheckEquals(joined, NewPkgsrcPath("base///././../rel")) +} + +func (s *Suite) Test_NewPackagePath(c *check.C) { + t := s.Init(c) + + p := NewPackagePath("../../category/package") + + t.CheckEquals(p.AsPath(), NewPath("../../category/package")) +} + +func (s *Suite) Test_PackagePath_AsPath(c *check.C) { + t := s.Init(c) + + pp := NewPackagePath("../../category/package/Makefile") + + p := pp.AsPath() + + t.CheckEquals(p.String(), "../../category/package/Makefile") +} + +func (s *Suite) Test_PackagePath_String(c *check.C) { + t := s.Init(c) + + pp := NewPackagePath("../../category/package/Makefile") + + str := pp.String() + + t.CheckEquals(str, "../../category/package/Makefile") +} + +func (s *Suite) Test_PackagePath_JoinNoClean(c *check.C) { + t := s.Init(c) + + pp := NewPackagePath("../../category/package/Makefile") + + p := pp.JoinNoClean("patches") + + t.CheckEquals(p.String(), "../../category/package/Makefile/patches") +} + +func (s *Suite) Test_PackagePath_IsEmpty(c *check.C) { + t := s.Init(c) + + test := func(p PackagePath, isEmpty bool) { + t.CheckEquals(p.IsEmpty(), isEmpty) + } + + test("", true) + test(".", false) +} + +func (s *Suite) Test_NewRelPath(c *check.C) { + t := s.Init(c) + + rel := NewRelPath("dir/file") + + t.CheckEquals(rel.String(), "dir/file") +} + +func (s *Suite) Test_NewRelPathString(c *check.C) { + t := s.Init(c) + + rel := NewRelPathString("dir/file") + + t.CheckEquals(rel.String(), "dir/file") +} + +func (s *Suite) Test_RelPath_AsPath(c *check.C) { + t := s.Init(c) + + rel := NewRelPath("relative") + + path := rel.AsPath() + + t.CheckEquals(path.String(), "relative") +} + +func (s *Suite) Test_RelPath_String(c *check.C) { + t := s.Init(c) + + rel := NewRelPath(".///rel") + + str := rel.String() + + t.CheckEquals(str, ".///rel") +} + +func (s *Suite) Test_RelPath_DirClean(c *check.C) { + t := s.Init(c) + + rel := NewRelPath("./dir/../dir///./file") + + dir := rel.DirClean() + + t.CheckEquals(dir, NewRelPath("dir")) +} + +func (s *Suite) Test_RelPath_DirNoClean(c *check.C) { + t := s.Init(c) + + rel := NewRelPath("./dir/../dir///./file") + + dir := rel.DirNoClean() + + t.CheckEquals(dir, NewRelPath("./dir/../dir")) +} + +func (s *Suite) Test_RelPath_Base(c *check.C) { + t := s.Init(c) + + rel := NewRelPath("./dir////./file") + + base := rel.Base() + + t.CheckEquals(base, "file") +} + +func (s *Suite) Test_RelPath_HasBase(c *check.C) { + t := s.Init(c) + + test := func(rel RelPath, base string, hasBase bool) { + t.CheckEquals(rel.HasBase(base), hasBase) + } + + test("./dir/Makefile", "Makefile", true) + test("./dir/Makefile", "Make", false) + test("./dir/Makefile", "file", false) + test("./dir/Makefile", "dir/Makefile", false) +} + +func (s *Suite) Test_RelPath_Parts(c *check.C) { + t := s.Init(c) + + rel := NewRelPath("./dir/.///base") + + parts := rel.Parts() + + t.CheckDeepEquals(parts, []string{"dir", "base"}) +} + +func (s *Suite) Test_RelPath_Count(c *check.C) { + t := s.Init(c) + + rel := NewRelPath("./dir/.///base") + + count := rel.Count() + + t.CheckDeepEquals(count, 2) +} + +func (s *Suite) Test_RelPath_Clean(c *check.C) { + t := s.Init(c) + + rel := NewRelPath("a/b/../../c/d/../../e/../f") + + cleaned := rel.Clean() + + t.CheckEquals(cleaned, NewRelPath("f")) +} + +func (s *Suite) Test_RelPath_CleanPath(c *check.C) { + t := s.Init(c) + + rel := NewRelPath("a/b/../../c/d/../../e/../f") + + cleaned := rel.CleanPath() + + t.CheckEquals(cleaned, NewRelPath("a/b/../../e/../f")) +} + +func (s *Suite) Test_RelPath_JoinNoClean(c *check.C) { + t := s.Init(c) + + rel := NewRelPath("basedir/.//") + + joined := rel.JoinNoClean("./other") + + t.CheckEquals(joined, NewRelPath("basedir/.///./other")) +} + +func (s *Suite) Test_RelPath_Replace(c *check.C) { + t := s.Init(c) + + rel := NewRelPath("dir/subdir/file") + + replaced := rel.Replace("/", ":") + + t.CheckEquals(replaced, NewRelPath("dir:subdir:file")) +} + +func (s *Suite) Test_RelPath_HasPrefixPath(c *check.C) { + t := s.Init(c) + + rel := NewRelPath("dir/subdir/file") + + t.CheckEquals(rel.HasPrefixPath("dir"), true) + t.CheckEquals(rel.HasPrefixPath("dir/sub"), false) + t.CheckEquals(rel.HasPrefixPath("subdir"), false) +} + +func (s *Suite) Test_RelPath_ContainsPath(c *check.C) { + t := s.Init(c) + + rel := NewRelPath("dir/subdir/file") + + t.CheckEquals(rel.ContainsPath("dir"), true) + t.CheckEquals(rel.ContainsPath("dir/sub"), false) + t.CheckEquals(rel.ContainsPath("subdir"), true) +} + +func (s *Suite) Test_RelPath_ContainsText(c *check.C) { + t := s.Init(c) + + rel := NewRelPath("dir/subdir/file") + + t.CheckEquals(rel.ContainsText("dir"), true) + t.CheckEquals(rel.ContainsText("dir/sub"), true) + t.CheckEquals(rel.ContainsText("subdir"), true) + t.CheckEquals(rel.ContainsText("super"), false) +} + +func (s *Suite) Test_RelPath_HasSuffixPath(c *check.C) { + t := s.Init(c) + + rel := NewRelPath("dir/subdir/file") + + t.CheckEquals(rel.HasSuffixPath("file"), true) + t.CheckEquals(rel.HasSuffixPath("subdir/file"), true) + t.CheckEquals(rel.HasSuffixPath("subdir"), false) +} Index: pkgsrc/pkgtools/pkglint/files/pkglint.0 diff -u pkgsrc/pkgtools/pkglint/files/pkglint.0:1.41 pkgsrc/pkgtools/pkglint/files/pkglint.0:1.42 --- pkgsrc/pkgtools/pkglint/files/pkglint.0:1.41 Sat Nov 30 20:35:11 2019 +++ pkgsrc/pkgtools/pkglint/files/pkglint.0 Mon Dec 2 23:32:09 2019 @@ -79,8 +79,6 @@ DDEESSCCRRIIPPTTIIOONN [[nnoo‐‐]]qquuoottiinngg Warn for possibly invalid quoting of make variables in shell programs and shell variables themselves. - [[nnoo‐‐]]ssppaaccee Emit notes for inconsistent use of whitespace. - OOtthheerr aarrgguummeennttss _d_i_r _._._. The pkgsrc directories to be checked. If omit‐ ted, the current directory is checked. Index: pkgsrc/pkgtools/pkglint/files/pkglint.1 diff -u pkgsrc/pkgtools/pkglint/files/pkglint.1:1.58 pkgsrc/pkgtools/pkglint/files/pkglint.1:1.59 --- pkgsrc/pkgtools/pkglint/files/pkglint.1:1.58 Sat Nov 30 20:35:11 2019 +++ pkgsrc/pkgtools/pkglint/files/pkglint.1 Mon Dec 2 23:32:09 2019 @@ -1,4 +1,4 @@ -.\" $NetBSD: pkglint.1,v 1.58 2019/11/30 20:35:11 rillig Exp $ +.\" $NetBSD: pkglint.1,v 1.59 2019/12/02 23:32:09 rillig Exp $ .\" From FreeBSD: portlint.1,v 1.8 1997/11/25 14:53:14 itojun Exp .\" .\" Copyright (c) 1997 by Jun-ichiro Itoh . @@ -105,8 +105,6 @@ Warn if a variable is used or modified o .It Cm [no-]quoting Warn for possibly invalid quoting of make variables in shell programs and shell variables themselves. -.It Cm [no-]space -Emit notes for inconsistent use of whitespace. .El .\" ======================================================================= .Ss Other arguments Index: pkgsrc/pkgtools/pkglint/files/shell_test.go diff -u pkgsrc/pkgtools/pkglint/files/shell_test.go:1.58 pkgsrc/pkgtools/pkglint/files/shell_test.go:1.59 --- pkgsrc/pkgtools/pkglint/files/shell_test.go:1.58 Wed Nov 27 22:10:07 2019 +++ pkgsrc/pkgtools/pkglint/files/shell_test.go Mon Dec 2 23:32:09 2019 @@ -344,55 +344,6 @@ func (s *Suite) Test_SimpleCommandChecke "WARN: Makefile:4: Please use ${ECHO_N} instead of \"echo -n\".") } -func (s *Suite) Test_ShellLineChecker__shell_comment_with_line_continuation(c *check.C) { - t := s.Init(c) - - t.SetUpTool("echo", "", AtRunTime) - - test := func(lines ...string) { - i := 0 - for ; i < len(lines) && hasPrefix(lines[i], "\t"); i++ { - } - - mklines := t.SetUpFileMkLines("Makefile", - append([]string{MkCvsID, "pre-install:"}, - lines[:i]...)...) - - mklines.Check() - - t.CheckOutput(lines[i:]) - } - - // The comment can start at the beginning of a follow-up line. - test( - "\techo first; \\", - "\t# comment at the beginning of a command \\", - "\techo \"hello\"", - - // TODO: Warn that the "echo hello" is commented out. - ) - - // The comment can start at the beginning of a simple command. - test( - "\techo first; # comment at the beginning of a command \\", - "\techo \"hello\"", - - // TODO: Warn that the "echo hello" is commented out. - ) - - // The comment can start at a word in the middle of a command. - test( - // TODO: Warn that the "echo hello" is commented out. - "\techo # comment starts inside a command \\", - "\techo \"hello\"") - - // If the comment starts in the last line, there's no further - // line that might be commented out accidentally. - test( - "\techo 'first line'; \\", - "\t# comment in last line") -} - func (s *Suite) Test_ShellLineChecker_checkConditionalCd(c *check.C) { t := s.Init(c) @@ -946,6 +897,55 @@ func (s *Suite) Test_ShellLineChecker_Ch "NOTE: filename.mk:1: Please use the SUBST framework instead of ${SED} and ${MV}.") } +func (s *Suite) Test_ShellLineChecker_CheckShellCommandLine__sed_and_mv_explained(c *check.C) { + t := s.Init(c) + + t.SetUpCommandLine("-Wall", "--explain") + t.SetUpVartypes() + t.SetUpTool("sed", "SED", AtRunTime) + t.SetUpTool("mv", "MV", AtRunTime) + ck := t.NewShellLineChecker("\t${RUN} ${SED} 's,#,// comment:,g' filename > filename.tmp; ${MV} filename.tmp filename") + + ck.CheckShellCommandLine(ck.mkline.ShellCommand()) + + t.CheckOutputLines( + "NOTE: filename.mk:1: Please use the SUBST framework instead of ${SED} and ${MV}.", + "", + "\tUsing the SUBST framework instead of explicit commands is easier to", + "\tunderstand, since all the complexity of using sed and mv is hidden", + "\tbehind the scenes.", + "", + sprintf("\tRun %q for more information.", bmakeHelp("subst")), + "", + "\tWhen migrating to the SUBST framework, pay attention to \"#\"", + "\tcharacters. In shell commands, make(1) does not interpret them as", + "\tcomment character, but in variable assignments it does. Therefore,", + "\tinstead of the shell command", + "", + "\t\tsed -e 's,#define foo,,'", + "", + "\tyou need to write", + "", + "\t\tSUBST_SED.foo+=\t's,\\#define foo,,'", + "") +} + +func (s *Suite) Test_ShellLineChecker_CheckShellCommandLine__sed_and_mv_autofix_explained(c *check.C) { + t := s.Init(c) + + t.SetUpCommandLine("-Wall", "--explain", "--autofix") + t.SetUpVartypes() + t.SetUpTool("sed", "SED", AtRunTime) + t.SetUpTool("mv", "MV", AtRunTime) + ck := t.NewShellLineChecker("\t${RUN} ${SED} 's,#,// comment:,g' filename > filename.tmp; ${MV} filename.tmp filename") + + ck.CheckShellCommandLine(ck.mkline.ShellCommand()) + + // Only ever output an explanation if there's a corresponding diagnostic. + // Even if Explain is called twice in a row. + t.CheckOutputEmpty() +} + func (s *Suite) Test_ShellLineChecker_CheckShellCommandLine__subshell(c *check.C) { t := s.Init(c) @@ -1558,6 +1558,74 @@ func (s *Suite) Test_ShellLineChecker_ch "WARN: filename.mk:1: ${CP} should not be used to install files.") } +func (s *Suite) Test_ShellLineChecker_checkMultiLineComment(c *check.C) { + t := s.Init(c) + + t.SetUpTool("echo", "", AtRunTime) + t.SetUpTool("sed", "", AtRunTime) + t.SetUpVartypes() + + test := func(lines ...string) { + i := 0 + for ; i < len(lines) && hasPrefix(lines[i], "\t"); i++ { + } + + mklines := t.SetUpFileMkLines("Makefile", + append([]string{MkCvsID, "pre-build:"}, + lines[:i]...)...) + + mklines.Check() + + t.CheckOutput(lines[i:]) + } + + // The comment can start at the beginning of a follow-up line. + test( + "\techo first; \\", + "\t# comment at the beginning of a command \\", + "\techo \"hello\"", + + "WARN: ~/Makefile:4: "+ + "The shell comment does not stop at the end of this line.") + + // The comment can start at the beginning of a simple command. + test( + "\techo first; # comment at the beginning of a command \\", + "\techo \"hello\"", + + "WARN: ~/Makefile:3: "+ + "The shell comment does not stop at the end of this line.") + + // The comment can start at a word in the middle of a command. + test( + "\techo # comment starts inside a command \\", + "\techo \"hello\"", + + "WARN: ~/Makefile:3: "+ + "The shell comment does not stop at the end of this line.") + + // If the comment starts in the last line, there's no further + // line that might be commented out accidentally. + test( + "\techo 'first line'; \\", + "\t# comment in last line") + + // If there's a shell token that extends over several lines, + // that's unusual enough that pkglint refuses to check this. + test( + "\techo 'before \\", + "\t\tafter'; \\", + "\t# comment \\", + "\techo 'still a comment'") + + test( + "\tsed -e s#@PREFIX@#${PREFIX}#g \\", + "\tfilename", + + "WARN: ~/Makefile:3--4: Substitution commands like "+ + "\"s#@PREFIX@#${PREFIX}#g\" should always be quoted.") +} + func (s *Suite) Test_splitIntoShellTokens__line_continuation(c *check.C) { t := s.Init(c) @@ -1713,3 +1781,24 @@ func (s *Suite) Test_splitIntoShellToken ">>", "append"}) t.CheckEquals(rest, "") } + +func (s *Suite) Test_splitIntoShellTokens__varuse(c *check.C) { + t := s.Init(c) + + test := func(text string, tokens ...string) { + line := t.NewLine("filename.mk", 1, "") + + words, rest := splitIntoShellTokens(line, text) + + t.CheckDeepEquals(words, tokens) + t.CheckEquals(rest, "") + } + + test( + "sed -e s#@PREFIX@#${PREFIX}#g filename", + + "sed", + "-e", + "s#@PREFIX@#${PREFIX}#g", + "filename") +} Index: pkgsrc/pkgtools/pkglint/files/pkglint.go diff -u pkgsrc/pkgtools/pkglint/files/pkglint.go:1.67 pkgsrc/pkgtools/pkglint/files/pkglint.go:1.68 --- pkgsrc/pkgtools/pkglint/files/pkglint.go:1.67 Sat Nov 30 20:35:11 2019 +++ pkgsrc/pkgtools/pkglint/files/pkglint.go Mon Dec 2 23:32:09 2019 @@ -25,14 +25,15 @@ type Pkglint struct { Pkgsrc Pkgsrc // Global data, mostly extracted from mk/*. Pkg *Package // The package that is currently checked, or nil. - Todo PathQueue // The files or directories that still need to be checked. - Wip bool // Is the currently checked file or package from pkgsrc-wip? - Infrastructure bool // Is the currently checked file from the pkgsrc infrastructure? - Testing bool // Is pkglint in self-testing mode (only during development)? - Experimental bool // For experimental features, only enabled individually in tests - Username string // For checking against OWNER and MAINTAINER + Todo CurrPathQueue // The files or directories that still need to be checked. - cvsEntriesDir Path // Cached to avoid I/O + Wip bool // Is the currently checked file or package from pkgsrc-wip? + Infrastructure bool // Is the currently checked file from the pkgsrc infrastructure? + Testing bool // Is pkglint in self-testing mode (only during development)? + Experimental bool // For experimental features, only enabled individually in tests + Username string // For checking against OWNER and MAINTAINER + + cvsEntriesDir CurrPath // Cached to avoid I/O cvsEntries map[string]CvsEntry Logger Logger @@ -45,7 +46,7 @@ type Pkglint struct { // cwd is the absolute path to the current working // directory. It is used for speeding up Relpath and abspath. // There is no other use for it. - cwd Path + cwd CurrPath InterPackage InterPackage } @@ -57,7 +58,7 @@ func NewPkglint(stdout io.Writer, stderr p := Pkglint{ res: regex.NewRegistry(), fileCache: NewFileCache(200), - cwd: NewPathSlash(cwd), + cwd: NewCurrPathSlash(cwd), interner: NewStringInterner()} p.Logger.out = NewSeparatorWriter(stdout) p.Logger.err = NewSeparatorWriter(stderr) @@ -71,17 +72,9 @@ func unusablePkglint() Pkglint { return type CmdOpts struct { CheckGlobal bool - // TODO: Are these Warn* options really all necessary? - // - // Some of them may have been unreliable in the past when they were new. - // Instead of these fine-grained options, there is already --only, which - // could be contrasted by a future --ignore option, in order to suppress - // individual checks. - WarnExtra, WarnPerm, - WarnQuoting, - WarnSpace bool + WarnQuoting bool Profiling, ShowHelp, @@ -208,11 +201,12 @@ func (pkglint *Pkglint) setUpProfiling() func (pkglint *Pkglint) prepareMainLoop() { firstDir := pkglint.Todo.Front() if firstDir.IsFile() { - firstDir = firstDir.Dir() + // FIXME: consider DirNoClean + firstDir = firstDir.DirClean() } relTopdir := findPkgsrcTopdir(firstDir) - if relTopdir == "" { + if relTopdir.IsEmpty() { // If the first argument to pkglint is not inside a pkgsrc tree, // pkglint doesn't know where to load the infrastructure files from, // and these are needed for virtually every single check. @@ -257,7 +251,6 @@ func (pkglint *Pkglint) ParseCommandLine warn.AddFlagVar("extra", &gopts.WarnExtra, false, "enable some extra warnings") warn.AddFlagVar("perm", &gopts.WarnPerm, false, "warn about unforeseen variable definition and use") warn.AddFlagVar("quoting", &gopts.WarnQuoting, false, "warn about quoting issues") - warn.AddFlagVar("space", &gopts.WarnSpace, false, "warn about inconsistent use of whitespace") remainingArgs, err := opts.Parse(args) if err != nil { @@ -280,7 +273,7 @@ func (pkglint *Pkglint) ParseCommandLine } for _, arg := range pkglint.Opts.args { - pkglint.Todo.Push(NewPathSlash(arg)) + pkglint.Todo.Push(NewCurrPathSlash(arg)) } if pkglint.Todo.IsEmpty() { pkglint.Todo.Push(".") @@ -296,7 +289,7 @@ func (pkglint *Pkglint) ParseCommandLine // // It sets up all the global state (infrastructure, wip) for accurately // classifying the entry. -func (pkglint *Pkglint) Check(dirent Path) { +func (pkglint *Pkglint) Check(dirent CurrPath) { if trace.Tracing { defer trace.Call(dirent)() } @@ -310,7 +303,7 @@ func (pkglint *Pkglint) Check(dirent Pat pkglint.checkMode(dirent, st.Mode()) } -func (pkglint *Pkglint) checkMode(dirent Path, mode os.FileMode) { +func (pkglint *Pkglint) checkMode(dirent CurrPath, mode os.FileMode) { // TODO: merge duplicate code in Package.checkDirent isDir := mode.IsDir() isReg := mode.IsRegular() @@ -321,7 +314,8 @@ func (pkglint *Pkglint) checkMode(dirent dir := dirent if !isDir { - dir = dirent.Dir() + // FIXME: consider DirNoClean + dir = dirent.DirClean() } basename := dirent.Base() @@ -330,8 +324,8 @@ func (pkglint *Pkglint) checkMode(dirent pkglint.Wip = pkgsrcRel.HasPrefixPath("wip") pkglint.Infrastructure = pkgsrcRel.HasPrefixPath("mk") pkgsrcdir := findPkgsrcTopdir(dir) - if pkgsrcdir == "" { - NewLineWhole(dirent).Errorf("Cannot determine the pkgsrc root directory for %q.", cleanpath(dir)) + if pkgsrcdir.IsEmpty() { + NewLineWhole(dirent).Errorf("Cannot determine the pkgsrc root directory for %q.", dir.CleanPath()) return } @@ -359,7 +353,7 @@ func (pkglint *Pkglint) checkMode(dirent // checkdirPackage checks a complete pkgsrc package, including each // of the files individually, and also when seen in combination. -func (pkglint *Pkglint) checkdirPackage(dir Path) { +func (pkglint *Pkglint) checkdirPackage(dir CurrPath) { if trace.Tracing { defer trace.Call(dir)() } @@ -373,7 +367,7 @@ func (pkglint *Pkglint) checkdirPackage( } // Returns the pkgsrc top-level directory, relative to the given directory. -func findPkgsrcTopdir(dirname Path) Path { +func findPkgsrcTopdir(dirname CurrPath) Path { for _, dir := range [...]Path{".", "..", "../..", "../../.."} { if dirname.JoinNoClean(dir).JoinNoClean("mk/bsd.pkg.mk").IsFile() { return dir @@ -423,7 +417,7 @@ func resolveVariableRefs(mklines *MkLine } } -func CheckFileOther(filename Path) { +func CheckFileOther(filename CurrPath) { if trace.Tracing { defer trace.Call(filename)() } @@ -526,7 +520,7 @@ func CheckLinesMessage(lines *Lines) { SaveAutofixChanges(lines) } -func CheckFileMk(filename Path) { +func CheckFileMk(filename CurrPath) { if trace.Tracing { defer trace.Call(filename)() } @@ -547,7 +541,7 @@ func CheckFileMk(filename Path) { // checkReg checks the given regular file. // depth is 3 for files in the package directory, and 4 or more for files // deeper in the directory hierarchy, such as in files/ or patches/. -func (pkglint *Pkglint) checkReg(filename Path, basename string, depth int) { +func (pkglint *Pkglint) checkReg(filename CurrPath, basename string, depth int) { if depth == 3 && !pkglint.Wip { // FIXME: There's no good reason for prohibiting a README file. @@ -607,16 +601,20 @@ func (pkglint *Pkglint) checkReg(filenam CheckLinesPatch(lines) } - case filename.Dir().Base() == "patches" && matches(filename.Base(), `^manual[^/]*$`): + // FIXME: consider DirNoClean + case filename.DirClean().Base() == "patches" && matches(filename.Base(), `^manual[^/]*$`): if trace.Tracing { trace.Stepf("Unchecked file %q.", filename) } - case filename.Dir().Base() == "patches": + // FIXME: consider DirNoClean + case filename.DirClean().Base() == "patches": NewLineWhole(filename).Warnf("Patch files should be named \"patch-\", followed by letters, '-', '_', '.', and digits only.") case (hasPrefix(basename, "Makefile") || hasSuffix(basename, ".mk")) && - !filename.Dir().ContainsPath("files"): // FIXME: G.Pkgsrc.Rel(filename) instead of filename + // FIXME: consider DirNoClean + // FIXME: G.Pkgsrc.Rel(filename) instead of filename + !filename.DirClean().ContainsPath("files"): CheckFileMk(filename) case hasPrefix(basename, "PLIST"): @@ -628,7 +626,8 @@ func (pkglint *Pkglint) checkReg(filenam // This only checks the file but doesn't register the changes globally. _ = pkglint.Pkgsrc.loadDocChangesFromFile(filename) - case filename.Dir().Base() == "files": + // FIXME: consider DirNoClean + case filename.DirClean().Base() == "files": // Skip files directly in the files/ directory, but not those further down. case basename == "spec": @@ -653,7 +652,7 @@ func (pkglint *Pkglint) matchesLicenseFi return basename == path.Base(licenseFile) } -func (pkglint *Pkglint) checkExecutable(filename Path, mode os.FileMode) { +func (pkglint *Pkglint) checkExecutable(filename CurrPath, mode os.FileMode) { if mode.Perm()&0111 == 0 { // Not executable at all. return @@ -679,7 +678,7 @@ func (pkglint *Pkglint) checkExecutable( fix.Describef(0, "Clearing executable bits") if autofix { if err := filename.Chmod(mode &^ 0111); err != nil { - G.Logger.TechErrorf(cleanpath(filename), "Cannot clear executable bits: %s", err) + G.Logger.TechErrorf(filename.CleanPath(), "Cannot clear executable bits: %s", err) } } }) @@ -733,8 +732,9 @@ func (pkglint *Pkglint) tools(mklines *M } } -func (pkglint *Pkglint) loadCvsEntries(filename Path) map[string]CvsEntry { - dir := filename.Dir() +func (pkglint *Pkglint) loadCvsEntries(filename CurrPath) map[string]CvsEntry { + // FIXME: consider DirNoClean + dir := filename.DirClean() if dir == pkglint.cvsEntriesDir { return pkglint.cvsEntries } @@ -759,14 +759,14 @@ func (pkglint *Pkglint) loadCvsEntries(f } } - lines := Load(dir+"/CVS/Entries", 0) + lines := Load(dir.JoinNoClean("CVS/Entries"), 0) if lines != nil { entries = make(map[string]CvsEntry) for _, line := range lines.Lines { handle(line, true, line.Text) } - logLines := Load(dir+"/CVS/Entries.Log", 0) + logLines := Load(dir.JoinNoClean("CVS/Entries.Log"), 0) if logLines != nil { for _, line := range logLines.Lines { text := line.Text @@ -784,9 +784,9 @@ func (pkglint *Pkglint) loadCvsEntries(f return entries } -func (pkglint *Pkglint) Abs(filename Path) Path { +func (pkglint *Pkglint) Abs(filename CurrPath) CurrPath { if !filename.IsAbs() { - return pkglint.cwd.JoinNoClean(filename).Clean() + return pkglint.cwd.JoinNoClean(filename.AsPath()).Clean() } return filename.Clean() } Index: pkgsrc/pkgtools/pkglint/files/pkgsrc.go diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.44 pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.45 --- pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.44 Sat Nov 30 20:35:11 2019 +++ pkgsrc/pkgtools/pkglint/files/pkgsrc.go Mon Dec 2 23:32:09 2019 @@ -17,9 +17,8 @@ import ( // Everything else (distfile hashes, package names) is recorded in the Pkglint // type instead. type Pkgsrc struct { - // The top directory (PKGSRCDIR), either absolute or relative to - // the current working directory. - topdir Path + // The top directory (PKGSRCDIR). + topdir CurrPath // The set of user-defined variables that are added to BUILD_DEFS // within the bsd.pkg.mk file. @@ -35,7 +34,7 @@ type Pkgsrc struct { suggestedUpdates []SuggestedUpdate suggestedWipUpdates []SuggestedUpdate - LastChange map[Path]*Change + LastChange map[PkgsrcPath]*Change LastFreezeStart string // e.g. "2018-01-01", or "" LastFreezeEnd string // e.g. "2018-01-01", or "" @@ -52,7 +51,7 @@ type Pkgsrc struct { vartypes VarTypeRegistry } -func NewPkgsrc(dir Path) Pkgsrc { +func NewPkgsrc(dir CurrPath) Pkgsrc { return Pkgsrc{ dir, make(map[string]bool), @@ -62,7 +61,7 @@ func NewPkgsrc(dir Path) Pkgsrc { make(map[string]string), nil, nil, - make(map[Path]*Change), + make(map[PkgsrcPath]*Change), "", "", make(map[string][]string), @@ -156,7 +155,7 @@ func (src *Pkgsrc) loadDocChanges() { } } - src.LastChange = make(map[Path]*Change) + src.LastChange = make(map[PkgsrcPath]*Change) for _, filename := range filenames { changes := src.loadDocChangesFromFile(docDir.JoinNoClean(filename)) for _, change := range changes { @@ -170,7 +169,7 @@ func (src *Pkgsrc) loadDocChanges() { src.checkRemovedAfterLastFreeze() } -func (src *Pkgsrc) loadDocChangesFromFile(filename Path) []*Change { +func (src *Pkgsrc) loadDocChangesFromFile(filename CurrPath) []*Change { warn := G.Opts.CheckGlobal && !G.Wip @@ -312,7 +311,7 @@ func (*Pkgsrc) parseDocChange(line *Line return &Change{ Location: line.Location, Action: action, - Pkgpath: NewPath(intern(pkgpath)), + Pkgpath: NewPkgsrcPath(NewPath(intern(pkgpath))), target: intern(condStr(n == 6, f[3], "")), Author: intern(author), Date: intern(date), @@ -405,7 +404,7 @@ func (src *Pkgsrc) loadUserDefinedVars() func (src *Pkgsrc) loadTools() { tools := src.Tools - toolFiles := []Path{"defaults.mk"} + toolFiles := []RelPath{"defaults.mk"} { toc := src.File("mk/tools/bsd.tools.mk") mklines := LoadMk(toc, MustSucceed|NotEmpty) @@ -413,7 +412,7 @@ func (src *Pkgsrc) loadTools() { if mkline.IsInclude() { includedFile := mkline.IncludedFile() if !includedFile.ContainsText("/") { - toolFiles = append(toolFiles, includedFile) + toolFiles = append(toolFiles, NewRelPath(includedFile)) } } } @@ -430,13 +429,13 @@ func (src *Pkgsrc) loadTools() { tools.def("true", "TRUE", true, AfterPrefsMk, nil) for _, basename := range toolFiles { - mklines := src.LoadMk("mk/tools/"+basename, MustSucceed|NotEmpty) + mklines := src.LoadMk(NewPkgsrcPath("mk/tools").JoinRel(basename), MustSucceed|NotEmpty) mklines.ForEach(func(mkline *MkLine) { tools.ParseToolLine(mklines, mkline, true, !mklines.indentation.IsConditional()) }) } - for _, relativeName := range [...]Path{"mk/bsd.prefs.mk", "mk/bsd.pkg.mk"} { + for _, relativeName := range [...]PkgsrcPath{"mk/bsd.prefs.mk", "mk/bsd.pkg.mk"} { mklines := src.LoadMk(relativeName, MustSucceed|NotEmpty) mklines.ForEach(func(mkline *MkLine) { @@ -670,7 +669,7 @@ func (src *Pkgsrc) loadUntypedVars() { } } - handleMkFile := func(path Path) { + handleMkFile := func(path CurrPath) { mklines := LoadMk(path, MustSucceed) mklines.collectVariables() mklines.collectUsedVariables() @@ -686,7 +685,7 @@ func (src *Pkgsrc) loadUntypedVars() { assertNil(err, "handleFile %q", pathName) baseName := info.Name() if info.Mode().IsRegular() && (hasSuffix(baseName, ".mk") || baseName == "mk.conf") { - handleMkFile(NewPath(filepath.ToSlash(pathName))) // FIXME: This is too deep to handle os-specific paths + handleMkFile(NewCurrPathSlash(pathName)) // FIXME: This is too deep to handle os-specific paths } return nil } @@ -775,7 +774,7 @@ func (src *Pkgsrc) loadDefaultBuildDefs( // Example: // Latest("lang", `^php[0-9]+$`, "../../lang/$0") // => "../../lang/php72" -func (src *Pkgsrc) Latest(category Path, re regex.Pattern, repl string) string { +func (src *Pkgsrc) Latest(category PkgsrcPath, re regex.Pattern, repl string) string { versions := src.ListVersions(category, re, repl, true) if len(versions) > 0 { @@ -791,7 +790,7 @@ func (src *Pkgsrc) Latest(category Path, // Example: // ListVersions("lang", `^php[0-9]+$`, "php-$0") // => {"php-53", "php-56", "php-73"} -func (src *Pkgsrc) ListVersions(category Path, re regex.Pattern, repl string, errorIfEmpty bool) []string { +func (src *Pkgsrc) ListVersions(category PkgsrcPath, re regex.Pattern, repl string, errorIfEmpty bool) []string { if G.Testing { // Regular expression must be anchored at both ends, to avoid typos. assert(hasPrefix(string(re), "^")) @@ -995,7 +994,7 @@ func (src *Pkgsrc) IsBuildDef(varname st // ReadDir lists the files and subdirectories from the given directory // (relative to the pkgsrc root), filtering out any ignored files (CVS/*) // and empty directories. -func (src *Pkgsrc) ReadDir(dirName Path) []os.FileInfo { +func (src *Pkgsrc) ReadDir(dirName PkgsrcPath) []os.FileInfo { dir := src.File(dirName) files, err := dir.ReadDir() if err != nil { @@ -1017,7 +1016,7 @@ func (src *Pkgsrc) ReadDir(dirName Path) // // During pkglint testing, these files often don't exist, as they are // emulated by setting their data structures manually. -func (src *Pkgsrc) LoadMkExisting(filename Path) *MkLines { +func (src *Pkgsrc) LoadMkExisting(filename PkgsrcPath) *MkLines { options := NotEmpty if !G.Testing { options |= MustSucceed @@ -1026,12 +1025,12 @@ func (src *Pkgsrc) LoadMkExisting(filena } // LoadMk loads the Makefile relative to the pkgsrc top directory. -func (src *Pkgsrc) LoadMk(filename Path, options LoadOptions) *MkLines { +func (src *Pkgsrc) LoadMk(filename PkgsrcPath, options LoadOptions) *MkLines { return LoadMk(src.File(filename), options) } // Load loads the file relative to the pkgsrc top directory. -func (src *Pkgsrc) Load(filename Path, options LoadOptions) *Lines { +func (src *Pkgsrc) Load(filename PkgsrcPath, options LoadOptions) *Lines { return Load(src.File(filename), options) } @@ -1042,9 +1041,6 @@ func (src *Pkgsrc) Load(filename Path, o // pkgsrc root and from there to the "to" filename. This produces the form // "../../category/package" that is found in DEPENDS and .include lines. // -// Both from and to are interpreted relative to the current working directory, -// unless they are absolute paths. -// // This function should only be used if the relative path from one file to // another cannot be computed in another way. The preferred way is to take // the relative filenames directly from the .include or exists() where they @@ -1052,7 +1048,7 @@ func (src *Pkgsrc) Load(filename Path, o // // TODO: Invent data types for all kinds of relative paths that occur in pkgsrc // and pkglint. Make sure that these paths cannot be accidentally mixed. -func (src *Pkgsrc) Relpath(from, to Path) (result Path) { +func (src *Pkgsrc) Relpath(from, to CurrPath) Path { cfrom := from.Clean() cto := to.Clean() @@ -1062,23 +1058,20 @@ func (src *Pkgsrc) Relpath(from, to Path // Take a shortcut for the common case from "dir" to "dir/subdir/...". if cto.HasPrefixPath(cfrom) { - rel := cfrom.Rel(cto) - if !rel.HasPrefixPath("..") { - return rel - } + return cfrom.Rel(cto) } // Take a shortcut for the common case from "category/package" to ".". // This is the most common variant in a complete pkgsrc scan. if cto == "." { fromParts := cfrom.Parts() - if len(fromParts) == 2 && fromParts[0] != ".." && fromParts[1] != ".." { + if len(fromParts) == 2 && fromParts[0] != ".." { return "../.." } } if cfrom == "." && !cto.IsAbs() { - return cto.Clean() + return cto.Clean().AsPath() } absFrom := G.Abs(cfrom) @@ -1098,7 +1091,7 @@ func (src *Pkgsrc) Relpath(from, to Path if len(fromParts) >= 2 && len(toParts) >= 2 { if fromParts[0] == toParts[0] && fromParts[1] == toParts[1] { var relParts []string - for _ = range fromParts[2:] { + for range fromParts[2:] { relParts = append(relParts, "..") } relParts = append(relParts, toParts[2:]...) @@ -1106,16 +1099,15 @@ func (src *Pkgsrc) Relpath(from, to Path } } - result = up.JoinNoClean(down).CleanDot() - return + return up.JoinNoClean(down).CleanDot() } // File resolves a filename relative to the pkgsrc top directory. // // Example: // NewPkgsrc("/usr/pkgsrc").File("distfiles") => "/usr/pkgsrc/distfiles" -func (src *Pkgsrc) File(relativeName Path) Path { - cleaned := relativeName.Clean() +func (src *Pkgsrc) File(relativeName PkgsrcPath) CurrPath { + cleaned := relativeName.AsPath().Clean() if cleaned == "." { return src.topdir.CleanDot() } @@ -1127,23 +1119,23 @@ func (src *Pkgsrc) File(relativeName Pat // // Example: // NewPkgsrc("/usr/pkgsrc").ToRel("/usr/pkgsrc/distfiles") => "distfiles" -func (src *Pkgsrc) ToRel(filename Path) Path { - return src.Relpath(src.topdir, filename) +func (src *Pkgsrc) ToRel(filename CurrPath) PkgsrcPath { + return NewPkgsrcPath(src.Relpath(src.topdir, filename)) } -// IsInfra returns whether the given filename (relative to the current -// working directory) is part of the pkgsrc infrastructure. -func (src *Pkgsrc) IsInfra(filename Path) bool { +// IsInfra returns whether the given filename is part of the pkgsrc +// infrastructure. +func (src *Pkgsrc) IsInfra(filename CurrPath) bool { rel := src.ToRel(filename) return rel.HasPrefixPath("mk") || rel.HasPrefixPath("wip/mk") } -func (src *Pkgsrc) IsInfraMain(filename Path) bool { +func (src *Pkgsrc) IsInfraMain(filename CurrPath) bool { rel := src.ToRel(filename) return rel.HasPrefixPath("mk") } -func (src *Pkgsrc) IsWip(filename Path) bool { +func (src *Pkgsrc) IsWip(filename CurrPath) bool { rel := src.ToRel(filename) return rel.HasPrefixPath("wip") } @@ -1152,7 +1144,7 @@ func (src *Pkgsrc) IsWip(filename Path) type Change struct { Location Location Action ChangeAction // Added, Updated, Downgraded, Renamed, Moved, Removed - Pkgpath Path // For renamed or moved packages, the previous PKGPATH + Pkgpath PkgsrcPath // For renamed or moved packages, the previous PKGPATH target string // The path or version number, depending on the action Author string Date string @@ -1165,9 +1157,9 @@ func (ch *Change) Version() string { } // Target returns the target PKGPATH for a Renamed or Moved package. -func (ch *Change) Target() Path { +func (ch *Change) Target() PkgsrcPath { assert(ch.Action == Renamed || ch.Action == Moved) - return NewPath(ch.target) + return NewPkgsrcPath(NewPath(ch.target)) } // Successor returns the successor for a Removed package. Index: pkgsrc/pkgtools/pkglint/files/plist.go diff -u pkgsrc/pkgtools/pkglint/files/plist.go:1.45 pkgsrc/pkgtools/pkglint/files/plist.go:1.46 --- pkgsrc/pkgtools/pkglint/files/plist.go:1.45 Sat Nov 23 23:35:56 2019 +++ pkgsrc/pkgtools/pkglint/files/plist.go Mon Dec 2 23:32:09 2019 @@ -111,12 +111,15 @@ func (ck *PlistChecker) collectFilesAndD if prev := ck.allFiles[path]; prev == nil || stringSliceLess(pline.conditions, prev.conditions) { ck.allFiles[path] = pline } - for dir := path.Dir(); dir != "."; dir = dir.Dir() { + // FIXME: consider DirNoClean + // FIXME: consider DirNoClean + for dir := path.DirClean(); dir != "."; dir = dir.DirClean() { ck.allDirs[dir] = pline } case first == '@': if m, dirname := match1(text, `^@exec \$\{MKDIR\} %D/(.*)$`); m { - for dir := NewPath(dirname); dir != "."; dir = dir.Dir() { + // FIXME: consider DirNoClean + for dir := NewPath(dirname); dir != "."; dir = dir.DirClean() { ck.allDirs[dir] = pline } } @@ -560,8 +563,8 @@ func (s *plistLineSorter) Sort() { mi := s.middle[i] mj := s.middle[j] less := mi.text < mj.text || - (mi.text == mj.text && stringSliceLess(mi.conditions, mj.conditions)) - if (i < j) != less { + mi.text == mj.text && stringSliceLess(mi.conditions, mj.conditions) + if i < j != less { s.changed = true } return less Index: pkgsrc/pkgtools/pkglint/files/redundantscope.go diff -u pkgsrc/pkgtools/pkglint/files/redundantscope.go:1.8 pkgsrc/pkgtools/pkglint/files/redundantscope.go:1.9 --- pkgsrc/pkgtools/pkglint/files/redundantscope.go:1.8 Sat Nov 23 23:35:56 2019 +++ pkgsrc/pkgtools/pkglint/files/redundantscope.go Mon Dec 2 23:32:09 2019 @@ -223,14 +223,14 @@ func (s *RedundantScope) onOverwrite(ove // one of two variable assignments is redundant. Two assignments can // only be redundant if one location includes the other. type includePath struct { - files []Path + files []CurrPath } -func (p *includePath) push(filename Path) { +func (p *includePath) push(filename CurrPath) { p.files = append(p.files, filename) } -func (p *includePath) popUntil(filename Path) { +func (p *includePath) popUntil(filename CurrPath) { for p.files[len(p.files)-1] != filename { p.files = p.files[:len(p.files)-1] } @@ -276,5 +276,5 @@ func (p *includePath) equals(other inclu } func (p *includePath) copy() includePath { - return includePath{append([]Path(nil), p.files...)} + return includePath{append([]CurrPath(nil), p.files...)} } Index: pkgsrc/pkgtools/pkglint/files/redundantscope_test.go diff -u pkgsrc/pkgtools/pkglint/files/redundantscope_test.go:1.8 pkgsrc/pkgtools/pkglint/files/redundantscope_test.go:1.9 --- pkgsrc/pkgtools/pkglint/files/redundantscope_test.go:1.8 Sat Nov 23 23:35:56 2019 +++ pkgsrc/pkgtools/pkglint/files/redundantscope_test.go Mon Dec 2 23:32:09 2019 @@ -1525,7 +1525,7 @@ func (s *Suite) Test_RedundantScope_hand func (s *Suite) Test_includePath_includes(c *check.C) { t := s.Init(c) - path := func(locations ...Path) includePath { + path := func(locations ...CurrPath) includePath { return includePath{locations} } @@ -1550,7 +1550,7 @@ func (s *Suite) Test_includePath_include func (s *Suite) Test_includePath_equals(c *check.C) { t := s.Init(c) - path := func(locations ...Path) includePath { + path := func(locations ...CurrPath) includePath { return includePath{locations} } Index: pkgsrc/pkgtools/pkglint/files/substcontext_test.go diff -u pkgsrc/pkgtools/pkglint/files/substcontext_test.go:1.30 pkgsrc/pkgtools/pkglint/files/substcontext_test.go:1.31 --- pkgsrc/pkgtools/pkglint/files/substcontext_test.go:1.30 Sun Nov 17 01:26:25 2019 +++ pkgsrc/pkgtools/pkglint/files/substcontext_test.go Mon Dec 2 23:32:09 2019 @@ -301,16 +301,16 @@ func (s *Suite) Test_SubstContext__neste func (s *Suite) Test_SubstContext__pre_patch(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wextra,no-space", "--show-autofix") + t.SetUpCommandLine("-Wextra", "--show-autofix") t.SetUpVartypes() mklines := t.NewMkLines("os.mk", MkCvsID, "", - "SUBST_CLASSES+= os", - "SUBST_STAGE.os= pre-patch", - "SUBST_FILES.os= guess-os.h", - "SUBST_SED.os= -e s,@OPSYS@,Darwin,") + "SUBST_CLASSES+=\tos", + "SUBST_STAGE.os=\tpre-patch", + "SUBST_FILES.os=\tguess-os.h", + "SUBST_SED.os=\t-e s,@OPSYS@,Darwin,") mklines.Check() @@ -322,16 +322,16 @@ func (s *Suite) Test_SubstContext__pre_p func (s *Suite) Test_SubstContext__post_patch(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wextra,no-space", "--show-autofix") + t.SetUpCommandLine("-Wextra", "--show-autofix") t.SetUpVartypes() mklines := t.NewMkLines("os.mk", MkCvsID, "", - "SUBST_CLASSES+= os", - "SUBST_STAGE.os= post-patch", - "SUBST_FILES.os= guess-os.h", - "SUBST_SED.os= -e s,@OPSYS@,Darwin,") + "SUBST_CLASSES+=\tos", + "SUBST_STAGE.os=\tpost-patch", + "SUBST_FILES.os=\tguess-os.h", + "SUBST_SED.os=\t-e s,@OPSYS@,Darwin,") mklines.Check() @@ -343,24 +343,23 @@ func (s *Suite) Test_SubstContext__post_ func (s *Suite) Test_SubstContext__with_NO_CONFIGURE(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wall,no-space") pkg := t.SetUpPackage("category/package", - "SUBST_CLASSES+= pre", - "SUBST_STAGE.pre= pre-configure", - "SUBST_FILES.pre= guess-os.h", - "SUBST_SED.pre= -e s,@OPSYS@,Darwin,", - "", - "SUBST_CLASSES+= post", - "SUBST_STAGE.post= post-configure", - "SUBST_FILES.post= guess-os.h", - "SUBST_SED.post= -e s,@OPSYS@,Darwin,", - "", - "SUBST_CLASSES+= e", - "SUBST_STAGE.e= post-extract", - "SUBST_FILES.e= guess-os.h", - "SUBST_SED.e= -e s,@OPSYS@,Darwin,", + "SUBST_CLASSES+=\t\tpre", + "SUBST_STAGE.pre=\tpre-configure", + "SUBST_FILES.pre=\tguess-os.h", + "SUBST_SED.pre=\t\t-e s,@OPSYS@,Darwin,", + "", + "SUBST_CLASSES+=\t\tpost", + "SUBST_STAGE.post=\tpost-configure", + "SUBST_FILES.post=\tguess-os.h", + "SUBST_SED.post=\t\t-e s,@OPSYS@,Darwin,", + "", + "SUBST_CLASSES+=\te", + "SUBST_STAGE.e=\tpost-extract", + "SUBST_FILES.e=\tguess-os.h", + "SUBST_SED.e=\t-e s,@OPSYS@,Darwin,", "", - "NO_CONFIGURE= yes") + "NO_CONFIGURE=\tyes") t.FinishSetUp() G.Check(pkg) @@ -375,12 +374,11 @@ func (s *Suite) Test_SubstContext__with_ func (s *Suite) Test_SubstContext__without_NO_CONFIGURE(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wall,no-space") pkg := t.SetUpPackage("category/package", - "SUBST_CLASSES+= pre", - "SUBST_STAGE.pre= pre-configure", - "SUBST_FILES.pre= guess-os.h", - "SUBST_SED.pre= -e s,@OPSYS@,Darwin,") + "SUBST_CLASSES+=\t\tpre", + "SUBST_STAGE.pre=\tpre-configure", + "SUBST_FILES.pre=\tguess-os.h", + "SUBST_SED.pre=\t\t-e s,@OPSYS@,Darwin,") t.FinishSetUp() G.Check(pkg) @@ -397,15 +395,15 @@ func (s *Suite) Test_SubstContext__adjac mklines := t.NewMkLines("os.mk", MkCvsID, "", - "SUBST_CLASSES+= 1", - "SUBST_STAGE.1= pre-configure", - "SUBST_FILES.1= file1", - "SUBST_SED.1= -e s,subst1,repl1,", - "SUBST_CLASSES+= 2", - "SUBST_SED.1+= -e s,subst1b,repl1b,", // Misplaced - "SUBST_STAGE.2= pre-configure", - "SUBST_FILES.2= file2", - "SUBST_SED.2= -e s,subst2,repl2,") + "SUBST_CLASSES+=\t1", + "SUBST_STAGE.1=\tpre-configure", + "SUBST_FILES.1=\tfile1", + "SUBST_SED.1=\t-e s,subst1,repl1,", + "SUBST_CLASSES+=\t2", + "SUBST_SED.1+=\t-e s,subst1b,repl1b,", // Misplaced + "SUBST_STAGE.2=\tpre-configure", + "SUBST_FILES.2=\tfile2", + "SUBST_SED.2=\t-e s,subst2,repl2,") mklines.Check() @@ -416,16 +414,15 @@ func (s *Suite) Test_SubstContext__adjac func (s *Suite) Test_SubstContext__do_patch(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wextra,no-space") t.SetUpVartypes() mklines := t.NewMkLines("os.mk", MkCvsID, "", - "SUBST_CLASSES+= os", - "SUBST_STAGE.os= do-patch", - "SUBST_FILES.os= guess-os.h", - "SUBST_SED.os= -e s,@OPSYS@,Darwin,") + "SUBST_CLASSES+=\tos", + "SUBST_STAGE.os=\tdo-patch", + "SUBST_FILES.os=\tguess-os.h", + "SUBST_SED.os=\t-e s,@OPSYS@,Darwin,") mklines.Check() @@ -439,18 +436,17 @@ func (s *Suite) Test_SubstContext__do_pa func (s *Suite) Test_SubstContext__SUBST_VARS_defined_in_block(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wextra,no-space") t.SetUpVartypes() mklines := t.NewMkLines("os.mk", MkCvsID, "", - "SUBST_CLASSES+= os", - "SUBST_STAGE.os= pre-configure", - "SUBST_FILES.os= guess-os.h", - "SUBST_VARS.os= TODAY1", - "TODAY1!= date", - "TODAY2!= date") + "SUBST_CLASSES+=\tos", + "SUBST_STAGE.os=\tpre-configure", + "SUBST_FILES.os=\tguess-os.h", + "SUBST_VARS.os=\tTODAY1", + "TODAY1!=\tdate", + "TODAY2!=\tdate") mklines.Check() @@ -464,19 +460,18 @@ func (s *Suite) Test_SubstContext__SUBST func (s *Suite) Test_SubstContext__SUBST_VARS_in_next_paragraph(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wextra,no-space") t.SetUpVartypes() mklines := t.NewMkLines("os.mk", MkCvsID, "", - "SUBST_CLASSES+= os", - "SUBST_STAGE.os= pre-configure", - "SUBST_FILES.os= guess-os.h", - "SUBST_VARS.os= TODAY1", + "SUBST_CLASSES+=\tos", + "SUBST_STAGE.os=\tpre-configure", + "SUBST_FILES.os=\tguess-os.h", + "SUBST_VARS.os=\tTODAY1", "", - "TODAY1!= date", - "TODAY2!= date") + "TODAY1!=\tdate", + "TODAY2!=\tdate") mklines.Check() @@ -487,16 +482,16 @@ func (s *Suite) Test_SubstContext__SUBST func (s *Suite) Test_SubstContext__multiple_SUBST_VARS(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wextra,no-space") + t.SetUpCommandLine("-Wextra") t.SetUpVartypes() mklines := t.NewMkLines("os.mk", MkCvsID, "", - "SUBST_CLASSES+= os", - "SUBST_STAGE.os= pre-configure", - "SUBST_FILES.os= guess-os.h", - "SUBST_VARS.os= PREFIX VARBASE") + "SUBST_CLASSES+=\tos", + "SUBST_STAGE.os=\tpre-configure", + "SUBST_FILES.os=\tguess-os.h", + "SUBST_VARS.os=\tPREFIX VARBASE") mklines.Check() @@ -530,7 +525,6 @@ func (s *Suite) Test_SubstContext__unusu func (s *Suite) Test_SubstContext_Directive__before_SUBST_CLASSES(c *check.C) { t := s.Init(c) - t.SetUpCommandLine("-Wextra,no-space") t.SetUpVartypes() t.DisableTracing() // Just for branch coverage. @@ -539,7 +533,7 @@ func (s *Suite) Test_SubstContext_Direct "", ".if 0", ".endif", - "SUBST_CLASSES+= os", + "SUBST_CLASSES+=\tos", ".elif 0") // Just for branch coverage. mklines.Check() Index: pkgsrc/pkgtools/pkglint/files/tools_test.go diff -u pkgsrc/pkgtools/pkglint/files/tools_test.go:1.22 pkgsrc/pkgtools/pkglint/files/tools_test.go:1.23 --- pkgsrc/pkgtools/pkglint/files/tools_test.go:1.22 Sat Nov 23 23:35:56 2019 +++ pkgsrc/pkgtools/pkglint/files/tools_test.go Mon Dec 2 23:32:09 2019 @@ -207,7 +207,6 @@ func (s *Suite) Test_Tools__builtin_mk(c t := s.Init(c) t.SetUpPkgsrc() - t.SetUpCommandLine("-Wall,no-space") t.CreateFileLines("mk/tools/defaults.mk", "TOOLS_CREATE+= load", "TOOLS_CREATE+= run", @@ -228,19 +227,19 @@ func (s *Suite) Test_Tools__builtin_mk(c mklines := t.SetUpFileMkLines("category/package/builtin.mk", MkCvsID, "", - "VAR!= ${ECHO} 'too early'", - "VAR!= ${LOAD} 'too early'", - "VAR!= ${RUN_CMD} 'never allowed'", - "VAR!= ${NOWHERE} 'never allowed'", + "VAR!=\t${ECHO} 'too early'", + "VAR!=\t${LOAD} 'too early'", + "VAR!=\t${RUN_CMD} 'never allowed'", + "VAR!=\t${NOWHERE} 'never allowed'", "", ".include \"../../mk/buildlink3/bsd.builtin.mk\"", "", - "VAR!= ${ECHO} 'valid'", - "VAR!= ${LOAD} 'valid'", - "VAR!= ${RUN_CMD} 'never allowed'", - "VAR!= ${NOWHERE} 'never allowed'", + "VAR!=\t${ECHO} 'valid'", + "VAR!=\t${LOAD} 'valid'", + "VAR!=\t${RUN_CMD} 'never allowed'", + "VAR!=\t${NOWHERE} 'never allowed'", "", - "VAR!= ${VAR}") + "VAR!=\t${VAR}") mklines.Check() @@ -257,7 +256,6 @@ func (s *Suite) Test_Tools__implicit_def t := s.Init(c) t.SetUpPkgsrc() - t.SetUpCommandLine("-Wall,no-space") t.CreateFileLines("mk/tools/defaults.mk", MkCvsID) // None t.CreateFileLines("mk/bsd.prefs.mk", @@ -278,7 +276,6 @@ func (s *Suite) Test_Tools__both_prefs_a t := s.Init(c) t.SetUpPkgsrc() - t.SetUpCommandLine("-Wall,no-space") t.CreateFileLines("mk/tools/defaults.mk", MkCvsID) t.CreateFileLines("mk/bsd.prefs.mk", @@ -297,7 +294,6 @@ func (s *Suite) Test_Tools__tools_having t := s.Init(c) t.SetUpPkgsrc() - t.SetUpCommandLine("-Wall,no-space") t.CreateFileLines("mk/tools/defaults.mk", "_TOOLS_VARNAME.awk= AWK", "_TOOLS_VARNAME.gawk= AWK", Index: pkgsrc/pkgtools/pkglint/files/toplevel.go diff -u pkgsrc/pkgtools/pkglint/files/toplevel.go:1.25 pkgsrc/pkgtools/pkglint/files/toplevel.go:1.26 --- pkgsrc/pkgtools/pkglint/files/toplevel.go:1.25 Sat Nov 30 20:35:11 2019 +++ pkgsrc/pkgtools/pkglint/files/toplevel.go Mon Dec 2 23:32:09 2019 @@ -1,18 +1,18 @@ package pkglint type Toplevel struct { - dir Path + dir CurrPath previousSubdir Path - subdirs []Path + subdirs []CurrPath } -func CheckdirToplevel(dir Path) { +func CheckdirToplevel(dir CurrPath) { if trace.Tracing { defer trace.Call(dir)() } ctx := Toplevel{dir, "", nil} - filename := dir + "/Makefile" + filename := dir.JoinNoClean("Makefile") mklines := LoadMk(filename, NotEmpty|LogErrors) if mklines == nil { Index: pkgsrc/pkgtools/pkglint/files/vardefs_test.go diff -u pkgsrc/pkgtools/pkglint/files/vardefs_test.go:1.25 pkgsrc/pkgtools/pkglint/files/vardefs_test.go:1.26 --- pkgsrc/pkgtools/pkglint/files/vardefs_test.go:1.25 Wed Nov 27 22:10:07 2019 +++ pkgsrc/pkgtools/pkglint/files/vardefs_test.go Mon Dec 2 23:32:09 2019 @@ -148,7 +148,7 @@ func (s *Suite) Test_VarTypeRegistry_enu G.Pkgsrc.vartypes.enumFromDirs( &G.Pkgsrc, "category", `^pack.*`, "$0", "default") }, - "FATAL: category: Must contain at least 1 "+ + "FATAL: ~/category: Must contain at least 1 "+ "subdirectory matching \"^pack.*\".") } @@ -177,10 +177,10 @@ func (s *Suite) Test_VarTypeRegistry_enu t.ExpectFatal( func() { - G.Pkgsrc.vartypes.enumFromFiles( + G.Pkgsrc.vartypes.enumFromFiles(&G.Pkgsrc, "mk/platform", `^(\w+)\.mk$`, "$1", "default") }, - "FATAL: mk/platform: Must contain at least 1 "+ + "FATAL: ~/mk/platform: Must contain at least 1 "+ "file matching \"^(\\\\w+)\\\\.mk$\".") } Index: pkgsrc/pkgtools/pkglint/files/varalignblock.go diff -u pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.9 pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.10 --- pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.9 Wed Nov 27 22:10:07 2019 +++ pkgsrc/pkgtools/pkglint/files/varalignblock.go Mon Dec 2 23:32:09 2019 @@ -169,8 +169,6 @@ type varalignLine struct { func (va *VaralignBlock) Process(mkline *MkLine) { switch { - case !G.Opts.WarnSpace: - case mkline.IsEmpty(): va.Finish() @@ -299,7 +297,7 @@ func (*VaralignBlock) rightMargin(infos } } } - return (min & -8) + 8 + return min&-8 + 8 } // optimalWidth computes the desired screen width for the variable assignment @@ -333,7 +331,7 @@ func (*VaralignBlock) optimalWidth(infos // Widths of the current indentation (including whitespace) var spaceWidths mklineInts for _, info := range infos { - if info.multiEmpty || info.rawIndex > 0 || (outlier > 0 && info.varnameOpWidth() == outlier) { + if info.multiEmpty || info.rawIndex > 0 || outlier > 0 && info.varnameOpWidth() == outlier { continue } spaceWidths.append(info.mkline, info.varnameOpSpaceWidth()) @@ -362,7 +360,7 @@ func (*VaralignBlock) optimalWidth(infos return 0 } - return (minVarnameOpWidth & -8) + 8 + return minVarnameOpWidth&-8 + 8 } // adjustLong allows any follow-up line to start either in column 8 or at @@ -464,7 +462,7 @@ func (*VaralignBlock) realignMultiEmptyI if hasSpace && column != oldColumn { fix.Notef("This variable value should be aligned with tabs, not spaces, to column %d.", column+1) } else if column != oldColumn { - fix.Notef("This variable value should be aligned to column %d.", column+1) + fix.Notef("This variable value should be aligned to column %d.", column+1) // TODO: to column %d instead of %d. } else { fix.Notef("Variable values should be aligned with tabs, not spaces.") } Index: pkgsrc/pkgtools/pkglint/files/vardefs.go diff -u pkgsrc/pkgtools/pkglint/files/vardefs.go:1.79 pkgsrc/pkgtools/pkglint/files/vardefs.go:1.80 --- pkgsrc/pkgtools/pkglint/files/vardefs.go:1.79 Wed Nov 27 22:10:07 2019 +++ pkgsrc/pkgtools/pkglint/files/vardefs.go Mon Dec 2 23:32:09 2019 @@ -335,6 +335,7 @@ func (reg *VarTypeRegistry) compilerLang languages := make(map[string]bool) if mklines != nil { for _, mkline := range mklines.mklines { + if mkline.IsVarassign() && mkline.Varname() == "_CXX_STD_VERSIONS" { words := mkline.ValueFields(mkline.Value()) for _, word := range words { @@ -368,10 +369,13 @@ func (reg *VarTypeRegistry) compilerLang // and for all variables matching one of the varcanons, all values // are added as allowed values. // -// If the file cannot be found, the allowed values are taken from -// defval. This is mostly useful when testing pkglint. -func (reg *VarTypeRegistry) enumFrom(pkgsrc *Pkgsrc, filename Path, defval string, varcanons ...string) *BasicType { - mklines := pkgsrc.LoadMkExisting(filename) +// If the file is not found, the allowed values are taken from +// defval. This is only done in the pkglint tests. +func (reg *VarTypeRegistry) enumFrom( + src *Pkgsrc, filename PkgsrcPath, defval string, + varcanons ...string) *BasicType { + + mklines := src.LoadMkExisting(filename) if mklines == nil { return enum(defval) } @@ -422,13 +426,16 @@ func (reg *VarTypeRegistry) enumFrom(pkg // that have a single number in them (such as php72) and ranks them // from earliest to latest. // -// If the directories cannot be found, the allowed values are taken -// from defval. This is mostly useful when testing pkglint. -func (reg *VarTypeRegistry) enumFromDirs(pkgsrc *Pkgsrc, category Path, re regex.Pattern, repl string, defval string) *BasicType { - versions := pkgsrc.ListVersions(category, re, repl, false) +// If no directories are found, the allowed values are taken +// from defval. This is only done in the pkglint tests. +func (reg *VarTypeRegistry) enumFromDirs( + src *Pkgsrc, category PkgsrcPath, re regex.Pattern, repl string, + defval string) *BasicType { + + versions := src.ListVersions(category, re, repl, false) if len(versions) == 0 { if !G.Testing { - NewLineWhole(category).Fatalf( + NewLineWhole(src.File(category)).Fatalf( "Must contain at least 1 subdirectory matching %q.", re) } return enum(defval) @@ -441,10 +448,13 @@ func (reg *VarTypeRegistry) enumFromDirs // filtering it through the regular expression and the replacement. // // If no files are found, the allowed values are taken -// from defval. This should only happen in the pkglint tests. -func (reg *VarTypeRegistry) enumFromFiles(basedir Path, re regex.Pattern, repl string, defval string) *BasicType { +// from defval. This is only done in the pkglint tests. +func (reg *VarTypeRegistry) enumFromFiles( + src *Pkgsrc, basedir PkgsrcPath, re regex.Pattern, repl string, + defval string) *BasicType { + var relevant []string - for _, filename := range dirglob(G.Pkgsrc.File(basedir)) { + for _, filename := range src.File(basedir).ReadPaths() { basename := filename.Base() if matches(basename, re) { relevant = append(relevant, replaceAll(basename, re, repl)) @@ -452,7 +462,7 @@ func (reg *VarTypeRegistry) enumFromFile } if len(relevant) == 0 { if !G.Testing { - NewLineWhole(basedir).Fatalf( + NewLineWhole(src.File(basedir)).Fatalf( "Must contain at least 1 file matching %q.", re) } return enum(defval) @@ -492,6 +502,11 @@ func (reg *VarTypeRegistry) Init(src *Pk "openjdk8 oracle-jdk8 openjdk7 sun-jdk7 jdk16 jdk15 kaffe", "_PKG_JVMS.*") + platforms := reg.enumFromFiles(src, + "mk/platform", + `(.*)\.mk$`, "$1", + "Cygwin DragonFly FreeBSD Linux NetBSD SunOS") + // Last synced with mk/defaults/mk.conf revision 1.300 (fe3d998769f). reg.usr("USE_CWRAPPERS", enum("yes no auto")) reg.usr("ALLOW_VULNERABLE_PACKAGES", BtYes) @@ -1264,7 +1279,7 @@ func (reg *VarTypeRegistry) Init(src *Pk reg.sys("MANOWN", BtUserGroupName) reg.pkglist("MASTER_SITES", BtFetchURL) - for _, filename := range []Path{"mk/fetch/sites.mk", "mk/fetch/fetch.mk"} { + for _, filename := range []PkgsrcPath{"mk/fetch/sites.mk", "mk/fetch/fetch.mk"} { sitesMk := src.LoadMkExisting(filename) if sitesMk != nil { sitesMk.ForEach(func(mkline *MkLine) { @@ -1307,8 +1322,7 @@ func (reg *VarTypeRegistry) Init(src *Pk reg.pkglistrat("ONLY_FOR_COMPILER", compilers) reg.pkglistrat("ONLY_FOR_PLATFORM", BtMachinePlatformPattern) reg.pkgrat("ONLY_FOR_UNPRIVILEGED", BtYesNo) - reg.sysload("OPSYS", reg.enumFromFiles("mk/platform", `(.*)\.mk$`, "$1", - "Cygwin DragonFly FreeBSD Linux NetBSD SunOS")) + reg.sysload("OPSYS", platforms) reg.pkglistbl3("OPSYSVARS", BtVariableName) reg.pkg("OSVERSION_SPECIFIC", BtYes) reg.sysload("OS_VERSION", BtVersion) Index: pkgsrc/pkgtools/pkglint/files/vartype.go diff -u pkgsrc/pkgtools/pkglint/files/vartype.go:1.39 pkgsrc/pkgtools/pkglint/files/vartype.go:1.40 --- pkgsrc/pkgtools/pkglint/files/vartype.go:1.39 Sun Nov 17 01:26:25 2019 +++ pkgsrc/pkgtools/pkglint/files/vartype.go Mon Dec 2 23:32:09 2019 @@ -349,9 +349,9 @@ var ( BtRelativePkgPath = &BasicType{"RelativePkgPath", (*VartypeCheck).RelativePkgPath} BtRestricted = &BasicType{"Restricted", (*VartypeCheck).Restricted} BtSedCommands = &BasicType{"SedCommands", (*VartypeCheck).SedCommands} - BtShellCommand = &BasicType{"ShellCommand", nil} - BtShellCommands = &BasicType{"ShellCommands", nil} - BtShellWord = &BasicType{"ShellWord", nil} + BtShellCommand = &BasicType{"ShellCommand", nil} // see func init below + BtShellCommands = &BasicType{"ShellCommands", nil} // see func init below + BtShellWord = &BasicType{"ShellWord", nil} // see func init below BtStage = &BasicType{"Stage", (*VartypeCheck).Stage} BtTool = &BasicType{"Tool", (*VartypeCheck).Tool} BtUnknown = &BasicType{"Unknown", (*VartypeCheck).Unknown} Index: pkgsrc/pkgtools/pkglint/files/vartypecheck.go diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.68 pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.69 --- pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.68 Wed Nov 27 22:10:07 2019 +++ pkgsrc/pkgtools/pkglint/files/vartypecheck.go Mon Dec 2 23:32:09 2019 @@ -225,18 +225,18 @@ func (cv *VartypeCheck) BuildlinkDepmeth } func (cv *VartypeCheck) Category() { - if cv.Value != "wip" && G.Pkgsrc.File(NewPath(cv.Value).JoinNoClean("Makefile")).IsFile() { + if cv.Value != "wip" && G.Pkgsrc.File(NewPkgsrcPath(NewPath(cv.Value)).JoinNoClean("Makefile")).IsFile() { return } switch cv.Value { case - "chinese", "crosspkgtools", + "chinese", "gnome", "gnustep", "japanese", "java", "kde", "korean", "linux", "local", - "packages", "perl5", "plan9", "python", + "perl5", "plan9", "python", "R", "ruby", "scm", "tcl", "tk", @@ -442,17 +442,17 @@ func (cv *VartypeCheck) DependencyWithPa parts := cv.MkLine.ValueSplit(value, ":") if len(parts) == 2 { pattern := parts[0] - relpath := parts[1] - pathParts := cv.MkLine.ValueSplit(relpath, "/") + relpath := NewRelPathString(parts[1]) + pathParts := relpath.Parts() pkg := pathParts[len(pathParts)-1] - if matches(relpath, `^\.\./[^.]`) { + if len(pathParts) >= 2 && pathParts[0] == ".." && pathParts[1] != ".." { cv.Warnf("Dependency paths should have the form \"../../category/package\".") cv.MkLine.ExplainRelativeDirs() } - if !containsVarRef(relpath) { - MkLineChecker{cv.MkLines, cv.MkLine}.CheckRelativePkgdir(NewPath(relpath)) + if !containsVarRef(relpath.String()) { + MkLineChecker{cv.MkLines, cv.MkLine}.CheckRelativePkgdir(relpath) } switch pkg { @@ -1076,7 +1076,7 @@ func (cv *VartypeCheck) Pkgpath() { return } - pkgpath := NewPath(value) + pkgpath := NewPkgsrcPath(NewPath(value)) if !G.Wip && pkgpath.HasPrefixPath("wip") { cv.MkLine.Errorf("A main pkgsrc package must not depend on a pkgsrc-wip package.") } @@ -1164,7 +1164,7 @@ func (cv *VartypeCheck) RPkgVer() { // RelativePkgDir refers to a package directory, e.g. ../../category/pkgbase. func (cv *VartypeCheck) RelativePkgDir() { - MkLineChecker{cv.MkLines, cv.MkLine}.CheckRelativePkgdir(NewPath(cv.Value)) + MkLineChecker{cv.MkLines, cv.MkLine}.CheckRelativePkgdir(NewRelPathString(cv.Value)) } // RelativePkgPath refers to a file or directory, e.g. ../../category/pkgbase, @@ -1172,7 +1172,7 @@ func (cv *VartypeCheck) RelativePkgDir() // // See RelativePkgDir, which requires a directory, not a file. func (cv *VartypeCheck) RelativePkgPath() { - MkLineChecker{cv.MkLines, cv.MkLine}.CheckRelativePath(NewPath(cv.Value), true) + MkLineChecker{cv.MkLines, cv.MkLine}.CheckRelativePath(NewRelPathString(cv.Value), true) } func (cv *VartypeCheck) Restricted() { Index: pkgsrc/pkgtools/pkglint/files/histogram/histogram.go diff -u pkgsrc/pkgtools/pkglint/files/histogram/histogram.go:1.4 pkgsrc/pkgtools/pkglint/files/histogram/histogram.go:1.5 --- pkgsrc/pkgtools/pkglint/files/histogram/histogram.go:1.4 Wed Nov 7 20:58:23 2018 +++ pkgsrc/pkgtools/pkglint/files/histogram/histogram.go Mon Dec 2 23:32:09 2019 @@ -35,7 +35,7 @@ func (h *Histogram) PrintStats(out io.Wr sort.SliceStable(entries, func(i, j int) bool { ei := entries[i] ej := entries[j] - return ej.count < ei.count || (ei.count == ej.count && ei.s < ej.s) + return ej.count < ei.count || ei.count == ej.count && ei.s < ej.s }) for i, entry := range entries { Index: pkgsrc/pkgtools/pkglint/files/intqa/qa.go diff -u pkgsrc/pkgtools/pkglint/files/intqa/qa.go:1.1 pkgsrc/pkgtools/pkglint/files/intqa/qa.go:1.2 --- pkgsrc/pkgtools/pkglint/files/intqa/qa.go:1.1 Wed Nov 27 22:10:07 2019 +++ pkgsrc/pkgtools/pkglint/files/intqa/qa.go Mon Dec 2 23:32:10 2019 @@ -43,7 +43,9 @@ const ( EMethodsSameFile ) -// QAChecker ensures that all test names follow a common naming scheme: +// QAChecker analyzes Go source code for consistency. +// +// Among others, it enforces a strict naming scheme for test methods: // Test_${Type}_${Method}__${description_using_underscores} // Each of the variable parts may be omitted. type QAChecker struct { @@ -90,8 +92,9 @@ func (ck *QAChecker) Configure(filenames func (ck *QAChecker) Check() { ck.load(".") - ck.checkTestees() + ck.checkTesteesTest() ck.checkTests() + ck.checkMethodsSameFile() ck.checkOrder() ck.print() } @@ -306,11 +309,6 @@ func (ck *QAChecker) checkTestDescr(test test.fullName(), test.descr) } -func (ck *QAChecker) checkTestees() { - ck.checkTesteesTest() - ck.checkTesteesMethodsSameFile() -} - // checkTesteesTest ensures that each testee has a corresponding unit test. func (ck *QAChecker) checkTesteesTest() { tested := make(map[*testee]bool) @@ -334,20 +332,24 @@ func (ck *QAChecker) checkTesteesTest() // checkTesteesMethodsSameFile ensures that all methods of a type are // defined in the same file or in the corresponding test file. -func (ck *QAChecker) checkTesteesMethodsSameFile() { - types := map[string]*testee{} +func (ck *QAChecker) checkMethodsSameFile() { + types := map[string]*code{} + var methods []*code + for _, testee := range ck.testees { if testee.isType() { - types[testee.Type] = testee + types[testee.Type] = &testee.code + } else if testee.isMethod() { + methods = append(methods, &testee.code) } } - - for _, testee := range ck.testees { - if !testee.isMethod() { - continue + for _, test := range ck.tests { + if test.isMethod() { + methods = append(methods, &test.code) } - method := testee + } + for _, method := range methods { typ := types[method.Type] if typ == nil || method.file == typ.file { continue @@ -356,9 +358,9 @@ func (ck *QAChecker) checkTesteesMethods if method.isTestScope() == typ.isTestScope() { ck.addError( EMethodsSameFile, - testee.code, + *method, "Method %s must be in %s, like its type.", - testee.fullName(), typ.file) + method.fullName(), typ.file) continue } @@ -369,9 +371,9 @@ func (ck *QAChecker) checkTesteesMethods ck.addError( EMethodsSameFile, - testee.code, + *method, "Method %s must be in %s, corresponding to its type.", - testee.fullName(), correctFile) + method.fullName(), correctFile) } } Index: pkgsrc/pkgtools/pkglint/files/intqa/qa_test.go diff -u pkgsrc/pkgtools/pkglint/files/intqa/qa_test.go:1.1 pkgsrc/pkgtools/pkglint/files/intqa/qa_test.go:1.2 --- pkgsrc/pkgtools/pkglint/files/intqa/qa_test.go:1.1 Wed Nov 27 22:10:07 2019 +++ pkgsrc/pkgtools/pkglint/files/intqa/qa_test.go Mon Dec 2 23:32:10 2019 @@ -293,6 +293,15 @@ func (s *Suite) Test_QAChecker_isTest(c testFunc("f_test.go", "func Test_Type_Method(t *testing.T) {}", true) testFunc("f_test.go", "func Test_Type_Method(X) {}", false) testFunc("f_test.go", "func Test_Type_Method(int) {}", false) + + // Having such a method in a test suite is unlikely since check.v1 + // checks the number of parameters. + testFunc("f_test.go", "func Test_Type_Method() {}", false) + testFunc("f_test.go", "func (s *Suite) Test(int, int) {}", false) + + // In a test helper type, it is reasonable that some of the method + // names start with "Test". This doesn't make them tests though. + testFunc("f_test.go", "func (h *Helper) TestEqInt(int, int) {}", false) } func (s *Suite) Test_QAChecker_addTestee(c *check.C) { @@ -437,18 +446,6 @@ func (s *Suite) Test_QAChecker_checkTest "must not use CamelCase in the first word.") } -func (s *Suite) Test_QAChecker_checkTestees(c *check.C) { - ck := s.Init(c) - - ck.testees = []*testee{s.newTestee("s.go", "", "Func", 0)} - ck.tests = nil // force an error - - ck.checkTestees() - - s.CheckErrors( - "Missing unit test \"Test_Func\" for \"Func\".") -} - func (s *Suite) Test_QAChecker_checkTesteesTest(c *check.C) { ck := s.Init(c) @@ -471,7 +468,7 @@ func (s *Suite) Test_QAChecker_checkTest "Missing unit test \"Test_Type_Method\" for \"Type.Method\".") } -func (s *Suite) Test_QAChecker_checkTesteesMethodsSameFile(c *check.C) { +func (s *Suite) Test_QAChecker_checkMethodsSameFile(c *check.C) { ck := s.Init(c) ck.addTestee(code{"main.go", "Main", "", 0}) @@ -482,14 +479,17 @@ func (s *Suite) Test_QAChecker_checkTest ck.addTestee(code{"main_test.go", "T", "", 100}) ck.addTestee(code{"main_test.go", "T", "MethodOk", 101}) ck.addTestee(code{"other_test.go", "T", "MethodWrong", 102}) + ck.addTest(code{"main_test.go", "T", "Test_MethodOk", 101}) + ck.addTest(code{"other_test.go", "T", "Test_MethodWrong", 102}) - ck.checkTesteesMethodsSameFile() + ck.checkMethodsSameFile() s.CheckErrors( "Method Main.MethodWrong must be in main.go, like its type.", "Method Main.MethodWrongTest must be in main_test.go, "+ "corresponding to its type.", - "Method T.MethodWrong must be in main_test.go, like its type.") + "Method T.MethodWrong must be in main_test.go, like its type.", + "Method T.Test_MethodWrong must be in main_test.go, like its type.") } func (s *Suite) Test_QAChecker_errorsMask(c *check.C) { Index: pkgsrc/pkgtools/pkglint/files/textproc/lexer_bench_test.go diff -u pkgsrc/pkgtools/pkglint/files/textproc/lexer_bench_test.go:1.1 pkgsrc/pkgtools/pkglint/files/textproc/lexer_bench_test.go:1.2 --- pkgsrc/pkgtools/pkglint/files/textproc/lexer_bench_test.go:1.1 Sun Dec 2 01:57:48 2018 +++ pkgsrc/pkgtools/pkglint/files/textproc/lexer_bench_test.go Mon Dec 2 23:32:10 2019 @@ -181,7 +181,7 @@ func Benchmark_Direct_Compare_fold_case( var sum int for i := 0; i < b.N; i++ { i := byte(i) - if 'A' <= (i&^0x20) && (i&^0x20) <= 'Z' || '0' <= i && i <= '9' || i == '_' { + if 'A' <= i&^0x20 && i&^0x20 <= 'Z' || '0' <= i && i <= '9' || i == '_' { sum++ } } Index: pkgsrc/pkgtools/pkglint/files/trace/tracing_test.go diff -u pkgsrc/pkgtools/pkglint/files/trace/tracing_test.go:1.6 pkgsrc/pkgtools/pkglint/files/trace/tracing_test.go:1.7 --- pkgsrc/pkgtools/pkglint/files/trace/tracing_test.go:1.6 Wed Nov 27 22:10:08 2019 +++ pkgsrc/pkgtools/pkglint/files/trace/tracing_test.go Mon Dec 2 23:32:10 2019 @@ -93,16 +93,28 @@ func (s *Suite) Test__fixed_argument_var "TRACE: - netbsd.org/pkglint/trace.(*Suite).Test__fixed_argument_variants.func1()\n") } -func (s *Suite) Test__stringer_arg(c *check.C) { +func (s *Suite) Test_Tracer_Call__Stringer_arg(c *check.C) { tracer := &s.Tracer output := s.captureTracingOutput(func() { - defer tracer.Call(str{}, &str{})() + defer tracer.Call(stringer{}, &stringer{})() }) c.Check(output, check.Equals, ""+ - "TRACE: + netbsd.org/pkglint/trace.(*Suite).Test__stringer_arg.func1(It's a string, It's a string)\n"+ - "TRACE: - netbsd.org/pkglint/trace.(*Suite).Test__stringer_arg.func1(It's a string, It's a string)\n") + "TRACE: + netbsd.org/pkglint/trace.(*Suite).Test_Tracer_Call__Stringer_arg.func1(It's a string, It's a string)\n"+ + "TRACE: - netbsd.org/pkglint/trace.(*Suite).Test_Tracer_Call__Stringer_arg.func1(It's a string, It's a string)\n") +} + +func (s *Suite) Test_Tracer_Call__GoStringer_arg(c *check.C) { + tracer := &s.Tracer + + output := s.captureTracingOutput(func() { + defer tracer.Call(goStringer{}, &goStringer{})() + }) + + c.Check(output, check.Equals, ""+ + "TRACE: + netbsd.org/pkglint/trace.(*Suite).Test_Tracer_Call__GoStringer_arg.func1(\"It's a string\", \"It's a string\")\n"+ + "TRACE: - netbsd.org/pkglint/trace.(*Suite).Test_Tracer_Call__GoStringer_arg.func1(\"It's a string\", \"It's a string\")\n") } func (s *Suite) Test_Tracer_traceCall__panic(c *check.C) { @@ -137,14 +149,16 @@ func (s *Suite) captureTracingOutput(act return out.String() } -type str struct{} +type stringer struct{} -func (str) String() string { - return "It's a string" -} +func (stringer) String() string { return "It's a string" } + +type goStringer struct{} + +func (goStringer) GoString() string { return "\"It's a string\"" } -func (s *Suite) Test__qa(c *check.C) { - ck := intqa.NewQAChecker(c.Errorf) +func Test__qa(t *testing.T) { + ck := intqa.NewQAChecker(t.Errorf) ck.Configure("*", "*", "*", -intqa.EMissingTest) ck.Check() } --_----------=_157532953163210--