Add installation of MuJoCo plugins as part of CMake installation logic #1515
Conversation
| @@ -79,6 +79,30 @@ else() | |||
| endif() | |||
|
|
|||
| option(MUJOCO_BUILD_MACOS_FRAMEWORKS "Build libraries as macOS Frameworks" OFF) | |||
| option(MUJOCO_INSTALL_PLUGINS "Install MuJoCo plugins" ON) | |||
There was a problem hiding this comment.
Do you mind making exactly the same changes to SimulateOptions.cmake?
SimulateOptions.cmake is used for SIMULATE_STANDALONE here:
mujoco/simulate/CMakeLists.txt
Line 60 in 9119239
and the two files should be kept in sync.
There was a problem hiding this comment.
Thanks, done and rebased.
Just an OT curiosity, if the two files must be kept in sync, there is any reason why we can't use the same MujocoOptions.cmake directly in mujoco/simulate/CMakeLists.txt, for example using include(../cmake/MujocoOptions.cmake) instead of include(SimulateOptions) ? Indeed inclusion of a file in a sibling directory is a bit ugly, but perhaps it would be easier to manage then two files that need to be manually kept in sync (unless you have something on the Google side to keep them in sync, in that case probably the problem is just for external contributors).
There was a problem hiding this comment.
Internally they are just one file that gets split in our CI so ya, this is a problem we save specifically for external contributors 🙂
There was a problem hiding this comment.
Annoyingly it looks like copybara still refuses to ingest this 😠
@nimrod-gileadi should I just copy-paste this into a CL?
Alternatively we could implement @traversaro's proposal to do the file duplication/referencing in cmake rather than copybara?
I'll approve yet again, maybe it'll work... 🤷♂️
traversaro
force-pushed
the
cmakeinstallplugin
branch
from
29c7256 to
603ae79
Compare
|
I'm back at work now. Will see if we can finally get this merged. |
Some users in my lab found confusing the fact that
simulatedid not found the plugins out of the box if mujoco was built from source, and apparently also other users had the same confusion (see for example #723 (comment)).This PR hopefully simplifies this by also installing as part of the CMake install step the mujoco plugins in
${CMAKE_INSTALL_PREFIX}/bin/mujoco_plugin, exactly wheresimulateexpects to find them, and where they are placed in the official release binaries.At the moment, I did not include this modifications for the case in which
MUJOCO_BUILD_MACOS_FRAMEWORKSisON, as it is not really clear to me how this should be handled. However, when building by sourceMUJOCO_BUILD_MACOS_FRAMEWORKSis set toOFF, so for all users that build from source, this PR ensure that the installedsimulatewill be able to find all the mujoco plugins out of the box.fyi @francesco-romano