Skip to content

Commit 5a8fdf4

Browse files
author
Kartik Raj
authored
Fix debugging when using "internalConsole" (#21068)
Closes #20828 Do case-insensitive merge of environment variables, always make sure to return the standard env key for an OS, similar to `process.env`.
1 parent f531bb8 commit 5a8fdf4

5 files changed

Lines changed: 83 additions & 23 deletions

File tree

‎src/client/common/platform/fs-paths.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,11 @@ export class FileSystemPathUtils implements IFileSystemPathUtils {
145145
}
146146

147147
export function normCasePath(filePath: string): string {
148-
return getOSType() === OSType.Windows ? nodepath.normalize(filePath).toUpperCase() : nodepath.normalize(filePath);
148+
return normCase(nodepath.normalize(filePath));
149+
}
150+
151+
export function normCase(s: string): string {
152+
return getOSType() === OSType.Windows ? s.toUpperCase() : s;
149153
}
150154

151155
/**

‎src/client/common/variables/environment.ts‎

Lines changed: 56 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { EventName } from '../../telemetry/constants';
1010
import { IFileSystem } from '../platform/types';
1111
import { IPathUtils } from '../types';
1212
import { EnvironmentVariables, IEnvironmentVariablesService } from './types';
13+
import { normCase } from '../platform/fs-paths';
1314

1415
@injectable()
1516
export class EnvironmentVariablesService implements IEnvironmentVariablesService {
@@ -61,6 +62,9 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService
6162
if (!target) {
6263
return;
6364
}
65+
const reference = target;
66+
target = normCaseKeys(target);
67+
source = normCaseKeys(source);
6468
const settingsNotToMerge = ['PYTHONPATH', this.pathVariable];
6569
Object.keys(source).forEach((setting) => {
6670
if (settingsNotToMerge.indexOf(setting) >= 0) {
@@ -70,6 +74,8 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService
7074
target[setting] = source[setting];
7175
}
7276
});
77+
restoreKeys(target);
78+
matchTarget(reference, target);
7379
}
7480

7581
public appendPythonPath(vars: EnvironmentVariables, ...pythonPaths: string[]) {
@@ -80,18 +86,24 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService
8086
return this.appendPaths(vars, this.pathVariable, ...paths);
8187
}
8288

83-
private get pathVariable(): 'Path' | 'PATH' {
89+
private get pathVariable(): string {
8490
if (!this._pathVariable) {
8591
this._pathVariable = this.pathUtils.getPathVariableName();
8692
}
87-
return this._pathVariable!;
93+
return normCase(this._pathVariable)!;
8894
}
8995

90-
private appendPaths(
91-
vars: EnvironmentVariables,
92-
variableName: 'PATH' | 'Path' | 'PYTHONPATH',
93-
...pathsToAppend: string[]
94-
) {
96+
private appendPaths(vars: EnvironmentVariables, variableName: string, ...pathsToAppend: string[]) {
97+
const reference = vars;
98+
vars = normCaseKeys(vars);
99+
variableName = normCase(variableName);
100+
vars = this._appendPaths(vars, variableName, ...pathsToAppend);
101+
restoreKeys(vars);
102+
matchTarget(reference, vars);
103+
return vars;
104+
}
105+
106+
private _appendPaths(vars: EnvironmentVariables, variableName: string, ...pathsToAppend: string[]) {
95107
const valueToAppend = pathsToAppend
96108
.filter((item) => typeof item === 'string' && item.trim().length > 0)
97109
.map((item) => item.trim())
@@ -183,3 +195,40 @@ function substituteEnvVars(
183195

184196
return value.replace(/\\\$/g, '$');
185197
}
198+
199+
export function normCaseKeys(env: EnvironmentVariables): EnvironmentVariables {
200+
const normalizedEnv: EnvironmentVariables = {};
201+
Object.keys(env).forEach((key) => {
202+
const normalizedKey = normCase(key);
203+
normalizedEnv[normalizedKey] = env[key];
204+
});
205+
return normalizedEnv;
206+
}
207+
208+
export function restoreKeys(env: EnvironmentVariables) {
209+
const processEnvKeys = Object.keys(process.env);
210+
processEnvKeys.forEach((processEnvKey) => {
211+
const originalKey = normCase(processEnvKey);
212+
if (originalKey !== processEnvKey && env[originalKey] !== undefined) {
213+
env[processEnvKey] = env[originalKey];
214+
delete env[originalKey];
215+
}
216+
});
217+
}
218+
219+
export function matchTarget(reference: EnvironmentVariables, target: EnvironmentVariables): void {
220+
Object.keys(reference).forEach((key) => {
221+
if (target.hasOwnProperty(key)) {
222+
reference[key] = target[key];
223+
} else {
224+
delete reference[key];
225+
}
226+
});
227+
228+
// Add any new keys from target to reference
229+
Object.keys(target).forEach((key) => {
230+
if (!reference.hasOwnProperty(key)) {
231+
reference[key] = target[key];
232+
}
233+
});
234+
}

‎src/client/debugger/extension/configuration/resolvers/helper.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,10 @@ export class DebugEnvironmentVariablesHelper implements IDebugEnvironmentVariabl
4848
}
4949

5050
// Append the PYTHONPATH and PATH variables.
51-
this.envParser.appendPath(env, debugLaunchEnvVars[pathVariableName]);
51+
this.envParser.appendPath(
52+
env,
53+
debugLaunchEnvVars[pathVariableName] ?? debugLaunchEnvVars[pathVariableName.toUpperCase()],
54+
);
5255
this.envParser.appendPythonPath(env, debugLaunchEnvVars.PYTHONPATH);
5356

5457
if (typeof env[pathVariableName] === 'string' && env[pathVariableName]!.length > 0) {

‎src/test/common/variables/envVarsService.unit.test.ts‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,16 @@ import * as TypeMoq from 'typemoq';
1010
import { IFileSystem } from '../../../client/common/platform/types';
1111
import { IPathUtils } from '../../../client/common/types';
1212
import { EnvironmentVariablesService, parseEnvFile } from '../../../client/common/variables/environment';
13+
import { getSearchPathEnvVarNames } from '../../../client/common/utils/exec';
1314

1415
use(chaiAsPromised);
1516

1617
type PathVar = 'Path' | 'PATH';
17-
const PATHS = [
18-
'Path', // Windows
19-
'PATH', // non-Windows
20-
];
18+
const PATHS = getSearchPathEnvVarNames();
2119

22-
suite('Environment Variables Service', () => {
20+
suite('xEnvironment Variables Service', () => {
2321
const filename = 'x/y/z/.env';
22+
const processEnvPath = getSearchPathEnvVarNames()[0];
2423
let pathUtils: TypeMoq.IMock<IPathUtils>;
2524
let fs: TypeMoq.IMock<IFileSystem>;
2625
let variablesService: EnvironmentVariablesService;
@@ -208,7 +207,7 @@ PYTHON=${BINDIR}/python3\n\
208207
expect(vars2).to.have.property('TWO', 'TWO', 'Incorrect value');
209208
expect(vars2).to.have.property('THREE', '3', 'Variable not merged');
210209
expect(vars2).to.have.property('PYTHONPATH', 'PYTHONPATH', 'Incorrect value');
211-
expect(vars2).to.have.property(pathVariable, 'PATH', 'Incorrect value');
210+
expect(vars2).to.have.property(processEnvPath, 'PATH', 'Incorrect value');
212211
verifyAll();
213212
});
214213

@@ -226,7 +225,7 @@ PYTHON=${BINDIR}/python3\n\
226225
expect(target).to.have.property('TWO', 'TWO', 'Incorrect value');
227226
expect(target).to.have.property('THREE', '3', 'Variable not merged');
228227
expect(target).to.have.property('PYTHONPATH', 'PYTHONPATH', 'Incorrect value');
229-
expect(target).to.have.property(pathVariable, 'PATH', 'Incorrect value');
228+
expect(target).to.have.property(processEnvPath, 'PATH', 'Incorrect value');
230229
verifyAll();
231230
});
232231
});
@@ -266,17 +265,17 @@ PYTHON=${BINDIR}/python3\n\
266265
variablesService.appendPath(vars);
267266
expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables');
268267
expect(vars).to.have.property('ONE', '1', 'Incorrect value');
269-
expect(vars).to.have.property(pathVariable, 'PATH', 'Incorrect value');
268+
expect(vars).to.have.property(processEnvPath, 'PATH', 'Incorrect value');
270269

271270
variablesService.appendPath(vars, '');
272271
expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables');
273272
expect(vars).to.have.property('ONE', '1', 'Incorrect value');
274-
expect(vars).to.have.property(pathVariable, 'PATH', 'Incorrect value');
273+
expect(vars).to.have.property(processEnvPath, 'PATH', 'Incorrect value');
275274

276275
variablesService.appendPath(vars, ' ', '');
277276
expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables');
278277
expect(vars).to.have.property('ONE', '1', 'Incorrect value');
279-
expect(vars).to.have.property(pathVariable, 'PATH', 'Incorrect value');
278+
expect(vars).to.have.property(processEnvPath, 'PATH', 'Incorrect value');
280279

281280
verifyAll();
282281
});
@@ -291,7 +290,11 @@ PYTHON=${BINDIR}/python3\n\
291290

292291
expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables');
293292
expect(vars).to.have.property('ONE', '1', 'Incorrect value');
294-
expect(vars).to.have.property(pathVariable, `PATH${path.delimiter}${pathToAppend}`, 'Incorrect value');
293+
expect(vars).to.have.property(
294+
processEnvPath,
295+
`PATH${path.delimiter}${pathToAppend}`,
296+
'Incorrect value',
297+
);
295298
verifyAll();
296299
});
297300
});

‎src/test/debugger/envVars.test.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { ConsoleType, LaunchRequestArguments } from '../../client/debugger/types
1515
import { isOs, OSType } from '../common';
1616
import { closeActiveWindows, initialize, initializeTest, IS_MULTI_ROOT_TEST, TEST_DEBUGGER } from '../initialize';
1717
import { UnitTestIocContainer } from '../testing/serviceRegistry';
18+
import { normCase } from '../../client/common/platform/fs-paths';
1819

1920
use(chaiAsPromised);
2021

@@ -109,9 +110,9 @@ suite('Resolving Environment Variables when Debugging', () => {
109110
});
110111

111112
async function testJsonEnvVariables(console: ConsoleType, expectedNumberOfVariables: number) {
112-
const prop1 = shortid.generate();
113-
const prop2 = shortid.generate();
114-
const prop3 = shortid.generate();
113+
const prop1 = normCase(shortid.generate());
114+
const prop2 = normCase(shortid.generate());
115+
const prop3 = normCase(shortid.generate());
115116
const env: Record<string, string> = {};
116117
env[prop1] = prop1;
117118
env[prop2] = prop2;

0 commit comments

Comments
 (0)