repos: double-check that all files from a rock are installed

Ensure that `luarocks` fails if an installation does not successfully
deploy all files registered in the `rock_manifest`.

See #1276.

7 files changed, 129 insertions, 9 deletions

diff --git a/spec/remove_spec.lua b/spec/remove_spec.lua
index 91b6a460..21a39696 100644
--- a/spec/remove_spec.lua
+++ b/spec/remove_spec.lua
@@ -2,6 +2,7 @@ local test_env = require("spec.util.test_env")
 local lfs = require("lfs")
 local run = test_env.run
 local testing_paths = test_env.testing_paths
+local env_variables = test_env.env_variables
 
 test_env.unload_luarocks()
 
@@ -9,7 +10,9 @@ local extra_rocks = {
    "/abelhas-1.1-1.src.rock",
    "/copas-2.0.1-1.src.rock",
    "/coxpcall-1.16.0-1.src.rock",
-   "/coxpcall-1.16.0-1.rockspec"
+   "/coxpcall-1.16.0-1.rockspec",
+   "/luafilesystem-1.7.0-1.src.rock",
+   "/luafilesystem-1.6.3-2.src.rock",
 }
 
 describe("luarocks remove #integration", function()
@@ -80,6 +83,36 @@ describe("luarocks remove #integration", function()
          assert.is.falsy(lfs.attributes(testing_paths.testing_sys_rocks .. "/coxpcall"))
          assert.is.falsy(output:find("Checking stability of dependencies"))
       end)
+
+      it("restores old versions", function()
+         assert.is_true(run.luarocks_bool("install luafilesystem 1.6.3"))
+         assert.is.truthy(lfs.attributes(testing_paths.testing_sys_tree .. "/lib/lua/"..env_variables.LUA_VERSION.."/lfs."..test_env.lib_extension))
+
+         if test_env.TEST_TARGET_OS ~= "windows" then
+            local fd = io.open(testing_paths.testing_sys_tree .. "/lib/lua/"..env_variables.LUA_VERSION.."/lfs."..test_env.lib_extension, "r")
+            assert(fd:read("*a"):match("LuaFileSystem 1.6.3", 1, true))
+            fd:close()
+         end
+
+         assert.is_true(run.luarocks_bool("install luafilesystem 1.7.0 --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_2-lfs."..test_env.lib_extension))
+
+         if test_env.TEST_TARGET_OS ~= "windows" then
+            local fd = io.open(testing_paths.testing_sys_tree .. "/lib/lua/"..env_variables.LUA_VERSION.."/lfs."..test_env.lib_extension, "r")
+            assert(fd:read("*a"):match("LuaFileSystem 1.7.0", 1, true))
+            fd:close()
+         end
+
+         assert.is_true(run.luarocks_bool("remove luafilesystem 1.7.0"))
+         assert.is.truthy(lfs.attributes(testing_paths.testing_sys_tree .. "/lib/lua/"..env_variables.LUA_VERSION.."/lfs."..test_env.lib_extension))
+
+         if test_env.TEST_TARGET_OS ~= "windows" then
+            local fd = io.open(testing_paths.testing_sys_tree .. "/lib/lua/"..env_variables.LUA_VERSION.."/lfs."..test_env.lib_extension, "r")
+            assert(fd:read("*a"):match("LuaFileSystem 1.6.3", 1, true))
+            fd:close()
+         end
+      end)
    end)
 
    it("#admin remove #ssh", function()
diff --git a/src/luarocks/cmd/build.lua b/src/luarocks/cmd/build.lua
index 4500bcc9..6ad76b6e 100644
--- a/src/luarocks/cmd/build.lua
+++ b/src/luarocks/cmd/build.lua
@@ -165,8 +165,10 @@ function cmd_build.command(args)
       util.printout()
    else
       if (not args.keep) and not cfg.keep_other_versions then
-         local ok, err = remove.remove_other_versions(name, version, args.force, args.force_fast)
+         local ok, err, warn = remove.remove_other_versions(name, version, args.force, args.force_fast)
          if not ok then
+            return nil, err
+         elseif warn then
             util.printerr(err)
          end
       end
diff --git a/src/luarocks/cmd/install.lua b/src/luarocks/cmd/install.lua
index da9e1ce3..0d6ad63f 100644
--- a/src/luarocks/cmd/install.lua
+++ b/src/luarocks/cmd/install.lua
@@ -195,8 +195,12 @@ local function install_rock_file(filename, opts)
    end
 
    if (not opts.keep) and not cfg.keep_other_versions then
-      local ok, err = remove.remove_other_versions(name, version, opts.force, opts.force_fast)
-      if not ok then util.printerr(err) end
+      local ok, err, warn = remove.remove_other_versions(name, version, opts.force, opts.force_fast)
+      if not ok then
+         return nil, err
+      elseif warn then
+         util.printerr(err)
+      end
    end
 
    writer.check_dependencies(nil, opts.deps_mode)
diff --git a/src/luarocks/cmd/make.lua b/src/luarocks/cmd/make.lua
index a72e6817..8b313bb9 100644
--- a/src/luarocks/cmd/make.lua
+++ b/src/luarocks/cmd/make.lua
@@ -142,8 +142,12 @@ function make.command(args)
       end
 
       if (not args.keep) and not cfg.keep_other_versions then
-         local ok, err = remove.remove_other_versions(name, version, args.force, args.force_fast)
-         if not ok then util.printerr(err) end
+         local ok, err, warn = remove.remove_other_versions(name, version, args.force, args.force_fast)
+         if not ok then
+            return nil, err
+         elseif warn then
+            util.printerr(warn)
+         end
       end
 
       writer.check_dependencies(nil, deps.get_deps_mode(args))
diff --git a/src/luarocks/cmd/purge.lua b/src/luarocks/cmd/purge.lua
index 2b5873d7..c300e286 100644
--- a/src/luarocks/cmd/purge.lua
+++ b/src/luarocks/cmd/purge.lua
@@ -62,9 +62,11 @@ function purge.command(args)
       for version, _ in util.sortedpairs(versions, sort) do
          if args.old_versions then
             util.printout("Keeping "..package.." "..version.."...")
-            local ok, err = remove.remove_other_versions(package, version, args.force, args.force_fast)
+            local ok, err, warn = remove.remove_other_versions(package, version, args.force, args.force_fast)
             if not ok then
                util.printerr(err)
+            elseif warn then
+               util.printerr(err)
             end
             break
          else
diff --git a/src/luarocks/remove.lua b/src/luarocks/remove.lua
index 9f816c7e..385930eb 100644
--- a/src/luarocks/remove.lua
+++ b/src/luarocks/remove.lua
@@ -7,6 +7,7 @@ local repos = require("luarocks.repos")
 local path = require("luarocks.path")
 local util = require("luarocks.util")
 local cfg = require("luarocks.core.cfg")
+local manif = require("luarocks.manif")
 local queries = require("luarocks.queries")
 
 --- Obtain a list of packages that depend on the given set of packages
@@ -107,10 +108,25 @@ function remove.remove_other_versions(name, version, force, fast)
    local results = {}
    local query = queries.new(name, nil, version, false, nil, "~=")
    search.local_manifest_search(results, cfg.rocks_dir, query)
+   local warn
    if results[name] then
-      return remove.remove_search_results(results, name, cfg.deps_mode, force, fast)
+      local ok, err = remove.remove_search_results(results, name, cfg.deps_mode, force, fast)
+      if not ok then -- downgrade failure to a warning
+         warn = err
+      end
    end
-   return true
+
+   if not fast then
+      -- since we're not using --keep, this means that all files of the rock being installed
+      -- should be available as non-versioned variants. Double-check that:
+      local rock_manifest, load_err = manif.load_rock_manifest(name, version)
+      local ok, err = repos.check_everything_is_installed(name, version, rock_manifest, cfg.root_dir, false)
+      if not ok then
+         return nil, err
+      end
+   end
+
+   return true, nil, warn
 end
 
 return remove
diff --git a/src/luarocks/repos.lua b/src/luarocks/repos.lua
index 8eecf8fd..6827cd0f 100644
--- a/src/luarocks/repos.lua
+++ b/src/luarocks/repos.lua
@@ -392,6 +392,28 @@ local function rollback_ops(ops, op_fn, n)
    end
 end
 
+--- Double check that all files referenced in `rock_manifest` are installed in `repo`.
+function repos.check_everything_is_installed(name, version, rock_manifest, repo, accept_versioned)
+   local missing = {}
+   for _, category in ipairs({"bin", "lua", "lib"}) do
+      local suffix = (category == "bin") and cfg.wrapper_suffix or ""
+      if rock_manifest[category] then
+         repos.recurse_rock_manifest_entry(rock_manifest[category], function(file_path)
+            local paths = get_deploy_paths(name, version, category, file_path, repo)
+            if not (fs.exists(paths.nv .. suffix) or (accept_versioned and fs.exists(paths.v .. suffix))) then
+               table.insert(missing, paths.nv .. suffix)
+            end
+         end)
+      end
+   end
+   if #missing > 0 then
+      return nil, "failed deploying files. " ..
+                  "The following files were not installed:\n" ..
+                  table.concat(missing, "\n")
+   end
+   return true
+end
+
 --- Deploy a package from the rocks subdirectory.
 -- @param name string: name of package
 -- @param version string: exact package version in string format
@@ -503,10 +525,37 @@ function repos.deploy_files(name, version, wrap_bin_scripts, deps_mode)
    end
    done_op_install()
 
+   local ok, err = repos.check_everything_is_installed(name, version, rock_manifest, repo, true)
+   if not ok then
+      return nil, err
+   end
+
    local writer = require("luarocks.manif.writer")
    return writer.add_to_manifest(name, version, nil, deps_mode)
 end
 
+local function add_to_double_checks(double_checks, name, version)
+   double_checks[name] = double_checks[name] or {}
+   double_checks[name][version] = true
+end
+
+local function double_check_all(double_checks, repo)
+   local errs = {}
+   for next_name, versions in pairs(double_checks) do
+      for next_version in pairs(versions) do
+         local rock_manifest, load_err = manif.load_rock_manifest(next_name, next_version)
+         local ok, err = repos.check_everything_is_installed(next_name, next_version, rock_manifest, repo, true)
+         if not ok then
+            table.insert(errs, err)
+         end
+      end
+   end
+   if next(errs) then
+      return nil, table.concat(errs, "\n")
+   end
+   return true
+end
+
 --- Delete a package from the local repository.
 -- @param name string: name of package
 -- @param version string: exact package version in string format
@@ -536,6 +585,8 @@ function repos.delete_version(name, version, deps_mode, quick)
    local renames = {}
    local deletes = {}
 
+   local double_checks = {}
+
    if rock_manifest.bin then
       repos.recurse_rock_manifest_entry(rock_manifest.bin, function(file_path)
          local paths = get_deploy_paths(name, version, "bin", file_path, repo)
@@ -547,6 +598,7 @@ function repos.delete_version(name, version, deps_mode, quick)
 
             local next_name, next_version = manif.get_next_provider("command", item_name)
             if next_name then
+               add_to_double_checks(double_checks, next_name, next_version)
                local next_paths = get_deploy_paths(next_name, next_version, "lua", file_path, repo)
                table.insert(renames, { src = next_paths.v, dst = next_paths.nv, suffix = cfg.wrapper_suffix })
             end
@@ -565,6 +617,7 @@ function repos.delete_version(name, version, deps_mode, quick)
 
             local next_name, next_version = manif.get_next_provider("module", item_name)
             if next_name then
+               add_to_double_checks(double_checks, next_name, next_version)
                local next_lua_paths = get_deploy_paths(next_name, next_version, "lua", file_path, repo)
                table.insert(renames, { src = next_lua_paths.v, dst = next_lua_paths.nv })
                local next_lib_paths = get_deploy_paths(next_name, next_version, "lib", file_path:gsub("%.[^.]+$", ".lua"), repo)
@@ -585,6 +638,7 @@ function repos.delete_version(name, version, deps_mode, quick)
 
             local next_name, next_version = manif.get_next_provider("module", item_name)
             if next_name then
+               add_to_double_checks(double_checks, next_name, next_version)
                local next_lua_paths = get_deploy_paths(next_name, next_version, "lua", file_path:gsub("%.[^.]+$", ".lua"), repo)
                table.insert(renames, { src = next_lua_paths.v, dst = next_lua_paths.nv })
                local next_lib_paths = get_deploy_paths(next_name, next_version, "lib", file_path, repo)
@@ -604,6 +658,11 @@ function repos.delete_version(name, version, deps_mode, quick)
       for _, op in ipairs(renames) do
          op_rename(op)
       end
+
+      local ok, err = double_check_all(double_checks, repo)
+      if not ok then
+         return nil, err
+      end
    end
 
    fs.delete(path.install_dir(name, version))