* [Tarantool-patches] [PATCH v2] build: disable LUAJIT_ENABLE_PAIRSMM @ 2020-03-06 16:28 Igor Munkin 2020-03-06 17:41 ` Igor Munkin 2020-03-17 7:30 ` Kirill Yukhin 0 siblings, 2 replies; 8+ messages in thread From: Igor Munkin @ 2020-03-06 16:28 UTC (permalink / raw) To: Vladislav Shpilevoy, Alexander Turenko, Kirill Yukhin; +Cc: tarantool-patches Since this build flag has been removed as a result of reverting the tarantool/luajit@d4e985a, its definition in the corresponding Tarantool cmake file is irrelevant. Furthermore, considering the breakage faced in #4770 the following tests are introduced: * the check whether space __pairs metamethod is set to space.pairs to create a Lua Fun iterator that handles __pairs manually underneath. * the check whether pairs builtin behaviour doesn't change when __pairs is set e.g. on space object. Follow-up #4560 Closes #4770 Signed-off-by: Igor Munkin <imun@tarantool.org> --- @ChangeLog (need to be added to already released versions): | __pairs/__ipairs metamethods handling is removed since we faced the | issues with the backward compatibility between Lua 5.1 and Lua 5.2 | within Tarantool modules as well as other third party code (gh-4770). Issue: https://github.com/tarantool/tarantool/issues/4770 Branch: https://github.com/tarantool/tarantool/tree/imun/gh-4770-broken-pairs v1: https://lists.tarantool.org/pipermail/tarantool-patches/2020-February/014375.html Changes in v2: * moved test to a separate file cmake/luajit.cmake | 1 - ...4770-broken-pairs-for-space-objects.result | 52 +++++++++++++++++++ ...70-broken-pairs-for-space-objects.test.lua | 18 +++++++ 3 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 test/box/gh-4770-broken-pairs-for-space-objects.result create mode 100644 test/box/gh-4770-broken-pairs-for-space-objects.test.lua diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake index 072db8269..10df633d5 100644 --- a/cmake/luajit.cmake +++ b/cmake/luajit.cmake @@ -217,7 +217,6 @@ macro(luajit_build) add_definitions(-DLUAJIT_USE_ASAN=1) set (luajit_ldflags ${luajit_ldflags} -fsanitize=address) endif() - add_definitions(-DLUAJIT_ENABLE_PAIRSMM=1) add_definitions(-DLUAJIT_SMART_STRINGS=1) # Add all COMPILE_DEFINITIONS to xcflags get_property(defs DIRECTORY PROPERTY COMPILE_DEFINITIONS) diff --git a/test/box/gh-4770-broken-pairs-for-space-objects.result b/test/box/gh-4770-broken-pairs-for-space-objects.result new file mode 100644 index 000000000..61618453e --- /dev/null +++ b/test/box/gh-4770-broken-pairs-for-space-objects.result @@ -0,0 +1,52 @@ +-- test-run result file version 2 +env = require('test_run') + | --- + | ... +test_run = env.new() + | --- + | ... +test_run:cmd("push filter 'table: .*' to 'table: <address>'") + | --- + | - true + | ... + +-- +-- gh-4770: Iteration through space with Lua builtin pairs routine +-- +box.cfg{} + | --- + | ... +s = box.schema.create_space('test') + | --- + | ... +-- Check whether __pairs is set for the space object, since Lua Fun +-- handles it manually underneath. +getmetatable(s).__pairs == s.pairs + | --- + | - true + | ... +-- Check whether pairs builtin behaviour doesn't change when the +-- __pairs is set. +keys = {} + | --- + | ... +for k, v in pairs(s) do keys[k] = true end + | --- + | ... +keys + | --- + | - engine: true + | before_replace: true + | field_count: true + | id: true + | on_replace: true + | temporary: true + | index: true + | is_local: true + | enabled: true + | name: true + | ck_constraint: true + | ... +s:drop() + | --- + | ... diff --git a/test/box/gh-4770-broken-pairs-for-space-objects.test.lua b/test/box/gh-4770-broken-pairs-for-space-objects.test.lua new file mode 100644 index 000000000..15024a803 --- /dev/null +++ b/test/box/gh-4770-broken-pairs-for-space-objects.test.lua @@ -0,0 +1,18 @@ +env = require('test_run') +test_run = env.new() +test_run:cmd("push filter 'table: .*' to 'table: <address>'") + +-- +-- gh-4770: Iteration through space with Lua builtin pairs routine +-- +box.cfg{} +s = box.schema.create_space('test') +-- Check whether __pairs is set for the space object, since Lua Fun +-- handles it manually underneath. +getmetatable(s).__pairs == s.pairs +-- Check whether pairs builtin behaviour doesn't change when the +-- __pairs is set. +keys = {} +for k, v in pairs(s) do keys[k] = true end +keys +s:drop() -- 2.25.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH v2] build: disable LUAJIT_ENABLE_PAIRSMM 2020-03-06 16:28 [Tarantool-patches] [PATCH v2] build: disable LUAJIT_ENABLE_PAIRSMM Igor Munkin @ 2020-03-06 17:41 ` Igor Munkin 2020-03-11 23:19 ` Vladislav Shpilevoy 2020-03-17 7:30 ` Kirill Yukhin 1 sibling, 1 reply; 8+ messages in thread From: Igor Munkin @ 2020-03-06 17:41 UTC (permalink / raw) To: Vladislav Shpilevoy; +Cc: tarantool-patches Vlad, I discussed your comments with Sasha Tu. offline and he totally agreed with both. Fixed, squashed and force-pushed the branch. Diffs are below. On 06.03.20, Igor Munkin wrote: > Since this build flag has been removed as a result of reverting the > tarantool/luajit@d4e985a, its definition in the corresponding Tarantool > cmake file is irrelevant. > > Furthermore, considering the breakage faced in #4770 the following tests > are introduced: > * the check whether space __pairs metamethod is set to space.pairs to > create a Lua Fun iterator that handles __pairs manually underneath. > * the check whether pairs builtin behaviour doesn't change when __pairs > is set e.g. on space object. > > Follow-up #4560 > Closes #4770 > > Signed-off-by: Igor Munkin <imun@tarantool.org> > --- > > @ChangeLog (need to be added to already released versions): > | __pairs/__ipairs metamethods handling is removed since we faced the > | issues with the backward compatibility between Lua 5.1 and Lua 5.2 > | within Tarantool modules as well as other third party code (gh-4770). > > Issue: https://github.com/tarantool/tarantool/issues/4770 > Branch: https://github.com/tarantool/tarantool/tree/imun/gh-4770-broken-pairs > > v1: https://lists.tarantool.org/pipermail/tarantool-patches/2020-February/014375.html > > Changes in v2: > * moved test to a separate file > > cmake/luajit.cmake | 1 - > ...4770-broken-pairs-for-space-objects.result | 52 +++++++++++++++++++ > ...70-broken-pairs-for-space-objects.test.lua | 18 +++++++ > 3 files changed, 70 insertions(+), 1 deletion(-) > create mode 100644 test/box/gh-4770-broken-pairs-for-space-objects.result > create mode 100644 test/box/gh-4770-broken-pairs-for-space-objects.test.lua > > diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake > index 072db8269..10df633d5 100644 > --- a/cmake/luajit.cmake > +++ b/cmake/luajit.cmake > @@ -217,7 +217,6 @@ macro(luajit_build) > add_definitions(-DLUAJIT_USE_ASAN=1) > set (luajit_ldflags ${luajit_ldflags} -fsanitize=address) > endif() > - add_definitions(-DLUAJIT_ENABLE_PAIRSMM=1) > add_definitions(-DLUAJIT_SMART_STRINGS=1) > # Add all COMPILE_DEFINITIONS to xcflags > get_property(defs DIRECTORY PROPERTY COMPILE_DEFINITIONS) > diff --git a/test/box/gh-4770-broken-pairs-for-space-objects.result b/test/box/gh-4770-broken-pairs-for-space-objects.result > new file mode 100644 > index 000000000..61618453e > --- /dev/null > +++ b/test/box/gh-4770-broken-pairs-for-space-objects.result > @@ -0,0 +1,52 @@ > +-- test-run result file version 2 > +env = require('test_run') > + | --- > + | ... > +test_run = env.new() > + | --- > + | ... > +test_run:cmd("push filter 'table: .*' to 'table: <address>'") > + | --- > + | - true > + | ... > + > +-- > +-- gh-4770: Iteration through space with Lua builtin pairs routine > +-- > +box.cfg{} > + | --- > + | ... ================================================================================ diff --git a/test/box/gh-4770-broken-pairs-for-space-objects.result b/test/box/gh-4770-broken-pairs-for-space-objects.result index 61618453e..2e31d5fbe 100644 --- a/test/box/gh-4770-broken-pairs-for-space-objects.result +++ b/test/box/gh-4770-broken-pairs-for-space-objects.result @@ -13,9 +13,6 @@ test_run:cmd("push filter 'table: .*' to 'table: <address>'") -- -- gh-4770: Iteration through space with Lua builtin pairs routine -- -box.cfg{} - | --- - | ... s = box.schema.create_space('test') | --- | ... ================================================================================ > +s = box.schema.create_space('test') > + | --- > + | ... > +-- Check whether __pairs is set for the space object, since Lua Fun > +-- handles it manually underneath. > +getmetatable(s).__pairs == s.pairs > + | --- > + | - true > + | ... > +-- Check whether pairs builtin behaviour doesn't change when the > +-- __pairs is set. > +keys = {} > + | --- > + | ... > +for k, v in pairs(s) do keys[k] = true end > + | --- > + | ... > +keys > + | --- > + | - engine: true > + | before_replace: true > + | field_count: true > + | id: true > + | on_replace: true > + | temporary: true > + | index: true > + | is_local: true > + | enabled: true > + | name: true > + | ck_constraint: true > + | ... ================================================================================ diff --git a/test/box/gh-4770-broken-pairs-for-space-objects.result b/test/box/gh-4770-broken-pairs-for-space-objects.result index 2e31d5fbe..3ac5995c2 100644 --- a/test/box/gh-4770-broken-pairs-for-space-objects.result +++ b/test/box/gh-4770-broken-pairs-for-space-objects.result @@ -30,19 +30,11 @@ keys = {} for k, v in pairs(s) do keys[k] = true end | --- | ... -keys - | --- - | - engine: true - | before_replace: true - | field_count: true - | id: true - | on_replace: true - | temporary: true - | index: true - | is_local: true - | enabled: true - | name: true - | ck_constraint: true +keys.name, keys.id, keys.engine + | --- + | - true + | - true + | - true | ... s:drop() | --- ================================================================================ > +s:drop() > + | --- > + | ... > diff --git a/test/box/gh-4770-broken-pairs-for-space-objects.test.lua b/test/box/gh-4770-broken-pairs-for-space-objects.test.lua > new file mode 100644 > index 000000000..15024a803 > --- /dev/null > +++ b/test/box/gh-4770-broken-pairs-for-space-objects.test.lua > @@ -0,0 +1,18 @@ > +env = require('test_run') > +test_run = env.new() > +test_run:cmd("push filter 'table: .*' to 'table: <address>'") > + > +-- > +-- gh-4770: Iteration through space with Lua builtin pairs routine > +-- > +box.cfg{} ================================================================================ diff --git a/test/box/gh-4770-broken-pairs-for-space-objects.test.lua b/test/box/gh-4770-broken-pairs-for-space-objects.test.lua index 15024a803..7acf76eb5 100644 --- a/test/box/gh-4770-broken-pairs-for-space-objects.test.lua +++ b/test/box/gh-4770-broken-pairs-for-space-objects.test.lua @@ -5,7 +5,6 @@ test_run:cmd("push filter 'table: .*' to 'table: <address>'") -- -- gh-4770: Iteration through space with Lua builtin pairs routine -- -box.cfg{} s = box.schema.create_space('test') -- Check whether __pairs is set for the space object, since Lua Fun -- handles it manually underneath. ================================================================================ > +s = box.schema.create_space('test') > +-- Check whether __pairs is set for the space object, since Lua Fun > +-- handles it manually underneath. > +getmetatable(s).__pairs == s.pairs > +-- Check whether pairs builtin behaviour doesn't change when the > +-- __pairs is set. > +keys = {} > +for k, v in pairs(s) do keys[k] = true end > +keys ================================================================================ diff --git a/test/box/gh-4770-broken-pairs-for-space-objects.test.lua b/test/box/gh-4770-broken-pairs-for-space-objects.test.lua index 7acf76eb5..315352b15 100644 --- a/test/box/gh-4770-broken-pairs-for-space-objects.test.lua +++ b/test/box/gh-4770-broken-pairs-for-space-objects.test.lua @@ -13,5 +13,5 @@ getmetatable(s).__pairs == s.pairs -- __pairs is set. keys = {} for k, v in pairs(s) do keys[k] = true end -keys +keys.name, keys.id, keys.engine s:drop() ================================================================================ > +s:drop() > -- > 2.25.0 > -- Best regards, IM ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH v2] build: disable LUAJIT_ENABLE_PAIRSMM 2020-03-06 17:41 ` Igor Munkin @ 2020-03-11 23:19 ` Vladislav Shpilevoy 2020-03-16 10:12 ` Igor Munkin 0 siblings, 1 reply; 8+ messages in thread From: Vladislav Shpilevoy @ 2020-03-11 23:19 UTC (permalink / raw) To: Igor Munkin; +Cc: tarantool-patches Hi! Thanks for the patch! > --- /dev/null > +++ b/test/box/gh-4770-broken-pairs-for-space-objects.result > @@ -0,0 +1,41 @@ > +-- test-run result file version 2 > +env = require('test_run') > + | --- > + | ... > +test_run = env.new() > + | --- > + | ... > +test_run:cmd("push filter 'table: .*' to 'table: <address>'") Why do you need the filter? There is nothing to filter out. You never print any 'table: .*' strings. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH v2] build: disable LUAJIT_ENABLE_PAIRSMM 2020-03-11 23:19 ` Vladislav Shpilevoy @ 2020-03-16 10:12 ` Igor Munkin 2020-03-16 22:05 ` Vladislav Shpilevoy 0 siblings, 1 reply; 8+ messages in thread From: Igor Munkin @ 2020-03-16 10:12 UTC (permalink / raw) To: Vladislav Shpilevoy; +Cc: tarantool-patches Vlad, On 12.03.20, Vladislav Shpilevoy wrote: > Hi! Thanks for the patch! > > > --- /dev/null > > +++ b/test/box/gh-4770-broken-pairs-for-space-objects.result > > @@ -0,0 +1,41 @@ > > +-- test-run result file version 2 > > +env = require('test_run') > > + | --- > > + | ... > > +test_run = env.new() > > + | --- > > + | ... > > +test_run:cmd("push filter 'table: .*' to 'table: <address>'") > > Why do you need the filter? There is nothing to filter out. > You never print any 'table: .*' strings. > Thanks for your comment! I overlooked this line when copy-pasting the preambule from box/misc.test.lua and I agree the filter is excess. Fixed, squashed, force-pushed to the branch. Diff is below: ================================================================================ diff --git a/test/box/gh-4770-broken-pairs-for-space-objects.result b/test/box/gh-4770-broken-pairs-for-space-objects.result index 3ac5995c2..c678714ee 100644 --- a/test/box/gh-4770-broken-pairs-for-space-objects.result +++ b/test/box/gh-4770-broken-pairs-for-space-objects.result @@ -1,14 +1,7 @@ -- test-run result file version 2 -env = require('test_run') +test_run = require('test_run').new() | --- | ... -test_run = env.new() - | --- - | ... -test_run:cmd("push filter 'table: .*' to 'table: <address>'") - | --- - | - true - | ... -- -- gh-4770: Iteration through space with Lua builtin pairs routine diff --git a/test/box/gh-4770-broken-pairs-for-space-objects.test.lua b/test/box/gh-4770-broken-pairs-for-space-objects.test.lua index 315352b15..a1ba1eeab 100644 --- a/test/box/gh-4770-broken-pairs-for-space-objects.test.lua +++ b/test/box/gh-4770-broken-pairs-for-space-objects.test.lua @@ -1,6 +1,4 @@ -env = require('test_run') -test_run = env.new() -test_run:cmd("push filter 'table: .*' to 'table: <address>'") +test_run = require('test_run').new() -- -- gh-4770: Iteration through space with Lua builtin pairs routine ================================================================================ -- Best regards, IM ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH v2] build: disable LUAJIT_ENABLE_PAIRSMM 2020-03-16 10:12 ` Igor Munkin @ 2020-03-16 22:05 ` Vladislav Shpilevoy 2020-03-16 22:46 ` Igor Munkin 0 siblings, 1 reply; 8+ messages in thread From: Vladislav Shpilevoy @ 2020-03-16 22:05 UTC (permalink / raw) To: Igor Munkin; +Cc: tarantool-patches > diff --git a/test/box/gh-4770-broken-pairs-for-space-objects.result b/test/box/gh-4770-broken-pairs-for-space-objects.result > index 3ac5995c2..c678714ee 100644 > --- a/test/box/gh-4770-broken-pairs-for-space-objects.result > +++ b/test/box/gh-4770-broken-pairs-for-space-objects.result > @@ -1,14 +1,7 @@ > -- test-run result file version 2 > -env = require('test_run') > +test_run = require('test_run').new() Seems like you don't need 'test_run' object either. But ok, LGTM. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH v2] build: disable LUAJIT_ENABLE_PAIRSMM 2020-03-16 22:05 ` Vladislav Shpilevoy @ 2020-03-16 22:46 ` Igor Munkin 0 siblings, 0 replies; 8+ messages in thread From: Igor Munkin @ 2020-03-16 22:46 UTC (permalink / raw) To: Vladislav Shpilevoy; +Cc: tarantool-patches Vlad, Thanks for your review. On 16.03.20, Vladislav Shpilevoy wrote: > > diff --git a/test/box/gh-4770-broken-pairs-for-space-objects.result b/test/box/gh-4770-broken-pairs-for-space-objects.result > > index 3ac5995c2..c678714ee 100644 > > --- a/test/box/gh-4770-broken-pairs-for-space-objects.result > > +++ b/test/box/gh-4770-broken-pairs-for-space-objects.result > > @@ -1,14 +1,7 @@ > > -- test-run result file version 2 > > -env = require('test_run') > > +test_run = require('test_run').new() > > Seems like you don't need 'test_run' object either. Fixed, squashed, force-pushed to the branch. Diff is below: ================================================================================ diff --git a/test/box/gh-4770-broken-pairs-for-space-objects.result b/test/box/gh-4770-broken-pairs-for-space-objects.result index c678714ee..6ab778d1a 100644 --- a/test/box/gh-4770-broken-pairs-for-space-objects.result +++ b/test/box/gh-4770-broken-pairs-for-space-objects.result @@ -1,8 +1,4 @@ -- test-run result file version 2 -test_run = require('test_run').new() - | --- - | ... - -- -- gh-4770: Iteration through space with Lua builtin pairs routine -- diff --git a/test/box/gh-4770-broken-pairs-for-space-objects.test.lua b/test/box/gh-4770-broken-pairs-for-space-objects.test.lua index a1ba1eeab..8c36bd305 100644 --- a/test/box/gh-4770-broken-pairs-for-space-objects.test.lua +++ b/test/box/gh-4770-broken-pairs-for-space-objects.test.lua @@ -1,5 +1,3 @@ -test_run = require('test_run').new() - -- -- gh-4770: Iteration through space with Lua builtin pairs routine -- ================================================================================ > > But ok, LGTM. -- Best regards, IM ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH v2] build: disable LUAJIT_ENABLE_PAIRSMM 2020-03-06 16:28 [Tarantool-patches] [PATCH v2] build: disable LUAJIT_ENABLE_PAIRSMM Igor Munkin 2020-03-06 17:41 ` Igor Munkin @ 2020-03-17 7:30 ` Kirill Yukhin 2020-03-17 7:37 ` Kirill Yukhin 1 sibling, 1 reply; 8+ messages in thread From: Kirill Yukhin @ 2020-03-17 7:30 UTC (permalink / raw) To: Igor Munkin; +Cc: tarantool-patches, Vladislav Shpilevoy Hello, On 06 мар 19:28, Igor Munkin wrote: > Since this build flag has been removed as a result of reverting the > tarantool/luajit@d4e985a, its definition in the corresponding Tarantool > cmake file is irrelevant. > > Furthermore, considering the breakage faced in #4770 the following tests > are introduced: > * the check whether space __pairs metamethod is set to space.pairs to > create a Lua Fun iterator that handles __pairs manually underneath. > * the check whether pairs builtin behaviour doesn't change when __pairs > is set e.g. on space object. > > Follow-up #4560 > Closes #4770 > > Signed-off-by: Igor Munkin <imun@tarantool.org> LGTM. Checked into master, 2.3 and 2.2. -- Regards, Kirill Yukhin ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH v2] build: disable LUAJIT_ENABLE_PAIRSMM 2020-03-17 7:30 ` Kirill Yukhin @ 2020-03-17 7:37 ` Kirill Yukhin 0 siblings, 0 replies; 8+ messages in thread From: Kirill Yukhin @ 2020-03-17 7:37 UTC (permalink / raw) To: Igor Munkin; +Cc: tarantool-patches, Vladislav Shpilevoy On 17 мар 10:30, Kirill Yukhin wrote: > Hello, > > On 06 мар 19:28, Igor Munkin wrote: > > Since this build flag has been removed as a result of reverting the > > tarantool/luajit@d4e985a, its definition in the corresponding Tarantool > > cmake file is irrelevant. > > > > Furthermore, considering the breakage faced in #4770 the following tests > > are introduced: > > * the check whether space __pairs metamethod is set to space.pairs to > > create a Lua Fun iterator that handles __pairs manually underneath. > > * the check whether pairs builtin behaviour doesn't change when __pairs > > is set e.g. on space object. > > > > Follow-up #4560 > > Closes #4770 > > > > Signed-off-by: Igor Munkin <imun@tarantool.org> > > LGTM. > Checked into master, 2.3 and 2.2. Checked into 1.10 as well. -- Regards, Kirill Yukhin ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-03-17 7:37 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-06 16:28 [Tarantool-patches] [PATCH v2] build: disable LUAJIT_ENABLE_PAIRSMM Igor Munkin 2020-03-06 17:41 ` Igor Munkin 2020-03-11 23:19 ` Vladislav Shpilevoy 2020-03-16 10:12 ` Igor Munkin 2020-03-16 22:05 ` Vladislav Shpilevoy 2020-03-16 22:46 ` Igor Munkin 2020-03-17 7:30 ` Kirill Yukhin 2020-03-17 7:37 ` Kirill Yukhin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox