[simbody] uniform target names and add usage #28863
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
MonicaLiu0311
added
category:port-feature
FabienPean
force-pushed
the
fix/simbody
branch
from
537b532 to
1773ade
Compare
FabienPean
force-pushed
the
fix/simbody
branch
from
1773ade to
bb9c45e
Compare
MonicaLiu0311
added
info:reviewed
| set(SHARED_TARGET ${SimTKSIMBODY_LIBRARY_NAME}) | ||
| -set(STATIC_TARGET ${SimTKSIMBODY_LIBRARY_NAME}_static) | ||
| +set(STATIC_TARGET ${SimTKSIMBODY_LIBRARY_NAME}) | ||
| set(SHARED_TARGET_VN ${SimTKSIMBODY_LIBRARY_NAME}${VN}) | ||
| -set(STATIC_TARGET_VN ${SimTKSIMBODY_LIBRARY_NAME}${VN}_static) | ||
| +set(STATIC_TARGET_VN ${SimTKSIMBODY_LIBRARY_NAME}${VN}) |
There was a problem hiding this comment.
Here and for SimTKcommon, SimTKmath:
The maintainer guidelines allow to rename binaries in order to have the same name for static vs. dynamic.
But IMO this doesn't extend to renaming exported CMake targets. The usage suggestion can use generator expressions, cf. zstd.
There was a problem hiding this comment.
Ok fine by me, but then does it even matter to rename the libraries if it is not related? The downstream user would just rely on the usage file no matter the library name. I would completely remove the patch to thin back the port in that case.
| Simbody provides CMake targets: | ||
|
|
||
| find_package(Simbody CONFIG REQUIRED) | ||
| target_link_libraries(main PRIVATE SimTKcommon SimTKmath SimTKsimbody) |
There was a problem hiding this comment.
I would guess that the actual usage only needs SimTKsimbody(-static), if properly exported.
There was a problem hiding this comment.
The targets are indeed incremental (SimTKcommon ⊂ SimTKmath ⊂ SimTKsimbody). Though each target is seldom usable and I have seen libraries relying on a subset specifically. Unless CMake can strip correctly simbody if someone use math only, then IMO usage file should reflect that.
Describe the pull request
What does your PR fix?
N.A.
Which triplets are supported/not supported? Have you updated the CI baseline?
Same triplets as before on vcpkg side (i.e. !uwp)
Does your PR follow the maintainer guide?
Yes, applies the following
If you have added/updated a port: Have you run
./vcpkg x-add-version --alland committed the result?Yes
If you are still working on the PR, open it as a Draft: https://github.blog/2019-02-14-introducing-draft-pull-requests/