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