Skip to content

Commit

Permalink
Merge branch 'main' into fix/install-tool-req-coalescing
Browse files Browse the repository at this point in the history
  • Loading branch information
nichmor committed Dec 2, 2024
2 parents 80fcb2e + 51d9920 commit 8405fd0
Show file tree
Hide file tree
Showing 73 changed files with 2,223 additions and 1,665 deletions.
41 changes: 28 additions & 13 deletions crates/pixi_build_frontend/src/tool/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ impl ToolCache {
// modify the current entry to the pending entry.
// this is an atomic operation
// because who holds the entry holds mutable access.

entry.insert(PendingOrFetched::Pending(Arc::downgrade(&sender)));

sender
Expand All @@ -85,6 +84,8 @@ impl ToolCache {

// Explicitly drop the entry, so we don't block any other tasks.
drop(entry);
// Drop the sender
drop(sender);

return match receiver.recv().await {
Ok(tool) => Ok(tool),
Expand Down Expand Up @@ -117,6 +118,7 @@ impl ToolCache {
// Let's start by installing tool. If an error occurs we immediately return
// the error. This will drop the sender and all other waiting tasks will
// receive an error.
// Installation happens outside the critical section
let tool = Arc::new(context.install(&spec, channel_config).await?);

// Store the fetched files in the entry.
Expand All @@ -136,7 +138,9 @@ mod tests {
use std::{collections::HashMap, path::PathBuf, sync::Arc};

use pixi_config::Config;
use rattler_conda_types::{ChannelConfig, MatchSpec, NamedChannelOrUrl, ParseStrictness};
use rattler_conda_types::{
ChannelConfig, MatchSpec, NamedChannelOrUrl, ParseStrictness, Platform,
};
use reqwest_middleware::ClientWithMiddleware;
use tokio::sync::{Barrier, Mutex};

Expand Down Expand Up @@ -171,34 +175,42 @@ mod tests {
Ok(isolated_tool)
}
}
/// Returns the platform to use for the tool cache. Python is not yet
/// available for win-arm64 so we use win-64.
pub fn compatible_target_platform() -> Platform {
match Platform::current() {
Platform::WinArm64 => Platform::Win64,
platform => platform,
}
}

#[tokio::test]
async fn test_tool_cache() {
// let mut cache = ToolCache::new();
let mut config = Config::default();
config.default_channels = vec![NamedChannelOrUrl::Name("conda-forge".to_string())];
let config = Config::for_tests();

let auth_client = ClientWithMiddleware::default();
let channel_config = ChannelConfig::default_with_root_dir(PathBuf::new());
let channel_config = config.global_channel_config();

let tool_context = ToolContext::builder()
.with_platform(compatible_target_platform())
.with_client(auth_client.clone())
.with_gateway(config.gateway(auth_client))
.build();

let tool_spec = IsolatedToolSpec {
specs: vec![MatchSpec::from_str("cowpy", ParseStrictness::Strict).unwrap()],
command: "cowpy".into(),
channels: Vec::from([NamedChannelOrUrl::Name("conda-forge".to_string())]),
specs: vec![MatchSpec::from_str("bat", ParseStrictness::Strict).unwrap()],
command: "bat".into(),
channels: config.default_channels.clone(),
};

let tool = tool_context
.instantiate(ToolSpec::Isolated(tool_spec), &channel_config)
.instantiate(ToolSpec::Isolated(tool_spec), channel_config)
.await
.unwrap();

let exec = tool.as_executable().unwrap();

exec.command().arg("hello").spawn().unwrap();
exec.command().arg("--version").spawn().unwrap();
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
Expand Down Expand Up @@ -275,7 +287,6 @@ mod tests {
async fn test_handle_a_failure() {
// This test verifies that during the installation of a tool, if an error occurs
// the tool is not cached and the next request will try to install the tool again.

// A test installer that will fail on the first request.
#[derive(Default, Clone)]
struct TestInstaller {
Expand All @@ -293,9 +304,13 @@ mod tests {
*count += 1;

if count == &1 {
dbg!("a");
miette::bail!("error on first request");
// tokio::time::sleep(tokio::time::Duration::from_secs(1)).await;
// panic!("tim is right!");
}

dbg!("b");
let isolated_tool =
IsolatedTool::new(spec.command.clone(), PathBuf::new(), HashMap::default());
Ok(isolated_tool)
Expand Down Expand Up @@ -324,7 +339,7 @@ mod tests {

// Let's imitate that we have 4 requests to install a tool
// we will use a barrier to ensure all tasks start at the same time.
let num_tasks = 4;
let num_tasks = 2;
let barrier = Arc::new(Barrier::new(num_tasks));
let mut handles = Vec::new();

Expand Down
29 changes: 26 additions & 3 deletions crates/pixi_build_frontend/src/tool/installer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ pub struct ToolContextBuilder {
client: ClientWithMiddleware,
cache_dir: PathBuf,
cache: ToolCache,
platform: Platform,
}

impl Default for ToolContextBuilder {
Expand All @@ -53,9 +54,18 @@ impl ToolContextBuilder {
client: ClientWithMiddleware::default(),
cache_dir: pixi_config::get_cache_dir().expect("we should have a cache dir"),
cache: ToolCache::default(),
platform: Platform::current(),
}
}

/// Set the platform to install tools for. This is usually the current
/// platform but could also be a compatible platform. For instance if the
/// current platform is win-arm64, the compatible platform could be win-64.
pub fn with_platform(mut self, platform: Platform) -> Self {
self.platform = platform;
self
}

/// Set the gateway for the tool context.
pub fn with_gateway(mut self, gateway: Gateway) -> Self {
self.gateway = Some(gateway);
Expand Down Expand Up @@ -92,6 +102,7 @@ impl ToolContextBuilder {
cache_dir: self.cache_dir,
client: self.client,
cache: self.cache,
platform: self.platform,
gateway,
}
}
Expand All @@ -100,7 +111,6 @@ impl ToolContextBuilder {
/// The tool context,
/// containing client, channels and gateway configuration
/// that will be used to resolve and install tools.
#[derive(Default)]
pub struct ToolContext {
// Authentication client to use for fetching repodata.
pub client: ClientWithMiddleware,
Expand All @@ -110,13 +120,24 @@ pub struct ToolContext {
pub gateway: Gateway,
// The cache to use for the tools.
pub cache: ToolCache,
/// The platform to install tools for. This is usually the current platform
/// but could also be a compatible platform. For instance if the current
/// platform is win-arm64, the compatible platform could be win-64.
pub platform: Platform,
}

impl Default for ToolContext {
fn default() -> Self {
Self::builder().build()
}
}

impl Debug for ToolContext {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("ToolContext")
.field("client", &self.client)
.field("cache_dir", &self.cache_dir)
.field("platform", &self.platform)
.finish()
}
}
Expand Down Expand Up @@ -181,7 +202,7 @@ impl ToolInstaller for ToolContext {
.gateway
.query(
channels.clone(),
[Platform::current(), Platform::NoArch],
[self.platform, Platform::NoArch],
spec.specs.clone(),
)
.recursive(true)
Expand Down Expand Up @@ -209,6 +230,7 @@ impl ToolInstaller for ToolContext {
spec.command.clone(),
spec.specs.clone(),
channels.iter().map(|c| c.base_url.to_string()).collect(),
self.platform,
);

let cached_dir = self
Expand Down Expand Up @@ -248,6 +270,7 @@ impl ToolInstaller for ToolContext {

// Install the environment
Installer::new()
.with_target_platform(self.platform)
.with_download_client(self.client.clone())
.with_package_cache(PackageCache::new(
self.cache_dir
Expand All @@ -259,7 +282,7 @@ impl ToolInstaller for ToolContext {

// Get the activation scripts
let activator =
Activator::from_path(&cached_dir, ShellEnum::default(), Platform::current()).unwrap();
Activator::from_path(&cached_dir, ShellEnum::default(), self.platform).unwrap();

let activation_scripts = activator
.run_activation(ActivationVariables::from_env().unwrap_or_default(), None)
Expand Down
Loading

0 comments on commit 8405fd0

Please sign in to comment.