Skip to content

Commit

Permalink
Show build output by default in uv build
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Sep 2, 2024
1 parent aebd4c8 commit 7a5bc50
Show file tree
Hide file tree
Showing 9 changed files with 617 additions and 39 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions crates/uv-build/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ uv-python = { workspace = true }
uv-types = { workspace = true }
uv-virtualenv = { workspace = true }

anstream = { workspace = true }
anyhow = { workspace = true }
fs-err = { workspace = true }
indoc = { workspace = true }
Expand Down
129 changes: 104 additions & 25 deletions crates/uv-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use rustc_hash::FxHashMap;
use serde::de::{value, SeqAccess, Visitor};
use serde::{de, Deserialize, Deserializer};
use std::ffi::OsString;
use std::fmt::Write;
use std::fmt::{Display, Formatter};
use std::io;
use std::path::{Path, PathBuf};
Expand All @@ -29,7 +30,7 @@ use distribution_types::Resolution;
use pep440_rs::Version;
use pep508_rs::PackageName;
use pypi_types::{Requirement, VerbatimParsedUrl};
use uv_configuration::{BuildKind, ConfigSettings};
use uv_configuration::{BuildKind, BuildOutput, ConfigSettings};
use uv_fs::{rename_with_retry, PythonExt, Simplified};
use uv_python::{Interpreter, PythonEnvironment};
use uv_types::{BuildContext, BuildIsolation, SourceBuildTrait};
Expand Down Expand Up @@ -93,22 +94,34 @@ pub enum Error {
#[error("Failed to run `{0}`")]
CommandFailed(PathBuf, #[source] io::Error),
#[error("{message} with {exit_code}\n--- stdout:\n{stdout}\n--- stderr:\n{stderr}\n---")]
BuildBackend {
BuildBackendOutput {
message: String,
exit_code: ExitStatus,
stdout: String,
stderr: String,
},
/// Nudge the user towards installing the missing dev library
#[error("{message} with {exit_code}\n--- stdout:\n{stdout}\n--- stderr:\n{stderr}\n---")]
MissingHeader {
MissingHeaderOutput {
message: String,
exit_code: ExitStatus,
stdout: String,
stderr: String,
#[source]
missing_header_cause: MissingHeaderCause,
},
#[error("{message} with {exit_code}")]
BuildBackend {
message: String,
exit_code: ExitStatus,
},
#[error("{message} with {exit_code}")]
MissingHeader {
message: String,
exit_code: ExitStatus,
#[source]
missing_header_cause: MissingHeaderCause,
},
#[error("Failed to build PATH for build script")]
BuildScriptPath(#[source] env::JoinPathsError),
}
Expand Down Expand Up @@ -161,6 +174,7 @@ impl Error {
fn from_command_output(
message: String,
output: &PythonRunnerOutput,
level: BuildOutput,
version_id: impl Into<String>,
) -> Self {
// In the cases I've seen it was the 5th and 3rd last line (see test case), 10 seems like a reasonable cutoff.
Expand All @@ -186,24 +200,71 @@ impl Error {
});

if let Some(missing_library) = missing_library {
return Self::MissingHeader {
return match level {
BuildOutput::Stderr => Self::MissingHeader {
message,
exit_code: output.status,
missing_header_cause: MissingHeaderCause {
missing_library,
version_id: version_id.into(),
},
},
BuildOutput::Debug => Self::MissingHeaderOutput {
message,
exit_code: output.status,
stdout: output.stdout.iter().join("\n"),
stderr: output.stderr.iter().join("\n"),
missing_header_cause: MissingHeaderCause {
missing_library,
version_id: version_id.into(),
},
},
};
}

match level {
BuildOutput::Stderr => Self::BuildBackend {
message,
exit_code: output.status,
},
BuildOutput::Debug => Self::BuildBackendOutput {
message,
exit_code: output.status,
stdout: output.stdout.iter().join("\n"),
stderr: output.stderr.iter().join("\n"),
missing_header_cause: MissingHeaderCause {
missing_library,
version_id: version_id.into(),
},
};
},
}
}
}

#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum Printer {
/// Send the build backend output to `stderr`.
Stderr,
/// Send the build backend output to `tracing`.
Debug,
}

impl From<BuildOutput> for Printer {
fn from(output: BuildOutput) -> Self {
match output {
BuildOutput::Stderr => Self::Stderr,
BuildOutput::Debug => Self::Debug,
}
}
}

Self::BuildBackend {
message,
exit_code: output.status,
stdout: output.stdout.iter().join("\n"),
stderr: output.stderr.iter().join("\n"),
impl Write for Printer {
fn write_str(&mut self, s: &str) -> std::fmt::Result {
match self {
Self::Stderr => {
anstream::eprint!("{s}");
}
Self::Debug => {
debug!("{}", s);
}
}
Ok(())
}
}

Expand Down Expand Up @@ -380,6 +441,8 @@ pub struct SourceBuild {
version_id: String,
/// Whether we do a regular PEP 517 build or an PEP 660 editable build
build_kind: BuildKind,
/// Whether to send build output to `stderr` or `tracing`, etc.
level: BuildOutput,
/// Modified PATH that contains the `venv_bin`, `user_path` and `system_path` variables in that order
modified_path: OsString,
/// Environment variables to be passed in during metadata or wheel building
Expand All @@ -405,6 +468,7 @@ impl SourceBuild {
build_isolation: BuildIsolation<'_>,
build_kind: BuildKind,
mut environment_variables: FxHashMap<OsString, OsString>,
level: BuildOutput,
concurrent_builds: usize,
) -> Result<Self, Error> {
let temp_dir = build_context.cache().environment()?;
Expand Down Expand Up @@ -488,7 +552,7 @@ impl SourceBuild {

// Create the PEP 517 build environment. If build isolation is disabled, we assume the build
// environment is already setup.
let runner = PythonRunner::new(concurrent_builds);
let runner = PythonRunner::new(concurrent_builds, level);
if build_isolation.is_isolated(package_name) {
create_pep517_build_environment(
&runner,
Expand All @@ -498,6 +562,7 @@ impl SourceBuild {
build_context,
&version_id,
build_kind,
level,
&config_settings,
&environment_variables,
&modified_path,
Expand All @@ -513,6 +578,7 @@ impl SourceBuild {
project,
venv,
build_kind,
level,
config_settings,
metadata_directory: None,
version_id,
Expand Down Expand Up @@ -698,6 +764,7 @@ impl SourceBuild {
return Err(Error::from_command_output(
format!("Build backend failed to determine metadata through `prepare_metadata_for_build_{}`", self.build_kind),
&output,
self.level,
&self.version_id,
));
}
Expand Down Expand Up @@ -827,6 +894,7 @@ impl SourceBuild {
self.build_kind, self.build_kind,
),
&output,
self.level,
&self.version_id,
));
}
Expand All @@ -839,6 +907,7 @@ impl SourceBuild {
self.build_kind, self.build_kind,
),
&output,
self.level,
&self.version_id,
));
}
Expand Down Expand Up @@ -871,6 +940,7 @@ async fn create_pep517_build_environment(
build_context: &impl BuildContext,
version_id: &str,
build_kind: BuildKind,
level: BuildOutput,
config_settings: &ConfigSettings,
environment_variables: &FxHashMap<OsString, OsString>,
modified_path: &OsString,
Expand Down Expand Up @@ -924,6 +994,7 @@ async fn create_pep517_build_environment(
return Err(Error::from_command_output(
format!("Build backend failed to determine extra requires with `build_{build_kind}()`"),
&output,
level,
version_id,
));
}
Expand All @@ -935,6 +1006,7 @@ async fn create_pep517_build_environment(
"Build backend failed to read extra requires from `get_requires_for_build_{build_kind}`: {err}"
),
&output,
level,
version_id,
)
})?;
Expand All @@ -946,6 +1018,7 @@ async fn create_pep517_build_environment(
"Build backend failed to return extra requires with `get_requires_for_build_{build_kind}`: {err}"
),
&output,
level,
version_id,
)
})?;
Expand Down Expand Up @@ -985,6 +1058,7 @@ async fn create_pep517_build_environment(
#[derive(Debug)]
struct PythonRunner {
control: Semaphore,
level: BuildOutput,
}

#[derive(Debug)]
Expand All @@ -995,10 +1069,11 @@ struct PythonRunnerOutput {
}

impl PythonRunner {
/// Create a `PythonRunner` with the provided concurrency limit.
fn new(concurrency: usize) -> PythonRunner {
PythonRunner {
/// Create a `PythonRunner` with the provided concurrency limit and output level.
fn new(concurrency: usize, level: BuildOutput) -> Self {
Self {
control: Semaphore::new(concurrency),
level,
}
}

Expand Down Expand Up @@ -1038,12 +1113,13 @@ impl PythonRunner {
let mut stdout_reader = tokio::io::BufReader::new(child.stdout.take().unwrap()).lines();
let mut stderr_reader = tokio::io::BufReader::new(child.stderr.take().unwrap()).lines();

let mut printer = Printer::from(self.level);
loop {
tokio::select! {
line = stdout_reader.next_line() => {
match line {
Ok(Some(line)) => {
debug!("{line}");
let _ = writeln!(printer, "{line}");
stdout_buf.push(line);
},
Ok(None) => break,
Expand All @@ -1053,7 +1129,7 @@ impl PythonRunner {
line = stderr_reader.next_line() => {
match line {
Ok(Some(line)) => {
debug!("{line}");
let _ = writeln!(printer, "{line}");
stderr_buf.push(line);
},
Ok(None) => break,
Expand Down Expand Up @@ -1081,9 +1157,9 @@ impl PythonRunner {
mod test {
use std::process::ExitStatus;

use indoc::indoc;

use crate::{Error, PythonRunnerOutput};
use indoc::indoc;
use uv_configuration::BuildOutput;

#[test]
fn missing_header() {
Expand Down Expand Up @@ -1114,9 +1190,10 @@ mod test {
let err = Error::from_command_output(
"Failed building wheel through setup.py".to_string(),
&output,
BuildOutput::Debug,
"pygraphviz-1.11",
);
assert!(matches!(err, Error::MissingHeader { .. }));
assert!(matches!(err, Error::MissingHeaderOutput { .. }));
// Unix uses exit status, Windows uses exit code.
let formatted = err.to_string().replace("exit status: ", "exit code: ");
insta::assert_snapshot!(formatted, @r###"
Expand Down Expand Up @@ -1166,9 +1243,10 @@ mod test {
let err = Error::from_command_output(
"Failed building wheel through setup.py".to_string(),
&output,
BuildOutput::Debug,
"pygraphviz-1.11",
);
assert!(matches!(err, Error::MissingHeader { .. }));
assert!(matches!(err, Error::MissingHeaderOutput { .. }));
// Unix uses exit status, Windows uses exit code.
let formatted = err.to_string().replace("exit status: ", "exit code: ");
insta::assert_snapshot!(formatted, @r###"
Expand Down Expand Up @@ -1211,9 +1289,10 @@ mod test {
let err = Error::from_command_output(
"Failed building wheel through setup.py".to_string(),
&output,
BuildOutput::Debug,
"pygraphviz-1.11",
);
assert!(matches!(err, Error::MissingHeader { .. }));
assert!(matches!(err, Error::MissingHeaderOutput { .. }));
// Unix uses exit status, Windows uses exit code.
let formatted = err.to_string().replace("exit status: ", "exit code: ");
insta::assert_snapshot!(formatted, @r###"
Expand Down
8 changes: 8 additions & 0 deletions crates/uv-configuration/src/build_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ impl Display for BuildKind {
}
}

#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum BuildOutput {
/// Send the build backend output to `stderr`.
Stderr,
/// Send the build backend output to `tracing`.
Debug,
}

#[derive(Debug, Default, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub struct BuildOptions {
Expand Down
4 changes: 3 additions & 1 deletion crates/uv-dispatch/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ use pypi_types::Requirement;
use uv_build::{SourceBuild, SourceBuildContext};
use uv_cache::Cache;
use uv_client::RegistryClient;
use uv_configuration::Concurrency;
use uv_configuration::{
BuildKind, BuildOptions, ConfigSettings, Constraints, IndexStrategy, Reinstall, SourceStrategy,
};
use uv_configuration::{BuildOutput, Concurrency};
use uv_distribution::DistributionDatabase;
use uv_git::GitResolver;
use uv_installer::{Installer, Plan, Planner, Preparer, SitePackages};
Expand Down Expand Up @@ -299,6 +299,7 @@ impl<'a> BuildContext for BuildDispatch<'a> {
version_id: &'data str,
dist: Option<&'data SourceDist>,
build_kind: BuildKind,
build_output: BuildOutput,
) -> Result<SourceBuild> {
let dist_name = dist.map(distribution_types::Name::name);
// Note we can only prevent builds by name for packages with names
Expand Down Expand Up @@ -330,6 +331,7 @@ impl<'a> BuildContext for BuildDispatch<'a> {
self.build_isolation,
build_kind,
self.build_extra_env_vars.clone(),
build_output,
self.concurrency.builds,
)
.boxed_local()
Expand Down
Loading

0 comments on commit 7a5bc50

Please sign in to comment.