-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Revert removals introduced in v78.0.0
#4911
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+45
−11
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
Postponed removals of deprecated dash-separated and uppercase fields in ``setup.cfg``. | ||
All packages with deprecated configurations are advised to move before 2026. | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,6 @@ | |
|
||
from setuptools.config.setupcfg import ConfigHandler, Target, read_configuration | ||
from setuptools.dist import Distribution, _Distribution | ||
from setuptools.errors import InvalidConfigError | ||
from setuptools.warnings import SetuptoolsDeprecationWarning | ||
|
||
from ..textwrap import DALS | ||
|
@@ -423,7 +422,7 @@ def test_not_utf8(self, tmpdir): | |
pass | ||
|
||
@pytest.mark.parametrize( | ||
("error_msg", "config"), | ||
("error_msg", "config", "invalid"), | ||
[ | ||
( | ||
"Invalid dash-separated key 'author-email' in 'metadata' (setup.cfg)", | ||
|
@@ -434,6 +433,7 @@ def test_not_utf8(self, tmpdir): | |
maintainer_email = [email protected] | ||
""" | ||
), | ||
{"author-email": "[email protected]"}, | ||
), | ||
( | ||
"Invalid uppercase key 'Name' in 'metadata' (setup.cfg)", | ||
|
@@ -444,14 +444,25 @@ def test_not_utf8(self, tmpdir): | |
description = Some description | ||
""" | ||
), | ||
{"Name": "foo"}, | ||
), | ||
], | ||
) | ||
def test_invalid_options_previously_deprecated(self, tmpdir, error_msg, config): | ||
# this test and related methods can be removed when no longer needed | ||
def test_invalid_options_previously_deprecated( | ||
self, tmpdir, error_msg, config, invalid | ||
): | ||
# This test and related methods can be removed when no longer needed. | ||
# Deprecation postponed due to push-back from the community in | ||
# https://github.com/pypa/setuptools/issues/4910 | ||
fake_env(tmpdir, config) | ||
with pytest.raises(InvalidConfigError, match=re.escape(error_msg)): | ||
get_dist(tmpdir).__enter__() | ||
with pytest.warns(SetuptoolsDeprecationWarning, match=re.escape(error_msg)): | ||
dist = get_dist(tmpdir).__enter__() | ||
|
||
tmpdir.join('setup.cfg').remove() | ||
|
||
for field, value in invalid.items(): | ||
attr = field.replace("-", "_").lower() | ||
assert getattr(dist.metadata, attr) == value | ||
|
||
|
||
class TestOptions: | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! But when you eventually do restore this behavior, could we please get the option to opt out?
Some packages are simply old & abandoned, and there's not much that consumers can do when trying to install a third-party package (as the many comments on #4910 would indicate!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The option is to pin setuptools via the
PIP_CONSTRAINT
environment variable (or the equivalent in UV).Please note that this only affects packages that do not publish a
wheel
to PyPI or other package index. Unless it is a module with native binary extensions, the recommendation of publishing wheels is a bit old already.All things considered this is likely a symptom of something bigger, more problematic, and the package you depend upon has been on borrowed time for a while. If you depend on packages that do not receive any support you might be subject to all sorts of risks including security risks. In that case there are a couple of things that can be done during the extended period:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That may well all be true. But why not show a loud warning for a while before removing totally? The reality is that we try to vet our packages carefully, but still woke up to Monday morning with lots of broken build processes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the issue does also affect all Linux distributions and downstream vendors that rebuild wheels from sources. Linux distributions like Debian, Fedora, and Gentoo do not use wheels. They take sdists and rebuild wheels.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…and given the scale of this issue, it will mean we won't be able upgrade setuptools for months. Or to put it more precisely, once again the burden of fixing all the fallout will be on downstreams, who spend their whole days putting out fires for free.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I completely sympathize with the goal of getting all dependencies to do the right thing and being able to simplify the setup tools code. But I agree with @danielcarcich. From a robustness principle, this seems very unfortunate.
While this is easy to fix in smaller repos, many large orgs deal with a ton of unfortunately old and unmaintained packages. Yes that's a fault of them (I'm in one), but here we are.
I hope to see this reverted and thanks to @abravalheri for starting this pull-request preemptively while having the discussion. (apologies is this conversation should be in #4910 instead)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A build tool needs to aim to interact with as many versions of libraries from the ecosystem as possible, not just the latest versions that adhere to modern best practices. Thanks for all the hard work maintaining a core utility and for responding quickly to the response today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I say this as someone who has contributed a patch to a popular community package to remediate a CVE, and maintained an internal fork waiting for a new release. My PR was accepted, but no release yet for over a year. Ok, not that big of a contribution, but you can't accuse me of never having contributed.
It will be 1. undifferentiated heavy lifting and 2. utter chaos to maintain internal forks or coordinate community forks for every single transitive dependency in our codebase, especially as others also have the dependencies as well.
If Setuptools does this again in 2026, it will be a repeat of this. How many warnings went unheeded this time? What are the chances those warnings will be heeded in the year to come? It will take years, not months, to untangle.
If the plan is to try this again in 2026, the package to be forked will be
setuptools
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While the contributions of Setuptools maintainers are very appreciated, I think it's important to realize that the Setuptools API has become a load-bearing component of the distribution infrastructure of many legacy Python packages that can no longer be easily updated, yet are widely needed for the foreseeable future. As such, I think it's simply not possible to introduce a change like this one without unbearable levels of disruption. A more finessed approach (like selectively treating these errors as warnings based on signals like the age of the package release, etc.) is necessary. And also a rethink of how to restrict and compartmentalize the packaging API surface so build dependencies for old packages can be bundled with them for future issues like these. PEP 517 takes us most of the way to where we want to be, but more work is needed I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concur. Yes, some projects will fix the issue in the meantime (and some did in response to this). But there are still a fair number of basically-unmaintained packages out there, and pushing this back by a year won't solve that.
Agree. I'm not saying that's how it should be, but that's a potential outcome if this still goes forward.
I know from personal experience how stressful and thankless a job it can be maintaining open source tools that hundreds of thousands or millions of devs depend on.
One of the key responsibilities from that is knowing that if you introduce a breaking change -- no matter HOW well you pre-communicate it -- you're potentially breaking tons of people, and some of them will simply settle on an old version and refuse to upgrade from then on. If enough of them are upset the project gets forked. The usual escape valve to prevent this is offering an override option to restore the previous behavior. It sucks, it can be maddening being so restricted in the ability to move beyond past technical/feature mistakes. But you learn to pick and choose your battles, and something like underscores-vs-hyphens usually isn't worth the fight.
TL;DR: "With great power comes great responsibility" is the mantra for critical open source projects -- writ large in all-caps bold.