pkgsrc-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[pkgsrc/trunk]: pkgsrc mk/tools: correctly quote arguments in the tool wrappe...



details:   https://anonhg.NetBSD.org/pkgsrc/rev/85f5dcfa7fdd
branches:  trunk
changeset: 331730:85f5dcfa7fdd
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Sun Mar 24 11:29:19 2019 +0000

description:
mk/tools: correctly quote arguments in the tool wrapper log

Before, the tool arguments were written to the log as plain strings. Now
the arguments are properly quoted, which makes it possible to replay the
commands by copying them from the .work.log file.

This only affects tools that are shell builtins (echo, true, false), get
additional arguments (mkdir -p) or define a custom TOOLS_SCRIPT
(pkg-config, to set an environment variable; or autotools). Tools that
are symlinked to the real tool are not affected.

The calls to the compiler are already properly logged since cwrappers
takes care of that. This commit therefore makes the log entries for the
compilers and the other tools more similar.

diffstat:

 mk/tools/create.mk                  |  19 ++++++----
 mk/tools/shquote.sh                 |  27 +++++++++++++++
 regress/tools/Makefile              |  12 +++---
 regress/tools/files/logging-test.sh |  65 ++++++++++++++----------------------
 regress/tools/files/shquote-test.sh |  54 ++++++++++++++++++++++++++++++
 5 files changed, 123 insertions(+), 54 deletions(-)

diffs (truncated from 314 to 300 lines):

diff -r 38fdac967e50 -r 85f5dcfa7fdd mk/tools/create.mk
--- a/mk/tools/create.mk        Sun Mar 24 10:41:28 2019 +0000
+++ b/mk/tools/create.mk        Sun Mar 24 11:29:19 2019 +0000
@@ -1,4 +1,4 @@
-# $NetBSD: create.mk,v 1.10 2019/03/24 08:40:07 rillig Exp $
+# $NetBSD: create.mk,v 1.11 2019/03/24 11:29:19 rillig Exp $
 #
 # Copyright (c) 2005, 2006 The NetBSD Foundation, Inc.
 # All rights reserved.
@@ -149,7 +149,7 @@
        if ${TEST} -n ${TOOLS_SCRIPT.${_t_}:Q}""; then                  \
                create=wrapper;                                         \
                script=${TOOLS_SCRIPT.${_t_}:Q};                        \
-               logprefix='"set args "$$@"; shift; "';                  \
+               logprefix='"set args$$shquoted_args; shift; "';         \
                logmain=${TOOLS_SCRIPT.${_t_}:Q:Q};                     \
                logsuffix='';                                           \
        elif ${TEST} -n ${TOOLS_PATH.${_t_}:Q}""; then                  \
@@ -158,7 +158,7 @@
                        script=${TOOLS_SCRIPT_DFLT.${_t_}:Q};           \
                        logprefix='';                                   \
                        logmain=${TOOLS_PATH.${_t_}:Q:Q}\"\ \"${TOOLS_ARGS.${_t_}:Q:Q}; \
-                       logsuffix=' "$$*"';                             \
+                       logsuffix='$$shquoted_args';                    \
                else                                                    \
                        case ${TOOLS_PATH.${_t_}:Q}"" in                \
                        /*)     create=symlink ;;                       \
@@ -166,7 +166,7 @@
                                script=${TOOLS_SCRIPT_DFLT.${_t_}:Q};   \
                                logprefix='';                           \
                                logmain=${TOOLS_PATH.${_t_}:Q:Q}\"\ \"; \
-                               logsuffix=' "$$*"';                     \
+                               logsuffix='$$shquoted_args';            \
                        esac;                                           \
                fi;                                                     \
        else                                                            \
@@ -175,11 +175,14 @@
        case "$$create" in                                              \
        wrapper)                                                        \
                { ${ECHO} '#!'${TOOLS_SHELL:Q};                         \
+                 ${ECHO} 'tools_wrapper_sed='${SED:Q:Q};               \
+                 ${SED} -e '/^$$/d' -e '/^\#/d' ${PKGSRCDIR}/mk/tools/shquote.sh; \
                  ${ECHO} 'wrapperlog="$${TOOLS_WRAPPER_LOG-'${_TOOLS_WRAP_LOG:Q}'}"'; \
-                 ${ECHO} '${ECHO} "[*] "'${.TARGET:Q}'" $$*" >> $$wrapperlog'; \
-                 ${ECHO} 'logprefix='$$logprefix; \
-                 ${ECHO} 'logmain='$$logmain; \
-                 ${ECHO} "${ECHO} '<.>' \"\$$logprefix\$$logmain\"$$logsuffix >> \$$wrapperlog"; \
+                 ${ECHO} 'shquote_args "$$@"';                         \
+                 ${ECHO} '${ECHO} "[*] "'${.TARGET:Q}'"$$shquoted_args" >> $$wrapperlog'; \
+                 ${ECHO} 'logprefix='$$logprefix;                      \
+                 ${ECHO} 'logmain='$$logmain;                          \
+                 ${ECHO} "${ECHO} '<.>' \"\$$logprefix\$$logmain$$logsuffix\" >> \$$wrapperlog"; \
                  ${ECHO} "$$script";                                   \
                } > ${.TARGET:Q};                                       \
                ${CHMOD} +x ${.TARGET:Q};                               \
diff -r 38fdac967e50 -r 85f5dcfa7fdd mk/tools/shquote.sh
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/mk/tools/shquote.sh       Sun Mar 24 11:29:19 2019 +0000
@@ -0,0 +1,27 @@
+#! /bin/sh
+# $NetBSD: shquote.sh,v 1.1 2019/03/24 11:29:19 rillig Exp $
+
+# Quotes all shell meta characters from $1 and writes the result to $shquoted.
+shquote()
+{
+       shquoted=$1
+       case $shquoted in
+       *\'*)
+               shquoted=`$tools_wrapper_sed -e 's,'\'','\''\\\\'\'''\'',g' <<EOF
+$shquoted
+EOF`
+       esac
+
+       case $shquoted in
+       *[!!%+,\-./0-9:=@A-Z_a-z]*|'')
+               shquoted="'$shquoted'"
+       esac
+}
+
+shquote_args() {
+       shquoted_args=''
+       for arg in "$@"; do
+               shquote "$arg"
+               shquoted_args="$shquoted_args $shquoted"
+       done
+}
diff -r 38fdac967e50 -r 85f5dcfa7fdd regress/tools/Makefile
--- a/regress/tools/Makefile    Sun Mar 24 10:41:28 2019 +0000
+++ b/regress/tools/Makefile    Sun Mar 24 11:29:19 2019 +0000
@@ -1,4 +1,4 @@
-# $NetBSD: Makefile,v 1.14 2019/03/24 08:40:07 rillig Exp $
+# $NetBSD: Makefile,v 1.15 2019/03/24 11:29:19 rillig Exp $
 #
 
 DISTNAME=      # not applicable
@@ -14,7 +14,7 @@
 WRKSRC=                ${WRKDIR}
 NO_CHECKSUM=   yes
 PLIST_SRC=     # none
-REGRESS_TESTS+=        logging
+REGRESS_TESTS+=        logging shquote
 REGRESS_TESTS+=        awk sed sh sort tar tr
 USE_TOOLS+=    awk sed sh sort tar tr
 
@@ -56,10 +56,10 @@
 TOOLS_ARGS.path-args=  " \"'\\$$" "*"
 
 do-build:
-.for t in ${REGRESS_TESTS}
-       ${RUN} cd ${WRKSRC};                                            \
-       ${ECHO_MSG} "Running testsuite "${t:Q};                         \
-       ${SH} ${FILESDIR}/${t:Q}-test.sh
+.for test in ${REGRESS_TESTS}
+       @${ECHO_MSG} "Running testsuite "${test:Q}
+       ${RUN} cd ${WRKSRC} \
+       && PKGSRCDIR=${PKGSRCDIR} ${SH} ${FILESDIR}/${test:Q}-test.sh
 .endfor
 
 .include "../../mk/bsd.pkg.mk"
diff -r 38fdac967e50 -r 85f5dcfa7fdd regress/tools/files/logging-test.sh
--- a/regress/tools/files/logging-test.sh       Sun Mar 24 10:41:28 2019 +0000
+++ b/regress/tools/files/logging-test.sh       Sun Mar 24 11:29:19 2019 +0000
@@ -1,14 +1,9 @@
 #! /bin/sh
