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

Kelly O'Hair Kelly.Ohair at oracle.com
Fri Feb 10 17🔞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 CROSSCOMPILEARCH 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