Skip to content

Commit 91bd1a8

Browse files
author
Kartik Raj
authored
Add support for base interpreter paths (#18441)
* Add support for shell-like interpreter paths * Add explanation in comment
1 parent 33e5411 commit 91bd1a8

File tree

4 files changed

+41
-31
lines changed

4 files changed

+41
-31
lines changed

src/client/common/process/pythonEnvironment.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4+
import * as path from 'path';
45
import { traceError, traceInfo } from '../../logging';
56
import { Conda, CondaEnvironmentInfo } from '../../pythonEnvironments/common/environmentManagers/conda';
67
import { buildPythonExecInfo, PythonExecInfo } from '../../pythonEnvironments/exec';
@@ -56,7 +57,7 @@ class PythonEnvironment implements IPythonEnvironment {
5657
return result;
5758
}
5859
const python = this.getExecutionInfo();
59-
const promise = getExecutablePath(python, this.deps.exec);
60+
const promise = getExecutablePath(python, this.deps.shellExec);
6061
this.cachedExecutablePath.set(this.pythonPath, promise);
6162
return promise;
6263
}
@@ -106,8 +107,19 @@ function createDeps(
106107
shellExec: (command: string, options?: ShellOptions) => Promise<ExecutionResult<string>>,
107108
) {
108109
return {
109-
getPythonArgv: (python: string) => pythonArgv || [python],
110-
getObservablePythonArgv: (python: string) => observablePythonArgv || [python],
110+
getPythonArgv: (python: string) => {
111+
if (path.basename(python) === python) {
112+
// Say when python is `py -3.8` or `conda run python`
113+
pythonArgv = python.split(' ');
114+
}
115+
return pythonArgv || [python];
116+
},
117+
getObservablePythonArgv: (python: string) => {
118+
if (path.basename(python) === python) {
119+
observablePythonArgv = python.split(' ');
120+
}
121+
return observablePythonArgv || [python];
122+
},
111123
isValidExecutable,
112124
exec: async (cmd: string, args: string[]) => exec(cmd, args, { throwOnStdErr: true }),
113125
shellExec: async (text: string, timeout: number) => shellExec(text, { timeout }),
Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,30 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4-
import { getExecutable as getPythonExecutableCommand } from '../../common/process/internal/python';
4+
import { getExecutable } from '../../common/process/internal/python';
5+
import { ExecutionResult } from '../../common/process/types';
56
import { copyPythonExecInfo, PythonExecInfo } from '../exec';
67

7-
type ExecResult = {
8-
stdout: string;
9-
};
10-
type ExecFunc = (command: string, args: string[]) => Promise<ExecResult>;
8+
type ShellExecFunc = (command: string, timeout: number) => Promise<ExecutionResult<string>>;
119

1210
/**
1311
* Find the filename for the corresponding Python executable.
1412
*
1513
* Effectively, we look up `sys.executable`.
1614
*
1715
* @param python - the information to use when running Python
18-
* @param exec - the function to use to run Python
16+
* @param shellExec - the function to use to run Python
1917
*/
20-
export async function getExecutablePath(python: PythonExecInfo, exec: ExecFunc): Promise<string> {
21-
const [args, parse] = getPythonExecutableCommand();
18+
export async function getExecutablePath(
19+
python: PythonExecInfo,
20+
shellExec: ShellExecFunc,
21+
timeout?: number,
22+
): Promise<string> {
23+
const [args, parse] = getExecutable();
2224
const info = copyPythonExecInfo(python, args);
23-
const result = await exec(info.command, info.args);
24-
return parse(result.stdout);
25+
const argv = [info.command, ...info.args];
26+
// Concat these together to make a set of quoted strings
27+
const quoted = argv.reduce((p, c) => (p ? `${p} ${c.toCommandArgument()}` : `${c.toCommandArgument()}`), '');
28+
const result = await shellExec(quoted, timeout ?? 15000);
29+
return parse(result.stdout.trim());
2530
}

src/test/common/process/pythonEnvironment.unit.test.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -192,9 +192,8 @@ suite('PythonEnvironment', () => {
192192
test('getExecutablePath should not return pythonPath if pythonPath is not a file', async () => {
193193
const executablePath = 'path/to/dummy/executable';
194194
fileSystem.setup((f) => f.pathExists(pythonPath)).returns(() => Promise.resolve(false));
195-
const argv = ['-c', 'import sys;print(sys.executable)'];
196195
processService
197-
.setup((p) => p.exec(pythonPath, argv, { throwOnStdErr: true }))
196+
.setup((p) => p.shellExec(`${pythonPath} -c "import sys;print(sys.executable)"`, TypeMoq.It.isAny()))
198197
.returns(() => Promise.resolve({ stdout: executablePath }));
199198
const env = createPythonEnv(pythonPath, processService.object, fileSystem.object);
200199

@@ -206,9 +205,8 @@ suite('PythonEnvironment', () => {
206205
test('getExecutablePath should throw if the result of exec() writes to stderr', async () => {
207206
const stderr = 'bar';
208207
fileSystem.setup((f) => f.pathExists(pythonPath)).returns(() => Promise.resolve(false));
209-
const argv = ['-c', 'import sys;print(sys.executable)'];
210208
processService
211-
.setup((p) => p.exec(pythonPath, argv, { throwOnStdErr: true }))
209+
.setup((p) => p.shellExec(`${pythonPath} -c "import sys;print(sys.executable)"`, TypeMoq.It.isAny()))
212210
.returns(() => Promise.reject(new StdErrError(stderr)));
213211
const env = createPythonEnv(pythonPath, processService.object, fileSystem.object);
214212

src/test/pythonEnvironments/info/executable.unit.test.ts

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,13 @@
22
// Licensed under the MIT License.
33

44
import { expect } from 'chai';
5-
import { IMock, Mock, MockBehavior } from 'typemoq';
6-
import { StdErrError } from '../../../client/common/process/types';
5+
import { IMock, Mock, MockBehavior, It } from 'typemoq';
6+
import { ExecutionResult, StdErrError } from '../../../client/common/process/types';
77
import { buildPythonExecInfo } from '../../../client/pythonEnvironments/exec';
88
import { getExecutablePath } from '../../../client/pythonEnvironments/info/executable';
99

10-
type ExecResult = {
11-
stdout: string;
12-
};
1310
interface IDeps {
14-
exec(command: string, args: string[]): Promise<ExecResult>;
11+
shellExec(command: string, timeout: number): Promise<ExecutionResult<string>>;
1512
}
1613

1714
suite('getExecutablePath()', () => {
@@ -24,11 +21,10 @@ suite('getExecutablePath()', () => {
2421

2522
test('should get the value by running python', async () => {
2623
const expected = 'path/to/dummy/executable';
27-
const argv = ['-c', 'import sys;print(sys.executable)'];
28-
deps.setup((d) => d.exec(python.command, argv))
24+
deps.setup((d) => d.shellExec(`${python.command} -c "import sys;print(sys.executable)"`, It.isAny()))
2925
// Return the expected value.
3026
.returns(() => Promise.resolve({ stdout: expected }));
31-
const exec = async (c: string, a: string[]) => deps.object.exec(c, a);
27+
const exec = async (c: string, a: number) => deps.object.shellExec(c, a);
3228

3329
const result = await getExecutablePath(python, exec);
3430

@@ -38,15 +34,14 @@ suite('getExecutablePath()', () => {
3834

3935
test('should throw if exec() fails', async () => {
4036
const stderr = 'oops';
41-
const argv = ['-c', 'import sys;print(sys.executable)'];
42-
deps.setup((d) => d.exec(python.command, argv))
37+
deps.setup((d) => d.shellExec(`${python.command} -c "import sys;print(sys.executable)"`, It.isAny()))
4338
// Throw an error.
4439
.returns(() => Promise.reject(new StdErrError(stderr)));
45-
const exec = async (c: string, a: string[]) => deps.object.exec(c, a);
40+
const exec = async (c: string, a: number) => deps.object.shellExec(c, a);
4641

47-
const result = getExecutablePath(python, exec);
42+
const promise = getExecutablePath(python, exec);
4843

49-
expect(result).to.eventually.be.rejectedWith(stderr);
44+
expect(promise).to.eventually.be.rejectedWith(stderr);
5045
deps.verifyAll();
5146
});
5247
});

0 commit comments

Comments
 (0)