Skip to content

Commit

Permalink
Add support for Project Specific RegisterTaskObject. (#199)
Browse files Browse the repository at this point in the history
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
(xamarin/xamarin-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 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 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
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 the `MSBuildExtensions` extension methods to include an
additional `RegisterTaskObjectKeyFlags` parameter:

	[Flags]
	public enum RegisterTaskObjectKeyFlags {
		None = 0,
		IncludeProjectFile = 1 << 0,
	}

Allowing:

	var marshalMethodsState = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal<MarshalMethodsState> (
	        ".:!MarshalMethods!:.",
	        RegisteredTaskObjectLifetime.Build,
	        RegisterTaskObjectKeyFlags.IncludeProjectFile
	);

When `RegisterTaskObjectKeyFlags.IncludeProjectFile` is specified,
then [`IBuildEngine.ProjectFileOfTaskNode`][2] is used as part of
the key with `RegisterTaskObject()`.  This helps ensure that builds
in different `.csproj` files will result in different keys.

The previous `MSBuildExtensions.GetRegisteredTaskObjectAssemblyLocal()`
and related overloads have been updated so that
`RegisterTaskObjectKeyFlags.IncludeProjectFile` is used by default.

[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
  • Loading branch information
dellis1972 committed Jan 11, 2023
1 parent 9f02d77 commit 76c076f
Showing 1 changed file with 58 additions and 6 deletions.
64 changes: 58 additions & 6 deletions src/Microsoft.Android.Build.BaseTasks/MSBuildExtensions.cs
Expand Up @@ -13,6 +13,12 @@

namespace Microsoft.Android.Build.Tasks
{
[Flags]
public enum RegisterTaskObjectKeyFlags {
None = 0,
IncludeProjectFile = 1 << 0,
}

public static class MSBuildExtensions
{
public static void LogDebugMessage (this TaskLoggingHelper log, string message, params object[] messageArgs)
Expand Down Expand Up @@ -252,34 +258,80 @@ 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.")]
public static void RegisterTaskObjectAssemblyLocal (this IBuildEngine4 engine, object key, object value, RegisteredTaskObjectLifetime lifetime, bool allowEarlyCollection = false) =>
engine.RegisterTaskObject ((AssemblyLocation, key), value, lifetime, allowEarlyCollection);
RegisterTaskObjectAssemblyLocal (engine, key, value, lifetime, allowEarlyCollection: false, flags: RegisterTaskObjectKeyFlags.IncludeProjectFile);

/// <summary>
/// IBuildEngine4.RegisterTaskObject, but adds the current assembly path into the key
/// </summary>
public static void RegisterTaskObjectAssemblyLocal (this IBuildEngine4 engine, object key, object value, RegisteredTaskObjectLifetime lifetime, bool allowEarlyCollection = false, RegisterTaskObjectKeyFlags flags = RegisterTaskObjectKeyFlags.IncludeProjectFile) =>
engine.RegisterTaskObject (engine.GetKey (AssemblyLocation, key, flags), value, lifetime, allowEarlyCollection);

/// <summary>
/// IBuildEngine4.GetRegisteredTaskObject, but adds the current assembly path into the key
/// </summary>
[Obsolete ("Use GetRegisteredTaskObjectAssemblyLocal (engine, key, lifetime, flags) instead.")]
public static object GetRegisteredTaskObjectAssemblyLocal (this IBuildEngine4 engine, object key, RegisteredTaskObjectLifetime lifetime) =>
engine.GetRegisteredTaskObject ((AssemblyLocation, key), lifetime);
GetRegisteredTaskObjectAssemblyLocal (engine, key, lifetime, flags: RegisterTaskObjectKeyFlags.IncludeProjectFile);

/// <summary>
/// IBuildEngine4.GetRegisteredTaskObject, but adds the current assembly path into the key
/// </summary>
public static object GetRegisteredTaskObjectAssemblyLocal (this IBuildEngine4 engine, object key, RegisteredTaskObjectLifetime lifetime, RegisterTaskObjectKeyFlags flags = RegisterTaskObjectKeyFlags.IncludeProjectFile) =>
engine.GetRegisteredTaskObject (engine.GetKey (AssemblyLocation, key, flags), lifetime);


/// <summary>
/// Generic version of IBuildEngine4.GetRegisteredTaskObject, but adds the current assembly path into the key
/// </summary>
[Obsolete ("Use GetRegisteredTaskObjectAssemblyLocal<T> (engine, key, lifetime, flags) instead.")]
public static T GetRegisteredTaskObjectAssemblyLocal<T> (this IBuildEngine4 engine, object key, RegisteredTaskObjectLifetime lifetime)
where T : class =>
engine.GetRegisteredTaskObject ((AssemblyLocation, key), lifetime) as T;
where T : class => GetRegisteredTaskObjectAssemblyLocal<T> (engine, key, lifetime, flags: RegisterTaskObjectKeyFlags.IncludeProjectFile);

/// <summary>
/// Generic version of IBuildEngine4.GetRegisteredTaskObject, but adds the current assembly path into the key
/// </summary>
public static T GetRegisteredTaskObjectAssemblyLocal<T> (this IBuildEngine4 engine, object key, RegisteredTaskObjectLifetime lifetime, RegisterTaskObjectKeyFlags flags = RegisterTaskObjectKeyFlags.IncludeProjectFile)
where T : class =>
engine.GetRegisteredTaskObject (engine.GetKey (AssemblyLocation, key, flags), lifetime) as T;

/// <summary>
/// IBuildEngine4.UnregisterTaskObject, but adds the current assembly path into the key
/// </summary>
[Obsolete ("Use UnregisterTaskObjectAssemblyLocal (engine, key, lifetime, flags) instead.")]
public static object UnregisterTaskObjectAssemblyLocal (this IBuildEngine4 engine, object key, RegisteredTaskObjectLifetime lifetime) =>
engine.UnregisterTaskObject ((AssemblyLocation, key), lifetime);
UnregisterTaskObjectAssemblyLocal (engine, key, lifetime, flags: RegisterTaskObjectKeyFlags.IncludeProjectFile);

/// <summary>
/// IBuildEngine4.UnregisterTaskObject, but adds the current assembly path into the key
/// </summary>
public static object UnregisterTaskObjectAssemblyLocal (this IBuildEngine4 engine, object key, RegisteredTaskObjectLifetime lifetime, RegisterTaskObjectKeyFlags flags = RegisterTaskObjectKeyFlags.IncludeProjectFile) =>
engine.UnregisterTaskObject (engine.GetKey (AssemblyLocation, key, flags), lifetime);

/// <summary>
/// Generic version of IBuildEngine4.UnregisterTaskObject, but adds the current assembly path into the key
/// </summary>
[Obsolete ("Use UnregisterTaskObjectAssemblyLocal<T> (engine, key, lifetime, flags) instead.")]
public static T UnregisterTaskObjectAssemblyLocal<T> (this IBuildEngine4 engine, object key, RegisteredTaskObjectLifetime lifetime)
where T : class => UnregisterTaskObjectAssemblyLocal<T> (engine, key, lifetime, flags: RegisterTaskObjectKeyFlags.IncludeProjectFile);

/// <summary>
/// Generic version of IBuildEngine4.UnregisterTaskObject, but adds the current assembly path into the key
/// </summary>
public static T UnregisterTaskObjectAssemblyLocal<T> (this IBuildEngine4 engine, object key, RegisteredTaskObjectLifetime lifetime, RegisterTaskObjectKeyFlags flags = RegisterTaskObjectKeyFlags.IncludeProjectFile)
where T : class =>
engine.UnregisterTaskObject ((AssemblyLocation, key), lifetime) as T;
engine.UnregisterTaskObject (engine.GetKey (AssemblyLocation, key, flags), lifetime) as T;

/// <summary>
/// Method to calculate the key for the RegisterTaskObject. This is based on the
/// RegisterTaskObjectKeyFlags which are passed.
/// </summary>
static object GetKey (this IBuildEngine4 engine, string location, object key, RegisterTaskObjectKeyFlags flags)
{
return ((flags & RegisterTaskObjectKeyFlags.IncludeProjectFile) != 0)
? (location, key, engine.ProjectFileOfTaskNode)
: (location, key, string.Empty);
}
}
}

0 comments on commit 76c076f

Please sign in to comment.