-
Notifications
You must be signed in to change notification settings - Fork 614
feat: add architecture compatibility checks and Visual C++ Redistributable installation support #752
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
Conversation
…table installation support
WalkthroughAdds a custom NSIS include script that checks CPU architecture compatibility and the presence of the VC++ Redistributable during installer initialization, prompting to download/install if missing, and aborting on incompatibility or failure. The electron-builder configuration is updated to include this script in the NSIS build. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Installer
participant System
participant Browser
participant Web as VC++ Download
participant VC as VC++ Installer
User->>Installer: Start installer
Installer->>System: Detect architecture
alt Incompatible
Installer-->>User: MessageBox: incompatible
Installer->>Browser: Open URL
Installer-->>User: Abort
else Compatible
Installer->>System: Check VC++ Redistributable
alt Missing
Installer-->>User: Prompt to download/install
Installer->>Web: Download vc_redist.x64.exe
Installer->>VC: ExecWait installer
Installer->>System: Recheck VC++
alt Still missing / failed
Installer-->>User: Error and abort
else Installed
Installer-->>User: Continue installation
end
else Present
Installer-->>User: Continue installation
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 2
🧹 Nitpick comments (3)
build/nsis-installer.nsh (3)
7-9: Minor typo in comment“latests downloads” → “latest downloads”.
Apply this diff:
-;Find latests downloads here: +; Find latest downloads here:
25-39: Make arch detection resilient to case and environment variance (optional)PROCESSOR_ARCHITECTURE values can be lowercased on some systems. Uppercasing before comparing avoids surprises.
Apply this diff:
- ReadEnvStr $2 "PROCESSOR_ARCHITECTURE" - ReadEnvStr $4 "PROCESSOR_ARCHITEW6432" + ReadEnvStr $2 "PROCESSOR_ARCHITECTURE" + ReadEnvStr $4 "PROCESSOR_ARCHITEW6432" + StrUpper $2 $2 + StrUpper $4 $4
106-113: Handle download failures explicitly (optional)inetc::get pushes a status string on the stack (e.g., “OK” or an error). Checking it gives clearer errors than relying on a post-install registry recheck.
Here’s a minimal pattern you can adapt (inside InstallVCRedist after inetc::get):
Pop $R0 StrCmp $R0 "OK" +2 0 MessageBox MB_ICONSTOP "Download failed: $R0" & Abort
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
-
build/nsis-installer.nsh(1 hunks) -
electron-builder.yml(1 hunks)
🔇 Additional comments (2)
electron-builder.yml (1)
61-61: NSIS include wiring — LGTMIncluding build/nsis-installer.nsh under nsis.include correctly hooks your customInit into the Electron Builder NSIS template.
build/nsis-installer.nsh (1)
41-56: Verify electron-builder defines APP_64_NAME / APP_ARM64_NAME at compile timeIf these defines aren’t set by electron-builder for your targets,
$3 will fall back to "x64" and could misreport compatibility. Please confirm these macros exist for your build matrix or switch to a more direct signal (e.g., using the $ {ARCH} define or separate artifacts per arch).Would you like me to adapt this to derive app arch from the installer filename or electron-builder’s standard defines to avoid relying on APP_* macros?
| Function checkVCRedist | ||
| ReadRegDWORD $0 HKLM "SOFTWARE\Microsoft\VisualStudio\14.0\VC\Runtimes\x64" "Installed" | ||
| FunctionEnd |
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.
VC++ detection reads the wrong registry view and ignores ARM64; may false-negative on 64-bit/ARM64 systems
NSIS is a 32-bit process; without SetRegView 64 you’ll read the 32-bit WOW6432Node. Also, you always check the x64 runtime and never the ARM64 runtime, so ARM64 systems will be misdetected.
Apply this diff to query the correct 64-bit view and pick the runtime key by system arch:
- Function checkVCRedist
- ReadRegDWORD $0 HKLM "SOFTWARE\Microsoft\VisualStudio\14.0\VC\Runtimes\x64" "Installed"
- FunctionEnd
+ Function checkVCRedist
+ ; Return 1 if the required VC++ runtime is installed for the current system arch (x64/arm64), else 0
+ Push $R0
+ Push $R1
+ Push $R2
+ StrCpy $0 "0"
+ ; Always query the 64-bit registry view on 64-bit Windows
+ SetRegView 64
+ ReadEnvStr $R1 "PROCESSOR_ARCHITECTURE"
+ ReadEnvStr $R2 "PROCESSOR_ARCHITEW6432"
+ ${If} $R1 == "ARM64"
+ ${OrIf} $R2 == "ARM64"
+ ClearErrors
+ ReadRegDWORD $R0 HKLM "SOFTWARE\Microsoft\VisualStudio\14.0\VC\Runtimes\arm64" "Installed"
+ ${Else}
+ ClearErrors
+ ReadRegDWORD $R0 HKLM "SOFTWARE\Microsoft\VisualStudio\14.0\VC\Runtimes\x64" "Installed"
+ ${EndIf}
+ ${If} ${Errors}
+ StrCpy $0 "0"
+ ${Else}
+ StrCpy $0 $R0
+ ${EndIf}
+ ; Restore default view for safety
+ SetRegView 32
+ Pop $R2
+ Pop $R1
+ Pop $R0
+ FunctionEnd📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Function checkVCRedist | |
| ReadRegDWORD $0 HKLM "SOFTWARE\Microsoft\VisualStudio\14.0\VC\Runtimes\x64" "Installed" | |
| FunctionEnd | |
| Function checkVCRedist | |
| ; Return 1 if the required VC++ runtime is installed for the current system arch (x64/arm64), else 0 | |
| Push $R0 | |
| Push $R1 | |
| Push $R2 | |
| StrCpy $0 "0" | |
| ; Always query the 64-bit registry view on 64-bit Windows | |
| SetRegView 64 | |
| ReadEnvStr $R1 "PROCESSOR_ARCHITECTURE" | |
| ReadEnvStr $R2 "PROCESSOR_ARCHITEW6432" | |
| ${If} $R1 == "ARM64" | |
| ${OrIf} $R2 == "ARM64" | |
| ClearErrors | |
| ReadRegDWORD $R0 HKLM "SOFTWARE\Microsoft\VisualStudio\14.0\VC\Runtimes\arm64" "Installed" | |
| ${Else} | |
| ClearErrors | |
| ReadRegDWORD $R0 HKLM "SOFTWARE\Microsoft\VisualStudio\14.0\VC\Runtimes\x64" "Installed" | |
| ${EndIf} | |
| ${If} ${Errors} | |
| StrCpy $0 "0" | |
| ${Else} | |
| StrCpy $0 $R0 | |
| ${EndIf} | |
| ; Restore default view for safety | |
| SetRegView 32 | |
| Pop $R2 | |
| Pop $R1 | |
| Pop $R0 | |
| FunctionEnd |
🤖 Prompt for AI Agents
In build/nsis-installer.nsh around lines 15 to 17, the current ReadRegDWORD runs
in the 32-bit registry view and always checks the x64 runtime key, causing false
negatives on 64-bit/ARM64 systems; call SetRegView 64 before reading the key
(and restore the previous view afterwards), detect the native system
architecture (e.g. check PROCESSOR_ARCHITECTURE or use your existing
RunningX64/RunningARM64 helper), choose the correct runtime registry subkey
("Runtimes\x64" for x64 or "Runtimes\arm64" for ARM64) and then ReadRegDWORD
"Installed" from that path so the installer correctly detects the VC++
redistributable on both x64 and ARM64 systems.
| MessageBox MB_YESNO "\ | ||
| NOTE: ${PRODUCT_NAME} requires $\r$\n\ | ||
| 'Microsoft Visual C++ Redistributable'$\r$\n\ | ||
| to function properly.$\r$\n$\r$\n\ | ||
| Download and install now?" /SD IDYES IDYES InstallVCRedist IDNO DontInstall | ||
| InstallVCRedist: | ||
| inetc::get /CAPTION " " /BANNER "Downloading Microsoft Visual C++ Redistributable..." "https://aka.ms/vs/17/release/vc_redist.x64.exe" "$TEMP\vc_redist.x64.exe" | ||
| ExecWait "$TEMP\vc_redist.x64.exe /install /norestart" | ||
| ;IfErrors InstallError ContinueInstall ; vc_redist exit code is unreliable :( | ||
| Call checkVCRedist | ||
| ${If} $0 == "1" | ||
| Goto ContinueInstall | ||
| ${EndIf} | ||
|
|
||
| ;InstallError: | ||
| MessageBox MB_ICONSTOP "\ | ||
| There was an unexpected error installing$\r$\n\ | ||
| Microsoft Visual C++ Redistributable.$\r$\n\ | ||
| The installation of ${PRODUCT_NAME} cannot continue." | ||
| DontInstall: | ||
| Abort | ||
| ${EndIf} |
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.
Download and install the correct VC++ Redist for the running system (ARM64 vs x64) and clean up the temp file
Currently you always fetch vc_redist.x64.exe. On ARM64 systems (and ARM64 builds) this can install the wrong runtime and leave the app broken. Also, the downloaded file isn’t deleted.
Apply this diff to select the right URL and clean up:
- InstallVCRedist:
- inetc::get /CAPTION " " /BANNER "Downloading Microsoft Visual C++ Redistributable..." "https://aka.ms/vs/17/release/vc_redist.x64.exe" "$TEMP\vc_redist.x64.exe"
- ExecWait "$TEMP\vc_redist.x64.exe /install /norestart"
+ InstallVCRedist:
+ ; Pick correct redist for the current system architecture
+ Push $5
+ Push $6
+ StrCpy $5 "x64"
+ StrCpy $6 "https://aka.ms/vs/17/release/vc_redist.x64.exe"
+ ${If} $1 == "arm64"
+ StrCpy $5 "arm64"
+ StrCpy $6 "https://aka.ms/vs/17/release/vc_redist.arm64.exe"
+ ${EndIf}
+ inetc::get /CAPTION " " /BANNER "Downloading Microsoft Visual C++ Redistributable..." "$6" "$TEMP\vc_redist.$5.exe"
+ ExecWait "$TEMP\vc_redist.$5.exe /install /norestart"
+ Delete "$TEMP\vc_redist.$5.exe"
+ Pop $6
+ Pop $5Optional: If you prefer a fully silent redist install, add /quiet to the ExecWait command line.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| MessageBox MB_YESNO "\ | |
| NOTE: ${PRODUCT_NAME} requires $\r$\n\ | |
| 'Microsoft Visual C++ Redistributable'$\r$\n\ | |
| to function properly.$\r$\n$\r$\n\ | |
| Download and install now?" /SD IDYES IDYES InstallVCRedist IDNO DontInstall | |
| InstallVCRedist: | |
| inetc::get /CAPTION " " /BANNER "Downloading Microsoft Visual C++ Redistributable..." "https://aka.ms/vs/17/release/vc_redist.x64.exe" "$TEMP\vc_redist.x64.exe" | |
| ExecWait "$TEMP\vc_redist.x64.exe /install /norestart" | |
| ;IfErrors InstallError ContinueInstall ; vc_redist exit code is unreliable :( | |
| Call checkVCRedist | |
| ${If} $0 == "1" | |
| Goto ContinueInstall | |
| ${EndIf} | |
| ;InstallError: | |
| MessageBox MB_ICONSTOP "\ | |
| There was an unexpected error installing$\r$\n\ | |
| Microsoft Visual C++ Redistributable.$\r$\n\ | |
| The installation of ${PRODUCT_NAME} cannot continue." | |
| DontInstall: | |
| Abort | |
| ${EndIf} | |
| MessageBox MB_YESNO "\ | |
| NOTE: ${PRODUCT_NAME} requires $\r$\n\ | |
| 'Microsoft Visual C++ Redistributable'$\r$\n\ | |
| to function properly.$\r$\n$\r$\n\ | |
| Download and install now?" /SD IDYES IDYES InstallVCRedist IDNO DontInstall | |
| InstallVCRedist: | |
| ; Pick correct redist for the current system architecture | |
| Push $5 | |
| Push $6 | |
| StrCpy $5 "x64" | |
| StrCpy $6 "https://aka.ms/vs/17/release/vc_redist.x64.exe" | |
| ${If} $1 == "arm64" | |
| StrCpy $5 "arm64" | |
| StrCpy $6 "https://aka.ms/vs/17/release/vc_redist.arm64.exe" | |
| ${EndIf} | |
| inetc::get /CAPTION " " /BANNER "Downloading Microsoft Visual C++ Redistributable..." "$6" "$TEMP\vc_redist.$5.exe" | |
| ExecWait "$TEMP\vc_redist.$5.exe /install /norestart" | |
| Delete "$TEMP\vc_redist.$5.exe" | |
| Pop $6 | |
| Pop $5 | |
| ;IfErrors InstallError ContinueInstall ; vc_redist exit code is unreliable :( | |
| Call checkVCRedist | |
| ${If} $0 == "1" | |
| Goto ContinueInstall | |
| ${EndIf} | |
| ;InstallError: | |
| MessageBox MB_ICONSTOP "\ | |
| There was an unexpected error installing$\r$\n\ | |
| Microsoft Visual C++ Redistributable.$\r$\n\ | |
| The installation of ${PRODUCT_NAME} cannot continue." | |
| DontInstall: | |
| Abort | |
| ${EndIf} |
🤖 Prompt for AI Agents
In build/nsis-installer.nsh around lines 100 to 121, the installer always
downloads vc_redist.x64.exe and never removes the temp file; update the script
to detect the OS architecture at runtime (ARM64 vs x64), choose the matching
Microsoft URL (use vc_redist.arm64.exe for ARM64, vc_redist.x64.exe otherwise),
download into "$TEMP\vc_redist.<arch>.exe", run ExecWait with your desired flags
(optionally add /quiet), then Call checkVCRedist and finally Delete the
downloaded temp file (e.g. Delete "$TEMP\vc_redist.<arch>.exe") in both success
and error paths so the temp file is cleaned up.
Pull Request Description (中文)
你的功能请求是否与某个问题有关?请描述一下。
请对问题进行清晰扼要的描述。
部分windows用户遇到缺少vc++ runtime错误。
请描述你希望的解决方案
通过nsis脚本,在安装时检查环境,缺少则提示下载安装。
桌面应用程序的 UI/UX 更改
如果此 PR 引入了 UI/UX 更改,请详细描述它们。
无应用相关修改,仅修改nsh脚本。
平台兼容性注意事项
如果此 PR 具有特定的平台兼容性考虑因素(Windows、macOS、Linux),请在此处描述。
此脚本在社区版本上增加了对应平台的vcdist下载(原脚本固定下载x64版本)
Summary by CodeRabbit