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

fix: don't need a python interpreter when not having pypi dependencies. #1366

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
95 changes: 93 additions & 2 deletions src/environment.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::consts::PIXI_UV_INSTALLER;
use crate::lock_file::UvResolutionContext;
use crate::progress::await_in_progress;
use crate::project::grouped_environment::GroupedEnvironmentName;
Expand All @@ -16,6 +17,7 @@ use crate::{
Project,
};
use dialoguer::theme::ColorfulTheme;
use distribution_types::{InstalledDist, Name};
use indexmap::IndexMap;
use miette::{IntoDiagnostic, WrapErr};
use rattler::{
Expand Down Expand Up @@ -257,7 +259,47 @@ pub async fn update_prefix_pypi(
lock_file_dir: &Path,
platform: Platform,
) -> miette::Result<()> {
// Remove python packages from a previous python distribution if the python version changed.
// If we have changed interpreter, we need to uninstall all site-packages from the old interpreter
// We need to do this before the pypi prefix update, because that requires a python interpreter.
let python_info = match status {
// If the python interpreter is removed, we need to uninstall all `pixi-uv` site-packages.
// And we don't need to continue with the rest of the pypi prefix update.
PythonStatus::Removed { old } => {
let site_packages_path = prefix.root().join(&old.site_packages_path);
if site_packages_path.exists() {
uninstall_outdated_site_packages(&site_packages_path).await?;
}
return Ok(());
}
// If the python interpreter is changed, we need to uninstall all site-packages from the old interpreter.
// And we continue the function to update the pypi packages.
PythonStatus::Changed { old, new } => {
// In windows the site-packages path stays the same, so we don't need to uninstall the site-packages ourselves.
if old.site_packages_path != new.site_packages_path {
let site_packages_path = prefix.root().join(&old.site_packages_path);
if site_packages_path.exists() {
uninstall_outdated_site_packages(&site_packages_path).await?;
}
}
new
}
// If the python interpreter is unchanged, and there are no pypi packages to install, we need to remove the site-packages.
// And we don't need to continue with the rest of the pypi prefix update.
PythonStatus::Unchanged(info) | PythonStatus::Added { new: info } => {
if pypi_records.is_empty() {
let site_packages_path = prefix.root().join(&info.site_packages_path);
if site_packages_path.exists() {
uninstall_outdated_site_packages(&site_packages_path).await?;
}
return Ok(());
}
info
}
// We can skip the pypi prefix update if there is not python interpreter in the environment.
PythonStatus::DoesNotExist => {
return Ok(());
}
};

// Install and/or remove python packages
progress::await_in_progress(
Expand All @@ -271,7 +313,7 @@ pub async fn update_prefix_pypi(
prefix,
conda_records,
pypi_records,
status,
&python_info.path,
system_requirements,
uv_context,
pypi_options,
Expand All @@ -283,6 +325,55 @@ pub async fn update_prefix_pypi(
.await
}

/// If the python interpreter is outdated, we need to uninstall all outdated site packages.
/// from the old interpreter.
/// TODO: optimize this by recording the installation of the site-packages to check if this is needed.
async fn uninstall_outdated_site_packages(site_packages: &Path) -> miette::Result<()> {
// Check if the old interpreter is outdated
let mut installed = vec![];
for entry in std::fs::read_dir(site_packages).into_diagnostic()? {
let entry = entry.into_diagnostic()?;
if entry.file_type().into_diagnostic()?.is_dir() {
let path = entry.path();

let installed_dist = InstalledDist::try_from_path(&path);
let Ok(installed_dist) = installed_dist else {
continue;
};

if let Some(installed_dist) = 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,
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);
}
}
}
}

// Uninstall all packages in old site-packages directory
for dist_info in installed {
let _summary = uv_installer::uninstall(&dist_info)
.await
.expect("uninstallation of old site-packages failed");
}

Ok(())
}

#[derive(Clone, Debug)]
pub enum PythonStatus {
/// The python interpreter changed from `old` to `new`.
Expand Down
65 changes: 2 additions & 63 deletions src/install_pypi.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::environment::PythonStatus;
use crate::prefix::Prefix;
use crate::project::manifest::pypi_options::PypiOptions;
use crate::uv_reporter::{UvReporter, UvReporterOptions};
Expand Down Expand Up @@ -534,54 +533,6 @@ fn whats_the_plan<'a>(
})
}

