From 1a2848c5e3ce3755653fce3e898c6bfe9a2fff73 Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Sun, 30 Aug 2020 15:30:56 +0100 Subject: [PATCH] Fix `cargo test` with `extension-module` feature. --- CHANGELOG.md | 1 + build.rs | 128 ++++++++++++++++++++++++++++------------------- guide/src/faq.md | 17 ++++--- 3 files changed, 86 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e75f883efb..12127a2d2bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Remove `PYO3_CROSS_INCLUDE_DIR` environment variable and the associated C header parsing functionality. ### Fixed +- Fix `cargo test` with `extension-module` feature. (Requires `cargo +nightly -Zextra-link-arg test` for now.) #[1123](https://github.com/PyO3/pyo3/pull/1123) - Remove FFI definition `PyCFunction_ClearFreeList` for Python 3.9 and later. [#1425](https://github.com/PyO3/pyo3/pull/1425) - `PYO3_CROSS_LIB_DIR` enviroment variable no long required when compiling for x86-64 Python from macOS arm64 and reverse. [#1428](https://github.com/PyO3/pyo3/pull/1428) - Fix FFI definition `_PyEval_RequestCodeExtraIndex` which took an argument of the wrong type. [#1429](https://github.com/PyO3/pyo3/pull/1429) diff --git a/build.rs b/build.rs index 55e4b08b420..5e425b21676 100644 --- a/build.rs +++ b/build.rs @@ -575,9 +575,19 @@ fn run_python_script(interpreter: &Path, script: &str) -> Result { } } -fn get_rustc_link_lib(config: &InterpreterConfig) -> Result { - let link_name = if cargo_env_var("CARGO_CFG_TARGET_OS").unwrap() == "windows" { - if is_abi3() { +fn get_library_link_name_unix(config: &InterpreterConfig) -> Result { + match config.implementation { + PythonImplementation::CPython => match &config.ld_version { + Some(ld_version) => Ok(format!("python{}", ld_version)), + None => bail!("failed to configure `ld_version` when compiling for unix"), + }, + PythonImplementation::PyPy => Ok(format!("pypy{}-c", config.version.major)), + } +} + +fn get_library_link_name(config: &InterpreterConfig) -> Result { + if cargo_env_var("CARGO_CFG_TARGET_OS").unwrap() == "windows" { + Ok(if is_abi3() { // Link against python3.lib for the stable ABI on Windows. // See https://www.python.org/dev/peps/pep-0384/#linkage // @@ -594,22 +604,10 @@ fn get_rustc_link_lib(config: &InterpreterConfig) -> Result { "pythonXY:python{}{}", config.version.major, config.version.minor ) - } + }) } else { - match config.implementation { - PythonImplementation::CPython => match &config.ld_version { - Some(ld_version) => format!("python{}", ld_version), - None => bail!("failed to configure `ld_version` when compiling for unix"), - }, - PythonImplementation::PyPy => format!("pypy{}-c", config.version.major), - } - }; - - Ok(format!( - "cargo:rustc-link-lib={link_model}{link_name}", - link_model = if config.shared { "" } else { "static=" }, - link_name = link_name - )) + get_library_link_name_unix(config) + } } fn get_venv_path() -> Option { @@ -748,46 +746,66 @@ fn ensure_python_version(interpreter_config: &InterpreterConfig) -> Result<()> { fn emit_cargo_configuration(interpreter_config: &InterpreterConfig) -> Result<()> { let target_os = cargo_env_var("CARGO_CFG_TARGET_OS").unwrap(); + + // Library search paths + + if let Some(libdir) = &interpreter_config.libdir { + println!("cargo:rustc-link-search=native={}", libdir); + } + if target_os == "windows" { + // libdir is only present on windows when cross-compiling, base_prefix is used on + // windows hosts + if let Some(base_prefix) = &interpreter_config.base_prefix { + println!("cargo:rustc-link-search=native={}\\libs", base_prefix); + } + } + + if interpreter_config.implementation == PythonImplementation::PyPy { + // PyPy 7.3.4 changed LIBDIR to point to base_prefix/lib as a regression, so need + // to hard-code /bin search path too: https://foss.heptapod.net/pypy/pypy/-/issues/3442 + // + // TODO: this workaround can probably be removed when PyPy 7.3.5 is released (and we + // can call it a PyPy bug). + if let Some(base_prefix) = &interpreter_config.base_prefix { + println!("cargo:rustc-link-search=native={}/bin", base_prefix); + } + } + + // Link libpython (if appropriate) + let is_extension_module = cargo_env_var("CARGO_FEATURE_EXTENSION_MODULE").is_some(); match (is_extension_module, target_os.as_str()) { - (_, "windows") => { - // always link on windows, even with extension module - println!("{}", get_rustc_link_lib(&interpreter_config)?); - // Set during cross-compiling. - if let Some(libdir) = &interpreter_config.libdir { - println!("cargo:rustc-link-search=native={}", libdir); - } - // Set if we have an interpreter to use. - if let Some(base_prefix) = &interpreter_config.base_prefix { - println!("cargo:rustc-link-search=native={}\\libs", base_prefix); - } - } - (true, "macos") => { - // with extension module on macos some extra linker arguments are needed - println!("cargo:rustc-cdylib-link-arg=-undefined"); - println!("cargo:rustc-cdylib-link-arg=dynamic_lookup"); + (_, "windows") | (_, "android") | (false, _) => { + // windows or android - always link to libpython + // other systems - only link libs if not extension module + println!( + "cargo:rustc-link-lib={link_model}{link_name}", + link_model = if interpreter_config.shared { + "" + } else { + "static=" + }, + link_name = get_library_link_name(&interpreter_config)? + ); } - (false, _) | (_, "android") => { - // other systems, only link libs if not extension module - // android always link. - println!("{}", get_rustc_link_lib(&interpreter_config)?); - if let Some(libdir) = &interpreter_config.libdir { - println!("cargo:rustc-link-search=native={}", libdir); - } - if interpreter_config.implementation == PythonImplementation::PyPy { - // PyPy 7.3.4 changed LIBDIR to point to base_prefix/lib as a regression, so need - // to hard-code /bin search path too: https://foss.heptapod.net/pypy/pypy/-/issues/3442 - // - // TODO: this workaround can probably be removed when PyPy 7.3.5 is released (and we - // can call it a PyPy bug). - if let Some(base_prefix) = &interpreter_config.base_prefix { - println!("cargo:rustc-link-search=native={}/bin", base_prefix); - } + (true, _) => { + // Extension module on unix system - only link non-lib targets + if target_os == "macos" { + // with extension module on macos some extra linker arguments are needed + println!("cargo:rustc-cdylib-link-arg=-undefined"); + println!("cargo:rustc-cdylib-link-arg=dynamic_lookup"); } + + let lib_name = get_library_link_name_unix(&interpreter_config)?; + println!("cargo:rustc-link-arg-bins=-l{}", lib_name); + println!("cargo:rustc-link-arg-tests=-l{}", lib_name); + println!("cargo:rustc-link-arg-benches=-l{}", lib_name); + println!("cargo:rustc-link-arg-examples=-l{}", lib_name); } - _ => {} } + // Sanity checks to prevent newcomers attempting static embedding + if env::var_os("CARGO_FEATURE_AUTO_INITIALIZE").is_some() { ensure!( interpreter_config.shared, @@ -807,13 +825,15 @@ fn emit_cargo_configuration(interpreter_config: &InterpreterConfig) -> Result<() // 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. + // Once cargo's `resolver = "2"` is stable (~ MSRV Rust 1.52), remove the check on the env var. ensure!( !interpreter_config.is_pypy() || env::var_os("PYO3_CI").is_some(), "The `auto-initialize` feature is not supported with PyPy." ); } + // Py_LIMITED_API cfg + let is_abi3 = is_abi3(); if interpreter_config.is_pypy() { @@ -842,10 +862,14 @@ fn emit_cargo_configuration(interpreter_config: &InterpreterConfig) -> Result<() interpreter_config.version.minor }; + // Py_3_x cfgs + for i in MINIMUM_SUPPORTED_VERSION.minor..=minor { println!("cargo:rustc-cfg=Py_3_{}", i); } + // py_sys_config cfgs + for flag in &interpreter_config.build_flags.0 { println!("cargo:rustc-cfg=py_sys_config=\"{}\"", flag) } diff --git a/guide/src/faq.md b/guide/src/faq.md index aeb2bc0051f..97cb57afbf3 100644 --- a/guide/src/faq.md +++ b/guide/src/faq.md @@ -15,17 +15,18 @@ PyO3 provides a struct [`GILOnceCell`] which works equivalently to `OnceCell` bu [`GILOnceCell`]: {{#PYO3_DOCS_URL}}/pyo3/once_cell/struct.GILOnceCell.html -## I can't run `cargo test`: I'm having linker issues like "Symbol not found" or "Undefined reference to _PyExc_SystemError"! +## I can't run `cargo test` or `cargo run`: I'm having linker issues like "Symbol not found" or "Undefined reference to _PyExc_SystemError"! -Currently, [#341](https://github.com/PyO3/pyo3/issues/341) causes `cargo test` to fail with linking errors when the `extension-module` feature is activated. For now you can work around this by making the `extension-module` feature optional and running the tests with `cargo test --no-default-features`: +On unix operating systems the `extension-module` feature is required to disable linking against libpython to meet criteria of how Python extension modules should be built. -```toml -[dependencies.pyo3] -version = "{{#PYO3_VERSION}}" +PyO3 is able to re-enable linking for binaries and tests in the project, but it requires a nightly cargo feature. To use this feature, you must opt into it, e.g.: -[features] -extension-module = ["pyo3/extension-module"] -default = ["extension-module"] +``` +# For cargo test +cargo +nightly -Zextra-link-arg test + +# For cargo run +cargo +nightly -Zextra-link-arg run ``` ## I can't run `cargo test`: my crate cannot be found for tests in `tests/` directory!