Skip to content

Commit

Permalink
[msbuild] Re-aot referencing assemblies. Fixes #17708. (#18509)
Browse files Browse the repository at this point in the history
If an assembly changes, then we must AOT compile that assembly again (which we already
did), in addition to any assembly that references the modified assembly (which we
didn't do).

So rework the AOTCompile target: remove the Inputs and Outputs (because the dependency
tracking is too complicated for MSBuild to resolve), and instead move the logic to
detect if an assembly must be AOT-compiled again into the AOTCompile task.

Note that this PR has a custom port to .NET 8: #18518.

Fixes #17708.

---------

Co-authored-by: Alex Soto <[email protected]>
  • Loading branch information
rolfbjarne and dalexsoto authored Aug 7, 2023
1 parent 6431e88 commit b17626f
Show file tree
Hide file tree
Showing 29 changed files with 332 additions and 19 deletions.
3 changes: 1 addition & 2 deletions dotnet/targets/Xamarin.Shared.Sdk.targets
Original file line number Diff line number Diff line change
Expand Up @@ -1053,8 +1053,7 @@
<Target Name="_AOTCompile"
Condition="'$(_RunAotCompiler)' == 'true'"
DependsOnTargets="$(_AOTCompileDependsOn)"
Inputs="@(_AssembliesToAOT)"
Outputs="@(_AssembliesToAOT -> '%(ObjectFile)');@(_AssembliesToAOT -> '%(LLVMFile)');">
>

<AOTCompile
SessionId="$(BuildSessionId)"
Expand Down
29 changes: 29 additions & 0 deletions msbuild/Xamarin.Localization.MSBuild/MSBStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -1504,4 +1504,33 @@
</comment>
</data>

<data name="E7116" xml:space="preserve">
<value>The assembly {0} does not provide an 'ObjectFile' metadata.</value>
<comment>
Don't translate: 'ObjectFile'.
</comment>
</data>

<data name="E7117" xml:space="preserve">
<value>The assembly {0} was passed multiple times as an input assembly (referenced from {1}).</value>
<comment>
{0}: the name of an assembly
{1}: the path to an assembly
</comment>
</data>

<data name="E7118" xml:space="preserve">
<value>Failed to AOT compile {0}, the AOT compiler exited with code {1}.</value>
<comment>
{0}: the path to an assembly
{1}: the exit code of a process
</comment>
</data>

<data name="E7119" xml:space="preserve">
<value>Encountered an assembly reference cycle related to the assembly {0}.</value>
<comment>
{0}: the path to an assembly
</comment>
</data>
</root>
135 changes: 122 additions & 13 deletions msbuild/Xamarin.MacDev.Tasks/Tasks/AOTCompileTaskBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
using Microsoft.Build.Framework;
using Microsoft.Build.Utilities;

using Mono.Cecil;

using Xamarin.Localization.MSBuild;
using Xamarin.Utils;

Expand Down Expand Up @@ -42,11 +44,111 @@ public abstract class AOTCompileTaskBase : XamarinTask {
public ITaskItem []? FileWrites { get; set; }
#endregion

class AssemblyInfo {
public ITaskItem TaskItem;
public bool? IsUpToDate;

public AssemblyInfo (ITaskItem item)
{
TaskItem = item;
}
}

Dictionary<string, AssemblyInfo> assemblyInfos = new Dictionary<string, AssemblyInfo> (StringComparer.OrdinalIgnoreCase);

string GetAssemblyName (ITaskItem item)
{
return Path.GetFileNameWithoutExtension (item.ItemSpec);
}

bool IsUpToDate (ITaskItem assembly)
{
var assemblyPath = assembly.ItemSpec;
var key = GetAssemblyName (assembly);
if (assemblyInfos.TryGetValue (key, out var info)) {
if (!info.IsUpToDate.HasValue) {
Log.LogError (MSBStrings.E7119 /* Encountered an assembly reference cycle related to the assembly {0}. */, assemblyPath);
info.IsUpToDate = false;
return false;
}
return info.IsUpToDate.Value;
}

info = new AssemblyInfo (assembly);
assemblyInfos [key] = info;

var finfo = new FileInfo (assemblyPath);
if (!finfo.Exists) {
Log.LogError (MSBStrings.E0158 /* The file {0} does not exist. */, assemblyPath);
info.IsUpToDate = false;
return false;
}

// ObjectFile is required
var objectFile = assembly.GetMetadata ("ObjectFile");
if (string.IsNullOrEmpty (objectFile)) {
Log.LogError (MSBStrings.E7116 /* The assembly {0} does not provide an 'ObjectFile' metadata. */, assembly.ItemSpec);
info.IsUpToDate = false;
return false;
}
var objectFileInfo = new FileInfo (objectFile);
if (!IsUpToDate (finfo, objectFileInfo)) {
Log.LogMessage (MessageImportance.Low, "The assembly {0} is not up-to-date with regards to the object file {1}", assemblyPath, objectFile);
info.IsUpToDate = false;
return false;
}

// LLVMFile is optional
var llvmFile = assembly.GetMetadata ("LLVMFile");
if (!string.IsNullOrEmpty (llvmFile)) {
var llvmFileInfo = new FileInfo (llvmFile);
if (!IsUpToDate (finfo, llvmFileInfo)) {
Log.LogMessage (MessageImportance.Low, "The assembly {0} is not up-to-date with regards to the llvm file {1}", assemblyPath, llvmFile);
info.IsUpToDate = false;
return false;
}
}

// We know now the assembly itself is up-to-date, but what about every referenced assembly?
// This assembly must be AOT-compiled again if any referenced assembly has changed as well.
using var ad = AssemblyDefinition.ReadAssembly (assembly.ItemSpec, new ReaderParameters { ReadingMode = ReadingMode.Deferred });
foreach (var ar in ad.MainModule.AssemblyReferences) {
var referencedItems = Assemblies.Where (v => string.Equals (GetAssemblyName (v), ar.Name, StringComparison.OrdinalIgnoreCase)).ToArray ();
if (referencedItems.Length == 0) {
Log.LogMessage (MessageImportance.Low, $"Ignoring unresolved assembly {ar.Name} (referenced from {assemblyPath}).");
continue;
} else if (referencedItems.Length > 1) {
Log.LogError (MSBStrings.E7117 /* The assembly {0} was passed multiple times as an input assembly (referenced from {1}). */, ar.Name, assemblyPath);
info.IsUpToDate = false;
return false;
}
var referencedItem = referencedItems [0];
if (!IsUpToDate (referencedItem)) {
info.IsUpToDate = false;
Log.LogMessage (MessageImportance.Low, "The assembly {0} is not up-to-date with regards to the reference {1}", assemblyPath, ar.Name);
return false;
}
}

Log.LogMessage (MessageImportance.Low, $"The AOT-compiled code for {assemblyPath} is up-to-date.");
info.IsUpToDate = true;
return true;
}

bool IsUpToDate (FileInfo input, FileInfo output)
{
if (!output.Exists)
return false;
return input.LastWriteTimeUtc <= output.LastWriteTimeUtc;
}

public override bool Execute ()
{
var inputs = new List<string> (Assemblies.Length);
for (var i = 0; i < Assemblies.Length; i++) {
inputs.Add (Path.GetFullPath (Assemblies [i].ItemSpec));
var input = Path.GetFullPath (Assemblies [i].ItemSpec);
inputs.Add (input);
Assemblies [i].SetMetadata ("Input", input);
}

// All the assemblies to AOT must be in the same directory
Expand All @@ -64,24 +166,31 @@ public override bool Execute ()
InputDirectory = assemblyDirectories [0];
}

// Figure out which assemblies need to be aot'ed, and which are up-to-date.
var assembliesToAOT = Assemblies.Where (asm => !IsUpToDate (asm)).ToArray ();
if (assembliesToAOT.Length == 0) {
Log.LogMessage (MessageImportance.Low, $"All the AOT-compiled code is up-to-date.");
return !Log.HasLoggedErrors;
}

Directory.CreateDirectory (OutputDirectory);

var aotAssemblyFiles = new List<ITaskItem> ();
var processes = new Task<Execution> [Assemblies.Length];
var processes = new Task<Execution> [assembliesToAOT.Length];

var environment = new Dictionary<string, string?> {
{ "MONO_PATH", Path.GetFullPath (InputDirectory) },
};

var globalAotArguments = AotArguments?.Select (v => v.ItemSpec).ToList ();
for (var i = 0; i < Assemblies.Length; i++) {
var asm = Assemblies [i];
var input = inputs [i];
var arch = Assemblies [i].GetMetadata ("Arch");
var aotArguments = Assemblies [i].GetMetadata ("Arguments");
var processArguments = Assemblies [i].GetMetadata ("ProcessArguments");
var aotData = Assemblies [i].GetMetadata ("AOTData");
var aotAssembly = Assemblies [i].GetMetadata ("AOTAssembly");
for (var i = 0; i < assembliesToAOT.Length; i++) {
var asm = assembliesToAOT [i];
var input = asm.GetMetadata ("Input");
var arch = asm.GetMetadata ("Arch");
var aotArguments = asm.GetMetadata ("Arguments");
var processArguments = asm.GetMetadata ("ProcessArguments");
var aotData = asm.GetMetadata ("AOTData");
var aotAssembly = asm.GetMetadata ("AOTAssembly");

var aotAssemblyItem = new TaskItem (aotAssembly);
aotAssemblyItem.SetMetadata ("Arguments", "-Xlinker -rpath -Xlinker @executable_path/ -Qunused-arguments -x assembler -D DEBUG");
Expand All @@ -106,7 +215,7 @@ public override bool Execute ()
processes [i] = ExecuteAsync (AOTCompilerPath, arguments, environment: environment, sdkDevPath: SdkDevPath, showErrorIfFailure: false /* we show our own error below */)
.ContinueWith ((v) => {
if (v.Result.ExitCode != 0)
Log.LogError ("Failed to AOT compile {0}, the AOT compiler exited with code {1}", Path.GetFileName (input), v.Result.ExitCode);
Log.LogError (MSBStrings.E7118 /* Failed to AOT compile {0}, the AOT compiler exited with code {1} */, Path.GetFileName (input), v.Result.ExitCode);

return System.Threading.Tasks.Task.FromResult<Execution> (v.Result);
}).Unwrap ();
Expand All @@ -118,8 +227,8 @@ public override bool Execute ()

// For Windows support it's necessary to have the files we're going to create as an Output parameter, so that the files are
// created on the windows side too, which makes the Inputs/Outputs logic work properly when working from Windows.
var objectFiles = Assemblies.Select (v => v.GetMetadata ("ObjectFile")).Where (v => !string.IsNullOrEmpty (v));
var llvmFiles = Assemblies.Select (v => v.GetMetadata ("LLVMFile")).Where (v => !string.IsNullOrEmpty (v));
var objectFiles = assembliesToAOT.Select (v => v.GetMetadata ("ObjectFile")).Where (v => !string.IsNullOrEmpty (v));
var llvmFiles = assembliesToAOT.Select (v => v.GetMetadata ("LLVMFile")).Where (v => !string.IsNullOrEmpty (v));
FileWrites = objectFiles.Union (llvmFiles).Select (v => new TaskItem (v)).ToArray ();

return !Log.HasLoggedErrors;
Expand Down
18 changes: 18 additions & 0 deletions tests/dotnet/RebuildTestAppWithLibraryReference/AppDelegate.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
using System;
using System.Runtime.InteropServices;

using Foundation;

namespace MySimpleApp {
public class Program {
static int Main (string [] args)
{
GC.KeepAlive (typeof (NSObject)); // prevent linking away the platform assembly

Console.WriteLine (Environment.GetEnvironmentVariable ("MAGIC_WORD"));
Console.WriteLine (Library.Class.SomeFunction ());

return args.Length;
}
}
}
15 changes: 15 additions & 0 deletions tests/dotnet/RebuildTestAppWithLibraryReference/Library/Library.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
using System;
using System.Runtime.InteropServices;

namespace Library {
public class Class {
public static string SomeFunction ()
{
#if FIRSTBUILD
return "FIRSTBUILD";
#elif SECONDBUILD
return "SECONDBUILD";
#endif
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>net$(BundledNETCoreAppTargetFrameworkVersion)</TargetFrameworks>
</PropertyGroup>
<Import Project="..\shared.csproj" />
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include ../shared.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
TOP=../../../..
include $(TOP)/tests/common/shared-dotnet-test.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>net$(BundledNETCoreAppTargetFrameworkVersion)</TargetFrameworks>
</PropertyGroup>
<Import Project="..\shared.csproj" />
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include ../shared.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>net$(BundledNETCoreAppTargetFrameworkVersion)</TargetFrameworks>
</PropertyGroup>
<Import Project="..\shared.csproj" />
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include ../shared.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0" encoding="utf-8"?>
<Project>
<PropertyGroup>
<OutputType>Library</OutputType>
<DefineConstants Condition="'$(BuildCounter)' == 'First'">$(DefineConstants);FIRSTBUILD</DefineConstants>
<DefineConstants Condition="'$(BuildCounter)' == 'Second'">$(DefineConstants);SECONDBUILD</DefineConstants>
</PropertyGroup>

<ItemGroup>
<Compile Include="../*.cs" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
TOP=../../../../..
include $(TOP)/tests/common/shared-dotnet.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>net$(BundledNETCoreAppTargetFrameworkVersion)</TargetFrameworks>
</PropertyGroup>
<Import Project="..\shared.csproj" />
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include ../shared.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include ../shared.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net$(BundledNETCoreAppTargetFrameworkVersion)-maccatalyst</TargetFramework>
</PropertyGroup>
<Import Project="..\shared.csproj" />
</Project>
2 changes: 2 additions & 0 deletions tests/dotnet/RebuildTestAppWithLibraryReference/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
TOP=../../..
include $(TOP)/tests/common/shared-dotnet-test.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include ../shared.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net$(BundledNETCoreAppTargetFrameworkVersion)-ios</TargetFramework>
</PropertyGroup>
<Import Project="..\shared.csproj" />
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include ../shared.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net$(BundledNETCoreAppTargetFrameworkVersion)-macos</TargetFramework>
</PropertyGroup>
<Import Project="..\shared.csproj" />
</Project>
19 changes: 19 additions & 0 deletions tests/dotnet/RebuildTestAppWithLibraryReference/shared.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?xml version="1.0" encoding="utf-8"?>
<Project>
<PropertyGroup>
<OutputType>Exe</OutputType>

<ApplicationTitle>RebuildTestAppWithLibraryReference</ApplicationTitle>
<ApplicationId>com.xamarin.rebuildtestappwithlibraryreference</ApplicationId>

<ExcludeTouchUnitReference>true</ExcludeTouchUnitReference>
<ExcludeNUnitLiteReference>true</ExcludeNUnitLiteReference>
</PropertyGroup>

<Import Project="../../common/shared-dotnet.csproj" />

<ItemGroup>
<Compile Include="../*.cs" />
<ProjectReference Include="../Library/$(_PlatformName)/Library.csproj" />
</ItemGroup>
</Project>
2 changes: 2 additions & 0 deletions tests/dotnet/RebuildTestAppWithLibraryReference/shared.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
TOP=../../../..
include $(TOP)/tests/common/shared-dotnet.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include ../shared.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net$(BundledNETCoreAppTargetFrameworkVersion)-tvos</TargetFramework>
</PropertyGroup>
<Import Project="..\shared.csproj" />
</Project>
Loading

7 comments on commit b17626f

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

Please sign in to comment.