diff --git a/src/libfetchers/cache.cc b/src/libfetchers/cache.cc index e071b4717b4..834c4f736ff 100644 --- a/src/libfetchers/cache.cc +++ b/src/libfetchers/cache.cc @@ -125,10 +125,14 @@ struct CacheImpl : Cache const Attrs & inAttrs) override { if (auto res = lookupExpired(store, inAttrs)) { - if (!res->expired) + if (!res->expired && res->storePathValid) return std::make_pair(std::move(res->infoAttrs), std::move(res->storePath)); - debug("ignoring expired cache entry '%s'", - attrsToJSON(inAttrs).dump()); + if (!res->storePathValid) { + auto inAttrsJSON = attrsToJSON(inAttrs).dump(); + debug("ignoring disappeared cache entry '%s'", inAttrsJSON); + } else + debug("ignoring expired cache entry '%s'", + attrsToJSON(inAttrs).dump()); } return {}; } @@ -153,19 +157,16 @@ struct CacheImpl : Cache auto timestamp = stmt.getInt(3); store.addTempRoot(storePath); - if (!store.isValidPath(storePath)) { - // FIXME: we could try to substitute 'storePath'. - debug("ignoring disappeared cache entry '%s'", inAttrsJSON); - return {}; - } + auto storePathValid = store.isValidPath(storePath); - debug("using cache entry '%s' -> '%s', '%s'", - inAttrsJSON, infoJSON, store.printStorePath(storePath)); + debug("using cache entry '%s' -> '%s', '%s', is valid: %s", + inAttrsJSON, infoJSON, store.printStorePath(storePath), storePathValid); return Result { .expired = !locked && (settings.tarballTtl.get() == 0 || timestamp + settings.tarballTtl < time(0)), + .storePathValid = storePathValid, .infoAttrs = jsonToAttrs(nlohmann::json::parse(infoJSON)), - .storePath = std::move(storePath) + .storePath = std::move(storePath), }; } }; diff --git a/src/libfetchers/cache.hh b/src/libfetchers/cache.hh index 791d77025aa..a1eea7b2480 100644 --- a/src/libfetchers/cache.hh +++ b/src/libfetchers/cache.hh @@ -63,6 +63,7 @@ struct Cache struct Result { bool expired = false; + bool storePathValid; Attrs infoAttrs; StorePath storePath; }; diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index 3b7709440b2..8fcf9cf1e56 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -17,7 +17,8 @@ DownloadFileResult downloadFile( const std::string & url, const std::string & name, bool locked, - const Headers & headers) + const Headers & headers, + bool onlyIfChanged) { // FIXME: check store @@ -32,6 +33,7 @@ DownloadFileResult downloadFile( auto useCached = [&]() -> DownloadFileResult { return { + .storePathValid = cached->storePathValid, .storePath = std::move(cached->storePath), .etag = getStrAttr(cached->infoAttrs, "etag"), .effectiveUrl = getStrAttr(cached->infoAttrs, "url"), @@ -39,18 +41,19 @@ DownloadFileResult downloadFile( }; }; - if (cached && !cached->expired) + bool storePathUseable = cached && (cached->storePathValid || onlyIfChanged); + if (storePathUseable && !cached->expired) return useCached(); FileTransferRequest request(url); request.headers = headers; - if (cached) + if (storePathUseable) request.expectedETag = getStrAttr(cached->infoAttrs, "etag"); FileTransferResult res; try { res = getFileTransfer()->download(request); } catch (FileTransferError & e) { - if (cached) { + if (storePathUseable) { warn("%s; using cached version", e.msg()); return useCached(); } else @@ -69,7 +72,7 @@ DownloadFileResult downloadFile( std::optional storePath; if (res.cached) { - assert(cached); + assert(storePathUseable); storePath = std::move(cached->storePath); } else { StringSink sink; @@ -111,6 +114,7 @@ DownloadFileResult downloadFile( locked); return { + .storePathValid = true, .storePath = std::move(*storePath), .etag = res.etag, .effectiveUrl = res.effectiveUri, @@ -132,6 +136,8 @@ DownloadTarballResult downloadTarball( }); auto cached = getCache()->lookupExpired(*store, inAttrs); + if (!cached->storePathValid) + cached = std::nullopt; if (cached && !cached->expired) return { @@ -140,7 +146,7 @@ DownloadTarballResult downloadTarball( .immutableUrl = maybeGetStrAttr(cached->infoAttrs, "immutableUrl"), }; - auto res = downloadFile(store, url, name, locked, headers); + auto res = downloadFile(store, url, name, locked, headers, cached.has_value()); std::optional unpackedStorePath; time_t lastModified; @@ -151,6 +157,11 @@ DownloadTarballResult downloadTarball( } else { Path tmpDir = createTempDir(); AutoDelete autoDelete(tmpDir, true); + if (!res.storePathValid) { + debug("source etag didn't match unpacked etag, or server returned 304 with a different etag."); + res = downloadFile(store, url, name, locked); + assert(res.storePathValid); + } unpackTarfile(store->toRealPath(res.storePath), tmpDir); auto members = readDirectory(tmpDir); if (members.size() != 1) @@ -278,6 +289,7 @@ struct FileInputScheme : CurlInputScheme std::pair fetch(ref store, const Input & input) override { auto file = downloadFile(store, getStrAttr(input.attrs, "url"), input.getName(), false); + assert(file.storePathValid); return {std::move(file.storePath), input}; } }; diff --git a/src/libfetchers/tarball.hh b/src/libfetchers/tarball.hh index 9e6b50b31de..34378ec31c9 100644 --- a/src/libfetchers/tarball.hh +++ b/src/libfetchers/tarball.hh @@ -13,6 +13,7 @@ namespace nix::fetchers { struct DownloadFileResult { + bool storePathValid; StorePath storePath; std::string etag; std::string effectiveUrl; @@ -24,7 +25,8 @@ DownloadFileResult downloadFile( const std::string & url, const std::string & name, bool locked, - const Headers & headers = {}); + const Headers & headers = {}, + bool onlyIfChanged = false); struct DownloadTarballResult {