Embed optional collision backends in core dart by jslee02 · Pull Request #2194 · dartsim/dart

jslee02 · 2025-11-16T21:04:41Z

Purpose

Stop shipping separate dart-collision-* components so the optional Bullet and ODE detectors behave like first-class features of the dart library.
Make the build configuration fail fast when those detectors are requested but their dependencies are missing, eliminating silent feature drops in CI and downstream packages.
Expose the new behavior through the generated CMake/package metadata so integrators immediately know which collision engines were compiled in.
Start the larger effort to remove per-dependency component targets (e.g., dart-optimizer-*) so future follow-up PRs can apply the same pattern across the rest of the tree.

Changes

Add cache flags DART_BUILD_COLLISION_BULLET / DART_BUILD_COLLISION_ODE (default ON in Pixi) and teach the Bullet/ODE subdirectories to
- bail out early when the option is OFF,
- emit FATAL_ERROR when dependencies are missing while the option is ON, and
- cmake_language(DEFER ...) their sources/includes/link flags directly onto the dart target so there are no standalone collision libraries.
Update the generated DARTConfig.cmake, pkg-config hints, examples, tutorials, benchmarks, tests, and dartpy bindings to link only against dart/dart-utils* while conditionally enabling Bullet/ODE-specific executables based on the new flags.
Refresh docs/onboarding, CHANGELOG, and AGENTS.md to document the embedded backends, the new options, and the libsdformat requirement.

Implications

Build behavior: Turning either option ON without the corresponding dependency now stops configuration instead of producing a partially featured build. Users who need Bullet/ODE simply enable the option (default ON in Pixi) and install libbullet-dev ≥3.25 or libode-dev ≥0.13; everyone else can set the option OFF to skip the backend entirely.
Package managers: The main dart artifact now carries Bullet/ODE when the options are enabled, so there is no need to produce or ship extra dart-collision-* packages. Downstream projects discover the availability via DART_BUILD_COLLISION_* cache exports or the existing HAVE_BULLET/HAVE_ODE config defines.
Users: Project code and CMake files no longer reference dart-collision-* targets; linking against dart is sufficient. Tests/examples guard Bullet/ODE-specific binaries with the new CMake options, so developers running partial builds receive clear skip/failure messages instead of link errors.
Follow-up work: Optimizer plugins and other optional integrations still produce component targets today; those will be cleaned up in subsequent PRs using the same approach as soon as their dependencies are audited.

Milestone: DART 7.0

Before creating a pull request

Run pixi run test-all to lint, build, and test your changes (not run in this environment)
Add unit tests for new functionality (covered by existing collision suites; no new APIs added)
Document new methods and classes
Add Python bindings (dartpy) if applicable (not applicable beyond the linking cleanup already in this change)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

tests/integration/CMakeLists.txt

jslee02 · 2025-11-16T21:15:34Z

@codex review

chatgpt-codex-connector · 2025-11-16T21:19:53Z

💡 Codex Review

dart/dart/collision/bullet/CMakeLists.txt

Lines 62 to 75 in 412fa65

    
           dart_add_core_headers(${hdrs} ${detail_hdrs}) 
        
           dart_add_core_sources(${srcs} ${detail_srcs}) 
        
           if(NOT BULLET_LIBRARIES) 
        
             message(FATAL_ERROR "Bullet was found but did not report BULLET_LIBRARIES.") 
        
           endif() 
        
           set(_dart_bullet_link_items ${BULLET_LIBRARIES}) 
        
           set(_dart_bullet_include_dirs ${BULLET_INCLUDE_DIRS}) 
        
           get_filename_component(_dart_collision_binary_dir ${CMAKE_CURRENT_BINARY_DIR} DIRECTORY) 
        
           get_filename_component(_dart_binary_dir ${_dart_collision_binary_dir} DIRECTORY) 
        
           cmake_language( 
        
             DEFER 
        
             DIRECTORY ${_dart_binary_dir} 
        
             CALL target_link_libraries dart PUBLIC ${_dart_bullet_link_items} 
        
           )

pkg-config omits Bullet/ODE libs after embedding them in dart

These lines wire Bullet directly into the core dart target, and the ODE file applies the same pattern. However cmake/dart.pc.in still exports only assimp, ccd, eigen3, fcl, octomap in Requires: and -ldart in Libs:. When a build enables Bullet or ODE (the default in Pixi), pkg-config --libs dart will not emit -lBullet or -lODE, so downstream projects that rely on pkg-config rather than CMake will fail to link due to missing symbols. The pkg-config template needs to append the conditional collision backends whenever they are compiled into dart.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

jslee02 · 2025-11-16T21:43:21Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

examples/add_delete_skels/CMakeLists.txt

CMakeLists.txt

jslee02 · 2025-11-16T23:54:34Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

dart/collision/ode/CMakeLists.txt

jslee02 · 2025-11-17T00:51:41Z

@codex review

chatgpt-codex-connector · 2025-11-17T00:56:41Z

Codex Review: Didn't find any major issues. Hooray!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

jslee02 · 2025-11-17T02:53:44Z

@codex review

chatgpt-codex-connector

💡 Codex Review

dart/dart/optimizer/ipopt/CMakeLists.txt

Lines 13 to 16 in 1d85c3e

    
           # Add target 
        
           dart_add_library(${target_name} ${hdrs} ${srcs}) 
        
           target_link_libraries(${target_name} PUBLIC dart IPOPT::ipopt)

Restore Ipopt version macros used by conditional includes

The Ipopt component no longer defines IPOPT_VERSION_MAJOR/MINOR/PATCH for the dart-optimizer-ipopt target. IpoptSolver.hpp still evaluates #if IPOPT_VERSION_GE(3, 13, 0) to choose between <IpIpoptApplication.hpp> and <coin/IpIpoptApplication.hpp>. Without these compile definitions the macros evaluate to zero, so any build against Ipopt ≥ 3.13 always goes down the legacy coin/... include path and fails because those headers no longer exist. Please reinstate the target_compile_definitions that populate the version macros or include IpoptConfig.h before the version check so modern Ipopt releases can compile.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

jslee02 · 2025-11-17T04:44:15Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

dart/CMakeLists.txt

jslee02 · 2025-11-17T05:01:55Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

CMakeLists.txt

jslee02 · 2025-11-17T14:38:02Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

dart/CMakeLists.txt

jslee02 · 2025-11-17T15:37:24Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

dart/collision/ode/CMakeLists.txt

jslee02 · 2025-11-17T16:36:49Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

scripts/patch_gz_physics.py

codecov · 2025-11-17T19:55:18Z

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 61.29%. Comparing base (827dd87) to head (4edfeae).
⚠️ Report is 321 commits behind head on main.

Files with missing lines	Patch %	Lines
dart/optimizer/ipopt/IpoptSolver.cpp	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2194      +/-   ##
==========================================
- Coverage   61.75%   61.29%   -0.47%     
==========================================
  Files         388      363      -25     
  Lines       33216    32043    -1173     
  Branches     4214     4090     -124     
==========================================
- Hits        20512    19640     -872     
+ Misses      12704    12403     -301

Flag	Coverage Δ
unittests	`61.29% <0.00%> (-0.47%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
dart/optimizer/ipopt/IpoptSolver.cpp	`74.24% <0.00%> (ø)`

... and 34 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Ensure pixi enables collision backends

3b94bb0

jslee02 added this to the DART 7.0 milestone Nov 16, 2025

chatgpt-codex-connector bot reviewed Nov 16, 2025

View reviewed changes

tests/integration/CMakeLists.txt Outdated Show resolved Hide resolved

jslee02 force-pushed the dart_coll_targets branch from 139475d to 412fa65 Compare November 16, 2025 21:14

jslee02 force-pushed the dart_coll_targets branch from 412fa65 to f30b5e5 Compare November 16, 2025 21:17

jslee02 force-pushed the dart_coll_targets branch from f30b5e5 to 89b582c Compare November 16, 2025 21:42

chatgpt-codex-connector bot reviewed Nov 16, 2025

View reviewed changes

examples/add_delete_skels/CMakeLists.txt Show resolved Hide resolved

CMakeLists.txt Show resolved Hide resolved

jslee02 force-pushed the dart_coll_targets branch from 89b582c to b9718f7 Compare November 16, 2025 23:33

chatgpt-codex-connector bot reviewed Nov 16, 2025

View reviewed changes

dart/collision/ode/CMakeLists.txt Show resolved Hide resolved

jslee02 force-pushed the dart_coll_targets branch from b9718f7 to 9ec6154 Compare November 17, 2025 00:50

Merge collision backends into core dart

1d85c3e

jslee02 force-pushed the dart_coll_targets branch from 9ec6154 to 1d85c3e Compare November 17, 2025 02:36

Merge branch 'main' into dart_coll_targets

7666aa5

chatgpt-codex-connector bot reviewed Nov 17, 2025

View reviewed changes

jslee02 added 2 commits November 16, 2025 19:02

Merge branch 'main' into dart_coll_targets

3155cf4

Merge branch 'main' into dart_coll_targets

65a1f9f

chatgpt-codex-connector bot reviewed Nov 17, 2025

View reviewed changes

dart/CMakeLists.txt Show resolved Hide resolved

jslee02 force-pushed the dart_coll_targets branch 2 times, most recently from 4df3d6d to 6d5bb22 Compare November 17, 2025 05:01

chatgpt-codex-connector bot reviewed Nov 17, 2025

View reviewed changes

CMakeLists.txt Show resolved Hide resolved

Merge collision backends into core dart

b2e9224

jslee02 force-pushed the dart_coll_targets branch from 6d5bb22 to b2e9224 Compare November 17, 2025 05:20

Attemp to fix gz build

784f803

jslee02 enabled auto-merge (squash) November 17, 2025 14:37

chatgpt-codex-connector bot reviewed Nov 17, 2025

View reviewed changes

dart/CMakeLists.txt Outdated Show resolved Hide resolved

jslee02 added 2 commits November 17, 2025 06:58

clean up pixi config-install exports

04044f8

cmake: keep ODE warning flag private

9280f4a

chatgpt-codex-connector bot reviewed Nov 17, 2025

View reviewed changes

dart/collision/ode/CMakeLists.txt Show resolved Hide resolved

cmake: guard ODE warning flag by compiler

2f2f5f5

chatgpt-codex-connector bot reviewed Nov 17, 2025

View reviewed changes

scripts/patch_gz_physics.py Show resolved Hide resolved

scripts: make gz patch idempotent

95828ac

jslee02 marked this pull request as draft November 17, 2025 17:46

auto-merge was automatically disabled November 17, 2025 17:46
Pull request was converted to draft

Merge branch 'main' into dart_coll_targets

c88c12e

Merge branch 'main' into dart_coll_targets

4edfeae

jslee02 marked this pull request as ready for review November 18, 2025 03:34

jslee02 merged commit 3668a90 into main Nov 18, 2025
25 of 33 checks passed

jslee02 deleted the dart_coll_targets branch November 18, 2025 03:34

	# Add target
	dart_add_library(${target_name} ${hdrs} ${srcs})
	target_link_libraries(${target_name} PUBLIC dart IPOPT::ipopt)

Conversation

jslee02 commented Nov 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Changes

Implications

Before creating a pull request

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

jslee02 commented Nov 16, 2025

Uh oh!

chatgpt-codex-connector bot commented Nov 16, 2025

💡 Codex Review

Uh oh!

jslee02 commented Nov 16, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

jslee02 commented Nov 16, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

jslee02 commented Nov 17, 2025

Uh oh!

chatgpt-codex-connector bot commented Nov 17, 2025

Uh oh!

jslee02 commented Nov 17, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

jslee02 commented Nov 17, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

jslee02 commented Nov 17, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

jslee02 commented Nov 17, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

jslee02 commented Nov 17, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

jslee02 commented Nov 17, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

codecov bot commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

jslee02 commented Nov 16, 2025 •

edited

Loading

codecov bot commented Nov 17, 2025 •

edited

Loading