fix(uv): drop powerpc64 support to fix latest version downloads#3678
fix(uv): drop powerpc64 support to fix latest version downloads#3678aignas merged 3 commits intobazel-contrib:mainfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the default uv version to 0.11.2 and modifies the toolchain to extract download URLs directly from the dist-manifest.json file. It also drops default support for the powerpc64 platform to resolve compatibility issues with newer uv versions. Review feedback highlights several typos and grammatical errors in the CHANGELOG.md and identifies a potential IndexError in the manifest parsing logic within python/uv/private/uv.bzl that should be handled more robustly.
| this particular case. | ||
| ([#3260](https://github.com/bazel-contrib/rules_python/issues/3260) and | ||
| [#2632](https://github.com/bazel-contrib/rules_python/issues/2632)) | ||
| * (uv) We will now use the download URL specified in the `uv`'s `dist_manifest.json` |
There was a problem hiding this comment.
The filename mentioned here is dist_manifest.json, but the actual filename used in the repository and by the uv project is dist-manifest.json (with a hyphen).
| * (uv) We will now use the download URL specified in the `uv`'s `dist_manifest.json` | |
| * (uv) We will now use the download URL specified in the `uv`'s `dist-manifest.json` |
| you may need to adjust them. What is more, the default uv version has been bumped | ||
| `0.11.2`. |
There was a problem hiding this comment.
There is a missing word "to" before the version number.
| you may need to adjust them. What is more, the default uv version has been bumped | |
| `0.11.2`. | |
| you may need to adjust them. What is more, the default uv version has been bumped | |
| to `0.11.2`. |
| * (uv) Downloads for versions `>=0.10` work again. In order to fix this we had | ||
| drop support for `powerpc64` platform. People interested in the platform can |
There was a problem hiding this comment.
There is a missing word "to" before "drop".
| * (uv) Downloads for versions `>=0.10` work again. In order to fix this we had | |
| drop support for `powerpc64` platform. People interested in the platform can | |
| * (uv) Downloads for versions `>=0.10` work again. In order to fix this we had to | |
| drop support for `powerpc64` platform. People interested in the platform can |
| base_url = ( | ||
| dist_manifest | ||
| .get("releases", [{}])[0] | ||
| .get("hosting", {}) | ||
| .get("simple", {}) | ||
| .get("download_url", base_url) | ||
| ) |
There was a problem hiding this comment.
The current implementation will raise an IndexError if the releases key exists in the manifest but is an empty list (e.g., {"releases": []}). It is safer to check if the list is non-empty before attempting to access the first element.
releases = dist_manifest.get("releases")
if releases:
base_url = releases[0].get("hosting", {}).get("simple", {}).get("download_url", base_url)
Before this PR we would index all of the available binaries and it would fail in the case if the `sha256` file is not found. It seems that this is the case for the `powerpc64`. In order to work this around, we just drop support for that particular platform. Whilst at it, bump the uv version. Fixes bazel-contrib#3676.
aignas
force-pushed
the
aignas.fix.bump-uv
branch
from
4a84f98 to
518aadc
Compare
2 participants
Before this PR we would index all of the available binaries and it would
fail in the case if the
sha256file is not found. It seems that thisis the case for the
powerpc64. In order to work this around, we justdrop support for that particular platform.
Whilst at it, bump the uv version.
Fixes #3676.