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

Check if libsc is already a known target. #301

Merged

Conversation

jmark
Copy link

@jmark jmark commented Apr 3, 2024

Check if libsc is already a known target.

Proposed changes:

This PR adds a check if libsc is already a known target in the CMake build system. If
this is the case, registering libsc is skipped.

This is useful for cases when p4est is included as dependency by other software projects, e.g. t8code,
which in turn also depends on libsc. If this check would not be there CMake registers libsc twice which
causes conflicts.

@jmark
Copy link
Author

jmark commented Apr 3, 2024

@sandro-elsweijer @svengoldberg Feel free to comment if you see room for improvement.

@cburstedde
Copy link
Owner

Thanks; well spotted. @scivision, would you think this works given other hierarchical projects?

@scivision
Copy link
Contributor

I would use

if(NOT TARGET SC::SC)

as both targets have the same scope. The "SC::SC" is a bit easier to grok when finding where something came from.

@jmark
Copy link
Author

jmark commented Apr 18, 2024

I would use

if(NOT TARGET SC::SC)

as both targets have the same scope. The "SC::SC" is a bit easier to grok when finding where something came from.

@scivision I applied your suggestion. Thanks for the advice!

@cburstedde
Copy link
Owner

Thanks so much @jmark and @scivision!

@cburstedde cburstedde merged commit 7b66b0a into cburstedde:develop Apr 18, 2024
17 checks passed
@jmark jmark deleted the cmake-conditional-libsc-registration branch April 19, 2024 07:49
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.

None yet

3 participants