Skip to content

fix: prefer compiler runtimes in Windows packages - #1201

Merged
i386 merged 2 commits into
mainfrom
agent/fix-windows-runtime-dependency-abi
Aug 9, 2026
Merged

fix: prefer compiler runtimes in Windows packages#1201
i386 merged 2 commits into
mainfrom
agent/fix-windows-runtime-dependency-abi

Conversation

@i386

@i386 i386 commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Fixes #1160

What changed

  • discover the active MinGW compiler directory before PATH and Vulkan SDK directories
  • pass the compiler directory ahead of SDK dependencies during Windows CPU/Vulkan packaging
  • add a regression test proving the compiler runtime wins when DLL names collide

Root cause

The collector selected the first DLL by filename, and the Vulkan SDK directory was ahead of the compiler runtime. That could package an incompatible libstdc++/libgcc/winpthread closure into the artifact.

Validation

  • python3 -m unittest discover -s scripts/tests -p "test*.py" (386 passed, 7 skipped)
  • bash -n scripts/package-native-runtime.sh

Windows artifact loadability still needs validation on a clean Windows runner.

Summary by CodeRabbit

  • Bug Fixes

    • Improved Windows CPU and Vulkan runtime packaging to select the correct compiler runtime libraries.
    • Prevented conflicting Vulkan SDK DLLs from being selected over the compiler’s required runtime DLLs.
    • Updated dependency discovery to prioritize the configured compiler location and system PATH before Vulkan SDK directories.
  • Tests

    • Added regression coverage for correct runtime DLL selection when duplicate libraries are present.

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ce594a8a-ca12-405e-8cd8-438af16f1200

📥 Commits

Reviewing files that changed from the base of the PR and between a6db2fb and 0ce8703.

📒 Files selected for processing (1)
  • scripts/package-native-runtime.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • scripts/package-native-runtime.sh

📝 Walkthrough

Walkthrough

Windows runtime dependency collection now searches the selected MinGW compiler directory before PATH and Vulkan SDK directories. Packaging resolves the compiler from configured variables or g++, and tests verify selection of the compiler-provided runtime DLL.

Changes

Windows runtime dependency resolution

Layer / File(s) Summary
Prioritize compiler runtime directories
scripts/windows-native-runtime-deps.py, scripts/package-native-runtime.sh
The resolver checks the first available g++ or gcc directory before PATH and Vulkan SDK directories. Packaging resolves the compiler from MINGW_CXX, CXX, or g++, and fails when an explicit compiler is unavailable.
Regression coverage
scripts/tests/test_windows_native_runtime_deps.py
The test verifies that dependency collection selects and copies libstdc++-6.dll from the compiler directory instead of a conflicting Vulkan SDK copy.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • Mesh-LLM/mesh-llm#963: Both changes prioritize MinGW compiler runtime libraries over Vulkan SDK copies.
  • Mesh-LLM/mesh-llm#1046: This change extends the Windows Vulkan dependency resolution and regression coverage.

Suggested reviewers: ndizazzo

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the primary change: preferring compiler runtimes in Windows packages.
Linked Issues check ✅ Passed The changes address #1160 by prioritizing the build compiler runtime and testing selection when DLL names conflict with Vulkan SDK copies.
Out of Scope Changes check ✅ Passed All changes support the linked issue by updating Windows dependency resolution and adding targeted regression coverage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/fix-windows-runtime-dependency-abi

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

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

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

This pull request is currently a draft. Reviews will not take place until the PR is marked as ready for review.

@i386
i386 marked this pull request as ready for review August 8, 2026 18:36
@github-actions
github-actions Bot requested a review from ndizazzo August 8, 2026 18:37

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/package-native-runtime.sh`:
- Around line 461-470: Update the compiler lookup in the BACKEND cpu/vulkan
block to use the build-time resolved compiler path rather than passing the raw
MINGW_CXX or CXX value to command -v. Ensure wrapper or argument-containing
selections resolve correctly, and report or fail when an explicitly selected
compiler cannot be located instead of silently omitting its directory; preserve
the dependency_args search-dir addition for valid paths.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c5294c9a-cd54-4b56-9d1b-b8d0f1f0c6a9

📥 Commits

Reviewing files that changed from the base of the PR and between 915e691 and a6db2fb.

📒 Files selected for processing (3)
  • scripts/package-native-runtime.sh
  • scripts/tests/test_windows_native_runtime_deps.py
  • scripts/windows-native-runtime-deps.py

Comment thread scripts/package-native-runtime.sh
@i386
i386 merged commit 11e3cc1 into main Aug 9, 2026
42 checks passed
@i386
i386 deleted the agent/fix-windows-runtime-dependency-abi branch August 9, 2026 00:53
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.

Windows Vulkan runtime bundles a mismatched MinGW libstdc++

2 participants