Skip to content

Support older Python and find-links#11

Open
vyasr wants to merge 4 commits intopep-xxx-wheel-variants-devfrom
fix/python_311_and_find_links
Open

Support older Python and find-links#11
vyasr wants to merge 4 commits intopep-xxx-wheel-variants-devfrom
fix/python_311_and_find_links

Conversation

@vyasr
Copy link
Member

@vyasr vyasr commented Jul 16, 2025

This PR allows running on versions of Python before 3.12 because the code is currently using some f-string syntax that requires PEP 701 (specifically, using double quotes inside an f-string that is already delimited by double quotes). Beyond that, this PR supports using pip's --find-links argument to discover local directories containing variant builds.

@DEKHTIARJonathan
Copy link
Member

@mgorny do you mind reviewing this PR ?

@DEKHTIARJonathan DEKHTIARJonathan requested a review from mgorny July 18, 2025 18:36
Comment on lines +934 to +941
# Since candidates_from_page does not get used to process file sources
# from find-links, manually inject evaluate_links calls here.
_links = [
Link(path_to_url(fl.variants_json))
for fl in collected_sources.find_links
if fl.variants_json is not None
]
self.evaluate_links(link_evaluator, _links)
Copy link
Member Author

Choose a reason for hiding this comment

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

Not a very clean way of doing things, but I figured better than nothing given that we're still prototyping. I'd be happy with a more structured solution at some point but I don't think it's urgent.

# these are normalized, but with .replace("_", "-")
return (
f"{wheel.name.replace("-", "_")}-{wheel.version.replace("-", "_")}-"
f"{wheel.name.replace('-', '_')}-{wheel.version.replace('-', '_')}-"
Copy link
Member Author

Choose a reason for hiding this comment

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

This is for pre-PEP 701 compatibility.

Copy link
Member Author

Choose a reason for hiding this comment

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

Presumably we would need this upstreamed to packaging itself as well (although we'll need the change in both places since pip will continue vendoring it anyway).

Copy link

Choose a reason for hiding this comment

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

Yeah, kinda. I didn't change that API since we didn't need it at the time, and — well, it can't return the variant label, so it won't be correct for variant wheels anyway.

That said, don't worry about upstreaming much — our pip fork is based on old version, and pip's changed too much in main to justify rebasing the demo.

project_filename = parse_sdist_filename(entry.name)[0]
except InvalidSdistFilename:
if entry.name.endswith("-variants.json"):
self.variants_json = entry.path
Copy link

Choose a reason for hiding this comment

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

Doesn't this class have to account for multiple versions? If yes, then you'd probably need a dict from package names+versions to paths, similarly to how we do it in the regular codepath.

Copy link

Choose a reason for hiding this comment

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

Yeah, kinda. I didn't change that API since we didn't need it at the time, and — well, it can't return the variant label, so it won't be correct for variant wheels anyway.

That said, don't worry about upstreaming much — our pip fork is based on old version, and pip's changed too much in main to justify rebasing the demo.

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.

3 participants