[PATCH] lib/oe/utils: ignore stderr while checking compiler version

[PATCH] lib/oe/utils: ignore stderr while checking compiler version

[Lars Poeschel]

From: Lars Poeschel <poeschel@lemonage.de>

The functions for checking the C compiler version call the compiler with
the --version argument and capture stdout and stderr to extract the
version information from that. This does not work if something goes
wrong for the compiler and it then prints some warning to stderr. The
following regex will certainly fail in that case.
It is better to just concentrate on stdout where the real version is
printed to and ignore stderr. So we have a chance to get a version info
even if a waring or error happened.

Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
 meta/lib/oe/utils.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/meta/lib/oe/utils.py b/meta/lib/oe/utils.py
index 9a2187e36f..4c7f54c777 100644
--- a/meta/lib/oe/utils.py
+++ b/meta/lib/oe/utils.py
@@ -391,7 +391,7 @@ def get_host_compiler_version(d, taskcontextonly=False):
         # this breaks the install-buildtools use-case
         # env["PATH"] = d.getVar("PATH")
         output = subprocess.check_output("%s --version" % compiler, \
-                    shell=True, env=env, stderr=subprocess.STDOUT).decode("utf-8")
+                    shell=True, env=env).decode("utf-8")
     except subprocess.CalledProcessError as e:
         bb.fatal("Error running %s --version: %s" % (compiler, e.output.decode("utf-8")))
 
@@ -417,7 +417,7 @@ def host_gcc_version(d, taskcontextonly=False):
         env = os.environ.copy()
         env["PATH"] = d.getVar("PATH")
         output = subprocess.check_output("%s --version" % compiler, \
-                    shell=True, env=env, stderr=subprocess.STDOUT).decode("utf-8")
+                    shell=True, env=env).decode("utf-8")
     except subprocess.CalledProcessError as e:
         bb.fatal("Error running %s --version: %s" % (compiler, e.output.decode("utf-8")))
 
-- 
2.30.2

Re: [OE-core] [PATCH] lib/oe/utils: ignore stderr while checking compiler version

[Richard Purdie]

On Sat, 2021-03-27 at 02:41 +0100, Lars Poeschel wrote:
> From: Lars Poeschel <poeschel@lemonage.de>
> 
> The functions for checking the C compiler version call the compiler with
> the --version argument and capture stdout and stderr to extract the
> version information from that. This does not work if something goes
> wrong for the compiler and it then prints some warning to stderr. The
> following regex will certainly fail in that case.
> It is better to just concentrate on stdout where the real version is
> printed to and ignore stderr. So we have a chance to get a version info
> even if a waring or error happened.
> 
> Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
> ---
>  meta/lib/oe/utils.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

What kind of warning/error would a compiler generate that we should 
ignore here? This seems a little odd...

Cheers,

Richard

Re: [OE-core] [PATCH] lib/oe/utils: ignore stderr while checking compiler version

[Lars Poeschel]

On Sat, Mar 27, 2021 at 01:54:11PM +0000, Richard Purdie wrote:
> On Sat, 2021-03-27 at 02:41 +0100, Lars Poeschel wrote:
> > From: Lars Poeschel <poeschel@lemonage.de>
> > 
> > The functions for checking the C compiler version call the compiler with
> > the --version argument and capture stdout and stderr to extract the
> > version information from that. This does not work if something goes
> > wrong for the compiler and it then prints some warning to stderr. The
> > following regex will certainly fail in that case.
> > It is better to just concentrate on stdout where the real version is
> > printed to and ignore stderr. So we have a chance to get a version info
> > even if a waring or error happened.
> > 
> > Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
> > ---
> >  meta/lib/oe/utils.py | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> What kind of warning/error would a compiler generate that we should 
> ignore here? This seems a little odd...

Well, to be more specific: It seems to come from the shell, that then
invokes the compiler. Nevertheless it is stored in the output variable.
In my case it contains:
/nix/store/9ywr69qi622lrmx5nn88gk8jpmihy0dz-bash-4.4-p23/bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
/nix/store/9ywr69qi622lrmx5nn88gk8jpmihy0dz-bash-4.4-p23/bin/bash: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
gcc (GCC) 9.3.0
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Regards,
Lars

Re: [OE-core] [PATCH] lib/oe/utils: ignore stderr while checking compiler version

[Denys Dmytriyenko]

On Sun, Mar 28, 2021 at 04:25:02AM +0200, Lars Poeschel wrote:
> On Sat, Mar 27, 2021 at 01:54:11PM +0000, Richard Purdie wrote:
> > On Sat, 2021-03-27 at 02:41 +0100, Lars Poeschel wrote:
> > > From: Lars Poeschel <poeschel@lemonage.de>
> > > 
> > > The functions for checking the C compiler version call the compiler with
> > > the --version argument and capture stdout and stderr to extract the
> > > version information from that. This does not work if something goes
> > > wrong for the compiler and it then prints some warning to stderr. The
> > > following regex will certainly fail in that case.
> > > It is better to just concentrate on stdout where the real version is
> > > printed to and ignore stderr. So we have a chance to get a version info
> > > even if a waring or error happened.
> > > 
> > > Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
> > > ---
> > >  meta/lib/oe/utils.py | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > What kind of warning/error would a compiler generate that we should 
> > ignore here? This seems a little odd...
> 
> Well, to be more specific: It seems to come from the shell, that then
> invokes the compiler. Nevertheless it is stored in the output variable.
> In my case it contains:
> /nix/store/9ywr69qi622lrmx5nn88gk8jpmihy0dz-bash-4.4-p23/bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
> /nix/store/9ywr69qi622lrmx5nn88gk8jpmihy0dz-bash-4.4-p23/bin/bash: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)

It is common to prefix command invocations with LC_ALL=C to avoid such issues.
For example:

https://git.openembedded.org/openembedded-core/tree/scripts/lib/recipetool/append.py#n151
| helptext = subprocess.check_output('LC_ALL=C %s --help' % command, shell=True).decode('utf-8')


> gcc (GCC) 9.3.0
> Copyright (C) 2019 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

-- 
Denys

Re: [OE-core] [PATCH] lib/oe/utils: ignore stderr while checking compiler version

[Richard Purdie]

On Sun, 2021-03-28 at 00:35 -0400, Denys Dmytriyenko wrote:
> On Sun, Mar 28, 2021 at 04:25:02AM +0200, Lars Poeschel wrote:
> > On Sat, Mar 27, 2021 at 01:54:11PM +0000, Richard Purdie wrote:
> > > On Sat, 2021-03-27 at 02:41 +0100, Lars Poeschel wrote:
> > > > From: Lars Poeschel <poeschel@lemonage.de>
> > > > 
> > > > The functions for checking the C compiler version call the compiler with
> > > > the --version argument and capture stdout and stderr to extract the
> > > > version information from that. This does not work if something goes
> > > > wrong for the compiler and it then prints some warning to stderr. The
> > > > following regex will certainly fail in that case.
> > > > It is better to just concentrate on stdout where the real version is
> > > > printed to and ignore stderr. So we have a chance to get a version info
> > > > even if a waring or error happened.
> > > > 
> > > > Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
> > > > ---
> > > >  meta/lib/oe/utils.py | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > What kind of warning/error would a compiler generate that we should 
> > > ignore here? This seems a little odd...
> > 
> > Well, to be more specific: It seems to come from the shell, that then
> > invokes the compiler. Nevertheless it is stored in the output variable.
> > In my case it contains:
> > /nix/store/9ywr69qi622lrmx5nn88gk8jpmihy0dz-bash-4.4-p23/bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
> > /nix/store/9ywr69qi622lrmx5nn88gk8jpmihy0dz-bash-4.4-p23/bin/bash: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
> 
> It is common to prefix command invocations with LC_ALL=C to avoid such issues.
> For example:
> 
> https://git.openembedded.org/openembedded-core/tree/scripts/lib/recipetool/append.py#n151
> > helptext = subprocess.check_output('LC_ALL=C %s --help' % command, shell=True).decode('utf-8')

