[PATCH v3 0/6] Add Binman code-coverage test to CI

Binman has 100% code coverage to ensure that future changes and refactors do not break existing entry types. This is a critical feature, given that it is relied on to produce images for all sorts of different SoCs and vendors.
With the NXP additions the 'binman test -T' step was missed, so the Binman coverage test is currently failing.
This series provides a means to close the testing gap. It cannot be applied until the tests are added, which should happen before -next is applied to -master
Changes in v3: - Drop the PATH modification altogether
Changes in v2: - Add new patch to fix a few typos in toolchain code - Add to azure also (oops) - Add to buildman requirements instead
Simon Glass (6): buildman: Add python3-coverage buildman: Add python3-pycryptodome buildman: Fix a few typos in toolchain code buildman: Always use the full path in CROSS_COMPILE u_boot_pylib: Use correct coverage tool within venv CI: Run code-coverage test for Binman
.azure-pipelines.yml | 5 ++- .gitlab-ci.yml | 4 +- tools/buildman/bsettings.py | 3 ++ tools/buildman/builder.py | 5 +-- tools/buildman/builderthread.py | 4 +- tools/buildman/cmdline.py | 2 - tools/buildman/control.py | 6 +-- tools/buildman/requirements.txt | 2 + tools/buildman/test.py | 75 +++++++++++++++++++++++++++++++++ tools/buildman/toolchain.py | 24 +++++------ tools/u_boot_pylib/test_util.py | 11 +++-- 11 files changed, 111 insertions(+), 30 deletions(-)

Add this package so we can run code-coverage tests for Binman.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Tom Rini trini@konsulko.com ---
(no changes since v2)
Changes in v2: - Add to buildman requirements instead
tools/buildman/requirements.txt | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/buildman/requirements.txt b/tools/buildman/requirements.txt index 4a31e69e4cb..564e54898a4 100644 --- a/tools/buildman/requirements.txt +++ b/tools/buildman/requirements.txt @@ -1,3 +1,4 @@ +coverage==6.2 jsonschema==4.17.3 pyyaml==6.0 yamllint==1.26.3

Add this package so we can run code-coverage tests for Binman.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Tom Rini trini@konsulko.com ---
(no changes since v2)
Changes in v2: - Add to buildman requirements instead
tools/buildman/requirements.txt | 1 + 1 file changed, 1 insertion(+)
Applied to u-boot-dm, thanks!

This is used by some Binman entry types, so add it to allow more tests to pass.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Tom Rini trini@konsulko.com ---
(no changes since v2)
Changes in v2: - Add to buildman requirements instead
tools/buildman/requirements.txt | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/buildman/requirements.txt b/tools/buildman/requirements.txt index 564e54898a4..052d0ed5c6f 100644 --- a/tools/buildman/requirements.txt +++ b/tools/buildman/requirements.txt @@ -1,4 +1,5 @@ coverage==6.2 jsonschema==4.17.3 +pycryptodome==3.20 pyyaml==6.0 yamllint==1.26.3

This is used by some Binman entry types, so add it to allow more tests to pass.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Tom Rini trini@konsulko.com ---
(no changes since v2)
Changes in v2: - Add to buildman requirements instead
tools/buildman/requirements.txt | 1 + 1 file changed, 1 insertion(+)
Applied to u-boot-dm, thanks!

Fix 'Thie' and capitalise 'unicode'.
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: Heinrich Schuchardt xypron.glpk@gmx.de ---
(no changes since v2)
Changes in v2: - Add new patch to fix a few typos in toolchain code
tools/buildman/toolchain.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/buildman/toolchain.py b/tools/buildman/toolchain.py index 79c7c11a110..324ad0e0821 100644 --- a/tools/buildman/toolchain.py +++ b/tools/buildman/toolchain.py @@ -175,9 +175,9 @@ class Toolchain: def MakeEnvironment(self, full_path): """Returns an environment for using the toolchain.
- Thie takes the current environment and adds CROSS_COMPILE so that + 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 + output and possibly Unicode encoded output of all build tools by adding LC_ALL=C.
Note that os.environb is used to obtain the environment, since in some

Fix 'Thie' and capitalise 'unicode'.
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: Heinrich Schuchardt xypron.glpk@gmx.de ---
(no changes since v2)
Changes in v2: - Add new patch to fix a few typos in toolchain code
tools/buildman/toolchain.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!

The feature to set the toolchain path does not seem to be needed. It causes problems with venv (see [1]). Let's remove it.
Add some tests while we are here.
It does not look like any docs changes are needed for this.
[1] https://patchwork.ozlabs.org/project/uboot/patch/20240621131423.2363294-6-sj...
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: Tom Rini trini@konsulko.com ---
Changes in v3: - Drop the PATH modification altogether
tools/buildman/bsettings.py | 3 ++ tools/buildman/builder.py | 5 +-- tools/buildman/builderthread.py | 4 +- tools/buildman/cmdline.py | 2 - tools/buildman/control.py | 6 +-- tools/buildman/test.py | 75 +++++++++++++++++++++++++++++++++ tools/buildman/toolchain.py | 20 ++++----- 7 files changed, 92 insertions(+), 23 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/builder.py b/tools/buildman/builder.py index f35175b4598..7c563cddada 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -255,7 +255,7 @@ class Builder:
def __init__(self, toolchains, base_dir, git_dir, num_threads, num_jobs, gnu_make='make', checkout=True, show_unknown=True, step=1, - no_subdirs=False, full_path=False, verbose_build=False, + no_subdirs=False, verbose_build=False, mrproper=False, per_board_out_dir=False, config_only=False, squash_config_y=False, warnings_as_errors=False, work_in_output=False, @@ -279,8 +279,6 @@ class Builder: step: 1 to process every commit, n to process every nth commit no_subdirs: Don't create subdirectories when building current source for a single board - full_path: Return the full path in CROSS_COMPILE and don't set - PATH verbose_build: Run build with V=1 and don't use 'make -s' mrproper: Always run 'make mrproper' when configuring per_board_out_dir: Build in a separate persistent directory per @@ -336,7 +334,6 @@ class Builder: self._step = step self._error_lines = 0 self.no_subdirs = no_subdirs - self.full_path = full_path self.verbose_build = verbose_build self.config_only = config_only self.squash_config_y = squash_config_y diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index a8599c0bb2a..c23c3254d2d 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -404,7 +404,7 @@ class BuilderThread(threading.Thread): the next incremental build """ # Set up the environment and command line - env = self.toolchain.MakeEnvironment(self.builder.full_path) + env = self.toolchain.MakeEnvironment() mkdir(out_dir)
args, cwd, src_dir = self._build_args(brd, out_dir, out_rel_dir, @@ -569,7 +569,7 @@ class BuilderThread(threading.Thread): outf.write(f'{result.return_code}')
# Write out the image and function size information and an objdump - env = result.toolchain.MakeEnvironment(self.builder.full_path) + env = result.toolchain.MakeEnvironment() with open(os.path.join(build_dir, 'out-env'), 'wb') as outf: for var in sorted(env.keys()): outf.write(b'%s="%s"' % (var, env[var])) diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py index 03211bd5aa5..5fda90508f2 100644 --- a/tools/buildman/cmdline.py +++ b/tools/buildman/cmdline.py @@ -121,8 +121,6 @@ def add_after_m(parser): help="Override host toochain to use for sandbox (e.g. 'clang-7')") parser.add_argument('-Q', '--quick', action='store_true', default=False, help='Do a rough build, with limited warning resolution') - parser.add_argument('-p', '--full-path', action='store_true', - default=False, help="Use full toolchain path in CROSS_COMPILE") parser.add_argument('-P', '--per-board-out-dir', action='store_true', default=False, help="Use an O= (output) directory per board rather than per thread") parser.add_argument('--print-arch', action='store_true', diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 8f6850c5211..3ca9e2e8761 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -653,10 +653,8 @@ def do_buildman(args, toolchains=None, make_func=None, brds=None, builder = Builder(toolchains, output_dir, git_dir, args.threads, args.jobs, checkout=True, show_unknown=args.show_unknown, step=args.step, - no_subdirs=args.no_subdirs, full_path=args.full_path, - verbose_build=args.verbose_build, - mrproper=args.mrproper, - per_board_out_dir=args.per_board_out_dir, + no_subdirs=args.no_subdirs, verbose_build=args.verbose_build, + mrproper=args.mrproper, per_board_out_dir=args.per_board_out_dir, config_only=args.config_only, squash_config_y=not args.preserve_config_y, warnings_as_errors=args.warnings_as_errors, diff --git a/tools/buildman/test.py b/tools/buildman/test.py index f92add7a7c5..ae9963eed4f 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,80 @@ class TestBuild(unittest.TestCase): self.assertEqual([ ['MARY="mary"', 'Missing expected line: CONFIG_MARY="mary"']], result)
+ def call_make_environment(self, tchn, in_env=None): + """Call Toolchain.MakeEnvironment() and process the result + + Args: + tchn (Toolchain): Toolchain to use + 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(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 + tchn = self.toolchains.Select('aarch64') + + # Normal case + diff = self.call_make_environment(tchn)[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)[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, env) + + self.assertNotIn(b'PATH', diff) + self.assertEqual(None, diff_path) + self.assertEqual( + {b'CROSS_COMPILE': b'/my/path/aarch64-linux-', b'LC_ALL': b'C'}, + 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)[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..739acf3ec53 100644 --- a/tools/buildman/toolchain.py +++ b/tools/buildman/toolchain.py @@ -90,7 +90,7 @@ class Toolchain: if self.arch == 'sandbox' and override_toolchain: self.gcc = override_toolchain
- env = self.MakeEnvironment(False) + env = self.MakeEnvironment()
# As a basic sanity check, run the C compiler with --version cmd = [fname, '--version'] @@ -172,7 +172,7 @@ class Toolchain: else: raise ValueError('Unknown arg to GetEnvArgs (%d)' % which)
- def MakeEnvironment(self, full_path): + def MakeEnvironment(self, env=None): """Returns an environment for using the toolchain.
This takes the current environment and adds CROSS_COMPILE so that @@ -188,25 +188,23 @@ class Toolchain: 569-570: surrogates not allowed
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. + current environment, with changes as needed to CROSS_COMPILE and + LC_ALL. """ - env = dict(os.environb) + env = dict(env or os.environb) + wrapper = self.GetWrapper()
if self.override_toolchain: # We'll use MakeArgs() to provide this pass - elif full_path: + else: env[b'CROSS_COMPILE'] = tools.to_bytes( 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']
env[b'LC_ALL'] = b'C'

On Sun, Jun 23, 2024 at 11:56:20AM -0600, Simon Glass wrote:
The feature to set the toolchain path does not seem to be needed. It causes problems with venv (see [1]). Let's remove it.
Add some tests while we are here.
It does not look like any docs changes are needed for this.
[1] https://patchwork.ozlabs.org/project/uboot/patch/20240621131423.2363294-6-sj...
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: Tom Rini trini@konsulko.com
Reviewed-by: Tom Rini trini@konsulko.com

On Sun, Jun 23, 2024 at 11:56:20AM -0600, Simon Glass wrote:
The feature to set the toolchain path does not seem to be needed. It causes problems with venv (see [1]). Let's remove it.
Add some tests while we are here.
It does not look like any docs changes are needed for this.
[1] https://patchwork.ozlabs.org/project/uboot/patch/20240621131423.2363294-6-sj...
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: Tom Rini trini@konsulko.com
Changes in v3:
- Drop the PATH modification altogether
tools/buildman/bsettings.py | 3 ++ tools/buildman/builder.py | 5 +-- tools/buildman/builderthread.py | 4 +- tools/buildman/cmdline.py | 2 - tools/buildman/control.py | 6 +-- tools/buildman/test.py | 75 +++++++++++++++++++++++++++++++++ tools/buildman/toolchain.py | 20 ++++----- 7 files changed, 92 insertions(+), 23 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/builder.py b/tools/buildman/builder.py index f35175b4598..7c563cddada 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -255,7 +255,7 @@ class Builder:
def __init__(self, toolchains, base_dir, git_dir, num_threads, num_jobs, gnu_make='make', checkout=True, show_unknown=True, step=1,
no_subdirs=False, full_path=False, verbose_build=False,
no_subdirs=False, verbose_build=False, mrproper=False, per_board_out_dir=False, config_only=False, squash_config_y=False, warnings_as_errors=False, work_in_output=False,
@@ -279,8 +279,6 @@ class Builder: step: 1 to process every commit, n to process every nth commit no_subdirs: Don't create subdirectories when building current source for a single board
full_path: Return the full path in CROSS_COMPILE and don't set
PATH verbose_build: Run build with V=1 and don't use 'make -s' mrproper: Always run 'make mrproper' when configuring per_board_out_dir: Build in a separate persistent directory per
@@ -336,7 +334,6 @@ class Builder: self._step = step self._error_lines = 0 self.no_subdirs = no_subdirs
self.full_path = full_path self.verbose_build = verbose_build self.config_only = config_only self.squash_config_y = squash_config_y
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index a8599c0bb2a..c23c3254d2d 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -404,7 +404,7 @@ class BuilderThread(threading.Thread): the next incremental build """ # Set up the environment and command line
env = self.toolchain.MakeEnvironment(self.builder.full_path)
env = self.toolchain.MakeEnvironment() mkdir(out_dir) args, cwd, src_dir = self._build_args(brd, out_dir, out_rel_dir,
@@ -569,7 +569,7 @@ class BuilderThread(threading.Thread): outf.write(f'{result.return_code}')
# Write out the image and function size information and an objdump
env = result.toolchain.MakeEnvironment(self.builder.full_path)
env = result.toolchain.MakeEnvironment() with open(os.path.join(build_dir, 'out-env'), 'wb') as outf: for var in sorted(env.keys()): outf.write(b'%s="%s"' % (var, env[var]))
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py index 03211bd5aa5..5fda90508f2 100644 --- a/tools/buildman/cmdline.py +++ b/tools/buildman/cmdline.py @@ -121,8 +121,6 @@ def add_after_m(parser): help="Override host toochain to use for sandbox (e.g. 'clang-7')") parser.add_argument('-Q', '--quick', action='store_true', default=False, help='Do a rough build, with limited warning resolution')
- parser.add_argument('-p', '--full-path', action='store_true',
parser.add_argument('-P', '--per-board-out-dir', action='store_true', default=False, help="Use an O= (output) directory per board rather than per thread") parser.add_argument('--print-arch', action='store_true',default=False, help="Use full toolchain path in CROSS_COMPILE")
diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 8f6850c5211..3ca9e2e8761 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -653,10 +653,8 @@ def do_buildman(args, toolchains=None, make_func=None, brds=None, builder = Builder(toolchains, output_dir, git_dir, args.threads, args.jobs, checkout=True, show_unknown=args.show_unknown, step=args.step,
no_subdirs=args.no_subdirs, full_path=args.full_path,
verbose_build=args.verbose_build,
mrproper=args.mrproper,
per_board_out_dir=args.per_board_out_dir,
no_subdirs=args.no_subdirs, verbose_build=args.verbose_build,
mrproper=args.mrproper, per_board_out_dir=args.per_board_out_dir, config_only=args.config_only, squash_config_y=not args.preserve_config_y, warnings_as_errors=args.warnings_as_errors,
diff --git a/tools/buildman/test.py b/tools/buildman/test.py index f92add7a7c5..ae9963eed4f 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)
Sorry Simon, but me and others love to be consistent.
Best regards, Andrejs.
self.toolchains.Add('gcc', test=False) # Avoid sending any output
@@ -747,6 +748,80 @@ class TestBuild(unittest.TestCase): self.assertEqual([ ['MARY="mary"', 'Missing expected line: CONFIG_MARY="mary"']], result)
- def call_make_environment(self, tchn, in_env=None):
"""Call Toolchain.MakeEnvironment() and process the result
Args:
tchn (Toolchain): Toolchain to use
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(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
tchn = self.toolchains.Select('aarch64')
# Normal case
diff = self.call_make_environment(tchn)[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)[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, env)
self.assertNotIn(b'PATH', diff)
self.assertEqual(None, diff_path)
self.assertEqual(
{b'CROSS_COMPILE': b'/my/path/aarch64-linux-', b'LC_ALL': b'C'},
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)[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..739acf3ec53 100644 --- a/tools/buildman/toolchain.py +++ b/tools/buildman/toolchain.py @@ -90,7 +90,7 @@ class Toolchain: if self.arch == 'sandbox' and override_toolchain: self.gcc = override_toolchain
env = self.MakeEnvironment(False)
env = self.MakeEnvironment() # As a basic sanity check, run the C compiler with --version cmd = [fname, '--version']
@@ -172,7 +172,7 @@ class Toolchain: else: raise ValueError('Unknown arg to GetEnvArgs (%d)' % which)
- def MakeEnvironment(self, full_path):
def MakeEnvironment(self, env=None): """Returns an environment for using the toolchain.
This takes the current environment and adds CROSS_COMPILE so that
@@ -188,25 +188,23 @@ class Toolchain: 569-570: surrogates not allowed
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.
current environment, with changes as needed to CROSS_COMPILE and
LC_ALL. """
env = dict(os.environb)
env = dict(env or os.environb)
wrapper = self.GetWrapper() if self.override_toolchain: # We'll use MakeArgs() to provide this pass
elif full_path:
else: env[b'CROSS_COMPILE'] = tools.to_bytes( 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'] env[b'LC_ALL'] = b'C'
-- 2.34.1

Hi Andrejs,
On Tue, 25 Jun 2024 at 01:06, Andrejs Cainikovs andrejs.cainikovs@toradex.com wrote:
On Sun, Jun 23, 2024 at 11:56:20AM -0600, Simon Glass wrote:
The feature to set the toolchain path does not seem to be needed. It causes problems with venv (see [1]). Let's remove it.
Add some tests while we are here.
It does not look like any docs changes are needed for this.
[1] https://patchwork.ozlabs.org/project/uboot/patch/20240621131423.2363294-6-sj...
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: Tom Rini trini@konsulko.com
Changes in v3:
- Drop the PATH modification altogether
tools/buildman/bsettings.py | 3 ++ tools/buildman/builder.py | 5 +-- tools/buildman/builderthread.py | 4 +- tools/buildman/cmdline.py | 2 - tools/buildman/control.py | 6 +-- tools/buildman/test.py | 75 +++++++++++++++++++++++++++++++++ tools/buildman/toolchain.py | 20 ++++----- 7 files changed, 92 insertions(+), 23 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/builder.py b/tools/buildman/builder.py index f35175b4598..7c563cddada 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -255,7 +255,7 @@ class Builder:
def __init__(self, toolchains, base_dir, git_dir, num_threads, num_jobs, gnu_make='make', checkout=True, show_unknown=True, step=1,
no_subdirs=False, full_path=False, verbose_build=False,
no_subdirs=False, verbose_build=False, mrproper=False, per_board_out_dir=False, config_only=False, squash_config_y=False, warnings_as_errors=False, work_in_output=False,
@@ -279,8 +279,6 @@ class Builder: step: 1 to process every commit, n to process every nth commit no_subdirs: Don't create subdirectories when building current source for a single board
full_path: Return the full path in CROSS_COMPILE and don't set
PATH verbose_build: Run build with V=1 and don't use 'make -s' mrproper: Always run 'make mrproper' when configuring per_board_out_dir: Build in a separate persistent directory per
@@ -336,7 +334,6 @@ class Builder: self._step = step self._error_lines = 0 self.no_subdirs = no_subdirs
self.full_path = full_path self.verbose_build = verbose_build self.config_only = config_only self.squash_config_y = squash_config_y
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index a8599c0bb2a..c23c3254d2d 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -404,7 +404,7 @@ class BuilderThread(threading.Thread): the next incremental build """ # Set up the environment and command line
env = self.toolchain.MakeEnvironment(self.builder.full_path)
env = self.toolchain.MakeEnvironment() mkdir(out_dir) args, cwd, src_dir = self._build_args(brd, out_dir, out_rel_dir,
@@ -569,7 +569,7 @@ class BuilderThread(threading.Thread): outf.write(f'{result.return_code}')
# Write out the image and function size information and an objdump
env = result.toolchain.MakeEnvironment(self.builder.full_path)
env = result.toolchain.MakeEnvironment() with open(os.path.join(build_dir, 'out-env'), 'wb') as outf: for var in sorted(env.keys()): outf.write(b'%s="%s"' % (var, env[var]))
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py index 03211bd5aa5..5fda90508f2 100644 --- a/tools/buildman/cmdline.py +++ b/tools/buildman/cmdline.py @@ -121,8 +121,6 @@ def add_after_m(parser): help="Override host toochain to use for sandbox (e.g. 'clang-7')") parser.add_argument('-Q', '--quick', action='store_true', default=False, help='Do a rough build, with limited warning resolution')
- parser.add_argument('-p', '--full-path', action='store_true',
parser.add_argument('-P', '--per-board-out-dir', action='store_true', default=False, help="Use an O= (output) directory per board rather than per thread") parser.add_argument('--print-arch', action='store_true',default=False, help="Use full toolchain path in CROSS_COMPILE")
diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 8f6850c5211..3ca9e2e8761 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -653,10 +653,8 @@ def do_buildman(args, toolchains=None, make_func=None, brds=None, builder = Builder(toolchains, output_dir, git_dir, args.threads, args.jobs, checkout=True, show_unknown=args.show_unknown, step=args.step,
no_subdirs=args.no_subdirs, full_path=args.full_path,
verbose_build=args.verbose_build,
mrproper=args.mrproper,
per_board_out_dir=args.per_board_out_dir,
no_subdirs=args.no_subdirs, verbose_build=args.verbose_build,
mrproper=args.mrproper, per_board_out_dir=args.per_board_out_dir, config_only=args.config_only, squash_config_y=not args.preserve_config_y, warnings_as_errors=args.warnings_as_errors,
diff --git a/tools/buildman/test.py b/tools/buildman/test.py index f92add7a7c5..ae9963eed4f 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)
Sorry Simon, but me and others love to be consistent.
That's fine, but do you have a comment on this patch?
Regards, Simon

On Tue, Jun 25, 2024 at 01:38:07PM +0100, Simon Glass wrote:
Hi Andrejs,
On Tue, 25 Jun 2024 at 01:06, Andrejs Cainikovs andrejs.cainikovs@toradex.com wrote:
On Sun, Jun 23, 2024 at 11:56:20AM -0600, Simon Glass wrote:
The feature to set the toolchain path does not seem to be needed. It causes problems with venv (see [1]). Let's remove it.
Add some tests while we are here.
It does not look like any docs changes are needed for this.
[1] https://patchwork.ozlabs.org/project/uboot/patch/20240621131423.2363294-6-sj...
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: Tom Rini trini@konsulko.com
Changes in v3:
- Drop the PATH modification altogether
tools/buildman/bsettings.py | 3 ++ tools/buildman/builder.py | 5 +-- tools/buildman/builderthread.py | 4 +- tools/buildman/cmdline.py | 2 - tools/buildman/control.py | 6 +-- tools/buildman/test.py | 75 +++++++++++++++++++++++++++++++++ tools/buildman/toolchain.py | 20 ++++----- 7 files changed, 92 insertions(+), 23 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/builder.py b/tools/buildman/builder.py index f35175b4598..7c563cddada 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -255,7 +255,7 @@ class Builder:
def __init__(self, toolchains, base_dir, git_dir, num_threads, num_jobs, gnu_make='make', checkout=True, show_unknown=True, step=1,
no_subdirs=False, full_path=False, verbose_build=False,
no_subdirs=False, verbose_build=False, mrproper=False, per_board_out_dir=False, config_only=False, squash_config_y=False, warnings_as_errors=False, work_in_output=False,
@@ -279,8 +279,6 @@ class Builder: step: 1 to process every commit, n to process every nth commit no_subdirs: Don't create subdirectories when building current source for a single board
full_path: Return the full path in CROSS_COMPILE and don't set
PATH verbose_build: Run build with V=1 and don't use 'make -s' mrproper: Always run 'make mrproper' when configuring per_board_out_dir: Build in a separate persistent directory per
@@ -336,7 +334,6 @@ class Builder: self._step = step self._error_lines = 0 self.no_subdirs = no_subdirs
self.full_path = full_path self.verbose_build = verbose_build self.config_only = config_only self.squash_config_y = squash_config_y
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index a8599c0bb2a..c23c3254d2d 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -404,7 +404,7 @@ class BuilderThread(threading.Thread): the next incremental build """ # Set up the environment and command line
env = self.toolchain.MakeEnvironment(self.builder.full_path)
env = self.toolchain.MakeEnvironment() mkdir(out_dir) args, cwd, src_dir = self._build_args(brd, out_dir, out_rel_dir,
@@ -569,7 +569,7 @@ class BuilderThread(threading.Thread): outf.write(f'{result.return_code}')
# Write out the image and function size information and an objdump
env = result.toolchain.MakeEnvironment(self.builder.full_path)
env = result.toolchain.MakeEnvironment() with open(os.path.join(build_dir, 'out-env'), 'wb') as outf: for var in sorted(env.keys()): outf.write(b'%s="%s"' % (var, env[var]))
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py index 03211bd5aa5..5fda90508f2 100644 --- a/tools/buildman/cmdline.py +++ b/tools/buildman/cmdline.py @@ -121,8 +121,6 @@ def add_after_m(parser): help="Override host toochain to use for sandbox (e.g. 'clang-7')") parser.add_argument('-Q', '--quick', action='store_true', default=False, help='Do a rough build, with limited warning resolution')
- parser.add_argument('-p', '--full-path', action='store_true',
parser.add_argument('-P', '--per-board-out-dir', action='store_true', default=False, help="Use an O= (output) directory per board rather than per thread") parser.add_argument('--print-arch', action='store_true',default=False, help="Use full toolchain path in CROSS_COMPILE")
diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 8f6850c5211..3ca9e2e8761 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -653,10 +653,8 @@ def do_buildman(args, toolchains=None, make_func=None, brds=None, builder = Builder(toolchains, output_dir, git_dir, args.threads, args.jobs, checkout=True, show_unknown=args.show_unknown, step=args.step,
no_subdirs=args.no_subdirs, full_path=args.full_path,
verbose_build=args.verbose_build,
mrproper=args.mrproper,
per_board_out_dir=args.per_board_out_dir,
no_subdirs=args.no_subdirs, verbose_build=args.verbose_build,
mrproper=args.mrproper, per_board_out_dir=args.per_board_out_dir, config_only=args.config_only, squash_config_y=not args.preserve_config_y, warnings_as_errors=args.warnings_as_errors,
diff --git a/tools/buildman/test.py b/tools/buildman/test.py index f92add7a7c5..ae9963eed4f 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)
Sorry Simon, but me and others love to be consistent.
That's fine, but do you have a comment on this patch?
Regards, Simon
I was just thinking about aarch64-linux-gcc (should it be aarch64-linux-gnu-gcc, btw?) without path for consistency. But this of course is very minor - feel free to disregard my comment.
Best regards, Andrejs.

Hi Andrejs,
On Tue, 25 Jun 2024 at 16:16, Andrejs Cainikovs andrejs.cainikovs@toradex.com wrote:
On Tue, Jun 25, 2024 at 01:38:07PM +0100, Simon Glass wrote:
Hi Andrejs,
On Tue, 25 Jun 2024 at 01:06, Andrejs Cainikovs andrejs.cainikovs@toradex.com wrote:
On Sun, Jun 23, 2024 at 11:56:20AM -0600, Simon Glass wrote:
The feature to set the toolchain path does not seem to be needed. It causes problems with venv (see [1]). Let's remove it.
Add some tests while we are here.
It does not look like any docs changes are needed for this.
[1] https://patchwork.ozlabs.org/project/uboot/patch/20240621131423.2363294-6-sj...
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: Tom Rini trini@konsulko.com
Changes in v3:
- Drop the PATH modification altogether
tools/buildman/bsettings.py | 3 ++ tools/buildman/builder.py | 5 +-- tools/buildman/builderthread.py | 4 +- tools/buildman/cmdline.py | 2 - tools/buildman/control.py | 6 +-- tools/buildman/test.py | 75 +++++++++++++++++++++++++++++++++ tools/buildman/toolchain.py | 20 ++++----- 7 files changed, 92 insertions(+), 23 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/builder.py b/tools/buildman/builder.py index f35175b4598..7c563cddada 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -255,7 +255,7 @@ class Builder:
def __init__(self, toolchains, base_dir, git_dir, num_threads, num_jobs, gnu_make='make', checkout=True, show_unknown=True, step=1,
no_subdirs=False, full_path=False, verbose_build=False,
no_subdirs=False, verbose_build=False, mrproper=False, per_board_out_dir=False, config_only=False, squash_config_y=False, warnings_as_errors=False, work_in_output=False,
@@ -279,8 +279,6 @@ class Builder: step: 1 to process every commit, n to process every nth commit no_subdirs: Don't create subdirectories when building current source for a single board
full_path: Return the full path in CROSS_COMPILE and don't set
PATH verbose_build: Run build with V=1 and don't use 'make -s' mrproper: Always run 'make mrproper' when configuring per_board_out_dir: Build in a separate persistent directory per
@@ -336,7 +334,6 @@ class Builder: self._step = step self._error_lines = 0 self.no_subdirs = no_subdirs
self.full_path = full_path self.verbose_build = verbose_build self.config_only = config_only self.squash_config_y = squash_config_y
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index a8599c0bb2a..c23c3254d2d 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -404,7 +404,7 @@ class BuilderThread(threading.Thread): the next incremental build """ # Set up the environment and command line
env = self.toolchain.MakeEnvironment(self.builder.full_path)
env = self.toolchain.MakeEnvironment() mkdir(out_dir) args, cwd, src_dir = self._build_args(brd, out_dir, out_rel_dir,
@@ -569,7 +569,7 @@ class BuilderThread(threading.Thread): outf.write(f'{result.return_code}')
# Write out the image and function size information and an objdump
env = result.toolchain.MakeEnvironment(self.builder.full_path)
env = result.toolchain.MakeEnvironment() with open(os.path.join(build_dir, 'out-env'), 'wb') as outf: for var in sorted(env.keys()): outf.write(b'%s="%s"' % (var, env[var]))
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py index 03211bd5aa5..5fda90508f2 100644 --- a/tools/buildman/cmdline.py +++ b/tools/buildman/cmdline.py @@ -121,8 +121,6 @@ def add_after_m(parser): help="Override host toochain to use for sandbox (e.g. 'clang-7')") parser.add_argument('-Q', '--quick', action='store_true', default=False, help='Do a rough build, with limited warning resolution')
- parser.add_argument('-p', '--full-path', action='store_true',
parser.add_argument('-P', '--per-board-out-dir', action='store_true', default=False, help="Use an O= (output) directory per board rather than per thread") parser.add_argument('--print-arch', action='store_true',default=False, help="Use full toolchain path in CROSS_COMPILE")
diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 8f6850c5211..3ca9e2e8761 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -653,10 +653,8 @@ def do_buildman(args, toolchains=None, make_func=None, brds=None, builder = Builder(toolchains, output_dir, git_dir, args.threads, args.jobs, checkout=True, show_unknown=args.show_unknown, step=args.step,
no_subdirs=args.no_subdirs, full_path=args.full_path,
verbose_build=args.verbose_build,
mrproper=args.mrproper,
per_board_out_dir=args.per_board_out_dir,
no_subdirs=args.no_subdirs, verbose_build=args.verbose_build,
mrproper=args.mrproper, per_board_out_dir=args.per_board_out_dir, config_only=args.config_only, squash_config_y=not args.preserve_config_y, warnings_as_errors=args.warnings_as_errors,
diff --git a/tools/buildman/test.py b/tools/buildman/test.py index f92add7a7c5..ae9963eed4f 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)
Sorry Simon, but me and others love to be consistent.
That's fine, but do you have a comment on this patch?
Regards, Simon
I was just thinking about aarch64-linux-gcc (should it be aarch64-linux-gnu-gcc, btw?) without path for consistency. But this of course is very minor - feel free to disregard my comment.
Oh I see...the point here is to test a toolchain which has a path prepended to it, That is why this test case is not consistent with the others.
Regards, Simon

On Wed, Jun 26, 2024 at 09:00:43AM +0100, Simon Glass wrote:
Hi Andrejs,
On Tue, 25 Jun 2024 at 16:16, Andrejs Cainikovs andrejs.cainikovs@toradex.com wrote:
On Tue, Jun 25, 2024 at 01:38:07PM +0100, Simon Glass wrote:
Hi Andrejs,
On Tue, 25 Jun 2024 at 01:06, Andrejs Cainikovs andrejs.cainikovs@toradex.com wrote:
On Sun, Jun 23, 2024 at 11:56:20AM -0600, Simon Glass wrote:
The feature to set the toolchain path does not seem to be needed. It causes problems with venv (see [1]). Let's remove it.
Add some tests while we are here.
It does not look like any docs changes are needed for this.
[1] https://patchwork.ozlabs.org/project/uboot/patch/20240621131423.2363294-6-sj...
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: Tom Rini trini@konsulko.com
Changes in v3:
- Drop the PATH modification altogether
tools/buildman/bsettings.py | 3 ++ tools/buildman/builder.py | 5 +-- tools/buildman/builderthread.py | 4 +- tools/buildman/cmdline.py | 2 - tools/buildman/control.py | 6 +-- tools/buildman/test.py | 75 +++++++++++++++++++++++++++++++++ tools/buildman/toolchain.py | 20 ++++----- 7 files changed, 92 insertions(+), 23 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/builder.py b/tools/buildman/builder.py index f35175b4598..7c563cddada 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -255,7 +255,7 @@ class Builder:
def __init__(self, toolchains, base_dir, git_dir, num_threads, num_jobs, gnu_make='make', checkout=True, show_unknown=True, step=1,
no_subdirs=False, full_path=False, verbose_build=False,
no_subdirs=False, verbose_build=False, mrproper=False, per_board_out_dir=False, config_only=False, squash_config_y=False, warnings_as_errors=False, work_in_output=False,
@@ -279,8 +279,6 @@ class Builder: step: 1 to process every commit, n to process every nth commit no_subdirs: Don't create subdirectories when building current source for a single board
full_path: Return the full path in CROSS_COMPILE and don't set
PATH verbose_build: Run build with V=1 and don't use 'make -s' mrproper: Always run 'make mrproper' when configuring per_board_out_dir: Build in a separate persistent directory per
@@ -336,7 +334,6 @@ class Builder: self._step = step self._error_lines = 0 self.no_subdirs = no_subdirs
self.full_path = full_path self.verbose_build = verbose_build self.config_only = config_only self.squash_config_y = squash_config_y
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index a8599c0bb2a..c23c3254d2d 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -404,7 +404,7 @@ class BuilderThread(threading.Thread): the next incremental build """ # Set up the environment and command line
env = self.toolchain.MakeEnvironment(self.builder.full_path)
env = self.toolchain.MakeEnvironment() mkdir(out_dir) args, cwd, src_dir = self._build_args(brd, out_dir, out_rel_dir,
@@ -569,7 +569,7 @@ class BuilderThread(threading.Thread): outf.write(f'{result.return_code}')
# Write out the image and function size information and an objdump
env = result.toolchain.MakeEnvironment(self.builder.full_path)
env = result.toolchain.MakeEnvironment() with open(os.path.join(build_dir, 'out-env'), 'wb') as outf: for var in sorted(env.keys()): outf.write(b'%s="%s"' % (var, env[var]))
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py index 03211bd5aa5..5fda90508f2 100644 --- a/tools/buildman/cmdline.py +++ b/tools/buildman/cmdline.py @@ -121,8 +121,6 @@ def add_after_m(parser): help="Override host toochain to use for sandbox (e.g. 'clang-7')") parser.add_argument('-Q', '--quick', action='store_true', default=False, help='Do a rough build, with limited warning resolution')
- parser.add_argument('-p', '--full-path', action='store_true',
parser.add_argument('-P', '--per-board-out-dir', action='store_true', default=False, help="Use an O= (output) directory per board rather than per thread") parser.add_argument('--print-arch', action='store_true',default=False, help="Use full toolchain path in CROSS_COMPILE")
diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 8f6850c5211..3ca9e2e8761 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -653,10 +653,8 @@ def do_buildman(args, toolchains=None, make_func=None, brds=None, builder = Builder(toolchains, output_dir, git_dir, args.threads, args.jobs, checkout=True, show_unknown=args.show_unknown, step=args.step,
no_subdirs=args.no_subdirs, full_path=args.full_path,
verbose_build=args.verbose_build,
mrproper=args.mrproper,
per_board_out_dir=args.per_board_out_dir,
no_subdirs=args.no_subdirs, verbose_build=args.verbose_build,
mrproper=args.mrproper, per_board_out_dir=args.per_board_out_dir, config_only=args.config_only, squash_config_y=not args.preserve_config_y, warnings_as_errors=args.warnings_as_errors,
diff --git a/tools/buildman/test.py b/tools/buildman/test.py index f92add7a7c5..ae9963eed4f 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)
Sorry Simon, but me and others love to be consistent.
That's fine, but do you have a comment on this patch?
Regards, Simon
I was just thinking about aarch64-linux-gcc (should it be aarch64-linux-gnu-gcc, btw?) without path for consistency. But this of course is very minor - feel free to disregard my comment.
Oh I see...the point here is to test a toolchain which has a path prepended to it, That is why this test case is not consistent with the others.
Regards, Simon
In this case:
Reviewed-by: Andrejs Cainikovs andrejs.cainikovs@toradex.com
Regards, Andrejs.

On Wed, Jun 26, 2024 at 09:00:43AM +0100, Simon Glass wrote:
Hi Andrejs,
On Tue, 25 Jun 2024 at 16:16, Andrejs Cainikovs andrejs.cainikovs@toradex.com wrote:
On Tue, Jun 25, 2024 at 01:38:07PM +0100, Simon Glass wrote:
Hi Andrejs,
On Tue, 25 Jun 2024 at 01:06, Andrejs Cainikovs andrejs.cainikovs@toradex.com wrote:
On Sun, Jun 23, 2024 at 11:56:20AM -0600, Simon Glass wrote:
The feature to set the toolchain path does not seem to be needed. It causes problems with venv (see [1]). Let's remove it.
Add some tests while we are here.
It does not look like any docs changes are needed for this.
[1] https://patchwork.ozlabs.org/project/uboot/patch/20240621131423.2363294-6-sj...
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: Tom Rini trini@konsulko.com
Changes in v3:
- Drop the PATH modification altogether
tools/buildman/bsettings.py | 3 ++ tools/buildman/builder.py | 5 +-- tools/buildman/builderthread.py | 4 +- tools/buildman/cmdline.py | 2 - tools/buildman/control.py | 6 +-- tools/buildman/test.py | 75 +++++++++++++++++++++++++++++++++ tools/buildman/toolchain.py | 20 ++++----- 7 files changed, 92 insertions(+), 23 deletions(-)
Applied to u-boot-dm, thanks!

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'

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(-)
Applied to u-boot-dm, thanks!

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 ---
(no changes since v2)
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;

On Sun, Jun 23, 2024 at 11:56:22AM -0600, Simon Glass wrote:
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
Reviewed-by: Tom Rini trini@konsulko.com
participants (3)
-
Andrejs Cainikovs
-
Simon Glass
-
Tom Rini