diff options
| author | rillig <rillig@pkgsrc.org> | 2018-12-21 08:05:24 +0000 |
|---|---|---|
| committer | rillig <rillig@pkgsrc.org> | 2018-12-21 08:05:24 +0000 |
| commit | bc8ff39f801ab1097b4cbc3adc836a04bac8a04e (patch) | |
| tree | 920eaee6b7e03def523a515e00d7455dbfb49f76 /pkgtools | |
| parent | d2314bc8f9e1ac341808312cdfd121696af00564 (diff) | |
| download | pkgsrc-bc8ff39f801ab1097b4cbc3adc836a04bac8a04e.tar.gz | |
pkgtools/pkglint: update to 5.6.10
Changes since 5.6.9:
* ALTERNATIVES files are correctly checked now. Before, pkglint had
suggested to remove the @PREFIX/ from the alternative, which was
wrong and simply didn't work.
* Diagnostics about variable assignments are ordered to report the
left-hand side first and then everything to the right of the
assignment operator.
* The pkglint output is escaped properly to avoid sending unwanted
escape sequences to the terminal.
* The items in .for loops are parsed taking "double" and 'single'
quotes into account since bmake does it in the same way since 2015.
* In DESCR files, overly long lines are only warned about if they
contain a space and therefore can be made shorter.
* In DESCR files, text like ${PREFIX} only gets a note if it refers
to a commonly known pkgsrc variable. This avoids distraction when
a package mentions ${prefix}/bin or ${template.property}.
* Lots of refactorings and small changes.
Diffstat (limited to 'pkgtools')
48 files changed, 694 insertions, 601 deletions
diff --git a/pkgtools/pkglint/files/alternatives.go b/pkgtools/pkglint/files/alternatives.go index 3fd6397a7ec..d7485f4d13e 100644 --- a/pkgtools/pkglint/files/alternatives.go +++ b/pkgtools/pkglint/files/alternatives.go @@ -1,8 +1,11 @@ package pkglint -import "strings" +import ( + "netbsd.org/pkglint/textproc" + "strings" +) -func CheckfileAlternatives(filename string) { +func CheckFileAlternatives(filename string) { lines := Load(filename, NotEmpty|LogErrors) if lines == nil { return @@ -13,35 +16,64 @@ func CheckfileAlternatives(filename string) { plist = G.Pkg.Plist } + checkPlistWrapper := func(line Line, wrapper string) { + if plist.Files[wrapper] { + line.Errorf("Alternative wrapper %q must not appear in the PLIST.", wrapper) + } + } + + checkPlistAlternative := func(line Line, alternative string) { + relImplementation := strings.Replace(alternative, "@PREFIX@/", "", 1) + plistName := replaceAll(relImplementation, `@(\w+)@`, "${$1}") + if plist.Files[plistName] || G.Pkg.vars.Defined("ALTERNATIVES_SRC") { + return + } + + switch { + + case hasPrefix(alternative, "/"): + // It's possible but unusual to refer to a fixed absolute path. + // These cannot be mentioned in the PLIST since they are not part of the package. + break + + case plistName == alternative: + line.Errorf("Alternative implementation %q must appear in the PLIST.", alternative) + + default: + line.Errorf("Alternative implementation %q must appear in the PLIST as %q.", alternative, plistName) + } + } + for _, line := range lines.Lines { - if m, wrapper, space, alternative := match3(line.Text, `^([^\t ]+)([ \t]+)([^\t ]+)`); m { - if plist.Files != nil { - if plist.Files[wrapper] { - line.Errorf("Alternative wrapper %q must not appear in the PLIST.", wrapper) - } - - relImplementation := strings.Replace(alternative, "@PREFIX@/", "", 1) - plistName := replaceAll(relImplementation, `@(\w+)@`, "${$1}") - if !plist.Files[plistName] && !G.Pkg.vars.Defined("ALTERNATIVES_SRC") { - if plistName != alternative { - line.Errorf("Alternative implementation %q must appear in the PLIST as %q.", alternative, plistName) - } else { - line.Errorf("Alternative implementation %q must appear in the PLIST.", alternative) - } - } - } + m, wrapper, space, alternative := match3(line.Text, `^([^\t ]+)([ \t]+)([^\t ]+)$`) + if !m { + line.Errorf("Invalid line %q.", line.Text) + G.Explain( + sprintf("Run %q for more information.", makeHelp("alternatives"))) + continue + } + + if plist.Files != nil { + checkPlistWrapper(line, wrapper) + checkPlistAlternative(line, alternative) + } + + switch { + case hasPrefix(alternative, "/"), hasPrefix(alternative, "@"): + break + case textproc.NewLexer(alternative).NextByteSet(textproc.Alnum) != -1: fix := line.Autofix() - fix.Notef("@PREFIX@/ can be omitted from the filename.") + fix.Errorf("Alternative implementation %q must be an absolute path.", alternative) fix.Explain( - "The alternative implementation is always interpreted relative to", - "${PREFIX}.") - fix.ReplaceAfter(space, "@PREFIX@/", "") + "It usually starts with @PREFIX@/... to refer to a path inside the installation prefix.") + fix.ReplaceAfter(space, alternative, "@PREFIX@/"+alternative) fix.Apply() - } else { - line.Errorf("Invalid ALTERNATIVES line %q.", line.Text) - G.Explain( - sprintf("Run %q for more information.", makeHelp("alternatives"))) + + default: + line.Errorf("Alternative implementation %q must be an absolute path.", alternative) + line.Explain( + "It usually starts with @PREFIX@/... to refer to a path inside the installation prefix.") } } } diff --git a/pkgtools/pkglint/files/alternatives_test.go b/pkgtools/pkglint/files/alternatives_test.go index 59d0021bf7b..a91baf0cf45 100644 --- a/pkgtools/pkglint/files/alternatives_test.go +++ b/pkgtools/pkglint/files/alternatives_test.go @@ -2,7 +2,7 @@ package pkglint import "gopkg.in/check.v1" -func (s *Suite) Test_CheckfileAlternatives__PLIST(c *check.C) { +func (s *Suite) Test_CheckFileAlternatives__PLIST(c *check.C) { t := s.Init(c) t.SetupPackage("category/package") @@ -12,26 +12,40 @@ func (s *Suite) Test_CheckfileAlternatives__PLIST(c *check.C) { "sbin/sendmail @PREFIX@/sbin/sendmail.exim@EXIMVER@", "bin/echo bin/gnu-echo", "bin/editor bin/vim -e", - "invalid") + "invalid", + "bin/browser\t${PREFIX}/bin/firefox", + "highscores @VARBASE@/game/scores", + "sbin/init /sbin/init") t.CreateFileLines("PLIST", PlistRcsID, "bin/echo", "bin/vim", "sbin/sendmail.exim${EXIMVER}") - G.CheckDirent(".") + G.Check(".") t.CheckOutputLines( "ERROR: ALTERNATIVES:1: Alternative implementation \"@PREFIX@/sbin/sendmail.postfix@POSTFIXVER@\" "+ "must appear in the PLIST as \"sbin/sendmail.postfix${POSTFIXVER}\".", - "NOTE: ALTERNATIVES:1: @PREFIX@/ can be omitted from the filename.", - "NOTE: ALTERNATIVES:2: @PREFIX@/ can be omitted from the filename.", "ERROR: ALTERNATIVES:3: Alternative wrapper \"bin/echo\" must not appear in the PLIST.", "ERROR: ALTERNATIVES:3: Alternative implementation \"bin/gnu-echo\" must appear in the PLIST.", - "ERROR: ALTERNATIVES:5: Invalid ALTERNATIVES line \"invalid\".") + "ERROR: ALTERNATIVES:3: Alternative implementation \"bin/gnu-echo\" must be an absolute path.", + "ERROR: ALTERNATIVES:4: Invalid line \"bin/editor bin/vim -e\".", + "ERROR: ALTERNATIVES:5: Invalid line \"invalid\".", + "ERROR: ALTERNATIVES:6: Alternative implementation \"${PREFIX}/bin/firefox\" must appear in the PLIST.", + "ERROR: ALTERNATIVES:6: Alternative implementation \"${PREFIX}/bin/firefox\" must be an absolute path.", + "ERROR: ALTERNATIVES:7: Alternative implementation \"@VARBASE@/game/scores\" "+ + "must appear in the PLIST as \"${VARBASE}/game/scores\".") + + t.SetupCommandLine("--autofix") + + G.Check(".") + + t.CheckOutputLines( + "AUTOFIX: ALTERNATIVES:3: Replacing \"bin/gnu-echo\" with \"@PREFIX@/bin/gnu-echo\".") } -func (s *Suite) Test_CheckfileAlternatives__empty(c *check.C) { +func (s *Suite) Test_CheckFileAlternatives__empty(c *check.C) { t := s.Init(c) t.Chdir("category/package") @@ -39,7 +53,7 @@ func (s *Suite) Test_CheckfileAlternatives__empty(c *check.C) { G.Pkg = NewPackage(".") - CheckfileAlternatives("ALTERNATIVES") + CheckFileAlternatives("ALTERNATIVES") t.CheckOutputLines( "ERROR: ALTERNATIVES: Must not be empty.") diff --git a/pkgtools/pkglint/files/autofix_test.go b/pkgtools/pkglint/files/autofix_test.go index 529b80ba1a1..8c947119423 100644 --- a/pkgtools/pkglint/files/autofix_test.go +++ b/pkgtools/pkglint/files/autofix_test.go @@ -865,8 +865,8 @@ func (s *Suite) Test_Autofix__lonely_source(c *check.C) { G.Pkgsrc.LoadInfrastructure() t.Chdir(".") - G.CheckDirent("x11/xorg-cf-files") - G.CheckDirent("x11/xorgproto") + G.Check("x11/xorg-cf-files") + G.Check("x11/xorgproto") t.CheckOutputLines( ">\tPRE_XORGPROTO_LIST_MISSING =\tapplewmproto", @@ -886,7 +886,7 @@ func (s *Suite) Test_Autofix__lonely_source_2(c *check.C) { G.Pkgsrc.LoadInfrastructure() t.Chdir(".") - G.CheckDirent("print/tex-bibtex8") + G.Check("print/tex-bibtex8") t.CheckOutputLines( ">\tMAKE_FLAGS+=\tCFLAGS=${CFLAGS.${PKGSRC_COMPILER}}", diff --git a/pkgtools/pkglint/files/buildlink3.go b/pkgtools/pkglint/files/buildlink3.go index 70d8071cb08..5994af2c85f 100644 --- a/pkgtools/pkglint/files/buildlink3.go +++ b/pkgtools/pkglint/files/buildlink3.go @@ -13,7 +13,7 @@ type Buildlink3Checker struct { abi, api *DependencyPattern } -func ChecklinesBuildlink3Mk(mklines MkLines) { +func CheckLinesBuildlink3Mk(mklines MkLines) { (&Buildlink3Checker{mklines: mklines}).Check() } @@ -64,7 +64,7 @@ func (ck *Buildlink3Checker) Check() { if G.Pkg != nil { // TODO: Commenting this line doesn't make any test fail, but it should. - G.Pkg.checklinesBuildlink3Inclusion(mklines) + G.Pkg.checkLinesBuildlink3Inclusion(mklines) } mklines.SaveAutofixChanges() diff --git a/pkgtools/pkglint/files/buildlink3_test.go b/pkgtools/pkglint/files/buildlink3_test.go index 5f1da8c782f..fdaa98fac51 100644 --- a/pkgtools/pkglint/files/buildlink3_test.go +++ b/pkgtools/pkglint/files/buildlink3_test.go @@ -2,7 +2,7 @@ package pkglint import "gopkg.in/check.v1" -func (s *Suite) Test_ChecklinesBuildlink3Mk__unfinished_url2pkg(c *check.C) { +func (s *Suite) Test_CheckLinesBuildlink3Mk__unfinished_url2pkg(c *check.C) { t := s.Init(c) t.SetupVartypes() @@ -27,7 +27,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk__unfinished_url2pkg(c *check.C) { "", "BUILDLINK_TREE+=\t-Xbae") - ChecklinesBuildlink3Mk(mklines) + CheckLinesBuildlink3Mk(mklines) t.CheckOutputLines( "ERROR: ~/category/package/buildlink3.mk:2: This comment indicates unfinished work (url2pkg).") @@ -39,7 +39,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk__unfinished_url2pkg(c *check.C) { // // Since then, pkglint also looks at files from mk/ when they are directly // included, and therefore finds the default definition for PKGNAME. -func (s *Suite) Test_ChecklinesBuildlink3Mk__name_mismatch_Haskell_incomplete(c *check.C) { +func (s *Suite) Test_CheckLinesBuildlink3Mk__name_mismatch_Haskell_incomplete(c *check.C) { t := s.Init(c) t.SetupPackage("x11/hs-X11", @@ -60,7 +60,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk__name_mismatch_Haskell_incomplete(c "", "BUILDLINK_TREE+=\t-hs-X11") - G.CheckDirent(".") + G.Check(".") // This warning only occurs because pkglint cannot see mk/haskell.mk in this test. t.CheckOutputLines( @@ -73,7 +73,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk__name_mismatch_Haskell_incomplete(c // // Since then, pkglint also looks at files from mk/ when they are directly // included, and therefore finds the default definition for PKGNAME. -func (s *Suite) Test_ChecklinesBuildlink3Mk__name_mismatch_Haskell_complete(c *check.C) { +func (s *Suite) Test_CheckLinesBuildlink3Mk__name_mismatch_Haskell_complete(c *check.C) { t := s.Init(c) t.CreateFileLines("mk/haskell.mk", @@ -99,12 +99,12 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk__name_mismatch_Haskell_complete(c *c "", "BUILDLINK_TREE+=\t-hs-X11") - G.CheckDirent(".") + G.Check(".") t.CheckOutputEmpty() } -func (s *Suite) Test_ChecklinesBuildlink3Mk__name_mismatch_multiple_inclusion(c *check.C) { +func (s *Suite) Test_CheckLinesBuildlink3Mk__name_mismatch_multiple_inclusion(c *check.C) { t := s.Init(c) t.SetupVartypes() @@ -120,7 +120,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk__name_mismatch_multiple_inclusion(c "", "BUILDLINK_TREE+=\t-pkgbase1") - ChecklinesBuildlink3Mk(mklines) + CheckLinesBuildlink3Mk(mklines) t.CheckOutputLines( "ERROR: buildlink3.mk:5: Package name mismatch between multiple-inclusion guard \"PKGBASE2\" "+ @@ -128,7 +128,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk__name_mismatch_multiple_inclusion(c "WARN: buildlink3.mk:9: Definition of BUILDLINK_API_DEPENDS is missing.") } -func (s *Suite) Test_ChecklinesBuildlink3Mk__name_mismatch_abi_api(c *check.C) { +func (s *Suite) Test_CheckLinesBuildlink3Mk__name_mismatch_abi_api(c *check.C) { t := s.Init(c) t.SetupVartypes() @@ -148,14 +148,14 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk__name_mismatch_abi_api(c *check.C) { "", "BUILDLINK_TREE+=\t-hs-X11") - ChecklinesBuildlink3Mk(mklines) + CheckLinesBuildlink3Mk(mklines) t.CheckOutputLines( "WARN: buildlink3.mk:9: Package name mismatch between ABI \"hs-X12\" and API \"hs-X11\" (from line 8).", "WARN: buildlink3.mk:10: Only buildlink variables for \"hs-X11\", not \"hs-X12\" may be set in this file.") } -func (s *Suite) Test_ChecklinesBuildlink3Mk__abi_api_versions(c *check.C) { +func (s *Suite) Test_CheckLinesBuildlink3Mk__abi_api_versions(c *check.C) { t := s.Init(c) t.SetupVartypes() @@ -174,7 +174,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk__abi_api_versions(c *check.C) { "", "BUILDLINK_TREE+=\t-hs-X11") - ChecklinesBuildlink3Mk(mklines) + CheckLinesBuildlink3Mk(mklines) t.CheckOutputLines( "WARN: buildlink3.mk:9: ABI version \"1.6.0\" should be at least API version \"1.6.1\" (see line 8).") @@ -205,14 +205,14 @@ func (s *Suite) Test_Buildlink3Checker_checkVarassign__abi_api_versions_brace(c "", "BUILDLINK_TREE+=\t-totem") - ChecklinesBuildlink3Mk(mklines) + CheckLinesBuildlink3Mk(mklines) // No warning about ABI "totem" and API "{totem,totem-xine}" // because that case is explicitly not checked. t.CheckOutputEmpty() } -func (s *Suite) Test_ChecklinesBuildlink3Mk__missing_BUILDLINK_TREE_at_beginning(c *check.C) { +func (s *Suite) Test_CheckLinesBuildlink3Mk__missing_BUILDLINK_TREE_at_beginning(c *check.C) { t := s.Init(c) t.SetupVartypes() @@ -223,7 +223,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk__missing_BUILDLINK_TREE_at_beginning "HS_X11_BUILDLINK3_MK:=", ".endif") - ChecklinesBuildlink3Mk(mklines) + CheckLinesBuildlink3Mk(mklines) t.CheckOutputLines( "WARN: buildlink3.mk:3: Expected a BUILDLINK_TREE line.") @@ -231,7 +231,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk__missing_BUILDLINK_TREE_at_beginning // In buildlink3.mk files, there is no need to place any comments // in the autogenerated code, therefore this warning is justified. -func (s *Suite) Test_ChecklinesBuildlink3Mk__missing_BUILDLINK_TREE_at_end(c *check.C) { +func (s *Suite) Test_CheckLinesBuildlink3Mk__missing_BUILDLINK_TREE_at_end(c *check.C) { t := s.Init(c) t.SetupVartypes() @@ -251,13 +251,13 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk__missing_BUILDLINK_TREE_at_end(c *ch "# needless comment", "BUILDLINK_TREE+=\t-hs-X11") - ChecklinesBuildlink3Mk(mklines) + CheckLinesBuildlink3Mk(mklines) t.CheckOutputLines( "WARN: buildlink3.mk:13: This line should contain the following text: BUILDLINK_TREE+=\t-hs-X11") } -func (s *Suite) Test_ChecklinesBuildlink3Mk__DEPMETHOD_placement(c *check.C) { +func (s *Suite) Test_CheckLinesBuildlink3Mk__DEPMETHOD_placement(c *check.C) { t := s.Init(c) t.SetupVartypes() @@ -278,13 +278,13 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk__DEPMETHOD_placement(c *check.C) { "", "BUILDLINK_TREE+=\t-hs-X11") - ChecklinesBuildlink3Mk(mklines) + CheckLinesBuildlink3Mk(mklines) t.CheckOutputLines( "WARN: buildlink3.mk:3: This line belongs inside the .ifdef block.") } -func (s *Suite) Test_ChecklinesBuildlink3Mk__multiple_inclusion_wrong(c *check.C) { +func (s *Suite) Test_CheckLinesBuildlink3Mk__multiple_inclusion_wrong(c *check.C) { t := s.Init(c) t.SetupVartypes() @@ -297,7 +297,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk__multiple_inclusion_wrong(c *check.C "UNRELATED_BUILDLINK3_MK:=", ".endif") - ChecklinesBuildlink3Mk(mklines) + CheckLinesBuildlink3Mk(mklines) t.CheckOutputLines( "WARN: buildlink3.mk:5: HS_X11_BUILDLINK3_MK is used but not defined.", @@ -305,7 +305,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk__multiple_inclusion_wrong(c *check.C "WARN: buildlink3.mk:6: This line should contain the following text: HS_X11_BUILDLINK3_MK:=") } -func (s *Suite) Test_ChecklinesBuildlink3Mk__missing_endif(c *check.C) { +func (s *Suite) Test_CheckLinesBuildlink3Mk__missing_endif(c *check.C) { t := s.Init(c) t.SetupVartypes() @@ -317,14 +317,14 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk__missing_endif(c *check.C) { ".if !defined(PKGBASE1_BUILDLINK3_MK)", "PKGBASE1_BUILDLINK3_MK:=") - ChecklinesBuildlink3Mk(mklines) + CheckLinesBuildlink3Mk(mklines) t.CheckOutputLines( "ERROR: buildlink3.mk:EOF: .if from line 5 must be closed.", "NOTE: buildlink3.mk:6: Empty line expected after this line.") } -func (s *Suite) Test_ChecklinesBuildlink3Mk__invalid_dependency_patterns(c *check.C) { +func (s *Suite) Test_CheckLinesBuildlink3Mk__invalid_dependency_patterns(c *check.C) { t := s.Init(c) t.SetupVartypes() @@ -344,14 +344,14 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk__invalid_dependency_patterns(c *chec "", "BUILDLINK_TREE+=\t-hs-X11") - ChecklinesBuildlink3Mk(mklines) + CheckLinesBuildlink3Mk(mklines) t.CheckOutputLines( "WARN: buildlink3.mk:9: Invalid dependency pattern \"hs-X11!=1.6.1\".", "WARN: buildlink3.mk:10: Invalid dependency pattern \"hs-X11!=1.6.1.2nb2\".") } -func (s *Suite) Test_ChecklinesBuildlink3Mk__PKGBASE_with_variable(c *check.C) { +func (s *Suite) Test_CheckLinesBuildlink3Mk__PKGBASE_with_variable(c *check.C) { t := s.Init(c) t.SetupVartypes() @@ -370,13 +370,13 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk__PKGBASE_with_variable(c *check.C) { "", "BUILDLINK_TREE+=\t-${PYPKGPREFIX}-wxWidgets") - ChecklinesBuildlink3Mk(mklines) + CheckLinesBuildlink3Mk(mklines) t.CheckOutputLines( "WARN: buildlink3.mk:3: Please use \"py\" instead of \"${PYPKGPREFIX}\" (also in other variables in this file).") } -func (s *Suite) Test_ChecklinesBuildlink3Mk__PKGBASE_with_unknown_variable(c *check.C) { +func (s *Suite) Test_CheckLinesBuildlink3Mk__PKGBASE_with_unknown_variable(c *check.C) { t := s.Init(c) t.SetupVartypes() @@ -395,7 +395,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk__PKGBASE_with_unknown_variable(c *ch "", "BUILDLINK_TREE+=\t-${LICENSE}-wxWidgets") - ChecklinesBuildlink3Mk(mklines) + CheckLinesBuildlink3Mk(mklines) t.CheckOutputLines( "WARN: buildlink3.mk:3: LICENSE may not be used in any file; it is a write-only variable.", @@ -451,7 +451,7 @@ func (s *Suite) Test_Buildlink3Checker_checkMainPart__nested_if(c *check.C) { "", "BUILDLINK_TREE+=\t-hs-X11") - ChecklinesBuildlink3Mk(mklines) + CheckLinesBuildlink3Mk(mklines) t.CheckOutputEmpty() } @@ -477,7 +477,7 @@ func (s *Suite) Test_Buildlink3Checker_checkMainPart__comment_at_end_of_file(c * "", "# the end") - ChecklinesBuildlink3Mk(mklines) + CheckLinesBuildlink3Mk(mklines) t.CheckOutputLines( "WARN: ~/category/package/buildlink3.mk:14: The file should end here.") diff --git a/pkgtools/pkglint/files/category_test.go b/pkgtools/pkglint/files/category_test.go index 3ded301ffba..c1b017a355a 100644 --- a/pkgtools/pkglint/files/category_test.go +++ b/pkgtools/pkglint/files/category_test.go @@ -84,7 +84,7 @@ func (s *Suite) Test_CheckdirCategory__wip(c *check.C) { "", ".include \"../mk/misc/category.mk\"") - G.CheckDirent(t.File("wip")) + G.Check(t.File("wip")) t.CheckOutputLines( "WARN: ~/wip/Makefile:8: Unknown shell command \"wip-specific-command\".") diff --git a/pkgtools/pkglint/files/check_test.go b/pkgtools/pkglint/files/check_test.go index 650a90ec877..4b1d3dac8bb 100644 --- a/pkgtools/pkglint/files/check_test.go +++ b/pkgtools/pkglint/files/check_test.go @@ -279,7 +279,7 @@ func (t *Tester) SetupCategory(name string) { // The given makefileLines start in line 20. Except if they are variable // definitions for already existing variables, then they replace that line. // -// Returns the path to the package, ready to be used with Pkglint.CheckDirent. +// Returns the path to the package, ready to be used with Pkglint.Check. // // After calling this method, individual files can be overwritten as necessary. // Then, G.Pkgsrc.LoadInfrastructure should be called to load all the files. diff --git a/pkgtools/pkglint/files/distinfo.go b/pkgtools/pkglint/files/distinfo.go index c333c446f51..4f3120d1642 100644 --- a/pkgtools/pkglint/files/distinfo.go +++ b/pkgtools/pkglint/files/distinfo.go @@ -8,7 +8,7 @@ import ( "strings" ) -func ChecklinesDistinfo(lines Lines) { +func CheckLinesDistinfo(lines Lines) { if trace.Tracing { defer trace.Call1(lines.FileName)() } @@ -27,7 +27,7 @@ func ChecklinesDistinfo(lines Lines) { lines, patchdir, distinfoIsCommitted, make(map[string]bool), "", nil, unknown, nil} ck.checkLines(lines) - ChecklinesTrailingEmptyLines(lines) + CheckLinesTrailingEmptyLines(lines) ck.checkUnrecordedPatches() SaveAutofixChanges(lines) } diff --git a/pkgtools/pkglint/files/distinfo_test.go b/pkgtools/pkglint/files/distinfo_test.go index 8d4a3ed5364..2cd7b361902 100644 --- a/pkgtools/pkglint/files/distinfo_test.go +++ b/pkgtools/pkglint/files/distinfo_test.go @@ -2,7 +2,7 @@ package pkglint import "gopkg.in/check.v1" -func (s *Suite) Test_ChecklinesDistinfo(c *check.C) { +func (s *Suite) Test_CheckLinesDistinfo(c *check.C) { t := s.Init(c) t.Chdir("category/package") @@ -23,7 +23,7 @@ func (s *Suite) Test_ChecklinesDistinfo(c *check.C) { "SHA1 (patch-nonexistent) = 1234") G.Pkg = NewPackage(".") - ChecklinesDistinfo(lines) + CheckLinesDistinfo(lines) t.CheckOutputLines( "ERROR: distinfo:1: Expected \"$"+"NetBSD$\".", @@ -85,7 +85,7 @@ func (s *Suite) Test_distinfoLinesChecker_checkGlobalDistfileMismatch(c *check.C "5 errors and 1 warning found.") } -func (s *Suite) Test_ChecklinesDistinfo__uncommitted_patch(c *check.C) { +func (s *Suite) Test_CheckLinesDistinfo__uncommitted_patch(c *check.C) { t := s.Init(c) t.SetupPackage("category/package") @@ -104,7 +104,7 @@ func (s *Suite) Test_ChecklinesDistinfo__uncommitted_patch(c *check.C) { "WARN: distinfo:3: patches/patch-aa is registered in distinfo but not added to CVS.") } -func (s *Suite) Test_ChecklinesDistinfo__unrecorded_patches(c *check.C) { +func (s *Suite) Test_CheckLinesDistinfo__unrecorded_patches(c *check.C) { t := s.Init(c) t.SetupPackage("category/package") @@ -130,7 +130,7 @@ func (s *Suite) Test_ChecklinesDistinfo__unrecorded_patches(c *check.C) { // The distinfo file and the patches are usually placed in the package // directory. By defining PATCHDIR or DISTINFO_FILE, a package can define // that they are somewhere else in pkgsrc. -func (s *Suite) Test_ChecklinesDistinfo__relative_path_in_distinfo(c *check.C) { +func (s *Suite) Test_CheckLinesDistinfo__relative_path_in_distinfo(c *check.C) { t := s.Init(c) t.SetupPackage("category/package", @@ -160,7 +160,7 @@ func (s *Suite) Test_ChecklinesDistinfo__relative_path_in_distinfo(c *check.C) { // When the distinfo file and the patches are placed in the same package, // their diagnostics use short relative paths. -func (s *Suite) Test_ChecklinesDistinfo__distinfo_and_patches_in_separate_directory(c *check.C) { +func (s *Suite) Test_CheckLinesDistinfo__distinfo_and_patches_in_separate_directory(c *check.C) { t := s.Init(c) t.SetupPackage("category/package", @@ -188,7 +188,7 @@ func (s *Suite) Test_ChecklinesDistinfo__distinfo_and_patches_in_separate_direct "is not recorded. Run \""+confMake+" makepatchsum\".") } -func (s *Suite) Test_ChecklinesDistinfo__manual_patches(c *check.C) { +func (s *Suite) Test_CheckLinesDistinfo__manual_patches(c *check.C) { t := s.Init(c) t.Chdir("category/package") @@ -198,7 +198,7 @@ func (s *Suite) Test_ChecklinesDistinfo__manual_patches(c *check.C) { "", "SHA1 (patch-aa) = ...") - ChecklinesDistinfo(lines) + CheckLinesDistinfo(lines) // When a distinfo file is checked on its own, without belonging to a package, // the PATCHDIR is not known and therefore no diagnostics are logged. @@ -206,7 +206,7 @@ func (s *Suite) Test_ChecklinesDistinfo__manual_patches(c *check.C) { G.Pkg = NewPackage(".") - ChecklinesDistinfo(lines) + CheckLinesDistinfo(lines) // When a distinfo file is checked in the context of a package, // the PATCHDIR is known, therefore the check is active. @@ -220,7 +220,7 @@ func (s *Suite) Test_ChecklinesDistinfo__manual_patches(c *check.C) { // infrastructure, there is nothing a package author can do about. // // XXX: Re-check the documentation for this test. -func (s *Suite) Test_ChecklinesDistinfo__missing_php_patches(c *check.C) { +func (s *Suite) Test_CheckLinesDistinfo__missing_php_patches(c *check.C) { t := s.Init(c) t.SetupPkgsrc() @@ -255,7 +255,7 @@ func (s *Suite) Test_ChecklinesDistinfo__missing_php_patches(c *check.C) { ".include \"../../lang/php/ext.mk\"", ".include \"../../mk/bsd.pkg.mk\"") - G.CheckDirent(t.File("archivers/php-bz2")) + G.Check(t.File("archivers/php-bz2")) t.CreateFileLines("archivers/php-zlib/Makefile", MkRcsID, @@ -263,7 +263,7 @@ func (s *Suite) Test_ChecklinesDistinfo__missing_php_patches(c *check.C) { ".include \"../../lang/php/ext.mk\"", ".include \"../../mk/bsd.pkg.mk\"") - G.CheckDirent(t.File("archivers/php-zlib")) + G.Check(t.File("archivers/php-zlib")) t.CheckOutputEmpty() } diff --git a/pkgtools/pkglint/files/line.go b/pkgtools/pkglint/files/line.go index c8de51f465c..e080bf1dc79 100644 --- a/pkgtools/pkglint/files/line.go +++ b/pkgtools/pkglint/files/line.go @@ -109,6 +109,11 @@ func (line *LineImpl) showSource(out *SeparatorWriter) { return } + write := func(prefix, line string) { + out.Write(prefix) + out.Write(escapePrintable(line)) + } + printDiff := func(rawLines []*RawLine) { prefix := ">\t" for _, rawLine := range rawLines { @@ -120,24 +125,24 @@ func (line *LineImpl) showSource(out *SeparatorWriter) { for _, rawLine := range rawLines { if rawLine.textnl != rawLine.orignl { if rawLine.orignl != "" { - out.Write("-\t" + rawLine.orignl) + write("-\t", rawLine.orignl) } if rawLine.textnl != "" { - out.Write("+\t" + rawLine.textnl) + write("+\t", rawLine.textnl) } } else { - out.Write(prefix + rawLine.orignl) + write(prefix, rawLine.orignl) } } } if line.autofix != nil { for _, before := range line.autofix.linesBefore { - out.Write("+\t" + before) + write("+\t", before) } printDiff(line.raw) for _, after := range line.autofix.linesAfter { - out.Write("+\t" + after) + write("+\t", after) } } else { printDiff(line.raw) diff --git a/pkgtools/pkglint/files/linechecker.go b/pkgtools/pkglint/files/linechecker.go index a6e9a12691e..103af5d050e 100644 --- a/pkgtools/pkglint/files/linechecker.go +++ b/pkgtools/pkglint/files/linechecker.go @@ -41,12 +41,20 @@ func (ck LineChecker) CheckAbsolutePathname(text string) { } func (ck LineChecker) CheckLength(maxLength int) { - if len(ck.line.Text) > maxLength { - ck.line.Warnf("Line too long (should be no more than %d characters).", maxLength) - G.Explain( - "Back in the old time, terminals with 80x25 characters were common.", - "And this is still the default size of many terminal emulators.", - "Moderately short lines also make reading easier.") + if len(ck.line.Text) <= maxLength { + return + } + + prefix := ck.line.Text[0:maxLength] + for i := 0; i < len(prefix); i++ { + if isHspace(prefix[i]) { + ck.line.Warnf("Line too long (should be no more than %d characters).", maxLength) + G.Explain( + "Back in the old time, terminals with 80x25 characters were common.", + "And this is still the default size of many terminal emulators.", + "Moderately short lines also make reading easier.") + return + } } } diff --git a/pkgtools/pkglint/files/linechecker_test.go b/pkgtools/pkglint/files/linechecker_test.go index f8db63bf9c3..0c07e5159cb 100644 --- a/pkgtools/pkglint/files/linechecker_test.go +++ b/pkgtools/pkglint/files/linechecker_test.go @@ -80,6 +80,19 @@ func (s *Suite) Test_LineChecker_CheckAbsolutePathname__disabled_by_default(c *c t.CheckOutputEmpty() } +func (s *Suite) Test_LineChecker_CheckLength(c *check.C) { + t := s.Init(c) + + line1 := t.NewLine("DESCR", 1, "A very long line with spaces") + line2 := t.NewLine("DESCR", 2, "A_very_long_line_without_spaces") + + LineChecker{line1}.CheckLength(20) + LineChecker{line2}.CheckLength(20) + + t.CheckOutputLines( + "WARN: DESCR:1: Line too long (should be no more than 20 characters).") +} + func (s *Suite) Test_LineChecker_CheckTrailingWhitespace(c *check.C) { t := s.Init(c) diff --git a/pkgtools/pkglint/files/lines_test.go b/pkgtools/pkglint/files/lines_test.go index fcdca3aa81f..63b52ff6b47 100644 --- a/pkgtools/pkglint/files/lines_test.go +++ b/pkgtools/pkglint/files/lines_test.go @@ -44,7 +44,7 @@ func (s *Suite) Test_Lines_CheckRcsID__wip(c *check.C) { t.CreateFileLines("wip/package/file5.mk", "# $"+"FreeBSD$") - G.CheckDirent(t.File("wip/package")) + G.Check(t.File("wip/package")) t.CheckOutputLines( "NOTE: ~/wip/package/file1.mk:1: Expected exactly \"# $"+"NetBSD$\".", @@ -54,11 +54,11 @@ func (s *Suite) Test_Lines_CheckRcsID__wip(c *check.C) { G.Logger.Opts.Autofix = true - G.CheckDirent(t.File("wip/package")) + G.Check(t.File("wip/package")) t.CheckOutputLines( - "AUTOFIX: ~/wip/package/file1.mk:1: Replacing \"# $NetBSD: lines_test.go,v 1.3 2018/12/17 00:15:39 rillig Exp $\" with \"# $NetBSD: lines_test.go,v 1.3 2018/12/17 00:15:39 rillig Exp $\".", - "AUTOFIX: ~/wip/package/file3.mk:1: Inserting a line \"# $NetBSD: lines_test.go,v 1.3 2018/12/17 00:15:39 rillig Exp $\" before this line.", - "AUTOFIX: ~/wip/package/file4.mk:1: Inserting a line \"# $NetBSD: lines_test.go,v 1.3 2018/12/17 00:15:39 rillig Exp $\" before this line.", - "AUTOFIX: ~/wip/package/file5.mk:1: Inserting a line \"# $NetBSD: lines_test.go,v 1.3 2018/12/17 00:15:39 rillig Exp $\" before this line.") + "AUTOFIX: ~/wip/package/file1.mk:1: Replacing \"# $NetBSD: lines_test.go,v 1.4 2018/12/21 08:05:24 rillig Exp $\" with \"# $NetBSD: lines_test.go,v 1.4 2018/12/21 08:05:24 rillig Exp $\".", + "AUTOFIX: ~/wip/package/file3.mk:1: Inserting a line \"# $NetBSD: lines_test.go,v 1.4 2018/12/21 08:05:24 rillig Exp $\" before this line.", + "AUTOFIX: ~/wip/package/file4.mk:1: Inserting a line \"# $NetBSD: lines_test.go,v 1.4 2018/12/21 08:05:24 rillig Exp $\" before this line.", + "AUTOFIX: ~/wip/package/file5.mk:1: Inserting a line \"# $NetBSD: lines_test.go,v 1.4 2018/12/21 08:05:24 rillig Exp $\" before this line.") } diff --git a/pkgtools/pkglint/files/logging.go b/pkgtools/pkglint/files/logging.go index 1120c240bbd..091bf02b0cc 100644 --- a/pkgtools/pkglint/files/logging.go +++ b/pkgtools/pkglint/files/logging.go @@ -101,7 +101,7 @@ func (l *Logger) Explain(explanation ...string) { if explanationLine != "" { l.out.Write("\t") } - l.out.WriteLine(explanationLine) + l.out.WriteLine(escapePrintable(explanationLine)) } l.out.WriteLine("") } diff --git a/pkgtools/pkglint/files/logging_test.go b/pkgtools/pkglint/files/logging_test.go index 77d0bbe0502..8c0a6ffce99 100644 --- a/pkgtools/pkglint/files/logging_test.go +++ b/pkgtools/pkglint/files/logging_test.go @@ -618,12 +618,17 @@ func (s *Suite) Test_Logger_Logf__traditional_format(c *check.C) { func (s *Suite) Test_Logger_Logf__strange_characters(c *check.C) { t := s.Init(c) - t.SetupCommandLine("--gcc-output-format") + t.SetupCommandLine("--gcc-output-format", "--source", "--explain") G.Logf(Note, "filename", "123", "Format.", "Unicode \U0001F645 and ANSI \x1B are never logged.") + G.Explain( + "Even a \u0007 in the explanation is silent.") t.CheckOutputLines( - "filename:123: note: Unicode U+1F645 and ANSI U+001B are never logged.") + "filename:123: note: Unicode <U+1F645> and ANSI <U+001B> are never logged.", + "", + "\tEven a <U+0007> in the explanation is silent.", + "") } func (s *Suite) Test_Logger_Diag__show_source(c *check.C) { diff --git a/pkgtools/pkglint/files/mkline_test.go b/pkgtools/pkglint/files/mkline_test.go index 86389770f57..cad1b44a2c7 100644 --- a/pkgtools/pkglint/files/mkline_test.go +++ b/pkgtools/pkglint/files/mkline_test.go @@ -148,7 +148,7 @@ func (s *Suite) Test_NewMkLine__autofix_space_after_varname(c *check.C) { "VARNAME+ +=\t${VARNAME+}", "pkgbase := pkglint") - CheckfileMk(filename) + CheckFileMk(filename) t.CheckOutputLines( "NOTE: ~/Makefile:2: Unnecessary space after variable name \"VARNAME\".", @@ -157,7 +157,7 @@ func (s *Suite) Test_NewMkLine__autofix_space_after_varname(c *check.C) { t.SetupCommandLine("-Wspace", "--autofix") - CheckfileMk(filename) + CheckFileMk(filename) t.CheckOutputLines( "AUTOFIX: ~/Makefile:2: Replacing \"VARNAME +=\" with \"VARNAME+=\".", @@ -597,7 +597,7 @@ func (s *Suite) Test_MkLine_VariableNeedsQuoting__PKGNAME_and_URL_list_in_URL_li MkRcsID, "MASTER_SITES=\tftp://ftp.gtk.org/${PKGNAME}/ ${MASTER_SITE_GNOME:=subdir/}") - MkLineChecker{G.Mk.mklines[1]}.checkVarassignVaruse() + MkLineChecker{G.Mk.mklines[1]}.checkVarassignRightVaruse() t.CheckOutputEmpty() // Don't warn about missing :Q modifiers. } @@ -612,7 +612,7 @@ func (s *Suite) Test_MkLine_VariableNeedsQuoting__tool_in_CONFIGURE_ENV(c *check "", "CONFIGURE_ENV+=\tSYS_TAR_COMMAND_PATH=${TOOLS_TAR:Q}") - MkLineChecker{mklines.mklines[2]}.checkVarassignVaruse() + MkLineChecker{mklines.mklines[2]}.checkVarassignRightVaruse() // The TOOLS_* variables only contain the path to the tool, // without any additional arguments that might be necessary @@ -633,8 +633,8 @@ func (s *Suite) Test_MkLine_VariableNeedsQuoting__backticks(c *check.C) { "COMPILE_CMD=\tcc `${CAT} ${WRKDIR}/compileflags`", "COMMENT_CMD=\techo `echo ${COMMENT}`") - MkLineChecker{mklines.mklines[2]}.checkVarassignVaruse() - MkLineChecker{mklines.mklines[3]}.checkVarassignVaruse() + MkLineChecker{mklines.mklines[2]}.checkVarassignRightVaruse() + MkLineChecker{mklines.mklines[3]}.checkVarassignRightVaruse() // Both CAT and WRKDIR are safe from quoting, therefore no warnings. // But COMMENT may contain arbitrary characters and therefore must diff --git a/pkgtools/pkglint/files/mklinechecker.go b/pkgtools/pkglint/files/mklinechecker.go index 3121a596284..2f0e99dcdc6 100644 --- a/pkgtools/pkglint/files/mklinechecker.go +++ b/pkgtools/pkglint/files/mklinechecker.go @@ -56,6 +56,7 @@ func (ck MkLineChecker) checkShellCommand() { fix.ReplaceRegex(`^\t\t+`, "\t", 1) fix.Apply() } + ck.checkText(shellCommand) NewShellLine(mkline).CheckShellCommandLine(shellCommand) } @@ -195,7 +196,7 @@ func (ck MkLineChecker) checkDirectiveFor(forVars map[string]bool, indentation * args := mkline.Args() if m, vars, _ := match2(args, `^([^\t ]+(?:[\t ]*[^\t ]+)*?)[\t ]+in[\t ]+(.*)$`); m { - for _, forvar := range fields(vars) { + for _, forvar := range strings.Fields(vars) { indentation.AddVar(forvar) if !G.Infrastructure && hasPrefix(forvar, "_") { mkline.Warnf("Variable names starting with an underscore (%s) are reserved for internal pkgsrc use.", forvar) @@ -219,10 +220,10 @@ func (ck MkLineChecker) checkDirectiveFor(forVars map[string]bool, indentation * // running pkglint over the whole pkgsrc tree did not produce any different result // whether guessed was true or false, so currently it is not worth investing // any work. - forLoopType := Vartype{lkSpace, BtUnknown, []ACLEntry{{"*", aclpAllRead}}, false} + forLoopType := Vartype{lkShell, BtUnknown, []ACLEntry{{"*", aclpAllRead}}, false} forLoopContext := VarUseContext{&forLoopType, vucTimeParse, vucQuotFor, false} - for _, forLoopVar := range mkline.DetermineUsedVariables() { - ck.CheckVaruse(&MkVarUse{forLoopVar, nil}, &forLoopContext) + for _, itemsVar := range mkline.DetermineUsedVariables() { + ck.CheckVaruse(&MkVarUse{itemsVar, nil}, &forLoopContext) } } } @@ -281,11 +282,11 @@ func (ck MkLineChecker) checkDependencyRule(allowedTargets map[string]bool) { } } -// checkVarassignPermissions checks the permissions for the left-hand side +// checkVarassignLeftPermissions checks the permissions for the left-hand side // of a variable assignment line. // // See checkVarusePermissions. -func (ck MkLineChecker) checkVarassignPermissions() { +func (ck MkLineChecker) checkVarassignLeftPermissions() { if !G.Opts.WarnPerm || G.Infrastructure { return } @@ -395,6 +396,7 @@ func (ck MkLineChecker) CheckVaruse(varuse *MkVarUse, vuc *VarUseContext) { if G.Opts.WarnQuoting && vuc.quoting != vucQuotUnknown && needsQuoting != unknown { // FIXME: Why "Shellword" when there's no indication that this is actually a shell type? + // It's for splitting the value into tokens, taking "double" and 'single' quotes into account. ck.CheckVaruseShellword(varname, vartype, vuc, varuse.Mod(), needsQuoting) } @@ -494,7 +496,7 @@ func (ck MkLineChecker) checkVaruseModifiersRange(varuse *MkVarUse) { // checkVarusePermissions checks the permissions for the right-hand side // of a variable assignment line. // -// See checkVarassignPermissions. +// See checkVarassignLeftPermissions. func (ck MkLineChecker) checkVarusePermissions(varname string, vartype *Vartype, vuc *VarUseContext) { if !G.Opts.WarnPerm { return @@ -752,12 +754,6 @@ func (ck MkLineChecker) CheckVaruseShellword(varname string, vartype *Vartype, v } func (ck MkLineChecker) checkVaruseDeprecated(varuse *MkVarUse) { - // Temporarily disabled since this method is not called for all places, - // such as ${_PKG_SILENT} in a shell command. - if varuse != nil { - return - } - varname := varuse.varname instead := G.Pkgsrc.Deprecated[varname] if instead == "" { @@ -796,6 +792,30 @@ func (ck MkLineChecker) checkVarassignDecreasingVersions() { } func (ck MkLineChecker) checkVarassign() { + ck.checkVarassignLeft() + ck.checkVarassignRight() +} + +// checkVarassignLeft checks everything to the left of the assignment operator. +func (ck MkLineChecker) checkVarassignLeft() { + varname := ck.MkLine.Varname() + if hasPrefix(varname, "_") && !G.Infrastructure { + ck.MkLine.Warnf("Variable names starting with an underscore (%s) are reserved for internal pkgsrc use.", varname) + } + + ck.checkVarassignLeftNotUsed() + ck.checkVarassignLeftDeprecated() + ck.checkVarassignLeftPermissions() + ck.checkVarassignLeftBsdPrefs() + + ck.checkTextVarUse( + ck.MkLine.Varname(), + &Vartype{lkNone, BtVariableName, []ACLEntry{{"*", aclpAll}}, false}, + vucTimeParse) +} + +// checkVarassignLeft checks everything to the right of the assignment operator. +func (ck MkLineChecker) checkVarassignRight() { mkline := ck.MkLine varname := mkline.Varname() op := mkline.Op() @@ -806,23 +826,15 @@ func (ck MkLineChecker) checkVarassign() { defer trace.Call(varname, op, value)() } - defineVar(mkline, varname) - ck.checkVarassignPermissions() - ck.checkVarassignBsdPrefs() - ck.checkText(value) ck.checkVartype(varname, op, value, comment) - ck.checkVarassignUnused() - - ck.checkVarassignSpecific() + ck.checkVarassignMisc() - ck.checkVarassignDeprecated() - - ck.checkVarassignVaruse() + ck.checkVarassignRightVaruse() } -func (ck MkLineChecker) checkVarassignDeprecated() { +func (ck MkLineChecker) checkVarassignLeftDeprecated() { varname := ck.MkLine.Varname() if fix := G.Pkgsrc.Deprecated[varname]; fix != "" { ck.MkLine.Warnf("Definition of %s is deprecated. %s", varname, fix) @@ -831,7 +843,7 @@ func (ck MkLineChecker) checkVarassignDeprecated() { } } -func (ck MkLineChecker) checkVarassignUnused() { +func (ck MkLineChecker) checkVarassignLeftNotUsed() { varname := ck.MkLine.Varname() varcanon := varnameCanon(varname) @@ -854,9 +866,10 @@ func (ck MkLineChecker) checkVarassignUnused() { } } -// checkVarassignVaruse checks that in a variable assignment, each variables used on either side -// of the assignment has the correct data type and quoting. -func (ck MkLineChecker) checkVarassignVaruse() { +// checkVarassignRightVaruse checks that in a variable assignment, +// each variable used on the right-hand side of the assignment operator +// has the correct data type and quoting. +func (ck MkLineChecker) checkVarassignRightVaruse() { if trace.Tracing { defer trace.Call()() } @@ -874,11 +887,6 @@ func (ck MkLineChecker) checkVarassignVaruse() { vartype = shellcommandsContextType } - ck.checkTextVarUse( - ck.MkLine.Varname(), - &Vartype{lkNone, BtVariableName, []ACLEntry{{"*", aclpAll}}, false}, - vucTimeParse) - if vartype != nil && vartype.IsShell() { ck.checkVarassignVaruseShell(vartype, time) } else { // XXX: This else looks as if it should be omitted. @@ -907,7 +915,7 @@ func (ck MkLineChecker) checkTextVarUse(text string, vartype *Vartype, time vucT } } -// checkVarassignVaruseShell is very similar to checkVarassignVaruse, they just differ +// checkVarassignVaruseShell is very similar to checkVarassignRightVaruse, they just differ // in the way they determine isWordPart. func (ck MkLineChecker) checkVarassignVaruseShell(vartype *Vartype, time vucTime) { if trace.Tracing { @@ -928,14 +936,14 @@ func (ck MkLineChecker) checkVarassignVaruseShell(vartype *Vartype, time vucTime atoms := NewShTokenizer(mkline.Line, mkline.Value(), false).ShAtoms() for i, atom := range atoms { if varuse := atom.VarUse(); varuse != nil { - isWordPart := isWordPart(atoms, i) - vuc := VarUseContext{vartype, time, atom.Quoting.ToVarUseContext(), isWordPart} + wordPart := isWordPart(atoms, i) + vuc := VarUseContext{vartype, time, atom.Quoting.ToVarUseContext(), wordPart} ck.CheckVaruse(varuse, &vuc) } } } -func (ck MkLineChecker) checkVarassignSpecific() { +func (ck MkLineChecker) checkVarassignMisc() { mkline := ck.MkLine varname := mkline.Varname() value := mkline.Value() @@ -944,10 +952,6 @@ func (ck MkLineChecker) checkVarassignSpecific() { mkline.Warnf("Please use the RCD_SCRIPTS mechanism to install rc.d scripts automatically to ${RCD_SCRIPTS_EXAMPLEDIR}.") } - if hasPrefix(varname, "_") && !G.Infrastructure { - mkline.Warnf("Variable names starting with an underscore (%s) are reserved for internal pkgsrc use.", varname) - } - if varname == "PYTHON_VERSIONS_ACCEPTED" { ck.checkVarassignDecreasingVersions() } @@ -983,7 +987,7 @@ func (ck MkLineChecker) checkVarassignSpecific() { } } -func (ck MkLineChecker) checkVarassignBsdPrefs() { +func (ck MkLineChecker) checkVarassignLeftBsdPrefs() { mkline := ck.MkLine switch mkline.Varcanon() { @@ -1054,11 +1058,6 @@ func (ck MkLineChecker) checkVartype(varname string, op MkOperator, value, comme case value == "": break - case vartype.kindOfList == lkSpace: - for _, word := range fields(value) { - ck.CheckVartypeBasic(varname, vartype.basicType, op, word, comment, vartype.guessed) - } - case vartype.kindOfList == lkShell: words, _ := splitIntoMkWords(mkline.Line, value) for _, word := range words { @@ -1094,8 +1093,6 @@ func (ck MkLineChecker) checkText(text string) { ck.checkTextWrksrcDotDot(text) ck.checkTextRpath(text) - ck.checkTextDeprecated(text) - } func (ck MkLineChecker) checkTextWrksrcDotDot(text string) { @@ -1124,28 +1121,6 @@ func (ck MkLineChecker) checkTextRpath(text string) { } } -func (ck MkLineChecker) checkTextDeprecated(text string) { - rest := text - for { - m, r := G.res.ReplaceFirst(rest, `(?:^|[^$])\$\{([-A-Z0-9a-z_]+)(\.[\-0-9A-Z_a-z]+)?(?::[^\}]+)?\}`, "") - if m == nil { - break - } - rest = r - - varbase, varext := m[1], m[2] - varname := varbase + varext - varcanon := varnameCanon(varname) - instead := G.Pkgsrc.Deprecated[varname] - if instead == "" { - instead = G.Pkgsrc.Deprecated[varcanon] - } - if instead != "" { - ck.MkLine.Warnf("Use of %q is deprecated. %s", varname, instead) - } - } -} - func (ck MkLineChecker) checkDirectiveCond() { mkline := ck.MkLine if trace.Tracing { diff --git a/pkgtools/pkglint/files/mklinechecker_test.go b/pkgtools/pkglint/files/mklinechecker_test.go index 36e154ce9db..7ce4f7350aa 100644 --- a/pkgtools/pkglint/files/mklinechecker_test.go +++ b/pkgtools/pkglint/files/mklinechecker_test.go @@ -2,6 +2,19 @@ package pkglint import "gopkg.in/check.v1" +func (s *Suite) Test_MkLineChecker_checkVarassignLeft(c *check.C) { + t := s.Init(c) + + mkline := t.NewMkLine("module.mk", 123, "_VARNAME=\tvalue") + + MkLineChecker{mkline}.checkVarassignLeft() + + t.CheckOutputLines( + "WARN: module.mk:123: Variable names starting with an underscore "+ + "(_VARNAME) are reserved for internal pkgsrc use.", + "WARN: module.mk:123: _VARNAME is defined but not used.") +} + func (s *Suite) Test_MkLineChecker_Check__url2pkg(c *check.C) { t := s.Init(c) @@ -339,7 +352,7 @@ func (s *Suite) Test_MkLineChecker_checkVarassign(c *check.C) { "WARN: Makefile:2: ac_cv_libpari_libs is defined but not used.") } -func (s *Suite) Test_MkLineChecker_checkVarassignPermissions(c *check.C) { +func (s *Suite) Test_MkLineChecker_checkVarassignLeftPermissions(c *check.C) { t := s.Init(c) t.SetupCommandLine("-Wall,no-space") @@ -358,7 +371,7 @@ func (s *Suite) Test_MkLineChecker_checkVarassignPermissions(c *check.C) { } // Don't check the permissions for infrastructure files since they have their own rules. -func (s *Suite) Test_MkLineChecker_checkVarassignPermissions__infrastructure(c *check.C) { +func (s *Suite) Test_MkLineChecker_checkVarassignLeftPermissions__infrastructure(c *check.C) { t := s.Init(c) t.SetupVartypes() @@ -368,19 +381,19 @@ func (s *Suite) Test_MkLineChecker_checkVarassignPermissions__infrastructure(c * "PKG_DEVELOPER?=\tyes") t.CreateFileLines("mk/bsd.pkg.mk") - G.CheckDirent(t.File("mk/infra.mk")) + G.Check(t.File("mk/infra.mk")) t.CheckOutputEmpty() } -func (s *Suite) Test_MkLineChecker_checkVarassignVaruse(c *check.C) { +func (s *Suite) Test_MkLineChecker_checkVarassignRightVaruse(c *check.C) { t := s.Init(c) t.SetupVartypes() mkline := t.NewMkLine("module.mk", 123, "PLIST_SUBST+=\tLOCALBASE=${LOCALBASE:Q}") - MkLineChecker{mkline}.checkVarassignVaruse() + MkLineChecker{mkline}.checkVarassignRightVaruse() t.CheckOutputLines( "WARN: module.mk:123: Please use PREFIX instead of LOCALBASE.", @@ -514,8 +527,8 @@ func (s *Suite) Test_MkLineChecker__unclosed_varuse(c *check.C) { mklines.Check() t.CheckOutputLines( - "WARN: Makefile:2: Unclosed Make variable starting at \"${EGDIR/apparmor.d $...\".", "WARN: Makefile:2: EGDIRS is defined but not used.", + "WARN: Makefile:2: Unclosed Make variable starting at \"${EGDIR/apparmor.d $...\".", // XXX: This warning is redundant because of the "Unclosed" warning above. "WARN: Makefile:2: Internal pkglint error in MkLine.Tokenize at "+ @@ -773,7 +786,7 @@ func (s *Suite) Test_MkLineChecker_CheckVaruseShellword__mstar_not_needed(c *che // This package is guaranteed to not use GNU_CONFIGURE. // Since the :M* hack is only needed for GNU_CONFIGURE, it is not necessary here. - G.CheckDirent(pkg) + G.Check(pkg) // FIXME: Duplicate diagnostics. t.CheckOutputLines( @@ -790,7 +803,7 @@ func (s *Suite) Test_MkLineChecker_CheckVaruseShellword__q_not_needed(c *check.C "MASTER_SITES=\t${HOMEPAGE:Q}") G.Pkgsrc.LoadInfrastructure() - G.CheckDirent(pkg) + G.Check(pkg) t.CheckOutputLines( "NOTE: ~/category/package/Makefile:5: The :Q operator isn't necessary for ${HOMEPAGE} here.") @@ -942,8 +955,7 @@ func (s *Suite) Test_MkLineChecker_CheckVaruse__deprecated_PKG_DEBUG(c *check.C) MkLineChecker{mkline}.Check() t.CheckOutputLines( - "WARN: module.mk:123: Use of \"_PKG_SILENT\" is deprecated. Use RUN (with more error checking) instead.", - "WARN: module.mk:123: Use of \"_PKG_DEBUG\" is deprecated. Use RUN (with more error checking) instead.") + "WARN: module.mk:123: Use of _PKG_SILENT and _PKG_DEBUG is deprecated. Use ${RUN} instead.") } // PR 46570, item "15. net/uucp/Makefile has a make loop" @@ -966,7 +978,7 @@ func (s *Suite) Test_MkLineChecker_checkVaruseUndefined__indirect_variables(c *c "WARN: net/uucp/Makefile:123: var is used but not defined.") } -func (s *Suite) Test_MkLineChecker_checkVarassignSpecific(c *check.C) { +func (s *Suite) Test_MkLineChecker_checkVarassignMisc(c *check.C) { t := s.Init(c) t.SetupPkgsrc() @@ -990,8 +1002,8 @@ func (s *Suite) Test_MkLineChecker_checkVarassignSpecific(c *check.C) { // TODO: Split this test into several, one for each topic. t.CheckOutputLines( "WARN: ~/module.mk:2: Please use the RCD_SCRIPTS mechanism to install rc.d scripts automatically to ${RCD_SCRIPTS_EXAMPLEDIR}.", - "WARN: ~/module.mk:3: _TOOLS_VARNAME.sed is defined but not used.", "WARN: ~/module.mk:3: Variable names starting with an underscore (_TOOLS_VARNAME.sed) are reserved for internal pkgsrc use.", + "WARN: ~/module.mk:3: _TOOLS_VARNAME.sed is defined but not used.", "WARN: ~/module.mk:4: PKGNAME should not be used in DIST_SUBDIR as it includes the PKGREVISION. Please use PKGNAME_NOREV instead.", "WARN: ~/module.mk:5: PKGNAME should not be used in WRKSRC as it includes the PKGREVISION. Please use PKGNAME_NOREV instead.", "WARN: ~/module.mk:6: SITES_distfile.tar.gz is defined but not used.", @@ -1019,8 +1031,10 @@ func (s *Suite) Test_MkLineChecker_checkText(c *check.C) { mklines.Check() + // FIXME: Duplicate diagnostics. t.CheckOutputLines( "WARN: ~/module.mk:2: Please use ${COMPILER_RPATH_FLAG} instead of \"-Wl,--rpath,\".", + "WARN: ~/module.mk:3: Use of \"GAMEGRP\" is deprecated. Use GAMES_GROUP instead.", "WARN: ~/module.mk:3: Use of \"GAMEGRP\" is deprecated. Use GAMES_GROUP instead.") } diff --git a/pkgtools/pkglint/files/mklines.go b/pkgtools/pkglint/files/mklines.go index 583b36c1870..f01305e060d 100644 --- a/pkgtools/pkglint/files/mklines.go +++ b/pkgtools/pkglint/files/mklines.go @@ -180,7 +180,7 @@ func (mklines *MkLinesImpl) checkAll() { substContext.Finish(NewMkLine(mklines.lines.EOFLine())) // TODO: mklines.EOFLine() varalign.Finish() - ChecklinesTrailingEmptyLines(mklines.lines) + CheckLinesTrailingEmptyLines(mklines.lines) } func (mklines *MkLinesImpl) checkVarassignPlist(mkline MkLine) { @@ -370,7 +370,7 @@ func (mklines *MkLinesImpl) collectDocumentedVariables() { text := mkline.Text switch { case hasPrefix(text, "#"): - words := fields(text) + words := strings.Fields(text) if len(words) <= 1 { break } diff --git a/pkgtools/pkglint/files/mktypes.go b/pkgtools/pkglint/files/mktypes.go index 1a09ba801a5..20ddec04447 100644 --- a/pkgtools/pkglint/files/mktypes.go +++ b/pkgtools/pkglint/files/mktypes.go @@ -33,9 +33,7 @@ type MkVarUse struct { modifiers []MkVarUseModifier // E.g. "Q", "S/from/to/" } -//func NewMkVarUse(varname string, modifiers ...MkVarUseModifier) *MkVarUse { -// return &MkVarUse{varname, modifiers} -//} +func (vu *MkVarUse) String() string { return sprintf("${%s%s}", vu.varname, vu.Mod()) } type MkVarUseModifier struct { Text string diff --git a/pkgtools/pkglint/files/options.go b/pkgtools/pkglint/files/options.go index d668bf48123..c82de63988b 100755 --- a/pkgtools/pkglint/files/options.go +++ b/pkgtools/pkglint/files/options.go @@ -1,6 +1,6 @@ package pkglint -func ChecklinesOptionsMk(mklines MkLines) { +func CheckLinesOptionsMk(mklines MkLines) { if trace.Tracing { defer trace.Call1(mklines.lines.FileName)() } @@ -37,7 +37,7 @@ loop: case mkline.IsVarassign(): switch mkline.Varcanon() { case "PKG_SUPPORTED_OPTIONS", "PKG_OPTIONS_GROUP.*", "PKG_OPTIONS_SET.*": - for _, option := range fields(mkline.Value()) { + for _, option := range mkline.ValueFields(mkline.Value()) { if !containsVarRef(option) { declaredOptions[option] = mkline optionsInDeclarationOrder = append(optionsInDeclarationOrder, option) diff --git a/pkgtools/pkglint/files/options_test.go b/pkgtools/pkglint/files/options_test.go index d76b27e9dea..25ad28beb7c 100755 --- a/pkgtools/pkglint/files/options_test.go +++ b/pkgtools/pkglint/files/options_test.go @@ -2,7 +2,7 @@ package pkglint import "gopkg.in/check.v1" -func (s *Suite) Test_ChecklinesOptionsMk(c *check.C) { +func (s *Suite) Test_CheckLinesOptionsMk(c *check.C) { t := s.Init(c) t.SetupCommandLine("-Wall,no-space") @@ -54,7 +54,7 @@ func (s *Suite) Test_ChecklinesOptionsMk(c *check.C) { ".elif !empty(PKG_OPTIONS:Msqlite)", ".endif") - ChecklinesOptionsMk(mklines) + CheckLinesOptionsMk(mklines) t.CheckOutputLines( "WARN: ~/category/package/options.mk:6: l is used but not defined.", @@ -72,7 +72,7 @@ func (s *Suite) Test_ChecklinesOptionsMk(c *check.C) { // variables, the whole analysis stops. // // This case doesn't happen in practice and thus is not worth being handled in detail. -func (s *Suite) Test_ChecklinesOptionsMk__unexpected_line(c *check.C) { +func (s *Suite) Test_CheckLinesOptionsMk__unexpected_line(c *check.C) { t := s.Init(c) t.SetupCommandLine("-Wno-space") @@ -89,13 +89,13 @@ func (s *Suite) Test_ChecklinesOptionsMk__unexpected_line(c *check.C) { "pre-configure:", "\techo \"In the pre-configure stage.\"") - ChecklinesOptionsMk(mklines) + CheckLinesOptionsMk(mklines) t.CheckOutputLines( "WARN: ~/category/package/options.mk:5: Expected inclusion of \"../../mk/bsd.options.mk\".") } -func (s *Suite) Test_ChecklinesOptionsMk__malformed_condition(c *check.C) { +func (s *Suite) Test_CheckLinesOptionsMk__malformed_condition(c *check.C) { t := s.Init(c) t.SetupCommandLine("-Wno-space") @@ -124,7 +124,7 @@ func (s *Suite) Test_ChecklinesOptionsMk__malformed_condition(c *check.C) { ".if ${OPSYS} == 'Darwin'", ".endif") - ChecklinesOptionsMk(mklines) + CheckLinesOptionsMk(mklines) t.CheckOutputLines( "WARN: ~/category/package/options.mk:13: Invalid condition, unrecognized part: \"${OPSYS} == 'Darwin'\".") diff --git a/pkgtools/pkglint/files/package.go b/pkgtools/pkglint/files/package.go index 2080784f762..a3a5453efe5 100644 --- a/pkgtools/pkglint/files/package.go +++ b/pkgtools/pkglint/files/package.go @@ -121,10 +121,10 @@ func (pkg *Package) checkPossibleDowngrade() { } } -// checklinesBuildlink3Inclusion checks whether the package Makefile and +// checkLinesBuildlink3Inclusion checks whether the package Makefile and // the corresponding buildlink3.mk agree for all included buildlink3.mk // files whether they are included conditionally or unconditionally. -func (pkg *Package) checklinesBuildlink3Inclusion(mklines MkLines) { +func (pkg *Package) checkLinesBuildlink3Inclusion(mklines MkLines) { if trace.Tracing { defer trace.Call0()() } diff --git a/pkgtools/pkglint/files/package_test.go b/pkgtools/pkglint/files/package_test.go index 306e0102ed0..3f8f43366eb 100644 --- a/pkgtools/pkglint/files/package_test.go +++ b/pkgtools/pkglint/files/package_test.go @@ -5,7 +5,7 @@ import ( "strings" ) -func (s *Suite) Test_Package_checklinesBuildlink3Inclusion__file_but_not_package(c *check.C) { +func (s *Suite) Test_Package_checkLinesBuildlink3Inclusion__file_but_not_package(c *check.C) { t := s.Init(c) t.CreateFileLines("category/dependency/buildlink3.mk") @@ -15,14 +15,14 @@ func (s *Suite) Test_Package_checklinesBuildlink3Inclusion__file_but_not_package "", ".include \"../../category/dependency/buildlink3.mk\"") - G.Pkg.checklinesBuildlink3Inclusion(mklines) + G.Pkg.checkLinesBuildlink3Inclusion(mklines) t.CheckOutputLines( "WARN: category/package/buildlink3.mk:3: " + "category/dependency/buildlink3.mk is included by this file but not by the package.") } -func (s *Suite) Test_Package_checklinesBuildlink3Inclusion__package_but_not_file(c *check.C) { +func (s *Suite) Test_Package_checkLinesBuildlink3Inclusion__package_but_not_file(c *check.C) { t := s.Init(c) t.CreateFileLines("category/dependency/buildlink3.mk") @@ -32,16 +32,16 @@ func (s *Suite) Test_Package_checklinesBuildlink3Inclusion__package_but_not_file MkRcsID) t.EnableTracingToLog() - G.Pkg.checklinesBuildlink3Inclusion(mklines) + G.Pkg.checkLinesBuildlink3Inclusion(mklines) // This is only traced but not logged as a regular warning since // several packages have build dependencies that are not needed // for building other packages. These cannot be flagged as warnings. t.CheckOutputLines( - "TRACE: + (*Package).checklinesBuildlink3Inclusion()", + "TRACE: + (*Package).checkLinesBuildlink3Inclusion()", "TRACE: 1 ../../category/dependency/buildlink3.mk/buildlink3.mk "+ "is included by the package but not by the buildlink3.mk file.", - "TRACE: - (*Package).checklinesBuildlink3Inclusion()") + "TRACE: - (*Package).checkLinesBuildlink3Inclusion()") } func (s *Suite) Test_Package_pkgnameFromDistname(c *check.C) { @@ -234,7 +234,7 @@ func (s *Suite) Test_Package_CheckVarorder__license(c *check.C) { t.SetupVartypes() - G.CheckDirent(t.File("x11/9term")) + G.Check(t.File("x11/9term")) // Since the error is grave enough, the warning about the correct position is suppressed. t.CheckOutputLines( @@ -358,7 +358,7 @@ func (s *Suite) Test_Package_determineEffectivePkgVars__same(c *check.C) { "DISTNAME=\tdistname-1.0", "PKGNAME=\tdistname-1.0") - G.CheckDirent(pkg) + G.Check(pkg) t.CheckOutputLines( "NOTE: ~/category/package/Makefile:20: " + @@ -372,7 +372,7 @@ func (s *Suite) Test_Package_determineEffectivePkgVars__invalid_DISTNAME(c *chec pkg := t.SetupPackage("category/package", "DISTNAME=\tpkgname-version") - G.CheckDirent(pkg) + G.Check(pkg) t.CheckOutputLines( "WARN: ~/category/package/Makefile:3: " + @@ -512,7 +512,7 @@ func (s *Suite) Test_Package__varuse_at_load_time(c *check.C) { t.SetupCommandLine("-q", "-Wall,no-space") G.Pkgsrc.LoadInfrastructure() - G.CheckDirent(t.File("category/pkgbase")) + G.Check(t.File("category/pkgbase")) t.CheckOutputLines( "WARN: ~/category/pkgbase/Makefile:14: To use the tool ${FALSE} at load time, bsd.prefs.mk has to be included before.", @@ -568,7 +568,7 @@ func (s *Suite) Test_Package_loadPackageMakefile__PECL_VERSION(c *check.C) { "PECL_VERSION=\t1.1.2", ".include \"../../lang/php/ext.mk\"") - G.CheckDirent(pkg) + G.Check(pkg) } func (s *Suite) Test_Package_checkIncludeConditionally__conditional_and_unconditional_include(c *check.C) { @@ -760,7 +760,7 @@ func (s *Suite) Test__distinfo_from_other_package(c *check.C) { "", "SHA1 (patch-aa) = 1234") - G.CheckDirent("x11/gst-x11") + G.Check("x11/gst-x11") t.CheckOutputLines( "WARN: x11/gst-x11/Makefile: Neither PLIST nor PLIST.common exist, and PLIST_SRC is unset.", @@ -777,7 +777,7 @@ func (s *Suite) Test_Package_checkfilePackageMakefile__GNU_CONFIGURE(c *check.C) "GNU_CONFIGURE=\tyes", "USE_LANGUAGES=\t#") - G.CheckDirent(pkg) + G.Check(pkg) t.CheckOutputLines( "WARN: ~/category/package/Makefile:20: GNU_CONFIGURE almost always needs a C compiler, but \"c\" is not added to USE_LANGUAGES in line 21.") @@ -793,7 +793,7 @@ func (s *Suite) Test_Package_checkfilePackageMakefile__GNU_CONFIGURE_ok(c *check "GNU_CONFIGURE=\tyes", "USE_LANGUAGES=\t# none, really") - G.CheckDirent(pkg) + G.Check(pkg) t.CheckOutputEmpty() } @@ -806,7 +806,7 @@ func (s *Suite) Test_Package_checkfilePackageMakefile__REPLACE_PERL(c *check.C) "REPLACE_PERL=\t*.pl", "NO_CONFIGURE=\tyes") - G.CheckDirent(pkg) + G.Check(pkg) t.CheckOutputLines( "WARN: ~/category/package/Makefile:20: REPLACE_PERL is ignored when NO_CONFIGURE is set (in line 21).") @@ -818,7 +818,7 @@ func (s *Suite) Test_Package_checkfilePackageMakefile__META_PACKAGE_with_distinf pkg := t.SetupPackage("category/package", "META_PACKAGE=\tyes") - G.CheckDirent(pkg) + G.Check(pkg) t.CheckOutputLines( "WARN: ~/category/package/distinfo: " + @@ -832,7 +832,7 @@ func (s *Suite) Test_Package_checkfilePackageMakefile__USE_IMAKE_and_USE_X11(c * "USE_X11=\tyes", "USE_IMAKE=\tyes") - G.CheckDirent(pkg) + G.Check(pkg) t.CheckOutputLines( "NOTE: ~/category/package/Makefile:21: USE_IMAKE makes USE_X11 in line 20 redundant.") @@ -846,7 +846,7 @@ func (s *Suite) Test_Package_readMakefile__skipping(c *check.C) { ".include \"${MYSQL_PKGSRCDIR:S/-client$/-server/}/buildlink3.mk\"") t.EnableTracingToLog() - G.CheckDirent(pkg) + G.Check(pkg) t.EnableSilentTracing() // Since 2018-12-16 there is no warning or note anymore for the @@ -891,7 +891,7 @@ func (s *Suite) Test_Package_readMakefile__relative(c *check.C) { pkg := t.SetupPackage("category/package", ".include \"../package/extra.mk\"") - G.CheckDirent(pkg) + G.Check(pkg) // FIXME: One of the below warnings is redundant. t.CheckOutputLines( @@ -915,7 +915,7 @@ func (s *Suite) Test_Package_checkLocallyModified(c *check.C) { pkg := t.SetupPackage("category/package", "MAINTAINER=\tpkgsrc-users@NetBSD.org") - G.CheckDirent(pkg) + G.Check(pkg) t.CheckOutputEmpty() @@ -924,7 +924,7 @@ func (s *Suite) Test_Package_checkLocallyModified(c *check.C) { t.SetupPackage("category/package", "MAINTAINER=\tmaintainer@example.org") - G.CheckDirent(pkg) + G.Check(pkg) t.CheckOutputLines( "NOTE: ~/category/package/Makefile: " + @@ -936,7 +936,7 @@ func (s *Suite) Test_Package_checkLocallyModified(c *check.C) { "#MAINTAINER=\t# undefined", "OWNER=\towner@example.org") - G.CheckDirent(pkg) + G.Check(pkg) t.CheckOutputLines( "WARN: ~/category/package/Makefile: " + @@ -948,7 +948,7 @@ func (s *Suite) Test_Package_checkLocallyModified(c *check.C) { "MAINTAINER=\tmaintainer@example.org", "OWNER=\towner@example.org") - G.CheckDirent(pkg) + G.Check(pkg) t.CheckOutputLines( "WARN: ~/category/package/Makefile: "+ @@ -960,7 +960,7 @@ func (s *Suite) Test_Package_checkLocallyModified(c *check.C) { G.Username = "owner" - G.CheckDirent(pkg) + G.Check(pkg) t.CheckOutputEmpty() } diff --git a/pkgtools/pkglint/files/patches.go b/pkgtools/pkglint/files/patches.go index f14718cd598..8963991ddbb 100644 --- a/pkgtools/pkglint/files/patches.go +++ b/pkgtools/pkglint/files/patches.go @@ -7,7 +7,7 @@ import ( "strings" ) -func ChecklinesPatch(lines Lines) { +func CheckLinesPatch(lines Lines) { if trace.Tracing { defer trace.Call1(lines.FileName)() } @@ -93,7 +93,7 @@ func (ck *PatchChecker) Check() { ck.lines.Errorf("Contains no patch.") } - ChecklinesTrailingEmptyLines(ck.lines) + CheckLinesTrailingEmptyLines(ck.lines) sha1Before, err := computePatchSha1Hex(ck.lines.FileName) if SaveAutofixChanges(ck.lines) && G.Pkg != nil && err == nil { sha1After, err := computePatchSha1Hex(ck.lines.FileName) diff --git a/pkgtools/pkglint/files/patches_test.go b/pkgtools/pkglint/files/patches_test.go index 0ac69793c1d..cfc5db52045 100644 --- a/pkgtools/pkglint/files/patches_test.go +++ b/pkgtools/pkglint/files/patches_test.go @@ -3,7 +3,7 @@ package pkglint import "gopkg.in/check.v1" // This is how each patch should look like. -func (s *Suite) Test_ChecklinesPatch__with_comment(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__with_comment(c *check.C) { t := s.Init(c) lines := t.NewLines("patch-WithComment", @@ -23,14 +23,14 @@ func (s *Suite) Test_ChecklinesPatch__with_comment(c *check.C) { "+new line", " context after") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputEmpty() } // To make the patch comment clearly visible, it should be surrounded by empty lines. // The missing empty lines are inserted by pkglint. -func (s *Suite) Test_ChecklinesPatch__without_empty_line__autofix(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__without_empty_line__autofix(c *check.C) { t := s.Init(c) t.Chdir("category/package") @@ -53,7 +53,7 @@ func (s *Suite) Test_ChecklinesPatch__without_empty_line__autofix(c *check.C) { t.SetupCommandLine("-Wall", "--autofix") G.Pkg = NewPackage(".") - ChecklinesPatch(patchLines) + CheckLinesPatch(patchLines) t.CheckOutputLines( "AUTOFIX: patch-WithoutEmptyLines:1: Inserting a line \"\" after this line.", @@ -79,7 +79,7 @@ func (s *Suite) Test_ChecklinesPatch__without_empty_line__autofix(c *check.C) { "SHA1 (some patch) = 4938fc8c0b483dc2e33e741b0da883d199e78164") } -func (s *Suite) Test_ChecklinesPatch__no_comment_and_no_empty_lines(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__no_comment_and_no_empty_lines(c *check.C) { t := s.Init(c) patchLines := t.SetupFileLines("patch-WithoutEmptyLines", @@ -90,7 +90,7 @@ func (s *Suite) Test_ChecklinesPatch__no_comment_and_no_empty_lines(c *check.C) "-old line", "+new line") - ChecklinesPatch(patchLines) + CheckLinesPatch(patchLines) // These duplicate notes are actually correct. There should be an // empty line above the documentation and one below it. Since there @@ -103,7 +103,7 @@ func (s *Suite) Test_ChecklinesPatch__no_comment_and_no_empty_lines(c *check.C) "NOTE: ~/patch-WithoutEmptyLines:2: Empty line expected.") } -func (s *Suite) Test_ChecklinesPatch__without_comment(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__without_comment(c *check.C) { t := s.Init(c) lines := t.NewLines("patch-WithoutComment", @@ -117,7 +117,7 @@ func (s *Suite) Test_ChecklinesPatch__without_comment(c *check.C) { "+old line", " context after") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputLines( "ERROR: patch-WithoutComment:3: Each patch must be documented.") @@ -125,7 +125,7 @@ func (s *Suite) Test_ChecklinesPatch__without_comment(c *check.C) { // Autogenerated git "comments" don't count as real comments since they // don't convey any intention of a human developer. -func (s *Suite) Test_ChecklinesPatch__git_without_comment(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__git_without_comment(c *check.C) { t := s.Init(c) lines := t.NewLines("patch-aa", @@ -139,7 +139,7 @@ func (s *Suite) Test_ChecklinesPatch__git_without_comment(c *check.C) { "-old", "+new") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputLines( "ERROR: patch-aa:5: Each patch must be documented.") @@ -160,7 +160,7 @@ func (s *Suite) Test_PatchChecker_checklineSourceAbsolutePathname(c *check.C) { "+val abspath = libdir + \"/libiconv.so.1.0\"", "+const char abspath[] = \"/dev/scd0\";") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputLines( "WARN: patch-aa:10: Found absolute pathname: /dev/scd0") @@ -183,7 +183,7 @@ func (s *Suite) Test_PatchChecker_checklineOtherAbsolutePathname(c *check.C) { "+abspath = $prefix + '/bin/program'", "+abspath=\"$prefix/bin/program\"") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputLines( "WARN: patch-aa:9: Found absolute pathname: /bin/") @@ -192,7 +192,7 @@ func (s *Suite) Test_PatchChecker_checklineOtherAbsolutePathname(c *check.C) { // The output of BSD Make typically contains "*** Error code". // In some really good patches, this output is included in the patch comment, // to document why the patch is necessary. -func (s *Suite) Test_ChecklinesPatch__error_code(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__error_code(c *check.C) { t := s.Init(c) lines := t.NewLines("patch-ErrorCode", @@ -208,12 +208,12 @@ func (s *Suite) Test_ChecklinesPatch__error_code(c *check.C) { "+old line", " context after") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputEmpty() } -func (s *Suite) Test_ChecklinesPatch__wrong_header_order(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__wrong_header_order(c *check.C) { t := s.Init(c) lines := t.NewLines("patch-WrongOrder", @@ -230,14 +230,14 @@ func (s *Suite) Test_ChecklinesPatch__wrong_header_order(c *check.C) { "+old line", " context after") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputLines( "WARN: patch-WrongOrder:7: Unified diff headers should be first ---, then +++.") } // Context diffs are old and deprecated. Therefore pkglint doesn't check them thoroughly. -func (s *Suite) Test_ChecklinesPatch__context_diff(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__context_diff(c *check.C) { t := s.Init(c) lines := t.NewLines("patch-ctx", @@ -247,14 +247,14 @@ func (s *Suite) Test_ChecklinesPatch__context_diff(c *check.C) { "*** history.c.orig", "--- history.c") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputLines( "ERROR: patch-ctx:4: Each patch must be documented.", "WARN: patch-ctx:4: Please use unified diffs (diff -u) for patches.") } -func (s *Suite) Test_ChecklinesPatch__no_patch(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__no_patch(c *check.C) { t := s.Init(c) lines := t.NewLines("patch-aa", @@ -263,13 +263,13 @@ func (s *Suite) Test_ChecklinesPatch__no_patch(c *check.C) { "-- oldfile", "++ newfile") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputLines( "ERROR: patch-aa: Contains no patch.") } -func (s *Suite) Test_ChecklinesPatch__two_patched_files(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__two_patched_files(c *check.C) { t := s.Init(c) lines := t.NewLines("patch-aa", @@ -290,14 +290,14 @@ func (s *Suite) Test_ChecklinesPatch__two_patched_files(c *check.C) { "-old", "+new") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputLines( "WARN: patch-aa: Contains patches for 2 files, should be only one.") } // The patch headers are only recognized as such if they appear directly below each other. -func (s *Suite) Test_ChecklinesPatch__documentation_that_looks_like_patch_lines(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__documentation_that_looks_like_patch_lines(c *check.C) { t := s.Init(c) lines := t.NewLines("patch-aa", @@ -309,13 +309,13 @@ func (s *Suite) Test_ChecklinesPatch__documentation_that_looks_like_patch_lines( "", "*** oldOrNewFile") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputLines( "ERROR: patch-aa: Contains no patch.") } -func (s *Suite) Test_ChecklinesPatch__only_unified_header_but_no_content(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__only_unified_header_but_no_content(c *check.C) { t := s.Init(c) lines := t.NewLines("patch-unified", @@ -326,13 +326,13 @@ func (s *Suite) Test_ChecklinesPatch__only_unified_header_but_no_content(c *chec "--- file.orig", "+++ file") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputLines( "ERROR: patch-unified:EOF: No patch hunks for \"file\".") } -func (s *Suite) Test_ChecklinesPatch__only_context_header_but_no_content(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__only_context_header_but_no_content(c *check.C) { t := s.Init(c) lines := t.NewLines("patch-context", @@ -343,7 +343,7 @@ func (s *Suite) Test_ChecklinesPatch__only_context_header_but_no_content(c *chec "*** file.orig", "--- file") - ChecklinesPatch(lines) + CheckLinesPatch(lines) // Context diffs are deprecated, therefore it is not worth // adding extra code for checking them thoroughly. @@ -354,7 +354,7 @@ func (s *Suite) Test_ChecklinesPatch__only_context_header_but_no_content(c *chec // TODO: Maybe this should only be checked if the patch changes // an absolute path to a relative one, because otherwise these // absolute paths may be intentional. -func (s *Suite) Test_ChecklinesPatch__Makefile_with_absolute_pathnames(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__Makefile_with_absolute_pathnames(c *check.C) { t := s.Init(c) t.SetupCommandLine("-Wabsname", "-Wno-extra") @@ -375,7 +375,7 @@ func (s *Suite) Test_ChecklinesPatch__Makefile_with_absolute_pathnames(c *check. "+\t${prefix}/bin/cp added", " \t/bin/cp context after") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputLines( "WARN: patch-unified:10: Found absolute pathname: /bin/cp", @@ -385,7 +385,7 @@ func (s *Suite) Test_ChecklinesPatch__Makefile_with_absolute_pathnames(c *check. // are also checked, to detect absolute paths that might be overlooked. G.Opts.WarnExtra = true - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputLines( "WARN: patch-unified:8: Found absolute pathname: /bin/cp", @@ -394,7 +394,7 @@ func (s *Suite) Test_ChecklinesPatch__Makefile_with_absolute_pathnames(c *check. "WARN: patch-unified:15: Found absolute pathname: /bin/cp") } -func (s *Suite) Test_ChecklinesPatch__no_newline_with_text_following(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__no_newline_with_text_following(c *check.C) { t := s.Init(c) lines := t.NewLines("patch-aa", @@ -411,13 +411,13 @@ func (s *Suite) Test_ChecklinesPatch__no_newline_with_text_following(c *check.C) "\\ No newline at end of file", "last line (a comment)") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputLines( "WARN: patch-aa:12: Empty line or end of file expected.") } -func (s *Suite) Test_ChecklinesPatch__no_newline(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__no_newline(c *check.C) { t := s.Init(c) lines := t.NewLines("patch-aa", @@ -433,14 +433,14 @@ func (s *Suite) Test_ChecklinesPatch__no_newline(c *check.C) { "+new", "\\ No newline at end of file") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputEmpty() } // Some patch files may end before reaching the expected line count (in this case 7 lines). // This is ok if only context lines are missing. These context lines are assumed to be empty lines. -func (s *Suite) Test_ChecklinesPatch__empty_lines_left_out_at_eof(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__empty_lines_left_out_at_eof(c *check.C) { t := s.Init(c) lines := t.NewLines("patch-aa", @@ -458,14 +458,14 @@ func (s *Suite) Test_ChecklinesPatch__empty_lines_left_out_at_eof(c *check.C) { " 5", " 6") // Line 7 was empty, therefore omitted - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputEmpty() } // In some context lines, the leading space character may be missing. // Since this is no problem for patch(1), pkglint also doesn't complain. -func (s *Suite) Test_ChecklinesPatch__context_lines_with_tab_instead_of_space(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__context_lines_with_tab_instead_of_space(c *check.C) { t := s.Init(c) lines := t.NewLines("patch-aa", @@ -481,28 +481,28 @@ func (s *Suite) Test_ChecklinesPatch__context_lines_with_tab_instead_of_space(c "+new", "\tcontext") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputEmpty() } // Before 2018-01-28, pkglint had panicked when checking an empty // patch file, as a slice index was out of bounds. -func (s *Suite) Test_ChecklinesPatch__autofix_empty_patch(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__autofix_empty_patch(c *check.C) { t := s.Init(c) t.SetupCommandLine("-Wall", "--autofix") lines := t.NewLines("patch-aa", RcsID) - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputEmpty() } // Before 2018-01-28, pkglint had panicked when checking an empty // patch file, as a slice index was out of bounds. -func (s *Suite) Test_ChecklinesPatch__autofix_long_empty_patch(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__autofix_long_empty_patch(c *check.C) { t := s.Init(c) t.SetupCommandLine("-Wall", "--autofix") @@ -510,12 +510,12 @@ func (s *Suite) Test_ChecklinesPatch__autofix_long_empty_patch(c *check.C) { RcsID, "") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputEmpty() } -func (s *Suite) Test_ChecklinesPatch__crlf_autofix(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__crlf_autofix(c *check.C) { t := s.Init(c) t.SetupCommandLine("-Wall", "--autofix") @@ -530,7 +530,7 @@ func (s *Suite) Test_ChecklinesPatch__crlf_autofix(c *check.C) { "-old line", "+new line") - ChecklinesPatch(lines) + CheckLinesPatch(lines) // To relieve the pkgsrc package maintainers from this boring work, // the pkgsrc infrastructure could fix these issues before actually @@ -539,7 +539,7 @@ func (s *Suite) Test_ChecklinesPatch__crlf_autofix(c *check.C) { "AUTOFIX: ~/patch-aa:7: Replacing \"\\r\\n\" with \"\\n\".") } -func (s *Suite) Test_ChecklinesPatch__autogenerated(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__autogenerated(c *check.C) { t := s.Init(c) lines := t.SetupFileLines("patch-aa", @@ -553,13 +553,13 @@ func (s *Suite) Test_ChecklinesPatch__autogenerated(c *check.C) { "-old line", "+: Avoid regenerating within pkgsrc") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputLines( "ERROR: ~/patch-aa:9: This code must not be included in patches.") } -func (s *Suite) Test_ChecklinesPatch__empty_context_lines_in_hunk(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__empty_context_lines_in_hunk(c *check.C) { t := s.Init(c) lines := t.SetupFileLines("patch-aa", @@ -574,7 +574,7 @@ func (s *Suite) Test_ChecklinesPatch__empty_context_lines_in_hunk(c *check.C) { "-old line", "+new line") - ChecklinesPatch(lines) + CheckLinesPatch(lines) // The first context line should start with a single space character, // but that would mean trailing whitespace, so it may be left out. @@ -584,7 +584,7 @@ func (s *Suite) Test_ChecklinesPatch__empty_context_lines_in_hunk(c *check.C) { t.CheckOutputEmpty() } -func (s *Suite) Test_ChecklinesPatch__invalid_line_in_hunk(c *check.C) { +func (s *Suite) Test_CheckLinesPatch__invalid_line_in_hunk(c *check.C) { t := s.Init(c) lines := t.SetupFileLines("patch-aa", @@ -600,7 +600,7 @@ func (s *Suite) Test_ChecklinesPatch__invalid_line_in_hunk(c *check.C) { "<<<<<<<<", "+new line") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputLines( "ERROR: ~/patch-aa:10: Invalid line in unified patch hunk: <<<<<<<<") @@ -620,7 +620,7 @@ func (s *Suite) Test_PatchChecker_checklineAdded__shell(c *check.C) { "-old line", "+new line") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputEmpty() } @@ -639,7 +639,7 @@ func (s *Suite) Test_PatchChecker_checklineAdded__text(c *check.C) { "-old line", "+new line") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputEmpty() } @@ -658,7 +658,7 @@ func (s *Suite) Test_PatchChecker_checklineAdded__unknown(c *check.C) { "-old line", "+new line") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputEmpty() } @@ -679,7 +679,7 @@ func (s *Suite) Test_PatchChecker_checktextRcsid(c *check.C) { "+new line", " $"+"Author: authorship $") - ChecklinesPatch(lines) + CheckLinesPatch(lines) t.CheckOutputLines( "WARN: ~/patch-aa:7: Found RCS tag \"$"+"Id$\". Please remove it.", diff --git a/pkgtools/pkglint/files/pkglint.go b/pkgtools/pkglint/files/pkglint.go index a62d9da7163..c648c45d896 100644 --- a/pkgtools/pkglint/files/pkglint.go +++ b/pkgtools/pkglint/files/pkglint.go @@ -201,7 +201,7 @@ func (pkglint *Pkglint) Main(argv ...string) (exitCode int) { for len(pkglint.Todo) > 0 { item := pkglint.Todo[0] pkglint.Todo = pkglint.Todo[1:] - pkglint.CheckDirent(item) + pkglint.Check(item) } pkglint.Pkgsrc.checkToplevelUnusedLicenses() @@ -285,38 +285,53 @@ func (pkglint *Pkglint) ParseCommandLine(args []string) int { return -1 } -// CheckDirent checks a directory or a single file. +// Check checks a directory entry, which can be a regular file, +// a directory or a symlink (only allowed for the working directory). +// +// This is the method that is called for each command line argument. +// +// It sets up all the global state (infrastructure, wip) for accurately +// classifying the entry. // // During tests, it assumes that Pkgsrc.LoadInfrastructure has been called. // It is the most high-level method for testing pkglint. -func (pkglint *Pkglint) CheckDirent(filename string) { +func (pkglint *Pkglint) Check(dirent string) { if trace.Tracing { - defer trace.Call1(filename)() + defer trace.Call1(dirent)() } - st, err := os.Lstat(filename) + st, err := os.Lstat(dirent) if err != nil || !st.Mode().IsDir() && !st.Mode().IsRegular() { - NewLineWhole(filename).Errorf("No such file or directory.") + NewLineWhole(dirent).Errorf("No such file or directory.") return } isDir := st.Mode().IsDir() isReg := st.Mode().IsRegular() - dir := ifelseStr(isReg, path.Dir(filename), filename) - pkgsrcRel := pkglint.Pkgsrc.ToRel(dir) + dir := dirent + if isReg { + dir = path.Dir(dirent) + } + + basename := path.Base(dirent) + pkgsrcRel := pkglint.Pkgsrc.ToRel(dirent) + pkglint.Wip = matches(pkgsrcRel, `^wip(/|$)`) pkglint.Infrastructure = matches(pkgsrcRel, `^mk(/|$)`) pkgsrcdir := findPkgsrcTopdir(dir) if pkgsrcdir == "" { - NewLineWhole(filename).Errorf("Cannot determine the pkgsrc root directory for %q.", cleanpath(dir)) + NewLineWhole(dirent).Errorf("Cannot determine the pkgsrc root directory for %q.", cleanpath(dir)) return } switch { - case isDir && isEmptyDir(filename): + case isDir && isEmptyDir(dirent): return + case isReg: - pkglint.Checkfile(filename) + depth := strings.Count(pkgsrcRel, "/") + pkglint.checkExecutable(dirent, st.Mode()) + pkglint.checkReg(dirent, basename, depth) return } @@ -328,7 +343,43 @@ func (pkglint *Pkglint) CheckDirent(filename string) { case ".": CheckdirToplevel(dir) default: - NewLineWhole(filename).Errorf("Cannot check directories outside a pkgsrc tree.") + NewLineWhole(dirent).Errorf("Cannot check directories outside a pkgsrc tree.") + } +} + +// checkDirent checks a directory entry based on its filename and its mode +// (regular file, directory, symlink). +func (pkglint *Pkglint) checkDirent(dirent string, mode os.FileMode) { + basename := path.Base(dirent) + + switch { + + case mode.IsRegular(): + pkgsrcRel := pkglint.Pkgsrc.ToRel(dirent) + depth := strings.Count(pkgsrcRel, "/") + pkglint.checkReg(dirent, basename, depth) + + case hasPrefix(basename, "work"): + if pkglint.Opts.Import { + NewLineWhole(dirent).Errorf("Must be cleaned up before committing the package.") + } + return + + case mode.IsDir(): + switch { + case basename == "files" || basename == "patches" || isIgnoredFilename(basename): + // Ok + case matches(dirent, `(?:^|/)files/[^/]*$`): + // Ok + case !isEmptyDir(dirent): + NewLineWhole(dirent).Warnf("Unknown directory name.") + } + + case mode&os.ModeSymlink != 0: + NewLineWhole(dirent).Warnf("Invalid symlink name.") + + default: + NewLineWhole(dirent).Errorf("Only files and directories are allowed in pkgsrc.") } } @@ -392,13 +443,18 @@ func (pkglint *Pkglint) checkdirPackage(dir string) { if path.Base(filename) == "Makefile" { if st, err := os.Lstat(filename); err == nil { - pkglint.checkExecutable(filename, st) + pkglint.checkExecutable(filename, st.Mode()) } if pkglint.Opts.CheckMakefile { pkg.checkfilePackageMakefile(filename, mklines) } } else { - pkglint.Checkfile(filename) + st, err := os.Lstat(filename) + if err != nil { + NewLineWhole(filename).Errorf("Cannot determine file type: %s", err) + } else { + pkglint.checkDirent(filename, st.Mode()) + } } if contains(filename, "/patches/patch-") { havePatches = true @@ -478,17 +534,17 @@ func resolveVariableRefs(text string) (resolved string) { } } -func CheckfileExtra(filename string) { +func CheckFileOther(filename string) { if trace.Tracing { defer trace.Call1(filename)() } if lines := Load(filename, NotEmpty|LogErrors); lines != nil { - ChecklinesTrailingEmptyLines(lines) + CheckLinesTrailingEmptyLines(lines) } } -func ChecklinesDescr(lines Lines) { +func CheckLinesDescr(lines Lines) { if trace.Tracing { defer trace.Call1(lines.FileName)() } @@ -498,11 +554,16 @@ func ChecklinesDescr(lines Lines) { ck.CheckLength(80) ck.CheckTrailingWhitespace() ck.CheckValidCharacters() + if contains(line.Text, "${") { - line.Notef("Variables are not expanded in the DESCR file.") + for _, token := range NewMkParser(nil, line.Text, false).MkTokens() { + if token.Varuse != nil && G.Pkgsrc.VariableType(token.Varuse.varname) != nil { + line.Notef("Variables are not expanded in the DESCR file.") + } + } } } - ChecklinesTrailingEmptyLines(lines) + CheckLinesTrailingEmptyLines(lines) if maxLines := 24; lines.Len() > maxLines { line := lines.Lines[maxLines] @@ -516,28 +577,30 @@ func ChecklinesDescr(lines Lines) { SaveAutofixChanges(lines) } -func ChecklinesMessage(lines Lines) { +func CheckLinesMessage(lines Lines) { if trace.Tracing { defer trace.Call1(lines.FileName)() } - explanation := []string{ - "A MESSAGE file should consist of a header line, having 75 \"=\"", - "characters, followed by a line containing only the RCS Id, then an", - "empty line, your text and finally the footer line, which is the", - "same as the header line."} + explanation := func() []string { + return []string{ + "A MESSAGE file should consist of a header line, having 75 \"=\"", + "characters, followed by a line containing only the RCS Id, then an", + "empty line, your text and finally the footer line, which is the", + "same as the header line."} + } if lines.Len() < 3 { lines.LastLine().Warnf("File too short.") - G.Explain(explanation...) + G.Explain(explanation()...) return } - hline := strings.Repeat("=", 75) + hline := "===========================================================================" if line := lines.Lines[0]; line.Text != hline { fix := line.Autofix() fix.Warnf("Expected a line of exactly 75 \"=\" characters.") - fix.Explain(explanation...) + fix.Explain(explanation()...) fix.InsertBefore(hline) fix.Apply() lines.CheckRcsID(0, ``, "") @@ -553,16 +616,16 @@ func ChecklinesMessage(lines Lines) { if lastLine := lines.LastLine(); lastLine.Text != hline { fix := lastLine.Autofix() fix.Warnf("Expected a line of exactly 75 \"=\" characters.") - fix.Explain(explanation...) + fix.Explain(explanation()...) fix.InsertAfter(hline) fix.Apply() } - ChecklinesTrailingEmptyLines(lines) + CheckLinesTrailingEmptyLines(lines) SaveAutofixChanges(lines) } -func CheckfileMk(filename string) { +func CheckFileMk(filename string) { if trace.Tracing { defer trace.Call1(filename)() } @@ -576,14 +639,7 @@ func CheckfileMk(filename string) { mklines.SaveAutofixChanges() } -func (pkglint *Pkglint) Checkfile(filename string) { - if trace.Tracing { - defer trace.Call1(filename)() - } - - basename := path.Base(filename) - pkgsrcRel := pkglint.Pkgsrc.ToRel(filename) - depth := strings.Count(pkgsrcRel, "/") +func (pkglint *Pkglint) checkReg(filename, basename string, depth int) { if depth == 2 && !pkglint.Wip { if contains(basename, "README") || contains(basename, "TODO") { @@ -593,8 +649,7 @@ func (pkglint *Pkglint) Checkfile(filename string) { } switch { - case hasPrefix(basename, "work"), - hasSuffix(basename, "~"), + case hasSuffix(basename, "~"), hasSuffix(basename, ".orig"), hasSuffix(basename, ".rej"), contains(basename, "README") && depth == 2, @@ -605,88 +660,56 @@ func (pkglint *Pkglint) Checkfile(filename string) { return } - st, err := os.Lstat(filename) - if err != nil { - NewLineWhole(filename).Errorf("Cannot determine file type: %s", err) - return - } - - pkglint.checkExecutable(filename, st) - pkglint.checkMode(filename, st.Mode()) -} - -// checkMode checks a directory entry based on its filename and its mode -// (regular file, directory, symlink). -func (pkglint *Pkglint) checkMode(filename string, mode os.FileMode) { - basename := path.Base(filename) switch { - case mode.IsDir(): - switch { - case basename == "files" || basename == "patches" || isIgnoredFilename(basename): - // Ok - case matches(filename, `(?:^|/)files/[^/]*$`): - // Ok - case !isEmptyDir(filename): - NewLineWhole(filename).Warnf("Unknown directory name.") - } - - case mode&os.ModeSymlink != 0: - if !hasPrefix(basename, "work") { - NewLineWhole(filename).Warnf("Unknown symlink name.") - } - - case !mode.IsRegular(): - NewLineWhole(filename).Errorf("Only files and directories are allowed in pkgsrc.") - case basename == "ALTERNATIVES": if pkglint.Opts.CheckAlternatives { - CheckfileAlternatives(filename) + CheckFileAlternatives(filename) } case basename == "buildlink3.mk": if pkglint.Opts.CheckBuildlink3 { if mklines := LoadMk(filename, NotEmpty|LogErrors); mklines != nil { - ChecklinesBuildlink3Mk(mklines) + CheckLinesBuildlink3Mk(mklines) } } case hasPrefix(basename, "DESCR"): if pkglint.Opts.CheckDescr { if lines := Load(filename, NotEmpty|LogErrors); lines != nil { - ChecklinesDescr(lines) + CheckLinesDescr(lines) } } case basename == "distinfo": if pkglint.Opts.CheckDistinfo { if lines := Load(filename, NotEmpty|LogErrors); lines != nil { - ChecklinesDistinfo(lines) + CheckLinesDistinfo(lines) } } case basename == "DEINSTALL" || basename == "INSTALL": if pkglint.Opts.CheckInstall { - CheckfileExtra(filename) + CheckFileOther(filename) } case hasPrefix(basename, "MESSAGE"): if pkglint.Opts.CheckMessage { if lines := Load(filename, NotEmpty|LogErrors); lines != nil { - ChecklinesMessage(lines) + CheckLinesMessage(lines) } } case basename == "options.mk": if pkglint.Opts.CheckOptions { if mklines := LoadMk(filename, NotEmpty|LogErrors); mklines != nil { - ChecklinesOptionsMk(mklines) + CheckLinesOptionsMk(mklines) } } case matches(basename, `^patch-[-A-Za-z0-9_.~+]*[A-Za-z0-9_]$`): if pkglint.Opts.CheckPatches { if lines := Load(filename, NotEmpty|LogErrors); lines != nil { - ChecklinesPatch(lines) + CheckLinesPatch(lines) } } @@ -700,13 +723,13 @@ func (pkglint *Pkglint) checkMode(filename string, mode os.FileMode) { case matches(basename, `^(?:.*\.mk|Makefile.*)$`) && !matches(filename, `files/`) && !matches(filename, `patches/`): if pkglint.Opts.CheckMk { - CheckfileMk(filename) + CheckFileMk(filename) } case hasPrefix(basename, "PLIST"): if pkglint.Opts.CheckPlist { if lines := Load(filename, NotEmpty|LogErrors); lines != nil { - ChecklinesPlist(lines) + CheckLinesPlist(lines) } } @@ -725,17 +748,17 @@ func (pkglint *Pkglint) checkMode(filename string, mode os.FileMode) { default: NewLineWhole(filename).Warnf("Unexpected file found.") if pkglint.Opts.CheckExtra { - CheckfileExtra(filename) + CheckFileOther(filename) } } } -func (pkglint *Pkglint) checkExecutable(filename string, st os.FileInfo) { +func (pkglint *Pkglint) checkExecutable(filename string, mode os.FileMode) { switch { - case !st.Mode().IsRegular(): + case !mode.IsRegular(): // Directories and other entries may be executable. - case st.Mode().Perm()&0111 == 0: + case mode.Perm()&0111 == 0: // Good. case isCommitted(filename): @@ -756,7 +779,7 @@ func (pkglint *Pkglint) checkExecutable(filename string, st os.FileInfo) { fix.Custom(func(showAutofix, autofix bool) { fix.Describef(0, "Clearing executable bits") if autofix { - if err := os.Chmod(filename, st.Mode()&^0111); err != nil { + if err := os.Chmod(filename, mode&^0111); err != nil { line.Errorf("Cannot clear executable bits: %s", err) } } @@ -765,14 +788,14 @@ func (pkglint *Pkglint) checkExecutable(filename string, st os.FileInfo) { } } -func ChecklinesTrailingEmptyLines(lines Lines) { - // XXX: Maybe move to LinesChecker if there are enough similar functions. - +func CheckLinesTrailingEmptyLines(lines Lines) { max := lines.Len() + last := max for last > 1 && lines.Lines[last-1].Text == "" { last-- } + if last != max { lines.Lines[last].Notef("Trailing empty lines.") } diff --git a/pkgtools/pkglint/files/pkglint_test.go b/pkgtools/pkglint/files/pkglint_test.go index 768900bd58f..54a1d34ec04 100644 --- a/pkgtools/pkglint/files/pkglint_test.go +++ b/pkgtools/pkglint/files/pkglint_test.go @@ -1,13 +1,10 @@ package pkglint import ( - "io/ioutil" - "path" - "strings" - "time" - "gopkg.in/check.v1" + "io/ioutil" "os" + "strings" ) func (s *Suite) Test_Pkglint_Main__help(c *check.C) { @@ -134,7 +131,7 @@ func (s *Suite) Test_Pkglint_Main__panic(c *check.C) { // initialize only those parts of the infrastructure they really // need. // -// Especially covers Pkglint.ShowSummary and Pkglint.Checkfile. +// Especially covers Pkglint.ShowSummary and Pkglint.checkReg. func (s *Suite) Test_Pkglint_Main__complete_package(c *check.C) { t := s.Init(c) @@ -283,47 +280,47 @@ func (s *Suite) Test_Pkglint__coverage(c *check.C) { G.out = NewSeparatorWriter(os.Stdout) G.err = NewSeparatorWriter(os.Stderr) trace.Out = os.Stdout - G.Main(append([]string{"pkglint"}, fields(cmdline)...)...) + G.Main(append([]string{"pkglint"}, strings.Fields(cmdline)...)...) } } -func (s *Suite) Test_Pkglint_CheckDirent__outside(c *check.C) { +func (s *Suite) Test_Pkglint_Check__outside(c *check.C) { t := s.Init(c) t.CreateFileLines("empty") - G.CheckDirent(t.File(".")) + G.Check(t.File(".")) t.CheckOutputLines( "ERROR: ~: Cannot determine the pkgsrc root directory for \"~\".") } -func (s *Suite) Test_Pkglint_CheckDirent__empty_directory(c *check.C) { +func (s *Suite) Test_Pkglint_Check__empty_directory(c *check.C) { t := s.Init(c) t.SetupPkgsrc() t.CreateFileLines("category/package/CVS/Entries") - G.CheckDirent(t.File("category/package")) + G.Check(t.File("category/package")) // Empty directories are silently skipped. t.CheckOutputEmpty() } -func (s *Suite) Test_Pkglint_CheckDirent__files_directory(c *check.C) { +func (s *Suite) Test_Pkglint_Check__files_directory(c *check.C) { t := s.Init(c) t.SetupPkgsrc() t.CreateFileLines("category/package/files/README.md") - G.CheckDirent(t.File("category/package/files")) + G.Check(t.File("category/package/files")) // This diagnostic is not really correct, but it's an edge case anyway. t.CheckOutputLines( "ERROR: ~/category/package/files: Cannot check directories outside a pkgsrc tree.") } -func (s *Suite) Test_Pkglint_CheckDirent__manual_patch(c *check.C) { +func (s *Suite) Test_Pkglint_Check__manual_patch(c *check.C) { t := s.Init(c) t.SetupPkgsrc() @@ -331,7 +328,7 @@ func (s *Suite) Test_Pkglint_CheckDirent__manual_patch(c *check.C) { t.CreateFileLines("category/package/Makefile", MkRcsID) - G.CheckDirent(t.File("category/package")) + G.Check(t.File("category/package")) t.CheckOutputLines( "WARN: ~/category/package/Makefile: Neither PLIST nor PLIST.common exist, and PLIST_SRC is unset.", @@ -340,7 +337,7 @@ func (s *Suite) Test_Pkglint_CheckDirent__manual_patch(c *check.C) { "WARN: ~/category/package/Makefile: Each package should define a COMMENT.") } -func (s *Suite) Test_Pkglint_CheckDirent(c *check.C) { +func (s *Suite) Test_Pkglint_Check(c *check.C) { t := s.Init(c) t.CreateFileLines("mk/bsd.pkg.mk") @@ -348,22 +345,22 @@ func (s *Suite) Test_Pkglint_CheckDirent(c *check.C) { t.CreateFileLines("category/Makefile") t.CreateFileLines("Makefile") - G.CheckDirent(t.File(".")) + G.Check(t.File(".")) t.CheckOutputLines( "ERROR: ~/Makefile: Must not be empty.") - G.CheckDirent(t.File("category")) + G.Check(t.File("category")) t.CheckOutputLines( "ERROR: ~/category/Makefile: Must not be empty.") - G.CheckDirent(t.File("category/package")) + G.Check(t.File("category/package")) t.CheckOutputLines( "ERROR: ~/category/package/Makefile: Must not be empty.") - G.CheckDirent(t.File("category/package/nonexistent")) + G.Check(t.File("category/package/nonexistent")) t.CheckOutputLines( "ERROR: ~/category/package/nonexistent: No such file or directory.") @@ -412,37 +409,46 @@ func (s *Suite) Test_resolveVariableRefs__special_chars(c *check.C) { c.Check(resolved, equals, "gst-plugins0.10-x11/distinfo") } -func (s *Suite) Test_ChecklinesDescr(c *check.C) { +func (s *Suite) Test_CheckLinesDescr(c *check.C) { t := s.Init(c) + t.SetupVartypes() lines := t.NewLines("DESCR", - strings.Repeat("X", 90), - "", "", "", "", "", "", "", "", "10", + "word "+strings.Repeat("X", 80), + strings.Repeat("X", 90), // No warning since there are no spaces. + "", "", "", "", "", "", "", "10", "Try ${PREFIX}", "", "", "", "", "", "", "", "", "20", - "", "", "", "", "", "", "", "", "", "30") + "... expressions like ${key} to ... ${unfinished", + "", "", "", "", "", "", "", "", "30") - ChecklinesDescr(lines) + CheckLinesDescr(lines) + // The package author may think that variables like ${PREFIX} + // are expanded in DESCR files too, but that doesn't happen. + // + // Variables that are not well-known in pkgsrc are not warned + // about since these are probably legitimate examples, as seen + // in devel/go-properties/DESCR. t.CheckOutputLines( "WARN: DESCR:1: Line too long (should be no more than 80 characters).", "NOTE: DESCR:11: Variables are not expanded in the DESCR file.", "WARN: DESCR:25: File too long (should be no more than 24 lines).") } -func (s *Suite) Test_ChecklinesMessage__short(c *check.C) { +func (s *Suite) Test_CheckLinesMessage__short(c *check.C) { t := s.Init(c) lines := t.NewLines("MESSAGE", "one line") - ChecklinesMessage(lines) + CheckLinesMessage(lines) t.CheckOutputLines( "WARN: MESSAGE:1: File too short.") } -func (s *Suite) Test_ChecklinesMessage__malformed(c *check.C) { +func (s *Suite) Test_CheckLinesMessage__malformed(c *check.C) { t := s.Init(c) lines := t.NewLines("MESSAGE", @@ -452,7 +458,7 @@ func (s *Suite) Test_ChecklinesMessage__malformed(c *check.C) { "4", "5") - ChecklinesMessage(lines) + CheckLinesMessage(lines) t.CheckOutputLines( "WARN: MESSAGE:1: Expected a line of exactly 75 \"=\" characters.", @@ -460,7 +466,7 @@ func (s *Suite) Test_ChecklinesMessage__malformed(c *check.C) { "WARN: MESSAGE:5: Expected a line of exactly 75 \"=\" characters.") } -func (s *Suite) Test_ChecklinesMessage__autofix(c *check.C) { +func (s *Suite) Test_CheckLinesMessage__autofix(c *check.C) { t := s.Init(c) t.SetupCommandLine("-Wall", "--autofix") @@ -471,7 +477,7 @@ func (s *Suite) Test_ChecklinesMessage__autofix(c *check.C) { "4", "5") - ChecklinesMessage(lines) + CheckLinesMessage(lines) t.CheckOutputLines( "AUTOFIX: ~/MESSAGE:1: Inserting a line "+ @@ -494,18 +500,18 @@ func (s *Suite) Test_ChecklinesMessage__autofix(c *check.C) { // Demonstrates that an ALTERNATIVES file can be tested individually, // without any dependencies on a whole package or a PLIST file. -func (s *Suite) Test_Pkglint_Checkfile__alternatives(c *check.C) { +func (s *Suite) Test_Pkglint_checkReg__alternatives(c *check.C) { t := s.Init(c) t.SetupPkgsrc() lines := t.SetupFileLines("category/package/ALTERNATIVES", - "bin/tar @PREFIX@/bin/gnu-tar") + "bin/tar bin/gnu-tar") G.Main("pkglint", lines.FileName) t.CheckOutputLines( - "NOTE: ~/category/package/ALTERNATIVES:1: @PREFIX@/ can be omitted from the filename.", - "Looks fine.", + "ERROR: ~/category/package/ALTERNATIVES:1: Alternative implementation \"bin/gnu-tar\" must be an absolute path.", + "1 error and 0 warnings found.", "(Run \"pkglint -e\" to show explanations.)") } @@ -543,7 +549,7 @@ func (s *Suite) Test_Pkglint__profiling_error(c *check.C) { c.Check(t.Output(), check.Matches, `^FATAL: Cannot create profiling file: open pkglint\.pprof: .*\n$`) } -func (s *Suite) Test_Pkglint_Checkfile__in_current_working_directory(c *check.C) { +func (s *Suite) Test_Pkglint_checkReg__in_current_working_directory(c *check.C) { t := s.Init(c) t.SetupPkgsrc() @@ -683,7 +689,7 @@ func (s *Suite) Test_Pkglint_ToolByVarname(c *check.C) { c.Check(G.ToolByVarname("TOOL").String(), equals, "tool:TOOL::AtRunTime") } -func (s *Suite) Test_CheckfileExtra(c *check.C) { +func (s *Suite) Test_CheckFileOther(c *check.C) { t := s.Init(c) t.SetupCommandLine("-Call", "-Wall,no-space") @@ -693,12 +699,12 @@ func (s *Suite) Test_CheckfileExtra(c *check.C) { t.CreateFileLines("category/package/DEINSTALL", "#! /bin/sh") - G.CheckDirent(pkg) + G.Check(pkg) t.CheckOutputEmpty() } -func (s *Suite) Test_Pkglint_Checkfile__before_import(c *check.C) { +func (s *Suite) Test_Pkglint_Check__invalid_files_before_import(c *check.C) { t := s.Init(c) t.SetupCommandLine("-Call", "-Wall,no-space", "--import") @@ -708,7 +714,7 @@ func (s *Suite) Test_Pkglint_Checkfile__before_import(c *check.C) { t.CreateFileLines("category/package/Makefile.orig") t.CreateFileLines("category/package/Makefile.rej") - G.CheckDirent(pkg) + G.Check(pkg) t.CheckOutputLines( "ERROR: ~/category/package/Makefile.orig: Must be cleaned up before committing the package.", @@ -717,7 +723,7 @@ func (s *Suite) Test_Pkglint_Checkfile__before_import(c *check.C) { "ERROR: ~/category/package/work: Must be cleaned up before committing the package.") } -func (s *Suite) Test_Pkglint_Checkfile__errors(c *check.C) { +func (s *Suite) Test_Pkglint_checkDirent__errors(c *check.C) { t := s.Init(c) t.SetupCommandLine("-Call", "-Wall,no-space") @@ -726,18 +732,17 @@ func (s *Suite) Test_Pkglint_Checkfile__errors(c *check.C) { t.CreateFileLines("category/package/files/subdir/subsub/file") G.Pkgsrc.LoadInfrastructure() - G.Checkfile(t.File("category/package/options.mk")) - G.Checkfile(t.File("category/package/files/subdir")) - G.Checkfile(t.File("category/package/files/subdir/subsub")) - G.Checkfile(t.File("category/package/files")) + G.checkDirent(t.File("category/package/options.mk"), 0444) + G.checkDirent(t.File("category/package/files/subdir"), 0555|os.ModeDir) + G.checkDirent(t.File("category/package/files/subdir/subsub"), 0555|os.ModeDir) + G.checkDirent(t.File("category/package/files"), 0555|os.ModeDir) - c.Check(t.Output(), check.Matches, `^`+ - `ERROR: ~/category/package/options.mk: Cannot determine file type: .*\n`+ - `WARN: ~/category/package/files/subdir/subsub: Unknown directory name\.\n`+ - `$`) + t.CheckOutputLines( + "ERROR: ~/category/package/options.mk: Cannot be read.", + "WARN: ~/category/package/files/subdir/subsub: Unknown directory name.") } -func (s *Suite) Test_Pkglint_Checkfile__file_selection(c *check.C) { +func (s *Suite) Test_Pkglint_checkDirent__file_selection(c *check.C) { t := s.Init(c) t.SetupCommandLine("-Call", "-Wall,no-space") @@ -750,16 +755,16 @@ func (s *Suite) Test_Pkglint_Checkfile__file_selection(c *check.C) { RcsID) G.Pkgsrc.LoadInfrastructure() - G.Checkfile(t.File("doc/CHANGES-2018")) - G.Checkfile(t.File("category/package/buildlink3.mk")) - G.Checkfile(t.File("category/package/unexpected.txt")) + G.checkDirent(t.File("doc/CHANGES-2018"), 0444) + G.checkDirent(t.File("category/package/buildlink3.mk"), 0444) + G.checkDirent(t.File("category/package/unexpected.txt"), 0444) t.CheckOutputLines( "WARN: ~/category/package/buildlink3.mk:EOF: Expected a BUILDLINK_TREE line.", "WARN: ~/category/package/unexpected.txt: Unexpected file found.") } -func (s *Suite) Test_Pkglint_Checkfile__readme_and_todo(c *check.C) { +func (s *Suite) Test_Pkglint_checkReg__readme_and_todo(c *check.C) { t := s.Init(c) t.CreateFileLines("category/Makefile", @@ -829,54 +834,54 @@ func (s *Suite) Test_Pkglint_Checkfile__readme_and_todo(c *check.C) { "4 errors and 0 warnings found.") } -func (s *Suite) Test_Pkglint_Checkfile__unknown_file_in_patches(c *check.C) { +func (s *Suite) Test_Pkglint_checkReg__unknown_file_in_patches(c *check.C) { t := s.Init(c) t.CreateFileDummyPatch("category/Makefile/patches/index") - G.Checkfile(t.File("category/Makefile/patches/index")) + G.checkReg(t.File("category/Makefile/patches/index"), "index", 3) t.CheckOutputLines( "WARN: ~/category/Makefile/patches/index: " + "Patch files should be named \"patch-\", followed by letters, '-', '_', '.', and digits only.") } -func (s *Suite) Test_Pkglint_Checkfile__file_in_files(c *check.C) { +func (s *Suite) Test_Pkglint_checkReg__file_in_files(c *check.C) { t := s.Init(c) t.CreateFileLines("category/package/files/index") - G.Checkfile(t.File("category/package/files/index")) + G.checkReg(t.File("category/package/files/index"), "index", 3) // These files are ignored since they could contain anything. t.CheckOutputEmpty() } -func (s *Suite) Test_Pkglint_Checkfile__spec(c *check.C) { +func (s *Suite) Test_Pkglint_checkReg__spec(c *check.C) { t := s.Init(c) t.CreateFileLines("category/package/spec") t.CreateFileLines("regress/package/spec") - G.Checkfile(t.File("category/package/spec")) - G.Checkfile(t.File("regress/package/spec")) + G.checkReg(t.File("category/package/spec"), "spec", 2) + G.checkReg(t.File("regress/package/spec"), "spec", 2) t.CheckOutputLines( "WARN: ~/category/package/spec: Only packages in regress/ may have spec files.") } -func (s *Suite) Test_Pkglint_checkMode__skipped(c *check.C) { +func (s *Suite) Test_Pkglint_checkDirent__skipped(c *check.C) { t := s.Init(c) - G.checkMode("work", os.ModeSymlink) - G.checkMode("work.i386", os.ModeSymlink) - G.checkMode("work.hostname", os.ModeSymlink) - G.checkMode("other", os.ModeSymlink) + G.checkDirent("work", os.ModeSymlink) + G.checkDirent("work.i386", os.ModeSymlink) + G.checkDirent("work.hostname", os.ModeSymlink) + G.checkDirent("other", os.ModeSymlink) - G.checkMode("device", os.ModeDevice) + G.checkDirent("device", os.ModeDevice) t.CheckOutputLines( - "WARN: other: Unknown symlink name.", + "WARN: other: Invalid symlink name.", "ERROR: device: Only files and directories are allowed in pkgsrc.") } @@ -941,7 +946,7 @@ func (s *Suite) Test_Pkglint_checkdirPackage__patch_without_distinfo(c *check.C) t.CreateFileDummyPatch("category/package/patches/patch-aa") t.Remove("category/package/distinfo") - G.CheckDirent(pkg) + G.Check(pkg) // FIXME: One of the below warnings is redundant. t.CheckOutputLines( @@ -989,7 +994,7 @@ func (s *Suite) Test_Pkglint_checkdirPackage__filename_with_variable(c *check.C) // // TODO: iterate over variables in simple .for loops like the above. // TODO: when implementing the above, take care of deeply nested loops (42.zip). - G.CheckDirent(pkg) + G.Check(pkg) t.CheckOutputEmpty() } @@ -1002,17 +1007,19 @@ func (s *Suite) Test_Pkglint_checkdirPackage__ALTERNATIVES(c *check.C) { t.CreateFileLines("category/package/ALTERNATIVES", "bin/wrapper bin/wrapper-impl") - G.CheckDirent(pkg) + G.Check(pkg) t.CheckOutputLines( - "ERROR: ~/category/package/ALTERNATIVES:1: " + - "Alternative implementation \"bin/wrapper-impl\" must appear in the PLIST.") + "ERROR: ~/category/package/ALTERNATIVES:1: "+ + "Alternative implementation \"bin/wrapper-impl\" must appear in the PLIST.", + "ERROR: ~/category/package/ALTERNATIVES:1: "+ + "Alternative implementation \"bin/wrapper-impl\" must be an absolute path.") } -func (s *Suite) Test_CheckfileMk__enoent(c *check.C) { +func (s *Suite) Test_CheckFileMk__enoent(c *check.C) { t := s.Init(c) - CheckfileMk(t.File("filename.mk")) + CheckFileMk(t.File("filename.mk")) t.CheckOutputLines( "ERROR: ~/filename.mk: Cannot be read.") @@ -1022,16 +1029,15 @@ func (s *Suite) Test_Pkglint_checkExecutable(c *check.C) { t := s.Init(c) filename := t.File("file.mk") - fileInfo := ExecutableFileInfo{path.Base(filename)} - G.checkExecutable(filename, fileInfo) + G.checkExecutable(filename, 0555) t.CheckOutputLines( "WARN: ~/file.mk: Should not be executable.") t.SetupCommandLine("--autofix") - G.checkExecutable(filename, fileInfo) + G.checkExecutable(filename, 0555) // FIXME: The error message "Cannot clear executable bits" is swallowed. t.CheckOutputLines( @@ -1044,9 +1050,8 @@ func (s *Suite) Test_Pkglint_checkExecutable__already_committed(c *check.C) { t.CreateFileLines("CVS/Entries", "/file.mk/modified////") filename := t.File("file.mk") - fileInfo := ExecutableFileInfo{path.Base(filename)} - G.checkExecutable(filename, fileInfo) + G.checkExecutable(filename, 0555) // See the "Too late" comment in Pkglint.checkExecutable. t.CheckOutputEmpty() @@ -1090,16 +1095,3 @@ func (s *Suite) Test_Main(c *check.C) { "Looks fine.") // outProfiling is not checked because it contains timing information. } - -// ExecutableFileInfo mocks a FileInfo because on Windows, -// regular files don't have the executable bit. -type ExecutableFileInfo struct { - name string -} - -func (i ExecutableFileInfo) Name() string { return i.name } -func (i ExecutableFileInfo) Size() int64 { return 13 } -func (i ExecutableFileInfo) Mode() os.FileMode { return 0777 } -func (i ExecutableFileInfo) ModTime() time.Time { return time.Unix(0, 0) } -func (i ExecutableFileInfo) IsDir() bool { return false } -func (i ExecutableFileInfo) Sys() interface{} { return nil } diff --git a/pkgtools/pkglint/files/pkgsrc.go b/pkgtools/pkglint/files/pkgsrc.go index 1617242072c..fba2faba51f 100644 --- a/pkgtools/pkglint/files/pkgsrc.go +++ b/pkgtools/pkglint/files/pkgsrc.go @@ -767,7 +767,7 @@ func (src *Pkgsrc) loadMasterSites() { if mkline.IsVarassign() { varname := mkline.Varname() if hasPrefix(varname, "MASTER_SITE_") && varname != "MASTER_SITE_BACKUP" { - for _, url := range fields(mkline.Value()) { + for _, url := range mkline.ValueFields(mkline.Value()) { if matches(url, `^(?:http://|https://|ftp://)`) { src.registerMasterSite(varname, url) } diff --git a/pkgtools/pkglint/files/pkgsrc_test.go b/pkgtools/pkglint/files/pkgsrc_test.go index 2ff92dab83f..6a45ab064d4 100644 --- a/pkgtools/pkglint/files/pkgsrc_test.go +++ b/pkgtools/pkglint/files/pkgsrc_test.go @@ -155,7 +155,7 @@ func (s *Suite) Test_Pkgsrc_loadTools__BUILD_DEFS(c *check.C) { "_BUILD_DEFS+=\tPKG_SYSCONFBASEDIR PKG_SYSCONFDIR") G.Pkgsrc.LoadInfrastructure() - G.CheckDirent(pkg) + G.Check(pkg) c.Check(G.Pkgsrc.IsBuildDef("PKG_SYSCONFDIR"), equals, true) c.Check(G.Pkgsrc.IsBuildDef("VARBASE"), equals, false) @@ -233,7 +233,7 @@ func (s *Suite) Test_Pkgsrc_loadDocChangesFromFile__wip(c *check.C) { "\to package-1.13 [cool new features]") G.Pkgsrc.LoadInfrastructure() - G.CheckDirent(pkg) + G.Check(pkg) t.CheckOutputLines( "WARN: ~/wip/package/Makefile:3: This package should be updated to 1.13 ([cool new features]).") @@ -463,13 +463,13 @@ func (s *Suite) Test_Pkgsrc_VariableType(c *check.C) { checkType("_PERL5_PACKLIST_AWK_STRIP_DESTDIR", "") checkType("SOME_DIR", "Pathname (guessed)") checkType("SOMEDIR", "Pathname (guessed)") - checkType("SEARCHPATHS", "ShellList of Pathname (guessed)") + checkType("SEARCHPATHS", "List of Pathname (guessed)") checkType("MYPACKAGE_USER", "UserGroupName (guessed)") checkType("MYPACKAGE_GROUP", "UserGroupName (guessed)") - checkType("MY_CMD_ENV", "ShellList of ShellWord (guessed)") - checkType("MY_CMD_ARGS", "ShellList of ShellWord (guessed)") - checkType("MY_CMD_CFLAGS", "ShellList of CFlag (guessed)") - checkType("MY_CMD_LDFLAGS", "ShellList of LdFlag (guessed)") + checkType("MY_CMD_ENV", "List of ShellWord (guessed)") + checkType("MY_CMD_ARGS", "List of ShellWord (guessed)") + checkType("MY_CMD_CFLAGS", "List of CFlag (guessed)") + checkType("MY_CMD_LDFLAGS", "List of LdFlag (guessed)") checkType("PLIST.abcde", "Yes") } @@ -482,12 +482,12 @@ func (s *Suite) Test_Pkgsrc_VariableType__varparam(c *check.C) { t1 := G.Pkgsrc.VariableType("FONT_DIRS") c.Assert(t1, check.NotNil) - c.Check(t1.String(), equals, "ShellList of PathMask (guessed)") + c.Check(t1.String(), equals, "List of PathMask (guessed)") t2 := G.Pkgsrc.VariableType("FONT_DIRS.ttf") c.Assert(t2, check.NotNil) - c.Check(t2.String(), equals, "ShellList of PathMask (guessed)") + c.Check(t2.String(), equals, "List of PathMask (guessed)") } // Guessing the variable type also works for variables that are @@ -516,7 +516,7 @@ func (s *Suite) Test_Pkgsrc_VariableType__from_mk(c *check.C) { G.Main("pkglint", "-Wall", pkg) if typ := G.Pkgsrc.VariableType("PKGSRC_MAKE_ENV"); c.Check(typ, check.NotNil) { - c.Check(typ.String(), equals, "ShellList of ShellWord (guessed)") + c.Check(typ.String(), equals, "List of ShellWord (guessed)") } if typ := G.Pkgsrc.VariableType("CPPPATH"); c.Check(typ, check.NotNil) { @@ -532,7 +532,7 @@ func (s *Suite) Test_Pkgsrc_VariableType__from_mk(c *check.C) { // and CPPPATH since these two variables are defined somewhere in the // infrastructure. t.CheckOutputLines( - "WARN: ~/category/package/Makefile:21: ABCPATH is used but not defined.", "WARN: ~/category/package/Makefile:21: PKGSRC_UNKNOWN_ENV is defined but not used.", + "WARN: ~/category/package/Makefile:21: ABCPATH is used but not defined.", "0 errors and 2 warnings found.") } diff --git a/pkgtools/pkglint/files/pkgver/vercmp.go b/pkgtools/pkglint/files/pkgver/vercmp.go index 2cc7a05be0f..e48a2df2e36 100644 --- a/pkgtools/pkglint/files/pkgver/vercmp.go +++ b/pkgtools/pkglint/files/pkgver/vercmp.go @@ -14,6 +14,7 @@ func imax(a, b int) int { } return b } + func icmp(a, b int) int { if a < b { return -1 @@ -80,9 +81,7 @@ func newVersion(vstr string) *version { func (v *version) Add(i int) { v.v = append(v.v, i) } -func isdigit(b byte) bool { - return '0' <= b && b <= '9' -} + func (v *version) Place(i int) int { if i < len(v.v) { return v.v[i] diff --git a/pkgtools/pkglint/files/plist.go b/pkgtools/pkglint/files/plist.go index 1dda4c7203c..8ad153b463b 100644 --- a/pkgtools/pkglint/files/plist.go +++ b/pkgtools/pkglint/files/plist.go @@ -7,7 +7,7 @@ import ( "strings" ) -func ChecklinesPlist(lines Lines) { +func CheckLinesPlist(lines Lines) { if trace.Tracing { defer trace.Call1(lines.FileName)() } @@ -63,7 +63,7 @@ func (ck *PlistChecker) Check(plainLines Lines) { ck.checkline(pline) pline.CheckTrailingWhitespace() } - ChecklinesTrailingEmptyLines(plainLines) + CheckLinesTrailingEmptyLines(plainLines) if G.Opts.WarnPlistSort { sorter := NewPlistLineSorter(plines) @@ -425,10 +425,10 @@ func (pline *PlistLine) CheckDirective(cmd, arg string) { "command in the PLIST.") case "imake-man": - args := fields(arg) + args := strings.Fields(arg) switch { case len(args) != 3: - pline.Warnf("Invalid number of arguments for imake-man.") + pline.Warnf("Invalid number of arguments for imake-man, should be 3.") case args[2] == "${IMAKE_MANNEWSUFFIX}": pline.warnImakeMannewsuffix() } diff --git a/pkgtools/pkglint/files/plist_test.go b/pkgtools/pkglint/files/plist_test.go index 8e8c5950a8d..12bcfdc2f98 100644 --- a/pkgtools/pkglint/files/plist_test.go +++ b/pkgtools/pkglint/files/plist_test.go @@ -2,7 +2,7 @@ package pkglint import "gopkg.in/check.v1" -func (s *Suite) Test_ChecklinesPlist(c *check.C) { +func (s *Suite) Test_CheckLinesPlist(c *check.C) { t := s.Init(c) G.Pkg = NewPackage(t.File("category/pkgbase")) @@ -26,7 +26,7 @@ func (s *Suite) Test_ChecklinesPlist(c *check.C) { "share/tzinfo", "share/tzinfo") - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "ERROR: PLIST:1: Expected \"@comment $"+"NetBSD$\".", @@ -49,19 +49,19 @@ func (s *Suite) Test_ChecklinesPlist(c *check.C) { "ERROR: PLIST:18: Duplicate filename \"share/tzinfo\", already appeared in line 17.") } -func (s *Suite) Test_ChecklinesPlist__empty(c *check.C) { +func (s *Suite) Test_CheckLinesPlist__empty(c *check.C) { t := s.Init(c) lines := t.NewLines("PLIST", PlistRcsID) - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "WARN: PLIST:1: PLIST files shouldn't be empty.") } -func (s *Suite) Test_ChecklinesPlist__common_end(c *check.C) { +func (s *Suite) Test_CheckLinesPlist__common_end(c *check.C) { t := s.Init(c) t.CreateFileLines("PLIST.common", @@ -71,12 +71,12 @@ func (s *Suite) Test_ChecklinesPlist__common_end(c *check.C) { PlistRcsID, "sbin/common_end") - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputEmpty() } -func (s *Suite) Test_ChecklinesPlist__condition(c *check.C) { +func (s *Suite) Test_CheckLinesPlist__condition(c *check.C) { t := s.Init(c) G.Pkg = NewPackage(t.File("category/pkgbase")) @@ -84,13 +84,13 @@ func (s *Suite) Test_ChecklinesPlist__condition(c *check.C) { PlistRcsID, "${PLIST.bincmds}bin/subdir/command") - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "WARN: PLIST:2: The bin/ directory should not have subdirectories.") } -func (s *Suite) Test_ChecklinesPlist__sorting(c *check.C) { +func (s *Suite) Test_CheckLinesPlist__sorting(c *check.C) { t := s.Init(c) t.SetupCommandLine("-Wplist-sort") @@ -102,7 +102,7 @@ func (s *Suite) Test_ChecklinesPlist__sorting(c *check.C) { "bin/otherprogram", "${PLIST.condition}bin/cat") - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "WARN: PLIST:5: \"bin/otherprogram\" should be sorted before \"sbin/program\".", @@ -173,7 +173,7 @@ func (s *Suite) Test_PlistChecker_checkpathMan__gz(c *check.C) { PlistRcsID, "man/man3/strerror.3.gz") - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "NOTE: PLIST:2: The .gz extension is unnecessary for manual pages.") @@ -186,7 +186,7 @@ func (s *Suite) Test_PlistChecker_checkpath__PKGMANDIR(c *check.C) { PlistRcsID, "${PKGMANDIR}/man1/sh.1") - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "NOTE: PLIST:2: PLIST files should mention \"man/\" instead of \"${PKGMANDIR}\".") @@ -199,7 +199,7 @@ func (s *Suite) Test_PlistChecker_checkpath__python_egg(c *check.C) { PlistRcsID, "${PYSITELIB}/gdspy-${PKGVERSION}-py${PYVERSSUFFIX}.egg-info/PKG-INFO") - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "WARN: PLIST:2: Include \"../../lang/python/egg.mk\" instead of listing .egg-info files directly.") @@ -230,7 +230,7 @@ func (s *Suite) Test_PlistChecker__autofix(c *check.C) { "@pkgdir etc/logrotate.d", "@pkgdir etc/sasl2") - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "WARN: ~/PLIST:3: \"lib/libvirt/connection-driver/libvirt_driver_nodedev.la\" "+ @@ -240,7 +240,7 @@ func (s *Suite) Test_PlistChecker__autofix(c *check.C) { "NOTE: ~/PLIST:6: PLIST files should mention \"man/\" instead of \"${PKGMANDIR}\".") t.SetupCommandLine("-Wall", "--autofix") - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "AUTOFIX: ~/PLIST:6: Replacing \"${PKGMANDIR}/\" with \"man/\".", @@ -285,7 +285,7 @@ func (s *Suite) Test_PlistChecker__remove_same_entries(c *check.C) { "${PLIST.option2}bin/false", "bin/true") - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "ERROR: ~/PLIST:2: Duplicate filename \"bin/true\", already appeared in line 3.", @@ -296,7 +296,7 @@ func (s *Suite) Test_PlistChecker__remove_same_entries(c *check.C) { t.SetupCommandLine("-Wall", "--autofix") - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "AUTOFIX: ~/PLIST:2: Deleting this line.", @@ -324,7 +324,7 @@ func (s *Suite) Test_PlistChecker__autofix_with_only(c *check.C) { "sbin/program", "bin/program") - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputEmpty() t.CheckFileLines("PLIST", @@ -341,7 +341,7 @@ func (s *Suite) Test_PlistChecker__exec_MKDIR(c *check.C) { "bin/program", "@exec ${MKDIR} %D/share/mk/subdir") - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputEmpty() } @@ -354,14 +354,14 @@ func (s *Suite) Test_PlistChecker__empty_line(c *check.C) { "", "bin/program") - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "WARN: ~/PLIST:2: PLISTs should not contain empty lines.") t.SetupCommandLine("-Wall", "--autofix") - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "AUTOFIX: ~/PLIST:2: Deleting this line.") @@ -378,7 +378,7 @@ func (s *Suite) Test_PlistChecker__unknown_line_type(c *check.C) { "---unknown", "+++unknown") - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "WARN: ~/PLIST:2: Unknown line type: ---unknown", @@ -392,7 +392,7 @@ func (s *Suite) Test_PlistChecker__doc(c *check.C) { PlistRcsID, "doc/html/index.html") - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "ERROR: ~/PLIST:2: Documentation must be installed under share/doc, not doc.") @@ -406,7 +406,7 @@ func (s *Suite) Test_PlistChecker__PKGLOCALEDIR(c *check.C) { "${PKGLOCALEDIR}/file") G.Pkg = NewPackage(t.File("category/package")) - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "WARN: ~/PLIST:2: PLIST contains ${PKGLOCALEDIR}, but USE_PKGLOCALEDIR was not found.") @@ -422,7 +422,7 @@ func (s *Suite) Test_PlistChecker__unwanted_entries(c *check.C) { "share/pkgbase/Makefile.orig") G.Pkg = NewPackage(t.File("category/package")) - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "WARN: ~/PLIST:2: perllocal.pod files should not be in the PLIST.", @@ -438,7 +438,7 @@ func (s *Suite) Test_PlistChecker_checkpathInfo(c *check.C) { "info/gmake.1.info") G.Pkg = NewPackage(t.File("category/package")) - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "WARN: ~/PLIST:2: Packages that install info files should set INFO_FILES in the Makefile.") @@ -456,7 +456,7 @@ func (s *Suite) Test_PlistChecker_checkpathLib(c *check.C) { G.Pkg = NewPackage(t.File("category/package")) G.Pkg.EffectivePkgbase = "package" - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "ERROR: ~/PLIST:2: Only the libiconv package may install lib/charset.alias.", @@ -472,7 +472,7 @@ func (s *Suite) Test_PlistChecker_checkpathMan(c *check.C) { "man/man1/program.8", "man/manx/program.x") - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "WARN: ~/PLIST:2: Mismatch between the section (1) and extension (8) of the manual page.", @@ -492,7 +492,7 @@ func (s *Suite) Test_PlistChecker_checkpathShare(c *check.C) { G.Pkg = NewPackage(t.File("category/package")) G.Pkg.EffectivePkgbase = "package" - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "WARN: ~/PLIST:2: Use of \"share/doc/html\" is deprecated. Use \"share/doc/${PKGBASE}\" instead.", @@ -510,7 +510,7 @@ func (s *Suite) Test_PlistLine_CheckTrailingWhitespace(c *check.C) { PlistRcsID, "bin/program \t") - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "ERROR: ~/PLIST:2: pkgsrc does not support filenames ending in whitespace.") @@ -529,13 +529,13 @@ func (s *Suite) Test_PlistLine_CheckDirective(c *check.C) { "@imake-man 1 2 ${IMAKE_MANNEWSUFFIX}", "@unknown") - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( "WARN: ~/PLIST:2: Please remove this line. It is no longer necessary.", "ERROR: ~/PLIST:3: ldconfig must be used with \"||/usr/bin/true\".", "WARN: ~/PLIST:5: @dirrm is obsolete. Please remove this line.", - "WARN: ~/PLIST:6: Invalid number of arguments for imake-man.", + "WARN: ~/PLIST:6: Invalid number of arguments for imake-man, should be 3.", "WARN: ~/PLIST:7: IMAKE_MANNEWSUFFIX is not meant to appear in PLISTs.", "WARN: ~/PLIST:8: Unknown PLIST directive \"@unknown\".") } @@ -551,14 +551,14 @@ func (s *Suite) Test_plistLineSorter__unsortable(c *check.C) { "bin/program1") t.EnableTracingToLog() - ChecklinesPlist(lines) + CheckLinesPlist(lines) t.CheckOutputLines( - "TRACE: + ChecklinesPlist(\"~/PLIST\")", + "TRACE: + CheckLinesPlist(\"~/PLIST\")", "TRACE: 1 + (*LinesImpl).CheckRcsID(\"@comment \", \"@comment \")", "TRACE: 1 - (*LinesImpl).CheckRcsID(\"@comment \", \"@comment \")", "TRACE: 1 ~/PLIST:2: bin/program${OPSYS}: This line prevents pkglint from sorting the PLIST automatically.", "TRACE: 1 + SaveAutofixChanges()", "TRACE: 1 - SaveAutofixChanges()", - "TRACE: - ChecklinesPlist(\"~/PLIST\")") + "TRACE: - CheckLinesPlist(\"~/PLIST\")") } diff --git a/pkgtools/pkglint/files/shell.go b/pkgtools/pkglint/files/shell.go index a83295704ff..93b20771071 100644 --- a/pkgtools/pkglint/files/shell.go +++ b/pkgtools/pkglint/files/shell.go @@ -10,6 +10,11 @@ import ( // TODO: Can ShellLine and ShellProgramChecker be merged into one type? +// ShellLine is either a line from a Makefile starting with a tab, +// thereby containing shell commands to be executed. +// +// Or it is a variable assignment line from a Makefile with a left-hand +// side variable that is of some shell-like type; see Vartype.IsShell. type ShellLine struct { mkline MkLine } @@ -293,7 +298,9 @@ func (shline *ShellLine) CheckShellCommandLine(shelltext string) { } setE := lexer.SkipString("${RUN}") if !setE { - lexer.NextString("${_PKG_SILENT}${_PKG_DEBUG}") + if lexer.NextString("${_PKG_SILENT}${_PKG_DEBUG}") != "" { + line.Warnf("Use of _PKG_SILENT and _PKG_DEBUG is deprecated. Use ${RUN} instead.") + } } shline.CheckShellCommand(lexer.Rest(), &setE, RunTime) diff --git a/pkgtools/pkglint/files/shell_test.go b/pkgtools/pkglint/files/shell_test.go index 9cf8173cd74..5e1e625d237 100644 --- a/pkgtools/pkglint/files/shell_test.go +++ b/pkgtools/pkglint/files/shell_test.go @@ -1007,6 +1007,7 @@ func (s *Suite) Test_ShellLine__variable_outside_quotes(c *check.C) { func (s *Suite) Test_ShellLine_CheckShellCommand__cd_inside_if(c *check.C) { t := s.Init(c) + t.SetupVartypes() t.SetupTool("echo", "ECHO", AtRunTime) mklines := t.NewMkLines("Makefile", MkRcsID, @@ -1023,6 +1024,7 @@ func (s *Suite) Test_ShellLine_CheckShellCommand__cd_inside_if(c *check.C) { func (s *Suite) Test_ShellLine_CheckShellCommand__negated_pipe(c *check.C) { t := s.Init(c) + t.SetupVartypes() t.SetupTool("echo", "ECHO", AtRunTime) t.SetupTool("test", "TEST", AtRunTime) mklines := t.NewMkLines("Makefile", @@ -1041,6 +1043,7 @@ func (s *Suite) Test_ShellLine_CheckShellCommand__subshell(c *check.C) { t := s.Init(c) t.SetupTool("echo", "ECHO", AtRunTime) + t.SetupTool("expr", "EXPR", AtRunTime) mklines := t.NewMkLines("Makefile", MkRcsID, "", @@ -1067,6 +1070,7 @@ func (s *Suite) Test_ShellLine_CheckShellCommand__subshell(c *check.C) { func (s *Suite) Test_ShellLine_CheckShellCommand__case_patterns_from_variable(c *check.C) { t := s.Init(c) + t.SetupVartypes() mklines := t.NewMkLines("Makefile", MkRcsID, "", @@ -1157,7 +1161,7 @@ func (s *Suite) Test_SimpleCommandChecker_handleCommandVariable__from_package(c MkRcsID, "PYTHON_BIN=\tmy_cmd") - G.CheckDirent(pkg) + G.Check(pkg) t.CheckOutputEmpty() } diff --git a/pkgtools/pkglint/files/substcontext.go b/pkgtools/pkglint/files/substcontext.go index 007d9b8e679..b69c7f673a7 100644 --- a/pkgtools/pkglint/files/substcontext.go +++ b/pkgtools/pkglint/files/substcontext.go @@ -55,7 +55,7 @@ func (ctx *SubstContext) Varassign(mkline MkLine) { op := mkline.Op() value := mkline.Value() if varcanon == "SUBST_CLASSES" || varcanon == "SUBST_CLASSES.*" { - classes := fields(value) + classes := mkline.ValueFields(value) if len(classes) > 1 { mkline.Warnf("Please add only one class at a time to SUBST_CLASSES.") } diff --git a/pkgtools/pkglint/files/substcontext_test.go b/pkgtools/pkglint/files/substcontext_test.go index a93379164c5..a3ae47c9b0e 100644 --- a/pkgtools/pkglint/files/substcontext_test.go +++ b/pkgtools/pkglint/files/substcontext_test.go @@ -255,7 +255,7 @@ func (s *Suite) Test_SubstContext__pre_configure_with_NO_CONFIGURE(c *check.C) { "", "NO_CONFIGURE= yes") - G.CheckDirent(pkg) + G.Check(pkg) t.CheckOutputLines( "WARN: ~/category/package/Makefile:21: SUBST_STAGE pre-configure has no effect when NO_CONFIGURE is set (in line 25).") diff --git a/pkgtools/pkglint/files/tools.go b/pkgtools/pkglint/files/tools.go index dea1c798adf..716a0e7b47f 100644 --- a/pkgtools/pkglint/files/tools.go +++ b/pkgtools/pkglint/files/tools.go @@ -219,7 +219,7 @@ func (tr *Tools) ParseToolLine(mkline MkLine, fromInfrastructure bool, addToUseT case "_TOOLS.*": if !containsVarRef(varparam) { tr.Define(varparam, "", mkline) - for _, tool := range fields(value) { + for _, tool := range mkline.ValueFields(value) { tr.Define(tool, "", mkline) } } @@ -248,7 +248,7 @@ func (tr *Tools) parseUseTools(mkline MkLine, createIfAbsent bool, addToUseTools return } - deps := fields(value) + deps := mkline.ValueFields(value) // See mk/tools/autoconf.mk:/^\.if !defined/ if matches(value, `\bautoconf213\b`) { diff --git a/pkgtools/pkglint/files/tools_test.go b/pkgtools/pkglint/files/tools_test.go index 74e86d068dd..101b9b05a46 100644 --- a/pkgtools/pkglint/files/tools_test.go +++ b/pkgtools/pkglint/files/tools_test.go @@ -500,7 +500,7 @@ func (s *Suite) Test_Tools__cmake(c *check.C) { ".endif") G.Pkgsrc.LoadInfrastructure() - G.CheckDirent(t.File("category/package")) + G.Check(t.File("category/package")) t.CheckOutputEmpty() } diff --git a/pkgtools/pkglint/files/trace/tracing.go b/pkgtools/pkglint/files/trace/tracing.go index 0dc3001894c..3674399c29a 100644 --- a/pkgtools/pkglint/files/trace/tracing.go +++ b/pkgtools/pkglint/files/trace/tracing.go @@ -84,11 +84,11 @@ func argsStr(args []interface{}) string { } func (t *Tracer) traceIndent() string { - indent := "" + var indent strings.Builder for i := 0; i < t.depth; i++ { - indent += fmt.Sprintf("%d ", i+1) + _, _ = fmt.Fprintf(&indent, "%d ", i+1) } - return indent + return indent.String() } func (t *Tracer) traceCall(args ...interface{}) func() { @@ -103,12 +103,12 @@ func (t *Tracer) traceCall(args ...interface{}) func() { } } indent := t.traceIndent() - _, _ = io.WriteString(t.Out, fmt.Sprintf("TRACE: %s+ %s(%s)\n", indent, funcname, argsStr(withoutResults(args)))) + _, _ = fmt.Fprintf(t.Out, "TRACE: %s+ %s(%s)\n", indent, funcname, argsStr(withoutResults(args))) t.depth++ return func() { t.depth-- - _, _ = io.WriteString(t.Out, fmt.Sprintf("TRACE: %s- %s(%s)\n", indent, funcname, argsStr(withResults(args)))) + _, _ = fmt.Fprintf(t.Out, "TRACE: %s- %s(%s)\n", indent, funcname, argsStr(withResults(args))) } } diff --git a/pkgtools/pkglint/files/util.go b/pkgtools/pkglint/files/util.go index aed027c5d63..276df4d171b 100644 --- a/pkgtools/pkglint/files/util.go +++ b/pkgtools/pkglint/files/util.go @@ -41,9 +41,6 @@ func hasSuffix(s, suffix string) bool { func sprintf(format string, args ...interface{}) string { return fmt.Sprintf(format, args...) } -func fields(s string) []string { - return strings.Fields(s) -} func matches(s string, re regex.Pattern) bool { return G.res.Matches(s, re) } @@ -454,14 +451,14 @@ func (s *Scope) Define(varname string, mkline MkLine) { if s.defined[varname] == nil { s.defined[varname] = mkline if trace.Tracing { - trace.Step2("Defining %q in line %s", varname, mkline.Linenos()) + trace.Step2("Defining %q in %s", varname, mkline.String()) } } varcanon := varnameCanon(varname) if varcanon != varname && s.defined[varcanon] == nil { s.defined[varcanon] = mkline if trace.Tracing { - trace.Step2("Defining %q in line %s", varcanon, mkline.Linenos()) + trace.Step2("Defining %q in %s", varcanon, mkline.String()) } } } @@ -475,14 +472,14 @@ func (s *Scope) Use(varname string, line MkLine) { if s.used[varname] == nil { s.used[varname] = line if trace.Tracing { - trace.Step2("Using %q in line %s", varname, line.Linenos()) + trace.Step2("Using %q in %s", varname, line.String()) } } varcanon := varnameCanon(varname) if varcanon != varname && s.used[varcanon] == nil { s.used[varcanon] = line if trace.Tracing { - trace.Step2("Using %q in line %s", varcanon, line.Linenos()) + trace.Step2("Using %q in %s", varcanon, line.String()) } } } @@ -930,9 +927,9 @@ func escapePrintable(s string) string { case rune(byte(r)) == r && textproc.XPrint.Contains(byte(rest[j])): escaped.WriteByte(byte(r)) case r == 0xFFFD && !hasPrefix(rest[j:], "\uFFFD"): - _, _ = fmt.Fprintf(&escaped, "\\x%02X", rest[j]) + _, _ = fmt.Fprintf(&escaped, "<\\x%02X>", rest[j]) default: - _, _ = fmt.Fprintf(&escaped, "%U", r) + _, _ = fmt.Fprintf(&escaped, "<%U>", r) } } return escaped.String() diff --git a/pkgtools/pkglint/files/util_test.go b/pkgtools/pkglint/files/util_test.go index fc2cc3d0027..e4d18633fc3 100644 --- a/pkgtools/pkglint/files/util_test.go +++ b/pkgtools/pkglint/files/util_test.go @@ -480,6 +480,7 @@ func (s *Suite) Test_wrap(c *check.C) { func (s *Suite) Test_escapePrintable(c *check.C) { c.Check(escapePrintable(""), equals, "") c.Check(escapePrintable("ASCII only~\n\t"), equals, "ASCII only~\n\t") - c.Check(escapePrintable("Bad \xFF character"), equals, "Bad \\xFF character") - c.Check(escapePrintable("Unicode \uFFFD replacement"), equals, "Unicode U+FFFD replacement") + c.Check(escapePrintable("Beep \u0007 control \u001F"), equals, "Beep <U+0007> control <U+001F>") + c.Check(escapePrintable("Bad \xFF character"), equals, "Bad <\\xFF> character") + c.Check(escapePrintable("Unicode \uFFFD replacement"), equals, "Unicode <U+FFFD> replacement") } diff --git a/pkgtools/pkglint/files/vardefs.go b/pkgtools/pkglint/files/vardefs.go index 1bf114f6d06..3784ed9f797 100644 --- a/pkgtools/pkglint/files/vardefs.go +++ b/pkgtools/pkglint/files/vardefs.go @@ -8,9 +8,9 @@ import ( // This file defines the specific type of some variables. // -// There are two types of lists: -// * lkShell is a list whose elements are split by shell rules -// * lkSpace is a list whose elements are split by whitespace +// Some are plain values, some are lists. +// Lists are split like in the shell, using "double" and 'single' quotes +// to enclose spaces. // // See vartypecheck.go for how these types are checked. @@ -90,7 +90,7 @@ func (src *Pkgsrc) InitVartypes() { if mklines != nil { for _, mkline := range mklines.mklines { if mkline.IsDirective() && mkline.Directive() == "for" { - words := fields(mkline.Args()) + words := mkline.ValueFields(mkline.Args()) if len(words) > 2 && words[0] == "_version_" { for _, word := range words[2:] { languages[word] = true @@ -507,20 +507,20 @@ func (src *Pkgsrc) InitVartypes() { acl("BDB_DEFAULT", lkNone, enum("db1 db2 db3 db4 db5 db6"), "") sys("BDB_LIBS", lkShell, BtLdFlag) sys("BDB_TYPE", lkNone, enum("db1 db2 db3 db4 db5 db6")) - sys("BIGENDIANPLATFORMS", lkSpace, BtMachinePlatformPattern) + sys("BIGENDIANPLATFORMS", lkShell, BtMachinePlatformPattern) sys("BINGRP", lkNone, BtUserGroupName) sys("BINMODE", lkNone, BtFileMode) sys("BINOWN", lkNone, BtUserGroupName) - acl("BOOTSTRAP_DEPENDS", lkSpace, BtDependencyWithPath, "Makefile, Makefile.common, *.mk: append") + acl("BOOTSTRAP_DEPENDS", lkShell, BtDependencyWithPath, "Makefile, Makefile.common, *.mk: append") pkg("BOOTSTRAP_PKG", lkNone, BtYesNo) acl("BROKEN", lkNone, BtMessage, "") pkg("BROKEN_GETTEXT_DETECTION", lkNone, BtYesNo) - pkglist("BROKEN_EXCEPT_ON_PLATFORM", lkSpace, BtMachinePlatformPattern) - pkglist("BROKEN_ON_PLATFORM", lkSpace, BtMachinePlatformPattern) + pkglist("BROKEN_EXCEPT_ON_PLATFORM", lkShell, BtMachinePlatformPattern) + pkglist("BROKEN_ON_PLATFORM", lkShell, BtMachinePlatformPattern) sys("BSD_MAKE_ENV", lkShell, BtShellWord) // TODO: Align the permissions of the various BUILDLINK_*.* variables with each other. - acl("BUILDLINK_ABI_DEPENDS.*", lkSpace, BtDependency, "buildlink3.mk, builtin.mk: append, use-loadtime; *: append") - acl("BUILDLINK_API_DEPENDS.*", lkSpace, BtDependency, "buildlink3.mk, builtin.mk: append, use-loadtime; *: append") + acl("BUILDLINK_ABI_DEPENDS.*", lkShell, BtDependency, "buildlink3.mk, builtin.mk: append, use-loadtime; *: append") + acl("BUILDLINK_API_DEPENDS.*", lkShell, BtDependency, "buildlink3.mk, builtin.mk: append, use-loadtime; *: append") acl("BUILDLINK_AUTO_DIRS.*", lkNone, BtYesNo, "buildlink3.mk: append") acl("BUILDLINK_CONTENTS_FILTER", lkNone, BtShellCommand, "") sys("BUILDLINK_CFLAGS", lkShell, BtCFlag) @@ -528,7 +528,7 @@ func (src *Pkgsrc) InitVartypes() { sys("BUILDLINK_CPPFLAGS", lkShell, BtCFlag) bl3list("BUILDLINK_CPPFLAGS.*", lkShell, BtCFlag) acl("BUILDLINK_CONTENTS_FILTER.*", lkNone, BtShellCommand, "buildlink3.mk: set") - acl("BUILDLINK_DEPENDS", lkSpace, BtIdentifier, "buildlink3.mk: append") + acl("BUILDLINK_DEPENDS", lkShell, BtIdentifier, "buildlink3.mk: append") acl("BUILDLINK_DEPMETHOD.*", lkShell, BtBuildlinkDepmethod, "buildlink3.mk: default, append, use; Makefile: set, append; Makefile.common, *.mk: append") acl("BUILDLINK_DIR", lkNone, BtPathname, "*: use") bl3list("BUILDLINK_FILES.*", lkShell, BtPathmask) @@ -553,7 +553,7 @@ func (src *Pkgsrc) InitVartypes() { acl("BUILDLINK_X11_DIR", lkNone, BtPathname, "*: use") acl("BUILD_DEFS", lkShell, BtVariableName, "Makefile, Makefile.common, options.mk: append") pkglist("BUILD_DEFS_EFFECTS", lkShell, BtVariableName) - acl("BUILD_DEPENDS", lkSpace, BtDependencyWithPath, "Makefile, Makefile.common, *.mk: append") + acl("BUILD_DEPENDS", lkShell, BtDependencyWithPath, "Makefile, Makefile.common, *.mk: append") pkglist("BUILD_DIRS", lkShell, BtWrksrcSubdirectory) pkglist("BUILD_ENV", lkShell, BtShellWord) sys("BUILD_MAKE_CMD", lkNone, BtShellCommand) @@ -622,7 +622,7 @@ func (src *Pkgsrc) InitVartypes() { acl("CONFIG_STATUS_OVERRIDE", lkShell, BtPathmask, "Makefile, Makefile.common: set, append") acl("CONFIG_SHELL", lkNone, BtPathname, "Makefile, Makefile.common: set") acl("CONFIG_SUB_OVERRIDE", lkShell, BtPathmask, "Makefile, Makefile.common: set, append") - pkglist("CONFLICTS", lkSpace, BtDependency) + pkglist("CONFLICTS", lkShell, BtDependency) pkglist("CONF_FILES", lkNone, BtConfFiles) pkg("CONF_FILES_MODE", lkNone, enum("0644 0640 0600 0400")) pkglist("CONF_FILES_PERMS", lkShell, BtPerms) @@ -640,7 +640,7 @@ func (src *Pkgsrc) InitVartypes() { acl("DEINSTALL_TEMPLATES", lkShell, BtPathname, "Makefile: set, append; Makefile.common: set, default, append") sys("DELAYED_ERROR_MSG", lkNone, BtShellCommand) sys("DELAYED_WARNING_MSG", lkNone, BtShellCommand) - pkglist("DEPENDS", lkSpace, BtDependencyWithPath) + pkglist("DEPENDS", lkShell, BtDependencyWithPath) usr("DEPENDS_TARGET", lkShell, BtIdentifier) acl("DESCR_SRC", lkShell, BtPathname, "Makefile: set, append; Makefile.common: default, set") sys("DESTDIR", lkNone, BtPathname) @@ -699,9 +699,9 @@ func (src *Pkgsrc) InitVartypes() { sys("EMUL_OPSYS", lkNone, enum("darwin freebsd hpux irix linux osf1 solaris sunos none")) pkg("EMUL_PKG_FMT", lkNone, enum("plain rpm")) usr("EMUL_PLATFORM", lkNone, BtEmulPlatform) - pkg("EMUL_PLATFORMS", lkSpace, BtEmulPlatform) - usr("EMUL_PREFER", lkSpace, BtEmulPlatform) - pkg("EMUL_REQD", lkSpace, BtDependency) + pkg("EMUL_PLATFORMS", lkShell, BtEmulPlatform) + usr("EMUL_PREFER", lkShell, BtEmulPlatform) + pkg("EMUL_REQD", lkShell, BtDependency) usr("EMUL_TYPE.*", lkNone, enum("native builtin suse suse-9.1 suse-9.x suse-10.0 suse-10.x")) sys("ERROR_CAT", lkNone, BtShellCommand) sys("ERROR_MSG", lkNone, BtShellCommand) @@ -763,8 +763,8 @@ func (src *Pkgsrc) InitVartypes() { pkg("ICON_THEMES", lkNone, BtYes) acl("IGNORE_PKG.*", lkNone, BtYes, "*: set, use-loadtime") sys("IMAKE", lkNone, BtShellCommand) - acl("INCOMPAT_CURSES", lkSpace, BtMachinePlatformPattern, "Makefile: set, append") - acl("INCOMPAT_ICONV", lkSpace, BtMachinePlatformPattern, "") + acl("INCOMPAT_CURSES", lkShell, BtMachinePlatformPattern, "Makefile: set, append") + acl("INCOMPAT_ICONV", lkShell, BtMachinePlatformPattern, "") acl("INFO_DIR", lkNone, BtPathname, "") // relative to PREFIX pkg("INFO_FILES", lkNone, BtYes) sys("INSTALL", lkNone, BtShellCommand) @@ -798,7 +798,7 @@ func (src *Pkgsrc) InitVartypes() { pkg("JAVA_HOME", lkNone, BtPathname) pkg("JAVA_NAME", lkNone, BtFileName) pkglist("JAVA_UNLIMIT", lkShell, enum("cmdsize datasize stacksize")) - pkglist("JAVA_WRAPPERS", lkSpace, BtFileName) + pkglist("JAVA_WRAPPERS", lkShell, BtFileName) pkg("JAVA_WRAPPER_BIN.*", lkNone, BtPathname) sys("KRB5BASE", lkNone, BtPathname) acl("KRB5_ACCEPTED", lkShell, enum("heimdal mit-krb5"), "") @@ -822,10 +822,10 @@ func (src *Pkgsrc) InitVartypes() { pkg("LICENSE_FILE", lkNone, BtPathname) sys("LINK.*", lkNone, BtShellCommand) sys("LINKER_RPATH_FLAG", lkNone, BtShellWord) - sys("LITTLEENDIANPLATFORMS", lkSpace, BtMachinePlatformPattern) + sys("LITTLEENDIANPLATFORMS", lkShell, BtMachinePlatformPattern) sys("LOWER_OPSYS", lkNone, BtIdentifier) sys("LOWER_VENDOR", lkNone, BtIdentifier) - sys("LP64PLATFORMS", lkSpace, BtMachinePlatformPattern) + sys("LP64PLATFORMS", lkShell, BtMachinePlatformPattern) acl("LTCONFIG_OVERRIDE", lkShell, BtPathmask, "Makefile: set, append; Makefile.common: append") sysload("MACHINE_ARCH", lkNone, enumMachineArch) sysload("MACHINE_GNU_ARCH", lkNone, enumMachineGnuArch) @@ -897,8 +897,8 @@ func (src *Pkgsrc) InitVartypes() { sys("NM", lkNone, BtShellCommand) sys("NONBINMODE", lkNone, BtFileMode) pkg("NOT_FOR_COMPILER", lkShell, compilers) - pkglist("NOT_FOR_BULK_PLATFORM", lkSpace, BtMachinePlatformPattern) - pkglist("NOT_FOR_PLATFORM", lkSpace, BtMachinePlatformPattern) + pkglist("NOT_FOR_BULK_PLATFORM", lkShell, BtMachinePlatformPattern) + pkglist("NOT_FOR_PLATFORM", lkShell, BtMachinePlatformPattern) pkg("NOT_FOR_UNPRIVILEGED", lkNone, BtYesNo) pkglist("NOT_PAX_ASLR_SAFE", lkShell, BtPathmask) pkglist("NOT_PAX_MPROTECT_SAFE", lkShell, BtPathmask) @@ -915,7 +915,7 @@ func (src *Pkgsrc) InitVartypes() { acl("NO_SRC_ON_FTP", lkNone, BtRestricted, "Makefile, Makefile.common: set") sysload("OBJECT_FMT", lkNone, enum("COFF ECOFF ELF SOM XCOFF Mach-O PE a.out")) pkglist("ONLY_FOR_COMPILER", lkShell, compilers) - pkglist("ONLY_FOR_PLATFORM", lkSpace, BtMachinePlatformPattern) + pkglist("ONLY_FOR_PLATFORM", lkShell, BtMachinePlatformPattern) pkg("ONLY_FOR_UNPRIVILEGED", lkNone, BtYesNo) sysload("OPSYS", lkNone, BtIdentifier) acl("OPSYSVARS", lkShell, BtVariableName, "Makefile, Makefile.common: append") @@ -1137,18 +1137,18 @@ func (src *Pkgsrc) InitVartypes() { acl("SUBST_SED.*", lkNone, BtSedCommands, "Makefile, Makefile.*, *.mk: set, append") pkg("SUBST_STAGE.*", lkNone, BtStage) pkglist("SUBST_VARS.*", lkShell, BtVariableName) - pkglist("SUPERSEDES", lkSpace, BtDependency) - acl("TEST_DEPENDS", lkSpace, BtDependencyWithPath, "Makefile, Makefile.common, *.mk: append") + pkglist("SUPERSEDES", lkShell, BtDependency) + acl("TEST_DEPENDS", lkShell, BtDependencyWithPath, "Makefile, Makefile.common, *.mk: append") pkglist("TEST_DIRS", lkShell, BtWrksrcSubdirectory) pkglist("TEST_ENV", lkShell, BtShellWord) acl("TEST_TARGET", lkShell, BtIdentifier, "Makefile: set; Makefile.common: default, set; options.mk: set, append") pkglist("TEXINFO_REQD", lkShell, BtVersion) - acl("TOOL_DEPENDS", lkSpace, BtDependencyWithPath, "Makefile, Makefile.common, *.mk: append") + acl("TOOL_DEPENDS", lkShell, BtDependencyWithPath, "Makefile, Makefile.common, *.mk: append") sys("TOOLS_ALIASES", lkShell, BtFileName) sys("TOOLS_BROKEN", lkShell, BtTool) sys("TOOLS_CMD.*", lkNone, BtPathname) acl("TOOLS_CREATE", lkShell, BtTool, "Makefile, Makefile.common, options.mk: append") - acl("TOOLS_DEPENDS.*", lkSpace, BtDependencyWithPath, "buildlink3.mk:; Makefile, Makefile.*: set, default; *: use") + acl("TOOLS_DEPENDS.*", lkShell, BtDependencyWithPath, "buildlink3.mk:; Makefile, Makefile.*: set, default; *: use") sys("TOOLS_GNU_MISSING", lkShell, BtTool) sys("TOOLS_NOOP", lkShell, BtTool) sys("TOOLS_PATH.*", lkNone, BtPathname) @@ -1202,17 +1202,17 @@ func (src *Pkgsrc) InitVartypes() { pkglist("_WRAP_EXTRA_ARGS.*", lkShell, BtShellWord) // Only for infrastructure files; see mk/misc/show.mk - acl("_VARGROUPS", lkSpace, BtIdentifier, "*: append") - acl("_USER_VARS.*", lkSpace, BtIdentifier, "*: append") - acl("_PKG_VARS.*", lkSpace, BtIdentifier, "*: append") - acl("_SYS_VARS.*", lkSpace, BtIdentifier, "*: append") - acl("_DEF_VARS.*", lkSpace, BtIdentifier, "*: append") - acl("_USE_VARS.*", lkSpace, BtIdentifier, "*: append") + acl("_VARGROUPS", lkShell, BtIdentifier, "*: append") + acl("_USER_VARS.*", lkShell, BtIdentifier, "*: append") + acl("_PKG_VARS.*", lkShell, BtIdentifier, "*: append") + acl("_SYS_VARS.*", lkShell, BtIdentifier, "*: append") + acl("_DEF_VARS.*", lkShell, BtIdentifier, "*: append") + acl("_USE_VARS.*", lkShell, BtIdentifier, "*: append") } func enum(values string) *BasicType { valueMap := make(map[string]bool) - for _, value := range fields(values) { + for _, value := range strings.Fields(values) { valueMap[value] = true } name := "enum: " + values + " " // See IsEnum diff --git a/pkgtools/pkglint/files/vardefs_test.go b/pkgtools/pkglint/files/vardefs_test.go index 647a5fb2322..534622791b6 100644 --- a/pkgtools/pkglint/files/vardefs_test.go +++ b/pkgtools/pkglint/files/vardefs_test.go @@ -46,11 +46,11 @@ func (s *Suite) Test_Pkgsrc_InitVartypes__enumFrom(c *check.C) { c.Check(vartype, equals, values) } - checkEnumValues("EMACS_VERSIONS_ACCEPTED", "ShellList of enum: emacs29 emacs31 ") + checkEnumValues("EMACS_VERSIONS_ACCEPTED", "List of enum: emacs29 emacs31 ") checkEnumValues("PKG_JVM", "enum: jdk16 openjdk7 openjdk8 oracle-jdk8 sun-jdk6 sun-jdk7 ") - checkEnumValues("USE_LANGUAGES", "ShellList of enum: ada c c++ c++03 c++0x c++11 c++14 c99 "+ + checkEnumValues("USE_LANGUAGES", "List of enum: ada c c++ c++03 c++0x c++11 c++14 c99 "+ "fortran fortran77 gnu++03 gnu++0x gnu++11 gnu++14 java obj-c++ objc ") - checkEnumValues("PKGSRC_COMPILER", "ShellList of enum: ccache distcc f2c g95 gcc ido mipspro-ucode sunpro ") + checkEnumValues("PKGSRC_COMPILER", "List of enum: ccache distcc f2c g95 gcc ido mipspro-ucode sunpro ") } func (s *Suite) Test_Pkgsrc_InitVartypes__enumFromDirs(c *check.C) { @@ -97,7 +97,7 @@ func (s *Suite) Test_Pkgsrc_InitVartypes__LP64PLATFORMS(c *check.C) { pkg := t.SetupPackage("category/package", "BROKEN_ON_PLATFORM=\t${LP64PLATFORMS}") - G.CheckDirent(pkg) + G.Check(pkg) // No warning about a missing :Q operator. // All PLATFORM variables must be either lkNone or lkSpace. diff --git a/pkgtools/pkglint/files/vartype.go b/pkgtools/pkglint/files/vartype.go index 1ef9f9fc6f6..53e3b2b4770 100644 --- a/pkgtools/pkglint/files/vartype.go +++ b/pkgtools/pkglint/files/vartype.go @@ -16,10 +16,9 @@ type Vartype struct { type KindOfList uint8 -// TODO: Remove lkSpace. Since 2015 the .for variables are split on shell words, like everywhere else. +// TODO: Rename lkNone to Plain, and lkShell to List. const ( lkNone KindOfList = iota // Plain data type - lkSpace // List entries are separated by whitespace; used in .for loops. lkShell // List entries are shell words; used in the :M, :S modifiers. ) @@ -105,11 +104,8 @@ func (vt *Vartype) AllowedFiles(perms ACLPermissions) string { // This distinction between "real lists" and "considered a list" makes // the implementation of checklineMkVartype easier. func (vt *Vartype) IsConsideredList() bool { - switch vt.kindOfList { - case lkShell: + if vt.kindOfList == lkShell { return true - case lkSpace: - return false } switch vt.basicType { case BtAwkCommand, BtSedCommands, BtShellCommand, BtShellCommands, BtLicense, BtConfFiles: @@ -123,7 +119,7 @@ func (vt *Vartype) MayBeAppendedTo() bool { } func (vt *Vartype) String() string { - listPrefix := [...]string{"", "SpaceList of ", "ShellList of "}[vt.kindOfList] + listPrefix := [...]string{"", "List of "}[vt.kindOfList] guessedSuffix := ifelseStr(vt.guessed, " (guessed)", "") return listPrefix + vt.basicType.name + guessedSuffix } diff --git a/pkgtools/pkglint/files/vartype_test.go b/pkgtools/pkglint/files/vartype_test.go index e1b485553ad..eac49471ca3 100644 --- a/pkgtools/pkglint/files/vartype_test.go +++ b/pkgtools/pkglint/files/vartype_test.go @@ -62,7 +62,7 @@ func (s *Suite) Test_Vartype_IsConsideredList(c *check.C) { t.SetupVartypes() c.Check(G.Pkgsrc.VariableType("COMMENT").IsConsideredList(), equals, false) - c.Check(G.Pkgsrc.VariableType("DEPENDS").IsConsideredList(), equals, false) + c.Check(G.Pkgsrc.VariableType("DEPENDS").IsConsideredList(), equals, true) c.Check(G.Pkgsrc.VariableType("PKG_FAIL_REASON").IsConsideredList(), equals, true) c.Check(G.Pkgsrc.VariableType("CONF_FILES").IsConsideredList(), equals, true) } diff --git a/pkgtools/pkglint/files/vartypecheck.go b/pkgtools/pkglint/files/vartypecheck.go index 50302598efd..3ddb0b98249 100644 --- a/pkgtools/pkglint/files/vartypecheck.go +++ b/pkgtools/pkglint/files/vartypecheck.go @@ -7,6 +7,11 @@ import ( ) type VartypeCheck struct { + // Note: if "go vet" or "go test" complains about a "variable with invalid type", update to go1.11.4. + // See https://github.com/golang/go/issues/28972. + // That doesn't help though since pkglint contains these "more convoluted alias declarations" + // mentioned in https://github.com/golang/go/commit/6971090515ba. + MkLine MkLine Line Line @@ -560,6 +565,7 @@ func (cv *VartypeCheck) Identifier() { return } if cv.Value != cv.ValueNoVar { + // TODO: Activate this warning again, or document why it is not useful. //line.logWarning("Identifiers should be given directly.") } switch { @@ -1073,7 +1079,7 @@ func (cv *VartypeCheck) UserGroupName() { } } -// VariableName checks that the value is a valid variable name. +// VariableName checks that the value is a valid variable name to be used in Makefiles. func (cv *VartypeCheck) VariableName() { if cv.Value == cv.ValueNoVar && !matches(cv.Value, `^[A-Z_][0-9A-Z_]*(?:[.].*)?$`) { cv.Warnf("%q is not a valid variable name.", cv.Value) diff --git a/pkgtools/pkglint/files/vartypecheck_test.go b/pkgtools/pkglint/files/vartypecheck_test.go index 02cb9d1984a..9e339328a5e 100644 --- a/pkgtools/pkglint/files/vartypecheck_test.go +++ b/pkgtools/pkglint/files/vartypecheck_test.go @@ -66,6 +66,8 @@ func (s *Suite) Test_VartypeCheck_Category(c *check.C) { func (s *Suite) Test_VartypeCheck_CFlag(c *check.C) { vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).CFlag) + vt.tester.SetupTool("pkg-config", "", AtRunTime) + vt.Varname("CFLAGS") vt.Op(opAssignAppend) vt.Values( @@ -491,6 +493,8 @@ func (s *Suite) Test_VartypeCheck_Integer(c *check.C) { func (s *Suite) Test_VartypeCheck_LdFlag(c *check.C) { vt := NewVartypeCheckTester(s.Init(c), (*VartypeCheck).LdFlag) + vt.tester.SetupTool("pkg-config", "", AtRunTime) + vt.Varname("LDFLAGS") vt.Op(opAssignAppend) vt.Values( |