[OE-core] [PATCH] classes/kernel-yocto: Cleanup getstatusoutput usage

[Richard Purdie]

On Wed, 2018-07-25 at 15:44 -0500, Joshua Watt wrote:
> On Wed, 2018-07-25 at 16:32 -0400, Bruce Ashfield wrote:
> > On Wed, Jul 25, 2018 at 4:20 PM, Bruce Ashfield
> > <bruce.ashfield at gmail.com> wrote:
> > > On Wed, Jul 25, 2018 at 4:17 PM, Burton, Ross <ross.burton at intel.
> > > co
> > > m> wrote:
> > > > On 25 July 2018 at 21:05, Bruce Ashfield <bruce.ashfield at gmail.
> > > > co
> > > > m> wrote:
> > > > > My question about the change .. is why ? I really don't like
> > > > > cleanup
> > > > > patches on principle, and want things pinned to a bug, a
> > > > > performance
> > > > > issue or something else.
> > > > 
> > > > See a post by RP to the oe-core list a few days ago: a general
> > > > cleanup
> > > > of Python code that spawns is well overdue.  In this case
> > > > oe.utils.getstatusoutpus is a wrapper that just calls a
> > > > deprecated
> > > > function in subprocess, is best to pass arguments as an array
> > > > instead
> > > > of string, best not to use a shell, etc.
> > > 
> > > ah ok.
> > > 
> > > I still don't like it, but it is what it is.
> > 
> > ... and I found the original email, I see the part about things
> > being
> > depreciated. Makes sense.
> > 
> > I'll queue it up and run some tests on some intentionally bad BSPs.
> 
> Works for me.
> 
> I wasn't sure about if the subprocess exceptions should be fatal or
> not. Suppressing them maintained the old behavior, but it didn't
> quite seem "correct" to me.

We should at least put a comment in about this as its not best practise
not to check return codes. I do understand it can make sense in some
cases. Part of the reason to clean these calls up is also to ensure we
illustrate best practise in OE-Core so that when people copy and paste
it to other layers, we don't end up with bad things spreading.

In general, using the "proper" calls encourages better error handling
so that when something does fail, the user gets decent error messages.
For that reason I'd usually recommend the stderr -> stdout mapping so
that errors are displayed if a command fails. I appreciate that its a
moot point here since you ignore failures but in general it is a good
practise to encourage. We've had a spate of problems recently where
errors have been hidden making problems much harder to understand and
debug.

Cheers,

Richard