Skip to content

[XABT] Refactor manifest merging and ACW map generation out of GenerateJavaStubs. #9827

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

Merged
merged 2 commits into from
Feb 26, 2025

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Feb 21, 2025

Begin breaking down the <GenerateJavaStubs> task into smaller, more manageable pieces by moving the manifest merging and ACW map generation into separate tasks. Once this is complete, we should be able to start moving these steps that require Cecil assembly scanning into linker steps (or equivalent), eventually saving a Cecil assembly scan per build.

Additionally, create a new GetProjectBuildSpecificTaskObjectKey to be used with GetRegisteredTaskObject. This version additional uses $(IntermediateOutpuPath) in the key to help ensure data from multi-targeted builds (net8.0-android,net9.0-android) is not overwritten.

@dotnet dotnet deleted a comment from azure-pipelines bot Feb 21, 2025
@jpobst jpobst marked this pull request as ready for review February 24, 2025 18:18
@jpobst jpobst requested a review from jonpryor February 24, 2025 18:18

namespace Xamarin.Android.Tasks;

public class GenerateMergedAndroidManifest : AndroidTask
Copy link
Contributor

Choose a reason for hiding this comment

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

Some of this can be trimmed down since we use the android manifestmerger these days.

AndroidManifestMerger` == `legacy is what this is keyed off.
But I guess this will be a staged approach.
Maybe we can rename this task a bit? Because unless we use legacy we use another tool to generate the final manifest. Perhaps g ?

Copy link
Contributor Author

@jpobst jpobst Feb 24, 2025

Choose a reason for hiding this comment

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

I don't see anything that prevents this logic from running if $(AndroidManifestMerger) != 'legacy'. If that is the intention, then that is definitely something we should look into in the future.

I can rename the task GenerateLegacyMergedAndroidManifest to help make the purpose clearer?

Copy link
Contributor

Choose a reason for hiding this comment

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

Well it is partially used in the new system. The main difference is the _MergedManifestDocuments ItemGroup is not populated in the new system. So that will be empty.

Copy link
Contributor

Choose a reason for hiding this comment

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

See

<_MergedManifestDocuments Condition=" '$(AndroidManifestMerger)' == 'legacy' " Include="@(ExtractedManifestDocuments)" />

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, then calling it GenerateLegacyMergedAndroidManifest is probably not a good name. Do you have a suggestion?

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe GenerateIntermediateAndroidManifest ?

Copy link
Contributor

Choose a reason for hiding this comment

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

or GenerateMainAndroidManifest since its the "root" manifest when we finally do the merging later.

@jpobst jpobst merged commit cf62814 into main Feb 26, 2025
58 checks passed
@jpobst jpobst deleted the dev/jpobst/separate-merger branch February 26, 2025 17:58
jonpryor pushed a commit that referenced this pull request Mar 3, 2025
…#9850)

Context: #9827

Continue breaking down the `<GenerateJavaStubs>` task into smaller,
more manageable pieces by moving the marshal methods and type map
generation steps into separate tasks.

TODO: start moving steps that require Cecil assembly scanning into
linker steps (or equivalent), eventually saving a Cecil assembly scan
per build.
@github-actions github-actions bot locked and limited conversation to collaborators Mar 29, 2025
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.

3 participants