Skip to content

Commit b797630

Browse files
committed
fixes
1 parent 895c3f8 commit b797630

File tree

5 files changed

+171
-23
lines changed

5 files changed

+171
-23
lines changed

WIP.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# Tests
2+
* Vertualenv wrapper
3+
* Venvs
4+
- multiple venvs
5+
- create venv and verify version
6+
- change the env that ws used to create the venv
7+
(e.g. create venv A, then creaet venv B from A, then re-create A using another Python version)
8+
Verify we can still get the version of venv B without spawning Python.
9+
TODO: How to verify we did not spawn Python?
10+
- Verify we can get the version of venv without spawning Python in windows either
11+
* Venv and Conda
12+
- Conda should not be treated as venv
13+
- Create a venv and verify it is identified correctly
14+
- Create a conda with the exact same name and verify it is identified as a conda and not venv
15+
* Conda
16+
* Test each tool with PATH updated to include the env in the current PATH
17+
* Test by creating a symlink manually and storing it in PATH (or is this the same as previous test?)
18+
19+
20+
/// Verification 4 & 5:
21+
/// Similarly for each environment use resolve method and verify we get the exact same information.
22+
23+
24+
Ensure we create and initialize EnvironmentApi only once in one place and share that all over.

crates/pet/src/resolve.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@
33

44
use std::{path::PathBuf, sync::Arc};
55

6-
use log::warn;
6+
use log::{trace, warn};
77
use pet_core::{
8-
arch::Architecture, os_environment::EnvironmentApi, python_environment::PythonEnvironment,
8+
arch::Architecture,
9+
os_environment::EnvironmentApi,
10+
python_environment::{PythonEnvironment, PythonEnvironmentBuilder},
911
Locator,
1012
};
1113
use pet_env_var_path::get_search_paths_from_env_variables;
@@ -35,12 +37,21 @@ pub fn resolve_environment(
3537
// Now try to resolve this fully, by spawning python.
3638
if let Some(ref executable) = env.executable {
3739
if let Some(info) = ResolvedPythonEnv::from(executable) {
40+
trace!(
41+
"In resolve_environment, Resolved Python Exe {:?} as {:?}",
42+
executable,
43+
info
44+
);
3845
let discovered = env.clone();
3946
let mut resolved = env.clone();
4047
let mut symlinks = resolved.symlinks.clone().unwrap_or_default();
4148

4249
symlinks.push(info.executable.clone());
4350
symlinks.append(&mut info.symlink.clone().unwrap_or_default());
51+
symlinks.sort();
52+
symlinks.dedup();
53+
54+
resolved.symlinks = Some(symlinks);
4455
resolved.version = Some(info.version);
4556
resolved.prefix = Some(info.prefix);
4657
resolved.arch = Some(if info.is64_bit {

crates/pet/tests/ci_jupyter_container.rs

Lines changed: 54 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,21 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4+
use std::sync::Once;
5+
46
mod common;
57

8+
static INIT: Once = Once::new();
9+
10+
/// Setup function that is only run once, even if called multiple times.
11+
fn setup() {
12+
INIT.call_once(|| {
13+
env_logger::builder()
14+
.filter(None, log::LevelFilter::Trace)
15+
.init();
16+
});
17+
}
18+
619
#[cfg(unix)]
720
#[cfg_attr(feature = "ci-jupyter-container", test)]
821
#[allow(dead_code)]
@@ -19,6 +32,8 @@ fn verify_python_in_jupyter_contaner() {
1932
use pet_reporter::test;
2033
use std::{path::PathBuf, sync::Arc};
2134

35+
setup();
36+
2237
let reporter = test::create_reporter();
2338
let environment = EnvironmentApi::new();
2439
let conda_locator = Arc::new(Conda::from(&environment));
@@ -121,23 +136,53 @@ fn verify_python_in_jupyter_contaner() {
121136
.iter()
122137
.find(|e| e.executable == env.executable)
123138
.expect(format!("Expected to find python environment {:?}", env.executable).as_str());
124-
assert_eq!(python_env.executable, env.executable);
125-
assert_eq!(python_env.category, env.category);
126-
assert_eq!(python_env.symlinks, env.symlinks);
127-
assert_eq!(python_env.manager, env.manager);
128-
assert_eq!(python_env.name, env.name);
129-
assert_eq!(python_env.version, env.version);
130-
assert_eq!(python_env.arch, env.arch);
139+
assert_eq!(
140+
python_env.executable, env.executable,
141+
"Expected exe to be same when comparing {:?} and {:?}",
142+
python_env, env
143+
);
144+
assert_eq!(
145+
python_env.category, env.category,
146+
"Expected category to be same when comparing {:?} and {:?}",
147+
python_env, env
148+
);
149+
assert_eq!(
150+
python_env.symlinks, env.symlinks,
151+
"Expected symlinks to be same when comparing {:?} and {:?}",
152+
python_env, env
153+
);
154+
assert_eq!(
155+
python_env.manager, env.manager,
156+
"Expected manager to be same when comparing {:?} and {:?}",
157+
python_env, env
158+
);
159+
assert_eq!(
160+
python_env.name, env.name,
161+
"Expected name to be same when comparing {:?} and {:?}",
162+
python_env, env
163+
);
164+
assert_eq!(
165+
python_env.version, env.version,
166+
"Expected version to be same when comparing {:?} and {:?}",
167+
python_env, env
168+
);
169+
assert_eq!(
170+
python_env.arch, env.arch,
171+
"Expected arch to be same when comparing {:?} and {:?}",
172+
python_env, env
173+
);
131174

132175
// known issue https://github.com/microsoft/python-environment-tools/issues/64
133176
if env.executable == Some(PathBuf::from("/home/codespace/.python/current/bin/python")) {
134177
assert!(
135178
python_env.prefix == Some(PathBuf::from("/home/codespace/.python/current"))
136179
|| python_env.prefix == Some(PathBuf::from("/usr/local/python/3.10.13")),
137-
"Expected {:?} to be {:?} or {:?}",
180+
"Expected {:?} to be {:?} or {:?} when comparing {:?} and {:?}",
138181
python_env.prefix,
139182
"/home/codespace/.python/current",
140-
"/usr/local/python/3.10.13"
183+
"/usr/local/python/3.10.13",
184+
python_env,
185+
env
141186
);
142187
}
143188
}

crates/pet/tests/ci_test.rs

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -96,22 +96,19 @@ fn verify_validity_of_discovered_envs() {
9696
threads.push(thread::spawn(move || {
9797
verify_validity_of_interpreter_info(e);
9898
}));
99-
// Verification 2:
100-
// For each enviornment, given the executable verify we can get the exact same information
101-
// Using the `locator.try_from` method (without having to find all environments).
102-
// I.e. we should be able to get the same information using only the executable.
103-
//
104-
// Verification 3:
105-
// Similarly for each environment use one of the known symlinks and verify we can get the same information.
106-
//
107-
// TODO: Verification 4 & 5:
108-
// Similarly for each environment use resolve method and verify we get the exact same information.
10999
let e = environment.clone();
110100
threads.push(thread::spawn(move || {
111101
for exe in &e.clone().symlinks.unwrap_or_default() {
112-
// Verification 2 & 3.
102+
// Verification 2:
103+
// For each enviornment, given the executable verify we can get the exact same information
104+
// Using the `locator.try_from` method (without having to find all environments).
105+
// I.e. we should be able to get the same information using only the executable.
106+
//
107+
// Verification 3:
108+
// Similarly for each environment use one of the known symlinks and verify we can get the same information.
113109
verify_we_can_get_same_env_info_using_from_with_exe(exe, environment.clone());
114-
// Verification 4 & 5.
110+
// Verification 4 & 5:
111+
// Similarly for each environment use resolve method and verify we get the exact same information.
115112
// verify_we_can_get_same_env_info_using_resolve_with_exe(exe, environment.clone());
116113
}
117114
}));

todo.md

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
# Support for
2+
3+
- PIPENV_MAX_DEPTH
4+
5+
# VirutalenvWrapper
6+
7+
- `find` is implemented
8+
Why? Ensure this same logic is implemented in the global lcoators
9+
Besides this logic might be wrong, as we might be marking `pipenv` as a `virtualenvwrapper` as a aresult of this logic.
10+
11+
12+
# Priority
13+
14+
- Need to ship => List envs and compare against Python extension (compare exe & type) & send telemetry
15+
- Resolve
16+
- Cache
17+
18+
* List all exes in the following bin dir
19+
/home/linuxbrew/.linuxbrew/Cellar/[email protected]/3.12.4/bin/python3.12
20+
21+
# Pyenv manager not reported if there are no pyenv environments.
22+
23+
# Sometimes users have .venv files and `python` folders,
24+
25+
Ensure we take this into account.
26+
27+
# Ensure we handle searching folders with 10000s of files
28+
29+
# Mac test
30+
31+
- Write tests for CI, to ensure there are no mac global python where the executable does not point to anytihng in `framework.python...`
32+
The executable must be something in `/usr/local/bin/python?`
33+
- Only one of the Mac-global python shoudl have two symlinks pointing to `/Library/Frameworks/Python.framework/Versions/Current/bin/python3?` exes
34+
35+
# Optimizations
36+
37+
- Avoid using `to_path` when using `read_dir`, where possible filter early
38+
- Avoid reading metadata, try using `PathBuf.is_file...` etc where possible.
39+
- Avoid checking if a file/dir exists, instead just open/read_dir and fail (thats one less I/O operation)
40+
41+
# Add `isWorkspaceLocator` to the Locator trait
42+
43+
This way we can create the locators in one place and re-use the list.
44+
45+
# Add CLI flag to wait for `conda_locator.find_with_conda_executable(conda_executable);` to complete so we can see if we missed anything
46+
47+
# Add CLI flag to print just summary, not everything, useful for debugging and see counts
48+
49+
# Add CLI flag to dump everything into a JSON file, useful for debugging and diagnostics
50+
51+
# Add CLI flag to resolve everything
52+
53+
# We need to handle the following scenarios
54+
55+
- Install commandline tools in mac
56+
/usr/bin/python3 points to that python env (/Library/Developer/CommandLineTools/usr/bin/python3)
57+
58+
- We cache this
59+
We have an env with /usr/bin/python3 and symlinks as /Library/Developer/CommandLineTools/usr/bin/python3
60+
61+
- Install X Code
62+
/usr/bin/python3 now points to another python env.
63+
/Applications/Xcode.app/Contents/Developer/usr/bin/python3
64+
65+
- Now when reading the cache, the executable `/usr/bin/python3`
66+
Does not point to the same place.
67+
Fortunately it doesn't work, as none of the locators directly support `/usr/bin/python3`
68+
We should
69+
- Ensure the ctime and mtime of the main exe is captured
70+
- Then when validating, check if that has changed, if not then continue with the validation.
71+
- If it has changed, then skip validation, and let the usual locator code handle it.

0 commit comments

Comments
 (0)