From c26fcb646eb79c44fa1974db6563c5f0efff0bc1 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Wed, 10 Jun 2026 17:45:16 +0200 Subject: [PATCH 1/2] fix: detect site-packages clobbering of conda packages by pypi wheels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The conda-pypi clobber check compared wheel RECORD paths against conda's paths.json entries by joining regular wheel files onto the wheel's destination site-packages directory and then stripping the prefix root. That directory comes from the interpreter's *virtualenv scheme*, which is relative to the prefix (e.g. lib/python3.12/site-packages), so the strip_prefix of an absolute prefix root always failed and every regular wheel file was silently skipped: site-packages clobbering (the common case) was never detected. Only PEP 427 .data/ files, which are joined onto the interpreter's absolute scheme directories, could match — which is why script clobbering (e.g. bin/prek) worked. The relative form is exactly the prefix-relative shape conda's paths.json uses, so accept it directly (rejecting paths that escape the prefix) and keep the prefix strip for the absolute .data paths. Also rename the misleading root_scheme field to site_packages with a doc comment on the relative convention, debug-log the previously silent skip and error paths, and align the unit-test fixture with the production (relative) convention; the old fixture used an absolute path, which is why the tests did not catch this. Reproduced with a conda + pypi boltons overlap: the warning now lists the overwritten site-packages files. --- .../src/conda_pypi_clobber.rs | 206 ++++++++++++++---- crates/pixi_install_pypi/src/install_wheel.rs | 4 + crates/pixi_install_pypi/src/lib.rs | 30 ++- 3 files changed, 188 insertions(+), 52 deletions(-) diff --git a/crates/pixi_install_pypi/src/conda_pypi_clobber.rs b/crates/pixi_install_pypi/src/conda_pypi_clobber.rs index ba2805fb9e..86a5e93900 100644 --- a/crates/pixi_install_pypi/src/conda_pypi_clobber.rs +++ b/crates/pixi_install_pypi/src/conda_pypi_clobber.rs @@ -16,13 +16,13 @@ use super::install_wheel::get_wheel_info; const MAX_CLOBBER_PATHS_PER_PACKAGE: usize = 5; #[derive(Default, Debug)] -pub(crate) struct ClobberReport(BTreeMap<(String, String), Vec>); +pub(crate) struct ClobberReport(BTreeMap<(String, String), Vec>); impl ClobberReport { fn entry( &mut self, key: (String, String), - ) -> btree_map::Entry<'_, (String, String), Vec> { + ) -> btree_map::Entry<'_, (String, String), Vec> { self.0.entry(key) } @@ -30,7 +30,7 @@ impl ClobberReport { self.0.is_empty() } - pub(crate) fn keys(&self) -> btree_map::Keys<'_, (String, String), Vec> { + pub(crate) fn keys(&self) -> btree_map::Keys<'_, (String, String), Vec> { self.0.keys() } } @@ -49,7 +49,7 @@ impl fmt::Display for ClobberReport { )?; for path in paths.iter().take(MAX_CLOBBER_PATHS_PER_PACKAGE) { - writeln!(f, " - {}", path.display())?; + writeln!(f, " - {}", path.as_path().display())?; } let remaining = paths.len().saturating_sub(MAX_CLOBBER_PATHS_PER_PACKAGE); @@ -65,7 +65,7 @@ impl fmt::Display for ClobberReport { #[derive(Default, Debug)] pub(crate) struct PypiCondaClobberRegistry { /// A registry of the paths of the installed conda paths and the package names - paths_registry: AHashMap, + paths_registry: AHashMap, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -98,8 +98,20 @@ fn parse_wheel_data_path(record_path: &Path) -> Option<(WheelDataScheme, &Path)> Some((scheme, components.as_path())) } +/// The destinations wheel files are expected to be installed to. +/// +/// Note that the `.data/` destinations below model *uv's* install +/// conventions via the interpreter's scheme paths. Where those diverge from +/// the layout a conda package recorded in its `paths.json` (e.g. header +/// locations), the comparison simply finds no match and the corresponding +/// clobbering goes unreported — the check is best-effort and can only +/// under-report, never false-positive. struct WheelInstallPaths<'a> { - root_scheme: &'a Path, + /// The wheel's destination site-packages directory. This is the + /// interpreter's virtualenv scheme directory, which is *relative to the + /// prefix root* (e.g. `lib/python3.12/site-packages`), unlike the + /// absolute scheme directories below. + site_packages: &'a Path, purelib: &'a Path, platlib: &'a Path, headers: &'a Path, @@ -125,18 +137,63 @@ fn wheel_record_install_path( }; } - install_paths.root_scheme.join(record_path) + install_paths.site_packages.join(record_path) } -fn conda_relative_wheel_record_path( - install_paths: &WheelInstallPaths<'_>, - record_path: impl AsRef, - prefix_root: &Path, -) -> Option { - normalize_std(&wheel_record_install_path(install_paths, record_path)) - .strip_prefix(prefix_root) - .ok() - .map(Path::to_path_buf) +/// A normalized path in the prefix-relative form conda's `paths.json` uses, +/// e.g. `lib/python3.12/site-packages/boltons/__init__.py`. +/// +/// Conda-installed paths and wheel RECORD entries can only be compared in +/// this form; the constructors are the only way to obtain a value, so the +/// convention cannot be mixed up with absolute or differently-rooted paths. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub(crate) struct CondaPrefixPath(PathBuf); + +impl CondaPrefixPath { + /// From a conda `PrefixRecord` path, which should be prefix-relative by + /// definition. Returns `None` for a malformed (non-relative) entry: such + /// a key could never match a wheel-side path anyway, and the clobber + /// check is best-effort. + fn from_conda_record(path: PathBuf) -> Option { + if path.is_relative() { + Some(Self(path)) + } else { + tracing::debug!( + "ignoring non-relative conda paths.json entry `{}` in the clobber registry", + path.display() + ); + None + } + } + + /// Convert a wheel RECORD entry to the prefix-relative form, or `None` + /// if the file lands outside the prefix. + fn from_wheel_record( + install_paths: &WheelInstallPaths<'_>, + record_path: impl AsRef, + prefix_root: &Path, + ) -> Option { + let path = normalize_std(&wheel_record_install_path(install_paths, record_path)); + if path.is_relative() { + // Regular wheel files are joined onto the *relative* site-packages + // scheme and are therefore already prefix-relative. A normalized path + // that still starts with `..` escapes the prefix. + if path.components().next() == Some(std::path::Component::ParentDir) { + return None; + } + Some(Self(path)) + } else { + // `.data/` files are joined onto absolute scheme directories + // and need the prefix stripped. + path.strip_prefix(prefix_root) + .ok() + .map(|path| Self(path.to_path_buf())) + } + } + + fn as_path(&self) -> &Path { + &self.0 + } } impl PypiCondaClobberRegistry { @@ -146,10 +203,11 @@ impl PypiCondaClobberRegistry { let mut registry = AHashMap::with_capacity(conda_packages.len() * 50); for record in conda_packages { for path in &record.paths_data.paths { - registry.insert( - path.relative_path.clone(), - record.repodata_record.package_record.name.clone(), - ); + let Some(path) = CondaPrefixPath::from_conda_record(path.relative_path.clone()) + else { + continue; + }; + registry.insert(path, record.repodata_record.package_record.name.clone()); } } Self { @@ -172,12 +230,24 @@ impl PypiCondaClobberRegistry { for wheel in wheels { let pypi_package = wheel.name().to_string(); - let Ok(Some(whl_info)) = get_wheel_info(wheel.path(), venv) else { - continue; + let whl_info = match get_wheel_info(wheel.path(), venv) { + Ok(Some(whl_info)) => whl_info, + Ok(None) => { + tracing::debug!( + "skipping conda-clobber check for '{pypi_package}': unknown wheel layout" + ); + continue; + } + Err(err) => { + tracing::debug!( + "skipping conda-clobber check for '{pypi_package}': failed to read wheel info: {err}" + ); + continue; + } }; let install_paths = WheelInstallPaths { - root_scheme: &whl_info.1, + site_packages: &whl_info.1, purelib: venv.interpreter().purelib(), platlib: venv.interpreter().platlib(), headers: venv.interpreter().include(), @@ -201,7 +271,7 @@ impl PypiCondaClobberRegistry { // to be relatively expensive. Let's revisit if we have a user hit this in the future. for entry in whl_info.0 { let Some(path_to_clobber) = - conda_relative_wheel_record_path(&install_paths, entry.path, prefix_root) + CondaPrefixPath::from_wheel_record(&install_paths, entry.path, prefix_root) else { continue; }; @@ -226,13 +296,15 @@ mod tests { use std::path::{Path, PathBuf}; use super::{ - ClobberReport, WheelDataScheme, WheelInstallPaths, conda_relative_wheel_record_path, - parse_wheel_data_path, + ClobberReport, CondaPrefixPath, WheelDataScheme, WheelInstallPaths, parse_wheel_data_path, }; fn install_paths(prefix: &Path) -> WheelInstallPaths<'_> { WheelInstallPaths { - root_scheme: Path::new("/prefix/lib/python3.12/site-packages"), + // The site-packages scheme is *relative to the prefix root* in + // production (it comes from the interpreter's virtualenv + // scheme), unlike the absolute scheme directories below. + site_packages: Path::new("lib/python3.12/site-packages"), purelib: Path::new("/prefix/lib/python3.12/site-packages"), platlib: Path::new("/prefix/lib/python3.12/site-packages"), headers: Path::new("/prefix/include/python3.12"), @@ -241,14 +313,58 @@ mod tests { } } + /// Regression test: regular wheel files (the common case) are joined onto + /// the relative site-packages scheme and must come out in the + /// prefix-relative form conda's `paths.json` uses. Before the fix these + /// all failed an absolute `strip_prefix` and site-packages clobbering was + /// never detected. + #[test] + fn regular_record_path_is_matched_prefix_relative() { + let prefix = Path::new("/prefix"); + let install_paths = install_paths(prefix); + + assert_eq!( + CondaPrefixPath::from_wheel_record(&install_paths, "boltons/__init__.py", prefix), + Some(CondaPrefixPath(PathBuf::from( + "lib/python3.12/site-packages/boltons/__init__.py" + ))) + ); + } + + /// Both sides of the comparison are interpreter-derived: conda's + /// `paths.json` entries land in `python_site_packages_dir` (rattler asks + /// the record), and the wheel side joins onto the interpreter's own + /// sysconfig scheme. A relocated site-packages — e.g. free-threaded + /// python's `lib/python3.13t/site-packages` — therefore matches without + /// special handling; nothing in this module hardcodes the layout. + #[test] + fn relocated_site_packages_scheme_is_matched() { + let prefix = Path::new("/prefix"); + let install_paths = WheelInstallPaths { + site_packages: Path::new("lib/python3.13t/site-packages"), + purelib: Path::new("/prefix/lib/python3.13t/site-packages"), + platlib: Path::new("/prefix/lib/python3.13t/site-packages"), + headers: Path::new("/prefix/include/python3.13t"), + scripts: Path::new("/prefix/bin"), + data: prefix, + }; + + assert_eq!( + CondaPrefixPath::from_wheel_record(&install_paths, "boltons/__init__.py", prefix), + Some(CondaPrefixPath(PathBuf::from( + "lib/python3.13t/site-packages/boltons/__init__.py" + ))) + ); + } + #[test] fn record_path_escaping_site_packages_is_matched_prefix_relative() { let prefix = Path::new("/prefix"); let install_paths = install_paths(prefix); assert_eq!( - conda_relative_wheel_record_path(&install_paths, "../../../bin/prek", prefix), - Some(PathBuf::from("bin/prek")) + CondaPrefixPath::from_wheel_record(&install_paths, "../../../bin/prek", prefix), + Some(CondaPrefixPath(PathBuf::from("bin/prek"))) ); } @@ -258,7 +374,7 @@ mod tests { let install_paths = install_paths(prefix); assert_eq!( - conda_relative_wheel_record_path(&install_paths, "../../../../../bin/prek", prefix), + CondaPrefixPath::from_wheel_record(&install_paths, "../../../../../bin/prek", prefix), None ); } @@ -282,12 +398,12 @@ mod tests { let install_paths = install_paths(prefix); assert_eq!( - conda_relative_wheel_record_path( + CondaPrefixPath::from_wheel_record( &install_paths, "prek-0.4.4.data/scripts/prek", prefix ), - Some(PathBuf::from("bin/prek")) + Some(CondaPrefixPath(PathBuf::from("bin/prek"))) ); } @@ -297,32 +413,40 @@ mod tests { let install_paths = install_paths(prefix); assert_eq!( - conda_relative_wheel_record_path( + CondaPrefixPath::from_wheel_record( &install_paths, "pkg-1.0.data/purelib/module.py", prefix ), - Some(PathBuf::from("lib/python3.12/site-packages/module.py")) + Some(CondaPrefixPath(PathBuf::from( + "lib/python3.12/site-packages/module.py" + ))) ); assert_eq!( - conda_relative_wheel_record_path( + CondaPrefixPath::from_wheel_record( &install_paths, "pkg-1.0.data/platlib/native.so", prefix ), - Some(PathBuf::from("lib/python3.12/site-packages/native.so")) + Some(CondaPrefixPath(PathBuf::from( + "lib/python3.12/site-packages/native.so" + ))) ); assert_eq!( - conda_relative_wheel_record_path(&install_paths, "pkg-1.0.data/headers/pkg.h", prefix), - Some(PathBuf::from("include/python3.12/pkg.h")) + CondaPrefixPath::from_wheel_record( + &install_paths, + "pkg-1.0.data/headers/pkg.h", + prefix + ), + Some(CondaPrefixPath(PathBuf::from("include/python3.12/pkg.h"))) ); assert_eq!( - conda_relative_wheel_record_path( + CondaPrefixPath::from_wheel_record( &install_paths, "pkg-1.0.data/data/share/pkg/data.txt", prefix ), - Some(PathBuf::from("share/pkg/data.txt")) + Some(CondaPrefixPath(PathBuf::from("share/pkg/data.txt"))) ); } @@ -332,7 +456,7 @@ mod tests { report .entry(("prek".to_string(), "prek".to_string())) .or_default() - .extend((1..=7).map(|idx| PathBuf::from(format!("bin/prek-{idx}")))); + .extend((1..=7).map(|idx| CondaPrefixPath(PathBuf::from(format!("bin/prek-{idx}"))))); assert_eq!( report.to_string(), diff --git a/crates/pixi_install_pypi/src/install_wheel.rs b/crates/pixi_install_pypi/src/install_wheel.rs index f194003989..3602cf3283 100644 --- a/crates/pixi_install_pypi/src/install_wheel.rs +++ b/crates/pixi_install_pypi/src/install_wheel.rs @@ -2,6 +2,10 @@ /// https://github.com/astral-sh/uv/tree/main/crates/install-wheel-rs use csv::ReaderBuilder; +/// The RECORD entries of a wheel and the site-packages directory it installs +/// into. Note that the directory comes from the interpreter's *virtualenv +/// scheme*, which is relative to the environment prefix (e.g. +/// `lib/python3.12/site-packages`), not an absolute path. type WheelInfo = (Vec, PathBuf); /// Returns records from `.dist-info/RECORD` and determines where diff --git a/crates/pixi_install_pypi/src/lib.rs b/crates/pixi_install_pypi/src/lib.rs index ffccfe7a0d..1414c3e1ff 100644 --- a/crates/pixi_install_pypi/src/lib.rs +++ b/crates/pixi_install_pypi/src/lib.rs @@ -1168,21 +1168,29 @@ impl<'a> PyPIEnvironmentUpdater<'a> { // Verify if pypi wheels will override existing conda packages and warn if they // are - if let Ok(Some(clobber_report)) = pypi_conda_clobber.clobber_on_installation( + match pypi_conda_clobber.clobber_on_installation( all_dists.to_vec(), &setup.venv, self.config.prefix.root(), ) { - tracing::warn!("{clobber_report}"); - - // because we are removing conda packages - // we filter the ones we already warn - if !installer_mismatch.is_empty() { - installer_mismatch.retain(|name| { - !clobber_report - .keys() - .any(|(_, conda_package)| conda_package == name) - }); + Ok(Some(clobber_report)) => { + tracing::warn!("{clobber_report}"); + + // because we are removing conda packages + // we filter the ones we already warn + if !installer_mismatch.is_empty() { + installer_mismatch.retain(|name| { + !clobber_report + .keys() + .any(|(_, conda_package)| conda_package == name) + }); + } + } + Ok(None) => {} + Err(err) => { + // The check is best-effort: don't fail the installation, but + // don't fail it silently either. + tracing::debug!("could not check for conda-pypi clobbering: {err}"); } } From 76ef184e08af07debbd6ded7cf129f726d30e707 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Thu, 11 Jun 2026 11:00:01 +0200 Subject: [PATCH 2/2] refactor: derive clobber-check destinations from uv's install layout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of joining regular wheel files onto the interpreter's virtualenv scheme *template* and .data files onto individually-listed absolute scheme dirs, derive all destinations from interpreter().layout().scheme — the exact layout uv's installer writes with — relative-ized against the interpreter's own sys_prefix. Prediction and writes now share one source and cannot drift; a custom site-packages location (python_site_packages_dir) flows through because the prediction is the interpreter's real layout, not the venv template. The check no longer needs the pixi-supplied prefix root at all: everything is compared in conda's prefix-relative form, and entries escaping the prefix (leading .. after normalization, or absolute RECORD entries) are skipped. get_wheel_info now returns the wheel kind instead of a scheme directory, and the kind selects between the purelib/platlib destinations. User-tested: boltons site-packages clobber (python 3.12 and free-threaded 3.13t, which relocates site-packages) and the prek bin/prek scripts clobber all warn correctly. --- .../src/conda_pypi_clobber.rs | 298 ++++++++++-------- crates/pixi_install_pypi/src/install_wheel.rs | 32 +- crates/pixi_install_pypi/src/lib.rs | 6 +- 3 files changed, 183 insertions(+), 153 deletions(-) diff --git a/crates/pixi_install_pypi/src/conda_pypi_clobber.rs b/crates/pixi_install_pypi/src/conda_pypi_clobber.rs index 86a5e93900..fbf795edc1 100644 --- a/crates/pixi_install_pypi/src/conda_pypi_clobber.rs +++ b/crates/pixi_install_pypi/src/conda_pypi_clobber.rs @@ -11,7 +11,7 @@ use uv_python::PythonEnvironment; use ahash::AHashMap; -use super::install_wheel::get_wheel_info; +use super::install_wheel::{LibKind, get_wheel_info}; const MAX_CLOBBER_PATHS_PER_PACKAGE: usize = 5; @@ -98,29 +98,45 @@ fn parse_wheel_data_path(record_path: &Path) -> Option<(WheelDataScheme, &Path)> Some((scheme, components.as_path())) } -/// The destinations wheel files are expected to be installed to. +/// The destinations wheel files are installed to, in prefix-relative form. /// -/// Note that the `.data/` destinations below model *uv's* install -/// conventions via the interpreter's scheme paths. Where those diverge from -/// the layout a conda package recorded in its `paths.json` (e.g. header -/// locations), the comparison simply finds no match and the corresponding -/// clobbering goes unreported — the check is best-effort and can only -/// under-report, never false-positive. -struct WheelInstallPaths<'a> { - /// The wheel's destination site-packages directory. This is the - /// interpreter's virtualenv scheme directory, which is *relative to the - /// prefix root* (e.g. `lib/python3.12/site-packages`), unlike the - /// absolute scheme directories below. - site_packages: &'a Path, - purelib: &'a Path, - platlib: &'a Path, - headers: &'a Path, - scripts: &'a Path, - data: &'a Path, +/// Derived from the same layout that uv's installer writes with +/// ([`uv_python::Interpreter::layout`]), so the prediction cannot drift from +/// the actual writes. The absolute layout paths are relative-ized against +/// the interpreter's own `sys_prefix`: both values come from a single +/// interpreter probe and therefore cannot disagree about path spelling +/// (e.g. resolved symlinks) the way two independently-derived paths could. +struct WheelInstallPaths { + purelib: PathBuf, + platlib: PathBuf, + headers: PathBuf, + scripts: PathBuf, + data: PathBuf, +} + +impl WheelInstallPaths { + /// Returns `None` when the interpreter's install scheme does not live + /// inside its `sys_prefix`, which cannot happen for a conda environment. + fn from_environment(venv: &PythonEnvironment) -> Option { + let interpreter = venv.interpreter(); + let sys_prefix = interpreter.sys_prefix(); + let scheme = interpreter.layout().scheme; + let rel = |path: PathBuf| -> Option { + path.strip_prefix(sys_prefix).ok().map(Path::to_path_buf) + }; + Some(Self { + purelib: rel(scheme.purelib)?, + platlib: rel(scheme.platlib)?, + headers: rel(scheme.include)?, + scripts: rel(scheme.scripts)?, + data: rel(scheme.data)?, + }) + } } fn wheel_record_install_path( - install_paths: &WheelInstallPaths<'_>, + install_paths: &WheelInstallPaths, + kind: LibKind, record_path: impl AsRef, ) -> PathBuf { let record_path = record_path.as_ref(); @@ -137,7 +153,11 @@ fn wheel_record_install_path( }; } - install_paths.site_packages.join(record_path) + match kind { + LibKind::Plat => install_paths.platlib.join(record_path), + // `Unknown` never reaches this point: `get_wheel_info` filters it out. + LibKind::Pure | LibKind::Unknown => install_paths.purelib.join(record_path), + } } /// A normalized path in the prefix-relative form conda's `paths.json` uses, @@ -169,25 +189,21 @@ impl CondaPrefixPath { /// Convert a wheel RECORD entry to the prefix-relative form, or `None` /// if the file lands outside the prefix. fn from_wheel_record( - install_paths: &WheelInstallPaths<'_>, + install_paths: &WheelInstallPaths, + kind: LibKind, record_path: impl AsRef, - prefix_root: &Path, ) -> Option { - let path = normalize_std(&wheel_record_install_path(install_paths, record_path)); - if path.is_relative() { - // Regular wheel files are joined onto the *relative* site-packages - // scheme and are therefore already prefix-relative. A normalized path - // that still starts with `..` escapes the prefix. - if path.components().next() == Some(std::path::Component::ParentDir) { - return None; - } - Some(Self(path)) - } else { - // `.data/` files are joined onto absolute scheme directories - // and need the prefix stripped. - path.strip_prefix(prefix_root) - .ok() - .map(|path| Self(path.to_path_buf())) + let path = normalize_std(&wheel_record_install_path(install_paths, kind, record_path)); + // All install destinations are prefix-relative, so the joined path is + // too — unless the RECORD entry escapes the prefix. A normalized path + // escapes when it does not start with a normal component: a leading + // `..` is a relative escape, and a leading root or drive prefix means + // the RECORD entry was absolute-ish and replaced the base on `join` + // (note that on Windows `is_absolute()` would miss root-relative + // paths like `\abs\evil`, hence the component check). + match path.components().next() { + Some(std::path::Component::Normal(_)) => Some(Self(path)), + _ => None, } } @@ -224,13 +240,19 @@ impl PypiCondaClobberRegistry { self, wheels: Vec, venv: &PythonEnvironment, - prefix_root: &Path, ) -> miette::Result> { + let Some(install_paths) = WheelInstallPaths::from_environment(venv) else { + tracing::debug!( + "skipping conda-clobber check: the interpreter's install scheme is not inside its sys_prefix" + ); + return Ok(None); + }; + let mut clobber_report = ClobberReport::default(); for wheel in wheels { let pypi_package = wheel.name().to_string(); - let whl_info = match get_wheel_info(wheel.path(), venv) { + let (records, kind) = match get_wheel_info(wheel.path()) { Ok(Some(whl_info)) => whl_info, Ok(None) => { tracing::debug!( @@ -246,15 +268,6 @@ impl PypiCondaClobberRegistry { } }; - let install_paths = WheelInstallPaths { - site_packages: &whl_info.1, - purelib: venv.interpreter().purelib(), - platlib: venv.interpreter().platlib(), - headers: venv.interpreter().include(), - scripts: venv.scripts(), - data: venv.interpreter().data(), - }; - // Important limitation: // // This check is based on files listed in the wheel RECORD before @@ -269,9 +282,9 @@ impl PypiCondaClobberRegistry { // // We decided to postpone this to a later point, as this check is going // to be relatively expensive. Let's revisit if we have a user hit this in the future. - for entry in whl_info.0 { + for entry in records { let Some(path_to_clobber) = - CondaPrefixPath::from_wheel_record(&install_paths, entry.path, prefix_root) + CondaPrefixPath::from_wheel_record(&install_paths, kind, entry.path) else { continue; }; @@ -293,158 +306,189 @@ impl PypiCondaClobberRegistry { #[cfg(test)] mod tests { - use std::path::{Path, PathBuf}; + use std::path::PathBuf; use super::{ ClobberReport, CondaPrefixPath, WheelDataScheme, WheelInstallPaths, parse_wheel_data_path, }; + use crate::install_wheel::LibKind; - fn install_paths(prefix: &Path) -> WheelInstallPaths<'_> { + /// All destinations are prefix-relative, mirroring what + /// `WheelInstallPaths::from_environment` produces. + fn install_paths() -> WheelInstallPaths { WheelInstallPaths { - // The site-packages scheme is *relative to the prefix root* in - // production (it comes from the interpreter's virtualenv - // scheme), unlike the absolute scheme directories below. - site_packages: Path::new("lib/python3.12/site-packages"), - purelib: Path::new("/prefix/lib/python3.12/site-packages"), - platlib: Path::new("/prefix/lib/python3.12/site-packages"), - headers: Path::new("/prefix/include/python3.12"), - scripts: Path::new("/prefix/bin"), - data: prefix, + purelib: PathBuf::from("lib/python3.12/site-packages"), + platlib: PathBuf::from("lib/python3.12/site-packages"), + headers: PathBuf::from("include/python3.12"), + scripts: PathBuf::from("bin"), + data: PathBuf::from(""), } } - /// Regression test: regular wheel files (the common case) are joined onto - /// the relative site-packages scheme and must come out in the - /// prefix-relative form conda's `paths.json` uses. Before the fix these - /// all failed an absolute `strip_prefix` and site-packages clobbering was - /// never detected. + /// Regression test: regular wheel files (the common case) must come out + /// in the prefix-relative form conda's `paths.json` uses. Before the fix + /// these all failed an absolute `strip_prefix` and site-packages + /// clobbering was never detected. #[test] fn regular_record_path_is_matched_prefix_relative() { - let prefix = Path::new("/prefix"); - let install_paths = install_paths(prefix); - assert_eq!( - CondaPrefixPath::from_wheel_record(&install_paths, "boltons/__init__.py", prefix), + CondaPrefixPath::from_wheel_record( + &install_paths(), + LibKind::Pure, + "boltons/__init__.py" + ), Some(CondaPrefixPath(PathBuf::from( "lib/python3.12/site-packages/boltons/__init__.py" ))) ); } - /// Both sides of the comparison are interpreter-derived: conda's - /// `paths.json` entries land in `python_site_packages_dir` (rattler asks - /// the record), and the wheel side joins onto the interpreter's own - /// sysconfig scheme. A relocated site-packages — e.g. free-threaded - /// python's `lib/python3.13t/site-packages` — therefore matches without - /// special handling; nothing in this module hardcodes the layout. + /// The wheel kind selects between the purelib and platlib destinations. #[test] - fn relocated_site_packages_scheme_is_matched() { - let prefix = Path::new("/prefix"); + fn platlib_wheel_uses_platlib_destination() { let install_paths = WheelInstallPaths { - site_packages: Path::new("lib/python3.13t/site-packages"), - purelib: Path::new("/prefix/lib/python3.13t/site-packages"), - platlib: Path::new("/prefix/lib/python3.13t/site-packages"), - headers: Path::new("/prefix/include/python3.13t"), - scripts: Path::new("/prefix/bin"), - data: prefix, + platlib: PathBuf::from("lib/python3.12/plat-packages"), + ..install_paths() }; assert_eq!( - CondaPrefixPath::from_wheel_record(&install_paths, "boltons/__init__.py", prefix), + CondaPrefixPath::from_wheel_record(&install_paths, LibKind::Plat, "native.so"), Some(CondaPrefixPath(PathBuf::from( - "lib/python3.13t/site-packages/boltons/__init__.py" + "lib/python3.12/plat-packages/native.so" ))) ); } + /// The destinations come from the interpreter's actual layout, so a + /// relocated site-packages (cf. `python_site_packages_dir`) flows through + /// both for regular files and for relative escapes — an escape resolves + /// against the *real* location, not a hardcoded one. #[test] - fn record_path_escaping_site_packages_is_matched_prefix_relative() { - let prefix = Path::new("/prefix"); - let install_paths = install_paths(prefix); + fn relocated_site_packages_is_matched() { + let install_paths = WheelInstallPaths { + purelib: PathBuf::from("weird/place/site-packages"), + platlib: PathBuf::from("weird/place/site-packages"), + ..install_paths() + }; + + assert_eq!( + CondaPrefixPath::from_wheel_record( + &install_paths, + LibKind::Pure, + "boltons/__init__.py" + ), + Some(CondaPrefixPath(PathBuf::from( + "weird/place/site-packages/boltons/__init__.py" + ))) + ); + assert_eq!( + CondaPrefixPath::from_wheel_record(&install_paths, LibKind::Pure, "../../bla"), + Some(CondaPrefixPath(PathBuf::from("weird/bla"))) + ); + } + /// A RECORD entry may escape *site-packages* and still land inside the + /// prefix; that is a regular, comparable file (prek ships its binary + /// like this). + #[test] + fn record_path_escaping_site_packages_is_matched_prefix_relative() { assert_eq!( - CondaPrefixPath::from_wheel_record(&install_paths, "../../../bin/prek", prefix), + CondaPrefixPath::from_wheel_record( + &install_paths(), + LibKind::Pure, + "../../../bin/prek" + ), Some(CondaPrefixPath(PathBuf::from("bin/prek"))) ); } + /// Entries that escape the *prefix* (or are absolute) cannot be expressed + /// in conda's prefix-relative form and are skipped. #[test] fn record_path_outside_prefix_is_ignored() { - let prefix = Path::new("/prefix"); - let install_paths = install_paths(prefix); - assert_eq!( - CondaPrefixPath::from_wheel_record(&install_paths, "../../../../../bin/prek", prefix), + CondaPrefixPath::from_wheel_record( + &install_paths(), + LibKind::Pure, + "../../../../../bin/prek" + ), None ); + assert_eq!( + CondaPrefixPath::from_wheel_record(&install_paths(), LibKind::Pure, "/abs/evil"), + None + ); + // On Windows a path can also be root-relative (`\abs\evil`, no drive + // prefix, not `is_absolute()`) or carry a drive prefix; both must be + // rejected too. + #[cfg(windows)] + { + assert_eq!( + CondaPrefixPath::from_wheel_record(&install_paths(), LibKind::Pure, "\\abs\\evil"), + None + ); + assert_eq!( + CondaPrefixPath::from_wheel_record( + &install_paths(), + LibKind::Pure, + "C:\\abs\\evil" + ), + None + ); + } } #[test] fn parses_pep427_data_scheme_paths() { assert_eq!( - parse_wheel_data_path(Path::new("prek-0.4.4.data/scripts/prek")), - Some((WheelDataScheme::Scripts, Path::new("prek"))) + parse_wheel_data_path(std::path::Path::new("prek-0.4.4.data/scripts/prek")), + Some((WheelDataScheme::Scripts, std::path::Path::new("prek"))) ); assert_eq!( - parse_wheel_data_path(Path::new("pkg-1.0.data/purelib/module.py")), - Some((WheelDataScheme::Purelib, Path::new("module.py"))) + parse_wheel_data_path(std::path::Path::new("pkg-1.0.data/purelib/module.py")), + Some((WheelDataScheme::Purelib, std::path::Path::new("module.py"))) ); - assert_eq!(parse_wheel_data_path(Path::new("prek/__init__.py")), None); - } - - #[test] - fn wheel_data_scripts_path_is_matched_prefix_relative() { - let prefix = Path::new("/prefix"); - let install_paths = install_paths(prefix); - assert_eq!( - CondaPrefixPath::from_wheel_record( - &install_paths, - "prek-0.4.4.data/scripts/prek", - prefix - ), - Some(CondaPrefixPath(PathBuf::from("bin/prek"))) + parse_wheel_data_path(std::path::Path::new("prek/__init__.py")), + None ); } #[test] fn wheel_data_scheme_paths_are_matched_prefix_relative() { - let prefix = Path::new("/prefix"); - let install_paths = install_paths(prefix); + let install_paths = install_paths(); assert_eq!( CondaPrefixPath::from_wheel_record( &install_paths, - "pkg-1.0.data/purelib/module.py", - prefix + LibKind::Pure, + "prek-0.4.4.data/scripts/prek" ), - Some(CondaPrefixPath(PathBuf::from( - "lib/python3.12/site-packages/module.py" - ))) + Some(CondaPrefixPath(PathBuf::from("bin/prek"))) ); assert_eq!( CondaPrefixPath::from_wheel_record( &install_paths, - "pkg-1.0.data/platlib/native.so", - prefix + LibKind::Pure, + "pkg-1.0.data/purelib/module.py" ), Some(CondaPrefixPath(PathBuf::from( - "lib/python3.12/site-packages/native.so" + "lib/python3.12/site-packages/module.py" ))) ); assert_eq!( CondaPrefixPath::from_wheel_record( &install_paths, - "pkg-1.0.data/headers/pkg.h", - prefix + LibKind::Pure, + "pkg-1.0.data/headers/pkg.h" ), Some(CondaPrefixPath(PathBuf::from("include/python3.12/pkg.h"))) ); assert_eq!( CondaPrefixPath::from_wheel_record( &install_paths, - "pkg-1.0.data/data/share/pkg/data.txt", - prefix + LibKind::Pure, + "pkg-1.0.data/data/share/pkg/data.txt" ), Some(CondaPrefixPath(PathBuf::from("share/pkg/data.txt"))) ); diff --git a/crates/pixi_install_pypi/src/install_wheel.rs b/crates/pixi_install_pypi/src/install_wheel.rs index 3602cf3283..17b410315f 100644 --- a/crates/pixi_install_pypi/src/install_wheel.rs +++ b/crates/pixi_install_pypi/src/install_wheel.rs @@ -2,21 +2,15 @@ /// https://github.com/astral-sh/uv/tree/main/crates/install-wheel-rs use csv::ReaderBuilder; -/// The RECORD entries of a wheel and the site-packages directory it installs -/// into. Note that the directory comes from the interpreter's *virtualenv -/// scheme*, which is relative to the environment prefix (e.g. -/// `lib/python3.12/site-packages`), not an absolute path. -type WheelInfo = (Vec, PathBuf); +/// The RECORD entries of a wheel and whether it installs into the `purelib` +/// or `platlib` directory. +type WheelInfo = (Vec, LibKind); -/// Returns records from `.dist-info/RECORD` and determines where -/// the wheel should be installed -/// (`purelib`, `platlib` or `unknown`). +/// Returns records from `.dist-info/RECORD` and whether the wheel installs +/// into `purelib` or `platlib` (`None` for an unknown wheel kind). /// /// This function is used to detect if Python wheels will clobber already installed Conda packages -pub(crate) fn get_wheel_info( - whl: &Path, - venv: &PythonEnvironment, -) -> miette::Result> { +pub(crate) fn get_wheel_info(whl: &Path) -> miette::Result> { let dist_info_prefix = find_dist_info(whl)?; // Read the RECORD file. let mut record_file = @@ -24,14 +18,11 @@ pub(crate) fn get_wheel_info( let records = read_record_file(&mut record_file)?; let whl_kind = get_wheel_kind(whl, dist_info_prefix).unwrap_or(LibKind::Unknown); + if whl_kind == LibKind::Unknown { + return Ok(None); + } - let site_packages_dir = match whl_kind { - LibKind::Unknown => return Ok(None), - LibKind::Plat => venv.interpreter().virtualenv().platlib.clone(), - LibKind::Pure => venv.interpreter().virtualenv().purelib.clone(), - }; - - Ok(Some((records, site_packages_dir))) + Ok(Some((records, whl_kind))) } /// Find the `dist-info` directory in an unzipped wheel. @@ -105,12 +96,11 @@ use std::{ collections::HashMap, fs::File, io::{BufRead, BufReader, Read}, - path::{Path, PathBuf}, + path::Path, }; use miette::IntoDiagnostic; use serde::{Deserialize, Serialize}; -use uv_python::PythonEnvironment; /// Line in a RECORD file /// diff --git a/crates/pixi_install_pypi/src/lib.rs b/crates/pixi_install_pypi/src/lib.rs index 1414c3e1ff..c4e7b4aa22 100644 --- a/crates/pixi_install_pypi/src/lib.rs +++ b/crates/pixi_install_pypi/src/lib.rs @@ -1168,11 +1168,7 @@ impl<'a> PyPIEnvironmentUpdater<'a> { // Verify if pypi wheels will override existing conda packages and warn if they // are - match pypi_conda_clobber.clobber_on_installation( - all_dists.to_vec(), - &setup.venv, - self.config.prefix.root(), - ) { + match pypi_conda_clobber.clobber_on_installation(all_dists.to_vec(), &setup.venv) { Ok(Some(clobber_report)) => { tracing::warn!("{clobber_report}");