Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH 0/3] Adapt luarocks for tarantoolctl
@ 2020-03-25 21:50 Leonid Vasiliev
  2020-03-25 21:50 ` [Tarantool-patches] [PATCH 1/3] Add the chdir option for make Leonid Vasiliev
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Leonid Vasiliev @ 2020-03-25 21:50 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: tarantool-patches

https://github.com/tarantool/tarantool/issues/4629
https://github.com/tarantool/luarocks/tree/lvasiliev/gh-4629-add-chdir-to-make
See corresponding patch for tarantoolctl
(https://github.com/tarantool/tarantool/tree/lvasiliev/gh-4629-forward-flags)

Motivation:
Imperfect integretion of the Luarocks to tarantoolctl
(malfunctioning flags, disabled commands, invalid help)

Was done:
Whitelist in tarantoolctl has been changed to blacklist in luarocks
Help has been updated
Option chdir has been moved from tarantoolctl to luarocks

@ChangeLog - see a comment in tarantool

Leonid Vasiliev (3):
  Add the chdir option for make
  Add a black list of the tarantoolctl options
  Adapt luarocks help for the tarantoolctl

 src/luarocks/cmd/config.lua      | 18 +++++++++---------
 src/luarocks/cmd/help.lua        | 10 +++-------
 src/luarocks/cmd/make.lua        | 13 +++++++++++--
 src/luarocks/cmd/new_version.lua |  2 +-
 src/luarocks/cmd/purge.lua       |  2 +-
 src/luarocks/util.lua            | 15 +++++++++++++++
 6 files changed, 40 insertions(+), 20 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Tarantool-patches] [PATCH 1/3] Add the chdir option for make
  2020-03-25 21:50 [Tarantool-patches] [PATCH 0/3] Adapt luarocks for tarantoolctl Leonid Vasiliev
@ 2020-03-25 21:50 ` Leonid Vasiliev
  2020-04-02  9:58   ` lvasiliev
  2020-04-04 19:02   ` Vladislav Shpilevoy
  2020-03-25 21:50 ` [Tarantool-patches] [PATCH 2/3] Add a black list of the tarantoolctl options Leonid Vasiliev
  2020-03-25 21:50 ` [Tarantool-patches] [PATCH 3/3] Adapt luarocks help for the tarantoolctl Leonid Vasiliev
  2 siblings, 2 replies; 13+ messages in thread
From: Leonid Vasiliev @ 2020-03-25 21:50 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: tarantool-patches

---
 src/luarocks/cmd/make.lua | 9 +++++++++
 src/luarocks/util.lua     | 1 +
 2 files changed, 10 insertions(+)

diff --git a/src/luarocks/cmd/make.lua b/src/luarocks/cmd/make.lua
index 4d81386..025ac11 100644
--- a/src/luarocks/cmd/make.lua
+++ b/src/luarocks/cmd/make.lua
@@ -59,6 +59,8 @@ only dependencies of the rockspec (see `luarocks help install`).
 --sign              To be used with --pack-binary-rock. Also produce
                     a signature file for the generated .rock file.
 
+--chdir=<path>      Specify a source directory of the rock.
+
 ]]
 
 --- Driver function for "make" command.
@@ -79,6 +81,13 @@ function make.command(flags, rockspec_filename)
       return nil, "Invalid argument: 'make' takes a rockspec as a parameter. "..util.see_help("make")
    end
    
+   if flags["chdir"] then
+      local ok, err = fs.change_dir(flags["chdir"])
+      if not ok then
+         return nil, err
+      end
+   end
+
    local rockspec, err, errcode = fetch.load_rockspec(rockspec_filename)
    if not rockspec then
       return nil, err
