android: fix LLVM version string related building errors

Submitted by Mauro Rossi on April 12, 2019, 11:26 p.m.

Details

Message ID 20190412232644.679-1-issor.oruam@gmail.com
State New
Headers show
Series "android: fix LLVM version string related building errors" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Mauro Rossi April 12, 2019, 11:26 p.m.
Fixes the following building errors:

external/mesa/src/gallium/drivers/r600/r600_pipe_common.c:1290:14:
error: expected ')'
                 ", LLVM " MESA_LLVM_VERSION_STRING
                           ^
<command line>:8:34: note: expanded from here
                                 ^
external/mesa/src/gallium/drivers/r600/r600_pipe_common.c:1287:10:
note: to match this '('
        snprintf(rscreen->renderer_string, sizeof(rscreen->renderer_string),
                ^
1 error generated.

Fixes: 05b114e ("simplify LLVM version string printing")
Signed-off-by: Mauro Rossi <issor.oruam@gmail.com>
---
 Android.mk | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/Android.mk b/Android.mk
index 09139e86d1..b835eb64e9 100644
--- a/Android.mk
+++ b/Android.mk
@@ -97,13 +97,13 @@  define mesa-build-with-llvm
   $(if $(filter $(MESA_ANDROID_MAJOR_VERSION), 4 5), \
     $(warning Unsupported LLVM version in Android $(MESA_ANDROID_MAJOR_VERSION)),) \
   $(if $(filter 6,$(MESA_ANDROID_MAJOR_VERSION)), \
-    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0307 -DMESA_LLVM_VERSION_STRING="3.7")) \
+    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0307 -DMESA_LLVM_VERSION_STRING=\"3.7\")) \
   $(if $(filter 7,$(MESA_ANDROID_MAJOR_VERSION)), \
-    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0700 -DMESA_LLVM_VERSION_STRING="7.0")) \
+    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0700 -DMESA_LLVM_VERSION_STRING=\"7.0\")) \
   $(if $(filter 8,$(MESA_ANDROID_MAJOR_VERSION)), \
-    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0700 -DMESA_LLVM_VERSION_STRING="7.0")) \
+    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0700 -DMESA_LLVM_VERSION_STRING=\"7.0\")) \
   $(if $(filter 9,$(MESA_ANDROID_MAJOR_VERSION)), \
-    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0309 -DMESA_LLVM_VERSION_STRING="3.9")) \
+    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0309 -DMESA_LLVM_VERSION_STRING=\"3.9\")) \
   $(eval LOCAL_SHARED_LIBRARIES += libLLVM70)
 endef
 

Comments

Just a message to Eric,

as per our previous private thread,

I've checked that the Android build works,
but we use libLLVM70 name in library dependency.

Please adapt and apply the patch to mesa dev branch,
to fix the breakage

\ prior to " in the LLVM version string value, just touching
Android.mk solves the problem for me

Mauro

On Sat, Apr 13, 2019 at 1:27 AM Mauro Rossi <issor.oruam@gmail.com> wrote:
>
> Fixes the following building errors:
>
> external/mesa/src/gallium/drivers/r600/r600_pipe_common.c:1290:14:
> error: expected ')'
>                  ", LLVM " MESA_LLVM_VERSION_STRING
>                            ^
> <command line>:8:34: note: expanded from here
>                                  ^
> external/mesa/src/gallium/drivers/r600/r600_pipe_common.c:1287:10:
> note: to match this '('
>         snprintf(rscreen->renderer_string, sizeof(rscreen->renderer_string),
>                 ^
> 1 error generated.
>
> Fixes: 05b114e ("simplify LLVM version string printing")
> Signed-off-by: Mauro Rossi <issor.oruam@gmail.com>
> ---
>  Android.mk | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/Android.mk b/Android.mk
> index 09139e86d1..b835eb64e9 100644
> --- a/Android.mk
> +++ b/Android.mk
> @@ -97,13 +97,13 @@ define mesa-build-with-llvm
>    $(if $(filter $(MESA_ANDROID_MAJOR_VERSION), 4 5), \
>      $(warning Unsupported LLVM version in Android $(MESA_ANDROID_MAJOR_VERSION)),) \
>    $(if $(filter 6,$(MESA_ANDROID_MAJOR_VERSION)), \
> -    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0307 -DMESA_LLVM_VERSION_STRING="3.7")) \
> +    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0307 -DMESA_LLVM_VERSION_STRING=\"3.7\")) \
>    $(if $(filter 7,$(MESA_ANDROID_MAJOR_VERSION)), \
> -    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0700 -DMESA_LLVM_VERSION_STRING="7.0")) \
> +    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0700 -DMESA_LLVM_VERSION_STRING=\"7.0\")) \
>    $(if $(filter 8,$(MESA_ANDROID_MAJOR_VERSION)), \
> -    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0700 -DMESA_LLVM_VERSION_STRING="7.0")) \
> +    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0700 -DMESA_LLVM_VERSION_STRING=\"7.0\")) \
>    $(if $(filter 9,$(MESA_ANDROID_MAJOR_VERSION)), \
> -    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0309 -DMESA_LLVM_VERSION_STRING="3.9")) \
> +    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0309 -DMESA_LLVM_VERSION_STRING=\"3.9\")) \
>    $(eval LOCAL_SHARED_LIBRARIES += libLLVM70)
>  endef
>
> --
> 2.20.1
>
On 2019-04-13 at 01:36, Mauro Rossi <issor.oruam@gmail.com> wrote:
> Just a message to Eric,
> 
> as per our previous private thread,

