diff --git a/src/Fallout.Build/Execution/BuildContext.cs b/src/Fallout.Build/Execution/BuildContext.cs new file mode 100644 index 00000000..aeb4256a --- /dev/null +++ b/src/Fallout.Build/Execution/BuildContext.cs @@ -0,0 +1,71 @@ +using System; +using System.Collections.Generic; +using System.Threading; +using Fallout.Common.Tooling; +using Fallout.Common.Utilities.Collections; +using Fallout.Common.ValueInjection; + +namespace Fallout.Common.Execution; + +/// +/// Per-run, process-ambient build state. Activated once at the top of +/// and disposed at the end of the run, so the process-global statics a build touches are owned by a +/// scope rather than leaking across invocations (the cleanup FT-1 centralised now lives here). +/// The static surface (e.g. ) reads through +/// , which is AsyncLocal — so concurrent runs each resolve their own +/// context. That isolates the ambient pointer and the per-run state hanging off it (the cancellation +/// handlers, the event subscriptions), not the process-global singletons the teardown resets +/// (in-memory sink, value-injection cache, tool-path resolver config); those stay shared, so +/// concurrent runs in one process still interfere through them. +/// +/// +/// FT-2 / #307. Intentionally +/// internal — not a public contract until the SDK lands (milestone #7). Subsequent steps move +/// the per-run services (parameters, logging scope, tool-path config) onto this context. +/// +internal sealed class BuildContext : IDisposable +{ + private static readonly AsyncLocal currentInstance = new(); + + /// The context for the current build run, or null outside a run. + public static BuildContext Current => currentInstance.Value; + + private readonly LinkedList cancellationHandlers = new(); + private readonly ConsoleCancelEventHandler onCancelKeyPress; + private readonly EventHandler onToolOptionsCreated; + + private BuildContext() + { + onCancelKeyPress = (_, _) => cancellationHandlers.ForEach(x => x()); + onToolOptionsCreated = (options, _) => VerbosityMapping.Apply((ToolOptions)options); + Console.CancelKeyPress += onCancelKeyPress; + ToolOptions.Created += onToolOptionsCreated; + } + + /// Creates the ambient context for a build run and installs it as . + public static BuildContext Activate() => currentInstance.Value = new BuildContext(); + + public void RegisterCancellationHandler(Action handler) => cancellationHandlers.AddFirst(handler); + + public void UnregisterCancellationHandler(Action handler) => cancellationHandlers.Remove(handler); + + public void Dispose() + { + // This scope's own subscriptions come off unconditionally — they are per-instance, so undoing + // them can never affect another context. + Console.CancelKeyPress -= onCancelKeyPress; + ToolOptions.Created -= onToolOptionsCreated; + + // The rest is shared process-wide state, so only the context that is still Current may reset + // it — a superseded scope disposing out of order must not clobber the newer run. + if (!ReferenceEquals(currentInstance.Value, this)) + return; + + Logging.InMemorySink.Instance.Clear(); + ValueInjectionUtility.ClearCache(); + NuGetToolPathResolver.Reset(); + NpmToolPathResolver.Reset(); + + currentInstance.Value = null; + } +} diff --git a/src/Fallout.Build/Execution/BuildManager.cs b/src/Fallout.Build/Execution/BuildManager.cs index eff09ba6..33fe49dc 100644 --- a/src/Fallout.Build/Execution/BuildManager.cs +++ b/src/Fallout.Build/Execution/BuildManager.cs @@ -8,7 +8,6 @@ using Fallout.Common.Tooling; using Fallout.Common.Utilities; using Fallout.Common.Utilities.Collections; -using Fallout.Common.ValueInjection; using Serilog; #pragma warning disable CA2255 @@ -18,12 +17,12 @@ internal static class BuildManager { private const int ErrorExitCode = -1; - private static readonly LinkedList cancellationHandlers = new(); - + // Facade over the active BuildContext (FT-2): callers register/unregister during a run; the + // context owns the handler list and discards it on dispose. public static event Action CancellationHandler { - add => cancellationHandlers.AddFirst(value); - remove => cancellationHandlers.Remove(value); + add => BuildContext.Current?.RegisterCancellationHandler(value); + remove => BuildContext.Current?.UnregisterCancellationHandler(value); } [ModuleInitializer] @@ -40,16 +39,11 @@ public static int Execute(Expression>[] defaultTargetExpressi Console.OutputEncoding = Encoding.UTF8; Console.InputEncoding = Encoding.UTF8; + // The per-run scope owns the global subscriptions + teardown (FT-2 / #307). Disposed at + // method exit, so re-invocation in the same process starts clean even if `new T()` throws. + using var context = BuildContext.Activate(); var build = new T(); - // Hold the per-run global subscriptions in locals so the finally can undo exactly them — - // otherwise each Execute in the same process (tests, hosted scenarios) accumulates handlers. - // FT-1 / #306. - ConsoleCancelEventHandler onCancelKeyPress = (_, _) => cancellationHandlers.ForEach(x => x()); - EventHandler onToolOptionsCreated = (options, _) => VerbosityMapping.Apply((ToolOptions)options); - Console.CancelKeyPress += onCancelKeyPress; - ToolOptions.Created += onToolOptionsCreated; - try { Logging.Configure(build); @@ -95,16 +89,8 @@ public static int Execute(Expression>[] defaultTargetExpressi { Finish(); Log.CloseAndFlush(); - - // FT-1 (#306): undo this run's process-global state so a subsequent Execute in the same - // process starts clean — no accumulated handlers, no carried-over log events / caches / config. - CancellationHandler -= Finish; - Console.CancelKeyPress -= onCancelKeyPress; - ToolOptions.Created -= onToolOptionsCreated; - Logging.InMemorySink.Instance.Clear(); - ValueInjectionUtility.ClearCache(); - NuGetToolPathResolver.Reset(); - NpmToolPathResolver.Reset(); + // Per-run teardown (handler unsubscription + state reset) is owned by the BuildContext, + // run when `context` is disposed at method exit. } void Finish() diff --git a/tests/Fallout.Build.Specs/BuildContextSpecs.cs b/tests/Fallout.Build.Specs/BuildContextSpecs.cs new file mode 100644 index 00000000..73f0c47b --- /dev/null +++ b/tests/Fallout.Build.Specs/BuildContextSpecs.cs @@ -0,0 +1,165 @@ +using System; +using System.Reflection; +using FluentAssertions; +using Fallout.Common.Execution; +using Fallout.Common.IO; +using Fallout.Common.Tooling; +using Fallout.Common.ValueInjection; +using Serilog.Events; +using Serilog.Parsing; +using Xunit; + +namespace Fallout.Common.Specs.Execution; + +/// +/// Covers (FT-2 / #307) — the per-run ambient scope that owns the +/// process-global state a build touches. is AsyncLocal, so +/// each case resolves its own context, but the singletons the teardown resets (in-memory sink, +/// value-injection cache, resolver config) remain process-wide — assertions that touch them key on a +/// sentinel rather than on the singleton being empty. +/// +[Collection(ProcessGlobalStateCollection.Name)] +public class BuildContextSpecs +{ + [Fact] + public void Current_is_null_outside_a_run() + { + BuildContext.Current.Should().BeNull(); + } + + [Fact] + public void Activate_installs_the_returned_context_as_current() + { + using var context = BuildContext.Activate(); + + BuildContext.Current.Should().BeSameAs(context); + } + + [Fact] + public void Dispose_clears_current() + { + var context = BuildContext.Activate(); + context.Dispose(); + + BuildContext.Current.Should().BeNull(); + } + + [Fact] + public void Disposing_a_superseded_context_leaves_the_newer_one_current() + { + var first = BuildContext.Activate(); + using var second = BuildContext.Activate(); + + // Disposing the older scope must not clobber the newer Current — the guard keys on identity. + first.Dispose(); + + BuildContext.Current.Should().BeSameAs(second); + } + + [Fact] + public void Disposing_a_superseded_context_leaves_the_shared_state_intact() + { + var sink = Logging.InMemorySink.Instance; + var sentinel = $"build-context-superseded-{Guid.NewGuid()}"; + + var first = BuildContext.Activate(); + using var second = BuildContext.Activate(); + NuGetToolPathResolver.EmbeddedPackagesDirectory = sentinel; + sink.Emit(CreateLogEvent(sentinel)); + + // `second` is the live run and this state belongs to it. The teardown resets are process-wide, + // so a superseded scope disposing out of order must skip them entirely — clearing Current is + // not the only thing the identity guard protects. + first.Dispose(); + + sink.LogEvents.Should().Contain(x => x.MessageTemplate.Text == sentinel); + NuGetToolPathResolver.EmbeddedPackagesDirectory.Should().Be(sentinel); + } + + [Fact] + public void Dispose_resets_the_tool_path_resolver_configuration() + { + using (BuildContext.Activate()) + { + NuGetToolPathResolver.EmbeddedPackagesDirectory = "/packages"; + NuGetToolPathResolver.NuGetPackagesConfigFile = "/packages.config"; + NpmToolPathResolver.NpmPackageJsonFile = (AbsolutePath)"/npm/package.json"; + } + + NuGetToolPathResolver.EmbeddedPackagesDirectory.Should().BeNull(); + NuGetToolPathResolver.NuGetPackagesConfigFile.Should().BeNull(); + NpmToolPathResolver.NpmPackageJsonFile.Should().BeNull(); + } + + [Fact] + public void Dispose_clears_the_value_injection_cache() + { + ValueInjectionUtility.ClearCache(); + CountingInjectionAttribute.Reset(); + var subject = new Subject(); + + using (BuildContext.Activate()) + ValueInjectionUtility.TryGetValue(() => subject.Value).Should().Be("1"); + + // The context's teardown cleared the cache, so this read re-injects rather than replaying the + // value the previous run computed. + ValueInjectionUtility.TryGetValue(() => subject.Value).Should().Be("2"); + CountingInjectionAttribute.Invocations.Should().Be(2); + } + + [Fact] + public void Cancellation_handler_facade_is_a_no_op_without_an_active_context() + { + Action noop = () => { }; + + // Register/unregister route through Current; outside a run there is nothing to route to, so + // the facade must swallow the call rather than throw. + var subscribe = () => + { + BuildManager.CancellationHandler += noop; + BuildManager.CancellationHandler -= noop; + }; + + subscribe.Should().NotThrow(); + } + + [Fact] + public void Dispose_runs_the_per_run_teardown() + { + var sink = Logging.InMemorySink.Instance; + var sentinel = $"build-context-teardown-{Guid.NewGuid()}"; + + using (BuildContext.Activate()) + sink.Emit(CreateLogEvent(sentinel)); + + // Leaving the scope disposes the context, whose teardown clears the shared in-memory sink so a + // subsequent run in the same process starts clean. Assert on the sentinel specifically so a + // build emitting on another flow can't make this flaky. + sink.LogEvents.Should().NotContain(x => x.MessageTemplate.Text == sentinel); + } + + private static LogEvent CreateLogEvent(string message) => + new( + timestamp: DateTimeOffset.UnixEpoch, + level: LogEventLevel.Warning, + exception: null, + messageTemplate: new MessageTemplateParser().Parse(message), + properties: Array.Empty()); + + // Makes the injected-value cache observable: the value is the running invocation count, so a + // cached read repeats the previous value while a re-injected one increments. + private sealed class Subject + { + [CountingInjection] + public string Value; + } + + private sealed class CountingInjectionAttribute : ValueInjectionAttributeBase + { + public static int Invocations { get; private set; } + + public static void Reset() => Invocations = 0; + + public override object GetValue(MemberInfo member, object instance) => (++Invocations).ToString(); + } +} diff --git a/tests/Fallout.Build.Specs/InMemorySinkSpecs.cs b/tests/Fallout.Build.Specs/InMemorySinkSpecs.cs index 77723743..dc7c0ec8 100644 --- a/tests/Fallout.Build.Specs/InMemorySinkSpecs.cs +++ b/tests/Fallout.Build.Specs/InMemorySinkSpecs.cs @@ -13,6 +13,7 @@ namespace Fallout.Common.Specs; /// stop one run's warnings/errors bleeding into the next build in the same process; Dispose() /// delegates to the same reset. /// +[Collection(ProcessGlobalStateCollection.Name)] public class InMemorySinkSpecs { private static readonly Logging.InMemorySink Sink = Logging.InMemorySink.Instance; diff --git a/tests/Fallout.Build.Specs/ProcessGlobalStateCollection.cs b/tests/Fallout.Build.Specs/ProcessGlobalStateCollection.cs new file mode 100644 index 00000000..f739aafd --- /dev/null +++ b/tests/Fallout.Build.Specs/ProcessGlobalStateCollection.cs @@ -0,0 +1,15 @@ +using Xunit; + +namespace Fallout.Common.Specs; + +/// +/// Serialises the specs that read or mutate the process-wide singletons a build run touches — the +/// in-memory sink, the value-injection cache, the tool-path resolver config. xUnit runs test classes +/// in parallel by default, and these singletons are shared across every flow, so without a common +/// collection one class's reset can land in the middle of another's arrange/assert. +/// +[CollectionDefinition(Name)] +public sealed class ProcessGlobalStateCollection +{ + public const string Name = "process-global state"; +} diff --git a/tests/Fallout.Build.Specs/ValueInjectionUtilitySpecs.cs b/tests/Fallout.Build.Specs/ValueInjectionUtilitySpecs.cs index f84480d1..5d8c6fa1 100644 --- a/tests/Fallout.Build.Specs/ValueInjectionUtilitySpecs.cs +++ b/tests/Fallout.Build.Specs/ValueInjectionUtilitySpecs.cs @@ -14,6 +14,7 @@ namespace Fallout.Common.Specs; /// A counting attribute makes the cache observable: the injected value is the running call count, so /// a cached read repeats the previous value while a post-ClearCache read re-injects a fresh one. /// +[Collection(ProcessGlobalStateCollection.Name)] public class ValueInjectionUtilitySpecs { [Fact]