From: Simon Glass Date: Sun, 23 Jun 2024 17:56:20 +0000 (-0600) Subject: buildman: Always use the full path in CROSS_COMPILE X-Git-Url: http://git.dujemihanovic.xyz/?a=commitdiff_plain;h=6c0a3cf75f72370deec3ee516a9dd377397af207;p=u-boot.git buildman: Always use the full path in CROSS_COMPILE 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-sjg@chromium.org/ Signed-off-by: Simon Glass Suggested-by: Tom Rini Reviewed-by: Tom Rini Reviewed-by: Andrejs Cainikovs --- diff --git a/tools/buildman/bsettings.py b/tools/buildman/bsettings.py index aea724fc55..a7358cfc08 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/builder.py b/tools/buildman/builder.py index c4384f53e8..c794177f2b 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, fallback_mrproper=False, per_board_out_dir=False, config_only=False, squash_config_y=False, warnings_as_errors=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 fallback_mrproper: Run 'make mrproper' and retry on build failure @@ -337,7 +335,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 bbe2f6f0d2..4e085d6d67 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -406,7 +406,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, @@ -574,7 +574,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 544a391a46..2673e749d5 100644 --- a/tools/buildman/cmdline.py +++ b/tools/buildman/cmdline.py @@ -123,8 +123,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 464835c5be..037854dfc0 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -788,10 +788,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, - fallback_mrproper=args.fallback_mrproper, + no_subdirs=args.no_subdirs, verbose_build=args.verbose_build, + mrproper=args.mrproper, fallback_mrproper=args.fallback_mrproper, per_board_out_dir=args.per_board_out_dir, config_only=args.config_only, squash_config_y=not args.preserve_config_y, diff --git a/tools/buildman/test.py b/tools/buildman/test.py index bfad309303..ac0a7ab06d 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,80 @@ 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, 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 324ad0e082..739acf3ec5 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'