Skip to content

Fix debugging when using "internalConsole" #21068

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/client/common/platform/fs-paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,11 @@ export class FileSystemPathUtils implements IFileSystemPathUtils {
}

export function normCasePath(filePath: string): string {
return getOSType() === OSType.Windows ? nodepath.normalize(filePath).toUpperCase() : nodepath.normalize(filePath);
return normCase(nodepath.normalize(filePath));
}

export function normCase(s: string): string {
return getOSType() === OSType.Windows ? s.toUpperCase() : s;
}

/**
Expand Down
63 changes: 56 additions & 7 deletions src/client/common/variables/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { EventName } from '../../telemetry/constants';
import { IFileSystem } from '../platform/types';
import { IPathUtils } from '../types';
import { EnvironmentVariables, IEnvironmentVariablesService } from './types';
import { normCase } from '../platform/fs-paths';

@injectable()
export class EnvironmentVariablesService implements IEnvironmentVariablesService {
Expand Down Expand Up @@ -61,6 +62,9 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService
if (!target) {
return;
}
const reference = target;
target = normCaseKeys(target);
source = normCaseKeys(source);
const settingsNotToMerge = ['PYTHONPATH', this.pathVariable];
Object.keys(source).forEach((setting) => {
if (settingsNotToMerge.indexOf(setting) >= 0) {
Expand All @@ -70,6 +74,8 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService
target[setting] = source[setting];
}
});
restoreKeys(target);
matchTarget(reference, target);
}

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

private get pathVariable(): 'Path' | 'PATH' {
private get pathVariable(): string {
if (!this._pathVariable) {
this._pathVariable = this.pathUtils.getPathVariableName();
}
return this._pathVariable!;
return normCase(this._pathVariable)!;
}

private appendPaths(
vars: EnvironmentVariables,
variableName: 'PATH' | 'Path' | 'PYTHONPATH',
...pathsToAppend: string[]
) {
private appendPaths(vars: EnvironmentVariables, variableName: string, ...pathsToAppend: string[]) {
const reference = vars;
vars = normCaseKeys(vars);
variableName = normCase(variableName);
vars = this._appendPaths(vars, variableName, ...pathsToAppend);
restoreKeys(vars);
matchTarget(reference, vars);
return vars;
}

private _appendPaths(vars: EnvironmentVariables, variableName: string, ...pathsToAppend: string[]) {
const valueToAppend = pathsToAppend
.filter((item) => typeof item === 'string' && item.trim().length > 0)
.map((item) => item.trim())
Expand Down Expand Up @@ -183,3 +195,40 @@ function substituteEnvVars(

return value.replace(/\\\$/g, '$');
}

export function normCaseKeys(env: EnvironmentVariables): EnvironmentVariables {
const normalizedEnv: EnvironmentVariables = {};
Object.keys(env).forEach((key) => {
const normalizedKey = normCase(key);
normalizedEnv[normalizedKey] = env[key];
});
return normalizedEnv;
}

export function restoreKeys(env: EnvironmentVariables) {
const processEnvKeys = Object.keys(process.env);
processEnvKeys.forEach((processEnvKey) => {
const originalKey = normCase(processEnvKey);
if (originalKey !== processEnvKey && env[originalKey] !== undefined) {
env[processEnvKey] = env[originalKey];
delete env[originalKey];
}
});
}

export function matchTarget(reference: EnvironmentVariables, target: EnvironmentVariables): void {
Object.keys(reference).forEach((key) => {
if (target.hasOwnProperty(key)) {
reference[key] = target[key];
} else {
delete reference[key];
}
});

// Add any new keys from target to reference
Object.keys(target).forEach((key) => {
if (!reference.hasOwnProperty(key)) {
reference[key] = target[key];
}
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ export class DebugEnvironmentVariablesHelper implements IDebugEnvironmentVariabl
}

// Append the PYTHONPATH and PATH variables.
this.envParser.appendPath(env, debugLaunchEnvVars[pathVariableName]);
this.envParser.appendPath(
env,
debugLaunchEnvVars[pathVariableName] ?? debugLaunchEnvVars[pathVariableName.toUpperCase()],
);
this.envParser.appendPythonPath(env, debugLaunchEnvVars.PYTHONPATH);

if (typeof env[pathVariableName] === 'string' && env[pathVariableName]!.length > 0) {
Expand Down
25 changes: 14 additions & 11 deletions src/test/common/variables/envVarsService.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,16 @@ import * as TypeMoq from 'typemoq';
import { IFileSystem } from '../../../client/common/platform/types';
import { IPathUtils } from '../../../client/common/types';
import { EnvironmentVariablesService, parseEnvFile } from '../../../client/common/variables/environment';
import { getSearchPathEnvVarNames } from '../../../client/common/utils/exec';

use(chaiAsPromised);

type PathVar = 'Path' | 'PATH';
const PATHS = [
'Path', // Windows
'PATH', // non-Windows
];
const PATHS = getSearchPathEnvVarNames();

suite('Environment Variables Service', () => {
suite('xEnvironment Variables Service', () => {
const filename = 'x/y/z/.env';
const processEnvPath = getSearchPathEnvVarNames()[0];
let pathUtils: TypeMoq.IMock<IPathUtils>;
let fs: TypeMoq.IMock<IFileSystem>;
let variablesService: EnvironmentVariablesService;
Expand Down Expand Up @@ -208,7 +207,7 @@ PYTHON=${BINDIR}/python3\n\
expect(vars2).to.have.property('TWO', 'TWO', 'Incorrect value');
expect(vars2).to.have.property('THREE', '3', 'Variable not merged');
expect(vars2).to.have.property('PYTHONPATH', 'PYTHONPATH', 'Incorrect value');
expect(vars2).to.have.property(pathVariable, 'PATH', 'Incorrect value');
expect(vars2).to.have.property(processEnvPath, 'PATH', 'Incorrect value');
verifyAll();
});

Expand All @@ -226,7 +225,7 @@ PYTHON=${BINDIR}/python3\n\
expect(target).to.have.property('TWO', 'TWO', 'Incorrect value');
expect(target).to.have.property('THREE', '3', 'Variable not merged');
expect(target).to.have.property('PYTHONPATH', 'PYTHONPATH', 'Incorrect value');
expect(target).to.have.property(pathVariable, 'PATH', 'Incorrect value');
expect(target).to.have.property(processEnvPath, 'PATH', 'Incorrect value');
verifyAll();
});
});
Expand Down Expand Up @@ -266,17 +265,17 @@ PYTHON=${BINDIR}/python3\n\
variablesService.appendPath(vars);
expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables');
expect(vars).to.have.property('ONE', '1', 'Incorrect value');
expect(vars).to.have.property(pathVariable, 'PATH', 'Incorrect value');
expect(vars).to.have.property(processEnvPath, 'PATH', 'Incorrect value');

variablesService.appendPath(vars, '');
expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables');
expect(vars).to.have.property('ONE', '1', 'Incorrect value');
expect(vars).to.have.property(pathVariable, 'PATH', 'Incorrect value');
expect(vars).to.have.property(processEnvPath, 'PATH', 'Incorrect value');

variablesService.appendPath(vars, ' ', '');
expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables');
expect(vars).to.have.property('ONE', '1', 'Incorrect value');
expect(vars).to.have.property(pathVariable, 'PATH', 'Incorrect value');
expect(vars).to.have.property(processEnvPath, 'PATH', 'Incorrect value');

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

expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables');
expect(vars).to.have.property('ONE', '1', 'Incorrect value');
expect(vars).to.have.property(pathVariable, `PATH${path.delimiter}${pathToAppend}`, 'Incorrect value');
expect(vars).to.have.property(
processEnvPath,
`PATH${path.delimiter}${pathToAppend}`,
'Incorrect value',
);
verifyAll();
});
});
Expand Down
7 changes: 4 additions & 3 deletions src/test/debugger/envVars.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { ConsoleType, LaunchRequestArguments } from '../../client/debugger/types
import { isOs, OSType } from '../common';
import { closeActiveWindows, initialize, initializeTest, IS_MULTI_ROOT_TEST, TEST_DEBUGGER } from '../initialize';
import { UnitTestIocContainer } from '../testing/serviceRegistry';
import { normCase } from '../../client/common/platform/fs-paths';

use(chaiAsPromised);

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

async function testJsonEnvVariables(console: ConsoleType, expectedNumberOfVariables: number) {
const prop1 = shortid.generate();
const prop2 = shortid.generate();
const prop3 = shortid.generate();
const prop1 = normCase(shortid.generate());
const prop2 = normCase(shortid.generate());
const prop3 = normCase(shortid.generate());
const env: Record<string, string> = {};
env[prop1] = prop1;
env[prop2] = prop2;
Expand Down