Skip to content

Commit

Permalink
pyo3-build-config: conditionalize symbols on resolve-config feature
Browse files Browse the repository at this point in the history
PR PyO3#1856 was buggy in that the `pyo3-build-config` crate didn't actually
work in library mode because `include_str!()` was attempting to resolve
missing files as part of populating some `const` values.

We could change the logic of these constants to make them lazy if
we wanted to support possibly getting access to the value. But the
simple solution is to conditionalize their presence on the crate
feature.

Test coverage for building and testing the crate in insolation with the
feature disabled has been added.

Various code has been conditionalized to avoid compiler warnings.

Also, it appears `cargo build|test -p pyo3-build-config
--no-default-features` still passes default features. This seems wrong
to me. But it is how my system behaves. Maybe it is an sccache bug?
I coded the new tests to `cd pyo3-build-config` first to work around.
  • Loading branch information
indygreg committed Sep 3, 2021
1 parent afd4d46 commit 4d5bd48
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 1 deletion.
12 changes: 12 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -176,11 +176,23 @@ jobs:
- name: Build (no features)
run: cargo build --lib --tests --no-default-features

# --no-default-features when used with `cargo build/test -p` doesn't seem to work!
- name: Build pyo3-build-config (no features)
run: |
cd pyo3-build-config
cargo build --no-default-features
# Run tests (except on PyPy, because no embedding API).
- if: ${{ !startsWith(matrix.python-version, 'pypy') }}
name: Test (no features)
run: cargo test --no-default-features

# --no-default-features when used with `cargo build/test -p` doesn't seem to work!
- name: Test pyo3-build-config (no features)
run: |
cd pyo3-build-config
cargo test --no-default-features
- name: Build (all additive features)
run: cargo build --lib --tests --no-default-features --features "${{ steps.settings.outputs.all_additive_features }}"

Expand Down
7 changes: 6 additions & 1 deletion pyo3-build-config/src/impl_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,7 @@ struct CrossCompileConfig {
arch: String,
}

#[allow(unused)]
pub fn any_cross_compiling_env_vars_set() -> bool {
env::var_os("PYO3_CROSS").is_some()
|| env::var_os("PYO3_CROSS_LIB_DIR").is_some()
Expand Down Expand Up @@ -1466,7 +1467,11 @@ mod tests {
}

#[test]
#[cfg(all(target_os = "linux", target_arch = "x86_64"))]
#[cfg(all(
target_os = "linux",
target_arch = "x86_64",
feature = "resolve-config"
))]
fn parse_sysconfigdata() {
// A best effort attempt to get test coverage for the sysconfigdata parsing.
// Might not complete successfully depending on host installation; that's ok as long as
Expand Down
13 changes: 13 additions & 0 deletions pyo3-build-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
mod errors;
mod impl_;

#[cfg(feature = "resolve-config")]
use std::io::Cursor;

#[cfg(feature = "resolve-config")]
use once_cell::sync::OnceCell;

pub use impl_::{BuildFlag, BuildFlags, InterpreterConfig, PythonImplementation, PythonVersion};
Expand All @@ -28,6 +30,7 @@ pub use impl_::{BuildFlag, BuildFlags, InterpreterConfig, PythonImplementation,
/// | `#[cfg(PyPy)]` | This marks code which is run when compiling for PyPy. |
///
/// For examples of how to use these attributes, [see PyO3's guide](https://pyo3.rs/latest/building_and_distribution/multiple_python_versions.html).
#[cfg(feature = "resolve-config")]
pub fn use_pyo3_cfgs() {
get().emit_pyo3_cfgs();
}
Expand Down Expand Up @@ -56,6 +59,7 @@ fn _add_extension_module_link_args(target_os: &str, mut writer: impl std::io::Wr
///
/// Because this will never change in a given compilation run, this is cached in a `once_cell`.
#[doc(hidden)]
#[cfg(feature = "resolve-config")]
pub fn get() -> &'static InterpreterConfig {
static CONFIG: OnceCell<InterpreterConfig> = OnceCell::new();
CONFIG.get_or_init(|| {
Expand All @@ -74,23 +78,28 @@ pub fn get() -> &'static InterpreterConfig {

/// Path where PyO3's build.rs will write configuration by default.
#[doc(hidden)]
#[cfg(feature = "resolve-config")]
const DEFAULT_CROSS_COMPILE_CONFIG_PATH: &str =
concat!(env!("OUT_DIR"), "/pyo3-cross-compile-config.txt");

/// Build configuration provided by `PYO3_CONFIG_FILE`. May be empty if env var not set.
#[doc(hidden)]
#[cfg(feature = "resolve-config")]
const CONFIG_FILE: &str = include_str!(concat!(env!("OUT_DIR"), "/pyo3-build-config-file.txt"));

/// Build configuration set if abi3 features enabled and `PYO3_NO_PYTHON` env var present. Empty if
/// not both present.
#[doc(hidden)]
#[cfg(feature = "resolve-config")]
const ABI3_CONFIG: &str = include_str!(concat!(env!("OUT_DIR"), "/pyo3-build-config-abi3.txt"));

/// Build configuration discovered by `pyo3-build-config` build script. Not aware of
/// cross-compilation settings.
#[doc(hidden)]
#[cfg(feature = "resolve-config")]
const HOST_CONFIG: &str = include_str!(concat!(env!("OUT_DIR"), "/pyo3-build-config.txt"));

#[cfg(feature = "resolve-config")]
fn abi3_config() -> InterpreterConfig {
let mut interpreter_config = InterpreterConfig::from_reader(Cursor::new(ABI3_CONFIG))
.expect("failed to parse hardcoded PyO3 abi3 config");
Expand All @@ -109,9 +118,12 @@ fn abi3_config() -> InterpreterConfig {
/// Please don't use these - they could change at any time.
#[doc(hidden)]
pub mod pyo3_build_script_impl {
#[cfg(feature = "resolve-config")]
use crate::errors::{Context, Result};
#[cfg(feature = "resolve-config")]
use std::path::Path;

#[cfg(feature = "resolve-config")]
use super::*;

pub mod errors {
Expand All @@ -126,6 +138,7 @@ pub mod pyo3_build_script_impl {
/// Differs from .get() above only in the cross-compile case, where PyO3's build script is
/// required to generate a new config (as it's the first build script which has access to the
/// correct value for CARGO_CFG_TARGET_OS).
#[cfg(feature = "resolve-config")]
pub fn resolve_interpreter_config() -> Result<InterpreterConfig> {
if !CONFIG_FILE.is_empty() {
InterpreterConfig::from_reader(Cursor::new(CONFIG_FILE))
Expand Down

0 comments on commit 4d5bd48

Please sign in to comment.