Skip to content

Conversation

@jonpryor
Copy link
Contributor

@jonpryor jonpryor commented Dec 21, 2021

Context: #867

There are two scenarios for which a "generalized" type & member
remapping solution would be useful:

  1. Desugaring
  2. Microsoft Intune MAM

Note: this new support is only present when targeting .NET 6+,
and will not (currently) be present in Classic Xamarin.Android.

~~ Desugaring ~~

Context: dotnet/android#4574 (comment)
Context: dotnet/android#6142 (comment)

The Android Runtime is responsible for running the Dalvik
bytecode within e.g. classes.dex. The Android runtime and Dalvik
bytecode formats have apparently changed over time in order to
support newer Java language features, such as support for static
interface methods:

package org.webrtc;

public /* partial */ interface EGLBase {
    public static EGLBase create() { /* … */}
}

Support for static interface methods was added in Java 8 and in
Android v8.0 (API-26). If you want to use static interface methods
on an Android device running API-25 or earlier, you must desugar
the Java Bytecode. The result of desugaring the above
org.webrtc.EGLBase type is that the static methods are moved to a
new type, with a $-CC suffix, "as if" the Java code were instead:

package org.webrtc;

public /* partial */ interface EGLBase {
    // …
}

public final /* partial */ class EGLBase$-CC {
    public static EGLBase create { /* … */ }
}

While this works for pure Java code, this does not work with
Xamarin.Android, because our bindings currently expect (require) that
the types & members our binding process detects don't "move":

// C# Binding for EGLBase
namespace Org.Webrtc {
    public partial interface IEGLBase {
        private static readonly JniPeerMembers _members = new JniPeerMembers ("org/webrtc/EGLBase", typeof (IEGLBase));
        public static IEGLBase Create()
        {
            const string __id = "create.()Lorg/webrtc/EglBase;"
            var __rm = _members.StaticMethods.InvokeObjectMethod (__id, null);
            return Java.Lang.Object.GetObject<IEGLBase>(__rm.Handle, JniHandleOwnership.TransferLocalRef);
        }
    }
}

The new JniPeerMembers(…) invocation will use JNIEnv::FindClass()
to lookup the org/webrtc/EGLBase type, and IEGLBase.Create() will
use JNIEnv::GetStaticMethodID() and JNIEnv::CallStaticObjectMethod()
to lookup and invoke the EGLBase.create() method.

However, when desugaring is used, there is no EGLBase.create()
method. Consequently, in a "desugared" environment,
Java.Lang.NoSuchMethodError will be thrown when IEGLBase.Create()
is invoked, as EGLBase.create() doesn't exist; it "should" instead
be looking for EGLBase$-CC.create()!

Introduce partial support for this scenario by adding the new method:

namespace Java.Interop {
    public partial class JniRuntime {
        public partial class JniTypeManager {
            public IReadOnlyList<string>? GetStaticMethodFallbackTypes (string jniSimpleReference) =>
                GetStaticMethodFallbackTypesCore (jniSimpleReference);
            protected virtual IReadOnlyList<string>? GetStaticMethodFallbackTypesCore (string jniSimpleReference) => null;
        }
    }
}

JniPeerMembers.JniStaticMethods.GetMethodInfo() will attempt to
lookup the static method, "as normal". If the method cannot be
found, then JniRuntime.JniTypeManager.GetStaticMethodFallbackTypes()
will be called, and we'll attempt to resolve the static method on each
type returned by GetStaticMethodFallbackTypes().
If GetStaticMethodFallbackTypes() returns null or no fallback type
provides the method, then a NoSuchMethodError will be thrown.

TODO: Update xamarin-android to override
GetStaticMethodFallbackTypes(), to return
$"{jniSimpleReference}$-CC".

~~ Microsoft Intune MAM ~~

Context: https://www.nuget.org/packages/Microsoft.Intune.MAM.Remapper.Tasks/
Context: https://docs.microsoft.com/en-us/mem/intune/developer/app-sdk-xamarin#remapper

The Microsoft Intune App SDK Xamarin Bindings is in a
similar-yet-different position: it involves Java & Dalvik Bytecode
manipulation for various security purposes, and in order to make that
work reasonably from Xamarin.Android, they also rewrite
Xamarin.Android IL so that it's consistent with the manipulated
Dalvik bytecode.

See readme.md and content/MonoAndroid10/remapping-config.json
within the Microsoft.Intune.MAM.Remapper.Tasks NuGet package
for some additional details/tidbits such as:

This task operates on assemblies to replace the base type classes
which Microsoft Intune requires to implement its SDK (for example,
Intune requires using a MAMActivity in place of Activity and
methods such as OnMAMCreate instead of OnCreate).

"ClassRewrites": [
  {
    "Class": {
      "From": "android.app.Activity",
      "To": "com.microsoft.intune.mam.client.app.MAMActivity"
    },
    "Methods": [
      {
        "MakeStatic": false,
        "OriginalName": "onCreate",
        "NewName": "onMAMCreate",
        "OriginalParams": [
          "android.os.Bundle"
        ]
      }
    ]
  },
  {	
    "Class": {
      "From": "android.content.pm.PackageManager",
      "To": "com.microsoft.intune.mam.client.content.pm.MAMPackageManagement"
    },
    "Methods": [
      {
        "MakeStatic": true,
        "OriginalName": "checkPermission",
      }
    ]
  }
]
"GlobalMethodCalls": [
  {
    "Class": {
      "From": "android.app.PendingIntent",
      "To": "com.microsoft.intune.mam.client.app.MAMPendingIntent"
    },
    "Methods": [
      {
        "MakeStatic": false,
        "OriginalName": "getActivities"
      },
    ]
  }
]

While what the InTune team has works, it suffers from a number of
"workflow" problems, e.g. incremental builds are problematic (they
might try to rewrite assemblies which were already rewritten), IL
rewriting is always "fun", and if we change IL binding patterns,
they'll need to support previous "binding styles" and newer styles.

Introduce partial support for this scenario by adding the following
members to Java.Interop.JniRuntime.JniTypeManager:

namespace Java.Interop {
    public partial class JniRuntime {

        public struct ReplacementMethodInfo {
            public  string? SourceJniType                   {get; set;}
            public  string? SourceJniMethodName             {get; set;}
            public  string? SourceJniMethodSignature        {get; set;}
            public  string? TargetJniType                   {get; set;}
            public  string? TargetJniMethodName             {get; set;}
            public  string? TargetJniMethodSignature        {get; set;}
            public  int?    TargetJniMethodParameterCount   {get; set;}
            public  bool    TargetJniMethodIsStatic         {get; set;}
        }

        public partial class JniTypeManager {

            public string? GetReplacementType (string jniSimpleReference) =>
                GetReplacementTypeCore (jniSimpleReference);
            protected virtual string? GetReplacementTypeCore (string jniSimpleReference) => null;

            public ReplacementMethodInfo? GetReplacementMethodInfo (string jniSimpleReference, string jniMethodName, string jniMethodSignature) =>
                GetReplacementMethodInfoCore (jniSimpleReference, jniMethodName,jniMethodSignature);
            protected virtual ReplacementMethodInfo? GetReplacementMethodInfoCore (string jniSimpleReference, string jniMethodName, string jniMethodSignature) => null;
        }
    }
}

These new methods are used by JniPeerMembers.

Consider "normal" usage of JniPeerMembers, within a binding:

partial class Activity {
    static readonly JniPeerMembers _members = new XAPeerMembers (jniPeerTypeName:"android/app/Activity", managedPeerType:typeof (Activity));
}

JniRuntime.JniTypeManager.GetReplacementType() allows "replacing"
the jniPeerTypeName value with a "replacement" JNI type name.
The "replacement" type will be used for all field and method lookups.
This allows supporting the ClassRewrites[0].Class.From /
ClassRewrites[0].Class.To semantics.

JniRuntime.JniTypeManager.GetReplacementMethodInfo() is the key
integration for all other "replacement" semantics. Given the
source type, source method name, and source JNI signature, it is
responsible for providing an overriding target type, target method
name, and target JNI signature.

For ClassRewrites[0].Methods[0], this allows "renaming"
Activity.onCreate() to MAMActivity.onMAMCreate():

var r = JniEnvironment.Runtime.TypeManager.GetReplacementMethodInfo (
    // Note: must match GetReplacementType() *output*, and since
    // `Activity` is mapped to `MAMActivity`…
    "com/microsoft/intune/mam/client/app/MAMActivity",
    "onCreate",
    "(Landroid/os/Bundle;)V"
);
// is equivalent to:
var r = new JniRuntime.ReplacementMethodInfo {
    SourceJniType                   = "com/microsoft/intune/mam/client/app/MAMActivity",
                                                                // from input parameter
    SourceJniMethodName             = "onCreate",               // from input parameter
    SourceJniMethodSignature        = "(Landroid/os/Bundle;)V", // from input parameter

    TargetJniType                   = "com/microsoft/intune/mam/client/app/MAMActivity",
    TargetJniMethodName             = "onMAMCreate",
    TargetJniMethodSignature        = null,                     // "use original value"
    TargetJniMethodParameterCount   = null,                     // unchanged
    TargetJniMethodIsStatic         = false,
}

ClassRewrites[1].Methods[0] is "weird", in that it has
MakeStatic: true. The semantics for when MakeStatic: true exists
is that the "original" method is an instance method, and the target
method is a static method, and the this instance now becomes
a parameter to the method. This is likewise supported via
JniRuntime.JniTypeManager.GetReplacementMethodInfo(), and is
identified by:

  1. ReplacementMethodInfo.TargetJniMethodParameterCount being a
    non-null value, and

  2. ReplacementMethodInfo.TargetJniMethodIsStatic is true.

Thus, ClassRewrites[1].Methods[0] is akin to:

var r = JniEnvironment.Runtime.TypeManager.GetReplacementMethodInfo (
    "android/content/pm/PackageManager",
    "checkPermission",
    "(Ljava/lang/String;Ljava/lang/String;)I"
);
// is equivalent to:
var r = new JniRuntime.ReplacementMethodInfo {
    SourceJniType                   = "android/content/pm/PackageManager",          // from input parameter
    SourceJniMethodName             = "checkPermission",                            // from input parameter
    SourceJniMethodSignature        = "(Ljava/lang/String;Ljava/lang/String;)I",    // from input parameter

    TargetJniType                   = "com/microsoft/intune/mam/client/content/pm/MAMPackageManagement",
    TargetJniMethodName             = "CheckPermission",
    TargetJniMethodSignature        = "(Landroid/content/pm/PackageManager;Ljava/lang/String;Ljava/lang/String;)I",
                                      // Note: `PackageManager` is inserted as new first parameter
    TargetJniMethodParameterCount   = 3,
    TargetJniMethodIsStatic         = true,
}

(Note that the subclass is responsible for providing this data.)

ReplacementMethodInfo.TargetJniMethodParameterCount must be the
number of parameters that the target method accepts. This is needed
so that JniPeerMembers.JniInstanceMethods.TryInvoke*StaticRedirect()
can appropriately reallocate the JniArgumentValue* array, so that
the this parameter can be added to the beginning of the parameters.

~~ Overhead? ~~

All these extra invocations into JniRuntime.JniTypeManager imply
additional overhead to invoke a Java method. However, this is all
done during the first time a method is looked up, after which the
JniMethodInfo instance is cached for a given
(type, method, signature) tuple.

To demonstrate overheads, samples/Hello has been updated to accept
a new -t value; when provided, it invokes the
java.lang.Object.hashCode() method 1 million times and calculates
the average invocation overhead:

% dotnet run --project samples/Hello -- -t
Object.hashCode: 1000000 invocations. Total=00:00:00.5196758; Average=0.0005196758ms

If we replace the .NET 6 samples/Hello/bin/Debug/Java.Interop.dll
assembly with e.g. bin/Debug/Java.Interop.dll, we can compare to
performance without these new changes, as the changes are only in
the .NET 6 build:

% \cp bin/Debug/Java.Interop{.dll,.pdb} samples/Hello/bin/Debug
% dotnet samples/Hello/bin/Debug/Hello.dll -t
Object.hashCode: 1000000 invocations. Total=00:00:00.5386676; Average=0.0005386676ms

There is a fair bit of variation in dotnet Hello.dll -t output,
but they're all roughly similar. There doesn't appear to be
significant overhead for this particular test.

~~ Other Changes ~~

Cleaned up the JniPeerMembers constructors. The Debug.Assert()
calls were duplicative and redundant.

Replace the Debug.Assert() with Debug.WriteLine().
Mono.Android.NET-Tests.apk was running into an "unfixable" scenario:

WARNING: ManagedPeerType <=> JniTypeName Mismatch!
javaVM.GetJniTypeInfoForType(typeof(Android.Runtime.JavaList)).JniTypeName="java/util/ArrayList" != "java/util/List"

This was because of Android.Runtime.JavaList using a
JniPeerMembers for List while registering ArrayList, causing
typemaps to associate JavaList with ArrayList:

[Register ("java/util/ArrayList", DoNotGenerateAcw=true)]
partial class JavaList {
    internal static readonly JniPeerMembers list_members =
        new XAPeerMembers ("java/util/List", typeof (JavaList), isInterface: true);
}

@jonpryor doesn't want to try fixing this right now; turning the
assertion into a diagnostic message feels preferable.

@jonpryor jonpryor force-pushed the jonp-member-remapping branch 2 times, most recently from f59e2fb to 4ab7633 Compare December 21, 2021 15:09
@jonpryor jonpryor marked this pull request as draft December 21, 2021 17:12
@jonpryor jonpryor force-pushed the jonp-member-remapping branch 5 times, most recently from e8be497 to b57e6b0 Compare December 22, 2021 03:42
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Dec 22, 2021
@jonpryor jonpryor force-pushed the jonp-member-remapping branch from b57e6b0 to 7e50f28 Compare December 27, 2021 19:39
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Dec 28, 2021
@jonpryor jonpryor force-pushed the jonp-member-remapping branch 3 times, most recently from 004d78b to 2cee715 Compare January 4, 2022 01:55
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Jan 6, 2022
Context: dotnet/java-interop#867
Context: dotnet/java-interop#936

Does It Build?

Does It Work?

(Is It Sane?)

For local "test" purposes, add a new `tools/remap-mam-json-to-xml`
utility which parses the MAM JSON file into XML.

	$ dotnet run --project tools/remap-mam-json-to-xml  -- \
	  $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json
	<replacements>
	  <replace-type from="android/app/Activity" to="com/microsoft/intune/mam/client/app/MAMActivity" />
	  …
	  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onStateNotSaved" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMStateNotSaved" target-method-instance-to-static="false" />
	</replacements>

dotnet bin/Debug/net6.0/remap-mam-json-to-xml.dll $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Jan 6, 2022
Context: dotnet/java-interop#867
Context: dotnet/java-interop#936

Does It Build?

Does It Work?

(Is It Sane?)

For local "test" purposes, add a new `tools/remap-mam-json-to-xml`
utility which parses the MAM JSON file into XML.

	$ dotnet run --project tools/remap-mam-json-to-xml  -- \
	  $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json
	<replacements>
	  <replace-type from="android/app/Activity" to="com/microsoft/intune/mam/client/app/MAMActivity" />
	  …
	  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onStateNotSaved" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMStateNotSaved" target-method-instance-to-static="false" />
	</replacements>

dotnet bin/Debug/net6.0/remap-mam-json-to-xml.dll $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Jan 21, 2022
Context: dotnet/java-interop#867
Context: dotnet/java-interop#936

Does It Build?

Does It Work?

(Is It Sane?)

For local "test" purposes, add a new `tools/remap-mam-json-to-xml`
utility which parses the MAM JSON file into XML.

	$ dotnet run --project tools/remap-mam-json-to-xml  -- \
	  $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json
	<replacements>
	  <replace-type from="android/app/Activity" to="com/microsoft/intune/mam/client/app/MAMActivity" />
	  …
	  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onStateNotSaved" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMStateNotSaved" target-method-instance-to-static="false" />
	</replacements>

dotnet bin/Debug/net6.0/remap-mam-json-to-xml.dll $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json

Add a new `@(_AndroidMamMappingFile)` item group which can be
used to produce the mapping XML file.

TODO: why is `$(_AndroidMamMappingXml)` e.g.
`obj/Debug/assets/obj/Debug/assets/xa-mam-mapping.xml`?
What's with the duplication?

Update src/java-runtime to use `javac -h`, to emit JNI headers.
(`javah` is not present in JDK11!)

Update src/monodroid to depend on src/java-runtime, then use the
headers generated by `javac -h` to ensure consistency.
Rando aside: apparently `createNewContextWithData` was inconsistent?)
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Jan 21, 2022
Context: dotnet/java-interop#867
Context: dotnet/java-interop#936

Does It Build?

Does It Work?

(Is It Sane?)

For local "test" purposes, add a new `tools/remap-mam-json-to-xml`
utility which parses the MAM JSON file into XML.

	$ dotnet run --project tools/remap-mam-json-to-xml  -- \
	  $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json
	<replacements>
	  <replace-type from="android/app/Activity" to="com/microsoft/intune/mam/client/app/MAMActivity" />
	  …
	  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onStateNotSaved" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMStateNotSaved" target-method-instance-to-static="false" />
	</replacements>

dotnet bin/Debug/net6.0/remap-mam-json-to-xml.dll $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json

Add a new `@(_AndroidMamMappingFile)` item group which can be
used to produce the mapping XML file.

TODO: why is `$(_AndroidMamMappingXml)` e.g.
`obj/Debug/assets/obj/Debug/assets/xa-mam-mapping.xml`?
What's with the duplication?

Update src/java-runtime to use `javac -h`, to emit JNI headers.
(`javah` is not present in JDK11!)

Update src/monodroid to depend on src/java-runtime, then use the
headers generated by `javac -h` to ensure consistency.
Rando aside: apparently `createNewContextWithData` was inconsistent?)
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Jan 21, 2022
Context: dotnet/java-interop#867
Context: dotnet/java-interop#936

Does It Build?

Does It Work?

(Is It Sane?)

For local "test" purposes, add a new `tools/remap-mam-json-to-xml`
utility which parses the MAM JSON file into XML.

	$ dotnet run --project tools/remap-mam-json-to-xml  -- \
	  $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json
	<replacements>
	  <replace-type from="android/app/Activity" to="com/microsoft/intune/mam/client/app/MAMActivity" />
	  …
	  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onStateNotSaved" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMStateNotSaved" target-method-instance-to-static="false" />
	</replacements>

dotnet bin/Debug/net6.0/remap-mam-json-to-xml.dll $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json

Add a new `@(_AndroidMamMappingFile)` item group which can be
used to produce the mapping XML file.

TODO: why is `$(_AndroidMamMappingXml)` e.g.
`obj/Debug/assets/obj/Debug/assets/xa-mam-mapping.xml`?
What's with the duplication?

Update src/java-runtime to use `javac -h`, to emit JNI headers.
(`javah` is not present in JDK11!)

Update src/monodroid to depend on src/java-runtime, then use the
headers generated by `javac -h` to ensure consistency.
Rando aside: apparently `createNewContextWithData` was inconsistent?)
@jonpryor jonpryor force-pushed the jonp-member-remapping branch from 2cee715 to ccebcb4 Compare February 2, 2022 21:26
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Feb 2, 2022
Context: dotnet/java-interop#867
Context: dotnet/java-interop#936

Does It Build?

Does It Work?

(Is It Sane?)

For local "test" purposes, add a new `tools/remap-mam-json-to-xml`
utility which parses the MAM JSON file into XML.

	$ dotnet run --project tools/remap-mam-json-to-xml  -- \
	  $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json
	<replacements>
	  <replace-type from="android/app/Activity" to="com/microsoft/intune/mam/client/app/MAMActivity" />
	  …
	  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onStateNotSaved" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMStateNotSaved" target-method-instance-to-static="false" />
	</replacements>

dotnet bin/Debug/net6.0/remap-mam-json-to-xml.dll $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json

Add a new `@(_AndroidMamMappingFile)` item group which can be
used to produce the mapping XML file.

TODO: why is `$(_AndroidMamMappingXml)` e.g.
`obj/Debug/assets/obj/Debug/assets/xa-mam-mapping.xml`?
What's with the duplication?

Update src/java-runtime to use `javac -h`, to emit JNI headers.
(`javah` is not present in JDK11!)

Update src/monodroid to depend on src/java-runtime, then use the
headers generated by `javac -h` to ensure consistency.
Rando aside: apparently `createNewContextWithData` was inconsistent?)
jonpryor added a commit to jonpryor/java.interop that referenced this pull request Feb 4, 2022
Context: 2d5431f
Context: 88d6093
Context: dotnet#936
Context: https://github.com/jonpryor/java.interop/commits/jonp-registration-scope

Versioning is hard.

Way back in 3e6a623 we tried to use the `GitInfo` NuGet package so
that *all* assemblies would have a version number which contained the
number of commits since `GitInfo.txt` changed.

This turned out to have unanticipated breakage, and was largely
reverted in 2d5431f for "core" libs like `Java.Interop.dll`.

This still presents a problem, though: the *point* to assembly
versioning is to prevent accidental breaking of referencing
assemblies.  If we add a new member to `Java.Interop.dll` but
*fail to update the version*, then that *permits* a scenario in which
an app/lib depends on the new member, but is built against a version
missing that member.  This result in runtime exceptions.

The whole reason this hasn't been a problem so far is because
`Java.Interop.dll` has barely changed in *years*.  (Plus, most usage
is hidden behind other layers and libs…)

However, *I want this to change*: dotnet#936 and
jonpryor/java.interop/jonp-registration-scope both *add* new public
API to `Java.Interop.dll`, and there are other features and
optimizations we're considering that would also require API changes.
A policy of "no changes" isn't tenable.

Thus, the first priority: allow `Java.Interop.dll` built for .NET 6 to
have a different `$(Version)` than the one built for .NET Standard 2.0.

Fixing this was unexpectedly problematic, as commit 88d6093:

>  Update[d] `Java.Interop.BootstrapTasks.csproj` to *no longer* `<Import/>`
> `VersionInfo.targets`, as this causes a circular dependency
> (`VersionInfo.targets` uses tasks from
> `Java.Interop.BootstrapTasks.dll`).  This is fine, as
> `Java.Interop.BootstrapTasks.dll` doesn't need to be versioned.

`Java.Interop.BootstrapTasks.dll` doesn't need to be versioned, but
*other libraries **do***, and this change change meant that
`VersionInfo.targets` was *never* used, which in turn meant that e.g.
`bin/BuildDebug/Version.props` was never created.

Re-introduce `VersionInfo.targets` by "moving" it into
`build-tools/VersionInfo/VersionInfo.csproj`, and adding
`VersionInfo.csproj` to `Java.Interop.BootstrapTasks.sln`.
This allows the `Prepare` target to create
`bin/Build$(Configuration)/Version.props`.

Next, update `Java.Interop.csproj` so that `$(Version)` is
*conditional* on `$(TargetFramework)`.  This allows it to continue
using version 0.1.0.0 for .NET Standard 2.0, and begin using
6.0.0.* for .NET 6+ builds.

Finally -- and most of the "noise" in this commit -- "cleanup and
unify" the disparate build systems.  `make prepare all` is one world,
while `dotnet` is a different and overlapping world.

Clean this up: the `dotnet`-based MSBuild environment is now our
Source Of Truth, and the `make` targets are updated to rely on the
`Prepare` target instead of duplicating most of the logic.

While cleaning up `Makefile`, remove other cruft such as the explicit
`gcc` calls to build the `NativeTiming` lib (we have a `.csproj` as
of 5c756b1; use it!), and rationalize
`Java.Runtime.Environment.dll.config` generation.
jonpryor added a commit to jonpryor/java.interop that referenced this pull request Feb 4, 2022
Context: 2d5431f
Context: 88d6093
Context: dotnet#936
Context: https://github.com/jonpryor/java.interop/commits/jonp-registration-scope

Versioning is hard.

Way back in 3e6a623 we tried to use the `GitInfo` NuGet package so
that *all* assemblies would have a version number which contained the
number of commits since `GitInfo.txt` changed.

This turned out to have unanticipated breakage, and was largely
reverted in 2d5431f for "core" libs like `Java.Interop.dll`.

This still presents a problem, though: the *point* to assembly
versioning is to prevent accidental breaking of referencing
assemblies.  If we add a new member to `Java.Interop.dll` but
*fail to update the version*, then that *permits* a scenario in which
an app/lib depends on the new member, but is built against a version
missing that member.  This result in runtime exceptions.

The whole reason this hasn't been a problem so far is because
`Java.Interop.dll` has barely changed in *years*.  (Plus, most usage
is hidden behind other layers and libs…)

However, *I want this to change*: dotnet#936 and
jonpryor/java.interop/jonp-registration-scope both *add* new public
API to `Java.Interop.dll`, and there are other features and
optimizations we're considering that would also require API changes.
A policy of "no changes" isn't tenable.

Thus, the first priority: allow `Java.Interop.dll` built for .NET 6 to
have a different `$(Version)` than the one built for .NET Standard 2.0.

Fixing this was unexpectedly problematic, as commit 88d6093:

>  Update[d] `Java.Interop.BootstrapTasks.csproj` to *no longer* `<Import/>`
> `VersionInfo.targets`, as this causes a circular dependency
> (`VersionInfo.targets` uses tasks from
> `Java.Interop.BootstrapTasks.dll`).  This is fine, as
> `Java.Interop.BootstrapTasks.dll` doesn't need to be versioned.

`Java.Interop.BootstrapTasks.dll` doesn't need to be versioned, but
*other libraries **do***, and this change change meant that
`VersionInfo.targets` was *never* used, which in turn meant that e.g.
`bin/BuildDebug/Version.props` was never created.

Re-introduce `VersionInfo.targets` by "moving" it into
`build-tools/VersionInfo/VersionInfo.csproj`, and adding
`VersionInfo.csproj` to `Java.Interop.BootstrapTasks.sln`.
This allows the `Prepare` target to create
`bin/Build$(Configuration)/Version.props`.

Next, update `Java.Interop.csproj` so that `$(Version)` is
*conditional* on `$(TargetFramework)`.  This allows it to continue
using version 0.1.0.0 for .NET Standard 2.0, and begin using
6.0.0.* for .NET 6+ builds.

Finally -- and most of the "noise" in this commit -- "cleanup and
unify" the disparate build systems.  `make prepare all` is one world,
while `dotnet` is a different and overlapping world.

Clean this up: the `dotnet`-based MSBuild environment is now our
Source Of Truth, and the `make` targets are updated to rely on the
`Prepare` target instead of duplicating most of the logic.

While cleaning up `Makefile`, remove other cruft such as the explicit
`gcc` calls to build the `NativeTiming` lib (we have a `.csproj` as
of 5c756b1; use it!), and rationalize
`Java.Runtime.Environment.dll.config` generation.
jonpryor added a commit to jonpryor/java.interop that referenced this pull request Feb 10, 2022
Context: 2d5431f
Context: 88d6093
Context: dotnet#936
Context: https://github.com/jonpryor/java.interop/commits/jonp-registration-scope

Versioning is hard.

Way back in 3e6a623 we tried to use the `GitInfo` NuGet package so
that *all* assemblies would have a version number which contained the
number of commits since `GitInfo.txt` changed.

This turned out to have unanticipated breakage, and was largely
reverted in 2d5431f for "core" libs like `Java.Interop.dll`, but
retained for "non-core" apps and libs like `generator.exe`.

This still presents a problem, though: the *point* to assembly
versioning is to prevent accidental breaking of referencing
assemblies.  If we add a new member to `Java.Interop.dll` but
*fail to update the version*, then that *permits* a scenario in which
an app/lib depends on the new member, but is built against a version
missing that member.  This results in runtime exceptions.

The whole reason this hasn't been a problem so far is because
`Java.Interop.dll` has barely changed in *years*.  (Plus, most usage
is hidden behind other layers and libs…)

However, *I want this to change*: dotnet#936 and
jonpryor/java.interop/jonp-registration-scope both *add* new public
API to `Java.Interop.dll`, and there are other features and
optimizations we're considering that would also require API changes.
A policy of "no changes" isn't tenable.

Thus: allow `Java.Interop.dll` built for .NET 6 to have a different
`$(Version)` than the one built for .NET Standard 2.0.

Fixing this was unexpectedly problematic, as commit 88d6093:

>  Update[d] `Java.Interop.BootstrapTasks.csproj` to *no longer*
> `<Import/>` `VersionInfo.targets`, as this causes a circular
> dependency (`VersionInfo.targets` uses tasks from
> `Java.Interop.BootstrapTasks.dll`).  This is fine, as
> `Java.Interop.BootstrapTasks.dll` doesn't need to be versioned.

`Java.Interop.BootstrapTasks.dll` doesn't need to be versioned, but
*other assemblies **do***, and this change change meant that
`VersionInfo.targets` was *never* used, which in turn meant that e.g.
`bin/BuildDebug/Version.props` was never created.

Re-think this process, while avoiding the circular dependency:

 1. `Java.Interop.BootstrapTasks.targets` now *generates* a
    `Version.props` file, re-introducing its existence.
    `Version.props` now contains "two sets" of versions:

      * ".NET Core" versions, based on `GitInfo.txt`, which is
        updated to contain version `6.0`), and
      * "Old" versions, which attempt to be compatible with the
        previous versioning pattern.

    Within those "sets", introduce "utility versions" and
    "core library" versions.

 2. Update/replace `VersionInfo.targets` with a
    `_SetInformationalVersion` target, which sets the
    `$(InformationalVersion)` MSBuild property based on the value
    of `$(Version)` and various other properties.

 3. Update most `.csproj` files, so that they "opt in" to which
    version convention they want: "utility" or "core library".

The result is that for "core libraries" such as `Java.Interop.dll`,
when builing for .NET Standard or MonoAndroid they will continue to
use version 0.1.0.0 (see also 2d5431f), while these assemblies will
use version 6.0.0.0 when built for .NET Core/6.

"Utility" assemblies such as `generator.exe` will contain a version
number which changes based on when `GitInfo.txt` was last changed.
When building for .NET Standard, they will use version
`0.2.$(GitSemVerPatch).$(GitCommits)`, which differs from the
pre-88d6093c pattern of `0.1.$(GitSemVerPatch).0`, but as my current
system Xamarin.Android v12.1.99.117 install shows that `generator.exe`
has version 0.1.31.0, "rebasing" on 0.2.x feels easiest.
When utility assemblies are built for .NET, they will instead use the
values from `GitInfo.txt`, e.g. `6.0.0.$(GitCommits)`.
jonpryor added a commit to jonpryor/java.interop that referenced this pull request Feb 10, 2022
Context: 2d5431f
Context: 88d6093
Context: dotnet#936
Context: https://github.com/jonpryor/java.interop/commits/jonp-registration-scope

Versioning is hard.

Way back in 3e6a623 we tried to use the `GitInfo` NuGet package so
that *all* assemblies would have a version number which contained the
number of commits since `GitInfo.txt` changed.

This turned out to have unanticipated breakage, and was largely
reverted in 2d5431f for "core" libs like `Java.Interop.dll`, but
retained for "utility" apps and libs like `generator.exe`.

This still presents a problem, though: the *point* to assembly
versioning is to prevent accidental breaking of referencing
assemblies.  If we add a new member to `Java.Interop.dll` but
*fail to update the version*, then that *permits* a scenario in which
an app/lib depends on the new member, but is built against a version
missing that member.  This results in runtime exceptions.

The whole reason this hasn't been a problem so far is because
`Java.Interop.dll` has barely changed in *years*.  (Plus, most usage
is hidden behind other layers and libs…)

However, *I want this to change*: dotnet#936 and
jonpryor/java.interop/jonp-registration-scope both *add* new public
API to `Java.Interop.dll`, and there are other features and
optimizations we're considering that would also require API changes.
A policy of "no changes" isn't tenable.

Thus: allow `Java.Interop.dll` built for .NET 6 to have a different
`$(Version)` than the one built for .NET Standard 2.0.

Fixing this was unexpectedly problematic, as commit 88d6093:

>  Update[d] `Java.Interop.BootstrapTasks.csproj` to *no longer*
> `<Import/>` `VersionInfo.targets`, as this causes a circular
> dependency (`VersionInfo.targets` uses tasks from
> `Java.Interop.BootstrapTasks.dll`).  This is fine, as
> `Java.Interop.BootstrapTasks.dll` doesn't need to be versioned.

`Java.Interop.BootstrapTasks.dll` doesn't need to be versioned, but
*other assemblies **do***, and this change change meant that
`VersionInfo.targets` was *never* used, which in turn meant that e.g.
`bin/BuildDebug/Version.props` was never created.

This change *also* implicitly reverted all remaining 3e6a623
behavior; in commit 13def0e (one prior to 88d6093), `generator.exe`
*had* a version of 0.1.45.0.  Starting with commit 88d6093,
`generator.exe` had version 1.0.0.0, which re-introduces the
"baseline" scenario which first necessitated 3e6a623!

Re-think this process, while avoiding the circular dependency:

 1. `Java.Interop.BootstrapTasks.targets` now *generates* a
    `Version.props` file, re-introducing its existence.
    `Version.props` now contains *four* version values:

      * `$(JINetToolVersion)`: Version to use for "utility"
        assemblies which target .NET.

      * `$(JINetCoreLibVersion)`: Version to use for "core library"
        assemblies which target .NET.

      * `$(JIOldToolVersion)`: Version to use for "utility"
        assemblies which target .NET Standard/Framework/etc.

      * `$(JIOldCoreLibVersion)`: Version to use for "core library"
        assemblies which target .NET Standard/Framework/etc.

    The `$(JINet*)` values derive their major, minor, and patch
    values from `GitInfo`, while the `$(JIOld*)` values are
    backward compatible (-ish).

 2. Update/replace `VersionInfo.targets` with a
    `_SetInformationalVersion` target, which sets the
    `$(InformationalVersion)` MSBuild property based on the value
    of `$(Version)` and various other properties.

 3. `Directory.Build.props` is updated to provide new
    `$(JIUtilityVersion)` and `$(JICoreLibVersion)` properties, which
    are set based on the `$(TargetFramework)` value.

 4. Set the default `$(Version)` value to `$(JIUtilityVersion)`.
    "Core library" assemblies must explicitly set `$(Version)` in
    their `.csproj` to instead be `$(JICoreLibVersion)`.

The result is that for "core libraries" such as `Java.Interop.dll`,
when building for .NET Standard or MonoAndroid they will continue to
use version 0.1.0.0 (see also 2d5431f), while these assemblies will
use version 6.0.0.0 when built for .NET Core/6.

"Utility" assemblies such as `generator.exe` will contain a version
number which changes based on when `GitInfo.txt` was last changed.
When building for .NET Standard, they will use version
`0.2.$(GitBaseVersionPatch).$(GitCommits)`, which differs from the
pre-88d6093c pattern of `0.1.$(GitSemVerPatch).0`, but as my current
system Xamarin.Android v12.1.99.117 install shows that `generator.exe`
has version 0.1.31.0, "rebasing" on 0.2.x feels easiest.
When utility assemblies are built for .NET, they will instead use the
values from `GitInfo.txt`, e.g. `6.0.0.$(GitCommits)`.

Finally, *always* set `$(FileVersion)` to:

	$(GitBaseVersionMajor).$(GitBaseVersionMinor).$(GitBaseVersionPatch).$(GitCommits)

[`$(FileVersion)`][0] is used to set
[`$(AssemblyFileVersionAttribute)`][1], which is *not* used for
assembly binding, but *is* shown in the Windows File Properties
dialog, and can be used to (indirectly) determine which commit an
assembly was built from.

[0]: https://docs.microsoft.com/en-us/dotnet/core/project-sdk/msbuild-props
[1]: https://docs.microsoft.com/en-us/dotnet/api/system.reflection.assemblyfileversionattribute?view=net-6.0
jonpryor added a commit that referenced this pull request Feb 11, 2022
Context: 2d5431f
Context: 88d6093
Context: #936
Context: https://github.com/jonpryor/java.interop/commits/jonp-registration-scope

Versioning is hard.

Way back in 3e6a623 we tried to use the `GitInfo` NuGet package so
that *all* assemblies would have a version number which contained the
number of commits since `GitInfo.txt` changed.

This turned out to have unanticipated breakage, and was largely
reverted in 2d5431f for "core" libs like `Java.Interop.dll`, but
retained for "utility" apps and libs like `generator.exe`.

This still presents a problem, though: the *point* to assembly
versioning is to prevent accidental breaking of referencing
assemblies.  If we add a new member to `Java.Interop.dll` but
*fail to update the version*, then that *permits* a scenario in which
an app/lib depends on the new member, but is built against a version
missing that member.  This results in runtime exceptions.

The whole reason this hasn't been a problem so far is because
`Java.Interop.dll` has barely changed in *years*.  (Plus, most usage
is hidden behind other layers and libs…)

However, *I want this to change*: #936 and
jonpryor/java.interop/jonp-registration-scope both *add* new public
API to `Java.Interop.dll`, and there are other features and
optimizations we're considering that would also require API changes.
A policy of "no changes" isn't tenable.

Thus: allow `Java.Interop.dll` built for .NET 6 to have a different
`$(Version)` than the one built for .NET Standard 2.0.

Fixing this was unexpectedly problematic, as commit 88d6093:

>  Update[d] `Java.Interop.BootstrapTasks.csproj` to *no longer*
> `<Import/>` `VersionInfo.targets`, as this causes a circular
> dependency (`VersionInfo.targets` uses tasks from
> `Java.Interop.BootstrapTasks.dll`).  This is fine, as
> `Java.Interop.BootstrapTasks.dll` doesn't need to be versioned.

`Java.Interop.BootstrapTasks.dll` doesn't need to be versioned, but
*other assemblies **do***, and this change change meant that
`VersionInfo.targets` was *never* used, which in turn meant that e.g.
`bin/BuildDebug/Version.props` was never created.

This change *also* implicitly reverted all remaining 3e6a623
behavior; in commit 13def0e (one prior to 88d6093), `generator.exe`
*had* a version of 0.1.45.0.  Starting with commit 88d6093,
`generator.exe` had version 1.0.0.0, which re-introduces the
"baseline" scenario which first necessitated 3e6a623!

Re-think this process, while avoiding the circular dependency:

 1. `Java.Interop.BootstrapTasks.targets` now *generates* a
    `Version.props` file, re-introducing its existence.
    `Version.props` now contains *four* version values:

      * `$(JINetToolVersion)`: Version to use for "utility"
        assemblies which target .NET.

      * `$(JINetCoreLibVersion)`: Version to use for "core library"
        assemblies which target .NET.

      * `$(JIOldToolVersion)`: Version to use for "utility"
        assemblies which target .NET Standard/Framework/etc.

      * `$(JIOldCoreLibVersion)`: Version to use for "core library"
        assemblies which target .NET Standard/Framework/etc.

    The `$(JINet*)` values derive their major, minor, and patch
    values from `GitInfo`, while the `$(JIOld*)` values are
    backward compatible (-ish).

 2. Update/replace `VersionInfo.targets` with a
    `_SetInformationalVersion` target, which sets the
    `$(InformationalVersion)` MSBuild property based on the value
    of `$(Version)` and various other properties.

 3. `Directory.Build.props` is updated to provide new
    `$(JIUtilityVersion)` and `$(JICoreLibVersion)` properties, which
    are set based on the `$(TargetFramework)` value.

 4. Set the default `$(Version)` value to `$(JIUtilityVersion)`.
    "Core library" assemblies must explicitly set `$(Version)` in
    their `.csproj` to instead be `$(JICoreLibVersion)`.

The result is that for "core libraries" such as `Java.Interop.dll`,
when building for .NET Standard or MonoAndroid they will continue to
use version 0.1.0.0 (see also 2d5431f), while these assemblies will
use version 6.0.0.0 when built for .NET Core/6.

"Utility" assemblies such as `generator.exe` will contain a version
number which changes based on when `GitInfo.txt` was last changed.
When building for .NET Standard, they will use version
`0.2.$(GitBaseVersionPatch).$(GitCommits)`, which differs from the
pre-88d6093c pattern of `0.1.$(GitSemVerPatch).0`, but as my current
system Xamarin.Android v12.1.99.117 install shows that `generator.exe`
has version 0.1.31.0, "rebasing" on 0.2.x feels easiest.
When utility assemblies are built for .NET, they will instead use the
values from `GitInfo.txt`, e.g. `6.0.0.$(GitCommits)`.

Finally, *always* set `$(FileVersion)` to:

	$(GitBaseVersionMajor).$(GitBaseVersionMinor).$(GitBaseVersionPatch).$(GitCommits)

[`$(FileVersion)`][0] is used to set
[`$(AssemblyFileVersionAttribute)`][1], which is *not* used for
assembly binding, but *is* shown in the Windows File Properties
dialog, and can be used to (indirectly) determine which commit an
assembly was built from.

[0]: https://docs.microsoft.com/en-us/dotnet/core/project-sdk/msbuild-props
[1]: https://docs.microsoft.com/en-us/dotnet/api/system.reflection.assemblyfileversionattribute?view=net-6.0
@jonpryor jonpryor requested a review from jpobst May 5, 2022 19:28
jonpryor added a commit to dotnet/android that referenced this pull request May 5, 2022
Context: dotnet/java-interop#867
Context: dotnet/java-interop#936

Does It Build?

Does It Work?

(Is It Sane?)

For local "test" purposes, add a new `tools/remap-mam-json-to-xml`
utility which parses the MAM JSON file into XML.

	$ dotnet run --project tools/remap-mam-json-to-xml  -- \
	  $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json
	<replacements>
	  <replace-type from="android/app/Activity" to="com/microsoft/intune/mam/client/app/MAMActivity" />
	  …
	  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onStateNotSaved" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMStateNotSaved" target-method-instance-to-static="false" />
	</replacements>

dotnet bin/Debug/net6.0/remap-mam-json-to-xml.dll $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json

Add a new `@(_AndroidMamMappingFile)` item group which can be
used to produce the mapping XML file.

TODO: why is `$(_AndroidMamMappingXml)` e.g.
`obj/Debug/assets/obj/Debug/assets/xa-mam-mapping.xml`?
What's with the duplication?

Update src/java-runtime to use `javac -h`, to emit JNI headers.
(`javah` is not present in JDK11!)

Update src/monodroid to depend on src/java-runtime, then use the
headers generated by `javac -h` to ensure consistency.
Rando aside: apparently `createNewContextWithData` was inconsistent?)
jonpryor added a commit to dotnet/android that referenced this pull request May 5, 2022
Context: dotnet/java-interop#867
Context: dotnet/java-interop#936

Does It Build?

Does It Work?

(Is It Sane?)

For local "test" purposes, add a new `tools/remap-mam-json-to-xml`
utility which parses the MAM JSON file into XML.

	$ dotnet run --project tools/remap-mam-json-to-xml  -- \
	  $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json
	<replacements>
	  <replace-type from="android/app/Activity" to="com/microsoft/intune/mam/client/app/MAMActivity" />
	  …
	  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onStateNotSaved" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMStateNotSaved" target-method-instance-to-static="false" />
	</replacements>

dotnet bin/Debug/net6.0/remap-mam-json-to-xml.dll $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json

Add a new `@(_AndroidMamMappingFile)` item group which can be
used to produce the mapping XML file.

TODO: why is `$(_AndroidMamMappingXml)` e.g.
`obj/Debug/assets/obj/Debug/assets/xa-mam-mapping.xml`?
What's with the duplication?

Update src/java-runtime to use `javac -h`, to emit JNI headers.
(`javah` is not present in JDK11!)

Update src/monodroid to depend on src/java-runtime, then use the
headers generated by `javac -h` to ensure consistency.
Rando aside: apparently `createNewContextWithData` was inconsistent?)
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.

@jonpryor jonpryor force-pushed the jonp-member-remapping branch from ec8791f to 7699359 Compare May 6, 2022 15:06
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request May 6, 2022
Context: dotnet/java-interop#867
Context: dotnet/java-interop#936

Does It Build?

Does It Work?

(Is It Sane?)

For local "test" purposes, add a new `tools/remap-mam-json-to-xml`
utility which parses the MAM JSON file into XML.

	$ dotnet run --project tools/remap-mam-json-to-xml  -- \
	  $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json
	<replacements>
	  <replace-type from="android/app/Activity" to="com/microsoft/intune/mam/client/app/MAMActivity" />
	  …
	  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onStateNotSaved" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMStateNotSaved" target-method-instance-to-static="false" />
	</replacements>

dotnet bin/Debug/net6.0/remap-mam-json-to-xml.dll $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json

Add a new `@(_AndroidMamMappingFile)` item group which can be
used to produce the mapping XML file.

TODO: why is `$(_AndroidMamMappingXml)` e.g.
`obj/Debug/assets/obj/Debug/assets/xa-mam-mapping.xml`?
What's with the duplication?

Update src/java-runtime to use `javac -h`, to emit JNI headers.
(`javah` is not present in JDK11!)

Update src/monodroid to depend on src/java-runtime, then use the
headers generated by `javac -h` to ensure consistency.
Rando aside: apparently `createNewContextWithData` was inconsistent?)
@jonpryor
Copy link
Contributor Author

jonpryor commented May 6, 2022

@jonathanpeppers asked:

Is there an example of what MAMActivity would look like?

With some digging, I found the .class file!

  1. Microsoft.Intune.MAM.Xamarin.Android NuGet

  2. Extract lib/MonoAndroid/Microsoft.Intune.Android.dll

    unzip microsoft.intune.mam.xamarin.android.3.0.4635.1.nupkg lib/MonoAndroid/Microsoft.Intune.Android.dll
    
  3. Extract resources from Microsoft.Intune.Android.dll

    monodis --mresources lib/MonoAndroid/Microsoft.Intune.Android.dll
    
  4. Extract classes.jar from __AndroidLibraryProjects__:

    unzip __AndroidLibraryProjects__.zip library_project_imports/bin/classes.jar
    

classes.jar contains MAMActivity:

% javap -cp library_project_imports/bin/classes.jar com/microsoft/intune/mam/client/app/MAMActivity
Compiled from "MAMActivity.java"
public abstract class com.microsoft.intune.mam.client.app.MAMActivity extends android.app.Activity implements com.microsoft.intune.mam.client.app.Hooked
Activity {
  com.microsoft.intune.mam.client.app.ActivityBehavior mBehavior;
  public com.microsoft.intune.mam.client.app.MAMActivity();
  protected void attachBaseContext(android.content.Context);
…
  protected final void onCreate(android.os.Bundle);
  public void onMAMCreate(android.os.Bundle);
…
  protected final void onDestroy();
  public void onMAMDestroy();
  public final void onDestroyReal();
…
  protected final void onActivityResult(int, int, android.content.Intent);
  public void onMAMActivityResult(int, int, android.content.Intent);
…
}

The Microsoft.Intune.MAM.Remapper.Tasks NuGet package contains the remapping information, in content/MonoAndroid10/remapping-config.json. You can use tools/remap-mam-json-to-xml in dotnet/android#6591 to "dump" it to something more meaningful in the context of this PR:

dotnet run --project tools/remap-mam-json-to-xml \
  ~/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json \
  > renames.xml
  <replace-type from="android/app/Activity" to="com/microsoft/intune/mam/client/app/MAMActivity" />
…
  <replace-method source-type="com/microsoft/intune/mam/client/app/MAMActivity" source-method-name="onActivityResult" source-method-signature="(IILandroid/content/Intent;)" target-type="com/microsoft/intune/mam/client/app/MAMActivity" target-method-name="onMAMActivityResult" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/app/MAMActivity" source-method-name="onCreate" source-method-signature="(Landroid/os/Bundle;)" target-type="com/microsoft/intune/mam/client/app/MAMActivity" target-method-name="onMAMCreate" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/app/MAMActivity" source-method-name="onDestroy" target-type="com/microsoft/intune/mam/client/app/MAMActivity" target-method-name="onMAMDestroy" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/app/MAMActivity" source-method-name="onNewIntent" source-method-signature="(Landroid/content/Intent;)" target-type="com/microsoft/intune/mam/client/app/MAMActivity" target-method-name="onMAMNewIntent" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/app/MAMActivity" source-method-name="onPause" target-type="com/microsoft/intune/mam/client/app/MAMActivity" target-method-name="onMAMPause" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/app/MAMActivity" source-method-name="onPostCreate" source-method-signature="(Landroid/os/Bundle;)" target-type="com/microsoft/intune/mam/client/app/MAMActivity" target-method-name="onMAMPostCreate" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/app/MAMActivity" source-method-name="onPostResume" target-type="com/microsoft/intune/mam/client/app/MAMActivity" target-method-name="onMAMPostResume" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/app/MAMActivity" source-method-name="onPrepareOptionsMenu" source-method-signature="(Landroid/view/Menu;)" target-type="com/microsoft/intune/mam/client/app/MAMActivity" target-method-name="onMAMPrepareOptionsMenu" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/app/MAMActivity" source-method-name="onProvideAssistContent" source-method-signature="(Landroid/app/assist/AssistContent;)" target-type="com/microsoft/intune/mam/client/app/MAMActivity" target-method-name="onMAMProvideAssistContent" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/app/MAMActivity" source-method-name="onProvideReferrer" target-type="com/microsoft/intune/mam/client/app/MAMActivity" target-method-name="onMAMProvideReferrer" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/app/MAMActivity" source-method-name="onResume" target-type="com/microsoft/intune/mam/client/app/MAMActivity" target-method-name="onMAMResume" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/app/MAMActivity" source-method-name="onSaveInstanceState" source-method-signature="(Landroid/os/Bundle;)" target-type="com/microsoft/intune/mam/client/app/MAMActivity" target-method-name="onMAMSaveInstanceState" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/app/MAMActivity" source-method-name="onSearchRequested" source-method-signature="(Landroid/view/SearchEvent;)" target-type="com/microsoft/intune/mam/client/app/MAMActivity" target-method-name="onMAMSearchRequested" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/app/MAMActivity" source-method-name="onStateNotSaved" target-type="com/microsoft/intune/mam/client/app/MAMActivity" target-method-name="onMAMStateNotSaved" target-method-instance-to-static="false" />

@jonpryor
Copy link
Contributor Author

jonpryor commented May 6, 2022

@jonathanpeppers asked:

I don't understand how they would make their library fixup other activity subclasses like MAUI:

I'm not sure I see how that matters? MauiAppCompatActivity inherits AndroidX.AppCompat.App.AppCompatActivity, which is remapped:

  <replace-type from="android/support/v7/app/AppCompatActivity" to="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" />

When AppCompatActivity.OnCreate() is invoked via C#, we'll consult the member remapping "table":

  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onActivityResult" source-method-signature="(IILandroid/content/Intent;)" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMActivityResult" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onCreate" source-method-signature="(Landroid/os/Bundle;)" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMCreate" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onDestroy" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMDestroy" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onNewIntent" source-method-signature="(Landroid/content/Intent;)" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMNewIntent" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onPause" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMPause" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onPostCreate" source-method-signature="(Landroid/os/Bundle;)" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMPostCreate" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onPostResume" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMPostResume" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onPrepareOptionsMenu" source-method-signature="(Landroid/view/Menu;)" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMPrepareOptionsMenu" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onProvideAssistContent" source-method-signature="(Landroid/app/assist/AssistContent;)" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMProvideAssistContent" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onProvideReferrer" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMProvideReferrer" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onResume" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMResume" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onSaveInstanceState" source-method-signature="(Landroid/os/Bundle;)" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMSaveInstanceState" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onSearchRequested" source-method-signature="(Landroid/view/SearchEvent;)" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMSearchRequested" target-method-instance-to-static="false" />
  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onStateNotSaved" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMStateNotSaved" target-method-instance-to-static="false" />

and remap onCreate() to onMAMCreate().

@jonathanpeppers
Copy link
Member

So this XML above is going to happen at runtime instead of build time.

I'm not seeing an example of what the new MAM library would actually do.

What does this look like:

<replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onCreate" source-method-signature="(Landroid/os/Bundle;)" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMCreate" target-method-instance-to-static="false" />

When you do it in C# instead?

Also, where do they run this code? Is there a place they can do it on startup? If most apps have an Activity as an entry point, that sounds tricky...

@jonpryor
Copy link
Contributor Author

jonpryor commented May 6, 2022

@jonathanpeppers said:

So this XML above is going to happen at runtime instead of build time.

I need to write up the PR description for dotnet/android#6591, don't I. :-)

What do you mean by "this XML above is going to happen"? I'll just need to guess which verb you meant…

Please turn your attention to this PR description:

JniRuntime.JniTypeManager.GetReplacementType() allows "replacing"
the jniPeerTypeName value with a "replacement" JNI type name.

JniRuntime.JniTypeManager.GetReplacementMethodInfo() is the key
integration for all other "replacement" semantics. Given the
source type, source method name, and source JNI signature, it is
responsible for providing an overriding target type, target method
name, and target JNI signature.

For ClassRewrites[0].Methods[0], this allows "renaming"
Activity.onCreate() to MAMActivity.onMAMCreate():

	var r = JniEnvironment.Runtime.TypeManager.GetReplacementMethodInfo (
	    "android/app/Activity",
	    "onCreate",
	    "(Landroid/os/Bundle;)V"
	);
	// is equivalent to:
	var r = new JniRuntime.ReplacementMethodInfo {
	    SourceJniType                   = "android/app/Activity",   // from input parameter
	    SourceJniMethodName             = "onCreate",               // from input parameter
	    SourceJniMethodSignature        = "(Landroid/os/Bundle;)V", // from input parameter

	    TargetJniType                   = "com/microsoft/intune/mam/client/app/MAMActivity",
	    TargetJniMethodName             = "onMAMCreate",
	    SourceJniMethodSignature        = null,                     // "use original value"
	    TargetJniMethodParameterCount   = null,                     // unchanged
	    TargetJniMethodIsStatic         = false,
	}

TypeManager.GetReplacementMethodInfo() thus allows JniPeerMembers to remap Activity.onCreate(Bundle) to MAMActivity.onMAMCreate(Bundle).

Not mentioned here is how TypeManager gets that remapping information. That is the purview of dotnet/android#6591, which takes the content/MonoAndroid10/remapping-config.json file in the Microsoft.Intune.MAM.Remapper.Tasks NuGet package and converts it into a "form" usable by AndroidTypeManager.

The specifics of that form "doesn't matter" here, just that it exists. That form also needs to change, for performance reasons; we don't really want to parse XML during on-device app startup! That's just what I could very easily prototype.

Which brings us to the above XML: it's a "translation" of the original JSON data into something that can be used to implement TypeManager.GetReplacementMethodInfo(). If you "squint" just right, this element:

<replace-method
    source-type="com/microsoft/intune/mam/client/app/MAMActivity"
    source-method-name="onActivityResult"
    source-method-signature="(IILandroid/content/Intent;)"
    target-type="com/microsoft/intune/mam/client/app/MAMActivity"
    target-method-name="onMAMActivityResult"
    target-method-instance-to-static="false"
/>

can be read as saying "given this TypeManager.GetReplacementMethodInfo() call":

var r = JniEnvironment.Runtime.TypeManager.GetReplacementMethodInfo (
    "com/microsoft/intune/mam/client/app/MAMActivity",
    "onActivityResult",
    "(IILandroid/content/Intent;)"
);

"then the data returned will be":

var r = new JniRuntime.ReplacementMethodInfo {
    SourceJniType                   = "com/microsoft/intune/mam/client/app/MAMActivity",   // from input parameter
    SourceJniMethodName             = "onActivityResult",               // from input parameter
    SourceJniMethodSignature        = "(IILandroid/content/Intent;)", // from input parameter
    TargetJniType                   = "com/microsoft/intune/mam/client/app/MAMActivity",
    TargetJniMethodName             = "onMAMActivityResult",
    SourceJniMethodSignature        = null,                     // "use original value"
    TargetJniMethodParameterCount   = null,                     // unchanged
    TargetJniMethodIsStatic         = false,
}

API-wise, XML isn't required. It's used as an implementation detail, which will need to die in a fire before .NET 7 GA. (Hence the use of tab-delimited data as the latest commit on dotnet/android#6591, in the hopes that TSV will allow us to do "typemap-style" string tables within libxamarin-app.so.)

Additionally, and this wasn't very clear, while the ability to remap types & members will exist in all apps, the underlying data will only exist if the (undocumented!) @(_AndroidMamMappingFile) item group is used. Meaning 99.999% of the time, no remapping information will be present, and these should all be no-ops (with virtual dispatch overhead 😔, but it should only be on first member lookup, so I think that'll be "fine"?).

Also, where do they run this code?

What does "this code" refer to? XML parsing? That's currently at app startup, if the XML blob exists in the app, and I want to turn that into constant data within libxamarin-app.so by .NET 7 GA.

If most apps have an Activity as an entry point, that sounds tricky...

Indeed, and that trickiness is also partially up to the InTune team. This isn't a "complete" "user story"; it's part of one. The other part is on the InTune team providing the appropriate JSON data, the appropriate .jar file, their Java bytecode rewriter…

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.

I don't think I have anything else to add on this PR, but I can comment further when the xamarin-android changes are final. There might be some general build-performance things I want to read through and understand there.

jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request May 10, 2022
Context: dotnet/java-interop#867
Context: dotnet/java-interop#936

Does It Build?

Does It Work?

(Is It Sane?)

For local "test" purposes, add a new `tools/remap-mam-json-to-xml`
utility which parses the MAM JSON file into XML.

	$ dotnet run --project tools/remap-mam-json-to-xml  -- \
	  $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json
	<replacements>
	  <replace-type from="android/app/Activity" to="com/microsoft/intune/mam/client/app/MAMActivity" />
	  …
	  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onStateNotSaved" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMStateNotSaved" target-method-instance-to-static="false" />
	</replacements>

dotnet bin/Debug/net6.0/remap-mam-json-to-xml.dll $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json

Add a new `@(_AndroidMamMappingFile)` item group which can be
used to produce the mapping XML file.

TODO: why is `$(_AndroidMamMappingXml)` e.g.
`obj/Debug/assets/obj/Debug/assets/xa-mam-mapping.xml`?
What's with the duplication?

Update src/java-runtime to use `javac -h`, to emit JNI headers.
(`javah` is not present in JDK11!)

Update src/monodroid to depend on src/java-runtime, then use the
headers generated by `javac -h` to ensure consistency.
Rando aside: apparently `createNewContextWithData` was inconsistent?)
@jonpryor jonpryor force-pushed the jonp-member-remapping branch from 7699359 to 19ebd47 Compare May 12, 2022 23:58
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request May 13, 2022
Context: dotnet/java-interop#867
Context: dotnet/java-interop#936

Does It Build?

Does It Work?

(Is It Sane?)

For local "test" purposes, add a new `tools/remap-mam-json-to-xml`
utility which parses the MAM JSON file into XML.

	$ dotnet run --project tools/remap-mam-json-to-xml  -- \
	  $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json
	<replacements>
	  <replace-type from="android/app/Activity" to="com/microsoft/intune/mam/client/app/MAMActivity" />
	  …
	  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onStateNotSaved" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMStateNotSaved" target-method-instance-to-static="false" />
	</replacements>

dotnet bin/Debug/net6.0/remap-mam-json-to-xml.dll $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json

Add a new `@(_AndroidMamMappingFile)` item group which can be
used to produce the mapping XML file.

TODO: why is `$(_AndroidMamMappingXml)` e.g.
`obj/Debug/assets/obj/Debug/assets/xa-mam-mapping.xml`?
What's with the duplication?

Update src/java-runtime to use `javac -h`, to emit JNI headers.
(`javah` is not present in JDK11!)

Update src/monodroid to depend on src/java-runtime, then use the
headers generated by `javac -h` to ensure consistency.
Rando aside: apparently `createNewContextWithData` was inconsistent?)
@jonpryor jonpryor force-pushed the jonp-member-remapping branch 2 times, most recently from dc95a02 to bce6918 Compare May 16, 2022 19:40
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request May 16, 2022
Context: dotnet/java-interop#867
Context: dotnet/java-interop#936

Does It Build?

Does It Work?

(Is It Sane?)

For local "test" purposes, add a new `tools/remap-mam-json-to-xml`
utility which parses the MAM JSON file into XML.

	$ dotnet run --project tools/remap-mam-json-to-xml  -- \
	  $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json
	<replacements>
	  <replace-type from="android/app/Activity" to="com/microsoft/intune/mam/client/app/MAMActivity" />
	  …
	  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onStateNotSaved" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMStateNotSaved" target-method-instance-to-static="false" />
	</replacements>

dotnet bin/Debug/net6.0/remap-mam-json-to-xml.dll $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json

Add a new `@(_AndroidMamMappingFile)` item group which can be
used to produce the mapping XML file.

TODO: why is `$(_AndroidMamMappingXml)` e.g.
`obj/Debug/assets/obj/Debug/assets/xa-mam-mapping.xml`?
What's with the duplication?

Update src/java-runtime to use `javac -h`, to emit JNI headers.
(`javah` is not present in JDK11!)

Update src/monodroid to depend on src/java-runtime, then use the
headers generated by `javac -h` to ensure consistency.
Rando aside: apparently `createNewContextWithData` was inconsistent?)
@dellis1972
Copy link
Contributor

In the PR description I think these lines are using the wrong parameter

SourceJniMethodSignature        = null,                     // "use original value"

Shouldn't this be TargetJniMethodSignature ?

I ask because if you look at the following example code

var r = new JniRuntime.ReplacementMethodInfo {
    SourceJniType                   = "com/microsoft/intune/mam/client/app/MAMActivity",   // from input parameter
    SourceJniMethodName             = "onActivityResult",               // from input parameter
    SourceJniMethodSignature        = "(IILandroid/content/Intent;)", // from input parameter
    TargetJniType                   = "com/microsoft/intune/mam/client/app/MAMActivity",
    TargetJniMethodName             = "onMAMActivityResult",
    SourceJniMethodSignature        = null,                     // "use original value"
    TargetJniMethodParameterCount   = null,                     // unchanged
    TargetJniMethodIsStatic         = false,
}

SourceJniMethodSignature is being set twice but TargetJniMethodSignature is never set.

jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request May 18, 2022
Context: dotnet/java-interop#867
Context: dotnet/java-interop#936

Does It Build?

Does It Work?

(Is It Sane?)

For local "test" purposes, add a new `tools/remap-mam-json-to-xml`
utility which parses the MAM JSON file into XML.

	$ dotnet run --project tools/remap-mam-json-to-xml  -- \
	  $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json
	<replacements>
	  <replace-type from="android/app/Activity" to="com/microsoft/intune/mam/client/app/MAMActivity" />
	  …
	  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onStateNotSaved" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMStateNotSaved" target-method-instance-to-static="false" />
	</replacements>

dotnet bin/Debug/net6.0/remap-mam-json-to-xml.dll $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json

Add a new `@(_AndroidMamMappingFile)` item group which can be
used to produce the mapping XML file.

TODO: why is `$(_AndroidMamMappingXml)` e.g.
`obj/Debug/assets/obj/Debug/assets/xa-mam-mapping.xml`?
What's with the duplication?

Update src/java-runtime to use `javac -h`, to emit JNI headers.
(`javah` is not present in JDK11!)

Update src/monodroid to depend on src/java-runtime, then use the
headers generated by `javac -h` to ensure consistency.
Rando aside: apparently `createNewContextWithData` was inconsistent?)
@jonpryor jonpryor force-pushed the jonp-member-remapping branch 2 times, most recently from 2d091c6 to 18acd25 Compare May 19, 2022 00:26
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request May 19, 2022
Context: dotnet/java-interop#867
Context: dotnet/java-interop#936

Does It Build?

Does It Work?

(Is It Sane?)

For local "test" purposes, add a new `tools/remap-mam-json-to-xml`
utility which parses the MAM JSON file into XML.

	$ dotnet run --project tools/remap-mam-json-to-xml  -- \
	  $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json
	<replacements>
	  <replace-type from="android/app/Activity" to="com/microsoft/intune/mam/client/app/MAMActivity" />
	  …
	  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onStateNotSaved" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMStateNotSaved" target-method-instance-to-static="false" />
	</replacements>

dotnet bin/Debug/net6.0/remap-mam-json-to-xml.dll $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json

Add a new `@(_AndroidMamMappingFile)` item group which can be
used to produce the mapping XML file.

TODO: why is `$(_AndroidMamMappingXml)` e.g.
`obj/Debug/assets/obj/Debug/assets/xa-mam-mapping.xml`?
What's with the duplication?

Update src/java-runtime to use `javac -h`, to emit JNI headers.
(`javah` is not present in JDK11!)

Update src/monodroid to depend on src/java-runtime, then use the
headers generated by `javac -h` to ensure consistency.
Rando aside: apparently `createNewContextWithData` was inconsistent?)
@jonpryor
Copy link
Contributor Author

Need to update commit message about removal of Debug.Assert(), and replacement w/ Console.WriteLine().

The reason was this:

ManagedPeerType <=> JniTypeName Mismatch! javaVM.GetJniTypeInfoForType(typeof(Android.Runtime.JavaList)).JniTypeName="java/util/ArrayList" != "java/util/List"

That's not easily fixable within xamarin-android right now, and with .NET 6 making Debug.Assert() an "abort the process" affair, changing this to a "runtime warning + continue execution" seems like the better idea.

jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request May 19, 2022
Context: dotnet/java-interop#867
Context: dotnet/java-interop#936

Does It Build?

Does It Work?

(Is It Sane?)

For local "test" purposes, add a new `tools/remap-mam-json-to-xml`
utility which parses the MAM JSON file into XML.

	$ dotnet run --project tools/remap-mam-json-to-xml  -- \
	  $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json
	<replacements>
	  <replace-type from="android/app/Activity" to="com/microsoft/intune/mam/client/app/MAMActivity" />
	  …
	  <replace-method source-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" source-method-name="onStateNotSaved" target-type="com/microsoft/intune/mam/client/support/v7/app/MAMAppCompatActivity" target-method-name="onMAMStateNotSaved" target-method-instance-to-static="false" />
	</replacements>

dotnet bin/Debug/net6.0/remap-mam-json-to-xml.dll $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json

Add a new `@(_AndroidMamMappingFile)` item group which can be
used to produce the mapping XML file.

TODO: why is `$(_AndroidMamMappingXml)` e.g.
`obj/Debug/assets/obj/Debug/assets/xa-mam-mapping.xml`?
What's with the duplication?

Update src/java-runtime to use `javac -h`, to emit JNI headers.
(`javah` is not present in JDK11!)

Update src/monodroid to depend on src/java-runtime, then use the
headers generated by `javac -h` to ensure consistency.
Rando aside: apparently `createNewContextWithData` was inconsistent?)
Context: dotnet#867

There are two scenarios for which a "generalized" type & member
remapping solution would be useful:

 1. Desugaring
 2. Microsoft Intune MAM

Note: this new support is only present when targeting .NET 6+,
and will not (currently) be present in Classic Xamarin.Android.

~~ Desugaring ~~

Context: dotnet/android#4574 (comment)
Context: dotnet/android#6142 (comment)

The [Android Runtime][0] is responsible for running the Dalvik
bytecode within e.g. `classes.dex`.  The Android runtime and Dalvik
bytecode formats have apparently changed over time in order to
support newer Java language features, such as support for static
interface methods:

	package org.webrtc;

	public /* partial */ interface EGLBase {
	    public static EGLBase create() { /* … */}
	}

Support for static interface methods was added in Java 8 and in
Android v8.0 (API-26).  If you want to use static interface methods
on an Android device running API-25 or earlier, you must [desugar][1]
the Java Bytecode.  The result of [desugaring][2] the above
`org.webrtc.EGLBase` type is that the static methods are moved to a
*new* type, with a `$-CC` suffix, "as if" the Java code were instead:

	package org.webrtc;

	public /* partial */ interface EGLBase {
	    // …
	}

	public final /* partial */ class EGLBase$-CC {
	    public static EGLBase create { /* … */ }
	}

While this works for pure Java code, this *does not work* with
Xamarin.Android, because our bindings currently expect (require) that
the types & members our binding process detects don't "move":

	// C# Binding for EGLBase
	namespace Org.Webrtc {
	    public partial interface IEGLBase {
	        private static readonly JniPeerMembers _members = new JniPeerMembers ("org/webrtc/EGLBase", typeof (IEGLBase));
	        public static IEGLBase Create()
	        {
	            const string __id = "create.()Lorg/webrtc/EglBase;"
	            var __rm = _members.StaticMethods.InvokeObjectMethod (__id, null);
	            return Java.Lang.Object.GetObject<IEGLBase>(__rm.Handle, JniHandleOwnership.TransferLocalRef);
	        }
	    }
	}

The `new JniPeerMembers(…)` invocation will use `JNIEnv::FindClass()`
to lookup the `org/webrtc/EGLBase` type, and `IEGLBase.Create()` will
use `JNIEnv::GetStaticMethodID()` and `JNIEnv::CallStaticObjectMethod()`
to lookup and invoke the `EGLBase.create()` method.

However, when desugaring is used, *there is no* `EGLBase.create()`
method.  Consequently, in a "desugared" environment,
`Java.Lang.NoSuchMethodError` will be thrown when `IEGLBase.Create()`
is invoked, as `EGLBase.create()` doesn't exist; it "should" instead
be looking for `EGLBase$-CC.create()`!

Introduce partial support for this scenario by adding the new method:

	namespace Java.Interop {
	    public partial class JniRuntime {
	        public partial class JniTypeManager {
	            public IReadOnlyList<string>? GetStaticMethodFallbackTypes (string jniSimpleReference) =>
	                GetStaticMethodFallbackTypesCore (jniSimpleReference);
	            protected virtual IReadOnlyList<string>? GetStaticMethodFallbackTypesCore (string jniSimpleReference) => null;
	        }
	    }
	}

`JniPeerMembers.JniStaticMethods.GetMethodInfo()` will attempt to
lookup the static method, "as normal".  If the method cannot be
found, then `JniRuntime.JniTypeManager.GetStaticMethodFallbackTypes()`
will be called, and we'll attempt to resolve the static method on each
type returned by `GetStaticMethodFallbackTypes()`.
If `GetStaticMethodFallbackTypes()` returns `null` or no fallback type
provides the method, then a `NoSuchMethodError` will be thrown.

TODO: Update xamarin-android to override
`GetStaticMethodFallbackTypes()`, to return
`$"{jniSimpleReference}$-CC"`.

~~ Microsoft Intune MAM ~~

Context: https://www.nuget.org/packages/Microsoft.Intune.MAM.Remapper.Tasks/
Context: https://docs.microsoft.com/en-us/mem/intune/developer/app-sdk-xamarin#remapper

The [Microsoft Intune App SDK Xamarin Bindings][3] is in a
similar-yet-different position: it involves Java & Dalvik Bytecode
manipulation for various security purposes, and in order to make that
work reasonably from Xamarin.Android, they *also* rewrite
Xamarin.Android IL so that it's consistent with the manipulated
Dalvik bytecode.

See `readme.md` and `content/MonoAndroid10/remapping-config.json`
within the [`Microsoft.Intune.MAM.Remapper.Tasks NuGet package`][4]
for some additional details/tidbits such as:

> This task operates on assemblies to replace the base type classes
> which Microsoft Intune requires to implement its SDK (for example,
> Intune requires using a `MAMActivity` in place of `Activity` and
> methods such as `OnMAMCreate` instead of `OnCreate`).

`remapping-config.json` contains JSON fragments such as:

	"ClassRewrites": [
	  {
	    "Class": {
	      "From": "android.app.Activity",
	      "To": "com.microsoft.intune.mam.client.app.MAMActivity"
	    },
	    "Methods": [
	      {
	        "MakeStatic": false,
	        "OriginalName": "onCreate",
	        "NewName": "onMAMCreate",
	        "OriginalParams": [
	          "android.os.Bundle"
	        ]
	      }
	    ]
	  },
	  {
	    "Class": {
	      "From": "android.content.pm.PackageManager",
	      "To": "com.microsoft.intune.mam.client.content.pm.MAMPackageManagement"
	    },
	    "Methods": [
	      {
	        "MakeStatic": true,
	        "OriginalName": "checkPermission",
	      }
	    ]
	  }
	]
	"GlobalMethodCalls": [
	  {
	    "Class": {
	      "From": "android.app.PendingIntent",
	      "To": "com.microsoft.intune.mam.client.app.MAMPendingIntent"
	    },
	    "Methods": [
	      {
	        "MakeStatic": false,
	        "OriginalName": "getActivities"
	      },
	    ]
	  }
	]

While what the InTune team has *works*, it suffers from a number of
"workflow" problems, e.g. incremental builds are problematic (they
might try to rewrite assemblies which were already rewritten), IL
rewriting is *always* "fun", and if we change IL binding patterns,
they'll need to support previous "binding styles" and newer styles.

Introduce partial support for this scenario by adding the following
members to `Java.Interop.JniRuntime.JniTypeManager`:

	namespace Java.Interop {
	    public partial class JniRuntime {

	        public struct ReplacementMethodInfo {
	            public  string? SourceJniType                   {get; set;}
	            public  string? SourceJniMethodName             {get; set;}
	            public  string? SourceJniMethodSignature        {get; set;}
	            public  string? TargetJniType                   {get; set;}
	            public  string? TargetJniMethodName             {get; set;}
	            public  string? TargetJniMethodSignature        {get; set;}
	            public  int?    TargetJniMethodParameterCount   {get; set;}
	            public  bool    TargetJniMethodInstanceToStatic {get; set;}
	        }

	        public partial class JniTypeManager {

	            public string? GetReplacementType (string jniSimpleReference) =>
	                GetReplacementTypeCore (jniSimpleReference);
	            protected virtual string? GetReplacementTypeCore (string jniSimpleReference) => null;

	            public ReplacementMethodInfo? GetReplacementMethodInfo (string jniSimpleReference, string jniMethodName, string jniMethodSignature) =>
	                GetReplacementMethodInfoCore (jniSimpleReference, jniMethodName,jniMethodSignature);
	            protected virtual ReplacementMethodInfo? GetReplacementMethodInfoCore (string jniSimpleReference, string jniMethodName, string jniMethodSignature) => null;
	        }
	    }
	}

These new methods are used by `JniPeerMembers`.

Consider "normal" usage of `JniPeerMembers`, within a binding:

	partial class Activity {
	    static readonly JniPeerMembers _members = new XAPeerMembers (jniPeerTypeName:"android/app/Activity", managedPeerType:typeof (Activity));
	}

`JniRuntime.JniTypeManager.GetReplacementType()` allows "replacing"
the `jniPeerTypeName` value with a "replacement" JNI type name.
The "replacement" type will be used for all field and method lookups.
This allows supporting the `ClassRewrites[0].Class.From` /
`ClassRewrites[0].Class.To` semantics.

`JniRuntime.JniTypeManager.GetReplacementMethodInfo()` is the key
integration for all other "replacement" semantics.  Given the
source type, source method name, and source JNI signature, it is
responsible for providing an *overriding* target type, target method
name, and target JNI signature.

For `ClassRewrites[0].Methods[0]`, this allows "renaming"
`Activity.onCreate()` to `MAMActivity.onMAMCreate()`:

	var r = JniEnvironment.Runtime.TypeManager.GetReplacementMethodInfo (
	    "android/app/Activity",
	    "onCreate",
	    "(Landroid/os/Bundle;)V"
	);
	// is equivalent to:
	var r = new JniRuntime.ReplacementMethodInfo {
	    SourceJniType                   = "android/app/Activity",   // from input parameter
	    SourceJniMethodName             = "onCreate",               // from input parameter
	    SourceJniMethodSignature        = "(Landroid/os/Bundle;)V", // from input parameter

	    TargetJniType                   = "com/microsoft/intune/mam/client/app/MAMActivity",
	    TargetJniMethodName             = "onMAMCreate",
	    SourceJniMethodSignature        = null,                     // "use original value"
	    TargetJniMethodParameterCount   = null,                     // unchanged
	    TargetJniMethodInstanceToStatic = false,
	}

`ClassRewrites[1].Methods[0]` is "weird", in that it has
`MakeStatic: true`.  The semantics for when `MakeStatic: true` exists
is that the "original" method is an *instance* method, and the target
method is a *`static`* method, *and* the `this` instance now becomes
a *parameter* to the method.  This is likewise supported via
`JniRuntime.JniTypeManager.GetReplacementMethodInfo()`, and is
identified by:

 1. `ReplacementMethodInfo.TargetJniMethodParameterCount` being a
    non-`null` value, and

 2. `ReplacementMethodInfo.TargetJniMethodInstanceToStatic` is true.

Thus, `ClassRewrites[1].Methods[0]` is akin to:

	var r = JniEnvironment.Runtime.TypeManager.GetReplacementMethodInfo (
	    "android/content/pm/PackageManager",
	    "checkPermission",
	    "(Ljava/lang/String;Ljava/lang/String;)I"
	);
	// is equivalent to:
	var r = new JniRuntime.ReplacementMethodInfo {
	    SourceJniType                   = "android/content/pm/PackageManager",              // from input parameter
	    SourceJniMethodName             = "checkPermission",                                // from input parameter
	    SourceJniMethodSignature        = "(Ljava/lang/String;Ljava/lang/String;)I",        // from input parameter

	    TargetJniType                   = "com/microsoft/intune/mam/client/content/pm/MAMPackageManagement",
	    TargetJniMethodName             = "checkPermission",
	    SourceJniMethodSignature        = "(Landroid/content/pm/PackageManager;Ljava/lang/String;Ljava/lang/String;)I",
	    TargetJniMethodParameterCount   = 3,
	    TargetJniMethodInstanceToStatic = true,
	}

(Note that the subclass is responsible for providing this data.)

`ReplacementMethodInfo.TargetJniMethodParameterCount` must be the
number of parameters that the target method accepts.  This is needed
so that `JniPeerMembers.JniInstanceMethods.TryInvoke*StaticRedirect()`
can appropriately reallocate the `JniArgumentValue*` array, so that
the `this` parameter can be added to the beginning of the parameters.

~~ Overhead? ~~

All these extra invocations into `JniRuntime.JniTypeManager` imply
additional overhead to invoke a Java method.  However, this is all
done during the *first* time a method is looked up, after which the
`JniMethodInfo` instance is *cached* for a given
(type, method, signature) tuple.

To demonstrate overheads, `samples/Hello` has been updated to accept
a new `-t` value; when provided, it invokes the
`java.lang.Object.hashCode()` method 1 million times and calculates
the average invocation overhead:

	% dotnet run --project samples/Hello -- -t
	Object.hashCode: 1000000 invocations. Total=00:00:00.5196758; Average=0.0005196758ms

If we replace the .NET 6 `samples/Hello/bin/Debug/Java.Interop.dll`
assembly with e.g. `bin/Debug/Java.Interop.dll`, we can compare to
performance *without* these new changes, as the changes are only in
the .NET 6 build:

	% \cp bin/Debug/Java.Interop{.dll,.pdb} samples/Hello/bin/Debug
	% dotnet samples/Hello/bin/Debug/Hello.dll -t
	Object.hashCode: 1000000 invocations. Total=00:00:00.5386676; Average=0.0005386676ms

There is a fair bit of variation in `dotnet Hello.dll -t` output,
but they're all roughly similar.  There doesn't appear to be
significant overhead for this particular test.

[0]: https://en.wikipedia.org/wiki/Android_Runtime
[1]: https://developer.android.com/studio/write/java8-support
[2]: https://developer.android.com/studio/write/java8-support-table
[3]: https://docs.microsoft.com/en-us/mem/intune/developer/app-sdk-xamarin
[4]: https://www.nuget.org/packages/Microsoft.Intune.MAM.Remapper.Tasks/
@jonpryor jonpryor force-pushed the jonp-member-remapping branch from 18acd25 to a7cb222 Compare May 19, 2022 18:37
@jonpryor jonpryor merged commit 1f27ab5 into dotnet:main May 20, 2022
jonpryor added a commit to dotnet/android that referenced this pull request May 20, 2022
Fixes: dotnet/java-interop#867

