Skip to content

Commit d2a3b43

Browse files
author
Mikhail Arkhipov
authored
Remove Homebrew installation (#537)
* Basic tokenizer * Fixed property names * Tests, round I * Tests, round II * tokenizer test * Remove temorary change * Fix merge issue * Merge conflict * Merge conflict * Completion test * Fix last line * Fix javascript math * Make test await for results * Add license headers * Rename definitions to types * License headers * Fix typo in completion details (typo) * Fix hover test * Russian translations * Update to better translation * Fix typo * #70 How to get all parameter info when filling in a function param list * Fix #70 How to get all parameter info when filling in a function param list * Clean up * Clean imports * CR feedback * Trim whitespace for test stability * More tests * Better handle no-parameters documentation * Better handle ellipsis and Python3 * Basic services * Install check * Output installer messages * Warn default Mac OS interpreter * Remove test change * Add tests * PR feedback * CR feedback * Mock process instead * Fix Brew detection * Update test * Elevated module install * Fix path check * Add check suppression option & suppress vor VE by default * Fix most linter tests * Merge conflict * Per-user install * Handle VE/Conda * Fix tests * Remove Homebrew * Fix OS name
1 parent 321e204 commit d2a3b43

File tree

2 files changed

+13
-187
lines changed

2 files changed

+13
-187
lines changed

src/client/common/installer/pythonInstallation.ts

Lines changed: 10 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -2,116 +2,38 @@
22
// Licensed under the MIT License.
33
'use strict';
44

5-
import { OutputChannel } from 'vscode';
65
import { IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE, InterpreterType } from '../../interpreter/contracts';
76
import { IServiceContainer } from '../../ioc/types';
87
import { IApplicationShell } from '../application/types';
98
import { IPythonSettings } from '../configSettings';
10-
import { STANDARD_OUTPUT_CHANNEL } from '../constants';
11-
import { IFileSystem, IPlatformService } from '../platform/types';
12-
import { IProcessService } from '../process/types';
13-
import { IOutputChannel } from '../types';
9+
import { IPlatformService } from '../platform/types';
1410

1511
export class PythonInstaller {
1612
private locator: IInterpreterLocatorService;
17-
private process: IProcessService;
18-
private fs: IFileSystem;
19-
private outputChannel: OutputChannel;
20-
private _platform: IPlatformService;
21-
private _shell: IApplicationShell;
13+
private shell: IApplicationShell;
2214

2315
constructor(private serviceContainer: IServiceContainer) {
2416
this.locator = serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE);
17+
this.shell = serviceContainer.get<IApplicationShell>(IApplicationShell);
2518
}
2619

2720
public async checkPythonInstallation(settings: IPythonSettings): Promise<boolean> {
2821
if (settings.disableInstallationChecks === true) {
2922
return true;
3023
}
31-
let interpreters = await this.locator.getInterpreters();
24+
const interpreters = await this.locator.getInterpreters();
3225
if (interpreters.length > 0) {
33-
if (this.platform.isMac &&
26+
const platform = this.serviceContainer.get<IPlatformService>(IPlatformService);
27+
if (platform.isMac &&
3428
settings.pythonPath === 'python' &&
3529
interpreters[0].type === InterpreterType.Unknown) {
36-
await this.shell.showWarningMessage('Selected interpreter is MacOS system Python which is not recommended. Please select different interpreter');
30+
await this.shell.showWarningMessage('Selected interpreter is macOS system Python which is not recommended. Please select different interpreter');
3731
}
3832
return true;
3933
}
4034

41-
if (!this.platform.isMac) {
42-
// Windows or Linux
43-
await this.shell.showErrorMessage('Python is not installed. Please download and install Python before using the extension.');
44-
this.shell.openUrl('https://www.python.org/downloads');
45-
return false;
46-
}
47-
48-
this.process = this.serviceContainer.get<IProcessService>(IProcessService);
49-
this.fs = this.serviceContainer.get<IFileSystem>(IFileSystem);
50-
this.outputChannel = this.serviceContainer.get<OutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
51-
52-
if (this.platform.isMac) {
53-
if (await this.shell.showErrorMessage('Python that comes with MacOS is not supported. Would you like to install regular Python now?', 'Yes', 'No') === 'Yes') {
54-
const brewInstalled = await this.ensureBrew();
55-
if (!brewInstalled) {
56-
await this.shell.showErrorMessage('Unable to install Homebrew package manager. Try installing it manually.');
57-
this.shell.openUrl('https://brew.sh');
58-
return false;
59-
}
60-
await this.executeAndOutput('brew', ['install', 'python']);
61-
}
62-
}
63-
64-
interpreters = await this.locator.getInterpreters();
65-
return interpreters.length > 0;
66-
}
67-
68-
private isBrewInstalled(): Promise<boolean> {
69-
return this.fs.fileExistsAsync('/usr/local/bin/brew');
70-
}
71-
72-
private async ensureBrew(): Promise<boolean> {
73-
if (await this.isBrewInstalled()) {
74-
return true;
75-
}
76-
const result = await this.executeAndOutput(
77-
'/usr/bin/ruby',
78-
['-e', '"$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"']);
79-
return result && await this.isBrewInstalled();
80-
}
81-
82-
private executeAndOutput(command: string, args: string[]): Promise<boolean> {
83-
let failed = false;
84-
this.outputChannel.show();
85-
86-
const result = this.process.execObservable(command, args, { mergeStdOutErr: true, throwOnStdErr: false });
87-
result.out.subscribe(output => {
88-
this.outputChannel.append(output.out);
89-
}, error => {
90-
failed = true;
91-
this.shell.showErrorMessage(`Unable to execute '${command}', error: ${error}`);
92-
});
93-
94-
return new Promise<boolean>((resolve, reject) => {
95-
if (failed) {
96-
resolve(false);
97-
}
98-
result.proc.on('exit', (code, signal) => {
99-
resolve(!signal);
100-
});
101-
});
102-
}
103-
104-
private get shell(): IApplicationShell {
105-
if (!this._shell) {
106-
this._shell = this.serviceContainer.get<IApplicationShell>(IApplicationShell);
107-
}
108-
return this._shell;
109-
}
110-
111-
private get platform(): IPlatformService {
112-
if (!this._platform) {
113-
this._platform = this.serviceContainer.get<IPlatformService>(IPlatformService);
114-
}
115-
return this._platform;
35+
await this.shell.showErrorMessage('Python is not installed. Please download and install Python before using the extension.');
36+
this.shell.openUrl('https://www.python.org/downloads');
37+
return false;
11638
}
11739
}

src/test/install/pythonInstallation.test.ts

Lines changed: 3 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,7 @@ import { IApplicationShell } from '../../client/common/application/types';
1212
import { IPythonSettings } from '../../client/common/configSettings';
1313
import { STANDARD_OUTPUT_CHANNEL } from '../../client/common/constants';
1414
import { PythonInstaller } from '../../client/common/installer/pythonInstallation';
15-
import { IFileSystem, IPlatformService } from '../../client/common/platform/types';
16-
import { IProcessService, ObservableExecutionResult, Output } from '../../client/common/process/types';
17-
import { IOutputChannel } from '../../client/common/types';
15+
import { IPlatformService } from '../../client/common/platform/types';
1816
import { IInterpreterLocatorService } from '../../client/interpreter/contracts';
1917
import { InterpreterType, PythonInterpreter } from '../../client/interpreter/contracts';
2018
import { ServiceContainer } from '../../client/ioc/container';
@@ -26,12 +24,9 @@ class TestContext {
2624
public serviceManager: ServiceManager;
2725
public serviceContainer: IServiceContainer;
2826
public platform: TypeMoq.IMock<IPlatformService>;
29-
public fileSystem: TypeMoq.IMock<IFileSystem>;
3027
public appShell: TypeMoq.IMock<IApplicationShell>;
3128
public locator: TypeMoq.IMock<IInterpreterLocatorService>;
3229
public settings: TypeMoq.IMock<IPythonSettings>;
33-
public process: TypeMoq.IMock<IProcessService>;
34-
public output: TypeMoq.IMock<vscode.OutputChannel>;
3530
public pythonInstaller: PythonInstaller;
3631

3732
constructor(isMac: boolean) {
@@ -40,19 +35,13 @@ class TestContext {
4035
this.serviceContainer = new ServiceContainer(cont);
4136

4237
this.platform = TypeMoq.Mock.ofType<IPlatformService>();
43-
this.fileSystem = TypeMoq.Mock.ofType<IFileSystem>();
4438
this.appShell = TypeMoq.Mock.ofType<IApplicationShell>();
4539
this.locator = TypeMoq.Mock.ofType<IInterpreterLocatorService>();
4640
this.settings = TypeMoq.Mock.ofType<IPythonSettings>();
47-
this.process = TypeMoq.Mock.ofType<IProcessService>();
48-
this.output = TypeMoq.Mock.ofType<vscode.OutputChannel>();
4941

5042
this.serviceManager.addSingletonInstance<IPlatformService>(IPlatformService, this.platform.object);
51-
this.serviceManager.addSingletonInstance<IFileSystem>(IFileSystem, this.fileSystem.object);
5243
this.serviceManager.addSingletonInstance<IApplicationShell>(IApplicationShell, this.appShell.object);
5344
this.serviceManager.addSingletonInstance<IInterpreterLocatorService>(IInterpreterLocatorService, this.locator.object);
54-
this.serviceManager.addSingletonInstance<IProcessService>(IProcessService, this.process.object);
55-
this.serviceManager.addSingletonInstance<vscode.OutputChannel>(IOutputChannel, this.output.object, STANDARD_OUTPUT_CHANNEL);
5645
this.pythonInstaller = new PythonInstaller(this.serviceContainer);
5746

5847
this.platform.setup(x => x.isMac).returns(() => isMac);
@@ -80,7 +69,7 @@ suite('Installation', () => {
8069
assert.equal(showErrorMessageCalled, false, 'Disabling checks has no effect');
8170
});
8271

83-
test('Windows: Python missing', async () => {
72+
test('Python missing', async () => {
8473
const c = new TestContext(false);
8574
let showErrorMessageCalled = false;
8675
let openUrlCalled = false;
@@ -100,7 +89,7 @@ suite('Installation', () => {
10089
assert.equal(url, 'https://www.python.org/downloads', 'Python download page is incorrect');
10190
});
10291

103-
test('Mac: Python missing', async () => {
92+
test('Mac: Default Python warning', async () => {
10493
const c = new TestContext(true);
10594
let called = false;
10695
c.appShell.setup(x => x.showWarningMessage(TypeMoq.It.isAnyString())).callback(() => called = true);
@@ -115,89 +104,4 @@ suite('Installation', () => {
115104
assert.equal(passed, true, 'Default MacOS Python not accepted');
116105
assert.equal(called, true, 'Warning not shown');
117106
});
118-
119-
test('Mac: Default Python, user refused install', async () => {
120-
const c = new TestContext(true);
121-
let errorMessage = '';
122-
123-
c.appShell
124-
.setup(x => x.showErrorMessage(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString()))
125-
.callback((m: string, a1: string, a2: string) => errorMessage = m)
126-
.returns(() => Promise.resolve('No'));
127-
c.locator.setup(x => x.getInterpreters()).returns(() => Promise.resolve([]));
128-
129-
const passed = await c.pythonInstaller.checkPythonInstallation(c.settings.object);
130-
assert.equal(passed, false, 'Default MacOS Python accepted');
131-
assert.equal(errorMessage.startsWith('Python that comes with MacOS is not supported'), true, 'Error message that MacOS Python not supported not shown');
132-
});
133-
134-
test('Mac: Default Python, Brew installation', async () => {
135-
const c = new TestContext(true);
136-
let errorMessage = '';
137-
let processName = '';
138-
let args;
139-
let brewPath;
140-
let outputShown = false;
141-
142-
c.appShell
143-
.setup(x => x.showErrorMessage(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString()))
144-
.returns(() => Promise.resolve('Yes'));
145-
c.appShell
146-
.setup(x => x.showErrorMessage(TypeMoq.It.isAnyString()))
147-
.callback((m: string) => errorMessage = m);
148-
c.locator.setup(x => x.getInterpreters()).returns(() => Promise.resolve([]));
149-
c.fileSystem
150-
.setup(x => x.fileExistsAsync(TypeMoq.It.isAnyString()))
151-
.returns((p: string) => {
152-
brewPath = p;
153-
return Promise.resolve(false);
154-
});
155-
156-
const childProcess = TypeMoq.Mock.ofType<ChildProcess>();
157-
childProcess
158-
.setup(p => p.on('exit', TypeMoq.It.isAny()))
159-
.callback((e: string, listener: (code, signal) => void) => {
160-
listener.call(0, undefined);
161-
});
162-
const processOutput: Output<string> = {
163-
source: 'stdout',
164-
out: 'started'
165-
};
166-
const observable = new Rx.Observable<Output<string>>(subscriber => subscriber.next(processOutput));
167-
const brewInstallProcess: ObservableExecutionResult<string> = {
168-
proc: childProcess.object,
169-
out: observable
170-
};
171-
172-
c.output.setup(x => x.show()).callback(() => outputShown = true);
173-
c.process
174-
.setup(x => x.execObservable(TypeMoq.It.isAnyString(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
175-
.callback((p: string, a: string[], o: SpawnOptions) => {
176-
processName = p;
177-
args = a;
178-
})
179-
.returns(() => brewInstallProcess);
180-
181-
await c.pythonInstaller.checkPythonInstallation(c.settings.object);
182-
183-
assert.notEqual(brewPath, undefined, 'Brew installer location not checked');
184-
assert.equal(brewPath, '/usr/local/bin/brew', 'Brew installer location is incorrect');
185-
assert.notEqual(processName, undefined, 'Brew installer not invoked');
186-
assert.equal(processName, '/usr/bin/ruby', 'Brew installer name is incorrect');
187-
assert.equal(args[0], '-e', 'Brew installer argument is incorrect');
188-
assert.equal(args[1], '"$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"', 'Homebrew installer argument is incorrect');
189-
assert.equal(outputShown, true, 'Output panel not shown');
190-
assert.equal(errorMessage.startsWith('Unable to install Homebrew'), true, 'Homebrew install failed message no shown');
191-
192-
c.fileSystem
193-
.setup(x => x.fileExistsAsync(TypeMoq.It.isAnyString()))
194-
.returns(() => Promise.resolve(true));
195-
errorMessage = '';
196-
197-
await c.pythonInstaller.checkPythonInstallation(c.settings.object);
198-
assert.equal(errorMessage, '', `Unexpected error message ${errorMessage}`);
199-
assert.equal(processName, 'brew', 'Brew installer name is incorrect');
200-
assert.equal(args[0], 'install', 'Brew "install" argument is incorrect');
201-
assert.equal(args[1], 'python', 'Brew "python" argument is incorrect');
202-
});
203107
});

0 commit comments

Comments
 (0)