[PATCH 1/2] submodule: preserve all arguments exactly when recursing

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

 



When performing a recursive status or update, any argments with whitespace
would be split along that whitespace when passed to the recursive invocation
of the update or status command.

This is caused by the special handling that sh provides to the $@ variable.
Status and update stored "$@" into a separate variable, and passed that
variable to the recursive invocation. Unfortunately, the special handling
afforded to $@ isn't given to this new variable, and word-breaking occurs
along whitespace boundaries.

We can work around this by taking advantage of an easy technique to quote
any arbitrary string in the shell. Because single-quoted strings don't
support backslash-escapes, any arbitrary string can be quoted by wrapping
it in single-quotes and replacing any single-quotes inside the string with
the sequence '\''.

This commit introduces a new shell function quote_words that uses the quote
trick to produce a string containing the quoted version of all its
arguments. This string can then be used with `set -` to restore the original
value of $@. This shell function is used in cmd_status and in cmd_update
to store the original value of $@, which is then restored before the
recursive invocation takes place.

Signed-off-by: Kevin Ballard <kevin@xxxxxx>
---
I tried to write tests for this, but there are only two ways to get args
with spaces to be accepted and passed to the recursive invocation.
The first is via the --reference flag, but I don't think it really makes
sense to use that flag in connection with --recursive and was not comfortable
using it in a test. The second is as a pathname after the flags, but it
also doesn't make sense to pass these to recursive invocations, and in fact
the subsequent commit fixes it so the pathnames are not passed to recursive
invocations.

That said, despite the lack of tests I still believe this is a worthwhile
change, and it will certainly future-proof the command in case new flags
are added. It's also a reasonable model for how to handle this problem
in other shell commands.
 git-submodule.sh |   43 +++++++++++++++++++++++++++++++++++++++----
 1 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 9ebbab7..ec7a5e4 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -64,6 +64,39 @@ module_list()
 }
 
 #
+# Emit a quoted version of the all argument suitable for passing to `eval`
+# $@ = words to quote
+#
+# This is intended to be used like the following:
+#   orig_args="$(quote_words "$@")"
+#   # do some work that includes calling shift
+#   eval "set - $orig_args"
+#   # now $@ has been restored, suitable for passing to another command
+#
+# Note that you cannot simply save off $@ into another variable because
+# the shell gives $@ and $* special handling in parameter expansion
+#
+quote_words ()
+{
+	while test $# -ne 0; do
+		# this can be done using sed like so:
+		#   printf "'%s'" "$(printf "%s" "$1" | sed -e "s/'/'\\\\''/g")"
+		# but in an attempt to avoid spawning a process for every argument, we'll
+		# just use the prefix/suffix pattern matching stuff
+		local word= suffix="$1" prefix=
+		while test -n "$suffix"
+		do
+			prefix="${suffix%%\'*}"
+			test "$prefix" != "$suffix" || break
+			suffix="${suffix#*\'}"
+			word="$word$prefix'\\''"
+		done
+		printf "'%s' " "$word$suffix"
+		shift
+	done
+}
+
+#
 # Map submodule path to submodule name
 #
 # $1 = path
@@ -374,7 +407,7 @@ cmd_init()
 cmd_update()
 {
 	# parse $args after "submodule ... update".
-	orig_args="$@"
+	orig_args="$(quote_words "$@")"
 	while test $# -ne 0
 	do
 		case "$1" in
@@ -500,7 +533,8 @@ cmd_update()
 
 		if test -n "$recursive"
 		then
-			(clear_local_git_env; cd "$path" && cmd_update $orig_args) ||
+			eval "set - $orig_args"
+			(clear_local_git_env; cd "$path" && cmd_update "$@") ||
 			die "Failed to recurse into submodule path '$path'"
 		fi
 	done
@@ -733,7 +767,7 @@ cmd_summary() {
 cmd_status()
 {
 	# parse $args after "submodule ... status".
-	orig_args="$@"
+	orig_args="$(quote_words "$@")"
 	while test $# -ne 0
 	do
 		case "$1" in
@@ -788,9 +822,10 @@ cmd_status()
 		then
 			(
 				prefix="$displaypath/"
+				eval "set - $orig_args"
 				clear_local_git_env
 				cd "$path" &&
-				cmd_status $orig_args
+				cmd_status "$@"
 			) ||
 			die "Failed to recurse into submodule path '$path'"
 		fi
-- 
1.7.3.2.200.ga1bd

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]