Please use subprocess.run() (which allows separate capture of stderr),
rather than switch the whole thing to popen.

Alex

On Tue, 19 Jul 2022 at 23:51, Shruthi Ravichandran
<[email protected]> wrote:
>
> Some opkg commands print an error during cleanup when the tmp_dir
> does not exist and an attempt is made to delete it. The error messages
> are harmless and the opkg commands eventually succeed.
> When these commands are run and stderr is piped to stdout, the error
> messages may clobber the stdout and cause unexpected results while
> parsing the output of the command. Therefore, when parsing the output
> of a command, do not pipe stderr to stdout. Instead, capture stderr
> and stdout separately, and upon success, send stderr to bb.note().
>
> Signed-off-by: Shruthi Ravichandran <[email protected]>
> ---
>  meta/lib/oe/package_manager/ipk/__init__.py | 30 ++++++++++++++-------
>  1 file changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/meta/lib/oe/package_manager/ipk/__init__.py 
> b/meta/lib/oe/package_manager/ipk/__init__.py
> index 6fd2f021b6..a768de5c30 100644
> --- a/meta/lib/oe/package_manager/ipk/__init__.py
> +++ b/meta/lib/oe/package_manager/ipk/__init__.py
> @@ -102,12 +102,17 @@ class OpkgDpkgPM(PackageManager):
>          This method extracts the common parts for Opkg and Dpkg
>          """
>
> -        try:
> -            output = subprocess.check_output(cmd, stderr=subprocess.STDOUT, 
> shell=True).decode("utf-8")
> -        except subprocess.CalledProcessError as e:
> +        proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, 
> stderr=subprocess.PIPE, shell=True)
> +        cmd_stdout, cmd_stderr = proc.communicate()
> +        cmd_stdout = cmd_stdout.decode("utf-8")
> +        cmd_stderr = cmd_stderr.decode("utf-8")
> +        if proc.returncode:
>              bb.fatal("Unable to list available packages. Command '%s' "
> -                     "returned %d:\n%s" % (cmd, e.returncode, 
> e.output.decode("utf-8")))
> -        return opkg_query(output)
> +                    "returned %d:\n%s" % (cmd, proc.returncode, cmd_stderr))
> +        elif cmd_stderr:
> +            bb.note("Command '%s' returned stderr: %s" % (cmd, cmd_stderr))
> +
> +        return opkg_query(cmd_stdout)
>
>      def extract(self, pkg, pkg_info):
>          """
> @@ -445,15 +450,20 @@ class OpkgPM(OpkgDpkgPM):
>          cmd = "%s %s --noaction install %s " % (self.opkg_cmd,
>                                                  opkg_args,
>                                                  ' '.join(pkgs))
> -        try:
> -            output = subprocess.check_output(cmd, stderr=subprocess.STDOUT, 
> shell=True)
> -        except subprocess.CalledProcessError as e:
> +
> +        proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, 
> stderr=subprocess.PIPE, shell=True)
> +        cmd_stdout, cmd_stderr = proc.communicate()
> +        cmd_stdout = cmd_stdout.decode("utf-8")
> +        cmd_stderr = cmd_stderr.decode("utf-8")
> +        if proc.returncode:
>              bb.fatal("Unable to dummy install packages. Command '%s' "
> -                     "returned %d:\n%s" % (cmd, e.returncode, 
> e.output.decode("utf-8")))
> +                     "returned %d:\n%s" % (cmd, proc.returncode, cmd_stderr))
> +        elif cmd_stderr:
> +            bb.note("Command '%s' returned stderr: %s" % (cmd, cmd_stderr))
>
>          bb.utils.remove(temp_rootfs, True)
>
> -        return output
> +        return cmd_stdout
>
>      def backup_packaging_data(self):
>          # Save the opkglib for increment ipk image generation
> --
> 2.20.1
>
>
> > -----Original Message-----
> > From: Alexander Kanavin <[email protected]>
> > Sent: Friday, July 1, 2022 5:26 AM
> > To: Shruthi Ravichandran <[email protected]>
> > Cc: [email protected]
> > Subject: Re: [OE-core][PATCH] package_manager/ipk: do not pipe stderr to 
> > stdout
> >
> > Yes, certainly.
> >
> > Alex
> >
> > On Fri, 1 Jul 2022 at 00:30, Shruthi Ravichandran
> > <[email protected]> wrote:
> > >
> > > The change currently does discard everything in stderr. I can update
> > > it to capture stderr and push it to bb.note on command success and
> > > bb.fatal on command failure. In fact, I can make those changes to the
> > > several other instances in this file where stderr is piped to stdout
> > > too. Would that be acceptable?
> > >
> > > Shruthi
> > >
> > > > -----Original Message-----
> > > > From: Alexander Kanavin <[email protected]>
> > > > Sent: Thursday, June 30, 2022 1:47 AM
> > > > To: Shruthi Ravichandran <[email protected]>
> > > > Cc: [email protected]
> > > > Subject: Re: [OE-core][PATCH] package_manager/ipk: do not pipe stderr 
> > > > to stdout
> > > >
> > > > Thanks for the information - perhaps this should be added to the commit 
> > > > message?
> > > >
> > > > Does this change discard things that appear on stderr completely, or
> > > > does it still go somewhere where it can be seen later?
> > > >
> > > > Alex
> > > >
> > > > On Wed, 29 Jun 2022 at 21:22, Shruthi Ravichandran
> > > > <[email protected]> wrote:
> > > > >
> > > > > Hi Alex,
> > > > >
> > > > > I've found that some OE commands print an error during cleanup when
> > > > > the tmp_dir does not exist and an attempt is made to delete it. I've
> > > > > submitted a patch to opkg to fix that.
> > > > > Link:
> > > >
> > https://urldefense.com/v3/__https://git.yoctoproject.org/opkg/commit/?id=8dfdda86afd407a66e3dc00a077bdcc8b53d54ea__;!!FbZ0ZwI
> > > > 3Qg!omDrJfbrjlLbY2OMgsAgQrIcnap222jrjZJAhJX_BWhVJfMi09419QAHH1bVf1VafwMRcVuWBaQ5k8y6gk-W-iLjh48$
> > > >  .
> > > > > That was the one instance that was causing an issue in our builds.
> > > > > There may be other instances that I don't know of. Given that, I think
> > > > > the package_manager code should be resilient against any such
> > > > > miscellaneous stderr messages, which do not result in the command
> > > > > itself failing.
> > > > >
> > > > > Hope that helps,
> > > > > Shruthi
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Alexander Kanavin <[email protected]>
> > > > > > Sent: Tuesday, June 28, 2022 1:33 PM
> > > > > > To: Shruthi Ravichandran <[email protected]>
> > > > > > Cc: [email protected]
> > > > > > Subject: Re: [OE-core][PATCH] package_manager/ipk: do not pipe 
> > > > > > stderr to stdout
> > > > > >
> > > > > > This needs additional justification. What are the error messages, 
> > > > > > why are they harmless and why the solution is to suppress them
> > instead
> > > > of
> > > > > > addressing the reasons they appear?
> > > > > >
> > > > > > Alex
> > > > > >
> > > > > > On Tue 28. Jun 2022 at 23.13, Shruthi Ravichandran 
> > > > > > <[email protected] <mailto:[email protected]> > 
> > > > > > wrote:
> > > > > >
> > > > > >
> > > > > >       When parsing the output of a command, do not pipe stderr to 
> > > > > > stdout.
> > > > > >       Opkg sometimes prints harmless error messages even when the 
> > > > > > opkg
> > > > > >       command succeeds. When stderr is piped to stdout, these error
> > > > > >       messages may clobber the stdout and cause unexpected results 
> > > > > > while
> > > > > >       parsing the output.
> > > > > >
> > > > > >       Signed-off-by: Shruthi Ravichandran 
> > > > > > <[email protected] <mailto:[email protected]> >
> > > > > >       ---
> > > > > >        meta/lib/oe/package_manager/ipk/__init__.py | 2 +-
> > > > > >        1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > >       diff --git a/meta/lib/oe/package_manager/ipk/__init__.py 
> > > > > > b/meta/lib/oe/package_manager/ipk/__init__.py
> > > > > >       index 4cd3963111..d7f3f31853 100644
> > > > > >       --- a/meta/lib/oe/package_manager/ipk/__init__.py
> > > > > >       +++ b/meta/lib/oe/package_manager/ipk/__init__.py
> > > > > >       @@ -103,7 +103,7 @@ class OpkgDpkgPM(PackageManager):
> > > > > >                """
> > > > > >
> > > > > >                try:
> > > > > >       -            output = subprocess.check_output(cmd, 
> > > > > > stderr=subprocess.STDOUT, shell=True).decode("utf-8")
> > > > > >       +            output = subprocess.check_output(cmd, 
> > > > > > shell=True).decode("utf-8")
> > > > > >                except subprocess.CalledProcessError as e:
> > > > > >                    bb.fatal("Unable to list available packages. 
> > > > > > Command '%s' "
> > > > > >                             "returned %d:\n%s" % (cmd, 
> > > > > > e.returncode, e.output.decode("utf-8")))
> > > > > >       --
> > > > > >       2.20.1
> > > > > >
> > > > > >
> > > > > >       
> > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > INTERNAL - NI CONFIDENTIAL
> > >
> > > INTERNAL - NI CONFIDENTIAL
>
> INTERNAL - NI CONFIDENTIAL
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#168294): 
https://lists.openembedded.org/g/openembedded-core/message/168294
Mute This Topic: https://lists.openembedded.org/mt/92493275/21656
Group Owner: [email protected]
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to