Skip to content

Conversation

@radekdoulik
Copy link
Member

Context: #5451

apk size comparison, BuildReleaseArm64False test:

> apkdiff -e dll$ before.apk after.apk
Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
  -         123 assemblies/Mono.Android.dll
    -             Type Java.Interop.Tools.JavaCallableWrappers.Crc64
    Type Java.Interop.Tools.TypeNameMappings.JavaNativeTypeManager
      -             Method static string ToHash (string, System.Security.Cryptography.HashAlgorithm)
      +             Method static string ToCrc64 (string)
    +             Type Java.Interop.Tools.JavaCallableWrappers.Crc64Helper
  -       2,975 assemblies/System.Security.Cryptography.Primitives.dll *1
Summary:
  -       3,098 Assemblies -0.41% (of 749,039)

Context: dotnet#5451

apk size comparison, BuildReleaseArm64False test:

    > apkdiff -e dll$ before.apk after.apk
    Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
      -         123 assemblies/Mono.Android.dll
        -             Type Java.Interop.Tools.JavaCallableWrappers.Crc64
        Type Java.Interop.Tools.TypeNameMappings.JavaNativeTypeManager
          -             Method static string ToHash (string, System.Security.Cryptography.HashAlgorithm)
          +             Method static string ToCrc64 (string)
        +             Type Java.Interop.Tools.JavaCallableWrappers.Crc64Helper
      -       2,975 assemblies/System.Security.Cryptography.Primitives.dll *1
    Summary:
      -       3,098 Assemblies -0.41% (of 749,039)
@radekdoulik
Copy link
Member Author

I think we might remove the Java.Interop.Tools.JavaCallableWrappers.Crc64 class from Mono.Android.dll completely. IIRC, it is not considered part of stable API.

@radekdoulik
Copy link
Member Author

This depends on dotnet/java-interop#769

@jonpryor
Copy link
Contributor

jonpryor commented Jan 5, 2021

This PR (#5452) and dotnet/java-interop#769 were co-dependent: the merge of dotnet/java-interop#769 breaks Java.Interop bumps.

Case in point: PR #5253 fails to build:

  /Users/builder/azdo/_work/2/xamarin-android/external/Java.Interop/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/Crc64.cs(58,16): error CS0117: 'Crc64Helper' does not contain a definition for 'HashCore' [/Users/builder/azdo/_work/2/xamarin-android/src/Xamarin.Android.Tools.JavadocImporter/Xamarin.Android.Tools.JavadocImporter.csproj]
  /Users/builder/azdo/_work/2/xamarin-android/external/Java.Interop/src/Java.Interop.Tools.TypeNameMappings/Java.Interop.Tools.TypeNameMappings/JavaNativeTypeManager.cs(648,27): error CS0117: 'Crc64Helper' does not contain a definition for 'Compute' [/Users/builder/azdo/_work/2/xamarin-android/src/Xamarin.Android.Tools.JavadocImporter/Xamarin.Android.Tools.JavadocImporter.csproj]

…because Crc64Helper.cs isn't included in the src/Mono.Android build.

jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Jan 5, 2021
Context: dotnet/java-interop@7d197f1
Context: dotnet#5452

dotnet/java-interop@7d197f17 introduced build breakage:

	…/xamarin-android/external/Java.Interop/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/Crc64.cs(58,16):
	  error CS0117: 'Crc64Helper' does not contain a definition for 'HashCore'
	…/xamarin-android/external/Java.Interop/src/Java.Interop.Tools.TypeNameMappings/Java.Interop.Tools.TypeNameMappings/JavaNativeTypeManager.cs(648,27):
	  error CS0117: 'Crc64Helper' does not contain a definition for 'Compute'

PR dotnet#5452 has the fix.

Include the build fix so that PR dotnet#5253 can build.
@radekdoulik radekdoulik marked this pull request as ready for review January 5, 2021 10:22
@radekdoulik radekdoulik requested a review from grendello January 5, 2021 10:22
@radekdoulik radekdoulik merged commit 5e8a76b into dotnet:master Jan 5, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jan 26, 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.

5 participants