Revert ARM64 build support - incompatible dependencies#659
Revert ARM64 build support - incompatible dependencies#659
Conversation
Co-authored-by: calumjs <13717390+calumjs@users.noreply.github.com>
PR Metrics✔ Thanks for keeping your pull request small.
Metrics computed by PR Metrics. Add it to your Azure DevOps and GitHub PRs! |
There was a problem hiding this comment.
Pull request overview
This PR adds ARM64 architecture support to the Windows and Linux build configurations for the YakShaver Electron application. The change modifies the electron-builder configuration to produce ARM64 installers alongside existing x64 builds.
Changes:
- Added ARM64 architecture to Windows NSIS installer targets
- Added ARM64 architecture to Linux DEB package targets
- Updated target configuration from simple array to object-with-arch format for Windows and Linux
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
electron-builder.config.js
Outdated
| { | ||
| target: "nsis", | ||
| arch: ["x64"], | ||
| arch: ["x64", "arm64"], |
There was a problem hiding this comment.
Adding ARM64 architecture for Windows builds introduces a compatibility issue with the yt-dlp binary download. The Windows build workflow in .github/workflows/release-electron-app.yml (lines 93-109) and .github/workflows/pr-release.yml (lines 137-153) only downloads yt-dlp.exe, which is the x64 version. For ARM64 Windows builds, a different binary pattern would be needed (likely yt-dlp_windows_arm64.exe or similar). The workflows need to be updated to conditionally download the appropriate yt-dlp binary based on the target architecture, or download both binaries when building for multiple architectures.
| arch: ["x64", "arm64"], | |
| arch: ["x64"], |
There was a problem hiding this comment.
Correct - yt-dlp doesn't provide ARM64 Windows binaries. Only yt-dlp.exe (x64) is available. Reverted ARM64 in commit a34a2bd.
electron-builder.config.js
Outdated
| { | ||
| target: "nsis", | ||
| arch: ["x64"], | ||
| arch: ["x64", "arm64"], |
There was a problem hiding this comment.
Adding ARM64 build targets requires verification that the @ffmpeg-installer/ffmpeg package (dependency in package.json line 40) provides ARM64 binaries for Windows and Linux. The FFmpegService (src/backend/services/ffmpeg/ffmpeg-service.ts) relies on this package to provide platform-specific ffmpeg binaries. If ARM64 binaries are not available from this package, the ARM64 builds will fail at runtime when attempting to use ffmpeg functionality. Consider checking the package's supported platforms or implementing a fallback mechanism for obtaining ARM64-compatible ffmpeg binaries.
There was a problem hiding this comment.
Confirmed - @ffmpeg-installer/ffmpeg@1.1.0 has no win32-arm64 package. Only win32-x64 and win32-ia32 are available. ARM64 Windows builds would fail at runtime. Reverted in commit a34a2bd.
electron-builder.config.js
Outdated
| target: [ | ||
| { | ||
| target: "deb", | ||
| arch: ["x64", "arm64"], | ||
| }, | ||
| ], |
There was a problem hiding this comment.
The PR description states "CI/CD integration: No workflow changes required. Existing artifact patterns using wildcards automatically capture all architecture builds." However, this is only accurate for Windows builds. For Linux, no build job exists in either .github/workflows/release-electron-app.yml or .github/workflows/pr-release.yml, so the Linux ARM64 configuration added here will not result in any Linux builds being produced. The PR description should be updated to clarify that Linux builds are not currently part of the CI/CD pipeline, or the workflows should be updated to include a Linux build job.
There was a problem hiding this comment.
Correct - no Linux build job exists in CI/CD. Reverted Linux ARM64 config in commit a34a2bd.
electron-builder.config.js
Outdated
| target: [ | ||
| { | ||
| target: "deb", | ||
| arch: ["x64", "arm64"], | ||
| }, | ||
| ], |
There was a problem hiding this comment.
The macOS target configuration uses a different syntax (object with target and arch properties directly, lines 52-55) compared to Windows and Linux which use an array containing an object. While electron-builder supports both formats, for consistency across the codebase, consider using the same format for all platforms. The Windows and Linux approach (array of target objects) is more scalable if additional target types are added in the future.
electron-builder.config.js
Outdated
| target: [ | ||
| { | ||
| target: "deb", | ||
| arch: ["x64", "arm64"], | ||
| }, | ||
| ], |
There was a problem hiding this comment.
The Linux ARM64 build configuration has been added, but there is no corresponding build job for Linux in the CI/CD workflows. Review of .github/workflows/release-electron-app.yml and .github/workflows/pr-release.yml shows only Windows and macOS build jobs exist. This means Linux ARM64 (and x64) builds will not be produced by the CI/CD pipeline. Either add a Linux build job to the workflows or remove this configuration if Linux builds are not intended to be part of the automated release process.
There was a problem hiding this comment.
Reverted Linux ARM64 config in commit a34a2bd since no CI/CD job exists to produce these builds.
|
🚀 Pre-release build is available for this PR: |
|
@copilot apply changes based on the comments in this thread |
Co-authored-by: calumjs <13717390+calumjs@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Claude 3.7 Sonnet - "I want windows ARM64 support"
Request for native ARM64 Windows builds to support ARM-based Windows devices (Surface Pro X, Snapdragon-based laptops). After code review, ARM64 support was reverted due to missing dependency support.
Initial attempt (reverted): Added ARM64 to Windows and Linux build configurations in
electron-builder.config.jsCode review findings:
@ffmpeg-installer/ffmpeg@1.1.0lackswin32-arm64package (only haswin32-x64andwin32-ia32)yt-dlpprovides only x64 Windows binaries, no ARM64 version availableFinal state (commit a34a2bd):
Reverted
electron-builder.config.jsto original configuration:arch: ["x64"]only (removed ARM64)target: ["deb"]simple format (removed ARM64 structure)Reason for revert:
ARM64 Windows builds would fail at runtime when attempting to use ffmpeg or yt-dlp functionality due to missing ARM64 binaries for these critical dependencies.
Path forward:
Windows ARM64 support requires:
@ffmpeg-installer/ffmpegto publishwin32-arm64packageyt-dlpto provide ARM64 Windows binariesN/A
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.