Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable

Erik Joelsson erik.joelsson at oracle.com
Wed Feb 8 08:14:53 UTC 2012


Thanks for looking at this!

On 2012-02-08 02:47, David Holmes wrote:
> It doesn't make sense to me to include SPEC in make/Makefile and 
> make/Defs.make because Makefile includes Defs.make. You only need the 
> -include in Defs.make (unless SPEC is going to define GAMMADIR or 
> ALT_OUTPUTDIR - in which case include it in the Makefile not Defs.make)
>
I checked this again and you are right, we don't set anything that 
warrants including SPEC in make/Makefile. I will move it to defs only.
> So this seems really ugly to me. If these were all set as Make 
> variables on a top-level make invocation then you wouldn't need to do 
> any of these tests. If the SPEC file is always going to set these 
> variables then why not either include SPEC or else do these 
> definitions eg:
>
> ifeq ($(SPEC),)
>   CC = ...
>   CXX = ..
>   ...
> endif
> # else SPEC already defined these
>
> this might need some refactoring to group the necessary settings 
> together.
>
This was how I initially did it, but I wasn't sure on the best solution. 
I also forgot about command line overriding normal assignments. With an 
explicit check for SPEC it's very obvious what we are trying to achieve. 
I will look into this and try to group things more neatly together for 
it. Hope to publish a new webrev later today.

/Erik



More information about the build-dev mailing list