Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
erik.joelsson at oracle.com
Wed Feb 8 00:14:53 PST 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 = ..
> # else SPEC already defined these
> this might need some refactoring to group the necessary settings
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.
More information about the build-dev