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

Kelly O'Hair Kelly.Ohair at oracle.com
Fri Feb 10 17:18:37 UTC 2012


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




More information about the build-dev mailing list