diff mbox

[v4] CMake: define GBM_BO_MAP only when symbol is found

Message ID 1494898079-20284-1-git-send-email-daniel.diaz@linaro.org
State Superseded
Headers show

Commit Message

Daniel Díaz May 16, 2017, 1:27 a.m. UTC
gbm_bo_map() and _unmap() have been added recently to Mesa,
and this update may not have reached all implementations of
GBM, such as the one provided by Mali r6, where said
definitions can be found in the header file but not in the
library itself. This leads to errors like the following when
linking:
  ../../../../lib/libpiglitutil_gl.so.0: undefined reference to `gbm_bo_unmap'
  ../../../../lib/libpiglitutil_gl.so.0: undefined reference to `gbm_bo_map'
  collect2: error: ld returned 1 exit status
  make[2]: *** [bin/point-sprite] Error 1

Instead of relying on the header file, actually try to link
using that symbol to determine if PIGLIT_HAS_GBM_BO_MAP
should be defined.

Signed-off-by: Daniel Díaz <daniel.diaz@linaro.org>
---
v2: Use CHECK_SYMBOL_EXISTS as suggested by Jan Vesely.
v3: Back to CHECK_FUNCTION_EXISTS.
v4: Use ${GBM_LIBRARIES} instead of 'gbm'.

 CMakeLists.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Jan Vesely May 16, 2017, 8:06 p.m. UTC | #1
On Mon, 2017-05-15 at 20:27 -0500, Daniel Díaz wrote:
> gbm_bo_map() and _unmap() have been added recently to Mesa,

> and this update may not have reached all implementations of

> GBM, such as the one provided by Mali r6, where said

> definitions can be found in the header file but not in the

> library itself. This leads to errors like the following when

> linking:

>   ../../../../lib/libpiglitutil_gl.so.0: undefined reference to `gbm_bo_unmap'

>   ../../../../lib/libpiglitutil_gl.so.0: undefined reference to `gbm_bo_map'

>   collect2: error: ld returned 1 exit status

>   make[2]: *** [bin/point-sprite] Error 1

> 

> Instead of relying on the header file, actually try to link

> using that symbol to determine if PIGLIT_HAS_GBM_BO_MAP

> should be defined.

> 

> Signed-off-by: Daniel Díaz <daniel.diaz@linaro.org>

> ---

> v2: Use CHECK_SYMBOL_EXISTS as suggested by Jan Vesely.

> v3: Back to CHECK_FUNCTION_EXISTS.

> v4: Use ${GBM_LIBRARIES} instead of 'gbm'.

> 

>  CMakeLists.txt | 5 +++--

>  1 file changed, 3 insertions(+), 2 deletions(-)

> 

> diff --git a/CMakeLists.txt b/CMakeLists.txt

> index a4ff99e..cc26fa8 100644

> --- a/CMakeLists.txt

> +++ b/CMakeLists.txt

> @@ -141,8 +141,9 @@ IF(${CMAKE_SYSTEM_NAME} MATCHES "Linux")

>  	if(GBM_FOUND)

>  		set(PIGLIT_HAS_GBM True)

>  		add_definitions(-DPIGLIT_HAS_GBM)

> -		if (GBM_VERSION VERSION_EQUAL "12.1" OR GBM_VERSION VERSION_GREATER "12.1")

> -			set(PIGLIT_HAS_GBM_BO_MAP True)

you still need to preserve the above line ^^, otherwise it breaks the
check on line 197.

Jan
> +		set(CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES} ${GBM_LIBRARIES})

> +		CHECK_FUNCTION_EXISTS(gbm_bo_map PIGLIT_HAS_GBM_BO_MAP)

> +		if (PIGLIT_HAS_GBM_BO_MAP)

>  			add_definitions(-DPIGLIT_HAS_GBM_BO_MAP)

>  		endif()

>  	endif(GBM_FOUND)
Daniel Díaz May 16, 2017, 8:34 p.m. UTC | #2
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hello!


On 05/16/2017 03:06 PM, Jan Vesely wrote:
> On Mon, 2017-05-15 at 20:27 -0500, Daniel Díaz wrote:
>> diff --git a/CMakeLists.txt b/CMakeLists.txt index 
>> a4ff99e..cc26fa8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt
>> @@ -141,8 +141,9 @@ IF(${CMAKE_SYSTEM_NAME} MATCHES "Linux")
>> if(GBM_FOUND) set(PIGLIT_HAS_GBM True) 
>> add_definitions(-DPIGLIT_HAS_GBM) -		if (GBM_VERSION 
>> VERSION_EQUAL "12.1" OR GBM_VERSION VERSION_GREATER "12.1") - 
>> set(PIGLIT_HAS_GBM_BO_MAP True)
> you still need to preserve the above line ^^, otherwise it breaks 
> the check on line 197.

But it is still defined, right? The CheckSymbolExists module defines
PIGLIT_HAS_GBM_BO_MAP which can be used in the condition down below,
in true ("1") or false ("") form.

This snippet (similar to line ~197) seems to work as expected:
  CHECK_FUNCTION_EXISTS(bogus_fx BOGUS_COND)
  if(False OR BOGUS_COND)
          message(FATAL_ERROR "True here")
  ELSE()
          message(FATAL_ERROR "False here")
  ENDIF()

Please let me know if I'm missing something.

Thanks and greetings

Daniel Díaz
daniel.diaz@linaro.org

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQEcBAEBCAAGBQJZG2JyAAoJECjqyWJ/sbDPdIMIAJJdqvIikw/ltwAUOPrk2gT9
xhOvt6pWUpyla9iOvyShUqziH15pArKU9Ghm+AP1LVhl7BeD1P6xDiB0JUGaqXV3
6+j7RtwjlBBzsJC6ILX5Gyk2s3KqaNyCICMTNScZAn6cWpsZsVZtNVmR4q6RwsHU
30kQyV6/wUE9WKuOmUbyCxXh4hJtsLa4F+ROXGpseKkHcMEhwlMcjaiZqORf0wm4
tBf+o0sF8Gohapjm1xkuqCrTEurm3l4F2VsxMMYpaofMH2FtTJjfBGXd83yrFYo5
2RKUwU0bD1JDIk1JSjaXWxarxYTwTHgT0qtvSlgUorK3piMu5UjzPw5YrFH7OTw=
=Ksw/
-----END PGP SIGNATURE-----
Jan Vesely May 16, 2017, 8:46 p.m. UTC | #3
On Tue, 2017-05-16 at 15:34 -0500, Daniel Díaz wrote:
> Hello!

> 

> 

> On 05/16/2017 03:06 PM, Jan Vesely wrote:

> > On Mon, 2017-05-15 at 20:27 -0500, Daniel Díaz wrote:

> > > diff --git a/CMakeLists.txt b/CMakeLists.txt index 

> > > a4ff99e..cc26fa8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt

> > > @@ -141,8 +141,9 @@ IF(${CMAKE_SYSTEM_NAME} MATCHES "Linux")

> > > if(GBM_FOUND) set(PIGLIT_HAS_GBM True) 

> > > add_definitions(-DPIGLIT_HAS_GBM) -		if (GBM_VERSION 

> > > VERSION_EQUAL "12.1" OR GBM_VERSION VERSION_GREATER "12.1") - 

> > > set(PIGLIT_HAS_GBM_BO_MAP True)

> > 

> > you still need to preserve the above line ^^, otherwise it breaks 

> > the check on line 197.

> 

> But it is still defined, right? The CheckSymbolExists module defines

> PIGLIT_HAS_GBM_BO_MAP which can be used in the condition down below,

> in true ("1") or false ("") form.


ah, right. I missed that you reuse the same variable name.
sorry for the noise. It forced me to dig a bit through cmake docs.
you can add my
Reviewed-by: Jan Vesely <jan.vesely@rutgers.edu>

if Dylan will take it.

Jan
> 

> This snippet (similar to line ~197) seems to work as expected:

>   CHECK_FUNCTION_EXISTS(bogus_fx BOGUS_COND)

>   if(False OR BOGUS_COND)

>           message(FATAL_ERROR "True here")

>   ELSE()

>           message(FATAL_ERROR "False here")

>   ENDIF()

> 

> Please let me know if I'm missing something.

> 

> Thanks and greetings

> 

> Daniel Díaz

> daniel.diaz@linaro.org

>
diff mbox

Patch

diff --git a/CMakeLists.txt b/CMakeLists.txt
index a4ff99e..cc26fa8 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -141,8 +141,9 @@  IF(${CMAKE_SYSTEM_NAME} MATCHES "Linux")
 	if(GBM_FOUND)
 		set(PIGLIT_HAS_GBM True)
 		add_definitions(-DPIGLIT_HAS_GBM)
-		if (GBM_VERSION VERSION_EQUAL "12.1" OR GBM_VERSION VERSION_GREATER "12.1")
-			set(PIGLIT_HAS_GBM_BO_MAP True)
+		set(CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES} ${GBM_LIBRARIES})
+		CHECK_FUNCTION_EXISTS(gbm_bo_map PIGLIT_HAS_GBM_BO_MAP)
+		if (PIGLIT_HAS_GBM_BO_MAP)
 			add_definitions(-DPIGLIT_HAS_GBM_BO_MAP)
 		endif()
 	endif(GBM_FOUND)