Remove interactive special case for logger verbosity in dotnet run (#47389)

Author: baronfel

src/Cli/Microsoft.DotNet.Cli.Utils/MSBuildArgs.cs

@@ -12,11 +12,12 @@ namespace Microsoft.DotNet.Cli.Utils;
 /// </summary>
 public sealed class MSBuildArgs
 {
-    private MSBuildArgs(ReadOnlyDictionary<string, string>? properties, ReadOnlyDictionary<string, string>? restoreProperties, string[]? targets, string[]? otherMSBuildArgs)
+    private MSBuildArgs(ReadOnlyDictionary<string, string>? properties, ReadOnlyDictionary<string, string>? restoreProperties, string[]? targets, VerbosityOptions? verbosity, string[]? otherMSBuildArgs)
     {
         GlobalProperties = properties;
         RestoreGlobalProperties = restoreProperties;
         RequestedTargets = targets;
+        Verbosity = verbosity;
         OtherMSBuildArgs = otherMSBuildArgs is not null
             ? [.. otherMSBuildArgs]
             : new List<string>();
@@ -36,9 +37,11 @@ private MSBuildArgs(ReadOnlyDictionary<string, string>? properties, ReadOnlyDict
     /// The ordered list of targets that should be passed to MSBuild.
     /// </summary>
     public string[]? RequestedTargets { get; }
+    public VerbosityOptions? Verbosity { get; }
 
     /// <summary>
-    /// All non <c>-p</c> and <c>-rp</c> arguments that should be passed to MSBuild.
+    /// All other arguments that aren't already explicitly modeled by this structure.
+    /// The main categories of these today are logger configurations
     /// </summary>
     public List<string> OtherMSBuildArgs { get; }
 
@@ -64,26 +67,34 @@ public static MSBuildArgs AnalyzeMSBuildArguments(IEnumerable<string> forwardedA
         var globalProperties = parseResult.GetResult("--property") is OptionResult propResult ? propResult.GetValueOrDefault<ReadOnlyDictionary<string, string>?>() : null;
         var restoreProperties = parseResult.GetResult("--restoreProperty") is OptionResult restoreResult ? restoreResult.GetValueOrDefault<ReadOnlyDictionary<string, string>?>() : null;
         var requestedTargets = parseResult.GetResult("--target") is OptionResult targetResult ? targetResult.GetValueOrDefault<string[]?>() : null;
+        var verbosity = parseResult.GetResult("--verbosity") is OptionResult verbosityResult
+            ? verbosityResult.GetValueOrDefault<VerbosityOptions?>()
+            : null;
         var otherMSBuildArgs = parseResult.UnmatchedTokens.ToArray();
         return new MSBuildArgs(
             properties: globalProperties,
             restoreProperties: restoreProperties,
             targets: requestedTargets,
-            otherMSBuildArgs: otherMSBuildArgs);
+            otherMSBuildArgs: otherMSBuildArgs,
+            verbosity: verbosity);
     }
 
 
     public static MSBuildArgs FromProperties(ReadOnlyDictionary<string, string>? properties)
     {
-        return new MSBuildArgs(properties, null, null, null);
+        return new MSBuildArgs(properties, null, null, null, null);
     }
 
     public static MSBuildArgs FromOtherArgs(params ReadOnlySpan<string> args)
     {
-        return new MSBuildArgs(null, null, null, args.ToArray());
+        return new MSBuildArgs(null, null, null, null, args.ToArray());
+    }
+    public static MSBuildArgs FromVerbosity(VerbosityOptions verbosity)
+    {
+        return new MSBuildArgs(null, null, null, verbosity, null);
     }
 
-    public static readonly MSBuildArgs ForHelp = new(null, null, null, ["--help"]);
+    public static readonly MSBuildArgs ForHelp = new(null, null, null, null, ["--help"]);
 
     /// <summary>
     /// Completely replaces the MSBuild arguments with the provided <paramref name="newArgs"/>.
@@ -94,7 +105,8 @@ public MSBuildArgs CloneWithExplicitArgs(string[] newArgs)
             properties: GlobalProperties,
             restoreProperties: RestoreGlobalProperties,
             targets: RequestedTargets,
-            otherMSBuildArgs: newArgs);
+            otherMSBuildArgs: newArgs,
+            verbosity: Verbosity);
     }
 
     /// <summary>
@@ -105,60 +117,71 @@ public MSBuildArgs CloneWithAdditionalArgs(params string[] additionalArgs)
         if (additionalArgs is null || additionalArgs.Length == 0)
         {
             // If there are no additional args, we can just return the current instance.
-            return new MSBuildArgs(GlobalProperties, RestoreGlobalProperties, RequestedTargets, OtherMSBuildArgs.ToArray());
+            return new MSBuildArgs(GlobalProperties, RestoreGlobalProperties, RequestedTargets, Verbosity, OtherMSBuildArgs.ToArray());
         }
 
-        return new MSBuildArgs(GlobalProperties, RestoreGlobalProperties, RequestedTargets, [.. OtherMSBuildArgs, .. additionalArgs]);
+        return new MSBuildArgs(GlobalProperties, RestoreGlobalProperties, RequestedTargets, Verbosity, [.. OtherMSBuildArgs, .. additionalArgs]);
     }
 
     public MSBuildArgs CloneWithAdditionalRestoreProperties(ReadOnlyDictionary<string, string>? additionalRestoreProperties)
     {
         if (additionalRestoreProperties is null || additionalRestoreProperties.Count == 0)
         {
             // If there are no additional restore properties, we can just return the current instance.
-            return new MSBuildArgs(GlobalProperties, RestoreGlobalProperties, RequestedTargets, OtherMSBuildArgs.ToArray());
+            return new MSBuildArgs(GlobalProperties, RestoreGlobalProperties, RequestedTargets, Verbosity, OtherMSBuildArgs.ToArray());
         }
         if (RestoreGlobalProperties is null)
         {
-            return new MSBuildArgs(GlobalProperties, additionalRestoreProperties, RequestedTargets, OtherMSBuildArgs.ToArray());
+            return new MSBuildArgs(GlobalProperties, additionalRestoreProperties, RequestedTargets, Verbosity, OtherMSBuildArgs.ToArray());
         }
 
         var newRestoreProperties = new Dictionary<string, string>(RestoreGlobalProperties, StringComparer.OrdinalIgnoreCase);
         foreach (var kvp in additionalRestoreProperties)
         {
             newRestoreProperties[kvp.Key] = kvp.Value;
         }
-        return new MSBuildArgs(GlobalProperties, new(newRestoreProperties), RequestedTargets, OtherMSBuildArgs.ToArray());
+        return new MSBuildArgs(GlobalProperties, new(newRestoreProperties), RequestedTargets, Verbosity, OtherMSBuildArgs.ToArray());
     }
 
     public MSBuildArgs CloneWithAdditionalProperties(ReadOnlyDictionary<string, string>? additionalProperties)
     {
         if (additionalProperties is null || additionalProperties.Count == 0)
         {
             // If there are no additional properties, we can just return the current instance.
-            return new MSBuildArgs(GlobalProperties, RestoreGlobalProperties, RequestedTargets, OtherMSBuildArgs.ToArray());
+            return new MSBuildArgs(GlobalProperties, RestoreGlobalProperties, RequestedTargets, Verbosity, OtherMSBuildArgs.ToArray());
         }
         if (GlobalProperties is null)
         {
-            return new MSBuildArgs(additionalProperties, RestoreGlobalProperties, RequestedTargets, OtherMSBuildArgs.ToArray());
+            return new MSBuildArgs(additionalProperties, RestoreGlobalProperties, RequestedTargets, Verbosity, OtherMSBuildArgs.ToArray());
         }
 
         var newProperties = new Dictionary<string, string>(GlobalProperties, StringComparer.OrdinalIgnoreCase);
         foreach (var kvp in additionalProperties)
         {
             newProperties[kvp.Key] = kvp.Value;
         }
-        return new MSBuildArgs(new(newProperties), RestoreGlobalProperties, RequestedTargets, OtherMSBuildArgs.ToArray());
+        return new MSBuildArgs(new(newProperties), RestoreGlobalProperties, RequestedTargets, Verbosity, OtherMSBuildArgs.ToArray());
     }
 
     public MSBuildArgs CloneWithAdditionalTarget(string additionalTarget)
     {
         string[] newTargets = RequestedTargets is not null
             ? [.. RequestedTargets, additionalTarget]
             : [ additionalTarget ];
-        return new MSBuildArgs(GlobalProperties, RestoreGlobalProperties, newTargets, OtherMSBuildArgs.ToArray());
+        return new MSBuildArgs(GlobalProperties, RestoreGlobalProperties, newTargets, Verbosity, OtherMSBuildArgs.ToArray());
     }
 
+    public MSBuildArgs CloneWithVerbosity(VerbosityOptions newVerbosity)
+    {
+        return new MSBuildArgs(GlobalProperties, RestoreGlobalProperties, RequestedTargets, newVerbosity, OtherMSBuildArgs.ToArray());
+    }
+
+    /// <summary>
+    /// This mutates the <see cref="MSBuildArgs"/> instance, applying all of the current global properties
+    /// to the restore properties dictionary. This is necessary because MSBuild's processing of restore properties
+    /// is _exclusive_ - as soon as it sees a <c>-rp</c> flag, it will not apply any <c>-p</c> flags
+    /// to the implicit restore operation.
+    /// </summary>
     public void ApplyPropertiesToRestore()
     {
         if (RestoreGlobalProperties is null)

src/Cli/Microsoft.DotNet.Cli.Utils/MSBuildForwardingAppWithoutLogging.cs

@@ -41,7 +41,7 @@ public static string MSBuildVersion
 
     private readonly Dictionary<string, string> _msbuildRequiredEnvironmentVariables = GetMSBuildRequiredEnvironmentVariables();
 
-    private readonly List<string> _msbuildRequiredParameters = [ "-maxcpucount", "-verbosity:m" ];
+    private readonly List<string> _msbuildRequiredParameters = [ "-maxcpucount", "--verbosity:m" ];
 
     public MSBuildForwardingAppWithoutLogging(MSBuildArgs msbuildArgs, string? msbuildPath = null, bool includeLogo = false, bool isRestoring = true)
     {
@@ -100,6 +100,7 @@ private string[] EmitMSBuildArgs(MSBuildArgs msbuildArgs) => [
         .. msbuildArgs.GlobalProperties?.Select(kvp => EmitProperty(kvp)) ?? [],
         .. msbuildArgs.RestoreGlobalProperties?.Select(kvp => EmitProperty(kvp, "restoreProperty")) ?? [],
         .. msbuildArgs.RequestedTargets?.Select(target => $"--target:{target}") ?? [],
+        .. msbuildArgs.Verbosity is not null ? new string[1] { $"--verbosity:{msbuildArgs.Verbosity}" } : [],
         .. msbuildArgs.OtherMSBuildArgs
     ];
 

src/Cli/Microsoft.DotNet.Cli.Utils/VerbosityOptions.cs

@@ -0,0 +1,21 @@
+namespace Microsoft.DotNet.Cli.Utils;
+
+/// <summary>
+/// Represents the desired verbosity level for command output.
+/// Maps mostly to MSBuild's verbosity levels.
+/// The odd naming is because we're currently leaning entirely on System.CommandLine's
+/// default enum parsing.
+/// </summary>
+public enum VerbosityOptions
+{
+    quiet,
+    q,
+    minimal,
+    m,
+    normal,
+    n,
+    detailed,
+    d,
+    diagnostic,
+    diag
+}

src/Cli/dotnet/CommandFactory/CommandResolution/ProjectToolsCommandResolver.cs

@@ -385,7 +385,7 @@ internal void GenerateDepsJsonFile(
         string? stdOut;
         string? stdErr;
 
-        var msbuildArgs = MSBuildArgs.AnalyzeMSBuildArguments([..args], CommonOptions.PropertiesOption, CommonOptions.RestorePropertiesOption, BuildCommandParser.TargetOption);
+        var msbuildArgs = MSBuildArgs.AnalyzeMSBuildArguments([..args], CommonOptions.PropertiesOption, CommonOptions.RestorePropertiesOption, BuildCommandParser.TargetOption, BuildCommandParser.VerbosityOption);
         var forwardingAppWithoutLogging = new MSBuildForwardingAppWithoutLogging(msbuildArgs, msBuildExePath);
         if (forwardingAppWithoutLogging.ExecuteMSBuildOutOfProc)
         {

src/Cli/dotnet/Commands/Build/BuildCommand.cs

@@ -43,7 +43,7 @@ public static CommandBase FromParseResult(ParseResult parseResult, string? msbui
                 noRestore: noRestore,
                 msbuildPath: msbuildPath
             ),
-            [CommonOptions.PropertiesOption, CommonOptions.RestorePropertiesOption, BuildCommandParser.TargetOption],
+            [CommonOptions.PropertiesOption, CommonOptions.RestorePropertiesOption, BuildCommandParser.TargetOption, BuildCommandParser.VerbosityOption],
             parseResult,
             msbuildPath
         );

src/Cli/dotnet/Commands/Build/BuildCommandParser.cs

@@ -58,7 +58,7 @@ internal static class BuildCommandParser
     /// </summary>
     public static readonly Option<string[]?> TargetOption = CommonOptions.MSBuildTargetOption();
 
-    public static readonly Option<VerbosityOptions?> VerbosityOption = CommonOptions.VerbosityOption();
+    public static readonly Option<Utils.VerbosityOptions?> VerbosityOption = CommonOptions.VerbosityOption();
 
     private static readonly Command Command = ConstructCommand();
 

src/Cli/dotnet/Commands/Clean/CleanCommand.cs

@@ -34,7 +34,7 @@ public static CommandBase FromParseResult(ParseResult result, string? msbuildPat
                 NoBuildMarkers = true,
             },
             static (msbuildArgs, msbuildPath) => new CleanCommand(msbuildArgs, msbuildPath),
-            [ CommonOptions.PropertiesOption, CommonOptions.RestorePropertiesOption, CleanCommandParser.TargetOption ],
+            [ CommonOptions.PropertiesOption, CommonOptions.RestorePropertiesOption, CleanCommandParser.TargetOption, CleanCommandParser.VerbosityOption ],
             result,
             msbuildPath
         );

src/Cli/dotnet/Commands/Clean/CleanCommandParser.cs

@@ -34,7 +34,7 @@ internal static class CleanCommandParser
 
     public static readonly Option<string[]> TargetOption = CommonOptions.RequiredMSBuildTargetOption("Clean");
 
-    public static readonly Option<VerbosityOptions> VerbosityOption = CommonOptions.VerbosityOption(VerbosityOptions.normal);
+    public static readonly Option<Utils.VerbosityOptions> VerbosityOption = CommonOptions.VerbosityOption(Utils.VerbosityOptions.normal);
 
 
     private static readonly Command Command = ConstructCommand();

src/Cli/dotnet/Commands/MSBuild/MSBuildCommand.cs

@@ -10,7 +10,7 @@ namespace Microsoft.DotNet.Cli.Commands.MSBuild;
 public class MSBuildCommand(
     IEnumerable<string> msbuildArgs,
     string? msbuildPath = null
-) : MSBuildForwardingApp(MSBuildArgs.AnalyzeMSBuildArguments([..msbuildArgs], CommonOptions.PropertiesOption, CommonOptions.RestorePropertiesOption, MSBuildCommandParser.TargetOption), msbuildPath, includeLogo: true)
+) : MSBuildForwardingApp(MSBuildArgs.AnalyzeMSBuildArguments([..msbuildArgs], CommonOptions.PropertiesOption, CommonOptions.RestorePropertiesOption, MSBuildCommandParser.TargetOption, CommonOptions.VerbosityOption()), msbuildPath, includeLogo: true)
 {
     public static MSBuildCommand FromArgs(string[] args, string? msbuildPath = null)
     {

src/Cli/dotnet/Commands/MSBuild/MSBuildForwardingApp.cs

@@ -38,7 +38,7 @@ private static MSBuildArgs ConcatTelemetryLogger(MSBuildArgs msbuildArgs)
     /// Mostly intended for quick/one-shot usage - most 'core' SDK commands should do more hands-on parsing.
     /// </summary>
     public MSBuildForwardingApp(IEnumerable<string> rawMSBuildArgs, string? msbuildPath = null) : this(
-        MSBuildArgs.AnalyzeMSBuildArguments(rawMSBuildArgs.ToArray(), CommonOptions.PropertiesOption, CommonOptions.RestorePropertiesOption, CommonOptions.MSBuildTargetOption()),
+        MSBuildArgs.AnalyzeMSBuildArguments(rawMSBuildArgs.ToArray(), CommonOptions.PropertiesOption, CommonOptions.RestorePropertiesOption, CommonOptions.MSBuildTargetOption(), CommonOptions.VerbosityOption()),
         msbuildPath)
     {
     }