Skip to content

Commit e26a1ed

Browse files
author
Kartik Raj
authored
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
1 parent e3ca457 commit e26a1ed

10 files changed

Lines changed: 32 additions & 32 deletions

File tree

‎news/2 Fixes/17244.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Do path comparisons appropriately in the new discovery component.

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { getOSType, OSType } from '../../../common/utils/platform';
66
import { getEmptyVersion, parseVersion } from './pythonVersion';
77

88
import { PythonVersion } from '.';
9+
import { normCasePath } from '../../common/externalDependencies';
910

1011
/**
1112
* Determine a best-effort Python version based on the given filename.
@@ -21,6 +22,7 @@ export function parseVersionFromExecutable(filename: string): PythonVersion {
2122
}
2223

2324
function parseBasename(basename: string): PythonVersion {
25+
basename = normCasePath(basename);
2426
if (getOSType() === OSType.Windows) {
2527
if (basename === 'python.exe') {
2628
// On Windows we can't assume it is 2.7.

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
getInterpreterPathFromDir,
1313
getPythonVersionFromPath,
1414
} from '../../../common/commonUtils';
15-
import { getWorkspaceFolders, isParentPath } from '../../../common/externalDependencies';
15+
import { arePathsSame, getWorkspaceFolders, isParentPath } from '../../../common/externalDependencies';
1616
import { AnacondaCompanyName, Conda } from '../../../common/environmentManagers/conda';
1717
import { parsePyenvVersion } from '../../../common/environmentManagers/pyenv';
1818
import { Architecture, getOSType, OSType } from '../../../../common/utils/platform';
@@ -82,7 +82,7 @@ async function updateEnvUsingRegistry(env: PythonEnvInfo): Promise<void> {
8282
traceError('Expected registry interpreter cache to be initialized already');
8383
interpreters = await getRegistryInterpreters();
8484
}
85-
const data = interpreters.find((i) => i.interpreterPath.toUpperCase() === env.executable.filename.toUpperCase());
85+
const data = interpreters.find((i) => arePathsSame(i.interpreterPath, env.executable.filename));
8686
if (data) {
8787
const versionStr = data.versionStr ?? data.sysVersionStr ?? data.interpreterPath;
8888
let version;

‎src/client/pythonEnvironments/common/commonUtils.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { comparePythonVersionSpecificity } from '../base/info/env';
1111
import { parseVersion } from '../base/info/pythonVersion';
1212
import { getPythonVersionFromConda } from './environmentManagers/conda';
1313
import { getPythonVersionFromPyvenvCfg } from './environmentManagers/simplevirtualenvs';
14+
import { normCasePath } from './externalDependencies';
1415
import * as posix from './posixUtils';
1516
import * as windows from './windowsUtils';
1617

@@ -362,7 +363,7 @@ export function getEnvironmentDirFromPath(interpreterPath: string): string {
362363
// env <--- Return this directory if it is not 'bin' or 'scripts'
363364
// |__ python <--- interpreterPath
364365
const dir = path.basename(path.dirname(interpreterPath));
365-
if (!skipDirs.includes(dir.toLowerCase())) {
366+
if (!skipDirs.map((e) => normCasePath(e)).includes(normCasePath(dir))) {
366367
return path.dirname(interpreterPath);
367368
}
368369

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import * as fsapi from 'fs-extra';
22
import * as path from 'path';
33
import { traceVerbose } from '../../../common/logger';
44
import { getEnvironmentVariable, getOSType, getUserHomeDir, OSType } from '../../../common/utils/platform';
5-
import { exec, getPythonSetting, pathExists, readFile } from '../externalDependencies';
5+
import { arePathsSame, exec, getPythonSetting, pathExists, readFile } from '../externalDependencies';
66

77
import { PythonVersion, UNKNOWN_PYTHON_VERSION } from '../../base/info';
88
import { parseVersion } from '../../base/info/pythonVersion';
@@ -373,7 +373,7 @@ export class Conda {
373373
}
374374

375375
function getName(prefix: string) {
376-
if (prefix === info.root_prefix) {
376+
if (info.root_prefix && arePathsSame(prefix, info.root_prefix)) {
377377
return 'base';
378378
}
379379

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import * as path from 'path';
55
import { traceError } from '../../../common/logger';
66
import { getEnvironmentVariable } from '../../../common/utils/platform';
7-
import { arePathsSame, pathExists, readFile } from '../externalDependencies';
7+
import { arePathsSame, normCasePath, pathExists, readFile } from '../externalDependencies';
88

99
function getSearchHeight() {
1010
// 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
108108
// project
109109
// |__ Pipfile <--- check if Pipfile exists here and return it
110110
// The name of the project (directory where Pipfile resides) is used as a prefix in the environment folder
111-
const envFolderName = path.basename(envFolder);
112-
if (!envFolderName.startsWith(`${path.basename(projectDir)}-`)) {
111+
const envFolderName = path.basename(normCasePath(envFolder));
112+
if (!envFolderName.startsWith(`${path.basename(normCasePath(projectDir))}-`)) {
113113
return undefined;
114114
}
115115

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

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import * as path from 'path';
22
import { getEnvironmentVariable, getOSType, getUserHomeDir, OSType } from '../../../common/utils/platform';
3-
import { arePathsSame, pathExists } from '../externalDependencies';
3+
import { arePathsSame, isParentPath, pathExists } from '../externalDependencies';
44

55
export function getPyenvDir(): string {
66
// Check if the pyenv environment variables exist: PYENV on Windows, PYENV_ROOT on Unix.
@@ -37,23 +37,14 @@ export function isPyenvShimDir(dirPath: string): boolean {
3737
*/
3838

3939
export async function isPyenvEnvironment(interpreterPath: string): Promise<boolean> {
40-
let pathToCheck = interpreterPath;
41-
let pyenvDir = getPyenvDir();
40+
const pathToCheck = interpreterPath;
41+
const pyenvDir = getPyenvDir();
4242

4343
if (!(await pathExists(pyenvDir))) {
4444
return false;
4545
}
4646

47-
if (!pyenvDir.endsWith(path.sep)) {
48-
pyenvDir += path.sep;
49-
}
50-
51-
if (getOSType() === OSType.Windows) {
52-
pyenvDir = pyenvDir.toUpperCase();
53-
pathToCheck = pathToCheck.toUpperCase();
54-
}
55-
56-
return pathToCheck.startsWith(pyenvDir);
47+
return isParentPath(pathToCheck, pyenvDir);
5748
}
5849

5950
export interface IPyenvVersionStrings {

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

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { getEnvironmentVariable, getOSType, getUserHomeDir, OSType } from '../..
88
import { PythonVersion, UNKNOWN_PYTHON_VERSION } from '../../base/info';
99
import { comparePythonVersionSpecificity } from '../../base/info/env';
1010
import { parseBasicVersion, parseRelease, parseVersion } from '../../base/info/pythonVersion';
11-
import { pathExists, readFile } from '../externalDependencies';
11+
import { isParentPath, pathExists, readFile } from '../externalDependencies';
1212

1313
function getPyvenvConfigPathsFrom(interpreterPath: string): string[] {
1414
const pyvenvConfigFile = 'pyvenv.cfg';
@@ -99,20 +99,13 @@ function getWorkOnHome(): Promise<string> {
9999
*/
100100
export async function isVirtualenvwrapperEnvironment(interpreterPath: string): Promise<boolean> {
101101
const workOnHomeDir = await getWorkOnHome();
102-
let pathToCheck = interpreterPath;
103-
let workOnRoot = workOnHomeDir;
104-
105-
if (getOSType() === OSType.Windows) {
106-
workOnRoot = workOnHomeDir.toUpperCase();
107-
pathToCheck = interpreterPath.toUpperCase();
108-
}
109102

110103
// For environment to be a virtualenvwrapper based it has to follow these two rules:
111104
// 1. It should be in a sub-directory under the WORKON_HOME
112105
// 2. It should be a valid virtualenv environment
113106
return (
114107
(await pathExists(workOnHomeDir)) &&
115-
pathToCheck.startsWith(`${workOnRoot}${path.sep}`) &&
108+
isParentPath(interpreterPath, workOnHomeDir) &&
116109
isVirtualenvEnvironment(interpreterPath)
117110
);
118111
}

‎src/client/pythonEnvironments/common/externalDependencies.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,9 @@ export const untildify: (value: string) => string = require('untildify');
8282
* @param parentPath The potential parent path to check for
8383
*/
8484
export function isParentPath(filePath: string, parentPath: string): boolean {
85+
if (!parentPath.endsWith(path.sep)) {
86+
parentPath += path.sep;
87+
}
8588
return normCasePath(filePath).startsWith(normCasePath(parentPath));
8689
}
8790

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { assert, expect } from 'chai';
55
import * as path from 'path';
66
import * as sinon from 'sinon';
77
import { ExecutionResult, ShellOptions } from '../../../../client/common/process/types';
8-
import { getUserHomeDir } from '../../../../client/common/utils/platform';
8+
import * as platformApis from '../../../../client/common/utils/platform';
99
import * as externalDependencies from '../../../../client/pythonEnvironments/common/externalDependencies';
1010
import { isPoetryEnvironment, Poetry } from '../../../../client/pythonEnvironments/common/environmentManagers/poetry';
1111
import { TEST_LAYOUT_ROOT } from '../commonTestConstants';
@@ -20,6 +20,12 @@ suite('isPoetryEnvironment Tests', () => {
2020
let getPythonSetting: sinon.SinonStub;
2121

2222
suite('Global poetry environment', async () => {
23+
setup(() => {
24+
sinon.stub(platformApis, 'getOSType').callsFake(() => platformApis.OSType.Windows);
25+
});
26+
teardown(() => {
27+
sinon.restore();
28+
});
2329
test('Return true if environment folder name matches global env pattern and environment is of virtual env type', async () => {
2430
const result = await isPoetryEnvironment(
2531
path.join(testPoetryDir, 'poetry-tutorial-project-6hnqYwvD-py3.8', 'Scripts', 'python.exe'),
@@ -60,16 +66,19 @@ suite('isPoetryEnvironment Tests', () => {
6066
});
6167

6268
test('Return true if environment folder name matches criteria for local envs', async () => {
69+
sinon.stub(platformApis, 'getOSType').callsFake(() => platformApis.OSType.Windows);
6370
const result = await isPoetryEnvironment(path.join(project1, '.venv', 'Scripts', 'python.exe'));
6471
expect(result).to.equal(true);
6572
});
6673

6774
test(`Return false if environment folder name is not named '.venv' for local envs`, async () => {
75+
sinon.stub(platformApis, 'getOSType').callsFake(() => platformApis.OSType.Windows);
6876
const result = await isPoetryEnvironment(path.join(project1, '.venv2', 'Scripts', 'python.exe'));
6977
expect(result).to.equal(false);
7078
});
7179

7280
test(`Return false if running poetry for project dir as cwd fails (pyproject.toml file is invalid)`, async () => {
81+
sinon.stub(platformApis, 'getOSType').callsFake(() => platformApis.OSType.Linux);
7382
const result = await isPoetryEnvironment(path.join(project4, '.venv', 'bin', 'python'));
7483
expect(result).to.equal(false);
7584
});
@@ -148,7 +157,7 @@ suite('Poetry binary is located correctly', async () => {
148157
});
149158

150159
test('When poetry is not available on PATH, try using the default poetry location if valid', async () => {
151-
const home = getUserHomeDir();
160+
const home = platformApis.getUserHomeDir();
152161
if (!home) {
153162
assert(true);
154163
return;

0 commit comments

Comments
 (0)