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 find_package() fix and runtime path #8335

Closed
wants to merge 7 commits into from

Conversation

rotolof
Copy link

@rotolof rotolof commented Jan 19, 2022

  • The first commit will install FindCUDNN.cmake alongside DarknetConfig.cmake, fixes Including darknet library in my project using cmake #4729.

    Without this, the CMake command:

    find_package(Darknet)

    will not work because CMake doesn't know how to find CUDNN.

  • The second commit will install the library libdarknet.so and binaries uselib and darknet into subdirectories of CMAKE_INSTALL_PREFIX and set the runtime paths accordingly.

    Without this, if you specified, for example, -DCMAKE_INSTALL_PREFIX=/usr/local that stuff was still installed in the build directory.

@cenit
Copy link
Collaborator

cenit commented Jan 19, 2022

about the second commit, while i can understand that in principle it would be the best default, in practice darknet has the legacy of producing build artifacts in the source root folder.
If you want to customize it, you need to pass -DINSTALL_LIB_DIR=/path/to/your/favourite/lib/dir
and -DINSTALL_BIN_DIR=/path/to/your/favourite/bin/dir

@cenit
Copy link
Collaborator

cenit commented Jan 19, 2022

for the first commit, i agree with it and i might accept it
at the moment the copy is performed by build.ps1, in this way it would be done by cmake itself

@rotolof
Copy link
Author

rotolof commented Jan 20, 2022

about the second commit, while i can understand that in principle it would be the best default, in practice darknet has the legacy of producing build artifacts in the source root folder.
If you want to customize it, you need to pass -DINSTALL_LIB_DIR=/path/to/your/favourite/lib/dir
and -DINSTALL_BIN_DIR=/path/to/your/favourite/bin/dir

I see, I was wondering if there was a reason for not doing that already. For me it's OK as long as I'm able to specify the installation path.
I'm reverting those changes but the runtime path still needs to be set otherwise uselib and darknet will not find darknet.so unless they are in the same directory.

@rotolof rotolof changed the title Cmake installation and find_package() fix Cmake find_package() fix and runtime path Jan 20, 2022
@cenit
Copy link
Collaborator

cenit commented Jan 20, 2022

ok thanks.
Can you please put the RPATH variable in CACHE too?
Also, please take care of the copy of all cmake modules that we distribute in this repo, so that downstream projects like yours, which do not use build.ps1 script, can benefit from them

@rotolof
Copy link
Author

rotolof commented Jan 20, 2022

ok thanks.
Can you please put the RPATH variable in CACHE too?

Done!

Also, please take care of the copy of all cmake modules that we distribute in this repo, so that downstream projects like yours, which do not use build.ps1 script, can benefit from them

At the moment it wouldn't be useful as FindStb and FindPThreads4W are not called in DarknetConfig.cmake. Maybe there's something wrong with the logic in DarknetConfig.cmake but I don't know the project well enough to tell.

If I'm not mistaken, Stb is embedded in the project and it's only used for building so it's not really a dependency, and Threads are searched with other modules?

@cenit
Copy link
Collaborator

cenit commented Jan 20, 2022

while you're right for Stb, this is not true for PThreads4W. But see #8303 for an open issue about that, which you just reminded me of having to take a decision

@rotolof
Copy link
Author

rotolof commented Jan 20, 2022

Ok, I added FindPThreads4W.cmake

@cenit cenit mentioned this pull request Mar 6, 2022
6 tasks
@cenit cenit closed this Jul 1, 2022
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.

Including darknet library in my project using cmake
2 participants