From a685ec8454dce73cb40d4d5fe72ee8e5e3d7823f Mon Sep 17 00:00:00 2001 From: Clara Berendsen Date: Thu, 30 Nov 2023 11:41:45 -0300 Subject: [PATCH 01/10] 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 0281aeeb..04cae833 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 9c4793cd..08bafc8e 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/10] 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 46eba0d4..75aa1048 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/10] 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 21c54fc4..fdf5d6ab 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/10] 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 27060ef1..630935f5 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/10] 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 ff348ba5..c724d221 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 7111b796..96e58a0d 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/10] 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 04cae833..34eea709 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/10] 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 bba60f3d..aca03575 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 34eea709..99e4626d 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/10] 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 fdf5d6ab..4654486c 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/10] 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 ef5298bc..74a5ed2b 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 a1cb3d99..28095ef9 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/10] 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 28095ef9..d44f7a23 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 630935f5..7780a567 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)