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

[net8] Review changes related with AndroidEnableMarshalMethods on net branch #11605

Closed
rmarinho opened this issue Nov 24, 2022 · 7 comments
Closed
Assignees
Labels
fixed-in-8.0.0-preview.1.7762 Look for this fix in 8.0.0-preview.1.7762! partner/android Issues for the Android SDK platform/android 🤖 t/bug Something isn't working

Comments

@rmarinho
Copy link
Member

rmarinho commented Nov 24, 2022

Description

WHen moving to net8, when building the Android we got some errors:

#11387 (comment)

D:\a\_work\1\s\bin\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.0-preview.1.50\tools\Xamarin.Android.Common.targets(1717,3): error XAGPM7009: System.InvalidOperationException: Unable to translate assembly name 'Xamarin.GooglePlayServices.Base' to its index [D:\a\_work\1\s\src\BlazorWebView\tests\MauiDeviceTests\MauiBlazorWebView.DeviceTests.csproj::TargetFramework=net8.0-android] 
D:\a\_work\1\s\bin\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.0-preview.1.50\tools\Xamarin.Android.Common.targets(1717,3): error XAGPM7009: at Xamarin.Android.Tasks.MarshalMethodsNativeAssemblyGenerator.WriteNativeMethods(LlvmIrGenerator generator, Dictionary`2 asmNameToIndex, LlvmIrVariableReference get_function_pointer_ref) [D:\a\_work\1\s\src\BlazorWebView\tests\MauiDeviceTests\MauiBlazorWebView.DeviceTests.csproj::TargetFramework=net8.0-android] 
D:\a\_work\1\s\bin\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.0-preview.1.50\tools\Xamarin.Android.Common.targets(1717,3): error XAGPM7009: at Xamarin.Android.Tasks.MarshalMethodsNativeAssemblyGenerator.Write(LlvmIrGenerator generator) [D:\a\_work\1\s\src\BlazorWebView\tests\MauiDeviceTests\MauiBlazorWebView.DeviceTests.csproj::TargetFramework=net8.0-android] 
D:\a\_work\1\s\bin\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.0-preview.1.50\tools\Xamarin.Android.Common.targets(1717,3): error XAGPM7009: at Xamarin.Android.Tasks.LLVMIR.LlvmIrComposer.Write(AndroidTargetArch arch, StreamWriter output, String fileName) [D:\a\_work\1\s\src\BlazorWebView\tests\MauiDeviceTests\MauiBlazorWebView.DeviceTests.csproj::TargetFramework=net8.0-android] 
D:\a\_work\1\s\bin\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.0-preview.1.50\tools\Xamarin.Android.Common.targets(1717,3): error XAGPM7009: at Xamarin.Android.Tasks.GeneratePackageManagerJava.AddEnvironment() [D:\a\_work\1\s\src\BlazorWebView\tests\MauiDeviceTests\MauiBlazorWebView.DeviceTests.csproj::TargetFramework=net8.0-android] 
D:\a\_work\1\s\bin\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.0-preview.1.50\tools\Xamarin.Android.Common.targets(1717,3): error XAGPM7009: at Xamarin.Android.Tasks.GeneratePackageManagerJava.RunTask() [D:\a\_work\1\s\src\BlazorWebView\tests\MauiDeviceTests\MauiBlazorWebView.DeviceTests.csproj::TargetFramework=net8.0-android] 
D:\a\_work\1\s\bin\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.0-preview.1.50\tools\Xamarin.Android.Common.targets(1717,3): error XAGPM7009: at Microsoft.Android.Build.Tasks.AndroidTask.Execute() in /Users/runner/work/1/s/xamarin-android/external/xamarin-android-tools/src/Microsoft.Android.Build.BaseTasks/AndroidTask.cs:line 22 [D:\a\_work\1\s\src\BlazorWebView\tests\MauiDeviceTests\MauiBlazorWebView.DeviceTests.csproj::TargetFramework=net8.0-android] 
D:\a\_work\1\s\bin\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.0-preview.1.50\tools\Xamarin.Android.Common.targets(1717,3): error XAGPM7009: System.InvalidOperationException: Unable to translate assembly name 'Xamarin.AndroidX.Activity' to its index [D:\a\_work\1\s\src\Graphics\samples\GraphicsTester.Android\GraphicsTester.Android.csproj] 
D:\a\_work\1\s\bin\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.0-preview.1.50\tools\Xamarin.Android.Common.targets(1717,3): error XAGPM7009: at Xamarin.Android.Tasks.MarshalMethodsNativeAssemblyGenerator.WriteNativeMethods(LlvmIrGenerator generator, Dictionary`2 asmNameToIndex, LlvmIrVariableReference get_function_pointer_ref) [D:\a\_work\1\s\src\Graphics\samples\GraphicsTester.Android\GraphicsTester.Android.csproj] 
D:\a\_work\1\s\bin\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.0-preview.1.50\tools\Xamarin.Android.Common.targets(1717,3): error XAGPM7009: at Xamarin.Android.Tasks.MarshalMethodsNativeAssemblyGenerator.Write(LlvmIrGenerator generator) [D:\a\_work\1\s\src\Graphics\samples\GraphicsTester.Android\GraphicsTester.Android.csproj] 
D:\a\_work\1\s\bin\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.0-preview.1.50\tools\Xamarin.Android.Common.targets(1717,3): error XAGPM7009: at Xamarin.Android.Tasks.LLVMIR.LlvmIrComposer.Write(AndroidTargetArch arch, StreamWriter output, String fileName) [D:\a\_work\1\s\src\Graphics\samples\GraphicsTester.Android\GraphicsTester.Android.csproj] 
D:\a\_work\1\s\bin\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.0-preview.1.50\tools\Xamarin.Android.Common.targets(1717,3): error XAGPM7009: at Xamarin.Android.Tasks.GeneratePackageManagerJava.AddEnvironment() [D:\a\_work\1\s\src\Graphics\samples\GraphicsTester.Android\GraphicsTester.Android.csproj] 
D:\a\_work\1\s\bin\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.0-preview.1.50\tools\Xamarin.Android.Common.targets(1717,3): error XAGPM7009: at Xamarin.Android.Tasks.GeneratePackageManagerJava.RunTask() [D:\a\_work\1\s\src\Graphics\samples\GraphicsTester.Android\GraphicsTester.Android.csproj] 
D:\a\_work\1\s\bin\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.0-preview.1.50\tools\Xamarin.Android.Common.targets(1717,3): error XAGPM7009: at Microsoft.Android.Build.Tasks.AndroidTask.Execute() in /Users/runner/work/1/s/xamarin-android/external/xamarin-android-tools/src/Microsoft.Android.Build.BaseTasks/AndroidTask.cs:line 22 [D:\a\_work\1\s\src\Graphics\samples\GraphicsTester.Android\GraphicsTester.Android.csproj] 
 290 Warning(s) 

We added this commit

0368b19

Steps to Reproduce

1 - Clone maui
2 - Checkout net8 branch
3 - Remove AndroidEnableMarshalMethods

Link to public reproduction project repository

https://github.com/dotnet/maui

Version with bug

Unknown/Other (please specify)

Last version that worked well

Unknown/Other

Affected platforms

Android

Affected platform versions

Android, net8

Did you find any workaround?

false

Relevant log output

No response

@rmarinho rmarinho added t/bug Something isn't working platform/android 🤖 partner/android Issues for the Android SDK labels Nov 24, 2022
@rmarinho rmarinho added this to the .NET 8 Planning milestone Nov 24, 2022
@jonathanpeppers jonathanpeppers changed the title [net8] Review changes related with DisableResizetizer on net branch [net8] Review changes related with AndroidEnableMarshalMethods on net branch Nov 28, 2022
@grendello
Copy link

I tried to reproduce the build issue with XA/main (dotnet/android@8e8439f) and the tip of maui/net8.0 (16f4c7f) and the build worked fine (on macOS):

$ ./build.sh --configuration=Release
ool 'cake.tool' (version '1.2.0') was restored. Available commands: dotnet-cake
Tool 'powershell' (version '7.2.1') was restored. Available commands: pwsh
Tool 'microsoft.dotnet.xharness.cli' (version '1.0.0-prerelease.22569.1') was restored. Available commands: xharness
Tool 'api-tools' (version '1.3.5') was restored. Available commands: api-tools

Restore was successful.
--configuration=Release

# Time passes

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:22:39.35

========================================
dotnet-pack-maui
========================================

# Some more time passes

Task                          Duration            
--------------------------------------------------
dotnet                        00:22:39.8010132    
dotnet-pack-maui              00:02:13.8872096    
dotnet-pack-additional        00:00:37.5861280    
dotnet-pack-library-packs     00:00:00.0038270    
dotnet-pack-docs              00:00:00.5913967    
--------------------------------------------------
Total:                        00:25:31.8696408 

$ echo $?
0

The build was done after removing the change that disabled marshal methods:

diff --git a/Directory.Build.props b/Directory.Build.props
index cccc263c3..20cac6ac7 100644
--- a/Directory.Build.props
+++ b/Directory.Build.props
@@ -158,6 +158,6 @@
     <!-- Disables the transitive restore of packages like Microsoft.AspNetCore.App.Ref, Microsoft.WindowsDesktop.App.Ref -->
     <DisableTransitiveFrameworkReferenceDownloads>true</DisableTransitiveFrameworkReferenceDownloads>
     <!-- TODO: temporarily disable this to get past Android build error in Release builds -->
-    <AndroidEnableMarshalMethods>false</AndroidEnableMarshalMethods>
+    <!-- AndroidEnableMarshalMethods>false</AndroidEnableMarshalMethods -->
   </PropertyGroup>
 </Project>

Is the build command I used the correct one to trigger the issue?

@rmarinho
Copy link
Member Author

Here's the build failure

https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=6977948&view=logs&j=a2b1c795-2687-5fd3-b418-4e4b87c7d580&t=f98b3571-5084-5d4e-0013-9a861ec79559&l=7277

not sure why we don't have bin logs..

But i m running a new build removing the change

@jonathanpeppers
Copy link
Member

@rmarinho so the failing command was this?

./build.ps1 --target=dotnet --configuration=Release

@rmarinho
Copy link
Member Author

@jonathanpeppers yes.. i did another run removing the fix and here is the build and binlog

https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=7021832&view=results

I have a bin log but it's to big to attach

@grendello
Copy link

@rmarinho at which binlog should I look?

@rmarinho
Copy link
Member Author

@grendello i sent you on Teams

dellis1972 added a commit to dellis1972/xamarin-android-tools that referenced this issue Jan 5, 2023
Context dotnet/maui#11605

If a solution has more than one Android "App" project
there is the potential that objects registered using the
`RegisterTaskObject` will be reused between the projects.

In most cases this is NOT the desired outcome. There are
a few instances where it is safe to share the registered
objects between projects. But for most of the time it is
specific to that project that is being built. Historically
we have probably got away with this because "most" users
only have one project.....

So lets update the MSBuildExtensions extension methods to
include an additional parameter. This will control if the
registered object will be "project" scope or "solution" scope.
Yes those are terms I just made up :D.

We do this by including the `engine.ProjectFileOfTaskNode` as
part of the key.

Initially the thought was that everything would be "solution" scope
if the `LiftTime` was set to `AppDomain`. However there are instances
if "solution" scope usage for non "AppDomain" entires. A good example
is the `aapt2` daemon which has a "LiftTime" of "Build" but can be
"solution" scope. This is why we add a new parameter to control this.

The default for this new parameter is `true`, this is to reduce the number
of changes. We assume most items will be "project" scope.
jonpryor pushed a commit to dotnet/android-tools that referenced this issue Jan 11, 2023
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 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
jonpryor added a commit to jonpryor/xamarin-android that referenced this issue 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
dellis1972 added a commit to dellis1972/xamarin-android-tools that referenced this issue Jan 13, 2023
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 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 `AndroidTask` to capture the current directory in a
`WorkingDirectory` property like `AsyncTask` does. Then introduce a new
method `ProjectSpecificTaskObjectKey` which can be used to generate a
key which includes the `WorkingDirectory`. Both `AndroidTask` and
`AndroidAsyncTask` include this new method.

Allowing:

	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://learn.microsoft.com/en-us/dotnet/api/microsoft.build.framework.ibuildengine.projectfileoftasknode?view=msbuild-17-netcore
jonpryor pushed a commit to dotnet/android-tools that referenced this issue Jan 18, 2023
…#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
jonpryor pushed a commit to dotnet/android that referenced this issue 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
jonathanpeppers added a commit that referenced this issue Jan 24, 2023
Changes: dotnet/android@8a20803...c1efcb5
Changes: dotnet/macios@df0151d...6b1b9f3
Changes: dotnet/installer@8c1708f...9962c6a
Changes: dotnet/runtime@5108757...5da4a9e
Changes: dotnet/emsdk@aecb1c7...66b9845

Updates:

* Microsoft.Android.Sdk.Windows: from 34.0.0-preview.1.50 to 34.0.0-preview.1.127
* Microsoft.iOS.Sdk: from 16.1.585-net8-p1 to 16.2.126-net8-p1
* Microsoft.Dotnet.Sdk.Internal: from 8.0.100-alpha.1.22526.2 to 8.0.100-alpha.1.23063.11
* Microsoft.NETCore.App.Ref: from 8.0.0-alpha.1.22524.5 to 8.0.0-alpha.1.23058.2
* Microsoft.NET.Workload.Emscripten.net7.Manifest-8.0.100: from 8.0.0-alpha.1.22510.1 to 8.0.0-alpha.1.22620.1

~~ Workloads ~~

The .NET SDK has a workaround for `microsoft.net.workload.mono.toolchain`:

dotnet/sdk@f5cb7e3

* HACK: copy folder to `microsoft.net.workload.mono.toolchain.net8` for now until this is resolved.

~~ Android ~~

The biggest changes now in xamarin-android/main are:

* Android enums now have appropriate `[SupportedOSPlatform]` attributes.
  This caused various new warnings in MAUI.

Example:

    src/Essentials/test/DeviceTests/Tests/Vibration_Tests.cs(16,62): error CA1416: This call site is reachable on: 'Android' 21.0 and later. 'BuildVersionCodes.M' is only supported on: 'android' 23.0 and later.
    src/Essentials/test/DeviceTests/Tests/Vibration_Tests.cs(34,62): error CA1416: This call site is reachable on: 'Android' 21.0 and later. 'BuildVersionCodes.M' is only supported on: 'android' 23.0 and later.

* Android now has a new implementation of `Resource.designer.cs`:

dotnet/android@dc3ccf2

We had to make various API changes for a new `Resource` type. Some we we be able to get rid of in the future, after we get:

dotnet/android#7721

* Remove IsMarshmallowOrNewer, IsNougatOrNewer

It appears the analyzer can't tell these values are using `OperatingSystem.IsAndroidVersionAtLeast()`.

It seems we can just use this API directly instead.

* Allow `$(AndroidEnableMarshalMethods)` again

This has the fix:

dotnet/android@22f10b2

Fixes: #11605

~~ Other changes ~~

* [essentials, compatibility] disable trimming for netstandard

* Minor iOS API Changes

* Fix CA2200 in test

Fixes:

    src\Controls\tests\Core.UnitTests\ImageButtonUnitTest.cs(215,6): error CA2200: Re-throwing caught exception changes stack information

Co-authored-by: Jonathan Peppers <[email protected]>
rmarinho pushed a commit that referenced this issue Jan 27, 2023
Changes: dotnet/android@8a20803...c1efcb5
Changes: dotnet/macios@df0151d...6b1b9f3
Changes: dotnet/installer@8c1708f...9962c6a
Changes: dotnet/runtime@5108757...5da4a9e
Changes: dotnet/emsdk@aecb1c7...66b9845

Updates:

* Microsoft.Android.Sdk.Windows: from 34.0.0-preview.1.50 to 34.0.0-preview.1.127
* Microsoft.iOS.Sdk: from 16.1.585-net8-p1 to 16.2.126-net8-p1
* Microsoft.Dotnet.Sdk.Internal: from 8.0.100-alpha.1.22526.2 to 8.0.100-alpha.1.23063.11
* Microsoft.NETCore.App.Ref: from 8.0.0-alpha.1.22524.5 to 8.0.0-alpha.1.23058.2
* Microsoft.NET.Workload.Emscripten.net7.Manifest-8.0.100: from 8.0.0-alpha.1.22510.1 to 8.0.0-alpha.1.22620.1

~~ Workloads ~~

The .NET SDK has a workaround for `microsoft.net.workload.mono.toolchain`:

dotnet/sdk@f5cb7e3

* HACK: copy folder to `microsoft.net.workload.mono.toolchain.net8` for now until this is resolved.

~~ Android ~~

The biggest changes now in xamarin-android/main are:

* Android enums now have appropriate `[SupportedOSPlatform]` attributes.
  This caused various new warnings in MAUI.

Example:

    src/Essentials/test/DeviceTests/Tests/Vibration_Tests.cs(16,62): error CA1416: This call site is reachable on: 'Android' 21.0 and later. 'BuildVersionCodes.M' is only supported on: 'android' 23.0 and later.
    src/Essentials/test/DeviceTests/Tests/Vibration_Tests.cs(34,62): error CA1416: This call site is reachable on: 'Android' 21.0 and later. 'BuildVersionCodes.M' is only supported on: 'android' 23.0 and later.

* Android now has a new implementation of `Resource.designer.cs`:

dotnet/android@dc3ccf2

We had to make various API changes for a new `Resource` type. Some we we be able to get rid of in the future, after we get:

dotnet/android#7721

* Remove IsMarshmallowOrNewer, IsNougatOrNewer

It appears the analyzer can't tell these values are using `OperatingSystem.IsAndroidVersionAtLeast()`.

It seems we can just use this API directly instead.

* Allow `$(AndroidEnableMarshalMethods)` again

This has the fix:

dotnet/android@22f10b2

Fixes: #11605

~~ Other changes ~~

* [essentials, compatibility] disable trimming for netstandard

* Minor iOS API Changes

* Fix CA2200 in test

Fixes:

    src\Controls\tests\Core.UnitTests\ImageButtonUnitTest.cs(215,6): error CA2200: Re-throwing caught exception changes stack information

Co-authored-by: Jonathan Peppers <[email protected]>
@jonathanpeppers
Copy link
Member

This is fixed in #12520, 99898bc.

@ghost ghost locked as resolved and limited conversation to collaborators Mar 15, 2023
@samhouts samhouts added the fixed-in-8.0.0-preview.1.7762 Look for this fix in 8.0.0-preview.1.7762! label May 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
fixed-in-8.0.0-preview.1.7762 Look for this fix in 8.0.0-preview.1.7762! partner/android Issues for the Android SDK platform/android 🤖 t/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants