Code Review Feedback

This commit is contained in:
Bryan 2015-11-17 19:50:19 -08:00
parent fd471025dc
commit 5db8ae5e6d
14 changed files with 192 additions and 320 deletions

View file

@ -1,124 +0,0 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Runtime.InteropServices;
using System.IO;
using Microsoft.Dnx.Runtime.Common.CommandLine;
using Microsoft.DotNet.Cli.Utils;
using Microsoft.DotNet.Tools.Common;
namespace Microsoft.DotNet.Tools.Compiler.Native
{
public static class Helpers
{
internal static T ParseEnum<T>(string value)
{
return (T)Enum.Parse(typeof(T), value, true);
}
internal static void CleanOrCreateDirectory(string path)
{
if (Directory.Exists(path))
{
try
{
Directory.Delete(path, recursive: true);
}
catch (Exception e)
{
Console.WriteLine("Unable to remove directory: " + path);
Console.WriteLine(e.Message);
}
}
Directory.CreateDirectory(path);
}
internal static ArchitectureMode GetCurrentArchitecture()
{
// TODO Support Architectures other than x64
return ArchitectureMode.x64;
}
internal static OSMode GetCurrentOS()
{
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
return OSMode.Windows;
}
else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
{
return OSMode.Mac;
}
else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux))
{
return OSMode.Linux;
}
else
{
throw new Exception("Unrecognized OS. dotnet-compile-native is compatible with Windows, OSX, and Linux");
}
}
internal static IEnumerable<string> SplitStringCommandLine(string commandLine)
{
bool inQuotes = false;
return commandLine.Split(c =>
{
if (c == '\"')
inQuotes = !inQuotes;
return !inQuotes && c == ' ';
})
.Select(arg => arg.Trim().TrimMatchingQuotes('\"'))
.Where(arg => !string.IsNullOrEmpty(arg));
}
internal static string TrimMatchingQuotes(this string input, char quote)
{
if ((input.Length >= 2) &&
(input[0] == quote) && (input[input.Length - 1] == quote))
return input.Substring(1, input.Length - 2);
return input;
}
public static IEnumerable<string> Split(this string str,
Func<char, bool> controller)
{
int nextPiece = 0;
for (int c = 0; c < str.Length; c++)
{
if (controller(str[c]))
{
yield return str.Substring(nextPiece, c - nextPiece);
nextPiece = c + 1;
}
}
yield return str.Substring(nextPiece);
}
}
internal class ArgValues
{
public string LogPath { get; set; }
public string InputManagedAssemblyPath { get; set; }
public string OutputDirectory { get; set; }
public string IntermediateDirectory { get; set; }
public string BuildType { get; set; }
public string Architecture { get; set; }
public string NativeMode { get; set; }
public List<string> ReferencePaths { get; set; }
public string IlcArgs { get; set; }
public List<string> LinkLibPaths { get; set; }
public string AppDepSDKPath { get; set; }
public string IlcPath { get; set; }
public string RuntimeLibPath { get; set; }
}
}

View file

@ -3,23 +3,22 @@ using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using Microsoft.Dnx.Runtime.Common.CommandLine;
using Microsoft.DotNet.Cli.Utils;
using Microsoft.DotNet.Tools.Common;
namespace Microsoft.DotNet.Tools.Compiler.Native
{
public class ILCompilerInvoker
{
public class ILCompilerInvoker
{
private readonly string ExecutableName = "corerun" + Constants.ExeSuffix;
private readonly string ILCompiler = "ilc.exe";
private static readonly Dictionary<NativeIntermediateMode, string> ModeOutputExtensionMap = new Dictionary<NativeIntermediateMode, string>
{
{ NativeIntermediateMode.cpp, ".cpp" },
{ NativeIntermediateMode.ryujit, ".obj" }
};
{
{ NativeIntermediateMode.cpp, ".cpp" },
{ NativeIntermediateMode.ryujit, ".obj" }
};
private static readonly Dictionary<OSMode, string> OSCoreLibNameMap = new Dictionary<OSMode, string>()
{
@ -28,78 +27,80 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
{OSMode.Mac, "System.Private.Corelib.dll" },
};
private string ArgStr { get; set; }
public ILCompilerInvoker(NativeCompileSettings config)
{
InitializeArgs(config);
}
private void InitializeArgs(NativeCompileSettings config)
{
var argsList = new List<string>();
private string ArgStr { get; set; }
private NativeCompileSettings config;
public ILCompilerInvoker(NativeCompileSettings config)
{
this.config = config;
InitializeArgs(config);
}
private void InitializeArgs(NativeCompileSettings config)
{
var argsList = new List<string>();
var managedPath = Path.Combine(config.IlcPath, ILCompiler);
argsList.Add(managedPath);
// Input File
var inputFilePath = config.InputManagedAssemblyPath;
argsList.Add(inputFilePath);
// System.Private.CoreLib Reference
var coreLibPath = Path.Combine(config.IlcPath, OSCoreLibNameMap[config.OS]);
argsList.Add($"-r \"{coreLibPath}\"");
// Dependency References
foreach (var reference in config.ReferencePaths)
{
argsList.Add($"-r \"{reference}\"");
}
// Set Output DetermineOutFile
var outFile = DetermineOutputFile(config);
argsList.Add($"-out \"{outFile}\"");
// Add Mode Flag TODO
if (config.NativeMode == NativeIntermediateMode.cpp)
{
argsList.Add("-cpp");
}
// Custom Ilc Args support
if (! string.IsNullOrEmpty(config.IlcArgs))
{
argsList.Add(config.IlcArgs);
}
this.ArgStr = string.Join(" ", argsList);
}
public int Invoke(NativeCompileSettings config)
{
var executablePath = Path.Combine(config.IlcPath, ExecutableName);
var result = Command.Create(executablePath, ArgStr)
.ForwardStdErr()
.ForwardStdOut()
.Execute();
return result.ExitCode;
}
public string DetermineOutputFile(NativeCompileSettings config)
{
var intermediateDirectory = config.IntermediateDirectory;
var extension = ModeOutputExtensionMap[config.NativeMode];
var filename = Path.GetFileNameWithoutExtension(config.InputManagedAssemblyPath);
var outFile = Path.Combine(intermediateDirectory, filename + extension);
return outFile;
}
}
// Input File
var inputFilePath = config.InputManagedAssemblyPath;
argsList.Add(inputFilePath);
// System.Private.CoreLib Reference
var coreLibPath = Path.Combine(config.IlcPath, OSCoreLibNameMap[config.OS]);
argsList.Add($"-r \"{coreLibPath}\"");
// Dependency References
foreach (var reference in config.ReferencePaths)
{
argsList.Add($"-r \"{reference}\"");
}
// Set Output DetermineOutFile
var outFile = DetermineOutputFile(config);
argsList.Add($"-out \"{outFile}\"");
// Add Mode Flag TODO
if (config.NativeMode == NativeIntermediateMode.cpp)
{
argsList.Add("-cpp");
}
// Custom Ilc Args support
if (! string.IsNullOrEmpty(config.IlcArgs))
{
argsList.Add(config.IlcArgs);
}
this.ArgStr = string.Join(" ", argsList);
}
public int Invoke()
{
var executablePath = Path.Combine(AppContext.BaseDirectory, ExecutableName);
var result = Command.Create(executablePath, ArgStr)
.ForwardStdErr()
.ForwardStdOut()
.Execute();
return result.ExitCode;
}
public string DetermineOutputFile(NativeCompileSettings config)
{
var intermediateDirectory = config.IntermediateDirectory;
var extension = ModeOutputExtensionMap[config.NativeMode];
var filename = Path.GetFileNameWithoutExtension(config.InputManagedAssemblyPath);
var outFile = Path.Combine(intermediateDirectory, filename + extension);
return outFile;
}
}
}

View file

@ -12,7 +12,7 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
{
public interface IPlatformNativeStep
{
int Invoke(NativeCompileSettings config);
int Invoke();
string DetermineOutputFile(NativeCompileSettings config);
bool CheckPreReqs();
}

View file

@ -96,11 +96,11 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
this.StepList = stepList;
}
public int Invoke(NativeCompileSettings config)
public int Invoke()
{
foreach(var step in StepList)
{
int result = step.Invoke(config);
int result = step.Invoke();
if (result != 0)
{
@ -113,7 +113,7 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
public string DetermineOutputFile(NativeCompileSettings config)
{
return StepList.Last().DetermineOutputFile(config);
return config.DetermineFinalOutputPath();
}
public bool CheckPreReqs()

View file

@ -23,21 +23,27 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
{
"libbootstrappercpp.a",
"libPortableRuntime.a",
"libSystem.Private.CoreLib.Native.a",
"System.Native.so"
"libSystem.Private.CoreLib.Native.a"
};
private readonly string[] appdeplibs = new string[]
{
"libSystem.Native.a"
};
private string CompilerArgStr { get; set; }
private NativeCompileSettings config;
public LinuxCppCompileStep(NativeCompileSettings config)
{
this.config = config;
InitializeArgs(config);
}
public int Invoke(NativeCompileSettings config)
public int Invoke()
{
var result = InvokeCompiler(config);
var result = InvokeCompiler();
if (result != 0)
{
Reporter.Error.WriteLine("Compilation of intermediate files failed.");
@ -66,7 +72,6 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
argsList.Add("-I");
argsList.Add(Path.Combine(config.AppDepSDKPath, "CPPSdk"));
// Input File
var inCppFile = DetermineInFile(config);
argsList.Add(inCppFile);
@ -74,25 +79,30 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
// Add Stubs
argsList.Add(Path.Combine(config.AppDepSDKPath, "CPPSdk/ubuntu.14.04/lxstubs.cpp"));
// Libs
foreach (var lib in libs)
{
var libPath = Path.Combine(config.RuntimeLibPath, lib);
var libPath = Path.Combine(config.IlcPath, lib);
argsList.Add(libPath);
}
// AppDep Libs
foreach (var lib in appdeplibs)
{
var libPath = Path.Combine(config.AppDepSDKPath, lib);
argsList.Add(libPath);
}
argsList.Add(cLibsFlags);
// Output
var libOut = DetermineOutputFile(config);
argsList.Add($"-o \"{libOut}\"");
this.CompilerArgStr = string.Join(" ", argsList);
}
private int InvokeCompiler(NativeCompileSettings config)
private int InvokeCompiler()
{
var result = Command.Create(CompilerName, CompilerArgStr)
.ForwardStdErr()

View file

@ -24,21 +24,27 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
{
"libbootstrapper.a",
"libRuntime.a",
"libSystem.Private.CoreLib.Native.a",
"System.Native.so"
"libSystem.Private.CoreLib.Native.a"
};
private readonly string[] appdeplibs = new string[]
{
"libSystem.Native.a"
};
private string CompilerArgStr { get; set; }
private NativeCompileSettings config;
public LinuxRyuJitCompileStep(NativeCompileSettings config)
{
this.config = config;
InitializeArgs(config);
}
public int Invoke(NativeCompileSettings config)
public int Invoke()
{
var result = InvokeCompiler(config);
var result = InvokeCompiler();
if (result != 0)
{
Reporter.Error.WriteLine("Compilation of intermediate files failed.");
@ -67,7 +73,14 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
// Libs
foreach (var lib in libs)
{
var libPath = Path.Combine(config.RuntimeLibPath, lib);
var libPath = Path.Combine(config.IlcPath, lib);
argsList.Add(libPath);
}
// AppDep Libs
foreach (var lib in appdeplibs)
{
var libPath = Path.Combine(config.AppDepSDKPath, lib);
argsList.Add(libPath);
}
@ -81,7 +94,7 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
this.CompilerArgStr = string.Join(" ", argsList);
}
private int InvokeCompiler(NativeCompileSettings config)
private int InvokeCompiler()
{
var result = Command.Create(CompilerName, CompilerArgStr)
.ForwardStdErr()
@ -124,11 +137,5 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
return outfile;
}
public bool RequiresLinkStep()
{
return false;
}
}
}

View file

@ -17,7 +17,7 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
throw new NotImplementedException("Mac Cpp Not Supported Yet");
}
public int Invoke(NativeCompileSettings config)
public int Invoke()
{
throw new NotImplementedException("mac cpp Not supported yet.");
}

View file

@ -12,7 +12,7 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
{
public class MacLinkStep : IPlatformNativeStep
{
public int Invoke(NativeCompileSettings config)
public int Invoke()
{
throw new NotImplementedException("Mac linker Not supported yet.");
}

View file

@ -27,13 +27,16 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
};
private string CompilerArgStr { get; set; }
private NativeCompileSettings config;
public WindowsCppCompileStep(NativeCompileSettings config)
{
this.config = config;
InitializeArgs(config);
}
public int Invoke(NativeCompileSettings config)
public int Invoke()
{
var result = WindowsCommon.SetVCVars();
if (result != 0)
@ -42,7 +45,7 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
return result;
}
result = InvokeCompiler(config);
result = InvokeCompiler();
if (result != 0)
{
Reporter.Error.WriteLine("Compilation of intermediate files failed.");
@ -85,7 +88,7 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
this.CompilerArgStr = string.Join(" ", argsList);
}
private int InvokeCompiler(NativeCompileSettings config)
private int InvokeCompiler()
{
var vcInstallDir = Environment.GetEnvironmentVariable("VS140COMNTOOLS");
var compilerPath = Path.Combine(vcInstallDir, VSBin, CompilerName);

View file

@ -47,13 +47,15 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
};
private string ArgStr { get; set; }
private NativeCompileSettings config;
public WindowsLinkStep(NativeCompileSettings config)
{
this.config = config;
InitializeArgs(config);
}
public int Invoke(NativeCompileSettings config)
public int Invoke()
{
var result = WindowsCommon.SetVCVars();
if (result != 0)
@ -62,7 +64,7 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
return result;
}
result = InvokeLinker(config);
result = InvokeLinker();
if (result != 0)
{
Reporter.Error.WriteLine("Linking of intermediate files failed.");
@ -94,7 +96,7 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
var SDKLibs = ModeLibMap[config.NativeMode];
foreach (var lib in SDKLibs)
{
argsList.Add(Path.Combine(config.RuntimeLibPath, lib));
argsList.Add(Path.Combine(config.IlcPath, lib));
}
// Link Libs
@ -112,7 +114,7 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
this.ArgStr = string.Join(" ", argsList);
}
private int InvokeLinker(NativeCompileSettings config)
private int InvokeLinker()
{
var vcInstallDir = Environment.GetEnvironmentVariable("VS140COMNTOOLS");
var linkerPath = Path.Combine(vcInstallDir, VSBin, LinkerName);

View file

@ -1,64 +1,53 @@
using System;
using System.Collections.Generic;
using System.IO;
using Microsoft.Dnx.Runtime.Common.CommandLine;
using Microsoft.DotNet.Cli.Utils;
using Microsoft.DotNet.Tools.Common;
namespace Microsoft.DotNet.Tools.Compiler.Native
{
public class NativeCompileSettings
{
public string LogPath { get; set; }
public string InputManagedAssemblyPath { get; set; }
public string OutputDirectory { get; set; }
public string IntermediateDirectory { get; set; }
public BuildConfiguration BuildType { get; set; }
public ArchitectureMode Architecture { get; set; }
public NativeIntermediateMode NativeMode { get; set; }
public OSMode OS { get; set; }
public List<string> ReferencePaths { get; set; }
// Optional Customization Points (Can be null)
public string IlcArgs { get; set; }
public List<string> LinkLibPaths { get; set; }
// Required Customization Points (Must have default)
public string AppDepSDKPath { get; set; }
public string IlcPath { get; set; }
public string RuntimeLibPath { get; set; }
public class NativeCompileSettings
{
public string LogPath { get; set; }
public string InputManagedAssemblyPath { get; set; }
public string OutputDirectory { get; set; }
public string IntermediateDirectory { get; set; }
public BuildConfiguration BuildType { get; set; }
public ArchitectureMode Architecture { get; set; }
public NativeIntermediateMode NativeMode { get; set; }
public OSMode OS { get; set; }
public List<string> ReferencePaths { get; set; }
// Optional Customization Points (Can be null)
public string IlcArgs { get; set; }
public List<string> LinkLibPaths { get; set; }
// Required Customization Points (Must have default)
public string AppDepSDKPath { get; set; }
public string IlcPath { get; set; }
public NativeCompileSettings()
{
LinkLibPaths = new List<string>();
ReferencePaths = new List<string>();
}
public string DetermineFinalOutputPath()
{
var outputDirectory = this.OutputDirectory;
var filename = Path.GetFileNameWithoutExtension(this.InputManagedAssemblyPath);
var outFile = Path.Combine(outputDirectory, filename + Constants.ExeSuffix);
return outFile;
}
}
public enum NativeIntermediateMode
{
cpp,
ryujit,
custom
}
public enum ArchitectureMode
{
x86,
x64
}
public enum OSMode
{
Linux,
Windows,
Mac
}
public enum BuildConfiguration
{
debug,
release
}
}
}

View file

@ -23,13 +23,13 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
public bool CompileToNative(NativeCompileSettings config)
{
int result = invoker.Invoke(config);
int result = invoker.Invoke();
if(result != 0)
{
return false;
}
result = intermediateCompiler.Invoke(config);
result = intermediateCompiler.Invoke();
if (result != 0)
{
return false;

View file

@ -96,7 +96,6 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
// Custom Extensibility Points to support CoreRT workflow TODO better descriptions
var ilcArgs = app.Option("--ilcargs <CODEGEN>", "Use to specify custom arguments for the IL Compiler.", CommandOptionType.SingleValue);
var ilcPathArg = app.Option("--ilcpath <ILC_PATH>", "Use to plug in a custom built ilc.exe", CommandOptionType.SingleValue);
var runtimeLibPathArg = app.Option("--runtimelib-path <LIB_PATH>", "Use to plug in custom runtime and bootstrapper libs.", CommandOptionType.SingleValue);
var linklibArg = app.Option("--linklib <LINKLIB>", "Use to link in additional static libs", CommandOptionType.MultipleValue);
// TEMPORARY Hack until CoreRT compatible Framework Libs are available
@ -121,7 +120,6 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
ReferencePaths = referencesArg.Values,
IlcArgs = ilcArgs.Value(),
IlcPath = ilcPathArg.Value(),
RuntimeLibPath = runtimeLibPathArg.Value(),
LinkLibPaths = linklibArg.Values,
AppDepSDKPath = appdepSDKPathArg.Value(),
LogPath = logpathArg.Value()
@ -129,8 +127,8 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
var config = ParseAndValidateArgs(cmdLineArgs);
Helpers.CleanOrCreateDirectory(config.OutputDirectory);
Helpers.CleanOrCreateDirectory(config.IntermediateDirectory);
DirectoryExtensions.CleanOrCreateDirectory(config.OutputDirectory);
DirectoryExtensions.CleanOrCreateDirectory(config.IntermediateDirectory);
var nativeCompiler = NativeCompiler.Create(config);
@ -158,13 +156,19 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
// Architecture
if(string.IsNullOrEmpty(args.Architecture))
{
config.Architecture = Helpers.GetCurrentArchitecture();
config.Architecture = RuntimeExtensions.GetCurrentArchitecture();
// CoreRT does not support x86 yet
if (config.Architecture != ArchitectureMode.x64)
{
throw new Exception("Native Compilation currently only supported for x64.");
}
}
else
{
try
{
config.Architecture = Helpers.ParseEnum<ArchitectureMode>(args.Architecture.ToLower());
config.Architecture = EnumExtensions.Parse<ArchitectureMode>(args.Architecture.ToLower());
}
catch (Exception e)
{
@ -181,7 +185,7 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
{
try
{
config.BuildType = Helpers.ParseEnum<BuildConfiguration>(args.BuildType.ToLower());
config.BuildType = EnumExtensions.Parse<BuildConfiguration>(args.BuildType.ToLower());
}
catch (Exception e)
{
@ -218,7 +222,7 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
{
try
{
config.NativeMode = Helpers.ParseEnum<NativeIntermediateMode>(args.NativeMode.ToLower());
config.NativeMode = EnumExtensions.Parse<NativeIntermediateMode>(args.NativeMode.ToLower());
}
catch (Exception e)
{
@ -262,21 +266,6 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
config.IlcPath = GetDefaultIlcPath();
}
// RuntimeLibPath
if (!string.IsNullOrEmpty(args.RuntimeLibPath))
{
if (!Directory.Exists(args.RuntimeLibPath))
{
throw new Exception("RuntimeLib Directory does not exist.");
}
config.RuntimeLibPath = args.RuntimeLibPath;
}
else
{
config.RuntimeLibPath = GetDefaultRuntimeLibPath();
}
// logpath
if (!string.IsNullOrEmpty(args.LogPath))
{
@ -302,7 +291,7 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
}
// OS
config.OS = Helpers.GetCurrentOS();
config.OS = RuntimeInformationExtensions.GetCurrentOS();
return config;
}
@ -344,10 +333,5 @@ namespace Microsoft.DotNet.Tools.Compiler.Native
{
return AppContext.BaseDirectory;
}
private static string GetDefaultRuntimeLibPath()
{
return AppContext.BaseDirectory;
}
}
}

View file

@ -24,7 +24,7 @@
"version": "1.0.0-*"
},
"Microsoft.DotNet.ILCompiler": "1.0.0-*",
"Microsoft.DotNet.ObjectWriter": "1.0.1-*",
"Microsoft.DotNet.ObjectWriter": "1.0.2-*",
"Microsoft.DotNet.RyuJit": "1.0.0-*"
},
"frameworks": {