Good progress, it build for me on Arch Linux with gcc, not working for clang yet(perhaps expected). A few comments from what I read so far,

- --with-toolchain-type=list shows up after quite a few configure going on already, we probably want to have this done in an earlier stage.

- The "available" toolchains feels to me should be available on the build system, i.e., they are installed and available to use. From what is implemented, it's more "valid choices" than "available".

- On the above note, should "list" list "available" or "valid choices"?

- Do we want to allow convention name cc and CC and detect what it is or simply ask for explicit choice as it is imeplemted now? Although I don't think it's likely for a system to install, say gcc, as cc instead of create a symbolic link alias for gcc, I cannot be sure.

- I understand clang is not a goal for this patch. From experience, it's almost work like gcc. Does it make sense to have gcc flags as a fallback? This bring up the question before, should we have a fallback default to unknown toolchain? Even if we check for supported toolchain, all such setting should probably still be defensive with gcc as default. Otherwise, I can configure with clang but quite a few setting would be missing.

- On windows, gcc is listed as an available toolchain, I think this is good. However, we still do TOOLCHAIN_SETUP_VISUAL_STUDIO_ENV if select gcc as toolchain. That should not be checked for gcc?

Cheers,
Henry


On 02/12/2014 06:58 AM, Magnus Ihse Bursie wrote:
We need to be able to handle different toolchains on the same platform,
e.g. both gcc and clang on macosx. We have felt the need for this for
quite some time, but it has become pressing due to the clang on macosx
issue. (We are on a path to replace gcc with clang as official build
platform on macosx, but we need to handle both toolchains during a
transition period.

To do this properly requires a complete reorganisation and cleanup of
toolchain.m4.

This is, as you can see, quite a large change in the configure system.
Unfortunately, due to the bad state of the code in toolchain.m4, it was
hard to do it in smaller steps. I have still tried to keep the changes
small. For instance:
* Full clang functionality was not a goal for this patch.
* Fixing all issues in toolchain.m4 was not a goal for this patch.
(Although I've sprinkled the code with FIXME that I intend to address as
soon as possible.)

Both of these will be addressed in follow-up patches.

In this step, the priority has been to:
1) introduce the toolchain type concept,
2) cleanup toolchain.m4 to be able support this, and
3) move flag handling out of toolchain.m4

This is a (somewhat cleaned up) log from the commits to my internal
working repo, it gives a hint on what kind of changes/refactorisations
that were performed:
* Separate flag handling from toolchain.m4 into new file flags.m4.
* Remove all non-flags handling from flags code (this has aldready been
handled elsewhere).
* Clean up flags.m4 heavily. Rename functions FLAGS_*. Group setting of
flags by flag type. Simplify logic if possible. Adapt to new toolchain
type.
* Adapt configure.ac to how stuff have moved and splitted.
* Remove old COMPILER_TYPE and COMPILER_NAME, fully replace with
TOOLCHAIN_TYPE.
* Print toolchain and improved compiler version info in
HELP_PRINT_SUMMARY_AND_WARNINGS.
* Move AC_SUBST to where variables were set up.
* Move TOOLCHAIN_SETUP_MSVCR_DLL etc into new function
LIB_SETUP_ON_WINDOWS in libraries.m4.
* Fix compiler version output for microsoft compiler.
* Fix version parsing for solstudio.
* Reorder and rename functions in toolchain.m4.
* Move strip and mcs flags setup to flags.m4.
* Break out Windows compiler platform check from
TOOLCHAIN_CHECK_COMPILER_VERSION.
* Make sure --with-tools-dir override everything, even on windows.
* Add/update documentation for all functions in toolchain.m4
* Add/improve implementation comments.
* Add lots of FIXME:s for stuff needing fixes (but not this time around).
* Remove AS_IF
* Fix broken indentation.
* Fix bug in printing in BASIC_FIXUP_EXECUTABLE.

Bug: https://bugs.openjdk.java.net/browse/JDK-8034788
WebRev:
http://cr.openjdk.java.net/~ihse/JDK-8034788-rewrite-toolchain-m4/webrev.01

/Magnus

Reply via email to