fix(icons): emit warning instead of silent no-op when Assets.car generation is unsupported - #5309
Conversation
…ration is unsupported `wails3 generate icons -iconcomposerinput appicon.icon -macassetdir darwin` previously exited 0 with no output when Assets.car could not be generated (wrong platform, macOS < 26, or actool unavailable). Users assumed the command worked because there was no error, but their icon artwork did not update. Each early-exit in generateMacAsset() now returns a descriptive wrapped error preserving errors.Is(err, ErrMacAssetNotSupported) semantics via %w. GenerateIcons() prints a Warning: line to stderr on the fallthrough path instead of silently swallowing it. Flag help text for -iconcomposerinput and -macassetdir surfaces the macOS 26 / actool 26+ requirement. Test skip condition for the Assets.car test also checks actool functionality. New TestGenerateIconsFallthrough verifies .ico is still produced when Assets.car is unavailable.
WalkthroughThe PR enhances the icon generation command to gracefully handle unsupported platforms when generating macOS assets via Icon Composer. When Assets.car generation fails on platforms lacking macOS 26+ or actool 26+, the code now emits a warning to stderr and continues generating icons from alternate input if available, preventing silent failures while maintaining backward compatibility. ChangesIcon Generation Fallback and Error Handling
Sequence DiagramsequenceDiagram
participant User
participant GenerateIcons
participant generateMacAsset
participant stderr
participant Filesystem
User->>GenerateIcons: Call with Input + IconComposerInput
GenerateIcons->>generateMacAsset: Attempt Assets.car generation
alt Mac Asset Unsupported
generateMacAsset-->>GenerateIcons: ErrMacAssetNotSupported (wrapped)
GenerateIcons->>stderr: Emit warning message
GenerateIcons->>GenerateIcons: Check options.Input available
alt Input Available
GenerateIcons->>Filesystem: Generate .ico from Input
GenerateIcons-->>User: Success with .ico only
else Input Empty
GenerateIcons-->>User: Error (no Input fallback)
end
else Mac Asset Supported
generateMacAsset->>Filesystem: Write Assets.car
generateMacAsset-->>GenerateIcons: Success
GenerateIcons->>Filesystem: Generate .ico from Input
GenerateIcons-->>User: Success with both Assets.car and .ico
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related Issues
Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.11.4)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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
There was a problem hiding this comment.
Pull request overview
Updates the wails3 generate icons command to surface when macOS Assets.car generation is skipped (unsupported platform/version/tooling), instead of silently doing nothing, while keeping the rest of icon generation working.
Changes:
- Wrap
ErrMacAssetNotSupportedwith descriptive context ingenerateMacAsset()while preservingerrors.Is(..., ErrMacAssetNotSupported)via%w. - Print a warning to stderr (and continue with input-based icon generation) when
Assets.cargeneration is unsupported but-inputis provided. - Update tests to skip macOS asset generation when
actoolisn’t functional, and add a fallthrough test ensuring.icogeneration still succeeds.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
v3/internal/commands/icons.go |
Adds contextual unsupported-reason errors and emits a stderr warning instead of silently swallowing unsupported Assets.car generation. |
v3/internal/commands/icons_test.go |
Improves darwin-only skip conditions and adds a test ensuring .ico generation still works when mac asset generation is unavailable. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| IconComposerInput string `description:"The input Icon Composer file (.icon) [requires macOS 26 or later with Xcode CLT actool 26+]"` | ||
| MacAssetDir string `description:"The output directory for the Mac assets (Assets.car and icons.icns) [requires macOS 26 or later with Xcode CLT actool 26+]"` |
| versionPlist, err := cmd.Output() | ||
| if err != nil { | ||
| return ErrMacAssetNotSupported | ||
| return fmt.Errorf("Assets.car generation requires actool 26 or later (actool not found at /usr/bin/actool — install Xcode): %w", ErrMacAssetNotSupported) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
v3/internal/commands/icons_test.go (1)
346-372: ⚡ Quick win
TestGenerateIconsFallthroughdoesn't assert that the warning was emitted toos.Stderr.The primary behavioral change in this PR — printing
"Warning: …"to stderr — is never verified. If thefmt.Fprintf(os.Stderr, …)call were accidentally removed or guarded by a wrong condition, this test would still pass as long as the.icofile is produced.Capturing stderr in a Go test is straightforward with
os.Pipe():🧪 Proposed stderr-capture addition
func TestGenerateIconsFallthrough(t *testing.T) { _, thisFile, _, _ := runtime.Caller(0) localDir := filepath.Dir(thisFile) exampleIcon := filepath.Join(localDir, "build_assets", "appicon.png") exampleIconFile := filepath.Join(localDir, "build_assets", "appicon.icon") tmpDir := t.TempDir() icoOut := filepath.Join(tmpDir, "appicon.ico") + // Capture os.Stderr to verify the warning is emitted on the fallthrough path. + origStderr := os.Stderr + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("os.Pipe: %v", err) + } + os.Stderr = w options := &IconsOptions{ Input: exampleIcon, WindowsFilename: icoOut, IconComposerInput: exampleIconFile, MacAssetDir: tmpDir, } - if err := GenerateIcons(options); err != nil { + genErr := GenerateIcons(options) + w.Close() + os.Stderr = origStderr + var stderrBuf strings.Builder + io.Copy(&stderrBuf, r) + + if genErr != nil { t.Fatalf("GenerateIcons() unexpected error: %v", err) } + // On platforms where Assets.car is unsupported the warning must be emitted. + // On macOS 26+ with actool 26+ the happy path runs and no warning is expected. + if runtime.GOOS != "darwin" { + if !strings.Contains(stderrBuf.String(), "Warning:") { + t.Errorf("expected a Warning on stderr, got: %q", stderrBuf.String()) + } + } f, err := os.Stat(icoOut) if err != nil { t.Fatalf("expected %s to exist: %v", icoOut, err) } if f.Size() == 0 { t.Fatal("appicon.ico is empty") } }You'll also need
"io"and"strings"in the import block.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v3/internal/commands/icons_test.go` around lines 346 - 372, Update TestGenerateIconsFallthrough to capture and assert stderr output: wrap the GenerateIcons(options) call with an os.Pipe-based capture of os.Stderr (save original os.Stderr, replace with pipe writer, close writer and read from pipe after call), then assert the captured string (use strings.Contains or similar) includes the expected "Warning:" message; keep test flow and file existence/size checks intact and restore os.Stderr afterwards. Target symbols: TestGenerateIconsFallthrough, GenerateIcons, IconsOptions and the fmt.Fprintf(os.Stderr, ...) warning emission.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@v3/internal/commands/icons_test.go`:
- Around line 346-372: Update TestGenerateIconsFallthrough to capture and assert
stderr output: wrap the GenerateIcons(options) call with an os.Pipe-based
capture of os.Stderr (save original os.Stderr, replace with pipe writer, close
writer and read from pipe after call), then assert the captured string (use
strings.Contains or similar) includes the expected "Warning:" message; keep test
flow and file existence/size checks intact and restore os.Stderr afterwards.
Target symbols: TestGenerateIconsFallthrough, GenerateIcons, IconsOptions and
the fmt.Fprintf(os.Stderr, ...) warning emission.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e9c7486c-8b92-486b-8706-79ad97f4974d
📒 Files selected for processing (2)
v3/internal/commands/icons.gov3/internal/commands/icons_test.go
leaanthony
added
Bug
|
Mac test: ✅ PASS — macOS 26.3, Xcode CLT only (no actool) Tested on a clean macOS 26.3 VM with Xcode Command Line Tools installed (no full Xcode, actool absent at Build: Unit tests:
Smoke test (
The fix behaves exactly as described: users on CLT-only setups now receive an actionable warning instead of a silent no-op. Full package test: |
Linux CI — PR #5309 test results ✅Tested on Ubuntu 24.04, Go 1.25.0, commit Build
Unit tests
Key results for the changed package:
The new Smoke test
Help text correctly shows updated descriptions: Linux verdict: PR-READY — no regressions, warning path verified. |
Windows test results — PR #5309Tested on: Windows 11, Go 1.26.2 windows/amd64, Unit tests
43 PASS, 2 SKIP, 0 FAIL ✅ Key results:
CLI smoke test (fallthrough path)VerdictThe fix works correctly on Windows:
No issues found on Windows. ✅ |
…mit warning instead of silent no-op when Assets.car generation is unsupported
Closes #5260.
Problem
wails3 generate icons -iconcomposerinput appicon.icon -macassetdir darwinexited 0 with no output when Assets.car couldn't be generated (wrong platform, macOS < 26, or actool unavailable). Users assumed the command worked because there was no error, butdarwin/Assets.carwas never updated.Changes
v3/internal/commands/icons.goreturn ErrMacAssetNotSupportedingenerateMacAsset()is now a descriptivefmt.Errorf("...: %w", ErrMacAssetNotSupported), covering platform, macOS version, actool availability, and actool version.errors.Is(err, ErrMacAssetNotSupported)semantics preserved via%w.GenerateIcons()now printsWarning: <reason>to stderr on the fallthrough path instead of silently swallowing the error.-iconcomposerinputand-macassetdirflag descriptions mention the macOS 26 + Xcode actool 26+ requirement.v3/internal/commands/icons_test.gorequireDarwinskip also checks actool functionality (handles macOS 26 machines with CLT but not full Xcode).TestGenerateIconsFallthrough: verifies.icois still produced when Assets.car is unavailable.Before
After
Test results
Summary by CodeRabbit
New Features
Bug Fixes
Tests