From 278114d559109199ff8e6f23b6700ab7909f5320 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Tue, 4 Jun 2019 19:45:16 +0200
Subject: [PATCH] Fix GC closure generation

---
 src/libexpr/primops/flake.cc | 17 +++++++++--------
 src/libexpr/primops/flake.hh | 26 +++++++++++++++++---------
 src/nix/installables.cc      | 21 +++++++++++++--------
 tests/flakes.sh              |  1 +
 4 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/src/libexpr/primops/flake.cc b/src/libexpr/primops/flake.cc
index 189663e51..d0405a377 100644
--- a/src/libexpr/primops/flake.cc
+++ b/src/libexpr/primops/flake.cc
@@ -50,7 +50,7 @@ void writeRegistry(const FlakeRegistry & registry, const Path & path)
     writeFile(path, json.dump(4)); // The '4' is the number of spaces used in the indentation in the json file.
 }
 
-NonFlakeDep::NonFlakeDep(const nlohmann::json & json)
+AbstractDep::AbstractDep(const nlohmann::json & json)
     : ref(json["uri"])
     , narHash(Hash((std::string) json["narHash"]))
 {
@@ -58,7 +58,7 @@ NonFlakeDep::NonFlakeDep(const nlohmann::json & json)
         throw Error("lockfile contains mutable flakeref '%s'", ref);
 }
 
-nlohmann::json NonFlakeDep::toJson() const
+nlohmann::json AbstractDep::toJson() const
 {
     nlohmann::json json;
     json["uri"] = ref.to_string();
@@ -66,22 +66,23 @@ nlohmann::json NonFlakeDep::toJson() const
     return json;
 }
 
+Path AbstractDep::computeStorePath(Store & store) const
+{
+    return store.makeFixedOutputPath(true, narHash, "source");
+}
+
 FlakeDep::FlakeDep(const nlohmann::json & json)
     : FlakeInputs(json)
+    , AbstractDep(json)
     , id(json["id"])
-    , ref(json["uri"])
-    , narHash(Hash((std::string) json["narHash"]))
 {
-    if (!ref.isImmutable())
-        throw Error("lockfile contains mutable flakeref '%s'", ref);
 }
 
 nlohmann::json FlakeDep::toJson() const
 {
     auto json = FlakeInputs::toJson();
+    json.update(AbstractDep::toJson());
     json["id"] = id;
-    json["uri"] = ref.to_string();
-    json["narHash"] = narHash.to_string(SRI);
     return json;
 }
 
diff --git a/src/libexpr/primops/flake.hh b/src/libexpr/primops/flake.hh
index 692fa744d..933bc2593 100644
--- a/src/libexpr/primops/flake.hh
+++ b/src/libexpr/primops/flake.hh
@@ -1,3 +1,5 @@
+#pragma once
+
 #include "types.hh"
 #include "flakeref.hh"
 
@@ -8,6 +10,7 @@ namespace nix {
 
 struct Value;
 class EvalState;
+class Store;
 
 namespace flake {
 
@@ -37,22 +40,29 @@ enum HandleLockFile : unsigned int
     , UseNewLockFile // `RecreateLockFile` without writing to file
     };
 
-struct NonFlakeDep
+struct AbstractDep
 {
     FlakeRef ref;
     Hash narHash;
 
-    NonFlakeDep(const FlakeRef & flakeRef, const Hash & narHash)
+    AbstractDep(const FlakeRef & flakeRef, const Hash & narHash)
         : ref(flakeRef), narHash(narHash) {};
 
-    NonFlakeDep(const nlohmann::json & json);
+    AbstractDep(const nlohmann::json & json);
+
+    nlohmann::json toJson() const;
+
+    Path computeStorePath(Store & store) const;
+};
+
+struct NonFlakeDep : AbstractDep
+{
+    using AbstractDep::AbstractDep;
 
     bool operator ==(const NonFlakeDep & other) const
     {
         return ref == other.ref && narHash == other.narHash;
     }
-
-    nlohmann::json toJson() const;
 };
 
 struct FlakeDep;
@@ -68,14 +78,12 @@ struct FlakeInputs
     nlohmann::json toJson() const;
 };
 
-struct FlakeDep : FlakeInputs
+struct FlakeDep : FlakeInputs, AbstractDep
 {
     FlakeId id;
-    FlakeRef ref;
-    Hash narHash;
 
     FlakeDep(const FlakeId & id, const FlakeRef & flakeRef, const Hash & narHash)
-        : id(id), ref(flakeRef), narHash(narHash) {};
+        : AbstractDep(flakeRef, narHash), id(id) {};
 
     FlakeDep(const nlohmann::json & json);
 
diff --git a/src/nix/installables.cc b/src/nix/installables.cc
index 86b4a9b93..c44a37f1e 100644
--- a/src/nix/installables.cc
+++ b/src/nix/installables.cc
@@ -190,22 +190,28 @@ void makeFlakeClosureGCRoot(Store & store,
     const FlakeRef & origFlakeRef,
     const flake::ResolvedFlake & resFlake)
 {
-#if 0
     if (std::get_if<FlakeRef::IsPath>(&origFlakeRef.data)) return;
 
     /* Get the store paths of all non-local flakes. */
     PathSet closure;
 
-    std::queue<std::reference_wrapper<const flake::ResolvedFlake>> queue;
-    queue.push(resFlake);
+    assert(store.isValidPath(resFlake.flake.sourceInfo.storePath));
+    closure.insert(resFlake.flake.sourceInfo.storePath);
+
+    std::queue<std::reference_wrapper<const flake::FlakeInputs>> queue;
+    queue.push(resFlake.lockFile);
 
     while (!queue.empty()) {
-        const flake::ResolvedFlake & flake = queue.front();
+        const flake::FlakeInputs & flake = queue.front();
         queue.pop();
-        if (!std::get_if<FlakeRef::IsPath>(&flake.flake.sourceInfo.resolvedRef.data))
-            closure.insert(flake.flake.sourceInfo.storePath);
-        for (const auto & dep : flake.flakeDeps)
+        /* Note: due to lazy fetching, these paths might not exist
+           yet. */
+        for (auto & dep : flake.flakeDeps) {
+            closure.insert(dep.second.computeStorePath(store));
             queue.push(dep.second);
+        }
+        for (auto & dep : flake.nonFlakeDeps)
+            closure.insert(dep.second.computeStorePath(store));
     }
 
     if (closure.empty()) return;
@@ -225,7 +231,6 @@ void makeFlakeClosureGCRoot(Store & store,
     debug("writing GC root '%s' for flake closure of '%s'", symlink, origFlakeRef);
     replaceSymlink(closurePath, symlink);
     store.addIndirectRoot(symlink);
-#endif
 }
 
 struct InstallableFlake : InstallableValue
diff --git a/tests/flakes.sh b/tests/flakes.sh
index 29845e5ed..f44b9509f 100644
--- a/tests/flakes.sh
+++ b/tests/flakes.sh
@@ -204,6 +204,7 @@ nix flake list --flake-registry file://$registry --tarball-ttl 0 | grep -q flake
 mv $registry.tmp $registry
 
 # Test whether flakes are registered as GC roots for offline use.
+# FIXME: use tarballs rather than git.
 rm -rf $TEST_HOME/.cache
 nix build -o $TEST_ROOT/result --flake-registry file://$registry file://$flake2Dir:bar
 mv $flake1Dir $flake1Dir.tmp