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

Improve target export #268

Merged
merged 3 commits into from
Mar 21, 2024
Merged

Improve target export #268

merged 3 commits into from
Mar 21, 2024

Conversation

ldowen
Copy link
Collaborator

@ldowen ldowen commented Feb 29, 2024

This fixes issues with target exports when using SpheralC.

Summary


ToDo :

  • Annotate RELEASE_NOTES.md with notable changes.
  • Create LLNLSpheral PR pointing at this branch. (PR#)
  • LLNLSpheral PR has passed all tests.

…SpheralC to use when necessary, removed BLT path export
@ldowen ldowen self-assigned this Feb 29, 2024
jmikeowen
jmikeowen previously approved these changes Mar 2, 2024
Copy link
Collaborator

@jmikeowen jmikeowen left a comment

Choose a reason for hiding this comment

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

Looks good to me

Comment on lines 18 to 26
set(CMAKE_C_COMPILER "@CMAKE_C_COMPILER@" CACHE PATH "Spheral C compiler path")
set(CMAKE_CXX_COMPILER "@CMAKE_CXX_COMPILER@" CACHE PATH "Spheral C++ compiler path")
set(CMAKE_Fortran_COMPILER "@CMAKE_Fortran_COMPILER@" CACHE PATH "Spheral C++ compiler path")
endif()
set(SPHERAL_ENABLE_MPI @ENABLE_MPI@ CACHE BOOL "")
if(SPHERAL_ENABLE_MPI AND NOT DEFINED MPI_C_COMPILER)
set(MPI_C_COMPILER "@MPI_C_COMPILER@" CACHE PATH "")
set(MPI_CXX_COMPILER "@MPI_CXX_COMPILER@" CACHE PATH "")
set(MPI_Fortran_COMPILER "@MPI_Fortran_COMPILER@" CACHE PATH "")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this going to set the compilers for every project that tries to include Spheral in it's CMake system? I think we might want to prefix these variable names with "SPHERAL_" so we aren't unintentionally forcing projects to use our compiler and MPI

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That was a concern of mine. I originally had it prefixed with SPHERAL but that didn't work because I think those flags needed to be set before spheral_cxx-targets is included. I hoped that guarding the sets with checks for whether they are already defined would be enough to prevent issues for other projects that already have those variables set. That was one of the things I hope Paul can try out.

@ldowen ldowen merged commit ea0df9b into develop Mar 21, 2024
1 check passed
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.

3 participants