Cherry pick dont automatically inject PYTHONSTARTUP (#24346) (#24386)

anthonykim1 · web-flow · commit 276939065078 · 2024-11-06T09:30:54.000-08:00
Resolves #24345 and #24290 and #24105 remove automatic injection from before so only thing that allows shell integration to user for Python terminal REPL is the setting itself.
diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts
@@ -2,16 +2,14 @@
 // Licensed under the MIT License.
 
 import { inject, injectable } from 'inversify';
-import * as path from 'path';
-import { CancellationToken, Disposable, Event, EventEmitter, Terminal } from 'vscode';
+import { CancellationToken, Disposable, Event, EventEmitter, Terminal, TerminalShellExecution } from 'vscode';
 import '../../common/extensions';
 import { IInterpreterService } from '../../interpreter/contracts';
 import { IServiceContainer } from '../../ioc/types';
 import { captureTelemetry } from '../../telemetry';
 import { EventName } from '../../telemetry/constants';
 import { ITerminalAutoActivation } from '../../terminals/types';
 import { ITerminalManager } from '../application/types';
-import { EXTENSION_ROOT_DIR } from '../constants';
 import { _SCRIPTS_DIR } from '../process/internal/scripts/constants';
 import { IConfigurationService, IDisposableRegistry } from '../types';
 import {
@@ -20,7 +18,6 @@ import {
     ITerminalService,
     TerminalCreationOptions,
     TerminalShellType,
-    ITerminalExecutedCommand,
 } from './types';
 import { traceVerbose } from '../../logging';
 
@@ -33,11 +30,12 @@ export class TerminalService implements ITerminalService, Disposable {
     private terminalHelper: ITerminalHelper;
     private terminalActivator: ITerminalActivator;
     private terminalAutoActivator: ITerminalAutoActivation;
-    private readonly envVarScript = path.join(EXTENSION_ROOT_DIR, 'python_files', 'pythonrc.py');
     private readonly executeCommandListeners: Set<Disposable> = new Set();
+    private _terminalFirstLaunched: boolean = true;
     public get onDidCloseTerminal(): Event<void> {
         return this.terminalClosed.event.bind(this.terminalClosed);
     }
+
     constructor(
         @inject(IServiceContainer) private serviceContainer: IServiceContainer,
         private readonly options?: TerminalCreationOptions,
@@ -76,24 +74,24 @@ export class TerminalService implements ITerminalService, Disposable {
         }
         this.terminal!.sendText(text);
     }
-    public async executeCommand(commandLine: string): Promise<ITerminalExecutedCommand | undefined> {
+    public async executeCommand(commandLine: string): Promise<TerminalShellExecution | undefined> {
         const terminal = this.terminal!;
         if (!this.options?.hideFromUser) {
             terminal.show(true);
         }
 
         // If terminal was just launched, wait some time for shell integration to onDidChangeShellIntegration.
-        if (!terminal.shellIntegration) {
+        if (!terminal.shellIntegration && this._terminalFirstLaunched) {
+            this._terminalFirstLaunched = false;
             const promise = new Promise<boolean>((resolve) => {
-                const shellIntegrationChangeEventListener = this.terminalManager.onDidChangeTerminalShellIntegration(
-                    () => {
-                        this.executeCommandListeners.delete(shellIntegrationChangeEventListener);
-                        resolve(true);
-                    },
-                );
+                const disposable = this.terminalManager.onDidChangeTerminalShellIntegration(() => {
+                    clearTimeout(timer);
+                    disposable.dispose();
+                    resolve(true);
+                });
                 const TIMEOUT_DURATION = 500;
-                setTimeout(() => {
-                    this.executeCommandListeners.add(shellIntegrationChangeEventListener);
+                const timer = setTimeout(() => {
+                    disposable.dispose();
                     resolve(true);
                 }, TIMEOUT_DURATION);
             });
@@ -102,18 +100,8 @@ export class TerminalService implements ITerminalService, Disposable {
 
         if (terminal.shellIntegration) {
             const execution = terminal.shellIntegration.executeCommand(commandLine);
-            return await new Promise((resolve) => {
-                const listener = this.terminalManager.onDidEndTerminalShellExecution((e) => {
-                    if (e.execution === execution) {
-                        this.executeCommandListeners.delete(listener);
-                        resolve({ execution, exitCode: e.exitCode });
-                    }
-                });
-                if (listener) {
-                    this.executeCommandListeners.add(listener);
-                }
-                traceVerbose(`Shell Integration is enabled, executeCommand: ${commandLine}`);
-            });
+            traceVerbose(`Shell Integration is enabled, executeCommand: ${commandLine}`);
+            return execution;
         } else {
             terminal.sendText(commandLine);
             traceVerbose(`Shell Integration is disabled, sendText: ${commandLine}`);
@@ -136,7 +124,6 @@ export class TerminalService implements ITerminalService, Disposable {
         this.terminalShellType = this.terminalHelper.identifyTerminalShell(this.terminal);
         this.terminal = this.terminalManager.createTerminal({
             name: this.options?.title || 'Python',
-            env: { PYTHONSTARTUP: this.envVarScript },
             hideFromUser: this.options?.hideFromUser,
         });
         this.terminalAutoActivator.disableAutoActivation(this.terminal);
diff --git a/src/client/common/terminal/syncTerminalService.ts b/src/client/common/terminal/syncTerminalService.ts
@@ -4,7 +4,7 @@
 'use strict';
 
 import { inject } from 'inversify';
-import { CancellationToken, Disposable, Event } from 'vscode';
+import { CancellationToken, Disposable, Event, TerminalShellExecution } from 'vscode';
 import { IInterpreterService } from '../../interpreter/contracts';
 import { traceVerbose } from '../../logging';
 import { PythonEnvironment } from '../../pythonEnvironments/info';
@@ -14,7 +14,7 @@ import * as internalScripts from '../process/internal/scripts';
 import { createDeferred, Deferred } from '../utils/async';
 import { noop } from '../utils/misc';
 import { TerminalService } from './service';
-import { ITerminalService, ITerminalExecutedCommand } from './types';
+import { ITerminalService } from './types';
 
 enum State {
     notStarted = 0,
@@ -145,7 +145,7 @@ export class SynchronousTerminalService implements ITerminalService, Disposable
     public sendText(text: string): Promise<void> {
         return this.terminalService.sendText(text);
     }
-    public executeCommand(commandLine: string): Promise<ITerminalExecutedCommand | undefined> {
+    public executeCommand(commandLine: string): Promise<TerminalShellExecution | undefined> {
         return this.terminalService.executeCommand(commandLine);
     }
     public show(preserveFocus?: boolean | undefined): Promise<void> {
diff --git a/src/client/common/terminal/types.ts b/src/client/common/terminal/types.ts
@@ -54,15 +54,10 @@ export interface ITerminalService extends IDisposable {
     ): Promise<void>;
     /** @deprecated */
     sendText(text: string): Promise<void>;
-    executeCommand(commandLine: string): Promise<ITerminalExecutedCommand | undefined>;
+    executeCommand(commandLine: string): Promise<TerminalShellExecution | undefined>;
     show(preserveFocus?: boolean): Promise<void>;
 }
 
-export interface ITerminalExecutedCommand {
-    execution: TerminalShellExecution;
-    exitCode: number | undefined;
-}
-
 export const ITerminalServiceFactory = Symbol('ITerminalServiceFactory');
 
 export type TerminalCreationOptions = {
diff --git a/src/test/common/terminals/helper.unit.test.ts b/src/test/common/terminals/helper.unit.test.ts
@@ -96,6 +96,30 @@ suite('Terminal Service helpers', () => {
     teardown(() => shellDetectorIdentifyTerminalShell.restore());
     suite('Misc', () => {
         setup(doSetup);
+        test('Creating terminal should not automatically contain PYTHONSTARTUP', () => {
+            const theTitle = 'Hello';
+            const terminal = 'Terminal Created';
+            when(terminalManager.createTerminal(anything())).thenReturn(terminal as any);
+            const term = helper.createTerminal(theTitle);
+            const args = capture(terminalManager.createTerminal).first()[0];
+            expect(term).to.be.deep.equal(terminal);
+            const terminalOptions = args.env;
+            const safeTerminalOptions = terminalOptions || {};
+            expect(safeTerminalOptions).to.not.have.property('PYTHONSTARTUP');
+        });
+
+        test('Env should be undefined if not explicitly passed in ', () => {
+            const theTitle = 'Hello';
+            const terminal = 'Terminal Created';
+            when(terminalManager.createTerminal(anything())).thenReturn(terminal as any);
+
+            const term = helper.createTerminal(theTitle);
+
+            verify(terminalManager.createTerminal(anything())).once();
+            const args = capture(terminalManager.createTerminal).first()[0];
+            expect(term).to.be.deep.equal(terminal);
+            expect(args.env).to.be.deep.equal(undefined);
+        });
 
         test('Create terminal without a title', () => {
             const terminal = 'Terminal Created';
diff --git a/src/test/smoke/smartSend.smoke.test.ts b/src/test/smoke/smartSend.smoke.test.ts
@@ -6,81 +6,78 @@ import { EXTENSION_ROOT_DIR_FOR_TESTS, IS_SMOKE_TEST } from '../constants';
 import { closeActiveWindows, initialize, initializeTest } from '../initialize';
 import { openFile, waitForCondition } from '../common';
 
-// TODO: This test is being flaky for windows, need to investigate why only fails on windows
-if (process.platform !== 'win32') {
-    suite('Smoke Test: Run Smart Selection and Advance Cursor', () => {
-        suiteSetup(async function () {
-            if (!IS_SMOKE_TEST) {
-                return this.skip();
-            }
-            await initialize();
-            return undefined;
-        });
+suite('Smoke Test: Run Smart Selection and Advance Cursor', async () => {
+    suiteSetup(async function () {
+        if (!IS_SMOKE_TEST) {
+            return this.skip();
+        }
+        await initialize();
+        return undefined;
+    });
 
-        setup(initializeTest);
-        suiteTeardown(closeActiveWindows);
-        teardown(closeActiveWindows);
+    setup(initializeTest);
+    suiteTeardown(closeActiveWindows);
+    teardown(closeActiveWindows);
 
-        test('Smart Send', async () => {
-            const file = path.join(
-                EXTENSION_ROOT_DIR_FOR_TESTS,
-                'src',
-                'testMultiRootWkspc',
-                'smokeTests',
-                'create_delete_file.py',
-            );
-            const outputFile = path.join(
-                EXTENSION_ROOT_DIR_FOR_TESTS,
-                'src',
-                'testMultiRootWkspc',
-                'smokeTests',
-                'smart_send_smoke.txt',
-            );
+    test('Smart Send', async () => {
+        const file = path.join(
+            EXTENSION_ROOT_DIR_FOR_TESTS,
+            'src',
+            'testMultiRootWkspc',
+            'smokeTests',
+            'create_delete_file.py',
+        );
+        const outputFile = path.join(
+            EXTENSION_ROOT_DIR_FOR_TESTS,
+            'src',
+            'testMultiRootWkspc',
+            'smokeTests',
+            'smart_send_smoke.txt',
+        );
 
-            await fs.remove(outputFile);
+        await fs.remove(outputFile);
 
-            const textDocument = await openFile(file);
+        const textDocument = await openFile(file);
 
-            if (vscode.window.activeTextEditor) {
-                const myPos = new vscode.Position(0, 0);
-                vscode.window.activeTextEditor!.selections = [new vscode.Selection(myPos, myPos)];
-            }
-            await vscode.commands
-                .executeCommand<void>('python.execSelectionInTerminal', textDocument.uri)
-                .then(undefined, (err) => {
-                    assert.fail(`Something went wrong running the Python file in the terminal: ${err}`);
-                });
+        if (vscode.window.activeTextEditor) {
+            const myPos = new vscode.Position(0, 0);
+            vscode.window.activeTextEditor!.selections = [new vscode.Selection(myPos, myPos)];
+        }
+        await vscode.commands
+            .executeCommand<void>('python.execSelectionInTerminal', textDocument.uri)
+            .then(undefined, (err) => {
+                assert.fail(`Something went wrong running the Python file in the terminal: ${err}`);
+            });
 
-            const checkIfFileHasBeenCreated = () => fs.pathExists(outputFile);
-            await waitForCondition(checkIfFileHasBeenCreated, 10_000, `"${outputFile}" file not created`);
+        const checkIfFileHasBeenCreated = () => fs.pathExists(outputFile);
+        await waitForCondition(checkIfFileHasBeenCreated, 20_000, `"${outputFile}" file not created`);
 
-            await vscode.commands
-                .executeCommand<void>('python.execSelectionInTerminal', textDocument.uri)
-                .then(undefined, (err) => {
-                    assert.fail(`Something went wrong running the Python file in the terminal: ${err}`);
-                });
-            await vscode.commands
-                .executeCommand<void>('python.execSelectionInTerminal', textDocument.uri)
-                .then(undefined, (err) => {
-                    assert.fail(`Something went wrong running the Python file in the terminal: ${err}`);
-                });
+        await vscode.commands
+            .executeCommand<void>('python.execSelectionInTerminal', textDocument.uri)
+            .then(undefined, (err) => {
+                assert.fail(`Something went wrong running the Python file in the terminal: ${err}`);
+            });
+        await vscode.commands
+            .executeCommand<void>('python.execSelectionInTerminal', textDocument.uri)
+            .then(undefined, (err) => {
+                assert.fail(`Something went wrong running the Python file in the terminal: ${err}`);
+            });
 
-            async function wait() {
-                return new Promise<void>((resolve) => {
-                    setTimeout(() => {
-                        resolve();
-                    }, 10000);
-                });
-            }
+        async function wait() {
+            return new Promise<void>((resolve) => {
+                setTimeout(() => {
+                    resolve();
+                }, 10000);
+            });
+        }
 
-            await wait();
+        await wait();
 
-            const deletedFile = !(await fs.pathExists(outputFile));
-            if (deletedFile) {
-                assert.ok(true, `"${outputFile}" file has been deleted`);
-            } else {
-                assert.fail(`"${outputFile}" file still exists`);
-            }
-        });
+        const deletedFile = !(await fs.pathExists(outputFile));
+        if (deletedFile) {
+            assert.ok(true, `"${outputFile}" file has been deleted`);
+        } else {
+            assert.fail(`"${outputFile}" file still exists`);
+        }
     });
-}
+});
diff --git a/src/test/smokeTest.ts b/src/test/smokeTest.ts
@@ -5,7 +5,6 @@
 
 // Must always be on top to setup expected env.
 process.env.VSC_PYTHON_SMOKE_TEST = '1';
-
 import { spawn } from 'child_process';
 import * as fs from '../client/common/platform/fs-paths';
 import * as glob from 'glob';
