forked from dotnet/android-tools
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add *Task.ProjectSpecificTaskObjectKey() for RegisterTaskObject() use (…
…dotnet#202) Context: dotnet/maui#11605 Context: dotnet/maui#11387 (comment) Context: http://github.com/xamarin/xamarin-android/commit/8bc7a3e84f95e70fe12790ac31ecd97957771cb2 In dotnet/maui#11605, when `$(AndroidEnableMarshalMethods)`=True (dotnet/android@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 [`<GeneratePackageManagerJava/>`][1]: var marshalMethodsState = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal<MarshalMethodsState> (".:!MarshalMethods!:.", RegisteredTaskObjectLifetime.Build); Further consider a `.sln` with two "App" projects, as the "App" project build hits `<GeneratePackageManagerJava/>`. 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 `<GeneratePackageManagerJava/>`. 4. `App2.csproj` is later built as part of the process, and *also* calls `<GeneratePackageManagerJava/>`. In particular note the key within `<GeneratePackageManagerJava/>`: `".:!MarshalMethods!:."`. This value is identical in all projects, 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 would 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` 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. Update `AndroidTask` and `AndroidToolTask` to capture the current directory in a `WorkingDirectory` property like [`AsyncTask`][2] does. Introduce new `ProjectSpecificTaskObjectKey()` instance methods into `AndroidTask`, `AndroidAsyncTask`, and `AndroidToolTask` which can be used to generate a key which includes `WorkingDirectory`. This allows: var marshalMethodsState = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal<MarshalMethodsState> ( ProjectSpecificTaskObjectKey (".:!MarshalMethods!:."), RegisteredTaskObjectLifetime.Build ); When `ProjectSpecificTaskObjectKey()` is used the `WorkingDirectory` is included in the key with `RegisterTaskObject()`. This helps ensure that builds in different `.csproj` files will result in different keys. [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://github.com/xamarin/Xamarin.Build.AsyncTask/blob/8b5fc6c4d13a3dfd1d17a2007e2143b6da3447d7/Xamarin.Build.AsyncTask/AsyncTask.cs#L59
- Loading branch information
1 parent
ac9ea09
commit 099fd95
Showing
6 changed files
with
245 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
100 changes: 100 additions & 0 deletions
100
tests/Microsoft.Android.Build.BaseTasks-Tests/AndroidToolTaskTests.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,100 @@ | ||
using System.IO; | ||
using System.Collections.Generic; | ||
using System.Threading.Tasks; | ||
using Microsoft.Android.Build.BaseTasks.Tests.Utilities; | ||
using Microsoft.Android.Build.Tasks; | ||
using NUnit.Framework; | ||
using Microsoft.Build.Framework; | ||
using Xamarin.Build; | ||
|
||
namespace Microsoft.Android.Build.BaseTasks.Tests | ||
{ | ||
[TestFixture] | ||
public class AndroidToolTaskTests | ||
{ | ||
public class MyAndroidTask : AndroidTask { | ||
public override string TaskPrefix {get;} = "MAT"; | ||
public string Key { get; set; } | ||
public string Value { get; set; } | ||
public bool ProjectSpecific { get; set; } = false; | ||
public override bool RunTask () | ||
{ | ||
var key = ProjectSpecific ? ProjectSpecificTaskObjectKey (Key) : (Key, (object)string.Empty); | ||
BuildEngine4.RegisterTaskObjectAssemblyLocal (key, Value, RegisteredTaskObjectLifetime.Build); | ||
return true; | ||
} | ||
} | ||
|
||
public class MyOtherAndroidTask : AndroidTask { | ||
public override string TaskPrefix {get;} = "MOAT"; | ||
public string Key { get; set; } | ||
public bool ProjectSpecific { get; set; } = false; | ||
|
||
[Output] | ||
public string Value { get; set; } | ||
public override bool RunTask () | ||
{ | ||
var key = ProjectSpecific ? ProjectSpecificTaskObjectKey (Key) : (Key, (object)string.Empty); | ||
Value = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal<string> (key, RegisteredTaskObjectLifetime.Build); | ||
return true; | ||
} | ||
} | ||
|
||
[Test] | ||
[TestCase (true, true, true)] | ||
[TestCase (false, false, true)] | ||
[TestCase (true, false, false)] | ||
[TestCase (false, true, false)] | ||
public void TestRegisterTaskObjectCanRetrieveCorrectItem (bool projectSpecificA, bool projectSpecificB, bool expectedResult) | ||
{ | ||
var engine = new MockBuildEngine (TestContext.Out) { | ||
}; | ||
var task = new MyAndroidTask () { | ||
BuildEngine = engine, | ||
Key = "Foo", | ||
Value = "Foo", | ||
ProjectSpecific = projectSpecificA, | ||
}; | ||
task.Execute (); | ||
var task2 = new MyOtherAndroidTask () { | ||
BuildEngine = engine, | ||
Key = "Foo", | ||
ProjectSpecific = projectSpecificB, | ||
}; | ||
task2.Execute (); | ||
Assert.AreEqual (expectedResult, string.Compare (task2.Value, task.Value, ignoreCase: true) == 0); | ||
} | ||
|
||
[Test] | ||
[TestCase (true, true, false)] | ||
[TestCase (false, false, true)] | ||
[TestCase (true, false, false)] | ||
[TestCase (false, true, false)] | ||
public void TestRegisterTaskObjectFailsWhenDirectoryChanges (bool projectSpecificA, bool projectSpecificB, bool expectedResult) | ||
{ | ||
var engine = new MockBuildEngine (TestContext.Out) { | ||
}; | ||
MyAndroidTask task; | ||
var currentDir = Directory.GetCurrentDirectory (); | ||
Directory.SetCurrentDirectory (Path.Combine (currentDir, "..")); | ||
try { | ||
task = new MyAndroidTask () { | ||
BuildEngine = engine, | ||
Key = "Foo", | ||
Value = "Foo", | ||
ProjectSpecific = projectSpecificA, | ||
}; | ||
} finally { | ||
Directory.SetCurrentDirectory (currentDir); | ||
} | ||
task.Execute (); | ||
var task2 = new MyOtherAndroidTask () { | ||
BuildEngine = engine, | ||
Key = "Foo", | ||
ProjectSpecific = projectSpecificB, | ||
}; | ||
task2.Execute (); | ||
Assert.AreEqual (expectedResult, string.Compare (task2.Value, task.Value, ignoreCase: true) == 0); | ||
} | ||
} | ||
} |
102 changes: 102 additions & 0 deletions
102
tests/Microsoft.Android.Build.BaseTasks-Tests/Utilites/MockBuildEngine.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
using System; | ||
using System.Collections; | ||
using System.Collections.Generic; | ||
using System.IO; | ||
using Microsoft.Build.Framework; | ||
|
||
namespace Microsoft.Android.Build.BaseTasks.Tests.Utilities { | ||
public class MockBuildEngine : IBuildEngine, IBuildEngine2, IBuildEngine3, IBuildEngine4 { | ||
public MockBuildEngine (TextWriter output, IList<BuildErrorEventArgs> errors = null, IList<BuildWarningEventArgs> warnings = null, IList<BuildMessageEventArgs> messages = null, IList<CustomBuildEventArgs> customEvents = null) | ||
{ | ||
this.Output = output; | ||
this.Errors = errors; | ||
this.Warnings = warnings; | ||
this.Messages = messages; | ||
this.CustomEvents = customEvents; | ||
} | ||
|
||
private TextWriter Output { get; } | ||
|
||
private IList<BuildErrorEventArgs> Errors { get; } | ||
|
||
private IList<BuildWarningEventArgs> Warnings { get; } | ||
|
||
private IList<BuildMessageEventArgs> Messages { get; } | ||
|
||
private IList<CustomBuildEventArgs> CustomEvents { get; } | ||
|
||
int IBuildEngine.ColumnNumberOfTaskNode => -1; | ||
|
||
bool IBuildEngine.ContinueOnError => false; | ||
|
||
int IBuildEngine.LineNumberOfTaskNode => -1; | ||
|
||
string IBuildEngine.ProjectFileOfTaskNode => "this.xml"; | ||
|
||
bool IBuildEngine2.IsRunningMultipleNodes => false; | ||
|
||
bool IBuildEngine.BuildProjectFile (string projectFileName, string [] targetNames, IDictionary globalProperties, IDictionary targetOutputs) => true; | ||
|
||
void IBuildEngine.LogCustomEvent (CustomBuildEventArgs e) | ||
{ | ||
this.Output.WriteLine ($"Custom: {e.Message}"); | ||
if (CustomEvents != null) | ||
CustomEvents.Add (e); | ||
} | ||
|
||
void IBuildEngine.LogErrorEvent (BuildErrorEventArgs e) | ||
{ | ||
this.Output.WriteLine ($"Error: {e.Message}"); | ||
if (Errors != null) | ||
Errors.Add (e); | ||
} | ||
|
||
void IBuildEngine.LogMessageEvent (BuildMessageEventArgs e) | ||
{ | ||
this.Output.WriteLine ($"Message: {e.Message}"); | ||
if (Messages != null) | ||
Messages.Add (e); | ||
} | ||
|
||
void IBuildEngine.LogWarningEvent (BuildWarningEventArgs e) | ||
{ | ||
this.Output.WriteLine ($"Warning: {e.Message}"); | ||
if (Warnings != null) | ||
Warnings.Add (e); | ||
} | ||
|
||
private Dictionary<object, object> Tasks = new Dictionary<object, object> (); | ||
|
||
void IBuildEngine4.RegisterTaskObject (object key, object obj, RegisteredTaskObjectLifetime lifetime, bool allowEarlyCollection) | ||
{ | ||
Tasks.Add (key, obj); | ||
} | ||
|
||
object IBuildEngine4.GetRegisteredTaskObject (object key, RegisteredTaskObjectLifetime lifetime) | ||
{ | ||
object obj = null; | ||
Tasks.TryGetValue (key, out obj); | ||
return obj; | ||
} | ||
|
||
object IBuildEngine4.UnregisterTaskObject (object key, RegisteredTaskObjectLifetime lifetime) | ||
{ | ||
var obj = Tasks [key]; | ||
Tasks.Remove (key); | ||
return obj; | ||
} | ||
|
||
BuildEngineResult IBuildEngine3.BuildProjectFilesInParallel (string [] projectFileNames, string [] targetNames, IDictionary [] globalProperties, IList<string> [] removeGlobalProperties, string [] toolsVersion, bool returnTargetOutputs) | ||
{ | ||
throw new NotImplementedException (); | ||
} | ||
|
||
void IBuildEngine3.Yield () { } | ||
|
||
void IBuildEngine3.Reacquire () { } | ||
|
||
bool IBuildEngine2.BuildProjectFile (string projectFileName, string [] targetNames, IDictionary globalProperties, IDictionary targetOutputs, string toolsVersion) => true; | ||
|
||
bool IBuildEngine2.BuildProjectFilesInParallel (string [] projectFileNames, string [] targetNames, IDictionary [] globalProperties, IDictionary [] targetOutputsPerProject, string [] toolsVersion, bool useResultsCache, bool unloadProjectsOnCompletion) => true; | ||
} | ||
} |