GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — fix `PathDistribution._normalized_name` implementation by benoit-pierre · Pull Request #377 · python/importlib_metadata

Conversation

@benoit-pierre

@benoit-pierre benoit-pierre commented May 12, 2022

Copy link
Copy Markdown
Contributor
  • correctly handle paths with no embedded version (e.g. pkg.egg-info must yield the name pkg, not pkg.egg)
  • apply PEP 503 normalization to the extracted names (e.g.: zope..inter_face-4.2.dist-info must yield the name zope_inter_face)

Both are necessary, or entry_points(…) can yield the entry-points of a shadowed distribution. For example: with a version of mypkg in the system' site-packages directory when working from another development checkout of the same package (with a mypgk.egg-info directory mishandled by the first bug).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@jaraco

jaraco commented May 21, 2022

Copy link
Copy Markdown
Member

Thanks @benoit-pierre for this proposal. Unfortunately, I'm struggling a little bit with it, mainly because it fixes two unreported bugs in one commit. I'm also uneasy about the added complexity that "normalized names" is adding here (requiring fixtures with multiple return values). Would you mind writing up a bug report describing how you encountered this bug and how we might replicate it (and prove the fix)?

I'm a little disappointed in the test suite right now. It seems the performance tests aren't running. Also, the linter didn't catch that name was shadowed and never used, which might have been an indicator that something was wrong.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
jaraco added a commit that referenced this pull request May 21, 2022
@jaraco
Add some (failing) tests for _name_from_stem. Ref #377
4461855
jaraco added a commit that referenced this pull request May 21, 2022
@jaraco
Fix issue when metadata on the file system has no version. No more eg…
01843df 
…g on face. Ref #377.
jaraco added a commit that referenced this pull request May 21, 2022
@jaraco
Update changelog. Ref #377.
9deee50
@jaraco jaraco mentioned this pull request May 21, 2022
Fix filesystem path distribution name inference parsing. #379
Merged
@jaraco

jaraco commented May 21, 2022

Copy link
Copy Markdown
Member

In #379, I've addressed the first issue.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jaraco
jaraco force-pushed the fix_PathDistribution._normalized_name_implementation branch 3 times, most recently from ad58bb0 to 1a1a3db Compare May 21, 2022 15:13
@benoit-pierre @jaraco
fix PathDistribution._normalized_name implementation
596abd0 
- apply PEP 503 normalization to the extracted names
  (e.g.: `zope..inter_face-4.2.dist-info` must yield
   the name `zope_inter_face`)

`entry_points(…)` can yield the entry-points
of a shadowed distribution. For example: with a version of `mypkg`
in the system' site-packages directory when working from another
development checkout of the same package (with a `mypkg.egg-info`
directory mishandled by the first bug).
@jaraco
jaraco force-pushed the fix_PathDistribution._normalized_name_implementation branch from 1a1a3db to 596abd0 Compare May 21, 2022 15:15
jaraco
jaraco reviewed May 21, 2022
View reviewed changes
Comment thread tests/test_main.py
assert version(pkg_name) == '1.0'
pkg_name, norm_pkg_name = self.pkg_with_dashes(self.site_dir)
dist = distribution(pkg_name)
assert dist._normalized_name == norm_pkg_name

@jaraco jaraco May 21, 2022

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Instead of testing the internal implementation details, I'd rather have a test that captures the failed expectation - namely that entry points of two packages whose file system names vary only by normalization won't deduplicate.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
jaraco added a commit that referenced this pull request May 21, 2022
@jaraco
Add test expanding expectation on unique packages to include normaliz…
6eb6f71 
…ation. Ref #377.
@jaraco

jaraco commented May 21, 2022

Copy link
Copy Markdown
Member

In #381, I've found I can capture the missed expectation by simply tweaking the test for test_entry_points_unique_packages to include the expected normalization.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jaraco

jaraco commented May 21, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

These changes are superseded by #379 and #381. Thanks for the contrib.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jaraco jaraco closed this May 21, 2022
@stephenfin stephenfin mentioned this pull request Oct 4, 2022
Presence of 'egg-info' metadata directory + use of virtualenv results in duplicate entry points #410
Closed
This was referenced Nov 4, 2022
Distribution._normalized_name for .egg-info has 'egg' in it. #413
Closed
[3.10] Package names with different metadata types aren't deduplicated python/cpython#99130
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jaraco jaraco jaraco left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@benoit-pierre @jaraco
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.