From a685ec8454dce73cb40d4d5fe72ee8e5e3d7823f Mon Sep 17 00:00:00 2001 From: Clara Berendsen Date: Thu, 30 Nov 2023 11:41:45 -0300 Subject: [PATCH 01/34] Pin version of empy to <4 (#603) There are breaking changes in 4.0 which require changes to colcon-core. --- setup.cfg | 2 +- stdeb.cfg | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/setup.cfg b/setup.cfg index 0281aeebf..04cae8339 100644 --- a/setup.cfg +++ b/setup.cfg @@ -29,7 +29,7 @@ python_requires = >=3.6 install_requires = coloredlogs; sys_platform == 'win32' distlib - EmPy + EmPy<4 importlib-metadata; python_version < "3.8" packaging # the pytest dependency and its extensions are provided for convenience diff --git a/stdeb.cfg b/stdeb.cfg index 9c4793cd9..08bafc8ed 100644 --- a/stdeb.cfg +++ b/stdeb.cfg @@ -1,6 +1,6 @@ [colcon-core] No-Python2: -Depends3: python3-distlib, python3-empy, python3-packaging, python3-pytest, python3-setuptools, python3 (>= 3.8) | python3-importlib-metadata +Depends3: python3-distlib, python3-empy (<4), python3-packaging, python3-pytest, python3-setuptools, python3 (>= 3.8) | python3-importlib-metadata Recommends3: python3-pytest-cov Suggests3: python3-pytest-repeat, python3-pytest-rerunfailures Suite: focal jammy bullseye bookworm From 8836d3a7f8750c1e41e54970c4cae948f0ce1f7d Mon Sep 17 00:00:00 2001 From: mosfet80 Date: Thu, 30 Nov 2023 16:01:14 +0100 Subject: [PATCH 02/34] Update to actions/checkout@v4 (#591) --- .github/workflows/ci.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 46eba0d4c..75aa10486 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -12,7 +12,7 @@ jobs: strategy: ${{steps.load.outputs.strategy}} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: repository: colcon/ci - id: load @@ -24,7 +24,7 @@ jobs: runs-on: ${{matrix.os}} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: actions/setup-python@v4 with: python-version: ${{matrix.python}} @@ -37,7 +37,7 @@ jobs: runs-on: ${{matrix.os}} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: actions/setup-python@v4 with: python-version: ${{matrix.python}} From 1e87212750ddd4bf24b0b17383a157b451aceedb Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Thu, 30 Nov 2023 09:05:27 -0600 Subject: [PATCH 03/34] 0.15.1 --- colcon_core/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/colcon_core/__init__.py b/colcon_core/__init__.py index 21c54fc4f..fdf5d6ab5 100644 --- a/colcon_core/__init__.py +++ b/colcon_core/__init__.py @@ -1,4 +1,4 @@ # Copyright 2016-2020 Dirk Thomas # Licensed under the Apache License, Version 2.0 -__version__ = '0.15.0' +__version__ = '0.15.1' From d29f38d5e9f6b3043a77bbe77f96f120eead0687 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Wed, 17 Jan 2024 15:50:54 -0600 Subject: [PATCH 04/34] Use a normalized package name in Python build test (#606) It appears that newer setuptools versions are no longer normalizing the package name. Rather than update the name that we're looking for, we should just use a normalized name to begin with so that the test works with both old and newer setuptools versions. --- test/test_build_python.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_build_python.py b/test/test_build_python.py index 27060ef16..630935f53 100644 --- a/test/test_build_python.py +++ b/test/test_build_python.py @@ -76,7 +76,7 @@ def _test_build_package(tmp_path_str, *, symlink_install): (pkg.path / 'setup.py').write_text( 'from setuptools import setup\n' 'setup(\n' - ' name="test_package",\n' + ' name="test-package",\n' ' packages=["my_module"],\n' ')\n' ) From 500dec0a3c64fea0700c5e648b9f0e3b3126371f Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Wed, 17 Jan 2024 16:05:39 -0600 Subject: [PATCH 05/34] Fix entry point discovery on Python < 3.10 (#604) It seems that the behavior of the importlib.metadata.entry_points function changed in Python 3.10 to automatically de-duplicate distributions, but prior to that the "shadowed" distributions were also enumerated. This change specifically ignores "shadowed" distributions so that they aren't identified as extension point overwrites. --- colcon_core/extension_point.py | 24 ++++++++++++++++-------- test/test_extension_point.py | 10 +++++++--- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/colcon_core/extension_point.py b/colcon_core/extension_point.py index ff348ba5b..c724d2213 100644 --- a/colcon_core/extension_point.py +++ b/colcon_core/extension_point.py @@ -37,6 +37,15 @@ EXTENSION_POINT_GROUP_NAME = 'colcon_core.extension_point' +def _get_unique_distributions(): + seen = set() + for dist in distributions(): + dist_name = dist.metadata['Name'] + if dist_name not in seen: + seen.add(dist_name) + yield dist + + def get_all_extension_points(): """ Get all extension points related to `colcon` and any of its extensions. @@ -51,12 +60,7 @@ def get_all_extension_points(): colcon_extension_points.setdefault(EXTENSION_POINT_GROUP_NAME, None) entry_points = defaultdict(dict) - seen = set() - for dist in distributions(): - dist_name = dist.metadata['Name'] - if dist_name in seen: - continue - seen.add(dist_name) + for dist in _get_unique_distributions(): for entry_point in dist.entry_points: # skip groups which are not registered as extension points if entry_point.group not in colcon_extension_points: @@ -70,7 +74,7 @@ def get_all_extension_points(): f"from '{dist._path}' " f"overwriting '{previous}'") entry_points[entry_point.group][entry_point.name] = \ - (entry_point.value, dist_name, dist.version) + (entry_point.value, dist.metadata['Name'], dist.version) return entry_points @@ -87,7 +91,11 @@ def get_extension_points(group): # Python 3.10 and newer query = entry_points(group=group) except TypeError: - query = entry_points().get(group, ()) + query = ( + entry_point + for dist in _get_unique_distributions() + for entry_point in dist.entry_points + if entry_point.group == group) for entry_point in query: if entry_point.name in extension_points: previous_entry_point = extension_points[entry_point.name] diff --git a/test/test_extension_point.py b/test/test_extension_point.py index 7111b7960..96e58a0de 100644 --- a/test/test_extension_point.py +++ b/test/test_extension_point.py @@ -54,8 +54,8 @@ def iter_entry_points(*, group=None): def distributions(): return [ - Dist(iter_entry_points(group='group1')), - Dist([EntryPoint('extC', 'eC', Group2.name)]), + Dist([Group1, ExtA, ExtB]), + Dist([Group2, EntryPoint('extC', 'eC', Group2.name)]), Dist([EntryPoint('extD', 'eD', 'groupX')]), ] @@ -71,7 +71,11 @@ def test_all_extension_points(): ): # successfully load a known entry point extension_points = get_all_extension_points() - assert set(extension_points.keys()) == {'group1', 'group2'} + assert set(extension_points.keys()) == { + EXTENSION_POINT_GROUP_NAME, + 'group1', + 'group2', + } assert set(extension_points['group1'].keys()) == {'extA', 'extB'} assert extension_points['group1']['extA'][0] == 'eA' From c024d98157cc70642b6331747bc18d59369dd5bb Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Fri, 19 Jan 2024 13:37:09 -0600 Subject: [PATCH 06/34] Update setuptools deprecations AGAIN (#607) It seems that setuptools has made changes which require us to update our deprecation warning suppression. I think it actually changed twice: once that regressed the stacklevel for the warning, and another time which changed the message entirely. TIL that suppressing setuptools warnings is akin to whack-a-mole. --- setup.cfg | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 04cae8339..34eea7094 100644 --- a/setup.cfg +++ b/setup.cfg @@ -68,8 +68,9 @@ exclude = filterwarnings = error # Suppress deprecation warnings in other packages + ignore:Deprecated call to `pkg_resources.declare_namespace\('paste'\)`:: ignore:lib2to3 package is deprecated::scspell - ignore:pkg_resources is deprecated as an API::colcon_core.entry_point + ignore:pkg_resources is deprecated as an API:: ignore:SelectableGroups dict interface is deprecated::flake8 ignore:The loop argument is deprecated::asyncio ignore:Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated::pydocstyle From 4f13c449984a60c618cd9ae263ffe50a101a4c19 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Fri, 19 Jan 2024 13:37:23 -0600 Subject: [PATCH 07/34] Add support for flake8 v6 (#608) It seems that flake8 v6 started checking indentation of multi-line string literals, but it doesn't seem to work correctly for f-strings. --- colcon_core/executor/__init__.py | 2 +- setup.cfg | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/colcon_core/executor/__init__.py b/colcon_core/executor/__init__.py index bba60f3dc..aca035758 100644 --- a/colcon_core/executor/__init__.py +++ b/colcon_core/executor/__init__.py @@ -244,7 +244,7 @@ def add_executor_arguments(parser): group.add_argument( '--executor', type=str, choices=keys, default=default, help=f'The executor to process all packages (default: {default})' - f'{descriptions}') + f'{descriptions}') # noqa: E131 for priority in extensions.keys(): extensions_same_prio = extensions[priority] diff --git a/setup.cfg b/setup.cfg index 34eea7094..99e4626d4 100644 --- a/setup.cfg +++ b/setup.cfg @@ -44,7 +44,7 @@ zip_safe = false [options.extras_require] test = - flake8>=3.6.0,<6 + flake8>=3.6.0,<7 flake8-blind-except flake8-builtins flake8-class-newline From 291f16c9c0bd646f541eaa80f0875f77b83bdc25 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Mon, 22 Jan 2024 09:57:25 -0600 Subject: [PATCH 08/34] 0.15.2 --- colcon_core/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/colcon_core/__init__.py b/colcon_core/__init__.py index fdf5d6ab5..4654486c3 100644 --- a/colcon_core/__init__.py +++ b/colcon_core/__init__.py @@ -1,4 +1,4 @@ # Copyright 2016-2020 Dirk Thomas # Licensed under the Apache License, Version 2.0 -__version__ = '0.15.1' +__version__ = '0.15.2' From 6529b6b1ff3d3b29422a38e2d99fee53de58daff Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Fri, 2 Feb 2024 15:25:03 -0600 Subject: [PATCH 09/34] Don't write bytecode when invoking Python tests (#611) This should help to avoid writing compiled bytecode into the source directories of Python packages during test invocation. --- colcon_core/task/python/test/__init__.py | 3 +++ test/spell_check.words | 1 + 2 files changed, 4 insertions(+) diff --git a/colcon_core/task/python/test/__init__.py b/colcon_core/task/python/test/__init__.py index ef5298bc1..74a5ed2b8 100644 --- a/colcon_core/task/python/test/__init__.py +++ b/colcon_core/task/python/test/__init__.py @@ -73,6 +73,9 @@ async def test(self, *, additional_hooks=None): # noqa: D102 logger.log(1, f"test.step() by extension '{key}'") try: + if 'PYTHONDONTWRITEBYTECODE' not in env: + env = dict(env) + env['PYTHONDONTWRITEBYTECODE'] = '1' return await extension.step(self.context, env, setup_py_data) except Exception as e: # noqa: F841 # catch exceptions raised in python testing step extension diff --git a/test/spell_check.words b/test/spell_check.words index a1cb3d99c..28095ef9e 100644 --- a/test/spell_check.words +++ b/test/spell_check.words @@ -79,6 +79,7 @@ purelib pydocstyle pytest pytests +pythondontwritebytecode pythonpath pythonscriptspath pythonwarnings From ad0feb4816e71abc70e86900de3cc2713dbcafd6 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Fri, 2 Feb 2024 18:52:32 -0600 Subject: [PATCH 10/34] Refactor python build test to improve coverage (#612) Switching to a parametrized approach also significantly reduces duplicated code in the test. --- test/spell_check.words | 2 + test/test_build_python.py | 212 ++++++++++++++++++-------------------- 2 files changed, 104 insertions(+), 110 deletions(-) diff --git a/test/spell_check.words b/test/spell_check.words index 28095ef9e..d44f7a23b 100644 --- a/test/spell_check.words +++ b/test/spell_check.words @@ -47,6 +47,7 @@ hookimpl hookwrapper https importlib +importorskip isatty iterdir junit @@ -56,6 +57,7 @@ lineno linter linux lstrip +minversion mkdtemp monkeypatch namedtuple diff --git a/test/test_build_python.py b/test/test_build_python.py index 630935f53..7780a5677 100644 --- a/test/test_build_python.py +++ b/test/test_build_python.py @@ -45,17 +45,32 @@ def monkey_patch_put_event_into_queue(monkeypatch): ) -def _test_build_package(tmp_path_str, *, symlink_install): +def _test_build_package( + tmp_path_str, *, symlink_install, setup_cfg, libexec_pattern, data_files +): + assert not libexec_pattern or setup_cfg, \ + 'The libexec pattern requires use of setup.cfg' + + if setup_cfg and data_files: + pytest.importorskip('setuptools', minversion='40.5.0') + event_loop = new_event_loop() asyncio.set_event_loop(event_loop) try: tmp_path = Path(tmp_path_str) python_build_task = PythonBuildTask() package = PackageDescriptor(tmp_path / 'src') - package.name = 'test_package' + package.name = 'test-package' package.type = 'python' package.metadata['get_python_setup_options'] = lambda _: { 'packages': ['my_module'], + **( + { + 'data_files': [ + ('share/test_package', ['test-resource']), + ] + } if data_files else {} + ) } context = TaskContext( @@ -73,15 +88,56 @@ def _test_build_package(tmp_path_str, *, symlink_install): pkg = python_build_task.context.pkg pkg.path.mkdir(exist_ok=True) - (pkg.path / 'setup.py').write_text( - 'from setuptools import setup\n' - 'setup(\n' - ' name="test-package",\n' - ' packages=["my_module"],\n' - ')\n' - ) + if setup_cfg: + (pkg.path / 'setup.py').write_text( + 'from setuptools import setup\n' + 'setup()\n' + ) + (pkg.path / 'setup.cfg').write_text( + '[metadata]\n' + 'name = test-package\n' + '[options]\n' + 'packages = find:\n' + '[options.entry_points]\n' + 'console_scripts =\n' + ' my_command = my_module:main\n' + + ( + '[develop]\n' + 'script-dir=$base/lib/test_package\n' + '[install]\n' + 'install-scripts=$base/lib/test_package\n' + if libexec_pattern else '' + ) + ( + '[options.data_files]\n' + 'share/test_package = test-resource\n' + if data_files else '' + ) + ) + else: + (pkg.path / 'setup.py').write_text( + 'from setuptools import setup\n' + 'setup(\n' + ' name="test-package",\n' + ' packages=["my_module"],\n' + ' entry_points={\n' + ' "console_scripts": ["my_command = my_module:main"],\n' + ' },\n' + + ( + ' data_files=[\n' + ' ("share/test_package", [\n' + ' "test-resource",\n' + ' ]),\n' + ' ],\n' + if data_files else '' + ) + + ')\n' + ) (pkg.path / 'my_module').mkdir(exist_ok=True) - (pkg.path / 'my_module' / '__init__.py').touch() + (pkg.path / 'test-resource').touch() + (pkg.path / 'my_module' / '__init__.py').write_text( + 'def main():\n' + ' print("Hello, World!")\n' + ) src_base = Path(python_build_task.context.args.path) @@ -94,108 +150,44 @@ def _test_build_package(tmp_path_str, *, symlink_install): build_base = Path(python_build_task.context.args.build_base) assert build_base.rglob('my_module/__init__.py') - return Path(python_build_task.context.args.install_base) + install_base = Path(python_build_task.context.args.install_base) + assert symlink_install == any(install_base.rglob( + 'test-package.egg-link')) + assert symlink_install != any(install_base.rglob( + 'PKG-INFO')) + assert libexec_pattern == any(install_base.rglob( + 'lib/test_package/my_command*')) + assert libexec_pattern != ( + any(install_base.rglob('bin/my_command*')) or + any(install_base.rglob('Scripts/my_command*'))) + assert data_files == any(install_base.rglob( + 'share/test_package/test-resource')) + + if not symlink_install: + pkg_info, = install_base.rglob('PKG-INFO') + assert 'Name: test-package' in pkg_info.read_text().splitlines() finally: event_loop.close() -def test_build_package(): - with TemporaryDirectory(prefix='test_colcon_') as tmp_path_str: - install_base = _test_build_package(tmp_path_str, symlink_install=False) - - assert 1 == len(list(install_base.rglob('my_module/__init__.py'))) - - pkg_info, = install_base.rglob('PKG-INFO') - assert 'Name: test-package' in pkg_info.read_text().splitlines() - - -def test_build_package_symlink(): - with TemporaryDirectory(prefix='test_colcon_') as tmp_path_str: - install_base = _test_build_package(tmp_path_str, symlink_install=True) - - assert 1 == len(list(install_base.rglob('test-package.egg-link'))) - - -def test_build_package_symlink_first(): +@pytest.mark.parametrize( + 'data_files', + [False, True]) +@pytest.mark.parametrize( + 'setup_cfg,libexec_pattern', + [(False, False), (True, False), (True, True)]) +@pytest.mark.parametrize( + 'symlink_first', + [False, True]) +def test_build_package(symlink_first, setup_cfg, libexec_pattern, data_files): with TemporaryDirectory(prefix='test_colcon_') as tmp_path_str: - install_base = _test_build_package(tmp_path_str, symlink_install=True) - - assert 1 == len(list(install_base.rglob('test-package.egg-link'))) - assert 0 == len(list(install_base.rglob('PKG-INFO'))) - - install_base = _test_build_package(tmp_path_str, symlink_install=False) - - assert 0 == len(list(install_base.rglob('test-package.egg-link'))) - assert 1 == len(list(install_base.rglob('PKG-INFO'))) - - -def test_build_package_symlink_second(): - with TemporaryDirectory(prefix='test_colcon_') as tmp_path_str: - install_base = _test_build_package(tmp_path_str, symlink_install=False) - - assert 0 == len(list(install_base.rglob('test-package.egg-link'))) - assert 1 == len(list(install_base.rglob('PKG-INFO'))) - - install_base = _test_build_package(tmp_path_str, symlink_install=True) - - assert 1 == len(list(install_base.rglob('test-package.egg-link'))) - assert 0 == len(list(install_base.rglob('PKG-INFO'))) - - -def test_build_package_libexec_pattern(): - event_loop = new_event_loop() - asyncio.set_event_loop(event_loop) - try: - with TemporaryDirectory(prefix='test_colcon_') as tmp_path_str: - tmp_path = Path(tmp_path_str) - python_build_task = PythonBuildTask() - package = PackageDescriptor(tmp_path / 'src') - package.name = 'test_package' - package.type = 'python' - - context = TaskContext( - pkg=package, - args=SimpleNamespace( - path=str(tmp_path / 'src'), - build_base=str(tmp_path / 'build'), - install_base=str(tmp_path / 'install'), - symlink_install=False, - ), - dependencies={} - ) - python_build_task.set_context(context=context) - - pkg = python_build_task.context.pkg - - pkg.path.mkdir(exist_ok=True) - (pkg.path / 'setup.py').write_text( - 'from setuptools import setup\n' - 'setup()\n' - ) - (pkg.path / 'setup.cfg').write_text( - '[metadata]\n' - 'name = test_package\n' - '[options]\n' - 'packages = find:\n' - '[options.entry_points]\n' - 'console_scripts =\n' - ' my_command = my_module:main\n' - '[develop]\n' - 'script-dir=$base/lib/test_package\n' - '[install]\n' - 'install-scripts=$base/lib/test_package\n' - ) - (pkg.path / 'my_module').mkdir(exist_ok=True) - (pkg.path / 'my_module' / '__init__.py').write_text( - 'def main():\n' - ' print("Hello, World!")\n' - ) - - rc = event_loop.run_until_complete(python_build_task.build()) - assert not rc - - install_base = Path(python_build_task.context.args.install_base) - assert list(install_base.rglob( - '**/lib/test_package/my_command*')) - finally: - event_loop.close() + _test_build_package( + tmp_path_str, symlink_install=symlink_first, + setup_cfg=setup_cfg, libexec_pattern=libexec_pattern, + data_files=data_files) + + # Test again with the symlink flag inverted to validate cleanup + _test_build_package( + tmp_path_str, symlink_install=not symlink_first, + setup_cfg=setup_cfg, libexec_pattern=libexec_pattern, + data_files=data_files) From f7dfec7efc54b8d9a4ce0ba6e43c7137b5810011 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Fri, 2 Feb 2024 22:34:58 -0600 Subject: [PATCH 11/34] Implement a custom distutils command to symlink data_files (#592) The default implementation of install_data will always copy files into the destination directory. When we use the 'develop' command, we actually need to specifically tell setuptools to do something with the data_files or they will be ignored. Instead of telling setuptools to use install_data as-is, we can implement a custom version of install_data that will try to symlink the files instead. --- colcon_core/distutils/__init__.py | 0 colcon_core/distutils/commands/__init__.py | 0 .../distutils/commands/symlink_data.py | 26 +++++++++++++++++++ colcon_core/task/python/build.py | 25 ++++++++++++++++-- setup.cfg | 2 ++ 5 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 colcon_core/distutils/__init__.py create mode 100644 colcon_core/distutils/commands/__init__.py create mode 100644 colcon_core/distutils/commands/symlink_data.py diff --git a/colcon_core/distutils/__init__.py b/colcon_core/distutils/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/colcon_core/distutils/commands/__init__.py b/colcon_core/distutils/commands/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/colcon_core/distutils/commands/symlink_data.py b/colcon_core/distutils/commands/symlink_data.py new file mode 100644 index 000000000..ae1c7e741 --- /dev/null +++ b/colcon_core/distutils/commands/symlink_data.py @@ -0,0 +1,26 @@ +# Copyright 2023 Open Source Robotics Foundation, Inc. +# Licensed under the Apache License, Version 2.0 + +from distutils.command.install_data import install_data +import os + + +class symlink_data(install_data): # noqa: N801 + """Like install_data, but symlink files instead of copying.""" + + def copy_file(self, src, dst, **kwargs): # noqa: D102 + if kwargs.get('link'): + return super().copy_file(src, dst, **kwargs) + + if self.force: + # os.symlink fails if the destination exists as a regular file + if os.path.isdir(dst): + target = os.path.join(dst, os.path.basename(src)) + else: + target = dst + if os.path.exists(dst) and not os.path.islink(dst): + os.remove(target) + + kwargs['link'] = 'sym' + src = os.path.abspath(src) + return super().copy_file(src, dst, **kwargs) diff --git a/colcon_core/task/python/build.py b/colcon_core/task/python/build.py index 65d0e9071..5dc89e442 100644 --- a/colcon_core/task/python/build.py +++ b/colcon_core/task/python/build.py @@ -118,13 +118,19 @@ async def build(self, *, additional_hooks=None): # noqa: D102 # prevent installation of dependencies specified in setup.py cmd.append('--single-version-externally-managed') self._append_install_layout(args, cmd) + if setup_py_data.get('data_files'): + cmd += ['install_data'] + if rc is not None: + cmd += ['--force'] completed = await run( self.context, cmd, cwd=args.path, env=env) if completed.returncode: return completed.returncode else: - self._undo_install(pkg, args, setup_py_data, python_lib) + rc = self._undo_install(pkg, args, setup_py_data, python_lib) + if rc: + return rc temp_symlinks = self._symlinks_in_build(args, setup_py_data) # invoke `setup.py develop` step in build space @@ -142,7 +148,9 @@ async def build(self, *, additional_hooks=None): # noqa: D102 '--no-deps', ] if setup_py_data.get('data_files'): - cmd += ['install_data'] + cmd += ['symlink_data'] + if rc is not None: + cmd += ['--force'] completed = await run( self.context, cmd, cwd=args.build_base, env=env) finally: @@ -189,6 +197,12 @@ async def _get_available_commands(self, path, env): return commands async def _undo_develop(self, pkg, args, env): + """ + Undo a previously run 'develop' command. + + :returns: None if develop was not previously detected, otherwise + an integer return code where zero indicates success. + """ # undo previous develop if .egg-info is found and develop symlinks egg_info = os.path.join( args.build_base, '%s.egg-info' % pkg.name.replace('-', '_')) @@ -207,6 +221,12 @@ async def _undo_develop(self, pkg, args, env): return completed.returncode def _undo_install(self, pkg, args, setup_py_data, python_lib): + """ + Undo a previously run 'install' command. + + :returns: None if install was not previously detected, otherwise + an integer return code where zero indicates success. + """ # undo previous install if install.log is found install_log = os.path.join(args.build_base, 'install.log') if not os.path.exists(install_log): @@ -246,6 +266,7 @@ def _undo_install(self, pkg, args, setup_py_data, python_lib): with suppress(OSError): os.rmdir(d) os.remove(install_log) + return 0 def _symlinks_in_build(self, args, setup_py_data): items = ['setup.py'] diff --git a/setup.cfg b/setup.cfg index 99e4626d4..e935db42d 100644 --- a/setup.cfg +++ b/setup.cfg @@ -145,6 +145,8 @@ colcon_core.verb = test = colcon_core.verb.test:TestVerb console_scripts = colcon = colcon_core.command:main +distutils.commands = + symlink_data = colcon_core.distutils.commands.symlink_data:symlink_data pytest11 = colcon_core_warnings_stderr = colcon_core.pytest.hooks From 9854d54bdf6d87f2aee1a1461f26fcd0817468b6 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Wed, 7 Feb 2024 15:12:16 -0600 Subject: [PATCH 12/34] Switch to workflow_call for GitHub Actions CI (#613) --- .github/workflows/bootstrap.yaml | 52 ++++++++++++++++++++++++++++ .github/workflows/ci.yaml | 59 +++----------------------------- 2 files changed, 56 insertions(+), 55 deletions(-) create mode 100644 .github/workflows/bootstrap.yaml diff --git a/.github/workflows/bootstrap.yaml b/.github/workflows/bootstrap.yaml new file mode 100644 index 000000000..a6e38c1f8 --- /dev/null +++ b/.github/workflows/bootstrap.yaml @@ -0,0 +1,52 @@ +name: Run bootstrap tests + +on: + workflow_call: + +jobs: + setup: + runs-on: ubuntu-latest + outputs: + strategy: ${{steps.load.outputs.strategy}} + + steps: + - uses: actions/checkout@v4 + with: + repository: colcon/ci + - id: load + run: | + strategy=$(jq -c -M '.' strategy.json) + echo "strategy=${strategy}" >> $GITHUB_OUTPUT + + bootstrap: + needs: [setup] + strategy: ${{ fromJson(needs.setup.outputs.strategy) }} + runs-on: ${{ matrix.os }} + + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: ${{matrix.python}} + - name: Install dependencies + run: | + python -m pip install -U pip setuptools + python -m pip install -U -e .[test] + python -m pip uninstall -y colcon-core + - name: Build and test + run: | + cd .. + python ${{ github.workspace }}/bin/colcon build --paths ${{ github.workspace }} + python ${{ github.workspace }}/bin/colcon test --paths ${{ github.workspace }} --return-code-on-test-failure + - name: Use the installed package (Bash) + if: ${{runner.os != 'windows'}} + shell: bash + run: | + . ../install/local_setup.sh + colcon --help + - name: Use the installed package (CMD) + if: ${{runner.os == 'windows'}} + shell: cmd + run: | + call ..\install\local_setup.bat + colcon --help diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 75aa10486..a70452784 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -6,60 +6,9 @@ on: pull_request: jobs: - setup: - runs-on: ubuntu-latest - outputs: - strategy: ${{steps.load.outputs.strategy}} - - steps: - - uses: actions/checkout@v4 - with: - repository: colcon/ci - - id: load - run: echo "strategy=$(echo $(cat strategy.json))" >> $GITHUB_OUTPUT - pytest: - needs: [setup] - strategy: ${{fromJson(needs.setup.outputs.strategy)}} - runs-on: ${{matrix.os}} - - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v4 - with: - python-version: ${{matrix.python}} - - uses: colcon/ci@v1 - - uses: codecov/codecov-action@v3 - + uses: colcon/ci/.github/workflows/pytest.yaml@main + with: + codecov: true bootstrap: - needs: [setup] - strategy: ${{fromJson(needs.setup.outputs.strategy)}} - runs-on: ${{matrix.os}} - - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v4 - with: - python-version: ${{matrix.python}} - - name: Install dependencies - run: | - python -m pip install -U pip setuptools - python -m pip install -U -e .[test] - python -m pip uninstall -y colcon-core - - name: Build and test - run: | - cd .. - python ${{github.workspace}}/bin/colcon build --paths ${{github.workspace}} - python ${{github.workspace}}/bin/colcon test --paths ${{github.workspace}} --return-code-on-test-failure - - name: Use the installed package (Bash) - if: ${{runner.os != 'windows'}} - shell: bash - run: | - . ../install/local_setup.sh - colcon --help - - name: Use the installed package (CMD) - if: ${{runner.os == 'windows'}} - shell: cmd - run: | - call ..\install\local_setup.bat - colcon --help + uses: ./.github/workflows/bootstrap.yaml From ee442627691a49f0d243aa954ad626c254c41fc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Steven!=20Ragnar=C3=B6k?= Date: Fri, 9 Feb 2024 12:13:46 -0800 Subject: [PATCH 13/34] Update deb platforms for release (#609) Added: * Ubuntu Noble (24.04 LTS pre-release) * Debian Trixie (testing) Dropped: * Debian Bullseye (oldstable) Retained: * Debian Bookworm (stable) * Ubuntu Focal (20.04 LTS) * Ubuntu Jammy (22.04 LTS) --- publish-python.yaml | 3 ++- stdeb.cfg | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/publish-python.yaml b/publish-python.yaml index 3a0a1f1a2..bd7c8693b 100644 --- a/publish-python.yaml +++ b/publish-python.yaml @@ -10,5 +10,6 @@ artifacts: distributions: - ubuntu:focal - ubuntu:jammy - - debian:bullseye + - ubuntu:noble - debian:bookworm + - debian:trixie diff --git a/stdeb.cfg b/stdeb.cfg index 08bafc8ed..3c5672b71 100644 --- a/stdeb.cfg +++ b/stdeb.cfg @@ -3,5 +3,5 @@ No-Python2: Depends3: python3-distlib, python3-empy (<4), python3-packaging, python3-pytest, python3-setuptools, python3 (>= 3.8) | python3-importlib-metadata Recommends3: python3-pytest-cov Suggests3: python3-pytest-repeat, python3-pytest-rerunfailures -Suite: focal jammy bullseye bookworm +Suite: focal jammy noble bookworm trixie X-Python3-Version: >= 3.6 From c7d8194849a75a41c523ec50f7dbc4331f6b086f Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Tue, 27 Feb 2024 09:41:55 -0600 Subject: [PATCH 14/34] Allow use of flake8 v7 (#615) All packages in the colcon org appear to test successfully against flake8 v7 now. --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index e935db42d..640bf8b21 100644 --- a/setup.cfg +++ b/setup.cfg @@ -44,7 +44,7 @@ zip_safe = false [options.extras_require] test = - flake8>=3.6.0,<7 + flake8>=3.6.0 flake8-blind-except flake8-builtins flake8-class-newline From a6631edb531066ed584b51ff0377efff7f0f4619 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Mon, 11 Mar 2024 17:59:58 -0500 Subject: [PATCH 15/34] Suppress setup.py deprecation warnings during build (#626) This warning is legitimate and something that developers should be aware of, but at present, the replacement does not provide all of the functionality necessary to support the currently available features in colcon. When that changes, we'll communicate the upgrade path to users and re-enable the deprecation warning. --- colcon_core/task/python/build.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/colcon_core/task/python/build.py b/colcon_core/task/python/build.py index 5dc89e442..442d5a620 100644 --- a/colcon_core/task/python/build.py +++ b/colcon_core/task/python/build.py @@ -8,7 +8,6 @@ from pathlib import Path import shutil import sys -from sys import executable from colcon_core.environment import create_environment_hooks from colcon_core.environment import create_environment_scripts @@ -26,6 +25,12 @@ logger = colcon_logger.getChild(__name__) +_PYTHON_CMD = [ + sys.executable, + '-W', + 'ignore:setup.py install is deprecated', +] + def _get_install_scripts(path): setup_cfg_path = os.path.join(path, 'setup.cfg') @@ -92,7 +97,7 @@ async def build(self, *, additional_hooks=None): # noqa: D102 # invoke `setup.py install` step with lots of arguments # to avoid placing any files in the source space - cmd = [executable, 'setup.py'] + cmd = _PYTHON_CMD + ['setup.py'] if 'egg_info' in available_commands: # `setup.py egg_info` requires the --egg-base to exist os.makedirs(args.build_base, exist_ok=True) @@ -139,8 +144,8 @@ async def build(self, *, additional_hooks=None): # noqa: D102 try: # --editable causes this to skip creating/editing the # easy-install.pth file - cmd = [ - executable, 'setup.py', + cmd = _PYTHON_CMD + [ + 'setup.py', 'develop', '--editable', '--build-directory', @@ -181,7 +186,7 @@ async def build(self, *, additional_hooks=None): # noqa: D102 async def _get_available_commands(self, path, env): output = await check_output( - [executable, 'setup.py', '--help-commands'], cwd=path, env=env) + _PYTHON_CMD + ['setup.py', '--help-commands'], cwd=path, env=env) commands = set() for line in output.splitlines(): if not line.startswith(b' '): @@ -208,8 +213,8 @@ async def _undo_develop(self, pkg, args, env): args.build_base, '%s.egg-info' % pkg.name.replace('-', '_')) setup_py_build_space = os.path.join(args.build_base, 'setup.py') if os.path.exists(egg_info) and os.path.islink(setup_py_build_space): - cmd = [ - executable, 'setup.py', + cmd = _PYTHON_CMD + [ + 'setup.py', 'develop', '--uninstall', '--editable', '--build-directory', os.path.join(args.build_base, 'build') From 82359bc8b5fab99c1564293da9ca48a51b6eec0a Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Mon, 11 Mar 2024 18:00:13 -0500 Subject: [PATCH 16/34] Re-work extension point test mocking (#622) The existing mocks are fragile and don't reliably work in all cases. This approach uses an actual Distribution instance so that we don't need to modify it to reflect upstream changes to the interface. --- test/test_extension_point.py | 78 +++++++++++++++++++----------------- 1 file changed, 41 insertions(+), 37 deletions(-) diff --git a/test/test_extension_point.py b/test/test_extension_point.py index 96e58a0de..6d6269619 100644 --- a/test/test_extension_point.py +++ b/test/test_extension_point.py @@ -6,6 +6,12 @@ from unittest.mock import DEFAULT from unittest.mock import patch +try: + from importlib.metadata import Distribution +except ImportError: + # TODO: Drop this with Python 3.7 support + from importlib_metadata import Distribution + from colcon_core.extension_point import EntryPoint from colcon_core.extension_point import EXTENSION_POINT_GROUP_NAME from colcon_core.extension_point import get_all_extension_points @@ -17,57 +23,55 @@ from .environment_context import EnvironmentContext -Group1 = EntryPoint('group1', 'g1', EXTENSION_POINT_GROUP_NAME) -Group2 = EntryPoint('group2', 'g2', EXTENSION_POINT_GROUP_NAME) -ExtA = EntryPoint('extA', 'eA', Group1.name) -ExtB = EntryPoint('extB', 'eB', Group1.name) - - -class Dist(): - - version = '0.0.0' +class _FakeDistribution(Distribution): def __init__(self, entry_points): - self.metadata = {'Name': f'dist-{id(self)}'} - self._entry_points = entry_points + entry_points_spec = [] + for group_name, group_members in entry_points.items(): + entry_points_spec.append(f'[{group_name}]') + for member_name, member_value in group_members: + entry_points_spec.append(f'{member_name} = {member_value}') + entry_points_spec.append('') + + self._files = { + 'PKG-INFO': f'Name: dist-{id(self)}\nVersion: 0.0.0\n', + 'entry_points.txt': '\n'.join(entry_points_spec) + '\n', + } - @property - def entry_points(self): - return list(self._entry_points) + def read_text(self, filename): + return self._files.get(filename) - @property - def name(self): - return self.metadata['Name'] + def locate_file(self, path): + return path -def iter_entry_points(*, group=None): - if group == EXTENSION_POINT_GROUP_NAME: - return [Group1, Group2] - elif group == Group1.name: - return [ExtA, ExtB] - assert not group - return { - EXTENSION_POINT_GROUP_NAME: [Group1, Group2], - Group1.name: [ExtA, ExtB], - } +def _distributions(): + yield _FakeDistribution({ + EXTENSION_POINT_GROUP_NAME: [('group1', 'g1')], + 'group1': [('extA', 'eA'), ('extB', 'eB')], + }) + yield _FakeDistribution({ + EXTENSION_POINT_GROUP_NAME: [('group2', 'g2')], + 'group2': [('extC', 'eC')], + }) + yield _FakeDistribution({ + 'groupX': [('extD', 'eD')], + }) -def distributions(): - return [ - Dist([Group1, ExtA, ExtB]), - Dist([Group2, EntryPoint('extC', 'eC', Group2.name)]), - Dist([EntryPoint('extD', 'eD', 'groupX')]), - ] +def _entry_points(): + for dist in _distributions(): + yield from dist.entry_points def test_all_extension_points(): with patch( 'colcon_core.extension_point.entry_points', - side_effect=iter_entry_points + side_effect=_entry_points ): with patch( 'colcon_core.extension_point.distributions', - side_effect=distributions + side_effect=_distributions ): # successfully load a known entry point extension_points = get_all_extension_points() @@ -84,11 +88,11 @@ def test_extension_point_blocklist(): # successful loading of extension point without a blocklist with patch( 'colcon_core.extension_point.entry_points', - side_effect=iter_entry_points + side_effect=_entry_points ): with patch( 'colcon_core.extension_point.distributions', - side_effect=distributions + side_effect=_distributions ): extension_points = get_extension_points('group1') assert 'extA' in extension_points.keys() From 6cf24eafdb1ab67ba59935f855157fb7b9d2b6bf Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Mon, 11 Mar 2024 18:00:44 -0500 Subject: [PATCH 17/34] Handle invocation with no available verbs or env vars (#620) It might be useful when measuring performance to invoke colcon with an entire class of extensions blocked. This change "handles" the case where no verbs are available and minimizes the output when there are no verbs or environment variables available. --- colcon_core/command.py | 17 ++++++++++------- test/test_command.py | 11 +++++++++++ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/colcon_core/command.py b/colcon_core/command.py index 07dddaf1c..fd159be10 100644 --- a/colcon_core/command.py +++ b/colcon_core/command.py @@ -232,14 +232,16 @@ def _parse_optional(self, arg_string): return None return result + epilog = get_environment_variables_epilog(environment_variables_group_name) + if epilog: + epilog += '\n\n' + epilog += READTHEDOCS_MESSAGE + # top level parser parser = CustomArgumentParser( prog=get_prog_name(), formatter_class=CustomFormatter, - epilog=( - get_environment_variables_epilog( - environment_variables_group_name - ) + '\n\n' + READTHEDOCS_MESSAGE)) + epilog=epilog) # enable introspecting and intercepting all command line arguments parser = decorate_argument_parser(parser) @@ -287,6 +289,8 @@ def get_environment_variables_epilog(group_name): """ # list environment variables with descriptions entry_points = load_extension_points(group_name) + if not entry_points: + return '' env_vars = { env_var.name: env_var.description for env_var in entry_points.values()} epilog_lines = [] @@ -376,7 +380,6 @@ def create_subparser(parser, cmd_name, verb_extensions, *, attribute): :returns: The special action object """ global colcon_logger - assert verb_extensions, 'No verb extensions' # list of available verbs with their descriptions verbs = [] @@ -387,9 +390,9 @@ def create_subparser(parser, cmd_name, verb_extensions, *, attribute): # add subparser with description of verb extensions subparser = parser.add_subparsers( title=f'{cmd_name} verbs', - description='\n'.join(verbs), + description='\n'.join(verbs) or None, dest=attribute, - help=f'call `{cmd_name} VERB -h` for specific help', + help=f'call `{cmd_name} VERB -h` for specific help' if verbs else None, ) return subparser diff --git a/test/test_command.py b/test/test_command.py index cdd547700..d2aca2d2c 100644 --- a/test/test_command.py +++ b/test/test_command.py @@ -82,6 +82,17 @@ def test_main(): assert rc == signal.SIGINT +def test_main_no_verbs_or_env(): + with ExtensionPointContext(): + with patch( + 'colcon_core.command.load_extension_points', + return_value={}, + ): + with pytest.raises(SystemExit) as e: + main(argv=['--help']) + assert e.value.code == 0 + + def test_create_parser(): with ExtensionPointContext(): parser = create_parser('colcon_core.environment_variable') From 2208a3b7a5eb810d8e65a0641d3e62cc7b9634c3 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Mon, 11 Mar 2024 18:18:16 -0500 Subject: [PATCH 18/34] Add an explicit cache on Python entry points (#614) Whenever we enumerate Python entry points to load colcon extension points, we're re-parsing metadata for every Python package found on the system. Worse yet, accessing attributes on importlib.metadata.Distribution typically results in re-reading the metadata each time, so we're hitting the disk pretty hard. We don't generally expect the entry points available to change, so we should cache that information once and parse each package's metadata a single time. This change jumps through a lot of hoops to specifically use the `importlib.metadata.entry_points()` function wherever possible because it has an optimization that allows us to avoid reading each package's metadata while still properly handling package shadowing between paths. This has a measurable impact on extension point loading performance. --- colcon_core/extension_point.py | 98 ++++++++++++++++++++++++---------- test/spell_check.words | 1 + test/test_extension_point.py | 40 ++++++++++++++ 3 files changed, 111 insertions(+), 28 deletions(-) diff --git a/colcon_core/extension_point.py b/colcon_core/extension_point.py index c724d2213..4bea3fc6b 100644 --- a/colcon_core/extension_point.py +++ b/colcon_core/extension_point.py @@ -3,8 +3,11 @@ # Licensed under the Apache License, Version 2.0 from collections import defaultdict +from itertools import chain import os +import sys import traceback +import warnings try: from importlib.metadata import distributions @@ -26,7 +29,6 @@ logger = colcon_logger.getChild(__name__) - """ The group name for entry points identifying colcon extension points. @@ -36,6 +38,8 @@ """ EXTENSION_POINT_GROUP_NAME = 'colcon_core.extension_point' +_ENTRY_POINTS_CACHE = [] + def _get_unique_distributions(): seen = set() @@ -46,6 +50,50 @@ def _get_unique_distributions(): yield dist +def _get_entry_points(): + for dist in _get_unique_distributions(): + for entry_point in dist.entry_points: + # Modern EntryPoint instances should already have this set + if not hasattr(entry_point, 'dist'): + entry_point.dist = dist + yield entry_point + + +def _get_cached_entry_points(): + if not _ENTRY_POINTS_CACHE: + if sys.version_info >= (3, 10): + # We prefer using importlib.metadata.entry_points because it + # has an internal optimization which allows us to load the entry + # points without reading the individual PKG-INFO files, while + # still visiting each unique distribution only once. + all_entry_points = entry_points() + if isinstance(all_entry_points, dict): + # Prior to Python 3.12, entry_points returned a (deprecated) + # dict. Unfortunately, the "future-proof" recommended + # pattern is to add filter parameters, but we actually + # want to cache everything so that doesn't work here. + with warnings.catch_warnings(): + warnings.filterwarnings( + 'ignore', + 'SelectableGroups dict interface is deprecated', + DeprecationWarning, + module=__name__) + all_entry_points = chain.from_iterable( + all_entry_points.values()) + _ENTRY_POINTS_CACHE.extend(all_entry_points) + else: + # If we don't have Python 3.10, we must read each PKG-INFO to + # get the name of the distribution so that we can skip the + # "shadowed" distributions properly. + _ENTRY_POINTS_CACHE.extend(_get_entry_points()) + return _ENTRY_POINTS_CACHE + + +def clear_entry_point_cache(): + """Purge the entry point cache.""" + _ENTRY_POINTS_CACHE.clear() + + def get_all_extension_points(): """ Get all extension points related to `colcon` and any of its extensions. @@ -59,23 +107,24 @@ def get_all_extension_points(): colcon_extension_points = get_extension_points(EXTENSION_POINT_GROUP_NAME) colcon_extension_points.setdefault(EXTENSION_POINT_GROUP_NAME, None) - entry_points = defaultdict(dict) - for dist in _get_unique_distributions(): - for entry_point in dist.entry_points: - # skip groups which are not registered as extension points - if entry_point.group not in colcon_extension_points: - continue - - if entry_point.name in entry_points[entry_point.group]: - previous = entry_points[entry_point.group][entry_point.name] - logger.error( - f"Entry point '{entry_point.group}.{entry_point.name}' is " - f"declared multiple times, '{entry_point.value}' " - f"from '{dist._path}' " - f"overwriting '{previous}'") - entry_points[entry_point.group][entry_point.name] = \ - (entry_point.value, dist.metadata['Name'], dist.version) - return entry_points + extension_points = defaultdict(dict) + for entry_point in _get_cached_entry_points(): + if entry_point.group not in colcon_extension_points: + continue + + dist_metadata = entry_point.dist.metadata + ep_tuple = ( + entry_point.value, + dist_metadata['Name'], dist_metadata['Version'], + ) + if entry_point.name in extension_points[entry_point.group]: + previous = extension_points[entry_point.group][entry_point.name] + logger.error( + f"Entry point '{entry_point.group}.{entry_point.name}' is " + f"declared multiple times, '{ep_tuple}' " + f"overwriting '{previous}'") + extension_points[entry_point.group][entry_point.name] = ep_tuple + return extension_points def get_extension_points(group): @@ -87,16 +136,9 @@ def get_extension_points(group): :rtype: dict """ extension_points = {} - try: - # Python 3.10 and newer - query = entry_points(group=group) - except TypeError: - query = ( - entry_point - for dist in _get_unique_distributions() - for entry_point in dist.entry_points - if entry_point.group == group) - for entry_point in query: + for entry_point in _get_cached_entry_points(): + if entry_point.group != group: + continue if entry_point.name in extension_points: previous_entry_point = extension_points[entry_point.name] logger.error( diff --git a/test/spell_check.words b/test/spell_check.words index d44f7a23b..96051a0ff 100644 --- a/test/spell_check.words +++ b/test/spell_check.words @@ -50,6 +50,7 @@ importlib importorskip isatty iterdir +itertools junit levelname libexec diff --git a/test/test_extension_point.py b/test/test_extension_point.py index 6d6269619..63f89edbb 100644 --- a/test/test_extension_point.py +++ b/test/test_extension_point.py @@ -12,6 +12,7 @@ # TODO: Drop this with Python 3.7 support from importlib_metadata import Distribution +from colcon_core.extension_point import clear_entry_point_cache from colcon_core.extension_point import EntryPoint from colcon_core.extension_point import EXTENSION_POINT_GROUP_NAME from colcon_core.extension_point import get_all_extension_points @@ -73,6 +74,8 @@ def test_all_extension_points(): 'colcon_core.extension_point.distributions', side_effect=_distributions ): + clear_entry_point_cache() + # successfully load a known entry point extension_points = get_all_extension_points() assert set(extension_points.keys()) == { @@ -94,12 +97,14 @@ def test_extension_point_blocklist(): 'colcon_core.extension_point.distributions', side_effect=_distributions ): + clear_entry_point_cache() extension_points = get_extension_points('group1') assert 'extA' in extension_points.keys() extension_point = extension_points['extA'] assert extension_point == 'eA' with patch.object(EntryPoint, 'load', return_value=None) as load: + clear_entry_point_cache() load_extension_point('extA', 'eA', 'group1') assert load.call_count == 1 @@ -108,12 +113,14 @@ def test_extension_point_blocklist(): with EnvironmentContext(COLCON_EXTENSION_BLOCKLIST=os.pathsep.join([ 'group1.extB', 'group2.extC']) ): + clear_entry_point_cache() load_extension_point('extA', 'eA', 'group1') assert load.call_count == 1 # entry point in a blocked group can't be loaded load.reset_mock() with EnvironmentContext(COLCON_EXTENSION_BLOCKLIST='group1'): + clear_entry_point_cache() with pytest.raises(RuntimeError) as e: load_extension_point('extA', 'eA', 'group1') assert 'The entry point group name is listed in the environment ' \ @@ -124,6 +131,7 @@ def test_extension_point_blocklist(): with EnvironmentContext(COLCON_EXTENSION_BLOCKLIST=os.pathsep.join([ 'group1.extA', 'group1.extB']) ): + clear_entry_point_cache() with pytest.raises(RuntimeError) as e: load_extension_point('extA', 'eA', 'group1') assert 'The entry point name is listed in the environment ' \ @@ -131,6 +139,38 @@ def test_extension_point_blocklist(): assert load.call_count == 0 +def test_redefined_extension_point(): + def _duped_distributions(): + yield from _distributions() + yield _FakeDistribution({ + 'group2': [('extC', 'eC-prime')], + }) + + def _duped_entry_points(): + for dist in _duped_distributions(): + yield from dist.entry_points + + with patch('colcon_core.extension_point.logger.error') as error: + with patch( + 'colcon_core.extension_point.entry_points', + side_effect=_duped_entry_points + ): + with patch( + 'colcon_core.extension_point.distributions', + side_effect=_duped_distributions + ): + clear_entry_point_cache() + extension_points = get_all_extension_points() + assert 'eC-prime' == extension_points['group2']['extC'][0] + assert error.call_count == 1 + + error.reset_mock() + clear_entry_point_cache() + extension_points = get_extension_points('group2') + assert 'eC-prime' == extension_points.get('extC') + assert error.call_count == 1 + + def entry_point_load(self, *args, **kwargs): if self.name == 'exception': raise Exception('entry point raising exception') From 03d619313015cd90768c24a5347c8664c4b2dc13 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Wed, 13 Mar 2024 14:00:41 -0500 Subject: [PATCH 19/34] 0.16.0 --- colcon_core/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/colcon_core/__init__.py b/colcon_core/__init__.py index 4654486c3..8da86e1f2 100644 --- a/colcon_core/__init__.py +++ b/colcon_core/__init__.py @@ -1,4 +1,4 @@ # Copyright 2016-2020 Dirk Thomas # Licensed under the Apache License, Version 2.0 -__version__ = '0.15.2' +__version__ = '0.16.0' From 532e2134fd8a9956ee783e27db1c336152121828 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Sat, 16 Mar 2024 18:41:56 -0500 Subject: [PATCH 20/34] Add Debian 'Replaces: colcon' field (#628) Upstream Debian has decided to package colcon, and they created a package simply called 'colcon' which provides only the /usr/bin/colcon executable and some weak dependencies as a sort of replacement for colcon-common-extensions. This conflicts with our python3-colcon-core package during unpacking. We could add 'Conflicts: colcon', but I think this is a great use case for 'Replaces:' because the only file provided by 'colcon' is /usr/bin/colcon, which we obviously provide here. This way, folks can still successfully install 'colcon' and get the weak dependencies even if our version of python-colcon-core is installed, and our copy of /usr/bin/colcon will take precedence over the one provided by 'colcon'. --- stdeb.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/stdeb.cfg b/stdeb.cfg index 3c5672b71..9eb1227d1 100644 --- a/stdeb.cfg +++ b/stdeb.cfg @@ -3,5 +3,6 @@ No-Python2: Depends3: python3-distlib, python3-empy (<4), python3-packaging, python3-pytest, python3-setuptools, python3 (>= 3.8) | python3-importlib-metadata Recommends3: python3-pytest-cov Suggests3: python3-pytest-repeat, python3-pytest-rerunfailures +Replaces3: colcon Suite: focal jammy noble bookworm trixie X-Python3-Version: >= 3.6 From eb53f12b555c6fe039a029c0dd0bf35f11828720 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Tue, 9 Apr 2024 12:00:45 -0400 Subject: [PATCH 21/34] Suppress flake8 A005 in existing colcon API (#636) Rather than suppress A005 completely, we can ignore it in our existing API to prevent new A005 violations from appearing. --- setup.cfg | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/setup.cfg b/setup.cfg index 640bf8b21..85391702f 100644 --- a/setup.cfg +++ b/setup.cfg @@ -156,6 +156,10 @@ colcon_core.task.python.template = *.em [flake8] import-order-style = google +per-file-ignores = + colcon_core/distutils/__init__.py:A005 + colcon_core/logging.py:A005 + colcon_core/subprocess.py:A005 [coverage:run] source = colcon_core From b04d936fc1b509c29c0da96877cc84872d223431 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Tue, 9 Apr 2024 12:23:31 -0400 Subject: [PATCH 22/34] Fix argument parsing in newer Python. (#635) The comment in the code has more details, but as of https://github.com/python/cpython/pull/114180 we need to check for both a 3-tuple and a 4-tuple. Signed-off-by: Chris Lalancette --- colcon_core/command.py | 10 +++++++++- test/spell_check.words | 2 ++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/colcon_core/command.py b/colcon_core/command.py index fd159be10..228d3b369 100644 --- a/colcon_core/command.py +++ b/colcon_core/command.py @@ -226,7 +226,15 @@ class CustomArgumentParser(argparse.ArgumentParser): def _parse_optional(self, arg_string): result = super()._parse_optional(arg_string) - if result == (None, arg_string, None): + # Up until https://github.com/python/cpython/pull/114180 , + # _parse_optional() returned a 3-tuple when it couldn't classify + # the option. As of that PR (which is in Python 3.13, and + # backported to Python 3.12), it returns a 4-tuple. Check for + # either here. + if result in ( + (None, arg_string, None), + (None, arg_string, None, None), + ): # in the case there the arg is classified as an unknown 'O' # override that and classify it as an 'A' return None diff --git a/test/spell_check.words b/test/spell_check.words index 96051a0ff..83548464c 100644 --- a/test/spell_check.words +++ b/test/spell_check.words @@ -3,6 +3,7 @@ apache argparse asyncio autouse +backported basepath bazqux blocklist @@ -17,6 +18,7 @@ configparser contextlib coroutine coroutines +cpython datetime debian debinfo From 15ed7d670ebb64871689fabb98a2bc2d459ae2c1 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Tue, 9 Apr 2024 11:28:06 -0500 Subject: [PATCH 23/34] 0.16.1 --- colcon_core/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/colcon_core/__init__.py b/colcon_core/__init__.py index 8da86e1f2..4f82143e3 100644 --- a/colcon_core/__init__.py +++ b/colcon_core/__init__.py @@ -1,4 +1,4 @@ # Copyright 2016-2020 Dirk Thomas # Licensed under the Apache License, Version 2.0 -__version__ = '0.16.0' +__version__ = '0.16.1' From b174608650a3521203e10d7950aa35a3c6d63dcc Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Fri, 24 May 2024 11:01:57 -0700 Subject: [PATCH 24/34] Use CODECOV_TOKEN to upload coverage (#648) --- .github/workflows/ci.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index a70452784..64661c675 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -8,7 +8,7 @@ on: jobs: pytest: uses: colcon/ci/.github/workflows/pytest.yaml@main - with: - codecov: true + secrets: + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} bootstrap: uses: ./.github/workflows/bootstrap.yaml From a067589ebe6c36c0ab62abb379caeafef9c39763 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Fri, 24 May 2024 11:16:51 -0700 Subject: [PATCH 25/34] Fix modified logger in add_file_handler, add tests (#649) It appears that the `logger` parameter was previously unused, and that the function just used `colcon_logger` directly. As it happens, the only use of `add_file_handler` passes `colcon_logger`, so the mistake had no effect. This fixes the bug and adds a simple test for that code. --- colcon_core/logging.py | 10 +++++----- test/spell_check.words | 1 + test/test_logging.py | 20 ++++++++++++++++++++ 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/colcon_core/logging.py b/colcon_core/logging.py index 0a555b6f0..67db572e7 100644 --- a/colcon_core/logging.py +++ b/colcon_core/logging.py @@ -95,15 +95,15 @@ def filter(self, record): # noqa: A003 if isinstance(handler, logging.StreamHandler): formatter = handler.formatter # filter colcon specific log messages from default stream handler - handler.addFilter(Filter(colcon_logger.name)) + handler.addFilter(Filter(logger.name)) # add a stream handler replacing the one filtered on the root logger handler = logging.StreamHandler() if formatter: # use same formatter as for stream handler handler.setFormatter(formatter) - handler.setLevel(colcon_logger.getEffectiveLevel()) - colcon_logger.addHandler(handler) + handler.setLevel(logger.getEffectiveLevel()) + logger.addHandler(handler) # add a file handler writing all log levels handler = logging.FileHandler(str(path)) @@ -121,9 +121,9 @@ def format_message_with_relative_time(record): # use same formatter as for stream handler handler.setFormatter(formatter) handler.setLevel(1) - colcon_logger.addHandler(handler) + logger.addHandler(handler) # change the logger to handle all levels - colcon_logger.setLevel(1) + logger.setLevel(1) return handler diff --git a/test/spell_check.words b/test/spell_check.words index 83548464c..07ac11178 100644 --- a/test/spell_check.words +++ b/test/spell_check.words @@ -130,6 +130,7 @@ testcase testsfailed testsuite thomas +tmpdir todo traceback tryfirst diff --git a/test/test_logging.py b/test/test_logging.py index c67673872..3de09b98a 100644 --- a/test/test_logging.py +++ b/test/test_logging.py @@ -2,8 +2,10 @@ # Licensed under the Apache License, Version 2.0 import logging +from pathlib import Path from unittest.mock import Mock +from colcon_core.logging import add_file_handler from colcon_core.logging import get_numeric_log_level from colcon_core.logging import set_logger_level_from_env import pytest @@ -56,3 +58,21 @@ def test_get_numeric_log_level(): with pytest.raises(ValueError) as e: get_numeric_log_level('-1') assert str(e.value).endswith('numeric log levels must be positive') + + +def test_add_file_handler(tmpdir): + log_path = Path(tmpdir) / 'test_add_file_handler.log' + log_path.touch() + logger = logging.getLogger('test_add_file_handler') + try: + logger.setLevel(logging.WARN) + add_file_handler(logger, log_path) + assert logger.getEffectiveLevel() != logging.WARN + logger.info('test_add_file_handler') + finally: + for handler in logger.handlers: + logger.removeHandler(handler) + handler.close() + + # check only that we logged SOMETHING to the file + assert log_path.stat().st_size > 10 From db84706de12a03dbfca814a75047f03e62cdb4ef Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Fri, 24 May 2024 11:22:23 -0700 Subject: [PATCH 26/34] Support alternate group names in get_*_extensions (#647) These functions are brief, but it would be nice not to have to duplicate them in other colcon packages which re-use the same extension frameworks. --- colcon_core/argument_parser/__init__.py | 6 ++++-- colcon_core/environment/__init__.py | 6 ++++-- colcon_core/event_handler/__init__.py | 6 ++++-- colcon_core/executor/__init__.py | 6 ++++-- colcon_core/package_augmentation/__init__.py | 6 ++++-- colcon_core/package_discovery/__init__.py | 6 ++++-- colcon_core/package_identification/__init__.py | 6 ++++-- colcon_core/package_selection/__init__.py | 6 ++++-- colcon_core/prefix_path/__init__.py | 6 ++++-- colcon_core/shell/__init__.py | 13 +++++++++---- colcon_core/task/python/test/__init__.py | 7 ++++--- colcon_core/verb/__init__.py | 6 ++++-- 12 files changed, 53 insertions(+), 27 deletions(-) diff --git a/colcon_core/argument_parser/__init__.py b/colcon_core/argument_parser/__init__.py index 44f743133..21f069489 100644 --- a/colcon_core/argument_parser/__init__.py +++ b/colcon_core/argument_parser/__init__.py @@ -42,7 +42,7 @@ def decorate_argument_parser(self, *, parser): raise NotImplementedError() -def get_argument_parser_extensions(): +def get_argument_parser_extensions(*, group_name=None): """ Get the available argument parser extensions. @@ -50,7 +50,9 @@ def get_argument_parser_extensions(): :rtype: OrderedDict """ - extensions = instantiate_extensions(__name__) + if group_name is None: + group_name = __name__ + extensions = instantiate_extensions(group_name) for name, extension in extensions.items(): extension.ARGUMENT_PARSER_DECORATOR_NAME = name return order_extensions_by_priority(extensions) diff --git a/colcon_core/environment/__init__.py b/colcon_core/environment/__init__.py index d57adbf3b..a7324d017 100644 --- a/colcon_core/environment/__init__.py +++ b/colcon_core/environment/__init__.py @@ -47,7 +47,7 @@ def create_environment_hooks(self, prefix_path, pkg_name): raise NotImplementedError() -def get_environment_extensions(): +def get_environment_extensions(*, group_name=None): """ Get the available environment extensions. @@ -55,7 +55,9 @@ def get_environment_extensions(): :rtype: OrderedDict """ - extensions = instantiate_extensions(__name__) + if group_name is None: + group_name = __name__ + extensions = instantiate_extensions(group_name) for name in list(extensions.keys()): extension = extensions[name] extension.ENVIRONMENT_NAME = name diff --git a/colcon_core/event_handler/__init__.py b/colcon_core/event_handler/__init__.py index a51756c5b..437894626 100644 --- a/colcon_core/event_handler/__init__.py +++ b/colcon_core/event_handler/__init__.py @@ -44,7 +44,7 @@ def __call__(self, event): raise NotImplementedError() -def get_event_handler_extensions(*, context): +def get_event_handler_extensions(*, context, group_name=None): """ Get the available event handler extensions. @@ -52,7 +52,9 @@ def get_event_handler_extensions(*, context): :rtype: OrderedDict """ - extensions = instantiate_extensions(__name__) + if group_name is None: + group_name = __name__ + extensions = instantiate_extensions(group_name) for name, extension in extensions.items(): extension.EVENT_HANDLER_NAME = name extension.context = context diff --git a/colcon_core/executor/__init__.py b/colcon_core/executor/__init__.py index aca035758..c0bdaccf9 100644 --- a/colcon_core/executor/__init__.py +++ b/colcon_core/executor/__init__.py @@ -197,7 +197,7 @@ def _flush(self): self._event_controller.flush() -def get_executor_extensions(): +def get_executor_extensions(*, group_name=None): """ Get the available executor extensions. @@ -206,7 +206,9 @@ def get_executor_extensions(): :rtype: OrderedDict """ - extensions = instantiate_extensions(__name__) + if group_name is None: + group_name = __name__ + extensions = instantiate_extensions(group_name) for name, extension in extensions.items(): extension.EXECUTOR_NAME = name return order_extensions_grouped_by_priority(extensions) diff --git a/colcon_core/package_augmentation/__init__.py b/colcon_core/package_augmentation/__init__.py index 3b579292f..e741292a1 100644 --- a/colcon_core/package_augmentation/__init__.py +++ b/colcon_core/package_augmentation/__init__.py @@ -65,7 +65,7 @@ def augment_package( raise NotImplementedError() -def get_package_augmentation_extensions(): +def get_package_augmentation_extensions(*, group_name=None): """ Get the available package augmentation extensions. @@ -73,7 +73,9 @@ def get_package_augmentation_extensions(): :rtype: OrderedDict """ - extensions = instantiate_extensions(__name__) + if group_name is None: + group_name = __name__ + extensions = instantiate_extensions(group_name) for name, extension in extensions.items(): extension.PACKAGE_AUGMENTATION_NAME = name return order_extensions_by_priority(extensions) diff --git a/colcon_core/package_discovery/__init__.py b/colcon_core/package_discovery/__init__.py index fc60f1470..c16f3b1d3 100644 --- a/colcon_core/package_discovery/__init__.py +++ b/colcon_core/package_discovery/__init__.py @@ -84,7 +84,7 @@ def discover(self, *, args, identification_extensions): raise NotImplementedError() -def get_package_discovery_extensions(): +def get_package_discovery_extensions(*, group_name=None): """ Get the available package discovery extensions. @@ -92,7 +92,9 @@ def get_package_discovery_extensions(): :rtype: OrderedDict """ - extensions = instantiate_extensions(__name__) + if group_name is None: + group_name = __name__ + extensions = instantiate_extensions(group_name) for name, extension in extensions.items(): extension.PACKAGE_DISCOVERY_NAME = name return order_extensions_by_priority(extensions) diff --git a/colcon_core/package_identification/__init__.py b/colcon_core/package_identification/__init__.py index 11e17d73f..01dd8a2ce 100644 --- a/colcon_core/package_identification/__init__.py +++ b/colcon_core/package_identification/__init__.py @@ -64,7 +64,7 @@ def identify(self, desc: PackageDescriptor): raise NotImplementedError() -def get_package_identification_extensions(): +def get_package_identification_extensions(*, group_name=None): """ Get the available package identification extensions. @@ -73,7 +73,9 @@ def get_package_identification_extensions(): :rtype: OrderedDict """ - extensions = instantiate_extensions(__name__) + if group_name is None: + group_name = __name__ + extensions = instantiate_extensions(group_name) for name, extension in extensions.items(): extension.PACKAGE_IDENTIFICATION_NAME = name return order_extensions_grouped_by_priority(extensions) diff --git a/colcon_core/package_selection/__init__.py b/colcon_core/package_selection/__init__.py index 6b1bdc9f0..bdbacd887 100644 --- a/colcon_core/package_selection/__init__.py +++ b/colcon_core/package_selection/__init__.py @@ -85,7 +85,7 @@ def add_arguments(parser): _add_package_selection_arguments(parser) -def get_package_selection_extensions(): +def get_package_selection_extensions(*, group_name=None): """ Get the available package selection extensions. @@ -93,7 +93,9 @@ def get_package_selection_extensions(): :rtype: OrderedDict """ - extensions = instantiate_extensions(__name__) + if group_name is None: + group_name = __name__ + extensions = instantiate_extensions(group_name) for name, extension in extensions.items(): extension.PACKAGE_SELECTION_NAME = name return order_extensions_by_priority(extensions) diff --git a/colcon_core/prefix_path/__init__.py b/colcon_core/prefix_path/__init__.py index 15aae0b2e..4fa34eb88 100644 --- a/colcon_core/prefix_path/__init__.py +++ b/colcon_core/prefix_path/__init__.py @@ -40,7 +40,7 @@ def extend_prefix_path(self, paths): raise NotImplementedError() -def get_prefix_path_extensions(): +def get_prefix_path_extensions(*, group_name=None): """ Get the available prefix path extensions. @@ -49,7 +49,9 @@ def get_prefix_path_extensions(): :rtype: OrderedDict """ - extensions = instantiate_extensions(__name__) + if group_name is None: + group_name = __name__ + extensions = instantiate_extensions(group_name) for name, extension in extensions.items(): extension.PREFIX_PATH_NAME = name return order_extensions_grouped_by_priority(extensions) diff --git a/colcon_core/shell/__init__.py b/colcon_core/shell/__init__.py index faa103ff7..7924b7953 100644 --- a/colcon_core/shell/__init__.py +++ b/colcon_core/shell/__init__.py @@ -273,7 +273,7 @@ async def generate_command_environment( raise NotImplementedError() -def get_shell_extensions(): +def get_shell_extensions(*, group_name=None): """ Get the available shell extensions. @@ -282,7 +282,9 @@ def get_shell_extensions(): :rtype: OrderedDict """ - extensions = instantiate_extensions(__name__) + if group_name is None: + group_name = __name__ + extensions = instantiate_extensions(group_name) for name, extension in extensions.items(): extension.SHELL_NAME = name return order_extensions_grouped_by_priority(extensions) @@ -593,7 +595,7 @@ def find_installed_packages(self, install_base: Path): raise NotImplementedError() -def get_find_installed_packages_extensions(): +def get_find_installed_packages_extensions(*, group_name=None): """ Get the available package identification extensions. @@ -602,7 +604,10 @@ def get_find_installed_packages_extensions(): :rtype: OrderedDict """ - extensions = instantiate_extensions(__name__ + '.find_installed_packages') + if group_name is None: + group_name = __name__ + extensions = instantiate_extensions( + group_name + '.find_installed_packages') for name, extension in extensions.items(): extension.PACKAGE_IDENTIFICATION_NAME = name return order_extensions_grouped_by_priority(extensions) diff --git a/colcon_core/task/python/test/__init__.py b/colcon_core/task/python/test/__init__.py index 74a5ed2b8..a01ec4784 100644 --- a/colcon_core/task/python/test/__init__.py +++ b/colcon_core/task/python/test/__init__.py @@ -137,7 +137,7 @@ async def step(self): raise NotImplementedError() -def get_python_testing_step_extensions(): +def get_python_testing_step_extensions(*, group_name=None): """ Get the available Python testing step extensions. @@ -145,8 +145,9 @@ def get_python_testing_step_extensions(): :rtype: OrderedDict """ - extensions = instantiate_extensions( - 'colcon_core.python_testing', unique_instance=False) + if group_name is None: + group_name = 'colcon_core.python_testing' + extensions = instantiate_extensions(group_name, unique_instance=False) for name in list(extensions.keys()): extension = extensions[name] extension.STEP_TYPE = name diff --git a/colcon_core/verb/__init__.py b/colcon_core/verb/__init__.py index 640dc218f..49913ba16 100644 --- a/colcon_core/verb/__init__.py +++ b/colcon_core/verb/__init__.py @@ -48,7 +48,7 @@ def main(self, *, context): raise NotImplementedError() -def get_verb_extensions(): +def get_verb_extensions(*, group_name=None): """ Get the available verb extensions. @@ -56,7 +56,9 @@ def get_verb_extensions(): :rtype: OrderedDict """ - extensions = instantiate_extensions(__name__) + if group_name is None: + group_name = __name__ + extensions = instantiate_extensions(group_name) for name, extension in extensions.items(): extension.VERB_NAME = name return order_extensions_by_name(extensions) From 5ff4911afe289a7e7c7fdf93f75e427d4e1137cc Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Fri, 24 May 2024 17:06:44 -0700 Subject: [PATCH 27/34] Drop superfluous 'global' statements from command.py (#621) I don't see any reason that these statements should be necessary and find their presence confusing. --- colcon_core/command.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/colcon_core/command.py b/colcon_core/command.py index 228d3b369..bad98e228 100644 --- a/colcon_core/command.py +++ b/colcon_core/command.py @@ -86,7 +86,6 @@ def register_command_exit_handler(handler): :param handler: The callable """ - global _command_exit_handlers if handler not in _command_exit_handlers: _command_exit_handlers.append(handler) @@ -113,7 +112,6 @@ def main(*, command_name='colcon', argv=None): :param list argv: The list of arguments :returns: The return code """ - global _command_exit_handlers try: return _main(command_name=command_name, argv=argv) except KeyboardInterrupt: @@ -126,7 +124,6 @@ def main(*, command_name='colcon', argv=None): def _main(*, command_name, argv): - global colcon_logger # default log level, for searchability: COLCON_LOG_LEVEL colcon_logger.setLevel(logging.WARNING) set_logger_level_from_env( @@ -387,8 +384,6 @@ def create_subparser(parser, cmd_name, verb_extensions, *, attribute): selected verb :returns: The special action object """ - global colcon_logger - # list of available verbs with their descriptions verbs = [] for name, extension in verb_extensions.items(): From d6641a81e8a85db8e7b088cbd7f59010e82f229f Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Wed, 29 May 2024 14:11:03 -0500 Subject: [PATCH 28/34] Suppress E501 in generated prefix util script (#644) We don't generally enforce linters in generated scripts, but it's easy enough to add suppressions to these lines, which vary in length based on the environment. --- colcon_core/shell/template/prefix_util.py.em | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/colcon_core/shell/template/prefix_util.py.em b/colcon_core/shell/template/prefix_util.py.em index 6f96913c3..9ed25d2fd 100644 --- a/colcon_core/shell/template/prefix_util.py.em +++ b/colcon_core/shell/template/prefix_util.py.em @@ -15,9 +15,9 @@ FORMAT_STR_SET_ENV_VAR = '@(shell_extension.FORMAT_STR_SET_ENV_VAR)' @{assert shell_extension.FORMAT_STR_USE_ENV_VAR is not None}@ FORMAT_STR_USE_ENV_VAR = '@(shell_extension.FORMAT_STR_USE_ENV_VAR)' @{assert shell_extension.FORMAT_STR_INVOKE_SCRIPT is not None}@ -FORMAT_STR_INVOKE_SCRIPT = '@(shell_extension.FORMAT_STR_INVOKE_SCRIPT)' -FORMAT_STR_REMOVE_LEADING_SEPARATOR = '@(shell_extension.FORMAT_STR_REMOVE_LEADING_SEPARATOR)' -FORMAT_STR_REMOVE_TRAILING_SEPARATOR = '@(shell_extension.FORMAT_STR_REMOVE_TRAILING_SEPARATOR)' +FORMAT_STR_INVOKE_SCRIPT = '@(shell_extension.FORMAT_STR_INVOKE_SCRIPT)' # noqa: E501 +FORMAT_STR_REMOVE_LEADING_SEPARATOR = '@(shell_extension.FORMAT_STR_REMOVE_LEADING_SEPARATOR)' # noqa: E501 +FORMAT_STR_REMOVE_TRAILING_SEPARATOR = '@(shell_extension.FORMAT_STR_REMOVE_TRAILING_SEPARATOR)' # noqa: E501 DSV_TYPE_APPEND_NON_DUPLICATE = 'append-non-duplicate' DSV_TYPE_PREPEND_NON_DUPLICATE = 'prepend-non-duplicate' From 86eb33b2868ae97586fa30699353afb10832d0cf Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Fri, 31 May 2024 12:16:45 -0500 Subject: [PATCH 29/34] Add colcon_core.logging.get_effective_console_level function (#650) When colcon routes log messages to log files at a different level from the console, it makes it a little more convoluted to determine what log level is actually set. When we're utilizing non-colcon libraries that also use python's logging module, we'll typically want to "synchronize" colcon's configured log level with the other library. This function can be used to determine what level colcon's console logging is set to. --- colcon_core/executor/sequential.py | 4 +++- colcon_core/logging.py | 17 +++++++++++++++++ test/test_logging.py | 26 ++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/colcon_core/executor/sequential.py b/colcon_core/executor/sequential.py index 1aab44e79..eeea8e1e3 100644 --- a/colcon_core/executor/sequential.py +++ b/colcon_core/executor/sequential.py @@ -10,6 +10,7 @@ from colcon_core.executor import ExecutorExtensionPoint from colcon_core.executor import OnError from colcon_core.logging import colcon_logger +from colcon_core.logging import get_effective_console_level from colcon_core.plugin_system import satisfies_version from colcon_core.subprocess import new_event_loop from colcon_core.subprocess import SIGINT_RESULT @@ -32,7 +33,8 @@ def __init__(self): # noqa: D107 def execute(self, args, jobs, *, on_error=OnError.interrupt): # noqa: D102 # avoid debug message from asyncio when colcon uses debug log level asyncio_logger = logging.getLogger('asyncio') - asyncio_logger.setLevel(logging.INFO) + log_level = get_effective_console_level(colcon_logger) + asyncio_logger.setLevel(log_level) rc = 0 loop = new_event_loop() diff --git a/colcon_core/logging.py b/colcon_core/logging.py index 67db572e7..b50ccc945 100644 --- a/colcon_core/logging.py +++ b/colcon_core/logging.py @@ -127,3 +127,20 @@ def format_message_with_relative_time(record): logger.setLevel(1) return handler + + +def get_effective_console_level(logger): + """ + Determine the effective log level of to the console. + + On a typical logger, this is the same as getEffectiveLevel(). After a call + to add_file_handler, this will continue to return the same level though + getEffectiveLevel() will now always return ``1``. + + :param logger: The logger to inspect + :returns: the log level + """ + for handler in logger.handlers: + if isinstance(handler, logging.StreamHandler): + return handler.level + return logger.getEffectiveLevel() diff --git a/test/test_logging.py b/test/test_logging.py index 3de09b98a..350b04a60 100644 --- a/test/test_logging.py +++ b/test/test_logging.py @@ -6,6 +6,7 @@ from unittest.mock import Mock from colcon_core.logging import add_file_handler +from colcon_core.logging import get_effective_console_level from colcon_core.logging import get_numeric_log_level from colcon_core.logging import set_logger_level_from_env import pytest @@ -76,3 +77,28 @@ def test_add_file_handler(tmpdir): # check only that we logged SOMETHING to the file assert log_path.stat().st_size > 10 + + +def test_get_effective_console_level(tmpdir): + logger = logging.getLogger('test_sync_console_log_level') + + # no level set + level = get_effective_console_level(logger) + assert level == logger.getEffectiveLevel() + + # change the level to ERROR + logger.setLevel(logging.ERROR) + level = get_effective_console_level(logger) + assert level == logger.getEffectiveLevel() == logging.ERROR + + # after add_file_handler + log_path = Path(tmpdir) / 'test_add_file_handler.log' + log_path.touch() + try: + add_file_handler(logger, log_path) + level = get_effective_console_level(logger) + assert level == logging.ERROR + finally: + for handler in logger.handlers: + logger.removeHandler(handler) + handler.close() From 485f5009205d7b50d3a21bb5e889cd94b42027a1 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Fri, 31 May 2024 16:15:23 -0500 Subject: [PATCH 30/34] Make distutils symlink_data command private (#645) We don't want anyone taking a dependency on this functionality, and we don't want to interfere with non-colcon use of setuptools or distutils, so it's best to just hide this entry point and make it available only when we're running our builds. --- colcon_core/task/python/build.py | 3 +++ .../task/python/colcon_distutils_commands/__init__.py | 0 .../colcon_distutils_commands-0.0.0.dist-info/METADATA | 3 +++ .../entry_points.txt | 2 ++ setup.cfg | 5 +++-- 5 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 colcon_core/task/python/colcon_distutils_commands/__init__.py create mode 100644 colcon_core/task/python/colcon_distutils_commands/colcon_distutils_commands-0.0.0.dist-info/METADATA create mode 100644 colcon_core/task/python/colcon_distutils_commands/colcon_distutils_commands-0.0.0.dist-info/entry_points.txt diff --git a/colcon_core/task/python/build.py b/colcon_core/task/python/build.py index 442d5a620..a72c1a54e 100644 --- a/colcon_core/task/python/build.py +++ b/colcon_core/task/python/build.py @@ -77,9 +77,12 @@ async def build(self, *, additional_hooks=None): # noqa: D102 python_lib = os.path.join( args.install_base, self._get_python_lib(args)) os.makedirs(python_lib, exist_ok=True) + distutils_commands = os.path.join( + os.path.dirname(__file__), 'colcon_distutils_commands') # and being in the PYTHONPATH env = dict(env) env['PYTHONPATH'] = str(prefix_override) + os.pathsep + \ + distutils_commands + os.pathsep + \ python_lib + os.pathsep + env.get('PYTHONPATH', '') # coverage capture interferes with sitecustomize # See also: https://docs.python.org/3/library/site.html#module-site diff --git a/colcon_core/task/python/colcon_distutils_commands/__init__.py b/colcon_core/task/python/colcon_distutils_commands/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/colcon_core/task/python/colcon_distutils_commands/colcon_distutils_commands-0.0.0.dist-info/METADATA b/colcon_core/task/python/colcon_distutils_commands/colcon_distutils_commands-0.0.0.dist-info/METADATA new file mode 100644 index 000000000..c97b22f57 --- /dev/null +++ b/colcon_core/task/python/colcon_distutils_commands/colcon_distutils_commands-0.0.0.dist-info/METADATA @@ -0,0 +1,3 @@ +Metadata-Version: 2.1 +Name: colcon_distutils_commands +Version: 0.0.0 diff --git a/colcon_core/task/python/colcon_distutils_commands/colcon_distutils_commands-0.0.0.dist-info/entry_points.txt b/colcon_core/task/python/colcon_distutils_commands/colcon_distutils_commands-0.0.0.dist-info/entry_points.txt new file mode 100644 index 000000000..cade4c272 --- /dev/null +++ b/colcon_core/task/python/colcon_distutils_commands/colcon_distutils_commands-0.0.0.dist-info/entry_points.txt @@ -0,0 +1,2 @@ +[distutils.commands] +symlink_data = colcon_core.distutils.commands.symlink_data:symlink_data diff --git a/setup.cfg b/setup.cfg index 85391702f..3928d02e7 100644 --- a/setup.cfg +++ b/setup.cfg @@ -145,14 +145,15 @@ colcon_core.verb = test = colcon_core.verb.test:TestVerb console_scripts = colcon = colcon_core.command:main -distutils.commands = - symlink_data = colcon_core.distutils.commands.symlink_data:symlink_data pytest11 = colcon_core_warnings_stderr = colcon_core.pytest.hooks [options.package_data] colcon_core.shell.template = *.em colcon_core.task.python.template = *.em +colcon_core.task.python.colcon_distutils_commands = + */METADATA + */entry_points.txt [flake8] import-order-style = google From 37dc1ddd1c8d09b75e10cc686b9b16965ccc2e6a Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Fri, 31 May 2024 16:19:34 -0500 Subject: [PATCH 31/34] Update colcon_core.command for generalized usage (#651) These non-breaking API changes are helpful for creating other CLIs based on colcon's extension model. --- colcon_core/command.py | 24 ++++++++++++++------ colcon_core/extension_point.py | 20 +++++++++++++++-- test/test_extension_point.py | 41 ++++++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 9 deletions(-) diff --git a/colcon_core/command.py b/colcon_core/command.py index bad98e228..10668edc6 100644 --- a/colcon_core/command.py +++ b/colcon_core/command.py @@ -90,7 +90,10 @@ def register_command_exit_handler(handler): _command_exit_handlers.append(handler) -def main(*, command_name='colcon', argv=None): +def main( + *, command_name='colcon', argv=None, verb_group_name=None, + environment_variable_group_name=None, +): """ Execute the main logic of the command. @@ -113,7 +116,10 @@ def main(*, command_name='colcon', argv=None): :returns: The return code """ try: - return _main(command_name=command_name, argv=argv) + return _main( + command_name=command_name, argv=argv, + verb_group_name=verb_group_name, + environment_variable_group_name=environment_variable_group_name) except KeyboardInterrupt: return signal.SIGINT finally: @@ -123,7 +129,9 @@ def main(*, command_name='colcon', argv=None): handler() -def _main(*, command_name, argv): +def _main( + *, command_name, argv, verb_group_name, environment_variable_group_name, +): # default log level, for searchability: COLCON_LOG_LEVEL colcon_logger.setLevel(logging.WARNING) set_logger_level_from_env( @@ -137,9 +145,9 @@ def _main(*, command_name, argv): path=(Path('~') / f'.{command_name}').expanduser(), env_var=f'{command_name}_HOME'.upper()) - parser = create_parser('colcon_core.environment_variable') + parser = create_parser(environment_variable_group_name) - verb_extensions = get_verb_extensions() + verb_extensions = get_verb_extensions(group_name=verb_group_name) # add subparsers for all verb extensions but without arguments for now subparser = create_subparser( @@ -203,7 +211,7 @@ def _main(*, command_name, argv): return verb_main(context, colcon_logger) -def create_parser(environment_variables_group_name): +def create_parser(environment_variables_group_name=None): """ Create the argument parser. @@ -283,7 +291,7 @@ def _split_lines(self, text, width): return lines -def get_environment_variables_epilog(group_name): +def get_environment_variables_epilog(group_name=None): """ Get a message enumerating the registered environment variables. @@ -292,6 +300,8 @@ def get_environment_variables_epilog(group_name): :returns: The message for the argument parser epilog :rtype: str """ + if group_name is None: + group_name = 'colcon_core.environment_variable' # list environment variables with descriptions entry_points = load_extension_points(group_name) if not entry_points: diff --git a/colcon_core/extension_point.py b/colcon_core/extension_point.py index 4bea3fc6b..e04ee0fdc 100644 --- a/colcon_core/extension_point.py +++ b/colcon_core/extension_point.py @@ -22,11 +22,14 @@ from colcon_core.environment_variable import EnvironmentVariable from colcon_core.logging import colcon_logger -"""Environment variable to block extensions""" -EXTENSION_BLOCKLIST_ENVIRONMENT_VARIABLE = EnvironmentVariable( +_EXTENSION_BLOCKLIST_ENVIRONMENT_VARIABLE = EnvironmentVariable( 'COLCON_EXTENSION_BLOCKLIST', 'Block extensions which should not be used') +"""Environment variable to block extensions""" +EXTENSION_BLOCKLIST_ENVIRONMENT_VARIABLE = \ + _EXTENSION_BLOCKLIST_ENVIRONMENT_VARIABLE + logger = colcon_logger.getChild(__name__) """ @@ -205,3 +208,16 @@ def load_extension_point(name, value, group): 'The entry point name is listed in the environment variable ' f"'{EXTENSION_BLOCKLIST_ENVIRONMENT_VARIABLE.name}'") return EntryPoint(name, value, group).load() + + +def override_blocklist_variable(variable): + """ + Override the blocklist environment variable. + + :param EnvironmentVariable variable: The new blocklist environment + variable, or None to reset to default. + """ + if variable is None: + variable = _EXTENSION_BLOCKLIST_ENVIRONMENT_VARIABLE + global EXTENSION_BLOCKLIST_ENVIRONMENT_VARIABLE + EXTENSION_BLOCKLIST_ENVIRONMENT_VARIABLE = variable diff --git a/test/test_extension_point.py b/test/test_extension_point.py index 63f89edbb..f0fa80438 100644 --- a/test/test_extension_point.py +++ b/test/test_extension_point.py @@ -12,6 +12,7 @@ # TODO: Drop this with Python 3.7 support from importlib_metadata import Distribution +from colcon_core.environment_variable import EnvironmentVariable from colcon_core.extension_point import clear_entry_point_cache from colcon_core.extension_point import EntryPoint from colcon_core.extension_point import EXTENSION_POINT_GROUP_NAME @@ -19,6 +20,7 @@ from colcon_core.extension_point import get_extension_points from colcon_core.extension_point import load_extension_point from colcon_core.extension_point import load_extension_points +from colcon_core.extension_point import override_blocklist_variable import pytest from .environment_context import EnvironmentContext @@ -139,6 +141,45 @@ def test_extension_point_blocklist(): assert load.call_count == 0 +def test_extension_point_blocklist_override(): + with patch.object(EntryPoint, 'load', return_value=None) as load: + clear_entry_point_cache() + + my_extension_blocklist = EnvironmentVariable( + 'MY_EXTENSION_BLOCKLIST', 'Foo bar baz') + override_blocklist_variable(my_extension_blocklist) + + try: + # entry point in default blocklist variable can be loaded + load.reset_mock() + with EnvironmentContext(COLCON_EXTENSION_BLOCKLIST='group1'): + clear_entry_point_cache() + load_extension_point('extA', 'eA', 'group1') + assert load.call_count == 1 + + # entry point in custom blocklist variable can't be loaded + load.reset_mock() + with EnvironmentContext(MY_EXTENSION_BLOCKLIST='group1'): + clear_entry_point_cache() + with pytest.raises(RuntimeError) as e: + load_extension_point('extA', 'eA', 'group1') + assert 'The entry point group name is listed in the ' \ + 'environment variable' in str(e.value) + assert load.call_count == 0 + finally: + override_blocklist_variable(None) + + # entry point in default blocklist variable can no longer be loaded + load.reset_mock() + with EnvironmentContext(COLCON_EXTENSION_BLOCKLIST='group1'): + clear_entry_point_cache() + with pytest.raises(RuntimeError) as e: + load_extension_point('extA', 'eA', 'group1') + assert 'The entry point group name is listed in the ' \ + 'environment variable' in str(e.value) + assert load.call_count == 0 + + def test_redefined_extension_point(): def _duped_distributions(): yield from _distributions() From 857ea3f58fcf0bddd4ed2a30a727f3df0f6bf16f Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Fri, 31 May 2024 18:19:01 -0500 Subject: [PATCH 32/34] Add central interface for defining feature flags (#640) The intended use for colcon feature flags is to ship pre-production and prototype features in a disabled state, which can be enabled by specifying a particular environment variable value. By using an environment variable, these possibly dangerous or unstable features are hidden from common users but are enabled in a way which can be audited. --- colcon_core/command.py | 4 ++ colcon_core/feature_flags.py | 71 +++++++++++++++++++++++ test/spell_check.words | 4 ++ test/test_feature_flags.py | 106 +++++++++++++++++++++++++++++++++++ 4 files changed, 185 insertions(+) create mode 100644 colcon_core/feature_flags.py create mode 100644 test/test_feature_flags.py diff --git a/colcon_core/command.py b/colcon_core/command.py index 10668edc6..901062d48 100644 --- a/colcon_core/command.py +++ b/colcon_core/command.py @@ -52,6 +52,7 @@ from colcon_core.argument_parser import decorate_argument_parser # noqa: E402 E501 I100 I202 from colcon_core.argument_parser import SuppressUsageOutput # noqa: E402 from colcon_core.extension_point import load_extension_points # noqa: E402 +from colcon_core.feature_flags import check_implemented_flags # noqa: E402 from colcon_core.location import create_log_path # noqa: E402 from colcon_core.location import get_log_path # noqa: E402 from colcon_core.location import set_default_config_path # noqa: E402 @@ -140,6 +141,9 @@ def _main( 'Command line arguments: {argv}' .format(argv=argv if argv is not None else sys.argv)) + # warn about any specified feature flags that are already implemented + check_implemented_flags() + # set default locations for config files, for searchability: COLCON_HOME set_default_config_path( path=(Path('~') / f'.{command_name}').expanduser(), diff --git a/colcon_core/feature_flags.py b/colcon_core/feature_flags.py new file mode 100644 index 000000000..56bb5bec4 --- /dev/null +++ b/colcon_core/feature_flags.py @@ -0,0 +1,71 @@ +# Copyright 2024 Open Source Robotics Foundation, Inc. +# Licensed under the Apache License, Version 2.0 + +import os + +from colcon_core.environment_variable import EnvironmentVariable +from colcon_core.logging import colcon_logger + +logger = colcon_logger.getChild(__name__) + +"""Environment variable to enable feature flags""" +FEATURE_FLAGS_ENVIRONMENT_VARIABLE = EnvironmentVariable( + 'COLCON_FEATURE_FLAGS', + 'Enable pre-production features and behaviors') + +_REPORTED_USES = set() + +IMPLEMENTED_FLAGS = set() + + +def check_implemented_flags(): + """Check for and warn about flags which have been implemented.""" + implemented = IMPLEMENTED_FLAGS.intersection(get_feature_flags()) + if implemented: + logger.warning( + 'The following feature flags have been implemented and should no ' + 'longer be specified in ' + f'{FEATURE_FLAGS_ENVIRONMENT_VARIABLE.name}: ' + f"{','.join(implemented)}") + + +def get_feature_flags(): + """ + Retrieve all enabled feature flags. + + :returns: List of enabled flags + :rtype: list + """ + return [ + flag for flag in ( + os.environ.get(FEATURE_FLAGS_ENVIRONMENT_VARIABLE.name) or '' + ).split(os.pathsep) if flag + ] + + +def is_feature_flag_set(flag): + """ + Determine if a specific feature flag is enabled. + + Feature flags are case-sensitive and separated by the os-specific path + separator character. + + :param str flag: Name of the flag to search for + + :returns: True if the flag is set + :rtype: bool + """ + if flag in IMPLEMENTED_FLAGS: + return True + elif flag in get_feature_flags(): + if flag not in _REPORTED_USES: + if not _REPORTED_USES: + logger.warning( + 'One or more feature flags have been enabled using the ' + f'{FEATURE_FLAGS_ENVIRONMENT_VARIABLE.name} environment ' + 'variable. These features may be unstable and may change ' + 'API or behavior at any time.') + logger.warning(f'Enabling feature: {flag}') + _REPORTED_USES.add(flag) + return True + return False diff --git a/test/spell_check.words b/test/spell_check.words index 07ac11178..552636bd4 100644 --- a/test/spell_check.words +++ b/test/spell_check.words @@ -1,3 +1,4 @@ +addfinalizer addopts apache argparse @@ -35,8 +36,10 @@ docstring executables exitstatus fdopen +ffoo filterwarnings foobar +fooo fromhex functools getcategory @@ -140,5 +143,6 @@ unittest unittests unlinking unrenamed +usefixtures wildcards workaround diff --git a/test/test_feature_flags.py b/test/test_feature_flags.py new file mode 100644 index 000000000..218b29773 --- /dev/null +++ b/test/test_feature_flags.py @@ -0,0 +1,106 @@ +# Copyright 2024 Open Source Robotics Foundation, Inc. +# Licensed under the Apache License, Version 2.0 + +import os +from unittest.mock import patch + +from colcon_core.feature_flags import check_implemented_flags +from colcon_core.feature_flags import FEATURE_FLAGS_ENVIRONMENT_VARIABLE +from colcon_core.feature_flags import get_feature_flags +from colcon_core.feature_flags import is_feature_flag_set +import pytest + + +_FLAGS_TO_TEST = ( + ('foo',), + ('foo', 'foo'), + ('foo', ''), + ('', 'foo'), + ('', 'foo', ''), + ('foo', 'bar'), + ('bar', 'foo'), + ('bar', 'foo', 'baz'), +) + + +@pytest.fixture +def feature_flags_value(request): + env = dict(os.environ) + if request.param is not None: + env[FEATURE_FLAGS_ENVIRONMENT_VARIABLE.name] = os.pathsep.join( + request.param) + else: + env.pop(FEATURE_FLAGS_ENVIRONMENT_VARIABLE.name, None) + + mock_env = patch('colcon_core.feature_flags.os.environ', env) + request.addfinalizer(mock_env.stop) + mock_env.start() + return request.param + + +@pytest.fixture +def feature_flag_reports(request): + reported_uses = patch('colcon_core.feature_flags._REPORTED_USES', set()) + request.addfinalizer(reported_uses.stop) + reported_uses.start() + return reported_uses + + +@pytest.mark.parametrize( + 'feature_flags_value', + _FLAGS_TO_TEST, + indirect=True) +@pytest.mark.usefixtures('feature_flags_value', 'feature_flag_reports') +def test_flag_is_set(): + with patch('colcon_core.feature_flags.logger.warning') as warn: + assert is_feature_flag_set('foo') + assert warn.call_count == 2 + assert is_feature_flag_set('foo') + assert warn.call_count == 2 + + +@pytest.mark.parametrize( + 'feature_flags_value', + (None, *_FLAGS_TO_TEST), + indirect=True) +@pytest.mark.usefixtures('feature_flags_value', 'feature_flag_reports') +def test_flag_not_set(): + with patch('colcon_core.feature_flags.logger.warning') as warn: + assert not is_feature_flag_set('') + assert not is_feature_flag_set('fo') + assert not is_feature_flag_set('oo') + assert not is_feature_flag_set('fooo') + assert not is_feature_flag_set('ffoo') + assert not is_feature_flag_set('qux') + assert warn.call_count == 0 + + +@pytest.mark.parametrize( + 'feature_flags_value', + (None, *_FLAGS_TO_TEST), + indirect=True) +@pytest.mark.usefixtures('feature_flags_value') +def test_get_flags(feature_flags_value): + assert [ + flag for flag in (feature_flags_value or ()) if flag + ] == get_feature_flags() + + +@pytest.mark.parametrize('feature_flags_value', (('baz',),), indirect=True) +@pytest.mark.usefixtures('feature_flags_value') +def test_implemented(): + with patch('colcon_core.feature_flags.IMPLEMENTED_FLAGS', {'foo'}): + with patch('colcon_core.feature_flags.logger.warning') as warn: + assert not is_feature_flag_set('bar') + assert warn.call_count == 0 + assert is_feature_flag_set('baz') + assert warn.call_count == 2 + assert is_feature_flag_set('foo') + assert warn.call_count == 2 + check_implemented_flags() + assert warn.call_count == 2 + + with patch('colcon_core.feature_flags.IMPLEMENTED_FLAGS', {'baz'}): + with patch('colcon_core.feature_flags.logger.warning') as warn: + check_implemented_flags() + assert warn.call_count == 1 From 1aa845d497757a03636881fd64677e21a46e6c23 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Mon, 3 Jun 2024 14:03:56 -0500 Subject: [PATCH 33/34] Re-work colcon_core.command.get_prog_name (#617) This function's purpose is to handle these special cases of argv[0]: * Invoked using python -m ... * Invoked using a path to the executable even though the executable is on the PATH This change enhances the path comparison to support normalization of that path, Windows long path prefixes, and also the easy-install behavior on Windows where argv[0] has no extension. Yet to be properly handled is invocation using python -c ... --- colcon_core/command.py | 25 ++++++++++-- test/spell_check.words | 1 + test/test_command.py | 86 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 3 deletions(-) diff --git a/colcon_core/command.py b/colcon_core/command.py index 901062d48..9c94118c6 100644 --- a/colcon_core/command.py +++ b/colcon_core/command.py @@ -275,9 +275,28 @@ def get_prog_name(): if basename == '__main__.py': # use the module name in case the script was invoked with python -m ... prog = os.path.basename(os.path.dirname(prog)) - elif shutil.which(basename) == prog: - # use basename only if it is on the PATH - prog = basename + else: + default_prog = shutil.which(basename) or '' + default_ext = os.path.splitext(default_prog)[1] + real_prog = prog + if ( + sys.platform == 'win32' and + os.path.splitext(real_prog)[1] != default_ext + ): + # On Windows, setuptools entry points drop the file extension from + # argv[0], but shutil.which does not. If the two don't end in the + # same extension, try appending the shutil extension for a better + # chance at matching. + real_prog += default_ext + try: + # The os.path.samefile requires that both files exist on disk, but + # has the advantage of working around symlinks, UNC-style paths, + # DOS 8.3 path atoms, and path normalization. + if os.path.samefile(default_prog, real_prog): + # use basename only if it is on the PATH + prog = basename + except (FileNotFoundError, NotADirectoryError): + pass return prog diff --git a/test/spell_check.words b/test/spell_check.words index 552636bd4..18c2677bf 100644 --- a/test/spell_check.words +++ b/test/spell_check.words @@ -116,6 +116,7 @@ setuptools shlex sigint sitecustomize +skipif sloretz stacklevel staticmethod diff --git a/test/test_command.py b/test/test_command.py index d2aca2d2c..c8c6b6e9e 100644 --- a/test/test_command.py +++ b/test/test_command.py @@ -1,15 +1,18 @@ # Copyright 2016-2018 Dirk Thomas # Licensed under the Apache License, Version 2.0 +import os import shutil import signal import sys from tempfile import mkdtemp +from tempfile import TemporaryDirectory from unittest.mock import Mock from unittest.mock import patch from colcon_core.command import CommandContext from colcon_core.command import create_parser +from colcon_core.command import get_prog_name from colcon_core.command import main from colcon_core.command import verb_main from colcon_core.environment_variable import EnvironmentVariable @@ -151,3 +154,86 @@ def test_verb_main(): assert logger.error.call_args[0][0].startswith( 'command_name verb_name: custom error message\n') assert 'Exception: custom error message' in logger.error.call_args[0][0] + + +def test_prog_name_module(): + argv = [os.path.join('foo', 'bar', '__main__.py')] + with patch('colcon_core.command.sys.argv', argv): + # prog should be the module containing __main__.py + assert get_prog_name() == 'bar' + + +def test_prog_name_on_path(): + # use __file__ since we know it exists + argv = [__file__] + with patch('colcon_core.command.sys.argv', argv): + with patch( + 'colcon_core.command.shutil.which', + return_value=__file__ + ): + # prog should be shortened to the basename + assert get_prog_name() == 'test_command.py' + + +def test_prog_name_not_on_path(): + # use __file__ since we know it exists + argv = [__file__] + with patch('colcon_core.command.sys.argv', argv): + with patch('colcon_core.command.shutil.which', return_value=None): + # prog should remain unchanged + assert get_prog_name() == __file__ + + +def test_prog_name_different_on_path(): + # use __file__ since we know it exists + argv = [__file__] + with patch('colcon_core.command.sys.argv', argv): + with patch( + 'colcon_core.command.shutil.which', + return_value=sys.executable + ): + # prog should remain unchanged + assert get_prog_name() == __file__ + + +def test_prog_name_not_a_file(): + # pick some file that doesn't actually exist on disk + no_such_file = os.path.join(__file__, 'foobar') + argv = [no_such_file] + with patch('colcon_core.command.sys.argv', argv): + with patch( + 'colcon_core.command.shutil.which', + return_value=no_such_file + ): + # prog should remain unchanged + assert get_prog_name() == no_such_file + + +@pytest.mark.skipif(sys.platform == 'win32', reason='Symlinks not supported.') +def test_prog_name_symlink(): + # use __file__ since we know it exists + with TemporaryDirectory(prefix='test_colcon_') as temp_dir: + linked_file = os.path.join(temp_dir, 'test_command.py') + os.symlink(__file__, linked_file) + + argv = [linked_file] + with patch('colcon_core.command.sys.argv', argv): + with patch( + 'colcon_core.command.shutil.which', + return_value=__file__ + ): + # prog should be shortened to the basename + assert get_prog_name() == 'test_command.py' + + +@pytest.mark.skipif(sys.platform != 'win32', reason='Only valid on Windows.') +def test_prog_name_easy_install(): + # use __file__ since we know it exists + argv = [__file__[:-3]] + with patch('colcon_core.command.sys.argv', argv): + with patch( + 'colcon_core.command.shutil.which', + return_value=__file__ + ): + # prog should be shortened to the basename + assert get_prog_name() == 'test_command' From 520052df1cc8cf86ab09bbcc7d2ca4e421cb7098 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Mon, 3 Jun 2024 14:07:03 -0500 Subject: [PATCH 34/34] Support complex recursive dependency category specification (#646) When computing the dependency graph, the existing API exposes a parameter for indicating which direct dependency categories to collect as well as what indirect (recursive) dependency categories to collect. This change allows the caller to specify different recursive dependency categories depending on which category included the dependency in the graph to begin with. --- colcon_core/package_decorator.py | 5 ++-- colcon_core/package_descriptor.py | 18 +++++++++--- colcon_core/package_selection/__init__.py | 5 ++-- test/test_package_descriptor.py | 34 +++++++++++++++++++++-- 4 files changed, 51 insertions(+), 11 deletions(-) diff --git a/colcon_core/package_decorator.py b/colcon_core/package_decorator.py index 256b7c859..3226da1a1 100644 --- a/colcon_core/package_decorator.py +++ b/colcon_core/package_decorator.py @@ -46,8 +46,9 @@ def add_recursive_dependencies( :param set decorators: The known packages to consider :param Iterable[str] direct_categories: The names of the direct categories - :param Iterable[str] recursive_categories: The names of the recursive - categories + :param Iterable[str]|Mapping[str, Iterable[str]] recursive_categories: + The names of the recursive categories, optionally mapped from the + immediate upstream category which included the dependency """ descriptors = [decorator.descriptor for decorator in decorators] for decorator in decorators: diff --git a/colcon_core/package_descriptor.py b/colcon_core/package_descriptor.py index 0e61006dc..bffdf9e5f 100644 --- a/colcon_core/package_descriptor.py +++ b/colcon_core/package_descriptor.py @@ -2,6 +2,7 @@ # Licensed under the Apache License, Version 2.0 from collections import defaultdict +from collections.abc import Mapping from copy import deepcopy import os from pathlib import Path @@ -105,12 +106,15 @@ def get_recursive_dependencies( consider :param Iterable[str] direct_categories: The names of the direct categories - :param Iterable[str] recursive_categories: The names of the recursive - categories + :param Iterable[str]|Mapping[str, Iterable[str]] recursive_categories: + The names of the recursive categories, optionally mapped from the + immediate upstream category which included the dependency :returns: The dependencies :rtype: set[DependencyDescriptor] :raises AssertionError: if a package lists itself as a dependency """ + if not isinstance(recursive_categories, Mapping): + recursive_categories = defaultdict(lambda: recursive_categories) # the following variable only exists for faster access within the loop descriptors_by_name = defaultdict(set) for d in descriptors: @@ -132,11 +136,17 @@ def get_recursive_dependencies( descs = descriptors_by_name[dep] if not descs: continue + categories = set() + for category in dep.metadata['categories']: + cats = recursive_categories.get(category) + if cats is None: + categories = None + break + categories.update(cats) # recursing into the same function of the dependency descriptor # queue recursive dependencies for d in descs: - queue |= d.get_dependencies( - categories=recursive_categories) + queue |= d.get_dependencies(categories=categories) # add the depth dep.metadata['depth'] = depth # add dependency to result set diff --git a/colcon_core/package_selection/__init__.py b/colcon_core/package_selection/__init__.py index bdbacd887..e76a474b0 100644 --- a/colcon_core/package_selection/__init__.py +++ b/colcon_core/package_selection/__init__.py @@ -138,8 +138,9 @@ def get_packages( :param additional_argument_names: A list of additional arguments to consider :param Iterable[str] direct_categories: The names of the direct categories - :param Iterable[str] recursive_categories: The names of the recursive - categories + :param Iterable[str]|Mapping[str, Iterable[str]] recursive_categories: + The names of the recursive categories, optionally mapped from the + immediate upstream category which included the dependency :rtype: list :raises RuntimeError: if the returned set of packages contains duplicates package names diff --git a/test/test_package_descriptor.py b/test/test_package_descriptor.py index 7c53c09a2..4baf7b247 100644 --- a/test/test_package_descriptor.py +++ b/test/test_package_descriptor.py @@ -1,6 +1,7 @@ # Copyright 2016-2018 Dirk Thomas # Licensed under the Apache License, Version 2.0 +from collections import defaultdict import os from pathlib import Path @@ -49,7 +50,8 @@ def test_get_dependencies(): assert "'self'" in str(e.value) -def test_get_recursive_dependencies(): +@pytest.fixture +def recursive_dependencies(): d = PackageDescriptor('/some/path') d.name = 'A' d.dependencies['build'].add('B') @@ -70,6 +72,7 @@ def test_get_recursive_dependencies(): d3.dependencies['build'].add('h') d3.dependencies['test'].add('G') d3.dependencies['test'].add('I') + d3.dependencies['test'].add('J') d4 = PackageDescriptor('/more/path') d4.name = 'G' @@ -80,10 +83,35 @@ def test_get_recursive_dependencies(): # circular dependencies should be ignored d5.dependencies['run'].add('A') - rec_deps = d.get_recursive_dependencies( - {d, d1, d2, d3, d4, d5}, + d6 = PackageDescriptor('/paths/galore') + d6.name = 'J' + + return d, {d, d1, d2, d3, d4, d5, d6} + + +def test_get_recursive_dependencies(recursive_dependencies): + desc, all_descs = recursive_dependencies + rec_deps = desc.get_recursive_dependencies( + all_descs, direct_categories=('build', 'run'), recursive_categories=('run', 'test')) + assert rec_deps == { + # direct dependencies + 'B', + # recursive dependencies + 'F', 'G', 'I', 'J', + } + + +def test_get_recursive_dependencies_map(recursive_dependencies): + recursive_categories = defaultdict(lambda: ('run', 'test')) + recursive_categories['run'] = ('run',) + + desc, all_descs = recursive_dependencies + rec_deps = desc.get_recursive_dependencies( + all_descs, + direct_categories=('build', 'run'), + recursive_categories=recursive_categories) assert rec_deps == { # direct dependencies 'B',