Re: [PATCH 2/2] threads: Use the right CMake check to decide whether to build the library.

[Andreas Schneider <asn@xxxxxxxxxxxxxx>]

On Wednesday 15 January 2014 22:56:31 Raphael Kubo da Costa wrote:
> CMAKE_HAVE_THREADS_LIBRARY may not be set in all cases where
> CMAKE_USE_PTHREADS_INIT is (for exampe, if one passes
> -DTHREADS_HAVE_PTHREAD_ARG=1 to CMake to tell it that -pthread is present
> and should be used), and we need the latter, not the former, to build our
> pthread.c.
> 
> This also allows us to simplify the build system code in threads/, as we can
> assume that pthreads is present (this was already the case before, since if
> CMAKE_USE_PTHREADS_INIT was false add_library() would not build any source
> file at all).

Hi Raphael,

thank you very much for your contribution!

> ---
>  src/CMakeLists.txt         |  4 ++--
>  src/threads/CMakeLists.txt | 24 +++---------------------
>  2 files changed, 5 insertions(+), 23 deletions(-)
> 
> diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
> index 83435d0..ced31c8 100644
> --- a/src/CMakeLists.txt
> +++ b/src/CMakeLists.txt
> @@ -288,6 +288,6 @@ if (WITH_STATIC_LIB)
>    )
>  endif (WITH_STATIC_LIB)
> 
> -if (CMAKE_HAVE_THREADS_LIBRARY)
> +if (CMAKE_USE_PTHREADS_INIT)
>      add_subdirectory(threads)
> -endif (CMAKE_HAVE_THREADS_LIBRARY)
> +endif (CMAKE_USE_PTHREADS_INIT)

I think this should be

if (CMAKE_HAVE_THREADS_LIBRARY OR CMAKE_USE_PTHREADS_INIT)

or will break other threading libraries than pthread

> diff --git a/src/threads/CMakeLists.txt b/src/threads/CMakeLists.txt
> index b95525e..50a3777 100644
> --- a/src/threads/CMakeLists.txt
> +++ b/src/threads/CMakeLists.txt
> @@ -24,30 +24,12 @@ if (WITH_STATIC_LIB)
>  endif (WITH_STATIC_LIB)
> 
>  set(LIBSSH_THREADS_LINK_LIBRARIES
> -    ${LIBSSH_SHARED_LIBRARY}
> -)
> -
> -set(libssh_threads_SRCS
> -)
> -
> -# build and link pthread
> -if (CMAKE_USE_PTHREADS_INIT)
> -    set(libssh_threads_SRCS
> -        ${libssh_threads_SRCS}
> -        pthread.c
> -    )
> -
> -    set(LIBSSH_THREADS_LINK_LIBRARIES
> -        ${LIBSSH_THREADS_LINK_LIBRARIES}
> -        ${CMAKE_THREAD_LIBS_INIT}
> -    )
> -endif (CMAKE_USE_PTHREADS_INIT)
> -
> -set(LIBSSH_THREADS_LINK_LIBRARIES
> -  ${LIBSSH_THREADS_LINK_LIBRARIES}
> +  ${LIBSSH_SHARED_LIBRARY} ${CMAKE_THREAD_LIBS_INIT}
>    CACHE INTERNAL "libssh threads link libraries"
>  )

We currently only have a pthread backend here, but we would like to encourage 
other to contribute. You should not assume that we have support only pthread 
cause this might not be the case in future :)


Could you please fix this and resend?


Thanks!

> 
> +set(libssh_threads_SRCS pthread.c)
> +
>  include_directories(
>    ${LIBSSH_THREADS_PUBLIC_INCLUDE_DIRS}
>    ${LIBSSH_THREADS_PRIVATE_INCLUDE_DIRS}

-- 
Andreas Schneider                   GPG-ID: CC014E3D
www.cryptomilk.org                asn@xxxxxxxxxxxxxx