Skip to content

Commit b208384

Browse files
author
Kartik Raj
authored
Add diagnostic to validate ComSpec (#20927)
Closes #16692
1 parent 5fd9f97 commit b208384

File tree

5 files changed

+369
-28
lines changed

5 files changed

+369
-28
lines changed

src/client/application/diagnostics/checks/pythonInterpreter.ts

Lines changed: 126 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { inject, injectable } from 'inversify';
66
import { DiagnosticSeverity, l10n } from 'vscode';
77
import '../../../common/extensions';
88
import * as path from 'path';
9-
import { IDisposableRegistry, IInterpreterPathService, Resource } from '../../../common/types';
9+
import { IConfigurationService, IDisposableRegistry, IInterpreterPathService, Resource } from '../../../common/types';
1010
import { IInterpreterService } from '../../../interpreter/contracts';
1111
import { IServiceContainer } from '../../../ioc/types';
1212
import { BaseDiagnostic, BaseDiagnosticsService } from '../base';
@@ -28,6 +28,12 @@ import { EventName } from '../../../telemetry/constants';
2828
import { IExtensionSingleActivationService } from '../../../activation/types';
2929
import { cache } from '../../../common/utils/decorators';
3030
import { noop } from '../../../common/utils/misc';
31+
import { getEnvironmentVariable, getOSType, OSType } from '../../../common/utils/platform';
32+
import { IFileSystem } from '../../../common/platform/types';
33+
import { traceError } from '../../../logging';
34+
import { getExecutable } from '../../../common/process/internal/python';
35+
import { getSearchPathEnvVarNames } from '../../../common/utils/exec';
36+
import { IProcessServiceFactory } from '../../../common/process/types';
3137

3238
const messages = {
3339
[DiagnosticCodes.NoPythonInterpretersDiagnostic]: l10n.t(
@@ -36,6 +42,15 @@ const messages = {
3642
[DiagnosticCodes.InvalidPythonInterpreterDiagnostic]: l10n.t(
3743
'An Invalid Python interpreter is selected{0}, please try changing it to enable features such as IntelliSense, linting, and debugging. See output for more details regarding why the interpreter is invalid.',
3844
),
45+
[DiagnosticCodes.InvalidComspecDiagnostic]: l10n.t(
46+
'We detected an issue with one of your environment variables that breaks features such as IntelliSense, linting and debugging. Try setting the "ComSpec" variable to a valid Command Prompt path in your system to fix it.',
47+
),
48+
[DiagnosticCodes.IncompletePathVarDiagnostic]: l10n.t(
49+
'We detected an issue with "Path" environment variable that breaks features such as IntelliSense, linting and debugging. Please edit it to make sure it contains the "SystemRoot" subdirectories.',
50+
),
51+
[DiagnosticCodes.DefaultShellErrorDiagnostic]: l10n.t(
52+
'We detected an issue with your default shell that breaks features such as IntelliSense, linting and debugging. Try resetting "ComSpec" and "Path" environment variables to fix it.',
53+
),
3954
};
4055

4156
export class InvalidPythonInterpreterDiagnostic extends BaseDiagnostic {
@@ -61,6 +76,17 @@ export class InvalidPythonInterpreterDiagnostic extends BaseDiagnostic {
6176
}
6277
}
6378

79+
type DefaultShellDiagnostics =
80+
| DiagnosticCodes.InvalidComspecDiagnostic
81+
| DiagnosticCodes.IncompletePathVarDiagnostic
82+
| DiagnosticCodes.DefaultShellErrorDiagnostic;
83+
84+
export class DefaultShellDiagnostic extends BaseDiagnostic {
85+
constructor(code: DefaultShellDiagnostics, resource: Resource, scope = DiagnosticScope.Global) {
86+
super(code, messages[code], DiagnosticSeverity.Error, scope, resource, undefined, 'always');
87+
}
88+
}
89+
6490
export const InvalidPythonInterpreterServiceId = 'InvalidPythonInterpreterServiceId';
6591

6692
@injectable()
@@ -73,7 +99,13 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService
7399
@inject(IDisposableRegistry) disposableRegistry: IDisposableRegistry,
74100
) {
75101
super(
76-
[DiagnosticCodes.NoPythonInterpretersDiagnostic, DiagnosticCodes.InvalidPythonInterpreterDiagnostic],
102+
[
103+
DiagnosticCodes.NoPythonInterpretersDiagnostic,
104+
DiagnosticCodes.InvalidPythonInterpreterDiagnostic,
105+
DiagnosticCodes.InvalidComspecDiagnostic,
106+
DiagnosticCodes.IncompletePathVarDiagnostic,
107+
DiagnosticCodes.DefaultShellErrorDiagnostic,
108+
],
77109
serviceContainer,
78110
disposableRegistry,
79111
false,
@@ -95,14 +127,17 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService
95127
);
96128
}
97129

98-
// eslint-disable-next-line class-methods-use-this
99-
public async diagnose(_resource: Resource): Promise<IDiagnostic[]> {
100-
return [];
130+
public async diagnose(resource: Resource): Promise<IDiagnostic[]> {
131+
return this.diagnoseDefaultShell(resource);
101132
}
102133

103134
public async _manualDiagnose(resource: Resource): Promise<IDiagnostic[]> {
104135
const workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
105136
const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
137+
const diagnostics = await this.diagnoseDefaultShell(resource);
138+
if (diagnostics.length > 0) {
139+
return diagnostics;
140+
}
106141
const hasInterpreters = await interpreterService.hasInterpreters();
107142
const interpreterPathService = this.serviceContainer.get<IInterpreterPathService>(IInterpreterPathService);
108143
const isInterpreterSetToDefault = interpreterPathService.get(resource) === 'python';
@@ -140,6 +175,72 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService
140175
return false;
141176
}
142177

178+
private async diagnoseDefaultShell(resource: Resource): Promise<IDiagnostic[]> {
179+
await this.isPathVarIncomplete();
180+
if (getOSType() !== OSType.Windows) {
181+
return [];
182+
}
183+
const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
184+
const currentInterpreter = await interpreterService.getActiveInterpreter(resource);
185+
if (currentInterpreter) {
186+
return [];
187+
}
188+
try {
189+
await this.shellExecPython();
190+
} catch (ex) {
191+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
192+
if ((ex as any).errno === -4058) {
193+
// ENOENT (-4058) error is thrown by Node when the default shell is invalid.
194+
traceError('ComSpec is likely set to an invalid value', getEnvironmentVariable('ComSpec'));
195+
if (await this.isComspecInvalid()) {
196+
return [new DefaultShellDiagnostic(DiagnosticCodes.InvalidComspecDiagnostic, resource)];
197+
}
198+
if (this.isPathVarIncomplete()) {
199+
traceError('PATH env var appears to be incomplete', process.env.Path, process.env.PATH);
200+
return [new DefaultShellDiagnostic(DiagnosticCodes.IncompletePathVarDiagnostic, resource)];
201+
}
202+
return [new DefaultShellDiagnostic(DiagnosticCodes.DefaultShellErrorDiagnostic, resource)];
203+
}
204+
}
205+
return [];
206+
}
207+
208+
private async isComspecInvalid() {
209+
const comSpec = getEnvironmentVariable('ComSpec') ?? '';
210+
const fs = this.serviceContainer.get<IFileSystem>(IFileSystem);
211+
return fs.fileExists(comSpec).then((exists) => !exists);
212+
}
213+
214+
// eslint-disable-next-line class-methods-use-this
215+
private isPathVarIncomplete() {
216+
const envVars = getSearchPathEnvVarNames();
217+
const systemRoot = getEnvironmentVariable('SystemRoot') ?? 'C:\\WINDOWS';
218+
for (const envVar of envVars) {
219+
const value = getEnvironmentVariable(envVar);
220+
if (value?.includes(systemRoot)) {
221+
return false;
222+
}
223+
}
224+
return true;
225+
}
226+
227+
@cache(-1, true)
228+
// eslint-disable-next-line class-methods-use-this
229+
private async shellExecPython() {
230+
const configurationService = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
231+
const { pythonPath } = configurationService.getSettings();
232+
const [args] = getExecutable();
233+
const argv = [pythonPath, ...args];
234+
// Concat these together to make a set of quoted strings
235+
const quoted = argv.reduce(
236+
(p, c) => (p ? `${p} ${c.toCommandArgumentForPythonExt()}` : `${c.toCommandArgumentForPythonExt()}`),
237+
'',
238+
);
239+
const processServiceFactory = this.serviceContainer.get<IProcessServiceFactory>(IProcessServiceFactory);
240+
const service = await processServiceFactory.create();
241+
return service.shellExec(quoted, { timeout: 15000 });
242+
}
243+
143244
@cache(1000, true) // This is to handle throttling of multiple events.
144245
protected async onHandle(diagnostics: IDiagnostic[]): Promise<void> {
145246
if (diagnostics.length === 0) {
@@ -163,6 +264,26 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService
163264

164265
private getCommandPrompts(diagnostic: IDiagnostic): { prompt: string; command?: IDiagnosticCommand }[] {
165266
const commandFactory = this.serviceContainer.get<IDiagnosticsCommandFactory>(IDiagnosticsCommandFactory);
267+
if (
268+
diagnostic.code === DiagnosticCodes.InvalidComspecDiagnostic ||
269+
diagnostic.code === DiagnosticCodes.IncompletePathVarDiagnostic ||
270+
diagnostic.code === DiagnosticCodes.DefaultShellErrorDiagnostic
271+
) {
272+
const links: Record<DefaultShellDiagnostics, string> = {
273+
InvalidComspecDiagnostic: 'https://aka.ms/AAk3djo',
274+
IncompletePathVarDiagnostic: 'https://aka.ms/AAk744c',
275+
DefaultShellErrorDiagnostic: 'https://aka.ms/AAk7qix',
276+
};
277+
return [
278+
{
279+
prompt: Common.seeInstructions,
280+
command: commandFactory.createCommand(diagnostic, {
281+
type: 'launch',
282+
options: links[diagnostic.code],
283+
}),
284+
},
285+
];
286+
}
166287
const prompts = [
167288
{
168289
prompt: Common.selectPythonInterpreter,

src/client/application/diagnostics/constants.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ export enum DiagnosticCodes {
1212
InvalidPythonPathInDebuggerLaunchDiagnostic = 'InvalidPythonPathInDebuggerLaunchDiagnostic',
1313
EnvironmentActivationInPowerShellWithBatchFilesNotSupportedDiagnostic = 'EnvironmentActivationInPowerShellWithBatchFilesNotSupportedDiagnostic',
1414
InvalidPythonInterpreterDiagnostic = 'InvalidPythonInterpreterDiagnostic',
15+
InvalidComspecDiagnostic = 'InvalidComspecDiagnostic',
16+
IncompletePathVarDiagnostic = 'IncompletePathVarDiagnostic',
17+
DefaultShellErrorDiagnostic = 'DefaultShellErrorDiagnostic',
1518
LSNotSupportedDiagnostic = 'LSNotSupportedDiagnostic',
1619
PythonPathDeprecatedDiagnostic = 'PythonPathDeprecatedDiagnostic',
1720
JustMyCodeDiagnostic = 'JustMyCodeDiagnostic',

src/client/common/utils/localize.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ export namespace Diagnostics {
4949

5050
export namespace Common {
5151
export const allow = l10n.t('Allow');
52+
export const seeInstructions = l10n.t('See Instructions');
5253
export const close = l10n.t('Close');
5354
export const bannerLabelYes = l10n.t('Yes');
5455
export const bannerLabelNo = l10n.t('No');

src/client/pythonEnvironments/info/executable.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,7 @@ import { copyPythonExecInfo, PythonExecInfo } from '../exec';
1414
* @param python - the information to use when running Python
1515
* @param shellExec - the function to use to run Python
1616
*/
17-
export async function getExecutablePath(
18-
python: PythonExecInfo,
19-
shellExec: ShellExecFunc,
20-
timeout?: number,
21-
): Promise<string | undefined> {
17+
export async function getExecutablePath(python: PythonExecInfo, shellExec: ShellExecFunc): Promise<string | undefined> {
2218
try {
2319
const [args, parse] = getExecutable();
2420
const info = copyPythonExecInfo(python, args);
@@ -28,7 +24,7 @@ export async function getExecutablePath(
2824
(p, c) => (p ? `${p} ${c.toCommandArgumentForPythonExt()}` : `${c.toCommandArgumentForPythonExt()}`),
2925
'',
3026
);
31-
const result = await shellExec(quoted, { timeout: timeout ?? 15000 });
27+
const result = await shellExec(quoted, { timeout: 15000 });
3228
const executable = parse(result.stdout.trim());
3329
if (executable === '') {
3430
throw new Error(`${quoted} resulted in empty stdout`);

0 commit comments

Comments
 (0)