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

[Xamarin.Android.Build.Tasks] Add Support for AndroidJavaSource to Bindings #5926

Merged
merged 10 commits into from May 12, 2022

Conversation

dellis1972
Copy link
Contributor

@dellis1972 dellis1972 commented May 12, 2021

It would be nice to be able to introduce custom Java code and API's to a
binding project. This would allow developers to simplify an API and
still expose that to the C# Developer.

Using the existing AndroidJavaSource ItemGroup we can do exactly that.
This ItemGroup now supports the Bind metadata which will build and
bind the java code prior to the main C# compiled running. Historically
AndroidJavaSource was only available on Application Projects. However
this ItemGroup can now be used in any project type. The Bind metadata
is true by default and will cause the Java code to be compiled into
a .jar file before the C# compiler is run. It will then be bound using
the Java.Interop Java->C# binding system. The final C# apis will be
generated an included in the final C# compilation.

There are some restrictions on the Java code. You can only use Java types
or types which are defined in .jar or .aar files you reference for that
project via AndroidLibrary or EmbeddedJar.
It is advised that you DO NOT export types which include Java Generics
via this mechanism. Like normal bindings, Java Generics cause significant
problems. Try to stuck to normal types if you can.

@dellis1972 dellis1972 added the do-not-merge PR should not be merged. label May 12, 2021
@dellis1972 dellis1972 force-pushed the bindingjavac branch 8 times, most recently from 48deff7 to 91fdc5b Compare May 20, 2021 11:11
@dellis1972 dellis1972 removed the do-not-merge PR should not be merged. label May 20, 2021
@dellis1972 dellis1972 force-pushed the bindingjavac branch 2 times, most recently from 6e5464d to 9b911de Compare May 24, 2021 10:19
@dellis1972 dellis1972 marked this pull request as ready for review May 24, 2021 11:29
@dellis1972 dellis1972 requested a review from jonpryor as a code owner May 24, 2021 11:29
@dellis1972
Copy link
Contributor Author

I need to make changes to this anyway to make sure the new targets do not run as part of a DTB.

@jonpryor
Copy link
Member

I can't easily answer this from reading the PR, but one thing we should try to ensure is that when %(AndroidJavaSource.Bind)=True, we also treat this item group "As If" the files were also part of a @(JavaSourceJar), so that java-source-utils extracts parameter names from the source code.

A "unit test check" for this would be to check that the C# binding for src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Resources/JavaSourceTestExtension.java uses the same parameter names as the Java source:

public string GreetWithQuestion (string name, Java.Util.Date date, string question) 

This may be "easiest done" by copying all %(AndroidJavaSource.Bind)=True files into a new -source.jar file:

<PropertyGroup>
  <_AndroidIntermediateBindingClassesSourceJar>$(IntermediateOutputPath)binding\bin\$(MSBuildProjectName)-source.jar</_AndroidIntermediateBindingClassesSourceJar>
</PropertyGroup>

Then adding $(_AndroidIntermediateBindingClassesSourceJar) to @(JavaSourceJar).

$(_AndroidIntermediateBindingClassesZip) and $(_AndroidIntermediateBindingClassesSourceJar) should also be "project outputs", i.e. "copied into $(OutputPath), so that dotnet pack/etc. will copy them into NuGet packages. @(EmbeddedResource) Is Bad™.

@jonpryor
Copy link
Member

We had a call today, and made the following decisions:

  • %(AndroidJavaSource.Bind) support will be a .NET 6+ only feature. It won't exist in Legacy.

  • %(AndroidJavaSource.Bind) will default to True. This will be a potential breaking change; it changes semantics, causing Java code to be compiled earlier than it currently is, but it also might not actually break anybody, as we think it'll only cause compilation errors if the @(AndroidJavaSource) references Java Callable Wrapper types. Fixing the break will be easy: just set %(Bind)=False.

    Also, @(AndroidJavaSource) isn't widely used, at least not on GitHub: https://github.com/search?q=androidjavasource+extension%3Acsproj&type=Code&ref=advsearch&l=&l=

  • For d16-11, we'll consider adding a check for @(AndroidJavaSource) use and issue a warning that the semantics will be changing for .NET 6.

@dellis1972 dellis1972 force-pushed the bindingjavac branch 3 times, most recently from e383c4a to 8761aef Compare June 8, 2021 11:27
@dellis1972 dellis1972 force-pushed the bindingjavac branch 7 times, most recently from 9b75cbc to cba4afb Compare May 5, 2022 10:16
…ndings

It would be nice to be able to introduce custom Java code and API's to a
binding project. This would allow developers to simplify an API and
still expose that to the C# Developer.

Using the existing `AndroidJavaSource` ItemGroup we can do exactly that.
This ItemGroup now supports the `Bind` metadata which will build and
bind the java code *prior* to the main C# compiled running. Historically
`AndroidJavaSource` was only available on Application Projects. However
this ItemGroup can now be used in any project type. The `Bind` metadata
is `true` by default and will cause the Java code to be compiled into
a .jar file before the C# compiler is run. It will then be bound using
the Java.Interop Java->C# binding system. The final C# apis will be
generated an included in the final C# compilation.

There are some restrictions on the Java code. You can only use Java types
or types which are defined in .jar or .aar files you reference for that
project via `AndroidLibrary` or `EmbeddedJar`.
It is advised that you DO NOT export types which include Java Generics
via this mechanism. Like normal bindings, Java Generics cause significant
problems. Try to stuck to normal types if you can.
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comments I had at this point are really minor. 👍

dellis1972 and others added 2 commits May 6, 2022 22:01
Co-authored-by: Jonathan Peppers <jonathan.peppers@gmail.com>
…n.Android.Javac.targets

Co-authored-by: Jonathan Peppers <jonathan.peppers@gmail.com>
@@ -128,6 +128,12 @@ Copyright (C) 2012 Xamarin Inc. All rights reserved.
/>
</CreateProperty>

<CreateProperty Value="$(_JavaSdkDirectory)bin">
<Output TaskParameter="Value" PropertyName="JavacToolPath"
Condition="'$(JavacToolPath)' == ''"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this also have And Exists('$(_JavaSdkDirectory)bin')? Or will we error out earlier if $(_JavaSdkDirectory) doesn't exist?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

…and shouldn't we start using <PropertyGroup/> instead of <CreateProperty/> here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this to that it matches what was already there. I can rework this to use PropertyGroup. Or perhaps we get this in, and then do a PR to remove ALL CreateProperty references?

@jonpryor
Copy link
Member

Commit message?

[Xamarin.Android.Build.Tasks] Add %(AndroidJavaSource.Bind) (#5926)

Update the `@(AndroidJavaSource)` Build action:

 1. In .NET 7, all `**\*.java` files are automatically added to the
    `@(AndroidJavaSource)` Build action.

 2. Add `%(AndroidJavaSource.Bind)` item metadata, a boolean value
    (default is True) which controls whether or not the file is
    compiled and bound *prior* to the C# build.

The combination of these means that custom Java code can be added to
any .NET Android project, and a binding of that Java code will be
usable by the C# code.  Both Java code and bindings will be included
in App builds of referencing apps.

There are some restrictions on the Java code.  You can only use Java
types or types which are defined in `.jar` or `.aar` files referenced
by the project via `@(AndroidLibrary)` or `@(EmbeddedJar)`.  It is
suggested that you *do not* have `publi` Java types which use Java
generics.  As with normal bindings, Java generics can cause
significant problems; try to stuck to normal types if you can.

@jonpryor
Copy link
Member

@dellis1972: I don't see any way to get useful parameter names into the bindings. We had previous suggested using <JavaSourceUtils/> to extract parameter names:

<Target Name="_ExtractJavadocsFromJavaSourceJars"
Condition=" '$(_UseLegacyJavadocImport)' != 'True' And '@(JavaSourceJar->Count())' != '0' "
DependsOnTargets="_GetJavaSourceJarJavadocFiles"
Inputs="@(JavaSourceJar)"
Outputs="@(_JavaSourceJavadocXml)">
<JavaSourceUtils
ContinueOnError="True"
JavaSourceUtilsJar="$(AndroidJavaSourceUtilsJar)"
InputFiles="%(_JavaSourceJavadocXml.OriginalItemSpec)"
JavadocCopyrightFile="%(_JavaSourceJavadocXml.CopyrightFile)"
JavadocUrlPrefix="%(_JavaSourceJavadocXml.UrlPrefix)"
JavadocUrlStyle="%(_JavaSourceJavadocXml.UrlStyle)"
JavadocDocRootUrl="%(_JavaSourceJavadocXml.DocRootUrl)"
JavaMaximumHeapSize="$(JavaMaximumHeapSize)"
JavaOptions="$(JavaOptions)"
JavaSdkDirectory="$(_JavaSdkDirectory)"
OutputJavadocXml="%(_JavaSourceJavadocXml.Identity)"
References="@(JavaSourceJar)"
/>

Alternatively, we could update the <Javac/> task to use javac -parameters.

However, I don't see either of these options here.

Are we doing anything to import parameter names?

@dellis1972
Copy link
Contributor Author

Are we doing anything to import parameter names?

We talked about this and I seem to remember we decided to do this at a later date, cos this PR was large enough already.

@jonpryor jonpryor merged commit 67ce902 into xamarin:main May 12, 2022
@dellis1972 dellis1972 deleted the bindingjavac branch May 16, 2022 09:25
jonathanpeppers pushed a commit that referenced this pull request Jun 17, 2022
Context: #5926
Fixes: dotnet/maui#8049

There was a bug causing response files to not work when trying to bind
Java libraries in a path containing a space, since the CLI argument
given to `class-parse`/`generator` was not properly quoted.

This has been fixed in 67ce902, however this is
a .NET 7 only PR.

Pull the fix out of that PR and port it to the `6.0.4xx` branch so it
will go into a .NET 6 servicing release.
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 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.

None yet

3 participants