Skip to content

Commit 504f412

Browse files
committed
Report more granular dependency installation results
1 parent 74c8073 commit 504f412

File tree

5 files changed

+32
-20
lines changed

5 files changed

+32
-20
lines changed

src/installRuntimeDependencies.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { PackageInstallation, LogPlatformInfo, InstallationSuccess } from './sha
88
import { EventStream } from './eventStream';
99
import { getRuntimeDependenciesPackages } from './tools/runtimeDependencyPackageUtils';
1010
import { getAbsolutePathPackagesToInstall } from './packageManager/getAbsolutePathPackagesToInstall';
11-
import IInstallDependencies from './packageManager/IInstallDependencies';
11+
import { DependencyInstallationResults, IInstallDependencies } from './packageManager/IInstallDependencies';
1212
import { AbsolutePathPackage } from './packageManager/absolutePathPackage';
1313

1414
export async function installRuntimeDependencies(
@@ -19,7 +19,7 @@ export async function installRuntimeDependencies(
1919
platformInfo: PlatformInformation,
2020
useFramework: boolean,
2121
requiredPackageIds: string[]
22-
): Promise<boolean> {
22+
): Promise<DependencyInstallationResults> {
2323
const runTimeDependencies = getRuntimeDependenciesPackages(packageJSON);
2424
const packagesToInstall = await getAbsolutePathPackagesToInstall(runTimeDependencies, platformInfo, extensionPath);
2525
const filteredPackages = filterOmniSharpPackage(packagesToInstall, useFramework);
@@ -30,15 +30,20 @@ export async function installRuntimeDependencies(
3030
// Display platform information and RID
3131
eventStream.post(new LogPlatformInfo(platformInfo));
3232

33-
if (await installDependencies(filteredRequiredPackages)) {
33+
const installationResults = await installDependencies(filteredRequiredPackages);
34+
35+
const failedPackages = Object.entries(installationResults)
36+
.filter(([, installed]) => !installed)
37+
.map(([name]) => name);
38+
if (failedPackages.length === 0) {
3439
eventStream.post(new InstallationSuccess());
35-
} else {
36-
return false;
3740
}
41+
42+
return installationResults;
3843
}
3944

40-
//All the required packages are already downloaded and installed
41-
return true;
45+
// There was nothing to install
46+
return {};
4247
}
4348

4449
function filterOmniSharpPackage(packages: AbsolutePathPackage[], useFramework: boolean) {

src/main.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ export async function activate(
119119
} else {
120120
const getCoreClrDebugPromise = async (languageServerStartedPromise: Promise<void>) => {
121121
let coreClrDebugPromise = Promise.resolve();
122-
if (runtimeDependenciesExist) {
122+
if (runtimeDependenciesExist['Debugger']) {
123123
// activate coreclr-debug
124124
coreClrDebugPromise = coreclrdebug.activate(
125125
context.extension,

src/packageManager/IInstallDependencies.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
import { AbsolutePathPackage } from './absolutePathPackage';
77

8-
export default interface IInstallDependencies {
9-
(packages: AbsolutePathPackage[]): Promise<boolean>;
8+
export type DependencyInstallationResults = { [name: string]: boolean };
9+
10+
export interface IInstallDependencies {
11+
(packages: AbsolutePathPackage[]): Promise<DependencyInstallationResults>;
1012
}

src/packageManager/downloadAndInstallPackages.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { PackageInstallStart } from '../shared/loggingEvents';
1717
import { DownloadValidator } from './isValidDownload';
1818
import { CancellationToken } from 'vscode';
1919
import { ITelemetryReporter } from '../shared/telemetryReporter';
20+
import { DependencyInstallationResults } from './IInstallDependencies';
2021

2122
export async function downloadAndInstallPackages(
2223
packages: AbsolutePathPackage[],
@@ -25,9 +26,9 @@ export async function downloadAndInstallPackages(
2526
downloadValidator: DownloadValidator,
2627
telemetryReporter?: ITelemetryReporter,
2728
token?: CancellationToken
28-
): Promise<boolean> {
29-
let downloadFailed = false;
29+
): Promise<DependencyInstallationResults> {
3030
eventStream.post(new PackageInstallStart());
31+
const results: DependencyInstallationResults = {};
3132
for (const pkg of packages) {
3233
let installationStage = 'touchBeginFile';
3334
try {
@@ -51,12 +52,16 @@ export async function downloadAndInstallPackages(
5152
await InstallZip(buffer, pkg.description, pkg.installPath, pkg.binaries, eventStream);
5253
installationStage = 'touchLockFile';
5354
await touchInstallFile(pkg.installPath, InstallFileType.Lock);
55+
results[pkg.id] = true;
5456
break;
5557
} else {
5658
eventStream.post(new IntegrityCheckFailure(pkg.description, pkg.url, willTryInstallingPackage()));
59+
results[pkg.id] = false;
5760
}
5861
}
5962
} catch (error) {
63+
results[pkg.id] = false;
64+
6065
if (error instanceof NestedError) {
6166
const packageError = new PackageError(error.message, pkg, error.err);
6267
eventStream.post(new InstallationFailure(installationStage, packageError));
@@ -66,8 +71,6 @@ export async function downloadAndInstallPackages(
6671

6772
// Send telemetry for the failure
6873
sendInstallationFailureTelemetry(pkg, installationStage, error);
69-
70-
downloadFailed = true;
7174
} finally {
7275
try {
7376
if (await installFileExists(pkg.installPath, InstallFileType.Begin)) {
@@ -79,7 +82,7 @@ export async function downloadAndInstallPackages(
7982
}
8083
}
8184

82-
return !downloadFailed;
85+
return results;
8386

8487
function sendInstallationFailureTelemetry(pkg: AbsolutePathPackage, installationStage: string, error: any): void {
8588
if (!telemetryReporter) {

test/omnisharp/omnisharpUnitTests/installRuntimeDependencies.test.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import { describe, test, expect, beforeEach } from '@jest/globals';
77
import { installRuntimeDependencies } from '../../../src/installRuntimeDependencies';
8-
import IInstallDependencies from '../../../src/packageManager/IInstallDependencies';
8+
import { IInstallDependencies } from '../../../src/packageManager/IInstallDependencies';
99
import { EventStream } from '../../../src/eventStream';
1010
import { PlatformInformation } from '../../../src/shared/platform';
1111
import TestEventBus from './testAssets/testEventBus';
@@ -28,7 +28,8 @@ describe(`${installRuntimeDependencies.name}`, () => {
2828
beforeEach(() => {
2929
eventStream = new EventStream();
3030
eventBus = new TestEventBus(eventStream);
31-
installDependencies = async () => Promise.resolve(true);
31+
installDependencies = async (packages) =>
32+
Promise.resolve(packages.reduce((acc, pkg) => ({ ...acc, [pkg.id]: true }), {}));
3233
});
3334

3435
describe('When all the dependencies already exist', () => {
@@ -90,7 +91,7 @@ describe(`${installRuntimeDependencies.name}`, () => {
9091
let inputPackage: AbsolutePathPackage[];
9192
installDependencies = async (packages) => {
9293
inputPackage = packages;
93-
return Promise.resolve(true);
94+
return Promise.resolve(packages.reduce((acc, pkg) => ({ ...acc, [pkg.id]: true }), {}));
9495
};
9596

9697
const installed = await installRuntimeDependencies(
@@ -111,7 +112,8 @@ describe(`${installRuntimeDependencies.name}`, () => {
111112
});
112113

113114
test('Returns false when installDependencies returns false', async () => {
114-
installDependencies = async () => Promise.resolve(false);
115+
installDependencies = async (packages) =>
116+
Promise.resolve(packages.reduce((acc, pkg) => ({ ...acc, [pkg.id]: false }), {}));
115117
const installed = await installRuntimeDependencies(
116118
packageJSON,
117119
extensionPath,
@@ -121,7 +123,7 @@ describe(`${installRuntimeDependencies.name}`, () => {
121123
useFramework,
122124
['myPackage']
123125
);
124-
expect(installed).toBe(false);
126+
expect(installed['myPackage']).toBe(false);
125127
});
126128
});
127129
});

0 commit comments

Comments
 (0)