Skip to content

WIP: Add support for side-by-side PS Core preview on Linux/macOS #1366

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.

Sign up for GitHub

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 6 commits into from
Jul 2, 2018
Merged

WIP: Add support for side-by-side PS Core preview on Linux/macOS #1366

Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b64259c
Add support for side-by-side PS Core preview on Linux/macOS
rkeithhill Jun 14, 2018
65d5a56
Adjust name of default PS Core on Linux/macOS
rkeithhill Jun 14, 2018
2ef97e4
Make a bit DRY-er. Thx @tylerl0706
rkeithhill Jun 14, 2018
fc22305
Remove support for pre-ship PS Core Linux/macOS binary name - powershell
rkeithhill Jun 19, 2018
d85f943
Add support for PowerShell Core installed in a Snap
rkeithhill Jun 28, 2018
7545dfd
Simplify impl of getting available PS on Linux/macOS
rkeithhill Jul 1, 2018
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
59 changes: 49 additions & 10 deletions src/platform.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@
*--------------------------------------------------------*/

import fs = require("fs");
import os = require("os");
import path = require("path");
import process = require("process");
import vscode = require("vscode");
import Settings = require("./settings");

const linuxExePath = "/usr/bin/pwsh";
const linuxPreviewExePath = "/usr/bin/pwsh-preview";

const macOSExePath = "/usr/local/bin/pwsh";
const macOSPreviewExePath = "/usr/local/bin/pwsh-preview";

export enum OperatingSystem {
Unknown,
Windows,
Expand Down Expand Up @@ -47,6 +51,13 @@ export function getPlatformDetails(): IPlatformDetails {
};
}

/**
* Gets the default instance of PowerShell for the specified platform.
* On Windows, the default version of PowerShell is "Windows PowerShell".
* @param platformDetails Specifies information about the platform - primarily the operating system.
* @param use32Bit On Windows, this boolean determines will find the 32-bit version of Windows PowerShell.
* @returns A string containing the path of the default version of PowerShell.
*/
export function getDefaultPowerShellPath(
platformDetails: IPlatformDetails,
use32Bit: boolean = false): string | null {
Expand All @@ -68,14 +79,16 @@ export function getDefaultPowerShellPath(
: SysnativePowerShellPath;
}
} else if (platformDetails.operatingSystem === OperatingSystem.MacOS) {
powerShellExePath = "/usr/local/bin/powershell";
if (fs.existsSync("/usr/local/bin/pwsh")) {
powerShellExePath = "/usr/local/bin/pwsh";
// Always default to the stable version of PowerShell (if installed) but handle case of only Preview installed
powerShellExePath = macOSExePath;
if (!fs.existsSync(macOSExePath) && fs.existsSync(macOSPreviewExePath)) {
powerShellExePath = macOSPreviewExePath;
}
} else if (platformDetails.operatingSystem === OperatingSystem.Linux) {
powerShellExePath = "/usr/bin/powershell";
if (fs.existsSync("/usr/bin/pwsh")) {
powerShellExePath = "/usr/bin/pwsh";
// Always default to the stable version of PowerShell (if installed) but handle case of only Preview installed
powerShellExePath = linuxExePath;
if (!fs.existsSync(linuxExePath) && fs.existsSync(linuxPreviewExePath)) {
powerShellExePath = linuxPreviewExePath;
}
}

Expand Down Expand Up @@ -108,6 +121,12 @@ export function fixWindowsPowerShellPath(powerShellExePath: string, platformDeta
return powerShellExePath;
}

/**
* Gets a list of all available PowerShell instance on the specified platform.
* @param platformDetails Specifies information about the platform - primarily the operating system.
* @param sessionSettings Specifies the user/workspace settings. Additional PowerShell exe paths loaded from settings.
* @returns An array of IPowerShellExeDetails objects with the PowerShell name & exe path for each instance found.
*/
export function getAvailablePowerShellExes(
platformDetails: IPlatformDetails,
sessionSettings: Settings.ISettings | undefined): IPowerShellExeDetails[] {
Expand Down Expand Up @@ -162,10 +181,30 @@ export function getAvailablePowerShellExes(
}
} else {
// Handle Linux and macOS case
const defaultExePath = this.getDefaultPowerShellPath(platformDetails);
paths.push({
versionName: "PowerShell Core",
exePath: this.getDefaultPowerShellPath(platformDetails),
versionName: "PowerShell Core" + (/-preview/.test(defaultExePath) ? " Preview" : ""),
exePath: defaultExePath,
});

// If defaultExePath is pwsh, check to see if pwsh-preview is also installed and if so, make it available.
// If the defaultExePath is already pwsh-preview, then pwsh is not installed - nothing to do.
let osExePath;
let osPreviewExePath;
if (platformDetails.operatingSystem === OperatingSystem.MacOS) {
osExePath = macOSExePath;
osPreviewExePath = macOSPreviewExePath;
} else if (platformDetails.operatingSystem === OperatingSystem.Linux) {
osExePath = linuxExePath;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we not need to do the same ternary op here to check for snap?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed here because getDefaultPowerShellPath already returns just one of: linuxExePath, linuxPreviewExePath, snapExePath, snapPreviewExePath. It searches in that order.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW I get the impression (perhaps incorrect impression) that on a Snap system, PowerShell will only be at /snap/bin/pwsh* and that you wouldn't also have PowerShell at /usr/bin/pwsh*. And on a straight (non-Snap) Linux system, there would be no /snap/bin/pwsh*. That is, these two sets are mutually exclusive. Is that correct?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So snap is really just a package manager that you first install with apt. So you could have installed pwsh by Snap and Apt on the same system.

Not quite sure why one would do that... but it's physically possible. I guess I could see something along the lines of installing pwsh with apt and pwsh-preview with snap maybe...

So it's physically possible to have a /snap/bin/pwsh and a /usr/bin/pwsh.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However! I just learned that /snap/bin/powershell gets created but /usr/bin/powershell does not. So if you installed PowerShell with Snap and Apt, typing pwsh at the terminal will get you the apt version... and typing powershell at the prompt will get you the Snap version.

This might be TMI but I figured I would share.

osPreviewExePath = linuxPreviewExePath;
}

if ((osExePath === defaultExePath) && fs.existsSync(osPreviewExePath)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's super rare, but isn't it possible for them to have legacy (powershell) and preview (pwsh-preview)?

If so, this if check might have to be:

defaultExePath !== osPreviewExePath && ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can fix this by testing for the legacy path here or maybe we should just remove support for the legacy "powershell"on Linux and macOS. This was abandoned somewhere around the transition from 6.0.0 alpha to beta. At this point, I'm not sure why anyone would still be running an alpha of 6.0.0 on Linux/macOS.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

totally on board. We should only support 6.0.0 release and onward.

paths.push({
versionName: "PowerShell Core Preview",
exePath: osPreviewExePath,
});
}
}

// When unit testing, we don't have session settings to test so skip reading this setting
Expand Down