Use `Resolver` in `pip sync` #2696

charliermarsh · 2024-03-27T17:22:56Z

Summary

This PR removes the custom DistFinder that we use in pip sync. This originally existed because VersionMap wasn't lazy, and so we saved a lot of time in DistFinder by reading distribution data lazily. But now, AFAICT, there's really no benefit. Maintaining DistFinder means we effectively have to maintain two resolvers, and end up fixing bugs in DistFinder that don't exist in the Resolver (like #2688.

Closes #2694.

Closes #2443.

Test Plan

I ran this benchmark a bunch. It's basically a wash. Sometimes one is faster than the other.

❯ python -m scripts.bench \
        --uv-path ./target/release/main \
        --uv-path ./target/release/uv \
        scripts/requirements/compiled/trio.txt --min-runs 50 --benchmark install-warm --warmup 25
Benchmark 1: ./target/release/main (install-warm)
  Time (mean ± σ):      54.0 ms ±  10.6 ms    [User: 8.7 ms, System: 98.1 ms]
  Range (min … max):    45.5 ms …  94.3 ms    50 runs

  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet PC without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.

Benchmark 2: ./target/release/uv (install-warm)
  Time (mean ± σ):      50.7 ms ±   9.2 ms    [User: 8.7 ms, System: 98.6 ms]
  Range (min … max):    44.0 ms …  98.6 ms    50 runs

  Warning: The first benchmarking run for this command was significantly slower than the rest (77.6 ms). This could be caused by (filesystem) caches that were not filled until after the first run. You should consider using the '--warmup' option to fill those caches before the actual benchmark. Alternatively, use the '--prepare' option to clear the caches before each timing run.

Summary
  './target/release/uv (install-warm)' ran
    1.06 ± 0.29 times faster than './target/release/main (install-warm)'

charliermarsh · 2024-03-27T17:23:17Z

crates/uv-dev/src/main.rs

@@ -66,7 +64,6 @@ enum Cli {
    /// cargo run --bin uv-dev -- resolve-many scripts/popular_packages/pypi_10k_most_dependents.txt
    /// ```
    ResolveMany(ResolveManyArgs),
-    InstallMany(InstallManyArgs),


Does anyone care about preserving this? I can migrate it but I wasn't sure if it was worth it. \cc @konstin

I've been annoyed maintaining it since I don't use it.

It was nice to have but honestly not if its effort to maintain, i don't run it anymore

Haha. If I had my way I would remove them all.

charliermarsh · 2024-03-27T17:25:09Z

crates/uv/tests/pip_sync.rs

-    error: Network connectivity is disabled, but the requested data wasn't found in the cache for: `black`
+    error: Because black==23.10.1 was not found in the cache and you require black==23.10.1, we can conclude that the requirements are unsatisfiable.
+
+    hint: Packages were unavailable because the network was disabled


This is an improvement, I think.

charliermarsh · 2024-03-27T17:25:18Z

crates/uv/tests/pip_sync.rs

-    error: markupsafe isn't available locally, but making network requests to registries was banned.
+    error: Because markupsafe==2.1.3 was not found in the provided package locations and you require markupsafe==2.1.3, we can conclude that the requirements are unsatisfiable.
+
+    hint: Packages were unavailable because index lookups were disabled and no additional package locations were provided (try: `--find-links <uri>`)


This is an improvement, I think.

charliermarsh · 2024-03-27T17:25:22Z

crates/uv/tests/pip_sync.rs

@@ -584,7 +584,7 @@ fn install_git_tag() -> Result<()> {
    Resolved 1 package in [TIME]
    Downloaded 1 package in [TIME]
    Installed 1 package in [TIME]
-     + werkzeug==2.0.0 (from git+https://github.com/pallets/werkzeug.git@2.0.0)
+     + werkzeug==2.0.0 (from git+https://github.com/pallets/werkzeug.git@af160e0b6b7ddd81c22f1652c728ff5ac72d5c74)


This is an improvement, I think.

charliermarsh · 2024-03-27T17:27:10Z

crates/uv/tests/pip_sync.rs

@@ -1801,7 +1803,7 @@ fn install_url_built_dist_cached() -> Result<()> {
    ----- stdout -----

    ----- stderr -----
-    Removed 2 files for tqdm ([SIZE])
+    Removed 3 files for tqdm ([SIZE])


I don't know why this changed. Will look into it.

We have an additional .msgpack file from the cache, because we fetch the Requires-Python for the URL.

charliermarsh · 2024-03-27T17:27:22Z

crates/uv/tests/pip_sync.rs

-///
-/// TODO(charlie): This currently passes, but should fail. `sync` does not currently validate the
-/// `Requires-Python` constraint for direct URL dependencies. (It _does_ respect `Requires-Python`
-/// for registry-based dependencies.)


This was #2443.

zanieb

This makes sense to me. Will have conflicts with #2596, we'll need to add support for local site packages to pip sync.

BurntSushi

Wow.

charliermarsh requested review from BurntSushi and zanieb March 27, 2024 17:23

charliermarsh commented Mar 27, 2024

View reviewed changes

charliermarsh force-pushed the charlie/finder branch from 8b95e1c to 21bf287 Compare March 27, 2024 17:24

charliermarsh marked this pull request as ready for review March 27, 2024 17:24

charliermarsh commented Mar 27, 2024

View reviewed changes

charliermarsh force-pushed the charlie/finder branch from 21bf287 to bc30872 Compare March 27, 2024 17:28

zanieb approved these changes Mar 27, 2024

View reviewed changes

BurntSushi approved these changes Mar 27, 2024

View reviewed changes

Remove Finder

061956e

charliermarsh force-pushed the charlie/finder branch from bc30872 to 061956e Compare March 27, 2024 20:50

charliermarsh merged commit f8fa887 into main Mar 27, 2024
31 checks passed

charliermarsh deleted the charlie/finder branch March 27, 2024 21:36

zanieb mentioned this pull request Mar 27, 2024

Consider installed packages during resolution #2596

Merged

8 tasks

BrewTestBot mentioned this pull request Mar 28, 2024

uv 0.1.25 Homebrew/homebrew-core#167401

Merged

wolfv mentioned this pull request Apr 17, 2024

feat: upgrade to new uv version 0.1.32 prefix-dev/pixi#1208

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `Resolver` in `pip sync` #2696

Use `Resolver` in `pip sync` #2696

charliermarsh commented Mar 27, 2024 •

edited

Loading

charliermarsh Mar 27, 2024

zanieb Mar 27, 2024

konstin Mar 27, 2024

charliermarsh Mar 27, 2024

charliermarsh Mar 27, 2024

charliermarsh Mar 27, 2024

charliermarsh Mar 27, 2024

charliermarsh Mar 27, 2024

charliermarsh Mar 27, 2024

charliermarsh Mar 27, 2024

zanieb left a comment •

edited

Loading

BurntSushi left a comment

Use Resolver in pip sync #2696

Use Resolver in pip sync #2696

Conversation

charliermarsh commented Mar 27, 2024 • edited Loading

Summary

Test Plan

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

zanieb left a comment • edited Loading

Choose a reason for hiding this comment

BurntSushi left a comment

Choose a reason for hiding this comment

Use `Resolver` in `pip sync` #2696

Use `Resolver` in `pip sync` #2696

charliermarsh commented Mar 27, 2024 •

edited

Loading

zanieb left a comment •

edited

Loading