GNOME Bugzilla – Bug 796264
Add android CI
Last modified: 2018-05-24 20:36:16 UTC
.
Created attachment 372227 [details] [review] Meson: Always fallback to proxy-libintl subproject An implementation of gettext is required, but it's not always needed e.g. for CI builds.
Created attachment 372228 [details] [review] Meson: Remove legacy code used to write pc files We are using meson's generator now so those variables are not used anymore.
Created attachment 372229 [details] [review] Add Android CI This adds to the CI a cross build for Android NDK r16 API 21 (because thats what GStreamer currently use) for arm64. GNU iconv must be built manually into our docker image because Android NDK doesn't seems to ship it. The latest NDK r17 API 28 has iconv.h but iconv_open() symbol isn't found by the linker. Looks like broken NDK. libffi also needs to be built manually because the meson subproject doesn't support building for Android platform. It needs a recent RC release because latest stable release is 4 years old and fails to build.
Review of attachment 372227 [details] [review]: ::: meson.build @@ +1669,3 @@ +# FIXME: glib-gettext.m4 has much more checks to detect broken/uncompatible +# implementations. This could be extended if issues are found in some platforms. +if cc.has_function('ngettext', prefix : '#include <libintl.h>') Does this also work when proxy-libintl is available in the system/prefix? The definition there is: #define ngettext g_libintl_ngettext So it should work, but I am not sure.
Review of attachment 372228 [details] [review]: lgtm
(In reply to Nirbheek Chauhan from comment #4) > Review of attachment 372227 [details] [review] [review]: > Does this also work when proxy-libintl is available in the system/prefix? > The definition there is: Hm, cc.has_function() won't find proxy-libintl because there is no -lintl, it's for the case it's defined in libc. But the dependency() case should find it, I guess.
(In reply to Xavier Claessens from comment #6) > (In reply to Nirbheek Chauhan from comment #4) > > Review of attachment 372227 [details] [review] [review] [review]: > > Does this also work when proxy-libintl is available in the system/prefix? > > The definition there is: > > Hm, cc.has_function() won't find proxy-libintl because there is no -lintl, > it's for the case it's defined in libc. But the dependency() case should > find it, I guess. I meant find_library()
Comment on attachment 372228 [details] [review] Meson: Remove legacy code used to write pc files Attachment 372228 [details] pushed as e2c154d - Meson: Remove legacy code used to write pc files
Created attachment 372250 [details] [review] Add Android CI This adds to the CI a cross build for Android NDK r16 API 21 (because thats what GStreamer currently use) for arm64. GNU iconv must be built manually into our docker image because Android NDK doesn't seems to ship it. The latest NDK r17 API 28 has iconv.h but iconv_open() symbol isn't found by the linker. Looks like broken NDK. libffi also needs to be built manually because the meson subproject doesn't support building for Android platform. It needs a recent RC release because latest stable release is 4 years old and fails to build.
(In reply to Xavier Claessens from comment #6) > (In reply to Nirbheek Chauhan from comment #4) > > Review of attachment 372227 [details] [review] [review] [review]: > > Does this also work when proxy-libintl is available in the system/prefix? > > The definition there is: > > Hm, cc.has_function() won't find proxy-libintl because there is no -lintl, > it's for the case it's defined in libc. But the dependency() case should > find it, I guess. Actually, this gives false-positive using mingw. cc.has_function() returns true, but when linking libglib-2.0 it says undefined reference to `libintl_textdomain' and many other libintl_ symbols. I checked what the meson has_function() does, and it has 2 steps, first it check if a program links and that one fails, then it checks for builtin and that pass. Smell like a bug in meson?
Created attachment 372267 [details] [review] Meson: Always fallback to proxy-libintl subproject An implementation of gettext is required, but it's not always needed e.g. for CI builds. -- Removed the #include in cc.has_function(), that fix false positives.
Note that Android NDK API 28 (the latest just released a few days ago) has iconv implementation. Sadly they screwed the r17 release and the symbols are missing from their libc. I reported that bug and it will be fixed in r17b release: https://github.com/android-ndk/ndk/issues/702. So we still have to build it manually for the CI until they fix that, and even then I guess we want to keep CI for older NDK API levels.
(In reply to Xavier Claessens from comment #11) > Created attachment 372267 [details] [review] [review] > Meson: Always fallback to proxy-libintl subproject > > An implementation of gettext is required, but it's not always needed > e.g. for CI builds. > > -- > > Removed the #include in cc.has_function(), that fix false positives. How does having a #include cause a false positive? The header exists but the library doesn't?
(In reply to Nirbheek Chauhan from comment #13) > How does having a #include cause a false positive? The header exists but the > library doesn't? Actually that change was needed for Windows CI, you can see it failing there: https://gitlab.gnome.org/xclaesse/glib/-/jobs/35956 The problem is with mingw, it has libintl.h installed and that header has: # define ngettext libintl_ngettext But you need -lintl to have the libintl_ngettext symbol, so cc.has_function() must return false. Meson does this: https://github.com/mesonbuild/meson/blob/master/mesonbuild/compilers/c.py#L682 We fall into the case where __has_builtin is NOT defined, but ngettext IS defined. So that test succeed. If you remove the #include <libintl.h> then you don't have ngettext defined and that test fail with error: '__builtin_ngettext' undeclared. So it goes into the cc.find_library('intl') case, that lib is found, and it will add -lintl and everyone is happy. That's probably a bug in meson, but just removing the header make it work in all cases, since the #include is not needed when using glibc.
Review of attachment 372250 [details] [review]: ::: .gitlab-ci.yml @@ +30,3 @@ - "${CI_PROJECT_DIR}/_coverage" +fedora-meson-android: How about `fedora-meson-android_ndk_r16_arm64` or something like that, to give a bit more information? Though I’m wondering with all these cross-builds if perhaps we should go with a different template, like something in the format `$host-$buildsystem-on-$build`. e.g. `android_ndk_r16-arm64-meson-on-fedora-x86_64`? Then the most important bit (the $host) is read first. @@ +35,3 @@ + - tags + script: + - meson --cross-file=/opt/meson_android_cross_file.txt -Diconv=gnu -Dinternal_pcre=true _build Plus `--buildtype debug --werror`? @@ +36,3 @@ + script: + - meson --cross-file=/opt/meson_android_cross_file.txt -Diconv=gnu -Dinternal_pcre=true _build + - ninja -C _build What about `meson test -C _build`? @@ +39,3 @@ + artifacts: + paths: + - "${CI_PROJECT_DIR}/_build/meson-logs" It would be good to get code coverage support here too, but probably in a follow-up task unless it’s trivial to add now. ::: .gitlab-ci/setup-android-ndk.sh @@ +1,1 @@ +#!/bin/bash Could do with a license header, since this file is likely to be cargo-culted everywhere. @@ +4,3 @@ + +# Download Android NDK +wget --quiet https://dl.google.com/android/repository/android-ndk-r16-linux-x86_64.zip Please check the SHA-512 sum of this download before using it, otherwise we will execute arbitrary code if dl.google.com is compromised. @@ +24,3 @@ + +# Cross build libiconv +wget --quiet http://ftp.gnu.org/pub/gnu/libiconv/libiconv-1.15.tar.gz Same here. @@ +33,3 @@ + +# Cross build libffi +wget --quiet https://github.com/libffi/libffi/releases/download/v3.3-rc0/libffi-3.3-rc0.tar.gz And here.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/1385.