[Mesa-dev] [PATCH] gallium: Fix automake for Android

[Tomasz Figa]

Hi Chad,

On Tue, Jun 19, 2018 at 3:46 PM Chad Versace <chadversary at chromium.org> wrote:
>
> Chromium OS uses Autotools and pkg-config when building Mesa for
> Android. The gallium drivers were failing to find the headers and
> libraries for zlib and Android's libbacktrace.
> ---
>  configure.ac                      | 6 +++++-
>  src/gallium/Automake.inc          | 5 +++++
>  src/gallium/auxiliary/Makefile.am | 4 ++++
>  3 files changed, 14 insertions(+), 1 deletion(-)
>

Please see my comments inline.

> diff --git a/configure.ac b/configure.ac
> index 7a0e4754208..5f0792cd8ef 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1830,7 +1830,11 @@ for plat in $platforms; do
>          ;;
>
>      android)
> -        PKG_CHECK_MODULES([ANDROID], [cutils hardware sync])
> +        android_modules='cutils hardware sync zlib'

What's Android-specific in zlib? We already check higher for zlib,
shouldn't we just add ZLIB_CFLAGS/LIBS to the respective Makefile.in?

> +        if test -n "$with_gallium_drivers"; then
> +            android_modules+=' backtrace'

I'd also say this should have a separate PKG_CHECK_MODULES clause, say
"BACKTRACE" and BACKTRACE_CFLAGS/LIBS added to respective Makefile.in.
Especially since we only need libbacktrace in u_debug_stack*.

> +        fi
> +        PKG_CHECK_MODULES([ANDROID], ["$android_modules"])
>          DEFINES="$DEFINES -DHAVE_ANDROID_PLATFORM"
>          ;;
>
> diff --git a/src/gallium/Automake.inc b/src/gallium/Automake.inc
> index 3e21aa71b5c..8181927a396 100644
> --- a/src/gallium/Automake.inc
> +++ b/src/gallium/Automake.inc
> @@ -59,6 +59,11 @@ GALLIUM_COMMON_LIB_DEPS += \
>         $(LIBDRM_LIBS)
>  endif
>
> +if HAVE_PLATFORM_ANDROID
> +GALLIUM_COMMON_LIB_DEPS += \
> +       $(ANDROID_LIBS)
> +endif
> +
>  GALLIUM_WINSYS_CFLAGS = \
>         -I$(top_srcdir)/src \
>         -I$(top_srcdir)/include \
> diff --git a/src/gallium/auxiliary/Makefile.am b/src/gallium/auxiliary/Makefile.am
> index 6b048b8eebf..03908198772 100644
> --- a/src/gallium/auxiliary/Makefile.am
> +++ b/src/gallium/auxiliary/Makefile.am
> @@ -21,6 +21,10 @@ libgallium_la_SOURCES = \
>         $(NIR_SOURCES) \
>         $(GENERATED_SOURCES)
>
> +if HAVE_PLATFORM_ANDROID
> +libgallium_la_SOURCES += util/u_debug_stack_android.cpp

Shouldn't we add ANDROID_CFLAGS and ANDROID_LIBS here rather than in
src/gallium/Automake.inc, since this is the module that actually needs
them?

Best regards,
Tomasz