Sorry, I completely forgot to actually send the fix :facepalm:

> 
> I've checked that the Android build works,
> but we use libLLVM70 name in library dependency.

I don't understand the second part is this sentence?

> 
> Please adapt and apply the patch to mesa dev branch,
> to fix the breakage
> 
> \ prior to " in the LLVM version string value, just touching
> Android.mk solves the problem for me

Yes, this is the correct fix (because of the `eval`, if anyone's wondering).

Reviewed-by: Eric Engestrom <eric@engestrom.ch>

Unfortunately, I'm on holiday for a week now. Can you push the fix yourself? Otherwise, Tapani can :)

> 
> Mauro
> 
> On Sat, Apr 13, 2019 at 1:27 AM Mauro Rossi <issor.oruam@gmail.com> wrote:
> >
> > Fixes the following building errors:
> >
> > external/mesa/src/gallium/drivers/r600/r600_pipe_common.c:1290:14:
> > error: expected ')'
> >                  ", LLVM " MESA_LLVM_VERSION_STRING
> >                            ^
> > <command line>:8:34: note: expanded from here
> >                                  ^
> > external/mesa/src/gallium/drivers/r600/r600_pipe_common.c:1287:10:
> > note: to match this '('
> >         snprintf(rscreen->renderer_string, sizeof(rscreen->renderer_string),
> >                 ^
> > 1 error generated.
> >
> > Fixes: 05b114e ("simplify LLVM version string printing")
> > Signed-off-by: Mauro Rossi <issor.oruam@gmail.com>
> > ---
> >  Android.mk | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/Android.mk b/Android.mk
> > index 09139e86d1..b835eb64e9 100644
> > --- a/Android.mk
> > +++ b/Android.mk
> > @@ -97,13 +97,13 @@ define mesa-build-with-llvm
> >    $(if $(filter $(MESA_ANDROID_MAJOR_VERSION), 4 5), \
> >      $(warning Unsupported LLVM version in Android $(MESA_ANDROID_MAJOR_VERSION)),) \
> >    $(if $(filter 6,$(MESA_ANDROID_MAJOR_VERSION)), \
> > -    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0307 -DMESA_LLVM_VERSION_STRING="3.7")) \
> > +    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0307 -DMESA_LLVM_VERSION_STRING=\"3.7\")) \
> >    $(if $(filter 7,$(MESA_ANDROID_MAJOR_VERSION)), \
> > -    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0700 -DMESA_LLVM_VERSION_STRING="7.0")) \
> > +    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0700 -DMESA_LLVM_VERSION_STRING=\"7.0\")) \
> >    $(if $(filter 8,$(MESA_ANDROID_MAJOR_VERSION)), \
> > -    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0700 -DMESA_LLVM_VERSION_STRING="7.0")) \
> > +    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0700 -DMESA_LLVM_VERSION_STRING=\"7.0\")) \
> >    $(if $(filter 9,$(MESA_ANDROID_MAJOR_VERSION)), \
> > -    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0309 -DMESA_LLVM_VERSION_STRING="3.9")) \
> > +    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0309 -DMESA_LLVM_VERSION_STRING=\"3.9\")) \
> >    $(eval LOCAL_SHARED_LIBRARIES += libLLVM70)
> >  endef
> >
> > --
> > 2.20.1
> >
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Hi,

On Sat, Apr 13, 2019 at 6:32 PM Eric Engestrom <eric@engestrom.ch> wrote:
>
> On 2019-04-13 at 01:36, Mauro Rossi <issor.oruam@gmail.com> wrote:
> > Just a message to Eric,
> >
> > as per our previous private thread,
>
> Sorry, I completely forgot to actually send the fix :facepalm:

No problem

>
> >
> > I've checked that the Android build works,
> > but we use libLLVM70 name in library dependency.
>
> I don't understand the second part is this sentence?

The patch I have submitted does not apply to mesa dev
because in android-x86 we build with libLLVM70.so shared dependency,
instead of libLLVM.so

AOSP has is own llvm, we had to do this to avoid library name
collision in Android build system

>
> >
> > Please adapt and apply the patch to mesa dev branch,
> > to fix the breakage
> >
> > \ prior to " in the LLVM version string value, just touching
> > Android.mk solves the problem for me
>
> Yes, this is the correct fix (because of the `eval`, if anyone's wondering).
>
> Reviewed-by: Eric Engestrom <eric@engestrom.ch>
>
> Unfortunately, I'm on holiday for a week now. Can you push the fix yourself? Otherwise, Tapani can :)

Ok, I'll proceed in the push

>
> >
> > Mauro
> >
> > On Sat, Apr 13, 2019 at 1:27 AM Mauro Rossi <issor.oruam@gmail.com> wrote:
> > >
> > > Fixes the following building errors:
> > >
> > > external/mesa/src/gallium/drivers/r600/r600_pipe_common.c:1290:14:
> > > error: expected ')'
> > >                  ", LLVM " MESA_LLVM_VERSION_STRING
> > >                            ^
> > > <command line>:8:34: note: expanded from here
> > >                                  ^
> > > external/mesa/src/gallium/drivers/r600/r600_pipe_common.c:1287:10:
> > > note: to match this '('
> > >         snprintf(rscreen->renderer_string, sizeof(rscreen->renderer_string),
> > >                 ^
> > > 1 error generated.
> > >
> > > Fixes: 05b114e ("simplify LLVM version string printing")
> > > Signed-off-by: Mauro Rossi <issor.oruam@gmail.com>
> > > ---
> > >  Android.mk | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/Android.mk b/Android.mk
> > > index 09139e86d1..b835eb64e9 100644
> > > --- a/Android.mk
> > > +++ b/Android.mk
> > > @@ -97,13 +97,13 @@ define mesa-build-with-llvm
> > >    $(if $(filter $(MESA_ANDROID_MAJOR_VERSION), 4 5), \
> > >      $(warning Unsupported LLVM version in Android $(MESA_ANDROID_MAJOR_VERSION)),) \
> > >    $(if $(filter 6,$(MESA_ANDROID_MAJOR_VERSION)), \
> > > -    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0307 -DMESA_LLVM_VERSION_STRING="3.7")) \
> > > +    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0307 -DMESA_LLVM_VERSION_STRING=\"3.7\")) \
> > >    $(if $(filter 7,$(MESA_ANDROID_MAJOR_VERSION)), \
> > > -    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0700 -DMESA_LLVM_VERSION_STRING="7.0")) \
> > > +    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0700 -DMESA_LLVM_VERSION_STRING=\"7.0\")) \
> > >    $(if $(filter 8,$(MESA_ANDROID_MAJOR_VERSION)), \
> > > -    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0700 -DMESA_LLVM_VERSION_STRING="7.0")) \
> > > +    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0700 -DMESA_LLVM_VERSION_STRING=\"7.0\")) \
> > >    $(if $(filter 9,$(MESA_ANDROID_MAJOR_VERSION)), \
> > > -    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0309 -DMESA_LLVM_VERSION_STRING="3.9")) \
> > > +    $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0309 -DMESA_LLVM_VERSION_STRING=\"3.9\")) \
> > >    $(eval LOCAL_SHARED_LIBRARIES += libLLVM70)
> > >  endef
> > >
> > > --
> > > 2.20.1
> > >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev