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

Simon Glass
2024-08-15T19:58:20Z

The Python virtualenv tool sets up a few things in the environment,
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>
---

(no changes since v2)

Changes in v2:
- Fix 'envronment' typo

tools/buildman/bsettings.py | 3 ++
tools/buildman/test.py | 84 +++++++++++++++++++++++++++++++++++++
tools/buildman/toolchain.py | 31 ++++++++++++--
3 files changed, 114 insertions(+), 4 deletions(-)

diff --git a/tools/buildman/bsettings.py b/tools/buildman/bsettings.py
index aea724fc559..a7358cfc08a 100644
--- a/tools/buildman/bsettings.py
+++ b/tools/buildman/bsettings.py
@@ -31,6 +31,9 @@ def setup(fname=''):
def add_file(data):
settings.read_file(io.StringIO(data))

+def add_section(name):
+ settings.add_section(name)
+
def get_items(section):
"""Get the items from a section of the config.

diff --git a/tools/buildman/test.py b/tools/buildman/test.py
index bfad3093030..6191af95445 100644
--- a/tools/buildman/test.py
+++ b/tools/buildman/test.py
@@ -148,6 +148,7 @@ class TestBuild(unittest.TestCase):
self.toolchains.Add('arm-linux-gcc', test=False)
self.toolchains.Add('sparc-linux-gcc', test=False)
self.toolchains.Add('powerpc-linux-gcc', test=False)
+ self.toolchains.Add('/path/to/aarch64-linux-gcc', test=False)
self.toolchains.Add('gcc', test=False)

# Avoid sending any output
@@ -868,6 +869,89 @@ class TestBuild(unittest.TestCase):
self.assertEqual([4, 5], control.read_procs(tmpdir))
self.assertEqual(self.finish_time, self.cur_time)

+ def call_make_environment(self, tchn, full_path, in_env=None):
+ """Call Toolchain.MakeEnvironment() and process the result
+
+ Args:
+ tchn (Toolchain): Toolchain to use
+ full_path (bool): True to return the full path in CROSS_COMPILE
+ rather than adding it to the PATH variable
+ in_env (dict): Input environment to use, None to use current env
+
+ Returns:
+ tuple:
+ dict: Changes that MakeEnvironment has made to the environment
+ key: Environment variable that was changed
+ value: New value (for PATH this only includes components
+ which were added)
+ str: Full value of the new PATH variable
+ """
+ env = tchn.MakeEnvironment(full_path, env=in_env)
+
+ # Get the original environment
+ orig_env = dict(os.environb if in_env is None else in_env)
+ orig_path = orig_env[b'PATH'].split(b':')
+
+ # Find new variables
+ diff = dict((k, env[k]) for k in env if orig_env.get(k) != env[k])
+
+ # Find new / different path components
+ diff_path = None
+ new_path = None
+ if b'PATH' in diff:
+ new_path = diff[b'PATH'].split(b':')
+ diff_paths = [p for p in new_path if p not in orig_path]
+ diff_path = b':'.join(p for p in new_path if p not in orig_path)
+ if diff_path:
+ diff[b'PATH'] = diff_path
+ else:
+ del diff[b'PATH']
+ return diff, new_path
+
+ def test_toolchain_env(self):
+ """Test PATH and other environment settings for toolchains"""
+ # Use a toolchain which has a path, so that full_path makes a difference
+ tchn = self.toolchains.Select('aarch64')
+
+ # Normal cases
+ diff = self.call_make_environment(tchn, full_path=False)[0]
+ self.assertEqual(
+ {b'CROSS_COMPILE': b'aarch64-linux-', b'LC_ALL': b'C',
+ b'PATH': b'/path/to'}, diff)
+
+ diff = self.call_make_environment(tchn, full_path=True)[0]
+ self.assertEqual(
+ {b'CROSS_COMPILE': b'/path/to/aarch64-linux-', b'LC_ALL': b'C'},
+ diff)
+
+ # When overriding the toolchain, only LC_ALL should be set
+ tchn.override_toolchain = True
+ diff = self.call_make_environment(tchn, full_path=True)[0]
+ self.assertEqual({b'LC_ALL': b'C'}, diff)
+
+ # Test that virtualenv is handled correctly
+ tchn.override_toolchain = False
+ sys.prefix = '/some/venv'
+ env = dict(os.environb)
+ env[b'PATH'] = b'/some/venv/bin:other/things'
+ tchn.path = '/my/path'
+ diff, diff_path = self.call_make_environment(tchn, False, env)
+
+ self.assertIn(b'PATH', diff)
+ self.assertEqual([b'/some/venv/bin', b'/my/path', b'other/things'],
+ diff_path)
+ self.assertEqual(
+ {b'CROSS_COMPILE': b'aarch64-linux-', b'LC_ALL': b'C',
+ b'PATH': b'/my/path'}, diff)
+
+ # Handle a toolchain wrapper
+ tchn.path = ''
+ bsettings.add_section('toolchain-wrapper')
+ bsettings.set_item('toolchain-wrapper', 'my-wrapper', 'fred')
+ diff = self.call_make_environment(tchn, full_path=True)[0]
+ self.assertEqual(
+ {b'CROSS_COMPILE': b'fred aarch64-linux-', b'LC_ALL': b'C'}, diff)
+

if __name__ == "__main__":
unittest.main()
diff --git a/tools/buildman/toolchain.py b/tools/buildman/toolchain.py
index 324ad0e0821..6ca79c2c0f9 100644
--- a/tools/buildman/toolchain.py
+++ b/tools/buildman/toolchain.py
@@ -172,13 +172,14 @@ class Toolchain:
else:
raise ValueError('Unknown arg to GetEnvArgs (%d)' % which)

- def MakeEnvironment(self, full_path):
+ def MakeEnvironment(self, full_path, env=None):
"""Returns an environment for using the toolchain.

