Fix bundling of assemblies in NDK with unified headers #1105
Conversation
This file contains hidden or 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
Contributor
|
What would be helpful is a test. :-) A simple test would be to update the NDK used for our build -- in (Which actually shows a pre-existing oversight: we have that project, but we don't appear to be using that project. That in turn should be possible by updating |
grendello
force-pushed
the
bundle-ndk16-fix
branch
from
ec039f6 to
d613081
Compare
jonpryor
added a commit
to jonpryor/xamarin-android
that referenced
this pull request
Dec 15, 2017
Context: https://developer.xamarin.com/releases/android/xamarin.android_7/xamarin.android_7.4/#API-10_is_Obsolete Context: dotnet#1105 Context: dotnet#1032 In Xamarin.Android 7.4, we announced that API-10 was obsolete and would be removed in a future release. PR dotnet#1032 attempted to do so, but does not yet work, so until now we have continued shipping API-10 assemblies, even though Xamarin.Android 7.4 hit stable on 2017-Aug-11. Meanwhile, PR dotnet#1105 includes an NDK bump, which in turn means that the lowest API level we can compile for will be *API-14*. Attempt to split the difference, assuming PR dotnet#1105 lands before PR dotnet#1032: Keep the supporting files for API-10, but don't build them, and explicitly remove `v2.3` from the `.vsix` file.
This was referenced Dec 15, 2017
Context: https://developercommunity.visualstudio.com/content/problem/166029/building-xamarinandroid-application-with-bundleass.html NDK 14 introduced experimental support for so-called "unified headers" which replace the per-platform C header files, NDK 15 deprecated the old headers and NDK 16 removed them completely. Xamarin.Android has been building itself with NDK configured to use unified headers for a while now, but we missed one piece of code which is affected by the change - mkbundle. This commit adds code to MakeBundleNativeCodeExternal task to make it possible to build code generated by mkbundle on systems with NDK that lacks the per-platform header files. This is done in a backward-compatible way but if your NDK has both the old and the new headers, the latter will be used. Additionally, minimum API versions to 14 for 32-bit targets (forward compatibility with NDK r16)
grendello
force-pushed
the
bundle-ndk16-fix
branch
from
d613081 to
60eee32
Compare
jonpryor
added a commit
that referenced
this pull request
Dec 16, 2017
Context: https://developer.xamarin.com/releases/android/xamarin.android_7/xamarin.android_7.4/#API-10_is_Obsolete Context: #1105 Context: #1032 In Xamarin.Android 7.4, we announced that API-10 was obsolete and would be removed in a future release. PR #1032 attempted to do so, but does not yet work, so until now we have continued shipping API-10 assemblies, even though Xamarin.Android 7.4 hit stable on 2017-Aug-11. Meanwhile, PR #1105 changes NDK use semantics such that *API-14* is now the lowest supported API level for NDK use. There is thus no point in shipping API-10 bindings. Attempt to split the difference, assuming PR #1105 lands before PR #1032: Keep the supporting files for API-10, but don't build them, and explicitly remove `v2.3` from the `.vsix` file.
|
Any workaround ? downgrade NDK ? |
Contributor
|
@softlion: Yes, the workaround is to downgrade to an NDK which provides the original per-API-level headers, not just the unified headers. NDK r14b qualifies. |
|
@softlion I can confirm that downgrading to NDK v15 also works. The breaking change was in v16 of Android NDK - https://github.com/android-ndk/ndk/wiki/Changelog-r16 |
|
What is the milestone for this merge ? |
|
2018/02/19 Still no milestone ? You have to be faster in releases. |
Contributor
|
@softlion: We generally don't set milestones on PRs. We set them on issues, and not always then. In this case, the fix is in the d15-6 branch, as d15-6 was branched after this was merged to master. It is thus in the Xamarin.Android 8.2 betas, and has been since Visual Studio 2017 15.6 Preview 2 (early January). |
jonpryor
pushed a commit
that referenced
this pull request
Aug 3, 2020
Context: xamarin/android-sdk-installer@2b4d1a6 Changes: xamarin/monodroid@5784a74...b12a3e8 * xamarin/monodroid@b12a3e8d5: Bump to xamarin/android-sdk-installer@2b4d1a6 & xamarin/xamarin-android/master@d583b7c (#1107) * xamarin/monodroid@3d84feddd: Bump to xamarin/xamarin-android/master@f0d565fe (#1106) * xamarin/monodroid@ec2931825: Bump to xamarin/android-sdk-installer@c6c12db0 (#1105) Sometimes, `<InstallAndroidDependencies/>` would fail when running on Windows, a'la: …\Xamarin\Android\Xamarin.Installer.Common.targets(12,3): Cannot access a disposed object. Object name: 'SslStream'. A fix for the `ObjectDisposedException` has been brought in via the `android-sdk-installer` bump in xamarin/monodroid@b12a3e8d. The `<InstallAndroidDependencies/>` test previously had issues due to clean up logic in the test framework that partially deleted the newly provisioned Android SDK on a subsequent build. I believe that we've been seeing successful results for this test because the subsequent build in the test would fall back to a different Android SDK that was already on disk. The test has been improved by asserting that the newly installed Android SDK is used during the subsequent build.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.
Context: https://developercommunity.visualstudio.com/content/problem/166029/building-xamarinandroid-application-with-bundleass.html
NDK 14r introduced experimental support for so-called "unified headers" which
replace the per-platform C header files, NDK 15 deprecated the old headers and
NDK 16 removed them completely. Xamarin.Android has been building itself with
NDK configured to use unified headers for a while now, but we missed one piece
of code which is affected by the change - mkbundle.
This commit adds code to MakeBundleNativeCodeExternal task to make it possible
to build code generated by mkbundle on systems with NDK that lacks the
per-platform header files. This is done in a backward-compatible way but if your
NDK has both the old and the new headers, the latter will be used.
Additionally, minimum API versions to 14 for 32-bit targets (forward
compatibility with NDK r16)