-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: add vcruntime for windows installer #5852
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
feat: add vcruntime for windows installer #5852
Conversation
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.
Caution
Changes requested ❌
Reviewed everything up to 72c93cd in 2 minutes and 51 seconds. Click for details.
- Reviewed
77
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
4
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src-tauri/tauri.bundle.windows.nsis.template:61
- Draft comment:
Ensure VCREDIST_URL and VCREDIST_FILENAME remain updated; consider referencing an official source for future-proofing. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
2. src-tauri/tauri.bundle.windows.nsis.template:623
- Draft comment:
Registry checks only verify non-empty version; consider explicitly validating that the detected version is >=14.0. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment raises a valid point about version validation. However, the code is specifically looking in registry locations that are guaranteed to only contain VC++ 14.0+ versions (VS2015-2022). The registry paths themselves enforce the version constraint. Adding explicit version validation would be redundant given the registry paths being checked. I could be wrong about the registry paths guaranteeing version 14.0+. There might be edge cases where older versions could appear in these locations. The registry paths are specifically for VS2015+ (which is VC++ 14.0+) and the Microsoft documentation confirms this. The code's approach of checking these specific paths is the standard way to detect modern VC++ redistributables. The comment should be deleted since the code's approach of using version-specific registry paths is already the correct way to validate VC++ 14.0+ presence.
3. src-tauri/tauri.bundle.windows.nsis.template:648
- Draft comment:
Double-check the quoting in the ExecWait call to handle spaces in the $TEMP path properly. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
4. src-tauri/tauri.bundle.windows.nsis.template:667
- Draft comment:
Consider adding an integrity check (e.g. checksum verification) for the downloaded installer to improve security. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% While adding checksum verification would improve security, there are several issues with this comment: 1) The download is from Microsoft's official HTTPS URL which provides transport security 2) Microsoft doesn't publish official checksums for the redistributable 3) Even if we had a checksum, it would need constant updates as Microsoft updates the redistributable 4) The comment doesn't provide specific guidance on implementation. The security concern is valid - verifying downloaded executables is a good practice. And this is a privileged installer that runs with elevated permissions. However, the practical value is limited since we're downloading directly from Microsoft via HTTPS, and maintaining checksums would be an ongoing burden with little security benefit given the trusted source. The comment should be removed. While technically correct, implementing checksum verification would add complexity without meaningful security benefits given we're downloading directly from Microsoft's official HTTPS URL.
Workflow ID: wflow_NWh1l2V0Ey8mb24f
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
${Else} | ||
DetailPrint "Visual C++ Redistributable installation failed with exit code: $2" | ||
MessageBox MB_ICONEXCLAMATION|MB_YESNO "Visual C++ Redistributable installation failed. Continue anyway?" IDYES continue_install | ||
Abort "Installation cancelled due to Visual C++ Redistributable failure" |
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.
For exit code 3010 (restart required) and other failures, consider setting a reboot flag or handling control flow more clearly instead of relying solely on labels after Abort.
Barecheck - Code coverage reportTotal: 35.02%Your code coverage diff: 0.02% ▴ ✅ All code changes are covered |
e8f7345
to
474000e
Compare
This pull request introduces support for checking and installing the Visual C++ Redistributable during the installation process for Windows builds. The key changes include defining necessary variables for the Redistributable and adding a new installation section to handle its detection, download, and installation.
Enhancements to Windows Installer:
Addition of VC++ Redistributable Variables: Defined
VCREDIST_URL
andVCREDIST_FILENAME
to store the download URL and filename for the Visual C++ Redistributable. (src-tauri/tauri.bundle.windows.nsis.template
, src-tauri/tauri.bundle.windows.nsis.templateR61-R62)New Installation Section for VC++ Redistributable: Added a
Section VCRedist
to:src-tauri/tauri.bundle.windows.nsis.template
, src-tauri/tauri.bundle.windows.nsis.templateR620-R676)Important
Add support for checking and installing Visual C++ Redistributable during Windows installation in
src-tauri/tauri.bundle.windows.nsis.template
.VCREDIST_URL
andVCREDIST_FILENAME
for Visual C++ Redistributable download URL and filename insrc-tauri/tauri.bundle.windows.nsis.template
.Section VCRedist
to check for Visual C++ Redistributable (2015-2022, version 14.0 or higher) in registry, download and install if not found, and handle errors and cleanup insrc-tauri/tauri.bundle.windows.nsis.template
.This description was created by
for 72c93cd. You can customize this summary. It will automatically update as commits are pushed.