Skip to content

[MonoAOTCompiler] more properties & custom WorkingDirectory #62725

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.

Sign up for GitHub

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

Merged
jonathanpeppers merged 1 commit into from
Dec 15, 2021
Merged

[MonoAOTCompiler] more properties & custom WorkingDirectory #62725

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
68 changes: 63 additions & 5 deletions src/tasks/AotCompilerTask/MonoAOTCompiler.cs
View file
Open in desktop
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; }
Comment on lines +194 to +212
Copy link
Member

@radical radical Dec 14, 2021

Choose a reason for hiding this comment

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

I wonder if it might be better to have a ITaskItem? CommonAotArguments with metadata which act as the key-value pairs to pass to --aot.

Copy link
Member Author

@jonathanpeppers jonathanpeppers Dec 14, 2021

Choose a reason for hiding this comment

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

LdFlags will contain a ; on Android. So it seems like using item metadata would always have an issue?

Our usage is now:

<MonoAOTCompiler
    Triple="$(_Triple)"
    ToolPrefix="$(_ToolPrefix)"
    MsymPath="$(_MsymPath)"
    Assemblies="@(_MonoAOTAssemblies)"
    CompilerBinaryPath="$(_MonoAOTCompilerPath)"
    DisableParallelAot="$(_DisableParallelAot)"
    IntermediateOutputPath="$(IntermediateOutputPath)"
    LibraryFormat="So"
    Mode="$(AndroidAotMode)"
    OutputDir="$(IntermediateOutputPath)aot\"
    OutputType="Library"
    UseAotDataFile="false"
    UseLLVM="$(EnableLLVM)"
    LLVMPath="$(_LLVMPath)"
    LdName="$(_LdName)"
    LdFlags="$(_LdFlags)"
    WorkingDirectory="$(MSBuildProjectDirectory)"
    AotArguments="$(AndroidAotAdditionalArguments)">
  <Output TaskParameter="CompiledAssemblies" ItemName="_AotCompiledAssemblies" />
  <Output TaskParameter="FileWrites"         ItemName="FileWrites" />
</MonoAOTCompiler>

But maybe $(AndroidAotAdditionalArguments) could be passed in a different way? What would the XML look like for ITaskItem? CommonAotArguments?

Copy link
Member

@radical radical Dec 14, 2021

Choose a reason for hiding this comment

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

LdFlags will contain a ; on Android. So it seems like using item metadata would always have an issue?

We won't split the metadata on CommonAotArguments.
As for the metadata names, we could either use the real argument name, like <ld-flags>foo; bar</ld-flags>, or we could have <LdFlags>foo; bar</LdFlags> and convert to the real name in the task.

  <CommonAotArguments Include="common">
    <ld-flags>foo;bar</ld-flags>
    <ld-name>xyz</ld-name>
  </CommonAotArguments>

or

  <CommonAotArguments Include="common">
    <LdFlags>foo;bar</LdFlags>
    <LdName>xyz</LdName>
  </CommonAotArguments>

And you could have a <PassVerbatim>..</PassVerbatim> metadata (or better name) for just any additional string to be passed to --aot.

Feel free to push back, I'm just thinking aloud.

Copy link
Member Author

@jonathanpeppers jonathanpeppers Dec 15, 2021

Choose a reason for hiding this comment

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

The weird part about using @(CommonAotArguments), is that you put Include="" with any random word inside. You can't leave out Include, or that would be an empty item group. I also like the idea of having C# properties, as it checks for typos -- you'll get an MSBuild error if you put LdlFlags instead of LdFlags.

Adding something like @(CommonAotArguments) might be ok if there is some unknown set. At least for Android, these changes now include every parameter we use. Are there many more not represented here?


/// <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)}");
}

if (exitCode != 0)
Expand Down