Skip to content
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.

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

Fix CS0121 ambiguity errors. #200

Merged
merged 1 commit into from
Jan 11, 2023

Conversation

jonpryor
Copy link
Member

@jonpryor jonpryor commented Jan 11, 2023

Context: 76c076f

When attempting to update xamarin/xamarin-android to use 4ea2d5ad, lots of CS0121 errors appeared:

…/src/Xamarin.Android.Build.Tasks/Tasks/FilterAssemblies.cs(80,18): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
…/src/Xamarin.Android.Build.Tasks/Tasks/FilterAssemblies.cs(87,18): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
…/src/Xamarin.Android.Build.Tasks/Tasks/FilterAssemblies.cs(94,18): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
…/src/Xamarin.Android.Build.Tasks/Tasks/FilterAssemblies.cs(98,17): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
…/src/Xamarin.Android.Build.Tasks/Tasks/GenerateCompressedAssembliesNativeSourceFiles.cs(72,17): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
…/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJniRemappingNativeCode.cs(95,17): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
…/src/Xamarin.Android.Build.Tasks/Tasks/WriteLockFile.cs(35,19): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
…/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs(524,18): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
…/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs(578,17): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
…/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs(393,11): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
…/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs(440,11): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
…/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs(501,11): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'

This is because when given e.g.:

partial class MSBuildExtensions {
    public static void RegisterTaskObjectAssemblyLocal (this IBuildEngine4 engine, object key, object value, RegisteredTaskObjectLifetime lifetime, bool allowEarlyCollection = false);
    public static void RegisterTaskObjectAssemblyLocal (this IBuildEngine4 engine, object key, object value, RegisteredTaskObjectLifetime lifetime, bool allowEarlyCollection = false, RegisterTaskObjectKeyFlags flags = RegisterTaskObjectKeyFlags.IncludeProjectFile);
}

then this expression:

engine.RegisterTaskObjectAssemblyLocal(key, value, lifetime);

has two possible matches, due to the default parameters.

Fix this by removing the default RegisteredTaskObjectLifetime parameter value, and (when appropriate) adding an overload:

partial class MSBuildExtensions {
    public static void RegisterTaskObjectAssemblyLocal (this IBuildEngine4 engine, object key, object value, RegisteredTaskObjectLifetime lifetime, bool allowEarlyCollection = false);
    public static void RegisterTaskObjectAssemblyLocal (this IBuildEngine4 engine, object key, object value, RegisteredTaskObjectLifetime lifetime, RegisterTaskObjectKeyFlags flagse);
    public static void RegisterTaskObjectAssemblyLocal (this IBuildEngine4 engine, object key, object value, RegisteredTaskObjectLifetime lifetime, bool allowEarlyCollection, RegisterTaskObjectKeyFlags flags);
}

Additionally, remove the [Obsolete] attributes. With RegisterTaskObjectKeyFlags.IncludeProjectFile part of the default behavior, there is no need to mark the original methods [Obsolete]. This allows existing code such as:

var value = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal (key, Lifetime);

to remain unchanged.

@jonpryor jonpryor force-pushed the jonp-remove-CS0121-ambiguity branch from f345f5f to 5b94219 Compare January 11, 2023 19:04
Context: 76c076f

When attempting to update xamarin/xamarin-android to use 4ea2d5ad,
lots of CS0121 errors appeared:

	…/src/Xamarin.Android.Build.Tasks/Tasks/FilterAssemblies.cs(80,18): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
	…/src/Xamarin.Android.Build.Tasks/Tasks/FilterAssemblies.cs(87,18): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
	…/src/Xamarin.Android.Build.Tasks/Tasks/FilterAssemblies.cs(94,18): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
	…/src/Xamarin.Android.Build.Tasks/Tasks/FilterAssemblies.cs(98,17): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
	…/src/Xamarin.Android.Build.Tasks/Tasks/GenerateCompressedAssembliesNativeSourceFiles.cs(72,17): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
	…/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJniRemappingNativeCode.cs(95,17): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
	…/src/Xamarin.Android.Build.Tasks/Tasks/WriteLockFile.cs(35,19): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
	…/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs(524,18): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
	…/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs(578,17): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
	…/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs(393,11): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
	…/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs(440,11): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'
	…/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs(501,11): error CS0121: The call is ambiguous between the following methods or properties: 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool)' and 'MSBuildExtensions.RegisterTaskObjectAssemblyLocal(IBuildEngine4, object, object, RegisteredTaskObjectLifetime, bool, RegisterTaskObjectKeyFlags)'

