Re: [OE-core] [PATCH] package_ipk.bbclass: print opkg output on error

["Khem Raj" <raj.khem@gmail.com>]

On Tue, May 19, 2020 at 10:07 PM Konrad Weihmann <kweihmann@outlook.com> wrote:
>
> Hi all,
>
> I don't like the idea of wrapping subprocess.check_* calls once more.
> IMHO too many possible workflows are possible so chances are low
> catching them all appropriately.
>
> check_output is a wrapper around subprocess.run, which is a wrapper
> around subprocess.popen which is wrapper of popen, so an additional
> wrapper doesn't sound like the best idea so far.
>

right, that's what I was contemplating as well.

> I'm okay with generalizing the output of a failed call so maybe just
> http://git.yoctoproject.org/cgit.cgi/poky/tree/bitbake/lib/bb/utils.py#n330
> could be used more widely.
>
> It all comes down to handling subprocess failures and in this particular
> case it was simply missing.
>
> Long story short, I'd rather fix them one by one (with respect to actual
> code flow) than abstracting things even more.
>
> Regards
> Konrad
>
> On 20.05.20 00:14, Khem Raj wrote:
> > On Tue, May 19, 2020 at 3:02 PM Richard Purdie
> > <richard.purdie@linuxfoundation.org> wrote:
> >>
> >> On Tue, 2020-05-19 at 20:36 +0200, Konrad Weihmann wrote:
> >>> in case the opkg command fails in before no output of the tool
> >>> itself was printed to assist the user with debugging the issue.
> >>> Print all output of the tool by using CalledProcessError wrapper around
> >>> the call
> >>>
> >>> Signed-off-by: Konrad Weihmann <kweihmann@outlook.com>
> >>> ---
> >>>   meta/classes/package_ipk.bbclass | 11 +++++++----
> >>>   1 file changed, 7 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/meta/classes/package_ipk.bbclass b/meta/classes/package_ipk.bbclass
> >>> index c008559e4a..5e7d552cc5 100644
> >>> --- a/meta/classes/package_ipk.bbclass
> >>> +++ b/meta/classes/package_ipk.bbclass
> >>> @@ -223,10 +223,13 @@ def ipk_write_pkg(pkg, d):
> >>>               conffiles.close()
> >>>
> >>>           os.chdir(basedir)
> >>> -        subprocess.check_output("PATH=\"%s\" %s %s %s" % (localdata.getVar("PATH"),
> >>> -                                                          d.getVar("OPKGBUILDCMD"), pkg, pkgoutdir),
> >>> -                                stderr=subprocess.STDOUT,
> >>> -                                shell=True)
> >>> +        try:
> >>> +            subprocess.check_output("PATH=\"%s\" %s %s %s" % (localdata.getVar("PATH"),
> >>> +                                                            d.getVar("OPKGBUILDCMD"), pkg, pkgoutdir),
> >>> +                                    stderr=subprocess.STDOUT,
> >>> +                                    shell=True)
> >>> +        except subprocess.CalledProcessError as exp:
> >>> +            bb.fatal("{} failed with {}: {}".format(exp.cmd, exp.returncode, exp.output.decode("utf-8")))
> >>>
> >>>           if d.getVar('IPK_SIGN_PACKAGES') == '1':
> >>>               ipkver = "%s-%s" % (d.getVar('PKGV'), d.getVar('PKGR'))
> >>
> >> I get the feeling we have a problem with our subprocess calls and
> >> debugging but its hard to know what to do about it. We can patch each
> >> call site like this but it starts to make we wonder whether we need a
> >> general wrapper, or we're missing something in bitbake or elsewhere to
> >> help with things.
> >>
> >> There is this code:
> >>
> >> http://git.yoctoproject.org/cgit.cgi/poky/tree/bitbake/lib/bb/utils.py#n378
> >>
> >> which is meant to help but it clearly isn't helping/catching all cases.
> >>
> >> I think we are going to need to look at a wider/more general solution
> >> or at least have a plan for one.
> >>
> >
> >
> > are you think of wrapping subprocess.check_output calls with some post
> > processing ?
> >
> >> Cheers,
> >>
> >> Richard
> >>
> >>