[Mesa-dev] [RFC 2/2] mesa: Android: enable altxmlconfig for O+

[Sumit Semwal]

Hello Dylan,

On Fri, 10 May 2019 at 22:57, Dylan Baker <dylan at pnwbakers.com> wrote:
>
> Hi Sumit,
>
> I haven't looked too closely at this, but there are a couple of things right
> away that jumped out to me. I've noted them inline below
Thanks for your quick review.
>
> Quoting Sumit Semwal (2019-05-09 23:02:57)
> > Android version O and later would like to use altxmlconfig
> > instead of libexpat based parsing.
> >
> > We enable altxmlconfig instead of xmlconfig.c for Android O+.
> >
> > Change-Id: Iaa51b2a1e9c16a1fdf6f120104429b19dc68f9eb
>
> Please do not include "Change-Id:" tags, we don't use them in mesa.
Sure, I will remove them for the next iteration.
>
> > Signed-off-by: Sumit Semwal <sumit.semwal at linaro.org>
> > ---
> >  src/gallium/targets/dri/Android.mk |  8 +++-----
> >  src/mesa/drivers/dri/Android.mk    | 12 +++++-------
> >  src/util/Android.mk                | 12 +++++-------
> >  src/util/Makefile.sources          |  2 +-
> >  4 files changed, 14 insertions(+), 20 deletions(-)
> >
> > diff --git a/src/gallium/targets/dri/Android.mk b/src/gallium/targets/dri/Android.mk
> > index 10b27a80ef3e..da462ac1230d 100644
> > --- a/src/gallium/targets/dri/Android.mk
> > +++ b/src/gallium/targets/dri/Android.mk
> > @@ -44,11 +44,9 @@ LOCAL_SHARED_LIBRARIES := \
> >         libglapi \
> >         libz
> >
> > -# If Android version >=8 MESA should static link libexpat else should dynamic link
> > -ifeq ($(shell test $(PLATFORM_SDK_VERSION) -ge 27; echo $$?), 0)
> > -LOCAL_STATIC_LIBRARIES := \
> > -       libexpat
> > -else
> > +# If Android version <8 MESA should dynamic link libexpat
> > +# version >= 8 would use altxmlconfig instead
> > +ifneq ($(shell test $(PLATFORM_SDK_VERSION) -ge 27; echo $$?), 0)
> >  LOCAL_SHARED_LIBRARIES += \
> >         libexpat
> >  endif
> > diff --git a/src/mesa/drivers/dri/Android.mk b/src/mesa/drivers/dri/Android.mk
> > index 60c8476a38a7..eefcb2475c6b 100644
> > --- a/src/mesa/drivers/dri/Android.mk
> > +++ b/src/mesa/drivers/dri/Android.mk
> > @@ -53,13 +53,11 @@ MESA_DRI_SHARED_LIBRARIES := \
> >         liblog \
> >         libz
> >
> > -# If Android version >=8 MESA should static link libexpat else should dynamic link
> > -ifeq ($(shell test $(PLATFORM_SDK_VERSION) -ge 27; echo $$?), 0)
> > -MESA_DRI_WHOLE_STATIC_LIBRARIES += \
> > -       libexpat
> > -else
> > -MESA_DRI_SHARED_LIBRARIES += \
> > -       libexpat
> > +# If Android version <8 MESA should dynamic link libexpat
> > +# version >= 8 would use altxmlconfig instead
> > +ifneq ($(shell test $(PLATFORM_SDK_VERSION) -ge 27; echo $$?), 0)
> > +LOCAL_SHARED_LIBRARIES += \
> > +        libexpat
> >  endif
> >
> >  #-----------------------------------------------
> > diff --git a/src/util/Android.mk b/src/util/Android.mk
> > index 6d770ca95756..c8cfdae91e6a 100644
> > --- a/src/util/Android.mk
> > +++ b/src/util/Android.mk
> > @@ -41,13 +41,11 @@ LOCAL_C_INCLUDES := \
> >         $(MESA_TOP)/src/gallium/include \
> >         $(MESA_TOP)/src/gallium/auxiliary
> >
> > -# If Android version >=8 MESA should static link libexpat else should dynamic link
> > -ifeq ($(shell test $(PLATFORM_SDK_VERSION) -ge 27; echo $$?), 0)
> > -LOCAL_STATIC_LIBRARIES := \
> > -       libexpat
> > -else
> > -LOCAL_SHARED_LIBRARIES := \
> > -       libexpat
> > +# If Android version <8 MESA should dynamic link libexpat
> > +# version >= 8 would use altxmlconfig instead
> > +ifneq ($(shell test $(PLATFORM_SDK_VERSION) -ge 27; echo $$?), 0)
> > +LOCAL_SHARED_LIBRARIES += \
> > +        libexpat
> >  endif
> >
> >  LOCAL_MODULE := libmesa_util
> > diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources
> > index ad8c56d3d1b7..617cc272d340 100644
> > --- a/src/util/Makefile.sources
> > +++ b/src/util/Makefile.sources
> > @@ -88,5 +88,5 @@ MESA_UTIL_GENERATED_FILES = \
> >         format_srgb.c
> >
> >  XMLCONFIG_FILES := \
> > -       xmlconfig.c \
> > +       altxmlconfig.c \
> >         xmlconfig.h
>
> Even though we've deleted autotools, scons still uses the Makefile.sources, have
> you run a scons build on Linux to make sure that things are still working
> correctly?
Ah, I didn't realise. In the next iteration, I will create a separate
target for the new file, so that existing users aren't affected.
>
> Dylan

Best,
Sumit.