Skip to content

update System.CommandLine and adopt to breaking changes #78577

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Nov 21, 2022
Merged

update System.CommandLine and adopt to breaking changes #78577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4f73db4
update System.CommandLine version
adamsitnik Nov 18, 2022
b142cbc
adopt to recent System.CommandLine breaking changes
adamsitnik Nov 18, 2022
6e7e0f4
missing fixes?
adamsitnik Nov 21, 2022
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
2 changes: 1 addition & 1 deletion eng/Versions.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@
<optimizationPGOCoreCLRVersion>1.0.0-prerelease.22511.6</optimizationPGOCoreCLRVersion>
<!-- Not auto-updated. -->
<MicrosoftDiaSymReaderNativeVersion>16.9.0-beta1.21055.5</MicrosoftDiaSymReaderNativeVersion>
<SystemCommandLineVersion>2.0.0-beta4.22355.1</SystemCommandLineVersion>
<SystemCommandLineVersion>2.0.0-beta4.22564.1</SystemCommandLineVersion>
Copy link
Member

@jkotas jkotas Nov 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

System.CommandLine is also listed in https://github.com/dotnet/runtime/blob/main/eng/Version.Details.xml#L83-L86 . Do we need to update it there as well?

Also, it does not sound right that this is in the "Not auto-updated." block. I would think that everything in Version.Details.xml is auto-updated.

Copy link
Member

@jkoritzinsky jkoritzinsky Nov 19, 2022

Choose a reason for hiding this comment

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

We don't have a subscription set up to auto-update System.CommandLine, and as a result the entry in Version.Details.xml doesn't actually do anything. We can add a subscription though and make this auto-update.

Copy link
Member

@jkotas jkotas Nov 19, 2022

Choose a reason for hiding this comment

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

We can add a subscription though and make this auto-update.

@jkoritzinsky Could you please add it?

I see that dotnet/sdk has a System.CommandLine subscription (dotnet/sdk#29131). We need to be on a plan where all repos contributing to a product have a subscription for a thing, or none have a subscription. Otherwise, we will end up with build breaks in source-build leg when the bits meet in the installer.

Copy link
Member

@jkoritzinsky jkoritzinsky Nov 19, 2022

Choose a reason for hiding this comment

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

After this PR is merged, I'll create and enable a subscription for System.CommandLine.

Copy link
Member

@am11 am11 Nov 19, 2022

Choose a reason for hiding this comment

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

subscription for System.CommandLine.

Btw, it's pulled from dotnet-libraries feed in NuGet.config at root.

Copy link
Member Author

@adamsitnik adamsitnik Nov 21, 2022

Choose a reason for hiding this comment

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

Could you please add it?

We are definitely going to do that, but when the time comes. Currently we are merging too many small breaking changes into S.CL. We would need to update SDK/runtime once a day to account for that.

My current plan is to gather a big batch of breaking changes (there will be a blocked PR in the SDK repo with dependencies update), get the first part of S.CL API approved by the API Review Board and then merge the changes into SDK and runtime.

Copy link
Member

@jkotas jkotas Nov 21, 2022

Choose a reason for hiding this comment

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

Ok, we need to disable the System.CommandLine subscription in dotnet/sdk then. Otherwise, there are going to be regular build breaks in dotnet/sdk and dotnet/installer.

Copy link
Member

@MichaelSimons MichaelSimons Nov 21, 2022

Choose a reason for hiding this comment

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

At one time S.CL was also flowing into Templating. Please make sure any other subscriptions are also disabled.

<TraceEventVersion>3.0.3</TraceEventVersion>
<NETStandardLibraryRefVersion>2.1.0</NETStandardLibraryRefVersion>
<NetStandardLibraryVersion>2.0.3</NetStandardLibraryVersion>
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/tools/Common/CommandLineHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ public static void MakeReproPackage(string makeReproPath, string outputFilePath,
}

IValueDescriptor descriptor = option;
object val = res.CommandResult.GetValueForOption(option);
object val = res.CommandResult.GetValue(option);
if (val is not null && !(descriptor.HasDefaultValue && descriptor.GetDefaultValue().Equals(val)))
{
if (val is IEnumerable<string> values)
Expand Down Expand Up @@ -213,7 +213,7 @@ public static void MakeReproPackage(string makeReproPath, string outputFilePath,

foreach (var argument in res.CommandResult.Command.Arguments)
{
object val = res.CommandResult.GetValueForArgument(argument);
object val = res.CommandResult.GetValue(argument);
if (val is IEnumerable<string> values)
{
foreach (string optInList in values)
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/tools/ILVerify/ILVerifyRootCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ internal sealed class ILVerifyRootCommand : RootCommand
public Option<string[]> Include { get; } =
new(new[] { "--include", "-i" }, "Use only methods/types/namespaces, which match the given regular expression(s)");
public Option<FileInfo> IncludeFile { get; } =
new Option<FileInfo>(new[] { "--include-file" }, "Same as --include, but the regular expression(s) are declared line by line in the specified file.").ExistingOnly();
new Option<FileInfo>(new[] { "--include-file" }, "Same as --include, but the regular expression(s) are declared line by line in the specified file.").AcceptExistingOnly();
public Option<string[]> Exclude { get; } =
new(new[] { "--exclude", "-e" }, "Skip methods/types/namespaces, which match the given regular expression(s)");
public Option<FileInfo> ExcludeFile { get; } =
new Option<FileInfo>(new[] { "--exclude-file" }, "Same as --exclude, but the regular expression(s) are declared line by line in the specified file.").ExistingOnly();
new Option<FileInfo>(new[] { "--exclude-file" }, "Same as --exclude, but the regular expression(s) are declared line by line in the specified file.").AcceptExistingOnly();
public Option<string[]> IgnoreError { get; } =
new(new[] { "--ignore-error", "-g" }, "Ignore errors, which match the given regular expression(s)");
public Option<FileInfo> IgnoreErrorFile { get; } =
new Option<FileInfo>(new[] { "--ignore-error-file" }, "Same as --ignore-error, but the regular expression(s) are declared line by line in the specified file.").ExistingOnly();
new Option<FileInfo>(new[] { "--ignore-error-file" }, "Same as --ignore-error, but the regular expression(s) are declared line by line in the specified file.").AcceptExistingOnly();
public Option<bool> Statistics { get; } =
new(new[] { "--statistics" }, "Print verification statistics");
public Option<bool> Verbose { get; } =
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/tools/ILVerify/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,8 +450,8 @@ public PEReader Resolve(string simpleName)
return null;
}

private T Get<T>(Option<T> option) => _command.Result.GetValueForOption(option);
private T Get<T>(Argument<T> argument) => _command.Result.GetValueForArgument(argument);
private T Get<T>(Option<T> option) => _command.Result.GetValue(option);
private T Get<T>(Argument<T> argument) => _command.Result.GetValue(argument);

private static int Main(string[] args) =>
new CommandLineBuilder(new ILVerifyRootCommand())
Expand Down
14 changes: 7 additions & 7 deletions src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,15 +235,15 @@ public ILCompilerRootCommand(string[] args) : base(".NET Native IL Compiler")
{
Result = context.ParseResult;

if (context.ParseResult.GetValueForOption(OptimizeSpace))
if (context.ParseResult.GetValue(OptimizeSpace))
{
OptimizationMode = OptimizationMode.PreferSize;
}
else if (context.ParseResult.GetValueForOption(OptimizeTime))
else if (context.ParseResult.GetValue(OptimizeTime))
{
OptimizationMode = OptimizationMode.PreferSpeed;
}
else if (context.ParseResult.GetValueForOption(Optimize))
else if (context.ParseResult.GetValue(Optimize))
{
OptimizationMode = OptimizationMode.Blended;
}
Expand All @@ -254,15 +254,15 @@ public ILCompilerRootCommand(string[] args) : base(".NET Native IL Compiler")

try
{
string makeReproPath = context.ParseResult.GetValueForOption(MakeReproPath);
string makeReproPath = context.ParseResult.GetValue(MakeReproPath);
if (makeReproPath != null)
{
// Create a repro package in the specified path
// This package will have the set of input files needed for compilation
// + the original command line arguments
// + a rsp file that should work to directly run out of the zip file

Helpers.MakeReproPackage(makeReproPath, context.ParseResult.GetValueForOption(OutputFilePath), args,
Helpers.MakeReproPackage(makeReproPath, context.ParseResult.GetValue(OutputFilePath), args,
context.ParseResult, new[] { "r", "reference", "m", "mibc", "rdxml", "directpinvokelist", "descriptor" });
}

Expand Down Expand Up @@ -290,9 +290,9 @@ public ILCompilerRootCommand(string[] args) : base(".NET Native IL Compiler")
});
}

public static IEnumerable<HelpSectionDelegate> GetExtendedHelp(HelpContext _)
public static IEnumerable<Action<HelpContext>> GetExtendedHelp(HelpContext _)
{
foreach (HelpSectionDelegate sectionDelegate in HelpBuilder.Default.GetLayout())
foreach (Action<HelpContext> sectionDelegate in HelpBuilder.Default.GetLayout())
yield return sectionDelegate;

yield return _ =>
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/tools/aot/ILCompiler/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public int Run()
// typeSystemContext.InputFilePaths = _command.Result.GetValueForArgument(inputFilePaths);
//
Dictionary<string, string> inputFilePaths = new Dictionary<string, string>();
foreach (var inputFile in _command.Result.GetValueForArgument(_command.InputFilePaths))
foreach (var inputFile in _command.Result.GetValue(_command.InputFilePaths))
{
try
{
Expand Down Expand Up @@ -652,7 +652,7 @@ private static IEnumerable<int> ProcessWarningCodes(IEnumerable<string> warningC
}
}

private T Get<T>(Option<T> option) => _command.Result.GetValueForOption(option);
private T Get<T>(Option<T> option) => _command.Result.GetValue(option);

private static int Main(string[] args) =>
new CommandLineBuilder(new ILCompilerRootCommand(args))
Expand Down
18 changes: 9 additions & 9 deletions src/coreclr/tools/aot/crossgen2/Crossgen2RootCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,16 +250,16 @@ public Crossgen2RootCommand(string[] args) : base(SR.Crossgen2BannerText)
this.SetHandler(context =>
{
Result = context.ParseResult;
CompositeOrInputBubble = context.ParseResult.GetValueForOption(Composite) | context.ParseResult.GetValueForOption(InputBubble);
if (context.ParseResult.GetValueForOption(OptimizeSpace))
CompositeOrInputBubble = context.ParseResult.GetValue(Composite) | context.ParseResult.GetValue(InputBubble);
if (context.ParseResult.GetValue(OptimizeSpace))
{
OptimizationMode = OptimizationMode.PreferSize;
}
else if (context.ParseResult.GetValueForOption(OptimizeTime))
else if (context.ParseResult.GetValue(OptimizeTime))
{
OptimizationMode = OptimizationMode.PreferSpeed;
}
else if (context.ParseResult.GetValueForOption(Optimize))
else if (context.ParseResult.GetValue(Optimize))
{
OptimizationMode = OptimizationMode.Blended;
}
Expand All @@ -270,23 +270,23 @@ public Crossgen2RootCommand(string[] args) : base(SR.Crossgen2BannerText)

try
{
int alignment = context.ParseResult.GetValueForOption(CustomPESectionAlignment);
int alignment = context.ParseResult.GetValue(CustomPESectionAlignment);
if (alignment != 0)
{
// Must be a power of two and >= 4096
if (alignment < 4096 || (alignment & (alignment - 1)) != 0)
throw new CommandLineException(SR.InvalidCustomPESectionAlignment);
}

string makeReproPath = context.ParseResult.GetValueForOption(MakeReproPath);
string makeReproPath = context.ParseResult.GetValue(MakeReproPath);
if (makeReproPath != null)
{
// Create a repro package in the specified path
// This package will have the set of input files needed for compilation
// + the original command line arguments
// + a rsp file that should work to directly run out of the zip file

Helpers.MakeReproPackage(makeReproPath, context.ParseResult.GetValueForOption(OutputFilePath), args,
Helpers.MakeReproPackage(makeReproPath, context.ParseResult.GetValue(OutputFilePath), args,
context.ParseResult, new[] { "r", "reference", "u", "unrooted-input-file-paths", "m", "mibc", "inputbubbleref" });
}

Expand Down Expand Up @@ -314,9 +314,9 @@ public Crossgen2RootCommand(string[] args) : base(SR.Crossgen2BannerText)
});
}

public static IEnumerable<HelpSectionDelegate> GetExtendedHelp(HelpContext _)
public static IEnumerable<Action<HelpContext>> GetExtendedHelp(HelpContext _)
{
foreach (HelpSectionDelegate sectionDelegate in HelpBuilder.Default.GetLayout())
foreach (Action<HelpContext> sectionDelegate in HelpBuilder.Default.GetLayout())
yield return sectionDelegate;

yield return _ =>
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/tools/aot/crossgen2/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public int Run()
ConfigureImageBase(targetDetails);

bool versionBubbleIncludesCoreLib = false;
Dictionary<string, string> inputFilePathsArg = _command.Result.GetValueForArgument(_command.InputFilePaths);
Dictionary<string, string> inputFilePathsArg = _command.Result.GetValue(_command.InputFilePaths);
Dictionary<string, string> unrootedInputFilePathsArg = Get(_command.UnrootedInputFilePaths);

if (_inputBubble)
Expand Down Expand Up @@ -886,7 +886,7 @@ internal static bool IsValidPublicKey(byte[] blob)
return true;
}

private T Get<T>(Option<T> option) => _command.Result.GetValueForOption(option);
private T Get<T>(Option<T> option) => _command.Result.GetValue(option);

private static int Main(string[] args) =>
new CommandLineBuilder(new Crossgen2RootCommand(args))
Expand Down
12 changes: 6 additions & 6 deletions src/coreclr/tools/dotnet-pgo/PgoRootCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,14 +170,14 @@ public PgoRootCommand(string[] args) : base(".NET PGO Tool")
FileType = PgoFileType.jittrace;
ProcessJitEvents = true;
ValidateOutputFile = false;
ProcessR2REvents = context.ParseResult.GetValueForOption(_includeReadyToRun);
ProcessR2REvents = context.ParseResult.GetValue(_includeReadyToRun);

if (context.ParseResult.GetValueForOption(_isSorted))
if (context.ParseResult.GetValue(_isSorted))
{
JitTraceOptions |= JitTraceOptions.sorted;
}

if (context.ParseResult.GetValueForOption(_showTimestamp))
if (context.ParseResult.GetValue(_showTimestamp))
{
JitTraceOptions |= JitTraceOptions.showtimestamp;
}
Expand Down Expand Up @@ -243,7 +243,7 @@ void TryExecuteWithContext(InvocationContext context, bool setVerbosity)

if (setVerbosity)
{
Verbosity verbosity = context.ParseResult.GetValueForOption(_verbosity);
Verbosity verbosity = context.ParseResult.GetValue(_verbosity);
BasicProgressMessages = (int)verbosity >= (int)Verbosity.normal;
Warnings = (int)verbosity >= (int)Verbosity.normal;
VerboseWarnings = (int)verbosity >= (int)Verbosity.detailed;
Expand All @@ -270,9 +270,9 @@ void TryExecuteWithContext(InvocationContext context, bool setVerbosity)
}
}

public static IEnumerable<HelpSectionDelegate> GetExtendedHelp(HelpContext context)
public static IEnumerable<Action<HelpContext>> GetExtendedHelp(HelpContext context)
{
foreach (HelpSectionDelegate sectionDelegate in HelpBuilder.Default.GetLayout())
foreach (Action<HelpContext> sectionDelegate in HelpBuilder.Default.GetLayout())
yield return sectionDelegate;

if (context.Command.Name == "create-mibc" || context.Command.Name == "create-jittrace")
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/tools/dotnet-pgo/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,8 @@ public Program(PgoRootCommand command)
_inputFilesToCompare = Get(command.InputFilesToCompare);
}

private T Get<T>(Option<T> option) => _command.Result.GetValueForOption(option);
private T Get<T>(Argument<T> argument) => _command.Result.GetValueForArgument(argument);
private T Get<T>(Option<T> option) => _command.Result.GetValue(option);
private T Get<T>(Argument<T> argument) => _command.Result.GetValue(argument);
private bool IsSet<T>(Option<T> option) => _command.Result.FindResultFor(option) != null;

private static int Main(string[] args) =>
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tools/r2rdump/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ public int Run()
return 0;
}

private T Get<T>(Option<T> option) => _command.Result.GetValueForOption(option);
private T Get<T>(Option<T> option) => _command.Result.GetValue(option);

public static int Main(string[] args) =>
new CommandLineBuilder(new R2RDumpRootCommand())
Expand Down
Loading