We have:

meta/conf/bitbake.conf:export LC_ALL = "en_US.UTF-8"

so if this is showing warnings, the rest of the build isn't going to be 
great either.

Why en_US.UTF-8? We need a utf-8 locale for python and a C utf-8 locale 
isn't standard. en_US is as close to standard across distros as we can get.

Adding LC_ALL=C everywhere isn't the right solution, we're certainly not 
prefxixing every shell command execution with that.

Cheers,

Richard

Re: [OE-core] [PATCH] lib/oe/utils: ignore stderr while checking compiler version

[Lars Poeschel]

On Sun, Mar 28, 2021 at 09:31:17AM +0100, Richard Purdie wrote:
> On Sun, 2021-03-28 at 00:35 -0400, Denys Dmytriyenko wrote:
> > On Sun, Mar 28, 2021 at 04:25:02AM +0200, Lars Poeschel wrote:
> > > On Sat, Mar 27, 2021 at 01:54:11PM +0000, Richard Purdie wrote:
> > > > On Sat, 2021-03-27 at 02:41 +0100, Lars Poeschel wrote:
> > > > > From: Lars Poeschel <poeschel@lemonage.de>
> > > > > 
> > > > > The functions for checking the C compiler version call the compiler with
> > > > > the --version argument and capture stdout and stderr to extract the
> > > > > version information from that. This does not work if something goes
> > > > > wrong for the compiler and it then prints some warning to stderr. The
> > > > > following regex will certainly fail in that case.
> > > > > It is better to just concentrate on stdout where the real version is
> > > > > printed to and ignore stderr. So we have a chance to get a version info
> > > > > even if a waring or error happened.
> > > > > 
> > > > > Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
> > > > > ---
> > > > >  meta/lib/oe/utils.py | 4 ++--
> > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > What kind of warning/error would a compiler generate that we should 
> > > > ignore here? This seems a little odd...
> > > 
> > > Well, to be more specific: It seems to come from the shell, that then
> > > invokes the compiler. Nevertheless it is stored in the output variable.
> > > In my case it contains:
> > > /nix/store/9ywr69qi622lrmx5nn88gk8jpmihy0dz-bash-4.4-p23/bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
> > > /nix/store/9ywr69qi622lrmx5nn88gk8jpmihy0dz-bash-4.4-p23/bin/bash: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
> > 
> > It is common to prefix command invocations with LC_ALL=C to avoid such issues.
> > For example:
> > 
> > https://git.openembedded.org/openembedded-core/tree/scripts/lib/recipetool/append.py#n151
> > > helptext = subprocess.check_output('LC_ALL=C %s --help' % command, shell=True).decode('utf-8')
> 
> We have:
> 
> meta/conf/bitbake.conf:export LC_ALL = "en_US.UTF-8"
> 
> so if this is showing warnings, the rest of the build isn't going to be 
> great either.

Well this may depend on what your expectations to a great build are.
With the small patch I proposed it at least builds me images that I can
run on my target. Does that qualify ? Would that justify the patch ?
I know building with nix is not a first class, maybe not even a second class
supported platform. And I know about the recommended locale.
I am willing to work out another solution, if you can point me in the
right direction.

> Why en_US.UTF-8? We need a utf-8 locale for python and a C utf-8 locale 
> isn't standard. en_US is as close to standard across distros as we can get.

Thanks for the explanation.

> Adding LC_ALL=C everywhere isn't the right solution, we're certainly not 
> prefxixing every shell command execution with that.

Besides that: I tried it and it does not work. It only gets me rid of the
second of the two warnings.

Regards,
Lars