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

Move MPIEXEC_NAME setting to top-level cmake #1141

Merged
merged 5 commits into from
Apr 6, 2021
Merged

Move MPIEXEC_NAME setting to top-level cmake #1141

merged 5 commits into from
Apr 6, 2021

Conversation

ohm314
Copy link
Member

@ohm314 ohm314 commented Apr 6, 2021

For convenience we set in various places the CMake variable MPIEXEC_NAME. In test/external/CMakeLists.txt this was not placed inside a test for MPI and caused the build to fail in some cases. This PR sets the variable in the top-level CMakeLists.txt so that the variable can be used everywhere.

This fixes #1140

@ohm314 ohm314 requested review from pramodk and olupton April 6, 2021 09:15
Copy link
Collaborator

@olupton olupton left a comment

Choose a reason for hiding this comment

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

Basically LGTM, just a couple of minor comments. Thanks!

test/CMakeLists.txt Outdated Show resolved Hide resolved
test/external/CMakeLists.txt Outdated Show resolved Hide resolved
@olupton
Copy link
Collaborator

olupton commented Apr 6, 2021

(I think the failures are because GitHub changed something and the builds hit resource limits; in #1137 I tested removing -j and the jobs started succeeding)

@codecov-io
Copy link

Codecov Report

Merging #1141 (fa97e06) into master (ef973e5) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1141   +/-   ##
=======================================
  Coverage   31.35%   31.35%           
=======================================
  Files         571      571           
  Lines      108810   108810           
=======================================
  Hits        34121    34121           
  Misses      74689    74689           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef973e5...fa97e06. Read the comment docs.

@pramodk pramodk merged commit af3896c into master Apr 6, 2021
@pramodk pramodk deleted the fix/1140 branch April 6, 2021 17:41
alexsavulescu pushed a commit that referenced this pull request Apr 13, 2021
alexsavulescu pushed a commit that referenced this pull request Apr 30, 2021
@alexsavulescu alexsavulescu mentioned this pull request Mar 22, 2022
15 tasks
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.

cmake fails in test/external/CMakeLists.txt when building without MPI
4 participants