Ensure prefix conda envs does not set name (#15859)

karthiknadig · web-flow · commit 676ef37e0bab · 2021-04-07T16:15:23.000-07:00
* Ensure prefix conda envs does not set name

* Update unit tests

* Add news

* Add message to assert

* Ensure pyvenv.cfg is created for the dummy environment

* Tweak tests

* Skip unknown global virtual environments

* Set kind for global virtual env tests

* Ensure unknowns are skipped in global virtual env resolver

* Tweak tests

* Remove locating when testing for updates

* Restore expected environment type in tests.

* Remove environment type all together

* Clean up
diff --git a/news/2 Fixes/15823.md b/news/2 Fixes/15823.md
@@ -0,0 +1 @@
+Fixes activation of prefixed conda environments.
diff --git a/src/client/pythonEnvironments/base/locators/common/resourceBasedLocator.ts b/src/client/pythonEnvironments/base/locators/common/resourceBasedLocator.ts
@@ -51,7 +51,7 @@ export abstract class LazyResourceBasedLocator extends Locator implements IDispo
     /**
      * The subclass implementation of resolveEnv().
      */
-    protected abstract async doResolveEnv(_env: string | PythonEnvInfo): Promise<PythonEnvInfo | undefined>;
+    protected abstract doResolveEnv(_env: string | PythonEnvInfo): Promise<PythonEnvInfo | undefined>;
 
     /**
      * This is where subclasses get their resources ready.
diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/workspaceVirtualEnvLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/workspaceVirtualEnvLocator.ts
@@ -121,8 +121,14 @@ export class WorkspaceVirtualEnvironmentLocator extends FSWatchingLocator {
                             // check multiple times. Those checks are file system heavy and
                             // we can use the kind to determine this anyway.
                             const kind = await getVirtualEnvKind(filename);
-                            yield buildSimpleVirtualEnvInfo(filename, kind);
-                            traceVerbose(`Workspace Virtual Environment: [added] ${filename}`);
+
+                            if (kind === PythonEnvKind.Unknown) {
+                                // We don't know the environment type so skip this one.
+                                traceVerbose(`Workspace Virtual Environment: [skipped] ${filename}`);
+                            } else {
+                                yield buildSimpleVirtualEnvInfo(filename, kind);
+                                traceVerbose(`Workspace Virtual Environment: [added] ${filename}`);
+                            }
                         } else {
                             traceVerbose(`Workspace Virtual Environment: [skipped] ${filename}`);
                         }
@@ -146,6 +152,9 @@ export class WorkspaceVirtualEnvironmentLocator extends FSWatchingLocator {
             // check multiple times. Those checks are file system heavy and
             // we can use the kind to determine this anyway.
             const kind = await getVirtualEnvKind(executablePath);
+            if (kind === PythonEnvKind.Unknown) {
+                return undefined;
+            }
             return buildSimpleVirtualEnvInfo(executablePath, kind, source);
         }
         return undefined;
diff --git a/src/client/pythonEnvironments/discovery/locators/services/condaLocator.ts b/src/client/pythonEnvironments/discovery/locators/services/condaLocator.ts
@@ -4,7 +4,7 @@ import '../../../../common/extensions';
 import { PythonEnvInfo, PythonEnvKind, PythonEnvSource } from '../../../base/info';
 import { buildEnvInfo } from '../../../base/info/env';
 import { IPythonEnvsIterator, Locator } from '../../../base/locator';
-import { getInterpreterPathFromDir } from '../../../common/commonUtils';
+import { getInterpreterPathFromDir, getPythonVersionFromPath } from '../../../common/commonUtils';
 import { AnacondaCompanyName, Conda } from './conda';
 import { resolveEnvFromIterator } from '../../../base/locatorUtils';
 import { traceVerbose } from '../../../../common/logger';
@@ -49,6 +49,7 @@ export class CondaEnvironmentLocator extends Locator {
                     org: AnacondaCompanyName,
                     location: prefix,
                     source: [PythonEnvSource.Conda],
+                    version: await getPythonVersionFromPath(executable),
                 });
                 if (name) {
                     info.name = name;
diff --git a/src/client/pythonEnvironments/discovery/locators/services/globalVirtualEnvronmentLocator.ts b/src/client/pythonEnvironments/discovery/locators/services/globalVirtualEnvronmentLocator.ts
@@ -134,8 +134,13 @@ export class GlobalVirtualEnvironmentLocator extends FSWatchingLocator {
                             // check multiple times. Those checks are file system heavy and
                             // we can use the kind to determine this anyway.
                             const kind = await getVirtualEnvKind(filename);
-                            yield buildSimpleVirtualEnvInfo(filename, kind);
-                            traceVerbose(`Global Virtual Environment: [added] ${filename}`);
+                            if (kind === PythonEnvKind.Unknown) {
+                                // We don't know the environment type so skip this one.
+                                traceVerbose(`Global Virtual Environment: [skipped] ${filename}`);
+                            } else {
+                                yield buildSimpleVirtualEnvInfo(filename, kind);
+                                traceVerbose(`Global Virtual Environment: [added] ${filename}`);
+                            }
                         } else {
                             traceVerbose(`Global Virtual Environment: [skipped] ${filename}`);
                         }
@@ -158,6 +163,10 @@ export class GlobalVirtualEnvironmentLocator extends FSWatchingLocator {
             // check multiple times. Those checks are file system heavy and
             // we can use the kind to determine this anyway.
             const kind = await getVirtualEnvKind(executablePath);
+            if (kind === PythonEnvKind.Unknown) {
+                // We don't know the environment type so skip this one.
+                return undefined;
+            }
             return buildSimpleVirtualEnvInfo(executablePath, kind);
         }
         return undefined;
diff --git a/src/client/pythonEnvironments/discovery/locators/services/poetryLocator.ts b/src/client/pythonEnvironments/discovery/locators/services/poetryLocator.ts
@@ -55,10 +55,6 @@ async function getRootVirtualEnvDir(root: string): Promise<string[]> {
     return rootDirs;
 }
 
-async function getVirtualEnvKind(interpreterPath: string): Promise<PythonEnvKind> {
-    return (await isPoetryEnvironment(interpreterPath)) ? PythonEnvKind.Poetry : PythonEnvKind.Unknown;
-}
-
 async function buildVirtualEnvInfo(
     executablePath: string,
     kind: PythonEnvKind,
@@ -143,8 +139,7 @@ export class PoetryLocator extends FSWatchingLocator {
     protected async doResolveEnv(env: string | PythonEnvInfo): Promise<PythonEnvInfo | undefined> {
         const executablePath = typeof env === 'string' ? env : env.executable.filename;
         const source = typeof env === 'string' ? [PythonEnvSource.Other] : uniq([PythonEnvSource.Other, ...env.source]);
-        const kind = await getVirtualEnvKind(executablePath);
-        if (kind === PythonEnvKind.Poetry) {
+        if (await isPoetryEnvironment(executablePath)) {
             const isLocal = isParentPath(executablePath, this.root);
             return buildVirtualEnvInfo(executablePath, PythonEnvKind.Poetry, source, isLocal);
         }
diff --git a/src/client/pythonEnvironments/index.ts b/src/client/pythonEnvironments/index.ts
@@ -119,27 +119,29 @@ async function createLocators(
 }
 
 function createNonWorkspaceLocators(ext: ExtensionState): ILocator[] {
-    let locators: (ILocator & Partial<IDisposable>)[];
+    const locators: (ILocator & Partial<IDisposable>)[] = [];
+    locators.push(
+        // OS-independent locators go here.
+        new PyenvLocator(),
+        new CondaEnvironmentLocator(),
+        new GlobalVirtualEnvironmentLocator(),
+        new CustomVirtualEnvironmentLocator(),
+    );
+
     if (getOSType() === OSType.Windows) {
-        locators = [
+        locators.push(
             // Windows specific locators go here.
             new WindowsRegistryLocator(),
             new WindowsStoreLocator(),
             new WindowsPathEnvVarLocator(),
-        ];
+        );
     } else {
-        locators = [
+        locators.push(
             // Linux/Mac locators go here.
             new PosixKnownPathsLocator(),
-        ];
+        );
     }
-    locators.push(
-        // OS-independent locators go here.
-        new GlobalVirtualEnvironmentLocator(),
-        new PyenvLocator(),
-        new CustomVirtualEnvironmentLocator(),
-        new CondaEnvironmentLocator(),
-    );
+
     const disposables = locators.filter((d) => d.dispose !== undefined) as IDisposable[];
     ext.disposables.push(...disposables);
     return locators;
diff --git a/src/test/pythonEnvironments/base/locators/lowLevel/workspaceVirtualEnvLocator.testvirtualenvs.ts b/src/test/pythonEnvironments/base/locators/lowLevel/workspaceVirtualEnvLocator.testvirtualenvs.ts
@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 import * as path from 'path';
+import { PythonEnvKind } from '../../../../../client/pythonEnvironments/base/info';
 import { WorkspaceVirtualEnvironmentLocator } from '../../../../../client/pythonEnvironments/base/locators/lowLevel/workspaceVirtualEnvLocator';
 import { TEST_LAYOUT_ROOT } from '../../../common/commonTestConstants';
 import { testLocatorWatcher } from '../../../discovery/locators/watcherTestUtils';
@@ -10,5 +11,6 @@ suite('WorkspaceVirtualEnvironment Locator', async () => {
     const testWorkspaceFolder = path.join(TEST_LAYOUT_ROOT, 'workspace', 'folder1');
     testLocatorWatcher(testWorkspaceFolder, async (root?: string) => new WorkspaceVirtualEnvironmentLocator(root!), {
         arg: testWorkspaceFolder,
+        kind: PythonEnvKind.Venv,
     });
 });
diff --git a/src/test/pythonEnvironments/base/locators/lowLevel/workspaceVirtualEnvLocator.unit.test.ts b/src/test/pythonEnvironments/base/locators/lowLevel/workspaceVirtualEnvLocator.unit.test.ts
@@ -122,25 +122,13 @@ suite('WorkspaceVirtualEnvironment Locator', () => {
 
     test('iterEnvs(): Non-Windows', async () => {
         const expectedEnvs = [
-            createExpectedEnvInfo(
-                path.join(testWorkspaceFolder, 'posix2conda', 'python'),
-                PythonEnvKind.Unknown,
-                { major: 3, minor: 8, micro: 5 },
-                'posix2conda',
-            ),
             createExpectedEnvInfo(
                 path.join(testWorkspaceFolder, '.direnv', 'posix1virtualenv', 'bin', 'python'),
                 PythonEnvKind.VirtualEnv,
                 { major: 3, minor: 8, micro: -1 },
                 'posix1virtualenv',
                 path.join(testWorkspaceFolder, '.direnv', 'posix1virtualenv'),
             ),
-            createExpectedEnvInfo(
-                path.join(testWorkspaceFolder, 'posix3custom', 'bin', 'python'),
-                PythonEnvKind.Unknown,
-                undefined,
-                'posix3custom',
-            ),
         ].sort((a, b) => a.executable.filename.localeCompare(b.executable.filename));
 
         const iterator = locator.iterEnvs();
@@ -153,12 +141,13 @@ suite('WorkspaceVirtualEnvironment Locator', () => {
     });
 
     test('resolveEnv(string)', async () => {
-        const interpreterPath = path.join(testWorkspaceFolder, 'posix2conda', 'python');
+        const interpreterPath = path.join(testWorkspaceFolder, '.direnv', 'posix1virtualenv', 'bin', 'python');
         const expected = createExpectedEnvInfo(
-            path.join(testWorkspaceFolder, 'posix2conda', 'python'),
-            PythonEnvKind.Unknown,
-            { major: 3, minor: 8, micro: 5 },
-            'posix2conda',
+            path.join(testWorkspaceFolder, '.direnv', 'posix1virtualenv', 'bin', 'python'),
+            PythonEnvKind.VirtualEnv,
+            { major: 3, minor: 8, micro: -1 },
+            'posix1virtualenv',
+            path.join(testWorkspaceFolder, '.direnv', 'posix1virtualenv'),
         );
 
         const actual = await locator.resolveEnv(interpreterPath);
@@ -167,12 +156,13 @@ suite('WorkspaceVirtualEnvironment Locator', () => {
     });
 
     test('resolveEnv(PythonEnvInfo)', async () => {
-        const interpreterPath = path.join(testWorkspaceFolder, 'posix2conda', 'python');
+        const interpreterPath = path.join(testWorkspaceFolder, '.direnv', 'posix1virtualenv', 'bin', 'python');
         const expected = createExpectedEnvInfo(
-            path.join(testWorkspaceFolder, 'posix2conda', 'python'),
-            PythonEnvKind.Unknown,
-            { major: 3, minor: 8, micro: 5 },
-            'posix2conda',
+            path.join(testWorkspaceFolder, '.direnv', 'posix1virtualenv', 'bin', 'python'),
+            PythonEnvKind.VirtualEnv,
+            { major: 3, minor: 8, micro: -1 },
+            'posix1virtualenv',
+            path.join(testWorkspaceFolder, '.direnv', 'posix1virtualenv'),
         );
 
         // Partially filled in env info object
diff --git a/src/test/pythonEnvironments/discovery/locators/condaHelper.unit.test.ts b/src/test/pythonEnvironments/discovery/locators/condaHelper.unit.test.ts
@@ -638,7 +638,13 @@ suite('Conda and its environments are located correctly', () => {
                     display: undefined,
                     searchLocation: undefined,
                     distro: { org: AnacondaCompanyName },
-                    version: { major: -1, minor: -1, micro: -1, release: { level: 'final', serial: 0 } },
+                    version: {
+                        major: -1,
+                        minor: -1,
+                        micro: -1,
+                        release: { level: 'final', serial: -1 },
+                        sysVersion: undefined,
+                    },
                     location: prefix,
                     executable: {
                         filename: path.join(prefix, 'bin', 'python'),
diff --git a/src/test/pythonEnvironments/discovery/locators/watcherTestUtils.ts b/src/test/pythonEnvironments/discovery/locators/watcherTestUtils.ts
@@ -48,15 +48,23 @@ class Venvs {
      * Creates a dummy environment by creating a fake executable.
      * @param name environment suffix name to create
      */
-    public async createDummyEnv(name: string): Promise<{ executable: string; envDir: string }> {
+    public async createDummyEnv(
+        name: string,
+        kind: PythonEnvKind | undefined,
+    ): Promise<{ executable: string; envDir: string }> {
         const envName = this.resolve(name);
-        const filepath = path.join(this.root, envName, getOSType() === OSType.Windows ? 'python.exe' : 'python');
+        const interpreterPath = path.join(this.root, envName, getOSType() === OSType.Windows ? 'python.exe' : 'python');
+        const configPath = path.join(this.root, envName, 'pyvenv.cfg');
         try {
-            await fs.createFile(filepath);
+            await fs.createFile(interpreterPath);
+            if (kind === PythonEnvKind.Venv) {
+                await fs.createFile(configPath);
+                await fs.writeFile(configPath, 'version = 3.9.2');
+            }
         } catch (err) {
-            throw new Error(`Failed to create python executable ${filepath}, Error: ${err}`);
+            throw new Error(`Failed to create python executable ${interpreterPath}, Error: ${err}`);
         }
-        return { executable: filepath, envDir: path.dirname(filepath) };
+        return { executable: interpreterPath, envDir: path.dirname(interpreterPath) };
     }
 
     // eslint-disable-next-line class-methods-use-this
@@ -233,7 +241,7 @@ export function testLocatorWatcher(
         // Create a dummy environment so we can update its executable later. We can't choose a real environment here.
         // Executables inside real environments can be symlinks, so writing on them can result in the real executable
         // being updated instead of the symlink.
-        const { executable, envDir } = await venvs.createDummyEnv('one');
+        const { executable, envDir } = await venvs.createDummyEnv('one', options?.kind);
         await setupLocator(async (e) => {
             if (e.type === FileChangeType.Changed) {
                 actualEvent = e;
@@ -243,19 +251,14 @@ export function testLocatorWatcher(
 
         await venvs.update(executable);
         await waitForChangeToBeDetected(deferred);
-        if (!options?.doNotVerifyIfLocated) {
-            const isFound = await isLocated(executable);
-            assert.ok(isFound);
-        }
-
-        assert.notEqual(actualEvent!, undefined, 'Wrong event emitted');
+        assert.notEqual(actualEvent!, undefined, 'Event was not emitted');
         if (options?.kind) {
-            assert.equal(actualEvent!.kind, options.kind, 'Wrong event emitted');
+            assert.equal(actualEvent!.kind, options.kind, 'Kind is not as expected');
         }
-        assert.notEqual(actualEvent!.searchLocation, undefined, 'Wrong event emitted');
+        assert.notEqual(actualEvent!.searchLocation, undefined, 'Search location is not set');
         assert.ok(
             externalDeps.arePathsSame(actualEvent!.searchLocation!.fsPath, path.dirname(envDir)),
-            'Wrong event emitted',
+            `Paths don't match ${actualEvent!.searchLocation!.fsPath} != ${path.dirname(envDir)}`,
         );
     });
 }

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+Fixes activation of prefixed conda environments.`