Re: Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
On 2012-02-16 07:21, David Holmes wrote:
On 16/02/2012 1:06 AM, Erik Joelsson wrote:
New webrev:
http://cr.openjdk.java.net/~erikj/7141244/webrev.04/
175 lines changed: 87 ins; 29 del; 59 mod; 3970 unchg
I discovered that the -include $(SPEC) line needed to be added to the
make/{bsd,solaris,linux}/makefiles/buildtree.make files. Otherwise the
hotspot build won't respect setting OPENJDK=true in the spec.
Is that the only change from version 3?
That is the only change yes.
Have you verified you can do all three variants: openjdk, jdk,
embedded jdk?
Not sure how you mean. The problem I corrected was discovered during
development in the build-infra repo, where I'm currently working on the
open vs non open scenarios. We haven't started looking at embedded much
yet (build-infra isn't currently building on anything but linux atm).
This issue, and the change, has no impact on the old build system,
either building just hotspot or from the root repo. I'm running my
hotspot-rt through jprt now anyway to be sure.
/Erik
Re: Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
New webrev:
http://cr.openjdk.java.net/~erikj/7141244/webrev.04/
175 lines changed: 87 ins; 29 del; 59 mod; 3970 unchg
I discovered that the -include $(SPEC) line needed to be added to the
make/{bsd,solaris,linux}/makefiles/buildtree.make files. Otherwise the
hotspot build won't respect setting OPENJDK=true in the spec.
It seems the windows build isn't respecting the OPENJDK variable today
so I won't bother fixing that.
/Erik
On 2012-02-07 17:30, 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)
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.
These changes have been verified to work for hotspot-rt. Jdk7u will be
verified as well before pushing.
/Erik
Re: Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
On 16/02/2012 1:06 AM, Erik Joelsson wrote:
New webrev:
http://cr.openjdk.java.net/~erikj/7141244/webrev.04/
175 lines changed: 87 ins; 29 del; 59 mod; 3970 unchg
I discovered that the -include $(SPEC) line needed to be added to the
make/{bsd,solaris,linux}/makefiles/buildtree.make files. Otherwise the
hotspot build won't respect setting OPENJDK=true in the spec.
Is that the only change from version 3?
Have you verified you can do all three variants: openjdk, jdk, embedded jdk?
It seems the windows build isn't respecting the OPENJDK variable today
so I won't bother fixing that.
Can't comment on that.
David
/Erik
On 2012-02-07 17:30, 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)
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.
These changes have been verified to work for hotspot-rt. Jdk7u will be
verified as well before pushing.
/Erik
Re: Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
2012/2/10 Kelly O'Hair kelly.oh...@oracle.com: Looks good to me. I assume you will work with John or someone in the hotspot team to get this integrated into one of the hotspot integration areas? Yes, we are already working with David Holmes to get these patches committed into hotspot. //Fredrik
Re: Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
Posted new webrev: http://cr.openjdk.java.net/~erikj/7141244/webrev.03/ 172 lines changed: 84 ins; 29 del; 59 mod; 3970 unchg See comments inline. On 2012-02-09 19:23, Kelly O'Hair wrote: The only issue I see is that it's using cygpath -m -a and not cygpath -s -m -a which I think means that the path could have spaces in it. Thanks for the review! There were indeed spaces in my own setup, but the quotes handled it. Getting rid of spaces is a good thing though so I added -s and it still works on my windows machine. On 2012-02-09 23:43, John Coomes wrote: Looks good. One minor request: in linux/makefiles/gcc.make, you moved the setting of STRIP under the SPEC conditional. Might as well fold it into the CROSS_COMPILE_ARCH conditional that's already there. Thanks for the review! That did look rather weird, I agree. Fixed it. /Erik
Re: Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
Looks good to me. I assume you will work with John or someone in the hotspot team to get this integrated into one of the hotspot integration areas? -kto On Feb 10, 2012, at 1:25 AM, Erik Joelsson wrote: Posted new webrev: http://cr.openjdk.java.net/~erikj/7141244/webrev.03/ 172 lines changed: 84 ins; 29 del; 59 mod; 3970 unchg See comments inline. On 2012-02-09 19:23, Kelly O'Hair wrote: The only issue I see is that it's using cygpath -m -a and not cygpath -s -m -a which I think means that the path could have spaces in it. Thanks for the review! There were indeed spaces in my own setup, but the quotes handled it. Getting rid of spaces is a good thing though so I added -s and it still works on my windows machine. On 2012-02-09 23:43, John Coomes wrote: Looks good. One minor request: in linux/makefiles/gcc.make, you moved the setting of STRIP under the SPEC conditional. Might as well fold it into the CROSS_COMPILE_ARCH conditional that's already there. Thanks for the review! That did look rather weird, I agree. Fixed it. /Erik
Re: Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
On 2012-02-09 03:51, David Holmes wrote: make/defs.make: + ifneq (,$(SPEC)) + include $(SPEC) + endif Having the blank first looks odd. I assume you aren't using -inlcude because you want to see errors if SPEC is set but not found. I guess it's an unconscious habit from java where you rather do .equals(something) to avoid NPE. I will switch it around. And the assumption is correct. We used -include at first, but I figured that we wanted to know if the include failed at least on the root level Makefile. make/windows/makefiles/compile.make: The definitions of MT=mt.exe in each block for the different VS versions seems redundant. If we factor this out is there any reason not to group: CXX=cl.exe MT=mt.exe RC=rc.exe LD=link.exe together and use the same if (,$(SPEC)) approach? Grouping them together would certainly look nicer, but MT isn't set for every possible compiler version. Not sure if that matters since we don't support older versions anyway, right? As for testing for SPEC, this is nmake and the SPEC file is only gnumake compatible. CXX, MT, RC and LD are sent in to nmake on the command line from gnumake. They are then generated into local.make which is in turn included by sub invocations of nmake. Sending in SPEC as well seemed redundant to me, but we could send it in as a flag signaling that configure should be in control. Wouldn't look obviously better to me though. I'm open for suggestions. /Erik
Re: Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
New webrev: http://cr.openjdk.java.net/~erikj/7141244/webrev.02/ http://cr.openjdk.java.net/%7Eerikj/7141244/webrev.02/ 177 lines changed: 89 ins; 29 del; 59 mod; 3970 unchg Changes since last time: * Moved the , to after $(SPEC) * Changed comment in gcc/sparcWorks.make according to suggestion from Fredrik. Haven't changed anything regarding the nmake files. /Erik On 2012-02-09 10:09, Erik Joelsson wrote: On 2012-02-09 03:51, David Holmes wrote: make/defs.make: + ifneq (,$(SPEC)) + include $(SPEC) + endif Having the blank first looks odd. I assume you aren't using -inlcude because you want to see errors if SPEC is set but not found. I guess it's an unconscious habit from java where you rather do .equals(something) to avoid NPE. I will switch it around. And the assumption is correct. We used -include at first, but I figured that we wanted to know if the include failed at least on the root level Makefile. make/windows/makefiles/compile.make: The definitions of MT=mt.exe in each block for the different VS versions seems redundant. If we factor this out is there any reason not to group: CXX=cl.exe MT=mt.exe RC=rc.exe LD=link.exe together and use the same if (,$(SPEC)) approach? Grouping them together would certainly look nicer, but MT isn't set for every possible compiler version. Not sure if that matters since we don't support older versions anyway, right? As for testing for SPEC, this is nmake and the SPEC file is only gnumake compatible. CXX, MT, RC and LD are sent in to nmake on the command line from gnumake. They are then generated into local.make which is in turn included by sub invocations of nmake. Sending in SPEC as well seemed redundant to me, but we could send it in as a flag signaling that configure should be in control. Wouldn't look obviously better to me though. I'm open for suggestions. /Erik
Re: Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
Looks good! //Fredrik - erik.joels...@oracle.com skrev: New webrev: http://cr.openjdk.java.net/~erikj/7141244/webrev.02/ http://cr.openjdk.java.net/%7Eerikj/7141244/webrev.02/ 177 lines changed: 89 ins; 29 del; 59 mod; 3970 unchg Changes since last time: * Moved the , to after $(SPEC) * Changed comment in gcc/sparcWorks.make according to suggestion from Fredrik. Haven't changed anything regarding the nmake files. /Erik On 2012-02-09 10:09, Erik Joelsson wrote: On 2012-02-09 03:51, David Holmes wrote: make/defs.make: + ifneq (,$(SPEC)) + include $(SPEC) + endif Having the blank first looks odd. I assume you aren't using -inlcude because you want to see errors if SPEC is set but not found. I guess it's an unconscious habit from java where you rather do .equals(something) to avoid NPE. I will switch it around. And the assumption is correct. We used -include at first, but I figured that we wanted to know if the include failed at least on the root level Makefile. make/windows/makefiles/compile.make: The definitions of MT=mt.exe in each block for the different VS versions seems redundant. If we factor this out is there any reason not to group: CXX=cl.exe MT=mt.exe RC=rc.exe LD=link.exe together and use the same if (,$(SPEC)) approach? Grouping them together would certainly look nicer, but MT isn't set for every possible compiler version. Not sure if that matters since we don't support older versions anyway, right? As for testing for SPEC, this is nmake and the SPEC file is only gnumake compatible. CXX, MT, RC and LD are sent in to nmake on the command line from gnumake. They are then generated into local.make which is in turn included by sub invocations of nmake. Sending in SPEC as well seemed redundant to me, but we could send it in as a flag signaling that configure should be in control. Wouldn't look obviously better to me though. I'm open for suggestions. /Erik
Re: Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
Added back runtime to the cc list On 9/02/2012 7:33 PM, Erik Joelsson wrote: New webrev: http://cr.openjdk.java.net/~erikj/7141244/webrev.02/ http://cr.openjdk.java.net/%7Eerikj/7141244/webrev.02/ 177 lines changed: 89 ins; 29 del; 59 mod; 3970 unchg Changes since last time: * Moved the , to after $(SPEC) * Changed comment in gcc/sparcWorks.make according to suggestion from Fredrik. Haven't changed anything regarding the nmake files. That's okay - it was just an observation. Seems okay to me. Let's see if we can get Kelly or someone else from HS to comment. David - /Erik On 2012-02-09 10:09, Erik Joelsson wrote: On 2012-02-09 03:51, David Holmes wrote: make/defs.make: + ifneq (,$(SPEC)) + include $(SPEC) + endif Having the blank first looks odd. I assume you aren't using -inlcude because you want to see errors if SPEC is set but not found. I guess it's an unconscious habit from java where you rather do .equals(something) to avoid NPE. I will switch it around. And the assumption is correct. We used -include at first, but I figured that we wanted to know if the include failed at least on the root level Makefile. make/windows/makefiles/compile.make: The definitions of MT=mt.exe in each block for the different VS versions seems redundant. If we factor this out is there any reason not to group: CXX=cl.exe MT=mt.exe RC=rc.exe LD=link.exe together and use the same if (,$(SPEC)) approach? Grouping them together would certainly look nicer, but MT isn't set for every possible compiler version. Not sure if that matters since we don't support older versions anyway, right? As for testing for SPEC, this is nmake and the SPEC file is only gnumake compatible. CXX, MT, RC and LD are sent in to nmake on the command line from gnumake. They are then generated into local.make which is in turn included by sub invocations of nmake. Sending in SPEC as well seemed redundant to me, but we could send it in as a flag signaling that configure should be in control. Wouldn't look obviously better to me though. I'm open for suggestions. /Erik
Re: Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
The only issue I see is that it's using cygpath -m -a and not cygpath -s -m -a which I think means that the path could have spaces in it. Otherwise, looks fine. -kto On Feb 9, 2012, at 1:33 AM, Erik Joelsson wrote: New webrev: http://cr.openjdk.java.net/~erikj/7141244/webrev.02/ http://cr.openjdk.java.net/%7Eerikj/7141244/webrev.02/ 177 lines changed: 89 ins; 29 del; 59 mod; 3970 unchg Changes since last time: * Moved the , to after $(SPEC) * Changed comment in gcc/sparcWorks.make according to suggestion from Fredrik. Haven't changed anything regarding the nmake files. /Erik On 2012-02-09 10:09, Erik Joelsson wrote: On 2012-02-09 03:51, David Holmes wrote: make/defs.make: + ifneq (,$(SPEC)) + include $(SPEC) + endif Having the blank first looks odd. I assume you aren't using -inlcude because you want to see errors if SPEC is set but not found. I guess it's an unconscious habit from java where you rather do .equals(something) to avoid NPE. I will switch it around. And the assumption is correct. We used -include at first, but I figured that we wanted to know if the include failed at least on the root level Makefile. make/windows/makefiles/compile.make: The definitions of MT=mt.exe in each block for the different VS versions seems redundant. If we factor this out is there any reason not to group: CXX=cl.exe MT=mt.exe RC=rc.exe LD=link.exe together and use the same if (,$(SPEC)) approach? Grouping them together would certainly look nicer, but MT isn't set for every possible compiler version. Not sure if that matters since we don't support older versions anyway, right? As for testing for SPEC, this is nmake and the SPEC file is only gnumake compatible. CXX, MT, RC and LD are sent in to nmake on the command line from gnumake. They are then generated into local.make which is in turn included by sub invocations of nmake. Sending in SPEC as well seemed redundant to me, but we could send it in as a flag signaling that configure should be in control. Wouldn't look obviously better to me though. I'm open for suggestions. /Erik
Re: Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
Erik Joelsson (erik.joels...@oracle.com) wrote: New webrev: http://cr.openjdk.java.net/~erikj/7141244/webrev.02/ http://cr.openjdk.java.net/%7Eerikj/7141244/webrev.02/ 177 lines changed: 89 ins; 29 del; 59 mod; 3970 unchg Changes since last time: * Moved the , to after $(SPEC) * Changed comment in gcc/sparcWorks.make according to suggestion from Fredrik. Looks good. One minor request: in linux/makefiles/gcc.make, you moved the setting of STRIP under the SPEC conditional. Might as well fold it into the CROSS_COMPILE_ARCH conditional that's already there. -John On 2012-02-09 10:09, Erik Joelsson wrote: On 2012-02-09 03:51, David Holmes wrote: make/defs.make: + ifneq (,$(SPEC)) + include $(SPEC) + endif Having the blank first looks odd. I assume you aren't using -inlcude because you want to see errors if SPEC is set but not found. I guess it's an unconscious habit from java where you rather do .equals(something) to avoid NPE. I will switch it around. And the assumption is correct. We used -include at first, but I figured that we wanted to know if the include failed at least on the root level Makefile. make/windows/makefiles/compile.make: The definitions of MT=mt.exe in each block for the different VS versions seems redundant. If we factor this out is there any reason not to group: CXX=cl.exe MT=mt.exe RC=rc.exe LD=link.exe together and use the same if (,$(SPEC)) approach? Grouping them together would certainly look nicer, but MT isn't set for every possible compiler version. Not sure if that matters since we don't support older versions anyway, right? As for testing for SPEC, this is nmake and the SPEC file is only gnumake compatible. CXX, MT, RC and LD are sent in to nmake on the command line from gnumake. They are then generated into local.make which is in turn included by sub invocations of nmake. Sending in SPEC as well seemed redundant to me, but we could send it in as a flag signaling that configure should be in control. Wouldn't look obviously better to me though. I'm open for suggestions. /Erik
Re: Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
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
Re: Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
Thanks for taking a look. While I haven't tried building in the way describe (yet), these changes shouldn't affect it. The idea is to leave everything in the hotspot build just like it is now, but let configure override things when configure is used, which is only when building the full forest. /Erik On 2012-02-08 02:54, Paul Hohensee wrote: Thanks for the explanation. As long as I can still do builds from make/os_family, I'm happy. Paul On 2/7/12 7:42 PM, David Holmes wrote: On 8/02/2012 2:55 AM, Paul Hohensee wrote: Can you make this work when building from the make/os_family directories? E.g., you could add the $(SPEC) include to make/solaris/Makefile, etc. I'm not sure that makes sense. The SPEC file only exists for the new build and in the new build you run configure for each platform, so if you configured for solaris then that is the only build you can do there. You can't decide to do a linux build instead - you need to re-run configure. I'll take a look at this ASAP but am somewhat swamped right now. David Thanks, Paul On 2/7/12 11: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) 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. These changes have been verified to work for hotspot-rt. Jdk7u will be verified as well before pushing. /Erik
Re: Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
- david.hol...@oracle.com skrev: This reminded me that one of the things on my wishlist for the new build was that I can still override something in SPEC by setting it on the command-line invocation of make. That should work, since a variable specified on the make command line, forcibly overrides any attempt to set it inside the makefiles. (Well, you can forcibly-forcibly override it again in the makefile, but we have no intention to do that.) We only need to make sure that the variables specified to the root makefile gets propagated to the sub-make command lines. Should be easy to fix. But it will not be part of this webrev. //Fredrik
Re: Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
New webrev up with the changes I detailed below: http://cr.openjdk.java.net/~erikj/7141244/webrev.01/ 177 lines changed: 89 ins; 29 del; 59 mod; 3970 unchg /Erik On 2012-02-08 09:14, Erik Joelsson wrote: 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
Re: Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
On 9/02/2012 1:36 AM, Erik Joelsson wrote: New webrev up with the changes I detailed below: http://cr.openjdk.java.net/~erikj/7141244/webrev.01/ 177 lines changed: 89 ins; 29 del; 59 mod; 3970 unchg make/defs.make: + ifneq (,$(SPEC)) + include $(SPEC) + endif Having the blank first looks odd. I assume you aren't using -inlcude because you want to see errors if SPEC is set but not found. make/windows/makefiles/compile.make: The definitions of MT=mt.exe in each block for the different VS versions seems redundant. If we factor this out is there any reason not to group: CXX=cl.exe MT=mt.exe RC=rc.exe LD=link.exe together and use the same if (,$(SPEC)) approach? David - /Erik On 2012-02-08 09:14, Erik Joelsson wrote: 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
Re: Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
Can you make this work when building from the make/os_family directories? E.g., you could add the $(SPEC) include to make/solaris/Makefile, etc. Thanks, Paul On 2/7/12 11: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) 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. These changes have been verified to work for hotspot-rt. Jdk7u will be verified as well before pushing. /Erik
Re: Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
Thanks for the explanation. As long as I can still do builds from make/os_family, I'm happy. Paul On 2/7/12 7:42 PM, David Holmes wrote: On 8/02/2012 2:55 AM, Paul Hohensee wrote: Can you make this work when building from the make/os_family directories? E.g., you could add the $(SPEC) include to make/solaris/Makefile, etc. I'm not sure that makes sense. The SPEC file only exists for the new build and in the new build you run configure for each platform, so if you configured for solaris then that is the only build you can do there. You can't decide to do a linux build instead - you need to re-run configure. I'll take a look at this ASAP but am somewhat swamped right now. David Thanks, Paul On 2/7/12 11: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) 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. These changes have been verified to work for hotspot-rt. Jdk7u will be verified as well before pushing. /Erik
