From d75ef6a945c5e405c9f8c902af6be86d5a3b8caf Mon Sep 17 00:00:00 2001 From: Devin Conathan Date: Tue, 3 Jan 2023 17:00:26 -0500 Subject: [PATCH 1/4] update how extras are extracted to handle cases with more than 2 groups --- docs/changelog/2791.bugfix.rst | 1 + .../python/virtual_env/package/util.py | 40 ++++++++++++------- .../package/test_python_package_util.py | 12 +++++- 3 files changed, 37 insertions(+), 16 deletions(-) create mode 100644 docs/changelog/2791.bugfix.rst diff --git a/docs/changelog/2791.bugfix.rst b/docs/changelog/2791.bugfix.rst new file mode 100644 index 000000000..4d0f5bfc7 --- /dev/null +++ b/docs/changelog/2791.bugfix.rst @@ -0,0 +1 @@ +Fix extracting extras from markers with >2 extras - by :user:`dconathan` \ No newline at end of file diff --git a/src/tox/tox_env/python/virtual_env/package/util.py b/src/tox/tox_env/python/virtual_env/package/util.py index e57efee97..c8501fd82 100644 --- a/src/tox/tox_env/python/virtual_env/package/util.py +++ b/src/tox/tox_env/python/virtual_env/package/util.py @@ -34,20 +34,32 @@ def extract_extra_markers(deps: list[Requirement]) -> list[tuple[Requirement, se for req in deps: req = deepcopy(req) markers: list[str | tuple[Variable, Variable, Variable]] = getattr(req.marker, "_markers", []) or [] - _at: int | None = None + new_markers: list[str | tuple[Variable, Variable, Variable]] = [] + + def _is_extra_marker(_marker) -> bool: + return ( + isinstance(_marker, tuple) + and len(_marker) == 3 + and _marker[0].value == "extra" + and _marker[1].value == "==" + ) + extra_markers = set() - for _at, (marker_key, op, marker_value) in ( - (_at_marker, marker) - for _at_marker, marker in enumerate(markers) - if isinstance(marker, tuple) and len(marker) == 3 - ): - if marker_key.value == "extra" and op.value == "==": # pragma: no branch - extra_markers.add(marker_value.value) - del markers[_at] - _at -= 1 - if _at >= 0 and (isinstance(markers[_at], str) and markers[_at] in ("and", "or")): - del markers[_at] - if len(markers) == 0: - req.marker = None + marker = markers.pop(0) if markers else None + while marker: + if _is_extra_marker(marker): + extra_markers.add(marker[2].value) + if new_markers and new_markers[-1] in ("and", "or"): + del new_markers[-1] + marker = markers.pop(0) if markers else None + if marker in ("and", "or"): + marker = markers.pop(0) if markers else None + else: + new_markers.append(marker) + marker = markers.pop(0) if markers else None + if new_markers: + req.marker._markers = new_markers + else: + req.marker = None result.append((req, extra_markers or {None})) return result diff --git a/tests/tox_env/python/virtual_env/package/test_python_package_util.py b/tests/tox_env/python/virtual_env/package/test_python_package_util.py index 5cb188128..87bf21cf9 100644 --- a/tests/tox_env/python/virtual_env/package/test_python_package_util.py +++ b/tests/tox_env/python/virtual_env/package/test_python_package_util.py @@ -67,5 +67,13 @@ def test_loads_deps_recursive_extras() -> None: def test_load_dependency_requirement_or_extras() -> None: requires = [Requirement('filelock<4.0.0,>=3.9.0; extra == "extras1" or extra == "extras2"')] - result = dependencies_with_extras(requires, {"extras1"}, "") - assert [str(r) for r in result] == ["filelock<4.0.0,>=3.9.0"] + for extras in ["extras1", "extras2"]: + result = dependencies_with_extras(requires, {extras}, "") + assert [str(r) for r in result] == ["filelock<4.0.0,>=3.9.0"] + + +def test_load_dependency_requirement_many_or_extras() -> None: + requires = [Requirement('filelock<4.0.0,>=3.9.0; extra == "extras1" or extra == "extras2" or extra == "extras3"')] + for extras in ["extras1", "extras2", "extras3"]: + result = dependencies_with_extras(requires, {extras}, "") + assert [str(r) for r in result] == ["filelock<4.0.0,>=3.9.0"] From 4d4e2a8de449a9a084ebb5ba051985f8b9147ecd Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 3 Jan 2023 22:11:35 +0000 Subject: [PATCH 2/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- docs/changelog/2791.bugfix.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/2791.bugfix.rst b/docs/changelog/2791.bugfix.rst index 4d0f5bfc7..ee7c6f755 100644 --- a/docs/changelog/2791.bugfix.rst +++ b/docs/changelog/2791.bugfix.rst @@ -1 +1 @@ -Fix extracting extras from markers with >2 extras - by :user:`dconathan` \ No newline at end of file +Fix extracting extras from markers with >2 extras - by :user:`dconathan` From 7ff1beff1e2800eb4d1520ae9c1a0c22aebfeef5 Mon Sep 17 00:00:00 2001 From: Devin Conathan Date: Tue, 3 Jan 2023 17:22:38 -0500 Subject: [PATCH 3/4] added some type annotations/ignore for mypy checks --- src/tox/tox_env/python/virtual_env/package/util.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tox/tox_env/python/virtual_env/package/util.py b/src/tox/tox_env/python/virtual_env/package/util.py index c8501fd82..ec8990cf5 100644 --- a/src/tox/tox_env/python/virtual_env/package/util.py +++ b/src/tox/tox_env/python/virtual_env/package/util.py @@ -36,7 +36,7 @@ def extract_extra_markers(deps: list[Requirement]) -> list[tuple[Requirement, se markers: list[str | tuple[Variable, Variable, Variable]] = getattr(req.marker, "_markers", []) or [] new_markers: list[str | tuple[Variable, Variable, Variable]] = [] - def _is_extra_marker(_marker) -> bool: + def _is_extra_marker(_marker: str | tuple[Variable, Variable, Variable]) -> bool: return ( isinstance(_marker, tuple) and len(_marker) == 3 @@ -48,7 +48,7 @@ def _is_extra_marker(_marker) -> bool: marker = markers.pop(0) if markers else None while marker: if _is_extra_marker(marker): - extra_markers.add(marker[2].value) + extra_markers.add(marker[2].value) # type: ignore if new_markers and new_markers[-1] in ("and", "or"): del new_markers[-1] marker = markers.pop(0) if markers else None @@ -58,7 +58,7 @@ def _is_extra_marker(_marker) -> bool: new_markers.append(marker) marker = markers.pop(0) if markers else None if new_markers: - req.marker._markers = new_markers + req.marker._markers = new_markers # type: ignore else: req.marker = None result.append((req, extra_markers or {None})) From 6dfdf537cf9cc6c737871f021c07e6bf676d4cf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Tue, 3 Jan 2023 20:41:56 -0800 Subject: [PATCH 4/4] PR feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Bernát Gábor --- docs/changelog/2791.bugfix.rst | 2 +- .../python/virtual_env/package/util.py | 68 ++++++++++--------- .../package/test_python_package_util.py | 8 +-- 3 files changed, 42 insertions(+), 36 deletions(-) diff --git a/docs/changelog/2791.bugfix.rst b/docs/changelog/2791.bugfix.rst index ee7c6f755..cc2e9f9cc 100644 --- a/docs/changelog/2791.bugfix.rst +++ b/docs/changelog/2791.bugfix.rst @@ -1 +1 @@ -Fix extracting extras from markers with >2 extras - by :user:`dconathan` +Fix extracting extras from markers with more than 2 extras in an or chain - by :user:`dconathan`. diff --git a/src/tox/tox_env/python/virtual_env/package/util.py b/src/tox/tox_env/python/virtual_env/package/util.py index ec8990cf5..948de48c9 100644 --- a/src/tox/tox_env/python/virtual_env/package/util.py +++ b/src/tox/tox_env/python/virtual_env/package/util.py @@ -1,8 +1,9 @@ from __future__ import annotations from copy import deepcopy +from typing import Optional, Set, cast -from packaging.markers import Variable # type: ignore[attr-defined] +from packaging.markers import Marker, Op, Value, Variable # type: ignore[attr-defined] from packaging.requirements import Requirement @@ -29,37 +30,42 @@ def dependencies_with_extras(deps: list[Requirement], extras: set[str], package_ def extract_extra_markers(deps: list[Requirement]) -> list[tuple[Requirement, set[str | None]]]: - # extras might show up as markers, move them into extras property - result: list[tuple[Requirement, set[str | None]]] = [] - for req in deps: - req = deepcopy(req) - markers: list[str | tuple[Variable, Variable, Variable]] = getattr(req.marker, "_markers", []) or [] - new_markers: list[str | tuple[Variable, Variable, Variable]] = [] + """ + Extract extra markers from dependencies. - def _is_extra_marker(_marker: str | tuple[Variable, Variable, Variable]) -> bool: - return ( - isinstance(_marker, tuple) - and len(_marker) == 3 - and _marker[0].value == "extra" - and _marker[1].value == "==" - ) + :param deps: the dependencies + :return: a list of requirement, extras set + """ + result = [_extract_extra_markers(d) for d in deps] + return result - extra_markers = set() - marker = markers.pop(0) if markers else None - while marker: - if _is_extra_marker(marker): - extra_markers.add(marker[2].value) # type: ignore - if new_markers and new_markers[-1] in ("and", "or"): - del new_markers[-1] - marker = markers.pop(0) if markers else None - if marker in ("and", "or"): - marker = markers.pop(0) if markers else None - else: - new_markers.append(marker) + +def _extract_extra_markers(req: Requirement) -> tuple[Requirement, set[str | None]]: + req = deepcopy(req) + markers: list[str | tuple[Variable, Op, Variable]] = getattr(req.marker, "_markers", []) or [] + new_markers: list[str | tuple[Variable, Op, Variable]] = [] + extra_markers: set[str] = set() # markers that have a key of extra + marker = markers.pop(0) if markers else None + while marker: + extra = _get_extra(marker) + if extra is not None: + extra_markers.add(extra) + if new_markers and new_markers[-1] in ("and", "or"): + del new_markers[-1] + marker = markers.pop(0) if markers else None + if marker in ("and", "or"): marker = markers.pop(0) if markers else None - if new_markers: - req.marker._markers = new_markers # type: ignore else: - req.marker = None - result.append((req, extra_markers or {None})) - return result + new_markers.append(marker) + marker = markers.pop(0) if markers else None + if new_markers: + cast(Marker, req.marker)._markers = new_markers + else: + req.marker = None + return req, cast(Set[Optional[str]], extra_markers) or {None} + + +def _get_extra(_marker: str | tuple[Variable, Op, Value]) -> str | None: + if isinstance(_marker, tuple) and len(_marker) == 3 and _marker[0].value == "extra" and _marker[1].value == "==": + return cast(str, _marker[2].value) + return None diff --git a/tests/tox_env/python/virtual_env/package/test_python_package_util.py b/tests/tox_env/python/virtual_env/package/test_python_package_util.py index 87bf21cf9..1e2e18816 100644 --- a/tests/tox_env/python/virtual_env/package/test_python_package_util.py +++ b/tests/tox_env/python/virtual_env/package/test_python_package_util.py @@ -72,8 +72,8 @@ def test_load_dependency_requirement_or_extras() -> None: assert [str(r) for r in result] == ["filelock<4.0.0,>=3.9.0"] -def test_load_dependency_requirement_many_or_extras() -> None: +@pytest.mark.parametrize("extra", ["extras1", "extras2", "extras3"]) +def test_load_dependency_requirement_many_or_extras(extra: str) -> None: requires = [Requirement('filelock<4.0.0,>=3.9.0; extra == "extras1" or extra == "extras2" or extra == "extras3"')] - for extras in ["extras1", "extras2", "extras3"]: - result = dependencies_with_extras(requires, {extras}, "") - assert [str(r) for r in result] == ["filelock<4.0.0,>=3.9.0"] + result = dependencies_with_extras(requires, {extra}, "") + assert [str(r) for r in result] == ["filelock<4.0.0,>=3.9.0"]