From 8b1d1c6cc8cb255c66c152eb8918b95184cc1257 Mon Sep 17 00:00:00 2001 From: rillig Date: Thu, 7 Jul 2016 12:09:26 +0000 Subject: Updated pkglint to 5.4.3. Changes since 5.4.2: * Variables like ${VAR_${OTHER_VAR}} are no longer checked for use/define mismatch * The check for plural variable names has been removed * The type of variables called *DESTDIR is no longer guessed to be a directory name * The check for unknown shell commands is disabled in Makefile sections that depend on OPSYS * The experimental hand-written shell parser has been replaced with a Yacc-generated one * Meta packages don't need a LICENSE * When PKGNAME is defined in terms of ${DISTNAME:S/from/to/:tl}, more modifiers (like :tl) are handled properly * When the MAINTAINER or OWNER of a package is not the current user, a warning is printed for modified files * The check for share/applications/*.desktop has been disabled, since pkglint would need to inspect the file's actual contents to see whether desktopdb.mk must be included or not * SUBST_CLASSES may also be SUBST_CLASSES.NetBSD * Loosened the usage restrictions for several variables, e.g. many variables that may be appended in a Makefile may also be set unconditionally * PKG_OPTIONS_VAR must be of the form PKG_OPTIONS.* --- pkgtools/pkglint/Makefile | 9 +- pkgtools/pkglint/files/check_test.go | 8 +- pkgtools/pkglint/files/globaldata.go | 3 + pkgtools/pkglint/files/licenses.go | 5 + pkgtools/pkglint/files/mkline.go | 71 +-- pkgtools/pkglint/files/mkline_test.go | 52 +- pkgtools/pkglint/files/mklines_test.go | 53 ++ pkgtools/pkglint/files/mkparser.go | 7 +- pkgtools/pkglint/files/mkparser_test.go | 2 +- pkgtools/pkglint/files/mkshparser.go | 823 +++++++--------------------- pkgtools/pkglint/files/mkshparser_test.go | 755 ++++++++++++++++--------- pkgtools/pkglint/files/mkshtypes.go | 158 ++---- pkgtools/pkglint/files/mkshwalker.go | 153 ++++++ pkgtools/pkglint/files/mkshwalker_test.go | 32 ++ pkgtools/pkglint/files/mktypes_test.go | 6 + pkgtools/pkglint/files/package.go | 94 +++- pkgtools/pkglint/files/package_test.go | 16 + pkgtools/pkglint/files/pkglint.go | 5 +- pkgtools/pkglint/files/plist.go | 3 +- pkgtools/pkglint/files/plist_test.go | 3 + pkgtools/pkglint/files/shell.go | 581 +++++++++----------- pkgtools/pkglint/files/shell.y | 422 ++++++++++++++ pkgtools/pkglint/files/shell_test.go | 100 ++-- pkgtools/pkglint/files/shtokenizer.go | 62 ++- pkgtools/pkglint/files/shtokenizer_test.go | 25 +- pkgtools/pkglint/files/shtypes.go | 16 +- pkgtools/pkglint/files/substcontext.go | 4 +- pkgtools/pkglint/files/substcontext_test.go | 17 + pkgtools/pkglint/files/util.go | 38 +- pkgtools/pkglint/files/util_test.go | 26 +- pkgtools/pkglint/files/vardefs.go | 73 ++- pkgtools/pkglint/files/vartypecheck.go | 5 + pkgtools/pkglint/files/vartypecheck_test.go | 6 +- pkgtools/pkglint/files/vercmp_test.go | 51 +- 34 files changed, 2170 insertions(+), 1514 deletions(-) create mode 100644 pkgtools/pkglint/files/mkshwalker.go create mode 100644 pkgtools/pkglint/files/mkshwalker_test.go create mode 100644 pkgtools/pkglint/files/shell.y (limited to 'pkgtools/pkglint') diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile index 3f48884ccf4..acf4854aeda 100644 --- a/pkgtools/pkglint/Makefile +++ b/pkgtools/pkglint/Makefile @@ -1,6 +1,6 @@ -# $NetBSD: Makefile,v 1.487 2016/06/19 18:03:29 wiz Exp $ +# $NetBSD: Makefile,v 1.488 2016/07/07 12:09:26 rillig Exp $ -PKGNAME= pkglint-5.4.2 +PKGNAME= pkglint-5.4.3 DISTFILES= # none CATEGORIES= pkgtools @@ -25,7 +25,10 @@ SUBST_SED.pkglint+= -e s\|@BMAKE@\|${MAKE:Q}\|g do-extract: ${RUN} mkdir -p ${WRKDIR}/pkglint/plist-clash - ${RUN} cd ${FILESDIR} && ${PAX} -rw *.go */*.go pkglint.[01] ${WRKDIR}/pkglint + ${RUN} cd ${FILESDIR} && ${PAX} -rw *.go *.y */*.go pkglint.[01] ${WRKDIR}/pkglint + +pre-build: + ${RUN} env GOPATH=${WRKDIR}:${BUILDLINK_DIR}/gopkg go generate ${GO_BUILD_PATTERN} do-install: do-install-man diff --git a/pkgtools/pkglint/files/check_test.go b/pkgtools/pkglint/files/check_test.go index 38e6c0e571d..441dd00ec2a 100644 --- a/pkgtools/pkglint/files/check_test.go +++ b/pkgtools/pkglint/files/check_test.go @@ -75,12 +75,18 @@ func (s *Suite) NewMkLines(fname string, lines ...string) *MkLines { return NewMkLines(s.NewLines(fname, lines...)) } -func (s *Suite) DebugToStdout() { +func (s *Suite) BeginDebugToStdout() { G.debugOut = os.Stdout G.logOut = os.Stdout G.opts.Debug = true } +func (s *Suite) EndDebugToStdout() { + G.debugOut = &s.stdout + G.logOut = &s.stdout + G.opts.Debug = false +} + func (s *Suite) UseCommandLine(c *check.C, args ...string) { exitcode := new(Pkglint).ParseCommandLine(append([]string{"pkglint"}, args...)) if exitcode != nil && *exitcode != 0 { diff --git a/pkgtools/pkglint/files/globaldata.go b/pkgtools/pkglint/files/globaldata.go index fc78473ac1d..3e67fa9fd3a 100644 --- a/pkgtools/pkglint/files/globaldata.go +++ b/pkgtools/pkglint/files/globaldata.go @@ -516,6 +516,9 @@ func (gd *GlobalData) loadDeprecatedVars() { // January 2016 "SUBST_POSTCMD.*": "Has been removed, as it seemed unused.", + + // June 2016 + "USE_CROSSBASE": "Has been removed.", } } diff --git a/pkgtools/pkglint/files/licenses.go b/pkgtools/pkglint/files/licenses.go index 38eb999cde0..0009e6e56e4 100644 --- a/pkgtools/pkglint/files/licenses.go +++ b/pkgtools/pkglint/files/licenses.go @@ -56,6 +56,11 @@ func checklineLicense(line *MkLine, value string) { "no-redistribution", "shareware": line.Warn1("License %q is deprecated.", license) + Explain( + "Instead of using these deprecated licenses, extract the actual", + "license from the package into the pkgsrc/licenses/ directory", + "and define LICENSE to that file name. See the pkgsrc guide,", + "keyword LICENSE, for more information.") } } } diff --git a/pkgtools/pkglint/files/mkline.go b/pkgtools/pkglint/files/mkline.go index ebe45b91baf..10b886d18d5 100644 --- a/pkgtools/pkglint/files/mkline.go +++ b/pkgtools/pkglint/files/mkline.go @@ -430,7 +430,7 @@ func (mkline *MkLine) CheckVaruse(varuse *MkVarUse, vuc *VarUseContext) { (vartype == nil || vartype.guessed) && !varIsUsed(varname) && !(G.Mk != nil && G.Mk.forVars[varname]) && - !hasPrefix(varname, "${") { + !containsVarRef(varname) { mkline.Warn1("%s is used but not defined. Spelling mistake?", varname) } @@ -999,61 +999,6 @@ func (mkline *MkLine) checkVarassignPlistComment(varname, value string) { } } -const reVarnamePlural = `^(?:` + - `.*[Ss]` + - `|.*LIST` + - `|.*_AWK` + - `|.*_ENV` + - `|.*_OVERRIDE` + - `|.*_PREREQ` + - `|.*_REQD` + - `|.*_SED` + - `|.*_SKIP` + - `|.*_SRC` + - `|.*_SUBST` + - `|.*_TARGET` + - `|.*_TMPL` + - `|BROKEN_EXCEPT_ON_PLATFORM` + - `|BROKEN_ON_PLATFORM` + - `|BUILDLINK_DEPMETHOD` + - `|BUILDLINK_LDADD` + - `|BUILDLINK_TRANSFORM` + - `|COMMENT` + - `|CRYPTO` + - `|DEINSTALL_TEMPLATE` + - `|EVAL_PREFIX` + - `|EXTRACT_ONLY` + - `|FETCH_MESSAGE` + - `|FIX_RPATH` + - `|GENERATE_PLIST` + - `|INSTALL_TEMPLATE` + - `|INTERACTIVE_STAGE` + - `|LICENSE` + - `|MASTER_SITE_.*` + - `|MASTER_SORT_REGEX` + - `|NOT_FOR_COMPILER` + - `|NOT_FOR_PLATFORM` + - `|ONLY_FOR_COMPILER` + - `|ONLY_FOR_PLATFORM` + - `|PERL5_PACKLIST` + - `|PLIST_CAT` + - `|PLIST_PRE` + - `|PKG_FAIL_REASON` + - `|PKG_SKIP_REASON` + - `|PREPEND_PATH` + - `|PYTHON_VERSIONS_INCOMPATIBLE` + - `|REPLACE_INTERPRETER` + - `|REPLACE_PERL` + - `|REPLACE_RUBY` + - `|RESTRICTED` + - `|SITES_.+` + - `|TOOLS_ALIASES\..+` + - `|TOOLS_BROKEN` + - `|TOOLS_CREATE` + - `|TOOLS_GNU_MISSING` + - `|TOOLS_NOOP` + - `)$` - func (mkline *MkLine) CheckVartype(varname string, op MkOperator, value, comment string) { if G.opts.Debug { defer tracecall(varname, op, value, comment)() @@ -1063,22 +1008,16 @@ func (mkline *MkLine) CheckVartype(varname string, op MkOperator, value, comment return } - varbase := varnameBase(varname) vartype := mkline.getVariableType(varname) if op == opAssignAppend { - if vartype != nil { - if !vartype.MayBeAppendedTo() { - mkline.Warn0("The \"+=\" operator should only be used with lists.") - } - } else if !hasPrefix(varbase, "_") && !matches(varbase, reVarnamePlural) { - mkline.Warn1("As %s is modified using \"+=\", its name should indicate plural.", varname) + if vartype != nil && !vartype.MayBeAppendedTo() { + mkline.Warn0("The \"+=\" operator should only be used with lists.") } } switch { case vartype == nil: - // Cannot check anything if the type is not known. if G.opts.Debug { traceStep1("Unchecked variable assignment for %s.", varname) } @@ -1380,7 +1319,7 @@ const ( ) func (nq NeedsQuoting) String() string { - return [...]string{"no", "yes", "doesn't matter", "don't known"}[nq] + return [...]string{"no", "yes", "doesn't matter", "don't know"}[nq] } func (mkline *MkLine) variableNeedsQuoting(varname string, vartype *Vartype, vuc *VarUseContext) (needsQuoting NeedsQuoting) { @@ -1530,7 +1469,7 @@ func (mkline *MkLine) getVariableType(varname string) *Vartype { switch { case hasSuffix(varbase, "DIRS"): gtype = &Vartype{lkShell, CheckvarPathmask, allowRuntime, true} - case hasSuffix(varbase, "DIR"), hasSuffix(varname, "_HOME"): + case hasSuffix(varbase, "DIR") && !hasSuffix(varbase, "DESTDIR"), hasSuffix(varname, "_HOME"): gtype = &Vartype{lkNone, CheckvarPathname, allowRuntime, true} case hasSuffix(varbase, "FILES"): gtype = &Vartype{lkShell, CheckvarPathmask, allowRuntime, true} diff --git a/pkgtools/pkglint/files/mkline_test.go b/pkgtools/pkglint/files/mkline_test.go index 62c6fd57183..ccbee10c3bb 100644 --- a/pkgtools/pkglint/files/mkline_test.go +++ b/pkgtools/pkglint/files/mkline_test.go @@ -292,16 +292,11 @@ func (s *Suite) TestMkLine_(c *check.C) { G.Mk = s.NewMkLines("Makefile", "# $"+"NetBSD$", - "ac_cv_libpari_libs+=\t-L${BUILDLINK_PREFIX.pari}/lib", // From math/clisp-pari/Makefile, rev. 1.8 - "var+=value") + "ac_cv_libpari_libs+=\t-L${BUILDLINK_PREFIX.pari}/lib") // From math/clisp-pari/Makefile, rev. 1.8 G.Mk.mklines[1].checkVarassign() - G.Mk.mklines[2].checkVarassign() - c.Check(s.Output(), equals, ""+ - "WARN: Makefile:2: ac_cv_libpari_libs is defined but not used. Spelling mistake?\n"+ - "WARN: Makefile:3: As var is modified using \"+=\", its name should indicate plural.\n"+ - "WARN: Makefile:3: var is defined but not used. Spelling mistake?\n") + c.Check(s.Output(), equals, "WARN: Makefile:2: ac_cv_libpari_libs is defined but not used. Spelling mistake?\n") } // In variable assignments, a plain '#' introduces a line comment, unless @@ -679,6 +674,24 @@ func (s *Suite) TestMkLine_variableNeedsQuoting_18(c *check.C) { c.Check(s.Output(), equals, "") // Don’t warn about missing :Q operators. } +func (s *Suite) Test_MkLine_variableNeedsQuoting_tool_in_CONFIGURE_ENV(c *check.C) { + s.UseCommandLine(c, "-Wall") + G.globalData.InitVartypes() + G.globalData.Tools = NewToolRegistry() + G.globalData.Tools.RegisterVarname("tar", "TAR") + + mklines := s.NewMkLines("Makefile", + "# $"+"NetBSD$", + "", + "CONFIGURE_ENV+=\tSYS_TAR_COMMAND_PATH=${TOOLS_TAR:Q}") + + mklines.mklines[2].checkVarassignVaruse() + + // The TOOLS_* variables only contain the path to the tool, + // without any additional arguments that might be necessary. + c.Check(s.Output(), equals, "NOTE: Makefile:3: The :Q operator isn't necessary for ${TOOLS_TAR} here.\n") +} + func (s *Suite) Test_MkLine_Varuse_Modifier_L(c *check.C) { s.UseCommandLine(c, "-Wall") G.globalData.InitVartypes() @@ -771,3 +784,28 @@ func (s *Suite) Test_MkLine_shell_varuse_in_backt_dquot(c *check.C) { c.Check(s.Output(), equals, "WARN: x11/motif/Makefile:3: Unknown shell command \"${GREP}\".\n") // No parse errors. } + +// See PR 46570, Ctrl+F "3. In lang/perl5". +func (s *Suite) Test_MkLine_getVariableType(c *check.C) { + mkline := NewMkLine(dummyLine) + + c.Check(mkline.getVariableType("_PERL5_PACKLIST_AWK_STRIP_DESTDIR"), check.IsNil) + c.Check(mkline.getVariableType("SOME_DIR").guessed, equals, true) + c.Check(mkline.getVariableType("SOMEDIR").guessed, equals, true) +} + +// See PR 46570, Ctrl+F "4. Shell quoting". +// Pkglint is correct, since this definition for CPPFLAGS should be +// seen by the shell as three words, not one word. +func (s *Suite) Test_MkLine_Cflags(c *check.C) { + G.globalData.InitVartypes() + mklines := s.NewMkLines("Makefile", + "# $"+"NetBSD$", + "CPPFLAGS.SunOS+=\t-DPIPECOMMAND=\\\"/usr/sbin/sendmail -bs %s\\\"") + + mklines.Check() + + c.Check(s.Output(), equals, ""+ + "WARN: Makefile:2: Unknown compiler flag \"-bs\".\n"+ + "WARN: Makefile:2: Compiler flag \"%s\\\\\\\"\" should start with a hyphen.\n") +} diff --git a/pkgtools/pkglint/files/mklines_test.go b/pkgtools/pkglint/files/mklines_test.go index 8de3aafd8dd..22c8ff33613 100644 --- a/pkgtools/pkglint/files/mklines_test.go +++ b/pkgtools/pkglint/files/mklines_test.go @@ -4,6 +4,8 @@ import ( check "gopkg.in/check.v1" ) +const mkrcsid = "# $" + "NetBSD$" + func (s *Suite) TestMkLines_AutofixConditionalIndentation(c *check.C) { s.UseCommandLine(c, "--autofix", "-Wspace") tmpfile := s.CreateTmpFile(c, "fname.mk", "") @@ -258,3 +260,54 @@ func (s *Suite) Test_MkLines_Indentation_DependsOn(c *check.C) { c.Check(s.Output(), equals, "NOTE: Makefile:4: Consider defining NOT_FOR_PLATFORM instead of setting PKG_SKIP_REASON depending on ${OPSYS}.\n") } + +// PR 46570, item "15. net/uucp/Makefile has a make loop" +func (s *Suite) Test_MkLines_indirect_variables(c *check.C) { + s.UseCommandLine(c, "-Wall") + mklines := s.NewMkLines("net/uucp/Makefile", + "# $"+"NetBSD$", + "", + "post-configure:", + ".for var in MAIL_PROGRAM CMDPATH", + "\t"+`${RUN} ${ECHO} "#define ${var} \""${UUCP_${var}}"\"`, + ".endfor") + + mklines.Check() + + // No warning about UUCP_${var} being used but not defined. + c.Check(s.Output(), equals, ""+ + "WARN: net/uucp/Makefile:5: Unknown shell command \"${ECHO}\".\n") +} + +func (s *Suite) Test_MkLines_Check_list_variable_as_part_of_word(c *check.C) { + s.UseCommandLine(c, "-Wall") + mklines := s.NewMkLines("converters/chef/Makefile", + mkrcsid, + "\tcd ${WRKSRC} && tr '\\r' '\\n' < ${DISTDIR}/${DIST_SUBDIR}/${DISTFILES} > chef.l") + + mklines.Check() + + c.Check(s.Output(), equals, ""+ + "WARN: converters/chef/Makefile:2: Unknown shell command \"tr\".\n"+ + "WARN: converters/chef/Makefile:2: The list variable DISTFILES should not be embedded in a word.\n") +} + +func (s *Suite) Test_MkLines_Check_absolute_pathname_depending_on_OPSYS(c *check.C) { + s.UseCommandLine(c, "-Wall") + G.globalData.InitVartypes() + mklines := s.NewMkLines("games/heretic2-demo/Makefile", + mkrcsid, + ".if ${OPSYS} == \"DragonFly\"", + "TOOLS_PLATFORM.gtar=\t/usr/bin/bsdtar", + ".endif", + "TOOLS_PLATFORM.gtar=\t/usr/bin/bsdtar") + + mklines.Check() + + // No warning about an unknown shell command in line 3, + // since that line depends on OPSYS. + c.Check(s.Output(), equals, ""+ + "WARN: games/heretic2-demo/Makefile:3: The variable TOOLS_PLATFORM.gtar may not be set by any package.\n"+ + "WARN: games/heretic2-demo/Makefile:5: The variable TOOLS_PLATFORM.gtar may not be set by any package.\n"+ + "WARN: games/heretic2-demo/Makefile:5: Unknown shell command \"/usr/bin/bsdtar\".\n") +} diff --git a/pkgtools/pkglint/files/mkparser.go b/pkgtools/pkglint/files/mkparser.go index 78ff5a74aac..3ddce64a869 100644 --- a/pkgtools/pkglint/files/mkparser.go +++ b/pkgtools/pkglint/files/mkparser.go @@ -27,7 +27,6 @@ func (p *MkParser) MkTokens() []*MkToken { continue } - needsReplace := false again: dollar := strings.IndexByte(repl.rest, '$') if dollar == -1 { @@ -35,13 +34,9 @@ func (p *MkParser) MkTokens() []*MkToken { } repl.Skip(dollar) if repl.AdvanceStr("$$") { - needsReplace = true goto again } text := repl.Since(mark) - if needsReplace { - text = strings.Replace(text, "$$", "$", -1) - } if text != "" { tokens = append(tokens, &MkToken{Text: text}) continue @@ -91,7 +86,7 @@ func (p *MkParser) VarUse() *MkVarUse { if repl.AdvanceStr("$<") { return &MkVarUse{"<", nil} } - if repl.AdvanceRegexp(`^\$(\w)`) { + if repl.PeekByte() == '$' && repl.AdvanceRegexp(`^\$(\w)`) { varname := repl.m[1] if p.EmitWarnings { p.Line.Warn1("$%[1]s is ambiguous. Use ${%[1]s} if you mean a Makefile variable or $$%[1]s if you mean a shell variable.", varname) diff --git a/pkgtools/pkglint/files/mkparser_test.go b/pkgtools/pkglint/files/mkparser_test.go index eb45759e56a..34f6839ec28 100644 --- a/pkgtools/pkglint/files/mkparser_test.go +++ b/pkgtools/pkglint/files/mkparser_test.go @@ -37,7 +37,7 @@ func (s *Suite) Test_MkParser_MkTokens(c *check.C) { check("literal", literal("literal")) check("\\/share\\/ { print \"share directory\" }", literal("\\/share\\/ { print \"share directory\" }")) check("find . -name \\*.orig -o -name \\*.pre", literal("find . -name \\*.orig -o -name \\*.pre")) - check("-e 's|\\$${EC2_HOME.*}|EC2_HOME}|g'", literal("-e 's|\\${EC2_HOME.*}|EC2_HOME}|g'")) + check("-e 's|\\$${EC2_HOME.*}|EC2_HOME}|g'", literal("-e 's|\\$${EC2_HOME.*}|EC2_HOME}|g'")) check("${VARIABLE}", varuse("VARIABLE")) check("${VARIABLE.param}", varuse("VARIABLE.param")) diff --git a/pkgtools/pkglint/files/mkshparser.go b/pkgtools/pkglint/files/mkshparser.go index 2fcf7f79fc5..600e0c92de9 100644 --- a/pkgtools/pkglint/files/mkshparser.go +++ b/pkgtools/pkglint/files/mkshparser.go @@ -5,651 +5,220 @@ import ( "strconv" ) -type MkShParser struct { - tok *ShTokenizer - curr *ShToken -} - -func NewMkShParser(line *Line, text string, emitWarnings bool) *MkShParser { - shp := NewShTokenizer(line, text, emitWarnings) - return &MkShParser{shp, nil} -} - -func (p *MkShParser) Program() (retval *MkShList) { - defer p.trace(&retval)() - - list := p.List() - if list == nil { - return nil - } - separator := p.Separator() - if separator == nil { - return list - } - return &MkShList{list.AndOrs, append(list.Separators, *separator)} -} - -// ::= AndOr (SeparatorOp AndOr)* -func (p *MkShParser) List() (retval *MkShList) { - defer p.trace(&retval)() - ok := false - defer p.rollback(&ok)() - - var andors []*MkShAndOr - var seps []MkShSeparator - - if andor := p.AndOr(); andor != nil { - andors = append(andors, andor) - } else { - return nil - } - -next: - mark := p.mark() - if sep := p.SeparatorOp(); sep != nil { - if andor := p.AndOr(); andor != nil { - andors = append(andors, andor) - seps = append(seps, *sep) - goto next - } - } - p.reset(mark) - - ok = true - return &MkShList{andors, seps} -} - -func (p *MkShParser) AndOr() (retval *MkShAndOr) { - defer p.trace(&retval)() - ok := false - defer p.rollback(&ok) - - var pipes []*MkShPipeline - var ops []string -nextpipe: - if pipe := p.Pipeline(); pipe != nil { - pipes = append(pipes, pipe) - switch op := p.peekText(); op { - case "&&", "||": - p.skip() - p.Linebreak() - ops = append(ops, op) - goto nextpipe - } - } - - if len(pipes) == len(ops) { - return nil - } - ok = true - return &MkShAndOr{pipes, ops} -} - -// ::= Command (msttPipe Linebreak Command)* -func (p *MkShParser) Pipeline() (retval *MkShPipeline) { - defer p.trace(&retval)() - ok := false - defer p.rollback(&ok)() - - bang := p.eat("!") - var cmds []*MkShCommand -nextcmd: - cmd := p.Command() - if cmd == nil { - return nil - } - cmds = append(cmds, cmd) - if p.eat("|") { - p.Linebreak() - goto nextcmd - } - ok = true - return &MkShPipeline{bang, cmds} -} - -func (p *MkShParser) Command() (retval *MkShCommand) { - defer p.trace(&retval)() - - if simple := p.SimpleCommand(); simple != nil { - return &MkShCommand{Simple: simple} - } - if compound := p.CompoundCommand(); compound != nil { - redirects := p.RedirectList() - return &MkShCommand{Compound: compound, Redirects: redirects} - } - if funcdef := p.FunctionDefinition(); funcdef != nil { - return &MkShCommand{FuncDef: funcdef} - } - return nil -} - -func (p *MkShParser) CompoundCommand() (retval *MkShCompoundCommand) { - defer p.trace(&retval)() - - if brace := p.BraceGroup(); brace != nil { - return &MkShCompoundCommand{Brace: brace} - } - if subshell := p.Subshell(); subshell != nil { - return &MkShCompoundCommand{Subshell: subshell} - } - if forclause := p.ForClause(); forclause != nil { - return &MkShCompoundCommand{For: forclause} - } - if caseclause := p.CaseClause(); caseclause != nil { - return &MkShCompoundCommand{Case: caseclause} - } - if ifclause := p.IfClause(); ifclause != nil { - return &MkShCompoundCommand{If: ifclause} - } - if whileclause := p.WhileClause(); whileclause != nil { - return &MkShCompoundCommand{While: whileclause} - } - if untilclause := p.UntilClause(); untilclause != nil { - return &MkShCompoundCommand{Until: untilclause} - } - return nil -} - -func (p *MkShParser) Subshell() (retval *MkShList) { - defer p.trace(&retval)() - ok := false - defer p.rollback(&ok)() - - if !p.eat("(") { - return nil - } - list := p.CompoundList() - if list == nil { - return nil - } - if !p.eat(")") { - return nil - } - ok = true - return list -} - -// ::= Newline* AndOr (Separator AndOr)* Separator? -func (p *MkShParser) CompoundList() (retval *MkShList) { - defer p.trace(&retval)() - ok := false - defer p.rollback(&ok)() - - p.Linebreak() - var andors []*MkShAndOr - var separators []MkShSeparator -nextandor: - if andor := p.AndOr(); andor != nil { - andors = append(andors, andor) - if sep := p.Separator(); sep != nil { - separators = append(separators, *sep) - goto nextandor - } - } - if len(andors) == 0 { - return nil - } - ok = true - return &MkShList{andors, separators} -} - -// ::= "for" msttWORD Linebreak DoGroup -// ::= "for" msttWORD Linebreak "in" SequentialSep DoGroup -// ::= "for" msttWORD Linebreak "in" Wordlist SequentialSep DoGroup -func (p *MkShParser) ForClause() (retval *MkShForClause) { - defer p.trace(&retval)() - ok := false - defer p.rollback(&ok)() - - if !p.eat("for") { - return nil - } - varword := p.Word(false) - if varword == nil || !matches(varword.MkText, `^[A-Z_a-z][0-9A-Za-z]*`) { - return nil - } - varname := varword.MkText - - var values []*ShToken - if p.eat("in") { - values = p.Wordlist() - } else { - values = []*ShToken{NewShToken("\"$$@\"", - NewShAtom(shtWord, "\"", shqDquot), - NewShAtom(shtWord, "$$@", shqDquot), - NewShAtom(shtWord, "\"", shqPlain))} - } - if values == nil || !p.SequentialSep() { - return nil - } - - p.Linebreak() - body := p.DoGroup() - if body == nil { - return nil - } - - ok = true - return &MkShForClause{varname, values, body} -} - -func (p *MkShParser) Wordlist() (retval []*ShToken) { - defer p.trace(&retval)() - - var words []*ShToken -nextword: - word := p.Word(false) - if word != nil { - words = append(words, word) - goto nextword - } - return words -} - -// ::= "case" msttWORD Linebreak "in" Linebreak CaseItem* "esac" -func (p *MkShParser) CaseClause() (retval *MkShCaseClause) { - defer p.trace(&retval)() - ok := false - defer p.rollback(&ok)() - - if !p.eat("case") { - return nil - } - - panic("CaseClause") - p.Linebreak() - p.CaseItem() - return nil -} - -// ::= "("? Pattern ")" (CompoundList | Linebreak) msttDSEMI? Linebreak -func (p *MkShParser) CaseItem() (retval *MkShCaseItem) { - defer p.trace(&retval)() - - panic("CaseItem") - p.Pattern() - p.Linebreak() - p.CompoundList() - return nil -} - -// ::= msttWORD -// ::= Pattern "|" msttWORD -func (p *MkShParser) Pattern() (retval []*ShToken) { - defer p.trace(&retval)() - ok := false - defer p.rollback(&ok)() - - var words []*ShToken -nextword: - word := p.Word(false) - if word == nil { - return nil - } - words = append(words, word) - if p.eat("|") { - goto nextword - - } - ok = true - return words -} - -func (p *MkShParser) IfClause() (retval *MkShIfClause) { - defer p.trace(&retval)() - ok := false - defer p.rollback(&ok)() - - var conds []*MkShList - var actions []*MkShList - var elseaction *MkShList - if !p.eat("if") { - return nil - } - -nextcond: - cond := p.CompoundList() - if cond == nil || !p.eat("then") { - return nil - } - action := p.CompoundList() - if action == nil { - return nil - } - conds = append(conds, cond) - actions = append(actions, action) - if p.eat("elif") { - goto nextcond - } - if p.eat("else") { - elseaction = p.CompoundList() - if elseaction == nil { - return nil - } - } - if !p.eat("fi") { - return nil - } - ok = true - return &MkShIfClause{conds, actions, elseaction} -} - -// ::= "while" CompoundList DoGroup -func (p *MkShParser) WhileClause() (retval *MkShLoopClause) { - defer p.trace(&retval)() - ok := false - defer p.rollback(&ok)() - - if !p.eat("while") { - return nil - } - - panic("WhileClause") - p.CompoundList() - p.DoGroup() - return nil -} - -// ::= "until" CompoundList DoGroup -func (p *MkShParser) UntilClause() (retval *MkShLoopClause) { - defer p.trace(&retval)() - ok := false - defer p.rollback(&ok)() - - if !p.eat("until") { - return nil - } - - panic("UntilClause") - p.CompoundList() - p.DoGroup() - return nil -} - -// ::= msttNAME "(" ")" Linebreak CompoundCommand Redirect* -func (p *MkShParser) FunctionDefinition() (retval *MkShFunctionDefinition) { - defer p.trace(&retval)() - ok := false - defer p.rollback(&ok)() - - funcname := p.Word(true) - if funcname == nil || !matches(funcname.MkText, `^[A-Z_a-z][0-9A-Z_a-z]*$`) { - return nil - } - - if !p.eat("(") || !p.eat(")") { - return nil - } - - p.Linebreak() - - body := p.CompoundCommand() - if body == nil { - return nil - } - - redirects := p.RedirectList() - ok = true - return &MkShFunctionDefinition{funcname.MkText, body, redirects} -} - -func (p *MkShParser) BraceGroup() (retval *MkShList) { - defer p.trace(&retval)() - ok := false - defer p.rollback(&ok)() - - if !p.eat("{") { - return nil - } - list := p.CompoundList() - if list == nil { - return nil - } - if !p.eat("}") { - return nil +func parseShellProgram(line *Line, program string) (list *MkShList, err error) { + if G.opts.Debug { + defer tracecall(program)() } - ok = true - return list -} -func (p *MkShParser) DoGroup() (retval *MkShList) { - defer p.trace(&retval)() - ok := false - defer p.rollback(&ok)() - - if !p.eat("do") { - return nil - } - list := p.CompoundList() - if list == nil { - return nil - } - if !p.eat("done") { - return nil - } - ok = true - return list -} + tokens, rest := splitIntoShellTokens(line, program) + lexer := NewShellLexer(tokens, rest) + parser := ­yParserImpl{} -func (p *MkShParser) SimpleCommand() (retval *MkShSimpleCommand) { - defer p.trace(&retval)() - ok := false - defer p.rollback(&ok)() + succeeded := parser.Parse(lexer) - var assignments []*ShToken - var name *ShToken - var args []*ShToken - var redirections []*MkShRedirection - first := true - seenName := false -nextword: - if word := p.Word(first); word != nil { - first = false - if !seenName && word.IsAssignment() { - assignments = append(assignments, word) - } else if !seenName { - name = word - seenName = true - } else { - args = append(args, word) - } - goto nextword + if succeeded == 0 && lexer.error == "" { + return lexer.result, nil } - if len(assignments) == 0 && name == nil && len(args) == 0 && len(redirections) == 0 { - return nil - } - ok = true - return &MkShSimpleCommand{assignments, name, args, redirections} + return nil, &ParseError{append([]string{lexer.current}, lexer.remaining...)} } -func (p *MkShParser) RedirectList() (retval []*MkShRedirection) { - defer p.trace(&retval)() - -nextredirect: - if redirect := p.IoRedirect(); redirect != nil { - retval = append(retval, redirect) - goto nextredirect - } - return nil +type ParseError struct { + RemainingTokens []string } -func (p *MkShParser) IoRedirect() (retval *MkShRedirection) { - defer p.trace(&retval)() - - if m, redirect, fdstr, op := match3(p.peekText(), `^((\d*)\s*(<|<&|>|>&|>>|<>|>\||<<|<<-))`); m { - target := p.peekText()[len(redirect):] - _, _, _ = fdstr, op, target - - fd, err := strconv.ParseInt(fdstr, 10, 32) - if err != nil { - fd = -1 - } - p.skip() - targetToken := NewShTokenizer(p.tok.mkp.Line, target, false).ShToken() - return &MkShRedirection{int(fd), op, targetToken} - } - return nil +func (e *ParseError) Error() string { + return fmt.Sprintf("parse error at %v", e.RemainingTokens) } -func (p *MkShParser) NewlineList() (retval bool) { - defer p.trace(&retval)() - - ok := false - for p.eat("\n") { - ok = true - } - return ok +type ShellLexer struct { + current string + ioredirect string + remaining []string + atCommandStart bool + sinceFor int + sinceCase int + error string + result *MkShList } -func (p *MkShParser) Linebreak() { - for p.eat("\n") { - } +func NewShellLexer(tokens []string, rest string) *ShellLexer { + return &ShellLexer{ + current: "", + ioredirect: "", + remaining: tokens, + atCommandStart: true, + error: rest} } - -func (p *MkShParser) SeparatorOp() (retval *MkShSeparator) { - defer p.trace(&retval)() - - if p.eat(";") { - op := MkShSeparator(";") - return &op +func (lex *ShellLexer) Lex(lval *shyySymType) (ttype int) { + if len(lex.remaining) == 0 { + return 0 } - if p.eat("&") { - op := MkShSeparator("&") - return &op - } - return nil -} - -func (p *MkShParser) Separator() (retval *MkShSeparator) { - defer p.trace(&retval)() - op := p.SeparatorOp() - if op == nil && p.eat("\n") { - sep := MkShSeparator('\n') - op = &sep - } - if op != nil { - p.Linebreak() - } - return op -} - -func (p *MkShParser) SequentialSep() (retval bool) { - defer p.trace(&retval)() - - if p.peekText() == ";" { - p.skip() - p.Linebreak() - return true - } else { - return p.NewlineList() - } -} - -func (p *MkShParser) Word(cmdstart bool) (retval *ShToken) { - defer p.trace(&retval)() - - if token := p.peek(); token != nil && token.IsWord() { - if cmdstart { - switch token.MkText { - case "while", "until", "for", "do", "done", - "if", "then", "else", "elif", "fi", - "{", "}": - return nil + if G.opts.Debug { + defer func() { + tname := shyyTokname(shyyTok2[ttype-shyyPrivate]) + switch ttype { + case tkWORD, tkASSIGNMENT_WORD: + traceStep("lex %v %q", tname, lval.Word.MkText) + case tkIO_NUMBER: + traceStep("lex %v %v", tname, lval.IONum) + default: + traceStep("lex %v", tname) } + }() + } + + token := lex.ioredirect + lex.ioredirect = "" + if token == "" { + token = lex.remaining[0] + lex.current = token + lex.remaining = lex.remaining[1:] + } + + switch token { + case ";": + lex.atCommandStart = true + return tkSEMI + case ";;": + lex.atCommandStart = true + return tkSEMISEMI + case "\n": + lex.atCommandStart = true + return tkNEWLINE + case "&": + lex.atCommandStart = true + return tkBACKGROUND + case "|": + lex.atCommandStart = true + return tkPIPE + case "(": + lex.atCommandStart = true + return tkLPAREN + case ")": + lex.atCommandStart = true + return tkRPAREN + case "&&": + lex.atCommandStart = true + return tkAND + case "||": + lex.atCommandStart = true + return tkOR + case ">": + lex.atCommandStart = false + return tkGT + case ">&": + lex.atCommandStart = false + return tkGTAND + case "<": + lex.atCommandStart = false + return tkLT + case "<&": + lex.atCommandStart = false + return tkLTAND + case "<>": + lex.atCommandStart = false + return tkLTGT + case ">>": + lex.atCommandStart = false + return tkGTGT + case "<<": + lex.atCommandStart = false + return tkLTLT + case "<<-": + lex.atCommandStart = false + return tkLTLTDASH + case ">|": + lex.atCommandStart = false + return tkGTPIPE + } + + if m, fdstr, op := match2(token, `^(\d+)(<<-|<<|<>|<&|>>|>&|>\||<|>)$`); m { + fd, _ := strconv.Atoi(fdstr) + lval.IONum = fd + lex.ioredirect = op + return tkIO_NUMBER + } + + if lex.atCommandStart { + lex.sinceCase = -1 + lex.sinceFor = -1 + switch token { + case "if": + return tkIF + case "then": + return tkTHEN + case "elif": + return tkELIF + case "else": + return tkELSE + case "fi": + return tkFI + case "for": + lex.atCommandStart = false + lex.sinceFor = 0 + return tkFOR + case "while": + return tkWHILE + case "until": + return tkUNTIL + case "do": + return tkDO + case "done": + return tkDONE + case "in": + lex.atCommandStart = false + return tkIN + case "case": + lex.atCommandStart = false + lex.sinceCase = 0 + return tkCASE + case "{": + return tkLBRACE + case "}": + return tkRBRACE + case "!": + return tkEXCLAM } - p.skip() - return token - } - return nil -} - -func (p *MkShParser) EOF() bool { - return p.peek() == nil -} - -func (p *MkShParser) peek() *ShToken { - if p.curr == nil { - nexttoken: - p.curr = p.tok.ShToken() - if p.curr == nil && !p.tok.parser.EOF() { - p.tok.mkp.Line.Warnf("Pkglint tokenize error at " + p.tok.parser.Rest()) - p.tok.mkp.Parser.repl.AdvanceRest() - return nil - } - if p.curr != nil && hasPrefix(p.curr.MkText, "#") { - goto nexttoken - } - } - //traceStep("MkShParser.peek %v rest=%q", p.curr, p.tok.mkp.repl.rest) - return p.curr -} - -func (p *MkShParser) peekText() string { - if next := p.peek(); next != nil { - return next.MkText } - return "" -} - -func (p *MkShParser) skip() { - p.curr = nil -} - -func (p *MkShParser) eat(s string) bool { - if p.peek() == nil { - return false - } - if p.peek().MkText == s { - p.skip() - return true - } - return false -} - -func (p *MkShParser) rollback(pok *bool) func() { - mark := p.mark() - return func() { - if !*pok { - p.reset(mark) - } - } -} - -func (p *MkShParser) trace(retval interface{}) func() { - if G.opts.Debug { - return tracecallInternal(p.peek(), p.restref(), "=>", ref(retval)) - } else { - return func() {} - } -} - -func (p *MkShParser) mark() MkShParserMark { - return MkShParserMark{p.tok.parser.repl.Mark(), p.curr} -} - -func (p *MkShParser) reset(mark MkShParserMark) { - p.tok.parser.repl.Reset(mark.rest) - p.curr = mark.curr -} - -func (p *MkShParser) restref() MkShParserRest { - return MkShParserRest{&p.tok.mkp.repl.rest} -} - -func (p *MkShParser) Rest() string { - return p.peekText() + p.tok.mkp.repl.AdvanceRest() -} - -type MkShParserMark struct { - rest PrefixReplacerMark - curr *ShToken -} - -type MkShParserRest struct { - restref *string -} -func (rest MkShParserRest) String() string { - return fmt.Sprintf("rest=%q", *rest.restref) + if lex.sinceFor >= 0 { + lex.sinceFor++ + } + if lex.sinceCase >= 0 { + lex.sinceCase++ + } + + switch { + case matches(token, `^\d+$`) && len(lex.remaining) != 0 && matches(lex.remaining[0], `^[<>]`): + ttype = tkIO_NUMBER + lval.IONum, _ = strconv.Atoi(token) + case lex.sinceFor == 2 && token == "in": + ttype = tkIN + lex.atCommandStart = false + case lex.sinceFor == 2 && token == "do": + ttype = tkDO + lex.atCommandStart = true + case lex.sinceCase == 2 && token == "in": + ttype = tkIN + lex.atCommandStart = false + case (lex.atCommandStart || lex.sinceCase == 3) && token == "esac": + ttype = tkESAC + lex.atCommandStart = false + case lex.atCommandStart && matches(token, `^[A-Za-z_]\w*=`): + ttype = tkASSIGNMENT_WORD + p := NewShTokenizer(dummyLine, token, false) + lval.Word = p.ShToken() + default: + ttype = tkWORD + p := NewShTokenizer(dummyLine, token, false) + lval.Word = p.ShToken() + lex.atCommandStart = false + } + + return ttype +} + +func (lex *ShellLexer) Error(s string) { + lex.error = s } diff --git a/pkgtools/pkglint/files/mkshparser_test.go b/pkgtools/pkglint/files/mkshparser_test.go index bf28129bcf2..e0d6bbd2a71 100644 --- a/pkgtools/pkglint/files/mkshparser_test.go +++ b/pkgtools/pkglint/files/mkshparser_test.go @@ -1,352 +1,595 @@ package main import ( - check "gopkg.in/check.v1" + "encoding/json" + "gopkg.in/check.v1" + "strconv" ) -func (s *Suite) Test_MkShParser_Program(c *check.C) { - parse := func(cmd string, expected *MkShList) { - p := NewMkShParser(dummyLine, cmd, false) - program := p.Program() - c.Check(program, deepEquals, expected) - c.Check(p.tok.parser.Rest(), equals, "") - c.Check(s.Output(), equals, "") - } - - if false { - parse(""+ - "\tcd ${WRKSRC} && ${FIND} ${${_list_}} -type f ! -name '*.orig' 2>/dev/null "+ - "| pax -rw -pm ${DESTDIR}${PREFIX}/${${_dir_}}", - NewMkShList()) - } +type ShSuite struct { + c *check.C } -func (s *Suite) Test_MkShParser_List(c *check.C) { +var _ = check.Suite(&ShSuite{}) + +func (s *ShSuite) Test_ShellParser_program(c *check.C) { + b := s.init(c) + + s.test("", + b.List()) + + s.test("echo ;", + b.List().AddCommand(b.SimpleCommand("echo")).AddSeparator(";")) + + s.test("echo", + b.List().AddCommand(b.SimpleCommand("echo"))) + + s.test(""+ + "cd ${WRKSRC} && ${FIND} ${${_list_}} -type f ! -name '*.orig' 2 > /dev/null "+ + "| pax -rw -pm ${DESTDIR}${PREFIX}/${${_dir_}}", + b.List().AddAndOr(b.AndOr( + b.Pipeline(false, b.SimpleCommand("cd", "${WRKSRC}"))).Add("&&", + b.Pipeline(false, + b.SimpleCommand("${FIND}", "${${_list_}}", "-type", "f", "!", "-name", "'*.orig'", "2>/dev/null"), + b.SimpleCommand("pax", "-rw", "-pm", "${DESTDIR}${PREFIX}/${${_dir_}}"))))) + + s.test("${CAT} ${PKGDIR}/PLIST | while read entry ; do : ; done", + b.List().AddAndOr(b.AndOr(b.Pipeline(false, + b.SimpleCommand("${CAT}", "${PKGDIR}/PLIST"), + b.While( + b.List().AddCommand(b.SimpleCommand("read", "entry")).AddSeparator(";"), + b.List().AddCommand(b.SimpleCommand(":")).AddSeparator(";")))))) + + s.test("while read entry ; do case \"$$entry\" in include/c-client/* ) ${INSTALL_DATA} $$src $$dest ; esac ; done", + b.List().AddCommand(b.While( + b.List().AddCommand(b.SimpleCommand("read", "entry")).AddSeparator(";"), + b.List().AddCommand(b.Case( + b.Token("\"$$entry\""), + b.CaseItem( + b.Words("include/c-client/*"), + b.List().AddCommand(b.SimpleCommand("${INSTALL_DATA}", "$$src", "$$dest")), + &SEP_SEMI))).AddSeparator(";")))) + + s.test("command | while condition ; do case selector in pattern ) : ; esac ; done", + b.List().AddAndOr(b.AndOr(b.Pipeline(false, + b.SimpleCommand("command"), + b.While( + b.List().AddCommand(b.SimpleCommand("condition")).AddSeparator(";"), + b.List().AddCommand(b.Case( + b.Token("selector"), + b.CaseItem( + b.Words("pattern"), + b.List().AddCommand(b.SimpleCommand(":")), + &SEP_SEMI))).AddSeparator(";")))))) + + s.test("command1 \n command2 \n command3", + b.List(). + AddCommand(b.SimpleCommand("command1")). + AddSeparator(SEP_NEWLINE). + AddCommand(b.SimpleCommand("command2")). + AddSeparator(SEP_NEWLINE). + AddCommand(b.SimpleCommand("command3"))) + + s.test("if condition; then action; else case selector in pattern) case-item-action ;; esac; fi", + b.List().AddCommand(b.If( + b.List().AddCommand(b.SimpleCommand("condition")).AddSeparator(";"), + b.List().AddCommand(b.SimpleCommand("action")).AddSeparator(";"), + b.List().AddCommand(b.Case( + b.Token("selector"), + b.CaseItem( + b.Words("pattern"), + b.List().AddCommand(b.SimpleCommand("case-item-action")), nil))).AddSeparator(";")))) +} + +func (s *ShSuite) Test_ShellParser_list(c *check.C) { + b := s.init(c) + + s.test("echo1 && echo2", + b.List().AddAndOr( + b.AndOr(b.Pipeline(false, b.SimpleCommand("echo1"))). + Add("&&", b.Pipeline(false, b.SimpleCommand("echo2"))))) + + s.test("echo1 ; echo2", + b.List(). + AddCommand(b.SimpleCommand("echo1")). + AddSeparator(";"). + AddCommand(b.SimpleCommand("echo2"))) + + s.test("echo1 ; echo2 &", + b.List(). + AddCommand(b.SimpleCommand("echo1")). + AddSeparator(";"). + AddCommand(b.SimpleCommand("echo2")). + AddSeparator("&")) +} + +func (s *ShSuite) Test_ShellParser_and_or(c *check.C) { + b := s.init(c) + + s.test("echo1 | echo2", + b.List().AddAndOr(b.AndOr(b.Pipeline(false, + b.SimpleCommand("echo1"), + b.SimpleCommand("echo2"))))) + + s.test("echo1 | echo2 && echo3 | echo4", + b.List().AddAndOr(b.AndOr( + b.Pipeline(false, + b.SimpleCommand("echo1"), + b.SimpleCommand("echo2")), + ).Add("&&", + b.Pipeline(false, + b.SimpleCommand("echo3"), + b.SimpleCommand("echo4"))))) + + s.test("echo1 | echo2 || ! echo3 | echo4", + b.List().AddAndOr(b.AndOr( + b.Pipeline(false, + b.SimpleCommand("echo1"), + b.SimpleCommand("echo2")), + ).Add("||", + b.Pipeline(true, + b.SimpleCommand("echo3"), + b.SimpleCommand("echo4"))))) +} + +func (s *ShSuite) Test_ShellParser_pipeline(c *check.C) { + b := s.init(c) + + s.test("command1 | command2", + b.List().AddAndOr(b.AndOr(b.Pipeline(false, + b.SimpleCommand("command1"), + b.SimpleCommand("command2"))))) + + s.test("! command1 | command2", + b.List().AddAndOr(b.AndOr(b.Pipeline(true, + b.SimpleCommand("command1"), + b.SimpleCommand("command2"))))) +} + +func (s *ShSuite) Test_ShellParser_pipe_sequence(c *check.C) { + b := s.init(c) + + s.test("command1 | if true ; then : ; fi", + b.List().AddAndOr(b.AndOr(b.Pipeline(false, + b.SimpleCommand("command1"), + b.If( + b.List().AddCommand(b.SimpleCommand("true")).AddSeparator(";"), + b.List().AddCommand(b.SimpleCommand(":")).AddSeparator(";")))))) +} + +func (s *ShSuite) Test_ShellParser_command(c *check.C) { + b := s.init(c) + + s.test("simple_command", + b.List().AddAndOr(b.AndOr(b.Pipeline(false, b.SimpleCommand("simple_command"))))) + + s.test("while 1 ; do 2 ; done", + b.List().AddAndOr(b.AndOr(b.Pipeline(false, + b.While( + b.List().AddCommand(b.SimpleCommand("1")).AddSeparator(";"), + b.List().AddCommand(b.SimpleCommand("2")).AddSeparator(";")))))) + + s.test("while 1 ; do 2 ; done 1 >& 2", + b.List().AddAndOr(b.AndOr(b.Pipeline(false, + b.While( + b.List().AddCommand(b.SimpleCommand("1")).AddSeparator(";"), + b.List().AddCommand(b.SimpleCommand("2")).AddSeparator(";"), + b.Redirection(1, ">&", "2")))))) + + s.test("func ( ) { echo hello ; } 2 >& 1", + b.List().AddCommand(b.Function( + "func", + b.Brace(b.List().AddCommand(b.SimpleCommand("echo", "hello")).AddSeparator(";")).Compound, + b.Redirection(2, ">&", "1")))) +} + +func (s *ShSuite) Test_ShellParser_compound_command(c *check.C) { + b := s.init(c) + + s.test("{ brace ; }", + b.List().AddCommand(b.Brace( + b.List().AddCommand(b.SimpleCommand("brace")).AddSeparator(";")))) + + s.test("( subshell )", + b.List().AddCommand(b.Subshell( + b.List().AddCommand(b.SimpleCommand("subshell"))))) + + s.test("for i in * ; do echo $i ; done", + b.List().AddCommand(b.For( + "i", + b.Words("*"), + b.List().AddCommand(b.SimpleCommand("echo", "$i")).AddSeparator(";")))) + + s.test("case $i in esac", + b.List().AddCommand(b.Case( + b.Token("$i")))) + +} + +func (s *ShSuite) Test_ShellParser_subshell(c *check.C) { + b := s.init(c) + + sub3 := b.Subshell(b.List().AddCommand(b.SimpleCommand("sub3"))) + sub2 := b.Subshell(b.List().AddCommand(sub3).AddSeparator(";").AddCommand(b.SimpleCommand("sub2"))) + sub1 := b.Subshell(b.List().AddCommand(sub2).AddSeparator(";").AddCommand(b.SimpleCommand("sub1"))) + s.test("( ( ( sub3 ) ; sub2 ) ; sub1 )", b.List().AddCommand(sub1)) +} + +func (s *ShSuite) Test_ShellParser_compound_list(c *check.C) { + b := s.init(c) -} - -func (s *Suite) Test_MkShParser_AndOr(c *check.C) { - parse := func(cmd string, expected *MkShAndOr) { - p := NewMkShParser(dummyLine, cmd, false) - andor := p.AndOr() - c.Check(andor, deepEquals, expected) - c.Check(p.tok.parser.Rest(), equals, "") - c.Check(s.Output(), equals, "") - } - tester := &MkShTester{c} + s.test("( \n echo )", + b.List().AddCommand(b.Subshell( + b.List().AddCommand(b.SimpleCommand("echo"))))) +} - parse("simplecmd", - NewMkShAndOr(NewMkShPipeline(false, tester.ParseCommand("simplecmd")))) +func (s *ShSuite) Test_ShellParser_term(c *check.C) { + b := s.init(c) - expected := NewMkShAndOr(NewMkShPipeline(false, tester.ParseCommand("simplecmd1"))) - expected.Add("&&", NewMkShPipeline(false, tester.ParseCommand("simplecmd2"))) - parse("simplecmd1 && simplecmd2", expected) + _ = b } -func (s *Suite) Test_MkShParser_Pipeline(c *check.C) { +func (s *ShSuite) Test_ShellParser_for_clause(c *check.C) { + b := s.init(c) -} + s.test("for var do echo $var ; done", + b.List().AddCommand(b.For( + "var", + b.Words("\"$$@\""), + b.List().AddCommand(b.SimpleCommand("echo", "$var")).AddSeparator(";")))) -func (s *Suite) Test_MkShParser_Command(c *check.C) { - parse := func(cmd string, expected *MkShCommand) { - p := NewMkShParser(dummyLine, cmd, false) - command := p.Command() - c.Check(command, deepEquals, expected) - c.Check(p.tok.parser.Rest(), equals, "") - c.Check(s.Output(), equals, "") - } - tester := &MkShTester{c} + // Only linebreak is allowed, but not semicolon. + s.test("for var \n do echo $var ; done", + b.List().AddCommand(b.For( + "var", + b.Words("\"$$@\""), + b.List().AddCommand(b.SimpleCommand("echo", "$var")).AddSeparator(";")))) - parse("simple", - &MkShCommand{Simple: tester.ParseSimpleCommand("simple")}) -} + s.test("for var in a b c ; do echo $var ; done", + b.List().AddCommand(b.For( + "var", + b.Words("a", "b", "c"), + b.List().AddCommand(b.SimpleCommand("echo", "$var")).AddSeparator(";")))) -func (s *Suite) Test_MkShParser_CompoundCommand(c *check.C) { + s.test("for var \n \n \n in a b c ; do echo $var ; done", + b.List().AddCommand(b.For( + "var", + b.Words("a", "b", "c"), + b.List().AddCommand(b.SimpleCommand("echo", "$var")).AddSeparator(";")))) -} - -func (s *Suite) Test_MkShParser_Subshell(c *check.C) { + s.test("for var in in esac ; do echo $var ; done", + b.List().AddCommand(b.For( + "var", + b.Words("in", "esac"), + b.List().AddCommand(b.SimpleCommand("echo", "$var")).AddSeparator(";")))) + // No semicolon necessary between the two “done”. + s.test("for i in 1; do for j in 1; do echo $$i$$j; done done", + b.List().AddCommand(b.For( + "i", + b.Words("1"), + b.List().AddCommand(b.For( + "j", + b.Words("1"), + b.List().AddCommand(b.SimpleCommand("echo", "$$i$$j")).AddSeparator(";")))))) } -func (s *Suite) Test_MkShParser_CompoundList(c *check.C) { - parse := func(cmd string, expected *MkShList) { - p := NewMkShParser(dummyLine, cmd, false) - compoundList := p.CompoundList() - c.Check(compoundList, deepEquals, expected) - c.Check(p.tok.parser.Rest(), equals, "") - c.Check(s.Output(), equals, "") - } - tester := &MkShTester{c} - - parse("simplecmd", - NewMkShList().AddAndOr(NewMkShAndOr(NewMkShPipeline(false, tester.ParseCommand("simplecmd"))))) +func (s *ShSuite) Test_ShellParser_case_clause(c *check.C) { + b := s.init(c) - simplecmd1 := NewMkShPipeline(false, tester.ParseCommand("simplecmd1")) - simplecmd2 := NewMkShPipeline(false, tester.ParseCommand("simplecmd2")) - expected := NewMkShList().AddAndOr(NewMkShAndOr(simplecmd1).Add("&&", simplecmd2)) - parse("simplecmd1 && simplecmd2", expected) -} + s.test("case $var in esac", + b.List().AddCommand(b.Case(b.Token("$var")))) -func (s *Suite) Test_MkShParser_ForClause(c *check.C) { - parse := func(cmd string, expected *MkShForClause) { - p := NewMkShParser(dummyLine, cmd, false) - forclause := p.ForClause() - c.Check(forclause, deepEquals, expected) - c.Check(p.tok.parser.Rest(), equals, "") - c.Check(s.Output(), equals, "") - } - tester := &MkShTester{c} + s.test("case selector in pattern) ;; pattern) esac", + b.List().AddCommand(b.Case( + b.Token("selector"), + b.CaseItem(b.Words("pattern"), b.List(), nil), + b.CaseItem(b.Words("pattern"), b.List(), nil)))) - params := []*ShToken{tester.Token("\"$$@\"")} - action := tester.ParseCompoundList("action;") - parse("for var; do action; done", - &MkShForClause{"var", params, action}) + s.test("case $$i in *.c | *.h ) echo C ;; * ) echo Other ; esac", + b.List().AddCommand(b.Case( + b.Token("$$i"), + b.CaseItem(b.Words("*.c", "*.h"), b.List().AddCommand(b.SimpleCommand("echo", "C")), nil), + b.CaseItem(b.Words("*"), b.List().AddCommand(b.SimpleCommand("echo", "Other")), &SEP_SEMI)))) - abc := []*ShToken{tester.Token("a"), tester.Token("b"), tester.Token("c")} - parse("for var in a b c; do action; done", - &MkShForClause{"var", abc, action}) + s.test("case $$i in *.c ) echo ; esac", + b.List().AddCommand(b.Case( + b.Token("$$i"), + b.CaseItem(b.Words("*.c"), b.List().AddCommand(b.SimpleCommand("echo")), &SEP_SEMI)))) - actions := tester.ParseCompoundList("action1 && action2;") - parse("for var in a b c; do action1 && action2; done", - &MkShForClause{"var", abc, actions}) -} + s.test("case selector in pattern) case-item-action ; esac", + b.List().AddCommand(b.Case( + b.Token("selector"), + b.CaseItem( + b.Words("pattern"), + b.List().AddCommand(b.SimpleCommand("case-item-action")), &SEP_SEMI)))) -func (s *Suite) Test_MkShParser_Wordlist(c *check.C) { + s.test("case selector in pattern) case-item-action ;; esac", + b.List().AddCommand(b.Case( + b.Token("selector"), + b.CaseItem( + b.Words("pattern"), + b.List().AddCommand(b.SimpleCommand("case-item-action")), nil)))) } -func (s *Suite) Test_MkShParser_CaseClause(c *check.C) { +func (s *ShSuite) Test_ShellParser_if_clause(c *check.C) { + b := s.init(c) -} + s.test( + "if true ; then echo yes ; else echo no ; fi", + b.List().AddCommand(b.If( + b.List().AddCommand(b.SimpleCommand("true")).AddSeparator(";"), + b.List().AddCommand(b.SimpleCommand("echo", "yes")).AddSeparator(";"), + b.List().AddCommand(b.SimpleCommand("echo", "no")).AddSeparator(";")))) -func (s *Suite) Test_MkShParser_CaseItem(c *check.C) { + // No semicolon necessary between the two “fi”. + s.test("if cond1; then if cond2; then action; fi fi", + b.List().AddCommand(b.If( + b.List().AddCommand(b.SimpleCommand("cond1")).AddSeparator(";"), + b.List().AddCommand(b.If( + b.List().AddCommand(b.SimpleCommand("cond2")).AddSeparator(";"), + b.List().AddCommand(b.SimpleCommand("action")).AddSeparator(";")))))) +} + +func (s *ShSuite) Test_ShellParser_while_clause(c *check.C) { + b := s.init(c) + + s.test("while condition ; do action ; done", + b.List().AddCommand(b.While( + b.List().AddCommand(b.SimpleCommand("condition")).AddSeparator(";"), + b.List().AddCommand(b.SimpleCommand("action")).AddSeparator(";")))) +} + +func (s *ShSuite) Test_ShellParser_until_clause(c *check.C) { + b := s.init(c) + s.test("until condition ; do action ; done", + b.List().AddCommand(b.Until( + b.List().AddCommand(b.SimpleCommand("condition")).AddSeparator(";"), + b.List().AddCommand(b.SimpleCommand("action")).AddSeparator(";")))) } -func (s *Suite) Test_MkShParser_Pattern(c *check.C) { +func (s *ShSuite) Test_ShellParser_function_definition(c *check.C) { + b := s.init(c) + _ = b } -func (s *Suite) Test_MkShParser_IfClause(c *check.C) { +func (s *ShSuite) Test_ShellParser_brace_group(c *check.C) { + b := s.init(c) + // No semicolon necessary after the closing brace. + s.test("if true; then { echo yes; } fi", + b.List().AddCommand(b.If( + b.List().AddCommand(b.SimpleCommand("true")).AddSeparator(";"), + b.List().AddCommand(b.Brace( + b.List().AddCommand(b.SimpleCommand("echo", "yes")).AddSeparator(";")))))) } -func (s *Suite) Test_MkShParser_WhileClause(c *check.C) { +func (s *ShSuite) Test_ShellParser_simple_command(c *check.C) { + b := s.init(c) -} + s.test( + "echo hello, world", + b.List().AddCommand(b.SimpleCommand("echo", "hello,", "world"))) -func (s *Suite) Test_MkShParser_UntilClause(c *check.C) { + s.test("echo ${PKGNAME:Q}", + b.List().AddCommand(b.SimpleCommand("echo", "${PKGNAME:Q}"))) -} + s.test("${ECHO} \"Double-quoted\" 'Single-quoted'", + b.List().AddCommand(b.SimpleCommand("${ECHO}", "\"Double-quoted\"", "'Single-quoted'"))) -func (s *Suite) Test_MkShParser_FunctionDefinition(c *check.C) { + s.test("`cat plain` \"`cat double`\" '`cat single`'", + b.List().AddCommand(b.SimpleCommand("`cat plain`", "\"`cat double`\"", "'`cat single`'"))) -} + s.test("`\"one word\"`", + b.List().AddCommand(b.SimpleCommand("`\"one word\"`"))) -func (s *Suite) Test_MkShParser_BraceGroup(c *check.C) { + s.test("PAGES=\"`ls -1 | ${SED} -e 's,3qt$$,3,'`\"", + b.List().AddCommand(b.SimpleCommand("PAGES=\"`ls -1 | ${SED} -e 's,3qt$$,3,'`\""))) -} + s.test("var=Plain var=\"Dquot\" var='Squot' var=Plain\"Dquot\"'Squot'", + b.List().AddCommand(b.SimpleCommand("var=Plain", "var=\"Dquot\"", "var='Squot'", "var=Plain\"Dquot\"'Squot'"))) -func (s *Suite) Test_MkShParser_DoGroup(c *check.C) { - tester := &MkShTester{c} - check := func(str string, expected *MkShList) { - p := NewMkShParser(dummyLine, str, false) - dogroup := p.DoGroup() - if c.Check(dogroup, check.NotNil) { - if !c.Check(dogroup, deepEquals, expected) { - for i, andor := range dogroup.AndOrs { - c.Check(andor, deepEquals, expected.AndOrs[i]) - } - } - } - c.Check(p.tok.parser.Rest(), equals, "") - c.Check(s.Output(), equals, "") - } + // RUN is a special Make variable since it ends with a semicolon; + // therefore it needs to be split off before passing the rest of + // the command to the shell command parser. + s.test("${RUN} subdir=\"`unzip -c \"$$e\" install.rdf | awk '/re/ { print \"hello\" }'`\"", + b.List().AddCommand(b.SimpleCommand("${RUN}", "subdir=\"`unzip -c \"$$e\" install.rdf | awk '/re/ { print \"hello\" }'`\""))) - andor := NewMkShAndOr(NewMkShPipeline(false, tester.ParseCommand("action"))) - check("do action; done", - &MkShList{[]*MkShAndOr{andor}, []MkShSeparator{";"}}) -} - -func (s *Suite) Test_MkShParser_SimpleCommand(c *check.C) { - parse := func(cmd string, builder *SimpleCommandBuilder) { - expected := builder.Cmd - p := NewMkShParser(dummyLine, cmd, false) - shcmd := p.SimpleCommand() - if c.Check(shcmd, check.NotNil) { - if !c.Check(shcmd, deepEquals, expected) { - for i, assignment := range shcmd.Assignments { - c.Check(assignment, deepEquals, expected.Assignments[i]) - } - c.Check(shcmd.Name, deepEquals, expected.Name) - for i, word := range shcmd.Args { - c.Check(word, deepEquals, expected.Args[i]) - } - for i, redirection := range shcmd.Redirections { - c.Check(redirection, deepEquals, expected.Redirections[i]) - } - } - } - c.Check(p.tok.parser.Rest(), equals, "") - c.Check(s.Output(), equals, "") - } + s.test("PATH=/nonexistent env PATH=${PATH:Q} true", + b.List().AddCommand(b.SimpleCommand("PATH=/nonexistent", "env", "PATH=${PATH:Q}", "true"))) - fail := func(noncmd string, expectedRest string) { - p := NewMkShParser(dummyLine, noncmd, false) - shcmd := p.SimpleCommand() - c.Check(shcmd, check.IsNil) - c.Check(p.tok.parser.Rest(), equals, expectedRest) - c.Check(s.Output(), equals, "") - } - tester := &MkShTester{c} + s.test("{OpenGrok args", + b.List().AddCommand(b.SimpleCommand("{OpenGrok", "args"))) +} - parse("echo ${PKGNAME:Q}", - NewSimpleCommandBuilder(). - Name(tester.Token("echo")). - Arg(tester.Token("${PKGNAME:Q}"))) +func (s *ShSuite) Test_ShellParser_io_redirect(c *check.C) { + b := s.init(c) - parse("${ECHO} \"Double-quoted\" 'Single-quoted'", - NewSimpleCommandBuilder(). - Name(tester.Token("${ECHO}")). - Arg(tester.Token("\"Double-quoted\"")). - Arg(tester.Token("'Single-quoted'"))) + s.test("echo >> ${PLIST_SRC}", + b.List().AddCommand(b.SimpleCommand("echo", ">>${PLIST_SRC}"))) - parse("`cat plain` \"`cat double`\" '`cat single`'", - NewSimpleCommandBuilder(). - Name(tester.Token("`cat plain`")). - Arg(tester.Token("\"`cat double`\"")). - Arg(tester.Token("'`cat single`'"))) + s.test("echo >> ${PLIST_SRC}", + b.List().AddCommand(b.SimpleCommand("echo", ">>${PLIST_SRC}"))) - parse("`\"one word\"`", - NewSimpleCommandBuilder(). - Name(tester.Token("`\"one word\"`"))) + s.test("echo 1>output 2>>append 3>|clobber 4>&5 6>append", + b.List().AddCommand(&MkShCommand{Simple: &MkShSimpleCommand{ + Assignments: nil, + Name: b.Token("echo"), + Args: nil, + Redirections: []*MkShRedirection{ + {1, ">", b.Token("output")}, + {2, ">>", b.Token("append")}, + {3, ">|", b.Token("clobber")}, + {4, ">&", b.Token("5")}, + {6, "<", b.Token("input")}, + {-1, ">>", b.Token("append")}}}})) - parse("PAGES=\"`ls -1 | ${SED} -e 's,3qt$$,3,'`\"", - NewSimpleCommandBuilder(). - Assignment(tester.Token("PAGES=\"`ls -1 | ${SED} -e 's,3qt$$,3,'`\""))) + s.test("echo 1> output 2>> append 3>| clobber 4>& 5 6< input >> append", + b.List().AddCommand(&MkShCommand{Simple: &MkShSimpleCommand{ + Assignments: nil, + Name: b.Token("echo"), + Args: nil, + Redirections: []*MkShRedirection{ + {1, ">", b.Token("output")}, + {2, ">>", b.Token("append")}, + {3, ">|", b.Token("clobber")}, + {4, ">&", b.Token("5")}, + {6, "<", b.Token("input")}, + {-1, ">>", b.Token("append")}}}})) +} - parse("var=Plain var=\"Dquot\" var='Squot' var=Plain\"Dquot\"'Squot'", - NewSimpleCommandBuilder(). - Assignment(tester.Token("var=Plain")). - Assignment(tester.Token("var=\"Dquot\"")). - Assignment(tester.Token("var='Squot'")). - Assignment(tester.Token("var=Plain\"Dquot\"'Squot'"))) +func (s *ShSuite) Test_ShellParser_io_here(c *check.C) { + b := s.init(c) - parse("${RUN} subdir=\"`unzip -c \"$$e\" install.rdf | awk '/re/ { print \"hello\" }'`\"", - NewSimpleCommandBuilder(). - Name(tester.Token("${RUN}")). - Arg(tester.Token("subdir=\"`unzip -c \"$$e\" install.rdf | awk '/re/ { print \"hello\" }'`\""))) + _ = b +} - parse("PATH=/nonexistent env PATH=${PATH:Q} true", - NewSimpleCommandBuilder(). - Assignment(tester.Token("PATH=/nonexistent")). - Name(tester.Token("env")). - Arg(tester.Token("PATH=${PATH:Q}")). - Arg(tester.Token("true"))) +func (s *ShSuite) init(c *check.C) *MkShBuilder { + s.c = c + return NewMkShBuilder() +} - parse("{OpenGrok args", - NewSimpleCommandBuilder(). - Name(tester.Token("{OpenGrok")). - Arg(tester.Token("args"))) +func (s *ShSuite) test(program string, expected *MkShList) { + tokens, rest := splitIntoShellTokens(dummyLine, program) + s.c.Check(rest, equals, "") + lexer := &ShellLexer{ + current: "", + remaining: tokens, + atCommandStart: true, + error: ""} + parser := ­yParserImpl{} - fail("if clause", "if clause") - fail("{ group; }", "{ group; }") + succeeded := parser.Parse(lexer) -} + c := s.c -func (s *Suite) Test_MkShParser_RedirectList(c *check.C) { + if ok1, ok2 := c.Check(succeeded, equals, 0), c.Check(lexer.error, equals, ""); ok1 && ok2 { + if !c.Check(lexer.result, deepEquals, expected) { + actualJson, actualErr := json.MarshalIndent(lexer.result, "", " ") + expectedJson, expectedErr := json.MarshalIndent(expected, "", " ") + if c.Check(actualErr, check.IsNil) && c.Check(expectedErr, check.IsNil) { + c.Check(string(actualJson), deepEquals, string(expectedJson)) + } + } + } else { + c.Check(lexer.remaining, deepEquals, []string{}) + } } -func (s *Suite) Test_MkShParser_IoRedirect(c *check.C) { +type MkShBuilder struct { } -func (s *Suite) Test_MkShParser_IoFile(c *check.C) { +func NewMkShBuilder() *MkShBuilder { + return &MkShBuilder{} } -func (s *Suite) Test_MkShParser_IoHere(c *check.C) { +func (b *MkShBuilder) List() *MkShList { + return NewMkShList() } -func (s *Suite) Test_MkShParser_NewlineList(c *check.C) { +func (b *MkShBuilder) AndOr(pipeline *MkShPipeline) *MkShAndOr { + return NewMkShAndOr(pipeline) } -func (s *Suite) Test_MkShParser_Linebreak(c *check.C) { +func (b *MkShBuilder) Pipeline(negated bool, cmds ...*MkShCommand) *MkShPipeline { + return NewMkShPipeline(negated, cmds...) } -func (s *Suite) Test_MkShParser_SeparatorOp(c *check.C) { - +func (b *MkShBuilder) SimpleCommand(words ...string) *MkShCommand { + cmd := &MkShSimpleCommand{} + assignments := true + for _, word := range words { + if assignments && matches(word, `^\w+=`) { + cmd.Assignments = append(cmd.Assignments, b.Token(word)) + } else if m, fdstr, op, rest := match3(word, `^(\d*)(<<-|<<|<&|>>|>&|>\||<|>)(.*)$`); m { + fd, err := strconv.Atoi(fdstr) + if err != nil { + fd = -1 + } + cmd.Redirections = append(cmd.Redirections, b.Redirection(fd, op, rest)) + } else { + assignments = false + if cmd.Name == nil { + cmd.Name = b.Token(word) + } else { + cmd.Args = append(cmd.Args, b.Token(word)) + } + } + } + return &MkShCommand{Simple: cmd} +} + +func (b *MkShBuilder) If(condActionElse ...*MkShList) *MkShCommand { + ifclause := &MkShIfClause{} + for i, part := range condActionElse { + if i%2 == 0 && i != len(condActionElse)-1 { + ifclause.Conds = append(ifclause.Conds, part) + } else if i%2 == 1 { + ifclause.Actions = append(ifclause.Actions, part) + } else { + ifclause.Else = part + } + } + return &MkShCommand{Compound: &MkShCompoundCommand{If: ifclause}} } -func (s *Suite) Test_MkShParser_Separator(c *check.C) { - +func (b *MkShBuilder) For(varname string, items []*ShToken, action *MkShList) *MkShCommand { + return &MkShCommand{Compound: &MkShCompoundCommand{For: &MkShForClause{varname, items, action}}} } -func (s *Suite) Test_MkShParser_SequentialSep(c *check.C) { - +func (b *MkShBuilder) Case(selector *ShToken, items ...*MkShCaseItem) *MkShCommand { + return &MkShCommand{Compound: &MkShCompoundCommand{Case: &MkShCaseClause{selector, items}}} } -func (s *Suite) Test_MkShParser_Word(c *check.C) { - +func (b *MkShBuilder) CaseItem(patterns []*ShToken, action *MkShList, separator *MkShSeparator) *MkShCaseItem { + return &MkShCaseItem{patterns, action, separator} } -type MkShTester struct { - c *check.C +func (b *MkShBuilder) While(cond, action *MkShList, redirects ...*MkShRedirection) *MkShCommand { + return &MkShCommand{ + Compound: &MkShCompoundCommand{ + Loop: &MkShLoopClause{cond, action, false}}, + Redirects: redirects} } -func (t *MkShTester) ParseCommand(str string) *MkShCommand { - p := NewMkShParser(dummyLine, str, false) - cmd := p.Command() - t.c.Check(cmd, check.NotNil) - t.c.Check(p.Rest(), equals, "") - return cmd +func (b *MkShBuilder) Until(cond, action *MkShList, redirects ...*MkShRedirection) *MkShCommand { + return &MkShCommand{ + Compound: &MkShCompoundCommand{ + Loop: &MkShLoopClause{cond, action, true}}, + Redirects: redirects} } -func (t *MkShTester) ParseSimpleCommand(str string) *MkShSimpleCommand { - p := NewMkShParser(dummyLine, str, false) - parsed := p.SimpleCommand() - t.c.Check(parsed, check.NotNil) - t.c.Check(p.Rest(), equals, "") - return parsed +func (b *MkShBuilder) Function(name string, body *MkShCompoundCommand, redirects ...*MkShRedirection) *MkShCommand { + return &MkShCommand{ + FuncDef: &MkShFunctionDefinition{name, body}, + Redirects: redirects} } -func (t *MkShTester) ParseCompoundList(str string) *MkShList { - p := NewMkShParser(dummyLine, str, false) - parsed := p.CompoundList() - t.c.Check(parsed, check.NotNil) - t.c.Check(p.Rest(), equals, "") - return parsed +func (b *MkShBuilder) Brace(list *MkShList) *MkShCommand { + return &MkShCommand{Compound: &MkShCompoundCommand{Brace: list}} } -func (t *MkShTester) Token(str string) *ShToken { - p := NewMkShParser(dummyLine, str, false) - parsed := p.peek() - p.skip() - t.c.Check(parsed, check.NotNil) - t.c.Check(p.Rest(), equals, "") - return parsed +func (b *MkShBuilder) Subshell(list *MkShList) *MkShCommand { + return &MkShCommand{Compound: &MkShCompoundCommand{Subshell: list}} } -type SimpleCommandBuilder struct { - Cmd *MkShSimpleCommand +func (b *MkShBuilder) Token(mktext string) *ShToken { + tokenizer := NewShTokenizer(dummyLine, mktext, false) + token := tokenizer.ShToken() + return token } -func NewSimpleCommandBuilder() *SimpleCommandBuilder { - cmd := &MkShSimpleCommand{} - return &SimpleCommandBuilder{cmd} -} -func (b *SimpleCommandBuilder) Name(name *ShToken) *SimpleCommandBuilder { - b.Cmd.Name = name - return b -} -func (b *SimpleCommandBuilder) Assignment(assignment *ShToken) *SimpleCommandBuilder { - b.Cmd.Assignments = append(b.Cmd.Assignments, assignment) - return b -} -func (b *SimpleCommandBuilder) Arg(arg *ShToken) *SimpleCommandBuilder { - b.Cmd.Args = append(b.Cmd.Args, arg) - return b +func (b *MkShBuilder) Words(words ...string) []*ShToken { + tokens := make([]*ShToken, len(words)) + for i, word := range words { + tokens[i] = b.Token(word) + } + return tokens } -func (b *SimpleCommandBuilder) Redirection(redirection *MkShRedirection) *SimpleCommandBuilder { - b.Cmd.Redirections = append(b.Cmd.Redirections, redirection) - return b + +func (b *MkShBuilder) Redirection(fd int, op string, target string) *MkShRedirection { + return &MkShRedirection{fd, op, b.Token(target)} } diff --git a/pkgtools/pkglint/files/mkshtypes.go b/pkgtools/pkglint/files/mkshtypes.go index 994c0450504..617ddced691 100644 --- a/pkgtools/pkglint/files/mkshtypes.go +++ b/pkgtools/pkglint/files/mkshtypes.go @@ -1,8 +1,6 @@ package main -import ( - "fmt" -) +import "fmt" type MkShList struct { AndOrs []*MkShAndOr @@ -13,10 +11,6 @@ func NewMkShList() *MkShList { return &MkShList{nil, nil} } -func (list *MkShList) String() string { - return fmt.Sprintf("MkShList(%v)", list.AndOrs) -} - func (list *MkShList) AddAndOr(andor *MkShAndOr) *MkShList { list.AndOrs = append(list.AndOrs, andor) return list @@ -36,10 +30,6 @@ func NewMkShAndOr(pipeline *MkShPipeline) *MkShAndOr { return &MkShAndOr{[]*MkShPipeline{pipeline}, nil} } -func (andor *MkShAndOr) String() string { - return fmt.Sprintf("MkShAndOr(%v)", andor.Pipes) -} - func (andor *MkShAndOr) Add(op string, pipeline *MkShPipeline) *MkShAndOr { andor.Pipes = append(andor.Pipes, pipeline) andor.Ops = append(andor.Ops, op) @@ -55,10 +45,6 @@ func NewMkShPipeline(negated bool, cmds ...*MkShCommand) *MkShPipeline { return &MkShPipeline{negated, cmds} } -func (pipe *MkShPipeline) String() string { - return fmt.Sprintf("MkShPipeline(%v)", pipe.Cmds) -} - func (pipe *MkShPipeline) Add(cmd *MkShCommand) *MkShPipeline { pipe.Cmds = append(pipe.Cmds, cmd) return pipe @@ -71,46 +57,13 @@ type MkShCommand struct { Redirects []*MkShRedirection // For Compound and FuncDef } -func (cmd *MkShCommand) String() string { - switch { - case cmd.Simple != nil: - return cmd.Simple.String() - case cmd.Compound != nil: - return cmd.Compound.String() - case cmd.FuncDef != nil: - return cmd.FuncDef.String() - } - return "MkShCommand(?)" -} - type MkShCompoundCommand struct { Brace *MkShList Subshell *MkShList For *MkShForClause Case *MkShCaseClause If *MkShIfClause - While *MkShLoopClause - Until *MkShLoopClause -} - -func (cmd *MkShCompoundCommand) String() string { - switch { - case cmd.Brace != nil: - return cmd.Brace.String() - case cmd.Subshell != nil: - return cmd.Subshell.String() - case cmd.For != nil: - return cmd.For.String() - case cmd.Case != nil: - return cmd.Case.String() - case cmd.If != nil: - return cmd.If.String() - case cmd.While != nil: - return cmd.While.String() - case cmd.Until != nil: - return cmd.Until.String() - } - return "MkShCompoundCommand(?)" + Loop *MkShLoopClause } type MkShForClause struct { @@ -119,22 +72,15 @@ type MkShForClause struct { Body *MkShList } -func (cl *MkShForClause) String() string { - return fmt.Sprintf("MkShForClause(%v, %v, %v)", cl.Varname, cl.Values, cl.Body) -} - type MkShCaseClause struct { Word *ShToken Cases []*MkShCaseItem } -func (cl *MkShCaseClause) String() string { - return fmt.Sprintf("MkShCaseClause(...)") -} - type MkShCaseItem struct { - Patterns []*ShToken - Action *MkShList + Patterns []*ShToken + Action *MkShList + Separator *MkShSeparator } type MkShIfClause struct { @@ -143,10 +89,6 @@ type MkShIfClause struct { Else *MkShList } -func (cl *MkShIfClause) String() string { - return "MkShIf(...)" -} - func (cl *MkShIfClause) Prepend(cond *MkShList, action *MkShList) { cl.Conds = append([]*MkShList{cond}, cl.Conds...) cl.Actions = append([]*MkShList{action}, cl.Actions...) @@ -158,18 +100,9 @@ type MkShLoopClause struct { Until bool } -func (cl *MkShLoopClause) String() string { - return "MkShLoop(...)" -} - type MkShFunctionDefinition struct { - Name string - Body *MkShCompoundCommand - Redirects []*MkShRedirection -} - -func (def *MkShFunctionDefinition) String() string { - return "MkShFunctionDef(...)" + Name string + Body *MkShCompoundCommand } type MkShSimpleCommand struct { @@ -179,33 +112,49 @@ type MkShSimpleCommand struct { Redirections []*MkShRedirection } -func (scmd *MkShSimpleCommand) String() string { - str := "SimpleCommand(" - first := true - sep := func() { - if first { - first = false - } else { - str += ", " - } +func NewStrCommand(cmd *MkShSimpleCommand) *StrCommand { + strcmd := &StrCommand{ + make([]string, len(cmd.Assignments)), + "", + make([]string, len(cmd.Args))} + for i, assignment := range cmd.Assignments { + strcmd.Assignments[i] = assignment.MkText } - for _, word := range scmd.Assignments { - sep() - str += word.MkText + if cmd.Name != nil { + strcmd.Name = cmd.Name.MkText } - if word := scmd.Name; word != nil { - sep() - str += word.MkText + for i, arg := range cmd.Args { + strcmd.Args[i] = arg.MkText } - for _, word := range scmd.Args { - sep() - str += word.MkText + return strcmd +} + +type StrCommand struct { + Assignments []string + Name string + Args []string +} + +func (c *StrCommand) HasOption(opt string) bool { + for _, arg := range c.Args { + if arg == opt { + return true + } } - for _, redirection := range scmd.Redirections { - sep() - str += redirection.String() + return false +} + +func (c *StrCommand) AnyArgMatches(pattern string) bool { + for _, arg := range c.Args { + if matches(arg, pattern) { + return true + } } - return str + ")" + return false +} + +func (c *StrCommand) String() string { + return fmt.Sprintf("%v %v %v", c.Assignments, c.Name, c.Args) } type MkShRedirection struct { @@ -214,18 +163,11 @@ type MkShRedirection struct { Target *ShToken } -func (r *MkShRedirection) String() string { - if r.Fd != -1 { - return fmt.Sprintf("%d%s%s", r.Fd, r.Op, r.Target.MkText) - } else { - return r.Op + r.Target.MkText - } -} - // One of ";", "&", "\n" type MkShSeparator string -func (sep *MkShSeparator) String() string { - return fmt.Sprintf("%q", sep) - -} +var ( + SEP_SEMI MkShSeparator = ";" + SEP_BACKGROUND MkShSeparator = "&" + SEP_NEWLINE MkShSeparator = "\n" +) diff --git a/pkgtools/pkglint/files/mkshwalker.go b/pkgtools/pkglint/files/mkshwalker.go new file mode 100644 index 00000000000..97c79becc0d --- /dev/null +++ b/pkgtools/pkglint/files/mkshwalker.go @@ -0,0 +1,153 @@ +package main + +type MkShWalker struct { +} + +func (w *MkShWalker) Walk(list *MkShList, callback func(node interface{})) { + for element := range w.iterate(list) { + callback(element) + } +} + +func (w *MkShWalker) iterate(list *MkShList) chan interface{} { + elements := make(chan interface{}) + + go func() { + w.walkList(list, elements) + close(elements) + }() + + return elements +} + +func (w *MkShWalker) walkList(list *MkShList, collector chan interface{}) { + collector <- list + + for _, andor := range list.AndOrs { + w.walkAndOr(andor, collector) + } +} + +func (w *MkShWalker) walkAndOr(andor *MkShAndOr, collector chan interface{}) { + collector <- andor + + for _, pipeline := range andor.Pipes { + w.walkPipeline(pipeline, collector) + } +} + +func (w *MkShWalker) walkPipeline(pipeline *MkShPipeline, collector chan interface{}) { + collector <- pipeline + + for _, command := range pipeline.Cmds { + w.walkCommand(command, collector) + } +} + +func (w *MkShWalker) walkCommand(command *MkShCommand, collector chan interface{}) { + collector <- command + + switch { + case command.Simple != nil: + w.walkSimpleCommand(command.Simple, collector) + case command.Compound != nil: + w.walkCompoundCommand(command.Compound, collector) + w.walkRedirects(command.Redirects, collector) + case command.FuncDef != nil: + w.walkFunctionDefinition(command.FuncDef, collector) + w.walkRedirects(command.Redirects, collector) + } +} + +func (w *MkShWalker) walkSimpleCommand(command *MkShSimpleCommand, collector chan interface{}) { + collector <- command + + w.walkWords(command.Assignments, collector) + if command.Name != nil { + w.walkWord(command.Name, collector) + } + w.walkWords(command.Args, collector) + w.walkRedirects(command.Redirections, collector) +} + +func (w *MkShWalker) walkCompoundCommand(command *MkShCompoundCommand, collector chan interface{}) { + collector <- command + + switch { + case command.Brace != nil: + w.walkList(command.Brace, collector) + case command.Case != nil: + w.walkCase(command.Case, collector) + case command.For != nil: + w.walkFor(command.For, collector) + case command.If != nil: + w.walkIf(command.If, collector) + case command.Loop != nil: + w.walkLoop(command.Loop, collector) + case command.Subshell != nil: + w.walkList(command.Subshell, collector) + } +} + +func (w *MkShWalker) walkCase(caseClause *MkShCaseClause, collector chan interface{}) { + collector <- caseClause + + w.walkWord(caseClause.Word, collector) + for _, caseItem := range caseClause.Cases { + collector <- caseItem + w.walkWords(caseItem.Patterns, collector) + w.walkList(caseItem.Action, collector) + } +} + +func (w *MkShWalker) walkFunctionDefinition(funcdef *MkShFunctionDefinition, collector chan interface{}) { + collector <- funcdef + + w.walkCompoundCommand(funcdef.Body, collector) +} + +func (w *MkShWalker) walkIf(ifClause *MkShIfClause, collector chan interface{}) { + collector <- ifClause + for i, cond := range ifClause.Conds { + w.walkList(cond, collector) + w.walkList(ifClause.Actions[i], collector) + } + if ifClause.Else != nil { + w.walkList(ifClause.Else, collector) + } +} + +func (w *MkShWalker) walkLoop(loop *MkShLoopClause, collector chan interface{}) { + collector <- loop + w.walkList(loop.Cond, collector) + w.walkList(loop.Action, collector) +} + +func (w *MkShWalker) walkWords(words []*ShToken, collector chan interface{}) { + collector <- words + + for _, word := range words { + w.walkWord(word, collector) + } +} + +func (w *MkShWalker) walkWord(word *ShToken, collector chan interface{}) { + collector <- word +} + +func (w *MkShWalker) walkRedirects(redirects []*MkShRedirection, collector chan interface{}) { + collector <- redirects + + for _, redirect := range redirects { + collector <- redirect + w.walkWord(redirect.Target, collector) + } +} + +func (w *MkShWalker) walkFor(forClause *MkShForClause, collector chan interface{}) { + collector <- forClause + + collector <- forClause.Varname + w.walkWords(forClause.Values, collector) + w.walkList(forClause.Body, collector) +} diff --git a/pkgtools/pkglint/files/mkshwalker_test.go b/pkgtools/pkglint/files/mkshwalker_test.go new file mode 100644 index 00000000000..fe36292b815 --- /dev/null +++ b/pkgtools/pkglint/files/mkshwalker_test.go @@ -0,0 +1,32 @@ +package main + +import ( + "gopkg.in/check.v1" +) + +func (s *Suite) Test_MkShWalker_Walk(c *check.C) { + list, err := parseShellProgram(dummyLine, ""+ + "if condition; then action; else case selector in pattern) case-item-action ;; esac; fi; "+ + "set -e; cd ${WRKSRC}/locale; "+ + "for lang in *.po; do "+ + " [ \"$${lang}\" = \"wxstd.po\" ] && continue; "+ + " ${TOOLS_PATH.msgfmt} -c -o \"$${lang%.po}.mo\" \"$${lang}\"; "+ + "done") + if c.Check(err, check.IsNil) && c.Check(list, check.NotNil) { + var commands []string + (*MkShWalker).Walk(nil, list, func(node interface{}) { + if cmd, ok := node.(*MkShSimpleCommand); ok { + commands = append(commands, NewStrCommand(cmd).String()) + } + }) + c.Check(commands, deepEquals, []string{ + "[] condition []", + "[] action []", + "[] case-item-action []", + "[] set [-e]", + "[] cd [${WRKSRC}/locale]", + "[] [ [\"$${lang}\" = \"wxstd.po\" ]]", + "[] continue []", + "[] ${TOOLS_PATH.msgfmt} [-c -o \"$${lang%.po}.mo\" \"$${lang}\"]"}) + } +} diff --git a/pkgtools/pkglint/files/mktypes_test.go b/pkgtools/pkglint/files/mktypes_test.go index 161b6f95737..cbeda6cc368 100644 --- a/pkgtools/pkglint/files/mktypes_test.go +++ b/pkgtools/pkglint/files/mktypes_test.go @@ -9,3 +9,9 @@ func (s *Suite) Test_MkVarUse_Mod(c *check.C) { c.Check(varuse.Mod(), equals, ":Q") } + +func (list *MkShList) AddCommand(command *MkShCommand) *MkShList { + pipeline := NewMkShPipeline(false, command) + andOr := NewMkShAndOr(pipeline) + return list.AddAndOr(andOr) +} diff --git a/pkgtools/pkglint/files/package.go b/pkgtools/pkglint/files/package.go index 1c2d4c2c7b3..bfc6104a42a 100644 --- a/pkgtools/pkglint/files/package.go +++ b/pkgtools/pkglint/files/package.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "os/user" "path" "regexp" "strconv" @@ -196,6 +197,7 @@ func checkdirPackage(pkgpath string) { } else if hasSuffix(fname, "/distinfo") { haveDistinfo = true } + pkg.checkLocallyModified(fname) } if G.opts.CheckDistinfo && G.opts.CheckPatches { @@ -387,7 +389,7 @@ func (pkg *Package) checkfilePackageMakefile(fname string, mklines *MkLines) { perlLine.Warn1("REPLACE_PERL is ignored when NO_CONFIGURE is set (in %s)", noconfLine.Line.ReferenceFrom(perlLine.Line)) } - if vardef["LICENSE"] == nil { + if vardef["LICENSE"] == nil && vardef["META_PACKAGE"] == nil { NewLineWhole(fname).Error0("Each package must define its LICENSE.") } @@ -484,19 +486,43 @@ func (pkg *Package) determineEffectivePkgVars() { } func (pkg *Package) pkgnameFromDistname(pkgname, distname string) string { - pkgname = strings.Replace(pkgname, "${DISTNAME}", distname, -1) + tokens := NewMkParser(dummyLine, pkgname, false).MkTokens() - if m, before, sep, subst, after := match4(pkgname, `^(.*)\$\{DISTNAME:S(.)([^\\}:]+)\}(.*)$`); m { - qsep := regexp.QuoteMeta(sep) - if m, left, from, right, to, mod := match5(subst, `^(\^?)([^:]*)(\$?)`+qsep+`([^:]*)`+qsep+`(g?)$`); m { - newPkgname := before + mkopSubst(distname, left != "", from, right != "", to, mod != "") + after + subst := func(str, smod string) (result string) { + if G.opts.Debug { + defer tracecall(str, smod, ref(result))() + } + qsep := regexp.QuoteMeta(smod[1:2]) + if m, left, from, right, to, flags := match5(smod, `^S`+qsep+`(\^?)([^:]*?)(\$?)`+qsep+`([^:]*)`+qsep+`([1g]*)$`); m { + result := mkopSubst(str, left != "", from, right != "", to, flags) if G.opts.Debug { - traceStep("%s: pkgnameFromDistname %q => %q", pkg.vardef["PKGNAME"], pkgname, newPkgname) + traceStep("subst %q %q => %q", str, smod, result) + } + return result + } + return str + } + + result := "" + for _, token := range tokens { + if token.Varuse != nil && token.Varuse.varname == "DISTNAME" { + newDistname := distname + for _, mod := range token.Varuse.modifiers { + if mod == "tl" { + newDistname = strings.ToLower(newDistname) + } else if hasPrefix(mod, "S") { + newDistname = subst(newDistname, mod) + } else { + newDistname = token.Text + break + } } - pkgname = newPkgname + result += newDistname + } else { + result += token.Text } } - return pkgname + return result } func (pkg *Package) checkUpdate() { @@ -758,3 +784,53 @@ func (mklines *MkLines) checkForUsedComment(relativeName string) { } SaveAutofixChanges(lines) } + +func (pkg *Package) checkLocallyModified(fname string) { + if G.opts.Debug { + defer tracecall(fname)() + } + + ownerLine := pkg.vardef["OWNER"] + maintainerLine := pkg.vardef["MAINTAINER"] + owner := "" + maintainer := "" + if ownerLine != nil && !containsVarRef(ownerLine.Value()) { + owner = ownerLine.Value() + } + if maintainerLine != nil && !containsVarRef(maintainerLine.Value()) && maintainerLine.Value() != "pkgsrc-users@NetBSD.org" { + maintainer = maintainerLine.Value() + } + if owner == "" && maintainer == "" { + return + } + + user, err := user.Current() + if err != nil || user.Username == "" { + return + } + // On Windows, this is `Computername\Username`. + username := regcomp(`^.*\\`).ReplaceAllString(user.Username, "") + + if G.opts.Debug { + traceStep("user=%q owner=%q maintainer=%q", username, owner, maintainer) + } + + if username == strings.Split(owner, "@")[0] || username == strings.Split(maintainer, "@")[0] { + return + } + + if isLocallyModified(fname) { + if owner != "" { + NewLineWhole(fname).Warn1("Don't commit changes to this file without asking the OWNER, %s.", owner) + Explain2( + "See the pkgsrc guide, section \"Package components\",", + "keyword \"owner\", for more information.") + } + if maintainer != "" { + NewLineWhole(fname).Note1("Please only commit changes that %s would approve.", maintainer) + Explain2( + "See the pkgsrc guide, section \"Package components\",", + "keyword \"maintainer\", for more information.") + } + } +} diff --git a/pkgtools/pkglint/files/package_test.go b/pkgtools/pkglint/files/package_test.go index 7c596c69054..6f8b16abd24 100644 --- a/pkgtools/pkglint/files/package_test.go +++ b/pkgtools/pkglint/files/package_test.go @@ -14,6 +14,9 @@ func (s *Suite) TestPkgnameFromDistname(c *check.C) { c.Check(pkg.pkgnameFromDistname("${DISTNAME:S|a|b|g}", "panama-0.13"), equals, "pbnbmb-0.13") c.Check(pkg.pkgnameFromDistname("${DISTNAME:S|^lib||}", "libncurses"), equals, "ncurses") c.Check(pkg.pkgnameFromDistname("${DISTNAME:S|^lib||}", "mylib"), equals, "mylib") + c.Check(pkg.pkgnameFromDistname("${DISTNAME:tl:S/-/./g:S/he/-/1}", "SaxonHE9-5-0-1J"), equals, "saxon-9.5.0.1j") + c.Check(pkg.pkgnameFromDistname("${DISTNAME:C/beta/.0./}", "fspanel-0.8beta1"), equals, "${DISTNAME:C/beta/.0./}") + c.Check(pkg.pkgnameFromDistname("${DISTNAME:S/-0$/.0/1}", "aspell-af-0.50-0"), equals, "aspell-af-0.50.0") c.Check(s.Output(), equals, "") } @@ -157,6 +160,19 @@ func (s *Suite) TestCheckdirPackage(c *check.C) { "WARN: ~/Makefile: No COMMENT given.\n") } +func (s *Suite) Test_Package_Meta_package_License(c *check.C) { + s.CreateTmpFileLines(c, "Makefile", + "# $"+"NetBSD$", + "", + "META_PACKAGE=\tyes") + G.CurrentDir = s.tmpdir + G.globalData.InitVartypes() + + checkdirPackage(s.tmpdir) + + c.Check(s.Output(), equals, "WARN: ~/Makefile: No COMMENT given.\n") // No error about missing LICENSE. +} + func (s *Suite) Test_Package_Varuse_LoadTime(c *check.C) { s.CreateTmpFileLines(c, "doc/CHANGES-2016", "# dummy") diff --git a/pkgtools/pkglint/files/pkglint.go b/pkgtools/pkglint/files/pkglint.go index 417782108ac..e89aff7085c 100644 --- a/pkgtools/pkglint/files/pkglint.go +++ b/pkgtools/pkglint/files/pkglint.go @@ -202,7 +202,7 @@ func Checkfile(fname string) { } case st.Mode()&os.ModeSymlink != 0: - if !matches(basename, `^work`) { + if !hasPrefix(basename, "work") { NewLineWhole(fname).Warn0("Unknown symlink name.") } @@ -284,6 +284,9 @@ func Checkfile(fname string) { case matches(fname, `(?:^|/)files/[^/]*$`): // Skip + case basename == "spec": + // Ok in regression tests + default: NewLineWhole(fname).Warn0("Unexpected file found.") if G.opts.CheckExtra { diff --git a/pkgtools/pkglint/files/plist.go b/pkgtools/pkglint/files/plist.go index 7df3ca97b23..7122f4eb0b2 100644 --- a/pkgtools/pkglint/files/plist.go +++ b/pkgtools/pkglint/files/plist.go @@ -326,7 +326,8 @@ func (ck *PlistChecker) checkpathSbin(pline *PlistLine) { func (ck *PlistChecker) checkpathShare(pline *PlistLine) { line, text := pline.line, pline.text switch { - case hasPrefix(text, "share/applications/") && hasSuffix(text, ".desktop"): + // Disabled due to PR 46570, item "10. It should stop". + case false && hasPrefix(text, "share/applications/") && hasSuffix(text, ".desktop"): f := "../../sysutils/desktop-file-utils/desktopdb.mk" if G.opts.WarnExtra && G.Pkg != nil && G.Pkg.included[f] == nil { line.Warn1("Packages that install a .desktop entry should .include %q.", f) diff --git a/pkgtools/pkglint/files/plist_test.go b/pkgtools/pkglint/files/plist_test.go index bb67ca0f69b..086d98311ac 100644 --- a/pkgtools/pkglint/files/plist_test.go +++ b/pkgtools/pkglint/files/plist_test.go @@ -145,6 +145,9 @@ func (s *Suite) TestPlistChecker_sort(c *check.C) { } func (s *Suite) TestPlistChecker_checkpathShare_Desktop(c *check.C) { + // Disabled due to PR 46570, item "10. It should stop". + return + s.UseCommandLine(c, "-Wextra") G.Pkg = NewPackage("category/pkgpath") diff --git a/pkgtools/pkglint/files/shell.go b/pkgtools/pkglint/files/shell.go index a52cc228f14..57e4db3d59a 100644 --- a/pkgtools/pkglint/files/shell.go +++ b/pkgtools/pkglint/files/shell.go @@ -8,34 +8,6 @@ import ( ) const ( - reShellToken = `^\s*(` + - `#.*` + // shell comment - `|(?:` + - `'[^']*'` + // single quoted string - "|\"`[^`]+`\"" + // backticks command execution in double quotes - `|"(?:\\.|[^"])*"` + // double quoted string - "|`[^`]*`" + // backticks command execution (very simple case) - `|\\\$\$` + // a shell-escaped dollar sign - `|\\[^\$]` + // other escaped characters - `|\$[\w_]` + // one-character make(1) variable - `|\$\$[0-9A-Z_a-z]+` + // shell variable - `|\$\$[!#?@]` + // special shell variables - `|\$\$[./]` + // unescaped dollar in shell, followed by punctuation - `|\$\$\$\$` + // the special pid shell variable - `|\$\$\{[0-9A-Z_a-z]+[#%:]?[^}]*\}` + // shell variable in braces - `|[^\(\)'\"\\\s;&\|<>` + "`" + `\$]` + // non-special character - `|\$\{[^\s\"'` + "`" + `]+` + // HACK: nested make(1) variables - `)+` + // any of the above may be repeated - `|\$\$\(` + // POSIX-style backticks replacement - `|;;?` + - `|&&?` + - `|\|\|?` + - `|\(` + - `|\)` + - `|>&` + - `|<>?` + - `|#.*)` reShVarassign = `^([A-Z_a-z]\w*)=` reShVarname = `(?:[!#*\-\d?@]|\$\$|[A-Za-z_]\w*)` reShVarexpansion = `(?:(?:#|##|%|%%|:-|:=|:\?|:\+|\+)[^$\\{}]*)` @@ -43,63 +15,6 @@ const ( reShDollar = `\\\$\$|` + reShVaruse + `|\$\$[,\-/|]` ) -// ShellCommandState -type scState uint8 - -const ( - scstStart scState = iota - scstCont - scstInstall - scstInstallD - scstMkdir - scstPax - scstPaxS - scstSed - scstSedE - scstSet - scstSetCont - scstCond - scstCondCont - scstCase - scstCaseIn - scstCaseLabel - scstCaseLabelCont - scstFor - scstForIn - scstForCont - scstEcho - scstInstallDir - scstInstallDir2 -) - -func (st scState) String() string { - return [...]string{ - "start", - "continuation", - "install", - "install -d", - "mkdir", - "pax", - "pax -s", - "sed", - "sed -e", - "set", - "set-continuation", - "cond", - "cond-continuation", - "case", - "case in", - "case label", - "case-label-continuation", - "for", - "for-in", - "for-continuation", - "echo", - "install-dir", - "install-dir2", - }[st] -} - type ShellLine struct { line *Line mkline *MkLine @@ -355,12 +270,6 @@ func (shline *ShellLine) variableNeedsQuoting(shvarname string) bool { return true } -type ShelltextContext struct { - shline *ShellLine - state scState - shellword string -} - func (shline *ShellLine) CheckShellCommandLine(shelltext string) { if G.opts.Debug { defer tracecall1(shelltext)() @@ -411,62 +320,44 @@ func (shline *ShellLine) CheckShellCommandLine(shelltext string) { } func (shline *ShellLine) CheckShellCommand(shellcmd string, pSetE *bool) { - if false { - p := NewMkShParser(shline.line, shellcmd, false) - cmds := p.Program() - rest := p.tok.parser.Rest() - if rest != "" { - traceStep("shellcmd=%q", shellcmd) - if cmds != nil { - for _, andor := range cmds.AndOrs { - traceStep("AndOr %v", andor) - } - } - shline.line.Warnf("Pkglint parse error in ShellLine.CheckShellCommand at %q", p.peekText()+rest) - } + if G.opts.Debug { + defer tracecall()() } - state := scstStart - tokens, rest := splitIntoShellTokens(shline.line, shellcmd) - if rest != "" { - shline.line.Warnf("Pkglint parse error in ShellLine.CheckShellCommand at %q (state=%s)", rest, state) + program, err := parseShellProgram(shline.line, shellcmd) + if err != nil && contains(shellcmd, "$$(") { // Hack until the shell parser can handle subshells. + shline.line.Warn0("Invoking subshells via $(...) is not portable enough.") + return + } + if err != nil { + shline.line.Warnf("Pkglint ShellLine.CheckShellCommand: %s", err) + return } - prevToken := "" - for _, token := range tokens { - if G.opts.Debug { - traceStep("checkShellCommand state=%v token=%q", state, token) - } + spc := &ShellProgramChecker{shline} + spc.checkConditionalCd(program) - { - noQuotingNeeded := state == scstCase || - state == scstForCont || - state == scstSetCont || - (state == scstStart && matches(token, reShVarassign)) - shline.CheckWord(token, !noQuotingNeeded) + (*MkShWalker).Walk(nil, program, func(node interface{}) { + if cmd, ok := node.(*MkShSimpleCommand); ok { + scc := NewSimpleCommandChecker(shline, cmd) + scc.Check() + if scc.strcmd.Name == "set" && scc.strcmd.AnyArgMatches(`^-.*e`) { + *pSetE = true + } } - st := &ShelltextContext{shline, state, token} - st.checkCommandStart() - st.checkConditionalCd() - if state != scstPaxS && state != scstSedE && state != scstCaseLabel { - shline.line.CheckAbsolutePathname(token) + if cmd, ok := node.(*MkShList); ok { + spc.checkSetE(cmd, pSetE) } - st.checkAutoMkdirs() - st.checkInstallMulti() - st.checkPaxPe() - st.checkQuoteSubstitution() - st.checkEchoN() - st.checkPipeExitcode() - st.checkSetE(pSetE, prevToken) - - if state == scstSet && hasPrefix(token, "-") && contains(token, "e") || state == scstStart && token == "${RUN}" { - *pSetE = true + + if cmd, ok := node.(*MkShPipeline); ok { + spc.checkPipeExitcode(shline.line, cmd) } - state = shline.nextState(state, token) - prevToken = token - } + if word, ok := node.(*ShToken); ok { + spc.checkWord(word, false) + } + }) } func (shline *ShellLine) CheckShellCommands(shellcmds string) { @@ -493,7 +384,9 @@ func (shline *ShellLine) checkHiddenAndSuppress(hiddenAndSuppress, rest string) // Shell comments may be hidden, since they cannot have side effects. default: - if m, cmd := match1(rest, reShellToken); m { + tokens, _ := splitIntoShellTokens(shline.line, rest) + if len(tokens) > 0 { + cmd := tokens[0] switch cmd { case "${DELAYED_ERROR_MSG}", "${DELAYED_WARNING_MSG}", "${DO_NADA}", @@ -525,28 +418,48 @@ func (shline *ShellLine) checkHiddenAndSuppress(hiddenAndSuppress, rest string) } } -func (ctx *ShelltextContext) checkCommandStart() { +type SimpleCommandChecker struct { + shline *ShellLine + cmd *MkShSimpleCommand + strcmd *StrCommand +} + +func NewSimpleCommandChecker(shline *ShellLine, cmd *MkShSimpleCommand) *SimpleCommandChecker { + strcmd := NewStrCommand(cmd) + return &SimpleCommandChecker{shline, cmd, strcmd} + +} + +func (c *SimpleCommandChecker) Check() { if G.opts.Debug { - defer tracecall2(ctx.state.String(), ctx.shellword)() + defer tracecall(c.strcmd)() } - state, shellword := ctx.state, ctx.shellword - if state != scstStart && state != scstCond { - return + c.checkCommandStart() + c.checkAbsolutePathnames() + c.checkAutoMkdirs() + c.checkInstallMulti() + c.checkPaxPe() + c.checkEchoN() +} + +func (ctx *SimpleCommandChecker) checkCommandStart() { + if G.opts.Debug { + defer tracecall()() } + shellword := ctx.strcmd.Name switch { - case shellword == "${RUN}": + case shellword == "${RUN}" || shellword == "": case ctx.handleForbiddenCommand(): case ctx.handleTool(): case ctx.handleCommandVariable(): - case matches(shellword, `^(?:\$\$\(|\(|\)|:|;|;;|&&|\|\||\{|\}|break|case|cd|continue|do|done|elif|else|esac|eval|exec|exit|export|fi|for|if|read|set|shift|then|umask|unset|while)$`): - case matches(shellword, `^\w+=`): // Variable assignment + case matches(shellword, `^(?::|break|cd|continue|eval|exec|exit|export|read|set|shift|umask|unset)$`): case hasPrefix(shellword, "./"): // All commands from the current directory are fine. case hasPrefix(shellword, "${PKGSRCDIR"): // With or without the :Q modifier case ctx.handleComment(): default: - if G.opts.WarnExtra { + if G.opts.WarnExtra && !(G.Mk != nil && G.Mk.indentation.DependsOn("OPSYS")) { ctx.shline.line.Warn1("Unknown shell command %q.", shellword) Explain3( "If you want your package to be portable to all platforms that pkgsrc", @@ -556,12 +469,12 @@ func (ctx *ShelltextContext) checkCommandStart() { } } -func (ctx *ShelltextContext) handleTool() bool { +func (ctx *SimpleCommandChecker) handleTool() bool { if G.opts.Debug { - defer tracecall1(ctx.shellword)() + defer tracecall()() } - shellword := ctx.shellword + shellword := ctx.strcmd.Name tool := G.globalData.Tools.byName[shellword] if tool == nil { return false @@ -579,10 +492,15 @@ func (ctx *ShelltextContext) handleTool() bool { return true } -func (ctx *ShelltextContext) handleForbiddenCommand() bool { - switch path.Base(ctx.shellword) { +func (ctx *SimpleCommandChecker) handleForbiddenCommand() bool { + if G.opts.Debug { + defer tracecall()() + } + + shellword := ctx.strcmd.Name + switch path.Base(shellword) { case "ktrace", "mktexlsr", "strace", "texconfig", "truss": - ctx.shline.line.Error1("%q must not be used in Makefiles.", ctx.shellword) + ctx.shline.line.Error1("%q must not be used in Makefiles.", shellword) Explain3( "This command must appear in INSTALL scripts, not in the package", "Makefile, so that the package also works if it is installed as a binary", @@ -592,12 +510,12 @@ func (ctx *ShelltextContext) handleForbiddenCommand() bool { return false } -func (ctx *ShelltextContext) handleCommandVariable() bool { +func (ctx *SimpleCommandChecker) handleCommandVariable() bool { if G.opts.Debug { - defer tracecall1(ctx.shellword)() + defer tracecall()() } - shellword := ctx.shellword + shellword := ctx.strcmd.Name if m, varname := match1(shellword, `^\$\{([\w_]+)\}$`); m { if tool := G.globalData.Tools.byVarname[varname]; tool != nil { @@ -622,12 +540,16 @@ func (ctx *ShelltextContext) handleCommandVariable() bool { return false } -func (ctx *ShelltextContext) handleComment() bool { +func (ctx *SimpleCommandChecker) handleComment() bool { if G.opts.Debug { - defer tracecall1(ctx.shellword)() + defer tracecall()() + } + + shellword := ctx.strcmd.Name + if G.opts.Debug { + defer tracecall1(shellword)() } - shellword := ctx.shellword if !hasPrefix(shellword, "#") { return false } @@ -660,91 +582,188 @@ func (ctx *ShelltextContext) handleComment() bool { return true } -func (ctx *ShelltextContext) checkConditionalCd() { - if ctx.state == scstCond && ctx.shellword == "cd" { - ctx.shline.line.Error0("The Solaris /bin/sh cannot handle \"cd\" inside conditionals.") - Explain3( - "When the Solaris shell is in \"set -e\" mode and \"cd\" fails, the", - "shell will exit, no matter if it is protected by an \"if\" or the", - "\"||\" operator.") - } +type ShellProgramChecker struct { + shline *ShellLine } -func (ctx *ShelltextContext) checkAutoMkdirs() { - state, shellword := ctx.state, ctx.shellword +func (c *ShellProgramChecker) checkConditionalCd(list *MkShList) { + if G.opts.Debug { + defer tracecall()() + } - line := ctx.shline.line - if (state == scstInstallD || state == scstMkdir) && matches(shellword, `^(?:\$\{DESTDIR\})?\$\{PREFIX(?:|:Q)\}/`) { - line.Warn1("Please use AUTO_MKDIRS instead of %q.", - ifelseStr(state == scstMkdir, "${MKDIR}", "${INSTALL} -d")) - Explain4( - "Setting AUTO_MKDIRS=yes automatically creates all directories that", - "are mentioned in the PLIST. If you need additional directories,", - "specify them in INSTALLATION_DIRS, which is a list of directories", - "relative to ${PREFIX}.") + getSimple := func(list *MkShList) *MkShSimpleCommand { + if len(list.AndOrs) == 1 { + if len(list.AndOrs[0].Pipes) == 1 { + if len(list.AndOrs[0].Pipes[0].Cmds) == 1 { + return list.AndOrs[0].Pipes[0].Cmds[0].Simple + } + } + } + return nil } - if (state == scstInstallDir || state == scstInstallDir2) && !contains(shellword, "$$") { - if m, dirname := match1(shellword, `^(?:\$\{DESTDIR\})?\$\{PREFIX(?:|:Q)\}/(.*)`); m { - line.Note1("You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= %s\" instead of this command.", dirname) - Explain( - "Many packages include a list of all needed directories in their", - "PLIST file. In such a case, you can just set AUTO_MKDIRS=yes and", - "be done. The pkgsrc infrastructure will then create all directories", - "in advance.", - "", - "To create directories that are not mentioned in the PLIST file, it", - "is easier to just list them in INSTALLATION_DIRS than to execute the", - "commands explicitly. That way, you don't have to think about which", - "of the many INSTALL_*_DIR variables is appropriate, since", - "INSTALLATION_DIRS takes care of that.") + checkConditionalCd := func(cmd *MkShSimpleCommand) { + if NewStrCommand(cmd).Name == "cd" { + c.shline.line.Error0("The Solaris /bin/sh cannot handle \"cd\" inside conditionals.") + Explain3( + "When the Solaris shell is in \"set -e\" mode and \"cd\" fails, the", + "shell will exit, no matter if it is protected by an \"if\" or the", + "\"||\" operator.") } } + + (*MkShWalker).Walk(nil, list, func(node interface{}) { + if cmd, ok := node.(*MkShIfClause); ok { + for _, cond := range cmd.Conds { + if simple := getSimple(cond); simple != nil { + checkConditionalCd(simple) + } + } + } + if cmd, ok := node.(*MkShLoopClause); ok { + if simple := getSimple(cmd.Cond); simple != nil { + checkConditionalCd(simple) + } + } + }) } -func (ctx *ShelltextContext) checkInstallMulti() { - if ctx.state == scstInstallDir2 && hasPrefix(ctx.shellword, "$") { - line := ctx.shline.line - line.Warn0("The INSTALL_*_DIR commands can only handle one directory at a time.") - Explain2( - "Many implementations of install(1) can handle more, but pkgsrc aims", - "at maximum portability.") +func (c *ShellProgramChecker) checkWords(words []*ShToken, checkQuoting bool) { + if G.opts.Debug { + defer tracecall()() + } + + for _, word := range words { + c.checkWord(word, checkQuoting) } } -func (ctx *ShelltextContext) checkPaxPe() { - if ctx.state == scstPax && ctx.shellword == "-pe" { - line := ctx.shline.line - line.Warn0("Please use the -pp option to pax(1) instead of -pe.") - Explain3( - "The -pe option tells pax to preserve the ownership of the files, which", - "means that the installed files will belong to the user that has built", - "the package.") +func (c *ShellProgramChecker) checkWord(word *ShToken, checkQuoting bool) { + if G.opts.Debug { + defer tracecall(word.MkText)() } + + c.shline.CheckWord(word.MkText, checkQuoting) } -func (ctx *ShelltextContext) checkQuoteSubstitution() { - if ctx.state == scstPaxS || ctx.state == scstSedE { - if false && !matches(ctx.shellword, `"^[\"\'].*[\"\']$`) { - line := ctx.shline.line - line.Warn1("Substitution commands like %q should always be quoted.", ctx.shellword) +func (c *SimpleCommandChecker) checkAbsolutePathnames() { + if G.opts.Debug { + defer tracecall()() + } + + cmdname := c.strcmd.Name + isSubst := false + for _, arg := range c.strcmd.Args { + if !isSubst { + c.shline.line.CheckAbsolutePathname(arg) + } + if false && isSubst && !matches(arg, `"^[\"\'].*[\"\']$`) { + c.shline.line.Warn1("Substitution commands like %q should always be quoted.", arg) Explain3( "Usually these substitution commands contain characters like '*' or", "other shell metacharacters that might lead to lookup of matching", "filenames and then expand to more than one word.") } + isSubst = cmdname == "${PAX}" && arg == "-s" || cmdname == "${SED}" && arg == "-e" } } -func (ctx *ShelltextContext) checkEchoN() { - if ctx.state == scstEcho && ctx.shellword == "-n" { +func (ctx *SimpleCommandChecker) checkAutoMkdirs() { + if G.opts.Debug { + defer tracecall()() + } + + cmdname := ctx.strcmd.Name + switch { + case cmdname == "${MKDIR}": + break + case cmdname == "${INSTALL}" && ctx.strcmd.HasOption("-d"): + cmdname = "${INSTALL} -d" + case matches(cmdname, `^\$\{INSTALL_.*_DIR\}$`): + break + default: + return + } + + for _, arg := range ctx.strcmd.Args { + if !contains(arg, "$$") && !matches(arg, `\$\{[_.]*[a-z]`) { + if m, dirname := match1(arg, `^(?:\$\{DESTDIR\})?\$\{PREFIX(?:|:Q)\}/(.*)`); m { + ctx.shline.line.Note2("You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= %s\" instead of %q.", dirname, cmdname) + Explain( + "Many packages include a list of all needed directories in their", + "PLIST file. In such a case, you can just set AUTO_MKDIRS=yes and", + "be done. The pkgsrc infrastructure will then create all directories", + "in advance.", + "", + "To create directories that are not mentioned in the PLIST file, it", + "is easier to just list them in INSTALLATION_DIRS than to execute the", + "commands explicitly. That way, you don't have to think about which", + "of the many INSTALL_*_DIR variables is appropriate, since", + "INSTALLATION_DIRS takes care of that.") + } + } + } +} + +func (ctx *SimpleCommandChecker) checkInstallMulti() { + if G.opts.Debug { + defer tracecall()() + } + + cmd := ctx.strcmd + + if hasPrefix(cmd.Name, "${INSTALL_") && hasSuffix(cmd.Name, "_DIR}") { + prevdir := "" + for i, arg := range cmd.Args { + switch { + case hasPrefix(arg, "-"): + break + case i > 0 && (cmd.Args[i-1] == "-m" || cmd.Args[i-1] == "-o" || cmd.Args[i-1] == "-g"): + break + default: + if prevdir != "" { + ctx.shline.line.Warn0("The INSTALL_*_DIR commands can only handle one directory at a time.") + Explain2( + "Many implementations of install(1) can handle more, but pkgsrc aims", + "at maximum portability.") + return + } + prevdir = arg + } + } + } +} + +func (ctx *SimpleCommandChecker) checkPaxPe() { + if G.opts.Debug { + defer tracecall()() + } + + if ctx.strcmd.Name == "${PAX}" && ctx.strcmd.HasOption("-pe") { + ctx.shline.line.Warn0("Please use the -pp option to pax(1) instead of -pe.") + Explain3( + "The -pe option tells pax to preserve the ownership of the files, which", + "means that the installed files will belong to the user that has built", + "the package.") + } +} + +func (ctx *SimpleCommandChecker) checkEchoN() { + if G.opts.Debug { + defer tracecall()() + } + + if ctx.strcmd.Name == "${ECHO}" && ctx.strcmd.HasOption("-n") { ctx.shline.line.Warn0("Please use ${ECHO_N} instead of \"echo -n\".") } } -func (ctx *ShelltextContext) checkPipeExitcode() { - if G.opts.WarnExtra && ctx.state != scstCaseLabelCont && ctx.shellword == "|" { - line := ctx.shline.line +func (ctx *ShellProgramChecker) checkPipeExitcode(line *Line, pipeline *MkShPipeline) { + if G.opts.Debug { + defer tracecall()() + } + + if G.opts.WarnExtra && len(pipeline.Cmds) > 1 { line.Warn0("The exitcode of the left-hand-side command of the pipe operator is ignored.") Explain( "In a shell command like \"cat *.txt | grep keyword\", if the command", @@ -755,10 +774,15 @@ func (ctx *ShelltextContext) checkPipeExitcode() { } } -func (ctx *ShelltextContext) checkSetE(eflag *bool, prevToken string) { - if G.opts.WarnExtra && ctx.shellword == ";" && ctx.state != scstCondCont && ctx.state != scstForCont && !*eflag { +func (ctx *ShellProgramChecker) checkSetE(list *MkShList, eflag *bool) { + if G.opts.Debug { + defer tracecall()() + } + + // Disabled until the shell parser can recognize "command || exit 1" reliably. + if false && G.opts.WarnExtra && !*eflag && "the current token" == ";" { *eflag = true - ctx.shline.line.Warn1("Please switch to \"set -e\" mode before using a semicolon (the one after %q) to separate commands.", prevToken) + ctx.shline.line.Warn1("Please switch to \"set -e\" mode before using a semicolon (the one after %q) to separate commands.", "previous token") Explain( "Normally, when a shell command fails (returns non-zero), the", "remaining commands are still executed. For example, the following", @@ -777,6 +801,10 @@ func (ctx *ShelltextContext) checkSetE(eflag *bool, prevToken string) { // Some shell commands should not be used in the install phase. func (shline *ShellLine) checkCommandUse(shellcmd string) { + if G.opts.Debug { + defer tracecall()() + } + if G.Mk == nil || !matches(G.Mk.target, `^(?:pre|do|post)-install$`) { return } @@ -815,113 +843,6 @@ func (shline *ShellLine) checkCommandUse(shellcmd string) { } } -func (shline *ShellLine) nextState(state scState, shellword string) scState { - switch { - case shellword == ";;": - return scstCaseLabel - case state == scstCaseLabelCont && shellword == "|": - return scstCaseLabel - case matches(shellword, `^[;&\|]+$`): - return scstStart - case state == scstStart: - switch shellword { - case "${INSTALL}": - return scstInstall - case "${MKDIR}": - return scstMkdir - case "${PAX}": - return scstPax - case "${SED}": - return scstSed - case "${ECHO}", "echo": - return scstEcho - case "${RUN}", "then", "else", "do", "(": - return scstStart - case "set": - return scstSet - case "if", "elif", "while": - return scstCond - case "case": - return scstCase - case "for": - return scstFor - default: - switch { - case matches(shellword, `^\$\{INSTALL_[A-Z]+_DIR\}$`): - return scstInstallDir - case matches(shellword, reShVarassign): - return scstStart - default: - return scstCont - } - } - case state == scstMkdir: - return scstMkdir - case state == scstInstall && shellword == "-d": - return scstInstallD - case state == scstInstall, state == scstInstallD: - if matches(shellword, `^-[ogm]$`) { - return scstCont // XXX: why not keep the state? - } - return state - case state == scstInstallDir && hasPrefix(shellword, "-"): - return scstCont - case state == scstInstallDir && hasPrefix(shellword, "$"): - return scstInstallDir2 - case state == scstInstallDir || state == scstInstallDir2: - return state - case state == scstPax && shellword == "-s": - return scstPaxS - case state == scstPax && hasPrefix(shellword, "-"): - return scstPax - case state == scstPax: - return scstCont - case state == scstPaxS: - return scstPax - case state == scstSed && shellword == "-e": - return scstSedE - case state == scstSed && hasPrefix(shellword, "-"): - return scstSed - case state == scstSed: - return scstCont - case state == scstSedE: - return scstSed - case state == scstSet: - return scstSetCont - case state == scstSetCont: - return scstSetCont - case state == scstCase: - return scstCaseIn - case state == scstCaseIn && shellword == "in": - return scstCaseLabel - case state == scstCaseLabel && shellword == "esac": - return scstCont - case state == scstCaseLabel: - return scstCaseLabelCont - case state == scstCaseLabelCont && shellword == ")": - return scstStart - case state == scstCont: - return scstCont - case state == scstCond: - return scstCondCont - case state == scstCondCont: - return scstCondCont - case state == scstFor: - return scstForIn - case state == scstForIn && shellword == "in": - return scstForCont - case state == scstForCont: - return scstForCont - case state == scstEcho: - return scstCont - default: - if G.opts.Debug { - traceStep("Internal pkglint error: shellword.nextState state=%s shellword=%q", state, shellword) - } - return scstStart - } -} - // Example: "word1 word2;;;" => "word1", "word2", ";;", ";" func splitIntoShellTokens(line *Line, text string) (tokens []string, rest string) { if G.opts.Debug { @@ -943,10 +864,10 @@ func splitIntoShellTokens(line *Line, text string) (tokens []string, rest string if atom.Type == shtSpace && q == shqPlain { emit() } else if atom.Type == shtWord || atom.Type == shtVaruse || atom.Quoting != shqPlain { - word += atom.Text + word += atom.MkText } else { emit() - tokens = append(tokens, atom.Text) + tokens = append(tokens, atom.MkText) } } emit() @@ -968,7 +889,7 @@ func splitIntoMkWords(line *Line, text string) (words []string, rest string) { words = append(words, word) word = "" } else { - word += atom.Text + word += atom.MkText } } if word != "" && atoms[len(atoms)-1].Quoting == shqPlain { diff --git a/pkgtools/pkglint/files/shell.y b/pkgtools/pkglint/files/shell.y new file mode 100644 index 00000000000..45080ca1c62 --- /dev/null +++ b/pkgtools/pkglint/files/shell.y @@ -0,0 +1,422 @@ +%{ +package main +%} + +%token tkWORD +%token tkASSIGNMENT_WORD +%token tkNEWLINE +%token tkIO_NUMBER +%token tkBACKGROUND +%token tkPIPE tkSEMI +%token tkAND tkOR tkSEMISEMI +%token tkLT tkGT tkLTLT tkGTGT tkLTAND tkGTAND tkLTGT tkLTLTDASH tkGTPIPE +%token tkIF tkTHEN tkELSE tkELIF tkFI tkDO tkDONE +%token tkCASE tkESAC tkWHILE tkUNTIL tkFOR +%token tkLPAREN tkRPAREN tkLBRACE tkRBRACE tkEXCLAM +%token tkIN + +%union { + IONum int + List *MkShList + AndOr *MkShAndOr + Pipeline *MkShPipeline + Command *MkShCommand + CompoundCommand *MkShCompoundCommand + Separator MkShSeparator + Simple *MkShSimpleCommand + FuncDef *MkShFunctionDefinition + For *MkShForClause + If *MkShIfClause + Case *MkShCaseClause + CaseItem *MkShCaseItem + Loop *MkShLoopClause + Words []*ShToken + Word *ShToken + Redirections []*MkShRedirection + Redirection *MkShRedirection +} + +%type start program compound_list brace_group subshell term do_group +%type and_or +%type pipeline pipe_sequence +%type command +%type compound_command +%type separator separator_op sequential_sep +%type simple_command cmd_prefix cmd_suffix +%type function_definition +%type for_clause +%type if_clause else_part +%type case_clause case_list case_list_ns +%type case_item case_item_ns +%type while_clause until_clause +%type wordlist case_selector pattern +%type filename cmd_word here_end +%type redirect_list +%type io_redirect io_file io_here + +%% + +start : program { + shyylex.(*ShellLexer).result = $$ +} + +program : compound_list { + $$ = $1 +} +program : /* empty */ { + $$ = &MkShList{} +} + +and_or : pipeline { + $$ = NewMkShAndOr($1) +} +and_or : and_or tkAND linebreak pipeline { + $$.Add("&&", $4) +} +and_or : and_or tkOR linebreak pipeline { + $$.Add("||", $4) +} + +pipeline : pipe_sequence { + /* empty */ +} +pipeline : tkEXCLAM pipe_sequence { + $$ = $2 + $$.Negated = true +} + +pipe_sequence : command { + $$ = NewMkShPipeline(false, $1) +} +pipe_sequence : pipe_sequence tkPIPE linebreak command { + $$.Add($4) +} + +command : simple_command { + $$ = &MkShCommand{Simple: $1} +} +command : compound_command { + $$ = &MkShCommand{Compound: $1} +} +command : compound_command redirect_list { + $$ = &MkShCommand{Compound: $1, Redirects: $2} +} +command : function_definition { + $$ = &MkShCommand{FuncDef: $1} +} +command : function_definition redirect_list { + $$ = &MkShCommand{FuncDef: $1, Redirects: $2} +} + +compound_command : brace_group { + $$ = &MkShCompoundCommand{Brace: $1} +} +compound_command : subshell { + $$ = &MkShCompoundCommand{Subshell: $1} +} +compound_command : for_clause { + $$ = &MkShCompoundCommand{For: $1} +} +compound_command : case_clause { + $$ = &MkShCompoundCommand{Case: $1} +} +compound_command : if_clause { + $$ = &MkShCompoundCommand{If: $1} +} +compound_command : while_clause { + $$ = &MkShCompoundCommand{Loop: $1} +} +compound_command : until_clause { + $$ = &MkShCompoundCommand{Loop: $1} +} + +subshell : tkLPAREN compound_list tkRPAREN { + $$ = $2 +} + +compound_list : linebreak term { + $$ = $2 +} +compound_list : linebreak term separator { + $$ = $2 + $$.AddSeparator($3) +} + +term : and_or { + $$ = NewMkShList() + $$.AddAndOr($1) +} +term : term separator and_or { + $$.AddSeparator($2) + $$.AddAndOr($3) +} + +for_clause : tkFOR tkWORD linebreak do_group { + args := NewShToken("\"$$@\"", + NewShAtom(shtWord, "\"",shqDquot), + NewShAtom(shtWord, "$$@",shqDquot), + NewShAtom(shtWord,"\"",shqPlain)) + $$ = &MkShForClause{$2.MkText, []*ShToken{args}, $4} +} +for_clause : tkFOR tkWORD linebreak tkIN sequential_sep do_group { + $$ = &MkShForClause{$2.MkText, nil, $6} +} +for_clause : tkFOR tkWORD linebreak tkIN wordlist sequential_sep do_group { + $$ = &MkShForClause{$2.MkText, $5, $7} +} + +wordlist : tkWORD { + $$ = append($$, $1) +} +wordlist : wordlist tkWORD { + $$ = append($$, $2) +} + +case_clause : tkCASE tkWORD linebreak tkIN linebreak case_list tkESAC { + $$ = $6 + $$.Word = $2 +} +case_clause : tkCASE tkWORD linebreak tkIN linebreak case_list_ns tkESAC { + $$ = $6 + $$.Word = $2 +} +case_clause : tkCASE tkWORD linebreak tkIN linebreak tkESAC { + $$ = &MkShCaseClause{$2, nil} +} + +case_list_ns : case_item_ns { + $$ = &MkShCaseClause{nil, nil} + $$.Cases = append($$.Cases, $1) +} +case_list_ns : case_list case_item_ns { + $$.Cases = append($$.Cases, $2) +} + +case_list : case_item { + $$ = &MkShCaseClause{nil, nil} + $$.Cases = append($$.Cases, $1) +} +case_list : case_list case_item { + $$.Cases = append($$.Cases, $2) +} + +case_selector : tkLPAREN pattern tkRPAREN { + $$ = $2 +} +case_selector : pattern tkRPAREN { + /* empty */ +} + +case_item_ns : case_selector linebreak { + $$ = &MkShCaseItem{$1, &MkShList{}, nil} +} +case_item_ns : case_selector linebreak term linebreak { + $$ = &MkShCaseItem{$1, $3, nil} +} +case_item_ns : case_selector linebreak term separator_op linebreak { + $$ = &MkShCaseItem{$1, $3, &$4} +} + +case_item : case_selector linebreak tkSEMISEMI linebreak { + $$ = &MkShCaseItem{$1, &MkShList{}, nil} +} +case_item : case_selector compound_list tkSEMISEMI linebreak { + $$ = &MkShCaseItem{$1, $2, nil} +} + +pattern : tkWORD { + $$ = nil + $$ = append($$, $1) +} +pattern : pattern tkPIPE tkWORD { + $$ = append($$, $3) +} + +if_clause : tkIF compound_list tkTHEN compound_list else_part tkFI { + $$ = $5 + $$.Prepend($2, $4) +} +if_clause : tkIF compound_list tkTHEN compound_list tkFI { + $$ = &MkShIfClause{} + $$.Prepend($2, $4) +} + +else_part : tkELIF compound_list tkTHEN compound_list { + $$ = &MkShIfClause{} + $$.Prepend($2, $4) +} +else_part : tkELIF compound_list tkTHEN compound_list else_part { + $$ = $5 + $$.Prepend($2, $4) +} +else_part : tkELSE compound_list { + $$ = &MkShIfClause{nil, nil, $2} +} + +while_clause : tkWHILE compound_list do_group { + $$ = &MkShLoopClause{$2, $3, false} +} +until_clause : tkUNTIL compound_list do_group { + $$ = &MkShLoopClause{$2, $3, true} +} + +function_definition : tkWORD tkLPAREN tkRPAREN linebreak compound_command { /* Apply rule 9 */ + $$ = &MkShFunctionDefinition{$1.MkText, $5} +} + +brace_group : tkLBRACE compound_list tkRBRACE { + $$ = $2 +} + +do_group : tkDO compound_list tkDONE { + $$ = $2 +} + +simple_command : cmd_prefix cmd_word cmd_suffix { + $$.Name = $2 + $$.Args = append($$.Args, $3.Args...) + $$.Redirections = append($$.Redirections, $3.Redirections...) +} +simple_command : cmd_prefix cmd_word { + $$.Name = $2 +} +simple_command : cmd_prefix { + /* empty */ +} +simple_command : tkWORD cmd_suffix { + $$ = $2 + $$.Name = $1 +} +simple_command : tkWORD { + $$ = &MkShSimpleCommand{Name: $1} +} + +cmd_word : tkWORD { /* Apply rule 7b */ + /* empty */ +} + +cmd_prefix : io_redirect { + $$ = &MkShSimpleCommand{} + $$.Redirections = append($$.Redirections, $1) +} +cmd_prefix : tkASSIGNMENT_WORD { + $$ = &MkShSimpleCommand{} + $$.Assignments = append($$.Assignments, $1) +} +cmd_prefix : cmd_prefix io_redirect { + $$.Redirections = append($$.Redirections, $2) +} +cmd_prefix : cmd_prefix tkASSIGNMENT_WORD { + $$.Assignments = append($$.Assignments, $2) +} + +cmd_suffix : io_redirect { + $$ = &MkShSimpleCommand{} + $$.Redirections = append($$.Redirections, $1) +} +cmd_suffix : tkWORD { + $$ = &MkShSimpleCommand{} + $$.Args = append($$.Args, $1) +} +cmd_suffix : cmd_suffix io_redirect { + $$.Redirections = append($$.Redirections, $2) +} +cmd_suffix : cmd_suffix tkWORD { + $$.Args = append($$.Args, $2) +} + +redirect_list : io_redirect { + $$ = nil + $$ = append($$, $1) +} +redirect_list : redirect_list io_redirect { + $$ = append($$, $2) +} + +io_redirect : io_file { + /* empty */ +} +io_redirect : tkIO_NUMBER io_file { + $$ = $2 + $$.Fd = $1 +} + +io_redirect : io_here { + /* empty */ +} +io_redirect : tkIO_NUMBER io_here { + $$ = $2 + $$.Fd = $1 +} + +io_file : tkLT filename { + $$ = &MkShRedirection{-1, "<", $2} +} +io_file : tkLTAND filename { + $$ = &MkShRedirection{-1, "<&", $2} +} +io_file : tkGT filename { + $$ = &MkShRedirection{-1, ">", $2} +} +io_file : tkGTAND filename { + $$ = &MkShRedirection{-1, ">&", $2} +} +io_file : tkGTGT filename { + $$ = &MkShRedirection{-1, ">>", $2} +} +io_file : tkLTGT filename { + $$ = &MkShRedirection{-1, "<>", $2} +} +io_file : tkGTPIPE filename { + $$ = &MkShRedirection{-1, ">|", $2} +} + +filename : tkWORD { /* Apply rule 2 */ + /* empty */ +} + +io_here : tkLTLT here_end { + $$ = &MkShRedirection{-1, "<<", $2} +} +io_here : tkLTLTDASH here_end { + $$ = &MkShRedirection{-1, "<<-", $2} +} + +here_end : tkWORD { /* Apply rule 3 */ + /* empty */ +} + +newline_list : tkNEWLINE { + /* empty */ +} +newline_list : newline_list tkNEWLINE { + /* empty */ +} + +linebreak : newline_list { + /* empty */ +} +linebreak : /* empty */ { + /* empty */ +} + +separator_op : tkBACKGROUND { + $$ = "&" +} +separator_op : tkSEMI { + $$ = ";" +} + +separator : separator_op linebreak { + /* empty */ +} +separator : newline_list { + $$ = "\n" +} + +sequential_sep : tkSEMI linebreak { + $$ = ";" +} +sequential_sep : tkNEWLINE linebreak { + $$ = "\n" +} diff --git a/pkgtools/pkglint/files/shell_test.go b/pkgtools/pkglint/files/shell_test.go index f6a94a7c8f8..c4993e40940 100644 --- a/pkgtools/pkglint/files/shell_test.go +++ b/pkgtools/pkglint/files/shell_test.go @@ -6,23 +6,6 @@ import ( check "gopkg.in/check.v1" ) -func (s *Suite) TestReShellToken(c *check.C) { - re := `^(?:` + reShellToken + `)$` - matches := check.NotNil - doesntMatch := check.IsNil - - c.Check(match("", re), doesntMatch) - c.Check(match("$var", re), matches) - c.Check(match("$var$var", re), matches) - c.Check(match("$var;;", re), doesntMatch) // More than one token - c.Check(match("'single-quoted'", re), matches) - c.Check(match("\"", re), doesntMatch) // Incomplete string - c.Check(match("'...'\"...\"", re), matches) // Mixed strings - c.Check(match("\"...\"", re), matches) - c.Check(match("`cat file`", re), matches) - c.Check(match("${file%.c}.o", re), matches) -} - func (s *Suite) Test_SplitIntoShellTokens_LineContinuation(c *check.C) { words, rest := splitIntoShellTokens(dummyLine, "if true; then \\") @@ -94,6 +77,32 @@ func (s *Suite) Test_SplitIntoMkWords_VaruseSpace(c *check.C) { c.Check(rest, equals, "") } +func (s *Suite) Test_splitIntoShellTokens_Redirect(c *check.C) { + words, rest := splitIntoShellTokens(dummyLine, "echo 1>output 2>>append 3>|clobber 4>&5 6>append") + + c.Check(words, deepEquals, []string{ + "echo", + "1>", "output", + "2>>", "append", + "3>|", "clobber", + "4>&", "5", + "6<", "input", + ">>", "append"}) + c.Check(rest, equals, "") + + words, rest = splitIntoShellTokens(dummyLine, "echo 1> output 2>> append 3>| clobber 4>& 5 6< input >> append") + + c.Check(words, deepEquals, []string{ + "echo", + "1>", "output", + "2>>", "append", + "3>|", "clobber", + "4>&", "5", + "6<", "input", + ">>", "append"}) + c.Check(rest, equals, "") +} + func (s *Suite) TestChecklineMkShellCommandLine(c *check.C) { s.UseCommandLine(c, "-Wall") G.Mk = s.NewMkLines("fname", @@ -108,9 +117,7 @@ func (s *Suite) TestChecklineMkShellCommandLine(c *check.C) { c.Check(s.Output(), equals, ""+ "WARN: fname:1: Unknown shell command \"uname\".\n"+ - "WARN: fname:1: Please switch to \"set -e\" mode before using a semicolon (the one after \"uname=`uname`\") to separate commands.\n"+ "WARN: fname:1: Unknown shell command \"echo\".\n"+ - "WARN: fname:1: Unquoted shell variable \"uname\".\n"+ "WARN: fname:1: Unknown shell command \"echo\".\n") s.RegisterTool(&Tool{Name: "echo", Predefined: true}) @@ -137,6 +144,12 @@ func (s *Suite) TestChecklineMkShellCommandLine(c *check.C) { "WARN: fname:1: COMMENT may not be used in any file; it is a write-only variable.\n"+ "WARN: fname:1: Please move ${COMMENT:Q} outside of any quoting characters.\n") + shline.CheckShellCommandLine("echo target=$@ exitcode=$$? '$$' \"\\$$\"") + + c.Check(s.Output(), equals, ""+ + "WARN: fname:1: Please use \"${.TARGET}\" instead of \"$@\".\n"+ + "WARN: fname:1: The $? shell variable is often not available in \"set -e\" mode.\n") + shline.CheckShellCommandLine("echo $$@") c.Check(s.Output(), equals, "WARN: fname:1: The $@ shell variable should only be used in double quotes.\n") @@ -145,9 +158,9 @@ func (s *Suite) TestChecklineMkShellCommandLine(c *check.C) { c.Check(s.Output(), equals, ""+ "WARN: fname:1: Pkglint parse error in ShTokenizer.ShAtom at \"$$\\\"\" (quoting=d)\n"+ - "WARN: fname:1: Pkglint parse error in ShellLine.CheckShellCommand at \"$$\\\"\" (state=start)\n") + "WARN: fname:1: Pkglint ShellLine.CheckShellCommand: parse error at [\"]\n") - shline.CheckShellCommandLine("echo \"\\n\"") // As seen by make(1); the shell sees: echo "\n" + shline.CheckShellCommandLine("echo \"\\n\"") c.Check(s.Output(), equals, "") @@ -163,8 +176,8 @@ func (s *Suite) TestChecklineMkShellCommandLine(c *check.C) { shline.CheckShellCommandLine("${RUN} subdir=\"`unzip -c \"$$e\" install.rdf | awk '/re/ { print \"hello\" }'`\"") c.Check(s.Output(), equals, ""+ - "WARN: fname:1: Unknown shell command \"unzip\".\n"+ "WARN: fname:1: The exitcode of the left-hand-side command of the pipe operator is ignored.\n"+ + "WARN: fname:1: Unknown shell command \"unzip\".\n"+ "WARN: fname:1: Unknown shell command \"awk\".\n") // From mail/thunderbird/Makefile, rev. 1.159 @@ -178,13 +191,12 @@ func (s *Suite) TestChecklineMkShellCommandLine(c *check.C) { c.Check(s.Output(), equals, ""+ "WARN: fname:1: XPI_FILES is used but not defined. Spelling mistake?\n"+ - "WARN: fname:1: UNZIP_CMD is used but not defined. Spelling mistake?\n"+ "WARN: fname:1: The exitcode of the left-hand-side command of the pipe operator is ignored.\n"+ + "WARN: fname:1: UNZIP_CMD is used but not defined. Spelling mistake?\n"+ "WARN: fname:1: Unknown shell command \"awk\".\n"+ - "WARN: fname:1: MKDIR is used but not defined. Spelling mistake?\n"+ "WARN: fname:1: Unknown shell command \"${MKDIR}\".\n"+ - "WARN: fname:1: UNZIP_CMD is used but not defined. Spelling mistake?\n"+ - "WARN: fname:1: Unquoted shell variable \"e\".\n") + "WARN: fname:1: MKDIR is used but not defined. Spelling mistake?\n"+ + "WARN: fname:1: UNZIP_CMD is used but not defined. Spelling mistake?\n") // From x11/wxGTK28/Makefile shline.CheckShellCommandLine("" + @@ -207,7 +219,14 @@ func (s *Suite) TestChecklineMkShellCommandLine(c *check.C) { shline.CheckShellCommandLine("${RUN} ${INSTALL_DATA_DIR} share/pkgbase ${PREFIX}/share/pkgbase") - c.Check(s.Output(), equals, "NOTE: fname:1: You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= share/pkgbase\" instead of this command.\n") + c.Check(s.Output(), equals, ""+ + "NOTE: fname:1: You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= share/pkgbase\" instead of \"${INSTALL_DATA_DIR}\".\n"+ + "WARN: fname:1: The INSTALL_*_DIR commands can only handle one directory at a time.\n") + + // See PR 46570, item "1. It does not" + shline.CheckShellCommandLine("for x in 1 2 3; do echo \"$$x\" || exit 1; done") + + c.Check(s.Output(), equals, "") // No warning about missing error checking. } func (s *Suite) TestShellLine_CheckShelltext_nofix(c *check.C) { @@ -271,6 +290,10 @@ func (s *Suite) TestShellLine_CheckShelltext_InternalError1(c *check.C) { c.Check(tokens, deepEquals, []string{text}) c.Check(rest, equals, "") + shline.CheckWord(text, false) + + c.Check(s.Output(), equals, "WARN: fname:1: Unknown shell command \"echo\".\n") + shline.CheckShellCommandLine(text) c.Check(s.Output(), equals, ""+ // No parse errors @@ -301,7 +324,7 @@ func (s *Suite) Test_ShellLine_CheckWord(c *check.C) { shline.CheckWord("${SED_FILE.${id}}", false) - c.Check(s.Output(), equals, "WARN: fname:1: SED_FILE.${id} is used but not defined. Spelling mistake?\n") + c.Check(s.Output(), equals, "") // No warning for variables that are partly indirect. shline.CheckWord("\"$@\"", false) @@ -436,8 +459,21 @@ func (s *Suite) TestShellLine_CheckShellCommandLine_InstallDirs(c *check.C) { shline.CheckShellCommandLine(shline.mkline.Shellcmd()) c.Check(s.Output(), equals, ""+ - "NOTE: Makefile:85: You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= dir1\" instead of this command.\n"+ - "NOTE: Makefile:85: You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= dir2\" instead of this command.\n"+ + "NOTE: Makefile:85: You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= dir1\" instead of \"${INSTALL_DATA_DIR}\".\n"+ + "NOTE: Makefile:85: You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= dir2\" instead of \"${INSTALL_DATA_DIR}\".\n"+ + "WARN: Makefile:85: The INSTALL_*_DIR commands can only handle one directory at a time.\n") + + shline.CheckShellCommandLine("${INSTALL_DATA_DIR} -d -m 0755 ${DESTDIR}${PREFIX}/share/examples/gdchart") + + // No warning about multiple directories, since 0755 is an option, not an argument. + c.Check(s.Output(), equals, ""+ + "NOTE: Makefile:85: You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= share/examples/gdchart\" instead of \"${INSTALL_DATA_DIR}\".\n") + + shline.CheckShellCommandLine("${INSTALL_DATA_DIR} -d -m 0755 ${DESTDIR}${PREFIX}/dir1 ${PREFIX}/dir2") + + c.Check(s.Output(), equals, ""+ + "NOTE: Makefile:85: You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= dir1\" instead of \"${INSTALL_DATA_DIR}\".\n"+ + "NOTE: Makefile:85: You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= dir2\" instead of \"${INSTALL_DATA_DIR}\".\n"+ "WARN: Makefile:85: The INSTALL_*_DIR commands can only handle one directory at a time.\n") } @@ -447,8 +483,8 @@ func (s *Suite) TestShellLine_CheckShellCommandLine_InstallD(c *check.C) { shline.CheckShellCommandLine(shline.mkline.Shellcmd()) c.Check(s.Output(), equals, ""+ - "WARN: Makefile:85: Please use AUTO_MKDIRS instead of \"${INSTALL} -d\".\n"+ - "WARN: Makefile:85: Please use AUTO_MKDIRS instead of \"${INSTALL} -d\".\n") + "NOTE: Makefile:85: You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= dir1\" instead of \"${INSTALL} -d\".\n"+ + "NOTE: Makefile:85: You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= dir2\" instead of \"${INSTALL} -d\".\n") } func (s *Suite) TestShellLine_(c *check.C) { diff --git a/pkgtools/pkglint/files/shtokenizer.go b/pkgtools/pkglint/files/shtokenizer.go index d21caab324b..68ee4557fc6 100644 --- a/pkgtools/pkglint/files/shtokenizer.go +++ b/pkgtools/pkglint/files/shtokenizer.go @@ -34,12 +34,16 @@ func (p *ShTokenizer) ShAtom(quoting ShQuoting) *ShAtom { atom = p.shAtomSquot() case shqBackt: atom = p.shAtomBackt() + case shqSubsh: + atom = p.shAtomSub() case shqDquotBackt: atom = p.shAtomDquotBackt() case shqBacktDquot: atom = p.shAtomBacktDquot() case shqBacktSquot: atom = p.shAtomBacktSquot() + case shqSubshSquot: + atom = p.shAtomSubshSquot() case shqDquotBacktDquot: atom = p.shAtomDquotBacktDquot() case shqDquotBacktSquot: @@ -59,6 +63,8 @@ func (p *ShTokenizer) shAtomPlain() *ShAtom { switch { case repl.AdvanceHspace(): return &ShAtom{shtSpace, repl.s, q, nil} + case repl.AdvanceStr("\n"): + return &ShAtom{shtNewline, repl.s, q, nil} case repl.AdvanceStr(";;"): return &ShAtom{shtCaseSeparator, repl.s, q, nil} case repl.AdvanceStr(";"): @@ -81,7 +87,7 @@ func (p *ShTokenizer) shAtomPlain() *ShAtom { return &ShAtom{shtWord, repl.s, shqSquot, nil} case repl.AdvanceStr("`"): return &ShAtom{shtWord, repl.s, shqBackt, nil} - case repl.AdvanceRegexp(`^(?:<|<<|>|>>|>&)`): + case repl.AdvanceRegexp(`^\d*(?:<<-|<<|<&|<>|>>|>&|>\||<|>)`): return &ShAtom{shtRedirect, repl.m[0], q, nil} case repl.AdvanceRegexp(`^#.*`): return &ShAtom{shtComment, repl.m[0], q, nil} @@ -155,6 +161,46 @@ func (p *ShTokenizer) shAtomBackt() *ShAtom { return nil } +func (p *ShTokenizer) shAtomSub() *ShAtom { + const q = shqSubsh + repl := p.parser.repl + mark := repl.Mark() + atom := func(typ ShAtomType) *ShAtom { + return NewShAtom(typ, repl.Since(mark), shqSubsh) + } + switch { + case repl.AdvanceHspace(): + return atom(shtSpace) + case repl.AdvanceStr(";;"): + return atom(shtCaseSeparator) + case repl.AdvanceStr(";"): + return atom(shtSemicolon) + case repl.AdvanceStr("||"): + return atom(shtOr) + case repl.AdvanceStr("&&"): + return atom(shtAnd) + case repl.AdvanceStr("|"): + return atom(shtPipe) + case repl.AdvanceStr("&"): + return atom(shtBackground) + case repl.AdvanceStr("\""): + //return &ShAtom{shtWord, repl.s, shqDquot, nil} + case repl.AdvanceStr("'"): + return &ShAtom{shtWord, repl.s, shqSubshSquot, nil} + case repl.AdvanceStr("`"): + //return &ShAtom{shtWord, repl.s, shqBackt, nil} + case repl.AdvanceRegexp(`^\d*(?:<<-|<<|<&|<>|>>|>&|>\||<|>)`): + return &ShAtom{shtRedirect, repl.m[0], q, nil} + case repl.AdvanceRegexp(`^#.*`): + return &ShAtom{shtComment, repl.m[0], q, nil} + case repl.AdvanceStr(")"): + return NewShAtom(shtWord, repl.s, shqPlain) + case repl.AdvanceRegexp(`^(?:[!#%*+,\-./0-9:=?@A-Z\[\]^_a-z{}~]+|\\[^$]|` + reShDollar + `)+`): + return &ShAtom{shtWord, repl.m[0], q, nil} + } + return nil +} + func (p *ShTokenizer) shAtomDquotBackt() *ShAtom { const q = shqDquotBackt repl := p.parser.repl @@ -216,6 +262,18 @@ func (p *ShTokenizer) shAtomBacktSquot() *ShAtom { return nil } +func (p *ShTokenizer) shAtomSubshSquot() *ShAtom { + const q = shqSubshSquot + repl := p.parser.repl + switch { + case repl.AdvanceStr("'"): + return &ShAtom{shtWord, repl.s, shqSubsh, nil} + case repl.AdvanceRegexp(`^([\t !"#%&()*+,\-./0-9:;<=>?@A-Z\[\\\]^_` + "`" + `a-z{|}~]+|\$\$)+`): + return &ShAtom{shtWord, repl.m[0], q, nil} + } + return nil +} + func (p *ShTokenizer) shAtomDquotBacktDquot() *ShAtom { const q = shqDquotBacktDquot repl := p.parser.repl @@ -281,7 +339,7 @@ func (p *ShTokenizer) ShToken() *ShToken { return nil } if atom := peek(); !atom.Type.IsWord() { - return NewShToken(atom.Text, atom) + return NewShToken(atom.MkText, atom) } nextatom: diff --git a/pkgtools/pkglint/files/shtokenizer_test.go b/pkgtools/pkglint/files/shtokenizer_test.go index c07bd03db6a..c2eef115eb1 100644 --- a/pkgtools/pkglint/files/shtokenizer_test.go +++ b/pkgtools/pkglint/files/shtokenizer_test.go @@ -15,9 +15,10 @@ func (s *Suite) Test_ShTokenizer_ShAtom(c *check.C) { } return p.Rest() } - check := func(s string, expected ...*ShAtom) { - rest := checkRest(s, expected...) + check := func(str string, expected ...*ShAtom) { + rest := checkRest(str, expected...) c.Check(rest, equals, "") + c.Check(s.Output(), equals, "") } token := func(typ ShAtomType, text string, quoting ShQuoting) *ShAtom { @@ -37,7 +38,7 @@ func (s *Suite) Test_ShTokenizer_ShAtom(c *check.C) { return &ShAtom{shtVaruse, text, shqPlain, varuse} } q := func(q ShQuoting, token *ShAtom) *ShAtom { - return &ShAtom{token.Type, token.Text, q, token.Data} + return &ShAtom{token.Type, token.MkText, q, token.Data} } whitespace := func(s string) *ShAtom { return token(shtSpace, s, shqPlain) } space := token(shtSpace, " ", shqPlain) @@ -302,6 +303,22 @@ func (s *Suite) Test_ShTokenizer_ShAtom(c *check.C) { word("then"), space, word("action2"), semicolon, space, word("else"), space, word("action3"), semicolon, space, word("fi")) + + if false { + check("$$(cat)", + token(shtWord, "$$(", shqSubsh), + token(shtWord, "cat", shqSubsh), + token(shtWord, ")", shqPlain)) + + check("$$(cat 'file')", + token(shtWord, "$$(", shqSubsh), + token(shtWord, "cat", shqSubsh), + token(shtSpace, " ", shqSubsh), + token(shtWord, "'", shqSubshSquot), + token(shtWord, "file", shqSubshSquot), + token(shtWord, "'", shqSubsh), + token(shtWord, ")", shqPlain)) + } } func (s *Suite) Test_Shtokenizer_ShAtom_Quoting(c *check.C) { @@ -314,7 +331,7 @@ func (s *Suite) Test_Shtokenizer_ShAtom_Quoting(c *check.C) { if token == nil { break } - result += token.Text + result += token.MkText if token.Quoting != q { q = token.Quoting result += "[" + q.String() + "]" diff --git a/pkgtools/pkglint/files/shtypes.go b/pkgtools/pkglint/files/shtypes.go index 08e9e63185d..b276919d434 100644 --- a/pkgtools/pkglint/files/shtypes.go +++ b/pkgtools/pkglint/files/shtypes.go @@ -23,6 +23,7 @@ const ( shtRedirect // >, <, >> shtComment // # ... shtSubshell // $$( + shtNewline // \n ) func (t ShAtomType) String() string { @@ -37,6 +38,7 @@ func (t ShAtomType) String() string { "or", "and", "redirect", "comment", + "newline", }[t] } @@ -50,7 +52,7 @@ func (t ShAtomType) IsWord() bool { func (t ShAtomType) IsCommandDelimiter() bool { switch t { - case shtSemicolon, shtPipe, shtBackground, shtAnd, shtOr, shtCaseSeparator: + case shtSemicolon, shtNewline, shtPipe, shtBackground, shtAnd, shtOr, shtCaseSeparator: return true } return false @@ -59,7 +61,7 @@ func (t ShAtomType) IsCommandDelimiter() bool { // @Beta type ShAtom struct { Type ShAtomType - Text string + MkText string Quoting ShQuoting Data interface{} } @@ -74,13 +76,13 @@ func NewShAtomVaruse(text string, quoting ShQuoting, varname string, modifiers . func (token *ShAtom) String() string { if token.Type == shtWord && token.Quoting == shqPlain && token.Data == nil { - return fmt.Sprintf("%q", token.Text) + return fmt.Sprintf("%q", token.MkText) } if token.Type == shtVaruse { varuse := token.Data.(*MkVarUse) return fmt.Sprintf("varuse(%q)", varuse.varname+varuse.Mod()) } - return fmt.Sprintf("ShAtom(%v, %q, %s)", token.Type, token.Text, token.Quoting) + return fmt.Sprintf("ShAtom(%v, %q, %s)", token.Type, token.MkText, token.Quoting) } // ShQuoting describes the context in which a string appears @@ -92,9 +94,11 @@ const ( shqDquot shqSquot shqBackt + shqSubsh shqDquotBackt shqBacktDquot shqBacktSquot + shqSubshSquot shqDquotBacktDquot shqDquotBacktSquot shqUnknown @@ -103,8 +107,8 @@ const ( func (q ShQuoting) String() string { return [...]string{ "plain", - "d", "s", "b", - "db", "bd", "bs", + "d", "s", "b", "S", + "db", "bd", "bs", "Ss", "dbd", "dbs", "unknown", }[q] diff --git a/pkgtools/pkglint/files/substcontext.go b/pkgtools/pkglint/files/substcontext.go index f70b930ee1a..c26d9235965 100644 --- a/pkgtools/pkglint/files/substcontext.go +++ b/pkgtools/pkglint/files/substcontext.go @@ -20,12 +20,12 @@ func (ctx *SubstContext) Varassign(mkline *MkLine) { varname := mkline.Varname() op := mkline.Op() value := mkline.Value() - if varname == "SUBST_CLASSES" { + if varname == "SUBST_CLASSES" || hasPrefix(varname, "SUBST_CLASSES.") { classes := splitOnSpace(value) if len(classes) > 1 { mkline.Warn0("Please add only one class at a time to SUBST_CLASSES.") } - if ctx.id != "" { + if ctx.id != "" && ctx.id != classes[0] { mkline.Warn0("SUBST_CLASSES should only appear once in a SUBST block.") } ctx.id = classes[0] diff --git a/pkgtools/pkglint/files/substcontext_test.go b/pkgtools/pkglint/files/substcontext_test.go index 380ccd855f7..49654fba5ad 100644 --- a/pkgtools/pkglint/files/substcontext_test.go +++ b/pkgtools/pkglint/files/substcontext_test.go @@ -49,6 +49,23 @@ func (s *Suite) TestSubstContext_Complete(c *check.C) { c.Check(s.Output(), equals, "") } +func (s *Suite) Test_SubstContext_OPSYSVARS(c *check.C) { + G.opts.WarnExtra = true + ctx := new(SubstContext) + + ctx.Varassign(newSubstLine(11, "SUBST_CLASSES.SunOS+=prefix")) + ctx.Varassign(newSubstLine(12, "SUBST_CLASSES.NetBSD+=prefix")) + ctx.Varassign(newSubstLine(13, "SUBST_FILES.prefix=Makefile")) + ctx.Varassign(newSubstLine(14, "SUBST_SED.prefix=s,@PREFIX@,${PREFIX},g")) + ctx.Varassign(newSubstLine(15, "SUBST_STAGE.prefix=post-configure")) + + c.Check(ctx.IsComplete(), equals, true) + + ctx.Finish(newSubstLine(15, "")) + + c.Check(s.Output(), equals, "") +} + func (s *Suite) TestSubstContext_NoClass(c *check.C) { s.UseCommandLine(c, "-Wextra") ctx := new(SubstContext) diff --git a/pkgtools/pkglint/files/util.go b/pkgtools/pkglint/files/util.go index 8614ef42e9c..4b8471f60df 100644 --- a/pkgtools/pkglint/files/util.go +++ b/pkgtools/pkglint/files/util.go @@ -84,6 +84,34 @@ func isCommitted(fname string) bool { return false } +func isLocallyModified(fname string) bool { + basename := path.Base(fname) + lines, err := readLines(path.Dir(fname)+"/CVS/Entries", false) + if err != nil { + return false + } + for _, line := range lines { + if hasPrefix(line.Text, "/"+basename+"/") { + cvsModTime, err := time.Parse(time.ANSIC, strings.Split(line.Text, "/")[3]) + if err != nil { + return false + } + st, err := os.Stat(fname) + if err != nil { + return false + } + + // https://msdn.microsoft.com/en-us/library/windows/desktop/ms724290(v=vs.85).aspx + delta := cvsModTime.Unix() - st.ModTime().Unix() + if G.opts.Debug { + traceStep("cvs.time=%v fs.time=%v delta=%v", cvsModTime, st.ModTime(), delta) + } + return !(-2 <= delta && delta <= 2) + } + } + return false +} + // Returns the number of columns that a string occupies when printed with // a tabulator size of 8. func tabLength(s string) int { @@ -498,12 +526,16 @@ func (r Ref) String() string { } // Emulates make(1)’s :S substitution operator. -func mkopSubst(s string, left bool, from string, right bool, to string, all bool) string { +func mkopSubst(s string, left bool, from string, right bool, to string, flags string) string { + if G.opts.Debug { + defer tracecall(s, left, from, right, to, flags)() + } re := ifelseStr(left, "^", "") + regexp.QuoteMeta(from) + ifelseStr(right, "$", "") done := false + gflag := contains(flags, "g") return regcomp(re).ReplaceAllStringFunc(s, func(match string) string { - if all || !done { - done = !all + if gflag || !done { + done = !gflag return to } return match diff --git a/pkgtools/pkglint/files/util_test.go b/pkgtools/pkglint/files/util_test.go index 5c83e84cac9..2e69b9cb796 100644 --- a/pkgtools/pkglint/files/util_test.go +++ b/pkgtools/pkglint/files/util_test.go @@ -5,30 +5,30 @@ import ( ) func (s *Suite) TestMkopSubst_middle(c *check.C) { - c.Check(mkopSubst("pkgname", false, "kgna", false, "ri", false), equals, "prime") - c.Check(mkopSubst("pkgname", false, "pkgname", false, "replacement", false), equals, "replacement") + c.Check(mkopSubst("pkgname", false, "kgna", false, "ri", ""), equals, "prime") + c.Check(mkopSubst("pkgname", false, "pkgname", false, "replacement", ""), equals, "replacement") } func (s *Suite) TestMkopSubst_left(c *check.C) { - c.Check(mkopSubst("pkgname", true, "kgna", false, "ri", false), equals, "pkgname") - c.Check(mkopSubst("pkgname", true, "pkgname", false, "replacement", false), equals, "replacement") + c.Check(mkopSubst("pkgname", true, "kgna", false, "ri", ""), equals, "pkgname") + c.Check(mkopSubst("pkgname", true, "pkgname", false, "replacement", ""), equals, "replacement") } func (s *Suite) TestMkopSubst_right(c *check.C) { - c.Check(mkopSubst("pkgname", false, "kgna", true, "ri", false), equals, "pkgname") - c.Check(mkopSubst("pkgname", false, "pkgname", true, "replacement", false), equals, "replacement") + c.Check(mkopSubst("pkgname", false, "kgna", true, "ri", ""), equals, "pkgname") + c.Check(mkopSubst("pkgname", false, "pkgname", true, "replacement", ""), equals, "replacement") } func (s *Suite) TestMkopSubst_leftRight(c *check.C) { - c.Check(mkopSubst("pkgname", true, "kgna", true, "ri", false), equals, "pkgname") - c.Check(mkopSubst("pkgname", false, "pkgname", false, "replacement", false), equals, "replacement") + c.Check(mkopSubst("pkgname", true, "kgna", true, "ri", ""), equals, "pkgname") + c.Check(mkopSubst("pkgname", false, "pkgname", false, "replacement", ""), equals, "replacement") } -func (s *Suite) TestMkopSubst_all(c *check.C) { - c.Check(mkopSubst("aaaaa", false, "a", false, "b", true), equals, "bbbbb") - c.Check(mkopSubst("aaaaa", true, "a", false, "b", true), equals, "baaaa") - c.Check(mkopSubst("aaaaa", false, "a", true, "b", true), equals, "aaaab") - c.Check(mkopSubst("aaaaa", true, "a", true, "b", true), equals, "aaaaa") +func (s *Suite) TestMkopSubst_gflag(c *check.C) { + c.Check(mkopSubst("aaaaa", false, "a", false, "b", "g"), equals, "bbbbb") + c.Check(mkopSubst("aaaaa", true, "a", false, "b", "g"), equals, "baaaa") + c.Check(mkopSubst("aaaaa", false, "a", true, "b", "g"), equals, "aaaab") + c.Check(mkopSubst("aaaaa", true, "a", true, "b", "g"), equals, "aaaaa") } func (s *Suite) TestReplaceFirst(c *check.C) { diff --git a/pkgtools/pkglint/files/vardefs.go b/pkgtools/pkglint/files/vardefs.go index 948d965aecc..98839ee809d 100644 --- a/pkgtools/pkglint/files/vardefs.go +++ b/pkgtools/pkglint/files/vardefs.go @@ -144,11 +144,12 @@ func (gd *GlobalData) InitVartypes() { acl("BUILDLINK_PASSTHRU_DIRS", lkShell, CheckvarPathname, "Makefile, Makefile.common, buildlink3.mk, hacks.mk: append") acl("BUILDLINK_PASSTHRU_RPATHDIRS", lkShell, CheckvarPathname, "Makefile, Makefile.common, buildlink3.mk, hacks.mk: append") acl("BUILDLINK_PKGSRCDIR.*", lkNone, CheckvarRelativePkgDir, "buildlink3.mk: default, use-loadtime") - acl("BUILDLINK_PREFIX.*", lkNone, CheckvarPathname, "builtin.mk: set, use; buildlink3.mk: use; Makefile, Makefile.common, *.mk: use") + acl("BUILDLINK_PREFIX.*", lkNone, CheckvarPathname, "builtin.mk: set, use; Makefile, Makefile.common, *.mk: use") acl("BUILDLINK_RPATHDIRS.*", lkShell, CheckvarPathname, "buildlink3.mk: append") acl("BUILDLINK_TARGETS", lkShell, CheckvarIdentifier, "") acl("BUILDLINK_FNAME_TRANSFORM.*", lkNone, CheckvarSedCommands, "Makefile, buildlink3.mk, builtin.mk, hacks.mk: append") acl("BUILDLINK_TRANSFORM", lkShell, CheckvarWrapperTransform, "*: append") + acl("BUILDLINK_TRANSFORM.*", lkShell, CheckvarWrapperTransform, "*: append") acl("BUILDLINK_TREE", lkShell, CheckvarIdentifier, "buildlink3.mk: append") acl("BUILD_DEFS", lkShell, CheckvarVarname, "Makefile, Makefile.common, options.mk: append") acl("BUILD_DEPENDS", lkSpace, CheckvarDependencyWithPath, "Makefile, Makefile.common, *.mk: append") @@ -157,12 +158,15 @@ func (gd *GlobalData) InitVartypes() { sys("BUILD_MAKE_CMD", lkNone, CheckvarShellCommand) pkglist("BUILD_MAKE_FLAGS", lkShell, CheckvarShellWord) pkglist("BUILD_TARGET", lkShell, CheckvarIdentifier) + pkglist("BUILD_TARGET.*", lkShell, CheckvarIdentifier) pkg("BUILD_USES_MSGFMT", lkNone, CheckvarYes) acl("BUILTIN_PKG", lkNone, CheckvarIdentifier, "builtin.mk: set, use-loadtime, use") acl("BUILTIN_PKG.*", lkNone, CheckvarPkgName, "builtin.mk: set, use-loadtime, use") acl("BUILTIN_FIND_FILES_VAR", lkShell, CheckvarVarname, "builtin.mk: set") acl("BUILTIN_FIND_FILES.*", lkShell, CheckvarPathname, "builtin.mk: set") acl("BUILTIN_FIND_GREP.*", lkNone, CheckvarString, "builtin.mk: set") + acl("BUILTIN_FIND_HEADERS_VAR", lkShell, CheckvarVarname, "builtin.mk: set") + acl("BUILTIN_FIND_HEADERS.*", lkShell, CheckvarPathname, "builtin.mk: set") acl("BUILTIN_FIND_LIBS", lkShell, CheckvarPathname, "builtin.mk: set") acl("BUILTIN_IMAKE_CHECK", lkShell, CheckvarUnchecked, "builtin.mk: set") acl("BUILTIN_IMAKE_CHECK.*", lkNone, CheckvarYesNo, "") @@ -171,9 +175,10 @@ func (gd *GlobalData) InitVartypes() { acl("CATEGORIES", lkShell, CheckvarCategory, "Makefile: set, append; Makefile.common: set, default, append") sys("CC_VERSION", lkNone, CheckvarMessage) sys("CC", lkNone, CheckvarShellCommand) - pkglist("CFLAGS*", lkShell, CheckvarCFlag) // may also be changed by the user + pkglist("CFLAGS", lkShell, CheckvarCFlag) // may also be changed by the user + pkglist("CFLAGS.*", lkShell, CheckvarCFlag) // may also be changed by the user acl("CHECK_BUILTIN", lkNone, CheckvarYesNo, "builtin.mk: default; Makefile: set") - acl("CHECK_BUILTIN.*", lkNone, CheckvarYesNo, "buildlink3.mk: set; builtin.mk: default; *: use-loadtime") + acl("CHECK_BUILTIN.*", lkNone, CheckvarYesNo, "Makefile, options.mk, buildlink3.mk: set; builtin.mk: default; *: use-loadtime") acl("CHECK_FILES_SKIP", lkShell, CheckvarBasicRegularExpression, "Makefile, Makefile.common: append") pkg("CHECK_FILES_SUPPORTED", lkNone, CheckvarYesNo) usr("CHECK_HEADERS", lkNone, CheckvarYesNo) @@ -190,11 +195,14 @@ func (gd *GlobalData) InitVartypes() { pkglist("CHECK_WRKREF_SKIP", lkShell, CheckvarPathmask) pkg("CMAKE_ARG_PATH", lkNone, CheckvarPathname) pkglist("CMAKE_ARGS", lkShell, CheckvarShellWord) + pkglist("CMAKE_ARGS.*", lkShell, CheckvarShellWord) acl("COMMENT", lkNone, CheckvarComment, "Makefile, Makefile.common: set, append") acl("COMPILER_RPATH_FLAG", lkNone, enum("-Wl,-rpath"), "*: use") pkglist("CONFIGURE_ARGS", lkShell, CheckvarShellWord) + pkglist("CONFIGURE_ARGS.*", lkShell, CheckvarShellWord) pkglist("CONFIGURE_DIRS", lkShell, CheckvarWrksrcSubdirectory) acl("CONFIGURE_ENV", lkShell, CheckvarShellWord, "Makefile, Makefile.common: append, set, use; buildlink3.mk, builtin.mk: append; *.mk: append, use") + acl("CONFIGURE_ENV.*", lkShell, CheckvarShellWord, "Makefile, Makefile.common: append, set, use; buildlink3.mk, builtin.mk: append; *.mk: append, use") pkg("CONFIGURE_HAS_INFODIR", lkNone, CheckvarYesNo) pkg("CONFIGURE_HAS_LIBDIR", lkNone, CheckvarYesNo) pkg("CONFIGURE_HAS_MANDIR", lkNone, CheckvarYesNo) @@ -209,10 +217,12 @@ func (gd *GlobalData) InitVartypes() { pkglist("CONF_FILES_PERMS", lkShell, CheckvarPerms) sys("COPY", lkNone, enum("-c")) // The flag that tells ${INSTALL} to copy a file sys("CPP", lkNone, CheckvarShellCommand) - pkglist("CPPFLAGS*", lkShell, CheckvarCFlag) + pkglist("CPPFLAGS", lkShell, CheckvarCFlag) + pkglist("CPPFLAGS.*", lkShell, CheckvarCFlag) acl("CRYPTO", lkNone, CheckvarYes, "Makefile: set") sys("CXX", lkNone, CheckvarShellCommand) - pkglist("CXXFLAGS*", lkShell, CheckvarCFlag) + pkglist("CXXFLAGS", lkShell, CheckvarCFlag) + pkglist("CXXFLAGS.*", lkShell, CheckvarCFlag) acl("DEINSTALL_FILE", lkNone, CheckvarPathname, "Makefile: set") acl("DEINSTALL_SRC", lkShell, CheckvarPathname, "Makefile: set; Makefile.common: default, set") acl("DEINSTALL_TEMPLATES", lkShell, CheckvarPathname, "Makefile: set, append; Makefile.common: set, default, append") @@ -220,7 +230,7 @@ func (gd *GlobalData) InitVartypes() { sys("DELAYED_WARNING_MSG", lkNone, CheckvarShellCommand) pkglist("DEPENDS", lkSpace, CheckvarDependencyWithPath) usr("DEPENDS_TARGET", lkShell, CheckvarIdentifier) - acl("DESCR_SRC", lkShell, CheckvarPathname, "Makefile: set; Makefile.common: default, set") + acl("DESCR_SRC", lkShell, CheckvarPathname, "Makefile: set, append; Makefile.common: default, set") sys("DESTDIR", lkNone, CheckvarPathname) acl("DESTDIR_VARNAME", lkNone, CheckvarVarname, "Makefile, Makefile.common: set") sys("DEVOSSAUDIO", lkNone, CheckvarPathname) @@ -376,17 +386,19 @@ func (gd *GlobalData) InitVartypes() { usr("KRB5_DEFAULT", lkNone, enum("heimdal mit-krb5")) sys("KRB5_TYPE", lkNone, CheckvarIdentifier) sys("LD", lkNone, CheckvarShellCommand) - pkglist("LDFLAGS*", lkShell, CheckvarLdFlag) + pkglist("LDFLAGS", lkShell, CheckvarLdFlag) + pkglist("LDFLAGS.*", lkShell, CheckvarLdFlag) sys("LIBGRP", lkNone, CheckvarUserGroupName) sys("LIBMODE", lkNone, CheckvarFileMode) sys("LIBOWN", lkNone, CheckvarUserGroupName) sys("LIBOSSAUDIO", lkNone, CheckvarPathname) - pkglist("LIBS*", lkShell, CheckvarLdFlag) + pkglist("LIBS", lkShell, CheckvarLdFlag) + pkglist("LIBS.*", lkShell, CheckvarLdFlag) sys("LIBTOOL", lkNone, CheckvarShellCommand) acl("LIBTOOL_OVERRIDE", lkShell, CheckvarPathmask, "Makefile: set, append") pkglist("LIBTOOL_REQD", lkShell, CheckvarVersion) - acl("LICENCE", lkNone, CheckvarLicense, "Makefile, Makefile.common: set; options.mk: set") - acl("LICENSE", lkNone, CheckvarLicense, "Makefile, Makefile.common: set; options.mk: set") + acl("LICENCE", lkNone, CheckvarLicense, "Makefile, Makefile.common, options.mk: set") + acl("LICENSE", lkNone, CheckvarLicense, "Makefile, Makefile.common, options.mk: set") pkg("LICENSE_FILE", lkNone, CheckvarPathname) sys("LINKER_RPATH_FLAG", lkNone, CheckvarShellWord) sys("LOWER_OPSYS", lkNone, CheckvarIdentifier) @@ -398,12 +410,14 @@ func (gd *GlobalData) InitVartypes() { acl("MAINTAINER", lkNone, CheckvarMailAddress, "Makefile: set; Makefile.common: default") sys("MAKE", lkNone, CheckvarShellCommand) pkglist("MAKEFLAGS", lkShell, CheckvarShellWord) - acl("MAKEVARS", lkShell, CheckvarVarname, "builtin.mk: append; buildlink3.mk: append; hacks.mk: append") + acl("MAKEVARS", lkShell, CheckvarVarname, "buildlink3.mk, builtin.mk, hacks.mk: append") pkglist("MAKE_DIRS", lkShell, CheckvarPathname) pkglist("MAKE_DIRS_PERMS", lkShell, CheckvarPerms) - acl("MAKE_ENV", lkShell, CheckvarShellWord, "Makefile: append, set, use; Makefile.common: append, set, use; buildlink3.mk: append; builtin.mk: append; *.mk: append, use") + acl("MAKE_ENV", lkShell, CheckvarShellWord, "Makefile, Makefile.common: append, set, use; buildlink3.mk, builtin.mk: append; *.mk: append, use") + acl("MAKE_ENV.*", lkShell, CheckvarShellWord, "Makefile, Makefile.common: append, set, use; buildlink3.mk, builtin.mk: append; *.mk: append, use") pkg("MAKE_FILE", lkNone, CheckvarPathname) pkglist("MAKE_FLAGS", lkShell, CheckvarShellWord) + pkglist("MAKE_FLAGS.*", lkShell, CheckvarShellWord) usr("MAKE_JOBS", lkNone, CheckvarInteger) pkg("MAKE_JOBS_SAFE", lkNone, CheckvarYesNo) pkg("MAKE_PROGRAM", lkNone, CheckvarShellCommand) @@ -445,7 +459,7 @@ func (gd *GlobalData) InitVartypes() { sys("MASTER_SITE_XCONTRIB", lkShell, CheckvarFetchURL) sys("MASTER_SITE_XEMACS", lkShell, CheckvarFetchURL) pkglist("MESSAGE_SRC", lkShell, CheckvarPathname) - acl("MESSAGE_SUBST", lkShell, CheckvarShellWord, "Makefile.common: append; Makefile: append; options.mk: append") + acl("MESSAGE_SUBST", lkShell, CheckvarShellWord, "Makefile, Makefile.common, options.mk: append") pkg("META_PACKAGE", lkNone, CheckvarYes) sys("MISSING_FEATURES", lkShell, CheckvarIdentifier) acl("MYSQL_VERSIONS_ACCEPTED", lkShell, enum("51 55 56"), "Makefile: set") @@ -485,8 +499,8 @@ func (gd *GlobalData) InitVartypes() { acl("PATCH_ARGS", lkShell, CheckvarShellWord, "") acl("PATCH_DIST_ARGS", lkShell, CheckvarShellWord, "Makefile: set, append") acl("PATCH_DIST_CAT", lkNone, CheckvarShellCommand, "") - acl("PATCH_DIST_STRIP*", lkNone, CheckvarShellWord, "Makefile, Makefile.common: set; buildlink3.mk:; builtin.mk:; *.mk: set") - acl("PATCH_SITES", lkShell, CheckvarFetchURL, "Makefile: set; options.mk: set; Makefile.common: set") + acl("PATCH_DIST_STRIP*", lkNone, CheckvarShellWord, "buildlink3.mk, builtin.mk:; Makefile, Makefile.common, *.mk: set") + acl("PATCH_SITES", lkShell, CheckvarFetchURL, "Makefile, Makefile.common, options.mk: set") acl("PATCH_STRIP", lkNone, CheckvarShellWord, "") pkg("PERL5_USE_PACKLIST", lkNone, CheckvarYesNo) acl("PERL5_PACKLIST", lkShell, CheckvarPerl5Packlist, "Makefile: set; options.mk: set, append") @@ -548,21 +562,21 @@ func (gd *GlobalData) InitVartypes() { acl("PKG_OPTIONS", lkSpace, CheckvarOption, "bsd.options.mk: set; *: use-loadtime, use") usr("PKG_OPTIONS.*", lkSpace, CheckvarOption) acl("PKG_OPTIONS_DEPRECATED_WARNINGS", lkShell, CheckvarShellWord, "") - acl("PKG_OPTIONS_GROUP.*", lkSpace, CheckvarOption, "options.mk: set; Makefile: set") - acl("PKG_OPTIONS_LEGACY_OPTS", lkSpace, CheckvarUnchecked, "Makefile, Makefile.common: append; options.mk: append") - acl("PKG_OPTIONS_LEGACY_VARS", lkSpace, CheckvarUnchecked, "Makefile, Makefile.common: append; options.mk: append") + acl("PKG_OPTIONS_GROUP.*", lkSpace, CheckvarOption, "Makefile, options.mk: set, append") + acl("PKG_OPTIONS_LEGACY_OPTS", lkSpace, CheckvarUnchecked, "Makefile, Makefile.common, options.mk: append") + acl("PKG_OPTIONS_LEGACY_VARS", lkSpace, CheckvarUnchecked, "Makefile, Makefile.common, options.mk: append") acl("PKG_OPTIONS_NONEMPTY_SETS", lkSpace, CheckvarIdentifier, "") acl("PKG_OPTIONS_OPTIONAL_GROUPS", lkSpace, CheckvarIdentifier, "options.mk: set, append") - acl("PKG_OPTIONS_REQUIRED_GROUPS", lkSpace, CheckvarIdentifier, "options.mk: set; Makefile: set") + acl("PKG_OPTIONS_REQUIRED_GROUPS", lkSpace, CheckvarIdentifier, "Makefile, options.mk: set") acl("PKG_OPTIONS_SET.*", lkSpace, CheckvarOption, "") - acl("PKG_OPTIONS_VAR", lkNone, CheckvarPkgOptionsVar, "options.mk: set; Makefile, Makefile.common: set; bsd.options.mk: use-loadtime") + acl("PKG_OPTIONS_VAR", lkNone, CheckvarPkgOptionsVar, "Makefile, Makefile.common, options.mk: set; bsd.options.mk: use-loadtime") acl("PKG_PRESERVE", lkNone, CheckvarYes, "Makefile: set") acl("PKG_SHELL", lkNone, CheckvarPathname, "Makefile, Makefile.common: set") acl("PKG_SHELL.*", lkNone, CheckvarPathname, "Makefile, Makefile.common: set") acl("PKG_SHLIBTOOL", lkNone, CheckvarPathname, "") pkglist("PKG_SKIP_REASON", lkShell, CheckvarShellWord) - acl("PKG_SUGGESTED_OPTIONS", lkShell, CheckvarOption, "options.mk: set, append; Makefile: set, append; Makefile.common: set") - acl("PKG_SUPPORTED_OPTIONS", lkShell, CheckvarOption, "options.mk: set, append, use; Makefile: set, append; Makefile.common: set") + acl("PKG_SUGGESTED_OPTIONS", lkShell, CheckvarOption, "Makefile, Makefile.common, options.mk: set, append") + acl("PKG_SUPPORTED_OPTIONS", lkShell, CheckvarOption, "Makefile: set, append; Makefile.common: set; options.mk: set, append, use") pkg("PKG_SYSCONFDIR*", lkNone, CheckvarPathname) pkglist("PKG_SYSCONFDIR_PERMS", lkShell, CheckvarPerms) sys("PKG_SYSCONFBASEDIR", lkNone, CheckvarPathname) @@ -586,7 +600,7 @@ func (gd *GlobalData) InitVartypes() { sys("PTHREAD_CFLAGS", lkShell, CheckvarCFlag) sys("PTHREAD_LDFLAGS", lkShell, CheckvarLdFlag) sys("PTHREAD_LIBS", lkShell, CheckvarLdFlag) - acl("PTHREAD_OPTS", lkShell, enum("native optional require"), "Makefile: set, append; Makefile.common: append; buildlink3.mk: append") + acl("PTHREAD_OPTS", lkShell, enum("native optional require"), "Makefile: set, append; Makefile.common, buildlink3.mk: append") sys("PTHREAD_TYPE", lkNone, CheckvarIdentifier) // Or "native" or "none". pkg("PY_PATCHPLIST", lkNone, CheckvarYes) acl("PYPKGPREFIX", lkNone, enum("py27 py33 py34 py35"), "pyversion.mk: set; *: use-loadtime, use") @@ -638,7 +652,8 @@ func (gd *GlobalData) InitVartypes() { sys("STEP_MSG", lkNone, CheckvarShellCommand) acl("SUBDIR", lkShell, CheckvarFilename, "Makefile: append; *:") acl("SUBST_CLASSES", lkShell, CheckvarIdentifier, "Makefile: set, append; *: append") - acl("SUBST_FILES.*", lkShell, CheckvarPathmask, "Makefile: set, append; Makefile.*, *.mk: set, append") + acl("SUBST_CLASSES.*", lkShell, CheckvarIdentifier, "Makefile: set, append; *: append") + acl("SUBST_FILES.*", lkShell, CheckvarPathmask, "Makefile, Makefile.*, *.mk: set, append") acl("SUBST_FILTER_CMD.*", lkNone, CheckvarShellCommand, "Makefile, Makefile.*, *.mk: set") acl("SUBST_MESSAGE.*", lkNone, CheckvarMessage, "Makefile, Makefile.*, *.mk: set") acl("SUBST_SED.*", lkNone, CheckvarSedCommands, "Makefile, Makefile.*, *.mk: set, append") @@ -672,10 +687,10 @@ func (gd *GlobalData) InitVartypes() { acl("USE_BUILTIN.*", lkNone, CheckvarYesNoIndirectly, "builtin.mk: set") pkg("USE_CMAKE", lkNone, CheckvarYes) usr("USE_DESTDIR", lkNone, CheckvarYes) - pkg("USE_FEATURES", lkShell, CheckvarIdentifier) + pkglist("USE_FEATURES", lkShell, CheckvarIdentifier) pkg("USE_GCC_RUNTIME", lkNone, CheckvarYesNo) pkg("USE_GNU_CONFIGURE_HOST", lkNone, CheckvarYesNo) - acl("USE_GNU_ICONV", lkNone, CheckvarYes, "Makefile, Makefile.common: set; options.mk: set") + acl("USE_GNU_ICONV", lkNone, CheckvarYes, "Makefile, Makefile.common, options.mk: set") acl("USE_IMAKE", lkNone, CheckvarYes, "Makefile: set") pkg("USE_JAVA", lkNone, enum("run yes build")) pkg("USE_JAVA2", lkNone, enum("YES yes no 1.4 1.5 6 7 8")) @@ -689,6 +704,7 @@ func (gd *GlobalData) InitVartypes() { pkg("USE_PKGLOCALEDIR", lkNone, CheckvarYesNo) usr("USE_PKGSRC_GCC", lkNone, CheckvarYes) acl("USE_TOOLS", lkShell, CheckvarTool, "*: append") + acl("USE_TOOLS.*", lkShell, CheckvarTool, "*: append") pkg("USE_X11", lkNone, CheckvarYes) sys("WARNING_MSG", lkNone, CheckvarShellCommand) sys("WARNING_CAT", lkNone, CheckvarShellCommand) @@ -753,6 +769,7 @@ func parseAclEntries(varname string, aclentries string) []AclEntry { return nil } var result []AclEntry + prevperms := "(first)" for _, arg := range strings.Split(aclentries, "; ") { var globs, perms string if fields := strings.SplitN(arg, ": ", 2); len(fields) == 2 { @@ -760,6 +777,10 @@ func parseAclEntries(varname string, aclentries string) []AclEntry { } else { globs = strings.TrimSuffix(arg, ":") } + if perms == prevperms { + fmt.Printf("Repeated permissions for %s: %s\n", varname, perms) + } + prevperms = perms var permissions AclPermissions for _, perm := range strings.Split(perms, ", ") { switch perm { diff --git a/pkgtools/pkglint/files/vartypecheck.go b/pkgtools/pkglint/files/vartypecheck.go index 76d1d741311..030c9f96a82 100644 --- a/pkgtools/pkglint/files/vartypecheck.go +++ b/pkgtools/pkglint/files/vartypecheck.go @@ -684,6 +684,11 @@ func (cv *VartypeCheck) PkgOptionsVar() { "very last file, but PKG_OPTIONS_VAR is evaluated earlier.", "Use ${PKGNAME:C/-[0-9].*//} instead.") } + + // PR 46570, item "6. It should complain in PKG_OPTIONS_VAR is wrong" + if !hasPrefix(cv.value, "PKG_OPTIONS.") { + cv.line.Error2("PKG_OPTIONS_VAR must be of the form %q, not %q.", "PKG_OPTIONS.*", cv.value) + } } // A directory name relative to the top-level pkgsrc directory. diff --git a/pkgtools/pkglint/files/vartypecheck_test.go b/pkgtools/pkglint/files/vartypecheck_test.go index e15ebaf2f30..2893ec9fb07 100644 --- a/pkgtools/pkglint/files/vartypecheck_test.go +++ b/pkgtools/pkglint/files/vartypecheck_test.go @@ -289,9 +289,11 @@ func (s *Suite) Test_VartypeCheck_Perms(c *check.C) { func (s *Suite) TestVartypeCheck_PkgOptionsVar(c *check.C) { runVartypeChecks("PKG_OPTIONS_VAR.screen", opAssign, (*VartypeCheck).PkgOptionsVar, - "PKG_OPTIONS.${PKGBASE}") + "PKG_OPTIONS.${PKGBASE}", + "PKG_OPTIONS.anypkgbase") - c.Check(s.Output(), equals, "ERROR: fname:1: PKGBASE must not be used in PKG_OPTIONS_VAR.\n") + c.Check(s.Output(), equals, ""+ + "ERROR: fname:1: PKGBASE must not be used in PKG_OPTIONS_VAR.\n") } func (s *Suite) TestVartypeCheck_PkgRevision(c *check.C) { diff --git a/pkgtools/pkglint/files/vercmp_test.go b/pkgtools/pkglint/files/vercmp_test.go index 190f5779559..11edd40aab9 100644 --- a/pkgtools/pkglint/files/vercmp_test.go +++ b/pkgtools/pkglint/files/vercmp_test.go @@ -15,15 +15,50 @@ func (s *Suite) TestMkversion(c *check.C) { c.Check(newVersion("nb1"), check.DeepEquals, &version{nil, 1}) c.Check(newVersion("1.0.1a"), deepEquals, &version{[]int{1, 0, 0, 0, 1, 1}, 0}) c.Check(newVersion("1.0.1z"), deepEquals, &version{[]int{1, 0, 0, 0, 1, 26}, 0}) + c.Check(newVersion("0pre20160620"), deepEquals, &version{[]int{0, -1, 20160620}, 0}) } func (s *Suite) TestPkgverCmp(c *check.C) { - c.Check(pkgverCmp("1.0", "1.0alpha"), equals, 1) - c.Check(pkgverCmp("1.0alpha", "1.0"), equals, -1) - c.Check(pkgverCmp("1.0nb1", "1.0"), equals, 1) - c.Check(pkgverCmp("1.0nb2", "1.0nb1"), equals, 1) - c.Check(pkgverCmp("2.0.1nb17", "2.0.1nb4"), equals, 1) - c.Check(pkgverCmp("2.0.1nb4", "2.0.1nb17"), equals, -1) - c.Check(pkgverCmp("2.0pre", "2.0rc"), equals, 0) - c.Check(pkgverCmp("2.0pre", "2.0pl"), equals, -1) + var versions = [][]string{ + {"0pre20160620"}, + {"0"}, + {"nb1"}, + {"0.0.1-SNAPSHOT"}, + {"1.0alpha"}, + {"1.0alpha3"}, + {"1", "1.0", "1.0.0"}, + {"1.0nb1"}, + {"1.0nb2"}, + {"1.0.1a"}, + {"1.0.1z"}, + {"2.0pre", "2.0rc"}, + {"2.0", "2.0pl"}, + {"2.0.1nb4"}, + {"2.0.1nb17"}, + {"2.5beta"}, + {"5.0"}, + {"5.0nb5"}, + {"5.5", "5.005"}, + {"20151110"}, + } + + for i, iversions := range versions { + for _, iversion := range iversions { + for j, jversions := range versions { + for _, jversion := range jversions { + actual := pkgverCmp(iversion, jversion) + if i < j && !(actual < 0) { + c.Check([]interface{}{i, iversion, j, jversion, "<0"}, deepEquals, []interface{}{i, iversion, j, jversion, actual}) + } + if i == j && !(actual == 0) { + c.Check([]interface{}{i, iversion, j, jversion, "==0"}, deepEquals, []interface{}{i, iversion, j, jversion, actual}) + } + if i > j && !(actual > 0) { + c.Check([]interface{}{i, iversion, j, jversion, ">0"}, deepEquals, []interface{}{i, iversion, j, jversion, actual}) + } + } + } + + } + } } -- cgit v1.2.3