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

FIX: Remove deprecation warning from vendored pkg_resources #12660

Merged
merged 3 commits into from
Jul 9, 2024

Conversation

effigies
Copy link
Contributor

Leaves a comment to flag the change for the next time setuptools is vendored.

Fixes gh-12243.

@effigies
Copy link
Contributor Author

Well, this turned into a hairier issue than I realized.

Copy link
Member

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

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

Hey thanks for your interest in improving pip!

Making changes exclusively to the vendored code is not permitted. The change needs to be carried as a patch as per pip's vendoring policy.

Also, CI is broken project-wide due to changes to GHA macOS runners. We're working on addressing it, don't worry about all of the red1 right now :)

Footnotes

  1. Well, actually, the vendoring job is failing due to the lack of a patch.

@effigies effigies force-pushed the fix-gh-12243 branch 2 times, most recently from 9842a28 to aa37b29 Compare April 29, 2024 16:52
@effigies
Copy link
Contributor Author

@ichard26 Thanks for the pointer! Switched to a patch. With this diff in a patch, the comment seems less necessary. I can pare it down or even remove it. LMK what makes sense to you.

@ichard26 ichard26 self-requested a review April 30, 2024 20:46
Copy link
Member

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

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

(deleted commentary on PYTHONWARNINGS=error)

Since the warning is now patched out, we could also remove the filter that's currently suppressing this warning:

# Suppress the pkg_resources deprecation warning
# Note - we use a module of .*pkg_resources to cover
# the normal case (pip._vendor.pkg_resources) and the
# devendored case (a bare pkg_resources)
warnings.filterwarnings(
action="ignore", category=DeprecationWarning, module=".*pkg_resources"
)

@effigies
Copy link
Contributor Author

effigies commented May 2, 2024

From this thread #11997 (comment), one goal of this filter was to play nicely if someone devendors:

Ideally, I'd have made the filter just on pip._vendor.pkg_resources, but that wouldn't have handled the devendoring case. I could have had 2 separate filters, or even left it for whoever does the devendoring to sort out, but that seems unnecessarily unfriendly.

I will defer to you if you still want it removed, but I had left it in intentionally, after reading that.

@pfmoore
Copy link
Member

pfmoore commented May 2, 2024

I would be happy to remove the filter, on the basis that anyone devendoring should be reviewing our patches and making arrangements to cover whatever fixes they make. But I'll be honest, I'm guessing what's reasonable here - ideally someone who actually devendors would be the best person to comment, but I've no idea how we could get such input.

@ichard26
Copy link
Member

ichard26 commented May 2, 2024

I don't want to claim any authority as I'm still new here, but my 2c is that if we don't have any expert opinions available, we may as well stick with the least disruptive change. The warning filter is not a maintenance burden and we don't need to potentially impact downstream unnecessarily. AFAIU, pkg_resources and associated code (like this) can be removed from the codebase entirely once Python 3.10 and eggs are unsupported.

@effigies
Copy link
Contributor Author

effigies commented May 3, 2024

Alright. It sounds like there's nothing for me to do, but please let me know if I'm misunderstanding and there's something left.

Copy link
Member

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

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

Thanks!

@ichard26 ichard26 added this to the 24.1 milestone May 5, 2024
@pradyunsg pradyunsg removed this from the 24.1 milestone May 6, 2024
@pradyunsg
Copy link
Member

I realise this has multiple approvals but I'm gonna say this isn't going to be added to 24.1 in the last minute, sorry.

@pradyunsg pradyunsg added this to the 24.2 milestone May 6, 2024
@pradyunsg pradyunsg merged commit bf0e5f2 into pypa:main Jul 9, 2024
24 checks passed
@pradyunsg
Copy link
Member

Thanks @effigies! ^.^

@effigies effigies deleted the fix-gh-12243 branch July 9, 2024 23:42
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suppress deprecation warnings in vendored projects during normal operation
5 participants