Fixes for python in /bin, /usr/bin, /usr/local/bin (#54)

For #53

Author: DonJayamanne

crates/pet-linux-global-python/src/lib.rs

@@ -1,9 +1,9 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-use std::{fs, path::PathBuf};
+use std::fs;
 
-use log::error;
+use log::warn;
 use pet_core::{
     python_environment::{PythonEnvironment, PythonEnvironmentBuilder, PythonEnvironmentCategory},
     reporter::Reporter,
@@ -44,34 +44,35 @@ impl Locator for LinuxGlobalPython {
             return None;
         }
 
-        if let (Some(prefix), Some(_)) = (env.prefix.clone(), env.version.clone()) {
-            let executable = env.executable.clone();
-
-            // If prefix or version is not available, then we cannot use this method.
-            // 1. For files in /bin or /usr/bin, the prefix is always /usr
-            // 2. For files in /usr/local/bin, the prefix is always /usr/local
-            if !executable.starts_with("/bin")
-                && !executable.starts_with("/usr/bin")
-                && !executable.starts_with("/usr/local/bin")
-                && !prefix.starts_with("/usr")
-                && !prefix.starts_with("/usr/local")
-            {
-                return None;
-            }
+        // If we do not have a version, then we cannot use this method.
+        // Without version means we have not spawned the Python exe, thus do not have the real info.
+        env.version.clone()?;
+        let prefix = env.prefix.clone()?;
+        let executable = env.executable.clone();
+
+        // If prefix or version is not available, then we cannot use this method.
+        // 1. For files in /bin or /usr/bin, the prefix is always /usr
+        // 2. For files in /usr/local/bin, the prefix is always /usr/local
+        if !executable.starts_with("/bin")
+            && !executable.starts_with("/usr/bin")
+            && !executable.starts_with("/usr/local/bin")
+            && !prefix.starts_with("/usr")
+            && !prefix.starts_with("/usr/local")
+        {
+            return None;
+        }
 
-            // All known global linux are always installed in `/bin` or `/usr/bin`
-            if executable.starts_with("/bin") || executable.starts_with("/usr/bin") {
-                get_python_in_usr_bin(env)
-            } else if executable.starts_with("/usr/local/bin") {
-                get_python_in_usr_local_bin(env)
-            } else {
-                error!(
-                    "Invalid state, ex ({:?}) is not in any of /bin, /usr/bin, /usr/local/bin",
+        // All known global linux are always installed in `/bin` or `/usr/bin` or `/usr/local/bin`
+        if executable.starts_with("/bin")
+            || executable.starts_with("/usr/bin")
+            || executable.starts_with("/usr/local/bin")
+        {
+            get_python_in_bin(env)
+        } else {
+            warn!(
+                    "Unknown Python exe ({:?}), not in any of the known locations /bin, /usr/bin, /usr/local/bin",
                     executable
                 );
-                None
-            }
-        } else {
             None
         }
     }
@@ -84,7 +85,7 @@ impl Locator for LinuxGlobalPython {
     }
 }
 
-fn get_python_in_usr_bin(env: &PythonEnv) -> Option<PythonEnvironment> {
+fn get_python_in_bin(env: &PythonEnv) -> Option<PythonEnvironment> {
     // If we do not have the prefix, then do not try
     // This method will be called with resolved Python where prefix & version is available.
     if env.version.clone().is_none() || env.prefix.clone().is_none() {
@@ -94,86 +95,54 @@ fn get_python_in_usr_bin(env: &PythonEnv) -> Option<PythonEnvironment> {
     let mut symlinks = env.symlinks.clone().unwrap_or_default();
     symlinks.push(executable.clone());
 
-    let bin = PathBuf::from("/bin");
-    let usr_bin = PathBuf::from("/usr/bin");
+    let bin = executable.parent()?;
+
+    // Keep track of what the exe resolves to.
+    // Will have a value only if the exe is in another dir
+    // E.g. /bin/python3 might be a symlink to /usr/bin/python3.12
+    // However due to legacy reasons we'll be treating these two as separate exes.
+    // Hence they will be separate Python environments.
+    let mut resolved_exe_is_from_another_dir = None;
 
     // Possible this exe is a symlink to another file in the same directory.
-    // E.g. /usr/bin/python3 is a symlink to /usr/bin/python3.12
+    // E.g. Generally /usr/bin/python3 is a symlink to /usr/bin/python3.12
+    // E.g. Generally /usr/local/bin/python3 is a symlink to /usr/local/bin/python3.12
+    // E.g. Generally /bin/python3 is a symlink to /bin/python3.12
     // let bin = executable.parent()?;
     // We use canonicalize to get the real path of the symlink.
     // Only used in this case, see notes for resolve_symlink.
     if let Some(symlink) = resolve_symlink(&executable).or(fs::canonicalize(&executable).ok()) {
         // Ensure this is a symlink in the bin or usr/bin directory.
-        if symlink.starts_with(&bin) || symlink.starts_with(&usr_bin) {
+        if symlink.starts_with(bin) {
             symlinks.push(symlink);
+        } else {
+            resolved_exe_is_from_another_dir = Some(symlink);
         }
     }
 
-    // Look for other symlinks in /usr/bin and /bin folder
-    // https://stackoverflow.com/questions/68728225/what-is-the-difference-between-usr-bin-python3-and-bin-python3
-    // We know that on linux there are symlinks in both places.
+    // Look for other symlinks in the same folder
+    // We know that on linux there are sym links in the same folder as the exe.
     // & they all point to one exe and have the same version and same prefix.
-    for possible_symlink in [find_executables(&bin), find_executables(&usr_bin)].concat() {
-        if let Some(symlink) =
-            resolve_symlink(&possible_symlink).or(fs::canonicalize(&possible_symlink).ok())
+    for possible_symlink in find_executables(bin).iter() {
+        if let Some(ref symlink) =
+            resolve_symlink(&possible_symlink).or(fs::canonicalize(possible_symlink).ok())
         {
-            // the file /bin/python3 is a symlink to /usr/bin/python3.12
-            // the file /bin/python3.12 is a symlink to /usr/bin/python3.12
-            // the file /usr/bin/python3 is a symlink to /usr/bin/python3.12
-            // Thus we have 3 symlinks pointing to the same exe /usr/bin/python3.12
-            if symlinks.contains(&symlink) {
-                symlinks.push(possible_symlink);
+            // Generally the file /bin/python3 is a symlink to /usr/bin/python3.12
+            // Generally the file /bin/python3.12 is a symlink to /usr/bin/python3.12
+            // Generally the file /usr/bin/python3 is a symlink to /usr/bin/python3.12
+            // HOWEVER, we will be treating the files in /bin and /usr/bin as different.
+            // Hence check whether the resolve symlink is in the same directory.
+            if symlink.starts_with(bin) & symlinks.contains(symlink) {
+                symlinks.push(possible_symlink.to_owned());
             }
-        }
-    }
-    symlinks.sort();
-    symlinks.dedup();
-
-    Some(
-        PythonEnvironmentBuilder::new(PythonEnvironmentCategory::LinuxGlobal)
-            .executable(Some(executable))
-            .version(env.version.clone())
-            .prefix(env.prefix.clone())
-            .symlinks(Some(symlinks))
-            .build(),
-    )
-}
-
-fn get_python_in_usr_local_bin(env: &PythonEnv) -> Option<PythonEnvironment> {
-    // If we do not have the prefix, then do not try
-    // This method will be called with resolved Python where prefix & version is available.
-    if env.version.clone().is_none() || env.prefix.clone().is_none() {
-        return None;
-    }
-    let executable = env.executable.clone();
-    let mut symlinks = env.symlinks.clone().unwrap_or_default();
-    symlinks.push(executable.clone());
 
-    let usr_local_bin = PathBuf::from("/usr/local/bin");
-
-    // Possible this exe is a symlink to another file in the same directory.
-    // E.g. /usr/local/bin/python3 could be a symlink to /usr/local/bin/python3.12
-    // let bin = executable.parent()?;
-    // We use canonicalize to get the real path of the symlink.
-    // Only used in this case, see notes for resolve_symlink.
-    if let Some(symlink) = resolve_symlink(&executable).or(fs::canonicalize(&executable).ok()) {
-        // Ensure this is a symlink in the bin or usr/local/bin directory.
-        if symlink.starts_with(&usr_local_bin) {
-            symlinks.push(symlink);
-        }
-    }
-
-    // Look for other symlinks in this same folder
-    for possible_symlink in find_executables(&usr_local_bin) {
-        if let Some(symlink) =
-            resolve_symlink(&possible_symlink).or(fs::canonicalize(&possible_symlink).ok())
-        {
-            // the file /bin/python3 is a symlink to /usr/bin/python3.12
-            // the file /bin/python3.12 is a symlink to /usr/bin/python3.12
-            // the file /usr/bin/python3 is a symlink to /usr/bin/python3.12
-            // Thus we have 3 symlinks pointing to the same exe /usr/bin/python3.12
-            if symlinks.contains(&symlink) {
-                symlinks.push(possible_symlink);
+            // Possible the env.executable = /bin/python3
+            // And the possible_symlink = /bin/python3.12
+            // & possible that both of the above are symlinks and point to /usr/bin/python3.12
+            // In this case /bin/python3 === /bin/python.3.12
+            // However as mentioned earlier we will not be treating these the same as /usr/bin/python3.12
+            if resolved_exe_is_from_another_dir == Some(symlink.to_owned()) {
+                symlinks.push(possible_symlink.to_owned());
             }
         }
     }

crates/pet/src/find.rs

@@ -14,7 +14,7 @@ use pet_python_utils::env::PythonEnv;
 use pet_python_utils::executable::{
     find_executable, find_executables, should_search_for_environments_in_path,
 };
-use std::collections::HashMap;
+use std::collections::BTreeMap;
 use std::fs;
 use std::path::PathBuf;
 use std::sync::Mutex;
@@ -25,7 +25,7 @@ use crate::locators::identify_python_environment_using_locators;
 
 pub struct Summary {
     pub time: Duration,
-    pub find_locators_times: HashMap<&'static str, Duration>,
+    pub find_locators_times: BTreeMap<&'static str, Duration>,
     pub find_locators_time: Duration,
     pub find_path_time: Duration,
     pub find_global_virtual_envs_time: Duration,
@@ -40,7 +40,7 @@ pub fn find_and_report_envs(
 ) -> Arc<Mutex<Summary>> {
     let summary = Arc::new(Mutex::new(Summary {
         time: Duration::from_secs(0),
-        find_locators_times: HashMap::new(),
+        find_locators_times: BTreeMap::new(),
         find_locators_time: Duration::from_secs(0),
         find_path_time: Duration::from_secs(0),
         find_global_virtual_envs_time: Duration::from_secs(0),

crates/pet/tests/ci_test.rs

@@ -208,6 +208,67 @@ fn verify_validity_of_interpreter_info(environment: PythonEnvironment) {
     }
 }
 
+#[cfg(unix)]
+#[cfg(target_os = "linux")]
+#[cfg_attr(feature = "ci", test)]
+#[allow(dead_code)]
+// On linux we /bin/python, /usr/bin/python and /usr/local/python are all separate environments.
+fn verify_bin_usr_bin_user_local_are_separate_python_envs() {
+    use pet::{find::find_and_report_envs, locators::create_locators};
+    use pet_conda::Conda;
+    use pet_core::os_environment::EnvironmentApi;
+    use pet_reporter::test;
+    use std::sync::Arc;
+
+    let reporter = test::create_reporter();
+    let environment = EnvironmentApi::new();
+    let conda_locator = Arc::new(Conda::from(&environment));
+
+    find_and_report_envs(
+        &reporter,
+        Default::default(),
+        &create_locators(conda_locator.clone()),
+        conda_locator,
+    );
+
+    let result = reporter.get_result();
+    let environments = result.environments;
+
+    // Python env /bin/python cannot have symlinks in /usr/bin or /usr/local
+    // Python env /usr/bin/python cannot have symlinks /bin or /usr/local
+    // Python env /usr/local/bin/python cannot have symlinks in /bin or /usr/bin
+    let bins = ["/bin", "/usr/bin", "/usr/local/bin"];
+    for bin in bins.iter() {
+        if let Some(bin_python) = environments.iter().find(|e| {
+            e.executable.clone().is_some()
+                && e.executable
+                    .clone()
+                    .unwrap()
+                    .parent()
+                    .unwrap()
+                    .starts_with(bin)
+        }) {
+            // If the exe is in /bin, then we can never have any symlinks to other folders such as /usr/bin or /usr/local
+            let other_bins = bins
+                .iter()
+                .filter(|b| *b != bin)
+                .map(|b| PathBuf::from(*b))
+                .collect::<Vec<PathBuf>>();
+            if let Some(symlinks) = &bin_python.symlinks {
+                for symlink in symlinks.iter() {
+                    let parent_of_symlink = symlink.parent().unwrap().to_path_buf();
+                    if other_bins.contains(&parent_of_symlink) {
+                        panic!(
+                            "Python environment {:?} cannot have a symlinks in {:?}",
+                            bin_python, other_bins
+                        );
+                    }
+                }
+            }
+        }
+    }
+}
+
 #[allow(dead_code)]
 fn get_conda_exe() -> &'static str {
     // On CI we expect conda to be in the current path.