RFR: JDK-8034788 Rewrite toolchain.m4 to support multiple toolchains per platform
henry.jen at oracle.com
Fri Feb 21 20:13:22 UTC 2014
I say ship it given you have tested it, and I don't see anything raise a flag. But I am not a JDK reviewer.
----- Original Message -----
From: magnus.ihse.bursie at oracle.com
To: erik.joelsson at oracle.com
Cc: build-dev at openjdk.java.net
Sent: Thursday, February 20, 2014 6:46:44 AM GMT -08:00 US/Canada Pacific
Subject: Re: RFR: JDK-8034788 Rewrite toolchain.m4 to support multiple toolchains per platform
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.)
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,
* 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.)
More information about the build-dev