diff --git a/README.md b/README.md index c8d5f6a..235a6bc 100644 --- a/README.md +++ b/README.md @@ -66,6 +66,22 @@ package_name_mode: both Once users and downstream projects have migrated, switch to `new` to stop generating the compatibility packages. New ROS 1 package names use the `ros-` prefix; new ROS 2 package names use `ros2-`. +## Python dependency + +By default every generated recipe has `python` in `build`, `host` and `run` (and +`numpy`/`pip` in `host`), so building for several Python versions rebuilds every +package once per version. With + +```yaml +conditional_python_dependency: true +``` + +only packages that need Python get it in `host`/`run`: `ament_python` packages, members +of `rosidl_interface_packages`, and packages that depend (build, exec, run or test) on +`rclpy`, `pybind11`, `python_cmake_module`, `ament_cmake_python` or a rosdep key +containing `python` or starting with `pybind`. All other packages only get a build-time +interpreter pinned to `python_min`, which does not create a Python variant. + ## Variants mode The optional `variants_mode` setting controls how rattler-build receives the diff --git a/vinca/recipes.py b/vinca/recipes.py index dbba622..bcd1845 100644 --- a/vinca/recipes.py +++ b/vinca/recipes.py @@ -75,6 +75,104 @@ "run": [], } +# Opt-in via `conditional_python_dependency: true` in vinca.yaml (default: off, every +# package gets python/numpy/pip as above). When on, only packages for which +# _package_needs_python() holds get python in host/run (plus numpy/pip in host); every +# other package keeps only a build-time interpreter, which ament's CMake tooling needs +# regardless (environment hooks, package.xml parsing, index generation). That one is +# pinned to python_min rather than a bare "python": a fully resolved version +# constraint is invisible to rattler-build's variant scan (confirmed with +# `rattler-build build --render-only`: one variant instead of one per pinned Python), +# so a package without Python content is not rebuilt per Python version. +# `default('3.11')` covers a conda_build_config.yaml without python_min, which would +# otherwise render the invalid spec "python .*". +_BUILD_TIME_PYTHON = "python ${{ python_min | default('3.11') }}.*" +_HOST_PYTHON = ("python", "numpy", "pip") + + +def _conditional_python(vinca_conf: dict[str, Any]) -> bool: + return bool(vinca_conf.get("conditional_python_dependency", False)) + + +# Dependency names that indicate a package's own recipe genuinely needs a +# Python interpreter/ABI in host/run (as opposed to Python merely being a +# build-time tool for ament's own scripts, see _BUILD_TIME_PYTHON). +# +# Deliberately NOT included: rosidl_default_generators/rosidl_generator_py. +# Real-world case found while testing on ros-humble: rclcpp (a pure C++ +# library, not a rosidl_interface_packages member) declares +# rosidl_default_generators solely to generate +# *test* message types (test_msgs) for its own test suite -- nothing to do +# with rclcpp's own shipped artifact having Python content. The +# member_of_groups check above is the precise, authoritative signal for "this +# package itself has rosidl-generated Python bindings"; these two names would +# only ever add noise on top of it. +_PYTHON_DEPENDENCY_MARKERS = frozenset( + { + "rclpy", + "pybind11", + "python_cmake_module", + "ament_cmake_python", + } +) + + +def _package_needs_python(package: catkin_pkg.package.Package, build_type: str) -> bool: + """Decide whether a package's OWN artifact needs Python (host/run), ahead of + building anything. + + This is deliberately conservative in the direction of false positives: a + package wrongly marked as needing Python just gets rebuilt once per Python + version for no benefit, whereas a package wrongly marked as NOT needing it + would silently be skipped when rebuilding for additional Python versions + and ship a stale/missing artifact for those versions. So every check here + is an "if in doubt, say yes": + + * ``ament_python`` packages are pure Python by construction. + * Any ``rosidl_interface_packages`` member (i.e. it has .msg/.srv/.action + files) gets rosidl_generator_py-compiled Python bindings unconditionally, + independent of what build_type or explicit dependencies it declares. + * Anything that actually depends (build, exec, run, or test -- a test-only + dependency still means Python must be importable to run the test suite + during the build) on a known Python-flavored package name, or on any + rosdep key containing "python" or starting with "pybind" (catches the + python3-*/python-* rosdep naming conventions along with pybind11 + variants), needs Python. + + ``buildtool_depend``/``buildtool_export_depend`` are deliberately NOT + scanned: by ROS's own convention that tag means "a tool needed to invoke + the build system" (e.g. many ament_cmake packages declare a plain + ``python3`` purely so a codegen + script can run at build time), never "this package's shipped artifact + contains Python content" -- and build-time-only Python is already covered + by the ``python_min``-pinned ``_BUILD_TIME_PYTHON`` every recipe gets. + + Everything else -- the vast majority of ROS packages, which are plain C/ + C++ libraries and nodes -- does not, and skips the Python host/run + dependency (and therefore the per-Python-version rebuild) entirely. + """ + if build_type == "ament_python": + return True + if any( + group.name == "rosidl_interface_packages" for group in package.member_of_groups + ): + return True + dependency_names = { + dependency.name + for dependency in ( + *package.build_depends, + *package.build_export_depends, + *package.exec_depends, + *package.run_depends, + *package.test_depends, + ) + } + if dependency_names & _PYTHON_DEPENDENCY_MARKERS: + return True + return any( + "python" in name or name.startswith("pybind") for name in dependency_names + ) + def get_depmods( vinca_conf: dict[str, Any], package_name: str, distro: Distro @@ -124,9 +222,18 @@ def _initial_output( vinca_conf: dict[str, Any], ) -> dict[str, Any]: if not is_dummy_metapackage(shortname, vinca_conf): + requirements = copy.deepcopy(_BASE_REQUIREMENTS) + if _conditional_python(vinca_conf): + requirements["build"] = [ + _BUILD_TIME_PYTHON if r == "python" else r + for r in requirements["build"] + ] + requirements["host"] = [ + r for r in requirements["host"] if r not in _HOST_PYTHON + ] return { "package": {"name": package_name, "version": version}, - "requirements": copy.deepcopy(_BASE_REQUIREMENTS), + "requirements": requirements, "build": {"script": ""}, } @@ -347,12 +454,19 @@ def generate_output( package = catkin_pkg.package.parse_package_string(xml) package.evaluate_conditions(os.environ) - python_dependencies = resolve_pkgname("python", vinca_conf, distro) - output["requirements"]["run"].extend(python_dependencies) - output["requirements"]["host"].extend(python_dependencies) - is_dummy = is_dummy_metapackage(shortname, vinca_conf) build_type = package.get_build_type() + + conditional_python = _conditional_python(vinca_conf) + if not conditional_python or ( + not is_dummy and _package_needs_python(package, build_type) + ): + python_dependencies = resolve_pkgname("python", vinca_conf, distro) + output["requirements"]["run"].extend(python_dependencies) + output["requirements"]["host"].extend(python_dependencies) + if conditional_python: + output["requirements"]["host"].extend(["numpy", "pip"]) + if not is_dummy: try: output["build"]["script"] = _BUILD_SCRIPTS[build_type] @@ -412,12 +526,18 @@ def generate_output( # Build tools normally belong in `host`, but git has to be in `build` so that it # is runnable on the build machine when cross-compiling. cmake is already part of # the base build requirements, so re-adding it would only create a duplicate. + # With conditional_python_dependency, python3/python as a buildtool_depend (just an + # interpreter for a codegen script, e.g. rclcpp's for ament_cmake_gen_version_h) is + # covered by _BUILD_TIME_PYTHON; a bare "python" here would put the package back + # into the per-Python-version build matrix. for dependency in build_tools: resolved = resolve_pkgname(dependency, vinca_conf, distro) if not resolved: unsatisfied.add(dependency) elif "git" in resolved: output["requirements"]["build"].extend(resolved) + elif conditional_python and resolved == ["python"]: + pass elif dependency != "cmake": build_dependencies.append(dependency) diff --git a/vinca/templates/build_ament_cmake.sh.in b/vinca/templates/build_ament_cmake.sh.in index cc25a25..f097989 100644 --- a/vinca/templates/build_ament_cmake.sh.in +++ b/vinca/templates/build_ament_cmake.sh.in @@ -14,6 +14,24 @@ cd build # necessary for correctly linking SIP files (from python_qt_bindings) export LINK=$CXX +# Speed up repeated local builds (e.g. rebuilding the same packages across +# several Python-version passes) by routing the C/C++ compiler through +# sccache -- opt-in via VINCA_USE_SCCACHE=1, rather than auto-detecting +# sccache on PATH, so this never silently changes compiler invocations +# just because sccache happens to be installed for some unrelated reason. +# Requires the top-level `rattler-build build` invocation to pass +# --no-build-id, since both ccache and sccache are sensitive to the +# timestamped build-directory paths rattler-build uses by default. +SCCACHE_CMAKE_ARGS="" +if [[ "${VINCA_USE_SCCACHE:-}" == "1" ]]; then + if command -v sccache >/dev/null 2>&1; then + SCCACHE_CMAKE_ARGS="-DCMAKE_C_COMPILER_LAUNCHER=sccache -DCMAKE_CXX_COMPILER_LAUNCHER=sccache" + else + echo "VINCA_USE_SCCACHE=1 but sccache was not found on PATH" >&2 + exit 1 + fi +fi + if [[ "$CONDA_BUILD_CROSS_COMPILATION" != "1" ]]; then PYTHON_EXECUTABLE=$PREFIX/bin/python PKG_CONFIG_EXECUTABLE=$PREFIX/bin/pkg-config @@ -58,8 +76,22 @@ fi; # PYTHON_INSTALL_DIR should be a relative path, see # https://github.com/ament/ament_cmake/blob/2.3.2/ament_cmake_python/README.md # So we compute the relative path of $SP_DIR w.r.t. to $PREFIX, -# but it is not trivial to do this in bash scripting, so let's do it via python -export PYTHON_INSTALL_DIR=`python -c "import os;print(os.path.relpath(os.environ['SP_DIR'],os.environ['PREFIX']))"` +# but it is not trivial to do this in bash scripting, so let's do it via python. +# rattler-build only sets $SP_DIR when python is one of the recipe's own host +# dependencies (with conditional_python_dependency, vinca only adds that for +# packages it detects actually need Python) -- but a handful of packages call ament_python_install_package() in +# their own CMakeLists for some small internal helper without declaring a +# Python-flavored dependency in package.xml at all (e.g. ament_cmake_test's +# test-utility module), so $SP_DIR can be legitimately unset even though this +# specific build still needs a real PYTHON_INSTALL_DIR. Fall back to computing +# the standard site-packages layout from whatever python is on PATH (the +# build-time-only interpreter every recipe gets, at minimum) instead of +# failing outright. +if [[ -n "${SP_DIR:-}" ]]; then + export PYTHON_INSTALL_DIR=`python -c "import os;print(os.path.relpath(os.environ['SP_DIR'],os.environ['PREFIX']))"` +else + export PYTHON_INSTALL_DIR=`python -c "import sys;print('lib/python%d.%d/site-packages' % sys.version_info[:2])"` +fi echo "Using PYTHON_INSTALL_DIR: $PYTHON_INSTALL_DIR" if [[ $target_platform =~ emscripten.* ]]; then @@ -132,6 +164,7 @@ $CMAKE_GEN \ -DCMAKE_OSX_DEPLOYMENT_TARGET=$OSX_DEPLOYMENT_TARGET \ --compile-no-warning-as-error \ $EXTRA_CMAKE_ARGS \ + $SCCACHE_CMAKE_ARGS \ @(additional_cmake_args) \ $WORK_DIR diff --git a/vinca/templates/build_catkin.sh.in b/vinca/templates/build_catkin.sh.in index 157fccf..f83ccf4 100644 --- a/vinca/templates/build_catkin.sh.in +++ b/vinca/templates/build_catkin.sh.in @@ -19,6 +19,24 @@ cd build # necessary for correctly linking SIP files (from python_qt_bindings) export LINK=$CXX +# Speed up repeated local builds (e.g. rebuilding the same packages across +# several Python-version passes) by routing the C/C++ compiler through +# sccache -- opt-in via VINCA_USE_SCCACHE=1, rather than auto-detecting +# sccache on PATH, so this never silently changes compiler invocations +# just because sccache happens to be installed for some unrelated reason. +# Requires the top-level `rattler-build build` invocation to pass +# --no-build-id, since both ccache and sccache are sensitive to the +# timestamped build-directory paths rattler-build uses by default. +SCCACHE_CMAKE_ARGS="" +if [[ "${VINCA_USE_SCCACHE:-}" == "1" ]]; then + if command -v sccache >/dev/null 2>&1; then + SCCACHE_CMAKE_ARGS="-DCMAKE_C_COMPILER_LAUNCHER=sccache -DCMAKE_CXX_COMPILER_LAUNCHER=sccache" + else + echo "VINCA_USE_SCCACHE=1 but sccache was not found on PATH" >&2 + exit 1 + fi +fi + if [[ "$CONDA_BUILD_CROSS_COMPILATION" != "1" ]]; then PYTHON_EXECUTABLE=$PREFIX/bin/python PKG_CONFIG_EXECUTABLE=$PREFIX/bin/pkg-config @@ -109,6 +127,7 @@ cmake ${CMAKE_ARGS} --compile-no-warning-as-error \ -DCATKIN_BUILD_BINARY_PACKAGE=$CATKIN_BUILD_BINARY_PACKAGE \ -DCMAKE_OSX_DEPLOYMENT_TARGET=$OSX_DEPLOYMENT_TARGET \ $EXTRA_CMAKE_ARGS \ + $SCCACHE_CMAKE_ARGS \ @(additional_cmake_args) \ -G "$GENERATOR" \ $SRC_DIR/$PKG_NAME/src/work/@(additional_folder) diff --git a/vinca/test_recipes.py b/vinca/test_recipes.py index 2809d87..37e3bd7 100644 --- a/vinca/test_recipes.py +++ b/vinca/test_recipes.py @@ -14,6 +14,7 @@ https://github.com/example/{name} {depends} {build_type} +{member_of_group} """ @@ -21,6 +22,9 @@ SYSTEM_PACKAGES = { "python": ["python"], + # matches robostack.yaml's real rosdep mapping: python3 -> the same + # conda "python" package as the plain "python" rosdep key. + "python3": ["python"], "python-setuptools": ["setuptools"], "cmake": ["cmake"], "git": ["git"], @@ -32,9 +36,16 @@ } -def package_xml(name, build_type="ament_cmake", depends=()): +def package_xml(name, build_type="ament_cmake", depends=(), member_of_group=None): body = "\n".join(f" <{tag}>{value}" for tag, value in depends) - return PACKAGE_XML.format(name=name, depends=body, build_type=build_type) + group = ( + f" {member_of_group}" + if member_of_group + else "" + ) + return PACKAGE_XML.format( + name=name, depends=body, build_type=build_type, member_of_group=group + ) class FakeDistro: @@ -95,12 +106,13 @@ def build( name, depends=(), build_type="ament_cmake", + member_of_group=None, unsatisfied=None, dependencies_only=False, **overrides, ): distro = FakeDistro( - {name: package_xml(name, build_type, depends)}, + {name: package_xml(name, build_type, depends, member_of_group)}, repository_by_name={name: f"https://github.com/ros2/{name}.git"}, ) return generate_output( @@ -222,6 +234,114 @@ def test_every_duplicate_cmake_is_replaced_by_the_selector(): ) +def build_conditional(name, **kwargs): + return build(name, conditional_python_dependency=True, **kwargs) + + +def _host_run_python_markers(output): + host = output["requirements"]["host"] + run = output["requirements"]["run"] + return "python" in host, "numpy" in host, "pip" in host, "python" in run + + +def test_python_dependency_is_unconditional_by_default(): + # Without conditional_python_dependency every package keeps python in + # build/host/run, exactly as before the option existed. + output = build("demo", depends=[("build_depend", "rclcpp")]) + + assert _host_run_python_markers(output) == (True, True, True, True) + assert "python" in output["requirements"]["build"] + + +def test_needs_python_plain_cpp_package_gets_no_python_dependency(): + # A pure C++ library/node (no rosidl interfaces, no python-flavored + # dependency) must not be pulled into the python host/run dependency -- + # and therefore not into a per-python-version rebuild -- at all. + output = build_conditional("demo", depends=[("build_depend", "rclcpp")]) + + assert _host_run_python_markers(output) == (False, False, False, False) + # The build-time-only interpreter ament's own tooling needs is still + # present, but pinned to a single fixed version rather than the bare + # "python" that would drag this recipe into the python variant matrix. + assert ( + "python ${{ python_min | default('3.11') }}.*" + in output["requirements"]["build"] + ) + assert "python" not in output["requirements"]["build"] + + +def test_needs_python_rosidl_interface_package_gets_python_dependency(): + # msg/srv/action packages declare membership in rosidl_interface_packages; + # rosidl_generator_py always compiles Python bindings for these, + # independent of whatever build_type or explicit depends they declare. + output = build_conditional( + "demo", + depends=[("build_depend", "rosidl_default_generators")], + member_of_group="rosidl_interface_packages", + ) + + assert _host_run_python_markers(output) == (True, True, True, True) + + +def test_needs_python_rclpy_dependency_gets_python_dependency(): + output = build_conditional("demo", depends=[("exec_depend", "rclpy")]) + + assert _host_run_python_markers(output) == (True, True, True, True) + + +def test_needs_python_rosdep_python3_key_gets_python_dependency(): + # Broad, deliberately permissive catch-all for the python3-*/python-* + # rosdep naming convention: false positives here are cheap (one extra + # per-python-version rebuild), false negatives are not (a silently + # missing artifact for the versions it wasn't rebuilt for). + output = build_conditional("demo", depends=[("exec_depend", "python3-yaml")]) + + assert _host_run_python_markers(output) == (True, True, True, True) + + +def test_needs_python_ament_python_build_type_gets_python_dependency(): + output = build_conditional("demo", build_type="ament_python") + + assert _host_run_python_markers(output) == (True, True, True, True) + + +def test_needs_python_test_only_dependency_still_counts(): + # A test-only dependency still means Python must be importable to run + # the test suite during the build, so it counts too. + output = build_conditional("demo", depends=[("test_depend", "rclpy")]) + + assert _host_run_python_markers(output) == (True, True, True, True) + + +def test_needs_python_ignores_buildtool_depend_on_python3(): + # Real-world case: rclcpp (a pure C++ library with no Python content of + # its own) declares python3 purely + # so ament_cmake_gen_version_h can run a codegen script at build time. + # buildtool_depend means "a tool needed to invoke the build", never + # "this package's shipped artifact has Python content", so it must not + # trigger the python host/run dependency (build-time-only Python is + # already covered unconditionally elsewhere). + output = build_conditional("demo", depends=[("buildtool_depend", "python3")]) + + assert _host_run_python_markers(output) == (False, False, False, False) + + +def test_needs_python_ignores_test_only_rosidl_default_generators(): + # Real-world case: rclcpp (not a rosidl_interface_packages member) has + # rosidl_default_generators solely to + # generate test_msgs for its own test suite -- nothing to do with + # rclcpp's own shipped artifact having Python content. The + # member_of_groups check is the precise signal for "this package itself + # has rosidl-generated Python bindings" (see the positive case above); + # rosidl_default_generators/rosidl_generator_py depended on for any other + # reason must not, by itself, trigger the python host/run dependency. + output = build_conditional( + "demo", depends=[("test_depend", "rosidl_default_generators")] + ) + + assert _host_run_python_markers(output) == (False, False, False, False) + + def test_mimick_vendor_moves_from_host_to_build(): output = build("demo", depends=[("build_depend", "mimick_vendor")])