Tarantool development patches archive
 help / color / mirror / Atom feed
From: Igor Munkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Bronnikov <estetus@gmail.com>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH] build: support build using Ninja
Date: Thu, 26 May 2022 19:21:25 +0300	[thread overview]
Message-ID: <Yo+pBf7TTnxvOkpA@tarantool.org> (raw)
In-Reply-To: <ee1a1146963fe6aad5bff59884e00b6cf24eaf1a.1653307030.git.sergeyb@tarantool.org>

Sergey,

Thanks for the patch! I only dump our offline discussion:
* We decided to add a separate CI pipeline to prevent ninja build to be
  broken again. It will be run on Linux/x86_64 with no CMake
  specification at all (i.e. -DLUAJIT_ENABLE_GC64=OFF and
  -DCMAKE_BUILD_TYPE=Release).
* We decided to refactor the targets related to LuaJIT executable, since
  ninja forbids using different targets building the same name. Hence
  instead of using parameterized dependencies of the fixed targets,
  let's use parameterized targets with fixed dependencies (hope it's
  not too tricky wording for you).

On 23.05.22, Sergey Bronnikov wrote:
> By default CMake generates files suitable for building a project with
> Make. However, it allows to generate files for Ninja too. Ninja [1] may
> build project a bit faster than Make, see [2]. Patch adds changes
> required for building with Ninja.

Minor: Please, describe these changes in detail.

> 
> How-to build wit Ninja:

Typo: s/wit/with/.

> 
> $ cmake -G Ninja -B build -S .
> $ ninja -C build luajit
> 
> On my laptop Ninja reduces building time by 14% (with Make it takes 5.7
> sec, with Ninja 3.9 sec). It is not so much, but without Ninja support in
> LuaJIT it is not possible to build Tarantool with Ninja.

Minor: This rationale can be moved below (after ---), since I see no
reason to mention particular results in the commit message. The links
below are more than enough.

> 
> 1. https://ninja-build.org/
> 2. https://mesonbuild.com/Simple-comparison.html
> ---
>  src/CMakeLists.txt      | 4 ++--
>  src/host/CMakeLists.txt | 6 ++++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 

<snipped>

> -- 
> 2.25.1
> 

-- 
Best regards,
IM

  reply	other threads:[~2022-05-26 16:28 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-23 12:01 Sergey Bronnikov via Tarantool-patches
2022-05-26 16:21 ` Igor Munkin via Tarantool-patches [this message]
2022-06-02 13:21   ` Sergey Bronnikov via Tarantool-patches

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=Yo+pBf7TTnxvOkpA@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=estetus@gmail.com \
    --cc=imun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH] build: support build using Ninja' \
    /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