-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
[OpenCV] update to 3.4.1 #2906
[OpenCV] update to 3.4.1 #2906
Conversation
@cenit Could you merge the latest master to this PR? I checked out your PR and I had to reinstall a lot of dependent libraries because of recent updates. @ras0219-msft Is it possible to have two separate copies of vcpkg and not have it conflict with each other for the ease of testing out PRs? I am running into problems and I am guessing it's because the environmental variables are shared and perhaps the registry are shared too. |
Multiple copies of vcpkg should be 100% independent and not conflict -- we make sure to never modify your environment or registry to ensure this is so. Both @alexkaratarakis and I use multiple enlistments daily on our work machine, so it does at least work on our machines 😄! One behavior that may be surprising is that if you run vcpkg from one repo with your current directory in another, it will attempt to act on the current path's repo. This can be problematic if the version skew is too great, but is easily fixed by ensuring you're running the vcpkg executable from the current repo. Could you share more about what isn't working for you? If it's more involved, perhaps posting a separate issue would be appropriate. |
@jasjuang I can confirm that multiple (tens as of now... 🤣) vcpkg enlistments can work on my machine too without any problem. I only have to disable the |
Tried to build |
Is it related to the infamous v140-only cuda problem or you found also problems with v140 in non-x64 non-dynamic linking triplets? If the former, unfortunately the fix is out of scope of this PR. If the latter, could you please share some logs of the errors you are experiencing? |
If you build |
@cenit I finally had a chance to test this out. |
I've also tested this PR and it works like a charm with Please also note that without this PR the port may not compile.
As pointed out by the compiler, the problem seems to be related to a missing Thanks @cenit for the effort and the PR. Hope to see it merged soon! |
Solid PR! Thanks! |
So how did you manage to do that? With which version of CUDA and which version of MSVC? With 9.1 and 19.13 I am unable to compile any CUDA application. An attempt to build
|
@paul-michalik I am able to compile it with CUDA 9.1, VS2017 15.4. |
@paul-michalik I'm also with CUDA 9.1, VS2017 15.4. What is 19.13 version you mentioned? |
@jasjuang @claudiofantacci It refers to the MSVC++ toolset version as reported by |
@paul-michalik be aware that upgrading to the latest toolset is always dangerous using CUDA and OpenCV. CUDA updates go at a lower pace wrt toolset update. This is a problem also on macOS when they update Xcode and the CLI toolset. I'm really confused by all the |
@claudiofantacci Well, it's not an OpenCV issue, I guess. It's CUDA which, as you say, does not update quick enough. The real problem is that the dependency between the nvv and the MSVC tool chain - this is something NVIDIA must get rid of... |
I don't think this is possible. NVCC uses a base compiler, e.g. gcc, llvm/clang or MSVC, to compile code (this is a very superifical explanation, but bear with me). I reckon there is not much they can do about it. |
Working features:
Untested features:
Broken features:
Regarding the missing library dir when linking in subprojects of some broken features (should be valid only for static builds, dynamic builds should be ok but I'd like a confirmation), when dependencies are found through the XXXXConfig.cmake script and not a FindXXXX.cmake one, the libraries are appended to the OpenCV dependencies without the full path and so any downstream project cannot properly link (without looking for that library too, in order to append the proper search path to the linker). I discussed the problem upstream with OpenCV devs and they told me it's a known issue with CMake. Now I am discussing the problem with CMake devs 😉
Regarding the debug/release mismatch (present also in some working features but in a non-drastic way), in my previous PR I tried to fix many of them with invasive modifications to the dependencies, implementing a proper suffix (
d
) for the libraries when built in debug mode, so that the two were distinguishable byCMake
. As of now, on the other hand, having the same name, when the library dependencies are collected and merged, only the debug ones survive and they trigger a link to the debug runtime also when used for release.I opened many merge requests on the official CMake repository in the meantime, with fixes and improvements for the various FindXXXX.cmake modules, to be ready to look for libraries with a
d
suffix (this should be the right approach, but unfortunately we live in a world which has to be retro compatible, so it will be a long and bumpy road before the final merge). In the meantime, I'd like also to reopen the discussion about jumping the gun here in vcpkg, add a suffix to libraries that still miss it and deploy a custom CMake module if necessary for each one. This would fix real world usage of those libraries, otherwise it is very difficult to use them correctly (the problem that emerges with OpenCV is common to any other project which has them as a dependency).As a side note, shall we reduce the base install to "no default features" (tested, it works)?