Respect visitation order for proxy packages by charliermarsh · Pull Request #10833 · astral-sh/uv

charliermarsh · 2025-01-21T23:15:37Z

Summary

This PR reverts #10441 and applies a different fix for #10425.

In #10441, I changed prioritization to visit proxies eagerly. I think this is actually wrong, since it means we prioritize proxy packages above everything else. And while a proxy only depends on itself, it does mean we're selecting a version for the proxy package earlier than anything else. So, if you look at #10828, we end up choosing a version for async-timeout before we choose a version for langchain, despite the latter being a first-party dependency. (async-timeout has a marker on it, so it has a proxy package, so we solve for it first.)

To fix #10425, we instead need to make sure we visit proxies in the order we see them. I think the virtual tiebreaker for proxies is reversed? We want to visit the package we see first, first.

So, in short: this reverts #10441, then corrects the ordering for visiting proxies.

Closes #10828.

This reverts commit 56d39d2.

konstin · 2025-01-22T12:14:54Z

To fix #10425, we instead need to make sure we visit proxies in the order we see them. I think the virtual tiebreaker for proxies is reversed? We want to visit the package we see first, first.

Would it fix #10425 if we visited the virtual packages inside the package-batch first?

Like:

A{foo}
A{bar}
A <- This one is last
B <- has no virtual packages
C{python_version}
C{extra}
C <- this one is last

konstin

See #10853 for a follow-up

konstin · 2025-01-22T11:14:43Z

crates/uv-resolver/src/resolver/mod.rs

+                // Verify that the package is allowed under the hash-checking policy.
+                if !self
+                    .hasher
+                    .allows_package(candidate.name(), candidate.version())
+                {
+                    return Ok(None);
+                }
+
                // Emit a request to fetch the metadata for this version.
                if self.index.distributions().register(candidate.version_id()) {
-                    // Verify that the package is allowed under the hash-checking policy.
-                    if !self
-                        .hasher
-                        .allows_package(candidate.name(), candidate.version())
-                    {
-                        return Err(ResolveError::UnhashedPackage(candidate.name().clone()));
-                    }
-


Where are the hash changes coming from?

konstin · 2025-01-22T12:09:00Z

crates/uv-resolver/src/dependency_provider.rs

-    /// Main priority and tiebreak for virtual packages
-    type Priority = (Option<PubGrubPriority>, u32);
+    /// Main priority and tiebreak for virtual packages.
+    type Priority = (Option<PubGrubPriority>, Option<PubGrubTiebreaker>);


charliermarsh · 2025-01-22T16:57:26Z

@konstin -- I actually don't think that would be sufficient. Imagine:

foo==1
bar==1

We resolve foo, which depends on bar ; sys_platform == "darwin". We'd then prioritize the bar ; sys_platform == "darwin" proxy over the base package, and we'd select a version of bar that didn't meet the bar==1 requirement.

charliermarsh · 2025-01-22T16:57:58Z

I think a separate thing we could do, to make the order here less critical, is change these hash-mismatch errors to incompatibilities, so we backtrack rather than fail hard just for trying an incompatible package.

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.5.22` -> `0.5.24` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>astral-sh/uv (astral-sh/uv)</summary> ### [`v0.5.24`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0524) [Compare Source](astral-sh/uv@0.5.23...0.5.24) ##### Enhancements - Improve determinism of resolution by always setting package priorities ([#10853](astral-sh/uv#10853)) - Upgrade to `cargo-dist` 0.28.0; improves several installer behaviors ([#10884](astral-sh/uv#10884)) ##### Performance - Remove dependencies clone in resolver ([#10880](astral-sh/uv#10880)) - Use Hashbrown's raw entry API to reduce hashes and clone in resolver priority determination ([#10881](astral-sh/uv#10881)) ##### Bug fixes - Allow fallback to Python download on non-critical discovery errors ([#10908](astral-sh/uv#10908)) ##### Preview features - Register managed Python version with the Windows Registry (PEP 514) ([#10634](astral-sh/uv#10634)) ##### Documentation - Improve documentation for some environment variables ([#10887](astral-sh/uv#10887)) - Add git subdirectory example ([#10894](astral-sh/uv#10894)) ### [`v0.5.23`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0523) [Compare Source](astral-sh/uv@0.5.22...0.5.23) ##### Enhancements - Add `--refresh` to `uv venv` ([#10834](astral-sh/uv#10834)) - Add `--no-default-groups` command-line flag ([#10618](astral-sh/uv#10618)) ##### Bug fixes - Sort extras and groups when comparing lockfile requirements ([#10856](astral-sh/uv#10856)) - Include `commit_id` and `requested_revision` in `direct_url.json` ([#10862](astral-sh/uv#10862)) - Invalidate lockfile when static versions change ([#10858](astral-sh/uv#10858)) - Make GitHub fast path errors non-fatal ([#10859](astral-sh/uv#10859)) - Remove warnings for `--frozen` and `--locked` in `uv run --script` ([#10840](astral-sh/uv#10840)) - Resolve `find-links` paths relative to the configuration file ([#10827](astral-sh/uv#10827)) - Respect visitation order for proxy packages ([#10833](astral-sh/uv#10833)) - Treat version mismatch errors as non-fatal in fast paths ([#10860](astral-sh/uv#10860)) - Mark `--locked` and `--upgrade` are conflicting ([#10836](astral-sh/uv#10836)) - Relax error checking around unconditional enabling of conflicting extras ([#10875](astral-sh/uv#10875)) ##### Documentation - Reduce ambiguity in conflicting extras example ([#10877](astral-sh/uv#10877)) - Update pre-commit documentation ([#10756](astral-sh/uv#10756)) ##### Error messages - Error when workspace contains conflicting Python requirements ([#10841](astral-sh/uv#10841)) - Improve uvx error message when uv is missing ([#9745](astral-sh/uv#9745)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Revert "Visit proxy packages eagerly (#10441)"

dd5db34

This reverts commit 56d39d2.

charliermarsh requested a review from konstin January 21, 2025 23:15

charliermarsh added bug Something isn't working resolver Related to the package resolver labels Jan 21, 2025

charliermarsh mentioned this pull request Jan 21, 2025

langchain version confusion with aiohttp started in uv 0.5.17 #10828

Closed

Add logic

d389f03

charliermarsh force-pushed the charlie/rev branch from 480e17a to d389f03 Compare January 21, 2025 23:17

konstin mentioned this pull request Jan 22, 2025

Add test for package priority size #10851

Closed

konstin approved these changes Jan 22, 2025

View reviewed changes

Update fixtures

d56b638

charliermarsh force-pushed the charlie/rev branch from ffb0c52 to d56b638 Compare January 22, 2025 17:04

charliermarsh enabled auto-merge (squash) January 22, 2025 17:04

charliermarsh merged commit f0dc1f4 into main Jan 22, 2025
68 checks passed

charliermarsh deleted the charlie/rev branch January 22, 2025 17:12

BrewTestBot mentioned this pull request Jan 23, 2025

uv 0.5.23 Homebrew/homebrew-core#205255

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Respect visitation order for proxy packages#10833

Respect visitation order for proxy packages#10833
charliermarsh merged 3 commits intomainfrom
charlie/rev

charliermarsh commented Jan 21, 2025

Uh oh!

konstin commented Jan 22, 2025

Uh oh!

konstin left a comment

Uh oh!

konstin Jan 22, 2025

Uh oh!

konstin Jan 22, 2025

Uh oh!

charliermarsh commented Jan 22, 2025

Uh oh!

charliermarsh commented Jan 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Conversation

charliermarsh commented Jan 21, 2025

Summary

Uh oh!

konstin commented Jan 22, 2025

Uh oh!

konstin left a comment

Choose a reason for hiding this comment

Uh oh!

konstin Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

konstin Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

charliermarsh commented Jan 22, 2025

Uh oh!

charliermarsh commented Jan 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants