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

[WIP] Cleaned CMake files #35

Closed

Conversation

diegoferigo
Copy link
Member

CMake 3.12 still has some minor issues with the current master. The reason is that UseSwig.cmake has been recently refactored.

CMake versions >= 3.8 have new utilities for swig, and it makes sense to start adopting them. However, since we need to support CMake 3.5, the previous logic cannot be entirely removed.

This PR contains the following changes:

For the time being I tested only generating the bindings and compiling them with octave. I used CMake 3.12.0 and CMake 3.10.2.

TODO

  • Fix install path of additional .i files from yarp
  • Test matlab
  • Test older cmake version (anyone here?)

cc @traversaro @nunoguedelha

@diegoferigo diegoferigo requested a review from traversaro August 1, 2018 11:50
@diegoferigo diegoferigo self-assigned this Aug 1, 2018
@nunoguedelha
Copy link
Collaborator

Test older cmake version (anyone here?)

I have cmake 3.11.4, so I can't test right now an older version.

Uses yarp.i from the install directory

I can test this within the next couple of days..

@traversaro
Copy link
Member

I guess just supporting CMake 3.10 is now ok.

@diegoferigo
Copy link
Member Author

Accordingly to the tests I did last year when I opened this PR I think is ok, but we should test it again. Most recent CMake version improved the swig integration even further. Fur future reference in this CMakeLists.txt there is a python example with CMake >= 3.12.

@traversaro
Copy link
Member

Unless someone wants to revive this PR, I suggest to just close it. Even if we want to adopt it, probably it now just make sense to require CMake 3.12, and then directly change the cmake without all the if(${CMAKE_VERSION} VERSION_GREATER 3.8) clauses.

@diegoferigo
Copy link
Member Author

Adapting this PR to the new minimum supported CMake version would not be a big task. This PR if I recall simplifies quite a lot the CMake logic, if no one else can dedicate some time in the next few weeks we can close it and reopen it in the future if needed.

@traversaro
Copy link
Member

This PR if I recall simplifies quite a lot the CMake logic, if no one else can dedicate some time in the next few weeks we can close it and reopen it in the future if needed.

I do not see any volunteer on the horizon. : )
If someone wants to work on this, feel free to reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants