[Piglit] [PATCH] add a PIGLIT_BUILD_GL_TESTS option

[Chad Versace]

NAK. Reasons below.

As an alternative to this patch's method, I would change PIGLIT_HAS_X11 to be a
cmake option, enabled by default on Linux, and base decisions on the option. I
generally frown on build systems that make decisions based on autodetected
dependencies.

On 10/18/2012 11:11 AM, Negreanu Marius wrote:
> From d9660648e5c41b2381fc82953bfede8d8451d8a2 Mon Sep 17 00:00:00 2001
> From: Adrian Marius Negreanu <adrian.m.negreanu at intel.com>
> Date: Thu, 18 Oct 2012 20:54:05 +0300
> Subject: [PATCH] use X11_X11_LIB_FOUND instead of CMAKE_SYSTEM_NAME
> 
> don't add X11_X11_LIB on systems that
> qualifies as Linux, but is missing the libX11.
> 
> Signed-off-by: Adrian Marius Negreanu <adrian.m.negreanu at intel.com>
> ---
>  CMakeLists.txt            | 2 +-
>  tests/util/CMakeLists.txt | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 58c1bcc..a6fbf84 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -61,7 +61,7 @@ if(PIGLIT_BUILD_CL_TESTS)
>         find_package(OpenCL REQUIRED)
>  endif(PIGLIT_BUILD_CL_TESTS)
> 
> -IF(${CMAKE_SYSTEM_NAME} MATCHES "Linux")
> +IF(X11_X11_LIB_FOUND)
>         set(PIGLIT_HAS_X11 True)
>         add_definitions(-DPIGLIT_HAS_X11)


This hunk will also disable PIGLIT_HAS_GBM, which is independent of X11. I see
no reason to ever disable PIGLIT_HAS_GBM on Linux.


> diff --git a/tests/util/CMakeLists.txt b/tests/util/CMakeLists.txt
> index bb29c62..371aacd 100644
> --- a/tests/util/CMakeLists.txt
> +++ b/tests/util/CMakeLists.txt
> @@ -50,12 +50,12 @@ set(UTIL_GL_LIBS
>          ${WAFFLE_LDFLAGS}
>         )
> 
> -if(${CMAKE_SYSTEM_NAME} MATCHES "Linux")
> +if(X11_X11_LIB_FOUND)
>         set(UTIL_GL_LIBS
>                 ${UTIL_GL_LIBS}
>                 ${X11_X11_LIB}
>         )
> -endif(${CMAKE_SYSTEM_NAME} MATCHES "Linux")
> +endif(X11_X11_LIB_FOUND)
> 
>  if(OPENGL_egl_LIBRARY)
>         set(UTIL_GL_SOURCES

This condition is silly. It shouldn't even be there. The CMakeLists should
unconditionally add X11_X11_LIB to UTIL_GL_LIBS, and just
rely on it expanding  to the empty string if the library isn't present.