From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp5.mail.ru (smtp5.mail.ru [94.100.179.24]) (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 5166B445320 for ; Wed, 8 Jul 2020 12:12:47 +0300 (MSK) References: From: Leonid Vasiliev Message-ID: Date: Wed, 8 Jul 2020 12:12:45 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v1 1/1] luarock: change a way to create manifest List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: imeevma@tarantool.org, yaroslav.dynnikov@tarantool.org, tarantool-patches@dev.tarantool.org Hi! Thank you for the patch. It looks good, but I have some questions/comments. On 07.07.2020 09:45, imeevma@tarantool.org wrote: > The first luarock installed creates a repository manifest. > However, before this patch, the generated manifest was already > filled in according to the files in the repository. This results > in an error if luarock was installed from * .all.rock and had > dependencies. The problem was that when creating the manifest, > information about luarock was written to the manifest when > installing its dependencies. After that, since information about > luarock already appears after installing the dependencies, but > before installing luarock, luarock was not installed in the > default directory. It was installed in another directory. > > After this patch, an empty manifest will be created instead of the > already filled manifest. It will be gradually filled during > installation of the luarock. > > Closes tarantool/tarantool#4704 > --- > https://github.com/tarantool/tarantool/issues/4704 > https://github.com/tarantool/luarocks/tree/imeevma/gh-4704-fix-rock-name > > src/luarocks/manif.lua | 23 +++++++++++++++++++---- > 1 file changed, 19 insertions(+), 4 deletions(-) > > diff --git a/src/luarocks/manif.lua b/src/luarocks/manif.lua > index 34ae02d..2c54200 100644 > --- a/src/luarocks/manif.lua > +++ b/src/luarocks/manif.lua > @@ -423,6 +423,21 @@ function manif.make_manifest(repo, deps_mode, remote) > return save_table(repo, "manifest", manifest) > end > > +-- Create an empty manifest file. A file called 'manifest' will be Function descriptions begin with "---". > +-- written in the root of the given repository directory. > +-- > +-- @param repo A local repository directory. > +-- @return boolean or (nil, string): True if manifest was > +-- generated, or nil and an error message. > +local function make_empty_manifest(repo) > + assert(type(repo) == "string") > + if not fs.is_dir(repo) then > + return nil, "Cannot access repository at "..repo > + end > + local manifest = { repository = {}, modules = {}, commands = {} } Seems like a good practice to add a manifest to the cache. manif_core.cache_manifest() Will be used in manif_core.load_local_manifest(). > + return save_table(repo, "manifest", manifest) > +end > + > --- Update manifest file for a local repository > -- adding information about a version of a package installed in that repository. > -- @param name string: Name of a package from the repository. > @@ -445,10 +460,10 @@ function manif.add_to_manifest(name, version, repo, deps_mode) > local manifest, err = manif_core.load_local_manifest(rocks_dir) > if not manifest then > util.printerr("No existing manifest. Attempting to rebuild...") > - -- Manifest built by `manif.make_manifest` should already > - -- include information about given name and version, > - -- no need to update it. > - return manif.make_manifest(rocks_dir, deps_mode) > + -- Create an empty manifest. > + local ok, err = make_empty_manifest(rocks_dir) > + if not ok then return nil, err end > + manifest, err = manif_core.load_local_manifest(rocks_dir) > end > > local results = {[name] = {[version] = {{arch = "installed", repo = rocks_dir}}}} >