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

Adapt System.CommandLine (v2) in more places #72082

Merged
merged 18 commits into from
Sep 23, 2022
Merged
Show file tree
Hide file tree
Changes from 7 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
289 changes: 289 additions & 0 deletions src/coreclr/tools/Common/CommandLineHelpers.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,289 @@
// Licensed to the .NET Foundation under one or more agreements.
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 code is derived from Common/CommandLine/CommandLineHelpers.cs. The main difference is that it doesn't depend on anything from Common/CommandLine/* and LINQ. The MakeReproPackage method in this file uses System.CommandLine's ParseResult instead of Internal.CommandLine's ArgumentSyntax.

In a follow-up PR, I will delete src/coreclr/tools/Common/CommandLine/* after switching dotnet-pgo (its last consumer) to System.CommandLine. ILVerify will also be switched to use this helper during that work (without the need for <DefineConstants>ILVerify, #if ILVERIFY).

// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.CommandLine.Binding;
using System.CommandLine.Parsing;
using System.IO;
using System.IO.Compression;
using System.Runtime.InteropServices;

using Internal.TypeSystem;

namespace System.CommandLine;

internal class CommandLineException : Exception
{
public CommandLineException(string message) : base(message) { }
}

//
// Helpers for command line processing
//
internal static class Helpers
{
public const string DefaultSystemModule = "System.Private.CoreLib";

public static Dictionary<string, string> BuildPathDictionay(IReadOnlyList<Token> tokens, bool strict)
{
Dictionary<string, string> dictionary = new(StringComparer.OrdinalIgnoreCase);

foreach (Token token in tokens)
{
AppendExpandedPaths(dictionary, token.Value, strict);
}

return dictionary;
}

public static TargetOS GetTargetOS(string token)
{
if(string.IsNullOrEmpty(token))
{
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
return Internal.TypeSystem.TargetOS.Windows;
else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux))
return Internal.TypeSystem.TargetOS.Linux;
else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
return Internal.TypeSystem.TargetOS.OSX;
else if (RuntimeInformation.IsOSPlatform(OSPlatform.FreeBSD))
return Internal.TypeSystem.TargetOS.FreeBSD;

throw new NotImplementedException();
}

if (token.Equals("windows", StringComparison.OrdinalIgnoreCase))
return Internal.TypeSystem.TargetOS.Windows;
else if (token.Equals("linux", StringComparison.OrdinalIgnoreCase))
return Internal.TypeSystem.TargetOS.Linux;
else if (token.Equals("osx", StringComparison.OrdinalIgnoreCase))
return Internal.TypeSystem.TargetOS.OSX;

throw new CommandLineException($"Target OS '{token}' is not supported");
}

public static TargetArchitecture GetTargetArchitecture(string token)
{
if(string.IsNullOrEmpty(token))
{
return RuntimeInformation.ProcessArchitecture switch
{
Architecture.X86 => Internal.TypeSystem.TargetArchitecture.X86,
Architecture.X64 => Internal.TypeSystem.TargetArchitecture.X64,
Architecture.Arm => Internal.TypeSystem.TargetArchitecture.ARM,
Architecture.Arm64 => Internal.TypeSystem.TargetArchitecture.ARM64,
_ => throw new NotImplementedException()
};
}

if (token.Equals("x86", StringComparison.OrdinalIgnoreCase))
return Internal.TypeSystem.TargetArchitecture.X86;
else if (token.Equals("x64", StringComparison.OrdinalIgnoreCase))
return Internal.TypeSystem.TargetArchitecture.X64;
else if (token.Equals("arm", StringComparison.OrdinalIgnoreCase))
return Internal.TypeSystem.TargetArchitecture.ARM;
else if (token.Equals("arm64", StringComparison.OrdinalIgnoreCase))
return Internal.TypeSystem.TargetArchitecture.ARM64;
else if (token.Equals("armel", StringComparison.OrdinalIgnoreCase))
return Internal.TypeSystem.TargetArchitecture.ARMEL;

throw new CommandLineException($"Target architecture '{token}' is not supported");
}

public static void MakeReproPackage(string makeReproPath, string outputFilePath, string[] args, ParseResult res, IEnumerable<string> inputOptions)
{
Directory.CreateDirectory(makeReproPath);

List<string> details = new List<string>();
details.Add("Tool version");
try
{
details.Add(Environment.GetCommandLineArgs()[0]);
}
catch { }
try
{
details.Add(System.Diagnostics.FileVersionInfo.GetVersionInfo(Environment.GetCommandLineArgs()[0]).ToString());
}
catch { }

details.Add("------------------------");
details.Add("Actual Command Line Args");
details.Add("------------------------");
details.AddRange(args);
foreach (string arg in args)
{
if (arg.StartsWith('@'))
{
string rspFileName = arg.Substring(1);
details.Add("------------------------");
details.Add(rspFileName);
details.Add("------------------------");
try
{
details.AddRange(File.ReadAllLines(rspFileName));
}
catch { }
}
}

HashCode hashCodeOfArgs = new HashCode();
foreach (string s in details)
hashCodeOfArgs.Add(s);

string zipFileName = ((uint)hashCodeOfArgs.ToHashCode()).ToString();

if (outputFilePath != null)
zipFileName = zipFileName + "_" + Path.GetFileName(outputFilePath);

zipFileName = Path.Combine(makeReproPath, Path.ChangeExtension(zipFileName, ".zip"));

Console.WriteLine($"Creating {zipFileName}");
using (var archive = ZipFile.Open(zipFileName, ZipArchiveMode.Create))
{
ZipArchiveEntry commandEntry = archive.CreateEntry("command.txt");
using (StreamWriter writer = new StreamWriter(commandEntry.Open()))
{
foreach (string s in details)
writer.WriteLine(s);
}

HashSet<string> inputOptionNames = new HashSet<string>(inputOptions);
Dictionary<string, string> inputToReproPackageFileName = new();

List<string> rspFile = new List<string>();
foreach (var option in res.CommandResult.Command.Options)
{
if (!res.HasOption(option) || option.Name == "make-repro-path")
{
continue;
}

IValueDescriptor descriptor = option;
object val = res.CommandResult.GetValueForOption(option);
if (val is not null && !(descriptor.HasDefaultValue && descriptor.GetDefaultValue().Equals(val)))
{
if (val is IEnumerable<string> values)
{
if (inputOptionNames.Contains(option.Name))
{
Dictionary<string, string> dictionary = new();
foreach (string optInList in values)
{
Helpers.AppendExpandedPaths(dictionary, optInList, false);
}
foreach (string inputFile in dictionary.Values)
{
rspFile.Add($"--{option.Name}:{ConvertFromInputPathToReproPackagePath(inputFile)}");
}
}
else
{
foreach (string optInList in values)
{
rspFile.Add($"--{option.Name}:{optInList}");
}
}
}
else
{
rspFile.Add($"--{option.Name}:{val}");
}
}
}

foreach (var argument in res.CommandResult.Command.Arguments)
{
object val = res.CommandResult.GetValueForArgument(argument);
if (val is IEnumerable<string> values)
{
foreach (string optInList in values)
{
rspFile.Add($"{ConvertFromInputPathToReproPackagePath((string)optInList)}");
}
}
else
{
rspFile.Add($"{ConvertFromInputPathToReproPackagePath((string)val)}");
}
}

ZipArchiveEntry rspEntry = archive.CreateEntry("repro.rsp");
using (StreamWriter writer = new StreamWriter(rspEntry.Open()))
{
foreach (string s in rspFile)
writer.WriteLine(s);
}

string ConvertFromInputPathToReproPackagePath(string inputPath)
{
if (inputToReproPackageFileName.TryGetValue(inputPath, out string reproPackagePath))
{
return reproPackagePath;
}

try
{
string inputFileDir = inputToReproPackageFileName.Count.ToString();
reproPackagePath = Path.Combine(inputFileDir, Path.GetFileName(inputPath));
archive.CreateEntryFromFile(inputPath, reproPackagePath);
inputToReproPackageFileName.Add(inputPath, reproPackagePath);

return reproPackagePath;
}
catch
{
return inputPath;
}
}
}
}

// Helper to create a collection of paths unique in their simple names.
private static void AppendExpandedPaths(Dictionary<string, string> dictionary, string pattern, bool strict)
{
bool empty = true;
string directoryName = Path.GetDirectoryName(pattern);
string searchPattern = Path.GetFileName(pattern);

if (directoryName == "")
directoryName = ".";

if (Directory.Exists(directoryName))
{
foreach (string fileName in Directory.EnumerateFiles(directoryName, searchPattern))
{
string fullFileName = Path.GetFullPath(fileName);

string simpleName = Path.GetFileNameWithoutExtension(fileName);

if (dictionary.ContainsKey(simpleName))
{
if (strict)
{
throw new CommandLineException("Multiple input files matching same simple name " +
fullFileName + " " + dictionary[simpleName]);
}
}
else
{
dictionary.Add(simpleName, fullFileName);
}

empty = false;
}
}

if (empty)
{
if (strict)
{
throw new CommandLineException("No files matching " + pattern);
}
else
{
Console.WriteLine("Warning: No files matching " + pattern);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ public enum TargetArchitecture
Unknown,
ARM,
ARM64,
ARMEL,
Copy link
Member

Choose a reason for hiding this comment

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

What is this used for?

This would mean that all places the check for TargetArchitecture.ARM have to check for TargetArchitecture.ARMEL too?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is used for (easier) strongly typed binding. In Program.ctor, it overrides ARMEL -> ARM since the rest of the implementation expects it with a flag to indicate if ARMEL ABI is to be used.

Alternatively, I can use the old approach to capture the flag during parsing (it is already using custom step in call to Option<T> ctor for --targetarch).

Copy link
Member

Choose a reason for hiding this comment

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

I think that would be better - since we do not expect this value to be valid, except in command line parsing.

Copy link
Member

Choose a reason for hiding this comment

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

FWIW #43706 (comment) is the previous conversation on why we don't have TargetArchitecture.Armel.

X64,
X86,
Wasm32,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public void TestDependencyGraphInvariants(EcmaMethod method)
new FullyBlockedMetadataBlockingPolicy(), new FullyBlockedManifestResourceBlockingPolicy(),
null, new NoStackTraceEmissionPolicy(), new NoDynamicInvokeThunkGenerationPolicy(),
new ILLink.Shared.TrimAnalysis.FlowAnnotations(Logger.Null, ilProvider), UsageBasedMetadataGenerationOptions.None,
Logger.Null, Array.Empty<KeyValuePair<string, bool>>(), Array.Empty<string>(), Array.Empty<string>());
Logger.Null, Array.Empty<KeyValuePair<string, bool>>(), Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>());

CompilationBuilder builder = new RyuJitCompilationBuilder(context, compilationGroup)
.UseILProvider(ilProvider);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ public UsageBasedMetadataManager(
Logger logger,
IEnumerable<KeyValuePair<string, bool>> featureSwitchValues,
IEnumerable<string> rootEntireAssembliesModules,
IEnumerable<string> additionalRootedAssemblies,
IEnumerable<string> trimmedAssemblies)
: base(typeSystemContext, blockingPolicy, resourceBlockingPolicy, logFile, stackTracePolicy, invokeThunkGenerationPolicy)
{
Expand All @@ -84,6 +85,7 @@ public UsageBasedMetadataManager(
_featureSwitchHashtable = new FeatureSwitchHashtable(new Dictionary<string, bool>(featureSwitchValues));

_rootEntireAssembliesModules = new HashSet<string>(rootEntireAssembliesModules);
_rootEntireAssembliesModules.UnionWith(additionalRootedAssemblies);
_trimmedAssemblies = new HashSet<string>(trimmedAssemblies);
}

Expand Down
21 changes: 2 additions & 19 deletions src/coreclr/tools/aot/ILCompiler/ILCompiler.props
Original file line number Diff line number Diff line change
Expand Up @@ -82,28 +82,11 @@
</ItemGroup>

<ItemGroup>
<Compile Include="..\..\Common\CommandLine\Argument.cs" />
<Compile Include="..\..\Common\CommandLine\Argument_1.cs" />
<Compile Include="..\..\Common\CommandLine\ArgumentCommand.cs" />
<Compile Include="..\..\Common\CommandLine\ArgumentCommand_1.cs" />
<Compile Include="..\..\Common\CommandLine\ArgumentLexer.cs" />
<Compile Include="..\..\Common\CommandLine\ArgumentList_1.cs" />
<Compile Include="..\..\Common\CommandLine\ArgumentParser.cs" />
<Compile Include="..\..\Common\CommandLine\ArgumentSyntax.cs" />
<Compile Include="..\..\Common\CommandLine\ArgumentSyntax_Definers.cs" />
<Compile Include="..\..\Common\CommandLine\ArgumentSyntaxException.cs" />
<Compile Include="..\..\Common\CommandLine\ArgumentToken.cs" />
<Compile Include="..\..\Common\CommandLine\CommandLineException.cs" />
<Compile Include="..\..\Common\CommandLine\CommandLineHelpers.cs" />
<Compile Include="..\..\Common\CommandLine\Enumerable.cs" />
<Compile Include="..\..\Common\CommandLine\HelpTextGenerator.cs" />
<Compile Include="..\..\Common\CommandLineHelpers.cs" />
</ItemGroup>

<ItemGroup>
<EmbeddedResource Include="..\..\Common\CommandLine\Resources\Strings.resx">
<GenerateSource>true</GenerateSource>
<ClassName>Internal.CommandLine.Strings</ClassName>
</EmbeddedResource>
<PackageReference Include="System.CommandLine" Version="$(SystemCommandLineVersion)" />
</ItemGroup>

<ItemGroup>
Expand Down
Loading