From 776eb97a4328ccfacbd4e795b4659b7367838db0 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 17 Jan 2022 19:28:42 +0100 Subject: [PATCH 1/3] serialise.hh: Use std::string_view --- src/libstore/build/derivation-goal.cc | 2 +- src/libutil/error.hh | 2 +- src/libutil/serialise.cc | 2 +- src/libutil/serialise.hh | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index d34e53fe8..151217b8b 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -278,7 +278,7 @@ void DerivationGoal::outputsSubstitutionTried() if (nrFailed > 0 && nrFailed > nrNoSubstituters + nrIncompleteClosure && !settings.tryFallback) { done(BuildResult::TransientFailure, - fmt("some substitutes for the outputs of derivation '%s' failed (usually happens due to networking issues); try '--fallback' to build derivation from source ", + Error("some substitutes for the outputs of derivation '%s' failed (usually happens due to networking issues); try '--fallback' to build derivation from source ", worker.store.printStorePath(drvPath))); return; } diff --git a/src/libutil/error.hh b/src/libutil/error.hh index ff58d3e00..6fe5e4857 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -137,7 +137,7 @@ public: { } template - BaseError(const std::string & fs, const Args & ... args) + explicit BaseError(const std::string & fs, const Args & ... args) : err { .level = lvlError, .msg = hintfmt(fs, args...) } { } diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index 16f3476c2..704359b4a 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -325,7 +325,7 @@ void writeString(std::string_view data, Sink & sink) } -Sink & operator << (Sink & sink, const string & s) +Sink & operator << (Sink & sink, std::string_view s) { writeString(s, sink); return sink; diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index 0fe6e8332..b3cfb06a2 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -317,10 +317,10 @@ inline Sink & operator << (Sink & sink, uint64_t n) return sink; } -Sink & operator << (Sink & sink, const string & s); +Sink & operator << (Sink & in, const Error & ex); +Sink & operator << (Sink & sink, std::string_view s); Sink & operator << (Sink & sink, const Strings & s); Sink & operator << (Sink & sink, const StringSet & s); -Sink & operator << (Sink & in, const Error & ex); MakeError(SerialisationError, Error); From 52ee7ec0028263f04e15c05256ca90fa62a0da66 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 17 Jan 2022 19:38:17 +0100 Subject: [PATCH 2/3] StringSource: Use std::string_view --- src/libutil/serialise.hh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index b3cfb06a2..e80fb9ecf 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -167,9 +167,9 @@ struct StringSink : Sink /* A source that reads data from a string. */ struct StringSource : Source { - const string & s; + std::string_view s; size_t pos; - StringSource(const string & _s) : s(_s), pos(0) { } + StringSource(std::string_view s) : s(s), pos(0) { } size_t read(char * data, size_t len) override; }; From d62a9390fcdc0f9e4971e5fab2f667567237b252 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 17 Jan 2022 22:20:05 +0100 Subject: [PATCH 3/3] Get rid of std::shared_ptr and ref These were needed back in the pre-C++11 era because we didn't have move semantics. But now we do. --- src/libfetchers/tarball.cc | 10 +++---- src/libstore/binary-cache-store.cc | 24 ++++++++--------- src/libstore/binary-cache-store.hh | 9 ++++--- src/libstore/build/local-derivation-goal.cc | 6 ++--- src/libstore/daemon.cc | 14 +++++----- src/libstore/export-import.cc | 6 ++--- src/libstore/filetransfer.cc | 20 +++++++------- src/libstore/filetransfer.hh | 8 +++--- src/libstore/http-binary-cache-store.cc | 8 +++--- src/libstore/legacy-ssh-store.cc | 2 +- src/libstore/local-fs-store.cc | 16 +++++------ src/libstore/local-fs-store.hh | 3 ++- src/libstore/local-store.cc | 4 +-- src/libstore/nar-accessor.cc | 8 +++--- src/libstore/nar-accessor.hh | 2 +- src/libstore/remote-fs-accessor.cc | 30 +++++++++++---------- src/libstore/remote-fs-accessor.hh | 3 +-- src/libstore/remote-store.cc | 2 +- src/libstore/s3-binary-cache-store.cc | 2 +- src/libstore/s3.hh | 4 ++- src/libstore/store-api.hh | 4 +-- src/libutil/compression.cc | 8 +++--- src/libutil/compression.hh | 4 +-- src/libutil/serialise.cc | 6 ++--- src/libutil/serialise.hh | 11 ++++---- src/libutil/tests/compression.cc | 28 +++++++++---------- src/libutil/util.cc | 4 +-- src/nix/add-to-store.cc | 6 ++--- src/nix/cat.cc | 2 +- src/nix/ls.cc | 2 +- src/nix/make-content-addressable.cc | 8 +++--- src/nix/profile.cc | 6 ++--- src/nix/upgrade-nix.cc | 2 +- 33 files changed, 138 insertions(+), 134 deletions(-) diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index 69d020748..c933475ca 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -67,18 +67,18 @@ DownloadFileResult downloadFile( storePath = std::move(cached->storePath); } else { StringSink sink; - dumpString(*res.data, sink); - auto hash = hashString(htSHA256, *res.data); + dumpString(res.data, sink); + auto hash = hashString(htSHA256, res.data); ValidPathInfo info { store->makeFixedOutputPath(FileIngestionMethod::Flat, hash, name), - hashString(htSHA256, *sink.s), + hashString(htSHA256, sink.s), }; - info.narSize = sink.s->size(); + info.narSize = sink.s.size(); info.ca = FixedOutputHash { .method = FileIngestionMethod::Flat, .hash = hash, }; - auto source = StringSource { *sink.s }; + auto source = StringSource(sink.s); store->addToStore(info, source, NoRepair, NoCheckSigs); storePath = std::move(info.path); } diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 13c086a46..8aec632e8 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -31,7 +31,7 @@ BinaryCacheStore::BinaryCacheStore(const Params & params) StringSink sink; sink << narVersionMagic1; - narMagic = *sink.s; + narMagic = sink.s; } void BinaryCacheStore::init() @@ -68,7 +68,7 @@ void BinaryCacheStore::upsertFile(const std::string & path, } void BinaryCacheStore::getFile(const std::string & path, - Callback> callback) noexcept + Callback> callback) noexcept { try { callback(getFile(path)); @@ -77,9 +77,9 @@ void BinaryCacheStore::getFile(const std::string & path, void BinaryCacheStore::getFile(const std::string & path, Sink & sink) { - std::promise> promise; + std::promise> promise; getFile(path, - {[&](std::future> result) { + {[&](std::future> result) { try { promise.set_value(result.get()); } catch (...) { @@ -89,15 +89,15 @@ void BinaryCacheStore::getFile(const std::string & path, Sink & sink) sink(*promise.get_future().get()); } -std::shared_ptr BinaryCacheStore::getFile(const std::string & path) +std::optional BinaryCacheStore::getFile(const std::string & path) { StringSink sink; try { getFile(path, sink); } catch (NoSuchBinaryCacheFile &) { - return nullptr; + return std::nullopt; } - return sink.s; + return std::move(sink.s); } std::string BinaryCacheStore::narInfoFileFor(const StorePath & storePath) @@ -367,7 +367,7 @@ void BinaryCacheStore::queryPathInfoUncached(const StorePath & storePath, auto callbackPtr = std::make_shared(std::move(callback)); getFile(narInfoFile, - {[=](std::future> fut) { + {[=](std::future> fut) { try { auto data = fut.get(); @@ -429,7 +429,7 @@ StorePath BinaryCacheStore::addTextToStore(const string & name, const string & s StringSink sink; dumpString(s, sink); - auto source = StringSource { *sink.s }; + StringSource source(sink.s); return addToStoreCommon(source, repair, CheckSigs, [&](HashResult nar) { ValidPathInfo info { path, nar.first }; info.narSize = nar.second; @@ -446,8 +446,8 @@ void BinaryCacheStore::queryRealisationUncached(const DrvOutput & id, auto callbackPtr = std::make_shared(std::move(callback)); - Callback> newCallback = { - [=](std::future> fut) { + Callback> newCallback = { + [=](std::future> fut) { try { auto data = fut.get(); if (!data) return (*callbackPtr)(nullptr); @@ -490,7 +490,7 @@ void BinaryCacheStore::addSignatures(const StorePath & storePath, const StringSe writeNarInfo(narInfo); } -std::shared_ptr BinaryCacheStore::getBuildLog(const StorePath & path) +std::optional BinaryCacheStore::getBuildLog(const StorePath & path) { auto drvPath = path; diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index 9815af591..46ff67c77 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -62,10 +62,11 @@ public: /* Fetch the specified file and call the specified callback with the result. A subclass may implement this asynchronously. */ - virtual void getFile(const std::string & path, - Callback> callback) noexcept; + virtual void getFile( + const std::string & path, + Callback> callback) noexcept; - std::shared_ptr getFile(const std::string & path); + std::optional getFile(const std::string & path); public: @@ -117,7 +118,7 @@ public: void addSignatures(const StorePath & storePath, const StringSet & sigs) override; - std::shared_ptr getBuildLog(const StorePath & path) override; + std::optional getBuildLog(const StorePath & path) override; }; diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index e4d2add73..0d0afea2d 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2226,8 +2226,8 @@ void LocalDerivationGoal::registerOutputs() StringSink sink; dumpPath(actualPath, sink); deletePath(actualPath); - sink.s = make_ref(rewriteStrings(*sink.s, outputRewrites)); - StringSource source(*sink.s); + sink.s = rewriteStrings(sink.s, outputRewrites); + StringSource source(sink.s); restorePath(actualPath, source); } }; @@ -2295,7 +2295,7 @@ void LocalDerivationGoal::registerOutputs() StringSink sink; dumpPath(actualPath, sink); RewritingSink rsink2(oldHashPart, std::string(finalPath.hashPart()), nextSink); - rsink2(*sink.s); + rsink2(sink.s); rsink2.flush(); }); Path tmpPath = actualPath + ".tmp"; diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 59325da79..5b817c587 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -69,7 +69,7 @@ struct TunnelLogger : public Logger StringSink buf; buf << STDERR_NEXT << (fs.s + "\n"); - enqueueMsg(*buf.s); + enqueueMsg(buf.s); } void logEI(const ErrorInfo & ei) override @@ -81,7 +81,7 @@ struct TunnelLogger : public Logger StringSink buf; buf << STDERR_NEXT << oss.str(); - enqueueMsg(*buf.s); + enqueueMsg(buf.s); } /* startWork() means that we're starting an operation for which we @@ -129,7 +129,7 @@ struct TunnelLogger : public Logger StringSink buf; buf << STDERR_START_ACTIVITY << act << lvl << type << s << fields << parent; - enqueueMsg(*buf.s); + enqueueMsg(buf.s); } void stopActivity(ActivityId act) override @@ -137,7 +137,7 @@ struct TunnelLogger : public Logger if (GET_PROTOCOL_MINOR(clientVersion) < 20) return; StringSink buf; buf << STDERR_STOP_ACTIVITY << act; - enqueueMsg(*buf.s); + enqueueMsg(buf.s); } void result(ActivityId act, ResultType type, const Fields & fields) override @@ -145,7 +145,7 @@ struct TunnelLogger : public Logger if (GET_PROTOCOL_MINOR(clientVersion) < 20) return; StringSink buf; buf << STDERR_RESULT << act << type << fields; - enqueueMsg(*buf.s); + enqueueMsg(buf.s); } }; @@ -852,14 +852,14 @@ static void performOp(TunnelLogger * logger, ref store, else { std::unique_ptr source; + StringSink saved; if (GET_PROTOCOL_MINOR(clientVersion) >= 21) source = std::make_unique(from, to); else { - StringSink saved; TeeSource tee { from, saved }; ParseSink ether; parseDump(ether, tee); - source = std::make_unique(std::move(*saved.s)); + source = std::make_unique(saved.s); } logger->startWork(); diff --git a/src/libstore/export-import.cc b/src/libstore/export-import.cc index 02c839520..9875da909 100644 --- a/src/libstore/export-import.cc +++ b/src/libstore/export-import.cc @@ -75,20 +75,20 @@ StorePaths Store::importPaths(Source & source, CheckSigsFlag checkSigs) auto references = worker_proto::read(*this, source, Phantom {}); auto deriver = readString(source); - auto narHash = hashString(htSHA256, *saved.s); + auto narHash = hashString(htSHA256, saved.s); ValidPathInfo info { path, narHash }; if (deriver != "") info.deriver = parseStorePath(deriver); info.references = references; - info.narSize = saved.s->size(); + info.narSize = saved.s.size(); // Ignore optional legacy signature. if (readInt(source) == 1) readString(source); // Can't use underlying source, which would have been exhausted - auto source = StringSource { *saved.s }; + auto source = StringSource(saved.s); addToStore(info, source, NoRepair, checkSigs); res.push_back(info.path); diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 17980ab22..6b62311cf 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -106,7 +106,7 @@ struct curlFileTransfer : public FileTransfer this->request.dataCallback(data); } } else - this->result.data->append(data); + this->result.data.append(data); }) { if (!request.expectedETag.empty()) @@ -195,7 +195,7 @@ struct curlFileTransfer : public FileTransfer std::smatch match; if (std::regex_match(line, match, statusLine)) { result.etag = ""; - result.data = std::make_shared(); + result.data.clear(); result.bodySize = 0; statusMsg = trim(match[1]); acceptRanges = false; @@ -340,7 +340,7 @@ struct curlFileTransfer : public FileTransfer if (writtenToSink) curl_easy_setopt(req, CURLOPT_RESUME_FROM_LARGE, writtenToSink); - result.data = std::make_shared(); + result.data.clear(); result.bodySize = 0; } @@ -434,21 +434,21 @@ struct curlFileTransfer : public FileTransfer attempt++; - std::shared_ptr response; + std::optional response; if (errorSink) - response = errorSink->s; + response = std::move(errorSink->s); auto exc = code == CURLE_ABORTED_BY_CALLBACK && _isInterrupted - ? FileTransferError(Interrupted, response, "%s of '%s' was interrupted", request.verb(), request.uri) + ? FileTransferError(Interrupted, std::move(response), "%s of '%s' was interrupted", request.verb(), request.uri) : httpStatus != 0 ? FileTransferError(err, - response, + std::move(response), fmt("unable to %s '%s': HTTP error %d ('%s')", request.verb(), request.uri, httpStatus, statusMsg) + (code == CURLE_OK ? "" : fmt(" (curl error: %s)", curl_easy_strerror(code))) ) : FileTransferError(err, - response, + std::move(response), fmt("unable to %s '%s': %s (%d)", request.verb(), request.uri, curl_easy_strerror(code), code)); @@ -705,7 +705,7 @@ struct curlFileTransfer : public FileTransfer FileTransferResult res; if (!s3Res.data) throw FileTransferError(NotFound, nullptr, "S3 object '%s' does not exist", request.uri); - res.data = s3Res.data; + res.data = std::move(*s3Res.data); callback(std::move(res)); #else throw nix::Error("cannot download '%s' because Nix is not built with S3 support", request.uri); @@ -859,7 +859,7 @@ void FileTransfer::download(FileTransferRequest && request, Sink & sink) } template -FileTransferError::FileTransferError(FileTransfer::Error error, std::shared_ptr response, const Args & ... args) +FileTransferError::FileTransferError(FileTransfer::Error error, std::optional response, const Args & ... args) : Error(args...), error(error), response(response) { const auto hf = hintfmt(args...); diff --git a/src/libstore/filetransfer.hh b/src/libstore/filetransfer.hh index 45d9ccf89..3e61b23b1 100644 --- a/src/libstore/filetransfer.hh +++ b/src/libstore/filetransfer.hh @@ -59,7 +59,7 @@ struct FileTransferRequest unsigned int baseRetryTimeMs = 250; ActivityId parentAct; bool decompress = true; - std::shared_ptr data; + std::optional data; std::string mimeType; std::function dataCallback; @@ -77,7 +77,7 @@ struct FileTransferResult bool cached = false; std::string etag; std::string effectiveUri; - std::shared_ptr data; + std::string data; uint64_t bodySize = 0; }; @@ -119,10 +119,10 @@ class FileTransferError : public Error { public: FileTransfer::Error error; - std::shared_ptr response; // intentionally optional + std::optional response; // intentionally optional template - FileTransferError(FileTransfer::Error error, std::shared_ptr response, const Args & ... args); + FileTransferError(FileTransfer::Error error, std::optional response, const Args & ... args); virtual const char* sname() const override { return "FileTransferError"; } }; diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc index 605ec4b28..3cb5efdbf 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -126,7 +126,7 @@ protected: const std::string & mimeType) override { auto req = makeRequest(path); - req.data = std::make_shared(StreamToSourceAdapter(istream).drain()); + req.data = StreamToSourceAdapter(istream).drain(); req.mimeType = mimeType; try { getFileTransfer()->upload(req); @@ -159,7 +159,7 @@ protected: } void getFile(const std::string & path, - Callback> callback) noexcept override + Callback> callback) noexcept override { checkEnabled(); @@ -170,10 +170,10 @@ protected: getFileTransfer()->enqueueFileTransfer(request, {[callbackPtr, this](std::future result) { try { - (*callbackPtr)(result.get().data); + (*callbackPtr)(std::move(result.get().data)); } catch (FileTransferError & e) { if (e.error == FileTransfer::NotFound || e.error == FileTransfer::Forbidden) - return (*callbackPtr)(std::shared_ptr()); + return (*callbackPtr)({}); maybeDisable(); callbackPtr->rethrow(); } catch (...) { diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 4861d185e..f8b2662af 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -94,7 +94,7 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor conn->sshConn->in.close(); auto msg = conn->from.drain(); throw Error("'nix-store --serve' protocol mismatch from '%s', got '%s'", - host, chomp(*saved.s + msg)); + host, chomp(saved.s + msg)); } conn->remoteVersion = readInt(conn->from); if (GET_PROTOCOL_MAJOR(conn->remoteVersion) != 0x200) diff --git a/src/libstore/local-fs-store.cc b/src/libstore/local-fs-store.cc index 6de13c73a..c933251db 100644 --- a/src/libstore/local-fs-store.cc +++ b/src/libstore/local-fs-store.cc @@ -87,34 +87,32 @@ void LocalFSStore::narFromPath(const StorePath & path, Sink & sink) const string LocalFSStore::drvsLogDir = "drvs"; - - -std::shared_ptr LocalFSStore::getBuildLog(const StorePath & path_) +std::optional LocalFSStore::getBuildLog(const StorePath & path_) { auto path = path_; if (!path.isDerivation()) { try { auto info = queryPathInfo(path); - if (!info->deriver) return nullptr; + if (!info->deriver) return std::nullopt; path = *info->deriver; } catch (InvalidPath &) { - return nullptr; + return std::nullopt; } } - auto baseName = std::string(baseNameOf(printStorePath(path))); + auto baseName = path.to_string(); for (int j = 0; j < 2; j++) { Path logPath = j == 0 - ? fmt("%s/%s/%s/%s", logDir, drvsLogDir, string(baseName, 0, 2), string(baseName, 2)) + ? fmt("%s/%s/%s/%s", logDir, drvsLogDir, baseName.substr(0, 2), baseName.substr(2)) : fmt("%s/%s/%s", logDir, drvsLogDir, baseName); Path logBz2Path = logPath + ".bz2"; if (pathExists(logPath)) - return std::make_shared(readFile(logPath)); + return readFile(logPath); else if (pathExists(logBz2Path)) { try { @@ -124,7 +122,7 @@ std::shared_ptr LocalFSStore::getBuildLog(const StorePath & path_) } - return nullptr; + return std::nullopt; } } diff --git a/src/libstore/local-fs-store.hh b/src/libstore/local-fs-store.hh index f8b19d00d..e44b27cc2 100644 --- a/src/libstore/local-fs-store.hh +++ b/src/libstore/local-fs-store.hh @@ -45,7 +45,8 @@ public: return getRealStoreDir() + "/" + std::string(storePath, storeDir.size() + 1); } - std::shared_ptr getBuildLog(const StorePath & path) override; + std::optional getBuildLog(const StorePath & path) override; + }; } diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 9ebdfd6ed..d3cebe720 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1461,12 +1461,12 @@ StorePath LocalStore::addTextToStore(const string & name, const string & s, StringSink sink; dumpString(s, sink); - auto narHash = hashString(htSHA256, *sink.s); + auto narHash = hashString(htSHA256, sink.s); optimisePath(realPath, repair); ValidPathInfo info { dstPath, narHash }; - info.narSize = sink.s->size(); + info.narSize = sink.s.size(); info.references = references; info.ca = TextHash { .hash = hash }; registerValidPath(info); diff --git a/src/libstore/nar-accessor.cc b/src/libstore/nar-accessor.cc index 784ebb719..7d27d7667 100644 --- a/src/libstore/nar-accessor.cc +++ b/src/libstore/nar-accessor.cc @@ -28,7 +28,7 @@ struct NarMember struct NarAccessor : public FSAccessor { - std::shared_ptr nar; + std::optional nar; GetNarBytes getNarBytes; @@ -104,7 +104,7 @@ struct NarAccessor : public FSAccessor } }; - NarAccessor(ref nar) : nar(nar) + NarAccessor(std::string && _nar) : nar(_nar) { StringSource source(*nar); NarIndexer indexer(*this, source); @@ -224,9 +224,9 @@ struct NarAccessor : public FSAccessor } }; -ref makeNarAccessor(ref nar) +ref makeNarAccessor(std::string && nar) { - return make_ref(nar); + return make_ref(std::move(nar)); } ref makeNarAccessor(Source & source) diff --git a/src/libstore/nar-accessor.hh b/src/libstore/nar-accessor.hh index 8af1272f6..c2241a04c 100644 --- a/src/libstore/nar-accessor.hh +++ b/src/libstore/nar-accessor.hh @@ -10,7 +10,7 @@ struct Source; /* Return an object that provides access to the contents of a NAR file. */ -ref makeNarAccessor(ref nar); +ref makeNarAccessor(std::string && nar); ref makeNarAccessor(Source & source); diff --git a/src/libstore/remote-fs-accessor.cc b/src/libstore/remote-fs-accessor.cc index f43456f0b..0ce335646 100644 --- a/src/libstore/remote-fs-accessor.cc +++ b/src/libstore/remote-fs-accessor.cc @@ -22,9 +22,18 @@ Path RemoteFSAccessor::makeCacheFile(std::string_view hashPart, const std::strin return fmt("%s/%s.%s", cacheDir, hashPart, ext); } -void RemoteFSAccessor::addToCache(std::string_view hashPart, const std::string & nar, - ref narAccessor) +ref RemoteFSAccessor::addToCache(std::string_view hashPart, std::string && nar) { + if (cacheDir != "") { + try { + /* FIXME: do this asynchronously. */ + writeFile(makeCacheFile(hashPart, "nar"), nar); + } catch (...) { + ignoreException(); + } + } + + auto narAccessor = makeNarAccessor(std::move(nar)); nars.emplace(hashPart, narAccessor); if (cacheDir != "") { @@ -33,14 +42,12 @@ void RemoteFSAccessor::addToCache(std::string_view hashPart, const std::string & JSONPlaceholder jsonRoot(str); listNar(jsonRoot, narAccessor, "", true); writeFile(makeCacheFile(hashPart, "ls"), str.str()); - - /* FIXME: do this asynchronously. */ - writeFile(makeCacheFile(hashPart, "nar"), nar); - } catch (...) { ignoreException(); } } + + return narAccessor; } std::pair, Path> RemoteFSAccessor::fetch(const Path & path_, bool requireValidPath) @@ -55,7 +62,6 @@ std::pair, Path> RemoteFSAccessor::fetch(const Path & path_, boo auto i = nars.find(std::string(storePath.hashPart())); if (i != nars.end()) return {i->second, restPath}; - StringSink sink; std::string listing; Path cacheFile; @@ -86,19 +92,15 @@ std::pair, Path> RemoteFSAccessor::fetch(const Path & path_, boo } catch (SysError &) { } try { - *sink.s = nix::readFile(cacheFile); - - auto narAccessor = makeNarAccessor(sink.s); + auto narAccessor = makeNarAccessor(nix::readFile(cacheFile)); nars.emplace(storePath.hashPart(), narAccessor); return {narAccessor, restPath}; - } catch (SysError &) { } } + StringSink sink; store->narFromPath(storePath, sink); - auto narAccessor = makeNarAccessor(sink.s); - addToCache(storePath.hashPart(), *sink.s, narAccessor); - return {narAccessor, restPath}; + return {addToCache(storePath.hashPart(), std::move(sink.s)), restPath}; } FSAccessor::Stat RemoteFSAccessor::stat(const Path & path) diff --git a/src/libstore/remote-fs-accessor.hh b/src/libstore/remote-fs-accessor.hh index 594852d0e..99f5544ef 100644 --- a/src/libstore/remote-fs-accessor.hh +++ b/src/libstore/remote-fs-accessor.hh @@ -20,8 +20,7 @@ class RemoteFSAccessor : public FSAccessor Path makeCacheFile(std::string_view hashPart, const std::string & ext); - void addToCache(std::string_view hashPart, const std::string & nar, - ref narAccessor); + ref addToCache(std::string_view hashPart, std::string && nar); public: diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 57cc260b0..6886103e1 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -172,7 +172,7 @@ void RemoteStore::initConnection(Connection & conn) it. */ conn.closeWrite(); auto msg = conn.from.drain(); - throw Error("protocol mismatch, got '%s'", chomp(*saved.s + msg)); + throw Error("protocol mismatch, got '%s'", chomp(saved.s + msg)); } conn.from >> conn.daemonVersion; diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 7accad7f4..a024e971d 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -385,7 +385,7 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStoreConfig, public virtual auto compress = [&](std::string compression) { auto compressed = nix::compress(compression, StreamToSourceAdapter(istream).drain()); - return std::make_shared(std::move(*compressed)); + return std::make_shared(std::move(compressed)); }; if (narinfoCompression != "" && hasSuffix(path, ".narinfo")) diff --git a/src/libstore/s3.hh b/src/libstore/s3.hh index 2042bffcf..3f55c74db 100644 --- a/src/libstore/s3.hh +++ b/src/libstore/s3.hh @@ -4,6 +4,8 @@ #include "ref.hh" +#include + namespace Aws { namespace Client { class ClientConfiguration; } } namespace Aws { namespace S3 { class S3Client; } } @@ -20,7 +22,7 @@ struct S3Helper struct FileTransferResult { - std::shared_ptr data; + std::optional data; unsigned int durationMs; }; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 3dd446f23..3567dcd1c 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -724,8 +724,8 @@ public: /* Return the build log of the specified store path, if available, or null otherwise. */ - virtual std::shared_ptr getBuildLog(const StorePath & path) - { return nullptr; } + virtual std::optional getBuildLog(const StorePath & path) + { return std::nullopt; } /* Hack to allow long-running processes like hydra-queue-runner to occasionally flush their path info cache. */ diff --git a/src/libutil/compression.cc b/src/libutil/compression.cc index f80ca664c..89180e7a7 100644 --- a/src/libutil/compression.cc +++ b/src/libutil/compression.cc @@ -190,13 +190,13 @@ struct BrotliDecompressionSink : ChunkedCompressionSink } }; -ref decompress(const std::string & method, const std::string & in) +std::string decompress(const std::string & method, std::string_view in) { StringSink ssink; auto sink = makeDecompressionSink(method, ssink); (*sink)(in); sink->finish(); - return ssink.s; + return std::move(ssink.s); } std::unique_ptr makeDecompressionSink(const std::string & method, Sink & nextSink) @@ -281,13 +281,13 @@ ref makeCompressionSink(const std::string & method, Sink & next throw UnknownCompressionMethod("unknown compression method '%s'", method); } -ref compress(const std::string & method, const std::string & in, const bool parallel, int level) +std::string compress(const std::string & method, std::string_view in, const bool parallel, int level) { StringSink ssink; auto sink = makeCompressionSink(method, ssink, parallel, level); (*sink)(in); sink->finish(); - return ssink.s; + return std::move(ssink.s); } } diff --git a/src/libutil/compression.hh b/src/libutil/compression.hh index 9b1e4a9d4..c470b82a5 100644 --- a/src/libutil/compression.hh +++ b/src/libutil/compression.hh @@ -15,11 +15,11 @@ struct CompressionSink : BufferedSink, FinishSink using FinishSink::finish; }; -ref decompress(const std::string & method, const std::string & in); +std::string decompress(const std::string & method, std::string_view in); std::unique_ptr makeDecompressionSink(const std::string & method, Sink & nextSink); -ref compress(const std::string & method, const std::string & in, const bool parallel = false, int level = -1); +std::string compress(const std::string & method, std::string_view in, const bool parallel = false, int level = -1); ref makeCompressionSink(const std::string & method, Sink & nextSink, const bool parallel = false, int level = -1); diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index 704359b4a..fe703de06 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -110,7 +110,7 @@ std::string Source::drain() { StringSink s; drainInto(s); - return *s.s; + return std::move(s.s); } @@ -450,11 +450,11 @@ Error readError(Source & source) void StringSink::operator () (std::string_view data) { static bool warned = false; - if (!warned && s->size() > threshold) { + if (!warned && s.size() > threshold) { warnLargeDump(); warned = true; } - s->append(data); + s.append(data); } size_t ChainSource::read(char * data, size_t len) diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index e80fb9ecf..fdd35aa00 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -154,12 +154,13 @@ private: /* A sink that writes data to a string. */ struct StringSink : Sink { - ref s; - StringSink() : s(make_ref()) { }; - explicit StringSink(const size_t reservedSize) : s(make_ref()) { - s->reserve(reservedSize); + std::string s; + StringSink() { } + explicit StringSink(const size_t reservedSize) + { + s.reserve(reservedSize); }; - StringSink(ref s) : s(s) { }; + StringSink(std::string && s) : s(std::move(s)) { }; void operator () (std::string_view data) override; }; diff --git a/src/libutil/tests/compression.cc b/src/libutil/tests/compression.cc index 2efa3266b..bbbf3500f 100644 --- a/src/libutil/tests/compression.cc +++ b/src/libutil/tests/compression.cc @@ -12,17 +12,17 @@ namespace nix { } TEST(compress, noneMethodDoesNothingToTheInput) { - ref o = compress("none", "this-is-a-test"); + auto o = compress("none", "this-is-a-test"); - ASSERT_EQ(*o, "this-is-a-test"); + ASSERT_EQ(o, "this-is-a-test"); } TEST(decompress, decompressNoneCompressed) { auto method = "none"; auto str = "slfja;sljfklsa;jfklsjfkl;sdjfkl;sadjfkl;sdjf;lsdfjsadlf"; - ref o = decompress(method, str); + auto o = decompress(method, str); - ASSERT_EQ(*o, str); + ASSERT_EQ(o, str); } TEST(decompress, decompressEmptyCompressed) { @@ -30,33 +30,33 @@ namespace nix { // (Content-Encoding == ""). auto method = ""; auto str = "slfja;sljfklsa;jfklsjfkl;sdjfkl;sadjfkl;sdjf;lsdfjsadlf"; - ref o = decompress(method, str); + auto o = decompress(method, str); - ASSERT_EQ(*o, str); + ASSERT_EQ(o, str); } TEST(decompress, decompressXzCompressed) { auto method = "xz"; auto str = "slfja;sljfklsa;jfklsjfkl;sdjfkl;sadjfkl;sdjf;lsdfjsadlf"; - ref o = decompress(method, *compress(method, str)); + auto o = decompress(method, compress(method, str)); - ASSERT_EQ(*o, str); + ASSERT_EQ(o, str); } TEST(decompress, decompressBzip2Compressed) { auto method = "bzip2"; auto str = "slfja;sljfklsa;jfklsjfkl;sdjfkl;sadjfkl;sdjf;lsdfjsadlf"; - ref o = decompress(method, *compress(method, str)); + auto o = decompress(method, compress(method, str)); - ASSERT_EQ(*o, str); + ASSERT_EQ(o, str); } TEST(decompress, decompressBrCompressed) { auto method = "br"; auto str = "slfja;sljfklsa;jfklsjfkl;sdjfkl;sadjfkl;sdjf;lsdfjsadlf"; - ref o = decompress(method, *compress(method, str)); + auto o = decompress(method, compress(method, str)); - ASSERT_EQ(*o, str); + ASSERT_EQ(o, str); } TEST(decompress, decompressInvalidInputThrowsCompressionError) { @@ -77,7 +77,7 @@ namespace nix { (*sink)(inputString); sink->finish(); - ASSERT_STREQ((*strSink.s).c_str(), inputString); + ASSERT_STREQ(strSink.s.c_str(), inputString); } TEST(makeCompressionSink, compressAndDecompress) { @@ -90,7 +90,7 @@ namespace nix { sink->finish(); decompressionSink->finish(); - ASSERT_STREQ((*strSink.s).c_str(), inputString); + ASSERT_STREQ(strSink.s.c_str(), inputString); } } diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 9edd69c64..f43161e33 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -670,7 +670,7 @@ string drainFD(int fd, bool block, const size_t reserveSize) { StringSink sink(reserveSize); drainFD(fd, sink, block); - return std::move(*sink.s); + return std::move(sink.s); } @@ -1055,7 +1055,7 @@ std::pair runProgram(RunOptions && options) status = e.status; } - return {status, std::move(*sink.s)}; + return {status, std::move(sink.s)}; } void runProgram2(const RunOptions & options) diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc index 2ae042789..5168413d2 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -32,7 +32,7 @@ struct CmdAddToStore : MixDryRun, StoreCommand StringSink sink; dumpPath(path, sink); - auto narHash = hashString(htSHA256, *sink.s); + auto narHash = hashString(htSHA256, sink.s); Hash hash = narHash; if (ingestionMethod == FileIngestionMethod::Flat) { @@ -45,14 +45,14 @@ struct CmdAddToStore : MixDryRun, StoreCommand store->makeFixedOutputPath(ingestionMethod, hash, *namePart), narHash, }; - info.narSize = sink.s->size(); + info.narSize = sink.s.size(); info.ca = std::optional { FixedOutputHash { .method = ingestionMethod, .hash = hash, } }; if (!dryRun) { - auto source = StringSource { *sink.s }; + auto source = StringSource(sink.s); store->addToStore(info, source); } diff --git a/src/nix/cat.cc b/src/nix/cat.cc index e28ee3c50..6420a0f79 100644 --- a/src/nix/cat.cc +++ b/src/nix/cat.cc @@ -78,7 +78,7 @@ struct CmdCatNar : StoreCommand, MixCat void run(ref store) override { - cat(makeNarAccessor(make_ref(readFile(narPath)))); + cat(makeNarAccessor(readFile(narPath))); } }; diff --git a/src/nix/ls.cc b/src/nix/ls.cc index c1dc9a95b..07554994b 100644 --- a/src/nix/ls.cc +++ b/src/nix/ls.cc @@ -157,7 +157,7 @@ struct CmdLsNar : Command, MixLs void run() override { - list(makeNarAccessor(make_ref(readFile(narPath)))); + list(makeNarAccessor(readFile(narPath))); } }; diff --git a/src/nix/make-content-addressable.cc b/src/nix/make-content-addressable.cc index 12f303a10..2e75a3b61 100644 --- a/src/nix/make-content-addressable.cc +++ b/src/nix/make-content-addressable.cc @@ -61,10 +61,10 @@ struct CmdMakeContentAddressable : StorePathsCommand, MixJSON } } - *sink.s = rewriteStrings(*sink.s, rewrites); + sink.s = rewriteStrings(sink.s, rewrites); HashModuloSink hashModuloSink(htSHA256, oldHashPart); - hashModuloSink(*sink.s); + hashModuloSink(sink.s); auto narHash = hashModuloSink.finish().first; @@ -74,7 +74,7 @@ struct CmdMakeContentAddressable : StorePathsCommand, MixJSON }; info.references = std::move(references); if (hasSelfReference) info.references.insert(info.path); - info.narSize = sink.s->size(); + info.narSize = sink.s.size(); info.ca = FixedOutputHash { .method = FileIngestionMethod::Recursive, .hash = info.narHash, @@ -85,7 +85,7 @@ struct CmdMakeContentAddressable : StorePathsCommand, MixJSON auto source = sinkToSource([&](Sink & nextSink) { RewritingSink rsink2(oldHashPart, std::string(info.path.hashPart()), nextSink); - rsink2(*sink.s); + rsink2(sink.s); rsink2.flush(); }); diff --git a/src/nix/profile.cc b/src/nix/profile.cc index 96a20f673..9b7c999af 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -157,17 +157,17 @@ struct ProfileManifest StringSink sink; dumpPath(tempDir, sink); - auto narHash = hashString(htSHA256, *sink.s); + auto narHash = hashString(htSHA256, sink.s); ValidPathInfo info { store->makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, "profile", references), narHash, }; info.references = std::move(references); - info.narSize = sink.s->size(); + info.narSize = sink.s.size(); info.ca = FixedOutputHash { .method = FileIngestionMethod::Recursive, .hash = info.narHash }; - auto source = StringSource { *sink.s }; + StringSource source(sink.s); store->addToStore(info, source); return std::move(info.path); diff --git a/src/nix/upgrade-nix.cc b/src/nix/upgrade-nix.cc index 9cd567896..17a5a77ee 100644 --- a/src/nix/upgrade-nix.cc +++ b/src/nix/upgrade-nix.cc @@ -140,7 +140,7 @@ struct CmdUpgradeNix : MixDryRun, StoreCommand auto state = std::make_unique(Strings(), store); auto v = state->allocValue(); - state->eval(state->parseExprFromString(*res.data, "/no-such-path"), *v); + state->eval(state->parseExprFromString(res.data, "/no-such-path"), *v); Bindings & bindings(*state->allocBindings(0)); auto v2 = findAlongAttrPath(*state, settings.thisSystem, bindings, *v).first;