[android-toolchain] Fix the _CreateMxeW32Toolchain Condition.
#122
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `_CreateMxeW32Toolchain` target condition checked for `:mxe-Win64:`, not `:mxe-Win32:`, which caused it to needlessly run whenever `:mxe-Win64:` was set (spending extra time), while *not* running when it was requested, i.e. when `:mxe-Win32:` was set. Additionally, I previously had an MXE SNAFU wherein `android-toolchain.targets` was updated to build `zlib`, but my local build output *didn't contain `zlib` outputs*. This resulted in obscure build errors and needless tearing out of limited hair, which could have been avoided if the `zlib` outputs had been listed as outputs that could be checked for. Thus, rework the existing `_CreateMxeW32Toolchain` and `_CreateMxeW64Toolchain` dichotomy, and instead introduce a new `@(_AndroidMxeToolchain)` item group with conditional values based on `$(AndroidSupportedHostJitAbisForConditionalChecks)`. This allows for a more "extensible" output file format that doesn't needlessly copy obscure identifiers like `i686-w64-mingw32.static` for output files. `_CreateMxeW32Toolchain` and `_CreateMxeW64Toolchain` are dead; long live `_CreateMxeToolchains`!
jonpryor
pushed a commit
that referenced
this pull request
Jun 10, 2021
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/
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
_CreateMxeW32Toolchaintarget condition checked for:mxe-Win64:, not:mxe-Win32:, which caused it to needlessly runwhenever
:mxe-Win64:was set (spending extra time), while notrunning when it was requested, i.e. when
:mxe-Win32:was set.Additionally, I previously had an MXE SNAFU wherein
android-toolchain.targetswas updated to buildzlib, but my localbuild output didn't contain
zliboutputs. This resulted in obscurebuild errors and needless tearing out of limited hair, which could
have been avoided if the
zliboutputs had been listed as outputsthat could be checked for.
Thus, rework the existing
_CreateMxeW32Toolchainand_CreateMxeW64Toolchaindichotomy, and instead introduce a new@(_AndroidMxeToolchain)item group with conditional values based on$(AndroidSupportedHostJitAbisForConditionalChecks). This allows fora more "extensible" output file format that doesn't needlessly copy
obscure identifiers like
i686-w64-mingw32.staticfor output files._CreateMxeW32Toolchainand_CreateMxeW64Toolchainare dead; longlive
_CreateMxeToolchains!