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

CMake: Dependency on Boost::filesystem is wrong #742

Open
Neumann-A opened this issue Mar 10, 2024 · 5 comments
Open

CMake: Dependency on Boost::filesystem is wrong #742

Neumann-A opened this issue Mar 10, 2024 · 5 comments
Assignees

Comments

@Neumann-A
Copy link

The CMake files for boost-gil declare a dependency on Boost::filesystem. However, that dependency is guarded by BOOST_GIL_IO_USE_BOOST_FILESYSTEM in the headers and there is no code in the CMake files which ever sets that preprocessor define. As such the dependency on Boost::filesystem is wrong or there needs to be a cmake option adding that dependency and the preprocessor define.

@sdebionne
Copy link
Contributor

Thank you for reporting this. Indeed we could add a CMake option (which default value would depend on the cpp standard) to remove a dependency that is not really needed in C++17 (unless BOOST_GIL_IO_USE_BOOST_FILESYSTEM is defined).

@simmplecoder simmplecoder self-assigned this Apr 2, 2024
@simmplecoder
Copy link
Contributor

@Neumann-A I will try to get a commit in this week, next week it should be implemented. Do you have some easy test case I could throw into the commit or should I try to write my own?

@Neumann-A
Copy link
Author

No I don't have a test case.

@simmplecoder
Copy link
Contributor

Sorry last days at work were very hectic. I have created a draft PR #743, I am a bit unsure how to create a test for it yet, but after I figure it out I will request a review. If you would like to test it out, you can pull my branch and set the following:

-DBOOST_GIL_USE_BOOST_FILESYSTEM=OFF
-DCMAKE_CXX_STANDARD=17 

C++ standard can be anything equal or higher to 17.

@mloskot
Copy link
Member

mloskot commented Apr 16, 2024

@simmplecoder I wouldn't worry about testing this with the already cases-overloaded CI. It would be great if one two people could try it out and confirm though.

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

No branches or pull requests

4 participants