Fallout-build / Fallout-build/Fallout

Parameters file: Enumeration-typed parameters (MSBuildTargetPlatform, …) silently fall back to their default

Open
#667 0 comments 0 reactions 0 assignees View on GitHub
bug needs-triage
Dominant language
C#
Stars
154
Forks
19
Avg merge
1d 22h
Merged PRs (30d)
15

Description

### Usage Information

Fallout / .net 10 / Windows 11

### Description

### Summary

Any `[Parameter]` whose type derives from `Fallout.Common.Tooling.Enumeration` cannot be read from the parameters file. Deserialization throws, the exception is caught and logged as a warning, and the parameter silently keeps its default value.

`AbsolutePath` had the same root cause and was fixed in #598 by adding a dedicated `JsonConverter` — but only for that one type, and only on the write path. The `Enumeration` family on the read path in `ArgumentsFromParametersFileAttribute` has no such converter.

### Root cause

`MSBuildTargetPlatform` relies on a `TypeConverter`:

```
Fallout.Common.Tools.MSBuild.MSBuildTargetPlatform : Fallout.Common.Tooling.Enumeration
[TypeConverter(typeof(Enumeration.TypeConverter))]
```

The command-line path goes through that converter and works. The parameters-file path does not: since #174 migrated `ArgumentsFromParametersFileAttribute` from `JObject.Parse` to `JsonNode.Parse` / `GetValue`, the value goes straight to `System.Text.Json`, which ignores `[TypeConverter]` — exactly the behaviour diagnosed in #588. `ObjectDefaultConverter` then treats the type as a plain POCO and expects a JSON object, so a string throws.

Newtonsoft fell back to `TypeDescriptor`/`TypeConverter` for string-to-object conversion, which is why this worked before the migration.

### Why this is hard to diagnose

Three properties of the current behaviour compound each other:

**The generated schema prescribes the shape the deserializer rejects.** Fallout writes `.fallout/build.schema.json` itself and declares `Platform` as a string enum. Following Fallout's own documented contract is what triggers the bug — there is no correct way to write this entry.

**The natural workaround is silent and worse.** Rewriting the entry as an object removes the warning but yields an `Enumeration` with `Value == null`, because the `Value` setter is protected and System.Text.Json skips it. That instance throws on use. Details in the workaround section.

**A parameters file that cannot be parsed is only a warning.** `ValueInjectionAttributeBase.TryGetValue` swallows the exception and leaves the property initializer in place, so the build proceeds with an unconfigured value. The failure then appears somewhere else entirely — or not at all, as a gated target that quietly stops running.

### Scope

Every `Enumeration`-derived parameter read from the parameters file. `Platform`, `Configuration` and `Verbosity` are all affected.

### Suggested fix

Register a `JsonConverter` for `Enumeration`-derived types in the `JsonSerializerOptions` used by `ArgumentsFromParametersFileAttribute`, reading the JSON string and delegating to the existing `Enumeration.TypeConverter`. This mirrors #598, but generically for the `Enumeration` family and on the read path.

Independently, consider failing the build when a value in the parameters file cannot be converted, rather than falling back to the default.

### Related

- #598 — same defect for `AbsolutePath`; fixed for that type only, write path only
- #588 — documents that System.Text.Json ignores `[TypeConverter]`
- #174 — the migration that moved `ArgumentsFromParametersFileAttribute` to System.Text.Json
- #457 — another parameter that silently fails to inject

### Environment

Fallout Execution Engine 10.4.0.15, Windows, .NETCoreApp v10.0.

### Reproduction Steps

Minimal repro — three files, `Fallout.Common` 10.4.0 as the only dependency. No MSBuild and no solution involved; the target only logs the parameter value, so this isolates parameter injection.

**`.fallout/parameters.json`**

```json
{
"Platform": "x86"
}
```

**`build/_build.csproj`**

```xml


Exe
net10.0

..
..
false



```

**`build/Build.cs`**

```csharp
using Fallout.Common;
using Fallout.Common.Tools.MSBuild;
using Serilog;

class Build : FalloutBuild
{
public static int Main() => Execute(x => x.ShowPlatform);

[Parameter("Platform for the build")]
readonly MSBuildTargetPlatform Platform = MSBuildTargetPlatform.MSIL;

Target ShowPlatform => _ => _
.Executes(() => Log.Information("Platform = {Platform}", Platform));
}
```

### Run

```
dotnet run --project build/_build.csproj
```

**Expected:** `Platform = x86`
**Actual:** `Platform = MSIL`, preceded by a warning:

```
[WRN] Could not inject value for Build.Platform
[INF] Platform = MSIL
```

The exception is only in `.fallout/temp/build..log`, not on the console:

```
W | | Could not inject value for Build.Platform
System.Text.Json.JsonException: The JSON value could not be converted to Fallout.Common.Tools.MSBuild.MSBuildTargetPlatform. Path: $ | LineNumber: 0 | BytePositionInLine: 5.
at System.Text.Json.ThrowHelper.ThrowJsonException_DeserializeUnableToConvertValue(Type propertyType)
at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value, Boolean& isPopulatedValue)
at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, T& value, JsonSerializerOptions options, ReadStack& state)
at System.Text.Json.Serialization.Metadata.JsonTypeInfo`1.Deserialize(Utf8JsonReader& reader, ReadStack& state)
at System.Text.Json.Serialization.Metadata.JsonTypeInfo`1.DeserializeAsObject(Utf8JsonReader& reader, ReadStack& state)
at System.Text.Json.JsonSerializer.ReadFromSpanAsObject(ReadOnlySpan`1 utf8Json, JsonTypeInfo jsonTypeInfo, Nullable`1 actualByteCount)
at System.Text.Json.JsonSerializer.ReadFromNodeAsObject(JsonNode node, JsonTypeInfo jsonTypeInfo)
at Fallout.Common.Execution.ArgumentsFromParametersFileAttribute.<>c__DisplayClass0_0.b__5(String parameter, Type destinationType)
in src/Fallout.Build/Execution/Extensions/ArgumentsFromParametersFileAttribute.cs:line 80
at Fallout.Common.ParameterService.g__TryFromProfileArguments|18_3(<>c__DisplayClass18_0&)
in src/Fallout.Build/Execution/ParameterService.cs:line 150
at Fallout.Common.ParameterService.GetParameter(String parameterName, Type destinationType, Nullable`1 separator)
in src/Fallout.Build/Execution/ParameterService.cs:line 155
at Fallout.Common.ParameterService.GetFromMemberInfo(MemberInfo member, Type destinationType, Func`4 provider)
in src/Fallout.Build/Execution/ParameterService.cs:line 129
at Fallout.Common.ParameterService.GetParameter[T](MemberInfo member, Type destinationType)
in src/Fallout.Build/Execution/ParameterService.Statics.cs:line 32
at Fallout.Common.ParameterAttribute.GetValue(MemberInfo member, Object instance)
in src/Fallout.Build/ParameterAttribute.cs:line 50
at Fallout.Common.ValueInjection.ValueInjectionAttributeBase.TryGetValue(MemberInfo member, Object instance)
in src/Fallout.Build/Execution/Extensibility/ValueInjectionAttributeBase.cs:line 18
```

### Two more runs, same repro

**The command line works** — that path still goes through `Enumeration.TypeConverter`:

```
dotnet run --project build/_build.csproj -- --platform x86
```
```
[INF] Platform = x86
```

**The obvious workaround makes it worse.** Writing the value as an object satisfies `ObjectDefaultConverter`, so the warning disappears — but `Enumeration.Value` has a protected setter (`Family, HideBySig, SpecialName`), so System.Text.Json skips it and returns an instance with `Value == null`:

```json
{
"Platform": { "Value": "x86" }
}
```
```
[INF] Platform = Capturing the property value threw an exception: ArgumentException
```

No warning, no error — just a broken object handed to the build.

### Summary of the three runs

| `.fallout/parameters.json` | command line | resulting `Platform` | warning |
|---|---|---|---|
| `"Platform": "x86"` | — | `MSIL` (the default) | yes |
| `"Platform": "x86"` | `--platform x86` | `x86` | no |
| `"Platform": { "Value": "x86" }` | — | broken instance, `Value == null` | no |

Environment: Fallout Execution Engine 10.4.0.15, Windows, .NETCoreApp v10.0.

### Expected Behavior

`Platform` is `MSBuildTargetPlatform.x86`, matching the parameters file.

`"Platform": "x86"` is the shape Fallout's own generated `.fallout/build.schema.json` prescribes:

```json
"Platform": { "type": "string", "enum": ["arm", "MSIL", "Win32", "x64", "x86"] }
```

A parameters file that validates against that schema should be readable by Fallout. The same string is accepted on the command line (`--platform x86`), so both paths should agree.

Secondary expectation: if a value in the parameters file cannot be converted, the build should fail with that error rather than warn and silently substitute a default. A malformed build configuration is not something a build should work around on its own.

### Actual Behavior

`Platform` keeps its default value `MSIL`. The value `"x86"` from the parameters file is discarded.

Console output:

```
[WRN] Could not inject value for Build.Platform
[INF] Platform = MSIL
```

The underlying exception appears only in `.fallout/temp/build..log`, never on the console — `System.Text.Json.JsonException: The JSON value could not be converted to Fallout.Common.Tools.MSBuild.MSBuildTargetPlatform` (full stack trace in the reproduction steps above).

The build then continues with a value nobody configured. In our real build this surfaced in two ways, both far from the cause:

- MSBuild was invoked with `/p:Platform="Any CPU"` and failed minutes later on a solution that only defines `x86`:
```
error MSB4126: The specified solution configuration "Debug|Any CPU" is invalid.
```
Nothing in that message points back to the warning at startup.
- A target gated on `OnlyWhenDynamic(() => Platform == MSBuildTargetPlatform.x86)` had quietly stopped running weeks earlier. No error, no warning about it — a build step simply no longer happened.

This affects every `Enumeration`-derived parameter read from the parameters file, not just `Platform`; `Configuration` and `Verbosity` behave the same way.

### Regression?

_No response_

### Known Workarounds

Pass the parameter on the command line. That path still goes through `Enumeration.TypeConverter` and converts correctly:

```
fallout --platform x86
```

For a fixed value, hardcoding the default in the build class also works, at the cost of losing the parameter.

**Do not** rewrite the entry as an object — it looks like it works and does not:

```json
{ "Platform": { "Value": "x86" } }
```

`ObjectDefaultConverter` is satisfied, so the warning disappears, but `Enumeration.Value` has a protected setter (`Family, HideBySig, SpecialName`). System.Text.Json cannot write it, skips the property, and returns an instance with `Value == null`. That instance is unusable — even rendering it into a log message throws:

```
[INF] Platform = Capturing the property value threw an exception: ArgumentException
```

This form is strictly worse than the string form: no warning, no error, broken value.

### Could you help with a pull-request?

No

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.