-
Notifications
You must be signed in to change notification settings - Fork 30
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
Do not modify CMAKE_FIND_LIBRARY_PREFIXES and CMAKE_FIND_LIBRARY_SUFFIXES on Windows #189
Conversation
…IXES on Windows `CMAKE_FIND_LIBRARY_PREFIXES` and `CMAKE_FIND_LIBRARY_SUFFIXES` are variables that are used by CMake to change the behaviour of the `find_library` call. Changing them to `.lib;.dll` in the CMake config file of every ignition project affect silently any downstream project that calls `find_package(ignition-<pkg>)`, even if transitevely, leading to subtle bugs. Signed-off-by: Silvio <[email protected]>
As long as I am aware of, no. I checked directly on CMake source code to be sure, and the only use (excluding
So I think we can safely exclude that in some way this change should affect the file name of the libraries.
Cool, that's a great way to test this. |
The test is failing but I guess it is related to gazebosim/gz-sim#990 ? As an alternative test, I also backported this change in conda-forge (conda-forge/libignition-cmake0-feedstock#20) to fix conda-forge/libignition-gazebo-feedstock#30, I can re-build a few ignition libraries and see if there are problems. |
This pull request has been mentioned on Gazebo Community. There might be relevant details there: https://community.gazebosim.org/t/new-ignition-releases-2022-01-10/1228/1 |
…IXES on Windows (gazebosim#189) `CMAKE_FIND_LIBRARY_PREFIXES` and `CMAKE_FIND_LIBRARY_SUFFIXES` are variables that are used by CMake to change the behaviour of the `find_library` call. Changing them to `.lib;.dll` in the CMake config file of every ignition project affect silently any downstream project that calls `find_package(ignition-<pkg>)`, even if transitevely, leading to subtle bugs. Signed-off-by: Silvio <[email protected]> Co-authored-by: Jose Luis Rivero <[email protected]>
🦟 Bug fix
Summary
CMAKE_FIND_LIBRARY_PREFIXES
andCMAKE_FIND_LIBRARY_SUFFIXES
are variables that are used byCMake to change the behaviour of the
find_library
calls. ChangingCMAKE_FIND_LIBRARY_SUFFIXES
to.lib;.dll
in the CMake config file of every ignition project affect silently any downstream project that callsfind_package(ignition-<pkg>)
, even if transitevely, leading to subtle bugs, such as the one investigate in conda-forge/libignition-gazebo-feedstock#30 (comment) .I tried to investigate why that code was there in the first place, and I tracked it back to c9beb6b#diff-ca85e9f4b24c567606682737f2466b715279730d7960ab5a5dd2a140f6ba2907R39 on the ign-cmake side, and back to gazebosim/gz-common@a0d286d#diff-c4216ffdb6192d9b0e30753bc4b385b7a38716c1dd6d73439e9b4f3c9b51fdddR37 on the ign-common side from which the ign-cmake code was copied. It seems that this was more a leftover rather then something that was actually fixing something, but I may be missing something fyi @nkoenig @mxgrey .
If a change in
CMAKE_FIND_LIBRARY_SUFFIXES
is actually needed, it should be done close to the actual affectedfind_library
call, and the previous value ofCMAKE_FIND_LIBRARY_SUFFIXES
should be restored as soon as possible.Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge