Skip to content

Commit

Permalink
Use GitCommitId to avoid pointless update checks
Browse files Browse the repository at this point in the history 
And remove an unused `IRunspaceDetails` interface.
  • Loading branch information
@andyleejordan
andyleejordan committed Dec 6, 2022
1 parent caf7fd3 commit 8801680
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 25 deletions.
1 change: 1 addition & 0 deletions .vsts-ci/templates/ci-general.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ steps:
inputs:
targetType: inline
script: |
Get-ChildItem env:
Get-Module -ListAvailable Pester
Install-Module InvokeBuild -Scope CurrentUser -Force
Install-Module platyPS -Scope CurrentUser -Force
Expand Down
29 changes: 26 additions & 3 deletions src/features/UpdatePowerShell.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,11 @@ export class UpdatePowerShell {
private sessionSettings: Settings,
private logger: ILogger,
versionDetails: IPowerShellVersionDetails) {
this.localVersion = new SemVer(versionDetails.version);
// We use the commit field as it's like
// '7.3.0-preview.3-508-g07175ae0ff8eb7306fe0b0fc7d...' which translates
// to SemVer. The version handler in PSES handles Windows PowerShell and
// just returns the first three fields like '5.1.22621'.
this.localVersion = new SemVer(versionDetails.commit);
this.architecture = versionDetails.architecture.toLowerCase();
}

Expand All @@ -77,8 +81,27 @@ export class UpdatePowerShell {
return false;
}

// TODO: Check if PowerShell is self-built, i.e. PSVersionInfo.GitCommitId.Length > 40.
// TODO: Check if preview is a daily build.
if (this.localVersion.prerelease.length > 1) {
// Daily builds look like '7.3.0-daily20221206.1' which split to
// ['daily20221206', '1'] and development builds look like
// '7.3.0-preview.3-508-g07175...' which splits to ['preview',
// '3-508-g0717...']. The ellipsis is hiding a 40 char hash.
const daily = this.localVersion.prerelease[0].toString();
const commit = this.localVersion.prerelease[1].toString();

// Skip if PowerShell is self-built, that is, this contains a commit hash.
if (commit.length >= 40) {
this.logger.writeDiagnostic("Not offering to update development build.");
return false;
}

// Skip if preview is a daily build.
if (daily.toLowerCase().startsWith("daily")) {
this.logger.writeDiagnostic("Not offering to update daily build.");
return false;
}
}

// TODO: Check if network is available?
// TODO: Only check once a week.
this.logger.writeDiagnostic("Should check for PowerShell update.");
Expand Down
20 changes: 6 additions & 14 deletions src/session.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
OperatingSystem, PowerShellExeFinder
} from "./platform";
import { LanguageClientConsumer } from "./languageClientConsumer";
import { SemVer } from "semver";

export enum SessionStatus {
NeverStarted,
Expand Down Expand Up @@ -54,17 +55,11 @@ export interface IEditorServicesSessionDetails {

export interface IPowerShellVersionDetails {
version: string;
displayVersion: string;
edition: string;
commit: string;
architecture: string;
}

export interface IRunspaceDetails {
powerShellVersion: IPowerShellVersionDetails;
runspaceType: RunspaceType;
connectionString: string;
}

export type IReadSessionFileCallback = (details: IEditorServicesSessionDetails) => void;

export const SendKeyPressNotificationType =
Expand Down Expand Up @@ -734,12 +729,9 @@ Type 'help' to get help.
this.languageStatusItem.detail = "PowerShell";

if (this.versionDetails !== undefined) {
const version = this.versionDetails.architecture.toLowerCase() !== "x64"
? `${this.versionDetails.displayVersion} (${this.versionDetails.architecture.toLowerCase()})`
: this.versionDetails.displayVersion;

this.languageStatusItem.text = "$(terminal-powershell) " + version;
this.languageStatusItem.detail += " " + version;
const semver = new SemVer(this.versionDetails.version);
this.languageStatusItem.text = `$(terminal-powershell) ${semver.major}.${semver.minor}`;
this.languageStatusItem.detail += ` ${this.versionDetails.commit} (${this.versionDetails.architecture.toLowerCase()})`;
}

if (statusText) {
Expand Down Expand Up @@ -835,7 +827,7 @@ Type 'help' to get help.
const powerShellSessionName =
currentPowerShellExe ?
currentPowerShellExe.displayName :
`PowerShell ${this.versionDetails.displayVersion} ` +
`PowerShell ${this.versionDetails.version} ` +
`(${this.versionDetails.architecture.toLowerCase()}) ${this.versionDetails.edition} Edition ` +
`[${this.versionDetails.version}]`;

Expand Down
41 changes: 33 additions & 8 deletions test/features/UpdatePowerShell.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ describe("UpdatePowerShell feature", function () {
settings.promptToUpdatePowerShell = false;
const version: IPowerShellVersionDetails = {
"version": "7.3.0",
"displayVersion": "7.3",
"edition": "Core",
"commit": "7.3.0",
"architecture": "X64"
};
// @ts-expect-error testing doesn't require all arguments.
Expand All @@ -41,10 +41,9 @@ describe("UpdatePowerShell feature", function () {

it("Won't check for Windows PowerShell", function () {
const version: IPowerShellVersionDetails = {
// TODO: This should handle e.g. 5.1.22621.436
"version": "5.1.0",
"displayVersion": "5.1",
"version": "5.1.22621",
"edition": "Desktop",
"commit": "5.1.22621",
"architecture": "X64"
};
// @ts-expect-error testing doesn't require all arguments.
Expand All @@ -53,12 +52,38 @@ describe("UpdatePowerShell feature", function () {
assert(!updater.shouldCheckForUpdate());
});

it("Won't check for a development build of PowerShell", function () {
const version: IPowerShellVersionDetails = {
"version": "7.3.0-preview.3",
"edition": "Core",
"commit": "7.3.0-preview.3-508-g07175ae0ff8eb7306fe0b0fc7d19bdef4fbf2d67",
"architecture": "Arm64"
};
// @ts-expect-error testing doesn't require all arguments.
const updater = new UpdatePowerShell(undefined, settings, testLogger, version);
// @ts-expect-error method is private.
assert(!updater.shouldCheckForUpdate());
});

it("Won't check for a daily build of PowerShell", function () {
const version: IPowerShellVersionDetails = {
"version": "7.3.0-daily20221206.1",
"edition": "Core",
"commit": "7.3.0-daily20221206.1",
"architecture": "Arm64"
};
// @ts-expect-error testing doesn't require all arguments.
const updater = new UpdatePowerShell(undefined, settings, testLogger, version);
// @ts-expect-error method is private.
assert(!updater.shouldCheckForUpdate());
});

it("Won't check if POWERSHELL_UPDATECHECK is 'Off'", function () {
process.env.POWERSHELL_UPDATECHECK = "Off";
const version: IPowerShellVersionDetails = {
"version": "7.3.0",
"displayVersion": "7.3",
"edition": "Core",
"commit": "7.3.0",
"architecture": "X64"
};
// @ts-expect-error testing doesn't require all arguments.
Expand All @@ -70,8 +95,8 @@ describe("UpdatePowerShell feature", function () {
it ("Should otherwise check to update PowerShell", function () {
const version: IPowerShellVersionDetails = {
"version": "7.3.0",
"displayVersion": "7.3",
"edition": "Core",
"commit": "7.3.0",
"architecture": "X64"
};
// @ts-expect-error testing doesn't require all arguments.
Expand All @@ -86,8 +111,8 @@ describe("UpdatePowerShell feature", function () {
process.env.POWERSHELL_UPDATECHECK = "LTS";
const version: IPowerShellVersionDetails = {
"version": "7.0.0",
"displayVersion": "7.0",
"edition": "Core",
"commit": "7.0.0",
"architecture": "X64"
};
// @ts-expect-error testing doesn't require all arguments.
Expand All @@ -101,8 +126,8 @@ describe("UpdatePowerShell feature", function () {
it("Would update to stable", async function() {
const version: IPowerShellVersionDetails = {
"version": "7.0.0",
"displayVersion": "7.0",
"edition": "Core",
"commit": "7.0.0",
"architecture": "X64"
};
// @ts-expect-error testing doesn't require all arguments.
Expand Down

0 comments on commit 8801680

Please sign in to comment.