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

Jump to bottom

bazel: Updates for garden (minus ABI break) #598

Open
wants to merge 12 commits into
base: gz-physics6
Choose a base branch
from
Open

bazel: Updates for garden (minus ABI break) #598

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
9be2145
bazel: updates for garden (#513)
mjcarroll Jan 31, 2024
45a0de5
Don't define GZ_PHYSICS_VERSION_NAMESPACE
mjcarroll Feb 6, 2024
1491536
Special dart destructor doesn't work with static linking
mjcarroll Feb 6, 2024
63ebe75
Minimize diff
mjcarroll Feb 6, 2024
4c0eb37
Fix plugin
mjcarroll Feb 6, 2024
df16ec9
Minimize diff
mjcarroll Feb 6, 2024
ddcd72b
Minimize diff
mjcarroll Feb 6, 2024
b928d99
Address reviewer feedback
mjcarroll Feb 13, 2024
51001d9
Change to RTLD_NODELETE and fix tests
mjcarroll Feb 15, 2024
74b3a81
Buildifier
mjcarroll Feb 15, 2024
116416d
Buildifier
mjcarroll Feb 15, 2024
4d6ef43
Merge remote-tracking branch 'origin/mjcarroll/garden_bazel' into mjc…
mjcarroll Feb 15, 2024
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
130 changes: 130 additions & 0 deletions BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
load(
"@gz//bazel/skylark:build_defs.bzl",
"GZ_FEATURES",
"GZ_ROOT",
"GZ_VISIBILITY",
"gz_configure_header",
"gz_export_header",
"gz_include_header",
)

package(
default_visibility = GZ_VISIBILITY,
mjcarroll marked this conversation as resolved.
Show resolved Hide resolved
features = GZ_FEATURES,
)

licenses(["notice"]) # Apache-2.0
mjcarroll marked this conversation as resolved.
Show resolved Hide resolved

exports_files(["LICENSE"])

gz_configure_header(
name = "physics_config_hh",
mjcarroll marked this conversation as resolved.
Show resolved Hide resolved
src = "include/gz/physics/config.hh.in",
cmakelists = ["CMakeLists.txt"],
defines = {
# These definitions are unused,
# this is merely to suppress generator warnings
"GZ_PHYSICS_ENGINE_INSTALL_DIR": "unused",
},
package = "physics",
)

gz_export_header(
name = "include/gz/physics/Export.hh",
export_base = "GZ_PHYSICS",
lib_name = "gz-physics",
visibility = ["//visibility:private"],
)

public_headers_no_gen = glob([
"include/gz/physics/*.hh",
"include/gz/physics/detail/*.hh",
])
mjcarroll marked this conversation as resolved.
Show resolved Hide resolved

sources = glob(
["src/*.cc"],
exclude = [
"src/*_TEST.cc",
],
)

gz_include_header(
name = "physics_hh_genrule",
mjcarroll marked this conversation as resolved.
Show resolved Hide resolved
out = "include/gz/physics.hh",
hdrs = public_headers_no_gen + [
"include/gz/physics/config.hh",
"include/gz/physics/Export.hh",
],
)

public_headers = public_headers_no_gen + [
"include/gz/physics/config.hh",
"include/gz/physics/Export.hh",
"include/gz/physics.hh",
]

cc_library(
name = "physics",
srcs = sources,
hdrs = public_headers,
defines = [
'GZ_PHYSICS_ENGINE_RELATIVE_INSTALL_DIR=\\"unused\\"',
"GZ_PHYSICS_BAZEL_BUILD=true",
],
includes = ["include"],
deps = [
GZ_ROOT + "math",
GZ_ROOT + "math/eigen3",
GZ_ROOT + "plugin:core",
GZ_ROOT + "plugin:loader",
GZ_ROOT + "utils",
],
)

cc_library(
name = "heightmap",
hdrs = [
"heightmap/include/gz/physics/heightmap/HeightmapShape.hh",
mjcarroll marked this conversation as resolved.
Show resolved Hide resolved
"heightmap/include/gz/physics/heightmap/detail/HeightmapShape.hh",
],
includes = ["heightmap/include"],
)

cc_library(
name = "mesh",
hdrs = [
"mesh/include/gz/physics/mesh/MeshShape.hh",
"mesh/include/gz/physics/mesh/detail/MeshShape.hh",
],
includes = ["mesh/include"],
)

cc_library(
name = "sdf",
hdrs = glob([
"sdf/include/gz/physics/sdf/*.hh",
]),
includes = ["sdf/include"],
deps = [
GZ_ROOT + "sdformat",
],
)

test_sources = glob(
include = ["src/*_TEST.cc"],
mjcarroll marked this conversation as resolved.
Show resolved Hide resolved
)

[cc_test(
name = src.replace("/", "_").replace(".cc", "").replace("src_", ""),
srcs = [src],
env = {
"GZ_BAZEL": "1",
"GZ_BAZEL_PATH": "physics",
},
deps = [
":physics",
GZ_ROOT + "physics/test:test_headers",
"@gtest",
"@gtest//:gtest_main",
],
) for src in test_sources]
45 changes: 45 additions & 0 deletions bullet-featherstone/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
load(
"@gz//bazel/skylark:build_defs.bzl",
"GZ_FEATURES",
mjcarroll marked this conversation as resolved.
Show resolved Hide resolved
"GZ_ROOT",
"GZ_VISIBILITY",
)

private_headers = glob(["src/*.hh"])

sources = glob(
["src/*.cc"],
exclude = [
"src/*_TEST.cc",
"src/plugin.cc",
],
)

cc_library(
name = "bullet-featherstone",
srcs = sources + private_headers,
includes = ["include"],
visibility = GZ_VISIBILITY,
deps = [
GZ_ROOT + "common",
GZ_ROOT + "common/graphics",
GZ_ROOT + "math/eigen3",
GZ_ROOT + "physics",
GZ_ROOT + "physics:sdf",
GZ_ROOT + "third_party/bullet3:BulletCollision",
GZ_ROOT + "third_party/bullet3:BulletDynamics",
],
)

cc_binary(
name = "libgz-physics-bullet-featherstone-plugin.so",
srcs = [
"src/plugin.cc",
],
linkshared = True,
visibility = GZ_VISIBILITY,
deps = [
":bullet-featherstone",
GZ_ROOT + "plugin:register",
],
)
55 changes: 55 additions & 0 deletions dartsim/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
load(
"@gz//bazel/skylark:build_defs.bzl",
"GZ_FEATURES",
mjcarroll marked this conversation as resolved.
Show resolved Hide resolved
"GZ_ROOT",
"GZ_VISIBILITY",
"gz_configure_header",
"gz_export_header",
"gz_include_header",
)

public_headers = ["include/gz/physics/dartsim/World.hh"]

private_headers = glob(["src/*.hh"])

sources = glob(
["src/*.cc"],
exclude = [
"src/*_TEST.cc",
"src/plugin.cc",
],
)

cc_library(
name = "dartsim",
srcs = sources + private_headers,
hdrs = public_headers,
includes = ["include"],
visibility = GZ_VISIBILITY,
deps = [
GZ_ROOT + "common",
GZ_ROOT + "common/geospatial",
GZ_ROOT + "common/profiler",
GZ_ROOT + "math/eigen3",
GZ_ROOT + "physics",
GZ_ROOT + "physics:heightmap",
GZ_ROOT + "physics:mesh",
GZ_ROOT + "physics:sdf",
GZ_ROOT + "third_party/dart:simulation",
GZ_ROOT + "third_party/dart:collision-bullet",
GZ_ROOT + "third_party/dart:collision-ode",
],
)

cc_binary(
name = "libgz-physics-dartsim-plugin.so",
srcs = [
"src/plugin.cc",
],
linkshared = True,
visibility = GZ_VISIBILITY,
deps = [
":dartsim",
GZ_ROOT + "plugin:register",
],
)
mjcarroll marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 3 additions & 1 deletion dartsim/src/plugin.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ class Plugin :
public virtual SimulationFeatures,
public virtual WorldFeatures { };

#ifndef GZ_PHYSICS_BAZEL_BUILD
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI @azeey this is what breaks when dartsim is statically linked via bazel. Removing this section causes the cashes to go away.

Copy link
Member

Choose a reason for hiding this comment

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

💸

Copy link
Contributor

Choose a reason for hiding this comment

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

I found that loading the plugins with RTLD_NODELETE and removing this section fixes the issue on macOS and Linux. We do that for Gazebo plugins already, so I think we should do the same for physics engines.

namespace {

// This is done as a partial fix for
Expand All @@ -85,7 +86,8 @@ struct UnregisterCollisionDetectors
};

UnregisterCollisionDetectors unregisterAtUnload;
}
Copy link
Member

