Merge pull request #5932 from edolstra/remove-shared-strings

Remove shared strings

src/libfetchers/tarball.cc

@@ -67,18 +67,18 @@ DownloadFileResult downloadFile(
         storePath = std::move(cached->storePath);
     } else {
         StringSink sink;
-        dumpString(*res.data, sink);
+        dumpString(res.data, sink);
-        auto hash = hashString(htSHA256, *res.data);
+        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);
     }

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<std::shared_ptr<std::string>> callback) noexcept
+    Callback<std::optional<std::string>> 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<std::shared_ptr<std::string>> promise;
+    std::promise<std::optional<std::string>> promise;
     getFile(path,
-        {[&](std::future<std::shared_ptr<std::string>> result) {
+        {[&](std::future<std::optional<std::string>> 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<std::string> BinaryCacheStore::getFile(const std::string & path)
+std::optional<std::string> 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<decltype(callback)>(std::move(callback));
 
     getFile(narInfoFile,
-        {[=](std::future<std::shared_ptr<std::string>> fut) {
+        {[=](std::future<std::optional<std::string>> 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<decltype(callback)>(std::move(callback));
 
-    Callback<std::shared_ptr<std::string>> newCallback = {
+    Callback<std::optional<std::string>> newCallback = {
-        [=](std::future<std::shared_ptr<std::string>> fut) {
+        [=](std::future<std::optional<std::string>> 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<std::string> BinaryCacheStore::getBuildLog(const StorePath & path)
+std::optional<std::string> BinaryCacheStore::getBuildLog(const StorePath & path)
 {
     auto drvPath = path;
 

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,
+    virtual void getFile(
-        Callback<std::shared_ptr<std::string>> callback) noexcept;
+        const std::string & path,
+        Callback<std::optional<std::string>> callback) noexcept;
 
-    std::shared_ptr<std::string> getFile(const std::string & path);
+    std::optional<std::string> getFile(const std::string & path);
 
 public:
 
@@ -117,7 +118,7 @@ public:
 
     void addSignatures(const StorePath & storePath, const StringSet & sigs) override;
 
-    std::shared_ptr<std::string> getBuildLog(const StorePath & path) override;
+    std::optional<std::string> getBuildLog(const StorePath & path) override;
 
 };
 

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;
     }

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<std::string>(rewriteStrings(*sink.s, outputRewrites));
+                sink.s = rewriteStrings(sink.s, outputRewrites);
-                StringSource source(*sink.s);
+                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";

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> store,
 
         else {
             std::unique_ptr<Source> source;
+            StringSink saved;
             if (GET_PROTOCOL_MINOR(clientVersion) >= 21)
                 source = std::make_unique<TunnelSource>(from, to);
             else {
-                StringSink saved;
                 TeeSource tee { from, saved };
                 ParseSink ether;
                 parseDump(ether, tee);
-                source = std::make_unique<StringSource>(std::move(*saved.s));
+                source = std::make_unique<StringSource>(saved.s);
             }
 
             logger->startWork();

src/libstore/export-import.cc

@@ -75,20 +75,20 @@ StorePaths Store::importPaths(Source & source, CheckSigsFlag checkSigs)
 
         auto references = worker_proto::read(*this, source, Phantom<StorePathSet> {});
         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);

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<std::string>();
+                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<std::string>();
+            result.data.clear();
             result.bodySize = 0;
         }
 
@@ -434,21 +434,21 @@ struct curlFileTransfer : public FileTransfer
 
                 attempt++;
 
-                std::shared_ptr<std::string> response;
+                std::optional<std::string> 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<typename... Args>
-FileTransferError::FileTransferError(FileTransfer::Error error, std::shared_ptr<string> response, const Args & ... args)
+FileTransferError::FileTransferError(FileTransfer::Error error, std::optional<std::string> response, const Args & ... args)
     : Error(args...), error(error), response(response)
 {
     const auto hf = hintfmt(args...);

src/libstore/filetransfer.hh

@@ -59,7 +59,7 @@ struct FileTransferRequest
     unsigned int baseRetryTimeMs = 250;
     ActivityId parentAct;
     bool decompress = true;
-    std::shared_ptr<std::string> data;
+    std::optional<std::string> data;
     std::string mimeType;
     std::function<void(std::string_view data)> dataCallback;
 
@@ -77,7 +77,7 @@ struct FileTransferResult
     bool cached = false;
     std::string etag;
     std::string effectiveUri;
-    std::shared_ptr<std::string> data;
+    std::string data;
     uint64_t bodySize = 0;
 };
 
@@ -119,10 +119,10 @@ class FileTransferError : public Error
 {
 public:
     FileTransfer::Error error;
-    std::shared_ptr<string> response; // intentionally optional
+    std::optional<string> response; // intentionally optional
 
     template<typename... Args>
-    FileTransferError(FileTransfer::Error error, std::shared_ptr<string> response, const Args & ... args);
+    FileTransferError(FileTransfer::Error error, std::optional<string> response, const Args & ... args);
 
     virtual const char* sname() const override { return "FileTransferError"; }
 };

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<string>(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<std::shared_ptr<std::string>> callback) noexcept override
+        Callback<std::optional<std::string>> callback) noexcept override
     {
         checkEnabled();
 
@@ -170,10 +170,10 @@ protected:
         getFileTransfer()->enqueueFileTransfer(request,
             {[callbackPtr, this](std::future<FileTransferResult> 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<std::string>());
+                        return (*callbackPtr)({});
                     maybeDisable();
                     callbackPtr->rethrow();
                 } catch (...) {

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)

src/libstore/local-fs-store.cc

@@ -87,34 +87,32 @@ void LocalFSStore::narFromPath(const StorePath & path, Sink & sink)
 
 const string LocalFSStore::drvsLogDir = "drvs";
 
-
+std::optional<std::string> LocalFSStore::getBuildLog(const StorePath & path_)
-
-std::shared_ptr<std::string> 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<std::string>(readFile(logPath));
+            return readFile(logPath);
 
         else if (pathExists(logBz2Path)) {
             try {
@@ -124,7 +122,7 @@ std::shared_ptr<std::string> LocalFSStore::getBuildLog(const StorePath & path_)
 
     }
 
-    return nullptr;
+    return std::nullopt;
 }
 
 }

src/libstore/local-fs-store.hh

@@ -45,7 +45,8 @@ public:
         return getRealStoreDir() + "/" + std::string(storePath, storeDir.size() + 1);
     }
 
-    std::shared_ptr<std::string> getBuildLog(const StorePath & path) override;
+    std::optional<std::string> getBuildLog(const StorePath & path) override;
+
 };
 
 }

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);

src/libstore/nar-accessor.cc

@@ -28,7 +28,7 @@ struct NarMember
 
 struct NarAccessor : public FSAccessor
 {
-    std::shared_ptr<const std::string> nar;
+    std::optional<const std::string> nar;
 
     GetNarBytes getNarBytes;
 
@@ -104,7 +104,7 @@ struct NarAccessor : public FSAccessor
         }
     };
 
-    NarAccessor(ref<const std::string> 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<FSAccessor> makeNarAccessor(ref<const std::string> nar)
+ref<FSAccessor> makeNarAccessor(std::string && nar)
 {
-    return make_ref<NarAccessor>(nar);
+    return make_ref<NarAccessor>(std::move(nar));
 }
 
 ref<FSAccessor> makeNarAccessor(Source & source)

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<FSAccessor> makeNarAccessor(ref<const std::string> nar);
+ref<FSAccessor> makeNarAccessor(std::string && nar);
 
 ref<FSAccessor> makeNarAccessor(Source & source);
 

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<FSAccessor> RemoteFSAccessor::addToCache(std::string_view hashPart, std::string && nar)
-    ref<FSAccessor> narAccessor)
 {
+    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<ref<FSAccessor>, Path> RemoteFSAccessor::fetch(const Path & path_, bool requireValidPath)
@@ -55,7 +62,6 @@ std::pair<ref<FSAccessor>, 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<ref<FSAccessor>, Path> RemoteFSAccessor::fetch(const Path & path_, boo
         } catch (SysError &) { }
 
         try {
-            *sink.s = nix::readFile(cacheFile);
+            auto narAccessor = makeNarAccessor(nix::readFile(cacheFile));
-
-            auto narAccessor = makeNarAccessor(sink.s);
             nars.emplace(storePath.hashPart(), narAccessor);
             return {narAccessor, restPath};
-
         } catch (SysError &) { }
     }
 
+    StringSink sink;
     store->narFromPath(storePath, sink);
-    auto narAccessor = makeNarAccessor(sink.s);
+    return {addToCache(storePath.hashPart(), std::move(sink.s)), restPath};
-    addToCache(storePath.hashPart(), *sink.s, narAccessor);
-    return {narAccessor, restPath};
 }
 
 FSAccessor::Stat RemoteFSAccessor::stat(const Path & path)

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<FSAccessor> addToCache(std::string_view hashPart, std::string && nar);
-        ref<FSAccessor> narAccessor);
 
 public:
 

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;

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::stringstream>(std::move(*compressed));
+            return std::make_shared<std::stringstream>(std::move(compressed));
         };
 
         if (narinfoCompression != "" && hasSuffix(path, ".narinfo"))

src/libstore/s3.hh

@@ -4,6 +4,8 @@
 
 #include "ref.hh"
 
+#include <optional>
+
 namespace Aws { namespace Client { class ClientConfiguration; } }
 namespace Aws { namespace S3 { class S3Client; } }
 
@@ -20,7 +22,7 @@ struct S3Helper
 
     struct FileTransferResult
     {
-        std::shared_ptr<std::string> data;
+        std::optional<std::string> data;
         unsigned int durationMs;
     };
 

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<std::string> getBuildLog(const StorePath & path)
+    virtual std::optional<std::string> getBuildLog(const StorePath & path)
-    { return nullptr; }
+    { return std::nullopt; }
 
     /* Hack to allow long-running processes like hydra-queue-runner to
        occasionally flush their path info cache. */

src/libutil/compression.cc

@@ -190,13 +190,13 @@ struct BrotliDecompressionSink : ChunkedCompressionSink
     }
 };
 
-ref<std::string> 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<FinishSink> makeDecompressionSink(const std::string & method, Sink & nextSink)
@@ -281,13 +281,13 @@ ref<CompressionSink> makeCompressionSink(const std::string & method, Sink & next
         throw UnknownCompressionMethod("unknown compression method '%s'", method);
 }
 
-ref<std::string> 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);
 }
 
 }

src/libutil/compression.hh

@@ -15,11 +15,11 @@ struct CompressionSink : BufferedSink, FinishSink
     using FinishSink::finish;
 };
 
-ref<std::string> decompress(const std::string & method, const std::string & in);
+std::string decompress(const std::string & method, std::string_view in);
 
 std::unique_ptr<FinishSink> makeDecompressionSink(const std::string & method, Sink & nextSink);
 
-ref<std::string> 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<CompressionSink> makeCompressionSink(const std::string & method, Sink & nextSink, const bool parallel = false, int level = -1);
 

src/libutil/error.hh

@@ -137,7 +137,7 @@ public:
     { }
 
     template<typename... Args>
-    BaseError(const std::string & fs, const Args & ... args)
+    explicit BaseError(const std::string & fs, const Args & ... args)
         : err { .level = lvlError, .msg = hintfmt(fs, args...) }
     { }
 

src/libutil/serialise.cc

@@ -110,7 +110,7 @@ std::string Source::drain()
 {
     StringSink s;
     drainInto(s);
-    return *s.s;
+    return std::move(s.s);
 }
 
 
@@ -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;
@@ -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)

src/libutil/serialise.hh

@@ -154,12 +154,13 @@ private:
 /* A sink that writes data to a string. */
 struct StringSink : Sink
 {
-    ref<std::string> s;
+    std::string s;
-    StringSink() : s(make_ref<std::string>()) { };
+    StringSink() { }
-    explicit StringSink(const size_t reservedSize) : s(make_ref<std::string>()) {
+    explicit StringSink(const size_t reservedSize)
-      s->reserve(reservedSize);
+    {
+      s.reserve(reservedSize);
     };
-    StringSink(ref<std::string> s) : s(s) { };
+    StringSink(std::string && s) : s(std::move(s)) { };
     void operator () (std::string_view data) override;
 };
 
@@ -167,9 +168,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;
 };
 
@@ -317,10 +318,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);

src/libutil/tests/compression.cc

@@ -12,17 +12,17 @@ namespace nix {
     }
 
     TEST(compress, noneMethodDoesNothingToTheInput) {
-        ref<std::string> 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<std::string> 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<std::string> 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<std::string> 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<std::string> 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<std::string> 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);
     }
 
 }

src/libutil/util.cc

@@ -672,7 +672,7 @@ string drainFD(int fd, bool block, const size_t reserveSize)
     // not care very much about the extra memory.
     StringSink sink(reserveSize + 2);
     drainFD(fd, sink, block);
-    return std::move(*sink.s);
+    return std::move(sink.s);
 }
 
 
@@ -1057,7 +1057,7 @@ std::pair<int, std::string> runProgram(RunOptions && options)
         status = e.status;
     }
 
-    return {status, std::move(*sink.s)};
+    return {status, std::move(sink.s)};
 }
 
 void runProgram2(const RunOptions & options)

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);
         }
 

src/nix/cat.cc

@@ -78,7 +78,7 @@ struct CmdCatNar : StoreCommand, MixCat
 
     void run(ref<Store> store) override
     {
-        cat(makeNarAccessor(make_ref<std::string>(readFile(narPath))));
+        cat(makeNarAccessor(readFile(narPath)));
     }
 };
 

src/nix/ls.cc

@@ -157,7 +157,7 @@ struct CmdLsNar : Command, MixLs
 
     void run() override
     {
-        list(makeNarAccessor(make_ref<std::string>(readFile(narPath))));
+        list(makeNarAccessor(readFile(narPath)));
     }
 };
 

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();
             });
 

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);

src/nix/upgrade-nix.cc

@@ -140,7 +140,7 @@ struct CmdUpgradeNix : MixDryRun, StoreCommand
 
         auto state = std::make_unique<EvalState>(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;