Skip to content

Commit 56051b8

Browse files
author
Kartik Raj
authored
Do a one-off transfer of existing values for python.pythonPath setting to new Interpreter storage if in DeprecatePythonPath experiment (#11053)
* Added functionality * Added tests to interpreter path service * Added tests for activation manager * News entry * Fix bug * Correct activation manager tests * Add tests for the new API method * Update unit tests
1 parent cefccd1 commit 56051b8

File tree

7 files changed

+867
-520
lines changed

7 files changed

+867
-520
lines changed

news/1 Enhancements/11052.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Do a one-off transfer of existing values for `python.pythonPath` setting to new Interpreter storage if in DeprecatePythonPath experiment.

src/client/activation/activationManager.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,12 @@ export class ExtensionActivationManager implements IExtensionActivationManager {
6666
return;
6767
}
6868
this.activatedWorkspaces.add(key);
69+
70+
if (this.experiments.inExperiment(DeprecatePythonPath.experiment)) {
71+
await this.interpreterPathService.copyOldInterpreterStorageValuesToNew(resource);
72+
}
73+
this.experiments.sendTelemetryIfInExperiment(DeprecatePythonPath.control);
74+
6975
// Get latest interpreter list in the background.
7076
this.interpreterService.getInterpreters(resource).ignoreErrors();
7177

src/client/common/interpreterPathService.ts

Lines changed: 83 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { ConfigurationChangeEvent, ConfigurationTarget, Event, EventEmitter, Uri
99
import { IWorkspaceService } from './application/types';
1010
import { PythonSettings } from './configSettings';
1111
import { isTestExecution } from './constants';
12+
import { FileSystemPaths } from './platform/fs-paths';
1213
import {
1314
IDisposable,
1415
IDisposableRegistry,
@@ -21,6 +22,9 @@ import {
2122
Resource
2223
} from './types';
2324

25+
export const workspaceKeysForWhichTheCopyIsDone_Key = 'workspaceKeysForWhichTheCopyIsDone_Key';
26+
export const workspaceFolderKeysForWhichTheCopyIsDone_Key = 'workspaceFolderKeysForWhichTheCopyIsDone_Key';
27+
export const isGlobalSettingCopiedKey = 'isGlobalSettingCopiedKey';
2428
export const defaultInterpreterPathSetting: keyof IPythonSettings = 'defaultInterpreterPath';
2529
const CI_PYTHON_PATH = getCIPythonPath();
2630

@@ -32,13 +36,18 @@ export function getCIPythonPath(): string {
3236
}
3337
@injectable()
3438
export class InterpreterPathService implements IInterpreterPathService {
39+
public get onDidChange(): Event<InterpreterConfigurationScope> {
40+
return this._didChangeInterpreterEmitter.event;
41+
}
3542
public _didChangeInterpreterEmitter = new EventEmitter<InterpreterConfigurationScope>();
43+
private fileSystemPaths: FileSystemPaths;
3644
constructor(
3745
@inject(IPersistentStateFactory) private readonly persistentStateFactory: IPersistentStateFactory,
3846
@inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService,
3947
@inject(IDisposableRegistry) disposables: IDisposable[]
4048
) {
4149
disposables.push(this.workspaceService.onDidChangeConfiguration(this.onDidChangeConfiguration.bind(this)));
50+
this.fileSystemPaths = FileSystemPaths.withDefaults();
4251
}
4352

4453
public async onDidChangeConfiguration(event: ConfigurationChangeEvent) {
@@ -88,8 +97,11 @@ export class InterpreterPathService implements IInterpreterPathService {
8897
resource = PythonSettings.getSettingsUriAndTarget(resource, this.workspaceService).uri;
8998
if (configTarget === ConfigurationTarget.Global) {
9099
const pythonConfig = this.workspaceService.getConfiguration('python');
91-
await pythonConfig.update('defaultInterpreterPath', pythonPath, true);
92-
this._didChangeInterpreterEmitter.fire({ uri: undefined, configTarget });
100+
const globalValue = pythonConfig.inspect<string>('defaultInterpreterPath')!.globalValue;
101+
if (globalValue !== pythonPath) {
102+
await pythonConfig.update('defaultInterpreterPath', pythonPath, true);
103+
this._didChangeInterpreterEmitter.fire({ uri: undefined, configTarget });
104+
}
93105
return;
94106
}
95107
if (!resource) {
@@ -100,12 +112,10 @@ export class InterpreterPathService implements IInterpreterPathService {
100112
settingKey,
101113
undefined
102114
);
103-
await persistentSetting.updateValue(pythonPath);
104-
this._didChangeInterpreterEmitter.fire({ uri: resource, configTarget });
105-
}
106-
107-
public get onDidChange(): Event<InterpreterConfigurationScope> {
108-
return this._didChangeInterpreterEmitter.event;
115+
if (persistentSetting.value !== pythonPath) {
116+
await persistentSetting.updateValue(pythonPath);
117+
this._didChangeInterpreterEmitter.fire({ uri: resource, configTarget });
118+
}
109119
}
110120

111121
public getSettingKey(
@@ -118,10 +128,74 @@ export class InterpreterPathService implements IInterpreterPathService {
118128
settingKey = `WORKSPACE_FOLDER_INTERPRETER_PATH_${folderKey}`;
119129
} else {
120130
settingKey = this.workspaceService.workspaceFile
121-
? `WORKSPACE_INTERPRETER_PATH_${this.workspaceService.workspaceFile.fsPath}`
131+
? `WORKSPACE_INTERPRETER_PATH_${this.fileSystemPaths.normCase(
132+
this.workspaceService.workspaceFile.fsPath
133+
)}`
122134
: // Only a single folder is opened, use fsPath of the folder as key
123135
`WORKSPACE_FOLDER_INTERPRETER_PATH_${folderKey}`;
124136
}
125137
return settingKey;
126138
}
139+
140+
public async copyOldInterpreterStorageValuesToNew(resource: Resource): Promise<void> {
141+
resource = PythonSettings.getSettingsUriAndTarget(resource, this.workspaceService).uri;
142+
const oldSettings = this.workspaceService.getConfiguration('python', resource).inspect<string>('pythonPath')!;
143+
await Promise.all([
144+
this._copyWorkspaceFolderValueToNewStorage(resource, oldSettings.workspaceFolderValue),
145+
this._copyWorkspaceValueToNewStorage(resource, oldSettings.workspaceValue),
146+
this._moveGlobalSettingValueToNewStorage(oldSettings.globalValue)
147+
]);
148+
}
149+
150+
public async _copyWorkspaceFolderValueToNewStorage(resource: Resource, value: string | undefined): Promise<void> {
151+
// Copy workspace folder setting into the new storage if it hasn't been copied already
152+
const workspaceFolderKey = this.workspaceService.getWorkspaceFolderIdentifier(resource);
153+
const flaggedWorkspaceFolderKeysStorage = this.persistentStateFactory.createGlobalPersistentState<string[]>(
154+
workspaceFolderKeysForWhichTheCopyIsDone_Key,
155+
[]
156+
);
157+
const flaggedWorkspaceFolderKeys = flaggedWorkspaceFolderKeysStorage.value;
158+
const shouldUpdateWorkspaceFolderSetting = !flaggedWorkspaceFolderKeys.includes(workspaceFolderKey);
159+
if (shouldUpdateWorkspaceFolderSetting) {
160+
await this.update(resource, ConfigurationTarget.WorkspaceFolder, value);
161+
await flaggedWorkspaceFolderKeysStorage.updateValue([workspaceFolderKey, ...flaggedWorkspaceFolderKeys]);
162+
}
163+
}
164+
165+
public async _copyWorkspaceValueToNewStorage(resource: Resource, value: string | undefined): Promise<void> {
166+
// Copy workspace setting into the new storage if it hasn't been copied already
167+
const workspaceKey = this.workspaceService.workspaceFile
168+
? this.fileSystemPaths.normCase(this.workspaceService.workspaceFile.fsPath)
169+
: undefined;
170+
if (!workspaceKey) {
171+
return;
172+
}
173+
const flaggedWorkspaceKeysStorage = this.persistentStateFactory.createGlobalPersistentState<string[]>(
174+
workspaceKeysForWhichTheCopyIsDone_Key,
175+
[]
176+
);
177+
const flaggedWorkspaceKeys = flaggedWorkspaceKeysStorage.value;
178+
const shouldUpdateWorkspaceSetting = !flaggedWorkspaceKeys.includes(workspaceKey);
179+
if (shouldUpdateWorkspaceSetting) {
180+
await this.update(resource, ConfigurationTarget.Workspace, value);
181+
await flaggedWorkspaceKeysStorage.updateValue([workspaceKey, ...flaggedWorkspaceKeys]);
182+
}
183+
}
184+
185+
public async _moveGlobalSettingValueToNewStorage(value: string | undefined) {
186+
// Move global setting into the new storage if it hasn't been moved already
187+
const isGlobalSettingCopiedStorage = this.persistentStateFactory.createGlobalPersistentState<boolean>(
188+
isGlobalSettingCopiedKey,
189+
false
190+
);
191+
const shouldUpdateGlobalSetting = !isGlobalSettingCopiedStorage.value;
192+
if (shouldUpdateGlobalSetting) {
193+
await this.update(undefined, ConfigurationTarget.Global, value);
194+
// Make sure to delete the original setting after copying it
195+
await this.workspaceService
196+
.getConfiguration('python')
197+
.update('pythonPath', undefined, ConfigurationTarget.Global);
198+
await isGlobalSettingCopiedStorage.updateValue(true);
199+
}
200+
}
127201
}

src/client/common/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -638,4 +638,5 @@ export interface IInterpreterPathService {
638638
get(resource: Resource): string;
639639
inspect(resource: Resource): InspectInterpreterSettingType;
640640
update(resource: Resource, configTarget: ConfigurationTarget, value: string | undefined): Promise<void>;
641+
copyOldInterpreterStorageValuesToNew(resource: Uri | undefined): Promise<void>;
641642
}

src/client/interpreter/autoSelection/constants.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,5 @@
66
export const unsafeInterpreterPromptKey = 'unsafeInterpreterPromptKey';
77
export const unsafeInterpretersKey = 'unsafeInterpretersKey';
88
export const safeInterpretersKey = 'safeInterpretersKey';
9-
export const flaggedWorkspacesKeysStorageKey = 'flaggedWorkspacesKeysStorageKey';
9+
export const flaggedWorkspacesKeysStorageKey = 'flaggedWorkspacesKeysInterpreterSecurityStorageKey';
1010
export const learnMoreOnInterpreterSecurityURI = 'https://aka.ms/AA7jfor';

0 commit comments

Comments
 (0)