Read verbosity from env in ICakeLog registration - #4949
Conversation
ICakeLog is now registered via a factory that reads the "CAKE_SETTINGS_VERBOSITY" environment variable, parses it as a Verbosity enum (case-insensitive), and defaults to Verbosity.Normal if parsing fails. CakeBuildLog is instantiated with the resolved verbosity using ActivatorUtilities.
devlead
left a comment
devlead
left a comment
There was a problem hiding this comment.
For Frosting, the Verbosity is set as part of execution here
So setting it in IoC isn't going to change it, as it's a singleton and set before execution.
The same pattern is used in Cake Tool
cake/src/Cake/Commands/DefaultCommand.cs
Line 66 in 59eb19d
Also, using System.Environment.GetEnvironmentVariable makes it untestable; for configuration in Cake, we use ICakeConfiguration as it comes from Cake IOC; it's testable (uses Cake environment and args and replaceable by Cake Modules. ICakeConfiguration also handles configuration via files, arguments, and environment variables. So it's idiomatic with how other settings are handled within Cake. And if we introduce a new configuration setting, we want, as far as possible and where it makes sense, to make it available for all Cake runners.
So a more correct change would be something roughly like below: a common helper method that lets you override verbosity via configuration and then falls back to settings (default command line) if no config is available
diff --git a/src/Cake.Core/Constants.cs b/src/Cake.Core/Constants.cs
index 7a202ca2..f2d1edf6 100644
--- a/src/Cake.Core/Constants.cs
+++ b/src/Cake.Core/Constants.cs
@@ -20,6 +20,7 @@ namespace Cake.Core
public const string NoMonoCoersion = "Settings_NoMonoCoersion";
public const string ShowProcessCommandLine = "Settings_ShowProcessCommandLine";
public const string UnifiedDependencyGraphForMultipleTargets = "Settings_UnifiedDependencyGraphForMultipleTargets";
+ public const string Verbosity = "Settings_Verbosity";
}
public static class Paths
diff --git a/src/Cake.Core/Extensions/CakeConfigurationExtensions.cs b/src/Cake.Core/Extensions/CakeConfigurationExtensions.cs
index 0462334d..9bf32c3f 100644
--- a/src/Cake.Core/Extensions/CakeConfigurationExtensions.cs
+++ b/src/Cake.Core/Extensions/CakeConfigurationExtensions.cs
@@ -1,6 +1,7 @@
using System;
using Cake.Core.Configuration;
+using Cake.Core.Diagnostics;
using Cake.Core.IO;
// ReSharper disable once CheckNamespace
@@ -64,5 +65,20 @@ namespace Cake.Core
var toolPath = configuration.GetToolPath(defaultRoot, environment).ExpandShortPath();
return toolPath.Combine("Modules").Collapse();
}
+
+ /// <summary>
+ /// Gets the verbosity level from the configuration.
+ /// </summary>
+ /// <param name="configuration">The Cake configuration.</param>
+ /// <returns>The configured verbosity level, or <paramref name="defaultValue"/> if not found or invalid.</returns>
+ public static Verbosity GetVerbosity(this ICakeConfiguration configuration, Verbosity defaultValue)
+ {
+ var verbosity = configuration.GetValue(Constants.Settings.Verbosity);
+ if (!string.IsNullOrWhiteSpace(verbosity) && Enum.TryParse<Verbosity>(verbosity, true, out var result))
+ {
+ return result;
+ }
+ return defaultValue;
+ }
}
}
diff --git a/src/Cake.Frosting/Internal/Commands/DefaultCommand.cs b/src/Cake.Frosting/Internal/Commands/DefaultCommand.cs
index b1de7d14..4e7ae75c 100644
--- a/src/Cake.Frosting/Internal/Commands/DefaultCommand.cs
+++ b/src/Cake.Frosting/Internal/Commands/DefaultCommand.cs
@@ -7,6 +7,7 @@ using System.Linq;
using Cake.Cli;
using Cake.Cli.Infrastructure;
using Cake.Core;
+using Cake.Core.Configuration;
using Cake.Core.Diagnostics;
using Cake.Core.IO;
using Cake.Core.Packaging;
@@ -52,7 +53,8 @@ namespace Cake.Frosting.Internal
// Set the log verbosity
var log = provider.GetRequiredService<ICakeLog>();
- log.Verbosity = settings.Verbosity;
+ var configuration = provider.GetRequiredService<ICakeConfiguration>();
+ log.Verbosity = configuration.GetVerbosity(settings.Verbosity);
// Set the working directory
SetWorkingDirectory(provider, settings);
diff --git a/src/Cake/Commands/DefaultCommand.cs b/src/Cake/Commands/DefaultCommand.cs
index 4082aa68..d431b700 100644
--- a/src/Cake/Commands/DefaultCommand.cs
+++ b/src/Cake/Commands/DefaultCommand.cs
@@ -7,6 +7,7 @@ using System.Collections.Generic;
using Cake.Cli;
using Cake.Cli.Infrastructure;
using Cake.Core;
+using Cake.Core.Configuration;
using Cake.Core.Diagnostics;
using Cake.Features.Bootstrapping;
using Cake.Features.Building;
@@ -25,6 +26,7 @@ namespace Cake.Commands
private readonly ICakeInfoFeature _info;
private readonly IConsole _console;
private readonly ICakeLog _log;
+ private readonly ICakeConfiguration _configuration;
/// <summary>
/// Initializes a new instance of the <see cref="DefaultCommand"/> class.
@@ -35,18 +37,21 @@ namespace Cake.Commands
/// <param name="info">The info feature.</param>
/// <param name="console">The console.</param>
/// <param name="log">The log.</param>
+ /// <param name="configuration">The configuration.</param>
public DefaultCommand(
IBuildFeature builder,
IBootstrapFeature bootstrapper,
ICakeVersionFeature version,
ICakeInfoFeature info,
IConsole console,
- ICakeLog log)
+ ICakeLog log,
+ ICakeConfiguration configuration)
{
_builder = builder;
_bootstrapper = bootstrapper;
_version = version;
_info = info;
+ _configuration = configuration;
_console = console;
_log = log;
}
@@ -63,7 +68,7 @@ namespace Cake.Commands
try
{
// Set log verbosity.
- _log.Verbosity = settings.Verbosity;
+ _log.Verbosity = _configuration.GetVerbosity(settings.Verbosity);
if (settings.ShowVersion)
{
@@ -82,7 +87,7 @@ namespace Cake.Commands
// Run the bootstrapper?
if (!settings.SkipBootstrap || settings.Bootstrap)
{
- int bootstrapperResult = PerformBootstrapping(context, settings, host);
+ int bootstrapperResult = PerformBootstrapping(context, settings, host, _log.Verbosity);
if (bootstrapperResult != 0 || settings.Bootstrap)
{
return bootstrapperResult;
@@ -125,7 +130,7 @@ namespace Cake.Commands
return BuildHostKind.Build;
}
- private int PerformBootstrapping(CommandContext context, DefaultCommandSettings settings, BuildHostKind host)
+ private int PerformBootstrapping(CommandContext context, DefaultCommandSettings settings, BuildHostKind host, Verbosity verbosity)
{
if (host != BuildHostKind.Build && host != BuildHostKind.DryRun)
{
@@ -137,7 +142,7 @@ namespace Cake.Commands
return _bootstrapper.Run(arguments, new BootstrapFeatureSettings
{
Script = settings.Script,
- Verbosity = settings.Verbosity
+ Verbosity = verbosity
});
}
diff --git a/src/Cake/Features/Bootstrapping/BootstrapFeature.cs b/src/Cake/Features/Bootstrapping/BootstrapFeature.cs
index f42f2cf6..281526e8 100644
--- a/src/Cake/Features/Bootstrapping/BootstrapFeature.cs
+++ b/src/Cake/Features/Bootstrapping/BootstrapFeature.cs
@@ -6,6 +6,7 @@ using System;
using System.Linq;
using Autofac;
using Cake.Core;
+using Cake.Core.Configuration;
using Cake.Core.Diagnostics;
using Cake.Core.IO;
using Cake.Core.Scripting;
@@ -72,7 +73,7 @@ namespace Cake.Features.Bootstrapping
// Set log verbosity for log in new scope.
var log = scope.Resolve<ICakeLog>();
- log.Verbosity = settings.Verbosity;
+ log.Verbosity = configuration.GetVerbosity(settings.Verbosity);
// Get the root directory.
var root = settings.Script.GetDirectory();
diff --git a/src/Cake/Features/Building/BuildFeature.cs b/src/Cake/Features/Building/BuildFeature.cs
index 7988d6b0..bc1fa62b 100644
--- a/src/Cake/Features/Building/BuildFeature.cs
+++ b/src/Cake/Features/Building/BuildFeature.cs
@@ -118,7 +118,7 @@ namespace Cake.Features.Building
// Set log verbosity for log in new scope.
var log = scope.Resolve<ICakeLog>();
- log.Verbosity = settings.Verbosity;
+ log.Verbosity = configuration.GetVerbosity(settings.Verbosity);
// Create the script host.
var host = CreateScriptHost(settings, scope);
|
Got it! |
|
@devlead, thinking deeper about this, this would have the Could |
Probably, but changing a type is generally a breaking change, and needs more thought. That said other configuration settings override default behavior by being present, so would follow how other configuration works. You can always override a environmental config value with a argument. |
That's not the case for verbosity, because, by the time you are setting it, you don't know if the verbosity was set as an argument or is the default for The best I could get, so far, is: if (Enum.TryParse<Verbosity>(Environment.GetEnvironmentVariable("CAKE_SETTINGS_VERBOSITY"), true, out var verbosity)
&& !args.Any(arg =>
arg.StartsWith("--verbosity=", StringComparison.OrdinalIgnoreCase)
|| arg.Equals("--verbosity", StringComparison.OrdinalIgnoreCase)
|| arg.Equals("-v", StringComparison.OrdinalIgnoreCase)))
{
args = [..args, $"--verbosity={verbosity}"];
}
return new CakeHost()
...
.Run(args); |
2 participants
Follow up from #4947.
PR Classification
Dependency injection behavior update to support configurable logging verbosity.
PR Summary
The registration of
ICakeLogin the DI container now uses a factory that reads verbosity from theCAKE_SETTINGS_VERBOSITYenvironment variable, defaulting toNormalif parsing fails.ICakeLogfrom a direct singleton to a factory method.CAKE_SETTINGS_VERBOSITYfor Verbosity enum, with fallback to Verbosity.Normal.CakeBuildLogis instantiated with the resolved verbosity usingActivatorUtilities.@devlead, what tests should I add?