Skip to content

Commit

Permalink
Ensure env manager executable is set (microsoft#23845)
Browse files Browse the repository at this point in the history
  • Loading branch information
@karthiknadig @eleanorjboyd
karthiknadig authored and eleanorjboyd committed Jul 30, 2024
1 parent 0c503c8 commit 7a3053a
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 40 deletions.
4 changes: 4 additions & 0 deletions src/client/pythonEnvironments/common/environmentManagers/conda.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -615,3 +615,7 @@ export class Conda {
return true;
}
}

export function setCondaBinary(executable: string): void {
Conda.setConda(executable);
}
104 changes: 64 additions & 40 deletions src/client/pythonEnvironments/nativeAPI.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,22 @@ import {
TriggerRefreshOptions,
} from './base/locator';
import { PythonEnvCollectionChangedEvent } from './base/watcher';
import { isNativeEnvInfo, NativeEnvInfo, NativePythonFinder } from './base/locators/common/nativePythonFinder';
import {
isNativeEnvInfo,
NativeEnvInfo,
NativeEnvManagerInfo,
NativePythonFinder,
} from './base/locators/common/nativePythonFinder';
import { createDeferred, Deferred } from '../common/utils/async';
import { Architecture } from '../common/utils/platform';
import { parseVersion } from './base/info/pythonVersion';
import { cache } from '../common/utils/decorators';
import { traceError, traceLog } from '../logging';
import { traceError, traceLog, traceWarn } from '../logging';
import { StopWatch } from '../common/utils/stopWatch';
import { FileChangeType } from '../common/platform/fileSystemWatcher';
import { categoryToKind } from './base/locators/common/nativePythonUtils';
import { setCondaBinary } from './common/environmentManagers/conda';
import { setPyEnvBinary } from './common/environmentManagers/pyenv';

function makeExecutablePath(prefix?: string): string {
if (!prefix) {
Expand Down Expand Up @@ -232,44 +239,10 @@ class NativePythonEnvironments implements IDiscoveryAPI, Disposable {
setImmediate(async () => {
try {
for await (const native of this.finder.refresh()) {
if (!isNativeEnvInfo(native) || !validEnv(native)) {
// eslint-disable-next-line no-continue
continue;
}
try {
const envPath = native.executable ?? native.prefix;
const version = native.version ? parseVersion(native.version) : undefined;

if (categoryToKind(native.kind) === PythonEnvKind.Conda && !native.executable) {
// This is a conda env without python, no point trying to resolve this.
// There is nothing to resolve
this.addEnv(native);
} else if (
envPath &&
(!version || version.major < 0 || version.minor < 0 || version.micro < 0)
) {
// We have a path, but no version info, try to resolve the environment.
this.finder
.resolve(envPath)
.then((env) => {
if (env) {
this.addEnv(env);
}
})
.ignoreErrors();
} else if (
envPath &&
version &&
version.major >= 0 &&
version.minor >= 0 &&
version.micro >= 0
) {
this.addEnv(native);
} else {
traceError(`Failed to process environment: ${JSON.stringify(native)}`);
}
} catch (err) {
traceError(`Failed to process environment: ${err}`);
if (isNativeEnvInfo(native)) {
this.processEnv(native);
} else {
this.processEnvManager(native);
}
}
this._refreshPromise?.resolve();
Expand All @@ -286,6 +259,57 @@ class NativePythonEnvironments implements IDiscoveryAPI, Disposable {
return this._refreshPromise?.promise;
}

private processEnv(native: NativeEnvInfo): void {
if (!validEnv(native)) {
return;
}

try {
const envPath = native.executable ?? native.prefix;
const version = native.version ? parseVersion(native.version) : undefined;

if (categoryToKind(native.kind) === PythonEnvKind.Conda && !native.executable) {
// This is a conda env without python, no point trying to resolve this.
// There is nothing to resolve
this.addEnv(native);
} else if (envPath && (!version || version.major < 0 || version.minor < 0 || version.micro < 0)) {
// We have a path, but no version info, try to resolve the environment.
this.finder
.resolve(envPath)
.then((env) => {
if (env) {
this.addEnv(env);
}
})
.ignoreErrors();
} else if (envPath && version && version.major >= 0 && version.minor >= 0 && version.micro >= 0) {
this.addEnv(native);
} else {
traceError(`Failed to process environment: ${JSON.stringify(native)}`);
}
} catch (err) {
traceError(`Failed to process environment: ${err}`);
}
}

// eslint-disable-next-line class-methods-use-this
private processEnvManager(native: NativeEnvManagerInfo) {
const tool = native.tool.toLowerCase();
switch (tool) {
case 'conda':
traceLog(`Conda environment manager found at: ${native.executable}`);
setCondaBinary(native.executable);
break;
case 'pyenv':
traceLog(`Pyenv environment manager found at: ${native.executable}`);
setPyEnvBinary(native.executable);
break;
default:
traceWarn(`Unknown environment manager: ${native.tool}`);
break;
}
}

getEnvs(_query?: PythonLocatorQuery): PythonEnvInfo[] {
return this._envs;
}
Expand Down
44 changes: 44 additions & 0 deletions src/test/pythonEnvironments/nativeAPI.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,21 @@ import * as nativeAPI from '../../client/pythonEnvironments/nativeAPI';
import { IDiscoveryAPI } from '../../client/pythonEnvironments/base/locator';
import {
NativeEnvInfo,
NativeEnvManagerInfo,
NativePythonFinder,
} from '../../client/pythonEnvironments/base/locators/common/nativePythonFinder';
import { Architecture } from '../../client/common/utils/platform';
import { PythonEnvInfo, PythonEnvKind, PythonEnvType } from '../../client/pythonEnvironments/base/info';
import { isWindows } from '../../client/common/platform/platformService';
import { NativePythonEnvironmentKind } from '../../client/pythonEnvironments/base/locators/common/nativePythonUtils';
import * as condaApi from '../../client/pythonEnvironments/common/environmentManagers/conda';
import * as pyenvApi from '../../client/pythonEnvironments/common/environmentManagers/pyenv';

suite('Native Python API', () => {
let api: IDiscoveryAPI;
let mockFinder: typemoq.IMock<NativePythonFinder>;
let setCondaBinaryStub: sinon.SinonStub;
let setPyEnvBinaryStub: sinon.SinonStub;

const basicEnv: NativeEnvInfo = {
displayName: 'Basic Python',
Expand Down Expand Up @@ -128,6 +133,9 @@ suite('Native Python API', () => {
setup(() => {
mockFinder = typemoq.Mock.ofType<NativePythonFinder>();
api = nativeAPI.createNativeEnvironmentsApi(mockFinder.object);

setCondaBinaryStub = sinon.stub(condaApi, 'setCondaBinary');
setPyEnvBinaryStub = sinon.stub(pyenvApi, 'setPyEnvBinary');
});

teardown(() => {
Expand Down Expand Up @@ -248,4 +256,40 @@ suite('Native Python API', () => {
await api.triggerRefresh();
assert.isUndefined(api.getRefreshPromise());
});

test('Setting conda binary', async () => {
const condaMgr: NativeEnvManagerInfo = {
tool: 'Conda',
executable: '/usr/bin/conda',
};
mockFinder
.setup((f) => f.refresh())
.returns(() => {
async function* generator() {
yield* [condaMgr];
}
return generator();
})
.verifiable(typemoq.Times.once());
await api.triggerRefresh();
assert.isTrue(setCondaBinaryStub.calledOnceWith(condaMgr.executable));
});

test('Setting pyenv binary', async () => {
const pyenvMgr: NativeEnvManagerInfo = {
tool: 'PyEnv',
executable: '/usr/bin/pyenv',
};
mockFinder
.setup((f) => f.refresh())
.returns(() => {
async function* generator() {
yield* [pyenvMgr];
}
return generator();
})
.verifiable(typemoq.Times.once());
await api.triggerRefresh();
assert.isTrue(setPyEnvBinaryStub.calledOnceWith(pyenvMgr.executable));
});
});

0 comments on commit 7a3053a

Please sign in to comment.