-
Notifications
You must be signed in to change notification settings - Fork 1.2k
NET tool roll forward error experience #38210
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
base: release/9.0.2xx
Are you sure you want to change the base?
Changes from all commits
c8a5942
31ccfc2
1c5bf35
3126fd0
4bb3a3c
0459205
b7cf735
bed712f
357069f
943a94e
af2dbe4
97dbdd5
8e2641f
a1e1584
43c115c
89daa6f
96b01a2
0fdfd91
e57862c
9d2d543
42e5642
ed062c9
56e36af
b443db7
f1403f1
f91507e
d4f55b2
1caa48a
bc7877c
a4f59a9
fe5c737
8d8902f
ff12a1a
52ca72b
f1f55d6
f03033c
40acd74
68acb5a
eb84eba
8258229
09ef33e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,7 @@ | ||
| // Copyright (c) .NET Foundation and contributors. All rights reserved. | ||
| // Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.IO; | ||
| using System.Linq; | ||
| using System.CommandLine; | ||
| using System.Reflection; | ||
| using System.Runtime.InteropServices; | ||
| using System.Threading.Tasks; | ||
| using Microsoft.DotNet.Cli.NuGetPackageDownloader; | ||
| using Microsoft.DotNet.Cli.Utils; | ||
| using Microsoft.DotNet.ToolPackage; | ||
|
|
@@ -27,10 +20,9 @@ | |
| using NuGet.Versioning; | ||
| using NuGet.Configuration; | ||
| using Microsoft.TemplateEngine.Utils; | ||
| using System.Text.Json; | ||
| using System.Xml; | ||
| using System.Text.Json.Nodes; | ||
| using Newtonsoft.Json.Linq; | ||
| using System.Text.Json; | ||
| using static Microsoft.DotNet.NativeWrapper.NETCoreSdkResolverNativeWrapper; | ||
|
|
||
| namespace Microsoft.DotNet.Cli.ToolPackage | ||
| { | ||
|
|
@@ -77,7 +69,8 @@ public IToolPackage InstallPackage(PackageLocation packageLocation, PackageId pa | |
| VersionRange versionRange = null, | ||
| string targetFramework = null, | ||
| bool isGlobalTool = false, | ||
| bool isGlobalToolRollForward = false | ||
| bool isGlobalToolRollForward = false, | ||
| bool forceInstall = false | ||
| ) | ||
| { | ||
| var packageRootDirectory = _toolPackageStore.GetRootPackageDirectory(packageId); | ||
|
|
@@ -111,7 +104,7 @@ public IToolPackage InstallPackage(PackageLocation packageLocation, PackageId pa | |
| } | ||
| NuGetVersion packageVersion = nugetPackageDownloader.GetBestPackageVersionAsync(packageId, versionRange, packageSourceLocation).GetAwaiter().GetResult(); | ||
|
|
||
| rollbackDirectory = isGlobalTool ? toolDownloadDir.Value: Path.Combine(toolDownloadDir.Value, packageId.ToString(), packageVersion.ToString()); | ||
| rollbackDirectory = isGlobalTool ? toolDownloadDir.Value : Path.Combine(toolDownloadDir.Value, packageId.ToString(), packageVersion.ToString()); | ||
|
|
||
| if (isGlobalTool) | ||
| { | ||
|
|
@@ -134,17 +127,17 @@ public IToolPackage InstallPackage(PackageLocation packageLocation, PackageId pa | |
| { | ||
| DownloadAndExtractPackage(packageId, nugetPackageDownloader, toolDownloadDir.Value, packageVersion, packageSourceLocation, includeUnlisted: givenSpecificVersion).GetAwaiter().GetResult(); | ||
| } | ||
| else if(isGlobalTool) | ||
| else if (isGlobalTool) | ||
| { | ||
| throw new ToolPackageException( | ||
| string.Format( | ||
| CommonLocalizableStrings.ToolPackageConflictPackageId, | ||
| packageId, | ||
| packageVersion.ToNormalizedString())); | ||
| } | ||
|
|
||
| CreateAssetFile(packageId, packageVersion, toolDownloadDir, assetFileDirectory, _runtimeJsonPath, targetFramework); | ||
|
|
||
| DirectoryPath toolReturnPackageDirectory; | ||
| DirectoryPath toolReturnJsonParentDirectory; | ||
|
|
||
|
|
@@ -168,12 +161,17 @@ public IToolPackage InstallPackage(PackageLocation packageLocation, PackageId pa | |
| packageDirectory: toolReturnPackageDirectory, | ||
| assetsJsonParentDirectory: toolReturnJsonParentDirectory); | ||
|
|
||
| if (!forceInstall && !isGlobalToolRollForward) | ||
| { | ||
| CheckIfRequiredRuntimeIsInstalled(toolPackageInstance, packageId, isGlobalTool); | ||
| } | ||
|
|
||
| if (isGlobalToolRollForward) | ||
| { | ||
| UpdateRuntimeConfig(toolPackageInstance); | ||
| } | ||
|
|
||
| return toolPackageInstance; | ||
| return toolPackageInstance; | ||
| }, | ||
| rollback: () => | ||
| { | ||
|
|
@@ -190,6 +188,44 @@ public IToolPackage InstallPackage(PackageLocation packageLocation, PackageId pa | |
| }); | ||
| } | ||
|
|
||
| private static void CheckIfRequiredRuntimeIsInstalled( | ||
| ToolPackageInstance toolPackageInstance, | ||
| PackageId packageId, | ||
| bool isGlobalTool | ||
| ) | ||
| { | ||
| var executableFilePath = toolPackageInstance.Commands[0].Executable; | ||
| var runtimeConfigFilePath = Path.ChangeExtension(executableFilePath.ToString(), ".runtimeconfig.json"); | ||
|
|
||
| // Check if the runtimeconfig.json file is compatible with the current runtime | ||
|
Member
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.
Just want to call out exactly what this means. The
Member
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. Actually, I think this is a larger problem. Checking for compatibility with the loaded runtime means checking for frameworks - not just the core runtime. Since Tools could depend on other frameworks like AspNetCore or WindowsDesktop, right? I think that is a test case that should be covered. I expect any tools like that would fail the compatibility check here.
Contributor
Author
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. Yes, tool could be depend on other frameworks.
Member
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. The new |
||
| if (File.Exists(runtimeConfigFilePath)) | ||
| { | ||
| string tfmValue = ""; | ||
| JsonElement rootElement = JsonDocument.Parse(File.ReadAllText(runtimeConfigFilePath)).RootElement; | ||
|
|
||
| if (rootElement.TryGetProperty("runtimeOptions", out JsonElement runtimeOptionsElement) && | ||
| runtimeOptionsElement.TryGetProperty("tfm", out JsonElement tfmElement)) | ||
| { | ||
| tfmValue = new string(tfmElement.GetString().Where(c => char.IsDigit(c) || c == '.').ToArray()); | ||
|
Member
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. It seems like you should be able to use the value directly instead of filtering it to digits and periods. Did you run into cases where there were invalid values or characters in it?
Contributor
Author
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. Thanks for the comment. This value contains
Member
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. Depending on the API this may or may not be necessary. Not sure yet. |
||
| } | ||
| var result = InitializeForRuntimeConfig(runtimeConfigFilePath); | ||
|
|
||
| // Error if tool is incompatible | ||
| var global = isGlobalTool ? " -g" : ""; | ||
| switch (result) | ||
| { | ||
| case InitializationRuntimeConfigResult.Success: | ||
| break; | ||
|
|
||
| default: | ||
| throw new GracefulException( | ||
| string.Format( | ||
| CommonLocalizableStrings.ToolPackageRuntimeConfigIncompatible, | ||
| packageId, tfmValue, global)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // The following methods are copied from the LockFileUtils class in Nuget.Client | ||
| private static void AddToolsAssets( | ||
| ManagedCodeConventions managedCodeConventions, | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.