[Tarantool-patches] [PATCH 3/3] Adapt luarocks help for the tarantoolctl

[Vladislav Shpilevoy]

On 05/04/2020 17:15, lvasiliev wrote:
> Hi! Thanks for the review. See PATCH v2 and comments bellow.
> 
> On 04.04.2020 22:02, Vladislav Shpilevoy wrote:
>> Thanks for the patch!
>>
>> It would be good to get any explanations here. My main
>> question is whether luarocks is a self sufficient tool,
>> or it is a library used solely by tarantoolctl? In the
>> latter case lots of things can be done simpler than they
>> are. In the former case it looks incorrect to use 'tarantoolct'
>> name anywhere in the sources.
> In my mind it is a self sufficient tool with some our patches (not only mine).

If this is a self-sufficient tool, then why did you replace
'luarocks' with 'tarantoolctl' in so many places here? Self
sufficient tool would not depend on tarantoolctl. On tarantool -
maybe. Since this is basically a Lua interpreter from luarocks'
point of view. But not on the tool based on tarantool. Before
your patch there was no a single mentioning of 'tarantoolct'.
Only of 'tarantool'. Now this encapsulation is broken.

> luaroks has been changed to tarantoolctl rocks in the cases where it used like program name, in my mind it's user frendly and more correct.

Yeah, that basically makes luarocks not a self-sufficient tool.
Because if I will invoke it using tarantool binary directly (not
tarantoolctl), then I will see 'tarantoolctl' references in 'help'
sections anyway.

>> See 3 comments below.
>>
>> On 25/03/2020 22:50, Leonid Vasiliev wrote:
>>> ---
>>>   src/luarocks/cmd/config.lua      | 18 +++++++++---------
>>>   src/luarocks/cmd/help.lua        | 10 +++-------
>>>   src/luarocks/cmd/make.lua        |  4 ++--
>>>   src/luarocks/cmd/new_version.lua |  2 +-
>>>   src/luarocks/cmd/purge.lua       |  2 +-
>>>   5 files changed, 16 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/src/luarocks/cmd/config.lua b/src/luarocks/cmd/config.lua
>>> index e9b2fef..d540dea 100644
>>> --- a/src/luarocks/cmd/config.lua
>>> +++ b/src/luarocks/cmd/config.lua
>>> @@ -17,9 +17,9 @@ config_cmd.help = [[
>>>     all config files and any command-line flags passed)
>>>       Examples:
>>> -     luarocks config lua_interpreter
>>> -     luarocks config variables.LUA_INCDIR
>>> -     luarocks config lua_version
>>> +     tarantoolctl rocks config lua_interpreter
>>> +     tarantoolctl rocks config variables.LUA_INCDIR
>>> +     tarantoolctl rocks config lua_version
>>>     * When given a configuration key and a value,
>>>     it overwrites the config file (see the --scope option below to determine which)
>>> @@ -31,26 +31,26 @@ config_cmd.help = [[
>>>       used by LuaRocks commands (eqivalent to passing --lua-version).
>>>       Examples:
>>> -     luarocks config variables.OPENSSL_DIR /usr/local/openssl
>>> -     luarocks config lua_dir /usr/local
>>> -     luarocks config lua_version 5.3
>>> +     tarantoolctl rocks config variables.OPENSSL_DIR /usr/local/openssl
>>> +     tarantoolctl rocks config lua_dir /usr/local
>>> +     tarantoolctl rocks config lua_version 5.3
>>>     * When given a configuration key and --unset,
>>>     it overwrites the config file (see the --scope option below to determine which)
>>>     and deletes that key from the file.
>>>   -  Example: luarocks config variables.OPENSSL_DIR --unset
>>> +  Example: tarantoolctl rocks config variables.OPENSSL_DIR --unset
>>>     * When given no arguments, it prints the entire currently active
>>>     configuration, resulting from reading the config files from
>>>     all scopes.
>>>   -  Example: luarocks config
>>> +  Example: tarantoolctl rocks config
>>>     OPTIONS
>>>   --scope=<scope>   The scope indicates which config file should be rewritten.
>>>                     Accepted values are "system", "user" or "project".
>>> -                  * Using a wrapper created with `luarocks init`,
>>> +                  * Using a wrapper created with `tarantoolctl rocks init`,
>>>                       the default is "project".
>>>                     * Using --local (or when `local_by_default` is `true`),
>>>                       the default is "user".
>>> diff --git a/src/luarocks/cmd/help.lua b/src/luarocks/cmd/help.lua
>>> index dcc9e35..689fd2d 100644
>>> --- a/src/luarocks/cmd/help.lua
>>> +++ b/src/luarocks/cmd/help.lua
>>> @@ -10,7 +10,7 @@ local util = require("luarocks.util")
>>>   local cfg = require("luarocks.core.cfg")
>>>   local dir = require("luarocks.dir")
>>>   -local program = util.this_program("luarocks")
>>> +local program = util.this_program("luarocks") .. " rocks"
>>
>> 1. So under some circumstances it is possible to get help in
>> form of 'luarocks rocks' instead of 'tarantoolctl rocks'?
> 'luarocks rocks'? In my mind - No.

Then why do you provide default value 'luarocks' to this function?

>> Why does this util.this_program() even take any parameters? Can
>> it just return 'tarantoolctl' constantly?
> Because it can be "tarantoolctl", "./tarantoolctl" or something else.

This is weird. So if I invoke tarantoolctl as is, I get this:

    $ ./extra/dist/tarantoolctl --help

    Tarantool client utility (2.4.0-89-g1c4da49bc)

    Usage:
        tarantoolctl start INSTANCE

            Start a Tarantool instance.


But if I add 'rocks' I suddenly get ./extra/dist prefix:

    $ ./extra/dist/tarantoolctl rocks --help

    LuaRocks 3.1.1, the Lua package manager

    NAME
            ./extra/dist/tarantoolctl - LuaRocks main command-line interface

    SYNOPSIS
            ./extra/dist/tarantoolctl [<flags...>] [VAR=VALUE]... <command> [<argument>] 


It does not look correct. Besides, it says here that the whole 'taranctoolctl'
is just LuaRocks command line interface:

    ./extra/dist/tarantoolctl - LuaRocks main command-line interface

But it is not. It is also instance manager, xlog reader, and some other things.

'tarantoolctl rocks' - is rocks interface. 'taranctoolctl' - is not. It is a
*set* of utilities. Not only rocks manager.

Moreover, SYNOPSIS section is now wrong. It says I can pass rocks commands
and flags right to 'tarantoolctl' without writing 'rocks' first.

>>>       --verbose              Display verbose output of commands executed.
>>>       --timeout=<seconds>    Timeout on network operations, in seconds.
>>>                              0 means no timeout (wait forever).
>>> diff --git a/src/luarocks/cmd/make.lua b/src/luarocks/cmd/make.lua
>>> index 025ac11..dc70d49 100644
>>> --- a/src/luarocks/cmd/make.lua
>>> +++ b/src/luarocks/cmd/make.lua
>>> @@ -32,8 +32,8 @@ This command is useful as a tool for debugging rockspecs.
>>>   To install rocks, you'll normally want to use the "install" and
>>>   "build" commands. See the help on those for details.
>>>   -NB: Use `luarocks install` with the `--only-deps` flag if you want to install
>>> -only dependencies of the rockspec (see `luarocks help install`).
>>> +NB: Use `tarantoolctl rocks install` with the `--only-deps` flag if you want to install
>>> +only dependencies of the rockspec (see `tarantoolctl rocks help install`).
>>>     --pack-binary-rock  Do not install rock. Instead, produce a .rock file
>>>                       with the contents of compilation in the current
>>> diff --git a/src/luarocks/cmd/new_version.lua b/src/luarocks/cmd/new_version.lua
>>> index f1181d4..e0ce760 100644
>>> --- a/src/luarocks/cmd/new_version.lua
>>> +++ b/src/luarocks/cmd/new_version.lua
>>> @@ -18,7 +18,7 @@ from a previous one.
>>>     If a package name is given, it downloads the latest rockspec from the
>>>   default server. If a rockspec is given, it uses it instead. If no argument
>>> -is given, it looks for a rockspec same way 'luarocks make' does.
>>> +is given, it looks for a rockspec same way 'tarantoolctl rocks make' does.
>>>     If the version number is not given and tag is passed using --tag,
>>>   it is used as the version, with 'v' removed from beginning.
>>> diff --git a/src/luarocks/cmd/purge.lua b/src/luarocks/cmd/purge.lua
>>> index 98b76a0..31b09e3 100644
>>> --- a/src/luarocks/cmd/purge.lua
>>> +++ b/src/luarocks/cmd/purge.lua
>>> @@ -21,7 +21,7 @@ purge.help = [[
>>>   This command removes rocks en masse from a given tree.
>>>   By default, it removes all rocks from a tree.
>>>   -The --tree argument is mandatory: luarocks purge does not
>>> +The --tree argument is mandatory: tarantoolctl rocks purge does not
>>>   assume a default tree.
>>>     --old-versions  Keep the highest-numbered version of each
>>
>> 3. If this commmit is indended to fully replace luarocks with tarantoolctl
>> in all comments and help texts, then it does not finish this task. I grepped
>> 'luarocks' and still see tons of places where luarocks is mentioned.
> I want to change luarocks to tarantoolctl rocks only where it used as program name in helps and stay luaroks where it used as a module name.

It is still used as a program name in:

luarocks/lfw/rocks/luafilesystem/1.5.0-1/doc/us/manual.html:91
luarocks/src/luarocks/deps.lua:464, 466, 657
luarocks/src/luarocks/purge.lua:22
luarocks/test/testing.sh has plenty of luarocks command mentionings.
luarocks/win32/bin/luarocksw.bat:11, 19, 21, 24, 26 //Windows, but anyway.

There may be more. I think to keep luarocks a self-sufficient
module not depending on tarantoolctl you need to be able to
pass program name as an argument somewhere. For example,
when taranctoolctl makes this:

    local cfg = require("luarocks.core.cfg")
    cfg.init()

It could pass caller's program name as an argument:

    cfg.init({progname = 'taranctoolctl rocks'})

and luarocks would use it instead of the default value
everywhere. Or pass it via a global variable in _G. Whatever.

I just noticed, that 'help' is not a part of this ticket. The ticket
was about using all luarocks options, except a few of them. So if
this task (replacing command name, fixing help) looks too big, it can
be moved to a new ticket and implemented after the upcoming releases.