-
Notifications
You must be signed in to change notification settings - Fork 301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix broken arm build #2017
Fix broken arm build #2017
Conversation
Caution Review failedThe pull request is closed. WalkthroughThe pull request implements a series of update changes across multiple configuration and documentation files. The build workflow configuration now activates the Linux ARM runner with uncommented lines, adds additional commands to install and initialize LXD, and includes a step for uploading Snapcraft logs upon failures. The packaging documentation is updated to list "binutils" as a new requirement. Additionally, the Electron builder configuration for Linux desktop entries has been refactored so that the desktop entry properties are now encapsulated within a nested "entry" object. The dependency specifications in the project manifest have been adjusted, with updated version ranges for both the electron-builder and electron-updater packages. There are no changes to public API declarations or exported entities. 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. 🔧 ESLint
electron-builder.config.cjsOops! Something went wrong! :( ESLint: 9.20.1 ConfigError: Config (unnamed): Unexpected key "0" found. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
package.json (1)
118-118
: Consider pinning electron-updater version.Using
^6.6
allows for automatic minor version updates which could introduce compatibility issues. Consider pinning to a specific version for better stability.- "electron-updater": "^6.6", + "electron-updater": "6.6.0",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (4)
.github/workflows/build-helper.yml
(3 hunks)BUILD.md
(1 hunks)electron-builder.config.cjs
(1 hunks)package.json
(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/build-helper.yml
27-27: label "ubuntu-24.04-arm" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Build for TestDriver.ai
- GitHub Check: Analyze (go)
🔇 Additional comments (4)
BUILD.md (1)
48-48
: LGTM! Added binutils as a required dependency.The addition of
binutils
is essential for ARM builds as it provides necessary tools for binary manipulation and linking.package.json (1)
66-66
:❓ Verification inconclusive
Verify compatibility with electron-builder v26.0.
The major version upgrade from v25 to v26 may include breaking changes.
Let's check the changelog for breaking changes:
🌐 Web query:
What are the breaking changes in electron-builder v26.0?
💡 Result:
The breaking changes in electron-builder v26.0 primarily focus on dependency management and build environment updates:
Breaking Changes
1. Native Dependency Handling
- Migrated from internal methods to electron-rebuild for compiling native dependencies during cross-platform builds[4][6].
- Affects projects relying on native modules (e.g.,
sqlite3
,sharp
).- Requires ensuring
electron-rebuild
is installed and configured if native modules are used.2. Build Environment Updates
- Upgraded Docker base image from
buildpack-deps:focal
tojammy
[4][6].
- Impacts Linux builds by changing default Python version (
python
→python3
) and Wine dependencies.3. Deprecated/Removed Features
- Removed internal Squirrel.Windows signing methods[6].
- Windows builds now sign the
vendor
directory directly instead of relying on Squirrel’s signing process[6].
Key Fixes and Additions in v26.x
- Added support for TypeScript configuration files (
electron-builder.ts
)[4].- Improved ARM64 support for cross-platform builds[3].
- Fixed circular dependency detection during module collection[6].
For migration guidance, refer to the electron-builder v26 release notes[4][6].
Citations:
- 1: https://releases.electronjs.org/release/v26.0.0
- 2: https://www.gitclear.com/open_repos/electron/electron/release/v26.0.0
- 3: https://learn.arm.com/learning-paths/laptops-and-desktops/electron/how-to-2/
- 4: https://github.com/electron-userland/electron-builder/blob/master/packages/electron-builder/CHANGELOG.md
- 5: https://www.gitclear.com/open_repos/electron/electron/release/v26.0.0-beta.12
- 6: https://github.com/electron-userland/electron-builder/releases
- 7: https://www.electron.build/index.html
- 8: https://electronjs.org/blog/electron-26-0
- 9: https://www.electron.build/electron-builder.Interface.ReleaseInfo.html
- 10: https://raw.githubusercontent.com/loopline-systems/electron-builder/master/CHANGELOG.md
Action Required: Verify Compatibility of Upgraded Dependencies
The dependency updates in
package.json
require careful review:
electron-builder (from v25.1.8 to v26.0):
The v26.0 upgrade introduces breaking changes such as:
- Migration to use electron-rebuild for native dependency handling (affecting modules like
sqlite3
orsharp
).- Updates in the build environment (e.g., Docker base image upgrade impacting Python and Wine dependencies on Linux builds).
- Removal of internal Squirrel.Windows signing methods, with changes to how Windows builds sign the
vendor
directory.Please review the v26.0 release notes and confirm that your build configurations and dependency usage are adjusted accordingly.
electron-updater (from 6.3.9 to ^6.6):
The update to a caret version increases flexibility. Ensure that any version constraints, update workflows, or integration points continue to work seamlessly.Test the build process across all target platforms to ensure there are no compatibility issues with these upgrades.
.github/workflows/build-helper.yml (2)
41-43
: LGTM! Added LXD setup for ARM builds.The addition of LXD installation and initialization is essential for containerized builds on ARM.
165-170
: LGTM! Added Snapcraft logs for better debugging.Uploading Snapcraft logs on failure will help diagnose ARM build issues.
The GitHub ARM runners have some bad dependencies. I'm adding a
snap refresh
andlxd init
to ensure these dependencies are good before we run the build.I'm also updating to the latest
electron-builder
andelectron-updater
.