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: dont remove non pixi pypi installs #1043

Merged
merged 15 commits into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ pub const ENVIRONMENTS_DIR: &str = "envs";
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 ONE_TIME_MESSAGES_DIR: &str = "one-time-messages";

Expand Down
99 changes: 76 additions & 23 deletions src/install_pypi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::sync::Arc;

use distribution_filename::DistFilename;

use itertools::Itertools;
use miette::{miette, IntoDiagnostic, WrapErr};
use pep440_rs::Version;
use pep508_rs::VerbatimUrl;
Expand All @@ -21,7 +22,7 @@ use uv_configuration::{ConfigSettings, SetupPyStrategy};
use uv_resolver::InMemoryIndex;
use uv_types::HashStrategy;

use crate::consts::PROJECT_MANIFEST;
use crate::consts::{PIXI_UV_INSTALLER, PROJECT_MANIFEST};
use crate::lock_file::UvResolutionContext;
use crate::project::manifest::SystemRequirements;

Expand Down Expand Up @@ -80,6 +81,10 @@ struct PixiInstallPlan {
/// Any distributions that are already installed in the current environment, and are
/// _not_ necessary to satisfy the requirements.
pub extraneous: Vec<InstalledDist>,

/// 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>,
}

/// Converts our locked data to a file
Expand Down Expand Up @@ -405,6 +410,8 @@ fn whats_the_plan<'a>(
// i.e. need to be removed before being installed
let mut reinstalls = vec![];

let mut installer_mismatch = vec![];

// First decide what we need to do with any editables
for resolved_editable in editables {
match resolved_editable {
Expand Down Expand Up @@ -438,35 +445,46 @@ fn whats_the_plan<'a>(
}
}

// Filter out packages not installed by uv
let installed = site_packages.iter().filter(|dist| {
dist.installer()
.unwrap_or_default()
.is_some_and(|installer| installer == "uv")
});

// Walk over all installed packages and check if they are required
for dist in installed {
if let Some(pkg) = required_map.remove(&dist.name()) {
// Check if we need to reinstall
match need_reinstall(dist, pkg, python_version)? {
ValidateInstall::Keep => {
// Continue with the loop
continue;
}
ValidateInstall::Reinstall => {
reinstalls.push(dist.clone());
for dist in site_packages.iter() {
// Check if we require the package to be installed
let pkg = required_map.remove(&dist.name());
// Get the installer name
let installer = dist
.installer()
// Empty string if no installer or any other error
.map_or(String::new(), |f| f.unwrap_or_default());

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 {
// Check if we need to reinstall
match need_reinstall(dist, pkg, python_version)? {
ValidateInstall::Keep => {
// We are done here
continue;
}
ValidateInstall::Reinstall => {
reinstalls.push(dist.clone());
}
}
}

// Check if we need to revalidate
// In that case
// Okay so we need to re-install the package
// let's see if we need the remote or local version

// Check if we need to revalidate the package
// then we should get it from the remote
if uv_cache.must_revalidate(&pkg.name) {
remote.push(convert_to_dist(pkg, lock_file_dir));
continue;
}

// Do we have in the cache?
// Have we cached the wheel?
let wheel = registry_index
.get(&pkg.name)
.find(|(version, _)| **version == pkg.version);
Expand All @@ -475,8 +493,12 @@ fn whats_the_plan<'a>(
} else {
remote.push(convert_to_dist(pkg, lock_file_dir));
}
} else if installer != PIXI_UV_INSTALLER {
// Ignore packages that we are not managed by us
continue;
} else {
// We can uninstall
// Add to the extraneous list
// as we do manage it but have no need for it
extraneous.push(dist.clone());
}
}
Expand Down Expand Up @@ -508,6 +530,7 @@ fn whats_the_plan<'a>(
remote,
reinstalls,
extraneous,
installer_mismatch,
})
}

Expand All @@ -527,7 +550,24 @@ async fn uninstall_outdated_site_packages(site_packages: &Path) -> miette::Resul
};

if let Some(installed_dist) = installed_dist {
installed.push(installed_dist);
// If we can't get the installer, we can't be certain that we have installed it
let installer = match installed_dist.installer() {
Ok(installer) => installer,
tdejager marked this conversation as resolved.
Show resolved Hide resolved
Err(e) => {
tracing::warn!(
"could not get installer for {}: {}, will not remove distribution",
installed_dist.name(),
e
);
continue;
}
};

// Only remove if have actually installed it
// by checking the installer
if installer.unwrap_or_default() == PIXI_UV_INSTALLER {
installed.push(installed_dist);
}
}
}
}
Expand Down Expand Up @@ -810,6 +850,7 @@ pub async fn update_python_distributions(
remote,
reinstalls,
extraneous,
installer_mismatch,
} = whats_the_plan(
&python_packages,
&editables_with_temp.resolved_editables,
Expand Down Expand Up @@ -906,6 +947,17 @@ 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 @@ -950,6 +1002,7 @@ pub async fn update_python_distributions(
let start = std::time::Instant::now();
uv_installer::Installer::new(&venv)
.with_link_mode(LinkMode::default())
.with_installer_name(Some(PIXI_UV_INSTALLER.to_string()))
.with_reporter(UvReporter::new(options))
.install(&wheels)
.unwrap();
Expand Down
8 changes: 8 additions & 0 deletions tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,14 @@ impl PixiControl {
self.tmpdir.path()
}

/// Get path to default environment
pub fn default_env_path(&self) -> miette::Result<PathBuf> {
let project = self.project()?;
let env = project.environment("default");
let env = env.ok_or_else(|| miette::miette!("default environment not found"))?;
Ok(self.tmpdir.path().join(env.dir()))
}

pub fn manifest_path(&self) -> PathBuf {
self.project_path().join(consts::PROJECT_MANIFEST)
}
Expand Down
71 changes: 67 additions & 4 deletions tests/install_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::common::builders::string_from_iter;
use crate::common::package_database::{Package, PackageDatabase};
use common::{LockFileExt, PixiControl};
use pixi::cli::{run, LockFileUsageArgs};
use pixi::consts::DEFAULT_ENVIRONMENT_NAME;
use pixi::consts::{DEFAULT_ENVIRONMENT_NAME, PIXI_UV_INSTALLER};
use rattler_conda_types::Platform;
use serial_test::serial;
use tempfile::TempDir;
Expand Down Expand Up @@ -226,7 +226,7 @@ async fn pypi_reinstall_python() {
.await
.unwrap();

let prefix = pixi.project().unwrap().root().join(".pixi/envs/default");
let prefix = pixi.default_env_path().unwrap();

let cache = uv_cache::Cache::temp().unwrap();

Expand Down Expand Up @@ -272,7 +272,7 @@ async fn pypi_add_remove() {
.await
.unwrap();

let prefix = pixi.project().unwrap().root().join(".pixi/envs/default");
let prefix = pixi.default_env_path().unwrap();

let cache = uv_cache::Cache::temp().unwrap();

Expand Down Expand Up @@ -352,7 +352,7 @@ async fn install_conda_meta_history() {
// Add and update lockfile with this version of python
pixi.add("python==3.11").with_install(true).await.unwrap();

let prefix = pixi.project().unwrap().root().join(".pixi/envs/default");
let prefix = pixi.default_env_path().unwrap();
let conda_meta_history_file = prefix.join("conda-meta/history");

assert!(conda_meta_history_file.exists());
Expand Down Expand Up @@ -399,3 +399,66 @@ async fn minimal_lockfile_update_pypi() {
pep508_rs::Requirement::from_str("click==7.1.2").unwrap()
));
}

/// Create a test that installs a package with pixi
/// change the installer and see if it does not touch the package
/// then change the installer back and see if it reinstalls the package
/// with a new version
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
#[serial]
#[cfg_attr(not(feature = "slow_integration_tests"), ignore)]
async fn test_installer_name() {
let pixi = PixiControl::new().unwrap();
pixi.init().await.unwrap();

// Add and update lockfile with this version of python
pixi.add("python==3.11").with_install(true).await.unwrap();
pixi.add("click==8.0.0")
.set_type(pixi::DependencyType::PypiDependency)
.with_install(true)
.await
.unwrap();
dbg!(pixi.default_env_path().unwrap());

// Get the correct dist-info folder
let dist_info = if cfg!(not(target_os = "windows")) {
pixi.default_env_path()
.unwrap()
.join("lib/python3.11/site-packages/click-8.0.0.dist-info")
} else {
let default_env_path = pixi.default_env_path().unwrap();
default_env_path.join("Lib/site-packages/click-8.0.0.dist-info")
};
// Check that installer name is uv-pixi
assert!(dist_info.exists(), "{dist_info:?} does not exist");
let installer = dist_info.join("INSTALLER");
let installer = std::fs::read_to_string(installer).unwrap();
assert_eq!(installer, PIXI_UV_INSTALLER);

// Write a new installer name to the INSTALLER file
// so that we fake that it is not installed by pixi
std::fs::write(dist_info.join("INSTALLER"), "not-pixi").unwrap();
pixi.remove("click==8.0.0")
.with_install(true)
.set_type(pixi::DependencyType::PypiDependency)
.await
.unwrap();

// dist info folder should still exists
// and should have the old installer name
// we know that pixi did not touch the package
assert!(dist_info.exists());
let installer = dist_info.join("INSTALLER");
let installer = std::fs::read_to_string(installer).unwrap();
assert_eq!(installer, "not-pixi");

// re-manage the package by adding it, this should cause a reinstall
pixi.add("click==8.0.0")
.set_type(pixi::DependencyType::PypiDependency)
.with_install(true)
.await
.unwrap();
let installer = dist_info.join("INSTALLER");
let installer = std::fs::read_to_string(installer).unwrap();
assert_eq!(installer, PIXI_UV_INSTALLER);
}
Loading