Skip to content

ci: make Windows tool setup idempotent and silence LNK4315 - #70

Merged
snape merged 1 commit into
mainfrom
fix-windows-builds
Jul 26, 2026
Merged

ci: make Windows tool setup idempotent and silence LNK4315#70
snape merged 1 commit into
mainfrom
fix-windows-builds

Conversation

@snape

@snape snape commented Jul 25, 2026

Copy link
Copy Markdown
Owner

No description provided.

@deepsource-io

deepsource-io Bot commented Jul 25, 2026

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in 8c564d2...8972520 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
C & C++ Jul 26, 2026 11:34p.m. Review ↗
Docker Jul 26, 2026 11:34p.m. Review ↗
Python Jul 26, 2026 11:34p.m. Review ↗
Secrets Jul 26, 2026 11:34p.m. Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Improved Windows build compatibility by handling linker warning 4315 during linking.
  • Chores

    • Improved Windows CI dependency setup by installing or updating tools only when needed.
    • Updated Windows test workflows to use Bazelisk for more consistent test execution.
    • Applied the same streamlined dependency management to Windows Meson builds.

Walkthrough

Windows Bazel linking now ignores linker code 4315. Windows CI workflows use conditional WinGet package checks to install or update tools, and Windows Bazel jobs invoke tests through Bazelisk.

Changes

Windows build and CI

Layer / File(s) Summary
Windows linker configuration
.bazelrc
Adds the Windows linker option /IGNORE:4315 to Bazel builds.
Windows CI tool provisioning
.github/workflows/ci.yml, .github/workflows/meson.yml
Repairs WinGet, checks Git, Bazelisk, CMake, Meson, and Ninja with exact matching, then silently installs missing packages or updates installed packages when updates are available.
Windows Bazel test invocation
.github/workflows/ci.yml
Changes the Windows Bazel test commands from bazel test to bazelisk test.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided, so there is nothing substantive to assess. Add a brief description of the Windows CI and Bazel linker changes so reviewers can quickly understand the intent.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the main changes: idempotent Windows tool setup and suppressing LNK4315.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

@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 @.github/workflows/ci.yml:
- Around line 447-448: Remove the “-r bazel” arguments from both Bazelisk
upgrade commands in .github/workflows/ci.yml at lines 447-448 and 494-495, while
preserving the Bazelisk package ID and ensuring the installation continues to
expose bazel for subsequent tests.
🪄 Autofix (Beta)

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: ASSERTIVE

Plan: Pro

Run ID: 4a95d35f-514c-4a57-ad05-dfdede48b4c8

📥 Commits

Reviewing files that changed from the base of the PR and between 8c564d2 and aeeb257.

📒 Files selected for processing (3)
  • .bazelrc
  • .github/workflows/ci.yml
  • .github/workflows/meson.yml

Comment thread .github/workflows/ci.yml Outdated
@snape
snape force-pushed the fix-windows-builds branch from aeeb257 to 0e6edf2 Compare July 26, 2026 00:14

@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 @.github/workflows/ci.yml:
- Around line 445-458: Replace each PowerShell `if (winget list --id …)`
condition with a WinGet existence check that suppresses command output and
branches on `$LASTEXITCODE`, or use `Get-WinGetPackage`. Apply this to
.github/workflows/ci.yml lines 445-458, 502-515, 982-995, and 1043-1056, plus
.github/workflows/meson.yml lines 415-435 and 477-497, preserving the existing
upgrade path for installed packages and install path for missing packages.
🪄 Autofix (Beta)

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: ASSERTIVE

Plan: Pro

Run ID: 0efa4907-4b3f-4aab-960c-33c2fb602944

📥 Commits

Reviewing files that changed from the base of the PR and between aeeb257 and 0e6edf2.

📒 Files selected for processing (3)
  • .bazelrc
  • .github/workflows/ci.yml
  • .github/workflows/meson.yml

Comment thread .github/workflows/ci.yml Outdated
@snape
snape force-pushed the fix-windows-builds branch 2 times, most recently from 490e810 to 7e6480b Compare July 26, 2026 00:32

@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 @.github/workflows/ci.yml:
- Around line 454-456: Remove the `-r bazel` arguments from both Bazelisk
upgrade commands in `.github/workflows/ci.yml` at lines 454-456 and 511-513,
leaving each `winget upgrade` targeted only at `Bazel.Bazelisk`; retain `-r
bazel` on the install paths.
🪄 Autofix (Beta)

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: ASSERTIVE

Plan: Pro

Run ID: d90f7dc4-4ee4-4828-9920-15c86b7bc704

📥 Commits

Reviewing files that changed from the base of the PR and between 490e810 and 7e6480b.

📒 Files selected for processing (3)
  • .bazelrc
  • .github/workflows/ci.yml
  • .github/workflows/meson.yml

Comment thread .github/workflows/ci.yml Outdated
@snape
snape force-pushed the fix-windows-builds branch 14 times, most recently from db66d04 to 3062fd0 Compare July 26, 2026 23:28
Signed-off-by: Jamie Snape <snape@me.com>
@snape
snape force-pushed the fix-windows-builds branch from 3062fd0 to 8972520 Compare July 26, 2026 23:34

@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 @.bazelrc:
- Line 52: Replace the Windows build option on the affected .bazelrc entry with
the compiler warning-disable flag /wd4315, removing the linker-only /IGNORE:4315
option so C4315 is suppressed during compilation.
🪄 Autofix (Beta)

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: ASSERTIVE

Plan: Pro

Run ID: ba4aaa80-342f-4000-8c2a-983e5b8b6894

📥 Commits

Reviewing files that changed from the base of the PR and between 7e6480b and 8972520.

📒 Files selected for processing (3)
  • .bazelrc
  • .github/workflows/ci.yml
  • .github/workflows/meson.yml

Comment thread .bazelrc
@snape
snape merged commit 6cb905c into main Jul 26, 2026
42 checks passed
@snape
snape deleted the fix-windows-builds branch July 26, 2026 23:48
snape added a commit that referenced this pull request Jul 27, 2026
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.

1 participant