Hello,
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.
/Erik
