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 direct_url in python PEP660 editable wheels (again) #20798

Merged
merged 9 commits into from
Apr 18, 2024

Conversation

cognifloyd
Copy link
Member

@cognifloyd cognifloyd commented Apr 16, 2024

We were using the first entry in source_roots_result.path_to_root to populate the editable wheel's direct_url. However, that does not work as I expected it to because SourceRootsRequest.dirs and SourceRootsResult.path_to_root both get sorted as soon as the dataclass is created (if not before).

PR #20486 was my first attempt at fixing this, but that wasn't enough. Since the heuristics used before #20486 and in #20486 were both inaccurate due to dataclasses getting sorted, this abandons the heuristics altogether, explicitly requesting the source root for the python_distribution target.

This time, I added a test to make sure direct_url.json has the contents we expect.

Note: This is ultimately a cosmetic bug. The direct_url gets printed in the output of pip list, but is otherwise not really exposed anywhere. So, it's odd when it's the wrong directory, but does not break any functionality.

We were using the first entry in `source_roots_result.path_to_root` to
populate the editable wheel's `direct_url`. However, that does not work
as I expected it to because `SourceRootsRequest.dirs` and
`SourceRootsResult.path_to_root` both get sorted as soon as the
dataclass is created (if not before).

PR #20486 was my first attempt at fixing this, but that wasn't enough.
Since the heuristics used before #20486 and in #20486 were both
inaccurate due to dataclasses getting sorted, this abandons the
heuristics altogether, explicitly requesting the source root for the
`python_distribution` target.
@cognifloyd cognifloyd added the category:bugfix Bug fixes for released features label Apr 16, 2024
@cognifloyd cognifloyd requested a review from benjyw April 16, 2024 23:24
@cognifloyd cognifloyd self-assigned this Apr 16, 2024
@cognifloyd cognifloyd added the backend: Python Python backend-related issues label Apr 16, 2024
@cognifloyd cognifloyd force-pushed the cognifloyd/fix_pep660_direct_url branch from 647e7b2 to 6e3bdd1 Compare April 17, 2024 22:21
@cognifloyd cognifloyd enabled auto-merge (squash) April 18, 2024 01:05
@cognifloyd cognifloyd disabled auto-merge April 18, 2024 01:05
@cognifloyd cognifloyd enabled auto-merge (squash) April 18, 2024 01:06
@cognifloyd cognifloyd disabled auto-merge April 18, 2024 01:07
@cognifloyd cognifloyd enabled auto-merge (squash) April 18, 2024 01:07
@cognifloyd cognifloyd merged commit c3fb9d0 into main Apr 18, 2024
24 checks passed
@cognifloyd cognifloyd deleted the cognifloyd/fix_pep660_direct_url branch April 18, 2024 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: Python Python backend-related issues category:bugfix Bug fixes for released features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants