pkgsrc-Changes archive

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

CVS commit: pkgsrc



Module Name:    pkgsrc
Committed By:   rillig
Date:           Sun Mar 24 11:29:19 UTC 2019

Modified Files:
        pkgsrc/mk/tools: create.mk
        pkgsrc/regress/tools: Makefile
        pkgsrc/regress/tools/files: logging-test.sh
Added Files:
        pkgsrc/mk/tools: shquote.sh
        pkgsrc/regress/tools/files: shquote-test.sh

Log Message:
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.


To generate a diff of this commit:
cvs rdiff -u -r1.10 -r1.11 pkgsrc/mk/tools/create.mk
cvs rdiff -u -r0 -r1.1 pkgsrc/mk/tools/shquote.sh
cvs rdiff -u -r1.14 -r1.15 pkgsrc/regress/tools/Makefile
cvs rdiff -u -r1.5 -r1.6 pkgsrc/regress/tools/files/logging-test.sh
cvs rdiff -u -r0 -r1.1 pkgsrc/regress/tools/files/shquote-test.sh

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: pkgsrc/mk/tools/create.mk
diff -u pkgsrc/mk/tools/create.mk:1.10 pkgsrc/mk/tools/create.mk:1.11
--- pkgsrc/mk/tools/create.mk:1.10      Sun Mar 24 08:40:07 2019
+++ pkgsrc/mk/tools/create.mk   Sun Mar 24 11:29:19 2019
@@ -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 @@ ${TOOLS_CMD.${_t_}}:
        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 @@ ${TOOLS_CMD.${_t_}}:
                        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 @@ ${TOOLS_CMD.${_t_}}:
                                script=${TOOLS_SCRIPT_DFLT.${_t_}:Q};   \
                                logprefix='';                           \
                                logmain=${TOOLS_PATH.${_t_}:Q:Q}\"\ \"; \
-                               logsuffix=' "$$*"';                     \
+                               logsuffix='$$shquoted_args';            \
                        esac;                                           \
                fi;                                                     \
        else                                                            \
@@ -175,11 +175,14 @@ ${TOOLS_CMD.${_t_}}:
        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};                               \

Index: pkgsrc/regress/tools/Makefile
diff -u pkgsrc/regress/tools/Makefile:1.14 pkgsrc/regress/tools/Makefile:1.15
--- pkgsrc/regress/tools/Makefile:1.14  Sun Mar 24 08:40:07 2019
+++ pkgsrc/regress/tools/Makefile       Sun Mar 24 11:29:19 2019
@@ -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 @@ LICENSE=      2-clause-bsd
 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_PATH.path-args=       echo
 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"

Index: pkgsrc/regress/tools/files/logging-test.sh
diff -u pkgsrc/regress/tools/files/logging-test.sh:1.5 pkgsrc/regress/tools/files/logging-test.sh:1.6
--- pkgsrc/regress/tools/files/logging-test.sh:1.5      Sun Mar 24 08:40:08 2019
+++ pkgsrc/regress/tools/files/logging-test.sh  Sun Mar 24 11:29:19 2019
@@ -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 @@ test_case "TOOLS_PATH without TOOLS_ARGS
        # 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 @@ test_case "TOOLS_PATH with TOOLS_ARGS"
 
        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 @@ test_case "TOOLS_PATH with TOOLS_ARGS co
        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 @@ test_case "TOOLS_SCRIPT with dquot"
 {
        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 @@ test_case "TOOLS_SCRIPT with backslashes
 {
        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 @@ test_case "TOOLS_SCRIPT with complicated
        #
        # 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 @@ test_case "TOOLS_SCRIPT with actual argu
                -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
 }

Added files:

Index: pkgsrc/mk/tools/shquote.sh
diff -u /dev/null pkgsrc/mk/tools/shquote.sh:1.1
--- /dev/null   Sun Mar 24 11:29:19 2019
+++ pkgsrc/mk/tools/shquote.sh  Sun Mar 24 11:29:19 2019
@@ -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
+}

Index: pkgsrc/regress/tools/files/shquote-test.sh
diff -u /dev/null pkgsrc/regress/tools/files/shquote-test.sh:1.1
--- /dev/null   Sun Mar 24 11:29:19 2019
+++ pkgsrc/regress/tools/files/shquote-test.sh  Sun Mar 24 11:29:19 2019
@@ -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 '_'
+test_shquote '`' becomes "'\`'"
+test_shquote 'abcdefghijklmnopqrstuvwxyz' becomes 'abcdefghijklmnopqrstuvwxyz'
+test_shquote '{' becomes "'{'"
+test_shquote '|' becomes "'|'"
+test_shquote '}' becomes "'}'"
+test_shquote '~' becomes "'~'"
+
+test_shquote 'a b' becomes "'a b'"
+test_shquote 'a   b' becomes "'a   b'"
+test_shquote ' ' becomes "'    '"
+test_shquote '-e asdf' becomes "'-e asdf'"
+test_shquote '-n' becomes '-n'
+test_shquote '\\\\\\\\' becomes \''\\\\\\\\'\'
+test_shquote \"\$\'\;\<\\\` becomes \'\"\$\'\\\'\'\;\<\\\`\'



Home | Main Index | Thread Index | Old Index