Add support for pure Python Poetry packages (#32)

This allows Poetry packages to be installed by Colcon even if they aren't also ROS packages. This was an artificial limitation in the first place whose only purpose was to add some additional correctness checks for ROS packages.

Fixes #29

README.md

@@ -88,7 +88,7 @@ The format is intended to be mostly identical to the `data_files` field used
 by [setuptools][setuptools-data-files]. The main differences are that copying
 entire directories is supported, and globbing is not yet implemented.
 
-All ROS projects must have, at minimum, these entries in the
+All ROS packages must have, at minimum, these entries in the
 `tool.colcon-poetry-ros.data-files` section (with `{package_name}` replaced
 with the name of your package):
 
@@ -105,16 +105,16 @@ file to the installation.
 
 ## Installing Dependencies
 
-Poetry dependencies are not installed as part of the node build process, but
-they can be installed using a separate tool that's included in this package.
+Poetry dependencies are not installed as part of the build process, but they
+can be installed using a separate tool that's included in this package.
 
 ```bash
 python3 -m colcon_poetry_ros.dependencies.install --base-paths <path to your nodes>
 ```
 
-This command installs each node's dependencies to Colcon's base install
-directory. This means that your dependencies live alongside your node's code
-after it's built, isolated from the rest of your system.
+This command installs each package's dependencies to Colcon's base install
+directory. This means that your dependencies live alongside your package's
+code after it's built, isolated from the rest of your system.
 
 If you customize `colcon build` with the `--install-base` or `--merge-install`
 flags, make sure to provide those to this tool as well.

colcon_poetry_ros/dependencies/install.py

@@ -7,8 +7,8 @@
 from tempfile import NamedTemporaryFile
 
 from colcon_poetry_ros.package_identification.poetry import (
-    PoetryROSPackage,
-    NotAPoetryROSPackage,
+    PoetryPackage,
+    NotAPoetryPackage,
 )
 
 
@@ -33,8 +33,8 @@ def main():
     logging.info("\nDependencies installed!")
 
 
-def _discover_packages(base_paths: List[Path]) -> List[PoetryROSPackage]:
-    projects: List[PoetryROSPackage] = []
+def _discover_packages(base_paths: List[Path]) -> List[PoetryPackage]:
+    projects: List[PoetryPackage] = []
 
     potential_packages = []
     for path in base_paths:
@@ -43,8 +43,8 @@ def _discover_packages(base_paths: List[Path]) -> List[PoetryROSPackage]:
     for path in potential_packages:
         if path.is_dir():
             try:
-                project = PoetryROSPackage(path)
-            except NotAPoetryROSPackage:
+                project = PoetryPackage(path)
+            except NotAPoetryPackage:
                 continue
             else:
                 projects.append(project)
@@ -60,7 +60,7 @@ def _discover_packages(base_paths: List[Path]) -> List[PoetryROSPackage]:
 
 
 def _install_dependencies_via_poetry_bundle(
-    project: PoetryROSPackage, install_base: Path, merge_install: bool
+    project: PoetryPackage, install_base: Path, merge_install: bool
 ) -> None:
     try:
         subprocess.run(
@@ -88,7 +88,7 @@ def _install_dependencies_via_poetry_bundle(
 
 
 def _install_dependencies_via_pip(
-    project: PoetryROSPackage, install_base: Path, merge_install: bool
+    project: PoetryPackage, install_base: Path, merge_install: bool
 ) -> None:
     with NamedTemporaryFile("w") as requirements_file:
         requirements_data = project.get_requirements_txt([])
@@ -127,7 +127,7 @@ def _install_dependencies_via_pip(
 
 def _parse_args() -> argparse.Namespace:
     parser = argparse.ArgumentParser(
-        description="Searches for ROS Poetry packages and installs their dependencies "
+        description="Searches for Poetry packages and installs their dependencies "
         "to a configurable install base"
     )
 

colcon_poetry_ros/package.py

@@ -1,38 +1,38 @@
+import re
 import subprocess
 from pathlib import Path
 import logging
 from tempfile import NamedTemporaryFile
 from typing import List, Set
 
 import toml
+from packaging.version import VERSION_PATTERN
 
+PACKAGE_NAME_PATTERN = r"^([A-Z0-9]|[A-Z0-9][A-Z0-9._-]*[A-Z0-9])$"
+"""Matches on valid package names when run with re.IGNORECASE.
+Pulled from: https://peps.python.org/pep-0508/#names
+"""
 
-class NotAPoetryROSPackage(Exception):
-    """The given directory does not point to a ROS Poetry project"""
 
+class NotAPoetryPackage(Exception):
+    """The given directory does not point to a Poetry project"""
 
-class PoetryROSPackage:
-    """Contains information on a ROS package defined with Poetry"""
+
+class PoetryPackage:
+    """Contains information on a package defined with Poetry"""
 
     def __init__(self, path: Path, logger: logging.Logger = logging):
         """
         :param path: The root path of the Poetry project
         :param logger: A logger to log with!
         """
         self.path = path
+        self.logger = logger
 
         self.pyproject_file = path / "pyproject.toml"
         if not self.pyproject_file.is_file():
             # Poetry requires a pyproject.toml to function
-            raise NotAPoetryROSPackage()
-
-        if not (path / "package.xml").is_file():
-            logger.info(
-                f"Ignoring pyproject.toml in {path} because the directory does "
-                f"not have a package.xml file. This suggests that it is not a ROS "
-                f"package."
-            )
-            raise NotAPoetryROSPackage()
+            raise NotAPoetryPackage()
 
         try:
             self.pyproject = toml.loads(self.pyproject_file.read_text())
@@ -47,7 +47,7 @@ def __init__(self, path: Path, logger: logging.Logger = logging):
                 f"section. The file is likely there to instruct a tool other than "
                 f"Poetry."
             )
-            raise NotAPoetryROSPackage()
+            raise NotAPoetryPackage()
 
         logger.info(f"Project {path} appears to be a Poetry ROS project")
 
@@ -129,12 +129,45 @@ def get_dependencies(self, extras: List[str]) -> Set[str]:
 
         dependencies = set()
 
-        for dependency_str in result.stdout.splitlines():
-            components = dependency_str.split()
-            if len(components) < 2:
-                raise RuntimeError(f"Invalid dependency format: {dependency_str}")
-
-            name, version = components
-            dependencies.add(f"{name}=={version}")
+        for line in result.stdout.splitlines():
+            try:
+                dependency = self._parse_dependency_line(line)
+            except ValueError as ex:
+                self.logger.warning(str(ex))
+            else:
+                dependencies.add(dependency)
 
         return dependencies
+
+    def _parse_dependency_line(self, line: str) -> str:
+        """Makes a best-effort attempt to parse lines from ``poetry show`` as
+        dependencies. Poetry does not have a stable CLI interface, so this logic may
+        not be sufficient now or in the future. A smarter approach is needed.
+
+        :param line: A raw line from ``poetry show``
+        :return: A dependency string in PEP440 format
+        """
+
+        components = line.split()
+        if len(components) < 2:
+            raise ValueError(f"Could not parse line '{line}' as a dependency")
+
+        name = components[0]
+        if re.match(PACKAGE_NAME_PATTERN, name, re.IGNORECASE) is None:
+            raise ValueError(f"Invalid dependency name '{name}'")
+
+        version = None
+
+        # Search for an item that looks like a version. Poetry adds other data in front
+        # of the version number under certain circumstances.
+        for item in components[1:]:
+            if re.match(VERSION_PATTERN, item, re.VERBOSE | re.IGNORECASE) is not None:
+                version = item
+
+        if version is None:
+            raise ValueError(
+                f"For dependency '{name}': Could not find version specification "
+                f"in '{line}'"
+            )
+
+        return f"{name}=={version}"

colcon_poetry_ros/package_augmentation/poetry.py

@@ -8,7 +8,7 @@
     create_dependency_descriptor, logger
 
 from colcon_poetry_ros import config
-from colcon_poetry_ros.package_identification.poetry import PoetryROSPackage
+from colcon_poetry_ros.package_identification.poetry import PoetryPackage
 
 
 class PoetryPackageAugmentation(PackageAugmentationExtensionPoint):
@@ -24,11 +24,11 @@ def __init__(self):
     def augment_package(
         self, desc: PackageDescriptor, *, additional_argument_names=None
     ):
-        if desc.type != "poetry":
+        if desc.type != "poetry.python":
             # Some other identifier claimed this package
             return
 
-        project = PoetryROSPackage(desc.path, logger)
+        project = PoetryPackage(desc.path, logger)
         project.check_lock_file_exists()
 
         if not shutil.which("poetry"):

colcon_poetry_ros/package_identification/poetry.py

@@ -2,7 +2,7 @@
 from colcon_core.package_identification import PackageIdentificationExtensionPoint, logger
 from colcon_core.plugin_system import satisfies_version
 
-from colcon_poetry_ros.package import PoetryROSPackage, NotAPoetryROSPackage
+from colcon_poetry_ros.package import PoetryPackage, NotAPoetryPackage
 
 
 class PoetryPackageIdentification(PackageIdentificationExtensionPoint):
@@ -21,13 +21,13 @@ def __init__(self):
         )
 
     def identify(self, desc: PackageDescriptor):
-        if desc.type is not None and desc.type != "poetry":
+        if desc.type is not None and desc.type != "poetry.python":
             # Some other identifier claimed this package
             return
 
         try:
-            project = PoetryROSPackage(desc.path, logger)
-        except NotAPoetryROSPackage:
+            project = PoetryPackage(desc.path, logger)
+        except NotAPoetryPackage:
             return
 
         if desc.name is not None and desc.name != project.name:

colcon_poetry_ros/task/poetry/build.py

@@ -176,24 +176,6 @@ async def _add_data_files(self) -> int:
             logger.error(f"{_DATA_FILES_TABLE} must be a table")
             return 1
 
-        includes_package_index = False
-        includes_package_manifest = False
-
-        package_index_path = (
-            Path(args.install_base)
-            / "share"
-            / "ament_index"
-            / "resource_index"
-            / "packages"
-            / pkg.name
-        )
-        package_manifest_path = (
-            Path(args.install_base)
-            / "share"
-            / pkg.name
-            / "package.xml"
-        )
-
         for destination, sources in data_files.items():
             if not isinstance(sources, list):
                 logger.error(
@@ -207,26 +189,6 @@ async def _add_data_files(self) -> int:
                 source_path = pkg.path / Path(source)
                 _copy_path(source_path, dest_path)
 
-                resulting_file = dest_path / source_path.name
-                if resulting_file == package_index_path:
-                    includes_package_index = True
-                elif resulting_file == package_manifest_path:
-                    includes_package_manifest = True
-
-        if not includes_package_index:
-            logger.error(
-                f"Packages must provide a marker in the package index as a data file. "
-                f"Add a data file at {package_index_path} with '{pkg.name}' as its "
-                f"content."
-            )
-            return 1
-        if not includes_package_manifest:
-            logger.error(
-                f"Packages must provide the package manifest as a data file. Add your "
-                f"package.xml as a data path at {package_manifest_path}."
-            )
-            return 1
-
         return 0
 
 

setup.cfg

@@ -25,6 +25,7 @@ packages = find:
 install_requires =
     colcon-core~=0.6
     toml~=0.10
+    packaging
     setuptools
 zip_safe = true