Re: [PATCH 2/4] classes/testsdk: Move the removal of bitbake PATH to eSDK context only

["Aníbal Limón" <anibal.limon@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2206 bytes --]



On 02/22/2016 11:27 AM, Randy Witt wrote:
> On 02/22/2016 09:05 AM, Aníbal Limón wrote:
>>
>>
>> On 02/22/2016 10:48 AM, Burton, Ross wrote:
>>> On 22 February 2016 at 16:37, Aníbal Limón
>>> <anibal.limon@linux.intel.com>
>>> wrote:
>>>
>>>> I agree with you to modify avoid_paths_in_environ for return the new
>>>> PATH variable is better than only modify it internally but for
>>>> simplicity i will maintain the os.environ['PATH'] set/restore
>>>> instead of
>>>> generate the environment line.
>>>>
>>>
>>> Totally agree with Randy here for what it's worth.  The
>>> environment-munging
>>> code in avoid_paths... should return the strings instead of manipulating
>>> the current environment so the caller has the choice whether to
>>> modify the
>>> current environment or pass a new environment to subprocess.  And in
>>> general I'd say that passing modified environments to subprocess is a
>>> cleaner solution as it means that there's no way cleanup can fail to
>>> happen.  Whilst that's just a try/except now, the code could get
>>> copied and
>>> extended and end up with codepaths that don't hit the right cleanup.  By
>>> having an explicit environment passed in, this isn't possible.
>>
>> Agree, now modified at,
>>
>> http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=alimon/esdk_update_v2&id=3143bf09130c52cd71e3f2f9795208e17152005d
>>
> 
> If you either convert path to a dict or have avoid_paths_in_environ()
> return a dict you can do:
> 
> +            output = subprocess.check_output(". %s > /dev/null; %s;" % \
> +                (self.tc.sdkenv, cmd), env=path, shell=True)
> 
> It's not quite obvious in the docs that you can pass that in, they refer
> you to the Popen docs.


I know that you could pass env in kwargs but since it's using shell=True
is easy to set PATH inline also it causes a little overhead for copy the
os.environ.

But if you want the another way that's ok, see [1].

[1]
http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=alimon/esdk_update_v2&id=ab84d325c5cbb751d7b18e861964b757d9682e0f

	alimon

> 
>> Cheers,
>>     alimon
>>
>>>
>>> Ross
>>>
>>
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]