Skip to content

Commit

Permalink
make abi3 feature apply to config imported from PYO3_BUILD_CONFIG (
Browse files Browse the repository at this point in the history
…#4497)

* make `abi3` feature apply to config imported from `PYO3_BUILD_CONFIG`

* fix pytests for abi3 feature
  • Loading branch information
davidhewitt authored Aug 29, 2024
1 parent 4689a35 commit 5d47fc6
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 23 deletions.
1 change: 1 addition & 0 deletions newsfragments/4497.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The `abi3` feature will now override config files provided via `PYO3_BUILD_CONFIG`.
28 changes: 6 additions & 22 deletions pyo3-build-config/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<InterpreterConfig>, name: &str) -> Result<bool> {
let target = Path::new(&env::var_os("OUT_DIR").unwrap()).join(name);
Expand All @@ -29,28 +29,12 @@ fn configure(interpreter_config: Option<InterpreterConfig>, name: &str) -> Resul
}
}

/// If PYO3_CONFIG_FILE is set, copy it into the crate.
fn config_file() -> Result<Option<InterpreterConfig>> {
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
Expand Down
29 changes: 29 additions & 0 deletions pyo3-build-config/src/impl_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Result<Self>> {
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<Path>) -> Result<Self> {
let path = path.as_ref();
Expand Down
3 changes: 3 additions & 0 deletions pytests/src/pyclasses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -83,6 +85,7 @@ pub fn pyclasses(m: &Bound<'_, PyModule>) -> PyResult<()> {
m.add_class::<PyClassIter>()?;
m.add_class::<AssertingBaseClass>()?;
m.add_class::<ClassWithoutConstructor>()?;
#[cfg(any(Py_3_10, not(Py_LIMITED_API)))]
m.add_class::<ClassWithDict>()?;

Ok(())
Expand Down
7 changes: 6 additions & 1 deletion pytests/tests/test_pyclasses.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 5d47fc6

Please sign in to comment.