From c3b45119180edbeaeaa98004532171c20ba21d95 Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Sun, 11 Apr 2021 22:25:45 +0100 Subject: [PATCH] build: avoid rebuilds when using clippy in a virtualenv --- CHANGELOG.md | 3 +- build.rs | 179 +++++++++++++------------ guide/src/building_and_distribution.md | 1 + 3 files changed, 96 insertions(+), 87 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a212f697d7..2d0d8510114 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,7 +53,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Fix use of Python argument for #[pymethods] inside macro expansions. [#1505](https://github.com/PyO3/pyo3/pull/1505) - Always use cross-compiling configuration if any of the environment variables are set. [#1514](https://github.com/PyO3/pyo3/pull/1514) - Support `EnvironmentError`, `IOError`, and `WindowsError` on PyPy. [#1533](https://github.com/PyO3/pyo3/pull/1533) -- Segfault when dereferencing `ffi::PyDateTimeAPI` without the GIL. [#1563](https://github.com/PyO3/pyo3/pull/1563) +- Fix unneccessary rebuilds when cycling between `cargo check` and `cargo clippy` in a Python virtualenv. [#1557](https://github.com/PyO3/pyo3/pull/1557) +- Fix segfault when dereferencing `ffi::PyDateTimeAPI` without the GIL. [#1563](https://github.com/PyO3/pyo3/pull/1563) ## [0.13.2] - 2021-02-12 ### Packaging diff --git a/build.rs b/build.rs index f81c404106e..8d83b9784d7 100644 --- a/build.rs +++ b/build.rs @@ -2,6 +2,7 @@ use std::{ collections::{HashMap, HashSet}, convert::AsRef, env, + ffi::OsString, fs::{self, DirEntry}, io, path::{Path, PathBuf}, @@ -35,6 +36,20 @@ macro_rules! warn { }; } +/// Gets an environment variable owned by cargo. +/// +/// Environment variables set by cargo are expected to be valid UTF8. +fn cargo_env_var(var: &str) -> Option { + env::var_os(var).map(|os_string| os_string.to_str().unwrap().into()) +} + +/// Gets an external environment variable, and registers the build script to rerun if +/// the variable changes. +fn env_var(var: &str) -> Option { + println!("cargo:rerun-if-env-changed={}", var); + env::var_os(var) +} + /// Information returned from python interpreter #[derive(Debug)] struct InterpreterConfig { @@ -83,7 +98,7 @@ impl FromStr for PythonInterpreterKind { } fn is_abi3() -> bool { - env::var_os("CARGO_FEATURE_ABI3").is_some() + cargo_env_var("CARGO_FEATURE_ABI3").is_some() } trait GetPrimitive { @@ -119,40 +134,22 @@ struct CrossCompileConfig { arch: String, } -impl CrossCompileConfig { - fn new() -> Result { - Ok(CrossCompileConfig { - lib_dir: CrossCompileConfig::validate_variable("PYO3_CROSS_LIB_DIR")?, - os: env::var("CARGO_CFG_TARGET_OS").unwrap(), - arch: env::var("CARGO_CFG_TARGET_ARCH").unwrap(), - version: env::var_os("PYO3_CROSS_PYTHON_VERSION").map(|s| s.into_string().unwrap()), - }) - } - - fn validate_variable(var: &str) -> Result { - let path = match env::var_os(var) { - Some(v) => v, - None => bail!( - "Must provide {} environment variable when cross-compiling", - var - ), - }; - - if fs::metadata(&path).is_err() { - bail!("{} value of {:?} does not exist", var, path) - } +fn cross_compiling() -> Result> { + let cross = env_var("PYO3_CROSS"); + let cross_lib_dir = env_var("PYO3_CROSS_LIB_DIR"); + let cross_python_version = env_var("PYO3_CROSS_PYTHON_VERSION"); - Ok(path.into()) - } -} + let target_arch = cargo_env_var("CARGO_CFG_TARGET_ARCH"); + let target_vendor = cargo_env_var("CARGO_CFG_TARGET_VENDOR"); + let target_os = cargo_env_var("CARGO_CFG_TARGET_OS"); -fn cross_compiling() -> Result> { - if env::var_os("PYO3_CROSS").is_none() - && env::var_os("PYO3_CROSS_LIB_DIR").is_none() - && env::var_os("PYO3_CROSS_PYTHON_VERSION").is_none() + if cross.is_none() && cross_lib_dir.is_none() && cross_python_version.is_none() { - let target = env::var("TARGET")?; - let host = env::var("HOST")?; + // No cross-compiling environment variables set; try to determine if this is a known case + // which is not cross-compilation. + + let target = cargo_env_var("TARGET").unwrap(); + let host = cargo_env_var("HOST").unwrap(); if target == host { // Not cross-compiling return Ok(None); @@ -173,20 +170,32 @@ fn cross_compiling() -> Result> { return Ok(None); } - if host.starts_with(&format!( - "{}-{}-{}", - env::var("CARGO_CFG_TARGET_ARCH")?, - env::var("CARGO_CFG_TARGET_VENDOR")?, - env::var("CARGO_CFG_TARGET_OS")? - )) { - // Not cross-compiling if arch-vendor-os is all the same - // e.g. x86_64-unknown-linux-musl on x86_64-unknown-linux-gnu host - return Ok(None); + if let (Some(arch), Some(vendor), Some(os)) = (&target_arch, &target_vendor, &target_os) { + if host.starts_with(&format!("{}-{}-{}", arch, vendor, os)) { + // Not cross-compiling if arch-vendor-os is all the same + // e.g. x86_64-unknown-linux-musl on x86_64-unknown-linux-gnu host + return Ok(None); + } } } - // Cross-compiling on any other platform - Ok(Some(CrossCompileConfig::new()?)) + // At this point we assume that we are cross compiling. + + Ok(Some(CrossCompileConfig { + lib_dir: cross_lib_dir + .ok_or("The PYO3_CROSS_LIB_DIR environment variable must be set when cross-compiling")? + .into(), + os: target_os.unwrap(), + arch: target_arch.unwrap(), + version: cross_python_version + .map(|os_string| { + os_string + .to_str() + .ok_or("PYO3_CROSS_PYTHON_VERSION is not valid utf-8.") + .map(str::to_owned) + }) + .transpose()?, + })) } /// A list of python interpreter compile-time preprocessor defines that @@ -226,7 +235,7 @@ impl BuildFlags { /// the interpreter and printing variables of interest from /// sysconfig.get_config_vars. fn from_interpreter(python_path: &Path) -> Result { - if env::var("CARGO_CFG_TARGET_OS").unwrap() == "windows" { + if cargo_env_var("CARGO_CFG_TARGET_OS").unwrap() == "windows" { return Ok(Self::windows_hardcoded()); } @@ -377,7 +386,7 @@ fn ends_with(entry: &DirEntry, pat: &str) -> bool { /// [1]: https://github.com/python/cpython/blob/3.5/Lib/sysconfig.py#L389 fn find_sysconfigdata(cross: &CrossCompileConfig) -> Result { let sysconfig_paths = search_lib_dir(&cross.lib_dir, &cross); - let sysconfig_name = env::var_os("_PYTHON_SYSCONFIGDATA_NAME"); + let sysconfig_name = env_var("_PYTHON_SYSCONFIGDATA_NAME"); let mut sysconfig_paths = sysconfig_paths .iter() .filter_map(|p| { @@ -525,7 +534,7 @@ fn windows_hardcoded_cross_compile( fn load_cross_compile_info( cross_compile_config: CrossCompileConfig, ) -> Result<(InterpreterConfig, BuildFlags)> { - match env::var_os("CARGO_CFG_TARGET_FAMILY") { + match cargo_env_var("CARGO_CFG_TARGET_FAMILY") { // Configure for unix platforms using the sysconfigdata file Some(os) if os == "unix" => load_cross_compile_from_sysconfigdata(cross_compile_config), // Use hardcoded interpreter config when targeting Windows @@ -580,14 +589,14 @@ fn run_python_script(interpreter: &Path, script: &str) -> Result { } fn get_rustc_link_lib(config: &InterpreterConfig) -> String { - let link_name = if env::var("CARGO_CFG_TARGET_OS").unwrap().as_str() == "windows" { + let link_name = if cargo_env_var("CARGO_CFG_TARGET_OS").unwrap() == "windows" { if is_abi3() { // Link against python3.lib for the stable ABI on Windows. // See https://www.python.org/dev/peps/pep-0384/#linkage // // This contains only the limited ABI symbols. "pythonXY:python3".to_owned() - } else if env::var("CARGO_CFG_TARGET_ENV").unwrap().as_str() == "gnu" { + } else if cargo_env_var("CARGO_CFG_TARGET_ENV").unwrap() == "gnu" { // https://packages.msys2.org/base/mingw-w64-python format!( "pythonXY:python{}.{}", @@ -613,13 +622,31 @@ fn get_rustc_link_lib(config: &InterpreterConfig) -> String { ) } +fn get_venv_path() -> Option { + match (env_var("VIRTUAL_ENV"), env_var("CONDA_PREFIX")) { + (Some(dir), None) => Some(PathBuf::from(dir)), + (None, Some(dir)) => Some(PathBuf::from(dir)), + (Some(_), Some(_)) => { + warn!( + "Both VIRTUAL_ENV and CONDA_PREFIX are set. PyO3 will ignore both of these for \ + locating the Python interpreter until you unset one of them." + ); + None + } + (None, None) => None, + } +} + fn find_interpreter() -> Result { - if let Some(exe) = env::var_os("PYO3_PYTHON") { - Ok(exe.into()) - } else if let Some(exe) = env::var_os("PYTHON_SYS_EXECUTABLE") { - // Backwards-compatible name for PYO3_PYTHON; this may be removed at some point in the future. + if let Some(exe) = env_var("PYO3_PYTHON") { Ok(exe.into()) + } else if let Some(venv_path) = get_venv_path() { + match cargo_env_var("CARGO_CFG_TARGET_OS").unwrap().as_str() { + "windows" => Ok(venv_path.join("Scripts\\python")), + _ => Ok(venv_path.join("bin/python")), + } } else { + println!("cargo:rerun-if-env-changed=PATH"); ["python", "python3"] .iter() .find(|bin| { @@ -692,7 +719,7 @@ print("calcsize_pointer", struct.calcsize("P")) let output = run_python_script(interpreter, script)?; let map: HashMap = parse_script_output(&output); let shared = match ( - env::var("CARGO_CFG_TARGET_OS").unwrap().as_str(), + cargo_env_var("CARGO_CFG_TARGET_OS").unwrap().as_str(), map["framework"].as_str(), map["shared"].as_str(), ) { @@ -735,8 +762,8 @@ fn configure(interpreter_config: &InterpreterConfig) -> Result<()> { check_target_architecture(interpreter_config)?; - let target_os = env::var("CARGO_CFG_TARGET_OS").unwrap(); - let is_extension_module = env::var_os("CARGO_FEATURE_EXTENSION_MODULE").is_some(); + let target_os = cargo_env_var("CARGO_CFG_TARGET_OS").unwrap(); + 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 @@ -833,7 +860,10 @@ fn configure(interpreter_config: &InterpreterConfig) -> Result<()> { fn check_target_architecture(interpreter_config: &InterpreterConfig) -> Result<()> { // Try to check whether the target architecture matches the python library - let rust_target = match env::var("CARGO_CFG_TARGET_POINTER_WIDTH")?.as_str() { + let rust_target = match cargo_env_var("CARGO_CFG_TARGET_POINTER_WIDTH") + .unwrap() + .as_str() + { "64" => "64-bit", "32" => "32-bit", x => bail!("unexpected Rust target pointer width: {}", x), @@ -868,10 +898,10 @@ fn check_target_architecture(interpreter_config: &InterpreterConfig) -> Result<( fn get_abi3_minor_version() -> Option { (PY3_MIN_MINOR..=ABI3_MAX_MINOR) - .find(|i| env::var_os(format!("CARGO_FEATURE_ABI3_PY3{}", i)).is_some()) + .find(|i| cargo_env_var(&format!("CARGO_FEATURE_ABI3_PY3{}", i)).is_some()) } -fn abi3_without_interpreter() -> Result<()> { +fn configure_abi3_without_interpreter() { println!("cargo:rustc-cfg=Py_LIMITED_API"); let mut flags = "FLAG_WITH_THREAD=1".to_string(); let abi_version = get_abi3_minor_version().unwrap_or(ABI3_MAX_MINOR); @@ -882,7 +912,7 @@ fn abi3_without_interpreter() -> Result<()> { println!("cargo:rustc-cfg=py_sys_config=\"WITH_THREAD\""); println!("cargo:python_flags={}", flags); - if env::var("CARGO_CFG_TARGET_FAMILY")? == "windows" { + if cargo_env_var("CARGO_CFG_TARGET_FAMILY").unwrap() == "windows" { // Unfortunately, on windows we can't build without at least providing // python.lib to the linker. While maturin tells the linker the location // of python.lib, we need to do the renaming here, otherwise cargo @@ -890,18 +920,17 @@ fn abi3_without_interpreter() -> Result<()> { // attribute with pythonXY. println!("cargo:rustc-link-lib=pythonXY:python3"); } - - Ok(()) } fn main_impl() -> Result<()> { // If PYO3_NO_PYTHON is set with abi3, we can build PyO3 without calling Python. // We only check for the abi3-py3{ABI3_MAX_MINOR} because lower versions depend on it. - if env::var_os("PYO3_NO_PYTHON").is_some() - && env::var_os(format!("CARGO_FEATURE_ABI3_PY3{}", ABI3_MAX_MINOR)).is_some() + if cargo_env_var("PYO3_NO_PYTHON").is_some() + && cargo_env_var(&format!("CARGO_FEATURE_ABI3_PY3{}", ABI3_MAX_MINOR)).is_some() { println!("cargo:rerun-if-env-changed=PYO3_NO_PYTHON"); - return abi3_without_interpreter(); + configure_abi3_without_interpreter(); + return Ok(()); } // 1. Setup cfg variables so we can do conditional compilation in this library based on the // python interpeter's compilation flags. This is necessary for e.g. matching the right unicode @@ -929,28 +958,6 @@ fn main_impl() -> Result<()> { println!("cargo:rustc-cfg={}=\"{}\"", CFG_KEY, flag) } - for var in &[ - "LIB", - "LD_LIBRARY_PATH", - "PYO3_PYTHON", - "PYO3_CROSS", - "PYO3_CROSS_LIB_DIR", - "PYO3_CROSS_PYTHON_VERSION", - ] { - println!("cargo:rerun-if-env-changed={}", var); - } - - if env::var_os("PYO3_PYTHON").is_none() { - // When PYO3_PYTHON is not used, PYTHON_SYS_EXECUTABLE has the highest priority. - // Let's watch it. - println!("cargo:rerun-if-env-changed=PYTHON_SYS_EXECUTABLE"); - if env::var_os("PYTHON_SYS_EXECUTABLE").is_none() { - // When PYTHON_SYS_EXECUTABLE is also not used, then we use PATH. - // Let's watch this, too. - println!("cargo:rerun-if-env-changed=PATH"); - } - } - Ok(()) } diff --git a/guide/src/building_and_distribution.md b/guide/src/building_and_distribution.md index 8b51b90340d..b1293241c2b 100644 --- a/guide/src/building_and_distribution.md +++ b/guide/src/building_and_distribution.md @@ -90,6 +90,7 @@ See [github.com/japaric/rust-cross](https://github.com/japaric/rust-cross) for a After you've obtained the above, you can build a cross compiled PyO3 module by setting a few extra environment variables: +* `PYO3_CROSS`: If present this variable forces PyO3 to configure as a cross-compilation. * `PYO3_CROSS_LIB_DIR`: This variable must be set to the directory containing the target's libpython DSO and the associated `_sysconfigdata*.py` file for Unix-like targets, or the Python DLL import libraries for the Windows target. * `PYO3_CROSS_PYTHON_VERSION`: Major and minor version (e.g. 3.9) of the target Python installation. This variable is only needed if PyO3 cannot determine the version to target from `abi3-py3*` features, or if there are multiple versions of Python present in `PYO3_CROSS_LIB_DIR`.