Re: [PATCH 1/2] submodule: fix confusing variable name

[Ramsay Jones <ramsay@ramsay1.demon.co.uk>]

Fredrik Gustafsson wrote:
> On Sun, Aug 04, 2013 at 07:34:48PM +0200, Jens Lehmann wrote:
>> But we'll have to use sm_path here (like everywhere else in the
>> submodule script), because we'll run into problems under Windows
>> otherwise (see 64394e3ae9 for details). Apart from that the patch
>> is fine.
> 
> We're still using path= in the foreach-script. Or rather, we're setting
> it. From what I can see and from the commit message 64394e3ae9 it could
> possible be a problem.

Please do not use a $path variable in any script intended to be run on
windows; those poor souls who would otherwise have to fix the bugs will
thank you! :-D

Actually, it's not so much the use of a $path variable, rather the act
of _exporting_ such a variable that causes the problem. (Which is why
using $path with eval_gettext[ln] is such a problem, of course.)

As noted in the above commit, $path is unfortunately a documented part
of the public API for the foreach subcommand. However, the foreach
subcommand is (mostly) fine; given the fact that the user script is
eval-ed in a context in which $path is not exported. The reason for
the 'mostly' is simply that the user could shoot himself in the foot
by export-ing $path in their script, so that something like:

    $ git submodule foreach 'export path; echo $path `git rev-parse HEAD`'

will indeed fail (ie git rev-parse will not execute).

> Not sure how to solve it though... Just a simple correction would break
> all script depending on that.

$path is part of the public API, so we can't just remove it. It would
require a deprecation period, etc,.  (Adding/documenting $sm_path as an
alternative *may* be worth doing. dunno.)

HTH

ATB,
Ramsay Jones