From 68159a77525d3c78825e88c4fb256f130b7fb6fd Mon Sep 17 00:00:00 2001 From: Jose Luis Rivero Date: Tue, 27 Aug 2024 21:32:34 +0200 Subject: [PATCH 1/4] Fix symbol checking test when compiled with debug symbols (#1474) * Fix symbol checking test The test that checks for prefixed binary symbols was broken when compiled with DebWithRelInfo since it was checking debugging symbols that broke the heuristics used. The commit fixes it doing a couple of actions: - Include the length of the namespace sdf: 3sdf - Check only dynamic symbols being exported Signed-off-by: Jose Luis Rivero * Update test/integration/all_symbols_have_version.bash.in Co-authored-by: Steve Peters Signed-off-by: Jose Luis Rivero --------- Signed-off-by: Jose Luis Rivero Co-authored-by: Steve Peters (cherry picked from commit 904706c2c02c5c333e916717e0a4ea6b3c74947e) --- test/integration/all_symbols_have_version.bash.in | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/integration/all_symbols_have_version.bash.in b/test/integration/all_symbols_have_version.bash.in index d69a58442..94912d34d 100644 --- a/test/integration/all_symbols_have_version.bash.in +++ b/test/integration/all_symbols_have_version.bash.in @@ -4,7 +4,13 @@ LIBPATH=$1 VERSIONED_NS=v@PROJECT_VERSION_MAJOR@ # Sanity check - there should be at least one symbol -NUM_SYMBOLS=$(nm $LIBPATH | grep -e "sdf" | wc -l) + +# nm options: +# -D to get only dynamic symbols exported +# 3 before the sdf is used by +# mangled symbols in C++ to check for the +# sdf namespace +NUM_SYMBOLS=$(nm -D $LIBPATH | grep -e "3sdf" | wc -l) if [ $NUM_SYMBOLS -eq 0 ] then @@ -13,7 +19,7 @@ then fi # There must be no unversioned symbols -UNVERSIONED_SYMBOLS=$(nm $LIBPATH | grep -e "sdf" | grep -e "$VERSIONED_NS" -v) +UNVERSIONED_SYMBOLS=$(nm -D $LIBPATH | grep -e "3sdf" | grep -e "$VERSIONED_NS" -v) UNVERSIONED_SYMBOL_CHARS=$(printf "$UNVERSIONED_SYMBOLS" | wc -m) if [ $UNVERSIONED_SYMBOL_CHARS -ne 0 ] From dcd3cd61402a2278db3ac021442a6e7458fa79b9 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 5 Nov 2024 18:48:43 -0800 Subject: [PATCH 2/4] Improve installation instructions (#1496) Backport of #1490. * Refer to https://brew.sh instead of duplicating the brew installation command. * List cmake variables in a markdown table. * Combine Ubuntu and macOS installation instructions (cherry picked from commit 22684cbe9144f9cf15e2df7dfa55457266caca44) Signed-off-by: Steve Peters Co-authored-by: Steve Peters --- README.md | 71 ++++++++++++++++++++----------------------------------- 1 file changed, 25 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index 4a8a8cf60..4c7d10c9d 100644 --- a/README.md +++ b/README.md @@ -71,9 +71,9 @@ which version you need, or leave it empty for version 1. ### macOS -On macOS, add OSRF packages: +On macOS, after installing the [Homebrew package manager](https://brew.sh), +add OSRF packages: ```sh - /bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)" brew tap osrf/simulation ``` @@ -129,6 +129,25 @@ git clone https://github.com/gazebosim/sdformat -b sdf<#> Be sure to replace `<#>` with a number value, such as 1 or 2, depending on which version you need. +### Install dependencies + +#### Ubuntu + +```sh +cd sdformat +sudo apt -y install \ + $(sort -u $(find . -iname 'packages-'`lsb_release -cs`'.apt' -o -iname 'packages.apt' | tr '\n' ' ')) +``` + +#### macOS + +```sh +brew install --only-dependencies sdformat<#> +``` + +Be sure to replace `<#>` with a number value, such as 14 or 15, depending on +which version you need. + ### Build from Source Standard installation can be performed in UNIX systems using the following @@ -144,12 +163,10 @@ make install sdformat supported cmake parameters at configuring time: -* `USE_INTERNAL_URDF` (`bool`) [default `False`]
- Use an internal copy of urdfdom 1.0.0 instead of look for one - installed in the system -* `USE_UPSTREAM_CFLAGS` (`bool`) [default `True`]
- Use the sdformat team compilation flags instead of the common set defined - by cmake. +| Name | Type | Default | Description | +|-----------------------|------|----------|--------------------------------------| +| `USE_INTERNAL_URDF` | BOOL | False | Use an internal copy of urdfdom 1.0.0 instead of looking for one installed in the system | +| `USE_UPSTREAM_CFLAGS` | BOOL | True | Use the sdformat team compilation flags instead of the common set defined by cmake. | ## Uninstallation @@ -160,44 +177,6 @@ cd build make uninstall ``` -## macOS - -### Prerequisites - -Clone the repository -```sh -git clone https://github.com/gazebosim/sdformat -b sdf<#> -``` -Be sure to replace `<#>` with a number value, such as 1 or 2, depending on -which version you need. - -Install dependencies -```sh -brew install --only-dependencies sdformat<#> -``` - -### Build from Source - -1. Configure and build - ```sh - cd sdformat - mkdir build - cd build - cmake .. # Consider specifying -DCMAKE_INSTALL_PREFIX=... - make - ``` - -2. Optionally, install and uninstall - ```sh - sudo make install - ``` - - To uninstall the software installed with the previous steps: - ```sh - cd build/ - sudo make uninstall - ``` - ## Windows ### Prerequisites From 91d9029b867a35349b3be85dbe9333feeb2b503c Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 7 Nov 2024 11:16:38 -0800 Subject: [PATCH 3/4] Permit building python bindings separately from libsdformat library (#1497) Backport of #1491 with adapted package finding logic and a note about requiring cmake 3.22.1. This allows the src/python_pybind11/CMakeLists.txt file to be built as a top-level cmake project against an external sdformat library, with documentation added to the README. The logic for finding pybind11 is also moved from the root CMakeLists.txt to python/CMakeLists.txt to reduce code duplication. When invoked through the root CMakeLists.txt, pybind11 is treated as an optional dependency, but when invoked from the python folder, pybind11 is treated as required by setting the variable CMAKE_REQUIRE_FIND_PACKAGE_pybind11 to TRUE. Signed-off-by: Steve Peters Signed-off-by: Silvio Traversaro Co-authored-by: Silvio Traversaro (cherry picked from commit 3dcdd55ee7a3ab0ac77b5cce56ba9629b79a70ac) --- CMakeLists.txt | 12 +----------- README.md | 25 +++++++++++++++++++++++++ python/CMakeLists.txt | 39 +++++++++++++++++++++++++++++++-------- 3 files changed, 57 insertions(+), 19 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b81c2acf0..6b0dd2f12 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -97,16 +97,6 @@ if (BUILD_SDF) ) if (NOT Python3_Development_FOUND) GZ_BUILD_WARNING("Python development libraries are missing: Python interfaces are disabled.") - else() - set(PYBIND11_PYTHON_VERSION 3) - find_package(pybind11 2.4 CONFIG QUIET) - - if (pybind11_FOUND) - message (STATUS "Searching for pybind11 - found version ${pybind11_VERSION}.") - else() - GZ_BUILD_WARNING("pybind11 is missing: Python interfaces are disabled.") - message (STATUS "Searching for pybind11 - not found.") - endif() endif() endif() @@ -159,7 +149,7 @@ if (BUILD_SDF) add_subdirectory(sdf) add_subdirectory(conf) add_subdirectory(doc) - if (pybind11_FOUND AND NOT SKIP_PYBIND11) + if (Python3_Development_FOUND AND NOT SKIP_PYBIND11) add_subdirectory(python) endif() endif(BUILD_SDF) diff --git a/README.md b/README.md index 4c7d10c9d..bab39d1f3 100644 --- a/README.md +++ b/README.md @@ -165,9 +165,34 @@ sdformat supported cmake parameters at configuring time: | Name | Type | Default | Description | |-----------------------|------|----------|--------------------------------------| +| `SKIP_PYBIND11` | BOOL | False | Skip generating Python bindings via pybind11 | | `USE_INTERNAL_URDF` | BOOL | False | Use an internal copy of urdfdom 1.0.0 instead of looking for one installed in the system | | `USE_UPSTREAM_CFLAGS` | BOOL | True | Use the sdformat team compilation flags instead of the common set defined by cmake. | +### Build python bindings separately from main library + +If you want to build Python bindings separately from the main libsdformat library +(for example if you want to build Python bindings for multiple versions of Python), +you can invoke cmake on the `python` folder instead of the root folder. +This requires cmake version 3.22.1 due to the use of a `CMAKE_REQUIRE_FIND_PACKAGE_*` +variable, which is newer than the minimum required version of cmake for sdformat14. +Specify the path to the python executable with which you wish to build bindings +in the `Python3_EXECUTABLE` cmake variable. +Specify the install path for the bindings in the `CMAKE_INSTALL_PREFIX` +variable, and be sure to set your `PYTHONPATH` accordingly after install. + +```bash +cd sdformat +mkdir build_python3 +cd build_python3 +cmake ../python \ + -DPython3_EXECUTABLE=/usr/local/bin/python3.12 \ + -DCMAKE_INSTALL_PREFIX= +``` + +See the homebrew [sdformat15 formula](https://github.com/osrf/homebrew-simulation/blob/027d06f5be49da1e40d01180aedae7f76dc7ff47/Formula/sdformat15.rb#L12-L56) +for an example of building bindings for multiple versions of Python. + ## Uninstallation To uninstall the software installed with the previous steps: diff --git a/python/CMakeLists.txt b/python/CMakeLists.txt index 864d38c18..41b511cc8 100644 --- a/python/CMakeLists.txt +++ b/python/CMakeLists.txt @@ -1,15 +1,39 @@ -if(WIN32 AND CMAKE_BUILD_TYPE STREQUAL "Debug") - # pybind11 logic for setting up a debug build when both a debug and release - # python interpreter are present in the system seems to be pretty much broken. - # This works around the issue. - set(PYTHON_LIBRARIES "${PYTHON_DEBUG_LIBRARIES}") +# Detect if we are doing a standalone build of the bindings, using an external sdformat +if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR) + cmake_minimum_required(VERSION 3.22.1) + set(SDF_VER 14) + project(sdformat${SDF_VER}-python VERSION ${SDF_VER}) + find_package(sdformat${SDF_VER} REQUIRED) + set(PROJECT_LIBRARY_TARGET_NAME "sdformat${PROJECT_VERSION_MAJOR}::sdformat${PROJECT_VERSION_MAJOR}") + # require python dependencies to be found + find_package(Python3 COMPONENTS Interpreter Development REQUIRED) + set(CMAKE_REQUIRE_FIND_PACKAGE_pybind11 TRUE) + include(GNUInstallDirs) + include(CTest) +elseif(${CMAKE_VERSION} VERSION_LESS "3.12.0") + # TODO: remove once the minimum CMake version is increased + if(WIN32 AND CMAKE_BUILD_TYPE STREQUAL "Debug") + # pybind11 logic for setting up a debug build when both a debug and release + # python interpreter are present in the system seems to be pretty much broken. + # This works around the issue. + set(PYTHON_LIBRARIES "${PYTHON_DEBUG_LIBRARIES}") + endif() endif() +set(PYBIND11_PYTHON_VERSION 3) +find_package(pybind11 2.4 CONFIG QUIET) + +if (pybind11_FOUND) + message (STATUS "Searching for pybind11 - found version ${pybind11_VERSION}.") +else() + message(WARNING "pybind11 is missing: Python interfaces are disabled.") + return() +endif() if(USE_SYSTEM_PATHS_FOR_PYTHON_INSTALLATION) if(${CMAKE_VERSION} VERSION_LESS "3.12.0") execute_process( - COMMAND "${PYTHON_EXECUTABLE}" -c "if True: + COMMAND "${Python3_EXECUTABLE}" -c "if True: from distutils import sysconfig as sc print(sc.get_python_lib(plat_specific=True))" OUTPUT_VARIABLE Python3_SITEARCH @@ -28,7 +52,7 @@ if(USE_SYSTEM_PATHS_FOR_PYTHON_INSTALLATION) endif() else() # If not a system installation, respect local paths - set(GZ_PYTHON_INSTALL_PATH ${GZ_LIB_INSTALL_DIR}/python) + set(GZ_PYTHON_INSTALL_PATH ${CMAKE_INSTALL_LIBDIR}/python) endif() # Set the build location and install location for a CPython extension @@ -124,7 +148,6 @@ if (BUILD_TESTING AND NOT WIN32) target_link_libraries(sdformattest PRIVATE ${PROJECT_LIBRARY_TARGET_NAME} - gz-utils${GZ_UTILS_VER}::gz-utils${GZ_UTILS_VER} ) set(python_tests From d0bcd405516678cb7840b245b9c62adfbaa083c9 Mon Sep 17 00:00:00 2001 From: Nate Koenig Date: Tue, 12 Nov 2024 10:07:00 -0800 Subject: [PATCH 4/4] Support removing the actor, light, or model from the root (#1492) * Support removing the actor, light, or model from the root Signed-off-by: Nate Koenig * Update src/Root_TEST.cc Co-authored-by: Steve Peters Signed-off-by: Nate Koenig * Update src/Root_TEST.cc Co-authored-by: Steve Peters Signed-off-by: Nate Koenig * Update src/Root_TEST.cc Co-authored-by: Steve Peters Signed-off-by: Nate Koenig * Update src/Root_TEST.cc Co-authored-by: Steve Peters Signed-off-by: Nate Koenig * Update src/Root_TEST.cc Co-authored-by: Steve Peters Signed-off-by: Nate Koenig * Update src/Root_TEST.cc Co-authored-by: Steve Peters Signed-off-by: Nate Koenig --------- Signed-off-by: Nate Koenig Co-authored-by: Steve Peters --- include/sdf/Root.hh | 5 +++++ python/src/sdf/pyRoot.cc | 4 ++++ python/test/pyRoot_TEST.py | 28 +++++++++++++++++++++++++ src/Root.cc | 6 ++++++ src/Root_TEST.cc | 42 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 85 insertions(+) diff --git a/include/sdf/Root.hh b/include/sdf/Root.hh index 9ad5f1632..75a25eb88 100644 --- a/include/sdf/Root.hh +++ b/include/sdf/Root.hh @@ -246,6 +246,11 @@ namespace sdf public: sdf::ElementPtr ToElement( const OutputConfig &_config = OutputConfig::GlobalConfig()) const; + /// \brief Remove the actor, light, or model if one of them exists. + /// The SDF Root object can only hold one, or none, from the set + /// [Actor, Light, Model]. + public: void ClearActorLightModel(); + /// \brief Private data pointer GZ_UTILS_IMPL_PTR(dataPtr) }; diff --git a/python/src/sdf/pyRoot.cc b/python/src/sdf/pyRoot.cc index 2da83830a..af94e78b5 100644 --- a/python/src/sdf/pyRoot.cc +++ b/python/src/sdf/pyRoot.cc @@ -96,6 +96,10 @@ void defineRoot(pybind11::object module) .def("set_light", &sdf::Root::SetLight, "Set the light object. This will override any existing model, " "actor, and light object.") + .def("clear_actor_light_model", &sdf::Root::ClearActorLightModel, + "Remove the actor, light, or model if one of them exists." + "The SDF Root object can only hold one, or none, from the set" + "[Actor, Light, Model].") .def("add_world", [](Root &self, const World &_world) { ThrowIfErrors(self.AddWorld(_world)); diff --git a/python/test/pyRoot_TEST.py b/python/test/pyRoot_TEST.py index 0a41bf667..785f94de3 100644 --- a/python/test/pyRoot_TEST.py +++ b/python/test/pyRoot_TEST.py @@ -358,5 +358,33 @@ def test_resolve_auto_inertials_with_save_calculation_configuration(self): self.assertEqual(len(inertialErr), 0) self.assertTrue(link.auto_inertia_saved()) + def test_clear_actor_light_model(self): + root = Root() + + # \TODO(anyone) Wrap the Actor class. + # self.assertEqual(None, root.actor()) + # actor1 = Actor() + # actor1.set_name("actor1") + # root.set_actor(actor1) + # self.assertNotEqual(None, root.actor()) + # root.clear_actor_light_model() + # self.assertEqual(None, root.actor()) + + self.assertEqual(None, root.light()) + light1 = Light() + light1.set_name("light1") + root.set_light(light1) + self.assertNotEqual(None, root.light()) + root.clear_actor_light_model() + self.assertEqual(None, root.light()) + + self.assertEqual(None, root.model()) + model1 = Model() + model1.set_name("model1") + root.set_model(model1) + self.assertNotEqual(None, root.model()) + root.clear_actor_light_model() + self.assertEqual(None, root.model()) + if __name__ == '__main__': unittest.main() diff --git a/src/Root.cc b/src/Root.cc index 03349d2af..6b5d4f618 100644 --- a/src/Root.cc +++ b/src/Root.cc @@ -644,3 +644,9 @@ sdf::ElementPtr Root::ToElement(const OutputConfig &_config) const return elem; } + +///////////////////////////////////////////////// +void Root::ClearActorLightModel() +{ + this->dataPtr->modelLightOrActor = std::monostate{}; +} diff --git a/src/Root_TEST.cc b/src/Root_TEST.cc index d48382276..b0b32cd32 100644 --- a/src/Root_TEST.cc +++ b/src/Root_TEST.cc @@ -670,3 +670,45 @@ TEST(DOMRoot, WorldByName) ASSERT_TRUE(root.WorldNameExists("world2")); EXPECT_EQ("world2", root.WorldByName("world2")->Name()); } + +///////////////////////////////////////////////// +TEST(DOMRoot, ClearActorLightModel) +{ + sdf::Root root; + EXPECT_EQ(nullptr, root.Actor()); + EXPECT_EQ(nullptr, root.Light()); + EXPECT_EQ(nullptr, root.Model()); + + sdf::Actor actor1; + actor1.SetName("actor1"); + root.SetActor(actor1); + EXPECT_NE(nullptr, root.Actor()); + EXPECT_EQ(nullptr, root.Light()); + EXPECT_EQ(nullptr, root.Model()); + root.ClearActorLightModel(); + EXPECT_EQ(nullptr, root.Actor()); + EXPECT_EQ(nullptr, root.Light()); + EXPECT_EQ(nullptr, root.Model()); + + sdf::Light light1; + light1.SetName("light1"); + root.SetLight(light1); + EXPECT_EQ(nullptr, root.Actor()); + EXPECT_NE(nullptr, root.Light()); + EXPECT_EQ(nullptr, root.Model()); + root.ClearActorLightModel(); + EXPECT_EQ(nullptr, root.Actor()); + EXPECT_EQ(nullptr, root.Light()); + EXPECT_EQ(nullptr, root.Model()); + + sdf::Model model1; + model1.SetName("model1"); + root.SetModel(model1); + EXPECT_EQ(nullptr, root.Actor()); + EXPECT_EQ(nullptr, root.Light()); + EXPECT_NE(nullptr, root.Model()); + root.ClearActorLightModel(); + EXPECT_EQ(nullptr, root.Actor()); + EXPECT_EQ(nullptr, root.Light()); + EXPECT_EQ(nullptr, root.Model()); +}