From 908668fb5df727da45c1d49b7ac0692428b63add Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Wed, 11 Jan 2023 12:37:57 -0500 Subject: [PATCH 1/5] Bump to xamarin/xamarin-android-tools/main@76c076fc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: https://github.com/dotnet/maui/issues/11605 Context: 8bc7a3e84f95e70fe12790ac31ecd97957771cb2 Changes: http://github.com/xamarin/xamarin-android-tools/compare/9f02d77692bca8c6585941de03750d5eaaca5c5a...47f95ab99f6201d956eecfa1c7b2fd5fa7e43946 * xamarin/xamarin-android-tools@47f95ab: Fix CS0121 ambiguity errors. (xamarin/xamarin-android-tools#200) * xamarin/xamarin-android-tools@76c076f: Add support for Project Specific RegisterTaskObject. (xamarin/xamarin-android-tools#199) In dotnet/maui#11605, when `$(AndroidEnableMarshalMethods)`=True (8bc7a3e8), the build may fail if the `.sln` contains more than one Android "App" project. We tracked this down to "undesired sharing" between project builds; the `obj` provided to [`IBuildEngine4.RegisterTaskObject()`][0] can be visible across project builds. Consider [``][1]: var marshalMethodsState = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal (".:!MarshalMethods!:.", RegisteredTaskObjectLifetime.Build); Further consider a `.sln` with two "App" projects, as the "App" project build hits ``. The lifetime of `.Build` is *not* tied to the the `Build` target of a given `.csproj`; rather, it's for the *build process*. This can result in: 1. `dotnet build Project.sln` is run; `Project.sln` references `App1.csproj` and `App2.csproj`. 2. `App1.csproj` is built. 3. `App1.csproj` calls ``. 4. `App2.csproj` is later built as part in the process, and *also* calls ``. In particular note the key within ``: `".:!MarshalMethods!:."`. This value is unchanged, and means that that when `App2.csproj` is built, it will be using the same key as was used with `App1.csproj`, and thus could be inadvertently using data intended for `App1.csproj`! This could result build errors: …\Xamarin.Android.Common.targets(1717,3): error XAGPM7009: System.InvalidOperationException: Unable to translate assembly name 'Xamarin.AndroidX.Activity' to its index …\Xamarin.Android.Common.targets(1717,3): error XAGPM7009: at Xamarin.Android.Tasks.MarshalMethodsNativeAssemblyGenerator.WriteNativeMethods(LlvmIrGenerator generator, Dictionary`2 asmNameToIndex, LlvmIrVariableReference get_function_pointer_ref) …\Xamarin.Android.Common.targets(1717,3): error XAGPM7009: at Xamarin.Android.Tasks.MarshalMethodsNativeAssemblyGenerator.Write(LlvmIrGenerator generator) …\Xamarin.Android.Common.targets(1717,3): error XAGPM7009: at Xamarin.Android.Tasks.LLVMIR.LlvmIrComposer.Write(AndroidTargetArch arch, StreamWriter output, String fileName) …\Xamarin.Android.Common.targets(1717,3): error XAGPM7009: at Xamarin.Android.Tasks.GeneratePackageManagerJava.AddEnvironment() …\Xamarin.Android.Common.targets(1717,3): error XAGPM7009: at Xamarin.Android.Tasks.GeneratePackageManagerJava.RunTask() The sharing of `RegisterTaskObject()` data across project builds is rarely desirable. There are a few instances where it is safe to share the registered objects between projects, e.g. `java` version information (keyed on `java` path). However, most of the time it is specific to the project that is being built. Historically we have probably got away with this because "most" users only have one project. xamarin/xamarin-android-tools@76c076f updated the `MSBuildExtensions` extension methods so that [`IBuildEngine.ProjectFileOfTaskNode`][2] would be part of the `RegisterTaskObject()` key. Review use of the `.RegisterTaskObjectAssemblyLocal()`, `.GetRegisteredTaskObjectAssemblyLocal()`, and `.UnregisterTaskObjectAssemblyLocal()` extension methods to ensure that `IBuildEngine.ProjectFileOfTaskNode` is used or excluded, as appropriate. This should fix the XAGPM7009 build errors. TODO: add unit test to trigger this build scenario. [0]: https://learn.microsoft.com/en-us/dotnet/api/microsoft.build.framework.ibuildengine4.registertaskobject?view=msbuild-17-netcore [1]: https://github.com/xamarin/xamarin-android/blob/c92ae5eb9fdcb3a2fd7c20f5b42dddf8b3ea781a/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs#L407 [2]: https://learn.microsoft.com/en-us/dotnet/api/microsoft.build.framework.ibuildengine.projectfileoftasknode?view=msbuild-17-netcore --- external/xamarin-android-tools | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/xamarin-android-tools b/external/xamarin-android-tools index 9f02d77692b..47f95ab99f6 160000 --- a/external/xamarin-android-tools +++ b/external/xamarin-android-tools @@ -1 +1 @@ -Subproject commit 9f02d77692bca8c6585941de03750d5eaaca5c5a +Subproject commit 47f95ab99f6201d956eecfa1c7b2fd5fa7e43946 From ff399abfdc0984ba65378c978a81f4a0c6b48604 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Thu, 12 Jan 2023 08:25:26 -0500 Subject: [PATCH 2/5] Actually update GetRegisteredTaskObjectAssemblyLocal calls! --- .../Tasks/AndroidDotnetToolTask.cs | 8 ++++---- src/Xamarin.Android.Build.Tasks/Utilities/Aapt2Daemon.cs | 5 +++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/AndroidDotnetToolTask.cs b/src/Xamarin.Android.Build.Tasks/Tasks/AndroidDotnetToolTask.cs index efa01c48478..8f26c94f85c 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/AndroidDotnetToolTask.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/AndroidDotnetToolTask.cs @@ -105,7 +105,7 @@ string FindDotnet () string FindMono () { - string mono = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal (MonoKey, Lifetime); + string mono = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal (MonoKey, Lifetime, flags: RegisterTaskObjectKeyFlags.None); if (!string.IsNullOrEmpty (mono)) { Log.LogDebugMessage ($"Found cached mono via {nameof (BuildEngine4.RegisterTaskObject)}"); return mono; @@ -116,7 +116,7 @@ string FindMono () foreach (var path in env.Split (Path.PathSeparator)) { if (File.Exists (mono = Path.Combine (path, "mono"))) { Log.LogDebugMessage ("Found mono in $PATH"); - BuildEngine4.RegisterTaskObjectAssemblyLocal (MonoKey, mono, Lifetime); + BuildEngine4.RegisterTaskObjectAssemblyLocal (MonoKey, mono, Lifetime, flags: RegisterTaskObjectKeyFlags.None); return mono; } } @@ -125,13 +125,13 @@ string FindMono () foreach (var path in KnownMonoPaths) { if (File.Exists (mono = path)) { Log.LogDebugMessage ($"Found mono in {nameof (KnownMonoPaths)}"); - BuildEngine4.RegisterTaskObjectAssemblyLocal (MonoKey, mono, Lifetime); + BuildEngine4.RegisterTaskObjectAssemblyLocal (MonoKey, mono, Lifetime, flags: RegisterTaskObjectKeyFlags.None); return mono; } } // Last resort - BuildEngine4.RegisterTaskObjectAssemblyLocal (MonoKey, mono = "mono", Lifetime); + BuildEngine4.RegisterTaskObjectAssemblyLocal (MonoKey, mono = "mono", Lifetime, flags: RegisterTaskObjectKeyFlags.None); return mono; } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/Aapt2Daemon.cs b/src/Xamarin.Android.Build.Tasks/Utilities/Aapt2Daemon.cs index ec6aa25a035..c89b73bc907 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/Aapt2Daemon.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/Aapt2Daemon.cs @@ -22,11 +22,12 @@ internal class Aapt2Daemon : IDisposable public static Aapt2Daemon GetInstance (IBuildEngine4 engine, string aapt2, int numberOfInstances, int initalNumberOfDaemons, bool registerInDomain = false) { var area = registerInDomain ? RegisteredTaskObjectLifetime.AppDomain : RegisteredTaskObjectLifetime.Build; - var daemon = engine.GetRegisteredTaskObjectAssemblyLocal (RegisterTaskObjectKey, area); + var flags = registerInDomain ? RegisterTaskObjectKeyFlags.None : RegisterTaskObjectKeyFlags.IncludeProjectFile; + var daemon = engine.GetRegisteredTaskObjectAssemblyLocal (RegisterTaskObjectKey, area, flags: flags); if (daemon == null) { daemon = new Aapt2Daemon (aapt2, numberOfInstances, initalNumberOfDaemons); - engine.RegisterTaskObjectAssemblyLocal (RegisterTaskObjectKey, daemon, area, allowEarlyCollection: false); + engine.RegisterTaskObjectAssemblyLocal (RegisterTaskObjectKey, daemon, area, allowEarlyCollection: false, flags: flags); } return daemon; } From 809149e5313b70105cacc78091a828532eb5bfd3 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Thu, 12 Jan 2023 13:04:55 +0000 Subject: [PATCH 3/5] Add a Unit Test --- .../IncrementalBuildTest.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs index 24fe06606b7..1c16ce4feb2 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs @@ -247,6 +247,30 @@ public void AllProjectsHaveSameOutputDirectory() sb.Dispose (); } + [Test] + public void BuildSolutionWithMultipleProjectsInParallel () + { + var testPath = Path.Combine ("temp", "BuildSolutionWithMultipleProjects"); + var sb = new SolutionBuilder("BuildSolutionWithMultipleProjects.sln") { + SolutionPath = Path.Combine (Root, testPath), + MaxCpuCount = 4, + }; + for (int i=1; i <= 4; i++) { + var app1 = new XamarinAndroidApplicationProject () { + ProjectName = $"App{i}", + PackageName = $"com.companyname.App{i}", + AotAssemblies = true, + IsRelease = true, + }; + app1.SetProperty ("AndroidEnableMarshalMethods", "True"); + sb.Projects.Add (app1); + } + sb.BuildingInsideVisualStudio = false; + Assert.IsTrue (sb.Build (), "Build of solution should have succeeded"); + Assert.IsTrue (sb.ReBuild (), "ReBuild of solution should have succeeded"); + sb.Dispose (); + } + [Test] public void JavacTaskDoesNotRunOnSecondBuild () { From c9d9fca5090099c1f38027509e74282755c47a8d Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Thu, 12 Jan 2023 08:30:36 -0500 Subject: [PATCH 4/5] Aapt2 daemon can always be shared across projects. --- src/Xamarin.Android.Build.Tasks/Utilities/Aapt2Daemon.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/Aapt2Daemon.cs b/src/Xamarin.Android.Build.Tasks/Utilities/Aapt2Daemon.cs index c89b73bc907..db31d2b2252 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/Aapt2Daemon.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/Aapt2Daemon.cs @@ -22,12 +22,11 @@ internal class Aapt2Daemon : IDisposable public static Aapt2Daemon GetInstance (IBuildEngine4 engine, string aapt2, int numberOfInstances, int initalNumberOfDaemons, bool registerInDomain = false) { var area = registerInDomain ? RegisteredTaskObjectLifetime.AppDomain : RegisteredTaskObjectLifetime.Build; - var flags = registerInDomain ? RegisterTaskObjectKeyFlags.None : RegisterTaskObjectKeyFlags.IncludeProjectFile; - var daemon = engine.GetRegisteredTaskObjectAssemblyLocal (RegisterTaskObjectKey, area, flags: flags); + var daemon = engine.GetRegisteredTaskObjectAssemblyLocal (RegisterTaskObjectKey, area, flags: RegisterTaskObjectKeyFlags.None); if (daemon == null) { daemon = new Aapt2Daemon (aapt2, numberOfInstances, initalNumberOfDaemons); - engine.RegisterTaskObjectAssemblyLocal (RegisterTaskObjectKey, daemon, area, allowEarlyCollection: false, flags: flags); + engine.RegisterTaskObjectAssemblyLocal (RegisterTaskObjectKey, daemon, area, allowEarlyCollection: false, flags: RegisterTaskObjectKeyFlags.None); } return daemon; } From 39425eb3a4ba810d7bac55bd4b62e1433bccdd0d Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Thu, 12 Jan 2023 11:43:40 -0500 Subject: [PATCH 5/5] =?UTF-8?q?Fix=20the=20Aapt2DaemonInstances(=E2=80=A6)?= =?UTF-8?q?=20test.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should have got a Daemon Expected: not null But was: null at Xamarin.Android.Build.Tests.Aapt2Tests.Aapt2DaemonInstances(Int32 maxInstances, Int32 expectedMax, Int32 expectedInstances, Int32 numLayouts) in /Users/builder/azdo/_work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/Aapt2Tests.cs:line 109 at InvokeStub_Aapt2Tests.Aapt2DaemonInstances(Object, Ob The tests *also* need to use the correct & consistent arguments to `engine.GetRegisteredTaskObjectAssemblyLocal(…)`. --- .../Tests/Xamarin.Android.Build.Tests/Tasks/Aapt2Tests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/Aapt2Tests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/Aapt2Tests.cs index 8ee83fc9876..dd2386b5d0e 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/Aapt2Tests.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/Aapt2Tests.cs @@ -105,7 +105,7 @@ public void Aapt2DaemonInstances (int maxInstances, int expectedMax, int expecte DaemonKeepInDomain = false, }; Assert.True (task.Execute (), $"task should have succeeded. {string.Join (";", errors.Select (x => x.Message))}"); - var daemon = engine.GetRegisteredTaskObjectAssemblyLocal (Aapt2Daemon.RegisterTaskObjectKey, RegisteredTaskObjectLifetime.Build); + var daemon = engine.GetRegisteredTaskObjectAssemblyLocal (Aapt2Daemon.RegisterTaskObjectKey, RegisteredTaskObjectLifetime.Build, flags: RegisterTaskObjectKeyFlags.None); Assert.IsNotNull (daemon, "Should have got a Daemon"); Assert.AreEqual (expectedMax, daemon.MaxInstances, $"Expected {expectedMax} but was {daemon.MaxInstances}"); Assert.AreEqual (expectedInstances, daemon.CurrentInstances, $"Expected {expectedInstances} but was {daemon.CurrentInstances}");