← Back to team overview

apport-hackers team mailing list archive

Re: [Merge] lp:~brian-murray/apport/sandbox-gdb into lp:apport

 

On Wed, Jan 25, 2017 at 10:52:12PM -0000, Steve Langasek wrote:
> Review: Needs Fixing
> 
> > One thing I don't feel great about is the way os.environ is modified
> > and restored for LD_LIBRARY_PATH etc as the code is duplicated in
> > apport-retrace and apport/report.py so suggestions welcome there!
> 
> Most of the subprocess family of calls can take an env= argument
> pointing to an environment to use for the subprocess. You could factor
> out a helper function to populate a copy environment, then pass this
> in to subprocess.call().

Cool, fixed.

> >From what I see, the code in apport-retrace isn't ever setting an
> >environment at all, it's saving an environment and then restoring
> >that same environment without ever modifying it in between. So, +1
> >indeed for factoring this out!

It was getting set in report.py's gdb_command, but by using subprocess's
env= argument I think things are clearer and cleaner now.

--
Brian Murray

-- 
https://code.launchpad.net/~brian-murray/apport/sandbox-gdb/+merge/315619
Your team Apport upstream developers is requested to review the proposed merge of lp:~brian-murray/apport/sandbox-gdb into lp:apport.


References