Skip to content

refactor(release): stage every ToDesktop target from one loop, test the rule - #1506

Open
benceruleanlu wants to merge 1 commit into
bl/arm64-beforebuild-verifies-shipped-pathfrom
bl/arm64-staging-loop-and-invariants
Open

benceruleanlu wants to merge 1 commit into
bl/arm64-beforebuild-verifies-shipped-pathfrom
bl/arm64-staging-loop-and-invariants

Conversation

@benceruleanlu

@benceruleanlu benceruleanlu commented Sep 8, 2026

Copy link
Copy Markdown
Member

Part of a stack of review fixes for #1486. Based on #1505.

Summary

The Linux staging added in #1486 repeated the Windows loop body in a different style, and the tests pinned that shell line verbatim along with the exact electron-builder path string. Quoting a path or folding the two together broke the suite while the release was untouched, and the assertions never checked the thing that matters: that the paths todesktop.json declares are the ones CI stages.

Feature behavior

Staging folds into a single source:target loop, with the ARM64 placeholder remaining the one special case. Verified to produce a byte-identical tree to the previous version by running both against the same fixture and diffing.

Test coverage and validation

The literal pins are replaced with the rules that actually govern packaging, each checked for every platform rather than Linux alone:

  • Every architecture of a platform declares the same resource shape, the destination plus the source basename in order. Windows had no coverage before.
  • Each target sources from the staging directory named after it. This is what catches an architecture reaching into another one's directory, which is the Linux ARM64 bug fix(linux): reject x64 runtimes on ARM64 #1486 fixes and the Windows one fix(release): align Windows target resource paths and bump rc.4 #1484 fixed. The shape rule cannot see it, since every source basename is bootstrap-python either way. Verified by pointing Windows ARM64 at windows-x64 and Linux ARM64 at linux-x64: both fail.
  • Every target directory declared in todesktop.json is staged by the release workflow. Verified by adding an unstaged target: it fails.
  • Adding -- to the mv in the workflow, a behaviour-preserving change, keeps all tests passing. The previous suite failed on it.
  • The electron-builder assertion now requires the ${arch} macro and no hardcoded architecture, rather than one exact string.

The file is renamed off the arm64 prefix now that it covers every platform. Full unit suite: 274 files, 4736 passed, 2 skipped. Typecheck, lint and format pass.

Change breakdown

Total changed lines: 274 (174 added, 100 deleted).

Product code

  • 0 files; +0 / -0; 0 changed lines; 0%.

Test code

  • 2 files; +169 / -95; 264 changed lines; 96.4% of total.
  • src/main/sources/standalone/arm64Packaging.test.ts
  • src/main/sources/standalone/packaging.test.ts

Documentation

  • 0 files; +0 / -0; 0 changed lines; 0%.

Configuration and CI

  • 1 files; +5 / -5; 10 changed lines; 3.6% of total.
  • .github/workflows/build-release.yml

Generated files

  • 0 files; +0 / -0; 0 changed lines; 0%.

Lockfiles

  • 0 files; +0 / -0; 0 changed lines; 0%.

Vendored code

  • 0 files; +0 / -0; 0 changed lines; 0%.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 42 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

This review ran on the open-source allowance, not this organization's plan, because the pull request author doesn't have an assigned seat. Waiting won't change this — ask an organization admin to assign them a seat, or add seats in Billing if every seat is already assigned, then retry.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: f9210522-4b53-4390-8558-11b3f59778e5

📥 Commits

Reviewing files that changed from the base of the PR and between d94aa83 and 2b9c6a5.

📒 Files selected for processing (3)
  • .github/workflows/build-release.yml
  • src/main/sources/standalone/arm64Packaging.test.ts
  • src/main/sources/standalone/packaging.test.ts

Comment @coderabbitai help to get the list of available commands.

…he rule

The Linux staging repeated the Windows loop body in a different style, and the
tests pinned that shell line verbatim plus the exact electron-builder path
string — so quoting a path or folding the two together broke the suite while
the release was untouched, and the assertions never checked that the paths
todesktop.json declares are the ones CI stages.

Fold both into a single source:target loop (verified to produce a
byte-identical tree) and replace the text pins with the rules that matter:
every architecture of a platform shares a resource shape, each target sources
from the staging directory named after it, and every declared target is
staged by the workflow. The middle rule is what catches an architecture
reaching into another one's directory, on Windows as well as Linux — neither
platform had that covered. Renamed off the arm64 prefix now that it covers
every platform.
@benceruleanlu
benceruleanlu force-pushed the bl/arm64-staging-loop-and-invariants branch from 588bea0 to 2b9c6a5 Compare September 9, 2026 01:33
@benceruleanlu
benceruleanlu removed this pull request from stack #1510 September 9, 2026 01:34
@benceruleanlu
benceruleanlu added this pull request to stack #1513 September 9, 2026 01:34

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants