[Skiboot] [PATCH] external/common: Adjust rules.mk to correctly use CROSS_COMPILE

[Cyril Bur]

On Tue, 2 Aug 2016 14:10:23 -0500
Patrick Williams <patrick at stwcx.xyz> wrote:

> On Tue, Aug 02, 2016 at 03:14:34PM +1000, Cyril Bur wrote:
> > diff --git a/external/common/rules.mk b/external/common/rules.mk
> > index bb12fd5..59e8905 100644
> > --- a/external/common/rules.mk
> > +++ b/external/common/rules.mk
> > @@ -1,5 +1,8 @@
> > -CC ?= $(CROSS_COMPILE)gcc
> > -LD ?= $(CROSS_COMPILE)ld
> > +ifdef CROSS_COMPILE
> > +CC = $(CROSS_COMPILE)gcc
> > +LD = $(CROSS_COMPILE)ld
> > +endif
> > +
> >  ARCH := $(shell $(GET_ARCH) "$(CROSS_COMPILE)")  
> 
> I don't see myself as a make expert, nor do I know what the best
> practices are in this regard.  Ultimately, you guys need to decide what
> is best for these makefiles but, with my current understanding, unless
> there is a corresponding change in both op-build and openbmc, this change
> will not work.

What exists doesn't make sense, I'm quite sure I hit what Oliver said
would happen. CC is always defined my make its self so ?= can't do what was
intended. This meant what while I used CROSS_COMPILE=/my/arm/cross-compiler, I
actually compiled for x86 as make defaulted to my system CC.

> 
> I've already spent a lot of time in the last month on getting libflash 
> / pflash makefiles set so we can do cross-compiling on ARM.  I know Joel,
> Brad and Stewart have also put in time in this area.  If this change
> goes in, I would prefer that someone volunteer to also do the
> corresponding updates to the op-build and openbmc distros.
> 

A lot of people have put a lot of effort in and it seems that each people had
their own use case they wanted to care about, heres mine:

As a somewhat frequent manual compiler of pflash and gard I don't want to jump
through hoops on the command line in order to get these to compile. The README
says to either include compilers is your PATH or to set CROSS. I feel this
should extend to the external/ binaries as well.

So, while I'm all for building skiboot and external/ tools in an automated
way, I actually do have a personal buildroot which builds pflash into an
initramfs, which I now suspect will have bitrotted as it only sets
CROSS_COMPILE...

It appears we broke manual compiling somewhere along the way. Admittedly I
should have been paying more attention to the list. We cannot be 'fixing'
skiboot for op-build or openbmc build processes which results in breaking
manual compilation.

> Now for my observations...
> 
> There is currently a CROSS and a CROSS_COMPILE variable in skiboot.
> What is each for?

I think they're the same, we should absolutely try to consolidate to one. The
README suggests using CROSS to specify a cross compiler, if we do all agree,
I'm more than happy to send a s/CROSS_COMPILE/CROSS/ patch to external/ tools

> 
> The ARCH variable is only set if CROSS_COMPILE is set with a proper
> prefix and the ARCH is critical for compiling libflash correctly.  Thus,
> with this change it will not be possible to set both a custom CC and
> allow the makefiles to continue to derive the ARCH from the prefix.  As

Makes sense, so how about:
-ARCH := $(shell $(GET_ARCH) "$(CROSS_COMPILE)")
+ARCH ?= $(shell $(GET_ARCH) "$(CROSS_COMPILE)")

> I have previously stated, having CC overwritten by CROSS_COMPILE breaks
> a non-simple CC and ties you to gcc (precluding clang).  We don't expect
> distros to determine ARCH themselves, do we?
> 

Both openbmc and op-build should know what arch they're building so setting
ARCH can't be a big deal? And with ARCH ?= could that work?

> Even the op-build recipe for libflash sets both CC and CROSS_COMPILE, so
> this isn't just a Yocto "problem".
> https://github.com/open-power/op-build/blob/master/openpower/package/libflash/libflash.mk#L20
>