diff --git a/Cargo.lock b/Cargo.lock index 8f22e5ae9..f2d33a690 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2176,11 +2176,14 @@ dependencies = [ "fs-err", "gourgeist", "indoc", + "insta", + "once_cell", "pep508_rs", "platform-host", "puffin-interpreter", "puffin-traits", "pyproject-toml", + "regex", "serde", "serde_json", "tar", diff --git a/crates/install-wheel-rs/src/linker.rs b/crates/install-wheel-rs/src/linker.rs index cc2a0875a..c3eed6ee3 100644 --- a/crates/install-wheel-rs/src/linker.rs +++ b/crates/install-wheel-rs/src/linker.rs @@ -61,7 +61,7 @@ pub fn install_wheel( let wheel_file_path = wheel .as_ref() .join(format!("{dist_info_prefix}.dist-info/WHEEL")); - let wheel_text = std::fs::read_to_string(&wheel_file_path)?; + let wheel_text = std::fs::read_to_string(wheel_file_path)?; parse_wheel_version(&wheel_text)?; // > 1.c If Root-Is-Purelib == ‘true’, unpack archive into purelib (site-packages). diff --git a/crates/puffin-build/Cargo.toml b/crates/puffin-build/Cargo.toml index ad0ebcdeb..fdccc786b 100644 --- a/crates/puffin-build/Cargo.toml +++ b/crates/puffin-build/Cargo.toml @@ -21,7 +21,9 @@ anyhow = { workspace = true } flate2 = { workspace = true } fs-err = { workspace = true } indoc = { workspace = true } +once_cell = { workspace = true } pyproject-toml = { workspace = true } +regex = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } tar = { workspace = true } @@ -32,3 +34,6 @@ toml = { workspace = true } tracing = { workspace = true } which = { workspace = true} zip = { workspace = true } + +[dev-dependencies] +insta = { version = "1.34.0" } diff --git a/crates/puffin-build/src/lib.rs b/crates/puffin-build/src/lib.rs index 2491ec2aa..4d0d84d08 100644 --- a/crates/puffin-build/src/lib.rs +++ b/crates/puffin-build/src/lib.rs @@ -2,6 +2,7 @@ //! //! +use std::fmt::{Display, Formatter}; use std::io; use std::io::BufRead; use std::path::{Path, PathBuf}; @@ -13,7 +14,9 @@ use flate2::read::GzDecoder; use fs_err as fs; use fs_err::{DirEntry, File}; use indoc::formatdoc; +use once_cell::sync::Lazy; use pyproject_toml::{BuildSystem, Project}; +use regex::Regex; use serde::{Deserialize, Serialize}; use tar::Archive; use tempfile::{tempdir, TempDir}; @@ -26,6 +29,14 @@ use pep508_rs::Requirement; use puffin_interpreter::{InterpreterInfo, Virtualenv}; use puffin_traits::BuildContext; +/// e.g. `pygraphviz/graphviz_wrap.c:3020:10: fatal error: graphviz/cgraph.h: No such file or directory` +static MISSING_HEADER_RE: Lazy = Lazy::new(|| { + Regex::new( + r".*\.(c|c..|h|h..):\d+:\d+: fatal error: (?
.*\.(h|h..)): No such file or directory" + ) + .unwrap() +}); + #[derive(Error, Debug)] pub enum Error { #[error(transparent)] @@ -50,14 +61,62 @@ pub enum Error { stdout: String, stderr: String, }, + /// Nudge the user towards installing the missing dev library + #[error("{message}:\n--- stdout:\n{stdout}\n--- stderr:\n{stderr}\n---")] + MissingHeader { + message: String, + stdout: String, + stderr: String, + #[source] + missing_header_cause: MissingHeaderCause, + }, +} + +#[derive(Debug, Error)] +pub struct MissingHeaderCause { + header: String, + // I've picked this over the better readable package name to make clear that you need to + // look for the build dependencies of that version or git commit respectively + package_id: String, +} + +impl Display for MissingHeaderCause { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!( + f, + "This error likely indicates that you need to install a library that provides \"{}\" for {}", + self.header, self.package_id + ) + } } impl Error { - fn from_command_output(message: String, output: &Output) -> Self { + fn from_command_output(message: String, output: &Output, package_id: &str) -> Self { + let stdout = String::from_utf8_lossy(&output.stdout).trim().to_string(); + let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + + // In the cases i've seen it was the 5th last line (see test case), 10 seems like a + // reasonable cutoff + if let Some(header) = + stderr.lines().rev().take(10).find_map(|line| { + Some(MISSING_HEADER_RE.captures(line.trim())?["header"].to_string()) + }) + { + return Self::MissingHeader { + message, + stdout, + stderr, + missing_header_cause: MissingHeaderCause { + header, + package_id: package_id.to_string(), + }, + }; + } + Self::BuildBackend { message, - stdout: String::from_utf8_lossy(&output.stdout).trim().to_string(), - stderr: String::from_utf8_lossy(&output.stderr).trim().to_string(), + stdout, + stderr, } } } @@ -124,17 +183,22 @@ pub struct SourceBuild { /// > directory created by prepare_metadata_for_build_wheel, including any unrecognized files /// > it created. metadata_directory: Option, + /// Package id such as `foo-1.2.3`, for error reporting + package_id: String, } impl SourceBuild { /// Create a virtual environment in which to build a source distribution, extracting the /// contents from an archive if necessary. + /// + /// `package_id` is for error reporting only. pub async fn setup( source: &Path, subdirectory: Option<&Path>, interpreter_info: &InterpreterInfo, build_context: &impl BuildContext, source_build_context: SourceBuildContext, + package_id: &str, ) -> Result { let temp_dir = tempdir()?; @@ -232,8 +296,14 @@ impl SourceBuild { }; if let Some(pep517_backend) = &pep517_backend { - create_pep517_build_environment(&source_tree, &venv, pep517_backend, build_context) - .await?; + create_pep517_build_environment( + &source_tree, + &venv, + pep517_backend, + build_context, + package_id, + ) + .await?; } else { if !source_tree.join("setup.py").is_file() { return Err(Error::InvalidSourceDistribution( @@ -249,6 +319,7 @@ impl SourceBuild { pep517_backend, venv, metadata_directory: None, + package_id: package_id.to_string(), }) } @@ -284,7 +355,7 @@ impl SourceBuild { return Err(Error::from_command_output( "Build backend failed to determine metadata through `prepare_metadata_for_build_wheel`".to_string(), &output, - )); + &self.package_id)); } let message = output .stdout @@ -301,6 +372,7 @@ impl SourceBuild { `prepare_metadata_for_build_wheel`: {err}" ), &output, + &self.package_id, ) })?; if message.is_empty() { @@ -336,6 +408,7 @@ impl SourceBuild { return Err(Error::from_command_output( "Failed building wheel through setup.py".to_string(), &output, + &self.package_id, )); } let dist = fs::read_dir(self.source_tree.join("dist"))?; @@ -346,7 +419,7 @@ impl SourceBuild { "Expected exactly wheel in `dist/` after invoking setup.py, found {dist_dir:?}" ), &output, - )); + &self.package_id)); }; // TODO(konstin): Faster copy such as reflink? Or maybe don't really let the user pick the target dir let wheel = wheel_dir.join(dist_wheel.file_name()); @@ -382,6 +455,7 @@ impl SourceBuild { return Err(Error::from_command_output( "Build backend failed to build wheel through `build_wheel()` ".to_string(), &output, + &self.package_id, )); } let stdout = String::from_utf8_lossy(&output.stdout); @@ -393,6 +467,7 @@ impl SourceBuild { "Build backend did not return the wheel filename through `build_wheel()`" .to_string(), &output, + &self.package_id, )); }; Ok(distribution_filename.to_string()) @@ -411,6 +486,7 @@ async fn create_pep517_build_environment( venv: &Virtualenv, pep517_backend: &Pep517Backend, build_context: &impl BuildContext, + package_id: &str, ) -> Result<(), Error> { debug!( "Calling `{}.get_requires_for_build_wheel()`", @@ -438,6 +514,7 @@ async fn create_pep517_build_environment( "Build backend failed to determine extras requires with `get_requires_for_build_wheel`" .to_string(), &output, + package_id, )); } let extra_requires = output @@ -456,6 +533,7 @@ async fn create_pep517_build_environment( `get_requires_for_build_wheel`: {err}" ), &output, + package_id, ) })?; // Some packages (such as tqdm 4.66.1) list only extra requires that have already been part of @@ -541,3 +619,69 @@ fn run_python_script( .output() .map_err(|err| Error::CommandFailed(python_interpreter.to_path_buf(), err)) } + +#[cfg(test)] +mod test { + use std::process::{ExitStatus, Output}; + + use crate::Error; + use indoc::indoc; + + #[test] + fn missing_header() { + let output = Output { + status: ExitStatus::default(), // This is wrong but `from_raw` is platform gated + stdout: indoc!(r#" + running bdist_wheel + running build + [...] + creating build/temp.linux-x86_64-cpython-39/pygraphviz + gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -DOPENSSL_NO_SSL3 -fPIC -DSWIG_PYTHON_STRICT_BYTE_CHAR -I/tmp/.tmpy6vVes/.venv/include -I/home/konsti/.pyenv/versions/3.9.18/include/python3.9 -c pygraphviz/graphviz_wrap.c -o build/temp.linux-x86_64-cpython-39/pygraphviz/graphviz_wrap.o + "# + ).as_bytes().to_vec(), + stderr: indoc!(r#" + warning: no files found matching '*.png' under directory 'doc' + warning: no files found matching '*.txt' under directory 'doc' + [...] + no previously-included directories found matching 'doc/build' + pygraphviz/graphviz_wrap.c:3020:10: fatal error: graphviz/cgraph.h: No such file or directory + 3020 | #include "graphviz/cgraph.h" + | ^~~~~~~~~~~~~~~~~~~ + compilation terminated. + error: command '/usr/bin/gcc' failed with exit code 1 + "# + ).as_bytes().to_vec(), + }; + + let err = Error::from_command_output( + "Failed building wheel through setup.py".to_string(), + &output, + "pygraphviz-1.11", + ); + assert!(matches!(err, Error::MissingHeader { .. })); + insta::assert_display_snapshot!(err, @r###" + Failed building wheel through setup.py: + --- stdout: + running bdist_wheel + running build + [...] + creating build/temp.linux-x86_64-cpython-39/pygraphviz + gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -DOPENSSL_NO_SSL3 -fPIC -DSWIG_PYTHON_STRICT_BYTE_CHAR -I/tmp/.tmpy6vVes/.venv/include -I/home/konsti/.pyenv/versions/3.9.18/include/python3.9 -c pygraphviz/graphviz_wrap.c -o build/temp.linux-x86_64-cpython-39/pygraphviz/graphviz_wrap.o + --- stderr: + warning: no files found matching '*.png' under directory 'doc' + warning: no files found matching '*.txt' under directory 'doc' + [...] + no previously-included directories found matching 'doc/build' + pygraphviz/graphviz_wrap.c:3020:10: fatal error: graphviz/cgraph.h: No such file or directory + 3020 | #include "graphviz/cgraph.h" + | ^~~~~~~~~~~~~~~~~~~ + compilation terminated. + error: command '/usr/bin/gcc' failed with exit code 1 + --- + "###); + insta::assert_display_snapshot!( + std::error::Error::source(&err).unwrap(), + @r###"This error likely indicates that you need to install a library that provides "graphviz/cgraph.h" for pygraphviz-1.11"### + ); + } +} diff --git a/crates/puffin-dev/src/build.rs b/crates/puffin-dev/src/build.rs index 9e448c27a..58b0b145f 100644 --- a/crates/puffin-dev/src/build.rs +++ b/crates/puffin-dev/src/build.rs @@ -54,7 +54,13 @@ pub(crate) async fn build(args: BuildArgs) -> Result { false, ); let wheel = build_dispatch - .build_source(&args.sdist, args.subdirectory.as_deref(), &wheel_dir) + .build_source( + &args.sdist, + args.subdirectory.as_deref(), + &wheel_dir, + // Good enough for the dev command + &args.sdist.display().to_string(), + ) .await?; Ok(wheel_dir.join(wheel)) } diff --git a/crates/puffin-dispatch/src/lib.rs b/crates/puffin-dispatch/src/lib.rs index 1f268d825..93a6cc354 100644 --- a/crates/puffin-dispatch/src/lib.rs +++ b/crates/puffin-dispatch/src/lib.rs @@ -91,9 +91,12 @@ impl BuildContext for BuildDispatch { &self.client, self, ); - let resolution_graph = resolver.resolve().await.context( - "No solution found when resolving build dependencies for source distribution:", - )?; + let resolution_graph = resolver.resolve().await.with_context(|| { + format!( + "No solution found when resolving build dependencies for source distribution: {}", + requirements.iter().map(ToString::to_string).join(", "), + ) + })?; Ok(resolution_graph.requirements()) }) } @@ -236,6 +239,7 @@ impl BuildContext for BuildDispatch { source: &'a Path, subdirectory: Option<&'a Path>, wheel_dir: &'a Path, + package_id: &'a str, ) -> Pin> + Send + 'a>> { Box::pin(async move { if self.no_build { @@ -247,6 +251,7 @@ impl BuildContext for BuildDispatch { &self.interpreter_info, self, self.source_build_context.clone(), + package_id, ) .await?; Ok(builder.build(wheel_dir)?) diff --git a/crates/puffin-installer/src/builder.rs b/crates/puffin-installer/src/builder.rs index beda9d65a..197e5ede2 100644 --- a/crates/puffin-installer/src/builder.rs +++ b/crates/puffin-installer/src/builder.rs @@ -90,6 +90,7 @@ async fn build_sdist( &distribution.sdist_file, distribution.subdirectory.as_deref(), &wheel_dir, + &distribution.remote.to_string(), ) .await?; let wheel_filename = wheel_dir.join(disk_filename); diff --git a/crates/puffin-resolver/src/distribution/source_distribution.rs b/crates/puffin-resolver/src/distribution/source_distribution.rs index 6fc2456c3..f80765d29 100644 --- a/crates/puffin-resolver/src/distribution/source_distribution.rs +++ b/crates/puffin-resolver/src/distribution/source_distribution.rs @@ -144,7 +144,12 @@ impl<'a, T: BuildContext> SourceDistributionFetcher<'a, T> { // Build the wheel. let disk_filename = self .build_context - .build_source(&sdist_file, subdirectory.as_deref(), &wheel_dir) + .build_source( + &sdist_file, + subdirectory.as_deref(), + &wheel_dir, + &distribution.to_string(), + ) .await?; // Read the metadata from the wheel. diff --git a/crates/puffin-resolver/tests/resolver.rs b/crates/puffin-resolver/tests/resolver.rs index e71da2953..2ec770c89 100644 --- a/crates/puffin-resolver/tests/resolver.rs +++ b/crates/puffin-resolver/tests/resolver.rs @@ -54,6 +54,7 @@ impl BuildContext for DummyContext { _sdist: &'a Path, _subdirectory: Option<&'a Path>, _wheel_dir: &'a Path, + _package_id: &'a str, ) -> Pin> + Send + 'a>> { panic!("The test should not need to build source distributions") } diff --git a/crates/puffin-traits/src/lib.rs b/crates/puffin-traits/src/lib.rs index f272ae9f3..a8bf9059a 100644 --- a/crates/puffin-traits/src/lib.rs +++ b/crates/puffin-traits/src/lib.rs @@ -81,10 +81,13 @@ pub trait BuildContext { /// Build a source distribution into a wheel from an archive. /// /// Returns the filename of the built wheel inside the given `wheel_dir`. + /// + /// `package_id` is for error reporting only. fn build_source<'a>( &'a self, source: &'a Path, subdirectory: Option<&'a Path>, wheel_dir: &'a Path, + package_id: &'a str, ) -> Pin> + Send + 'a>>; }