diff options
author | rillig <rillig@pkgsrc.org> | 2018-08-09 20:08:12 +0000 |
---|---|---|
committer | rillig <rillig@pkgsrc.org> | 2018-08-09 20:08:12 +0000 |
commit | 0636d1ab4b82f4ea671db92974aed9d2a5df868c (patch) | |
tree | a5d182626823386693914f6ba2546c9940cd4c13 /pkgtools | |
parent | c03f545bfbe32dba8ea705cc0c044be3e8a33843 (diff) | |
download | pkgsrc-0636d1ab4b82f4ea671db92974aed9d2a5df868c.tar.gz |
pkgtools/pkglint: Update to 5.5.16
Changes since 5.5.15:
* Add checks for options.mk files
* Treat redundant variable definitions as notes, not as warnings
* Check doc/CHANGES-* for typos in the dates (only for 2018 and later)
* Lots of cleanup in the test code
Diffstat (limited to 'pkgtools')
-rw-r--r-- | pkgtools/pkglint/Makefile | 4 | ||||
-rw-r--r-- | pkgtools/pkglint/files/autofix_test.go | 4 | ||||
-rw-r--r-- | pkgtools/pkglint/files/check_test.go | 5 | ||||
-rw-r--r-- | pkgtools/pkglint/files/expecter.go | 38 | ||||
-rw-r--r-- | pkgtools/pkglint/files/files_test.go | 12 | ||||
-rw-r--r-- | pkgtools/pkglint/files/mkline_test.go | 3 | ||||
-rw-r--r-- | pkgtools/pkglint/files/mklinechecker_test.go | 9 | ||||
-rw-r--r-- | pkgtools/pkglint/files/mklines_test.go | 12 | ||||
-rwxr-xr-x | pkgtools/pkglint/files/mklines_varalign_test.go | 7 | ||||
-rwxr-xr-x | pkgtools/pkglint/files/options.go | 106 | ||||
-rwxr-xr-x | pkgtools/pkglint/files/options_test.go | 78 | ||||
-rw-r--r-- | pkgtools/pkglint/files/package_test.go | 7 | ||||
-rw-r--r-- | pkgtools/pkglint/files/pkglint.go | 9 | ||||
-rw-r--r-- | pkgtools/pkglint/files/pkglint_test.go | 1 | ||||
-rw-r--r-- | pkgtools/pkglint/files/pkgsrc.go | 24 | ||||
-rw-r--r-- | pkgtools/pkglint/files/pkgsrc_test.go | 34 | ||||
-rw-r--r-- | pkgtools/pkglint/files/shell_test.go | 4 | ||||
-rw-r--r-- | pkgtools/pkglint/files/util.go | 5 |
18 files changed, 319 insertions, 43 deletions
diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile index 560366cd0b3..b31d44108a5 100644 --- a/pkgtools/pkglint/Makefile +++ b/pkgtools/pkglint/Makefile @@ -1,6 +1,6 @@ -# $NetBSD: Makefile,v 1.543 2018/07/28 18:31:23 rillig Exp $ +# $NetBSD: Makefile,v 1.544 2018/08/09 20:08:12 rillig Exp $ -PKGNAME= pkglint-5.5.15 +PKGNAME= pkglint-5.5.16 DISTFILES= # none CATEGORIES= pkgtools diff --git a/pkgtools/pkglint/files/autofix_test.go b/pkgtools/pkglint/files/autofix_test.go index ba05aca8f4d..e9d00ef809f 100644 --- a/pkgtools/pkglint/files/autofix_test.go +++ b/pkgtools/pkglint/files/autofix_test.go @@ -257,12 +257,12 @@ func (s *Suite) Test_Autofix_show_source_code(c *check.C) { t := s.Init(c) t.SetupCommandLine("--show-autofix", "--source") - lines := t.SetupFileLinesContinuation("Makefile", + mklines := t.SetupFileMkLines("Makefile", MkRcsID, "before \\", "The old song \\", "after") - line := lines[1] + line := mklines.lines[1] { fix := line.Autofix() diff --git a/pkgtools/pkglint/files/check_test.go b/pkgtools/pkglint/files/check_test.go index 51a692e8c7c..90ed786126b 100644 --- a/pkgtools/pkglint/files/check_test.go +++ b/pkgtools/pkglint/files/check_test.go @@ -154,9 +154,10 @@ func (t *Tester) SetupFileLines(relativeFilename string, lines ...string) []Line // SetupFileLines creates a temporary file and writes the given lines to it. // The file is then read in, handling line continuations for Makefiles. -func (t *Tester) SetupFileLinesContinuation(relativeFilename string, lines ...string) []Line { +func (t *Tester) SetupFileMkLines(relativeFilename string, lines ...string) *MkLines { filename := t.CreateFileLines(relativeFilename, lines...) - return LoadExistingLines(filename, true) + plainLines := LoadExistingLines(filename, true) + return NewMkLines(plainLines) } func (t *Tester) CreateFileLines(relativeFilename string, lines ...string) (filename string) { diff --git a/pkgtools/pkglint/files/expecter.go b/pkgtools/pkglint/files/expecter.go index 608ea69822d..2b15defdac2 100644 --- a/pkgtools/pkglint/files/expecter.go +++ b/pkgtools/pkglint/files/expecter.go @@ -82,6 +82,16 @@ func (exp *Expecter) AdvanceIfEquals(text string) bool { return !exp.EOF() && exp.lines[exp.index].Text == text && exp.Advance() } +func (exp *Expecter) AdvanceWhile(pred func(line Line) bool) { + if trace.Tracing { + defer trace.Call(exp.CurrentLine().Text)() + } + + for !exp.EOF() && !pred(exp.CurrentLine()) { + exp.Advance() + } +} + func (exp *Expecter) ExpectEmptyLine(warnSpace bool) bool { if exp.AdvanceIfEquals("") { return true @@ -110,3 +120,31 @@ func (exp *Expecter) ExpectText(text string) bool { func (exp *Expecter) SkipToFooter() { exp.index = len(exp.lines) - 2 } + +// MkExpecter records the state when checking a list of Makefile lines from top to bottom. +type MkExpecter struct { + mklines *MkLines + Expecter +} + +func NewMkExpecter(mklines *MkLines) *MkExpecter { + return &MkExpecter{mklines, *NewExpecter(mklines.lines)} +} + +func (exp *MkExpecter) CurrentMkLine() MkLine { + return exp.mklines.mklines[exp.index] +} + +func (exp *MkExpecter) PreviousMkLine() MkLine { + return exp.mklines.mklines[exp.index-1] +} + +func (exp *MkExpecter) AdvanceWhile(pred func(mkline MkLine) bool) { + if trace.Tracing { + defer trace.Call(exp.CurrentMkLine().Text)() + } + + for !exp.EOF() && pred(exp.CurrentMkLine()) { + exp.Advance() + } +} diff --git a/pkgtools/pkglint/files/files_test.go b/pkgtools/pkglint/files/files_test.go index 255ab2ff33a..3a90f286ede 100644 --- a/pkgtools/pkglint/files/files_test.go +++ b/pkgtools/pkglint/files/files_test.go @@ -34,7 +34,7 @@ func (s *Suite) Test_convertToLogicalLines_continuation(c *check.C) { func (s *Suite) Test_convertToLogicalLines__comments(c *check.C) { t := s.Init(c) - lines := t.SetupFileLinesContinuation("comment.mk", + mklines := t.SetupFileMkLines("comment.mk", "# This is a comment", "", "#\\", @@ -60,7 +60,7 @@ func (s *Suite) Test_convertToLogicalLines__comments(c *check.C) { "This is no comment") var texts []string - for _, line := range lines { + for _, line := range mklines.lines { texts = append(texts, line.Text) } @@ -83,7 +83,7 @@ func (s *Suite) Test_convertToLogicalLines__comments(c *check.C) { "This is no comment"}) var rawTexts []string - for _, line := range lines { + for _, line := range mklines.lines { for _, rawLine := range line.raw { rawTexts = append(rawTexts, rawLine.textnl) } @@ -113,6 +113,12 @@ func (s *Suite) Test_convertToLogicalLines__comments(c *check.C) { "This is a comment\n", "#\\\\\\\\\\\\\n", "This is no comment\n"}) + + // This is just a side-effect and not relevant for this particular test. + t.CheckOutputLines( + "ERROR: ~/comment.mk:15: Unknown Makefile line format.", + "ERROR: ~/comment.mk:19: Unknown Makefile line format.", + "ERROR: ~/comment.mk:23: Unknown Makefile line format.") } func (s *Suite) Test_convertToLogicalLines_continuationInLastLine(c *check.C) { diff --git a/pkgtools/pkglint/files/mkline_test.go b/pkgtools/pkglint/files/mkline_test.go index fca2b66f201..84e623e814a 100644 --- a/pkgtools/pkglint/files/mkline_test.go +++ b/pkgtools/pkglint/files/mkline_test.go @@ -679,13 +679,12 @@ func (s *Suite) Test_MkLine_variableNeedsQuoting__only_remove_known(c *check.C) t.SetupCommandLine("-Wall", "--autofix") t.SetupVartypes() - lines := t.SetupFileLinesContinuation("Makefile", + mklines := t.SetupFileMkLines("Makefile", MkRcsID, "", "demo: .PHONY", "\t${ECHO} ${WRKSRC:Q}", "\t${ECHO} ${FOODIR:Q}") - mklines := NewMkLines(lines) mklines.Check() diff --git a/pkgtools/pkglint/files/mklinechecker_test.go b/pkgtools/pkglint/files/mklinechecker_test.go index 6db863a0eb1..051b397aeb9 100644 --- a/pkgtools/pkglint/files/mklinechecker_test.go +++ b/pkgtools/pkglint/files/mklinechecker_test.go @@ -330,14 +330,13 @@ func (s *Suite) Test_MkLineChecker_checkDirectiveIndentation_autofix(c *check.C) t.SetupCommandLine("-Wall", "--autofix") t.SetupVartypes() - lines := t.SetupFileLinesContinuation("options.mk", + mklines := t.SetupFileMkLines("options.mk", MkRcsID, ".if ${PKGNAME} == pkgname", ".if \\", " ${PLATFORM:MNetBSD-4.*}", ".endif", ".endif") - mklines := NewMkLines(lines) mklines.Check() @@ -359,12 +358,11 @@ func (s *Suite) Test_MkLineChecker_CheckVaruseShellword(c *check.C) { t.SetupCommandLine("-Wall") t.SetupVartypes() - lines := t.SetupFileLinesContinuation("options.mk", + mklines := t.SetupFileMkLines("options.mk", MkRcsID, "GOPATH=\t${WRKDIR}", "do-build:", "\tcd ${WRKSRC} && GOPATH=${GOPATH} PATH=${PATH} :") - mklines := NewMkLines(lines) mklines.Check() @@ -387,11 +385,10 @@ func (s *Suite) Test_MkLineChecker_CheckVaruse_eq_nonlist(c *check.C) { t.SetupCommandLine("-Wall") t.SetupVartypes() t.SetupMasterSite("MASTER_SITE_GITHUB", "https://github.com/") - lines := t.SetupFileLinesContinuation("options.mk", + mklines := t.SetupFileMkLines("options.mk", MkRcsID, "WRKSRC=\t\t${WRKDIR:=/subdir}", "MASTER_SITES=\t${MASTER_SITE_GITHUB:=organization/}") - mklines := NewMkLines(lines) mklines.Check() diff --git a/pkgtools/pkglint/files/mklines_test.go b/pkgtools/pkglint/files/mklines_test.go index 6d830144bc6..4a4729a8428 100644 --- a/pkgtools/pkglint/files/mklines_test.go +++ b/pkgtools/pkglint/files/mklines_test.go @@ -599,6 +599,18 @@ func (s *Suite) Test_MkLines_CheckRedundantVariables__different_value(c *check.C t.CheckOutputEmpty() } +func (s *Suite) Test_MkLines_CheckRedundantVariables__overwrite_same_value(c *check.C) { + t := s.Init(c) + mklines := t.NewMkLines("module.mk", + "VAR=\tvalue ${OTHER}", + "VAR=\tvalue ${OTHER}") + + mklines.CheckRedundantVariables() + + t.CheckOutputLines( + "NOTE: module.mk:1: Definition of VAR is redundant because of line 2.") +} + func (s *Suite) Test_MkLines_CheckRedundantVariables__procedure_call(c *check.C) { t := s.Init(c) mklines := t.NewMkLines("mk/pthread.buildlink3.mk", diff --git a/pkgtools/pkglint/files/mklines_varalign_test.go b/pkgtools/pkglint/files/mklines_varalign_test.go index 894b48be7cf..109f9720325 100755 --- a/pkgtools/pkglint/files/mklines_varalign_test.go +++ b/pkgtools/pkglint/files/mklines_varalign_test.go @@ -52,8 +52,7 @@ func (vt *VaralignTester) runDefault() { } vt.tester.SetupCommandLine(cmdline...) - lines := vt.tester.SetupFileLinesContinuation("Makefile", vt.input...) - mklines := NewMkLines(lines) + mklines := vt.tester.SetupFileMkLines("Makefile", vt.input...) varalign := VaralignBlock{} for _, mkline := range mklines.mklines { @@ -71,9 +70,7 @@ func (vt *VaralignTester) runAutofix() { } vt.tester.SetupCommandLine(cmdline...) - lines := vt.tester.SetupFileLinesContinuation("Makefile", vt.input...) - - mklines := NewMkLines(lines) + mklines := vt.tester.SetupFileMkLines("Makefile", vt.input...) var varalign VaralignBlock for _, mkline := range mklines.mklines { diff --git a/pkgtools/pkglint/files/options.go b/pkgtools/pkglint/files/options.go new file mode 100755 index 00000000000..37f8b6e9018 --- /dev/null +++ b/pkgtools/pkglint/files/options.go @@ -0,0 +1,106 @@ +package main + +import "netbsd.org/pkglint/trace" + +func ChecklinesOptionsMk(mklines *MkLines) { + if trace.Tracing { + defer trace.Call1(mklines.lines[0].Filename)() + } + + mklines.Check() + + exp := NewMkExpecter(mklines) + exp.AdvanceWhile(func(mkline MkLine) bool { return mkline.IsComment() || mkline.IsEmpty() }) + + if exp.EOF() || !(exp.CurrentMkLine().IsVarassign() && exp.CurrentMkLine().Varname() == "PKG_OPTIONS_VAR") { + exp.CurrentLine().Warnf("Expected definition of PKG_OPTIONS_VAR.") + Explain( + "The input variables in an options.mk file should always be", + "mentioned in the same order: PKG_OPTIONS_VAR, ", + "PKG_SUPPORTED_OPTIONS, PKG_SUGGESTED_OPTIONS. This way, the", + "options.mk files have the same structure and are easy to understand.") + return + } + exp.Advance() + + declaredOptions := make(map[string]MkLine) + handledOptions := make(map[string]MkLine) + var optionsInDeclarationOrder []string + + // The conditionals are typically for OPSYS and MACHINE_ARCH. +loop: + for ; !exp.EOF(); exp.Advance() { + mkline := exp.CurrentMkLine() + switch { + case mkline.IsComment(): + case mkline.IsEmpty(): + case mkline.IsVarassign(): + varname := mkline.Varname() + if varname == "PKG_SUPPORTED_OPTIONS" || hasPrefix(varname, "PKG_OPTIONS_GROUP.") { + for _, option := range splitOnSpace(mkline.Value()) { + if option == mkline.WithoutMakeVariables(option) { + declaredOptions[option] = mkline + optionsInDeclarationOrder = append(optionsInDeclarationOrder, option) + } + } + } + case mkline.IsCond(): + case mkline.IsInclude(): + includedFile := mkline.IncludeFile() + switch { + case matches(includedFile, `/[^/]+\.buildlink3\.mk$`): + case matches(includedFile, `/[^/]+\.builtin\.mk$`): + case includedFile == "../../mk/bsd.prefs.mk": + case includedFile == "../../mk/bsd.fast.prefs.mk": + + case includedFile == "../../mk/bsd.options.mk": + exp.Advance() + break loop + } + default: + exp.CurrentLine().Warnf("Expected inclusion of \"../../mk/bsd.options.mk\".") + Explain( + "After defining the input variables (PKG_OPTIONS_VAR, etc.),", + "bsd.options.mk should be included to do the actual processing.", + "No other actions should take place in this part of the file", + "in order to have the same structure in all options.mk files.") + return + } + } + + for ; !exp.EOF(); exp.Advance() { + mkline := exp.CurrentMkLine() + if mkline.IsCond() && mkline.Directive() == "if" { + cond := NewMkParser(mkline.Line, mkline.Args(), false).MkCond() + if cond != nil { + cond.Visit("empty", func(t *Tree) { + varuse := t.args[0].(MkVarUse) + if varuse.varname == "PKG_OPTIONS" && len(varuse.modifiers) == 1 && hasPrefix(varuse.modifiers[0], "M") { + option := varuse.modifiers[0][1:] + handledOptions[option] = mkline + optionsInDeclarationOrder = append(optionsInDeclarationOrder, option) + } + }) + } + } + } + + for _, option := range optionsInDeclarationOrder { + declared := declaredOptions[option] + handled := handledOptions[option] + if declared != nil && handled == nil { + declared.Warnf("Option %q should be handled below in an .if block.", option) + Explain( + "If an option is not processed in this file, it may either be a", + "typo, or the option does not have any effect.") + } + if declared == nil && handled != nil { + handled.Warnf("Option %q is handled but not declared above.", option) + Explain( + "This block of code will never be run since PKG_OPTIONS cannot", + "contain this value. This is most probably a typo.") + } + } + + SaveAutofixChanges(mklines.lines) +} diff --git a/pkgtools/pkglint/files/options_test.go b/pkgtools/pkglint/files/options_test.go new file mode 100755 index 00000000000..4306e8dde38 --- /dev/null +++ b/pkgtools/pkglint/files/options_test.go @@ -0,0 +1,78 @@ +package main + +import "gopkg.in/check.v1" + +func (s *Suite) Test_ChecklinesOptionsMk(c *check.C) { + t := s.Init(c) + + t.SetupCommandLine("-Wno-space") + t.SetupVartypes() + t.SetupOption("mc-charset", "") + t.SetupOption("ncurses", "") + t.SetupOption("slang", "") + t.SetupOption("x11", "") + + t.SetupFileMkLines("mk/bsd.options.mk", + MkRcsID) + + mklines := t.SetupFileMkLines("category/package/options.mk", + MkRcsID, + "", + "PKG_OPTIONS_VAR= PKG_OPTIONS.mc", + "PKG_OPTIONS_REQUIRED_GROUPS= screen", + "PKG_OPTIONS_GROUP.screen= ncurses slang", + "PKG_SUPPORTED_OPTIONS= mc-charset x11 lang-${l}", + "PKG_SUGGESTED_OPTIONS= mc-charset slang", + "", + ".include \"../../mk/bsd.options.mk\"", + "", + ".if !empty(PKG_OPTIONS:Mx11)", + ".endif", + "", + ".if !empty(PKG_OPTIONS:Mundeclared)", + ".endif") + + G.CurrentDir = t.TmpDir() + G.CurPkgsrcdir = "." + + ChecklinesOptionsMk(mklines) + + t.CheckOutputLines( + "WARN: ~/category/package/options.mk:14: Unknown option \"undeclared\".", + "WARN: ~/category/package/options.mk:5: Option \"ncurses\" should be handled below in an .if block.", + "WARN: ~/category/package/options.mk:5: Option \"slang\" should be handled below in an .if block.", + "WARN: ~/category/package/options.mk:6: Option \"mc-charset\" should be handled below in an .if block.", + "WARN: ~/category/package/options.mk:14: Option \"undeclared\" is handled but not declared above.") +} + +func (s *Suite) Test_ChecklinesOptionsMk__unexpected_line(c *check.C) { + t := s.Init(c) + + t.SetupCommandLine("-Wno-space") + t.SetupVartypes() + t.SetupOption("mc-charset", "") + t.SetupOption("ncurses", "") + t.SetupOption("slang", "") + t.SetupOption("x11", "") + + t.SetupFileMkLines("mk/bsd.options.mk", + MkRcsID) + + mklines := t.SetupFileMkLines("category/package/options.mk", + MkRcsID, + "", + "PKG_OPTIONS_VAR= PKG_OPTIONS.mc", + "PKG_SUPPORTED_OPTIONS= mc-charset x11 lang-${l}", + "PKG_SUGGESTED_OPTIONS= mc-charset", + "", + "pre-configure:", + "\techo \"In the pre-configure stage.\"") + + G.CurrentDir = t.TmpDir() + G.CurPkgsrcdir = "." + + ChecklinesOptionsMk(mklines) + + t.CheckOutputLines( + "WARN: ~/category/package/options.mk:7: Expected inclusion of \"../../mk/bsd.options.mk\".") +} diff --git a/pkgtools/pkglint/files/package_test.go b/pkgtools/pkglint/files/package_test.go index 2726a272f7f..33ca7f13405 100644 --- a/pkgtools/pkglint/files/package_test.go +++ b/pkgtools/pkglint/files/package_test.go @@ -422,8 +422,8 @@ func (s *Suite) Test_Package_loadPackageMakefile(c *check.C) { // A file including itself does not lead to an endless loop while parsing // but may still produce unexpected warnings, such as redundant definitions. t.CheckOutputLines( - "WARN: ~/category/package/Makefile:3: Variable PKGNAME is overwritten in Makefile:3.", - "WARN: ~/category/package/Makefile:4: Variable DISTNAME is overwritten in Makefile:4.") + "NOTE: ~/category/package/Makefile:3: Definition of PKGNAME is redundant because of Makefile:3.", + "NOTE: ~/category/package/Makefile:4: Definition of DISTNAME is redundant because of Makefile:4.") } func (s *Suite) Test_Package_conditionalAndUnconditionalInclude(c *check.C) { @@ -471,7 +471,8 @@ func (s *Suite) Test_Package_conditionalAndUnconditionalInclude(c *check.C) { "WARN: ~/category/package/options.mk:4: \"../../devel/zlib/buildlink3.mk\" is "+ "included conditionally here (depending on PKG_OPTIONS) and unconditionally in Makefile:5.", "WARN: ~/category/package/options.mk:6: \"../../sysutils/coreutils/buildlink3.mk\" is "+ - "included unconditionally here and conditionally in Makefile:7 (depending on OPSYS).") + "included unconditionally here and conditionally in Makefile:7 (depending on OPSYS).", + "WARN: ~/category/package/options.mk:3: Expected definition of PKG_OPTIONS_VAR.") } // See https://github.com/rillig/pkglint/issues/1 diff --git a/pkgtools/pkglint/files/pkglint.go b/pkgtools/pkglint/files/pkglint.go index 318e4130b94..9ad5fbd3d7b 100644 --- a/pkgtools/pkglint/files/pkglint.go +++ b/pkgtools/pkglint/files/pkglint.go @@ -65,6 +65,7 @@ type CmdOpts struct { CheckMakefile, CheckMessage, CheckMk, + CheckOptions, CheckPatches, CheckPlist bool @@ -220,6 +221,7 @@ func (pkglint *Pkglint) ParseCommandLine(args []string) *int { check.AddFlagVar("Makefile", &gopts.CheckMakefile, true, "check Makefiles") check.AddFlagVar("MESSAGE", &gopts.CheckMessage, true, "check MESSAGE file") check.AddFlagVar("mk", &gopts.CheckMk, true, "check other .mk files") + check.AddFlagVar("options", &gopts.CheckOptions, true, "check options.mk files") check.AddFlagVar("patches", &gopts.CheckPatches, true, "check patches") check.AddFlagVar("PLIST", &gopts.CheckPlist, true, "check PLIST files") @@ -543,6 +545,13 @@ func (pkglint *Pkglint) Checkfile(fname string) { } } + case basename == "options.mk": + if pkglint.opts.CheckOptions { + if lines := LoadNonemptyLines(fname, true); lines != nil { + ChecklinesOptionsMk(NewMkLines(lines)) + } + } + case matches(basename, `^patch-[-A-Za-z0-9_.~+]*[A-Za-z0-9_]$`): if pkglint.opts.CheckPatches { if lines := LoadNonemptyLines(fname, false); lines != nil { diff --git a/pkgtools/pkglint/files/pkglint_test.go b/pkgtools/pkglint/files/pkglint_test.go index 6d744b61b64..b9625888f71 100644 --- a/pkgtools/pkglint/files/pkglint_test.go +++ b/pkgtools/pkglint/files/pkglint_test.go @@ -92,6 +92,7 @@ func (s *Suite) Test_Pkglint_Main__unknown_option(c *check.C) { " Makefile check Makefiles (enabled)", " MESSAGE check MESSAGE file (enabled)", " mk check other .mk files (enabled)", + " options check options.mk files (enabled)", " patches check patches (enabled)", " PLIST check PLIST files (enabled)", "", diff --git a/pkgtools/pkglint/files/pkgsrc.go b/pkgtools/pkglint/files/pkgsrc.go index fdd6e7ac7f2..46a6c214b04 100644 --- a/pkgtools/pkglint/files/pkgsrc.go +++ b/pkgtools/pkglint/files/pkgsrc.go @@ -301,10 +301,34 @@ func (src *Pkgsrc) loadDocChangesFromFile(fname string) []*Change { return nil } + year := "" + if m, yyyy := match1(fname, `-(\d+)$`); m && yyyy >= "2018" { + year = yyyy + } + var changes []*Change for _, line := range lines { if change := parseChange(line); change != nil { changes = append(changes, change) + if year != "" && change.Date[0:4] != year { + line.Warnf("Year %s for %s does not match the file name %s.", change.Date[0:4], change.Pkgpath, fname) + } + if len(changes) >= 2 && year != "" { + if prev := changes[len(changes)-2]; change.Date < prev.Date { + line.Warnf("Date %s for %s is earlier than %s for %s.", change.Date, change.Pkgpath, prev.Date, prev.Pkgpath) + Explain( + "The entries in doc/CHANGES should be in chronological order, and", + "all dates are assumed to be in the UTC timezone, to prevent time", + "warps.", + "", + "To fix this, determine which of the involved dates are correct", + "and which aren't.", + "", + "To prevent this kind of mistakes in the future, make sure that", + "your system time is correct and use \""+confMake+" cce\" to commit", + "the changes entry.") + } + } } else if text := line.Text; len(text) >= 2 && text[0] == '\t' && 'A' <= text[1] && text[1] <= 'Z' { line.Warnf("Unknown doc/CHANGES line: %q", text) Explain("See mk/misc/developer.mk for the rules.") diff --git a/pkgtools/pkglint/files/pkgsrc_test.go b/pkgtools/pkglint/files/pkgsrc_test.go index 991b4e9af56..981706e5558 100644 --- a/pkgtools/pkglint/files/pkgsrc_test.go +++ b/pkgtools/pkglint/files/pkgsrc_test.go @@ -126,25 +126,29 @@ func (s *Suite) Test_Pkgsrc_loadTools(c *check.C) { func (s *Suite) Test_Pkgsrc_loadDocChangesFromFile(c *check.C) { t := s.Init(c) - t.SetupFileLines("doc/CHANGES-2015", - "\tAdded category/package version 1.0 [author1 2015-01-01]", - "\tUpdated category/package to 1.5 [author2 2015-01-02]", - "\tRenamed category/package to category/pkg [author3 2015-01-03]", - "\tMoved category/package to other/package [author4 2015-01-04]", - "\tRemoved category/package [author5 2015-01-05]", - "\tRemoved category/package successor category/package2 [author6 2015-01-06]", - "\tDowngraded category/package to 1.2 [author7 2015-01-07]") + t.SetupFileLines("doc/CHANGES-2018", + "\tAdded category/package version 1.0 [author1 2015-01-01]", // Wrong year + "\tUpdated category/package to 1.5 [author2 2018-01-02]", + "\tRenamed category/package to category/pkg [author3 2018-01-03]", + "\tMoved category/package to other/package [author4 2018-01-04]", + "\tRemoved category/package [author5 2018-01-09]", // Too far in the future + "\tRemoved category/package successor category/package2 [author6 2018-01-06]", + "\tDowngraded category/package to 1.2 [author7 2018-01-07]") - changes := G.Pkgsrc.loadDocChangesFromFile(t.TmpDir() + "/doc/CHANGES-2015") + changes := G.Pkgsrc.loadDocChangesFromFile(t.TmpDir() + "/doc/CHANGES-2018") c.Assert(len(changes), equals, 7) c.Check(*changes[0], equals, Change{changes[0].Line, "Added", "category/package", "1.0", "author1", "2015-01-01"}) - c.Check(*changes[1], equals, Change{changes[1].Line, "Updated", "category/package", "1.5", "author2", "2015-01-02"}) - c.Check(*changes[2], equals, Change{changes[2].Line, "Renamed", "category/package", "", "author3", "2015-01-03"}) - c.Check(*changes[3], equals, Change{changes[3].Line, "Moved", "category/package", "", "author4", "2015-01-04"}) - c.Check(*changes[4], equals, Change{changes[4].Line, "Removed", "category/package", "", "author5", "2015-01-05"}) - c.Check(*changes[5], equals, Change{changes[5].Line, "Removed", "category/package", "", "author6", "2015-01-06"}) - c.Check(*changes[6], equals, Change{changes[6].Line, "Downgraded", "category/package", "1.2", "author7", "2015-01-07"}) + c.Check(*changes[1], equals, Change{changes[1].Line, "Updated", "category/package", "1.5", "author2", "2018-01-02"}) + c.Check(*changes[2], equals, Change{changes[2].Line, "Renamed", "category/package", "", "author3", "2018-01-03"}) + c.Check(*changes[3], equals, Change{changes[3].Line, "Moved", "category/package", "", "author4", "2018-01-04"}) + c.Check(*changes[4], equals, Change{changes[4].Line, "Removed", "category/package", "", "author5", "2018-01-09"}) + c.Check(*changes[5], equals, Change{changes[5].Line, "Removed", "category/package", "", "author6", "2018-01-06"}) + c.Check(*changes[6], equals, Change{changes[6].Line, "Downgraded", "category/package", "1.2", "author7", "2018-01-07"}) + + t.CheckOutputLines( + "WARN: ~/doc/CHANGES-2018:1: Year 2015 for category/package does not match the file name ~/doc/CHANGES-2018.", + "WARN: ~/doc/CHANGES-2018:6: Date 2018-01-06 for category/package is earlier than 2018-01-09 for category/package.") } func (s *Suite) Test_Pkgsrc_deprecated(c *check.C) { diff --git a/pkgtools/pkglint/files/shell_test.go b/pkgtools/pkglint/files/shell_test.go index de03db762f0..d8d5bb96231 100644 --- a/pkgtools/pkglint/files/shell_test.go +++ b/pkgtools/pkglint/files/shell_test.go @@ -636,13 +636,13 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine__install_option_d(c *check. func (s *Suite) Test_ShellLine__shell_comment_with_line_continuation(c *check.C) { t := s.Init(c) - lines := t.SetupFileLinesContinuation("Makefile", + mklines := t.SetupFileMkLines("Makefile", MkRcsID, "pre-install:", "\t"+"# comment\\", "\t"+"echo \"hello\"") - NewMkLines(lines).Check() + mklines.Check() t.CheckOutputLines( "WARN: ~/Makefile:3--4: A shell comment does not stop at the end of line.") diff --git a/pkgtools/pkglint/files/util.go b/pkgtools/pkglint/files/util.go index 478943ae2aa..1ea5f6315b7 100644 --- a/pkgtools/pkglint/files/util.go +++ b/pkgtools/pkglint/files/util.go @@ -568,7 +568,7 @@ func naturalLess(str1, str2 string) bool { // like defining PKGNAME, then evaluating it using :=, then defining it again. // This pattern is so error-prone that it should not appear in pkgsrc at all, // thus pkglint doesn't even expect it. (Well, except for the PKGNAME case, -// but that's deep in the infrastructure and only affects the "nb13" extension. +// but that's deep in the infrastructure and only affects the "nb13" extension.) type RedundantScope struct { vars map[string]*redundantScopeVarinfo condLevel int @@ -610,6 +610,9 @@ func (s *RedundantScope) Handle(mkline MkLine) { s.vars[varname] = &redundantScopeVarinfo{mkline, value} } } else if existing != nil { + if op == opAssign && existing.value == value { + op = opAssignDefault + } switch op { case opAssign: if s.OnOverwrite != nil { |