Make the Store API more type-safe

Most functions now take a StorePath argument rather than a Path (which
is just an alias for std::string). The StorePath constructor ensures
that the path is syntactically correct (i.e. it looks like
<store-dir>/<base32-hash>-<name>). Similarly, functions like
buildPaths() now take a StorePathWithOutputs, rather than abusing Path
by adding a '!<outputs>' suffix.

Note that the StorePath type is implemented in Rust. This involves
some hackery to allow Rust values to be used directly in C++, via a
helper type whose destructor calls the Rust type's drop()
function. The main issue is the dynamic nature of C++ move semantics:
after we have moved a Rust value, we should not call the drop function
on the original value. So when we move a value, we set the original
value to bitwise zero, and the destructor only calls drop() if the
value is not bitwise zero. This should be sufficient for most types.

Also lots of minor cleanups to the C++ API to make it more modern
(e.g. using std::optional and std::string_view in some places).

1 files changed, 118 insertions, 108 deletions

diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
index 2a89b7c98..8c55da268 100644
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@@ -22,23 +22,22 @@
 namespace nix {
 
 
-Path readStorePath(Store & store, Source & from)
+template<> StorePathSet readStorePaths(const Store & store, Source & from)
 {
-    Path path = readString(from);
-    store.assertStorePath(path);
-    return path;
+    StorePathSet paths;
+    for (auto & i : readStrings<Strings>(from))
+        paths.insert(store.parseStorePath(i));
+    return paths;
 }
 
 
-template<class T> T readStorePaths(Store & store, Source & from)
+void writeStorePaths(const Store & store, Sink & out, const StorePathSet & paths)
 {
-    T paths = readStrings<T>(from);
-    for (auto & i : paths) store.assertStorePath(i);
-    return paths;
+    out << paths.size();
+    for (auto & i : paths)
+        out << store.printStorePath(i);
 }
 
-template PathSet readStorePaths(Store & store, Source & from);
-template Paths readStorePaths(Store & store, Source & from);
 
 /* TODO: Separate these store impls into different files, give them better names */
 RemoteStore::RemoteStore(const Params & params)
@@ -254,60 +253,62 @@ ConnectionHandle RemoteStore::getConnection()
 }
 
 
-bool RemoteStore::isValidPathUncached(const Path & path)
+bool RemoteStore::isValidPathUncached(const StorePath & path)
 {
     auto conn(getConnection());
-    conn->to << wopIsValidPath << path;
+    conn->to << wopIsValidPath << printStorePath(path);
     conn.processStderr();
     return readInt(conn->from);
 }
 
 
-PathSet RemoteStore::queryValidPaths(const PathSet & paths, SubstituteFlag maybeSubstitute)
+StorePathSet RemoteStore::queryValidPaths(const StorePathSet & paths, SubstituteFlag maybeSubstitute)
 {
     auto conn(getConnection());
     if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 12) {
-        PathSet res;
+        StorePathSet res;
         for (auto & i : paths)
-            if (isValidPath(i)) res.insert(i);
+            if (isValidPath(i)) res.insert(i.clone());
         return res;
     } else {
-        conn->to << wopQueryValidPaths << paths;
+        conn->to << wopQueryValidPaths;
+        writeStorePaths(*this, conn->to, paths);
         conn.processStderr();
-        return readStorePaths<PathSet>(*this, conn->from);
+        return readStorePaths<StorePathSet>(*this, conn->from);
     }
 }
 
 
-PathSet RemoteStore::queryAllValidPaths()
+StorePathSet RemoteStore::queryAllValidPaths()
 {
     auto conn(getConnection());
     conn->to << wopQueryAllValidPaths;
     conn.processStderr();
-    return readStorePaths<PathSet>(*this, conn->from);
+    return readStorePaths<StorePathSet>(*this, conn->from);
 }
 
 
-PathSet RemoteStore::querySubstitutablePaths(const PathSet & paths)
+StorePathSet RemoteStore::querySubstitutablePaths(const StorePathSet & paths)
 {
     auto conn(getConnection());
     if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 12) {
-        PathSet res;
+        StorePathSet res;
         for (auto & i : paths) {
-            conn->to << wopHasSubstitutes << i;
+            conn->to << wopHasSubstitutes << printStorePath(i);
             conn.processStderr();
-            if (readInt(conn->from)) res.insert(i);
+            if (readInt(conn->from)) res.insert(i.clone());
         }
         return res;
     } else {
-        conn->to << wopQuerySubstitutablePaths << paths;
+        conn->to << wopQuerySubstitutablePaths;
+        writeStorePaths(*this, conn->to, paths);
         conn.processStderr();
-        return readStorePaths<PathSet>(*this, conn->from);
+        return readStorePaths<StorePathSet>(*this, conn->from);
     }
 }
 
 
-void RemoteStore::querySubstitutablePathInfos(const PathSet & paths,
+void RemoteStore::querySubstitutablePathInfos(const StorePathSet & paths,
     SubstitutablePathInfos & infos)
 {
     if (paths.empty()) return;
@@ -318,29 +319,31 @@ void RemoteStore::querySubstitutablePathInfos(const PathSet & paths,
 
         for (auto & i : paths) {
             SubstitutablePathInfo info;
-            conn->to << wopQuerySubstitutablePathInfo << i;
+            conn->to << wopQuerySubstitutablePathInfo << printStorePath(i);
             conn.processStderr();
             unsigned int reply = readInt(conn->from);
             if (reply == 0) continue;
-            info.deriver = readString(conn->from);
-            if (info.deriver != "") assertStorePath(info.deriver);
-            info.references = readStorePaths<PathSet>(*this, conn->from);
+            auto deriver = readString(conn->from);
+            if (deriver != "")
+                info.deriver = parseStorePath(deriver);
+            info.references = readStorePaths<StorePathSet>(*this, conn->from);
             info.downloadSize = readLongLong(conn->from);
             info.narSize = readLongLong(conn->from);
-            infos[i] = info;
+            infos.insert_or_assign(i.clone(), std::move(info));
         }
 
     } else {
 
-        conn->to << wopQuerySubstitutablePathInfos << paths;
+        conn->to << wopQuerySubstitutablePathInfos;
+        writeStorePaths(*this, conn->to, paths);
         conn.processStderr();
         size_t count = readNum<size_t>(conn->from);
         for (size_t n = 0; n < count; n++) {
-            Path path = readStorePath(*this, conn->from);
-            SubstitutablePathInfo & info(infos[path]);
-            info.deriver = readString(conn->from);
-            if (info.deriver != "") assertStorePath(info.deriver);
-            info.references = readStorePaths<PathSet>(*this, conn->from);
+            SubstitutablePathInfo & info(infos[parseStorePath(readString(conn->from))]);
+            auto deriver = readString(conn->from);
+            if (deriver != "")
+                info.deriver = parseStorePath(deriver);
+            info.references = readStorePaths<StorePathSet>(*this, conn->from);
             info.downloadSize = readLongLong(conn->from);
             info.narSize = readLongLong(conn->from);
         }
@@ -349,14 +352,14 @@ void RemoteStore::querySubstitutablePathInfos(const PathSet & paths,
 }
 
 
-void RemoteStore::queryPathInfoUncached(const Path & path,
+void RemoteStore::queryPathInfoUncached(const StorePath & path,
     Callback<std::shared_ptr<const ValidPathInfo>> callback) noexcept
 {
     try {
         std::shared_ptr<ValidPathInfo> info;
         {
             auto conn(getConnection());
-            conn->to << wopQueryPathInfo << path;
+            conn->to << wopQueryPathInfo << printStorePath(path);
             try {
                 conn.processStderr();
             } catch (Error & e) {
@@ -367,14 +370,13 @@ void RemoteStore::queryPathInfoUncached(const Path & path,
             }
             if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 17) {
                 bool valid; conn->from >> valid;
-                if (!valid) throw InvalidPath(format("path '%s' is not valid") % path);
+                if (!valid) throw InvalidPath("path '%s' is not valid", printStorePath(path));
             }
-            info = std::make_shared<ValidPathInfo>();
-            info->path = path;
-            info->deriver = readString(conn->from);
-            if (info->deriver != "") assertStorePath(info->deriver);
+            info = std::make_shared<ValidPathInfo>(path.clone());
+            auto deriver = readString(conn->from);
+            if (deriver != "") info->deriver = parseStorePath(deriver);
             info->narHash = Hash(readString(conn->from), htSHA256);
-            info->references = readStorePaths<PathSet>(*this, conn->from);
+            info->references = readStorePaths<StorePathSet>(*this, conn->from);
             conn->from >> info->registrationTime >> info->narSize;
             if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 16) {
                 conn->from >> info->ultimate;
@@ -387,52 +389,52 @@ void RemoteStore::queryPathInfoUncached(const Path & path,
 }
 
 
-void RemoteStore::queryReferrers(const Path & path,
-    PathSet & referrers)
+void RemoteStore::queryReferrers(const StorePath & path,
+    StorePathSet & referrers)
 {
     auto conn(getConnection());
-    conn->to << wopQueryReferrers << path;
+    conn->to << wopQueryReferrers << printStorePath(path);
     conn.processStderr();
-    PathSet referrers2 = readStorePaths<PathSet>(*this, conn->from);
-    referrers.insert(referrers2.begin(), referrers2.end());
+    for (auto & i : readStorePaths<StorePathSet>(*this, conn->from))
+        referrers.insert(i.clone());
 }
 
 
-PathSet RemoteStore::queryValidDerivers(const Path & path)
+StorePathSet RemoteStore::queryValidDerivers(const StorePath & path)
 {
     auto conn(getConnection());
-    conn->to << wopQueryValidDerivers << path;
+    conn->to << wopQueryValidDerivers << printStorePath(path);
     conn.processStderr();
-    return readStorePaths<PathSet>(*this, conn->from);
+    return readStorePaths<StorePathSet>(*this, conn->from);
 }
 
 
-PathSet RemoteStore::queryDerivationOutputs(const Path & path)
+StorePathSet RemoteStore::queryDerivationOutputs(const StorePath & path)
 {
     auto conn(getConnection());
-    conn->to << wopQueryDerivationOutputs << path;
+    conn->to << wopQueryDerivationOutputs << printStorePath(path);
     conn.processStderr();
-    return readStorePaths<PathSet>(*this, conn->from);
+    return readStorePaths<StorePathSet>(*this, conn->from);
 }
 
 
-PathSet RemoteStore::queryDerivationOutputNames(const Path & path)
+PathSet RemoteStore::queryDerivationOutputNames(const StorePath & path)
 {
     auto conn(getConnection());
-    conn->to << wopQueryDerivationOutputNames << path;
+    conn->to << wopQueryDerivationOutputNames << printStorePath(path);
     conn.processStderr();
     return readStrings<PathSet>(conn->from);
 }
 
 
-Path RemoteStore::queryPathFromHashPart(const string & hashPart)
+std::optional<StorePath> RemoteStore::queryPathFromHashPart(const std::string & hashPart)
 {
     auto conn(getConnection());
     conn->to << wopQueryPathFromHashPart << hashPart;
     conn.processStderr();
     Path path = readString(conn->from);
-    if (!path.empty()) assertStorePath(path);
-    return path;
+    if (path.empty()) return {};
+    return parseStorePath(path);
 }
 
 
@@ -450,9 +452,10 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source,
             copyNAR(source, sink);
             sink
                 << exportMagic
-                << info.path
-                << info.references
-                << info.deriver
+                << printStorePath(info.path);
+            writeStorePaths(*this, sink, info.references);
+            sink
+                << (info.deriver ? printStorePath(*info.deriver) : "")
                 << 0 // == no legacy signature
                 << 0 // == no path follows
                 ;
@@ -460,14 +463,17 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source,
 
         conn.processStderr(0, source2.get());
 
-        auto importedPaths = readStorePaths<PathSet>(*this, conn->from);
+        auto importedPaths = readStorePaths<StorePathSet>(*this, conn->from);
         assert(importedPaths.size() <= 1);
     }
 
     else {
         conn->to << wopAddToStoreNar
-                 << info.path << info.deriver << info.narHash.to_string(Base16, false)
-                 << info.references << info.registrationTime << info.narSize
+                 << printStorePath(info.path)
+                 << (info.deriver ? printStorePath(*info.deriver) : "")
+                 << info.narHash.to_string(Base16, false);
+        writeStorePaths(*this, conn->to, info.references);
+        conn->to << info.registrationTime << info.narSize
                  << info.ultimate << info.sigs << info.ca
                  << repair << !checkSigs;
         bool tunnel = GET_PROTOCOL_MINOR(conn->daemonVersion) >= 21;
@@ -477,7 +483,7 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source,
 }
 
 
-Path RemoteStore::addToStore(const string & name, const Path & _srcPath,
+StorePath RemoteStore::addToStore(const string & name, const Path & _srcPath,
     bool recursive, HashType hashAlgo, PathFilter & filter, RepairFlag repair)
 {
     if (repair) throw Error("repairing is not supported when building through the Nix daemon");
@@ -511,54 +517,52 @@ Path RemoteStore::addToStore(const string & name, const Path & _srcPath,
         throw;
     }
 
-    return readStorePath(*this, conn->from);
+    return parseStorePath(readString(conn->from));
 }
 
 
-Path RemoteStore::addTextToStore(const string & name, const string & s,
-    const PathSet & references, RepairFlag repair)
+StorePath RemoteStore::addTextToStore(const string & name, const string & s,
+    const StorePathSet & references, RepairFlag repair)
 {
     if (repair) throw Error("repairing is not supported when building through the Nix daemon");
 
     auto conn(getConnection());
-    conn->to << wopAddTextToStore << name << s << references;
+    conn->to << wopAddTextToStore << name << s;
+    writeStorePaths(*this, conn->to, references);
 
     conn.processStderr();
-    return readStorePath(*this, conn->from);
+    return parseStorePath(readString(conn->from));
 }
 
 
-void RemoteStore::buildPaths(const PathSet & drvPaths, BuildMode buildMode)
+void RemoteStore::buildPaths(const std::vector<StorePathWithOutputs> & drvPaths, BuildMode buildMode)
 {
     auto conn(getConnection());
     conn->to << wopBuildPaths;
-    if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 13) {
-        conn->to << drvPaths;
-        if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 15)
-            conn->to << buildMode;
-        else
-            /* Old daemons did not take a 'buildMode' parameter, so we
-               need to validate it here on the client side.  */
-            if (buildMode != bmNormal)
-                throw Error("repairing or checking is not supported when building through the Nix daemon");
-    } else {
-        /* For backwards compatibility with old daemons, strip output
-           identifiers. */
-        PathSet drvPaths2;
-        for (auto & i : drvPaths)
-            drvPaths2.insert(string(i, 0, i.find('!')));
-        conn->to << drvPaths2;
-    }
+    assert(GET_PROTOCOL_MINOR(conn->daemonVersion) >= 13);
+    Strings ss;
+    for (auto & p : drvPaths)
+        ss.push_back(p.to_string(*this));
+    conn->to << ss;
+    if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 15)
+        conn->to << buildMode;
+    else
+        /* Old daemons did not take a 'buildMode' parameter, so we
+           need to validate it here on the client side.  */
+        if (buildMode != bmNormal)
+            throw Error("repairing or checking is not supported when building through the Nix daemon");
     conn.processStderr();
     readInt(conn->from);
 }
 
 
-BuildResult RemoteStore::buildDerivation(const Path & drvPath, const BasicDerivation & drv,
+BuildResult RemoteStore::buildDerivation(const StorePath & drvPath, const BasicDerivation & drv,
     BuildMode buildMode)
 {
     auto conn(getConnection());
-    conn->to << wopBuildDerivation << drvPath << drv << buildMode;
+    conn->to << wopBuildDerivation << printStorePath(drvPath);
+    writeDerivation(conn->to, *this, drv);
+    conn->to << buildMode;
     conn.processStderr();
     BuildResult res;
     unsigned int status;
@@ -568,19 +572,19 @@ BuildResult RemoteStore::buildDerivation(const Path & drvPath, const BasicDeriva
 }
 
 
-void RemoteStore::ensurePath(const Path & path)
+void RemoteStore::ensurePath(const StorePath & path)
 {
     auto conn(getConnection());
-    conn->to << wopEnsurePath << path;
+    conn->to << wopEnsurePath << printStorePath(path);
     conn.processStderr();
     readInt(conn->from);
 }
 
 
-void RemoteStore::addTempRoot(const Path & path)
+void RemoteStore::addTempRoot(const StorePath & path)
 {
     auto conn(getConnection());
-    conn->to << wopAddTempRoot << path;
+    conn->to << wopAddTempRoot << printStorePath(path);
     conn.processStderr();
     readInt(conn->from);
 }
@@ -613,8 +617,8 @@ Roots RemoteStore::findRoots(bool censor)
     Roots result;
     while (count--) {
         Path link = readString(conn->from);
-        Path target = readStorePath(*this, conn->from);
-        result[target].emplace(link);
+        auto target = parseStorePath(readString(conn->from));
+        result[std::move(target)].emplace(link);
     }
     return result;
 }
@@ -625,7 +629,9 @@ void RemoteStore::collectGarbage(const GCOptions & options, GCResults & results)
     auto conn(getConnection());
 
     conn->to
-        << wopCollectGarbage << options.action << options.pathsToDelete << options.ignoreLiveness
+        << wopCollectGarbage << options.action;
+    writeStorePaths(*this, conn->to, options.pathsToDelete);
+    conn->to << options.ignoreLiveness
         << options.maxFreed
         /* removed options */
         << 0 << 0 << 0;
@@ -661,17 +667,17 @@ bool RemoteStore::verifyStore(bool checkContents, RepairFlag repair)
 }
 
 
-void RemoteStore::addSignatures(const Path & storePath, const StringSet & sigs)
+void RemoteStore::addSignatures(const StorePath & storePath, const StringSet & sigs)
 {
     auto conn(getConnection());
-    conn->to << wopAddSignatures << storePath << sigs;
+    conn->to << wopAddSignatures << printStorePath(storePath) << sigs;
     conn.processStderr();
     readInt(conn->from);
 }
 
 
-void RemoteStore::queryMissing(const PathSet & targets,
-    PathSet & willBuild, PathSet & willSubstitute, PathSet & unknown,
+void RemoteStore::queryMissing(const std::vector<StorePathWithOutputs> & targets,
+    StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown,
     unsigned long long & downloadSize, unsigned long long & narSize)
 {
     {
@@ -680,11 +686,15 @@ void RemoteStore::queryMissing(const PathSet & targets,
             // Don't hold the connection handle in the fallback case
             // to prevent a deadlock.
             goto fallback;
-        conn->to << wopQueryMissing << targets;
+        conn->to << wopQueryMissing;
+        Strings ss;
+        for (auto & p : targets)
+            ss.push_back(p.to_string(*this));
+        conn->to << ss;
         conn.processStderr();
-        willBuild = readStorePaths<PathSet>(*this, conn->from);
-        willSubstitute = readStorePaths<PathSet>(*this, conn->from);
-        unknown = readStorePaths<PathSet>(*this, conn->from);
+        willBuild = readStorePaths<StorePathSet>(*this, conn->from);
+        willSubstitute = readStorePaths<StorePathSet>(*this, conn->from);
+        unknown = readStorePaths<StorePathSet>(*this, conn->from);
         conn->from >> downloadSize >> narSize;
         return;
     }