Hi, Sergey!
Thanks for the review!
Fixed all of your comments, except for the one about
`luacheckrc` and _TARANTOOL. I believe, it should be
done in a separate patch.
Here is the diff for other changes:
 
diff --git a/src/luajit.c b/src/luajit.c
index 84472464..b63c92d1 100644
--- a/src/luajit.c
+++ b/src/luajit.c
@@ -79,6 +79,17 @@ static void print_usage(void)
   fflush(stderr);
 }
 
+static void print_tools_usage(void)
+{
+  fputs("usage: ", stderr);
+  fputs(progname, stderr);
+  fputs(" -t<cmd>\n"
+  "Available tools are:\n"
+  "  -m [--leak-only] input  Memprof profile data parser.\n"
+  "  -s input                Sysprof profile data parser.\n", stderr);
+  fflush(stderr);
+}
+
 static void l_message(const char *pname, const char *msg)
 {
   if (pname) { fputs(pname, stderr); fputc(':', stderr); fputc(' ', stderr); }
@@ -387,7 +398,7 @@ static int dotoolcmd(lua_State *L, const char *cmd)
   case 's':
     return runtoolcmd(L, "sysprof");
   default:
-    l_message(progname, "unknown tool command");
+    print_tools_usage();
     break;
   }
   return -1;
@@ -454,8 +465,6 @@ static int collectargs(char **argv, int *flags)
       break;
     case 't':
       *flags |= FLAGS_TOOL;
-      if (argv[i][2] == '\0') return -1;
-      if (argv[i + 1] == NULL) return -1;
       return i + 1;
     case 'e':
       *flags |= FLAGS_EXEC;
diff --git a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
index ce99535a..8187c594 100644
--- a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
+++ b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
@@ -28,12 +28,11 @@ local TABLE_SIZE = 20
 local SMOKE_CMD_SET = {
   {
     cmd = EXECUTABLE .. ' -t ' .. BAD_PATH,
-    -- luacheck: no global
-    like = _TARANTOOL and '.+unknown tool command' or 'usage:.+',
+    like = '.+Available tools.+',
   },
   {
     cmd = EXECUTABLE .. ' -ta ' .. BAD_PATH,
-    like = '.+unknown tool command',
+    like = '.+Available tools.+',
   },
 }
 
 
 
--
Best regards,
Maxim Kokryashkin
 
 
Четверг, 7 сентября 2023, 12:41 +03:00 от Sergey Bronnikov <sergeyb@tarantool.org>:
 
Hi, Max


On 8/31/23 14:35, Maxim Kokryashkin wrote:
> It is really inconvenient to use a standalone shell script to parse
> binary dumps from profilers. That is why this commit introduces a
> CLI flag for tools in the LuaJIT, so now it is possible to parse
> a memprof dump as simple as:
> ```
> luajit -tm memprof.bin
> luajit -tm --leak-only memprof.bin
> ```
>
> And the sysprof too:
> ```
> luajit -ts sysprof.bin
> luajit -ts --split sysprof.bin

$ ./build/src/luajit -ts --split sysprof.bin
luajit-parse-sysprof.lua: ERROR: unrecognized option `--split'. Try
`--help'.


> ```
>
> The `luajit-parse-memprof` and `luajit-parse-sysprof` are purged
> as a result of these changes.
Typo: s/The/The scripts/
>
> Closes tarantool/tarantool#5688
> ---
<snipped>
> diff --git a/src/luajit.c b/src/luajit.c
> index 3a3ec247..f57b7e95 100644
> --- a/src/luajit.c
> +++ b/src/luajit.c
> @@ -72,6 +72,7 @@ static void print_usage(void)
> " -O[opt] Control LuaJIT optimizations.\n"
> " -i Enter interactive mode after executing " LUA_QL("script") ".\n"
> " -v Show version information.\n"
> + " -t<cmd> Execute tool.\n"


When I run "luajit -b" without arguments LuaJIT shows me a usage for "-b":

[0] ~/sources/MRG/tarantool/third_party/luajit$ ./build/src/luajit -b
Save LuaJIT bytecode: luajit -b[options] input output
   -l        Only list bytecode.
   -s        Strip debug info (default).
   -g        Keep debug info.
   -n name   Set module name (default: auto-detect from input name).
   -t type   Set output file type (default: auto-detect from output name).
   -a arch   Override architecture for object files (default: native).
   -o os     Override OS for object files (default: native).
   -e chunk  Use chunk string as input.
   --        Stop handling options.
   -         Use stdin as input and/or stdout as output.

File types: c h obj o raw (default)


When I run "luajit -t" without arguments LuaJIT shows me the same usage,
that is not helpful:


[0] ~/sources/MRG/tarantool/third_party/luajit$ ./build/src/luajit -t
usage: ./build/src/luajit [options]... [script [args]...].
Available options are:
   -e chunk  Execute string 'chunk'.
   -l name   Require library 'name'.
   -b ...    Save or list bytecode.
   -j cmd    Perform LuaJIT control command.
   -O[opt]   Control LuaJIT optimizations.
   -i        Enter interactive mode after executing 'script'.
   -v        Show version information.
   -t<cmd>   Execute tool.
   -E        Ignore environment variables.
   --        Stop handling options.
   -         Execute stdin and stop handling options.

Please show a usage with available arguments for "-t":


Parse profile dump: luajit -t[options] input
   -m       parse memprof dump.
   -s        parse sysprof dump (default).

   --leak-only ...

   --split ...



> " -E Ignore environment variables.\n"
> " -- Stop handling options.\n"
> " - Execute stdin and stop handling options.\n", stderr);
> @@ -361,6 +362,37 @@ static int dojitcmd(lua_State *L, const char *cmd)
> return runcmdopt(L, opt ? opt+1 : opt);
> }
>

<snipped>


> diff --git a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
> new file mode 100644
> index 00000000..4dc4f6a1
> --- /dev/null
> +++ b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
> @@ -0,0 +1,127 @@
> +local tap = require('tap')
> +local test = tap.test('gh-5688-tool-cli-flag'):skipcond({
> + ['Profile tools are implemented for x86_64 only'] = jit.arch ~= 'x86' and
> + jit.arch ~= 'x64',

indentation


> + ['Profile tools are implemented for Linux only'] = jit.os ~= 'Linux',
> + -- XXX: Tarantool integration is required to run this test properly.
> + -- luacheck: no global
> + ['No profile tools CLI option integration'] = _TARANTOOL,
> +})
> +
> +test:plan(3)
> +
> +jit.off()
> +jit.flush()
> +
> +local table_new = require('table.new')
> +local utils = require('utils')
> +
> +local BAD_PATH = utils.tools.profilename('bad-path-tmp.bin')
> +local TMP_BINFILE_MEMPROF = utils.tools.profilename('memprofdata.tmp.bin')
> +local TMP_BINFILE_SYSPROF = utils.tools.profilename('sysprofdata.tmp.bin')
> +
> +local EXECUTABLE = utils.exec.luacmd(arg)
> +local MEMPROF_PARSER = EXECUTABLE .. ' -tm '
> +local SYSPROF_PARSER = EXECUTABLE .. ' -ts '
> +
> +local REDIRECT_OUTPUT = ' 2>&1'
> +
> +local TABLE_SIZE = 20
> +
> +local SMOKE_CMD_SET = {
> + {
> + cmd = EXECUTABLE .. ' -t ' .. BAD_PATH,
> + -- luacheck: no global
> + like = _TARANTOOL and '.+unknown tool command' or 'usage:.+',

Let's define this global in .luacheckrc.

I think we will use _TARANTOOL more and this inline suppressions will be
annoying.


<no obvious problems, snipped>