Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 46 additions & 6 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,29 +29,68 @@ concurrency:

jobs:
build-windows:
name: Windows installer
name: Windows ${{ matrix.arch }} installer
runs-on: windows-latest
strategy:
fail-fast: false
matrix:
arch: [x64, arm64]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dispatch input arch: is now misleading. This arch: [x64, arm64] matrix here is the right shape. But the dispatch input arch: (around line 9, not in this diff) is consumed only by the macOS matrix expression at line 103. The Windows matrix is hardcoded to arch: [x64, arm64] regardless. So:

  • A maintainer running this workflow manually with arch=arm64 will only build the macOS arm64 leg, leaving Windows builds untouched.
  • The label "macOS architecture to build" describes the actual behavior, but a user skimming the input would reasonably expect it to gate both.

Either rename to macos_arch: and document the scope, or wire this input into the Windows matrix too (matching the macOS shape). Recommend the former — Windows builds are better off always running both arches on tag pushes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✓ Correct matrix shape — [x64, arm64] mirrors the macOS matrix and the dispatch input. windows-latest (x64) cross-compiles for arm64; this avoids needing the windows-11-arm preview runner which would require MSVC ARM64 pre-installed.

One nit: the concurrency: block above this should include arch in the group key (e.g. ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}-${{ matrix.arch }}) so two arm64 / x64 builds don't cancel each other when a maintainer pushes a commit mid-build.

steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Setup Node.js
uses: ./.github/actions/setup

- name: Ensure MSVC ARM64 build tools

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two failure modes here that need guarding.

  1. If vswhere -latest returns no installation (the Build Tools are sometimes registered differently on fresh runner images), $installPath is empty and vs_installer modify --installPath "" errors out confusingly. Validate $installPath before invoking.

  2. If the install fails (network blip, transient vswhere state, locked file), the build proceeds with the default x64 toolchain and silently produces x64 binaries under the arm64 artifact path. CI will be green; the arm64 installer will be x64. Wrap the install in a post-check that fails the step if Microsoft.VisualStudio.Component.VC.Tools.ARM64 is still missing. Use throw instead of Write-Host on failure.

if: matrix.arch == 'arm64'
shell: pwsh
run: |
$vswhere = "C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe"
$installPath = & $vswhere -latest -products * -property installationPath
$hasArm64 = & $vswhere -latest -products * `
-requires Microsoft.VisualStudio.Component.VC.Tools.ARM64 `
-property installationPath
if (-not $hasArm64) {
Write-Host "Installing MSVC ARM64 build tools component..."
$installer = "C:\Program Files (x86)\Microsoft Visual Studio\Installer\vs_installer.exe"
& $installer modify --installPath "$installPath" `
--add Microsoft.VisualStudio.Component.VC.Tools.ARM64 `
--quiet --norestart --wait
} else {
Write-Host "MSVC ARM64 build tools already present."
}

- name: Ensure win32-arm64 sharp binary

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead-code step — sharp is not a dependency of this repo. Verified: zero references in package-lock.json, not present in node_modules/. The comment claims "sharp (transitive via @xenova/transformers)" — false; @xenova/transformers@^2.17.2 does not pull sharp. npm rebuild sharp with no installed package is a no-op (warning, exit 0). Same issue pre-exists in the macOS step at line ~116.

Either delete both Ensure sharp prebuilt steps, or correct the comment and add the missing dependency. Don't ship a step whose rationale is wrong.

# sharp (transitive via @xenova/transformers) only fetches a host-arch
# prebuilt at install time, so on the x64 runner the packaged arm64 app
# would otherwise ship an x64 sharp.node. Best-effort fetch of the
# win32-arm64 prebuilt; non-fatal so it never blocks the build. Verify
# the first arm64 release loads sharp if auto-captions use it.
if: matrix.arch == 'arm64'
continue-on-error: true
shell: bash
run: npm_config_platform=win32 npm_config_arch=arm64 npm rebuild sharp

- name: Cache caption assets
uses: actions/cache@v4
with:
path: caption-assets
key: caption-assets-${{ runner.os }}-${{ hashFiles('scripts/fetch-caption-model.mjs') }}

- name: Build Windows app
- name: Build Windows app (x64)
if: matrix.arch == 'x64'
run: npm run build:win -- --publish never

- name: Build Windows app (arm64)
if: matrix.arch == 'arm64'
run: npm run build:win:arm64 -- --publish never

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

npm run build:win:arm64 -- --publish never is the right invocation — cross-compiles arm64 from x64 runner. The --publish never correctly defers to the publish-release job.


- name: Upload Windows installer
uses: actions/upload-artifact@v4
with:
name: openscreen-windows
path: release/**/Openscreen.Setup.*.exe
name: openscreen-windows-${{ matrix.arch }}
path: release/**/Openscreen.Setup.*-${{ matrix.arch }}.exe
if-no-files-found: error
retention-days: 30

Expand Down Expand Up @@ -330,10 +369,11 @@ jobs:
echo "prerelease_flag=$PRERELEASE_FLAG" >> "$GITHUB_OUTPUT"
echo "notes_start_tag=$NOTES_START_TAG" >> "$GITHUB_OUTPUT"

- name: Download Windows installer
- name: Download Windows installers
uses: actions/download-artifact@v4
with:
name: openscreen-windows
pattern: openscreen-windows-*

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

download-artifact@v4 pattern: openscreen-windows-* with merge-multiple: true correctly pulls both openscreen-windows-x64 and openscreen-windows-arm64 into one folder for the publish-release job. This is the right v4 syntax for multi-artifact downloads.

merge-multiple: true
path: artifacts/windows

- name: Download macOS arm64 DMG
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ After running this command, proceed to **System Settings > Privacy & Security**

### Windows

