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
