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

feat: download script for uv bin #57

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rikublock
Copy link
Contributor

Summary

This PR introduces a download script to simplify fetching the uv binary, primarily aimed at making the local development environment setup easier.

Changes include:

  • Move uv version config to package.json
  • Add SHA256 hash verification
  • Update build workflow
  • Update Readme section

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Tests added / updated (if applicable)
  • Documentation added / updated (if applicable)

- move uv config to package.json
- compare sha256 hash
Copy link
Collaborator

@psychedelicious psychedelicious left a comment

Choose a reason for hiding this comment

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

Thanks, this is great! I noticed a couple things during review and testing.

Generally, I have a idea to make it a bit easier to manage the uv versions.

The uv releases on GH use consistent URLs, where the checksum URL is the same as the executable, but with .sha256 appended.

For example, for macOS:

  • https://github.com/astral-sh/uv/releases/download/<version>/uv-aarch64-apple-darwin.tar.gz
  • https://github.com/astral-sh/uv/releases/download/<version>/uv-aarch64-apple-darwin.tar.gz.sha256

Instead of hardcoding executable URL and the hash in package.json, we could:

  • Record only the uv version in package.json
  • Add the URL "patterns" to download_uv.ts
  • Download both executable and checksum in the download script

It's a bit more work to set up, but we can avoid the error-prone manual checksum downloading and copying.

@rikublock
Copy link
Contributor Author

Generally, I have a idea to make it a bit easier to manage the uv versions.

The uv releases on GH use consistent URLs, where the checksum URL is the same as the executable, but with .sha256 appended.

For example, for macOS:

* `https://github.com/astral-sh/uv/releases/download/<version>/uv-aarch64-apple-darwin.tar.gz`

* `https://github.com/astral-sh/uv/releases/download/<version>/uv-aarch64-apple-darwin.tar.gz.sha256`

Instead of hardcoding executable URL and the hash in package.json, we could:

* Record only the `uv` version in `package.json`

* Add the URL "patterns" to `download_uv.ts`

* Download both executable and checksum in the download script

It's a bit more work to set up, but we can avoid the error-prone manual checksum downloading and copying.

I agree that the new configuration is slightly more cumbersome and potentially error-prone. However, the changes are intentional:

  • Keeping the source generic: Using the full URL instead of just the version number allows flexibility in case we want to use a source other than GitHub. Updating the version number in three URLs still feels reasonably manageable.
  • Purpose of the SHA-256 check: The goal of verifying the hash is to add a basic sanity check, ensuring the binary hasn’t been tampered with after the initial configuration. However, downloading an unsigned checksum from the same source somewhat defeats this purpose. If we can’t ensure the integrity of the checksum, I’d prefer to remove the hash check altogether.

Please let me know how you’d like to proceed.

@psychedelicious
Copy link
Collaborator

Thanks for sharing your reasoning, it makes sense. It's not a major hassle to manually update the version & hashes. I'm happy with the setup as you've written it in this PR.

I'll do a final review, test and merge in the next day or two. Thanks!

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