-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
[blaze][ceres][clapack][geogram][lapack][lapack-reference][opencv4][selene] Fix deps issues #13229
Conversation
e53a6c0
to
9297195
Compare
…issues As part of a full rebuild in microsoft#13019, several transitive rebuild issues were found. 1) selene was dynamically detecting the presence of OpenCV, which was not on its dependency list. 2) The dynamic OpenCV was using `find_package()` instead of `find_dependency()` 3) The dynamic OpenCV was always finding all dependencies instead of just those that were used during build. 4) CMake's FindHDF5 module requires C language support 5) ceres was passing `REQUIRED` into find_dependency (not needed -- find_dependency implies REQUIRED) 6) FindLAPACK.cmake was sometimes provided by the meta-port lapack, sometimes by the dependency lapack-reference/clapack 7) The lapack-reference vcpkg-cmake-wrapper was directly including the FindLAPACK, which doesn't respect variables such as QUIET and REQUIRED Interestingly, the particular issue required a cascade of failures to occur to actually cause a build failure; almost any of these fixes would independently fix the build.
9297195
to
22935b5
Compare
The same cleaner solution for find_dependencies can be applied to opencv2/3. I used find_package and QUIET to “hack” the system, but it is not as robust as this. |
On uwp, the dependencies are disabled: This per-platform logic is already implemented above in the |
Hi there @ras0219 :) zxing-cpp failed on x64-windows-static due to the default build-depends not having protobuf: BillyONeal@f9cc362 |
(I have those changes prepared in https://github.com/BillyONeal/vcpkg/commits/robert_review_fix if you want to use it as a base) |
@ras0219 and I talked about this stuff out of band. |
I'm looking into the dbow2 failure (looks like the transitive dependency through opencv isn't being handled appropriately) From dbow2:
|
…into dev/roschuma/lapack
…roschuma/lapack
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
…ich is not available in UWP.
…not supported there.
Submitted the Blaze patch upstream: https://bitbucket.org/blaze-lib/blaze/pull-requests/44/fix-inability-to-build-blaze-on-some |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that the library zxing-cpp
doesn't have protobuf
as a dependency, and opencv
installs protobuf
. Not sure why it was failing the CI.
@dan-shaw Good point. |
Thanks for your contribution! |
As part of a full rebuild in #13019, several transitive rebuild issues were found.
find_package()
instead offind_dependency()
REQUIRED
into find_dependency (not needed -- find_dependency implies REQUIRED)Interestingly, the particular issue required a cascade of failures to occur to actually cause a build failure; almost any of these fixes would independently fix the build.