Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions crates/pet-telemetry/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,6 @@ log = "0.4.21"
env_logger = "0.10.2"
lazy_static = "1.4.0"
regex = "1.10.4"

[dev-dependencies]
tempfile = "3.10"
131 changes: 125 additions & 6 deletions crates/pet-telemetry/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub fn report_inaccuracies_identified_after_resolving(
_reporter: &dyn Reporter,
env: &PythonEnvironment,
resolved: &PythonEnvironment,
) -> Option<()> {
) -> Option<InaccuratePythonEnvironmentInfo> {
let known_symlinks = env.symlinks.clone().unwrap_or_default();
let resolved_executable = &resolved.executable.clone()?;
let norm_cased_executable = norm_case(resolved_executable);
Expand All @@ -39,10 +39,21 @@ pub fn report_inaccuracies_identified_after_resolving(
executable_not_in_symlinks = false;
}

let mut invalid_prefix = env.prefix.clone().unwrap_or_default() != resolved.prefix.clone()?;
if env.prefix.clone().is_none() {
invalid_prefix = false;
}
let invalid_prefix = if let Some(ref env_prefix) = env.prefix {
let resolved_prefix = resolved.prefix.clone()?;
// Canonicalize both paths to handle symlinks — a venv prefix like
// /usr/local/venvs/myvenv may be a symlink to /usr/local/venvs/versioned/myvenv-1.0.51,
// and sys.prefix returns the resolved target. Without this, the comparison
// produces a false positive "Prefix is incorrect" warning. (See #358)
// Wrap in norm_case to handle Windows UNC prefix (`\\?\`) from canonicalize.
let env_canonical =
norm_case(std::fs::canonicalize(env_prefix).unwrap_or(env_prefix.clone()));
let resolved_canonical =
norm_case(std::fs::canonicalize(&resolved_prefix).unwrap_or(resolved_prefix));
env_canonical != resolved_canonical
} else {
false
};

let mut invalid_arch = env.arch.clone() != resolved.arch.clone();
if env.arch.clone().is_none() {
Expand Down Expand Up @@ -73,8 +84,9 @@ pub fn report_inaccuracies_identified_after_resolving(
env, resolved, event
);
// reporter.report_telemetry(TelemetryEvent::InaccuratePythonEnvironmentInfo(event));
return Some(event);
}
Option::Some(())
None
}

fn are_versions_different(actual: &str, expected: &str) -> Option<bool> {
Expand All @@ -84,3 +96,110 @@ fn are_versions_different(actual: &str, expected: &str) -> Option<bool> {
let expected = expected.get(1)?.as_str().to_string();
Some(actual != expected)
}

#[cfg(test)]
mod tests {
use super::*;
use pet_core::{
manager::EnvManager,
python_environment::{PythonEnvironmentBuilder, PythonEnvironmentKind},
telemetry::TelemetryEvent,
};
use std::path::PathBuf;

struct NoopReporter;
impl Reporter for NoopReporter {
fn report_manager(&self, _: &EnvManager) {}
fn report_environment(&self, _: &PythonEnvironment) {}
fn report_telemetry(&self, _: &TelemetryEvent) {}
}

fn make_env(
executable: PathBuf,
prefix: PathBuf,
version: &str,
symlinks: Vec<PathBuf>,
) -> PythonEnvironment {
PythonEnvironmentBuilder::new(Some(PythonEnvironmentKind::Venv))
.executable(Some(executable))
.prefix(Some(prefix))
.version(Some(version.to_string()))
.symlinks(Some(symlinks))
.build()
}

#[test]
fn same_prefix_is_not_flagged() {
let dir = tempfile::tempdir().unwrap();
let prefix = dir.path().to_path_buf();
let exe = prefix.join("bin").join("python");

let env = make_env(exe.clone(), prefix.clone(), "3.12.7", vec![exe.clone()]);
let resolved = make_env(exe.clone(), prefix, "3.12.7", vec![exe]);

let result = report_inaccuracies_identified_after_resolving(&NoopReporter, &env, &resolved);
assert!(result.is_none(), "identical prefixes should not be flagged");
}

#[cfg(unix)]
#[test]
fn symlinked_prefix_is_not_flagged() {
let dir = tempfile::tempdir().unwrap();
let real_prefix = dir.path().join("versioned").join("myvenv-1.0.51");
std::fs::create_dir_all(&real_prefix).unwrap();
let symlink_prefix = dir.path().join("myvenv");
std::os::unix::fs::symlink(&real_prefix, &symlink_prefix).unwrap();

let exe = symlink_prefix.join("bin").join("python");

// Discovery sees the symlink path as prefix
let env = make_env(exe.clone(), symlink_prefix, "3.12.7", vec![exe.clone()]);
// Resolution (spawning Python) returns the canonical path
let resolved = make_env(exe.clone(), real_prefix, "3.12.7", vec![exe]);

let result = report_inaccuracies_identified_after_resolving(&NoopReporter, &env, &resolved);
assert!(
result.is_none(),
"symlinked prefix to the same directory should not be flagged"
);
}

#[test]
fn different_prefix_is_flagged() {
let dir = tempfile::tempdir().unwrap();
let prefix_a = dir.path().join("env_a");
let prefix_b = dir.path().join("env_b");
std::fs::create_dir_all(&prefix_a).unwrap();
std::fs::create_dir_all(&prefix_b).unwrap();

let exe = prefix_a.join("bin").join("python");

let env = make_env(exe.clone(), prefix_a, "3.12.7", vec![exe.clone()]);
let resolved = make_env(exe.clone(), prefix_b, "3.12.7", vec![exe]);

let result = report_inaccuracies_identified_after_resolving(&NoopReporter, &env, &resolved);
let event = result.expect("genuinely different prefixes should be flagged");
assert_eq!(event.invalid_prefix, Some(true));
}

#[test]
fn none_prefix_is_not_flagged() {
let dir = tempfile::tempdir().unwrap();
let prefix = dir.path().to_path_buf();
let exe = prefix.join("bin").join("python");

// env has no prefix
let env = PythonEnvironmentBuilder::new(Some(PythonEnvironmentKind::Venv))
.executable(Some(exe.clone()))
.version(Some("3.12.7".to_string()))
.symlinks(Some(vec![exe.clone()]))
.build();
let resolved = make_env(exe.clone(), prefix, "3.12.7", vec![exe]);

let result = report_inaccuracies_identified_after_resolving(&NoopReporter, &env, &resolved);
assert!(
result.is_none(),
"None prefix should not cause any inaccuracy flag"
);
}
}
Loading