diff --git a/src/luarocks/util.lua b/src/luarocks/util.lua
index abf6d90..8ccda27 100644
--- a/src/luarocks/util.lua
+++ b/src/luarocks/util.lua
@@ -92,6 +92,7 @@ local supported_flags = {
    ["binary"] = true,
    ["branch"] = "<branch-name>",
    ["build-deps"] = true,
+   ["chdir"] = "<path>",
    ["debug"] = true,
    ["deps"] = true,
    ["deps-mode"] = "<mode>",
-- 
2.7.4

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Tarantool-patches] [PATCH 2/3] Add a black list of the tarantoolctl options
  2020-03-25 21:50 [Tarantool-patches] [PATCH 0/3] Adapt luarocks for tarantoolctl Leonid Vasiliev
  2020-03-25 21:50 ` [Tarantool-patches] [PATCH 1/3] Add the chdir option for make Leonid Vasiliev
@ 2020-03-25 21:50 ` Leonid Vasiliev
  2020-04-04 19:02   ` Vladislav Shpilevoy
  2020-03-25 21:50 ` [Tarantool-patches] [PATCH 3/3] Adapt luarocks help for the tarantoolctl Leonid Vasiliev
  2 siblings, 1 reply; 13+ messages in thread
From: Leonid Vasiliev @ 2020-03-25 21:50 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: tarantool-patches

Luarocks code style has been used
---
 src/luarocks/util.lua | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/src/luarocks/util.lua b/src/luarocks/util.lua
index 8ccda27..0172d08 100644
--- a/src/luarocks/util.lua
+++ b/src/luarocks/util.lua
@@ -171,6 +171,14 @@ local supported_flags = {
    ["version"] = true,
 }
 
+-- The tarantool unsupported arguments list.
+local tarantool_black_list = {
+   ["global"] = true,
+   ["local"] = true,
+   ["lua-version"] = true,
+   ["lua-dir"] = true,
+}
+
 --- Extract flags from an arguments list.
 -- Given string arguments, extract flag arguments into a flags set.
 -- For example, given "foo", "--tux=beep", "--bla", "bar", "--baz",
@@ -189,6 +197,9 @@ function util.parse_flags(...)
       elseif state == "initial" and flag then
          local var,val = flag:match("([a-z_%-]*)=(.*)")
          if val then
+            if tarantool_black_list[var] then
+               return { ERROR = "Invalid argument: flag --"..var.." is not supported by tarantoolctl roks." }
+            end
             local vartype = supported_flags[var]
             if type(vartype) == "string" then
                if val == "" and vartype:sub(1,1) ~= '"' then
@@ -204,6 +215,9 @@ function util.parse_flags(...)
             end
          else
             local var = flag
+            if tarantool_black_list[var] then
+               return { ERROR = "Invalid argument: flag --"..var.." is not supported by tarantoolctl roks." }
+            end
             local vartype = supported_flags[var]
             if type(vartype) == "string" then
                i = i + 1
-- 
2.7.4

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Tarantool-patches] [PATCH 3/3] Adapt luarocks help for the tarantoolctl
  2020-03-25 21:50 [Tarantool-patches] [PATCH 0/3] Adapt luarocks for tarantoolctl Leonid Vasiliev
  2020-03-25 21:50 ` [Tarantool-patches] [PATCH 1/3] Add the chdir option for make Leonid Vasiliev
  2020-03-25 21:50 ` [Tarantool-patches] [PATCH 2/3] Add a black list of the tarantoolctl options Leonid Vasiliev
@ 2020-03-25 21:50 ` Leonid Vasiliev
  2020-04-04 19:02   ` Vladislav Shpilevoy
  2 siblings, 1 reply; 13+ messages in thread
From: Leonid Vasiliev @ 2020-03-25 21:50 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: tarantool-patches

---
 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"
 
 help.help_summary = "Help on commands. Type '"..program.." help <command>' for more."
 
@@ -20,7 +20,8 @@ help.help = [[
 ]]
 
 local function print_banner()
-   util.printout("\nLuaRocks "..cfg.program_version..", the Lua package manager")
+   util.printout("\nTarantoolctl rocks, the Lua package manager"
+                 .. " based on LuaRocks " .. cfg.program_version)
 end
 
 local function print_section(section)
@@ -62,12 +63,7 @@ function help.command(description, commands, command)
 	                       (overrides any entries in the config file)
 	--only-sources=<url>   Restrict downloads to paths matching the
 	                       given URL.
-        --lua-dir=<prefix>     Which Lua installation to use.
-	--lua-version=<ver>    Which Lua version to use.
 	--tree=<tree>          Which tree to operate on.
-	--local                Use the tree in the user's home directory.
-	                       To enable it, see ']]..program..[[ help path'.
-	--global               Use the system tree when `local_by_default` is `true`.
 	--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
-- 
2.7.4

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Tarantool-patches] [PATCH 1/3] Add the chdir option for make
  2020-03-25 21:50 ` [Tarantool-patches] [PATCH 1/3] Add the chdir option for make Leonid Vasiliev
@ 2020-04-02  9:58   ` lvasiliev
  2020-04-04 19:02   ` Vladislav Shpilevoy
  1 sibling, 0 replies; 13+ messages in thread
From: lvasiliev @ 2020-04-02  9:58 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: tarantool-patches

diff --git a/src/luarocks/cmd/make.lua b/src/luarocks/cmd/make.lua
index dc70d49..660c3fe 100644
--- a/src/luarocks/cmd/make.lua
+++ b/src/luarocks/cmd/make.lua
@@ -70,6 +70,13 @@ only dependencies of the rockspec (see `tarantoolctl 
rocks help install`).
  function make.command(flags, rockspec_filename)
     assert(type(rockspec_filename) == "string" or not rockspec_filename)

+   if flags["chdir"] then
+      local ok, err = fs.change_dir(flags["chdir"])
+      if not ok then
+         return nil, err
+      end
+   end
+
     if not rockspec_filename then
        local err
        rockspec_filename, err = util.get_default_rockspec()
@@ -80,13 +87,6 @@ function make.command(flags, rockspec_filename)
     if not rockspec_filename:match("rockspec$") then
        return nil, "Invalid argument: 'make' takes a rockspec as a 
parameter. "..util.see_help("make")
     end
-
-   if flags["chdir"] then
-      local ok, err = fs.change_dir(flags["chdir"])
-      if not ok then
-         return nil, err
-      end
-   end

     local rockspec, err, errcode = fetch.load_rockspec(rockspec_filename)
     if not rockspec then

On 26.03.2020 0:50, Leonid Vasiliev wrote:
> ---
>   src/luarocks/cmd/make.lua | 9 +++++++++
>   src/luarocks/util.lua     | 1 +
>   2 files changed, 10 insertions(+)
> 
> diff --git a/src/luarocks/cmd/make.lua b/src/luarocks/cmd/make.lua
> index 4d81386..025ac11 100644
> --- a/src/luarocks/cmd/make.lua
> +++ b/src/luarocks/cmd/make.lua
> @@ -59,6 +59,8 @@ only dependencies of the rockspec (see `luarocks help install`).
>   --sign              To be used with --pack-binary-rock. Also produce
>                       a signature file for the generated .rock file.
>   
> +--chdir=<path>      Specify a source directory of the rock.
> +
>   ]]
>   
>   --- Driver function for "make" command.
> @@ -79,6 +81,13 @@ function make.command(flags, rockspec_filename)
>         return nil, "Invalid argument: 'make' takes a rockspec as a parameter. "..util.see_help("make")
>      end
>      
> +   if flags["chdir"] then
> +      local ok, err = fs.change_dir(flags["chdir"])
> +      if not ok then
> +         return nil, err
> +      end
> +   end
> +
>      local rockspec, err, errcode = fetch.load_rockspec(rockspec_filename)
>      if not rockspec then
>         return nil, err
> diff --git a/src/luarocks/util.lua b/src/luarocks/util.lua
> index abf6d90..8ccda27 100644
> --- a/src/luarocks/util.lua
> +++ b/src/luarocks/util.lua
> @@ -92,6 +92,7 @@ local supported_flags = {
>      ["binary"] = true,
>      ["branch"] = "<branch-name>",
>      ["build-deps"] = true,
> +   ["chdir"] = "<path>",
>      ["debug"] = true,
>      ["deps"] = true,
>      ["deps-mode"] = "<mode>",
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Tarantool-patches] [PATCH 3/3] Adapt luarocks help for the tarantoolctl
  2020-03-25 21:50 ` [Tarantool-patches] [PATCH 3/3] Adapt luarocks help for the tarantoolctl Leonid Vasiliev
@ 2020-04-04 19:02   ` Vladislav Shpilevoy
  2020-04-05 15:15     ` lvasiliev
  0 siblings, 1 reply; 13+ messages in thread
From: Vladislav Shpilevoy @ 2020-04-04 19:02 UTC (permalink / raw)
  To: Leonid Vasiliev; +Cc: tarantool-patches

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.

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'?

Why does this util.this_program() even take any parameters? Can
it just return 'tarantoolctl' constantly?

>  help.help_summary = "Help on commands. Type '"..program.." help <command>' for more."
>  
> @@ -20,7 +20,8 @@ help.help = [[
>  ]]
>  
>  local function print_banner()
> -   util.printout("\nLuaRocks "..cfg.program_version..", the Lua package manager")
> +   util.printout("\nTarantoolctl rocks, the Lua package manager"
> +                 .. " based on LuaRocks " .. cfg.program_version)
>  end
>  
>  local function print_section(section)
> @@ -62,12 +63,7 @@ function help.command(description, commands, command)
>  	                       (overrides any entries in the config file)
>  	--only-sources=<url>   Restrict downloads to paths matching the
>  	                       given URL.
> -        --lua-dir=<prefix>     Which Lua installation to use.
> -	--lua-version=<ver>    Which Lua version to use.
>  	--tree=<tree>          Which tree to operate on.
> -	--local                Use the tree in the user's home directory.
> -	                       To enable it, see ']]..program..[[ help path'.
> -	--global               Use the system tree when `local_by_default` is `true`.

2. I am sure these removals should happen in the previous commit.

>  	--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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Tarantool-patches] [PATCH 2/3] Add a black list of the tarantoolctl options
  2020-03-25 21:50 ` [Tarantool-patches] [PATCH 2/3] Add a black list of the tarantoolctl options Leonid Vasiliev
@ 2020-04-04 19:02   ` Vladislav Shpilevoy
  2020-04-05 15:11     ` lvasiliev
  0 siblings, 1 reply; 13+ messages in thread
From: Vladislav Shpilevoy @ 2020-04-04 19:02 UTC (permalink / raw)
  To: Leonid Vasiliev; +Cc: tarantool-patches

Thanks for the patch!

See 3 comments below.

On 25/03/2020 22:50, Leonid Vasiliev wrote:
> Luarocks code style has been used

1. This message does not give any useful info. Better explain, why
did you add the list to the luarocks instead of to tarantoolctl.

> ---
>  src/luarocks/util.lua | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/src/luarocks/util.lua b/src/luarocks/util.lua
> index 8ccda27..0172d08 100644
> --- a/src/luarocks/util.lua
> +++ b/src/luarocks/util.lua
> @@ -171,6 +171,14 @@ local supported_flags = {
>     ["version"] = true,
>  }
>  
> +-- The tarantool unsupported arguments list.
> +local tarantool_black_list = {
> +   ["global"] = true,
> +   ["local"] = true,
> +   ["lua-version"] = true,
> +   ["lua-dir"] = true,

2. Why can't you just remove these flags from 'supported_flags'?
Or make them false there?

> +}
> +
>  --- Extract flags from an arguments list.
>  -- Given string arguments, extract flag arguments into a flags set.
>  -- For example, given "foo", "--tux=beep", "--bla", "bar", "--baz",
> @@ -189,6 +197,9 @@ function util.parse_flags(...)
>        elseif state == "initial" and flag then
>           local var,val = flag:match("([a-z_%-]*)=(.*)")
>           if val then
> +            if tarantool_black_list[var] then
> +               return { ERROR = "Invalid argument: flag --"..var.." is not supported by tarantoolctl roks." }

3. Probably 'rocks' instead of 'roks'? The same below.

> +            end
>              local vartype = supported_flags[var]
>              if type(vartype) == "string" then
>                 if val == "" and vartype:sub(1,1) ~= '"' then
> @@ -204,6 +215,9 @@ function util.parse_flags(...)
>              end
>           else
>              local var = flag
> +            if tarantool_black_list[var] then
> +               return { ERROR = "Invalid argument: flag --"..var.." is not supported by tarantoolctl roks." }
> +            end
>              local vartype = supported_flags[var]
>              if type(vartype) == "string" then
>                 i = i + 1
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Tarantool-patches] [PATCH 1/3] Add the chdir option for make
  2020-03-25 21:50 ` [Tarantool-patches] [PATCH 1/3] Add the chdir option for make Leonid Vasiliev
  2020-04-02  9:58   ` lvasiliev
@ 2020-04-04 19:02   ` Vladislav Shpilevoy
  2020-04-05 15:03     ` lvasiliev
  1 sibling, 1 reply; 13+ messages in thread
From: Vladislav Shpilevoy @ 2020-04-04 19:02 UTC (permalink / raw)
  To: Leonid Vasiliev; +Cc: tarantool-patches

Hi! Thanks for the patch!

Looks like you sent not the latest version. Or you didn't push
the latest version. Because version on the branch is different.

Please, sync the email thread and the github branch.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Tarantool-patches] [PATCH 1/3] Add the chdir option for make
  2020-04-04 19:02   ` Vladislav Shpilevoy
