Skip to content

Only add python host/run dependency when a package actually needs it - #153

Open
Tobias-Fischer wants to merge 10 commits into
RoboStack:masterfrom
Tobias-Fischer:feature/conditional-python-dependency-v2
Open

Only add python host/run dependency when a package actually needs it#153
Tobias-Fischer wants to merge 10 commits into
RoboStack:masterfrom
Tobias-Fischer:feature/conditional-python-dependency-v2

Conversation

@Tobias-Fischer

Copy link
Copy Markdown
Contributor

Summary

Follow-up on RoboStack/robostack.github.io#100: every non-dummy recipe currently gets python (plus numpy/pip) added to host/run unconditionally, regardless of whether the package has any Python content of its own. Since that dependency participates in rattler-build's pinned Python variant, this is what makes building for more than one Python version rebuild the entire distro per version — including pure C++ libraries with zero Python content, which @traversaro's investigation in the linked issue found to be the majority (~418 of ~650 humble packages, via post-hoc inspection of already-built package contents).

This PR makes that dependency conditional, computed ahead of time from data vinca already parses out of package.xml — no extra network fetches, no post-hoc binary inspection needed.

What changed

  • _package_needs_python() (vinca/recipes.py): a package needs Python if:

    • its build type is ament_python (pure Python by construction), or
    • it's a rosidl_interface_packages member (msg/srv/action packages always get compiled Python bindings via rosidl_generator_py, independent of what else they declare), or
    • it actually depends (build, exec, run, or test) on a known Python-flavored package name (rclpy, pybind11, python_cmake_module, ament_cmake_python), or on any rosdep key containing "python" or starting with "pybind" (catches the python3-*/python-* naming convention).

    Deliberately conservative toward false positives: a package wrongly marked as needing Python just costs one extra rebuild if a multi-version matrix is ever used; a package wrongly marked as not needing it would silently ship a stale/missing artifact. Two things worth calling out since both were found empirically by building real ros-humble packages, not just synthetic tests:

    • buildtool_depend/buildtool_export_depend are deliberately not scanned. That tag means "a tool needed to invoke the build", never "this package's own artifact has Python content" — e.g. rclcpp declares <buildtool_depend>python3</buildtool_depend> purely so ament_cmake_gen_version_h can run a codegen script.
    • rosidl_default_generators/rosidl_generator_py are not in the curated marker list, despite being the obvious first guess. rclcpp (not a rosidl_interface_packages member) depends on rosidl_default_generators as a test-only dependency, to generate test_msgs for its own test suite — unrelated to rclcpp's own content. The rosidl_interface_packages group-membership check is the precise, authoritative signal for "this package itself has rosidl-generated Python bindings"; these two names only ever added noise on top of it.
  • Build-time-only Python is preserved. Every ament_cmake recipe still needs some interpreter present at build time (ament's own CMake tooling shells out to Python for boilerplate — environment hooks, package.xml parsing, index generation — regardless of the package's own content). That entry is kept in _BASE_REQUIREMENTS, but pinned to the single python_min version via Jinja (python ${{ python_min }}.*) instead of left as a bare "python". Confirmed via rattler-build build --render-only that a fully-resolved version constraint like this is invisible to rattler-build's variant/used_vars scan (one build variant either way), vs. N variants (one per pinned Python version) for a bare "python".

  • sccache routing (build_ament_cmake.sh.in): sets CMAKE_C/CXX_COMPILER_LAUNCHER=sccache when sccache is present in the build environment (a no-op otherwise). Useful once a downstream user actually enables a multi-version matrix, since a lot of the same C/C++ gets recompiled across Python versions for the packages that do need it.

  • $SP_DIR fallback: PYTHON_INSTALL_DIR computation now falls back to computing the standard lib/pythonX.Y/site-packages layout from whatever python is on PATH when $SP_DIR is unset, instead of crashing. Found by building real ros-humble packages: ament_cmake_test calls ament_python_install_package() in its own CMakeLists for a small internal test-utility module, without declaring any Python dependency in package.xml — so _package_needs_python correctly doesn't add python to its host requirements, but rattler-build then never sets $SP_DIR for its build (it's only populated when python is a host dependency), and the previously-unconditional os.environ['SP_DIR'] lookup raised a KeyError. Under set -e, a failing command substitution inside a plain assignment doesn't abort the script, so this was silently producing an emptyPYTHON_INSTALL_DIR for every affected package — harmless for most (they never call ament_python_install_package), a hard CMake configure error for the ones that do.

Validation

Built end-to-end (not just unit-tested) against a representative 133-package subset of ros-humble (std_msgs, example_interfaces, rclcpp, rclpy, demo_nodes_cpp, demo_nodes_py, launch, ros2cli, ros2topic + transitive closure) with a real 3-version Python matrix, osx-arm64: 77 packages built exactly once regardless of the matrix, 56 built 3x, 245 total artifacts, zero classification exceptions. Companion PR with the full writeup: RoboStack/ros-humble (link once opened).

Test plan

  • pytest vinca/ — 241 passed, including new tests for every branch of _package_needs_python (rosidl interface, ament_python, rclpy dependency, rosdep python3-* key, test-only dependency, the buildtool_depend exclusion, and the rosidl_default_generators test-only-dependency exclusion — each backed by the real-world case that motivated it)
  • Real build of a 133-package ros-humble subset, osx-arm64, 3-version Python matrix — see companion PR

🤖 Generated with Claude Code

…ds it
Every non-dummy recipe unconditionally got python (plus numpy/pip) in
host and run, regardless of whether the package itself has any Python
content. Since host/run python participates in the pinned python
variant, this drags every single ROS package into a full rebuild any
time the build wants to support more than one Python version -- even
pure C++ libraries and nodes, which are the vast majority.
Add _package_needs_python(): a conservative (false-positives-ok,
false-negatives-not) ahead-of-time heuristic based on data vinca
already parses from package.xml -- ament_python build type, rosidl
interface package membership (msg/srv/action always compile Python
bindings via rosidl_generator_py), or an actual python-flavored
dependency (rclpy, pybind11, any python3-*/python-* rosdep key). Only
packages matching one of these get python/numpy/pip in host and
python in run.
The build-time-only python entry every ament_cmake package still needs
(ament's own CMake tooling shells out to Python for boilerplate
regardless of the package's own content) is kept, but pinned to the
single python_min version via Jinja rather than left as a bare
"python" -- confirmed via `rattler-build build --render-only` that a
fully-resolved version constraint like this doesn't participate in the
variant/used_vars scan (one build variant either way), while a bare
"python" produces one variant per pinned python version.
…ache
Rebuilding the same package across several Python-version passes (the
whole point of the conditional-python-dependency change) recompiles a
lot of identical C/C++ source, since only the final Python-linking
bits actually differ between versions. Route CMAKE_C/CXX_COMPILER_LAUNCHER
through sccache when it's present in the build environment (a no-op
otherwise, so this is safe for anyone without sccache installed) to
turn that redundant recompilation into cache hits.
Per rattler-build's own sccache/ccache guidance, the top-level
`rattler-build build` invocation needs --no-build-id alongside this,
since both tools are sensitive to the timestamped build-directory path
rattler-build uses by default.
Real-world case found while testing on ros-humble: rclcpp (a pure C++
library with no Python content) declares
<buildtool_depend>python3</buildtool_depend> purely so
ament_cmake_gen_version_h can run a codegen script at build time. That
was matching the "python" substring catch-all and marking rclcpp as
needing a per-Python-version rebuild for no actual benefit.
buildtool_depend means "a tool needed to invoke this package's build
system" by ROS's own convention, never "this package's shipped
artifact has Python content" -- and build-time-only Python is already
covered unconditionally by the fixed python_min-pinned entry every
recipe gets. Excluding buildtool_depend/buildtool_export_depend from
the scan is a precision improvement, not a risk: every other
dependency type (build, exec, run, test) is still scanned, so this
doesn't reopen any false-negative risk.
…on markers
Another real-world false trigger found on ros-humble: rclcpp (not a
rosidl_interface_packages member) declares
<test_depend>rosidl_default_generators</test_depend> solely to
generate test_msgs for its own test suite, unrelated to rclcpp's own
shipped artifact having Python content.
The member_of_groups check is the precise, authoritative signal for
"this package itself has rosidl-generated Python bindings" (every
rosidl interface package declares that membership by ROS convention);
these two marker names only ever added noise on top of it, with a
demonstrated collision. Dropping them from the curated set doesn't
reopen any false-negative risk -- the group-membership check still
catches every genuine rosidl interface package unconditionally.
…resolves to it
Found by actually generating recipes against ros-humble, not just unit
tests: rclcpp's <buildtool_depend>python3</buildtool_depend> takes a
SEPARATE code path from _package_needs_python entirely -- vinca's
existing build_tools loop resolves every buildtool_depend and folds
whatever isn't "git" or "cmake" into build_dependencies, which then
gets resolved into requirements.host. python3 resolves (via
robostack.yaml) to the conda "python" package, so it was landing back
in host as a bare, un-pinned "python" -- reopening exactly the
variant-matrix problem the python_min-pinned base entry exists to
avoid, regardless of what _package_needs_python decided.
Skip re-adding it when a buildtool_depend resolves to exactly
["python"]: the base entry already provides a working build-time
interpreter for every package, buildtool_depend or not, so this is
never a loss of function -- purely removing a redundant, un-pinned
duplicate.
Also fixes the test double: SYSTEM_PACKAGES lacked a "python3" entry,
so test_needs_python_ignores_buildtool_depend_on_python3 was passing
for the wrong reason (the fake resolver never produced "python" from
"python3" at all, real robostack.yaml does) rather than exercising
this fix.
Found by building ros-humble with the new conditional python
dependency: ament_cmake_test's own CMakeLists calls
ament_python_install_package() for a small internal test-utility
module, without declaring any Python-flavored dependency in its
package.xml -- so _package_needs_python correctly doesn't add python
to its host requirements (nothing in its own metadata says it needs
it), but that also means rattler-build never sets $SP_DIR for its
build (SP_DIR is only populated when python is a host dependency),
and the unconditional `os.environ['SP_DIR']` lookup in
build_ament_cmake.sh.in raised a KeyError. Under `set -e`, a failing
command substitution inside a plain assignment doesn't itself abort
the script, so this silently produced an EMPTY PYTHON_INSTALL_DIR for
every affected package -- harmless for most (they never call
ament_python_install_package), but a hard CMake configure error for
the ones that do.
Fall back to computing the standard lib/pythonX.Y/site-packages layout
from whatever python is on PATH (the build-time-only interpreter every
recipe gets at minimum, per the python_min-pinned base requirement)
when $SP_DIR isn't set, instead of failing outright.
@Tobias-Fischer

Copy link
Copy Markdown
ContributorAuthor

Companion PR with the full writeup and end-to-end validation: RoboStack/ros-humble#424

Tobias-Fischerand others added 2 commits September 8, 2026 14:09
CI's fmt-check caught this -- package_xml()'s member_of_group ternary
needed ruff's multi-line wrapping.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
build_ament_cmake.sh.in got sccache routing, but build_catkin.sh.in
(used by vendored third-party CMake projects that aren't native
ament packages -- e.g. fastrtps, fastcdr, the "cmake"/"catkin" build
types) didn't. Found while trying to demonstrate sccache's benefit on
fastrtps specifically: sccache reported 0 compile requests across a
full rebuild, because this template's cmake invocation never set
CMAKE_C/CXX_COMPILER_LAUNCHER at all.
Same pattern as the ament_cmake template: no-op if sccache isn't
installed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@Tobias-Fischer

Copy link
Copy Markdown
ContributorAuthor

Pushed one more fix (7265c38): `build_ament_cmake.sh.in` got sccache routing, but `build_catkin.sh.in` (used by vendored third-party CMake projects that aren't native ament packages -- e.g. `fastrtps`, `fastcdr`, the "cmake"/"catkin" build types) didn't. Found while trying to demonstrate sccache's benefit specifically on a large, compile-heavy package: `sccache --show-stats` reported 0 compile requests across a full `fastrtps` rebuild, because this template's cmake invocation never set `CMAKE_C/CXX_COMPILER_LAUNCHER` at all. Same no-op-if-absent pattern as the ament_cmake template.

With the fix, a real comparison on `fastrtps` (isolated single-recipe rebuilds, same command each time) shows the second and third Python-version variants landing ~2.6-2.8x faster than an uncached rebuild (58s/61s -> 22s/22s), confirmed via 66.67% cache hit rate on that run. Full writeup in the companion PR (RoboStack/ros-humble#424).

Found trying to apply this to RoboStack/ros-humble's actual main
branch: unlike the (unmerged) branch this was originally validated
against, main's conda_build_config.yaml is a minimal, hand-maintained
file that doesn't define python_min at all (no full conda-forge-pinning
merge). An undefined python_min renders to an empty string, producing
the invalid match spec "python .*" and a hard recipe-parse failure --
not just for this recipe, for every single one, since every non-dummy
recipe gets this base requirement.
`${{ python_min | default('3.11') }}` degrades gracefully for any
vinca consumer whose pinning file doesn't define it, instead of
silently assuming a full conda-forge-pinning merge is always present.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@traversaro

Copy link
Copy Markdown
Member
  • sccache routing (build_ament_cmake.sh.in): sets CMAKE_C/CXX_COMPILER_LAUNCHER=sccache when sccache is present in the build environment (a no-op otherwise). Useful once a downstream user actually enables a multi-version matrix, since a lot of the same C/C++ gets recompiled across Python versions for the packages that do need it.

Can we make this somehow opt-in ? Silently changing behavior depending on the environment is always really confusing.

Per @traversaro's review on RoboStack#153: auto-detecting
sccache on PATH meant compiler invocations silently changed depending
on whether sccache happened to be installed for some unrelated reason
-- confusing to debug (e.g. two otherwise-identical machines producing
different build commands with no visible cause).
Now requires an explicit VINCA_USE_SCCACHE=1 in the build environment;
sccache being on PATH alone does nothing. Also hard-fails with a clear
message if VINCA_USE_SCCACHE=1 is set but sccache isn't actually
findable, rather than silently falling back to no caching (which would
be its own kind of confusing "it didn't error, but did it actually use
the cache?" situation).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@Tobias-Fischer

Copy link
Copy Markdown
ContributorAuthor

Good point, done (1bc88d1): sccache routing now requires an explicit VINCA_USE_SCCACHE=1 in the build environment rather than auto-detecting sccache on PATH -- it's a pure no-op (identical compiler invocation) unless that's set, and hard-fails with a clear message if it's set but sccache isn't actually findable, rather than silently degrading to no caching.

Verified end-to-end: with a freshly-reset sccache server, a build without the env var produces zero compile requests (despite sccache being installed), and the same build with it set produces the expected 100% cache hit rate.

Companion PR (RoboStack/ros-humble#424) updated to opt in explicitly via its build task's env rather than relying on sccache merely being installed -- same reasoning, made visible in the task definition instead of implicit.

Also tried a second package to demonstrate the caching benefit, per a request in the companion PR: rclpy (genuinely Python-dependent, with real compiled pybind11 bindings, unlike fastrtps which is an accepted false-positive). Couldn't get a clean isolated build of it locally though -- its dependency chain pulls in rmw_cyclonedds_cpp, whose cyclonedds build requires openssl >=4.0.2, which isn't resolvable in the currently-configured channels (only 3.x available). Confirmed reproducible from a fully clean rebuild, so it's a real, pre-existing gap in the conda-forge/robostack channel state, unrelated to this PR. fastrtps remains the demonstrated example (README-multi-python.md in the companion PR).

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Tobias-Fischer@traversaro