Skip to content
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

Bump minimum required version of CMake to 3.16 and update CMake SWIG policies #732

Merged
merged 3 commits into from
Sep 9, 2020

Conversation

diegoferigo
Copy link
Member

@diegoferigo diegoferigo commented Sep 9, 2020

This PR fixes the error due to a bump of the minimum requirement of CMake that is being proposed in #731.

The problem is that recent CMake releases set by default the NEW version of policy CMP0078 that affects the naming of the target and the resulting library. This change breaks the old CMake structure.

In addition to this fix, I also applied the NEW version of policy CMP0086 that allows removing the generic CMAKE_SWIG_FLAGS replacing it with set_property calls.


P.S. I never used SWIG bindings in a multi-thread application and I'm not sure what's the best way to handle the SWIG flags. It seems that the -threads option that was already enabled is enough to disable the GIL, however as discussed in swig/swig#927 maybe it is not enough and other options should be added in the .i file. Let's keep things as they were before but also keep it in mind in case of multi-threading problems.

@traversaro
Copy link
Member

Let's keep things as they were before but also keep it in mind in case of multi-threading problems.

I totally agree!

@diegoferigo diegoferigo changed the title Update CMake SWIG policies Bump minimum required version of CMake to 3.16 and update CMake SWIG policies Sep 9, 2020
@diegoferigo
Copy link
Member Author

There are some errors occurring due to the old swig-related CMake policies. I'm not finding the right combination that works in CMake 3.13. Since these checks would have been removed anyway by #731, maybe it's better closing #731 and replacing it entirely with this PR.

@traversaro If after 0e8cdbe the tests pass, feel free to proceed in this way.

@traversaro
Copy link
Member

There are some errors occurring due to the old swig-related CMake policies. I'm not finding the right combination that works in CMake 3.13. Since these checks would have been removed anyway by #731, maybe it's better closing #731 and replacing it entirely with this PR.

@traversaro If after 0e8cdbe the tests pass, feel free to proceed in this way.

Thanks, I just updated the Changelog here as well.

@traversaro traversaro merged commit 166f8e2 into robotology:devel Sep 9, 2020
@diegoferigo diegoferigo deleted the fix/swig-cmake-policies branch September 9, 2020 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants