Deprecate PythonPath - Part 1 (#11011)

* Modified `Select interpreter` command & add a reset interpreter command (#10373)

* Modified `Select interpreter` command to support setting interpreter at workspace level

* Added a command to reset value of Python interpreter

* Code reviews

* Update src/client/interpreter/configuration/interpreterSelector.ts

Co-Authored-By: Karthik Nadig <kanadig@microsoft.com>

Co-authored-by: Karthik Nadig <kanadig@microsoft.com>

* Implemented new interpreter storage supporting multiroot workspaces (#10325)

* Added python.defaultInterpreterPath setting

* Created persistent storage infrastructure

* Added vscode.workspace.workspaceFile API

* Implemented inspect interpreter path

* Implement config service updater for pythonPath

* Correct build errors

* Change extension to use new infrastructure

* Store service container in a global variable

* Update settings in the new way src/test/common

* Correct cyclic dependency between IExperimentsManager and IConfigurationService

* Detect change in new interpreter storage and act accordingly

* Added reset python interpreters command

* Cache the auto selected interpreter

* Log python interpreter in the output channel

* Fix autoselection and speed up the interpreter change process

* Reset experiments

* Reset python interpreters for workspace

* Remove reset interpreter command from this PR

* Added news entry

* Update getWorkspaceFolderIdentifier

* Code reviews

* Dispose handler

* More code reviews

* Resolve merge conflicts

* Fix issues

* Some more fixes

* More fixes

* Do not assume ACTIVATED_SERVICE_CONTAINER is defined

* Fix running tests

* Fix cacheUtils.ts and test/common.ts

* Correct cacheResourceSpecificInterpreterData decorator

* Added tests for decorator-like implementation in enviroment provider

* Code reviews

* Fix interpreter service unit tests

* Fix interpreter display unit tests

* Fix mac interpreter unit tests

* Fix interpreter selector unit tests

* Fix configSettings pythonPath unit tests

* Fix configSettings unit tests

* Rebase with master

* Fix installer.test.ts tests

* Fix moduleInstaller.test.ts tests

* Fix pythonPathUpdater.test.ts tests

* Fix pythonProc.simple.multiroot.test.ts tests

* Fix data science functional tests

* Fix smoke tests

* Use user friendly path in the output channel

* Use symbols directly

* Fix bug with configSettings.ts

* Fix venv tests

* Run all tests with the new interpreter storage"

* Restore YAML pipeline

* Fix absolute path resolver

* Run all tests with old interpreter storage

* Reduce run time of venv tests

* Run only mac multiroot

* Remove experiment from experiments.json

* Added tests for Interpreter path service

* saa

* Ensure we use `CI_PYTHON_PATH` when running tests with the new interpreter storage

* Reset global interpreter path as well

* Added tests for mac interpreter diagnostic

* Added tests for src\client\interpreter\autoSelection\rules\settings.ts

* Added tests for src\client\interpreter\configuration\pythonPathUpdaterServiceFactory.ts

* Added tests for src\client\interpreter\autoSelection\rules\workspaceEnv.ts

* Added tests for src\client\interpreter\interpreterService.ts

* Added tests for src\client\common\configuration\service.ts

* Added tests for src\client\startupTelemetry.ts

* Correct tests

* Module installer tests

* Code reviews

* More code reviews

* Fix lint error

* Prompt when an "unsafe" workspace python environment is to be autoselected  (#10430)

* Implement prompt when an unsafe workspace python environment is to be autoselected

* Code reviews

* Refactor pythonPath stuff into a private method

* Use variable instead of hardcoding 'python' string

* Added tests for src\client\interpreter\autoSelection\interpreterSecurity\interpreterSecurityCommands.ts

* Don't register command in the constructor

* Refactored code

* Updated tests

* Code reviews

* Moved bannerLabelYes/No to Common() namespace

* Update content for news entry

* Corrected activation manager tests

* Fix single workspace CI tests

* Fix multiroot tests + Code reviews

* Rename Interpreter Security clear command

* Fix tests

* Added intellisense for pythonpath experiment

* Fix multiroot tests

* Rename `Reset interpreter` command to `Clear workspace interpreter setting

* Fix bug with interpreter security storage

* Correct prettier errors

* Edit news entries appropriately

* Git rebase

* Code reviews

Co-authored-by: Karthik Nadig <kanadig@microsoft.com>

experiments.json

@@ -160,6 +160,17 @@
         "salt": "EnableIPyWidgets",
         "min": 0,
         "max": 0
+    },
+    {
+        "name": "DeprecatePythonPath - experiment",
+        "salt": "DeprecatePythonPath",
+        "min": 0,
+        "max": 0
+    },
+    {
+        "name": "DeprecatePythonPath - control",
+        "salt": "DeprecatePythonPath",
+        "min": 100,
+        "max": 100
     }
-
 ]

news/1 Enhancements/10325.md

@@ -0,0 +1 @@
+Use new interpreter storage supporting multiroot workspaces when in Deprecate PythonPath experiment.

news/1 Enhancements/10372.md

@@ -0,0 +1 @@
+Modified `Select interpreter` command to support setting interpreter at workspace level.

news/1 Enhancements/10374.md

@@ -0,0 +1 @@
+Added a command `Clear Workspace Interpreter Setting` to clear value of Python interpreter from workspace settings.

news/1 Enhancements/10879.md

@@ -0,0 +1 @@
+Prompt when an "untrusted" workspace Python environment is to be auto selected when in Deprecate PythonPath experiment.

news/1 Enhancements/10912.md

@@ -0,0 +1 @@
+Added a command `Reset stored info for untrusted Interpreters` to reset "untrusted" interpreters storage when in Deprecate PythonPath experiment.

news/1 Enhancements/11021.md

@@ -0,0 +1 @@
+Added a user setting `python.defaultInterpreterPath` to set up the default interpreter path when in Deprecate PythonPath experiment.

package.json

@@ -74,6 +74,8 @@
         "onCommand:python.switchOffInsidersChannel",
         "onCommand:python.switchToDailyChannel",
         "onCommand:python.switchToWeeklyChannel",
+        "onCommand:python.clearWorkspaceInterpreter",
+        "onCommand:python.resetInterpreterSecurityStorage",
         "onCommand:python.datascience.createnewnotebook",
         "onCommand:python.datascience.showhistorypane",
         "onCommand:python.datascience.importnotebook",
@@ -224,6 +226,16 @@
                 "title": "%python.command.python.switchToWeeklyChannel.title%",
                 "category": "Python"
             },
+            {
+                "command": "python.clearWorkspaceInterpreter",
+                "title": "%python.command.python.clearWorkspaceInterpreter.title%",
+                "category": "Python"
+            },
+            {
+                "command": "python.resetInterpreterSecurityStorage",
+                "title": "%python.command.python.resetInterpreterSecurityStorage.title%",
+                "category": "Python"
+            },
             {
                 "command": "python.refactorExtractVariable",
                 "title": "%python.command.python.refactorExtractVariable.title%",
@@ -753,6 +765,16 @@
                     "category": "Python",
                     "when": "config.python.insidersChannel != 'weekly'"
                 },
+                {
+                    "command": "python.clearWorkspaceInterpreter",
+                    "title": "%python.command.python.clearWorkspaceInterpreter.title%",
+                    "category": "Python"
+                },
+                {
+                    "command": "python.resetInterpreterSecurityStorage",
+                    "title": "%python.command.python.resetInterpreterSecurityStorage.title%",
+                    "category": "Python"
+                },
                 {
                     "command": "python.viewOutput",
                     "title": "%python.command.python.viewOutput.title%",
@@ -1554,6 +1576,12 @@
                     "description": "Enables/disables A/B tests.",
                     "scope": "machine"
                 },
+                "python.defaultInterpreterPath": {
+                    "type": "string",
+                    "default": "python",
+                    "description": "Path to Python, you can use a custom version of Python by modifying this setting to include the full path.",
+                    "scope": "machine"
+                },
                 "python.experiments.optInto": {
                     "type": "array",
                     "default": [],
@@ -1571,6 +1599,7 @@
                             "UseTerminalToGetActivatedEnvVars - experiment",
                             "CollectLSRequestTiming - experiment",
                             "CollectNodeLSRequestTiming - experiment",
+                            "DeprecatePythonPath - experiment",
                             "All"
                         ]
                     },
@@ -1594,6 +1623,7 @@
                             "UseTerminalToGetActivatedEnvVars - experiment",
                             "CollectLSRequestTiming - experiment",
                             "CollectNodeLSRequestTiming - experiment",
+                            "DeprecatePythonPath - experiment",
                             "All"
                         ]
                     },

package.nls.json

@@ -10,6 +10,8 @@
     "python.command.python.switchOffInsidersChannel.title": "Switch to Default Channel",
     "python.command.python.switchToDailyChannel.title": "Switch to Insiders Daily Channel",
     "python.command.python.switchToWeeklyChannel.title": "Switch to Insiders Weekly Channel",
+    "python.command.python.clearWorkspaceInterpreter.title": "Clear Workspace Interpreter Setting",
+    "python.command.python.resetInterpreterSecurityStorage.title": "Reset Stored Info for Untrusted Interpreters",
     "python.command.python.refactorExtractVariable.title": "Extract Variable",
     "python.command.python.refactorExtractMethod.title": "Extract Method",
     "python.command.python.viewOutput.title": "Show Output",
@@ -131,14 +133,20 @@
     "DataScience.connectingToJupyter": "Connecting to Jupyter server",
     "Experiments.inGroup": "User belongs to experiment group '{0}'",
     "Interpreters.RefreshingInterpreters": "Refreshing Python Interpreters",
+    "Interpreters.entireWorkspace": "Entire workspace",
+    "Interpreters.pythonInterpreterPath": "Python interpreter path: {0}",
     "Interpreters.LoadingInterpreters": "Loading Python Interpreters",
+    "Interpreters.unsafeInterpreterMessage": "We found a Python environment in this workspace. Do you want to select it to start up the features in the Python extension? Only accept if you trust this environment.",
     "Interpreters.condaInheritEnvMessage": "We noticed you're using a conda environment. If you are experiencing issues with this environment in the integrated terminal, we recommend that you let the Python extension change \"terminal.integrated.inheritEnv\" to false in your user settings.",
     "Logging.CurrentWorkingDirectory": "cwd:",
+    "Common.bannerLabelYes": "Yes",
+    "Common.bannerLabelNo": "No",
     "Common.doNotShowAgain": "Do not show again",
     "Common.reload": "Reload",
     "Common.moreInfo": "More Info",
     "Common.and": "and",
     "Common.install": "Install",
+    "Common.learnMore": "Learn more",
     "OutputChannelNames.languageServer": "Python Language Server",
     "OutputChannelNames.python": "Python",
     "OutputChannelNames.pythonTest": "Python Test Log",
@@ -166,8 +174,6 @@
     "DataScienceSurveyBanner.bannerLabelYes": "Yes, take survey now",
     "DataScienceSurveyBanner.bannerLabelNo": "No, thanks",
     "InteractiveShiftEnterBanner.bannerMessage": "Would you like to run code in the 'Python Interactive' window (an IPython console) for 'shift-enter'? Select 'No' to continue to run code in the Python Terminal. This can be changed later in settings.",
-    "InteractiveShiftEnterBanner.bannerLabelYes": "Yes",
-    "InteractiveShiftEnterBanner.bannerLabelNo": "No",
     "DataScience.restartingKernelStatus": "Restarting IPython Kernel",
     "DataScience.executingCode": "Executing Cell",
     "DataScience.collapseAll": "Collapse all cell inputs",

src/client/activation/activationManager.ts

@@ -7,18 +7,21 @@ import { inject, injectable, multiInject } from 'inversify';
 import { TextDocument } from 'vscode';
 import { IApplicationDiagnostics } from '../application/types';
 import { IActiveResourceService, IDocumentManager, IWorkspaceService } from '../common/application/types';
-import { PYTHON_LANGUAGE } from '../common/constants';
+import { DEFAULT_INTERPRETER_SETTING, PYTHON_LANGUAGE } from '../common/constants';
+import { DeprecatePythonPath } from '../common/experimentGroups';
 import { traceDecorators } from '../common/logger';
 import { IFileSystem } from '../common/platform/types';
-import { IDisposable, Resource } from '../common/types';
-import { IInterpreterAutoSelectionService } from '../interpreter/autoSelection/types';
+import { IDisposable, IExperimentsManager, IInterpreterPathService, Resource } from '../common/types';
+import { createDeferred, Deferred } from '../common/utils/async';
+import { IInterpreterAutoSelectionService, IInterpreterSecurityService } from '../interpreter/autoSelection/types';
 import { IInterpreterService } from '../interpreter/contracts';
 import { sendActivationTelemetry } from '../telemetry/envFileTelemetry';
 import { IExtensionActivationManager, IExtensionActivationService, IExtensionSingleActivationService } from './types';
 
 @injectable()
 export class ExtensionActivationManager implements IExtensionActivationManager {
     public readonly activatedWorkspaces = new Set<string>();
+    protected readonly isInterpreterSetForWorkspacePromises = new Map<string, Deferred<void>>();
     private readonly disposables: IDisposable[] = [];
     private docOpenedHandler?: IDisposable;
     constructor(
@@ -31,7 +34,10 @@ export class ExtensionActivationManager implements IExtensionActivationManager {
         @inject(IApplicationDiagnostics) private readonly appDiagnostics: IApplicationDiagnostics,
         @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService,
         @inject(IFileSystem) private readonly fileSystem: IFileSystem,
-        @inject(IActiveResourceService) private readonly activeResourceService: IActiveResourceService
+        @inject(IActiveResourceService) private readonly activeResourceService: IActiveResourceService,
+        @inject(IExperimentsManager) private readonly experiments: IExperimentsManager,
+        @inject(IInterpreterPathService) private readonly interpreterPathService: IInterpreterPathService,
+        @inject(IInterpreterSecurityService) private readonly interpreterSecurityService: IInterpreterSecurityService
     ) {}
 
     public dispose() {
@@ -66,6 +72,7 @@ export class ExtensionActivationManager implements IExtensionActivationManager {
         await sendActivationTelemetry(this.fileSystem, this.workspaceService, resource);
 
         await this.autoSelection.autoSelectInterpreter(resource);
+        await this.evaluateAutoSelectedInterpreterSafety(resource);
         await Promise.all(this.activationServices.map((item) => item.activate(resource)));
         await this.appDiagnostics.performPreStartupHealthCheck(resource);
     }
@@ -88,8 +95,38 @@ export class ExtensionActivationManager implements IExtensionActivationManager {
         const folder = this.workspaceService.getWorkspaceFolder(doc.uri);
         this.activateWorkspace(folder ? folder.uri : undefined).ignoreErrors();
     }
+
+    public async evaluateAutoSelectedInterpreterSafety(resource: Resource) {
+        if (this.experiments.inExperiment(DeprecatePythonPath.experiment)) {
+            const workspaceKey = this.getWorkspaceKey(resource);
+            const interpreterSettingValue = this.interpreterPathService.get(resource);
+            if (interpreterSettingValue.length === 0 || interpreterSettingValue === DEFAULT_INTERPRETER_SETTING) {
+                // Setting is not set, extension will use the autoselected value. Make sure it's safe.
+                const interpreter = this.autoSelection.getAutoSelectedInterpreter(resource);
+                if (interpreter) {
+                    const isInterpreterSetForWorkspace = createDeferred<void>();
+                    this.isInterpreterSetForWorkspacePromises.set(workspaceKey, isInterpreterSetForWorkspace);
+                    await Promise.race([
+                        isInterpreterSetForWorkspace.promise,
+                        this.interpreterSecurityService.evaluateAndRecordInterpreterSafety(interpreter, resource)
+                    ]);
+                }
+            } else {
+                // Resolve any concurrent calls waiting on the promise
+                if (this.isInterpreterSetForWorkspacePromises.has(workspaceKey)) {
+                    this.isInterpreterSetForWorkspacePromises.get(workspaceKey)!.resolve();
+                    this.isInterpreterSetForWorkspacePromises.delete(workspaceKey);
+                }
+            }
+        }
+        this.experiments.sendTelemetryIfInExperiment(DeprecatePythonPath.control);
+    }
+
     protected addHandlers() {
         this.disposables.push(this.workspaceService.onDidChangeWorkspaceFolders(this.onWorkspaceFoldersChanged, this));
+        this.disposables.push(
+            this.interpreterPathService.onDidChange((i) => this.evaluateAutoSelectedInterpreterSafety(i.uri))
+        );
     }
     protected addRemoveDocOpenedHandlers() {
         if (this.hasMultipleWorkspaces()) {

src/client/activation/serviceRegistry.ts

@@ -1,7 +1,6 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
-import { ActiveResourceService } from '../common/application/activeResource';
-import { IActiveResourceService } from '../common/application/types';
+
 import { registerTypes as registerDotNetTypes } from '../common/dotnet/serviceRegistry';
 import { INugetRepository } from '../common/nuget/types';
 import {
@@ -211,6 +210,5 @@ export function registerTypes(serviceManager: IServiceManager, languageServerTyp
         IExtensionSingleActivationService,
         ExtensionSurveyPrompt
     );
-    serviceManager.addSingleton<IActiveResourceService>(IActiveResourceService, ActiveResourceService);
     serviceManager.addSingleton<IExtensionSingleActivationService>(IExtensionSingleActivationService, AATesting);
 }

src/client/application/diagnostics/checks/macPythonInterpreter.ts

@@ -6,9 +6,17 @@
 import { inject, injectable } from 'inversify';
 import { ConfigurationChangeEvent, DiagnosticSeverity, Uri } from 'vscode';
 import { IWorkspaceService } from '../../../common/application/types';
+import { DeprecatePythonPath } from '../../../common/experimentGroups';
 import '../../../common/extensions';
 import { IPlatformService } from '../../../common/platform/types';
-import { IConfigurationService, IDisposableRegistry, Resource } from '../../../common/types';
+import {
+    IConfigurationService,
+    IDisposableRegistry,
+    IExperimentsManager,
+    IInterpreterPathService,
+    InterpreterConfigurationScope,
+    Resource
+} from '../../../common/types';
 import { IInterpreterHelper, IInterpreterService, InterpreterType } from '../../../interpreter/contracts';
 import { IServiceContainer } from '../../../ioc/types';
 import { BaseDiagnostic, BaseDiagnosticsService } from '../base';
@@ -133,18 +141,37 @@ export class InvalidMacPythonInterpreterService extends BaseDiagnosticsService {
     protected addPythonPathChangedHandler() {
         const workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
         const disposables = this.serviceContainer.get<IDisposableRegistry>(IDisposableRegistry);
+        const interpreterPathService = this.serviceContainer.get<IInterpreterPathService>(IInterpreterPathService);
+        const experiments = this.serviceContainer.get<IExperimentsManager>(IExperimentsManager);
+        if (experiments.inExperiment(DeprecatePythonPath.experiment)) {
+            disposables.push(interpreterPathService.onDidChange((i) => this.onDidChangeConfiguration(undefined, i)));
+        }
+        experiments.sendTelemetryIfInExperiment(DeprecatePythonPath.control);
         disposables.push(workspaceService.onDidChangeConfiguration(this.onDidChangeConfiguration.bind(this)));
     }
-    protected async onDidChangeConfiguration(event: ConfigurationChangeEvent) {
-        const workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
-        const workspacesUris: (Uri | undefined)[] = workspaceService.hasWorkspaceFolders
-            ? workspaceService.workspaceFolders!.map((workspace) => workspace.uri)
-            : [undefined];
-        const workspaceUriIndex = workspacesUris.findIndex((uri) =>
-            event.affectsConfiguration('python.pythonPath', uri)
-        );
-        if (workspaceUriIndex === -1) {
-            return;
+    protected async onDidChangeConfiguration(
+        event?: ConfigurationChangeEvent,
+        interpreterConfigurationScope?: InterpreterConfigurationScope
+    ) {
+        let workspaceUri: Resource;
+        if (event) {
+            const workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
+            const workspacesUris: (Uri | undefined)[] = workspaceService.hasWorkspaceFolders
+                ? workspaceService.workspaceFolders!.map((workspace) => workspace.uri)
+                : [undefined];
+            const workspaceUriIndex = workspacesUris.findIndex((uri) =>
+                event.affectsConfiguration('python.pythonPath', uri)
+            );
+            if (workspaceUriIndex === -1) {
+                return;
+            }
+            workspaceUri = workspacesUris[workspaceUriIndex];
+        } else if (interpreterConfigurationScope) {
+            workspaceUri = interpreterConfigurationScope.uri;
+        } else {
+            throw new Error(
+                'One of `interpreterConfigurationScope` or `event` should be defined when calling `onDidChangeConfiguration`.'
+            );
         }
         // Lets wait, for more changes, dirty simple throttling.
         if (this.timeOut) {
@@ -154,7 +181,7 @@ export class InvalidMacPythonInterpreterService extends BaseDiagnosticsService {
         }
         this.timeOut = setTimeout(() => {
             this.timeOut = undefined;
-            this.diagnose(workspacesUris[workspaceUriIndex])
+            this.diagnose(workspaceUri)
                 .then((diagnostics) => this.handle(diagnostics))
                 .ignoreErrors();
         }, this.changeThrottleTimeout);

src/client/common/application/commands.ts

@@ -22,6 +22,8 @@ export type CommandsWithoutArgs = keyof ICommandNameWithoutArgumentTypeMapping;
 interface ICommandNameWithoutArgumentTypeMapping {
     [Commands.SwitchToInsidersDaily]: [];
     [Commands.SwitchToInsidersWeekly]: [];
+    [Commands.ClearWorkspaceInterpreter]: [];
+    [Commands.ResetInterpreterSecurityStorage]: [];
     [Commands.SwitchOffInsidersChannel]: [];
     [Commands.Set_Interpreter]: [];
     [Commands.Set_ShebangInterpreter]: [];

src/client/common/application/types.ts

@@ -667,6 +667,37 @@ export interface IWorkspaceService {
      */
     readonly workspaceFolders: readonly WorkspaceFolder[] | undefined;
 
+    /**
+     * The location of the workspace file, for example:
+     *
+     * `file:///Users/name/Development/myProject.code-workspace`
+     *
+     * or
+     *
+     * `untitled:1555503116870`
+     *
+     * for a workspace that is untitled and not yet saved.
+     *
+     * Depending on the workspace that is opened, the value will be:
+     *  * `undefined` when no workspace or  a single folder is opened
+     *  * the path of the workspace file as `Uri` otherwise. if the workspace
+     * is untitled, the returned URI will use the `untitled:` scheme
+     *
+     * The location can e.g. be used with the `vscode.openFolder` command to
+     * open the workspace again after it has been closed.
+     *
+     * **Example:**
+     * ```typescript
+     * vscode.commands.executeCommand('vscode.openFolder', uriOfWorkspace);
+     * ```
+     *
+     * **Note:** it is not advised to use `workspace.workspaceFile` to write
+     * configuration data into the file. You can use `workspace.getConfiguration().update()`
+     * for that purpose which will work both when a single folder is opened as
+     * well as an untitled or saved workspace.
+     */
+    readonly workspaceFile: Resource;
+
     /**
      * An event that is emitted when a workspace folder is added or removed.
      */