Skip to content

Commit 80de493

Browse files
fix: address PR review - pin toolchain versions, fix dasel stdin
Review feedback on the Windows FFI build tooling: - env-setup.ps1: pin protoc (35.1) and dasel (v3.11.1) instead of pulling "latest". "latest" had silently moved dasel v2 -> v3, whose selector syntax differs and broke build-win.ps1; pinning makes the build reproducible and removes the supply-chain risk. - build-win.ps1: feed dasel via `cmd /c "... < file"` input redirection instead of a `type ... |` pipe. dasel v3 on Windows only reads stdin from an OS file-handle redirect; pipes (pwsh, cmd, .NET stdin) deliver nothing, so the old pipe path never actually patched the profile. - build-win.ps1: rename cargoToml/patchToml vars to *Path to disambiguate path from content. - Patch.toml: make it the single source for the profile flags; annotate each flag with its purpose. - BUILD-WINDOWS.md: drop the redundant "(via dasel)" note and the duplicated flag list, point at Patch.toml instead.
1 parent beeb2ca commit 80de493

4 files changed

Lines changed: 37 additions & 32 deletions

File tree

BuildScripts~/windows/BUILD-WINDOWS.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ Build the **`livekit-ffi`** crate as a release cdylib **with PDB debug symbols**
1010
| File | What it is |
1111
|------|------------|
1212
| `build.config.psd1` | Config: `Tag` (which release to download/build) and `InstallToPlugins` (where the output goes). Edit this, not the scripts. |
13-
| `Patch.toml` | The `[profile.release]` that gets overlaid onto the downloaded `Cargo.toml` so the build emits debug symbols. Edit here to change build flags. |
13+
| `Patch.toml` | The `[profile.release]` overlaid onto the downloaded `Cargo.toml`. See its header comment for what each flag does and why; edit it to change build flags. |
1414
| `env-setup.ps1` | One-time toolchain install: VS2022 Build Tools (MSVC v143) + Windows 11 SDK, Git, Rust (MSVC), libclang, protoc, dasel. Requires Python already on PATH. |
15-
| `build-win.ps1` | Clones the `Tag` source (with nested submodules) into `.src/`, overlays `Patch.toml` onto `[profile.release]` (via dasel) for PDB output, runs `cargo build`, and places the DLL + PDB per `InstallToPlugins`. |
15+
| `build-win.ps1` | Clones the `Tag` source (with nested submodules) into `.src/`, overlays `Patch.toml` onto `[profile.release]` for PDB output, runs `cargo build`, and places the DLL + PDB per `InstallToPlugins`. |
1616

1717
## Quick start
1818

@@ -52,7 +52,7 @@ The source is cloned to `<SourceDir>\rust-sdks-<tag>\` and reused on re-runs. Th
5252

5353
- **Source is downloaded, not vendored.** `build-win.ps1` does `git clone --recurse-submodules` of `livekit/rust-sdks` at the tag into `.src/` (gitignored); this also pulls the nested `yuv-sys/libyuv` + `livekit-protocol/protocol` submodules. webrtc is downloaded separately by `livekit-ffi/build.rs`. Nothing is added to this repo. (The `client-sdk-rust~` submodule that lives here is for C# proto generation via `generate_proto.sh`, not for this build.)
5454
- **`+crt-static` comes from upstream, not the script.** The downloaded source's `.cargo/config.toml` sets `target-feature=+crt-static` for `x86_64-pc-windows-msvc`; cargo picks it up because the build runs from the source root. The script does not set it.
55-
- **The profile patch is deliberate, and done with dasel.** `build-win.ps1` overlays `Patch.toml`'s `[profile.release]` onto the downloaded `Cargo.toml` using dasel (proper TOML parsing rather than a fragile regex): `debug = 2` + `split-debuginfo = "packed"` for the PDB, plus `lto`/`opt-level = "z"`/`panic = "abort"`/`codegen-units = 1`. dasel reformats the file (reorders tables, single-quotes strings, drops comments), which is harmless since the checkout under `.src/` is a throwaway. This matches upstream's own release profile, so for recent tags it just re-asserts existing values. It requires the source to already define `[profile.release]` (livekit-ffi tags do); if one ever doesn't, the patch step fails loudly rather than silently shipping a symbol-less build.
55+
- **The profile patch is deliberate, and done with dasel.** `build-win.ps1` overlays `Patch.toml`'s `[profile.release]` onto the downloaded `Cargo.toml` using dasel (proper TOML parsing rather than a fragile regex); see `Patch.toml` for the flags and what each is for. dasel reformats the file (reorders tables, single-quotes strings, drops comments), which is harmless since the checkout under `.src/` is a throwaway. This matches upstream's own release profile, so for recent tags it just re-asserts existing values. It requires the source to already define `[profile.release]` (livekit-ffi tags do); if one ever doesn't, the patch step fails loudly rather than silently shipping a symbol-less build.
5656
- **Long paths.** With the default `SourceDir = '.src'` the clone lives under this repo folder, so webrtc's deeply nested files can hit the 260-char limit. The script clones with `core.longpaths=true`; if a build step still trips on path length, set `SourceDir` to a short absolute path (e.g. `C:\src`), enable Windows long paths (`LongPathsEnabled=1`), or move the repo nearer the drive root.
5757

5858
## Why VS2022 + Windows 11 SDK

BuildScripts~/windows/Patch.toml

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
1-
# Patch.toml - the [profile.release] that build-win.ps1 overlays onto the downloaded Cargo.toml
2-
# (via dasel) so the build emits separate debug symbols (PDB on Windows, dSYM on macOS).
3-
# This mirrors upstream rust-sdks' own release profile; edit here to change build flags.
1+
# Patch.toml - the [profile.release] that build-win.ps1 overlays onto the downloaded Cargo.toml,
2+
# so the build emits separate debug symbols (a .pdb on Windows, a .dSYM on macOS) instead of
3+
# baking them into the library. This mirrors upstream rust-sdks' own release profile; it is the
4+
# single source for these flags - edit here to change them.
45
[profile.release]
5-
# traceable build
6-
debug = 2 # goes to dSYM
7-
split-debuginfo = "packed" # pack dSYM
8-
strip = "symbols" # symbols are striped from dylib but persist in dSYM
6+
# Emit debug symbols to a separate file (.pdb on Windows, .dSYM on macOS) instead of the binary:
7+
debug = 2
8+
split-debuginfo = "packed"
9+
strip = "symbols"
10+
# Optimization flags below match upstream's release profile.
911
lto = true
10-
1112
opt-level = "z"
1213
codegen-units = 1
1314
panic = "abort"

BuildScripts~/windows/build-win.ps1

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -91,16 +91,20 @@ if (-not (Test-Path (Join-Path $repo 'yuv-sys\libyuv\include\libyuv'))) { throw
9191
# --- 3. Patch [profile.release] (traceable build with PDB) -----------------
9292
# Overlay the profile from Patch.toml with dasel (parses TOML properly, unlike a regex). debug=2
9393
# makes the MSVC linker emit livekit_ffi.pdb; strip=symbols strips the DLL while the PDB keeps the
94-
# symbols. Routed through cmd so stdin pipes reliably across PowerShell versions. dasel reformats
95-
# the file (reorders/quotes/drops comments); harmless for this throwaway build checkout.
94+
# symbols. dasel reformats the file (reorders/quotes/drops comments); harmless for this throwaway
95+
# build checkout.
96+
#
97+
# Fed via `cmd /c "... < file"`, not a pipe: dasel v3 on Windows reads stdin only from a real
98+
# file-handle redirect - any pipe delivers nothing and the selector fails. PowerShell 5.1 has no
99+
# `<` operator, so this one step shells out to cmd.
96100
Step "Patch [profile.release]"
97-
$cargoToml = Join-Path $repo 'Cargo.toml'
98-
$patchToml = Join-Path $PSScriptRoot 'Patch.toml'
99-
$cmd = "type `"$cargoToml`" | `"$dasel`" -i toml -o toml --var `"patch=toml:file:$patchToml`" `"profile.release = `$patch.profile.release`" --root"
101+
$cargoTomlPath = Join-Path $repo 'Cargo.toml'
102+
$patchTomlPath = Join-Path $PSScriptRoot 'Patch.toml'
103+
$cmd = "`"$dasel`" -i toml -o toml --var `"patch=toml:file:$patchTomlPath`" `"profile.release = `$patch.profile.release`" --root < `"$cargoTomlPath`""
100104
$patched = cmd /c $cmd
101105
if ($LASTEXITCODE -ne 0 -or -not $patched) { throw "dasel failed to patch [profile.release] (does $Tag define one?)" }
102-
[System.IO.File]::WriteAllText($cargoToml, (($patched -join "`n").TrimEnd() + "`n"), (New-Object System.Text.UTF8Encoding($false))) # no BOM
103-
Info "patched $cargoToml via dasel"
106+
[System.IO.File]::WriteAllText($cargoTomlPath, (($patched -join "`n").TrimEnd() + "`n"), (New-Object System.Text.UTF8Encoding($false))) # no BOM
107+
Info "patched $cargoTomlPath via dasel"
104108

105109
# --- 4. Build --------------------------------------------------------------
106110
Step "cargo build --release -p livekit-ffi"

BuildScripts~/windows/env-setup.ps1

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55

66
[CmdletBinding()]
77
param(
8-
[string] $ProtocDir = "$env:LOCALAPPDATA\protoc", # where protoc is unpacked
9-
[string] $DaselDir = "$env:LOCALAPPDATA\dasel" # where dasel.exe is placed
8+
[string] $ProtocDir = "$env:LOCALAPPDATA\protoc", # where protoc is unpacked
9+
[string] $DaselDir = "$env:LOCALAPPDATA\dasel", # where dasel.exe is placed
10+
[string] $ProtocVersion = '35.1', # pinned, not "latest" (reproducible builds)
11+
[string] $DaselVersion = 'v3.11.1' # pinned; build-win.ps1 targets this major (v3)
1012
)
1113

1214
$ErrorActionPreference = 'Stop'
@@ -49,22 +51,20 @@ python -m pip install --upgrade libclang
4951
$libclang = python -c "import clang, os; print(os.path.join(os.path.dirname(clang.__file__), 'native'))"
5052
Write-Host " LIBCLANG_PATH = $libclang"
5153

52-
# 5. protoc: required by the livekit-ffi build script (prost-build). Unpack the latest release.
53-
Step "protoc (latest release)"
54-
$rel = Invoke-RestMethod -Headers @{ 'User-Agent' = 'build' } https://api.github.com/repos/protocolbuffers/protobuf/releases/latest
55-
$asset = $rel.assets | Where-Object { $_.name -match 'protoc-.*-win64\.zip' } | Select-Object -First 1
56-
$zip = "$env:TEMP\$($asset.name)"
57-
Invoke-WebRequest $asset.browser_download_url -OutFile $zip
54+
# 5. protoc: required by the livekit-ffi build script (prost-build). Pinned, not "latest", so a
55+
# future protoc release can't change the toolchain under us.
56+
Step "protoc (v$ProtocVersion)"
57+
$zip = "$env:TEMP\protoc-$ProtocVersion-win64.zip"
58+
Invoke-WebRequest "https://github.com/protocolbuffers/protobuf/releases/download/v$ProtocVersion/protoc-$ProtocVersion-win64.zip" -OutFile $zip
5859
New-Item -ItemType Directory -Force -Path $ProtocDir | Out-Null
5960
tar -xf $zip -C $ProtocDir # -> $ProtocDir\bin\protoc.exe
60-
Write-Host " PROTOC = $ProtocDir\bin\protoc.exe ($($rel.tag_name))"
61+
Write-Host " PROTOC = $ProtocDir\bin\protoc.exe (v$ProtocVersion)"
6162

6263
# 6. dasel: build-win.ps1 uses it to patch [profile.release] in Cargo.toml. Single static binary.
63-
Step "dasel (latest release)"
64-
$rel = Invoke-RestMethod -Headers @{ 'User-Agent' = 'build' } https://api.github.com/repos/TomWright/dasel/releases/latest
65-
$asset = $rel.assets | Where-Object { $_.name -match 'dasel_windows_amd64\.exe$' } | Select-Object -First 1
64+
# Pinned: "latest" drifts across majors (the v2 -> v3 selector-syntax change breaks build-win.ps1).
65+
Step "dasel ($DaselVersion)"
6666
New-Item -ItemType Directory -Force -Path $DaselDir | Out-Null
67-
Invoke-WebRequest $asset.browser_download_url -OutFile (Join-Path $DaselDir 'dasel.exe')
68-
Write-Host " DASEL = $DaselDir\dasel.exe ($($rel.tag_name))"
67+
Invoke-WebRequest "https://github.com/TomWright/dasel/releases/download/$DaselVersion/dasel_windows_amd64.exe" -OutFile (Join-Path $DaselDir 'dasel.exe')
68+
Write-Host " DASEL = $DaselDir\dasel.exe ($DaselVersion)"
6969

7070
Step "Toolchain installed. Next: .\build-win.ps1"

0 commit comments

Comments
 (0)