GNOME Bugzilla – Bug 788273
fix build failure for Android Nougat
Last modified: 2018-05-16 06:46:11 UTC
if 'target_distro_version = DistroVersion.ANDROID_NOUGAT' is set, gettext and openh264 fail to build.
Created attachment 360579 [details] [review] gettext: Unset checking FSETLOCKING for ANDROID With Android Nougat, it fails to build because of missing 'stdio_ext.h'. If CFLAGS has '-I<NDK path>/sysroot/usr/include', gettext can be built again. However, it causes build problems for the other recipe. Disabling FSETLOCKING is much easier to build all recipes.
Created attachment 360580 [details] [review] [1/3] openh264: Use correct Android platform version for Nougat
Comment on attachment 360579 [details] [review] gettext: Unset checking FSETLOCKING for ANDROID How does it fail to build after adding the include path? It might make sense to switch from gettext to libintl-stub
> How does it fail to build after adding the include path? I didn't capture the log so I tried to build this again. With the include path, gettext is built, but glib is failed to find 'libintl.h'. > It might make sense to switch from gettext to libintl-stub I'm not sure what 'libintl-stub' is. is it an option or alternative of gettext?
Created attachment 360720 [details] [review] android: Update to ndk 15c to use sysroot path (In reply to Justin J. Kim from comment #4) > > How does it fail to build after adding the include path? > > I didn't capture the log so I tried to build this again. > With the include path, gettext is built, but glib is failed to find > 'libintl.h'. I got a false result because I used two different versions of NDKs at the same time. Please, ignore my last comment. I did mistake in local while testing the other stuffs for Nougat. Android NDK 13b doesn't provide "sysroot", but the version 15c does. If sysroot path is added to CFLAGS, the gettext problem is solved.
Created attachment 360721 [details] [review] gettext: Add sysroot include path to CFLAGS for android build
Created attachment 360817 [details] [review] gettext: Add sysroot include path to CFLAGS for android build Suppress warning by "-Wno-unused-but-set-variable" too.
I tried to build from clone again to be sure what I was doing correctly. If I use `-I<NDK path>/sysroot/usr/include` (not <NDK path>/platforms/android-xx/), cebero fails to build in iconv, gnutls. iconv. ``` aarch64-linux-android-gcc -fuse-ld=gold -DHAVE_CONFIG_H -DEXEEXT=\"\" -I. -I.. -I../lib -I../intl -DDEPENDS_ON_LIBICONV=1 -DDEPENDS_ON_LIBINTL=1 --sysroot=/Users/justin/git/cerbero/build/android-ndk-r15c/platforms/android-24/arch-arm64 -I/Users/justin/git/cerbero/build/android-ndk-r15c/platforms/android-24/arch-arm64/usr/include -DANDROID -DPIC -Wall -g -Os --sysroot=/Users/justin/git/cerbero/build/android-ndk-r15c/platforms/android-24/arch-arm64 -I/Users/justin/git/cerbero/build/android-ndk-r15c/sysroot/usr/include -I/Users/justin/git/cerbero/build/android-ndk-r15c/platforms/android-24/arch-arm64/usr/include -ffunction-sections -funwind-tables -fstack-protector -no-canonical-prefixes -fPIC -DANDROID -DPIC -Wa,--noexecstack -c allocator.c In file included from /Users/justin/git/cerbero/build/android-ndk-r15c/sysroot/usr/include/malloc.h:22:0, from /Users/justin/git/cerbero/build/android-ndk-r15c/sysroot/usr/include/stdlib.h:36, from ./stdlib.h:35, from allocator.c:4: ./stdio.h:1010:1: error: 'gets' undeclared here (not in a function) _GL_WARN_ON_USE (gets, "gets is a security hole - use fgets instead"); ^ make[2]: *** [allocator.o] Error 1 make[1]: *** [all] Error 2 make: *** [all] Error 2 ``` gnutls ``` random.c:36:1: error: invalid initializer static atomic_uint rnd_initialized = 0; ^ In file included from random.c:26:0: random.c: In function '_gnutls_rnd_init': random.c:40:15: error: wrong type argument to unary exclamation mark if (unlikely(!rnd_initialized)) { ^ gnutls_int.h:77:42: note: in definition of macro 'unlikely' #define unlikely(x) __builtin_expect((x), 0) ^ random.c:42:20: error: incompatible types when assigning to type 'atomic_uint' from type 'int' rnd_initialized = 1; ^ random.c:47:7: error: wrong type argument to unary exclamation mark if (!rnd_initialized) { ^ random.c:53:20: error: incompatible types when assigning to type 'atomic_uint' from type 'int' rnd_initialized = 1; ^ random.c: In function '_gnutls_rnd_deinit': random.c:110:6: error: used struct type value where scalar is required if (rnd_initialized && _gnutls_rnd_ops.deinit != NULL) { ^ random.c:113:18: error: incompatible types when assigning to type 'atomic_uint' from type 'int' rnd_initialized = 0; ^ random.c: In function 'gnutls_rnd_refresh': random.c:165:6: error: used struct type value where scalar is required if (rnd_initialized && _gnutls_rnd_ops.rnd_refresh) ^ make[4]: *** [random.lo] Error 1 ``` It seems to be caused because NDK headers have different definitions than usual headers. For Android N, FSETLOCKING should be disabled in gettext. But, still I wonder what is 'libintl-stub'?
https://github.com/centricular/proxy-libintl is what I meant.
See also https://bugzilla.gnome.org/show_bug.cgi?id=774803
*** Bug 774803 has been marked as a duplicate of this bug. ***
Created attachment 361935 [details] [review] [1/3] proxy-libintl: add the alternative of gettext
Created attachment 361936 [details] [review] [2/3] gettext: removed and replaced with proxy-libintl
Created attachment 361937 [details] [review] [3/3] android: Update to ndk r15c
Regarding proxy-libintl, I added direct shell command to build it. Please, review and give me advice if there's a nicer way.
Here's working branch on my personal repository. https://gitlab.collabora.com/joykim/cerbero/commits/bgo/788273
Created attachment 361962 [details] [review] [1/3] proxy-libintl: add the alternative of gettext add a static library.
Review of attachment 361962 [details] [review]: This project uses the meson build system so it should be built with meson + ninja. However, since we don't have a mechanism for ensuring that those are available during build (I have a WIP branch, but it's not ready to merge), and because this is a simple project, we can make do without it. Have you tested that everything is able to build against it and work just fine on Android? I only tested this on Windows and Linux in my branch.
(In reply to Nirbheek Chauhan from comment #18) > Review of attachment 361962 [details] [review] [review]: > Have you tested that everything is able to build against it and work just > fine on Android? Yes, I've checked it works correctly on Android. > I only tested this on Windows and Linux in my branch. How about MacOS and iOS? I wonder whether I should use '.dylib' for them or 'so' is just okay.
(In reply to Justin J. Kim from comment #19) > > I only tested this on Windows and Linux in my branch. > > How about MacOS and iOS? I wonder whether I should use '.dylib' for them or > 'so' is just okay. Oh, I didn't test *this* patch, I tested my own branch, which means I know that proxy-libintl itself works on Windows and Linux. You do need to change the extensions to match the OS, yes. For Windows this should be `.dll` and on macOS it should be `.dylib`. I think on iOS we use static libraries everywhere, but I am not sure. Does this handle multiarch properly? That is needed on iOS too.
See bug 789316, if that is merged you should be able to run `meson` in configure() and `ninja` in compile().
Created attachment 363197 [details] [review] [1/5] proxy-libintl: add the alternative of gettext
Created attachment 363198 [details] [review] [2/5] glib: Drop-checking-gettext
Created attachment 363199 [details] [review] [3/5] gettext: remove runtime_deps
Created attachment 363200 [details] [review] [4/5] remove gettext from packages/base-system
Created attachment 363201 [details] [review] [5/5] android: Update to ndk r15c
Created attachment 363206 [details] [review] [2/5] glib: Drop-checking-gettext
I know we shouldn't use a random master of a library, but currently, proxy-libintl doesn't have any release. @Nirbheek Chauhan, When do you make a release of 'proxy-libintl'? I tested my patches were built and run well on Android. For the other target platforms, I let them use gettext.
There are no releases of proxy-libintl. It hasn't really seen any changes since the initial version. I can make a github release if you need it for Cerbero.
(In reply to Nirbheek Chauhan from comment #29) > There are no releases of proxy-libintl. It hasn't really seen any changes > since the initial version. I can make a github release if you need it for > Cerbero. I saw you added a patch to support a static library for porxy-libintl on github. Can you please make a release? Then I can update patch to see the release version again.
(In reply to Justin J. Kim from comment #30) > I saw you added a patch to support a static library for porxy-libintl on > github. Can you please make a release? Then I can update patch to see the > release version again. https://github.com/centricular/proxy-libintl/releases/tag/0.1
Created attachment 365342 [details] [review] [2/3] proxy-libintl: add the alternative of gettext Add proxy-libintl 0.1
Created attachment 365343 [details] [review] [2/5] glib: Drop-checking-gettext rebased.
Created attachment 365344 [details] [review] [3/5] gettext: remove runtime_deps rebased and fixed conflict
Created attachment 365345 [details] [review] [4/5] remove gettext from packages/base-system rebased.
Created attachment 365346 [details] [review] [5/5] android: Update to ndk r15c rebased.
Review of attachment 365345 [details] [review]: ::: packages/base-system-1.0.package @@ +24,3 @@ Platform.DARWIN: ['gettext:libs:lang', 'bionic-fixup:libs'], Platform.LINUX: [], + Platform.ANDROID: ['proxy-libintl:libs:lang', 'libiconv:libs:lang', 'gnustl', Should do the same for iOS
Created attachment 365516 [details] [review] [2/5] glib: Drop checking gettext if Android or iOS
Created attachment 365517 [details] [review] [3/5] gettext: Remove runtime_deps if Android or iOS
Created attachment 365518 [details] [review] [4/5] base-system: Remove gettext if Android or iOS
Comment on attachment 365346 [details] [review] [5/5] android: Update to ndk r15c r15c is old already, also see https://bugzilla.gnome.org/show_bug.cgi?id=790753
Review of attachment 365516 [details] [review]: ::: recipes/glib.recipe @@ +111,3 @@ self.make = '%s private_LDFLAGS=""' % self.make + self.deps += ['proxy-libintl'] + self.patches += ['glib/0001-configure.ac-Assuming-that-proxy-libintl-is-used.patch'] Conditional patches are not ideal. Can you make this a patch that is always applied?
Comment on attachment 365518 [details] [review] [4/5] base-system: Remove gettext if Android or iOS Please also squash all the libproxy-intl commits together, they must all be merged at once. Between them, everything would fail to build or work.
Comment on attachment 365346 [details] [review] [5/5] android: Update to ndk r15c Move to a newer version.
Created attachment 365572 [details] [review] [3/3] use proxy-libintl for android and ios
Comment on attachment 365572 [details] [review] [3/3] use proxy-libintl for android and ios Looks good, now just waiting for meson support in cerbero
Did you submitted your glib patch upstream? IMHO we should try to have a link to upstream bug for every patch we include in cerbero. It's a nigthmare to update glib version because we have no references at all atm.
(In reply to Xavier Claessens from comment #47) > Did you submitted your glib patch upstream? yes, it was upstreamed[0]. > IMHO we should try to have a link to upstream bug for every patch we include in cerbero. The backported patch[1] in cerbero has the upstream link in its content, but I don't have better idea to let comitters know the reference. [0] https://bugzilla.gnome.org/show_bug.cgi?id=788270 [1] https://bugzilla.gnome.org/show_bug.cgi?id=790058
Created attachment 370774 [details] [review] [1/3] openh264: Use correct Android platform version for Nougat Rebased onto master to fix a conflict.
Created attachment 370776 [details] [review] [3/3] use proxy-libintl for android and ios Added libsoup patch not to check intl version.
These should be good to go now that we have meson support ?
Review of attachment 365342 [details] [review]: ::: recipes/proxy-libintl.recipe @@ +9,3 @@ + stype = SourceType.GIT + btype = BuildType.MESON + remotes = {'origin': 'https://github.com/centricular/proxy-libintl.git'} I have talked to the good folks at the Frida project, from where I had initially forked this. They have created a new repository for this. We should use that: https://github.com/frida/proxy-libintl The 0.1 tag should continue to work.
Created attachment 371880 [details] [review] [2/3] proxy-libintl: add the alternative of gettext Changed fetch url.
These patches are built successfully on my environment. please review and accept them if applicable.