-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Setuptools 68.2+ produces metadata with extra that’s read incorrectly by importlib.metadata #12267
base: main
Are you sure you want to change the base?
Conversation
You've probably already seen this, but just in case you haven't, I expect this is related to the changes in pypa/setuptools#3903. |
This only seems to fail on Python 3.11 (with only setuptools 68.2), but nothing in the setuptools change seems to jump out. There are many things involved though so I’m not sure yet. |
Bisecting brought me to pypa/setuptools@807ab8a, a part of pypa/setuptools#3904 instead. From what I can tell, the commits in 3904 and 3903 are pretty closely coupled though, so the actual logic change might as well happened in that PR you mentioned, but was enabled in that specific commit. No idea. This only happens on 3.11 because that’s the cutoff point pip switches to use impotlib.metadata (instead of This can be validated by running the test (with setuptools 68.2, on Python 3.11+) with environment variable |
Hah, found the culprit. Our importlib.metadata backend sanitises pip/src/pip/_internal/metadata/importlib/_dists.py Lines 210 to 213 in 3e232ce
But the pip/src/pip/_internal/metadata/pkg_resources.py Lines 218 to 219 in 3e232ce
I think some change included in 68.2 changes how extra values are processed and causes the raw extra values in |
So... #12002 which I completely forgot to re-review? :) |
Actually I think that PR plus one more change would be needed; the PR only changes the resolution logic, but the extra would still be read wrong when pip tries to actually install the resolved candidates. |
I just merged a change that should skip the failing test for now so it doesn’t block everyone’s workflow on other stuff. |
Work in progress…