Skip to content

[cudnn] add other locations to find lib already installed#17346

Merged
strega-nil merged 6 commits intomicrosoft:masterfrom
cenit:dev/cenit/cudnn_linux
Apr 19, 2021
Merged

[cudnn] add other locations to find lib already installed#17346
strega-nil merged 6 commits intomicrosoft:masterfrom
cenit:dev/cenit/cudnn_linux

Conversation

@cenit
Copy link
Contributor

@cenit cenit commented Apr 18, 2021

Describe the pull request

  • What does your PR fix?

    Fixes [cudnn:x64-windows] build failure while installing opencv #17283
    Without this PR, CUDNN cannot be found in some default linux folders when installed with apt-get. Also, moved all reasonable variables to ENV, since CMake has to look for them, otherwise they are unuseful

  • Which triplets are supported/not supported? Have you updated the CI baseline?

    all

  • Does your PR follow the maintainer guide?

    Yes

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

    Yes

@cenit cenit changed the title [cudnn] [cudnn] add other locations to find lib already installed Apr 18, 2021
@cenit
Copy link
Contributor Author

cenit commented Apr 18, 2021

@rubenpraets @PBoog can you please tell me if this PR fixes your problem? It should, and it also fixes some problems I observed in some other linux configs

@JackBoosY JackBoosY added the category:port-bug The issue is with a library, which is something the port should already support label Apr 19, 2021
@rubenpraets
Copy link

I did a pull of your branch and yes, this solves my problem. Thanks!

@JackBoosY JackBoosY added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Apr 19, 2021
@PBoog
Copy link

PBoog commented Apr 19, 2021

Works for me, thanks for the fix!

@@ -1,5 +1,6 @@
Source: cudnn
Version: 7.6.5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we change this to 8 in concert with #17331 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it really depends for me
on one side, the broader the compatibility the better (and so i’d push for even a lower version in the minimum accepted by the portfile)
on another hand, these port files which are not installing anything and just finding compatible versions on the systems are already breaking binary caches at least for me. Different configs produce the same hash due to the same portfile and become invalid when consumed on the wrong system. To fix this at least reducing acceptable versions is helpful

@strega-nil strega-nil merged commit 1826fb8 into microsoft:master Apr 19, 2021
@cenit cenit deleted the dev/cenit/cudnn_linux branch April 19, 2021 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:port-bug The issue is with a library, which is something the port should already support info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[cudnn:x64-windows] build failure while installing opencv

6 participants