-# $NetBSD: logging-test.sh,v 1.5 2019/03/24 08:40:08 rillig Exp $
+# $NetBSD: logging-test.sh,v 1.6 2019/03/24 11:29:19 rillig Exp $
 
 # Up to March 2019, the command logging for the wrapped tools didn't properly
 # quote the command line arguments. This meant the logging did not reflect
 # the actual tool command line.
-#
-# As of March 2019 the logging has been fixed for tool wrappers that consist
-# only of a TOOLS_PATH.${tool} and TOOLS_ARGS.${tool}. For tools with custom
-# TOOLS_SCRIPTS it's much more difficult to do the quoting properly. See the
-# wrapper for makeinfo for a good example.
 
 set -eu
 
@@ -64,17 +59,15 @@
        # argument. This is because the echo command doesn't get any
        # additional arguments by the tool wrapper (TOOLS_ARGS.echo).
 
-       # TODO: To avoid unintended file expansion when replaying the
-       # commands, all arguments must be shquoted.
        assert_log <<'EOF'
-[*] WRKDIR/.tools/bin/echo begin * * * end
-<.> echo  begin * * * end
-[*] WRKDIR/.tools/bin/echo dquot " end
-<.> echo  dquot " end
-[*] WRKDIR/.tools/bin/echo squot ' end
-<.> echo  squot ' end
-[*] WRKDIR/.tools/bin/echo five \\\\\ end
-<.> echo  five \\\\\ end
+[*] WRKDIR/.tools/bin/echo begin '*' '*' '*' end
+<.> echo  begin '*' '*' '*' end
+[*] WRKDIR/.tools/bin/echo dquot '"' end
+<.> echo  dquot '"' end
+[*] WRKDIR/.tools/bin/echo squot ''\''' end
+<.> echo  squot ''\''' end
+[*] WRKDIR/.tools/bin/echo five '\\\\\' end
+<.> echo  five '\\\\\' end
 EOF
 }
 
@@ -84,13 +77,12 @@
 
        run_tool mkdir "directory with spaces"
 
-       # The log doesn't show delimiters for the arguments, which makes
-       # the call to mkdir ambiguous. Doing proper shell quoting would
-       # require code similar to shquote from mk/scripts/shell-lib.
-       # This may make the tools wrapper slower.
+       # The TOOLS_ARGS are already quoted, therefore they all look
+       # different in the log. The actual arguments, on the other hand,
+       # all look the same.
        assert_log <<'EOF'
-[*] WRKDIR/.tools/bin/mkdir directory with spaces
-<.> BINDIR/mkdir -p directory with spaces
+[*] WRKDIR/.tools/bin/mkdir 'directory with spaces'
+<.> BINDIR/mkdir -p 'directory with spaces'
 EOF
 }
 
@@ -99,18 +91,18 @@
        run_tool path-args-dquot "and" \" "\"" '"'
 
        assert_log <<'EOF'
-[*] WRKDIR/.tools/bin/path-args-dquot and " " "
-<.> echo \" "\"" '"' and " " "
+[*] WRKDIR/.tools/bin/path-args-dquot and '"' '"' '"'
+<.> echo \" "\"" '"' and '"' '"' '"'
 EOF
 }
 
 test_case "TOOLS_PATH with TOOLS_ARGS containing special characters"
 {
-       run_tool path-args "and" " \"'\\$" "*"
+       run_tool path-args "and" " \"'\\\$" "*"
 
        assert_log <<'EOF'
-[*] WRKDIR/.tools/bin/path-args and  "'\$ *
-<.> echo " \"'\\$" "*" and  "'\$ *
+[*] WRKDIR/.tools/bin/path-args and ' "'\''\$' '*'
+<.> echo " \"'\\$" "*" and ' "'\''\$' '*'
 EOF
 }
 
@@ -118,11 +110,9 @@
 {
        run_tool script-dquot
 
-       # The following log output contains a trailing whitespace. This
-       # is because the tool didn't get any actual arguments.
        assert_log <<'EOF'
-[*] WRKDIR/.tools/bin/script-dquot 
-<.> set args ; shift; echo "hello; world"
+[*] WRKDIR/.tools/bin/script-dquot
+<.> set args; shift; echo "hello; world"
 EOF
 }
 
@@ -130,11 +120,9 @@
 {
        run_tool script-backslash
 
-       # The following log output contains a trailing whitespace. This
-       # is because the tool didn't get any actual arguments.
        assert_log <<'EOF'
-[*] WRKDIR/.tools/bin/script-backslash 
-<.> set args ; shift; echo hello\;\ world
+[*] WRKDIR/.tools/bin/script-backslash
+<.> set args; shift; echo hello\;\ world
 EOF
 }
 
@@ -149,8 +137,6 @@
        #
        # In this example, the $0 becomes unrealistic when the command
        # is replayed. In practice $0 is seldom used.
-       #
-       # TODO: In the "set arg", the arguments must be shquoted.
        assert_log <<'EOF'
 [*] WRKDIR/.tools/bin/for-loop one two three
 <.> set args one two three; shift; printf '%s' "$0";  for arg in "$@"; do  printf ' <%s>' "$arg";  done;  printf '\n'
@@ -165,9 +151,8 @@
                -DDD="\"a b\"" \
                -DB=\"a\ b\"
 
-       # TODO: Add proper quoting for the arguments.
        assert_log <<'EOF'
-[*] WRKDIR/.tools/bin/for-loop -DSD="a b" -DSS='a b' -DDD="a b" -DB="a b"
-<.> set args -DSD="a b" -DSS='a b' -DDD="a b" -DB="a b"; shift; printf '%s' "$0";  for arg in "$@"; do  printf ' <%s>' "$arg";  done;  printf '\n'
+[*] WRKDIR/.tools/bin/for-loop '-DSD="a b"' '-DSS='\''a b'\''' '-DDD="a b"' '-DB="a b"'
+<.> set args '-DSD="a b"' '-DSS='\''a b'\''' '-DDD="a b"' '-DB="a b"'; shift; printf '%s' "$0";  for arg in "$@"; do  printf ' <%s>' "$arg";  done;  printf '\n'
 EOF
 }
diff -r 38fdac967e50 -r 85f5dcfa7fdd regress/tools/files/shquote-test.sh
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/regress/tools/files/shquote-test.sh       Sun Mar 24 11:29:19 2019 +0000
@@ -0,0 +1,54 @@
+#! /bin/sh
+set -eu
+
+. "${PKGSRCDIR}/mk/tools/shquote.sh"
+
+# test_shquote $in becomes $out
+test_shquote() {
+       shquote "$1"
+       [ "x$shquoted" = "x$3" ] && return
+       printf 'input:    %s\nexpected: %s\nactual:   %s\n' "$1" "$3" "$shquoted" 1>&2
+       exit 1
+}
+
+tools_wrapper_sed=${SED:-/usr/bin/sed}
+
+test_shquote '' becomes "''"
+
+test_shquote ' ' becomes "' '"
+test_shquote '!' becomes '!'
+test_shquote '"' becomes \'\"\'
+test_shquote '#' becomes "'#'"
+test_shquote '$' becomes "'$'"
+test_shquote '%' becomes '%'
+test_shquote '&' becomes "'&'"
+test_shquote \' becomes "''\\'''"
+test_shquote '(' becomes "'('"
+test_shquote ')' becomes "')'"
+test_shquote '*' becomes "'*'"
+test_shquote '+,-./0123456789:' becomes '+,-./0123456789:'
+test_shquote ';' becomes "';'"
+test_shquote '<' becomes "'<'"
+test_shquote '=' becomes '='
+test_shquote '>' becomes "'>'"
+test_shquote '?' becomes "'?'"
+test_shquote '@ABCDEFGHIJKLMNOPQRSTUVWXYZ' becomes '@ABCDEFGHIJKLMNOPQRSTUVWXYZ'
+test_shquote '[' becomes "'['"
+test_shquote '\' becomes \'\\\'
+test_shquote ']' becomes "']'"
+test_shquote '^' becomes "'^'"
+test_shquote '_' becomes '_'



Home | Main Index | Thread Index | Old Index