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 08:40:08 UTC 2019
Modified Files:
pkgsrc/mk/tools: create.mk
pkgsrc/regress/tools: Makefile
pkgsrc/regress/tools/files: logging-test.sh
Log Message:
mk/tools: fix quoting when logging tool invocations
When a package or the infrastructure defined a tool with custom
TOOLS_ARGS or TOOLS_SCRIPT containing special characters, these could
lead to unintuitive interactions at the time when that tool invocation
was logged in the tool wrapper log. Some of the logging output ended up
on stdout, while some of the normal output ended up in the log, and parts
of the quoted arguments were even evaluated as shell commands.
The logging of the wrapped tool commands is not perfect yet, but at least
it's much more predictable now.
To generate a diff of this commit:
cvs rdiff -u -r1.9 -r1.10 pkgsrc/mk/tools/create.mk
cvs rdiff -u -r1.13 -r1.14 pkgsrc/regress/tools/Makefile
cvs rdiff -u -r1.4 -r1.5 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.9 pkgsrc/mk/tools/create.mk:1.10
--- pkgsrc/mk/tools/create.mk:1.9 Fri Mar 22 22:13:21 2019
+++ pkgsrc/mk/tools/create.mk Sun Mar 24 08:40:07 2019
@@ -1,4 +1,4 @@
-# $NetBSD: create.mk,v 1.9 2019/03/22 22:13:21 rillig Exp $
+# $NetBSD: create.mk,v 1.10 2019/03/24 08:40:07 rillig Exp $
#
# Copyright (c) 2005, 2006 The NetBSD Foundation, Inc.
# All rights reserved.
@@ -137,8 +137,6 @@ 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_}}
@@ -151,18 +149,24 @@ ${TOOLS_CMD.${_t_}}:
if ${TEST} -n ${TOOLS_SCRIPT.${_t_}:Q}""; then \
create=wrapper; \
script=${TOOLS_SCRIPT.${_t_}:Q}; \
- logscript="$$script"; \
+ logprefix='"set args "$$@"; shift; "'; \
+ logmain=${TOOLS_SCRIPT.${_t_}:Q:Q}; \
+ logsuffix=''; \
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}; \
+ logprefix=''; \
+ logmain=${TOOLS_PATH.${_t_}:Q:Q}\"\ \"${TOOLS_ARGS.${_t_}:Q:Q}; \
+ logsuffix=' "$$*"'; \
else \
case ${TOOLS_PATH.${_t_}:Q}"" in \
/*) create=symlink ;; \
*) create=wrapper; \
script=${TOOLS_SCRIPT_DFLT.${_t_}:Q}; \
- logscript=${_TOOLS_LOGSCRIPT_DFLT.${_t_}:Q}; \
+ logprefix=''; \
+ logmain=${TOOLS_PATH.${_t_}:Q:Q}\"\ \"; \
+ logsuffix=' "$$*"'; \
esac; \
fi; \
else \
@@ -173,7 +177,9 @@ ${TOOLS_CMD.${_t_}}:
{ ${ECHO} '#!'${TOOLS_SHELL:Q}; \
${ECHO} 'wrapperlog="$${TOOLS_WRAPPER_LOG-'${_TOOLS_WRAP_LOG:Q}'}"'; \
${ECHO} '${ECHO} "[*] "'${.TARGET:Q}'" $$*" >> $$wrapperlog'; \
- ${ECHO} "${ECHO} \"<.> $$logscript\" >> \$$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.13 pkgsrc/regress/tools/Makefile:1.14
--- pkgsrc/regress/tools/Makefile:1.13 Sat Mar 23 22:59:11 2019
+++ pkgsrc/regress/tools/Makefile Sun Mar 24 08:40:07 2019
@@ -1,4 +1,4 @@
-# $NetBSD: Makefile,v 1.13 2019/03/23 22:59:11 rillig Exp $
+# $NetBSD: Makefile,v 1.14 2019/03/24 08:40:07 rillig Exp $
#
DISTNAME= # not applicable
@@ -43,6 +43,18 @@ TOOLS_SCRIPT.for-loop= \
done; \
printf '\n'
+# Demonstrates that double quotes in both the TOOLS_ARGS and the actual
+# arguments are properly logged.
+TOOLS_CREATE+= path-args-dquot
+TOOLS_PATH.path-args-dquot= echo
+TOOLS_ARGS.path-args-dquot= \" "\"" '"'
+
+# Demonstrates that both the TOOLS_ARGS and the actual arguments are
+# properly logged.
+TOOLS_CREATE+= path-args
+TOOLS_PATH.path-args= echo
+TOOLS_ARGS.path-args= " \"'\\$$" "*"
+
do-build:
.for t in ${REGRESS_TESTS}
${RUN} cd ${WRKSRC}; \
Index: pkgsrc/regress/tools/files/logging-test.sh
diff -u pkgsrc/regress/tools/files/logging-test.sh:1.4 pkgsrc/regress/tools/files/logging-test.sh:1.5
--- pkgsrc/regress/tools/files/logging-test.sh:1.4 Sat Mar 23 22:59:11 2019
+++ pkgsrc/regress/tools/files/logging-test.sh Sun Mar 24 08:40:08 2019
@@ -1,5 +1,5 @@
#! /bin/sh
-# $NetBSD: logging-test.sh,v 1.4 2019/03/23 22:59:11 rillig Exp $
+# $NetBSD: logging-test.sh,v 1.5 2019/03/24 08:40:08 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
@@ -64,6 +64,8 @@ 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
@@ -92,20 +94,35 @@ test_case "TOOLS_PATH with TOOLS_ARGS"
EOF
}
+test_case "TOOLS_PATH with TOOLS_ARGS containing double quotes"
+{
+ run_tool path-args-dquot "and" \" "\"" '"'
+
+ assert_log <<'EOF'
+[*] 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" " \"'\\$" "*"
+
+ assert_log <<'EOF'
+[*] WRKDIR/.tools/bin/path-args and "'\$ *
+<.> echo " \"'\\$" "*" and "'\$ *
+EOF
+}
+
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.
- #
- # FIXME: the "echo oops" occurs because the script is not
- # properly quoted during logging.
assert_log <<'EOF'
[*] WRKDIR/.tools/bin/script-dquot
-[*] WRKDIR/.tools/bin/world
-<.> echo oops
-oops
+<.> set args ; shift; echo "hello; world"
EOF
}
@@ -117,7 +134,7 @@ test_case "TOOLS_SCRIPT with backslashes
# is because the tool didn't get any actual arguments.
assert_log <<'EOF'
[*] WRKDIR/.tools/bin/script-backslash
-<.> echo hello\;\ world
+<.> set args ; shift; echo hello\;\ world
EOF
}
@@ -125,10 +142,18 @@ test_case "TOOLS_SCRIPT with complicated
{
run_tool for-loop "one" "two" "three"
- # TODO: Add proper quoting for the printf argument inside the loop.
+ # The actual command is written to the log in a form as close as
+ # possible to replay it. Since the command may do anything with
+ # its arguments, it's the safest way to set them first and then
+ # just log the command verbatim.
+ #
+ # 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
-<.> printf '%s' WRKDIR/.tools/bin/for-loop; for arg in one two three; do printf ' <%s>' ; done; printf '\n'
+<.> set args one two three; shift; printf '%s' "$0"; for arg in "$@"; do printf ' <%s>' "$arg"; done; printf '\n'
EOF
}
@@ -143,6 +168,6 @@ test_case "TOOLS_SCRIPT with actual argu
# 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"
-<.> printf '%s' WRKDIR/.tools/bin/for-loop; for arg in -DSD="a b" -DSS='a b' -DDD="a b" -DB="a b"; do printf ' <%s>' ; done; printf '\n'
+<.> 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
}
Home |
Main Index |
Thread Index |
Old Index