Better detection for conflicting packages#17623
Merged
Conversation
EliteTK
approved these changes
Jan 20, 2026
crates/uv/tests/it/pip_install.rs
Outdated
| .child("sum.py") | ||
| .write_str("print('Hi!')")?; | ||
|
|
||
| // Check that overlapping packages don't show a warning by default |
Contributor
There was a problem hiding this comment.
This comment seems to contradict the output now.
crates/uv/tests/it/pip_install.rs
Outdated
| .child("matrix") | ||
| .child("product.py") | ||
| .write_str("print('Hi!')")?; | ||
| // This file collides to, but we always show the first file. |
Contributor
There was a problem hiding this comment.
Suggested change
| // This file collides to, but we always show the first file. | |
| // This file collides too, but we always show the first file. |
Typo I think?
crates/uv/tests/it/pip_install.rs
Outdated
| .child("matrix") | ||
| .child("product.py") | ||
| .write_str("print('Hello world')")?; | ||
| // This file collides to, but we always show the first file. |
Contributor
There was a problem hiding this comment.
Suggested change
| // This file collides to, but we always show the first file. | |
| // This file collides too, but we always show the first file. |
ditto
Comment on lines
+25
to
28
| /// Avoid and track conflicts between packages. | ||
| #[allow(clippy::struct_field_names)] | ||
| #[derive(Debug, Default)] | ||
| pub struct Locks { |
Contributor
There was a problem hiding this comment.
Maybe the struct should itself also be renamed to more closely reflect its use... Although I am not sure what that name should be.
Member
Author
There was a problem hiding this comment.
Stuck at the same question of naming 😅
| files | ||
| .entry(relative) | ||
| .or_default() | ||
| .insert((wheel.clone(), dir_entry.metadata()?.len())); |
Contributor
There was a problem hiding this comment.
We never pass files outside of this function so it can hold references.
Suggested change
| .insert((wheel.clone(), dir_entry.metadata()?.len())); | |
| .insert((wheel, dir_entry.metadata()?.len())); |
You'll need to edit the type signature above too (although you can switch the key types to placeholders as they can be inferred).
Member
Author
There was a problem hiding this comment.
The problem is that when we recurse we need a Oh that's the other path&BTreeSet<(WheelFilename, PathBuf)> instead of a &BTreeSet<(&WheelFilename, PathBuf)>, while the initial call has a &BTreeSet<(WheelFilename, PathBuf)>, I took the easy way out and cloned (we could do some IntoIterator but it doesn't seem worth it).
| subdirectories | ||
| .entry(relative) | ||
| .or_default() | ||
| .insert((wheel.clone(), dir_entry.path().clone())); |
Contributor
There was a problem hiding this comment.
Suggested change
| .insert((wheel.clone(), dir_entry.path().clone())); | |
| .insert((wheel.clone(), dir_entry.path())); |
Contributor
There was a problem hiding this comment.
Technically the wheel clone could be dropped here too if you feel like it. But it would require some reshuffling so not sure it's totally worth it.
Comment on lines
+54
to
+57
| // Only register top level modules | ||
| if relative.components().count() != 1 { | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
Given the doc comment, it's kind of confusing for it to take "relative" (&Path) at all.
I think one of these may be preferable:
- It could take an absolute path and a module name (i.e.
Component) and the responsibility on passing just the module name should fall on the parent. - The function could just be renamed and the doc comment reworded?
Member
Author
There was a problem hiding this comment.
I've renamed the function and made it about paths instead of modules, that matches much better what it's about now.
Comment on lines
+147
to
+148
| // Ensure we always warn for the same first file. | ||
| entries.sort_by_key(DirEntry::path); |
Contributor
There was a problem hiding this comment.
Suggested change
| // Ensure we always warn for the same first file. | |
| entries.sort_by_key(DirEntry::path); |
Sorting here doesn't affect the iteration order for BTreeMap.
Member
Author
There was a problem hiding this comment.
Good catch - That is now redundant due to the BTreeSet.
Member
Author
|
Thank you for the great review! I added an explicit check for files directly in site-packages now too. |
0f49e6d to
784125d
Compare
In the previous iteration, conflict detection was based on top level modules. This would work if all namespace packages correctly omitted the `__init__.py`, but e.g. the nvidia packages include an empty `nvida/__init__.py`. Instead, we track overlapping top level modules during installation and perform conflict analysis after installation, recursing only as far as necessary. Before: ``` $ uv venv -c -q && uv pip install --preview nvidia-nvjitlink-cu12==12.8.93 nvidia-nvtx-cu12==12.8.90 Resolved 2 packages in 0.99ms ░░░░░░░░░░░░░░░░░░░░ [0/2] Installing wheels... warning: The module `nvidia` is provided by more than one package, which causes an install race condition and can result in a broken module. Consider removing your dependency on either `nvidia-nvtx-cu12` (v12.8.90) or `nvidia-nvjitlink-cu12` (v12.8.93). Installed 2 packages in 3ms + nvidia-nvjitlink-cu12==12.8.93 + nvidia-nvtx-cu12==12.8.90 ``` After: ``` $ uv venv -c -q && cargo run -q pip install --preview nvidia-nvjitlink-cu12==12.8.93 nvidia-nvtx-cu12==12.8.90 Resolved 2 packages in 3ms Installed 2 packages in 4ms + nvidia-nvjitlink-cu12==12.8.93 + nvidia-nvtx-cu12==12.8.90 ``` Still detected true positive: ``` $ uv venv -c -q && cargo run -q pip install --no-progress opencv-python opencv-contrib-python --no-build --no-deps --preview Resolved 2 packages in 5ms warning: The file `cv2/__init__.pyi` is provided by more than one package, which causes an install race condition and can result in a broken module. Packages containing the file: * opencv-contrib-python (opencv_contrib_python-4.13.0.90-cp37-abi3-manylinux_2_28_x86_64.whl) * opencv-python (opencv_python-4.13.0.90-cp37-abi3-manylinux_2_28_x86_64.whl) Installed 2 packages in 6ms + opencv-contrib-python==4.13.0.90 + opencv-python==4.13.0.90 ```
36c5369 to
8423175
Compare
tmeijn
pushed a commit
to tmeijn/dotfiles
that referenced
this pull request
Jan 27, 2026
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.9.26` → `0.9.27` | 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.9.27`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0927) [Compare Source](astral-sh/uv@0.9.26...0.9.27) Released on 2026-01-26. ##### Python - Upgrade Pyodide to 0.29.2 ([#​17652](astral-sh/uv#17652)) - Upgrade to GraalPy 25.0.2 ([#​17634](astral-sh/uv#17634)) ##### Enhancements - Add `-t` shortform for `--target` to `uv pip` subcommands ([#​17501](astral-sh/uv#17501)) - Add support for ROCm 7.0 and 7.1 accelerator backends ([#​17681](astral-sh/uv#17681)) - Further improve free-threading ABI incompatibility errors ([#​17491](astral-sh/uv#17491)) - Implement `uv pip freeze --exclude` flag ([#​17045](astral-sh/uv#17045)) - Improve warnings for `--system` and `--no-system` in `uv venv` ([#​17647](astral-sh/uv#17647)) - Make `uv pip compile` attempt to download a specified `--python-version` if it can. ([#​17249](astral-sh/uv#17249)) - Support Trusted Publishing with pyx ([#​17438](astral-sh/uv#17438)) - Fix JSON schema for `exclude-newer-package` ([#​17665](astral-sh/uv#17665)) ##### Preview features - Better detection for conflicting packages ([#​17623](astral-sh/uv#17623)) - Upgrade based on outdated build versions in `uv python upgrade` ([#​17653](astral-sh/uv#17653)) ##### Bug fixes - Change chocolatey system test to ensure uv uses the right python ([#​17533](astral-sh/uv#17533)) - Fix infinite loop when `SSL_CERT_FILE` is a directory ([#​17503](astral-sh/uv#17503)) ##### Documentation - Add cargo-xwin to the CONTRIBUTING guide ([#​17507](astral-sh/uv#17507)) - Fix typo in the documentation of UV\_PUBLISH\_INDEX ([#​17672](astral-sh/uv#17672)) - Move MSRV to platform support section ([#​17534](astral-sh/uv#17534)) - Update the testing instructions in the CONTRIBUTING guide ([#​17528](astral-sh/uv#17528)) - Use `--locked` to install `cargo-xwin` in guide ([#​17530](astral-sh/uv#17530)) - Warn about PyPy being unmaintained ([#​17643](astral-sh/uv#17643)) - docs: Correct gitlab-ci.yml to .gitlab-ci.yml ([#​17682](astral-sh/uv#17682)) ##### Other changes - Update MSRV to 1.91 ([#​17677](astral-sh/uv#17677)) </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. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0Mi45Mi40IiwidXBkYXRlZEluVmVyIjoiNDIuOTIuNCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90IiwiYXV0b21hdGlvbjpib3QtYXV0aG9yZWQiLCJkZXBlbmRlbmN5LXR5cGU6OnBhdGNoIl19-->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In the previous iteration, conflict detection was based on top level modules. This would work if all namespace packages correctly omitted the
__init__.py, but e.g. the nvidia packages include an emptynvida/__init__.py.Instead, we track overlapping top level modules during installation and perform conflict analysis after installation, recursing only as far as necessary.
See #13437 for a list of reported conflicts.
Before:
After:
Still detected true positive: