Do not assume the Linux distro to be debian if dnf is not found while installing python (microsoft/vscode-python#19582)

* Implement

* Code reviews

Co-authored-by: Raymond Zhao <[email protected]>

Co-authored-by: Raymond Zhao <[email protected]>

Author: 2 people

extensions/positron-python/src/client/common/utils/localize.ts

@@ -277,6 +277,10 @@ export namespace Interpreters {
         'Interpreters.selectInterpreterTip',
         'Tip: you can change the Python interpreter used by the Python extension by clicking on the Python version in the status bar',
     );
+    export const installPythonTerminalMessage = localize(
+        'Interpreters.installPythonTerminalMessage',
+        '💡 Please try installing the python package using your package manager. Alternatively you can also download it from https://www.python.org/downloads',
+    );
 }
 
 export namespace InterpreterQuickPickList {

extensions/positron-python/src/client/interpreter/configuration/interpreterSelector/commands/installPython/installPythonViaTerminal.ts

@@ -9,11 +9,17 @@ import { inject, injectable } from 'inversify';
 import { IExtensionSingleActivationService } from '../../../../../activation/types';
 import { Commands } from '../../../../../common/constants';
 import { IDisposableRegistry } from '../../../../../common/types';
-import { ITerminalServiceFactory } from '../../../../../common/terminal/types';
-import { ICommandManager } from '../../../../../common/application/types';
+import { ICommandManager, ITerminalManager } from '../../../../../common/application/types';
 import { sleep } from '../../../../../common/utils/async';
 import { OSType } from '../../../../../common/utils/platform';
 import { traceVerbose } from '../../../../../logging';
+import { Interpreters } from '../../../../../common/utils/localize';
+
+enum PackageManagers {
+    brew = 'brew',
+    apt = 'apt',
+    dnf = 'dnf',
+}
 
 /**
  * Runs commands listed in walkthrough to install Python.
@@ -22,9 +28,15 @@ import { traceVerbose } from '../../../../../logging';
 export class InstallPythonViaTerminal implements IExtensionSingleActivationService {
     public readonly supportedWorkspaceTypes = { untrustedWorkspace: true, virtualWorkspace: false };
 
+    private readonly packageManagerCommands: Record<PackageManagers, string[]> = {
+        brew: ['brew install python3'],
+        dnf: ['sudo dnf install python3'],
+        apt: ['sudo apt-get update', 'sudo apt-get install python3 python3-venv python3-pip'],
+    };
+
     constructor(
         @inject(ICommandManager) private readonly commandManager: ICommandManager,
-        @inject(ITerminalServiceFactory) private readonly terminalServiceFactory: ITerminalServiceFactory,
+        @inject(ITerminalManager) private readonly terminalManager: ITerminalManager,
         @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry,
     ) {}
 
@@ -42,36 +54,49 @@ export class InstallPythonViaTerminal implements IExtensionSingleActivationServi
     }
 
     public async _installPythonOnUnix(os: OSType.Linux | OSType.OSX): Promise<void> {
-        const terminalService = this.terminalServiceFactory.getTerminalService({});
-        const commands = await getCommands(os);
+        const commands = await this.getCommands(os);
+        const terminal = this.terminalManager.createTerminal({
+            name: 'Python',
+            message: commands.length ? undefined : Interpreters.installPythonTerminalMessage,
+        });
+        terminal.show(true);
+        await waitForTerminalToStartup();
         for (const command of commands) {
-            await terminalService.sendText(command);
+            terminal.sendText(command);
             await waitForCommandToProcess();
         }
     }
-}
 
-async function getCommands(os: OSType.Linux | OSType.OSX) {
-    if (os === OSType.OSX) {
-        return ['brew install python3'];
+    private async getCommands(os: OSType.Linux | OSType.OSX) {
+        if (os === OSType.OSX) {
+            return this.packageManagerCommands[PackageManagers.brew];
+        }
+        return this.getCommandsForLinux();
     }
-    return getCommandsForLinux();
-}
 
-async function getCommandsForLinux() {
-    let isDnfAvailable = false;
-    try {
-        const which = require('which') as typeof whichTypes;
-        const resolvedPath = await which('dnf');
-        traceVerbose('Resolved path to dnf module:', resolvedPath);
-        isDnfAvailable = resolvedPath.trim().length > 0;
-    } catch (ex) {
-        traceVerbose('Dnf not found', ex);
-        isDnfAvailable = false;
+    private async getCommandsForLinux() {
+        for (const packageManager of [PackageManagers.apt, PackageManagers.dnf]) {
+            let isPackageAvailable = false;
+            try {
+                const which = require('which') as typeof whichTypes;
+                const resolvedPath = await which(packageManager);
+                traceVerbose(`Resolved path to ${packageManager} module:`, resolvedPath);
+                isPackageAvailable = resolvedPath.trim().length > 0;
+            } catch (ex) {
+                traceVerbose(`${packageManager} not found`, ex);
+                isPackageAvailable = false;
+            }
+            if (isPackageAvailable) {
+                return this.packageManagerCommands[packageManager];
+            }
+        }
+        return [];
     }
-    return isDnfAvailable
-        ? ['sudo dnf install python3']
-        : ['sudo apt-get update', 'sudo apt-get install python3 python3-venv python3-pip'];
+}
+
+async function waitForTerminalToStartup() {
+    // Sometimes the terminal takes some time to start up before it can start accepting input.
+    await sleep(100);
 }
 
 async function waitForCommandToProcess() {

extensions/positron-python/src/test/configuration/interpreterSelector/commands/installPythonViaTerminal.unit.test.ts

@@ -3,27 +3,34 @@
 
 'use strict';
 
+import { expect } from 'chai';
 import rewiremock from 'rewiremock';
 import * as sinon from 'sinon';
 import { anything, instance, mock, verify, when } from 'ts-mockito';
 import * as TypeMoq from 'typemoq';
-import { ICommandManager } from '../../../../client/common/application/types';
+import { ICommandManager, ITerminalManager } from '../../../../client/common/application/types';
 import { Commands } from '../../../../client/common/constants';
-import { ITerminalService, ITerminalServiceFactory } from '../../../../client/common/terminal/types';
+import { ITerminalService } from '../../../../client/common/terminal/types';
 import { IDisposable } from '../../../../client/common/types';
+import { Interpreters } from '../../../../client/common/utils/localize';
 import { InstallPythonViaTerminal } from '../../../../client/interpreter/configuration/interpreterSelector/commands/installPython/installPythonViaTerminal';
 
 suite('Install Python via Terminal', () => {
     let cmdManager: ICommandManager;
-    let terminalServiceFactory: ITerminalServiceFactory;
+    let terminalServiceFactory: ITerminalManager;
     let installPythonCommand: InstallPythonViaTerminal;
     let terminalService: ITerminalService;
+    let message: string | undefined;
     setup(() => {
         rewiremock.enable();
         cmdManager = mock<ICommandManager>();
-        terminalServiceFactory = mock<ITerminalServiceFactory>();
+        terminalServiceFactory = mock<ITerminalManager>();
         terminalService = mock<ITerminalService>();
-        when(terminalServiceFactory.getTerminalService(anything())).thenReturn(instance(terminalService));
+        message = undefined;
+        when(terminalServiceFactory.createTerminal(anything())).thenCall((options) => {
+            message = options.message;
+            return instance(terminalService);
+        });
         installPythonCommand = new InstallPythonViaTerminal(instance(cmdManager), instance(terminalServiceFactory), []);
     });
 
@@ -32,12 +39,18 @@ suite('Install Python via Terminal', () => {
         sinon.restore();
     });
 
-    test('Sends expected commands when InstallPythonOnLinux command is executed if no dnf is available', async () => {
+    test('Sends expected commands when InstallPythonOnLinux command is executed if apt is available', async () => {
         let installCommandHandler: () => Promise<void>;
         when(cmdManager.registerCommand(Commands.InstallPythonOnLinux, anything())).thenCall((_, cb) => {
             installCommandHandler = cb;
             return TypeMoq.Mock.ofType<IDisposable>().object;
         });
+        rewiremock('which').with((cmd: string) => {
+            if (cmd === 'apt') {
+                return 'path/to/apt';
+            }
+            throw new Error('Command not found');
+        });
         await installPythonCommand.activate();
         when(terminalService.sendText('sudo apt-get update')).thenResolve();
         when(terminalService.sendText('sudo apt-get install python3 python3-venv python3-pip')).thenResolve();
@@ -67,6 +80,23 @@ suite('Install Python via Terminal', () => {
         await installCommandHandler!();
 
         verify(terminalService.sendText('sudo dnf install python3')).once();
+        expect(message).to.be.equal(undefined);
+    });
+
+    test('Creates terminal with appropriate message when InstallPythonOnLinux command is executed if no known linux package managers are available', async () => {
+        let installCommandHandler: () => Promise<void>;
+        when(cmdManager.registerCommand(Commands.InstallPythonOnLinux, anything())).thenCall((_, cb) => {
+            installCommandHandler = cb;
+            return TypeMoq.Mock.ofType<IDisposable>().object;
+        });
+        rewiremock('which').with((_cmd: string) => {
+            throw new Error('Command not found');
+        });
+
+        await installPythonCommand.activate();
+        await installCommandHandler!();
+
+        expect(message).to.be.equal(Interpreters.installPythonTerminalMessage);
     });
 
     test('Sends expected commands on Mac when InstallPythonOnMac command is executed if no dnf is available', async () => {
@@ -81,5 +111,6 @@ suite('Install Python via Terminal', () => {
         await installCommandHandler!();
 
         verify(terminalService.sendText('brew install python3')).once();
+        expect(message).to.be.equal(undefined);
     });
 });