Export the ezc3d target. #116
Conversation
|
@aymanhab @carmichaelong @nickbianco I thought you might be curious about these CMake changes. |
|
Nice! Thanks about that! @chrisdembia |
|
@chrisdembia Thanks for doing this. Would it be possible to write a recipe for importing libraries into the OpenSim build/ecosystem? and to put that in on github or confluence? More libraries will need to be integrated moving forward (e.g. with moco) and having such a template would be extremely helpful and time saving. |
| set(${PROJECT_NAME}_LIB_FOLDER lib/${PROJECT_NAME}) | ||
| set(${PROJECT_NAME}_BIN_FOLDER lib/${PROJECT_NAME}) | ||
| set(${PROJECT_NAME}_INCLUDE_FOLDER include/${PROJECT_NAME}) |
There was a problem hiding this comment.
RPath is no longer set properly for the Python interface if ${CMAKE_INSTALL_PREFIX} is not present. On my computer I get
Set runtime path of "/home/pariterre/Programmation/miniconda3/envs/ezc3d-dev/lib/python3.8/site-packages/ezc3d/_ezc3d.so" to "lib/ezc3d"
which is not the right path (unless you change the LD_LIBRARY_PATH which I don't want to do).
Any suggestions?
There was a problem hiding this comment.
There are two possibilities for the RPATH for python bindings:
A. If the python bindings are expected to be used from their installed location, then you can use a relative RPATH, because you know the relative location of the ezc3d library.
B. If the python bindings are expected to be installed into your python distribution, then you need to use an absolute RPATH to the ezc3d library.
Which do you prefer? I think OpenSim has a CMake flag to toggle between A and B. We have spent a lot of time getting RPATH to work well for OpenSim, so we can refer to what OpenSim does.
There was a problem hiding this comment.
I think I prefer the case B since I install the python binding in the Python site-package folder.
Unless it makes it incompatible with OpenSim, lets keep it as B :)
There was a problem hiding this comment.
Okay I pushed a change that fixed the RPATH to be absolute again.
|
@pariterre I've pushed some fixes to the bindings. I have not yet fixed the RPATH, as this requires some discussion about the desired RPATH. |
| newParam.set( | ||
| std::vector<double>(data.begin(), data.end()), | ||
| dimension); |
There was a problem hiding this comment.
Thx! I did not update the Matlab interface yet with the new changes :)
|
Ready for review again. |
Codecov Report
@@ Coverage Diff @@
## dev #116 +/- ##
=======================================
Coverage 97.78% 97.78%
=======================================
Files 31 31
Lines 3703 3703
=======================================
Hits 3621 3621
Misses 82 82
Continue to review full report at Codecov.
|
|
Yay! Thank you for the quick turnaround. Let me know if there are other problems I caused by accident. |
This PR supports PR opensim-org/opensim-core#2695 by creating an
ezc3dTargets.cmakefile in the ezc3d installation.Without this PR,
ezc3dConfig.cmake.inhas some remnants of what goes in aFindezc3d.cmakefile. With a Config file, there is no need to "find" the library; a Config file knows where it is because it's part of the library's installation.I also made the ezc3d installation relocatable by using the PATH_VARS argument to
configure_package_config_file().