| Message ID | 655de02f937e3ea3a8f5306ef510d817bee8065b.1725012294.git.jerome.forissier@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Miscellaneous fixes | expand |
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 >
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
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 --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']
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(-)