Skip to content

A handful of S.CL-related cleanups and reorgs from the OTel branch to minimize diffs #50164

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
16 changes: 14 additions & 2 deletions src/Cli/Microsoft.DotNet.Cli.Utils/Activities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,27 @@
namespace Microsoft.DotNet.Cli.Utils;

/// <summary>
/// Contains helpers for working with <see cref="System.Diagnostics.Activity">Activities</see> in the .NET CLI.
/// Contains helpers for working with <see cref="Activity">Activities</see> in the .NET CLI.
/// </summary>
public static class Activities
{

/// <summary>
/// The main entrypoint for creating <see cref="Activity">Activities</see> in the .NET CLI.
/// All activities created in the CLI should use this <see cref="ActivitySource"/>, to allow
/// consumers to easily filter and trace CLI activities.
/// </summary>
public static ActivitySource Source { get; } = new("dotnet-cli", Product.Version);

/// <summary>
/// The environment variable used to transfer the chain of parent activity IDs.
/// This should be used when constructing new sub-processes in order to
/// track spans across calls.
/// </summary>
public const string TRACEPARENT = nameof(TRACEPARENT);
/// <summary>
/// The environment variable used to transfer the trace state of the parent activities.
/// This should be used when constructing new sub-processes in order to
/// track spans across calls.
/// </summary>
public const string TRACESTATE = nameof(TRACESTATE);
Comment on lines +20 to +31
Copy link
Contributor

Choose a reason for hiding this comment

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

TRACEPARENT and TRACESTATE are added in #50163 too. I'd prefer having them added in only one pull request.

Do you intend to insert any new entries into Activity.TraceStateString, or just pass it through unchanged?

}
22 changes: 19 additions & 3 deletions src/Cli/dotnet/CommonOptionsFactory.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable disable

using System.CommandLine;
using System.CommandLine.Invocation;
using Microsoft.DotNet.Cli.Utils;

namespace Microsoft.DotNet.Cli;

Expand All @@ -19,6 +19,22 @@ internal static class CommonOptionsFactory
{
Description = CliStrings.SDKDiagnosticsCommandDefinition,
Recursive = recursive,
Arity = ArgumentArity.Zero
Arity = ArgumentArity.Zero,
Action = new SetDiagnosticModeAction()
};

/// <summary>
/// Sets a few verbose diagnostics flags across the CLI.
/// Other commands may also use this to set their verbosity flags to a higher value or similar behaviors.
/// </summary>
internal class SetDiagnosticModeAction() : SynchronousCommandLineAction
{
public override int Invoke(ParseResult parseResult)
{
Environment.SetEnvironmentVariable(CommandLoggingContext.Variables.Verbose, bool.TrueString);
CommandLoggingContext.SetVerbose(true);
Reporter.Reset();
return 0;
}
}
}
55 changes: 18 additions & 37 deletions src/Cli/dotnet/Extensions/ParseResultExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,17 @@ namespace Microsoft.DotNet.Cli.Extensions;

public static class ParseResultExtensions
{
///<summary>
/// <summary>
/// Finds the command of the parse result and invokes help for that command.
/// If no command is specified, invokes help for the application.
///<summary>
///<remarks>
/// <summary>
/// <remarks>
/// This is accomplished by finding a set of tokens that should be valid and appending a help token
/// to that list, then re-parsing the list of tokens. This is not ideal - either we should have a direct way
/// of invoking help for a ParseResult, or we should eliminate this custom, ad-hoc help invocation by moving
/// more situations that want to show help into Parsing Errors (which trigger help in the default System.CommandLine pipeline)
/// or custom Invocation Middleware, so we can more easily create our version of a HelpResult type.
///</remarks>
/// </remarks>
public static void ShowHelp(this ParseResult parseResult)
{
// take from the start of the list until we hit an option/--/unparsed token
Expand Down Expand Up @@ -56,14 +56,17 @@ public static void ShowHelpOrErrorIfAppropriate(this ParseResult parseResult)
}
}

///<summary>Splits a .NET format string by the format placeholders (the {N} parts) to get an array of the literal parts, to be used in message-checking</summary>
/// <summary>
/// Splits a .NET format string by the format placeholders (the {N} parts) to get an array of the literal parts, to be used in message-checking.
/// </summary>
static string[] DistinctFormatStringParts(string formatString)
{
return Regex.Split(formatString, @"{[0-9]+}"); // match the literal '{', followed by any of 0-9 one or more times, followed by the literal '}'
}


/// <summary>given a string and a series of parts, ensures that all parts are present in the string in sequential order</summary>
/// <summary>
/// Given a string and a series of parts, ensures that all parts are present in the string in sequential order.
/// </summary>
static bool ErrorContainsAllParts(ReadOnlySpan<char> error, string[] parts)
{
foreach (var part in parts)
Expand All @@ -85,9 +88,12 @@ static bool ErrorContainsAllParts(ReadOnlySpan<char> error, string[] parts)

public static string RootSubCommandResult(this ParseResult parseResult)
{
return parseResult.RootCommandResult.Children?
.Select(child => GetSymbolResultValue(parseResult, child))
.FirstOrDefault(subcommand => !string.IsNullOrEmpty(subcommand)) ?? string.Empty;
CommandResult commandResult = parseResult.CommandResult;
while (commandResult != parseResult.RootCommandResult && commandResult.Parent is CommandResult parentCommand)
{
commandResult = parentCommand;
}
return commandResult.Command.Name;
}

public static bool IsDotnetBuiltInCommand(this ParseResult parseResult)
Expand All @@ -101,12 +107,7 @@ public static bool IsTopLevelDotnetCommand(this ParseResult parseResult)
return parseResult.CommandResult.Command.Equals(Parser.RootCommand) && string.IsNullOrEmpty(parseResult.RootSubCommandResult());
}

public static bool CanBeInvoked(this ParseResult parseResult)
{
return Parser.GetBuiltInCommand(parseResult.RootSubCommandResult()) != null ||
parseResult.Tokens.Any(token => token.Type == TokenType.Directive) ||
(parseResult.IsTopLevelDotnetCommand() && string.IsNullOrEmpty(parseResult.GetValue(Parser.DotnetSubCommand)));
}
public static bool CanBeInvoked(this ParseResult parseResult) => parseResult.Action is not null;

public static int HandleMissingCommand(this ParseResult parseResult)
{
Expand Down Expand Up @@ -163,34 +164,14 @@ public static bool DiagOptionPrecedesSubcommand(this string[] args, string subCo
return false;
}

private static string? GetSymbolResultValue(ParseResult parseResult, SymbolResult symbolResult) => symbolResult switch
{
CommandResult commandResult => commandResult.Command.Name,
ArgumentResult argResult => argResult.Tokens.FirstOrDefault()?.Value,
_ => parseResult.GetResult(Parser.DotnetSubCommand)?.GetValueOrDefault<string>()
};

public static bool BothArchAndOsOptionsSpecified(this ParseResult parseResult) =>
(parseResult.HasOption(CommonOptions.ArchitectureOption) ||
parseResult.HasOption(CommonOptions.LongFormArchitectureOption)) &&
parseResult.HasOption(CommonOptions.OperatingSystemOption);

internal static string? GetCommandLineRuntimeIdentifier(this ParseResult parseResult)
{
return parseResult.HasOption(CommonOptions.RuntimeOptionName) ?
parseResult.GetValue<string>(CommonOptions.RuntimeOptionName) :
parseResult.HasOption(CommonOptions.OperatingSystemOption) ||
parseResult.HasOption(CommonOptions.ArchitectureOption) ||
parseResult.HasOption(CommonOptions.LongFormArchitectureOption) ?
CommonOptions.ResolveRidShorthandOptionsToRuntimeIdentifier(
parseResult.GetValue(CommonOptions.OperatingSystemOption),
CommonOptions.ArchOptionValue(parseResult)) :
null;
}

public static bool UsingRunCommandShorthandProjectOption(this ParseResult parseResult)
{
if (parseResult.HasOption(RunCommandParser.PropertyOption) && parseResult.GetValue(RunCommandParser.PropertyOption)!.Any())
if (parseResult.HasOption(RunCommandParser.PropertyOption) && (parseResult.GetValue(RunCommandParser.PropertyOption)?.Any() ?? false))
{
var projVals = parseResult.GetRunCommandShorthandProjectValues();
if (projVals?.Any() is true)
Expand Down
69 changes: 44 additions & 25 deletions src/Cli/dotnet/Parser.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable disable

using System.CommandLine;
using System.CommandLine.Completions;
using System.CommandLine.Invocation;
Expand Down Expand Up @@ -96,14 +94,44 @@ public static class Parser

public static readonly Option<bool> VersionOption = new("--version")
{
Arity = ArgumentArity.Zero
Arity = ArgumentArity.Zero,
Action = new PrintVersionAction()
};

internal class PrintVersionAction : SynchronousCommandLineAction
{
public PrintVersionAction()
{
Terminating = true;
}

public override int Invoke(ParseResult parseResult)
{
CommandLineInfo.PrintVersion();
return 0;
}
}

public static readonly Option<bool> InfoOption = new("--info")
{
Arity = ArgumentArity.Zero
Arity = ArgumentArity.Zero,
Action = new PrintInfoAction()
};

internal class PrintInfoAction : SynchronousCommandLineAction
{
public PrintInfoAction()
{
Terminating = true;
}

public override int Invoke(ParseResult parseResult)
{
CommandLineInfo.PrintInfo();
return 0;
}
}

public static readonly Option<bool> ListSdksOption = new("--list-sdks")
{
Arity = ArgumentArity.Zero
Expand Down Expand Up @@ -169,31 +197,22 @@ private static RootCommand ConfigureCommandLine(RootCommand rootCommand)

rootCommand.SetAction(parseResult =>
{
if (parseResult.GetValue(DiagOption) && parseResult.Tokens.Count == 1)
{
// when user does not specify any args except of diagnostics ("dotnet -d"), we do nothing
// as Program.ProcessArgs already enabled the diagnostic output
return 0;
}
else
{
// when user does not specify any args (just "dotnet"), a usage needs to be printed
parseResult.InvocationConfiguration.Output.WriteLine(CliUsage.HelpText);
return 0;
}
// when user does not specify any args (just "dotnet"), a usage needs to be printed
parseResult.InvocationConfiguration.Output.WriteLine(CliUsage.HelpText);
return 0;
});

return rootCommand;
}

public static Command GetBuiltInCommand(string commandName) =>
public static Command? GetBuiltInCommand(string commandName) =>
Subcommands.FirstOrDefault(c => c.Name.Equals(commandName, StringComparison.OrdinalIgnoreCase));

/// <summary>
/// Implements token-per-line response file handling for the CLI. We use this instead of the built-in S.CL handling
/// to ensure backwards-compatibility with MSBuild.
/// </summary>
public static bool TokenPerLine(string tokenToReplace, out IReadOnlyList<string> replacementTokens, out string errorMessage)
public static bool TokenPerLine(string tokenToReplace, out IReadOnlyList<string>? replacementTokens, out string? errorMessage)
{
var filePath = Path.GetFullPath(tokenToReplace);
if (File.Exists(filePath))
Expand Down Expand Up @@ -256,8 +275,7 @@ public static bool TokenPerLine(string tokenToReplace, out IReadOnlyList<string>
public static int Invoke(string[] args) => Invoke(Parse(args));
public static Task<int> InvokeAsync(string[] args, CancellationToken cancellationToken = default) => InvokeAsync(Parse(args), cancellationToken);


internal static int ExceptionHandler(Exception exception, ParseResult parseResult)
internal static int ExceptionHandler(Exception? exception, ParseResult parseResult)
{
if (exception is TargetInvocationException)
{
Expand All @@ -277,13 +295,13 @@ internal static int ExceptionHandler(Exception exception, ParseResult parseResul
exception.Message.Red().Bold());
parseResult.ShowHelp();
}
else if (exception.GetType().Name.Equals("WorkloadManifestCompositionException"))
else if (exception is not null && exception.GetType().Name.Equals("WorkloadManifestCompositionException"))
{
Reporter.Error.WriteLine(CommandLoggingContext.IsVerbose ?
exception.ToString().Red().Bold() :
exception.Message.Red().Bold());
}
else
else if (exception is not null)
{
Reporter.Error.Write("Unhandled exception: ".Red().Bold());
Reporter.Error.WriteLine(CommandLoggingContext.IsVerbose ?
Expand Down Expand Up @@ -371,7 +389,7 @@ public override void Write(HelpContext context)
}
else if (command.Name.Equals(FormatCommandParser.GetCommand().Name))
{
var arguments = context.ParseResult.GetValue(FormatCommandParser.Arguments);
var arguments = context.ParseResult.GetValue(FormatCommandParser.Arguments) ?? [];
new FormatForwardingApp([.. arguments, .. helpArgs]).Execute();
}
else if (command.Name.Equals(FsiCommandParser.GetCommand().Name))
Expand All @@ -398,9 +416,9 @@ public override void Write(HelpContext context)
{
if (command.Name.Equals(ListReferenceCommandParser.GetCommand().Name))
{
Command listCommand = command.Parents.Single() as Command;
Command? listCommand = command.Parents.Single() as Command;

for (int i = 0; i < listCommand.Arguments.Count; i++)
for (int i = 0; i < listCommand?.Arguments.Count; i++)
{
if (listCommand.Arguments[i].Name == CliStrings.SolutionOrProjectArgumentName)
{
Expand Down Expand Up @@ -431,6 +449,7 @@ internal PrintCliSchemaAction()
{
Terminating = true;
}

public override int Invoke(ParseResult parseResult)
{
CliSchema.PrintCliSchema(parseResult.CommandResult, parseResult.InvocationConfiguration.Output, Program.TelemetryClient);
Expand Down
14 changes: 6 additions & 8 deletions src/Cli/dotnet/PerformanceLogEventSource.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable disable

using System.Diagnostics;
using System.Diagnostics.Tracing;
using System.Reflection;
Expand All @@ -21,7 +19,7 @@ private PerformanceLogEventSource()
}

[NonEvent]
internal void LogStartUpInformation(PerformanceLogStartupInformation startupInfo)
internal void LogStartUpInformation(PerformanceLogStartupInformation? startupInfo)
{
if (!IsEnabled())
{
Expand All @@ -33,7 +31,7 @@ internal void LogStartUpInformation(PerformanceLogStartupInformation startupInfo

LogMachineConfiguration();
OSInfo(RuntimeEnvironment.OperatingSystem, RuntimeEnvironment.OperatingSystemVersion, RuntimeEnvironment.OperatingSystemPlatform.ToString());
SDKInfo(Product.Version, commitSha, RuntimeInformation.RuntimeIdentifier, versionFile.BuildRid, AppContext.BaseDirectory);
SDKInfo(Product.Version, commitSha, RuntimeInformation.RuntimeIdentifier, versionFile.BuildRid!, AppContext.BaseDirectory);
EnvironmentInfo(Environment.CommandLine);
LogMemoryConfiguration();
LogDrives();
Expand All @@ -44,7 +42,7 @@ internal void LogStartUpInformation(PerformanceLogStartupInformation startupInfo
{
if (startupInfo.TimedAssembly != null)
{
AssemblyLoad(startupInfo.TimedAssembly.GetName().Name, startupInfo.AssemblyLoadTime.TotalMilliseconds);
AssemblyLoad(startupInfo.TimedAssembly.GetName().Name!, startupInfo.AssemblyLoadTime.TotalMilliseconds);
}

Process currentProcess = Process.GetCurrentProcess();
Expand Down Expand Up @@ -312,7 +310,7 @@ public PerformanceLogStartupInformation(DateTime mainTimeStamp)
}

internal DateTime MainTimeStamp { get; private set; }
internal Assembly TimedAssembly { get; private set; }
internal Assembly? TimedAssembly { get; private set; }
internal TimeSpan AssemblyLoadTime { get; private set; }

private void MeasureModuleLoad()
Expand All @@ -323,7 +321,7 @@ private void MeasureModuleLoad()
{
foreach (Assembly loadedAssembly in AppDomain.CurrentDomain.GetAssemblies())
{
if (loadedAssembly.GetName().Name.Equals(assemblyName))
if (loadedAssembly.GetName().Name is string n && n.Equals(assemblyName))
{
// If the assembly is already loaded, then bail.
return;
Expand Down Expand Up @@ -426,7 +424,7 @@ private void Initialize()
{
using (StreamReader reader = new(File.OpenRead("/proc/meminfo")))
{
string line;
string? line;
while (!Valid && ((line = reader.ReadLine()) != null))
{
if (line.StartsWith(MemTotal) || line.StartsWith(MemAvailable))
Expand Down
Loading
Loading