1324331 - Build fails if MOZ_NATIVE_DEVICES is not set

Status: RESOLVED FIXED

(Firefox Build System :: Android Studio and Gradle Integration, defect)

Description

[Matías Zúñiga]

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20161213225203

Steps to reproduce:

Tried to build the last fennec beta, with a custom configure.sh that unsets MOZ_NATIVE_DEVICES. (also reproduced in trunk)



Actual results:

build fails with
 *****************************************************
 ***     Error: The Fennec JNI code has changed    ***
 * To update generated code in the tree, please run  *

  make -C /my-build-dir/obj-arm-linux-androideabi/mobile/android/base update-fennec-wrappers

 * Repeat the build, and check in any changes.       *
 *****************************************************

it seems that this happens since bug 1294479 was merged (not verified). 
the auto-generated bindings has stuff from PresentationMediaPlayerManager.java, but that file is only included when MOZ_NATIVE_DEVICES is set, so the build-time-generated bindings differs and the build fails.


Expected results:

build should finish without errors, as with the latest release

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Matías Zúñiga from comment #0)
> it seems that this happens since bug 1294479 was merged (not verified). 
> the auto-generated bindings has stuff from
> PresentationMediaPlayerManager.java, but that file is only included when
> MOZ_NATIVE_DEVICES is set, so the build-time-generated bindings differs and
> the build fails.

Jim will know.

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

Ideally we would generate fresh bindings when building, but for now, we can either split those methods into separate files, or add some kind of support for include guards to the binding generator.

Comment 3

[Jan Henning [:JanH]]

Not sure if we still need to do something for Beta now that a few versions have passed, but I'm not seeing JNI related problems any more (at least when using ./mach build with --with-gradle).

Instead, there are two other problems:
- The easy one: PresentationMediaPlayerManager.java and RemotePresentationService.java need to be added here (https://dxr.mozilla.org/mozilla-central/rev/bdb2387396b4a74dfefb7c983733eed3625e906a/mobile/android/app/build.gradle#147)
- The slightly more difficult one: The custom tab ActionBarPresenter is using R.drawable.ic_close_light (https://dxr.mozilla.org/mozilla-central/rev/bdb2387396b4a74dfefb7c983733eed3625e906a/mobile/android/base/java/org/mozilla/gecko/customtabs/ActionBarPresenter.java#208), which doesn't work with MOZ_NATIVE_DEVICES unset

Comment 4

[Matías Zúñiga]

Attachment: Build with mediarouter-v7 even without MOZ_NATIVE_DEVICES.

Yeah, the JNI problem is still a thing.

For the ActionBarPresenter problem, the drawable comes from android.support.v7.mediarouter (not included if MOZ_NATIVE_DEVICES is not set). For that I'm using the attached patch... I'm adding ~0.2 MiB just for a drawable, but its the easy fix.

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8893191 [details] [diff] [review]
Build with mediarouter-v7 even without MOZ_NATIVE_DEVICES.

Review of attachment 8893191 [details] [diff] [review]:
-----------------------------------------------------------------

This part looks good. But you will have to mirror this change in the build.gradle files (Follow MOZ_NATIVE_DEVICES).

Comment 6

[Matías Zúñiga]

Attachment: Build with mediarouter-v7 even without MOZ_NATIVE_DEVICES.

Comment 7

[Matías Zúñiga]

Attachment: Add missing excludes when building without MOZ_NATIVE_DEVICES.

Fix the other issue reported by :JanH

Now trunk can be built without MOZ_NATIVE_DEVICES, but just with gradle (because of the JNI issue)

Comment 8

[Tad]

Attachment: Build with mediarouter-v7 even without MOZ_NATIVE_DEVICE

This is attachment 8895219 [details] [diff] [review] rebased on the latest master as of 2017-12-18

Comment 9

[Tad]

I have rebased attachement 8895219 and tested these two patches to work.

Comment 10

[Nick Alexander :nalexander [he/him]]

I've closed this as part of Bug 1419581.  Thanks for all your work on this, Tad and Matias!