RFR: JDK-8077824: Introduce DefineNativeToolchain to handle toolchain configurations

Tim Bell tim.bell at oracle.com
Thu Apr 16 17:33:25 UTC 2015


Erik:

> New webrev: http://cr.openjdk.java.net/~erikj/8077824/webrev.root.02/
>
> On 2015-04-16 10:42, Magnus Ihse Bursie wrote:
>>
>> In general, I'm very happy with this. It's a nice encapsulation!
>>
> Thanks!
>> However, I do have a bunch of issues/questions:
>>
>> * AR is missing from the documentation of DefineNativeToolchain.
>>
> Fixed
>> * LDCXX is mentioned in the documentation of DefineNativeToolchain, 
>> but not used. It seems that it should not be used there, but it 
>> should be removed from the documentation.
> Correct, removed.
>>
>> * DefineNativeToolchain seems to do nothing if EXTENDS is not set. 
>> This is since all the "magic" is done by NamedParamsMacroTemplate. 
>> Maybe this should be explained in a comment.
>>
> Added a comment
>> * The comment "LANG C or C++" should be removed for 
>> SetupNativeCompilation.
>>
> Removed
>> * The call to $(CC) when processing RC files, should (perhaps?) be 
>> $1_CC now.
>>
> Fixed
>> * $1_MT should be in VARDEPS. (Maybe not really a regression, but it 
>> would be nice to fix it now)
>>
> Added
>> * I can't find that $1_CC is in VARDEPS either, should probably go in 
>> somewhere. In general, all the TOOLCHAIN variables should go in VARDEPS.
>>
> $1_CC and $1_CXX were already, but for the COMPILE_VARDEPS used for 
> compilation, not for linking. Missing was however $1_AS and some other 
> variables used in the linker recipe. I added all that I could find.
>> * BUILD_LIBJAWT seems to have lost a LANG := C++ but not gained a 
>> TOOLCHAIN := TOOLCHAIN_LINK_CXX. The same goes for LIBAWT_LANG, 
>> BUILD_LIBSUNMSCAPI, BUILD_LIBJSOUNDDS and the accessbridge stuff in 
>> Lib-jdk.accessibility.gmk.
>>
>> Is there some reason that the change here does not change the 
>> resulting link behavior? Or is this an oversight?
>>
> This is on purpose. The Windows only libraries link the same way 
> regardless of C or C++ so no need to add special configuration for them

Even better.  Looks good to me.

Tim




More information about the build-dev mailing list