The patch applied to flann is renaming targets which affects downstream users. (E.g. the reason why I see a regression in https://dev.azure.com/vcpkg/public/_build/results?buildId=27030&view=logs&j=4c4e0d3e-e499-5796-93bc-a44aeda33b38)
Instead of doing e.g. this:
install (
- TARGETS flann flann_s
+ TARGETS flann
EXPORT ${targets_export_name}
replace it with
install (
- TARGETS flann flann_s
+ TARGETS ${FLANN_TARGET_EXPORTS}
EXPORT ${targets_export_name}
and set FLANN_TARGET_EXPORTS to the correct target.
Instead of doing that:
-add_definitions(-D_FLANN_VERSION=${FLANN_VERSION})
+add_definitions(-D_FLANN_VERSION=${FLANN_VERSION} -std=c++11)
use target_compile_features(flann_cpp(_s) PUBLIC cxx_std_11)
The same can be said about the cuda targets
Note that this would mean to have different target names depending on the static or dynamic build. Isn鈥檛 that a no-no on vcpkg, as I was trying to say in the thread before, when you asked about the reasons of the renaming?
OTOH I agree with the c++11 fix
@Neumann-A Can you describe it in more detail?
let me try to discuss what was my part in this. In flann I made a patch to remove flann_s (and every _s target) so that static and dynamic targets had the same name. That was because many times was said that vcpkg should abstract static and dynamic away from downstream users.
The fact is that this makes it more difficult for downstream projects which are ready to use different target names of dependencies for different build configurations: they break because they are not expecting the unique name but different ones, specific for each link type. It might be necessary to patch also these projects to accept the same target name independently from the build config
let me try to discuss what was my part in this. In flann I made a patch to remove flann_s (and every _s target) so that static and dynamic targets had the same name. That was because many times was said that vcpkg should abstract static and dynamic away from downstream users.
Instead of patching the difference away I would add a new ALIAS target which points to the correct static/shared target depending on build type
The fact is that this makes it more difficult for downstream projects which are ready to use different target names of dependencies for different build configurations: they break because they are not expecting the unique name but different ones, specific for each link type. It might be necessary to patch also these projects to accept the same target name independently from the build config
The problem was that the downstream project was doing it correctly given the right CMake options.
If an ALIAS target pointing to correct target is given downstream project don't need to differentiate between the different upstream targets. This is helpful for downstream projects not correctly differentiating between the different upstream targets.
In general VCPKG should not rename/change any upstream targets. It should be handled/treated in the same way library naming is handled within VCPKG
Most helpful comment
Instead of patching the difference away I would add a new ALIAS target which points to the correct static/shared target depending on build type
The problem was that the downstream project was doing it correctly given the right CMake options.
If an ALIAS target pointing to correct target is given downstream project don't need to differentiate between the different upstream targets. This is helpful for downstream projects not correctly differentiating between the different upstream targets.
In general VCPKG should not rename/change any upstream targets. It should be handled/treated in the same way library naming is handled within VCPKG