cmake: Switch suitable PUBLIC target_link_libraries to PRIVATE
#6178
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ryanvolz
force-pushed
the
reduce_public_linking
branch
from
89b0689
to
bcaaaff
Compare
Declaring a library as PUBLIC in target_link_libraries tells downstream linkers of the target to also need to link to the PUBLIC libraries. This is necessary when the headers of those libraries are required by the public headers of the target library. There were some instances where linked libraries were declared PUBLIC but were not required to use the public headers, so they have been switched to PRIVATE declarations. This will allow OOT modules to reduce their linking and dependencies in some cases. Signed-off-by: Ryan Volz <ryan.volz@gmail.com>
mormj
force-pushed
the
reduce_public_linking
branch
from
bcaaaff
to
1c8b7d4
Compare
mormj
approved these changes
Nov 16, 2022
willcode
added
ported-to-3.10
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Declaring a library as
PUBLICintarget_link_librariestells downstream linkers of the target to also need to link to thePUBLIClibraries. This is necessary when the headers of those libraries are required by the public headers of the target library. There were some instances where linked libraries were declaredPUBLICbut were not required to use the public headers, so they have been switched toPRIVATEdeclarations. This will allow OOT modules to reduce their linking and dependencies in some cases.This could use some scrutiny from people more familiar with the various modules that are affected. I made these changes based purely on searching for every
target_link_librariescall and then searching the public headers to see if the linked libraries were#included somewhere. It's very possible I missed things.Boost::program_optionsused to be aPUBLIClink of gnuradio-runtime, but I think it makes more sense as something that is linked by individual executables that need it, so I moved that where appropriate.This also should conform to the new CMake formatting, which resulted in some more formatting changes in files that I guess were missed in that earlier PR.
Related Issue
Fixes #6109.
Which blocks/areas does this affect?
CMake in many modules.
Testing Done
None besides CI build testing. The real effects of this are downstream with building OOTs, so it would be good to try building ones that use affected modules. That might be easiest to do if this were merged just to main first to see if anyone notices problems.
Checklist