Skip to content

Commit 2299859

Browse files
committed
polish and address comments
1 parent 65b04e6 commit 2299859

File tree

11 files changed

+132
-126
lines changed

11 files changed

+132
-126
lines changed

docs/startup-flow.md

Lines changed: 61 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,9 @@ python environments extension begins activation
2020
1. spawn PET process (`createNativePythonFinder`)
2121
1. sets up a JSON-RPC connection to it over stdin/stdout
2222
2. register all built-in managers in parallel (Promise.all):
23-
- system: create SysPythonManager + VenvManager + PipPackageManager, register immediately
24-
- ✅ NO PET call — managers are created and registered with no tool detection
25-
- sets up file watcher for venv activation scripts
23+
- system: create SysPythonManager + VenvManager + PipPackageManager, register immediately (✅ NO PET call, sets up file watcher)
2624
- conda: `getConda(nativeFinder)` checks settings → cache → persistent state → PATH
27-
- if found → register CondaEnvManager + CondaPackageManager
28-
- if not found → PET fallback as last resort (rarely hit, conda is usually on PATH)
29-
- if not found at all → skip, send MANAGER_REGISTRATION.SKIPPED telemetry
30-
- pyenv: create PyEnvManager, register immediately
31-
- ✅ NO PET call — always registers unconditionally (lazy discovery)
32-
- pipenv: create PipenvManager, register immediately
33-
- ✅ NO PET call — always registers unconditionally (lazy discovery)
34-
- poetry: create PoetryManager + PoetryPackageManager, register immediately
25+
- pyenv & pipenv & poetry: create PyEnvManager, register immediately
3526
- ✅ NO PET call — always registers unconditionally (lazy discovery)
3627
- shellStartupVars: initialize
3728
- all managers fire `onDidChangeEnvironmentManager` → ManagerReady resolves
@@ -40,99 +31,65 @@ python environments extension begins activation
4031

4132
**--- gate point: `applyInitialEnvironmentSelection` ---**
4233

43-
📊 TELEMETRY: ENV_SELECTION.STARTED { duration (activation→here), registeredManagerCount, registeredManagerIds, workspaceFolderCount }
44-
45-
1. for each workspace folder + global scope (no workspace case), run `resolvePriorityChainCore` to find manager:
46-
- P1: pythonProjects[] setting → specific manager for this project
47-
- P2: user-configured defaultEnvManager setting
48-
- P3: user-configured python.defaultInterpreterPath → nativeFinder.resolve(path)
49-
- P4: auto-discovery → try venv manager, fall back to system python
50-
- for workspace scope: call `venvManager.get(scope)`
51-
- if venv found (local .venv/venv) → use venv manager with that env
52-
- if no local venv → venv manager may still return its `globalEnv` (system Python)
53-
- if venvManager.get returns undefined → fall back to system python manager
54-
- for global scope: use system python manager directly
55-
56-
2. get the environment from the winning priority level:
57-
58-
--- fork point: `result.environment ?? await result.manager.get(folder.uri)` ---
59-
left side truthy = envPreResolved | left side undefined = managerDiscovery
60-
61-
envPreResolved — P3 won (interpreter → manager):
62-
`resolvePriorityChainCore` calls `tryResolveInterpreterPath()`:
63-
1. `nativeFinder.resolve(path)` — single PET call, resolves just this one binary
64-
2. find which manager owns the resolved env (by managerId)
65-
3. return { manager, environment } — BOTH are known
66-
→ result.environment is set → the `??` short-circuits
67-
→ no `manager.get()` called, no `initialize()`, no full discovery
68-
69-
managerDiscovery — P1, P2, or P4 won (manager → interpreter):
70-
`resolvePriorityChainCore` returns { manager, environment: undefined }
71-
→ falls through to `await result.manager.get(scope)`
72-
73-
**--- inner fork: fast path vs slow path (tryFastPathGet in fastPath.ts) ---**
74-
Conditions checked before entering fast path:
75-
a. `_initialized` deferred is undefined (never created) OR has not yet completed
76-
b. scope is a `Uri` (not global/undefined)
77-
78-
FAST PATH (background init kickoff + optional early return):
79-
**Race-condition safety (runs before any await):**
80-
1. if `_initialized` doesn't exist yet:
81-
- create deferred and **register immediately** via `setInitialized()` callback
82-
- this blocks concurrent callers from spawning duplicate background inits
83-
- kick off `startBackgroundInit()` as fire-and-forget
84-
- this happens as soon as (a) and (b) are true, **even if** no persisted path exists
85-
2. get project fsPath: `getProjectFsPathForScope(api, scope)`
86-
- prefers resolved project path if available, falls back to scope.fsPath
87-
- shared across all managers to avoid lambda duplication
88-
3. read persisted path (only if scope is a `Uri`; may return undefined)
89-
4. if a persisted path exists:
90-
- attempt `resolve(persistedPath)`
91-
- failure (no env, mismatched manager, etc.) → fall through to SLOW PATH
92-
- success → return env immediately (background init continues in parallel)
93-
**Failure recovery (in startBackgroundInit error handler):**
94-
- if background init throws: `setInitialized(undefined)` — clear deferred so next `get()` call retries init
95-
96-
SLOW PATH — fast path conditions not met, or fast path failed:
97-
4. `initialize()` — lazy, once-only per manager (guarded by `_initialized` deferred)
98-
**Once-only guarantee:**
99-
- first caller creates `_initialized` deferred (if not already created by fast path)
100-
- concurrent callers see the existing deferred and await it instead of re-running init
101-
- deferred is **not cleared on failure** here (unlike in fast-path background handler)
102-
so only one init attempt runs, but subsequent calls still await the same failed init
103-
**Note:** In the fast path, if background init fails, the deferred is cleared to allow retry
104-
a. `nativeFinder.refresh(hardRefresh=false)`:
105-
→ internally calls `handleSoftRefresh()` → computes cache key from options
106-
- on reload: cache is empty (Map was destroyed) → cache miss
107-
- falls through to `handleHardRefresh()`
108-
→ `handleHardRefresh()` adds request to WorkerPool queue (concurrency 1):
109-
1. run `configure()` to setup PET search paths
110-
2. run `refresh` — PET scans filesystem
111-
- PET may use its own on-disk cache
112-
3. returns NativeInfo[] (all envs of all types)
113-
- result stored in in-memory cache so subsequent managers get instant cache hit
114-
b. filter results to this manager's env type (e.g. conda filters to kind=conda)
115-
c. convert NativeEnvInfo → PythonEnvironment objects → populate collection
116-
d. `loadEnvMap()` — reads persisted env path from workspace state
117-
→ matches path against PET discovery results
118-
→ populates `fsPathToEnv` map
119-
5. look up scope in `fsPathToEnv` → return the matched env
120-
121-
📊 TELEMETRY: ENV_SELECTION.RESULT (per scope) { duration (priority chain + manager.get), scope, prioritySource, managerId, path, hasPersistedSelection }
122-
123-
3. env is cached in memory (no settings.json write)
124-
4. Python extension / status bar can now get the selected env via `api.getEnvironment(scope)`
125-
126-
📊 TELEMETRY: EXTENSION.MANAGER_REGISTRATION_DURATION { duration (activation→here), result, failureStage?, errorType? }
127-
128-
SIDEBAR ACCESS (on-demand, if user opens Python environments panel):
129-
- view iterates `providers.managers` → all registered managers appear (including pyenv/pipenv/poetry)
130-
- user expands a manager node → `getChildren()``manager.getEnvironments('all')`
131-
`initialize()` (lazy, once-only) → `nativeFinder.refresh(false)`:
132-
- if cache populated from earlier env selection → instant cache hit
133-
- if first access → warm PET call (no concurrent pressure, single caller)
134-
→ environments appear under the manager node
135-
→ if no environments found → "No environments" placeholder shown
34+
📊 TELEMETRY: ENV_SELECTION.STARTED { duration, registeredManagerCount, registeredManagerIds, workspaceFolderCount }
35+
36+
**Step 1 — pick a manager** (`resolvePriorityChainCore`, per workspace folder + global):
37+
38+
| Priority | Source | Returns |
39+
|----------|--------|---------|
40+
| P1 | `pythonProjects[]` setting | manager only |
41+
| P2 | `defaultEnvManager` setting | manager only |
42+
| P3 | `python.defaultInterpreterPath``nativeFinder.resolve(path)` | manager **+ environment** |
43+
| P4 | auto-discovery: venv → system python fallback | manager only |
44+
45+
**Step 2 — get the environment** (`result.environment ?? await result.manager.get(scope)`):
46+
47+
- **If P3 won:** environment is already resolved → done, no `get()` call needed.
48+
- **Otherwise:** calls `manager.get(scope)`, which has two internal paths:
49+
50+
**Fast path** (`tryFastPathGet` in `fastPath.ts`) — entered when `_initialized` hasn't completed and scope is a `Uri`:
51+
1. Synchronously create `_initialized` deferred + kick off `startBackgroundInit()` (fire-and-forget full PET discovery)
52+
2. Read persisted env path from workspace state
53+
3. If persisted path exists → `resolve(path)` → return immediately (background init continues in parallel)
54+
4. If no persisted path or resolve fails → fall through to slow path
55+
- *On background init failure:* clears `_initialized` so next `get()` retries
56+
57+
**Slow path** — fast path skipped or failed:
58+
1. `initialize()` — lazy, once-only (guarded by `_initialized` deferred, concurrent callers await it)
59+
- `nativeFinder.refresh(false)` → PET scan (cached across managers after first call)
60+
- Filter results to this manager's type → populate `collection`
61+
- `loadEnvMap()` → match persisted paths against discovered envs
62+
2. Look up scope in `fsPathToEnv` → return matched env
63+
64+
📊 TELEMETRY: ENV_SELECTION.RESULT (per scope) { duration, scope, prioritySource, managerId, path, hasPersistedSelection }
65+
66+
**Step 3 — done:**
67+
- env cached in memory (no settings.json write)
68+
- available via `api.getEnvironment(scope)`
69+
70+
📊 TELEMETRY: EXTENSION.MANAGER_REGISTRATION_DURATION { duration, result, failureStage?, errorType? }
71+
72+
---
73+
74+
### Other entry points to `initialize()`
75+
76+
All three trigger `initialize()` lazily (once-only, guarded by `_initialized` deferred). After the first call completes, subsequent calls are no-ops.
77+
78+
**`manager.get(scope)`** — environment selection (Step 2 above):
79+
- Called during `applyInitialEnvironmentSelection` or when settings change triggers re-selection
80+
- Fast path may resolve immediately; slow path awaits `initialize()`
81+
82+
**`manager.getEnvironments(scope)`** — sidebar / listing:
83+
- Called when user expands a manager node in the Python environments panel
84+
- Also called by any API consumer requesting the full environment list
85+
- If PET cache populated from earlier `get()` → instant hit; otherwise warm PET call
86+
87+
**`manager.resolve(context)`** — path resolution:
88+
- Called when resolving a specific Python binary path to check if it belongs to this manager
89+
- Used by `tryResolveInterpreterPath()` in the priority chain (P3) and by external API consumers
90+
- Awaits `initialize()`, then delegates to manager-specific resolve (e.g., `resolvePipenvPath`)
91+
92+
---
13693

13794
POST-INIT:
13895
1. register terminal package watcher

src/common/telemetry/constants.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ export enum EventNames {
109109
* - managerName: string (e.g. 'pipenv', 'poetry', 'pyenv')
110110
* - result: 'success' | 'tool_not_found' | 'error'
111111
* - envCount: number (environments discovered)
112-
* - toolSource: string (how the CLI was found: 'settings', 'cache', 'persistentState', 'path', 'pet', 'none')
112+
* - toolSource: string (how the CLI was found: 'settings', 'local', 'pet', 'none')
113113
* - errorType: string (classified error category, on failure only)
114114
*/
115115
MANAGER_LAZY_INIT = 'MANAGER.LAZY_INIT',

src/extension.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -601,9 +601,9 @@ export async function activate(context: ExtensionContext): Promise<PythonEnviron
601601
'conda',
602602
registerCondaFeatures(nativeFinder, context.subscriptions, outputChannel, projectManager),
603603
),
604-
safeRegister('pyenv', registerPyenvFeatures(nativeFinder, context.subscriptions)),
605-
safeRegister('pipenv', registerPipenvFeatures(nativeFinder, context.subscriptions)),
606-
safeRegister('poetry', registerPoetryFeatures(nativeFinder, context.subscriptions, outputChannel)),
604+
safeRegister('pyenv', registerPyenvFeatures(nativeFinder, context.subscriptions, projectManager)),
605+
safeRegister('pipenv', registerPipenvFeatures(nativeFinder, context.subscriptions, projectManager)),
606+
safeRegister('poetry', registerPoetryFeatures(nativeFinder, context.subscriptions, outputChannel, projectManager)),
607607
safeRegister('shellStartupVars', shellStartupVarsMgr.initialize()),
608608
]);
609609

