Skip to content

Commit

Permalink
[MonoAOTCompiler] more properties & custom WorkingDirectory
Browse files Browse the repository at this point in the history
Fixes: #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 `<MonoAOTCompiler/>` 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`:

dotnet/android#6562

I'm able to enable several AOT tests related to #56163.
  • Loading branch information
jonathanpeppers authored and github-actions committed Dec 15, 2021
1 parent 3992fc3 commit 9f93966
Showing 1 changed file with 63 additions and 5 deletions.
68 changes: 63 additions & 5 deletions src/tasks/AotCompilerTask/MonoAOTCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -191,12 +191,39 @@ public class MonoAOTCompiler : Microsoft.Build.Utilities.Task
/// </summary>
public string? CacheFilePath { get; set; }

/// <summary>
/// Passes additional, custom arguments to --aot
/// </summary>
public string? AotArguments { get; set; }

/// <summary>
/// Passes temp-path to the AOT compiler
/// </summary>
public string? TempPath { get; set; }

/// <summary>
/// Passes ld-name to the AOT compiler, for use with UseLLVM=true
/// </summary>
public string? LdName { get; set; }

/// <summary>
/// Passes ld-flags to the AOT compiler, for use with UseLLVM=true
/// </summary>
public string? LdFlags { get; set; }

/// <summary>
/// Specify WorkingDirectory for the AOT compiler
/// </summary>
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<string> _fileWrites = new();

private IList<ITaskItem>? _assembliesToCompile;
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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.
Expand All @@ -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}";
Expand All @@ -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);
}
Expand Down Expand Up @@ -741,7 +799,7 @@ private bool PrecompileLibrary(PrecompileArguments args)
StringBuilder envStr = new StringBuilder(string.Empty);
foreach (KeyValuePair<string, string> 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);
Expand Down

0 comments on commit 9f93966

Please sign in to comment.