[PATCH RFC 0/1] kbuild: enable overriding the compiler using the environment

[PATCH RFC 0/1] kbuild: enable overriding the compiler using the environment

[Guillaume Tucker]

When building with clang and there is no gcc available, running
merge_config.sh fails without this fix because it can't build
scripts/basic/fixdep with HOSTCC hard-coded to be gcc in the top-level
Makefile.  This was discovered while trying to build big-endian arm64
kernels with clang for kernelci.org in a Docker container with only clang
as a host compiler.

While this fix seems like a very obvious thing to do, it's equally
surprising that it hasn't been done before.  This is why I'm sending this
as an RFC; there may be a very good reason why the compiler variables
still need to be hard-coded with gcc in the top-level Makefile.

Guillaume Tucker (1):
  kbuild: enable overriding the compiler using the environment

 Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

--
2.20.1

[PATCH RFC 1/1] kbuild: enable overriding the compiler using the environment

[Guillaume Tucker]

Only use gcc/g++ for HOSTCC, HOSTCXX and CC by default if they are not
already defined in the environment.  This fixes cases such as building
host tools with clang without having gcc installed.

The issue was initially hit when running merge_config.sh with clang
only as it failed to build "HOSTCC scripts/basic/fixdep".

Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
---
 Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 23cdf1f41364..c8608126750d 100644
--- a/Makefile
+++ b/Makefile
@@ -400,8 +400,8 @@ HOST_LFS_CFLAGS := $(shell getconf LFS_CFLAGS 2>/dev/null)
 HOST_LFS_LDFLAGS := $(shell getconf LFS_LDFLAGS 2>/dev/null)
 HOST_LFS_LIBS := $(shell getconf LFS_LIBS 2>/dev/null)
 
-HOSTCC       = gcc
-HOSTCXX      = g++
+HOSTCC      ?= gcc
+HOSTCXX     ?= g++
 KBUILD_HOSTCFLAGS   := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \
 		-fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS) \
 		$(HOSTCFLAGS)
@@ -412,7 +412,7 @@ KBUILD_HOSTLDLIBS   := $(HOST_LFS_LIBS) $(HOSTLDLIBS)
 # Make variables (CC, etc...)
 AS		= $(CROSS_COMPILE)as
 LD		= $(CROSS_COMPILE)ld
-CC		= $(CROSS_COMPILE)gcc
+CC	       ?= $(CROSS_COMPILE)gcc
 CPP		= $(CC) -E
 AR		= $(CROSS_COMPILE)ar
 NM		= $(CROSS_COMPILE)nm
-- 
2.20.1

Re: [PATCH RFC 0/1] kbuild: enable overriding the compiler using the environment

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 492 bytes --]

On Thu, Aug 08, 2019 at 11:06:51PM +0200, Guillaume Tucker wrote:

> While this fix seems like a very obvious thing to do, it's equally
> surprising that it hasn't been done before.  This is why I'm sending this
> as an RFC; there may be a very good reason why the compiler variables
> still need to be hard-coded with gcc in the top-level Makefile.

Probably the few people building kernels who didn't have GCC installed
and ran into this just went and installed it when they saw the error.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH RFC 1/1] kbuild: enable overriding the compiler using the environment

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 421 bytes --]

On Thu, Aug 08, 2019 at 11:06:52PM +0200, Guillaume Tucker wrote:

> @@ -412,7 +412,7 @@ KBUILD_HOSTLDLIBS   := $(HOST_LFS_LIBS) $(HOSTLDLIBS)
>  # Make variables (CC, etc...)
>  AS		= $(CROSS_COMPILE)as
>  LD		= $(CROSS_COMPILE)ld
> -CC		= $(CROSS_COMPILE)gcc
> +CC	       ?= $(CROSS_COMPILE)gcc
>  CPP		= $(CC) -E
>  AR		= $(CROSS_COMPILE)ar
>  NM		= $(CROSS_COMPILE)nm

