[v3] Pass build flags to binding generator #4023
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request updates the changelog to document various changes, additions, and fixes in the project. Key updates include the renaming of several service methods, the introduction of support for aarch64 AppImage builds, and enhancements to the Changes
Suggested labels
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/src/content/docs/changelog.mdx (1)
49-49: LGTM! Consider enhancing the changelog entry.The changelog entry accurately documents the fix. However, it could be more helpful to users if it included a brief note about the impact, such as "ensuring consistent rebuilds when build flags change".
-Pass build flags to binding generator by [@fbbdev](https://github.com/fbbdev) in [#4023](https://github.com/wailsapp/wails/pull/4023) +Pass build flags to binding generator by [@fbbdev](https://github.com/fbbdev) in [#4023](https://github.com/wailsapp/wails/pull/4023) - ensures consistent rebuilds when build flags changev3/internal/commands/build_assets/linux/Taskfile.yml (1)
89-89: Minor whitespace cleanup in package generation tasks.Cosmetic changes to remove trailing spaces after
cmds:declarations.Also applies to: 94-94, 99-99
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/src/content/docs/changelog.mdx(1 hunks)v3/internal/commands/build_assets/Taskfile.tmpl.yml(3 hunks)v3/internal/commands/build_assets/darwin/Taskfile.yml(1 hunks)v3/internal/commands/build_assets/linux/Taskfile.yml(2 hunks)v3/internal/commands/build_assets/windows/Taskfile.yml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run Go Tests (ubuntu-latest, 1.23)
- GitHub Check: Run Go Tests (macos-latest, 1.23)
- GitHub Check: Run Go Tests (windows-latest, 1.23)
🔇 Additional comments (5)
v3/internal/commands/build_assets/Taskfile.tmpl.yml (2)
34-36: LGTM! BUILD_FLAGS propagation added correctly.The addition of BUILD_FLAGS to the frontend build task ensures proper propagation of build flags to dependent tasks.
52-52: LGTM! Binding generator now receives build flags.The removal of the
sourcessection (not visible in diff) and the addition of BUILD_FLAGS to the command ensures that:
- The binding generator runs on every build, fixing the issue where it wouldn't rerun when build flags changed
- Build flags are correctly propagated to the binding generator
v3/internal/commands/build_assets/windows/Taskfile.yml (1)
13-16: LGTM! Consistent BUILD_FLAGS implementation.The BUILD_FLAGS and PRODUCTION variables are correctly referenced, maintaining consistency with the template implementation.
v3/internal/commands/build_assets/darwin/Taskfile.yml (1)
12-14: LGTM! Platform-specific BUILD_FLAGS implementation.The BUILD_FLAGS reference is correctly implemented while preserving macOS-specific build configurations.
v3/internal/commands/build_assets/linux/Taskfile.yml (1)
12-14: LGTM! Consistent BUILD_FLAGS implementation.The BUILD_FLAGS reference is correctly implemented, maintaining consistency with other platforms.
The `/wails:include` directive allows for the inclusion of additional JS/TS files in the generated bindings.
fbbdev
force-pushed
the
v3-alpha-bugfix/bindgen-build-flags
branch
from
a66b23a to
b9970f9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
v3/internal/commands/build_assets/Taskfile.tmpl.yml(3 hunks)v3/internal/term/term.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run Go Tests (ubuntu-latest, 1.23)
- GitHub Check: Run Go Tests (macos-latest, 1.23)
- GitHub Check: Run Go Tests (windows-latest, 1.23)
🔇 Additional comments (3)
v3/internal/commands/build_assets/Taskfile.tmpl.yml (3)
86-86: Formatting change only.
25-25: LGTM! The changes improve build tracking and dependency management.The updates correctly implement:
- Task labeling with PRODUCTION state for better tracking
- Comprehensive artifact tracking with wildcards
- BUILD_FLAGS dependency to trigger rebuilds when flags change
Let's verify the task rerun behavior:
Also applies to: 31-31, 35-37
✅ Verification successful
✅ Verified: Task configuration correctly implements build flag propagation
The implementation properly handles:
- Production mode tracking in task label
- Build flags propagation through generate:bindings dependency
- Comprehensive artifact tracking with
dist/**/*🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the task properly tracks dependencies and reruns # Check if the task label and deps are properly configured rg -A 5 'label: build:frontend.*PRODUCTION' && \ rg -A 5 'BUILD_FLAGS:\s+ref: \.BUILD_FLAGS'Length of output: 567
Script:
#!/bin/bash # Get the complete frontend build task definition and check for build flags rg -A 20 '^ build:frontend:' v3/internal/commands/build_assets/Taskfile.tmpl.yml # Also check for any build flags references in the file rg -i 'build.*flags' v3/internal/commands/build_assets/Taskfile.tmpl.ymlLength of output: 962
47-47: LGTM! The changes improve binding generation reliability.The updates correctly implement:
- Build flags tracking in task label
- Precise source file tracking with proper exclusions
- Proper handling of dev/production mode changes
- Clean command formatting with proper flag passing
Let's verify the bindings generation behavior:
Also applies to: 52-54, 59-59, 61-61
✅ Verification successful
Binding generation task changes are correctly implemented
The verification confirms:
- Task label properly tracks BUILD_FLAGS
- Source patterns correctly handle frontend exclusions and bindings tracking
- Command structure properly handles flags and typescript option
- Generated bindings directory setup is correct
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify binding generation patterns and command structure # Check source patterns echo "Verifying source patterns..." fd -t f -e ts -e js . | grep -v 'frontend/' || true fd -t f -e ts -e js 'frontend/bindings' || true # Check command structure echo "Verifying command structure..." rg 'wails3 generate bindings.*BUILD_FLAGS.*-clean=true' -A 2Length of output: 110986
Script:
#!/bin/bash # Description: Verify binding generation directory and command structure # Check if frontend/bindings directory exists echo "Checking frontend/bindings directory..." fd -t d 'frontend/bindings' || true # Verify command structure with better grep pattern echo "Verifying command structure..." rg -l 'wails3.*generate.*bindings' || true # Check specific task file content echo "Checking Taskfile content..." cat v3/internal/commands/build_assets/Taskfile.tmpl.yml || trueLength of output: 3114
| @@ -74,7 +74,7 @@ func Warning(input any) { | |||
| } | |||
|
|
|||
| func Warningf(input any, args ...any) { | |||
| output(input, pterm.Warning, args) | |||
| output(input, pterm.Warning, args...) | |||
There was a problem hiding this comment.
LGTM! Critical bug fix for variadic argument handling.
The change correctly expands the variadic arguments when passing them to the output function. This fixes a potential bug where format strings expecting multiple arguments would receive the slice as a single argument instead.
There was a problem hiding this comment.
@leaanthony Here's the committable code:
func Warningf(input any, args ...any) {
output(input, pterm.Warning, args...)
}
Description
This patch fixes an issue with template Taskfiles whereby Go build flags are not correctly propagated to the binding generator. Due to a limitation in the Taskfile implementation, the job won't rerun automatically when the flags change.
Additionally, it ensures the PRODUCTION mode variable is propagated to the frontend build task, and tracks both variables as dependencies using the task label method.
Somewhat counterintuitively, both the binding generator's output and Vite's output are included in the two tasks' sources. That is done to ensure dev builds rerun just after a production build alters bindings or frontend artifacts, or vice versa. Because the binding generator's output and Vite's output are both deterministic, that won't induce an infinite rerun loop, but will stabilise after two runs.
Preliminary discussion happened on the Discord feedback channel at https://discord.com/channels/1042734330029547630/1329148312171057172
Type of change
How Has This Been Tested?
Various templates have been tested on macOS by running both
wails3 dev,wails3 buildandwails3 package, and verifying that the binding generator:runs every time;
receives the same flags that are passed to
go build.Windows
macOS
Linux
Test Configuration
Checklist:
website/src/pages/changelog.mdxwith details of this PRI have commented my code, particularly in hard-to-understand areasI have made corresponding changes to the documentationSummary by CodeRabbit
Summary by CodeRabbit
New Features
wails doctorcommandwindow-callexample for service method callsapp.OpenFileManager()methodImprovements
Bug Fixes
Documentation