Cache manifest more aggressively

* Introduce cache_timeout config option (default 10 seconds)
* Do not re-check for the last-modified time of a file to
  be downloaded for cfg.cache_timeout seconds
* Do not re-unzip if zip file was fetched from cache

5 files changed, 65 insertions, 28 deletions

diff --git a/src/luarocks/core/cfg.lua b/src/luarocks/core/cfg.lua
index c6824153..859f77e0 100644
--- a/src/luarocks/core/cfg.lua
+++ b/src/luarocks/core/cfg.lua
@@ -188,6 +188,8 @@ local function make_defaults(lua_version, target_cpu, platforms, home)
       hooks_enabled = true,
       deps_mode = "one",
       check_certificates = false,
+      
+      cache_timeout = 10,
 
       lua_modules_path = "/share/lua/"..lua_version,
       lib_modules_path = "/lib/lua/"..lua_version,
diff --git a/src/luarocks/fetch.lua b/src/luarocks/fetch.lua
index 8fa7d6a6..6835462a 100644
--- a/src/luarocks/fetch.lua
+++ b/src/luarocks/fetch.lua
@@ -21,9 +21,16 @@ local cfg = require("luarocks.core.cfg")
 -- filename can be given explicitly as this second argument.
 -- @param cache boolean: compare remote timestamps via HTTP HEAD prior to
 -- re-downloading the file.
--- @return string or (nil, string, [string]): the absolute local pathname for the
--- fetched file, or nil and a message in case of errors, followed by
--- an optional error code.
+-- @return (string, nil, nil, boolean) or (nil, string, [string]):
+-- in case of success:
+-- * the absolute local pathname for the fetched file
+-- * nil
+-- * nil
+-- * `true` if the file was fetched from cache
+-- in case of failure:
+-- * nil
+-- * an error message
+-- * an optional error code.
 function fetch.fetch_url(url, filename, cache)
    assert(type(url) == "string")
    assert(type(filename) == "string" or not filename)
@@ -32,11 +39,11 @@ function fetch.fetch_url(url, filename, cache)
    if protocol == "file" then
       return fs.absolute_name(pathname)
    elseif dir.is_basic_protocol(protocol) then
-      local ok, name = fs.download(url, filename, cache)
+      local ok, name, from_cache = fs.download(url, filename, cache)
       if not ok then
          return nil, "Failed downloading "..url..(filename and " - "..filename or ""), "network"
       end
-      return name
+      return name, nil, nil, from_cache
    else
       return nil, "Unsupported protocol "..protocol
    end
diff --git a/src/luarocks/fs/lua.lua b/src/luarocks/fs/lua.lua
index 6625ddda..313204b9 100644
--- a/src/luarocks/fs/lua.lua
+++ b/src/luarocks/fs/lua.lua
@@ -734,6 +734,14 @@ local function request(url, method, http, loop_control)
    end
 end
 
+local function write_timestamp(filename, data)
+   local utfd = io.open(filename, "w")
+   if utfd then
+      utfd:write(data)
+      utfd:close()
+   end
+end
+
 -- @param url string: URL to fetch.
 -- @param filename string: local filename of the file to fetch.
 -- @param http table: The library to use (http from LuaSocket or LuaSec)
@@ -747,12 +755,25 @@ local function http_request(url, filename, http, cache)
       if tsfd then
          local timestamp = tsfd:read("*a")
          tsfd:close()
+         local unixtime
+         local utfd = io.open(filename..".unixtime", "r")
+         if utfd then
+            unixtime = tonumber(utfd:read("*a"))
+            utfd:close()
+         end
+         if unixtime then
+            if os.time() - unixtime < cfg.cache_timeout then
+               return true, nil, nil, true
+            end
+         end
+
          local result, status, headers, err = request(url, "HEAD", http)
          if not result then
             return nil, status, headers
          end
          if status == 200 and headers["last-modified"] == timestamp then
-            return true
+            write_timestamp(filename .. ".unixtime", os.time())
+            return true, nil, nil, true
          end
       end
    end
@@ -761,11 +782,8 @@ local function http_request(url, filename, http, cache)
       return nil, status, headers
    end
    if cache and headers["last-modified"] then
-      local tsfd = io.open(filename..".timestamp", "w")
-      if tsfd then
-         tsfd:write(headers["last-modified"])
-         tsfd:close()
-      end
+      write_timestamp(filename .. ".timestamp", headers["last-modified"])
+      write_timestamp(filename .. ".unixtime", os.time())
    end
    local file = io.open(filename, "wb")
    if not file then return nil, 0, {} end
@@ -796,8 +814,14 @@ local downloader_warning = false
 -- resulting local filename of the remote file as the basename of the URL;
 -- if that is not correct (due to a redirection, for example), the local
 -- filename can be given explicitly as this second argument.
--- @return (boolean, string): true and the filename on success,
--- false and the error message on failure.
+-- @return (boolean, string, boolean):
+-- In case of success:
+-- * true
+-- * a string with the filename
+-- * true if the file was retrieved from local cache
+-- In case of failure:
+-- * false
+-- * error message
 function fs_lua.download(url, filename, cache)
    assert(type(url) == "string")
    assert(type(filename) == "string" or not filename)
@@ -809,15 +833,16 @@ function fs_lua.download(url, filename, cache)
       return fs.use_downloader(url, filename, cache)
    end
 
-   local ok, err, https_err
+   local ok, err, https_err, from_cache
    if util.starts_with(url, "http:") then
-      ok, err, https_err = http_request(url, filename, http, cache)
+      ok, err, https_err, from_cache = http_request(url, filename, http, cache)
    elseif util.starts_with(url, "ftp:") then
       ok, err = ftp_request(url, filename)
    elseif util.starts_with(url, "https:") then
       -- skip LuaSec when proxy is enabled since it is not supported
       if luasec_ok and not os.getenv("https_proxy") then
-         ok, err = http_request(url, filename, https, cache)
+         local _
+         ok, err, _, from_cache = http_request(url, filename, https, cache)
       else
          https_err = true
       end
@@ -834,7 +859,7 @@ function fs_lua.download(url, filename, cache)
    elseif not ok then
       return nil, err
    end
-   return true, filename
+   return true, filename, from_cache
 end
 
 else --...if socket_ok == false then
diff --git a/src/luarocks/fs/tools.lua b/src/luarocks/fs/tools.lua
index 52915781..ef5abe63 100644
--- a/src/luarocks/fs/tools.lua
+++ b/src/luarocks/fs/tools.lua
@@ -154,6 +154,7 @@ function tools.use_downloader(url, filename, cache)
       if cache then
          -- --timestamping is incompatible with --output-document,
          -- but that's not a problem for our use cases.
+         fs.delete(filename .. ".unixtime")
          fs.change_dir(dir.dir_name(filename))
          ok = fs.execute_quiet(wget_cmd.." --timestamping ", url)
          fs.pop_dir()
diff --git a/src/luarocks/manif.lua b/src/luarocks/manif.lua
index 0bef6f13..9026bc5b 100644
--- a/src/luarocks/manif.lua
+++ b/src/luarocks/manif.lua
@@ -74,11 +74,11 @@ local function fetch_manifest_from(repo_url, filename)
    if not ok then
       return nil, "Failed creating temporary cache directory "..cache_dir
    end
-   local file, err, errcode = fetch.fetch_url(url, dir.path(cache_dir, filename), true)
+   local file, err, errcode, from_cache = fetch.fetch_url(url, dir.path(cache_dir, filename), true)
    if not file then
       return nil, "Failed fetching manifest for "..repo_url..(err and " - "..err or ""), errcode
    end
-   return file
+   return file, nil, nil, from_cache
 end
 
 --- Load a local or remote manifest describing a repository.
@@ -106,7 +106,7 @@ function manif.load_manifest(repo_url, lua_version)
    }
 
    local protocol, repodir = dir.split_url(repo_url)
-   local pathname
+   local pathname, from_cache
    if protocol == "file" then
       for _, filename in ipairs(filenames) do
          pathname = dir.path(repodir, filename)
@@ -117,7 +117,7 @@ function manif.load_manifest(repo_url, lua_version)
    else
       local err, errcode
       for _, filename in ipairs(filenames) do
-         pathname, err, errcode = fetch_manifest_from(repo_url, filename)
+         pathname, err, errcode, from_cache = fetch_manifest_from(repo_url, filename)
          if pathname then
             break
          end
@@ -131,13 +131,15 @@ function manif.load_manifest(repo_url, lua_version)
       local dirname = dir.dir_name(pathname)
       fs.change_dir(dirname)
       local nozip = pathname:match("(.*)%.zip$")
-      fs.delete(nozip)
-      local ok, err = fs.unzip(pathname)
-      fs.pop_dir()
-      if not ok then
-         fs.delete(pathname)
-         fs.delete(pathname..".timestamp")
-         return nil, "Failed extracting manifest file: " .. err
+      if not from_cache then
+         fs.delete(nozip)
+         local ok, err = fs.unzip(pathname)
+         fs.pop_dir()
+         if not ok then
+            fs.delete(pathname)
+            fs.delete(pathname..".timestamp")
+            return nil, "Failed extracting manifest file: " .. err
+         end
       end
       pathname = nozip
    end