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 Callable Wrappers should be emitted & built as part of Build #4278

Open
jonpryor opened this issue Feb 18, 2020 · 1 comment
Open

Java Callable Wrappers should be emitted & built as part of Build #4278

jonpryor opened this issue Feb 18, 2020 · 1 comment
Assignees
Labels
Area: Bindings Issues in Java Library Binding projects. enhancement Proposed change to current functionality.

Comments

@jonpryor
Copy link
Member

Context: dotnet/android-libraries#56

Currently, binding projects only emit binding assemblies. They don't do any "sanity checking" to ensure that the binding assembly is usable.

Enter dotnet/android-libraries#56: the Xamarin.Google.Android.Material 1.1.0-rc1 NuGet package contains a bug, as it binds the com.google.android.material.button.MaterialButton.OnPressedChangeListener interface as Google.Android.Material.Button.MaterialButton/IOnPressedChangeListener. However, MaterialButton.OnPressedChangeListener is not public; it's (presumably?) package-private, and as such it cannot be used from outside of its containing package.

The result is that if an app tries to use the Xamarin.Google.Android.Material NuGet package, it will fail to build:

javac error JAVAC0000:  error: OnPressedChangeListener is not public in MaterialButton; cannot be accessed from outside package

This is in part a generator bug: dotnet/java-interop#572

That said, there is also a "defense in depth" argument to be made here: the binding built, but it should never have built in the first place!

To ensure that binding projects are actually usable, the Build target for binding projects should also:

  1. Generate the Java Callable Wrappers for the binding assembly, and
  2. Build the Java Callable Wrappers.

This would avoid the need to create a new App project which references the Binding project to ensure that it works, which may result in "false positives" (e.g. if the App test project is built in Release config, the types which emit the "invalid" Java Callable Wrappers may never be emitted, resulting in a "good App" but a "bad binding assembly").

Steps to Reproduce

Expected Behavior

Actual Behavior

Version Information

Log File

@jonpryor jonpryor added the Area: Bindings Issues in Java Library Binding projects. label Feb 18, 2020
@gugavaro gugavaro added this to the Under Consideration milestone Feb 19, 2020
@jpobst jpobst removed their assignment Apr 1, 2020
@jpobst jpobst modified the milestones: Under Consideration, .NET 8 Sep 13, 2022
@jpobst jpobst added the enhancement Proposed change to current functionality. label Sep 13, 2022
@jpobst jpobst self-assigned this Sep 13, 2022
@jpobst jpobst modified the milestones: .NET 8, .NET 9 Planning May 1, 2023
@jpobst
Copy link
Contributor

jpobst commented Dec 5, 2023

After initial exploration, the key hurdle for this proposal is that it requires a binding project to have access to all dependency Java libraries in order to run javac on the generated Java Callable Wrappers.

Today, the binding process only consumes managed (C#) bindings for referenced dependencies. There is no requirement that the Java libraries be available to the binding project build.

An alternative user flow might be:

  • Create an Android Application
  • Add any binding(s) to verify JCW for
  • Set a property like $(AndroidVerifyBindingsCallableWrappers)
  • This will generate a full set of callable wrappers and compile them using the Java libs collected via a normal Android Application build

Granted, this is definitely a less than ideal user flow, but one that could be implemented with what we have today.

jonpryor pushed a commit to dotnet/java-interop that referenced this issue Feb 21, 2024
Context: #540

Refactor `Java.Interop.Tools.JavaCallableWrappers` to be more
maintainable and modular.  Adopts the same architecture as
`generator`: using a `CecilImporter` to import data from a compiled
assembly into an intermediate model.  Java code is then generated
from the intermediate model.

Refactoring and updating this code should make it not only more
maintainable, but also easier to extend with potential upcoming
features we are considering:

  - Moving `jcw-gen` into the `ILLink` pipeline
  - [Sanity-check Java Callable Wrappers][0]
  - Possibly creating a Kotlin backend

[0]: dotnet/android#4278
@jpobst jpobst removed this from the .NET 9 milestone May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Bindings Issues in Java Library Binding projects. enhancement Proposed change to current functionality.
Projects
None yet
Development

No branches or pull requests

3 participants