Re: [Tarantool-patches] [v2][PATCH 4/5] build/ninja: create file lists outside of cmake commands

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Sergey, thanks for review!

On 15.06.2022 12:19, Sergey Kaplun via Tarantool-patches wrote:
> Hi, Sergey!
>
> Thanks for the patch!
>
> LGTM, except a few nits regarding the commit message.
>
> On 02.06.22, Sergey Bronnikov via Tarantool-patches wrote:
>> Patch adds a last change required for building LuaJIT with Ninja - using
> Typo: s/Patch/The patch/
Fixed, thanks.
>
>> glob inside CMake commands (add_custom_command and
>> set_source_files_properties) breaks buildng with Ninja.
> Typo: s/buildng/building/

Fixed, thanks!


>> 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 comparison in [2].
> Typo: s/comparison/the comparison/
Fixed, thanks!
>> How-to build with Ninja:
>>
>> $ cmake -G Ninja -B build -S .
>> $ cmake --build build --parallel
> Side note: Unfortunately `make test` command (or what should I use for
> tests, when build with ninja?)

Use 'ninja' instead:

sergeyb@pony:~/sources/MRG/tarantool/third_party/luajit$ cmake -G Ninja 
-B . -S .
-- The C compiler identification is GNU 9.4.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- [SetVersion] Reading version from VCS: v2.1.0-beta3-193-gd8c86e28
-- The ASM compiler identification is GNU
-- Found assembler: /usr/bin/cc
-- Configuring done
-- Generating done
-- Build files have been written to: 
/home/sergeyb/sources/MRG/tarantool/third_party/luajit
sergeyb@pony:~/sources/MRG/tarantool/third_party/luajit$ ninja -f 
build.ninja
[165/165] Linking C static library src/libluajit.a
sergeyb@pony:~/sources/MRG/tarantool/third_party/luajit$ ninja -f 
build.ninja test
<snipped>


 >>> closing state <<<

sergeyb@pony:~/sources/MRG/tarantool/third_party/luajit$ echo $?
0
sergeyb@pony:~/sources/MRG/tarantool/third_party/luajit$

or even do everything (confgure and build) using CMake:

sergeyb@pony:~/sources/MRG/tarantool/third_party/luajit$ cmake -G Ninja 
-B . -S .
-- The C compiler identification is GNU 9.4.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- [SetVersion] Reading version from VCS: v2.1.0-beta3-193-gd8c86e28
-- The ASM compiler identification is GNU
-- Found assembler: /usr/bin/cc
-- Configuring done
-- Generating done
-- Build files have been written to: 
/home/sergeyb/sources/MRG/tarantool/third_party/luajit
sergeyb@pony:~/sources/MRG/tarantool/third_party/luajit$ cmake --build . 
--parallel
[165/165] Linking C static library src/libluajit.a
sergeyb@pony:~/sources/MRG/tarantool/third_party/luajit$

> fails with the following error:
>
> | Checking /home/burii/reviews/luajit/ninja/build/src/jit/vmdef.lua 3 warnings
> |
> |     /home/burii/reviews/luajit/ninja/build/src/jit/vmdef.lua:5:121: (W631) line is too long (595 > 120)
> |     /home/burii/reviews/luajit/ninja/build/src/jit/vmdef.lua:7:121: (W631) line is too long (613 > 120)
> |     /home/burii/reviews/luajit/ninja/build/src/jit/vmdef.lua:11:121: (W631) line is too long (273 > 120)
>
> But this is the problem of OOS build (default for ninja), not this
> patch, IINM.

Hmm, it works fine for me (see output above).

But yes, luacheck has a problems with running OOS. See description of 
the problem in [1].

The problem was workarounded in Tarantool source repository, see commit:

commit af448464d15f60b87f1c9ef41a7816911c889459
Author: Alexander Turenko <alexander.turenko@tarantool.org>
Date:   Wed Nov 11 10:09:25 2020 +0300

     tools: fix luacheck invocation in different cases

     Now `make luacheck` gracefully handles different cases[^1]: in-source
     and out-of-source build (within the source tree or outside), current
     working directory as a real path or with symlink components.

     As result of looking into those problems I filed the issue [1] against
     luacheck. It seems, there are problems around absolute paths with
     symlinks components.

     [^1]: We have the similar problems with LuaJIT's luacheck rule. They
           will be fixed in a separate patch.

     [1]: https://github.com/mpeterv/luacheck/issues/208

     Reviewed-by: Sergey Bronnikov <sergeyb@tarantool.org>
     Reviewed-by: Igor Munkin <imun@tarantool.org>

1. https://github.com/mpeterv/luacheck/issues/208

>
>> 1. https://ninja-build.org/
>> 2. https://mesonbuild.com/Simple-comparison.html
>> ---
>>   src/host/CMakeLists.txt | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/host/CMakeLists.txt b/src/host/CMakeLists.txt
>> index e01db87d..1ce3e224 100644
>> --- a/src/host/CMakeLists.txt
>> +++ b/src/host/CMakeLists.txt
> <snipped>
>
>> -- 
>> 2.25.1
>>