repos: rollback on installation failure (#1101)

2 files changed, 88 insertions, 15 deletions

diff --git a/spec/install_spec.lua b/spec/install_spec.lua
index 3b0b22fb..9b082ab8 100644
--- a/spec/install_spec.lua
+++ b/spec/install_spec.lua
@@ -81,7 +81,7 @@ describe("luarocks install #integration", function()
          assert.is_true(run.luarocks_bool("show luasocket"))
       end)
 
-      it("installs a package without its documentation #only", function()
+      it("installs a package without its documentation", function()
          assert.is_true(run.luarocks_bool("install wsapi 1.6 --no-doc"))
          assert.is_true(run.luarocks_bool("show wsapi 1.6"))
          assert.is.falsy(lfs.attributes(testing_paths.testing_sys_rocks .. "/wsapi/1.6-1/doc"))
@@ -195,6 +195,39 @@ describe("luarocks install #integration", function()
          assert.is_true(os.remove("luasocket-3.0rc1-2." .. test_env.platform .. ".rock"))
       end)
 
+      it("installation rolls back on failure", function()
+         assert.is_true(run.luarocks_bool("build --pack-binary-rock luasocket 3.0rc1-2"))
+         local luadir = testing_paths.testing_sys_tree .. "/share/lua/"..env_variables.LUA_VERSION
+         lfs.mkdir(luadir)
+
+         run.luarocks_bool("remove " .. "luasocket")
+
+         -- create a file where a folder should be
+         local fd = io.open(luadir .. "/socket", "w")
+         fd:write("\n")
+         fd:close()
+
+         -- try to install and fail
+         assert.is_false(run.luarocks_bool("install " .. "luasocket-3.0rc1-2." .. test_env.platform .. ".rock"))
+
+         -- file is still there
+         assert.is.truthy(lfs.attributes(luadir .. "/socket"))
+         -- no left overs from failed installation
+         assert.is.falsy(lfs.attributes(luadir .. "/mime.lua"))
+
+         -- remove file
+         assert.is_true(os.remove(luadir .. "/socket"))
+
+         -- try again and succeed
+         assert.is_true(run.luarocks_bool("install " .. "luasocket-3.0rc1-2." .. test_env.platform .. ".rock"))
+
+         -- files installed successfully
+         assert.is.truthy(lfs.attributes(luadir .. "/socket/ftp.lua"))
+         assert.is.truthy(lfs.attributes(luadir .. "/mime.lua"))
+
+         assert.is_true(os.remove("luasocket-3.0rc1-2." .. test_env.platform .. ".rock"))
+      end)
+
       it("binary rock of cprint", function()
          assert.is_true(run.luarocks_bool("build --pack-binary-rock cprint"))
          assert.is_true(run.luarocks_bool("install cprint-0.1-2." .. test_env.platform .. ".rock"))
diff --git a/src/luarocks/repos.lua b/src/luarocks/repos.lua
index 0058d928..2b0d3fcb 100644
--- a/src/luarocks/repos.lua
+++ b/src/luarocks/repos.lua
@@ -276,6 +276,7 @@ local function backup_existing(should_backup, target)
       if not move_ok then
          return nil, move_err
       end
+      return backup
    end
 end
 
@@ -285,12 +286,30 @@ local function op_install(op)
       return nil, err
    end
 
+   local backup, err = backup_existing(op.backup, op.realdst or op.dst)
+   if err then
+      return nil, err
+   end
+   if backup then
+      op.backup_file = backup
+   end
+
    ok, err = op.fn(op.src, op.dst, op.backup)
    if not ok then
       return nil, err
    end
 
    fs.remove_dir_tree_if_empty(dir.dir_name(op.src))
+   return true
+end
+
+local function rollback_install(op)
+   fs.delete(op.dst)
+   if op.backup_file then
+      fs.move(op.backup_file, op.dst)
+   end
+   fs.remove_dir_tree_if_empty(dir.dir_name(op.dst))
+   return true
 end
 
 local function op_rename(op)
@@ -311,6 +330,10 @@ local function op_rename(op)
    end
 end
 
+local function rollback_rename(op)
+   return op_rename({ src = op.dst, dst = op.src })
+end
+
 local function op_delete(op)
    if op.suffix then
       local suffix = check_suffix(op.name, op.suffix)
@@ -322,6 +345,12 @@ local function op_delete(op)
    return ok, err
 end
 
+local function rollback_ops(ops, op_fn, n)
+   for i = 1, n do
+      op_fn(ops[i])
+   end
+end
+
 --- Deploy a package from the rocks subdirectory.
 -- @param name string: name of package
 -- @param version string: exact package version in string format
@@ -341,23 +370,19 @@ function repos.deploy_files(name, version, wrap_bin_scripts, deps_mode)
    local renames = {}
    local installs = {}
 
-   local function install_binary(source, target, should_backup)
+   local function install_binary(source, target)
       if wrap_bin_scripts and fs.is_lua(source) then
-         backup_existing(should_backup, target .. (cfg.wrapper_suffix or ""))
          return fs.wrap_script(source, target, deps_mode, name, version)
       else
-         backup_existing(should_backup, target)
          return fs.copy_binary(source, target)
       end
    end
 
-   local function move_lua(source, target, should_backup)
-      backup_existing(should_backup, target)
+   local function move_lua(source, target)
       return fs.move(source, target, "read")
    end
 
-   local function move_lib(source, target, should_backup)
-      backup_existing(should_backup, target)
+   local function move_lib(source, target)
       return fs.move(source, target, "exec")
    end
 
@@ -372,8 +397,12 @@ function repos.deploy_files(name, version, wrap_bin_scripts, deps_mode)
             local cur_paths = get_deploy_paths(cur_name, cur_version, "bin", file_path, repo)
             table.insert(renames, { src = cur_paths.nv, dst = cur_paths.v, suffix = cfg.wrapper_suffix })
          end
+         local target = mode == "nv" and paths.nv or paths.v
          local backup = name ~= cur_name or version ~= cur_version
-         table.insert(installs, { fn = install_binary, src = source, dst = mode == "nv" and paths.nv or paths.v, backup = backup })
+         local realdst = (wrap_bin_scripts and fs.is_lua(source))
+                         and (target .. (cfg.wrapper_suffix or ""))
+                         or target
+         table.insert(installs, { fn = install_binary, src = source, dst = target, backup = backup, realdst = realdst })
       end)
    end
 
@@ -390,8 +419,9 @@ function repos.deploy_files(name, version, wrap_bin_scripts, deps_mode)
             cur_paths = get_deploy_paths(cur_name, cur_version, "lib", file_path:gsub("%.lua$", "." .. cfg.lib_extension), repo)
             table.insert(renames, { src = cur_paths.nv, dst = cur_paths.v })
          end
+         local target = mode == "nv" and paths.nv or paths.v
          local backup = name ~= cur_name or version ~= cur_version
-         table.insert(installs, { fn = move_lua, src = source, dst = mode == "nv" and paths.nv or paths.v, backup = backup })
+         table.insert(installs, { fn = move_lua, src = source, dst = target, backup = backup })
       end)
    end
 
@@ -408,16 +438,26 @@ function repos.deploy_files(name, version, wrap_bin_scripts, deps_mode)
             cur_paths = get_deploy_paths(cur_name, cur_version, "lib", file_path, repo)
             table.insert(renames, { src = cur_paths.nv, dst = cur_paths.v })
          end
+         local target = mode == "nv" and paths.nv or paths.v
          local backup = name ~= cur_name or version ~= cur_version
-         table.insert(installs, { fn = move_lib, src = source, dst = mode == "nv" and paths.nv or paths.v, backup = backup })
+         table.insert(installs, { fn = move_lib, src = source, dst = target, backup = backup })
       end)
    end
 
-   for _, op in ipairs(renames) do
-      op_rename(op)
+   for i, op in ipairs(renames) do
+      local ok, err = op_rename(op)
+      if not ok then
+         rollback_ops(renames, rollback_rename, i - 1)
+         return nil, err
+      end
    end
-   for _, op in ipairs(installs) do
-      op_install(op)
+   for i, op in ipairs(installs) do
+      local ok, err = op_install(op)
+      if not ok then
+         rollback_ops(installs, rollback_install, i - 1)
+         rollback_ops(renames, rollback_rename, #renames)
+         return nil, err
+      end
    end
 
    local writer = require("luarocks.manif.writer")