Skip to content

Commit 32e057b

Browse files
committed
Stop quoting interpreter path in adapter factory
Remove shell-style quoting of the interpreter executable when constructing DebugAdapterExecutable and add a comment explaining why (child_process.spawn is invoked without a shell and manual quotes become part of the filename causing ENOENT). Drop the fileToCommandArgumentForPythonExt import and update unit tests to expect unquoted interpreter paths for both default and custom debug adapter paths. References regressions reported in microsoft#1013 and analysis of microsoft#964.
1 parent 260fe4c commit 32e057b

2 files changed

Lines changed: 10 additions & 16 deletions

File tree

src/extension/debugger/adapter/factory.ts

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import { sendTelemetryEvent } from '../../telemetry';
2222
import { Commands, EXTENSION_ROOT_DIR } from '../../common/constants';
2323
import { Common, DebugConfigStrings, Interpreters } from '../../common/utils/localize';
2424
import { IPersistentStateFactory } from '../../common/types';
25-
import { fileToCommandArgumentForPythonExt } from '../../common/stringUtils';
2625
import { PythonEnvironment } from '../../envExtApi';
2726
import { resolveEnvironment, getInterpreterDetails, runPythonExtensionCommand } from '../../common/python';
2827

@@ -79,13 +78,12 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
7978
}
8079

8180
let executable = command.shift() ?? 'python';
82-
83-
// Always ensure interpreter/command is quoted if necessary. Previously this was
84-
// only done in the debugAdapterPath branch which meant that in the common case
85-
// (using the built‑in adapter path) an interpreter path containing spaces would
86-
// be passed unquoted, resulting in a fork/spawn failure on Windows. See bug
87-
// report for details.
88-
executable = fileToCommandArgumentForPythonExt(executable);
81+
// DO NOT apply shell-style quoting here.
82+
// The 'executable' path is passed to 'DebugAdapterExecutable', which internally
83+
// uses 'child_process.spawn' in a non-shell environment.
84+
// Manual quoting will cause the OS (especially Windows) to treat the quotes
85+
// as part of the filename, leading to ENOENT.
86+
// See regression reported in #1013 and analysis of #964.
8987

9088
// "logToFile" is not handled directly by the adapter - instead, we need to pass
9189
// the corresponding CLI switch when spawning it.

src/test/unittest/adapter/factory.unit.test.ts

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -304,28 +304,24 @@ suite('Debugging - Adapter Factory', () => {
304304

305305
assert.deepStrictEqual(descriptor, debugExecutable);
306306
});
307-
test('Add quotes to interpreter path with spaces (default adapter path)', async () => {
307+
test('Should not add quotes to interpreter path with spaces (default adapter path)', async () => {
308308
const session = createSession({});
309309
const interpreterPathSpaces = 'path/to/python interpreter with spaces';
310-
const interpreterPathSpacesQuoted = `"${interpreterPathSpaces}"`;
311-
const debugExecutable = new DebugAdapterExecutable(interpreterPathSpacesQuoted, [debugAdapterPath]);
312-
310+
const debugExecutable = new DebugAdapterExecutable(interpreterPathSpaces, [debugAdapterPath]);
313311
getInterpreterDetailsStub.resolves({ path: [interpreterPathSpaces] });
314312
const interpreterSpacePath: PythonEnvironment = createInterpreter(interpreterPathSpaces, '3.7.4-test');
315313
// Add architecture for completeness.
316314
(interpreterSpacePath as any).architecture = Architecture.Unknown;
317315
resolveEnvironmentStub.withArgs(interpreterPathSpaces).resolves(interpreterSpacePath);
318316
const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);
319-
320317
assert.deepStrictEqual(descriptor, debugExecutable);
321318
});
322319

323-
test('Add quotes to interpreter path with spaces when debugAdapterPath is specified', async () => {
320+
test('Should not add quotes to interpreter path with spaces when debugAdapterPath is specified', async () => {
324321
const customAdapterPath = 'custom/debug/adapter/customAdapterPath';
325322
const session = createSession({ debugAdapterPath: customAdapterPath });
326323
const interpreterPathSpaces = 'path/to/python interpreter with spaces';
327-
const interpreterPathSpacesQuoted = `"${interpreterPathSpaces}"`;
328-
const debugExecutable = new DebugAdapterExecutable(interpreterPathSpacesQuoted, [customAdapterPath]);
324+
const debugExecutable = new DebugAdapterExecutable(interpreterPathSpaces, [customAdapterPath]);
329325

330326
getInterpreterDetailsStub.resolves({ path: [interpreterPathSpaces] });
331327
const interpreterSpacePath: PythonEnvironment = createInterpreter(interpreterPathSpaces, '3.7.4-test');

0 commit comments

Comments
 (0)