Choose a reason for hiding this comment

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

I think this removal is the source of the macOS test failures

} // namespace
#endif // GZ_PHYSICS_BAZEL_BUILD

GZ_PHYSICS_ADD_PLUGIN(Plugin, FeaturePolicy3d, DartsimFeatures)

Expand Down
19 changes: 13 additions & 6 deletions src/InstallationDirectories.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,27 +28,28 @@ namespace gz
{
namespace physics
{
inline namespace GZ_PHYSICS_VERSION_NAMESPACE {
Copy link
Member

Choose a reason for hiding this comment

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

given that this is still in the header file, is it ok to change it here?

https://github.com/gazebosim/gz-physics/blob/mjcarroll/garden_bazel/include/gz/physics/InstallationDirectories.hh#L30

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just moved it a little further down (

gz-physics/src/InstallationDirectories.cc

Line 149 in ddcd72b

inline namespace GZ_PHYSICS_VERSION_NAMESPACE {
), and put the rest in an anonymous namespace since it's not part of the public API

Copy link
Member

Choose a reason for hiding this comment

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

👍

namespace {

// We locally import the gz::common::joinPaths function
// See https://github.com/gazebosim/gz-physics/pull/507#discussion_r1186919267
// for more details

// Function imported from
// https://github.com/gazebosim/gz-common/blob/ignition-common4_4.6.2/src/FilesystemBoost.cc#L507
#ifndef WIN32
static const char preferred_separator = '/';
#ifndef _WIN32
const char preferred_separator = '/';
#else // Windows
static const char preferred_separator = '\\';
#endif
const std::string separator(const std::string &_p)
std::string separator(const std::string &_p)
{
return _p + preferred_separator;
}

#ifdef _WIN32
// Function imported from
// https://github.com/gazebosim/gz-common/blob/ignition-common4_4.6.2/src/Filesystem.cc#L227
std::string checkWindowsPath(const std::string _path)
std::string checkWindowsPath(const std::string &_path)
{
if (_path.empty())
return _path;
Expand All @@ -75,6 +76,7 @@ std::string checkWindowsPath(const std::string _path)
result, std::regex("[<>:\"|?*]"), "");
return result;
}
#endif

// Function imported from
// https://github.com/gazebosim/gz-common/blob/ignition-common4_4.6.2/src/Filesystem.cc#L256
Expand All @@ -101,7 +103,7 @@ std::string joinPaths(const std::string &_path1,
char replacement = '/';
#else
char replacement = '\\';
#endif
#endif // _WIN32

// Sanitize the start of the path.
size_t index = 0;
Expand Down Expand Up @@ -142,7 +144,12 @@ std::string joinPaths(const std::string &_path1,
#endif // _WIN32
return path;
}
}

inline namespace GZ_PHYSICS_VERSION_NAMESPACE {
#ifdef GZ_PHYSICS_BAZEL_BUILD
std::string getInstallPrefix() { return "physics"; }
#endif

std::string getEngineInstallDir()
{
Expand Down
Loading