[PATCH STABLE] build: include a dummy $PATH in the custom environment used by build.py

[Pierre-Yves David]

On 11/02/2016 02:40 PM, Gábor STEFANIK wrote:
>>
>
>
> --------------------------------------------------------------------------
> This message, including its attachments, is confidential. For more information please read NNG's email policy here:
> http://www.nng.com/emailpolicy/
> By responding to this email you accept the email policy.
>
>
> -----Original Message-----
>> From: Mercurial-devel [mailto:mercurial-devel-bounces at mercurial-scm.org]
>> On Behalf Of Pierre-Yves David
>> Sent: Tuesday, November 1, 2016 4:56 PM
>> To: Jun Wu <quark at fb.com>
>> Cc: mercurial-devel at mercurial-scm.org
>> Subject: Re: [PATCH STABLE] build: include a dummy $PATH in the custom
>> environment used by build.py
>>
>>
>>
>> On 11/01/2016 04:39 PM, Jun Wu wrote:
>>> Excerpts from Pierre-Yves David's message of 2016-10-29 02:18:56 +0200:
>>>> My question is more "What is the meaning and effect of adding "." here?
>>>> I'm fine with trying to fix pypiwin32 but we have to be aware of the
>>>> actual consequence. A related quest is "What is happening is 'PATH'
>>>> is already in the envirement? are we overwriting it?
>>>
>>> For Windows, PATH='.' is equivalent to PATH='' since Windows searches
>>> for PWD by design.
>>>
>>> For *nix, PATH='' is better. PATH='.' has undesired side-effects.
>>>
>>> I checked the code, if PATH is already set, it will be dropped by our
>>> code because we created a new "env" dict and pass it to "runhg" ->
>>> "runcmd" -> "subprocess.Popen(..., env=env)" so the new process does
>>> not inherit the old PATH.
>>>
>>> We rely on "sys.executable" to be correct to find the correct Python.
>>>
>>> I think having an empty value, or inherit it from os.environ are both
>>> acceptable solutions. But it's pypiwin32 to blame anyway.
>>
>> It seem slike we should at least inherit a value if one exists. This patch does
>> not seems ready for inclusion. I'm dropping it from patchwork and we'll
>> revisit post release.
>
> I believe the point of using a custom environment here is precisely to avoid passing the real $PATH.
> Inheriting from the system environment would defeat this.
>
> The only reason for including a PATH entry here is so site.py can do os.environ['PATH'] instead of os.environ.get('PATH').
> Unfortunately some python modules (like pypiwin32) assume that a $PATH environment variable must always exist.

Ha true, when set, 'env' does not inherit any variable. So setting the 
path to '' should be fine. That said, the comment this env dictionary is 
quite specific about its goal.

# Execute hg out of this directory with a custom environment which
# takes care to not use any hgrc files and do no localization.

We should add a note about the why windows required PATH in some case.

I think we are clear enough now. (Jun do you agree?)

Can you send a V2 with an empty PATH and an updated comment?

Cheers,

-- 
Pierre-Yves David