Bump to xamarin/Java.Interop:master@d1cce190 #1998

jonpryor · 2018-07-25T11:40:35Z

Fix jnimarshalmethod-gen.exe to load assemblies without symbols.

Allow jnimarshalmethod-gen.exe to use $MONO_PATH.

Update Java.Runtime.Environment.dll to not use a
Java.Runtime.Environment.dll.config file to specify the JVM
library location.

atsushieno · 2018-07-25T17:30:45Z

Could we get this PR based on the latest Java.Interop?

jonpryor · 2018-07-25T20:09:52Z

On the plus side, once bumped to dotnet/java-interop@9fecba2a, it builds.

On the minus side, it results in API breakage (?!).

I do not immediately understand how/why any of those members would change from a Java.Interop bump. More investigation forthcoming.

jonpryor · 2018-07-26T19:39:16Z

The tale so far: the API breakage appears to happen because bin/BuildDebug/api is changed as part of the Java.Interop bump, and something in that change causes e.g. IsConnected to be removed, e.g.

-    <class abstract="false" deprecated="not deprecated" extends="android.nfc.tech.BasicTagTechnology" extends-generic-aware="android.nfc.tech.BasicTagTechnology" final="true" name="NdefFormatable" static="false" visibility="public">
-      <method abstract="false" deprecated="not deprecated" final="false" name="close" native="false" return="void" static="false" synchronized="false" visibility="public">
+    <class abstract="false" deprecated="not deprecated" extends="android.nfc.tech.BasicTagTechnology" extends-generic-aware="android.nfc.tech.BasicTagTechnology" jni-extends="Landroid/nfc/tech/BasicTagTechnology;" final="true" name="NdefFormatable" static="false" visibility="public" jni-signature="Landroid/nfc/tech/NdefFormatable;">
+      <method abstract="false" deprecated="not deprecated" final="false" name="close" jni-signature="()V" bridge="true" native="false" return="void" jni-return="V" static="false" synchronized="false" synthetic="true" visibility="public">
         <exception name="IOException" type="java.io.IOException">
         </exception>
       </method>
-      <method abstract="false" deprecated="not deprecated" final="false" name="connect" native="false" return="void" static="false" synchronized="false" visibility="public">
+      <method abstract="false" deprecated="not deprecated" final="false" name="connect" jni-signature="()V" bridge="true" native="false" return="void" jni-return="V" static="false" synchronized="false" synthetic="true" visibility="public">
         <exception name="IOException" type="java.io.IOException">
         </exception>
       </method>
-      <method abstract="false" deprecated="not deprecated" final="false" name="format" native="false" return="void" static="false" synchronized="false" visibility="public">
-        <parameter name="firstMessage" type="android.nfc.NdefMessage">
+      <method abstract="false" deprecated="not deprecated" final="false" name="format" jni-signature="(Landroid/nfc/NdefMessage;)V" bridge="false" native="false" return="void" jni-return="V" static="false" synchronized="false" synthetic="false" visibility="public">
+        <parameter name="firstMessage" type="android.nfc.NdefMessage" jni-type="Landroid/nfc/NdefMessage;">
        </parameter>
         <exception name="FormatException" type="android.nfc.FormatException">
         </exception>
         <exception name="IOException" type="java.io.IOException">
         </exception>
       </method>
-      <method abstract="false" deprecated="not deprecated" final="false" name="formatReadOnly" native="false" return="void" static="false" synchronized="false" visibility="public">
-        <parameter name="firstMessage" type="android.nfc.NdefMessage">
+      <method abstract="false" deprecated="not deprecated" final="false" name="formatReadOnly" jni-signature="(Landroid/nfc/NdefMessage;)V" bridge="false" native="false" return="void" jni-return="V" static="false" synchronized="false" synthetic="false" visibility="public">
+        <parameter name="firstMessage" type="android.nfc.NdefMessage" jni-type="Landroid/nfc/NdefMessage;">
         </parameter>
         <exception name="FormatException" type="android.nfc.FormatException">
         </exception>
         <exception name="IOException" type="java.io.IOException">
         </exception>
       </method>
-      <method abstract="false" deprecated="not deprecated" final="false" name="get" native="false" return="android.nfc.tech.NdefFormatable" static="true" synchronized="false" visibility="public">
-        <parameter name="tag" type="android.nfc.Tag">
+      <method abstract="false" deprecated="not deprecated" final="false" name="get" jni-signature="(Landroid/nfc/Tag;)Landroid/nfc/tech/NdefFormatable;" bridge="false" native="false" return="android.nfc.tech.NdefFormatable" jni-return="Landroid/nfc/tech/NdefFormatable;" static="true" synchronized="false" synthetic="false" visibility="public">
+        <parameter name="tag" type="android.nfc.Tag" jni-type="Landroid/nfc/Tag;">
         </parameter>
       </method>
-      <method abstract="false" deprecated="not deprecated" final="false" name="getTag" native="false" return="android.nfc.Tag" static="false" synchronized="false" visibility="public">
+      <method abstract="false" deprecated="not deprecated" final="false" name="getTag" jni-signature="()Landroid/nfc/Tag;" bridge="true" native="false" return="android.nfc.Tag" jni-return="Landroid/nfc/Tag;" static="false" synchronized="false" synthetic="true" visibility="public">
       </method>
-      <method abstract="false" deprecated="not deprecated" final="false" name="isConnected" native="false" return="boolean" static="false" synchronized="false" visibility="public">
+      <method abstract="false" deprecated="not deprecated" final="false" name="isConnected" jni-signature="()Z" bridge="true" native="false" return="boolean" jni-return="Z" static="false" synchronized="false" synthetic="true" visibility="public">
       </method>
     </class>

I believe that these changes are due to Java.Interop@0881accd.

(Just making that determination took longer than I'd have liked.)

Still unknown is:

What change to bin/BuildDebug/api causes the API breakage? Is it the above diff, or something else?
What change in Java.Interop causes the changes to bin/BuildDebug/api?
What's the fix?

jonpryor · 2018-07-26T20:20:13Z

What change to bin/BuildDebug/api causes the API breakage? Is it the above diff, or something else?

It appears that the previous diff causes the breakage: taking a "known good" api.xml and copying just the <class/> declaration for NdefFormatable results in the IsConnected property not being generated.

What change in Java.Interop causes the changes to bin/BuildDebug/api?

Because the presence of the added attributes cause IsConnected to not be bound, Java.Interop@0881accd is the likely cause.

That isn't necessarily a root cause. Presumably generator alters behavior in the presence of these added attributes. Is that the case?

What's the fix?

This requires some investigation in the generator side; what about the new attributes causes generator to skip members?

jonpryor · 2018-07-26T20:45:25Z

Less than the previous diff is necessary to cause NdefFormatable.IsConnected to be skipped; just this diff to obj/Debug/android-28/mcw/api.xml will cause it to be skipped:

--- obj/Debug/android-28.good/mcw/api.xml	2018-07-26 15:48:51.000000000 -0400
+++ obj/Debug/android-28/mcw/api.xml	2018-07-26 16:33:01.000000000 -0400
@@ -36866,7 +36866,10 @@
         <parameter name="tag" type="android.nfc.Tag"></parameter>
       </method>
       <method abstract="false" deprecated="not deprecated" final="false" name="getTag" native="false" return="android.nfc.Tag" static="false" synchronized="false" visibility="public" />
-      <method abstract="false" deprecated="not deprecated" final="false" name="isConnected" native="false" return="boolean" static="false" synchronized="false" visibility="public" />
+      <method abstract="false" deprecated="not deprecated" final="false" name="isConnected" jni-signature="()Z" bridge="true" native="false" return="boolean" jni-return="Z" static="false" synchronized="false" synthetic="true" visibility="public" />
     </class>
     <class abstract="false" deprecated="not deprecated" extends="android.nfc.tech.BasicTagTechnology" extends-generic-aware="android.nfc.tech.BasicTagTechnology" final="true" name="NfcA" static="false" visibility="public">
       <method abstract="false" deprecated="not deprecated" final="false" name="close" native="false" return="void" static="false" synchronized="false" visibility="public">

This smaller differ allows actual reading and understanding (wow!), allowing a hypothesis: is it the //method[@synthetic='true'] which causes the member to be skipped?

One edit later, and that's exactly the case: the presence of //method[@synthetic='true'] causes generator to skip binding the member, and because of Java.Interop@0881accd this attribute is "carried forward", whereas previously the synthetic attribute was "skipped" and didn't make its way into bin/BuildDebug/api/*.xml.in.

jonpryor · 2018-07-26T21:02:20Z

On the "why is generator skipping synthetic methods" side of things, that's because generator skips synthetic methods. That change was introduced in monodroid@2c515356:

Don't emit synthetic methods, as this results in "duplicate" methods
-- one for the "real" method, one for the synthetic method.

That was in the context of using class-parse output "as-is", which isn't something we currently do -- we use build-tools/api-xml-adjuster output -- and is thus a scenario that doesn't actually exist, in practice, because the //@synthetic attribute was never present "in the wild."

Until now.

jonpryor · 2018-07-26T21:39:21Z

That investigation out of the way, now we can discuss (3): what's the fix?

There are two plausible fixes:

"Partially revert" Java.Interop@0881accd so that the synthetic attribute isn't copied, or
Update generator to not pay attention to the synthetic attribute.

Given that synthetic handling was never A Thing™ -- the commit which introduced the change said that the changes were incomplete -- it makes sense to remove the handling of //method/@synthetic from generator until it can be properly fixed:

While using class-parse to bind Android N Preview 1 (see also
bb5888bc, c9e5cbd6), the generated code has several errors.

Fix some of them:

jonpryor · 2018-07-26T21:45:40Z

Related (while I'm looking at it): the JavaFinalize() was removed because XmlClassGen would skip methods with //method[@name='finalize' and @jni-signature='()V']. Prior to this Java.Interop bump, bin/BuildDebug/api/*.xml.in wouldn't have the //method/@jni-signature attributes either, so this check would never be executed.

As such, this check should also be removed.

Context: dotnet/android#1998 (comment) Context: https://github.com/xamarin/monodroid/commit/2c515356d9a671f729aff2e81c4c2f6184749722 Long ago, `generator` was written to consume API XML descriptions provided by Google. This approach worked reasonably well until Honeycomb, at which point Google didn't provide an XML API description when the API-H `android.jar` was released. The fix was `jar2xml`, which used Java Reflection around `android.jar` to generate a similar API XML description which `generator` could use. `jar2xml` in turn had its own share of problems, which resulted in `class-parse`, which had a *different* share of problems. :-) The original hope was that `generator` could directly use `class-parse` output. Turns out, it wasn't that easy, hence `Xamarin.Android.Tools.ApiXmlAdjuster`, which took `class-parse` output and "munged" it into something resembling `jar2xml` output (e.g. with respect to whether method overrides were included). However, *before* `Xamarin.Android.Tools.ApiXmlAdjuster` existed, I tried to get `generator` to consume `class-parse` output directly, and in that spirit monodroid@2c515356 updated `generator` to *skip* processing of `synthetic` methods: > Don't emit `synthetic` methods, as this results in "duplicate" methods > -- one for the "real" method, one for the synthetic method. However, the scenario monodroid@2c515356 attempted to address -- direct support for `class-parse` output -- is not something that has ever been used, or been tested, or even *worked*. Thus, the special-casing of `synthetic` methods is effectively dead code, all the more so because `Xamarin.Android.Tools.ApiXmlAdjuster` never emitted the `//method/@synthetic` attribute in the first place, so `generator`s special-casing of `synthetic` methods was never hit. ...until 0881acc, which updated `Xamarin.Android.Tools.ApiXmlAdjuster` to include and copy over *all* source attributes, *including* `//method/@synthetic`. *NOW* `generator` began to see methods with `//method[@Synthetic='true']`, *and **skip** them*. The resulting code *compiled*, but had [ABI breakage][0]. Remove the special-casing logic for `synthetic` methods from `generator`. monodroid@2c515356 never supported direct consumption of `class-parse` output by `generator`, meaning such an effort will require more work *anyway*, and removing `synthetic` special-casing from `generator` will allow it to consume post-0881accd `Xamarin.Android.Tools.ApiXmlAdjuster` output without breaking things. [0]: https://jenkins.mono-project.com/job/xamarin-android-pr-builder/3642/API_20Compatibility_20Checks/

jonpryor · 2018-07-26T22:19:38Z

I believe that the ABI breakage will be fixed by: dotnet/java-interop#348

Context: dotnet/android#1998 (comment) Context: https://github.com/xamarin/monodroid/commit/2c515356d9a671f729aff2e81c4c2f6184749722 Long ago, `generator` was written to consume API XML descriptions provided by Google. This approach worked reasonably well until Honeycomb, at which point Google didn't provide an XML API description when the API-H `android.jar` was released. The fix was `jar2xml`, which used Java Reflection around `android.jar` to generate a similar API XML description which `generator` could use. `jar2xml` in turn had its own share of problems, which resulted in `class-parse`, which had a *different* share of problems. :-) The original hope was that `generator` could directly use `class-parse` output. Turns out, it wasn't that easy, hence `Xamarin.Android.Tools.ApiXmlAdjuster`, which took `class-parse` output and "munged" it into something resembling `jar2xml` output (e.g. with respect to whether method overrides were included). However, *before* `Xamarin.Android.Tools.ApiXmlAdjuster` existed, I tried to get `generator` to consume `class-parse` output directly, and in that spirit monodroid@2c515356 updated `generator` to *skip* processing of `synthetic` methods: > Don't emit `synthetic` methods, as this results in "duplicate" methods > -- one for the "real" method, one for the synthetic method. However, the scenario monodroid@2c515356 attempted to address -- direct support for `class-parse` output -- is not something that has ever been used, or been tested, or even *worked*. Thus, the special-casing of `synthetic` methods is effectively dead code, all the more so because `Xamarin.Android.Tools.ApiXmlAdjuster` never emitted the `//method/@synthetic` attribute in the first place, so `generator`s special-casing of `synthetic` methods was never hit. ...until 0881acc, which updated `Xamarin.Android.Tools.ApiXmlAdjuster` to include and copy over *all* source attributes, *including* `//method/@synthetic`. *NOW* `generator` began to see methods with `//method[@Synthetic='true']`, *and **skip** them*. The resulting code *compiled*, but had [ABI breakage][0]. Remove the special-casing logic for `synthetic` methods from `generator`. monodroid@2c515356 never supported direct consumption of `class-parse` output by `generator`, meaning such an effort will require more work *anyway*, and removing `synthetic` special-casing from `generator` will allow it to consume post-0881accd `Xamarin.Android.Tools.ApiXmlAdjuster` output without breaking things. [0]: https://jenkins.mono-project.com/job/xamarin-android-pr-builder/3642/API_20Compatibility_20Checks/

Fix `jnimarshalmethod-gen.exe` to load assemblies without symbols. Allow `jnimarshalmethod-gen.exe` to use `$MONO_PATH`. Update `Java.Runtime.Environment.dll` to *not* use a `Java.Runtime.Environment.dll.config` file to specify the JVM library location.

This reverts commit 997b890.

Context: dotnet/android#1998 (comment) Context: https://github.com/xamarin/monodroid/commit/2c515356d9a671f729aff2e81c4c2f6184749722 Long ago, `generator` was written to consume API XML descriptions provided by Google. This approach worked reasonably well until Honeycomb, at which point Google didn't provide an XML API description when the API-H `android.jar` was released. The fix was `jar2xml`, which used Java Reflection around `android.jar` to generate a similar API XML description which `generator` could use. `jar2xml` in turn had its own share of problems, which resulted in `class-parse`, which had a *different* share of problems. :-) The original hope was that `generator` could directly use `class-parse` output. Turns out, it wasn't that easy, hence `Xamarin.Android.Tools.ApiXmlAdjuster`, which took `class-parse` output and "munged" it into something resembling `jar2xml` output (e.g. with respect to whether method overrides were included). However, *before* `Xamarin.Android.Tools.ApiXmlAdjuster` existed, I tried to get `generator` to consume `class-parse` output directly, and in that spirit monodroid@2c515356 updated `generator` to *skip* processing of `synthetic` methods: > Don't emit `synthetic` methods, as this results in "duplicate" methods > -- one for the "real" method, one for the synthetic method. However, the scenario monodroid@2c515356 attempted to address -- direct support for `class-parse` output -- is not something that has ever been used, or been tested, or even *worked*. Thus, the special-casing of `synthetic` methods is effectively dead code, all the more so because `Xamarin.Android.Tools.ApiXmlAdjuster` never emitted the `//method/@synthetic` attribute in the first place, so `generator`s special-casing of `synthetic` methods was never hit. ...until 0881acc, which updated `Xamarin.Android.Tools.ApiXmlAdjuster` to include and copy over *all* source attributes, *including* `//method/@synthetic`. *NOW* `generator` began to see methods with `//method[@Synthetic='true']`, *and **skip** them*. The resulting code *compiled*, but had [ABI breakage][0]. Remove the special-casing logic for `synthetic` methods from `generator`. monodroid@2c515356 never supported direct consumption of `class-parse` output by `generator`, meaning such an effort will require more work *anyway*, and removing `synthetic` special-casing from `generator` will allow it to consume post-0881accd `Xamarin.Android.Tools.ApiXmlAdjuster` output without breaking things. [0]: https://jenkins.mono-project.com/job/xamarin-android-pr-builder/3642/API_20Compatibility_20Checks/

jonpryor force-pushed the jonp-bump-ji-d1cce190 branch from dc459dd to 868d649 Compare July 25, 2018 17:46

jonpryor mentioned this pull request Jul 26, 2018

[generator] Don't special case synthetic methods dotnet/java-interop#348

Merged

jonpryor force-pushed the jonp-bump-ji-d1cce190 branch from 868d649 to e12d96a Compare July 27, 2018 11:42

jonpryor merged commit 997b890 into dotnet:master Jul 27, 2018

jonpryor added a commit that referenced this pull request Jul 27, 2018

Revert "Bump to xamarin/Java.Interop:master@78f73018 (#1998)"

348d621

This reverts commit 997b890.

github-actions bot locked and limited conversation to collaborators Feb 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Bump to xamarin/Java.Interop:master@d1cce190 #1998

Bump to xamarin/Java.Interop:master@d1cce190 #1998

jonpryor commented Jul 25, 2018

Uh oh!

atsushieno commented Jul 25, 2018

Uh oh!

jonpryor commented Jul 25, 2018

Uh oh!

jonpryor commented Jul 26, 2018 •

edited

Loading

Uh oh!

jonpryor commented Jul 26, 2018

Uh oh!

jonpryor commented Jul 26, 2018

Uh oh!

jonpryor commented Jul 26, 2018

Uh oh!

jonpryor commented Jul 26, 2018

Uh oh!

jonpryor commented Jul 26, 2018

Uh oh!

jonpryor commented Jul 26, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Bump to xamarin/Java.Interop:master@d1cce190 #1998

Bump to xamarin/Java.Interop:master@d1cce190 #1998

Conversation

jonpryor commented Jul 25, 2018

Uh oh!

atsushieno commented Jul 25, 2018

Uh oh!

jonpryor commented Jul 25, 2018

Uh oh!

jonpryor commented Jul 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonpryor commented Jul 26, 2018

Uh oh!

jonpryor commented Jul 26, 2018

Uh oh!

jonpryor commented Jul 26, 2018

Uh oh!

jonpryor commented Jul 26, 2018

Uh oh!

jonpryor commented Jul 26, 2018

Uh oh!

jonpryor commented Jul 26, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jonpryor commented Jul 26, 2018 •

edited

Loading