Fix compiler test to not succeed with mpicc #956
Conversation
The compiler test for matching the intel compilers (icc, icpc, icl) was incorrectly matching the parallel compiler `mpicc`. This causes the `-w3` flag to be added to `WARNFLAGS` and `WARNFLAGS_MAINTAINER` and the cmake config fails since `mpicc` may not recognize that flag if it is not intel-based.
CMakeLists.txt
Outdated
| @@ -134,7 +134,7 @@ if(WITH_GZFILEOP) | |||
| add_definitions(-DWITH_GZFILEOP) | |||
| endif() | |||
|
|
|||
| if("${CMAKE_C_COMPILER}" MATCHES "icc" OR "${CMAKE_C_COMPILER}" MATCHES "icpc" OR "${CMAKE_C_COMPILER}" MATCHES "icl") | |||
| if(("${CMAKE_C_COMPILER}" MATCHES "icc" AND NOT "${CMAKE_C_COMPILER}" MATCHES "mpicc") OR "${CMAKE_C_COMPILER}" MATCHES "icpc" OR "${CMAKE_C_COMPILER}" MATCHES "icl") | |||
There was a problem hiding this comment.
I would rather see mpicc having own section before section for icc... That way this test doesn't become hard to read.
That new section would list correct flags that mpicc supports.
There was a problem hiding this comment.
mpicc is typically a wrapper around either gcc or clang or intel or ibm compilers. Would need to be able to somehow determine the underlying compiler (which is already done by CMake:
s1041980:zlib-ng-2.0.2(master)> CC=mpicc cmake -Bbuild -DCMAKE_INSTALL_PREFIX=${INSTALL_PATH} -DZLIB_COMPAT=YES .
-- Using CMake version 3.19.8
-- ZLIB_HEADER_VERSION: 1.2.11
-- ZLIBNG_HEADER_VERSION: 2.0.2
-- The C compiler identification is Clang 8.0.1
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /opt/local/bin/mpicc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
So maybe the test needs to be done on whatever symbol is set for C compiler identification? I will look closer to see how to make this cleaner.
There was a problem hiding this comment.
As far as I know, MPI has own package for CMake, so overriding CC and CXX is discouraged.
There was a problem hiding this comment.
In all of the parallel MPI builds that we do (Trilinos, SEACAS, HDF5, NetCDF), we set the compiler to mpicc and mpicxx. CMake does recognize that this is a parallel compiler and handles it from there...
|
Another thing that is already fixed in #906. |
|
@nmoinvaz Thanks. That matches the fix I was just getting ready to change this to. |
|
Why close this? I thought you were going to make the changes? This patch would be more isolated and we could get it in sooner. |
|
Sorry, I thought that it was fixed in #906. I can make the change |
Instead of trying to match compiler names, rely on the CMake logic that sets the compiler id and use that in the test.
|
@gsjaardema can you also squash the commits? |
I think the `APPLE` shoudl be `CMAKE_HOST_APPLE` also
|
@nmoinvaz Sorry, I've just been editting directly in github, so not sure how to squash the commits from here... |
|
Looks like the squash can be done at merge time -- https://docs.github.com/en/github/administering-a-repository/configuring-commit-squashing-for-pull-requests |
|
Editing directly on GitHub website is generally a bad idea... In the past we have required every contributor to squash pull requests themselves. I don't have adequate permissions or capacity to do required changes to our commit policy or repository settings. I will leave that kind of stuff to @Dead2 for now... |
|
Yes, I agree. I typically only do it for what I assume will be a short minimal change to one file; however, in this case it didn't work out that way. |
- Include porting guide in release packages zlib-ng#917 - Documentation improvements zlib-ng#913 zlib-ng#949 - Added Windows ARM binaries in release packages zlib-ng#916 - Fix crash on ARMv7 zlib-ng#927 - Fix building on FreeBSD zlib-ng#921 - Fix building with musl on aarch64 zlib-ng#936 zlib-ng#952 - Fix ARM float-abi detection zlib-ng#918 - Fix cmake detection of risc-v architectures zlib-ng#942 - Minor buildsystem fixes zlib-ng#922 zlib-ng#924 zlib-ng#933 zlib-ng#938 zlib-ng#950 - Improve zlib-compat build zlib-ng#915 zlib-ng#944 - CI/Test improvements zlib-ng#926 zlib-ng#929 zlib-ng#927 zlib-ng#937 zlib-ng#939 zlib-ng#940
|
I've got this messed up... Starting over... |
The compiler test for matching the intel compilers (icc, icpc, icl) was incorrectly matching the parallel compiler
mpicc. This causes the-w3flag to be added toWARNFLAGSandWARNFLAGS_MAINTAINERand the cmake config fails sincempiccmay not recognize that flag if it is not intel-based.