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

KokkosKernelsConfig.cmake fixes for TPLs #522

Merged
merged 1 commit into from
Dec 12, 2019

Conversation

jjwilke
Copy link
Contributor

@jjwilke jjwilke commented Dec 12, 2019

  • Guard add_target with ifs to make sure not added twice
  • Misc fixes for the append config line function

@jjwilke jjwilke requested a review from ndellingwood December 12, 2019 21:34
@jjwilke jjwilke added this to the August Release 3.0 milestone Dec 12, 2019
@jjwilke jjwilke added the bug label Dec 12, 2019
@ndellingwood
Copy link
Contributor

@jjwilke what issues did you run into to require this change and in what types of builds?

@jjwilke
Copy link
Contributor Author

jjwilke commented Dec 12, 2019

Related to kokkos/kokkos#2587. After fixing this for Kokkos, realized KokkosKernels would have the same problem.

Testing on CUDA builds, I caught the other issue.

@jjwilke
Copy link
Contributor Author

jjwilke commented Dec 12, 2019

If you have multiple calls to find_package(KokkosKernels) or find_dependency(KokkosKernels) CMake would load KokkosKernelsConfig.cmake twice and calling add_library twice for the same library.

This was never actually an issue for any KokkosKernels projects, but would have been.

@ndellingwood
Copy link
Contributor

@jjwilke thanks for the explanation, shall I merge into develop or will that break your current work-flow?

@jjwilke
Copy link
Contributor Author

jjwilke commented Dec 12, 2019

This should merge into develop ASAP.

@ndellingwood ndellingwood merged commit 7c4fdfe into kokkos:develop Dec 12, 2019
@ndellingwood
Copy link
Contributor

@jjwilke merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants