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 option to use a system installed sc #306

Merged
merged 5 commits into from
Jul 15, 2024
Merged

CMake option to use a system installed sc #306

merged 5 commits into from
Jul 15, 2024

Conversation

sandro-elsweijer
Copy link

Add CMake option to use a system installed SC

Proposed changes:
For our new t8code CMake CI we need the option to build p4est with a system installed SC version.
This option already exists in p4ests autotools build system but does not exist in the CMake build system.
This PR adds this option. The standard behavior of p4ests CMake build system is not altered since the standard policy is to use the SC which ships with p4est.

@sandro-elsweijer
Copy link
Author

Whoopsie, I did not see the other compile option and merged them now.

@cburstedde
Copy link
Owner

Thanks so much! We're in the middle of merging a few CMake updates in libsc and will then align p4est to work the same way as much as possible. Let's keep an eye on these merges to make sure your code works cleanly with those.

CMakeLists.txt Outdated Show resolved Hide resolved
@scivision
Copy link
Contributor

This PR should be rebased on develop p4est branch to make the CI pass

@cburstedde
Copy link
Owner

Thanks again! I'd be hoping that this runs fine on top of the latest develop branch.

@sandro-elsweijer
Copy link
Author

@cburstedde Done, at least locally all tests pass with the libsc shipped by p4est and also with a locally installed libsc (:

@cburstedde
Copy link
Owner

Thanks, and welcome to the list of developers! :)

As a follow-up, please consider #312 and let us know if that affects this update.

@cburstedde cburstedde merged commit 8206f0e into cburstedde:develop Jul 15, 2024
12 checks passed
@cburstedde
Copy link
Owner

Ah, btw., if (not necessarily for this one, but in the future) you might add a line to doc/release_notes.txt.

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.

3 participants