Why only for CC and not for anything else here?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH RFC 1/1] kbuild: enable overriding the compiler using the environment

[Nick Desaulniers]

On Thu, Aug 8, 2019 at 2:07 PM Guillaume Tucker
<guillaume.tucker@collabora.com> wrote:
>
> Only use gcc/g++ for HOSTCC, HOSTCXX and CC by default if they are not
> already defined in the environment.  This fixes cases such as building
> host tools with clang without having gcc installed.
>
> The issue was initially hit when running merge_config.sh with clang
> only as it failed to build "HOSTCC scripts/basic/fixdep".

Thanks for the patch.  I don't quite follow the exact error.

When building with Clang, I usually do:

$ make CC=clang HOSTCC=clang ...

are you trying to fix the case where you do:

$ make CC=clang ...
<no HOSTCC set>
when GCC is not installed?  Because if so, I think it would be easier
to just specify HOSTCC=clang, but maybe I'm misunderstanding the
issue?

>
> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
> ---
>  Makefile | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 23cdf1f41364..c8608126750d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -400,8 +400,8 @@ HOST_LFS_CFLAGS := $(shell getconf LFS_CFLAGS 2>/dev/null)
>  HOST_LFS_LDFLAGS := $(shell getconf LFS_LDFLAGS 2>/dev/null)
>  HOST_LFS_LIBS := $(shell getconf LFS_LIBS 2>/dev/null)
>
> -HOSTCC       = gcc
> -HOSTCXX      = g++
> +HOSTCC      ?= gcc
> +HOSTCXX     ?= g++
>  KBUILD_HOSTCFLAGS   := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \
>                 -fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS) \
>                 $(HOSTCFLAGS)
> @@ -412,7 +412,7 @@ KBUILD_HOSTLDLIBS   := $(HOST_LFS_LIBS) $(HOSTLDLIBS)
>  # Make variables (CC, etc...)
>  AS             = $(CROSS_COMPILE)as
>  LD             = $(CROSS_COMPILE)ld
> -CC             = $(CROSS_COMPILE)gcc
> +CC            ?= $(CROSS_COMPILE)gcc
>  CPP            = $(CC) -E
>  AR             = $(CROSS_COMPILE)ar
>  NM             = $(CROSS_COMPILE)nm
> --
> 2.20.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH RFC 1/1] kbuild: enable overriding the compiler using the environment

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 649 bytes --]

On Thu, Aug 08, 2019 at 03:42:32PM -0700, Nick Desaulniers wrote:

> are you trying to fix the case where you do:

> $ make CC=clang ...
> <no HOSTCC set>
> when GCC is not installed?  Because if so, I think it would be easier
> to just specify HOSTCC=clang, but maybe I'm misunderstanding the
> issue?

It's that merge_config.sh calls make as part of its work.  When doing
that you can't use command line overrides, merge_config.sh would need to
pass them through explicitly which is probably more trouble than it's
worth so it doesn't.  Instead you need to set environment variables but
you then need to use ?= instead of = so make will use them.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH RFC 1/1] kbuild: enable overriding the compiler using the environment

[Nathan Chancellor]

On Thu, Aug 08, 2019 at 03:42:32PM -0700, 'Nick Desaulniers' via Clang Built Linux wrote:
> On Thu, Aug 8, 2019 at 2:07 PM Guillaume Tucker
> <guillaume.tucker@collabora.com> wrote:
> >
> > Only use gcc/g++ for HOSTCC, HOSTCXX and CC by default if they are not
> > already defined in the environment.  This fixes cases such as building
> > host tools with clang without having gcc installed.
> >
> > The issue was initially hit when running merge_config.sh with clang
> > only as it failed to build "HOSTCC scripts/basic/fixdep".
> 
> Thanks for the patch.  I don't quite follow the exact error.
> 
> When building with Clang, I usually do:
> 
> $ make CC=clang HOSTCC=clang ...
> 
> are you trying to fix the case where you do:
> 
> $ make CC=clang ...
> <no HOSTCC set>
> when GCC is not installed?  Because if so, I think it would be easier
> to just specify HOSTCC=clang, but maybe I'm misunderstanding the
> issue?

As I understand it,

$ make CC=clang HOSTCC=clang

works fine. What doesn't currently work is:

$ export CC=clang
$ export HOSTCC=clang
$ make

This is problematic because there is no way for CC, HOSTCC, and HOSTCXX
to be passed to make within scripts/kconfig/merge_config.sh.

A quick test before and after the patch:

$ ( export HOSTCC=clang; make -j$(nproc) O=out defconfig V=1 )
...
  gcc -Wp,-MD,scripts/kconfig/.conf.o.d -Wall -Wmissing-prototypes...
  gcc -Wp,-MD,scripts/kconfig/.confdata.o.d -Wall -Wmissing-prototypes...
...
$ ( export HOSTCC=clang; make -j$(nproc) O=out defconfig V=1 )
...
  clang -Wp,-MD,scripts/kconfig/.conf.o.d -Wall -Wmissing-prototypes -Wstrict-prototypes...
  clang -Wp,-MD,scripts/kconfig/.confdata.o.d -Wall -Wmissing-prototypes -Wstrict-prototypes...
...

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>

I wonder if all variable should be converted to that scheme or just the
ones that are needed in this instance. I also wonder if this will cause
any issues with people who define these variables in their environment
already; if so, maybe merge_config.sh should be updated to support
passing CC, HOSTCC, and HOSTCXX to make.

Cheers,
Nathan

Re: [PATCH RFC 1/1] kbuild: enable overriding the compiler using the environment

[Guillaume Tucker]

On 09/08/2019 00:35, Mark Brown wrote:
> On Thu, Aug 08, 2019 at 11:06:52PM +0200, Guillaume Tucker wrote:
> 
>> @@ -412,7 +412,7 @@ KBUILD_HOSTLDLIBS   := $(HOST_LFS_LIBS) $(HOSTLDLIBS)
>>  # Make variables (CC, etc...)
>>  AS		= $(CROSS_COMPILE)as
>>  LD		= $(CROSS_COMPILE)ld
>> -CC		= $(CROSS_COMPILE)gcc
>> +CC	       ?= $(CROSS_COMPILE)gcc
>>  CPP		= $(CC) -E
>>  AR		= $(CROSS_COMPILE)ar
>>  NM		= $(CROSS_COMPILE)nm
> 
> Why only for CC and not for anything else here?

This was the smallest possible change to fix the issue and what I
tested for this RFC.  Of course, if using ?= is a valid way to
fix it then it would seem logical to apply it to the other
variables defined there.

Guillaume

Re: [PATCH RFC 1/1] kbuild: enable overriding the compiler using the environment

[Guillaume Tucker]

