Merge remote-tracking branch 'origin/master' into flakes

5 files changed, 77 insertions, 47 deletions

diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc
index 5dba84aaf..0067032af 100644
--- a/src/libstore/derivations.cc
+++ b/src/libstore/derivations.cc
@@ -213,15 +213,26 @@ Derivation Store::derivationFromPath(const StorePath & drvPath)
 }
 
 
-static void printString(string & res, const string & s)
+static void printString(string & res, std::string_view s)
+{
+    char buf[s.size() * 2 + 2];
+    char * p = buf;
+    *p++ = '"';
+    for (auto c : s)
+        if (c == '\"' || c == '\\') { *p++ = '\\'; *p++ = c; }
+        else if (c == '\n') { *p++ = '\\'; *p++ = 'n'; }
+        else if (c == '\r') { *p++ = '\\'; *p++ = 'r'; }
+        else if (c == '\t') { *p++ = '\\'; *p++ = 't'; }
+        else *p++ = c;
+    *p++ = '"';
+    res.append(buf, p - buf);
+}
+
+
+static void printUnquotedString(string & res, std::string_view s)
 {
     res += '"';
-    for (const char * i = s.c_str(); *i; i++)
-        if (*i == '\"' || *i == '\\') { res += "\\"; res += *i; }
-        else if (*i == '\n') res += "\\n";
-        else if (*i == '\r') res += "\\r";
-        else if (*i == '\t') res += "\\t";
-        else res += *i;
+    res.append(s);
     res += '"';
 }
 
@@ -239,6 +250,19 @@ static void printStrings(string & res, ForwardIterator i, ForwardIterator j)
 }
 
 
+template<class ForwardIterator>
+static void printUnquotedStrings(string & res, ForwardIterator i, ForwardIterator j)
+{
+    res += '[';
+    bool first = true;
+    for ( ; i != j; ++i) {
+        if (first) first = false; else res += ',';
+        printUnquotedString(res, *i);
+    }
+    res += ']';
+}
+
+
 string Derivation::unparse(const Store & store, bool maskOutputs,
     std::map<std::string, StringSet> * actualInputs) const
 {
@@ -249,10 +273,10 @@ string Derivation::unparse(const Store & store, bool maskOutputs,
     bool first = true;
     for (auto & i : outputs) {
         if (first) first = false; else s += ',';
-        s += '('; printString(s, i.first);
-        s += ','; printString(s, maskOutputs ? "" : store.printStorePath(i.second.path));
-        s += ','; printString(s, i.second.hashAlgo);
-        s += ','; printString(s, i.second.hash);
+        s += '('; printUnquotedString(s, i.first);
+        s += ','; printUnquotedString(s, maskOutputs ? "" : store.printStorePath(i.second.path));
+        s += ','; printUnquotedString(s, i.second.hashAlgo);
+        s += ','; printUnquotedString(s, i.second.hash);
         s += ')';
     }
 
@@ -261,24 +285,24 @@ string Derivation::unparse(const Store & store, bool maskOutputs,
     if (actualInputs) {
         for (auto & i : *actualInputs) {
             if (first) first = false; else s += ',';
-            s += '('; printString(s, i.first);
-            s += ','; printStrings(s, i.second.begin(), i.second.end());
+            s += '('; printUnquotedString(s, i.first);
+            s += ','; printUnquotedStrings(s, i.second.begin(), i.second.end());
             s += ')';
         }
     } else {
         for (auto & i : inputDrvs) {
             if (first) first = false; else s += ',';
-            s += '('; printString(s, store.printStorePath(i.first));
-            s += ','; printStrings(s, i.second.begin(), i.second.end());
+            s += '('; printUnquotedString(s, store.printStorePath(i.first));
+            s += ','; printUnquotedStrings(s, i.second.begin(), i.second.end());
             s += ')';
         }
     }
 
     s += "],";
     auto paths = store.printStorePathSet(inputSrcs); // FIXME: slow
-    printStrings(s, paths.begin(), paths.end());
+    printUnquotedStrings(s, paths.begin(), paths.end());
 
-    s += ','; printString(s, platform);
+    s += ','; printUnquotedString(s, platform);
     s += ','; printString(s, builder);
     s += ','; printStrings(s, args.begin(), args.end());
 
diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
index 690febc5b..0c3d89611 100644
--- a/src/libstore/gc.cc
+++ b/src/libstore/gc.cc
@@ -255,12 +255,11 @@ void LocalStore::findTempRoots(FDs & fds, Roots & tempRoots, bool censor)
 void LocalStore::findRoots(const Path & path, unsigned char type, Roots & roots)
 {
     auto foundRoot = [&](const Path & path, const Path & target) {
-        Path storePath = toStorePath(target);
-        // FIXME
-        if (isStorePath(storePath) && isValidPath(parseStorePath(storePath)))
-            roots[parseStorePath(storePath)].emplace(path);
+        auto storePath = maybeParseStorePath(toStorePath(target));
+        if (storePath && isValidPath(*storePath))
+            roots[std::move(*storePath)].emplace(path);
         else
-            printInfo("skipping invalid root from '%1%' to '%2%'", path, storePath);
+            printInfo("skipping invalid root from '%1%' to '%2%'", path, target);
     };
 
     try {
@@ -296,10 +295,9 @@ void LocalStore::findRoots(const Path & path, unsigned char type, Roots & roots)
         }
 
         else if (type == DT_REG) {
-            Path storePath = storeDir + "/" + std::string(baseNameOf(path));
-            // FIXME
-            if (isStorePath(storePath) && isValidPath(parseStorePath(storePath)))
-                roots[parseStorePath(storePath)].emplace(path);
+            auto storePath = maybeParseStorePath(storeDir + "/" + std::string(baseNameOf(path)));
+            if (storePath && isValidPath(*storePath))
+                roots[std::move(*storePath)].emplace(path);
         }
 
     }
@@ -523,14 +521,14 @@ void LocalStore::deletePathRecursive(GCState & state, const Path & path)
 
     unsigned long long size = 0;
 
-    // FIXME
-    if (isStorePath(path) && isValidPath(parseStorePath(path))) {
+    auto storePath = maybeParseStorePath(path);
+    if (storePath && isValidPath(*storePath)) {
         StorePathSet referrers;
-        queryReferrers(parseStorePath(path), referrers);
+        queryReferrers(*storePath, referrers);
         for (auto & i : referrers)
             if (printStorePath(i) != path) deletePathRecursive(state, printStorePath(i));
-        size = queryPathInfo(parseStorePath(path))->narSize;
-        invalidatePathChecked(parseStorePath(path));
+        size = queryPathInfo(*storePath)->narSize;
+        invalidatePathChecked(*storePath);
     }
 
     Path realPath = realStoreDir + "/" + std::string(baseNameOf(path));
@@ -593,8 +591,7 @@ bool LocalStore::canReachRoot(GCState & state, StorePathSet & visited, const Sto
 
     visited.insert(path.clone());
 
-    //FIXME
-    if (!isStorePath(printStorePath(path)) || !isValidPath(path)) return false;
+    if (!isValidPath(path)) return false;
 
     StorePathSet incoming;
 
@@ -637,8 +634,9 @@ void LocalStore::tryToDelete(GCState & state, const Path & path)
 
     //Activity act(*logger, lvlDebug, format("considering whether to delete '%1%'") % path);
 
-    // FIXME
-    if (!isStorePath(path) || !isValidPath(parseStorePath(path))) {
+    auto storePath = maybeParseStorePath(path);
+
+    if (!storePath || !isValidPath(*storePath)) {
         /* A lock file belonging to a path that we're building right
            now isn't garbage. */
         if (isActiveTempFile(state, path, ".lock")) return;
@@ -655,7 +653,7 @@ void LocalStore::tryToDelete(GCState & state, const Path & path)
 
     StorePathSet visited;
 
-    if (canReachRoot(state, visited, parseStorePath(path))) {
+    if (storePath && canReachRoot(state, visited, *storePath)) {
         debug("cannot delete '%s' because it's still reachable", path);
     } else {
         /* No path we visited was a root, so everything is garbage.
@@ -818,8 +816,8 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results)
                 string name = dirent->d_name;
                 if (name == "." || name == "..") continue;
                 Path path = storeDir + "/" + name;
-                // FIXME
-                if (isStorePath(path) && isValidPath(parseStorePath(path)))
+                auto storePath = maybeParseStorePath(path);
+                if (storePath && isValidPath(*storePath))
                     entries.push_back(path);
                 else
                     tryToDelete(state, path);
diff --git a/src/libstore/path.cc b/src/libstore/path.cc
index a33bec3ed..9a28aa96a 100644
--- a/src/libstore/path.cc
+++ b/src/libstore/path.cc
@@ -55,6 +55,20 @@ StorePath Store::parseStorePath(std::string_view path) const
     return StorePath::make(path, storeDir);
 }
 
+std::optional<StorePath> Store::maybeParseStorePath(std::string_view path) const
+{
+    try {
+        return parseStorePath(path);
+    } catch (Error &) {
+        return {};
+    }
+}
+
+bool Store::isStorePath(std::string_view path) const
+{
+    return (bool) maybeParseStorePath(path);
+}
+
 StorePathSet Store::parseStorePathSet(const PathSet & paths) const
 {
     StorePathSet res;
diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
index 5ca77b563..eb17d5cd2 100644
--- a/src/libstore/store-api.cc
+++ b/src/libstore/store-api.cc
@@ -20,14 +20,6 @@ bool Store::isInStore(const Path & path) const
 }
 
 
-bool Store::isStorePath(const Path & path) const
-{
-    return isInStore(path)
-        && path.size() >= storeDir.size() + 1 + storePathHashLen
-        && path.find('/', storeDir.size() + 1) == Path::npos;
-}
-
-
 Path Store::toStorePath(const Path & path) const
 {
     if (!isInStore(path))
diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh
index 85851e211..88cfa5b5a 100644
--- a/src/libstore/store-api.hh
+++ b/src/libstore/store-api.hh
@@ -282,6 +282,8 @@ public:
 
     StorePath parseStorePath(std::string_view path) const;
 
+    std::optional<StorePath> maybeParseStorePath(std::string_view path) const;
+
     std::string printStorePath(const StorePath & path) const;
 
     // FIXME: remove
@@ -304,7 +306,7 @@ public:
 
     /* Return true if ‘path’ is a store path, i.e. a direct child of
        the Nix store. */
-    bool isStorePath(const Path & path) const;
+    bool isStorePath(std::string_view path) const;
 
     /* Chop off the parts after the top-level store name, e.g.,
        /nix/store/abcd-foo/bar => /nix/store/abcd-foo. */