Do not conflate extra markers and extra dependencies (#2605)

Resolves #2603

Author: gaborbernat

docs/changelog/2603.bugfix.rst

@@ -0,0 +1 @@
+Fix extras not being kept for install dependencies - by :user:`gaborbernat`.

src/tox/tox_env/python/virtual_env/package/util.py

@@ -7,20 +7,18 @@
 
 
 def dependencies_with_extras(deps: list[Requirement], extras: set[str], package_name: str) -> list[Requirement]:
-    deps = _normalize_req(deps)
+    deps_with_markers = extract_extra_markers(deps)
     result: list[Requirement] = []
     found: set[str] = set()
     todo: set[str | None] = extras | {None}
     visited: set[str | None] = set()
     while todo:
         new_extras: set[str | None] = set()
-        for req in deps:
-            if todo & (req.extras or {None}):  # type: ignore[arg-type]
+        for req, extra_markers in deps_with_markers:
+            if todo & extra_markers:
                 if req.name == package_name:  # support for recursive extras
                     new_extras.update(req.extras or set())
                 else:
-                    req = deepcopy(req)
-                    req.extras.clear()  # strip the extra part as the installation will invoke it without
                     req_str = str(req)
                     if req_str not in found:
                         found.add(req_str)
@@ -30,26 +28,27 @@ def dependencies_with_extras(deps: list[Requirement], extras: set[str], package_
     return result
 
 
-def _normalize_req(deps: list[Requirement]) -> list[Requirement]:
+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[Requirement] = []
+    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 []
         _at: int | None = None
+        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
-                req.extras.add(marker_value.value)
+                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
                 break
-        result.append(req)
+        result.append((req, extra_markers or {None}))
     return result

tests/tox_env/python/virtual_env/package/test_python_package_util.py

@@ -50,12 +50,12 @@ def test_load_dependency_many_extra(pkg_with_extras: PathDistribution) -> None:
 def test_loads_deps_recursive_extras() -> None:
     requires = [
         Requirement("no-extra"),
-        Requirement("dep1[dev]"),
-        Requirement("dep1[test]"),
-        Requirement("dep2[test]"),
-        Requirement("dep3[docs]"),
-        Requirement("name[dev]"),
-        Requirement("name[test,dev]"),
+        Requirement('dep1[magic]; extra=="dev"'),
+        Requirement('dep1; extra=="test"'),
+        Requirement('dep2[a,b]; extra=="test"'),
+        Requirement('dep3; extra=="docs"'),
+        Requirement('name; extra=="dev"'),
+        Requirement('name[test]; extra=="dev"'),
     ]
     result = dependencies_with_extras(requires, {"dev"}, "name")
-    assert [str(i) for i in result] == ["no-extra", "dep1", "dep2"]
+    assert [str(i) for i in result] == ["no-extra", "dep1[magic]", "dep1", "dep2[a,b]"]