From d1b0909894a302540f979d904dd378af1cad620c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 15 Apr 2016 15:11:34 +0200 Subject: 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. --- src/libstore/binary-cache-store.cc | 19 ++++++++++++------- src/libstore/binary-cache-store.hh | 4 +++- src/libstore/builtins.cc | 9 +++++---- src/libstore/download.cc | 9 +++++---- src/libstore/download.hh | 3 ++- src/libstore/http-binary-cache-store.cc | 10 ++++++++-- src/libstore/local-binary-cache-store.cc | 11 ++++++++--- src/libstore/store-api.hh | 1 + src/libutil/compression.cc | 6 +++--- src/libutil/compression.hh | 4 +++- src/nix-prefetch-url/nix-prefetch-url.cc | 4 ++-- 11 files changed, 52 insertions(+), 28 deletions(-) (limited to 'src') 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(getFile(narInfoFile), narInfoFile); + auto data = getFile(narInfoFile); + if (!data) + throw InvalidPath(format("path ‘%s’ is not valid") % storePath); + auto narInfo = make_ref(*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 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 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()) { 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, 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 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 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 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 LocalBinaryCacheStore::getFile(const std::string & path) { - return readFile(binaryCacheDir + "/" + path); + try { + return std::make_shared(readFile(binaryCacheDir + "/" + path)); + } catch (SysError & e) { + if (e.errNo == ENOENT) return 0; + throw; + } } ref openLocalBinaryCacheStore(std::shared_ptr 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) } diff --git a/src/libutil/compression.cc b/src/libutil/compression.cc index a3fa0dab737b..b047d305cfa6 100644 --- a/src/libutil/compression.cc +++ b/src/libutil/compression.cc @@ -55,7 +55,7 @@ std::string compressXZ(const std::string & in) } } -std::string decompressXZ(const std::string & in) +ref decompressXZ(const std::string & in) { LzmaStream strm; @@ -66,7 +66,7 @@ std::string decompressXZ(const std::string & in) lzma_action action = LZMA_RUN; uint8_t outbuf[BUFSIZ]; - string res; + ref res = make_ref(); strm().next_in = (uint8_t *) in.c_str(); strm().avail_in = in.size(); strm().next_out = outbuf; @@ -80,7 +80,7 @@ std::string decompressXZ(const std::string & in) lzma_ret ret = lzma_code(&strm(), action); if (strm().avail_out == 0 || ret == LZMA_STREAM_END) { - res.append((char *) outbuf, sizeof(outbuf) - strm().avail_out); + res->append((char *) outbuf, sizeof(outbuf) - strm().avail_out); strm().next_out = outbuf; strm().avail_out = sizeof(outbuf); } diff --git a/src/libutil/compression.hh b/src/libutil/compression.hh index eb1697fc4aa4..79a796db7756 100644 --- a/src/libutil/compression.hh +++ b/src/libutil/compression.hh @@ -1,11 +1,13 @@ #pragma once +#include "ref.hh" + #include namespace nix { std::string compressXZ(const std::string & in); -std::string decompressXZ(const std::string & in); +ref decompressXZ(const std::string & in); } diff --git a/src/nix-prefetch-url/nix-prefetch-url.cc b/src/nix-prefetch-url/nix-prefetch-url.cc index c65961a15720..64da10513711 100644 --- a/src/nix-prefetch-url/nix-prefetch-url.cc +++ b/src/nix-prefetch-url/nix-prefetch-url.cc @@ -162,7 +162,7 @@ int main(int argc, char * * argv) AutoDelete tmpDir(createTempDir(), true); Path tmpFile = (Path) tmpDir + "/tmp"; - writeFile(tmpFile, result.data); + writeFile(tmpFile, *result.data); /* Optionally unpack the file. */ if (unpack) { @@ -186,7 +186,7 @@ int main(int argc, char * * argv) /* FIXME: inefficient; addToStore() will also hash this. */ - hash = unpack ? hashPath(ht, tmpFile).first : hashString(ht, result.data); + hash = unpack ? hashPath(ht, tmpFile).first : hashString(ht, *result.data); if (expectedHash != Hash(ht) && expectedHash != hash) throw Error(format("hash mismatch for ‘%1%’") % uri); -- cgit 1.4.1