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

[Simon Glass]

Hi Tom,

On Thu, 20 Jun 2024 at 17:30, Tom Rini <trini at konsulko.com> wrote:
>
> 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 at 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 at 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 at 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?

Not its own PATH: the one it passes to U-Boot's 'make'.

I'm not sure why, actually. It is such a long time ago that I don't remember.

I see:

~/.buildman-toolchains/gcc-13.2.0-nolibc/arm-linux-gnueabi/bin/arm-linux-gnueabi-ld

and

~/.buildman-toolchains/gcc-13.2.0-nolibc/arm-linux-gnueabi/arm-linux-gnueabi/bin/ld

but interestingly there is no gcc in the latter directory, which there
was in 4.6 (and presumably for some time after).

Certainly for sandbox there is no prefix, so we cannot add it in that
case, and sandbox is actually the arch used to run these tests.

What are you suggesting we change about this patch?

Regards,
Simon