Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMake MPI detection updates #1619

Merged
merged 3 commits into from
Jul 29, 2020
Merged

Conversation

RonRahaman
Copy link
Contributor

In this PR, CMake will test for MPI by using the cache variable CMAKE_CXX_COMPILER, rather than the environment variable CXX.

This will not change the behavior for anyone accustomed to using the env variable. This is because CMake searches for usable compilers in this order and then sets the cache var:

  1. Already-set cache variables: (CMAKE_CXX_COMPILER from command line or GUI)
  2. Environment variables ($CXX)
  3. Common C++ compiler names in $PATH

This should fix a few minor problems. For example, it will fix the behavior of find(HDF5) when re-running CMake.

@RonRahaman RonRahaman changed the title CMake test MPI with cache var instead of env var CMake MPI detection updates Jul 27, 2020
@RonRahaman
Copy link
Contributor Author

I also updated this to use FindMPI for CMake versions 3.10+. In 3.10, the FindMPI module was greatly improved and is actually usable :) . See the release notes: https://cmake.org/cmake/help/v3.10/release/3.10.html#modules.

@RonRahaman
Copy link
Contributor Author

After some more testing, I see that the FindMPI has a few corner cases. So I will put that on the back burner and propose just the first commit.

Copy link
Contributor

@paulromano paulromano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense -- thanks for the fix @RonRahaman!

@paulromano paulromano merged commit 7c13710 into openmc-dev:develop Jul 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants