-
Notifications
You must be signed in to change notification settings - Fork 677
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.
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
Enable multiple download of omnisharp #2065
Changes from 8 commits
01bc4ae
7aa752d
d3fc133
7e11418
180f438
88221c3
9d6b82c
a917cd6
7cbccd8
29deec5
ea4c351
05a528c
4dde51c
1ce54c1
7cf9886
5c78308
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
/*--------------------------------------------------------------------------------------------- | ||
* Copyright (c) Microsoft Corporation. All rights reserved. | ||
* Licensed under the MIT License. See License.txt in the project root for license information. | ||
*--------------------------------------------------------------------------------------------*/ | ||
import * as vscode from 'vscode'; | ||
import { Status, PackageError } from './packages'; | ||
import { PlatformInformation } from './platform'; | ||
import { Logger } from './logger'; | ||
import TelemetryReporter from 'vscode-extension-telemetry'; | ||
|
||
export function GetNetworkDependencies() { | ||
const config = vscode.workspace.getConfiguration(); | ||
const proxy = config.get<string>('http.proxy'); | ||
const strictSSL = config.get('http.proxyStrictSSL', true); | ||
return { Proxy: proxy, StrictSSL: strictSSL }; | ||
} | ||
|
||
export function SetStatus() { | ||
let statusItem = vscode.window.createStatusBarItem(vscode.StatusBarAlignment.Right); | ||
let status: Status = { | ||
setMessage: text => { | ||
statusItem.text = text; | ||
statusItem.show(); | ||
}, | ||
setDetail: text => { | ||
statusItem.tooltip = text; | ||
statusItem.show(); | ||
} | ||
}; | ||
|
||
return { StatusItem: statusItem, Status: status }; | ||
} | ||
|
||
export function LogPlatformInformation(logger: Logger, platformInfo: PlatformInformation) { | ||
logger.appendLine(`Platform: ${platformInfo.toString()}`); | ||
logger.appendLine(); | ||
} | ||
|
||
export function ReportInstallationError(logger: Logger, error, telemetryProps: any, installationStage: string) { | ||
let errorMessage: string; | ||
if (error instanceof PackageError) { | ||
// we can log the message in a PackageError to telemetry as we do not put PII in PackageError messages | ||
telemetryProps['error.message'] = error.message; | ||
if (error.innerError) { | ||
errorMessage = error.innerError.toString(); | ||
} | ||
else { | ||
errorMessage = error.message; | ||
} | ||
if (error.pkg) { | ||
telemetryProps['error.packageUrl'] = error.pkg.url; | ||
} | ||
} | ||
else { | ||
// do not log raw errorMessage in telemetry as it is likely to contain PII. | ||
errorMessage = error.toString(); | ||
} | ||
|
||
logger.appendLine(); | ||
logger.appendLine(`Failed at stage: ${installationStage}`); | ||
logger.appendLine(errorMessage); | ||
} | ||
|
||
export function SendInstallationTelemetry(logger: Logger, reporter: TelemetryReporter, telemetryProps: any, installationStage: string, platformInfo: PlatformInformation) { | ||
telemetryProps['installStage'] = installationStage; | ||
telemetryProps['platform.architecture'] = platformInfo.architecture; | ||
telemetryProps['platform.platform'] = platformInfo.platform; | ||
if (platformInfo.distribution) { | ||
telemetryProps['platform.distribution'] = platformInfo.distribution.toTelemetryString(); | ||
} | ||
if (reporter) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is reporter ever null? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I dont think so. But I just followed by the pattern followed by CSharpExtDownloader. Currently this pattern helps because in testing I am conveniently passing this field as null, but in future we might want to test the Telemetry as well. For the current PR, I think we can retain this pattern. What do you think ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, we can refactor later. |
||
reporter.sendTelemetryEvent('Acquisition', telemetryProps); | ||
} | ||
|
||
logger.appendLine(); | ||
installationStage = ''; | ||
logger.appendLine('Finished'); | ||
logger.appendLine(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
/*--------------------------------------------------------------------------------------------- | ||
* Copyright (c) Microsoft Corporation. All rights reserved. | ||
* Licensed under the MIT License. See License.txt in the project root for license information. | ||
*--------------------------------------------------------------------------------------------*/ | ||
|
||
import * as vscode from 'vscode'; | ||
import { PackageManager, Package, Status } from '../packages'; | ||
import { PlatformInformation } from '../platform'; | ||
import { Logger } from '../logger'; | ||
import TelemetryReporter from 'vscode-extension-telemetry'; | ||
import { GetPackagesFromVersion, GetVersionFilePackage } from './OmnisharpPackageCreator'; | ||
import { SetStatus, LogPlatformInformation, ReportInstallationError, SendInstallationTelemetry, GetNetworkDependencies } from '../OmnisharpDownload.Helper'; | ||
|
||
export class OmnisharpDownloader { | ||
private status: Status; | ||
private statusItem: vscode.StatusBarItem; | ||
private proxy: string; | ||
private strictSSL: boolean; | ||
private packageManager: PackageManager; | ||
private telemetryProps: any; | ||
|
||
public constructor( | ||
private channel: vscode.OutputChannel, | ||
private logger: Logger, | ||
private packageJSON: any, | ||
private platformInfo: PlatformInformation, | ||
private reporter?: TelemetryReporter) { | ||
|
||
let statusObject = SetStatus(); | ||
this.status = statusObject.Status; | ||
this.statusItem = statusObject.StatusItem; | ||
|
||
let networkObject = GetNetworkDependencies(); | ||
this.proxy = networkObject.Proxy; | ||
this.strictSSL = networkObject.StrictSSL; | ||
|
||
this.telemetryProps = {}; | ||
this.packageManager = new PackageManager(this.platformInfo, this.packageJSON); | ||
} | ||
|
||
public async DownloadAndInstallOmnisharp(version: string, serverUrl: string, installPath: string) { | ||
this.logger.append('Installing Omnisharp Packages...'); | ||
this.logger.appendLine(); | ||
this.channel.show(); | ||
|
||
let installationStage = ''; | ||
|
||
if (this.reporter) { | ||
this.reporter.sendTelemetryEvent("AcquisitionStart"); | ||
} | ||
|
||
try { | ||
LogPlatformInformation(this.logger, this.platformInfo); | ||
|
||
installationStage = 'getPackageInfo'; | ||
let packages: Package[] = GetPackagesFromVersion(version, this.packageJSON.runtimeDependencies, serverUrl, installPath); | ||
|
||
installationStage = 'downloadPackages'; | ||
|
||
// Specify the packages that the package manager needs to download | ||
this.packageManager.SetVersionPackagesForDownload(packages); | ||
await this.packageManager.DownloadPackages(this.logger, this.status, this.proxy, this.strictSSL); | ||
|
||
this.logger.appendLine(); | ||
|
||
installationStage = 'installPackages'; | ||
await this.packageManager.InstallPackages(this.logger, this.status); | ||
|
||
installationStage = 'completeSuccess'; | ||
} | ||
catch (error) { | ||
ReportInstallationError(this.logger, error, this.telemetryProps, installationStage); | ||
throw error;// throw the error up to the server | ||
} | ||
finally { | ||
SendInstallationTelemetry(this.logger, this.reporter, this.telemetryProps, installationStage, this.platformInfo); | ||
this.statusItem.dispose(); | ||
} | ||
} | ||
|
||
public async GetLatestVersion(serverUrl: string, versionFilePathInServer): Promise<string> { | ||
let installationStage = 'getLatestVersionInfoFile'; | ||
try { | ||
this.logger.appendLine('Getting latest build information...'); | ||
this.logger.appendLine(); | ||
//The package manager needs a package format to download, hence we form a package for the latest version file | ||
let filePackage = GetVersionFilePackage(serverUrl, versionFilePathInServer); | ||
//Fetch the latest version information from the file | ||
return await this.packageManager.GetLatestVersionFromFile(this.logger, this.status, this.proxy, this.strictSSL, filePackage); | ||
} | ||
catch (error) { | ||
ReportInstallationError(this.logger, error, this.telemetryProps, installationStage); | ||
throw error; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be a dev dependency only?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is automatically added when I npm install the package. What do you think should be changed here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would just move this dependency in the devdependecies list and fix it to 7.1.1 instead of compatible versions