Re: [Tarantool-patches] [PATCH luajit 2/5] build: replace GNU Make with CMake

[Igor Munkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Sergey,

Thanks for your review!

On 11.02.21, Sergey Kaplun wrote:
> Hi, Igor!
> 
> Thanks for the patch!
> 
> Appreciate your hard work! Thanks for the verbose comments!
> The building process is faster now!
> I'm sorry: I can't see your GUI innovations, unfortunately. :(
> 
> I've looked into branch version.
> Please consider my comments below.

I've fixed all typos you've mentioned, so I left only those I haven't or
have changed in a different way.

> 
> On 02.02.21, Igor Munkin wrote:
> > In scope of this patch the LuaJIT build system is partially ported from
> 
> Typo: s/In scope/In the scope/

I strongly doubt. Neither you, nor me: Within.

> 

<snipped>

> > old build system (see the recipe in the previous commit).
> 
> This paragraph describes only supported OS. There is no description of
> supported build system options.
> IINM there is no cross-compiling support, stripping debug info, amalgam
> build for now, it should be mentioned in the commit message too.

Here is the new wording for this section:
| Within this patch the LuaJIT build system is partially ported from GNU
| Make to CMake. These changes provide CMake build system for all
| supported host architectures but only for the following OS: GNU/Linux,
| OSX, FreeBSD. For other platrforms and specific builds (such as 'amalg',
| stripped binary and shared library, cross-compiling support) use the old
| build system (see the recipe in the previous commit).

> 
> > 
> > Several components of the new build system such as automatic version
> > detection, source files list generation and some recipes for
> > CMakeLists.txt are taken verbatim or adapted from LuaVela repository.
> > 
> > Part of tarantool/tarantool#4862
> > 
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> 
> Side note: This is not related to the patch directly but what about
> Tarantool COPYRIGHT notice in the LuaJIT code and inside new cmake files
> in particular?

We definitely need to make such activity, but I have no idea how to do
it a right way. Let's postpone it and discuss later, if you don't mind.

> 
> >  .gitignore                 |  12 +-
> >  CMakeLists.txt             | 261 +++++++++++++++++++++++++
> >  cmake/LuaJITUtils.cmake    |  31 +++
> >  cmake/MakeSourceList.cmake |  47 +++++
> >  cmake/SetDynASMFlags.cmake | 130 ++++++++++++
> >  cmake/SetTargetFlags.cmake |  42 ++++
> >  cmake/SetVersion.cmake     |  45 +++++
> >  etc/CMakeLists.txt         |  32 +++
> >  src/CMakeLists.txt         | 391 +++++++++++++++++++++++++++++++++++++
> >  src/host/CMakeLists.txt    |  61 ++++++
> >  tools/CMakeLists.txt       |  77 ++++++++
> >  11 files changed, 1128 insertions(+), 1 deletion(-)
> >  create mode 100644 CMakeLists.txt
> >  create mode 100644 cmake/LuaJITUtils.cmake
> >  create mode 100644 cmake/MakeSourceList.cmake
> >  create mode 100644 cmake/SetDynASMFlags.cmake
> >  create mode 100644 cmake/SetTargetFlags.cmake
> >  create mode 100644 cmake/SetVersion.cmake
> >  create mode 100644 etc/CMakeLists.txt
> >  create mode 100644 src/CMakeLists.txt
> >  create mode 100644 src/host/CMakeLists.txt
> >  create mode 100644 tools/CMakeLists.txt
> 
> Side note: should we update <doc/install.html> (change Makefile ->
> Makefile.original)? Looks more related to the previous patch.

OK, fixed, squashed with the previous commit. Here is the diff:

================================================================================

diff --git a/doc/install.html b/doc/install.html
index c491c60..b2e4753 100644
--- a/doc/install.html
+++ b/doc/install.html
@@ -164,9 +164,9 @@ hold all user-configurable settings:
 </p>
 <ul>
 <li><tt>src/luaconf.h</tt> sets some configuration variables.</li>
-<li><tt>Makefile</tt> has settings for <b>installing</b> LuaJIT (POSIX
+<li><tt>Makefile.original</tt> has settings for <b>installing</b> LuaJIT (POSIX
 only).</li>
-<li><tt>src/Makefile</tt> has settings for <b>compiling</b> LuaJIT
+<li><tt>src/Makefile.original</tt> has settings for <b>compiling</b> LuaJIT
 under POSIX, MinGW or Cygwin.</li>
 <li><tt>src/msvcbuild.bat</tt> has settings for compiling LuaJIT with
 MSVC or WinSDK.</li>
@@ -228,7 +228,7 @@ variable is not set, then it's forced to <tt>10.4</tt>.
 </p>
 <h3>Installing LuaJIT</h3>
 <p>
-The top-level Makefile installs LuaJIT by default under
+The top-level Makefile.original installs LuaJIT by default under
 <tt>/usr/local</tt>, i.e. the executable ends up in
 <tt>/usr/local/bin</tt> and so on. You need root privileges
 to write to this path. So, assuming sudo is installed on your system,
@@ -366,7 +366,7 @@ target OS differ, or you'll get assembler or linker errors:
 </p>
 <ul>
 <li>E.g. if you're compiling on a Windows or OSX host for embedded Linux or Android, you need to add <tt>TARGET_SYS=Linux</tt> to the examples below.</li>
-<li>For a minimal target OS, you may need to disable the built-in allocator in <tt>src/Makefile</tt> and use <tt>TARGET_SYS=Other</tt>.</li>
+<li>For a minimal target OS, you may need to disable the built-in allocator in <tt>src/Makefile.original</tt> and use <tt>TARGET_SYS=Other</tt>.</li>
 <li>Don't forget to specify the same <tt>TARGET_SYS</tt> for the install step, too.</li>
 </ul>
 <p>
@@ -647,7 +647,7 @@ the paths needed to locate the shared library.</li>
 to a shadow tree instead of the root tree of the build system.</li>
 <li><tt>MULTILIB</tt> sets the architecture-specific library path component
 for multilib systems. The default is <tt>lib</tt>.</li>
-<li>Have a look at the top-level <tt>Makefile</tt> and <tt>src/Makefile</tt>
+<li>Have a look at the top-level <tt>Makefile.original</tt> and <tt>src/Makefile.original</tt>
 for additional variables to tweak. The following variables <em>may</em> be
 overridden, but it's <em>not</em> recommended, except for special needs
 like cross-builds:

================================================================================

NB: I haven't break the lines intentionally to make the diff minimal.

> 

<snipped>

> > diff --git a/CMakeLists.txt b/CMakeLists.txt
> > new file mode 100644
> > index 0000000..0dba5d8
> > --- /dev/null
> > +++ b/CMakeLists.txt
> > @@ -0,0 +1,261 @@
> > +# LuaJIT -- interpreter and JIT compiler for Lua language.
> > +# This is the main entry point for building, testing and
> > +# packaging the project.
> > +# Major portions taken verbatim or adapted from the uJIT.
> > +# Copyright (C) 2015-2019 IPONWEB Ltd.
> > +
> > +# --- Initial setup ------------------------------------------------------------
> > +
> > +cmake_minimum_required(VERSION 2.8.12 FATAL_ERROR)
> > +project(LuaJIT C)
> > +
> > +#
> 
> Nit: Comment style is inconsistent. Feel free to ignore.

Fixed, squashed, force-pushed to the branch. Diff is below:

================================================================================

diff --git a/CMakeLists.txt b/CMakeLists.txt
index af9f8e4..e5be172 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -11,12 +11,10 @@
 cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
 project(LuaJIT C)
 
-#
 # XXX: Originally CMake machinery is introduced to make LuaJIT
 # testing self-sufficient. Since there are only few systems
 # covered with the tests in our CI, there is no need to support
 # others for now and the original build system can be used.
-#
 
 if(NOT(
    CMAKE_SYSTEM_NAME STREQUAL "Linux" OR

================================================================================

> 
> > +# XXX: Originally CMake machinery is introduced to make LuaJIT
> > +# testing self-sufficient. Since there are only few systems
> 
> Typo: s/only few systems/only a few systems/

No, I want to emphasize the amount, so the article changes the meaning.

> 

<snipped>

> > +
> > +set(BUILDMODE_VALUES mixed static dynamic)
> 
> See nothing bad to transfer comments described each option here.

Added, squashed, force-pushed to the branch. Diff is below:

================================================================================

diff --git a/CMakeLists.txt b/CMakeLists.txt
index e5be172..786dbe0 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -59,7 +59,14 @@ set(LUAJIT_INCLUDEDIR
   include/luajit-${LUAJIT_VERSION_MAJOR}.${LUAJIT_VERSION_MINOR}
 )
 
+# Mixed mode creates a static + dynamic library and a statically
+# linked luajit. Static mode creates a static library and a
+# statically linked luajit. Dynamic mode creates a dynamic library
+# and a dynamically linked luajit.
+# XXX: dynamically linked executable will only run when the
+# library is installed!
 set(BUILDMODE_VALUES mixed static dynamic)
+# The default build mode is mixed mode on POSIX.
 list(GET BUILDMODE_VALUES 0 BUILDMODE_DEFAULT)
 set(BUILDMODE ${BUILDMODE_DEFAULT} CACHE STRING
   "Build mode. Choose one of the following: ${BUILDMODE_VALUES}."

================================================================================

> 
> > +list(GET BUILDMODE_VALUES 0 BUILDMODE_DEFAULT)

<snipped>

> > +# --- Compilation flags setup --------------------------------------------------
> > +
> > +if(NOT CMAKE_INSTALL_PREFIX STREQUAL "/usr/local")
> > +  AppendFlags(TARGET_C_FLAGS -DLUA_ROOT='"${CMAKE_INSTALL_PREFIX}"')
> 
> In Makefile.original I see the following check inside this branch:
> 
> | ifneq (/usr,$(PREFIX))
> |   TARGET_DYNXLDOPTS= -Wl,-rpath,$(TARGET_LIBPATH)
> | endif
> 
> Is it related to unsupported build systems?

AFAICS, it is. Anyway, if you're having troubles, feel free to share.

> 
> I've found some warnings on macOS in the configuration stage, that looks
> related:
> 
> | CMake Warning (dev):
> |   Policy CMP0042 is not set: MACOSX_RPATH is enabled by default.  Run "cmake
> |   --help-policy CMP0042" for policy details.  Use the cmake_policy command to
> |   set the policy and suppress this warning.
> |
> |   MACOSX_RPATH is not specified for the following targets:
> |
> |    libluajit_shared
> |
> | This warning is for project developers.  Use -Wno-dev to suppress it.
> 
> Version of macOS:
> | $ uname -a
> | Darwin MacBook-Pro.local 19.6.0 Darwin Kernel Version 19.6.0: Mon Aug 31 22:12:52 PDT 2020; root:xnu-6153.141.2~1/RELEASE_X86_64 x86_64
> 

I've seen this warning, and have tried almost all combinations of the
MACOS_RPATH, CMP0042, and target options -- nothing has helped. BTW,
this warning breaks nothing for me (on my old Mac). If this bothers you,
please report your issues. If you have a solution, please share it.

> > +endif()
> > +
> > +if(CMAKE_LIBRARY_ARCHITECTURE)
> > +  AppendFlags(TARGET_C_FLAGS -DLUA_MULTILIB='"lib/${CMAKE_LIBRARY_ARCHITECTURE}"')
> > +endif()
> 
> What about `LUA_LMULTILIB`?
> 
> Side note: should we provide `DESTDIR` or/and `MULTILIB` control variable
> like it does in Makefile.original?

DESTDIR is supported out of the box in CMake. MULTILIB is "autodetected"
via CMake[1] (I hope, but never tried). Regardging LMULTILIB, I have no
idea what its purpose is. Do you?

> 
> > +

<snipped>

> > +AppendFlags(CMAKE_C_FLAGS -Wall)
> > +option(LUAJIT_ENABLE_WARNINGS "Build LuaJIT with warnings enabled" OFF)
> 
> Nit: This confused me a little. All warnings already enabled. This flag
> enables extra warnings. I may be change like the following:
> s/LUAJIT_ENABLE_WARNINGS/LUAJIT_ENABLE_EXTRA_WARNINGS/
> s/warnings/extra warnings/
> or
> s/LUAJIT_ENABLE_WARNINGS/LUAJIT_ENABLE_MORE_WARNINGS/
> s/warnings/more warnings/
> Feel free to ignore.

Honestly, I don't want a huge option name. Let's return to the subj in
scope of the options revision that we discussed with Timur nearby.

Ignoring for now.

> 

<snipped>

> > +# Permanently disable the FFI extension to reduce the size of the
> > +# LuaJIT executable. But please consider that the FFI library is
> > +# compiled-in, but NOT loaded by default. It only allocates any
> > +# memory, if you actually make use of it.
> > +option(LUAJIT_DISABLE_FFI "FFI support" OFF)
> > +if(LUAJIT_DISABLE_FFI)
> > +  AppendFlags(TARGET_C_FLAGS -DLUAJIT_DISABLE_FFI)
> > +endif()
> > +set(LUAJIT_HAS_FFI NOT LUAJIT_DISABLE_FFI)
> 
> Don't get your point here. `set()` does not support any expressions
> as I've known from [1][2]. This just always defines `LUAJIT_HAS_FFI`.

OMG, CMake is the greatest shit in the world. I just dropped these
auxiliary variables, squashed with the previous commit. Diff is below:

================================================================================

diff --git a/CMakeLists.txt b/CMakeLists.txt
index a1536ed..13208d1 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -130,7 +130,6 @@ option(LUAJIT_DISABLE_FFI "FFI support" OFF)
 if(LUAJIT_DISABLE_FFI)
   AppendFlags(TARGET_C_FLAGS -DLUAJIT_DISABLE_FFI)
 endif()
-set(LUAJIT_HAS_FFI NOT LUAJIT_DISABLE_FFI)
 
 # Features from Lua 5.2 that are unlikely to break existing code
 # are enabled by default. Some other features that *might* break
@@ -149,7 +148,6 @@ option(LUAJIT_DISABLE_JIT "JIT support" OFF)
 if(LUAJIT_DISABLE_JIT)
   AppendFlags(TARGET_C_FLAGS -DLUAJIT_DISABLE_JIT)
 endif()
-set(LUAJIT_HAS_JIT NOT LUAJIT_DISABLE_JIT)
 
 # Some architectures (e.g. PPC) can use either single-number (1)
 # or dual-number (2) mode. Please see LJ_ARCH_NUMMODE in lj_arch.h
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index df9b85d..e70b786 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -140,17 +140,20 @@ make_source_list(SOURCES_CORE_NO_JIT_FFI
 
 set(SOURCES_CORE ${SOURCES_CORE_NO_JIT_FFI})
 
-if(LUAJIT_HAS_JIT)
+# Build JIT sources if JIT support is enabled.
+if(NOT LUAJIT_DISABLE_JIT)
   list(APPEND SOURCES_CORE ${SOURCES_JIT})
   if(LUAJIT_ENABLE_GDBJIT)
     list(APPEND SOURCES_CORE ${CMAKE_CURRENT_SOURCE_DIR}/lj_gdbjit.c)
   endif()
 endif()
 
-if(LUAJIT_HAS_FFI)
+# Build FFI sources if FFI support is enabled.
+if(NOT LUAJIT_DISABLE_FFI)
   list(APPEND SOURCES_CORE ${SOURCES_FFI})
-  if(NOT LUAJIT_HAS_JIT)
-    # Needed for lj_mcode_sync.
+  # Build lj_mcode.c if JIT support is disabled since
+  # <lj_mcode_sync> is used in src/lj_ccallback.c.
+  if(LUAJIT_DISABLE_JIT)
     list(APPEND SOURCES_CORE ${CMAKE_CURRENT_SOURCE_DIR}/lj_mcode.c)
   endif()
 endif()

================================================================================

> 

<snipped>

> 
> > +
> > +# Some architectures (e.g. PPC) can use either single-number (1)
> > +# or dual-number (2) mode. Uncomment one of these lines to
> 
> Nit: See nothing to uncomment :). May be change it?

*Кхъ*. Removed :)

================================================================================

diff --git a/CMakeLists.txt b/CMakeLists.txt
index ca03fda..cea0a53 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -152,9 +152,8 @@ endif()
 set(LUAJIT_HAS_JIT NOT LUAJIT_DISABLE_JIT)
 
 # Some architectures (e.g. PPC) can use either single-number (1)
-# or dual-number (2) mode. Uncomment one of these lines to
-# override the default mode. Please see LJ_ARCH_NUMMODE in
-# lj_arch.h for details.
+# or dual-number (2) mode. Please see LJ_ARCH_NUMMODE in lj_arch.h
+# for details.
 set(LUAJIT_NUMMODE_VALUES 1 2)
 set(LUAJIT_NUMMODE_DEFAULT "")
 set(LUAJIT_NUMMODE ${LUAJIT_NUMMODE_DEFAULT} CACHE STRING

================================================================================

> 

<snipped>

> 
> Nit: It would be nice to see "Debugging support" section here like for
> other sections like "Main source tree" or so on.
> Feel free to ignore.

These are in "Compilation flags setup" section.

Ignoring.

> 

<snipped>

> 
> > +# Turn on assertions for the Lua/C API to debug problems with
> > +# lua_* calls. This is rather slow -- use only while developing C
> > +# libraries/embeddings.
> > +option(LUAJIT_USE_APICHECK "Assertions for the Lua/C API" OFF)
> > +if(LUAJIT_USE_APICHECK)
> > +  AppendFlags(TARGET_C_FLAGS -DLUA_USE_APICHECK)
> > +endif()
> 
> Hmm, as for me looks more user friendly not to change option names here.
> Just leave `LUA_USE_APICHECK` here.
> I've already singed wings on it, when run Lua/LuaJIT/lua-Harness
> tests! Yeees, I'd not looked at Cmake warnings carefully :(

Renamed these flags, but the new naming is more consistent, so we'll
revert it back in scope of build options refactoring. Diff is below:

================================================================================

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 25d86c0..673cf36 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -230,16 +230,16 @@ endif()
 # Turn on assertions for the Lua/C API to debug problems with
 # lua_* calls. This is rather slow -- use only while developing C
 # libraries/embeddings.
-option(LUAJIT_USE_APICHECK "Assertions for the Lua/C API" OFF)
-if(LUAJIT_USE_APICHECK)
+option(LUA_USE_APICHECK "Assertions for the Lua/C API" OFF)
+if(LUA_USE_APICHECK)
   AppendFlags(TARGET_C_FLAGS -DLUA_USE_APICHECK)
 endif()
 
 # Turn on assertions for the whole LuaJIT VM. This significantly
 # slows down everything. Use only if you suspect a problem with
 # LuaJIT itself.
-option(LUAJIT_USE_ASSERT "Assertions for the whole LuaJIT VM" OFF)
-if(LUAJIT_USE_ASSERT)
+option(LUA_USE_ASSERT "Assertions for the whole LuaJIT VM" OFF)
+if(LUA_USE_ASSERT)
   AppendFlags(TARGET_C_FLAGS -DLUA_USE_ASSERT)
 endif()
 

================================================================================

> 

<snipped>

> 
> > +
> > +# TODO: Implement a configuration option to enable ASAN.
> > +# There are two entries of LUAJIT_USE_ASAN define:
> > +# $ grep -rnF 'LUAJIT_USE_ASAN' .
> > +# ./src/lj_str.c:15:#if LUAJIT_USE_ASAN
> > +# ./src/host/buildvm.c:36:#if LUAJIT_USE_ASAN
> > +# At the same time this flag is not provided by LuaJIT original
> > +# build system (i.e. src/Makefile.original) so there are no
> > +# related compiler and linker flags passed. This should be done
> > +# the right way later.
> 
> Good catch! I decide to check what else is missing. Here are some other
> options (sorry, for that dump, just want to save it anywhere):
> | $ grep -rnF 'LUAJIT_CTYPE_CHECK_ANCHOR'
> | src/lj_ctype.c:139:#ifdef LUAJIT_CTYPE_CHECK_ANCHOR
> | src/lj_ctype.c:159:#ifdef LUAJIT_CTYPE_CHECK_ANCHOR
> 
> | $ grep -rnF 'LUAJIT_DEBUG_RA'
> | src/lj_asm.c:192:/* #define LUAJIT_DEBUG_RA */
> | src/lj_asm.c:194:#ifdef LUAJIT_DEBUG_RA

I tried only this one, while debugging the RENAME issue.
It would be nice to export it.

> 
> | $ grep -rnF 'LUAJIT_DISABLE_DEBUGINFO'
> | src/lj_parse.c:1397:#ifndef LUAJIT_DISABLE_DEBUGINFO
> | src/lj_parse.c:2700:#ifdef LUAJIT_DISABLE_DEBUGINFO
> | Binary file src/.lj_parse.c.swp matches
> | src/lj_memprof.c:102:  ** -DLUAJIT_DISABLE_DEBUGINFO flag.
> 
> | $ grep -rnF 'LUAJIT_DISABLE_PROFILE'
> | src/lj_arch.h:501:#if defined(LUAJIT_DISABLE_PROFILE)
> 
> | $ grep -rnF 'LUAJIT_DISABLE_VMEVENT'
> | src/lj_trace.c:796:#ifndef LUAJIT_DISABLE_VMEVENT
> | src/lj_vmevent.h:33:#ifdef LUAJIT_DISABLE_VMEVENT
> | src/lib_jit.c:118:#ifdef LUAJIT_DISABLE_VMEVENT
> 
> | $ grep -rnF 'LUAJIT_ENABLE_CHECKHOOK'
> | src/lj_record.c:2621:#ifdef LUAJIT_ENABLE_CHECKHOOK
> 
> | $ grep -rnF 'LUAJIT_ENABLE_TABLE_BUMP'
> | src/lj_record.c:259:#ifdef LUAJIT_ENABLE_TABLE_BUMP
> | src/lj_record.c:1179:#ifdef LUAJIT_ENABLE_TABLE_BUMP
> | src/lj_record.c:1477:#ifdef LUAJIT_ENABLE_TABLE_BUMP
> | src/lj_record.c:1523:#ifdef LUAJIT_ENABLE_TABLE_BUMP
> | src/lj_record.c:1874:#ifdef LUAJIT_ENABLE_TABLE_BUMP
> | src/lj_record.c:2321:#ifdef LUAJIT_ENABLE_TABLE_BUMP
> | src/lj_record.c:2544:#ifdef LUAJIT_ENABLE_TABLE_BUMP
> | src/lj_jit.h:455:#ifdef LUAJIT_ENABLE_TABLE_BUMP
> 
> | $ grep -rnF 'LUAJIT_NO_UNALIGNED'
> | src/lj_def.h:287:#if defined(_M_PPC) && defined(LUAJIT_NO_UNALIGNED)
> 
> | $ grep -rnF 'LUAJIT_USE_PERFTOOLS'
> | src/lj_trace.c:85:#ifdef LUAJIT_USE_PERFTOOLS
> | src/lj_trace.c:163:#ifdef LUAJIT_USE_PERFTOOLS
> 
> | $ grep -rnF 'LUAJIT_UNWIND_EXTERNAL'
> | doc/extensions.html:402:<td class="excplatform">ARM <tt>-DLUAJIT_UNWIND_EXTERNAL</tt></td>
> | src/lj_err.c:53:** unwinding with -DLUAJIT_UNWIND_EXTERNAL. *All* C code must be compiled
> | src/lj_err.c:64:#if defined(__GNUC__) && (LJ_TARGET_X64 || defined(LUAJIT_UNWIND_EXTERNAL)) && !LJ_NO_UNWIND
> | src/lj_err.c:521:  ** enabled LUAJIT_UNWIND_EXTERNAL and forgot to recompile *every*
> 
> They not all related to debugging (AFAICT only `LUAJIT_USE_PERFTOOLS`,
> `LUAJIT_CTYPE_CHECK_ANCHOR` and `LUAJIT_DEBUG_RA`), more to configure.
> May be it is better to create separate issue(s) for all these options?

Feel free to do it :)

> 

<snipped>

> 
> > diff --git a/cmake/LuaJITUtils.cmake b/cmake/LuaJITUtils.cmake
> > new file mode 100644
> > index 0000000..faaef6b
> > --- /dev/null
> > +++ b/cmake/LuaJITUtils.cmake
> > @@ -0,0 +1,31 @@
> > +function(LuaJITTestArch outvar strflags)
> > +  # XXX: <execute_process> simply splits the COMMAND argument by
> > +  # spaces with no further parsing. At the same time GCC is bad in
> > +  # argument handling, so let's help it a bit.
> > +  separate_arguments(TEST_C_FLAGS UNIX_COMMAND ${strflags})
> 
> Nit: TESTARCH_C_FLAGS looks more verbose to me. Just TEST seems to be
> referring to testing.
> Feel free to ignore.

Renamed, squashed with the previous commit. Here is the diff:

================================================================================

diff --git a/cmake/LuaJITUtils.cmake b/cmake/LuaJITUtils.cmake
index faaef6b..1769675 100644
--- a/cmake/LuaJITUtils.cmake
+++ b/cmake/LuaJITUtils.cmake
@@ -2,9 +2,9 @@ function(LuaJITTestArch outvar strflags)
   # XXX: <execute_process> simply splits the COMMAND argument by
   # spaces with no further parsing. At the same time GCC is bad in
   # argument handling, so let's help it a bit.
-  separate_arguments(TEST_C_FLAGS UNIX_COMMAND ${strflags})
+  separate_arguments(TESTARCH_C_FLAGS UNIX_COMMAND ${strflags})
   execute_process(
-    COMMAND ${CMAKE_C_COMPILER} ${TEST_C_FLAGS} -E lj_arch.h -dM
+    COMMAND ${CMAKE_C_COMPILER} ${TESTARCH_C_FLAGS} -E lj_arch.h -dM
     WORKING_DIRECTORY ${LUAJIT_SOURCE_DIR}
     OUTPUT_VARIABLE TESTARCH
   )

================================================================================

> 
> Nit: Also, it would be nice to see comments here, why do we use this
> approach instead use of CMAKE_HOST_SYSTEM_PROCESSOR variable (is set by
> `uname -p`).
> Feel free to ignore.

I have no clear explanation of these and I definitely don't want to
write a bullshit in the comments, so I will either add TODO here if you
insist, or ignore this one.

> > +  execute_process(
> > +    COMMAND ${CMAKE_C_COMPILER} ${TEST_C_FLAGS} -E lj_arch.h -dM
> 
> May be it will better to provide not default compiler here?
> It will be easier to adjust cross compiling with this.

I don't get this, but as we discussed before cross compiling activity is
out of the scope of this issue.

> 
> > +    WORKING_DIRECTORY ${LUAJIT_SOURCE_DIR}
> > +    OUTPUT_VARIABLE TESTARCH
> > +  )
> > +  set(${outvar} ${TESTARCH} PARENT_SCOPE)
> > +endfunction()
> > +
> > +function(LuaJITArch outvar testarch)
> > +  foreach(TRYARCH X64 X86 ARM ARM64 PPC MIPS64 MIPS)
> 
> I occur the following error, when build inside Docker emulating
> aarch64 (`FROM arm64v8/centos`):
> 
> | Error: pointer size mismatch in cross-build.
> | Try: make HOST_CC="gcc -m32" CROSS=...
> |
> | make[2]: *** [src/CMakeFiles/vm_static.dir/build.make:62: src/lj_vm.S] Error 1
> | make[1]: *** [CMakeFiles/Makefile2:193: src/CMakeFiles/vm_static.dir/all] Error 2
> | make[1]: *** Waiting for unfinished jobs....
> | Error: pointer size mismatch in cross-build.
> | Try: make HOST_CC="gcc -m32" CROSS=...
> |
> | make[2]: *** [src/CMakeFiles/vm_shared.dir/build.make:62: src/lj_vm.S] Error 1
> | make[1]: *** [CMakeFiles/Makefile2:394: src/CMakeFiles/vm_shared.dir/all] Error 2
> 
> All OK with the old Makefile:
> 
> | $ make -f Makefile.original -j
> | ==== Building LuaJIT 2.1.0-beta3 ====
> | ...
> | ==== Successfully built LuaJIT 2.1.0-beta3 ====
> 
> Content of <src/host/CMakeFiles/buildvm.dir/build.make>:
> | src/host/buildvm_arch.h: dynasm/*.lua
> |         @$(CMAKE_COMMAND) -E cmake_echo_color --switch=$(COLOR) --blue --bold --progress-dir=/luajit/CMakeFiles --progress-num=$(CMAKE_PROGRESS_1) "Generating buildvm_arch.h"
> |         cd /luajit/src/host && /luajit/src/host/minilua /luajit/dynasm/dynasm.lua -D ENDIAN_LE -D P64 -D JIT -D FFI -D DUALNUM -D FPU -D HFABI -D VER=80 -o buildvm_arch.h /luajit/src/vm_arm.dasc
> 
> vm_arm.dasc instead vm_arm64.dasc here.

Oops.

> 
> All because LJ_TARGET_ARM matched before LJ_TARGET_ARM64 in the line
> below. And it works for MIPS, because of different order :)
> Swap arches and leave comments about an order, please.

Neat, many thanks for testing and investigation! Fixed, squashed,
force-pushed to the branch. Diff is below:

================================================================================

diff --git a/cmake/LuaJITUtils.cmake b/cmake/LuaJITUtils.cmake
index 1769675..0a662f7 100644
--- a/cmake/LuaJITUtils.cmake
+++ b/cmake/LuaJITUtils.cmake
@@ -12,7 +12,7 @@ function(LuaJITTestArch outvar strflags)
 endfunction()
 
 function(LuaJITArch outvar testarch)
-  foreach(TRYARCH X64 X86 ARM ARM64 PPC MIPS64 MIPS)
+  foreach(TRYARCH X64 X86 ARM64 ARM PPC MIPS64 MIPS)
     string(FIND "${testarch}" "LJ_TARGET_${TRYARCH}" FOUND)
     if(FOUND EQUAL -1)
       continue()

================================================================================

> 
> Nit: As far as we can't organize this CPU arches in alphabetical order
> I propose reverse-alphabetical ;):

God, please, no...

> 
> | X86 X64 PPC MIPS64 MIPS ARM64 ARM
> 
> Feel free to ignore (it's more a joke).

Not funny, dude...

> 
> > +    string(FIND "${testarch}" "LJ_TARGET_${TRYARCH}" FOUND)
> > +    if(FOUND EQUAL -1)
> > +      continue()
> 
> `continue()` command was introduced in 3.2 CMake version
> according to [1]. So even 3.1 version is not enough.

Shit, that can't be true... But thanks for checking it!

> 
> This is a result for verision 3.0.2.
> | CMake Error at cmake/LuaJITUtils.cmake:18 (continue):
> |   Unknown CMake command "continue".
> | Call Stack (most recent call first):
> |   cmake/SetTargetFlags.cmake:16 (LuaJITArch)
> |   src/CMakeLists.txt:164 (include)
> |
> |
> | -- Configuring incomplete, errors occurred!
> 
> Please, add `cmake_minimum_required()` command to the top, or do not use

Hell no, I'll just rewrite this part and leave *non-toxic* comment about
it. When CMake will be updated in Tarantool next time, we'll refactor
this part. But not today.

> `continue()` command.

Reimplemented, squashed, force-pushed to the branch. Diff is below:

================================================================================

diff --git a/cmake/LuaJITUtils.cmake b/cmake/LuaJITUtils.cmake
index 0a662f7..2c4a27e 100644
--- a/cmake/LuaJITUtils.cmake
+++ b/cmake/LuaJITUtils.cmake
@@ -14,12 +14,18 @@ endfunction()
 function(LuaJITArch outvar testarch)
   foreach(TRYARCH X64 X86 ARM64 ARM PPC MIPS64 MIPS)
     string(FIND "${testarch}" "LJ_TARGET_${TRYARCH}" FOUND)
-    if(FOUND EQUAL -1)
-      continue()
+    # FIXME: <continue> is introduced in CMake version 3.2, but
+    # the minimum required version now is 3.1. This is not such a
+    # vital feature, so it's not used now. However, when CMake
+    # version is bumped next time, it's better to rewrite this
+    # part using <continue> for "early return".
+    # For more info see CMake Release notes for 3.2 version.
+    # https://cmake.org/cmake/help/latest/release/3.2.html#commands
+    if(NOT FOUND EQUAL -1)
+      string(TOLOWER ${TRYARCH} LUAJIT_ARCH)
+      set(${outvar} ${LUAJIT_ARCH} PARENT_SCOPE)
+      return()
     endif()
-    string(TOLOWER ${TRYARCH} LUAJIT_ARCH)
-    set(${outvar} ${LUAJIT_ARCH} PARENT_SCOPE)
-    return()
   endforeach()
   message(FATAL_ERROR "[LuaJITArch] Unsupported target architecture")
 endfunction()

================================================================================

> 
> > +    endif()
> > +    string(TOLOWER ${TRYARCH} LUAJIT_ARCH)
> > +    set(${outvar} ${LUAJIT_ARCH} PARENT_SCOPE)
> > +    return()
> > +  endforeach()
> > +  message(FATAL_ERROR "[LuaJITArch] Unsupported target architecture")
> 
> It would be nice to check return status from <lj_arch.h> preprocessing
> command. On ppc64 inside Docker (`FROM ppc64le/debian`) configuration
> ends successfully, arch detected as "ppc64":
> 
> | lj_arch.h:436:2: error: #error "No support for PowerPC 64 bit mode (yet)"
> |  #error "No support for PowerPC 64 bit mode (yet)"
> |   ^~~~~
> | lj_arch.h:436:2: error: #error "No support for PowerPC 64 bit mode (yet)"
> |  #error "No support for PowerPC 64 bit mode (yet)"
> |   ^~~~~
> | -- Configuring done
> | WARNING: Target "luajit_static" has EXCLUDE_FROM_ALL set and will not be built by default but an install rule has been provided for it.  CMake does not define behavior for this case.
> | WARNING: Target "libluajit_static" has EXCLUDE_FROM_ALL set and will not be built by default but an install rule has been provided for it.  CMake does not define behavior for this case.
> | WARNING: Target "libluajit_shared" has EXCLUDE_FROM_ALL set and will not be built by default but an install rule has been provided for it.  CMake does not define behavior for this case.
> | WARNING: Target "libluajit_shared" has EXCLUDE_FROM_ALL set and will not be built by default but an install rule has been provided for it.  CMake does not define behavior for this case.
> | -- Generating done
> | -- Build files have been written to: /luajit

Nice! Added, squashed, force-pushed to the branch. Diff is below:

================================================================================

diff --git a/cmake/LuaJITUtils.cmake b/cmake/LuaJITUtils.cmake
index 2c4a27e..be0a8b7 100644
--- a/cmake/LuaJITUtils.cmake
+++ b/cmake/LuaJITUtils.cmake
@@ -7,8 +7,15 @@ function(LuaJITTestArch outvar strflags)
     COMMAND ${CMAKE_C_COMPILER} ${TESTARCH_C_FLAGS} -E lj_arch.h -dM
     WORKING_DIRECTORY ${LUAJIT_SOURCE_DIR}
     OUTPUT_VARIABLE TESTARCH
+    RESULT_VARIABLE TESTARCH_RC
   )
-  set(${outvar} ${TESTARCH} PARENT_SCOPE)
+  if(TESTARCH_RC EQUAL 0)
+    set(${outvar} ${TESTARCH} PARENT_SCOPE)
+  else()
+    # XXX: Yield a special value (an empty string) to respect the
+    # failed preprocessor routine and then fail arch detection.
+    set(${outvar} "" PARENT_SCOPE)
+  endif()
 endfunction()
 
 function(LuaJITArch outvar testarch)

================================================================================

> 
> > +endfunction()
> 

<snipped>

> 
> > diff --git a/cmake/SetDynASMFlags.cmake b/cmake/SetDynASMFlags.cmake
> > new file mode 100644
> > index 0000000..9a920c3
> > --- /dev/null
> > +++ b/cmake/SetDynASMFlags.cmake
> > @@ -0,0 +1,130 @@
> > +# This module exposes following variables to the project:
> 
> Side note: Nice approach, I like it!
> 
> > +# * HOST_C_FLAGS
> > +# * DYNASM_ARCH
> > +# * DYNASM_FLAGS
> > +
> > +# XXX: buildvm includes core headers and thus has to be built
> > +# with the same flags and defines as the LuaJIT core itself.
> > +set(HOST_C_FLAGS)
> > +set(DYNASM_ARCH)
> > +set(DYNASM_FLAGS)
> > +
> > +LuaJITTestArch(TESTARCH "${TARGET_C_FLAGS} ${HOST_CFLAGS}")
> 
> Nit: Looks like it is necessary to use <Makefile.original> to define
> LUAJIT_TARGET directly? I think that should be mentioned in the commit
> message too.

Sorry, don't get this one.

> Feel free to ignore.
> 

<snipped>

> 
> > diff --git a/cmake/SetTargetFlags.cmake b/cmake/SetTargetFlags.cmake
> > new file mode 100644
> > index 0000000..260fc6b
> > --- /dev/null
> > +++ b/cmake/SetTargetFlags.cmake

<snipped>

> 
> Are -dynamiclib and -fPIC flags forced set by CMake?
> Also, I don't find -install_name -compatibility_version -current_version
> flags. Are they insignificant?

Could you please share the corresponding part of "make VERBOSE=1"? These
variables should be set by CMake.

> 
> I got warnings from linker in the build stage on macOS:
> 
> | $ uname -a
> | Darwin MacBook-Pro.local 19.6.0 Darwin Kernel Version 19.6.0: Mon Aug 31 22:12:52 PDT 2020; root:xnu-6153.141.2~1/RELEASE_X86_64 x86_64
> 
> | ld: warning: -seg1addr not 16384 byte aligned, rounding up
> | /Library/Developer/CommandLineTools/usr/bin/ranlib: file: libluajit.a(lj_opt_split.c.o) has no symbols
> | [ 98%] Built target libluajit_shared
> | /Library/Developer/CommandLineTools/usr/bin/ranlib: file: libluajit.a(lj_opt_split.c.o) has no symbols
> 
> Is it related to this?

No, you'll see this if you run <ar> with no suppressing its output. The
same issue is also presented for lj_gdbjit.c when GDBJIT support is
disabled.

> 

<snipped>

> 
> > +# XXX: ASAN-related build flags are stored in CMAKE_C_FLAGS.
> > +set(TARGET_VM_FLAGS "${CMAKE_C_FLAGS} ${TARGET_C_FLAGS}")
> > +
> > +unset(TESTARCH)
> 
> Looks like you forgot to unset LUAJIT_ARCH. Or to add it in the header
> note.

The former, thanks!

> 

<snipped>

> 
> > diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
> > new file mode 100644
> > index 0000000..8ada1a4
> > --- /dev/null
> > +++ b/src/CMakeLists.txt
> > @@ -0,0 +1,391 @@
> 
> <snipped>
> 
> > +make_source_list(SOURCES_UTILS
> > +  SOURCES
> > +    lj_alloc.c
> > +    lj_char.c
> > +    lj_utils_leb128.c
> > +    lj_vmmath.c
> > +    lj_wbuf.c
> 
> Looks like there is no need to compile <lj_utils_leb128.c> and
> <lj_wbuf.c>, when the memory profiler is disabled.

There is no need, but strictly saying both units are not strictly
related to memprof, but rather providing a standalone functionality. So,
I leave it unchanged.

> 

<snipped>

> 
> | # handle this manually on his side. Hence check whether the
> | # targets used below are presented for the chosen build mode.
> | # See more info in CMake docs below:
> | # https://cmake.org/cmake/help/v3.1/prop_tgt/EXCLUDE_FROM_ALL.html
> 
> This warnings are annoying (cmake version is 3.13.4):

Did you try the latest version of the patchset? All these <install>
rules are moved under condition after Timur comment.

> 
> | -- Found assembler: /usr/bin/cc
> | -- Configuring done
> | WARNING: Target "luajit_static" has EXCLUDE_FROM_ALL set and will not be built by default but an install rule has been provided for it.  CMake does not define behavior for this case.
> | WARNING: Target "libluajit_static" has EXCLUDE_FROM_ALL set and will not be built by default but an install rule has been provided for it.  CMake does not define behavior for this case.
> | WARNING: Target "libluajit_shared" has EXCLUDE_FROM_ALL set and will not be built by default but an install rule has been provided for it.  CMake does not define behavior for this case.
> | WARNING: Target "libluajit_shared" has EXCLUDE_FROM_ALL set and will not be built by default but an install rule has been provided for it.  CMake does not define behavior for this case.
> 
> Can we suppress them?
> 
> > +install(TARGETS ${LUAJIT_DEPS}
> 
> The development releases of LuaJIT deliberately are not installed like
> "luajit", but "luajit-2.1.0-beta3". Do we want to change this behaviour?

What do you mean by that? Could you please provide an example?

> 
> > +  RUNTIME
> > +  DESTINATION bin
> > +  COMPONENT luajit
> > +)
> > +install(TARGETS ${LIBLUAJIT_STATIC_DEPS}
> > +  ARCHIVE
> > +  DESTINATION lib
> > +  COMPONENT luajit
> > +)
> > +install(TARGETS ${LIBLUAJIT_SHARED_DEPS}
> > +  LIBRARY
> > +  DESTINATION lib
> > +  COMPONENT luajit
> > +)
> > +
> 
> <snipped>
> 
> > +
> > +install(FILES
> > +    ${CMAKE_CURRENT_SOURCE_DIR}/jit/bc.lua
> > +    ${CMAKE_CURRENT_SOURCE_DIR}/jit/bcsave.lua
> > +    ${CMAKE_CURRENT_SOURCE_DIR}/jit/dump.lua
> > +    ${CMAKE_CURRENT_SOURCE_DIR}/jit/p.lua
> > +    ${CMAKE_CURRENT_SOURCE_DIR}/jit/v.lua
> > +    ${CMAKE_CURRENT_SOURCE_DIR}/jit/zone.lua
> > +    ${CMAKE_CURRENT_SOURCE_DIR}/jit/dis_x86.lua
> > +    ${CMAKE_CURRENT_SOURCE_DIR}/jit/dis_x64.lua
> > +    ${CMAKE_CURRENT_SOURCE_DIR}/jit/dis_arm.lua
> > +    ${CMAKE_CURRENT_SOURCE_DIR}/jit/dis_arm64.lua
> > +    ${CMAKE_CURRENT_SOURCE_DIR}/jit/dis_arm64be.lua
> > +    ${CMAKE_CURRENT_SOURCE_DIR}/jit/dis_ppc.lua
> > +    ${CMAKE_CURRENT_SOURCE_DIR}/jit/dis_mips.lua
> > +    ${CMAKE_CURRENT_SOURCE_DIR}/jit/dis_mipsel.lua
> > +    ${CMAKE_CURRENT_SOURCE_DIR}/jit/dis_mips64.lua
> > +    ${CMAKE_CURRENT_SOURCE_DIR}/jit/dis_mips64el.lua
> > +    ${CMAKE_CURRENT_BINARY_DIR}/jit/vmdef.lua
> 
> Nit: Is order important? Why are they not listed in alphabetical order?
> Feel free to ignore.

Reordered, squashed, force-pushed to the branch.

> 
> > +  DESTINATION ${LUAJIT_DATAROOTDIR}/jit
> > +  PERMISSIONS
> > +    OWNER_READ OWNER_WRITE
> > +    GROUP_READ
> > +    WORLD_READ
> > +  COMPONENT luajit
> > +)
> 
> As I read from [4] CMake does not provide a default way to uninstall
> installed files. But fortunately there is an example how doing it
> in [4] directly. I think it is good to implement this part for
> backward compatibility (especially, we have all prerequisites).

OK, but I adjusted it a bit:

================================================================================

diff --git a/.gitignore b/.gitignore
index a21ee1c..2103a30 100644
--- a/.gitignore
+++ b/.gitignore
@@ -15,6 +15,7 @@ CMakeCache.txt
 CMakeFiles
 Makefile
 cmake_install.cmake
+cmake_uninstall.cmake
 compile_commands.json
 install_manifest.txt
 luajit-parse-memprof
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 16a9d5b..6bba9f4 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -261,3 +261,16 @@ add_subdirectory(etc)
 # --- Tools --------------------------------------------------------------------
 
 add_subdirectory(tools)
+
+# --- Misc rules ---------------------------------------------------------------
+
+# XXX: Implement <uninstall> target using the following recipe:
+# https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake
+if(NOT TARGET uninstall)
+  configure_file(${CMAKE_MODULE_PATH}/cmake_uninstall.cmake.in
+    cmake_uninstall.cmake @ONLY ESCAPE_QUOTES)
+
+  add_custom_target(uninstall
+    COMMAND ${CMAKE_COMMAND} -P ${CMAKE_CURRENT_BINARY_DIR}/cmake_uninstall.cmake
+  )
+endif()
diff --git a/cmake/cmake_uninstall.cmake.in b/cmake/cmake_uninstall.cmake.in
new file mode 100644
index 0000000..fc3b885
--- /dev/null
+++ b/cmake/cmake_uninstall.cmake.in
@@ -0,0 +1,24 @@
+if(NOT EXISTS "@CMAKE_BINARY_DIR@/install_manifest.txt")
+  message(FATAL_ERROR "Cannot find install manifest: @CMAKE_BINARY_DIR@/install_manifest.txt")
+endif()
+
+# XXX: This loop removes only entries from install_manifest.txt,
+# but do nothing for the directories created while installation.
+# Honestly, the recipe is awful, but is better than nothing.
+file(READ "@CMAKE_BINARY_DIR@/install_manifest.txt" files)
+string(REGEX REPLACE "\n" ";" files "${files}")
+foreach(file ${files})
+  message(STATUS "Uninstalling $ENV{DESTDIR}${file}")
+  if(IS_SYMLINK "$ENV{DESTDIR}${file}" OR EXISTS "$ENV{DESTDIR}${file}")
+    execute_process(
+      COMMAND @CMAKE_COMMAND@ -E remove "$ENV{DESTDIR}${file}"
+      RESULT_VARIABLE REMOVE_RC
+      OUTPUT_QUIET
+    )
+    if(NOT "${REMOVE_RC}" STREQUAL 0)
+      message(FATAL_ERROR "Problem when removing $ENV{DESTDIR}${file}")
+    endif()
+  else(IS_SYMLINK "$ENV{DESTDIR}${file}" OR EXISTS "$ENV{DESTDIR}${file}")
+    message(STATUS "File $ENV{DESTDIR}${file} does not exist.")
+  endif()
+endforeach()

================================================================================

> 
> > diff --git a/src/host/CMakeLists.txt b/src/host/CMakeLists.txt
> > new file mode 100644
> > index 0000000..fe2de5c
> > --- /dev/null
> > +++ b/src/host/CMakeLists.txt
> > @@ -0,0 +1,61 @@
> > +# Building the toolchain for LuaJIT VM preprocessing.
> > +
> > +cmake_minimum_required(VERSION 2.8.12 FATAL_ERROR)
> > +
> > +# FIXME: Both minilua and buildvm need to be build with the HOST_*
> 
> Sorry, I don't get it. What is the HOST_* toolchain?

HOST_CC, HOST_CFLAGS, HOST_LDFLAGS, HOST_LIBS, HOST_SYS -- see
<src/Makefile.original> for more details.

> 
> > +# toolchain.
> > +
> > +# XXX: DynASM flags are set considering the target arch
> 

<snipped>

> 
> > diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt
> > new file mode 100644
> > index 0000000..f9ffc5e
> > --- /dev/null
> > +++ b/tools/CMakeLists.txt
> > @@ -0,0 +1,77 @@
> > +# Building tools for developing with uJIT.
> > +# Major portions taken verbatim or adapted from the uJIT.
> > +# Copyright (C) 2015-2019 IPONWEB Ltd.
> > +
> > +# See the rationale in the root CMakeLists.txt
> > +cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
> > +
> > +set(LUAJIT_TOOLS_DEPS)
> > +
> > +if(LUAJIT_DISABLE_MEMPROF)
> > +  message(STATUS "LuaJIT memory profiler support is disabled")
> > +else()
> > +  # XXX: Can use genex here since the value need to be evaluated
> > +  # on the configuration phase. Fortunately, we know the exact
> 
> Typo: s/on/in/

Niether of these. 'at' or 'while' are fine (I chose the former).

> 
> > +  # path where LuaJIT binary is located.
> > +  set(LUAJIT_TOOLS_BIN ${LUAJIT_BINARY_DIR}/${LUAJIT_CLI_NAME})
> > +  set(LUAJIT_TOOLS_DIR ${CMAKE_CURRENT_SOURCE_DIR})
> > +  configure_file(luajit-parse-memprof.in luajit-parse-memprof @ONLY ESCAPE_QUOTES)
> > +
> 
> <snipped>
> 
> > +  install(FILES
> > +      ${CMAKE_CURRENT_SOURCE_DIR}/memprof.lua
> > +    DESTINATION ${LUAJIT_DATAROOTDIR}
> > +    PERMISSIONS
> > +      OWNER_READ OWNER_WRITE
> > +      GROUP_READ
> > +      WORLD_READ
> > +    COMPONENT tools-parse-memprof
> > +  )
> > +  install(CODE
> 
> Is it reasonable to install memrpof tooling for not supported arches?
> Yes, it's already working in that way, but maybe it is redundant.

I see no problem: this part works fine on all hosts, so you can take the
binary dump and parse it anywhere.

> 
> > +    # XXX: Since the auxiliary script need to be configured in
> > +    # other way it need to be reconfigured it prior to its
> > +    # installation. Unfortunately, we need to manually specify
> 
> Nit: I propose to reformulate and split the first sentence like the
> following:
> 
> | XXX: The auxiliary script needs to be configured for use in
> | repository directly. In another way it needs to be reconfigured
> | prior to installation.
> 
> Feel free to ignore.

See the changes below.

> 
> > +    # the installation path in <configure_file> command.
> > +    # Hope this script will be gone as a result of the issue below
> > +    # https://github.com/tarantool/tarantool/issues/5688.
> > +    "
> > +      set(LUAJIT_TOOLS_BIN ${CMAKE_INSTALL_PREFIX}/bin/${LUAJIT_CLI_NAME})
> > +      set(LUAJIT_TOOLS_DIR ${CMAKE_INSTALL_PREFIX}/${LUAJIT_DATAROOTDIR})
> > +      configure_file(${CMAKE_CURRENT_SOURCE_DIR}/luajit-parse-memprof.in
> > +        ${CMAKE_INSTALL_PREFIX}/bin/luajit-parse-memprof @ONLY ESCAPE_QUOTES)
> > +      message(STATUS \"Installing: ${CMAKE_INSTALL_PREFIX}/bin/luajit-parse-memprof\")
> > +    "
> > +    COMPONENT tools-parse-memprof
> > +  )
> 
> The result file is not executable.

Shit. BTW, the one generated above in bindir is neither. You can't
imagine how I reproach myself about approving this file addition. It's
like a vermiform appendix that we can't cut off right now. I've made the
template executable though this is nonsense but I failed to find another
normal solution. Anyway, I left the comment regarding it, squashed with
the previous commit. Here is the diff for both comments above:

================================================================================

diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt
index 60535cd..cb49bdc 100644
--- a/tools/CMakeLists.txt
+++ b/tools/CMakeLists.txt
@@ -16,6 +16,13 @@ else()
   # path where LuaJIT binary is located.
   set(LUAJIT_TOOLS_BIN ${LUAJIT_BINARY_DIR}/${LUAJIT_CLI_NAME})
   set(LUAJIT_TOOLS_DIR ${CMAKE_CURRENT_SOURCE_DIR})
+  # XXX: Unfortunately, there is no convenient way to set
+  # particular permissions to the output file via CMake.
+  # Furthermore, I even failed to copy the given file to the same
+  # path to change its permissions. After looking at the docs, I
+  # realized that the valid solution would be too monstrous for
+  # such a simple task. As a result I've made the template itself
+  # executable, so the issue is resolved.
   configure_file(luajit-parse-memprof.in luajit-parse-memprof @ONLY ESCAPE_QUOTES)
 
   add_custom_target(tools-parse-memprof EXCLUDE_FROM_ALL DEPENDS
@@ -58,18 +65,24 @@ else()
     COMPONENT tools-parse-memprof
   )
   install(CODE
-    # XXX: Since the auxiliary script need to be configured in
-    # other way it need to be reconfigured it prior to its
-    # installation. Unfortunately, we need to manually specify
-    # the installation path in <configure_file> command.
-    # Hope this script will be gone as a result of the issue below
+    # XXX: The auxiliary script needs to be configured for to be
+    # used in repository directly. It also need to be reconfigured
+    # prior to its installation. The temporary <configure_file>
+    # output is stored to the project build directory and removed
+    # later after being installed. This script will have gone as a
+    # result of the issue:
     # https://github.com/tarantool/tarantool/issues/5688.
     "
+      set(LJPM_INSTALL_PATH ${CMAKE_INSTALL_PREFIX}/bin/luajit-parse-memprof)
       set(LUAJIT_TOOLS_BIN ${CMAKE_INSTALL_PREFIX}/bin/${LUAJIT_CLI_NAME})
       set(LUAJIT_TOOLS_DIR ${CMAKE_INSTALL_PREFIX}/${LUAJIT_DATAROOTDIR})
       configure_file(${CMAKE_CURRENT_SOURCE_DIR}/luajit-parse-memprof.in
-        ${CMAKE_INSTALL_PREFIX}/bin/luajit-parse-memprof @ONLY ESCAPE_QUOTES)
-      message(STATUS \"Installing: ${CMAKE_INSTALL_PREFIX}/bin/luajit-parse-memprof\")
+        ${PROJECT_BINARY_DIR}/luajit-parse-memprof @ONLY ESCAPE_QUOTES)
+      file(INSTALL ${PROJECT_BINARY_DIR}/luajit-parse-memprof
+        DESTINATION ${CMAKE_INSTALL_PREFIX}/bin
+        USE_SOURCE_PERMISSIONS
+      )
+      file(REMOVE ${PROJECT_BINARY_DIR}/luajit-parse-memprof)
     "
     COMPONENT tools-parse-memprof
   )
diff --git a/tools/luajit-parse-memprof.in b/tools/luajit-parse-memprof.in
old mode 100644
new mode 100755

================================================================================

> 
> > +endif()
> > +
> > +add_custom_target(LuaJIT-tools DEPENDS ${LUAJIT_TOOLS_DEPS})
> 
> Side note: why don't you provide custom `tools` target for memprof like
> in the previous patch?

For what? In the previous patch there was an additional work to do, that
is a dependency for the main targets, so I grouped it into a separate
target.

> 
> > -- 
> > 2.25.0
> > 
> 
> [1]: https://stackoverflow.com/questions/41773161/negate-boolean-variable-in-cmake
> [2]: https://cmake.org/pipermail/cmake/2015-July/061116.html
> [3]: https://cmake.org/cmake/help/latest/command/continue.html
> [4]: https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake
> 
> -- 
> Best regards,
> Sergey Kaplun

[1]: https://cmake.org/cmake/help/v3.1/variable/CMAKE_LIBRARY_ARCHITECTURE.html

-- 
Best regards,
IM