Fix missing requirements with pyproject.toml (#3223)

abravalheri · web-flow · commit 2582a8bc1a93 · 2022-03-27T17:32:52.000+01:00
diff --git a/changelog.d/3223.misc.rst b/changelog.d/3223.misc.rst
@@ -0,0 +1,2 @@
+Fixed missing requirements with environment markers when
+``optional-dependencies`` is set in ``pyproject.toml``.
diff --git a/setuptools/__init__.py b/setuptools/__init__.py
@@ -58,6 +58,14 @@ def __init__(self, attrs):
             # Prevent accidentally triggering discovery with incomplete set of attrs
             self.set_defaults._disable()
 
+        def _get_project_config_files(self, filenames=None):
+            """Ignore ``pyproject.toml``, they are not related to setup_requires"""
+            try:
+                cfg, toml = super()._split_standard_project_metadata(filenames)
+                return cfg, ()
+            except Exception:
+                return filenames, ()
+
         def finalize_options(self):
             """
             Disable finalize_options to avoid building the working set.
diff --git a/setuptools/config/_apply_pyprojecttoml.py b/setuptools/config/_apply_pyprojecttoml.py
@@ -194,6 +194,16 @@ def _python_requires(dist: "Distribution", val: dict, _root_dir):
     _set_config(dist, "python_requires", SpecifierSet(val))
 
 
+def _dependencies(dist: "Distribution", val: list, _root_dir):
+    existing = getattr(dist, "install_requires", [])
+    _set_config(dist, "install_requires", existing + val)
+
+
+def _optional_dependencies(dist: "Distribution", val: dict, _root_dir):
+    existing = getattr(dist, "extras_require", {})
+    _set_config(dist, "extras_require", {**existing, **val})
+
+
 def _unify_entry_points(project_table: dict):
     project = project_table
     entry_points = project.pop("entry-points", project.pop("entry_points", {}))
@@ -293,7 +303,7 @@ def _some_attrgetter(*items):
     """
     def _acessor(obj):
         values = (_attrgetter(i)(obj) for i in items)
-        return next((i for i in values if i), None)
+        return next((i for i in values if i is not None), None)
     return _acessor
 
 
@@ -303,8 +313,8 @@ def _acessor(obj):
     "authors": partial(_people, kind="author"),
     "maintainers": partial(_people, kind="maintainer"),
     "urls": _project_urls,
-    "dependencies": "install_requires",
-    "optional_dependencies": "extras_require",
+    "dependencies": _dependencies,
+    "optional_dependencies": _optional_dependencies,
     "requires_python": _python_requires,
 }
 
diff --git a/setuptools/config/pyprojecttoml.py b/setuptools/config/pyprojecttoml.py
@@ -9,7 +9,8 @@
 from setuptools.errors import FileError, OptionError
 
 from . import expand as _expand
-from ._apply_pyprojecttoml import apply, _PREVIOUSLY_DEFINED, _WouldIgnoreField
+from ._apply_pyprojecttoml import apply as _apply
+from ._apply_pyprojecttoml import _PREVIOUSLY_DEFINED, _WouldIgnoreField
 
 if TYPE_CHECKING:
     from setuptools.dist import Distribution  # noqa
@@ -44,13 +45,15 @@ def validate(config: dict, filepath: _Path):
 
 
 def apply_configuration(
-    dist: "Distribution", filepath: _Path, ignore_option_errors=False,
+    dist: "Distribution",
+    filepath: _Path,
+    ignore_option_errors=False,
 ) -> "Distribution":
     """Apply the configuration from a ``pyproject.toml`` file into an existing
     distribution object.
     """
     config = read_configuration(filepath, True, ignore_option_errors, dist)
