Skip to content
Merged
Changes from all 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
[MonoAOTCompiler] more properties & custom WorkingDirectory
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
commit 9f93966d77831620d01f6a422dbb9542ec760772
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