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

David Holmes david.holmes at oracle.com
Wed Feb 8 01:47:19 UTC 2012

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

More information about the build-dev mailing list