Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog.d/3206.change.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Fixed behaviour when both ``install_requires`` (in ``setup.py``) and
``dependencies`` (in ``pyproject.toml``) are specified.
The configuration in ``pyproject.toml`` will take precedence over ``setup.py``
(in accordance with PEP 621). A warning was added to inform users.
6 changes: 4 additions & 2 deletions setuptools/config/_apply_pyprojecttoml.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,10 @@ def _python_requires(dist: "Distribution", val: dict, _root_dir):


def _dependencies(dist: "Distribution", val: list, _root_dir):
existing = getattr(dist, "install_requires", [])
_set_config(dist, "install_requires", existing + val)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I'm reading PEP 621 wrong, couldn't dependencies also be one of the keys that can be specified as dynamic? Wouldn't this always overwrite?

Copy link
Contributor Author

@abravalheri abravalheri May 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @nullableVoidPtr thank you very much for the review. It is very appreaciated!

Yes, that is correct, dynamic can contain dependencies. But in that case the dependencies key cannot be present in the project table (according to PEP 621, a key can only be specified either statically or dynamically, never both -- this should enforced during the validation phase).

In turn, this means that the _dependencies function will not be called (line 66 of setuptools/config/_apply_pyprojecttoml.py). Right?

Copy link
Contributor

@nullableVoidPtr nullableVoidPtr May 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yep, all good; wasn't looking at the bigger picture 😅 LGTM

if getattr(dist, "install_requires", []):
msg = "`install_requires` overwritten in `pyproject.toml` (dependencies)"
warnings.warn(msg)
_set_config(dist, "install_requires", val)


def _optional_dependencies(dist: "Distribution", val: dict, _root_dir):
Expand Down
9 changes: 9 additions & 0 deletions setuptools/tests/config/test_apply_pyprojecttoml.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,15 @@ def test_listed_in_dynamic(self, tmp_path, attr, field, value):
dist_value = _some_attrgetter(f"metadata.{attr}", attr)(dist)
assert dist_value == value

def test_warning_overwritten_dependencies(self, tmp_path):
src = "[project]\nname='pkg'\nversion='0.1'\ndependencies=['click']\n"
pyproject = tmp_path / "pyproject.toml"
pyproject.write_text(src, encoding="utf-8")
dist = makedist(tmp_path, install_requires=["wheel"])
with pytest.warns(match="`install_requires` overwritten"):
dist = pyprojecttoml.apply_configuration(dist, pyproject)
assert "wheel" not in dist.install_requires

def test_optional_dependencies_dont_remove_env_markers(self, tmp_path):
"""
Internally setuptools converts dependencies with markers to "extras".
Expand Down