-
Notifications
You must be signed in to change notification settings - Fork 285
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
Fix MSVC linking for assimp and fcl #1510
Conversation
cc @scpeters |
@jslee02 I don't know if you prefer this PR to target |
It seems that the vcpkg version on AppVeyor is quite old and does not contain the fcl 0.6 that was updated in March 2020: microsoft/vcpkg#10025 . I can add a fallback to the old logic, but I am afraid it would be broken that as well. |
I just tried with the latest vcpkg and the configuration with fcl and assimp enabled works fine. @jslee02 do you think we can tag a new release in https://github.com/dartsim/vcpkg-build to get a new version of the dependencies? |
I tested this with |
Please ignore the AppVeyor build for now. It needs to use
A new version is published for Please change the below line to dart/.github/workflows/ccpp.yml Line 186 in 185fbc2
Let me know if |
this was fixed by microsoft/vcpkg#14061 |
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.
this works for ignition-physics3
Unfortunately there were a few bugs in vcpkg ports that were fixed only recently (latest PR: microsoft/vcpkg#14061), so probably generating a new archive against a new commit of vcpkg may be the simpler solution, and then switch to a new release of vcpkg once they do one (unfortunately vcpkg release are not really regular). In robotology/robotology-superbuild-dependencies-vcpkg#32 I am using microsoft/vcpkg@70f192e and it seems to be a good commit without anything strange. |
New build for microsoft/vcpkg@70f192e is released. You could use |
Great! |
Codecov Report
@@ Coverage Diff @@
## master #1510 +/- ##
==========================================
- Coverage 58.20% 58.17% -0.04%
==========================================
Files 412 414 +2
Lines 29913 30008 +95
==========================================
+ Hits 17412 17456 +44
- Misses 12501 12552 +51
|
This PR fixes the linking of tests that use fcl and assimp on MSVC, hopefully fixing the problems reported in gazebosim/gz-physics#87 (comment) .
As the existing code is working well in CI for non-MSVC platforms, and as support for CMake imported targets can vary depending on the platform (for example,
assimp::assimp
is basically broken in Ubuntu 20.04 apt packages: https://bugs.launchpad.net/ubuntu/+source/assimp/+bug/1882427 and robotology/idyntree#693), to minimize the possible regressions I just used the imported targets for fcl and assimp when using MSVC.