BinaryCacheStore::readFile(): Return a shared_ptr to a string

This allows readFile() to indicate that a file doesn't exist, and
might eliminate some large string copying.

8 files changed, 44 insertions, 22 deletions

diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc
index 473a0b2614bb..7c8fdfd0811c 100644
--- a/src/libstore/binary-cache-store.cc
+++ b/src/libstore/binary-cache-store.cc
@@ -119,7 +119,10 @@ NarInfo BinaryCacheStore::readNarInfo(const Path & storePath)
     }
 
     auto narInfoFile = narInfoFileFor(storePath);
-    auto narInfo = make_ref<NarInfo>(getFile(narInfoFile), narInfoFile);
+    auto data = getFile(narInfoFile);
+    if (!data)
+        throw InvalidPath(format("path ‘%s’ is not valid") % storePath);
+    auto narInfo = make_ref<NarInfo>(*data, narInfoFile);
     if (narInfo->path != storePath)
         throw Error(format("NAR info file for store path ‘%1%’ does not match ‘%2%’") % narInfo->path % storePath);
 
@@ -162,25 +165,27 @@ void BinaryCacheStore::narFromPath(const Path & storePath, Sink & sink)
 
     auto nar = getFile(res.url);
 
+    if (!nar) throw Error(format("file ‘%s’ missing from binary cache") % res.url);
+
     stats.narRead++;
-    stats.narReadCompressedBytes += nar.size();
+    stats.narReadCompressedBytes += nar->size();
 
     /* Decompress the NAR. FIXME: would be nice to have the remote
        side do this. */
     if (res.compression == "none")
         ;
     else if (res.compression == "xz")
-        nar = decompressXZ(nar);
+        nar = decompressXZ(*nar);
     else
         throw Error(format("unknown NAR compression type ‘%1%’") % nar);
 
-    stats.narReadBytes += nar.size();
+    stats.narReadBytes += nar->size();
 
-    printMsg(lvlTalkative, format("exporting path ‘%1%’ (%2% bytes)") % storePath % nar.size());
+    printMsg(lvlTalkative, format("exporting path ‘%1%’ (%2% bytes)") % storePath % nar->size());
 
-    assert(nar.size() % 8 == 0);
+    assert(nar->size() % 8 == 0);
 
-    sink((unsigned char *) nar.c_str(), nar.size());
+    sink((unsigned char *) nar->c_str(), nar->size());
 }
 
 void BinaryCacheStore::exportPath(const Path & storePath, bool sign, Sink & sink)
diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh
index 9e7b0ad9a384..55bba627825c 100644
--- a/src/libstore/binary-cache-store.hh
+++ b/src/libstore/binary-cache-store.hh
@@ -39,7 +39,9 @@ protected:
 
     virtual void upsertFile(const std::string & path, const std::string & data) = 0;
 
-    virtual std::string getFile(const std::string & path) = 0;
+    /* Return the contents of the specified file, or null if it
+       doesn't exist. */
+    virtual std::shared_ptr<std::string> getFile(const std::string & path) = 0;
 
 public:
 
diff --git a/src/libstore/builtins.cc b/src/libstore/builtins.cc
index c22c44f3c7e3..50417b644a02 100644
--- a/src/libstore/builtins.cc
+++ b/src/libstore/builtins.cc
@@ -20,6 +20,7 @@ void builtinFetchurl(const BasicDerivation & drv)
     options.showProgress = DownloadOptions::yes;
 
     auto data = makeDownloader()->download(url->second, options);
+    assert(data.data);
 
     auto out = drv.env.find("out");
     if (out == drv.env.end()) throw Error("attribute ‘url’ missing");
@@ -29,12 +30,12 @@ void builtinFetchurl(const BasicDerivation & drv)
 
     auto unpack = drv.env.find("unpack");
     if (unpack != drv.env.end() && unpack->second == "1") {
-        if (string(data.data, 0, 6) == string("\xfd" "7zXZ\0", 6))
-            data.data = decompressXZ(data.data);
-        StringSource source(data.data);
+        if (string(*data.data, 0, 6) == string("\xfd" "7zXZ\0", 6))
+            data.data = decompressXZ(*data.data);
+        StringSource source(*data.data);
         restorePath(storePath, source);
     } else
-        writeFile(storePath, data.data);
+        writeFile(storePath, *data.data);
 
     auto executable = drv.env.find("executable");
     if (executable != drv.env.end() && executable->second == "1") {
diff --git a/src/libstore/download.cc b/src/libstore/download.cc
index 7277751b48e7..8cd3ad741e10 100644
--- a/src/libstore/download.cc
+++ b/src/libstore/download.cc
@@ -29,7 +29,7 @@ std::string resolveUri(const std::string & uri)
 struct CurlDownloader : public Downloader
 {
     CURL * curl;
-    string data;
+    ref<std::string> data;
     string etag, status, expectedETag;
 
     struct curl_slist * requestHeaders;
@@ -41,7 +41,7 @@ struct CurlDownloader : public Downloader
     size_t writeCallback(void * contents, size_t size, size_t nmemb)
     {
         size_t realSize = size * nmemb;
-        data.append((char *) contents, realSize);
+        data->append((char *) contents, realSize);
         return realSize;
     }
 
@@ -110,6 +110,7 @@ struct CurlDownloader : public Downloader
     }
 
     CurlDownloader()
+        : data(make_ref<std::string>())
     {
         requestHeaders = 0;
 
@@ -156,7 +157,7 @@ struct CurlDownloader : public Downloader
             curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 0);
         }
 
-        data.clear();
+        data->clear();
 
         if (requestHeaders) {
             curl_slist_free_all(requestHeaders);
@@ -269,7 +270,7 @@ Path Downloader::downloadCached(ref<Store> store, const string & url_, bool unpa
             auto res = download(url, options);
 
             if (!res.cached)
-                storePath = store->addTextToStore(name, res.data, PathSet(), false);
+                storePath = store->addTextToStore(name, *res.data, PathSet(), false);
 
             assert(!storePath.empty());
             replaceSymlink(storePath, fileLink);
diff --git a/src/libstore/download.hh b/src/libstore/download.hh
index 5dd2d2c82dec..eb2b76678ac7 100644
--- a/src/libstore/download.hh
+++ b/src/libstore/download.hh
@@ -17,7 +17,8 @@ struct DownloadOptions
 struct DownloadResult
 {
     bool cached;
-    string data, etag;
+    string etag;
+    std::shared_ptr<std::string> data;
 };
 
 class Store;
diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc
index 8a719db150aa..6dcea1cbf090 100644
--- a/src/libstore/http-binary-cache-store.cc
+++ b/src/libstore/http-binary-cache-store.cc
@@ -58,12 +58,18 @@ protected:
         throw Error("uploading to an HTTP binary cache is not supported");
     }
 
-    std::string getFile(const std::string & path) override
+    std::shared_ptr<std::string> getFile(const std::string & path) override
     {
         auto downloader(downloaders.get());
         DownloadOptions options;
         options.showProgress = DownloadOptions::no;
-        return downloader->download(cacheUri + "/" + path, options).data;
+        try {
+            return downloader->download(cacheUri + "/" + path, options).data;
+        } catch (DownloadError & e) {
+            if (e.error == Downloader::NotFound || e.error == Downloader::Forbidden)
+                return 0;
+            throw;
+        }
     }
 
 };
diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc
index efd6d47254f2..7968c98b9558 100644
--- a/src/libstore/local-binary-cache-store.cc
+++ b/src/libstore/local-binary-cache-store.cc
@@ -22,7 +22,7 @@ protected:
 
     void upsertFile(const std::string & path, const std::string & data) override;
 
-    std::string getFile(const std::string & path) override;
+    std::shared_ptr<std::string> getFile(const std::string & path) override;
 
 };
 
@@ -59,9 +59,14 @@ void LocalBinaryCacheStore::upsertFile(const std::string & path, const std::stri
     atomicWrite(binaryCacheDir + "/" + path, data);
 }
 
-std::string LocalBinaryCacheStore::getFile(const std::string & path)
+std::shared_ptr<std::string> LocalBinaryCacheStore::getFile(const std::string & path)
 {
-    return readFile(binaryCacheDir + "/" + path);
+    try {
+        return std::make_shared<std::string>(readFile(binaryCacheDir + "/" + path));
+    } catch (SysError & e) {
+        if (e.errNo == ENOENT) return 0;
+        throw;
+    }
 }
 
 ref<Store> openLocalBinaryCacheStore(std::shared_ptr<Store> localStore,
diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh
index ae5631ba0b7c..8827654fafdd 100644
--- a/src/libstore/store-api.hh
+++ b/src/libstore/store-api.hh
@@ -511,6 +511,7 @@ ValidPathInfo decodeValidPathInfo(std::istream & str,
 
 MakeError(SubstError, Error)
 MakeError(BuildError, Error) /* denotes a permanent build failure */
+MakeError(InvalidPath, Error)
 
 
 }