Skip to content

Prefer valid distributions when collecting entry points - #536

Closed
binggao1230 wants to merge 1 commit into
python:mainfrom
binggao1230:fix-534-entry-points-prefer-valid-dist
Closed

binggao1230 wants to merge 1 commit into
python:mainfrom
binggao1230:fix-534-entry-points-prefer-valid-dist

Conversation

@binggao1230

@binggao1230 binggao1230 commented Jun 19, 2026 •

Copy link
Copy Markdown

Fixes #534

Summary

  • add a duplicate-aware distribution selector for entry_points() that replaces an invalid/empty dist-info entry with a later valid distribution of the same normalized name
  • keep normal discovery order for unique distributions, avoiding the broader cost of preferring valid metadata for every distribution
  • add a regression test where an empty older foo.dist-info appears before a valid one with entry points

Tests

Without the fix, the new test fails with KeyError: 'main'.

PYTHONPATH=. uv run --python 3.11 --with '.[test]' pytest tests/test_main.py::InvalidMetadataTests::test_entry_points_prefer_valid_distribution -q
PYTHONPATH=. uv run --python 3.11 --with '.[test]' pytest tests/test_main.py::InvalidMetadataTests tests/test_api.py::APITests::test_entry_points tests/test_api.py::APITests::test_entry_points_unique_packages_normalized -q
PYTHONPATH=. uv run --python 3.11 --with '.[test]' pytest -q
PYTHONPATH=. uv run --python 3.11 --with '.[test,check]' pytest --ruff -q
PYTHONPATH=. uv run --python 3.11 --with '.[test,type]' pytest --mypy -q
git diff --check

AI assistance was used under my direction.

@rossburton

Copy link
Copy Markdown

I've not tested this but I'm not convinced it will actually work in all cases. Our reason for this is that thanks to package upgrades and removals we'll end up with eg wheel-0.47.0.dist-info/ existing in site-packages, but it will be completely empty.

This is discovered as a distribution, and because metadata() falls back to "parse an empty string" it still has valid metadata, it's just that the metadata is empty. I'm currently hacking a test into pypa/build to check that dist.metadata.name is actually set.

@rossburton

Copy link
Copy Markdown

I take that back, latest importlib_metadata has different behaviour and correctly raises an exception.

@rossburton

Copy link
Copy Markdown

I must admit I don't understand the reasoning behind returning invalid distributions at all: if some empty directories that look like a dist but have no content are found, why return them at all?

@binggao1230

Copy link
Copy Markdown
Author

I agree that globally dropping invalid distributions is the cleaner end state if the project wants that behavior. I kept this PR narrower for two reasons:

  1. Distribution.discover()/distributions() is the lower-level discovery surface, and changing it to filter path entries would be a broader API/behavior change than entry_points() duplicate resolution.
  2. The concrete failure here is narrower: when two distributions normalize to the same name, an invalid/empty older dist-info can win the unique() selection and hide the later valid distribution that actually has the entry point. Preferring the valid candidate only inside that duplicate-name choice fixes that failure without changing what raw discovery returns.

So this patch is conservative rather than philosophical: it avoids letting an invalid duplicate shadow a valid package, but does not decide whether invalid dist-info directories should disappear from discovery everywhere. If you prefer the broader policy, I can rework it in that direction.

@jaraco

jaraco commented Oct 23, 2026

Copy link
Copy Markdown
Member

I must admit I don't understand the reasoning behind returning invalid distributions at all: if some empty directories that look like a dist but have no content are found, why return them at all?

It's because the presence of the directory implies presence of a package at that version. The distribution is there even though it has no metadata. One could theoretically put a 'other-metadata.txt' in the -dist-info directory, and that would be queryable using this API. The API is trying to reflect the reality of the situation and be lenient to corruption or future evolution or extension.

@jaraco

jaraco commented Oct 23, 2026

Copy link
Copy Markdown
Member

In #550, I implemented an even narrower approach, which was to implement an imperfect sorting heuristic to paper over the most common case where a newer version replaces an older version but the older version metadata isn't cleaned up properly.

That approach will help the common case, but will fail to help in the case where:

  • The replacement package is in fact older, so it's actually the older package that is valid and should be preferred.
  • The lexicographic ordering doesn't reflect the true version ordering.

It almost makes me wonder if the sorting should instead sort by timestamp on the directory, which would more reliably reflect "newest".

Comment on lines +1129 to +1136
unique: dict[str, Distribution] = {}
for dist in dists:
name = dist._normalized_name
if name not in unique:
unique[name] = dist
elif not has_metadata(unique[name]) and has_metadata(dist):
unique[name] = dist
return unique.values()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This approach undermines the intent of _unique, which was to produce a lazily-generated iterable of distributions. By constructing the dict, it changes the shape of the wrapper. I agree this proposal does what's intended; I'm just uneasy with how it (necessarily) interferes with the flow at this stage.

@jaraco

jaraco commented Oct 23, 2026

Copy link
Copy Markdown
Member

I'm now reconsidering the approach in #550 and wondering if instead we shouldn't take the _prefer_valid approach.

@binggao1230

Copy link
Copy Markdown
Author

Thanks for taking this forward in #553. That version addresses the same failure while preserving the lazy _unique pipeline and bounding the metadata-detection cost, so I’m closing this in favor of that implementation.

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

Successfully merging this pull request may close these issues.

entry_points doesn't handle empty .dist-info files well

3 participants