| Message ID | 20240623175622.1468600-5-sjg@chromium.org |
|---|---|
| State | Accepted |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | Add Binman code-coverage test to CI | expand |
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-sjg@chromium.org/ > > 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-sjg@chromium.org/ > > 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) 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-sjg@chromium.org/ > > > > 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) > > 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-sjg@chromium.org/ > > > > > > 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) > > > > 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-sjg@chromium.org/ > > > > > > > > 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) > > > > > > 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-sjg@chromium.org/ > > > > > > > > > > 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) > > > > > > > > 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-sjg@chromium.org/ > > > > > > > > > > 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!
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'
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 <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(-)