From 869bafc619423c806258978064ff42fa473e9fc7 Mon Sep 17 00:00:00 2001 From: Paddy Horan Date: Thu, 20 Dec 2018 23:59:01 -0500 Subject: [PATCH 1/9] Add support for conda envs on windows --- src/python_interpreter.rs | 173 ++++++++++++++++++++++++++------------ 1 file changed, 117 insertions(+), 56 deletions(-) diff --git a/src/python_interpreter.rs b/src/python_interpreter.rs index ca086cafe..2f54faa49 100644 --- a/src/python_interpreter.rs +++ b/src/python_interpreter.rs @@ -29,6 +29,35 @@ print(json.dumps({ })) "##; +/// Identifies conditions where we do not want to build wheels +fn windows_interpreter_no_build( + major: usize, + minor: usize, + target_width: usize, + pointer_width: usize, +) -> bool { + // Don't use python 2.6 + if major == 2 && minor != 7 { + return true; + } + + // Ignore python 3.0 - 3.4 + if major == 3 && minor < 5 { + return true; + } + + // There can be 32-bit installations on a 64-bit machine, but we can't link + // those for 64-bit targets + if pointer_width != target_width { + println!( + "{}.{} is installed as {}-bit, while the target is {}-bit. Skipping.", + major, minor, pointer_width, target_width + ); + return true; + } + false +} + /// Uses `py -0` to get a list of all installed python versions and then /// `sys.executable` to determine the path. /// @@ -36,72 +65,104 @@ print(json.dumps({ /// on windows the binary is always called "python.exe". We also have to make /// sure that the pointer width (32-bit or 64-bit) matches across platforms fn find_all_windows(target: &Target) -> Result, Error> { - let execution = Command::new("py").arg("-0").output(); - let output = execution - .context("Couldn't run 'py' command. Do you have python installed and in PATH?")?; - let expr = Regex::new(r" -(\d).(\d)-(\d+)(?: .*)?").unwrap(); - let lines = str::from_utf8(&output.stdout).unwrap().lines(); + let code = "import sys; print(sys.executable or '')"; let mut interpreter = vec![]; - for line in lines { - if let Some(capture) = expr.captures(line) { - let code = "import sys; print(sys.executable or '')"; - let context = "Expected a digit"; - - let major = capture - .get(1) - .unwrap() - .as_str() - .parse::() - .context(context)?; - let minor = capture - .get(2) - .unwrap() - .as_str() - .parse::() - .context(context)?; - let pointer_width = capture - .get(3) - .unwrap() - .as_str() - .parse::() - .context(context)?; - - // Don't use python 2.6 - if major == 2 && minor != 7 { - continue; - } - // Ignore python 3.0 - 3.4 - if major == 3 && minor < 5 { - continue; - } + // If Python is installed from Python.org it should include the "python launcher" + // which is used to find the installed interpreters + let execution = Command::new("py").arg("-0").output(); + if let Ok(output) = execution { + let expr = Regex::new(r" -(\d).(\d)-(\d+)(?: .*)?").unwrap(); + let lines = str::from_utf8(&output.stdout).unwrap().lines(); + for line in lines { + if let Some(capture) = expr.captures(line) { + let context = "Expected a digit"; + + let major = capture + .get(1) + .unwrap() + .as_str() + .parse::() + .context(context)?; + let minor = capture + .get(2) + .unwrap() + .as_str() + .parse::() + .context(context)?; + let pointer_width = capture + .get(3) + .unwrap() + .as_str() + .parse::() + .context(context)?; + + if windows_interpreter_no_build(major, minor, target.pointer_width(), pointer_width) + { + continue; + } - // There can be 32-bit installations on a 64-bit machine, but we can't link - // those for 64-bit targets - if pointer_width != target.pointer_width() { - println!( - "{}.{} is installed as {}-bit, while the target is {}-bit. Skipping.", - major, - minor, - pointer_width, - target.pointer_width() - ); - continue; + let version = format!("-{}.{}-{}", major, minor, pointer_width); + + let output = Command::new("py") + .args(&[&version, "-c", code]) + .output() + .unwrap(); + let path = str::from_utf8(&output.stdout).unwrap().trim(); + if !output.status.success() || path.trim().is_empty() { + bail!("Couldn't determine the path to python for `py {}`", version); + } + interpreter.push(path.to_string()); } + } + } - let version = format!("-{}.{}-{}", major, minor, pointer_width); + // Conda environments are also supported on windows + let conda_info = Command::new("conda").arg("info").arg("-e").output(); + if let Ok(output) = conda_info { + let lines = str::from_utf8(&output.stdout).unwrap().lines(); + let re = Regex::new(r"(\w|\\|:|-)+$").unwrap(); + let mut paths = vec![]; + for i in lines.into_iter() { + if !i.starts_with("#") { + if let Some(capture) = re.captures(&i) { + paths.push(String::from(&capture[0])); + } + } + } - let output = Command::new("py") - .args(&[&version, "-c", code]) + for path in paths { + let executable = format!(r"{}\python", path); + let python_info = Command::new(&executable) + .arg("-c") + .arg("import sys; print(sys.version)") .output() - .unwrap(); - let path = str::from_utf8(&output.stdout).unwrap().trim(); - if !output.status.success() || path.trim().is_empty() { - bail!("Couldn't determine the path to python for `py {}`", version); + .expect("Error getting Python version info from conda env..."); + let version_info = str::from_utf8(&python_info.stdout).unwrap(); + let expr = Regex::new(r"(\d).(\d).(\d+)").unwrap(); + if let Some(capture) = expr.captures(version_info) { + let major = capture.get(1).unwrap().as_str().parse::().unwrap(); + let minor = capture.get(2).unwrap().as_str().parse::().unwrap(); + let pointer_width = if version_info.contains("64 bit (AMD64)") { + 64_usize + } else { + 32_usize + }; + + if windows_interpreter_no_build(major, minor, target.pointer_width(), pointer_width) + { + continue; + } + + interpreter.push(executable); } - interpreter.push(path.to_string()); } } + if interpreter.is_empty() { + bail!( + "Could not find any interpreters, are you sure you have python installed on your PATH?" + ); + }; Ok(interpreter) } From a4313cc0625b1b28b909cf0be8f07304a5cd0abc Mon Sep 17 00:00:00 2001 From: Paddy Horan Date: Tue, 25 Dec 2018 22:08:56 -0500 Subject: [PATCH 2/9] Avoid re-building for same Python version on windows --- src/python_interpreter.rs | 74 ++++++++++++++++++++++----------------- 1 file changed, 41 insertions(+), 33 deletions(-) diff --git a/src/python_interpreter.rs b/src/python_interpreter.rs index 2f54faa49..425a379a0 100644 --- a/src/python_interpreter.rs +++ b/src/python_interpreter.rs @@ -2,6 +2,7 @@ use crate::Target; use failure::{Error, Fail, ResultExt}; use regex::Regex; use serde_json; +use std::collections::HashSet; use std::fmt; use std::io; use std::path::Path; @@ -67,6 +68,7 @@ fn windows_interpreter_no_build( fn find_all_windows(target: &Target) -> Result, Error> { let code = "import sys; print(sys.executable or '')"; let mut interpreter = vec![]; + let mut versions_found = HashSet::new(); // If Python is installed from Python.org it should include the "python launcher" // which is used to find the installed interpreters @@ -90,29 +92,32 @@ fn find_all_windows(target: &Target) -> Result, Error> { .as_str() .parse::() .context(context)?; - let pointer_width = capture - .get(3) - .unwrap() - .as_str() - .parse::() - .context(context)?; - - if windows_interpreter_no_build(major, minor, target.pointer_width(), pointer_width) - { - continue; + if !versions_found.contains(&(major, minor)) { + let pointer_width = capture + .get(3) + .unwrap() + .as_str() + .parse::() + .context(context)?; + + if windows_interpreter_no_build(major, minor, target.pointer_width(), pointer_width) + { + continue; + } + + let version = format!("-{}.{}-{}", major, minor, pointer_width); + + let output = Command::new("py") + .args(&[&version, "-c", code]) + .output() + .unwrap(); + let path = str::from_utf8(&output.stdout).unwrap().trim(); + if !output.status.success() || path.trim().is_empty() { + bail!("Couldn't determine the path to python for `py {}`", version); + } + interpreter.push(path.to_string()); + versions_found.insert((major, minor)); } - - let version = format!("-{}.{}-{}", major, minor, pointer_width); - - let output = Command::new("py") - .args(&[&version, "-c", code]) - .output() - .unwrap(); - let path = str::from_utf8(&output.stdout).unwrap().trim(); - if !output.status.success() || path.trim().is_empty() { - bail!("Couldn't determine the path to python for `py {}`", version); - } - interpreter.push(path.to_string()); } } } @@ -143,18 +148,21 @@ fn find_all_windows(target: &Target) -> Result, Error> { if let Some(capture) = expr.captures(version_info) { let major = capture.get(1).unwrap().as_str().parse::().unwrap(); let minor = capture.get(2).unwrap().as_str().parse::().unwrap(); - let pointer_width = if version_info.contains("64 bit (AMD64)") { - 64_usize - } else { - 32_usize - }; - - if windows_interpreter_no_build(major, minor, target.pointer_width(), pointer_width) - { - continue; + if !versions_found.contains(&(major, minor)) { + let pointer_width = if version_info.contains("64 bit (AMD64)") { + 64_usize + } else { + 32_usize + }; + + if windows_interpreter_no_build(major, minor, target.pointer_width(), pointer_width) + { + continue; + } + + interpreter.push(executable); + versions_found.insert((major, minor)); } - - interpreter.push(executable); } } } From 34885a347fc981604cbc6646407fb7470a2585a0 Mon Sep 17 00:00:00 2001 From: Paddy Horan Date: Tue, 25 Dec 2018 22:24:39 -0500 Subject: [PATCH 3/9] Fixed `clippy` and `rustfmt` issues --- src/python_interpreter.rs | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/python_interpreter.rs b/src/python_interpreter.rs index 425a379a0..cfbaa32a5 100644 --- a/src/python_interpreter.rs +++ b/src/python_interpreter.rs @@ -100,8 +100,12 @@ fn find_all_windows(target: &Target) -> Result, Error> { .parse::() .context(context)?; - if windows_interpreter_no_build(major, minor, target.pointer_width(), pointer_width) - { + if windows_interpreter_no_build( + major, + minor, + target.pointer_width(), + pointer_width, + ) { continue; } @@ -128,8 +132,8 @@ fn find_all_windows(target: &Target) -> Result, Error> { let lines = str::from_utf8(&output.stdout).unwrap().lines(); let re = Regex::new(r"(\w|\\|:|-)+$").unwrap(); let mut paths = vec![]; - for i in lines.into_iter() { - if !i.starts_with("#") { + for i in lines { + if !i.starts_with('#') { if let Some(capture) = re.captures(&i) { paths.push(String::from(&capture[0])); } @@ -155,10 +159,14 @@ fn find_all_windows(target: &Target) -> Result, Error> { 32_usize }; - if windows_interpreter_no_build(major, minor, target.pointer_width(), pointer_width) - { - continue; - } + if windows_interpreter_no_build( + major, + minor, + target.pointer_width(), + pointer_width, + ) { + continue; + } interpreter.push(executable); versions_found.insert((major, minor)); From 82e687c1296b9045c4dc7905ee703242d9a6956a Mon Sep 17 00:00:00 2001 From: Paddy Horan Date: Fri, 28 Dec 2018 22:27:31 -0500 Subject: [PATCH 4/9] Added documentation for expected command line output --- src/python_interpreter.rs | 42 ++++++++++++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/src/python_interpreter.rs b/src/python_interpreter.rs index cfbaa32a5..f8a6bd4bb 100644 --- a/src/python_interpreter.rs +++ b/src/python_interpreter.rs @@ -59,12 +59,44 @@ fn windows_interpreter_no_build( false } -/// Uses `py -0` to get a list of all installed python versions and then -/// `sys.executable` to determine the path. +/// On windows regular Python installs are supported along with environments +/// being managed by `conda`. /// -/// We can't use the the linux trick with trying different binary names since -/// on windows the binary is always called "python.exe". We also have to make -/// sure that the pointer width (32-bit or 64-bit) matches across platforms +/// We can't use the linux trick with trying different binary names since on +/// windows the binary is always called "python.exe". However, whether dealing +/// with regular Python installs or `conda` environments there are tools we can +/// use to query the information regarding installed interpreters. +/// +/// Regular Python installs downloaded from Python.org will include the python +/// launcher by default. We can use the launcher to find the information we need +/// for each installed interpreter using `py -0` which produces something like +/// the following output (the path can by determined using `sys.executable`): +/// +/// ```bash +/// Installed Pythons found by py Launcher for Windows +/// -3.7-64 * +/// -3.6-32 +/// -2.7-64 +/// ``` +/// +/// When using `conda` we can use the `conda info -e` command to retrieve information +/// regarding the installed interpreters being managed by `conda`. This is an example +/// of the output expected: +/// +/// ```bash +/// # conda environments: +/// # +/// base C:\Users\\Anaconda3 +/// foo1 * C:\Users\\Anaconda3\envs\foo1 +/// foo2 * C:\Users\\Anaconda3\envs\foo2 +/// ``` +/// +/// The information required can either by obtained by parsing this output directly or +/// by invoking the interpreters to obtain the information. +/// +/// As well as the version numbers, etc. of the interpreters we also have to find the +/// pointer width to make sure that the pointer width (32-bit or 64-bit) matches across +/// platforms. fn find_all_windows(target: &Target) -> Result, Error> { let code = "import sys; print(sys.executable or '')"; let mut interpreter = vec![]; From b2cde24c4ace81b8e3ea981aae6c199df0a774d6 Mon Sep 17 00:00:00 2001 From: Paddy Horan Date: Fri, 28 Dec 2018 22:41:17 -0500 Subject: [PATCH 5/9] Update to use `Path::join`. --- src/python_interpreter.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/python_interpreter.rs b/src/python_interpreter.rs index f8a6bd4bb..202fb0344 100644 --- a/src/python_interpreter.rs +++ b/src/python_interpreter.rs @@ -173,7 +173,7 @@ fn find_all_windows(target: &Target) -> Result, Error> { } for path in paths { - let executable = format!(r"{}\python", path); + let executable = Path::new(&path).join("python"); let python_info = Command::new(&executable) .arg("-c") .arg("import sys; print(sys.version)") @@ -200,7 +200,7 @@ fn find_all_windows(target: &Target) -> Result, Error> { continue; } - interpreter.push(executable); + interpreter.push(String::from(executable.to_str().unwrap())); versions_found.insert((major, minor)); } } From 22a0fa91f513a2a8b51f4ba6ce9f9f619293a6ba Mon Sep 17 00:00:00 2001 From: Paddy Horan Date: Tue, 15 Jan 2019 23:14:40 -0500 Subject: [PATCH 6/9] typo --- src/build_context.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/build_context.rs b/src/build_context.rs index ed125760d..ab9819488 100644 --- a/src/build_context.rs +++ b/src/build_context.rs @@ -71,7 +71,7 @@ pub struct BuildContext { impl BuildContext { /// Checks which kind of bindings we have (pyo3/rust-cypthon or cffi or bin) and calls the /// correct builder. Returns a Vec that contains location, python tag (e.g. py2.py3 or cp35) - /// and for bindings the python intepreter they bind against. + /// and for bindings the python interpreter they bind against. pub fn build_wheels(&self) -> Result)>, Error> { fs::create_dir_all(&self.out) .context("Failed to create the target directory for the wheels")?; From 502e21029ee56180b78cda88b882fb8b5ba45a07 Mon Sep 17 00:00:00 2001 From: Paddy Horan Date: Tue, 15 Jan 2019 23:25:00 -0500 Subject: [PATCH 7/9] Initial CI for conda bindings. --- appveyor.yml | 1 + tests/test_integration.rs | 73 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 8dd2a0a85..6c927cce4 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -30,6 +30,7 @@ matrix: fast_finish: true install: + - cinst miniconda3 - ps: | # For the gnu target we need gcc, provided by mingw. mingw which is already preinstalled, # but we need the right version (32-bit or 64-bit) to the PATH. diff --git a/tests/test_integration.rs b/tests/test_integration.rs index 51e62bdee..7f90d7dd2 100644 --- a/tests/test_integration.rs +++ b/tests/test_integration.rs @@ -1,6 +1,6 @@ use crate::common::check_installed; use pyo3_pack::{BuildOptions, Target}; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::str; use structopt::StructOpt; @@ -31,6 +31,12 @@ fn test_integration_get_fourtytwo() { test_integration(Path::new("get-fourtytwo"), None); } +#[cfg(target_os = "windows")] +#[test] +fn test_integration_get_fourtytwo_conda() { + test_integration_conda(Path::new("get-fourtytwo"), None); +} + #[test] fn test_integration_points() { test_integration(Path::new("points"), Some("cffi".to_string())); @@ -134,3 +140,68 @@ fn test_integration(package: &Path, bindings: Option) { check_installed(&package, &python).unwrap(); } } + +fn create_conda_env(name: &str, major: usize, minor: usize) { + Command::new("conda") + .arg("create") + .arg("-n") + .arg(name) + .arg(format!("python={}.{}", major, minor)) + .arg("-q") + .arg("-y") + .output() + .expect("Conda not available."); +} + +fn test_integration_conda(package: &Path, bindings: Option) { + let package_string = package.join("Cargo.toml").display().to_string(); + + // Create environments to build against + create_conda_env("pyo3-build-env-27", 2, 7); + create_conda_env("pyo3-build-env-35", 3, 5); + create_conda_env("pyo3-build-env-36", 3, 6); + create_conda_env("pyo3-build-env-37", 3, 7); + + // The first string is ignored by clap + let cli = if let Some(ref bindings) = bindings { + vec![ + "build", + "--manifest-path", + &package_string, + "--bindings", + bindings, + ] + } else { + vec!["build", "--manifest-path", &package_string] + }; + + let options = BuildOptions::from_iter_safe(cli).unwrap(); + + let wheels = options + .into_build_context(false, false) + .unwrap() + .build_wheels() + .unwrap(); + + for (filename, _, python_interpreter) in wheels { + if let Some(pi) = python_interpreter { + let executable = pi.executable; + + let output = Command::new(&executable) + .args(&[ + "-m", + "pip", + "install", + "--force-reinstall", + &adjust_canonicalization(filename), + ]) + .stderr(Stdio::inherit()) + .output() + .unwrap(); + if !output.status.success() { + panic!(); + } + check_installed(&package, &executable).unwrap(); + } + } +} From 37260daffdcf9e9f64b5d678a5b4da3bfd23a27a Mon Sep 17 00:00:00 2001 From: Paddy Horan Date: Wed, 16 Jan 2019 21:22:50 -0500 Subject: [PATCH 8/9] Update to trigger CI --- tests/test_integration.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_integration.rs b/tests/test_integration.rs index 7f90d7dd2..a8e457457 100644 --- a/tests/test_integration.rs +++ b/tests/test_integration.rs @@ -141,6 +141,7 @@ fn test_integration(package: &Path, bindings: Option) { } } +/// Creates conda environments fn create_conda_env(name: &str, major: usize, minor: usize) { Command::new("conda") .arg("create") From 61de39d116470e692167c240668577b0e935f5fb Mon Sep 17 00:00:00 2001 From: Paddy Horan Date: Sun, 20 Jan 2019 23:19:45 -0500 Subject: [PATCH 9/9] Addressed review comments --- tests/test_integration.rs | 52 ++++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/tests/test_integration.rs b/tests/test_integration.rs index a8e457457..129ce5367 100644 --- a/tests/test_integration.rs +++ b/tests/test_integration.rs @@ -157,11 +157,12 @@ fn create_conda_env(name: &str, major: usize, minor: usize) { fn test_integration_conda(package: &Path, bindings: Option) { let package_string = package.join("Cargo.toml").display().to_string(); - // Create environments to build against - create_conda_env("pyo3-build-env-27", 2, 7); - create_conda_env("pyo3-build-env-35", 3, 5); - create_conda_env("pyo3-build-env-36", 3, 6); - create_conda_env("pyo3-build-env-37", 3, 7); + // Create environments to build against, prepended with "A" to ensure that integration + // tests are executed with these environments + create_conda_env("A-pyo3-build-env-27", 2, 7); + create_conda_env("A-pyo3-build-env-35", 3, 5); + create_conda_env("A-pyo3-build-env-36", 3, 6); + create_conda_env("A-pyo3-build-env-37", 3, 7); // The first string is ignored by clap let cli = if let Some(ref bindings) = bindings { @@ -184,25 +185,36 @@ fn test_integration_conda(package: &Path, bindings: Option) { .build_wheels() .unwrap(); + let mut conda_wheels: Vec<(PathBuf, PathBuf)> = vec![]; for (filename, _, python_interpreter) in wheels { if let Some(pi) = python_interpreter { let executable = pi.executable; - - let output = Command::new(&executable) - .args(&[ - "-m", - "pip", - "install", - "--force-reinstall", - &adjust_canonicalization(filename), - ]) - .stderr(Stdio::inherit()) - .output() - .unwrap(); - if !output.status.success() { - panic!(); + if executable.to_str().unwrap().contains("pyo3-build-env-") { + conda_wheels.push((filename, executable)) } - check_installed(&package, &executable).unwrap(); } } + + assert_eq!( + 4, + conda_wheels.len(), + "Error creating or detecting conda environments." + ); + for (wheel_file, executable) in conda_wheels { + let output = Command::new(&executable) + .args(&[ + "-m", + "pip", + "install", + "--force-reinstall", + &adjust_canonicalization(wheel_file), + ]) + .stderr(Stdio::inherit()) + .output() + .unwrap(); + if !output.status.success() { + panic!(); + } + check_installed(&package, &executable).unwrap(); + } }