diff options
Diffstat (limited to 'pkgtools/pkglint')
-rw-r--r-- | pkgtools/pkglint/TODO | 4 | ||||
-rw-r--r-- | pkgtools/pkglint/files/PkgLint/Shell.pm | 708 | ||||
-rw-r--r-- | pkgtools/pkglint/files/pkglint.pl | 716 |
3 files changed, 711 insertions, 717 deletions
diff --git a/pkgtools/pkglint/TODO b/pkgtools/pkglint/TODO index b127fe340a1..8f96041401b 100644 --- a/pkgtools/pkglint/TODO +++ b/pkgtools/pkglint/TODO @@ -1,4 +1,4 @@ -$NetBSD: TODO,v 1.81 2015/10/11 07:27:12 rillig Exp $ +$NetBSD: TODO,v 1.82 2015/10/15 02:09:49 rillig Exp $ Please add your own entries at the bottom of this file. If possible, include the name of an example package where a warning should occur. @@ -12,8 +12,6 @@ include the name of an example package where a warning should occur. don't. Find out how pkglint can distinguish them. * Make sure that no variable is modified at load-time after it has been used once. This should at least flag BUILD_DEFS in bsd.pkg.mk. -* Implement unescape_shellword, which is required for the type check for - SedCommand. * Invent an annotation scheme for files that intentionally define variables for use in other files. * ${MACHINE_ARCH}-${LOWER_OPSYS}elf in PLISTs etc. is a NetBSD config.guess diff --git a/pkgtools/pkglint/files/PkgLint/Shell.pm b/pkgtools/pkglint/files/PkgLint/Shell.pm new file mode 100644 index 00000000000..69802ba1bb7 --- /dev/null +++ b/pkgtools/pkglint/files/PkgLint/Shell.pm @@ -0,0 +1,708 @@ +# $NetBSD: Shell.pm,v 1.1 2015/10/15 02:09:49 rillig Exp $ +# +# Parsing and checking shell commands embedded in Makefiles +# + +sub checkline_mk_shellword($$$) { + my ($line, $shellword, $check_quoting) = @_; + my ($rest, $state, $value); + + $opt_debug_trace and $line->log_debug("checkline_mk_shellword(\"${shellword}\", ${check_quoting})."); + use constant shellcommand_context_type => PkgLint::Type->new( + LK_NONE, "ShellCommand", [[ qr".*", "adsu" ]], NOT_GUESSED + ); + use constant shellword_vuc => PkgLint::VarUseContext->new( + VUC_TIME_UNKNOWN, + shellcommand_context_type, + VUC_SHELLWORD_PLAIN, + VUC_EXTENT_WORD + ); + + if ($shellword =~ m"^\$\{(${regex_varname})(:[^{}]+)?\}$") { + my ($varname, $mod) = ($1, $2); + + checkline_mk_varuse($line, $varname, defined($mod) ? $mod : "", shellword_vuc); + return; + } + + if ($shellword =~ m"\$\{PREFIX\}/man(?:$|/)") { + $line->log_warning("Please use \${PKGMANDIR} instead of \"man\"."); + } + + if ($shellword =~ m"etc/rc\.d") { + $line->log_warning("Please use the RCD_SCRIPTS mechanism to install rc.d scripts automatically to \${RCD_SCRIPTS_EXAMPLEDIR}."); + } + + # Note: SWST means [S]hell[W]ord [ST]ate + use enum qw(:SWST_ PLAIN SQUOT DQUOT DQUOT_BACKT BACKT); + use constant statename => [ + "SWST_PLAIN", "SWST_SQUOT", "SWST_DQUOT", + "SWST_DQUOT_BACKT", "SWST_BACKT", + ]; + use constant user_statename => [ + "unquoted string", "single quoted string", + "double quoted string", "backticks inside double quoted string", + "backticks", + ]; + + $rest = ($shellword =~ m"^#") ? "" : $shellword; + $state = SWST_PLAIN; + while ($rest ne "") { + + $opt_debug_shell and $line->log_debug(statename->[$state] . ": ${rest}"); + + # When we are parsing inside backticks, it is more + # reasonable to check the whole shell command + # recursively, instead of splitting off the first + # make(1) variable (see the elsif below). + if ($state == SWST_BACKT || $state == SWST_DQUOT_BACKT) { + + # Scan for the end of the backticks, checking + # for single backslashes and removing one level + # of backslashes. Backslashes are only removed + # before a dollar, a backslash or a backtick. + # + # References: + # * http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_06_03 + my $stripped = ""; + while ($rest ne "") { + if ($rest =~ s/^\`//) { #` + $state = ($state == SWST_BACKT) ? SWST_PLAIN : SWST_DQUOT; + goto end_of_backquotes; + } elsif ($rest =~ s/^\\([\\\`\$])//) { #` + $stripped .= $1; + } elsif ($rest =~ s/^(\\)//) { + $line->log_warning("Backslashes should be doubled inside backticks."); + $stripped .= $1; + } elsif ($state == SWST_DQUOT_BACKT && $rest =~ s/^"//) { #" + $line->log_warning("Double quotes inside backticks inside double quotes are error prone."); + $line->explain_warning( +"According to the SUSv3, they produce undefined results.", +"", +"See the paragraph starting \"Within the backquoted ...\" in", +"http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html"); + } elsif ($rest =~ s/^([^\\\`]+)//) { #` + $stripped .= $1; + } else { + assert(false, "rest=$rest"); + } + } + $line->log_error("Unfinished backquotes: rest=$rest"); + + end_of_backquotes: + # Check the resulting command. + checkline_mk_shelltext($line, $stripped); + + # make(1) variables have the same syntax, no matter in + # which state we are currently. + } elsif ($rest =~ s/^\$\{(${regex_varname}|[\@])(:[^\{]+)?\}// + || $rest =~ s/^\$\((${regex_varname}|[\@])(:[^\)]+)?\)// + || $rest =~ s/^\$([\w\@])//) { + my ($varname, $mod) = ($1, $2); + + if ($varname eq "\@") { + $line->log_warning("Please use \"\${.TARGET}\" instead of \"\$\@\"."); + $line->explain_warning( +"The variable \$\@ can easily be confused with the shell variable of the", +"same name, which has a completely different meaning."); + $varname = ".TARGET"; + } + + if ($state == SWST_PLAIN && defined($mod) && $mod =~ m":Q$") { + # Fine. + + } elsif ($state == SWST_BACKT) { + # Don't check here, to avoid false positives + # for tool names. + + } elsif (($state == SWST_SQUOT || $state == SWST_DQUOT) && $varname =~ m"^(?:.*DIR|.*FILE|.*PATH|.*_VAR|PREFIX|.*BASE|PKGNAME)$") { + # This is ok if we don't allow these + # variables to have embedded [\$\\\"\'\`]. + + } elsif ($state == SWST_DQUOT && defined($mod) && $mod =~ m":Q$") { + $line->log_warning("Please don't use the :Q operator in double quotes."); + $line->explain_warning( +"Either remove the :Q or the double quotes. In most cases, it is more", +"appropriate to remove the double quotes."); + + } + + my $ctx = PkgLint::VarUseContext->new_from_pool( + VUC_TIME_UNKNOWN, + shellcommand_context_type, + ($state == SWST_PLAIN) ? VUC_SHELLWORD_PLAIN + : ($state == SWST_DQUOT) ? VUC_SHELLWORD_DQUOT + : ($state == SWST_SQUOT) ? VUC_SHELLWORD_SQUOT + : ($state == SWST_BACKT) ? VUC_SHELLWORD_BACKT + : VUC_SHELLWORD_UNKNOWN, + VUC_EXTENT_WORD_PART + ); + if ($varname ne "\@") { + checkline_mk_varuse($line, $varname, defined($mod) ? $mod : "", $ctx); + } + + # The syntax of the variable modifiers can get quite + # hairy. In lack of motivation, we just skip anything + # complicated, hoping that at least the braces are + # balanced. + } elsif ($rest =~ s/^\$\{//) { + my $braces = 1; + while ($rest ne "" && $braces > 0) { + if ($rest =~ s/^\}//) { + $braces--; + } elsif ($rest =~ s/^\{//) { + $braces++; + } elsif ($rest =~ s/^[^{}]+//) { + # Nothing to do here. + } else { + last; + } + } + + } elsif ($state == SWST_PLAIN) { + + # XXX: This is only true for the "first" word in a + # shell command, not for every word. For example, + # FOO_ENV+= VAR=`command` may be underquoted. + if (false && $rest =~ m"([\w_]+)=\"\`") { + $line->log_note("In the assignment to \"$1\", you don't need double quotes around backticks."); + $line->explain_note( +"Assignments are a special context, where no double quotes are needed", +"around backticks. In other contexts, the double quotes are necessary."); + } + + if ($rest =~ s/^[!#\%&\(\)*+,\-.\/0-9:;<=>?\@A-Z\[\]^_a-z{|}~]+//) { + } elsif ($rest =~ s/^\'//) { + $state = SWST_SQUOT; + } elsif ($rest =~ s/^\"//) { + $state = SWST_DQUOT; + } elsif ($rest =~ s/^\`//) { #` + $state = SWST_BACKT; + } elsif ($rest =~ s/^\\(?:[ !"#'\(\)*;?\\^{|}]|\$\$)//) { + } elsif ($rest =~ s/^\$\$([0-9A-Z_a-z]+|\#)// + || $rest =~ s/^\$\$\{([0-9A-Z_a-z]+|\#)\}// + || $rest =~ s/^\$\$(\$)\$//) { + my ($shvarname) = ($1); + if ($opt_warn_quoting && $check_quoting) { + $line->log_warning("Unquoted shell variable \"${shvarname}\"."); + $line->explain_warning( +"When a shell variable contains white-space, it is expanded (split into", +"multiple words) when it is written as \$variable in a shell script.", +"If that is not intended, you should add quotation marks around it,", +"like \"\$variable\". Then, the variable will always expand to a single", +"word, preserving all white-space and other special characters.", +"", +"Example:", +"\tfname=\"Curriculum vitae.doc\"", +"\tcp \$fname /tmp", +"\t# tries to copy the two files \"Curriculum\" and \"Vitae.doc\"", +"\tcp \"\$fname\" /tmp", +"\t# copies one file, as intended"); + } + + } elsif ($rest =~ s/^\$\@//) { + $line->log_warning("Please use \"\${.TARGET}\" instead of \"\$@\"."); + $line->explain_warning( +"It is more readable and prevents confusion with the shell variable of", +"the same name."); + + } elsif ($rest =~ s/^\$\$\@//) { + $line->log_warning("The \$@ shell variable should only be used in double quotes."); + + } elsif ($rest =~ s/^\$\$\?//) { + $line->log_warning("The \$? shell variable is often not available in \"set -e\" mode."); + + } elsif ($rest =~ s/^\$\$\(/(/) { + $line->log_warning("Invoking subshells via \$(...) is not portable enough."); + $line->explain_warning( +"The Solaris /bin/sh does not know this way to execute a command in a", +"subshell. Please use backticks (\`...\`) as a replacement."); + + } else { + last; + } + + } elsif ($state == SWST_SQUOT) { + if ($rest =~ s/^\'//) { + $state = SWST_PLAIN; + } elsif ($rest =~ s/^[^\$\']+//) { #' + } elsif ($rest =~ s/^\$\$//) { + } else { + last; + } + + } elsif ($state == SWST_DQUOT) { + if ($rest =~ s/^\"//) { + $state = SWST_PLAIN; + } elsif ($rest =~ s/^\`//) { #` + $state = SWST_DQUOT_BACKT; + } elsif ($rest =~ s/^[^\$"\\\`]+//) { #` + } elsif ($rest =~ s/^\\(?:[\\\"\`]|\$\$)//) { #` + } elsif ($rest =~ s/^\$\$\{([0-9A-Za-z_]+)\}// + || $rest =~ s/^\$\$([0-9A-Z_a-z]+|[!#?\@]|\$\$)//) { + my ($shvarname) = ($1); + $opt_debug_shell and $line->log_debug("[checkline_mk_shellword] Found double-quoted variable ${shvarname}."); + } elsif ($rest =~ s/^\$\$//) { + $line->log_warning("Unquoted \$ or strange shell variable found."); + } elsif ($rest =~ s/^\\(.)//) { + my ($char) = ($1); + $line->log_warning("Please use \"\\\\${char}\" instead of \"\\${char}\"."); + $line->explain_warning( +"Although the current code may work, it is not good style to rely on", +"the shell passing \"\\${char}\" exactly as is, and not discarding the", +"backslash. Alternatively you can use single quotes instead of double", +"quotes."); + } else { + last; + } + + } else { + last; + } + } + if ($rest !~ m"^\s*$") { + $line->log_error("Internal pkglint error: " . statename->[$state] . ": rest=${rest}"); + } +} + +# Some shell commands should not be used in the install phase. +# +sub checkline_mk_shellcmd_use($$) { + my ($line, $shellcmd) = @_; + + use constant allowed_install_commands => array_to_hash(qw( + ${INSTALL} + ${INSTALL_DATA} ${INSTALL_DATA_DIR} + ${INSTALL_LIB} ${INSTALL_LIB_DIR} + ${INSTALL_MAN} ${INSTALL_MAN_DIR} + ${INSTALL_PROGRAM} ${INSTALL_PROGRAM_DIR} + ${INSTALL_SCRIPT} + ${LIBTOOL} + ${LN} + ${PAX} + )); + use constant discouraged_install_commands => array_to_hash(qw( + sed ${SED} + tr ${TR} + )); + + if (defined($mkctx_target) && $mkctx_target =~ m"^(?:pre|do|post)-install") { + + if (exists(allowed_install_commands->{$shellcmd})) { + # Fine. + + } elsif (exists(discouraged_install_commands->{$shellcmd})) { + $line->log_warning("The shell command \"${shellcmd}\" should not be used in the install phase."); + $line->explain_warning( +"In the install phase, the only thing that should be done is to install", +"the prepared files to their final location. The file's contents should", +"not be changed anymore."); + + } elsif ($shellcmd eq "\${CP}") { + $line->log_warning("\${CP} should not be used to install files."); + $line->explain_warning( +"The \${CP} command is highly platform dependent and cannot overwrite", +"files that don't have write permission. Please use \${PAX} instead.", +"", +"For example, instead of", +"\t\${CP} -R \${WRKSRC}/* \${PREFIX}/foodir", +"you should use", +"\tcd \${WRKSRC} && \${PAX} -wr * \${PREFIX}/foodir"); + + } else { + $opt_debug_misc and $line->log_debug("May \"${shellcmd}\" be used in the install phase?"); + } + } +} + +sub checkline_mk_shelltext($$) { + my ($line, $text) = @_; + my ($vartools, $state, $rest, $set_e_mode); + + $opt_debug_trace and $line->log_debug("checkline_mk_shelltext(\"$text\")"); + + # Note: SCST is the abbreviation for [S]hell [C]ommand [ST]ate. + use constant scst => qw( + START CONT + INSTALL INSTALL_D + MKDIR + PAX PAX_S + SED SED_E + SET SET_CONT + COND COND_CONT + CASE CASE_IN CASE_LABEL CASE_LABEL_CONT CASE_PAREN + FOR FOR_IN FOR_CONT + ECHO + INSTALL_DIR INSTALL_DIR2 + ); + use enum (":SCST_", scst); + use constant scst_statename => [ map { "SCST_$_" } scst ]; + + use constant forbidden_commands => array_to_hash(qw( + ktrace + mktexlsr + strace + texconfig truss + )); + + if ($text =~ m"\$\{SED\}" && $text =~ m"\$\{MV\}") { + $line->log_note("Please use the SUBST framework instead of \${SED} and \${MV}."); + $line->explain_note( +"When converting things, pay attention to \"#\" characters. In shell", +"commands make(1) does not interpret them as comment character, but", +"in other lines it does. Therefore, instead of the shell command", +"", +"\tsed -e 's,#define foo,,'", +"", +"you need to write", +"", +"\tSUBST_SED.foo+=\t's,\\#define foo,,'"); + } + + if ($text =~ m"^\@*-(.*(MKDIR|INSTALL.*-d|INSTALL_.*_DIR).*)") { + my ($mkdir_cmd) = ($1); + + $line->log_note("You don't need to use \"-\" before ${mkdir_cmd}."); + } + + $vartools = get_vartool_names(); + $rest = $text; + + use constant hidden_shell_commands => array_to_hash(qw( + ${DELAYED_ERROR_MSG} ${DELAYED_WARNING_MSG} + ${DO_NADA} + ${ECHO} ${ECHO_MSG} ${ECHO_N} ${ERROR_CAT} ${ERROR_MSG} + ${FAIL_MSG} + ${PHASE_MSG} ${PRINTF} + ${SHCOMMENT} ${STEP_MSG} + ${WARNING_CAT} ${WARNING_MSG} + )); + + $set_e_mode = false; + + if ($rest =~ s/^\s*([-@]*)(\$\{_PKG_SILENT\}\$\{_PKG_DEBUG\}|\$\{RUN\}|)//) { + my ($hidden, $macro) = ($1, $2); + + if ($hidden !~ m"\@") { + # Nothing is hidden at all. + + } elsif (defined($mkctx_target) && $mkctx_target =~ m"^(?:show-.*|.*-message)$") { + # In some targets commands may be hidden. + + } elsif ($rest =~ m"^#") { + # Shell comments may be hidden, as they have no side effects + + } elsif ($rest =~ $regex_shellword) { + my ($cmd) = ($1); + + if (!exists(hidden_shell_commands->{$cmd})) { + $line->log_warning("The shell command \"${cmd}\" should not be hidden."); + $line->explain_warning( +"Hidden shell commands do not appear on the terminal or in the log file", +"when they are executed. When they fail, the error message cannot be", +"assigned to the command, which is very difficult to debug."); + } + } + + if ($hidden =~ m"-") { + $line->log_warning("The use of a leading \"-\" to suppress errors is deprecated."); + $line->explain_warning( +"If you really want to ignore any errors from this command (including", +"all errors you never thought of), append \"|| \${TRUE}\" to the", +"command."); + } + + if ($macro eq "\${RUN}") { + $set_e_mode = true; + } + } + + $state = SCST_START; + while ($rest =~ s/^$regex_shellword//) { + my ($shellword) = ($1); + + $opt_debug_shell and $line->log_debug(scst_statename->[$state] . ": ${shellword}"); + + checkline_mk_shellword($line, $shellword, !( + $state == SCST_CASE + || $state == SCST_FOR_CONT + || $state == SCST_SET_CONT + || ($state == SCST_START && $shellword =~ regex_sh_varassign))); + + # + # Actions associated with the current state + # and the symbol on the "input tape". + # + + if ($state == SCST_START || $state == SCST_COND) { + my ($type); + + if ($shellword eq "\${RUN}") { + # Just skip this one. + + } elsif (exists(forbidden_commands->{basename($shellword)})) { + $line->log_error("${shellword} must not be used in Makefiles."); + $line->explain_error( +"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", +"package via pkg_add."); + + } elsif (exists(get_tool_names()->{$shellword})) { + if (!exists($mkctx_tools->{$shellword}) && !exists($mkctx_tools->{"g$shellword"})) { + $line->log_warning("The \"${shellword}\" tool is used but not added to USE_TOOLS."); + } + + if (exists(get_required_vartools()->{$shellword})) { + $line->log_warning("Please use \"\${" . get_vartool_names()->{$shellword} . "}\" instead of \"${shellword}\"."); + } + + checkline_mk_shellcmd_use($line, $shellword); + + } elsif ($shellword =~ m"^\$\{([\w_]+)\}$" && exists(get_varname_to_toolname()->{$1})) { + my ($vartool) = ($1); + my $plain_tool = get_varname_to_toolname()->{$vartool}; + + if (!exists($mkctx_tools->{$plain_tool})) { + $line->log_warning("The \"${plain_tool}\" tool is used but not added to USE_TOOLS."); + } + + # Deactivated to allow package developers to write + # consistent code that uses ${TOOL} in all places. + if (false && defined($mkctx_target) && $mkctx_target =~ m"^(?:pre|do|post)-(?:extract|patch|wrapper|configure|build|install|package|clean)$") { + if (!exists(get_required_vartool_varnames()->{$vartool})) { + $opt_warn_extra and $line->log_note("You can write \"${plain_tool}\" instead of \"${shellword}\"."); + $opt_warn_extra and $line->explain_note( +"The wrapper framework from pkgsrc takes care that a sufficiently", +"capable implementation of that tool will be selected.", +"", +"Calling the commands by their plain name instead of the macros is", +"only available in the {pre,do,post}-* targets. For all other targets,", +"you should still use the macros."); + } + } + + checkline_mk_shellcmd_use($line, $shellword); + + } elsif ($shellword =~ m"^\$\{([\w_]+)\}$" && defined($type = get_variable_type($line, $1)) && $type->basic_type eq "ShellCommand") { + checkline_mk_shellcmd_use($line, $shellword); + + } elsif ($shellword =~ m"^\$\{(\w+)\}$" && defined($pkgctx_vardef) && exists($pkgctx_vardef->{$1})) { + # When the package author has explicitly + # defined a command variable, assume it + # to be valid. + + } elsif ($shellword =~ m"^(?:\(|\)|:|;|;;|&&|\|\||\{|\}|break|case|cd|continue|do|done|elif|else|esac|eval|exec|exit|export|fi|for|if|read|set|shift|then|umask|unset|while)$") { + # Shell builtins are fine. + + } elsif ($shellword =~ m"^[\w_]+=.*$") { + # Variable assignment. + + } elsif ($shellword =~ m"^\./.*$") { + # All commands from the current directory are fine. + + } elsif ($shellword =~ m"^#") { + my $semicolon = ($shellword =~ m";"); + my $multiline = ($line->lines =~ m"--"); + + if ($semicolon) { + $line->log_warning("A shell comment should not contain semicolons."); + } + if ($multiline) { + $line->log_warning("A shell comment does not stop at the end of line."); + } + + if ($semicolon || $multiline) { + $line->explain_warning( +"When you split a shell command into multiple lines that are continued", +"with a backslash, they will nevertheless be converted to a single line", +"before the shell sees them. That means that even if it _looks_ like that", +"the comment only spans one line in the Makefile, in fact it spans until", +"the end of the whole shell command. To insert a comment into shell code,", +"you can pass it as an argument to the \${SHCOMMENT} macro, which expands", +"to a command doing nothing. Note that any special characters are", +"nevertheless interpreted by the shell."); + } + + } else { + $opt_warn_extra and $line->log_warning("Unknown shell command \"${shellword}\"."); + $opt_warn_extra and $line->explain_warning( +"If you want your package to be portable to all platforms that pkgsrc", +"supports, you should only use shell commands that are covered by the", +"tools framework."); + + } + } + + if ($state == SCST_COND && $shellword eq "cd") { + $line->log_error("The Solaris /bin/sh cannot handle \"cd\" inside conditionals."); + $line->explain_error( +"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."); + } + + if (($state != SCST_PAX_S && $state != SCST_SED_E && $state != SCST_CASE_LABEL)) { + checkline_mk_absolute_pathname($line, $shellword); + } + + if (($state == SCST_INSTALL_D || $state == SCST_MKDIR) && $shellword =~ m"^(?:\$\{DESTDIR\})?\$\{PREFIX(?:|:Q)\}/") { + $line->log_warning("Please use AUTO_MKDIRS instead of " + . (($state == SCST_MKDIR) ? "\${MKDIR}" : "\${INSTALL} -d") + . "."); + $line->explain_warning( +"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}."); + } + + if (($state == SCST_INSTALL_DIR || $state == SCST_INSTALL_DIR2) && $shellword !~ regex_mk_shellvaruse && $shellword =~ m"^(?:\$\{DESTDIR\})?\$\{PREFIX(?:|:Q)\}/(.*)") { + my ($dirname) = ($1); + + $line->log_note("You can use AUTO_MKDIRS=yes or INSTALLATION_DIRS+= ${dirname} instead of this command."); + $line->explain_note( +"This saves you some typing. You also don't have to think about which of", +"the many INSTALL_*_DIR macros is appropriate, since INSTALLATION_DIRS", +"takes care of that.", +"", +"Note that you should only do this if the package creates _all_", +"directories it needs before trying to install files into them.", +"", +"Many packages include a list of all needed directories in their PLIST", +"file. In that case, you can just set AUTO_MKDIRS=yes and be done."); + } + + if ($state == SCST_INSTALL_DIR2 && $shellword =~ m"^\$") { + $line->log_warning("The INSTALL_*_DIR commands can only handle one directory at a time."); + $line->explain_warning( +"Many implementations of install(1) can handle more, but pkgsrc aims at", +"maximum portability."); + } + + if ($state == SCST_PAX && $shellword eq "-pe") { + $line->log_warning("Please use the -pp option to pax(1) instead of -pe."); + $line->explain_warning( +"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. That's a Bad Thing."); + } + + if ($state == SCST_PAX_S || $state == SCST_SED_E) { + if (false && $shellword !~ m"^[\"\'].*[\"\']$") { + $line->log_warning("Substitution commands like \"${shellword}\" should always be quoted."); + $line->explain_warning( +"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."); + } + } + + if ($state == SCST_ECHO && $shellword eq "-n") { + $line->log_warning("Please use \${ECHO_N} instead of \"echo -n\"."); + } + + if ($opt_warn_extra && $state != SCST_CASE_LABEL_CONT && $shellword eq "|") { + $line->log_warning("The exitcode of the left-hand-side command of the pipe operator is ignored."); + $line->explain_warning( +"If you need to detect the failure of the left-hand-side command, use", +"temporary files to save the output of the command."); + } + + if ($opt_warn_extra && $shellword eq ";" && $state != SCST_COND_CONT && $state != SCST_FOR_CONT && !$set_e_mode) { + $line->log_warning("Please switch to \"set -e\" mode before using a semicolon to separate commands."); + $line->explain_warning( +"Older versions of the NetBSD make(1) had run the shell commands using", +"the \"-e\" option of /bin/sh. In 2004, this behavior has been changed to", +"follow the POSIX conventions, which is to not use the \"-e\" option.", +"The consequence of this change is that shell programs don't terminate", +"as soon as an error occurs, but try to continue with the next command.", +"Imagine what would happen for these commands:", +" cd \"\$HOME\"; cd /nonexistent; rm -rf *", +"To fix this warning, either insert \"set -e\" at the beginning of this", +"line or use the \"&&\" operator instead of the semicolon."); + } + + # + # State transition. + # + + if ($state == SCST_SET && $shellword =~ m"^-.*e") { + $set_e_mode = true; + } + if ($state == SCST_START && $shellword eq "\${RUN}") { + $set_e_mode = true; + } + + $state = ($shellword eq ";;") ? SCST_CASE_LABEL + # Note: The order of the following two lines is important. + : ($state == SCST_CASE_LABEL_CONT && $shellword eq "|") ? SCST_CASE_LABEL + : ($shellword =~ m"^[;&\|]+$") ? SCST_START + : ($state == SCST_START) ? ( + ($shellword eq "\${INSTALL}") ? SCST_INSTALL + : ($shellword eq "\${MKDIR}") ? SCST_MKDIR + : ($shellword eq "\${PAX}") ? SCST_PAX + : ($shellword eq "\${SED}") ? SCST_SED + : ($shellword eq "\${ECHO}") ? SCST_ECHO + : ($shellword eq "\${RUN}") ? SCST_START + : ($shellword eq "echo") ? SCST_ECHO + : ($shellword eq "set") ? SCST_SET + : ($shellword =~ m"^(?:if|elif|while)$") ? SCST_COND + : ($shellword =~ m"^(?:then|else|do)$") ? SCST_START + : ($shellword eq "case") ? SCST_CASE + : ($shellword eq "for") ? SCST_FOR + : ($shellword eq "(") ? SCST_START + : ($shellword =~ m"^\$\{INSTALL_[A-Z]+_DIR\}$") ? SCST_INSTALL_DIR + : ($shellword =~ regex_sh_varassign) ? SCST_START + : SCST_CONT) + : ($state == SCST_MKDIR) ? SCST_MKDIR + : ($state == SCST_INSTALL && $shellword eq "-d") ? SCST_INSTALL_D + : ($state == SCST_INSTALL || $state == SCST_INSTALL_D) ? ( + ($shellword =~ m"^-[ogm]$") ? SCST_CONT + : $state) + : ($state == SCST_INSTALL_DIR) ? ( + ($shellword =~ m"^-") ? SCST_CONT + : ($shellword =~ m"^\$") ? SCST_INSTALL_DIR2 + : $state) + : ($state == SCST_INSTALL_DIR2) ? $state + : ($state == SCST_PAX) ? ( + ($shellword eq "-s") ? SCST_PAX_S + : ($shellword =~ m"^-") ? SCST_PAX + : SCST_CONT) + : ($state == SCST_PAX_S) ? SCST_PAX + : ($state == SCST_SED) ? ( + ($shellword eq "-e") ? SCST_SED_E + : ($shellword =~ m"^-") ? SCST_SED + : SCST_CONT) + : ($state == SCST_SED_E) ? SCST_SED + : ($state == SCST_SET) ? SCST_SET_CONT + : ($state == SCST_SET_CONT) ? SCST_SET_CONT + : ($state == SCST_CASE) ? SCST_CASE_IN + : ($state == SCST_CASE_IN && $shellword eq "in") ? SCST_CASE_LABEL + : ($state == SCST_CASE_LABEL && $shellword eq "esac") ? SCST_CONT + : ($state == SCST_CASE_LABEL) ? SCST_CASE_LABEL_CONT + : ($state == SCST_CASE_LABEL_CONT && $shellword eq ")") ? SCST_START + : ($state == SCST_CONT) ? SCST_CONT + : ($state == SCST_COND) ? SCST_COND_CONT + : ($state == SCST_COND_CONT) ? SCST_COND_CONT + : ($state == SCST_FOR) ? SCST_FOR_IN + : ($state == SCST_FOR_IN && $shellword eq "in") ? SCST_FOR_CONT + : ($state == SCST_FOR_CONT) ? SCST_FOR_CONT + : ($state == SCST_ECHO) ? SCST_CONT + : do { + $line->log_warning("[" . scst_statename->[$state] . " ${shellword}] Keeping the current state."); + $state; + }; + } + + if ($rest !~ m"^\s*$") { + $line->log_error("Internal pkglint error: " . scst_statename->[$state] . ": rest=${rest}"); + } +} + +sub checkline_mk_shellcmd($$) { + my ($line, $shellcmd) = @_; + + checkline_mk_text($line, $shellcmd); + checkline_mk_shelltext($line, $shellcmd); +} + diff --git a/pkgtools/pkglint/files/pkglint.pl b/pkgtools/pkglint/files/pkglint.pl index 138a0f335e7..637ef40e9de 100644 --- a/pkgtools/pkglint/files/pkglint.pl +++ b/pkgtools/pkglint/files/pkglint.pl @@ -1,5 +1,5 @@ #! @PERL@ -# $NetBSD: pkglint.pl,v 1.891 2015/10/15 01:57:47 rillig Exp $ +# $NetBSD: pkglint.pl,v 1.892 2015/10/15 02:09:49 rillig Exp $ # # pkglint - static analyzer and checker for pkgsrc packages @@ -1482,17 +1482,6 @@ sub strip_mk_comment($) { return $text; } -# Returns the value of a shell word, with one level of quoting removed. -# This makes pattern matching easier when the interesting part is not -# what make(1) or sh(1) sees, but the command that is called by the -# shell. This function does not resolve or replace any variables. -sub unescape_shellword($) { - my ($text) = @_; - - # TODO: implement this. - assert(false, "unescape_shellword is not yet implemented."); -} - # Removes all uses of make variables from a string. sub remove_variables($) { my ($text) = @_; @@ -2797,708 +2786,7 @@ sub checkline_mk_text($$) { } -sub checkline_mk_shellword($$$) { - my ($line, $shellword, $check_quoting) = @_; - my ($rest, $state, $value); - - $opt_debug_trace and $line->log_debug("checkline_mk_shellword(\"${shellword}\", ${check_quoting})."); - use constant shellcommand_context_type => PkgLint::Type->new( - LK_NONE, "ShellCommand", [[ qr".*", "adsu" ]], NOT_GUESSED - ); - use constant shellword_vuc => PkgLint::VarUseContext->new( - VUC_TIME_UNKNOWN, - shellcommand_context_type, - VUC_SHELLWORD_PLAIN, - VUC_EXTENT_WORD - ); - - if ($shellword =~ m"^\$\{(${regex_varname})(:[^{}]+)?\}$") { - my ($varname, $mod) = ($1, $2); - - checkline_mk_varuse($line, $varname, defined($mod) ? $mod : "", shellword_vuc); - return; - } - - if ($shellword =~ m"\$\{PREFIX\}/man(?:$|/)") { - $line->log_warning("Please use \${PKGMANDIR} instead of \"man\"."); - } - - if ($shellword =~ m"etc/rc\.d") { - $line->log_warning("Please use the RCD_SCRIPTS mechanism to install rc.d scripts automatically to \${RCD_SCRIPTS_EXAMPLEDIR}."); - } - - # Note: SWST means [S]hell[W]ord [ST]ate - use enum qw(:SWST_ PLAIN SQUOT DQUOT DQUOT_BACKT BACKT); - use constant statename => [ - "SWST_PLAIN", "SWST_SQUOT", "SWST_DQUOT", - "SWST_DQUOT_BACKT", "SWST_BACKT", - ]; - use constant user_statename => [ - "unquoted string", "single quoted string", - "double quoted string", "backticks inside double quoted string", - "backticks", - ]; - - $rest = ($shellword =~ m"^#") ? "" : $shellword; - $state = SWST_PLAIN; - while ($rest ne "") { - - $opt_debug_shell and $line->log_debug(statename->[$state] . ": ${rest}"); - - # When we are parsing inside backticks, it is more - # reasonable to check the whole shell command - # recursively, instead of splitting off the first - # make(1) variable (see the elsif below). - if ($state == SWST_BACKT || $state == SWST_DQUOT_BACKT) { - - # Scan for the end of the backticks, checking - # for single backslashes and removing one level - # of backslashes. Backslashes are only removed - # before a dollar, a backslash or a backtick. - # - # References: - # * http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_06_03 - my $stripped = ""; - while ($rest ne "") { - if ($rest =~ s/^\`//) { - $state = ($state == SWST_BACKT) ? SWST_PLAIN : SWST_DQUOT; - goto end_of_backquotes; - } elsif ($rest =~ s/^\\([\\\`\$])//) { - $stripped .= $1; - } elsif ($rest =~ s/^(\\)//) { - $line->log_warning("Backslashes should be doubled inside backticks."); - $stripped .= $1; - } elsif ($state == SWST_DQUOT_BACKT && $rest =~ s/^"//) { - $line->log_warning("Double quotes inside backticks inside double quotes are error prone."); - $line->explain_warning( -"According to the SUSv3, they produce undefined results.", -"", -"See the paragraph starting \"Within the backquoted ...\" in", -"http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html"); - } elsif ($rest =~ s/^([^\\\`]+)//) { - $stripped .= $1; - } else { - assert(false, "rest=$rest"); - } - } - $line->log_error("Unfinished backquotes: rest=$rest"); - - end_of_backquotes: - # Check the resulting command. - checkline_mk_shelltext($line, $stripped); - - # make(1) variables have the same syntax, no matter in - # which state we are currently. - } elsif ($rest =~ s/^\$\{(${regex_varname}|[\@])(:[^\{]+)?\}// - || $rest =~ s/^\$\((${regex_varname}|[\@])(:[^\)]+)?\)// - || $rest =~ s/^\$([\w\@])//) { - my ($varname, $mod) = ($1, $2); - - if ($varname eq "\@") { - $line->log_warning("Please use \"\${.TARGET}\" instead of \"\$\@\"."); - $line->explain_warning( -"The variable \$\@ can easily be confused with the shell variable of the", -"same name, which has a completely different meaning."); - $varname = ".TARGET"; - } - - if ($state == SWST_PLAIN && defined($mod) && $mod =~ m":Q$") { - # Fine. - - } elsif ($state == SWST_BACKT) { - # Don't check here, to avoid false positives - # for tool names. - - } elsif (($state == SWST_SQUOT || $state == SWST_DQUOT) && $varname =~ m"^(?:.*DIR|.*FILE|.*PATH|.*_VAR|PREFIX|.*BASE|PKGNAME)$") { - # This is ok if we don't allow these - # variables to have embedded [\$\\\"\'\`]. - - } elsif ($state == SWST_DQUOT && defined($mod) && $mod =~ m":Q$") { - $line->log_warning("Please don't use the :Q operator in double quotes."); - $line->explain_warning( -"Either remove the :Q or the double quotes. In most cases, it is more", -"appropriate to remove the double quotes."); - - } - - my $ctx = PkgLint::VarUseContext->new_from_pool( - VUC_TIME_UNKNOWN, - shellcommand_context_type, - ($state == SWST_PLAIN) ? VUC_SHELLWORD_PLAIN - : ($state == SWST_DQUOT) ? VUC_SHELLWORD_DQUOT - : ($state == SWST_SQUOT) ? VUC_SHELLWORD_SQUOT - : ($state == SWST_BACKT) ? VUC_SHELLWORD_BACKT - : VUC_SHELLWORD_UNKNOWN, - VUC_EXTENT_WORD_PART - ); - if ($varname ne "\@") { - checkline_mk_varuse($line, $varname, defined($mod) ? $mod : "", $ctx); - } - - # The syntax of the variable modifiers can get quite - # hairy. In lack of motivation, we just skip anything - # complicated, hoping that at least the braces are - # balanced. - } elsif ($rest =~ s/^\$\{//) { - my $braces = 1; - while ($rest ne "" && $braces > 0) { - if ($rest =~ s/^\}//) { - $braces--; - } elsif ($rest =~ s/^\{//) { - $braces++; - } elsif ($rest =~ s/^[^{}]+//) { - # Nothing to do here. - } else { - last; - } - } - - } elsif ($state == SWST_PLAIN) { - - # XXX: This is only true for the "first" word in a - # shell command, not for every word. For example, - # FOO_ENV+= VAR=`command` may be underquoted. - if (false && $rest =~ m"([\w_]+)=\"\`") { - $line->log_note("In the assignment to \"$1\", you don't need double quotes around backticks."); - $line->explain_note( -"Assignments are a special context, where no double quotes are needed", -"around backticks. In other contexts, the double quotes are necessary."); - } - - if ($rest =~ s/^[!#\%&\(\)*+,\-.\/0-9:;<=>?\@A-Z\[\]^_a-z{|}~]+//) { - } elsif ($rest =~ s/^\'//) { - $state = SWST_SQUOT; - } elsif ($rest =~ s/^\"//) { - $state = SWST_DQUOT; - } elsif ($rest =~ s/^\`//) { - $state = SWST_BACKT; - } elsif ($rest =~ s/^\\(?:[ !"#'\(\)*;?\\^{|}]|\$\$)//) { - } elsif ($rest =~ s/^\$\$([0-9A-Z_a-z]+|\#)// - || $rest =~ s/^\$\$\{([0-9A-Z_a-z]+|\#)\}// - || $rest =~ s/^\$\$(\$)\$//) { - my ($shvarname) = ($1); - if ($opt_warn_quoting && $check_quoting) { - $line->log_warning("Unquoted shell variable \"${shvarname}\"."); - $line->explain_warning( -"When a shell variable contains white-space, it is expanded (split into", -"multiple words) when it is written as \$variable in a shell script.", -"If that is not intended, you should add quotation marks around it,", -"like \"\$variable\". Then, the variable will always expand to a single", -"word, preserving all white-space and other special characters.", -"", -"Example:", -"\tfname=\"Curriculum vitae.doc\"", -"\tcp \$fname /tmp", -"\t# tries to copy the two files \"Curriculum\" and \"Vitae.doc\"", -"\tcp \"\$fname\" /tmp", -"\t# copies one file, as intended"); - } - - } elsif ($rest =~ s/^\$\@//) { - $line->log_warning("Please use \"\${.TARGET}\" instead of \"\$@\"."); - $line->explain_warning( -"It is more readable and prevents confusion with the shell variable of", -"the same name."); - - } elsif ($rest =~ s/^\$\$\@//) { - $line->log_warning("The \$@ shell variable should only be used in double quotes."); - - } elsif ($rest =~ s/^\$\$\?//) { - $line->log_warning("The \$? shell variable is often not available in \"set -e\" mode."); - - } elsif ($rest =~ s/^\$\$\(/(/) { - $line->log_warning("Invoking subshells via \$(...) is not portable enough."); - $line->explain_warning( -"The Solaris /bin/sh does not know this way to execute a command in a", -"subshell. Please use backticks (\`...\`) as a replacement."); - - } else { - last; - } - - } elsif ($state == SWST_SQUOT) { - if ($rest =~ s/^\'//) { - $state = SWST_PLAIN; - } elsif ($rest =~ s/^[^\$\']+//) { - } elsif ($rest =~ s/^\$\$//) { - } else { - last; - } - - } elsif ($state == SWST_DQUOT) { - if ($rest =~ s/^\"//) { - $state = SWST_PLAIN; - } elsif ($rest =~ s/^\`//) { - $state = SWST_DQUOT_BACKT; - } elsif ($rest =~ s/^[^\$"\\\`]+//) { - } elsif ($rest =~ s/^\\(?:[\\\"\`]|\$\$)//) { - } elsif ($rest =~ s/^\$\$\{([0-9A-Za-z_]+)\}// - || $rest =~ s/^\$\$([0-9A-Z_a-z]+|[!#?\@]|\$\$)//) { - my ($shvarname) = ($1); - $opt_debug_shell and $line->log_debug("[checkline_mk_shellword] Found double-quoted variable ${shvarname}."); - } elsif ($rest =~ s/^\$\$//) { - $line->log_warning("Unquoted \$ or strange shell variable found."); - } elsif ($rest =~ s/^\\(.)//) { - my ($char) = ($1); - $line->log_warning("Please use \"\\\\${char}\" instead of \"\\${char}\"."); - $line->explain_warning( -"Although the current code may work, it is not good style to rely on", -"the shell passing \"\\${char}\" exactly as is, and not discarding the", -"backslash. Alternatively you can use single quotes instead of double", -"quotes."); - } else { - last; - } - - } else { - last; - } - } - if ($rest !~ m"^\s*$") { - $line->log_error("Internal pkglint error: " . statename->[$state] . ": rest=${rest}"); - } -} - -# Some shell commands should not be used in the install phase. -# -sub checkline_mk_shellcmd_use($$) { - my ($line, $shellcmd) = @_; - - use constant allowed_install_commands => array_to_hash(qw( - ${INSTALL} - ${INSTALL_DATA} ${INSTALL_DATA_DIR} - ${INSTALL_LIB} ${INSTALL_LIB_DIR} - ${INSTALL_MAN} ${INSTALL_MAN_DIR} - ${INSTALL_PROGRAM} ${INSTALL_PROGRAM_DIR} - ${INSTALL_SCRIPT} - ${LIBTOOL} - ${LN} - ${PAX} - )); - use constant discouraged_install_commands => array_to_hash(qw( - sed ${SED} - tr ${TR} - )); - - if (defined($mkctx_target) && $mkctx_target =~ m"^(?:pre|do|post)-install") { - - if (exists(allowed_install_commands->{$shellcmd})) { - # Fine. - - } elsif (exists(discouraged_install_commands->{$shellcmd})) { - $line->log_warning("The shell command \"${shellcmd}\" should not be used in the install phase."); - $line->explain_warning( -"In the install phase, the only thing that should be done is to install", -"the prepared files to their final location. The file's contents should", -"not be changed anymore."); - - } elsif ($shellcmd eq "\${CP}") { - $line->log_warning("\${CP} should not be used to install files."); - $line->explain_warning( -"The \${CP} command is highly platform dependent and cannot overwrite", -"files that don't have write permission. Please use \${PAX} instead.", -"", -"For example, instead of", -"\t\${CP} -R \${WRKSRC}/* \${PREFIX}/foodir", -"you should use", -"\tcd \${WRKSRC} && \${PAX} -wr * \${PREFIX}/foodir"); - - } else { - $opt_debug_misc and $line->log_debug("May \"${shellcmd}\" be used in the install phase?"); - } - } -} - -sub checkline_mk_shelltext($$) { - my ($line, $text) = @_; - my ($vartools, $state, $rest, $set_e_mode); - - $opt_debug_trace and $line->log_debug("checkline_mk_shelltext(\"$text\")"); - - # Note: SCST is the abbreviation for [S]hell [C]ommand [ST]ate. - use constant scst => qw( - START CONT - INSTALL INSTALL_D - MKDIR - PAX PAX_S - SED SED_E - SET SET_CONT - COND COND_CONT - CASE CASE_IN CASE_LABEL CASE_LABEL_CONT CASE_PAREN - FOR FOR_IN FOR_CONT - ECHO - INSTALL_DIR INSTALL_DIR2 - ); - use enum (":SCST_", scst); - use constant scst_statename => [ map { "SCST_$_" } scst ]; - - use constant forbidden_commands => array_to_hash(qw( - ktrace - mktexlsr - strace - texconfig truss - )); - - if ($text =~ m"\$\{SED\}" && $text =~ m"\$\{MV\}") { - $line->log_note("Please use the SUBST framework instead of \${SED} and \${MV}."); - $line->explain_note( -"When converting things, pay attention to \"#\" characters. In shell", -"commands make(1) does not interpret them as comment character, but", -"in other lines it does. Therefore, instead of the shell command", -"", -"\tsed -e 's,#define foo,,'", -"", -"you need to write", -"", -"\tSUBST_SED.foo+=\t's,\\#define foo,,'"); - } - - if ($text =~ m"^\@*-(.*(MKDIR|INSTALL.*-d|INSTALL_.*_DIR).*)") { - my ($mkdir_cmd) = ($1); - - $line->log_note("You don't need to use \"-\" before ${mkdir_cmd}."); - } - - $vartools = get_vartool_names(); - $rest = $text; - - use constant hidden_shell_commands => array_to_hash(qw( - ${DELAYED_ERROR_MSG} ${DELAYED_WARNING_MSG} - ${DO_NADA} - ${ECHO} ${ECHO_MSG} ${ECHO_N} ${ERROR_CAT} ${ERROR_MSG} - ${FAIL_MSG} - ${PHASE_MSG} ${PRINTF} - ${SHCOMMENT} ${STEP_MSG} - ${WARNING_CAT} ${WARNING_MSG} - )); - - $set_e_mode = false; - - if ($rest =~ s/^\s*([-@]*)(\$\{_PKG_SILENT\}\$\{_PKG_DEBUG\}|\$\{RUN\}|)//) { - my ($hidden, $macro) = ($1, $2); - - if ($hidden !~ m"\@") { - # Nothing is hidden at all. - - } elsif (defined($mkctx_target) && $mkctx_target =~ m"^(?:show-.*|.*-message)$") { - # In some targets commands may be hidden. - - } elsif ($rest =~ m"^#") { - # Shell comments may be hidden, as they have no side effects - - } elsif ($rest =~ $regex_shellword) { - my ($cmd) = ($1); - - if (!exists(hidden_shell_commands->{$cmd})) { - $line->log_warning("The shell command \"${cmd}\" should not be hidden."); - $line->explain_warning( -"Hidden shell commands do not appear on the terminal or in the log file", -"when they are executed. When they fail, the error message cannot be", -"assigned to the command, which is very difficult to debug."); - } - } - - if ($hidden =~ m"-") { - $line->log_warning("The use of a leading \"-\" to suppress errors is deprecated."); - $line->explain_warning( -"If you really want to ignore any errors from this command (including", -"all errors you never thought of), append \"|| \${TRUE}\" to the", -"command."); - } - - if ($macro eq "\${RUN}") { - $set_e_mode = true; - } - } - - $state = SCST_START; - while ($rest =~ s/^$regex_shellword//) { - my ($shellword) = ($1); - - $opt_debug_shell and $line->log_debug(scst_statename->[$state] . ": ${shellword}"); - - checkline_mk_shellword($line, $shellword, !( - $state == SCST_CASE - || $state == SCST_FOR_CONT - || $state == SCST_SET_CONT - || ($state == SCST_START && $shellword =~ regex_sh_varassign))); - - # - # Actions associated with the current state - # and the symbol on the "input tape". - # - - if ($state == SCST_START || $state == SCST_COND) { - my ($type); - - if ($shellword eq "\${RUN}") { - # Just skip this one. - - } elsif (exists(forbidden_commands->{basename($shellword)})) { - $line->log_error("${shellword} must not be used in Makefiles."); - $line->explain_error( -"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", -"package via pkg_add."); - - } elsif (exists(get_tool_names()->{$shellword})) { - if (!exists($mkctx_tools->{$shellword}) && !exists($mkctx_tools->{"g$shellword"})) { - $line->log_warning("The \"${shellword}\" tool is used but not added to USE_TOOLS."); - } - - if (exists(get_required_vartools()->{$shellword})) { - $line->log_warning("Please use \"\${" . get_vartool_names()->{$shellword} . "}\" instead of \"${shellword}\"."); - } - - checkline_mk_shellcmd_use($line, $shellword); - - } elsif ($shellword =~ m"^\$\{([\w_]+)\}$" && exists(get_varname_to_toolname()->{$1})) { - my ($vartool) = ($1); - my $plain_tool = get_varname_to_toolname()->{$vartool}; - - if (!exists($mkctx_tools->{$plain_tool})) { - $line->log_warning("The \"${plain_tool}\" tool is used but not added to USE_TOOLS."); - } - - # Deactivated to allow package developers to write - # consistent code that uses ${TOOL} in all places. - if (false && defined($mkctx_target) && $mkctx_target =~ m"^(?:pre|do|post)-(?:extract|patch|wrapper|configure|build|install|package|clean)$") { - if (!exists(get_required_vartool_varnames()->{$vartool})) { - $opt_warn_extra and $line->log_note("You can write \"${plain_tool}\" instead of \"${shellword}\"."); - $opt_warn_extra and $line->explain_note( -"The wrapper framework from pkgsrc takes care that a sufficiently", -"capable implementation of that tool will be selected.", -"", -"Calling the commands by their plain name instead of the macros is", -"only available in the {pre,do,post}-* targets. For all other targets,", -"you should still use the macros."); - } - } - - checkline_mk_shellcmd_use($line, $shellword); - - } elsif ($shellword =~ m"^\$\{([\w_]+)\}$" && defined($type = get_variable_type($line, $1)) && $type->basic_type eq "ShellCommand") { - checkline_mk_shellcmd_use($line, $shellword); - - } elsif ($shellword =~ m"^\$\{(\w+)\}$" && defined($pkgctx_vardef) && exists($pkgctx_vardef->{$1})) { - # When the package author has explicitly - # defined a command variable, assume it - # to be valid. - - } elsif ($shellword =~ m"^(?:\(|\)|:|;|;;|&&|\|\||\{|\}|break|case|cd|continue|do|done|elif|else|esac|eval|exec|exit|export|fi|for|if|read|set|shift|then|umask|unset|while)$") { - # Shell builtins are fine. - - } elsif ($shellword =~ m"^[\w_]+=.*$") { - # Variable assignment. - - } elsif ($shellword =~ m"^\./.*$") { - # All commands from the current directory are fine. - - } elsif ($shellword =~ m"^#") { - my $semicolon = ($shellword =~ m";"); - my $multiline = ($line->lines =~ m"--"); - - if ($semicolon) { - $line->log_warning("A shell comment should not contain semicolons."); - } - if ($multiline) { - $line->log_warning("A shell comment does not stop at the end of line."); - } - - if ($semicolon || $multiline) { - $line->explain_warning( -"When you split a shell command into multiple lines that are continued", -"with a backslash, they will nevertheless be converted to a single line", -"before the shell sees them. That means that even if it _looks_ like that", -"the comment only spans one line in the Makefile, in fact it spans until", -"the end of the whole shell command. To insert a comment into shell code,", -"you can pass it as an argument to the \${SHCOMMENT} macro, which expands", -"to a command doing nothing. Note that any special characters are", -"nevertheless interpreted by the shell."); - } - - } else { - $opt_warn_extra and $line->log_warning("Unknown shell command \"${shellword}\"."); - $opt_warn_extra and $line->explain_warning( -"If you want your package to be portable to all platforms that pkgsrc", -"supports, you should only use shell commands that are covered by the", -"tools framework."); - - } - } - - if ($state == SCST_COND && $shellword eq "cd") { - $line->log_error("The Solaris /bin/sh cannot handle \"cd\" inside conditionals."); - $line->explain_error( -"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."); - } - - if (($state != SCST_PAX_S && $state != SCST_SED_E && $state != SCST_CASE_LABEL)) { - checkline_mk_absolute_pathname($line, $shellword); - } - - if (($state == SCST_INSTALL_D || $state == SCST_MKDIR) && $shellword =~ m"^(?:\$\{DESTDIR\})?\$\{PREFIX(?:|:Q)\}/") { - $line->log_warning("Please use AUTO_MKDIRS instead of " - . (($state == SCST_MKDIR) ? "\${MKDIR}" : "\${INSTALL} -d") - . "."); - $line->explain_warning( -"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}."); - } - - if (($state == SCST_INSTALL_DIR || $state == SCST_INSTALL_DIR2) && $shellword !~ regex_mk_shellvaruse && $shellword =~ m"^(?:\$\{DESTDIR\})?\$\{PREFIX(?:|:Q)\}/(.*)") { - my ($dirname) = ($1); - - $line->log_note("You can use AUTO_MKDIRS=yes or INSTALLATION_DIRS+= ${dirname} instead of this command."); - $line->explain_note( -"This saves you some typing. You also don't have to think about which of", -"the many INSTALL_*_DIR macros is appropriate, since INSTALLATION_DIRS", -"takes care of that.", -"", -"Note that you should only do this if the package creates _all_", -"directories it needs before trying to install files into them.", -"", -"Many packages include a list of all needed directories in their PLIST", -"file. In that case, you can just set AUTO_MKDIRS=yes and be done."); - } - - if ($state == SCST_INSTALL_DIR2 && $shellword =~ m"^\$") { - $line->log_warning("The INSTALL_*_DIR commands can only handle one directory at a time."); - $line->explain_warning( -"Many implementations of install(1) can handle more, but pkgsrc aims at", -"maximum portability."); - } - - if ($state == SCST_PAX && $shellword eq "-pe") { - $line->log_warning("Please use the -pp option to pax(1) instead of -pe."); - $line->explain_warning( -"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. That's a Bad Thing."); - } - - if ($state == SCST_PAX_S || $state == SCST_SED_E) { - if (false && $shellword !~ m"^[\"\'].*[\"\']$") { - $line->log_warning("Substitution commands like \"${shellword}\" should always be quoted."); - $line->explain_warning( -"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."); - } - } - - if ($state == SCST_ECHO && $shellword eq "-n") { - $line->log_warning("Please use \${ECHO_N} instead of \"echo -n\"."); - } - - if ($opt_warn_extra && $state != SCST_CASE_LABEL_CONT && $shellword eq "|") { - $line->log_warning("The exitcode of the left-hand-side command of the pipe operator is ignored."); - $line->explain_warning( -"If you need to detect the failure of the left-hand-side command, use", -"temporary files to save the output of the command."); - } - - if ($opt_warn_extra && $shellword eq ";" && $state != SCST_COND_CONT && $state != SCST_FOR_CONT && !$set_e_mode) { - $line->log_warning("Please switch to \"set -e\" mode before using a semicolon to separate commands."); - $line->explain_warning( -"Older versions of the NetBSD make(1) had run the shell commands using", -"the \"-e\" option of /bin/sh. In 2004, this behavior has been changed to", -"follow the POSIX conventions, which is to not use the \"-e\" option.", -"The consequence of this change is that shell programs don't terminate", -"as soon as an error occurs, but try to continue with the next command.", -"Imagine what would happen for these commands:", -" cd \"\$HOME\"; cd /nonexistent; rm -rf *", -"To fix this warning, either insert \"set -e\" at the beginning of this", -"line or use the \"&&\" operator instead of the semicolon."); - } - - # - # State transition. - # - - if ($state == SCST_SET && $shellword =~ m"^-.*e") { - $set_e_mode = true; - } - if ($state == SCST_START && $shellword eq "\${RUN}") { - $set_e_mode = true; - } - - $state = ($shellword eq ";;") ? SCST_CASE_LABEL - # Note: The order of the following two lines is important. - : ($state == SCST_CASE_LABEL_CONT && $shellword eq "|") ? SCST_CASE_LABEL - : ($shellword =~ m"^[;&\|]+$") ? SCST_START - : ($state == SCST_START) ? ( - ($shellword eq "\${INSTALL}") ? SCST_INSTALL - : ($shellword eq "\${MKDIR}") ? SCST_MKDIR - : ($shellword eq "\${PAX}") ? SCST_PAX - : ($shellword eq "\${SED}") ? SCST_SED - : ($shellword eq "\${ECHO}") ? SCST_ECHO - : ($shellword eq "\${RUN}") ? SCST_START - : ($shellword eq "echo") ? SCST_ECHO - : ($shellword eq "set") ? SCST_SET - : ($shellword =~ m"^(?:if|elif|while)$") ? SCST_COND - : ($shellword =~ m"^(?:then|else|do)$") ? SCST_START - : ($shellword eq "case") ? SCST_CASE - : ($shellword eq "for") ? SCST_FOR - : ($shellword eq "(") ? SCST_START - : ($shellword =~ m"^\$\{INSTALL_[A-Z]+_DIR\}$") ? SCST_INSTALL_DIR - : ($shellword =~ regex_sh_varassign) ? SCST_START - : SCST_CONT) - : ($state == SCST_MKDIR) ? SCST_MKDIR - : ($state == SCST_INSTALL && $shellword eq "-d") ? SCST_INSTALL_D - : ($state == SCST_INSTALL || $state == SCST_INSTALL_D) ? ( - ($shellword =~ m"^-[ogm]$") ? SCST_CONT - : $state) - : ($state == SCST_INSTALL_DIR) ? ( - ($shellword =~ m"^-") ? SCST_CONT - : ($shellword =~ m"^\$") ? SCST_INSTALL_DIR2 - : $state) - : ($state == SCST_INSTALL_DIR2) ? $state - : ($state == SCST_PAX) ? ( - ($shellword eq "-s") ? SCST_PAX_S - : ($shellword =~ m"^-") ? SCST_PAX - : SCST_CONT) - : ($state == SCST_PAX_S) ? SCST_PAX - : ($state == SCST_SED) ? ( - ($shellword eq "-e") ? SCST_SED_E - : ($shellword =~ m"^-") ? SCST_SED - : SCST_CONT) - : ($state == SCST_SED_E) ? SCST_SED - : ($state == SCST_SET) ? SCST_SET_CONT - : ($state == SCST_SET_CONT) ? SCST_SET_CONT - : ($state == SCST_CASE) ? SCST_CASE_IN - : ($state == SCST_CASE_IN && $shellword eq "in") ? SCST_CASE_LABEL - : ($state == SCST_CASE_LABEL && $shellword eq "esac") ? SCST_CONT - : ($state == SCST_CASE_LABEL) ? SCST_CASE_LABEL_CONT - : ($state == SCST_CASE_LABEL_CONT && $shellword eq ")") ? SCST_START - : ($state == SCST_CONT) ? SCST_CONT - : ($state == SCST_COND) ? SCST_COND_CONT - : ($state == SCST_COND_CONT) ? SCST_COND_CONT - : ($state == SCST_FOR) ? SCST_FOR_IN - : ($state == SCST_FOR_IN && $shellword eq "in") ? SCST_FOR_CONT - : ($state == SCST_FOR_CONT) ? SCST_FOR_CONT - : ($state == SCST_ECHO) ? SCST_CONT - : do { - $line->log_warning("[" . scst_statename->[$state] . " ${shellword}] Keeping the current state."); - $state; - }; - } - - if ($rest !~ m"^\s*$") { - $line->log_error("Internal pkglint error: " . scst_statename->[$state] . ": rest=${rest}"); - } -} - -sub checkline_mk_shellcmd($$) { - my ($line, $shellcmd) = @_; - - checkline_mk_text($line, $shellcmd); - checkline_mk_shelltext($line, $shellcmd); -} +#include PkgLint/Shell.pm sub expand_permission($) { my ($perm) = @_; |