@ 2020-04-05 15:03     ` lvasiliev
  0 siblings, 0 replies; 13+ messages in thread
From: lvasiliev @ 2020-04-05 15:03 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

Hi! Thanks for the review. See PATCH v2.

On 04.04.2020 22:02, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
> 
> Looks like you sent not the latest version. Or you didn't push
> the latest version. Because version on the branch is different.
> 
> Please, sync the email thread and the github branch.
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Tarantool-patches] [PATCH 2/3] Add a black list of the tarantoolctl options
  2020-04-04 19:02   ` Vladislav Shpilevoy
@ 2020-04-05 15:11     ` lvasiliev
  2020-04-05 18:39       ` Vladislav Shpilevoy
  0 siblings, 1 reply; 13+ messages in thread
From: lvasiliev @ 2020-04-05 15:11 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

Hi! Thanks for the review. See PATCH v2 and comments bellow.

On 04.04.2020 22:02, Vladislav Shpilevoy wrote:
> Thanks for the patch!
> 
> See 3 comments below.
> 
> On 25/03/2020 22:50, Leonid Vasiliev wrote:
>> Luarocks code style has been used
> 
> 1. This message does not give any useful info. Better explain, why
> did you add the list to the luarocks instead of to tarantoolctl.
It's about changes in tarantool and luaroks code style)
About black list.
The main goal of the task:
"The point of the task is refuse from double parsing of luarocks options 
(first in tarantoolctl and second in luarocks module), cleanup help and 
replace a white-list filter (most options are prohibited) to a 
black-list filter (most options allowed). So, if the functionality is 
work at luarocks, it must work at tarantoolctl rocks too"

