| Message ID | 1470195679-25985-1-git-send-email-oohall@gmail.com |
|---|---|
| State | Rejected |
| Headers | show |
On Wed, 3 Aug 2016 13:41:19 +1000 Oliver O'Halloran <oohall@gmail.com> wrote: > The intended usage of CROSS_COMPILE is to supply the common toolchain > prefix so that CC, LD, etc can be set automatically by make. Build systems > that set these variables directly don't need (or want) this behaviour so > using CROSS_COMPILE for any other purpose is inherently broken. > > Currently CROSS_COMPILE is used to set CC/LD (as above), but it is also > used when determining the target architecture. The implementation of this > only requires CROSS_COMPILE set be set so get_arch.sh can use the cross > toolchain's the C preprocessor directly. This is unnecessary since > equivalent functionality can be achieved using CC in preprocessor mode > with the -E flag which is supported by both gcc and clang. > > Cc: Cyril Bur <cyril.bur@au1.ibm.com> > Cc: Patrick Williams <patrick@stwcx.xyz> > Signed-off-by: Oliver O'Halloran <oohall@gmail.com> > --- > external/common/get_arch.sh | 2 +- > external/common/rules.mk | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/external/common/get_arch.sh b/external/common/get_arch.sh > index f4beb1df4faf..084cf79acdcf 100755 > --- a/external/common/get_arch.sh > +++ b/external/common/get_arch.sh > @@ -7,5 +7,5 @@ echo -n ARCH_X86 > echo -n ARCH_ARM > #else > echo -n ARCH_UNKNOWN > -#endif" | $1cpp | /bin/sh > +#endif" | $1 -E - | /bin/sh > > diff --git a/external/common/rules.mk b/external/common/rules.mk > index bb12fd5b286b..172dda7113d8 100644 > --- a/external/common/rules.mk > +++ b/external/common/rules.mk > @@ -1,6 +1,6 @@ > CC ?= $(CROSS_COMPILE)gcc > LD ?= $(CROSS_COMPILE)ld Will need my patch otherwise CC will remain the system compiler which on my laptop yields an x86 binary despite giving an arm CROSS_COMPILE. With both patches applied appears to do work for me. > -ARCH := $(shell $(GET_ARCH) "$(CROSS_COMPILE)") > +ARCH := $(shell $(GET_ARCH) "$(CC)") > > ifeq ($(ARCH),ARCH_ARM) > arch := arm
Cyril Bur <cyril.bur@au1.ibm.com> writes: > On Wed, 3 Aug 2016 13:41:19 +1000 > Oliver O'Halloran <oohall@gmail.com> wrote: > >> The intended usage of CROSS_COMPILE is to supply the common toolchain >> prefix so that CC, LD, etc can be set automatically by make. Build systems >> that set these variables directly don't need (or want) this behaviour so >> using CROSS_COMPILE for any other purpose is inherently broken. >> >> Currently CROSS_COMPILE is used to set CC/LD (as above), but it is also >> used when determining the target architecture. The implementation of this >> only requires CROSS_COMPILE set be set so get_arch.sh can use the cross >> toolchain's the C preprocessor directly. This is unnecessary since >> equivalent functionality can be achieved using CC in preprocessor mode >> with the -E flag which is supported by both gcc and clang. So... I've been watching all the various patches to all the hand-written and hacked makefiles for our userspace bits and doing *everything* in my power to not go off and rewrite them all to be autotools based and push it out just before tagging skiboot 5.3.0. The steady state for hand written makefiles for userspace is this: broken for somebody. So, I'm currently very much inclined to not merge any more fixes unless they come from a packager (which pretty much means OpenBMC at this point in time) or they're a patch to move everything over to autotools, where at least the brain damage is well known rather than custom.
diff --git a/external/common/get_arch.sh b/external/common/get_arch.sh index f4beb1df4faf..084cf79acdcf 100755 --- a/external/common/get_arch.sh +++ b/external/common/get_arch.sh @@ -7,5 +7,5 @@ echo -n ARCH_X86 echo -n ARCH_ARM #else echo -n ARCH_UNKNOWN -#endif" | $1cpp | /bin/sh +#endif" | $1 -E - | /bin/sh diff --git a/external/common/rules.mk b/external/common/rules.mk index bb12fd5b286b..172dda7113d8 100644 --- a/external/common/rules.mk +++ b/external/common/rules.mk @@ -1,6 +1,6 @@ CC ?= $(CROSS_COMPILE)gcc LD ?= $(CROSS_COMPILE)ld -ARCH := $(shell $(GET_ARCH) "$(CROSS_COMPILE)") +ARCH := $(shell $(GET_ARCH) "$(CC)") ifeq ($(ARCH),ARCH_ARM) arch := arm
The intended usage of CROSS_COMPILE is to supply the common toolchain prefix so that CC, LD, etc can be set automatically by make. Build systems that set these variables directly don't need (or want) this behaviour so using CROSS_COMPILE for any other purpose is inherently broken. Currently CROSS_COMPILE is used to set CC/LD (as above), but it is also used when determining the target architecture. The implementation of this only requires CROSS_COMPILE set be set so get_arch.sh can use the cross toolchain's the C preprocessor directly. This is unnecessary since equivalent functionality can be achieved using CC in preprocessor mode with the -E flag which is supported by both gcc and clang. Cc: Cyril Bur <cyril.bur@au1.ibm.com> Cc: Patrick Williams <patrick@stwcx.xyz> Signed-off-by: Oliver O'Halloran <oohall@gmail.com> --- external/common/get_arch.sh | 2 +- external/common/rules.mk | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)