Re: [PATCH 3/5] buildman: Support building within a Python venv

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 2552 bytes --]

On Thu, Jun 20, 2024 at 05:05:30PM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 20 Jun 2024 at 08:32, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Jun 20, 2024 at 07:19:35AM -0600, Simon Glass wrote:
> >
> > > The Python virtualenv tool sets up a few things in the envronment,
> > > putting its path first in the PATH environment variable and setting up
> > > a sys.prefix different from the sys.base_prefix value.
> > >
> > > At present buildman puts the toolchain path first in PATH so that it can
> > > be found easily during the build. For sandbox this causes problems since
> > > /usr/bin/gcc (for example) results in '/usr/bin' being prepended to the
> > > PATH variable. As a result, the venv is partially disabled.
> > >
> > > The result is that sandbox builds within a venv ignore the venv, e.g.
> > > when looking for packages.
> > >
> > > Correct this by detecting the venv and adding the toolchain path after
> > > the venv path.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> >
> > Why are we using PATH at all in this case? Shouldn't we just be setting
> > CROSS_COMPILE=/full/path/to/the/prefix ?
> 
> This is the -p option to buildman. The original commit was:
> 
> commit bb1501f2c22c979961b735db775605cccedd98f6
> Author: Simon Glass <sjg@chromium.org>
> Date:   Mon Dec 1 17:34:00 2014 -0700
> 
>     buildman: Add an option to use the full tool chain path
> 
>     In some cases there may be multiple toolchains with the same name in the
>     path. Provide an option to use the full path in the CROSS_COMPILE
>     environment variable.
> 
>     Note: Wolfgang mentioned that this is dangerous since in some cases there
>     may be other tools on the path that are needed. So this is set up as an
>     option, not the default. I will need test confirmation (i.e. that this
>     commit fixes a real problem) before merging it.
> 
> As to why we don't always do this, well that is back in the mists of
> time, 10 years ago.
> 
> BTW, this is raising a point ("let's change the behaviour") separate
> from the goal of this commit, which is to fix a problem with venv,
> albeit that if we made -p the only option, then we could potentially
> drop all PATH changes. Perhaps toolchains are built differently now,
> such that they always invoke their tools using the same prefix and
> dir?

Wait, I'm confused. buildman internally updates its own PATH to avoid
calling CROSS_COMPILE with the full path due to a concern about
toolchain bugs?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]