gh-96398: Improve accuracy of compiler checks in configure.ac#117815
Uh oh!
There was an error while loading. Please reload this page.
Conversation
erlend-aasland commented Apr 12, 2024
cc. @ronaldoussoren and @ned-deily |
Uh oh!
There was an error while loading. Please reload this page.
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.
Uh oh!
There was an error while loading. Please reload this page.
erlend-aasland commented Apr 13, 2024
@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.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as resolved.
This comment was marked as resolved.
igalic commented Apr 13, 2024
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 |
erlend-aasland changed the title erlend-aasland changed the title 
configure.ac Outdated
| 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?
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?
Yes, this should use ac_cv_cc_name, not CC_BASENAME.
This doesn't seem to correspond to the definition of AC_PATH_TOOL:
Indeed; I suggest we look at this in a separate PR.
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?
Addressed with 9900633.
freakboy3742 commented Apr 18, 2024
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. |
erlend-aasland added the 🔨 test-with-buildbots 
This comment was marked as outdated.
This comment was marked as outdated.
bedevere-bot removed the 🔨 test-with-buildbots erlend-aasland commented Oct 15, 2024
It seems there are refleaks on In the mean time, feel free to take another look, if needed. |
erlend-aasland added the 🔨 test-with-buildbots bedevere-bot commented Oct 28, 2024
🤖 New build scheduled with the buildbot fleet by @erlend-aasland for commit d309996 🤖 If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
bedevere-bot removed the 🔨 test-with-buildbots erlend-aasland added the 🔨 test-with-buildbots bedevere-bot commented Oct 29, 2024
🤖 New build scheduled with the buildbot fleet by @erlend-aasland for commit 9900633 🤖 If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
bedevere-bot removed the 🔨 test-with-buildbots erlend-aasland commented Nov 5, 2024
Heads-up: I plan to merge this within a few days. |
erlend-aasland commented Nov 7, 2024
Thanks for reviewing, everyone! |
…ython#117815) The following variables are now used in compiler checks: - $ac_cv_gcc_compat is set to 'yes' for GCC compatible compilers (the C preprocessor defines the __GNUC__ macro) - for compiler basename checks, use $CC_BASENAME (may contain platform triplets) - for the rest, use $ac_cv_cc_name (does not contain platform triplets)
…ython#117815) The following variables are now used in compiler checks: - $ac_cv_gcc_compat is set to 'yes' for GCC compatible compilers (the C preprocessor defines the __GNUC__ macro) - for compiler basename checks, use $CC_BASENAME (may contain platform triplets) - for the rest, use $ac_cv_cc_name (does not contain platform triplets)
7 participants
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.