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

bootstrap using uv #754

Merged
merged 19 commits into from
Feb 25, 2024
Merged

bootstrap using uv #754

merged 19 commits into from
Feb 25, 2024

Conversation

dsp
Copy link
Contributor

@dsp dsp commented Feb 23, 2024

NOTE This is the first iteration on the PR. There is probably a bunch to clean up. Nevertheless I am happy to get early feedback and iterate.

This PR aims to remove pip-tools with uv. We do this by:

  1. Create a download url generator for uv downloads.
  2. Rework our generated downloads structure to encapsulate python and uv downloads.
  3. Implement bootstrapping using UV using the download urls. We will always fetch the only version we have in our downloads.

At the moment we only include one version of uv in the rye binary and ensure this one is used for bootstrapping (multiple versions can co-exists in $RYE_HOME/uv)

Open questions:

  1. Should we fall-back to pip-tools if uv downloads fail?
  2. How to best test this? Should we make it optionally behind a feature flag --use-uv for a while?

@dsp dsp marked this pull request as draft February 23, 2024 16:18
dsp added 5 commits February 23, 2024 16:42
We are starting a common module that includes some functionality that
we will reuse when writing a script to fetch uv downloads.
A simple, probably not very efficient, script to generate rust code
referencing the most recent uv version download. This will be used
for bootstrapping rye.
We move the current sources into sources::py to distinguish it from the
upcoming sources::uv. This is quite a big refactor, but it's worthwhile
to keep python and other potential bootstrapping tools separated.

Notable we also introduce a directory sources/generated which includes
all generated download urls.
@dsp dsp force-pushed the uv-bootstrap branch 5 times, most recently from d54747c to 04d6b54 Compare February 23, 2024 20:42
@dsp dsp marked this pull request as ready for review February 23, 2024 21:16
@dsp dsp marked this pull request as draft February 24, 2024 09:13
@dsp
Copy link
Contributor Author

dsp commented Feb 24, 2024

The PR works with behavior.use-uv=true. Still have to make it work for behavior.use-uv=false, since we don't bootstrap pip-tools themselves at the moment.

TODO:

  • Ensure pip-tools are installed so that sync with pip-tools works when bootstrapping with uv.
  • Uninstall uv when doing self uninstall
  • Update docs

@dsp
Copy link
Contributor Author

dsp commented Feb 24, 2024

Okay the pull request as is works with pip-tools. There is an open question regarding pip-tools handling. At the moment pip-tools bootstraps into $RYE_HOME/pip-tools since we need it to bootstrap $RYE_HOME/self. However, since we now bootstrap with uv, we could just add pip-tools to SELF_REQUIREMENTS and use that during sync when behavior.use-uv is false. However, while i think that's the right thing to do, it's probably best if we do this in a separate PR. Let uv bootstrapping get released and after a while change pip tools. At least have it in another PR, since we squash commits during merging, we ight want to have it separately in case we need to revert it.

@dsp dsp marked this pull request as ready for review February 24, 2024 22:14
@dsp
Copy link
Contributor Author

dsp commented Feb 24, 2024

I ran some basic local tests, both with uv bootstrapping:

  1. Use uv and create a new project and sync: Works ✅
  2. Use pip-tools and create a new project and sync: Works ✅

@mitsuhiko
Copy link
Collaborator

However, since we now bootstrap with uv, we could just add pip-tools to SELF_REQUIREMENTS and use that during sync when behavior.use-uv is false

The issue is that pip-tools needs a fresh venv matching the target python version :( Cannot be installed into the global venv.

@dsp
Copy link
Contributor Author

dsp commented Feb 24, 2024

However, since we now bootstrap with uv, we could just add pip-tools to SELF_REQUIREMENTS and use that during sync when behavior.use-uv is false

The issue is that pip-tools needs a fresh venv matching the target python version :( Cannot be installed into the global venv.

Right. Let's keep it then how it is. There is a good chance I've missed a piece to the puzzle, but with my basic testing the PR is good for review.

Copy link
Collaborator

@mitsuhiko mitsuhiko left a comment

Choose a reason for hiding this comment

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

The uv==0.1.9 line in the self requirements at this point probably should be removed. Otherwise you end up with two uvs for no good reason.

// Request a download for the default uv binary for this platform.
// For instance on aarch64 macos this will request a compatible uv version.
let download = UvDownload::try_from(UvRequest::default())?;
let uv_dir = get_app_dir().join("uv").join(download.version());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there much of a benefit of having multiple uv versions and not just to retain only the latest?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main reason here is that i can check for the version with one simple stat call and not have to execv uv. This allows me in later changes to make use of ensure_exists to obtain a Uv on any code path. It also opens the door for pinning uv if we ever need to (I don't see why, but it feels more future proof). I am happy to just nuke the uv directory before I download a new version which will keep it clean for now. If you insist i can remove the version.

@mitsuhiko
Copy link
Collaborator

Other than the few nits I think this looks good. Also makes for a quicker bootstrap process. I think a good followup would be to make the virtualenv quiet as it now spits out some quite confusing output during installation:

Downloaded [email protected]
Using Python 3.12.1 interpreter at C:\Users\armin\.rye\py\[email protected]\install\python.exe
Creating virtualenv at: C:\Users\armin\.rye\self
Activate with: C:\Users\armin\.rye\self\Scripts\activate
Resolved 1 package in 2ms
Installed 1 package in 95ms
 + pip==23.3.2
Resolved 35 packages in 24ms

Since that env really only exists internally I don't think it makes sense to reveal where it is and how it's being activated.

@dsp
Copy link
Contributor Author

dsp commented Feb 25, 2024

The uv==0.1.9 line in the self requirements at this point probably should be removed. Otherwise you end up with two uvs for no good reason.

Okay that makes sense. This will add a few more changes as we rely on uv being in .rye/self/bin. I'll follow up with adjustment to path we use. A later changset will make use of Uv everywhere where we currently check for Config::current().use_uv() and then construct path to the binary. This should be more sane and allows us to have a canoncial, encapsulated way to construct the binary path. It will also allow us to ensure on a type level that in certain contexts we always operate on a venv.

@dsp
Copy link
Contributor Author

dsp commented Feb 25, 2024

Okay changes made: I keep uv/[version]/uv for now but will clean old directories after we downloaded and extracted the new version. This feels to me a bit more future proof and allows nicely to inspect which uv version we are on.

self-venv creation is now using CommandOutput::Quiet to surpress any call to uv during the process.

@dsp dsp requested a review from mitsuhiko February 25, 2024 14:15
@mitsuhiko mitsuhiko merged commit 4b52db0 into astral-sh:main Feb 25, 2024
6 checks passed
@mitsuhiko
Copy link
Collaborator

Looks good!

@God-damnit-all
Copy link

This is the commit that made it so rye tools install generates a rye-venv.json file, correct?

I really do not like the fact that it now means that a properly configured tool now has to point to an absolute path and it needs to be edited should RYE_HOME ever be relocated. I tried setting use-uv = false in the config under [behavior] in hopes of getting the old way it worked back, but no luck.

@bluss
Copy link
Contributor

bluss commented Mar 8, 2024

@ImportTaste what about all other non-relocatable things in a python virtualenv, is there a solution for them?

Let's say I inspect for example ~/.rye/self/bin/pip3 and find that its shebang has an absolute path as well to the python in the venv, this is typically how virtual environments work unfortunately(!). But since it's a long standing issue, I could see there could be tools to "relocate" them.

In my experience, python virtual environments need to be recreated and can't be relocated unfortunately.

@God-damnit-all
Copy link

@ImportTaste what about all other non-relocatable things in a python virtualenv, is there a solution for them?

Let's say I inspect for example ~/.rye/self/bin/pip3 and find that its shebang has an absolute path as well to the python in the venv, this is typically how virtual environments work unfortunately(!). But since it's a long standing issue, I could see there could be tools to "relocate" them.

In my experience, python virtual environments need to be recreated and can't be relocated unfortunately.

I wasn't aware of that. I guess nevermind then.

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.

4 participants