bugfix : fix error on building runnable .app on MacOS when 'name' and 'outputfilename' are different. #3789
Conversation
…veProjectData to export OutputFilename \n mod : modified CFBundleExecutable values to .OutputFilename in info.dev.plist, info.plist
WalkthroughThe changes involve modifications to the build configuration for macOS applications, specifically in the handling of executable names. The Changes
Assessment against linked issues
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
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
v2/pkg/buildassets/buildassets.go (1)
117-119: LGTM: Initialization of OutputFilename in resolveProjectDataThe addition of
OutputFilename: projectData.OutputFilenamein theassetDatainitialization is correct and necessary to make the new field available for template resolution. This change, combined with the struct modification, effectively addresses the issue described in #3786.Consider adding a validation check to ensure that
projectData.OutputFilenameis not empty before using it. This could prevent potential issues if theOutputFilenameis not set in the project configuration. Here's a suggested implementation:data := &assetData{ Name: projectData.Name, Info: projectData.Info, - OutputFilename: projectData.OutputFilename, + OutputFilename: projectData.OutputFilename, +} + +if data.OutputFilename == "" { + return nil, fmt.Errorf("OutputFilename is not set in project data") }This addition would make the function more robust against potential configuration errors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- v2/pkg/buildassets/build/darwin/Info.dev.plist (1 hunks)
- v2/pkg/buildassets/build/darwin/Info.plist (1 hunks)
- v2/pkg/buildassets/buildassets.go (2 hunks)
- website/src/pages/changelog.mdx (1 hunks)
🔇 Additional comments (13)
v2/pkg/buildassets/build/darwin/Info.plist (2)
Line range hint
1-67: Additional files and changes to reviewWhile the change in this file is crucial and correctly addresses the main issue, there are other important aspects of this PR that we haven't been able to review:
- The changes to
assetDatastruct andresolveProjectDatafunction mentioned in the PR summary.- The update to the changelog as indicated in the PR checklist.
- The changes to
info.dev.plistfile, also mentioned in the PR summary.To ensure a comprehensive review, it would be beneficial to examine these additional files and changes.
To locate the relevant files and changes, please run the following script:
#!/bin/bash # Description: Locate relevant files and changes for the PR # Search for assetData struct echo "Searching for assetData struct:" rg --type go 'type\s+assetData\s+struct' -A 10 echo -e "\nSearching for resolveProjectData function:" rg --type go 'func\s+resolveProjectData' -A 10 echo -e "\nSearching for changelog updates:" rg --type md 'fix.*error.*building.*runnable.*app.*MacOS' -g 'CHANGELOG.md' echo -e "\nSearching for info.dev.plist file:" fd 'info.dev.plist'This will help us locate and review all the relevant changes for this PR.
9-9: Approved: Critical fix for macOS app bundle executable namingThis change correctly addresses the issue where the executable file was not generated properly when 'name' and 'outputfilename' differed. By using
{{.OutputFilename}}instead of{{.Name}}for theCFBundleExecutablekey, the Info.plist will now correctly reference the actual executable name, allowing the application to run as expected.To ensure consistency across the project, please run the following script to check if similar changes are needed in other plist files, particularly
info.dev.plist:This will help us verify if the fix has been applied consistently across all relevant files.
v2/pkg/buildassets/build/darwin/Info.dev.plist (1)
9-9: Correct implementation to use OutputFilenameThis change correctly implements the use of
{{.OutputFilename}}instead of{{.Name}}for theCFBundleExecutablekey. This aligns with the PR objective and should resolve the issue where the executable file was not generated correctly when 'name' and 'outputfilename' fields in wails.json were different.To ensure consistency across the project, please run the following command to check if a similar change has been made in the
Info.plistfile:If the command doesn't return a result, it might indicate that the
Info.plistfile needs a similar update.v2/pkg/buildassets/buildassets.go (2)
105-107: LGTM: Addition of OutputFilename field to assetData structThe addition of the
OutputFilenamefield to theassetDatastruct is a good solution to address the issue described in #3786. This change allows for separate handling of the 'outputfilename' and 'name' values, which is crucial for fixing the build error on MacOS when these values differ.
Line range hint
1-146: Summary: Changes effectively address the build issue on MacOSThe modifications to
v2/pkg/buildassets/buildassets.gosuccessfully introduce support for handling 'outputfilename' separately from 'name'. These changes are crucial for resolving the build error on MacOS when these values differ, as described in issue #3786.To ensure these changes have the intended effect across the codebase, please run the following verification script:
This script will help ensure that the
OutputFilenameis correctly used throughout the project, particularly in theinfo.plistfiles and any build scripts that might reference the executable name.✅ Verification successful
Further Verification Needed: Expand Search for
OutputFilenameUsageTo ensure that all instances of
OutputFilenameare correctly handled across the codebase, please run the following verification scripts:These scripts will help identify any additional usages of
OutputFilenamein various configuration and build script files, ensuring comprehensive verification of the changes.
Verification Successful:
OutputFilenameProperly Handled Across CodebaseThe recent changes to
v2/pkg/buildassets/buildassets.gocorrectly implement separate handling ofOutputFilename, as evidenced by its proper usage in theInfo.plistandInfo.dev.plistfiles. No issues were found in other configuration or build script files.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of OutputFilename in template files and other relevant locations # Test 1: Check if OutputFilename is used in info.plist files echo "Checking info.plist files for OutputFilename usage:" rg --type xml "OutputFilename" -g "*info*.plist" # Test 2: Verify if there are any remaining instances where Name is used instead of OutputFilename for executable names echo "Checking for potential missed Name usage instead of OutputFilename:" rg --type xml "CFBundleExecutable.*Name" -g "*info*.plist" # Test 3: Check other Go files for proper handling of OutputFilename echo "Checking other Go files for OutputFilename handling:" rg --type go "OutputFilename" # Test 4: Verify if there are any build scripts that might need updating echo "Checking build scripts for potential updates:" rg "outputfilename" -g "*.sh" -g "*.bat" -g "*.ps1"Length of output: 2658
Script:
#!/bin/bash # Description: Comprehensive search for OutputFilename usage across all relevant files # Test 1: Search for OutputFilename in any plist files echo "Searching for OutputFilename in all plist files:" rg "OutputFilename" -g "*.plist" # Test 2: Search for OutputFilename in JSON configuration files echo "Searching for OutputFilename in JSON files:" rg "OutputFilename" -g "*.json" # Test 3: Search for OutputFilename in YAML configuration files echo "Searching for OutputFilename in YAML files:" rg "OutputFilename" -g "*.yaml" -g "*.yml" # Test 4: Search for OutputFilename in all build-related scripts without restricting file types echo "Searching for OutputFilename in all build-related scripts (any script file):" rg "OutputFilename" -g "*.sh" -g "*.bat" -g "*.ps1" -g "*.make" -g "*.build" -g "*.gradle" -g "*.xml"Length of output: 853
website/src/pages/changelog.mdx (8)
34-35: New feature: Fixed file permissions for generated filesThis change improves security and consistency by setting appropriate file permissions for generated files.
Line range hint
39-41: Multiple bug fixes and improvementsSeveral issues have been addressed, including fixes for the plain template, website layout, and Windows-specific problems.
🧰 Tools
🪛 LanguageTool
[grammar] ~31-~31: The correct preposition appears to be “on”.
Context: ...ps://github.com/mrf345) - Fixed dialogs in Windows when using Go 1.23 in [PR](http...(IN_WINDOWS)
Line range hint
43-46: Changes to development workflowThe development process has been improved with changes to dependency installation and asset handling.
🧰 Tools
🪛 LanguageTool
[grammar] ~31-~31: The correct preposition appears to be “on”.
Context: ...ps://github.com/mrf345) - Fixed dialogs in Windows when using Go 1.23 in [PR](http...(IN_WINDOWS)
Line range hint
1-7: Changelog format and versioningThe changelog follows the Keep a Changelog format and uses Semantic Versioning, which is a good practice for maintaining clear and consistent release notes.
🧰 Tools
🪛 LanguageTool
[grammar] ~31-~31: The correct preposition appears to be “on”.
Context: ...ps://github.com/mrf345) - Fixed dialogs in Windows when using Go 1.23 in [PR](http...(IN_WINDOWS)
Line range hint
9-16: Changelog categoriesThe changelog uses standard categories (Added, Changed, Deprecated, Removed, Fixed, Security) which helps in quickly identifying the nature of changes.
🧰 Tools
🪛 LanguageTool
[grammar] ~31-~31: The correct preposition appears to be “on”.
Context: ...ps://github.com/mrf345) - Fixed dialogs in Windows when using Go 1.23 in [PR](http...(IN_WINDOWS)
Line range hint
18-19: Latest version: v2.0.0The changelog indicates that Wails has reached a major version 2.0.0, which likely includes significant changes or improvements.
🧰 Tools
🪛 LanguageTool
[grammar] ~31-~31: The correct preposition appears to be “on”.
Context: ...ps://github.com/mrf345) - Fixed dialogs in Windows when using Go 1.23 in [PR](http...(IN_WINDOWS)
Line range hint
1-1639: Well-maintained and informative changelogThe Wails changelog is comprehensive, well-structured, and follows best practices for changelog maintenance. It provides clear information about new features, bug fixes, and breaking changes across versions, which is valuable for users and contributors.
🧰 Tools
🪛 LanguageTool
[grammar] ~31-~31: The correct preposition appears to be “on”.
Context: ...ps://github.com/mrf345) - Fixed dialogs in Windows when using Go 1.23 in [PR](http...(IN_WINDOWS)
Line range hint
48-49: Deprecation notice: Fullscreen appoptionThe
Fullscreenapplication option is being deprecated in favor ofWindowStartState.#!/bin/bash # Check for usage of deprecated Fullscreen option grep -r "Fullscreen" . --include="*.go"🧰 Tools
🪛 LanguageTool
[grammar] ~31-~31: The correct preposition appears to be “on”.
Context: ...ps://github.com/mrf345) - Fixed dialogs in Windows when using Go 1.23 in [PR](http...(IN_WINDOWS)
| ### Changed | ||
| - Modified docs to reflect the correct password syntax for the `gon-sign.json` file [PR](https://github.com/wailsapp/wails/pull/3620) by [@ignasbernotas](github.com/ignasbernotas) |
There was a problem hiding this comment.
💡 Codebase verification
CSS Drag Detection Feature Not Implemented
The UseCSSDrag flag is set to true in the changelog files; however, there is no corresponding implementation in the codebase to enable CSS-based drag detection.
🔗 Analysis chain
New experimental feature: CSS Drag detection
This adds support for CSS-based drag detection, which could improve user interaction with draggable elements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the new CSS drag detection is properly implemented
grep -r "UseCSSDrag" .
Length of output: 5366
Script:
#!/bin/bash
# Verify if the new CSS drag detection is properly implemented
grep -rn "UseCSSDrag" .
Length of output: 878
Script:
#!/bin/bash
# Search for usage of UseCSSDrag in conditional statements or feature implementations
rg "UseCSSDrag" --context 2
Length of output: 3729
mod : added OutputFilename field to assetData
mod : modified resolveProjectData to export OutputFilename
mod : modified CFBundleExecutable values to .OutputFilename in info.dev.plist, info.plist
Description
There was an issue where the executable file was not properly generated when the name and outputfilename in the existing wails.json were different. The cause of this issue was that outputfilename was not being used under pkg/buildassets. As a result, I have modified the code to properly export outputfilename and use it in CFBundleExecutable as well.
Fixes #3786
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
After building Wails with the modified source code,
go build ./cmd/wailsI built a test project using the generated binary.
Test Configuration
Checklist:
website/src/pages/changelog.mdxwith details of this PRI have commented my code, particularly in hard-to-understand areasI have added tests that prove my fix is effective or that my feature worksSummary by CodeRabbit
OutputFilenamefield to enhance asset data structure.outputfilenameandnamefields in thewails.jsonfile.