| Message ID | 3885ccdcbdbe83eb367e8344584df944adc76e34.1565297255.git.guillaume.tucker@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kbuild: enable overriding the compiler using the environment | expand |
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?
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 >
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.
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
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
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
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.
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.
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.
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
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(-)