Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
david.holmes at oracle.com
Tue Feb 7 17:47:19 PST 2012
On 8/02/2012 2:30 AM, Erik Joelsson wrote:
> 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:
CC = ...
CXX = ..
# else SPEC already defined these
this might need some refactoring to group the necessary settings together.
> These changes have been verified to work for hotspot-rt. Jdk7u will be
> verified as well before pushing.
More information about the hotspot-runtime-dev