Re: [Tarantool-patches] [PATCH luajit 1/2][v2] Fix embedded bytecode loader.

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi,


On 9/11/23 11:45, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the fixes!
>
> On 07.09.23, Sergey Bronnikov wrote:
>> Hi, Sergey
>>

<snipped>

>>
>>> | ""
>>>
>>> Nit: use single quotes instead of double quotes if possible.
>> Without context it is difficult to get what is line you talk about.
>>
>> As I see everything is fine with quotes in version on the branch.
>>
> I'm talking about the same line (got this version from the branch):
>
> | +local function basename(path)
> | +  return path:match("[^/]*$")
> | +end

Fixed.


--- a/test/tarantool-tests/utils/tools.lua
+++ b/test/tarantool-tests/utils/tools.lua
@@ -10,7 +10,7 @@ function M.basedir(path)
  end

  function M.basename(path)
-  return path:match("[^/]*$")
+  return path:match('[^/]*$')
  end

  function M.profilename(name)

>>> Nit: `[^/\\]` is better since it also covers Windows.
>>> See <test/lua-Harness-tests/314-regex.t:167>
>>> | local dirname = arg[0]:gsub('([^/\\]+)$', '')
>>> Since we don't support Windows feel free to ignore.
>>>
>>>> +end
>>>> +
>>>> +local stdout_msg = 'Lango team'
>>>> +local lua_code = ('print(%q)'):format(stdout_msg)
>>>> +local fpath = os.tmpname()
>>>> +local path_lua = ('%s.lua'):format(fpath)
>>>> +local path_c = ('%s.c'):format(fpath)
>>>> +local path_so = ('%s.so'):format(fpath)
>>> Minor: I suppose it should be renamed to `path_shared`, since on macOS
>>> they have the ".dyld" suffix for shared libs. Hence, we need to use the
>>> suffix in format of the shared library name too. You may take some
>>> inspiration from here [1].
>> Fixed.
> Hmm, I don't see these updates on the branch [1]. Maybe you forgot to push
> your local changes to the repository (*)?

Right, I forgot to force-push ;(

Force-pushed now.


<snipped>

>>> Nit: Indent is 4 spaces instead of 2.
>>>
>>>> +    return path:match('(.*' .. sep .. ')') or './'
>>> It's better to mention that the pattern matching is greedy, so we match
>>> until the last separator.
>>
>> Updated.
> This is missing too (*).
Added a comment.
>
<snipped>