Skip to content

Commit

Permalink
fix: try to increase rlimit (#1766)
Browse files Browse the repository at this point in the history
  • Loading branch information
baszalmstra authored Aug 12, 2024
1 parent 345e6f4 commit 30bf94c
Show file tree
Hide file tree
Showing 9 changed files with 188 additions and 57 deletions.
10 changes: 10 additions & 0 deletions Cargo.lock

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

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ requirements-txt = { git = "https://github.com/astral-sh/uv", tag = "0.2.18" }
reqwest = { version = "0.12.4", default-features = false }
reqwest-middleware = "0.3.0"
reqwest-retry = "0.5.0"
rlimit = "0.10.1"
rstest = "0.19.0"
self-replace = "1.3.7"
serde = "1.0.198"
Expand Down Expand Up @@ -246,6 +247,7 @@ reqwest = { workspace = true, features = [
] }
reqwest-middleware = { workspace = true }
reqwest-retry = { workspace = true }
rlimit = { workspace = true }
self-replace = { workspace = true }
serde = { workspace = true }
serde_ignored = { workspace = true }
Expand Down
4 changes: 3 additions & 1 deletion src/cli/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,12 +213,13 @@ pub async fn execute(args: Args) -> miette::Result<()> {

// Solve the updated project.
let LockFileDerivedData {
project: _, // We don't need the project here
lock_file,
package_cache,
uv_context,
updated_conda_prefixes,
updated_pypi_prefixes,
..
io_concurrency_limit,
} = UpdateContext::builder(&project)
.with_lock_file(unlocked_lock_file)
.with_no_install(prefix_update_config.no_install())
Expand Down Expand Up @@ -262,6 +263,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
updated_conda_prefixes,
updated_pypi_prefixes,
uv_context,
io_concurrency_limit,
};
if !prefix_update_config.no_lockfile_update {
updated_lock_file.write_to_disk()?;
Expand Down
8 changes: 5 additions & 3 deletions src/cli/global/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ use clap::Parser;
use indexmap::IndexMap;
use itertools::Itertools;
use miette::IntoDiagnostic;
use pixi_config::{self, Config, ConfigCli};
use pixi_progress::{await_in_progress, global_multi_progress};
use rattler::{
install::{DefaultProgressFormatter, IndicatifReporter, Installer},
package_cache::PackageCache,
Expand All @@ -25,9 +27,7 @@ use super::common::{
channel_name_from_prefix, find_designated_package, get_client_and_sparse_repodata,
load_package_records, BinDir, BinEnvDir,
};
use crate::{cli::has_specs::HasSpecs, prefix::Prefix};
use pixi_config::{self, Config, ConfigCli};
use pixi_progress::{await_in_progress, global_multi_progress};
use crate::{cli::has_specs::HasSpecs, prefix::Prefix, rlimit::try_increase_rlimit_to_sensible};

/// Installs the defined package in a global accessible location.
#[derive(Parser, Debug)]
Expand Down Expand Up @@ -394,6 +394,8 @@ pub(super) async fn globally_install_package(
authenticated_client: ClientWithMiddleware,
platform: Platform,
) -> miette::Result<(PrefixRecord, Vec<PathBuf>, bool)> {
try_increase_rlimit_to_sensible();

// Create the binary environment prefix where we install or update the package
let BinEnvDir(bin_prefix) = BinEnvDir::create(package_name).await?;
let prefix = Prefix::new(bin_prefix);
Expand Down
95 changes: 56 additions & 39 deletions src/environment.rs
Original file line number Diff line number Diff line change
@@ -1,36 +1,41 @@
use crate::lock_file::UvResolutionContext;
use crate::{
install_pypi,
lock_file::UpdateLockFileOptions,
prefix::Prefix,
project::{grouped_environment::GroupedEnvironment, Environment},
Project,
use std::{
collections::HashMap,
convert::identity,
io::ErrorKind,
path::{Path, PathBuf},
sync::Arc,
};

use dialoguer::theme::ColorfulTheme;
use distribution_types::{InstalledDist, Name};
use fancy_display::FancyDisplay;
use miette::{IntoDiagnostic, WrapErr};
use pixi_consts::consts;
use pixi_progress::{await_in_progress, global_multi_progress};

use crate::project::HasProjectRef;
use pixi_manifest::{EnvironmentName, FeaturesExt, SystemRequirements};
use rattler::install::{DefaultProgressFormatter, IndicatifReporter, Installer};
use pixi_progress::{await_in_progress, global_multi_progress};
use rattler::{
install::{PythonInfo, Transaction},
install::{DefaultProgressFormatter, IndicatifReporter, Installer, PythonInfo, Transaction},
package_cache::PackageCache,
};
use rattler_conda_types::{Platform, PrefixRecord, RepoDataRecord};
use rattler_lock::{PypiIndexes, PypiPackageData, PypiPackageEnvironmentData};
use reqwest_middleware::ClientWithMiddleware;
use serde::{Deserialize, Serialize};
use std::convert::identity;
use std::path::PathBuf;
use std::{collections::HashMap, io::ErrorKind, path::Path};
use tokio::sync::Semaphore;

/// Verify the location of the prefix folder is not changed so the applied prefix path is still valid.
/// Errors when there is a file system error or the path does not align with the defined prefix.
/// Returns false when the file is not present.
use crate::{
install_pypi,
lock_file::{UpdateLockFileOptions, UvResolutionContext},
prefix::Prefix,
project::{grouped_environment::GroupedEnvironment, Environment, HasProjectRef},
rlimit::try_increase_rlimit_to_sensible,
Project,
};

/// Verify the location of the prefix folder is not changed so the applied
/// prefix path is still valid. Errors when there is a file system error or the
/// path does not align with the defined prefix. Returns false when the file is
/// not present.
pub async fn verify_prefix_location_unchanged(environment_dir: &Path) -> miette::Result<()> {
let prefix_file = environment_dir
.join("conda-meta")
Expand Down Expand Up @@ -164,8 +169,9 @@ pub(crate) struct EnvironmentFile {
pub(crate) environment_name: String,
pub(crate) pixi_version: String,
}
/// Write information about the environment to a file in the environment directory.
/// This can be useful for other tools that only know the environment directory to find the original project.
/// Write information about the environment to a file in the environment
/// directory. This can be useful for other tools that only know the environment
/// directory to find the original project.
pub fn write_environment_file(
environment_dir: &Path,
env_file: EnvironmentFile,
Expand Down Expand Up @@ -241,13 +247,14 @@ impl LockFileUsage {
}
}

/// Returns the prefix associated with the given environment. If the prefix doesn't exist or is not
/// up-to-date it is updated.
/// Returns the prefix associated with the given environment. If the prefix
/// doesn't exist or is not up-to-date it is updated.
///
/// The `sparse_repo_data` is used when the lock-file is update. We pass it into this function to
/// make sure the data is not loaded twice since the repodata takes up a lot of memory and takes a
/// while to load. If `sparse_repo_data` is `None` it will be downloaded. If the lock-file is not
/// updated, the `sparse_repo_data` is ignored.
/// The `sparse_repo_data` is used when the lock-file is update. We pass it into
/// this function to make sure the data is not loaded twice since the repodata
/// takes up a lot of memory and takes a while to load. If `sparse_repo_data` is
/// `None` it will be downloaded. If the lock-file is not updated, the
/// `sparse_repo_data` is ignored.
pub async fn get_up_to_date_prefix(
environment: &Environment<'_>,
lock_file_usage: LockFileUsage,
Expand Down Expand Up @@ -298,8 +305,9 @@ pub async fn update_prefix_pypi(
lock_file_dir: &Path,
platform: Platform,
) -> miette::Result<()> {
// 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.
// 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.
Expand All @@ -310,10 +318,11 @@ pub async fn update_prefix_pypi(
}
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.
// 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.
// 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() {
Expand All @@ -322,8 +331,9 @@ pub async fn update_prefix_pypi(
}
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.
// 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);
Expand Down Expand Up @@ -364,9 +374,10 @@ 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.
/// 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![];
Expand Down Expand Up @@ -451,7 +462,8 @@ impl PythonStatus {
}
}

/// Returns the info of the current situation (e.g. after the transaction completed).
/// Returns the info of the current situation (e.g. after the transaction
/// completed).
pub fn current_info(&self) -> Option<&PythonInfo> {
match self {
PythonStatus::Changed { new, .. }
Expand All @@ -461,7 +473,8 @@ impl PythonStatus {
}
}

/// Returns the location of the python interpreter relative to the root of the prefix.
/// Returns the location of the python interpreter relative to the root of
/// the prefix.
pub fn location(&self) -> Option<&Path> {
Some(&self.current_info()?.path)
}
Expand All @@ -478,14 +491,18 @@ pub async fn update_prefix_conda(
platform: Platform,
progress_bar_message: &str,
progress_bar_prefix: &str,
io_concurrency_limit: Arc<Semaphore>,
) -> miette::Result<PythonStatus> {
// Try to increase the rlimit to a sensible value for installation.
try_increase_rlimit_to_sensible();

// Execute the operations that are returned by the solver.
let result = await_in_progress(
format!("{progress_bar_prefix}{progress_bar_message}",),
|pb| async {
Installer::new()
.with_download_client(authenticated_client)
.with_io_concurrency_limit(100)
.with_io_concurrency_semaphore(io_concurrency_limit)
.with_execute_link_scripts(false)
.with_installed_packages(installed_packages)
.with_target_platform(platform)
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub mod task;
mod uv_reporter;

mod repodata;
mod rlimit;

pub use lock_file::load_lock_file;
pub use lock_file::UpdateLockFileOptions;
Expand Down
Loading

0 comments on commit 30bf94c

Please sign in to comment.