[pkgsrc/trunk]: pkgsrc mk/tools: fix unintended filename expansion in the too...

[rillig <rillig%pkgsrc.org@localhost>]

details:   https://anonhg.NetBSD.org/pkgsrc/rev/bfe4300ca36d
branches:  trunk
changeset: 393687:bfe4300ca36d
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Fri Mar 22 22:13:21 2019 +0000

description:
mk/tools: fix unintended filename expansion in the tool wrapper log file

diffstat:

 mk/tools/create.mk                  |  11 +++++++--
 regress/tools/files/logging-test.sh |  41 +++++++++++++++++++++++++++++-------
 2 files changed, 41 insertions(+), 11 deletions(-)

diffs (116 lines):

diff -r 76cf753487f3 -r bfe4300ca36d mk/tools/create.mk
--- a/mk/tools/create.mk        Fri Mar 22 20:56:16 2019 +0000
+++ b/mk/tools/create.mk        Fri Mar 22 22:13:21 2019 +0000
@@ -1,4 +1,4 @@
-# $NetBSD: create.mk,v 1.8 2018/08/22 20:48:37 maya Exp $
+# $NetBSD: create.mk,v 1.9 2019/03/22 22:13:21 rillig Exp $
 #
 # Copyright (c) 2005, 2006 The NetBSD Foundation, Inc.
 # All rights reserved.
@@ -137,6 +137,8 @@
 TOOLS_PATH.${_t_}?=            ${FALSE}
 TOOLS_SCRIPT_DFLT.${_t_}=      \
        ${TOOLS_PATH.${_t_}} ${TOOLS_ARGS.${_t_}} "$$@"
+_TOOLS_LOGSCRIPT_DFLT.${_t_}=  \
+       ${TOOLS_PATH.${_t_}} ${TOOLS_ARGS.${_t_}} $$*
 
 override-tools: ${TOOLS_CMD.${_t_}}
 
@@ -149,15 +151,18 @@
        if ${TEST} -n ${TOOLS_SCRIPT.${_t_}:Q}""; then                  \
                create=wrapper;                                         \
                script=${TOOLS_SCRIPT.${_t_}:Q};                        \
+               logscript="$$script";                                   \
        elif ${TEST} -n ${TOOLS_PATH.${_t_}:Q}""; then                  \
                if ${TEST} -n ${TOOLS_ARGS.${_t_}:Q}""; then            \
                        create=wrapper;                                 \
                        script=${TOOLS_SCRIPT_DFLT.${_t_}:Q};           \
+                       logscript=${_TOOLS_LOGSCRIPT_DFLT.${_t_}:Q};    \
                else                                                    \
                        case ${TOOLS_PATH.${_t_}:Q}"" in                \
                        /*)     create=symlink ;;                       \
                        *)      create=wrapper;                         \
                                script=${TOOLS_SCRIPT_DFLT.${_t_}:Q};   \
+                               logscript=${_TOOLS_LOGSCRIPT_DFLT.${_t_}:Q}; \
                        esac;                                           \
                fi;                                                     \
        else                                                            \
@@ -167,8 +172,8 @@
        wrapper)                                                        \
                { ${ECHO} '#!'${TOOLS_SHELL:Q};                         \
                  ${ECHO} 'wrapperlog="$${TOOLS_WRAPPER_LOG-'${_TOOLS_WRAP_LOG:Q}'}"'; \
-                 ${ECHO} '${ECHO} "[*] "'${.TARGET:Q}'" $$@" >> $$wrapperlog'; \
-                 ${ECHO} "${ECHO} \"<.> $$script\" >> \$$wrapperlog";  \
+                 ${ECHO} '${ECHO} "[*] "'${.TARGET:Q}'" $$*" >> $$wrapperlog'; \
+                 ${ECHO} "${ECHO} \"<.> $$logscript\" >> \$$wrapperlog"; \
                  ${ECHO} "$$script";                                   \
                } > ${.TARGET:Q};                                       \
                ${CHMOD} +x ${.TARGET:Q};                               \
diff -r 76cf753487f3 -r bfe4300ca36d regress/tools/files/logging-test.sh
--- a/regress/tools/files/logging-test.sh       Fri Mar 22 20:56:16 2019 +0000
+++ b/regress/tools/files/logging-test.sh       Fri Mar 22 22:13:21 2019 +0000
@@ -1,9 +1,14 @@
 #! /bin/sh
-# $NetBSD: logging-test.sh,v 1.1 2019/03/22 20:56:16 rillig Exp $
+# $NetBSD: logging-test.sh,v 1.2 2019/03/22 22:13:21 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
 
@@ -18,9 +23,14 @@
 TOOLS_WRAPPER_LOG="$tools_log"
 export TOOLS_WRAPPER_LOG
 
-# Forcibly call the echo from the tools directory. This tool is wrapped and
-# logged.
+# Forcibly call the tools from the tools directory, not the shell builtins.
+# The echo tool is a wrapped tool without additional arguments.
+# The mkdir tool is a wrapped tool that always gets the -p option.
 (exec echo "begin" "*" "*" "*" "end")
+(exec echo "dquot" "\"" "end")
+(exec echo "squot" "'" "end")
+(exec echo "five" '\\\\\' "end")
+(exec mkdir "directory with spaces")
 
 unset TOOLS_WRAPPER_LOG
 
@@ -31,11 +41,26 @@
        assert_equal "$1" "$expected" "$actual"
 }
 
-sed 's,/.*/\.tools/,WRKDIR/.tools/,' < "$tools_log" > "$nopath_log"
+# Replace the variable parts from the output with placeholders.
+sed < "$tools_log" > "$nopath_log"             \
+       -e 's,/.*/\.tools/,WRKDIR/.tools/,'     \
+       -e 's,^<.> /[^ ]*/,<.> BINDIR/,'
 
-assert_file_equals "$nopath_log" <<EOF
+# The double space in the "echo  begin" below is because the echo command
+# doesn't get any additional arguments by the tool wrapper (TOOLS_ARGS.echo).
+#
+# 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.
+assert_file_equals "$nopath_log" <<'EOF'
 [*] WRKDIR/.tools/bin/echo begin * * * end
-<.> echo  begin tools.log tools.log tools.log 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/mkdir directory with spaces
+<.> BINDIR/mkdir -p directory with spaces
 EOF
-# FIXME: the above output is not quoted correctly.
-# The tools.log should not appear there.