From 34621489def6e0a0f7240d330a9990904ab214d1 Mon Sep 17 00:00:00 2001 From: Vasil Kotsev <9307969+SonnyRR@users.noreply.github.com> Date: Sat, 10 Oct 2026 11:34:14 +0300 Subject: [PATCH 1/3] Fall back to the default when an environment variable cannot be read A parameter value the build could not parse used to abort the process through Assert.Fail. Built-in parameters are resolved inside FalloutBuild's static constructor, so the exception escaped before any target ran and before the consumer had a chance to handle it. The standard Unix HOST variable made this routine. Every zsh user has HOST exported, and Fallout reads it as the value for the Host parameter, so builds failed at startup with "Value 'M2' could not be converted to 'Host'". The same happened for any unusable value, for example VERBOSITY set to something that is not a verbosity level. Parameters now report a warning and use the default instead. Warnings go to standard error rather than Serilog, because Serilog is not configured yet at this point in startup. That also makes the existing "multiple values are provided" warning visible, which it never was. Fixes #705. --- .../Execution/ParameterService.cs | 30 +++++--- .../ParameterServiceSpecs.cs | 75 +++++++++++++++++++ 2 files changed, 94 insertions(+), 11 deletions(-) diff --git a/src/Fallout.Build/Execution/ParameterService.cs b/src/Fallout.Build/Execution/ParameterService.cs index 9ad5d6fe4..cd9a4417d 100644 --- a/src/Fallout.Build/Execution/ParameterService.cs +++ b/src/Fallout.Build/Execution/ParameterService.cs @@ -196,7 +196,7 @@ static string GetTrimmedName(string name) if (alternativeValues.Count > 1) { - Log.Warning("Could not resolve {VariableName} since multiple values are provided", variableName); + ReportWarning($"could not resolve '{variableName}' since multiple values are provided"); } if (alternativeValues.Count == 1) @@ -213,17 +213,25 @@ static string GetTrimmedName(string name) { return Convert(value, destinationType, separator, booleanDefault: false); } - catch (Exception ex) + catch (Exception exception) { - Assert.Fail(new[] - { - ex.Message, - $"Resolving parameter '{variableName}' failed. Environment variable was:", - value - }.JoinNewLine()); - - // ReSharper disable once HeuristicUnreachableCode - return null; + // A value the build cannot read must not abort the process. Built-in parameters are + // resolved inside FalloutBuild's static constructor, so throwing here kills the build + // before the consumer can act on it and before any target runs. + ReportWarning( + $"could not resolve '{variableName}' from environment variable value '{value}'. " + + $"Ignoring it and using the default. {exception.Message}"); + + return destinationType.GetDefaultValue(); } } + + /// + /// Writes a warning to standard error. Parameters are resolved before Serilog is configured, + /// so anything sent to the log is never shown. + /// + private static void ReportWarning(string message) + { + Console.Error.WriteLine($"warning: {message}"); + } } diff --git a/tests/Fallout.Build.Specs/ParameterServiceSpecs.cs b/tests/Fallout.Build.Specs/ParameterServiceSpecs.cs index 484212288..30317a960 100644 --- a/tests/Fallout.Build.Specs/ParameterServiceSpecs.cs +++ b/tests/Fallout.Build.Specs/ParameterServiceSpecs.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.IO; using Fallout.Build.Execution; using Fallout.Common.IO; using FluentAssertions; @@ -8,6 +9,7 @@ namespace Fallout.Common.Specs; +[Collection(ProcessGlobalStateCollection.Name)] public class ParameterServiceSpecs { private ParameterService GetService(string[] commandLineArguments = null, @@ -171,6 +173,79 @@ public void TestPrecedence() service.GetParameter("string", typeof(string), separator: null).Should().Be("commitMessage"); } + [Fact] + public void UnusableHostEnvironmentValue_UsesDefaultAndWarns() + { + // Arrange. The standard Unix HOST variable. FalloutBuild resolves the Host parameter + // from the environment before any target runs. + var environmentVariables = new Dictionary { ["HOST"] = "M2" }; + + // Act + var (value, error) = ResolveWithEnvironment("Host", typeof(Host), environmentVariables); + + // Assert + value.Should().BeNull(); + error.Should().Contain("could not resolve 'Host'"); + error.Should().Contain("M2"); + } + + [Fact] + public void UnusableEnvironmentValue_UsesDefaultAndWarns() + { + // Arrange + var environmentVariables = new Dictionary { ["VERBOSITY"] = "bogus" }; + + // Act + var (value, error) = ResolveWithEnvironment("Verbosity", typeof(Verbosity?), environmentVariables); + + // Assert + value.Should().BeNull(); + error.Should().Contain("could not resolve 'Verbosity'"); + error.Should().Contain("bogus"); + } + + [Fact] + public void SeveralMatchingEnvironmentValues_UseDefaultAndWarn() + { + // Arrange. Two spellings of the same parameter name, neither an exact match. + var environmentVariables = new Dictionary + { + ["verbosity"] = "Normal", + ["VERBOSITY"] = "Verbose" + }; + + // Act + var (value, error) = ResolveWithEnvironment("Verbosity", typeof(Verbosity?), environmentVariables); + + // Assert + value.Should().BeNull(); + error.Should().Contain("multiple values are provided"); + } + + private (object Value, string Error) ResolveWithEnvironment( + string parameterName, + Type destinationType, + IDictionary environmentVariables) + { + var service = GetService(environmentVariables: environmentVariables); + var original = Console.Error; + using var writer = new StringWriter(); + + object value; + try + { + // Parameters resolve before Serilog is configured, so warnings go to standard error. + Console.SetError(writer); + value = service.GetParameter(parameterName, destinationType, separator: null); + } + finally + { + Console.SetError(original); + } + + return (value, writer.ToString()); + } + #pragma warning disable CS0649 private class TestBuild : FalloutBuild, ITestComponent { From 5de7b9ec57373f5f9d024dd6cb54c79701b92ac2 Mon Sep 17 00:00:00 2001 From: Vasil Kotsev <9307969+SonnyRR@users.noreply.github.com> Date: Sat, 10 Oct 2026 11:36:22 +0300 Subject: [PATCH 2/3] Match the FALLOUT_ prefix when resolving parameters from the environment The documentation states that parameters can be set with a FALLOUT_ prefix, but the resolver only ever looked for NUKE_. A FALLOUT_ variable was ignored. The resolver now matches FALLOUT_ and NUKE_ before the bare parameter name. Prefixed names come first because the bare match is case-insensitive, so on Unix the HOST variable matches the Host parameter and would otherwise shadow FALLOUT_HOST. This also fixes a case where an explicit override was dropped. With HOST and NUKE_HOST both set, both were collected as candidates, the resolver saw more than one, and it fell back to the default. The prefixed value now wins. Fixes #457. --- docs/website/02-fundamentals/06-parameters.md | 4 ++ .../Execution/ParameterService.cs | 12 ++++- .../ParameterServiceSpecs.cs | 53 +++++++++++++++++++ 3 files changed, 67 insertions(+), 2 deletions(-) diff --git a/docs/website/02-fundamentals/06-parameters.md b/docs/website/02-fundamentals/06-parameters.md index 012397a1c..a48c3eac7 100644 --- a/docs/website/02-fundamentals/06-parameters.md +++ b/docs/website/02-fundamentals/06-parameters.md @@ -85,6 +85,10 @@ SET MY_PARAMETER = SET FALLOUT_MY_PARAMETER = ``` +A prefixed name takes precedence over the bare parameter name. Use the prefix when the two would otherwise collide, for example with the `Host` parameter and the `HOST` variable that every Unix shell sets to the machine's hostname. + +If a value cannot be read, Fallout reports a warning on standard error and uses the default for that parameter. It does not stop the build. + ## Required Parameters You can specify a parameter as a [target requirement](../02-fundamentals/05-targets.md#requirements) using the following shorthand syntax: diff --git a/src/Fallout.Build/Execution/ParameterService.cs b/src/Fallout.Build/Execution/ParameterService.cs index cd9a4417d..5989c66a7 100644 --- a/src/Fallout.Build/Execution/ParameterService.cs +++ b/src/Fallout.Build/Execution/ParameterService.cs @@ -190,10 +190,18 @@ static string GetTrimmedName(string name) if (!Variables.TryGetValue(variableName, out var value)) { var trimmedVariableName = GetTrimmedName(variableName); - var alternativeValues = Variables - .Where(x => GetTrimmedName(x.Key).EqualsOrdinalIgnoreCase(trimmedVariableName) || + + // Prefixed names are matched before the bare parameter name. FALLOUT_HOST would + // otherwise be shadowed by an unrelated bare HOST, which is the hostname variable + // every Unix shell exports. + var prefixedValues = Variables + .Where(x => GetTrimmedName(x.Key).EqualsOrdinalIgnoreCase($"FALLOUT{trimmedVariableName}") || GetTrimmedName(x.Key).EqualsOrdinalIgnoreCase($"NUKE{trimmedVariableName}")).ToList(); + var alternativeValues = prefixedValues.Count > 0 + ? prefixedValues + : Variables.Where(x => GetTrimmedName(x.Key).EqualsOrdinalIgnoreCase(trimmedVariableName)).ToList(); + if (alternativeValues.Count > 1) { ReportWarning($"could not resolve '{variableName}' since multiple values are provided"); diff --git a/tests/Fallout.Build.Specs/ParameterServiceSpecs.cs b/tests/Fallout.Build.Specs/ParameterServiceSpecs.cs index 30317a960..0b3205f3d 100644 --- a/tests/Fallout.Build.Specs/ParameterServiceSpecs.cs +++ b/tests/Fallout.Build.Specs/ParameterServiceSpecs.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.IO; using Fallout.Build.Execution; +using Fallout.Common.Execution; using Fallout.Common.IO; using FluentAssertions; using Xunit; @@ -222,6 +223,58 @@ public void SeveralMatchingEnvironmentValues_UseDefaultAndWarn() error.Should().Contain("multiple values are provided"); } + [Theory] + [InlineData("FALLOUT_VERBOSITY")] + [InlineData("fallout_verbosity")] + [InlineData("Fallout_Verbosity")] + public void PrefixedEnvironmentValue_IsResolved(string name) + { + // Arrange + var environmentVariables = new Dictionary { [name] = "Verbose" }; + + // Act + var (value, _) = ResolveWithEnvironment("Verbosity", typeof(Verbosity?), environmentVariables); + + // Assert + value.Should().Be(Verbosity.Verbose); + } + + [Fact] + public void PrefixedHostValue_TakesPrecedenceOverBareHost() + { + // Arrange. The Unix hostname variable is present and cannot be converted. + var environmentVariables = new Dictionary + { + ["HOST"] = "M2", + ["FALLOUT_HOST"] = "Terminal" + }; + + // Act + var (value, error) = ResolveWithEnvironment("Host", typeof(Host), environmentVariables); + + // Assert + value.Should().BeOfType(); + error.Should().BeEmpty(); + } + + [Fact] + public void LegacyPrefixedHostValue_TakesPrecedenceOverBareHost() + { + // Arrange + var environmentVariables = new Dictionary + { + ["HOST"] = "M2", + ["NUKE_HOST"] = "Terminal" + }; + + // Act + var (value, error) = ResolveWithEnvironment("Host", typeof(Host), environmentVariables); + + // Assert + value.Should().BeOfType(); + error.Should().BeEmpty(); + } + private (object Value, string Error) ResolveWithEnvironment( string parameterName, Type destinationType, From d7d376d8f984aae5db51e5ac0c0af4deea7ac2ab Mon Sep 17 00:00:00 2001 From: Vasil Kotsev <9307969+SonnyRR@users.noreply.github.com> Date: Thu, 15 Oct 2026 17:22:50 +0300 Subject: [PATCH 3/3] Log environment parameter warnings through Serilog instead of Console Review feedback on the PR asked to keep logging on the logger and not call Console directly. - Restore the original Log.Warning call for the "multiple values" case. - Log the conversion failure with Log.Warning and keep `ex` as the exception variable name. - Remove the Console.Error helper. - Tests capture warnings through a Serilog sink filtered by the VariableName property, so log output from test classes that run in parallel cannot leak into the assertions. - Docs: state the lookup order (exact name, prefixed name, bare fuzzy match), drop "on standard error", and stop claiming every Unix shell exports HOST. Only some login shells do, for example on openSUSE. --- docs/website/02-fundamentals/06-parameters.md | 4 +- .../Execution/ParameterService.cs | 34 +++++----- .../ParameterServiceSpecs.cs | 62 ++++++++++++------- 3 files changed, 56 insertions(+), 44 deletions(-) diff --git a/docs/website/02-fundamentals/06-parameters.md b/docs/website/02-fundamentals/06-parameters.md index a48c3eac7..6b0f0780f 100644 --- a/docs/website/02-fundamentals/06-parameters.md +++ b/docs/website/02-fundamentals/06-parameters.md @@ -85,9 +85,9 @@ SET MY_PARAMETER = SET FALLOUT_MY_PARAMETER = ``` -A prefixed name takes precedence over the bare parameter name. Use the prefix when the two would otherwise collide, for example with the `Host` parameter and the `HOST` variable that every Unix shell sets to the machine's hostname. +Environment variables are checked in this order: the exact parameter name, then the `FALLOUT_` or `NUKE_` prefixed name, then the bare name with casing and underscores ignored. Use the prefix when the bare name collides with an unrelated variable. For example, login shells on some systems export `HOST` with the machine's hostname, which would otherwise be read as the `Host` parameter. -If a value cannot be read, Fallout reports a warning on standard error and uses the default for that parameter. It does not stop the build. +If a value cannot be converted, Fallout logs a warning and uses the default for that parameter. It does not stop the build. ## Required Parameters diff --git a/src/Fallout.Build/Execution/ParameterService.cs b/src/Fallout.Build/Execution/ParameterService.cs index 5989c66a7..70ecd68ec 100644 --- a/src/Fallout.Build/Execution/ParameterService.cs +++ b/src/Fallout.Build/Execution/ParameterService.cs @@ -191,9 +191,9 @@ static string GetTrimmedName(string name) { var trimmedVariableName = GetTrimmedName(variableName); - // Prefixed names are matched before the bare parameter name. FALLOUT_HOST would - // otherwise be shadowed by an unrelated bare HOST, which is the hostname variable - // every Unix shell exports. + // Prefixed names are matched before the bare parameter name. Otherwise an unrelated + // bare variable would shadow the prefixed one, for example the HOST variable that login + // shells on openSUSE export would shadow FALLOUT_HOST. var prefixedValues = Variables .Where(x => GetTrimmedName(x.Key).EqualsOrdinalIgnoreCase($"FALLOUT{trimmedVariableName}") || GetTrimmedName(x.Key).EqualsOrdinalIgnoreCase($"NUKE{trimmedVariableName}")).ToList(); @@ -204,7 +204,7 @@ static string GetTrimmedName(string name) if (alternativeValues.Count > 1) { - ReportWarning($"could not resolve '{variableName}' since multiple values are provided"); + Log.Warning("Could not resolve {VariableName} since multiple values are provided", variableName); } if (alternativeValues.Count == 1) @@ -221,25 +221,19 @@ static string GetTrimmedName(string name) { return Convert(value, destinationType, separator, booleanDefault: false); } - catch (Exception exception) + catch (Exception ex) { - // A value the build cannot read must not abort the process. Built-in parameters are - // resolved inside FalloutBuild's static constructor, so throwing here kills the build - // before the consumer can act on it and before any target runs. - ReportWarning( - $"could not resolve '{variableName}' from environment variable value '{value}'. " + - $"Ignoring it and using the default. {exception.Message}"); + // The environment is shared with the operating system, so an unrelated variable such as + // the Unix HOST can match a parameter by accident. Built-in parameters are also resolved + // in FalloutBuild's static constructor, where throwing aborts the process before any + // target runs. Warn and use the default instead. + Log.Warning( + "Could not resolve {VariableName} from environment variable value {Value}. Using the default value instead. {Reason}", + variableName, + value, + ex.Message); return destinationType.GetDefaultValue(); } } - - /// - /// Writes a warning to standard error. Parameters are resolved before Serilog is configured, - /// so anything sent to the log is never shown. - /// - private static void ReportWarning(string message) - { - Console.Error.WriteLine($"warning: {message}"); - } } diff --git a/tests/Fallout.Build.Specs/ParameterServiceSpecs.cs b/tests/Fallout.Build.Specs/ParameterServiceSpecs.cs index 0b3205f3d..ff37345ef 100644 --- a/tests/Fallout.Build.Specs/ParameterServiceSpecs.cs +++ b/tests/Fallout.Build.Specs/ParameterServiceSpecs.cs @@ -1,10 +1,12 @@ using System; using System.Collections.Generic; -using System.IO; using Fallout.Build.Execution; using Fallout.Common.Execution; using Fallout.Common.IO; using FluentAssertions; +using Serilog; +using Serilog.Core; +using Serilog.Events; using Xunit; using static Fallout.Common.Utilities.ReflectionUtility; @@ -182,12 +184,11 @@ public void UnusableHostEnvironmentValue_UsesDefaultAndWarns() var environmentVariables = new Dictionary { ["HOST"] = "M2" }; // Act - var (value, error) = ResolveWithEnvironment("Host", typeof(Host), environmentVariables); + var (value, warnings) = ResolveWithEnvironment("Host", typeof(Host), environmentVariables); // Assert value.Should().BeNull(); - error.Should().Contain("could not resolve 'Host'"); - error.Should().Contain("M2"); + warnings.Should().ContainSingle().Which.Should().Contain("Host").And.Contain("M2"); } [Fact] @@ -197,12 +198,11 @@ public void UnusableEnvironmentValue_UsesDefaultAndWarns() var environmentVariables = new Dictionary { ["VERBOSITY"] = "bogus" }; // Act - var (value, error) = ResolveWithEnvironment("Verbosity", typeof(Verbosity?), environmentVariables); + var (value, warnings) = ResolveWithEnvironment("Verbosity", typeof(Verbosity?), environmentVariables); // Assert value.Should().BeNull(); - error.Should().Contain("could not resolve 'Verbosity'"); - error.Should().Contain("bogus"); + warnings.Should().ContainSingle().Which.Should().Contain("Verbosity").And.Contain("bogus"); } [Fact] @@ -216,11 +216,11 @@ public void SeveralMatchingEnvironmentValues_UseDefaultAndWarn() }; // Act - var (value, error) = ResolveWithEnvironment("Verbosity", typeof(Verbosity?), environmentVariables); + var (value, warnings) = ResolveWithEnvironment("Verbosity", typeof(Verbosity?), environmentVariables); // Assert value.Should().BeNull(); - error.Should().Contain("multiple values are provided"); + warnings.Should().ContainSingle().Which.Should().Contain("multiple values are provided"); } [Theory] @@ -250,11 +250,11 @@ public void PrefixedHostValue_TakesPrecedenceOverBareHost() }; // Act - var (value, error) = ResolveWithEnvironment("Host", typeof(Host), environmentVariables); + var (value, warnings) = ResolveWithEnvironment("Host", typeof(Host), environmentVariables); // Assert value.Should().BeOfType(); - error.Should().BeEmpty(); + warnings.Should().BeEmpty(); } [Fact] @@ -268,35 +268,53 @@ public void LegacyPrefixedHostValue_TakesPrecedenceOverBareHost() }; // Act - var (value, error) = ResolveWithEnvironment("Host", typeof(Host), environmentVariables); + var (value, warnings) = ResolveWithEnvironment("Host", typeof(Host), environmentVariables); // Assert value.Should().BeOfType(); - error.Should().BeEmpty(); + warnings.Should().BeEmpty(); } - private (object Value, string Error) ResolveWithEnvironment( + /// + /// Resolves one parameter from the given environment and returns it together with the warnings + /// logged for that parameter. Swaps the process-wide logger for the duration of the call, so + /// the sink keeps only events tagged with the parameter name. Test classes outside the shared + /// collection run in parallel and would otherwise leak their log output into the result. + /// + private (object Value, IReadOnlyList Warnings) ResolveWithEnvironment( string parameterName, Type destinationType, IDictionary environmentVariables) { var service = GetService(environmentVariables: environmentVariables); - var original = Console.Error; - using var writer = new StringWriter(); + var sink = new ParameterWarningSink(parameterName); + var original = Log.Logger; - object value; try { - // Parameters resolve before Serilog is configured, so warnings go to standard error. - Console.SetError(writer); - value = service.GetParameter(parameterName, destinationType, separator: null); + Log.Logger = new LoggerConfiguration().WriteTo.Sink(sink, LogEventLevel.Warning).CreateLogger(); + var value = service.GetParameter(parameterName, destinationType, separator: null); + return (value, sink.Messages); } finally { - Console.SetError(original); + Log.Logger = original; } + } + + private class ParameterWarningSink(string parameterName) : ILogEventSink + { + public List Messages { get; } = new(); - return (value, writer.ToString()); + public void Emit(LogEvent logEvent) + { + if (logEvent.Properties.TryGetValue("VariableName", out var property) && + property is ScalarValue { Value: string name } && + name == parameterName) + { + Messages.Add(logEvent.RenderMessage()); + } + } } #pragma warning disable CS0649