Re: [PATCH] git submodule foreach: Skip eval for more than one argument

From: Matthijs Kooijman <matthijs@stdin.nl>
Date: Mar 4, 2014, 13:51 UTC
Message-ID: <20140304135106.GD11566@login.drsnuggles.stderr.nl>
In-Reply-To: <alpine.DEB.2.00.1309261605330.20647@dr-wily.mit.edu>

Hey folks,

On Thu, Sep 26, 2013 at 04:10:15PM -0400, Anders Kaseorg wrote:

> ‘eval "$@"’ created an extra layer of shell interpretation, which was
> probably not expected by a user who passed multiple arguments to git
> submodule foreach:

It seems this patch has broken the use of $name, $path, etc. inside the command ran by foreach (when it contains more than one argument):

matthijs@grubby:~/test$ git --version git version 1.9.0 matthijs@grubby:~/test$ git submodule foreach echo '$name' Entering 'test' $name

But it works on the single-argument version:

matthijs@grubby:~/test$ git submodule foreach 'echo $name' Entering 'test' test

And it used to work in older versions:

matthijs@login:~/test$ git --version git version 1.7.5.4 matthijs@login:~/test$ git submodule foreach 'echo $name' Entering 'test' test matthijs@login:~/test$ git submodule foreach echo '$name' Entering 'test' test

I'm not sure how to fix this exactly. Adding "export" for the variables in git-submodule.sh seems obvious but doesn't seem to be a complete solution. This makes the variables available in the environment of any commands called (so git submodule sh -c 'echo $name') works, but the git submodule foreach echo '$name' above still doesn't work, since the "$@" used does not do any substitution, it just executes $@ as a commandline unmodified. Ideally, you would do variable substitution, but not word splitting, but I'm not sure how to do that. Also, you'd still need one more layer of backslash escapes, which is probably what this commit wanted to prevent...

Note that saying "you should use the single argument version if you need those variables" doesn't seem possible in all cases. In particular, I'm creating an alias that calls git submodule foreach, where the alias contains part of the command and the rest of command comes from arguments to the alias, meaning we always have at least two arguments...

Finally, the new behaviour (e.g., eval with one argument, directly execute with multiple) is not documented in the manpage, but it seems relevant enough to need documentation?

Gr.

Matthijs

Show 39 quoted lines

> 
> $ 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>
> ---
>  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
> 
> 
>