[Tarantool-patches] [PATCH v3 0/2] Adapt luarocks for tarantoolctl

[Tarantool-patches] [PATCH v3 0/2] Adapt luarocks for tarantoolctl

[Leonid Vasiliev]

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)

Was done:
whitelist of tarantoolctl don't used for luarocks flags
Option chdir has been moved from tarantoolctl to luarocks

@ChangeLog - see a comment in tarantool

Leonid Vasiliev (2):
  Add the chdir option for make
  Delete flags which can't be used with tarantoolctl rocks

 src/luarocks/cmd/help.lua | 5 -----
 src/luarocks/cmd/make.lua | 8 ++++++++
 src/luarocks/util.lua     | 5 +----
 3 files changed, 9 insertions(+), 9 deletions(-)

-- 
2.7.4

[Tarantool-patches] [PATCH v3 1/2] Add the chdir option for make

[Leonid Vasiliev]

Flag --chdir for make command (with help) has been added.
It's add possibility to specify a source directory of the rock when make.
---
 src/luarocks/cmd/make.lua | 8 ++++++++
 src/luarocks/util.lua     | 1 +
 2 files changed, 9 insertions(+)

diff --git a/src/luarocks/cmd/make.lua b/src/luarocks/cmd/make.lua
index 4d81386..015b01d 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.
@@ -68,6 +70,12 @@ only dependencies of the rockspec (see `luarocks 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()
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

[Tarantool-patches] [PATCH v3 2/2] Delete flags which can't be used with tarantoolctl rocks

[Leonid Vasiliev]

---
Added changes according to https://lists.tarantool.org/pipermail/tarantool-patches/2020-April/015459.html
I discussed the transfer of the blacklist to Luaroсks with
Alexander Turenko at the beginning of the assignment and
was decided that it's overkill since we already have patches specific for tarantool.

 src/luarocks/cmd/help.lua | 5 -----
 src/luarocks/util.lua     | 4 ----
 2 files changed, 9 deletions(-)

diff --git a/src/luarocks/cmd/help.lua b/src/luarocks/cmd/help.lua
index dcc9e35..202fe43 100644
--- a/src/luarocks/cmd/help.lua
+++ b/src/luarocks/cmd/help.lua
@@ -62,12 +62,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/util.lua b/src/luarocks/util.lua
index 8ccda27..96d4536 100644
--- a/src/luarocks/util.lua
+++ b/src/luarocks/util.lua
@@ -101,7 +101,6 @@ local supported_flags = {
    ["force"] = true,
    ["force-fast"] = true,
    ["from"] = "<server>",
-   ["global"] = true,
    ["help"] = true,
    ["home"] = true,
    ["homepage"] = "\"<url>\"",
@@ -113,14 +112,11 @@ local supported_flags = {
    ["lib"] = "<library>",
    ["license"] = "\"<text>\"",
    ["list"] = true,
-   ["local"] = true,
    ["local-tree"] = true,
    ["lr-bin"] = true,
    ["lr-cpath"] = true,
    ["lr-path"] = true,
-   ["lua-dir"] = "<path>",
    ["lua-version"] = "<vers>",
-   ["lua-versions"] = "<versions>",
    ["lua-ver"] = true,
    ["lua-incdir"] = true,
    ["lua-libdir"] = true,
-- 
2.7.4

Re: [Tarantool-patches] [PATCH v3 2/2] Delete flags which can't be used with tarantoolctl rocks

[lvasiliev]

Has been discarded from patchset.

On 09.04.2020 9:33, Leonid Vasiliev wrote:
> ---
> Added changes according to https://lists.tarantool.org/pipermail/tarantool-patches/2020-April/015459.html
> I discussed the transfer of the blacklist to Luaroсks with
> Alexander Turenko at the beginning of the assignment and
> was decided that it's overkill since we already have patches specific for tarantool.
> 
>   src/luarocks/cmd/help.lua | 5 -----
>   src/luarocks/util.lua     | 4 ----
>   2 files changed, 9 deletions(-)
> 
> diff --git a/src/luarocks/cmd/help.lua b/src/luarocks/cmd/help.lua
> index dcc9e35..202fe43 100644
> --- a/src/luarocks/cmd/help.lua
> +++ b/src/luarocks/cmd/help.lua
> @@ -62,12 +62,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/util.lua b/src/luarocks/util.lua
> index 8ccda27..96d4536 100644
> --- a/src/luarocks/util.lua
> +++ b/src/luarocks/util.lua
> @@ -101,7 +101,6 @@ local supported_flags = {
>      ["force"] = true,
>      ["force-fast"] = true,
>      ["from"] = "<server>",
> -   ["global"] = true,
>      ["help"] = true,
>      ["home"] = true,
>      ["homepage"] = "\"<url>\"",
> @@ -113,14 +112,11 @@ local supported_flags = {
>      ["lib"] = "<library>",
>      ["license"] = "\"<text>\"",
>      ["list"] = true,
> -   ["local"] = true,
>      ["local-tree"] = true,
>      ["lr-bin"] = true,
>      ["lr-cpath"] = true,
>      ["lr-path"] = true,
> -   ["lua-dir"] = "<path>",
>      ["lua-version"] = "<vers>",
> -   ["lua-versions"] = "<versions>",
>      ["lua-ver"] = true,
>      ["lua-incdir"] = true,
>      ["lua-libdir"] = true,
> 

Re: [Tarantool-patches] [PATCH v3 2/2] Delete flags which can't be used with tarantoolctl rocks

[Yaroslav Dynnikov]

[-- Attachment #1: Type: text/plain, Size: 2799 bytes --]

Hi, Leonid.

LGTM

On Mon, 13 Apr 2020 at 14:59, lvasiliev <lvasiliev@tarantool.org> wrote:

> Has been discarded from patchset.
>
> On 09.04.2020 9:33, Leonid Vasiliev wrote:
> > ---
> > Added changes according to
> https://lists.tarantool.org/pipermail/tarantool-patches/2020-April/015459.html
> > I discussed the transfer of the blacklist to Luaroсks with
> > Alexander Turenko at the beginning of the assignment and
> > was decided that it's overkill since we already have patches specific
> for tarantool.
> >
> >   src/luarocks/cmd/help.lua | 5 -----
> >   src/luarocks/util.lua     | 4 ----
> >   2 files changed, 9 deletions(-)
> >
> > diff --git a/src/luarocks/cmd/help.lua b/src/luarocks/cmd/help.lua
> > index dcc9e35..202fe43 100644
> > --- a/src/luarocks/cmd/help.lua
> > +++ b/src/luarocks/cmd/help.lua
> > @@ -62,12 +62,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/util.lua b/src/luarocks/util.lua
> > index 8ccda27..96d4536 100644
> > --- a/src/luarocks/util.lua
> > +++ b/src/luarocks/util.lua
> > @@ -101,7 +101,6 @@ local supported_flags = {
> >      ["force"] = true,
> >      ["force-fast"] = true,
> >      ["from"] = "<server>",
> > -   ["global"] = true,
> >      ["help"] = true,
> >      ["home"] = true,
> >      ["homepage"] = "\"<url>\"",
> > @@ -113,14 +112,11 @@ local supported_flags = {
> >      ["lib"] = "<library>",
> >      ["license"] = "\"<text>\"",
> >      ["list"] = true,
> > -   ["local"] = true,
> >      ["local-tree"] = true,
> >      ["lr-bin"] = true,
> >      ["lr-cpath"] = true,
> >      ["lr-path"] = true,
> > -   ["lua-dir"] = "<path>",
> >      ["lua-version"] = "<vers>",
> > -   ["lua-versions"] = "<versions>",
> >      ["lua-ver"] = true,
> >      ["lua-incdir"] = true,
> >      ["lua-libdir"] = true,
> >
>


-- 
С уважением.
Дынников Ярослав.

[-- Attachment #2: Type: text/html, Size: 4227 bytes --]

Re: [Tarantool-patches] [PATCH v3 0/2] Adapt luarocks for tarantoolctl

[Vladislav Shpilevoy]

Hi!

Thanks for the patchset!

LGTM.

But please, next time don't silently respond with a new patch
on my comments. I may be wrong sometimes. Honestly I am wrong
quite often. Like Vladimir said once, "mailing list exists for
discussions".

I added Yaroslav to CC since he expressed a will to join the
review.

On 09/04/2020 08:33, Leonid Vasiliev wrote:
> 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)
> 
> Was done:
> whitelist of tarantoolctl don't used for luarocks flags
> Option chdir has been moved from tarantoolctl to luarocks
> 
> @ChangeLog - see a comment in tarantool
> 
> Leonid Vasiliev (2):
>   Add the chdir option for make
>   Delete flags which can't be used with tarantoolctl rocks
> 
>  src/luarocks/cmd/help.lua | 5 -----
>  src/luarocks/cmd/make.lua | 8 ++++++++
>  src/luarocks/util.lua     | 5 +----
>  3 files changed, 9 insertions(+), 9 deletions(-)
> 

Re: [Tarantool-patches] [PATCH v3 0/2] Adapt luarocks for tarantoolctl

[lvasiliev]

Hi! Thanks for the reviews.

On 10.04.2020 3:00, Vladislav Shpilevoy wrote:
> Hi!
> 
> Thanks for the patchset!
> 
> LGTM.
> 
> But please, next time don't silently respond with a new patch
> on my comments. I may be wrong sometimes. Honestly I am wrong
> quite often. Like Vladimir said once, "mailing list exists for
> discussions".
> 
OK
> I added Yaroslav to CC since he expressed a will to join the
> review.
> 
> On 09/04/2020 08:33, Leonid Vasiliev wrote:
>> 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)
>>
>> Was done:
>> whitelist of tarantoolctl don't used for luarocks flags
>> Option chdir has been moved from tarantoolctl to luarocks
>>
>> @ChangeLog - see a comment in tarantool
>>
>> Leonid Vasiliev (2):
>>    Add the chdir option for make
>>    Delete flags which can't be used with tarantoolctl rocks
>>
>>   src/luarocks/cmd/help.lua | 5 -----
>>   src/luarocks/cmd/make.lua | 8 ++++++++
>>   src/luarocks/util.lua     | 5 +----
>>   3 files changed, 9 insertions(+), 9 deletions(-)
>>

Re: [Tarantool-patches] [PATCH v3 0/2] Adapt luarocks for tarantoolctl

[Yaroslav Dynnikov]

[-- Attachment #1: Type: text/plain, Size: 3166 bytes --]

Hi, Leonid.

The first patch ("Add the chdir option for make" 8e8db7f
<https://github.com/tarantool/luarocks/commit/8e8db7f255a855c585245c288414ceb097576e07>)
looks good to
me. I hope you'll be able to find time and propose it to upstream, in my
opinion it'll helpful there too.

As of the second one ("Delete flags which can't be used with
tarantoolctl rocks" 1b51b2f
<https://github.com/tarantool/luarocks/commit/1b51b2fe1c194719a7a8a74220a954a91c21f2a7>),
I'm worried that our fork every day goes
farther from upstream. You didn't say a word about necessity to *remove*
it, but it looks like another postponed problem. Luarocks version is
already 3.3.1, while we still basing 3.1.1. And every such commit makes
it harder to rebase.

The last time we were upgrading luarocks we've spent several weeks of
worktime to resolve all the problems. And, disappointingly, those
changes weren't merged to 1.10, so my colleagues usually come back with
the same problems as 2 years ago. Please, don't screw over our future
selves.

Speaking about the patch in tarantool itself ("rocks: forward options to
luarocks" 28a3b55f
<https://github.com/tarantool/tarantool/commit/28a3b55fbc94c10b284201955e8030de75e03f3f>),
I've run some commands and it seems to be OK.
But I agree with Vlad that documentation should be more detailed. You
can't just link the luarocks wiki (at first, because our version
differs, but, moreover, because it's imperfect too). I don't believe
anybody but you would be able to tell supported features from
unsupported and find mistakes in wiki, so I suggest to clarify it
somewhere (in the commit message or in any other place you like).

On Fri, 10 Apr 2020 at 03:00, Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
wrote:

> Hi!
>
> Thanks for the patchset!
>
> LGTM.
>
> But please, next time don't silently respond with a new patch
> on my comments. I may be wrong sometimes. Honestly I am wrong
> quite often. Like Vladimir said once, "mailing list exists for
> discussions".
>
> I added Yaroslav to CC since he expressed a will to join the
> review.
>
> On 09/04/2020 08:33, Leonid Vasiliev wrote:
> > 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)
> >
> > Was done:
> > whitelist of tarantoolctl don't used for luarocks flags
> > Option chdir has been moved from tarantoolctl to luarocks
> >
> > @ChangeLog - see a comment in tarantool
> >
> > Leonid Vasiliev (2):
> >   Add the chdir option for make
> >   Delete flags which can't be used with tarantoolctl rocks
> >
> >  src/luarocks/cmd/help.lua | 5 -----
> >  src/luarocks/cmd/make.lua | 8 ++++++++
> >  src/luarocks/util.lua     | 5 +----
> >  3 files changed, 9 insertions(+), 9 deletions(-)
> >
>


-- 
С уважением.
Дынников Ярослав.

[-- Attachment #2: Type: text/html, Size: 4398 bytes --]

Re: [Tarantool-patches] [PATCH v3 0/2] Adapt luarocks for tarantoolctl

[lvasiliev]

Hi!Thanks for the feedback.

On 10.04.2020 20:02, Yaroslav Dynnikov wrote:
> Hi, Leonid.
> 
> The first patch ("Add the chdir option for make" 8e8db7f 
> <https://github.com/tarantool/luarocks/commit/8e8db7f255a855c585245c288414ceb097576e07>) 
> looks good to
> me. I hope you'll be able to find time and propose it to upstream, in my
> opinion it'll helpful there too.
> 
> As of the second one ("Delete flags which can't be used with
> tarantoolctl rocks" 1b51b2f 
> <https://github.com/tarantool/luarocks/commit/1b51b2fe1c194719a7a8a74220a954a91c21f2a7>), 
> I'm worried that our fork every day goes
> farther from upstream. You didn't say a word about necessity to /remove/
> it, but it looks like another postponed problem. Luarocks version is
> already 3.3.1, while we still basing 3.1.1. And every such commit makes
> it harder to rebase. >
> The last time we were upgrading luarocks we've spent several weeks of
> worktime to resolve all the problems. And, disappointingly, those
> changes weren't merged to 1.10, so my colleagues usually come back with
> the same problems as 2 years ago. Please, don't screw over our future
> selves.
In my view, argument parsing is now implemented conceptually incorrectly 
because the LuaRoks flags are parsed first according to the tarantoolctl 
rules and then according to the LuaRocks rules, and these parsings have 
a mutual influence. When upgrading to a new version of LuaRocks (in case 
of changing the list of supported flags), you will must to analyze which 
flags to support, if the flags have not changed, there will be no 
conflicts. If you have two parsers, you will need to constantly maintain 
them in a consistent state and flags other commands will be affect to 
LuaRocks. So, in my horrible opinion if you have a conflict after rebase
here, you must resolve it at the rebase time.
> 
> Speaking about the patch in tarantool itself ("rocks: forward options to
> luarocks" 28a3b55f 
> <https://github.com/tarantool/tarantool/commit/28a3b55fbc94c10b284201955e8030de75e03f3f>), 
> I've run some commands and it seems to be OK.
> But I agree with Vlad that documentation should be more detailed. You
> can't just link the luarocks wiki (at first, because our version
> differs, but, moreover, because it's imperfect too). I don't believe
> anybody but you would be able to tell supported features from
> unsupported and find mistakes in wiki, so I suggest to clarify it
> somewhere (in the commit message or in any other place you like).
> 
I have no experience using LuaRocks. If you have an approximate list of 
differences that we want to add to documentation, I can apply it to 
default. But after all, commands and flags were added that we did not 
use, why their behavior is different from default?
> On Fri, 10 Apr 2020 at 03:00, Vladislav Shpilevoy 
> <v.shpilevoy@tarantool.org <mailto:v.shpilevoy@tarantool.org>> wrote:
> 
>     Hi!
> 
>     Thanks for the patchset!
> 
>     LGTM.
> 
>     But please, next time don't silently respond with a new patch
>     on my comments. I may be wrong sometimes. Honestly I am wrong
>     quite often. Like Vladimir said once, "mailing list exists for
>     discussions".
> 
>     I added Yaroslav to CC since he expressed a will to join the
>     review.
> 
>     On 09/04/2020 08:33, Leonid Vasiliev wrote:
>      > 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)
>      >
>      > Was done:
>      > whitelist of tarantoolctl don't used for luarocks flags
>      > Option chdir has been moved from tarantoolctl to luarocks
>      >
>      > @ChangeLog - see a comment in tarantool
>      >
>      > Leonid Vasiliev (2):
>      >   Add the chdir option for make
>      >   Delete flags which can't be used with tarantoolctl rocks
>      >
>      >  src/luarocks/cmd/help.lua | 5 -----
>      >  src/luarocks/cmd/make.lua | 8 ++++++++
>      >  src/luarocks/util.lua     | 5 +----
>      >  3 files changed, 9 insertions(+), 9 deletions(-)
>      >
> 
> 
> 
> -- 
> С уважением.
> Дынников Ярослав.

Re: [Tarantool-patches] [PATCH v3 0/2] Adapt luarocks for tarantoolctl

[Vladislav Shpilevoy]

> On 10/04/2020 19:02, Yaroslav Dynnikov wrote:
> Hi, Leonid.
> 
> The first patch ("Add the chdir option for make" 8e8db7f <https://github.com/tarantool/luarocks/commit/8e8db7f255a855c585245c288414ceb097576e07>) looks good to
> me. I hope you'll be able to find time and propose it to upstream, in my
> opinion it'll helpful there too.
> 
> As of the second one ("Delete flags which can't be used with
> tarantoolctl rocks" 1b51b2f <https://github.com/tarantool/luarocks/commit/1b51b2fe1c194719a7a8a74220a954a91c21f2a7>), I'm worried that our fork every day goes
> farther from upstream. You didn't say a word about necessity to /remove/
> it, but it looks like another postponed problem.

Leonid, btw, why do we remove them, again? I was always thinking it
is a part of the ticket, but I don't see it there except in your
comments.

> Luarocks version is
> already 3.3.1, while we still basing 3.1.1. And every such commit makes
> it harder to rebase.
> 
> The last time we were upgrading luarocks we've spent several weeks of
> worktime to resolve all the problems. And, disappointingly, those
> changes weren't merged to 1.10, so my colleagues usually come back with
> the same problems as 2 years ago. Please, don't screw over our future
> selves.

While we sure should be as close to the upstream as possible, I think
the rebase problem will appear always when you do any patches, and
rebase once per 100 years.

Re: [Tarantool-patches] [PATCH v3 0/2] Adapt luarocks for tarantoolctl

[lvasiliev]

On 11.04.2020 17:23, Vladislav Shpilevoy wrote:

>> As of the second one ("Delete flags which can't be used with
>> tarantoolctl rocks" 1b51b2f <https://github.com/tarantool/luarocks/commit/1b51b2fe1c194719a7a8a74220a954a91c21f2a7>), I'm worried that our fork every day goes
>> farther from upstream. You didn't say a word about necessity to /remove/
>> it, but it looks like another postponed problem.
> 
> Leonid, btw, why do we remove them, again? I was always thinking it
> is a part of the ticket, but I don't see it there except in your
> comments.
Because after talking with Yaroslav, we decided that the user should not 
be able to use these flags.
When I worked on task, I was still too young and did not know that all 
summaries of discussions should be written down in comments on task.

Re: [Tarantool-patches] [PATCH v3 0/2] Adapt luarocks for tarantoolctl

[Yaroslav Dynnikov]

[-- Attachment #1: Type: text/plain, Size: 3351 bytes --]

Hi, guys.

Recently we talked to Leonid in chat and discussed the necessity of
removing flags. I've offered Leonid to check behavior, so here is what
I've found.

My point of view (now and then) is that they're neither useful nor
harmful. Earlier I did agree to blacklist them, but looking at Leonids
patch set this week I've changed my mind as the second commit seems
just worthless to me.

Inline help says:

```
--lua-dir=<prefix>     Which Lua installation to use.
--lua-version=<ver>    Which Lua version to use.
--global               Use the system tree when `local_by_default` is
`true`.
--local                Use the tree in the user's home directory.
                       To enable it, see
'/opt/tarantool-install/bin/tarantoolctl help path'.

Rocks trees in use:
        /tmp/tmp.JY1fgy8La9/.rocks ("user")
        /opt/tarantool-install ("system")
```

* Tarantool does define `LOCAL_BY_DEFAULT = true`,
  (i.e. `--local` is our default behavior)

* "System tree" is the place where tarantool is installed
  (`-DCMAKE_INSTALL_PREFIX`).

* "Home directory" in tarantool is overriden with `./.rocks`.

* See "rocks: update luarocks to 3.1.1" 4222c1f6
<https://github.com/tarantool/tarantool/commit/4222c1f6451973f446f9c8be28c7012435416fa3>
.

* Both `--local` and `--global` are only shortcuts for predefined
  `--tree=<tree>`, which isn't going to be removed.

* Speaking of `--lua-dir` and `--lua-version`, they look to be neither
  useful nor harmful.

Please, notice that command-line help refers to `tarantoolctl help
path`, but

1. It should be `tarantoolctl rocks help` (I guess there are many of
   such places)
2. `rocks path` command is disabled in your patch.

I'm a lazy developer and I don't like unnecessary restrictions. If I
were you, I'd drop the second commit ("Delete flags which can't be used
with tarantoolctl rocks" 1b51b2f) and enable back `rocks path` command.
Let people decide whether they need it or not.

Finally, a few words about documentation. I've tried to write down all
supported commands and flags, and I found it's really difficult and
verbose. I'm withdrawing my suggestion to clarify it, I don't know what
to do. Perhaps, documentation should just refer to inline help, as it's the
only up-to-date information, and still correct enough.



On Sun, 12 Apr 2020 at 20:25, lvasiliev <lvasiliev@tarantool.org> wrote:

>
>
> On 11.04.2020 17:23, Vladislav Shpilevoy wrote:
>
> >> As of the second one ("Delete flags which can't be used with
> >> tarantoolctl rocks" 1b51b2f <
> https://github.com/tarantool/luarocks/commit/1b51b2fe1c194719a7a8a74220a954a91c21f2a7>),
> I'm worried that our fork every day goes
> >> farther from upstream. You didn't say a word about necessity to /remove/
> >> it, but it looks like another postponed problem.
> >
> > Leonid, btw, why do we remove them, again? I was always thinking it
> > is a part of the ticket, but I don't see it there except in your
> > comments.
> Because after talking with Yaroslav, we decided that the user should not
> be able to use these flags.
> When I worked on task, I was still too young and did not know that all
> summaries of discussions should be written down in comments on task.
>


-- 
С уважением.
Дынников Ярослав.

[-- Attachment #2: Type: text/html, Size: 4413 bytes --]

Re: [Tarantool-patches] [PATCH v3 0/2] Adapt luarocks for tarantoolctl

[Vladislav Shpilevoy]

> Please, notice that command-line help refers to `tarantoolctl help
> path`, but
> 
> 1. It should be `tarantoolctl rocks help` (I guess there are many of
>    such places)

Fix of 'help' sections appeared to be a task far from trivial. There
are tens of places, where 'luarocks' should be replaced with
'tarantoolctl rocks'. That would make the fork even more far from the
upstream, and would make luarocks not self-sufficient. Because now it
depends only on 'tarantool', but will depend on 'tarantoolctl' too.

See other emails on the 'help' subject in this thread. I don't know
what to do with that really. My best proposal was to pass an option to
luarocks init in tarantoolctl with a program name to use, and replace
it where necessary. But this is also very intrusive.

Re: [Tarantool-patches] [PATCH v3 0/2] Adapt luarocks for tarantoolctl

[lvasiliev]

Hi. Thanks you for the feedback!

On 12.04.2020 21:30, Yaroslav Dynnikov wrote:
> Hi, guys.
> 
> Recently we talked to Leonid in chat and discussed the necessity of
> removing flags. I've offered Leonid to check behavior, so here is what
> I've found.
> 
> My point of view (now and then) is that they're neither useful nor
> harmful. Earlier I did agree to blacklist them, but looking at Leonids
> patch set this week I've changed my mind as the second commit seems
> just worthless to me.
> 
> Inline help says:
> 
> ```
> --lua-dir=<prefix>     Which Lua installation to use.
> --lua-version=<ver>    Which Lua version to use.
> --global               Use the system tree when `local_by_default` is 
> `true`.
> --local                Use the tree in the user's home directory.
>                         To enable it, see 
> '/opt/tarantool-install/bin/tarantoolctl help path'.
> 
> Rocks trees in use:
>          /tmp/tmp.JY1fgy8La9/.rocks ("user")
>          /opt/tarantool-install ("system")
> ```
> 
> * Tarantool does define `LOCAL_BY_DEFAULT = true`,
>    (i.e. `--local` is our default behavior)
> 
> * "System tree" is the place where tarantool is installed
>    (`-DCMAKE_INSTALL_PREFIX`).
> 
> * "Home directory" in tarantool is overriden with `./.rocks`.
> 
> * See "rocks: update luarocks to 3.1.1" 4222c1f6 
> <https://github.com/tarantool/tarantool/commit/4222c1f6451973f446f9c8be28c7012435416fa3>.
> 
> * Both `--local` and `--global` are only shortcuts for predefined
>    `--tree=<tree>`, which isn't going to be removed.
> 
> * Speaking of `--lua-dir` and `--lua-version`, they look to be neither
>    useful nor harmful.
> 
Do I understand correctly that you propose to discard the commit with 
deleting flags?
> Please, notice that command-line help refers to `tarantoolctl help
> path`, but
> 
> 1. It should be `tarantoolctl rocks help` (I guess there are many of
>     such places)
It was commented by Vlad
> 2. `rocks path` command is disabled in your patch.
> 
Rocks path was disabled before my patch and it wasn't at the list (list 
from ticket https://github.com/tarantool/tarantool/issues/4629) of 
desired commands to include. Do you propose to add the "path" command?
> I'm a lazy developer and I don't like unnecessary restrictions. If I
> were you, I'd drop the second commit ("Delete flags which can't be used
> with tarantoolctl rocks" 1b51b2f) and enable back `rocks path` command.
> Let people decide whether they need it or not.
> 
> Finally, a few words about documentation. I've tried to write down all
> supported commands and flags, and I found it's really difficult and
> verbose. I'm withdrawing my suggestion to clarify it, I don't know what
> to do. Perhaps, documentation should just refer to inline help, as it's the
> only up-to-date information, and still correct enough.
> 

Ok

> Дынников Ярослав.

Re: [Tarantool-patches] [PATCH v3 0/2] Adapt luarocks for tarantoolctl

[lvasiliev]

Hi.
I had a conversation with Yaroslav and he agrees to set LGTM if the 
second patch will be removed from the patchset.
I don't mind.
Vlad, do you agree?