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

Update cmake policy #2102

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jongbongan
Copy link
Contributor

CMake prefers not to support FindBoost if its version is greater or equal to 3.30. Therefore, config-based find_package is preferred.

(https://cmake.org/cmake/help/latest/policy/CMP0167.html)

It will resolve the issue #2088

@coveralls
Copy link

coveralls commented Oct 21, 2024

Coverage Status

coverage: 72.784%. remained the same
when pulling a0e9ba4 on jongbongan:update_cmake_policy
into 5ee8f89 on lballabio:master.

@@ -90,6 +90,7 @@ jobs:
- name: Compile
env:
BOOST_ROOT: C:\local\boost
CMAKE_PREFIX_PATH: C:\local\boost\lib64-msvc-14.3\cmake\Boost-1.85.0
Copy link
Owner

Choose a reason for hiding this comment

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

It seems we're going from having to tell CMake fewer information to having to tell it more. How comes we have to be so specific? Shouldn't Boost install files where CMake can find them without having to specify the exact directory in the depths of the Boost folder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, the purpose of the added code is to inform CMake of the location of BoostConfig.cmake, since CMake couldn’t locate it on its own. Do you have any suggestions for improving this workaround?

Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure. I would have thought that the Boost installer took care of putting BoostConfig.cmake somewhere where CMake can find it. If that's not the case, maybe we could do that ourselves? In any case, I'm kind of worried that people now would have to take new explicit steps when they upgrade Boost...

Copy link
Contributor Author

@jongbongan jongbongan Oct 21, 2024

Choose a reason for hiding this comment

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

I agree. The problem occurs when the operating system is Windows.(installing boost w/o pkg manager) Locally, I ran some tests with the raw CMakeLists.txt, but I couldn’t configure the project because, once again, CMake couldn’t find the correct location for BoostConfig.cmake. So, I temporarily added the absolute path in CMakeLists.txt (set(CMAKE_PREFIX_PATH "absolute path")), which seems like a poor solution to this issue.

Copy link
Owner

Choose a reason for hiding this comment

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

It might be a good question to ask on the Boost mailing list, what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I don’t have any experience with contacting the Boost mailing list. I don’t think my solution is the best way to handle this issue, so I’ll look into the problem more carefully. I’ll also find out how to contact the Boost mailing list.

Copy link
Owner

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps, it improves the understanding if you set the environment variable Boost_DIR instead of CMAKE_PREFIX_PATH as it is a Boost specific hint where to search for BoostConfig.cmake.

It seems we're going from having to tell CMake fewer information to having to tell it more.

BOOST_ROOT becomes obsolete with the use of Boost_DIR (or CMAKE_PREFIX_PATH), we can remove it. So, at least we would be even regarding the amount of information we have to provide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps, it improves the understanding if you set the environment variable Boost_DIR instead of CMAKE_PREFIX_PATH as it is a Boost specific hint where to search for BoostConfig.cmake.

It seems we're going from having to tell CMake fewer information to having to tell it more.

BOOST_ROOT becomes obsolete with the use of Boost_DIR (or CMAKE_PREFIX_PATH), we can remove it. So, at least we would be even regarding the amount of information we have to provide.

Thanks. I will try with applying this comment.

@CLAassistant
Copy link

CLAassistant commented Oct 24, 2024

CLA assistant check
All committers have signed the CLA.

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.

5 participants