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.

Sign up for GitHub

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

Add installation of MuJoCo plugins as part of CMake installation logic #1515

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add installation of MuJoCo plugins as part of CMake installation logic #1515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 1 addition & 16 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,21 +160,6 @@ jobs:
- name: Install MuJoCo
working-directory: build
run: cmake --install .
- name: Copy plugins (POSIX)
if: ${{ runner.os != 'Windows' }}
working-directory: build
run: mkdir -p ${{ matrix.tmpdir }}/mujoco_install/mujoco_plugin &&
cp lib/libactuator.* ${{ matrix.tmpdir }}/mujoco_install/mujoco_plugin &&
cp lib/libelasticity.* ${{ matrix.tmpdir }}/mujoco_install/mujoco_plugin &&
cp lib/libsensor.* ${{ matrix.tmpdir }}/mujoco_install/mujoco_plugin &&
cp lib/libsdf.* ${{ matrix.tmpdir }}/mujoco_install/mujoco_plugin
- name: Copy plugins (Windows)
if: ${{ runner.os == 'Windows' }}
working-directory: build
run: mkdir -p ${{ matrix.tmpdir }}/mujoco_install/mujoco_plugin &&
cp bin/Release/actuator.dll ${{ matrix.tmpdir }}/mujoco_install/mujoco_plugin &&
cp bin/Release/elasticity.dll ${{ matrix.tmpdir }}/mujoco_install/mujoco_plugin &&
cp bin/Release/sensor.dll ${{ matrix.tmpdir }}/mujoco_install/mujoco_plugin
- name: Configure samples
working-directory: sample
run: >
Expand Down Expand Up @@ -212,7 +197,7 @@ jobs:
run: >
source ${{ matrix.tmpdir }}/venv/bin/activate &&
MUJOCO_PATH="${{ matrix.tmpdir }}/mujoco_install"
MUJOCO_PLUGIN_PATH="${{ matrix.tmpdir }}/mujoco_install/mujoco_plugin"
MUJOCO_PLUGIN_PATH="${{ matrix.tmpdir }}/mujoco_install/bin/mujoco_plugin"
MUJOCO_CMAKE_ARGS="-DCMAKE_INTERPROCEDURAL_OPTIMIZATION:BOOL=OFF ${{ matrix.cmake_args }}"
pip wheel -v --no-deps mujoco-*.tar.gz
- name: Install Python bindings
Expand Down
24 changes: 24 additions & 0 deletions cmake/MujocoOptions.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

include(SimulateOptions)

and the two files should be kept in sync.

Copy link
Contributor Author

@traversaro traversaro Mar 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Internally they are just one file that gets split in our CI so ya, this is a problem we save specifically for external contributors 🙂

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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... 🤷‍♂️

mark_as_advanced(MUJOCO_INSTALL_PLUGINS)

# Define function to simplify installations of plugins
function(mujoco_install_plugin plugin_target_name)
# Do not install plugin on macOS when using macOS Frameworks
if(MUJOCO_INSTALL_PLUGINS AND NOT MUJOCO_BUILD_MACOS_FRAMEWORKS)
if(WIN32)
install(TARGETS ${plugin_target_name}
# consistently with the official mujoco binary format, we do
# not want to install .lib import libraries on Windows, and
# only install .dll . To do so, we install non-.dll files to
# a fake install directory that is not actually in the install path
ARCHIVE DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/fake_plugin_install"
LIBRARY DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/fake_plugin_install"
RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}/mujoco_plugin")
else()
install(TARGETS ${plugin_target_name}
ARCHIVE DESTINATION "${CMAKE_INSTALL_BINDIR}/mujoco_plugin"
LIBRARY DESTINATION "${CMAKE_INSTALL_BINDIR}/mujoco_plugin"
RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}/mujoco_plugin")
endif()
endif()
endfunction()

# Get some extra link options.
include(MujocoLinkOptions)
Expand Down
2 changes: 2 additions & 0 deletions plugin/actuator/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,5 @@ target_link_options(
${MUJOCO_MACOS_LINK_OPTIONS}
${EXTRA_LINK_OPTIONS}
)

mujoco_install_plugin(actuator)
2 changes: 2 additions & 0 deletions plugin/elasticity/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,5 @@ target_link_options(
${MUJOCO_MACOS_LINK_OPTIONS}
${EXTRA_LINK_OPTIONS}
)

mujoco_install_plugin(elasticity)
2 changes: 2 additions & 0 deletions plugin/sdf/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,5 @@ target_link_options(
${MUJOCO_MACOS_LINK_OPTIONS}
${EXTRA_LINK_OPTIONS}
)

mujoco_install_plugin(sdf)
2 changes: 2 additions & 0 deletions plugin/sensor/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,5 @@ target_link_options(
${MUJOCO_MACOS_LINK_OPTIONS}
${EXTRA_LINK_OPTIONS}
)

mujoco_install_plugin(sensor)
24 changes: 24 additions & 0 deletions simulate/cmake/SimulateOptions.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
mark_as_advanced(MUJOCO_INSTALL_PLUGINS)

# Define function to simplify installations of plugins
function(mujoco_install_plugin plugin_target_name)
# Do not install plugin on macOS when using macOS Frameworks
if(MUJOCO_INSTALL_PLUGINS AND NOT MUJOCO_BUILD_MACOS_FRAMEWORKS)
if(WIN32)
install(TARGETS ${plugin_target_name}
# consistently with the official mujoco binary format, we do
# not want to install .lib import libraries on Windows, and
# only install .dll . To do so, we install non-.dll files to
# a fake install directory that is not actually in the install path
ARCHIVE DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/fake_plugin_install"
LIBRARY DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/fake_plugin_install"
RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}/mujoco_plugin")
else()
install(TARGETS ${plugin_target_name}
ARCHIVE DESTINATION "${CMAKE_INSTALL_BINDIR}/mujoco_plugin"
LIBRARY DESTINATION "${CMAKE_INSTALL_BINDIR}/mujoco_plugin"
RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}/mujoco_plugin")
endif()
endif()
endfunction()

# Get some extra link options.
include(MujocoLinkOptions)
Expand Down
Loading