Do path comparisons appropriately in the discovery component (microsoft#17244)

* Do path comparisons appropriately in the discovery component

* News entry

* Fix some tests

* Some more fixes

* Fix poetry unit tests

Author: Kartik Raj

news/2 Fixes/17244.md

@@ -0,0 +1 @@
+Do path comparisons appropriately in the new discovery component.

src/client/pythonEnvironments/base/info/executable.ts

@@ -6,6 +6,7 @@ import { getOSType, OSType } from '../../../common/utils/platform';
 import { getEmptyVersion, parseVersion } from './pythonVersion';
 
 import { PythonVersion } from '.';
+import { normCasePath } from '../../common/externalDependencies';
 
 /**
  * Determine a best-effort Python version based on the given filename.
@@ -21,6 +22,7 @@ export function parseVersionFromExecutable(filename: string): PythonVersion {
 }
 
 function parseBasename(basename: string): PythonVersion {
+    basename = normCasePath(basename);
     if (getOSType() === OSType.Windows) {
         if (basename === 'python.exe') {
             // On Windows we can't assume it is 2.7.

src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts

@@ -12,7 +12,7 @@ import {
     getInterpreterPathFromDir,
     getPythonVersionFromPath,
 } from '../../../common/commonUtils';
-import { getWorkspaceFolders, isParentPath } from '../../../common/externalDependencies';
+import { arePathsSame, getWorkspaceFolders, isParentPath } from '../../../common/externalDependencies';
 import { AnacondaCompanyName, Conda } from '../../../common/environmentManagers/conda';
 import { parsePyenvVersion } from '../../../common/environmentManagers/pyenv';
 import { Architecture, getOSType, OSType } from '../../../../common/utils/platform';
@@ -82,7 +82,7 @@ async function updateEnvUsingRegistry(env: PythonEnvInfo): Promise<void> {
         traceError('Expected registry interpreter cache to be initialized already');
         interpreters = await getRegistryInterpreters();
     }
-    const data = interpreters.find((i) => i.interpreterPath.toUpperCase() === env.executable.filename.toUpperCase());
+    const data = interpreters.find((i) => arePathsSame(i.interpreterPath, env.executable.filename));
     if (data) {
         const versionStr = data.versionStr ?? data.sysVersionStr ?? data.interpreterPath;
         let version;

src/client/pythonEnvironments/common/commonUtils.ts

@@ -11,6 +11,7 @@ import { comparePythonVersionSpecificity } from '../base/info/env';
 import { parseVersion } from '../base/info/pythonVersion';
 import { getPythonVersionFromConda } from './environmentManagers/conda';
 import { getPythonVersionFromPyvenvCfg } from './environmentManagers/simplevirtualenvs';
+import { normCasePath } from './externalDependencies';
 import * as posix from './posixUtils';
 import * as windows from './windowsUtils';
 
@@ -362,7 +363,7 @@ export function getEnvironmentDirFromPath(interpreterPath: string): string {
     // env <--- Return this directory if it is not 'bin' or 'scripts'
     // |__ python  <--- interpreterPath
     const dir = path.basename(path.dirname(interpreterPath));
-    if (!skipDirs.includes(dir.toLowerCase())) {
+    if (!skipDirs.map((e) => normCasePath(e)).includes(normCasePath(dir))) {
         return path.dirname(interpreterPath);
     }
 

src/client/pythonEnvironments/common/environmentManagers/conda.ts

@@ -2,7 +2,7 @@ import * as fsapi from 'fs-extra';
 import * as path from 'path';
 import { traceVerbose } from '../../../common/logger';
 import { getEnvironmentVariable, getOSType, getUserHomeDir, OSType } from '../../../common/utils/platform';
-import { exec, getPythonSetting, pathExists, readFile } from '../externalDependencies';
+import { arePathsSame, exec, getPythonSetting, pathExists, readFile } from '../externalDependencies';
 
 import { PythonVersion, UNKNOWN_PYTHON_VERSION } from '../../base/info';
 import { parseVersion } from '../../base/info/pythonVersion';
@@ -373,7 +373,7 @@ export class Conda {
         }
 
         function getName(prefix: string) {
-            if (prefix === info.root_prefix) {
+            if (info.root_prefix && arePathsSame(prefix, info.root_prefix)) {
                 return 'base';
             }
 

src/client/pythonEnvironments/common/environmentManagers/pipenv.ts

@@ -4,7 +4,7 @@
 import * as path from 'path';
 import { traceError } from '../../../common/logger';
 import { getEnvironmentVariable } from '../../../common/utils/platform';
-import { arePathsSame, pathExists, readFile } from '../externalDependencies';
+import { arePathsSame, normCasePath, pathExists, readFile } from '../externalDependencies';
 
 function getSearchHeight() {
     // PIPENV_MAX_DEPTH tells pipenv the maximum number of directories to recursively search for
@@ -108,8 +108,8 @@ async function getPipfileIfGlobal(interpreterPath: string): Promise<string | und
     //  project
     // |__ Pipfile  <--- check if Pipfile exists here and return it
     // The name of the project (directory where Pipfile resides) is used as a prefix in the environment folder
-    const envFolderName = path.basename(envFolder);
-    if (!envFolderName.startsWith(`${path.basename(projectDir)}-`)) {
+    const envFolderName = path.basename(normCasePath(envFolder));
+    if (!envFolderName.startsWith(`${path.basename(normCasePath(projectDir))}-`)) {
         return undefined;
     }
 

src/client/pythonEnvironments/common/environmentManagers/pyenv.ts

@@ -1,6 +1,6 @@
 import * as path from 'path';
 import { getEnvironmentVariable, getOSType, getUserHomeDir, OSType } from '../../../common/utils/platform';
-import { arePathsSame, pathExists } from '../externalDependencies';
+import { arePathsSame, isParentPath, pathExists } from '../externalDependencies';
 
 export function getPyenvDir(): string {
     // Check if the pyenv environment variables exist: PYENV on Windows, PYENV_ROOT on Unix.
@@ -37,23 +37,14 @@ export function isPyenvShimDir(dirPath: string): boolean {
  */
 
 export async function isPyenvEnvironment(interpreterPath: string): Promise<boolean> {
-    let pathToCheck = interpreterPath;
-    let pyenvDir = getPyenvDir();
+    const pathToCheck = interpreterPath;
+    const pyenvDir = getPyenvDir();
 
     if (!(await pathExists(pyenvDir))) {
         return false;
     }
 
-    if (!pyenvDir.endsWith(path.sep)) {
-        pyenvDir += path.sep;
-    }
-
-    if (getOSType() === OSType.Windows) {
-        pyenvDir = pyenvDir.toUpperCase();
-        pathToCheck = pathToCheck.toUpperCase();
-    }
-
-    return pathToCheck.startsWith(pyenvDir);
+    return isParentPath(pathToCheck, pyenvDir);
 }
 
 export interface IPyenvVersionStrings {

src/client/pythonEnvironments/common/environmentManagers/simplevirtualenvs.ts

@@ -8,7 +8,7 @@ import { getEnvironmentVariable, getOSType, getUserHomeDir, OSType } from '../..
 import { PythonVersion, UNKNOWN_PYTHON_VERSION } from '../../base/info';
 import { comparePythonVersionSpecificity } from '../../base/info/env';
 import { parseBasicVersion, parseRelease, parseVersion } from '../../base/info/pythonVersion';
-import { pathExists, readFile } from '../externalDependencies';
+import { isParentPath, pathExists, readFile } from '../externalDependencies';
 
 function getPyvenvConfigPathsFrom(interpreterPath: string): string[] {
     const pyvenvConfigFile = 'pyvenv.cfg';
@@ -99,20 +99,13 @@ function getWorkOnHome(): Promise<string> {
  */
 export async function isVirtualenvwrapperEnvironment(interpreterPath: string): Promise<boolean> {
     const workOnHomeDir = await getWorkOnHome();
-    let pathToCheck = interpreterPath;
-    let workOnRoot = workOnHomeDir;
-
-    if (getOSType() === OSType.Windows) {
-        workOnRoot = workOnHomeDir.toUpperCase();
-        pathToCheck = interpreterPath.toUpperCase();
-    }
 
     // For environment to be a virtualenvwrapper based it has to follow these two rules:
     // 1. It should be in a sub-directory under the WORKON_HOME
     // 2. It should be a valid virtualenv environment
     return (
         (await pathExists(workOnHomeDir)) &&
-        pathToCheck.startsWith(`${workOnRoot}${path.sep}`) &&
+        isParentPath(interpreterPath, workOnHomeDir) &&
         isVirtualenvEnvironment(interpreterPath)
     );
 }

src/client/pythonEnvironments/common/externalDependencies.ts

@@ -82,6 +82,9 @@ export const untildify: (value: string) => string = require('untildify');
  * @param parentPath The potential parent path to check for
  */
 export function isParentPath(filePath: string, parentPath: string): boolean {
+    if (!parentPath.endsWith(path.sep)) {
+        parentPath += path.sep;
+    }
     return normCasePath(filePath).startsWith(normCasePath(parentPath));
 }
 

src/test/pythonEnvironments/common/environmentManagers/poetry.unit.test.ts

@@ -5,7 +5,7 @@ import { assert, expect } from 'chai';
 import * as path from 'path';
 import * as sinon from 'sinon';
 import { ExecutionResult, ShellOptions } from '../../../../client/common/process/types';
-import { getUserHomeDir } from '../../../../client/common/utils/platform';
+import * as platformApis from '../../../../client/common/utils/platform';
 import * as externalDependencies from '../../../../client/pythonEnvironments/common/externalDependencies';
 import { isPoetryEnvironment, Poetry } from '../../../../client/pythonEnvironments/common/environmentManagers/poetry';
 import { TEST_LAYOUT_ROOT } from '../commonTestConstants';
@@ -20,6 +20,12 @@ suite('isPoetryEnvironment Tests', () => {
     let getPythonSetting: sinon.SinonStub;
 
     suite('Global poetry environment', async () => {
+        setup(() => {
+            sinon.stub(platformApis, 'getOSType').callsFake(() => platformApis.OSType.Windows);
+        });
+        teardown(() => {
+            sinon.restore();
+        });
         test('Return true if environment folder name matches global env pattern and environment is of virtual env type', async () => {
             const result = await isPoetryEnvironment(
                 path.join(testPoetryDir, 'poetry-tutorial-project-6hnqYwvD-py3.8', 'Scripts', 'python.exe'),
@@ -60,16 +66,19 @@ suite('isPoetryEnvironment Tests', () => {
         });
 
         test('Return true if environment folder name matches criteria for local envs', async () => {
+            sinon.stub(platformApis, 'getOSType').callsFake(() => platformApis.OSType.Windows);
             const result = await isPoetryEnvironment(path.join(project1, '.venv', 'Scripts', 'python.exe'));
             expect(result).to.equal(true);
         });
 
         test(`Return false if environment folder name is not named '.venv' for local envs`, async () => {
+            sinon.stub(platformApis, 'getOSType').callsFake(() => platformApis.OSType.Windows);
             const result = await isPoetryEnvironment(path.join(project1, '.venv2', 'Scripts', 'python.exe'));
             expect(result).to.equal(false);
         });
 
         test(`Return false if running poetry for project dir as cwd fails (pyproject.toml file is invalid)`, async () => {
+            sinon.stub(platformApis, 'getOSType').callsFake(() => platformApis.OSType.Linux);
             const result = await isPoetryEnvironment(path.join(project4, '.venv', 'bin', 'python'));
             expect(result).to.equal(false);
         });
@@ -148,7 +157,7 @@ suite('Poetry binary is located correctly', async () => {
     });
 
     test('When poetry is not available on PATH, try using the default poetry location if valid', async () => {
-        const home = getUserHomeDir();
+        const home = platformApis.getUserHomeDir();
         if (!home) {
             assert(true);
             return;