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

[cuda] correct env variables for newer cuda versions #12244

Merged
merged 2 commits into from
Jul 9, 2020

Conversation

Neumann-A
Copy link
Contributor

A bit of version updating for newer cuda versions

@JackBoosY
Copy link
Contributor

Does this PR fixes #11976?

@JackBoosY JackBoosY self-assigned this Jul 3, 2020
@JackBoosY JackBoosY added category:port-update The issue is with a library, which is requesting update new revision category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) labels Jul 3, 2020
@Neumann-A
Copy link
Contributor Author

@JackBoosY: At least for me:

Starting package 1/19: cuda:x86-windows
Building package cuda[core]:x86-windows...
-- Found CUDA V11.0.167
-- Performing post-build validation
-- Performing post-build validation done
Building package cuda[core]:x86-windows... done
Installing package cuda[core]:x86-windows...
Installing package cuda[core]:x86-windows... done

@JackBoosY
Copy link
Contributor

CUDA worked very well in the past, so I think this is caused by CUDA or findCUDA.cmake update. Do we need to judge based on the version of CUDA?

@JackBoosY JackBoosY linked an issue Jul 3, 2020 that may be closed by this pull request
@Neumann-A
Copy link
Contributor Author

I think it is related to updating visual studio which somehow wipes CUDA_PATH
After upgrading CUDA to 11 I got the following env vars now:

CUDA_PATH=C:\Program Files\NVIDIA GPU Computing Toolkit\CUDA\v11.0
CUDA_PATH_V10_2=C:\Program Files\NVIDIA GPU Computing Toolkit\CUDA\v10.2
CUDA_PATH_V11_0=C:\Program Files\NVIDIA GPU Computing Toolkit\CUDA\v11.0

CUDA also worked for me two months or so ago and suddenly broke. So it is highly likely that a VS upgrade removed CUDA_PATH and only left the versioned CUDA_PATH_V_XX_X.

@JackBoosY
Copy link
Contributor

Anyway, this changes LGTM.

@MVoz
Copy link
Contributor

MVoz commented Jul 3, 2020

new rules are now in effect)) CONTROL file

Version: 10.1-2

Version: 10.1
Port-Version: 2

ports/cuda/CONTROL Outdated Show resolved Hide resolved
Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>
@Neumann-A
Copy link
Contributor Author

@JackBoosY: We might need to define CUDA_PATHS on a more global level so that dependent ports can actually leverage the additional search paths but that is something for a different PR

@JackBoosY
Copy link
Contributor

@Neumann-A think we should add it to vcpkg_common_definitions or add a vcpkg function to provide it.

@JackBoosY
Copy link
Contributor

JackBoosY commented Jul 3, 2020

Let my colleague check this PR.
@ras0219 @ras0219-msft

@cenit
Copy link
Contributor

cenit commented Jul 3, 2020

CUDA_PATH should suffice tbh and it is working for cuda 11 for me without this PR :/ (and I also have vs fully up to date...)

@Neumann-A
Copy link
Contributor Author

Another possibility is that a Windows Update killed the CUDA_PATH environment variable. Something remove it. nvcc was still on path for me so the PATH variable has not been touched

@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Jul 6, 2020
@BillyONeal
Copy link
Member

The version check here looks bogus and we are still looking for a better way to publish the detected CUDA information for other ports, so I think we should merge this as is but we don't want to close the door on fixing that the right way.

Also this version check looks wrong https://github.com/microsoft/vcpkg/pull/12244/files#diff-efb436f7552399e0b189b636d44230f0R81

@ras0219-msft ras0219-msft merged commit 513cac2 into microsoft:master Jul 9, 2020
@cenit
Copy link
Contributor

cenit commented Jul 9, 2020

I remain on my idea that CUDA_PATH has to be defined in an environment for CUDA to be really working. It does not matter that this PR looks like fixing something, it’s just hiding problems which will reappear downwards. So, I would have left only CUDA_PATH for finding cuda. But again, it’s already merged, so be it.
The mood might be because today’s round of updates broke many ports on me, maybe...

@Neumann-A Neumann-A deleted the update_cuda branch July 17, 2020 19:23
@StarGate-One
Copy link
Contributor

@Neumann-A I lost CUDA 11 (along with some other VS extension) in both my VS2017 and VS2019 with the latest upgrades of VS. I had to reinstall the CUDA package to get it back in both versions.
I do not know why some VS upgrades (especially like a 16.6.3 to 16.6.4) wipe out some extensions but keep others????

strega-nil pushed a commit to strega-nil/vcpkg that referenced this pull request May 5, 2021
* [cuda] correct env variables for newer cuda versions

* Update ports/cuda/CONTROL

Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>

Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-update The issue is with a library, which is requesting update new revision category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[cuda] build failure
7 participants