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

[darknet] enable ninja #7064

Merged
merged 3 commits into from
Jun 28, 2019
Merged

Conversation

cenit
Copy link
Contributor

@cenit cenit commented Jun 27, 2019

I also left the same lengthy comment which follows inside the code itself, since it might be useful also for other ports (we should think about integrating it inside vcpkg sources maybe, or propose a fix to CMake upstream)

CMake looks for nvcc only in PATH and CUDACXX env vars for the Ninja generator. Since we filter path on vcpkg and CUDACXX env var is not set by CUDA installer on Windows, CMake cannot find CUDA when using Ninja generator, so we need to manually enlight it if necessary (https://gitlab.kitware.com/cmake/cmake/issues/19173). Otherwise we could just disable Ninja and use MSBuild, but unfortunately CUDA installer does not integrate with some distributions of MSBuild (like the ones inside Build Tools), making CUDA unavailable otherwise in those cases, which we want to avoid
Also, ninja is faster, so it's the best choice :)

edit: this comment is valid only for new CMake first class CUDA integration (enable_language(CUDA)). Old find_package(CUDA) has many tricks under its belt to be able to find CUDA in our cases, which is why for now it was not a common problem (I think darknet is the only port as of now using the new CMake first class CUDA integration 🥇 )

@vicroms vicroms self-assigned this Jun 28, 2019
@vicroms vicroms merged commit 7917599 into microsoft:master Jun 28, 2019
@cenit cenit deleted the dev/cenit/darknet_ninja branch June 29, 2019 08:02
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