This takes the current environment and adds CROSS_COMPILE so that
the tool chain will operate correctly. This also disables localized
output and possibly Unicode encoded output of all build tools by
- adding LC_ALL=C.
+ adding LC_ALL=C. For the case where full_path is False, it prepends
+ the toolchain to PATH

Note that os.environb is used to obtain the environment, since in some
cases the environment many contain non-ASCII characters and we see
@@ -187,15 +188,21 @@ class Toolchain:
UnicodeEncodeError: 'utf-8' codec can't encode characters in position
569-570: surrogates not allowed

+ When running inside a Python venv, care is taken not to put the
+ toolchain path before the venv path, so that builds initiated by
+ buildman will still respect the venv.
+
Args:
full_path: Return the full path in CROSS_COMPILE and don't set
PATH
+ env (dict of bytes): Original environment, used for testing
Returns:
Dict containing the (bytes) environment to use. This is based on the
current environment, with changes as needed to CROSS_COMPILE, PATH
and LC_ALL.
"""
- env = dict(os.environb)
+ env = dict(env or os.environb)
+
wrapper = self.GetWrapper()

if self.override_toolchain:
@@ -206,7 +213,23 @@ class Toolchain:
wrapper + os.path.join(self.path, self.cross))
else:
env[b'CROSS_COMPILE'] = tools.to_bytes(wrapper + self.cross)
- env[b'PATH'] = tools.to_bytes(self.path) + b':' + env[b'PATH']
+
+ # Detect a Python virtualenv and avoid defeating it
+ if sys.prefix != sys.base_prefix:
+ paths = env[b'PATH'].split(b':')
+ new_paths = []
+ to_insert = tools.to_bytes(self.path)
+ insert_after = tools.to_bytes(sys.prefix)
+ for path in paths:
+ new_paths.append(path)
+ if to_insert and path.startswith(insert_after):
+ new_paths.append(to_insert)
+ to_insert = None
+ if to_insert:
+ new_paths.append(to_insert)
+ env[b'PATH'] = b':'.join(new_paths)
+ else:
+ env[b'PATH'] = tools.to_bytes(self.path) + b':' + env[b'PATH']

env[b'LC_ALL'] = b'C'

--
2.34.1