* [PATCH v2 4/6] buildman: Support building within a Python venv
2024-06-21 13:14 [PATCH v2 0/6] Add Binman code-coverage test to CI Simon Glass
` (2 preceding siblings ...)
2024-06-21 13:14 ` [PATCH v2 3/6] buildman: Fix a few typos in toolchain code Simon Glass
@ 2024-06-21 13:14 ` Simon Glass
2024-06-21 13:14 ` [PATCH v2 5/6] u_boot_pylib: Use correct coverage tool within venv Simon Glass
2024-06-21 13:14 ` [PATCH v2 6/6] CI: Run code-coverage test for Binman Simon Glass
5 siblings, 0 replies; 9+ messages in thread
From: Simon Glass @ 2024-06-21 13:14 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Tom Rini, Simon Glass, Alexey Brodkin, Heinrich Schuchardt,
Quentin Schulz
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>
---
Changes in v2:
- Fix 'envronment' typo
tools/buildman/bsettings.py | 3 ++
tools/buildman/test.py | 83 +++++++++++++++++++++++++++++++++++++
tools/buildman/toolchain.py | 31 ++++++++++++--
3 files changed, 113 insertions(+), 4 deletions(-)
diff --git a/tools/buildman/bsettings.py b/tools/buildman/bsettings.py
index e225ac2ca0f..1be1d45e0fa 100644
--- a/tools/buildman/bsettings.py
+++ b/tools/buildman/bsettings.py
@@ -31,6 +31,9 @@ def setup(fname=''):
def add_file(data):
settings.readfp(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 f92add7a7c5..83219182fe0 100644
--- a/tools/buildman/test.py
+++ b/tools/buildman/test.py
@@ -146,6 +146,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
@@ -747,6 +748,88 @@ class TestBuild(unittest.TestCase):
self.assertEqual([
['MARY="mary"', 'Missing expected line: CONFIG_MARY="mary"']], result)
+ 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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 5/6] u_boot_pylib: Use correct coverage tool within venv
2024-06-21 13:14 [PATCH v2 0/6] Add Binman code-coverage test to CI Simon Glass
` (3 preceding siblings ...)
2024-06-21 13:14 ` [PATCH v2 4/6] buildman: Support building within a Python venv Simon Glass
@ 2024-06-21 13:14 ` Simon Glass
2024-06-21 13:14 ` [PATCH v2 6/6] CI: Run code-coverage test for Binman Simon Glass
5 siblings, 0 replies; 9+ messages in thread
From: Simon Glass @ 2024-06-21 13:14 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Simon Glass
When running within a Python venv we must use the 'coverage' tool (which
is within the venv) so that the venv packages are used in preference to
system packages. Otherwise the coverage tests run in a different
environment from the normal tests and may fail due to missing packages.
Handle this by detecting the venv and changing the tool name.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
tools/u_boot_pylib/test_util.py | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/tools/u_boot_pylib/test_util.py b/tools/u_boot_pylib/test_util.py
index f18d385d995..857ce58c98c 100644
--- a/tools/u_boot_pylib/test_util.py
+++ b/tools/u_boot_pylib/test_util.py
@@ -60,12 +60,17 @@ def run_test_coverage(prog, filter_fname, exclude_list, build_dir, required=None
prefix = ''
if build_dir:
prefix = 'PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools ' % build_dir
- cmd = ('%spython3-coverage run '
- '--omit "%s" %s %s %s %s' % (prefix, ','.join(glob_list),
+
+ # Detect a Python virtualenv and use 'coverage' instead
+ covtool = ('python3-coverage' if sys.prefix == sys.base_prefix else
+ 'coverage')
+
+ cmd = ('%s%s run '
+ '--omit "%s" %s %s %s %s' % (prefix, covtool, ','.join(glob_list),
prog, extra_args or '', test_cmd,
single_thread or '-P1'))
os.system(cmd)
- stdout = command.output('python3-coverage', 'report')
+ stdout = command.output(covtool, 'report')
lines = stdout.splitlines()
if required:
# Convert '/path/to/name.py' just the module name 'name'
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 6/6] CI: Run code-coverage test for Binman
2024-06-21 13:14 [PATCH v2 0/6] Add Binman code-coverage test to CI Simon Glass
` (4 preceding siblings ...)
2024-06-21 13:14 ` [PATCH v2 5/6] u_boot_pylib: Use correct coverage tool within venv Simon Glass
@ 2024-06-21 13:14 ` Simon Glass
5 siblings, 0 replies; 9+ messages in thread
From: Simon Glass @ 2024-06-21 13:14 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Tom Rini, Simon Glass, Andrejs Cainikovs, Leo Yu-Chi Liang,
Marek Vasut, Sean Anderson
Binman includes a good set of tests covering all of its functionality.
This includes a code-coverage test.
However to date the code-coverage test has not been checked
automatically by CI, relying on people to run 'binman test -T'
themselves.
Plug the gap to avoid bugs creeping in future.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Add to azure also (oops)
.azure-pipelines.yml | 5 ++++-
.gitlab-ci.yml | 4 +++-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index 27f69583c65..65d1d639e49 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -128,7 +128,10 @@ stages:
export PATH=${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc:${PATH}
./tools/buildman/buildman -T0 -o ${UBOOT_TRAVIS_BUILD_DIR} -w --board tools-only
set -ex
- ./tools/binman/binman --toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools test
+ export TOOLPATH="--toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools --toolpath /opt/coreboot"
+ ./tools/binman/binman ${TOOLPATH} test
+ # Avoid "Permission denied: 'cov'" error by using a temporary file
+ COVERAGE_FILE=/tmp/.coverage ./tools/binman/binman ${TOOLPATH} test -T
./tools/buildman/buildman -t
./tools/dtoc/dtoc -t
./tools/patman/patman test
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 165f765a833..eb01fa4868d 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -201,7 +201,9 @@ Run binman, buildman, dtoc, Kconfig and patman testsuites:
./tools/buildman/buildman -T0 -o ${UBOOT_TRAVIS_BUILD_DIR} -w
--board tools-only;
set -e;
- ./tools/binman/binman --toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools test;
+ export TOOLPATH="--toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools --toolpath /opt/coreboot";
+ ./tools/binman/binman ${TOOLPATH} test;
+ ./tools/binman/binman ${TOOLPATH} test -T;
./tools/buildman/buildman -t;
./tools/dtoc/dtoc -t;
./tools/patman/patman test;
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread