Add general improvements to the fs module

5 files changed, 61 insertions, 44 deletions

diff --git a/spec/fs_spec.lua b/spec/fs_spec.lua
index eb8425e2..66453404 100644
--- a/spec/fs_spec.lua
+++ b/spec/fs_spec.lua
@@ -161,11 +161,19 @@ describe("Luarocks fs test #unit", function()
       it("does nothing if the argument is a file", function()
          tmpfile1 = get_tmp_path()
          create_file(tmpfile1)
-         assert.falsy(pcall(fs.dir_iterator, tmpfile1))
+         local crt = coroutine.create(fs.dir_iterator)
+         while coroutine.status(crt) ~= "dead" do
+            local ok, val = coroutine.resume(crt, tmpfile1)
+            assert.falsy(ok and res)
+         end
       end)
 
       it("does nothing if the argument is invalid", function()
-         assert.falsy(pcall(fs.dir_iterator, "/nonexistent"))
+         local crt = coroutine.create(fs.dir_iterator)
+         while coroutine.status(crt) ~= "dead" do
+            local ok, val = coroutine.resume(crt, "/nonexistent")
+            assert.falsy(ok and res)
+         end
       end)
    end)
    
@@ -530,11 +538,13 @@ describe("Luarocks fs test #unit", function()
          tmpdir = get_tmp_path()
          lfs.mkdir(tmpdir)
          assert.truthy(fs.change_dir(tmpdir))
-         local success = fs.change_dir_to_root()
-         if not is_win then
-            assert.truthy(success)
+         assert.truthy(fs.change_dir_to_root())
+         if is_win then
+            local curr_dir = fs.current_dir()
+            assert.truthy(curr_dir == "C:\\" or curr_dir == "/")
+         else
+            assert.same("/", fs.current_dir())
          end
-         assert.same("/", fs.current_dir())
       end)
 
       it("returns false and does nothing if the current directory is not valid #unix", function()
@@ -749,18 +759,14 @@ describe("Luarocks fs test #unit", function()
       end)
       
       it("does nothing if the argument is nonexistent", function()
-         assert.falsy(pcall(fs.list_dir("/nonexistent")))
+         assert.same(fs.list_dir("/nonexistent"), {})
       end)
       
       it("does nothing if the argument doesn't have the proper permissions", function()
          tmpdir = get_tmp_path()
          lfs.mkdir(tmpdir)
          make_unreadable(tmpdir)
-         if is_win then
-            assert.same(fs.list_dir(tmpdir), {})
-         else
-            assert.falsy(pcall(fs.list_dir, tmpdir))
-         end
+         assert.same(fs.list_dir(tmpdir), {})
       end)
    end)
 
@@ -1024,11 +1030,7 @@ describe("Luarocks fs test #unit", function()
          tmpdir = get_tmp_path()
          lfs.mkdir(tmpdir)
          make_unreadable(tmpdir)
-         if is_win then
-            assert.same(fs.find(tmpdir), {})
-         else
-            assert.falsy(pcall(fs.find, tmpdir))
-         end
+         assert.same(fs.find(tmpdir), {})
       end)
    end)
    
diff --git a/spec/remove_spec.lua b/spec/remove_spec.lua
index 55264a75..f43b55c6 100644
--- a/spec/remove_spec.lua
+++ b/spec/remove_spec.lua
@@ -7,9 +7,9 @@ test_env.unload_luarocks()
 
 local extra_rocks = {
    "/abelhas-1.1-1.rockspec",
-   "/lualogging-1.3.0-1.src.rock",
-   "/luasocket-3.0rc1-2.src.rock",
-   "/luasocket-3.0rc1-2.rockspec"
+   "/copas-2.0.1-1.src.rock",
+   "/coxpcall-1.16.0-1.src.rock",
+   "/coxpcall-1.16.0-1.rockspec"
 }
 
 describe("LuaRocks remove tests #integration", function()
@@ -52,38 +52,38 @@ describe("LuaRocks remove tests #integration", function()
 
    describe("LuaRocks remove more complex tests", function()
       before_each(function()
-         assert.is.truthy(test_env.need_rock("luasocket"))
+         assert.is.truthy(test_env.need_rock("coxpcall"))
       end)
 
       it("LuaRocks remove fail, break dependencies", function()
-         assert.is.truthy(lfs.attributes(testing_paths.testing_sys_rocks .. "/luasocket"))
-         assert.is_true(run.luarocks_bool("build lualogging"))
+         assert.is.truthy(lfs.attributes(testing_paths.testing_sys_rocks .. "/coxpcall"))
+         assert.is_true(run.luarocks_bool("build copas"))
 
-         assert.is_false(run.luarocks_bool("remove luasocket"))
-         assert.is.truthy(lfs.attributes(testing_paths.testing_sys_rocks .. "/luasocket"))
+         assert.is_false(run.luarocks_bool("remove coxpcall"))
+         assert.is.truthy(lfs.attributes(testing_paths.testing_sys_rocks .. "/coxpcall"))
       end)
       
       it("LuaRocks remove force", function()
-         assert.is.truthy(lfs.attributes(testing_paths.testing_sys_rocks .. "/luasocket"))
-         assert.is_true(run.luarocks_bool("build lualogging"))
+         assert.is.truthy(lfs.attributes(testing_paths.testing_sys_rocks .. "/coxpcall"))
+         assert.is_true(run.luarocks_bool("build copas"))
 
-         local output = run.luarocks("remove --force luasocket")
-         assert.is.falsy(lfs.attributes(testing_paths.testing_sys_rocks .. "/luasocket"))
+         local output = run.luarocks("remove --force coxpcall")
+         assert.is.falsy(lfs.attributes(testing_paths.testing_sys_rocks .. "/coxpcall"))
          assert.is.truthy(output:find("Checking stability of dependencies"))
       end)
       
       it("LuaRocks remove force fast", function()
-         assert.is.truthy(lfs.attributes(testing_paths.testing_sys_rocks .. "/luasocket"))
-         assert.is_true(run.luarocks_bool("build lualogging"))
+         assert.is.truthy(lfs.attributes(testing_paths.testing_sys_rocks .. "/coxpcall"))
+         assert.is_true(run.luarocks_bool("build copas"))
 
-         local output = run.luarocks("remove --force-fast luasocket")
-         assert.is.falsy(lfs.attributes(testing_paths.testing_sys_rocks .. "/luasocket"))
+         local output = run.luarocks("remove --force-fast coxpcall")
+         assert.is.falsy(lfs.attributes(testing_paths.testing_sys_rocks .. "/coxpcall"))
          assert.is.falsy(output:find("Checking stability of dependencies"))
       end)
    end)
 
    it("#admin remove #ssh", function()
-      assert.is_true(run.luarocks_admin_bool("--server=testing remove luasocket-3.0rc1-2.src.rock"))
+      assert.is_true(run.luarocks_admin_bool("--server=testing remove coxpcall-1.16.0-1.src.rock"))
    end)
    
    it("#admin remove missing", function()
diff --git a/src/luarocks/fs/lua.lua b/src/luarocks/fs/lua.lua
index 373dcbdd..9da3875b 100644
--- a/src/luarocks/fs/lua.lua
+++ b/src/luarocks/fs/lua.lua
@@ -464,9 +464,13 @@ end
 --- Internal implementation function for fs.dir.
 -- Yields a filename on each iteration.
 -- @param at string: directory to list
--- @return nil
+-- @return nil or (nil and string): an error message on failure 
 function fs_lua.dir_iterator(at)
-   for file in lfs.dir(at) do
+   local pok, iter, arg = pcall(lfs.dir, at)
+   if not pok then
+      return nil, iter
+   end
+   for file in iter, arg do
       if file ~= "." and file ~= ".." then
          coroutine.yield(file)
       end
@@ -479,7 +483,11 @@ end
 -- @param prefix string: Auxiliary prefix string to form pathname.
 -- @param result table: Array of strings where results are collected.
 local function recursive_find(cwd, prefix, result)
-   for file in lfs.dir(cwd) do
+   local pok, iter, arg = pcall(lfs.dir, cwd)
+   if not pok then
+      return nil
+   end
+   for file in iter, arg do
       if file ~= "." and file ~= ".." then
          local item = prefix .. file
          table.insert(result, item)
@@ -502,9 +510,6 @@ function fs_lua.find(at)
       at = fs.current_dir()
    end
    at = dir.normalize(at)
-   if not fs.is_dir(at) then
-      return {}
-   end
    local result = {}
    recursive_find(at, "", result)
    return result
diff --git a/src/luarocks/fs/tools.lua b/src/luarocks/fs/tools.lua
index 0cfdc9f7..d2dc08ae 100644
--- a/src/luarocks/fs/tools.lua
+++ b/src/luarocks/fs/tools.lua
@@ -82,7 +82,12 @@ end
 -- Allows leaving a directory (e.g. for deleting it) in
 -- a crossplatform way.
 function tools.change_dir_to_root()
+   local curr_dir = fs.current_dir()
+   if not curr_dir or not fs.is_dir(curr_dir) then
+      return false
+   end
    table.insert(dir_stack, "/")
+   return true
 end
 
 --- Change working directory to the previous in the directory stack.
@@ -113,7 +118,7 @@ end
 -- @param at string: directory to list
 -- @return nil
 function tools.dir_iterator(at)
-   local pipe = io.popen(fs.command_at(at, fs.Q(vars.LS)))
+   local pipe = io.popen(fs.command_at(at, fs.Q(vars.LS), true))
    for file in pipe:lines() do
       if file ~= "." and file ~= ".." then
          coroutine.yield(file)
diff --git a/src/luarocks/fs/win32/tools.lua b/src/luarocks/fs/win32/tools.lua
index c267b316..de3d9031 100644
--- a/src/luarocks/fs/win32/tools.lua
+++ b/src/luarocks/fs/win32/tools.lua
@@ -13,10 +13,15 @@ local vars = setmetatable({}, { __index = function(_,k) return cfg.variables[k]
 --- Adds prefix to command to make it run from a directory.
 -- @param directory string: Path to a directory.
 -- @param cmd string: A command-line string.
+-- @param exit_on_error bool: Exits immediately if entering the directory failed.
 -- @return string: The command-line with prefix.
-function tools.command_at(directory, cmd)
+function tools.command_at(directory, cmd, exit_on_error)
    local drive = directory:match("^([A-Za-z]:)")
-   cmd = "cd " .. fs.Q(directory) .. " & " .. cmd
+   local op = " & "
+   if exit_on_error then
+      op = " && "
+   end
+   local cmd = "cd " .. fs.Q(directory) .. op .. cmd
    if drive then
       cmd = drive .. " & " .. cmd
    end
@@ -113,7 +118,7 @@ function tools.find(at)
       return {}
    end
    local result = {}
-   local pipe = io.popen(fs.command_at(at, fs.quiet_stderr(fs.Q(vars.FIND))))
+   local pipe = io.popen(fs.command_at(at, fs.quiet_stderr(fs.Q(vars.FIND)), true))
    for file in pipe:lines() do
       -- Windows find is a bit different
       local first_two = file:sub(1,2)