Skip to content
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

feat: warn on pypi conda clobbering #1353

Merged
merged 16 commits into from
May 17, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ clap-verbosity-flag = "2.2.0"
clap_complete = "4.5.2"
console = { version = "0.15.8", features = ["windows-console-colors"] }
crossbeam-channel = "0.5.12"
csv = "1.3.0"
deno_task_shell = "0.16.0"
dialoguer = "0.11.0"
dirs = "5.0.1"
Expand Down
1 change: 1 addition & 0 deletions src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub const SOLVE_GROUP_ENVIRONMENTS_DIR: &str = "solve-group-envs";
pub const PYPI_DEPENDENCIES: &str = "pypi-dependencies";
pub const TASK_CACHE_DIR: &str = "task-cache-v0";
pub const PIXI_UV_INSTALLER: &str = "uv-pixi";
pub const CONDA_INSTALLER: &str = "conda";

pub const ONE_TIME_MESSAGES_DIR: &str = "one-time-messages";

Expand Down
130 changes: 108 additions & 22 deletions src/install_pypi.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::install_wheel::get_wheel_info;
use crate::prefix::Prefix;
use crate::project::manifest::pypi_options::PypiOptions;
use crate::uv_reporter::{UvReporter, UvReporterOptions};
Expand Down Expand Up @@ -31,11 +32,11 @@ use distribution_types::{
};
use install_wheel_rs::linker::LinkMode;

use rattler_conda_types::{Platform, RepoDataRecord};
use rattler_conda_types::{Platform, PrefixRecord, RepoDataRecord};
use rattler_lock::{PypiPackageData, PypiPackageEnvironmentData, UrlOrPath};

use std::collections::HashMap;
use std::path::Path;
use std::collections::{HashMap, HashSet};
use std::path::{Path, PathBuf};
use std::str::FromStr;
use std::time::Duration;

Expand Down Expand Up @@ -83,7 +84,7 @@ struct PixiInstallPlan {

/// Keep track of any packages that have been re-installed because of installer mismatch
/// we can warn the user later that this has happened
pub installer_mismatch: Vec<PackageName>,
pub installer_mismatch: Vec<String>,
}

/// Converts our locked data to a file
Expand Down Expand Up @@ -444,6 +445,10 @@ fn whats_the_plan<'a>(
}
}

// Used to verify if there are any additional .dist-info installed
// that should be removed
let required_map_copy = required_map.clone();

// Walk over all installed packages and check if they are required
for dist in site_packages.iter() {
// Check if we require the package to be installed
Expand All @@ -454,13 +459,15 @@ fn whats_the_plan<'a>(
// Empty string if no installer or any other error
.map_or(String::new(), |f| f.unwrap_or_default());

if required_map_copy.contains_key(&dist.name()) && installer != PIXI_UV_INSTALLER {
// We are managing the package but something else has installed a version
// let's re-install to make sure that we have the **correct** version
reinstalls.push(dist.clone());
installer_mismatch.push(dist.name().to_string());
}

if let Some(pkg) = pkg {
if installer != PIXI_UV_INSTALLER {
// We are managing the package but something else has installed a version
// let's re-install to make sure that we have the **correct** version
reinstalls.push(dist.clone());
installer_mismatch.push(dist.name().clone());
} else {
if installer == PIXI_UV_INSTALLER {
// Check if we need to reinstall
match need_reinstall(dist, pkg, python_version)? {
ValidateInstall::Keep => {
Expand Down Expand Up @@ -789,7 +796,7 @@ pub async fn update_python_distributions(
remote,
reinstalls,
extraneous,
installer_mismatch,
mut installer_mismatch,
} = whats_the_plan(
&python_packages,
&editables_with_temp.resolved_editables,
Expand All @@ -800,6 +807,19 @@ pub async fn update_python_distributions(
lock_file_dir,
)?;

// Determine the currently installed conda packages.
let installed_packages = prefix
.find_installed_packages(None)
.await
.with_context(|| {
format!(
"failed to determine the currently installed packages for {}",
prefix.root().display()
)
})?;

let pypi_conda_clobber = PypiCondaClobberRegistry::with_conda_packages(&installed_packages);

// Nothing to do.
if remote.is_empty() && local.is_empty() && reinstalls.is_empty() && extraneous.is_empty() {
let s = if python_packages.len() == 1 { "" } else { "s" };
Expand Down Expand Up @@ -886,17 +906,6 @@ pub async fn update_python_distributions(
wheels
};

// Notify the user if there are any packages that were re-installed because they were installed
// by a different installer.
if !installer_mismatch.is_empty() {
let packages = installer_mismatch
.iter()
.map(|name| name.to_string())
.join(", ");
// BREAK(0.20.1): change this into a warning in a future release
tracing::info!("These pypi-packages were re-installed because they were previously installed by a different installer but are currently managed by pixi: \n\t{packages}")
}

// Remove any unnecessary packages.
if !extraneous.is_empty() || !reinstalls.is_empty() {
let start = std::time::Instant::now();
Expand Down Expand Up @@ -932,11 +941,40 @@ pub async fn update_python_distributions(

// Install the resolved distributions.
let wheels = wheels.into_iter().chain(local).collect::<Vec<_>>();

// Verify if pypi wheels will override existent conda packages
nichmor marked this conversation as resolved.
Show resolved Hide resolved
// and warn if they are
if let Ok(Some(clobber_packages)) =
pypi_conda_clobber.clobber_on_instalation(wheels.clone(), &venv)
{
let packages_names = clobber_packages.iter().join(", ");

tracing::warn!("These conda-packages will be overridden by pypi: \n\t{packages_names}");

// because we are removing conda packages
// we filter the ones we already warn
if !installer_mismatch.is_empty() {
installer_mismatch.retain(|name| !packages_names.contains(name));
}
}

if !installer_mismatch.is_empty() {
// Notify the user if there are any packages that were re-installed because they were installed
// by a different installer.
let packages = installer_mismatch
.iter()
.map(|name| name.to_string())
.join(", ");
// BREAK(0.20.1): change this into a warning in a future release
tracing::warn!("These pypi-packages were re-installed because they were previously installed by a different installer but are currently managed by pixi: \n\t{packages}")
}

let options = UvReporterOptions::new()
.with_length(wheels.len() as u64)
.with_capacity(wheels.len() + 30)
.with_starting_tasks(wheels.iter().map(|d| format!("{}", d.name())))
.with_top_level_message("Installing distributions");

if !wheels.is_empty() {
let start = std::time::Instant::now();
uv_installer::Installer::new(&venv)
Expand All @@ -959,3 +997,51 @@ pub async fn update_python_distributions(

Ok(())
}

#[derive(Default, Debug)]
struct PypiCondaClobberRegistry {
paths_registry: HashMap<PathBuf, rattler_conda_types::PackageName>,
}

impl PypiCondaClobberRegistry {
pub fn with_conda_packages(conda_packages: &[PrefixRecord]) -> Self {
let mut registry = HashMap::default();
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(),
);
}
}
Self {
paths_registry: registry,
}
}

pub fn clobber_on_instalation(
self,
wheels: Vec<CachedDist>,
venv: &PythonEnvironment,
) -> miette::Result<Option<HashSet<String>>> {
let mut clobber_packages: HashSet<String> = HashSet::default();

for wheel in wheels {
let Ok(Some(whl_info)) = get_wheel_info(wheel.path(), venv) else {
continue;
};

for entry in whl_info.0 {
let path_to_clobber = whl_info.1.join(entry.path);

if let Some(name) = self.paths_registry.get(&path_to_clobber) {
clobber_packages.insert(name.as_normalized().to_string());
}
}
}
if clobber_packages.is_empty() {
return Ok(None);
}
Ok(Some(clobber_packages))
}
}
tdejager marked this conversation as resolved.
Show resolved Hide resolved