From d3c6aee0b3a01ecbf3ba5f5ec91015e5bd6fee41 Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Mon, 1 Jun 2026 02:42:49 +0200 Subject: [PATCH 1/6] Wire existing bv options to buildvana.json (Config Phase 2) Make the *Settings injectables config-aware so each option resolves flag -> buildvana.json -> default: - Add DotNetSettings/DotNetInvocationSettings, resolving dotnet.configuration and the per-command (all/restore/build/test/pack/nugetPush) args and env. DotNetService merges dotnet.all with the per-command settings, appending forwarded command-line arguments last; BuildPipeline takes its default configuration from DotNetSettings instead of a hardcoded "Release". - Make ReleaseSettings config-aware: configuration chain (--configuration -> release.configuration -> dotnet.configuration), release.checkPublicApi, release.dogfood, release.changelogUpdates + release.emptyChangelog (config-only, replacing the removed --unstable-changelog/--require-changelog flags), and release.generateDocsFrom. - ChangelogService.PrepareForRelease substitutes release.emptyChangelog text for an empty "Unreleased changes" section; an empty section with no substitute fails the release. - Replace mainBranch with release.generateDocsFrom: remove GitService.MainBranch and FindMainBranch, the --main-branch global flag and its pre-parsing; gate documentation generation by matching the current branch against the configured regular expressions, and point the changelog permalink at the current branch. Closes #269 Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 18 +-- .../DotNetInvocationConfig.cs | 5 + src/Buildvana.Tool/Build/BuildPipeline.cs | 29 +++-- .../CommandLine/CliArgSplitter.cs | 3 +- src/Buildvana.Tool/Program.cs | 9 +- .../Services/ChangelogService.cs | 12 +- .../Services/DotNetInvocationSettings.cs | 23 ++++ src/Buildvana.Tool/Services/DotNetService.cs | 76 +++++++++--- src/Buildvana.Tool/Services/DotNetSettings.cs | 63 ++++++++++ src/Buildvana.Tool/Services/Git/GitService.cs | 66 +--------- .../Internal/GitHub/GitHubServerAdapter.cs | 2 +- .../Subcommands/GlobalSettings.cs | 4 - .../Subcommands/ReleaseCommand.cs | 41 ++++--- .../Subcommands/ReleaseSettings.cs | 115 +++++++++++++----- .../CliArgSplitterTests.cs | 8 -- .../ReleaseSettingsTests.cs | 105 ++++++++++++++-- .../SettingsHelpReflectionTests.cs | 4 +- 17 files changed, 399 insertions(+), 184 deletions(-) create mode 100644 src/Buildvana.Tool/Services/DotNetInvocationSettings.cs create mode 100644 src/Buildvana.Tool/Services/DotNetSettings.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index bb1dd14..6b075e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `bv --version` prints the tool's informational version and exits without running a command and without printing the startup logo. - `bv` root help (`bv --help`) now shows a `GLOBAL OPTIONS:` section listing the options every subcommand inherits (`--verbosity`/`-v`, `--color`, `--no-color`, `--nologo`, `--version`). These options are now position-independent (accepted before or after the subcommand name) and case-insensitive, matching the rest of `bv`'s option surface. - Commands that forward extra arguments to `dotnet` (`restore`, `build`, `test`, `pack`) are marked as such in `bv`'s root help, and their per-command help (`bv --help`) includes a `FORWARDED ARGUMENTS` section. -- Buildvana now recognizes a repository-root configuration file, `buildvana.json` (or its commented variant `buildvana.jsonc`). The file is **inert** in this release: it is discovered, parsed, validated, and exposed to `bv`, but no setting affects the build yet. A committed JSON schema (`schemas/buildvana.schema.json`) is generated from the typed model, so editors can validate and document the file; unknown keys, an invalid file, or the presence of both `buildvana.json` and `buildvana.jsonc` in the same directory are reported as errors. Both `bv` and the SDK now treat a `buildvana.json`/`buildvana.jsonc` file as a home-directory marker, alongside `.buildvana-home` and Git markers; home-directory discovery now stops at the nearest directory (the starting directory included) that contains any marker. +- Buildvana now recognizes a repository-root configuration file, `buildvana.json` (or its commented variant `buildvana.jsonc`). It is discovered, parsed, validated, and exposed to `bv`; the settings it currently drives are listed below, and more are wired in over subsequent releases. A committed JSON schema (`schemas/buildvana.schema.json`) is generated from the typed model, so editors can validate and document the file; unknown keys, an invalid file, or the presence of both `buildvana.json` and `buildvana.jsonc` in the same directory are reported as errors. Both `bv` and the SDK now treat a `buildvana.json`/`buildvana.jsonc` file as a home-directory marker, alongside `.buildvana-home` and Git markers; home-directory discovery now stops at the nearest directory (the starting directory included) that contains any marker. +- `buildvana.json` now drives several build and release settings that were previously CLI-only or hardcoded. Each resolves as CLI flag (where one exists) → `buildvana.json` → built-in default: + - the default build configuration (`dotnet.configuration`, default `Release`), used by `bv restore`/`build`/`test`/`pack` and as the base of `bv release`'s configuration chain (`--configuration` → `release.configuration` → `dotnet.configuration`); + - extra arguments and environment variables for each `dotnet` invocation: `dotnet.all` (applied to every invocation) merged with the per-command `dotnet.restore`/`dotnet.build`/`dotnet.test`/`dotnet.pack`/`dotnet.nugetPush`, each carrying `args` and `env`. Arguments are appended in the order base → `dotnet.all` → per-command → forwarded command-line arguments (so a `--` argument still wins); environment variables apply `dotnet.all` then the per-command entries; + - the `bv release` settings `release.checkPublicApi`, `release.dogfood`, `release.changelogUpdates` + `release.emptyChangelog`, and `release.generateDocsFrom`. ### Changes to existing features @@ -29,19 +33,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **BREAKING CHANGE**: `bv clean` (formerly known as `bv prepare`) now deletes the `TestResults` directory at the repository root. - `bv clean` (formerly known as `bv prepare`) no longer deletes per-project `TestResults` directories. - `dotnet bv release` no longer folds self-reference (dogfood) updates into the "Prepare release" commit. They now go into a separate `Update self-references to [skip ci]` commit pushed on top, in the same push. The release tag binds to the "Prepare release" commit, so checking out the tag and rebuilding now reproduces the actually-released source state (which still references the previously-published versions). `[skip ci]` is required on the dogfood commit because the new packages are usually not yet published at push time. -- **BREAKING CHANGE**: Five `bv release` options have been renamed: +- **BREAKING CHANGE**: Three `bv release` options have been renamed: - `--versionSpecChange` → `--bump` - `--checkPublicApiFiles` → `--check-public-api` - - `--updateChangelogOnPrerelease` → `--unstable-changelog` - - `--ensureChangelogNotEmpty` → `--require-changelog` - `--updateSelfReferences` → `--dogfood` - **BREAKING CHANGE**: `bv` no longer accepts CLI option values via environment variables. The following env vars are no longer recognized as defaults for their CLI counterparts: - `CONFIGURATION` (`--configuration`) - - `MAIN_BRANCH` (`--main-branch`) - `VERSION_SPEC_CHANGE` (`--versionSpecChange`, now `--bump`) - `CHECK_PUBLIC_API_FILES` (`--checkPublicApiFiles`, now `--check-public-api`) - - `UPDATE_CHANGELOG_ON_PRERELEASE` (`--updateChangelogOnPrerelease`, now `--unstable-changelog`) - - `ENSURE_CHANGELOG_NOT_EMPTY` (`--ensureChangelogNotEmpty`, now `--require-changelog`) - `UPDATE_SELF_REFERENCES` (`--updateSelfReferences`, now `--dogfood`) Pass values via CLI flags instead. @@ -60,8 +59,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `bv test -- --report-trx` reaches the test application. - **BREAKING CHANGE**: `bv release` rejects a `--` separator (and anything after it): unlike the pipeline commands, it has no underlying `dotnet` pass-through to forward arguments to. - **BREAKING CHANGE**: `bv` no longer forces `-maxcpucount:1` on the `dotnet` invocations of `restore`/`build`/`test`/`pack`. MSBuild now uses its default parallelism unless you forward your own `-m`/`-maxcpucount` switch. -- **BREAKING CHANGE**: The `-c`/`--configuration` option is no longer parsed by `bv restore`/`build`/`test`/`pack`; for those commands it is just another forwarded argument, passed after the `--` separator. `bv` emits `Release` as an overridable default, so a forwarded `-c`/`-p:Configuration=` still wins (e.g. `bv build -- -c Debug` builds `Debug`). `bv release` keeps `-c`/`--configuration` as a parsed option, since it needs the value to locate build artifacts. -- `--main-branch` is now a global option: it is accepted at any position on the command line (before or after the subcommand name), appears in the `GLOBAL OPTIONS` section of help, and is honored by the commands that talk to Git. +- **BREAKING CHANGE**: The `-c`/`--configuration` option is no longer parsed by `bv restore`/`build`/`test`/`pack`; for those commands it is just another forwarded argument, passed after the `--` separator. `bv` emits the configured default build configuration (`dotnet.configuration` in `buildvana.json`, or `Release`) as an overridable default, so a forwarded `-c`/`-p:Configuration=` still wins (e.g. `bv build -- -c Debug` builds `Debug`). `bv release` keeps `-c`/`--configuration` as a parsed option, since it needs the value to locate build artifacts. +- **BREAKING CHANGE**: The `--main-branch` global option has been removed, along with `bv`'s main-branch discovery. Documentation generation on release is now gated by `release.generateDocsFrom` in `buildvana.json`: a list of regular expressions matched against the current short branch name (default `["^main$", "^master$"]`, reproducing the previous main/master discovery). The human-curated changelog permalink in generated release notes now points at the release branch itself rather than the discovered main branch. +- **BREAKING CHANGE**: The `--unstable-changelog` and `--require-changelog` options of `bv release` have been removed with no CLI replacement; changelog policy is repository-stable, not per-invocation. Configure it in `buildvana.json` instead: `release.changelogUpdates` (`none` | `stable` | `all`, default `stable`) selects which releases update the changelog, and `release.emptyChangelog` provides substitute text for an empty "Unreleased changes" section (when unset, an empty section fails the release, matching the previous `--require-changelog` default of `true`). - `bv`'s build commands (`clean`, `restore`, `build`, `test`, `pack`) and `release` now observe cancellation. Pressing Ctrl-C (or a host cancelling the operation) stops the pipeline promptly: it stops launching further steps and terminates the running `dotnet` child process instead of waiting for it to finish, then `bv` exits with code 130. Partial build output may be left behind on cancellation; `bv clean` recovers. ### Bugs fixed in this release diff --git a/src/Buildvana.Core.Configuration/DotNetInvocationConfig.cs b/src/Buildvana.Core.Configuration/DotNetInvocationConfig.cs index 9f00953..a274af0 100644 --- a/src/Buildvana.Core.Configuration/DotNetInvocationConfig.cs +++ b/src/Buildvana.Core.Configuration/DotNetInvocationConfig.cs @@ -3,9 +3,14 @@ using System.Collections.Generic; using System.ComponentModel; +using JetBrains.Annotations; namespace Buildvana.Core.Configuration; +/// +/// Configures the extra arguments and environment variables for one kind of dotnet invocation. +/// +[UsedImplicitly(ImplicitUseTargetFlags.WithMembers)] public sealed record DotNetInvocationConfig { /// diff --git a/src/Buildvana.Tool/Build/BuildPipeline.cs b/src/Buildvana.Tool/Build/BuildPipeline.cs index e408c27..ebddf67 100644 --- a/src/Buildvana.Tool/Build/BuildPipeline.cs +++ b/src/Buildvana.Tool/Build/BuildPipeline.cs @@ -22,14 +22,9 @@ namespace Buildvana.Tool.Build; /// internal sealed class BuildPipeline { - /// - /// The MSBuild configuration used when a caller does not specify one. It is emitted as an overridable - /// default, so a user-forwarded -c/-p:Configuration= wins. - /// - public const string DefaultConfiguration = "Release"; - private readonly SolutionContext _solution; private readonly DotNetService _dotnet; + private readonly DotNetSettings _dotnetSettings; private readonly IReporter _reporter; private readonly IReadOnlyList _forwardedArgs; @@ -39,15 +34,18 @@ internal sealed class BuildPipeline public BuildPipeline( SolutionContext solution, DotNetService dotnet, + DotNetSettings dotnetSettings, IReporter reporter, CommandParameters parameters) { Guard.IsNotNull(solution); Guard.IsNotNull(dotnet); + Guard.IsNotNull(dotnetSettings); Guard.IsNotNull(reporter); Guard.IsNotNull(parameters); _solution = solution; _dotnet = dotnet; + _dotnetSettings = dotnetSettings; _reporter = reporter; _forwardedArgs = parameters.Forwarded; } @@ -56,10 +54,10 @@ public BuildPipeline( /// Runs the pipeline from through , inclusive. /// /// The last step to run. - /// The MSBuild configuration to build. + /// The MSBuild configuration to build, or to use the configured default (). /// A token to observe while running the pipeline. When signalled, the pipeline stops launching further steps and the running dotnet child process is terminated. /// A representing the ongoing operation. - public Task RunThroughAsync(BuildStep last, string configuration = DefaultConfiguration, CancellationToken cancellationToken = default) + public Task RunThroughAsync(BuildStep last, string? configuration = null, CancellationToken cancellationToken = default) => RunRangeAsync(BuildStep.Clean, last, configuration, cancellationToken); /// @@ -67,10 +65,10 @@ public Task RunThroughAsync(BuildStep last, string configuration = DefaultConfig /// /// The first step to run. /// The last step to run. - /// The MSBuild configuration to build. + /// The MSBuild configuration to build, or to use the configured default (). /// A token to observe while running the pipeline. When signalled, the pipeline stops launching further steps and the running dotnet child process is terminated. /// A representing the ongoing operation. - public async Task RunRangeAsync(BuildStep first, BuildStep last, string configuration = DefaultConfiguration, CancellationToken cancellationToken = default) + public async Task RunRangeAsync(BuildStep first, BuildStep last, string? configuration = null, CancellationToken cancellationToken = default) { Guard.IsLessThanOrEqualTo((int)first, (int)last, nameof(first)); for (var step = first; step <= last; step++) @@ -84,19 +82,20 @@ public async Task RunRangeAsync(BuildStep first, BuildStep last, string configur /// Runs a single pipeline step. /// /// The step to run. - /// The MSBuild configuration to build (ignored by and ). + /// The MSBuild configuration to build (ignored by and ), or to use the configured default (). /// A token to observe while running the step. When signalled, the running dotnet child process is terminated. /// A representing the ongoing operation. - public async Task RunAsync(BuildStep step, string configuration = DefaultConfiguration, CancellationToken cancellationToken = default) + public async Task RunAsync(BuildStep step, string? configuration = null, CancellationToken cancellationToken = default) { + var resolvedConfiguration = configuration ?? _dotnetSettings.Configuration; using var activity = _reporter.BeginActivity(step.ToString()); var task = step switch { BuildStep.Clean => CleanAsync(cancellationToken), BuildStep.Restore => RestoreAsync(cancellationToken), - BuildStep.Build => BuildAsync(configuration, cancellationToken), - BuildStep.Test => TestAsync(configuration, cancellationToken), - BuildStep.Pack => PackAsync(configuration, cancellationToken), + BuildStep.Build => BuildAsync(resolvedConfiguration, cancellationToken), + BuildStep.Test => TestAsync(resolvedConfiguration, cancellationToken), + BuildStep.Pack => PackAsync(resolvedConfiguration, cancellationToken), _ => ThrowHelper.ThrowArgumentOutOfRangeException(nameof(step), step, "Unknown build step."), }; await task.ConfigureAwait(false); diff --git a/src/Buildvana.Tool/CommandLine/CliArgSplitter.cs b/src/Buildvana.Tool/CommandLine/CliArgSplitter.cs index 64de71b..6269659 100644 --- a/src/Buildvana.Tool/CommandLine/CliArgSplitter.cs +++ b/src/Buildvana.Tool/CommandLine/CliArgSplitter.cs @@ -31,13 +31,12 @@ public static ParsedCommandLine Split(IReadOnlyList args) var reader = new CliOptionReader(working); var verbosity = reader.ReadValue("--verbosity", "-v"); - var mainBranch = reader.ReadValue("--main-branch"); var color = reader.ReadFlag("--color"); var noColor = reader.ReadFlag("--no-color"); var nologo = reader.ReadFlag("--nologo"); var version = reader.ReadFlag("--version"); var helpRequested = reader.ReadFlag("--help", "-h"); - var globals = new GlobalSettings(verbosity, mainBranch, color, noColor, nologo, version); + var globals = new GlobalSettings(verbosity, color, noColor, nologo, version); var (subcommand, positionals, optionTokens) = Classify(reader.Remaining); return new ParsedCommandLine(globals, helpRequested, subcommand, positionals, optionTokens, forwarded); diff --git a/src/Buildvana.Tool/Program.cs b/src/Buildvana.Tool/Program.cs index d3e1fee..26c9308 100644 --- a/src/Buildvana.Tool/Program.cs +++ b/src/Buildvana.Tool/Program.cs @@ -174,13 +174,16 @@ private static ServiceProvider BuildServiceProvider( .AddSingleton(reporter) .AddSingleton(globals) .AddSingleton(new CommandParameters(parsed.OptionTokens, parsed.Forwarded)) - .AddSingleton(static sp => ReleaseSettings.Parse(sp.GetRequiredService().Options)) + .AddSingleton(static sp => ReleaseSettings.Parse( + sp.GetRequiredService().Options, + sp.GetRequiredService(), + sp.GetRequiredService())) .AddSingleton(static _ => new DiscoveredHomeDirectoryProvider(Environment.CurrentDirectory)) // Lazy by design: this factory (and thus discovery, parsing, and validation) runs on first resolve. - // No Phase 1 command resolves BuildvanaConfig, so a malformed buildvana.json stays inert until a - // Phase 2 consumer reads it. + // A malformed buildvana.json stays inert until a consumer (e.g. DotNetSettings or ReleaseSettings) reads it. .AddSingleton(static sp => BuildvanaConfigLoader.Load(sp.GetRequiredService().HomeDirectory)) + .AddSingleton() .AddSingleton() .AddSingleton() .AddSingleton() diff --git a/src/Buildvana.Tool/Services/ChangelogService.cs b/src/Buildvana.Tool/Services/ChangelogService.cs index 7a709a5..4df000f 100644 --- a/src/Buildvana.Tool/Services/ChangelogService.cs +++ b/src/Buildvana.Tool/Services/ChangelogService.cs @@ -92,7 +92,10 @@ public bool HasUnreleasedChanges() /// Prepares the changelog for a new release by moving the contents of the "Unreleased changes" section /// to a new section. /// - public void PrepareForRelease() + /// Text to use as the new section's body when the "Unreleased changes" + /// section has no content. When , an empty section is moved verbatim (producing a + /// title-only section). + public void PrepareForRelease(string? emptyChangelogSubstitute = null) { _reporter.Info("Updating changelog..."); var encoding = new UTF8Encoding(false, true); @@ -211,6 +214,13 @@ List CollectNewSectionLines() result.AddRange(lines); } + // When the "Unreleased changes" section has no real content, substitute the configured text (if any). + if (emptyChangelogSubstitute is not null && result.All(string.IsNullOrWhiteSpace)) + { + result.Clear(); + result.AddRange(emptyChangelogSubstitute.ReplaceLineEndings("\n").Split('\n')); + } + return result; } } diff --git a/src/Buildvana.Tool/Services/DotNetInvocationSettings.cs b/src/Buildvana.Tool/Services/DotNetInvocationSettings.cs new file mode 100644 index 0000000..bf15f75 --- /dev/null +++ b/src/Buildvana.Tool/Services/DotNetInvocationSettings.cs @@ -0,0 +1,23 @@ +// Copyright (C) Tenacom and Contributors. Licensed under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System.Collections.Generic; +using System.Collections.ObjectModel; + +namespace Buildvana.Tool.Services; + +/// +/// The resolved extra arguments and environment variables for one kind of dotnet invocation, +/// taken from a . +/// +/// Extra arguments appended to the invocation. +/// Environment variables applied on top of the inherited environment, keyed by variable name. +internal sealed record DotNetInvocationSettings( + IReadOnlyList Args, + IReadOnlyDictionary Env) +{ + /// + /// Gets an empty set of invocation settings (no extra arguments, no environment variables). + /// + public static DotNetInvocationSettings Empty { get; } = new([], ReadOnlyDictionary.Empty); +} diff --git a/src/Buildvana.Tool/Services/DotNetService.cs b/src/Buildvana.Tool/Services/DotNetService.cs index 239e939..59b5721 100644 --- a/src/Buildvana.Tool/Services/DotNetService.cs +++ b/src/Buildvana.Tool/Services/DotNetService.cs @@ -38,6 +38,7 @@ private static readonly string DotNetMuxer private readonly Lazy _nugetPushConfigurationLazy; private readonly ServerAdapter _server; private readonly VersionService _version; + private readonly DotNetSettings _settings; /// /// Initializes a new instance of the class. @@ -47,18 +48,21 @@ public DotNetService( IProcessRunner processRunner, Lazy nugetPushConfigurationLazy, ServerAdapter server, - VersionService version) + VersionService version, + DotNetSettings settings) { Guard.IsNotNull(reporter); Guard.IsNotNull(processRunner); Guard.IsNotNull(nugetPushConfigurationLazy); Guard.IsNotNull(server); Guard.IsNotNull(version); + Guard.IsNotNull(settings); _reporter = reporter; _processRunner = processRunner; _nugetPushConfigurationLazy = nugetPushConfigurationLazy; _server = server; _version = version; + _settings = settings; } /// @@ -78,11 +82,10 @@ public Task RestoreSolutionAsync(SolutionContext solution, IReadOnlyList solution.SolutionPath, "--disable-parallel", "-nologo", - ..forwardedArgs, ContinuousIntegrationBuildArg(asMSBuildPassthrough: true), ]; - return RunDotNetAsync(args, cancellationToken: cancellationToken); + return RunDotNetAsync(args, _settings.Restore, forwardedArgs, cancellationToken: cancellationToken); } /// @@ -106,11 +109,10 @@ public Task BuildSolutionAsync(SolutionContext solution, string configuration, I "-nologo", $"-p:Configuration={configuration}", .. restore ? Array.Empty() : ["--no-restore"], - ..forwardedArgs, ContinuousIntegrationBuildArg(asMSBuildPassthrough: true), ]; - return RunDotNetAsync(args, cancellationToken: cancellationToken); + return RunDotNetAsync(args, _settings.Build, forwardedArgs, cancellationToken: cancellationToken); } /// @@ -139,7 +141,7 @@ public async Task TestSolutionAsync(SolutionContext solution, string configurati // bv-internal MSBuild evaluation: do not forward the user's arguments here, as they may be // test-application options that `dotnet msbuild` would reject. string[] probeArgs = ["msbuild", projectPath, "-nologo", "-getProperty:IsTestingPlatformApplication"]; - var probe = await RunDotNetAsync(probeArgs, null, InvocationKind.Internal, cancellationToken: cancellationToken).ConfigureAwait(false); + var probe = await RunDotNetAsync(probeArgs, invocationKind: InvocationKind.Internal, cancellationToken: cancellationToken).ConfigureAwait(false); if (string.Equals(probe.StandardOutput.Trim(), "true", StringComparison.OrdinalIgnoreCase)) { @@ -170,11 +172,10 @@ public async Task TestSolutionAsync(SolutionContext solution, string configurati CommonPaths.TestResults, "--output", _reporter.IsVerbosityAtLeast(Verbosity.Detailed) ? "Detailed" : "Normal", - ..forwardedArgs, - ContinuousIntegrationBuildArg(asMSBuildPassthrough: false), + ContinuousIntegrationBuildArg(asMSBuildPassthrough: false), ]; - await RunDotNetAsync(args, cancellationToken: cancellationToken).ConfigureAwait(false); + await RunDotNetAsync(args, _settings.Test, forwardedArgs, cancellationToken: cancellationToken).ConfigureAwait(false); } /// @@ -200,11 +201,10 @@ public Task PackSolutionAsync(SolutionContext solution, string configuration, IR $"-p:Configuration={configuration}", .. restore ? Array.Empty() : ["--no-restore"], .. build ? Array.Empty() : ["--no-build"], - ..forwardedArgs, ContinuousIntegrationBuildArg(asMSBuildPassthrough: true), ]; - return RunDotNetAsync(args, cancellationToken: cancellationToken); + return RunDotNetAsync(args, _settings.Pack, forwardedArgs, cancellationToken: cancellationToken); } /// @@ -244,7 +244,7 @@ .. _reporter.IsVerbosityAtLeast(Verbosity.Diagnostic) ? ["-d"] : Array.Empty - /// Runs a dotnet command with the specified arguments, forwarding the output to the reporter according to the current verbosity. + /// Runs a dotnet command, forwarding the output to the reporter according to the current verbosity. /// - /// The arguments to pass to dotnet, excluding the verbosity argument, which is automatically appended according to the current verbosity. - /// The kind of invocation, which determines how `dotnet` verbosity and output streaming are handled. + /// The base arguments to pass to dotnet (everything the calling method constructs, + /// including the ContinuousIntegrationBuild argument). The verbosity argument is appended automatically + /// according to the current verbosity and invocation kind. + /// The per-command configured settings, whose arguments and environment variables are + /// merged with the settings and applied to the invocation. Pass + /// for bv-internal invocations that must not receive user-configured arguments + /// (e.g. MSBuild property probes). + /// Extra arguments forwarded verbatim from the command line, appended after the + /// configured arguments so they take precedence. + /// The kind of invocation, which determines how dotnet verbosity and output streaming are handled. /// A token that, when signalled, terminates the spawned dotnet child process. /// A representing the ongoing operation, with a result describing child process outcome. private Task RunDotNetAsync( - IEnumerable args, - IReadOnlyDictionary? environment = null, + IReadOnlyList args, + DotNetInvocationSettings? invocation = null, + IReadOnlyList? commandLineArgs = null, InvocationKind invocationKind = InvocationKind.Normal, CancellationToken cancellationToken = default) { @@ -282,12 +291,43 @@ private Task RunDotNetAsync( _ => throw new UnreachableException(), }; + // Order: base args (constructed by the caller) → dotnet.all args → per-command args → command-line args. + // Command-line args come last so they override configured arguments. + IReadOnlyList configuredArgs = invocation is null ? [] : [.. _settings.All.Args, .. invocation.Args]; + string[] finalArgs = [.. args, .. configuredArgs, .. commandLineArgs ?? []]; + var environment = invocation is null ? null : BuildEnvironment(invocation); + return _processRunner.RunAsync( DotNetMuxer, - appendVerbosity ? args.Append($"--verbosity={_reporter.Verbosity}") : args, + appendVerbosity ? finalArgs.Append($"--verbosity={_reporter.Verbosity}") : finalArgs, environment: environment, onStdout: streamOutput ? (x) => _reporter.ChildOutput(x, streamVerbosity) : null, onStderr: streamOutput ? (x) => _reporter.ChildError(x, streamVerbosity) : null, cancellationToken: cancellationToken); } + + // Merges environment variables in the order dotnet.all env → per-command env, so a per-command entry overrides + // an "all" entry with the same name. Returns null when there is nothing to apply, leaving the child environment unchanged. + private Dictionary? BuildEnvironment(DotNetInvocationSettings invocation) + { + var all = _settings.All.Env; + var perCommand = invocation.Env; + if (all.Count == 0 && perCommand.Count == 0) + { + return null; + } + + var merged = new Dictionary(all.Count + perCommand.Count, StringComparer.Ordinal); + foreach (var (key, value) in all) + { + merged[key] = value; + } + + foreach (var (key, value) in perCommand) + { + merged[key] = value; + } + + return merged; + } } diff --git a/src/Buildvana.Tool/Services/DotNetSettings.cs b/src/Buildvana.Tool/Services/DotNetSettings.cs new file mode 100644 index 0000000..ddf85c4 --- /dev/null +++ b/src/Buildvana.Tool/Services/DotNetSettings.cs @@ -0,0 +1,63 @@ +// Copyright (C) Tenacom and Contributors. Licensed under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System.Collections.Generic; +using Buildvana.Core.Configuration; +using CommunityToolkit.Diagnostics; + +namespace Buildvana.Tool.Services; + +/// +/// Resolves dotnet CLI settings from the dotnet section of a : +/// the default build configuration plus the per-command extra arguments and environment variables. +/// +internal sealed class DotNetSettings +{ + /// + /// The build configuration used when neither the configuration chain nor the command line specifies one. + /// + public const string DefaultConfiguration = "Release"; + + /// + /// Initializes a new instance of the class. + /// + /// The Buildvana configuration to read the dotnet section from. + public DotNetSettings(BuildvanaConfig config) + { + Guard.IsNotNull(config); + var dotnet = config.DotNet; + Configuration = dotnet?.Configuration ?? DefaultConfiguration; + All = Resolve(dotnet?.All); + Restore = Resolve(dotnet?.Restore); + Build = Resolve(dotnet?.Build); + Test = Resolve(dotnet?.Test); + Pack = Resolve(dotnet?.Pack); + NugetPush = Resolve(dotnet?.NugetPush); + } + + /// Gets the default build configuration (dotnet.configuration, or "Release"). + public string Configuration { get; } + + /// Gets the settings common to every dotnet command (dotnet.all). + public DotNetInvocationSettings All { get; } + + /// Gets the settings for the dotnet restore command (dotnet.restore). + public DotNetInvocationSettings Restore { get; } + + /// Gets the settings for the dotnet build command (dotnet.build). + public DotNetInvocationSettings Build { get; } + + /// Gets the settings for the dotnet test command (dotnet.test). + public DotNetInvocationSettings Test { get; } + + /// Gets the settings for the dotnet pack command (dotnet.pack). + public DotNetInvocationSettings Pack { get; } + + /// Gets the settings for the dotnet nuget push command (dotnet.nugetPush). + public DotNetInvocationSettings NugetPush { get; } + + private static DotNetInvocationSettings Resolve(DotNetInvocationConfig? config) + => config is null + ? DotNetInvocationSettings.Empty + : new(config.Args ?? [], config.Env ?? new Dictionary()); +} diff --git a/src/Buildvana.Tool/Services/Git/GitService.cs b/src/Buildvana.Tool/Services/Git/GitService.cs index 55d0844..d5f8c9b 100644 --- a/src/Buildvana.Tool/Services/Git/GitService.cs +++ b/src/Buildvana.Tool/Services/Git/GitService.cs @@ -13,7 +13,6 @@ using JetBrains.Annotations; using LibGit2Sharp; using NuGet.Versioning; -using GlobalSettings = Buildvana.Tool.Subcommands.GlobalSettings; namespace Buildvana.Tool.Services.Git; @@ -27,11 +26,10 @@ internal sealed class GitService : IDisposable private readonly IHomeDirectoryProvider _home; private readonly Repository _repository; - public GitService(IReporter reporter, IHomeDirectoryProvider home, GlobalSettings globals) + public GitService(IReporter reporter, IHomeDirectoryProvider home) { Guard.IsNotNull(reporter); Guard.IsNotNull(home); - Guard.IsNotNull(globals); _reporter = reporter; _home = home; var homeDirectory = home.HomeDirectory; @@ -42,7 +40,6 @@ public GitService(IReporter reporter, IHomeDirectoryProvider home, GlobalSetting OriginUrl = new(originUrl); var headName = _repository.Head.CanonicalName; CurrentBranch = headName.StartsWith("refs/heads/", StringComparison.Ordinal) ? _repository.Head.FriendlyName : string.Empty; - MainBranch = FindMainBranch(origin, globals.MainBranch ?? string.Empty); } /// @@ -55,12 +52,6 @@ public GitService(IReporter reporter, IHomeDirectoryProvider home, GlobalSetting /// public Uri OriginUrl { get; } - /// - /// Gets the name of the main Git branch. - /// - /// The name of the main branch. - public string MainBranch { get; } - /// /// Gets the name of the current Git branch. /// @@ -300,59 +291,4 @@ private bool TryGetOriginInfo([MaybeNullWhen(false)] out string name, [MaybeNull _reporter.Detail($"Origin remote is '{name}' ({url})"); return true; } - - private string FindMainBranch(string origin, string configuredMainBranch) - { - var haveConfiguredMainBranch = !string.IsNullOrEmpty(configuredMainBranch); - var mainBranchFound = false; - var mainFound = false; - var masterFound = false; - var mainValue = $"{origin}/main"; - var masterValue = $"{origin}/master"; - var configuredValue = string.Empty; - if (haveConfiguredMainBranch) - { - _reporter.Detail($"Looking for main branch on remote '{origin}' (configured value is '{configuredMainBranch}')..."); - configuredValue = $"{origin}/{configuredMainBranch}"; - } - else - { - _reporter.Detail($"Looking for main branch on remote '{origin}' (no configured value)..."); - } - - foreach (var branch in _repository.Branches.Select(static x => x.FriendlyName)) - { - if (haveConfiguredMainBranch && branch == configuredValue) - { - _reporter.Detail($" '{branch}' <-- configured value"); - mainBranchFound = true; - } - else - { - _reporter.Detail($" '{branch}'"); - if (branch == mainValue) - { - mainFound = true; - } - else if (branch == masterValue) - { - masterFound = true; - } - } - } - - var mainBranch = mainBranchFound ? configuredMainBranch - : mainFound ? "main" - : masterFound ? "master" - : null; - - if (mainBranch is null) - { - _reporter.Detail($"Main branch not found on remote '{origin}'."); - return string.Empty; - } - - _reporter.Detail($"Main branch '{mainBranch}' found on remote '{origin}'."); - return mainBranch; - } } diff --git a/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs b/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs index 0a059e2..e42a67c 100644 --- a/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs +++ b/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs @@ -170,7 +170,7 @@ public async Task PublishReleaseAsync(Release release, string targetCommitish) }; var generateNotesResponse = await client.Repository.Release.GenerateReleaseNotes(RepositoryOwner, RepositoryName, releaseNotesRequest).ConfigureAwait(false); - var body = $"We also have a [human-curated changelog]({GetFileUrl("CHANGELOG.md", _git.MainBranch)}).\n\n---\n\n" + var body = $"We also have a [human-curated changelog]({GetFileUrl("CHANGELOG.md", _git.CurrentBranch)}).\n\n---\n\n" + generateNotesResponse.Body; _reporter.Info($"Publishing the previously created release as {tag} (target {targetCommitish})..."); diff --git a/src/Buildvana.Tool/Subcommands/GlobalSettings.cs b/src/Buildvana.Tool/Subcommands/GlobalSettings.cs index f92fedd..50fffe2 100644 --- a/src/Buildvana.Tool/Subcommands/GlobalSettings.cs +++ b/src/Buildvana.Tool/Subcommands/GlobalSettings.cs @@ -13,7 +13,6 @@ namespace Buildvana.Tool.Subcommands; /// reflected by the help renderer. /// /// The raw --verbosity / -v value, or if none was passed. -/// The raw --main-branch value, or if none was passed. /// Whether --color was passed. /// Whether --no-color was passed. /// Whether --nologo was passed. @@ -23,9 +22,6 @@ internal sealed record GlobalSettings( [property: BvOption("-v|--verbosity ")] [property: Description("Logging verbosity. One of: quiet, minimal, normal, detailed, diagnostic. Defaults to normal.")] string? Verbosity, - [property: BvOption("--main-branch ")] - [property: Description("Name of the repository's main branch. Defaults to 'main'.")] - string? MainBranch, [property: BvOption("--color")] [property: Description("Force ANSI color output even when not connected to a TTY.")] bool Color, diff --git a/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs b/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs index 0cc8d11..184d939 100644 --- a/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs +++ b/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs @@ -9,6 +9,7 @@ using System.Threading; using System.Threading.Tasks; using Buildvana.Core; +using Buildvana.Core.Configuration; using Buildvana.Core.ConsoleOutput; using Buildvana.Core.HomeDirectory; using Buildvana.Core.Json; @@ -131,37 +132,47 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) } } - // Update changelog only on non-prerelease, unless forced + // Update changelog according to the configured policy (none | stable | all). var changelogUpdated = false; + var changelogUpdates = settings.ResolveChangelogUpdates(); + var shouldUpdateChangelog = changelogUpdates != ChangelogUpdates.None + && (changelogUpdates == ChangelogUpdates.All || !version.IsPrerelease); if (!changelog.Exists) { reporter.Info($"Changelog update skipped: {ChangelogService.FileName} not found."); } - else if (!version.IsPrerelease || settings.ResolveUnstableChangelog()) + else if (!shouldUpdateChangelog) { - if (settings.ResolveRequireChangelog()) + var reason = changelogUpdates == ChangelogUpdates.None + ? "changelog updates are disabled (release.changelogUpdates is 'none')." + : "not needed on prerelease."; + reporter.Info($"Changelog update skipped: {reason}"); + } + else + { + // An empty "Unreleased changes" section is substituted from release.emptyChangelog when configured; + // otherwise the release fails. + string? emptyChangelogSubstitute = null; + if (changelog.HasUnreleasedChanges()) { - BuildFailedException.ThrowIfNot( - changelog.HasUnreleasedChanges(), - "Changelog check failed: the \"Unreleased changes\" section is empty or only contains sub-section headings."); - reporter.Info("Changelog check successful: the \"Unreleased changes\" section is not empty."); } else { - reporter.Info("Changelog check skipped: option 'requireChangelog' is false."); + emptyChangelogSubstitute = settings.ResolveEmptyChangelog(); + BuildFailedException.ThrowIfNot( + emptyChangelogSubstitute is not null, + "Changelog check failed: the \"Unreleased changes\" section is empty or only contains sub-section headings, and no substitute text is configured (release.emptyChangelog)."); + + reporter.Info("Changelog \"Unreleased changes\" section is empty; substituting the configured release.emptyChangelog text."); } // Update the changelog and commit the change before building. // This ensures that the Git height is up to date when computing a version for the build artifacts. - changelog.PrepareForRelease(); + changelog.PrepareForRelease(emptyChangelogSubstitute); release.UpdateRepository(ChangelogService.FileName); changelogUpdated = true; } - else - { - reporter.Info("Changelog update skipped: not needed on prerelease."); - } // At this point we know what the actual published version will be. // Time for a final consistency check. @@ -269,9 +280,9 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) { reporter.Info("Documentation generation skipped: not needed on prerelease."); } - else if (git.CurrentBranch != git.MainBranch) + else if (!settings.MatchesDocsBranch(git.CurrentBranch)) { - reporter.Info($"Documentation generation skipped: releasing from '{git.CurrentBranch}', not '{git.MainBranch}'."); + reporter.Info($"Documentation generation skipped: branch '{git.CurrentBranch}' does not match release.generateDocsFrom."); } else { diff --git a/src/Buildvana.Tool/Subcommands/ReleaseSettings.cs b/src/Buildvana.Tool/Subcommands/ReleaseSettings.cs index 89f5d21..97d7d77 100644 --- a/src/Buildvana.Tool/Subcommands/ReleaseSettings.cs +++ b/src/Buildvana.Tool/Subcommands/ReleaseSettings.cs @@ -4,23 +4,41 @@ using System; using System.Collections.Generic; using System.ComponentModel; +using System.Text.RegularExpressions; using Buildvana.Core; +using Buildvana.Core.Configuration; using Buildvana.Tool.CommandLine; +using Buildvana.Tool.Services; using Buildvana.Tool.Services.Versioning; +using CommunityToolkit.Diagnostics; namespace Buildvana.Tool.Subcommands; /// -/// Options for the release command, parsed from the command-line option tokens by . +/// Options for the release command. The flag values are parsed from the command-line option tokens by +/// ; each Resolve* method then merges the flag with the release configuration +/// section and a hardcoded default (flag → config → default). /// Decorated with / for the help renderer. /// internal sealed class ReleaseSettings { + private static readonly IReadOnlyList DefaultGenerateDocsFrom = ["^main$", "^master$"]; + private static readonly TimeSpan RegexMatchTimeout = TimeSpan.FromSeconds(1); + + private readonly ReleaseConfig? _config; + private readonly DotNetSettings _dotNetSettings; + + private ReleaseSettings(ReleaseConfig? config, DotNetSettings dotNetSettings) + { + _config = config; + _dotNetSettings = dotNetSettings; + } + /// /// Gets the MSBuild configuration to build. /// [BvOption("-c|--configuration ")] - [Description("MSBuild configuration to build. Defaults to 'Release'.")] + [Description("MSBuild configuration to build. Defaults to the configured value, or 'Release'.")] public string? Configuration { get; init; } /// @@ -44,20 +62,6 @@ internal sealed class ReleaseSettings [Description("Check the public API when computing version-spec changes. Defaults to true.")] public bool? CheckPublicApi { get; init; } - /// - /// Gets a value indicating whether the changelog is updated on unstable (prerelease) versions. - /// - [BvOption("--unstable-changelog ")] - [Description("Update the changelog on unstable (prerelease) versions. Defaults to false.")] - public bool? UnstableChangelog { get; init; } - - /// - /// Gets a value indicating whether the build is failed if the 'Unreleased changes' section is empty. - /// - [BvOption("--require-changelog ")] - [Description("Fail the build if the 'Unreleased changes' section is empty. Defaults to true.")] - public bool? RequireChangelog { get; init; } - /// /// Gets a value indicating whether in-tree references to packages produced by this release are updated. /// @@ -67,21 +71,24 @@ internal sealed class ReleaseSettings /// /// Parses the command's option tokens into a , rejecting any option the command - /// does not recognize. + /// does not recognize, and binds the configuration sources consulted by the Resolve* methods. /// /// The option tokens for the release command (from CommandParameters.Options). + /// The Buildvana configuration whose release section layers between the flags and the defaults. + /// The resolved dotnet settings, providing the fallback build configuration. /// The parsed settings. /// An option value is invalid, or an unrecognized option was given. - public static ReleaseSettings Parse(IReadOnlyList options) + public static ReleaseSettings Parse(IReadOnlyList options, BuildvanaConfig config, DotNetSettings dotNetSettings) { + Guard.IsNotNull(options); + Guard.IsNotNull(config); + Guard.IsNotNull(dotNetSettings); var reader = new CliOptionReader(options); - var settings = new ReleaseSettings + var settings = new ReleaseSettings(config.Release, dotNetSettings) { Configuration = reader.ReadValue("--configuration", "-c"), Bump = reader.ReadValue("--bump"), CheckPublicApi = ParseBool(reader.ReadValue("--check-public-api"), "--check-public-api"), - UnstableChangelog = ParseBool(reader.ReadValue("--unstable-changelog"), "--unstable-changelog"), - RequireChangelog = ParseBool(reader.ReadValue("--require-changelog"), "--require-changelog"), Dogfood = ParseBool(reader.ReadValue("--dogfood"), "--dogfood"), }; @@ -111,29 +118,75 @@ public VersionSpecChange ResolveBump() } /// - /// Gets the resolved MSBuild configuration: if set, otherwise "Release". + /// Gets the resolved MSBuild configuration: (the --configuration flag) if set, + /// otherwise release.configuration, otherwise the configured dotnet default. + /// + public string ResolveConfiguration() => Configuration ?? _config?.Configuration ?? _dotNetSettings.Configuration; + + /// + /// Returns if set, otherwise release.checkPublicApi, otherwise . /// - public string ResolveConfiguration() => Configuration ?? "Release"; + public bool ResolveCheckPublicApi() => CheckPublicApi ?? _config?.CheckPublicApi ?? true; /// - /// Returns if set, otherwise . + /// Returns the configured changelog-update policy (release.changelogUpdates), or + /// when unset. /// - public bool ResolveCheckPublicApi() => CheckPublicApi ?? true; + public ChangelogUpdates ResolveChangelogUpdates() => _config?.ChangelogUpdates ?? ChangelogUpdates.Stable; /// - /// Returns if set, otherwise . + /// Returns the text substituted for an empty changelog (release.emptyChangelog), or + /// when unset (in which case an empty changelog fails the release). /// - public bool ResolveUnstableChangelog() => UnstableChangelog ?? false; + public string? ResolveEmptyChangelog() => _config?.EmptyChangelog; /// - /// Returns if set, otherwise . + /// Returns if set, otherwise release.dogfood, otherwise . /// - public bool ResolveRequireChangelog() => RequireChangelog ?? true; + public bool ResolveDogfood() => Dogfood ?? _config?.Dogfood ?? true; /// - /// Returns if set, otherwise . + /// Determines whether documentation is generated when releasing from , by matching it + /// against the configured release.generateDocsFrom regular expressions (default ^main$/^master$). /// - public bool ResolveDogfood() => Dogfood ?? true; + /// The short name of the branch the release is created from. + /// if matches at least one pattern; otherwise, . + /// A configured pattern is not a valid regular expression, or matching timed out. + public bool MatchesDocsBranch(string branch) + { + Guard.IsNotNull(branch); + if (string.IsNullOrEmpty(branch)) + { + return false; + } + + var patterns = _config?.GenerateDocsFrom ?? DefaultGenerateDocsFrom; + foreach (var pattern in patterns) + { + if (IsMatch(pattern, branch)) + { + return true; + } + } + + return false; + } + + private static bool IsMatch(string pattern, string branch) + { + try + { + return Regex.IsMatch(branch, pattern, RegexOptions.CultureInvariant, RegexMatchTimeout); + } + catch (ArgumentException ex) + { + throw new BuildFailedException($"Invalid regular expression '{pattern}' in release.generateDocsFrom: {ex.Message}"); + } + catch (RegexMatchTimeoutException) + { + throw new BuildFailedException($"Regular expression '{pattern}' in release.generateDocsFrom timed out matching branch '{branch}'."); + } + } private static bool? ParseBool(string? raw, string optionName) { diff --git a/tests/Buildvana.Tool.Tests/CliArgSplitterTests.cs b/tests/Buildvana.Tool.Tests/CliArgSplitterTests.cs index f54c774..5cb68aa 100644 --- a/tests/Buildvana.Tool.Tests/CliArgSplitterTests.cs +++ b/tests/Buildvana.Tool.Tests/CliArgSplitterTests.cs @@ -37,14 +37,6 @@ public async Task Split_StripsValueGlobalBeforeSubcommandDetection() await Assert.That(result.OptionTokens.Count).IsEqualTo(0); } - [Test] - public async Task Split_StripsMainBranchGlobalBeforeSubcommand() - { - var result = CliArgSplitter.Split(["--main-branch", "develop", "release"]); - await Assert.That(result.Globals.MainBranch).IsEqualTo("develop"); - await Assert.That(result.Subcommand).IsEqualTo("release"); - } - [Test] public async Task Split_StripsFlagGlobalsOnEitherSideOfSubcommand() { diff --git a/tests/Buildvana.Tool.Tests/ReleaseSettingsTests.cs b/tests/Buildvana.Tool.Tests/ReleaseSettingsTests.cs index c82eec0..84df245 100644 --- a/tests/Buildvana.Tool.Tests/ReleaseSettingsTests.cs +++ b/tests/Buildvana.Tool.Tests/ReleaseSettingsTests.cs @@ -2,6 +2,8 @@ // See the LICENSE file in the project root for full license information. using Buildvana.Core; +using Buildvana.Core.Configuration; +using Buildvana.Tool.Services; using Buildvana.Tool.Services.Versioning; using Buildvana.Tool.Subcommands; @@ -10,39 +12,116 @@ internal sealed class ReleaseSettingsTests [Test] public async Task Parse_Defaults_ResolveToExpectedValues() { - var settings = ReleaseSettings.Parse([]); + var settings = Parse([]); await Assert.That(settings.ResolveConfiguration()).IsEqualTo("Release"); await Assert.That(settings.ResolveBump()).IsEqualTo(VersionSpecChange.None); await Assert.That(settings.ResolveCheckPublicApi()).IsTrue(); - await Assert.That(settings.ResolveUnstableChangelog()).IsFalse(); - await Assert.That(settings.ResolveRequireChangelog()).IsTrue(); + await Assert.That(settings.ResolveChangelogUpdates()).IsEqualTo(ChangelogUpdates.Stable); + await Assert.That(settings.ResolveEmptyChangelog()).IsNull(); await Assert.That(settings.ResolveDogfood()).IsTrue(); } [Test] public async Task Parse_ReadsConfiguration_ShortAndInlineForms() { - await Assert.That(ReleaseSettings.Parse(["-c", "Debug"]).ResolveConfiguration()).IsEqualTo("Debug"); - await Assert.That(ReleaseSettings.Parse(["--configuration=Debug"]).ResolveConfiguration()).IsEqualTo("Debug"); + await Assert.That(Parse(["-c", "Debug"]).ResolveConfiguration()).IsEqualTo("Debug"); + await Assert.That(Parse(["--configuration=Debug"]).ResolveConfiguration()).IsEqualTo("Debug"); + } + + [Test] + public async Task ResolveConfiguration_FollowsFlagOverReleaseOverDotNetChain() + { + var config = new BuildvanaConfig + { + DotNet = new() { Configuration = "DotNetConfig" }, + Release = new() { Configuration = "ReleaseConfig" }, + }; + + // Flag wins over both config sections. + await Assert.That(Parse(["-c", "FlagConfig"], config).ResolveConfiguration()).IsEqualTo("FlagConfig"); + + // With no flag, release.configuration wins over dotnet.configuration. + await Assert.That(Parse([], config).ResolveConfiguration()).IsEqualTo("ReleaseConfig"); + + // With neither flag nor release.configuration, dotnet.configuration is used. + var dotNetOnly = new BuildvanaConfig { DotNet = new() { Configuration = "DotNetConfig" } }; + await Assert.That(Parse([], dotNetOnly).ResolveConfiguration()).IsEqualTo("DotNetConfig"); + } + + [Test] + public async Task Resolve_ReadsReleaseConfig_WhenFlagsAbsent() + { + var config = new BuildvanaConfig + { + Release = new() + { + CheckPublicApi = false, + Dogfood = false, + ChangelogUpdates = ChangelogUpdates.All, + EmptyChangelog = "Nothing to see here.", + }, + }; + + var settings = Parse([], config); + await Assert.That(settings.ResolveCheckPublicApi()).IsFalse(); + await Assert.That(settings.ResolveDogfood()).IsFalse(); + await Assert.That(settings.ResolveChangelogUpdates()).IsEqualTo(ChangelogUpdates.All); + await Assert.That(settings.ResolveEmptyChangelog()).IsEqualTo("Nothing to see here."); + } + + [Test] + public async Task Resolve_FlagsWin_OverReleaseConfig() + { + var config = new BuildvanaConfig { Release = new() { CheckPublicApi = false, Dogfood = false } }; + var settings = Parse(["--check-public-api", "true", "--dogfood=true"], config); + await Assert.That(settings.ResolveCheckPublicApi()).IsTrue(); + await Assert.That(settings.ResolveDogfood()).IsTrue(); + } + + [Test] + public async Task MatchesDocsBranch_DefaultsToMainAndMaster() + { + var settings = Parse([]); + await Assert.That(settings.MatchesDocsBranch("main")).IsTrue(); + await Assert.That(settings.MatchesDocsBranch("master")).IsTrue(); + await Assert.That(settings.MatchesDocsBranch("develop")).IsFalse(); + await Assert.That(settings.MatchesDocsBranch(string.Empty)).IsFalse(); + } + + [Test] + public async Task MatchesDocsBranch_UsesConfiguredPatterns() + { + var config = new BuildvanaConfig { Release = new() { GenerateDocsFrom = ["^release/.+$"] } }; + var settings = Parse([], config); + await Assert.That(settings.MatchesDocsBranch("release/2.0")).IsTrue(); + await Assert.That(settings.MatchesDocsBranch("main")).IsFalse(); + } + + [Test] + public async Task MatchesDocsBranch_Throws_OnInvalidPattern() + { + var config = new BuildvanaConfig { Release = new() { GenerateDocsFrom = ["["] } }; + var settings = Parse([], config); + await Assert.That(() => settings.MatchesDocsBranch("main")).Throws(); } [Test] public async Task Parse_ReadsBumpEnum() { - await Assert.That(ReleaseSettings.Parse(["--bump", "minor"]).ResolveBump()).IsEqualTo(VersionSpecChange.Minor); + await Assert.That(Parse(["--bump", "minor"]).ResolveBump()).IsEqualTo(VersionSpecChange.Minor); } [Test] public async Task ResolveBump_Throws_OnInvalidValue() { - var settings = ReleaseSettings.Parse(["--bump", "bogus"]); + var settings = Parse(["--bump", "bogus"]); await Assert.That(settings.ResolveBump).Throws(); } [Test] public async Task Parse_ReadsBoolOptions_SpaceAndInlineForms() { - var settings = ReleaseSettings.Parse(["--check-public-api", "false", "--dogfood=false"]); + var settings = Parse(["--check-public-api", "false", "--dogfood=false"]); await Assert.That(settings.ResolveCheckPublicApi()).IsFalse(); await Assert.That(settings.ResolveDogfood()).IsFalse(); } @@ -50,12 +129,18 @@ public async Task Parse_ReadsBoolOptions_SpaceAndInlineForms() [Test] public async Task Parse_Throws_OnInvalidBool() { - await Assert.That(() => ReleaseSettings.Parse(["--dogfood", "maybe"])).Throws(); + await Assert.That(() => Parse(["--dogfood", "maybe"])).Throws(); } [Test] public async Task Parse_Throws_OnUnknownOption() { - await Assert.That(() => ReleaseSettings.Parse(["--bogus"])).Throws(); + await Assert.That(() => Parse(["--bogus"])).Throws(); + } + + private static ReleaseSettings Parse(string[] options, BuildvanaConfig? config = null) + { + config ??= new BuildvanaConfig(); + return ReleaseSettings.Parse(options, config, new DotNetSettings(config)); } } diff --git a/tests/Buildvana.Tool.Tests/SettingsHelpReflectionTests.cs b/tests/Buildvana.Tool.Tests/SettingsHelpReflectionTests.cs index c9c1995..08d06cc 100644 --- a/tests/Buildvana.Tool.Tests/SettingsHelpReflectionTests.cs +++ b/tests/Buildvana.Tool.Tests/SettingsHelpReflectionTests.cs @@ -11,14 +11,14 @@ internal sealed class SettingsHelpReflectionTests public async Task GlobalSettings_ExposesOptionsInHelpOrder() { var names = OptionLongNames(typeof(GlobalSettings)); - await Assert.That(names).IsEqualTo("--verbosity,--main-branch,--color,--no-color,--nologo,--version"); + await Assert.That(names).IsEqualTo("--verbosity,--color,--no-color,--nologo,--version"); } [Test] public async Task ReleaseSettings_ExposesOptionsInHelpOrder() { var names = OptionLongNames(typeof(ReleaseSettings)); - await Assert.That(names).IsEqualTo("--configuration,--bump,--check-public-api,--unstable-changelog,--require-changelog,--dogfood"); + await Assert.That(names).IsEqualTo("--configuration,--bump,--check-public-api,--dogfood"); } private static string OptionLongNames(Type type) From 1b028406c80bb4940207bc5271fbb58f9169035f Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Mon, 1 Jun 2026 09:03:33 +0200 Subject: [PATCH 2/6] Add clarification in changelog An invalid configuration file will prevent execution of _any_ `bv` subcommand, even those that do not depend on configuration. This has been flagged as a behavioral change during review. It is a deliberate decision and stays, but a clarification was due. --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b075e9..59625f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `bv --version` prints the tool's informational version and exits without running a command and without printing the startup logo. - `bv` root help (`bv --help`) now shows a `GLOBAL OPTIONS:` section listing the options every subcommand inherits (`--verbosity`/`-v`, `--color`, `--no-color`, `--nologo`, `--version`). These options are now position-independent (accepted before or after the subcommand name) and case-insensitive, matching the rest of `bv`'s option surface. - Commands that forward extra arguments to `dotnet` (`restore`, `build`, `test`, `pack`) are marked as such in `bv`'s root help, and their per-command help (`bv --help`) includes a `FORWARDED ARGUMENTS` section. -- Buildvana now recognizes a repository-root configuration file, `buildvana.json` (or its commented variant `buildvana.jsonc`). It is discovered, parsed, validated, and exposed to `bv`; the settings it currently drives are listed below, and more are wired in over subsequent releases. A committed JSON schema (`schemas/buildvana.schema.json`) is generated from the typed model, so editors can validate and document the file; unknown keys, an invalid file, or the presence of both `buildvana.json` and `buildvana.jsonc` in the same directory are reported as errors. Both `bv` and the SDK now treat a `buildvana.json`/`buildvana.jsonc` file as a home-directory marker, alongside `.buildvana-home` and Git markers; home-directory discovery now stops at the nearest directory (the starting directory included) that contains any marker. +- Buildvana now recognizes a repository-root configuration file, `buildvana.json` (or its commented variant `buildvana.jsonc`). It is discovered, parsed, validated, and exposed to `bv`; the settings it currently drives are listed below, and more are wired in over subsequent releases. A committed JSON schema (`schemas/buildvana.schema.json`) is generated from the typed model, so editors can validate and document the file; unknown keys, an invalid file, or the presence of both `buildvana.json` and `buildvana.jsonc` in the same directory are reported as errors and will prevent `bv` from executing _any_ subcommand, even those that are not driven by any configuration setting (e.g., `clean`). +- Both `bv` and the SDK now treat a `buildvana.json`/`buildvana.jsonc` file as a home-directory marker, alongside `.buildvana-home` and Git markers; home-directory discovery now stops at the nearest directory (the starting directory included) that contains any marker. - `buildvana.json` now drives several build and release settings that were previously CLI-only or hardcoded. Each resolves as CLI flag (where one exists) → `buildvana.json` → built-in default: - the default build configuration (`dotnet.configuration`, default `Release`), used by `bv restore`/`build`/`test`/`pack` and as the base of `bv release`'s configuration chain (`--configuration` → `release.configuration` → `dotnet.configuration`); - extra arguments and environment variables for each `dotnet` invocation: `dotnet.all` (applied to every invocation) merged with the per-command `dotnet.restore`/`dotnet.build`/`dotnet.test`/`dotnet.pack`/`dotnet.nugetPush`, each carrying `args` and `env`. Arguments are appended in the order base → `dotnet.all` → per-command → forwarded command-line arguments (so a `--` argument still wins); environment variables apply `dotnet.all` then the per-command entries; From 5bdc383bffa76feeac2d01483e73ce6c1f937eb7 Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Mon, 1 Jun 2026 16:29:24 +0200 Subject: [PATCH 3/6] Simplify DotNetService.RunDotNetAsync to caller-controlled mechanics Replace the InvocationKind enum (Normal/Informational/Internal) with direct, mechanical parameters on RunDotNetAsync, so each call site spells out exactly how it controls the invocation: - args + tiers (IReadOnlyList, folded left to right) + commandLineArgs + trailingArgs, applied in that order. trailingArgs win over everything, so ContinuousIntegrationBuild moves there and stays unoverridable by forwarded/configured arguments. - appendVerbosity and an OutputStreaming struct (Disabled / Unconditional / AtVerbosity) replace the streamOutput/streamVerbosity pair, making the illegal "disabled but with a verbosity" state unrepresentable. - The internal MSBuild probe passes empty tiers instead of relying on a null sentinel; BuildEnvironment folds into RunDotNetAsync. Remove the now-stale partial on DotNetService where only the enum needed it, then re-add it for the OutputStreaming nested struct. Co-Authored-By: Claude Opus 4.8 --- .../Services/DotNetService.InvocationKind.cs | 26 --- .../Services/DotNetService.OutputStreaming.cs | 50 ++++++ src/Buildvana.Tool/Services/DotNetService.cs | 149 ++++++++++-------- 3 files changed, 135 insertions(+), 90 deletions(-) delete mode 100644 src/Buildvana.Tool/Services/DotNetService.InvocationKind.cs create mode 100644 src/Buildvana.Tool/Services/DotNetService.OutputStreaming.cs diff --git a/src/Buildvana.Tool/Services/DotNetService.InvocationKind.cs b/src/Buildvana.Tool/Services/DotNetService.InvocationKind.cs deleted file mode 100644 index 8601827..0000000 --- a/src/Buildvana.Tool/Services/DotNetService.InvocationKind.cs +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright (C) Tenacom and Contributors. Licensed under the MIT license. -// See the LICENSE file in the project root for full license information. - -namespace Buildvana.Tool.Services; - -partial class DotNetService -{ - private enum InvocationKind - { - /// - /// A normal invocation: `dotnet` accepts the `--verbosity` argument and the user is interested in the output. - /// - Normal, - - /// - /// An informational invocation: the user is interested in the output, but `dotnet` does not accept the `--verbosity` argument. - /// `dotnet` output is streamed at `Normal` verbosity. - /// - Informational, - - /// - /// An internal invocation: the user is not interested in the output, and the `--verbosity` argument, if any, is already set appropriately. - /// - Internal, - } -} diff --git a/src/Buildvana.Tool/Services/DotNetService.OutputStreaming.cs b/src/Buildvana.Tool/Services/DotNetService.OutputStreaming.cs new file mode 100644 index 0000000..aaacd3e --- /dev/null +++ b/src/Buildvana.Tool/Services/DotNetService.OutputStreaming.cs @@ -0,0 +1,50 @@ +// Copyright (C) Tenacom and Contributors. Licensed under the MIT license. +// See the LICENSE file in the project root for full license information. + +using Buildvana.Core.ConsoleOutput; + +namespace Buildvana.Tool.Services; + +partial class DotNetService +{ + /// + /// Represents the output streaming configuration for a dotnet invocation. + /// + private readonly struct OutputStreaming + { + /// + /// Determines whether output streaming is enabled. + /// + public readonly bool Enabled; + + /// + /// The minimum verbosity level at which dotnet output will be streamed. + /// If this field is , dotnet output will be streamed regardless of the current verbosity level. + /// + /// + /// This field will be ignored by if is . + /// + public readonly Verbosity? Verbosity; + + private OutputStreaming(bool enabled, Verbosity? verbosity) + { + Enabled = enabled; + Verbosity = verbosity; + } + + /// + /// Gets an instance representing disabled output streaming. + /// + public static OutputStreaming Disabled => new(false, null); + + /// + /// Gets an instance representing enabled output streaming unrestrained by verbosity. + /// + public static OutputStreaming Unconditional => new(true, null); + + /// + /// Gets an instance representing enabled output streaming at the specified verbosity level. + /// + public static OutputStreaming AtVerbosity(Verbosity verbosity) => new(true, verbosity); + } +} diff --git a/src/Buildvana.Tool/Services/DotNetService.cs b/src/Buildvana.Tool/Services/DotNetService.cs index 59b5721..1cf9332 100644 --- a/src/Buildvana.Tool/Services/DotNetService.cs +++ b/src/Buildvana.Tool/Services/DotNetService.cs @@ -3,11 +3,11 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Linq; using System.Threading; using System.Threading.Tasks; using Buildvana.Core.ConsoleOutput; +using Buildvana.Core.Process; using Buildvana.Tool.Configuration; using Buildvana.Tool.Infrastructure; using Buildvana.Tool.Services.ServerAdapters; @@ -16,9 +16,6 @@ using Buildvana.Tool.Utilities; using CommunityToolkit.Diagnostics; -using IProcessRunner = Buildvana.Core.Process.IProcessRunner; -using ProcessResult = Buildvana.Core.Process.ProcessResult; - namespace Buildvana.Tool.Services; /// @@ -82,10 +79,16 @@ public Task RestoreSolutionAsync(SolutionContext solution, IReadOnlyList solution.SolutionPath, "--disable-parallel", "-nologo", - ContinuousIntegrationBuildArg(asMSBuildPassthrough: true), ]; - return RunDotNetAsync(args, _settings.Restore, forwardedArgs, cancellationToken: cancellationToken); + return RunDotNetAsync( + args, + tiers: [_settings.All, _settings.Restore], + commandLineArgs: forwardedArgs, + trailingArgs: [ContinuousIntegrationBuildArg(asMSBuildPassthrough: true)], + appendVerbosity: true, + outputStreaming: OutputStreaming.Unconditional, + cancellationToken: cancellationToken); } /// @@ -109,10 +112,16 @@ public Task BuildSolutionAsync(SolutionContext solution, string configuration, I "-nologo", $"-p:Configuration={configuration}", .. restore ? Array.Empty() : ["--no-restore"], - ContinuousIntegrationBuildArg(asMSBuildPassthrough: true), ]; - return RunDotNetAsync(args, _settings.Build, forwardedArgs, cancellationToken: cancellationToken); + return RunDotNetAsync( + args, + tiers: [_settings.All, _settings.Build], + commandLineArgs: forwardedArgs, + trailingArgs: [ContinuousIntegrationBuildArg(asMSBuildPassthrough: true)], + appendVerbosity: true, + outputStreaming: OutputStreaming.Unconditional, + cancellationToken: cancellationToken); } /// @@ -141,7 +150,14 @@ public async Task TestSolutionAsync(SolutionContext solution, string configurati // bv-internal MSBuild evaluation: do not forward the user's arguments here, as they may be // test-application options that `dotnet msbuild` would reject. string[] probeArgs = ["msbuild", projectPath, "-nologo", "-getProperty:IsTestingPlatformApplication"]; - var probe = await RunDotNetAsync(probeArgs, invocationKind: InvocationKind.Internal, cancellationToken: cancellationToken).ConfigureAwait(false); + var probe = await RunDotNetAsync( + probeArgs, + tiers: [], + commandLineArgs: [], + trailingArgs: [], + appendVerbosity: false, + outputStreaming: OutputStreaming.Disabled, + cancellationToken: cancellationToken).ConfigureAwait(false); if (string.Equals(probe.StandardOutput.Trim(), "true", StringComparison.OrdinalIgnoreCase)) { @@ -172,10 +188,16 @@ public async Task TestSolutionAsync(SolutionContext solution, string configurati CommonPaths.TestResults, "--output", _reporter.IsVerbosityAtLeast(Verbosity.Detailed) ? "Detailed" : "Normal", - ContinuousIntegrationBuildArg(asMSBuildPassthrough: false), ]; - await RunDotNetAsync(args, _settings.Test, forwardedArgs, cancellationToken: cancellationToken).ConfigureAwait(false); + await RunDotNetAsync( + args, + tiers: [_settings.All, _settings.Test], + commandLineArgs: forwardedArgs, + trailingArgs: [ContinuousIntegrationBuildArg(asMSBuildPassthrough: false)], + appendVerbosity: true, + outputStreaming: OutputStreaming.Unconditional, + cancellationToken: cancellationToken).ConfigureAwait(false); } /// @@ -201,10 +223,16 @@ public Task PackSolutionAsync(SolutionContext solution, string configuration, IR $"-p:Configuration={configuration}", .. restore ? Array.Empty() : ["--no-restore"], .. build ? Array.Empty() : ["--no-build"], - ContinuousIntegrationBuildArg(asMSBuildPassthrough: true), ]; - return RunDotNetAsync(args, _settings.Pack, forwardedArgs, cancellationToken: cancellationToken); + return RunDotNetAsync( + args, + tiers: [_settings.All, _settings.Pack], + commandLineArgs: forwardedArgs, + trailingArgs: [ContinuousIntegrationBuildArg(asMSBuildPassthrough: true)], + appendVerbosity: true, + outputStreaming: OutputStreaming.Unconditional, + cancellationToken: cancellationToken); } /// @@ -244,7 +272,14 @@ .. _reporter.IsVerbosityAtLeast(Verbosity.Diagnostic) ? ["-d"] : Array.Empty /// Runs a dotnet command, forwarding the output to the reporter according to the current verbosity. /// - /// The base arguments to pass to dotnet (everything the calling method constructs, - /// including the ContinuousIntegrationBuild argument). The verbosity argument is appended automatically - /// according to the current verbosity and invocation kind. - /// The per-command configured settings, whose arguments and environment variables are - /// merged with the settings and applied to the invocation. Pass - /// for bv-internal invocations that must not receive user-configured arguments - /// (e.g. MSBuild property probes). + /// The base arguments to pass to dotnet, constructed by the calling method. + /// The configured settings to apply, folded left to right: each tier's arguments are + /// appended (after ) and its environment variables are layered on top of the previous + /// tiers', so a later tier overrides an earlier one with the same variable name. Pass an empty list for + /// bv-internal invocations that must not receive user-configured arguments (e.g. MSBuild property probes). /// Extra arguments forwarded verbatim from the command line, appended after the - /// configured arguments so they take precedence. - /// The kind of invocation, which determines how dotnet verbosity and output streaming are handled. + /// configured arguments so they take precedence over them. + /// Arguments appended after everything else, so they cannot be overridden by + /// configured or command-line arguments (e.g. the ContinuousIntegrationBuild property). + /// to append a --verbosity argument reflecting the + /// current verbosity; for commands that do not accept it (e.g. dotnet nuget push) + /// or already carry it. + /// The output streaming configuration for the dotnet invocation. /// A token that, when signalled, terminates the spawned dotnet child process. /// A representing the ongoing operation, with a result describing child process outcome. private Task RunDotNetAsync( IReadOnlyList args, - DotNetInvocationSettings? invocation = null, - IReadOnlyList? commandLineArgs = null, - InvocationKind invocationKind = InvocationKind.Normal, + IReadOnlyList tiers, + IReadOnlyList commandLineArgs, + IReadOnlyList trailingArgs, + bool appendVerbosity, + OutputStreaming outputStreaming, CancellationToken cancellationToken = default) { - var (appendVerbosity, streamOutput, streamVerbosity) = invocationKind switch { - InvocationKind.Normal => (AppendVerbosity: true, StreamOutput: true, StreamVerbosity: null as Verbosity?), - InvocationKind.Informational => (AppendVerbosity: false, StreamOutput: true, StreamVerbosity: Verbosity.Normal), - InvocationKind.Internal => (AppendVerbosity: false, StreamOutput: false, StreamVerbosity: null), - _ => throw new UnreachableException(), - }; + // Fold the tiers in a single pass: append each tier's arguments, layer its environment variables on top of + // the previous tiers' (so a later tier overrides an earlier one with the same name). A null result environment + // leaves the child environment unchanged. + var foldedArgs = new List(args); + Dictionary? environment = null; + foreach (var tier in tiers) + { + foldedArgs.AddRange(tier.Args); + foreach (var (key, value) in tier.Env) + { + environment ??= new Dictionary(StringComparer.Ordinal); + environment[key] = value; + } + } - // Order: base args (constructed by the caller) → dotnet.all args → per-command args → command-line args. - // Command-line args come last so they override configured arguments. - IReadOnlyList configuredArgs = invocation is null ? [] : [.. _settings.All.Args, .. invocation.Args]; - string[] finalArgs = [.. args, .. configuredArgs, .. commandLineArgs ?? []]; - var environment = invocation is null ? null : BuildEnvironment(invocation); + // Order: base args → tier args → command-line args (override configured args) → trailing args (override + // everything, so a forwarded argument cannot countermand them). + string[] finalArgs = [.. foldedArgs, .. commandLineArgs, .. trailingArgs]; return _processRunner.RunAsync( DotNetMuxer, appendVerbosity ? finalArgs.Append($"--verbosity={_reporter.Verbosity}") : finalArgs, environment: environment, - onStdout: streamOutput ? (x) => _reporter.ChildOutput(x, streamVerbosity) : null, - onStderr: streamOutput ? (x) => _reporter.ChildError(x, streamVerbosity) : null, + onStdout: outputStreaming.Enabled ? (x) => _reporter.ChildOutput(x, outputStreaming.Verbosity) : null, + onStderr: outputStreaming.Enabled ? (x) => _reporter.ChildError(x, outputStreaming.Verbosity) : null, cancellationToken: cancellationToken); } - - // Merges environment variables in the order dotnet.all env → per-command env, so a per-command entry overrides - // an "all" entry with the same name. Returns null when there is nothing to apply, leaving the child environment unchanged. - private Dictionary? BuildEnvironment(DotNetInvocationSettings invocation) - { - var all = _settings.All.Env; - var perCommand = invocation.Env; - if (all.Count == 0 && perCommand.Count == 0) - { - return null; - } - - var merged = new Dictionary(all.Count + perCommand.Count, StringComparer.Ordinal); - foreach (var (key, value) in all) - { - merged[key] = value; - } - - foreach (var (key, value) in perCommand) - { - merged[key] = value; - } - - return merged; - } } From 6e48c6ffbcb5fb71a42d7421626c69964bab6aff Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Mon, 1 Jun 2026 16:40:05 +0200 Subject: [PATCH 4/6] Use `ReadOnlyDictionary<,>.Empty` instead of creating a dictionary --- src/Buildvana.Tool/Services/DotNetSettings.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Buildvana.Tool/Services/DotNetSettings.cs b/src/Buildvana.Tool/Services/DotNetSettings.cs index ddf85c4..1d43624 100644 --- a/src/Buildvana.Tool/Services/DotNetSettings.cs +++ b/src/Buildvana.Tool/Services/DotNetSettings.cs @@ -2,6 +2,7 @@ // See the LICENSE file in the project root for full license information. using System.Collections.Generic; +using System.Collections.ObjectModel; using Buildvana.Core.Configuration; using CommunityToolkit.Diagnostics; @@ -59,5 +60,5 @@ public DotNetSettings(BuildvanaConfig config) private static DotNetInvocationSettings Resolve(DotNetInvocationConfig? config) => config is null ? DotNetInvocationSettings.Empty - : new(config.Args ?? [], config.Env ?? new Dictionary()); + : new(config.Args ?? [], config.Env ?? ReadOnlyDictionary.Empty); } From 63ff47d2729a30b5a5457806829345c563695989 Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Mon, 1 Jun 2026 16:43:48 +0200 Subject: [PATCH 5/6] Remove redundant null check --- src/Buildvana.Tool/Subcommands/ReleaseSettings.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Buildvana.Tool/Subcommands/ReleaseSettings.cs b/src/Buildvana.Tool/Subcommands/ReleaseSettings.cs index 97d7d77..64cc686 100644 --- a/src/Buildvana.Tool/Subcommands/ReleaseSettings.cs +++ b/src/Buildvana.Tool/Subcommands/ReleaseSettings.cs @@ -155,7 +155,7 @@ public VersionSpecChange ResolveBump() public bool MatchesDocsBranch(string branch) { Guard.IsNotNull(branch); - if (string.IsNullOrEmpty(branch)) + if (branch.Length == 0) { return false; } From 9b2be715142772f3fcc27ca6dc2ee536aab5be62 Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Mon, 1 Jun 2026 17:36:00 +0200 Subject: [PATCH 6/6] Extract DotNetService arg/env merge into testable helper Pull the tier-folding loop out of the private RunDotNetAsync into an internal static MergeInvocation method, and add unit tests covering the arg ordering and environment layering (tier override, null-value removal, and the no-tiers/no-env cases). Co-Authored-By: Claude Opus 4.8 --- src/Buildvana.Tool/Services/DotNetService.cs | 62 +++++++++---- .../DotNetServiceMergeInvocationTests.cs | 92 +++++++++++++++++++ 2 files changed, 135 insertions(+), 19 deletions(-) create mode 100644 tests/Buildvana.Tool.Tests/DotNetServiceMergeInvocationTests.cs diff --git a/src/Buildvana.Tool/Services/DotNetService.cs b/src/Buildvana.Tool/Services/DotNetService.cs index 1cf9332..c4f31ff 100644 --- a/src/Buildvana.Tool/Services/DotNetService.cs +++ b/src/Buildvana.Tool/Services/DotNetService.cs @@ -285,6 +285,48 @@ await RunDotNetAsync( _reporter.Info($"Pushed {packages.Length} packages to {target.Source}."); } + /// + /// Folds the base arguments, configured tiers, command-line arguments, and trailing arguments into the final + /// argument list and resolved environment for a dotnet invocation. + /// + /// The base arguments constructed by the calling method. + /// The configured settings to apply, folded left to right: each tier's arguments are appended + /// (after ) and its environment variables are layered on top of the previous tiers', so a + /// later tier overrides an earlier one with the same variable name. + /// Extra arguments forwarded verbatim from the command line, appended after the + /// configured arguments so they take precedence over them. + /// Arguments appended after everything else, so they cannot be overridden by configured + /// or command-line arguments. + /// A tuple containing the final argument list and the resolved environment, the latter being + /// when no tier contributes an environment variable (which leaves the child environment + /// unchanged). + internal static (string[] Args, IReadOnlyDictionary? Environment) MergeInvocation( + IReadOnlyList args, + IReadOnlyList tiers, + IReadOnlyList commandLineArgs, + IReadOnlyList trailingArgs) + { + // Fold the tiers in a single pass: append each tier's arguments, layer its environment variables on top of + // the previous tiers' (so a later tier overrides an earlier one with the same name). A null result environment + // leaves the child environment unchanged. + var foldedArgs = new List(args); + Dictionary? environment = null; + foreach (var tier in tiers) + { + foldedArgs.AddRange(tier.Args); + foreach (var (key, value) in tier.Env) + { + environment ??= new Dictionary(StringComparer.Ordinal); + environment[key] = value; + } + } + + // Order: base args → tier args → command-line args (override configured args) → trailing args (override + // everything, so a forwarded argument cannot countermand them). + string[] finalArgs = [.. foldedArgs, .. commandLineArgs, .. trailingArgs]; + return (finalArgs, environment); + } + /// /// Return a parameter string that reflects whether we're running in CI. /// @@ -324,25 +366,7 @@ private Task RunDotNetAsync( OutputStreaming outputStreaming, CancellationToken cancellationToken = default) { - // Fold the tiers in a single pass: append each tier's arguments, layer its environment variables on top of - // the previous tiers' (so a later tier overrides an earlier one with the same name). A null result environment - // leaves the child environment unchanged. - var foldedArgs = new List(args); - Dictionary? environment = null; - foreach (var tier in tiers) - { - foldedArgs.AddRange(tier.Args); - foreach (var (key, value) in tier.Env) - { - environment ??= new Dictionary(StringComparer.Ordinal); - environment[key] = value; - } - } - - // Order: base args → tier args → command-line args (override configured args) → trailing args (override - // everything, so a forwarded argument cannot countermand them). - string[] finalArgs = [.. foldedArgs, .. commandLineArgs, .. trailingArgs]; - + var (finalArgs, environment) = MergeInvocation(args, tiers, commandLineArgs, trailingArgs); return _processRunner.RunAsync( DotNetMuxer, appendVerbosity ? finalArgs.Append($"--verbosity={_reporter.Verbosity}") : finalArgs, diff --git a/tests/Buildvana.Tool.Tests/DotNetServiceMergeInvocationTests.cs b/tests/Buildvana.Tool.Tests/DotNetServiceMergeInvocationTests.cs new file mode 100644 index 0000000..519ef1b --- /dev/null +++ b/tests/Buildvana.Tool.Tests/DotNetServiceMergeInvocationTests.cs @@ -0,0 +1,92 @@ +// Copyright (C) Tenacom and Contributors. Licensed under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System; +using System.Collections.Generic; +using Buildvana.Tool.Services; + +internal sealed class DotNetServiceMergeInvocationTests +{ + [Test] + public async Task MergeInvocation_FoldsBaseTierCommandLineAndTrailingArgsInOrder() + { + var all = Settings(["--all-arg"], ("A", "1")); + var restore = Settings(["--restore-arg"], ("B", "2")); + + var (args, env) = DotNetService.MergeInvocation( + ["restore", "sln"], + [all, restore], + ["--cli"], + ["-p:CI=true"]); + + await Assert.That(Join(args)).IsEqualTo("restore|sln|--all-arg|--restore-arg|--cli|-p:CI=true"); + await Assert.That(env).IsNotNull(); + await Assert.That(env!.Count).IsEqualTo(2); + await Assert.That(env["A"]).IsEqualTo("1"); + await Assert.That(env["B"]).IsEqualTo("2"); + } + + [Test] + public async Task MergeInvocation_LaterTierOverridesEarlierTierEnvVariable() + { + var all = Settings([], ("KEY", "from-all"), ("ONLY_ALL", "kept")); + var build = Settings([], ("KEY", "from-build")); + + var (_, env) = DotNetService.MergeInvocation([], [all, build], [], []); + + await Assert.That(env).IsNotNull(); + await Assert.That(env!.Count).IsEqualTo(2); + await Assert.That(env["KEY"]).IsEqualTo("from-build"); + await Assert.That(env["ONLY_ALL"]).IsEqualTo("kept"); + } + + [Test] + public async Task MergeInvocation_PreservesNullEnvValueAsRemoval() + { + var tier = Settings([], ("REMOVE_ME", null)); + + var (_, env) = DotNetService.MergeInvocation([], [tier], [], []); + + await Assert.That(env).IsNotNull(); + await Assert.That(env!.ContainsKey("REMOVE_ME")).IsTrue(); + await Assert.That(env["REMOVE_ME"]).IsNull(); + } + + [Test] + public async Task MergeInvocation_WithoutTiersSkipsConfiguredArgsAndLeavesEnvironmentUnchanged() + { + var (args, env) = DotNetService.MergeInvocation( + ["msbuild", "proj", "-getProperty:IsTestingPlatformApplication"], + [], + ["--cli"], + ["-p:CI=false"]); + + await Assert.That(Join(args)).IsEqualTo("msbuild|proj|-getProperty:IsTestingPlatformApplication|--cli|-p:CI=false"); + await Assert.That(env).IsNull(); + } + + [Test] + public async Task MergeInvocation_WithTiersButNoEnvVariablesLeavesEnvironmentUnchanged() + { + var all = Settings(["--all-arg"]); + var build = Settings(["--build-arg"]); + + var (args, env) = DotNetService.MergeInvocation(["build"], [all, build], [], []); + + await Assert.That(Join(args)).IsEqualTo("build|--all-arg|--build-arg"); + await Assert.That(env).IsNull(); + } + + private static DotNetInvocationSettings Settings(IReadOnlyList args, params (string Key, string? Value)[] env) + { + var dictionary = new Dictionary(StringComparer.Ordinal); + foreach (var (key, value) in env) + { + dictionary[key] = value; + } + + return new DotNetInvocationSettings(args, dictionary); + } + + private static string Join(IReadOnlyList tokens) => string.Join('|', tokens); +}