-
-
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?
Conversation
peter-sabath
commented
Sep 24, 2025
- Updated VisualCppPrerequisite to use the VS 2022 Runtime.
- Running the application is not tried before checking for missing Prerequisites to catch cases where an installed .NET runtime is accepted, but not the desired one (See comments in code)
Updated VisualCppPrerequisite to use the VS 2022 Runtime Running the application is not tried before checking for missing Prerequisites to catch cases where an installed .NET runtime is accepted, but not the desired one
| // 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. |
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.
.NET 8.0.4 is installed but 8.0.20 is desired
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 IsInstalled checks 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.
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.
Pull Request Overview
This PR updates the Visual C++ runtime dependency from VS 2019 to VS 2022 and changes the application startup logic to check prerequisites before attempting to run the target assembly. The key changes ensure that applications use the correct runtime versions rather than accepting older compatible versions.
- Updated Visual C++ prerequisite to require VS 2022 runtime instead of VS 2015-2019
- Modified startup flow to check prerequisites before running the application to prevent issues with compatible but undesired runtime versions
- Enhanced version validation to ensure minimum minor version requirements are met
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| VisualCppPrerequisite.cs | Updates runtime requirement to VS 2022 with proper version validation |
| BootstrapperBase.cs | Changes startup order to validate prerequisites before attempting application execution |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (registryKey == null) | ||
| return false; | ||
|
|
||
| // than 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.
Grammar error in comment. Should be 'then' instead of 'than'.
| // than check if the minor version is ok for 2022 | |
| // then check if the minor version is ok for 2022 |
| return false; | ||
|
|
||
| // than check if the minor version is ok for 2022 | ||
| var minorVersion = Convert.ToInt32(registryKey.GetValue("Minor", 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.
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 |
| return false; | ||
|
|
||
| // than check if the minor version is ok for 2022 | ||
| var minorVersion = Convert.ToInt32(registryKey.GetValue("Minor", 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 |
| // Check for missing prerequisites and install them | ||
| var missingPrerequisites = targetAssembly.GetMissingPrerequisites(); | ||
| if (!missingPrerequisites.Any()) | ||
| return targetAssembly.Run(args); |
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.
Inconsistent indentation. Line 122 uses tabs while the surrounding code uses spaces. Should maintain consistent indentation style.
| // 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); |
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.
Inconsistent indentation. This line and subsequent lines use tabs while the surrounding code uses spaces. Should maintain consistent indentation style.
|
Hi, thanks for the PR. My recommendation is to separate the two changes into two separate PRs so it's easier to review. |