Download the `.exe` installer directly from the [Releases page](https://github.com/EtienneLescot/openscreen/releases).
Download the `.exe` installer directly from the [Releases page](https://github.com/EtienneLescot/openscreen/releases). Windows builds are provided for both x64 (`Openscreen.Setup.<version>-x64.exe`) and ARM64 (`Openscreen.Setup.<version>-arm64.exe`).

### Linux

Expand Down
2 changes: 1 addition & 1 deletion electron-builder.json5
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
"nsis"
],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✓ Correct: the ${arch} suffix in the artifactName (Openscreen.Setup.${version}-${arch}.${ext}) gives each Windows arch its own distinct artifact name, so the matrix produces openscreen-windows-x64 and openscreen-windows-arm64 separately. Matches the upload name: at line 87 of build.yml.

Adjacent issue (not changed in this PR): the extraResources filter at line 98 (unchanged in this PR, in a different hunk) still uses ["win32-*/*"], which pulls both win32-x64/wgc-capture.exe and win32-arm64/wgc-capture.exe into every Windows installer — the x64 installer ships a useless arm64 helper and vice versa (doubles WGC-helper bloat). Same waste already exists on macOS at line 66 with ["darwin-*/*"]. Worth fixing in this same PR since you're touching the pattern. Switch to a per-arch filter — either split extraResources per arch in the electron-builder config, or build with an arch-specific override (--config.win.extraResources.filter=...).

"icon": "icons/icons/win/icon.ico",
"artifactName": "${productName}.Setup.${version}.${ext}",
"artifactName": "${productName}.Setup.${version}-${arch}.${ext}",
"extraResources": [
{
"from": "electron/native/bin",
Expand Down
6 changes: 5 additions & 1 deletion electron/native/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,14 @@ Windows native recording is resolved from one of these locations:
Build the Windows helper with:

```powershell
# Builds for the host architecture by default (x64 on x64 hosts, arm64 on arm64 hosts).
npm run build:native:win

# Explicit target architecture (native on a matching host, or cross-compiled otherwise):
node scripts/build-windows-wgc-helper.mjs --arch arm64
```

The build writes the CMake output to `electron/native/wgc-capture/build/wgc-capture.exe` and copies the redistributable binary to `electron/native/bin/win32-x64/wgc-capture.exe`.
The target architecture is resolved from `--arch` (or the `OPENSCREEN_WIN_HELPER_ARCH` env var), falling back to the host arch. Cross-compiling requires the matching MSVC component — "VS C++ ARM64/ARM64EC build tools" for `arm64`. The build writes the CMake output to `electron/native/wgc-capture/build/wgc-capture.exe` and copies the redistributable binary to `electron/native/bin/win32-<arch>/wgc-capture.exe` (e.g. `win32-arm64`).

The helper contract is process-based: the app starts the process with one JSON argument and sends commands on stdin. `stop\n` finalizes the recording. During migration the helper prints both newline-delimited JSON events and the legacy text messages `Recording started` / `Recording stopped. Output path: <path>`.

Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
"build:mac": "npm run build:native:mac && tsc && vite build && electron-builder --mac",
"build:native:win": "node scripts/build-windows-wgc-helper.mjs",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To prevent host-architecture compilation conflicts on Windows ARM64 developer machines (where npm run build:win might compile the ARM64 helper instead of the x64 one required by the x64 packaged app), we should explicitly pass the --arch x64 flag here:

"build:native:win": "node scripts/build-windows-wgc-helper.mjs --arch x64"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

build:native:win doesn't force --arch x64. This is the existing reviewer-thread issue from EtienneLescot. As-is, an arm64 dev running npm run build:win will compile an arm64 helper while intending to ship x64. Recommend:

"build:native:win": "node scripts/build-windows-wgc-helper.mjs --arch x64"

so the default script explicitly targets x64. The new build:native:win:arm64 script already does the right thing for the arm64 case.

If you reject this change, push back in the thread with reasoning before requesting re-review.

"build:win": "npm run build:native:win && tsc && vite build && electron-builder --win --config.npmRebuild=false",
"build:native:win:arm64": "node scripts/build-windows-wgc-helper.mjs --arch arm64",
"build:win:arm64": "npm run build:native:win:arm64 && tsc && vite build && electron-builder --win --arm64 --config.npmRebuild=false",
"build:linux": "tsc && vite build && electron-builder --linux AppImage deb pacman --config.npmRebuild=false",
"test": "vitest --run",
"test:watch": "vitest",
Expand Down
49 changes: 45 additions & 4 deletions scripts/build-windows-wgc-helper.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,37 @@ import os from "node:os";
import path from "node:path";
import { fileURLToPath } from "node:url";

import {
resolveTargetArch,
resolveVcvarsArch,
winBinDirName,
} from "./windows-helper-arch.mjs";

function parseArchFlag(argv) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parseArchFlag has a silent fallback hole. eq.slice("--arch=".length) returns "" for --arch= (empty value). parseArchFlag returns "", resolveTargetArch calls normalizeArch("") which returns undefined, the CLI path is skipped, and the script falls back to host arch. The user typed an explicit flag and got the host anyway, with no warning.

Two fixes:

  1. Guard against empty values: if (!value) throw new Error(...) in parseArchFlag.
  2. Move parseArchFlag into scripts/windows-helper-arch.mjs so it's exported and testable. Currently it's neither exported nor tested, even though the rest of the arch-resolution logic is.

const eq = argv.find((a) => a.startsWith("--arch="));
if (eq) {
return eq.slice("--arch=".length);
}
const idx = argv.indexOf("--arch");
if (idx !== -1) {
return argv[idx + 1];
}
return undefined;
}

const TARGET_ARCH = resolveTargetArch({
cliArch: parseArchFlag(process.argv.slice(2)),
envArch: process.env.OPENSCREEN_WIN_HELPER_ARCH,
hostArch: process.arch,
});
const VCVARS_ARCH = resolveVcvarsArch(process.arch, TARGET_ARCH);

const __dirname = path.dirname(fileURLToPath(import.meta.url));
const ROOT = path.join(__dirname, "..");
const SOURCE_DIR = path.join(ROOT, "electron", "native", "wgc-capture");
const BUILD_DIR = path.join(SOURCE_DIR, "build");
const COMPAT_LIB_DIR = path.join(BUILD_DIR, "compat-libs");
const BIN_DIR = path.join(ROOT, "electron", "native", "bin", "win32-x64");
const BIN_DIR = path.join(ROOT, "electron", "native", "bin", winBinDirName(TARGET_ARCH));
const CMAKE = process.env.CMAKE_EXE ?? "cmake";

function findVcVarsAll() {
Expand Down Expand Up @@ -75,7 +100,7 @@ function findWindowsSdkUmLibDir() {
return fs
.readdirSync(sdkLibRoot, { withFileTypes: true })
.filter((entry) => entry.isDirectory())
.map((entry) => path.join(sdkLibRoot, entry.name, "um", "x64"))
.map((entry) => path.join(sdkLibRoot, entry.name, "um", TARGET_ARCH))
.filter((candidate) => fs.existsSync(path.join(candidate, "kernel32.lib")))
.sort()
.at(-1);
Expand Down Expand Up @@ -115,10 +140,10 @@ async function runInVsEnv(command) {
cmdPath,
[
"@echo off",
`call "${vcvarsAll}" x64`,
`call "${vcvarsAll}" ${VCVARS_ARCH}`,
"if errorlevel 1 exit /b %errorlevel%",
`if not exist "${COMPAT_LIB_DIR}" mkdir "${COMPAT_LIB_DIR}"`,
`for %%L in (gdi32.lib gdiplus.lib winspool.lib shell32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib) do if not exist "%WindowsSdkDir%Lib\\%WindowsSDKLibVersion%um\\x64\\%%L" copy /Y "%WindowsSdkDir%Lib\\%WindowsSDKLibVersion%um\\x64\\kernel32.Lib" "${COMPAT_LIB_DIR}\\%%L" >nul`,
`for %%L in (gdi32.lib gdiplus.lib winspool.lib shell32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib) do if not exist "%WindowsSdkDir%Lib\\%WindowsSDKLibVersion%um\\${TARGET_ARCH}\\%%L" copy /Y "%WindowsSdkDir%Lib\\%WindowsSDKLibVersion%um\\${TARGET_ARCH}\\kernel32.Lib" "${COMPAT_LIB_DIR}\\%%L" >nul`,
"if errorlevel 1 exit /b %errorlevel%",
`set "LIB=${sdkUmLibDir ? `${sdkUmLibDir};` : ""}%LIB%;${COMPAT_LIB_DIR}"`,
command,
Expand All @@ -138,7 +163,23 @@ if (process.platform !== "win32") {
process.exit(0);
}

console.log(`Building Windows WGC helper for target arch: ${TARGET_ARCH} (vcvars: ${VCVARS_ARCH})`);

// CMake caches the detected compiler in the build directory. Reusing a build
// dir that was configured for a different target arch silently produces
// wrong-arch binaries (the cached compiler wins over the new vcvars env). Wipe
// the build dir when the target arch changes; same-arch reruns stay incremental.
const ARCH_STAMP = path.join(BUILD_DIR, ".target-arch");
if (fs.existsSync(BUILD_DIR)) {
const previousArch = fs.existsSync(ARCH_STAMP)
? fs.readFileSync(ARCH_STAMP, "utf8").trim()
: "";
if (previousArch !== TARGET_ARCH) {
fs.rmSync(BUILD_DIR, { recursive: true, force: true });
}
}
fs.mkdirSync(BUILD_DIR, { recursive: true });
fs.writeFileSync(ARCH_STAMP, TARGET_ARCH);

await runInVsEnv(
`"${CMAKE}" -S "${SOURCE_DIR}" -B "${BUILD_DIR}" -G Ninja -DCMAKE_BUILD_TYPE=Release`,
Expand Down
62 changes: 62 additions & 0 deletions scripts/windows-helper-arch.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Pure helpers that map a target CPU architecture to the parameters the Windows
// native-helper build needs. Kept side-effect free so it is unit-testable and
// importable from both the build script and Vitest.

export const SUPPORTED_ARCHES = ["x64", "arm64"];

const ALIASES = new Map([
["x64", "x64"],
["amd64", "x64"],
["x86_64", "x64"],
["arm64", "arm64"],
["aarch64", "arm64"],
]);

export function normalizeArch(value) {
if (value === undefined || value === null || value === "") {
return undefined;
}
return ALIASES.get(String(value).toLowerCase());
}

export function resolveTargetArch({ cliArch, envArch, hostArch } = {}) {
for (const [label, raw] of [
["--arch", cliArch],
["OPENSCREEN_WIN_HELPER_ARCH", envArch],
]) {
if (raw !== undefined && raw !== null && raw !== "") {
const normalized = normalizeArch(raw);
if (!normalized) {
throw new Error(
`Invalid ${label} value "${raw}". Expected one of: ${SUPPORTED_ARCHES.join(", ")}.`,
);
}
return normalized;
}
}

const host = normalizeArch(hostArch);
if (!host) {
throw new Error(
`Unsupported host architecture "${hostArch}". Pass --arch with one of: ${SUPPORTED_ARCHES.join(", ")}.`,
);
}
return host;
}

export function resolveVcvarsArch(hostArch, targetArch) {
const host = normalizeArch(hostArch);
const target = normalizeArch(targetArch);
if (!host || !target) {
throw new Error(`Unsupported architecture pair host="${hostArch}" target="${targetArch}".`);
}
return host === target ? target : `${host}_${target}`;
}

export function winBinDirName(targetArch) {
const target = normalizeArch(targetArch);
if (!target) {
throw new Error(`Unsupported target architecture "${targetArch}".`);
}
return `win32-${target}`;
}
71 changes: 71 additions & 0 deletions scripts/windows-helper-arch.test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { describe, expect, it } from "vitest";
import {
SUPPORTED_ARCHES,
normalizeArch,
resolveTargetArch,
resolveVcvarsArch,
winBinDirName,
} from "./windows-helper-arch.mjs";

describe("normalizeArch", () => {
it("maps known aliases to canonical arches", () => {
expect(normalizeArch("x64")).toBe("x64");
expect(normalizeArch("amd64")).toBe("x64");
expect(normalizeArch("x86_64")).toBe("x64");
expect(normalizeArch("arm64")).toBe("arm64");
expect(normalizeArch("aarch64")).toBe("arm64");
expect(normalizeArch("ARM64")).toBe("arm64");
});

it("returns undefined for empty or unknown values", () => {
expect(normalizeArch(undefined)).toBeUndefined();
expect(normalizeArch("")).toBeUndefined();
expect(normalizeArch("mips")).toBeUndefined();
});
});

describe("resolveTargetArch", () => {
it("prefers the CLI arch over env and host", () => {
expect(resolveTargetArch({ cliArch: "arm64", envArch: "x64", hostArch: "x64" })).toBe("arm64");
});

it("falls back to env when no CLI arch", () => {
expect(resolveTargetArch({ envArch: "arm64", hostArch: "x64" })).toBe("arm64");
});

it("defaults to the host arch when nothing explicit is given", () => {
expect(resolveTargetArch({ hostArch: "arm64" })).toBe("arm64");
expect(resolveTargetArch({ hostArch: "x64" })).toBe("x64");
});

it("throws on an invalid explicit arch", () => {
expect(() => resolveTargetArch({ cliArch: "mips", hostArch: "x64" })).toThrow(/Invalid/);
});

it("throws on an unsupported host with no explicit arch", () => {
expect(() => resolveTargetArch({ hostArch: "ppc64" })).toThrow(/host/i);
});
});

describe("resolveVcvarsArch", () => {
it("returns native tokens when host equals target", () => {
expect(resolveVcvarsArch("x64", "x64")).toBe("x64");
expect(resolveVcvarsArch("arm64", "arm64")).toBe("arm64");
});

it("returns cross tokens as <host>_<target>", () => {
expect(resolveVcvarsArch("x64", "arm64")).toBe("x64_arm64");
expect(resolveVcvarsArch("arm64", "x64")).toBe("arm64_x64");
});
});

describe("winBinDirName", () => {
it("builds the packaged bin folder name", () => {
expect(winBinDirName("x64")).toBe("win32-x64");
expect(winBinDirName("arm64")).toBe("win32-arm64");
});
});

it("exposes the supported arch list", () => {
expect(SUPPORTED_ARCHES).toEqual(["x64", "arm64"]);
});
2 changes: 1 addition & 1 deletion vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export default defineConfig({
test: {
globals: true,
environment: "jsdom",
include: ["{src,electron,.github}/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}"],
include: ["{src,electron,.github,scripts}/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}"],
exclude: ["src/**/*.browser.test.{ts,tsx}"],
},
resolve: {
Expand Down
Loading