add basic "install from lock file" operation #3340

BurntSushi · 2024-05-02T18:38:57Z

This PR principally adds a routine for converting a Lock to a
Resolution, where a Resolution is a map of package names pinned to
a specific version.

I'm not sure that a Resolution is ultimately what we want here (we
might need more stuff), but this was the quickest route I could find to
plug a Lock into our existing uv pip install infrastructure.

This commit also does a little refactoring of the Lock types. The
main thing is to permit extra state on some of the types (like a
by_id map on Lock for quick lookups of distributions) that aren't
included in the serialization format of a Lock. We achieve this
by defining separate Wire types that are automatically converted
to-and-from via serde.

Note that like with the lock file format types themselves, we leave a
few todo!() expressions around. The main idea is to get something
minimally working without spending too much effort here. (A fair bit
of refactoring will be required to generate a lock file, and it's
not clear how much this code will wind up needing to change anyway.)
In particular, we only handle the case of installing wheels from a
registry.

A demonstration of the full flow:

$ requirements.in
anyio
$ cargo run -p uv -- pip compile -p3.10 requirements.in --unstable-uv-lock-file
$ uv venv
$ cargo run -p uv -- pip install --unstable-uv-lock-file anyio -r requirements.in
Installed 5 packages in 7ms
 + anyio==4.3.0
 + exceptiongroup==1.2.1
 + idna==3.7
 + sniffio==1.3.1
 + typing-extensions==4.11.0

