Skip to content

Conversation

@chrisirhc
Copy link
Contributor

@chrisirhc chrisirhc commented Mar 9, 2025

This PR adds support for installing wheels via direct urls in the requirements lock file:

foo==0.0.1 @ https://someurl.org/package.whl
bar==0.0.1 @ https://someurl.org/package.tar.gz

This is to improve parity between bazel downloader and pip behavior.
Before this change, direct urls used fallback to pip install.

Partially addresses #2363 as it does not add support for git urls.

@chrisirhc chrisirhc changed the title WIP sketch of adding bazel downloader support for direct urls WIP sketch of adding bazel downloader support for direct urls for wheels Mar 9, 2025
@chrisirhc chrisirhc changed the title WIP sketch of adding bazel downloader support for direct urls for wheels WIP feat: of adding support for direct urls for wheels in bazel downloader Mar 9, 2025
@chrisirhc chrisirhc changed the title WIP feat: of adding support for direct urls for wheels in bazel downloader WIP feat: add support for direct urls for wheels in bazel downloader Mar 9, 2025
@chrisirhc chrisirhc changed the title WIP feat: add support for direct urls for wheels in bazel downloader WIP feat: support direct urls for wheels in bazel downloader Mar 9, 2025
@chrisirhc chrisirhc changed the title WIP feat: support direct urls for wheels in bazel downloader feat(pypi): support direct urls for wheels in bazel downloader Mar 10, 2025
@chrisirhc chrisirhc marked this pull request as ready for review March 10, 2025 00:29
@aignas
Copy link
Collaborator

aignas commented Mar 10, 2025

Thank you for the contribution.

My guess is that direct urls can return sdists via checking the filename for the extensions listed in here. If I can get confirmation on this, I can update this PR to add sdist support for direct urls or leave that as a separate PR.

I think this part might be good to leave for another PR.

Copy link
Collaborator

@aignas aignas left a comment

Choose a reason for hiding this comment

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

LGTM in general, I think we can correctly infer the sdist vs not via the direct URLs, so I think its fine to just do it.

Could you please also add the CHANGELOG note about the added support.

@chrisirhc chrisirhc requested a review from aignas March 11, 2025 08:41
Copy link
Collaborator

@aignas aignas left a comment

Choose a reason for hiding this comment

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

Thank you so much!

@aignas aignas added this pull request to the merge queue Mar 12, 2025
Merged via the queue into bazel-contrib:main with commit 5a8f6c4 Mar 12, 2025
4 checks passed
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.

2 participants