This is because when given e.g.:

	partial class MSBuildExtensions {
	    public static void RegisterTaskObjectAssemblyLocal (this IBuildEngine4 engine, object key, object value, RegisteredTaskObjectLifetime lifetime, bool allowEarlyCollection = false);
	    public static void RegisterTaskObjectAssemblyLocal (this IBuildEngine4 engine, object key, object value, RegisteredTaskObjectLifetime lifetime, bool allowEarlyCollection = false, RegisterTaskObjectKeyFlags flags = RegisterTaskObjectKeyFlags.IncludeProjectFile);
	}

then this expression:

	engine.RegisterTaskObjectAssemblyLocal(key, value, lifetime);

has two possible matches, due to the default parameters.

Fix this by *removing* the default `RegisteredTaskObjectLifetime`
parameter value, and (when appropriate) adding an overload:

	partial class MSBuildExtensions {
	    public static void RegisterTaskObjectAssemblyLocal (this IBuildEngine4 engine, object key, object value, RegisteredTaskObjectLifetime lifetime, bool allowEarlyCollection = false);
	    public static void RegisterTaskObjectAssemblyLocal (this IBuildEngine4 engine, object key, object value, RegisteredTaskObjectLifetime lifetime, RegisterTaskObjectKeyFlags flagse);
	    public static void RegisterTaskObjectAssemblyLocal (this IBuildEngine4 engine, object key, object value, RegisteredTaskObjectLifetime lifetime, bool allowEarlyCollection, RegisterTaskObjectKeyFlags flags);
	}

Additionally, *remove* the `[Obsolete]` attributes.  With
`RegisterTaskObjectKeyFlags.IncludeProjectFile` part of the default
behavior, there is no need to mark the original methods `[Obsolete]`.
This allows existing code such as:

	var value = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal (key, Lifetime);

to remain unchanged.
@jonpryor jonpryor force-pushed the jonp-remove-CS0121-ambiguity branch from 5b94219 to 970aa2d Compare January 11, 2023 19:06
@jonpryor
Copy link
Member Author

jonpryor commented Jan 11, 2023

diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/AndroidDotnetToolTask.cs b/src/Xamarin.Android.Build.Tasks/Tasks/AndroidDotnetToolTask.cs
index efa01c484..8f26c94f8 100644
--- a/src/Xamarin.Android.Build.Tasks/Tasks/AndroidDotnetToolTask.cs
+++ b/src/Xamarin.Android.Build.Tasks/Tasks/AndroidDotnetToolTask.cs
@@ -105,7 +105,7 @@ namespace Xamarin.Android.Tasks
 
 		string FindMono ()
 		{
-			string mono = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal<string> (MonoKey, Lifetime);
+			string mono = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal<string> (MonoKey, Lifetime, flags: RegisterTaskObjectKeyFlags.None);
 			if (!string.IsNullOrEmpty (mono)) {
 				Log.LogDebugMessage ($"Found cached mono via {nameof (BuildEngine4.RegisterTaskObject)}");
 				return mono;
@@ -116,7 +116,7 @@ namespace Xamarin.Android.Tasks
 				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 @@ namespace Xamarin.Android.Tasks
 			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 ec6aa25a0..c89b73bc9 100644
--- a/src/Xamarin.Android.Build.Tasks/Utilities/Aapt2Daemon.cs
+++ b/src/Xamarin.Android.Build.Tasks/Utilities/Aapt2Daemon.cs
@@ -22,11 +22,12 @@ namespace Xamarin.Android.Tasks
 		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<Aapt2Daemon> (RegisterTaskObjectKey, area);
+			var flags = registerInDomain ? RegisterTaskObjectKeyFlags.None : RegisterTaskObjectKeyFlags.IncludeProjectFile;
+			var daemon = engine.GetRegisteredTaskObjectAssemblyLocal<Aapt2Daemon> (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;
 		}

@@ -258,68 +258,69 @@ public static void SetDestinationSubPath (this ITaskItem assembly)
/// <summary>
/// IBuildEngine4.RegisterTaskObject, but adds the current assembly path into the key
/// </summary>
[Obsolete ("Use RegisterTaskObjectAssemblyLocal (engine, key, value, allowEarlyCollection, lifetime, flags) instead.")]
Copy link
Member

Choose a reason for hiding this comment

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

The only question I had was if we wanted the warning, and I think the answer was no:

https://discord.com/channels/732297728826277939/732297837953679412/1062810262782087228

Copy link
Member Author

Choose a reason for hiding this comment

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

At the time we thought we wanted the warning, but when attempting to bump I saw lots of warnings that would result in the code being "worse" for no real benefit; this:

var value = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal (key, Lifetime);

would need to become:

var value = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal (key, Lifetime, RegisterTaskObjectKeyFlags.IncludeProjectFile);

…but that was the default behavior for the [Obsolete] methods!

It felt like a net reduction in clarity, not an improvement.

@jonpryor jonpryor merged commit 47f95ab into dotnet:main Jan 11, 2023
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Jan 11, 2023
Context: dotnet/maui#11605
Context: 8bc7a3e

Changes: http://github.com/xamarin/xamarin-android-tools/compare/9f02d77692bca8c6585941de03750d5eaaca5c5a...47f95ab99f6201d956eecfa1c7b2fd5fa7e43946

  * dotnet/android-tools@47f95ab: Fix CS0121 ambiguity errors. (dotnet/android-tools#200)
  * dotnet/android-tools@76c076f: Add support for Project Specific RegisterTaskObject. (dotnet/android-tools#199)

In dotnet/maui#11605, when `$(AndroidEnableMarshalMethods)`=True
(8bc7a3e), 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 in the process, and *also*
    calls `<GeneratePackageManagerJava/>`.

In particular note the key within `<GeneratePackageManagerJava/>`:
`".:!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.

dotnet/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
jonpryor added a commit to jonpryor/java.interop that referenced this pull request Jan 12, 2023
Changes: dotnet/android-tools@29f11f2...47f95ab

  * dotnet/android-tools@47f95ab: Fix CS0121 ambiguity errors. (dotnet/android-tools#200)
  * dotnet/android-tools@76c076f: Add support for Project Specific RegisterTaskObject. (dotnet/android-tools#199)
  * dotnet/android-tools@9f02d77: Add reference to System.Security.Cryptography.Xml (dotnet/android-tools#198)
  * dotnet/android-tools@fa3711b: [build] Update NuGet package versions (dotnet/android-tools#196)
  * dotnet/android-tools@59cac90: Enable CodeQL (dotnet/android-tools#197)
  * dotnet/android-tools@9f56dec: Move from `netcoreapp3.1` to `net6.0` (dotnet/android-tools#195)
  * dotnet/android-tools@0be567a: Use Environment.SpecialFolder.UserProfile, not SpecialFolder.Personal (dotnet/android-tools#194)
jonpryor added a commit to jonpryor/java.interop that referenced this pull request Jan 18, 2023
Changes: dotnet/android-tools@29f11f2...099fd95

  * dotnet/android-tools@099fd95: Add *Task.ProjectSpecificTaskObjectKey() for RegisterTaskObject() use (dotnet/android-tools#202)
  * dotnet/android-tools@ac9ea09: Revert IBuildEngine.ProjectFileOfTaskNode use. (dotnet/android-tools#201)
  * dotnet/android-tools@47f95ab: Fix CS0121 ambiguity errors. (dotnet/android-tools#200)
  * dotnet/android-tools@76c076f: Add support for Project Specific RegisterTaskObject. (dotnet/android-tools#199)
  * dotnet/android-tools@9f02d77: Add reference to System.Security.Cryptography.Xml (dotnet/android-tools#198)
  * dotnet/android-tools@fa3711b: [build] Update NuGet package versions (dotnet/android-tools#196)
  * dotnet/android-tools@59cac90: Enable CodeQL (dotnet/android-tools#197)
  * dotnet/android-tools@9f56dec: Move from `netcoreapp3.1` to `net6.0` (dotnet/android-tools#195)
  * dotnet/android-tools@0be567a: Use Environment.SpecialFolder.UserProfile, not SpecialFolder.Personal (dotnet/android-tools#194)
jonpryor pushed a commit to dotnet/android that referenced this pull request Jan 19, 2023
Context: dotnet/maui#11605
Context: dotnet/maui#11387 (comment)
Context: http://github.com/xamarin/xamarin-android/commit/8bc7a3e84f95e70fe12790ac31ecd97957771cb2

Changes: dotnet/android-tools@9f02d77...099fd95

  * dotnet/android-tools@099fd95: Add *Task.ProjectSpecificTaskObjectKey() for RegisterTaskObject() use (dotnet/android-tools#202)
  * dotnet/android-tools@ac9ea09: Revert IBuildEngine.ProjectFileOfTaskNode use. (dotnet/android-tools#201)
  * dotnet/android-tools@47f95ab: Fix CS0121 ambiguity errors. (dotnet/android-tools#200)
  * dotnet/android-tools@76c076f: Add support for Project Specific RegisterTaskObject. (dotnet/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 [`<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/>`, e.g. the new
`IncrementalBuildTest.BuildSolutionWithMultipleProjectsInParallel()`
unit test.  The lifetime of `RegisteredTaskObjectLifetime.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.

dotnet/android-tools@099fd95 added the methods
`AndroidTask.ProjectSpecificTaskObjectKey(object)`, 
`AndroidAsyncTask.ProjectSpecificTaskObjectKey(object)`, and
`AndroidToolTask.ProjectSpecificTaskObjectKey(object)` which returns
an `object` for use as the key to `RegisteredTaskObject()`, and the
value is a tuple which includes the input `object` *and* the
`Directory.GetCurrentDirectory()` value present when the task was
created.  (Background note: when MSBuild builds a project, it calls
`Directory.SetCurrentDirectory()` to the directory of the current
project, which is why relative file paths to work in `.csproj` files.)

Review use of the `MSBuildExtensions.RegisterTaskObjectAssemblyLocal()`
extension method and audit the key value.  If the registered value is
project specific, update the callsite to use
`ProjectSpecificTaskObjectKey(object)`, ensuring that project-specific
data is not accidentally reused in a different project.

[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
jonpryor added a commit to dotnet/java-interop that referenced this pull request Jan 25, 2023
Changes: dotnet/android-tools@29f11f2...099fd95

  * dotnet/android-tools@099fd95: Add *Task.ProjectSpecificTaskObjectKey() for RegisterTaskObject() use (dotnet/android-tools#202)
  * dotnet/android-tools@ac9ea09: Revert IBuildEngine.ProjectFileOfTaskNode use. (dotnet/android-tools#201)
  * dotnet/android-tools@47f95ab: Fix CS0121 ambiguity errors. (dotnet/android-tools#200)
  * dotnet/android-tools@76c076f: Add support for Project Specific RegisterTaskObject. (dotnet/android-tools#199)
  * dotnet/android-tools@9f02d77: Add reference to System.Security.Cryptography.Xml (dotnet/android-tools#198)
  * dotnet/android-tools@fa3711b: [build] Update NuGet package versions (dotnet/android-tools#196)
  * dotnet/android-tools@59cac90: Enable CodeQL (dotnet/android-tools#197)
  * dotnet/android-tools@9f56dec: Move from `netcoreapp3.1` to `net6.0` (dotnet/android-tools#195)
  * dotnet/android-tools@0be567a: Use Environment.SpecialFolder.UserProfile, not SpecialFolder.Personal (dotnet/android-tools#194)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants