Warn when two packages write to the same module

crates/uv-install-wheel/src/install.rs

@@ -75,7 +75,7 @@ pub fn install_wheel(
         LibKind::Pure => &layout.scheme.purelib,
         LibKind::Plat => &layout.scheme.platlib,
     };
-    let num_unpacked = link_mode.link_wheel_files(site_packages, &wheel, locks)?;
+    let num_unpacked = link_mode.link_wheel_files(site_packages, &wheel, locks, filename)?;
     trace!(?name, "Extracted {num_unpacked} files");
 
     // Read the RECORD file.

crates/uv-install-wheel/src/linker.rs

@@ -4,16 +4,53 @@ use fs_err::DirEntry;
 use reflink_copy as reflink;
 use rustc_hash::FxHashMap;
 use serde::{Deserialize, Serialize};
+use std::ffi::{OsStr, OsString};
 use std::path::{Path, PathBuf};
 use std::sync::{Arc, Mutex};
 use std::time::SystemTime;
 use tempfile::tempdir_in;
 use tracing::{debug, instrument, trace};
-use uv_warnings::warn_user_once;
+use uv_distribution_filename::WheelFilename;
+use uv_fs::Simplified;
+use uv_warnings::{warn_user, warn_user_once};
 use walkdir::WalkDir;
 
 #[derive(Debug, Default)]
-pub struct Locks(Mutex<FxHashMap<PathBuf, Arc<Mutex<()>>>>);
+pub struct Locks {
+    /// The parent directory of a file in a synchronized copy
+    copy_dir_locks: Mutex<FxHashMap<PathBuf, Arc<Mutex<()>>>>,
+    /// Top level modules (excluding namespaces) we write to.
+    modules: Mutex<FxHashMap<OsString, WheelFilename>>,
+}
+
+impl Locks {
+    /// Warn when a module exists in multiple packages.
+    fn warn_module_conflict(&self, module: &OsStr, wheel_a: &WheelFilename) {
+        if let Some(wheel_b) = self
+            .modules
+            .lock()
+            .unwrap()
+            .insert(module.to_os_string(), wheel_a.clone())
+        {
+            // Sort for consistent output, at least with two packages
+            let (wheel_a, wheel_b) = if wheel_b.name > wheel_a.name {
+                (&wheel_b, wheel_a)
+            } else {
+                (wheel_a, &wheel_b)
+            };
+            warn_user!(
+                "The module `{}` 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 `{}` (v{}) or `{}` (v{}).",
+                module.simplified_display().green(),
+                wheel_a.name.cyan(),
+                wheel_a.version.cyan(),
+                wheel_b.name.cyan(),
+                wheel_b.version.cyan(),
+            );
+        }
+    }
+}
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(deny_unknown_fields, rename_all = "kebab-case")]
@@ -48,12 +85,13 @@ impl LinkMode {
         site_packages: impl AsRef<Path>,
         wheel: impl AsRef<Path>,
         locks: &Locks,
+        filename: &WheelFilename,
     ) -> Result<usize, Error> {
         match self {
-            Self::Clone => clone_wheel_files(site_packages, wheel, locks),
-            Self::Copy => copy_wheel_files(site_packages, wheel, locks),
-            Self::Hardlink => hardlink_wheel_files(site_packages, wheel, locks),
-            Self::Symlink => symlink_wheel_files(site_packages, wheel, locks),
+            Self::Clone => clone_wheel_files(site_packages, wheel, locks, filename),
+            Self::Copy => copy_wheel_files(site_packages, wheel, locks, filename),
+            Self::Hardlink => hardlink_wheel_files(site_packages, wheel, locks, filename),
+            Self::Symlink => symlink_wheel_files(site_packages, wheel, locks, filename),
         }
     }
 
@@ -73,18 +111,25 @@ fn clone_wheel_files(
     site_packages: impl AsRef<Path>,
     wheel: impl AsRef<Path>,
     locks: &Locks,
+    filename: &WheelFilename,
 ) -> Result<usize, Error> {
+    let wheel = wheel.as_ref();
     let mut count = 0usize;
     let mut attempt = Attempt::default();
 
-    for entry in fs::read_dir(wheel.as_ref())? {
-        clone_recursive(
-            site_packages.as_ref(),
-            wheel.as_ref(),
-            locks,
-            &entry?,
-            &mut attempt,
-        )?;
+    for entry in fs::read_dir(wheel)? {
+        let entry = entry?;
+        if entry.path().join("__init__.py").is_file() {
+            locks.warn_module_conflict(
+                entry
+                    .path()
+                    .strip_prefix(wheel)
+                    .expect("wheel path starts with wheel root")
+                    .as_os_str(),
+                filename,
+            );
+        }
+        clone_recursive(site_packages.as_ref(), wheel, locks, &entry, &mut attempt)?;
         count += 1;
     }
 
@@ -153,7 +198,10 @@ fn clone_recursive(
 ) -> Result<(), Error> {
     // Determine the existing and destination paths.
     let from = entry.path();
-    let to = site_packages.join(from.strip_prefix(wheel).unwrap());
+    let to = site_packages.join(
+        from.strip_prefix(wheel)
+            .expect("wheel path starts with wheel root"),
+    );
 
     trace!("Cloning {} to {}", from.display(), to.display());
 
@@ -249,17 +297,19 @@ fn copy_wheel_files(
     site_packages: impl AsRef<Path>,
     wheel: impl AsRef<Path>,
     locks: &Locks,
+    filename: &WheelFilename,
 ) -> Result<usize, Error> {
     let mut count = 0usize;
 
     // Walk over the directory.
     for entry in WalkDir::new(&wheel) {
         let entry = entry?;
         let path = entry.path();
-
         let relative = path.strip_prefix(&wheel).expect("walkdir starts with root");
         let out_path = site_packages.as_ref().join(relative);
 
+        warn_module_conflict(locks, filename, relative);
+
         if entry.file_type().is_dir() {
             fs::create_dir_all(&out_path)?;
             continue;
@@ -278,6 +328,7 @@ fn hardlink_wheel_files(
     site_packages: impl AsRef<Path>,
     wheel: impl AsRef<Path>,
     locks: &Locks,
+    filename: &WheelFilename,
 ) -> Result<usize, Error> {
     let mut attempt = Attempt::default();
     let mut count = 0usize;
@@ -286,10 +337,11 @@ fn hardlink_wheel_files(
     for entry in WalkDir::new(&wheel) {
         let entry = entry?;
         let path = entry.path();
-
         let relative = path.strip_prefix(&wheel).expect("walkdir starts with root");
         let out_path = site_packages.as_ref().join(relative);
 
+        warn_module_conflict(locks, filename, relative);
+
         if entry.file_type().is_dir() {
             fs::create_dir_all(&out_path)?;
             continue;
@@ -376,6 +428,7 @@ fn symlink_wheel_files(
     site_packages: impl AsRef<Path>,
     wheel: impl AsRef<Path>,
     locks: &Locks,
+    filename: &WheelFilename,
 ) -> Result<usize, Error> {
     let mut attempt = Attempt::default();
     let mut count = 0usize;
@@ -384,10 +437,11 @@ fn symlink_wheel_files(
     for entry in WalkDir::new(&wheel) {
         let entry = entry?;
         let path = entry.path();
-
         let relative = path.strip_prefix(&wheel).unwrap();
         let out_path = site_packages.as_ref().join(relative);
 
+        warn_module_conflict(locks, filename, relative);
+
         if entry.file_type().is_dir() {
             fs::create_dir_all(&out_path)?;
             continue;
@@ -476,7 +530,7 @@ fn symlink_wheel_files(
 fn synchronized_copy(from: &Path, to: &Path, locks: &Locks) -> std::io::Result<()> {
     // Ensure we have a lock for the directory.
     let dir_lock = {
-        let mut locks_guard = locks.0.lock().unwrap();
+        let mut locks_guard = locks.copy_dir_locks.lock().unwrap();
         locks_guard
             .entry(to.parent().unwrap().to_path_buf())
             .or_insert_with(|| Arc::new(Mutex::new(())))
@@ -492,6 +546,18 @@ fn synchronized_copy(from: &Path, to: &Path, locks: &Locks) -> std::io::Result<(
     Ok(())
 }
 
+/// Warn when a module exists in multiple packages.
+fn warn_module_conflict(locks: &Locks, filename: &WheelFilename, relative: &Path) {
+    // Check for `__init__.py` to account for namespace packages.
+    // TODO(konsti): We need to warn for overlapping namespace packages, too.
+    if relative.components().count() == 2
+        && relative.components().next_back().unwrap().as_os_str() == "__init__.py"
+    {
+        // Modules must be UTF-8, but we can skip the conversion using OsStr.
+        locks.warn_module_conflict(relative.components().next().unwrap().as_os_str(), filename);
+    }
+}
+
 #[cfg(unix)]
 fn create_symlink<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) -> std::io::Result<()> {
     fs_err::os::unix::fs::symlink(original, link)

crates/uv/tests/it/pip_install.rs

@@ -12455,3 +12455,129 @@ fn pip_install_build_dependencies_respect_locked_versions() -> Result<()> {
 
     Ok(())
 }
+
+/// Check that we warn for overlapping packages but not for correctly overlapping namespace
+/// packages.
+#[test]
+fn overlapping_packages_warning() -> Result<()> {
+    let context = TestContext::new("3.12");
+
+    let built_by_uv = context.workspace_root.join("scripts/packages/built-by-uv");
+
+    // Overlaps with `built-by-uv`
+    let also_build_by_uv = context.temp_dir.child("also-built-by-uv");
+    also_build_by_uv.child("pyproject.toml").write_str(
+        r#"
+        [project]
+        name = "also-built-by-uv"
+        version = "0.1.0"
+        requires-python = ">=3.12"
+
+        [tool.uv.build-backend]
+        module-name = "built_by_uv"
+
+        [build-system]
+        requires = ["uv_build>=0.7,<10000"]
+        build-backend = "uv_build"
+        "#,
+    )?;
+    also_build_by_uv
+        .child("src")
+        .child("built_by_uv")
+        .child("__init__.py")
+        .touch()?;
+
+    // Check that overlapping packages show a warning
+    uv_snapshot!(context.filters(), context.pip_install()
+        .arg("--no-deps")
+        .arg(&built_by_uv)
+        .arg(also_build_by_uv.path()), @r"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Resolved 2 packages in [TIME]
+    Prepared 2 packages in [TIME]
+    warning: The module `built_by_uv` 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 `built-by-uv` (v0.1.0) or `also-built-by-uv` (v0.1.0).
+    Installed 2 packages in [TIME]
+     + also-built-by-uv==0.1.0 (from file://[TEMP_DIR]/also-built-by-uv)
+     + built-by-uv==0.1.0 (from file://[WORKSPACE]/scripts/packages/built-by-uv)
+    "
+    );
+
+    // Check that correctly overlapping namespace packages don't show a warning
+    uv_snapshot!(context.pip_install()
+        .arg("--no-deps")
+        .arg("poetry")
+        .arg("poetry-core"), @r"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Resolved 2 packages in [TIME]
+    Prepared 2 packages in [TIME]
+    Installed 2 packages in [TIME]
+     + poetry==1.8.2
+     + poetry-core==1.9.0
+    "
+    );
+
+    // Check that we can uninstall even if the venv is bogus.
+    uv_snapshot!(context.filters(), context.pip_uninstall()
+        .arg("built_by_uv"), @r"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Uninstalled 1 package in [TIME]
+     - built-by-uv==0.1.0 (from file://[WORKSPACE]/scripts/packages/built-by-uv)
+    "
+    );
+    uv_snapshot!(context.filters(), context.pip_uninstall()
+        .arg("also_built_by_uv"), @r"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Uninstalled 1 package in [TIME]
+     - also-built-by-uv==0.1.0 (from file://[TEMP_DIR]/also-built-by-uv)
+    "
+    );
+
+    // Check that installing one of the packages on their own doesn't warn.
+    uv_snapshot!(context.filters(), context.pip_install()
+        .arg("--no-deps")
+        .arg(built_by_uv), @r"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Resolved 1 package in [TIME]
+    Prepared 1 package in [TIME]
+    Installed 1 package in [TIME]
+     + built-by-uv==0.1.0 (from file://[WORKSPACE]/scripts/packages/built-by-uv)
+    "
+    );
+    // Currently, we don't warn if we install them one wheel at a time.
+    uv_snapshot!(context.filters(), context.pip_install()
+        .arg("--no-deps")
+        .arg(also_build_by_uv.path()), @r"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Resolved 1 package in [TIME]
+    Prepared 1 package in [TIME]
+    Installed 1 package in [TIME]
+     + also-built-by-uv==0.1.0 (from file://[TEMP_DIR]/also-built-by-uv)
+    "
+    );
+
+    Ok(())
+}