[Tarantool-patches] [PATCH 2/3] build: adjust LuaJIT build system

[Timur Safin]

: From: Igor Munkin <imun at tarantool.org>
: Subject: Re: [PATCH 2/3] build: adjust LuaJIT build system
: 
: Sergey,
: 
: Thanks for your review!
: 
...
: 
: > > +macro(TestAndAppendFLag flags flag)
: >
: > Nit: Why macro not function? It is visible to all project after
: > including.
: > Feel free to ignore.
: 
: <flags> variable need to be expanded twice: the first time to
: interpolate the variable name and the second time to interpolate that
: variable value. Unfortunately, there is no way to implement it in a
: different way via CMake.

Yup, with function instead of macro it would be very verbose and
Unreadable. Macro is the simplest and cleanest way here for their
purpose.

: 
: Ignoring.
: 
: >
: > > +    string(REGEX REPLACE "-" "_" TESTFLAG ${flag})
: > > +    string(TOUPPER ${TESTFLAG} TESTFLAG)
: > > +    # XXX: can't use string(PREPEND ...) on ancient versions.
: > > +    set(TESTFLAG "CC_HAS${TESTFLAG}")
: > > +    if(${${TESTFLAG}})
: > > +        set(${flags} "${${flags}} ${flag}")
: 
: Here is the problem spot.
: 
: > >      endif()
: > >  endmacro()
: > >
: 
: <snipped>
: 
: 
: >
: > > +    "Generate <test> target" FORCE)
: > > +
: > > +# Enable internal LuaJIT assertions for Tarantool Debug build.
: > > +if(CMAKE_BUILD_TYPE STREQUAL "Debug")
: >
: > Side note: I get tons of warnings like:
: >
: > | [ 47%] Building C object
: third_party/luajit/src/CMakeFiles/core_static.dir/lib_string.c.o
: > | [ 47%] Building C object
: third_party/luajit/src/CMakeFiles/core_static.dir/lib_table.c.o
: > | /home/burii/reviews/tarantool/luajit-
: cmake/third_party/luajit/src/lj_strfmt.c: In function 'lj_strfmt_putfxint':
: > | /home/burii/reviews/tarantool/luajit-
: cmake/third_party/luajit/src/lj_strfmt.c:260:9: warning: variable 'ps' set
: but not used [-Wunused-but-set-variable]
: > |   260 |   char *ps;
: > |       |         ^~
: >
: > If build with `-DCMAKE_BUILD_TYPE=Debug` first, **do not** run
: > `rm CMakeCache.txt` and then build with
: > `-DCMAKE_BUILD_TYPE=RelWithDebInfo`. I am not familiar with CMake, is it
: > an expected behaviour?
: 
: I don't know. I guess there is something cached in CMakeCache.txt that
: leads to such warnings. Maybe Timur will clarify this behaviour.

Yes, toolchain settings (including compiler settings and build mode)
Are cached in CMakeCache.txt and not reassigned from command-line if
there is cached value. You have to delete toolchain cached values
to make all variables be redefined.

If you not delete CMAkeCache.txt you get shorter cmake cycle,
which might be not exactly what you hoped to get.

: 
: >
: 
: <snipped>
: 
: > > +add_dependencies(build_bundled_libs libluajit)
: >
: > Nit: Looks like it should be in the root <CMakeLists.txt>, not here.
: > Feel free to ignore.
: 
: Originally, it was here, so I would like to leave it here too.

The more local - the better. Agreed. 

: 
: Ignoring.
: 
: >
: 

Timur