diff --git a/docs/managing-python-projects.md b/docs/managing-python-projects.md index 645b3a1a..9f64f773 100644 --- a/docs/managing-python-projects.md +++ b/docs/managing-python-projects.md @@ -211,6 +211,8 @@ You can set default managers that apply to all projects without explicit overrid } ``` +> **Important**: The extension never writes settings to the User (global) scope. All extension-managed settings are written at the Workspace or Workspace Folder level only. This prevents the extension from setting values that persist across unrelated projects and cause unexpected interference (see [#1468](https://github.com/microsoft/vscode-python-environments/issues/1468)). If a user wants a user-level default, they can set it manually in their User `settings.json`. + ## Working with Multi-Root Workspaces Multi-root workspaces contain multiple top-level folders. The extension handles these seamlessly: diff --git a/src/features/settings/settingHelpers.ts b/src/features/settings/settingHelpers.ts index 4ef9f9ab..0ee67396 100644 --- a/src/features/settings/settingHelpers.ts +++ b/src/features/settings/settingHelpers.ts @@ -2,7 +2,7 @@ import * as path from 'path'; import { ConfigurationScope, ConfigurationTarget, Uri, WorkspaceConfiguration, WorkspaceFolder } from 'vscode'; import { PythonProject } from '../../api'; import { DEFAULT_ENV_MANAGER_ID, DEFAULT_PACKAGE_MANAGER_ID } from '../../common/constants'; -import { traceError, traceInfo, traceWarn } from '../../common/logging'; +import { traceError, traceInfo, traceVerbose, traceWarn } from '../../common/logging'; import * as workspaceApis from '../../common/workspace.apis'; import { PythonProjectManager, PythonProjectSettings } from '../../internal.api'; @@ -81,8 +81,16 @@ export function getDefaultPkgManagerSetting( return defaultManager; } +function traceIgnoredGlobalManagerEdits(functionName: string, count: number): void { + if (count > 0) { + traceVerbose( + `[${functionName}] Ignoring ${count} edit(s) without a project because python-envs does not persist manager defaults to User/global settings.`, + ); + } +} + export interface EditAllManagerSettings { - // undefined means global + // Edits without a project are ignored; python-envs does not persist manager defaults to User/global settings. project?: PythonProject; envManager: string; packageManager: string; @@ -95,24 +103,25 @@ interface EditAllManagerSettingsInternal { export async function setAllManagerSettings(edits: EditAllManagerSettings[]): Promise { const noWorkspace: EditAllManagerSettingsInternal[] = []; const workspaces = new Map(); - edits - .filter((e) => !!e.project) - .map((e) => e as EditAllManagerSettingsInternal) - .forEach((e) => { - const w = workspaceApis.getWorkspaceFolder(e.project.uri); - if (w) { - workspaces.set(w, [ - ...(workspaces.get(w) || []), - { project: e.project, envManager: e.envManager, packageManager: e.packageManager }, - ]); - } else { - noWorkspace.push({ project: e.project, envManager: e.envManager, packageManager: e.packageManager }); - } - }); + const projectEdits = edits.filter((e): e is EditAllManagerSettingsInternal => !!e.project); + traceIgnoredGlobalManagerEdits('setAllManagerSettings', edits.length - projectEdits.length); + projectEdits.forEach((e) => { + const w = workspaceApis.getWorkspaceFolder(e.project.uri); + if (w) { + workspaces.set(w, [ + ...(workspaces.get(w) || []), + { project: e.project, envManager: e.envManager, packageManager: e.packageManager }, + ]); + } else { + noWorkspace.push({ project: e.project, envManager: e.envManager, packageManager: e.packageManager }); + } + }); noWorkspace.forEach((e) => { if (e.project) { - traceInfo(`Unable to find workspace for ${e.project.uri.fsPath}, will use global settings for this.`); + traceInfo( + `Unable to find workspace for ${e.project.uri.fsPath}, skipping settings update because no User/global fallback is written.`, + ); } }); @@ -195,23 +204,11 @@ export async function setAllManagerSettings(edits: EditAllManagerSettings[]): Pr } }); - const config = workspaceApis.getConfiguration('python-envs', undefined); - edits - .filter((e) => !e.project) - .forEach((e) => { - if (config.get('defaultEnvManager') !== e.envManager) { - promises.push(config.update('defaultEnvManager', e.envManager, ConfigurationTarget.Global)); - } - if (config.get('defaultPackageManager') !== e.packageManager) { - promises.push(config.update('defaultPackageManager', e.packageManager, ConfigurationTarget.Global)); - } - }); - await Promise.all(promises); } export interface EditEnvManagerSettings { - // undefined means global + // Edits without a project are ignored; python-envs does not persist manager defaults to User/global settings. project?: PythonProject; envManager: string; } @@ -222,17 +219,16 @@ interface EditEnvManagerSettingsInternal { export async function setEnvironmentManager(edits: EditEnvManagerSettings[]): Promise { const noWorkspace: EditEnvManagerSettingsInternal[] = []; const workspaces = new Map(); - edits - .filter((e) => !!e.project) - .map((e) => e as EditEnvManagerSettingsInternal) - .forEach((e) => { - const w = workspaceApis.getWorkspaceFolder(e.project.uri); - if (w) { - workspaces.set(w, [...(workspaces.get(w) || []), { project: e.project, envManager: e.envManager }]); - } else { - noWorkspace.push({ project: e.project, envManager: e.envManager }); - } - }); + const projectEdits = edits.filter((e): e is EditEnvManagerSettingsInternal => !!e.project); + traceIgnoredGlobalManagerEdits('setEnvironmentManager', edits.length - projectEdits.length); + projectEdits.forEach((e) => { + const w = workspaceApis.getWorkspaceFolder(e.project.uri); + if (w) { + workspaces.set(w, [...(workspaces.get(w) || []), { project: e.project, envManager: e.envManager }]); + } else { + noWorkspace.push({ project: e.project, envManager: e.envManager }); + } + }); noWorkspace.forEach((e) => { if (e.project) { @@ -269,20 +265,11 @@ export async function setEnvironmentManager(edits: EditEnvManagerSettings[]): Pr } }); - const config = workspaceApis.getConfiguration('python-envs', undefined); - edits - .filter((e) => !e.project) - .forEach((e) => { - if (config.get('defaultEnvManager') !== e.envManager) { - promises.push(config.update('defaultEnvManager', e.envManager, ConfigurationTarget.Global)); - } - }); - await Promise.all(promises); } export interface EditPackageManagerSettings { - // undefined means global + // Edits without a project are ignored; python-envs does not persist manager defaults to User/global settings. project?: PythonProject; packageManager: string; } @@ -293,20 +280,16 @@ interface EditPackageManagerSettingsInternal { export async function setPackageManager(edits: EditPackageManagerSettings[]): Promise { const noWorkspace: EditPackageManagerSettingsInternal[] = []; const workspaces = new Map(); - edits - .filter((e) => !!e.project) - .map((e) => e as EditPackageManagerSettingsInternal) - .forEach((e) => { - const w = workspaceApis.getWorkspaceFolder(e.project.uri); - if (w) { - workspaces.set(w, [ - ...(workspaces.get(w) || []), - { project: e.project, packageManager: e.packageManager }, - ]); - } else { - noWorkspace.push({ project: e.project, packageManager: e.packageManager }); - } - }); + const projectEdits = edits.filter((e): e is EditPackageManagerSettingsInternal => !!e.project); + traceIgnoredGlobalManagerEdits('setPackageManager', edits.length - projectEdits.length); + projectEdits.forEach((e) => { + const w = workspaceApis.getWorkspaceFolder(e.project.uri); + if (w) { + workspaces.set(w, [...(workspaces.get(w) || []), { project: e.project, packageManager: e.packageManager }]); + } else { + noWorkspace.push({ project: e.project, packageManager: e.packageManager }); + } + }); noWorkspace.forEach((e) => { if (e.project) { @@ -343,15 +326,6 @@ export async function setPackageManager(edits: EditPackageManagerSettings[]): Pr } }); - const config = workspaceApis.getConfiguration('python-envs', undefined); - edits - .filter((e) => !e.project) - .forEach((e) => { - if (config.get('defaultPackageManager') !== e.packageManager) { - promises.push(config.update('defaultPackageManager', e.packageManager, ConfigurationTarget.Global)); - } - }); - await Promise.all(promises); } diff --git a/src/test/features/settings/settingHelpers.unit.test.ts b/src/test/features/settings/settingHelpers.unit.test.ts index 1cb3a0c1..40233435 100644 --- a/src/test/features/settings/settingHelpers.unit.test.ts +++ b/src/test/features/settings/settingHelpers.unit.test.ts @@ -3,6 +3,7 @@ import * as assert from 'assert'; import * as path from 'path'; import * as sinon from 'sinon'; import { ConfigurationTarget, Uri, WorkspaceFolder } from 'vscode'; +import * as logging from '../../../common/logging'; import * as workspaceApis from '../../../common/workspace.apis'; import { addPythonProjectSetting, @@ -22,12 +23,8 @@ function getTestWorkspacePath(): string { } /** - * These tests verify that settings ARE written when the value changes, - * regardless of whether it's the default/system manager or not. - * - * Note: These tests focus on the global settings path (project=undefined) because - * workspace-scoped tests would require mocking workspace.getWorkspaceFolder which - * cannot be easily stubbed in unit tests. + * These tests verify that manager edits without a project do not write settings + * and are logged explicitly as ignored global edits. */ suite('Setting Helpers - Settings Write Behavior', () => { const SYSTEM_MANAGER_ID = 'ms-python.python:system'; @@ -35,7 +32,7 @@ suite('Setting Helpers - Settings Write Behavior', () => { const PIP_MANAGER_ID = 'ms-python.python:pip'; const CONDA_MANAGER_ID = 'ms-python.python:conda'; - let updateCalls: Array<{ key: string; value: unknown; target: ConfigurationTarget }>; + let updateCalls: Array<{ key: string; value: unknown; target: boolean | ConfigurationTarget | undefined }>; setup(() => { updateCalls = []; @@ -46,7 +43,7 @@ suite('Setting Helpers - Settings Write Behavior', () => { }); /** - * Creates a mock WorkspaceConfiguration that tracks update calls + * Creates a mock WorkspaceConfiguration that tracks update calls. */ function createMockConfig(options: { defaultEnvManagerGlobalValue?: string; @@ -99,7 +96,7 @@ suite('Setting Helpers - Settings Write Behavior', () => { updateCalls.push({ key: section, value, - target: configurationTarget as ConfigurationTarget, + target: configurationTarget, }); return Promise.resolve(); }; @@ -108,11 +105,12 @@ suite('Setting Helpers - Settings Write Behavior', () => { } suite('setAllManagerSettings - Global Settings', () => { - test('should write global defaultEnvManager when value differs from current', async () => { + test('should NOT write global defaultEnvManager even when value differs from current', async () => { const mockConfig = createMockConfig({ currentEnvManager: VENV_MANAGER_ID, }); sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); + const traceVerboseStub = sinon.stub(logging, 'traceVerbose'); await setAllManagerSettings([ { @@ -122,34 +120,15 @@ suite('Setting Helpers - Settings Write Behavior', () => { }, ]); - const envManagerUpdates = updateCalls.filter( - (c) => c.key === 'defaultEnvManager' && c.target === ConfigurationTarget.Global, - ); - assert.strictEqual(envManagerUpdates.length, 1, 'Should write global defaultEnvManager when value differs'); - assert.strictEqual(envManagerUpdates[0].value, SYSTEM_MANAGER_ID); - }); - - test('should NOT write global defaultEnvManager when value is same as current', async () => { - const mockConfig = createMockConfig({ - currentEnvManager: SYSTEM_MANAGER_ID, - }); - sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); - - await setAllManagerSettings([ - { - project: undefined, - envManager: SYSTEM_MANAGER_ID, - packageManager: PIP_MANAGER_ID, - }, - ]); - - const envManagerUpdates = updateCalls.filter( - (c) => c.key === 'defaultEnvManager' && c.target === ConfigurationTarget.Global, + const envManagerUpdates = updateCalls.filter((c) => c.key === 'defaultEnvManager'); + assert.strictEqual(envManagerUpdates.length, 0, 'Should never write defaultEnvManager for global edits'); + sinon.assert.calledWithMatch( + traceVerboseStub, + '[setAllManagerSettings] Ignoring 1 edit(s) without a project because python-envs does not persist manager defaults to User/global settings.', ); - assert.strictEqual(envManagerUpdates.length, 0, 'Should NOT write when value is same as current'); }); - test('should write global defaultPackageManager when value differs from current', async () => { + test('should NOT write global defaultPackageManager even when value differs from current', async () => { const mockConfig = createMockConfig({ currentEnvManager: VENV_MANAGER_ID, currentPkgManager: PIP_MANAGER_ID, @@ -164,24 +143,22 @@ suite('Setting Helpers - Settings Write Behavior', () => { }, ]); - const pkgManagerUpdates = updateCalls.filter( - (c) => c.key === 'defaultPackageManager' && c.target === ConfigurationTarget.Global, - ); + const pkgManagerUpdates = updateCalls.filter((c) => c.key === 'defaultPackageManager'); assert.strictEqual( pkgManagerUpdates.length, - 1, - 'Should write global defaultPackageManager when value differs', + 0, + 'Should never write defaultPackageManager for global edits', ); - assert.strictEqual(pkgManagerUpdates[0].value, CONDA_MANAGER_ID); }); }); suite('setEnvironmentManager - Global Settings', () => { - test('should write when value differs from current', async () => { + test('should NOT write to global even when value differs from current', async () => { const mockConfig = createMockConfig({ currentEnvManager: VENV_MANAGER_ID, }); sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); + const traceVerboseStub = sinon.stub(logging, 'traceVerbose'); await setEnvironmentManager([ { @@ -190,38 +167,22 @@ suite('Setting Helpers - Settings Write Behavior', () => { }, ]); - const envManagerUpdates = updateCalls.filter( - (c) => c.key === 'defaultEnvManager' && c.target === ConfigurationTarget.Global, - ); - assert.strictEqual(envManagerUpdates.length, 1, 'Should write global defaultEnvManager when value differs'); - }); - - test('should NOT write when value is same as current', async () => { - const mockConfig = createMockConfig({ - currentEnvManager: SYSTEM_MANAGER_ID, - }); - sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); - - await setEnvironmentManager([ - { - project: undefined, - envManager: SYSTEM_MANAGER_ID, - }, - ]); - - const envManagerUpdates = updateCalls.filter( - (c) => c.key === 'defaultEnvManager' && c.target === ConfigurationTarget.Global, + const envManagerUpdates = updateCalls.filter((c) => c.key === 'defaultEnvManager'); + assert.strictEqual(envManagerUpdates.length, 0, 'Should never write defaultEnvManager for global edits'); + sinon.assert.calledWithMatch( + traceVerboseStub, + '[setEnvironmentManager] Ignoring 1 edit(s) without a project because python-envs does not persist manager defaults to User/global settings.', ); - assert.strictEqual(envManagerUpdates.length, 0, 'Should NOT write when value is same'); }); }); suite('setPackageManager - Global Settings', () => { - test('should write when value differs from current', async () => { + test('should NOT write to global even when value differs from current', async () => { const mockConfig = createMockConfig({ currentPkgManager: PIP_MANAGER_ID, }); sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); + const traceVerboseStub = sinon.stub(logging, 'traceVerbose'); await setPackageManager([ { @@ -230,33 +191,16 @@ suite('Setting Helpers - Settings Write Behavior', () => { }, ]); - const pkgManagerUpdates = updateCalls.filter( - (c) => c.key === 'defaultPackageManager' && c.target === ConfigurationTarget.Global, - ); + const pkgManagerUpdates = updateCalls.filter((c) => c.key === 'defaultPackageManager'); assert.strictEqual( pkgManagerUpdates.length, - 1, - 'Should write global defaultPackageManager when value differs', + 0, + 'Should never write defaultPackageManager for global edits', ); - }); - - test('should NOT write when value is same as current', async () => { - const mockConfig = createMockConfig({ - currentPkgManager: PIP_MANAGER_ID, - }); - sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); - - await setPackageManager([ - { - project: undefined, - packageManager: PIP_MANAGER_ID, - }, - ]); - - const pkgManagerUpdates = updateCalls.filter( - (c) => c.key === 'defaultPackageManager' && c.target === ConfigurationTarget.Global, + sinon.assert.calledWithMatch( + traceVerboseStub, + '[setPackageManager] Ignoring 1 edit(s) without a project because python-envs does not persist manager defaults to User/global settings.', ); - assert.strictEqual(pkgManagerUpdates.length, 0, 'Should NOT write when value is same'); }); }); }); @@ -278,7 +222,7 @@ suite('Setting Helpers - Empty Path Bug Fix', () => { index: 0, }; - let updateCalls: Array<{ key: string; value: unknown; target: ConfigurationTarget }>; + let updateCalls: Array<{ key: string; value: unknown; target: boolean | ConfigurationTarget | undefined }>; setup(() => { updateCalls = []; @@ -316,7 +260,7 @@ suite('Setting Helpers - Empty Path Bug Fix', () => { updateCalls.push({ key: section, value, - target: configurationTarget as ConfigurationTarget, + target: configurationTarget, }); return Promise.resolve(); };