Skip to content

Commit 6170c64

Browse files
karthiknadigkarthiknadig
authored
fix: bad install step when attempting editable install (#402)
For #369
1 parent e74470f commit 6170c64

File tree

3 files changed

+140
-16
lines changed

3 files changed

+140
-16
lines changed

‎src/managers/builtin/pipUtils.ts

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
1+
import * as tomljs from '@iarna/toml';
12
import * as fse from 'fs-extra';
23
import * as path from 'path';
3-
import * as tomljs from '@iarna/toml';
44
import { LogOutputChannel, ProgressLocation, QuickInputButtons, QuickPickItem, Uri } from 'vscode';
5-
import { showQuickPickWithButtons, withProgress } from '../../common/window.apis';
6-
import { PackageManagement, Pickers, VenvManagerStrings } from '../../common/localize';
75
import { PackageManagementOptions, PythonEnvironment, PythonEnvironmentApi, PythonProject } from '../../api';
8-
import { findFiles } from '../../common/workspace.apis';
96
import { EXTENSION_ROOT_DIR } from '../../common/constants';
10-
import { selectFromCommonPackagesToInstall, selectFromInstallableToInstall } from '../common/pickers';
7+
import { PackageManagement, Pickers, VenvManagerStrings } from '../../common/localize';
118
import { traceInfo } from '../../common/logging';
12-
import { refreshPipPackages } from './utils';
13-
import { mergePackages } from '../common/utils';
9+
import { showQuickPickWithButtons, withProgress } from '../../common/window.apis';
10+
import { findFiles } from '../../common/workspace.apis';
11+
import { selectFromCommonPackagesToInstall, selectFromInstallableToInstall } from '../common/pickers';
1412
import { Installable } from '../common/types';
13+
import { mergePackages } from '../common/utils';
14+
import { refreshPipPackages } from './utils';
1515

1616
async function tomlParse(fsPath: string, log?: LogOutputChannel): Promise<tomljs.JsonMap> {
1717
try {
@@ -29,6 +29,7 @@ function isPipInstallableToml(toml: tomljs.JsonMap): boolean {
2929

3030
function getTomlInstallable(toml: tomljs.JsonMap, tomlPath: Uri): Installable[] {
3131
const extras: Installable[] = [];
32+
const projectDir = path.dirname(tomlPath.fsPath);
3233

3334
if (isPipInstallableToml(toml)) {
3435
const name = path.basename(tomlPath.fsPath);
@@ -37,7 +38,7 @@ function getTomlInstallable(toml: tomljs.JsonMap, tomlPath: Uri): Installable[]
3738
displayName: name,
3839
description: VenvManagerStrings.installEditable,
3940
group: 'TOML',
40-
args: ['-e', path.dirname(tomlPath.fsPath)],
41+
args: ['-e', projectDir],
4142
uri: tomlPath,
4243
});
4344
}
@@ -49,7 +50,8 @@ function getTomlInstallable(toml: tomljs.JsonMap, tomlPath: Uri): Installable[]
4950
name: key,
5051
displayName: key,
5152
group: 'TOML',
52-
args: ['-e', `.[${key}]`],
53+
// Use a single -e argument with the extras specified as part of the path
54+
args: ['-e', `${projectDir}[${key}]`],
5355
uri: tomlPath,
5456
});
5557
}

‎src/managers/builtin/utils.ts

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,18 @@ import {
88
PythonEnvironmentApi,
99
PythonEnvironmentInfo,
1010
} from '../../api';
11+
import { showErrorMessageWithLogs } from '../../common/errors/utils';
12+
import { SysManagerStrings } from '../../common/localize';
13+
import { withProgress } from '../../common/window.apis';
1114
import {
1215
isNativeEnvInfo,
1316
NativeEnvInfo,
1417
NativePythonEnvironmentKind,
1518
NativePythonFinder,
1619
} from '../common/nativePythonFinder';
1720
import { shortVersion, sortEnvironments } from '../common/utils';
18-
import { SysManagerStrings } from '../../common/localize';
19-
import { isUvInstalled, runUV, runPython } from './helpers';
21+
import { isUvInstalled, runPython, runUV } from './helpers';
2022
import { parsePipList, PipPackage } from './pipListUtils';
21-
import { withProgress } from '../../common/window.apis';
22-
import { showErrorMessageWithLogs } from '../../common/errors/utils';
2323

2424
function asPackageQuickPickItem(name: string, version?: string): QuickPickItem {
2525
return {
@@ -222,25 +222,66 @@ export async function managePackages(
222222
installArgs.push('--upgrade');
223223
}
224224
if (options.install && options.install.length > 0) {
225+
const processedInstallArgs = processEditableInstallArgs(options.install);
226+
225227
if (useUv) {
226228
await runUV(
227-
[...installArgs, '--python', environment.execInfo.run.executable, ...options.install],
229+
[...installArgs, '--python', environment.execInfo.run.executable, ...processedInstallArgs],
228230
undefined,
229231
manager.log,
230232
token,
231233
);
232234
} else {
233235
await runPython(
234236
environment.execInfo.run.executable,
235-
['-m', ...installArgs, ...options.install],
237+
['-m', ...installArgs, ...processedInstallArgs],
236238
undefined,
237239
manager.log,
238240
token,
239241
);
240242
}
241243
}
242244

243-
return refreshPackages(environment, api, manager);
245+
return await refreshPackages(environment, api, manager);
246+
}
247+
248+
/**
249+
* Process pip install arguments to correctly handle editable installs with extras
250+
* This function will combine consecutive -e arguments that represent the same package with extras
251+
*/
252+
export function processEditableInstallArgs(args: string[]): string[] {
253+
const processedArgs: string[] = [];
254+
let i = 0;
255+
256+
while (i < args.length) {
257+
if (args[i] === '-e') {
258+
const packagePath = args[i + 1];
259+
if (!packagePath) {
260+
processedArgs.push(args[i]);
261+
i++;
262+
continue;
263+
}
264+
265+
if (i + 2 < args.length && args[i + 2] === '-e' && i + 3 < args.length) {
266+
const nextArg = args[i + 3];
267+
268+
if (nextArg.startsWith('.[') && nextArg.includes(']')) {
269+
const combinedPath = packagePath + nextArg.substring(1);
270+
processedArgs.push('-e', combinedPath);
271+
i += 4;
272+
continue;
273+
}
274+
}
275+
276+
processedArgs.push(args[i], packagePath);
277+
i += 2;
278+
} else {
279+
processedArgs.push(args[i]);
280+
i++;
281+
}
282+
}
283+
284+
return processedArgs;
244285
}
245286

246287
export async function resolveSystemPythonEnvironmentPath(

‎src/test/managers/builtin/installArgs.unit.test.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import assert from 'assert';
2+
import { processEditableInstallArgs } from '../../../managers/builtin/utils';
3+
4+
suite('Process Editable Install Arguments Tests', () => {
5+
test('should handle empty args array', () => {
6+
const result = processEditableInstallArgs([]);
7+
assert.deepStrictEqual(result, [], 'Should return empty array for empty input');
8+
});
9+
10+
test('should pass through non-editable install args unchanged', () => {
11+
const args = ['numpy', 'pandas==2.0.0', '--user'];
12+
const result = processEditableInstallArgs(args);
13+
assert.deepStrictEqual(result, args, 'Should return regular args unchanged');
14+
});
15+
16+
test('should pass through single -e argument unchanged', () => {
17+
const args = ['-e', 'c:/path/to/package'];
18+
const result = processEditableInstallArgs(args);
19+
assert.deepStrictEqual(result, args, 'Should return single -e arg unchanged');
20+
});
21+
22+
test('should pass through multiple unrelated -e arguments unchanged', () => {
23+
const args = ['-e', 'c:/path/to/package1', '-e', 'c:/path/to/package2'];
24+
const result = processEditableInstallArgs(args);
25+
assert.deepStrictEqual(result, args, 'Should return multiple unrelated -e args unchanged');
26+
});
27+
28+
test('should combine -e with extras correctly', () => {
29+
const args = ['-e', 'c:/path/to/package', '-e', '.[testing]'];
30+
const expected = ['-e', 'c:/path/to/package[testing]'];
31+
const result = processEditableInstallArgs(args);
32+
assert.deepStrictEqual(result, expected, 'Should combine -e with extras correctly');
33+
});
34+
35+
test('should handle multiple editable installs with extras correctly', () => {
36+
const args = ['-e', 'c:/path/to/package1', '-e', '.[testing]', '-e', 'c:/path/to/package2', '-e', '.[dev]'];
37+
const expected = ['-e', 'c:/path/to/package1[testing]', '-e', 'c:/path/to/package2[dev]'];
38+
const result = processEditableInstallArgs(args);
39+
assert.deepStrictEqual(result, expected, 'Should handle multiple editable installs with extras');
40+
});
41+
42+
test('should handle mixed regular and editable installs correctly', () => {
43+
const args = ['numpy', '-e', 'c:/path/to/package', '-e', '.[testing]', 'pandas==2.0.0'];
44+
const expected = ['numpy', '-e', 'c:/path/to/package[testing]', 'pandas==2.0.0'];
45+
const result = processEditableInstallArgs(args);
46+
assert.deepStrictEqual(result, expected, 'Should handle mixed regular and editable installs');
47+
});
48+
49+
test('should handle incomplete -e arguments gracefully', () => {
50+
const args = ['-e'];
51+
const result = processEditableInstallArgs(args);
52+
assert.deepStrictEqual(result, ['-e'], 'Should handle incomplete -e arguments');
53+
});
54+
55+
test('should not combine -e args when second is not an extras specification', () => {
56+
const args = ['-e', 'c:/path/to/package1', '-e', 'c:/path/to/package2'];
57+
const result = processEditableInstallArgs(args);
58+
assert.deepStrictEqual(result, args, 'Should not combine when second -e arg is not an extras spec');
59+
});
60+
61+
test('should handle extras with multiple requirements', () => {
62+
const args = ['-e', 'c:/path/to/package', '-e', '.[testing,dev]'];
63+
const expected = ['-e', 'c:/path/to/package[testing,dev]'];
64+
const result = processEditableInstallArgs(args);
65+
assert.deepStrictEqual(result, expected, 'Should handle extras with multiple requirements');
66+
});
67+
68+
test('should handle Windows-style paths correctly', () => {
69+
const args = ['-e', 'C:\\path\\to\\package', '-e', '.[testing]'];
70+
const expected = ['-e', 'C:\\path\\to\\package[testing]'];
71+
const result = processEditableInstallArgs(args);
72+
assert.deepStrictEqual(result, expected, 'Should handle Windows paths correctly');
73+
});
74+
75+
test('should handle editable installs followed by other args', () => {
76+
const args = ['-e', 'c:/path/to/package', '-e', '.[testing]', '--no-deps'];
77+
const expected = ['-e', 'c:/path/to/package[testing]', '--no-deps'];
78+
const result = processEditableInstallArgs(args);
79+
assert.deepStrictEqual(result, expected, 'Should handle editable installs followed by other args');
80+
});
81+
});

0 commit comments

Comments
 (0)