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

[David Holmes]

Hi Erik,

On 8/02/2012 2:30 AM, Erik Joelsson wrote:
> http://cr.openjdk.java.net/~erikj/7141244/webrev.00/
> <http://cr.openjdk.java.net/%7Eerikj/7141244/webrev.00/>
> 178 lines changed: 115 ins; 7 del; 56 mod; 4625 unchg
>
> 7141244: build-infra merge: Include $(SPEC) in makefiles and make
> variables overridable
>
> The build-infra project is starting to move into jdk8. For the hotspot
> build to stay compatible with the changes, key hotspot makefiles need to
> add an optional include statement:
>
>
> -include $(SPEC)

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)

> In the new build system, the spec file is generated by configure and
> contains the configuration for the build. Only a handfull of files need
> to add this line.
>
> In addition to including the spec file, some variables need to be
> changed to only be set conditionally so that a value from the spec file
> takes precedence. These are: CC, CXX, CPP, AS, MCS, STRIP, NM, NAWK (and
> for windows RC and MT). The hotspot makefiles should check each variable
> if it's already set or if it has a gnumake default value.

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.

David
-----

> These changes have been verified to work for hotspot-rt. Jdk7u will be
> verified as well before pushing.
>
> /Erik
>