diff --git a/.gitattributes b/.gitattributes index 154f7c37..d9dbbd32 100644 --- a/.gitattributes +++ b/.gitattributes @@ -7,3 +7,6 @@ *.rs text eol=lf *.ts text eol=lf *.tsx text eol=lf +# Same drift, caught on the native helper's build file: an edit from Windows +# rewrote all 67 lines as CRLF and buried a 22-line change in a 156-line diff. +CMakeLists.txt text eol=lf diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 59ee3a15..9c3a6eba 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -138,6 +138,48 @@ jobs: } Write-Output "$($expected.Count) tile assets present in $($appx.Name), byte-identical to build/appx/" + # Twice running, a release shipped a dependency that could not be resolved on + # the target machine, and both times someone outside the project found it: + # 1.9.0's compositor addon was reached through PATH, which MSIX ignores, and + # 1.9.1's capture helper needed the Visual C++ Redistributable, which is not + # part of Windows. Each fix arrived with a guard aimed at the failure already + # understood, and neither guard would have caught the other. + # + # This step is the one that generalises: it registers the package and asks the + # Windows loader to resolve every shipped binary for real. It deliberately does + # not record anything — a runner has no useful GPU or desktop session, and a + # flaky gate gets switched off. The loader is what broke both times. + # + # `powershell`, not `pwsh`: the Appx module is not loaded natively in + # PowerShell 7 and needs -UseWindowsPowerShell to work at all. + - name: Verify native binaries load under package identity + shell: powershell + run: | + $ErrorActionPreference = "Stop" + + # Loose registration of an unsigned package needs Developer Mode. The runner + # is discarded after the job, so enabling it here costs nothing; the script + # itself refuses to touch this, because on a real machine it is the owner's + # setting to make. + $key = "HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\AppModelUnlock" + New-Item -Path $key -Force | Out-Null + Set-ItemProperty -Path $key -Name AllowDevelopmentWithoutDevLicense -Value 1 -Type DWord + + $appx = Get-ChildItem release -Recurse -Filter *.appx | Select-Object -First 1 + if (-not $appx) { throw "no .appx found under release/" } + + # Explicit exit rather than letting the error surface on its own: a + # terminating error does not set $LASTEXITCODE, and the runner's epilogue + # only inspects that. Trusting it would let a failed verification report a + # green step, which is the exact shape of bug this job exists to stop. + try { + & "$env:GITHUB_WORKSPACE\scripts\verify-appx-native.ps1" -Appx $appx.FullName + } + catch { + Write-Output "::error::$($_.Exception.Message)" + exit 1 + } + - name: Upload Windows Store package uses: actions/upload-artifact@v7 with: diff --git a/crates/.cargo/config.toml b/crates/.cargo/config.toml index ca5ccb6b..29197773 100644 --- a/crates/.cargo/config.toml +++ b/crates/.cargo/config.toml @@ -12,6 +12,23 @@ FFMPEG_DIR = { value = "thirdparty/ffmpeg-n8.1.2-win64-lgpl-shared", relative = true } LIBCLANG_PATH = "C:\\Program Files\\LLVM\\bin" +# CRT statique. Par défaut, un cdylib MSVC importe VCRUNTIME140.dll, qui ne fait PAS +# partie de Windows : elle vient du Redistribuable Visual C++. Toute machine ayant +# déjà compilé ce dépôt — ou installé à peu près n'importe quelle autre application +# de bureau — l'a dans System32, donc la dépendance est invisible en local comme en +# CI. Sur une image Windows propre, `require()` de compositor_view.node échoue et +# l'éditeur s'ouvre sans aperçu pendant que le son continue : le symptôme EXACT que +# la colocation avec les DLL ffmpeg (build-windows-compositor-addon.mjs) était +# censée corriger. Elle a bien corrigé sa cause à elle — MSIX qui ignore PATH — mais +# celle-ci restait, et aucune machine de dev ne peut la faire apparaître. +# +# Rien de partagé avec l'hôte ne traverse la frontière du CRT : napi-rs ne fait +# passer que des napi_value opaques, et les Buffer rendus à Node portent un +# finalizer qui libère côté addon ce que l'addon a alloué. Le CRT statique est donc +# sans effet de bord ici. scripts/before-pack.cjs échoue si l'import revient. +[target.x86_64-pc-windows-msvc] +rustflags = ["-C", "target-feature=+crt-static"] + # macOS : BtbN ne publie pas de build macOS (cf. scripts/fetch-ffmpeg.mjs), donc on # laisse `crates/compositor/build.rs` chercher MAC_FFMPEG_DIR (env var explicite posée # par la CI macOS ou par le dev local) ou un répertoire attendu sous `thirdparty/`. diff --git a/electron/native/wgc-capture/CMakeLists.txt b/electron/native/wgc-capture/CMakeLists.txt index 32c5d6ef..d1f55066 100644 --- a/electron/native/wgc-capture/CMakeLists.txt +++ b/electron/native/wgc-capture/CMakeLists.txt @@ -9,6 +9,28 @@ set(CMAKE_CXX_STANDARD_LIBRARIES project(openscreen-wgc-capture LANGUAGES CXX) +# Static CRT (/MT), not CMake's default /MD. With /MD both helpers import +# VCRUNTIME140.dll, VCRUNTIME140_1.dll and MSVCP140.dll, which are NOT part of +# Windows — they come from the Visual C++ Redistributable. Every machine that has +# ever built this repo, or installed almost any other desktop app, already has +# them in System32, so the dependency is invisible in local testing and in CI. +# +# On a clean Windows image it is fatal: the loader kills the process before main() +# and the parent only sees an exit code. Store certification runs on exactly such +# an image and rejected 1.9.1 for it, reporting +# +# Error: Native Windows capture exited before recording started (code=3221225781) +# +# 3221225781 is 0xC0000135, STATUS_DLL_NOT_FOUND. Recording was impossible on the +# test device while the app itself started fine, because Electron already links +# the CRT statically. +# +# These are standalone processes that share no CRT state with anything, so /MT is +# free: it costs ~100 KB each and removes the dependency instead of obliging us to +# redistribute Microsoft's DLLs beside them. scripts/before-pack.cjs fails the +# build if it ever comes back. +set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>") + set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_CXX_EXTENSIONS OFF) diff --git a/scripts/before-pack.cjs b/scripts/before-pack.cjs index 26421796..23cc8fd8 100644 --- a/scripts/before-pack.cjs +++ b/scripts/before-pack.cjs @@ -242,14 +242,151 @@ const WIN_REQUIRED = [ })), ]; +/** + * DLLs that come from the Visual C++ Redistributable rather than from Windows itself. + * + * The `api-ms-win-crt-*` api-sets are deliberately absent: that is the UCRT, which IS + * part of Windows 10 and later. These are not, and no machine is obliged to have them. + */ +const VC_REDIST_DLL = /^(msvcp|vcruntime|concrt)\d+/i; + +/** + * The DLL names a PE binary imports — just enough of the format to walk the import + * directory, so this needs no dumpbin and therefore no Visual Studio on the runner. + */ +function importedDlls(file) { + const b = fs.readFileSync(file); + // Every read below is bounds-checked through this, so a truncated or non-PE file + // arrives at the message this function means to give rather than at a RangeError + // from readUInt32LE. The diagnostic is the whole product here: the caller's job is + // to explain an absence, and "offset is out of bounds" explains nothing. + const notPe = () => new Error(`${file} is not a PE binary, or is truncated`); + const u32 = (at) => { + if (at < 0 || at + 4 > b.length) throw notPe(); + return b.readUInt32LE(at); + }; + const u16 = (at) => { + if (at < 0 || at + 2 > b.length) throw notPe(); + return b.readUInt16LE(at); + }; + + if (u16(0) !== 0x5a4d) throw notPe(); // "MZ" + const pe = u32(0x3c); + if (u32(pe) !== 0x00004550) throw notPe(); // "PE\0\0" + const opt = pe + 24; + // The optional header's fixed part is 96 bytes for PE32 and 112 for PE32+ (five + // fields widen to 8 bytes); the data directories follow it. + const dirs = opt + (u16(opt) === 0x20b ? 112 : 96); + // NumberOfRvaAndSizes is the last field before the directories, so it sits four + // bytes back whichever the format. Without it, a binary declaring fewer entries + // than we index would have unrelated header bytes read as an RVA. + const dirCount = u32(dirs - 4); + + const sections = []; + for (let i = 0; i < u16(pe + 6); i++) { + const s = opt + u16(pe + 20) + i * 40; + sections.push({ va: u32(s + 12), size: u32(s + 16), ptr: u32(s + 20) }); + } + const fileOffset = (rva) => { + const s = sections.find((s) => rva >= s.va && rva < s.va + s.size); + if (!s) throw new Error(`${file}: RVA 0x${rva.toString(16)} is in no section`); + return s.ptr + (rva - s.va); + }; + + const names = []; + // Both directories are arrays of fixed-size descriptors ending in an all-zero one, + // and both hold the DLL name as an RVA at a fixed offset. Reading only the first + // would miss a delay-loaded dependency entirely — the loader resolves those on + // first call rather than at load time, so the failure would arrive later and + // nowhere near the cause, which is worse than the one this guard was written for. + const walk = (index, stride, nameOffset) => { + if (dirCount <= index) return; + const rva = u32(dirs + index * 8); + if (!rva) return; + for (let entry = fileOffset(rva); ; entry += stride) { + const nameRva = u32(entry + nameOffset); + if (!nameRva) return; + const at = fileOffset(nameRva); + names.push(b.subarray(at, b.indexOf(0, at)).toString("latin1")); + } + }; + walk(1, 20, 12); // IMAGE_IMPORT_DESCRIPTOR.Name + walk(13, 32, 4); // ImgDelayDescr.rvaDLLName + return names; +} + +/** + * Nothing we ship may depend on the Visual C++ Redistributable. + * + * This is the one failure this whole hook could not see. Every machine that builds this + * repo, and most machines that have ever installed a desktop app, carry those DLLs in + * System32 — so a binary that needs them works in local testing, in CI, and in every + * package format, while being unloadable on a clean Windows image. There is no way to + * reproduce it here; only the import table tells the truth. + * + * Store certification rejected 1.9.1 for exactly this: the WGC helper was built against + * the dynamic CRT and died in the loader before main(), so the app reported + * `Native Windows capture exited before recording started (code=3221225781)` + * — 0xC0000135, STATUS_DLL_NOT_FOUND — and recording was impossible on the test device. + * `compositor_view.node` had the same defect and would have failed certification a + * second time, on the preview, right after the helper was fixed. + * + * The fix is per-toolchain, hence the two-part message: /MT for the CMake helpers + * (electron/native/wgc-capture/CMakeLists.txt), `+crt-static` for the Rust addon + * (crates/.cargo/config.toml). + */ +function checkWinNoRedistDependency(dir) { + const scanned = fs + .readdirSync(dir) + .filter((name) => /\.(exe|dll|node)$/i.test(name)) + .map((name) => ({ name, imports: importedDlls(path.join(dir, name)) })); + + // A guard that silently stops looking is worse than no guard: it reports "clean" for + // the rest of the project's life. Every native binary imports something — kernel32 at + // the very least — so an empty result means the parser broke, not that the file is + // self-contained. Asserted here against the real payload rather than a synthetic PE + // fixture, which would only ever prove this parser agrees with itself. + const unread = scanned.filter((entry) => entry.imports.length === 0); + if (unread.length > 0) { + throw new Error( + `Refusing to package: read no imports at all from ${unread.map((e) => e.name).join(", ")}.\n\n` + + "Every native binary imports at least kernel32, so this is a bug in importedDlls()\n" + + "(scripts/before-pack.cjs), not a self-contained binary. Fix the parser — leaving it\n" + + "is how the Visual C++ Redistributable dependency gets back into a shipped build.", + ); + } + + const offenders = scanned + .map((entry) => ({ name: entry.name, bad: entry.imports.filter((d) => VC_REDIST_DLL.test(d)) })) + .filter((entry) => entry.bad.length > 0); + if (offenders.length === 0) { + return; + } + + throw new Error( + "Refusing to package binaries that need the Visual C++ Redistributable.\n\n" + + ` looked in: ${path.relative(ROOT, dir)}\n\n` + + `${offenders.map((o) => ` - ${o.name} imports ${o.bad.join(", ")}`).join("\n")}\n\n` + + "Those DLLs are not part of Windows. On a clean image the loader kills the process\n" + + "before main() (0xC0000135) or fails require(), and the app can only report an exit\n" + + "code. It works on every developer machine, which is why this is checked here.\n\n" + + "Build against the static CRT instead:\n" + + " - CMake helpers: CMAKE_MSVC_RUNTIME_LIBRARY MultiThreaded (electron/native/wgc-capture)\n" + + " - Rust addon: -C target-feature=+crt-static (crates/.cargo/config.toml)\n\n" + + "For a third-party binary that cannot be rebuilt, ship the DLLs it needs beside it.", + ); +} + function checkWinNativePayload() { + const dir = path.join(ROOT, "electron", "native", "bin", "win32-x64"); checkNativePayload({ - dir: path.join(ROOT, "electron", "native", "bin", "win32-x64"), + dir, required: WIN_REQUIRED, osLabel: "Windows", bundleNoun: "the installer", emptyDirFix: `${FIX}\n\nThe STT helper and the capture helper are separate builds — see\ntechnical-documentation/engineering/build-and-packaging.md.`, }); + checkWinNoRedistDependency(dir); } function checkMacNativePayload(context) { diff --git a/scripts/verify-appx-native.ps1 b/scripts/verify-appx-native.ps1 new file mode 100644 index 00000000..f82b4072 --- /dev/null +++ b/scripts/verify-appx-native.ps1 @@ -0,0 +1,272 @@ +<# +.SYNOPSIS +Proves every native binary in a built .appx can actually load, from inside a +registered MSIX package. + +.DESCRIPTION +Twice in a row a Windows release shipped a dependency that could not be resolved +on the target machine, and both times someone else found it: + + 1.9.0 compositor_view.node sat one directory away from its ffmpeg DLLs and was + reached via PATH. MSIX resolves dependent DLLs through the package graph + and ignores PATH, so the Store build loaded no compositor: the editor + opened with a permanently blank preview while audio kept playing. + + 1.9.1 wgc-capture.exe, cursor-sampler.exe and compositor_view.node imported + VCRUNTIME140/MSVCP140 from the Visual C++ Redistributable, which is not + part of Windows. Store certification rejected the submission: recording + died with 0xC0000135, STATUS_DLL_NOT_FOUND, before main(). + +Each fix came with a guard aimed at the failure already understood, and neither +guard would have caught the other one. That is the gap this script closes. Rather +than asserting a known-bad pattern, it registers the package and asks the Windows +loader to resolve every shipped binary for real. Whatever the next unresolvable +dependency turns out to be, this fails on it. + +It deliberately does NOT record anything. A real capture needs a GPU and a desktop +session, which a CI runner does not usefully have, and a flaky gate gets switched +off. The loader is the part that broke both times, and it can be tested with +neither. + +ASCII only, on purpose: Windows PowerShell 5.1 reads a .ps1 as ANSI unless it +carries a BOM, so a stray em dash turns into a parser error rather than a typo. + +.PARAMETER Appx +The .appx to verify. + +.PARAMETER KeepRegistered +Leave the package registered afterwards, to click through the app by hand. + +.EXAMPLE +powershell -File scripts/verify-appx-native.ps1 -Appx release/1.9.1/Openscreen.Setup.1.9.1.appx + +.NOTES +Loose registration needs Developer Mode (Settings > System > For developers). +This script will not enable it: that is a machine-wide setting and its owner +should be the one turning it on. CI sets AllowDevelopmentWithoutDevLicense itself, +on a runner that is thrown away afterwards. + +Run it with Windows PowerShell, not pwsh: the Appx module is not loaded natively +in PowerShell 7 and needs -UseWindowsPowerShell to work at all. +#> +[CmdletBinding(DefaultParameterSetName = "Verify")] +param( + [Parameter(Mandatory, ParameterSetName = "Verify")] + [string]$Appx, + + [Parameter(ParameterSetName = "Verify")] + [switch]$KeepRegistered, + + # Set when the script re-invokes itself inside the package container. Not for + # direct use: outside the container it proves nothing, because a developer + # machine resolves through PATH and System32 exactly the way the Store does not. + [Parameter(Mandatory, ParameterSetName = "InPackage")] + [switch]$InPackage, + + [Parameter(Mandatory, ParameterSetName = "InPackage")] + [string]$PackageRoot, + + [Parameter(Mandatory, ParameterSetName = "InPackage")] + [string]$ReportPath +) + +$ErrorActionPreference = "Stop" + +# 0xC0000135. Surfaces as a negative Int32 through Process.ExitCode. +$STATUS_DLL_NOT_FOUND = -1073741515 + +function Get-NativeDir { + param([string]$Root) + return (Join-Path $Root "app\resources\electron\native\bin\win32-x64") +} + +# ---------------------------------------------------------------- in-package -- + +if ($InPackage) { + # LOAD_WITH_ALTERED_SEARCH_PATH is the flag Node passes for a .node addon, so + # the module's own directory is searched for its dependencies. Using the same + # flag is what makes this a test of require() rather than of something adjacent. + Add-Type -Namespace OpenScreen -Name Loader -MemberDefinition @' +[System.Runtime.InteropServices.DllImport("kernel32.dll", SetLastError = true, CharSet = System.Runtime.InteropServices.CharSet.Unicode)] +public static extern System.IntPtr LoadLibraryExW(string path, System.IntPtr file, uint flags); +'@ + + $results = @() + $dir = Get-NativeDir -Root $PackageRoot + + # This path is hard-coded, so a change to the AppX resource layout silently moves + # the payload out from under it. Without this check Get-ChildItem throws under + # $ErrorActionPreference = "Stop", the child dies before writing its report, and + # the parent spends its full timeout to conclude only that no report arrived -- + # true, useless, and three minutes late. Report the real cause as a finding + # instead, so it travels back through the same channel as every other failure. + if (-not (Test-Path $dir)) { + @([pscustomobject]@{ + name = $dir + kind = "layout" + ok = $false + detail = "the package has no native payload at this path; the AppX resource layout changed" + }) | ConvertTo-Json -Depth 4 | Set-Content -Path $ReportPath -Encoding utf8 + return + } + + foreach ($file in Get-ChildItem -Path $dir -File | Where-Object { $_.Extension -match '^\.(dll|node)$' }) { + $handle = [OpenScreen.Loader]::LoadLibraryExW($file.FullName, [IntPtr]::Zero, 0x00000008) + $lastError = [System.Runtime.InteropServices.Marshal]::GetLastWin32Error() + $loaded = ($handle -ne [IntPtr]::Zero) + # 126 is ERROR_MOD_NOT_FOUND: a dependency of this binary is missing, which + # is the same story 0xC0000135 tells about an executable. + $detail = "loaded" + if (-not $loaded) { $detail = "LoadLibraryEx failed, GetLastError=$lastError" } + $results += [pscustomobject]@{ name = $file.Name; kind = "load"; ok = $loaded; detail = $detail } + } + + # The helpers are separate processes, so their imports resolve at CreateProcess + # time and a failure never reaches their own code. Started with no arguments on + # purpose: each one prints its usage and exits, which needs no GPU, no desktop + # session and no capture, while still proving the loader let it start. A binary + # the loader rejects produces NO output at all and exits STATUS_DLL_NOT_FOUND. + foreach ($name in @("wgc-capture.exe", "cursor-sampler.exe", "whisper-stt-server.exe")) { + $exe = Join-Path $dir $name + if (-not (Test-Path $exe)) { + $results += [pscustomobject]@{ name = $name; kind = "start"; ok = $false; detail = "not present in the package" } + continue + } + + $psi = New-Object System.Diagnostics.ProcessStartInfo + $psi.FileName = $exe + $psi.UseShellExecute = $false + $psi.RedirectStandardOutput = $true + $psi.RedirectStandardError = $true + $psi.CreateNoWindow = $true + $proc = [System.Diagnostics.Process]::Start($psi) + # Both pipes are started draining before anything blocks. Reading one to the + # end while the other fills its buffer deadlocks the pair: the child blocks + # writing to stderr, the parent blocks reading stdout, and the WaitForExit + # below never runs to break it. Today these print three words each, so the + # buffer never fills -- the hang would arrive the day a helper turns chatty, + # which is exactly when this check is earning its place. + $stdoutTask = $proc.StandardOutput.ReadToEndAsync() + $stderrTask = $proc.StandardError.ReadToEndAsync() + if (-not $proc.WaitForExit(20000)) { $proc.Kill() } + $stdout = $stdoutTask.GetAwaiter().GetResult() + $stderr = $stderrTask.GetAwaiter().GetResult() + + $exitCode = "still running" + if ($proc.HasExited) { $exitCode = $proc.ExitCode } + + $said = ($stdout + $stderr).Trim() + $firstLine = "" + if ($said.Length -gt 0) { $firstLine = @($said -split "`r?`n")[0] } + + # "Said something" is the assertion, not "exited non-zero": these all exit 1 + # when told nothing to do. Only a process that reached its own main() prints. + $ok = ($said.Length -gt 0) -and ($exitCode -ne $STATUS_DLL_NOT_FOUND) + $detail = "exit=$exitCode with NO output, killed by the loader before main()" + if ($said.Length -gt 0) { $detail = "exit=$exitCode, said: $firstLine" } + $results += [pscustomobject]@{ name = $name; kind = "start"; ok = $ok; detail = $detail } + } + + $results | ConvertTo-Json -Depth 4 | Set-Content -Path $ReportPath -Encoding utf8 + return +} + +# ------------------------------------------------------------- orchestration -- + +$appxPath = (Resolve-Path $Appx).Path +$work = Join-Path ([System.IO.Path]::GetTempPath()) ("openscreen-appx-verify-" + [System.IO.Path]::GetRandomFileName()) +$extracted = Join-Path $work "package" +$report = Join-Path $work "report.json" +$registered = $null + +try { + Write-Host "Extracting $([System.IO.Path]::GetFileName($appxPath))" + New-Item -ItemType Directory -Path $work -Force | Out-Null + # Expand-Archive insists on the extension even though an .appx is a plain zip. + $zip = Join-Path $work "package.zip" + Copy-Item $appxPath $zip + Expand-Archive -Path $zip -DestinationPath $extracted -Force + Remove-Item $zip -Force + + # A loose registration is unsigned by definition, and these two describe a + # signed layout. Leaving them makes Add-AppxPackage reject the directory. + Remove-Item (Join-Path $extracted "AppxSignature.p7x"), (Join-Path $extracted "AppxBlockMap.xml") -Force -ErrorAction SilentlyContinue + + $manifest = Join-Path $extracted "AppxManifest.xml" + if (-not (Test-Path $manifest)) { throw "no AppxManifest.xml in $appxPath, is it really an appx?" } + + Write-Host "Registering the package" + try { + Add-AppxPackage -Register $manifest -ErrorAction Stop + } + catch { + throw "Add-AppxPackage -Register failed: $($_.Exception.Message)`n`nLoose registration needs Developer Mode (Settings > System > For developers)." + } + + $pkg = Get-AppxPackage -Name "EtienneLescot.OpenScreen" + if (-not $pkg) { throw "the package registered but cannot be found by name" } + $registered = $pkg.PackageFullName + Write-Host "Registered $($pkg.PackageFullName)" + + # Invoke-CommandInDesktopPackage gives the child package identity, which is the + # entire point: outside it, PATH and System32 paper over exactly the failures + # being looked for. It returns no output, so the child reports through a file. + $childArgs = "-NoProfile -ExecutionPolicy Bypass -File `"$PSCommandPath`" -InPackage -PackageRoot `"$extracted`" -ReportPath `"$report`"" + Invoke-CommandInDesktopPackage ` + -PackageFamilyName $pkg.PackageFamilyName ` + -AppId "Openscreen" ` + -Command "powershell.exe" ` + -Args $childArgs ` + -ErrorAction Stop + + $deadline = (Get-Date).AddSeconds(180) + while (-not (Test-Path $report) -and (Get-Date) -lt $deadline) { Start-Sleep -Milliseconds 500 } + if (-not (Test-Path $report)) { throw "the in-package probe never wrote its report" } + + # `@(ConvertFrom-Json ...)` looks like it produces an array and does not: Windows + # PowerShell 5.1 writes the whole deserialized array to the pipeline as ONE + # object, so @() wraps it into a single-element array holding an array. The first + # version of this script printed "All 1 native binaries load" for seventeen of + # them, and `Where-Object { -not $_.ok }` evaluated `-not` against an array of + # booleans, which is always false. It would have reported success no matter what + # the probe found. `foreach` over the variable enumerates properly. + $results = @() + foreach ($item in (Get-Content $report -Raw | ConvertFrom-Json)) { $results += $item } + if ($results.Count -eq 0) { throw "the in-package probe found no native binaries to check" } + + # Printed before anything is asserted, so whatever the probe actually found is on + # screen even when the run ends in a throw. + foreach ($r in $results) { + $mark = "ok " + if (-not $r.ok) { $mark = "FAIL" } + Write-Host " $mark $($r.kind)`t$($r.name)`t$($r.detail)" + } + + # An explicit failure is reported ahead of the count check below, which would + # otherwise swallow it: the probe reports a missing payload directory as a single + # finding, and "only checked 1 binaries" would replace an accurate diagnosis with + # a vague one. + $failed = @($results | Where-Object { -not $_.ok }) + if ($failed.Count -gt 0) { + foreach ($r in $failed) { Write-Output "::error::$($r.name): $($r.detail)" } + throw "$($failed.Count) of $($results.Count) checks failed under package identity" + } + + # Reached only when everything passed, which is exactly when a probe that quietly + # stopped looking is indistinguishable from a healthy package. The payload is + # fourteen libraries and three helpers; an exact count would be brittle, but a + # clean report covering two files is not a pass, it is a broken probe. + if ($results.Count -lt 10) { + throw "the in-package probe only checked $($results.Count) binaries, which is too few to be the real payload" + } + + Write-Host "All $($results.Count) native binaries load under package identity." +} +finally { + if ($registered -and -not $KeepRegistered) { + Remove-AppxPackage -Package $registered -ErrorAction SilentlyContinue + } + if (-not $KeepRegistered) { + Remove-Item $work -Recurse -Force -ErrorAction SilentlyContinue + } +} diff --git a/technical-documentation/engineering/build-and-packaging.md b/technical-documentation/engineering/build-and-packaging.md index 9795be99..7ff612b8 100644 --- a/technical-documentation/engineering/build-and-packaging.md +++ b/technical-documentation/engineering/build-and-packaging.md @@ -64,6 +64,72 @@ This shipped: the 1.9.0 Store build loaded no compositor at all, so the editor o `electron/native/bin/`, local native build directories, the compositor build output, models, and caches are gitignored. Rebuilding from a source checkout therefore requires the complete platform toolchain and third-party SDKs; running the generic `npm run build` alone does not manufacture missing native artifacts. The Windows compositor's D3D11/FFmpeg prerequisites are described by the source POC in `crates/README.md`, while capture helper lookup and output conventions are documented in `electron/native/README.md`. +### Nothing Windows ships may need the Visual C++ Redistributable + +`VCRUNTIME140.dll`, `VCRUNTIME140_1.dll` and `MSVCP140.dll` are **not part of Windows**. They come from the Visual C++ Redistributable, which arrives with Visual Studio, with the Rust MSVC toolchain, and with most desktop applications — so every machine that can build this repo already has them in `System32`, and so does almost every machine anyone would test on. A binary that depends on them therefore works locally, works in CI, and works in every packaging format, while being unloadable on a clean Windows image. + +That is not a theoretical image. Store certification runs on one, and it rejected 1.9.1: + +```text +Error: Native Windows capture exited before recording started (code=3221225781) +``` + +`3221225781` is `0xC0000135`, `STATUS_DLL_NOT_FOUND`. The loader killed `wgc-capture.exe` before `main()`, so the parent only ever saw an exit code, and **screen recording was impossible on the test device** while the app itself started normally — Electron already links the CRT statically, which is why the window opened at all. + +The import tables of the shipped 1.9.1 payload: + +| Binary | Needed from the redistributable | Symptom on a clean machine | +|---|---|---| +| `wgc-capture.exe` | `VCRUNTIME140`, `VCRUNTIME140_1`, `MSVCP140` | recording fails instantly with an exit code | +| `cursor-sampler.exe` | `VCRUNTIME140`, `VCRUNTIME140_1`, `MSVCP140` | cursor capture unavailable | +| `compositor_view.node` | `VCRUNTIME140` | `require()` fails — blank preview, audio keeps playing | + +The third row matters as much as the first. It is the *same visible symptom* as the MSIX/`PATH` bug documented above, from an unrelated cause, and colocation does nothing for it. Fixing only the helper would have failed certification a second time, on the preview, and looked like a regression of a fix that was actually correct. + +The fix is to link the CRT statically, which removes the dependency instead of obliging us to redistribute Microsoft's DLLs beside our own: + +- CMake helpers — `set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>")` in `electron/native/wgc-capture/CMakeLists.txt`. These are standalone processes that share no CRT state with anything, so `/MT` costs about 100 KB each and nothing else. +- Rust addon — `-C target-feature=+crt-static` in `crates/.cargo/config.toml`. Safe for the napi cdylib: only opaque `napi_value`s cross the boundary, and Buffers handed to Node carry a finalizer that frees, in the addon, what the addon allocated. + +`scripts/before-pack.cjs` now reads the import table of every `.exe`/`.dll`/`.node` in `electron/native/bin/win32-x64/` and refuses to package if any of them imports `msvcp*`/`vcruntime*`/`concrt*`. The `api-ms-win-crt-*` api-sets are deliberately not flagged: that is the UCRT, which does ship with Windows 10 and later. + +**Local testing cannot confirm this class of fix.** This machine has the redistributable and always will, so a successful run here proves the build is not broken — it says nothing about the clean-machine behaviour. The import table is the only evidence for that half, which is why the guard reads it rather than running anything. + +### Verifying a package actually loads + +The two failures above were found by the Store, not by us, and each was fixed with a guard aimed at the failure already understood — the colocation check would never have caught the redistributable, and the import-table check would never have caught the `PATH` bug. Both are worth keeping, and neither generalises. + +`scripts/verify-appx-native.ps1` is the check that does. It registers a built `.appx` and asks the Windows loader to resolve every shipped binary from inside the package: `LoadLibraryEx` with `LOAD_WITH_ALTERED_SEARCH_PATH` for each `.dll`/`.node` — the same call Node makes for an addon — and, for each helper executable, a start with no arguments. Whatever the next unresolvable dependency turns out to be, this fails on it. + +```bash +powershell -File scripts/verify-appx-native.ps1 -Appx release/1.9.1/Openscreen.Setup.1.9.1.appx +``` + +Add `-KeepRegistered` to leave the package installed and click through the app afterwards. Loose registration needs Developer Mode; the script will not enable it for you, because that is a machine-wide setting. The `Windows Store package` job runs the same script on every build, enabling Developer Mode on the runner it is about to discard. + +Two things it deliberately does not do. It never records: a real capture needs a GPU and a desktop session that a CI runner does not usefully have, and a flaky gate gets switched off — the loader is the part that broke both times, and it can be tested without either. And it proves nothing about a machine that lacks a runtime, because every runner and every developer machine has the Visual C++ Redistributable; that half is held by the import-table check in `before-pack.cjs`. + +### Testing without the build machine's advantages + +Every failure in this section shares one shape: **the machines we test on have more installed than the machines we ship to.** A developer box carries the Visual C++ Redistributable because Visual Studio put it there; a CI runner carries a newer glibc than the distros the README claims. Nothing run on either can reveal an absence, so "it works here" is not evidence about anything, however many times it is repeated. + +Three layers address it, and the order matters: + +1. **Remove the dependency at the source.** A statically linked CRT cannot be missing; a runner pinned to the oldest supported distro cannot bind symbols the user does not have. This is the only layer that needs no testing at all, so prefer it whenever there is a choice. +2. **Prove it automatically.** Static analysis for absences that are known and enumerable (`before-pack.cjs` reads import tables and ELF symbol-version needs), and a real load for everything else (`verify-appx-native.ps1` under package identity). +3. **A pristine machine before a Store submission.** The only layer that catches a failure nobody has thought of yet. + +For layer 3, keep a virtual machine whose entire value is what is *not* installed in it. VirtualBox and VMware Workstation both run on Windows 11 Home, which has neither Windows Sandbox nor Hyper-V; Microsoft publishes Windows 11 ISOs at no cost, and an unactivated install is fine for this. + +**Snapshot it the moment Windows finishes installing, before anything else touches it.** That snapshot is the asset — the VM itself is disposable. Restore it after every test, and never install Visual Studio, the Rust toolchain, or anything that carries a redistributable, or it quietly becomes another machine that cannot tell you anything. + +What to run inside it, cheapest first: + +- **The `.exe` installer.** No Developer Mode, no ceremony, and it carries the same native binaries as the appx. Record something and open the editor: if recording starts and the preview renders, the missing-runtime class is clear for both Windows channels at once. +- **The appx**, for what is specific to MSIX — package-graph DLL resolution, which the `.exe` cannot exercise. Enable Developer Mode and run `verify-appx-native.ps1`, then launch the app by hand. Developer Mode installs no runtime, so it does not compromise what the VM is for. + +Expect the compositor to report a software backend: a VM has no real GPU, and `probeBackend()` returning `"software"` there is correct, not a regression. Enable the hypervisor's 3D acceleration so the preview renders at all. + ### Stale native artifacts **On Windows, always package with `npm run build:win`, not `npm run build`.**