-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
[WIP] [gazebo10] Add new port #8178
Conversation
I recommend using draft PRs for WIP https://github.blog/2019-02-14-introducing-draft-pull-requests/. It helps up know when you feel it is ready for checkin. |
Thanks, I actually wanted to run some CI on some early version of the port to collect preliminary info, and I was not sure if CI was active for Draft PRs. |
Yes, that would be the correct strategy. We're not in the business of shipping applications with vcpkg so while we allow ports to build executable files, we also require those executables to be moved to a
You're correct, that doesn't work on non-Windows triplets. The correct way to handle is to move the binaries by name, for example: set(EXECUTABLE_SUFFIX)
if (VCPKG_TARGET_IS_WINDOWS)
set(EXECUTABLE_SUFFIX ".exe")
endif()
if(NOT DEFINED VCPKG_BUILD_TYPE OR VCPKG_BUILD_TYPE STREQUAL "debug")
file(COPY "program1${EXECUTABLE_SUFFIX}" DESTINATION "tools/gazebo/debug")
# other output files to be moved
... |
/azp run |
Thanks for the reply. Unfortunately the port is still incomplete, as the required amount of patches turned out to be non-negligible, I finally fixed compilation (almost all the changes has been submitted upstream: https://bitbucket.org/osrf/gazebo/pull-requests/) but several post-build vcpkg checks are still failing. |
0914f4f
to
b132fee
Compare
As of b132fee, the port is ready and compiles successfully (at least on my machine). Proper generation of Release and Debug
I am not sure when I will be able to debug this. I remember experiencing this problem also in the past, @seanyen perhaps do you have some hint on what could be the problem given your experience working with Gazebo on Windows? |
@traversaro Thank you for putting this together, I've had such a headache trying to build Gazebo for Windows on my own. I've managed to get both gzclient and gzserver running on my machine with the following "setup.bat" script run beforehand: @REM Add paths to Windows. Run this script prior to running gzserver.exe
@REM and gzclient.ext on a Windows machine.
@REM Meant to be run from inside {installdir}
@set build_type=Release
@if not "%1"=="" set build_type=%1
@echo Configuring for build type %build_type%
@REM Need absolute paths in order to run anything inside install dir
@set gz_root_path=%~dp0
@set gz_build_path=%gz_root_path%tools\gazebo10\%build_type%
@set gz_resource_path=%gz_root_path%share\gazebo-10
@pushd %~dp0
@set deps_path=%CD%\bin
@popd
@echo - script path is %gz_root_path%
@echo - build path is %gz_build_path%
@echo - resource path is %gz_resource_path%
@echo - dependencies path is %deps_path%
@set HOME=%HOMEDRIVE%%HOMEPATH%
@set GAZEBO_MODEL_PATH=%gz_resource_path%models
@set GAZEBO_PLUGINS_PATH=%gz_root_path%bin\gazebo-10\plugins
@set GAZEBO_RESOURCE_PATH=%gz_resource_path%;%gz_root_path%Media
@set OGRE_RESOURCE_PATH=%gz_root_path%bin
@set PATH=%gz_build_path%;%PATH% There appears to still be some problems with certain resources - particularly shaders with Thank you for the work so far though! |
That is great, thanks a lot for working on this. Eventually, can I take some parts of your
|
Have you tried to run your script for launching Gazebo in Debug mode? I think that in that case, also |
Yes, feel free to use parts of the setup.bat in the port. I had derived it from win_addpath.bat I added %gz_root_path%Media to the resource path but I wasn't sure if it was needed or not, it didn't seem to make any visible difference. I also haven't tried the debug build, I agree it probably won't work as is. |
Preliminary version, not ready for merge.
91a661d
to
e7836e5
Compare
@stephenjust I saw a few related PR in Gazebo (https://bitbucket.org/osrf/gazebo/pull-requests/3150/fix-texture-loading-on-ogre-111-112/diff and https://bitbucket.org/osrf/gazebo/pull-requests/3151), do you think it make sense to add them as patches to the port? |
/azp run |
Ogre::String programName = this->baseName + "VP_" + | ||
Ogre::StringConverter::toString(_permutation); | ||
|
||
-#if OGRE_DEBUG_MODE |
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.
Why not force OGRE_DEBUG_MODE
to 0
in CMakeLists.txt
?
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.
Because OGRE_DEBUG_MODE
is already defned to 1
in OGRE headers if OGRE is compiled in Debug, so forcing it to 0
could create problems in the compilation of OGRE headers. See the related upstream PR : https://bitbucket.org/osrf/gazebo/pull-requests/3131 .
file(INSTALL ${SOURCE_PATH}/LICENSE DESTINATION ${CURRENT_PACKAGES_DIR}/share/gazebo10 RENAME copyright) | ||
|
||
# Post-build test for cmake libraries | ||
vcpkg_test_cmake(PACKAGE_NAME gazebo) |
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.
vcpkg_test_cmake
is deprecated, please remove this line.
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.
Ack.
|
||
# Remove debug files | ||
file(REMOVE_RECURSE ${CURRENT_PACKAGES_DIR}/debug/include | ||
${CURRENT_PACKAGES_DIR}/debug/lib/cmake |
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.
${CURRENT_PACKAGES_DIR}/debug/lib/cmake should be deleted by vcpkg_fixup_cmake_targets
automatically.
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.
Good catch, thanks!
set(EXECUTABLE_SUFFIX ".exe") | ||
endif() | ||
|
||
if(NOT DEFINED VCPKG_BUILD_TYPE OR VCPKG_BUILD_TYPE STREQUAL "release") |
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.
I prefer to use patch to fix the installation path with tools.
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.
Just to understand, what is the rational to prefer a patch for this change that will never be upstreamed, as the installation of executables in tools/<port>
is a vcpkg peculiarity?
Do you have a good example port that can be used as an example for a patch that fixes tool installation?
The current strategy was proposed by @vicroms in #8178 (comment) .
This PR contains too many patches, maybe we can wait for upstream to accept them and use the new version? |
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.
Some replies.
I think this is indeed a good idea, as Gazebo 11 that should contain all the proposed patch should be released at the end of January, and then we can add a gazebo11 port. Furthermore, as mentioned in #8178 (comment) I am not able to use the existing port, so we would need at least someone that is able to correctly use the port to actually merge it. |
For future reference, I had this problem also with Gazebo compiled outside of vcpkg because I had a rather old Windows 10 version (1709). By updating to Windows 10 1903, the problem was fixed. |
Add new port for the Gazebo simulator and its libraries.
Fix #8014 .
Currently a WIP, as it is needs #8177 and #8136 to be merged before.
Furthermore, at the moment the part of handling executables is a draft, as I am not sure how about the following points:
gazebo
and related executables is a tool for C++ projects in the sense that is a launcher for simulations that contain C++ plugins developed against the Gazebo C++ API . For this reason, on Windows to launch Gazebo plugins compiled in Debug, you need to have the Gazebo executables compiled in Debug, and for plugins compiled in Release, you need the Release executables. As far as I saw, all the ports that install tools only install the release version, as they are typically tools only used for code generation or similar build duties.What is the correct strategy to adopt in this case? Install the tools under
tools/gazebo10/debug
?Note that there is an ongoing discussion on this in Is there a good reason why the vcpkg cmake toolchain is not adding (/debug)/bin to PATH? #7826, if the tools could be installed in
bin
and indebug/bin
there would be no problem at all. Perhaps @seanyen you had already some plan on this for using vcpkg for generating Chocolatey packages for ROS?tools
/executables
installation meant to be handled onx64-osx
andx64-linux
triplets? Some ports I saw identify executables by grepping all the files that end in.exe
in thebin
directory (see for examplevcpkg/ports/pcl/portfile.cmake
Line 62 in ecfc714