gh-96398: Improve accuracy of compiler checks in configure.ac #117815
Conversation
|
cc. @ronaldoussoren and @ned-deily |
This comment was marked as outdated.
This comment was marked as outdated.
|
In order to solve the WASI error, I propose to introduce a "GCC compatible" check, and use that as a gate to the compiler flag checks. UPDATE: proposed in #117825 |
Introduce a cached variable $ac_cv_gcc_compat and set it to 'yes' if the C preprocessor defines the __GNUC__ macro.
erlend-aasland
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
@igalic: I did not spot the question; did it get lost when you posted the comment? |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
a less bleary eyed review reveals that I was reading the LTO code wrong. not sure why my question didn't post at all, tho |
This comment was marked as resolved.
This comment was marked as resolved.
| @@ -1396,7 +1400,7 @@ AC_MSG_RESULT([$EXPORTSYMS]) | |||
| AC_SUBST([GNULD]) | |||
| AC_MSG_CHECKING([for GNU ld]) | |||
| ac_prog=ld | |||
| if test "$GCC" = yes; then | |||
| if test "$ac_cv_cc_name" = "gcc"; then | |||
There was a problem hiding this comment.
Consider still using $GCC here, alternatively $CC_BASENAME.
There was a problem hiding this comment.
CC_BASENAME may contain a platform triplet, so ac_cv_cc_name seems better.
| @@ -1845,7 +1847,7 @@ AC_MSG_RESULT([$PROFILE_TASK]) | |||
|
|
|||
| llvm_bin_dir='' | |||
| llvm_path="${PATH}" | |||
| if test "${CC}" = "clang" | |||
| if test "${ac_cv_cc_name}" = "clang" | |||
There was a problem hiding this comment.
Consider using $CC_BASENAME here.
There was a problem hiding this comment.
CC_BASENAME may contain a platform triplet, so ac_cv_cc_name seems better.
For the same reason, when the following code does clang_bin=`which clang`, shouldn't that be clang_bin=$CC?
| AC_MSG_CHECKING([which compiler should be used]) | ||
| case "${UNIVERSALSDK}" in | ||
| */MacOSX10.4u.sdk) | ||
| # Build using 10.4 SDK, force usage of gcc when the | ||
| # compiler is gcc, otherwise the user will get very | ||
| # confusing error messages when building on OSX 10.6 | ||
| CC=gcc-4.0 | ||
| CPP=cpp-4.0 | ||
| ;; | ||
| esac | ||
| AC_MSG_RESULT([$CC]) |
There was a problem hiding this comment.
Consider reverting this check and instead base it on $CC_BASENAME.
There was a problem hiding this comment.
Since our current minimum is apparently macOS 10.9, maybe this entire block can be removed.
There was a problem hiding this comment.
Believe it or not, current Pythons are still being built for and used on very old macOS systems (see, for example, the MacPorts project) but, to do so, one needs to use newer compilers than those supplied with the original releases (like gcc-4.0). So I agree, we should just remove this block.
erlend-aasland
changed the title
erlend-aasland
changed the title
| case "$CC_BASENAME" in | ||
| gcc) AC_PATH_TOOL([CXX], [g++], [g++], [notfound]) ;; | ||
| cc) AC_PATH_TOOL([CXX], [c++], [c++], [notfound]) ;; | ||
| clang|*/clang) AC_PATH_TOOL([CXX], [clang++], [clang++], [notfound]) ;; | ||
| icc|*/icc) AC_PATH_TOOL([CXX], [icpc], [icpc], [notfound]) ;; | ||
| clang) AC_PATH_TOOL([CXX], [clang++], [clang++], [notfound]) ;; | ||
| icc) AC_PATH_TOOL([CXX], [icpc], [icpc], [notfound]) ;; |
There was a problem hiding this comment.
This doesn't seem to correspond to the definition of AC_PATH_TOOL:
AC_PATH_TOOL (variable, prog-to-check-for, [value-if-not-found], [path = ‘$PATH’])
- variable = CXX
- prog-to-check-for = g++
- value-if-not-found = g++ (?)
- path = notfound (??)
There was a problem hiding this comment.
Also, CC_BASENAME may contain a platform triplet, so shouldn't this use ac_cv_cc_name instead?
|
FWIW: @mhsmith pinged me on this PR, because of the potential for clash with the iOS compiler shims (around L400 of configure.ac). I've confirmed that iOS builds still work with these changes. |
Some checks require a GCC compatible compiler; use $ac_cv_gcc_compat for these.
Some checks require checking the compiler basename; use $CC_BASENAME for these.
For the rest, use $ac_cv_cc_name.