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