Context: dotnet/java-interop@1f27ab5
Context: #6142 (comment)
Context: #7020

Changes: dotnet/java-interop@843f3c7...1f27ab5

  * dotnet/java-interop@1f27ab55: [Java.Interop] Type & Member Remapping Support (#936)
  * dotnet/java-interop@02aa54e0: [Java.Interop.Tools.JavaCallableWrappers] marshal method decl types (#987)
  * dotnet/java-interop@e7bacc37: [ci] Update azure-pipelines.yaml to Pin .NET 6.0.202 (#986)
  * dotnet/java-interop@fb94d598: [Java.Interop.Tools.JavaCallableWrappers] Collect overriden methods (#985)
  * dotnet/java-interop@3fcce746: [Java.Interop.{Dynamic,Export}] Nullable Reference Type support (#980)

~~ The Scenarios ~~

Our Java binding infrastructure involves looking up types and methods
via [JNI][0], and assumes that types and methods won't "move" in an
unexpected manner.  Methods which move from a subclass to a
superclass works transparently.  Methods which are moved to an
entirely different class causes us problems.

Case in point: [desugaring][0], which can *move* Java types to places
that our bindings don't expect.  For example, [`Arrays.stream(T[])`][1]
may be moved into a `DesugarArrays` class, or a default interface
method `Example.m()` may be moved into an `Example$-CC` type.
Java.Interop has not supported such changes, resulting in throwing a
`Java.Lang.NoSuchMethodError` on Android versions where methods are
not where we expect.

Additionally, the [InTune Mobile Application Management][3] team
needs a expanded type and member lookup mechanism in order to
simplify how they maintain their product.  Currently they make things
work by rewriting IL, which can be brittle.


~~ Build actions ~~

To improve support for this, dotnet/java-interop#936 introduces
new `virtual` methods into `Java.Interop.JniRuntime.JniTypeManager`
which are called as part of type and member lookup, allowing
`AndroidTypeManager` to participate in the type and member resolution
process.

`AndroidTypeManager` in turn needs to know what types and members can
be remapped, and what they should be remapped to.  Some of these can
be algorithmic, such as pre-pending `Desugar` or appending `$-CC` for
the Desugar case.

The InTune use case involves a table, contained within the
[Microsoft.Intune.MAM.Remapper.Tasks NuGet package][4].

Update `src/Xamarin.Android.Build.Tasks` to add a new
`@(_AndroidRemapMembers)` Build action.  This build action is not
externally supported; it's to help test the feature.  Files with this
build action are XML files which control type and member remapping:

	<replacements>
	  <replace-type
	      from="android/app/Activity"
	      to="com/microsoft/intune/mam/client/app/MAMActivity" />
	  <replace-method
	      source-type="com/microsoft/intune/mam/client/app/MAMActivity"
	      source-method-name="onCreate" source-method-signature="(Landroid/os/Bundle;)V"
	      target-type="com/microsoft/intune/mam/client/app/MAMActivity"
	      target-method-name="onMAMCreate" target-method-instance-to-static="false" />
	</replacements>

`//replacements/replace-method` is structured with each attribute
corresponding to a member on the `JniRuntime.ReplacementMethodInfo`
structure, in dotnet/java-interop@1f27ab55.

  * `//replace-method/@source-type` is
    `JniRuntime.ReplacementMethodInfo.SourceJniType`
  * `//replace-method/@source-method-name` is
    `JniRuntime.ReplacementMethodInfo.SourceJniMethodName`
  * `//replace-method/@source-method-signature` is
    `JniRuntime.ReplacementMethodInfo.SourceJniMethodSignature`
  * `//replace-method/@target-type` is
    `JniRuntime.ReplacementMethodInfo.TargetJniType`
  * `//replace-method/@target-method-name` is
    `JniRuntime.ReplacementMethodInfo.TargetJniMethodName`
  * `//replace-method/@target-method-signature` is
    `JniRuntime.ReplacementMethodInfo.TargetJniMethodSignature`
    This attribute is optional.
  * `//replace-method/@target-method-parameter-count` is
    `JniRuntime.ReplacementMethodInfo.TargetJniMethodParameterCount`.
    This attribute is optional.
  * `//replace-method/@target-method-instance-to-static` is
    `JniRuntime.ReplacementMethodInfo.TargetJniMethodIsStatic`

`@source-type`, `@source-method-name`, and `@source-method-signature`
combined serve as a "key" for looking up the associated `@target-*`
information.

Update `src/Xamarin.Android.Build.Tasks` to add a new
`@(_AndroidMamMappingFile)` Build action.  This build action is not
externally supported; it's to help test the feature.  Files with this
build action are expected to be JSON documents which follow the
current conventions of `remapping-config.json`, within the
`Microsoft.Intune.MAM.Remapper.Tasks` NuGet package.  This build
action is not externally supported; this is currently for testing
purposes.  `@(_AndroidMamMappingFile)` files are processed at build
time into `@(_AndroidRemapMembers)` XML files.

During App builds, all `@(_AndroidRemapMembers)` files are merged
into an `@(AndrodAsset)` named `xa-internal/xa-mam-mapping.xml`.
This asset is opened and provided to `JNIEnv.Initialize()` as part of
native app startup.


~~ Putting it all together ~~

This will only work on .NET 7+.

App project has a `@(_AndroidRemapMembers)` file.  This item is
processed during App build, stored into the `.apk`, and read during
app startup on an Android device.

Given a Java binding such as:

	public partial class Activity {
	    static readonly JniPeerMembers _members = new XAPeerMembers ("android/app/Activity", typeof (Activity));
	}

when the `JniPeerMembers` constructor runs, it will call
`JniEnvironment.Runtime.TypeManager.GetReplacementType("android/app/Activity")`.
If `@(_AndroidRemapMembers)` is based on the InTune
`remapping-config.json` file, then `android/app/Activity` is mapped
to `com/microsoft/intune/mam/client/app/MAMActivity`, and
`JNIEnv::FindClass()` will be told to lookup `MAMActivity`, *not*
`Activity`.

*If `MAMActivity` can't be found*, e.g. you're testing this all out,
the app will ~immediately crash, as `MAMActivity` doesn't exist. 😅

If `MAMActivity` can be found, eventually `Activity.OnCreate()` will
need to be invoked:

	partial class Activity {
	    protected virtual unsafe void OnCreate (Android.OS.Bundle? savedInstanceState)
	    {
	        const string __id = "onCreate.(Landroid/os/Bundle;)V";
	        try {
	            JniArgumentValue* __args = stackalloc JniArgumentValue [1];
	            __args [0] = new JniArgumentValue ((savedInstanceState == null) ? IntPtr.Zero : ((global::Java.Lang.Object) savedInstanceState).Handle);
	            _members.InstanceMethods.InvokeVirtualVoidMethod (__id, this, __args);
	        } finally {
	            global::System.GC.KeepAlive (savedInstanceState);
	        }
	    }
	}

`_members.InstanceMethods.InvokeVirtualVoidMethod()` will internally
make a call similar to:

	var r = JniEnvironment.Runtime.TypeManager.GetReplacementMethodInfo (
	    "com/microsoft/intune/mam/client/app/MAMActivity",
	    "onCreate",
	    "(Landroid/os/Bundle;)V"
	);

The data returned will be equivalent to:

	var r = new JniRuntime.ReplacementMethodInfo {
	    SourceJniType                   = "com/microsoft/intune/mam/client/app/MAMActivity",    // from input parameter
	    SourceJniMethodName             = "onCreate",                                           // from input parameter
	    SourceJniMethodSignature        = "(Landroid/os/Bundle;)V",                             // from input parameter

	    TargetJniType                   = "com/microsoft/intune/mam/client/app/MAMActivity",    // from //replace-method/@target-type
	    TargetJniMethodName             = "onMAMCreate",                                        // from //replace-method/@target-method-name
	    TargetJniMethodSignature        = "(Landroid/os/Bundle;)V",                             // from input parameter, as signature didn't change
	    TargetJniMethodParameterCount   = 1,                                                    // computed based on signature
	    TargetJniMethodIsStatic         = false,                                                // from //replace-method/@target-method-instance-to-static
	}

This will allow `_members.InstanceMethods.InvokeVirtualVoidMethod()`
to instead resolve and invoke `MAMActivity.onMAMCreate()`.


~~ Tools ~~

`tools/remap-mam-json-to-xml` is added, and will process the InTune
JSON file into `@(_AndroidRemapMembers)` XML:

	$ dotnet run --project tools/remap-mam-json-to-xml  -- \
	  $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json
	<replacements>…


~~ Unit Tests ~~

`@(_AndroidRemapMembers)` usage is required by
`Mono.Android.NET-Tests.apk`, as `Java.Interop-Tests.dll` exercises
the type and member remapping logic.


~~ Unrelated `gref+` logging fixes ~~

When `debug.mono.log` = `gref+`, the app could crash:

	signal 11 (SIGSEGV), code 2 (SEGV_ACCERR), fault addr 0x79c045dead

This was likely because a constant string was provided to
`OSBridge::_write_stack_trace()`, which tried to write into the
constant string, promptly blowing things up.

Workaround: don't use `gref+` logging when a GC occurs?
(Fortunately, `gref+` logging isn't the default.)

Better workaround: Don't Do That™. Don't write to const strings.


~~ About `@(_AndroidRemapMembers)` Semantics… ~~

 1. Changing the Java hierarchy "requires" changing the managed
    hierarchy to mirror it.

    If we rename `Activity` to `RemapActivity` but *don't* change
    `MainActivity` to inherit the (bound!) `Example.RemapActivity`,
    the app *crashes*:

        JNI DETECTED ERROR IN APPLICATION: can't call void example.RemapActivity.onMyCreate(android.os.Bundle) on instance of example.MainActivity

    This can be "fixed" *without* changing the base class
    of `MainActivity` by instead changing the base class of the
    Java Callable Wrapper for `MainActivity` to `example.RemapActivity`.
    This can be done manually (just edit the files in `obj/…`!), but
    isn't really supported in "normal" xamarin-android usage
    (the next Clean will wipe your changes).

    Presumably InTune would make this Just Work by e.g. patching the
    `MainActivity.class` file.

 2. `/replacements/replace-type` interacts with
    `/replacements/replace-method`: at runtime, `//replace-type@from`
    *no longer exists*, meaning you ***cannot*** use that name
    in `//replace-method/@source-type` either!

    If `Activity` is remapped to `RemapActivity`, then *there is no*
    `Activity.onCreate()` method to similarly remap.  Instead, you
    need to specify `RemapActivity.onCreate()`.

    This warps the brain a bit.

    This:

        <replace-method
            source-type="example/RemapActivity"
            source-method-name="onCreate"
            target-type="example/RemapActivity"
            target-method-name="onMyCreate" target-method-instance-to-static="false" />

    not this:

        <replace-method
            source-type="android/app/Activity"
            source-method-name="onCreate"
            target-type="example/RemapActivity"
            target-method-name="onMyCreate" target-method-instance-to-static="false" />

 3. Don't intermix type renames with
    `/replace-method/@target-method-instance-to-static='true']`.
    It *can* be done, but also warps the brain.

    The deal with `@target-method-instance-to-static` is that it
    it changes the target method signature -- unless explicitly
    provided in `/replace-method/@target-method-signature` --
    so that the "source declaring type" is a prefix.

    Thus given

        <replace-method
            source-type="android/view/View"
            source-method-name="setOnClickListener"
            target-type="example/ViewHelper"
            target-method-name="mySetOnClickListener" target-method-instance-to-static="true" />

    we'll look for `ViewHelper.mySetOnClickListener(View, View.OnClickListener)`.

    If we renamed `View` to `MyView`, we would instead look for
    `ViewHelper.mySetOnClickListener(MyView, View.OnClickListener)`
    (note changed parameter type).

    This almost certainly *won't* work.


~~ InTune Integration Testing? ~~

For "more complete" InTune integration testing, one will want the
path to `remapping-config.json`, without hardcoding things.
This can be done with `%(PackageReference.GeneratePathProperty)`=True
and using `$(PkgMicrosoft_Intune_MAM_Remapper_Tasks)`:

	<ItemGroup>
	  <PackageReference
	      Include="Microsoft.Intune.MAM.Remapper.Tasks"
	      Version="0.1.4635.1"
	      IncludeAssets="none"
	      GeneratePathProperty="True"
	      ReferenceOutputAssembly="False"
	  />
	</ItemGroup>
	<Target Name="_AddMamFiles"
	    BeforeTargets="_AddAndroidCustomMetaData">
	  <ItemGroup>
	    <_AndroidMamMappingFile Include="$(PkgMicrosoft_Intune_MAM_Remapper_Tasks)/content/MonoAndroid10/remapping-config.json" />
	  </ItemGroup>
	</Target>

This is still fraught with some peril, as it likely also depends on
getting the right "inner" build, which may require using the plural
`$(TargetFrameworks)` property, not the singular `$(TargetFramework)`.
This might still be a useful start.


~~ TODO ~~

Optimize this mess:
#7020

[0]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html
[1]: https://developer.android.com/studio/write/java8-support#library-desugaring
[2]: https://developer.android.com/reference/java/util/Arrays#stream(T[])
[3]: https://docs.microsoft.com/en-us/mem/intune/fundamentals/what-is-intune
[4]: https://www.nuget.org/packages/Microsoft.Intune.MAM.Remapper.Tasks/
simonrozsival pushed a commit to simonrozsival/xamarin-android that referenced this pull request May 31, 2022
Fixes: dotnet/java-interop#867

Context: dotnet/java-interop@1f27ab5
Context: dotnet#6142 (comment)
Context: dotnet#7020

Changes: dotnet/java-interop@843f3c7...1f27ab5

  * dotnet/java-interop@1f27ab55: [Java.Interop] Type & Member Remapping Support (dotnet#936)
  * dotnet/java-interop@02aa54e0: [Java.Interop.Tools.JavaCallableWrappers] marshal method decl types (dotnet#987)
  * dotnet/java-interop@e7bacc37: [ci] Update azure-pipelines.yaml to Pin .NET 6.0.202 (dotnet#986)
  * dotnet/java-interop@fb94d598: [Java.Interop.Tools.JavaCallableWrappers] Collect overriden methods (dotnet#985)
  * dotnet/java-interop@3fcce746: [Java.Interop.{Dynamic,Export}] Nullable Reference Type support (dotnet#980)

~~ The Scenarios ~~

Our Java binding infrastructure involves looking up types and methods
via [JNI][0], and assumes that types and methods won't "move" in an
unexpected manner.  Methods which move from a subclass to a
superclass works transparently.  Methods which are moved to an
entirely different class causes us problems.

Case in point: [desugaring][0], which can *move* Java types to places
that our bindings don't expect.  For example, [`Arrays.stream(T[])`][1]
may be moved into a `DesugarArrays` class, or a default interface
method `Example.m()` may be moved into an `Example$-CC` type.
Java.Interop has not supported such changes, resulting in throwing a
`Java.Lang.NoSuchMethodError` on Android versions where methods are
not where we expect.

Additionally, the [InTune Mobile Application Management][3] team
needs a expanded type and member lookup mechanism in order to
simplify how they maintain their product.  Currently they make things
work by rewriting IL, which can be brittle.


~~ Build actions ~~

To improve support for this, dotnet/java-interop#936 introduces
new `virtual` methods into `Java.Interop.JniRuntime.JniTypeManager`
which are called as part of type and member lookup, allowing
`AndroidTypeManager` to participate in the type and member resolution
process.

`AndroidTypeManager` in turn needs to know what types and members can
be remapped, and what they should be remapped to.  Some of these can
be algorithmic, such as pre-pending `Desugar` or appending `$-CC` for
the Desugar case.

The InTune use case involves a table, contained within the
[Microsoft.Intune.MAM.Remapper.Tasks NuGet package][4].

Update `src/Xamarin.Android.Build.Tasks` to add a new
`@(_AndroidRemapMembers)` Build action.  This build action is not
externally supported; it's to help test the feature.  Files with this
build action are XML files which control type and member remapping:

	<replacements>
	  <replace-type
	      from="android/app/Activity"
	      to="com/microsoft/intune/mam/client/app/MAMActivity" />
	  <replace-method
	      source-type="com/microsoft/intune/mam/client/app/MAMActivity"
	      source-method-name="onCreate" source-method-signature="(Landroid/os/Bundle;)V"
	      target-type="com/microsoft/intune/mam/client/app/MAMActivity"
	      target-method-name="onMAMCreate" target-method-instance-to-static="false" />
	</replacements>

`//replacements/replace-method` is structured with each attribute
corresponding to a member on the `JniRuntime.ReplacementMethodInfo`
structure, in dotnet/java-interop@1f27ab55.

  * `//replace-method/@source-type` is
    `JniRuntime.ReplacementMethodInfo.SourceJniType`
  * `//replace-method/@source-method-name` is
    `JniRuntime.ReplacementMethodInfo.SourceJniMethodName`
  * `//replace-method/@source-method-signature` is
    `JniRuntime.ReplacementMethodInfo.SourceJniMethodSignature`
  * `//replace-method/@target-type` is
    `JniRuntime.ReplacementMethodInfo.TargetJniType`
  * `//replace-method/@target-method-name` is
    `JniRuntime.ReplacementMethodInfo.TargetJniMethodName`
  * `//replace-method/@target-method-signature` is
    `JniRuntime.ReplacementMethodInfo.TargetJniMethodSignature`
    This attribute is optional.
  * `//replace-method/@target-method-parameter-count` is
    `JniRuntime.ReplacementMethodInfo.TargetJniMethodParameterCount`.
    This attribute is optional.
  * `//replace-method/@target-method-instance-to-static` is
    `JniRuntime.ReplacementMethodInfo.TargetJniMethodIsStatic`

`@source-type`, `@source-method-name`, and `@source-method-signature`
combined serve as a "key" for looking up the associated `@target-*`
information.

Update `src/Xamarin.Android.Build.Tasks` to add a new
`@(_AndroidMamMappingFile)` Build action.  This build action is not
externally supported; it's to help test the feature.  Files with this
build action are expected to be JSON documents which follow the
current conventions of `remapping-config.json`, within the
`Microsoft.Intune.MAM.Remapper.Tasks` NuGet package.  This build
action is not externally supported; this is currently for testing
purposes.  `@(_AndroidMamMappingFile)` files are processed at build
time into `@(_AndroidRemapMembers)` XML files.

During App builds, all `@(_AndroidRemapMembers)` files are merged
into an `@(AndrodAsset)` named `xa-internal/xa-mam-mapping.xml`.
This asset is opened and provided to `JNIEnv.Initialize()` as part of
native app startup.


~~ Putting it all together ~~

This will only work on .NET 7+.

App project has a `@(_AndroidRemapMembers)` file.  This item is
processed during App build, stored into the `.apk`, and read during
app startup on an Android device.

Given a Java binding such as:

	public partial class Activity {
	    static readonly JniPeerMembers _members = new XAPeerMembers ("android/app/Activity", typeof (Activity));
	}

when the `JniPeerMembers` constructor runs, it will call
`JniEnvironment.Runtime.TypeManager.GetReplacementType("android/app/Activity")`.
If `@(_AndroidRemapMembers)` is based on the InTune
`remapping-config.json` file, then `android/app/Activity` is mapped
to `com/microsoft/intune/mam/client/app/MAMActivity`, and
`JNIEnv::FindClass()` will be told to lookup `MAMActivity`, *not*
`Activity`.

*If `MAMActivity` can't be found*, e.g. you're testing this all out,
the app will ~immediately crash, as `MAMActivity` doesn't exist. 😅

If `MAMActivity` can be found, eventually `Activity.OnCreate()` will
need to be invoked:

	partial class Activity {
	    protected virtual unsafe void OnCreate (Android.OS.Bundle? savedInstanceState)
	    {
	        const string __id = "onCreate.(Landroid/os/Bundle;)V";
	        try {
	            JniArgumentValue* __args = stackalloc JniArgumentValue [1];
	            __args [0] = new JniArgumentValue ((savedInstanceState == null) ? IntPtr.Zero : ((global::Java.Lang.Object) savedInstanceState).Handle);
	            _members.InstanceMethods.InvokeVirtualVoidMethod (__id, this, __args);
	        } finally {
	            global::System.GC.KeepAlive (savedInstanceState);
	        }
	    }
	}

`_members.InstanceMethods.InvokeVirtualVoidMethod()` will internally
make a call similar to:

	var r = JniEnvironment.Runtime.TypeManager.GetReplacementMethodInfo (
	    "com/microsoft/intune/mam/client/app/MAMActivity",
	    "onCreate",
	    "(Landroid/os/Bundle;)V"
	);

The data returned will be equivalent to:

	var r = new JniRuntime.ReplacementMethodInfo {
	    SourceJniType                   = "com/microsoft/intune/mam/client/app/MAMActivity",    // from input parameter
	    SourceJniMethodName             = "onCreate",                                           // from input parameter
	    SourceJniMethodSignature        = "(Landroid/os/Bundle;)V",                             // from input parameter

	    TargetJniType                   = "com/microsoft/intune/mam/client/app/MAMActivity",    // from //replace-method/@target-type
	    TargetJniMethodName             = "onMAMCreate",                                        // from //replace-method/@target-method-name
	    TargetJniMethodSignature        = "(Landroid/os/Bundle;)V",                             // from input parameter, as signature didn't change
	    TargetJniMethodParameterCount   = 1,                                                    // computed based on signature
	    TargetJniMethodIsStatic         = false,                                                // from //replace-method/@target-method-instance-to-static
	}

This will allow `_members.InstanceMethods.InvokeVirtualVoidMethod()`
to instead resolve and invoke `MAMActivity.onMAMCreate()`.


~~ Tools ~~

`tools/remap-mam-json-to-xml` is added, and will process the InTune
JSON file into `@(_AndroidRemapMembers)` XML:

	$ dotnet run --project tools/remap-mam-json-to-xml  -- \
	  $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json
	<replacements>…


~~ Unit Tests ~~

`@(_AndroidRemapMembers)` usage is required by
`Mono.Android.NET-Tests.apk`, as `Java.Interop-Tests.dll` exercises
the type and member remapping logic.


~~ Unrelated `gref+` logging fixes ~~

When `debug.mono.log` = `gref+`, the app could crash:

	signal 11 (SIGSEGV), code 2 (SEGV_ACCERR), fault addr 0x79c045dead

This was likely because a constant string was provided to
`OSBridge::_write_stack_trace()`, which tried to write into the
constant string, promptly blowing things up.

Workaround: don't use `gref+` logging when a GC occurs?
(Fortunately, `gref+` logging isn't the default.)

Better workaround: Don't Do That™. Don't write to const strings.


~~ About `@(_AndroidRemapMembers)` Semantics… ~~

 1. Changing the Java hierarchy "requires" changing the managed
    hierarchy to mirror it.

    If we rename `Activity` to `RemapActivity` but *don't* change
    `MainActivity` to inherit the (bound!) `Example.RemapActivity`,
    the app *crashes*:

        JNI DETECTED ERROR IN APPLICATION: can't call void example.RemapActivity.onMyCreate(android.os.Bundle) on instance of example.MainActivity

    This can be "fixed" *without* changing the base class
    of `MainActivity` by instead changing the base class of the
    Java Callable Wrapper for `MainActivity` to `example.RemapActivity`.
    This can be done manually (just edit the files in `obj/…`!), but
    isn't really supported in "normal" xamarin-android usage
    (the next Clean will wipe your changes).

    Presumably InTune would make this Just Work by e.g. patching the
    `MainActivity.class` file.

 2. `/replacements/replace-type` interacts with
    `/replacements/replace-method`: at runtime, `//replace-type@from`
    *no longer exists*, meaning you ***cannot*** use that name
    in `//replace-method/@source-type` either!

    If `Activity` is remapped to `RemapActivity`, then *there is no*
    `Activity.onCreate()` method to similarly remap.  Instead, you
    need to specify `RemapActivity.onCreate()`.

    This warps the brain a bit.

    This:

        <replace-method
            source-type="example/RemapActivity"
            source-method-name="onCreate"
            target-type="example/RemapActivity"
            target-method-name="onMyCreate" target-method-instance-to-static="false" />

    not this:

        <replace-method
            source-type="android/app/Activity"
            source-method-name="onCreate"
            target-type="example/RemapActivity"
            target-method-name="onMyCreate" target-method-instance-to-static="false" />

 3. Don't intermix type renames with
    `/replace-method/@target-method-instance-to-static='true']`.
    It *can* be done, but also warps the brain.

    The deal with `@target-method-instance-to-static` is that it
    it changes the target method signature -- unless explicitly
    provided in `/replace-method/@target-method-signature` --
    so that the "source declaring type" is a prefix.

    Thus given

        <replace-method
            source-type="android/view/View"
            source-method-name="setOnClickListener"
            target-type="example/ViewHelper"
            target-method-name="mySetOnClickListener" target-method-instance-to-static="true" />

    we'll look for `ViewHelper.mySetOnClickListener(View, View.OnClickListener)`.

    If we renamed `View` to `MyView`, we would instead look for
    `ViewHelper.mySetOnClickListener(MyView, View.OnClickListener)`
    (note changed parameter type).

    This almost certainly *won't* work.


~~ InTune Integration Testing? ~~

For "more complete" InTune integration testing, one will want the
path to `remapping-config.json`, without hardcoding things.
This can be done with `%(PackageReference.GeneratePathProperty)`=True
and using `$(PkgMicrosoft_Intune_MAM_Remapper_Tasks)`:

	<ItemGroup>
	  <PackageReference
	      Include="Microsoft.Intune.MAM.Remapper.Tasks"
	      Version="0.1.4635.1"
	      IncludeAssets="none"
	      GeneratePathProperty="True"
	      ReferenceOutputAssembly="False"
	  />
	</ItemGroup>
	<Target Name="_AddMamFiles"
	    BeforeTargets="_AddAndroidCustomMetaData">
	  <ItemGroup>
	    <_AndroidMamMappingFile Include="$(PkgMicrosoft_Intune_MAM_Remapper_Tasks)/content/MonoAndroid10/remapping-config.json" />
	  </ItemGroup>
	</Target>

This is still fraught with some peril, as it likely also depends on
getting the right "inner" build, which may require using the plural
`$(TargetFrameworks)` property, not the singular `$(TargetFramework)`.
This might still be a useful start.


~~ TODO ~~

Optimize this mess:
dotnet#7020

[0]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html
[1]: https://developer.android.com/studio/write/java8-support#library-desugaring
[2]: https://developer.android.com/reference/java/util/Arrays#stream(T[])
[3]: https://docs.microsoft.com/en-us/mem/intune/fundamentals/what-is-intune
[4]: https://www.nuget.org/packages/Microsoft.Intune.MAM.Remapper.Tasks/
@github-actions github-actions bot locked and limited conversation to collaborators Apr 13, 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.

3 participants