From 0425166447aad7dbe84c82d7015bb78733187c00 Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Fri, 6 Aug 2021 08:49:36 +0100 Subject: [PATCH] pyo3-build-config: improve test coverage --- pyo3-build-config/src/errors.rs | 28 ++++++ pyo3-build-config/src/impl_.rs | 165 ++++++++++++++++++++++++++++---- pyo3-build-config/src/lib.rs | 34 ++++++- 3 files changed, 205 insertions(+), 22 deletions(-) diff --git a/pyo3-build-config/src/errors.rs b/pyo3-build-config/src/errors.rs index ba740fe5e06..61e8dc37fdd 100644 --- a/pyo3-build-config/src/errors.rs +++ b/pyo3-build-config/src/errors.rs @@ -115,3 +115,31 @@ where }) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn error_report() { + let error: Result<()> = Err(Error::from("there was an internal error")) + .with_context(|| format!("failed to do {}", "something difficult")) + .context("things went wrong"); + + assert_eq!( + error + .unwrap_err() + .report() + .to_string() + .split('\n') + .collect::>(), + vec![ + "things went wrong", + "caused by:", + " - 0: failed to do something difficult", + " - 1: there was an internal error", + "" + ] + ); + } +} diff --git a/pyo3-build-config/src/impl_.rs b/pyo3-build-config/src/impl_.rs index 982a252d22a..419f1767602 100644 --- a/pyo3-build-config/src/impl_.rs +++ b/pyo3-build-config/src/impl_.rs @@ -86,7 +86,7 @@ impl InterpreterConfig { #[doc(hidden)] pub fn from_interpreter(interpreter: impl AsRef) -> Result { - let script = r#" + const SCRIPT: &str = r#" # Allow the script to run on Python 2, so that nicer error can be printed later. from __future__ import print_function @@ -133,7 +133,7 @@ print_if_set("base_prefix", base_prefix) print("executable", sys.executable) print("calcsize_pointer", struct.calcsize("P")) "#; - let output = run_python_script(interpreter.as_ref(), script)?; + let output = run_python_script(interpreter.as_ref(), SCRIPT)?; let map: HashMap = parse_script_output(&output); let shared = map["shared"].as_str() == "True"; @@ -151,16 +151,16 @@ print("calcsize_pointer", struct.calcsize("P")) let lib_name = if cfg!(windows) { default_lib_name_windows( - &version, + version, abi3, &cargo_env_var("CARGO_CFG_TARGET_ENV").unwrap(), ) } else { default_lib_name_unix( - &version, + version, implementation, map.get("ld_version").map(String::as_str), - )? + ) }; let lib_dir = if cfg!(windows) { @@ -556,6 +556,10 @@ impl BuildFlags { BuildFlag::COUNT_ALLOCS, ]; + pub fn new() -> Self { + BuildFlags(HashSet::new()) + } + fn from_config_map(config_map: &HashMap) -> Self { Self( BuildFlags::ALL @@ -874,10 +878,10 @@ fn load_cross_compile_from_sysconfigdata( abi3: is_abi3(), lib_dir: cross_compile_config.lib_dir.to_str().map(String::from), lib_name: Some(default_lib_name_unix( - &version, + version, implementation, sysconfig_data.get("LDVERSION").map(String::as_str), - )?), + )), executable: None, pointer_width, build_flags: BuildFlags::from_config_map(&sysconfig_data).fixup(version, implementation), @@ -895,7 +899,7 @@ fn windows_hardcoded_cross_compile( version, shared: true, abi3: is_abi3(), - lib_name: Some(default_lib_name_windows(&version, false, "msvc")), + lib_name: Some(default_lib_name_windows(version, false, "msvc")), lib_dir: cross_compile_config.lib_dir.to_str().map(String::from), executable: None, pointer_width: None, @@ -932,7 +936,7 @@ fn load_cross_compile_config( // This contains only the limited ABI symbols. const WINDOWS_ABI3_LIB_NAME: &str = "python3"; -fn default_lib_name_windows(version: &PythonVersion, abi3: bool, target_env: &str) -> String { +fn default_lib_name_windows(version: PythonVersion, abi3: bool, target_env: &str) -> String { if abi3 { WINDOWS_ABI3_LIB_NAME.to_owned() } else if target_env == "gnu" { @@ -944,16 +948,16 @@ fn default_lib_name_windows(version: &PythonVersion, abi3: bool, target_env: &st } fn default_lib_name_unix( - version: &PythonVersion, + version: PythonVersion, implementation: PythonImplementation, ld_version: Option<&str>, -) -> Result { +) -> String { match implementation { - PythonImplementation::CPython => match &ld_version { - Some(ld_version) => Ok(format!("python{}", ld_version)), - None => bail!("failed to configure `ld_version` when compiling for unix"), + PythonImplementation::CPython => match ld_version { + Some(ld_version) => format!("python{}", ld_version), + None => format!("python{}.{}", version.major, version.minor), }, - PythonImplementation::PyPy => Ok(format!("pypy{}-c", version.major)), + PythonImplementation::PyPy => format!("pypy{}-c", version.major), } } @@ -1050,7 +1054,7 @@ fn fixup_config_for_abi3( if let Some(version) = abi3_version { ensure!( version <= config.version, - "cannot set a mininimum Python version {} higher than the interpreter version {} \ + "cannot set a minimum Python version {} higher than the interpreter version {} \ (the minimum Python version is implied by the abi3-py3{} feature)", version, config.version, @@ -1165,7 +1169,7 @@ mod tests { #[test] fn build_flags_fixup_py36_debug() { - let mut build_flags = BuildFlags(HashSet::new()); + let mut build_flags = BuildFlags::new(); build_flags.0.insert(BuildFlag::Py_DEBUG); build_flags = build_flags.fixup( @@ -1180,7 +1184,7 @@ mod tests { #[test] fn build_flags_fixup_py37_debug() { - let mut build_flags = BuildFlags(HashSet::new()); + let mut build_flags = BuildFlags::new(); build_flags.0.insert(BuildFlag::Py_DEBUG); build_flags = build_flags.fixup(PythonVersion::PY37, PythonImplementation::CPython); @@ -1194,7 +1198,7 @@ mod tests { #[test] fn build_flags_fixup_pypy() { - let mut build_flags = BuildFlags(HashSet::new()); + let mut build_flags = BuildFlags::new(); build_flags = build_flags.fixup( PythonVersion { major: 3, minor: 6 }, @@ -1204,4 +1208,127 @@ mod tests { // PyPy always has WITH_THREAD assert!(build_flags.0.contains(&BuildFlag::WITH_THREAD)); } + + #[test] + fn parse_script_output() { + let output = "foo bar\nbar foobar\n\n"; + let map = super::parse_script_output(output); + assert_eq!(map.len(), 2); + assert_eq!(map["foo"], "bar"); + assert_eq!(map["bar"], "foobar"); + } + + #[test] + fn windows_hardcoded_cross_compile() { + let cross_config = CrossCompileConfig { + lib_dir: "C:\\some\\path".into(), + version: Some(PythonVersion { major: 3, minor: 6 }), + os: "os".into(), + arch: "arch".into(), + }; + + assert_eq!( + super::windows_hardcoded_cross_compile(cross_config).unwrap(), + InterpreterConfig { + implementation: PythonImplementation::CPython, + version: PythonVersion { major: 3, minor: 6 }, + shared: true, + abi3: false, + lib_name: Some("python36".into()), + lib_dir: Some("C:\\some\\path".into()), + executable: None, + pointer_width: None, + build_flags: BuildFlags::windows_hardcoded() + } + ); + } + + #[test] + fn default_lib_name_windows() { + assert_eq!( + super::default_lib_name_windows(PythonVersion { major: 3, minor: 6 }, false, "mvsc"), + "python36", + ); + assert_eq!( + super::default_lib_name_windows(PythonVersion { major: 3, minor: 6 }, true, "mvsc"), + "python3", + ); + assert_eq!( + super::default_lib_name_windows(PythonVersion { major: 3, minor: 6 }, false, "gnu"), + "python3.6", + ); + assert_eq!( + super::default_lib_name_windows(PythonVersion { major: 3, minor: 6 }, true, "gnu"), + "python3", + ); + } + + #[test] + fn default_lib_name_unix() { + use PythonImplementation::*; + // Defaults to pythonX.Y for CPython + assert_eq!( + super::default_lib_name_unix(PythonVersion { major: 3, minor: 6 }, CPython, None), + "python3.6", + ); + assert_eq!( + super::default_lib_name_unix(PythonVersion { major: 3, minor: 9 }, CPython, None), + "python3.9", + ); + // Can use ldversion to override for CPython + assert_eq!( + super::default_lib_name_unix( + PythonVersion { major: 3, minor: 9 }, + CPython, + Some("3.7md") + ), + "python3.7md", + ); + + // PyPy ignores ldversion + assert_eq!( + super::default_lib_name_unix(PythonVersion { major: 3, minor: 9 }, PyPy, Some("3.7md")), + "pypy3-c", + ); + } + + #[test] + fn interpreter_version_reduced_to_abi3() { + let mut config = InterpreterConfig { + abi3: true, + build_flags: BuildFlags::new(), + pointer_width: None, + executable: None, + implementation: PythonImplementation::CPython, + lib_dir: None, + lib_name: None, + shared: true, + version: PythonVersion { major: 3, minor: 7 }, + }; + + fixup_config_for_abi3(&mut config, Some(PythonVersion { major: 3, minor: 6 })).unwrap(); + assert_eq!(config.version, PythonVersion { major: 3, minor: 6 }); + } + + #[test] + fn abi3_version_cannot_be_higher_than_interpreter() { + let mut config = InterpreterConfig { + abi3: true, + build_flags: BuildFlags::new(), + pointer_width: None, + executable: None, + implementation: PythonImplementation::CPython, + lib_dir: None, + lib_name: None, + shared: true, + version: PythonVersion { major: 3, minor: 6 }, + }; + + assert!( + fixup_config_for_abi3(&mut config, Some(PythonVersion { major: 3, minor: 7 })) + .unwrap_err() + .to_string() + .contains("cannot set a minimum Python version 3.7 higher than the interpreter version 3.6") + ); + } } diff --git a/pyo3-build-config/src/lib.rs b/pyo3-build-config/src/lib.rs index be3f4093d97..b677b95473b 100644 --- a/pyo3-build-config/src/lib.rs +++ b/pyo3-build-config/src/lib.rs @@ -43,9 +43,16 @@ pub fn use_pyo3_cfgs() { /// This is currently a no-op on non-macOS platforms, however may emit additional linker arguments /// in future if deemed necessarys. pub fn add_extension_module_link_args() { - if impl_::cargo_env_var("CARGO_CFG_TARGET_OS").unwrap() == "macos" { - println!("cargo:rustc-cdylib-link-arg=-undefined"); - println!("cargo:rustc-cdylib-link-arg=dynamic_lookup"); + _add_extension_module_link_args( + &impl_::cargo_env_var("CARGO_CFG_TARGET_OS").unwrap(), + std::io::stdout(), + ) +} + +fn _add_extension_module_link_args(target_os: &str, mut writer: impl std::io::Write) { + if target_os == "macos" { + writeln!(writer, "cargo:rustc-cdylib-link-arg=-undefined").unwrap(); + writeln!(writer, "cargo:rustc-cdylib-link-arg=dynamic_lookup").unwrap(); } } @@ -152,3 +159,24 @@ pub mod pyo3_build_script_impl { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn extension_module_link_args() { + let mut buf = Vec::new(); + + // Does nothing on non-mac + _add_extension_module_link_args("windows", &mut buf); + assert_eq!(buf, Vec::new()); + + _add_extension_module_link_args("macos", &mut buf); + assert_eq!( + std::str::from_utf8(&buf).unwrap(), + "cargo:rustc-cdylib-link-arg=-undefined\n\ + cargo:rustc-cdylib-link-arg=dynamic_lookup\n" + ); + } +}