RFR: JDK-8034788 Rewrite toolchain.m4 to support multiple toolchains per platform

Erik Joelsson erik.joelsson at oracle.com
Mon Feb 24 10:56:15 UTC 2014


Looks good to me.

/Erik

On 2014-02-20 15:43, Magnus Ihse Bursie wrote:
> On 2014-02-13 22:27, Magnus Ihse Bursie wrote:
>>
>> It turned out that it was not a good idea to line break AC_MSG_* 
>> functions. :-( I didn't verify that properly before; my bad.
>>
>> Here's a new-new review which restores the old long (but 
>> properly-looking) AC_MSG lines.
>>
>> (If you're curious, the line break was printed verbatim. Putting a 
>> trailing \ did remove the line break, but then the indentation level 
>> showed up as verbatim spaces in the output.)
>>
>> WebRev: 
>> http://cr.openjdk.java.net/~ihse/JDK-8034788-rewrite-toolchain-m4/webrev.03
>
> Now I have finally really put this change to the test: building with 
> and without the patch on all platforms and running the compare script, 
> to compare the build result with and without the patch. Of course, I 
> found some bugs that I have missed. On the upside, now I am *really* 
> confident in this patch. I'm running a final round of confirmation 
> tests (to make sure my latest fixes didn't cause any other breakage) 
> but I'd be surprised if there turned up anything.
>
> Given that the remaining test round is green, I'd like to get a final 
> round of ack's from the reviewers.
>
> Unfortunately, webrev can't provide easy differential reviews, but the 
> changes since last time are:
> * Added AC_SUBST(TOOLCHAIN_TYPE) in 
> TOOLCHAIN_DETERMINE_TOOLCHAIN_TYPE. (oops! Thank's Erik for that one)
> * Fixed a typo in LIB_SETUP_STATIC_LINK_LIBSTDCPP in libraries.m4, 
> $TOOLCHAIn_TYPE was not found so we got LIBCXX wrong on macosx.
> * On Solaris, $TR a-z A-Z does not work as expected. Replaced with 
> longer but safer version when deriving OPENJDK_TARGET_OS_UPPERCASE.
> * For solstudio, the CC_HIGHEST setting was missing spaces between 
> some arguments, making -fsimple-fsingle-xalias_level... look like a 
> single, invalid argument.
> * The compiler version string for solstudio used [...] in sed instead 
> of @<:@...@:>@. But m4 eats the [] so we need this ugly escaping. Not 
> the first time I forget that one. :-/
> * The recent reconfigure patch was slightly incorrect, we shouldn't 
> put "..." around the command line arguments when calling configure in 
> the makefile. (Can't see how that one got through all my testing :-( 
> but failed at the very first real-world test.)
>
> WebRev: 
> http://cr.openjdk.java.net/~ihse/JDK-8034788-rewrite-toolchain-m4/webrev.04
>
> /Magnus




More information about the build-dev mailing list