In order to install from a lock file, we start from the root and do a
breadth first traversal over its dependencies. We aren't yet filtering
on marker expressions (since they aren't in the lock file yet), but we
should be able to add that in the future. In so doing, the traversal
should select only the subset of distributions relevant for the current
platform.

charliermarsh · 2024-05-03T00:18:05Z

crates/uv-resolver/src/lock.rs

+    /// Returns the distribution with the given name. If there are multiple
+    /// matching distributions, then an error is returned. If there are no
+    /// matching distributions, then `Ok(None)` is returned.
+    fn find_by_name(&self, name: &str) -> Result<Option<&Distribution>, String> {


Should this use LockError rather than String?

It's only an internal helper for right now, and its only use asserts that is succeeds. I don't know that this belongs in LockError, and right now, it's only used to find the "root" package. And I expect all of that to change.

charliermarsh · 2024-05-03T00:19:09Z

crates/uv/src/cli.rs

@@ -1308,6 +1308,9 @@ pub(crate) struct PipInstallArgs {
    /// print the resulting plan.
    #[arg(long)]
    pub(crate) dry_run: bool,
+
+    #[arg(long, hide = true)]
+    pub(crate) unstable_uv_lock_file: Option<String>,


Nit: can we call this unstable_lock_file (omitting uv)?

It matches the name I added in: https://github.com/astral-sh/uv/pull/3314/files#diff-ae55eef503096c43098e973865bd5ed612871dcd1d3d335631a8027deb3828d3R605

This flag is hidden, undocumented and intended to be temporary. Its only purpose is to expose "install from uv.lock" on the CLI so that we can interact with it.

I mostly just have a strong dislike for using uv in things (outside of crate names, since it's roughly a namespace), like structs or arguments. But not a big deal if it's temporary.

charliermarsh · 2024-05-03T00:19:58Z

crates/uv/src/commands/pip_install.rs

-            eprint!("{report:?}");
-            return Ok(ExitStatus::Failure);
+    let resolution = if let Some(ref root) = uv_lock {
+        let encoded = fs::tokio::read_to_string("uv.lock").await?;


So this assumes that it's in the current working directory?

Yes. It's the simplest thing to do I think. I expect this code to be eventually deleted. (Like, I don't expect uv pip install should ever read a uv.lock file.)

charliermarsh · 2024-05-03T00:20:34Z

crates/uv-resolver/src/lock.rs

+        root_name: &str,
+    ) -> Resolution {
+        let root = self
+            .find_by_name(root_name)


Could / should this just be a special package named root?

I'm not sure that privileging the notion of "root" in the lock file makes sense, particularly given that we want to have workspace support. Cargo went in the same direction:

Cargo metadata subcommand should work with workspaces rust-lang/cargo#3003

Remove [root] section from Cargo.lock file rust-lang/cargo#4563

Cargo.lock in a workspace lists an arbitrary package as the "[root]" rust-lang/cargo#3704

Ok agreed. This makes more sense now that I see that this isn't ever intended to be part of pip install, but rather, something that stems from a known project root.

charliermarsh

I'm a little confused about the root concept, and why we need to pass anyio as an argument to --unstable-lock. What's the eventual vision there?

charliermarsh · 2024-05-03T00:24:33Z

Separately, what are the expected semantics for pip install with a lockfile? Does the lockfile get regenerated? Should we limit it to pip sync for now for simplicity?

crates/uv-resolver/src/lock.rs

BurntSushi · 2024-05-03T11:46:45Z

@charliermarsh

I'm a little confused about the root concept, and why we need to pass anyio as an argument to --unstable-lock. What's the eventual vision there?

Hmm, so I added this as a TODO comment in the source:

            // TODO: In the future, we should derive the root distribution
            // from the pyproject.toml, but I don't think the infrastructure
            // for that is in place yet. For now, we ask the caller to specify
            // the root package name explicitly, and we assume here that it is
            // correct.

The basic idea here is that the lock file should contain an entry for each package in the current uv workspace. That concept doesn't really exist yet as far as I know, so those packages do not yet get added to the lock file. So we need some other way to pick the root. Right now, we just ask for it. But this is the sort of thing we should be auto-detecting by reading the pyproject.toml.

Separately, what are the expected semantics for pip install with a lockfile? Does the lockfile get regenerated? Should we limit it to pip sync for now for simplicity?

I'm not sure there are any expected semantics for pip install with uv.lock. It's not an obviously well defined operation to me. It's only here now as a temporary way of exposing interaction with uv.lock.

This commit adds a routine for converting a `Lock` to a `Resolution`, where a `Resolution` is a map of package names pinned to a specific version. I'm not sure that a `Resolution` is ultimately what we want here (we might need more stuff), but this was the quickest route I could find to plug a `Lock` into the existing `pip install` infrastructure. This commit also does a little refactoring of the `Lock` types. The main thing is to permit extra state on some of the types (like a `by_id` map on `Lock` for quick lookups of distributions) that aren't included in the serialization format of a `Lock`. We achieve this by defining separate `Wire` types that are automatically converted to-and-from via `serde`. Note like with the lock file format types themselves, we leave a few `todo!()` expressions around. The main idea is to get something minimally working without spending too much effort here. (A fair bit of refactoring will be required to generate a lock file, and it's not clear how much this code will wind up needing to change anyway.) In particular, we only handle the case of installing wheels from a registry.

Like the similar flag on `uv pip compile`, this is meant to be a temporary toggle to expose "install from lock file" on the CLI. We expect to remove this eventually. The larger diff for `pip_install.rs` is mostly just due to moving some pieces together that we specifically don't want to run (like resolution) when installing from a lock file.

BurntSushi requested review from konstin and charliermarsh May 2, 2024 18:39

charliermarsh reviewed May 3, 2024

View reviewed changes

charliermarsh approved these changes May 3, 2024

View reviewed changes

konstin approved these changes May 3, 2024

View reviewed changes

crates/uv-resolver/src/lock.rs Show resolved Hide resolved

BurntSushi added 2 commits May 3, 2024 07:56

BurntSushi force-pushed the ag/lock-install branch from e9c69d0 to 498c709 Compare May 3, 2024 11:56

BurntSushi merged commit 7772e62 into main May 3, 2024
43 checks passed

BurntSushi deleted the ag/lock-install branch May 3, 2024 12:18

BurntSushi added the preview Experimental behavior label May 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add basic "install from lock file" operation #3340

add basic "install from lock file" operation #3340

BurntSushi commented May 2, 2024

charliermarsh May 3, 2024

BurntSushi May 3, 2024

charliermarsh May 3, 2024

BurntSushi May 3, 2024

charliermarsh May 3, 2024

charliermarsh May 3, 2024

BurntSushi May 3, 2024

charliermarsh May 3, 2024

charliermarsh May 3, 2024

BurntSushi May 3, 2024

charliermarsh May 3, 2024

charliermarsh left a comment

charliermarsh commented May 3, 2024

BurntSushi commented May 3, 2024

add basic "install from lock file" operation #3340

add basic "install from lock file" operation #3340

Conversation

BurntSushi commented May 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charliermarsh left a comment

Choose a reason for hiding this comment

charliermarsh commented May 3, 2024

BurntSushi commented May 3, 2024