From 2fbe57d6299ef06f6e8110d062f556b035a128bb Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Wed, 14 Jul 2021 22:13:58 +0100 Subject: [PATCH] pyo3-build-config: improve error messaging --- build.rs | 50 +++++---- pyo3-build-config/src/errors.rs | 86 ++++++++++++++++ pyo3-build-config/src/impl_.rs | 175 ++++++++++++++++++++------------ pyo3-build-config/src/lib.rs | 2 + 4 files changed, 225 insertions(+), 88 deletions(-) create mode 100644 pyo3-build-config/src/errors.rs diff --git a/build.rs b/build.rs index 22099430..3f562c50 100644 --- a/build.rs +++ b/build.rs @@ -1,23 +1,14 @@ use std::{env, process::Command}; -use pyo3_build_config::{InterpreterConfig, PythonImplementation, PythonVersion}; - -type Result = std::result::Result>; +use pyo3_build_config::{ + bail, ensure, + errors::{Context, Result}, + InterpreterConfig, PythonImplementation, PythonVersion, +}; /// Minimum Python version PyO3 supports. const MINIMUM_SUPPORTED_VERSION: PythonVersion = PythonVersion { major: 3, minor: 6 }; -// A simple macro for returning an error. Resembles anyhow::bail. -macro_rules! bail { - ($msg: expr) => { return Err($msg.into()); }; - ($fmt: literal $($args: tt)+) => { return Err(format!($fmt $($args)+).into()); }; -} - -// A simple macro for checking a condition. Resembles anyhow::ensure. -macro_rules! ensure { - ($condition:expr, $($args: tt)+) => { if !($condition) { bail!($($args)+) } }; -} - fn ensure_python_version(interpreter_config: &InterpreterConfig) -> Result<()> { ensure!( interpreter_config.version >= MINIMUM_SUPPORTED_VERSION, @@ -87,9 +78,7 @@ fn get_rustc_link_lib(config: &InterpreterConfig) -> Result { match config.implementation { PythonImplementation::CPython => match &config.ld_version { Some(ld_version) => format!("python{}", ld_version), - None => { - return Err("failed to configure `ld_version` when compiling for unix".into()) - } + None => bail!("failed to configure `ld_version` when compiling for unix"), }, PythonImplementation::PyPy => format!("pypy{}-c", config.version.major), } @@ -157,7 +146,7 @@ fn emit_cargo_configuration(interpreter_config: &InterpreterConfig) -> Result<() if env::var_os("CARGO_FEATURE_AUTO_INITIALIZE").is_some() { if !interpreter_config.shared { - return Err(format!( + bail!( "The `auto-initialize` feature is enabled, but your python installation only supports \ embedding the Python interpreter statically. If you are attempting to run tests, or a \ binary which is okay to link dynamically, install a Python distribution which ships \ @@ -170,15 +159,14 @@ fn emit_cargo_configuration(interpreter_config: &InterpreterConfig) -> Result<() https://pyo3.rs/v{pyo3_version}/\ building_and_distribution.html#embedding-python-in-rust", pyo3_version = env::var("CARGO_PKG_VERSION").unwrap() - ) - .into()); + ); } // TODO: PYO3_CI env is a hack to workaround CI with PyPy, where the `dev-dependencies` // currently cause `auto-initialize` to be enabled in CI. // Once cargo's `resolver = "2"` is stable (~ MSRV Rust 1.52), remove this. if interpreter_config.is_pypy() && env::var_os("PYO3_CI").is_none() { - return Err("The `auto-initialize` feature is not supported with PyPy.".into()); + bail!("The `auto-initialize` feature is not supported with PyPy."); } } @@ -194,7 +182,14 @@ fn configure_pyo3() -> Result<()> { ensure_python_version(&interpreter_config)?; ensure_target_architecture(&interpreter_config)?; emit_cargo_configuration(&interpreter_config)?; - interpreter_config.to_writer(&mut std::fs::File::create(pyo3_build_config::PATH)?)?; + interpreter_config.to_writer( + &mut std::fs::File::create(pyo3_build_config::PATH).with_context(|| { + format!( + "failed to create config file at {}", + pyo3_build_config::PATH + ) + })?, + )?; interpreter_config.emit_pyo3_cfgs(); // Enable use of const generics on Rust 1.51 and greater @@ -208,7 +203,18 @@ fn configure_pyo3() -> Result<()> { fn main() { // Print out error messages using display, to get nicer formatting. if let Err(e) = configure_pyo3() { + use std::error::Error; eprintln!("error: {}", e); + let mut source = e.source(); + if source.is_some() { + eprintln!("caused by:"); + let mut index = 0; + while let Some(some_source) = source { + eprintln!(" - {}: {}", index, some_source); + source = some_source.source(); + index += 1; + } + } std::process::exit(1) } } diff --git a/pyo3-build-config/src/errors.rs b/pyo3-build-config/src/errors.rs new file mode 100644 index 00000000..8d2903a3 --- /dev/null +++ b/pyo3-build-config/src/errors.rs @@ -0,0 +1,86 @@ +/// A simple macro for returning an error. Resembles anyhow::bail. +#[macro_export] +macro_rules! bail { + ($msg: expr) => { return Err($msg.into()); }; + ($fmt: literal $($args: tt)+) => { return Err(format!($fmt $($args)+).into()); }; +} + +/// A simple macro for checking a condition. Resembles anyhow::ensure. +#[macro_export] +macro_rules! ensure { + ($condition:expr, $($args: tt)+) => { if !($condition) { bail!($($args)+) } }; +} + +/// Show warning. If needed, please extend this macro to support arguments. +#[macro_export] +macro_rules! warn { + ($msg: literal) => { + println!(concat!("cargo:warning=", $msg)); + }; +} + +/// A simple error implementation which allows chaining of errors, inspired somewhat by anyhow. +#[derive(Debug)] +pub struct Error { + value: String, + source: Option>, +} + +impl std::fmt::Display for Error { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.value) + } +} + +impl std::error::Error for Error { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + self.source.as_deref() + } +} + +impl From for Error { + fn from(value: String) -> Self { + Self { + value, + source: None, + } + } +} + +impl From<&'_ str> for Error { + fn from(value: &str) -> Self { + value.to_string().into() + } +} + +impl From for Error { + fn from(_: std::convert::Infallible) -> Self { + unreachable!() + } +} + +pub type Result = std::result::Result; + +pub trait Context { + fn context(self, message: impl Into) -> Result; + fn with_context(self, message: impl FnOnce() -> String) -> Result; +} + +impl Context for Result +where + E: std::error::Error + 'static, +{ + fn context(self, message: impl Into) -> Result { + self.map_err(|error| Error { + value: message.into(), + source: Some(Box::new(error)), + }) + } + + fn with_context(self, message: impl FnOnce() -> String) -> Result { + self.map_err(|error| Error { + value: message(), + source: Some(Box::new(error)), + }) + } +} diff --git a/pyo3-build-config/src/impl_.rs b/pyo3-build-config/src/impl_.rs index 63af429e..3186e1a1 100644 --- a/pyo3-build-config/src/impl_.rs +++ b/pyo3-build-config/src/impl_.rs @@ -11,31 +11,17 @@ use std::{ str::FromStr, }; +use crate::{ + bail, ensure, + errors::{Context, Error, Result}, + warn, +}; + /// Minimum Python version PyO3 supports. const MINIMUM_SUPPORTED_VERSION: PythonVersion = PythonVersion { major: 3, minor: 6 }; /// Maximum Python version that can be used as minimum required Python version with abi3. const ABI3_MAX_MINOR: u8 = 9; -type Result = std::result::Result>; - -// A simple macro for returning an error. Resembles anyhow::bail. -macro_rules! bail { - ($msg: expr) => { return Err($msg.into()); }; - ($fmt: literal $($args: tt)+) => { return Err(format!($fmt $($args)+).into()); }; -} - -// A simple macro for checking a condition. Resembles anyhow::ensure. -macro_rules! ensure { - ($condition:expr, $($args: tt)+) => { if !($condition) { bail!($($args)+) } }; -} - -// Show warning. If needed, please extend this macro to support arguments. -macro_rules! warn { - ($msg: literal) => { - println!(concat!("cargo:warning=", $msg)); - }; -} - /// Gets an environment variable owned by cargo. /// /// Environment variables set by cargo are expected to be valid UTF8. @@ -104,20 +90,45 @@ impl InterpreterConfig { pub fn from_reader(reader: impl Read) -> Result { let reader = BufReader::new(reader); let mut lines = reader.lines(); - let major = lines.next().ok_or("expected major version")??.parse()?; - let minor = lines.next().ok_or("expected minor version")??.parse()?; - let libdir = parse_option_string(lines.next().ok_or("expected libdir")??)?; - let shared = lines.next().ok_or("expected shared")??.parse()?; - let abi3 = lines.next().ok_or("expected abi3")??.parse()?; - let ld_version = parse_option_string(lines.next().ok_or("expected ld_version")??)?; - let base_prefix = parse_option_string(lines.next().ok_or("expected base_prefix")??)?; - let executable = parse_option_string(lines.next().ok_or("expected executable")??)?; - let calcsize_pointer = - parse_option_string(lines.next().ok_or("expected calcsize_pointer")??)?; - let implementation = lines.next().ok_or("expected implementation")??.parse()?; + + macro_rules! parse_line { + ($value:literal) => { + lines + .next() + .ok_or(concat!("reached end of config when reading ", $value))? + .context(concat!("failed to read ", $value, " from config"))? + .parse() + .context(concat!("failed to parse ", $value, " from config")) + }; + } + + macro_rules! parse_option_line { + ($value:literal) => { + parse_option_string( + lines + .next() + .ok_or(concat!("reached end of config when reading ", $value))? + .context(concat!("failed to read ", $value, " from config"))?, + ) + .context(concat!("failed to parse ", $value, "from config")) + }; + } + + let major = parse_line!("major version")?; + let minor = parse_line!("minor version")?; + let libdir = parse_option_line!("libdir")?; + let shared = parse_line!("shared")?; + let abi3 = parse_line!("abi3")?; + let ld_version = parse_option_line!("ld_version")?; + let base_prefix = parse_option_line!("base_prefix")?; + let executable = parse_option_line!("executable")?; + let calcsize_pointer = parse_option_line!("calcsize_pointer")?; + let implementation = parse_line!("implementation")?; let mut build_flags = BuildFlags(HashSet::new()); for line in lines { - build_flags.0.insert(line?.parse()?); + build_flags + .0 + .insert(line.context("failed to read flag from config")?.parse()?); } Ok(InterpreterConfig { version: PythonVersion { major, minor }, @@ -135,39 +146,52 @@ impl InterpreterConfig { #[doc(hidden)] pub fn to_writer(&self, mut writer: impl Write) -> Result<()> { - macro_rules! writeln_option { - ($writer:expr, $opt:expr) => { - match &$opt { - Some(value) => writeln!($writer, "{}", value), - None => writeln!($writer, "null"), - } + macro_rules! write_line { + ($value:expr) => { + writeln!(writer, "{}", $value).context(concat!( + "failed to write ", + stringify!($value), + " to config" + )) }; } - writeln!(writer, "{}", self.version.major)?; - writeln!(writer, "{}", self.version.minor)?; - writeln_option!(writer, self.libdir)?; - writeln!(writer, "{}", self.shared)?; - writeln!(writer, "{}", self.abi3)?; - writeln_option!(writer, self.ld_version)?; - writeln_option!(writer, self.base_prefix)?; - writeln_option!(writer, self.executable)?; - writeln_option!(writer, self.calcsize_pointer)?; - writeln!(writer, "{}", self.implementation)?; + + macro_rules! write_option_line { + ($opt:expr) => { + match &$opt { + Some(value) => writeln!(writer, "{}", value), + None => writeln!(writer, "null"), + } + .context(concat!( + "failed to write ", + stringify!($value), + " to config" + )) + }; + } + + write_line!(self.version.major)?; + write_line!(self.version.minor)?; + write_option_line!(self.libdir)?; + write_line!(self.shared)?; + write_line!(self.abi3)?; + write_option_line!(self.ld_version)?; + write_option_line!(self.base_prefix)?; + write_option_line!(self.executable)?; + write_option_line!(self.calcsize_pointer)?; + write_line!(self.implementation)?; for flag in &self.build_flags.0 { - writeln!(writer, "{}", flag)?; + write_line!(flag)?; } Ok(()) } } -fn parse_option_string(string: String) -> Result> -where - ::Err: std::error::Error + 'static, -{ +fn parse_option_string(string: String) -> Result, ::Err> { if string == "null" { Ok(None) } else { - Ok(string.parse().map(Some)?) + string.parse().map(Some) } } @@ -203,12 +227,12 @@ impl Display for PythonImplementation { } impl FromStr for PythonImplementation { - type Err = Box; + type Err = Error; fn from_str(s: &str) -> Result { match s { "CPython" => Ok(PythonImplementation::CPython), "PyPy" => Ok(PythonImplementation::PyPy), - _ => bail!("Invalid interpreter: {}", s), + _ => bail!("unknown interpreter: {}", s), } } } @@ -219,7 +243,9 @@ fn is_abi3() -> bool { trait GetPrimitive { fn get_bool(&self, key: &str) -> Result; - fn get_numeric(&self, key: &str) -> Result; + fn get_numeric(&self, key: &str) -> Result + where + T::Err: std::error::Error + 'static; } impl GetPrimitive for HashMap { @@ -235,11 +261,14 @@ impl GetPrimitive for HashMap { } } - fn get_numeric(&self, key: &str) -> Result { + fn get_numeric(&self, key: &str) -> Result + where + T::Err: std::error::Error + 'static, + { self.get(key) .ok_or(format!("{} is not defined", key))? .parse::() - .map_err(|_| format!("Could not parse value of {}", key).into()) + .with_context(|| format!("Could not parse value of {}", key)) } } @@ -334,8 +363,8 @@ impl Display for BuildFlag { } impl FromStr for BuildFlag { - type Err = Box; - fn from_str(s: &str) -> Result { + type Err = std::convert::Infallible; + fn from_str(s: &str) -> Result { match s { "WITH_THREAD" => Ok(BuildFlag::WITH_THREAD), "Py_DEBUG" => Ok(BuildFlag::Py_DEBUG), @@ -474,7 +503,12 @@ fn parse_script_output(output: &str) -> HashMap { /// python executable and library. Here it is read and added to a script to extract only what is /// necessary. This necessitates a python interpreter for the host machine to work. fn parse_sysconfigdata(config_path: impl AsRef) -> Result> { - let mut script = fs::read_to_string(config_path)?; + let mut script = fs::read_to_string(config_path.as_ref()).with_context(|| { + format!( + "failed to read config from {}", + config_path.as_ref().display() + ) + })?; script += r#" print("version_major", build_time_vars["VERSION"][0]) # 3 print("version_minor", build_time_vars["VERSION"][2]) # E.g., 8 @@ -751,7 +785,8 @@ fn run_python_script(interpreter: &Path, script: &str) -> Result { err ), Ok(ok) if !ok.status.success() => bail!("Python script failed"), - Ok(ok) => Ok(String::from_utf8(ok.stdout)?), + Ok(ok) => Ok(String::from_utf8(ok.stdout) + .context("failed to parse Python script output as utf-8")?), } } @@ -859,8 +894,12 @@ print("calcsize_pointer", struct.calcsize("P")) }; let version = PythonVersion { - major: map["version_major"].parse()?, - minor: map["version_minor"].parse()?, + major: map["version_major"] + .parse() + .context("failed to parse major version")?, + minor: map["version_minor"] + .parse() + .context("failed to parse minor version")?, }; let implementation = map["implementation"].parse()?; @@ -874,7 +913,11 @@ print("calcsize_pointer", struct.calcsize("P")) ld_version: map.get("ld_version").cloned(), base_prefix: map.get("base_prefix").cloned(), executable: map.get("executable").cloned(), - calcsize_pointer: Some(map["calcsize_pointer"].parse()?), + calcsize_pointer: Some( + map["calcsize_pointer"] + .parse() + .context("failed to parse calcsize_pointer")?, + ), build_flags: BuildFlags::from_interpreter(interpreter)?.fixup(version, implementation), }) } diff --git a/pyo3-build-config/src/lib.rs b/pyo3-build-config/src/lib.rs index 980d6689..a0fe3baa 100644 --- a/pyo3-build-config/src/lib.rs +++ b/pyo3-build-config/src/lib.rs @@ -14,6 +14,8 @@ //! //! For examples of how to use these attributes, [see PyO3's guide](https://pyo3.rs/main/building_and_distribution/multiple_python_versions.html). +#[doc(hidden)] +pub mod errors; mod impl_; use once_cell::sync::OnceCell;