-
Notifications
You must be signed in to change notification settings - Fork 533
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bundled multidex jar #124
Bundled multidex jar #124
Conversation
Android SDK had obsoleted multidex support components, so we'd rather embed the jar directly in our SDK.
What this change actually does are: - additionally download support m2repository in android-toolchains. - extract multidex jar from support multidex aar. - extend UnzipDirectoryChildren task to support non-children extraction.
That sounds like we should distribute this file within the Xamarin.Android SDK. Can we do that? What is the redistributable license for it? Option 2 is more complicated, but it can be done on the end-users machine, and thus it doesn't have any redistribution requirements like Option 1 does. |
It should be APACHE2 as descibed in android-sdk-*/extras/android/m2repository/NOTICE.txt. |
Maybe we should copy that as something like lib/mandroid/MULTIDEX_JAR_LICENSE.txt to make it explicit. |
If we copy the license and update e.g. |
Where can I find the existing license file? There must be one since we have already including a lot of third party softwares this must not be the first one, right? |
A better solution is, as I initially suggested as an example solution, copy the support library license to lib/mandroid as MULTIDEX_JAR_LICENSE because just having one License.rtf in the product does not resolve the problem that people cannot find the exact relevant license for the .jar file. |
…ENSE. It is to make it clear that the jar file is from Android SDK support library.
@atsushieno The same thing (using m2repository) should be done for proguard.jar, since it also uses the deprecated one (android-sdk\tools\proguard\lib\proguard.jar). That version is still ProGuard 4.7, which still does not support JDK1.8. |
It is not exactly the same code path so I won't handle it within this specific PR, but could you tell me in which directory in Android support m2repository it should be? I cannot locate it. |
My current plan to support Java8 class file is to handle it via jack, which seems to also handle proguard stuff. That needs not a small set of changes (but not very big). |
It is not, my mistake. Gradle downloads it, but it's not inside m2repo. |
Oh okay, that actually still makes it much easier. Thanks for the great input! |
$(BuildDependsOn); | ||
_CopyExtractedMultiDexJar; | ||
</BuildDependsOn> | ||
<_AndroidSdkLocation>$(ANDROID_SDK_PATH)</_AndroidSdkLocation> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be the $(AndroidSdkFullPath)
MSBuild property. Nothing is going to export an $(ANDROID_SDK_PATH)
environment variable.
The $(AndroidSdkFullPath)
property is brought in through the importing of Configuration.props
in Xamarin.Android.Build.Tasks.csproj
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelatedly: Please fix your editor to expand tabs into spaces on .targets
files and other XML files, with 2 space indents.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. monodroid does export that environment variable. Want me to eliminate the variables that are used ONLY in monodroid in the same manner? For example, InstantRunEnabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What should happen for "pure" xamarin-android builds? $(ANDROID_SDK_PATH)
won't be set, so e.g. $(_AndroidSdkLocation)\$(_MultiDexAarInAndroidSdk)
will be /extras/android/...
, which won't be readable or writable, and might result in a xamarin-android build error.
That seems less than desirable.
Perhaps this should use a fallback?
<_AndroidSdkLocation Condition=" '$(AndroidSdkFullPath)' != '' ">$(AndroidSdkFullPath)</_AndroidSdkLocation>
<_AndroidSdkLocation Condition=" '$(_AndroidSdkLocation)' == '' ">$(ANDROID_SDK_PATH)</_AndroidSdkLocation>
This should allow xamarin-android to build while allowing the monodroid build to override.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Want me to eliminate the variables that are used ONLY in monodroid in the same manner? For example, InstantRunEnabled.
If they can be removed, that would likely be best. If they can't be removed -- which may be the case for $(_InstantRunEnabled)
, which is used with the <DetermineJavaLibrariesToCompile/>
task -- then perhaps it should be renamed to be something more meaningful w/o context/memory/history.
For example, instead of DetermineJavaLibrariesToCompile.EnableInstantRun
, rename it to DetermineJavaLibrariesToCompile.SkipMonoPlatformJar
. For those just reading Xamarin.Android.Common.targets
, I assume that seeing SkipMonoPlatformJar="$(_SkipMonoPlatformJar)"
would be more meaningful than EnableInstantRun="$(_InstantRunEnabled)"
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pure xamarin-android configuration for _AndroidSdkLocation is defined immediately below.
Hello! I'm the build bot for the Mono project. I need approval from a Mono team member to build this pull request. A team member should reply with "approve" to approve a build of this pull request, "whitelist" to whitelist this and all future pull requests from this contributor, or "build" to explicitly request a build, even if one has already been done. Contributors can ignore this message. |
* [msbuild] switch to bundled multidex jar Android SDK had obsoleted multidex support components, so we'd rather embed the jar directly in our SDK. * Copy android-support-multidex.jar into mandroid directory. What this change actually does are: - additionally download support m2repository in android-toolchains. - extract multidex jar from support multidex aar. - extend UnzipDirectoryChildren task to support non-children extraction. * Copy support library license file to lib/mandroid as MULTIDEX_JAR_LICENSE. It is to make it clear that the jar file is from Android SDK support library.
Changes: xamarin/monodroid@76c04cd...1f2ce15 * xamarin/monodroid@1f2ce1562: [tools/msbuild] only run _GetPrimaryCpuAbi for Fast Dev (#1208) * xamarin/monodroid@691310ede: Bump android-sdk-installer to use Mono.Unix (#1207) * xamarin/monodroid@48843fcb2: [tools/msbuild] <GetPrimaryCpuAbi/> selects backup RIDs for .NET 6 (#1205) * xamarin/monodroid@4af48f54b: [tools/fastdev] Add error checking when writing data to disk. (#1204) * xamarin/monodroid@65b7b2dd4: [tools/fastdev] Rework Unix timestamp calculation code in xamarin.find. (#1202) * xamarin/monodroid@401f170e9: [build] fix `dotnet tool install` command (#1203) * xamarin/monodroid@a879b250b: Bump to 032d840, xamarin/androidtools@355d015 (#1199) * xamarin/monodroid@9f9ee378c: [tools/msbuild] Missing translations for XA0135 (#1198) Changes: dotnet/android-tools@683f375...49936d6 * dotnet/android-tools@49936d6: Merge pull request #124 from xamarin/update-libzipsharp * dotnet/android-tools@ef78dfc: Bump LibZipSharp to 2.0.0-alpha6 * dotnet/android-tools@e3d708c: [BaseTasks] fix `\`-delimited paths on macOS (#122) * dotnet/android-tools@bdcf899: Reference the new Mono.Unix nuget (#123) * dotnet/android-tools@90d7621: [BaseTasks] add ABI detection for RIDs (#121) * dotnet/android-tools@79e3b97: [JdkInfo] handle invalid XML from /usr/libexec/java_home (#120) * dotnet/android-tools@81519fe: Add SECURITY.md (#119) Xamarin.Android uses the Mono's Mono.Posix assembly on Unix machines to perform tasks not possible with BCL classes, provided by the [`Mono.Posix.NETStandard` NuGet][0]. The `Mono.Posix` source has been extracted into the [mono/mono.posix repo][1], which is used to build the new [`Mono.Unix` NuGet package][2]. Update the xamarin/xamarin-android repo -- and various dependencies -- to use the `Mono.Unix` package instead of `Mono.Posix.NETStandard`. This includes the [`Xamarin.LibZipSharp` NuGet][3], as of dotnet/android-libzipsharp@cf5e33c6. `Mono.Unix` no longer uses the older `libMonoPosixHelper` dynamic library, replaced by a new `libMono.Unix` native library. Unfortunately, this change broke a number of tests since the `Mono.Unix.dll` assembly was no longer able to find its companion native shared library. While the `Mono.Posix.NETStandard` NuGet package provides the `libMonoPosixHelper` native library, in practice the *actual* `libMonoPosixHelper` that was used was the "system" library included with the system mono. `Mono.Unix`'s new native helper library, however, must be taken from the NuGet and both the Mono and dotnet runtimes must be told where to load the library from once a P/Invoke into `Mono.Unix` is encountered in managed code. The native library is copied from the NuGet to the referencing application's output directory and it should be loaded from there. This proved to be easy for the "legacy" Mono: a simple [`dllmap` configuration][4] and everything works as it should. With `dotnet` however, dllmap doesn't work. `dotnet` has instead a number of mechanisms to configure where the native libraries can be found (5 I think). Unfortunately, the mechanisms either require that a main executable of the application calls the APIs on entry (e.g. in the `Main()` method) or that a JSON configuration file is provided for the application, telling the runtime where the native libraries reside. In case of `Xamarin.Android.Build.Tasks` there is no main executable we can configure, since it works in the MSBuild context, providing tasks and utilities to build Xamarin.Android apps. In this instance, `dotnet` could be persuaded to find the libraries by calling one of the 5 APIs. The problem with this approach, however, is that this action would have to be performed at **every** possible entry point to the `Xamarin.Android.Build.Tasks` assembly, since any of them could be used as the first one. While certainly possible, it would be both fragile and an unnecessary maintenance burden. Instead, a simpler (albeit a bit kludgy) solution was chosen: the `src/Xamarin.Android.Build.Tasks` build process now takes care of generating a fat (multi-architecture) binary for macOS hosts (including `x86-64` and `arm64` architectures) using the `lipo` utility, then it copies the resulting binary to the same directory where `Xamarin.Android.Build.Tasks.dll` and `Mono.Unix.dll` live. The Linux shared library is also copied to the same location. The `dotnet` runtime is able to find and load shared libraries that are in the same directory as the assembly that needs them and everything works as expected. [0]: https://www.nuget.org/packages/Mono.Posix.NETStandard/5.20.1-preview [1]: https://github.com/mono/mono.posix [2]: https://www.nuget.org/packages/Mono.Unix/ [3]: https://www.nuget.org/packages/Xamarin.LibZipSharp [4]: https://www.mono-project.com/docs/advanced/pinvoke/dllmap/
We need to switch multidex support library from obsolete support package to m2repository. (Because current one in use is, obsoleted. Those who set custom Android SDK cannot find it because they don't know much about Android SDK...)
Switching to m2repository is not that simple. There is no android-support-multidex.jar anymore. So far I can think of two solution options described below:
1: bundle android-support-multidex.jar in the resulting SDK just as a binary. It is the safest, most reliable and almost backward compatible solution. We'll just have to copy the jar to bin/Debug/lib/mandroid.
2: implement all below:
I believe there is no reason to chose option 2 in terms of implementation/QA cost. Just take the easier option.