From 9f93966d77831620d01f6a422dbb9542ec760772 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Fri, 10 Dec 2021 11:16:11 -0600 Subject: [PATCH] [MonoAOTCompiler] more properties & custom WorkingDirectory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: https://github.com/dotnet/runtime/issues/56163 PR #58523 fixed something on Windows, but it didn't actually address our issues on Android. A directory name like `foo Ümläüts` fails: * `mkdir 'foo Ümläüts' ; cd 'foo Ümläüts'` * `dotnet new android` * `dotnet build -c Release -p:RunAOTCompilation=true` (adding `-p:EnableLLVM=true` complicates further) The error: Precompiling failed for C:\src\foo Ümläüts\obj\Release\android-arm64\linked\System.Private.CoreLib.dll: Error: Loaded assembly 'C:\src\foo ├£ml├ñ├╝ts\obj\Release\android-arm64\linked\System.Private.CoreLib.dll' doesn't match original file name 'C:\foo ▄mlΣⁿts\obj\Release\android-arm64\linked\System.Private.CoreLib.dll'. Set MONO_PATH to the assembly's location. Reviewing the existing AOT implementation in Xamarin.Android, I found out *why* Xamarin.Android works: [AOT] response file obj\Release\120\aot\arm64-v8a\App36.dll\response.txt: --llvm "--aot=temp-path=obj\Release\120\aot\arm64-v8a\App36.dll,llvm-path=C:\Program Files\Microsoft Visual Studio\2022\Preview\MSBuild\Xamarin\Android,outfile=obj\Release\120\aot\arm64-v8a\libaot-App36.dll.so,msym-dir=obj\Release\120\aot\arm64-v8a,asmwriter,mtriple=aarch64-linux-android,tool-prefix=C:\Program Files (x86)\Android\android-sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\bin\aarch64-linux-android-,ld-name=ld.EXE,ld-flags=\"-LC:\Program Files (x86)\Android\android-sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\lib\gcc\aarch64-linux-android\4.9.x\";\"-LC:\Program Files (x86)\Android\android-sdk\ndk-bundle\platforms\android-21\arch-arm64\usr\lib\";\"C:\Program Files (x86)\Android\android-sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\lib\gcc\aarch64-linux-android\4.9.x\libgcc.a\";\"C:\Program Files (x86)\Android\android-sdk\ndk-bundle\platforms\android-21\arch-arm64\usr\lib\libc.so\";\"C:\Program Files (x86)\Android\android-sdk\ndk-bundle\platforms\android-21\arch-arm64\usr\lib\libm.so\"" C:\Users\jopepper\source\repos\App36\App36\obj\Release\120\android\assets\shrunk\App36.dll 1. Xamarin.Android passes *relative* paths. The `foo Ümläüts` directory name doesn't even come into play for some arguments. 2. With LLVM, `ld-flags` contains a `;`. The existing code splits on `;` and joins on `,`: https://github.com/dotnet/runtime/blob/25c207351c4f57cf2daa98caaf327a8b8d83edb8/src/tasks/AotCompilerTask/MonoAOTCompiler.cs#L505-L509 So we lose any `;` delimiters for the `ld-flags` value, they get replaced by `,`. I think the solution here is: 1. Add several missing properties to `` so we don't have to rely on the `%(AotArguments)` item metadata. No splitting on `;` would be required, `ld-flags` can be passed in and used as-is. 2. Add a new `WorkingDirectory` property. When this is set, assume paths passed in might be relative -- and don't transform paths by calling `Path.GetFullPath()`. Lastly, I fixed a place where the UTF8 encoding wasn't passed when MSBuild logging the response file. These changes I tried to make in a way where this shouldn't break other .NET workloads like wasm. If existing MSBuild targets call this task (not using the new properties), the behavior should remain unchanged. I tested these changes by commiting a modified `MonoAOTCompiler.dll`: https://github.com/xamarin/xamarin-android/pull/6562 I'm able to enable several AOT tests related to dotnet/runtime#56163. --- src/tasks/AotCompilerTask/MonoAOTCompiler.cs | 68 ++++++++++++++++++-- 1 file changed, 63 insertions(+), 5 deletions(-) diff --git a/src/tasks/AotCompilerTask/MonoAOTCompiler.cs b/src/tasks/AotCompilerTask/MonoAOTCompiler.cs index 87642d8d2fb32b..319ba6280a0dcc 100644 --- a/src/tasks/AotCompilerTask/MonoAOTCompiler.cs +++ b/src/tasks/AotCompilerTask/MonoAOTCompiler.cs @@ -191,12 +191,39 @@ public class MonoAOTCompiler : Microsoft.Build.Utilities.Task /// public string? CacheFilePath { get; set; } + /// + /// Passes additional, custom arguments to --aot + /// + public string? AotArguments { get; set; } + + /// + /// Passes temp-path to the AOT compiler + /// + public string? TempPath { get; set; } + + /// + /// Passes ld-name to the AOT compiler, for use with UseLLVM=true + /// + public string? LdName { get; set; } + + /// + /// Passes ld-flags to the AOT compiler, for use with UseLLVM=true + /// + public string? LdFlags { get; set; } + + /// + /// Specify WorkingDirectory for the AOT compiler + /// + public string? WorkingDirectory { get; set; } + [Required] public string IntermediateOutputPath { get; set; } = string.Empty; [Output] public string[]? FileWrites { get; private set; } + private static readonly Encoding s_utf8Encoding = new UTF8Encoding(false); + private List _fileWrites = new(); private IList? _assembliesToCompile; @@ -225,7 +252,9 @@ private bool ProcessAndValidateArguments() return false; } - if (!Path.IsPathRooted(OutputDir)) + // A relative path might be used along with WorkingDirectory, + // only call Path.GetFullPath() if WorkingDirectory is blank. + if (string.IsNullOrEmpty(WorkingDirectory) && !Path.IsPathRooted(OutputDir)) OutputDir = Path.GetFullPath(OutputDir); if (!Directory.Exists(OutputDir)) @@ -682,6 +711,26 @@ private PrecompileArguments GetPrecompileArgumentsFor(ITaskItem assemblyItem, st } } + if (!string.IsNullOrEmpty(AotArguments)) + { + aotArgs.Add(AotArguments); + } + + if (!string.IsNullOrEmpty(TempPath)) + { + aotArgs.Add($"temp-path={TempPath}"); + } + + if (!string.IsNullOrEmpty(LdName)) + { + aotArgs.Add($"ld-name={LdName}"); + } + + if (!string.IsNullOrEmpty(LdFlags)) + { + aotArgs.Add($"ld-flags={LdFlags}"); + } + // we need to quote the entire --aot arguments here to make sure it is parsed // on Windows as one argument. Otherwise it will be split up into multiple // values, which wont work. @@ -694,7 +743,16 @@ private PrecompileArguments GetPrecompileArgumentsFor(ITaskItem assemblyItem, st } else { - processArgs.Add('"' + assemblyFilename + '"'); + if (string.IsNullOrEmpty(WorkingDirectory)) + { + processArgs.Add('"' + assemblyFilename + '"'); + } + else + { + // If WorkingDirectory is supplied, the caller could be passing in a relative path + // Use the original ItemSpec that was passed in. + processArgs.Add('"' + assemblyItem.ItemSpec + '"'); + } } monoPaths = $"{assemblyDir}{Path.PathSeparator}{monoPaths}"; @@ -706,14 +764,14 @@ private PrecompileArguments GetPrecompileArgumentsFor(ITaskItem assemblyItem, st var responseFileContent = string.Join(" ", processArgs); var responseFilePath = Path.GetTempFileName(); - using (var sw = new StreamWriter(responseFilePath, append: false, encoding: new UTF8Encoding(false))) + using (var sw = new StreamWriter(responseFilePath, append: false, encoding: s_utf8Encoding)) { sw.WriteLine(responseFileContent); } return new PrecompileArguments(ResponseFilePath: responseFilePath, EnvironmentVariables: envVariables, - WorkingDir: assemblyDir, + WorkingDir: string.IsNullOrEmpty(WorkingDirectory) ? assemblyDir : WorkingDirectory, AOTAssembly: aotAssembly, ProxyFiles: proxyFiles); } @@ -741,7 +799,7 @@ private bool PrecompileLibrary(PrecompileArguments args) StringBuilder envStr = new StringBuilder(string.Empty); foreach (KeyValuePair kvp in args.EnvironmentVariables) envStr.Append($"{kvp.Key}={kvp.Value} "); - Log.LogMessage(importance, $"{msgPrefix}Exec (with response file contents expanded) in {args.WorkingDir}: {envStr}{CompilerBinaryPath} {File.ReadAllText(args.ResponseFilePath)}"); + Log.LogMessage(importance, $"{msgPrefix}Exec (with response file contents expanded) in {args.WorkingDir}: {envStr}{CompilerBinaryPath} {File.ReadAllText(args.ResponseFilePath, s_utf8Encoding)}"); } Log.LogMessage(importance, output);