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 alimon > >> Cheers, >> alimon >> >>> >>> Ross >>> >> >