-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Improve error message when installing tools targeting higher .NET versions #51665
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
Changes from all commits
21d375c
8787231
a612275
1618a37
008fccc
0c2384b
b9f9b35
fa4e0f5
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -97,7 +97,7 @@ public ToolPackageInstance(PackageId id, | |||||
| ResolvedPackageVersion = Version; | ||||||
| } | ||||||
|
|
||||||
| var toolConfiguration = DeserializeToolConfiguration(library, packageDirectory, _fileSystem); | ||||||
| var toolConfiguration = DeserializeToolConfiguration(library, packageDirectory, ResolvedPackageId, _fileSystem); | ||||||
| Warnings = toolConfiguration.Warnings; | ||||||
|
|
||||||
| var installPath = new VersionFolderPathResolver(PackageDirectory.Value).GetInstallPath(ResolvedPackageId.ToString(), ResolvedPackageVersion); | ||||||
|
|
@@ -165,17 +165,58 @@ public static ToolConfiguration GetToolConfiguration(PackageId id, | |||||
| { | ||||||
| var lockFile = new LockFileFormat().Read(assetsJsonParentDirectory.WithFile(AssetsFileName).Value); | ||||||
| var lockFileTargetLibrary = FindLibraryInLockFile(lockFile); | ||||||
| return DeserializeToolConfiguration(lockFileTargetLibrary, packageDirectory, fileSystem); | ||||||
| return DeserializeToolConfiguration(lockFileTargetLibrary, packageDirectory, id, fileSystem); | ||||||
|
|
||||||
| } | ||||||
|
|
||||||
| private static ToolConfiguration DeserializeToolConfiguration(LockFileTargetLibrary library, DirectoryPath packageDirectory, IFileSystem fileSystem) | ||||||
| private static ToolConfiguration DeserializeToolConfiguration(LockFileTargetLibrary library, DirectoryPath packageDirectory, PackageId packageId, IFileSystem fileSystem) | ||||||
| { | ||||||
| try | ||||||
| { | ||||||
| var dotnetToolSettings = FindItemInTargetLibrary(library, ToolSettingsFileName); | ||||||
| if (dotnetToolSettings == null) | ||||||
| { | ||||||
| // Check if this is because of framework incompatibility | ||||||
| // Load available frameworks from the package to provide better error messages | ||||||
| var installPath = new VersionFolderPathResolver(packageDirectory.Value).GetInstallPath(library.Name, library.Version); | ||||||
| var toolsPackagePath = Path.Combine(installPath, "tools"); | ||||||
|
|
||||||
| if (fileSystem.Directory.Exists(toolsPackagePath)) | ||||||
| { | ||||||
| var availableFrameworks = fileSystem.Directory.EnumerateDirectories(toolsPackagePath) | ||||||
| .Select(path => NuGetFramework.ParseFolder(Path.GetFileName(path))) | ||||||
| .Where(f => f.Framework == FrameworkConstants.FrameworkIdentifiers.NetCoreApp) | ||||||
| .ToList(); | ||||||
|
|
||||||
| if (availableFrameworks.Count > 0) | ||||||
| { | ||||||
| var currentFramework = new NuGetFramework(FrameworkConstants.FrameworkIdentifiers.NetCoreApp, new Version(Environment.Version.Major, Environment.Version.Minor)); | ||||||
|
|
||||||
| // Find the minimum framework version required by the tool | ||||||
| var minRequiredFramework = availableFrameworks.MinBy(f => f.Version); | ||||||
|
|
||||||
| // If all available frameworks require a higher version than current runtime | ||||||
| if (minRequiredFramework != null && minRequiredFramework.Version > currentFramework.Version) | ||||||
| { | ||||||
| var requiredVersionString = $".NET {minRequiredFramework.Version.Major}.{minRequiredFramework.Version.Minor}"; | ||||||
| var currentVersionString = $".NET {currentFramework.Version.Major}.{currentFramework.Version.Minor}"; | ||||||
|
|
||||||
| var errorMessage = string.Format( | ||||||
| CliStrings.ToolRequiresHigherDotNetVersion, | ||||||
| packageId, | ||||||
| requiredVersionString, | ||||||
| currentVersionString); | ||||||
|
|
||||||
| var suggestion = string.Format( | ||||||
| CliStrings.ToolRequiresHigherDotNetVersionSuggestion, | ||||||
| minRequiredFramework.Version.Major, | ||||||
| currentFramework.Version.Major); | ||||||
|
|
||||||
| throw new GracefulException($"{errorMessage} {suggestion}", isUserError: false); | ||||||
|
||||||
| throw new GracefulException($"{errorMessage} {suggestion}", isUserError: false); | |
| throw new GracefulException($"{errorMessage} {suggestion}", isUserError: true); |
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.
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.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
The error message format parameters may be misleading. In line 201-202, the code constructs version strings like ".NET 99.0" by using
{Version.Major}.{Version.Minor}, but in line 212-213, the suggestion only uses the major version like ".NET {minRequiredFramework.Version.Major}". This creates an inconsistency where the error message shows ".NET 99.0" but the suggestion says ".NET 99".Consider using consistent formatting in both places. Either use Major.Minor in both (e.g., ".NET 99.0" throughout), or use just Major in both (e.g., ".NET 99" throughout). The latter is more common in .NET version naming (e.g., ".NET 8" instead of ".NET 8.0").