From 1f8a088b90b46bf514190d3fe2885e9eb17aa635 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miro=20Hron=C4=8Dok?= Date: Sat, 26 Jun 2021 16:24:21 +0200 Subject: [PATCH 1/9] %pytest: Set $PYTEST_ADDOPTS when %{__pytest_addopts} is defined Related to https://bugzilla.redhat.com/show_bug.cgi?id=1935212 --- macros.python3 | 1 + python-rpm-macros.spec | 6 +++++- tests/test_evals.py | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/macros.python3 b/macros.python3 index 323e675..38fd8e3 100644 --- a/macros.python3 +++ b/macros.python3 @@ -86,4 +86,5 @@ PATH="%{buildroot}%{_bindir}:$PATH"\\\ PYTHONPATH="${PYTHONPATH:-%{buildroot}%{python3_sitearch}:%{buildroot}%{python3_sitelib}}"\\\ PYTHONDONTWRITEBYTECODE=1\\\ + %{?__pytest_addopts:PYTEST_ADDOPTS="${PYTEST_ADDOPTS:-} %{__pytest_addopts}"}\\\ %__pytest} diff --git a/python-rpm-macros.spec b/python-rpm-macros.spec index 20ec1d7..f75c68c 100644 --- a/python-rpm-macros.spec +++ b/python-rpm-macros.spec @@ -22,7 +22,7 @@ License: MIT and Python # The macro is defined in python-srpm-macros. %{?load:%{SOURCE102}} Version: %{__default_python3_version} -Release: 36%{?dist} +Release: 37%{?dist} BuildArch: noarch @@ -95,6 +95,10 @@ install -m 644 compileall2.py %{buildroot}%{_rpmconfigdir}/redhat/ %changelog +* Mon Jun 28 2021 Miro Hrončok - 3.9-37 +- %%pytest: Set $PYTEST_ADDOPTS when %%{__pytest_addopts} is defined +- Related: rhzb#1935212 + * Mon Mar 29 2021 Miro Hrončok - 3.9-36 - Allow commas as argument separator for extras names in %%python_extras_subpkg - Fixes: rhbz#1936486 diff --git a/tests/test_evals.py b/tests/test_evals.py index e2909c3..dff88ba 100644 --- a/tests/test_evals.py +++ b/tests/test_evals.py @@ -199,6 +199,39 @@ def test_pytest_command_suffix(): assert '/usr/bin/pytest-3.6 -v' in lines[-1] +def test_pytest_undefined_addopts_are_not_set(): + lines = rpm_eval('%pytest', __pytest_addopts=None) + assert 'PYTEST_ADDOPTS' not in '\n'.join(lines) + + +def test_pytest_defined_addopts_are_set(): + lines = rpm_eval('%pytest', __pytest_addopts="--ignore=stuff") + assert 'PYTEST_ADDOPTS="${PYTEST_ADDOPTS:-} --ignore=stuff"' in '\n'.join(lines) + + +@pytest.mark.parametrize('__pytest_addopts', ['--macronized-option', 'x y z', None]) +def test_pytest_addopts_preserves_envvar(__pytest_addopts): + # this is the line a packager might put in the spec file before running %pytest: + spec_line = 'export PYTEST_ADDOPTS="--exported-option1 --exported-option2"' + + # instead of actually running /usr/bin/pytest, + # we run a small shell script that echoes the tested value for inspection + lines = rpm_eval('%pytest', __pytest_addopts=__pytest_addopts, + __pytest="sh -c 'echo $PYTEST_ADDOPTS'") + + echoed = shell_stdout('\n'.join([spec_line] + lines)) + + # assert all values were echoed + assert '--exported-option1' in echoed + assert '--exported-option2' in echoed + if __pytest_addopts is not None: + assert __pytest_addopts in echoed + + # assert the options are separated + assert 'option--' not in echoed + assert 'z--' not in echoed + + def test_pypi_source_default_name(): url = rpm_eval('%pypi_source', name='foo', version='6')[0] From 23849da5174b3356a2d3bde06dc858414fdd58e0 Mon Sep 17 00:00:00 2001 From: Karolina Surma Date: Wed, 7 Apr 2021 16:48:04 +0200 Subject: [PATCH 2/9] Test %python3_sitelib and %python3_sitearch Cherry-picked from 9d2fcef3 and 39166a7b. --- tests/test_evals.py | 98 ++++++++++++++++++++++++++++++--------------- tests/tests.yml | 1 + 2 files changed, 67 insertions(+), 32 deletions(-) diff --git a/tests/test_evals.py b/tests/test_evals.py index dff88ba..0392a4d 100644 --- a/tests/test_evals.py +++ b/tests/test_evals.py @@ -38,6 +38,17 @@ def rpm_eval(expression, fails=False, **kwargs): return cp.stdout.strip().splitlines() +@pytest.fixture(scope="session") +def lib(): + lib_eval = rpm_eval("%_lib")[0] + if lib_eval == "%_lib" and TESTED_FILES: + raise ValueError( + "%_lib is not resolved to an actual value. " + "You may want to include /usr/lib/rpm/platform/x86_64-linux/macros to TESTED_FILES." + ) + return lib_eval + + def shell_stdout(script): return subprocess.check_output(script, env={**os.environ, 'LANG': 'C.utf-8'}, @@ -233,57 +244,57 @@ def test_pytest_addopts_preserves_envvar(__pytest_addopts): def test_pypi_source_default_name(): - url = rpm_eval('%pypi_source', - name='foo', version='6')[0] - assert url == 'https://files.pythonhosted.org/packages/source/f/foo/foo-6.tar.gz' + urls = rpm_eval('%pypi_source', + name='foo', version='6') + assert urls == ['https://files.pythonhosted.org/packages/source/f/foo/foo-6.tar.gz'] def test_pypi_source_default_srcname(): - url = rpm_eval('%pypi_source', - name='python-foo', srcname='foo', version='6')[0] - assert url == 'https://files.pythonhosted.org/packages/source/f/foo/foo-6.tar.gz' + urls = rpm_eval('%pypi_source', + name='python-foo', srcname='foo', version='6') + assert urls == ['https://files.pythonhosted.org/packages/source/f/foo/foo-6.tar.gz'] def test_pypi_source_default_pypi_name(): - url = rpm_eval('%pypi_source', - name='python-foo', pypi_name='foo', version='6')[0] - assert url == 'https://files.pythonhosted.org/packages/source/f/foo/foo-6.tar.gz' + urls = rpm_eval('%pypi_source', + name='python-foo', pypi_name='foo', version='6') + assert urls == ['https://files.pythonhosted.org/packages/source/f/foo/foo-6.tar.gz'] def test_pypi_source_default_name_uppercase(): - url = rpm_eval('%pypi_source', - name='Foo', version='6')[0] - assert url == 'https://files.pythonhosted.org/packages/source/F/Foo/Foo-6.tar.gz' + urls = rpm_eval('%pypi_source', + name='Foo', version='6') + assert urls == ['https://files.pythonhosted.org/packages/source/F/Foo/Foo-6.tar.gz'] def test_pypi_source_provided_name(): - url = rpm_eval('%pypi_source foo', - name='python-bar', pypi_name='bar', version='6')[0] - assert url == 'https://files.pythonhosted.org/packages/source/f/foo/foo-6.tar.gz' + urls = rpm_eval('%pypi_source foo', + name='python-bar', pypi_name='bar', version='6') + assert urls == ['https://files.pythonhosted.org/packages/source/f/foo/foo-6.tar.gz'] def test_pypi_source_provided_name_version(): - url = rpm_eval('%pypi_source foo 6', - name='python-bar', pypi_name='bar', version='3')[0] - assert url == 'https://files.pythonhosted.org/packages/source/f/foo/foo-6.tar.gz' + urls = rpm_eval('%pypi_source foo 6', + name='python-bar', pypi_name='bar', version='3') + assert urls == ['https://files.pythonhosted.org/packages/source/f/foo/foo-6.tar.gz'] def test_pypi_source_provided_name_version_ext(): url = rpm_eval('%pypi_source foo 6 zip', - name='python-bar', pypi_name='bar', version='3')[0] - assert url == 'https://files.pythonhosted.org/packages/source/f/foo/foo-6.zip' + name='python-bar', pypi_name='bar', version='3') + assert url == ['https://files.pythonhosted.org/packages/source/f/foo/foo-6.zip'] def test_pypi_source_prerelease(): - url = rpm_eval('%pypi_source', - name='python-foo', pypi_name='foo', version='6~b2')[0] - assert url == 'https://files.pythonhosted.org/packages/source/f/foo/foo-6b2.tar.gz' + urls = rpm_eval('%pypi_source', + name='python-foo', pypi_name='foo', version='6~b2') + assert urls == ['https://files.pythonhosted.org/packages/source/f/foo/foo-6b2.tar.gz'] def test_pypi_source_explicit_tilde(): - url = rpm_eval('%pypi_source foo 6~6', - name='python-foo', pypi_name='foo', version='6')[0] - assert url == 'https://files.pythonhosted.org/packages/source/f/foo/foo-6~6.tar.gz' + urls = rpm_eval('%pypi_source foo 6~6', + name='python-foo', pypi_name='foo', version='6') + assert urls == ['https://files.pythonhosted.org/packages/source/f/foo/foo-6~6.tar.gz'] def test_py3_shebang_fix(): @@ -323,9 +334,8 @@ def test_pycached_in_sitelib(): ] -def test_pycached_in_sitearch(): +def test_pycached_in_sitearch(lib): lines = rpm_eval('%pycached %{python3_sitearch}/foo*.py') - lib = rpm_eval('%_lib')[0] assert lines == [ f'/usr/{lib}/python{X_Y}/site-packages/foo*.py', f'/usr/{lib}/python{X_Y}/site-packages/__pycache__/foo*.cpython-{XY}{{,.opt-?}}.pyc' @@ -543,8 +553,8 @@ unversioned_macros = pytest.mark.parametrize('macro', [ @unversioned_macros def test_unversioned_python_errors(macro): lines = rpm_eval(macro, fails=True) - assert lines[0] == ('error: attempt to use unversioned python, ' - 'define %__python to /usr/bin/python2 or /usr/bin/python3 explicitly') + assert lines == ['error: attempt to use unversioned python, ' + 'define %__python to /usr/bin/python2 or /usr/bin/python3 explicitly'] @unversioned_macros @@ -559,9 +569,33 @@ x86_64_only = pytest.mark.skipif(platform.machine() != "x86_64", reason="works o @x86_64_only def test_platform_triplet(): - assert rpm_eval("%python3_platform_triplet")[0] == "x86_64-linux-gnu" + assert rpm_eval("%python3_platform_triplet") == ["x86_64-linux-gnu"] @x86_64_only def test_ext_suffix(): - assert rpm_eval("%python3_ext_suffix")[0] == f".cpython-{XY}-x86_64-linux-gnu.so" + assert rpm_eval("%python3_ext_suffix") == [f".cpython-{XY}-x86_64-linux-gnu.so"] + + +def test_python_sitelib_value(): + macro = '%python_sitelib' + assert rpm_eval(macro, __python='/usr/bin/python3.6') == [f'/usr/lib/python3.6/site-packages'] + assert rpm_eval(macro, __python='%__python3') == [f'/usr/lib/python{X_Y}/site-packages'] + + +def test_python3_sitelib_value(): + macro = '%python3_sitelib' + assert rpm_eval(macro, __python3='/usr/bin/python3.6') == [f'/usr/lib/python3.6/site-packages'] + assert rpm_eval(macro) == [f'/usr/lib/python{X_Y}/site-packages'] + + +def test_python_sitearch_value(lib): + macro = '%python_sitearch' + assert rpm_eval(macro, __python='/usr/bin/python3.6') == [f'/usr/{lib}/python3.6/site-packages'] + assert rpm_eval(macro, __python='%__python3') == [f'/usr/{lib}/python{X_Y}/site-packages'] + + +def test_python3_sitearch_value(lib): + macro = '%python3_sitearch' + assert rpm_eval(macro, __python3='/usr/bin/python3.6') == [f'/usr/{lib}/python3.6/site-packages'] + assert rpm_eval(macro) == [f'/usr/{lib}/python{X_Y}/site-packages'] diff --git a/tests/tests.yml b/tests/tests.yml index 83968d2..d6090ec 100644 --- a/tests/tests.yml +++ b/tests/tests.yml @@ -25,4 +25,5 @@ - python3-rpm-macros - python3-devel - python3-pytest + - python3.6 From fe81d653a1ff72df8b5440f9aaa360340e56bc6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miro=20Hron=C4=8Dok?= Date: Wed, 7 Jul 2021 14:46:04 +0200 Subject: [PATCH 3/9] Introduce %py3_check_import With $PATH and $PYTHONPATH set to the %buildroot, the macro tries to import the given Python 3 module(s). Useful as a smoke test in %check when ruining tests is not feasible. Accepts spaces or commas as separators. Package python-six: %check %py3_check_import six Executing(%check): ... ... + PATH=... + PYTHONPATH=... + PYTHONDONTWRITEBYTECODE=1 + /usr/bin/python3 -c 'import six' + RPM_EC=0 ++ jobs -p + exit 0 %py3_check_import six seven ... + /usr/bin/python3 -c 'import six, seven' Traceback (most recent call last): File "", line 1, in ModuleNotFoundError: No module named 'seven' error: Bad exit status from ... (%check) ... %py3_check_import five, six, seven + /usr/bin/python3 -c 'import five, six, seven' Traceback (most recent call last): File "", line 1, in ModuleNotFoundError: No module named 'five' error: Bad exit status from ... (%check) Package python-packaging: %py3_check_import packaging, packaging.markers packaging.requirements, packaging.tags Executing(%check): ... ... + PATH=... + PYTHONPATH=... + PYTHONDONTWRITEBYTECODE=1 + /usr/bin/python3 -c 'import packaging, packaging.markers, packaging.requirements, packaging.tags' + RPM_EC=0 ++ jobs -p + exit 0 %py3_check_import packaging, packaging.markers packaging.notachance, packaging.tags ... + /usr/bin/python3 -c 'import packaging, packaging.markers, packaging.notachance, packaging.tags' Traceback (most recent call last): File "", line 1, in ModuleNotFoundError: No module named 'packaging.notachance' error: Bad exit status from ... (%check) --- macros.python | 13 ++++++++++++ macros.python3 | 13 ++++++++++++ python-rpm-macros.spec | 5 ++++- tests/test_evals.py | 45 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+), 1 deletion(-) diff --git a/macros.python b/macros.python index fb0d061..4bf2c46 100644 --- a/macros.python +++ b/macros.python @@ -66,6 +66,19 @@ done } +# With $PATH and $PYTHONPATH set to the %%buildroot, +# try to import the given Python module(s). +# Useful as a smoke test in %%check when running tests is not feasible. +# Use spaces or commas as separators. +%py_check_import() %{expand:\\\ + (cd %{_topdir} &&\\\ + PATH="%{buildroot}%{_bindir}:$PATH"\\\ + PYTHONPATH="${PYTHONPATH:-%{buildroot}%{python_sitearch}:%{buildroot}%{python_sitelib}}"\\\ + PYTHONDONTWRITEBYTECODE=1\\\ + %{__python} -c "import %{lua:local m=rpm.expand('%{?*}'):gsub('[%s,]+', ', ');print(m)}" + ) +} + %python_provide() %{lua: local python = require "fedora.srpm.python" function string.starts(String,Start) diff --git a/macros.python3 b/macros.python3 index 38fd8e3..8f7202a 100644 --- a/macros.python3 +++ b/macros.python3 @@ -64,6 +64,19 @@ done } +# With $PATH and $PYTHONPATH set to the %%buildroot, +# try to import the given Python 3 module(s). +# Useful as a smoke test in %%check when running tests is not feasible. +# Use spaces or commas as separators. +%py3_check_import() %{expand:\\\ + (cd %{_topdir} &&\\\ + PATH="%{buildroot}%{_bindir}:$PATH"\\\ + PYTHONPATH="${PYTHONPATH:-%{buildroot}%{python3_sitearch}:%{buildroot}%{python3_sitelib}}"\\\ + PYTHONDONTWRITEBYTECODE=1\\\ + %{__python3} -c "import %{lua:local m=rpm.expand('%{?*}'):gsub('[%s,]+', ', ');print(m)}" + ) +} + # This only supports Python 3.5+ and will never work with Python 2. # Hence, it has no Python version in the name. %pycached() %{lua: diff --git a/python-rpm-macros.spec b/python-rpm-macros.spec index f75c68c..1c54152 100644 --- a/python-rpm-macros.spec +++ b/python-rpm-macros.spec @@ -22,7 +22,7 @@ License: MIT and Python # The macro is defined in python-srpm-macros. %{?load:%{SOURCE102}} Version: %{__default_python3_version} -Release: 37%{?dist} +Release: 38%{?dist} BuildArch: noarch @@ -95,6 +95,9 @@ install -m 644 compileall2.py %{buildroot}%{_rpmconfigdir}/redhat/ %changelog +* Wed Jul 07 2021 Miro Hrončok - 3.9-38 +- Introduce %%py3_check_import + * Mon Jun 28 2021 Miro Hrončok - 3.9-37 - %%pytest: Set $PYTEST_ADDOPTS when %%{__pytest_addopts} is defined - Related: rhzb#1935212 diff --git a/tests/test_evals.py b/tests/test_evals.py index 0392a4d..a54bb83 100644 --- a/tests/test_evals.py +++ b/tests/test_evals.py @@ -1,6 +1,7 @@ import os import subprocess import platform +import re import sys import textwrap @@ -547,6 +548,7 @@ unversioned_macros = pytest.mark.parametrize('macro', [ '%py_install', '%py_install_egg', '%py_install_wheel', + '%py_check_import', ]) @@ -599,3 +601,46 @@ def test_python3_sitearch_value(lib): macro = '%python3_sitearch' assert rpm_eval(macro, __python3='/usr/bin/python3.6') == [f'/usr/{lib}/python3.6/site-packages'] assert rpm_eval(macro) == [f'/usr/{lib}/python{X_Y}/site-packages'] + + +@pytest.mark.parametrize( + 'args, imports', + [ + ('six', 'six'), + ('five six seven', 'five, six, seven'), + ('six,seven, eight', 'six, seven, eight'), + ('six.quarter six.half,, SIX', 'six.quarter, six.half, SIX'), + ] +) +@pytest.mark.parametrize('__python3', [None, f'/usr/bin/python{X_Y}', '/usr/bin/python3.6']) +def test_py3_check_import(args, imports, __python3, lib): + x_y = X_Y + macors = { + 'buildroot': 'BUILDROOT', + '_topdir': 'TOPDIR', + } + if __python3 is not None: + macors['__python3'] = __python3 + # If the __python3 command has version at the end, parse it and expect it. + # Note that the command is used to determine %python3_sitelib and %python3_sitearch, + # so we only test known CPython schemes here and not PyPy for simplicity. + # We also only test main Python + 3.6 because those are required by the CI config. + if (match := re.match(r'.+python(\d+\.\d+)$', __python3)): + x_y = match.group(1) + + lines = rpm_eval(f'%py3_check_import {args}', **macors) + + # An equality check is a bit inflexible here, + # every time we change the macro we need to change this test. + # However actually executing it and verifying the result is much harder :/ + # At least, let's make the lines saner to check: + lines = [line.rstrip('\\').strip() for line in lines] + expected = textwrap.dedent(fr""" + (cd TOPDIR && + PATH="BUILDROOT/usr/bin:$PATH" + PYTHONPATH="${{PYTHONPATH:-BUILDROOT/usr/{lib}/python{x_y}/site-packages:BUILDROOT/usr/lib/python{x_y}/site-packages}}" + PYTHONDONTWRITEBYTECODE=1 + {__python3 or '/usr/bin/python3'} -c "import {imports}" + ) + """) + assert lines == expected.splitlines() From 4c2c37895d1dc3ba5d3080e5dd5c46e54d3d9d65 Mon Sep 17 00:00:00 2001 From: Tomas Orsava Date: Wed, 29 Sep 2021 12:47:19 +0200 Subject: [PATCH 4/9] Define a new macros %python_wheel_dir and %python_wheel_pkg_prefix --- macros.python-srpm | 14 ++++++++++++++ python-rpm-macros.spec | 5 ++++- tests/test_evals.py | 18 ++++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/macros.python-srpm b/macros.python-srpm index efe2bba..ecd08e7 100644 --- a/macros.python-srpm +++ b/macros.python-srpm @@ -52,6 +52,20 @@ %py3_other_build /bin/true %py3_other_install /bin/true +# Define where Python wheels will be stored and the prefix of -wheel packages +# - In Fedora we want wheel subpackages named e.g. `python-pip-wheel` that +# install packages into `/usr/share/python-wheels`. Both names are not +# versioned, because they're used by all Python 3 stacks. +# - In RHEL we want wheel packages named e.g. `python3-pip-wheel` and +# `python3.11-pip-wheel` that install packages into similarly versioned +# locations. We want each Python stack in RHEL to have their own wheels, +# because the main python3 wheels (which we can't upgrade) will likely be +# quite old by the time we're adding new alternate Python stacks. +# - In ELN we want to follow Fedora, because builds for ELN and Fedora rawhide +# need to be interoperable. +%python_wheel_pkg_prefix python%{?rhel:%{!?eln:%{python3_pkgversion}}} +%python_wheel_dir %{_datadir}/%{python_wheel_pkg_prefix}-wheels + # === Macros for Build/Requires tags using Python dist tags === diff --git a/python-rpm-macros.spec b/python-rpm-macros.spec index 1c54152..524c60c 100644 --- a/python-rpm-macros.spec +++ b/python-rpm-macros.spec @@ -22,7 +22,7 @@ License: MIT and Python # The macro is defined in python-srpm-macros. %{?load:%{SOURCE102}} Version: %{__default_python3_version} -Release: 38%{?dist} +Release: 39%{?dist} BuildArch: noarch @@ -95,6 +95,9 @@ install -m 644 compileall2.py %{buildroot}%{_rpmconfigdir}/redhat/ %changelog +* Wed Sep 29 2021 Tomas Orsava - 3.9-39 +- Define a new macros %%python_wheel_dir and %%python_wheel_pkg_prefix + * Wed Jul 07 2021 Miro Hrončok - 3.9-38 - Introduce %%py3_check_import diff --git a/tests/test_evals.py b/tests/test_evals.py index a54bb83..b63f4ad 100644 --- a/tests/test_evals.py +++ b/tests/test_evals.py @@ -194,6 +194,24 @@ def test_py_provides_with_evr(): assert len(lines) == 3 +def test_python_wheel_pkg_prefix(): + assert rpm_eval('%python_wheel_pkg_prefix', fedora='44', rhel=None, eln=None) == ['python'] + assert rpm_eval('%python_wheel_pkg_prefix', fedora='44', rhel=None, eln=None, python3_pkgversion='3.9') == ['python'] + assert rpm_eval('%python_wheel_pkg_prefix', fedora=None, rhel='1', eln='1') == ['python'] + assert rpm_eval('%python_wheel_pkg_prefix', fedora=None, rhel='1', eln=None) == ['python3'] + assert rpm_eval('%python_wheel_pkg_prefix', fedora=None, rhel='1', eln=None, python3_pkgversion='3.10') == ['python3.10'] + assert rpm_eval('%python_wheel_pkg_prefix', fedora=None, rhel='1', eln=None, python3_pkgversion='3.11') == ['python3.11'] + + +def test_python_wheel_dir(): + assert rpm_eval('%python_wheel_dir', fedora='44', rhel=None, eln=None) == ['/usr/share/python-wheels'] + assert rpm_eval('%python_wheel_dir', fedora='44', rhel=None, eln=None, python3_pkgversion='3.9') == ['/usr/share/python-wheels'] + assert rpm_eval('%python_wheel_dir', fedora=None, rhel='1', eln='1') == ['/usr/share/python-wheels'] + assert rpm_eval('%python_wheel_dir', fedora=None, rhel='1', eln=None) == ['/usr/share/python3-wheels'] + assert rpm_eval('%python_wheel_dir', fedora=None, rhel='1', eln=None, python3_pkgversion='3.10') == ['/usr/share/python3.10-wheels'] + assert rpm_eval('%python_wheel_dir', fedora=None, rhel='1', eln=None, python3_pkgversion='3.11') == ['/usr/share/python3.11-wheels'] + + def test_pytest_passes_options_naturally(): lines = rpm_eval('%pytest -k foo') assert '/usr/bin/pytest -k foo' in lines[-1] From 51d0619929a7300b7332b1c1294cf2168c1dc6c8 Mon Sep 17 00:00:00 2001 From: Karolina Surma Date: Mon, 18 Oct 2021 16:33:04 +0200 Subject: [PATCH 5/9] Add new options for %%py{3}_check_import: -f, -t, -e -f: optionally read a file with module names to test -t: bool flag - if set, filter only top-level modules -e: optionally exclude module names matching the given glob (Unix shell-style wildcards) Importing all modules may cause bogus failures in some cases, eg. when the imported code assumes there is an existing graphical window. Such behaviour may be by design, hence for automatic processing it's more convinient to - in some cases - check only for top-level modules or filter out the troublemakers. --- import_all_modules.py | 152 ++++++++++++ macros.python | 13 +- macros.python3 | 13 +- python-rpm-macros.spec | 12 +- tests/test_evals.py | 30 +-- tests/test_import_all_modules.py | 392 +++++++++++++++++++++++++++++++ tests/tests.yml | 2 +- 7 files changed, 585 insertions(+), 29 deletions(-) create mode 100644 import_all_modules.py create mode 100644 tests/test_import_all_modules.py diff --git a/import_all_modules.py b/import_all_modules.py new file mode 100644 index 0000000..7536133 --- /dev/null +++ b/import_all_modules.py @@ -0,0 +1,152 @@ +'''Script to perform import of each module given to %%py_check_import +''' +import argparse +import importlib +import fnmatch +import re +import sys + +from contextlib import contextmanager +from pathlib import Path + + +def read_modules_files(file_paths): + '''Read module names from the files (modules must be newline separated). + + Return the module names list or, if no files were provided, an empty list. + ''' + + if not file_paths: + return [] + + modules = [] + for file in file_paths: + file_contents = file.read_text() + modules.extend(file_contents.split()) + return modules + + +def read_modules_from_cli(argv): + '''Read module names from command-line arguments (space or comma separated). + + Return the module names list. + ''' + + if not argv: + return [] + + # %%py3_check_import allows to separate module list with comma or whitespace, + # we need to unify the output to a list of particular elements + modules_as_str = ' '.join(argv) + modules = re.split(r'[\s,]+', modules_as_str) + return modules + + +def filter_top_level_modules_only(modules): + '''Filter out entries with nested modules (containing dot) ie. 'foo.bar'. + + Return the list of top-level modules. + ''' + + return [module for module in modules if '.' not in module] + + +def any_match(text, globs): + '''Return True if any of given globs fnmatchcase's the given text.''' + + return any(fnmatch.fnmatchcase(text, g) for g in globs) + + +def exclude_unwanted_module_globs(globs, modules): + '''Filter out entries which match the either of the globs given as argv. + + Return the list of filtered modules. + ''' + + return [m for m in modules if not any_match(m, globs)] + + +def read_modules_from_all_args(args): + '''Return a joined list of modules from all given command-line arguments. + ''' + + modules = read_modules_files(args.filename) + modules.extend(read_modules_from_cli(args.modules)) + if args.exclude: + modules = exclude_unwanted_module_globs(args.exclude, modules) + + if args.top_level: + modules = filter_top_level_modules_only(modules) + + # Error when someone accidentally managed to filter out everything + if len(modules) == 0: + raise ValueError('No modules to check were left') + + return modules + + +def import_modules(modules): + '''Procedure to perform import check for each module name from the given list of modules. + ''' + + for module in modules: + print('Check import:', module, file=sys.stderr) + importlib.import_module(module) + + +def argparser(): + parser = argparse.ArgumentParser( + description='Generate list of all importable modules for import check.' + ) + parser.add_argument( + 'modules', nargs='*', + help=('Add modules to check the import (space or comma separated).'), + ) + parser.add_argument( + '-f', '--filename', action='append', type=Path, + help='Add importable module names list from file.', + ) + parser.add_argument( + '-t', '--top-level', action='store_true', + help='Check only top-level modules.', + ) + parser.add_argument( + '-e', '--exclude', action='append', + help='Provide modules globs to be excluded from the check.', + ) + return parser + + +@contextmanager +def remove_unwanteds_from_sys_path(): + '''Remove cwd and this script's parent from sys.path for the import test. + Bring the original contents back after import is done (or failed) + ''' + + cwd_absolute = Path.cwd().absolute() + this_file_parent = Path(__file__).parent.absolute() + old_sys_path = list(sys.path) + for path in old_sys_path: + if Path(path).absolute() in (cwd_absolute, this_file_parent): + sys.path.remove(path) + try: + yield + finally: + sys.path = old_sys_path + + +def main(argv=None): + + cli_args = argparser().parse_args(argv) + + if not cli_args.modules and not cli_args.filename: + raise ValueError('No modules to check were provided') + + modules = read_modules_from_all_args(cli_args) + + with remove_unwanteds_from_sys_path(): + import_modules(modules) + + +if __name__ == '__main__': + main() diff --git a/macros.python b/macros.python index 4bf2c46..f0821d3 100644 --- a/macros.python +++ b/macros.python @@ -67,16 +67,17 @@ } # With $PATH and $PYTHONPATH set to the %%buildroot, -# try to import the given Python module(s). +# try to import the Python module(s) given as command-line args or read from file (-f). +# Filter and check import on only top-level modules using -t flag. +# Exclude unwanted modules by passing their globs to -e option. # Useful as a smoke test in %%check when running tests is not feasible. -# Use spaces or commas as separators. -%py_check_import() %{expand:\\\ - (cd %{_topdir} &&\\\ +# Use spaces or commas as separators if providing list directly. +# Use newlines as separators if providing list in a file. +%py_check_import(e:tf:) %{expand:\\\ PATH="%{buildroot}%{_bindir}:$PATH"\\\ PYTHONPATH="${PYTHONPATH:-%{buildroot}%{python_sitearch}:%{buildroot}%{python_sitelib}}"\\\ PYTHONDONTWRITEBYTECODE=1\\\ - %{__python} -c "import %{lua:local m=rpm.expand('%{?*}'):gsub('[%s,]+', ', ');print(m)}" - ) + %{__python} -%{py_shebang_flags} %{_rpmconfigdir}/redhat/import_all_modules.py %{?**} } %python_provide() %{lua: diff --git a/macros.python3 b/macros.python3 index 8f7202a..ddc9d4b 100644 --- a/macros.python3 +++ b/macros.python3 @@ -65,16 +65,17 @@ } # With $PATH and $PYTHONPATH set to the %%buildroot, -# try to import the given Python 3 module(s). +# try to import the Python 3 module(s) given as command-line args or read from file (-f). +# Filter and check import on only top-level modules using -t flag. +# Exclude unwanted modules by passing their globs to -e option. # Useful as a smoke test in %%check when running tests is not feasible. -# Use spaces or commas as separators. -%py3_check_import() %{expand:\\\ - (cd %{_topdir} &&\\\ +# Use spaces or commas as separators if providing list directly. +# Use newlines as separators if providing list in a file. +%py3_check_import(e:tf:) %{expand:\\\ PATH="%{buildroot}%{_bindir}:$PATH"\\\ PYTHONPATH="${PYTHONPATH:-%{buildroot}%{python3_sitearch}:%{buildroot}%{python3_sitelib}}"\\\ PYTHONDONTWRITEBYTECODE=1\\\ - %{__python3} -c "import %{lua:local m=rpm.expand('%{?*}'):gsub('[%s,]+', ', ');print(m)}" - ) + %{__python3} -%{py3_shebang_flags} %{_rpmconfigdir}/redhat/import_all_modules.py %{?**} } # This only supports Python 3.5+ and will never work with Python 2. diff --git a/python-rpm-macros.spec b/python-rpm-macros.spec index 524c60c..a204864 100644 --- a/python-rpm-macros.spec +++ b/python-rpm-macros.spec @@ -13,8 +13,9 @@ Source201: python.lua # Python code %global compileall2_version 0.7.1 Source301: https://github.com/fedora-python/compileall2/raw/v%{compileall2_version}/compileall2.py +Source302: import_all_modules.py -# macros and lua: MIT, compileall2.py: PSFv2 +# macros and lua: MIT, compileall2.py: PSFv2, import_all_modules.py: MIT License: MIT and Python # The package version MUST be always the same as %%{__default_python3_version}. @@ -22,7 +23,7 @@ License: MIT and Python # The macro is defined in python-srpm-macros. %{?load:%{SOURCE102}} Version: %{__default_python3_version} -Release: 39%{?dist} +Release: 40%{?dist} BuildArch: noarch @@ -79,6 +80,7 @@ install -p -m 644 -t %{buildroot}%{_rpmluadir}/fedora/srpm python.lua mkdir -p %{buildroot}%{_rpmconfigdir}/redhat install -m 644 compileall2.py %{buildroot}%{_rpmconfigdir}/redhat/ +install -m 644 import_all_modules.py %{buildroot}%{_rpmconfigdir}/redhat/ %files @@ -88,6 +90,7 @@ install -m 644 compileall2.py %{buildroot}%{_rpmconfigdir}/redhat/ %files -n python-srpm-macros %{rpmmacrodir}/macros.python-srpm %{_rpmconfigdir}/redhat/compileall2.py +%{_rpmconfigdir}/redhat/import_all_modules.py %{_rpmluadir}/fedora/srpm/python.lua %files -n python3-rpm-macros @@ -95,6 +98,11 @@ install -m 644 compileall2.py %{buildroot}%{_rpmconfigdir}/redhat/ %changelog +* Mon Oct 25 2021 Karolina Surma - 3.9-40 +- Introduce -f (read from file) option to %%py{3}_check_import +- Introduce -t (filter top-level modules) option to %%py{3}_check_import +- Introduce -e (exclude module globs) option to %%py{3}_check_import + * Wed Sep 29 2021 Tomas Orsava - 3.9-39 - Define a new macros %%python_wheel_dir and %%python_wheel_pkg_prefix diff --git a/tests/test_evals.py b/tests/test_evals.py index b63f4ad..0174449 100644 --- a/tests/test_evals.py +++ b/tests/test_evals.py @@ -622,23 +622,27 @@ def test_python3_sitearch_value(lib): @pytest.mark.parametrize( - 'args, imports', + 'args', [ - ('six', 'six'), - ('five six seven', 'five, six, seven'), - ('six,seven, eight', 'six, seven, eight'), - ('six.quarter six.half,, SIX', 'six.quarter, six.half, SIX'), + 'six', + '-f foo.txt', + '-t -f foo.txt six, seven', + '-e "foo*" -f foo.txt six, seven', + 'six.quarter six.half,, SIX', ] ) -@pytest.mark.parametrize('__python3', [None, f'/usr/bin/python{X_Y}', '/usr/bin/python3.6']) -def test_py3_check_import(args, imports, __python3, lib): +@pytest.mark.parametrize('__python3', + [None, + f'/usr/bin/python{X_Y}', + '/usr/bin/python3.6']) +def test_py3_check_import(args, __python3, lib): x_y = X_Y - macors = { + macros = { 'buildroot': 'BUILDROOT', - '_topdir': 'TOPDIR', + '_rpmconfigdir': 'RPMCONFIGDIR', } if __python3 is not None: - macors['__python3'] = __python3 + macros['__python3'] = __python3 # If the __python3 command has version at the end, parse it and expect it. # Note that the command is used to determine %python3_sitelib and %python3_sitearch, # so we only test known CPython schemes here and not PyPy for simplicity. @@ -646,7 +650,7 @@ def test_py3_check_import(args, imports, __python3, lib): if (match := re.match(r'.+python(\d+\.\d+)$', __python3)): x_y = match.group(1) - lines = rpm_eval(f'%py3_check_import {args}', **macors) + lines = rpm_eval(f'%py3_check_import {args}', **macros) # An equality check is a bit inflexible here, # every time we change the macro we need to change this test. @@ -654,11 +658,9 @@ def test_py3_check_import(args, imports, __python3, lib): # At least, let's make the lines saner to check: lines = [line.rstrip('\\').strip() for line in lines] expected = textwrap.dedent(fr""" - (cd TOPDIR && PATH="BUILDROOT/usr/bin:$PATH" PYTHONPATH="${{PYTHONPATH:-BUILDROOT/usr/{lib}/python{x_y}/site-packages:BUILDROOT/usr/lib/python{x_y}/site-packages}}" PYTHONDONTWRITEBYTECODE=1 - {__python3 or '/usr/bin/python3'} -c "import {imports}" - ) + {__python3 or '/usr/bin/python3'} -s RPMCONFIGDIR/redhat/import_all_modules.py {args} """) assert lines == expected.splitlines() diff --git a/tests/test_import_all_modules.py b/tests/test_import_all_modules.py new file mode 100644 index 0000000..490ef1d --- /dev/null +++ b/tests/test_import_all_modules.py @@ -0,0 +1,392 @@ +from import_all_modules import argparser, exclude_unwanted_module_globs +from import_all_modules import main as modules_main +from import_all_modules import read_modules_from_cli, filter_top_level_modules_only + +from pathlib import Path + +import pytest +import shlex +import sys + + +@pytest.fixture(autouse=True) +def preserve_sys_path(): + original_sys_path = list(sys.path) + yield + sys.path = original_sys_path + + +@pytest.fixture(autouse=True) +def preserve_sys_modules(): + original_sys_modules = dict(sys.modules) + yield + sys.modules = original_sys_modules + + +@pytest.mark.parametrize( + 'args, imports', + [ + ('six', ['six']), + ('five six seven', ['five', 'six', 'seven']), + ('six,seven, eight', ['six', 'seven', 'eight']), + ('six.quarter six.half,, SIX', ['six.quarter', 'six.half', 'SIX']), + ] +) +def test_read_modules_from_cli(args, imports): + argv = shlex.split(args) + cli_args = argparser().parse_args(argv) + assert read_modules_from_cli(cli_args.modules) == imports + + +@pytest.mark.parametrize( + 'all_mods, imports', + [ + (['six'], ['six']), + (['five', 'six', 'seven'], ['five', 'six', 'seven']), + (['six.seven', 'eight'], ['eight']), + (['SIX', 'six.quarter', 'six.half.and.sth', 'seven'], ['SIX', 'seven']), + ], +) +def test_filter_top_level_modules_only(all_mods, imports): + assert filter_top_level_modules_only(all_mods) == imports + + +@pytest.mark.parametrize( + 'globs, expected', + [ + (['*.*'], ['foo', 'boo']), + (['?oo'], ['foo.bar', 'foo.bar.baz', 'foo.baz']), + (['*.baz'], ['foo', 'foo.bar', 'boo']), + (['foo'], ['foo.bar', 'foo.bar.baz', 'foo.baz', 'boo']), + (['foo*'], ['boo']), + (['foo*', '*bar'], ['boo']), + (['foo', 'bar'], ['foo.bar', 'foo.bar.baz', 'foo.baz', 'boo']), + (['*'], []), + ] +) +def test_exclude_unwanted_module_globs(globs, expected): + my_modules = ['foo', 'foo.bar', 'foo.bar.baz', 'foo.baz', 'boo'] + tested = exclude_unwanted_module_globs(globs, my_modules) + assert tested == expected + + +def test_cli_with_all_args(): + '''A smoke test, all args must be parsed correctly.''' + mods = ['foo', 'foo.bar', 'baz'] + files = ['-f', './foo'] + top = ['-t'] + exclude = ['-e', 'foo*'] + cli_args = argparser().parse_args([*mods, *files, *top, *exclude]) + + assert cli_args.filename == [Path('foo')] + assert cli_args.top_level is True + assert cli_args.modules == ['foo', 'foo.bar', 'baz'] + assert cli_args.exclude == ['foo*'] + + +def test_cli_without_filename_toplevel(): + '''Modules provided on command line (without files) must be parsed correctly.''' + mods = ['foo', 'foo.bar', 'baz'] + cli_args = argparser().parse_args(mods) + + assert cli_args.filename is None + assert cli_args.top_level is False + assert cli_args.modules == ['foo', 'foo.bar', 'baz'] + + +def test_cli_with_filename_no_cli_mods(): + '''Files (without any modules provided on command line) must be parsed correctly.''' + + files = ['-f', './foo', '-f', './bar', '-f', './baz'] + cli_args = argparser().parse_args(files) + + assert cli_args.filename == [Path('foo'), Path('./bar'), Path('./baz')] + assert not cli_args.top_level + + +def test_main_raises_error_when_no_modules_provided(): + '''If no filename nor modules were provided, ValueError is raised.''' + + with pytest.raises(ValueError): + modules_main([]) + + +def test_import_all_modules_does_not_import(): + '''Ensure the files from /usr/lib/rpm/redhat cannot be imported and + checked for import''' + + # We already imported it in this file once, make sure it's not imported + # from the cache + sys.modules.pop('import_all_modules') + with pytest.raises(ModuleNotFoundError): + modules_main(['import_all_modules']) + + +def test_modules_from_cwd_not_found(tmp_path, monkeypatch): + test_module = tmp_path / 'this_is_a_module_in_cwd.py' + test_module.write_text('') + monkeypatch.chdir(tmp_path) + with pytest.raises(ModuleNotFoundError): + modules_main(['this_is_a_module_in_cwd']) + + +def test_modules_from_sys_path_found(tmp_path): + test_module = tmp_path / 'this_is_a_module_in_sys_path.py' + test_module.write_text('') + sys.path.append(str(tmp_path)) + modules_main(['this_is_a_module_in_sys_path']) + assert 'this_is_a_module_in_sys_path' in sys.modules + + +def test_modules_from_file_are_found(tmp_path): + test_file = tmp_path / 'this_is_a_file_in_tmp_path.txt' + test_file.write_text('math\nwave\nsunau\n') + + # Make sure the tested modules are not already in sys.modules + for m in ('math', 'wave', 'sunau'): + sys.modules.pop(m, None) + + modules_main(['-f', str(test_file)]) + + assert 'sunau' in sys.modules + assert 'math' in sys.modules + assert 'wave' in sys.modules + + +def test_modules_from_files_are_found(tmp_path): + test_file_1 = tmp_path / 'this_is_a_file_in_tmp_path_1.txt' + test_file_2 = tmp_path / 'this_is_a_file_in_tmp_path_2.txt' + test_file_3 = tmp_path / 'this_is_a_file_in_tmp_path_3.txt' + + test_file_1.write_text('math\nwave\n') + test_file_2.write_text('sunau\npathlib\n') + test_file_3.write_text('logging\nsunau\n') + + # Make sure the tested modules are not already in sys.modules + for m in ('math', 'wave', 'sunau', 'pathlib', 'logging'): + sys.modules.pop(m, None) + + modules_main(['-f', str(test_file_1), '-f', str(test_file_2), '-f', str(test_file_3), ]) + for module in ('sunau', 'math', 'wave', 'pathlib', 'logging'): + assert module in sys.modules + + +def test_nonexisting_modules_raise_exception_on_import(tmp_path): + test_file = tmp_path / 'this_is_a_file_in_tmp_path.txt' + test_file.write_text('nonexisting_module\nanother\n') + with pytest.raises(ModuleNotFoundError): + modules_main(['-f', str(test_file)]) + + +def test_nested_modules_found_when_expected(tmp_path, monkeypatch, capsys): + + # This one is supposed to raise an error + cwd_path = tmp_path / 'test_cwd' + Path.mkdir(cwd_path) + test_module_1 = cwd_path / 'this_is_a_module_in_cwd.py' + + # Nested structure that is supposed to be importable + nested_path_1 = tmp_path / 'nested' + nested_path_2 = nested_path_1 / 'more_nested' + + for path in (nested_path_1, nested_path_2): + Path.mkdir(path) + + test_module_2 = tmp_path / 'this_is_a_module_in_level_0.py' + test_module_3 = nested_path_1 / 'this_is_a_module_in_level_1.py' + test_module_4 = nested_path_2 / 'this_is_a_module_in_level_2.py' + + for module in (test_module_1, test_module_2, test_module_3, test_module_4): + module.write_text('') + + sys.path.append(str(tmp_path)) + monkeypatch.chdir(cwd_path) + + with pytest.raises(ModuleNotFoundError): + modules_main([ + 'this_is_a_module_in_level_0', + 'nested.this_is_a_module_in_level_1', + 'nested.more_nested.this_is_a_module_in_level_2', + 'this_is_a_module_in_cwd']) + + _, err = capsys.readouterr() + assert 'Check import: this_is_a_module_in_level_0' in err + assert 'Check import: nested.this_is_a_module_in_level_1' in err + assert 'Check import: nested.more_nested.this_is_a_module_in_level_2' in err + assert 'Check import: this_is_a_module_in_cwd' in err + + +def test_modules_both_from_files_and_cli_are_imported(tmp_path): + test_file_1 = tmp_path / 'this_is_a_file_in_tmp_path_1.txt' + test_file_1.write_text('this_is_a_module_in_tmp_path_1') + + test_file_2 = tmp_path / 'this_is_a_file_in_tmp_path_2.txt' + test_file_2.write_text('this_is_a_module_in_tmp_path_2') + + test_module_1 = tmp_path / 'this_is_a_module_in_tmp_path_1.py' + test_module_2 = tmp_path / 'this_is_a_module_in_tmp_path_2.py' + test_module_3 = tmp_path / 'this_is_a_module_in_tmp_path_3.py' + + for module in (test_module_1, test_module_2, test_module_3): + module.write_text('') + + sys.path.append(str(tmp_path)) + modules_main([ + '-f', str(test_file_1), + 'this_is_a_module_in_tmp_path_3', + '-f', str(test_file_2), + ]) + + expected = ( + 'this_is_a_module_in_tmp_path_1', + 'this_is_a_module_in_tmp_path_2', + 'this_is_a_module_in_tmp_path_3', + ) + for module in expected: + assert module in sys.modules + + +def test_non_existing_module_raises_exception(tmp_path): + + test_module_1 = tmp_path / 'this_is_a_module_in_tmp_path_1.py' + test_module_1.write_text('') + sys.path.append(str(tmp_path)) + + with pytest.raises(ModuleNotFoundError): + modules_main([ + 'this_is_a_module_in_tmp_path_1', + 'this_is_a_module_in_tmp_path_2', + ]) + + +def test_module_with_error_propagates_exception(tmp_path): + + test_module_1 = tmp_path / 'this_is_a_module_in_tmp_path_1.py' + test_module_1.write_text('0/0') + sys.path.append(str(tmp_path)) + + # The correct exception must be raised + with pytest.raises(ZeroDivisionError): + modules_main([ + 'this_is_a_module_in_tmp_path_1', + ]) + + +def test_correct_modules_are_excluded(tmp_path): + test_module_1 = tmp_path / 'module_in_tmp_path_1.py' + test_module_2 = tmp_path / 'module_in_tmp_path_2.py' + test_module_3 = tmp_path / 'module_in_tmp_path_3.py' + + for module in (test_module_1, test_module_2, test_module_3): + module.write_text('') + + sys.path.append(str(tmp_path)) + test_file_1 = tmp_path / 'a_file_in_tmp_path_1.txt' + test_file_1.write_text('module_in_tmp_path_1\nmodule_in_tmp_path_2\nmodule_in_tmp_path_3\n') + + modules_main([ + '-e', 'module_in_tmp_path_2', + '-f', str(test_file_1), + '-e', 'module_in_tmp_path_3', + ]) + + assert 'module_in_tmp_path_1' in sys.modules + assert 'module_in_tmp_path_2' not in sys.modules + assert 'module_in_tmp_path_3' not in sys.modules + + +def test_excluding_all_modules_raises_error(tmp_path): + test_module_1 = tmp_path / 'module_in_tmp_path_1.py' + test_module_2 = tmp_path / 'module_in_tmp_path_2.py' + test_module_3 = tmp_path / 'module_in_tmp_path_3.py' + + for module in (test_module_1, test_module_2, test_module_3): + module.write_text('') + + sys.path.append(str(tmp_path)) + test_file_1 = tmp_path / 'a_file_in_tmp_path_1.txt' + test_file_1.write_text('module_in_tmp_path_1\nmodule_in_tmp_path_2\nmodule_in_tmp_path_3\n') + + with pytest.raises(ValueError): + modules_main([ + '-e', 'module_in_tmp_path*', + '-f', str(test_file_1), + ]) + + +def test_only_toplevel_modules_found(tmp_path): + + # Nested structure that is supposed to be importable + nested_path_1 = tmp_path / 'nested' + nested_path_2 = nested_path_1 / 'more_nested' + + for path in (nested_path_1, nested_path_2): + Path.mkdir(path) + + test_module_1 = tmp_path / 'this_is_a_module_in_level_0.py' + test_module_2 = nested_path_1 / 'this_is_a_module_in_level_1.py' + test_module_3 = nested_path_2 / 'this_is_a_module_in_level_2.py' + + for module in (test_module_1, test_module_2, test_module_3): + module.write_text('') + + sys.path.append(str(tmp_path)) + + modules_main([ + 'this_is_a_module_in_level_0', + 'nested.this_is_a_module_in_level_1', + 'nested.more_nested.this_is_a_module_in_level_2', + '-t']) + + assert 'nested.this_is_a_module_in_level_1' not in sys.modules + assert 'nested.more_nested.this_is_a_module_in_level_2' not in sys.modules + + +def test_only_toplevel_included_modules_found(tmp_path): + + # Nested structure that is supposed to be importable + nested_path_1 = tmp_path / 'nested' + nested_path_2 = nested_path_1 / 'more_nested' + + for path in (nested_path_1, nested_path_2): + Path.mkdir(path) + + test_module_1 = tmp_path / 'this_is_a_module_in_level_0.py' + test_module_4 = tmp_path / 'this_is_another_module_in_level_0.py' + + test_module_2 = nested_path_1 / 'this_is_a_module_in_level_1.py' + test_module_3 = nested_path_2 / 'this_is_a_module_in_level_2.py' + + for module in (test_module_1, test_module_2, test_module_3, test_module_4): + module.write_text('') + + sys.path.append(str(tmp_path)) + + modules_main([ + 'this_is_a_module_in_level_0', + 'this_is_another_module_in_level_0', + 'nested.this_is_a_module_in_level_1', + 'nested.more_nested.this_is_a_module_in_level_2', + '-t', + '-e', '*another*' + ]) + + assert 'nested.this_is_a_module_in_level_1' not in sys.modules + assert 'nested.more_nested.this_is_a_module_in_level_2' not in sys.modules + assert 'this_is_another_module_in_level_0' not in sys.modules + assert 'this_is_a_module_in_level_0' in sys.modules + + +def test_module_list_from_relative_path(tmp_path, monkeypatch): + + monkeypatch.chdir(tmp_path) + test_file_1 = Path('this_is_a_file_in_cwd.txt') + test_file_1.write_text('wave') + + sys.modules.pop('wave', None) + + modules_main([ + '-f', 'this_is_a_file_in_cwd.txt' + ]) + + assert 'wave' in sys.modules diff --git a/tests/tests.yml b/tests/tests.yml index d6090ec..28a9bac 100644 --- a/tests/tests.yml +++ b/tests/tests.yml @@ -15,7 +15,7 @@ tests: - pytest: dir: . - run: pytest -v + run: PYTHONPATH=/usr/lib/rpm/redhat pytest -v - manual_byte_compilation: dir: . run: rpmbuild -ba pythontest.spec From aa7cf4e2bd6f67aa9a6773dddb16322801afef6f Mon Sep 17 00:00:00 2001 From: Karolina Surma Date: Mon, 1 Nov 2021 09:42:39 +0100 Subject: [PATCH 6/9] Allow multiline arguments processing for %%py3_check_import Fixes the regression introduced with the macro reimplementation. Resolves: rhbz#2018809 --- import_all_modules.py | 4 ++++ macros.python | 7 +++++-- macros.python3 | 7 +++++-- python-rpm-macros.spec | 6 +++++- tests/test_evals.py | 22 +++++++++++++--------- tests/test_import_all_modules.py | 1 + 6 files changed, 33 insertions(+), 14 deletions(-) diff --git a/import_all_modules.py b/import_all_modules.py index 7536133..5788b8c 100644 --- a/import_all_modules.py +++ b/import_all_modules.py @@ -39,6 +39,10 @@ def read_modules_from_cli(argv): # we need to unify the output to a list of particular elements modules_as_str = ' '.join(argv) modules = re.split(r'[\s,]+', modules_as_str) + # Because of shell expansion in some less typical cases it may happen + # that a trailing space will occur at the end of the list. + # Remove the empty items from the list before passing it further + modules = [m for m in modules if m] return modules diff --git a/macros.python b/macros.python index f0821d3..9f05dae 100644 --- a/macros.python +++ b/macros.python @@ -77,8 +77,11 @@ PATH="%{buildroot}%{_bindir}:$PATH"\\\ PYTHONPATH="${PYTHONPATH:-%{buildroot}%{python_sitearch}:%{buildroot}%{python_sitelib}}"\\\ PYTHONDONTWRITEBYTECODE=1\\\ - %{__python} -%{py_shebang_flags} %{_rpmconfigdir}/redhat/import_all_modules.py %{?**} -} + %{__python} -%{py_shebang_flags} %{_rpmconfigdir}/redhat/import_all_modules.py\\\ + %{lua: + -- handle multiline arguments correctly, see https://bugzilla.redhat.com/2018809 + local args=rpm.expand('%{?**}'):gsub("[%s\\\\]*%s+", " ");print(args) + }} %python_provide() %{lua: local python = require "fedora.srpm.python" diff --git a/macros.python3 b/macros.python3 index ddc9d4b..a4a990e 100644 --- a/macros.python3 +++ b/macros.python3 @@ -75,8 +75,11 @@ PATH="%{buildroot}%{_bindir}:$PATH"\\\ PYTHONPATH="${PYTHONPATH:-%{buildroot}%{python3_sitearch}:%{buildroot}%{python3_sitelib}}"\\\ PYTHONDONTWRITEBYTECODE=1\\\ - %{__python3} -%{py3_shebang_flags} %{_rpmconfigdir}/redhat/import_all_modules.py %{?**} -} + %{__python3} -%{py3_shebang_flags} %{_rpmconfigdir}/redhat/import_all_modules.py\\\ + %{lua: + -- handle multiline arguments correctly, see https://bugzilla.redhat.com/2018809 + local args=rpm.expand('%{?**}'):gsub("[%s\\\\]*%s+", " ");print(args) + }} # This only supports Python 3.5+ and will never work with Python 2. # Hence, it has no Python version in the name. diff --git a/python-rpm-macros.spec b/python-rpm-macros.spec index a204864..21f4f4e 100644 --- a/python-rpm-macros.spec +++ b/python-rpm-macros.spec @@ -23,7 +23,7 @@ License: MIT and Python # The macro is defined in python-srpm-macros. %{?load:%{SOURCE102}} Version: %{__default_python3_version} -Release: 40%{?dist} +Release: 41%{?dist} BuildArch: noarch @@ -98,6 +98,10 @@ install -m 644 import_all_modules.py %{buildroot}%{_rpmconfigdir}/redhat/ %changelog +* Mon Nov 01 2021 Karolina Surma - 3.9-41 +- Fix multiline arguments processing for %%py_check_import +Resolves: rhbz#2018809 + * Mon Oct 25 2021 Karolina Surma - 3.9-40 - Introduce -f (read from file) option to %%py{3}_check_import - Introduce -t (filter top-level modules) option to %%py{3}_check_import diff --git a/tests/test_evals.py b/tests/test_evals.py index 0174449..07e06e4 100644 --- a/tests/test_evals.py +++ b/tests/test_evals.py @@ -622,20 +622,22 @@ def test_python3_sitearch_value(lib): @pytest.mark.parametrize( - 'args', + 'args, expected_args', [ - 'six', - '-f foo.txt', - '-t -f foo.txt six, seven', - '-e "foo*" -f foo.txt six, seven', - 'six.quarter six.half,, SIX', + ('six', 'six'), + ('-f foo.txt', '-f foo.txt'), + ('-t -f foo.txt six, seven', '-t -f foo.txt six, seven'), + ('-e "foo*" -f foo.txt six, seven', '-e "foo*" -f foo.txt six, seven'), + ('six.quarter six.half,, SIX', 'six.quarter six.half,, SIX'), + ('-f foo.txt six\nsix.half\nSIX', '-f foo.txt six six.half SIX'), + ('six \\ -e six.half', 'six -e six.half'), ] ) @pytest.mark.parametrize('__python3', [None, f'/usr/bin/python{X_Y}', '/usr/bin/python3.6']) -def test_py3_check_import(args, __python3, lib): +def test_py3_check_import(args, expected_args, __python3, lib): x_y = X_Y macros = { 'buildroot': 'BUILDROOT', @@ -650,7 +652,8 @@ def test_py3_check_import(args, __python3, lib): if (match := re.match(r'.+python(\d+\.\d+)$', __python3)): x_y = match.group(1) - lines = rpm_eval(f'%py3_check_import {args}', **macros) + invocation = '%{py3_check_import ' + args +'}' + lines = rpm_eval(invocation, **macros) # An equality check is a bit inflexible here, # every time we change the macro we need to change this test. @@ -661,6 +664,7 @@ def test_py3_check_import(args, __python3, lib): PATH="BUILDROOT/usr/bin:$PATH" PYTHONPATH="${{PYTHONPATH:-BUILDROOT/usr/{lib}/python{x_y}/site-packages:BUILDROOT/usr/lib/python{x_y}/site-packages}}" PYTHONDONTWRITEBYTECODE=1 - {__python3 or '/usr/bin/python3'} -s RPMCONFIGDIR/redhat/import_all_modules.py {args} + {__python3 or '/usr/bin/python3'} -s RPMCONFIGDIR/redhat/import_all_modules.py + {expected_args} """) assert lines == expected.splitlines() diff --git a/tests/test_import_all_modules.py b/tests/test_import_all_modules.py index 490ef1d..d60f6e4 100644 --- a/tests/test_import_all_modules.py +++ b/tests/test_import_all_modules.py @@ -30,6 +30,7 @@ def preserve_sys_modules(): ('five six seven', ['five', 'six', 'seven']), ('six,seven, eight', ['six', 'seven', 'eight']), ('six.quarter six.half,, SIX', ['six.quarter', 'six.half', 'SIX']), + ('six.quarter six.half,, SIX \\ ', ['six.quarter', 'six.half', 'SIX']), ] ) def test_read_modules_from_cli(args, imports): From 4a65e2cfa69c47080448285448ebc67adacd3d2b Mon Sep 17 00:00:00 2001 From: Karolina Surma Date: Mon, 1 Nov 2021 13:17:42 +0100 Subject: [PATCH 7/9] Fix %%py_shebang_flags handling within %%py_check_import %%py{3}_check_import now respects the custom setting of %%py{3}_shebang_flags and invokes Python with the respective values. If %%py{3}_shebang_flags is undefined or set to no value, there no flags are passed to Python on invoke. Resolves: rhbz#2018615 --- macros.python | 13 ++++++++++--- macros.python3 | 13 ++++++++++--- python-rpm-macros.spec | 2 ++ tests/test_evals.py | 25 +++++++++++++++++++++++-- 4 files changed, 45 insertions(+), 8 deletions(-) diff --git a/macros.python b/macros.python index 9f05dae..cb4ee5b 100644 --- a/macros.python +++ b/macros.python @@ -68,6 +68,7 @@ # With $PATH and $PYTHONPATH set to the %%buildroot, # try to import the Python module(s) given as command-line args or read from file (-f). +# Respect the custom values of %%py_shebang_flags or set nothing if it's undefined. # Filter and check import on only top-level modules using -t flag. # Exclude unwanted modules by passing their globs to -e option. # Useful as a smoke test in %%check when running tests is not feasible. @@ -77,11 +78,17 @@ PATH="%{buildroot}%{_bindir}:$PATH"\\\ PYTHONPATH="${PYTHONPATH:-%{buildroot}%{python_sitearch}:%{buildroot}%{python_sitelib}}"\\\ PYTHONDONTWRITEBYTECODE=1\\\ - %{__python} -%{py_shebang_flags} %{_rpmconfigdir}/redhat/import_all_modules.py\\\ %{lua: + local command = "%{__python} " + if rpm.expand("%{?py_shebang_flags}") ~= "" then + command = command .. "-%{py_shebang_flags}" + end + command = command .. " %{_rpmconfigdir}/redhat/import_all_modules.py " -- handle multiline arguments correctly, see https://bugzilla.redhat.com/2018809 - local args=rpm.expand('%{?**}'):gsub("[%s\\\\]*%s+", " ");print(args) - }} + local args=rpm.expand('%{?**}'):gsub("[%s\\\\]*%s+", " ") + print(command .. args) + } +} %python_provide() %{lua: local python = require "fedora.srpm.python" diff --git a/macros.python3 b/macros.python3 index a4a990e..283cc68 100644 --- a/macros.python3 +++ b/macros.python3 @@ -66,6 +66,7 @@ # With $PATH and $PYTHONPATH set to the %%buildroot, # try to import the Python 3 module(s) given as command-line args or read from file (-f). +# Respect the custom values of %%py3_shebang_flags or set nothing if it's undefined. # Filter and check import on only top-level modules using -t flag. # Exclude unwanted modules by passing their globs to -e option. # Useful as a smoke test in %%check when running tests is not feasible. @@ -75,11 +76,17 @@ PATH="%{buildroot}%{_bindir}:$PATH"\\\ PYTHONPATH="${PYTHONPATH:-%{buildroot}%{python3_sitearch}:%{buildroot}%{python3_sitelib}}"\\\ PYTHONDONTWRITEBYTECODE=1\\\ - %{__python3} -%{py3_shebang_flags} %{_rpmconfigdir}/redhat/import_all_modules.py\\\ %{lua: + local command = "%{__python3} " + if rpm.expand("%{?py3_shebang_flags}") ~= "" then + command = command .. "-%{py3_shebang_flags}" + end + command = command .. " %{_rpmconfigdir}/redhat/import_all_modules.py " -- handle multiline arguments correctly, see https://bugzilla.redhat.com/2018809 - local args=rpm.expand('%{?**}'):gsub("[%s\\\\]*%s+", " ");print(args) - }} + local args=rpm.expand('%{?**}'):gsub("[%s\\\\]*%s+", " ") + print(command .. args) + } +} # This only supports Python 3.5+ and will never work with Python 2. # Hence, it has no Python version in the name. diff --git a/python-rpm-macros.spec b/python-rpm-macros.spec index 21f4f4e..e6c8ff8 100644 --- a/python-rpm-macros.spec +++ b/python-rpm-macros.spec @@ -101,6 +101,8 @@ install -m 644 import_all_modules.py %{buildroot}%{_rpmconfigdir}/redhat/ * Mon Nov 01 2021 Karolina Surma - 3.9-41 - Fix multiline arguments processing for %%py_check_import Resolves: rhbz#2018809 +- Fix %%py_shebang_flags handling within %%py_check_import +Resolves: rhbz#2018615 * Mon Oct 25 2021 Karolina Surma - 3.9-40 - Introduce -f (read from file) option to %%py{3}_check_import diff --git a/tests/test_evals.py b/tests/test_evals.py index 07e06e4..0edb5a2 100644 --- a/tests/test_evals.py +++ b/tests/test_evals.py @@ -642,6 +642,7 @@ def test_py3_check_import(args, expected_args, __python3, lib): macros = { 'buildroot': 'BUILDROOT', '_rpmconfigdir': 'RPMCONFIGDIR', + 'py3_shebang_flags': 's', } if __python3 is not None: macros['__python3'] = __python3 @@ -664,7 +665,27 @@ def test_py3_check_import(args, expected_args, __python3, lib): PATH="BUILDROOT/usr/bin:$PATH" PYTHONPATH="${{PYTHONPATH:-BUILDROOT/usr/{lib}/python{x_y}/site-packages:BUILDROOT/usr/lib/python{x_y}/site-packages}}" PYTHONDONTWRITEBYTECODE=1 - {__python3 or '/usr/bin/python3'} -s RPMCONFIGDIR/redhat/import_all_modules.py - {expected_args} + {__python3 or '/usr/bin/python3'} -s RPMCONFIGDIR/redhat/import_all_modules.py {expected_args} """) assert lines == expected.splitlines() + + +@pytest.mark.parametrize( + 'shebang_flags_value, expected_shebang_flags', + [ + ('s', '-s'), + ('%{nil}', ''), + (None, ''), + ('Es', '-Es'), + ] +) +def test_py3_check_import_respects_shebang_flags(shebang_flags_value, expected_shebang_flags, lib): + macros = { + '_rpmconfigdir': 'RPMCONFIGDIR', + '__python3': '/usr/bin/python3', + 'py3_shebang_flags': shebang_flags_value, + } + lines = rpm_eval('%py3_check_import sys', **macros) + # Compare the last line of the command, that's where lua part is evaluated + expected = f'/usr/bin/python3 {expected_shebang_flags} RPMCONFIGDIR/redhat/import_all_modules.py sys' + assert lines[-1].strip() == expected From ebc198dc840de56c39d2e06c2855eb071fd8a580 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miro=20Hron=C4=8Dok?= Date: Mon, 1 Nov 2021 15:53:43 +0100 Subject: [PATCH 8/9] %py(3)_check_import: Process .pth files in site(arch|lib) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=2018551 --- import_all_modules.py | 15 +++++++++++++++ macros.python | 1 + macros.python3 | 1 + python-rpm-macros.spec | 2 ++ tests/test_evals.py | 1 + tests/test_import_all_modules.py | 33 ++++++++++++++++++++++++++++++++ 6 files changed, 53 insertions(+) diff --git a/import_all_modules.py b/import_all_modules.py index 5788b8c..3930236 100644 --- a/import_all_modules.py +++ b/import_all_modules.py @@ -3,7 +3,9 @@ import argparse import importlib import fnmatch +import os import re +import site import sys from contextlib import contextmanager @@ -139,6 +141,18 @@ def remove_unwanteds_from_sys_path(): sys.path = old_sys_path +def addsitedirs_from_environ(): + '''Load directories from the _PYTHONSITE environment variable (separated by :) + and load the ones already present in sys.path via site.addsitedir() + to handle .pth files in them. + + This is needed to properly import old-style namespace packages with nspkg.pth files. + See https://bugzilla.redhat.com/2018551 for a more detailed rationale.''' + for path in os.getenv('_PYTHONSITE', '').split(':'): + if path in sys.path: + site.addsitedir(path) + + def main(argv=None): cli_args = argparser().parse_args(argv) @@ -149,6 +163,7 @@ def main(argv=None): modules = read_modules_from_all_args(cli_args) with remove_unwanteds_from_sys_path(): + addsitedirs_from_environ() import_modules(modules) diff --git a/macros.python b/macros.python index cb4ee5b..a773e57 100644 --- a/macros.python +++ b/macros.python @@ -77,6 +77,7 @@ %py_check_import(e:tf:) %{expand:\\\ PATH="%{buildroot}%{_bindir}:$PATH"\\\ PYTHONPATH="${PYTHONPATH:-%{buildroot}%{python_sitearch}:%{buildroot}%{python_sitelib}}"\\\ + _PYTHONSITE="%{buildroot}%{python_sitearch}:%{buildroot}%{python_sitelib}"\\\ PYTHONDONTWRITEBYTECODE=1\\\ %{lua: local command = "%{__python} " diff --git a/macros.python3 b/macros.python3 index 283cc68..d545d8f 100644 --- a/macros.python3 +++ b/macros.python3 @@ -75,6 +75,7 @@ %py3_check_import(e:tf:) %{expand:\\\ PATH="%{buildroot}%{_bindir}:$PATH"\\\ PYTHONPATH="${PYTHONPATH:-%{buildroot}%{python3_sitearch}:%{buildroot}%{python3_sitelib}}"\\\ + _PYTHONSITE="%{buildroot}%{python3_sitearch}:%{buildroot}%{python3_sitelib}"\\\ PYTHONDONTWRITEBYTECODE=1\\\ %{lua: local command = "%{__python3} " diff --git a/python-rpm-macros.spec b/python-rpm-macros.spec index e6c8ff8..965d93d 100644 --- a/python-rpm-macros.spec +++ b/python-rpm-macros.spec @@ -103,6 +103,8 @@ install -m 644 import_all_modules.py %{buildroot}%{_rpmconfigdir}/redhat/ Resolves: rhbz#2018809 - Fix %%py_shebang_flags handling within %%py_check_import Resolves: rhbz#2018615 +- Process .pth files in buildroot's sitedirs in %%py_check_import +Resolves: rhbz#2018551 * Mon Oct 25 2021 Karolina Surma - 3.9-40 - Introduce -f (read from file) option to %%py{3}_check_import diff --git a/tests/test_evals.py b/tests/test_evals.py index 0edb5a2..2991328 100644 --- a/tests/test_evals.py +++ b/tests/test_evals.py @@ -664,6 +664,7 @@ def test_py3_check_import(args, expected_args, __python3, lib): expected = textwrap.dedent(fr""" PATH="BUILDROOT/usr/bin:$PATH" PYTHONPATH="${{PYTHONPATH:-BUILDROOT/usr/{lib}/python{x_y}/site-packages:BUILDROOT/usr/lib/python{x_y}/site-packages}}" + _PYTHONSITE="BUILDROOT/usr/{lib}/python{x_y}/site-packages:BUILDROOT/usr/lib/python{x_y}/site-packages" PYTHONDONTWRITEBYTECODE=1 {__python3 or '/usr/bin/python3'} -s RPMCONFIGDIR/redhat/import_all_modules.py {expected_args} """) diff --git a/tests/test_import_all_modules.py b/tests/test_import_all_modules.py index d60f6e4..52e1d7e 100644 --- a/tests/test_import_all_modules.py +++ b/tests/test_import_all_modules.py @@ -391,3 +391,36 @@ def test_module_list_from_relative_path(tmp_path, monkeypatch): ]) assert 'wave' in sys.modules + + +@pytest.mark.parametrize('arch_in_path', [True, False]) +def test_pth_files_are_read_from__PYTHONSITE(arch_in_path, tmp_path, monkeypatch, capsys): + sitearch = tmp_path / 'lib64' + sitearch.mkdir() + sitelib = tmp_path / 'lib' + sitelib.mkdir() + + for where, word in (sitearch, "ARCH"), (sitelib, "LIB"), (sitelib, "MOD"): + module = where / f'print{word}.py' + module.write_text(f'print("{word}")') + + pth_sitearch = sitearch / 'ARCH.pth' + pth_sitearch.write_text('import printARCH\n') + + pth_sitelib = sitelib / 'LIB.pth' + pth_sitelib.write_text('import printLIB\n') + + if arch_in_path: + sys.path.append(str(sitearch)) + sys.path.append(str(sitelib)) + + # we always add sitearch to _PYTHONSITE + # but when not in sys.path, it should not be processed for .pth files + monkeypatch.setenv('_PYTHONSITE', f'{sitearch}:{sitelib}') + + modules_main(['printMOD']) + out, err = capsys.readouterr() + if arch_in_path: + assert out == 'ARCH\nLIB\nMOD\n' + else: + assert out == 'LIB\nMOD\n' From 65bf55d5d3e92747adee22628b84933ba3f1547f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miro=20Hron=C4=8Dok?= Date: Mon, 1 Nov 2021 16:04:22 +0100 Subject: [PATCH 9/9] Move import_all_modules out of python-srpm-macros There's no need for it in the default buildroot. --- python-rpm-macros.spec | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/python-rpm-macros.spec b/python-rpm-macros.spec index 965d93d..f51af6f 100644 --- a/python-rpm-macros.spec +++ b/python-rpm-macros.spec @@ -59,7 +59,7 @@ Summary: RPM macros for building Python 3 packages # For %%__python3 and %%python3 Requires: python-srpm-macros = %{version}-%{release} -# For %%py_setup +# For %%py_setup and import_all_modules.py Requires: python-rpm-macros = %{version}-%{release} %description -n python3-rpm-macros @@ -86,11 +86,11 @@ install -m 644 import_all_modules.py %{buildroot}%{_rpmconfigdir}/redhat/ %files %{rpmmacrodir}/macros.python %{rpmmacrodir}/macros.pybytecompile +%{_rpmconfigdir}/redhat/import_all_modules.py %files -n python-srpm-macros %{rpmmacrodir}/macros.python-srpm %{_rpmconfigdir}/redhat/compileall2.py -%{_rpmconfigdir}/redhat/import_all_modules.py %{_rpmluadir}/fedora/srpm/python.lua %files -n python3-rpm-macros @@ -105,6 +105,7 @@ Resolves: rhbz#2018809 Resolves: rhbz#2018615 - Process .pth files in buildroot's sitedirs in %%py_check_import Resolves: rhbz#2018551 +- Move import_all_modules.py from python-srpm-macros to python-rpm-macros * Mon Oct 25 2021 Karolina Surma - 3.9-40 - Introduce -f (read from file) option to %%py{3}_check_import