All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [OE-core][PATCH v3] package_manager/ipk: do not pipe stderr to stdout
@ 2022-07-21  0:19 Shruthi Ravichandran
  2022-07-22 18:00 ` Alexander Kanavin
  0 siblings, 1 reply; 3+ messages in thread
From: Shruthi Ravichandran @ 2022-07-21  0:19 UTC (permalink / raw)
  To: Alexander Kanavin; +Cc: openembedded-core

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 <shruthi.ravichandran@ni.com>
---
 meta/lib/oe/package_manager/ipk/__init__.py | 23 ++++++++++++---------
 1 file changed, 13 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..7cbea0fa80 100644
--- a/meta/lib/oe/package_manager/ipk/__init__.py
+++ b/meta/lib/oe/package_manager/ipk/__init__.py
@@ -102,12 +102,14 @@ 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.run(cmd, capture_output=True, encoding="utf-8", shell=True)
+        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, proc.stderr))
+        elif proc.stderr:
+            bb.note("Command '%s' returned stderr: %s" % (cmd, proc.stderr))
+
+        return opkg_query(proc.stdout)

     def extract(self, pkg, pkg_info):
         """
@@ -445,15 +447,16 @@ 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.run(cmd, capture_output=True, encoding="utf-8", shell=True)
+        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, proc.stderr))
+        elif proc.stderr:
+            bb.note("Command '%s' returned stderr: %s" % (cmd, proc.stderr))

         bb.utils.remove(temp_rootfs, True)

-        return output
+        return proc.stdout

     def backup_packaging_data(self):
         # Save the opkglib for increment ipk image generation
--
2.20.1


> -----Original Message-----
> From: Alexander Kanavin <alex.kanavin@gmail.com>
> Sent: Wednesday, July 20, 2022 1:37 AM
> To: Shruthi Ravichandran <shruthi.ravichandran@ni.com>
> Cc: openembedded-core@lists.openembedded.org
> Subject: Re: [OE-core][PATCH v2] package_manager/ipk: do not pipe stderr to stdout
>
> 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
> <shruthi.ravichandran@ni.com> 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 <shruthi.ravichandran@ni.com>
> > ---
> >  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 <alex.kanavin@gmail.com>
> > > Sent: Friday, July 1, 2022 5:26 AM
> > > To: Shruthi Ravichandran <shruthi.ravichandran@ni.com>
> > > Cc: openembedded-core@lists.openembedded.org
> > > 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
> > > <shruthi.ravichandran@ni.com> 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 <alex.kanavin@gmail.com>
> > > > > Sent: Thursday, June 30, 2022 1:47 AM
> > > > > To: Shruthi Ravichandran <shruthi.ravichandran@ni.com>
> > > > > Cc: openembedded-core@lists.openembedded.org
> > > > > 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
> > > > > <shruthi.ravichandran@ni.com> 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 <alex.kanavin@gmail.com>
> > > > > > > Sent: Tuesday, June 28, 2022 1:33 PM
> > > > > > > To: Shruthi Ravichandran <shruthi.ravichandran@ni.com>
> > > > > > > Cc: openembedded-core@lists.openembedded.org
> > > > > > > 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 <shruthi.ravichandran@ni.com <mailto:shruthi.ravichandran@ni.com> >
> 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 <shruthi.ravichandran@ni.com <mailto:shruthi.ravichandran@ni.com> >
> > > > > > >       ---
> > > > > > >        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
> > > > > > >
> > > > > > >
> > > > > > >       -=-=-=-=-=-=-=-=-=-=-=-
> > > > > > >       Links: You receive all messages sent to this group.
> > > > > > >       View/Reply Online (#167354): https://urldefense.com/v3/__https://lists.openembedded.org/g/openembedded-
> > > > >
> > >
> core/message/167354__;!!FbZ0ZwI3Qg!omDrJfbrjlLbY2OMgsAgQrIcnap222jrjZJAhJX_BWhVJfMi09419QAHH1bVf1VafwMRcVuWBaQ5k8y6
> > > > > gk-W-P8eXeA$
> > > > > > > <https://urldefense.com/v3/__https://lists.openembedded.org/g/openembedded-
> > > > > > > core/message/167354__;!!FbZ0ZwI3Qg!qc0xbL3QBz_illnPIJoeHb6mGA1kBLndLjrGDjlfDwR7rkQcM8i7ZTte5y_GL4XNQru-
> > > > > > > qSvIV3p_DxpS6tCIX3jHMdg$>
> > > > > > >       Mute This Topic:
> > > > >
> > >
> https://urldefense.com/v3/__https://lists.openembedded.org/mt/92051989/1686489__;!!FbZ0ZwI3Qg!omDrJfbrjlLbY2OMgsAgQrIcnap222
> > > > > jrjZJAhJX_BWhVJfMi09419QAHH1bVf1VafwMRcVuWBaQ5k8y6gk-WvQyK1Es$
> > > > > > >
> > > > >
> > >
> <https://urldefense.com/v3/__https://lists.openembedded.org/mt/92051989/1686489__;!!FbZ0ZwI3Qg!qc0xbL3QBz_illnPIJoeHb6mGA1kB
> > > > > > > LndLjrGDjlfDwR7rkQcM8i7ZTte5y_GL4XNQru-qSvIV3p_DxpS6tCIs81jASw$>
> > > > > > >       Group Owner: openembedded-core+owner@lists.openembedded.org <mailto:openembedded-
> > > > > > > core%2Bowner@lists.openembedded.org>
> > > > > > >       Unsubscribe: https://urldefense.com/v3/__https://lists.openembedded.org/g/openembedded-
> > > > > core/unsub__;!!FbZ0ZwI3Qg!omDrJfbrjlLbY2OMgsAgQrIcnap222jrjZJAhJX_BWhVJfMi09419QAHH1bVf1VafwMRcVuWBaQ5k8y6gk-
> > > > > WWbrhU-c$
> > > > > > > <https://urldefense.com/v3/__https://lists.openembedded.org/g/openembedded-
> > > > > > > core/unsub__;!!FbZ0ZwI3Qg!qc0xbL3QBz_illnPIJoeHb6mGA1kBLndLjrGDjlfDwR7rkQcM8i7ZTte5y_GL4XNQru-
> > > > > qSvIV3p_DxpS6tCIADJRjJc$>
> > > > > > > [alex.kanavin@gmail.com <mailto:alex.kanavin@gmail.com> ]
> > > > > > >       -=-=-=-=-=-=-=-=-=-=-=-
> > > > > > >
> > > > > > >
> > > > > >
> > > > > >
> > > > > > INTERNAL - NI CONFIDENTIAL
> > > >
> > > > INTERNAL - NI CONFIDENTIAL
> >
> > INTERNAL - NI CONFIDENTIAL

INTERNAL - NI CONFIDENTIAL

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [OE-core][PATCH v3] package_manager/ipk: do not pipe stderr to stdout
  2022-07-21  0:19 [OE-core][PATCH v3] package_manager/ipk: do not pipe stderr to stdout Shruthi Ravichandran
@ 2022-07-22 18:00 ` Alexander Kanavin
  0 siblings, 0 replies; 3+ messages in thread
From: Alexander Kanavin @ 2022-07-22 18:00 UTC (permalink / raw)
  To: Shruthi Ravichandran; +Cc: openembedded-core

Thanks, this looks fine. Can you resend the patch using 'git
send-email'? I'm not sure if sending the patch as a quoted reply to me
allows it to be picked from the mailing list.

Alex

On Thu, 21 Jul 2022 at 02:19, Shruthi Ravichandran
<shruthi.ravichandran@ni.com> 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 <shruthi.ravichandran@ni.com>
> ---
>  meta/lib/oe/package_manager/ipk/__init__.py | 23 ++++++++++++---------
>  1 file changed, 13 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..7cbea0fa80 100644
> --- a/meta/lib/oe/package_manager/ipk/__init__.py
> +++ b/meta/lib/oe/package_manager/ipk/__init__.py
> @@ -102,12 +102,14 @@ 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.run(cmd, capture_output=True, encoding="utf-8", shell=True)
> +        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, proc.stderr))
> +        elif proc.stderr:
> +            bb.note("Command '%s' returned stderr: %s" % (cmd, proc.stderr))
> +
> +        return opkg_query(proc.stdout)
>
>      def extract(self, pkg, pkg_info):
>          """
> @@ -445,15 +447,16 @@ 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.run(cmd, capture_output=True, encoding="utf-8", shell=True)
> +        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, proc.stderr))
> +        elif proc.stderr:
> +            bb.note("Command '%s' returned stderr: %s" % (cmd, proc.stderr))
>
>          bb.utils.remove(temp_rootfs, True)
>
> -        return output
> +        return proc.stdout
>
>      def backup_packaging_data(self):
>          # Save the opkglib for increment ipk image generation
> --
> 2.20.1
>
>
> > -----Original Message-----
> > From: Alexander Kanavin <alex.kanavin@gmail.com>
> > Sent: Wednesday, July 20, 2022 1:37 AM
> > To: Shruthi Ravichandran <shruthi.ravichandran@ni.com>
> > Cc: openembedded-core@lists.openembedded.org
> > Subject: Re: [OE-core][PATCH v2] package_manager/ipk: do not pipe stderr to stdout
> >
> > 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
> > <shruthi.ravichandran@ni.com> 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 <shruthi.ravichandran@ni.com>
> > > ---
> > >  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 <alex.kanavin@gmail.com>
> > > > Sent: Friday, July 1, 2022 5:26 AM
> > > > To: Shruthi Ravichandran <shruthi.ravichandran@ni.com>
> > > > Cc: openembedded-core@lists.openembedded.org
> > > > 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
> > > > <shruthi.ravichandran@ni.com> 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 <alex.kanavin@gmail.com>
> > > > > > Sent: Thursday, June 30, 2022 1:47 AM
> > > > > > To: Shruthi Ravichandran <shruthi.ravichandran@ni.com>
> > > > > > Cc: openembedded-core@lists.openembedded.org
> > > > > > 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
> > > > > > <shruthi.ravichandran@ni.com> 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 <alex.kanavin@gmail.com>
> > > > > > > > Sent: Tuesday, June 28, 2022 1:33 PM
> > > > > > > > To: Shruthi Ravichandran <shruthi.ravichandran@ni.com>
> > > > > > > > Cc: openembedded-core@lists.openembedded.org
> > > > > > > > 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 <shruthi.ravichandran@ni.com <mailto:shruthi.ravichandran@ni.com> >
> > 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 <shruthi.ravichandran@ni.com <mailto:shruthi.ravichandran@ni.com> >
> > > > > > > >       ---
> > > > > > > >        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
> > > > > > > >
> > > > > > > >
> > > > > > > >       -=-=-=-=-=-=-=-=-=-=-=-
> > > > > > > >       Links: You receive all messages sent to this group.
> > > > > > > >       View/Reply Online (#167354): https://urldefense.com/v3/__https://lists.openembedded.org/g/openembedded-
> > > > > >
> > > >
> > core/message/167354__;!!FbZ0ZwI3Qg!omDrJfbrjlLbY2OMgsAgQrIcnap222jrjZJAhJX_BWhVJfMi09419QAHH1bVf1VafwMRcVuWBaQ5k8y6
> > > > > > gk-W-P8eXeA$
> > > > > > > > <https://urldefense.com/v3/__https://lists.openembedded.org/g/openembedded-
> > > > > > > > core/message/167354__;!!FbZ0ZwI3Qg!qc0xbL3QBz_illnPIJoeHb6mGA1kBLndLjrGDjlfDwR7rkQcM8i7ZTte5y_GL4XNQru-
> > > > > > > > qSvIV3p_DxpS6tCIX3jHMdg$>
> > > > > > > >       Mute This Topic:
> > > > > >
> > > >
> > https://urldefense.com/v3/__https://lists.openembedded.org/mt/92051989/1686489__;!!FbZ0ZwI3Qg!omDrJfbrjlLbY2OMgsAgQrIcnap222
> > > > > > jrjZJAhJX_BWhVJfMi09419QAHH1bVf1VafwMRcVuWBaQ5k8y6gk-WvQyK1Es$
> > > > > > > >
> > > > > >
> > > >
> > <https://urldefense.com/v3/__https://lists.openembedded.org/mt/92051989/1686489__;!!FbZ0ZwI3Qg!qc0xbL3QBz_illnPIJoeHb6mGA1kB
> > > > > > > > LndLjrGDjlfDwR7rkQcM8i7ZTte5y_GL4XNQru-qSvIV3p_DxpS6tCIs81jASw$>
> > > > > > > >       Group Owner: openembedded-core+owner@lists.openembedded.org <mailto:openembedded-
> > > > > > > > core%2Bowner@lists.openembedded.org>
> > > > > > > >       Unsubscribe: https://urldefense.com/v3/__https://lists.openembedded.org/g/openembedded-
> > > > > > core/unsub__;!!FbZ0ZwI3Qg!omDrJfbrjlLbY2OMgsAgQrIcnap222jrjZJAhJX_BWhVJfMi09419QAHH1bVf1VafwMRcVuWBaQ5k8y6gk-
> > > > > > WWbrhU-c$
> > > > > > > > <https://urldefense.com/v3/__https://lists.openembedded.org/g/openembedded-
> > > > > > > > core/unsub__;!!FbZ0ZwI3Qg!qc0xbL3QBz_illnPIJoeHb6mGA1kBLndLjrGDjlfDwR7rkQcM8i7ZTte5y_GL4XNQru-
> > > > > > qSvIV3p_DxpS6tCIADJRjJc$>
> > > > > > > > [alex.kanavin@gmail.com <mailto:alex.kanavin@gmail.com> ]
> > > > > > > >       -=-=-=-=-=-=-=-=-=-=-=-
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > INTERNAL - NI CONFIDENTIAL
> > > > >
> > > > > INTERNAL - NI CONFIDENTIAL
> > >
> > > INTERNAL - NI CONFIDENTIAL
>
> INTERNAL - NI CONFIDENTIAL


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [OE-core][PATCH v3] package_manager/ipk: do not pipe stderr to stdout
@ 2022-07-22 18:54 Shruthi Ravichandran
  0 siblings, 0 replies; 3+ messages in thread
From: Shruthi Ravichandran @ 2022-07-22 18:54 UTC (permalink / raw)
  To: openembedded-core; +Cc: Shruthi Ravichandran

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 <shruthi.ravichandran@ni.com>
---
 meta/lib/oe/package_manager/ipk/__init__.py | 23 ++++++++++++---------
 1 file changed, 13 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..7cbea0fa80 100644
--- a/meta/lib/oe/package_manager/ipk/__init__.py
+++ b/meta/lib/oe/package_manager/ipk/__init__.py
@@ -102,12 +102,14 @@ 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.run(cmd, capture_output=True, encoding="utf-8", shell=True)
+        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, proc.stderr))
+        elif proc.stderr:
+            bb.note("Command '%s' returned stderr: %s" % (cmd, proc.stderr))
+
+        return opkg_query(proc.stdout)
 
     def extract(self, pkg, pkg_info):
         """
@@ -445,15 +447,16 @@ 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.run(cmd, capture_output=True, encoding="utf-8", shell=True)
+        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, proc.stderr))
+        elif proc.stderr:
+            bb.note("Command '%s' returned stderr: %s" % (cmd, proc.stderr))
 
         bb.utils.remove(temp_rootfs, True)
 
-        return output
+        return proc.stdout
 
     def backup_packaging_data(self):
         # Save the opkglib for increment ipk image generation
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-07-22 18:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-21  0:19 [OE-core][PATCH v3] package_manager/ipk: do not pipe stderr to stdout Shruthi Ravichandran
2022-07-22 18:00 ` Alexander Kanavin
2022-07-22 18:54 Shruthi Ravichandran

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.