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

[manifest-attribute-codegen] Automatically generate manifest attributes from Android SDK data. #8781

Merged
merged 4 commits into from
Jun 7, 2024

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Mar 1, 2024

Fixes #8272
Context: #8729

Today, we do not have an established process for detecting new XML elements and attributes allowed in AndroidManifest.xml and surfacing them to users via our manifest attributes like [ActivityAttribute]. This leads to users having to use manual workarounds until our attributes can be updated in a future .NET version.

Additionally, when we do add new properties to these attributes, it requires manually updating multiple files by hand that must remain in sync, eg:

‎src/Mono.Android/Android.App/IntentFilterAttribute.cs
src/Xamarin.Android.Build.Tasks/Mono.Android/IntentFilterAttribute.Partial.cs

This commit extends our existing manifest-attribute-codegen script that can parse Android SDKs to determine the possible allowed values in AndroidManifest.xml to additionally generate the code needed by both Mono.Android and Xamarin.Android.Build.Tasks so that we can make updates easily and correctly.

This process is driven by metadata.xml (unrelated to Java binding "metadata") which allows process customization and enforces that every available AndroidManifest.xml value has been accounted for. This ensures that we are making explicit decisions for every new value.

This process is documented in build-tools/manifest-attribute-codegen/README.md.

Also removed the ANDROID_* $(DefineConstants) for Xamarin.Android.Build.Tasks as we no longer build separate assemblies for old Android API levels.

Note this commit does not change any existing manifest attributes or the properties they expose. It merely generates what we expose today. We will determine additional properties to expose in a future commit.

@jpobst jpobst marked this pull request as ready for review March 25, 2024 18:21

Required metadata:

- **name** - The name of the new element in AndroidManifest.xml, like `activity` or `application`
Copy link
Member

Choose a reason for hiding this comment

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

…like here: where do these name values come from?


w.Write ($" <a name='{Name}'{format}{api_level}");

if (Enums.Count > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

This bit feels like "dead code" as nothing appears to hit this code path; git grep '<enum-definition name' has no matches. Perhaps this is premature?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was existing code that I didn't change:

https://github.com/xamarin/xamarin-android/blob/b521487d0226db265baaa83f0f53e2e75497c420/build-tools/manifest-attribute-codegen/manifest-attribute-codegen.cs#L165-L177

I'm not sure what the original intent was, but maybe the SDK used to specify valid values for some attributes?

public List<ElementDefinition> Elements { get; } = new List<ElementDefinition> ();

// Creates a new ManifestDefinition for a single Android API from the given file path
public static ManifestDefinition FromFile (string filePath)
Copy link
Member

Choose a reason for hiding this comment

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

May want to mention that filePath is expected to be similar to $HOME/android-toolchain/sdk/platforms/android-34/data/res/values/attrs_manifest.xml, and thus dir_name would be android-34, and elements + //declare-styleable is coming from attrs_manifest.xml.


public string? WritePermission { get; set; }

#if XABT_MANIFEST_EXTENSIONS
Copy link
Member

Choose a reason for hiding this comment

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

Why have this #If block in each file instead of having this within e.g. the src/Xamarin.Android.Build.Tasks/Mono.Android/*.Partial.cs files?

Review and update `README.md` so that @jonpryor understands it.
In particular, `xaprepare android-sdk-platforms=all` Is Not A Thing™
and needed to be replaced with a command that actually worked.

Update `manifest-attribute-codegen` to:

  * Normalize the output path so that it works properly on macOS/Linux
  * Has a comment after `#endif // XABT_MANIFEST_EXTENSIONS`, because
    @jonpryor thinks code is clearer that way.
    (This is also how I found out that paths needed to be normalized!)
  * Remove extraneous whitespace from `<AppendTargetFrameworkToOutputPath/>`.
  * Update the `GenerateManifestAttributes` target so that `-output-manifest`
    is used to update `manifest-definition.xml`.

The change of the `#endif` required updating lots of source code.
@jonpryor
Copy link
Member

jonpryor commented Jun 7, 2024

[manifest-attribute-codegen] Generate custom attribute declarations (#8781)

Fixes: https://github.com/xamarin/xamarin-android/issues/8272

Context: https://github.com/xamarin/xamarin-android/issues/8235
Context: https://github.com/xamarin/xamarin-android/issues/8729
Context: e790874a21ae0846531b473b003dbc598f0f0310

Previously, we did not have an established process for detecting new
XML elements and attributes allowed in `AndroidManifest.xml` and
surfacing them to users via our manifest attributes like
`[ActivityAttribute]`.  This leads to users having to use manual
workarounds until our attributes can be updated.

Additionally, whenever we do add new properties to these attributes,
it requires manually updating multiple files by hand that must remain
in sync, eg:

  * [src/Mono.Android/Android.App/IntentFilterAttribute.cs](https://github.com/xamarin/xamarin-android/blob/180dd5205ab270bb74bb853754665db9cb5d65f1/src/Mono.Android/Android.App/IntentFilterAttribute.cs#L9)
  * [src/Xamarin.Android.Build.Tasks/Mono.Android/IntentFilterAttribute.Partial.cs](https://github.com/xamarin/xamarin-android/blob/180dd5205ab270bb74bb853754665db9cb5d65f1/src/Xamarin.Android.Build.Tasks/Mono.Android/IntentFilterAttribute.Partial.cs#L14)

The `build-tools/manifest-attribute-codegen` utility (e790874a) has
support to parse Android SDK `attrs_manifest.xml` files, which
specifies what elements and attributes are valid within
`AndroidManifest.xml`.

Update `manifest-attribute-codegen` to do what it's name already
implied: generate code!  It now reads a `metadata.xml` file which
controls which custom attributes to emit, where to emit them, and
what members those custom attributes should have (among other things).
This makes it easier to ensure that code shared by `src/Mono.Android`
and `src/Xamarin.Android.Build.Tasks` are consistent, meaking it
easier to correctly add support for new attributes and/or
attribute members.

Generated file semantics and naming conventions: consider the C# type
`Android.App.ActivityAttribute`.

  * `src\Xamarin.Android.NamingCustomAttributes\Android.App\ActivityAttribute.cs`
    contains the C# `partial` class declaration that can be shared
    by both `src\Mono.Android` and `src\Xamarin.Android.Build.Tasks`.
    This file also contains a `#if XABT_MANIFEST_EXTENSIONS` block
    which is only used by `src\Xamarin.Android.Build.Tasks`.

  * `src/Xamarin.Android.Build.Tasks/Mono.Android/ActivityAttribute.Partial.cs`
    contains the C# `partial` class declaration with code specific
    to `Xamarin.Android.Build.Tasks.dll`.

  * `src/Xamarin.Android.NamingCustomAttributes/Android.App/ActivityAttribute.Partial.cs`
    contains the C# `partial` class declaration with code specific
    to `Mono.Android.dll`.

`metadata.xml` contents and the update process is documented in
`build-tools/manifest-attribute-codegen/README.md`.

Also removed the `ANDROID_*` values from `$(DefineConstants)` for
`Xamarin.Android.Build.Tasks.csproj` as we no longer build separate
assemblies for old Android API levels.

Note this commit does not change any existing manifest attributes or
the properties they expose.  It merely generates what we expose today.
We will determine additional properties to expose in a future commit.

@jonpryor jonpryor merged commit 3ab74db into main Jun 7, 2024
48 checks passed
@jonpryor jonpryor deleted the manifest-maker branch June 7, 2024 17:57
grendello added a commit that referenced this pull request Jun 17, 2024
* main: (22 commits)
  Bump to dotnet/android-tools@1c09dcc (#9026)
  Bump to dotnet/java-interop@ccafbe6 (#9025)
  [Mono.Android-Tests] Fix repo URL in redirect tests (#9035)
  [ci] Update checkout path for nightly build (#9028)
  [ci] Fix android source path for MAUI test job (#9030)
  Link Code of Conduct (#9034)
  [ci] Update sdk-insertions trigger to manual only (#9029)
  Update java-interop and android-tools submodule mentions (#9023)
  LEGO: Merge pull request 9022
  [Xamarin.Android.Build.Tasks] fastdev works with aab files (#8990)
  Use new binutils URL (#9019)
  Localized file check-in by OneLocBuild Task: Build definition ID 17928: Build ID 9686669 (#9011)
  LEGO: Merge pull request 9015
  [api-merge] Update "constant" values to mirror latest API levels (#9004)
  [Mono.Android] Fix wrong value for `ApplicationExitInfoReason.Other` (#9003)
  [Mono.Android] Fix omitted Gl* constants. (#9009)
  [manifest-attribute-codegen] Generate custom attribute declarations (#8781)
  [tests] Reduce default build output verbosity (#9002)
  [templates] Update Wear OS en template string (#9005)
  [build] Do not provision JDK 8 (#8999)
  ...
@github-actions github-actions bot locked and limited conversation to collaborators Jul 8, 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.

Create process to automatically generate AndroidManifest.xml attributes
2 participants