Repository: pypa/setuptools
Pull request #4567 (2024-08-13 16:29:44)

Ensured all methods in `setuptools.modified` raise a consistant `distutils.error.DistutilsError` type
<!-- First time contributors: Take a moment to review https://setuptools.pypa.io/en/latest/development/developer-guide.html! -->
<!-- Remove sections if not applicable -->

## Summary of changes

The following code should now work as expected in all cases:
```py
from setuptools.modified import newer_pairwise_group
from distutils.error import DistutilsError

try:
    newer_pairwise_group(...)
except DistutilsError:
    pass
```

I'm not entirely certain my test is correct. Hopefully it should be.

### Pull Request Checklist
- [x] Changes have tests
- [x] News fragment added in [`newsfragments/`].
  _(See [documentation][PR docs] for details)_


[`newsfragments/`]: https://github.com/pypa/setuptools/tree/master/newsfragments
[PR docs]:
https://setuptools.pypa.io/en/latest/development/developer-guide.html#making-a-pull-request



Patch:
diff --git a/newsfragments/4567.bugfix.rst b/newsfragments/4567.bugfix.rst
new file mode 100644
index 000000000..7d7bb282e
--- /dev/null
+++ b/newsfragments/4567.bugfix.rst
@@ -0,0 +1,4 @@
+Ensured methods in ``setuptools.modified`` preferably raise a consistent
+``distutils.errors.DistutilsError`` type
+(except in the deprecated use case of ``SETUPTOOLS_USE_DISTUTILS=stdlib``)
+-- by :user:`Avasam`
diff --git a/newsfragments/4696.bugfix.rst b/newsfragments/4696.bugfix.rst
new file mode 100644
index 000000000..77ebf87c4
--- /dev/null
+++ b/newsfragments/4696.bugfix.rst
@@ -0,0 +1,4 @@
+Fix clashes for ``optional-dependencies`` in ``pyproject.toml`` and
+``extra_requires`` in ``setup.cfg/setup.py``.
+As per PEP 621, ``optional-dependencies`` has to be honoured and dynamic
+behaviour is not allowed.
diff --git a/setuptools/command/build_clib.py b/setuptools/command/build_clib.py
index d532762eb..eab08e70f 100644
--- a/setuptools/command/build_clib.py
+++ b/setuptools/command/build_clib.py
@@ -1,17 +1,10 @@
 from ..dist import Distribution
+from ..modified import newer_pairwise_group
 
 import distutils.command.build_clib as orig
 from distutils import log
 from distutils.errors import DistutilsSetupError
 
-try:
-    from distutils._modified import (  # pyright: ignore[reportMissingImports]
-        newer_pairwise_group,
-    )
-except ImportError:
-    # fallback for SETUPTOOLS_USE_DISTUTILS=stdlib
-    from .._distutils._modified import newer_pairwise_group
-
 
 class build_clib(orig.build_clib):
     """
diff --git a/setuptools/config/_apply_pyprojecttoml.py b/setuptools/config/_apply_pyprojecttoml.py
index 23179f354..16fe753b5 100644
--- a/setuptools/config/_apply_pyprojecttoml.py
+++ b/setuptools/config/_apply_pyprojecttoml.py
@@ -217,8 +217,10 @@ def _dependencies(dist: Distribution, val: list, _root_dir):
 
 
 def _optional_dependencies(dist: Distribution, val: dict, _root_dir):
-    existing = getattr(dist, "extras_require", None) or {}
-    dist.extras_require = {**existing, **val}
+    if getattr(dist, "extras_require", None):
+        msg = "`extras_require` overwritten in `pyproject.toml` (optional-dependencies)"
+        SetuptoolsWarning.emit(msg)
+    dist.extras_require = val
 
 
 def _ext_modules(dist: Distribution, val: list[dict]) -> list[Extension]:
diff --git a/setuptools/config/setupcfg.py b/setuptools/config/setupcfg.py
index 4fee109e2..5b4e1e8d9 100644
--- a/setuptools/config/setupcfg.py
+++ b/setuptools/config/setupcfg.py
@@ -304,7 +304,7 @@ class ConfigHandler(Generic[Target]):
             return
 
         simple_setter = functools.partial(target_obj.__setattr__, option_name)
-        setter = getattr(target_obj, 'set_%s' % option_name, simple_setter)
+        setter = getattr(target_obj, f"set_{option_name}", simple_setter)
         setter(parsed)
 
         self.set_options.append(option_name)
@@ -372,8 +372,8 @@ class ConfigHandler(Generic[Target]):
             exclude_directive = 'file:'
             if value.startswith(exclude_directive):
                 raise ValueError(
-                    'Only strings are accepted for the {0} field, '
-                    'files are not accepted'.format(key)
+                    f'Only strings are accepted for the {key} field, '
+                    'files are not accepted'
                 )
             return value
 
@@ -491,12 +491,12 @@ class ConfigHandler(Generic[Target]):
         for section_name, section_options in self.sections.items():
             method_postfix = ''
             if section_name:  # [section.option] variant
-                method_postfix = '_%s' % section_name
+                method_postfix = f"_{section_name}"
 
             section_parser_method: Callable | None = getattr(
                 self,
                 # Dots in section names are translated into dunderscores.
-                ('parse_section%s' % method_postfix).replace('.', '__'),
+                f'parse_section{method_postfix}'.replace('.', '__'),
                 None,
             )
 
@@ -701,10 +701,7 @@ class ConfigOptionsHandler(ConfigHandler["Distribution"]):
         section_data = self._parse_section_to_dict(section_options, self._parse_list)
 
         valid_keys = ['where', 'include', 'exclude']
-
-        find_kwargs = dict([
-            (k, v) for k, v in section_data.items() if k in valid_keys and v
-        ])
+        find_kwargs = {k: v for k, v in section_data.items() if k in valid_keys and v}
 
         where = find_kwargs.get('where')
         if where is not None:
diff --git a/setuptools/modified.py b/setuptools/modified.py
index 245a61580..6ba02fab6 100644
--- a/setuptools/modified.py
+++ b/setuptools/modified.py
@@ -1,8 +1,18 @@
-from ._distutils._modified import (
-    newer,
-    newer_group,
-    newer_pairwise,
-    newer_pairwise_group,
-)
+try:
+    # Ensure a DistutilsError raised by these methods is the same as distutils.errors.DistutilsError
+    from distutils._modified import (
+        newer,
+        newer_group,
+        newer_pairwise,
+        newer_pairwise_group,
+    )
+except ImportError:
+    # fallback for SETUPTOOLS_USE_DISTUTILS=stdlib, because _modified never existed in stdlib
+    from ._distutils._modified import (
+        newer,
+        newer_group,
+        newer_pairwise,
+        newer_pairwise_group,
+    )
 
 __all__ = ['newer', 'newer_pairwise', 'newer_group', 'newer_pairwise_group']


Test patch:
diff --git a/setuptools/tests/config/test_pyprojecttoml_dynamic_deps.py b/setuptools/tests/config/test_pyprojecttoml_dynamic_deps.py
index 37e5234a4..e42f28ffa 100644
--- a/setuptools/tests/config/test_pyprojecttoml_dynamic_deps.py
+++ b/setuptools/tests/config/test_pyprojecttoml_dynamic_deps.py
@@ -5,6 +5,7 @@ from jaraco import path
 
 from setuptools.config.pyprojecttoml import apply_configuration
 from setuptools.dist import Distribution
+from setuptools.warnings import SetuptoolsWarning
 
 
 def test_dynamic_dependencies(tmp_path):
@@ -77,23 +78,32 @@ def test_mixed_dynamic_optional_dependencies(tmp_path):
 
             [tool.setuptools.dynamic.optional-dependencies.images]
             file = ["requirements-images.txt"]
-
-            [build-system]
-            requires = ["setuptools", "wheel"]
-            build-backend = "setuptools.build_meta"
             """
         ),
     }
 
     path.build(files, prefix=tmp_path)
-
-    # Test that the mix-and-match doesn't currently validate.
     pyproject = tmp_path / "pyproject.toml"
     with pytest.raises(ValueError, match="project.optional-dependencies"):
         apply_configuration(Distribution(), pyproject)
 
-    # Explicitly disable the validation and try again, to see that the mix-and-match
-    # result would be correct.
-    dist = Distribution()
-    dist = apply_configuration(dist, pyproject, ignore_option_errors=True)
-    assert dist.extras_require == {"docs": ["sphinx"], "images": ["pillow~=42.0"]}
+
+def test_mixed_extras_require_optional_dependencies(tmp_path):
+    files = {
+        "pyproject.toml": cleandoc(
+            """
+            [project]
+            name = "myproj"
+            version = "1.0"
+            optional-dependencies.docs = ["sphinx"]
+            """
+        ),
+    }
+
+    path.build(files, prefix=tmp_path)
+    pyproject = tmp_path / "pyproject.toml"
+
+    with pytest.warns(SetuptoolsWarning, match=".extras_require. overwritten"):
+        dist = Distribution({"extras_require": {"hello": ["world"]}})
+        dist = apply_configuration(dist, pyproject)
+        assert dist.extras_require == {"docs": ["sphinx"]}
diff --git a/setuptools/tests/test_distutils_adoption.py b/setuptools/tests/test_distutils_adoption.py
index 0b020ba9f..aabfdd283 100644
--- a/setuptools/tests/test_distutils_adoption.py
+++ b/setuptools/tests/test_distutils_adoption.py
@@ -114,6 +114,7 @@ print("success")
 """
 
 
+@pytest.mark.usefixtures("tmpdir_cwd")
 @pytest.mark.parametrize(
     "distutils_version, imported_module",
     [
@@ -125,9 +126,7 @@ print("success")
         ("local", "archive_util"),
     ],
 )
-def test_modules_are_not_duplicated_on_import(
-    distutils_version, imported_module, tmpdir_cwd, venv
-):
+def test_modules_are_not_duplicated_on_import(distutils_version, imported_module, venv):
     env = dict(SETUPTOOLS_USE_DISTUTILS=distutils_version)
     script = ENSURE_IMPORTS_ARE_NOT_DUPLICATED.format(imported_module=imported_module)
     cmd = ['python', '-c', script]
@@ -145,6 +144,7 @@ print("success")
 """
 
 
+@pytest.mark.usefixtures("tmpdir_cwd")
 @pytest.mark.parametrize(
     "distutils_version",
     [
@@ -152,8 +152,47 @@ print("success")
         pytest.param("stdlib", marks=skip_without_stdlib_distutils),
     ],
 )
-def test_log_module_is_not_duplicated_on_import(distutils_version, tmpdir_cwd, venv):
+def test_log_module_is_not_duplicated_on_import(distutils_version, venv):
     env = dict(SETUPTOOLS_USE_DISTUTILS=distutils_version)
     cmd = ['python', '-c', ENSURE_LOG_IMPORT_IS_NOT_DUPLICATED]
     output = venv.run(cmd, env=win_sr(env), **_TEXT_KWARGS).strip()
     assert output == "success"
+
+
+ENSURE_CONSISTENT_ERROR_FROM_MODIFIED_PY = r"""
+from setuptools.modified import newer
+from {imported_module}.errors import DistutilsError
+
+# Can't use pytest.raises in this context
+try:
+    newer("", "")
+except DistutilsError:
+    print("success")
+else:
+    raise AssertionError("Expected to raise")
+"""
+
+
+@pytest.mark.usefixtures("tmpdir_cwd")
+@pytest.mark.parametrize(
+    "distutils_version, imported_module",
+    [
+        ("local", "distutils"),
+        # Unfortunately we still get ._distutils.errors.DistutilsError with SETUPTOOLS_USE_DISTUTILS=stdlib
+        # But that's a deprecated use-case we don't mind not fully supporting in newer code
+        pytest.param(
+            "stdlib", "setuptools._distutils", marks=skip_without_stdlib_distutils
+        ),
+    ],
+)
+def test_consistent_error_from_modified_py(distutils_version, imported_module, venv):
+    env = dict(SETUPTOOLS_USE_DISTUTILS=distutils_version)
+    cmd = [
+        'python',
+        '-c',
+        ENSURE_CONSISTENT_ERROR_FROM_MODIFIED_PY.format(
+            imported_module=imported_module
+        ),
+    ]
+    output = venv.run(cmd, env=win_sr(env), **_TEXT_KWARGS).strip()
+    assert output == "success"