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

spdlog leaks into librmm (?) #2

Closed
leofang opened this issue Apr 28, 2021 · 10 comments · Fixed by #3
Closed

spdlog leaks into librmm (?) #2

leofang opened this issue Apr 28, 2021 · 10 comments · Fixed by #3

Comments

@leofang
Copy link
Member

leofang commented Apr 28, 2021

Not sure if this is intended --- I downloaded the librmm package and inspected the files locally, and this is what I saw:
截圖 2021-04-28 下午12 06 24
There are headers and also a static library for spdlog, but should these be included in the librmm package? I thought spdlog is a standalone package?

- spdlog =1.7.0

My guess is these are due to spdlog being fetched by cmake instead of conda.

cc: @kkraus14 @jakirkham @rlratzel

@kkraus14
Copy link
Contributor

Yep, need it in the host section instead of just the run section

@isuruf
Copy link
Member

isuruf commented Apr 28, 2021

lib64 needs to be changed to lib

@leofang
Copy link
Member Author

leofang commented Apr 28, 2021

Yep, need it in the host section instead of just the run section

It has run_exports so I think we just need the host section
https://github.com/conda-forge/spdlog-feedstock/blob/c96430f1cb8e576908e2cd295e9bf73a4a245d57/recipe/meta.yaml#L15-L16

@leofang
Copy link
Member Author

leofang commented Apr 28, 2021

@kkraus14 What exactly does the build script do? Why isn't it generating a librmm.so shared library?

@leofang
Copy link
Member Author

leofang commented Apr 28, 2021

(I mean I know it's a header-only library, so an equivalent question is why do we need a cmake build script for rmm? We didn't need one for Thrust/CUB.)

@kkraus14
Copy link
Contributor

I don't quite remember if the CMake install process generates the correct CMake pieces to allow someone to do a find_package(RMM)

@leofang
Copy link
Member Author

leofang commented Apr 28, 2021

OK. There are rmm-*.cmake included in the package, so I think this is why.

@jakirkham
Copy link
Member

Think we need it in both host and run since it is also needed by libraries including RMM

@leofang
Copy link
Member Author

leofang commented Apr 28, 2021

Yes, @jakirkham, I just pushed #3.

@leofang leofang mentioned this issue Apr 28, 2021
5 tasks
@jakirkham
Copy link
Member

Thanks Leo! 😄

Proposing we mark the old packages broken in PR ( conda-forge/admin-requests#259 ). So that users don't get these by accident

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 a pull request may close this issue.

4 participants