From 9d1a0578b4b20a325cbeef29aa87f002b0882017 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Wed, 28 Aug 2024 08:42:14 +0100 Subject: [PATCH 1/2] make `abi3` feature apply to config imported from `PYO3_BUILD_CONFIG` --- newsfragments/4497.changed.md | 1 + pyo3-build-config/build.rs | 28 ++++++---------------------- pyo3-build-config/src/impl_.rs | 29 +++++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 22 deletions(-) create mode 100644 newsfragments/4497.changed.md diff --git a/newsfragments/4497.changed.md b/newsfragments/4497.changed.md new file mode 100644 index 00000000000..594b6d373e5 --- /dev/null +++ b/newsfragments/4497.changed.md @@ -0,0 +1 @@ +The `abi3` feature will now override config files provided via `PYO3_BUILD_CONFIG`. diff --git a/pyo3-build-config/build.rs b/pyo3-build-config/build.rs index 309a78c87da..a6e767edcf0 100644 --- a/pyo3-build-config/build.rs +++ b/pyo3-build-config/build.rs @@ -12,7 +12,7 @@ mod errors; use std::{env, path::Path}; use errors::{Context, Result}; -use impl_::{env_var, make_interpreter_config, InterpreterConfig}; +use impl_::{make_interpreter_config, InterpreterConfig}; fn configure(interpreter_config: Option, name: &str) -> Result { let target = Path::new(&env::var_os("OUT_DIR").unwrap()).join(name); @@ -29,28 +29,12 @@ fn configure(interpreter_config: Option, name: &str) -> Resul } } -/// If PYO3_CONFIG_FILE is set, copy it into the crate. -fn config_file() -> Result> { - if let Some(path) = env_var("PYO3_CONFIG_FILE") { - let path = Path::new(&path); - println!("cargo:rerun-if-changed={}", path.display()); - // Absolute path is necessary because this build script is run with a cwd different to the - // original `cargo build` instruction. - ensure!( - path.is_absolute(), - "PYO3_CONFIG_FILE must be an absolute path" - ); - - let interpreter_config = InterpreterConfig::from_path(path) - .context("failed to parse contents of PYO3_CONFIG_FILE")?; - Ok(Some(interpreter_config)) - } else { - Ok(None) - } -} - fn generate_build_configs() -> Result<()> { - let configured = configure(config_file()?, "pyo3-build-config-file.txt")?; + // If PYO3_CONFIG_FILE is set, copy it into the crate. + let configured = configure( + InterpreterConfig::from_pyo3_config_file_env().transpose()?, + "pyo3-build-config-file.txt", + )?; if configured { // Don't bother trying to find an interpreter on the host system diff --git a/pyo3-build-config/src/impl_.rs b/pyo3-build-config/src/impl_.rs index c8f68864727..e254a26cd27 100644 --- a/pyo3-build-config/src/impl_.rs +++ b/pyo3-build-config/src/impl_.rs @@ -402,6 +402,35 @@ print("ext_suffix", get_config_var("EXT_SUFFIX")) }) } + /// Import an externally-provided config file. + /// + /// The `abi3` features, if set, may apply an `abi3` constraint to the Python version. + #[allow(dead_code)] // only used in build.rs + pub(super) fn from_pyo3_config_file_env() -> Option> { + cargo_env_var("PYO3_CONFIG_FILE").map(|path| { + let path = Path::new(&path); + println!("cargo:rerun-if-changed={}", path.display()); + // Absolute path is necessary because this build script is run with a cwd different to the + // original `cargo build` instruction. + ensure!( + path.is_absolute(), + "PYO3_CONFIG_FILE must be an absolute path" + ); + + let mut config = InterpreterConfig::from_path(path) + .context("failed to parse contents of PYO3_CONFIG_FILE")?; + // If the abi3 feature is enabled, the minimum Python version is constrained by the abi3 + // feature. + // + // TODO: abi3 is a property of the build mode, not the interpreter. Should this be + // removed from `InterpreterConfig`? + config.abi3 |= is_abi3(); + config.fixup_for_abi3_version(get_abi3_version())?; + + Ok(config) + }) + } + #[doc(hidden)] pub fn from_path(path: impl AsRef) -> Result { let path = path.as_ref(); From 744a283a2f7ff604ffd08baf046c9986aa0fc3b8 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Thu, 29 Aug 2024 06:03:49 +0100 Subject: [PATCH 2/2] fix pytests for abi3 feature --- pytests/src/pyclasses.rs | 3 +++ pytests/tests/test_pyclasses.py | 7 ++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/pytests/src/pyclasses.rs b/pytests/src/pyclasses.rs index f7e4681af70..e499b436395 100644 --- a/pytests/src/pyclasses.rs +++ b/pytests/src/pyclasses.rs @@ -66,9 +66,11 @@ impl AssertingBaseClass { #[pyclass] struct ClassWithoutConstructor; +#[cfg(any(Py_3_10, not(Py_LIMITED_API)))] #[pyclass(dict)] struct ClassWithDict; +#[cfg(any(Py_3_10, not(Py_LIMITED_API)))] #[pymethods] impl ClassWithDict { #[new] @@ -83,6 +85,7 @@ pub fn pyclasses(m: &Bound<'_, PyModule>) -> PyResult<()> { m.add_class::()?; m.add_class::()?; m.add_class::()?; + #[cfg(any(Py_3_10, not(Py_LIMITED_API)))] m.add_class::()?; Ok(()) diff --git a/pytests/tests/test_pyclasses.py b/pytests/tests/test_pyclasses.py index e91e75fa58a..a1424fc75aa 100644 --- a/pytests/tests/test_pyclasses.py +++ b/pytests/tests/test_pyclasses.py @@ -89,7 +89,12 @@ def test_no_constructor_defined_propagates_cause(cls: Type): def test_dict(): - d = pyclasses.ClassWithDict() + try: + ClassWithDict = pyclasses.ClassWithDict + except AttributeError: + pytest.skip("not defined using abi3 < 3.9") + + d = ClassWithDict() assert d.__dict__ == {} d.foo = 42