From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id B04A86E21E; Fri, 11 Feb 2022 19:39:15 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org B04A86E21E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1644597555; bh=DZ3scsJxqVGWKq5CnHovZbdQfjN/lvaiBDl470CaLVI=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=AbYnJKRetMmvEcaD2/estdZ/ANqhqnBmvw+lHtFPPs5Zb1QQCulYkJgWcivtg6ORH vEk/Pw1dY2v+JIr/sfFG/b6fDXUMPvC8bCKrGoSG5eggOYgcMQ5H22ppFhUpTD70F7 It6Xufm+sTgRNzdJ4miIBEkK0mi3IDA4B1WdXuig= Received: from smtp3.mail.ru (smtp3.mail.ru [94.100.179.58]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 342416E21E for ; Fri, 11 Feb 2022 19:38:48 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 342416E21E Received: by smtp3.mail.ru with esmtpa (envelope-from ) id 1nIYwZ-0007de-IB; Fri, 11 Feb 2022 19:38:48 +0300 Message-ID: <5cf1afe8-97e2-07ae-8203-24a957fafa78@tarantool.org> Date: Fri, 11 Feb 2022 19:38:46 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.5.1 To: Vladislav Shpilevoy , tarantool-patches@dev.tarantool.org References: <1e64752b75ac76272779b2a3c15c878d65cc20b8.1644366575.git.v.shpilevoy@tarantool.org> <43e7f638-c5da-22d3-2553-113141a786e5@tarantool.org> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD94A599DE53EADEE5705EB66965380F0CDB4C116533E9EB873182A05F5380850403E8DA2971A6F6BEC0A95211D757232F9D1136CF6AAC65718FC23EE4E334E6AC7 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE728F774C865CF4B07EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006378997215BCAA11D778638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8F8E31C5029E5858ED3776DCFA60F94AB117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BAA867293B0326636D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE437C869540D2AB0F1B0CC92B5A49C88ED8FC6C240DEA7642DBF02ECDB25306B2B78CF848AE20165D0A6AB1C7CE11FEE38ED1AC82D843A2BB03F1AB874ED89028C4224003CC836476EA7A3FFF5B025636E2021AF6380DFAD1A18204E546F3947CB11811A4A51E3B096D1867E19FE1407959CC434672EE6371089D37D7C0E48F6C8AA50765F7900637AF8E4F18C523FAA9EFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-8FC586DF: 6EFBBC1D9D64D975 X-C1DE0DAB: 0D63561A33F958A52ABE737107F4BE4203BD5A3A37ACF9649DFFA370F559A085D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7506FE1F977233B9BB410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34728AF701C68E4539BDCDE57F80393B52E48997701752F0FA1B86051FD1FE085478A531652D7F4F501D7E09C32AA3244C496059206F53F9833C87BB6591032A55795D98D676DD64D0729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojtO9NuZTWcnMOXak5xmdlzw== X-Mailru-Sender: 583F1D7ACE8F49BD508BD8DBBD09B14DE953011063C6E35E838A6D0CA698BD62334BF1DBEFB2805C80EE221D05932256AD9BA6614E257C8ED9E51C16F2486AFBE342CF4F05FB7E8CB0DAF586E7D11B3E67EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH vshard 2/4] util: introduce Tarantool's semver parser X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Oleg Babin via Tarantool-patches Reply-To: Oleg Babin Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 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. 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--g > + -- \____/\___/\___/\_____/ > + -- 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--g'. > + 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 '-g'. > + 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, > +}