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
40 changes: 40 additions & 0 deletions src/common/inlineScript/routingRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { Disposable, Event, EventEmitter, Uri } from 'vscode';
import { normalizeDependency } from './cacheKey';
import { InlineScriptMetadata } from './metadata';
import { normalizePath } from '../utils/pathUtils';
import type { InlineScriptEnvErrorCategory } from '../telemetry/constants';

export interface InlineScriptRouteabilityChangeEvent {
readonly uri: Uri;
Expand All @@ -20,6 +21,19 @@ export interface InlineScriptMetadataChangeEvent {
readonly metadataRevision: number;
}

/**
* Outcome of the last inline-script setup attempt for a script: a `failed` reason, or a benign
* `skipped` (an environment was built but intentionally not associated). Diagnostic side-channel —
* not routing state — recorded by the env manager and read once by the interactive setup command.
*/
export type InlineScriptSetupOutcome =
| {
readonly kind: 'failed';
readonly category: InlineScriptEnvErrorCategory;
readonly requiresPython?: string;
}
| { readonly kind: 'skipped' };

interface ScriptRoutingState {
readonly uri?: Uri;
readonly metadata?: InlineScriptMetadata;
Expand All @@ -31,6 +45,7 @@ interface ScriptRoutingState {
export class InlineScriptRoutingRegistry implements Disposable {
private readonly states = new Map<string, ScriptRoutingState>();
private readonly metadataRevisions = new Map<string, number>();
private readonly setupOutcomes = new Map<string, InlineScriptSetupOutcome>();
private readonly _onDidChangeRouteability = new EventEmitter<InlineScriptRouteabilityChangeEvent>();
private readonly _onDidChangeMetadata = new EventEmitter<InlineScriptMetadataChangeEvent>();

Expand Down Expand Up @@ -121,6 +136,30 @@ export class InlineScriptRoutingRegistry implements Disposable {
return scriptPath ? this.states.get(scriptPath)?.validatedAssociation === true : false;
}

public noteSetupOutcome(script: Uri | string, outcome: InlineScriptSetupOutcome): void {
const scriptPath = getInlineScriptRoutingKey(script);
if (scriptPath) {
this.setupOutcomes.set(scriptPath, outcome);
}
}

public clearSetupOutcome(script: Uri | string): void {
const scriptPath = getInlineScriptRoutingKey(script);
if (scriptPath) {
this.setupOutcomes.delete(scriptPath);
}
}

public takeSetupOutcome(script: Uri | string): InlineScriptSetupOutcome | undefined {
const scriptPath = getInlineScriptRoutingKey(script);
if (!scriptPath) {
return undefined;
}
const outcome = this.setupOutcomes.get(scriptPath);
this.setupOutcomes.delete(scriptPath);
return outcome;
}

public shouldRoute(uri: Uri): boolean {
const scriptPath = getInlineScriptRoutingKey(uri);
return scriptPath ? this.isRouteable(this.states.get(scriptPath)) : false;
Expand All @@ -129,6 +168,7 @@ export class InlineScriptRoutingRegistry implements Disposable {
public dispose(): void {
this.states.clear();
this.metadataRevisions.clear();
this.setupOutcomes.clear();
this._onDidChangeMetadata.dispose();
this._onDidChangeRouteability.dispose();
}
Expand Down
66 changes: 61 additions & 5 deletions src/features/inlineScript/setupEnvironment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ import { readInlineScriptMetadataFromFile } from '../../common/inlineScript/meta
import { InlineScriptRoutingRegistry } from '../../common/inlineScript/routingRegistry';
import { traceError, traceInfo } from '../../common/logging';
import { normalizePath } from '../../common/utils/pathUtils';
import { showErrorMessage, showInformationMessage, showQuickPickWithButtons } from '../../common/window.apis';
import {
showErrorMessage,
showInformationMessage,
showQuickPickWithButtons,
showWarningMessage,
} from '../../common/window.apis';
import { asRelativePath, findFiles, getOpenTextDocuments } from '../../common/workspace.apis';
import { EnvironmentManagers } from '../../internal.api';
import { registerInlineScriptCodeLens } from './codeLens';
Expand Down Expand Up @@ -41,9 +46,9 @@ const METADATA_READ_CONCURRENCY = 20;
* - `setEnvironment` persists the association, registers the exact script project, and publishes the
* per-file environment change so routing picks up the inline environment.
*
* Returns the environment on success, or `undefined` when creation produced none (the manager has
* already surfaced the reason — a declined install, no compatible Python, or a cancelled/failed
* build — and emitted telemetry).
* Returns the environment on success, or `undefined` when creation produced none. Failures and
* benign skips are recorded on the routing registry for the interactive setup command to surface or
* suppress; see {@link InlineScriptRoutingRegistry.takeSetupOutcome}.
*/
export async function setUpInlineScriptEnvironment(
scriptUri: Uri,
Expand All @@ -66,6 +71,7 @@ export async function setUpInlineScriptEnvironment(
// environment was built for stale metadata. Skip associating it rather than overwrite a newer
// setup; the current metadata's CodeLens stays so the user can run setup again.
traceInfo(`Inline-script metadata for ${scriptUri.fsPath} changed during setup; skipping association.`);
routing.noteSetupOutcome(scriptUri, { kind: 'skipped' });
return undefined;
}
await em.setEnvironment(scriptUri, environment);
Expand Down Expand Up @@ -103,7 +109,10 @@ function setupInlineScriptEnvironmentHandler(
return;
}
try {
await setUpInlineScriptEnvironment(uri, em, routing);
const environment = await setUpInlineScriptEnvironment(uri, em, routing);
if (!environment) {
notifyInlineScriptSetupOutcome(uri, routing);
}
} catch (error) {
traceError(`Failed to set up the inline-script environment for ${uri.fsPath}:`, error);
showErrorMessage(
Expand All @@ -115,6 +124,53 @@ function setupInlineScriptEnvironmentHandler(
};
}

function notifyInlineScriptSetupOutcome(uri: Uri, routing: InlineScriptRoutingRegistry): void {
const outcome = routing.takeSetupOutcome(uri);
if (outcome?.kind === 'skipped') {
// Env built but intentionally not associated (metadata changed mid-setup); stay silent.
return;
}
if (outcome?.kind === 'failed') {
if (outcome.category === 'compatible-python-declined') {
// User declined the install prompt; don't nag.
return;
}
if (outcome.category === 'no-compatible-python') {
showWarningMessage(buildNoCompatiblePythonMessage(outcome.requiresPython));
return;
}
}
showErrorMessage(
l10n.t('Failed to set up the environment for this script. See the Python Environments output for details.'),
);
}

/**
* "No compatible Python" message. Calls out an exact two-segment pin like `==3.11` (PEP 440 =
* exactly 3.11.0, often not installable) so the user understands why nothing matched.
*/
function buildNoCompatiblePythonMessage(requiresPython?: string): string {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this maybe use the new PythonVersion class for pargins and extracting major/minor?

const spec = requiresPython?.trim();
if (!spec) {
return l10n.t(
'No compatible Python could be found or installed for this script. Install a Python 3 interpreter, then try again. See the Python Environments output for details.',
);
}
const exactMinor = /^==\s*(\d+\.\d+)\s*$/.exec(spec);
if (exactMinor) {
const minor = exactMinor[1];
return l10n.t(
'No compatible Python matches this script\'s requires-python "{0}". "{0}" requires exactly {1}.0, which may not be available to install.',
spec,
minor,
);
}
return l10n.t(
'No compatible Python was found or could be installed for requires-python "{0}". Try a broader version specifier. See the Python Environments output for details.',
spec,
);
}

interface InlineScriptQuickPickItem extends QuickPickItem {
readonly uri: Uri;
readonly configured: boolean;
Expand Down
6 changes: 6 additions & 0 deletions src/managers/builtin/inlineScript/envManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable {
this.log.warn('Inline-script environment creation requires exactly one local file URI.');
return undefined;
}
this.routingRegistry.clearSetupOutcome(scriptUri);

const metadata = await readInlineScriptMetadataFromFile(scriptUri);
if (!metadata) {
Expand Down Expand Up @@ -337,6 +338,11 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable {
if (baseSelection.errorCategory) {
this.sendInlineScriptEnvErrorTelemetry(baseSelection.errorCategory);
}
this.routingRegistry.noteSetupOutcome(scriptUri, {
kind: 'failed',
category: baseSelection.errorCategory ?? 'setup-failure',
requiresPython: metadata.requiresPython,
});
this.log.warn(
`No compatible Python is available for inline-script environment creation: ${scriptUri.fsPath}.`,
);
Expand Down
49 changes: 41 additions & 8 deletions src/managers/builtin/venvUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,43 @@ export interface CreateWithProgressOptions {
readonly nameStyle?: VenvNameStyle;
}

/**
* The interpreter to build a venv from. Usually the environment's own executable, but a discovered
* base can point at a launcher/shim outside its prefix (e.g. uv's `.local/bin/pythonX.Y`), which uv
* cannot inspect for older Pythons (`uv venv --python <shim>` fails to initialize). When the
* executable is not inside its own prefix, use the interpreter inside the prefix instead.
*/
export async function getBaseInterpreterForVenv(basePython: PythonEnvironment): Promise<string | undefined> {
const executable = basePython.execInfo?.run.executable;
const sysPrefix = basePython.sysPrefix;
if (!executable || !sysPrefix || !path.isAbsolute(executable) || !path.isAbsolute(sysPrefix)) {
return executable;
}
if (isInterpreterInsidePrefix(executable, sysPrefix)) {
return executable;
}
for (const candidate of [
path.join(sysPrefix, 'python.exe'),
path.join(sysPrefix, 'bin', 'python'),
path.join(sysPrefix, 'bin', 'python3'),
]) {
if (await fsapi.pathExists(candidate)) {
return candidate;
}
}
return executable;
}

function isInterpreterInsidePrefix(executable: string, prefix: string): boolean {
const relative = path.relative(prefix, executable);
return (
relative.length > 0 &&
relative !== '..' &&
!relative.startsWith(`..${path.sep}`) &&
!path.isAbsolute(relative)
);
}

export async function createWithProgress(
nativeFinder: NativePythonFinder,
api: PythonEnvironmentApi,
Expand Down Expand Up @@ -396,20 +433,16 @@ export async function createWithProgress(
try {
const useUv = await shouldUseUv(log, basePython.environmentPath.fsPath);
// env creation
if (basePython.execInfo?.run.executable) {
const baseExecutable = await getBaseInterpreterForVenv(basePython);
if (baseExecutable) {
if (useUv) {
await runUV(
['venv', '--verbose', '--seed', '--python', basePython.execInfo?.run.executable, envPath],
['venv', '--verbose', '--seed', '--python', baseExecutable, envPath],
venvRoot.fsPath,
log,
);
} else {
await runPython(
basePython.execInfo.run.executable,
['-m', 'venv', envPath],
venvRoot.fsPath,
manager.log,
);
await runPython(baseExecutable, ['-m', 'venv', envPath], venvRoot.fsPath, manager.log);
}
if (!(await fsapi.pathExists(pythonPath))) {
throw new Error('no python executable found in virtual environment');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import * as windowApis from '../../../common/window.apis';
import { getVenvPythonPath } from '../../../common/utils/virtualEnvironment';
import * as builtinHelpers from '../../../managers/builtin/helpers';
import * as uvEnvironments from '../../../managers/builtin/uvEnvironments';
import { createWithProgress } from '../../../managers/builtin/venvUtils';
import { createWithProgress, getBaseInterpreterForVenv } from '../../../managers/builtin/venvUtils';
import { NativePythonEnvironmentKind, NativePythonFinder } from '../../../managers/common/nativePythonFinder';
import * as managerUtils from '../../../managers/common/utils';

Expand Down Expand Up @@ -132,3 +132,51 @@ suite('createWithProgress uv tracking', () => {
assert.strictEqual(result.pkgInstallationCancelled, true);
});
});

suite('getBaseInterpreterForVenv', () => {
let tempRoot: string;

setup(async () => {
tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'base-interp-'));
});

teardown(async () => {
await fs.remove(tempRoot);
});

function makeBase(executable: string, sysPrefix: string): PythonEnvironment {
return {
envId: { id: 'base', managerId: 'ms-python.python:system' },
name: 'base',
displayName: 'base',
displayPath: executable,
version: '3.8.20',
environmentPath: Uri.file(executable),
execInfo: { run: { executable } },
sysPrefix,
} as PythonEnvironment;
}

const inPrefixInterpreter = (prefix: string): string =>
process.platform === 'win32' ? path.join(prefix, 'python.exe') : path.join(prefix, 'bin', 'python');

test('returns the executable unchanged when it lives inside its own prefix', async () => {
const executable = inPrefixInterpreter(tempRoot);
const result = await getBaseInterpreterForVenv(makeBase(executable, tempRoot));
assert.strictEqual(result, executable);
});

test('redirects a shim outside the prefix to the interpreter inside the prefix', async () => {
const realInterpreter = inPrefixInterpreter(tempRoot);
await fs.outputFile(realInterpreter, '');
const shim = path.join(os.tmpdir(), 'shim-bin', 'python3.8.exe');
const result = await getBaseInterpreterForVenv(makeBase(shim, tempRoot));
assert.strictEqual(result, realInterpreter);
});

test('falls back to the original executable when no interpreter exists in the prefix', async () => {
const shim = path.join(os.tmpdir(), 'shim-bin', 'python3.8.exe');
const result = await getBaseInterpreterForVenv(makeBase(shim, tempRoot));
assert.strictEqual(result, shim);
});
});
Loading