From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 C66E34696C3 for ; Sat, 4 Apr 2020 22:30:04 +0300 (MSK) References: <1090ea3aa089beee61c44d0e42b3bcf6ea250782.1585171743.git.lvasiliev@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Sat, 4 Apr 2020 21:30:02 +0200 MIME-Version: 1.0 In-Reply-To: <1090ea3aa089beee61c44d0e42b3bcf6ea250782.1585171743.git.lvasiliev@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH] rocks: forward options to luarocks List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Leonid Vasiliev Cc: tarantool-patches@dev.tarantool.org Thanks for the patch! See 5 comments below. On 25/03/2020 23:01, Leonid Vasiliev wrote: > From: Leonid > > https://github.com/tarantool/tarantool/issues/4629 > https://github.com/tarantool/tarantool/tree/lvasiliev/gh-4629-forward-flags > > Corresponding thread https://lists.tarantool.org/pipermail/tarantool-patches/2020-March/015143.html > > @Changelog > Add tarantoolctl rocks commands: build config download init lint new_version purge which write_rockspec > Fix forwarding flags to luarocks > Help section has been updated 1. Please, remove ChangeLog from the commit message. We never put them here. All what is not added to the commit message should be put after '---' below. Branch, issue, and all other links and comments too. It was asked by Oleg too, but seems the changelog is still here. > The policy for check of luarocks flags has been changed (moved from tarantoolctl to luarocks). > We had a white-list of luarocks options in tarantoolctl before. > Now we have a black-list of options (for tarantool) in luarocks. > Help section has been updated. > Chdir has been moved to luarocks. > > @TarantoolBot document > Title: Update tarantoolctl rocks > tarantoolctl rocks commands has been added: > build > config > download > init > lint > new_version > purge > which > write_rockspec > > Closes #4629 > --- This is the place for all text not added to the commit message. > extra/dist/tarantoolctl.in | 157 ++++++++------------------------------------- > third_party/luarocks | 2 +- > 2 files changed, 28 insertions(+), 131 deletions(-) > > diff --git a/extra/dist/tarantoolctl.in b/extra/dist/tarantoolctl.in > index 6daf866..66dfec8 100755 > --- a/extra/dist/tarantoolctl.in > +++ b/extra/dist/tarantoolctl.in > @@ -21,7 +21,6 @@ ffi.cdef[[ > int kill(int pid, int sig); > int isatty(int fd); > int getppid(void); > -int chdir(const char *path); > ]] > > local TIMEOUT_INFINITY = 100 * 365 * 86400 > @@ -936,14 +935,6 @@ local function rocks() > cfg.init() > local util = require("luarocks.util") > local command_line = require("luarocks.cmd") > - local options = keyword_arguments > - local key = nil > - if options["only-server"] ~= nil then > - key = find_arg("only%-server") > - else > - key = find_arg("server") > - end > - table.insert(positional_arguments, key) > > -- Tweak help messages > util.see_help = function(command, program) > @@ -951,27 +942,36 @@ local function rocks() > return "See Tarantool documentation for help." > end > > - -- Enable only useful commands > + -- Disabled: path, upload > local rocks_commands = { > + build = "luarocks.cmd.build", > + config = "luarocks.cmd.config", > + doc = "luarocks.cmd.doc", > + download = "luarocks.cmd.download", > + help = "luarocks.cmd.help", > + init = "luarocks.cmd.init", > install = "luarocks.cmd.install", > - search = "luarocks.cmd.search", > + lint = "luarocks.cmd.lint", > list = "luarocks.cmd.list", > - remove = "luarocks.cmd.remove", > - show = "luarocks.cmd.show", > make = "luarocks.cmd.make", > + make_manifest = "luarocks.admin.cmd.make_manifest", > + new_version = "luarocks.cmd.new_version", > pack = "luarocks.cmd.pack", > - unpack = "luarocks.cmd.unpack", > - doc = "luarocks.cmd.doc", > - help = "luarocks.cmd.help", > + purge = "luarocks.cmd.purge", > + remove = "luarocks.cmd.remove", > + search = "luarocks.cmd.search", > + show = "luarocks.cmd.show", > test = "luarocks.cmd.test", > - make_manifest = "luarocks.admin.cmd.make_manifest", > + unpack = "luarocks.cmd.unpack", > + which = "luarocks.cmd.which", > + write_rockspec = "luarocks.cmd.write_rockspec", > } > > - if keyword_arguments.chdir then > - ffi.C.chdir(keyword_arguments.chdir) > - end > + --prepare arguments for luarocks 2. Please, add whitespace after '--', start the sentence with a capital letter, and end with a dot. > + shift_argv(lua_arguments, 0, 1) > -- Call LuaRocks > - command_line.run_command('', rocks_commands, nil, unpack(positional_arguments)) > + command_line.run_command('LuaRocks main command-line interface', 3. I thought we should not print any 'LuaRocks' text when run 'tarantoolctl rocks'. So why did you change this? > + rocks_commands, nil, unpack(lua_arguments)) > end > > local function exit_wrapper(func) > @@ -1186,117 +1186,14 @@ local commands = setmetatable({ > }, rocks = { > func = exit_wrapper(rocks), process = process_remote, help = { > header = > - "%s rocks [install|make|remove|show|search|list|pack|unpack|doc|test|make_manifest|help]", > + "%s rocks - package managment", 4. 'managment' -> 'management'. Also none of the commands print it like that. They print description and command line separately. See 'tarantoolctl help'. In the command line after 'rocks' should be said something like 'COMMAND'. And in the description you can provide, well, description. > description = > [=[ > - Package management. > + See tarantoolctl rocks help > ]=], > @@ -1411,7 +1305,10 @@ local function populate_arguments() > end > > local function main() > - populate_arguments() > + if command_name ~= 'rocks' then -- rocks parse arguments themselves > + populate_arguments() > + end 5. You probably don't need to change that. Let it parse. You don't use the result anyway. > + > local cmd_pair = commands[command_name] > if #arg < 2 then > log.error("Not enough arguments for '%s' command\n", command_name)