[Tarantool-patches] [PATCH vshard 2/4] util: introduce Tarantool's semver parser
Oleg Babin
olegrok at tarantool.org
Fri Feb 11 19:38:46 MSK 2022
Thanks for your answers and changes. LGTM.
On 11.02.2022 01:33, Vladislav Shpilevoy wrote:
> Thanks for the review!
>
> On 09.02.2022 18:53, Oleg Babin via Tarantool-patches wrote:
>> Thanks for your patch.
>>
>>
>> Looks good. Probably it's worth to move this code into separate module.
>>
>> There are several modules in Tarantool ecosystem that implement its own parsers.
> I don't mind if somebody would move it. And heap.lua too. But I don't really
> want to do that myself since it requires too much work not related to
> programming. Just the necessity to install CI in a new repository is enough
> for me to give that up.
>
>>> +
>>> +local function version_new(id_major, id_middle, id_minor, rel_type, rel_num,
>>> + id_commit)
>> I think we could validate that at least id_major is not nil.
> Yes, we can.
>
> ====================
> @@ -76,7 +76,8 @@ local version_mt = {
>
> local function version_new(id_major, id_middle, id_minor, rel_type, rel_num,
> id_commit)
> - -- There is no any validation - the API is not public.
> + -- There is no any proper validation - the API is not public.
> + assert(id_major and id_middle and id_minor)
> return setmetatable({
> id_major = id_major,
> id_middle = id_middle,
> ====================
>
>
> New patch:
>
> ====================
> util: introduce Tarantool's semver parser
>
> Tarantool's version since recently has semver-like format. It
> made impossible to check for features existence if they were
> introduced not in the first release of one x.x.x triplet.
>
> An alternative would be to test for feature existence by its
> usage, but it is not always possible with ease.
>
> This patch improves version parser in vshard to understand new
> version names.
>
> It will be used by a future commit about netbox's return_raw
> feature adoption to skip its tests for old versions
> (<= 2.10.0-beta2).
>
> Needed for #312
> ---
> test/unit-luatest/suite.ini | 5 +
> test/unit-luatest/version_test.lua | 179 +++++++++++++++++++++++++++++
> vshard/CMakeLists.txt | 2 +-
> vshard/router/init.lua | 2 +-
> vshard/storage/init.lua | 2 +-
> vshard/util.lua | 17 +--
> vshard/version.lua | 149 ++++++++++++++++++++++++
> 7 files changed, 340 insertions(+), 16 deletions(-)
> create mode 100644 test/unit-luatest/suite.ini
> create mode 100644 test/unit-luatest/version_test.lua
> create mode 100644 vshard/version.lua
>
> diff --git a/test/unit-luatest/suite.ini b/test/unit-luatest/suite.ini
> new file mode 100644
> index 0000000..303032c
> --- /dev/null
> +++ b/test/unit-luatest/suite.ini
> @@ -0,0 +1,5 @@
> +[default]
> +core = luatest
> +description = Unit tests
> +is_parallel = True
> +release_disabled =
> diff --git a/test/unit-luatest/version_test.lua b/test/unit-luatest/version_test.lua
> new file mode 100644
> index 0000000..905d36e
> --- /dev/null
> +++ b/test/unit-luatest/version_test.lua
> @@ -0,0 +1,179 @@
> +local t = require('luatest')
> +local lversion = require('vshard.version')
> +
> +local g = t.group('version')
> +
> +g.test_order = function(g)
> + -- Example of a full version: 2.10.0-beta2-86-gc9981a567.
> + local versions = {
> + {
> + str = '1.2.3-alpha',
> + ver = lversion.new(1, 2, 3, 'alpha', 0, 0),
> + },
> + {
> + str = '1.2.3-alpha-30',
> + ver = lversion.new(1, 2, 3, 'alpha', 0, 30),
> + },
> + {
> + str = '1.2.3-alpha-45',
> + ver = lversion.new(1, 2, 3, 'alpha', 0, 45),
> + },
> + {
> + str = '1.2.3-alpha1',
> + ver = lversion.new(1, 2, 3, 'alpha', 1, 0),
> + },
> + {
> + str = '1.2.3-alpha1-45',
> + ver = lversion.new(1, 2, 3, 'alpha', 1, 45),
> + },
> + {
> + str = '1.2.3-alpha2',
> + ver = lversion.new(1, 2, 3, 'alpha', 2, 0),
> + },
> + {
> + str = '1.2.3-alpha2-45',
> + ver = lversion.new(1, 2, 3, 'alpha', 2, 45),
> + },
> + {
> + str = '1.2.3-beta',
> + ver = lversion.new(1, 2, 3, 'beta', 0, 0),
> + },
> + {
> + str = '1.2.3-beta-45',
> + ver = lversion.new(1, 2, 3, 'beta', 0, 45),
> + },
> + {
> + str = '1.2.3-beta1',
> + ver = lversion.new(1, 2, 3, 'beta', 1, 0),
> + },
> + {
> + str = '1.2.3-beta1-45',
> + ver = lversion.new(1, 2, 3, 'beta', 1, 45),
> + },
> + {
> + str = '1.2.3-beta2',
> + ver = lversion.new(1, 2, 3, 'beta', 2, 0),
> + },
> + {
> + str = '1.2.3-beta2-45',
> + ver = lversion.new(1, 2, 3, 'beta', 2, 45),
> + },
> + {
> + str = '1.2.3-rc',
> + ver = lversion.new(1, 2, 3, 'rc', 0, 0),
> + },
> + {
> + str = '1.2.3-rc-45',
> + ver = lversion.new(1, 2, 3, 'rc', 0, 45),
> + },
> + {
> + str = '1.2.3-rc1',
> + ver = lversion.new(1, 2, 3, 'rc', 1, 0),
> + },
> + {
> + str = '1.2.3-rc1-45',
> + ver = lversion.new(1, 2, 3, 'rc', 1, 45),
> + },
> + {
> + str = '1.2.3-rc2',
> + ver = lversion.new(1, 2, 3, 'rc', 2, 0),
> + },
> + {
> + str = '1.2.3-rc2-45',
> + ver = lversion.new(1, 2, 3, 'rc', 2, 45),
> + },
> + {
> + str = '1.2.3-rc3',
> + ver = lversion.new(1, 2, 3, 'rc', 3, 0),
> + },
> + {
> + str = '1.2.3-rc4',
> + ver = lversion.new(1, 2, 3, 'rc', 4, 0),
> + },
> + {
> + str = '1.2.3',
> + ver = lversion.new(1, 2, 3, nil, 0, 0),
> + },
> + {
> + str = '1.2.4',
> + ver = lversion.new(1, 2, 4, nil, 0, 0),
> + },
> + {
> + str = '1.2.5-alpha',
> + ver = lversion.new(1, 2, 5, 'alpha', 0, 0),
> + },
> + {
> + str = '1.2.5-alpha1-45-gc9981a567',
> + ver = lversion.new(1, 2, 5, 'alpha', 1, 45),
> + },
> + {
> + str = '1.2.6-',
> + ver = lversion.new(1, 2, 6, nil, 0, 0),
> + },
> + {
> + str = '1.2.7-alpha-',
> + ver = lversion.new(1, 2, 7, 'alpha', 0, 0),
> + },
> + {
> + str = '1.2.7-alpha1-',
> + ver = lversion.new(1, 2, 7, 'alpha', 1, 0),
> + },
> + {
> + str = '1.2.7-alpha1-45',
> + ver = lversion.new(1, 2, 7, 'alpha', 1, 45),
> + },
> + {
> + str = '1.2.7-alpha1-46-',
> + ver = lversion.new(1, 2, 7, 'alpha', 1, 46),
> + },
> + {
> + str = '1.2.8-alpha',
> + ver = lversion.new(1, 2, 8, 'alpha', 0, 0),
> + },
> + {
> + str = '1.2.8-beta',
> + ver = lversion.new(1, 2, 8, 'beta', 0, 0),
> + },
> + {
> + str = '1.2.8-rc',
> + ver = lversion.new(1, 2, 8, 'rc', 0, 0),
> + },
> + {
> + str = '1.2.9',
> + ver = lversion.new(1, 2, 9, nil, 0, 0),
> + },
> + }
> + for i, v in pairs(versions) do
> + local ver = lversion.parse(v.str)
> + t.assert(ver == v.ver, ('versions ==, %d'):format(i))
> + t.assert(not (ver ~= v.ver), ('versions not ~=, %d'):format(i))
> + t.assert(not (ver < v.ver), ('versions not <, %d'):format(i))
> + t.assert(not (ver > v.ver), ('versions not >, %d'):format(i))
> + t.assert(ver <= v.ver, ('versions <=, %d'):format(i))
> + t.assert(ver >= v.ver, ('versions <=, %d'):format(i))
> + if i > 1 then
> + local prev = versions[i - 1].ver
> + t.assert(prev < ver, ('versions <, %d'):format(i))
> + t.assert(prev <= ver, ('versions <=, %d'):format(i))
> + t.assert(not (prev > ver), ('versions not >, %d'):format(i))
> + t.assert(not (prev >= ver), ('versions not >=, %d'):format(i))
> +
> + t.assert(not (ver < prev), ('versions not <, %d'):format(i))
> + t.assert(not (ver <= prev), ('versions not <=, %d'):format(i))
> + t.assert(ver > prev, ('versions >, %d'):format(i))
> + t.assert(ver >= prev, ('versions >=, %d'):format(i))
> +
> + t.assert(ver ~= prev, ('versions ~=, %d'):format(i))
> + t.assert(not (ver == prev), ('versions not ==, %d'):format(i))
> + end
> + end
> +end
> +
> +g.test_error = function(g)
> + t.assert_error_msg_contains('Could not parse version', lversion.parse,
> + 'bad version')
> + t.assert_error_msg_contains('Could not parse version', lversion.parse,
> + '1.x.x')
> + t.assert_error_msg_contains('Could not parse version', lversion.parse,
> + '1.2.x')
> +end
> diff --git a/vshard/CMakeLists.txt b/vshard/CMakeLists.txt
> index 2d9be25..29500f6 100644
> --- a/vshard/CMakeLists.txt
> +++ b/vshard/CMakeLists.txt
> @@ -7,5 +7,5 @@ add_subdirectory(router)
>
> # Install module
> install(FILES cfg.lua error.lua consts.lua hash.lua init.lua replicaset.lua
> - util.lua rlist.lua heap.lua registry.lua
> + util.lua rlist.lua heap.lua registry.lua version.lua
> DESTINATION ${TARANTOOL_INSTALL_LUADIR}/vshard)
> diff --git a/vshard/router/init.lua b/vshard/router/init.lua
> index 064bd5a..44ed801 100644
> --- a/vshard/router/init.lua
> +++ b/vshard/router/init.lua
> @@ -7,7 +7,7 @@ local MODULE_INTERNALS = '__module_vshard_router'
> -- Reload requirements, in case this module is reloaded manually.
> if rawget(_G, MODULE_INTERNALS) then
> local vshard_modules = {
> - 'vshard.consts', 'vshard.error', 'vshard.cfg',
> + 'vshard.consts', 'vshard.error', 'vshard.cfg', 'vshard.version',
> 'vshard.hash', 'vshard.replicaset', 'vshard.util'
> }
> for _, module in pairs(vshard_modules) do
> diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua
> index 1642609..6820ad0 100644
> --- a/vshard/storage/init.lua
> +++ b/vshard/storage/init.lua
> @@ -13,7 +13,7 @@ local MODULE_INTERNALS = '__module_vshard_storage'
> -- Reload requirements, in case this module is reloaded manually.
> if rawget(_G, MODULE_INTERNALS) then
> local vshard_modules = {
> - 'vshard.consts', 'vshard.error', 'vshard.cfg',
> + 'vshard.consts', 'vshard.error', 'vshard.cfg', 'vshard.version',
> 'vshard.replicaset', 'vshard.util',
> 'vshard.storage.reload_evolution', 'vshard.rlist', 'vshard.registry',
> 'vshard.heap', 'vshard.storage.ref', 'vshard.storage.sched',
> diff --git a/vshard/util.lua b/vshard/util.lua
> index 366fdea..9e0212a 100644
> --- a/vshard/util.lua
> +++ b/vshard/util.lua
> @@ -2,6 +2,7 @@
> local log = require('log')
> local fiber = require('fiber')
> local lerror = require('vshard.error')
> +local lversion = require('vshard.version')
>
> local MODULE_INTERNALS = '__module_vshard_util'
> local M = rawget(_G, MODULE_INTERNALS)
> @@ -16,13 +17,7 @@ if not M then
> rawset(_G, MODULE_INTERNALS, M)
> end
>
> -local version_str = _TARANTOOL:sub(1, _TARANTOOL:find('-')-1)
> -local dot = version_str:find('%.')
> -local major = tonumber(version_str:sub(1, dot - 1))
> -version_str = version_str:sub(dot + 1)
> -dot = version_str:find('%.')
> -local middle = tonumber(version_str:sub(1, dot - 1))
> -local minor = tonumber(version_str:sub(dot + 1))
> +local tnt_version = lversion.parse(_TARANTOOL)
>
> --
> -- Extract parts of a tuple.
> @@ -146,12 +141,8 @@ end
> --
> -- Check if Tarantool version is >= that a specified one.
> --
> -local function version_is_at_least(major_need, middle_need, minor_need)
> - if major > major_need then return true end
> - if major < major_need then return false end
> - if middle > middle_need then return true end
> - if middle < middle_need then return false end
> - return minor >= minor_need
> +local function version_is_at_least(...)
> + return tnt_version >= lversion.new(...)
> end
>
> if not version_is_at_least(1, 10, 1) then
> diff --git a/vshard/version.lua b/vshard/version.lua
> new file mode 100644
> index 0000000..a53eb60
> --- /dev/null
> +++ b/vshard/version.lua
> @@ -0,0 +1,149 @@
> +--
> +-- Semver parser adopted to Tarantool's versions.
> +-- Almost everything is the same as inhttps://semver.org.
> +--
> +-- Tarantool's version has format:
> +--
> +-- x.x.x-typen-commit-ghash
> +--
> +-- * x.x.x - major, middle, minor release numbers;
> +-- * typen - release type and its optional number: alpha1, beta5, rc10.
> +-- Optional;
> +-- * commit - commit count since the latest release. Optional;
> +-- * ghash - latest commit hash in format g<hash>. Optional.
> +--
> +-- Differences with the semver docs:
> +--
> +-- * No support for nested releases like x.x.x-alpha.beta. Only x.x.x-alpha.
> +-- * Release number is written right after its type. Not 'alpha.1' but 'alpha1'.
> +--
> +
> +local release_type_weight = {
> + alpha = 0,
> + beta = 1,
> + rc = 2,
> +}
> +
> +local function release_type_cmp(t1, t2)
> + t1 = release_type_weight[t1]
> + t2 = release_type_weight[t2]
> + -- 'No release type' means the greatest.
> + if not t1 then
> + if not t2 then
> + return 0
> + end
> + return 1
> + end
> + if not t2 then
> + return -1
> + end
> + return t1 - t2
> +end
> +
> +local function version_cmp(ver1, ver2)
> + if ver1.id_major ~= ver2.id_major then
> + return ver1.id_major - ver2.id_major
> + end
> + if ver1.id_middle ~= ver2.id_middle then
> + return ver1.id_middle - ver2.id_middle
> + end
> + if ver1.id_minor ~= ver2.id_minor then
> + return ver1.id_minor - ver2.id_minor
> + end
> + if ver1.rel_type ~= ver2.rel_type then
> + return release_type_cmp(ver1.rel_type, ver2.rel_type)
> + end
> + if ver1.rel_num ~= ver2.rel_num then
> + return ver1.rel_num - ver2.rel_num
> + end
> + if ver1.id_commit ~= ver2.id_commit then
> + return ver1.id_commit - ver2.id_commit
> + end
> + return 0
> +end
> +
> +local version_mt = {
> + __eq = function(l, r)
> + return version_cmp(l, r) == 0
> + end,
> + __lt = function(l, r)
> + return version_cmp(l, r) < 0
> + end,
> + __le = function(l, r)
> + return version_cmp(l, r) <= 0
> + end,
> +}
> +
> +local function version_new(id_major, id_middle, id_minor, rel_type, rel_num,
> + id_commit)
> + -- There is no any proper validation - the API is not public.
> + assert(id_major and id_middle and id_minor)
> + return setmetatable({
> + id_major = id_major,
> + id_middle = id_middle,
> + id_minor = id_minor,
> + rel_type = rel_type,
> + rel_num = rel_num,
> + id_commit = id_commit,
> + }, version_mt)
> +end
> +
> +local function version_parse(version_str)
> + -- x.x.x-name<num>-<num>-g<commit>
> + -- \____/\___/\___/\_____/
> + -- P1 P2 P3 P4
> + local id_major, id_middle, id_minor
> + local rel_type
> + local rel_num = 0
> + local id_commit = 0
> + local pos
> +
> + -- Part 1 - version ID triplet.
> + id_major, id_middle, id_minor = version_str:match('^(%d+)%.(%d+)%.(%d+)')
> + if not id_major or not id_middle or not id_minor then
> + error(('Could not parse version: %s'):format(version_str))
> + end
> + id_major = tonumber(id_major)
> + id_middle = tonumber(id_middle)
> + id_minor = tonumber(id_minor)
> +
> + -- Cut to 'name<num>-<num>-g<commit>'.
> + pos = version_str:find('-')
> + if not pos then
> + goto finish
> + end
> + version_str = version_str:sub(pos + 1)
> +
> + -- Part 2 and 3 - release name, might be absent.
> + rel_type, rel_num = version_str:match('^(%a+)(%d+)')
> + if not rel_type then
> + rel_type = version_str:match('^(%a+)')
> + rel_num = 0
> + else
> + rel_num = tonumber(rel_num)
> + end
> +
> + -- Cut to '<num>-g<commit>'.
> + pos = version_str:find('-')
> + if not pos then
> + goto finish
> + end
> + version_str = version_str:sub(pos + 1)
> +
> + -- Part 4 - commit count since latest release, might be absent.
> + id_commit = version_str:match('^(%d+)')
> + if not id_commit then
> + id_commit = 0
> + else
> + id_commit = tonumber(id_commit)
> + end
> +
> +::finish::
> + return version_new(id_major, id_middle, id_minor, rel_type, rel_num,
> + id_commit)
> +end
> +
> +return {
> + parse = version_parse,
> + new = version_new,
> +}
More information about the Tarantool-patches
mailing list