Re: [PATCH] git submodule foreach: Skip eval for more than one argument
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 $ 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 ande...@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 signature.asc Description: Digital signature
Re: [PATCH] git submodule foreach: Skip eval for more than one argument
On Tue, Mar 4, 2014 at 2:51 PM, Matthijs Kooijman matth...@stdin.nl wrote: matthijs@grubby:~/test$ git submodule foreach echo '$name' Entering 'test' $name jherland@beta ~/test$ echo '$name' $name What would you expect echo '$name' to do? What happens if you use double instead of single quotes? ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git submodule foreach: Skip eval for more than one argument
On Tue, Mar 04, 2014 at 03:53:24PM +0100, Johan Herland wrote: On Tue, Mar 4, 2014 at 2:51 PM, Matthijs Kooijman matth...@stdin.nl wrote: matthijs@grubby:~/test$ git submodule foreach echo '$name' Entering 'test' $name jherland@beta ~/test$ echo '$name' $name What would you expect echo '$name' to do? If I run git submodule foreach each '$name', then my shell eats the single quotes (which are only to prevent my shell from interpreting $name). git submodule will see $name, so it will run echo $name, not echo '$name'. What happens if you use double instead of single quotes? Then my shell eats up the double quotes _and_ replaces $name with nothing, so I can't expect git submodule to replace it with the submodule name then :-) Does that help to clarify what I mean? Gr. Matthijs signature.asc Description: Digital signature
Re: [PATCH] git submodule foreach: Skip eval for more than one argument
On Tue, Mar 4, 2014 at 3:57 PM, Matthijs Kooijman matth...@stdin.nl wrote: On Tue, Mar 04, 2014 at 03:53:24PM +0100, Johan Herland wrote: What would you expect echo '$name' to do? If I run git submodule foreach each '$name', then my shell eats the single quotes (which are only to prevent my shell from interpreting $name). git submodule will see $name, so it will run echo $name, not echo '$name'. What happens if you use double instead of single quotes? Then my shell eats up the double quotes _and_ replaces $name with nothing, so I can't expect git submodule to replace it with the submodule name then :-) Does that help to clarify what I mean? Ok, so IINM, Anders' original commit was about making git submodule foreach command behave more like command (from a naive user's perspective), while you rather expect to insert quotes/escapes to finely control exactly when shell interpretation happens. Aren't these POVs mutually incompatible? Is the only 'real' solution to forbid multitple arguments, and force everybody to quote the entire command? I don't particularly care which way it goes, as long as (a) the common case behaves as most users would expect, (b) the uncommon/complicated case is still _possible_ (though not necessarily simple), and (c) we don't break a sizable number of existing users. ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git submodule foreach: Skip eval for more than one argument
Hey Johan,
Ok, so IINM, Anders' original commit was about making git submodule
foreach command behave more like command (from a naive user's
perspective),
Ok, that makes sense.
while you rather expect to insert quotes/escapes to finely control
exactly when shell interpretation happens.
Well, I mostly expect that the $name and $path that git submodule makes
available to each command invocation can actually be used by the
command.
Aren't these POVs mutually incompatible? Is the only 'real' solution
to forbid multitple arguments, and force everybody to quote the entire
command?
Yes, I think you're right that they're mutually exclusive. Specifically,
if you expect git submodule foreach command to behave like command,
that means you expect the (interactive) shell to do all the
interpolation, word-splitting, etc. If so, you can't then later still do
interpolation (of course, you could do sed magic to just replace $name
and $path, etc., but that's broken).
I don't particularly care which way it goes, as long as (a) the common
case behaves as most users would expect, (b) the uncommon/complicated
case is still _possible_ (though not necessarily simple), and (c) we
don't break a sizable number of existing users.
Well, if you call submodule directly, you can now just put everything in
a single command and get $name interpolation.
As I mentioned, I couldn't do this because I was using a git alias.
However, a bit of fiddling showed a solution to that using a shell
function:
[alias]
each = !f(){ git submodule foreach --quiet \echo \\$name $*\;}; f
This uses a shell function to collect all alias arguments and then uses
$* to expand them again into the single submodule foreach argument. Note
that $* is expanded when evaluating the alias, while \\$name is expanded
later inside submodule.
This suggests that with the current code, the more complicated cases are
still possible. There is one catch in this approach, in that the
original word splitting is not preserved ($* expands to just the
unquoted arguments as a single word). I'm not sure if this is fixable
($@ expands to multiple quoted words, but then foreach sees multiple
arguments and doesn't do the eval). One would need to escape the output
of $@ somehow (e.g., add \ before , but that would become terribly
complicated I expect...).
Perhaps an explicit --eval switch to git submodule makes sense for
complete control? If it has a correspondning --no-eval, you can even
pass a single-argument command without evalling, while still keeping the
current least surprise approach as the default?
Whatever behaviour is settled for, it should be documented in the
submodule manpage (which I think is not the case now).
Gr.
Matthijs
signature.asc
Description: Digital signature
Re: [PATCH] git submodule foreach: Skip eval for more than one argument
On Thu, Sep 26, 2013 at 10:10 PM, Anders Kaseorg ande...@mit.edu 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: $ 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 ande...@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 --- 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, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git submodule foreach: Skip eval for more than one argument
‘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 ande...@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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
