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

[Java.Interop] JniType.RegisterNativeMethods() is public #214

Merged

Conversation

jonpryor
Copy link
Member

Context: dotnet/android#1027
Context: https://jenkins.mono-project.com/job/xamarin-android-pr-builder/2048/

Commit 7d51163 changed the visibility of
JniType.RegisterNativeMethods() from public to internal, in an
effort to reduce the likelihood of "user error" from calling
JNIEnv::RegisterNatives() multiple times, as
JNIEnv::RegisterNatives() invocations are replacements, not
"additive"; calling it more than once is generally Wrong™.

Unfortunately, JniType.RegisterNativeMethods() is part of the
public and shipping API, so we can't remove it, as seen in the
xamarin-android PR build output:

ABI BREAK IN: Java.Interop.dll
<!-- start namespace Java.Interop --> <div>
<h2>Namespace Java.Interop</h2>
<!-- start type JniType --> <div>
<h3>Type Changed: Java.Interop.JniType</h3>
<p>Removed method:</p>

<pre>
        <span class='removed removed-method breaking' data-is-breaking>public void RegisterNativeMethods (JniNativeMethodRegistration[]);</span>
</pre>

Revert the visibility of JniType.RegisterNativeMethods() to
public, so that we don't break API compatibility.

Context: dotnet/android#1027
Context: https://jenkins.mono-project.com/job/xamarin-android-pr-builder/2048/

Commit 7d51163 changed the visibility of
`JniType.RegisterNativeMethods()` from `public` to `internal`, in an
effort to reduce the likelihood of "user error" from calling
`JNIEnv::RegisterNatives()` multiple times, as
`JNIEnv::RegisterNatives()` invocations are *replacements*, not
"additive"; calling it more than once is *generally* Wrong™.

*Unfortunately*, `JniType.RegisterNativeMethods()` is part of the
public and shipping API, so we can't remove it, as seen in the
[xamarin-android PR build][pr2048] output:

[pr2048]: https://jenkins.mono-project.com/job/xamarin-android-pr-builder/2048/consoleText

	ABI BREAK IN: Java.Interop.dll
	<!-- start namespace Java.Interop --> <div>
	<h2>Namespace Java.Interop</h2>
	<!-- start type JniType --> <div>
	<h3>Type Changed: Java.Interop.JniType</h3>
	<p>Removed method:</p>

	<pre>
	        <span class='removed removed-method breaking' data-is-breaking>public void RegisterNativeMethods (JniNativeMethodRegistration[]);</span>
	</pre>

Revert the visibility of `JniType.RegisterNativeMethods()` to
`public`, so that we don't break API compatibility.
@jonpryor jonpryor merged commit 1bcfcc5 into dotnet:master Nov 15, 2017
@github-actions github-actions bot locked and limited conversation to collaborators Apr 15, 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