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: 2522 bytes --]



On 02/22/2016 11:47 AM, Aníbal Limón wrote:
> 
> 
> 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

Bad rev this is the one that works [1].

[1]
http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=alimon/esdk_update_v2&id=228e8e506f946a1245798b0d0e818f63aca221ce
> 
> 	alimon
> 
>>
>>> Cheers,
>>>     alimon
>>>
>>>>
>>>> Ross
>>>>
>>>
>>
> 
> 
> 


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