[Xamarin.Android.Build.Tasks] support Embedded DSOs #2579
Conversation
|
The test failures I need to look into, it looks like this new feature isn't working during incremental builds. It looks like if |
|
What's worrying is this failure: https://jenkins.mono-project.com/job/xamarin-android-pr-builder-release/275/testReport/Xamarin.Android.EmbeddedDSO_Test/nunit/Possible_Crash___Release_Bundle/ Apparently there's no
Do we not capture |
|
This appears to be why the I'll need to try to find the relevant |
|
After lots of digging, I think (hope? pray?) that the places that Thus:
if (!zipFile->getEntryInfo(zipEntry, NULL, &uncompLen, NULL, NULL, NULL, NULL)) {
return INSTALL_FAILED_INVALID_APK;
}
if (!zipFile->getEntryInfo(zipEntry, &method, &uncompLen, NULL, &offset, &when, &crc)) {
ALOGD("Couldn't read zip entry info\n");
return INSTALL_FAILED_INVALID_APK;
}
if (!extractNativeLibs) {
// check if library is uncompressed and page-aligned
if (method != ZipFileRO::kCompressStored) {
ALOGD("Library '%s' is compressed - will not be able to open it directly from apk.\n",
fileName);
return INSTALL_FAILED_INVALID_APK;
}
if (offset % PAGE_SIZE != 0) {
ALOGD("Library '%s' is not page-aligned - will not be able to open it directly from"
" apk.\n", fileName);
return INSTALL_FAILED_INVALID_APK;
}
return INSTALL_SUCCEEDED;
}
ZipFileRO* zipFile = reinterpret_cast<ZipFileRO*>(apkHandle);
if (zipFile == NULL) {
return INSTALL_FAILED_INVALID_APK;
}
std::unique_ptr<NativeLibrariesIterator> it(
NativeLibrariesIterator::create(zipFile, debuggable));
if (it.get() == NULL) {
return INSTALL_FAILED_INVALID_APK;
}
const ScopedUtfChars cpuAbi(env, javaCpuAbi);
if (cpuAbi.c_str() == NULL) {
// This would've thrown, so this return code isn't observable by
// Java.
return INSTALL_FAILED_INVALID_APK;
}
ZipFileRO* zipFile = reinterpret_cast<ZipFileRO*>(apkHandle);
if (zipFile == NULL) {
return INSTALL_FAILED_INVALID_APK;
}
std::unique_ptr<NativeLibrariesIterator> it(
NativeLibrariesIterator::create(zipFile, debuggable));
if (it.get() == NULL) {
return INSTALL_FAILED_INVALID_APK;
}All of which leads me to believe that something is "wrong" with the |
|
@jonpryor I can reproduce locally. I think an incremental build can get the apk in a state where the .so files are compressed, but the env var is present. Build + delete apk + build again. Looking into it. |
Context: dotnet#2154 Fixes: dotnet#2415 Currently, in order to activate the embedded DSO support, one has to perform the following actions manually: 1. Add the `android:extractNativeLibs="false"` attribute to the `<application>` element in the `Properties/AndroidManifest.xml` file. 2. Add the following property to the project file: <AndroidStoreUncompressedFileExtensions>.so</AndroidStoreUncompressedFileExtensions> 3. Add an android environment file to the project with a line which says: __XA_DSO_IN_APK=1 ~~ Changes ~~ The presence of `android:extractNativeLibs="false"` in `AndroidManifest.xml` should setup all the extra "stuff" so developers don't have to manually do it. `android:extractNativeLibs="false"` would automatically do the following: 1. `.so` will be appended to `$(AndroidStoreUncompressedFileExtensions)`. Both the `<Aapt/>` and `<BuildApk/>` MSBuild tasks use this property. 3. A new generated file in `$(IntermediateOutputPath)` will add `__XA_DSO_IN_APK=1` as an `@(AndroidEnvironment)` build item. The problem here is with incremental builds... If `_GenerateJavaStubs` is skipped, we still need to know if `android:extractNativeLibs` is set. To make this work, I added a `CacheFile` property on the `GenerateJavaStubs` MSBuild task. It writes a simple XML document in `$(IntermediateOutputPath)javastubs.cache` such as: <Properties> <EmbeddedDSOsEnabled>True</EmbeddedDSOsEnabled> </Properties> The file will not exist at all unless the value is `True`, which will prevent this feature from impacting build times. I also added an MSBuild test to verify these changes are happening. I was also able to update `tests/EmbeddedDSOs` to rely on the new functionality. ~~ Other Changes ~~ I updated `_GenerateJavaStubs` to follow the "stamp" file convention of using `$(_AndroidStampDirectory)_GenerateJavaStubs.stamp`.
jonathanpeppers
force-pushed
the
msbuild-dsos
branch
from
4291641 to
d02910f
Compare
|
Spitballing a documentation rewrite: https://gist.github.com/jonpryor/fc46ef8f7933cede130eadeeafc736b9 My problem with the existing documentation is twofold:
(2) is closely related to (1): why should developers care about "DSO"s? It doesn't help that "DSO" isn't even defined within that document. (How many people will know that it means "Dynamic Shared Objects"? How many will care to know?) We "care" because we've riddled our codebase with the phrase "DSO", but that doesn't mean our users need to care. |
`<WriteLinesToFile WriteOnlyWhenDifferent="True"/>` is wrong
|
The documentation changes seemed good to me, I think I may have fixed something minor, otherwise looked good. |
|
The reported unit test failure is unrelated to this PR. |
4 participants
Context: #2154
Fixes: #2415
Currently, in order to activate the embedded DSO support, one has to
perform the following actions manually:
Add the
android:extractNativeLibs="false"attribute to the<application>element in theProperties/AndroidManifest.xmlfile.
Add the following property to the project file:
says:
Changes
The presence of
android:extractNativeLibs="false"inAndroidManifest.xmlshould setup all the extra "stuff" so developersdon't have to manually do it.
android:extractNativeLibs="false"would automatically do thefollowing:
.sowill be appended to$(AndroidStoreUncompressedFileExtensions). Both the<Aapt/>and<BuildApk/>MSBuild tasks use this property.A new generated file in
$(IntermediateOutputPath)will add__XA_DSO_IN_APK=1as an@(AndroidEnvironment)build item.I also added an MSBuild test to verify these changes are happening. I
was also able to update
tests/EmbeddedDSOsto rely on the newfunctionality.