On 09/08/2019 07:15, Nathan Chancellor wrote:
> On Thu, Aug 08, 2019 at 03:42:32PM -0700, 'Nick Desaulniers' via Clang Built Linux wrote:
>> On Thu, Aug 8, 2019 at 2:07 PM Guillaume Tucker
>> <guillaume.tucker@collabora.com> wrote:
>>>
>>> Only use gcc/g++ for HOSTCC, HOSTCXX and CC by default if they are not
>>> already defined in the environment.  This fixes cases such as building
>>> host tools with clang without having gcc installed.
>>>
>>> The issue was initially hit when running merge_config.sh with clang
>>> only as it failed to build "HOSTCC scripts/basic/fixdep".
>>
>> Thanks for the patch.  I don't quite follow the exact error.
>>
>> When building with Clang, I usually do:
>>
>> $ make CC=clang HOSTCC=clang ...
>>
>> are you trying to fix the case where you do:
>>
>> $ make CC=clang ...
>> <no HOSTCC set>
>> when GCC is not installed?  Because if so, I think it would be easier
>> to just specify HOSTCC=clang, but maybe I'm misunderstanding the
>> issue?
> 
> As I understand it,
> 
> $ make CC=clang HOSTCC=clang
> 
> works fine. What doesn't currently work is:
> 
> $ export CC=clang
> $ export HOSTCC=clang
> $ make
> 
> This is problematic because there is no way for CC, HOSTCC, and HOSTCXX
> to be passed to make within scripts/kconfig/merge_config.sh.
> 
> A quick test before and after the patch:
> 
> $ ( export HOSTCC=clang; make -j$(nproc) O=out defconfig V=1 )
> ...
>   gcc -Wp,-MD,scripts/kconfig/.conf.o.d -Wall -Wmissing-prototypes...
>   gcc -Wp,-MD,scripts/kconfig/.confdata.o.d -Wall -Wmissing-prototypes...
> ...
> $ ( export HOSTCC=clang; make -j$(nproc) O=out defconfig V=1 )
> ...
>   clang -Wp,-MD,scripts/kconfig/.conf.o.d -Wall -Wmissing-prototypes -Wstrict-prototypes...
>   clang -Wp,-MD,scripts/kconfig/.confdata.o.d -Wall -Wmissing-prototypes -Wstrict-prototypes...
> ...
> 
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>

Thanks for the review.

> I wonder if all variable should be converted to that scheme or just the
> ones that are needed in this instance. I also wonder if this will cause

This is what Mark also asked.  If we want to use ?= then I can
send another patch to cover all the other variables.  It also
makes sense to be able to choose an alternative linker, in
particular LLVM's ld.lld was brought up recently in some KernelCI
discussions.

> any issues with people who define these variables in their environment
> already; if so, maybe merge_config.sh should be updated to support
> passing CC, HOSTCC, and HOSTCXX to make.

I think the reason for the RFC essentially boils down to this.
On the other hand, if someone exports HOSTCC or CC to use some
specific compiler, they would expect it to be used.  It would
seem like a bit strange to export one value for a variable and
then pass another one to make (i.e. "export CC=gcc; make
CC=clang").  Also, passing all the variables to make in
merge_config.sh as well as any other place where this may happen
is likely to be rather error-prone and hard to maintain, say if
new variables get introduced in the Makefile or if some new
scripts start calling make.

So I'll prepare a new patch using the ?= approach.  Meanwhile
we'll see if someone can find a good reason why this can actually
be problematic.

Best wishes,
Guillaume

Re: [PATCH RFC 1/1] kbuild: enable overriding the compiler using the environment

[Masahiro Yamada]

On Fri, Aug 9, 2019 at 2:15 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Thu, Aug 08, 2019 at 03:42:32PM -0700, 'Nick Desaulniers' via Clang Built Linux wrote:
> > On Thu, Aug 8, 2019 at 2:07 PM Guillaume Tucker
> > <guillaume.tucker@collabora.com> wrote:
> > >
> > > Only use gcc/g++ for HOSTCC, HOSTCXX and CC by default if they are not
> > > already defined in the environment.  This fixes cases such as building
> > > host tools with clang without having gcc installed.
> > >
> > > The issue was initially hit when running merge_config.sh with clang
> > > only as it failed to build "HOSTCC scripts/basic/fixdep".
> >
> > Thanks for the patch.  I don't quite follow the exact error.
> >
> > When building with Clang, I usually do:
> >
> > $ make CC=clang HOSTCC=clang ...
> >
> > are you trying to fix the case where you do:
> >
> > $ make CC=clang ...
> > <no HOSTCC set>
> > when GCC is not installed?  Because if so, I think it would be easier
> > to just specify HOSTCC=clang, but maybe I'm misunderstanding the
> > issue?
>
> As I understand it,
>
> $ make CC=clang HOSTCC=clang
>
> works fine. What doesn't currently work is:
>
> $ export CC=clang
> $ export HOSTCC=clang
> $ make
>
> This is problematic because there is no way for CC, HOSTCC, and HOSTCXX
> to be passed to make within scripts/kconfig/merge_config.sh.

Is it so problematic?

If you start from make, CC=clang and HOSTCC=clang are propagated to sub-make
even via shell scripts such as merge_config.sh

Only the problem I see is the situation where
a user directly runs scripts/kconfig/merge_config.sh
without using make as a start-point.

A user can wrap merge_config.sh with a simple Makefile
if they want to override CC, HOSTCC, etc.


"You can easily pass environment variables" means
"the build system may accidentally pick up them
when it is not desirable."




> A quick test before and after the patch:
>
> $ ( export HOSTCC=clang; make -j$(nproc) O=out defconfig V=1 )
> ...
>   gcc -Wp,-MD,scripts/kconfig/.conf.o.d -Wall -Wmissing-prototypes...
>   gcc -Wp,-MD,scripts/kconfig/.confdata.o.d -Wall -Wmissing-prototypes...
> ...
> $ ( export HOSTCC=clang; make -j$(nproc) O=out defconfig V=1 )
> ...
>   clang -Wp,-MD,scripts/kconfig/.conf.o.d -Wall -Wmissing-prototypes -Wstrict-prototypes...
>   clang -Wp,-MD,scripts/kconfig/.confdata.o.d -Wall -Wmissing-prototypes -Wstrict-prototypes...
> ...
>
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
>
> I wonder if all variable should be converted to that scheme or just the
> ones that are needed in this instance. I also wonder if this will cause
> any issues with people who define these variables in their environment
> already; if so, maybe merge_config.sh should be updated to support
> passing CC, HOSTCC, and HOSTCXX to make.

This is not a problem for upstream code, at least.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH RFC 1/1] kbuild: enable overriding the compiler using the environment

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1134 bytes --]

On Tue, Aug 13, 2019 at 01:37:14AM +0900, Masahiro Yamada wrote:
> On Fri, Aug 9, 2019 at 2:15 PM Nathan Chancellor

> > This is problematic because there is no way for CC, HOSTCC, and HOSTCXX
> > to be passed to make within scripts/kconfig/merge_config.sh.

> Is it so problematic?

> If you start from make, CC=clang and HOSTCC=clang are propagated to sub-make
> even via shell scripts such as merge_config.sh

> Only the problem I see is the situation where
> a user directly runs scripts/kconfig/merge_config.sh
> without using make as a start-point.

This is really a very common thing for testing infrastructure to do,
it'll combine a base defconfig with a fragment enabling extra stuff
either to directly cover that extra stuff or to ensure that
configuration options needed for testsuites get turned on.  

> A user can wrap merge_config.sh with a simple Makefile
> if they want to override CC, HOSTCC, etc.

If we want to do that it seems sensible to provide that Makefile
upstream so there's a standard thing, it might also help people notice
that they need to do this and avoid getting surprised.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH RFC 1/1] kbuild: enable overriding the compiler using the environment

[Nick Desaulniers]

On Mon, Aug 12, 2019 at 10:14 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Tue, Aug 13, 2019 at 01:37:14AM +0900, Masahiro Yamada wrote:
> > Only the problem I see is the situation where
> > a user directly runs scripts/kconfig/merge_config.sh
> > without using make as a start-point.

Further, if it's possible to detect if merge_config.sh was invoked
from Make or not, it might be useful to warn or error when not invoked
via Make.

>
> This is really a very common thing for testing infrastructure to do,
> it'll combine a base defconfig with a fragment enabling extra stuff
> either to directly cover that extra stuff or to ensure that
> configuration options needed for testsuites get turned on.
>
> > A user can wrap merge_config.sh with a simple Makefile
> > if they want to override CC, HOSTCC, etc.
>
> If we want to do that it seems sensible to provide that Makefile
> upstream so there's a standard thing, it might also help people notice
> that they need to do this and avoid getting surprised.



-- 
Thanks,
~Nick Desaulniers