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

ESMX_LINK_PACKAGES to work with package namespaces and other minor clean-up #182

Merged
merged 6 commits into from
Sep 6, 2023

Conversation

theurich
Copy link
Member

@theurich theurich commented Aug 31, 2023

This PR fixes an issue I encountered with the OpenMP and OpenACC packages under ESMX, using the link_packages feature under application in esmxBuild.yaml. Both these packages use namespaces, and cannot be used without namespace specification in target_link_libraries(). However, this call in Driver/CMakeLists.txt is redundant anyway, because if any component depends on a specific package (i.e. you need to use the link_packages option in the first place), then the component itself will have added the require target_link_libraries(). And the component will make that call with the correct namespace specification, e.g. for a specific language it uses, since locally it is known what it needs - after all it was the reason for the dependency in the first place.

All the other changes in this PR are minor clean-ups. A few typo corrections (in build options), bump ESMF dependency to 8.6.0, where it should be, and adding flags for the NVHPC compiler which I am currently experimenting with on Derecho.

@theurich theurich changed the title ESMX_LINK_PACKAGES to work with package namespaces ESMX_LINK_PACKAGES to work with package namespaces and other minor clean-up Aug 31, 2023
@theurich theurich marked this pull request as ready for review September 1, 2023 16:07
@theurich theurich self-assigned this Sep 1, 2023
@danrosen25
Copy link
Member

@theurich
I see the issue. It's intended to give a way link targets to the esmx_driver at the driver level if they are shared across different applications (for a cleaner yaml file) but it doesn't have fine enough control for finding a package then linking the needed target. We should rethink how this works.

@theurich
Copy link
Member Author

theurich commented Sep 6, 2023

@danrosen25 the approach in this PR works for what I am using the link_packages feature. It allows my ESMX applications to link against OpenMP and OpenACC packages correctly.
I'd like to merge the PR into develop just to move forward. Even if maybe we need to rethink some of the details down the road.

Copy link
Member

@danrosen25 danrosen25 left a comment

Choose a reason for hiding this comment

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

Sounds good. And thanks for cleaning up some of my bugs.

@theurich theurich merged commit d6dc119 into develop Sep 6, 2023
2 checks passed
@theurich theurich deleted the fix/esmx-link-package branch September 6, 2023 21:37
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.

2 participants