Android: gallium_dri: add include to get "xmlpool/options.h"

Submitted by Mauro Rossi on Dec. 17, 2017, 11:34 p.m.

Details

Message ID 20171217233446.10799-1-issor.oruam@gmail.com
State New
Headers show
Series "Android: gallium_dri: add include to get "xmlpool/options.h"" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Mauro Rossi Dec. 17, 2017, 11:34 p.m.
target.c requires "xmlpool/options.h" generated header
or the following tricky Android building error may appear:

In file included from external/mesa/src/gallium/targets/dri/target.c:1:
In file included from external/mesa/src/gallium/auxiliary/target-helpers/drm_helper.h:185:
out/target/product/x86_64/gen/STATIC_LIBRARIES/libmesa_pipe_radeonsi_intermediates/radeonsi/si_driinfo.h:19:7: error: expected '}'
      DRI_CONF_RADEONSI_ENABLE_SISCHED("false")
      ^
external/mesa/src/gallium/auxiliary/target-helpers/drm_helper.h:182:55: note: to match this '{'
   static const struct drm_conf_ret xml_options_ret = {
                                                      ^
1 error generated.

Fixes: 0f8c5de869 ("radeonsi: prepare for driver-specific driconf options")

Cc: "17.3" <mesa-stable@lists.freedesktop.org>
---
 src/gallium/targets/dri/Android.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/gallium/targets/dri/Android.mk b/src/gallium/targets/dri/Android.mk
index d9923043af..a5b5f92ea0 100644
--- a/src/gallium/targets/dri/Android.mk
+++ b/src/gallium/targets/dri/Android.mk
@@ -26,10 +26,10 @@  LOCAL_PATH := $(call my-dir)
 include $(CLEAR_VARS)
 
 LOCAL_MODULE := gallium_dri
-
+LOCAL_MODULE_CLASS := SHARED_LIBRARIES
 LOCAL_MODULE_RELATIVE_PATH := $(MESA_DRI_MODULE_REL_PATH)
 LOCAL_SRC_FILES := target.c
-
+LOCAL_C_INCLUDES := $(call generated-sources-dir-for,STATIC_LIBRARIES,libmesa_util,,)
 LOCAL_CFLAGS :=
 
 # We need --undefined-version as some functions in dri.sym may be missing

Comments

On 17 December 2017 at 23:34, Mauro Rossi <issor.oruam@gmail.com> wrote:
> target.c requires "xmlpool/options.h" generated header
> or the following tricky Android building error may appear:
>
> In file included from external/mesa/src/gallium/targets/dri/target.c:1:
> In file included from external/mesa/src/gallium/auxiliary/target-helpers/drm_helper.h:185:
> out/target/product/x86_64/gen/STATIC_LIBRARIES/libmesa_pipe_radeonsi_intermediates/radeonsi/si_driinfo.h:19:7: error: expected '}'
>       DRI_CONF_RADEONSI_ENABLE_SISCHED("false")
>       ^
> external/mesa/src/gallium/auxiliary/target-helpers/drm_helper.h:182:55: note: to match this '{'
>    static const struct drm_conf_ret xml_options_ret = {
>                                                       ^
> 1 error generated.
>
> Fixes: 0f8c5de869 ("radeonsi: prepare for driver-specific driconf options")
>
> Cc: "17.3" <mesa-stable@lists.freedesktop.org>
> ---
>  src/gallium/targets/dri/Android.mk | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/targets/dri/Android.mk b/src/gallium/targets/dri/Android.mk
> index d9923043af..a5b5f92ea0 100644
> --- a/src/gallium/targets/dri/Android.mk
> +++ b/src/gallium/targets/dri/Android.mk
> @@ -26,10 +26,10 @@ LOCAL_PATH := $(call my-dir)
>  include $(CLEAR_VARS)
>
>  LOCAL_MODULE := gallium_dri
> -
> +LOCAL_MODULE_CLASS := SHARED_LIBRARIES
>  LOCAL_MODULE_RELATIVE_PATH := $(MESA_DRI_MODULE_REL_PATH)
>  LOCAL_SRC_FILES := target.c
> -
> +LOCAL_C_INCLUDES := $(call generated-sources-dir-for,STATIC_LIBRARIES,libmesa_util,,)
>  LOCAL_CFLAGS :=
>
This is slightly confusing, quick grep shows:
./src/util/Android.mk
...
LOCAL_MODULE := libmesa_util
...
LOCAL_EXPORT_C_INCLUDE_DIRS := $(intermediates)
...

src/gallium/targets/dri/Android.mk
LOCAL_WHOLE_STATIC_LIBRARIES := \
    ...
    libmesa_util \
    ...

src/gallium/targets/dri/Android.mk-     libmesa_loader

- libmesa_util already exports

AKA
 - static lib exports the includes
 - foo_dri.so already pulls the static lib

+ misc: if LOCAL_MODULE_CLASS is omitted we default to SHARED_LIBRARIES

Can you confirm the above are present in the tree you're using? Did it
work with older Android - aka, something in their system broke?


Thanks
Emil
Il 18/dic/2017 17:27, "Emil Velikov" <emil.l.velikov@gmail.com> ha scritto:

On 17 December 2017 at 23:34, Mauro Rossi <issor.oruam@gmail.com> wrote:
> target.c requires "xmlpool/options.h" generated header
> or the following tricky Android building error may appear:
>
> In file included from external/mesa/src/gallium/targets/dri/target.c:1:
> In file included from external/mesa/src/gallium/
auxiliary/target-helpers/drm_helper.h:185:
> out/target/product/x86_64/gen/STATIC_LIBRARIES/libmesa_pipe_
radeonsi_intermediates/radeonsi/si_driinfo.h:19:7: error: expected '}'
>       DRI_CONF_RADEONSI_ENABLE_SISCHED("false")
>       ^
> external/mesa/src/gallium/auxiliary/target-helpers/drm_helper.h:182:55:
note: to match this '{'
>    static const struct drm_conf_ret xml_options_ret = {
>                                                       ^
> 1 error generated.
>
> Fixes: 0f8c5de869 ("radeonsi: prepare for driver-specific driconf
options")
>
> Cc: "17.3" <mesa-stable@lists.freedesktop.org>
> ---
>  src/gallium/targets/dri/Android.mk | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/targets/dri/Android.mk b/src/gallium/targets/dri/
Android.mk
> index d9923043af..a5b5f92ea0 100644
> --- a/src/gallium/targets/dri/Android.mk
> +++ b/src/gallium/targets/dri/Android.mk
> @@ -26,10 +26,10 @@ LOCAL_PATH := $(call my-dir)
>  include $(CLEAR_VARS)
>
>  LOCAL_MODULE := gallium_dri
> -
> +LOCAL_MODULE_CLASS := SHARED_LIBRARIES
>  LOCAL_MODULE_RELATIVE_PATH := $(MESA_DRI_MODULE_REL_PATH)
>  LOCAL_SRC_FILES := target.c
> -
> +LOCAL_C_INCLUDES := $(call generated-sources-dir-for,
STATIC_LIBRARIES,libmesa_util,,)
>  LOCAL_CFLAGS :=
>
This is slightly confusing, quick grep shows:
./src/util/Android.mk
...
LOCAL_MODULE := libmesa_util
...
LOCAL_EXPORT_C_INCLUDE_DIRS := $(intermediates)
...

src/gallium/targets/dri/Android.mk
LOCAL_WHOLE_STATIC_LIBRARIES := \
    ...
    libmesa_util \
    ...

src/gallium/targets/dri/Android.mk-     libmesa_loader

- libmesa_util already exports


AKA

 - static lib exports the includes
 - foo_dri.so already pulls the static lib


All confirmed and it's strange to us too, but when checking out Mesa 17.3
and 17.4 branches we, i.e. me and Quiang Yu, get the error.

Maybe Qiang could explain, I just added the only possibile dependency.


+ misc: if LOCAL_MODULE_CLASS is omitted we default to SHARED_LIBRARIES


LOCAL_MODULE_CLASS is required to be able to use generated-sources-dir-for
macro


Can you confirm the above are present in the tree you're using? Did it
work with older Android - aka, something in their system broke?


Same issue happening with Nougat, when checking out 17.3 or 17.4
Cheers



Thanks
Emil
I met this problem when upgrade mesa 17.2 to 17.3, and build without
a make clean. A clean build won't have this problem.

Regards,
Qiang
On Mon, Dec 18, 2017 at 8:45 PM, Yu, Qiang <Qiang.Yu@amd.com> wrote:
> I met this problem when upgrade mesa 17.2 to 17.3, and build without
> a make clean. A clean build won't have this problem.

I think I'm seeing a similar problem on CI builds with current master,
but with intel_screen.c:

external/mesa3d/src/mesa/drivers/dri/i965/intel_screen.c:60:7: error:
expected '}'
      DRI_CONF_MESA_NO_ERROR("false")
      ^

For the CI jobs, it is dependent on which build machine the job runs on.


In the past, we've had some cross module dependency problems with
generated files which were fixed with:

LOCAL_GENERATED_SOURCES += $(MESA_DRI_OPTIONS_H)

I think this is needed because we need a dependency on the source
file, not just the include directory. libmesa_pipe_radeonsi is missing
this. However, with the problem I'm hitting, this dependency is there.

Rob
Hi,
I tried to add LOCAL_GENERATED_SOURCES += $(MESA_DRI_OPTIONS_H)

incrementally three places:

src/gallium/drivers/radeonsi/Android.mk
src/gallium/targets/dri/Android.mk
and src/gallium/auxiliary/Android.mk

but it didn't work as expected

I also tried by putting later the export in src/util/Android.mk
after local generation rules for  $(MESA_DRI_OPTIONS_H).
but it has no effect.

BTW the include is present in automake rules [1]
as part of GALLIUM_TARGET_CFLAGS [2]
which includes -I$(top_builddir)/src/util/

so the patch is at least in conformance with automake rules for gallium_dri
target.
If you see better ways please send me istructions

Mauro

[1]
https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/targets/dri/Makefile.am

[2]  https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/Automake.inc




2017-12-19 23:38 GMT+01:00 Rob Herring <robh@kernel.org>:

> On Mon, Dec 18, 2017 at 8:45 PM, Yu, Qiang <Qiang.Yu@amd.com> wrote:
> > I met this problem when upgrade mesa 17.2 to 17.3, and build without
> > a make clean. A clean build won't have this problem.
>
> I think I'm seeing a similar problem on CI builds with current master,
> but with intel_screen.c:
>
> external/mesa3d/src/mesa/drivers/dri/i965/intel_screen.c:60:7: error:
> expected '}'
>       DRI_CONF_MESA_NO_ERROR("false")
>       ^
>
> For the CI jobs, it is dependent on which build machine the job runs on.
>
>
> In the past, we've had some cross module dependency problems with
> generated files which were fixed with:
>
> LOCAL_GENERATED_SOURCES += $(MESA_DRI_OPTIONS_H)
>
> I think this is needed because we need a dependency on the source
> file, not just the include directory. libmesa_pipe_radeonsi is missing
> this. However, with the problem I'm hitting, this dependency is there.
>
> Rob
>