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

PathDistribution normalized name ignores name components after first "-" #459

Closed
itamaro opened this issue May 20, 2023 · 6 comments
Closed

Comments

@itamaro
Copy link

itamaro commented May 20, 2023

I came across this issue when switching from python 3.10.5 to 3.10.9. It manifested as flake8 failure due to missing entry points when flake8-plugin-utils is also installed.

minimal repro showing the issue:

# python 3.10.9
>>> from importlib.metadata import PathDistribution
>>> PathDistribution("flake8.egg-info")._normalized_name
'flake8'
>>> PathDistribution("flake8-plugin-utils.egg-info")._normalized_name
'flake8'
# python 3.10.5
>>> from importlib.metadata import PathDistribution
>>> PathDistribution("flake8.egg-info")._normalized_name
'flake8.egg'
>>> PathDistribution("flake8-plugin-utils.egg-info")._normalized_name
'flake8'

the fact that both normalized names are identical in 3.10.9 makes entry_points() miss entry points data from flake8.

generally, based on PyPA name normalization spec, I would expect the normalized name for flake8-plugin-utils to be flake8-plugin-utils, not flake8.

@jaraco
Copy link
Member

jaraco commented Jun 19, 2023

I believe flake8-plugin-utils is not correctly normalized for a name in the file system. While you're right that the PEP 503 normalization does prefer - to _, this separator is inadequate for prefixes on .egg-info or .dist-info because those filenames expect - to separate the name from the version and other discriminators. That's why the normalization specifically and intentionally expects underscore in place of dash in filenames and specifically relies on - to separate the name.

We probably could trace what the differences were between Python 3.10.5 and .9, but probably they were intentional and more correct.

Before we go down that route, can you explain more about how you ended up with a file called flake8-plugin-utils.egg-info? When I install he package pip-run, I end up with pip_run in my file system:

>>> import sys
>>> sys.path
['', '/Library/Frameworks/Python.framework/Versions/3.12/lib/python312.zip', '/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12', '/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/lib-dynload', '/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages']
>>> import pathlib
>>> list(pathlib.Path(sys.path[-1]).glob('pip*run*info'))
[PosixPath('/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/pip_run-11.0.0.dist-info')]

Similarly, if I install flake8-plugin-utils:

 ~ $ pip-run flake8-plugin-utils -- -q
>>> import sys, pathlib
>>> list(pathlib.Path(sys.path[1]).glob('flake8*info'))
[PosixPath('/var/folders/sx/n5gkrgfx6zd91ymxr2sr9wvw00n8zm/T/pip-run-yw4ze02_/flake8_plugin_utils-1.3.2.dist-info')]

This all leads me to believe that flake8-plugin-utils.egg-info is not a valid form. Please help me understand why that's not the case.

@itamaro
Copy link
Author

itamaro commented Jun 19, 2023

Thank you for the reply @jaraco !

We probably could trace what the differences were between Python 3.10.5 and .9, but probably they were intentional and more correct.

I believe this was the relevant change. I agree it's more correct (e.g. the normalized name for "flake8.egg-info" goes from "flake8.egg" to "flake8"), but it is also making a backwards incompatible change (maybe justifiable, I don't have all the context).

Before we go down that route, can you explain more about how you ended up with a file called flake8-plugin-utils.egg-info?

I can explain, but I can't claim my explanation is good :)
The short version is that we have our own internal package manager, and it apparently uses the pypi package name (with dashes) as is to generate the egg-info / dist-info directories.

This all leads me to believe that flake8-plugin-utils.egg-info is not a valid form. Please help me understand why that's not the case.

This may be the case, and if it is, we need to fix our package manager to apply consistent normalization.
My concern with this conclusion is that while our internal implementation was supposedly PyPA-compliant, it was still wrong (in practice), and I don't know how we were supposed to come up with the correct normalization without reading the importlib metadata normalization implementation that you linked above (and that changed between 3.10 bugfix versions).

@jaraco
Copy link
Member

jaraco commented Jul 8, 2023

I can explain, but I can't claim my explanation is good :)
The short version is that we have our own internal package manager, and it apparently uses the pypi package name (with dashes) as is to generate the egg-info / dist-info directories.

That makes sense, and it's not a big surprise that after creating a standard for package names in a repository that people would be tempted to use that standard elsewhere, even in places where it's incompatible with established designs (such as in metadata filename conventions).

In my opinion, normalization is broken in a few dimensions, but the designs are established and are what they are.

That thread reminds me that I did discover the alternate spec for dist-info names, which explicitly calls out the need to replace dashes with underscores.

So while the "correct normalization" was perhaps difficult to find, it was in fact present and has been implied by the implementations of other installers for some time.

And while importlib_metadata did incidentally support that form for some time, that support was not intentional and it was a bug that it was allowed. So while it did have a backward-incompatible effect to fix that bug, it was not breaking a tested/supported form, so wasn't treated as such. In retrospect, if we'd expected this to have breaking effects, we might have indicated it as a breaking change or at least given users a stronger message about what was changed broken.

At this point, there's not much else I think this project could do except maybe to enhance the changelog to indicate the break.

Let me know how this project can support you.

@itamaro
Copy link
Author

itamaro commented Jul 8, 2023

thanks, that's informative and helpful!

this doc is indeed helpful - I wish it was more discoverable and maybe linked from the other places that could potentially lead to the confusion.

maybe highlighting this change in the release notes of 3.10.9 and 3.11 could help as well.

feel free to close this issue or repurpose it for docs improvements.

@jaraco
Copy link
Member

jaraco commented Sep 21, 2023

I've added a note to the importlib_metadata changelog so the issue could be readily discovered there. Looking at the Python release notes, I think the story is harder. The "what's new" document is for point releases and not bugfixes (e.g. 3.10, not 3.10.9). There is a news file for patch releases, so the notice could go there, but I'm not confident that's discoverable enough to be worthwhile to amend. Moreover, those notes already link to the changelog entry in importlib_metadata. If you think it would be worthwhile to amend the entry in the changelog, just say so and I'll make the change.

If you can think of a better way to increase the visibility of the concern, I'm open to other suggestions. Otherwise, I think this bug report (and the related one in cpython) will be sufficient to surface the concern for others affected.

@jaraco jaraco closed this as completed Sep 21, 2023
@itamaro
Copy link
Author

itamaro commented Sep 21, 2023

thanks @jaraco !

I hope the note you added and the existence of this issue would be sufficient for future stumblers.

toofar added a commit to qutebrowser/qutebrowser that referenced this issue Mar 24, 2024
See 2f3d016 for the corresponding importlib_resources one.
Also this I guess? I don't really understand it, everything seems happy. python/importlib_metadata#459
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants