Skip to content

Add a reproducible SDK-free build and cross-component validation gate - #69

Closed
JimboFGC wants to merge 2 commits into
Confetti3:releasefrom
JimboFGC:contribution/development-validation
Closed

JimboFGC wants to merge 2 commits into
Confetti3:releasefrom
JimboFGC:contribution/development-validation

Conversation

@JimboFGC

@JimboFGC JimboFGC commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

This makes the development-build and validation gate reproducible across components. SDK-free output remains isolated in build/current-no-discord and its stage directory; the public release packaging path continues to require the configured Discord SDK build.

Behavior and implementation

  • Resolve explicit Python/Node/Cargo/Rustup paths and the repository's pinned Rust channel once in BuildEnvironment.ps1. An invalid override fails; an override already later in PATH is promoted and its absolute executable is used.
  • Require Node 22.18+ in the 22.x family, or Node 24+, consistently in preflight, SDK execution, and package metadata. Node 23 is deliberately excluded.
  • Pass the shared pinned RUSTUP_TOOLCHAIN to every CMake Rust build/test. Register service, SDK, and build-validation suites separately from Windows helper compilation, including helper-off/core configurations.
  • Force testing and component registration on for the full build gate. Require CTest's --no-tests=error, parse actual JUnit results, require a nonempty complete run with no failures/errors/skips and the expected suite names, and derive the receipt's result from that evidence.
  • Preserve upstream source and binary provenance checks, explicit network/Discord exclusions, and SDK integration checks that fail when their bridge binary is missing.
  • Keep supplemental service/lint/Linux checks and live network/gameplay acceptance explicitly unexecuted in the receipt unless separately run.

Review regression coverage

BuildValidation checks valid and malformed/empty/partial/skipped/duplicate JUnit reports, Node boundaries, executable overrides, helper-off registrations in Windows/non-Windows CMake fixtures, and use of the pinned channel when the default toolchain is deliberately unavailable. The fixtures execute all four service registrations rather than only searching generated CMake text.

Complete workspace coverage

scripts/rust-workspaces.json is the shared inventory for the helper and the ember, rooms, short-link and reports services. CTest registration, the Windows/Linux quality runners, the CI matrix, advisory scanning and the build receipt's required suites consume it. CMake compares the inventory with independently discovered rust/*/Cargo.toml and server/*/Cargo.toml files and requires lockfiles and unique test names. An omitted future service fails configuration instead of disappearing from the gate. Dependabot also includes server/ember-reports.

Adding reports uncovered three tests that assumed the Linux-only isolated symbolication worker. Those worker tests now explicitly run on Linux; a non-Linux regression verifies that unsupported workers preserve raw exception facts and report worker_unsupported without launching a worker. Report processing behavior is unchanged. This is platform-specific coverage, not a claim that the Linux worker runs on Windows.

Validation of the current revision

PR head: 07f45c1baf717a2bf5dfe5e89e4f80cb08a08ab0. The SDK-free Windows x86 RelWithDebInfo Launcher, Updater and Sidecar, together with the pinned x64 networking helper, built successfully in the combined integration tree 02a982a42db7d710aae3babb590b71907a6104e4.

  • 14 selected native/render CTests passed, with no failed or skipped tests: LauncherHomeLinks, LauncherLinks, RoomRematch, SettingsNavigation, ShellJourney, RematchNavigation, IrohMatchSession, RematchRuntime, SpectatorRuntime, StartupUpdate, UpdaterRun, UiRender, UiRenderLocales and UiRenderAtlas.
  • Actual DX9 rendering covered 77,787 frames across 20 English/pseudo viewport/DPI configurations, including ultrawide/4K. The separate remaining-locale sweep also passed, as did the atlas stress run (150 frames / 144 rebuilds).
  • 16 serial live-helper network fixture runs passed: room lifecycle, queue acknowledgements, recovery, game transport, authorized match, terminal recovery, spectators and quick rematch, each on normal and forced-relay routes. Executable/DLL hashes were unchanged during the run. These use real helper processes and synthetic match fixtures; they do not run SSFIV.exe or establish two-PC gameplay acceptance.
  • The workspace-inventory regression passed, including independently discovered manifests and omitted-service rejection. On Windows, the reports service passed 35 Rust tests and Clippy with warnings denied. The Linux worker-specific tests were not run locally.

This is focused validation of these changes and their integration, not a new full development-gate receipt. Discord SDK-enabled builds, native game/controller sessions, clean-machine installation and local Linux execution remain outside this run.

Integration and merge order

Checked against release cd4daa5c2fb19ee5a3ecd14d0e2d9cbee753aa4f. All 136 pairs among the 17 current contribution branches merge cleanly. Three complete merge orders produce the same tree 02a982a42db7d710aae3babb590b71907a6104e4. The seven revised branches were also compared with all 15 other open developer branches (105 comparisons): these fixes introduce no new conflicting pairs or additional conflict files relative to their previous heads.

This PR already overlapped #101, #95, #89, #88, #76, #37 and #35 before these fixes. Those existing overlaps remain and must be reconciled with the actual target branch if that work lands first. Published heads were checked; unpushed work cannot be assessed. The integration results preserve branch history; after a squash/rebase merge of a shared prerequisite, refresh affected dependent branches and rerun their checks before merging.

@JimboFGC
JimboFGC marked this pull request as ready for review October 10, 2026 01:21

@Confetti3 Confetti3 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks, this is a useful change. Keeping the SDK-free output in its own build and stage directory, leaving packaging on the designated directories, and making the SDK integration tests fail when the bridge binary is missing are all the right calls. A few things need to change so the local gate checks what it says it checks.

1. The new service tests do not use the toolchain the prerequisite check approved. check-build-prerequisites.ps1 verifies the pinned channel with rustup run <channel>, and test-sdk.ps1 builds with cargo +<channel>, but the ember-short and ember-rooms tests registered at cmake/sf4e_helper.cmake:57-63 run plain cargo. Only server/ember has a rust-toolchain.toml; the other two directories have none, so cargo uses the machine default. With 1.98.0 installed and an older default, preflight passes and these tests then stop on the manifests' rust-version = "1.98". With a newer default they run on an unpinned compiler. CI avoids this by setting RUSTUP_TOOLCHAIN. Please do the same here: read the channel from rust/sf4-net/rust-toolchain.toml in CMake and pass RUSTUP_TOOLCHAIN in the cmake -E env list for every Rust test (or add a rust-toolchain.toml to the two service directories). A test of the case where the pinned channel is installed but is not the default would cover it.

2. The gate can still report passing tests when none ran. scripts/build-current.ps1:35 calls CTest without --no-tests=error, configure does not set BUILD_TESTING=ON, and the receipt at line 47 still records testsPassed=(!$SkipTests) without looking at any result. The normal configuration enables testing, so this needs a build directory that was configured with testing off, but then the receipt would still say tests passed. Please pass -DBUILD_TESTING=ON, add --no-tests=error, and set testsPassed from the JUnit file you now write (tests greater than zero, no failures, no skips). Checking that the new suites (EmberRust, ember-shortRust, ember-roomsRust, BridgeSdk) appear in that file would also protect against a registration being dropped later.

3. SF4E_PYTHON does not reliably choose the interpreter. Enter-EmberLocalTools (scripts/BuildEnvironment.ps1:46-51) only puts the override's directory on PATH when it is not already somewhere on PATH, and check-build-prerequisites.ps1:41 then uses Get-Command python. If the directory is already later in PATH, an earlier interpreter (or the Windows Store alias) is the one checked. A path that does not exist is ignored silently. Please resolve SF4E_PYTHON directly when it is set, fail if it is not a file, and run that executable.

4. The Node version check accepts versions that cannot run the SDK tests. scripts/test-sdk.ps1:9 and check-build-prerequisites.ps1:15 accept anything above 22.18, including Node 23.0 to 23.5. The test command runs .ts files directly, and type stripping is only on by default from 22.18 and 23.6. Those releases would pass the check and then fail the SDK step. The engines entry in sdk/typescript/package.json (>=22.18) has the same gap. Either require 22.18 or 24 and later in all three places, or add the flag to the test command.

5. Suggestion: register the independent suites outside the helper switch. The short-link, room-supervisor and SDK tests are nested under both WIN32 and SF4E_BUILD_IROH_HELPER (cmake/sf4e_helper.cmake:12-69), but none of them depends on the helper. With the helper off, or on a non-Windows configure, they disappear even though BUILD_TESTING is on. A small CMake file controlled by BUILD_TESTING, registering the Rust workspaces in one loop with the shared toolchain, target directory and timeout, would keep that separate from helper compilation. find_program(... pwsh REQUIRED) would then only apply where the SDK test is registered.

6. Suggestion: resolve each tool once. Node, Python and the Rust channel are chosen separately in BuildEnvironment.ps1, check-build-prerequisites.ps1 and test-sdk.ps1 (the channel is even parsed with two different patterns), and build-current.ps1 runs discovery again after the check. Items 1, 3 and 4 above are all places where the checked tool and the executed tool can differ. Resolving the executable paths and the channel once in BuildEnvironment.ps1 and having the check and the scripts consume those values would remove that class of problem without adding a framework.

The native build and the full gate on a clean machine still need to be run after these changes.

Extract this focused change from the locally tested development build. Keep other features out of the upstream review; the pull request records implementation details, prior integration evidence, and outstanding isolated validation.
@JimboFGC
JimboFGC force-pushed the contribution/development-validation branch from e7e50d8 to 6735ae5 Compare October 17, 2026 01:42
@JimboFGC JimboFGC closed this Oct 17, 2026
@JimboFGC
JimboFGC deleted the contribution/development-validation branch October 17, 2026 22:11
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.

2 participants