-
Notifications
You must be signed in to change notification settings - Fork 417
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
Add namespace fcl to exported CMake targets #518
base: master
Are you sure you want to change the base?
Add namespace fcl to exported CMake targets #518
Conversation
As this is effectively a breaking change for downstream projects that link against the add_library(fcl ALIAS fcl::fcl)
if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.17)
set_property(TARGET fcl PROPERTY DEPRECATION "Deprecated target. Please use fcl::fcl instead.")
endif() |
I didn't want to include an alias without any warning, but thank you for pointing me to the new It doesn't work on an ALIAS target
but this would work (or using add_library(fcl INTERFACE IMPORTED)
set_target_properties(fcl PROPERTIES INTERFACE_LINK_LIBRARIES "fcl::fcl")
if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.17)
set_property(TARGET fcl PROPERTY DEPRECATION "Deprecated target. Please use fcl::fcl instead.")
endif() |
Great, thanks! I think fcl requires CMake 3.10, so using |
Great, I'll update the PR accordingly then. While fcl requires CMake 3.10, a consumer project of the config file can have a lower version. In this case, this would lead to the following error:
Changing this to use |
Thanks, indeed I was not thinking of that case. |
Probably for people that consume fcl via CMake's FetchContent module, it could also make sense to add a Line 49 in b2a749f add_library(${PROJECT_NAME}::${PROJECT_NAME} ALIAS ${PROJECT_NAME}) |
Good point, I've just added this to the PR. |
This patch adds the namespace
fcl::
to the exported CMake targetfcl
(resulting infcl::fcl
) as recommended by CMake for imported targets (see CMake documentation A Sample Find Module and CMP0028):This change is