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

Use <AndroidNamespaceReplacement> instead of metadata to rename namespaces. #555

Merged
merged 1 commit into from
May 26, 2022

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented May 17, 2022

Context: dotnet/android#6643
Context: dotnet/java-interop#727

Use <AndroidNamespaceReplacement> instead of Metadata to specify namespaces. This allows us to significantly cut down the amount of work needed to ensure proper namespaces.

image

For example, all of the following lines are needed simply to correctly capitalize AndroidX:

<attr path="/api/package[@name='androidx.core.app']" name="managedName">AndroidX.Core.App</attr>
<attr path="/api/package[@name='androidx.core.content']" name="managedName">AndroidX.Core.Content</attr>
<attr path="/api/package[@name='androidx.core.database']" name="managedName">AndroidX.Core.Database</attr>
<attr path="/api/package[@name='androidx.core.graphics']" name="managedName">AndroidX.Core.Graphics</attr>
<attr path="/api/package[@name='androidx.core.graphics.drawable']" name="managedName">AndroidX.Core.Graphics.Drawable</attr>
<attr path="/api/package[@name='androidx.core.hardware.display']" name="managedName">AndroidX.Core.Hardware.Display</attr>
<attr path="/api/package[@name='androidx.core.hardware.fingerprint']" name="managedName">AndroidX.Core.Hardware.Fingerprint</attr>
<attr path="/api/package[@name='androidx.core.internal']" name="managedName">AndroidX.Core.Internal</attr>
<attr path="/api/package[@name='androidx.core.internal.view']" name="managedName">AndroidX.Core.Internal.View</attr>

This can be replaced with:

<AndroidNamespaceReplacement Include='Androidx' Replacement='AndroidX' />

Additionally add "standard" replacements for combined words that require special capitalization:

<AndroidNamespaceReplacement Include='accessibilityservice' Replacement='AccessibilityService' />

This ensures that these words are always capitalized consistently across all the packages in this repo.

Going forward, we will rarely need to manually handle new namespaces. We should only need to worry if a new combined-word namespace is added.

(This is why #558 is important, since we will not need to actively adjust every new namespace.)

@jpobst jpobst marked this pull request as ready for review May 24, 2022 18:31
@jpobst jpobst requested a review from moljac May 24, 2022 18:31
@moljac
Copy link
Member

moljac commented May 25, 2022

Going forward, we will rarely need to manually handle new namespaces. We should only need to worry if a new combined-word namespace is added.

How are we going to detect those rare cases? I mean, exceptions in the rules are usually the most difficult to detect and fix.

My presumption was:

if Androidx.* was detected => new namespace appeared => check the whole namespace for the correctness.

I don't remember case when it failed.

The reason there is Metadata.Namespaces.xml was simply attempt to separate metadata generation for bindings and it was the result of collecting packagenames with MSBuild Custom Task Nuget that could be added (if desired) to bindings project:

My attempt to learn MSBuild when I had more time:

https://github.com/HolisticWare-Xamarin-Tools/HolisticWare.Xamarin.Tools.Bindings.XamarinAndroid.MetadataXmlSpitter/blob/master/source/HolisticWare.Xamarin.Tools.Bindings.XamarinAndroid.ApiXmlSpitter/PackagesToNamespacesSpitter.cs

@jpobst
Copy link
Contributor Author

jpobst commented May 25, 2022

How are we going to detect those rare cases? I mean, exceptions in the rules are usually the most difficult to detect and fix.

Every new namespace will have to be added to the published-namespaces.txt file in order to pass CI. It is now no longer possible for a new namespace to get added (or removed) to a package without us knowing.

It should be much easier to review changes to that file in a PR diff rather than having to examine every api-diff for new namespaces.

Detecting these rare cases if we generally do not have to make adjustments for namespaces was absolutely a concern. It is the reason published-namespaces.txt was added. 😁

@jpobst jpobst merged commit 0aaa542 into main May 26, 2022
@jpobst jpobst deleted the ns-replace branch May 26, 2022 13:28
@jpobst jpobst mentioned this pull request May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants