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

Fix Windows build when using vcpkg #3132

Merged
merged 4 commits into from
Dec 14, 2021
Merged

Fix Windows build when using vcpkg #3132

merged 4 commits into from
Dec 14, 2021

Conversation

Ace314159
Copy link
Contributor

Here are a list of changes I had to make to get Gazebo building on Windows when using vcpkg for dependencies:

  • Using TO_NATIVE_PATH for the default system paths for GAZEBO_PLUGIN_PATH, GAZEBO_MODEL_PATH, and GAZEBO_RESOURCE_PATH uses \ for the path separation characters without escaping and although escaping isn't required in CMake, it needs to be escaped in the C++ code. Therefore, I changed it to TO_CMAKE_PATH so the path separation character is always /.
  • When using vcpkg to build, Graphviz can be found with PkgConfig, so I allow Windows to also use that method if PkgConfig can be found.
  • /permissive- is needed to fix a dart error as described here
  • /Zc:strictStrings- is needed in order to assign a char* with a string in ode/misc.h
  • DIFFERENCE is a macro defined by a Windows header and needs to be undefed in order to be used in MeshCSG.hh

@scpeters
Copy link
Member

scpeters commented Dec 2, 2021

sorry I just introduced some conflicts by merging #3138. do you mind resolving them?

@scpeters
Copy link
Member

scpeters commented Dec 2, 2021

actually let me try

cmake/FindGraphviz.cmake Outdated Show resolved Hide resolved
@@ -181,7 +181,7 @@ macro (gz_setup_windows)
endif()

if (MSVC)
add_compile_options(/Zc:__cplusplus)
add_compile_options(/Zc:__cplusplus /permissive- /Zc:strictStrings-)
Copy link
Member

Choose a reason for hiding this comment

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

the windows CI build is broken; I'm guessing it's related to this change?

[ 43%] Building CXX object gazebo/transport/CMakeFiles/gazebo_transport.dir/IOManager.cc.obj
C:\Jenkins\workspace\gazebo-ci-pr_any-windows7-amd64\ws\install\include\boost/interprocess/detail/win32_api.hpp(892): error C2116: 'GetProcessTimes': function parameter lists do not match between declarations
C:\Program Files (x86)\Windows Kits\10\include\10.0.19041.0\um\processthreadsapi.h(106): note: see declaration of 'GetProcessTimes'
C:\Jenkins\workspace\gazebo-ci-pr_any-windows7-amd64\ws\install\include\boost/interprocess/detail/win32_api.hpp(892): error C2733: 'GetProcessTimes': you cannot overload a function with 'extern "C"' linkage
C:\Program Files (x86)\Windows Kits\10\include\10.0.19041.0\um\processthreadsapi.h(106): note: see declaration of 'GetProcessTimes'
C:\Jenkins\workspace\gazebo-ci-pr_any-windows7-amd64\ws\install\include\boost/interprocess/detail/win32_api.hpp(908): error C2371: 'GetFileType': redefinition; different basic types
C:\Program Files (x86)\Windows Kits\10\include\10.0.19041.0\um\fileapi.h(708): note: see declaration of 'GetFileType'
C:\Jenkins\workspace\gazebo-ci-pr_any-windows7-amd64\ws\install\include\boost/interprocess/detail/win32_api.hpp(908): error C2733: 'GetFileType': you cannot overload a function with 'extern "C"' linkage
...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pushed a change. Hopefully that should fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are test failures, but those seem to have been happening for a while. Both tests seem to fail because they are using command not available on Windows cmd (make and rm). Would it be worth modifying them or removing them?

@scpeters
Copy link
Member

looks ok to me. What do you think @traversaro ?

@traversaro
Copy link
Collaborator

looks ok to me. What do you think @traversaro ?

Ok for me!

@scpeters scpeters merged commit adb0834 into gazebosim:gazebo11 Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants