[Tarantool-patches] [v2][PATCH 2/5] build/ninja: create target with cli binary only once
Sergey Kaplun
skaplun at tarantool.org
Wed Jun 15 12:10:55 MSK 2022
Hi, Sergey!
Thanks for the patch!
LGTM, except a few nits below.
On 02.06.22, Sergey Bronnikov via Tarantool-patches wrote:
> CMakeLists defines target for cli binary two times (for static and
> shared build type) with the same output names. CMake generates two
> targets with same names and it breaks Ninja:
>
> ninja: Entering directory `build/'
> ninja: error: build.ninja:1656: multiple rules generate src/luajit [-w
> dupbuild=err]
>
> Patch adds variables required for different build types and create
Typo? s/create/creates/
> target for cli binary only once to avoid conflict.
> ---
> src/CMakeLists.txt | 40 ++++++++++++++++------------------------
> 1 file changed, 16 insertions(+), 24 deletions(-)
>
> diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
> index 8b651c5a..104ce999 100644
> --- a/src/CMakeLists.txt
> +++ b/src/CMakeLists.txt
> @@ -313,40 +313,32 @@ target_link_libraries(libluajit_shared ${TARGET_LIBS})
>
> # Compiling and linking CLIs.
>
<snipped>
> if(NOT BUILDMODE STREQUAL "dynamic")
> set(LIBLUAJIT_STATIC_DEPS libluajit_static)
> set(LUAJIT_DEPS luajit_static)
> + set(LUAJIT_BIN luajit_static)
> + set(LUAJIT_LIB libluajit_static)
> endif()
> if(NOT BUILDMODE STREQUAL "static")
> set(LIBLUAJIT_SHARED_DEPS libluajit_shared)
> set(LUAJIT_DEPS luajit_shared)
> + set(LUAJIT_BIN luajit_shared)
> + set(LUAJIT_LIB libluajit_shared)
> endif()
> set(LIBLUAJIT_DEPS ${LIBLUAJIT_STATIC_DEPS} ${LIBLUAJIT_SHARED_DEPS})
Nit: Looks like the LUAJIT_DEPS variable duplicates work of
LUAJIT_BIN variable now.
>
> +add_executable(${LUAJIT_BIN} EXCLUDE_FROM_ALL ${CLI_SOURCES})
> +set_target_properties(luajit_shared PROPERTIES
> + OUTPUT_NAME "${LUAJIT_CLI_NAME}"
> + COMPILE_FLAGS "${TARGET_C_FLAGS}"
> + LINK_FLAGS "${TARGET_BIN_FLAGS}"
> + RUNTIME_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
> +)
> +target_include_directories(${LUAJIT_BIN} PRIVATE
> + ${CMAKE_CURRENT_BINARY_DIR}
> +)
> +target_link_libraries(${LUAJIT_BIN} ${LUAJIT_LIB} ${TARGET_LIBS})
> +
> # XXX: The variable is used in testing, so PARENT_SCOPE option
> # is obligatory.
> set(LUAJIT_BINARY $<TARGET_FILE:${LUAJIT_DEPS}> PARENT_SCOPE)
> --
> 2.25.1
>
--
Best regards,
Sergey Kaplun
More information about the Tarantool-patches
mailing list