From be168372490c5b4a27114100650bda7c0fd29164 Mon Sep 17 00:00:00 2001 From: nichmor Date: Tue, 19 Dec 2023 11:51:43 +0200 Subject: [PATCH 01/13] create venv directly from rust --- crates/rattler_installs_packages/Cargo.toml | 1 - .../src/artifacts/wheel.rs | 9 + .../src/python_env/system_python.rs | 21 +- .../src/python_env/venv.rs | 194 +++++++++++++++++- 4 files changed, 218 insertions(+), 7 deletions(-) diff --git a/crates/rattler_installs_packages/Cargo.toml b/crates/rattler_installs_packages/Cargo.toml index 4c2fa504..e8f22140 100644 --- a/crates/rattler_installs_packages/Cargo.toml +++ b/crates/rattler_installs_packages/Cargo.toml @@ -57,7 +57,6 @@ tracing = { version = "0.1.37", default-features = false, features = ["attribute url = { version = "2.4.1", features = ["serde"] } zip = "0.6.6" resolvo = { version = "0.2.0", default-features = false } -which = "4.4.2" pathdiff = "0.2.1" async_http_range_reader = "0.3.0" async_zip = { version = "0.0.15", features = ["tokio", "deflate"] } diff --git a/crates/rattler_installs_packages/src/artifacts/wheel.rs b/crates/rattler_installs_packages/src/artifacts/wheel.rs index 04ef71b8..c2e4aeb2 100644 --- a/crates/rattler_installs_packages/src/artifacts/wheel.rs +++ b/crates/rattler_installs_packages/src/artifacts/wheel.rs @@ -502,6 +502,15 @@ impl InstallPaths { &self.data } + /// Returns the location of the include directory + pub fn include(&self) -> PathBuf { + if self.windows { + PathBuf::from("Include") + } else { + PathBuf::from("include") + } + } + /// Returns the location of the headers directory. The location of headers is specific to a /// distribution name. pub fn headers(&self, distribution_name: &str) -> PathBuf { diff --git a/crates/rattler_installs_packages/src/python_env/system_python.rs b/crates/rattler_installs_packages/src/python_env/system_python.rs index 2fa17c5d..b7e784a6 100644 --- a/crates/rattler_installs_packages/src/python_env/system_python.rs +++ b/crates/rattler_installs_packages/src/python_env/system_python.rs @@ -11,13 +11,28 @@ pub enum FindPythonError { } /// Try to find the python executable in the current environment. +/// Using sys.executable aproach will return original interpretator path +/// and not the shim in case of using which pub fn system_python_executable() -> Result { // When installed with homebrew on macOS, the python3 executable is called `python3` instead // Also on some ubuntu installs this is the case // For windows it should just be python - which::which("python3") - .or_else(|_| which::which("python")) - .map_err(|_| FindPythonError::NotFound) + let output = std::process::Command::new("python3") + .arg("-c") + .arg("import sys; print(sys.executable, end='')") + .output() + .or_else(|_| { + std::process::Command::new("python") + .arg("-c") + .arg("import sys; print(sys.executable, end='')") + .output() + }) + .map_err(|_| FindPythonError::NotFound)?; + + let stdout = String::from_utf8_lossy(&output.stdout); + let python_path = PathBuf::from_str(&stdout).map_err(|_| FindPythonError::NotFound)?; + + Ok(python_path) } /// Errors that can occur while trying to parse the python version diff --git a/crates/rattler_installs_packages/src/python_env/venv.rs b/crates/rattler_installs_packages/src/python_env/venv.rs index b77a1e03..9a35d338 100644 --- a/crates/rattler_installs_packages/src/python_env/venv.rs +++ b/crates/rattler_installs_packages/src/python_env/venv.rs @@ -8,10 +8,27 @@ use crate::python_env::{ system_python_executable, FindPythonError, ParsePythonInterpreterVersionError, PythonInterpreterVersion, }; +use std::ffi::OsStr; +use std::fmt::Debug; +use std::fs; +use std::io::ErrorKind; use std::path::{Path, PathBuf}; use std::process::{Command, Output}; use thiserror::Error; +#[cfg(unix)] +pub fn copy_file, U: AsRef>(from: P, to: U) -> std::io::Result<()> { + std::os::unix::fs::symlink(from, to)?; + Ok(()) +} + +#[cfg(windows)] +pub fn copy_file, U: AsRef>(from: P, to: U) -> std::io::Result<()> { + fs::copy(from, to)?; + fs::set_permissions(to, fs::Permissions::from_mode(0o755)).unwrap(); + Ok(()) +} + /// Specifies where to find the python executable #[derive(Debug, Clone, Default, PartialEq, Eq)] pub enum PythonLocation { @@ -116,11 +133,141 @@ impl VEnv { /// Create a virtual environment at specified directory /// for the platform we are running on pub fn create(venv_dir: &Path, python: PythonLocation) -> Result { - Self::create_custom(venv_dir, python, cfg!(windows)) + Self::create_without_subprocess(venv_dir, python, cfg!(windows)) + } + + /// Ensure all directories are created + pub fn create_without_subprocess( + venv_abs_dir: &Path, + python: PythonLocation, + windows: bool, + ) -> Result { + let base_python_path = python.executable()?; + let base_python_version = PythonInterpreterVersion::from_path(&base_python_path)?; + let base_python_name = base_python_path + .file_name() + .expect("Cannot extract base python name"); + + let install_paths = InstallPaths::for_venv(base_python_version.clone(), windows); + + Self::create_install_paths(venv_abs_dir, &install_paths)?; + + Self::create_pyvenv(venv_abs_dir, &base_python_path, base_python_version.clone())?; + + let exe_path = install_paths.scripts().join(base_python_name); + + let abs_exe_path = venv_abs_dir.join(exe_path); + + Self::setup_python(&abs_exe_path, &base_python_path, base_python_version)?; + + Ok(Self { + location: venv_abs_dir.to_path_buf(), + install_paths, + }) + } + + /// Create all directories based on venv install paths mapping + pub fn create_install_paths( + venv_abs_path: &Path, + install_paths: &InstallPaths, + ) -> std::io::Result<()> { + if !venv_abs_path.exists() { + fs::create_dir_all(venv_abs_path)?; + } + + let libpath = Path::new(&venv_abs_path).join(install_paths.site_packages()); + let include_path = Path::new(&venv_abs_path).join(install_paths.include()); + let bin_path = Path::new(&venv_abs_path).join(install_paths.scripts()); + + let paths_to_create = [libpath, include_path, bin_path]; + + for path in paths_to_create.iter() { + if !path.exists() { + fs::create_dir_all(path)?; + } + } + + #[cfg(all(target_pointer_width = "64", unix, not(target_os = "macos")))] + { + let lib64 = venv_abs_path.join("lib64"); + if !lib64.exists() { + std::os::unix::fs::symlink("lib", lib64)?; + } + } + + Ok(()) + } + + /// create pyvenv.cfg and write it's content based on system python + pub fn create_pyvenv( + venv_path: &Path, + python_path: &Path, + python_version: PythonInterpreterVersion, + ) -> std::io::Result<()> { + let venv_name = venv_path + .file_name() + .and_then(OsStr::to_str) + .ok_or_else(|| { + std::io::Error::new( + ErrorKind::InvalidData, + format!( + "cannot extract base name from venv path {}", + venv_path.display() + ), + ) + })?; + + let pyenv_cfg_content = format!( + r#" +home = {} +include-system-site-packages = false +version = {}.{}.{} +prompt = {}"#, + python_path.parent().unwrap().display(), + python_version.major, + python_version.minor, + python_version.patch, + venv_name, + ); + + let cfg_path = Path::new(&venv_path).join("pyvenv.cfg"); + std::fs::write(cfg_path, pyenv_cfg_content)?; + Ok(()) + } + + /// copy original python executable and populate other suffixed binaries + pub fn setup_python( + venv_exe_path: &Path, + original_python_exe: &Path, + python_version: PythonInterpreterVersion, + ) -> std::io::Result<()> { + if !venv_exe_path.exists() { + copy_file(original_python_exe, venv_exe_path)?; + } + + println!("original python exe is {:?}", original_python_exe); + + let python_bins = [ + "python", + "python3", + &format!("python{}.{}", python_version.major, python_version.minor).to_string(), + ]; + + let venv_bin = venv_exe_path.parent().unwrap(); + + for bin_name in python_bins.into_iter() { + let venv_python_bin = venv_bin.join(bin_name); + if !venv_python_bin.exists() { + copy_file(venv_exe_path, &venv_python_bin)?; + } + } + + Ok(()) } /// Create a virtual environment at specified directory /// allows specifying if this is a windows venv + /// venv_dir is an absolute path pub fn create_custom( venv_dir: &Path, python: PythonLocation, @@ -155,13 +302,16 @@ mod tests { use super::VEnv; use crate::python_env::PythonLocation; use crate::types::NormalizedPackageName; - use std::path::Path; + use std::env; + use std::path::{Path, PathBuf}; use std::str::FromStr; #[test] pub fn venv_creation() { let venv_dir = tempfile::tempdir().unwrap(); - let venv = VEnv::create(venv_dir.path(), PythonLocation::System).unwrap(); + // let venv_dir = env::current_dir().unwrap().join(PathBuf::from(".my_direct_venv")); + let venv = VEnv::create(&venv_dir.path(), PythonLocation::System).unwrap(); + println!("PYTHON IS {:?}", venv.python_executable()); // Does python exist assert!(venv.python_executable().is_file()); @@ -187,4 +337,42 @@ mod tests { "('A d i E u ', False)" ); } + + #[test] + pub fn verify_base_and_venv_exec() { + // let venv_dir = tempfile::tempdir().unwrap(); + let venv_dir = PathBuf::from(".my_direct_venv"); + + let abs_venv_dir = env::current_dir().unwrap().join(venv_dir); + + // let create_venv = VEnv::ensure_directories(venv_dir.as_path(), PythonLocation::System).unwrap(); + let venv = VEnv::create(&abs_venv_dir, PythonLocation::System).unwrap(); + + println!("Path is {:?}", venv.location); + + // // Does python exist + // assert!(venv.python_executable().is_file()); + + // // Install wheel + // let wheel = crate::artifacts::Wheel::from_path( + // &Path::new(env!("CARGO_MANIFEST_DIR")) + // .join("../../test-data/wheels/wordle_python-2.3.32-py3-none-any.whl"), + // &NormalizedPackageName::from_str("wordle_python").unwrap(), + // ) + // .unwrap(); + // venv.install_wheel(&wheel, &Default::default()).unwrap(); + + // // See if it worked + // let output = venv + // .execute_script( + // &Path::new(env!("CARGO_MANIFEST_DIR")) + // .join("../../test-data/scripts/test_wordle.py"), + // ) + // .unwrap(); + + // assert_eq!( + // String::from_utf8(output.stdout).unwrap().trim(), + // "('A d i E u ', False)" + // ); + } } From cad08ee74962f2609944b8a3eaefd21cda3da99c Mon Sep 17 00:00:00 2001 From: nichmor Date: Tue, 19 Dec 2023 12:21:44 +0200 Subject: [PATCH 02/13] add tests to verify venv creation --- .../src/python_env/venv.rs | 130 ++++++++---------- 1 file changed, 57 insertions(+), 73 deletions(-) diff --git a/crates/rattler_installs_packages/src/python_env/venv.rs b/crates/rattler_installs_packages/src/python_env/venv.rs index 9a35d338..4e53fdf6 100644 --- a/crates/rattler_installs_packages/src/python_env/venv.rs +++ b/crates/rattler_installs_packages/src/python_env/venv.rs @@ -133,11 +133,13 @@ impl VEnv { /// Create a virtual environment at specified directory /// for the platform we are running on pub fn create(venv_dir: &Path, python: PythonLocation) -> Result { - Self::create_without_subprocess(venv_dir, python, cfg!(windows)) + Self::create_custom(venv_dir, python, cfg!(windows)) } - /// Ensure all directories are created - pub fn create_without_subprocess( + /// Create a virtual environment at specified directory + /// allows specifying if this is a windows venv + /// venv_dir is an absolute path + pub fn create_custom( venv_abs_dir: &Path, python: PythonLocation, windows: bool, @@ -187,6 +189,8 @@ impl VEnv { } } + /// # https://bugs.python.org/issue21197 + /// create lib64 as a symlink to lib on 64-bit non-OS X POSIX #[cfg(all(target_pointer_width = "64", unix, not(target_os = "macos")))] { let lib64 = venv_abs_path.join("lib64"); @@ -245,8 +249,6 @@ prompt = {}"#, copy_file(original_python_exe, venv_exe_path)?; } - println!("original python exe is {:?}", original_python_exe); - let python_bins = [ "python", "python3", @@ -264,37 +266,6 @@ prompt = {}"#, Ok(()) } - - /// Create a virtual environment at specified directory - /// allows specifying if this is a windows venv - /// venv_dir is an absolute path - pub fn create_custom( - venv_dir: &Path, - python: PythonLocation, - windows: bool, - ) -> Result { - // Find python executable - let python = python.executable()?; - - // Execute command - // Don't need pip for our use-case - let output = Command::new(&python) - .arg("-m") - .arg("venv") - .arg(venv_dir) - .arg("--without-pip") - .output()?; - - // Parse output - if !output.status.success() { - let stdout = String::from_utf8_lossy(&output.stderr); - return Err(VEnvError::FailedToRun(stdout.to_string())); - } - - let version = PythonInterpreterVersion::from_path(&python)?; - let install_paths = InstallPaths::for_venv(version, windows); - Ok(VEnv::new(venv_dir.to_path_buf(), install_paths)) - } } #[cfg(test)] @@ -309,9 +280,8 @@ mod tests { #[test] pub fn venv_creation() { let venv_dir = tempfile::tempdir().unwrap(); - // let venv_dir = env::current_dir().unwrap().join(PathBuf::from(".my_direct_venv")); let venv = VEnv::create(&venv_dir.path(), PythonLocation::System).unwrap(); - println!("PYTHON IS {:?}", venv.python_executable()); + // Does python exist assert!(venv.python_executable().is_file()); @@ -339,40 +309,54 @@ mod tests { } #[test] - pub fn verify_base_and_venv_exec() { - // let venv_dir = tempfile::tempdir().unwrap(); - let venv_dir = PathBuf::from(".my_direct_venv"); - - let abs_venv_dir = env::current_dir().unwrap().join(venv_dir); - - // let create_venv = VEnv::ensure_directories(venv_dir.as_path(), PythonLocation::System).unwrap(); - let venv = VEnv::create(&abs_venv_dir, PythonLocation::System).unwrap(); - - println!("Path is {:?}", venv.location); - - // // Does python exist - // assert!(venv.python_executable().is_file()); - - // // Install wheel - // let wheel = crate::artifacts::Wheel::from_path( - // &Path::new(env!("CARGO_MANIFEST_DIR")) - // .join("../../test-data/wheels/wordle_python-2.3.32-py3-none-any.whl"), - // &NormalizedPackageName::from_str("wordle_python").unwrap(), - // ) - // .unwrap(); - // venv.install_wheel(&wheel, &Default::default()).unwrap(); - - // // See if it worked - // let output = venv - // .execute_script( - // &Path::new(env!("CARGO_MANIFEST_DIR")) - // .join("../../test-data/scripts/test_wordle.py"), - // ) - // .unwrap(); - - // assert_eq!( - // String::from_utf8(output.stdout).unwrap().trim(), - // "('A d i E u ', False)" - // ); + pub fn test_python_set_env_prefix() { + let venv_dir = tempfile::tempdir().unwrap(); + + let venv = VEnv::create(venv_dir.path(), PythonLocation::System).unwrap(); + let system_exec = crate::python_env::system_python_executable().unwrap(); + + let base_prefix_output = venv + .execute_command("import sys; print(sys.base_prefix, end='')") + .unwrap(); + let base_prefix = String::from_utf8_lossy(&base_prefix_output.stdout); + + let venv_prefix_output = venv + .execute_command("import sys; print(sys.prefix, end='')") + .unwrap(); + let venv_prefix = String::from_utf8_lossy(&venv_prefix_output.stdout); + + assert!( + base_prefix != venv_prefix, + "base prefix of venv should be different from prefix" + ) + } + + #[test] + pub fn test_python_install_paths_are_created() { + let venv_dir = tempfile::tempdir().unwrap(); + + let venv = VEnv::create(venv_dir.path(), PythonLocation::System).unwrap(); + let install_paths = venv.install_paths; + + let platlib_path = venv_dir.path().join(install_paths.platlib()); + let scripts_path = venv_dir.path().join(install_paths.scripts()); + let include_path = venv_dir.path().join(install_paths.include()); + + assert!(platlib_path.exists(), "platlib path is not created"); + assert!(scripts_path.exists(), "scripts path is not created"); + assert!(include_path.exists(), "include path is not created"); + } + + #[test] + pub fn test_same_venv_can_be_created_twice() { + let venv_dir = tempfile::tempdir().unwrap(); + + let venv = VEnv::create(venv_dir.path(), PythonLocation::System).unwrap(); + let another_same_venv = VEnv::create(venv_dir.path(), PythonLocation::System).unwrap(); + + assert!( + venv.location == another_same_venv.location, + "same venv was not created in same location" + ) } } From ddf494c96b49f3a1c6408294a2e9847d3f471f24 Mon Sep 17 00:00:00 2001 From: nichmor Date: Tue, 19 Dec 2023 12:34:36 +0200 Subject: [PATCH 03/13] change unwrap to expect --- .../src/python_env/venv.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/crates/rattler_installs_packages/src/python_env/venv.rs b/crates/rattler_installs_packages/src/python_env/venv.rs index 4e53fdf6..6b8c9dad 100644 --- a/crates/rattler_installs_packages/src/python_env/venv.rs +++ b/crates/rattler_installs_packages/src/python_env/venv.rs @@ -189,7 +189,7 @@ impl VEnv { } } - /// # https://bugs.python.org/issue21197 + /// https://bugs.python.org/issue21197 /// create lib64 as a symlink to lib on 64-bit non-OS X POSIX #[cfg(all(target_pointer_width = "64", unix, not(target_os = "macos")))] { @@ -202,7 +202,7 @@ impl VEnv { Ok(()) } - /// create pyvenv.cfg and write it's content based on system python + /// Create pyvenv.cfg and write it's content based on system python pub fn create_pyvenv( venv_path: &Path, python_path: &Path, @@ -227,7 +227,10 @@ home = {} include-system-site-packages = false version = {}.{}.{} prompt = {}"#, - python_path.parent().unwrap().display(), + python_path + .parent() + .expect("system python path should have parent folder") + .display(), python_version.major, python_version.minor, python_version.patch, @@ -239,7 +242,7 @@ prompt = {}"#, Ok(()) } - /// copy original python executable and populate other suffixed binaries + /// Copy original python executable and populate other suffixed binaries pub fn setup_python( venv_exe_path: &Path, original_python_exe: &Path, @@ -255,7 +258,9 @@ prompt = {}"#, &format!("python{}.{}", python_version.major, python_version.minor).to_string(), ]; - let venv_bin = venv_exe_path.parent().unwrap(); + let venv_bin = venv_exe_path + .parent() + .expect("venv exe binary should have parent folder"); for bin_name in python_bins.into_iter() { let venv_python_bin = venv_bin.join(bin_name); @@ -274,7 +279,7 @@ mod tests { use crate::python_env::PythonLocation; use crate::types::NormalizedPackageName; use std::env; - use std::path::{Path, PathBuf}; + use std::path::Path; use std::str::FromStr; #[test] From 9f6c5391bed288737bd888cbcffc96320d04d079 Mon Sep 17 00:00:00 2001 From: nichmor Date: Tue, 19 Dec 2023 12:36:09 +0200 Subject: [PATCH 04/13] remove abs dir comment --- crates/rattler_installs_packages/src/python_env/venv.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/rattler_installs_packages/src/python_env/venv.rs b/crates/rattler_installs_packages/src/python_env/venv.rs index 6b8c9dad..b5b77cbb 100644 --- a/crates/rattler_installs_packages/src/python_env/venv.rs +++ b/crates/rattler_installs_packages/src/python_env/venv.rs @@ -138,7 +138,6 @@ impl VEnv { /// Create a virtual environment at specified directory /// allows specifying if this is a windows venv - /// venv_dir is an absolute path pub fn create_custom( venv_abs_dir: &Path, python: PythonLocation, From 379c5fedf4e569d833593eadfc3362cdd8018d5d Mon Sep 17 00:00:00 2001 From: nichmor Date: Tue, 19 Dec 2023 12:37:05 +0200 Subject: [PATCH 05/13] group statements --- crates/rattler_installs_packages/src/python_env/venv.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/rattler_installs_packages/src/python_env/venv.rs b/crates/rattler_installs_packages/src/python_env/venv.rs index b5b77cbb..3a063f70 100644 --- a/crates/rattler_installs_packages/src/python_env/venv.rs +++ b/crates/rattler_installs_packages/src/python_env/venv.rs @@ -152,11 +152,9 @@ impl VEnv { let install_paths = InstallPaths::for_venv(base_python_version.clone(), windows); Self::create_install_paths(venv_abs_dir, &install_paths)?; - Self::create_pyvenv(venv_abs_dir, &base_python_path, base_python_version.clone())?; let exe_path = install_paths.scripts().join(base_python_name); - let abs_exe_path = venv_abs_dir.join(exe_path); Self::setup_python(&abs_exe_path, &base_python_path, base_python_version)?; From e611cb002d92347f4d4d534881db6a2f5eeca542 Mon Sep 17 00:00:00 2001 From: nichmor Date: Tue, 19 Dec 2023 13:31:31 +0200 Subject: [PATCH 06/13] add windows implementation for setup_python --- .../src/python_env/venv.rs | 57 +++++++++++++++---- 1 file changed, 45 insertions(+), 12 deletions(-) diff --git a/crates/rattler_installs_packages/src/python_env/venv.rs b/crates/rattler_installs_packages/src/python_env/venv.rs index 3a063f70..cbcf7df8 100644 --- a/crates/rattler_installs_packages/src/python_env/venv.rs +++ b/crates/rattler_installs_packages/src/python_env/venv.rs @@ -186,8 +186,8 @@ impl VEnv { } } - /// https://bugs.python.org/issue21197 - /// create lib64 as a symlink to lib on 64-bit non-OS X POSIX + // https://bugs.python.org/issue21197 + // create lib64 as a symlink to lib on 64-bit non-OS X POSIX #[cfg(all(target_pointer_width = "64", unix, not(target_os = "macos")))] { let lib64 = venv_abs_path.join("lib64"); @@ -249,20 +249,53 @@ prompt = {}"#, copy_file(original_python_exe, venv_exe_path)?; } - let python_bins = [ - "python", - "python3", - &format!("python{}.{}", python_version.major, python_version.minor).to_string(), - ]; - let venv_bin = venv_exe_path .parent() .expect("venv exe binary should have parent folder"); - for bin_name in python_bins.into_iter() { - let venv_python_bin = venv_bin.join(bin_name); - if !venv_python_bin.exists() { - copy_file(venv_exe_path, &venv_python_bin)?; + #[cfg(not(windows))] + { + let python_bins = [ + "python", + "python3", + &format!("python{}.{}", python_version.major, python_version.minor).to_string(), + ]; + + for bin_name in python_bins.into_iter() { + let venv_python_bin = venv_bin.join(bin_name); + if !venv_python_bin.exists() { + copy_file(venv_exe_path, &venv_python_bin)?; + } + } + } + + #[cfg(windows)] + { + let base_exe_name = venv_exe_path + .file_name() + .expect("Cannot get windows venv exe name"); + let python_bins = [ + "python.exe", + "python_d.exe", + "pythonw.exe", + "pythonw_d.exe", + base_exe_name + .to_str() + .expect("cannot convert windows venv exe name"), + ]; + + let original_python_bin_dir = original_python_exe + .parent() + .expect("Cannot get system python parent folder"); + for bin_name in python_bins.into_iter() { + let original_python_bin = original_python_bin_dir.join(bin_name); + + if original_python_bin.exists() { + let venv_python_bin = venv_bin.join(bin_name); + if !venv_python_bin.exists() { + copy_file(venv_exe_path, &venv_python_bin)?; + } + } } } From 3c272ccc73c4c93984e3434c287c7358348aa550 Mon Sep 17 00:00:00 2001 From: nichmor Date: Tue, 19 Dec 2023 13:38:46 +0200 Subject: [PATCH 07/13] add missing clone trait --- crates/rattler_installs_packages/src/python_env/system_python.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/rattler_installs_packages/src/python_env/system_python.rs b/crates/rattler_installs_packages/src/python_env/system_python.rs index b7e784a6..83479681 100644 --- a/crates/rattler_installs_packages/src/python_env/system_python.rs +++ b/crates/rattler_installs_packages/src/python_env/system_python.rs @@ -44,6 +44,7 @@ pub enum ParsePythonInterpreterVersionError { FindPythonError(#[from] FindPythonError), } +#[derive(Clone)] pub struct PythonInterpreterVersion { pub major: u32, pub minor: u32, From 1d9908de61b5253b508d1f80ddb460f54db96dcf Mon Sep 17 00:00:00 2001 From: nichmor Date: Tue, 19 Dec 2023 13:46:27 +0200 Subject: [PATCH 08/13] use ::new instead of struct construct --- crates/rattler_installs_packages/src/python_env/venv.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/crates/rattler_installs_packages/src/python_env/venv.rs b/crates/rattler_installs_packages/src/python_env/venv.rs index cbcf7df8..3ec562cf 100644 --- a/crates/rattler_installs_packages/src/python_env/venv.rs +++ b/crates/rattler_installs_packages/src/python_env/venv.rs @@ -56,8 +56,6 @@ pub enum VEnvError { FindPythonError(#[from] FindPythonError), #[error(transparent)] ParsePythonInterpreterVersionError(#[from] ParsePythonInterpreterVersionError), - #[error("failed to run 'python -m venv': `{0}`")] - FailedToRun(String), #[error(transparent)] FailedToCreate(#[from] std::io::Error), } @@ -159,10 +157,7 @@ impl VEnv { Self::setup_python(&abs_exe_path, &base_python_path, base_python_version)?; - Ok(Self { - location: venv_abs_dir.to_path_buf(), - install_paths, - }) + Ok(VEnv::new(venv_abs_dir.to_path_buf(), install_paths)) } /// Create all directories based on venv install paths mapping @@ -348,7 +343,6 @@ mod tests { let venv_dir = tempfile::tempdir().unwrap(); let venv = VEnv::create(venv_dir.path(), PythonLocation::System).unwrap(); - let system_exec = crate::python_env::system_python_executable().unwrap(); let base_prefix_output = venv .execute_command("import sys; print(sys.base_prefix, end='')") From 88380463db72c6326928d2bc62a265ad6a327e61 Mon Sep 17 00:00:00 2001 From: nichmor Date: Tue, 19 Dec 2023 13:56:12 +0200 Subject: [PATCH 09/13] remove set permision on windows --- crates/rattler_installs_packages/src/python_env/venv.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/rattler_installs_packages/src/python_env/venv.rs b/crates/rattler_installs_packages/src/python_env/venv.rs index 3ec562cf..cf4c2422 100644 --- a/crates/rattler_installs_packages/src/python_env/venv.rs +++ b/crates/rattler_installs_packages/src/python_env/venv.rs @@ -25,7 +25,6 @@ pub fn copy_file, U: AsRef>(from: P, to: U) -> std::io::Res #[cfg(windows)] pub fn copy_file, U: AsRef>(from: P, to: U) -> std::io::Result<()> { fs::copy(from, to)?; - fs::set_permissions(to, fs::Permissions::from_mode(0o755)).unwrap(); Ok(()) } From 0cba97631a4fde6d6c289e2d76efab1a5352894a Mon Sep 17 00:00:00 2001 From: nichmor Date: Tue, 19 Dec 2023 17:45:38 +0200 Subject: [PATCH 10/13] add better error handling and optional windows parameter --- .../src/python_env/system_python.rs | 7 ++++++- crates/rattler_installs_packages/src/python_env/venv.rs | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/rattler_installs_packages/src/python_env/system_python.rs b/crates/rattler_installs_packages/src/python_env/system_python.rs index 83479681..dd9374bc 100644 --- a/crates/rattler_installs_packages/src/python_env/system_python.rs +++ b/crates/rattler_installs_packages/src/python_env/system_python.rs @@ -30,7 +30,12 @@ pub fn system_python_executable() -> Result { .map_err(|_| FindPythonError::NotFound)?; let stdout = String::from_utf8_lossy(&output.stdout); - let python_path = PathBuf::from_str(&stdout).map_err(|_| FindPythonError::NotFound)?; + let python_path = PathBuf::from_str(&stdout).unwrap(); + + // sys.executable can return empty string or python's None + if !python_path.exists() { + return Err(FindPythonError::NotFound); + } Ok(python_path) } diff --git a/crates/rattler_installs_packages/src/python_env/venv.rs b/crates/rattler_installs_packages/src/python_env/venv.rs index cf4c2422..a5ece1b2 100644 --- a/crates/rattler_installs_packages/src/python_env/venv.rs +++ b/crates/rattler_installs_packages/src/python_env/venv.rs @@ -237,7 +237,7 @@ prompt = {}"#, pub fn setup_python( venv_exe_path: &Path, original_python_exe: &Path, - python_version: PythonInterpreterVersion, + #[cfg(not(windows))] python_version: PythonInterpreterVersion, ) -> std::io::Result<()> { if !venv_exe_path.exists() { copy_file(original_python_exe, venv_exe_path)?; From ecd02c4a404d742877c52474dc058a5bcca184e7 Mon Sep 17 00:00:00 2001 From: nichmor Date: Tue, 19 Dec 2023 18:13:37 +0200 Subject: [PATCH 11/13] add matching for finding system executable --- .../src/python_env/system_python.rs | 10 +++++++--- .../rattler_installs_packages/src/python_env/venv.rs | 10 +++++++++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/crates/rattler_installs_packages/src/python_env/system_python.rs b/crates/rattler_installs_packages/src/python_env/system_python.rs index dd9374bc..e1a0ed9e 100644 --- a/crates/rattler_installs_packages/src/python_env/system_python.rs +++ b/crates/rattler_installs_packages/src/python_env/system_python.rs @@ -17,7 +17,8 @@ pub fn system_python_executable() -> Result { // When installed with homebrew on macOS, the python3 executable is called `python3` instead // Also on some ubuntu installs this is the case // For windows it should just be python - let output = std::process::Command::new("python3") + + let output = match std::process::Command::new("python3") .arg("-c") .arg("import sys; print(sys.executable, end='')") .output() @@ -26,8 +27,11 @@ pub fn system_python_executable() -> Result { .arg("-c") .arg("import sys; print(sys.executable, end='')") .output() - }) - .map_err(|_| FindPythonError::NotFound)?; + }) { + Err(e) if e.kind() == ErrorKind::NotFound => return Err(FindPythonError::NotFound), + Err(e) => return Err(FindPythonError::IoError(e)), + Ok(output) => output, + }; let stdout = String::from_utf8_lossy(&output.stdout); let python_path = PathBuf::from_str(&stdout).unwrap(); diff --git a/crates/rattler_installs_packages/src/python_env/venv.rs b/crates/rattler_installs_packages/src/python_env/venv.rs index a5ece1b2..5b523e10 100644 --- a/crates/rattler_installs_packages/src/python_env/venv.rs +++ b/crates/rattler_installs_packages/src/python_env/venv.rs @@ -154,7 +154,15 @@ impl VEnv { let exe_path = install_paths.scripts().join(base_python_name); let abs_exe_path = venv_abs_dir.join(exe_path); - Self::setup_python(&abs_exe_path, &base_python_path, base_python_version)?; + #[cfg(not(windows))] + { + Self::setup_python(&abs_exe_path, &base_python_path, base_python_version)?; + } + + #[cfg(windows)] + { + Self::setup_python(&abs_exe_path, &base_python_path)?; + } Ok(VEnv::new(venv_abs_dir.to_path_buf(), install_paths)) } From 45ab4924f15482c846d5c7e8c5d89d5fbd155334 Mon Sep 17 00:00:00 2001 From: nichmor Date: Tue, 19 Dec 2023 19:37:07 +0200 Subject: [PATCH 12/13] make expect lowercase --- crates/rattler_installs_packages/src/python_env/venv.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/rattler_installs_packages/src/python_env/venv.rs b/crates/rattler_installs_packages/src/python_env/venv.rs index 5b523e10..e65ef255 100644 --- a/crates/rattler_installs_packages/src/python_env/venv.rs +++ b/crates/rattler_installs_packages/src/python_env/venv.rs @@ -275,7 +275,7 @@ prompt = {}"#, { let base_exe_name = venv_exe_path .file_name() - .expect("Cannot get windows venv exe name"); + .expect("cannot get windows venv exe name"); let python_bins = [ "python.exe", "python_d.exe", @@ -288,7 +288,7 @@ prompt = {}"#, let original_python_bin_dir = original_python_exe .parent() - .expect("Cannot get system python parent folder"); + .expect("cannot get system python parent folder"); for bin_name in python_bins.into_iter() { let original_python_bin = original_python_bin_dir.join(bin_name); From f6aed6eb5b98520f0229512a6f4ca6cd10550ec8 Mon Sep 17 00:00:00 2001 From: nichmor Date: Tue, 19 Dec 2023 20:16:20 +0200 Subject: [PATCH 13/13] add errorkind trait * add errorkind trait --- .../rattler_installs_packages/src/python_env/system_python.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/rattler_installs_packages/src/python_env/system_python.rs b/crates/rattler_installs_packages/src/python_env/system_python.rs index e1a0ed9e..110251be 100644 --- a/crates/rattler_installs_packages/src/python_env/system_python.rs +++ b/crates/rattler_installs_packages/src/python_env/system_python.rs @@ -1,4 +1,5 @@ use itertools::Itertools; +use std::io::ErrorKind; use std::path::{Path, PathBuf}; use std::str::FromStr; use thiserror::Error; @@ -8,6 +9,8 @@ use thiserror::Error; pub enum FindPythonError { #[error("could not find python executable")] NotFound, + #[error(transparent)] + IoError(#[from] std::io::Error), } /// Try to find the python executable in the current environment.