We don't want double parsing. And current parsing in tarantoolctl 
doesn't take into account matching commands with options.
> 
>> ---
>>   src/luarocks/util.lua | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/src/luarocks/util.lua b/src/luarocks/util.lua
>> index 8ccda27..0172d08 100644
>> --- a/src/luarocks/util.lua
>> +++ b/src/luarocks/util.lua
>> @@ -171,6 +171,14 @@ local supported_flags = {
>>      ["version"] = true,
>>   }
>>   
>> +-- The tarantool unsupported arguments list.
>> +local tarantool_black_list = {
>> +   ["global"] = true,
>> +   ["local"] = true,
>> +   ["lua-version"] = true,
>> +   ["lua-dir"] = true,
> 
> 2. Why can't you just remove these flags from 'supported_flags'?
> Or make them false there?
Because the black-filter is more clearly. Easy to see the tarantool 
exceptions.
> 
>> +}
>> +
>>   --- Extract flags from an arguments list.
>>   -- Given string arguments, extract flag arguments into a flags set.
>>   -- For example, given "foo", "--tux=beep", "--bla", "bar", "--baz",
>> @@ -189,6 +197,9 @@ function util.parse_flags(...)
>>         elseif state == "initial" and flag then
>>            local var,val = flag:match("([a-z_%-]*)=(.*)")
>>            if val then
>> +            if tarantool_black_list[var] then
>> +               return { ERROR = "Invalid argument: flag --"..var.." is not supported by tarantoolctl roks." }
> 
> 3. Probably 'rocks' instead of 'roks'? The same below.
Fixed
> 
>> +            end
>>               local vartype = supported_flags[var]
>>               if type(vartype) == "string" then
>>                  if val == "" and vartype:sub(1,1) ~= '"' then
>> @@ -204,6 +215,9 @@ function util.parse_flags(...)
>>               end
>>            else
>>               local var = flag
>> +            if tarantool_black_list[var] then
>> +               return { ERROR = "Invalid argument: flag --"..var.." is not supported by tarantoolctl roks." }
>> +            end
>>               local vartype = supported_flags[var]
>>               if type(vartype) == "string" then
>>                  i = i + 1
>>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Tarantool-patches] [PATCH 3/3] Adapt luarocks help for the tarantoolctl
  2020-04-04 19:02   ` Vladislav Shpilevoy
@ 2020-04-05 15:15     ` lvasiliev
  2020-04-05 18:39       ` Vladislav Shpilevoy
  0 siblings, 1 reply; 13+ messages in thread
From: lvasiliev @ 2020-04-05 15:15 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

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). 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.
> 
> 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.
> 
> 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.
> 
>>   help.help_summary = "Help on commands. Type '"..program.." help <command>' for more."
>>   
>> @@ -20,7 +20,8 @@ help.help = [[
>>   ]]
>>   
>>   local function print_banner()
>> -   util.printout("\nLuaRocks "..cfg.program_version..", the Lua package manager")
>> +   util.printout("\nTarantoolctl rocks, the Lua package manager"
>> +                 .. " based on LuaRocks " .. cfg.program_version)
>>   end
>>   
>>   local function print_section(section)
>> @@ -62,12 +63,7 @@ function help.command(description, commands, command)
>>   	                       (overrides any entries in the config file)
>>   	--only-sources=<url>   Restrict downloads to paths matching the
>>   	                       given URL.
>> -        --lua-dir=<prefix>     Which Lua installation to use.
>> -	--lua-version=<ver>    Which Lua version to use.
>>   	--tree=<tree>          Which tree to operate on.
>> -	--local                Use the tree in the user's home directory.
>> -	                       To enable it, see ']]..program..[[ help path'.
>> -	--global               Use the system tree when `local_by_default` is `true`.
> 
> 2. I am sure these removals should happen in the previous commit.
Moved
> 
>>   	--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.
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Tarantool-patches] [PATCH 3/3] Adapt luarocks help for the tarantoolctl
  2020-04-05 15:15     ` lvasiliev
@ 2020-04-05 18:39       ` Vladislav Shpilevoy
  0 siblings, 0 replies; 13+ messages in thread
From: Vladislav Shpilevoy @ 2020-04-05 18:39 UTC (permalink / raw)
  To: lvasiliev; +Cc: tarantool-patches

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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Tarantool-patches] [PATCH 2/3] Add a black list of the tarantoolctl options
  2020-04-05 15:11     ` lvasiliev
