Skip to content

Commit 099fd95

Browse files
authored
Add *Task.ProjectSpecificTaskObjectKey() for RegisterTaskObject() use (#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
1 parent ac9ea09 commit 099fd95

File tree

6 files changed

+245
-1
lines changed

6 files changed

+245
-1
lines changed

src/Microsoft.Android.Build.BaseTasks/AndroidAsyncTask.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,5 +46,7 @@ public virtual bool RunTask ()
4646
/// * RunTaskAsync is already on a background thread
4747
/// </summary>
4848
public virtual System.Threading.Tasks.Task RunTaskAsync () => System.Threading.Tasks.Task.CompletedTask;
49+
50+
protected object ProjectSpecificTaskObjectKey (object key) => (key, WorkingDirectory);
4951
}
5052
}

src/Microsoft.Android.Build.BaseTasks/AndroidTask.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// https://github.com/xamarin/xamarin-android/blob/9fca138604c53989e1cff7fc0c2e939583b4da28/src/Xamarin.Android.Build.Tasks/Tasks/AndroidTask.cs#L10
22

33
using System;
4+
using System.IO;
45
using Microsoft.Build.Utilities;
56

67
namespace Microsoft.Android.Build.Tasks
@@ -11,6 +12,13 @@ public abstract class AndroidTask : Task
1112
{
1213
public abstract string TaskPrefix { get; }
1314

15+
protected string WorkingDirectory { get; private set; }
16+
17+
public AndroidTask ()
18+
{
19+
WorkingDirectory = Directory.GetCurrentDirectory();
20+
}
21+
1422
public override bool Execute ()
1523
{
1624
try {
@@ -22,5 +30,7 @@ public override bool Execute ()
2230
}
2331

2432
public abstract bool RunTask ();
33+
34+
protected object ProjectSpecificTaskObjectKey (object key) => (key, WorkingDirectory);
2535
}
2636
}

src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// https://github.com/xamarin/xamarin-android/blob/9fca138604c53989e1cff7fc0c2e939583b4da28/src/Xamarin.Android.Build.Tasks/Tasks/AndroidTask.cs#L75
22

33
using System;
4+
using System.IO;
45
using Microsoft.Build.Utilities;
56

67
namespace Microsoft.Android.Build.Tasks
@@ -9,6 +10,13 @@ public abstract class AndroidToolTask : ToolTask
910
{
1011
public abstract string TaskPrefix { get; }
1112

13+
protected string WorkingDirectory { get; private set; }
14+
15+
public AndroidToolTask ()
16+
{
17+
WorkingDirectory = Directory.GetCurrentDirectory();
18+
}
19+
1220
public override bool Execute ()
1321
{
1422
try {
@@ -22,5 +30,7 @@ public override bool Execute ()
2230
// Most ToolTask's do not override Execute and
2331
// just expect the base to be called
2432
public virtual bool RunTask () => base.Execute ();
33+
34+
protected object ProjectSpecificTaskObjectKey (object key) => (key, WorkingDirectory);
2535
}
2636
}

src/Microsoft.Android.Build.BaseTasks/MSBuildExtensions.cs

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,18 +251,30 @@ public static void SetDestinationSubPath (this ITaskItem assembly)
251251

252252
/// <summary>
253253
/// IBuildEngine4.RegisterTaskObject, but adds the current assembly path into the key
254+
/// The `key` should be unique to a project unless it is a global item.
255+
/// Ideally the key should be the full path of a file in the project directory structure.
256+
/// Or you can use the `ProjectSpecificTaskObjectKey` method of the `AndroidTask` to generate
257+
/// a project specific key if needed.
254258
/// </summary>
255259
public static void RegisterTaskObjectAssemblyLocal (this IBuildEngine4 engine, object key, object value, RegisteredTaskObjectLifetime lifetime, bool allowEarlyCollection = false) =>
256260
engine.RegisterTaskObject ((AssemblyLocation, key), value, lifetime, allowEarlyCollection);
257261

258262
/// <summary>
259263
/// IBuildEngine4.GetRegisteredTaskObject, but adds the current assembly path into the key
264+
/// The `key` should be unique to a project unless it is a global item.
265+
/// Ideally the key should be the full path of a file in the project directory structure.
266+
/// Or you can use the `ProjectSpecificTaskObjectKey` method of the `AndroidTask` to generate
267+
/// a project specific key if needed.
260268
/// </summary>
261269
public static object GetRegisteredTaskObjectAssemblyLocal (this IBuildEngine4 engine, object key, RegisteredTaskObjectLifetime lifetime) =>
262270
engine.GetRegisteredTaskObject ((AssemblyLocation, key), lifetime);
263271

264272
/// <summary>
265273
/// Generic version of IBuildEngine4.GetRegisteredTaskObject, but adds the current assembly path into the key
274+
/// The `key` should be unique to a project unless it is a global item.
275+
/// Ideally the key should be the full path of a file in the project directory structure.
276+
/// Or you can use the `ProjectSpecificTaskObjectKey` method of the `AndroidTask` to generate
277+
/// a project specific key if needed.
266278
/// </summary>
267279
public static T GetRegisteredTaskObjectAssemblyLocal<T> (this IBuildEngine4 engine, object key, RegisteredTaskObjectLifetime lifetime)
268280
where T : class =>
@@ -271,12 +283,20 @@ public static T GetRegisteredTaskObjectAssemblyLocal<T> (this IBuildEngine4 engi
271283

272284
/// <summary>
273285
/// IBuildEngine4.UnregisterTaskObject, but adds the current assembly path into the key
286+
/// The `key` should be unique to a project unless it is a global item.
287+
/// Ideally the key should be the full path of a file in the project directory structure.
288+
/// Or you can use the `ProjectSpecificTaskObjectKey` method of the `AndroidTask` to generate
289+
/// a project specific key if needed.
274290
/// </summary>
275291
public static object UnregisterTaskObjectAssemblyLocal (this IBuildEngine4 engine, object key, RegisteredTaskObjectLifetime lifetime) =>
276292
engine.UnregisterTaskObject ((AssemblyLocation, key), lifetime);
277293

278294
/// <summary>
279-
/// Generic version of IBuildEngine4.UnregisterTaskObject, but adds the current assembly path into the key
295+
/// Generic version of IBuildEngine4.UnregisterTaskObject, but adds the current assembly path into the key.
296+
/// The `key` should be unique to a project unless it is a global item.
297+
/// Ideally the key should be the full path of a file in the project directory structure.
298+
/// Or you can use the `ProjectSpecificTaskObjectKey` method of the `AndroidTask` to generate
299+
/// a project specific key if needed.
280300
/// </summary>
281301
public static T UnregisterTaskObjectAssemblyLocal<T> (this IBuildEngine4 engine, object key, RegisteredTaskObjectLifetime lifetime)
282302
where T : class =>
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
using System.IO;
2+
using System.Collections.Generic;
3+
using System.Threading.Tasks;
4+
using Microsoft.Android.Build.BaseTasks.Tests.Utilities;
5+
using Microsoft.Android.Build.Tasks;
6+
using NUnit.Framework;
7+
using Microsoft.Build.Framework;
8+
using Xamarin.Build;
9+
10+
namespace Microsoft.Android.Build.BaseTasks.Tests
11+
{
12+
[TestFixture]
13+
public class AndroidToolTaskTests
14+
{
15+
public class MyAndroidTask : AndroidTask {
16+
public override string TaskPrefix {get;} = "MAT";
17+
public string Key { get; set; }
18+
public string Value { get; set; }
19+
public bool ProjectSpecific { get; set; } = false;
20+
public override bool RunTask ()
21+
{
22+
var key = ProjectSpecific ? ProjectSpecificTaskObjectKey (Key) : (Key, (object)string.Empty);
23+
BuildEngine4.RegisterTaskObjectAssemblyLocal (key, Value, RegisteredTaskObjectLifetime.Build);
24+
return true;
25+
}
26+
}
27+
28+
public class MyOtherAndroidTask : AndroidTask {
29+
public override string TaskPrefix {get;} = "MOAT";
30+
public string Key { get; set; }
31+
public bool ProjectSpecific { get; set; } = false;
32+
33+
[Output]
34+
public string Value { get; set; }
35+
public override bool RunTask ()
36+
{
37+
var key = ProjectSpecific ? ProjectSpecificTaskObjectKey (Key) : (Key, (object)string.Empty);
38+
Value = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal<string> (key, RegisteredTaskObjectLifetime.Build);
39+
return true;
40+
}
41+
}
42+
43+
[Test]
44+
[TestCase (true, true, true)]
45+
[TestCase (false, false, true)]
46+
[TestCase (true, false, false)]
47+
[TestCase (false, true, false)]
48+
public void TestRegisterTaskObjectCanRetrieveCorrectItem (bool projectSpecificA, bool projectSpecificB, bool expectedResult)
49+
{
50+
var engine = new MockBuildEngine (TestContext.Out) {
51+
};
52+
var task = new MyAndroidTask () {
53+
BuildEngine = engine,
54+
Key = "Foo",
55+
Value = "Foo",
56+
ProjectSpecific = projectSpecificA,
57+
};
58+
task.Execute ();
59+
var task2 = new MyOtherAndroidTask () {
60+
BuildEngine = engine,
61+
Key = "Foo",
62+
ProjectSpecific = projectSpecificB,
63+
};
64+
task2.Execute ();
65+
Assert.AreEqual (expectedResult, string.Compare (task2.Value, task.Value, ignoreCase: true) == 0);
66+
}
67+
68+
[Test]
69+
[TestCase (true, true, false)]
70+
[TestCase (false, false, true)]
71+
[TestCase (true, false, false)]
72+
[TestCase (false, true, false)]
73+
public void TestRegisterTaskObjectFailsWhenDirectoryChanges (bool projectSpecificA, bool projectSpecificB, bool expectedResult)
74+
{
75+
var engine = new MockBuildEngine (TestContext.Out) {
76+
};
77+
MyAndroidTask task;
78+
var currentDir = Directory.GetCurrentDirectory ();
79+
Directory.SetCurrentDirectory (Path.Combine (currentDir, ".."));
80+
try {
81+
task = new MyAndroidTask () {
82+
BuildEngine = engine,
83+
Key = "Foo",
84+
Value = "Foo",
85+
ProjectSpecific = projectSpecificA,
86+
};
87+
} finally {
88+
Directory.SetCurrentDirectory (currentDir);
89+
}
90+
task.Execute ();
91+
var task2 = new MyOtherAndroidTask () {
92+
BuildEngine = engine,
93+
Key = "Foo",
94+
ProjectSpecific = projectSpecificB,
95+
};
96+
task2.Execute ();
97+
Assert.AreEqual (expectedResult, string.Compare (task2.Value, task.Value, ignoreCase: true) == 0);
98+
}
99+
}
100+
}
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
using System;
2+
using System.Collections;
3+
using System.Collections.Generic;
4+
using System.IO;
5+
using Microsoft.Build.Framework;
6+
7+
namespace Microsoft.Android.Build.BaseTasks.Tests.Utilities {
8+
public class MockBuildEngine : IBuildEngine, IBuildEngine2, IBuildEngine3, IBuildEngine4 {
9+
public MockBuildEngine (TextWriter output, IList<BuildErrorEventArgs> errors = null, IList<BuildWarningEventArgs> warnings = null, IList<BuildMessageEventArgs> messages = null, IList<CustomBuildEventArgs> customEvents = null)
10+
{
11+
this.Output = output;
12+
this.Errors = errors;
13+
this.Warnings = warnings;
14+
this.Messages = messages;
15+
this.CustomEvents = customEvents;
16+
}
17+
18+
private TextWriter Output { get; }
19+
20+
private IList<BuildErrorEventArgs> Errors { get; }
21+
22+
private IList<BuildWarningEventArgs> Warnings { get; }
23+
24+
private IList<BuildMessageEventArgs> Messages { get; }
25+
26+
private IList<CustomBuildEventArgs> CustomEvents { get; }
27+
28+
int IBuildEngine.ColumnNumberOfTaskNode => -1;
29+
30+
bool IBuildEngine.ContinueOnError => false;
31+
32+
int IBuildEngine.LineNumberOfTaskNode => -1;
33+
34+
string IBuildEngine.ProjectFileOfTaskNode => "this.xml";
35+
36+
bool IBuildEngine2.IsRunningMultipleNodes => false;
37+
38+
bool IBuildEngine.BuildProjectFile (string projectFileName, string [] targetNames, IDictionary globalProperties, IDictionary targetOutputs) => true;
39+
40+
void IBuildEngine.LogCustomEvent (CustomBuildEventArgs e)
41+
{
42+
this.Output.WriteLine ($"Custom: {e.Message}");
43+
if (CustomEvents != null)
44+
CustomEvents.Add (e);
45+
}
46+
47+
void IBuildEngine.LogErrorEvent (BuildErrorEventArgs e)
48+
{
49+
this.Output.WriteLine ($"Error: {e.Message}");
50+
if (Errors != null)
51+
Errors.Add (e);
52+
}
53+
54+
void IBuildEngine.LogMessageEvent (BuildMessageEventArgs e)
55+
{
56+
this.Output.WriteLine ($"Message: {e.Message}");
57+
if (Messages != null)
58+
Messages.Add (e);
59+
}
60+
61+
void IBuildEngine.LogWarningEvent (BuildWarningEventArgs e)
62+
{
63+
this.Output.WriteLine ($"Warning: {e.Message}");
64+
if (Warnings != null)
65+
Warnings.Add (e);
66+
}
67+
68+
private Dictionary<object, object> Tasks = new Dictionary<object, object> ();
69+
70+
void IBuildEngine4.RegisterTaskObject (object key, object obj, RegisteredTaskObjectLifetime lifetime, bool allowEarlyCollection)
71+
{
72+
Tasks.Add (key, obj);
73+
}
74+
75+
object IBuildEngine4.GetRegisteredTaskObject (object key, RegisteredTaskObjectLifetime lifetime)
76+
{
77+
object obj = null;
78+
Tasks.TryGetValue (key, out obj);
79+
return obj;
80+
}
81+
82+
object IBuildEngine4.UnregisterTaskObject (object key, RegisteredTaskObjectLifetime lifetime)
83+
{
84+
var obj = Tasks [key];
85+
Tasks.Remove (key);
86+
return obj;
87+
}
88+
89+
BuildEngineResult IBuildEngine3.BuildProjectFilesInParallel (string [] projectFileNames, string [] targetNames, IDictionary [] globalProperties, IList<string> [] removeGlobalProperties, string [] toolsVersion, bool returnTargetOutputs)
90+
{
91+
throw new NotImplementedException ();
92+
}
93+
94+
void IBuildEngine3.Yield () { }
95+
96+
void IBuildEngine3.Reacquire () { }
97+
98+
bool IBuildEngine2.BuildProjectFile (string projectFileName, string [] targetNames, IDictionary globalProperties, IDictionary targetOutputs, string toolsVersion) => true;
99+
100+
bool IBuildEngine2.BuildProjectFilesInParallel (string [] projectFileNames, string [] targetNames, IDictionary [] globalProperties, IDictionary [] targetOutputsPerProject, string [] toolsVersion, bool useResultsCache, bool unloadProjectsOnCompletion) => true;
101+
}
102+
}

0 commit comments

Comments
 (0)