Skip to content

Allow multiple self-dependencies #338

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

Merged
merged 1 commit into from
May 22, 2025

Conversation

konstin
Copy link
Member

@konstin konstin commented May 9, 2025

In uv, we don't use the DependencyConstraints map, but pass in the dependencies through an iterator. This means there can be duplicate dependencies in the input. This would previously make merge_dependents panic if a package dependent on itself twice with the same range:

[package]
name = "foo"
version = "0.1.0"
dependencies = ["foo", "foo"]

The fix is to ignore self-dependencies when merging dependents, given that they are always trivially true or trivially false.

@konstin konstin requested a review from Eh2406 May 9, 2025 14:31
In uv, we don't use the `DependencyConstraints` map, but pass in the dependencies through an iterator. This means there can be duplicate dependencies in the input. This would previously make `merge_dependents` panic if a package dependent on itself twice with the same range:

```toml
[package]
name = "foo"
version = "0.1.0"
dependencies = ["foo", "foo"]
```

The fix is to ignore self-dependencies when merging dependents, given that they are always trivially true or trivially false.
@konstin konstin force-pushed the konsti/dev/allow-multiple-self-dependencies branch from 22c7c0b to 7018db9 Compare May 9, 2025 16:43
Copy link
Member

@mpizenberg mpizenberg left a comment

Choose a reason for hiding this comment

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

It makes sense. And I don’t see any obvious downside considering merge_dependents is just an optimization.

@Eh2406 Eh2406 added this pull request to the merge queue May 22, 2025
Merged via the queue into dev with commit ee0596f May 22, 2025
7 checks passed
@Eh2406 Eh2406 deleted the konsti/dev/allow-multiple-self-dependencies branch May 22, 2025 20:43
konstin added a commit to astral-sh/uv that referenced this pull request May 23, 2025
With pubgrub-rs/pubgrub#338 merged, we update PubGrub to 06ec5a5f59ffaeb6cf5079c6cb184467da06c9db
konstin added a commit to astral-sh/uv that referenced this pull request May 23, 2025
With pubgrub-rs/pubgrub#338 merged, we update
PubGrub to 06ec5a5f59ffaeb6cf5079c6cb184467da06c9db
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.

3 participants