Bug 9342 - Vet subprocess calls
Summary: Vet subprocess calls
Status: VERIFIED FIXED
Alias: None
Product: OE-Core
Classification: Build System, Metadata & Runtime
Component: core (show other bugs)
Version: 2.1
Hardware: All Multiple
: Medium+ enhancement
Target Milestone: 2.3 M1
Assignee: Stephano Cetola
QA Contact: Juan Ramos
URL:
Whiteboard:
Depends on:
Blocks:
 
Reported: 2016-03-24 14:29 UTC by Ross Burton
Modified: 2017-07-11 17:38 UTC (History)
7 users (show)

See Also:
OS type for building Yocto: ---
Type of Regression: ---
Verified:
Documentation change: No (bug/feature does not impact docs)


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ross Burton 2016-03-24 14:29:03 UTC
We should vet all users of subprocess, as unless you pass stderr=subprocess.STDOUT then stderr will disappear and this may mean errors are unreported.
Comment 1 Joshua Lock 2016-07-01 15:46:39 UTC
We should also vet oe.path.check_output() calls
Comment 2 Ross Burton 2016-07-01 15:53:45 UTC
This is a proper yak shaver
* Review bb.process in light of Python 3.4's subprocess implementation, in particular some of the issues in py2 have been fixed so some cruft can likely be removed
* Note that Python 3.5 has a subprocess.run() convenience function that does all sorts of useful magic.  Maybe it would be sensible to clone that API in bb.process() for consistency and the ability to just use subprocess.run() in the future
* Review subprocess calls, moving as much as possible to lists instead of strings with shell=True, and show stderr on error.
Comment 3 Ross Burton 2016-07-01 15:59:56 UTC
There were just two callers of oe.path.check_output, so I just sent a patch to use subprocess directly.
Comment 4 Ross Burton 2016-07-14 00:03:21 UTC
Stephano, I appreciate this bug is rather vague, hopefully comment #2 makes some sense.  Feel free to talk to me if you're not sure about anything!
Comment 5 Ross Burton 2016-07-21 14:35:33 UTC
oe.utils.getstatusoutput() is a wrapper around subprocess.getstatusouput() in Py3, which is basically deprecated and behaves almost entirely unlike run().  This also should be gutted.
Comment 6 Stephano Cetola 2016-09-27 23:39:11 UTC
Cleaned up sub.call and sub.check_output calls in meta/lib/oe/.  Looking at Popen calls next.
Comment 7 Stephano Cetola 2016-10-06 04:06:40 UTC
As I slowly pull apart all the strings, pipes, and redirects in package_manager.py, I am reminded of a rat infested spool of razorwire surrounded by bees.

With any luck this should be done tomorrow.
Comment 8 Stephano Cetola 2016-10-07 03:36:12 UTC
Checked in the needed changes.
Comment 9 Juan Ramos 2017-07-11 17:38:44 UTC
According to Stephano, any test of the “package” and “package_manager” features in OE will test the changes. Since these are present in nightly test, Im changing this to verified.