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.

Sign up for GitHub

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

Jump to bottom

Feature/cmake export targets #238

Merged
merged 23 commits into from
Oct 2, 2023
Merged

Feature/cmake export targets #238

merged 23 commits into from
Oct 2, 2023

Conversation

ldowen
Copy link
Collaborator

@ldowen ldowen commented Sep 19, 2023 •
edited
Loading

Summary

  • This PR is a feature for properly exporting CMake targets
  • It does the following:
    • Provides First Class Project support for CMake,
    • Spheral and its dependencies are exported to allow the use of find_package for importing into another project.

ToDo :

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

@ldowen
Export targets mostly working, changes in PYB11 are necessary and oth…
d5ec3db 
…er python interface stuff is still broken
@ldowen
SpheralC tests are working with this export configuration, next is wo…
746fc0e 
…rking on python build
@ldowen
Added logic with function inputs to align with LLNLSpheral needs, sho…
129df60 
…uld be working with python version now, temporarily using different branches for polyclipper and pyb11generator
@ldowen
Added include for LLNLSpheral obj install cmake file
4755df3
@ldowen
Add include_directories statement back in
16c2198
@ldowen
Remove the BUILD_INTERFACE before the eigen_INCLUDE statement from spack
ca042fe
@ldowen
Prevent warnings for PolyClipper by treating it as system
83d3414
@ldowen
Add redundant system calls on targets for testing
bc71522
@ldowen
Fixed typo for treating imported includes as system
6d081a0
@ldowen
Fixed bug with serial compilation, updated PYB11 and PolyClipper subm…
fb8857b 
…odules and relevant cmake variable names
@ldowen
Update PolyClipper submodule
b95dcc1
@ldowen
Fixed issue with PolyClipper docs trying to build during CI
2d721c5
@ldowen
Update PolyClipper
5b6ac71
@ldowen
Fix bug where find_library was only called for first library file, ad…
04be531 
…d lib to directory to INSTALL_RPATH for CXX targets
@ldowen
Change spheral_config file to set BLT location, update PolyClipper an…
94caa89 
…d added variable for setting a separate install location for it, add a build-dir option to host-config-build.py
mdavis36
mdavis36 requested changes Sep 25, 2023
View reviewed changes
.gitmodules Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
cmake/InstallTPLs.cmake Outdated Show resolved Hide resolved
cmake/InstallTPLs.cmake Outdated Show resolved Hide resolved
cmake/InstallTPLs.cmake Outdated Show resolved Hide resolved
src/CMakeLists.txt Outdated Show resolved Hide resolved
src/CMakeLists.txt Outdated Show resolved Hide resolved
src/CXXTests/CMakeLists.txt Outdated Show resolved Hide resolved
cmake/spheral_cxx-config.cmake.in Show resolved Hide resolved
cmake/spheral_cxx-config.cmake.in Outdated Show resolved Hide resolved
mdavis36
mdavis36 requested changes Sep 26, 2023
View reviewed changes
cmake/InstallTPLs.cmake Outdated Show resolved Hide resolved
src/CMakeLists.txt Outdated Show resolved Hide resolved
src/CMakeLists.txt Outdated Show resolved Hide resolved
mdavis36
mdavis36 reviewed Sep 26, 2023
View reviewed changes
cmake/SetupSpheral.cmake Outdated Show resolved Hide resolved
@ldowen
Changed spheral_add_obj_library function to require global variable n…
c6e2f7e 
…ame as input, added variable name input for these function calls, moved the install and export calls into the TPL function, merged lists for source CMakeList.txt
@ldowen
Need to check if ENABLE_CXXONLY is on before including certain packages
be452f7
@ldowen
Minor cosmetic changes
f700bde
@ldowen
Point PYB11Generator and PolyClipper submodules back to their master …
97c51ab 
…branches and update them
mdavis36
mdavis36 previously approved these changes Sep 29, 2023
View reviewed changes
Copy link
Collaborator

@mdavis36 mdavis36 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This looks good to go for me, a lot of good improvements to our build system here and I'm sure our users will appreciate the export targets for their own build systems. If you could fill out some information about the changes made here in the PR description and in RELEASE_NOTES then once tests pass in LLNLSpheral with this commit we can merge.

@jmikeowen may want to take a pass at this too.

jmikeowen
jmikeowen previously approved these changes Sep 29, 2023
View reviewed changes
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.

Yep, looks good to go to me.

@ldowen ldowen dismissed stale reviews from jmikeowen and mdavis36 via b2a4f13 September 29, 2023 18:37
jmikeowen
jmikeowen approved these changes Oct 2, 2023
View reviewed changes
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.

Good to go

@ldowen ldowen merged commit daece81 into develop Oct 2, 2023
1 check passed
@ldowen ldowen deleted the feature/cmake_export_targets branch October 2, 2023 17:16
@ldowen ldowen mentioned this pull request Oct 10, 2023
Closed
@mdavis36 mdavis36 mentioned this pull request Dec 4, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mdavis36 mdavis36 mdavis36 approved these changes

@jmikeowen jmikeowen jmikeowen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ldowen @jmikeowen @mdavis36