-    return apply(dist, config, filepath)
+    return _apply(dist, config, filepath)
 
 
 def read_configuration(
@@ -279,11 +282,12 @@ def _expand_all_dynamic(self, dist: "Distribution", package_dir: Mapping[str, st
         )
         # `None` indicates there is nothing in `tool.setuptools.dynamic` but the value
         # might have already been set by setup.py/extensions, so avoid overwriting.
-        self.project_cfg.update({k: v for k, v in obtained_dynamic.items() if v})
+        updates = {k: v for k, v in obtained_dynamic.items() if v is not None}
+        self.project_cfg.update(updates)
 
     def _ensure_previously_set(self, dist: "Distribution", field: str):
         previous = _PREVIOUSLY_DEFINED[field](dist)
-        if not previous and not self.ignore_option_errors:
+        if previous is None and not self.ignore_option_errors:
             msg = (
                 f"No configuration found for dynamic {field!r}.\n"
                 "Some dynamic fields need to be specified via `tool.setuptools.dynamic`"
diff --git a/setuptools/config/setupcfg.py b/setuptools/config/setupcfg.py
@@ -70,7 +70,7 @@ def apply_configuration(dist: "Distribution", filepath: _Path) -> "Distribution"
 def _apply(
     dist: "Distribution", filepath: _Path,
     other_files: Iterable[_Path] = (),
-    ignore_option_errors: bool = False
+    ignore_option_errors: bool = False,
 ) -> Tuple["ConfigHandler", ...]:
     """Read configuration from ``filepath`` and applies to the ``dist`` object."""
     from setuptools.dist import _Distribution
@@ -677,9 +677,8 @@ def parse_section_extras_require(self, section_options):
         :param dict section_options:
         """
         parse_list = partial(self._parse_list, separator=';')
-        self['extras_require'] = self._parse_section_to_dict(
-            section_options, parse_list
-        )
+        parsed = self._parse_section_to_dict(section_options, parse_list)
+        self['extras_require'] = parsed
 
     def parse_section_data_files(self, section_options):
         """Parses `data_files` configuration file section.
diff --git a/setuptools/dist.py b/setuptools/dist.py
@@ -468,6 +468,11 @@ def __init__(self, attrs=None):
             },
         )
 
+        # Save the original dependencies before they are processed into the egg format
+        self._orig_extras_require = {}
+        self._orig_install_requires = []
+        self._tmp_extras_require = defaultdict(ordered_set.OrderedSet)
+
         self.set_defaults = ConfigDiscovery(self)
 
         self._set_metadata_defaults(attrs)
@@ -540,6 +545,8 @@ def _finalize_requires(self):
             self.metadata.python_requires = self.python_requires
 
         if getattr(self, 'extras_require', None):
+            # Save original before it is messed by _convert_extras_requirements
+            self._orig_extras_require = self._orig_extras_require or self.extras_require
             for extra in self.extras_require.keys():
                 # Since this gets called multiple times at points where the
                 # keys have become 'converted' extras, ensure that we are only
@@ -548,6 +555,10 @@ def _finalize_requires(self):
                 if extra:
                     self.metadata.provides_extras.add(extra)
 
+        if getattr(self, 'install_requires', None) and not self._orig_install_requires:
+            # Save original before it is messed by _move_install_requirements_markers
+            self._orig_install_requires = self.install_requires
+
         self._convert_extras_requirements()
         self._move_install_requirements_markers()
 
@@ -558,7 +569,8 @@ def _convert_extras_requirements(self):
         `"extra:{marker}": ["barbazquux"]`.
         """
         spec_ext_reqs = getattr(self, 'extras_require', None) or {}
-        self._tmp_extras_require = defaultdict(list)
+        tmp = defaultdict(ordered_set.OrderedSet)
+        self._tmp_extras_require = getattr(self, '_tmp_extras_require', tmp)
         for section, v in spec_ext_reqs.items():
             # Do not strip empty sections.
             self._tmp_extras_require[section]
@@ -596,7 +608,8 @@ def is_simple_req(req):
         for r in complex_reqs:
             self._tmp_extras_require[':' + str(r.marker)].append(r)
         self.extras_require = dict(
-            (k, [str(r) for r in map(self._clean_req, v)])
+            # list(dict.fromkeys(...))  ensures a list of unique strings
+            (k, list(dict.fromkeys(str(r) for r in map(self._clean_req, v))))
             for k, v in self._tmp_extras_require.items()
         )
 
@@ -814,10 +827,8 @@ def _set_command_options(self, command_obj, option_dict=None):  # noqa: C901
             except ValueError as e:
                 raise DistutilsOptionError(e) from e
 
-    def parse_config_files(self, filenames=None, ignore_option_errors=False):
-        """Parses configuration files from various levels
-        and loads configuration.
-        """
+    def _get_project_config_files(self, filenames):
+        """Add default file and split between INI and TOML"""
         tomlfiles = []
         standard_project_metadata = Path(self.src_root or os.curdir, "pyproject.toml")
         if filenames is not None:
@@ -826,8 +837,15 @@ def parse_config_files(self, filenames=None, ignore_option_errors=False):
             tomlfiles = list(parts[1])  # 2nd element => predicate is True
         elif standard_project_metadata.exists():
             tomlfiles = [standard_project_metadata]
+        return filenames, tomlfiles
+
+    def parse_config_files(self, filenames=None, ignore_option_errors=False):
+        """Parses configuration files from various levels
+        and loads configuration.
+        """
+        inifiles, tomlfiles = self._get_project_config_files(filenames)
 
-        self._parse_config_files(filenames=filenames)
+        self._parse_config_files(filenames=inifiles)
 
         setupcfg.parse_configuration(
             self, self.command_options, ignore_option_errors=ignore_option_errors
diff --git a/setuptools/tests/config/test_apply_pyprojecttoml.py b/setuptools/tests/config/test_apply_pyprojecttoml.py
@@ -14,7 +14,8 @@
 from setuptools.dist import Distribution
 from setuptools.config import setupcfg, pyprojecttoml
 from setuptools.config import expand
-from setuptools.config._apply_pyprojecttoml import _WouldIgnoreField
+from setuptools.config._apply_pyprojecttoml import _WouldIgnoreField, _some_attrgetter
+from setuptools.command.egg_info import write_requirements
 
 
 EXAMPLES = (Path(__file__).parent / "setupcfg_examples.txt").read_text()
@@ -207,12 +208,12 @@ def test_license_and_license_files(tmp_path):
 
 
 class TestPresetField:
-    def pyproject(self, tmp_path, dynamic):
+    def pyproject(self, tmp_path, dynamic, extra_content=""):
         content = f"[project]\nname = 'proj'\ndynamic = {dynamic!r}\n"
         if "version" not in dynamic:
             content += "version = '42'\n"
         file = tmp_path / "pyproject.toml"
-        file.write_text(content, encoding="utf-8")
+        file.write_text(content + extra_content, encoding="utf-8")
         return file
 
     @pytest.mark.parametrize(
@@ -233,12 +234,14 @@ def test_not_listed_in_dynamic(self, tmp_path, attr, field, value):
             dist = pyprojecttoml.apply_configuration(dist, pyproject)
 
         # TODO: Once support for pyproject.toml config stabilizes attr should be None
-        dist_value = getattr(dist, attr, None) or getattr(dist.metadata, attr, object())
+        dist_value = _some_attrgetter(f"metadata.{attr}", attr)(dist)
         assert dist_value == value
 
     @pytest.mark.parametrize(
         "attr, field, value",
         [
+            ("install_requires", "dependencies", []),
+            ("extras_require", "optional-dependencies", {}),
             ("install_requires", "dependencies", ["six"]),
             ("classifiers", "classifiers", ["Private :: Classifier"]),
         ]
@@ -247,9 +250,31 @@ def test_listed_in_dynamic(self, tmp_path, attr, field, value):
         pyproject = self.pyproject(tmp_path, [field])
         dist = makedist(tmp_path, **{attr: value})
         dist = pyprojecttoml.apply_configuration(dist, pyproject)
-        dist_value = getattr(dist, attr, None) or getattr(dist.metadata, attr, object())
+        dist_value = _some_attrgetter(f"metadata.{attr}", attr)(dist)
         assert dist_value == value
 
+    def test_optional_dependencies_dont_remove_env_markers(self, tmp_path):
+        """
+        Internally setuptools converts dependencies with markers to "extras".
+        If ``install_requires`` is given by ``setup.py``, we have to ensure that
+        applying ``optional-dependencies`` does not overwrite the mandatory
+        dependencies with markers (see #3204).
+        """
+        # If setuptools replace its internal mechanism that uses `requires.txt`
+        # this test has to be rewritten to adapt accordingly
+        extra = "\n[project.optional-dependencies]\nfoo = ['bar>1']\n"
+        pyproject = self.pyproject(tmp_path, ["dependencies"], extra)
+        install_req = ['importlib-resources (>=3.0.0) ; python_version < "3.7"']
+        dist = makedist(tmp_path, install_requires=install_req)
+        dist = pyprojecttoml.apply_configuration(dist, pyproject)
+        assert "foo" in dist.extras_require
+        assert ':python_version < "3.7"' in dist.extras_require
+        egg_info = dist.get_command_obj("egg_info")
+        write_requirements(egg_info, tmp_path, tmp_path / "requires.txt")
+        reqs = (tmp_path / "requires.txt").read_text(encoding="utf-8")
+        assert "importlib-resources" in reqs
+        assert "bar" in reqs
+
 
 # --- Auxiliary Functions ---
 

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+Fixed missing requirements with environment markers when`
	`2`	+``optional-dependencies`` is set in ``pyproject.toml``.