Unify the vcpkg package set across all gz libs. Remove all packages before every build. #907
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
j-rivero
force-pushed
the
jrivero/use_common_deps_vcpkg
branch
from
0809aaf
to
14640e0
Compare
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
j-rivero
force-pushed
the
jrivero/use_common_deps_vcpkg
branch
from
14640e0
to
f433a46
Compare
9 tasks
|
For reference, the package list obtained from current beefy: |
azeey
approved these changes
Apr 12, 2023
| @@ -4,10 +4,8 @@ set VCS_DIRECTORY=gz-common | |||
| set PLATFORM_TO_BUILD=x86_amd64 | |||
| set IGN_CLEAN_WORKSPACE=true | |||
|
|
|||
| set DEPEN_PKGS=assimp ffmpeg freeimage gts tinyxml2 | |||
| for /f %%i in ('python "%SCRIPT_DIR%\tools\detect_cmake_major_version.py" "%WORKSPACE%\%VCS_DIRECTORY%\CMakeLists.txt"') do set GZ_MAJOR_VERSION=%%i | |||
| if %GZ_MAJOR_VERSION% GEQ 5 ( | |||
There was a problem hiding this comment.
Can also remove the if block then.
There was a problem hiding this comment.
ouch, removed 05a2dcf
| @@ -5,7 +5,6 @@ set PLATFORM_TO_BUILD=x86_amd64 | |||
| set IGN_CLEAN_WORKSPACE=true | |||
|
|
|||
| :: tinyxml2 from msgs | |||
There was a problem hiding this comment.
ouch, removed 05a2dcf
| @@ -4,10 +4,8 @@ set VCS_DIRECTORY=gz-physics | |||
| set PLATFORM_TO_BUILD=x86_amd64 | |||
| set IGN_CLEAN_WORKSPACE=true | |||
|
|
|||
| set DEPEN_PKGS=eigen3 tinyxml2 | |||
| for /f %%i in ('python "%SCRIPT_DIR%\tools\detect_cmake_major_version.py" "%WORKSPACE%\%VCS_DIRECTORY%\CMakeLists.txt"') do set GZ_MAJOR_VERSION=%%i | |||
There was a problem hiding this comment.
Remove version checking logic since it's not needed anymore
| call %win_lib% :remove_vcpkg_installation || goto :error | ||
| echo # BEGIN SECTION: install all vcpkg dependencies | ||
| call %win_lib% :setup_vcpkg_all_dependencies || goto :error | ||
| echo # END SECTION |
There was a problem hiding this comment.
It would be nice (maybe a follow-up PR) to print the list of installed packages and their versions for debugging purposes.
There was a problem hiding this comment.
+100. I'll do it after merging this PR.
nuclearsandwich
approved these changes
Apr 12, 2023
| call %LIB_DIR%\windows_env_vars.bat || goto :error | ||
| if [%VCPKG_INSTALLED_FILES_DIR%]==[] ( | ||
| echo VCPKG_INSTALLED_FILES_DIR seems empty, refuse to delete | ||
| goto :error |
There was a problem hiding this comment.
Are there situations where we'd expect this directory to be empty, such as with a freshly provisioned host, and if so is the goto :error recoverable or would this halt the build?
| set VCPKG_CMD=%VCPKG_DIR%\vcpkg.exe | ||
| set VCPKG_CMAKE_TOOLCHAIN_FILE=%VCPKG_DIR%/scripts/buildsystems/vcpkg.cmake | ||
| if NOT DEFINED VCPKG_SNAPSHOT ( | ||
| :: see https://github.com/microsoft/vcpkg/releases | ||
| set VCPKG_SNAPSHOT=2022.02.23 | ||
| ) | ||
| :: Set of common gz dependencies expected up to Garden | ||
| set VCPKG_DEPENDENCIES_LEGACY=assimp boost-core bullet3 ccd console-bridge cppzmq cuda curl dlfcn-win32 eigen3 fcl ffmpeg freeimage gdal gflags glib gts jsoncpp libyaml libzip ogre ogre2 ogre22 openssl protobuf pybind11 qt5 qt5-winextras qwt sqlite3 tinyxml2 zeromq |
There was a problem hiding this comment.
If I understand correctly, the per-package dependencies have all been dropped in favor of this single list of dependencies. I think that's fine because the packaging for Windows is done using conda-forge and so there's limited utility in using the vcpkg installations to try and smoke out missing dependencies.
There was a problem hiding this comment.
If I understand correctly, the per-package dependencies have all been dropped in favor of this single list of dependencies. I think that's fine because the packaging for Windows is done using conda-forge and so there's limited utility in using the vcpkg installations to try and smoke out missing dependencies.
This is correct.
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
That should be a one time process of compiling + generating packages. I just relaunch
Compilation is starting on both after around 4 minutes of running. There is a bit of overhead but should be very few minutes (less than 5) not hours. Please let me know if you see more delays. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #904.
The current non-declared behavior in our Windows CI nodes is to never remove a vcpkg package from a CI system once it is installed by any job in Jenkins. Historical reasons for this are in #904. This effectively creates the same effect that having a unified set of packages available for all the builds.
This PR implements that reality by:
With this the PR can clean up all packages installed by vcpkg:
The clean up brings the bad side effect of the need to go through the installations of packages all the times for all the libs with all dependencies. This is done in a reasonable amount of time (see testing) since we are using an static snapshot of vcpkg packages and vcpkg has implemented the binary caching a while ago so all the runs does not need to compile anything new but install existing zips. To make the binary caching to work I needed to add a Temp path for this because the default for the LOCAL SYSTEM user by the Jenkins Server was not writable for it:
Testing
Searching for missing support in the configuration of the libraries: I've only found relevant the expected lack of Dart. There are other minor (platform related things) like the lack of swing, psutils or doxygen.
Second gz-sim7 build to see the cache in action
.
The simple gz-utils using the installation of all packages