diff --git a/crates/pet-core/src/os_environment.rs b/crates/pet-core/src/os_environment.rs index da800953..7a8b6ef7 100644 --- a/crates/pet-core/src/os_environment.rs +++ b/crates/pet-core/src/os_environment.rs @@ -53,7 +53,7 @@ impl Environment for EnvironmentApi { .into_iter() .filter(|p| p.exists()) .collect::>(); - + trace!("Env PATH: {:?}", paths); self.global_search_locations .lock() .unwrap() diff --git a/crates/pet-homebrew/src/environments.rs b/crates/pet-homebrew/src/environments.rs index 063dc1c0..4ee970da 100644 --- a/crates/pet-homebrew/src/environments.rs +++ b/crates/pet-homebrew/src/environments.rs @@ -26,7 +26,10 @@ pub fn get_python_info( None => None, }; - let mut symlinks = vec![python_exe_from_bin_dir.to_path_buf()]; + let mut symlinks = vec![ + python_exe_from_bin_dir.to_path_buf(), + resolved_exe.to_path_buf(), + ]; if let Some(version) = &version { symlinks.append(&mut get_known_symlinks(resolved_exe, version)); } @@ -92,7 +95,7 @@ fn get_prefix(_resolved_file: &Path) -> Option { // } // // 2. Linux - // if resolved_file.starts_with("/home/linuxbrew/.linuxbrew/Cellar") { + // if resolved_file.starts_with("/home/linuxbrew/.linuxbrew") { // // Resolved exe is something like `/home/linuxbrew/.linuxbrew/Cellar/python@3.12/3.12.3/bin/python3.12` // let reg_ex = Regex::new("/home/linuxbrew/.linuxbrew/Cellar/python@(\\d+\\.?\\d+\\.?)/(\\d+\\.?\\d+\\.?\\d+\\.?)/bin/python.*").unwrap(); // let captures = reg_ex.captures(resolved_file)?; diff --git a/crates/pet-homebrew/src/lib.rs b/crates/pet-homebrew/src/lib.rs index b1448127..bb5e5cdc 100644 --- a/crates/pet-homebrew/src/lib.rs +++ b/crates/pet-homebrew/src/lib.rs @@ -13,7 +13,8 @@ use pet_core::{ use pet_fs::path::resolve_symlink; use pet_python_utils::{env::PythonEnv, executable::find_executables}; use pet_virtualenv::is_virtualenv; -use std::{path::PathBuf, thread}; +use std::{fs, path::PathBuf, thread}; +use sym_links::is_homebrew_python; mod env_variables; mod environment_locations; @@ -52,7 +53,17 @@ fn from(env: &PythonEnv) -> Option { // Hence we never end up reporting 3.10 for home brew (as mentioned when you try to resolve the exe it points to existing install, now homebrew). let exe = env.executable.clone(); let exe_file_name = exe.file_name()?; - let resolved_file = resolve_symlink(&exe).unwrap_or(exe.clone()); + let mut resolved_file = resolve_symlink(&exe).unwrap_or(exe.clone()); + + // Possible the resolve exe needs to be resolved once again using canonicalize. + // Sometimes a symlink points to another symlink, and we need to resolve it to get the real exe. + // And for some reason even though they are symlinks, they are not resolved by `resolve_symlink`. + if let Some(resolved) = resolve_symlink(&exe).or(fs::canonicalize(&exe).ok()) { + if is_homebrew_python(&resolved) { + resolved_file = resolved; + } + } + // Cellar is where the executables will be installed, see below link // https://docs.brew.sh/Formula-Cookbook#an-introduction // From above link > Homebrew installs formulae to the Cellar at $(brew --cellar) @@ -72,7 +83,7 @@ fn from(env: &PythonEnv) -> Option { &PathBuf::from("/opt/homebrew/bin").join(exe_file_name), &resolved_file, ) - } else if resolved_file.starts_with("/home/linuxbrew/.linuxbrew/Cellar") { + } else if resolved_file.starts_with("/home/linuxbrew/.linuxbrew") { // Symlink - /home/linuxbrew/.linuxbrew/bin/python3.12 // Symlink - /home/linuxbrew/.linuxbrew/opt/python@3.12/bin/python3.12 // Real exe - /home/linuxbrew/.linuxbrew/Cellar/python@3.12/3.12.3/bin/python3.12 diff --git a/crates/pet-homebrew/src/sym_links.rs b/crates/pet-homebrew/src/sym_links.rs index fdf8efb5..2162c62e 100644 --- a/crates/pet-homebrew/src/sym_links.rs +++ b/crates/pet-homebrew/src/sym_links.rs @@ -5,13 +5,22 @@ use lazy_static::lazy_static; use pet_fs::path::resolve_symlink; use pet_python_utils::executable::find_executables; use regex::Regex; -use std::path::{Path, PathBuf}; +use std::{ + fs, + path::{Path, PathBuf}, +}; lazy_static! { static ref PYTHON_VERSION: Regex = Regex::new(r"/python@((\d+\.?)*)/").expect("error parsing Version regex for Homebrew"); } +pub fn is_homebrew_python(exe: &Path) -> bool { + exe.starts_with("/opt/homebrew/Cellar") + || exe.starts_with("/usr/local/Cellar") + || exe.starts_with("/home/linuxbrew/.linuxbrew") +} + pub fn get_known_symlinks( symlink_resolved_python_exe: &Path, full_version: &String, @@ -85,6 +94,10 @@ pub fn get_known_symlinks_impl( PathBuf::from(format!("/opt/homebrew/Cellar/python@{}/{}/Frameworks/Python.framework/Versions/Current/bin/python{}", version, full_version, version)), PathBuf::from(format!("/opt/homebrew/Frameworks/Python.framework/Versions/{}/bin/python{}", version, version)), PathBuf::from(format!("/opt/homebrew/Frameworks/Python.framework/Versions/Current/bin/python{}", version)), + PathBuf::from(format!("/usr/local/opt/python@{}/bin/python3", version)), + PathBuf::from(format!("/usr/local/opt/python@{}/bin/python{}", version, version)), + PathBuf::from("/usr/local/opt/python@3/bin/python3"), + PathBuf::from(format!("/usr/local/opt/python@3/bin/python{}", version)), // Check if this symlink is pointing to the same place as the resolved python exe PathBuf::from(format!("/opt/homebrew/opt/python3/bin/python{}", version)), // Check if this symlink is pointing to the same place as the resolved python exe @@ -94,7 +107,11 @@ pub fn get_known_symlinks_impl( ] { // Validate the symlinks - if possible_symlink == symlink_resolved_python_exe || resolve_symlink(&possible_symlink).unwrap_or_default() == symlink_resolved_python_exe { + if symlinks.contains( + &resolve_symlink(&possible_symlink) + .or(fs::canonicalize(&possible_symlink).ok()) + .unwrap_or_default(), + ) { symlinks.push(possible_symlink); } } @@ -125,12 +142,10 @@ pub fn get_known_symlinks_impl( // 1. python 3.8 has sysprefix in /usr/local/Cellar/python@3.9/3.9.19/Frameworks/Python.framework/Versions/3.9 // 2. python 3.9 has sysprefix in /usr/local/opt/python@3.9/Frameworks/Python.framework/Versions/3.9 // 3. python 3.11 has sysprefix in /usr/local/opt/python@3.11/Frameworks/Python.framework/Versions/3.11 - // Hence till we know more about it, this path is not included, but left as commented - // So we can add it later if needed & tested - // PathBuf::from(format!( - // "/usr/local/opt/python@{}/bin/python{}", - // version, version - // )), + PathBuf::from(format!("/usr/local/opt/python@{}/bin/python3", version)), + PathBuf::from(format!("/usr/local/opt/python@{}/bin/python{}", version, version)), + PathBuf::from("/usr/local/opt/python@3/bin/python3"), + PathBuf::from(format!("/usr/local/opt/python@3/bin/python{}", version)), PathBuf::from(format!( "/usr/local/Cellar/python@{}/{}/bin/python{}", version, full_version, version @@ -149,7 +164,11 @@ pub fn get_known_symlinks_impl( ] { // Validate the symlinks - if possible_symlink == symlink_resolved_python_exe || resolve_symlink(&possible_symlink).unwrap_or_default() == symlink_resolved_python_exe { + if symlinks.contains( + &resolve_symlink(&possible_symlink) + // .or(fs::canonicalize(&possible_symlink).ok()) + .unwrap_or_default(), + ) { symlinks.push(possible_symlink); } } @@ -160,7 +179,7 @@ pub fn get_known_symlinks_impl( }, None => vec![], } - } else if symlink_resolved_python_exe.starts_with("/home/linuxbrew/.linuxbrew/Cellar") { + } else if symlink_resolved_python_exe.starts_with("/home/linuxbrew/.linuxbrew") { // Real exe - /home/linuxbrew/.linuxbrew/Cellar/python@3.12/3.12.3/bin/python3.12 // Known symlinks include @@ -175,11 +194,24 @@ pub fn get_known_symlinks_impl( // See previous explanation let mut symlinks = vec![symlink_resolved_python_exe.to_owned()]; for possible_symlink in [ + PathBuf::from("/home/linuxbrew/.linuxbrew/bin/python3"), PathBuf::from(format!("/home/linuxbrew/.linuxbrew/bin/python{}", version)), + PathBuf::from(format!( + "/home/linuxbrew/.linuxbrew/Cellar/python@{}/{}/bin/python{}", + version, full_version, version + )), + PathBuf::from(format!( + "/home/linuxbrew/.linuxbrew/Cellar/python@{}/{}/bin/python3", + version, full_version + )), PathBuf::from(format!( "/home/linuxbrew/.linuxbrew/opt/python@{}/bin/python{}", version, version )), + PathBuf::from(format!( + "/home/linuxbrew/.linuxbrew/opt/python@{}/bin/python3", + version + )), // This is a special folder, if users install python using other means, this file // might get overridden. So we should only add this if this files points to the same place PathBuf::from(format!("/usr/local/bin/python{}", version)), @@ -189,10 +221,11 @@ pub fn get_known_symlinks_impl( PathBuf::from("/usr/local/bin/python"), ] { // Validate the symlinks - if possible_symlink == symlink_resolved_python_exe - || resolve_symlink(&possible_symlink).unwrap_or_default() - == symlink_resolved_python_exe - { + if symlinks.contains( + &resolve_symlink(&possible_symlink) + .or(fs::canonicalize(&possible_symlink).ok()) + .unwrap_or_default(), + ) { symlinks.push(possible_symlink); } } diff --git a/crates/pet/tests/ci_homebrew_container.rs b/crates/pet/tests/ci_homebrew_container.rs index 1fc12ddd..15903b25 100644 --- a/crates/pet/tests/ci_homebrew_container.rs +++ b/crates/pet/tests/ci_homebrew_container.rs @@ -37,6 +37,8 @@ fn verify_python_in_homebrew_contaner() { symlinks: Some(vec![ PathBuf::from("/home/linuxbrew/.linuxbrew/bin/python3"), PathBuf::from("/home/linuxbrew/.linuxbrew/bin/python3.12"), + PathBuf::from("/home/linuxbrew/.linuxbrew/opt/python@3.12/bin/python3"), + PathBuf::from("/home/linuxbrew/.linuxbrew/opt/python@3.12/bin/python3.12"), // On CI the Python version can change with minor updates, so we don't check the full version. // PathBuf::from("/home/linuxbrew/.linuxbrew/Cellar/python@3.12/3.12.4/bin/python3.12"), ]), @@ -48,6 +50,7 @@ fn verify_python_in_homebrew_contaner() { version: Some("3.11.9".to_string()), // This can change on CI, so we don't check it symlinks: Some(vec![ PathBuf::from("/home/linuxbrew/.linuxbrew/bin/python3.11"), + PathBuf::from("/home/linuxbrew/.linuxbrew/opt/python@3.11/bin/python3.11"), // On CI the Python version can change with minor updates, so we don't check the full version. // PathBuf::from("/home/linuxbrew/.linuxbrew/Cellar/python@3.11/3.11.9/bin/python3.11"), ]), diff --git a/crates/pet/tests/ci_test.rs b/crates/pet/tests/ci_test.rs index b42dc5eb..ef75a40d 100644 --- a/crates/pet/tests/ci_test.rs +++ b/crates/pet/tests/ci_test.rs @@ -1,11 +1,11 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use std::{env, path::PathBuf, sync::Once}; +use std::{path::PathBuf, sync::Once}; use common::{does_version_match, resolve_test_path}; use lazy_static::lazy_static; -use log::{error, trace, warn}; +use log::{error, trace}; use pet::{locators::identify_python_environment_using_locators, resolve::resolve_environment}; use pet_core::{ arch::Architecture, @@ -13,7 +13,6 @@ use pet_core::{ }; use pet_env_var_path::get_search_paths_from_env_variables; use pet_python_utils::env::PythonEnv; -use pet_reporter::environment; use regex::Regex; use serde::Deserialize; @@ -35,7 +34,6 @@ fn setup() { mod common; -// #[cfg(unix)] #[cfg_attr( any( feature = "ci", @@ -109,7 +107,7 @@ fn verify_validity_of_discovered_envs() { verify_we_can_get_same_env_info_using_from_with_exe(exe, environment.clone()); // Verification 4 & 5: // Similarly for each environment use resolve method and verify we get the exact same information. - // verify_we_can_get_same_env_info_using_resolve_with_exe(exe, environment.clone()); + verify_we_can_get_same_env_info_using_resolve_with_exe(exe, environment.clone()); } })); } @@ -337,17 +335,17 @@ fn verify_we_can_get_same_env_info_using_from_with_exe( ); } -fn compare_environments(env: PythonEnvironment, environment: PythonEnvironment, method: &str) { - let mut env = env.clone(); - let mut environment = environment.clone(); +fn compare_environments(actual: PythonEnvironment, expected: PythonEnvironment, method: &str) { + let mut actual = actual.clone(); + let mut expected = expected.clone(); assert_eq!( - env.category, - environment.clone().category, + actual.category, + expected.clone().category, "Category mismatch when using {} for {:?} and {:?}", method, - environment, - env + expected, + actual ); // if env.category != environment.clone().category { @@ -358,7 +356,7 @@ fn compare_environments(env: PythonEnvironment, environment: PythonEnvironment, // } if let (Some(version), Some(expected_version)) = - (environment.clone().version, env.clone().version) + (expected.clone().version, actual.clone().version) { assert!( does_version_match(&version, &expected_version), @@ -366,8 +364,8 @@ fn compare_environments(env: PythonEnvironment, environment: PythonEnvironment, method, expected_version, version, - env.clone(), - environment.clone() + actual.clone(), + expected.clone() ); // if !does_version_match(&version, &expected_version) { // error!("Version mismatch when using {} for (expected {:?} to start with {:?}) for env = {:?} and environment = {:?}", @@ -381,31 +379,32 @@ fn compare_environments(env: PythonEnvironment, environment: PythonEnvironment, } // We have compared the versions, now ensure they are treated as the same // So that we can compare the objects easily - env.version = environment.clone().version; + actual.version = expected.clone().version; - if let Some(prefix) = environment.clone().prefix { - if env.clone().executable == Some(PathBuf::from("/usr/local/python/current/bin/python")) + if let Some(prefix) = expected.clone().prefix { + if actual.clone().executable == Some(PathBuf::from("/usr/local/python/current/bin/python")) && (prefix.to_str().unwrap() == "/usr/local/python/current" - && env.clone().prefix == Some(PathBuf::from("/usr/local/python/3.10.13"))) + && actual.clone().prefix == Some(PathBuf::from("/usr/local/python/3.10.13"))) || (prefix.to_str().unwrap() == "/usr/local/python/3.10.13" - && env.clone().prefix == Some(PathBuf::from("/usr/local/python/current"))) + && actual.clone().prefix == Some(PathBuf::from("/usr/local/python/current"))) { // known issue https://github.com/microsoft/python-environment-tools/issues/64 - env.prefix = environment.clone().prefix; - } else if env.clone().executable + actual.prefix = expected.clone().prefix; + } else if actual.clone().executable == Some(PathBuf::from("/home/codespace/.python/current/bin/python")) && (prefix.to_str().unwrap() == "/home/codespace/.python/current" - && env.clone().prefix == Some(PathBuf::from("/usr/local/python/3.10.13"))) + && actual.clone().prefix == Some(PathBuf::from("/usr/local/python/3.10.13"))) || (prefix.to_str().unwrap() == "/usr/local/python/3.10.13" - && env.clone().prefix == Some(PathBuf::from("/home/codespace/.python/current"))) + && actual.clone().prefix == Some(PathBuf::from("/home/codespace/.python/current"))) { // known issue https://github.com/microsoft/python-environment-tools/issues/64 - env.prefix = environment.clone().prefix; + actual.prefix = expected.clone().prefix; } } // known issue - env.symlinks = Some( - env.clone() + actual.symlinks = Some( + actual + .clone() .symlinks .unwrap_or_default() .iter() @@ -422,8 +421,8 @@ fn compare_environments(env: PythonEnvironment, environment: PythonEnvironment, .map(|p| p.to_path_buf()) .collect::>(), ); - environment.symlinks = Some( - environment + expected.symlinks = Some( + expected .clone() .symlinks .unwrap_or_default() @@ -443,31 +442,31 @@ fn compare_environments(env: PythonEnvironment, environment: PythonEnvironment, ); // if we know the arch, then verify it - if environment.arch.as_ref().is_some() && env.arch.as_ref().is_some() { - if env.arch.as_ref() != environment.arch.as_ref() { + if expected.arch.as_ref().is_some() && actual.arch.as_ref().is_some() { + if actual.arch.as_ref() != expected.arch.as_ref() { error!( "Arch mismatch when using {} for {:?} and {:?}", - method, environment, env + method, expected, actual ); } } - env.arch = environment.clone().arch; + actual.arch = expected.clone().arch; // if we know the prefix, then verify it - if environment.prefix.as_ref().is_some() && env.prefix.as_ref().is_some() { - if env.prefix.as_ref() != environment.prefix.as_ref() { + if expected.prefix.as_ref().is_some() && actual.prefix.as_ref().is_some() { + if actual.prefix.as_ref() != expected.prefix.as_ref() { error!( "Prefirx mismatch when using {} for {:?} and {:?}", - method, environment, env + method, expected, actual ); } } - env.prefix = environment.clone().prefix; + actual.prefix = expected.clone().prefix; assert_eq!( - env, environment, + actual, expected, "Environment mismatch when using {} for {:?}", - method, environment + method, expected ); // if env != environment { @@ -523,8 +522,8 @@ fn verify_we_can_get_same_env_info_using_resolve_with_exe( return; } compare_environments( - environment, env.resolved.unwrap(), + environment, format!("resolve using exe {:?}", executable).as_str(), ); }