[Tarantool-patches] [PATCH 6/7] box/console: switch to new lua serializer
Oleg Babin
olegrok at tarantool.org
Mon May 18 15:21:00 MSK 2020
Hi! Thanks for your patch. LGTM.
Please file an issue to remove serpent in the next version.
On 12/05/2020 16:50, Cyrill Gorcunov wrote:
> I do not drop the serpent module for a while
> since I need to make sure that everything work
> as expected as we've not break backward compatibility
> significantly (though we didn't claim the lua mode
> output is stable enough).
>
> Fixes #4682
>
> Signed-off-by: Cyrill Gorcunov <gorcunov at gmail.com>
> ---
> src/box/lua/console.c | 77 +++++++++++++++++++++++++++++++
> src/box/lua/console.lua | 76 +++++++++++++-----------------
> test/app-tap/console_lua.test.lua | 18 ++++----
> 3 files changed, 119 insertions(+), 52 deletions(-)
>
> diff --git a/src/box/lua/console.c b/src/box/lua/console.c
> index c8e0dea58..d1bf7949c 100644
> --- a/src/box/lua/console.c
> +++ b/src/box/lua/console.c
> @@ -39,6 +39,7 @@
> #include "coio.h"
> #include "lua/msgpack.h"
> #include "lua-yaml/lyaml.h"
> +#include "serialize_lua.h"
> #include <lua.h>
> #include <lauxlib.h>
> #include <lualib.h>
> @@ -50,6 +51,7 @@
> extern char serpent_lua[];
>
> static struct luaL_serializer *serializer_yaml;
> +static struct luaL_serializer *serializer_lua;
>
> /*
> * Completion engine (Mike Paul's).
> @@ -68,6 +70,54 @@ lua_rl_complete(lua_State *L, const char *text, int start, int end);
> */
> static struct lua_State *readline_L;
>
> +/**
> + * Encode Lua object into Lua form.
> + */
> +static int
> +lbox_console_format_lua(struct lua_State *L)
> +{
> + lua_dumper_opts_t opts;
> + int arg_count;
> +
> + /* Parse options and remove them */
> + lua_parse_opts(L, &opts);
> + lua_remove(L, 1);
> +
> + arg_count = lua_gettop(L);
> +
> + /* If nothing to process just exit early */
> + if (arg_count == 0)
> + return 0;
> +
> + /*
> + * When processing arguments we might
> + * need to modify reference (for example
> + * when __index references to object itself)
> + * thus make a copy of incoming data.
> + *
> + * Note that in yaml there is no map for
> + * Lua's "nil" value so for yaml encoder
> + * we do
> + *
> + * if (lua_isnil(L, i + 1))
> + * luaL_pushnull(L);
> + * else
> + * lua_pushvalue(L, i + 1);
> + *
> + *
> + * For lua mode we have to preserve "nil".
> + */
> + lua_createtable(L, arg_count, 0);
> + for (int i = 0; i < arg_count; ++i) {
> + lua_pushvalue(L, i + 1);
> + lua_rawseti(L, -2, i + 1);
> + }
> +
> + lua_replace(L, 1);
> + lua_settop(L, 1);
> + return lua_encode(L, serializer_lua, &opts);
> +}
> +
> /*
> * console_completion_handler()
> * Called by readline to collect plausible completions;
> @@ -557,6 +607,7 @@ tarantool_lua_console_init(struct lua_State *L)
> {"add_history", lbox_console_add_history},
> {"completion_handler", lbox_console_completion_handler},
> {"format_yaml", lbox_console_format_yaml},
> + {"format_lua", lbox_console_format_lua},
> {NULL, NULL}
> };
> luaL_register_module(L, "console", consolelib);
> @@ -581,6 +632,32 @@ tarantool_lua_console_init(struct lua_State *L)
> * load_history work the same way.
> */
> lua_setfield(L, -2, "formatter");
> +
> + /*
> + * We don't export it as a module
> + * for a while, so the library
> + * is kept empty.
> + */
> + static const luaL_Reg lualib[] = {
> + { },
> + };
> +
> + serializer_lua = luaL_newserializer(L, NULL, lualib);
> + serializer_lua->has_compact = 1;
> + serializer_lua->encode_invalid_numbers = 1;
> + serializer_lua->encode_load_metatables = 1;
> + serializer_lua->encode_use_tostring = 1;
> + serializer_lua->encode_invalid_as_nil = 1;
> +
> + /*
> + * Keep a reference to this module so it
> + * won't be unloaded.
> + */
> + lua_setfield(L, -2, "formatter_lua");
> +
> + /* Output formatter in Lua mode */
> + lua_serializer_init(L);
> +
> struct session_vtab console_session_vtab = {
> .push = console_session_push,
> .fd = console_session_fd,
> diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua
> index 5642ca956..6ea27a393 100644
> --- a/src/box/lua/console.lua
> +++ b/src/box/lua/console.lua
> @@ -69,60 +69,50 @@ output_handlers["yaml"] = function(status, opts, ...)
> return internal.format_yaml({ error = err })
> end
>
> --- A map for internal symbols in case if they
> --- are not inside tables and serpent won't be
> --- able to handle them properly.
> -local lua_map_direct_symbols = {
> - [box.NULL] = 'box.NULL',
> -}
> -
> --- A map for internal symbols in case if they
> --- are coming from tables and we need to depict
> --- them into user known values.
> -local lua_map_table_symbols = {
> - ['"cdata<void %*>: NULL"'] = 'box.NULL'
> -}
> -
> --
> --- Map internal symbols which serpent doesn't
> --- know about to a known representation.
> -local serpent_map_symbols = function(tag, head, body, tail, level)
> - for k,v in pairs(lua_map_table_symbols) do
> - body = body:gsub(k, v)
> +-- Format a Lua value.
> +local function format_lua_value(status, internal_opts, value)
> + local err
> + if status then
> + status, err = pcall(internal.format_lua, internal_opts, value)
> + if status then
> + return err
> + else
> + local m = 'console: exception while formatting output: "%s"'
> + err = m:format(tostring(err))
> + end
> + else
> + err = value
> + if err == nil then
> + err = box.NULL
> + end
> end
> - return tag..head..body..tail
> + return internal.format_lua(internal_opts, { error = err })
> end
>
> --
> --- Format a Lua value.
> -local function format_lua_value(value, opts)
> - for k,v in pairs(lua_map_direct_symbols) do
> - if k == value then
> - return v
> - end
> - end
> - local serpent_opts = {
> - custom = serpent_map_symbols,
> - comment = false,
> - nocode = true,
> - }
> +-- Convert options from user form to the internal format.
> +local function gen_lua_opts(opts)
> if opts == "block" then
> - return serpent.block(value, serpent_opts)
> + return {block=true, indent=2}
> + else
> + return {block=false, indent=2}
> end
> - return serpent.line(value, serpent_opts)
> end
>
> output_handlers["lua"] = function(status, opts, ...)
> local collect = {}
> --
> - -- If no data present at least EOS should be put,
> - -- otherwise wire readers won't be able to find
> - -- where the end of string is.
> - if not ... then
> - return output_eos["lua"]
> - end
> + -- Convert options to internal dictionary.
> + local internal_opts = gen_lua_opts(opts)
> for i = 1, select('#', ...) do
> - collect[i] = format_lua_value(select(i, ...), opts)
> + collect[i] = format_lua_value(status, internal_opts, select(i, ...))
> + if collect[i] == nil then
> + --
> + -- table.concat doesn't work for nil
> + -- so convert it explicitly.
> + collect[i] = "nil"
> + end
> end
> return table.concat(collect, ', ') .. output_eos["lua"]
> end
> @@ -210,8 +200,8 @@ end
>
> -- Used by console_session_push.
> box_internal.format_lua_push = function(value)
> - local opts = current_output()["opts"]
> - value = format_lua_value(value, opts)
> + local internal_opts = gen_lua_opts(current_output()["opts"])
> + value = format_lua_value(true, internal_opts, value)
> return '-- Push\n' .. value .. ';'
> end
>
> diff --git a/test/app-tap/console_lua.test.lua b/test/app-tap/console_lua.test.lua
> index 3ed6aad97..8bb6eb0b3 100755
> --- a/test/app-tap/console_lua.test.lua
> +++ b/test/app-tap/console_lua.test.lua
> @@ -102,29 +102,29 @@ local cases = {
> prepare = 'a = {1, 2, 3}',
> opts = {block = false},
> input = '1, 2, nil, a',
> - expected = '1, 2, box.NULL, {1, 2, 3}',
> + expected = '1, 2, nil, {1, 2, 3}',
> cleanup = 'a = nil',
> }, {
> name = 'multireturn block mode',
> prepare = 'a = {1, 2, 3}',
> opts = {block = true},
> input = '1, 2, nil, a',
> - expected = '1, 2, box.NULL, {\n 1,\n 2,\n 3\n}',
> + expected = '1, 2, nil, {\n 1,\n 2,\n 3\n}',
> cleanup = 'a = nil',
> }, {
> - name = 'trailing nils, line mode',
> + name = 'trailing nils, box.NULL, line mode',
> opts = {block = false},
> - input = '1, nil, nil, nil',
> - expected = '1, box.NULL, box.NULL, box.NULL',
> + input = '1, nil, box.NULL, nil',
> + expected = '1, nil, box.NULL, nil',
> }, {
> - name = 'trailing nils, block mode',
> + name = 'trailing nils, box.NULL, block mode',
> opts = {block = true},
> - input = '1, nil, nil, nil',
> - expected = '1, box.NULL, box.NULL, box.NULL',
> + input = '1, nil, box.NULL, nil',
> + expected = '1, nil, box.NULL, nil',
> }, {
> name = 'empty output',
> input = '\\set output',
> - expected = '"Specify output format: lua or yaml."',
> + expected = '{error = \"Specify output format: lua or yaml.\"}',
> }
> }
>
>
More information about the Tarantool-patches
mailing list