Skip to content

Conversation

@hllshiro
Copy link
Collaborator

@hllshiro hllshiro commented Aug 18, 2025

Pull Request Description (中文)

你的功能请求是否与某个问题有关?请描述一下。
请对问题进行清晰扼要的描述。
部分windows用户遇到缺少vc++ runtime错误。

请描述你希望的解决方案
通过nsis脚本,在安装时检查环境,缺少则提示下载安装。

  • 安装平台不一致提示重新下载
82f123439c905fe74f2b8460cf892be5
  • 缺少运行时环境时提示下载,取消则终止安装
55be45ade1fbd28c5788984d69193590 image

桌面应用程序的 UI/UX 更改
如果此 PR 引入了 UI/UX 更改,请详细描述它们。
无应用相关修改,仅修改nsh脚本。

平台兼容性注意事项
如果此 PR 具有特定的平台兼容性考虑因素(Windows、macOS、Linux),请在此处描述。

  • 是否有任何平台特定的行为或代码调整?此修改仅与windows相关
  • 你是否已在所有相关平台上进行过测试?仅在windows x64测试,缺少arm环境测试

此脚本在社区版本上增加了对应平台的vcdist下载(原脚本固定下载x64版本)

Summary by CodeRabbit

  • New Features
    • Windows installer now verifies your system’s architecture (x64/ARM64/x86) and prevents installing incompatible builds, with a clear message and help link.
    • Automatically detects the presence of the required Microsoft Visual C++ Redistributable and offers to download and install it if missing.
    • Streamlined pre-install checks with guided prompts and safer abort behavior to avoid partial or broken installations.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 18, 2025

Walkthrough

Adds 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

Cohort / File(s) Summary
NSIS pre-install checks
build/nsis-installer.nsh
Introduces checkArchitectureCompatibility(), checkVCRedist(), and a customInit macro to gate installation on CPU architecture and VC++ Redistributable presence; handles user prompts, download via inetc::get, ExecWait, and abort paths.
Build configuration
electron-builder.yml
Adds nsis.include: build/nsis-installer.nsh to incorporate the new installer logic.

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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A rabbit taps the NSIS drum,
“Check your bits before we come!”
VC redist, hop to install—
or we won’t burrow in at all.
With builder’s map tucked in our vest,
we sniff, we fetch, we pass the test.
Thump-thump—setup’s done, now rest.

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
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch optimize/vsredist-check-nsh

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@hllshiro hllshiro marked this pull request as ready for review August 19, 2025 03:53
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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.

📥 Commits

Reviewing files that changed from the base of the PR and between b11d1a9 and 9c71078.

📒 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 — LGTM

Including 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 time

If 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?

Comment on lines +15 to +17
Function checkVCRedist
ReadRegDWORD $0 HKLM "SOFTWARE\Microsoft\VisualStudio\14.0\VC\Runtimes\x64" "Installed"
FunctionEnd
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
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.

Comment on lines +100 to +121
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}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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 $5

Optional: 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.

Suggested change
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.

@zerob13 zerob13 merged commit 4fa10dc into dev Aug 19, 2025
6 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Aug 19, 2025
@hllshiro hllshiro mentioned this pull request Aug 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants