Fix AMD64 appimage generation #3898
Conversation
WalkthroughThe pull request includes significant updates to the changelog, documenting new features, changes, and bug fixes. Key additions are the Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🔇 Additional comments (2)mkdocs-website/docs/en/changelog.md (2)Line range hint The file properly adheres to:
🧰 Tools🪛 LanguageTool[duplication] ~43-~43: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
The changelog entry correctly documents the AMD64 appimage compilation fix. However, it would be helpful to reference PR #3854 which introduced the original issue. Let's verify the connection between these PRs: Consider updating the entry to: -Fixed amd64 appimage compile by @atterpac in [#3898](https://github.com/wailsapp/wails/pull/3898)
+Fixed amd64 appimage compile (regression from [#3854](https://github.com/wailsapp/wails/pull/3854)) by @atterpac in [#3898](https://github.com/wailsapp/wails/pull/3898)✅ Verification successfulAdd reference to PR #3854 in the changelog entry Based on the PR description from #3898 which states "#3854 introduced a bug for compiling appimage for amd64 CPUs", and the fact that both PRs modify the same files, particularly Consider updating to: -Fixed amd64 appimage compile by @atterpac in [#3898](https://github.com/wailsapp/wails/pull/3898)
+Fixed amd64 appimage compile (regression from [#3854](https://github.com/wailsapp/wails/pull/3854)) by @atterpac in [#3898](https://github.com/wailsapp/wails/pull/3898)🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify the relationship between PR #3898 and PR #3854
# Expected: Find references to AMD64/amd64 architecture in PR #3854
# Check PR #3854 for AMD64/amd64 related changes
gh pr view 3854 --json body,comments | jq -r '.body, .comments[].body' | grep -i "amd64\|amd64"
Length of output: 97 Script: #!/bin/bash
# Let's try a different approach to find the connection between these PRs
# First, let's check the PR description and comments of #3898 to see if it mentions #3854
gh pr view 3898 --json body,comments | jq -r '.body, .comments[].body'
# Also, let's check for any appimage-related changes in both PRs
gh pr view 3898 --json files | jq -r '.files[].path'
gh pr view 3854 --json files | jq -r '.files[].path'
Length of output: 16048 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
|
changelog
atterpac
force-pushed
the
v3/fix-appimage-packaging
branch
from
699743d to
b3d678e
Compare
![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
🧹 Outside diff range and nitpick comments (1)
v3/internal/commands/appimage.go (1)
78-81: Consider adding a comment explaining the architecture mappings.While the implementation is correct, it would be helpful to add a comment explaining that these mappings align AppImage's expected architecture names with Go's runtime.GOARCH values.
// Architecture-specific variables using a map +// Maps Go's runtime.GOARCH values to AppImage's architecture naming convention archDetails := map[string]string{ "arm64": "aarch64", "amd64": "x86_64", "x86_64": "x86_64", }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
mkdocs-website/docs/en/changelog.md(1 hunks)v3/internal/commands/appimage.go(1 hunks)
🔇 Additional comments (3)
v3/internal/commands/appimage.go (1)
79-79: LGTM! The AMD64 architecture mapping is correct.
The addition of "amd64" mapping to "x86_64" is the right fix as these are equivalent architecture names, with "x86_64" being the standard notation used in AppImage filenames.
Let's verify the architecture handling across the codebase:
✅ Verification successful
Let me generate new shell scripts to verify the architecture handling across the codebase:
The AMD64 architecture mapping is correct and consistent across the codebase
The mapping of "amd64" to "x86_64" in the AppImage generation is correct and consistent with the rest of the codebase. The search results confirm:
- The mapping is used correctly for AppImage generation where "amd64" needs to be translated to "x86_64" for Linux compatibility
- The codebase consistently uses "amd64" internally as the architecture identifier
- The architecture handling is properly implemented across different build targets (Windows NSIS installers, macOS universal binaries, Linux AppImages)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify architecture handling consistency
# Expected: Find any other places where architecture mapping might need to be updated
# Check for other architecture mappings or constants
rg -g '!*.md' -i '(amd64|x86_64).*=|const.*(amd64|x86_64)'
# Check for architecture-specific logic
ast-grep --pattern 'if $_ == "amd64" { $$$ }'
Length of output: 100194
Script:
#!/bin/bash
# Search for architecture-related mappings and constants
rg -g '!*.md' -i '(amd64|x86_64).*=|const.*(amd64|x86_64)|map\[string\]string.*amd64'
# Search for architecture-specific build logic
ast-grep --pattern 'if $_.Arch == "amd64" { $$$ }'
Length of output: 27805
mkdocs-website/docs/en/changelog.md (2)
Line range hint 1-24: LGTM: Changelog follows proper formatting guidelines
The changelog properly follows the Keep a Changelog format:
- Clear sections for different types of changes (Added/Changed/Fixed)
- Proper attribution with GitHub usernames and PR links
- Clear version headers with dates
- Consistent formatting throughout
🧰 Tools
🪛 LanguageTool
[duplication] ~43-~43: Possible typo: you repeated a word
Context: ...ny](https://github.com/leaanthony) ### Fixed - Fixed amd64 appimage compile by @atterpac in ...
(ENGLISH_WORD_REPEAT_RULE)
44-44: Verify changelog entry matches PR objectives
The changelog entry correctly documents the fix for AMD64 appimage compilation, which aligns with the PR objectives. The entry follows the proper format with PR link and author attribution.
Let's verify if there are any related AMD64 appimage issues in the codebase:
✅ Verification successful
Changelog entry accurately reflects the code changes
The changelog entry correctly documents the fix for AMD64 appimage compilation. The code changes in v3/internal/commands/appimage.go show the addition of the "amd64" key to the archDetails map, which maps to "x86_64". This change directly addresses the issue mentioned in PR #3898, where AMD64 AppImage compilation was broken. The PR description confirms this was to fix a bug introduced in #3854.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for AMD64/amd64 related appimage code and issues
# Look for appimage-related code files
fd -t f appimage
# Search for AMD64/amd64 related code in appimage files
rg -i "amd64|AMD64" $(fd -t f appimage)
Length of output: 257
Script:
#!/bin/bash
# Let's check the specific changes in the PR related to AMD64 appimage
rg -A 5 -B 5 "amd64" v3/internal/commands/appimage.go
# Also check the test file for any related changes
cat v3/internal/commands/appimage_test.go
# Let's also check recent commits or PRs mentioning appimage and amd64
gh pr view 3898
Length of output: 5750
|
Nice catch! |
#3854 introduced a bug for compiling appimage for amd64 CPUs this PR adds the amd64 to the archdeatilas map to remedy the issue.
Summary by CodeRabbit
New Features
wails3 update build-assetsandwails3 generate runtimecommands.InitialPositionoption for window positioning.PathandPathsin the application package.Bug Fixes
AlwaysOnTopfunctionality on macOS and various platform-specific issues.