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

[Xamarin.Android.Build.Tasks] use ProjectSpecificTaskObjectKey in <PrepareDSOWrapperState/> #9340

Merged
merged 1 commit into from
Sep 30, 2024

Conversation

jonathanpeppers
Copy link
Member

Context: dotnet/maui#24539 (comment)

In .NET MAUI's build, they failed to bump .NET for Android because of the following error:

dotnet\packs\Microsoft.Android.Sdk.Windows\35.0.0-rc.2.130\tools\Xamarin.Android.Common.Debugging.targets(139,2): error XABLD7009: System.InvalidOperationException: Internal error: archive DSO stub location not known for architecture 'X86'
at Xamarin.Android.Tasks.DSOWrapperGenerator.WrapIt(AndroidTargetArch targetArch, String payloadFilePath, String outputFileName, IBuildEngine4 buildEngine, TaskLoggingHelper log) in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Utilities/DSOWrapperGenerator.cs:line 86
at Xamarin.Android.Tasks.BuildApk.AddRuntimeConfigBlob(ZipArchiveEx apk) in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs:line 404
at Xamarin.Android.Tasks.BuildApk.ExecuteWithAbi(String[] supportedAbis, String apkInputPath, String apkOutputPath, Boolean debug, Boolean compress, IDictionary`2 compressedAssembliesInfo, String assemblyStoreApkName) in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs:line 215
at Xamarin.Android.Tasks.BuildApk.RunTask() in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs:line 357
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 25

In 5ebcb1d, we introduced some MSBuild RegisterTaskObject() usage that failed in specific way:

  • MAUI has a .sln with multiple "app" projects: device tests, samples, etc. building in parallel

  • The <PrepareDSOWrapperState/> MSBuild task runs for project A, saving x64 and arm64 values.

  • Project B goes to run <BuildApk/> but x86 is missing.

For now, we can fix this by using ProjectSpecificTaskObjectKey() that wraps the key with a Tuple such as:

(key, WorkingDirectory)

Which, should result in a unique key per project.

In a future PR, we could consider removing this RegisterTaskObject() usage completely, and doing all the work inside the <BuildApk/> MSBuild task instead.

…<PrepareDSOWrapperState/>`

Context: dotnet/maui#24539 (comment)

In .NET MAUI's build, they failed to bump .NET for Android because of
the following error:

    dotnet\packs\Microsoft.Android.Sdk.Windows\35.0.0-rc.2.130\tools\Xamarin.Android.Common.Debugging.targets(139,2): error XABLD7009: System.InvalidOperationException: Internal error: archive DSO stub location not known for architecture 'X86'
    at Xamarin.Android.Tasks.DSOWrapperGenerator.WrapIt(AndroidTargetArch targetArch, String payloadFilePath, String outputFileName, IBuildEngine4 buildEngine, TaskLoggingHelper log) in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Utilities/DSOWrapperGenerator.cs:line 86
    at Xamarin.Android.Tasks.BuildApk.AddRuntimeConfigBlob(ZipArchiveEx apk) in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs:line 404
    at Xamarin.Android.Tasks.BuildApk.ExecuteWithAbi(String[] supportedAbis, String apkInputPath, String apkOutputPath, Boolean debug, Boolean compress, IDictionary`2 compressedAssembliesInfo, String assemblyStoreApkName) in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs:line 215
    at Xamarin.Android.Tasks.BuildApk.RunTask() in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs:line 357
    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 25

In 5ebcb1d, we introduced some MSBuild `RegisterTaskObject()` usage
that failed in specific way:

* MAUI has a `.sln` with multiple "app" projects: device tests,
  samples, etc. building in parallel

* The `<PrepareDSOWrapperState/>` MSBuild task runs for project A,
  saving x64 and arm64 values.

* Project B goes to run `<BuildApk/>` but x86 is missing.

For now, we can fix this by using `ProjectSpecificTaskObjectKey()` that
wraps the key with a `Tuple` such as:

    (key, WorkingDirectory)

Which, should result in a unique key per project.

In a future PR, we could consider removing this `RegisterTaskObject()`
usage completely, and doing all the work inside the `<BuildApk/>`
MSBuild task instead.
Comment on lines +379 to +381
internal DSOWrapperGenerator.Config EnsureConfig ()
{
var config = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal<DSOWrapperGenerator.Config> (ProjectSpecificTaskObjectKey (DSOWrapperGenerator.RegisteredConfigKey), RegisteredTaskObjectLifetime.Build);
Copy link
Member Author

Choose a reason for hiding this comment

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

@jonpryor
Copy link
Member

@jonathanpeppers, @dellis1972: the need to use ProjectSpecificTaskObjectKey() suggests that we should re-audit Xamarin.Android.Build.Tasks use of "RegisterTaskObject" to ensure we're doing the right thing.

At a glance, I'm not sure we are?

% git grep RegisterTaskO src/Xamarin.Android.Build.Tasks
…
src/Xamarin.Android.Build.Tasks/Tasks/ResolveAndroidTooling.cs:                         BuildEngine4.RegisterTaskObject (key, Aapt2Version, RegisteredTaskObjectLifetime.AppDomain, allowEarlyCollection: false);
src/Xamarin.Android.Build.Tasks/Tasks/ResolveJdkJvmPath.cs:                     BuildEngine4.RegisterTaskObject (key, path, RegisteredTaskObjectLifetime.AppDomain, allowEarlyCollection: false);
src/Xamarin.Android.Build.Tasks/Tasks/ValidateJavaVersion.cs:                           BuildEngine4.RegisterTaskObject (key, versionNumber, RegisteredTaskObjectLifetime.AppDomain, allowEarlyCollection: false);

Shouldn't all of these be instead using RegisterTaskObjectAssemblyLocal()?

@dellis1972
Copy link
Contributor

@jonathanpeppers, @dellis1972: the need to use ProjectSpecificTaskObjectKey() suggests that we should re-audit Xamarin.Android.Build.Tasks use of "RegisterTaskObject" to ensure we're doing the right thing.

At a glance, I'm not sure we are?

% git grep RegisterTaskO src/Xamarin.Android.Build.Tasks
…
src/Xamarin.Android.Build.Tasks/Tasks/ResolveAndroidTooling.cs:                         BuildEngine4.RegisterTaskObject (key, Aapt2Version, RegisteredTaskObjectLifetime.AppDomain, allowEarlyCollection: false);
src/Xamarin.Android.Build.Tasks/Tasks/ResolveJdkJvmPath.cs:                     BuildEngine4.RegisterTaskObject (key, path, RegisteredTaskObjectLifetime.AppDomain, allowEarlyCollection: false);
src/Xamarin.Android.Build.Tasks/Tasks/ValidateJavaVersion.cs:                           BuildEngine4.RegisterTaskObject (key, versionNumber, RegisteredTaskObjectLifetime.AppDomain, allowEarlyCollection: false);

Shouldn't all of these be instead using RegisterTaskObjectAssemblyLocal()?

Those are related to tooling, so they would be better off being global rather than project specific. That way we only resolve the paths once , not per project

@jonathanpeppers
Copy link
Member Author

Shouldn't all of these be instead using RegisterTaskObjectAssemblyLocal()?

For these, the key is a path and the value is a string. This allows a .NET 8 class library and a .NET 9 app in the same solution to share these values. We could use RegisterTaskObjectAssemblyLocal(), but I have a memory of testing these and it could even share between .NET 6+ and Xamarin.Android successfully.

@jonathanpeppers jonathanpeppers merged commit 230ebb0 into main Sep 30, 2024
56 of 58 checks passed
@jonathanpeppers jonathanpeppers deleted the ProjectSpecificTaskObjectKey branch September 30, 2024 15:39
jonathanpeppers added a commit that referenced this pull request Sep 30, 2024
…<PrepareDSOWrapperState/>` (#9340)

Context: dotnet/maui#24539 (comment)

In .NET MAUI's build, they failed to bump .NET for Android because of
the following error:

    dotnet\packs\Microsoft.Android.Sdk.Windows\35.0.0-rc.2.130\tools\Xamarin.Android.Common.Debugging.targets(139,2): error XABLD7009: System.InvalidOperationException: Internal error: archive DSO stub location not known for architecture 'X86'
    at Xamarin.Android.Tasks.DSOWrapperGenerator.WrapIt(AndroidTargetArch targetArch, String payloadFilePath, String outputFileName, IBuildEngine4 buildEngine, TaskLoggingHelper log) in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Utilities/DSOWrapperGenerator.cs:line 86
    at Xamarin.Android.Tasks.BuildApk.AddRuntimeConfigBlob(ZipArchiveEx apk) in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs:line 404
    at Xamarin.Android.Tasks.BuildApk.ExecuteWithAbi(String[] supportedAbis, String apkInputPath, String apkOutputPath, Boolean debug, Boolean compress, IDictionary`2 compressedAssembliesInfo, String assemblyStoreApkName) in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs:line 215
    at Xamarin.Android.Tasks.BuildApk.RunTask() in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs:line 357
    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 25

In 5ebcb1d, we introduced some MSBuild `RegisterTaskObject()` usage
that failed in specific way:

* MAUI has a `.sln` with multiple "app" projects: device tests,
  samples, etc. building in parallel

* The `<PrepareDSOWrapperState/>` MSBuild task runs for project A,
  saving x64 and arm64 values.

* Project B goes to run `<BuildApk/>` but x86 is missing.

For now, we can fix this by using `ProjectSpecificTaskObjectKey()` that
wraps the key with a `Tuple` such as:

    (key, WorkingDirectory)

Which, should result in a unique key per project.

In a future PR, we could consider removing this `RegisterTaskObject()`
usage completely, and doing all the work inside the `<BuildApk/>`
MSBuild task instead.
jonathanpeppers added a commit that referenced this pull request Sep 30, 2024
…<PrepareDSOWrapperState/>` (#9340)

Context: dotnet/maui#24539 (comment)

In .NET MAUI's build, they failed to bump .NET for Android because of
the following error:

    dotnet\packs\Microsoft.Android.Sdk.Windows\35.0.0-rc.2.130\tools\Xamarin.Android.Common.Debugging.targets(139,2): error XABLD7009: System.InvalidOperationException: Internal error: archive DSO stub location not known for architecture 'X86'
    at Xamarin.Android.Tasks.DSOWrapperGenerator.WrapIt(AndroidTargetArch targetArch, String payloadFilePath, String outputFileName, IBuildEngine4 buildEngine, TaskLoggingHelper log) in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Utilities/DSOWrapperGenerator.cs:line 86
    at Xamarin.Android.Tasks.BuildApk.AddRuntimeConfigBlob(ZipArchiveEx apk) in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs:line 404
    at Xamarin.Android.Tasks.BuildApk.ExecuteWithAbi(String[] supportedAbis, String apkInputPath, String apkOutputPath, Boolean debug, Boolean compress, IDictionary`2 compressedAssembliesInfo, String assemblyStoreApkName) in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs:line 215
    at Xamarin.Android.Tasks.BuildApk.RunTask() in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs:line 357
    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 25

In 5ebcb1d, we introduced some MSBuild `RegisterTaskObject()` usage
that failed in specific way:

* MAUI has a `.sln` with multiple "app" projects: device tests,
  samples, etc. building in parallel

* The `<PrepareDSOWrapperState/>` MSBuild task runs for project A,
  saving x64 and arm64 values.

* Project B goes to run `<BuildApk/>` but x86 is missing.

For now, we can fix this by using `ProjectSpecificTaskObjectKey()` that
wraps the key with a `Tuple` such as:

    (key, WorkingDirectory)

Which, should result in a unique key per project.

In a future PR, we could consider removing this `RegisterTaskObject()`
usage completely, and doing all the work inside the `<BuildApk/>`
MSBuild task instead.
jonathanpeppers added a commit that referenced this pull request Sep 30, 2024
…<PrepareDSOWrapperState/>` (#9340)

Context: dotnet/maui#24539 (comment)

In .NET MAUI's build, they failed to bump .NET for Android because of
the following error:

    dotnet\packs\Microsoft.Android.Sdk.Windows\35.0.0-rc.2.130\tools\Xamarin.Android.Common.Debugging.targets(139,2): error XABLD7009: System.InvalidOperationException: Internal error: archive DSO stub location not known for architecture 'X86'
    at Xamarin.Android.Tasks.DSOWrapperGenerator.WrapIt(AndroidTargetArch targetArch, String payloadFilePath, String outputFileName, IBuildEngine4 buildEngine, TaskLoggingHelper log) in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Utilities/DSOWrapperGenerator.cs:line 86
    at Xamarin.Android.Tasks.BuildApk.AddRuntimeConfigBlob(ZipArchiveEx apk) in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs:line 404
    at Xamarin.Android.Tasks.BuildApk.ExecuteWithAbi(String[] supportedAbis, String apkInputPath, String apkOutputPath, Boolean debug, Boolean compress, IDictionary`2 compressedAssembliesInfo, String assemblyStoreApkName) in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs:line 215
    at Xamarin.Android.Tasks.BuildApk.RunTask() in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs:line 357
    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 25

In 5ebcb1d, we introduced some MSBuild `RegisterTaskObject()` usage
that failed in specific way:

* MAUI has a `.sln` with multiple "app" projects: device tests,
  samples, etc. building in parallel

* The `<PrepareDSOWrapperState/>` MSBuild task runs for project A,
  saving x64 and arm64 values.

* Project B goes to run `<BuildApk/>` but x86 is missing.

For now, we can fix this by using `ProjectSpecificTaskObjectKey()` that
wraps the key with a `Tuple` such as:

    (key, WorkingDirectory)

Which, should result in a unique key per project.

In a future PR, we could consider removing this `RegisterTaskObject()`
usage completely, and doing all the work inside the `<BuildApk/>`
MSBuild task instead.
@github-actions github-actions bot locked and limited conversation to collaborators Oct 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants