Skip to content

Remove additional detection of installed egg distributions when using the imporlib.metadata backend #12308

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Remove additional detection of installed egg distributions when using the imporlib.metadata backend #12308

wants to merge 5 commits into from
+27 −71

Conversation

sbidoul
Copy link
Member

@sbidoul sbidoul commented Oct 1, 2023 •
edited
Loading

I could not find the deprecation issue for this, but this was scheduled for removal in 23.3.

Towards #12330

@sbidoul sbidoul added this to the 23.3 milestone Oct 1, 2023
@sbidoul
Copy link
Member Author

sbidoul commented Oct 1, 2023

This is also not super urgent and could be deferred.

pradyunsg
pradyunsg approved these changes Oct 1, 2023
View reviewed changes
Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

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

One news fragment change, but rest seems fine to me!

@sbidoul
Copy link
Member Author

sbidoul commented Oct 1, 2023

Thinking further about this, I'd propose to defer the removal for one year at least because

  • we are not nearly close to removing pkg_resources
  • the removal of the setup.py install code path was not so long ago and users may unknowingly have installed eggs if they did not have wheel installed, which was also the default for python -m venv
  • if we remove discovery, uninstallation must be done manually and this may generate a lot of support issues

@uranusjr
Copy link
Member

uranusjr commented Oct 2, 2023

It’s also only until recently related issues started popping up (#12297 and IIRC there’s another one I closed). Either distributions take a lot of time to upgrade to Python 3.11, or people take a lot of time to upgrade to it. In either case, more time is likely a good idea since removing the warning would simply cause dangling files.

@sbidoul sbidoul marked this pull request as draft October 2, 2023 20:37
@sbidoul sbidoul modified the milestones: 23.3, 24.3 Oct 8, 2023
@sbidoul sbidoul mentioned this pull request Oct 8, 2023
Merged
@ichard26 ichard26 mentioned this pull request Dec 6, 2024
Closed
@sbidoul sbidoul modified the milestones: 25.0, 25.1 Dec 7, 2024
@sbidoul
Copy link
Member Author

sbidoul commented Dec 7, 2024 •
edited
Loading

The related issue has gone_in="25.1", so I changed the milestone accordingly

@ichard26 ichard26 mentioned this pull request Mar 7, 2025
Open
@pfmoore
Copy link
Member

pfmoore commented Apr 1, 2025

This was discussed briefly here. If we don't plan on removing support for 25.1, then we need to do the following:

  1. Update the deprecation notice (this might need a little work, as we're not just changing the release, but basing the removal on what Python version we support).
  2. Change or just remove the milestone for this PR.

Alternatively, we need to resolve the conflicts here and merge it.

I don't mind either way, but some action needs to happen in the next couple of weeks.

The default, if no-one picks this up, is that I'll update the deprecation to say release 25.2, and we can have this same discussion in another 3 months 🙂

@sbidoul sbidoul modified the milestones: 25.1, 26.3 Apr 5, 2025
@sbidoul
Copy link
Member Author

sbidoul commented Apr 5, 2025

I postponed this to 26.3. Support for detecting eggs distributions has been disabled for Python >= 3.14 and the plan is to remove it for other versions when we stop supporting Python 3.10, i.e. not earlier than 26.3.

@sbidoul sbidoul added the type: deprecation Related to deprecation / removal. label Apr 6, 2025
@sbidoul sbidoul changed the title Remove support for installed eggs distributions Remove support for additional detection of installed eggs distributions when using the imporlib.metadata backend Apr 6, 2025
@sbidoul sbidoul modified the milestones: 26.3, 25.1 Apr 6, 2025
@sbidoul
Copy link
Member Author

sbidoul commented Apr 6, 2025

Getting this back into 25.1, following the problems discovered in #12330 (comment) and following comments. Sorry for the noise.

@sbidoul sbidoul marked this pull request as ready for review April 6, 2025 22:27
@sbidoul
Copy link
Member Author

sbidoul commented Apr 6, 2025

With a hack to correctly uninstall zipped egg distributions, all tests pass.

sbidoul
sbidoul commented Apr 6, 2025
View reviewed changes
@ichard26 ichard26 mentioned this pull request Apr 7, 2025
Closed
@ichard26
Copy link
Member

I'll review this tomorrow, but I'm generally supportive of killing off eggs.

@sbidoul sbidoul changed the title Remove support for additional detection of installed eggs distributions when using the imporlib.metadata backend Remove additional detection of installed egg distributions when using the imporlib.metadata backend Apr 12, 2025
@sbidoul
Copy link
Member Author

sbidoul commented Apr 12, 2025

I still need to update the news entry. I think I will make it a bug fix, which is we stop listing installed eggs multiple times.

I'll look a little bit more, but so far I did not find any feature removed by this PR.

@sbidoul
Remove redundant detection of installed eggs
9df247e 
The importlib.metadata backend will discover them as long as they
are in sys.path, which setup.py install and easy_install do by adding them
to easy-install.pth.
sbidoul added 4 commits April 12, 2025 12:26
@sbidoul
Make double uninstall test independent of order
bd7a6db 
Discovery order depends on the metadata backend.

For the importlib backend, it has changed following the previous commit,
because now the .egg-link file is found while inspecting site-package,
and the .egg directory is found after because it comes after in sys.path.

Before the dedictated .egg detection logic was triggered while inspecting
site package and found it, before looking for .egg-link.
@sbidoul
Fix uninstallation of zipped eggs
a2c806e
@sbidoul
Rename find_linked to find_legacy_editables
bfc989a
@sbidoul
Clarify what the removal of the pkg_ressources backend implies
d2d7c69 
This does not actually imply that eggs are not detected.
When they are in sys.path they continue to be detected.
sbidoul
sbidoul commented Apr 12, 2025
View reviewed changes
tests/functional/test_uninstall.py
@pytest.mark.skipif(
"sys.version_info >= (3, 14)",
reason="Uninstall of .egg distributions not supported in Python 3.14+",
)
Copy link
Member Author

@sbidoul sbidoul Apr 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I initially thought this test failure was due to eggs not being detected with the importlib.metadata backend, but it turns out it's just a matter of the uninstallation order depending on the backend.

@sbidoul
Copy link
Member Author

sbidoul commented Apr 12, 2025

This should be ready, now. I recommend reviewing individual commits for easier understanding.

ichard26

This comment was marked as outdated.

Sign in to view

@sbidoul sbidoul mentioned this pull request Apr 14, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ichard26 ichard26 ichard26 left review comments

@pradyunsg pradyunsg pradyunsg approved these changes

Assignees
No one assigned
Labels
bot:chronographer:provided type: deprecation Related to deprecation / removal.
Projects
None yet
Milestone
25.1
Development

Successfully merging this pull request may close these issues.
5 participants
@sbidoul @uranusjr @pfmoore @ichard26 @pradyunsg