/// If the python interpreter is outdated, we need to uninstall all outdated site packages.
/// from the old interpreter.
async fn uninstall_outdated_site_packages(site_packages: &Path) -> miette::Result<()> {
// Check if the old interpreter is outdated
let mut installed = vec![];
for entry in std::fs::read_dir(site_packages).into_diagnostic()? {
let entry = entry.into_diagnostic()?;
if entry.file_type().into_diagnostic()?.is_dir() {
let path = entry.path();

let installed_dist = InstalledDist::try_from_path(&path);
let Ok(installed_dist) = installed_dist else {
continue;
};

if let Some(installed_dist) = 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,
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);
}
}
}
}

// Uninstall all packages in old site-packages directory
for dist_info in installed {
let _summary = uv_installer::uninstall(&dist_info)
.await
.expect("uninstallation of old site-packages failed");
}

Ok(())
}

/// Result of resolving editables
/// we need to store the temp_dir until the install is finished
struct EditablesWithTemp {
Expand Down Expand Up @@ -731,26 +682,14 @@ pub async fn update_python_distributions(
prefix: &Prefix,
conda_package: &[RepoDataRecord],
python_packages: &[CombinedPypiPackageData],
status: &PythonStatus,
python_interpreter_path: &Path,
system_requirements: &SystemRequirements,
uv_context: UvResolutionContext,
pypi_options: &PypiOptions,
environment_variables: &HashMap<String, String>,
platform: Platform,
) -> miette::Result<()> {
let start = std::time::Instant::now();
let Some(python_info) = status.current_info() else {
// No python interpreter in the environment, so there is nothing to do here.
return Ok(());
};

// If we have changed interpreter, we need to uninstall all site-packages from the old interpreter
if let PythonStatus::Changed { old, new: _ } = status {
let site_packages_path = prefix.root().join(&old.site_packages_path);
if site_packages_path.exists() {
uninstall_outdated_site_packages(&site_packages_path).await?;
}
}

// Determine the current environment markers.
let python_record = conda_package
Expand Down Expand Up @@ -788,7 +727,7 @@ pub async fn update_python_distributions(
let in_memory_index = InMemoryIndex::default();
let config_settings = ConfigSettings::default();

let python_location = prefix.root().join(&python_info.path);
let python_location = prefix.root().join(python_interpreter_path);
let interpreter = Interpreter::query(&python_location, &uv_context.cache).into_diagnostic()?;

tracing::debug!("[Install] Using Python Interpreter: {:?}", interpreter);
Expand Down
8 changes: 6 additions & 2 deletions src/lock_file/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ impl<'p> LockFileDerivedData<'p> {
let pypi_records = self.pypi_records(environment, platform).unwrap_or_default();

// No `uv` support for WASM right now
// TODO - figure out if we can create the `uv` context more lazily
if platform.arch() == Some(Arch::Wasm32) {
return Ok(prefix);
}
Expand Down Expand Up @@ -1524,7 +1523,12 @@ async fn spawn_solve_pypi_task(
let python_path = python_status
.location()
.map(|path| prefix.root().join(path))
.ok_or_else(|| miette::miette!("missing python interpreter from environment"))?;
.ok_or_else(|| {
miette::miette!(
help = "Use `pixi add python` to install the latest python interpreter.",
"missing python interpreter from environment"
)
})?;

let start = Instant::now();

Expand Down
14 changes: 12 additions & 2 deletions tests/install_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,17 @@ async fn install_locked() {
let pixi = PixiControl::new().unwrap();
pixi.init().await.unwrap();
// Add and update lockfile with this version of python
pixi.add("python==3.10.0").await.unwrap();
let python_version = if cfg!(target_os = "macos") && cfg!(target_arch = "aarch64") {
"python==3.10.0"
} else if cfg!(target_os = "windows") {
// Abusing this test to also test the `add` function of older version of python
// Before this wasn't possible because uv queried the python interpreter, even without pypi dependencies.
"python==3.6.0"
} else {
"python==2.7.15"
};

pixi.add(python_version).await.unwrap();

// Add new version of python only to the manifest
pixi.add("python==3.9.0")
Expand All @@ -139,7 +149,7 @@ async fn install_locked() {
assert!(lock.contains_match_spec(
DEFAULT_ENVIRONMENT_NAME,
Platform::current(),
"python==3.10.0"
python_version
));

// After an install with lockfile update the locked install should succeed.
Expand Down
Loading