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

Require at least Boost 1.71, support Boost 1.86, switch to BoostConfig.cmake #6105

Merged
merged 2 commits into from
Aug 21, 2024

Conversation

mvieth
Copy link
Member

@mvieth mvieth commented Aug 15, 2024

Remove warning about old policy in CMake 3.30: https://cmake.org/cmake/help/latest/policy/CMP0167.html#policy:CMP0167
BoostConfig.cmake is available in Boost 1.70 and higher. With BoostConfig.cmake, it is no longer necessary to keep Boost_ADDITIONAL_VERSIONS updated.
Ubuntu 20.04 uses Boost 1.71.0 as the default version (Boost 1.67 is available but not the default).
See also: https://cmake.org/cmake/help/latest/module/FindBoost.html#module:FindBoost

@mvieth mvieth added changelog: enhancement Meta-information for changelog generation module: cmake labels Aug 15, 2024
larshg
larshg previously approved these changes Aug 15, 2024
@larshg
Copy link
Contributor

larshg commented Aug 16, 2024

We probably need to use a newer commit hash for vcpkg on x86 - the boost-cmake package quite new. microsoft/vcpkg@bcf3d00

@mvieth
Copy link
Member Author

mvieth commented Aug 16, 2024

Ah yes, you are right. Which one should we choose? The latest commit on the vcpkg master branch? Or the one where boost-cmake was just added?

@larshg
Copy link
Contributor

larshg commented Aug 16, 2024

Ah yes, you are right. Which one should we choose? The latest commit on the vcpkg master branch? Or the one where boost-cmake was just added?

Since x86 should use the oldest version possible, I think we should opt for the oldest tag, that the commit is within, ie https://github.com/microsoft/vcpkg/releases/tag/2024.05.24 which commit hash: 01f602195983451bc83e72f4214af2cbc495aa94

@@ -114,7 +114,7 @@ jobs:
PLATFORM: x86
TAG: windows2022-x86
GENERATOR: "'Visual Studio 16 2019' -A Win32"
VCPKGCOMMIT: 8eb57355a4ffb410a2e94c07b4dca2dffbee8e50
VCPKGCOMMIT: 01f602195983451bc83e72f4214af2cbc495aa94
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
VCPKGCOMMIT: 01f602195983451bc83e72f4214af2cbc495aa94
VCPKGCOMMIT: f7423ee180c4b7f40d43402c2feb3859161ef625

There was a problem with the hashes with the boost port, seems like we have to use release 2024.06.15 instead.

@larshg
Copy link
Contributor

larshg commented Aug 17, 2024

Do we need to split out the docker updates, as its only pushed when build on merge?

@mvieth
Copy link
Member Author

mvieth commented Aug 17, 2024

Do we need to split out the docker updates, as its only pushed when build on merge?

I can do that, yes. It would be the safer approach, otherwise we would have no guarantee that the Windows x86 CI turns green with the new Docker image.

set(Boost_ADDITIONAL_VERSIONS "@Boost_MAJOR_VERSION@.@Boost_MINOR_VERSION@.@Boost_SUBMINOR_VERSION@" "@Boost_MAJOR_VERSION@.@Boost_MINOR_VERSION@" ${Boost_ADDITIONAL_VERSIONS})

find_package(Boost 1.65.0 ${QUIET_} COMPONENTS @PCLCONFIG_AVAILABLE_BOOST_MODULES@)
find_package(Boost 1.71.0 ${QUIET_} COMPONENTS @PCLCONFIG_AVAILABLE_BOOST_MODULES@ CONFIG)
Copy link

Choose a reason for hiding this comment

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

Suggested change
find_package(Boost 1.71.0 ${QUIET_} COMPONENTS @PCLCONFIG_AVAILABLE_BOOST_MODULES@ CONFIG)
find_dependency(Boost 1.71.0 COMPONENTS @PCLCONFIG_AVAILABLE_BOOST_MODULES@ CONFIG)

Needs include(CMakeFindDependencyMacro) somewhere above.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, but that is a bit off-topic in this pull request, and I would like to merge this pull request as soon as possible because PCL's incompatibility with Boost 1.86 makes one of our CI checks fail. Perhaps we can look at find_dependency again in the future, in another pull request.

@mvieth mvieth merged commit 0c8bf25 into PointCloudLibrary:master Aug 21, 2024
14 checks passed
@mvieth mvieth deleted the boost_1_86 branch August 21, 2024 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: enhancement Meta-information for changelog generation module: cmake
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants