-
Notifications
You must be signed in to change notification settings - Fork 37
Implement Python installation via uv #1198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request implements Python installation via uv when no Python environments are found, addressing issue #1172. The feature prompts users to install Python using the uv tool, which can install Python without requiring Python to be present first. This streamlines the experience for new Python users or those setting up fresh machines.
Changes:
- Added Python installation functionality through uv with cross-platform support (Windows, macOS, Linux)
- Integrated installation prompts at two trigger points: extension activation and environment creation
- Added persistent "Don't ask again" state management for user preferences
- Implemented comprehensive telemetry tracking for installation flow
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/managers/builtin/uvPythonInstaller.ts | New module implementing uv and Python installation logic with platform-specific commands, task execution, and user prompts |
| src/test/managers/builtin/uvPythonInstaller.unit.test.ts | Unit tests for prompt behavior, persistent state management, and telemetry |
| src/managers/builtin/sysPythonManager.ts | Added prompt during initialization and create method to install Python globally via uv |
| src/managers/builtin/venvManager.ts | Added prompt during venv creation when no global Python environments exist |
| src/features/views/treeViewItems.ts | Updated UI labels for system manager to show "click to install" when no Python found |
| src/common/telemetry/constants.ts | Added four telemetry events for tracking installation flow |
| src/common/localize.ts | Added localized strings for installation UI and messages |
Comments suppressed due to low confidence (5)
src/managers/builtin/uvPythonInstaller.ts:253
- Direct usage of
window.showErrorMessageandwindow.showInformationMessagefrom the vscode module should be replaced with the wrapper functions fromsrc/common/window.apis.ts. While one wrapper is already imported and used for the prompt (line 189), these direct calls should be replaced with the importedshowErrorMessagefrom window.apis for consistency.
window.showErrorMessage(UvInstallStrings.uvInstallFailed);
return false;
}
}
// Step 2: Install Python via uv
const pythonSuccess = await installPythonViaUv(log);
if (!pythonSuccess) {
sendTelemetryEvent(EventNames.UV_PYTHON_INSTALL_FAILED, undefined, { stage: 'pythonInstall' });
window.showErrorMessage(UvInstallStrings.installFailed);
return false;
}
// Step 3: Refresh environments to detect newly installed Python
traceInfo('Refreshing environments after Python installation...');
await api.refreshEnvironments(undefined);
sendTelemetryEvent(EventNames.UV_PYTHON_INSTALL_COMPLETED);
window.showInformationMessage(UvInstallStrings.installComplete);
src/managers/builtin/uvPythonInstaller.ts:36
- The
isCommandAvailablefunction doesn't handle the case where the spawned process might hang indefinitely. If the--versioncommand forcurlorwgetdoesn't respond, the Promise will never resolve, causinggetUvInstallCommandto hang. Consider adding a timeout mechanism usingsetTimeoutto reject the promise after a reasonable duration (e.g., 5 seconds).
async function isCommandAvailable(command: string): Promise<boolean> {
return new Promise((resolve) => {
const proc = spawnProcess(command, ['--version']);
proc.on('error', () => resolve(false));
proc.on('exit', (code) => resolve(code === 0));
});
}
src/managers/builtin/uvPythonInstaller.ts:95
- Direct usage of
tasks.onDidEndTaskProcessfrom the vscode module should ideally use a wrapper function for consistency with the codebase pattern. However, no wrapper exists yet insrc/common/tasks.apis.ts. Consider adding a wrapper function there and using it here, similar to howexecuteTaskis wrapped.
const disposable = tasks.onDidEndTaskProcess((e) => {
src/managers/builtin/sysPythonManager.ts:242
- The comment states "The installPythonWithUv function already refreshes environments", but there's a potential timing issue. The
installPythonWithUvfunction callsapi.refreshEnvironments(undefined)which likely triggerssysPythonManager.refresh(), updatingsysPythonManager.collection. However, thecreatemethod immediately reads fromthis.collectionwhich might not yet reflect the newly installed Python if the refresh is still in progress or hasn't propagated. Consider awaiting a small delay or re-fetching environments explicitly within thecreatemethod after theinstallPythonWithUvcall to ensure the collection is up-to-date.
const success = await installPythonWithUv(this.api, this.log);
if (success) {
// Return the latest Python environment after installation
// The installPythonWithUv function already refreshes environments
return getLatest(this.collection);
src/test/managers/builtin/uvPythonInstaller.unit.test.ts:123
- Missing test case for when user clicks "Install Python" button. The tests cover the "Don't ask again" and dismiss scenarios, but there's no test verifying that when the user selects
UvInstallStrings.installPython, the function callsinstallPythonWithUvand returns its result. This is a critical flow that should be tested to ensure proper integration.
suite('uvPythonInstaller - promptInstallPythonViaUv', () => {
let mockLog: LogOutputChannel;
let mockApi: { refreshEnvironments: sinon.SinonStub };
let isUvInstalledStub: sinon.SinonStub;
let showInformationMessageStub: sinon.SinonStub;
let sendTelemetryEventStub: sinon.SinonStub;
let mockState: { get: sinon.SinonStub; set: sinon.SinonStub; clear: sinon.SinonStub };
setup(() => {
mockLog = createMockLogOutputChannel();
mockApi = { refreshEnvironments: sinon.stub().resolves() };
mockState = {
get: sinon.stub(),
set: sinon.stub().resolves(),
clear: sinon.stub().resolves(),
};
sinon.stub(persistentState, 'getGlobalPersistentState').resolves(mockState);
isUvInstalledStub = sinon.stub(helpers, 'isUvInstalled');
showInformationMessageStub = sinon.stub(windowApis, 'showInformationMessage');
sendTelemetryEventStub = sinon.stub(telemetrySender, 'sendTelemetryEvent');
});
teardown(() => {
sinon.restore();
});
test('should return false when "Don\'t ask again" is set', async () => {
mockState.get.resolves(true);
const result = await promptInstallPythonViaUv('activation', mockApi as any, mockLog);
assert.strictEqual(result, false);
assert(showInformationMessageStub.notCalled, 'Should not show message when dont ask again is set');
assert(sendTelemetryEventStub.notCalled, 'Should not send telemetry when skipping prompt');
});
test('should show correct prompt when uv is installed', async () => {
mockState.get.resolves(false);
isUvInstalledStub.resolves(true);
showInformationMessageStub.resolves(undefined); // User dismissed
await promptInstallPythonViaUv('activation', mockApi as any, mockLog);
assert(
showInformationMessageStub.calledWith(
UvInstallStrings.installPythonPrompt,
UvInstallStrings.installPython,
UvInstallStrings.dontAskAgain,
),
'Should show install Python prompt when uv is installed',
);
});
test('should show correct prompt when uv is NOT installed', async () => {
mockState.get.resolves(false);
isUvInstalledStub.resolves(false);
showInformationMessageStub.resolves(undefined); // User dismissed
await promptInstallPythonViaUv('activation', mockApi as any, mockLog);
assert(
showInformationMessageStub.calledWith(
UvInstallStrings.installPythonAndUvPrompt,
UvInstallStrings.installPython,
UvInstallStrings.dontAskAgain,
),
'Should show install Python AND uv prompt when uv is not installed',
);
});
test('should set persistent state when user clicks "Don\'t ask again"', async () => {
mockState.get.resolves(false);
isUvInstalledStub.resolves(true);
showInformationMessageStub.resolves(UvInstallStrings.dontAskAgain);
const result = await promptInstallPythonViaUv('activation', mockApi as any, mockLog);
assert.strictEqual(result, false);
assert(mockState.set.calledWith('python-envs:uv:UV_INSTALL_PYTHON_DONT_ASK', true), 'Should set dont ask flag');
});
test('should return false when user dismisses the dialog', async () => {
mockState.get.resolves(false);
isUvInstalledStub.resolves(true);
showInformationMessageStub.resolves(undefined); // User dismissed
const result = await promptInstallPythonViaUv('activation', mockApi as any, mockLog);
assert.strictEqual(result, false);
});
test('should send telemetry with correct trigger', async () => {
mockState.get.resolves(false);
isUvInstalledStub.resolves(true);
showInformationMessageStub.resolves(undefined);
await promptInstallPythonViaUv('createEnvironment', mockApi as any, mockLog);
assert(
sendTelemetryEventStub.calledWith(EventNames.UV_PYTHON_INSTALL_PROMPTED, undefined, {
trigger: 'createEnvironment',
}),
'Should send telemetry with createEnvironment trigger',
);
});
});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
src/managers/builtin/sysPythonManager.ts:266
- In
create(), when a Python is installed and resolved, it’s added tocollectionbut not persisted as the global selection and not reflected inglobalEnv/fsPathToEnv. If the goal is “install a global Python”, consider setting it asglobalEnvand persisting it (e.g., viathis.set(undefined, resolved)orsetSystemEnvForGlobal(resolved.environmentPath.fsPath)) so subsequentget()calls and restarts see the installed interpreter.
const pythonPath = await installPythonWithUv(this.log, selectedVersion);
if (pythonPath) {
// Resolve the installed Python using NativePythonFinder instead of full refresh
const resolved = await resolveSystemPythonEnvironmentPath(pythonPath, this.nativeFinder, this.api, this);
if (resolved) {
// Add to collection and fire change event
this.collection.push(resolved);
this._onDidChangeEnvironments.fire([{ environment: resolved, kind: EnvironmentChangeKind.add }]);
return resolved;
}
…await, handle empty envs
- Use monotonic counter instead of Date.now() in ShellExecution.computeId() for deterministic IDs - Export UV_INSTALL_PYTHON_DONT_ASK_KEY constant and use in tests - Capitalize 'Python' in noEnvFound localized string - Re-check uv availability after installation to handle PATH issues - Add uvInstallRestartRequired localized string for when uv is not on PATH
…lobalEnv, fix comments, add tests
eb4d249 to
083a1da
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/managers/builtin/uvPythonInstaller.ts:294
selectPythonVersionToInstall()contains important filtering/grouping logic (cpython+default only, de-dupe by minor version, handling empty list) but isn’t covered by unit tests. Consider adding unit tests by stubbinggetAvailablePythonVersions()/spawnProcessandshowQuickPickto validate the presented items and the returned selected version.
export async function selectPythonVersionToInstall(): Promise<string | undefined> {
const versions = await withProgress(
{
location: ProgressLocation.Notification,
title: UvInstallStrings.fetchingVersions,
},
async () => getAvailablePythonVersions(),
);
if (versions.length === 0) {
showErrorMessage(UvInstallStrings.failedToFetchVersions);
return undefined;
}
// Filter to only default variant (not freethreaded) and group by minor version
const seenMinorVersions = new Set<string>();
const items: PythonVersionQuickPickItem[] = [];
for (const v of versions) {
// Only include default variant CPython
if (v.variant !== 'default' || v.implementation !== 'cpython') {
continue;
}
// Create a minor version key (e.g., "3.13")
const minorKey = `${v.version_parts.major}.${v.version_parts.minor}`;
// Only show the latest patch for each minor version (they come sorted from uv)
if (seenMinorVersions.has(minorKey)) {
continue;
}
seenMinorVersions.add(minorKey);
const isInstalled = v.path !== null;
items.push({
label: `Python ${v.version}`,
description: isInstalled ? `$(check) ${UvInstallStrings.installed}` : undefined,
detail: isInstalled ? (v.path ?? undefined) : undefined,
version: v.version,
isInstalled,
});
}
const selected = await showQuickPick(items, {
placeHolder: UvInstallStrings.selectPythonVersion,
ignoreFocusOut: true,
});
if (!selected) {
return undefined;
}
return selected.version;
}
Address PR review comment: cover JSON parsing, version prefix matching, empty output, chunked stdout, process errors, and non-zero exit code scenarios.
Fixes #1172
Introduce functionality to prompt users for Python installation using uv when no environments are found. This includes updates to the UI and event tracking for the installation process.