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/concurrent env setup #226

Merged
merged 6 commits into from
Feb 16, 2024
Merged
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
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
@@ -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;
@@ -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()),
}
}
}
Original file line number Diff line number Diff line change
@@ -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()));
@@ -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())
@@ -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
@@ -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
@@ -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
Loading