src/managers/common/fastPath.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
import { Uri } from 'vscode';
55
import { GetEnvironmentScope, PythonEnvironment, PythonEnvironmentApi } from '../../api';
6-
import { traceError, traceWarn } from '../../common/logging';
6+
import { traceError, traceVerbose, traceWarn } from '../../common/logging';
77
import { createDeferred, Deferred } from '../../common/utils/deferred';
88

99
/**
@@ -62,7 +62,6 @@ export async function tryFastPathGet(opts: FastPathOptions): Promise<FastPathRes
6262

6363
let deferred = opts.initialized;
6464
if (!deferred) {
65-
// Register deferred before any await to avoid concurrent callers starting duplicate inits.
6665
deferred = createDeferred<void>();
6766
opts.setInitialized(deferred);
6867
const deferredRef = deferred;
@@ -93,8 +92,13 @@ export async function tryFastPathGet(opts: FastPathOptions): Promise<FastPathRes
9392
return { env: resolved };
9493
}
9594
} catch (err) {
96-
traceWarn(`[${opts.label}] Fast path resolve failed for '${persistedPath}', falling back to full init:`, err);
95+
traceWarn(
96+
`[${opts.label}] Fast path resolve failed for '${persistedPath}', falling back to full init:`,
97+
err,
98+
);
9799
}
100+
} else {
101+
traceVerbose(`[${opts.label}] Fast path: no persisted path, falling through to slow path`);
98102
}
99103

100104
return undefined;

src/managers/pipenv/main.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,18 @@ import { Disposable } from 'vscode';
22
import { PythonEnvironmentApi } from '../../api';
33
import { traceInfo } from '../../common/logging';
44
import { getPythonApi } from '../../features/pythonApi';
5+
import { PythonProjectManager } from '../../internal.api';
56
import { NativePythonFinder } from '../common/nativePythonFinder';
67
import { PipenvManager } from './pipenvManager';
78

89
export async function registerPipenvFeatures(
910
nativeFinder: NativePythonFinder,
1011
disposables: Disposable[],
12+
projectManager: PythonProjectManager,
1113
): Promise<void> {
1214
const api: PythonEnvironmentApi = await getPythonApi();
1315

1416
traceInfo('Registering pipenv manager (environments will be discovered lazily)');
15-
const mgr = new PipenvManager(nativeFinder, api);
17+
const mgr = new PipenvManager(nativeFinder, api, projectManager);
1618
disposables.push(mgr, api.registerEnvironmentManager(mgr));
1719
}

src/managers/pipenv/pipenvManager.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { EventEmitter, MarkdownString, ProgressLocation, Uri } from 'vscode';
1+
import { Disposable, EventEmitter, MarkdownString, ProgressLocation, Uri, workspace } from 'vscode';
22
import {
33
DidChangeEnvironmentEventArgs,
44
DidChangeEnvironmentsEventArgs,
@@ -15,15 +15,18 @@ import {
1515
SetEnvironmentScope,
1616
} from '../../api';
1717
import { PipenvStrings } from '../../common/localize';
18+
import { traceError, traceInfo } from '../../common/logging';
1819
import { StopWatch } from '../../common/stopWatch';
1920
import { EventNames } from '../../common/telemetry/constants';
2021
import { classifyError } from '../../common/telemetry/errorClassifier';
2122
import { sendTelemetryEvent } from '../../common/telemetry/sender';
2223
import { createDeferred, Deferred } from '../../common/utils/deferred';
2324
import { normalizePath } from '../../common/utils/pathUtils';
2425
import { withProgress } from '../../common/window.apis';
26+
import { PythonProjectManager } from '../../internal.api';
2527
import { getProjectFsPathForScope, tryFastPathGet } from '../common/fastPath';
2628
import { NativePythonFinder } from '../common/nativePythonFinder';
29+
import { notifyMissingManagerIfDefault } from '../common/utils';
2730
import {
2831
clearPipenvCache,
2932
getPipenv,
@@ -36,7 +39,7 @@ import {
3639
setPipenvForWorkspaces,
3740
} from './pipenvUtils';
3841

39-
export class PipenvManager implements EnvironmentManager {
42+
export class PipenvManager implements EnvironmentManager, Disposable {
4043
private collection: PythonEnvironment[] = [];
4144
private fsPathToEnv: Map<string, PythonEnvironment> = new Map();
4245
private globalEnv: PythonEnvironment | undefined;
@@ -59,6 +62,7 @@ export class PipenvManager implements EnvironmentManager {
5962
constructor(
6063
public readonly nativeFinder: NativePythonFinder,
6164
public readonly api: PythonEnvironmentApi,
65+
private readonly projectManager?: PythonProjectManager,
6266
) {
6367
this.name = 'pipenv';
6468
this.displayName = 'Pipenv';
@@ -77,7 +81,6 @@ export class PipenvManager implements EnvironmentManager {
7781
if (this._initialized) {
7882
return this._initialized.promise;
7983
}
80-
8184
this._initialized = createDeferred();
8285
const stopWatch = new StopWatch();
8386
let result: 'success' | 'tool_not_found' | 'error' = 'success';
@@ -87,9 +90,10 @@ export class PipenvManager implements EnvironmentManager {
8790

8891
try {
8992
// Check if tool is findable before PET refresh (settings/cache/PATH only, no PET)
93+
const hasExplicitSetting = !!workspace.getConfiguration('python').get<string>('pipenvPath');
9094
const preRefreshTool = await getPipenv();
9195
if (preRefreshTool) {
92-
toolSource = 'path';
96+
toolSource = hasExplicitSetting ? 'settings' : 'local';
9397
}
9498

9599
await withProgress(
@@ -117,10 +121,14 @@ export class PipenvManager implements EnvironmentManager {
117121

118122
if (toolSource === 'none') {
119123
result = 'tool_not_found';
124+
if (this.projectManager) {
125+
await notifyMissingManagerIfDefault('ms-python.python:pipenv', this.projectManager, this.api);
126+
}
120127
}
121128
} catch (ex) {
122129
result = 'error';
123130
errorType = classifyError(ex);
131+
traceError('Pipenv lazy initialization failed', ex);
124132
} finally {
125133
sendTelemetryEvent(EventNames.MANAGER_LAZY_INIT, stopWatch.elapsedTime, {
126134
managerName: 'pipenv',
@@ -171,6 +179,7 @@ export class PipenvManager implements EnvironmentManager {
171179
title: PipenvStrings.pipenvRefreshing,
172180
},
173181
async () => {
182+
traceInfo('Refreshing Pipenv Environments');
174183
const oldCollection = [...this.collection];
175184
this.collection = await refreshPipenv(hardRefresh, this.nativeFinder, this.api, this);
176185
await this.loadEnvMap();

src/managers/poetry/main.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { Disposable, LogOutputChannel } from 'vscode';
22
import { PythonEnvironmentApi } from '../../api';
33
import { traceInfo } from '../../common/logging';
44
import { getPythonApi } from '../../features/pythonApi';
5+
import { PythonProjectManager } from '../../internal.api';
56
import { NativePythonFinder } from '../common/nativePythonFinder';
67
import { PoetryManager } from './poetryManager';
78
import { PoetryPackageManager } from './poetryPackageManager';
@@ -10,11 +11,12 @@ export async function registerPoetryFeatures(
1011
nativeFinder: NativePythonFinder,
1112
disposables: Disposable[],
1213
outputChannel: LogOutputChannel,
14+
projectManager: PythonProjectManager,
1315
): Promise<void> {
1416
const api: PythonEnvironmentApi = await getPythonApi();
1517

1618
traceInfo('Registering poetry manager (environments will be discovered lazily)');
17-
const envManager = new PoetryManager(nativeFinder, api);
19+
const envManager = new PoetryManager(nativeFinder, api, projectManager);
1820
const pkgManager = new PoetryPackageManager(api, outputChannel, envManager);
1921

2022
disposables.push(

0 commit comments

Comments
 (0)