Skip to content
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

[FR] More gradual breakage of setuptools.command.test #4520

Closed
1 task done
hauntsaninja opened this issue Jul 29, 2024 · 17 comments · Fixed by #4522
Closed
1 task done

[FR] More gradual breakage of setuptools.command.test #4520

hauntsaninja opened this issue Jul 29, 2024 · 17 comments · Fixed by #4522
Assignees

Comments

@hauntsaninja
Copy link
Contributor

hauntsaninja commented Jul 29, 2024

What's the problem this feature will solve?

First, let me just say that I do not envy the position of maintaining setuptools, and if this issue is at all burdensome, please ignore it :-)

Regarding #4519, in the event that setuptools chooses to revert something (looks like issue is already top ten all time by emoji count), I'm wondering if there are creative ways to introduce this breakage more slowly

Describe the solution you'd like

Currently, anyone trying to install a package that has a test command breaks. This is unfortunate, because people may not even be using that test command (in fact, hopefully not, since that's been messaged for years), and so no one may even have have seen the warning.

Instead, we could maybe follow something like the following sequence of steps:

  1. Add a deprecation warning in setuptools.command.test.__getattr__
  2. Make run in setuptools.command.test.test raise an error

This would greatly increase the number of people to whom the warning is visible.
It would also accomplish the goal of killing python setup.py test, while not yet breaking installs from those package versions.

Code of Conduct

  • I agree to follow the PSF Code of Conduct
@mkielar
Copy link

mkielar commented Jul 29, 2024

I'm just a regular Dev, having the same "happy morning" as probably 1/3 of the programming world, and in no way affiliated with setuptools. But the test command has been deprecated since 41.5.0 which was released almost 5 years ago.

I'd say that that was gradual enough ;)

@chrisirhc
Copy link

chrisirhc commented Jul 29, 2024

This is unfortunate, because people may not even be using that test command (in fact, hopefully not, since that's been messaged for years), and so no one may even have have seen the warning.

This is likely the crux of the issue. A breaking change would need to at least message at the point that is about to break.
In this case, if the plan was to break install on these packages, then the message should've been on the "install" for at least one major version.

This gives folks time to find workarounds, and also provides some time for the change to be rolled out with known workarounds.
However, I'm seeing people on #4519 scrambling to find workarounds. That's an indicator that the change wasn't gradual to these folks.

This isn't a ding on the contributors, I'm just stating my observation in the issues.

Add a deprecation warning in setuptools.command.test.getattr

Just a note on this, you could add a link to an open issue on Github so users can contribute workarounds there.

@purificant
Copy link

setuptools is core package in the python ecosystem, unfortunately now there are dozens of popular packages that are failing to install, there needs to be a better way to handle this deprecation end to end and not just within the confines of the setuptools codebase

@abravalheri
Copy link
Contributor

abravalheri commented Jul 29, 2024

This is likely the crux of the issue. A breaking change would need to at least message at the point that is about to break.
In this case, if the plan was to break install on these packages, then the message should've been on the "install" for at least one major version.

Hi @chrisirhc, please note that this message you are referring to was there for 5 years.

@manugarri
Copy link

setuptools is core package in the python ecosystem, unfortunately now there are dozens of popular packages that are failing to install, there needs to be a better way to handle this deprecation end to end and not just within the confines of the setuptools codebase

Exactly this, can we realistically expect the 100s of package maintainers (volunteers most of them) to be up to date on the state of a core build tool like setuptools? For a non trivial chunk of them, setuptools is just some copypaste template you use to turn your source code into a package.

@max-wittig
Copy link

Hi @chrisirhc, please note that this message you are referring to was there for 5 years.

@abravalheri But nobody know that it was planned to be removed so was it really deprecated, if nobody knew?

@deceze
Copy link

deceze commented Jul 29, 2024

please note that this message you are referring to was there for 5 years.

As a user of setuptools/pip/poetry who uses this toolchain to install packages I need, this is the first time I've ever seen anything about this deprecation, and I've gotten to know about it by deploy tasks failing. This can't be the correct way to spread the word.

Yes, it has been the failure of many maintainers and/or developers sticking with old packages etc., but that's simply how this works in the real world. We oftentimes have to deal with old packages, end of story. setuptools is so deeply ingrained in so much of the Python ecosystem, removing anything from it must be treated with much much more caution than this has been. "There's been a message there nobody saw" isn't cutting it.

@leorochael
Copy link
Contributor

leorochael commented Jul 29, 2024

It's perfectly reasonable that a package should rely on a specific, not-the-most-recent, version of setuptools as a build-backend.

And it's perfectly reasonable that major versions of setuptools could cause older versions of packages that depend on older versions of setuptools to fail to build.

I agree with this bug report that the deprecation should have been more visible, but I also believe there is an unrealistic assumption that newer versions of setuptools will always be able to build older versions of packages that have setuptools backend.

And unfortunately that assumption has been tacitly supported by Setuptools documentation itself, in failing to call attention to the fact that build backends should have an upper bound version pin.

To this end, I've reported #4521.

I hope that this issue will move developers of setuptools depending packages to remember to upper-pin the major versions of their build-backends.

@sevdog
Copy link

sevdog commented Jul 29, 2024

The test command was deprecated 5 years ago, in the mean time the only "warning" for whoever was using it was a log in a single method:

def run(self):
self.announce(
"WARNING: Testing via this command is deprecated and will be "
"removed in a future version. Users looking for a generic test "
"entry point independent of test runner are encouraged to use "
"tox.",
log.WARN,
)

There were no other warnings to inform a user which had left a legacy element inside its own codebase telling "ehy, you are using a deprecated element, remove it".

A warning which is only displayed if a class-method is run was not enough to inform users.

@AvnerCohen
Copy link

AvnerCohen commented Jul 29, 2024

In an effort to find the right balance for such a deeply ingrained infrastructure, and from some of the comments, I suspect some people have more control over build flow vs others who have less, I feel that "deprecation" can be a range.

It can be a breaking change that causes hard failure as we are seeing here, but it can also be a stub NoOp or even a warning, but one that is not causing any harm to existing setup.

This will mean zero maintenance is needed (the goal for deprecation in the 1st place) without the extra breakage noise.

@jaraco
Copy link
Member

jaraco commented Jul 29, 2024

Instead, we could maybe follow something like the following sequence of steps:

1. Add a deprecation warning in `setuptools.command.test.__getattr__`

2. Make `run` in `setuptools.command.test.test` raise an error

This sounds reasonable.

I'm trying to decide how to approach. One option, instead of reverting the changes, simply restore a minimum interface to address these concerns. I think that would have the effect described above.

I plan to scan through other issues and PRs first, and then pursue that option.

@jaraco jaraco self-assigned this Jul 29, 2024
@jaraco jaraco removed the Needs Triage Issues that need to be evaluated for severity and status. label Jul 29, 2024
@jaraco
Copy link
Member

jaraco commented Jul 29, 2024

For reference, the code was removed in #4458.

@jaraco
Copy link
Member

jaraco commented Jul 29, 2024

Setuptools does run "integration" tests on each tagged release against these packages. Those tests did pass (after one failure in brotli).

Perhaps celery or requests could be added to those integration tests. Since it's a bit of a shotgun approach, the best we can do is hope that the sample of packages we've selected catch any missed expectations. In this case, they did not. I'd be fine with adding celery and requests to that list.

@marksteward
Copy link

marksteward commented Jul 29, 2024

Oops sorry, deleted my comment after realising it was sort of redundant with comments elsewhere. Thanks for the pointer, will have a play.

@jaraco
Copy link
Member

jaraco commented Jul 29, 2024

I've already confirmed that adding requests and celery fail on 72.x but pass on #4522, so I'm going to include that in the PR.

@jaraco jaraco closed this as completed in c437aaa Jul 29, 2024
gentoo-bot pushed a commit to gentoo/gentoo that referenced this issue Jul 29, 2024
In 72.0.0 the test command was finally removed as has been threatened
for a while. It turns out that many people still had referenced it in
their setup.py even if they didn't actually use it, so simply building a
wheel then failed -- the only deprecation warning had been if you
actually attempted to use `setup.py test`.

Setuptools 72.1.0 adds a deprecation shim to allow those packages to
still build wheels successfully, to give projects time and warning that
they need to remove this last remaining trace.

We drop the old version too, as we don't want users to hit the version
that breaks wheel building.

Bug: pypa/setuptools#4520
Bug: pypa/setuptools#4522
Signed-off-by: Eli Schwartz <[email protected]>
@lazka
Copy link
Contributor

lazka commented Jul 29, 2024

Here are the packages in the pypi top 2500 which import the test command:

$ ./download_pypi_top.py top2500 2500
$ ./search_pypi_top.py top2500 "setuptools.command.test" -q | grep import
top2500/clickclick-20.10.2.tar.gz: clickclick-20.10.2/setup.py: from setuptools.command.test import test as TestCommand
top2500/amqp-5.2.0.tar.gz: amqp-5.2.0/setup.py: import setuptools.command.test
top2500/autobahn-23.6.2.tar.gz: autobahn-23.6.2/setup.py: from setuptools.command.test import test as test_command
top2500/boxsdk-3.11.0.tar.gz: boxsdk-3.11.0/setup.py: from setuptools.command.test import test as TestCommand
top2500/dogpile.cache-1.3.3.tar.gz: dogpile.cache-1.3.3/setup.py: from setuptools.command.test import test as TestCommand
top2500/fbprophet-0.7.1.tar.gz: fbprophet-0.7.1/setup.py: from setuptools.command.test import test as test_command
top2500/ffmpy-0.3.2.tar.gz: ffmpy-0.3.2/setup.py: from setuptools.command.test import test as TestCommand  # noqa
top2500/diskcache-5.6.3.tar.gz: diskcache-5.6.3/setup.py: from setuptools.command.test import test as TestCommand
top2500/flaky-3.8.1.tar.gz: flaky-3.8.1/setup.py: from setuptools.command.test import test as TestCommand
top2500/django-celery-beat-2.6.0.tar.gz: django-celery-beat-2.6.0/setup.py: import setuptools.command.test
top2500/furl-2.1.3.tar.gz: furl-2.1.3/setup.py: from setuptools.command.test import test as TestCommand
top2500/django_celery_results-2.5.1.tar.gz: django_celery_results-2.5.1/setup.py: import setuptools.command.test
top2500/gender-guesser-0.4.0.tar.gz: gender-guesser-0.4.0/setup.py: from setuptools.command.test import test as TestCommand
top2500/ibm-cloud-sdk-core-3.20.3.tar.gz: ibm-cloud-sdk-core-3.20.3/setup.py: from setuptools.command.test import test as TestCommand
top2500/intervaltree-3.1.0.tar.gz: intervaltree-3.1.0/setup.py: from setuptools.command.test import test as TestCommand
top2500/httplib2-0.22.0.tar.gz: httplib2-0.22.0/setup.py: import setuptools.command.test
top2500/graphene-3.3.tar.gz: graphene-3.3/setup.py: from setuptools.command.test import test as TestCommand
top2500/elastic-apm-6.22.3.tar.gz: elastic-apm-6.22.3/setup.py: from setuptools.command.test import test as TestCommand
top2500/JPype1-1.5.0.tar.gz: JPype1-1.5.0/setupext/pytester.py: from setuptools.command.test import test as TestCommand
top2500/mongoengine-0.28.2.tar.gz: mongoengine-0.28.2/setup.py: from setuptools.command.test import test as TestCommand
top2500/pdfkit-1.0.0.tar.gz: pdfkit-1.0.0/setup.py: from setuptools.command.test import test as TestCommand
top2500/orderedmultidict-1.0.1.tar.gz: orderedmultidict-1.0.1/setup.py: from setuptools.command.test import test as TestCommand
top2500/pep8-naming-0.14.1.tar.gz: pep8-naming-0.14.1/setup.py: from setuptools.command.test import test as TestCommand
top2500/moviepy-1.0.3.tar.gz: moviepy-1.0.3/setup.py: from setuptools.command.test import test as TestCommand
top2500/PyHive-0.7.0.tar.gz: PyHive-0.7.0/setup.py: from setuptools.command.test import test as TestCommand
top2500/python-consul-1.1.0.tar.gz: python-consul-1.1.0/setup.py: from setuptools.command.test import test as TestCommand
top2500/python-dateutil-2.9.0.post0.tar.gz: python-dateutil-2.9.0.post0/setup.py: from setuptools.command.test import test as TestCommand
top2500/requests-2.32.3.tar.gz: requests-2.32.3/setup.py: from setuptools.command.test import test as TestCommand
top2500/responses-0.25.3.tar.gz: responses-0.25.3/setup.py: from setuptools.command.test import test as TestCommand
top2500/raven-6.10.0.tar.gz: raven-6.10.0/setup.py: from setuptools.command.test import test as TestCommand
top2500/sortedcontainers-2.4.0.tar.gz: sortedcontainers-2.4.0/setup.py: from setuptools.command.test import test as TestCommand
top2500/soundfile-0.12.1.tar.gz: soundfile-0.12.1/setup.py: from setuptools.command.test import test as TestCommand
top2500/suds-jurko-0.6.tar.bz2: suds-jurko-0.6/setup.py: import setuptools.command.test
top2500/smartsheet-python-sdk-3.0.3.tar.gz: smartsheet-python-sdk-3.0.3/setup.py: from setuptools.command.test import test as TestCommand
top2500/sanic-24.6.0.tar.gz: sanic-24.6.0/setup.py: from setuptools.command.test import test as TestCommand
top2500/node-semver-0.9.0.tar.gz: node-semver-0.9.0/setup.py: from setuptools.command.test import test as TestCommand
top2500/nose-1.3.7.tar.gz: nose-1.3.7/setup3lib.py: import setuptools.command.test
top2500/pymssql-2.3.0.tar.gz: pymssql-2.3.0/setup.py: from setuptools.command.test import test as TestCommand
top2500/pytest-runner-6.0.1.tar.gz: pytest-runner-6.0.1/ptr/__init__.py: import setuptools.command.test as orig
top2500/vine-5.1.0.tar.gz: vine-5.1.0/setup.py: import setuptools.command.test
top2500/virtualenv-clone-0.5.7.tar.gz: virtualenv-clone-0.5.7/setup.py: from setuptools.command.test import test as TestCommand
top2500/Wand-0.6.13.tar.gz: Wand-0.6.13/setup.py: from setuptools.command.test import test
top2500/celery-5.4.0.tar.gz: celery-5.4.0/setup.py: import setuptools.command.test
top2500/poetry-1.8.3.tar.gz: poetry-1.8.3/tests/utils/fixtures/setups/requests/setup.py: from setuptools.command.test import test as TestCommand
top2500/poetry-1.8.3.tar.gz: poetry-1.8.3/tests/utils/fixtures/setups/sqlalchemy/setup.py: from setuptools.command.test import test as TestCommand
top2500/vcrpy-6.0.1.tar.gz: vcrpy-6.0.1/setup.py: from setuptools.command.test import test as TestCommand

Doing such checks would probably help to estimate the fallout from future setuptools removals.

@zahlman
Copy link

zahlman commented Jul 31, 2024

This is likely the crux of the issue. A breaking change would need to at least message at the point that is about to break. In this case, if the plan was to break install on these packages, then the message should've been on the "install" for at least one major version.

It wasn't planned to break installation. It's a consequence of the fact that setup.py scripts are written to import setuptools (because they were designed to be run like python setup.py, even though that's now deprecated) and then access whatever sub-packages and modules. The code for using Setuptools to test (by implementing a TestCommand) is intertwined with the code needed for the setup() call that empowers Setuptools to build the package, which in turn is part of the process of installing them.

All of this is a consequence of the fundamental design of Setuptools, in turn adapting the design of Distutils, which goes back decades to well before there were standards like PEP 517/518/621, or wheels, or even Pip.

There are many ways that developers can avoid or mitigate the problem. In a significant fraction of cases, they could drop setup.py completely and use pyproject.toml (or even just setup.cfg) to give the necessary project metadata. In almost all of those cases, too, they could just provide a wheel, which is better for users when possible, and is straightforward for projects that use only Python for their own code (they may even have dependencies that in turn use extensions).

Ideally, developers would get a deprecation warning from the import of setuptools itself, when they try to run python setup.py. I think the logic is already in place to detect when setup.py is being run indirectly by Setuptools (for backwards compatibility, so that it can check what setup calls are made and then do the main part of the work). So it would just have to emit a warning when this isn't happening.

However, there's nothing Setuptools can do about developers who:

  • are using an outdated version of Setuptools locally for developing the project, either because nothing has motivated them to upgrade or because they heard about the PEP 517 etc. changes and didn't want to give up the old workflow

  • have actually abandoned the project years ago, and don't know that there are still many people using it

Nothing has forced these developers to upgrade locally to a Setuptools version that would produce the warning. So even in principle they can't be expected to have seen it. End users, on the other hand, routinely get the latest version of Setuptools pushed on them by Pip. Unfortunately, in Setuptools 72.0.0, the code intended to enable a deprecated test workflow for developers, causes an install failure for users - a serious violation of the principle of least surprise.

However, I'm seeing people on #4519 scrambling to find workarounds. That's an indicator that the change wasn't gradual to these folks.

It's developers who have had years to do something about the problem, but end users who were scrambling. There's really nothing Setuptools could do to make the change appear "gradual" to end users who are installing something via Pip, without cooperation from the developers of the other packages that Pip is installing.

And as long as installation from an sdist is supported, Pip can't really do anything about it either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.