From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp62.i.mail.ru (smtp62.i.mail.ru [217.69.128.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id BE6B7469719 for ; Thu, 19 Mar 2020 01:12:51 +0300 (MSK) References: <67564a04d3afd0ca4a9f9b0bd8f8bdcf9595b377.1574153367.git.lvasiliev@tarantool.org> From: Vladislav Shpilevoy Message-ID: <26f90174-dca5-8d93-68f1-701a3c1fa796@tarantool.org> Date: Wed, 18 Mar 2020 23:12:50 +0100 MIME-Version: 1.0 In-Reply-To: <67564a04d3afd0ca4a9f9b0bd8f8bdcf9595b377.1574153367.git.lvasiliev@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH LUAROCKS v2 1/2] Add the chdir option for make List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Leonid , alexander.turenko@tarantool.org Cc: tarantool-patches@dev.tarantool.org Thanks for the patch! First of all, please, drop the unnecessary diff. 11 from 14 hunks of your commit are not needed, and their drop does not affect anything. Secondly, please, provide a descriptive commit message. Why changes were made, and how can I test them. I don't know anything about rocks and don't use them, so it would be cool if you could give me the exact commands I should try in my console to check how your patch works, and what is changed. Also your commit says "add chdir option". So it assumes the public behaviour is changed. Is it? If it is, then add a docbot request, if these options are documented anywhere. Otherwise the commit title is misleading. On 19/11/2019 09:52, Leonid wrote: > --- > src/luarocks/cmd/make.lua | 22 ++++++++++++++++------ > src/luarocks/util.lua | 11 ++++++----- > 2 files changed, 22 insertions(+), 11 deletions(-) > > diff --git a/src/luarocks/cmd/make.lua b/src/luarocks/cmd/make.lua > index 4d81386..4cd63f1 100644 > --- a/src/luarocks/cmd/make.lua > +++ b/src/luarocks/cmd/make.lua > @@ -1,7 +1,7 @@ > > --- Module implementing the LuaRocks "make" command. > -- Builds sources in the current directory, but unlike "build", > --- it does not fetch sources, etc., assuming everything is > +-- it does not fetch sources, etc., assuming everything is > -- available in the current directory. > local make = {} > > @@ -20,7 +20,7 @@ make.help_summary = "Compile package in current directory using a rockspec." > make.help_arguments = "[--pack-binary-rock] []" > make.help = [[ > Builds sources in the current directory, but unlike "build", > -it does not fetch sources, etc., assuming everything is > +it does not fetch sources, etc., assuming everything is > available in the current directory. If no argument is given, > it looks for a rockspec in the current directory and in "rockspec/" > and "rockspecs/" subdirectories, picking the rockspec with newest version > @@ -28,7 +28,7 @@ or without version name. If rockspecs for different rocks are found > or there are several rockspecs without version, you must specify which to use, > through the command-line. > > -This command is useful as a tool for debugging rockspecs. > +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. > > @@ -45,7 +45,7 @@ only dependencies of the rockspec (see `luarocks help install`). > in the configuration file. > > --branch= Override the `source.branch` field in the loaded > - rockspec. Allows to specify a different branch to > + rockspec. Allows to specify a different branch to > fetch. Particularly for "dev" rocks. > > --verify Verify signature of the rockspec or src.rock being > @@ -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= Specify a source directory of the rock. > + > ]] > > --- Driver function for "make" command. > @@ -67,7 +69,7 @@ only dependencies of the rockspec (see `luarocks help install`). > -- error message otherwise. exitcode is optionally returned. > function make.command(flags, rockspec_filename) > assert(type(rockspec_filename) == "string" or not rockspec_filename) > - > + > if not rockspec_filename then > local err > rockspec_filename, err = util.get_default_rockspec() > @@ -78,7 +80,14 @@ 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 > return nil, err > @@ -96,6 +105,7 @@ function make.command(flags, rockspec_filename) > verify = not not flags["verify"], > }) > > + > if flags["sign"] and not flags["pack-binary-rock"] then > return nil, "In the make command, --sign is meant to be used only with --pack-binary-rock" > end > diff --git a/src/luarocks/util.lua b/src/luarocks/util.lua > index abf6d90..fadc93f 100644 > --- a/src/luarocks/util.lua > +++ b/src/luarocks/util.lua > @@ -33,7 +33,7 @@ local debug = require("debug") > -- which can be used to remove the item later from the list. > function util.schedule_function(f, ...) > assert(type(f) == "function") > - > + > local item = { fn = f, args = {...} } > table.insert(scheduled_functions, item) > return item > @@ -92,6 +92,7 @@ local supported_flags = { > ["binary"] = true, > ["branch"] = "", > ["build-deps"] = true, > + ["chdir"] = "", > ["debug"] = true, > ["deps"] = true, > ["deps-mode"] = "", > @@ -276,12 +277,12 @@ end > -- exists in vars. Only string values are processed; this function > -- does not scan subtables recursively. > -- @param tbl table: Table to have its string values modified. > --- @param vars table: Table containing string-string key-value pairs > +-- @param vars table: Table containing string-string key-value pairs > -- representing variables to replace in the strings values of tbl. > function util.variable_substitutions(tbl, vars) > assert(type(tbl) == "table") > assert(type(vars) == "table") > - > + > local updated = {} > for k, v in pairs(tbl) do > if type(v) == "string" then > @@ -698,9 +699,9 @@ end > > function util.opts_table(type_name, valid_opts) > local opts_mt = {} > - > + > opts_mt.__index = opts_mt > - > + > function opts_mt.type() > return type_name > end >