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
Patch zstd 1.4.4 to include cmake_minimum_version bump to 2.8.12 #579
Conversation
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
… (#587) Signed-off-by: Emerson Knapp <eknapp@amazon.com> Co-authored-by: Emerson Knapp <537409+emersonknapp@users.noreply.github.com>
${extra_cmake_args}) | ||
${extra_cmake_args} | ||
# Note: zstd v1.4.6 will include the following fix. When that is released, upgrade and remove this patch. | ||
PATCH_COMMAND patch -p1 -d . < ${CMAKE_CURRENT_SOURCE_DIR}/cmake_minimum_required_2.8.12.patch) |
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.
should something like in https://github.com/ros2/rviz/blob/2206b9257716c05dc4d5eaa7ffae12e434f151ca/rviz_ogre_vendor/CMakeLists.txt#L157 be done here?
when setting up a windows machine for debugging an issue, I needed to install patch from choco but the places that were using find_package(Patch)
were working
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.
#632 is the planned solution, which should also work well without this dependency
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.
Sounds good, we will need something like that before the Galactic release as the current build from source instructions doesn't seem to be working on Windows
Fixes #573
This eliminates CMake warnings on the Windows build (see https://ci.ros2.org/job/ci_windows/13244/cmake/#issuesContent)
We can remove the patch when a new zstd version gets released