Divide the package manager into separate components #2188
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,68 +4,62 @@ | |
| *--------------------------------------------------------------------------------------------*/ | ||
|
|
||
| import * as util from './common'; | ||
| import { PlatformInformation } from './platform'; | ||
| import { PackageInstallation, LogPlatformInfo, InstallationSuccess, InstallationFailure } from './omnisharp/loggingEvents'; | ||
| import { EventStream } from './EventStream'; | ||
| import { DownloadAndInstallPackages } from './packageManager/PackageManager'; | ||
| import { Package } from './packageManager/Package'; | ||
| import { NetworkSettingsProvider } from './NetworkSettings'; | ||
| import { ResolveFilePaths } from './packageManager/PackageFilePathResolver'; | ||
|
|
||
| /* | ||
| * Class used to download the runtime dependencies of the C# Extension | ||
| */ | ||
| export class CSharpExtDownloader { | ||
|
|
||
| public constructor( | ||
| private provider: NetworkSettingsProvider, | ||
| private eventStream: EventStream, | ||
| private packageJSON: any, | ||
| private platformInfo: PlatformInformation) { | ||
| } | ||
|
|
||
| public async installRuntimeDependencies(): Promise<boolean> { | ||
| this.eventStream.post(new PackageInstallation("C# dependencies")); | ||
| let success = false; | ||
| let installationStage = ''; | ||
|
|
||
| try { | ||
|
|
||
| await util.touchInstallFile(util.InstallFileType.Begin); | ||
| // Display platform information and RID | ||
| this.eventStream.post(new LogPlatformInfo(this.platformInfo)); | ||
| let runTimeDependencies = this.GetRunTimeDependenciesPackages(); | ||
| runTimeDependencies.forEach(pkg => ResolveFilePaths(pkg)); | ||
| installationStage = 'downloadAndInstallPackages'; | ||
| await DownloadAndInstallPackages(runTimeDependencies, this.provider, this.platformInfo, this.eventStream); | ||
| installationStage = 'touchLockFile'; | ||
| await util.touchInstallFile(util.InstallFileType.Lock); | ||
| success = true; | ||
| this.eventStream.post(new InstallationSuccess()); | ||
| } | ||
| catch (error) { | ||
| this.eventStream.post(new InstallationFailure(installationStage, error)); | ||
| } | ||
| finally { | ||
| try { | ||
| util.deleteInstallFile(util.InstallFileType.Begin); | ||
| } | ||
| catch (error) { } | ||
| return success; | ||
| } | ||
| } | ||
|
|
||
| private GetRunTimeDependenciesPackages(): Package[] { | ||
| if (this.packageJSON.runtimeDependencies) { | ||
| return JSON.parse(JSON.stringify(<Package[]>this.packageJSON.runtimeDependencies)); | ||
|
There was a problem hiding this comment. Not sure I understand. There was a problem hiding this comment. By this we are copying these packages by 'value' using this method There was a problem hiding this comment. This is the common practice to "deep copy" json object in js... it is awkward but it is what it is |
||
| } | ||
|
|
||
| return null; | ||
|
There was a problem hiding this comment. What does the .foreach on line 38 do if we return null here? How would that even happen? User deleted/edited package.json? There was a problem hiding this comment. Yes right. Changed it to throw an error instead. However I am not sure what the scenario would be. Do you think we should remove the check ? |
||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| /*--------------------------------------------------------------------------------------------- | ||
| * Copyright (c) Microsoft Corporation. All rights reserved. | ||
| * Licensed under the MIT License. See License.txt in the project root for license information. | ||
| *--------------------------------------------------------------------------------------------*/ | ||
| import * as tmp from 'tmp'; | ||
| import { NestedError } from './packageManager/Package'; | ||
| import { rimraf } from 'async-file'; | ||
|
|
||
| export async function createTmpFile(): Promise<TmpAsset> { | ||
| const tmpFile = await new Promise<tmp.SynchrounousResult>((resolve, reject) => { | ||
| tmp.file({ prefix: 'package-' }, (err, path, fd, cleanupCallback) => { | ||
| if (err) { | ||
| return reject(new NestedError('Error from tmp.file', err)); | ||
| } | ||
| if (fd == 0) { | ||
| return reject(new NestedError("Temporary package file unavailable")); | ||
| } | ||
|
|
||
| resolve(<tmp.SynchrounousResult>{ name: path, fd: fd, removeCallback: cleanupCallback }); | ||
| }); | ||
| }); | ||
|
|
||
| return { | ||
| fd: tmpFile.fd, | ||
| name: tmpFile.name, | ||
| dispose: () => tmpFile.removeCallback() | ||
| }; | ||
| } | ||
|
|
||
| export async function createTmpDir(unsafeCleanup: boolean): Promise<TmpAsset> { | ||
| const tmpDir = await new Promise<tmp.SynchrounousResult>((resolve, reject) => { | ||
| tmp.dir({ unsafeCleanup }, (err, path, cleanupCallback) => { | ||
| if (err) { | ||
| return reject(new NestedError('Error from tmp.dir', err)); | ||
| } | ||
|
|
||
| resolve(<tmp.SynchrounousResult>{ name: path, removeCallback: cleanupCallback }); | ||
| }); | ||
| }); | ||
|
|
||
| return { | ||
| fd: tmpDir.fd, | ||
| name: tmpDir.name, | ||
| dispose: () => { | ||
| if (unsafeCleanup) { | ||
| rimraf(tmpDir.name);//to delete directories that have folders inside them | ||
| } | ||
| else { | ||
| tmpDir.removeCallback(); | ||
| } | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| export interface TmpAsset { | ||
| fd: number; | ||
| name: string; | ||
| dispose: () => void; | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| /*--------------------------------------------------------------------------------------------- | ||
| * Copyright (c) Microsoft Corporation. All rights reserved. | ||
| * Licensed under the MIT License. See License.txt in the project root for license information. | ||
| *--------------------------------------------------------------------------------------------*/ | ||
|
|
||
| import { vscode } from "./vscodeAdapter"; | ||
|
|
||
| export default class NetworkSettings { | ||
| constructor(public readonly proxy: string, public readonly strictSSL: boolean) { | ||
| } | ||
| } | ||
|
|
||
| export interface NetworkSettingsProvider { | ||
| //to do: make this async | ||
| (): NetworkSettings; | ||
| } | ||
|
|
||
| export function vscodeNetworkSettingsProvider(vscode: vscode): NetworkSettingsProvider { | ||
| return () => { | ||
| const config = vscode.workspace.getConfiguration(); | ||
| const proxy = config.get<string>('http.proxy'); | ||
| const strictSSL = config.get('http.proxyStrictSSL', true); | ||
| return new NetworkSettings(proxy, strictSSL); | ||
| }; | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Since c#extdl'er assumes that DownloadAndInstallPacakges returning means the lockfile can be changed, maybe we should just move lockfile manipulation into the downloader component?
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.
We are planning to remove the Lockfile concept(in a subsequent PR) entirely and using the InstallTestPath to check whether the packages are installed or not.