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

From: Stefan Beller <sbeller@google.com>
Date: Apr 24, 2017, 22:17 UTC
Message-ID: <CAGZ79kbU4LcRFHh5dM7i+L5DKpxKSrK-mZdYEeScrzVnYZsJmg@mail.gmail.com>
In-Reply-To: <efe8e82d-a021-5fc4-492a-e6e0ab7d52d3@ramsayjones.plus.com>

On Mon, Apr 24, 2017 at 3:11 PM, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:

Show 26 quoted lines

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

Thanks, Stefan