Tests to verify data extracted is accurate (#19)

Author: DonJayamanne

.github/workflows/pr-check.yml

@@ -77,6 +77,13 @@ jobs:
       - name: Check Conda version
         run: conda info --all
 
+      - name: Create Conda Environments
+        run: |
+          conda create -n test-env1 python=3.12 -y
+          conda create -n test-env-no-python -y
+          conda create -p ./prefix-envs/.conda1 python=3.12 -y
+          conda create -p ./prefix-envs/.conda-nopy -y
+
       - name: Install pipenv
         run: pip install pipenv
 

Cargo.lock

@@ -7,7 +7,10 @@ use environments::get_python_info;
 use pet_core::{
     os_environment::Environment, python_environment::PythonEnvironment, Locator, LocatorResult,
 };
-use pet_utils::{env::PythonEnv, executable::resolve_symlink};
+use pet_utils::{
+    env::PythonEnv,
+    executable::{find_executables, resolve_symlink},
+};
 use std::{collections::HashSet, path::PathBuf};
 
 mod env_variables;
@@ -84,24 +87,25 @@ impl Locator for Homebrew {
         let mut reported: HashSet<String> = HashSet::new();
         let mut environments: Vec<PythonEnvironment> = vec![];
         for homebrew_prefix_bin in get_homebrew_prefix_bin(&self.environment) {
-            for file in std::fs::read_dir(&homebrew_prefix_bin)
-                .ok()?
-                .filter_map(Result::ok)
-                .filter(|f| {
-                    let file_name = f.file_name().to_str().unwrap_or_default().to_lowercase();
-                    file_name.starts_with("python")
-                    // If this file name is `python3`, then ignore this for now.
+            for file in find_executables(&homebrew_prefix_bin).iter().filter(|f| {
+                let file_name = f
+                    .file_name()
+                    .unwrap_or_default()
+                    .to_str()
+                    .unwrap_or_default()
+                    .to_lowercase();
+                file_name.starts_with("python")
+                // If this file name is `python3`, then ignore this for now.
                     // We would prefer to use `python3.x` instead of `python3`.
                     // That way its more consistent and future proof
                         && file_name != "python3"
                         && file_name != "python"
-                })
-            {
+            }) {
                 // Sometimes we end up with other python installs in the Homebrew bin directory.
                 // E.g. /usr/local/bin is treated as a location where homebrew can be found (homebrew bin)
                 // However this is a very generic location, and we might end up with other python installs here.
                 // Hence call `resolve` to correctly identify homebrew python installs.
-                let env_to_resolve = PythonEnv::new(file.path(), None, None);
+                let env_to_resolve = PythonEnv::new(file.clone(), None, None);
                 if let Some(env) = resolve(&env_to_resolve, &mut reported) {
                     environments.push(env);
                 }

crates/pet-homebrew/src/lib.rs

@@ -9,7 +9,7 @@ use pet_core::{
     python_environment::{PythonEnvironment, PythonEnvironmentBuilder, PythonEnvironmentCategory},
     LocatorResult,
 };
-use pet_utils::{executable::find_executable, pyvenv_cfg::PyVenvCfg};
+use pet_utils::{executable::find_executable, headers::Headers, pyvenv_cfg::PyVenvCfg};
 use regex::Regex;
 use std::{fs, path::Path, sync::Arc};
 
@@ -72,6 +72,8 @@ pub fn get_pure_python_environment(
 ) -> Option<PythonEnvironment> {
     let file_name = path.file_name()?.to_string_lossy().to_string();
     let version = get_version(&file_name)?;
+    // If we can get the version from the header files, thats more accurate.
+    let version = Headers::get_version(path).unwrap_or(version.clone());
 
     let arch = if file_name.ends_with("-win32") {
         Some(Architecture::X86)

crates/pet-pyenv/src/environments.rs

@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+use log::error;
 use pet_core::{
     arch::Architecture,
     python_environment::{PythonEnvironment, PythonEnvironmentCategory},
@@ -116,3 +117,17 @@ impl Environment {
         }
     }
 }
+
+pub fn get_environment_key(env: &PythonEnvironment) -> Option<&PathBuf> {
+    if let Some(exe) = &env.executable {
+        Some(exe)
+    } else if let Some(prefix) = &env.prefix {
+        Some(prefix)
+    } else {
+        error!(
+            "Failed to report environment due to lack of exe & prefix: {:?}",
+            env
+        );
+        None
+    }
+}

crates/pet-reporter/src/environment.rs

@@ -1,9 +1,12 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-use crate::{environment::Environment, manager::Manager};
+use crate::{
+    environment::{get_environment_key, Environment},
+    manager::Manager,
+};
 use env_logger::Builder;
-use log::{error, LevelFilter};
+use log::LevelFilter;
 use pet_core::{manager::EnvManager, python_environment::PythonEnvironment, reporter::Reporter};
 use pet_jsonrpc::send_message;
 use serde::{Deserialize, Serialize};
@@ -45,20 +48,6 @@ pub fn create_reporter() -> impl Reporter {
     }
 }
 
-fn get_environment_key(env: &PythonEnvironment) -> Option<&PathBuf> {
-    if let Some(exe) = &env.executable {
-        Some(exe)
-    } else if let Some(prefix) = &env.prefix {
-        Some(prefix)
-    } else {
-        error!(
-            "Failed to report environment due to lack of exe & prefix: {:?}",
-            env
-        );
-        None
-    }
-}
-
 #[derive(Serialize, Deserialize, PartialEq, Debug, Eq, Clone)]
 pub enum LogLevel {
     #[serde(rename = "debug")]

crates/pet-reporter/src/jsonrpc.rs

@@ -5,3 +5,4 @@ mod environment;
 pub mod jsonrpc;
 mod manager;
 pub mod stdio;
+pub mod test;

crates/pet-reporter/src/lib.rs

@@ -1,9 +1,12 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-use crate::{environment::Environment, manager::Manager};
+use crate::{
+    environment::{get_environment_key, Environment},
+    manager::Manager,
+};
 use env_logger::Builder;
-use log::{error, LevelFilter};
+use log::LevelFilter;
 use pet_core::{manager::EnvManager, python_environment::PythonEnvironment, reporter::Reporter};
 use serde::{Deserialize, Serialize};
 use std::{
@@ -46,20 +49,6 @@ pub fn create_reporter() -> impl Reporter {
     }
 }
 
-fn get_environment_key(env: &PythonEnvironment) -> Option<&PathBuf> {
-    if let Some(exe) = &env.executable {
-        Some(exe)
-    } else if let Some(prefix) = &env.prefix {
-        Some(prefix)
-    } else {
-        error!(
-            "Failed to report environment due to lack of exe & prefix: {:?}",
-            env
-        );
-        None
-    }
-}
-
 #[derive(Serialize, Deserialize, PartialEq, Debug, Eq, Clone)]
 pub enum LogLevel {
     #[serde(rename = "debug")]

crates/pet-reporter/src/stdio.rs

@@ -0,0 +1,40 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+use crate::environment::get_environment_key;
+use pet_core::{manager::EnvManager, python_environment::PythonEnvironment, reporter::Reporter};
+use std::collections::HashMap;
+use std::{
+    path::PathBuf,
+    sync::{Arc, Mutex},
+};
+
+pub struct TestReporter {
+    pub reported_managers: Arc<Mutex<HashMap<PathBuf, EnvManager>>>,
+    pub reported_environments: Arc<Mutex<HashMap<PathBuf, PythonEnvironment>>>,
+}
+
+impl Reporter for TestReporter {
+    fn report_manager(&self, manager: &EnvManager) {
+        let mut reported_managers = self.reported_managers.lock().unwrap();
+        if !reported_managers.contains_key(&manager.executable) {
+            reported_managers.insert(manager.executable.clone(), manager.clone());
+        }
+    }
+
+    fn report_environment(&self, env: &PythonEnvironment) {
+        if let Some(key) = get_environment_key(env) {
+            let mut reported_environments = self.reported_environments.lock().unwrap();
+            if !reported_environments.contains_key(key) {
+                reported_environments.insert(key.clone(), env.clone());
+            }
+        }
+    }
+}
+
+pub fn create_reporter() -> TestReporter {
+    TestReporter {
+        reported_managers: Arc::new(Mutex::new(HashMap::new())),
+        reported_environments: Arc::new(Mutex::new(HashMap::new())),
+    }
+}

crates/pet-reporter/src/test.rs

@@ -103,22 +103,24 @@ fn is_python_executable_name(exe: &Path) -> bool {
 // If the real file == exe, then it is not a symlink.
 pub fn resolve_symlink(exe: &Path) -> Option<PathBuf> {
     let name = exe.file_name()?.to_string_lossy();
-    // TODO: What is -config and -build?
+    // In bin directory of homebrew, we have files like python-build, python-config, python3-config
     if !name.starts_with("python") || name.ends_with("-config") || name.ends_with("-build") {
         return None;
     }
-
-    // If the file == symlink, then it is not a symlink.
-    // We already have the resolved file, no need to return that again.
-    if let Ok(real_file) = fs::read_link(exe) {
-        if real_file == exe {
-            None
-        } else {
-            Some(real_file)
+    if let Ok(metadata) = std::fs::symlink_metadata(exe) {
+        if metadata.is_file() || !metadata.file_type().is_symlink() {
+            return Some(exe.to_path_buf());
         }
-    } else {
-        None
+        if let Ok(readlink) = std::fs::canonicalize(exe) {
+            if readlink == exe {
+                return None;
+            } else {
+                return Some(readlink);
+            }
+        }
+        return Some(exe.to_path_buf());
     }
+    Some(exe.to_path_buf())
 }
 
 // Given a list of executables, return the one with the shortest path.

crates/pet-utils/src/executable.rs

@@ -6,7 +6,7 @@ use regex::Regex;
 use std::{fs, path::Path};
 
 lazy_static! {
-    static ref VERSION: Regex = Regex::new(r#"#define\s+PY_VERSION\s+"((\d+\.?)*)"#)
+    static ref VERSION: Regex = Regex::new(r#"#define\s+PY_VERSION\s+"((\d+\.?)*.*)\""#)
         .expect("error parsing Version regex for partchlevel.h");
 }
 
@@ -33,9 +33,31 @@ pub fn get_version(path: &Path) -> Option<String> {
     if path.ends_with(bin) {
         path.pop();
     }
-    let headers_path = if cfg!(windows) { "Headers" } else { "include" };
-    let patchlevel_h = path.join(headers_path).join("patchlevel.h");
-    let contents = fs::read_to_string(patchlevel_h).ok()?;
+    let headers_path = path.join(if cfg!(windows) { "Headers" } else { "include" });
+    let patchlevel_h = headers_path.join("patchlevel.h");
+    let mut contents = "".to_string();
+    if let Ok(result) = fs::read_to_string(patchlevel_h) {
+        contents = result;
+    } else if fs::metadata(&headers_path).is_err() {
+        // Such a path does not exist, get out.
+        return None;
+    } else {
+        // Try the other path
+        // Sometimes we have it in a sub directory such as `python3.10`
+        if let Ok(readdir) = fs::read_dir(&headers_path) {
+            for path in readdir.filter_map(Result::ok).map(|e| e.path()) {
+                if let Ok(metadata) = fs::metadata(&path) {
+                    if metadata.is_dir() {
+                        let patchlevel_h = path.join("patchlevel.h");
+                        if let Ok(result) = fs::read_to_string(patchlevel_h) {
+                            contents = result;
+                            break;
+                        }
+                    }
+                }
+            }
+        }
+    }
     for line in contents.lines() {
         if let Some(captures) = VERSION.captures(line) {
             if let Some(value) = captures.get(1) {

crates/pet-utils/src/headers.rs

@@ -8,6 +8,11 @@ use std::{
 
 // Similar to fs::canonicalize, but ignores UNC paths and returns the path as is (for windows).
 pub fn normalize<P: AsRef<Path>>(path: P) -> PathBuf {
+    // On unix do not use canonicalize, results in weird issues with homebrew paths
+    if cfg!(unix) {
+        return path.as_ref().to_path_buf();
+    }
+
     if let Ok(resolved) = fs::canonicalize(&path) {
         if cfg!(unix) {
             return resolved;

crates/pet-utils/src/path.rs

@@ -74,4 +74,12 @@ fn version_from_header_files() {
     let path: PathBuf = resolve_test_path(&["unix", "headers", "python3.9.9", "bin"]).into();
     let version = SysPrefix::get_version(&path).unwrap();
     assert_eq!(version, "3.9.9");
+
+    let path: PathBuf = resolve_test_path(&["unix", "headers", "python3.10-dev", "bin"]).into();
+    let version = SysPrefix::get_version(&path).unwrap();
+    assert_eq!(version, "3.10.14+");
+
+    let path: PathBuf = resolve_test_path(&["unix", "headers", "python3.13", "bin"]).into();
+    let version = SysPrefix::get_version(&path).unwrap();
+    assert_eq!(version, "3.13.0a5");
 }

crates/pet-utils/tests/sys_prefix_test.rs

@@ -0,0 +1,35 @@
+
+/* Python version identification scheme.
+
+   When the major or minor version changes, the VERSION variable in
+   configure.ac must also be changed.
+
+   There is also (independent) API version information in modsupport.h.
+*/
+
+/* Values for PY_RELEASE_LEVEL */
+#define PY_RELEASE_LEVEL_ALPHA  0xA
+#define PY_RELEASE_LEVEL_BETA   0xB
+#define PY_RELEASE_LEVEL_GAMMA  0xC     /* For release candidates */
+#define PY_RELEASE_LEVEL_FINAL  0xF     /* Serial should be 0 here */
+                                        /* Higher for patch releases */
+
+/* Version parsed out into numeric values */
+/*--start constants--*/
+#define PY_MAJOR_VERSION        3
+#define PY_MINOR_VERSION        10
+#define PY_MICRO_VERSION        14
+#define PY_RELEASE_LEVEL        PY_RELEASE_LEVEL_FINAL
+#define PY_RELEASE_SERIAL       0
+
+/* Version as a string */
+#define PY_VERSION              "3.10.14+"
+/*--end constants--*/
+
+/* Version as a single 4-byte hex number, e.g. 0x010502B2 == 1.5.2b2.
+   Use this for numeric comparisons, e.g. #if PY_VERSION_HEX >= ... */
+#define PY_VERSION_HEX ((PY_MAJOR_VERSION << 24) | \
+                        (PY_MINOR_VERSION << 16) | \
+                        (PY_MICRO_VERSION <<  8) | \
+                        (PY_RELEASE_LEVEL <<  4) | \
+                        (PY_RELEASE_SERIAL << 0))