-
-
Notifications
You must be signed in to change notification settings - Fork 17
Updated to VS2022 runtimes & changed Start/Install order #55
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: master
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -111,38 +111,28 @@ IPrerequisite[] missingPrerequisites | |
|
|
||
| private int Run(TargetAssembly targetAssembly, string[] args) | ||
| { | ||
| try | ||
| { | ||
| // Hot path: attempt to run the target first without any checks | ||
| return targetAssembly.Run(args); | ||
| } | ||
| // Possible exception causes: | ||
| // - .NET host not found (DirectoryNotFoundException) | ||
| // - .NET host failed to initialize (BootstrapperException) | ||
| catch | ||
| { | ||
| // Check for missing prerequisites and install them | ||
| var missingPrerequisites = targetAssembly.GetMissingPrerequisites(); | ||
| if (missingPrerequisites.Any()) | ||
| { | ||
| var isReadyToRun = PromptAndInstall(targetAssembly, missingPrerequisites); | ||
| // need to check the prerequisites first, otherwise the application may start but fail later | ||
| // e.g. VS runtime 2019 are installed, but app needs 2022 | ||
| // e.g. .NET 8.0.4 is installed but 8.0.20 is desired | ||
| // both conditions does not prevent starting the app, but is not what the dev desired. | ||
|
|
||
| // User did not accept the installation or reboot is required | ||
| if (!isReadyToRun) | ||
| return 0xB007; | ||
| // Check for missing prerequisites and install them | ||
| var missingPrerequisites = targetAssembly.GetMissingPrerequisites(); | ||
| if (!missingPrerequisites.Any()) | ||
| return targetAssembly.Run(args); | ||
|
Comment on lines
+119
to
+122
|
||
|
|
||
| // Reset the environment to update PATH and other variables | ||
| // that may have been changed by the installation process. | ||
| EnvironmentEx.RefreshEnvironmentVariables(); | ||
| var isReadyToRun = PromptAndInstall(targetAssembly, missingPrerequisites); | ||
|
||
|
|
||
| // Attempt to run the target again | ||
| return targetAssembly.Run(args); | ||
| } | ||
| // User did not accept the installation or reboot is required | ||
| if (!isReadyToRun) | ||
| return 0xB007; | ||
|
|
||
| // There are no missing prerequisites to install, meaning that the | ||
| // app failed to run for reasons unrelated to the bootstrapper. | ||
| throw; | ||
| } | ||
| // Reset the environment to update PATH and other variables | ||
| // that may have been changed by the installation process. | ||
| EnvironmentEx.RefreshEnvironmentVariables(); | ||
|
|
||
| // Attempt to run the target | ||
| return targetAssembly.Run(args); | ||
| } | ||
|
|
||
| public int Run(string[] args) | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,5 @@ | ||||||||||||||||||||||||||
| using System; | ||||||||||||||||||||||||||
| using System.Runtime.InteropServices; | ||||||||||||||||||||||||||
| using DotnetRuntimeBootstrapper.AppHost.Core.Platform; | ||||||||||||||||||||||||||
| using DotnetRuntimeBootstrapper.AppHost.Core.Utils; | ||||||||||||||||||||||||||
| using DotnetRuntimeBootstrapper.AppHost.Core.Utils.Extensions; | ||||||||||||||||||||||||||
|
|
@@ -8,25 +9,35 @@ namespace DotnetRuntimeBootstrapper.AppHost.Core.Prerequisites; | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| internal class VisualCppPrerequisite : IPrerequisite | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| public string DisplayName => "Visual C++ Redistributable 2015-2019"; | ||||||||||||||||||||||||||
| public string DisplayName => "Visual C++ Redistributable 2015-2022"; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| public bool IsInstalled() => | ||||||||||||||||||||||||||
| Registry.LocalMachine.ContainsSubKey( | ||||||||||||||||||||||||||
| public bool IsInstalled() | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| var registryKey = Registry.LocalMachine.OpenSubKey( | ||||||||||||||||||||||||||
| ( | ||||||||||||||||||||||||||
| OperatingSystemEx.ProcessorArchitecture.Is64Bit() | ||||||||||||||||||||||||||
| ? "SOFTWARE\\Wow6432Node\\" | ||||||||||||||||||||||||||
| : "SOFTWARE\\" | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| + "Microsoft\\VisualStudio\\14.0\\VC\\Runtimes\\" | ||||||||||||||||||||||||||
| + OperatingSystemEx.ProcessorArchitecture.GetMoniker() | ||||||||||||||||||||||||||
| + OperatingSystemEx.ProcessorArchitecture.GetMoniker(), | ||||||||||||||||||||||||||
| false | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (registryKey == null) | ||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // than check if the minor version is ok for 2022 | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
| // than check if the minor version is ok for 2022 | |
| // then check if the minor version is ok for 2022 |
Copilot
AI
Sep 25, 2025
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.
GetValue can return null, which will cause Convert.ToInt32 to throw an exception. The registry value should be validated before conversion.
| var minorVersion = Convert.ToInt32(registryKey.GetValue("Minor", 0)); | |
| var minorValue = registryKey.GetValue("Minor", 0); | |
| int minorVersion = 0; | |
| if (minorValue is int i) | |
| minorVersion = i; | |
| else if (minorValue is string s && int.TryParse(s, out var parsed)) | |
| minorVersion = parsed; | |
| // else minorVersion remains 0 |
Copilot
AI
Sep 25, 2025
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 magic number 44 lacks explanation. Add a comment explaining why this specific minor version is required for VS 2022 compatibility.
| var minorVersion = Convert.ToInt32(registryKey.GetValue("Minor", 0)); | |
| var minorVersion = Convert.ToInt32(registryKey.GetValue("Minor", 0)); | |
| // Visual Studio 2022 requires at least minor version 44 of the VC++ runtime. | |
| // See: https://learn.microsoft.com/en-us/cpp/windows/latest-supported-vc-redist |
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 understand the use case but the design philosophy was to ensure that the bootstrapper imposes minimal startup overhead on the app. If we check for prerequisites before each launch, we'd be delaying it.
This shouldn't be an issue if the user specifies a minimum runtime version.
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.
Hi Tyrrrz,
I understand your reasoning, and it works fine with setting the minimum runtime version in the project.
However, this does not work for the VS runtimes, where VS 2022 is required for some native dependencies, but only VS 2019 is installed. So starting the .NET app will succeed, but the application will still fail.
This also applies to the KB updates you have added. If an acceptable .NET runtime is present, those prerequisites don't get installed.
And as long as the
IsInstalledchecks do not require a download, their impact on startup time should be negligible.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 be more comfortable if the forced check would be an off-by-default option. Even though the delay is negligible, it can still be impactful in some scenarios, such as CLI apps that are ran in quick succession.