* [Tarantool-patches] [PATCH 0/2] Minor fixes for new LuaJIT extention library
@ 2020-10-17 11:13 Sergey Kaplun
2020-10-17 11:13 ` [Tarantool-patches] [PATCH 1/2] build: provide missing LuaJIT lmisclib.h header Sergey Kaplun
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Sergey Kaplun @ 2020-10-17 11:13 UTC (permalink / raw)
To: Alexander Turenko, Igor Munkin, Alexander V . Tikhonov; +Cc: tarantool-patches
This patchset consists from 2 parts:
1) Provide corresponding new LuaJIT header within tarantool at install.
2) Add missing public C API symbols from the new lib into exports.h
Branch: https://github.com/tarantool/tarantool/tree/skaplun/gh-5187-luajit-include-headers
CI is red, but fails related to replication tests or at osx_* jobs
(see also https://github.com/tarantool/tarantool/issues/5423).
CI: https://gitlab.com/tarantool/tarantool/-/pipelines/203892776
Sergey Kaplun (2):
build: provide missing LuaJIT lmisclib.h header
lua: add missing LuaJIT export symbols
FreeBSD/databases/tarantool/pkg-plist | 1 +
cmake/luajit.cmake | 2 +-
rpm/tarantool.spec | 1 +
src/exports.h | 2 ++
4 files changed, 5 insertions(+), 1 deletion(-)
--
2.28.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Tarantool-patches] [PATCH 1/2] build: provide missing LuaJIT lmisclib.h header
2020-10-17 11:13 [Tarantool-patches] [PATCH 0/2] Minor fixes for new LuaJIT extention library Sergey Kaplun
@ 2020-10-17 11:13 ` Sergey Kaplun
2020-10-18 19:19 ` Igor Munkin
2020-10-17 11:13 ` [Tarantool-patches] [PATCH 2/2] lua: add missing LuaJIT export symbols Sergey Kaplun
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Sergey Kaplun @ 2020-10-17 11:13 UTC (permalink / raw)
To: Alexander Turenko, Igor Munkin, Alexander V . Tikhonov; +Cc: tarantool-patches
Since LuaJIT provides extended LuaC API introduced in the scope of
5a61e1ab54b5c66bfebd836db1ac47996611e065 ('misc: add C and Lua API
for platform metrics') corresponding header should be installed
together with tarantool.
Follows up tarantool/tarantool#5187
---
I checked all references of LuaJIT provided headers as follows:
===================================================================
grep "lualib.h" -R ~/builds_workspace/tarantool/master/ | grep -v -e tags -e CMakeFiles -e "#include" -e Makefile.dep -e third_party
/home/burii/builds_workspace/tarantool/master/rpm/tarantool.spec:%{_includedir}/tarantool/lualib.h
Binary file /home/burii/builds_workspace/tarantool/master/cmake/.luajit.cmake.swp matches
/home/burii/builds_workspace/tarantool/master/cmake/luajit.cmake: install (FILES ${inc}/lua.h ${inc}/lualib.h ${inc}/lauxlib.h
/home/burii/builds_workspace/tarantool/master/FreeBSD/databases/tarantool/pkg-plist:include/tarantool/lualib.h
===================================================================
Did I miss something?
FreeBSD/databases/tarantool/pkg-plist | 1 +
cmake/luajit.cmake | 2 +-
rpm/tarantool.spec | 1 +
3 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/FreeBSD/databases/tarantool/pkg-plist b/FreeBSD/databases/tarantool/pkg-plist
index 9e3905d60..6df72be9d 100644
--- a/FreeBSD/databases/tarantool/pkg-plist
+++ b/FreeBSD/databases/tarantool/pkg-plist
@@ -8,6 +8,7 @@ include/tarantool/lua.hpp
include/tarantool/luaconf.h
include/tarantool/luajit.h
include/tarantool/lualib.h
+include/tarantool/lmisclib.h
include/tarantool/module.h
man/man1/tarantool.1.gz
man/man1/tarantoolctl.1.gz
diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake
index 555bc8371..1c7784e11 100644
--- a/cmake/luajit.cmake
+++ b/cmake/luajit.cmake
@@ -287,7 +287,7 @@ macro(luajit_build)
unset (luajit_buildoptions)
set (inc ${PROJECT_SOURCE_DIR}/third_party/luajit/src)
install (FILES ${inc}/lua.h ${inc}/lualib.h ${inc}/lauxlib.h
- ${inc}/luaconf.h ${inc}/lua.hpp ${inc}/luajit.h
+ ${inc}/luaconf.h ${inc}/lua.hpp ${inc}/luajit.h ${inc}/lmisclib.h
DESTINATION ${MODULE_INCLUDEDIR})
endmacro()
diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
index eedc0312c..3a1a8f2be 100644
--- a/rpm/tarantool.spec
+++ b/rpm/tarantool.spec
@@ -256,6 +256,7 @@ fi
%{_includedir}/tarantool/luajit.h
%{_includedir}/tarantool/lualib.h
%{_includedir}/tarantool/module.h
+%{_includedir}/tarantool/lmisclib.h
%changelog
* Tue Sep 12 2017 Roman Tsisyk <roman@tarantool.org> 1.7.5.46-1
--
2.28.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Tarantool-patches] [PATCH 2/2] lua: add missing LuaJIT export symbols
2020-10-17 11:13 [Tarantool-patches] [PATCH 0/2] Minor fixes for new LuaJIT extention library Sergey Kaplun
2020-10-17 11:13 ` [Tarantool-patches] [PATCH 1/2] build: provide missing LuaJIT lmisclib.h header Sergey Kaplun
@ 2020-10-17 11:13 ` Sergey Kaplun
2020-10-18 18:43 ` Igor Munkin
2020-10-21 16:08 ` [Tarantool-patches] [PATCH 0/2] Minor fixes for new LuaJIT extention library Alexander Turenko
2020-10-21 16:17 ` Alexander V. Tikhonov
3 siblings, 1 reply; 12+ messages in thread
From: Sergey Kaplun @ 2020-10-17 11:13 UTC (permalink / raw)
To: Alexander Turenko, Igor Munkin, Alexander V . Tikhonov; +Cc: tarantool-patches
Since LuaJIT provides public C API symbols that are used in the final
executable, the linker may not just throw it away.
Nevertheless for future compatibility all symbols from LuaJIT public API
should be additionaly added at exports.h file.
Follows up tarantool/tarantool#5187
---
src/exports.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/exports.h b/src/exports.h
index 8bab22574..867a027dc 100644
--- a/src/exports.h
+++ b/src/exports.h
@@ -400,6 +400,7 @@ EXPORT(luaL_traceback)
EXPORT(luaL_typerror)
EXPORT(luaL_unref)
EXPORT(luaL_where)
+EXPORT(luaM_metrics)
EXPORT(luaopen_base)
EXPORT(luaopen_bit)
EXPORT(luaopen_debug)
@@ -407,6 +408,7 @@ EXPORT(luaopen_ffi)
EXPORT(luaopen_io)
EXPORT(luaopen_jit)
EXPORT(luaopen_math)
+EXPORT(luaopen_misc)
EXPORT(luaopen_os)
EXPORT(luaopen_package)
EXPORT(luaopen_string)
--
2.28.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH 2/2] lua: add missing LuaJIT export symbols
2020-10-17 11:13 ` [Tarantool-patches] [PATCH 2/2] lua: add missing LuaJIT export symbols Sergey Kaplun
@ 2020-10-18 18:43 ` Igor Munkin
2020-10-19 4:57 ` Sergey Kaplun
0 siblings, 1 reply; 12+ messages in thread
From: Igor Munkin @ 2020-10-18 18:43 UTC (permalink / raw)
To: Sergey Kaplun; +Cc: tarantool-patches, Alexander Turenko
Sergey,
Thanks for the patch! LGTM, except a single nit.
On 17.10.20, Sergey Kaplun wrote:
> Since LuaJIT provides public C API symbols that are used in the final
> executable, the linker may not just throw it away.
> Nevertheless for future compatibility all symbols from LuaJIT public API
> should be additionaly added at exports.h file.
>
> Follows up tarantool/tarantool#5187
Minor: You don't need to use full repo reference here.
> ---
> src/exports.h | 2 ++
> 1 file changed, 2 insertions(+)
>
<snipped>
> --
> 2.28.0
>
--
Best regards,
IM
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH 1/2] build: provide missing LuaJIT lmisclib.h header
2020-10-17 11:13 ` [Tarantool-patches] [PATCH 1/2] build: provide missing LuaJIT lmisclib.h header Sergey Kaplun
@ 2020-10-18 19:19 ` Igor Munkin
2020-10-19 4:55 ` Sergey Kaplun
0 siblings, 1 reply; 12+ messages in thread
From: Igor Munkin @ 2020-10-18 19:19 UTC (permalink / raw)
To: Sergey Kaplun; +Cc: tarantool-patches, Alexander Turenko
Sergey,
Thanks for the patch! LGTM, except the nits below.
On 17.10.20, Sergey Kaplun wrote:
> Since LuaJIT provides extended LuaC API introduced in the scope of
> 5a61e1ab54b5c66bfebd836db1ac47996611e065 ('misc: add C and Lua API
> for platform metrics') corresponding header should be installed
> together with tarantool.
Typo: s/tarantool/Tarantool/.
Minor: Strictly saying it should not. The header is provided by another
package (so called "devel" package), so I propose the following wording:
| corresponding header should be provided along with other Tarantool
| development files.
>
> Follows up tarantool/tarantool#5187
Minor: You don't need to use full repo reference here.
> ---
>
> I checked all references of LuaJIT provided headers as follows:
>
> ===================================================================
> grep "lualib.h" -R ~/builds_workspace/tarantool/master/ | grep -v -e tags -e CMakeFiles -e "#include" -e Makefile.dep -e third_party
> /home/burii/builds_workspace/tarantool/master/rpm/tarantool.spec:%{_includedir}/tarantool/lualib.h
> Binary file /home/burii/builds_workspace/tarantool/master/cmake/.luajit.cmake.swp matches
> /home/burii/builds_workspace/tarantool/master/cmake/luajit.cmake: install (FILES ${inc}/lua.h ${inc}/lualib.h ${inc}/lauxlib.h
> /home/burii/builds_workspace/tarantool/master/FreeBSD/databases/tarantool/pkg-plist:include/tarantool/lualib.h
> ===================================================================
>
> Did I miss something?
I guess no: Gentoo ebuilds are maintained outside the Tarantool repo by
Sasha Tu. However, I have no idea, how packages for Debian-based distros
are built. Anyway, it looks everything inside this repo is fixed.
>
>
> FreeBSD/databases/tarantool/pkg-plist | 1 +
> cmake/luajit.cmake | 2 +-
> rpm/tarantool.spec | 1 +
> 3 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/FreeBSD/databases/tarantool/pkg-plist b/FreeBSD/databases/tarantool/pkg-plist
> index 9e3905d60..6df72be9d 100644
> --- a/FreeBSD/databases/tarantool/pkg-plist
> +++ b/FreeBSD/databases/tarantool/pkg-plist
> @@ -8,6 +8,7 @@ include/tarantool/lua.hpp
> include/tarantool/luaconf.h
> include/tarantool/luajit.h
> include/tarantool/lualib.h
> +include/tarantool/lmisclib.h
Minor: Please sort the entries.
> include/tarantool/module.h
> man/man1/tarantool.1.gz
> man/man1/tarantoolctl.1.gz
<snipped>
> diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
> index eedc0312c..3a1a8f2be 100644
> --- a/rpm/tarantool.spec
> +++ b/rpm/tarantool.spec
> @@ -256,6 +256,7 @@ fi
> %{_includedir}/tarantool/luajit.h
> %{_includedir}/tarantool/lualib.h
> %{_includedir}/tarantool/module.h
> +%{_includedir}/tarantool/lmisclib.h
>
> %changelog
> * Tue Sep 12 2017 Roman Tsisyk <roman@tarantool.org> 1.7.5.46-1
Side note: It looks we totally ignore updating %changelog section.
> --
> 2.28.0
>
--
Best regards,
IM
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH 1/2] build: provide missing LuaJIT lmisclib.h header
2020-10-18 19:19 ` Igor Munkin
@ 2020-10-19 4:55 ` Sergey Kaplun
2020-10-20 12:07 ` Sergey Ostanevich
0 siblings, 1 reply; 12+ messages in thread
From: Sergey Kaplun @ 2020-10-19 4:55 UTC (permalink / raw)
To: Igor Munkin; +Cc: tarantool-patches, Alexander Turenko
Hi, Igor!
Thanks for the review!
On 18.10.20, Igor Munkin wrote:
> Sergey,
>
> Thanks for the patch! LGTM, except the nits below.
>
> On 17.10.20, Sergey Kaplun wrote:
> > Since LuaJIT provides extended LuaC API introduced in the scope of
> > 5a61e1ab54b5c66bfebd836db1ac47996611e065 ('misc: add C and Lua API
> > for platform metrics') corresponding header should be installed
> > together with tarantool.
>
> Typo: s/tarantool/Tarantool/.
> Minor: Strictly saying it should not. The header is provided by another
> package (so called "devel" package), so I propose the following wording:
> | corresponding header should be provided along with other Tarantool
> | development files.
>
> >
> > Follows up tarantool/tarantool#5187
>
> Minor: You don't need to use full repo reference here.
Thanks! I've updated commit message considering your comments here.
>
> > ---
> >
> > I checked all references of LuaJIT provided headers as follows:
> >
> > ===================================================================
> > grep "lualib.h" -R ~/builds_workspace/tarantool/master/ | grep -v -e tags -e CMakeFiles -e "#include" -e Makefile.dep -e third_party
> > /home/burii/builds_workspace/tarantool/master/rpm/tarantool.spec:%{_includedir}/tarantool/lualib.h
> > Binary file /home/burii/builds_workspace/tarantool/master/cmake/.luajit.cmake.swp matches
> > /home/burii/builds_workspace/tarantool/master/cmake/luajit.cmake: install (FILES ${inc}/lua.h ${inc}/lualib.h ${inc}/lauxlib.h
> > /home/burii/builds_workspace/tarantool/master/FreeBSD/databases/tarantool/pkg-plist:include/tarantool/lualib.h
> > ===================================================================
> >
> > Did I miss something?
>
> I guess no: Gentoo ebuilds are maintained outside the Tarantool repo by
> Sasha Tu. However, I have no idea, how packages for Debian-based distros
> are built. Anyway, it looks everything inside this repo is fixed.
>
> >
> >
> > FreeBSD/databases/tarantool/pkg-plist | 1 +
> > cmake/luajit.cmake | 2 +-
> > rpm/tarantool.spec | 1 +
> > 3 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/FreeBSD/databases/tarantool/pkg-plist b/FreeBSD/databases/tarantool/pkg-plist
> > index 9e3905d60..6df72be9d 100644
> > --- a/FreeBSD/databases/tarantool/pkg-plist
> > +++ b/FreeBSD/databases/tarantool/pkg-plist
> > @@ -8,6 +8,7 @@ include/tarantool/lua.hpp
> > include/tarantool/luaconf.h
> > include/tarantool/luajit.h
> > include/tarantool/lualib.h
> > +include/tarantool/lmisclib.h
>
> Minor: Please sort the entries.
I've sorted entries here and inside rpm/tarantool.spec. See itterative
patch below. Branch is force pushed.
>
> > include/tarantool/module.h
> > man/man1/tarantool.1.gz
> > man/man1/tarantoolctl.1.gz
>
> <snipped>
>
> > diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
> > index eedc0312c..3a1a8f2be 100644
> > --- a/rpm/tarantool.spec
> > +++ b/rpm/tarantool.spec
> > @@ -256,6 +256,7 @@ fi
> > %{_includedir}/tarantool/luajit.h
> > %{_includedir}/tarantool/lualib.h
> > %{_includedir}/tarantool/module.h
> > +%{_includedir}/tarantool/lmisclib.h
> >
> > %changelog
> > * Tue Sep 12 2017 Roman Tsisyk <roman@tarantool.org> 1.7.5.46-1
>
> Side note: It looks we totally ignore updating %changelog section.
>
> > --
> > 2.28.0
> >
>
> --
> Best regards,
> IM
===================================================================
diff --git a/FreeBSD/databases/tarantool/pkg-plist b/FreeBSD/databases/tarantool/pkg-plist
index 6df72be9d..8e2f3afde 100644
--- a/FreeBSD/databases/tarantool/pkg-plist
+++ b/FreeBSD/databases/tarantool/pkg-plist
@@ -3,12 +3,12 @@ bin/tarantoolctl
%%ETCDIR%%/default/tarantool
%%ETCDIR%%/instances.available/example.lua
include/tarantool/lauxlib.h
+include/tarantool/lmisclib.h
include/tarantool/lua.h
include/tarantool/lua.hpp
include/tarantool/luaconf.h
include/tarantool/luajit.h
include/tarantool/lualib.h
-include/tarantool/lmisclib.h
include/tarantool/module.h
man/man1/tarantool.1.gz
man/man1/tarantoolctl.1.gz
diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
index 3a1a8f2be..9ac7a1a04 100644
--- a/rpm/tarantool.spec
+++ b/rpm/tarantool.spec
@@ -250,13 +250,13 @@ fi
%files devel
%dir %{_includedir}/tarantool
%{_includedir}/tarantool/lauxlib.h
+%{_includedir}/tarantool/lmisclib.h
%{_includedir}/tarantool/luaconf.h
%{_includedir}/tarantool/lua.h
%{_includedir}/tarantool/lua.hpp
%{_includedir}/tarantool/luajit.h
%{_includedir}/tarantool/lualib.h
%{_includedir}/tarantool/module.h
-%{_includedir}/tarantool/lmisclib.h
%changelog
* Tue Sep 12 2017 Roman Tsisyk <roman@tarantool.org> 1.7.5.46-1
===================================================================
--
Best regards,
Sergey Kaplun
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH 2/2] lua: add missing LuaJIT export symbols
2020-10-18 18:43 ` Igor Munkin
@ 2020-10-19 4:57 ` Sergey Kaplun
2020-10-20 12:09 ` Sergey Ostanevich
0 siblings, 1 reply; 12+ messages in thread
From: Sergey Kaplun @ 2020-10-19 4:57 UTC (permalink / raw)
To: Igor Munkin; +Cc: tarantool-patches, Alexander Turenko
Igor,
Thanks for the review! I've updated commit message considering your
comment here. Branch is force pushed.
On 18.10.20, Igor Munkin wrote:
> Sergey,
>
> Thanks for the patch! LGTM, except a single nit.
>
> On 17.10.20, Sergey Kaplun wrote:
> > Since LuaJIT provides public C API symbols that are used in the final
> > executable, the linker may not just throw it away.
> > Nevertheless for future compatibility all symbols from LuaJIT public API
> > should be additionaly added at exports.h file.
> >
> > Follows up tarantool/tarantool#5187
>
> Minor: You don't need to use full repo reference here.
>
> > ---
> > src/exports.h | 2 ++
> > 1 file changed, 2 insertions(+)
> >
>
> <snipped>
>
> > --
> > 2.28.0
> >
>
> --
> Best regards,
> IM
--
Best regards,
Sergey Kaplun
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH 1/2] build: provide missing LuaJIT lmisclib.h header
2020-10-19 4:55 ` Sergey Kaplun
@ 2020-10-20 12:07 ` Sergey Ostanevich
0 siblings, 0 replies; 12+ messages in thread
From: Sergey Ostanevich @ 2020-10-20 12:07 UTC (permalink / raw)
To: Sergey Kaplun; +Cc: Alexander Turenko, tarantool-patches
Hi!
Thanks, this one LGTM on the latest branch.
Sergos.
On 19 окт 07:55, Sergey Kaplun wrote:
> Hi, Igor!
> Thanks for the review!
>
> On 18.10.20, Igor Munkin wrote:
> > Sergey,
> >
> > Thanks for the patch! LGTM, except the nits below.
> >
> > On 17.10.20, Sergey Kaplun wrote:
> > > Since LuaJIT provides extended LuaC API introduced in the scope of
> > > 5a61e1ab54b5c66bfebd836db1ac47996611e065 ('misc: add C and Lua API
> > > for platform metrics') corresponding header should be installed
> > > together with tarantool.
> >
> > Typo: s/tarantool/Tarantool/.
> > Minor: Strictly saying it should not. The header is provided by another
> > package (so called "devel" package), so I propose the following wording:
> > | corresponding header should be provided along with other Tarantool
> > | development files.
> >
> > >
> > > Follows up tarantool/tarantool#5187
> >
> > Minor: You don't need to use full repo reference here.
>
> Thanks! I've updated commit message considering your comments here.
>
> >
> > > ---
> > >
> > > I checked all references of LuaJIT provided headers as follows:
> > >
> > > ===================================================================
> > > grep "lualib.h" -R ~/builds_workspace/tarantool/master/ | grep -v -e tags -e CMakeFiles -e "#include" -e Makefile.dep -e third_party
> > > /home/burii/builds_workspace/tarantool/master/rpm/tarantool.spec:%{_includedir}/tarantool/lualib.h
> > > Binary file /home/burii/builds_workspace/tarantool/master/cmake/.luajit.cmake.swp matches
> > > /home/burii/builds_workspace/tarantool/master/cmake/luajit.cmake: install (FILES ${inc}/lua.h ${inc}/lualib.h ${inc}/lauxlib.h
> > > /home/burii/builds_workspace/tarantool/master/FreeBSD/databases/tarantool/pkg-plist:include/tarantool/lualib.h
> > > ===================================================================
> > >
> > > Did I miss something?
> >
> > I guess no: Gentoo ebuilds are maintained outside the Tarantool repo by
> > Sasha Tu. However, I have no idea, how packages for Debian-based distros
> > are built. Anyway, it looks everything inside this repo is fixed.
> >
> > >
> > >
> > > FreeBSD/databases/tarantool/pkg-plist | 1 +
> > > cmake/luajit.cmake | 2 +-
> > > rpm/tarantool.spec | 1 +
> > > 3 files changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/FreeBSD/databases/tarantool/pkg-plist b/FreeBSD/databases/tarantool/pkg-plist
> > > index 9e3905d60..6df72be9d 100644
> > > --- a/FreeBSD/databases/tarantool/pkg-plist
> > > +++ b/FreeBSD/databases/tarantool/pkg-plist
> > > @@ -8,6 +8,7 @@ include/tarantool/lua.hpp
> > > include/tarantool/luaconf.h
> > > include/tarantool/luajit.h
> > > include/tarantool/lualib.h
> > > +include/tarantool/lmisclib.h
> >
> > Minor: Please sort the entries.
>
> I've sorted entries here and inside rpm/tarantool.spec. See itterative
> patch below. Branch is force pushed.
>
> >
> > > include/tarantool/module.h
> > > man/man1/tarantool.1.gz
> > > man/man1/tarantoolctl.1.gz
> >
> > <snipped>
> >
> > > diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
> > > index eedc0312c..3a1a8f2be 100644
> > > --- a/rpm/tarantool.spec
> > > +++ b/rpm/tarantool.spec
> > > @@ -256,6 +256,7 @@ fi
> > > %{_includedir}/tarantool/luajit.h
> > > %{_includedir}/tarantool/lualib.h
> > > %{_includedir}/tarantool/module.h
> > > +%{_includedir}/tarantool/lmisclib.h
> > >
> > > %changelog
> > > * Tue Sep 12 2017 Roman Tsisyk <roman@tarantool.org> 1.7.5.46-1
> >
> > Side note: It looks we totally ignore updating %changelog section.
> >
> > > --
> > > 2.28.0
> > >
> >
> > --
> > Best regards,
> > IM
>
> ===================================================================
> diff --git a/FreeBSD/databases/tarantool/pkg-plist b/FreeBSD/databases/tarantool/pkg-plist
> index 6df72be9d..8e2f3afde 100644
> --- a/FreeBSD/databases/tarantool/pkg-plist
> +++ b/FreeBSD/databases/tarantool/pkg-plist
> @@ -3,12 +3,12 @@ bin/tarantoolctl
> %%ETCDIR%%/default/tarantool
> %%ETCDIR%%/instances.available/example.lua
> include/tarantool/lauxlib.h
> +include/tarantool/lmisclib.h
> include/tarantool/lua.h
> include/tarantool/lua.hpp
> include/tarantool/luaconf.h
> include/tarantool/luajit.h
> include/tarantool/lualib.h
> -include/tarantool/lmisclib.h
> include/tarantool/module.h
> man/man1/tarantool.1.gz
> man/man1/tarantoolctl.1.gz
> diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
> index 3a1a8f2be..9ac7a1a04 100644
> --- a/rpm/tarantool.spec
> +++ b/rpm/tarantool.spec
> @@ -250,13 +250,13 @@ fi
> %files devel
> %dir %{_includedir}/tarantool
> %{_includedir}/tarantool/lauxlib.h
> +%{_includedir}/tarantool/lmisclib.h
> %{_includedir}/tarantool/luaconf.h
> %{_includedir}/tarantool/lua.h
> %{_includedir}/tarantool/lua.hpp
> %{_includedir}/tarantool/luajit.h
> %{_includedir}/tarantool/lualib.h
> %{_includedir}/tarantool/module.h
> -%{_includedir}/tarantool/lmisclib.h
>
> %changelog
> * Tue Sep 12 2017 Roman Tsisyk <roman@tarantool.org> 1.7.5.46-1
> ===================================================================
>
> --
> Best regards,
> Sergey Kaplun
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH 2/2] lua: add missing LuaJIT export symbols
2020-10-19 4:57 ` Sergey Kaplun
@ 2020-10-20 12:09 ` Sergey Ostanevich
2020-10-20 13:03 ` Sergey Kaplun
0 siblings, 1 reply; 12+ messages in thread
From: Sergey Ostanevich @ 2020-10-20 12:09 UTC (permalink / raw)
To: Sergey Kaplun; +Cc: Alexander Turenko, tarantool-patches
Hi!
I reviewed the latest branch, LGTM with some updates in message below.
On 19 окт 07:57, Sergey Kaplun wrote:
> Igor,
>
> Thanks for the review! I've updated commit message considering your
> comment here. Branch is force pushed.
>
> On 18.10.20, Igor Munkin wrote:
> > Sergey,
> >
> > Thanks for the patch! LGTM, except a single nit.
> >
> > On 17.10.20, Sergey Kaplun wrote:
> > > Since LuaJIT provides public C API symbols that are used in the final
> > > executable, the linker may not just throw it away.
> > > Nevertheless for future compatibility all symbols from LuaJIT public API
> > > should be additionaly added at exports.h file.
^^^^^^^^^^^^^^^^^^^^
Perhaps simply
added to
> > >
> > > Follows up tarantool/tarantool#5187
> >
> > Minor: You don't need to use full repo reference here.
> >
> > > ---
> > > src/exports.h | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> >
> > <snipped>
> >
> > > --
> > > 2.28.0
> > >
> >
> > --
> > Best regards,
> > IM
>
> --
> Best regards,
> Sergey Kaplun
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH 2/2] lua: add missing LuaJIT export symbols
2020-10-20 12:09 ` Sergey Ostanevich
@ 2020-10-20 13:03 ` Sergey Kaplun
0 siblings, 0 replies; 12+ messages in thread
From: Sergey Kaplun @ 2020-10-20 13:03 UTC (permalink / raw)
To: Sergey Ostanevich; +Cc: Alexander Turenko, tarantool-patches
Hi, Sergos!
Thanks for the review!
On 20.10.20, Sergey Ostanevich wrote:
> Hi!
>
> I reviewed the latest branch, LGTM with some updates in message below.
>
> On 19 окт 07:57, Sergey Kaplun wrote:
> > Igor,
> >
> > Thanks for the review! I've updated commit message considering your
> > comment here. Branch is force pushed.
> >
> > On 18.10.20, Igor Munkin wrote:
> > > Sergey,
> > >
> > > Thanks for the patch! LGTM, except a single nit.
> > >
> > > On 17.10.20, Sergey Kaplun wrote:
> > > > Since LuaJIT provides public C API symbols that are used in the final
> > > > executable, the linker may not just throw it away.
> > > > Nevertheless for future compatibility all symbols from LuaJIT public API
> > > > should be additionaly added at exports.h file.
> ^^^^^^^^^^^^^^^^^^^^
> Perhaps simply
> added to
Yep, sounds better. Updated and force pushed to the branch.
>
> > > >
> > > > Follows up tarantool/tarantool#5187
> > >
> > > Minor: You don't need to use full repo reference here.
> > >
> > > > ---
> > > > src/exports.h | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > >
> > > <snipped>
> > >
> > > > --
> > > > 2.28.0
> > > >
> > >
> > > --
> > > Best regards,
> > > IM
> >
> > --
> > Best regards,
> > Sergey Kaplun
--
Best regards,
Sergey Kaplun
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH 0/2] Minor fixes for new LuaJIT extention library
2020-10-17 11:13 [Tarantool-patches] [PATCH 0/2] Minor fixes for new LuaJIT extention library Sergey Kaplun
2020-10-17 11:13 ` [Tarantool-patches] [PATCH 1/2] build: provide missing LuaJIT lmisclib.h header Sergey Kaplun
2020-10-17 11:13 ` [Tarantool-patches] [PATCH 2/2] lua: add missing LuaJIT export symbols Sergey Kaplun
@ 2020-10-21 16:08 ` Alexander Turenko
2020-10-21 16:17 ` Alexander V. Tikhonov
3 siblings, 0 replies; 12+ messages in thread
From: Alexander Turenko @ 2020-10-21 16:08 UTC (permalink / raw)
To: Sergey Kaplun; +Cc: tarantool-patches
LGTM from my side.
I don't see any problems in the patchset.
External Gentoo ebuild, Arch PKGBUILD, FreeBSD port do not list headers
to install explicitly (just like in-repo Debian rules).
https://github.com/tarantool/gentoo-overlay/blob/master/dev-db/tarantool/tarantool-9999.ebuild
https://github.com/archlinux/svntogit-community/blob/master/tarantool/trunk/PKGBUILD
https://svnweb.freebsd.org/ports/head/databases/tarantool/Makefile?view=markup
WBR, Alexander Turenko.
On Sat, Oct 17, 2020 at 02:13:29PM +0300, Sergey Kaplun wrote:
> This patchset consists from 2 parts:
> 1) Provide corresponding new LuaJIT header within tarantool at install.
> 2) Add missing public C API symbols from the new lib into exports.h
>
> Branch: https://github.com/tarantool/tarantool/tree/skaplun/gh-5187-luajit-include-headers
>
> CI is red, but fails related to replication tests or at osx_* jobs
> (see also https://github.com/tarantool/tarantool/issues/5423).
> CI: https://gitlab.com/tarantool/tarantool/-/pipelines/203892776
>
> Sergey Kaplun (2):
> build: provide missing LuaJIT lmisclib.h header
> lua: add missing LuaJIT export symbols
>
> FreeBSD/databases/tarantool/pkg-plist | 1 +
> cmake/luajit.cmake | 2 +-
> rpm/tarantool.spec | 1 +
> src/exports.h | 2 ++
> 4 files changed, 5 insertions(+), 1 deletion(-)
>
> --
> 2.28.0
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH 0/2] Minor fixes for new LuaJIT extention library
2020-10-17 11:13 [Tarantool-patches] [PATCH 0/2] Minor fixes for new LuaJIT extention library Sergey Kaplun
` (2 preceding siblings ...)
2020-10-21 16:08 ` [Tarantool-patches] [PATCH 0/2] Minor fixes for new LuaJIT extention library Alexander Turenko
@ 2020-10-21 16:17 ` Alexander V. Tikhonov
3 siblings, 0 replies; 12+ messages in thread
From: Alexander V. Tikhonov @ 2020-10-21 16:17 UTC (permalink / raw)
To: Sergey Kaplun; +Cc: tarantool-patches
Hi Sergey, Ive checked tests results of your patchset, and no new
degradations found [1]. Patchset LGTM.
[1] - https://gitlab.com/tarantool/tarantool/-/pipelines/205179305
On Sat, Oct 17, 2020 at 02:13:29PM +0300, Sergey Kaplun wrote:
> This patchset consists from 2 parts:
> 1) Provide corresponding new LuaJIT header within tarantool at install.
> 2) Add missing public C API symbols from the new lib into exports.h
>
> Branch: https://github.com/tarantool/tarantool/tree/skaplun/gh-5187-luajit-include-headers
>
> CI is red, but fails related to replication tests or at osx_* jobs
> (see also https://github.com/tarantool/tarantool/issues/5423).
> CI: https://gitlab.com/tarantool/tarantool/-/pipelines/203892776
>
> Sergey Kaplun (2):
> build: provide missing LuaJIT lmisclib.h header
> lua: add missing LuaJIT export symbols
>
> FreeBSD/databases/tarantool/pkg-plist | 1 +
> cmake/luajit.cmake | 2 +-
> rpm/tarantool.spec | 1 +
> src/exports.h | 2 ++
> 4 files changed, 5 insertions(+), 1 deletion(-)
>
> --
> 2.28.0
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-10-21 16:17 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-17 11:13 [Tarantool-patches] [PATCH 0/2] Minor fixes for new LuaJIT extention library Sergey Kaplun
2020-10-17 11:13 ` [Tarantool-patches] [PATCH 1/2] build: provide missing LuaJIT lmisclib.h header Sergey Kaplun
2020-10-18 19:19 ` Igor Munkin
2020-10-19 4:55 ` Sergey Kaplun
2020-10-20 12:07 ` Sergey Ostanevich
2020-10-17 11:13 ` [Tarantool-patches] [PATCH 2/2] lua: add missing LuaJIT export symbols Sergey Kaplun
2020-10-18 18:43 ` Igor Munkin
2020-10-19 4:57 ` Sergey Kaplun
2020-10-20 12:09 ` Sergey Ostanevich
2020-10-20 13:03 ` Sergey Kaplun
2020-10-21 16:08 ` [Tarantool-patches] [PATCH 0/2] Minor fixes for new LuaJIT extention library Alexander Turenko
2020-10-21 16:17 ` Alexander V. Tikhonov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox