feat(ros): add inline package mappings (#369)

Author: daenny

backends/pixi-build-ros/pixi.lock

@@ -21,7 +21,9 @@ test = "pytest tests"
 [feature.test.dependencies]
 pixi-pycharm = ">=0.0.8,<0.1"
 pytest = "*"
-py-pixi-build-backend = { path = "../../py-pixi-build-backend" }
+py-pixi-build-backend = "*"
+# Skipping as it's a long build, uncomment this when developing from source
+# py-pixi-build-backend = { path = "../../py-pixi-build-backend" }
 
 [package.build.backend]
 name = "pixi-build-python"
@@ -42,5 +44,4 @@ pyyaml = "*"
 pixi-build-api-version = ">=2,<3"
 # should be added to `py-pixi-build-backend`
 typing-extensions = "*"
-#py-pixi-build-backend = "*"
-py-pixi-build-backend = { path = "../../py-pixi-build-backend" }
+py-pixi-build-backend = "*"

backends/pixi-build-ros/pixi.toml

@@ -28,6 +28,7 @@
     convert_package_xml_to_catkin_package,
     get_package_xml_content,
     load_package_map_data,
+    PackageMappingSource,
 )
 
 
@@ -43,7 +44,7 @@ def _parse_str_as_abs_path(value: str | Path, manifest_root: Path) -> Path:
     return value
 
 
-class ROSBackendConfig(pydantic.BaseModel, extra="forbid"):
+class ROSBackendConfig(pydantic.BaseModel, extra="forbid", arbitrary_types_allowed=True):
     """ROS backend configuration."""
 
     noarch: Optional[bool] = None
@@ -58,7 +59,9 @@ class ROSBackendConfig(pydantic.BaseModel, extra="forbid"):
     distro: Optional[str] = None
 
     # Extra package mappings to use in the build
-    extra_package_mappings: List[Path] = pydantic.Field(default_factory=list, alias="extra-package-mappings")
+    extra_package_mappings: List[PackageMappingSource] = pydantic.Field(
+        default_factory=list, alias="extra-package-mappings"
+    )
 
     def is_noarch(self) -> bool:
         """Whether to build a noarch package or a platform-specific package."""
@@ -77,18 +80,40 @@ def _parse_debug_dir(cls, value, info: pydantic.ValidationInfo) -> Optional[Path
 
     @pydantic.field_validator("extra_package_mappings", mode="before")
     @classmethod
-    def _parse_package_mappings(cls, input_value, info: pydantic.ValidationInfo) -> Optional[List[Path]]:
+    def _parse_package_mappings(
+        cls, input_value, info: pydantic.ValidationInfo
+    ) -> Optional[List[PackageMappingSource]]:
         """Parse additional package mappings if set."""
         if input_value is None:
             return []
+
         base_path = Path(os.getcwd())
         if info.context and "manifest_root" in info.context:
             base_path = Path(info.context["manifest_root"])
 
-        res = []
-        for path_value in input_value:
-            res.append(_parse_str_as_abs_path(path_value, base_path))
-        return res
+        result: List[PackageMappingSource] = []
+        for raw_entry in input_value:
+            # match for cases
+            # it's already a package mapping source (usually for testing)
+            if isinstance(raw_entry, PackageMappingSource):
+                entry = raw_entry
+            elif isinstance(raw_entry, dict):
+                if "file" in raw_entry:
+                    file_value = raw_entry["file"]
+                    entry = PackageMappingSource.from_file(_parse_str_as_abs_path(file_value, base_path))
+                elif "mapping" in raw_entry:
+                    mapping_value = raw_entry["mapping"]
+                    entry = PackageMappingSource.from_mapping(mapping_value)
+                else:
+                    entry = PackageMappingSource.from_mapping(raw_entry)
+            elif isinstance(raw_entry, str | Path):
+                entry = PackageMappingSource.from_file(_parse_str_as_abs_path(raw_entry, base_path))
+            else:
+                raise ValueError(
+                    f"Unrecognized entry for extra-package-mappings: {raw_entry} of type {type(raw_entry)}."
+                )
+            result.append(entry)
+        return result
 
 
 class ROSGenerator(GenerateRecipeProtocol):
@@ -131,7 +156,9 @@ def generate_recipe(
         if not robostack_file.is_file():
             robostack_file = Path(__file__).parent.parent.parent / "robostack.yaml"
 
-        package_map_data = load_package_map_data([robostack_file] + backend_config.extra_package_mappings)
+        package_map_data = load_package_map_data(
+            backend_config.extra_package_mappings + [PackageMappingSource.from_file(robostack_file)]
+        )
 
         # Get requirements from package.xml
         package_requirements = package_xml_to_conda_requirements(package_xml, distro, host_platform, package_map_data)

backends/pixi-build-ros/src/pixi_build_ros/ros_generator.py

@@ -1,7 +1,7 @@
 import os
 from itertools import chain
 from pathlib import Path
-from typing import Any, List, Dict
+from typing import Any, Dict, List, Optional, Sequence, Union
 
 import yaml
 from catkin_pkg.package import Package as CatkinPackage, parse_package_string
@@ -15,6 +15,38 @@
 PackageMapEntry = Dict[str, List[str] | Dict[str, List[str]]]
 
 
+class PackageMappingSource:
+    """Describes where additional package mapping data comes from."""
+
+    def __init__(self, mapping: Dict[str, PackageMapEntry]):
+        if mapping is None:
+            raise ValueError("PackageMappingSource mapping cannot be null.")
+        if not isinstance(mapping, dict):
+            raise TypeError("PackageMappingSource mapping must be a dictionary.")
+        # Copy to keep the source immutable for callers.
+        self.mapping: Dict[str, PackageMapEntry] = dict(mapping)
+
+    @classmethod
+    def from_mapping(cls, mapping: Dict[str, PackageMapEntry]) -> "PackageMappingSource":
+        """Create a source directly from a mapping dictionary."""
+        return cls(mapping)
+
+    @classmethod
+    def from_file(cls, file_path: Union[str, Path]) -> "PackageMappingSource":
+        """Create a source from a mapping file."""
+        path = Path(file_path)
+        if not path.exists():
+            raise ValueError(f"Additional package map file '{path}' not found.")
+        with open(path, "r") as f:
+            data = yaml.safe_load(f) or {}
+        if not isinstance(data, dict):
+            raise TypeError("Expected package map file to contain a dictionary.")
+        return cls(data)
+
+    def get_package_mapping(self) -> Dict[str, PackageMapEntry]:
+        return dict(self.mapping)
+
+
 def get_build_input_globs(config: Any, editable: bool) -> List[str]:
     """Get build input globs for ROS package."""
     base_globs = [
@@ -70,15 +102,12 @@ def convert_package_xml_to_catkin_package(package_xml_content: str) -> CatkinPac
     return package_xml
 
 
-def load_package_map_data(package_map_path: List[Path]) -> Dict[str, PackageMapEntry]:
-    """Load the package map data from the given path."""
-    result = {}
-    for path in package_map_path:
-        if not path.exists():
-            raise ValueError(f"Additional package map path '{path}' not found.")
-        # this blindly overwrites the data, but that's ok for now'
-        with open(path, "r") as f:
-            result.update(yaml.safe_load(f))
+def load_package_map_data(package_map_sources: List[PackageMappingSource]) -> Dict[str, PackageMapEntry]:
+    """Load and merge package map data from files and inline mappings."""
+
+    result: Dict[str, PackageMapEntry] = {}
+    for source in reversed(package_map_sources):
+        result.update(source.get_package_mapping())
     return result
 
 

backends/pixi-build-ros/src/pixi_build_ros/utils.py

@@ -3,7 +3,11 @@
 
 import pytest
 
-from pixi_build_ros.utils import PackageMapEntry, load_package_map_data
+from pixi_build_ros.utils import (
+    PackageMapEntry,
+    load_package_map_data,
+    PackageMappingSource,
+)
 
 
 @pytest.fixture
@@ -22,4 +26,4 @@ def package_xmls(test_data_dir) -> Path:
 def package_map() -> Dict[str, PackageMapEntry]:
     """Load the package map"""
     robostack_file = Path(__file__).parent.parent / "robostack.yaml"
-    return load_package_map_data([robostack_file])
+    return load_package_map_data([PackageMappingSource.from_file(robostack_file)])

backends/pixi-build-ros/tests/conftest.py

@@ -4,21 +4,52 @@
 from pixi_build_backend.types.platform import Platform
 from pixi_build_backend.types.project_model import ProjectModelV1
 
-from pixi_build_ros.ros_generator import ROSGenerator
-from pixi_build_ros.utils import load_package_map_data
+from pixi_build_ros.ros_generator import ROSGenerator, ROSBackendConfig
+from pixi_build_ros.utils import load_package_map_data, PackageMappingSource
 
 
 def test_package_loading(test_data_dir: Path):
     """Load the package map with overwrites."""
     robostack_file = Path(__file__).parent.parent / "robostack.yaml"
     other_package_map = test_data_dir / "other_package_map.yaml"
-    result = load_package_map_data([robostack_file, other_package_map])
+    result = load_package_map_data(
+        [
+            PackageMappingSource.from_file(other_package_map),
+            PackageMappingSource.from_file(robostack_file),
+        ]
+    )
     assert "new_package" in result
     assert result["new_package"]["conda"] == ["new-package"], "Should be added"
     assert result["alsa-oss"]["conda"] == ["other-alsa-oss"], "Should be overwritten"
+    assert "robostack" not in result["alsa-oss"], "Should be overwritten due to priority of package maps"
     assert "zlib" in result, "Should still be present"
 
 
+def test_package_loading_with_inline_mappings(test_data_dir: Path):
+    """Load package map data from a mix of files and inline entries."""
+    robostack_file = Path(__file__).parent.parent / "robostack.yaml"
+    inline_entries = {
+        "inline-package": {"conda": ["inline-conda"]},
+        "inline-ros": {"ros": ["inline-ros"]},
+    }
+    # Create config for ROS backend
+    config = {
+        "distro": "noetic",
+        "noarch": False,
+        "extra-package-mappings": [
+            {"file": str(test_data_dir / "other_package_map.yaml")},
+            {"mapping": inline_entries},
+            robostack_file,
+        ],
+    }
+    parsed_config = ROSBackendConfig.model_validate(config, context={"manifest_root": test_data_dir})
+    result = load_package_map_data(parsed_config.extra_package_mappings)
+
+    assert result["inline-package"]["conda"] == ["inline-conda"]
+    assert result["inline-ros"]["ros"] == ["inline-ros"]
+    assert "zlib" in result, "Should still contain base entries"
+
+
 def test_generate_recipe_with_custom_ros(package_xmls: Path, test_data_dir: Path):
     """Test the generate_recipe function of ROSGenerator."""
     # Create a temporary directory to simulate the package directory
@@ -62,6 +93,47 @@ def test_generate_recipe_with_custom_ros(package_xmls: Path, test_data_dir: Path
         assert "ros-noetic-ros-package-msgs" in req_string
 
 
+def test_generate_recipe_with_inline_package_mappings(package_xmls: Path, test_data_dir: Path):
+    """Inline entries should be merged into the package map."""
+
+    with tempfile.TemporaryDirectory() as temp_dir:
+        temp_path = Path(temp_dir)
+
+        package_xml_source = package_xmls / "custom_ros.xml"
+        package_xml_dest = temp_path / "package.xml"
+        package_xml_dest.write_text(package_xml_source.read_text(encoding="utf-8"))
+
+        model = ProjectModelV1()
+
+        config = {
+            "distro": "noetic",
+            "noarch": False,
+            "extra-package-mappings": [
+                {
+                    "mapping": {
+                        "ros_package": {"ros": ["ros-custom2", "ros-custom2-msgs"]},
+                    }
+                },
+                {"file": str(test_data_dir / "other_package_map.yaml")},
+            ],
+        }
+
+        host_platform = Platform.current()
+
+        generator = ROSGenerator()
+
+        generated_recipe = generator.generate_recipe(
+            model=model,
+            config=config,
+            manifest_path=str(temp_path),
+            host_platform=host_platform,
+        )
+
+        req_string = list(str(req) for req in generated_recipe.recipe.requirements.run)
+        assert "ros-noetic-ros-custom2" in req_string
+        assert "ros-noetic-ros-custom2-msgs" in req_string
+
+
 def test_package_map_does_not_exist(package_xmls: Path, test_data_dir: Path):
     """Test the generate_recipe function of ROSGenerator."""
     # Create a temporary directory to simulate the package directory
@@ -96,4 +168,4 @@ def test_package_map_does_not_exist(package_xmls: Path, test_data_dir: Path):
                 manifest_path=str(temp_path),
                 host_platform=host_platform,
             )
-        assert "Additional package map path" in str(excinfo.value)
+        assert "Additional package map file" in str(excinfo.value)

backends/pixi-build-ros/tests/test_package_map.py

@@ -15,7 +15,12 @@ build-release = "cargo build --release"
 build-ci = "cargo build --profile ci --locked"
 nextest = "cargo nextest run --workspace --all-targets"
 doctest = "cargo test --doc"
-test = [{ task = "nextest" }, { task = "doctest" }]
+pytest-build-ros = "pixi run --manifest-path backends/pixi-build-ros test"
+test = [
+  { task = "nextest" },
+  { task = "doctest" },
+  { task = "pytest-build-ros" },
+]
 generate-matrix = "python scripts/generate-matrix.py"
 
 install-pixi-build-python = { cmd = "cargo install --path crates/pixi-build-python --locked --force" }