-
Notifications
You must be signed in to change notification settings - Fork 90
Fix to return all installed DotNetSDK versions #117
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
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 |
|---|---|---|
|
|
@@ -49,13 +49,13 @@ public static VisualStudioInstance GetInstance(string dotNetSdkPath) | |
| { | ||
| return null; | ||
| } | ||
|
|
||
| // Components of the SDK often have dependencies on the runtime they shipped with, including that several tasks that shipped | ||
| // in the .NET 5 SDK rely on the .NET 5.0 runtime. Assuming the runtime that shipped with a particular SDK has the same version, | ||
| // this ensures that we don't choose an SDK that doesn't work with the runtime of the chosen application. This is not guaranteed | ||
| // to always work but should work for now. | ||
| if (major > Environment.Version.Major || | ||
| (major == Environment.Version.Major && minor > Environment.Version.Minor)) | ||
| if (major < Environment.Version.Major || | ||
| (major == Environment.Version.Major && minor < Environment.Version.Minor)) | ||
| { | ||
| return null; | ||
| } | ||
|
|
@@ -82,58 +82,58 @@ private static IEnumerable<string> GetDotNetBasePaths(string workingDirectory) | |
| const string DOTNET_CLI_UI_LANGUAGE = nameof(DOTNET_CLI_UI_LANGUAGE); | ||
|
|
||
| Process process; | ||
| var lines = new List<string>(); | ||
| try | ||
| { | ||
| var startInfo = new ProcessStartInfo("dotnet", "--info") | ||
| process = new Process() | ||
|
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. Is this change necessary? |
||
| { | ||
| WorkingDirectory = workingDirectory, | ||
| CreateNoWindow = true, | ||
| UseShellExecute = false, | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true | ||
| StartInfo = new ProcessStartInfo("dotnet", "--info") | ||
| { | ||
| WorkingDirectory = workingDirectory, | ||
| CreateNoWindow = true, | ||
| UseShellExecute = false, | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true | ||
| } | ||
| }; | ||
|
|
||
| // Ensure that we set the DOTNET_CLI_UI_LANGUAGE environment variable to "en-US" before | ||
| // running 'dotnet --info'. Otherwise, we may get localized results. | ||
| startInfo.EnvironmentVariables[DOTNET_CLI_UI_LANGUAGE] = "en-US"; | ||
| process.StartInfo.EnvironmentVariables[DOTNET_CLI_UI_LANGUAGE] = "en-US"; | ||
|
|
||
| process = Process.Start(startInfo); | ||
| process.OutputDataReceived += (_, e) => | ||
| { | ||
| if (!string.IsNullOrWhiteSpace(e.Data)) | ||
| { | ||
| lines.Add(e.Data); | ||
| } | ||
| }; | ||
|
|
||
| process.Start(); | ||
|
Comment on lines
+104
to
+112
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. This reordering and moving: 👍🏻 |
||
| } | ||
| catch | ||
| { | ||
| // when error running dotnet command, consider dotnet as not available | ||
| yield break; | ||
| } | ||
|
|
||
| if (process.HasExited) | ||
| { | ||
| yield break; | ||
| } | ||
|
|
||
| var lines = new List<string>(); | ||
| process.OutputDataReceived += (_, e) => | ||
| { | ||
| if (!string.IsNullOrWhiteSpace(e.Data)) | ||
| { | ||
| lines.Add(e.Data); | ||
| } | ||
| }; | ||
|
|
||
| process.BeginOutputReadLine(); | ||
|
|
||
| process.WaitForExit(); | ||
|
|
||
| var outputString = string.Join(Environment.NewLine, lines); | ||
|
|
||
| var matched = DotNetBasePathRegex.Match(outputString); | ||
| if (!matched.Success) | ||
| // We return the version in use at the front of the list in order to ensure | ||
| // FirstOrDefault always returns the version in use. | ||
| // The BasePath might not be found if the required version (via global.json) | ||
| // is not installed. In that case, we can still iterate to the other SDKs installed | ||
| string basePath = null; | ||
| if (matched.Success) | ||
| { | ||
| yield break; | ||
| basePath = matched.Groups[1].Value.Trim(); | ||
| yield return basePath; | ||
| } | ||
|
Comment on lines
+131
to
136
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. This looks good too. |
||
|
|
||
| var basePath = matched.Groups[1].Value.Trim(); | ||
|
|
||
| yield return basePath; // We return the version in use at the front of the list in order to ensure FirstOrDefault always returns the version in use. | ||
|
|
||
| var lineSdkIndex = lines.FindIndex(line => line.Contains("SDKs installed")); | ||
|
|
||
|
|
||
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 was correct before.
The .NET SDK is build concurrently with the .NET runtime, and SDK features may use new runtime features. The .NET 5.0 SDK must execute on the .NET 5.0 runtime. We expect that it will also run fine on .NET 6.0 and higher, barring surprising .NET runtime breaking changes.