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

[api-merge] Update "constant" values if they change between API levels. #9004

Merged
merged 2 commits into from
Jun 7, 2024

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Jun 5, 2024

Context: #9000

In #9000, we learned that:

  • Constant values can change between Android API levels
  • api-merge does not update constant values in the final api.xml if they do change

Update api-merge to update constant values if they change, and update api-VanillaIceCream.xml with the new api-merge logic.

@jpobst
Copy link
Contributor Author

jpobst commented Jun 5, 2024

With this updated api-VanillaIceCream.xml, the only reported constant mismatch is the one reported in #9000, which was caused by an error in our enumification process:

Mismatch: android/app/ApplicationExitInfo.REASON_OTHER 10 != 13 [API-30]

@jpobst jpobst force-pushed the api-merge-update-constants branch from 89ecb38 to 96e1e86 Compare June 5, 2024 14:59
@jpobst jpobst marked this pull request as ready for review June 5, 2024 16:50
@jpobst jpobst requested a review from jonpryor as a code owner June 5, 2024 16:50
Spaces, not tabs.
@jonpryor
Copy link
Member

jonpryor commented Jun 7, 2024

Draft commit message:

Context: https://github.com/xamarin/xamarin-android/issues/9000
Context: ac3b405759dfcb3f24a551e8f408ed551b78612f

In Issue #9000, we learned that:

  - Constant values can change between Android API levels
  - `api-merge` does not update constant values in the final `api.xml`
    if they do change

Update `api-merge` to update constant values if they change, and update
`api-VanillaIceCream.xml` with the new `api-merge` logic.

@jonpryor
Copy link
Member

jonpryor commented Jun 7, 2024

@jpobst wrote:

With this updated api-VanillaIceCream.xml, the only reported constant mismatch is the one reported in #9000, which was caused by an error in our enumification process:

Mismatch: android/app/ApplicationExitInfo.REASON_OTHER 10 != 13 [API-30]

Where does this "Mismatch" message come from? I don't see anything in this PR which would emit such a message, and git grep 'Mismatch: ' doesn't find anything either.

@jpobst
Copy link
Contributor Author

jpobst commented Jun 7, 2024

It is a quick local script I wrote that compares map.csv to api.xml.

We can leave the bones of it here for future reference:

class MismatchedEnumValues
{
	static string enum_csv = @"C:\code\xamarin-android\src\Mono.Android\map.csv";
	static string api_xml = @"C:\code\xamarin-android-backport\src\Mono.Android\Profiles\api-VanillaIceCream.xml";

	public static void Do ()
	{
		var consts = ConstantsParser.FromEnumMapCsv (enum_csv).Where (c => c.Action.In (ConstantAction.Enumify)).ToList ();

		var xml = new XmlDocument ();
		xml.Load (api_xml);

		foreach (var c in consts) {
			var package = c.JavaPackage.StartsWith ("I:") ? c.JavaPackage.Substring (2) : c.JavaPackage;
			var elem = xml.SelectSingleNode ($"/api/package[@name='{package.Replace ('/', '.')}']/*[@name='{c.JavaType.Replace ('$', '.')}']/field[@name='{c.JavaName}']");

			if (elem is null) {
				Console.WriteLine ($"Orphan: Could not find {c.JavaPackage}.{c.JavaType}.{c.JavaName} [API-{c.ApiLevel}]");
				continue;
			}

			var value = elem.Attributes? ["value"]?.Value ?? "";

			if (value != c.Value)
				Console.WriteLine ($"Mismatch: {c.JavaPackage}/{c.JavaType}.{c.JavaName} {c.Value} != {value} [API-{c.ApiLevel}]");
		}
	}
}

(Requires a reference to Java.Interop.Tools.Generator project.)

@jonpryor jonpryor merged commit eb7fdf7 into main Jun 7, 2024
21 of 48 checks passed
@jonpryor jonpryor deleted the api-merge-update-constants branch June 7, 2024 18:42
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)
  ...
grendello added a commit that referenced this pull request Jun 21, 2024
* main: (26 commits)
  Make APK and shared library alignment configurable (#9046)
  [r8] update proguard rule to keep .NET runtime classes (#9044)
  Explicitly align to 4k (#9041)
  [trimming] preserve custom views and `$(AndroidHttpClientHandlerType)` (#8954)
  Ignore split configs when bundle config moves shared libraries to base.apk (#8987)
  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)
  ...
@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.

2 participants