Repository: tox-dev/tox

Issue:
tox4: not all extras are evaluated
I have a tox ini with a `testenv` incl `extras = test` and a `testenv:coverage` incl. `extras=test, coverage`.

The coverage env fails with `pytest-cov` missing - only in `tox4`.

# reproduce

```
git clone git@github.com:morepath/more.cerberus.git

cd more.cerburus

tox4 -r
```

This only happens when running all envs - the error does not show when running `coverage` in isolation.

Tested both with current tox pre and tox head.

```
coverage: remove tox env folder /home/jugmac00/Projects/more.cerberus/.tox/4/coverage
coverage: install_package_deps> .tox/4/coverage/bin/python -I -m pip install 'setuptools>=40.8.0' wheel 'cerberus<2.0.0,>=1.3.2' 'morepath>=0.19' 'pytest>=2.9.1' pytest-remove-stale-bytecode webtest
coverage: install_package> .tox/4/coverage/bin/python -I -m pip install --no-deps --force-reinstall --no-build-isolation -e /home/jugmac00/Projects/more.cerberus
coverage: commands[0]> pytest --cov --cov-fail-under=100
ERROR: usage: pytest [options] [file_or_dir] [file_or_dir] [...]
pytest: error: unrecognized arguments: --cov --cov-fail-under=100
  inifile: /home/jugmac00/Projects/more.cerberus/setup.cfg
  rootdir: /home/jugmac00/Projects/more.cerberus

coverage: exit 4 (0.20 seconds) /home/jugmac00/Projects/more.cerberus> pytest --cov --cov-fail-under=100
.pkg: _exit> python /home/jugmac00/opt/tox4/lib/python3.8/site-packages/tox/util/pep517/backend.py True setuptools.build_meta __legacy__
coverage: FAIL ✖ in 4.42 seconds
```

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/tox-dev/tox/pull/1869?src=pr&el=h1) Report
> Merging [#1869](https://codecov.io/gh/tox-dev/tox/pull/1869?src=pr&el=desc) (3a569a8) into [rewrite](https://codecov.io/gh/tox-dev/tox/commit/f17774fdd9fceea40564110bdeac7ce4b30ed677?el=desc) (f17774f) will **increase** coverage by `0.19%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/tox-dev/tox/pull/1869/graphs/tree.svg?width=650&height=150&src=pr&token=DYodAwDCZ5)](https://codecov.io/gh/tox-dev/tox/pull/1869?src=pr&el=tree)

```diff
@@             Coverage Diff              @@
##           rewrite     #1869      +/-   ##
============================================
+ Coverage    99.80%   100.00%   +0.19%     
============================================
  Files          134       133       -1     
  Lines         7364      7269      -95     
  Branches       733       715      -18     
============================================
- Hits          7350      7269      -81     
+ Misses           3         0       -3     
+ Partials        11         0      -11     
```

| Flag | Coverage Δ | |
|---|---|---|
| tests | `100.00% <100.00%> (+0.19%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/tox-dev/tox/pull/1869?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/tox/tox\_env/python/api.py](https://codecov.io/gh/tox-dev/tox/pull/1869/diff?src=pr&el=tree#diff-c3JjL3RveC90b3hfZW52L3B5dGhvbi9hcGkucHk=) | `100.00% <100.00%> (+0.68%)` | :arrow_up: |
| [src/tox/tox\_env/python/virtual\_env/package/api.py](https://codecov.io/gh/tox-dev/tox/pull/1869/diff?src=pr&el=tree#diff-c3JjL3RveC90b3hfZW52L3B5dGhvbi92aXJ0dWFsX2Vudi9wYWNrYWdlL2FwaS5weQ==) | `100.00% <100.00%> (ø)` | |
| [tests/tox\_env/python/virtual\_env/test\_package.py](https://codecov.io/gh/tox-dev/tox/pull/1869/diff?src=pr&el=tree#diff-dGVzdHMvdG94X2Vudi9weXRob24vdmlydHVhbF9lbnYvdGVzdF9wYWNrYWdlLnB5) | `100.00% <100.00%> (ø)` | |
| [src/tox/pytest.py](https://codecov.io/gh/tox-dev/tox/pull/1869/diff?src=pr&el=tree#diff-c3JjL3RveC9weXRlc3QucHk=) | `100.00% <0.00%> (ø)` | |
| [src/tox/tox\_env/api.py](https://codecov.io/gh/tox-dev/tox/pull/1869/diff?src=pr&el=tree#diff-c3JjL3RveC90b3hfZW52L2FwaS5weQ==) | `100.00% <0.00%> (ø)` | |
| [src/tox/session/state.py](https://codecov.io/gh/tox-dev/tox/pull/1869/diff?src=pr&el=tree#diff-c3JjL3RveC9zZXNzaW9uL3N0YXRlLnB5) | `100.00% <0.00%> (ø)` | |
| [src/tox/execute/request.py](https://codecov.io/gh/tox-dev/tox/pull/1869/diff?src=pr&el=tree#diff-c3JjL3RveC9leGVjdXRlL3JlcXVlc3QucHk=) | `100.00% <0.00%> (ø)` | |
| [src/tox/util/pep517/backend.py](https://codecov.io/gh/tox-dev/tox/pull/1869/diff?src=pr&el=tree#diff-c3JjL3RveC91dGlsL3BlcDUxNy9iYWNrZW5kLnB5) | `100.00% <0.00%> (ø)` | |
| [src/tox/execute/pep517\_backend.py](https://codecov.io/gh/tox-dev/tox/pull/1869/diff?src=pr&el=tree#diff-c3JjL3RveC9leGVjdXRlL3BlcDUxN19iYWNrZW5kLnB5) | `100.00% <0.00%> (ø)` | |
| [src/tox/session/cmd/run/common.py](https://codecov.io/gh/tox-dev/tox/pull/1869/diff?src=pr&el=tree#diff-c3JjL3RveC9zZXNzaW9uL2NtZC9ydW4vY29tbW9uLnB5) | `100.00% <0.00%> (ø)` | |
| ... and [8 more](https://codecov.io/gh/tox-dev/tox/pull/1869/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/tox-dev/tox/pull/1869?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/tox-dev/tox/pull/1869?src=pr&el=footer). Last update [f17774f...7ea918c](https://codecov.io/gh/tox-dev/tox/pull/1869?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/docs/changelog/1868.feature.rst b/docs/changelog/1868.feature.rst
new file mode 100644
index 00000000..8511d2f4
--- /dev/null
+++ b/docs/changelog/1868.feature.rst
@@ -0,0 +1,2 @@
+Not all package dependencies are installed when different tox environments in the same run use different set of
+extras - by :user:`gaborbernat`.
diff --git a/src/tox/tox_env/python/api.py b/src/tox/tox_env/python/api.py
index 015ff16b..e5402a0f 100644
--- a/src/tox/tox_env/python/api.py
+++ b/src/tox/tox_env/python/api.py
@@ -222,8 +222,7 @@ class Python(ToxEnv, ABC):
                 # bail out and force recreate
                 logging.warning(f"recreate env because dependencies removed: {', '.join(str(i) for i in missing)}")
                 raise Recreate
-            new_deps_str = set(conf_deps) - set(old)
-            new_deps = [PythonDep(Requirement(i)) for i in new_deps_str]
+            new_deps = [PythonDep(Requirement(i)) for i in conf_deps if i not in old]
             self.install_python_packages(packages=new_deps, of_type=of_type)
         return False
 
diff --git a/src/tox/tox_env/python/virtual_env/package/api.py b/src/tox/tox_env/python/virtual_env/package/api.py
index b3a0a11b..738af1e1 100644
--- a/src/tox/tox_env/python/virtual_env/package/api.py
+++ b/src/tox/tox_env/python/virtual_env/package/api.py
@@ -1,6 +1,7 @@
 import os
 import sys
 from contextlib import contextmanager
+from copy import deepcopy
 from enum import Enum
 from pathlib import Path
 from threading import RLock
@@ -95,6 +96,7 @@ class Pep517VirtualEnvPackage(VirtualEnv, PythonPackage, Frontend):
         self._build_wheel_cache: Optional[WheelResult] = None
         self._backend_executor: Optional[LocalSubProcessPep517Executor] = None
         self._package_dependencies: Optional[List[Requirement]] = None
+        self._package_dev_dependencies: Optional[List[Requirement]] = None
         self._lock = RLock()  # can build only one package at a time
         self._package: Dict[Tuple[PackageType, str], Any] = {}
         self._run_env_to_wheel_builder_env: Dict[str, PackageToxEnv] = {}
@@ -203,27 +205,28 @@ class Pep517VirtualEnvPackage(VirtualEnv, PythonPackage, Frontend):
 
     def get_package_dependencies(self, for_env: EnvConfigSet) -> List[Requirement]:
         env_name = for_env.name
-        extras: Set[str] = for_env["extras"]
         with self._lock:
             if self._package_dependencies is None:  # pragma: no branch
                 self._ensure_meta_present()
-                dependencies: List[Requirement] = []
-                of_type, _ = self._run_env_to_info[env_name]
-                if of_type == PackageType.dev:
-                    dependencies.extend(self.requires())
-                    dependencies.extend(self.get_requires_for_build_sdist().requires)
-                dependencies.extend(self.discover_package_dependencies(self._distribution_meta, extras))
-                self._package_dependencies = dependencies
-        return self._package_dependencies
+                requires: List[str] = cast(PathDistribution, self._distribution_meta).requires or []
+                self._package_dependencies = [Requirement(i) for i in requires]
+            of_type, _ = self._run_env_to_info[env_name]
+            if of_type == PackageType.dev and self._package_dev_dependencies is None:
+                self._package_dev_dependencies = [*self.requires(), *self.get_requires_for_build_sdist().requires]
+        if of_type == PackageType.dev:
+            result: List[Requirement] = cast(List[Requirement], self._package_dev_dependencies).copy()
+        else:
+            result = []
+        extras: Set[str] = for_env["extras"]
+        result.extend(self.dependencies_with_extras(self._package_dependencies, extras))
+        return result
 
     @staticmethod
-    def discover_package_dependencies(meta: PathDistribution, extras: Set[str]) -> List[Requirement]:
+    def dependencies_with_extras(deps: List[Requirement], extras: Set[str]) -> List[Requirement]:
         result: List[Requirement] = []
-        requires = meta.requires or []
-        for req_str in requires:
-            req = Requirement(req_str)
+        for req in deps:
+            req = deepcopy(req)
             markers: List[Union[str, Tuple[Variable, Variable, Variable]]] = getattr(req.marker, "_markers", []) or []
-
             # find the extra marker (if has)
             _at: Optional[int] = None
             extra: Optional[str] = None
@@ -241,7 +244,6 @@ class Pep517VirtualEnvPackage(VirtualEnv, PythonPackage, Frontend):
                     if len(markers) == 0:
                         req.marker = None
                     break
-            # continue only if this extra should be included
             if not (extra is None or extra in extras):
                 continue
             result.append(req)


Test patch:
diff --git a/tests/tox_env/python/virtual_env/test_package.py b/tests/tox_env/python/virtual_env/test_package.py
index 9ba1f14e..d3ad4328 100644
--- a/tests/tox_env/python/virtual_env/test_package.py
+++ b/tests/tox_env/python/virtual_env/test_package.py
@@ -1,5 +1,6 @@
 import sys
 from itertools import zip_longest
+from pathlib import Path
 from textwrap import dedent
 
 import pytest
@@ -37,7 +38,7 @@ def test_tox_ini_package_type_invalid(tox_project: ToxProjectCreator) -> None:
 
 
 @pytest.fixture(scope="session")
-def pkg_with_extras(tmp_path_factory: TempPathFactory) -> PathDistribution:  # type: ignore[no-any-unimported]
+def pkg_with_extras_project(tmp_path_factory: TempPathFactory) -> Path:
     py_ver = ".".join(str(i) for i in sys.version_info[0:2])
     setup_cfg = f"""
     [metadata]
@@ -64,14 +65,19 @@ def pkg_with_extras(tmp_path_factory: TempPathFactory) -> PathDistribution:  # t
     (tmp_path / "setup.py").write_text("from setuptools import setup; setup()")
     toml = '[build-system]\nrequires=["setuptools", "wheel"]\nbuild-backend = "setuptools.build_meta"'
     (tmp_path / "pyproject.toml").write_text(toml)
-    frontend = SubprocessFrontend(*SubprocessFrontend.create_args_from_folder(tmp_path)[:-1])
-    meta = tmp_path / "meta"
+    return tmp_path
+
+
+@pytest.fixture(scope="session")
+def pkg_with_extras(pkg_with_extras_project: Path) -> PathDistribution:  # type: ignore[no-any-unimported]
+    frontend = SubprocessFrontend(*SubprocessFrontend.create_args_from_folder(pkg_with_extras_project)[:-1])
+    meta = pkg_with_extras_project / "meta"
     result = frontend.prepare_metadata_for_build_wheel(meta)
     return Distribution.at(result.metadata)
 
 
 def test_load_dependency_no_extra(pkg_with_extras: PathDistribution) -> None:  # type: ignore[no-any-unimported]
-    result = Pep517VirtualEnvPackage.discover_package_dependencies(pkg_with_extras, set())
+    result = Pep517VirtualEnvPackage.dependencies_with_extras([Requirement(i) for i in pkg_with_extras.requires], set())
     for left, right in zip_longest(result, (Requirement("appdirs>=1.4.3"), Requirement("colorama>=0.4.3"))):
         assert isinstance(right, Requirement)
         assert str(left) == str(right)
@@ -79,7 +85,9 @@ def test_load_dependency_no_extra(pkg_with_extras: PathDistribution) -> None:  #
 
 def test_load_dependency_many_extra(pkg_with_extras: PathDistribution) -> None:  # type: ignore[no-any-unimported]
     py_ver = ".".join(str(i) for i in sys.version_info[0:2])
-    result = Pep517VirtualEnvPackage.discover_package_dependencies(pkg_with_extras, {"docs", "testing"})
+    result = Pep517VirtualEnvPackage.dependencies_with_extras(
+        [Requirement(i) for i in pkg_with_extras.requires], {"docs", "testing"}
+    )
     exp = [
         Requirement("appdirs>=1.4.3"),
         Requirement("colorama>=0.4.3"),
@@ -91,3 +99,19 @@ def test_load_dependency_many_extra(pkg_with_extras: PathDistribution) -> None:
     for left, right in zip_longest(result, exp):
         assert isinstance(right, Requirement)
         assert str(left) == str(right)
+
+
+def test_get_package_deps_different_extras(pkg_with_extras_project: Path, tox_project: ToxProjectCreator) -> None:
+    proj = tox_project({"tox.ini": "[testenv:a]\npackage=dev\nextras=docs\n[testenv:b]\npackage=sdist\nextras=format"})
+    execute_calls = proj.patch_execute(lambda r: 0 if "install" in r.run_id else None)
+    result = proj.run("r", "--root", str(pkg_with_extras_project), "-e", "a,b")
+    result.assert_success()
+    installs = {
+        i[0][0].conf.name: i[0][3].cmd[5:]
+        for i in execute_calls.call_args_list
+        if i[0][3].run_id.startswith("install_package_deps")
+    }
+    assert installs == {
+        "a": ["setuptools", "wheel", "appdirs>=1.4.3", "colorama>=0.4.3", "sphinx>=3", "sphinx-rtd-theme<1,>=0.4.3"],
+        "b": ["appdirs>=1.4.3", "colorama>=0.4.3", "black>=3", "flake8"],
+    }