From 2b9c6a5c8bfed96994fdac8e95408a37d58a5dad Mon Sep 17 00:00:00 2001 From: Benjamin Lu Date: Tue, 8 Sep 2026 15:43:17 -0700 Subject: [PATCH] refactor(release): stage every ToDesktop target from one loop, test the rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .github/workflows/build-release.yml | 10 +- .../sources/standalone/arm64Packaging.test.ts | 95 ---------- src/main/sources/standalone/packaging.test.ts | 169 ++++++++++++++++++ 3 files changed, 174 insertions(+), 100 deletions(-) delete mode 100644 src/main/sources/standalone/arm64Packaging.test.ts create mode 100644 src/main/sources/standalone/packaging.test.ts diff --git a/.github/workflows/build-release.yml b/.github/workflows/build-release.yml index 262e66d55..3d3b03d28 100644 --- a/.github/workflows/build-release.yml +++ b/.github/workflows/build-release.yml @@ -160,14 +160,14 @@ jobs: shell: bash run: | set -euo pipefail - for arch in x64 arm64; do - source="bootstrap-python/win-${arch}" - target="todesktop-targets/windows-${arch}/bootstrap-python" + # :. Every target that + # todesktop.json sources from todesktop-targets/ needs an entry here. + for pair in win-x64:windows-x64 win-arm64:windows-arm64 linux-x64:linux-x64; do + source="bootstrap-python/${pair%%:*}" + target="todesktop-targets/${pair##*:}/bootstrap-python" mkdir -p "$(dirname "$target")" mv "$source" "$target" done - mkdir -p todesktop-targets/linux-x64 - mv bootstrap-python/linux-x64 todesktop-targets/linux-x64/bootstrap-python # ToDesktop requires matching resource paths across architectures. # Keep the ARM64 directory nonempty for upload, without a Python binary. mkdir -p todesktop-targets/linux-arm64/bootstrap-python diff --git a/src/main/sources/standalone/arm64Packaging.test.ts b/src/main/sources/standalone/arm64Packaging.test.ts deleted file mode 100644 index a87587a16..000000000 --- a/src/main/sources/standalone/arm64Packaging.test.ts +++ /dev/null @@ -1,95 +0,0 @@ -import fs from 'fs' -import path from 'path' -import { parse } from 'yaml' -import { describe, expect, it } from 'vitest' - -interface ExtraResource { - from: string - to: string -} - -interface ToDesktopConfig { - extraResources?: ExtraResource[] - targetOverrides?: Record> - platformOverrides?: Record -} - -function readToDesktopConfig(): ToDesktopConfig { - return JSON.parse( - fs.readFileSync(path.join(process.cwd(), 'todesktop.json'), 'utf-8') - ) as ToDesktopConfig -} - -const destinations = (resources: ExtraResource[] = []): string[] => - resources.map((resource) => resource.to).sort() - -describe('Linux ARM64 packaging', () => { - // A target list replaces, rather than merges with, the platform list, and - // ToDesktop decides server-side which architectures a platform builds. An - // architecture we did not enumerate falls back to `platformOverrides`, and - // without one it lands on the top-level list — `./lib` alone, so the - // package ships with no apparmor-profile and no bootstrap-python. - it('keeps a platform-level fallback for every platform with per-target overrides', () => { - const config = readToDesktopConfig() - const platforms = Object.keys(config.targetOverrides ?? {}) - expect(platforms.length).toBeGreaterThan(0) - - for (const platform of platforms) { - const fallback = config.platformOverrides?.[platform] - expect(fallback, `platformOverrides.${platform} is missing`).toBeDefined() - for (const [arch, target] of Object.entries(config.targetOverrides![platform]!)) { - expect( - destinations(fallback!.extraResources), - `platformOverrides.${platform} must cover every destination of targetOverrides.${platform}.${arch}` - ).toEqual(destinations(target.extraResources)) - } - } - }) - - it('uses matching ToDesktop resource paths with a separate ARM64 placeholder', () => { - const config = readToDesktopConfig() - const linuxTargets = config.targetOverrides?.linux - const x64Resources = linuxTargets?.x64?.extraResources ?? [] - const arm64Resources = linuxTargets?.arm64?.extraResources ?? [] - const x64Bootstrap = x64Resources.find((resource) => resource.to === 'bootstrap-python') - const arm64Bootstrap = arm64Resources.find((resource) => resource.to === 'bootstrap-python') - - // ToDesktop permits architecture-specific sources, but every target must - // have the same destinations and source basenames, in the same order. - const resourcePaths = (resources: ExtraResource[]): string[] => - resources.map((resource) => path.posix.join(resource.to, path.posix.basename(resource.from))) - expect(resourcePaths(arm64Resources)).toEqual(resourcePaths(x64Resources)) - - expect(x64Bootstrap).toEqual({ - from: './todesktop-targets/linux-x64/bootstrap-python', - to: 'bootstrap-python' - }) - expect(path.posix.basename(x64Bootstrap!.from)).toBe(x64Bootstrap!.to) - expect(arm64Bootstrap).toEqual({ - from: './todesktop-targets/linux-arm64/bootstrap-python', - to: 'bootstrap-python' - }) - }) - - it('stages the Linux x64 bootstrap at the target-specific ToDesktop path', () => { - const workflow = fs.readFileSync( - path.join(process.cwd(), '.github', 'workflows', 'build-release.yml'), - 'utf-8' - ) - - expect(workflow).toContain( - 'mv bootstrap-python/linux-x64 todesktop-targets/linux-x64/bootstrap-python' - ) - }) - - it('resolves local electron-builder bootstraps from the target architecture', () => { - const config = parse( - fs.readFileSync(path.join(process.cwd(), 'electron-builder.yml'), 'utf-8') - ) as { linux?: { extraResources?: ExtraResource[] } } - const bootstrap = config.linux?.extraResources?.find( - (resource) => resource.to === 'bootstrap-python' - ) - - expect(bootstrap?.from).toBe('bootstrap-python/linux-${arch}') - }) -}) diff --git a/src/main/sources/standalone/packaging.test.ts b/src/main/sources/standalone/packaging.test.ts new file mode 100644 index 000000000..e03864401 --- /dev/null +++ b/src/main/sources/standalone/packaging.test.ts @@ -0,0 +1,169 @@ +import fs from 'fs' +import path from 'path' +import { parse } from 'yaml' +import { describe, expect, it } from 'vitest' + +// Packaging config lives at the repo root; these tests sit under src/ because +// vitest only collects `src/**/*.test.ts`. They assert the rules ToDesktop and +// electron-builder impose on the resource lists, not the spelling of any one +// path — a release must not break because a path was quoted differently. +interface ExtraResource { + from: string + to: string +} + +interface ToDesktopConfig { + extraResources?: ExtraResource[] + targetOverrides?: Record> + platformOverrides?: Record +} + +const repoRoot = process.cwd() + +function readToDesktopConfig(): ToDesktopConfig { + return JSON.parse( + fs.readFileSync(path.join(repoRoot, 'todesktop.json'), 'utf-8') + ) as ToDesktopConfig +} + +const destinations = (resources: ExtraResource[] = []): string[] => + resources.map((resource) => resource.to).sort() + +/** What ToDesktop compares across a platform's targets: the destination and + * the basename of the source, in declaration order. */ +const resourceShape = (resources: ExtraResource[] = []): string[] => + resources.map((resource) => path.posix.join(resource.to, path.posix.basename(resource.from))) + +/** Every target directory staged under `todesktop-targets/`, as declared by + * the config that packaging reads. */ +function stagedTargetDirs(config: ToDesktopConfig): string[] { + const lists = [ + ...Object.values(config.targetOverrides ?? {}).flatMap((archs) => + Object.values(archs).map((target) => target.extraResources) + ), + ...Object.values(config.platformOverrides ?? {}).map((p) => p.extraResources), + config.extraResources + ] + const dirs = new Set() + for (const list of lists) { + for (const resource of list ?? []) { + const match = /^\.\/todesktop-targets\/([^/]+)\//.exec(resource.from) + if (match) dirs.add(match[1]!) + } + } + return [...dirs].sort() +} + +describe('ToDesktop resource packaging', () => { + // A target list replaces, rather than merges with, the platform list, and + // ToDesktop decides server-side which architectures a platform builds. An + // architecture we did not enumerate falls back to `platformOverrides`, and + // without one it lands on the top-level list — `./lib` alone, so the + // package ships with no apparmor-profile and no bootstrap-python. + it('keeps a platform-level fallback for every platform with per-target overrides', () => { + const config = readToDesktopConfig() + const platforms = Object.keys(config.targetOverrides ?? {}) + expect(platforms.length).toBeGreaterThan(0) + + for (const platform of platforms) { + const fallback = config.platformOverrides?.[platform] + expect(fallback, `platformOverrides.${platform} is missing`).toBeDefined() + for (const [arch, target] of Object.entries(config.targetOverrides![platform]!)) { + expect( + destinations(fallback!.extraResources), + `platformOverrides.${platform} must cover every destination of targetOverrides.${platform}.${arch}` + ).toEqual(destinations(target.extraResources)) + } + } + }) + + // ToDesktop permits architecture-specific sources, but every target of a + // platform must declare the same destinations and source basenames in the + // same order. Checked for every platform, so Windows is covered too. + it('gives every architecture of a platform the same resource shape', () => { + const config = readToDesktopConfig() + + for (const [platform, archs] of Object.entries(config.targetOverrides ?? {})) { + const entries = Object.entries(archs) + expect( + entries.length, + `targetOverrides.${platform} declares no architectures` + ).toBeGreaterThan(1) + const [firstArch, firstTarget] = entries[0]! + for (const [arch, target] of entries.slice(1)) { + expect( + resourceShape(target.extraResources), + `targetOverrides.${platform}.${arch} must match .${firstArch}` + ).toEqual(resourceShape(firstTarget.extraResources)) + } + for (const [arch, target] of entries) { + for (const resource of target.extraResources ?? []) { + expect( + path.posix.basename(resource.from), + `targetOverrides.${platform}.${arch} source basename must equal its destination` + ).toBe(resource.to) + } + } + } + }) + + // Staging directories are named after the target they serve, so a target + // reaching into another one is how an architecture ends up shipping a + // foreign interpreter — the ARM64 bug this branch fixes for Linux, and the + // Windows one #1484 fixed. The shape rule above cannot see it: every + // source basename is `bootstrap-python` either way. + it('sources each target from the staging directory named after it', () => { + const config = readToDesktopConfig() + let checked = 0 + + for (const [platform, archs] of Object.entries(config.targetOverrides ?? {})) { + for (const [arch, target] of Object.entries(archs)) { + for (const resource of target.extraResources ?? []) { + const match = /^\.\/todesktop-targets\/([^/]+)\//.exec(resource.from) + if (!match) continue + checked++ + expect(match[1], `targetOverrides.${platform}.${arch} stages from ${match[1]}`).toBe( + `${platform}-${arch}` + ) + } + } + } + + expect(checked, 'no targets source from todesktop-targets/').toBeGreaterThan(0) + }) + + // The release workflow stages these directories before `todesktop build`; + // a target declared here but never staged uploads an empty resource. + it('stages every declared target directory in the release workflow', () => { + const config = readToDesktopConfig() + const workflow = parse( + fs.readFileSync(path.join(repoRoot, '.github', 'workflows', 'build-release.yml'), 'utf-8') + ) as { jobs: Record } + + const steps = Object.values(workflow.jobs).flatMap((job) => job.steps ?? []) + const staging = steps.find((step) => step.name?.includes('Stage bootstrap-python')) + expect(staging?.run, 'no bootstrap-python staging step in build-release.yml').toBeTruthy() + + const targets = stagedTargetDirs(config) + expect(targets.length).toBeGreaterThan(0) + for (const target of targets) { + expect(staging!.run, `staging step never mentions ${target}`).toContain(target) + } + }) +}) + +describe('electron-builder resource packaging', () => { + // Local Linux builds must resolve the bootstrap for the architecture being + // packaged; a hardcoded directory puts an x64 interpreter in an ARM64 app. + it('resolves the Linux bootstrap from the target architecture', () => { + const config = parse(fs.readFileSync(path.join(repoRoot, 'electron-builder.yml'), 'utf-8')) as { + linux?: { extraResources?: ExtraResource[] } + } + const bootstrap = config.linux?.extraResources?.find( + (resource) => resource.to === 'bootstrap-python' + ) + + expect(bootstrap?.from).toContain('${arch}') + expect(bootstrap?.from).not.toContain('linux-x64') + }) +})