From 78e84d2c903e85ec620c08275519dac65e9cbf97 Mon Sep 17 00:00:00 2001 From: Peter Melnichenko Date: Wed, 19 Oct 2016 18:25:27 +0300 Subject: Remove some useless code in luarocks.build Same as 6639022. --- src/luarocks/build.lua | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/luarocks/build.lua b/src/luarocks/build.lua index b306b050..f52fb7a8 100644 --- a/src/luarocks/build.lua +++ b/src/luarocks/build.lua @@ -72,10 +72,7 @@ local function install_files(files, location, is_module_path, perms) if not ok then return nil, err end if filename:match("%.lua$") then local basename = modname:match("([^.]+)$") - local baseinfo = filename:gsub("%.lua$", "") - if basename ~= baseinfo then - filename = basename..".lua" - end + filename = basename..".lua" end else dest = dir.path(location, dir.dir_name(modname)) -- cgit v1.2.3-55-g6feb From 9c239f33a929d300a45b74cc968fb34ffd526150 Mon Sep 17 00:00:00 2001 From: Peter Melnichenko Date: Wed, 19 Oct 2016 22:57:44 +0300 Subject: Add a test for #302 --- spec/install_spec.lua | 22 ++++++++++++++++++++++ test/test_environment.lua | 1 + 2 files changed, 23 insertions(+) diff --git a/spec/install_spec.lua b/spec/install_spec.lua index 2bbf4221..f068af76 100644 --- a/spec/install_spec.lua +++ b/spec/install_spec.lua @@ -21,6 +21,8 @@ local extra_rocks = { "/wsapi-1.6-1.src.rock", "/luafilesystem-1.6.3-2.src.rock", "/luafilesystem-1.6.3-1.src.rock", + "/luacheck-0.7.3-1.src.rock", + "/luacheck-0.8.0-1.src.rock", } describe("LuaRocks install tests #blackbox #b_install", function() @@ -87,13 +89,33 @@ describe("LuaRocks install tests #blackbox #b_install", function() it('LuaRocks install - handle versioned modules when installing another version with --keep #268', function() assert.is_true(run.luarocks_bool("install luafilesystem")) assert.is.truthy(lfs.attributes(testing_paths.testing_sys_tree .. "/lib/lua/"..env_variables.LUA_VERSION.."/lfs."..test_env.lib_extension)) + assert.is_true(run.luarocks_bool("install luafilesystem 1.6.3-1 --keep")) assert.is.truthy(lfs.attributes(testing_paths.testing_sys_tree .. "/lib/lua/"..env_variables.LUA_VERSION.."/lfs."..test_env.lib_extension)) assert.is.truthy(lfs.attributes(testing_paths.testing_sys_tree .. "/lib/lua/"..env_variables.LUA_VERSION.."/luafilesystem_1_6_3_1-lfs."..test_env.lib_extension)) + assert.is_true(run.luarocks_bool("install luafilesystem")) assert.is.truthy(lfs.attributes(testing_paths.testing_sys_tree .. "/lib/lua/"..env_variables.LUA_VERSION.."/lfs."..test_env.lib_extension)) assert.is.falsy(lfs.attributes(testing_paths.testing_sys_tree .. "/lib/lua/"..env_variables.LUA_VERSION.."/luafilesystem_1_6_3_1-lfs."..test_env.lib_extension)) end) + + it('LuaRocks install - handle versioned modules and commands from different files when upgrading #302', function() + assert.is_true(run.luarocks_bool("install luacheck 0.7.3 --deps-mode=none")) + assert.is.truthy(lfs.attributes(testing_paths.testing_sys_tree .. "/share/lua/"..env_variables.LUA_VERSION.."/luacheck.lua")) + assert.is.truthy(lfs.attributes(testing_paths.testing_sys_tree .. "/bin/luacheck"..test_env.wrapper_extension)) + + assert.is_true(run.luarocks_bool("install luacheck 0.8.0 --deps-mode=none")) + assert.is.truthy(lfs.attributes(testing_paths.testing_sys_tree .. "/share/lua/"..env_variables.LUA_VERSION.."/luacheck/init.lua")) + assert.is.truthy(lfs.attributes(testing_paths.testing_sys_tree .. "/bin/luacheck"..test_env.wrapper_extension)) + assert.is.falsy(lfs.attributes(testing_paths.testing_sys_tree .. "/share/lua/"..env_variables.LUA_VERSION.."/luacheck_0_7_3_1-luacheck.lua")) + assert.is.falsy(lfs.attributes(testing_paths.testing_sys_tree .. "/bin/luacheck_0_7_3_1-luacheck"..test_env.wrapper_extension)) + + assert.is_true(run.luarocks_bool("install luacheck 0.7.3 --keep --deps-mode=none")) + assert.is.truthy(lfs.attributes(testing_paths.testing_sys_tree .. "/share/lua/"..env_variables.LUA_VERSION.."/luacheck/init.lua")) + assert.is.truthy(lfs.attributes(testing_paths.testing_sys_tree .. "/bin/luacheck"..test_env.wrapper_extension)) + assert.is.truthy(lfs.attributes(testing_paths.testing_sys_tree .. "/share/lua/"..env_variables.LUA_VERSION.."/luacheck_0_7_3_1-luacheck.lua")) + assert.is.truthy(lfs.attributes(testing_paths.testing_sys_tree .. "/bin/luacheck_0_7_3_1-luacheck"..test_env.wrapper_extension)) + end) it("LuaRocks install only-deps of luasocket packed rock", function() assert.is_true(run.luarocks_bool("build --pack-binary-rock luasocket 3.0rc1-2")) diff --git a/test/test_environment.lua b/test/test_environment.lua index 1d0b0c32..196202df 100644 --- a/test/test_environment.lua +++ b/test/test_environment.lua @@ -515,6 +515,7 @@ function test_env.setup_specs(extra_rocks) test_env.platform = execute_output(test_env.testing_paths.lua .. " -e \"print(require('luarocks.cfg').arch)\"", false, test_env.env_variables) test_env.lib_extension = execute_output(test_env.testing_paths.lua .. " -e \"print(require('luarocks.cfg').lib_extension)\"", false, test_env.env_variables) + test_env.wrapper_extension = test_env.TEST_TARGET_OS == "windows" and ".bat" or "" test_env.md5sums = create_md5sums(test_env.testing_paths) test_env.setup_done = true title("RUNNING TESTS") -- cgit v1.2.3-55-g6feb From fd913d6aab284668854a71a97ac64e9a7dbdd0b8 Mon Sep 17 00:00:00 2001 From: Peter Melnichenko Date: Wed, 19 Oct 2016 21:37:26 +0300 Subject: Fix conflict resolution on deploy/delete When deploying or deleting files, resolve conflicts purely based on module names and command names, not file names. Also, don't assume that in case of a conflict both packages have the same file providing the module or command; it can be false due to binary wrappers and `path_to_module("mod/init.lua")` == `path_to_module("mod.lua"). --- src/luarocks/manif.lua | 54 +++++++++++----- src/luarocks/repos.lua | 170 +++++++++++++++++++++++++++++++------------------ 2 files changed, 146 insertions(+), 78 deletions(-) diff --git a/src/luarocks/manif.lua b/src/luarocks/manif.lua index a8bbf279..1b75452b 100644 --- a/src/luarocks/manif.lua +++ b/src/luarocks/manif.lua @@ -469,33 +469,34 @@ local function relative_path(from_dir, to_file) return (to_file:sub(#from_dir + 1):gsub("^[\\/]*", "")) end -local function find_providers(file, root) - assert(type(file) == "string") - root = root or cfg.root_dir - - local manifest, err = manif_core.load_local_manifest(path.rocks_dir(root)) - if not manifest then - return nil, "untracked" - end +local function file_manifest_coordinates(manifest, file, root) local deploy_bin = path.deploy_bin_dir(root) local deploy_lua = path.deploy_lua_dir(root) local deploy_lib = path.deploy_lib_dir(root) - local key, manifest_tbl if util.starts_with(file, deploy_lua) then - manifest_tbl = manifest.modules - key = path.path_to_module(relative_path(deploy_lua, file):gsub("\\", "/")) + return "modules", path.path_to_module(relative_path(deploy_lua, file):gsub("\\", "/")), deploy_lua elseif util.starts_with(file, deploy_lib) then - manifest_tbl = manifest.modules - key = path.path_to_module(relative_path(deploy_lib, file):gsub("\\", "/")) + return "modules", path.path_to_module(relative_path(deploy_lib, file):gsub("\\", "/")), deploy_lib elseif util.starts_with(file, deploy_bin) then - manifest_tbl = manifest.commands - key = relative_path(deploy_bin, file) + return "commands", relative_path(deploy_bin, file), deploy_bin else assert(false, "Assertion failed: '"..file.."' is not a deployed file.") end +end + +local function find_providers(file, root) + assert(type(file) == "string") + root = root or cfg.root_dir + + local manifest, err = manif_core.load_local_manifest(path.rocks_dir(root)) + if not manifest then + return nil, "untracked" + end + + local type_key, key = file_manifest_coordinates(manifest, file, root) - local providers = manifest_tbl[key] + local providers = manifest[type_key][key] if not providers then return nil, "untracked" end @@ -523,4 +524,25 @@ function manif.find_next_provider(file, root) end end +--- Given a file conflicting with a module or command +-- provided by a version of a package, return which file +-- in that version corresponds to the conflicting item. +-- @param name string: name of the package with conflicting module or command. +-- @param version string: version of the package with conflicting module or command. +-- @param file string: full, unversioned path to a deployed file. +-- @return string: full, unversioned path to a deployed file in +-- given package that conflicts with given file. +function manif.find_conflicting_file(name, version, file, root) + root = root or cfg.root_dir + + local manifest = manif_core.load_local_manifest(path.rocks_dir(root)) + if not manifest then + return + end + + local entry_table = manifest.repository[name][version][1] + local type_key, key, deploy_dir = file_manifest_coordinates(manifest, file, root) + return dir.path(deploy_dir, entry_table[type_key][key]) +end + return manif diff --git a/src/luarocks/repos.lua b/src/luarocks/repos.lua index a1cf2bb4..762192a3 100644 --- a/src/luarocks/repos.lua +++ b/src/luarocks/repos.lua @@ -162,22 +162,6 @@ local function install_binary(source, target, name, version) end end -local function resolve_conflict(target, deploy_dir, name, version) - local cname, cversion = manif.find_current_provider(target) - if not cname then - return nil, cversion - end - if name ~= cname or deps.compare_versions(version, cversion) then - local versioned = path.versioned_name(target, deploy_dir, cname, cversion) - local ok, err = fs.make_dir(dir.dir_name(versioned)) - if not ok then return nil, err end - fs.move(target, versioned) - return target - else - return path.versioned_name(target, deploy_dir, name, version) - end -end - function repos.should_wrap_bin_scripts(rockspec) assert(type(rockspec) == "table") @@ -190,11 +174,71 @@ function repos.should_wrap_bin_scripts(rockspec) return true end +local function find_suffixed(file, suffix) + local filenames = {file} + if suffix and suffix ~= "" then + table.insert(filenames, 1, file .. suffix) + end + + for _, filename in ipairs(filenames) do + if fs.exists(filename) then + return filename + end + end +end + +local function move_suffixed(from_file, to_file, suffix) + local suffixed_from_file = find_suffixed(from_file, suffix) + if not suffixed_from_file then + return nil, "File not found" + end + + suffix = suffixed_from_file:sub(#from_file + 1) + local suffixed_to_file = to_file .. suffix + return fs.move(suffixed_from_file, suffixed_to_file) +end + +local function delete_suffixed(file, suffix) + local suffixed_file = find_suffixed(file, suffix) + if not suffixed_file then + return nil, "File not found", "not found" + end + + fs.delete(suffixed_file) + if fs.exists(suffixed_file) then + return nil, "Failed deleting " .. suffixed_file, "fail" + end + + return true +end + +local function resolve_conflict(target, deploy_dir, name, version, cur_name, cur_version, suffix) + if name < cur_name or (name == cur_name and deps.compare_versions(version, cur_version)) then + -- New version has priority. Move currently provided version back using versioned name. + local cur_target = manif.find_conflicting_file(cur_name, cur_version, target) + local versioned = path.versioned_name(cur_target, deploy_dir, cur_name, cur_version) + + local ok, err = fs.make_dir(dir.dir_name(versioned)) + if not ok then + return nil, err + end + + ok, err = move_suffixed(cur_target, versioned, suffix) + if not ok then + return nil, err + end + + return target + else + -- Current version has priority, deploy new version using versioned name. + return path.versioned_name(target, deploy_dir, name, version) + end +end + --- Deploy a package from the rocks subdirectory. --- It is maintained that for each file the one that is provided +-- It is maintained that for each module and command the one that is provided -- by the newest version of the lexicographically smallest package --- is installed using unversioned name, and other versions of the file --- use versioned names. +-- is installed using unversioned name, and other versions use versioned names. -- @param name string: name of package -- @param version string: exact package version in string format -- @param wrap_bin_scripts bool: whether commands written in Lua should be wrapped. @@ -206,34 +250,40 @@ function repos.deploy_files(name, version, wrap_bin_scripts, deps_mode) assert(type(version) == "string") assert(type(wrap_bin_scripts) == "boolean") - local function deploy_file_tree(file_tree, path_fn, deploy_dir, move_fn) + local function deploy_file_tree(file_tree, path_fn, deploy_dir, move_fn, suffix) local source_dir = path_fn(name, version) return recurse_rock_manifest_tree(file_tree, function(parent_path, parent_module, file) local source = dir.path(source_dir, parent_path, file) local target = dir.path(deploy_dir, parent_path, file) - local ok, err + + local cur_name, cur_version = manif.find_current_provider(target) + if cur_name then + local resolve_err + target, resolve_err = resolve_conflict(target, deploy_dir, name, version, cur_name, cur_version, suffix) + if not target then + return nil, resolve_err + end + end + if fs.exists(target) then - local new_target, err = resolve_conflict(target, deploy_dir, name, version) - if err == "untracked" then - local backup = target - repeat - backup = backup.."~" - until not fs.exists(backup) -- slight race condition here, but shouldn't be a problem. - util.printerr("Warning: "..target.." is not tracked by this installation of LuaRocks. Moving it to "..backup) - fs.move(target, backup) - elseif err then - return nil, err.." Cannot install new version." - else - target = new_target + local backup = target + repeat + backup = backup.."~" + until not fs.exists(backup) -- slight race condition here, but shouldn't be a problem. + + util.printerr("Warning: "..target.." is not tracked by this installation of LuaRocks. Moving it to "..backup) + local ok, err = fs.move(target, backup) + if not ok then + return nil, err end end - ok, err = fs.make_dir(dir.dir_name(target)) + + local ok, err = fs.make_dir(dir.dir_name(target)) if not ok then return nil, err end ok, err = move_fn(source, target, name, version) fs.remove_dir_tree_if_empty(dir.dir_name(source)) - if not ok then return nil, err end - return true + return ok, err end ) end @@ -243,7 +293,7 @@ function repos.deploy_files(name, version, wrap_bin_scripts, deps_mode) local ok, err = true if rock_manifest.bin then local move_bin_fn = wrap_bin_scripts and install_binary or fs.copy_binary - ok, err = deploy_file_tree(rock_manifest.bin, path.bin_dir, cfg.deploy_bin_dir, move_bin_fn) + ok, err = deploy_file_tree(rock_manifest.bin, path.bin_dir, cfg.deploy_bin_dir, move_bin_fn, cfg.wrapper_suffix) end local function make_mover(perms) return function (src, dest) @@ -264,26 +314,10 @@ function repos.deploy_files(name, version, wrap_bin_scripts, deps_mode) return manif.update_manifest(name, version, nil, deps_mode) end -local function delete_suffixed(filename, suffix) - local filenames = { filename } - if suffix and suffix ~= "" then filenames = { filename..suffix, filename } end - for _, name in ipairs(filenames) do - if fs.exists(name) then - fs.delete(name) - if fs.exists(name) then - return nil, "Failed deleting "..name, "fail" - end - return true, name - end - end - return false, "File not found", "not found" -end - --- Delete a package from the local repository. --- It is maintained that for each file the one that is provided +-- It is maintained that for each module and command the one that is provided -- by the newest version of the lexicographically smallest package --- is installed using unversioned name, and other versions of the file --- use versioned names. +-- is installed using unversioned name, and other versions use versioned names. -- @param name string: name of package -- @param version string: exact package version in string format -- @param deps_mode: string: Which trees to check dependencies for: @@ -303,19 +337,31 @@ function repos.delete_version(name, version, deps_mode, quick) function(parent_path, parent_module, file) local target = dir.path(deploy_dir, parent_path, file) local versioned = path.versioned_name(target, deploy_dir, name, version) - local ok, name, err = delete_suffixed(versioned, suffix) + + local ok, err, err_type = delete_suffixed(versioned, suffix) if ok then fs.remove_dir_tree_if_empty(dir.dir_name(versioned)) return true + elseif err_type == "fail" then + return nil, err + end + + ok, err = delete_suffixed(target, suffix) + if not ok then + return nil, err end - if err == "fail" then return nil, name end - ok, name, err = delete_suffixed(target, suffix) - if err == "fail" then return nil, name end + if not quick then local next_name, next_version = manif.find_next_provider(target) if next_name then - local versioned = path.versioned_name(name, deploy_dir, next_name, next_version) - fs.move(versioned, name) + local next_target = manif.find_conflicting_file(next_name, next_version, target) + local next_versioned = path.versioned_name(next_target, deploy_dir, next_name, next_version) + + ok, err = move_suffixed(next_versioned, next_target, suffix) + if not ok then + return nil, err + end + fs.remove_dir_tree_if_empty(dir.dir_name(versioned)) end end @@ -340,7 +386,7 @@ function repos.delete_version(name, version, deps_mode, quick) if ok and rock_manifest.lib then ok, err = delete_deployed_file_tree(rock_manifest.lib, cfg.deploy_lib_dir) end - if err then return nil, err end + if not ok then return nil, err end fs.delete(path.install_dir(name, version)) if not get_installed_versions(name) then -- cgit v1.2.3-55-g6feb From ed2cbf4046b76e7f08a17577d286c08f2439e93f Mon Sep 17 00:00:00 2001 From: Peter Melnichenko Date: Thu, 20 Oct 2016 00:41:21 +0300 Subject: Add a test for wrapped script backup --- spec/install_spec.lua | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/install_spec.lua b/spec/install_spec.lua index f068af76..a00396fa 100644 --- a/spec/install_spec.lua +++ b/spec/install_spec.lua @@ -100,9 +100,11 @@ describe("LuaRocks install tests #blackbox #b_install", function() end) it('LuaRocks install - handle versioned modules and commands from different files when upgrading #302', function() + io.open(testing_paths.testing_sys_tree .. "/bin/luacheck"..test_env.wrapper_extension, "w"):close() assert.is_true(run.luarocks_bool("install luacheck 0.7.3 --deps-mode=none")) assert.is.truthy(lfs.attributes(testing_paths.testing_sys_tree .. "/share/lua/"..env_variables.LUA_VERSION.."/luacheck.lua")) assert.is.truthy(lfs.attributes(testing_paths.testing_sys_tree .. "/bin/luacheck"..test_env.wrapper_extension)) + assert.is.truthy(lfs.attributes(testing_paths.testing_sys_tree .. "/bin/luacheck"..test_env.wrapper_extension .. "~")) assert.is_true(run.luarocks_bool("install luacheck 0.8.0 --deps-mode=none")) assert.is.truthy(lfs.attributes(testing_paths.testing_sys_tree .. "/share/lua/"..env_variables.LUA_VERSION.."/luacheck/init.lua")) -- cgit v1.2.3-55-g6feb From c7b91ce031a3b7877f59951fc8570c97f598e771 Mon Sep 17 00:00:00 2001 From: Peter Melnichenko Date: Thu, 20 Oct 2016 00:41:40 +0300 Subject: Fix backup of wrapped scripts on deploy When deploying script to bin/script.bat, check and back up bin/script.bat, not bin/script. --- src/luarocks/repos.lua | 51 +++++++++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/src/luarocks/repos.lua b/src/luarocks/repos.lua index 762192a3..161cdd8a 100644 --- a/src/luarocks/repos.lua +++ b/src/luarocks/repos.lua @@ -151,17 +151,6 @@ function repos.run_hook(rockspec, hook_name) return true end -local function install_binary(source, target, name, version) - assert(type(source) == "string") - assert(type(target) == "string") - - if fs.is_lua(source) then - return fs.wrap_script(source, target, name, version) - else - return fs.copy_binary(source, target) - end -end - function repos.should_wrap_bin_scripts(rockspec) assert(type(rockspec) == "table") @@ -266,22 +255,24 @@ function repos.deploy_files(name, version, wrap_bin_scripts, deps_mode) end end - if fs.exists(target) then - local backup = target + local ok, err = fs.make_dir(dir.dir_name(target)) + if not ok then return nil, err end + + local suffixed_target, mover = move_fn(source, target, name, version) + if fs.exists(suffixed_target) then + local backup = suffixed_target repeat backup = backup.."~" - until not fs.exists(backup) -- slight race condition here, but shouldn't be a problem. + until not fs.exists(backup) -- Slight race condition here, but shouldn't be a problem. - util.printerr("Warning: "..target.." is not tracked by this installation of LuaRocks. Moving it to "..backup) - local ok, err = fs.move(target, backup) + util.printerr("Warning: "..suffixed_target.." is not tracked by this installation of LuaRocks. Moving it to "..backup) + local ok, err = fs.move(suffixed_target, backup) if not ok then return nil, err end end - local ok, err = fs.make_dir(dir.dir_name(target)) - if not ok then return nil, err end - ok, err = move_fn(source, target, name, version) + ok, err = mover() fs.remove_dir_tree_if_empty(dir.dir_name(source)) return ok, err end @@ -289,17 +280,25 @@ function repos.deploy_files(name, version, wrap_bin_scripts, deps_mode) end local rock_manifest = manif.load_rock_manifest(name, version) - - local ok, err = true - if rock_manifest.bin then - local move_bin_fn = wrap_bin_scripts and install_binary or fs.copy_binary - ok, err = deploy_file_tree(rock_manifest.bin, path.bin_dir, cfg.deploy_bin_dir, move_bin_fn, cfg.wrapper_suffix) + + local function install_binary(source, target, name, version) + if wrap_bin_scripts and fs.is_lua(source) then + return target .. (cfg.wrapper_suffix or ""), function() return fs.wrap_script(source, target, name, version) end + else + return target, function() return fs.copy_binary(source, target) end + end end + local function make_mover(perms) - return function (src, dest) - return fs.move(src, dest, perms) + return function(source, target) + return target, function() return fs.move(source, target, perms) end end end + + local ok, err = true + if rock_manifest.bin then + ok, err = deploy_file_tree(rock_manifest.bin, path.bin_dir, cfg.deploy_bin_dir, install_binary, cfg.wrapper_suffix) + end if ok and rock_manifest.lua then ok, err = deploy_file_tree(rock_manifest.lua, path.lua_dir, cfg.deploy_lua_dir, make_mover(cfg.perm_read)) end -- cgit v1.2.3-55-g6feb