CMake: Removed files used by nothing in the build system #548
Conversation
Signed-off-by: Marcus Müller <marcus@hostalia.de>
marcusmueller
force-pushed
the
remove_unused_cmake_infra
branch
from
824fe94
to
27e5d5e
Compare
There was a problem hiding this comment.
LGTM. Thanks for this simplification!
At first, I thought about bumping the minimum Boost version. However, Boost is only required on old systems that lack compilers with std::filesystem support. A modern system comes with a modern compiler and doesn't require Boost at all.
There was a problem hiding this comment.
1 required change before I'll accept this. Also a meta-discussion, thinking we want to update minimum Boost to something reasonably recent, like even 1.60.0. When was Boost::filesystem added? At issue here is that by keeping min version at 1.35, we still need the Boost_NOGO_VERSIONS stuff from VolkBOOST.cmake. So, either we leave this all alone or we bump the Boost version so-as to not need the NOGO stuff.
| PUBLIC ${Boost_INCLUDE_DIRS} | ||
| ) | ||
| target_link_libraries(volk_profile PRIVATE ${Boost_LIBRARIES}) | ||
| target_link_libraries(volk_profile PRIVATE Boost::filesystem) |
There was a problem hiding this comment.
Still need to use the Boost_INCLUDE_DIRS for includes here
There was a problem hiding this comment.
target_link_libraries with a config target does add the includes to the target, or am I mistaken?
For example, gnuradio/gr-uhd/lib/CMakeLists.txt: https://github.com/gnuradio/gnuradio/blob/master/gr-uhd/lib/CMakeLists.txt#L34
We target_link_library the gnuradio-uhd target against the UHD::UDH target, which publicly says "hey, include this", which leads to the target automagically gaining these include dependencies.
|
Re: boost bumping: yes, I'd say we should do that. 1.35 is positively ancient, pulling up with GNU Radio makes sense to me – chances are that non-GR projects that use VOLK have at least the same Boost versions as GR 3.8 could rely on |
|
I'm OK to bump the minimum Boost version. I just don't see the need to do so. Say, we bump the minimum version to 1.65. In that case, we can assume a reasonable recent compiler to be available. This compiler will support We removed the last CI case for Linux, when we dropped tests for Ubuntu 14.04. Long story short. Maybe it's time to have another look at #174 and go through with it. |
|
Boost is purged from #557 ... so, closing this PR as it has been superseded! |
|
OK ... will be added ... soon. Closing this is safe :) |
No description provided.