CVS commit: pkgsrc

["Roland Illig" <rillig%netbsd.org@localhost>]

Module Name:    pkgsrc
Committed By:   rillig
Date:           Fri Mar 22 22:13:21 UTC 2019

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

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


To generate a diff of this commit:
cvs rdiff -u -r1.8 -r1.9 pkgsrc/mk/tools/create.mk
cvs rdiff -u -r1.1 -r1.2 pkgsrc/regress/tools/files/logging-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.8 pkgsrc/mk/tools/create.mk:1.9
--- pkgsrc/mk/tools/create.mk:1.8       Wed Aug 22 20:48:37 2018
+++ pkgsrc/mk/tools/create.mk   Fri Mar 22 22:13:21 2019
@@ -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_CMD.${_t_}?=          ${TOOLS_DIR}/bin/${_
 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 @@ ${TOOLS_CMD.${_t_}}:
        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 @@ ${TOOLS_CMD.${_t_}}:
        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};                               \

Index: pkgsrc/regress/tools/files/logging-test.sh
diff -u pkgsrc/regress/tools/files/logging-test.sh:1.1 pkgsrc/regress/tools/files/logging-test.sh:1.2
--- pkgsrc/regress/tools/files/logging-test.sh:1.1      Fri Mar 22 20:56:16 2019
+++ pkgsrc/regress/tools/files/logging-test.sh  Fri Mar 22 22:13:21 2019
@@ -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 @@ rm -f "$tools_log" "$nopath_log"
 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_file_equals() {
        assert_equal "$1" "$expected" "$actual"
 }
 
-sed 's,/.*/\.tools/,WRKDIR/.tools/,' < "$tools_log" > "$nopath_log"
-
-assert_file_equals "$nopath_log" <<EOF
+# Replace the variable parts from the output with placeholders.
+sed < "$tools_log" > "$nopath_log"             \
+       -e 's,/.*/\.tools/,WRKDIR/.tools/,'     \
+       -e 's,^<.> /[^ ]*/,<.> BINDIR/,'
+
+# 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.