<HTML><BODY><div>Hi, Sergey!</div><div>Thanks for the review!</div><div>Fixed all of your comments, except for the one about</div><div>`luacheckrc` and _TARANTOOL. I believe, it should be</div><div>done in a separate patch.</div><div>Here is the diff for other changes:</div><div> </div><div><div><div>diff --git a/src/luajit.c b/src/luajit.c</div><div>index 84472464..b63c92d1 100644</div><div>--- a/src/luajit.c</div><div>+++ b/src/luajit.c</div><div>@@ -79,6 +79,17 @@ static void print_usage(void)</div><div> fflush(stderr);</div><div> }</div><div> </div><div>+static void print_tools_usage(void)</div><div>+{</div><div>+ fputs("usage: ", stderr);</div><div>+ fputs(progname, stderr);</div><div>+ fputs(" -t<cmd>\n"</div><div>+ "Available tools are:\n"</div><div>+ " -m [--leak-only] input Memprof profile data parser.\n"</div><div>+ " -s input Sysprof profile data parser.\n", stderr);</div><div>+ fflush(stderr);</div><div>+}</div><div>+</div><div> static void l_message(const char *pname, const char *msg)</div><div> {</div><div> if (pname) { fputs(pname, stderr); fputc(':', stderr); fputc(' ', stderr); }</div><div>@@ -387,7 +398,7 @@ static int dotoolcmd(lua_State *L, const char *cmd)</div><div> case 's':</div><div> return runtoolcmd(L, "sysprof");</div><div> default:</div><div>- l_message(progname, "unknown tool command");</div><div>+ print_tools_usage();</div><div> break;</div><div> }</div><div> return -1;</div><div>@@ -454,8 +465,6 @@ static int collectargs(char **argv, int *flags)</div><div> break;</div><div> case 't':</div><div> *flags |= FLAGS_TOOL;</div><div>- if (argv[i][2] == '\0') return -1;</div><div>- if (argv[i + 1] == NULL) return -1;</div><div> return i + 1;</div><div> case 'e':</div><div> *flags |= FLAGS_EXEC;</div><div>diff --git a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua</div><div>index ce99535a..8187c594 100644</div><div>--- a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua</div><div>+++ b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua</div><div>@@ -28,12 +28,11 @@ local TABLE_SIZE = 20</div><div> local SMOKE_CMD_SET = {</div><div> {</div><div> cmd = EXECUTABLE .. ' -t ' .. BAD_PATH,</div><div>- -- luacheck: no global</div><div>- like = _TARANTOOL and '.+unknown tool command' or 'usage:.+',</div><div>+ like = '.+Available tools.+',</div><div> },</div><div> {</div><div> cmd = EXECUTABLE .. ' -ta ' .. BAD_PATH,</div><div>- like = '.+unknown tool command',</div><div>+ like = '.+Available tools.+',</div><div> },</div><div> }</div><div> </div></div></div><div> </div><div> </div><div data-signature-widget="container"><div data-signature-widget="content"><div>--<br>Best regards,</div><div>Maxim Kokryashkin</div></div></div><div> </div><div> </div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">Четверг, 7 сентября 2023, 12:41 +03:00 от Sergey Bronnikov <sergeyb@tarantool.org>:<br> <div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_16940796681954437009_BODY">Hi, Max<br><br><br>On <span class="js-phone-number">8/31/23 14</span>:35, Maxim Kokryashkin wrote:<br>> It is really inconvenient to use a standalone shell script to parse<br>> binary dumps from profilers. That is why this commit introduces a<br>> CLI flag for tools in the LuaJIT, so now it is possible to parse<br>> a memprof dump as simple as:<br>> ```<br>> luajit -tm memprof.bin<br>> luajit -tm --leak-only memprof.bin<br>> ```<br>><br>> And the sysprof too:<br>> ```<br>> luajit -ts sysprof.bin<br>> luajit -ts --split sysprof.bin<br><br>$ ./build/src/luajit -ts --split sysprof.bin<br>luajit-parse-sysprof.lua: ERROR: unrecognized option `--split'. Try<br>`--help'.<br><br><br>> ```<br>><br>> The `luajit-parse-memprof` and `luajit-parse-sysprof` are purged<br>> as a result of these changes.<br>Typo: s/The/The scripts/<br>><br>> Closes tarantool/tarantool#5688<br>> ---<br><snipped><br>> diff --git a/src/luajit.c b/src/luajit.c<br>> index 3a3ec247..f57b7e95 100644<br>> --- a/src/luajit.c<br>> +++ b/src/luajit.c<br>> @@ -72,6 +72,7 @@ static void print_usage(void)<br>> " -O[opt] Control LuaJIT optimizations.\n"<br>> " -i Enter interactive mode after executing " LUA_QL("script") ".\n"<br>> " -v Show version information.\n"<br>> + " -t<cmd> Execute tool.\n"<br><br><br>When I run "luajit -b" without arguments LuaJIT shows me a usage for "-b":<br><br>[0] ~/sources/MRG/tarantool/third_party/luajit$ ./build/src/luajit -b<br>Save LuaJIT bytecode: luajit -b[options] input output<br> -l Only list bytecode.<br> -s Strip debug info (default).<br> -g Keep debug info.<br> -n name Set module name (default: auto-detect from input name).<br> -t type Set output file type (default: auto-detect from output name).<br> -a arch Override architecture for object files (default: native).<br> -o os Override OS for object files (default: native).<br> -e chunk Use chunk string as input.<br> -- Stop handling options.<br> - Use stdin as input and/or stdout as output.<br><br>File types: c h obj o raw (default)<br><br><br>When I run "luajit -t" without arguments LuaJIT shows me the same usage,<br>that is not helpful:<br><br><br>[0] ~/sources/MRG/tarantool/third_party/luajit$ ./build/src/luajit -t<br>usage: ./build/src/luajit [options]... [script [args]...].<br>Available options are:<br> -e chunk Execute string 'chunk'.<br> -l name Require library 'name'.<br> -b ... Save or list bytecode.<br> -j cmd Perform LuaJIT control command.<br> -O[opt] Control LuaJIT optimizations.<br> -i Enter interactive mode after executing 'script'.<br> -v Show version information.<br> -t<cmd> Execute tool.<br> -E Ignore environment variables.<br> -- Stop handling options.<br> - Execute stdin and stop handling options.<br><br>Please show a usage with available arguments for "-t":<br><br><br>Parse profile dump: luajit -t[options] input<br> -m parse memprof dump.<br> -s parse sysprof dump (default).<br><br> --leak-only ...<br><br> --split ...<br><br><br><br>> " -E Ignore environment variables.\n"<br>> " -- Stop handling options.\n"<br>> " - Execute stdin and stop handling options.\n", stderr);<br>> @@ -361,6 +362,37 @@ static int dojitcmd(lua_State *L, const char *cmd)<br>> return runcmdopt(L, opt ? opt+1 : opt);<br>> }<br>><br><br><snipped><br><br><br>> diff --git a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua<br>> new file mode 100644<br>> index 00000000..4dc4f6a1<br>> --- /dev/null<br>> +++ b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua<br>> @@ -0,0 +1,127 @@<br>> +local tap = require('tap')<br>> +local test = tap.test('gh-5688-tool-cli-flag'):skipcond({<br>> + ['Profile tools are implemented for x86_64 only'] = jit.arch ~= 'x86' and<br>> + jit.arch ~= 'x64',<br><br>indentation<br><br><br>> + ['Profile tools are implemented for Linux only'] = jit.os ~= 'Linux',<br>> + -- XXX: Tarantool integration is required to run this test properly.<br>> + -- luacheck: no global<br>> + ['No profile tools CLI option integration'] = _TARANTOOL,<br>> +})<br>> +<br>> +test:plan(3)<br>> +<br>> +jit.off()<br>> +jit.flush()<br>> +<br>> +local table_new = require('table.new')<br>> +local utils = require('utils')<br>> +<br>> +local BAD_PATH = utils.tools.profilename('bad-path-tmp.bin')<br>> +local TMP_BINFILE_MEMPROF = utils.tools.profilename('memprofdata.tmp.bin')<br>> +local TMP_BINFILE_SYSPROF = utils.tools.profilename('sysprofdata.tmp.bin')<br>> +<br>> +local EXECUTABLE = utils.exec.luacmd(arg)<br>> +local MEMPROF_PARSER = EXECUTABLE .. ' -tm '<br>> +local SYSPROF_PARSER = EXECUTABLE .. ' -ts '<br>> +<br>> +local REDIRECT_OUTPUT = ' 2>&1'<br>> +<br>> +local TABLE_SIZE = 20<br>> +<br>> +local SMOKE_CMD_SET = {<br>> + {<br>> + cmd = EXECUTABLE .. ' -t ' .. BAD_PATH,<br>> + -- luacheck: no global<br>> + like = _TARANTOOL and '.+unknown tool command' or 'usage:.+',<br><br>Let's define this global in .luacheckrc.<br><br>I think we will use _TARANTOOL more and this inline suppressions will be<br>annoying.<br><br><br><no obvious problems, snipped></div></div></div></div></blockquote><div> </div></BODY></HTML>