Make copy_contents have the same behaviour on Windows and on Unix

3 files changed, 21 insertions, 22 deletions

diff --git a/spec/fs_spec.lua b/spec/fs_spec.lua
index b36fded3..a74980f3 100644
--- a/spec/fs_spec.lua
+++ b/spec/fs_spec.lua
@@ -564,9 +564,6 @@ describe("Luarocks fs test #whitebox #w_fs", function()
          fd:close()
          dstdir = os.tmpname()
          os.remove(dstdir)
-         if is_win then
-            lfs.mkdir(dstdir)
-         end
       end
 
       it("returns true and copies the contents (with their permissions) of the source dir to the destination dir", function()
@@ -604,11 +601,7 @@ describe("Luarocks fs test #whitebox #w_fs", function()
          os.remove(srcdir)
          dstdir = os.tmpname()
          os.remove(dstdir)
-         if is_win then
-            fs.copy_contents(srcdir, dstdir, nil)
-         else
-            assert.falsy(pcall(fs.copy_contents, srcdir, dstdir, nil))
-         end
+         assert.falsy(fs.copy_contents(srcdir, dstdir, nil))
          assert.falsy(exists_file(dstdir))
       end)
 
@@ -619,18 +612,14 @@ describe("Luarocks fs test #whitebox #w_fs", function()
          fd:close()
          dstdir = os.tmpname()
          os.remove(dstdir)
-         if is_win then
-            fs.copy_contents(srcdir, dstdir, nil)
-         else
-            assert.falsy(pcall(fs.copy_contents, srcdir, dstdir, nil))
-         end
+         assert.falsy(fs.copy_contents(srcdir, dstdir, nil))
          assert.falsy(exists_file(dstdir))
       end)
 
       it("returns false and does nothing if the source dir doesn't have the proper permissions #unix", function()
          create_dir_tree()
          assert(fs.chmod(srcdir, "333"))
-         assert.falsy(pcall(fs.copy_contents, srcdir, dstdir, nil))
+         assert.falsy(fs.copy_contents(srcdir, dstdir, nil))
          assert.falsy(exists_file(dstdir))
       end)
    end)
diff --git a/src/luarocks/fs/lua.lua b/src/luarocks/fs/lua.lua
index 38a37f0a..dfb6de41 100644
--- a/src/luarocks/fs/lua.lua
+++ b/src/luarocks/fs/lua.lua
@@ -323,6 +323,9 @@ local function recursive_copy(src, dest, perms)
       local subdir = dir.path(dest, dir.base_name(src))
       local ok, err = fs.make_dir(subdir)
       if not ok then return nil, err end
+      if pcall(lfs.dir, src) == false then
+         return false
+      end
       for file in lfs.dir(src) do
          if file ~= "." and file ~= ".." then
             local ok = recursive_copy(dir.path(src, file), subdir, perms)
@@ -336,15 +339,19 @@ end
 --- Recursively copy the contents of a directory.
 -- @param src string: Pathname of source
 -- @param dest string: Pathname of destination
--- @param perms string or nil: Optional permissions. 
+-- @param perms string or nil: Optional permissions.
 -- @return boolean or (boolean, string): true on success, false on failure,
 -- plus an error message.
 function fs_lua.copy_contents(src, dest, perms)
    assert(src and dest)
    src = dir.normalize(src)
    dest = dir.normalize(dest)
-   assert(lfs.attributes(src, "mode") == "directory")
-
+   if not fs.is_dir(src) then
+      return false, src .. " is not a directory"
+   end
+   if pcall(lfs.dir, src) == false then
+      return false, "Permission denied"
+   end
    for file in lfs.dir(src) do
       if file ~= "." and file ~= ".." then
          local ok = recursive_copy(dir.path(src, file), dest, perms)
@@ -539,14 +546,14 @@ local redirect_protocols = {
 
 local function request(url, method, http, loop_control)
    local result = {}
-   
+
    local proxy = cfg.http_proxy
    if type(proxy) ~= "string" then proxy = nil end
    -- LuaSocket's http.request crashes when given URLs missing the scheme part.
    if proxy and not proxy:find("://") then
       proxy = "http://" .. proxy
    end
-   
+
    if cfg.show_downloads then
       io.write(method.." "..url.." ...\n")
    end
@@ -608,7 +615,7 @@ end
 -- @param http table: The library to use (http from LuaSocket or LuaSec)
 -- @param cache boolean: Whether to use a `.timestamp` file to check
 -- via the HTTP Last-Modified header if the full download is needed.
--- @return (boolean | (nil, string, string?)): True if successful, or 
+-- @return (boolean | (nil, string, string?)): True if successful, or
 -- nil, error message and optionally HTTPS error in case of errors.
 local function http_request(url, filename, http, cache)
    if cache then
@@ -899,7 +906,7 @@ function fs_lua.is_lua(filename)
   local lua = fs.Q(dir.path(cfg.variables["LUA_BINDIR"], cfg.lua_interpreter))  -- get lua interpreter configured
   -- execute on configured interpreter, might not be the same as the interpreter LR is run on
   local result = fs.execute_string(lua..[[ -e "if loadfile(']]..filename..[[') then os.exit() else os.exit(1) end"]])
-  return (result == true) 
+  return (result == true)
 end
 
 return fs_lua
diff --git a/src/luarocks/fs/win32/tools.lua b/src/luarocks/fs/win32/tools.lua
index 7929fdec..b59d9391 100644
--- a/src/luarocks/fs/win32/tools.lua
+++ b/src/luarocks/fs/win32/tools.lua
@@ -79,7 +79,10 @@ end
 -- plus an error message.
 function tools.copy_contents(src, dest)
    assert(src and dest)
-   if fs.execute_quiet(fs.Q(vars.CP), "-dR", src.."\\*.*", dest) then
+   if not fs.is_dir(src) then
+      return false, src .. " is not a directory"
+   end
+   if fs.make_dir(dest) and fs.execute_quiet(fs.Q(vars.CP), "-dR", src.."\\*.*", dest) then
       return true
    else
       return false, "Failed copying "..src.." to "..dest