Skip to content

Move ParameterService onto the BuildContext (FT-4)#452

Open
ChrisonSimtian wants to merge 2 commits into
Fallout-build:mainfrom
ChrisonSimtian:engine/ft4-parameter-service
Open

Move ParameterService onto the BuildContext (FT-4)#452
ChrisonSimtian wants to merge 2 commits into
Fallout-build:mainfrom
ChrisonSimtian:engine/ft4-parameter-service

Conversation

@ChrisonSimtian

@ChrisonSimtian ChrisonSimtian commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Moves ParameterService from a static singleton onto BuildContext, so each build run gets its own instance instead of sharing process-wide state.

What changed

  • Adds a Parameters property to BuildContext holding a per-run ParameterService.
  • ParameterService.Instance becomes a facade over BuildContext.Current.Parameters, with a lazy ambient fallback for use outside a run.
  • Adds four cases to BuildContextSpecs: the facade resolves the running context's instance, falls back to a stable instance outside a run, each run gets its own service, and a mutated per-run field doesn't leak into the next run.

Why

Production and the specs now use the same instance form — the specs already built ParameterService directly, but production relied on the static singleton. The singleton's mutable fields (ArgumentsFromFilesService, ArgumentsFromCommitMessageService) used to live for the whole process, so state could leak between runs. Scoping the service to BuildContext closes that gap.

The ambient ("ready-to-use without setup") ​fallback stays process-wide for now, since there's no cross-run state left to leak on that path. It can be removed once we confirm nothing uses ParameterService outside a run.

Non-breaking — the static ParameterService API is unchanged.

Part of #309 (FT-4, the third step of the engine de-statification work), following #450 (FT-1) and #451 (FT-2, which set up the pattern this PR reuses). Part of the epic #315.

Verification

Full build + full spec suite green (14 projects, 0 failures).

@ChrisonSimtian ChrisonSimtian added the target/vCurrent Targets the current version label Jun 30, 2026
@ChrisonSimtian ChrisonSimtian added target/vNext Targets the next calendar-version and removed target/vCurrent Targets the current version labels Jul 20, 2026
The active ParameterService is now the per-run instance held on BuildContext.Parameters; the static
ParameterService.Instance becomes a facade over BuildContext.Current. This is the first service to
ride the FT-2 rail: production and tests now exercise the same instance form (killing the test/prod
divergence — tests already `new ParameterService(funcs)`), and the per-run instance + its mutable
fields (ArgumentsFromFilesService / ArgumentsFromCommitMessageService) no longer leak across runs.

A lazy fallback covers access outside a build run (no cross-run state to leak there); it can retire
once that path is confirmed dead. Non-breaking — the static API is unchanged.
Four cases on the FT-4 seam: the static facade resolves the running context's instance, it falls
back to a stable ambient instance outside a run, each run gets its own service, and a mutated
per-run field does not survive into the next run.
@ChrisonSimtian
ChrisonSimtian force-pushed the engine/ft4-parameter-service branch from 119fee5 to 69b4088 Compare July 26, 2026 10:17
@ChrisonSimtian ChrisonSimtian added target/vCurrent Targets the current version skip-changelog and removed target/vNext Targets the next calendar-version labels Jul 26, 2026
@ChrisonSimtian
ChrisonSimtian marked this pull request as ready for review July 26, 2026 10:18
@ChrisonSimtian
ChrisonSimtian requested a review from a team as a code owner July 26, 2026 10:18
ChrisonSimtian added a commit to ChrisonSimtian/Fallout that referenced this pull request Jul 26, 2026
Runs a fixture build end-to-end, twice in one process, and asserts what the de-static work promised:
both runs succeed, the second gets its own context/parameter service/log sink, it opens on an empty
sink, and the static facades inside a run point at that run.

This is what the original FT-9 commit set out to do. Its unit-level assertions have since landed
with the steps they belong to (FT-2 in Fallout-build#451, FT-4 in Fallout-build#452, FT-6 in Fallout-build#453), so what is left here is
the end-to-end case they could not cover — and it immediately found the leaked file-sink logger
fixed in the previous commit. Verified as a real guard: reverting that fix fails 4 of these 5 specs.

Two process-wide inputs are pinned to make a build runnable under a test host — the argument parser
(swapped and restored) and the statically-resolved root directory (asserted to carry the .fallout
marker, since UpdateNotification prompts for input without it). Both are de-statification candidates
in their own right.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog target/vCurrent Targets the current version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant