Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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"
Comment thread
karthiknadig marked this conversation as resolved.
Outdated
123 changes: 119 additions & 4 deletions crates/pet-telemetry/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
Comment thread
karthiknadig marked this conversation as resolved.

let mut invalid_arch = env.arch.clone() != resolved.arch.clone();
if env.arch.clone().is_none() {
Expand Down Expand Up @@ -84,3 +95,107 @@ 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]);

// Should not warn — prefixes are identical
let result = report_inaccuracies_identified_after_resolving(&NoopReporter, &env, &resolved);
assert!(result.is_some());
}
Comment thread
karthiknadig marked this conversation as resolved.
Outdated

#[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]);

// Should NOT warn — both paths resolve to the same directory
let result = report_inaccuracies_identified_after_resolving(&NoopReporter, &env, &resolved);
assert!(result.is_some());
}

#[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]);

// Should warn — prefixes are genuinely different
// The function still returns Some(()), but the warn! macro fires internally.
let result = report_inaccuracies_identified_after_resolving(&NoopReporter, &env, &resolved);
assert!(result.is_some());
}

#[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_some());
}
}
Loading