Skip to content

Commit 926dc43

Browse files
andyleejordanandyleejordan
authored
Merge pull request #4121 from PowerShell/andschwa/exists-cleanup
Use `vscode.workspace.fs` and suppress startup banner for `dotnet` installs of PowerShell
2 parents add1ae8 + 86b7fe5 commit 926dc43

File tree

10 files changed

+278
-127
lines changed

10 files changed

+278
-127
lines changed

‎src/features/Examples.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export class ExamplesFeature implements vscode.Disposable {
1515
vscode.commands.executeCommand("vscode.openFolder", this.examplesPath, true);
1616
// Return existence of the path for testing. The `vscode.openFolder`
1717
// command should do this, but doesn't (yet).
18-
return utils.fileExists(this.examplesPath);
18+
return utils.checkIfFileExists(this.examplesPath);
1919
});
2020
}
2121

‎src/features/PesterTests.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ export class PesterTestsFeature implements vscode.Disposable {
134134
//
135135
// Ensure the necessary script exists (for testing). The debugger will
136136
// start regardless, but we also pass its success along.
137-
return utils.fileExists(this.invokePesterStubScriptPath)
137+
return utils.checkIfFileExists(this.invokePesterStubScriptPath)
138138
&& vscode.debug.startDebugging(vscode.workspace.workspaceFolders?.[0], launchConfig);
139139
}
140140
}

‎src/platform.ts‎

Lines changed: 44 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,25 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4-
import * as child_process from "child_process";
54
import * as fs from "fs";
65
import * as os from "os";
76
import * as path from "path";
87
import * as process from "process";
98
import { IPowerShellAdditionalExePathSettings } from "./settings";
9+
// This uses require so we can rewire it in unit tests!
10+
// tslint:disable-next-line:no-var-requires
11+
const utils = require("./utils")
1012

1113
const WindowsPowerShell64BitLabel = "Windows PowerShell (x64)";
1214
const WindowsPowerShell32BitLabel = "Windows PowerShell (x86)";
1315

14-
const LinuxExePath = "/usr/bin/pwsh";
16+
const LinuxExePath = "/usr/bin/pwsh";
1517
const LinuxPreviewExePath = "/usr/bin/pwsh-preview";
1618

17-
const SnapExePath = "/snap/bin/pwsh";
18-
const SnapPreviewExePath = "/snap/bin/pwsh-preview";
19+
const SnapExePath = "/snap/bin/pwsh";
20+
const SnapPreviewExePath = "/snap/bin/pwsh-preview";
1921

20-
const MacOSExePath = "/usr/local/bin/pwsh";
22+
const MacOSExePath = "/usr/local/bin/pwsh";
2123
const MacOSPreviewExePath = "/usr/local/bin/pwsh-preview";
2224

2325
export enum OperatingSystem {
@@ -36,6 +38,7 @@ export interface IPlatformDetails {
3638
export interface IPowerShellExeDetails {
3739
readonly displayName: string;
3840
readonly exePath: string;
41+
readonly supportsProperArguments: boolean;
3942
}
4043

4144
export function getPlatformDetails(): IPlatformDetails {
@@ -97,17 +100,21 @@ export class PowerShellExeFinder {
97100
/**
98101
* Returns the first available PowerShell executable found in the search order.
99102
*/
100-
public getFirstAvailablePowerShellInstallation(): IPowerShellExeDetails {
101-
for (const pwsh of this.enumeratePowerShellInstallations()) {
103+
public async getFirstAvailablePowerShellInstallation(): Promise<IPowerShellExeDetails> {
104+
for await (const pwsh of this.enumeratePowerShellInstallations()) {
102105
return pwsh;
103106
}
104107
}
105108

106109
/**
107110
* Get an array of all PowerShell executables found when searching for PowerShell installations.
108111
*/
109-
public getAllAvailablePowerShellInstallations(): IPowerShellExeDetails[] {
110-
return Array.from(this.enumeratePowerShellInstallations());
112+
public async getAllAvailablePowerShellInstallations(): Promise<IPowerShellExeDetails[]> {
113+
const array: IPowerShellExeDetails[] = [];
114+
for await (const pwsh of this.enumeratePowerShellInstallations()) {
115+
array.push(pwsh);
116+
}
117+
return array;
111118
}
112119

113120
/**
@@ -137,18 +144,18 @@ export class PowerShellExeFinder {
137144
* PowerShell items returned by this object are verified
138145
* to exist on the filesystem.
139146
*/
140-
public *enumeratePowerShellInstallations(): Iterable<IPowerShellExeDetails> {
147+
public async *enumeratePowerShellInstallations(): AsyncIterable<IPowerShellExeDetails> {
141148
// Get the default PowerShell installations first
142-
for (const defaultPwsh of this.enumerateDefaultPowerShellInstallations()) {
143-
if (defaultPwsh && defaultPwsh.exists()) {
149+
for await (const defaultPwsh of this.enumerateDefaultPowerShellInstallations()) {
150+
if (defaultPwsh && await defaultPwsh.exists()) {
144151
yield defaultPwsh;
145152
}
146153
}
147154

148155
// Also show any additionally configured PowerShells
149156
// These may be duplicates of the default installations, but given a different name.
150157
for (const additionalPwsh of this.enumerateAdditionalPowerShellInstallations()) {
151-
if (additionalPwsh && additionalPwsh.exists()) {
158+
if (additionalPwsh && await additionalPwsh.exists()) {
152159
yield additionalPwsh;
153160
}
154161
}
@@ -159,7 +166,7 @@ export class PowerShellExeFinder {
159166
* Returned values may not exist, but come with an .exists property
160167
* which will check whether the executable exists.
161168
*/
162-
private *enumerateDefaultPowerShellInstallations(): Iterable<IPossiblePowerShellExe> {
169+
private async *enumerateDefaultPowerShellInstallations(): AsyncIterable<IPossiblePowerShellExe> {
163170
// Find PSCore stable first
164171
yield this.findPSCoreStable();
165172

@@ -174,7 +181,7 @@ export class PowerShellExeFinder {
174181
yield this.findPSCoreWindowsInstallation({ useAlternateBitness: true });
175182

176183
// Also look for the MSIX/UWP installation
177-
yield this.findPSCoreMsix();
184+
yield await this.findPSCoreMsix();
178185

179186
break;
180187
}
@@ -213,7 +220,7 @@ export class PowerShellExeFinder {
213220
}
214221

215222
/**
216-
* Iterates through the configured additonal PowerShell executable locations,
223+
* Iterates through the configured additional PowerShell executable locations,
217224
* without checking for their existence.
218225
*/
219226
private *enumerateAdditionalPowerShellInstallations(): Iterable<IPossiblePowerShellExe> {
@@ -227,7 +234,7 @@ export class PowerShellExeFinder {
227234
}
228235
}
229236

230-
private findPSCoreStable(): IPossiblePowerShellExe {
237+
private async findPSCoreStable(): Promise<IPossiblePowerShellExe> {
231238
switch (this.platformDetails.operatingSystem) {
232239
case OperatingSystem.Linux:
233240
return new PossiblePowerShellExe(LinuxExePath, "PowerShell");
@@ -236,11 +243,11 @@ export class PowerShellExeFinder {
236243
return new PossiblePowerShellExe(MacOSExePath, "PowerShell");
237244

238245
case OperatingSystem.Windows:
239-
return this.findPSCoreWindowsInstallation();
246+
return await this.findPSCoreWindowsInstallation();
240247
}
241248
}
242249

243-
private findPSCorePreview(): IPossiblePowerShellExe {
250+
private async findPSCorePreview(): Promise<IPossiblePowerShellExe> {
244251
switch (this.platformDetails.operatingSystem) {
245252
case OperatingSystem.Linux:
246253
return new PossiblePowerShellExe(LinuxPreviewExePath, "PowerShell Preview");
@@ -249,7 +256,7 @@ export class PowerShellExeFinder {
249256
return new PossiblePowerShellExe(MacOSPreviewExePath, "PowerShell Preview");
250257

251258
case OperatingSystem.Windows:
252-
return this.findPSCoreWindowsInstallation({ findPreview: true });
259+
return await this.findPSCoreWindowsInstallation({ findPreview: true });
253260
}
254261
}
255262

@@ -260,10 +267,11 @@ export class PowerShellExeFinder {
260267

261268
const dotnetGlobalToolExePath: string = path.join(os.homedir(), ".dotnet", "tools", exeName);
262269

263-
return new PossiblePowerShellExe(dotnetGlobalToolExePath, ".NET Core PowerShell Global Tool");
270+
// The dotnet installed version of PowerShell does not support proper argument parsing, and so it fails with our multi-line startup banner.
271+
return new PossiblePowerShellExe(dotnetGlobalToolExePath, ".NET Core PowerShell Global Tool", undefined, false);
264272
}
265273

266-
private findPSCoreMsix({ findPreview }: { findPreview?: boolean } = {}): IPossiblePowerShellExe {
274+
private async findPSCoreMsix({ findPreview }: { findPreview?: boolean } = {}): Promise<IPossiblePowerShellExe> {
267275
// We can't proceed if there's no LOCALAPPDATA path
268276
if (!process.env.LOCALAPPDATA) {
269277
return null;
@@ -272,7 +280,7 @@ export class PowerShellExeFinder {
272280
// Find the base directory for MSIX application exe shortcuts
273281
const msixAppDir = path.join(process.env.LOCALAPPDATA, "Microsoft", "WindowsApps");
274282

275-
if (!fileExistsSync(msixAppDir)) {
283+
if (!await utils.checkIfDirectoryExists(msixAppDir)) {
276284
return null;
277285
}
278286

@@ -282,6 +290,7 @@ export class PowerShellExeFinder {
282290
: { pwshMsixDirRegex: PowerShellExeFinder.PwshMsixRegex, pwshMsixName: "PowerShell (Store)" };
283291

284292
// We should find only one such application, so return on the first one
293+
// TODO: Use VS Code async fs API for this.
285294
for (const subdir of fs.readdirSync(msixAppDir)) {
286295
if (pwshMsixDirRegex.test(subdir)) {
287296
const pwshMsixPath = path.join(msixAppDir, subdir, "pwsh.exe");
@@ -301,9 +310,9 @@ export class PowerShellExeFinder {
301310
return new PossiblePowerShellExe(SnapPreviewExePath, "PowerShell Preview Snap");
302311
}
303312

304-
private findPSCoreWindowsInstallation(
313+
private async findPSCoreWindowsInstallation(
305314
{ useAlternateBitness = false, findPreview = false }:
306-
{ useAlternateBitness?: boolean; findPreview?: boolean } = {}): IPossiblePowerShellExe {
315+
{ useAlternateBitness?: boolean; findPreview?: boolean } = {}): Promise<IPossiblePowerShellExe> {
307316

308317
const programFilesPath: string = this.getProgramFilesPath({ useAlternateBitness });
309318

@@ -314,13 +323,7 @@ export class PowerShellExeFinder {
314323
const powerShellInstallBaseDir = path.join(programFilesPath, "PowerShell");
315324

316325
// Ensure the base directory exists
317-
try {
318-
const powerShellInstallBaseDirLStat = fs.lstatSync(powerShellInstallBaseDir);
319-
if (!powerShellInstallBaseDirLStat.isDirectory())
320-
{
321-
return null;
322-
}
323-
} catch {
326+
if (!await utils.checkIfDirectoryExists(powerShellInstallBaseDir)) {
324327
return null;
325328
}
326329

@@ -366,7 +369,7 @@ export class PowerShellExeFinder {
366369

367370
// Now look for the file
368371
const exePath = path.join(powerShellInstallBaseDir, item, "pwsh.exe");
369-
if (!fs.existsSync(exePath)) {
372+
if (!await utils.checkIfFileExists(exePath)) {
370373
continue;
371374
}
372375

@@ -413,7 +416,7 @@ export class PowerShellExeFinder {
413416
displayName = WindowsPowerShell32BitLabel;
414417
}
415418

416-
winPS = new PossiblePowerShellExe(winPSPath, displayName, { knownToExist: true });
419+
winPS = new PossiblePowerShellExe(winPSPath, displayName, true);
417420

418421
if (useAlternateBitness) {
419422
this.alternateBitnessWinPS = winPS;
@@ -479,40 +482,20 @@ export function getWindowsSystemPowerShellPath(systemFolderName: string) {
479482
"powershell.exe");
480483
}
481484

482-
function fileExistsSync(filePath: string): boolean {
483-
try {
484-
// This will throw if the path does not exist,
485-
// and otherwise returns a value that we don't care about
486-
fs.lstatSync(filePath);
487-
return true;
488-
} catch {
489-
return false;
490-
}
491-
}
492-
493485
interface IPossiblePowerShellExe extends IPowerShellExeDetails {
494-
exists(): boolean;
486+
exists(): Promise<boolean>;
495487
}
496488

497489
class PossiblePowerShellExe implements IPossiblePowerShellExe {
498-
public readonly exePath: string;
499-
public readonly displayName: string;
500-
501-
private knownToExist: boolean;
502-
503490
constructor(
504-
pathToExe: string,
505-
installationName: string,
506-
{ knownToExist = false }: { knownToExist?: boolean } = {}) {
507-
508-
this.exePath = pathToExe;
509-
this.displayName = installationName;
510-
this.knownToExist = knownToExist || undefined;
511-
}
491+
public readonly exePath: string,
492+
public readonly displayName: string,
493+
private knownToExist?: boolean,
494+
public readonly supportsProperArguments: boolean = true) { }
512495

513-
public exists(): boolean {
496+
public async exists(): Promise<boolean> {
514497
if (this.knownToExist === undefined) {
515-
this.knownToExist = fileExistsSync(this.exePath);
498+
this.knownToExist = await utils.checkIfFileExists(this.exePath);
516499
}
517500
return this.knownToExist;
518501
}

‎src/process.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import vscode = require("vscode");
99
import { Logger } from "./logging";
1010
import Settings = require("./settings");
1111
import utils = require("./utils");
12-
import { IEditorServicesSessionDetails, SessionManager } from "./session";
12+
import { IEditorServicesSessionDetails } from "./session";
1313

1414
export class PowerShellProcess {
1515
public static escapeSingleQuotes(psPath: string): string {
@@ -83,12 +83,14 @@ export class PowerShellProcess {
8383
PowerShellProcess.escapeSingleQuotes(psesModulePath) +
8484
"'; Start-EditorServices " + this.startPsesArgs;
8585

86+
// On Windows we unfortunately can't Base64 encode the startup command
87+
// because it annoys some poorly implemented anti-virus scanners.
8688
if (utils.isWindows) {
8789
powerShellArgs.push(
8890
"-Command",
8991
startEditorServices);
9092
} else {
91-
// Use -EncodedCommand for better quote support on non-Windows
93+
// Otherwise use -EncodedCommand for better quote support.
9294
powerShellArgs.push(
9395
"-EncodedCommand",
9496
Buffer.from(startEditorServices, "utf16le").toString("base64"));

‎src/session.ts‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -148,9 +148,9 @@ export class SessionManager implements Middleware {
148148
this.migrateWhitespaceAroundPipeSetting();
149149

150150
try {
151-
let powerShellExeDetails;
151+
let powerShellExeDetails: IPowerShellExeDetails;
152152
if (this.sessionSettings.powerShellDefaultVersion) {
153-
for (const details of this.powershellExeFinder.enumeratePowerShellInstallations()) {
153+
for await (const details of this.powershellExeFinder.enumeratePowerShellInstallations()) {
154154
// Need to compare names case-insensitively, from https://stackoverflow.com/a/2140723
155155
const wantedName = this.sessionSettings.powerShellDefaultVersion;
156156
if (wantedName.localeCompare(details.displayName, undefined, { sensitivity: "accent" }) === 0) {
@@ -161,7 +161,7 @@ export class SessionManager implements Middleware {
161161
}
162162

163163
this.PowerShellExeDetails = powerShellExeDetails ||
164-
this.powershellExeFinder.getFirstAvailablePowerShellInstallation();
164+
await this.powershellExeFinder.getFirstAvailablePowerShellInstallation();
165165

166166
} catch (e) {
167167
this.log.writeError(`Error occurred while searching for a PowerShell executable:\n${e}`);
@@ -215,6 +215,13 @@ export class SessionManager implements Middleware {
215215

216216
if (this.sessionSettings.integratedConsole.suppressStartupBanner) {
217217
this.editorServicesArgs += "-StartupBanner '' ";
218+
} else if (utils.isWindows && !this.PowerShellExeDetails.supportsProperArguments) {
219+
// NOTE: On Windows we don't Base64 encode the startup command
220+
// because it annoys some poorly implemented anti-virus scanners.
221+
// Unfortunately this means that for some installs of PowerShell
222+
// (such as through the `dotnet` package manager), we can't include
223+
// a multi-line startup banner as the quotes break the command.
224+
this.editorServicesArgs += `-StartupBanner '${this.HostName} Extension v${this.HostVersion}' `;
218225
} else {
219226
const startupBanner = `${this.HostName} Extension v${this.HostVersion}
220227
Copyright (c) Microsoft Corporation.
@@ -463,7 +470,7 @@ Type 'help' to get help.
463470
private registerCommands(): void {
464471
this.registeredCommands = [
465472
vscode.commands.registerCommand("PowerShell.RestartSession", () => { this.restartSession(); }),
466-
vscode.commands.registerCommand(this.ShowSessionMenuCommandName, () => { this.showSessionMenu(); }),
473+
vscode.commands.registerCommand(this.ShowSessionMenuCommandName, async () => { await this.showSessionMenu(); }),
467474
vscode.workspace.onDidChangeConfiguration(async () => { await this.onConfigurationUpdated(); }),
468475
vscode.commands.registerCommand(
469476
"PowerShell.ShowSessionConsole", (isExecute?: boolean) => { this.showSessionConsole(isExecute); }),
@@ -795,8 +802,8 @@ Type 'help' to get help.
795802
}
796803
}
797804

798-
private showSessionMenu() {
799-
const availablePowerShellExes = this.powershellExeFinder.getAllAvailablePowerShellInstallations();
805+
private async showSessionMenu() {
806+
const availablePowerShellExes = await this.powershellExeFinder.getAllAvailablePowerShellInstallations();
800807

801808
let sessionText: string;
802809

0 commit comments

Comments
 (0)