diff mbox series

[v2,02/16] buildman/toolchain.py: set CROSS_COMPILE only if self.cross is set

Message ID 655de02f937e3ea3a8f5306ef510d817bee8065b.1725012294.git.jerome.forissier@linaro.org
State New
Headers show
Series Miscellaneous fixes | expand

Commit Message

Jerome Forissier Aug. 30, 2024, 10:16 a.m. UTC
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
 +In file included from boot/bootmeth_efi.c:16:
 +include/efi_default_filename.h:20:15: error: operator '==' has no left operand
 +   20 | #if HOST_ARCH == HOST_ARCH_X86_64
 +      |               ^~
 [...]

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 tools/buildman/toolchain.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Peter Robinson Aug. 30, 2024, 11:58 a.m. UTC | #1
On Fri, 30 Aug 2024 at 11:17, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
> 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
>  +In file included from boot/bootmeth_efi.c:16:
>  +include/efi_default_filename.h:20:15: error: operator '==' has no left operand
>  +   20 | #if HOST_ARCH == HOST_ARCH_X86_64
>  +      |               ^~
>  [...]
>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reviewed-by: Peter Robinson <pbrobinson@gmail.com>
> ---
>  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 324ad0e082..ff987a9eea 100644
> --- a/tools/buildman/toolchain.py
> +++ b/tools/buildman/toolchain.py
> @@ -201,10 +201,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)
>              env[b'PATH'] = tools.to_bytes(self.path) + b':' + env[b'PATH']
>
> --
> 2.40.1
>
Simon Glass Sept. 1, 2024, 8:10 p.m. UTC | #2
Hi Jerome,

On Fri, 30 Aug 2024 at 04:17, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
> 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
>  +In file included from boot/bootmeth_efi.c:16:
>  +include/efi_default_filename.h:20:15: error: operator '==' has no left operand
>  +   20 | #if HOST_ARCH == HOST_ARCH_X86_64
>  +      |               ^~
>  [...]
>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---
>  tools/buildman/toolchain.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Can you please put this on top of [1]? I am trying to resolve a
code-coverage problem in the same function.

Also, please do add a test case.

>
> diff --git a/tools/buildman/toolchain.py b/tools/buildman/toolchain.py
> index 324ad0e082..ff987a9eea 100644
> --- a/tools/buildman/toolchain.py
> +++ b/tools/buildman/toolchain.py
> @@ -201,10 +201,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)
>              env[b'PATH'] = tools.to_bytes(self.path) + b':' + env[b'PATH']
>
> --
> 2.40.1
>

Regards,
Simon
Jerome Forissier Sept. 2, 2024, 3:12 p.m. UTC | #3
Hi Simon,

On 9/1/24 22:10, Simon Glass wrote:
> Hi Jerome,
> 
> On Fri, 30 Aug 2024 at 04:17, Jerome Forissier
> <jerome.forissier@linaro.org> wrote:
>>
>> 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
>>  +In file included from boot/bootmeth_efi.c:16:
>>  +include/efi_default_filename.h:20:15: error: operator '==' has no left operand
>>  +   20 | #if HOST_ARCH == HOST_ARCH_X86_64
>>  +      |               ^~
>>  [...]
>>
>> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
>> ---
>>  tools/buildman/toolchain.py | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Can you please put this on top of [1]? I am trying to resolve a
> code-coverage problem in the same function.

Which [1] and what do you mean by "on top off"?
- pick patch [1], then pick this patch (resulting in two patches
in the series)
- or pick patch [1] and combine it with this one?

> 
> Also, please do add a test case.

Done in v3.

Thanks,
diff mbox series

Patch

diff --git a/tools/buildman/toolchain.py b/tools/buildman/toolchain.py
index 324ad0e082..ff987a9eea 100644
--- a/tools/buildman/toolchain.py
+++ b/tools/buildman/toolchain.py
@@ -201,10 +201,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)
             env[b'PATH'] = tools.to_bytes(self.path) + b':' + env[b'PATH']