RFR: JDK-8034788 Rewrite toolchain.m4 to support multiple toolchains per platform
erik.joelsson at oracle.com
Mon Feb 24 10:56:15 UTC 2014
Looks good to me.
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.)
> 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.)
More information about the build-dev