Skip to content

Commit

Permalink
Fix/concurrent env setup (#226)
Browse files Browse the repository at this point in the history
So this allows for concurrent metadata setup and wheel building.
Basically it checks that a setup request is already in-flight or not.

I have some ideas to make this generic, so that we can potentially
re-use, but seeing as we are going to revisit the caching and so forth,
I think it's best to wait :)
  • Loading branch information
tdejager authored Feb 16, 2024
1 parent d92a751 commit a6ab742
Show file tree
Hide file tree
Showing 4 changed files with 252 additions and 84 deletions.
12 changes: 7 additions & 5 deletions crates/rattler_installs_packages/src/utils/streaming_or_local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,14 @@ impl StreamingOrLocal {
match self {
StreamingOrLocal::Streaming(mut streaming) => streaming.read_to_end(bytes).await,
StreamingOrLocal::Local(mut local) => {
match tokio::task::spawn_blocking(move || {
let read_to_end = move || {
let mut bytes = Vec::new();
local.read_to_end(&mut bytes).map(|_| bytes)
})
.map_err(JoinError::try_into_panic)
.await
};

match tokio::task::spawn_blocking(read_to_end)
.map_err(JoinError::try_into_panic)
.await
{
Ok(Ok(result)) => {
*bytes = result;
Expand All @@ -71,7 +73,7 @@ impl StreamingOrLocal {
Ok(Err(err)) => Err(err),
// Resume the panic on the main task
Err(Ok(panic)) => std::panic::resume_unwind(panic),
Err(Err(_)) => Err(std::io::ErrorKind::Interrupted.into()),
Err(Err(_)) => Err(io::ErrorKind::Interrupted.into()),
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,11 @@ impl BuildEnvironment {
/// This uses the `GetRequiresForBuildWheel` entry point of the build backend.
/// this might not be available for all build backends.
/// and it can also return an empty list of requirements.
fn get_extra_requirements(&self) -> Result<HashSet<Requirement>, WheelBuildError> {
let output = self.run_command("GetRequiresForBuildWheel")?;
fn get_extra_requirements(
&self,
output_dir: &Path,
) -> Result<HashSet<Requirement>, WheelBuildError> {
let output = self.run_command("GetRequiresForBuildWheel", output_dir)?;
if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr);
return Err(WheelBuildError::Error(stderr.to_string()));
Expand Down Expand Up @@ -217,7 +220,10 @@ impl BuildEnvironment {
wheel_builder: &WheelBuilder,
) -> Result<(), WheelBuildError> {
// Get extra requirements if any
let extra_requirements = self.get_extra_requirements()?;
// Because we are using the build environment to get the extra requirements
// and we should only do this once
// its fine to use the work_dir as the output_dir
let extra_requirements = self.get_extra_requirements(&self.work_dir())?;

// Combine previous requirements with extra requirements
let combined_requirements = HashSet::from_iter(self.build_requirements.iter().cloned())
Expand Down Expand Up @@ -281,7 +287,11 @@ impl BuildEnvironment {
}

/// Run a command in the build environment
pub(crate) fn run_command(&self, stage: &str) -> Result<Output, WheelBuildError> {
pub(crate) fn run_command(
&self,
stage: &str,
output_dir: &Path,
) -> Result<Output, WheelBuildError> {
// We modify the environment of the user
// so that we can use the scripts directory to run the build frontend
// e.g maturin depends on an executable in the scripts directory
Expand Down Expand Up @@ -315,7 +325,6 @@ impl BuildEnvironment {
if self.clean_env {
base_command.env_clear();
}
let work_dir = self.work_dir.path();
base_command
.current_dir(&self.package_dir)
// pass all env variables defined by user
Expand All @@ -324,10 +333,10 @@ impl BuildEnvironment {
// it will overwritten by more actual one
.env("PATH", path_var)
// Script to run
.arg(work_dir.join("build_frontend.py"))
.arg(self.work_dir().join("build_frontend.py"))
// The working directory to use
// will contain the output of the build
.arg(work_dir.as_path())
.arg(output_dir)
// Build system entry point
.arg(&self.entry_point)
// Building Wheel or Metadata
Expand Down
Loading

0 comments on commit a6ab742

Please sign in to comment.