Skip to content

ci: harden Go Windows linker setup#49

Merged
willkill07 merged 2 commits into
NVIDIA:mainfrom
willkill07:wkk_ci-go-windows-linker-hardening
May 3, 2026
Merged

ci: harden Go Windows linker setup#49
willkill07 merged 2 commits into
NVIDIA:mainfrom
willkill07:wkk_ci-go-windows-linker-hardening

Conversation

@willkill07
Copy link
Copy Markdown
Member

@willkill07 willkill07 commented May 3, 2026

Overview

Harden the Go binding CI recipe so Windows jobs reliably select the clang/lld linker path when GitHub-hosted runner shell metadata changes.

  • I confirm this contribution is my own work, or I have the right to submit it under this project's license.
  • I searched existing issues and open pull requests, and this does not duplicate existing work.

Details

  • Detect Windows from RUNNER_OS, OSTYPE, or uname -s instead of relying only on OSTYPE.
  • Force the Windows Go CGO linker to clang/clang++ and keep the existing -fuse-ld=lld behavior so Go does not fall back to MinGW GCC for the MSVC-built FFI library.
  • Emit the selected Windows linker settings in CI logs to make future failures easier to diagnose.

Where should the reviewer start?

Start with justfile, specifically the test-go recipe's Windows detection and linker setup.

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

  • Relates to: none

Summary by CodeRabbit

  • Chores
    • Improved Windows platform detection in the build system to check multiple environment variables for more reliable identification.
    • Reorganized Windows-specific build configurations for better maintainability.
    • Refined CI build process conditional logic.

Signed-off-by: Will Killian <wkillian@nvidia.com>
@willkill07 willkill07 requested a review from a team as a code owner May 3, 2026 20:15
@github-actions github-actions Bot added size:S PR is small ci labels May 3, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 3, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: fec02a62-472c-4358-b494-a08adebf0596

📥 Commits

Reviewing files that changed from the base of the PR and between bcc7b33 and b6197f4.

📒 Files selected for processing (1)
  • justfile
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
  • GitHub Check: WebAssembly / Test / macos-arm64
  • GitHub Check: WebAssembly / Test / windows-amd64
  • GitHub Check: Go / Test / windows-arm64
  • GitHub Check: WebAssembly / Test / windows-arm64
  • GitHub Check: Rust / Test / windows-arm64
  • GitHub Check: Go / Test / windows-amd64
  • GitHub Check: Rust / Test / windows-amd64
  • GitHub Check: Python / Test / windows-amd64
  • GitHub Check: Python / Test / macos-arm64
  • GitHub Check: Node.js / Test / windows-arm64
  • GitHub Check: Node.js / Test / macos-arm64
  • GitHub Check: Node.js / Test / windows-amd64
  • GitHub Check: Go / Test / macos-arm64
  • GitHub Check: Python / Test / windows-arm64
  • GitHub Check: Rust / Test / macos-arm64
🧰 Additional context used
📓 Path-based instructions (2)
justfile

📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)

justfile: Use just set-version <version> command to update release-version source files including Cargo.toml workspace package version, workspace dependencies, and Node package metadata
Keep helper code functions set_project_version, set_cargo_workspace_version, and set_node_package_version aligned with the same version fields updated by the workflow

Files:

  • justfile
{.github/**,.gitlab-ci.yml,.pre-commit-config.yaml,justfile,scripts/**}

⚙️ CodeRabbit configuration file

{.github/**,.gitlab-ci.yml,.pre-commit-config.yaml,justfile,scripts/**}: Review automation changes for reproducibility, pinned versions where appropriate, secret handling, and consistency with the documented validation matrix.
Pay attention to commands that need generated native artifacts, FFI libraries, or platform-specific environment variables.

Files:

  • justfile
🔇 Additional comments (1)
justfile (1)

786-827: Windows gating and linker setup look consistent.

Consolidating detection behind is_windows keeps the clang/lld path, PATH adjustment, and diagnostic logging on the same branch, which reduces the chance of Windows-specific setup drifting across the recipe.


Walkthrough

The test-go target refactors Windows OS detection by introducing an is_windows computed flag derived from RUNNER_OS, OSTYPE, and uname -s, replacing scattered pattern-matching logic. Windows-specific behavior—toolchain setup, linker flags, tool installation—now consistently gates on this flag.

Changes

OS Detection and Windows Configuration

Layer / File(s) Summary
OS Detection Setup
justfile (lines 786–798)
New is_windows boolean computed from RUNNER_OS, OSTYPE, and uname -s output; replaces direct OSTYPE pattern matching for Windows detection.
Toolchain Configuration
justfile (lines 786–798)
Windows-only CC/CXX clang setup now gates on computed is_windows flag instead of raw OSTYPE matching.
CI Coverage Integration
justfile (lines 818–824)
In CI mode, Windows-specific linker flag (-w), tool installation (go-junit-report, gocover-cobertura), and path setup consolidated under single if [[ "$is_windows" == true ]] condition.
Diagnostic Output
justfile (lines 825–827)
Windows-only debug echo prints detected RUNNER_OS, OSTYPE, uname, CC, CXX, and accumulated go_ldflags.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title follows Conventional Commits format with 'ci' type and concise imperative summary, is under 72 characters, uses lowercase, contains no trailing period, and accurately reflects the main change of hardening Windows linker setup.
Description check ✅ Passed The description includes all required sections (Overview with checkboxes, Details, Where to start, Related Issues) and provides specific technical details about the Windows detection mechanism and linker hardening changes.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Review rate limit: 8/10 reviews remaining, refill in 11 minutes and 2 seconds.

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

@willkill07 willkill07 self-assigned this May 3, 2026
@willkill07 willkill07 merged commit c200917 into NVIDIA:main May 3, 2026
59 checks passed
@willkill07 willkill07 added this to the 0.2.0 milestone May 4, 2026
@willkill07 willkill07 deleted the wkk_ci-go-windows-linker-hardening branch May 6, 2026 18:40
@willkill07 willkill07 added the Maintenance CI or Build or general repository maintenance label May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Maintenance CI or Build or general repository maintenance size:S PR is small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant