ci: add Windows ARM64 build support in release workflow#202
Conversation
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 2fea17e in 7 seconds. Click for details.
- Reviewed
48lines of code in2files - Skipped
0files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_1fEG1oxLdl6CIgbu
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Greptile SummaryThis PR adds proper Windows ARM64 build support to both CI workflows by using the Key changes:
Technical approach:
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[CI Workflow Triggered] --> B{Platform Matrix}
B --> C[macOS Latest]
B --> D[Ubuntu 22.04/24.04]
B --> E[Ubuntu ARM]
B --> F[Windows Latest]
B --> G[Windows 11 ARM - NEW]
G --> H{Check matrix.args}
H -->|args present| I[Run: rustup target add aarch64-pc-windows-msvc]
H -->|args null| J[Skip target installation]
I --> K[Build with --target aarch64-pc-windows-msvc]
J --> L[Build without target args]
C --> M[Standard Build Steps]
D --> M
E --> M
F --> M
M --> N[Checkout → Dependencies → Build]
K --> N
L --> N
Last reviewed commit: 2fea17e |
|
Found the root cause of the Windows ARM64 compile failure. Root cause
The aw-tauri code itself has already been fixed in this PR (using FixOpened ActivityWatch/aw-server-rust#562 to replace Temporarily updated this PR's Cargo.toml to point at the fix branch so CI can be validated now. Will revert to |
Status updateActivityWatch/aw-server-rust#562 is now CI green (all 7 checks: format, clippy, ubuntu, macOS, windows, Android, coverage) and ready for review/merge. Current state
Next steps
Or if preferred: a maintainer can approve the CI here to verify the fix works before merging #562, since this PR's temporary Cargo.toml override already validates the fix. @0xbrayo — let me know if you'd like me to do step 2 once #562 is merged. |
|
The current CI failure is a transient network error — the runner failed to fetch the This is infrastructure-level flakiness, not a code issue. Path forward: Once ActivityWatch/aw-server-rust#562 (the appdirs → dirs fix) is merged, I can update this PR's |
CI fix: regenerated Cargo.lockRoot cause found: The Fixes applied:
CI should now be able to fetch the dependencies. Waiting for results. |
- Add windows-11-arm runner to matrix for aarch64-pc-windows-msvc - Add rustup target add step for cross-compilation targets Previously the Windows ARM64 entry used windows-latest (x64 runner) which cannot produce correct ARM64 artifacts. windows-11-arm is the correct native ARM64 runner. Fixes the issue reported by @0xbrayo in PR ActivityWatch#80.
79cf4b8 to
c908e4e
Compare
PR cleaned up — now mergeableRebased this PR onto current master and stripped out the temporary Cargo.toml/Cargo.lock changes that were previously added to work around the aw-webui submodule issue. What's in this PR now:
Removed from PR:
The PR is now mergeable and the diff is clean. CI still needs maintainer approval to run (first-time external contributor). |
|
update the cargo.toml to point back to activitywatch master |
|
The Cargo.toml already points to the upstream activitywatch master — the temporary fork references were removed in the cleanup rebase (mentioned in my 'PR cleaned up — now mergeable' comment above from Feb 21). The current PR only changes |
|
@ellipsis-dev review |
|
@TimeToBuildBob Seems this didn't touch the build.yml? Should also build for Windows ARM64 so we know that it works, before the release workflow is triggered on an actual release (a bad time to notice failing CI) |
Adds windows-11-arm runner with aarch64-pc-windows-msvc target to the build matrix, matching what release.yml already has. This ensures Windows ARM64 builds are verified on every push/PR, rather than discovering failures during an actual release. Also adds the `rustup target add` step for non-default targets. Requested-by: @ErikBjare in ActivityWatch#202
|
Good catch — you're right, build.yml didn't have it. Created #203 to add |
* ci(build): add Windows ARM64 to build workflow Adds windows-11-arm runner with aarch64-pc-windows-msvc target to the build matrix, matching what release.yml already has. This ensures Windows ARM64 builds are verified on every push/PR, rather than discovering failures during an actual release. Also adds the `rustup target add` step for non-default targets. Requested-by: @ErikBjare in #202 * fix: replace appdirs with dirs to fix Windows ARM64 build appdirs 0.2.0 pulls in ole32-sys and shell32-sys, which depend on winapi 0.2.8. That crate lacks aarch64-pc-windows-msvc support — it only defines ULONG_PTR/LONG_PTR for x86/x86_64 targets, causing compile errors on Windows ARM64. Replace appdirs with the dirs 5.0.1 crate, which uses windows-sys (no winapi dependency at all) and has full ARM64 support. Path equivalents used: - user_config_dir("activitywatch") -> config_dir() / "activitywatch" - user_data_dir("activitywatch") -> data_dir() / "activitywatch" - user_cache_dir("activitywatch") -> cache_dir() / "activitywatch" - user_log_dir (Windows) -> data_local_dir() / "activitywatch" / "Logs" - user_log_dir (macOS) -> home_dir() / "Library" / "Logs" / "activitywatch" All resulting paths match the previous appdirs behavior. * fix(windows): add winbase feature to winapi dependency * ci(build): pre-install WiX on Windows ARM64 to fix TLS bundling error * ci(build): install WiX to tauri cache dir on Windows ARM64 * fix(ci): pre-install NSIS + WiX for Windows ARM64 TLS workaround The windows-11-arm runner lacks TLS in its default HTTP client, causing tauri-bundler's internal downloader to fail for both WiX and NSIS. Previous commit fixed WiX only. After WiX succeeded, the build failed at NSIS download with the same 'no TLS backend' error. Now pre-downloads: - WiX 3.14 binaries - NSIS 3 base toolset - nsis_tauri_utils plugin v0.4.1 All downloaded via PowerShell (which has proper TLS) into the exact cache paths tauri-bundler expects (%LOCALAPPDATA%\tauri\). * fix(ci): rename extracted nsis-3.08/ to NSIS/ for tauri cache The nsis-3.zip contains files under nsis-3.08/ directory, but tauri-bundler expects them at %LOCALAPPDATA%\tauri\NSIS. The bundler normally extracts the zip then renames nsis-3.08 -> NSIS. Our pre-install step was missing the rename, so tauri didn't find the cached NSIS and tried to re-download (which fails due to TLS). * fix(ci): use correct nsis_tauri_utils version (v0.4.2 for tauri-cli 2.2.7) tauri-cli 2.2.7 expects nsis_tauri_utils v0.4.2 (not v0.4.1 from older tauri-v2.2.2 tag). Hash mismatch caused bundler to try re-downloading, which fails due to missing TLS on windows-11-arm runner. * ci: use --bundles msi for ARM64, skip NSIS (no TLS backend) * ci: retry ARM64 MSI build (previous run hit npm ECONNRESET) --------- Co-authored-by: TimeToBuildBob <timotobuildbob@gmail.com>
Fixes the Windows ARM64 CI issues raised in #80.
Changes:
windows-11-armrunner to release.yml matrix for theaarch64-pc-windows-msvctargetwindows-latest(x64 runner) for ARM64, which produces incorrect binariesrustup target addstep for builds using non-default targets (ARM64 cross-compilation)Note: The
build.ymldoesn't need the same fix since it doesn't have ARM64 targets in its matrix.