diff mbox series

[v4,01/15] buildman/toolchain.py: do not set CROSS_COMPILE for sandbox

Message ID 721993374cd63a2cddced7aff045fd0f23d9ecca.1726048404.git.jerome.forissier@linaro.org
State Accepted
Commit 0c2d7ad3e0635a52d291035d9044922ef7b19e7a
Headers show
Series Miscellaneous fixes | expand

Commit Message

Jerome Forissier Sept. 11, 2024, 9:58 a.m. UTC
When building for sandbox, self.cross is empty.

In MakeEnvironment(), CROSS_COMPILE is defined to be self.cross (with
or without a full path), optionally prefixed by the toolchain wrapper
defined in ~/.buildman. This is fine when self.cross is not empty, but
it doesn't make sense when it is:
- Either there is no wrapper and we end up with an empty CROSS_COMPILE
which is the same as not defining it (the host compiler will be used),
- Or there is a wrapper and CROSS_COMPILE will contain only the wrapper
which obviously is not a valid compiler, hence an error.

Test case:

 $ sudo apt install ccache
 $ grep -q toolchain-wrapper ~/.buildman || \
     printf "[toolchain-wrapper]\nwrapper = ccache\n" >>~/.buildman
 $ make mrproper
 $ ./tools/buildman/buildman sandbox_noinst
 $ ./tools/buildman/buildman sandbox_noinst
 Building current source for 1 boards (1 thread, 24 jobs per thread)
    sandbox:  +   sandbox_noinst
 +arch/sandbox/lib/reloc_sandbox_efi.c:10:15: error: operator '==' has no left operand
 +   10 | #if HOST_ARCH == HOST_ARCH_X86_64
 +      |               ^~
[...]

The GetEnvArgs function is modified too, since the VAR_CROSS_COMPILE
case has the same issue.

In tools/buildman/test.py, testGetEnvArgs is extended and
testMakeEnvironment is added. They check the 'arm' and 'sandbox'
toolchains, with and without a wrapper.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 tools/buildman/test.py      | 46 +++++++++++++++++++++++++++++++++++++
 tools/buildman/toolchain.py |  6 +++--
 2 files changed, 50 insertions(+), 2 deletions(-)

Comments

Simon Glass Sept. 12, 2024, 1:01 a.m. UTC | #1
Hi Jerome,

On Wed, 11 Sept 2024 at 03:58, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
> When building for sandbox, self.cross is empty.
>
> In MakeEnvironment(), CROSS_COMPILE is defined to be self.cross (with
> or without a full path), optionally prefixed by the toolchain wrapper
> defined in ~/.buildman. This is fine when self.cross is not empty, but
> it doesn't make sense when it is:
> - Either there is no wrapper and we end up with an empty CROSS_COMPILE
> which is the same as not defining it (the host compiler will be used),
> - Or there is a wrapper and CROSS_COMPILE will contain only the wrapper
> which obviously is not a valid compiler, hence an error.
>
> Test case:
>
>  $ sudo apt install ccache
>  $ grep -q toolchain-wrapper ~/.buildman || \
>      printf "[toolchain-wrapper]\nwrapper = ccache\n" >>~/.buildman
>  $ make mrproper
>  $ ./tools/buildman/buildman sandbox_noinst
>  $ ./tools/buildman/buildman sandbox_noinst
>  Building current source for 1 boards (1 thread, 24 jobs per thread)
>     sandbox:  +   sandbox_noinst
>  +arch/sandbox/lib/reloc_sandbox_efi.c:10:15: error: operator '==' has no left operand
>  +   10 | #if HOST_ARCH == HOST_ARCH_X86_64
>  +      |               ^~
> [...]
>
> The GetEnvArgs function is modified too, since the VAR_CROSS_COMPILE
> case has the same issue.
>
> In tools/buildman/test.py, testGetEnvArgs is extended and
> testMakeEnvironment is added. They check the 'arm' and 'sandbox'
> toolchains, with and without a wrapper.
>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---
>  tools/buildman/test.py      | 46 +++++++++++++++++++++++++++++++++++++
>  tools/buildman/toolchain.py |  6 +++--
>  2 files changed, 50 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

I did not expect to still get that same error now, but OK.
diff mbox series

Patch

diff --git a/tools/buildman/test.py b/tools/buildman/test.py
index 5eed013d51c..46aa2a17916 100644
--- a/tools/buildman/test.py
+++ b/tools/buildman/test.py
@@ -36,6 +36,16 @@  main: /usr/sbin
 x86: i386 x86_64
 '''
 
+settings_data_wrapper = '''
+# Buildman settings file
+
+[toolchain]
+main: /usr/sbin
+
+[toolchain-wrapper]
+wrapper = ccache
+'''
+
 migration = '''===================== WARNING ======================
 This board does not use CONFIG_DM. CONFIG_DM will be
 compulsory starting with the v2020.01 release.
@@ -606,6 +616,9 @@  class TestBuild(unittest.TestCase):
                          tc.GetEnvArgs(toolchain.VAR_ARCH))
         self.assertEqual('', tc.GetEnvArgs(toolchain.VAR_MAKE_ARGS))
 
+        tc = self.toolchains.Select('sandbox')
+        self.assertEqual('', tc.GetEnvArgs(toolchain.VAR_CROSS_COMPILE))
+
         self.toolchains.Add('/path/to/x86_64-linux-gcc', test=False)
         tc = self.toolchains.Select('x86')
         self.assertEqual('/path/to',
@@ -614,6 +627,39 @@  class TestBuild(unittest.TestCase):
         self.assertEqual('HOSTCC=clang CC=clang',
                          tc.GetEnvArgs(toolchain.VAR_MAKE_ARGS))
 
+        # Test config with ccache wrapper
+        bsettings.setup(None)
+        bsettings.add_file(settings_data_wrapper)
+
+        tc = self.toolchains.Select('arm')
+        self.assertEqual('ccache arm-linux-',
+                         tc.GetEnvArgs(toolchain.VAR_CROSS_COMPILE))
+
+        tc = self.toolchains.Select('sandbox')
+        self.assertEqual('', tc.GetEnvArgs(toolchain.VAR_CROSS_COMPILE))
+
+    def testMakeEnvironment(self):
+        """Test the MakeEnvironment function"""
+        tc = self.toolchains.Select('arm')
+        env = tc.MakeEnvironment(False)
+        self.assertEqual(env[b'CROSS_COMPILE'], b'arm-linux-')
+
+        tc = self.toolchains.Select('sandbox')
+        env = tc.MakeEnvironment(False)
+        self.assertTrue(b'CROSS_COMPILE' not in env)
+
+        # Test config with ccache wrapper
+        bsettings.setup(None)
+        bsettings.add_file(settings_data_wrapper)
+
+        tc = self.toolchains.Select('arm')
+        env = tc.MakeEnvironment(False)
+        self.assertEqual(env[b'CROSS_COMPILE'], b'ccache arm-linux-')
+
+        tc = self.toolchains.Select('sandbox')
+        env = tc.MakeEnvironment(False)
+        self.assertTrue(b'CROSS_COMPILE' not in env)
+
     def testPrepareOutputSpace(self):
         def _Touch(fname):
             tools.write_file(os.path.join(base_dir, fname), b'')
diff --git a/tools/buildman/toolchain.py b/tools/buildman/toolchain.py
index 6ca79c2c0f9..a7d7883b851 100644
--- a/tools/buildman/toolchain.py
+++ b/tools/buildman/toolchain.py
@@ -159,6 +159,8 @@  class Toolchain:
         if which == VAR_CROSS_COMPILE:
             wrapper = self.GetWrapper()
             base = '' if self.arch == 'sandbox' else self.path
+            if (base == '' and self.cross == ''):
+                return ''
             return wrapper + os.path.join(base, self.cross)
         elif which == VAR_PATH:
             return self.path
@@ -208,10 +210,10 @@  class Toolchain:
         if self.override_toolchain:
             # We'll use MakeArgs() to provide this
             pass
-        elif full_path:
+        elif full_path and self.cross:
             env[b'CROSS_COMPILE'] = tools.to_bytes(
                 wrapper + os.path.join(self.path, self.cross))
-        else:
+        elif self.cross:
             env[b'CROSS_COMPILE'] = tools.to_bytes(wrapper + self.cross)
 
             # Detect a Python virtualenv and avoid defeating it