From 82119475d87f95b6a3301dad388fc924a84c54aa Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Thu, 28 Jan 2021 15:27:28 -0300 Subject: [PATCH 01/22] Add rosidl generate CLI. Signed-off-by: Michel Hidalgo --- rosidl_cli/completion/rosidl-argcomplete.bash | 19 +++ rosidl_cli/completion/rosidl-argcomplete.zsh | 23 +++ rosidl_cli/package.xml | 30 ++++ rosidl_cli/pytest.ini | 2 + rosidl_cli/resource/package.dsv | 2 + rosidl_cli/resource/rosidl_cli | 0 rosidl_cli/rosidl_cli/__init__.py | 0 rosidl_cli/rosidl_cli/cli.py | 111 ++++++++++++++ rosidl_cli/rosidl_cli/command/__init__.py | 31 ++++ .../rosidl_cli/command/generate/__init__.py | 85 +++++++++++ .../rosidl_cli/command/generate/extensions.py | 57 +++++++ rosidl_cli/rosidl_cli/common.py | 26 ++++ rosidl_cli/rosidl_cli/entry_points.py | 144 ++++++++++++++++++ rosidl_cli/rosidl_cli/extensions.py | 57 +++++++ rosidl_cli/setup.py | 48 ++++++ rosidl_cli/test/test_copyright.py | 23 +++ rosidl_cli/test/test_entry_points.py | 28 ++++ rosidl_cli/test/test_flake8.py | 25 +++ rosidl_cli/test/test_pep257.py | 23 +++ rosidl_cli/test/test_xmllint.py | 23 +++ 20 files changed, 757 insertions(+) create mode 100644 rosidl_cli/completion/rosidl-argcomplete.bash create mode 100644 rosidl_cli/completion/rosidl-argcomplete.zsh create mode 100644 rosidl_cli/package.xml create mode 100644 rosidl_cli/pytest.ini create mode 100644 rosidl_cli/resource/package.dsv create mode 100644 rosidl_cli/resource/rosidl_cli create mode 100644 rosidl_cli/rosidl_cli/__init__.py create mode 100644 rosidl_cli/rosidl_cli/cli.py create mode 100644 rosidl_cli/rosidl_cli/command/__init__.py create mode 100644 rosidl_cli/rosidl_cli/command/generate/__init__.py create mode 100644 rosidl_cli/rosidl_cli/command/generate/extensions.py create mode 100644 rosidl_cli/rosidl_cli/common.py create mode 100644 rosidl_cli/rosidl_cli/entry_points.py create mode 100644 rosidl_cli/rosidl_cli/extensions.py create mode 100644 rosidl_cli/setup.py create mode 100644 rosidl_cli/test/test_copyright.py create mode 100644 rosidl_cli/test/test_entry_points.py create mode 100644 rosidl_cli/test/test_flake8.py create mode 100644 rosidl_cli/test/test_pep257.py create mode 100644 rosidl_cli/test/test_xmllint.py diff --git a/rosidl_cli/completion/rosidl-argcomplete.bash b/rosidl_cli/completion/rosidl-argcomplete.bash new file mode 100644 index 000000000..0262ec2a0 --- /dev/null +++ b/rosidl_cli/completion/rosidl-argcomplete.bash @@ -0,0 +1,19 @@ +# Copyright 2021 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +if type register-python-argcomplete3 > /dev/null 2>&1; then + eval "$(register-python-argcomplete3 rosidl)" +elif type register-python-argcomplete > /dev/null 2>&1; then + eval "$(register-python-argcomplete rosidl)" +fi diff --git a/rosidl_cli/completion/rosidl-argcomplete.zsh b/rosidl_cli/completion/rosidl-argcomplete.zsh new file mode 100644 index 000000000..9a86dbd23 --- /dev/null +++ b/rosidl_cli/completion/rosidl-argcomplete.zsh @@ -0,0 +1,23 @@ +# Copyright 2021 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +autoload -U +X compinit && compinit +autoload -U +X bashcompinit && bashcompinit + +# Get this scripts directory +__rosidl_cli_completion_dir=${0:a:h} +# Just source the bash version, it works in zsh too +source "$__rosidl_cli_completion_dir/rosidl-argcomplete.bash" +# Cleanup +unset __rosidl_cli_completion_dir diff --git a/rosidl_cli/package.xml b/rosidl_cli/package.xml new file mode 100644 index 000000000..e52c9eedd --- /dev/null +++ b/rosidl_cli/package.xml @@ -0,0 +1,30 @@ + + + + rosidl_cli + 0.1.0 + + Command line tools for ROS interface generation. + + Chris Lalancette + Shane Loretz + Apache License 2.0 + + Michel Hidalgo + + python3-argcomplete + python3-importlib-metadata + python3-netifaces + python3-packaging + python3-pkg-resources + + ament_copyright + ament_flake8 + ament_pep257 + ament_xmllint + python3-pytest + + + ament_python + + diff --git a/rosidl_cli/pytest.ini b/rosidl_cli/pytest.ini new file mode 100644 index 000000000..fe55d2ed6 --- /dev/null +++ b/rosidl_cli/pytest.ini @@ -0,0 +1,2 @@ +[pytest] +junit_family=xunit2 diff --git a/rosidl_cli/resource/package.dsv b/rosidl_cli/resource/package.dsv new file mode 100644 index 000000000..9293e2a38 --- /dev/null +++ b/rosidl_cli/resource/package.dsv @@ -0,0 +1,2 @@ +source;share/rosidl_cli/environment/rosidl-argcomplete.bash +source;share/rosidl_cli/environment/rosidl-argcomplete.zsh diff --git a/rosidl_cli/resource/rosidl_cli b/rosidl_cli/resource/rosidl_cli new file mode 100644 index 000000000..e69de29bb diff --git a/rosidl_cli/rosidl_cli/__init__.py b/rosidl_cli/rosidl_cli/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/rosidl_cli/rosidl_cli/cli.py b/rosidl_cli/rosidl_cli/cli.py new file mode 100644 index 000000000..4fda533a1 --- /dev/null +++ b/rosidl_cli/rosidl_cli/cli.py @@ -0,0 +1,111 @@ +# Copyright 2021 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import argparse +import signal + +from rosidl_cli.command.generate import GenerateCommand +from rosidl_cli.common import get_first_line_doc + + +def add_subparsers(parser, cli_name, commands, dest='_command'): + """ + Create argparse subparser for each command. + + The ``cli_name`` is used for the title and description of the + ``add_subparsers`` function call. + + For each command a subparser is created. + If the command has an ``add_arguments`` method it is being called. + + :param parser: the parent argument parser + :type parser: :py:class:`argparse.ArgumentParser` + :param str cli_name: name of the command line command to which the + subparsers are being added + :param commands: each of the commands contributing specific arguments + :type commands: :py:class:`List[Command]` + :param str dest: name of the attribute under which the selected + command will be stored + """ + # add subparser with description of available subparsers + description = '' + + commands = sorted(commands, key=lambda command: command.name) + max_length = max(len(command.name) for command in commands) + for command in commands: + description += '%s %s\n' % ( + command.name.ljust(max_length), + get_first_line_doc(command)) + subparser = parser.add_subparsers( + title='Commands', description=description, + metavar=f'Call `{cli_name} -h` for more detailed usage.') + # use a name which doesn't collide with any argument + # but is readable when shown as part of the the usage information + subparser.dest = ' ' + dest.lstrip('_') + subparser.required = True + + # add extension specific sub-parser with its arguments + for command in commands: + command_parser = subparser.add_parser( + command.name, + description=get_first_line_doc(command), + formatter_class=argparse.RawDescriptionHelpFormatter) + command_parser.set_defaults(**{dest: command}) + if hasattr(command, 'add_arguments'): + command.add_arguments( + command_parser, f'{cli_name} {command.name}') + + return subparser + + +def main(*, script_name='rosidl', argv=None, description=None, commands=None): + if description is None: + description = f'{script_name} is an extensible command-line tool ' \ + 'for ROS interface generation.' + + # top level parser + parser = argparse.ArgumentParser( + description=description, + formatter_class=argparse.RawDescriptionHelpFormatter + ) + + if commands is None: + commands = [GenerateCommand()] + + # add arguments for command extension(s) + add_subparsers( + parser, + script_name, + commands + ) + + # register argcomplete hook if available + try: + from argcomplete import autocomplete + except ImportError: + pass + else: + autocomplete(parser, exclude=['-h', '--help']) + + # parse the command line arguments + args = parser.parse_args(args=argv) + + # call the main method of the command + try: + rc = args._command.main(parser=parser, args=args) + except KeyboardInterrupt: + rc = signal.SIGINT + except (ValueError, RuntimeError) as e: + rc = str(e) + return rc diff --git a/rosidl_cli/rosidl_cli/command/__init__.py b/rosidl_cli/rosidl_cli/command/__init__.py new file mode 100644 index 000000000..d62b4f261 --- /dev/null +++ b/rosidl_cli/rosidl_cli/command/__init__.py @@ -0,0 +1,31 @@ +# Copyright 2021 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +class Command: + """ + The extension point for 'command' extensions. + + The following methods must be defined: + * `main` + + The following methods can be defined: + * `add_arguments` + """ + + def add_arguments(self, parser, cli_name, *, argv=None): + pass + + def main(self, *, parser, args): + raise NotImplementedError() diff --git a/rosidl_cli/rosidl_cli/command/generate/__init__.py b/rosidl_cli/rosidl_cli/command/generate/__init__.py new file mode 100644 index 000000000..cf5488e26 --- /dev/null +++ b/rosidl_cli/rosidl_cli/command/generate/__init__.py @@ -0,0 +1,85 @@ +# Copyright 2021 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os + +from rosidl_cli.command import Command + +from .extensions import load_type_extensions +from .extensions import load_typesupport_extensions + + +class GenerateCommand(Command): + """Generate source code from ROS interface files.""" + + name = 'generate' + + def add_arguments(self, parser, cli_name): + parser.add_argument( + '-o', '--output-path', metavar='PATH', default=os.getcwd(), + help=('Path to directory to hold generated source code files. ' + "Defaults to '.'.")) + parser.add_argument( + '-t', '--type', metavar='TYPE_SPEC', + dest='type_specs', action='append', default=[], + help=('Target type representations for generation, backed by a ' + 'tool extension. Specified by name plus an optional PEP440 ' + 'version specifier set may be provided.')) + parser.add_argument( + '-ts', '--type-support', metavar='TYPESUPPORT_SPEC', + dest='typesupport_specs', action='append', default=[], + help=('Target type supports for generation, backed by a tool ' + 'extension. Specified by name plus an optional PEP440 ' + 'version specifier set may be provided.')) + parser.add_argument( + '-I', '--include-path', metavar='PATH', + dest='include_paths', action='append', default=[], + help='Paths to include dependency interface definition files from.') + parser.add_argument( + 'package_name', help='Name of the package to generate code for') + parser.add_argument( + 'interface-files', metavar='interface_file', nargs='+', + help=('Relative paths to ROS interface definition files. If a path is ' + "prefixed by an absolute path followed by a colon ':', " + 'path resolution is performed against that absolute path.')) + + def main(self, *, parser, args): + extensions = [] + + unspecific_generation = \ + not args.type_specs and not args.typesupport_specs + + if args.type_specs or unspecific_generation: + extensions.extend(load_type_extensions( + specs=args.type_specs, + strict=not unspecific_generation)) + + if args.typesupport_specs or unspecific_generation: + extensions.extend(load_typesupport_extensions( + specs=args.typesupport_specs, + strict=not unspecific_generation)) + + if unspecific_generation and not extensions: + return 'No type nor typesupport extensions were found' + + if len(extensions) > 1: + for extension in extensions: + extension.generate( + args.package_name, args.interface_files, args.include_paths, + output_path=os.path.join(args.output_path, extension.name)) + else: + extensions[0].generate( + args.package_name, args.interface_files, + args.include_paths, args.output_path + ) diff --git a/rosidl_cli/rosidl_cli/command/generate/extensions.py b/rosidl_cli/rosidl_cli/command/generate/extensions.py new file mode 100644 index 000000000..4c976d7e7 --- /dev/null +++ b/rosidl_cli/rosidl_cli/command/generate/extensions.py @@ -0,0 +1,57 @@ +# Copyright 2021 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from rosidl_cli.extensions import Extension +from rosidl_cli.extensions import load_extensions + + +class GenerateCommandExtension(Extension): + """ + The extension point for source code generation. + + The following methods must be defined: + * `generate` + """ + + def generate( + self, + package_name, + interface_files, + include_paths, + output_path + ): + """ + Generate source code. + + Paths to interface definition files are relative paths optionally + prefixed by an absolute path followed by a colon ':', in which case + path resolution is to be performed against that absolute path. + + :param package_name: name of the package to generate source code for + :param interface_files: list of paths to interface definition files + :param include_paths: list of paths to include dependency interface + definition files from. + :param output_path: path to directory to hold generated source code files + """ + raise NotImplementedError() + + +def load_type_extensions(**kwargs): + """Load extensions for type representation source code generation.""" + return load_extensions('rosidl_cli.command.generate.type_extensions', **kwargs) + + +def load_typesupport_extensions(**kwargs): + """Load extensions for type support source code generation.""" + return load_extensions('rosidl_cli.command.generate.typesupport_extensions', **kwargs) diff --git a/rosidl_cli/rosidl_cli/common.py b/rosidl_cli/rosidl_cli/common.py new file mode 100644 index 000000000..0da986bc3 --- /dev/null +++ b/rosidl_cli/rosidl_cli/common.py @@ -0,0 +1,26 @@ +# Copyright 2021 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +def get_first_line_doc(any_type): + if not any_type.__doc__: + return '' + lines = any_type.__doc__.splitlines() + if not lines: + return '' + if lines[0]: + line = lines[0] + elif len(lines) > 1: + line = lines[1] + return line.strip().rstrip('.') diff --git a/rosidl_cli/rosidl_cli/entry_points.py b/rosidl_cli/rosidl_cli/entry_points.py new file mode 100644 index 000000000..1d29470d8 --- /dev/null +++ b/rosidl_cli/rosidl_cli/entry_points.py @@ -0,0 +1,144 @@ +# Copyright 2021 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import logging +import re + +try: + import importlib.metadata as importlib_metadata +except ModuleNotFoundError: + import importlib_metadata + +from packaging.specifiers import BaseSpecifier +from packaging.specifiers import InvalidSpecifier +from packaging.specifiers import SpecifierSet +from packaging.version import Version + + +logger = logging.getLogger(__name__) + + +def normalize_entry_point_specs(specs): + """ + Normalize a collection of entry point specifications. + + A normalized collection of entry point specifications is mapping from entry + point names to entry point version :py:class:`BaseSpecifier` instances. + A denormalized collection may also be an iterable of entry point specification + strings i.e. an entry point name optionally followed by a PEP440 version specifier. + + :param specs: a (de)normalized collection of entry point specifications + :type: iterable or dict + :returns: mapping of entry point name to :py:class:`BaseSpecifier` instances + :rtype: dict + """ + if not isinstance(specs, dict): + pattern = re.compile(r'^([\w.-]+)(.*)') + + normalized_specs = {} + for spec in specs: + match = pattern.match(spec) + if match is None: + raise ValueError(f'Invalid name in spec: {spec}') + name = match[1] + try: + specifier = SpecifierSet(match[2]) + except InvalidSpecifier: + raise ValueError(f'Invalid specifier in spec: {spec}') + if name in normalized_specs: + specifier &= normalized_specs[name] + normalized_specs[name] = specifier + specs = normalized_specs + for specifier in specs.values(): + if not isinstance(specifier, BaseSpecifier): + raise TypeError(f"'{specifier}' is not an specifier") + return specs + + +def get_entry_points(group_name, *, specs=None, strict=False): + """ + Get entry points for a specific group. + + See :py:function:`normalize_entry_point_specs()` for further reference + on entry point specifications. + + :param str group_name: the name of the entry point group + :param specs: an optional collection of entry point specifications, + :type: iterable or dict + :param bool strict: whether to raise or warn on error + :returns: mapping from entry point names to ``EntryPoint`` instances + :rtype: dict + """ + if specs is not None: + specs = normalize_entry_point_specs(specs) + + entry_points = {} + entry_points_per_group = importlib_metadata.entry_points() + for entry_point in entry_points_per_group.get(group_name, []): + name = entry_point.name + if name in entry_points: + msg = (f"Found duplicate entry point '{name}': " + 'got {entry_point} and {entry_points[name]}') + if strict: + raise RuntimeError(msg) + logger.warning(msg) + continue + if specs: + if name not in specs: + continue + version = Version(entry_point.distro.version) + if version not in specs[name]: + msg = (f"Spec '{name}{specs[name]}'" + f' cannot be met: found {version}') + if strict: + raise RuntimeError(msg) + logger.warning(msg) + continue + entry_points[name] = entry_point + if specs: + pending = set(specs) - set(entry_points) + if pending: + msg = 'Some specs could not be met: ' + ', '.join([ + f'{name}{specs[name]}' for name in pending + ]) + if strict: + raise RuntimeError(msg) + logger.warning(msg) + return entry_points + + +def load_entry_points(group_name, *, strict=False, **kwargs): + """ + Load entry points for a specific group. + + See :py:function:`get_entry_points` for further reference on + additional keyword arguments. + + :param str group_name: the name of the entry point group + :param bool strict: whether to raise or warn on error + :returns: mapping from entry point name to loaded entry point + :rtype: dict + """ + loaded_entry_points = {} + for name, entry_point in get_entry_points( + group_name, strict=strict, **kwargs + ).items(): + try: + loaded_entry_points[name] = entry_point.load() + except Exception as e: # noqa: F841 + msg = f"Failed to load entry point '{name}': {e}" + if strict: + raise RuntimeError(msg) + logger.warning(msg) + return loaded_entry_points diff --git a/rosidl_cli/rosidl_cli/extensions.py b/rosidl_cli/rosidl_cli/extensions.py new file mode 100644 index 000000000..7dee3ec0d --- /dev/null +++ b/rosidl_cli/rosidl_cli/extensions.py @@ -0,0 +1,57 @@ +# Copyright 2021 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import logging + +from rosidl_cli.entry_points import load_entry_points + + +logger = logging.getLogger(__name__) + + +class Extension: + """A generic extension point.""" + + def __init__(self, name): + self.__name = name + + @property + def name(self): + return self.__name + + +def load_extensions(group_name, *, strict=False, **kwargs): + """ + Load extensions for a specific group. + + See :py:function:`load_entry_points` for further reference on + additional keyword arguments. + + :param str group_name: the name of the extension group + :param bool strict: whether to raise or warn on error + :returns: a list of :py:class:`Extension` instances + :rtype: list + """ + extensions = [] + for name, factory in load_entry_points( + group_name, strict=strict, **kwargs + ).items(): + try: + extensions.append(factory(name)) + except Exception as e: + msg = f"Failed to build extension '{name}': {e}" + if strict: + raise RuntimeError(msg) + logger.warning(msg) + return extensions diff --git a/rosidl_cli/setup.py b/rosidl_cli/setup.py new file mode 100644 index 000000000..81b501f9e --- /dev/null +++ b/rosidl_cli/setup.py @@ -0,0 +1,48 @@ +from setuptools import find_packages +from setuptools import setup + +setup( + name='rosidl_cli', + version='0.1.0', + packages=find_packages(exclude=['test']), + extras_require={ + 'completion': ['argcomplete'], + }, + data_files=[ + ('share/ament_index/resource_index/packages', [ + 'resource/rosidl_cli', + ]), + ('share/rosidl_cli', [ + 'package.xml', + 'resource/package.dsv', + ]), + ('share/rosidl_cli/environment', [ + 'completion/rosidl-argcomplete.bash', + 'completion/rosidl-argcomplete.zsh' + ]), + ], + zip_safe=False, + author='Michel Hidalgo', + author_email='michel@ekumenlabs.com', + maintainer='Chris Lalancette, Shane Loretz', + maintainer_email='clalancette@openrobotics.org, sloretz@openrobotics.org', + url='https://github.com/ros2/rosidl/tree/master/rosidl_cli', + download_url='https://github.com/ros2/rosidl/releases', + keywords=[], + classifiers=[ + 'Environment :: Console', + 'Intended Audience :: Developers', + 'License :: OSI Approved :: Apache Software License', + 'Programming Language :: Python', + ], + description='Command line tools for ROS interface generation.', + long_description="""\ +The tooling provides a single command line script for ROS interface source code generation.""", + license='Apache License, Version 2.0', + tests_require=['pytest'], + entry_points={ + 'console_scripts': [ + 'rosidl = rosidl_cli.cli:main', + ], + } +) diff --git a/rosidl_cli/test/test_copyright.py b/rosidl_cli/test/test_copyright.py new file mode 100644 index 000000000..cf0fae31f --- /dev/null +++ b/rosidl_cli/test/test_copyright.py @@ -0,0 +1,23 @@ +# Copyright 2017 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from ament_copyright.main import main +import pytest + + +@pytest.mark.copyright +@pytest.mark.linter +def test_copyright(): + rc = main(argv=['.', 'test']) + assert rc == 0, 'Found errors' diff --git a/rosidl_cli/test/test_entry_points.py b/rosidl_cli/test/test_entry_points.py new file mode 100644 index 000000000..aa030a9b8 --- /dev/null +++ b/rosidl_cli/test/test_entry_points.py @@ -0,0 +1,28 @@ +# Copyright 2021 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import pytest + +from rosidl_cli.entry_points import normalize_entry_point_specs +from rosidl_cli.entry_points import SpecifierSet + + +def test_entry_point_spec_normalization(): + assert normalize_entry_point_specs([]) == {} + + with pytest.raises(ValueError): + normalize_entry_point_specs(['name?']) + + specs = normalize_entry_point_specs(['c', 'cpp==0.1.0']) + assert specs == {'c': SpecifierSet(''), 'cpp': SpecifierSet('==0.1.0')} diff --git a/rosidl_cli/test/test_flake8.py b/rosidl_cli/test/test_flake8.py new file mode 100644 index 000000000..27ee1078f --- /dev/null +++ b/rosidl_cli/test/test_flake8.py @@ -0,0 +1,25 @@ +# Copyright 2017 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from ament_flake8.main import main_with_errors +import pytest + + +@pytest.mark.flake8 +@pytest.mark.linter +def test_flake8(): + rc, errors = main_with_errors(argv=[]) + assert rc == 0, \ + 'Found %d code style errors / warnings:\n' % len(errors) + \ + '\n'.join(errors) diff --git a/rosidl_cli/test/test_pep257.py b/rosidl_cli/test/test_pep257.py new file mode 100644 index 000000000..0e38a6c60 --- /dev/null +++ b/rosidl_cli/test/test_pep257.py @@ -0,0 +1,23 @@ +# Copyright 2017 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from ament_pep257.main import main +import pytest + + +@pytest.mark.linter +@pytest.mark.pep257 +def test_pep257(): + rc = main(argv=[]) + assert rc == 0, 'Found code style errors / warnings' diff --git a/rosidl_cli/test/test_xmllint.py b/rosidl_cli/test/test_xmllint.py new file mode 100644 index 000000000..f46285e71 --- /dev/null +++ b/rosidl_cli/test/test_xmllint.py @@ -0,0 +1,23 @@ +# Copyright 2019 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from ament_xmllint.main import main +import pytest + + +@pytest.mark.linter +@pytest.mark.xmllint +def test_xmllint(): + rc = main(argv=[]) + assert rc == 0, 'Found errors' From 8445319930f87f33ddc504901837d2cea30955e5 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 9 Feb 2021 12:50:31 -0300 Subject: [PATCH 02/22] Adjust rosidl generate interface_files help string Signed-off-by: Michel Hidalgo --- rosidl_cli/rosidl_cli/command/generate/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rosidl_cli/rosidl_cli/command/generate/__init__.py b/rosidl_cli/rosidl_cli/command/generate/__init__.py index cf5488e26..e0206832d 100644 --- a/rosidl_cli/rosidl_cli/command/generate/__init__.py +++ b/rosidl_cli/rosidl_cli/command/generate/__init__.py @@ -49,10 +49,10 @@ def add_arguments(self, parser, cli_name): parser.add_argument( 'package_name', help='Name of the package to generate code for') parser.add_argument( - 'interface-files', metavar='interface_file', nargs='+', - help=('Relative paths to ROS interface definition files. If a path is ' - "prefixed by an absolute path followed by a colon ':', " - 'path resolution is performed against that absolute path.')) + 'interface_files', metavar='interface_file', nargs='+', + help=('Normalized relative path to a ROS interface definition file. ' + "If prefixed by another path followed by a colon ':', " + 'path resolution is performed against such path.')) def main(self, *, parser, args): extensions = [] From 5d899715c09c54f17e670ea78787a2f651bb07e4 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 9 Feb 2021 12:51:03 -0300 Subject: [PATCH 03/22] Deal with pre-3.3.0 importlib.metadata Signed-off-by: Michel Hidalgo --- rosidl_cli/rosidl_cli/entry_points.py | 38 +++++++++++++++------------ 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/rosidl_cli/rosidl_cli/entry_points.py b/rosidl_cli/rosidl_cli/entry_points.py index 1d29470d8..47640c3f2 100644 --- a/rosidl_cli/rosidl_cli/entry_points.py +++ b/rosidl_cli/rosidl_cli/entry_points.py @@ -84,28 +84,32 @@ def get_entry_points(group_name, *, specs=None, strict=False): specs = normalize_entry_point_specs(specs) entry_points = {} - entry_points_per_group = importlib_metadata.entry_points() - for entry_point in entry_points_per_group.get(group_name, []): - name = entry_point.name - if name in entry_points: - msg = (f"Found duplicate entry point '{name}': " - 'got {entry_point} and {entry_points[name]}') - if strict: - raise RuntimeError(msg) - logger.warning(msg) - continue - if specs: - if name not in specs: + # NOTE(hidmic): importlib.metadata.EntryPoint instances + # only expose their Distribution in version 3.3.0 onwards + for dist in importlib_metadata.distributions(): + for entry_point in dist.entry_points: + if entry_point.group != group_name: continue - version = Version(entry_point.distro.version) - if version not in specs[name]: - msg = (f"Spec '{name}{specs[name]}'" - f' cannot be met: found {version}') + name = entry_point.name + if name in entry_points: + msg = (f"Found duplicate entry point '{name}': " + 'got {entry_point} and {entry_points[name]}') if strict: raise RuntimeError(msg) logger.warning(msg) continue - entry_points[name] = entry_point + if specs: + if name not in specs: + continue + version = Version(dist.version) + if version not in specs[name]: + msg = (f"Spec '{name}{specs[name]}'" + f' cannot be met: found {version}') + if strict: + raise RuntimeError(msg) + logger.warning(msg) + continue + entry_points[name] = entry_point if specs: pending = set(specs) - set(entry_points) if pending: From 6c9016038aa394844f11407526b7add065b2f5ba Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 9 Feb 2021 12:51:40 -0300 Subject: [PATCH 04/22] Add helper functions for rosidl generate extensions Signed-off-by: Michel Hidalgo --- .../rosidl_cli/command/generate/helpers.py | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 rosidl_cli/rosidl_cli/command/generate/helpers.py diff --git a/rosidl_cli/rosidl_cli/command/generate/helpers.py b/rosidl_cli/rosidl_cli/command/generate/helpers.py new file mode 100644 index 000000000..909193e96 --- /dev/null +++ b/rosidl_cli/rosidl_cli/command/generate/helpers.py @@ -0,0 +1,67 @@ +# Copyright 2021 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import contextlib +import glob +import json +import os +import tempfile + + +def package_name_from_include_path(path): + return os.path.basename(os.path.dirname(os.path.dirname(path))) + + +def dependencies_from_include_paths(include_paths): + return list({ + f'{package_name_from_include_path(path)}:{path}' + for include_path in include_paths + for path in glob.iglob(f'{include_path}/**/*.idl', recursive=True) + }) + + +def idl_tuples_from_interface_files(interface_files): + idl_tuples = [] + for path in interface_files: + if ':' not in path: + prefix = os.getcwd() + stem = path + else: + prefix, _, stem = path.partition(':') + prefix = os.path.realpath(prefix) + if os.path.isabs(stem): + raise ValueError() + idl_tuples.append(f'{prefix}:{stem}') + return idl_tuples + + +@contextlib.contextmanager +def legacy_generator_arguments_file( + package_name, interface_files, + include_paths, templates_path, + output_path +): + idl_tuples = idl_tuples_from_interface_files(interface_files) + interface_dependencies = dependencies_from_include_paths(include_paths) + with tempfile.NamedTemporaryFile(mode='w') as tmp: + tmp.write(json.dumps({ + 'package_name': package_name, + 'output_dir': output_path, + 'template_dir': templates_path, + 'idl_tuples': idl_tuples, + 'ros_interface_dependencies': interface_dependencies, + 'target_dependencies': [] + })) + tmp.flush() + yield tmp.name From 2dcc401dc3e8f163680bff0c81d97836cbea60ed Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Thu, 11 Feb 2021 19:11:15 -0300 Subject: [PATCH 05/22] Add a few tests and docs Signed-off-by: Michel Hidalgo --- .../rosidl_cli/command/generate/helpers.py | 77 +++++++++++++++---- rosidl_cli/test/rosidl_cli/test_common.py | 33 ++++++++ .../{ => rosidl_cli}/test_entry_points.py | 0 .../rosidl_cli/test_files/bar/msg/Bar.idl | 0 .../test/rosidl_cli/test_generate_helpers.py | 55 +++++++++++++ 5 files changed, 149 insertions(+), 16 deletions(-) create mode 100644 rosidl_cli/test/rosidl_cli/test_common.py rename rosidl_cli/test/{ => rosidl_cli}/test_entry_points.py (100%) create mode 100644 rosidl_cli/test/rosidl_cli/test_files/bar/msg/Bar.idl create mode 100644 rosidl_cli/test/rosidl_cli/test_generate_helpers.py diff --git a/rosidl_cli/rosidl_cli/command/generate/helpers.py b/rosidl_cli/rosidl_cli/command/generate/helpers.py index 909193e96..e787d39e7 100644 --- a/rosidl_cli/rosidl_cli/command/generate/helpers.py +++ b/rosidl_cli/rosidl_cli/command/generate/helpers.py @@ -13,52 +13,97 @@ # limitations under the License. import contextlib -import glob import json -import os +import pathlib import tempfile def package_name_from_include_path(path): - return os.path.basename(os.path.dirname(os.path.dirname(path))) + """ + Derive ROS package name from a ROS interface dependency include path. + + This function assumes ROS interface definition files follow the typical + ``rosidl`` install space layout i.e. 'package_name/subfolder/interface.idl'. + """ + return pathlib.Path(path).parents[1].name def dependencies_from_include_paths(include_paths): + """ + Collect dependencies' ROS interface definition files from include paths. + + Interface definition file paths from dependencies are absolute paths + prefixed by the name of package they belong to followed by a colon ':'. + """ return list({ f'{package_name_from_include_path(path)}:{path}' - for include_path in include_paths - for path in glob.iglob(f'{include_path}/**/*.idl', recursive=True) + for include_path in map(pathlib.Path, include_paths) + for path in include_path.resolve().glob('**/*.idl') }) def idl_tuples_from_interface_files(interface_files): + """ + Express ROS interface definition file paths as IDL tuples. + + An IDL tuple is a relative path prefixed by by an absolute path against + which to resolve it followed by a colon ':'. This function then applies + the following logic: + - If a given path follows this pattern, it is passed through. + - If a given path is prefixed by a relative path, it is resolved + relative to the current working directory. + - If a given path has no prefixes, the current working directory is + used as prefix. + """ idl_tuples = [] for path in interface_files: - if ':' not in path: - prefix = os.getcwd() + path_as_string = str(path) + if ':' not in path_as_string: + prefix = pathlib.Path.cwd() stem = path else: - prefix, _, stem = path.partition(':') - prefix = os.path.realpath(prefix) - if os.path.isabs(stem): - raise ValueError() - idl_tuples.append(f'{prefix}:{stem}') + prefix, _, stem = path_as_string.rpartition(':') + prefix = pathlib.Path(prefix).resolve() + stem = pathlib.Path(stem) + if stem.is_absolute(): + raise ValueError('Interface definition file path ' + f'{stem} cannot be absolute') + idl_tuples.append(f'{prefix}:{stem.as_posix()}') return idl_tuples @contextlib.contextmanager def legacy_generator_arguments_file( - package_name, interface_files, - include_paths, templates_path, + *, + package_name, + interface_files, + include_paths, + templates_path, output_path ): + """ + Generate a temporary rosidl generator arguments file. + + :param package_name: Name of the ROS package for which to generate code + :param interface_files: Relative paths to ROS interface definition files, + optionally prefixed by another absolute or relative path followed by + a colon ':'. The former relative paths will be used as a prototype to + namespace generated code (if applicable). + :param include_paths: Paths where ROS package dependencies' interface + definition files may be found + :param templates_path: Path to the templates directory for the + generator script this arguments are for + :param output_path: Path to the output directory for generated code + """ idl_tuples = idl_tuples_from_interface_files(interface_files) interface_dependencies = dependencies_from_include_paths(include_paths) + output_path = pathlib.Path(output_path).resolve() + templates_path = pathlib.Path(templates_path).resolve() with tempfile.NamedTemporaryFile(mode='w') as tmp: tmp.write(json.dumps({ 'package_name': package_name, - 'output_dir': output_path, - 'template_dir': templates_path, + 'output_dir': str(output_path), + 'template_dir': str(templates_path), 'idl_tuples': idl_tuples, 'ros_interface_dependencies': interface_dependencies, 'target_dependencies': [] diff --git a/rosidl_cli/test/rosidl_cli/test_common.py b/rosidl_cli/test/rosidl_cli/test_common.py new file mode 100644 index 000000000..d2ac4563e --- /dev/null +++ b/rosidl_cli/test/rosidl_cli/test_common.py @@ -0,0 +1,33 @@ +# Copyright 2021 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from rosidl_cli.common import get_first_line_doc + + +def test_getting_first_line_from_docstring(): + """Check it gets the first line.""" + func = test_getting_first_line_from_docstring + line = get_first_line_doc(func) + assert line == 'Check it gets the first line' + + +def test_getting_first_line_from_multiline_docstring(): + """ + Check it really gets the first non-empty line. + + Additional paragraph to please flake8. + """ + func = test_getting_first_line_from_multiline_docstring + line = get_first_line_doc(func) + assert line == 'Check it really gets the first non-empty line' diff --git a/rosidl_cli/test/test_entry_points.py b/rosidl_cli/test/rosidl_cli/test_entry_points.py similarity index 100% rename from rosidl_cli/test/test_entry_points.py rename to rosidl_cli/test/rosidl_cli/test_entry_points.py diff --git a/rosidl_cli/test/rosidl_cli/test_files/bar/msg/Bar.idl b/rosidl_cli/test/rosidl_cli/test_files/bar/msg/Bar.idl new file mode 100644 index 000000000..e69de29bb diff --git a/rosidl_cli/test/rosidl_cli/test_generate_helpers.py b/rosidl_cli/test/rosidl_cli/test_generate_helpers.py new file mode 100644 index 000000000..52c390cc2 --- /dev/null +++ b/rosidl_cli/test/rosidl_cli/test_generate_helpers.py @@ -0,0 +1,55 @@ +# Copyright 2021 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import json +import os +import pathlib + +import pytest + +from rosidl_cli.command.generate.helpers import legacy_generator_arguments_file + + +@pytest.fixture +def current_path(request): + path = pathlib.Path(request.module.__file__) + path = path.resolve() + path = path.parent + cwd = pathlib.Path.cwd() + try: + os.chdir(str(path)) + yield path + finally: + os.chdir(str(cwd)) + + +def test_legacy_generator_arguments_file(current_path): + with legacy_generator_arguments_file( + package_name='foo', + interface_files=['msg/Foo.idl'], + include_paths=['test_files/bar'], + templates_path='templates', + output_path='tmp', + ) as path: + with open(path, 'r') as fd: + args = json.load(fd) + assert args['package_name'] == 'foo' + assert args['output_dir'] == str(current_path / 'tmp') + assert args['template_dir'] == str(current_path / 'templates') + assert args['idl_tuples'] == [f'{current_path}:msg/Foo.idl'] + path_to_dep = pathlib.Path('test_files/bar/msg/Bar.idl') + assert args['ros_interface_dependencies'] == [ + 'bar:' + str(current_path / path_to_dep) + ] + assert not pathlib.Path(path).exists() From f7a9d3a8a6d06ce3985d0b2408930d612c00343a Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Mon, 15 Feb 2021 17:30:21 -0300 Subject: [PATCH 06/22] Add helper function to generate visibility control files Signed-off-by: Michel Hidalgo --- .../rosidl_cli/command/generate/helpers.py | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/rosidl_cli/rosidl_cli/command/generate/helpers.py b/rosidl_cli/rosidl_cli/command/generate/helpers.py index e787d39e7..af5841dfb 100644 --- a/rosidl_cli/rosidl_cli/command/generate/helpers.py +++ b/rosidl_cli/rosidl_cli/command/generate/helpers.py @@ -14,6 +14,7 @@ import contextlib import json +import os import pathlib import tempfile @@ -110,3 +111,31 @@ def legacy_generator_arguments_file( })) tmp.flush() yield tmp.name + + +def generate_visibility_control_file( + *, + package_name, + template_path, + output_path +): + """ + Generate a visibility control file from a template. + + :param package_name: Name of the ROS package for which + to generate the file. + :param template_path: Path to template visibility control file. + May contain @PROJECT_NAME@ and @PROJECT_NAME_UPPER@ placeholders, + to be substituted by the package name, accordingly. + :param output_path: Path to visibility control file after interpolation. + """ + os.makedirs(os.path.dirname(output_path), exist_ok=True) + + with open(template_path, 'r') as fd: + content = fd.read() + + content = content.replace('@PROJECT_NAME@', package_name) + content = content.replace('@PROJECT_NAME_UPPER@', package_name.upper()) + + with open(output_path, 'w') as fd: + fd.write(content) From 3a75bcdbc3e2088ecd9929cdac7ad912b4de1dee Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Fri, 26 Feb 2021 14:32:10 -0300 Subject: [PATCH 07/22] Address peer review comments. Signed-off-by: Michel Hidalgo --- rosidl_cli/package.xml | 2 -- rosidl_cli/rosidl_cli/command/generate/helpers.py | 2 +- rosidl_cli/rosidl_cli/common.py | 10 +++++++--- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/rosidl_cli/package.xml b/rosidl_cli/package.xml index e52c9eedd..f3c46e9ba 100644 --- a/rosidl_cli/package.xml +++ b/rosidl_cli/package.xml @@ -14,9 +14,7 @@ python3-argcomplete python3-importlib-metadata - python3-netifaces python3-packaging - python3-pkg-resources ament_copyright ament_flake8 diff --git a/rosidl_cli/rosidl_cli/command/generate/helpers.py b/rosidl_cli/rosidl_cli/command/generate/helpers.py index af5841dfb..292cbf115 100644 --- a/rosidl_cli/rosidl_cli/command/generate/helpers.py +++ b/rosidl_cli/rosidl_cli/command/generate/helpers.py @@ -26,7 +26,7 @@ def package_name_from_include_path(path): This function assumes ROS interface definition files follow the typical ``rosidl`` install space layout i.e. 'package_name/subfolder/interface.idl'. """ - return pathlib.Path(path).parents[1].name + return pathlib.Path(path).resolve().parents[1].name def dependencies_from_include_paths(include_paths): diff --git a/rosidl_cli/rosidl_cli/common.py b/rosidl_cli/rosidl_cli/common.py index 0da986bc3..c91db5d59 100644 --- a/rosidl_cli/rosidl_cli/common.py +++ b/rosidl_cli/rosidl_cli/common.py @@ -14,10 +14,14 @@ def get_first_line_doc(any_type): - if not any_type.__doc__: + docstring = any_type.__doc__ + if not docstring: return '' - lines = any_type.__doc__.splitlines() - if not lines: + lines = [ + line.strip() for line in + docstring.splitlines() + ] + if not any(lines): return '' if lines[0]: line = lines[0] From d892fc2a4dbf536b7d1cd383dc7a0f9999d0b9f7 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Fri, 26 Feb 2021 15:20:58 -0300 Subject: [PATCH 08/22] Address more peer review comments. Signed-off-by: Michel Hidalgo --- rosidl_cli/rosidl_cli/cli.py | 4 +--- rosidl_cli/rosidl_cli/command/__init__.py | 2 -- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/rosidl_cli/rosidl_cli/cli.py b/rosidl_cli/rosidl_cli/cli.py index 4fda533a1..8cc6bd75d 100644 --- a/rosidl_cli/rosidl_cli/cli.py +++ b/rosidl_cli/rosidl_cli/cli.py @@ -62,9 +62,7 @@ def add_subparsers(parser, cli_name, commands, dest='_command'): description=get_first_line_doc(command), formatter_class=argparse.RawDescriptionHelpFormatter) command_parser.set_defaults(**{dest: command}) - if hasattr(command, 'add_arguments'): - command.add_arguments( - command_parser, f'{cli_name} {command.name}') + command.add_arguments(command_parser, f'{cli_name} {command.name}') return subparser diff --git a/rosidl_cli/rosidl_cli/command/__init__.py b/rosidl_cli/rosidl_cli/command/__init__.py index d62b4f261..3b6c969f2 100644 --- a/rosidl_cli/rosidl_cli/command/__init__.py +++ b/rosidl_cli/rosidl_cli/command/__init__.py @@ -19,8 +19,6 @@ class Command: The following methods must be defined: * `main` - - The following methods can be defined: * `add_arguments` """ From 219869e59a12de12a433642622fba04b6b7a36c1 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Fri, 26 Feb 2021 19:38:29 -0300 Subject: [PATCH 09/22] Simplify get_first_line_doc() Signed-off-by: Michel Hidalgo --- rosidl_cli/rosidl_cli/common.py | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/rosidl_cli/rosidl_cli/common.py b/rosidl_cli/rosidl_cli/common.py index c91db5d59..669cda0d4 100644 --- a/rosidl_cli/rosidl_cli/common.py +++ b/rosidl_cli/rosidl_cli/common.py @@ -14,17 +14,8 @@ def get_first_line_doc(any_type): - docstring = any_type.__doc__ - if not docstring: - return '' - lines = [ - line.strip() for line in - docstring.splitlines() - ] - if not any(lines): - return '' - if lines[0]: - line = lines[0] - elif len(lines) > 1: - line = lines[1] - return line.strip().rstrip('.') + for line in any_type.__doc__.splitlines(): + line = line.strip() + if line: + return line.rstrip('.') + return '' From b148c07b2922abbac67c23a6ea15a5c92221ec5a Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Fri, 26 Feb 2021 19:51:38 -0300 Subject: [PATCH 10/22] Handle no docstring case Signed-off-by: Michel Hidalgo --- rosidl_cli/rosidl_cli/common.py | 9 +++++---- rosidl_cli/test/rosidl_cli/test_common.py | 6 ++++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/rosidl_cli/rosidl_cli/common.py b/rosidl_cli/rosidl_cli/common.py index 669cda0d4..1f94c2c63 100644 --- a/rosidl_cli/rosidl_cli/common.py +++ b/rosidl_cli/rosidl_cli/common.py @@ -14,8 +14,9 @@ def get_first_line_doc(any_type): - for line in any_type.__doc__.splitlines(): - line = line.strip() - if line: - return line.rstrip('.') + if any_type.__doc__: + for line in any_type.__doc__.splitlines(): + line = line.strip() + if line: + return line.rstrip('.') return '' diff --git a/rosidl_cli/test/rosidl_cli/test_common.py b/rosidl_cli/test/rosidl_cli/test_common.py index d2ac4563e..166f8bf0f 100644 --- a/rosidl_cli/test/rosidl_cli/test_common.py +++ b/rosidl_cli/test/rosidl_cli/test_common.py @@ -15,6 +15,12 @@ from rosidl_cli.common import get_first_line_doc +def test_getting_first_line_from_no_docstring(): + func = test_getting_first_line_from_no_docstring + line = get_first_line_doc(func) + assert line == '' + + def test_getting_first_line_from_docstring(): """Check it gets the first line.""" func = test_getting_first_line_from_docstring From 1140a0913c991a5063f515fe1b253546b2e080fb Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Mon, 1 Mar 2021 10:34:02 -0300 Subject: [PATCH 11/22] Drop rosidl.cli.main() optional arguments. Signed-off-by: Michel Hidalgo --- rosidl_cli/rosidl_cli/cli.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/rosidl_cli/rosidl_cli/cli.py b/rosidl_cli/rosidl_cli/cli.py index 8cc6bd75d..8fd99fa23 100644 --- a/rosidl_cli/rosidl_cli/cli.py +++ b/rosidl_cli/rosidl_cli/cli.py @@ -67,10 +67,10 @@ def add_subparsers(parser, cli_name, commands, dest='_command'): return subparser -def main(*, script_name='rosidl', argv=None, description=None, commands=None): - if description is None: - description = f'{script_name} is an extensible command-line tool ' \ - 'for ROS interface generation.' +def main(): + script_name = 'rosidl' + description = f'{script_name} is an extensible command-line tool ' \ + 'for ROS interface generation.' # top level parser parser = argparse.ArgumentParser( @@ -78,8 +78,7 @@ def main(*, script_name='rosidl', argv=None, description=None, commands=None): formatter_class=argparse.RawDescriptionHelpFormatter ) - if commands is None: - commands = [GenerateCommand()] + commands = [GenerateCommand()] # add arguments for command extension(s) add_subparsers( @@ -97,7 +96,7 @@ def main(*, script_name='rosidl', argv=None, description=None, commands=None): autocomplete(parser, exclude=['-h', '--help']) # parse the command line arguments - args = parser.parse_args(args=argv) + args = parser.parse_args() # call the main method of the command try: From e6cc39d9507b5b8e2e95dc874c3d8377d716123e Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Fri, 26 Feb 2021 13:38:56 -0800 Subject: [PATCH 12/22] Remove unused cli_name and argv Signed-off-by: Shane Loretz --- rosidl_cli/rosidl_cli/cli.py | 2 +- rosidl_cli/rosidl_cli/command/__init__.py | 2 +- rosidl_cli/rosidl_cli/command/generate/__init__.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rosidl_cli/rosidl_cli/cli.py b/rosidl_cli/rosidl_cli/cli.py index 8fd99fa23..fa98fbc16 100644 --- a/rosidl_cli/rosidl_cli/cli.py +++ b/rosidl_cli/rosidl_cli/cli.py @@ -62,7 +62,7 @@ def add_subparsers(parser, cli_name, commands, dest='_command'): description=get_first_line_doc(command), formatter_class=argparse.RawDescriptionHelpFormatter) command_parser.set_defaults(**{dest: command}) - command.add_arguments(command_parser, f'{cli_name} {command.name}') + command.add_arguments(command_parser) return subparser diff --git a/rosidl_cli/rosidl_cli/command/__init__.py b/rosidl_cli/rosidl_cli/command/__init__.py index 3b6c969f2..22d035bb5 100644 --- a/rosidl_cli/rosidl_cli/command/__init__.py +++ b/rosidl_cli/rosidl_cli/command/__init__.py @@ -22,7 +22,7 @@ class Command: * `add_arguments` """ - def add_arguments(self, parser, cli_name, *, argv=None): + def add_arguments(self, parser): pass def main(self, *, parser, args): diff --git a/rosidl_cli/rosidl_cli/command/generate/__init__.py b/rosidl_cli/rosidl_cli/command/generate/__init__.py index e0206832d..4ec143a59 100644 --- a/rosidl_cli/rosidl_cli/command/generate/__init__.py +++ b/rosidl_cli/rosidl_cli/command/generate/__init__.py @@ -25,7 +25,7 @@ class GenerateCommand(Command): name = 'generate' - def add_arguments(self, parser, cli_name): + def add_arguments(self, parser): parser.add_argument( '-o', '--output-path', metavar='PATH', default=os.getcwd(), help=('Path to directory to hold generated source code files. ' From f19a759ef1e1d62598fb9b536beacabfaacb8881 Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Fri, 26 Feb 2021 13:39:45 -0800 Subject: [PATCH 13/22] Comment tweaks for clarity Signed-off-by: Shane Loretz --- rosidl_cli/rosidl_cli/cli.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rosidl_cli/rosidl_cli/cli.py b/rosidl_cli/rosidl_cli/cli.py index fa98fbc16..53dc3e773 100644 --- a/rosidl_cli/rosidl_cli/cli.py +++ b/rosidl_cli/rosidl_cli/cli.py @@ -27,7 +27,6 @@ def add_subparsers(parser, cli_name, commands, dest='_command'): ``add_subparsers`` function call. For each command a subparser is created. - If the command has an ``add_arguments`` method it is being called. :param parser: the parent argument parser :type parser: :py:class:`argparse.ArgumentParser` @@ -55,7 +54,7 @@ def add_subparsers(parser, cli_name, commands, dest='_command'): subparser.dest = ' ' + dest.lstrip('_') subparser.required = True - # add extension specific sub-parser with its arguments + # add extension specific sub-sub-parser with its arguments for command in commands: command_parser = subparser.add_parser( command.name, From 687a78ff3aa28fd58f101d9c812c11e6897180ad Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Fri, 26 Feb 2021 13:41:11 -0800 Subject: [PATCH 14/22] Remove unused dest argument No need for ` command`, rosidl has contol over all sub commands and can make sure none adds an argument called _command. Signed-off-by: Shane Loretz --- rosidl_cli/rosidl_cli/cli.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/rosidl_cli/rosidl_cli/cli.py b/rosidl_cli/rosidl_cli/cli.py index 53dc3e773..372304ae6 100644 --- a/rosidl_cli/rosidl_cli/cli.py +++ b/rosidl_cli/rosidl_cli/cli.py @@ -19,7 +19,7 @@ from rosidl_cli.common import get_first_line_doc -def add_subparsers(parser, cli_name, commands, dest='_command'): +def add_subparsers(parser, cli_name, commands): """ Create argparse subparser for each command. @@ -34,8 +34,6 @@ def add_subparsers(parser, cli_name, commands, dest='_command'): subparsers are being added :param commands: each of the commands contributing specific arguments :type commands: :py:class:`List[Command]` - :param str dest: name of the attribute under which the selected - command will be stored """ # add subparser with description of available subparsers description = '' @@ -49,9 +47,7 @@ def add_subparsers(parser, cli_name, commands, dest='_command'): subparser = parser.add_subparsers( title='Commands', description=description, metavar=f'Call `{cli_name} -h` for more detailed usage.') - # use a name which doesn't collide with any argument - # but is readable when shown as part of the the usage information - subparser.dest = ' ' + dest.lstrip('_') + subparser.dest = '_command' subparser.required = True # add extension specific sub-sub-parser with its arguments @@ -60,7 +56,7 @@ def add_subparsers(parser, cli_name, commands, dest='_command'): command.name, description=get_first_line_doc(command), formatter_class=argparse.RawDescriptionHelpFormatter) - command_parser.set_defaults(**{dest: command}) + command_parser.set_defaults(_command=command) command.add_arguments(command_parser) return subparser From 9939a1bb28cc60ed336d02a21b7e3e47bd303216 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Mon, 1 Mar 2021 15:04:58 -0300 Subject: [PATCH 15/22] Drop version specifiers Signed-off-by: Michel Hidalgo --- rosidl_cli/package.xml | 1 - .../rosidl_cli/command/generate/__init__.py | 8 +- rosidl_cli/rosidl_cli/entry_points.py | 92 +++---------------- .../test/rosidl_cli/test_entry_points.py | 28 ------ 4 files changed, 16 insertions(+), 113 deletions(-) delete mode 100644 rosidl_cli/test/rosidl_cli/test_entry_points.py diff --git a/rosidl_cli/package.xml b/rosidl_cli/package.xml index f3c46e9ba..d9a85f7e1 100644 --- a/rosidl_cli/package.xml +++ b/rosidl_cli/package.xml @@ -14,7 +14,6 @@ python3-argcomplete python3-importlib-metadata - python3-packaging ament_copyright ament_flake8 diff --git a/rosidl_cli/rosidl_cli/command/generate/__init__.py b/rosidl_cli/rosidl_cli/command/generate/__init__.py index 4ec143a59..a52a1baf5 100644 --- a/rosidl_cli/rosidl_cli/command/generate/__init__.py +++ b/rosidl_cli/rosidl_cli/command/generate/__init__.py @@ -33,15 +33,11 @@ def add_arguments(self, parser): parser.add_argument( '-t', '--type', metavar='TYPE_SPEC', dest='type_specs', action='append', default=[], - help=('Target type representations for generation, backed by a ' - 'tool extension. Specified by name plus an optional PEP440 ' - 'version specifier set may be provided.')) + help='Target type representations for generation.') parser.add_argument( '-ts', '--type-support', metavar='TYPESUPPORT_SPEC', dest='typesupport_specs', action='append', default=[], - help=('Target type supports for generation, backed by a tool ' - 'extension. Specified by name plus an optional PEP440 ' - 'version specifier set may be provided.')) + help='Target type supports for generation.') parser.add_argument( '-I', '--include-path', metavar='PATH', dest='include_paths', action='append', default=[], diff --git a/rosidl_cli/rosidl_cli/entry_points.py b/rosidl_cli/rosidl_cli/entry_points.py index 47640c3f2..15906c8ad 100644 --- a/rosidl_cli/rosidl_cli/entry_points.py +++ b/rosidl_cli/rosidl_cli/entry_points.py @@ -13,103 +13,39 @@ # limitations under the License. import logging -import re try: import importlib.metadata as importlib_metadata except ModuleNotFoundError: import importlib_metadata -from packaging.specifiers import BaseSpecifier -from packaging.specifiers import InvalidSpecifier -from packaging.specifiers import SpecifierSet -from packaging.version import Version - logger = logging.getLogger(__name__) -def normalize_entry_point_specs(specs): - """ - Normalize a collection of entry point specifications. - - A normalized collection of entry point specifications is mapping from entry - point names to entry point version :py:class:`BaseSpecifier` instances. - A denormalized collection may also be an iterable of entry point specification - strings i.e. an entry point name optionally followed by a PEP440 version specifier. - - :param specs: a (de)normalized collection of entry point specifications - :type: iterable or dict - :returns: mapping of entry point name to :py:class:`BaseSpecifier` instances - :rtype: dict - """ - if not isinstance(specs, dict): - pattern = re.compile(r'^([\w.-]+)(.*)') - - normalized_specs = {} - for spec in specs: - match = pattern.match(spec) - if match is None: - raise ValueError(f'Invalid name in spec: {spec}') - name = match[1] - try: - specifier = SpecifierSet(match[2]) - except InvalidSpecifier: - raise ValueError(f'Invalid specifier in spec: {spec}') - if name in normalized_specs: - specifier &= normalized_specs[name] - normalized_specs[name] = specifier - specs = normalized_specs - for specifier in specs.values(): - if not isinstance(specifier, BaseSpecifier): - raise TypeError(f"'{specifier}' is not an specifier") - return specs - - def get_entry_points(group_name, *, specs=None, strict=False): """ - Get entry points for a specific group. - - See :py:function:`normalize_entry_point_specs()` for further reference - on entry point specifications. + Get entry points from a specific group. :param str group_name: the name of the entry point group - :param specs: an optional collection of entry point specifications, - :type: iterable or dict + :param list specs: an optional collection of entry point names to retrieve :param bool strict: whether to raise or warn on error :returns: mapping from entry point names to ``EntryPoint`` instances :rtype: dict """ - if specs is not None: - specs = normalize_entry_point_specs(specs) - entry_points = {} - # NOTE(hidmic): importlib.metadata.EntryPoint instances - # only expose their Distribution in version 3.3.0 onwards - for dist in importlib_metadata.distributions(): - for entry_point in dist.entry_points: - if entry_point.group != group_name: - continue - name = entry_point.name - if name in entry_points: - msg = (f"Found duplicate entry point '{name}': " - 'got {entry_point} and {entry_points[name]}') - if strict: - raise RuntimeError(msg) - logger.warning(msg) - continue - if specs: - if name not in specs: - continue - version = Version(dist.version) - if version not in specs[name]: - msg = (f"Spec '{name}{specs[name]}'" - f' cannot be met: found {version}') - if strict: - raise RuntimeError(msg) - logger.warning(msg) - continue - entry_points[name] = entry_point + for entry_point in importlib_metadata.entry_points().get(group_name, []): + name = entry_point.name + if name in entry_points: + msg = (f"Found duplicate entry point '{name}': " + 'got {entry_point} and {entry_points[name]}') + if strict: + raise RuntimeError(msg) + logger.warning(msg) + continue + if specs and name not in specs: + continue + entry_points[name] = entry_point if specs: pending = set(specs) - set(entry_points) if pending: diff --git a/rosidl_cli/test/rosidl_cli/test_entry_points.py b/rosidl_cli/test/rosidl_cli/test_entry_points.py deleted file mode 100644 index aa030a9b8..000000000 --- a/rosidl_cli/test/rosidl_cli/test_entry_points.py +++ /dev/null @@ -1,28 +0,0 @@ -# Copyright 2021 Open Source Robotics Foundation, Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import pytest - -from rosidl_cli.entry_points import normalize_entry_point_specs -from rosidl_cli.entry_points import SpecifierSet - - -def test_entry_point_spec_normalization(): - assert normalize_entry_point_specs([]) == {} - - with pytest.raises(ValueError): - normalize_entry_point_specs(['name?']) - - specs = normalize_entry_point_specs(['c', 'cpp==0.1.0']) - assert specs == {'c': SpecifierSet(''), 'cpp': SpecifierSet('==0.1.0')} From b581585ab48e924efe9c78445301767bd87451fe Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Mon, 1 Mar 2021 15:10:21 -0300 Subject: [PATCH 16/22] Add TODO to use target_dependencies Signed-off-by: Michel Hidalgo --- rosidl_cli/rosidl_cli/command/generate/helpers.py | 1 + 1 file changed, 1 insertion(+) diff --git a/rosidl_cli/rosidl_cli/command/generate/helpers.py b/rosidl_cli/rosidl_cli/command/generate/helpers.py index 292cbf115..cb7be571d 100644 --- a/rosidl_cli/rosidl_cli/command/generate/helpers.py +++ b/rosidl_cli/rosidl_cli/command/generate/helpers.py @@ -107,6 +107,7 @@ def legacy_generator_arguments_file( 'template_dir': str(templates_path), 'idl_tuples': idl_tuples, 'ros_interface_dependencies': interface_dependencies, + # TODO(hidmic): re-enable output file caching 'target_dependencies': [] })) tmp.flush() From 9036d3b9632eac2ea2581a39381e3e58a1c6ade5 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Mon, 1 Mar 2021 15:18:03 -0300 Subject: [PATCH 17/22] Apply minor tweaks Signed-off-by: Michel Hidalgo --- rosidl_cli/rosidl_cli/command/generate/__init__.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/rosidl_cli/rosidl_cli/command/generate/__init__.py b/rosidl_cli/rosidl_cli/command/generate/__init__.py index a52a1baf5..d1e3661a2 100644 --- a/rosidl_cli/rosidl_cli/command/generate/__init__.py +++ b/rosidl_cli/rosidl_cli/command/generate/__init__.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -import os +import pathlib from rosidl_cli.command import Command @@ -21,13 +21,14 @@ class GenerateCommand(Command): - """Generate source code from ROS interface files.""" + """Generate source code from interface definition files.""" name = 'generate' def add_arguments(self, parser): parser.add_argument( - '-o', '--output-path', metavar='PATH', default=os.getcwd(), + '-o', '--output-path', type=pathlib.Path, + metavar='PATH', default=pathlib.Path.cwd(), help=('Path to directory to hold generated source code files. ' "Defaults to '.'.")) parser.add_argument( @@ -39,14 +40,14 @@ def add_arguments(self, parser): dest='typesupport_specs', action='append', default=[], help='Target type supports for generation.') parser.add_argument( - '-I', '--include-path', metavar='PATH', + '-I', '--include-path', type=pathlib.Path, metavar='PATH', dest='include_paths', action='append', default=[], help='Paths to include dependency interface definition files from.') parser.add_argument( 'package_name', help='Name of the package to generate code for') parser.add_argument( 'interface_files', metavar='interface_file', nargs='+', - help=('Normalized relative path to a ROS interface definition file. ' + help=('Normalized relative path to interface definition file. ' "If prefixed by another path followed by a colon ':', " 'path resolution is performed against such path.')) @@ -73,7 +74,7 @@ def main(self, *, parser, args): for extension in extensions: extension.generate( args.package_name, args.interface_files, args.include_paths, - output_path=os.path.join(args.output_path, extension.name)) + output_path=args.output_path / extension.name) else: extensions[0].generate( args.package_name, args.interface_files, From fc87e450831b31fe53d76672b12a09083b257245 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Mon, 1 Mar 2021 16:34:12 -0300 Subject: [PATCH 18/22] Fix specs use cases Signed-off-by: Michel Hidalgo --- rosidl_cli/rosidl_cli/entry_points.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/rosidl_cli/rosidl_cli/entry_points.py b/rosidl_cli/rosidl_cli/entry_points.py index 15906c8ad..9e3948fc5 100644 --- a/rosidl_cli/rosidl_cli/entry_points.py +++ b/rosidl_cli/rosidl_cli/entry_points.py @@ -33,6 +33,8 @@ def get_entry_points(group_name, *, specs=None, strict=False): :returns: mapping from entry point names to ``EntryPoint`` instances :rtype: dict """ + if specs is not None: + specs = set(specs) entry_points = {} for entry_point in importlib_metadata.entry_points().get(group_name, []): name = entry_point.name @@ -47,11 +49,10 @@ def get_entry_points(group_name, *, specs=None, strict=False): continue entry_points[name] = entry_point if specs: - pending = set(specs) - set(entry_points) + pending = specs - set(entry_points) if pending: - msg = 'Some specs could not be met: ' + ', '.join([ - f'{name}{specs[name]}' for name in pending - ]) + msg = 'Some specs could not be met: ' + msg += ', '.join(map(str, pending)) if strict: raise RuntimeError(msg) logger.warning(msg) From 76bd5ccf1295faed85d545b94a104f009dba516e Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Mon, 1 Mar 2021 18:38:32 -0300 Subject: [PATCH 19/22] Address peer review comments Signed-off-by: Michel Hidalgo --- rosidl_cli/rosidl_cli/cli.py | 2 +- rosidl_cli/rosidl_cli/command/generate/__init__.py | 2 +- rosidl_cli/rosidl_cli/command/generate/helpers.py | 6 +++--- rosidl_cli/rosidl_cli/extensions.py | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/rosidl_cli/rosidl_cli/cli.py b/rosidl_cli/rosidl_cli/cli.py index 372304ae6..18eff8042 100644 --- a/rosidl_cli/rosidl_cli/cli.py +++ b/rosidl_cli/rosidl_cli/cli.py @@ -95,7 +95,7 @@ def main(): # call the main method of the command try: - rc = args._command.main(parser=parser, args=args) + rc = args._command.main(args=args) except KeyboardInterrupt: rc = signal.SIGINT except (ValueError, RuntimeError) as e: diff --git a/rosidl_cli/rosidl_cli/command/generate/__init__.py b/rosidl_cli/rosidl_cli/command/generate/__init__.py index d1e3661a2..b65b95f57 100644 --- a/rosidl_cli/rosidl_cli/command/generate/__init__.py +++ b/rosidl_cli/rosidl_cli/command/generate/__init__.py @@ -51,7 +51,7 @@ def add_arguments(self, parser): "If prefixed by another path followed by a colon ':', " 'path resolution is performed against such path.')) - def main(self, *, parser, args): + def main(self, *, args): extensions = [] unspecific_generation = \ diff --git a/rosidl_cli/rosidl_cli/command/generate/helpers.py b/rosidl_cli/rosidl_cli/command/generate/helpers.py index cb7be571d..b5a226a32 100644 --- a/rosidl_cli/rosidl_cli/command/generate/helpers.py +++ b/rosidl_cli/rosidl_cli/command/generate/helpers.py @@ -19,9 +19,9 @@ import tempfile -def package_name_from_include_path(path): +def package_name_from_interface_file_path(path): """ - Derive ROS package name from a ROS interface dependency include path. + Derive ROS package name from a ROS interface definition file path. This function assumes ROS interface definition files follow the typical ``rosidl`` install space layout i.e. 'package_name/subfolder/interface.idl'. @@ -37,7 +37,7 @@ def dependencies_from_include_paths(include_paths): prefixed by the name of package they belong to followed by a colon ':'. """ return list({ - f'{package_name_from_include_path(path)}:{path}' + f'{package_name_from_interface_file_path(path)}:{path}' for include_path in map(pathlib.Path, include_paths) for path in include_path.resolve().glob('**/*.idl') }) diff --git a/rosidl_cli/rosidl_cli/extensions.py b/rosidl_cli/rosidl_cli/extensions.py index 7dee3ec0d..14488df66 100644 --- a/rosidl_cli/rosidl_cli/extensions.py +++ b/rosidl_cli/rosidl_cli/extensions.py @@ -50,7 +50,7 @@ def load_extensions(group_name, *, strict=False, **kwargs): try: extensions.append(factory(name)) except Exception as e: - msg = f"Failed to build extension '{name}': {e}" + msg = f"Failed to instantiate extension '{name}': {e}" if strict: raise RuntimeError(msg) logger.warning(msg) From 0128d9d24ccffb1c3d71e402033de8ba1725ab3e Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 2 Mar 2021 14:11:44 -0300 Subject: [PATCH 20/22] Address final peer review comments. Signed-off-by: Michel Hidalgo --- rosidl_cli/rosidl_cli/entry_points.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rosidl_cli/rosidl_cli/entry_points.py b/rosidl_cli/rosidl_cli/entry_points.py index 9e3948fc5..cc5ee6107 100644 --- a/rosidl_cli/rosidl_cli/entry_points.py +++ b/rosidl_cli/rosidl_cli/entry_points.py @@ -38,6 +38,8 @@ def get_entry_points(group_name, *, specs=None, strict=False): entry_points = {} for entry_point in importlib_metadata.entry_points().get(group_name, []): name = entry_point.name + if specs and name not in specs: + continue if name in entry_points: msg = (f"Found duplicate entry point '{name}': " 'got {entry_point} and {entry_points[name]}') @@ -45,8 +47,6 @@ def get_entry_points(group_name, *, specs=None, strict=False): raise RuntimeError(msg) logger.warning(msg) continue - if specs and name not in specs: - continue entry_points[name] = entry_point if specs: pending = specs - set(entry_points) From b24e4fae433562e8572cb455831fbfdd0b49893a Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 2 Mar 2021 17:43:05 -0300 Subject: [PATCH 21/22] Handle tempfile quirks on Windows. Signed-off-by: Michel Hidalgo --- rosidl_cli/rosidl_cli/command/generate/helpers.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/rosidl_cli/rosidl_cli/command/generate/helpers.py b/rosidl_cli/rosidl_cli/command/generate/helpers.py index b5a226a32..9b9a49ec0 100644 --- a/rosidl_cli/rosidl_cli/command/generate/helpers.py +++ b/rosidl_cli/rosidl_cli/command/generate/helpers.py @@ -100,7 +100,9 @@ def legacy_generator_arguments_file( interface_dependencies = dependencies_from_include_paths(include_paths) output_path = pathlib.Path(output_path).resolve() templates_path = pathlib.Path(templates_path).resolve() - with tempfile.NamedTemporaryFile(mode='w') as tmp: + # NOTE(hidmic): named temporary files cannot be opened twice on Windows, + # so close it and manually remove it when leaving the context + with tempfile.NamedTemporaryFile(mode='w', delete=False) as tmp: tmp.write(json.dumps({ 'package_name': package_name, 'output_dir': str(output_path), @@ -110,8 +112,15 @@ def legacy_generator_arguments_file( # TODO(hidmic): re-enable output file caching 'target_dependencies': [] })) - tmp.flush() - yield tmp.name + path_to_file = pathlib.Path(tmp.name) + path_to_file = path_to_file.resolve() + try: + yield path_to_file + finally: + try: + os.unlink(path_to_file) + except FileNotFoundError: + pass def generate_visibility_control_file( From f5f5e2b1aeb7aed30dc3fa1a556f6b3b1b9977a7 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 2 Mar 2021 18:18:03 -0300 Subject: [PATCH 22/22] Use os.path.abspath() to get absolute paths. Signed-off-by: Michel Hidalgo --- .../rosidl_cli/command/generate/helpers.py | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/rosidl_cli/rosidl_cli/command/generate/helpers.py b/rosidl_cli/rosidl_cli/command/generate/helpers.py index 9b9a49ec0..643d86813 100644 --- a/rosidl_cli/rosidl_cli/command/generate/helpers.py +++ b/rosidl_cli/rosidl_cli/command/generate/helpers.py @@ -26,7 +26,7 @@ def package_name_from_interface_file_path(path): This function assumes ROS interface definition files follow the typical ``rosidl`` install space layout i.e. 'package_name/subfolder/interface.idl'. """ - return pathlib.Path(path).resolve().parents[1].name + return pathlib.Path(os.path.abspath(path)).parents[1].name def dependencies_from_include_paths(include_paths): @@ -38,8 +38,10 @@ def dependencies_from_include_paths(include_paths): """ return list({ f'{package_name_from_interface_file_path(path)}:{path}' - for include_path in map(pathlib.Path, include_paths) - for path in include_path.resolve().glob('**/*.idl') + for include_path in include_paths + for path in pathlib.Path( + os.path.abspath(include_path) + ).glob('**/*.idl') }) @@ -64,7 +66,7 @@ def idl_tuples_from_interface_files(interface_files): stem = path else: prefix, _, stem = path_as_string.rpartition(':') - prefix = pathlib.Path(prefix).resolve() + prefix = os.path.abspath(prefix) stem = pathlib.Path(stem) if stem.is_absolute(): raise ValueError('Interface definition file path ' @@ -98,22 +100,21 @@ def legacy_generator_arguments_file( """ idl_tuples = idl_tuples_from_interface_files(interface_files) interface_dependencies = dependencies_from_include_paths(include_paths) - output_path = pathlib.Path(output_path).resolve() - templates_path = pathlib.Path(templates_path).resolve() + output_path = os.path.abspath(output_path) + templates_path = os.path.abspath(templates_path) # NOTE(hidmic): named temporary files cannot be opened twice on Windows, # so close it and manually remove it when leaving the context with tempfile.NamedTemporaryFile(mode='w', delete=False) as tmp: tmp.write(json.dumps({ 'package_name': package_name, - 'output_dir': str(output_path), - 'template_dir': str(templates_path), + 'output_dir': output_path, + 'template_dir': templates_path, 'idl_tuples': idl_tuples, 'ros_interface_dependencies': interface_dependencies, # TODO(hidmic): re-enable output file caching 'target_dependencies': [] })) - path_to_file = pathlib.Path(tmp.name) - path_to_file = path_to_file.resolve() + path_to_file = os.path.abspath(tmp.name) try: yield path_to_file finally: