Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/client/common/installer/condaInstaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ export class CondaInstaller extends ModuleInstaller {
flags: ModuleInstallFlags = 0,
): Promise<ExecutionInfo> {
const condaService = this.serviceContainer.get<ICondaService>(ICondaService);
const condaFile = await condaService.getCondaFile();
// Installation using `conda.exe` sometimes fails with a HTTP error on Windows:
// https://github.com/conda/conda/issues/11399
// Execute in a shell which uses a `conda.bat` file instead, using which installation works.
const useShell = true;
const condaFile = await condaService.getCondaFile(useShell);

const pythonPath = isResource(resource)
? this.serviceContainer.get<IConfigurationService>(IConfigurationService).getSettings(resource).pythonPath
Expand Down Expand Up @@ -117,8 +121,7 @@ export class CondaInstaller extends ModuleInstaller {
return {
args,
execPath: condaFile,
// Execute in a shell as `conda` on windows refers to `conda.bat`, which requires a shell to work.
useShell: true,
useShell,
};
}

Expand Down
2 changes: 1 addition & 1 deletion src/client/interpreter/contracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export const ICondaService = Symbol('ICondaService');
* Interface carries the properties which are not available via the discovery component interface.
*/
export interface ICondaService {
getCondaFile(): Promise<string>;
getCondaFile(forShellExecution?: boolean): Promise<string>;
isCondaAvailable(): Promise<boolean>;
getCondaVersion(): Promise<SemVer | undefined>;
getInterpreterPathForEnvironment(condaEnv: CondaEnvironmentInfo): Promise<string | undefined>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ async function buildEnvironmentInfoUsingCondaRun(env: PythonEnvInfo): Promise<In
if (!condaEnv) {
return undefined;
}
const python = await conda?.getRunPythonArgs(condaEnv);
const python = await conda?.getRunPythonArgs(condaEnv, true);
if (!python) {
return undefined;
}
Expand Down
35 changes: 23 additions & 12 deletions src/client/pythonEnvironments/common/environmentManagers/conda.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
readFile,
shellExecute,
onDidChangePythonSetting,
exec,
} from '../externalDependencies';

import { PythonVersion, UNKNOWN_PYTHON_VERSION } from '../../base/info';
Expand Down Expand Up @@ -243,13 +244,19 @@ export class Conda {

private condaInfoCached: Promise<CondaInfo> | undefined;

/**
* Carries path to conda binary to be used for shell execution.
*/
public readonly shellCommand: string;

/**
* Creates a Conda service corresponding to the corresponding "conda" command.
*
* @param command - Command used to spawn conda. This has the same meaning as the
* first argument of spawn() - i.e. it can be a full path, or just a binary name.
*/
constructor(readonly command: string) {
constructor(readonly command: string, shellCommand?: string) {
this.shellCommand = shellCommand ?? command;
onDidChangePythonSetting(CONDAPATH_SETTING_KEY, () => {
Conda.condaPromise = undefined;
});
Expand Down Expand Up @@ -379,7 +386,7 @@ export class Conda {
if (condaBatFile) {
const condaBat = new Conda(condaBatFile);
await condaBat.getInfo();
conda = condaBat;
conda = new Conda(condaPath, condaBatFile);
}
} catch (ex) {
traceVerbose('Failed to spawn conda bat file', condaBatFile, ex);
Expand Down Expand Up @@ -414,12 +421,10 @@ export class Conda {
/**
* Temporarily cache result for this particular command.
*/
// @cache(30_000, true, 10_000)
@cache(30_000, true, 10_000)
// eslint-disable-next-line class-methods-use-this
private async getInfoImpl(command: string): Promise<CondaInfo> {
const quoted = [command.toCommandArgument(), 'info', '--json'].join(' ');
// Execute in a shell as `conda` on windows refers to `conda.bat`, which requires a shell to work.
const result = await shellExecute(quoted, { timeout: CONDA_GENERAL_TIMEOUT });
const result = await exec(command, ['info', '--json'], { timeout: CONDA_GENERAL_TIMEOUT });
traceVerbose(`conda info --json: ${result.stdout}`);
return JSON.parse(result.stdout);
}
Expand All @@ -428,7 +433,7 @@ export class Conda {
* Retrieves list of Python environments known to this conda.
* Corresponds to "conda env list --json", but also computes environment names.
*/
// @cache(30_000, true, 10_000)
@cache(30_000, true, 10_000)
public async getEnvList(useCache?: boolean): Promise<CondaEnvInfo[]> {
const info = await this.getInfo(useCache);
const { envs } = info;
Expand Down Expand Up @@ -496,7 +501,7 @@ export class Conda {
return undefined;
}

public async getRunPythonArgs(env: CondaEnvInfo): Promise<string[] | undefined> {
public async getRunPythonArgs(env: CondaEnvInfo, forShellExecution?: boolean): Promise<string[] | undefined> {
const condaVersion = await this.getCondaVersion();
if (condaVersion && lt(condaVersion, CONDA_RUN_VERSION)) {
return undefined;
Expand All @@ -507,7 +512,15 @@ export class Conda {
} else {
args.push('-p', env.prefix);
}
return [this.command, 'run', ...args, '--no-capture-output', '--live-stream', 'python', OUTPUT_MARKER_SCRIPT];
return [
forShellExecution ? this.shellCommand : this.command,
'run',
...args,
'--no-capture-output',
'--live-stream',
'python',
OUTPUT_MARKER_SCRIPT,
];
}

/**
Expand All @@ -520,9 +533,7 @@ export class Conda {
if (info && info.conda_version) {
versionString = info.conda_version;
} else {
const quoted = `${this.command.toCommandArgument()} --version`;
// Execute in a shell as `conda` on windows refers to `conda.bat`, which requires a shell to work.
const stdOut = await shellExecute(quoted, { timeout: CONDA_GENERAL_TIMEOUT })
const stdOut = await exec(this.command, ['--version'], { timeout: CONDA_GENERAL_TIMEOUT })
.then((result) => result.stdout.trim())
.catch<string | undefined>(() => undefined);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@ export class CondaService implements ICondaService {
* Return the path to the "conda file".
*/
// eslint-disable-next-line class-methods-use-this
public async getCondaFile(): Promise<string> {
return Conda.getConda().then((conda) => conda?.command ?? 'conda');
public async getCondaFile(forShellExecution?: boolean): Promise<string> {
return Conda.getConda().then((conda) => {
const command = forShellExecution ? conda?.shellCommand : conda?.command;
return command ?? 'conda';
});
}

// eslint-disable-next-line class-methods-use-this
Expand Down
4 changes: 2 additions & 2 deletions src/test/common/installer/condaInstaller.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ suite('Common - Conda Installer', () => {

when(configService.getSettings(uri)).thenReturn(instance(settings));
when(settings.pythonPath).thenReturn(pythonPath);
when(condaService.getCondaFile()).thenResolve(condaPath);
when(condaService.getCondaFile(true)).thenResolve(condaPath);
when(condaLocatorService.getCondaEnvironment(pythonPath)).thenResolve(condaEnv);

const execInfo = await installer.getExecutionInfo('abc', uri);
Expand All @@ -122,7 +122,7 @@ suite('Common - Conda Installer', () => {

when(configService.getSettings(uri)).thenReturn(instance(settings));
when(settings.pythonPath).thenReturn(pythonPath);
when(condaService.getCondaFile()).thenResolve(condaPath);
when(condaService.getCondaFile(true)).thenResolve(condaPath);
when(condaLocatorService.getCondaEnvironment(pythonPath)).thenResolve(condaEnv);

const execInfo = await installer.getExecutionInfo('abc', uri);
Expand Down
3 changes: 3 additions & 0 deletions src/test/common/installer/moduleInstaller.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,9 @@ suite('Module Installer', () => {

const condaService = TypeMoq.Mock.ofType<ICondaService>();
condaService.setup((c) => c.getCondaFile()).returns(() => Promise.resolve(condaExecutable));
condaService
.setup((c) => c.getCondaFile(true))
.returns(() => Promise.resolve(condaExecutable));

const condaLocatorService = TypeMoq.Mock.ofType<IComponentAdapter>();
serviceContainer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,17 +297,12 @@ suite('Python envs locator - Environments Resolver', () => {
if (getOSType() !== OSType.Windows) {
this.skip();
}
stubShellExec.restore();
sinon.stub(externalDependencies, 'getPythonSetting').withArgs('condaPath').returns('conda');
sinon.stub(externalDependencies, 'shellExecute').callsFake(async (quoted: string) => {
const [command, ...args] = quoted.split(' ');
sinon.stub(externalDependencies, 'exec').callsFake(async (command: string, args: string[]) => {
if (command === 'conda' && args[0] === 'info' && args[1] === '--json') {
return { stdout: JSON.stringify(condaInfo(path.join(envsWithoutPython, 'condaLackingPython'))) };
}
return {
stdout:
'{"versionInfo": [3, 8, 3, "final", 0], "sysPrefix": "path", "sysVersion": "3.8.3 (tags/v3.8.3:6f8c832, May 13 2020, 22:37:02) [MSC v.1924 64 bit (AMD64)]", "is64Bit": true}',
};
throw new Error(`${command} is missing or is not executable`);
});
const parentLocator = new SimpleLocator([]);
const resolver = new PythonEnvsResolver(parentLocator, envInfoService);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,7 @@ suite('Resolver Utils', () => {

test('resolveEnv (Windows)', async () => {
sinon.stub(platformApis, 'getOSType').callsFake(() => platformApis.OSType.Windows);
sinon.stub(externalDependencies, 'shellExecute').callsFake(async (quoted: string) => {
const [command, ...args] = quoted.split(' ');
sinon.stub(externalDependencies, 'exec').callsFake(async (command: string, args: string[]) => {
if (command === 'conda' && args[0] === 'info' && args[1] === '--json') {
return { stdout: JSON.stringify(condaInfo(condaPrefixWindows)) };
}
Expand All @@ -263,8 +262,7 @@ suite('Resolver Utils', () => {

test('resolveEnv (non-Windows)', async () => {
sinon.stub(platformApis, 'getOSType').callsFake(() => platformApis.OSType.Linux);
sinon.stub(externalDependencies, 'shellExecute').callsFake(async (quoted: string) => {
const [command, ...args] = quoted.split(' ');
sinon.stub(externalDependencies, 'exec').callsFake(async (command: string, args: string[]) => {
if (command === 'conda' && args[0] === 'info' && args[1] === '--json') {
return { stdout: JSON.stringify(condaInfo(condaPrefixNonWindows)) };
}
Expand All @@ -282,7 +280,7 @@ suite('Resolver Utils', () => {

test('resolveEnv: If no conda binary found, resolve as an unknown environment', async () => {
sinon.stub(platformApis, 'getOSType').callsFake(() => platformApis.OSType.Windows);
sinon.stub(externalDependencies, 'shellExecute').callsFake(async (command: string) => {
sinon.stub(externalDependencies, 'exec').callsFake(async (command: string) => {
throw new Error(`${command} is missing or is not executable`);
});
const actual = await resolveBasicEnv({
Expand Down Expand Up @@ -605,7 +603,7 @@ suite('Resolver Utils', () => {
});

test('If data provided by registry is less informative than kind resolvers, do not use it to update environment', async () => {
sinon.stub(externalDependencies, 'shellExecute').callsFake(async (command: string) => {
sinon.stub(externalDependencies, 'exec').callsFake(async (command: string) => {
throw new Error(`${command} is missing or is not executable`);
});
const interpreterPath = path.join(regTestRoot, 'conda3', 'python.exe');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,7 @@ suite('Conda and its environments are located correctly', () => {
return contents;
});

sinon.stub(externalDependencies, 'shellExecute').callsFake(async (quoted: string) => {
const [command, ...args] = quoted.split(' ');
sinon.stub(externalDependencies, 'exec').callsFake(async (command: string, args: string[]) => {
for (const prefix of ['', ...execPath]) {
const contents = getFile(path.join(prefix, command));
if (args[0] === 'info' && args[1] === '--json') {
Expand Down