fix(bindings): strip debug info on Windows to avoid Go 1.25 DWARF5 PE bug#5524
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds SyncFrontendDistToEmbedTarget and calls it from Build to copy frontend ChangesFrontend sync + bindings build
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes the bindings helper build by disabling CGO so wailsbindings.exe uses Go’s internal linker on Windows with Go 1.25. It also includes additional custom frontend embed-directory synchronization behavior that is outside the described CGO fix.
Changes:
- Sets
CGO_ENABLED=0when building the temporary bindings binary. - Adds
SyncFrontendDistToEmbedTargetand wires it into the build flow after frontend builds. - Adds tests for custom frontend
distsynchronization behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
v2/pkg/commands/bindings/bindings.go |
Disables CGO for the bindings helper build environment. |
v2/pkg/commands/build/build.go |
Adds custom frontend dist synchronization and cleanup during builds. |
v2/test/5034/frontend_dir_test.go |
Adds tests covering custom frontend sync/no-op behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@v2/pkg/commands/build/build.go`:
- Around line 213-232: The loop that copies frontendDistDir into each embed
target accumulates syncedDirs but returns noop on a CopyDir error, leaking
partially-copied directories; change the error return inside the loop to return
a cleanup closure bound to the current syncedDirs (which removes those copied
embedDistDir entries and restores any .gitkeep) along with the wrapped error
instead of noop, and ensure the Build caller defers the returned cleanup()
before checking the error so cleanup runs on failure; update references in this
fix around the for loop that uses embedDetails, the syncedDirs slice, the
fs.CopyDir call, and the cleanup()/noop return site.
- Around line 234-243: The cleanup closure in build.go recreates a ".gitkeep"
file but the rest of the codebase (CreateEmbedDirectories and project templates)
use "gitkeep", causing working-tree churn; update the cleanup function to create
"gitkeep" (not ".gitkeep") for each dir in syncedDirs and adjust any test
expectations that currently assert for ".gitkeep" to expect "gitkeep" instead
(refer to the cleanup variable/closure and syncedDirs in build.go to locate the
change).
In `@v2/test/5034/frontend_dir_test.go`:
- Around line 13-30: TestEmbedDiscoveryInTempDir currently only logs results and
never fails; update it to assert the expected outcome from
staticanalysis.GetEmbedDetails: ensure err is nil, assert embedDetails has the
expected count (e.g., 1), and verify the discovered embed path(s) match the
expected value (e.g., "frontend/dist") and/or that d.BaseDir/ d.GetFullPath()
point to the temp project; use t.Fatalf/t.Fatalf or the testify/require helpers
to make the test fail when these conditions are not met.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d6de05f6-f631-40a6-8b81-9107b26fc049
📒 Files selected for processing (3)
v2/pkg/commands/bindings/bindings.gov2/pkg/commands/build/build.gov2/test/5034/frontend_dir_test.go
leaanthony
force-pushed
the
fix/bindings-cgo-windows
branch
from
eccbd34 to
03fab14
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
v2/pkg/commands/bindings/bindings.go (1)
56-61:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: Implementation doesn't match PR title and objectives.
The PR title states "set CGO_ENABLED=0 when building wailsbindings binary" and the objectives explicitly describe setting
CGO_ENABLED=0to force the internal linker. However,CGO_ENABLED=0is never set in the environment. Instead, the code uses-ldflags=-linkmode=internal(see lines 64-69).If the bindings binary is pure Go as stated in the PR objectives, setting
CGO_ENABLED=0is the cleaner approach and would align with the PR title. It would also prevent any CGO dependencies from being accidentally introduced.🔧 Proposed fix to align with PR objectives
envBuild := os.Environ() envBuild = shell.SetEnv(envBuild, "GOOS", runtime.GOOS) envBuild = shell.SetEnv(envBuild, "GOARCH", runtime.GOARCH) +envBuild = shell.SetEnv(envBuild, "CGO_ENABLED", "0") // wailsbindings is executed on the build machine. // So, use the default C compiler, not the one set for cross compiling. envBuild = shell.RemoveEnv(envBuild, "CC")🤖 Prompt for 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. In `@v2/pkg/commands/bindings/bindings.go` around lines 56 - 61, The bindings build setup in bindings.go does not follow the stated objective because CGO_ENABLED is never disabled; update the environment construction in the bindings build flow to explicitly set CGO_ENABLED=0 alongside the existing GOOS/GOARCH and CC handling in the build setup. Keep the change centered in the code that prepares envBuild for wailsbindings so the binary is forced to be pure Go, and remove or avoid relying on the internal linker flag workaround if it is only compensating for CGO being enabled.
🤖 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 `@v2/pkg/commands/bindings/bindings.go`:
- Around line 63-70: The Windows-specific comment in
v2/pkg/commands/bindings/bindings.go is misleading about CGO: update the comment
around the buildArgs block (the portion that appends
"-ldflags=-linkmode=internal") to state clearly that the bindings package is
pure-Go and does not require CGO (CgoFiles is empty) and that the internal
linker flag is used only to avoid a Windows PE/loader issue introduced in Go
1.25; remove or rephrase "it still supports CGO" to avoid implying CGO is
required. Optionally, to guarantee a pure-Go build even if unrelated CGO sources
are present, set the environment variable CGO_ENABLED=0 when invoking the go
build for the bindings.
---
Outside diff comments:
In `@v2/pkg/commands/bindings/bindings.go`:
- Around line 56-61: The bindings build setup in bindings.go does not follow the
stated objective because CGO_ENABLED is never disabled; update the environment
construction in the bindings build flow to explicitly set CGO_ENABLED=0
alongside the existing GOOS/GOARCH and CC handling in the build setup. Keep the
change centered in the code that prepares envBuild for wailsbindings so the
binary is forced to be pure Go, and remove or avoid relying on the internal
linker flag workaround if it is only compensating for CGO being enabled.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eb326d9f-d28f-4412-8fe0-19df0dd48f19
📒 Files selected for processing (1)
v2/pkg/commands/bindings/bindings.go
leaanthony
changed the title
Test results from win-node1 (Windows 11, Build 26200)Tested on
PE headers in all cases: Conclusion: The bug could not be reproduced on Windows 11. The reported failures were on Windows 10 22H2 (Build 19045). This may be a Windows 10-specific issue, a pre-release-specific issue, or environment-specific (specific gcc version, CGO flags, etc.). The fix ( |
… bug
Go 1.25 switched to DWARF5 debug format by default. On Windows, the
internal linker emits malformed PE section headers when DWARF5 sections
are present, producing a binary that Windows refuses to execute:
fork/exec wailsbindings.exe: %1 is not a valid Win32 application.
fork/exec wailsbindings.exe: This version of %1 is not compatible
with the version of Windows you're running.
Pass -ldflags="-s -w" when building wailsbindings.exe on Windows.
Stripping debug info removes DWARF5 sections entirely, producing clean
PE headers. This is appropriate for a temporary tool binary that is
deleted immediately after use and never debugged.
The fix works regardless of whether the user's project uses CGO.
Upstream: golang/go#75077, golang/go#75121
Fixes #4605
Fixes #4551
leaanthony
force-pushed
the
fix/bindings-cgo-windows
branch
from
6e38440 to
c7b5070
Compare
|
Addressing review feedback: The PR branch was originally cut before PR #5251 was merged, so it inadvertently included those All review comments about The CodeRabbit comment about "it still supports CGO" wording referred to an earlier iteration that used |
Summary
Fixes #4605 and #4551.
Go 1.25 switched to DWARF5 debug format by default. On Windows, the internal linker emits malformed PE section headers when DWARF5 sections are present (golang/go#75077, golang/go#75121). The generated
wailsbindings.exefails to launch with:or
Downgrading to Go 1.24 fixes it because Go 1.24 used DWARF4 by default.
Root cause
golang/go#75077 documents the issue: DWARF5 sections are placed at virtual addresses outside the normal Windows PE address space, making the binary unrunnable. The fix in that issue is to strip debug info entirely with
-s -w.Fix
On Windows, pass
-ldflags="-s -w"to thego buildthat compileswailsbindings.exe. This strips debug symbols and DWARF sections from the binary, producing clean PE headers. It is appropriate for this binary because:wailsbindings.exeis a temporary tool deleted immediately after use (defer os.Remove(filename)) — it is never debuggedThe flag is only added on
runtime.GOOS == "windows"— no effect on macOS/Linux builds.Test plan
wails devwith Go 1.25.x on Windows (amd64), project without CGO — bindings generate successfullywails devwith Go 1.25.x on Windows (amd64), project with CGO — bindings generate successfullywails devwith Go 1.24 on Windows — still workswails devon macOS / Linux with Go 1.25 — unchanged (flag not added)go test ./v2/pkg/commands/bindings/...passesSummary by CodeRabbit
New Features
Bug Fixes
Tests