Skip to content
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

Fix monodroid COMMON_CFLAGS usage and always build with RELEASE. #82

Merged
merged 3 commits into from
Jul 12, 2016

Conversation

atsushieno
Copy link
Contributor

The primary problem was that when app launches it tries to load assemblies:

W/monodroid( 2061): Using override path: /data/data/com.xamarin.android.helloworld/files/.__override__

It should not happen, because we don't have fastdev here.

However, it was weird, that the problem occured even with app Release builds.

Turned out that in src/monodroid, Android.mk referenced COMMON_FLAGS,
which did not exist. Fix typo.

Then, since we don't have fastdev, monodroid needs to be built always
in RELEASE mode. So, add -DRELEASE=1 even for Debug build.

The primary problem was that when app launches it tries to load assemblies:

	W/monodroid( 2061): Using override path: /data/data/com.xamarin.android.helloworld/files/.__override__

It should not happen, because we don't have fastdev here.

However, it was weird, that the problem occured even with app Release builds.

Turned out that in src/monodroid, Android.mk referenced COMMON_FLAGS,
which did not exist. Fix typo.

Then, since we don't have fastdev, monodroid needs to be built always
in RELEASE mode. So, add -DRELEASE=1 even for Debug build.
@jonpryor
Copy link
Member

It should not happen, because we don't have fastdev here.

Why shouldn't it happen? Not having a /t:Install target or having fast deployment within Xamarin.Android.Common.targets is orthogonal to libmonodroid.so supporting the fast deployment directory.

Why can't we do both?

Thus, I see no need to build all versions of libmonodroid.so with RELEASE defined.

(Additionally, because of that message, we do have fastdev, just a limited form. The command-line won't use it, but once the app has been launched once and the fastdev directory created, it becomes entirely possible to manually push files there, which will come in extremely handy for fixing bugs, e.g. put a new mscorlib.dll in the fastdev directory while testing a bug fix...)

The change on line 11 is important. Doh!

@atsushieno
Copy link
Contributor Author

As I mentioned before, current xamarin-android builds apk that NEVER WORKS. It is exactly because of this. And to make it worst, Debug is the DEFAULT behavior.

What you are saying is "if you can manually do the same as existing fastdev e.g. copy all those assemblies to the (formerly-undocumented-and-unknown) directory, it works" which makes no sense to everyone but you.

We need to ship sources that works for everyone.

@jonpryor
Copy link
Member

As I mentioned before, current xamarin-android builds apk that NEVER WORKS

Yes, you mentioned that before. What you didn't mention is how/why it never works, or why defining RELEASE would fix it.

All RELEASE means is that the fastdev directory is not searched for assemblies. That should be all. Looking for assemblies within the fastdev directory should not be a cause of a crash.

Meaning this:

W/monodroid( 2061): Using override path: /data/data/com.xamarin.android.helloworld/files/.__override__

should not be relevant.

What is relevant? This:

F/monodroid: No assemblies found in '/data/user/0/scratch.debugrelease/files/.__override__' or '/storage/emulated/0/Android/data/scratch.debugrelease/files/.__override__'. Assuming this is part of Fast Deployment. Exiting...

That's a problem, and would explain why things don't work.

That message is from monodroid-glue.c:2585:

if (!mono_mkbundle_init && user_assemblies_count == 0 && count_override_assemblies () == 0) {
    log_fatal (LOG_DEFAULT, "No assemblies found in '%s' or '%s'. Assuming this is part of Fast Deployment. Exiting...",
            override_dirs [0],
            (MAX_OVERRIDES > 1 && override_dirs [1] != NULL) ? override_dirs [1] : "<unavailable>");
    exit (FATAL_EXIT_NO_ASSEMBLIES);
}

However, for that code to even be reached, user_assemblies_count must be zero.

THAT is the bug: user_assemblies_count is zero, even though there are assemblies within the .apk:

$ unzip -l bin/Debug/*-Signed.apk | grep assemblies
    64512  06-16-16 10:21   assemblies/Scratch.DebugRelease.dll
     2188  06-16-16 10:21   assemblies/Scratch.DebugRelease.dll.mdb
  1973760  06-16-16 10:21   assemblies/Xamarin.Android.Support.v4.dll
   888832  06-16-16 10:21   assemblies/Xamarin.Android.Support.v7.AppCompat.dll
...

@jonpryor
Copy link
Member

Further investigation shows that while the assemblies are "found", they are skipped here:

if (unzGetCurrentFileInfo (file, &info, cur_entry_name, sizeof (cur_entry_name)-1, NULL, 0, NULL, 0) != UNZ_OK ||
        info.compression_method != 0 ||
        unzOpenCurrentFile3 (file, NULL, NULL, 1, NULL) != UNZ_OK ||
        unzGetRawFileOffset (file, &offset) != UNZ_OK) {
    continue;
}

Indeed, if we look at the .apk:

$ unzip -lv bin/Debug/*-Signed.apk | grep assemblies/
   64512  Defl:N    21394  67%  06-16-16 10:38  6d65706b  assemblies/Scratch.DebugRelease.dll
    2188  Defl:N     1138  48%  06-16-16 10:38  507b14b4  assemblies/Scratch.DebugRelease.dll.mdb
...

We see that they're stored with Defl:N, when they should be Stored.

This impacts everything: environment, typemap.jm, and typemap.mj should also be stored, but they're not.

No wonder nothing works. :-(

@jonpryor
Copy link
Member

Further analysis shows the actual culprit: System.IO.Compression on Mono 4.4 doesn't support CompressionLevel.NoCompression.

Consequently, our work to migrate away from Ionic.Zip -- which didn't work on Linux because it hardcoded \ as the directory separator char! -- toward System.IO.Compression...doesn't work.

@jonpryor
Copy link
Member

Oh, and this is "wonderful": .NET behaves the same as Mono 4.4, meaning CompressionLevel.NoCompression does not mean "store." I'm not sure what it's supposed to mean, but it doesn't mean "store," which is what we had been assuming.

So this isn't a Mono "bug"; it's behaving as .NET does. (Though Mono does ignore the CompressionLevel setting...)

So what's the fix? Ionic.Zip is busted -- at least the NuGet package version -- and System.IO.Compression doesn't work either?

Current plan is for @dellis1972 to attempt to complete @grendello's work at https://github.com/grendello/LibZipSharp, and get a decent .zip processing library...

@atsushieno
Copy link
Contributor Author

That's actually what I thought we needed to investigate too, I was aware of the problem that even with my fix app assemblies were not expanded. Thanks for the investigation, that's great progress.

My fix for this is however to resolve the issue in another level, and the scope of the fix is limited to whatever I stated.

@jonpryor
Copy link
Member

My fix for this is however to resolve the issue in another level, and the scope of the fix is limited to whatever I stated.

I don't know how to interpret this. :-(

To reiterate, for this PR -- PR #82 -- I do not like the change on line 6. I see no reason to always define RELEASE, and I see that as being counterproductive to our own use/experience. We want to have the fast deployment directory while debugging and bug fixing, and it's presence is not preventing apps from working.

The change on line 11 should absolutely happen.

@atsushieno
Copy link
Contributor Author

We want to have the fast deployment directory while debugging and bug fixing

Let's not tell a lie.

We did NOT want to bring the debugging and bugfixing experience in xamarin-android.

If it were totally doable within command line tools, then it should have been part of this xamarin-android sources. It was not included in xamarin-android "because it is NOT part of SDK but is part of IDEs feature".

Since it is NOT part of xamarin-android feature, it does NOT exist.

@atsushieno
Copy link
Contributor Author

How about this: now that we (would) build both debug and release runtimes and it's easily switch between those, leave debug build as is (i.e. for existing "debug" purpose) and we default to Release build in xabuild (add /p:Configuration=Release which will be overridable default value)

@jonpryor
Copy link
Member

jonpryor commented Jul 6, 2016

leave debug build as is (i.e. for existing "debug" purpose) and we default to Release build in xabuild

The problem with this is that it results in an unusable xabuild in the default scenario:

$ make prepare
$ make
$ tools/scripts/xabuild samples/HelloWorld/HelloWorld.csproj
# boom

The current default build only builds the Debug configuration. Changing xabuild to use the Release configuration means that it's unusable, unless you explicitly build the Release configuration first:

$ make CONFIGURATION=Release

I thus don't think that this is a good idea.

@atsushieno
Copy link
Contributor Author

Technically it depends on #105

@jonpryor jonpryor merged commit d7a03f8 into dotnet:master Jul 12, 2016
radical pushed a commit that referenced this pull request May 8, 2018
Rename `reader` variable to `writer`, to match actual use/semantics.

Avoid recurring `<package>` element occurrence: when reading multiple
API XML files which contain the same `//package/@name` value, there
would be multiple `<package/>` elements in the resulting document.

It was almost harmless, but it could result in XPath
query differences.
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Apr 30, 2020
Changes: dotnet/android-tools@12f52ac...f5fcb9f

  * dotnet/android-tools@f5fcb9f: [Xamarin.Android.Tools.AndroidSdk] Add support for cmdline-tools (dotnet#83)
  * dotnet/android-tools@f473ff9: [AndroidSdkWindows] Guard against exception checking registry (dotnet#79)
  * dotnet/android-tools@36d7fee: JetBrains OpenJDK 11 detection (dotnet#82)
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request May 7, 2020
Changes: dotnet/android-tools@12f52ac...f5fcb9f

  * dotnet/android-tools@f5fcb9f: [Xamarin.Android.Tools.AndroidSdk] Add support for cmdline-tools (dotnet#83)
  * dotnet/android-tools@f473ff9: [AndroidSdkWindows] Guard against exception checking registry (dotnet#79)
  * dotnet/android-tools@36d7fee: JetBrains OpenJDK 11 detection (dotnet#82)
jonpryor added a commit that referenced this pull request May 7, 2020
Changes: dotnet/android-tools@12f52ac...f5fcb9f

  * dotnet/android-tools@f5fcb9f: [Xamarin.Android.Tools.AndroidSdk] Add support for cmdline-tools (#83)
  * dotnet/android-tools@f473ff9: [AndroidSdkWindows] Guard against exception checking registry (#79)
  * dotnet/android-tools@36d7fee: JetBrains OpenJDK 11 detection (#82)
jonpryor added a commit that referenced this pull request May 8, 2020
Changes: dotnet/android-tools@12f52ac...23c4fe0

  * dotnet/android-tools@23c4fe0: [Xamarin.Android.Tools.AndroidSdk] Add support for cmdline-tools (#83)
  * dotnet/android-tools@cf9d325: [AndroidSdkWindows] Guard against exception checking registry (#79)
  * dotnet/android-tools@310c5cf: JetBrains OpenJDK 11 detection (#82)
@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.

3 participants