Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alexander Turenko <alexander.turenko@tarantool.org>
To: Konstantin Belyavskiy <k.belyavskiy@tarantool.org>
Cc: tarantool-patches@freelists.org
Subject: [tarantool-patches] Re: [PATCH] use TarantoolConfig instead of FindTarantool
Date: Tue, 31 Jul 2018 15:45:44 +0300	[thread overview]
Message-ID: <20180731124544.7l7cq5xjzakj4cqb@tkn_work_nb> (raw)
In-Reply-To: <20180726143906.37863-1-k.belyavskiy@tarantool.org>

Hi!

I don't have much knowledge about cmake, but here is my two coins.

WBR, Alexander Turenko.

On Thu, Jul 26, 2018 at 05:39:06PM +0300, Konstantin Belyavskiy wrote:
> Use TarantoolConfig.cmake instead of FindTarantool.cmake and
> distribute it within tarantool-dev package.
> Module, which requires tarantool id its deps now can simply do:
>  find_package(Tarantool CONFIG REQUIRED)
> and does not obligate to have FindTarantool.cmake any more.
> Tested with following modules:
> avro-schema, cbench
> 
> Proposal for #3147
> ---
> Ticket: https://github.com/tarantool/tarantool/issues/3147
> Branch: https://github.com/tarantool/tarantool/tree/kbelyavs/gh-3147-use-tarantoolconfig-instead-findtarantool
>  CMakeLists.txt        |  1 +
>  TarantoolConfig.cmake | 44 ++++++++++++++++++++++++++++++++++++++++++++

Shouldn't we place the file in cmake/ or extra/? I guess extra/ is
better choice for files, which don't used during build and just
installed.

We don't install this new file in RPM/Deb developers packages? We
should, I think.

>  2 files changed, 45 insertions(+)
>  create mode 100644 TarantoolConfig.cmake
> 
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index d0cae0f01..f3f002f86 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -218,6 +218,7 @@ set(MODULE_LIBSUFFIX "${CMAKE_SHARED_LIBRARY_SUFFIX}")
>  
>  install(DIRECTORY DESTINATION ${MODULE_LUADIR})
>  install(DIRECTORY DESTINATION ${MODULE_LIBDIR})
> +install(FILES TarantoolConfig.cmake DESTINATION cmake)
>  
>  include(cmake/multilib.cmake)
>  
> diff --git a/TarantoolConfig.cmake b/TarantoolConfig.cmake
> new file mode 100644
> index 000000000..f0595c5c3
> --- /dev/null
> +++ b/TarantoolConfig.cmake
> @@ -0,0 +1,44 @@
> +# Define GNU standard installation directories
> +include(GNUInstallDirs)
> +
> +macro(extract_definition name output input)
> +    string(REGEX MATCH "#define[\t ]+${name}[\t ]+\"([^\"]*)\""
> +        _t "${input}")
> +    string(REGEX REPLACE "#define[\t ]+${name}[\t ]+\"(.*)\"" "\\1"
> +        ${output} "${_t}")
> +endmacro()
> +
> +find_path(TARANTOOL_INCLUDE_DIR tarantool/module.h
> +  HINTS ENV TARANTOOL_DIR /usr/local/include
> +)
> +
> +if(TARANTOOL_INCLUDE_DIR)
> +    set(_config "-")
> +    file(READ "${TARANTOOL_INCLUDE_DIR}/tarantool/module.h" _config0)
> +    string(REPLACE "\\" "\\\\" _config ${_config0})
> +    unset(_config0)
> +    extract_definition(PACKAGE_VERSION TARANTOOL_VERSION ${_config})
> +    extract_definition(INSTALL_PREFIX _install_prefix ${_config})
> +    unset(_config)
> +endif()
> +
> +include(FindPackageHandleStandardArgs)
> +find_package_handle_standard_args(TARANTOOL
> +    REQUIRED_VARS TARANTOOL_INCLUDE_DIR VERSION_VAR TARANTOOL_VERSION)
> +if(TARANTOOL_FOUND)
> +    set(TARANTOOL_INCLUDE_DIRS "${TARANTOOL_INCLUDE_DIR}"
> +                               "${TARANTOOL_INCLUDE_DIR}/tarantool/"
> +        CACHE PATH "Include directories for Tarantool")
> +    set(TARANTOOL_INSTALL_LIBDIR "${CMAKE_INSTALL_LIBDIR}/tarantool"
> +        CACHE PATH "Directory for storing Lua modules written in Lua")
> +    set(TARANTOOL_INSTALL_LUADIR "${CMAKE_INSTALL_DATADIR}/tarantool"
> +        CACHE PATH "Directory for storing Lua modules written in C")

I understand this is pure copy of FindTarantool.cmake from avro-schema,
but 'in Lua' and 'in C' in descriptions should be swapped.

> +
> +    if (NOT TARANTOOL_FIND_QUIETLY AND NOT FIND_TARANTOOL_DETAILS)
> +        set(FIND_TARANTOOL_DETAILS ON CACHE INTERNAL "Details about TARANTOOL")
> +        message(STATUS "Tarantool LUADIR is ${TARANTOOL_INSTALL_LUADIR}")
> +        message(STATUS "Tarantool LIBDIR is ${TARANTOOL_INSTALL_LIBDIR}")
> +    endif ()
> +endif()
> +mark_as_advanced(TARANTOOL_INCLUDE_DIRS TARANTOOL_INSTALL_LIBDIR
> +    TARANTOOL_INSTALL_LUADIR)
> -- 
> 2.14.3 (Apple Git-98)
> 
> 

      reply	other threads:[~2018-07-31 12:45 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-26 14:39 [tarantool-patches] " Konstantin Belyavskiy
2018-07-31 12:45 ` Alexander Turenko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180731124544.7l7cq5xjzakj4cqb@tkn_work_nb \
    --to=alexander.turenko@tarantool.org \
    --cc=k.belyavskiy@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH] use TarantoolConfig instead of FindTarantool' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox