Skip to content

Conversation

@radekdoulik
Copy link
Member

@radekdoulik radekdoulik commented May 30, 2018

Build the tool to be used for generating the marshal methods, it will
be used when user enables it with the
AndroidGenerateJniMarshalMethods property.

Update our copy of Java.Runtime.Environment.dll.config.
Add dll maps to map libmono-android as java-interop shared library
for Java.Runtime.Environment.dll. Similar way as we do for
Java.Interop.dll in monodroid.targets

Share the dllmaps in build-tools/scripts/java-interop.dllmap

Example content of Java.Runtime.Environment.dll.config:

<configuration>
  <dllmap dll="java-interop" os="osx" target="lib/host-Darwin/libmono-android.debug.dylib" />
  <dllmap dll="java-interop" os="linux" target="lib/host-Linux/libmono-android.debug.so" />
  <dllmap dll="java-interop" os="windows" wordsize="64" target="lib/host-mxe-Win64/libmono-android.debug.dll" />
  <dllmap dll="java-interop" os="windows" wordsize="32" target="lib/host-mxe-Win32/libmono-android.debug.dll" />
  <dllmap dll="jvm.dll" os="osx" target="/Library/Java/JavaVirtualMachines/jdk1.8.0_144.jdk/Contents/Home/jre/lib/jli/libjli.dylib"/>
</configuration>

@radekdoulik radekdoulik requested a review from jonpryor May 30, 2018 10:17
@radekdoulik
Copy link
Member Author

It requires these PRs to be merged first: #1739 and dotnet/java-interop#321

@radekdoulik radekdoulik added the do-not-merge PR should not be merged. label May 30, 2018
@radekdoulik radekdoulik force-pushed the pr-pregenerate-marshal-methods-for-Mono-Android branch from 2b1a0cb to e5dd505 Compare May 31, 2018 17:59
@radekdoulik radekdoulik removed the do-not-merge PR should not be merged. label May 31, 2018
@radekdoulik radekdoulik force-pushed the pr-pregenerate-marshal-methods-for-Mono-Android branch 3 times, most recently from db07430 to fa627c7 Compare June 1, 2018 07:14
BeforeTargets="CoreCompile"
Inputs="$(JavaInteropFullPath)\tools\jnimarshalmethod-gen\Xamarin.Android.Tools.JniMarshalMethodGenerator.csproj"
Outputs="$(XAInstallPrefix)xbuild\Xamarin\Android\jnimarshalmethod-gen.exe;$(XAInstallPrefix)xbuild\Xamarin\Android\Java.Runtime.Environment.dll.config">
<MSBuild
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed?

Explicit <MSBuild/> invocations annoy Atsushi whenever files get out of sync. What will happen is that the .csproj isn't updated, but one of the source files for that project is updated, but the project won't necessarily be rebuilt, which later results in a "mismatch" that can only be fixed by git clean -xdf, and everyone is sad.

Additionally, I don't see why this is needed in the first place. Xamarin.Android.Tools.JniMarshalMethodGenerator.csproj is added to Xamarin.Android.sln; therefore, it should be built, eventually. The @(ProjectReference) should also ensure that the project is built.

Why is this <MSBuild/> needed at all?

<Name>jcw-gen</Name>
<ReferenceOutputAssembly>False</ReferenceOutputAssembly>
</ProjectReference>
<ProjectReference Include="..\..\external\Java.Interop\tools\jnimarshalmethod-gen\Xamarin.Android.Tools.JniMarshalMethodGenerator.csproj">
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed at all?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably not needed. It was there to be sure that it is available at the right time. When I removed the part that pre-generated the marshal methods in Mono.Android I didn't want to break it, so rather kept it there.

OK, I guess I will try another round.

Projects="$(JavaInteropFullPath)\tools\jnimarshalmethod-gen\Xamarin.Android.Tools.JniMarshalMethodGenerator.csproj"
/>
<Exec
Command="sed -i .in.bak -e '/&lt;configuration&gt;/r $(MSBuildThisFileDirectory)\..\..\build-tools\scripts\java-interop.dllmap' $(XAInstallPrefix)xbuild\Xamarin\Android\Java.Runtime.Environment.dll.config"
Copy link
Contributor

Choose a reason for hiding this comment

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

This part seems...okay...except for two things:

  1. This won't build on Windows. Can we instead use the <ReplaceFileContents/> task?
  2. The output file is in $(XAInstallPrefix)xbuild\Xamarin\Android. Why is this being generated in Mono.Android.targets, which places outputs into $(XAInstallPrefix)xbuild-frameworks\...?

If this belongs anywhere, it's in src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.targets.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have meanwhile moved it to src/monodroid/monodroid.targets which is hopefully good place too. We handle the configs at one place and the dllmaps use the monodroid shared libraries.

I can try to use ReplaceFileContents task. Hope it could be done as an addition to speedup the merge process.

@jonpryor
Copy link
Contributor

jonpryor commented Jun 1, 2018

Additionally, the PR title and comment are now entirely unrelated to what the PR does.

@radekdoulik radekdoulik force-pushed the pr-pregenerate-marshal-methods-for-Mono-Android branch from fa627c7 to ad314b6 Compare June 1, 2018 11:07
<Name>dependencies</Name>
<ReferenceOutputAssembly>False</ReferenceOutputAssembly>
</ProjectReference>
<ProjectReference Include="..\..\external\Java.Interop\tools\jnimarshalmethod-gen\Xamarin.Android.Tools.JniMarshalMethodGenerator.csproj">
Copy link
Member Author

Choose a reason for hiding this comment

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

It is here to make sure we have $(XAInstallPrefix)xbuild\Xamarin\Android\Java.Runtime.Environment.dll.config before modifying it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Given you have a sed -- and/or <ReplaceFileContents/> -- which can have any input path, why does $(XAInstallPrefix)xbuild\Xamarin\Android\Java.Runtime.Environment.dll.config need to exist before it's modified?

You could do, sed-wise:

sed regex... source-file > dest-file

@radekdoulik radekdoulik changed the title [Mono.Android] Pregenerate marshal methods in Mono.Android.dll [monodroid] Build and config jnimarshalmethod-gen.exe Jun 1, 2018
@radekdoulik radekdoulik force-pushed the pr-pregenerate-marshal-methods-for-Mono-Android branch from ad314b6 to 41d877a Compare June 1, 2018 11:11
@jonpryor
Copy link
Contributor

jonpryor commented Jun 1, 2018

As a non-sed-based example:

diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.targets
index f60093ab..69537ee7 100644
--- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.targets
+++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.targets
@@ -10,6 +10,7 @@
     <_GeneratedProfileClass>$(MSBuildThisFileDirectory)$(IntermediateOutputPath)Profile.g.cs</_GeneratedProfileClass>
     <BuildDependsOn>
       _GenerateXACommonProps;
+      _GenerateJavaRuntimeEnvironmentConfig;
       $(BuildDependsOn);
       _CopyExtractedMultiDexJar;
       _BuildMonoScripts;
@@ -136,6 +137,22 @@
         Replacements="@PACKAGE_VERSION@=$(ProductVersion);@PACKAGE_VERSION_BUILD@=$(XAVersionCommitCount)">
     </ReplaceFileContents>
   </Target>
+  <Target Name="_GenerateJavaRuntimeEnvironmentConfig"
+      Inputs="$(JavaInteropFullPath)\src\Java.Runtime.Environment\Java.Runtime.Environment.dll.config.in"
+      Outputs="$(OutputPath)Java.Runtime.Environment.dll.config">
+    <ReadLinesFromFile File="..\..\build-tools\scripts\java-interop.dllmap">
+      <Output TaskParameter="Lines" ItemName="_JavaInteropDllMapContent" />
+    </ReadLinesFromFile>
+    <PropertyGroup>
+      <_DllMap>@(_JavaInteropDllMapContent->'%(Identity)', '%0a')</_DllMap>
+    </PropertyGroup>
+    <Message Text="# jonp: _JavaInteropDllMapContent=```$(_DllMap)```" />
+    <ReplaceFileContents
+        SourceFile="$(JavaInteropFullPath)\src\Java.Runtime.Environment\Java.Runtime.Environment.dll.config.in"
+        DestinationFile="$(OutputPath)Java.Runtime.Environment.dll.config"
+        Replacements="@JAVA_RUNTIME_ENVIRONMENT_DLLMAP@=$(_DllMap)"
+    />
+  </Target>
   <Target Name="_GenerateProfileClass"
       BeforeTargets="CoreCompile"
       Inputs="@(_SharedRuntimeAssemblies)"

The above generates bin/Debug/lib/xamarin.android/xbuild/Xamarin/Android/Java.Runtime.Environment.dll.config with contents:

<configuration>
  <dllmap dll="jvm.dll" os="@OS_NAME@" target="@JI_JVM_PATH@"/>
<dllmap dll="java-interop" os="osx" target="lib/host-Darwin/libmono-android.debug.dylib" />
<dllmap dll="java-interop" os="linux" target="lib/host-Linux/libmono-android.debug.so" />
<dllmap dll="java-interop" os="windows" wordsize="64" target="lib/host-mxe-Win64/libmono-android.debug.dll" />
<dllmap dll="java-interop" os="windows" wordsize="32" target="lib/host-mxe-Win32/libmono-android.debug.dll" />
</configuration>

Which admittedly is a tad weird, as we have that jvm.dll entry, but that's plausibly fixed .

@radekdoulik radekdoulik force-pushed the pr-pregenerate-marshal-methods-for-Mono-Android branch 2 times, most recently from 14f79e8 to 4100294 Compare June 11, 2018 20:11
@radekdoulik
Copy link
Member Author

Updated to use ReplaceFileContents. Had to fiddle with the text a bit to avoid the task to parse the replacement text as it contains = chars.

@radekdoulik
Copy link
Member Author

Lets handle the jvm path in next PR(s).

@radekdoulik radekdoulik force-pushed the pr-pregenerate-marshal-methods-for-Mono-Android branch from 4100294 to c1aea95 Compare June 12, 2018 09:25
Build the tool to be used for generating the marshal methods, it will
be used when user enables it with the
`AndroidGenerateJniMarshalMethods` property.
Add dll maps to map `libmono-android` as `java-interop` shared library
for `Java.Runtime.Environment.dll`. Similar way as we do for
`Java.Interop.dll` in `monodroid.targets`
@radekdoulik radekdoulik force-pushed the pr-pregenerate-marshal-methods-for-Mono-Android branch from c1aea95 to 6c53d0e Compare June 13, 2018 06:25
@jonpryor jonpryor merged commit d18c45a into dotnet:master Jun 14, 2018
@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants