fs: always check for failure of fs.which_tool (#1157)

3 files changed, 31 insertions, 22 deletions

diff --git a/src/luarocks/fetch.lua b/src/luarocks/fetch.lua
index 464061d1..da912d3e 100644
--- a/src/luarocks/fetch.lua
+++ b/src/luarocks/fetch.lua
@@ -31,7 +31,7 @@ function fetch.fetch_caching(url)
 
    local file, err, errcode, from_cache = fetch.fetch_url(url, cachefile, true)
    if not file then
-      return nil, "Failed downloading "..repo_url..(err and " - "..err or ""), errcode
+      return nil, err or "Failed downloading "..url, errcode
    end
    return file, nil, nil, from_cache
 end
@@ -67,7 +67,7 @@ function fetch.fetch_url(url, filename, cache)
    elseif dir.is_basic_protocol(protocol) then
       local ok, name, from_cache = fs.download(url, filename, cache)
       if not ok then
-         return nil, "Failed downloading "..url..(filename and " - "..filename or ""), "network"
+         return nil, "Failed downloading "..url..(name and " - "..name or ""), "network"
       end
       return name, nil, nil, from_cache
    else
diff --git a/src/luarocks/fs/lua.lua b/src/luarocks/fs/lua.lua
index c5cebd2f..b6314a44 100644
--- a/src/luarocks/fs/lua.lua
+++ b/src/luarocks/fs/lua.lua
@@ -905,8 +905,11 @@ function fs_lua.download(url, filename, cache)
       err = "Unsupported protocol"
    end
    if https_err then
+      local downloader, err = fs.which_tool("downloader")
+      if not downloader then
+         return nil, err
+      end
       if not downloader_warning then
-         local downloader = fs.which_tool("downloader")
          util.warning("falling back to "..downloader.." - install luasec to get native HTTPS support")
          downloader_warning = true
       end
diff --git a/src/luarocks/fs/tools.lua b/src/luarocks/fs/tools.lua
index 541352c2..fdb93dba 100644
--- a/src/luarocks/fs/tools.lua
+++ b/src/luarocks/fs/tools.lua
@@ -15,10 +15,12 @@ do
    
    local tool_options = {
       downloader = {
+         desc = "downloader",
          { var = "WGET", name = "wget" },
          { var = "CURL", name = "curl" },
       },
       md5checker = {
+         desc = "MD5 checker",
          { var = "MD5SUM", name = "md5sum" },
          { var = "OPENSSL", name = "openssl", cmdarg = "md5", checkarg = "version" },
          { var = "MD5", name = "md5", checkarg = "-shello" },
@@ -27,8 +29,10 @@ do
 
    function tools.which_tool(tooltype)
       local tool = tool_cache[tooltype]
+      local names = {}
       if not tool then
          for _, opt in ipairs(tool_options[tooltype]) do
+            table.insert(names, opt.name)
             if fs.is_tool_available(vars[opt.var], opt.name, opt.checkarg) then
                tool = opt
                tool_cache[tooltype] = opt
@@ -37,7 +41,8 @@ do
          end
       end
       if not tool then
-         return nil
+         local tool_names = table.concat(names, ", ", 1, #names - 1) .. " or " .. names[#names]
+         return nil, "no " .. tool_options[tooltype].desc .. " tool available," .. " please install " .. tool_names .. " in your system"
       end
       return tool.name, vars[tool.var] .. (tool.cmdarg and " "..tool.cmdarg or "")
    end
@@ -143,9 +148,12 @@ function tools.use_downloader(url, filename, cache)
 
    filename = fs.absolute_name(filename or dir.base_name(url))
    
-   local downloader = fs.which_tool("downloader")
+   local downloader, err = fs.which_tool("downloader")
+   if not downloader then
+      return nil, err
+   end
 
-   local ok
+   local ok = false
    if downloader == "wget" then
       local wget_cmd = vars.WGET.." "..vars.WGETNOCERTFLAG.." --no-cache --user-agent=\""..cfg.user_agent.." via wget\" --quiet "
       if cfg.connection_timeout and cfg.connection_timeout > 0 then
@@ -172,14 +180,12 @@ function tools.use_downloader(url, filename, cache)
          curl_cmd = curl_cmd .. " -R -z \"" .. filename .. "\" "
       end
       ok = fs.execute_string(fs.quiet_stderr(curl_cmd..fs.Q(url).." --output "..fs.Q(filename)))
-   else
-      return false, "No downloader tool available -- please install 'wget' or 'curl' in your system"
    end
    if ok then
       return true, filename
    else
       os.remove(filename)
-      return false, "Failed downloading " .. url
+      return false, "failed downloading " .. url
    end
 end
 
@@ -188,20 +194,20 @@ end
 -- @return string: The MD5 checksum or nil + message
 function tools.get_md5(file)
    local ok, md5checker = fs.which_tool("md5checker")
-   if ok then
-      local pipe = io.popen(md5checker.." "..fs.Q(fs.absolute_name(file)))
-      local computed = pipe:read("*l")
-      pipe:close()
-      if computed then
-         computed = computed:match("("..("%x"):rep(32)..")")
-      end
-      if computed then
-         return computed
-      else
-         return nil, "Failed to compute MD5 hash for file "..tostring(fs.absolute_name(file))
-      end
+   if not ok then
+      return false, md5checker
+   end
+
+   local pipe = io.popen(md5checker.." "..fs.Q(fs.absolute_name(file)))
+   local computed = pipe:read("*l")
+   pipe:close()
+   if computed then
+      computed = computed:match("("..("%x"):rep(32)..")")
+   end
+   if computed then
+      return computed
    else
-      return false, "No MD5 checking tool available -- please install 'md5', 'md5sum' or 'openssl' in your system"
+      return nil, "Failed to compute MD5 hash for file "..tostring(fs.absolute_name(file))
    end
 end