Skip to content

Conversation

Minh141120
Copy link
Member

@Minh141120 Minh141120 commented Sep 24, 2025

Describe Your Changes

This pull request adds support for automatically installing the Vulkan Runtime during Windows application setup. The changes ensure the Vulkan Runtime installer is downloaded, bundled with the app, and installed if not already present on the user's system.

Fixes Issues

Self Checklist


Important

Adds Vulkan Runtime installer download and installation during Windows setup, updating scripts, configuration, and installer hooks.

  • Behavior:
    • Adds Vulkan Runtime installer download to download-lib.mjs.
    • Bundles VulkanRT-X64-1.4.321.0-Installer.exe in tauri.windows.conf.json.
    • Installs Vulkan Runtime if not present using hooks.nsh.
  • Scripts:
    • Updates download-lib.mjs to download Vulkan Runtime installer and copy to resources.
  • Configuration:
    • Adds Vulkan Runtime installer to resources in tauri.windows.conf.json.
  • Installer Hooks:
    • Adds logic in hooks.nsh to check and install Vulkan Runtime if not already installed.

This description was created by Ellipsis for 545aaf1. You can customize this summary. It will automatically update as commits are pushed.

@Minh141120 Minh141120 marked this pull request as ready for review September 24, 2025 09:32
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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 545aaf1 in 1 minute and 45 seconds. Click for details.
  • Reviewed 88 lines of code in 3 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. scripts/download-lib.mjs:110
  • Draft comment:
    Log or handle errors in the copySync try/catch block for the Vulkan installer instead of silently swallowing them.
  • 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 suggests changing error handling, but this code follows an established pattern in the file. The comment "Expect EEXIST error" suggests they're specifically only catching expected file-already-exists errors and letting other errors propagate up. This is actually a reasonable pattern - catch expected errors, let unexpected ones bubble up to the main error handler. The main() function already has error handling that will log and exit. I could be wrong about the intention of the error handling pattern. Maybe there are other important errors that should be handled specifically for the Vulkan installer. The consistent pattern across multiple copySync operations and the presence of top-level error handling strongly suggests this is intentional error handling design, not an oversight. The comment should be deleted as the current error handling follows an established pattern in the codebase and appears intentionally designed this way.
2. src-tauri/tauri.windows.conf.json:8
  • Draft comment:
    Ensure that bundling the Vulkan Runtime Installer is intended and consider integrating an integrity or version check if needed.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 50% <= threshold 50% The comment suggests ensuring that bundling the Vulkan Runtime Installer is intended, which is not allowed. However, it also suggests considering integrating an integrity or version check, which is a specific suggestion. The first part of the comment should be removed, but the second part is valid.
3. src-tauri/windows/hooks.nsh:51
  • Draft comment:
    Avoid duplicating the Vulkan installer version string; consider using a macro or constant so that version updates occur in one place only.
  • Reason this comment was not posted:
    Marked as duplicate.
4. src-tauri/windows/hooks.nsh:54
  • Draft comment:
    Enhance error handling for the ExecWait call for the Vulkan installer by providing more detailed logging for non-zero exit codes.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative and does not provide a specific suggestion or request for change. It does not align with the rules for good comments, as it doesn't ask for a specific action or clarification from the PR author.

Workflow ID: wflow_TgDsVeoEOzSPsFCz

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

}

// Download Vulkan Runtime Installer
const vulkanRtFilename = 'VulkanRT-X64-1.4.321.0-Installer.exe'
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider refactoring the hard-coded Vulkan installer version and URL into configurable constants to simplify future updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant