Re: [PATCH] git submodule foreach: Skip eval for more than one argument
On Thu, Sep 26, 2013 at 10:10 PM, Anders Kaseorg <andersk@mit.edu> wrote:
Show 15 quoted lines
> ‘eval "$@"’ created an extra layer of shell interpretation, which was
> probably not expected by a user who passed multiple arguments to git
> submodule foreach:
>
> $ git grep "'"
> [searches for single quotes]
> $ git submodule foreach git grep "'"
> Entering '[submodule]'
> /usr/lib/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted string
> Stopping at '[submodule]'; script returned non-zero status.
>
> To fix this, if the user passed more than one argument, just execute
> "$@" directly instead of passing it to eval.
>
> Signed-off-by: Anders Kaseorg <andersk@mit.edu>
The change looks good, and the existing tests (in t7407) pass. :-)
Two comments, however:
1. Please add the use case you mention above as a new test case, so
that we can easily catch future regressions.
2. If we are unlucky there might be existing users that work around
the existing behavior by adding an extra level of quoting (i.e. doing
the equivalent of git submodule foreach git grep "\'" in your example
above). Will their workaround break as a result of your change? Is
that acceptable?
Have fun! :)
...Johan
Show 25 quoted lines
> ---
> git-submodule.sh | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index c17bef1..3381864 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -545,7 +545,12 @@ cmd_foreach()
> sm_path=$(relative_path "$sm_path") &&
> # we make $path available to scripts ...
> path=$sm_path &&
> - eval "$@" &&
> + if [ $# -eq 1 ]
> + then
> + eval "$1"
> + else
> + "$@"
> + fi &&
> if test -n "$recursive"
> then
> cmd_foreach "--recursive" "$@"
> --
> 1.8.4
>
--
Johan Herland, <johan@herland.net>
www.herland.net
Johan Herland <johan@herland.net>