Re: [GSoC][RFC/PATCH v2] submodule: port subcommand foreach from shell to C

> On Mon, Apr 24, 2017 at 3:11 PM, Ramsay Jones
> <ramsay@ramsayjones.plus.com> wrote:
>>
>>
>> On 24/04/17 21:03, Stefan Beller wrote:
>> [snip]
>>
>>> +
>>> + argv_array_pushf(&cp.env_array, "name=%s", sub->name);
>>> + argv_array_pushf(&cp.env_array, "path=%s", displaypath);
>>> + argv_array_pushf(&cp.env_array, "sm_path=%s", displaypath);
>>>
>>> You mention keeping 'sm_path' in the notes after the commit message. I would
>>> add that part to the commit message, to explain why we have multiple variables
>>> that have the same value. Maybe even a comment in the code:
>>>
>>>     /* Keep sm_path for historic reasons, see tests in 091a6eb0fee. */
>>>     .. sm_path ..
>>
>> Hmm, you need to be a bit careful with putting 'path' in the
>> environment (if you then export it to sub-processes) on windows
>> (cygwin, MinGW, GfW). See commit 64394e3ae9. I would have liked
>> to remove $path altogether from the 'submodule-foreach api' in
>> that commit, but users and their scripts were already using it
>> (so I couldn't just drop it, without some deprecation period).
>> So long as whatever was being 'eval'-ed in the script didn't
>> export $path, ...
>>
> 
> Oh, I misread the comment
> 
>      # we make $path available to scripts ...
>      path=$sm_path
> 
> as it was such a casual friendly thing to say in that context.
> So the *real* historic baggage is
>     argv_array_pushf(&cp.env_array, "path=%s", displaypath);
> whereas
>     argv_array_pushf(&cp.env_array, "sm_path=%s", displaypath);
> is considered the correct way to go.