@ 2020-04-05 18:39       ` Vladislav Shpilevoy
  0 siblings, 0 replies; 13+ messages in thread
From: Vladislav Shpilevoy @ 2020-04-05 18:39 UTC (permalink / raw)
  To: lvasiliev; +Cc: tarantool-patches

On 05/04/2020 17:11, 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!
>>
>> See 3 comments below.
>>
>> On 25/03/2020 22:50, Leonid Vasiliev wrote:
>>> Luarocks code style has been used
>>
>> 1. This message does not give any useful info. Better explain, why
>> did you add the list to the luarocks instead of to tarantoolctl.
> It's about changes in tarantool and luaroks code style)

But it is not. You literally made functional changes here.
Code style is indentation level, naming policy and so on. This
patch banned some options. It is not a code style change.

> About black list.
> The main goal of the task:
> "The point of the task is refuse from double parsing of luarocks options (first in tarantoolctl and second in luarocks module), cleanup help and replace a white-list filter (most options are prohibited) to a black-list filter (most options allowed). So, if the functionality is work at luarocks, it must work at tarantoolctl rocks too"

Please, write something about that in the commit messages.
Currently the commit titles are close to useless since they
don't explain the changes. They just narrate the patchset.
I can see what patchset is doing without these titles. But I
don't see why. I don't see the whole picture. I cite these
messages here:

    Add the chdir option for make

    Add a black list for tarantoolctl rocks options

    Adapt luarocks help for tarantoolctl rocks

From these messages it is impossible to understand anything.
They seems like small refactoring changes from these titles
even though they are not. And the ticket is actually big, has
some discussion. Neither of this is reflected here.

> We don't want double parsing. And current parsing in tarantoolctl doesn't take into account matching commands with options.

This has nothing to do with double parsing. This is only
about functionality. By eliminating parsing from tarantoolctl
you will save a few microseconds in a tool, whose typical
operation lasts seconds.

But ok, I understood now. You don't want to complicate
tarantoolctl with checking options of the luarocks submodule.
It that case the blacklist checking should be in luarocks indeed.

This is also not reflected in the commit message.

>>> ---
>>>   src/luarocks/util.lua | 14 ++++++++++++++
>>>   1 file changed, 14 insertions(+)
>>>
>>> diff --git a/src/luarocks/util.lua b/src/luarocks/util.lua
>>> index 8ccda27..0172d08 100644
>>> --- a/src/luarocks/util.lua
>>> +++ b/src/luarocks/util.lua
>>> @@ -171,6 +171,14 @@ local supported_flags = {
>>>      ["version"] = true,
>>>   }
>>>   +-- The tarantool unsupported arguments list.
>>> +local tarantool_black_list = {
>>> +   ["global"] = true,
>>> +   ["local"] = true,
>>> +   ["lua-version"] = true,
>>> +   ["lua-dir"] = true,
>>
>> 2. Why can't you just remove these flags from 'supported_flags'?
>> Or make them false there?
> Because the black-filter is more clearly. Easy to see the tarantool exceptions.

They are not 'tarantool' exceptions, from what I see. They are
'taranctoolctl rocks' exceptions. It is ok to depend on tarantool. But
not ok to depend on 'tarantoolctl'. Because this is a cyclic dependency.

These options either should be disabled on the whole, or as a last
resort a blacklist can be passed in cfg.init() by whoever invokes
luarocks tool. In our case - by tarantoolctl, here:

    local cfg = require("luarocks.core.cfg")
    cfg.init({option_blacklist = {...}})

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-04-05 18:40 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-25 21:50 [Tarantool-patches] [PATCH 0/3] Adapt luarocks for tarantoolctl Leonid Vasiliev
2020-03-25 21:50 ` [Tarantool-patches] [PATCH 1/3] Add the chdir option for make Leonid Vasiliev
2020-04-02  9:58   ` lvasiliev
2020-04-04 19:02   ` Vladislav Shpilevoy
2020-04-05 15:03     ` lvasiliev
2020-03-25 21:50 ` [Tarantool-patches] [PATCH 2/3] Add a black list of the tarantoolctl options Leonid Vasiliev
2020-04-04 19:02   ` Vladislav Shpilevoy
2020-04-05 15:11     ` lvasiliev
2020-04-05 18:39       ` Vladislav Shpilevoy
2020-03-25 21:50 ` [Tarantool-patches] [PATCH 3/3] Adapt luarocks help for the tarantoolctl Leonid Vasiliev
2020-04-04 19:02   ` Vladislav Shpilevoy
2020-04-05 15:15     ` lvasiliev
2020-04-05 18:39       ` Vladislav Shpilevoy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox