Skip to content

cmake: update variables on reconfigure#239

Open
hannesbrandt wants to merge 2 commits intocburstedde:developfrom
hannesbrandt:fix-cmake-reconfigure
Open

cmake: update variables on reconfigure#239
hannesbrandt wants to merge 2 commits intocburstedde:developfrom
hannesbrandt:fix-cmake-reconfigure

Conversation

@hannesbrandt
Copy link
Collaborator

Update variables on reconfigure

Proposed changes:
Enabling MPI in cmake and then disabling it in a later reconfiguration causes an issue. While SC_ENABLE_MPI is updated correctly, other MPI-related variables like SC_ENABLE_MPIIO remain true, which causes errors during the build step.

There already is an if-clause in cmake/config.cmake that checks if SC_ENABLE_MPI was changed and updates the other MPI-related variables as desired. The clause is entered, if SC_ENABLE_MPI is different from its previous value CACHED_SC_ENABLE_MPI and updates CACHED_SC_ENABLE_MPI inside.
The update of CACHED_SC_ENABLE_MPI uses the set-function (https://cmake.org/cmake/help/latest/command/set.html). According to its documentation, it does not necessarily overwrite the value of exisiting cache entries when the FORCE option is not enabled.

Since cache entries are meant to provide user-settable values this does not overwrite existing
cache entries by default. Use the FORCE option to overwrite existing entries.

This PR adds FORCE to the set command, so that the if-clause will be activated as intended.

@cburstedde
Copy link
Owner

Thanks! This seems to be unrelated, but are you able to have the CI run through?

@hannesbrandt
Copy link
Collaborator Author

I've added some tests that were not yet included in the CMakeLists.txt, see #238.
Regarding the CI, the first issue occurred in the autotools run of test_io_file. In the rerun after the most recent commit, the same test was successful for autotools but failed for cmake. And after triggering another rerun by hand, both versions were successful. So the issue does not seem to occur consistently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants