Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BLOCKED] update System.CommandLine #7559

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions eng/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,7 @@
<NewtonsoftJsonVersion>13.0.1</NewtonsoftJsonVersion>
<PerfolizerVersion>0.2.1</PerfolizerVersion>
<SQLitePCLRawVersion>1.1.2</SQLitePCLRawVersion>
<SystemCommandLineRenderingVersion>2.0.0-beta1.20074.1</SystemCommandLineRenderingVersion>
<SystemCommandLineVersion>2.0.0-beta1.21216.1</SystemCommandLineVersion>
<SystemCommandLineVersion>2.0.0-beta4.25072.1</SystemCommandLineVersion>
<SystemComponentModelCompositionVersion>4.7.0</SystemComponentModelCompositionVersion>
<SystemCompositionVersion>8.0.0-preview.4.23259.5</SystemCompositionVersion>
<XunitCombinatorialVersion>1.2.7</XunitCombinatorialVersion>
Expand Down
49 changes: 33 additions & 16 deletions src/Tools/PerfDiff/DiffCommand.cs
Original file line number Diff line number Diff line change
@@ -1,36 +1,53 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System.CommandLine;
using System.Threading.Tasks;

namespace PerfDiff
{
internal static class DiffCommand
{
// This delegate should be kept in Sync with the FormatCommand options and argument names
// so that values bind correctly.
internal delegate Task<int> Handler(
string baseline,
string results,
string? verbosity,
bool failOnRegression,
IConsole console);

internal static string[] VerbosityLevels => new[] { "q", "quiet", "m", "minimal", "n", "normal", "d", "detailed", "diag", "diagnostic" };

internal static RootCommand CreateCommandLineOptions()
{
// Sync changes to option and argument names with the FormatCommant.Handler above.
var baseline = new Option<string>("--baseline")
{
Description = "folder that contains the baseline performance run data"
};
baseline.AcceptLegalFilePathsOnly();
var results = new Option<string>("--results")
{
Description = "folder that contains the performance restults"
};
results.AcceptLegalFilePathsOnly();
var verbosity = new Option<string>("--verbosity", "-v")
{
Description = "Set the verbosity level. Allowed values are q[uiet], m[inimal], n[ormal], d[etailed], and diag[nostic]"
};
verbosity.AcceptOnlyFromAmong(VerbosityLevels);
var failOnRegression = new Option<bool>("--failOnRegression")
{
Description = "Should return non-zero exit code if regression detected"
};

var rootCommand = new RootCommand
{
new Option<string?>("--baseline", () => null, "folder that contains the baseline performance run data").LegalFilePathsOnly(),
new Option<string?>("--results", () => null, "folder that contains the performance restults").LegalFilePathsOnly(),
new Option<string>(new[] { "--verbosity", "-v" }, "Set the verbosity level. Allowed values are q[uiet], m[inimal], n[ormal], d[etailed], and diag[nostic]").FromAmong(VerbosityLevels),
new Option<bool>(new[] { "--failOnRegression" }, "Should return non-zero exit code if regression detected"),
baseline,
results,
verbosity,
failOnRegression
};

rootCommand.Description = "diff two sets of performance results";

rootCommand.SetAction((parseResult, ct) => Program.RunAsync(
baseline: parseResult.GetValue(baseline)!,
results: parseResult.GetValue(results)!,
verbosity: parseResult.GetValue(verbosity),
failOnRegression: parseResult.GetValue(failOnRegression),
token: ct
));

return rootCommand;
}
}
Expand Down
53 changes: 22 additions & 31 deletions src/Tools/PerfDiff/Logging/SimpleConsoleLogger.cs
Original file line number Diff line number Diff line change
@@ -1,21 +1,20 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using Microsoft.Extensions.Logging;
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.CommandLine;
using System.CommandLine.Rendering;
using Microsoft.Extensions.Logging;

namespace PerfDiff.Logging
{
internal sealed class SimpleConsoleLogger : ILogger
{
private readonly object _gate = new();
private static readonly bool ColorsAreSupported =
!(OperatingSystem.IsBrowser() || OperatingSystem.IsAndroid() || OperatingSystem.IsIOS() || OperatingSystem.IsTvOS())
&& !Console.IsOutputRedirected;

private static readonly object _gate = new();

private readonly IConsole _console;
private readonly ITerminal _terminal;
private readonly LogLevel _minimalLogLevel;
private readonly LogLevel _minimalErrorLevel;

Expand All @@ -30,10 +29,8 @@ internal sealed class SimpleConsoleLogger : ILogger
[LogLevel.None] = ConsoleColor.White,
}.ToImmutableDictionary();

public SimpleConsoleLogger(IConsole console, LogLevel minimalLogLevel, LogLevel minimalErrorLevel)
public SimpleConsoleLogger(LogLevel minimalLogLevel, LogLevel minimalErrorLevel)
{
_terminal = console.GetTerminal();
_console = console;
_minimalLogLevel = minimalLogLevel;
_minimalErrorLevel = minimalErrorLevel;
}
Expand All @@ -49,14 +46,8 @@ public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Except
{
var message = formatter(state, exception);
var logToErrorStream = logLevel >= _minimalErrorLevel;
if (_terminal is null)
{
LogToConsole(_console, message, logToErrorStream);
}
else
{
LogToTerminal(message, logLevel, logToErrorStream);
}

Log(message, logLevel, logToErrorStream);
}
}

Expand All @@ -70,25 +61,25 @@ public IDisposable BeginScope<TState>(TState state)
return NullScope.Instance;
}

private void LogToTerminal(string message, LogLevel logLevel, bool logToErrorStream)
private void Log(string message, LogLevel logLevel, bool logToErrorStream)
{
var messageColor = LogLevelColorMap[logLevel];
_terminal.ForegroundColor = messageColor;

LogToConsole(_terminal, message, logToErrorStream);

_terminal.ResetColor();
}
if (ColorsAreSupported)
{
Console.ForegroundColor = LogLevelColorMap[logLevel];
}

private static void LogToConsole(IConsole console, string message, bool logToErrorStream)
{
if (logToErrorStream)
{
console.Error.Write($"{message}{Environment.NewLine}");
Console.Error.WriteLine(message);
}
else
{
console.Out.Write($" {message}{Environment.NewLine}");
Console.Out.WriteLine(message);
}

if (ColorsAreSupported)
{
Console.ResetColor();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System.CommandLine;
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using Microsoft.Extensions.Logging;

namespace PerfDiff.Logging
{
internal static class SimpleConsoleLoggerFactoryExtensions
{
public static ILoggerFactory AddSimpleConsole(this ILoggerFactory factory, IConsole console, LogLevel minimalLogLevel, LogLevel minimalErrorLevel)
public static ILoggerFactory AddSimpleConsole(this ILoggerFactory factory, LogLevel minimalLogLevel, LogLevel minimalErrorLevel)
{
factory.AddProvider(new SimpleConsoleLoggerProvider(console, minimalLogLevel, minimalErrorLevel));
factory.AddProvider(new SimpleConsoleLoggerProvider(minimalLogLevel, minimalErrorLevel));
return factory;
}
}
Expand Down
10 changes: 3 additions & 7 deletions src/Tools/PerfDiff/Logging/SimpleConsoleLoggerProvider.cs
Original file line number Diff line number Diff line change
@@ -1,27 +1,23 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using Microsoft.Extensions.Logging;

using System.CommandLine;

namespace PerfDiff.Logging
{
internal sealed class SimpleConsoleLoggerProvider : ILoggerProvider
{
private readonly IConsole _console;
private readonly LogLevel _minimalLogLevel;
private readonly LogLevel _minimalErrorLevel;

public SimpleConsoleLoggerProvider(IConsole console, LogLevel minimalLogLevel, LogLevel minimalErrorLevel)
public SimpleConsoleLoggerProvider(LogLevel minimalLogLevel, LogLevel minimalErrorLevel)
{
_console = console;
_minimalLogLevel = minimalLogLevel;
_minimalErrorLevel = minimalErrorLevel;
}

public ILogger CreateLogger(string categoryName)
{
return new SimpleConsoleLogger(_console, _minimalLogLevel, _minimalErrorLevel);
return new SimpleConsoleLogger(_minimalLogLevel, _minimalErrorLevel);
}

public void Dispose()
Expand Down
1 change: 0 additions & 1 deletion src/Tools/PerfDiff/PerfDiff.csproj
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
Expand All @@ -9,7 +9,6 @@

<ItemGroup>
<PackageReference Include="System.CommandLine" Version="$(SystemCommandLineVersion)" />
<PackageReference Include="System.CommandLine.Rendering" Version="$(SystemCommandLineRenderingVersion)" />
<PackageReference Include="Microsoft.Diagnostics.Tracing.TraceEvent" Version="$(MicrosoftDiagnosticsTracingTraceEventVersion)" />
<PackageReference Include="Microsoft.Extensions.Logging" Version="$(MicrosoftExtensionsLoggingVersion)" />
<PackageReference Include="Newtonsoft.Json" Version="$(NewtonsoftJsonVersion)" />
Expand Down
39 changes: 7 additions & 32 deletions src/Tools/PerfDiff/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@
using PerfDiff.Logging;

using System;
using System.CommandLine;
using System.CommandLine.Invocation;
using System.CommandLine.Parsing;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -18,48 +15,26 @@ namespace PerfDiff
internal sealed class Program
{
internal const int UnhandledExceptionExitCode = 1;
private static ParseResult? s_parseResult;

private static async Task<int> Main(string[] args)
{
var rootCommand = DiffCommand.CreateCommandLineOptions();
rootCommand.Handler = CommandHandler.Create(new DiffCommand.Handler(RunAsync));

// Parse the incoming args so we can give warnings when deprecated options are used.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not able to find any other usage of Program.RunAsync so I've assumed that it's most likely some leftover and removed it.

FWIW if there will be any parsing errors detected, the Parse(args).InvokeAsync() call is going to get them printed to output and also return error code.

s_parseResult = rootCommand.Parse(args);

return await rootCommand.InvokeAsync(args).ConfigureAwait(false);
}
private static Task<int> Main(string[] args)
=> DiffCommand.CreateCommandLineOptions().Parse(args).InvokeAsync();

public static async Task<int> RunAsync(
string baseline,
string results,
string? verbosity,
bool failOnRegression,
IConsole console)
CancellationToken token)
{
if (s_parseResult == null)
{
return 1;
}

// Setup logging.
var logLevel = GetLogLevel(verbosity);
var logger = SetupLogging(console, minimalLogLevel: logLevel, minimalErrorLevel: LogLevel.Warning);

// Hook so we can cancel and exit when ctrl+c is pressed.
var cancellationTokenSource = new CancellationTokenSource();
Console.CancelKeyPress += (sender, e) =>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now being performed out of the box by System.CommandLine. All you need it to pass the cancellation token in SetAction to given async method

{
e.Cancel = true;
cancellationTokenSource.Cancel();
};
var logger = SetupLogging(minimalLogLevel: logLevel, minimalErrorLevel: LogLevel.Warning);

var currentDirectory = string.Empty;

try
{
var exitCode = await PerfDiff.CompareAsync(baseline, results, failOnRegression, logger, cancellationTokenSource.Token).ConfigureAwait(false);
var exitCode = await PerfDiff.CompareAsync(baseline, results, failOnRegression, logger, token).ConfigureAwait(false);
return exitCode;
}
catch (FileNotFoundException fex)
Expand All @@ -83,10 +58,10 @@ public static async Task<int> RunAsync(
}
}

static ILogger<Program> SetupLogging(IConsole console, LogLevel minimalLogLevel, LogLevel minimalErrorLevel)
static ILogger<Program> SetupLogging(LogLevel minimalLogLevel, LogLevel minimalErrorLevel)
{
var serviceCollection = new ServiceCollection();
serviceCollection.AddSingleton(new LoggerFactory().AddSimpleConsole(console, minimalLogLevel, minimalErrorLevel));
serviceCollection.AddSingleton(new LoggerFactory().AddSimpleConsole(minimalLogLevel, minimalErrorLevel));
serviceCollection.AddLogging();

var serviceProvider = serviceCollection.BuildServiceProvider();
Expand Down
Loading