Merge pull request #3645 from mkenigs/fetchOrSubstituteTree-improvements

Cache tree in fetchOrSubstituteTree

src/libexpr/flake/flake.cc

@@ -12,38 +12,23 @@ using namespace flake;
 
 namespace flake {
 
-/* If 'allowLookup' is true, then resolve 'flakeRef' using the
-   registries. */
-static FlakeRef maybeLookupFlake(
-    ref<Store> store,
-    const FlakeRef & flakeRef,
-    bool allowLookup)
-{
-    if (!flakeRef.input.isDirect()) {
-        if (allowLookup)
-            return flakeRef.resolve(store);
-        else
-            throw Error("'%s' is an indirect flake reference, but registry lookups are not allowed", flakeRef);
-    } else
-        return flakeRef;
-}
+typedef std::pair<Tree, FlakeRef> FetchedFlake;
+typedef std::vector<std::pair<FlakeRef, FetchedFlake>> FlakeCache;
 
-typedef std::vector<std::pair<FlakeRef, FlakeRef>> FlakeCache;
-
-static FlakeRef lookupInFlakeCache(
+static std::optional<FetchedFlake> lookupInFlakeCache(
     const FlakeCache & flakeCache,
     const FlakeRef & flakeRef)
 {
     // FIXME: inefficient.
     for (auto & i : flakeCache) {
         if (flakeRef == i.first) {
             debug("mapping '%s' to previously seen input '%s' -> '%s",
-                flakeRef, i.first, i.second);
+                flakeRef, i.first, i.second.second);
             return i.second;
         }
     }
 
-    return flakeRef;
+    return std::nullopt;
 }
 
 static std::tuple<fetchers::Tree, FlakeRef, FlakeRef> fetchOrSubstituteTree(
@@ -52,17 +37,32 @@ static std::tuple<fetchers::Tree, FlakeRef, FlakeRef> fetchOrSubstituteTree(
     bool allowLookup,
     FlakeCache & flakeCache)
 {
-    auto resolvedRef = lookupInFlakeCache(flakeCache,
-        maybeLookupFlake(state.store,
-            lookupInFlakeCache(flakeCache, originalRef), allowLookup));
-
-    auto [tree, lockedRef] = resolvedRef.fetchTree(state.store);
+    auto fetched = lookupInFlakeCache(flakeCache, originalRef);
+    FlakeRef resolvedRef = originalRef;
+
+    if (!fetched) {
+        if (originalRef.input.isDirect()) {
+            fetched.emplace(originalRef.fetchTree(state.store));
+        } else {
+            if (allowLookup) {
+                resolvedRef = originalRef.resolve(state.store);
+                auto fetchedResolved = lookupInFlakeCache(flakeCache, originalRef);
+                if (!fetchedResolved) fetchedResolved.emplace(resolvedRef.fetchTree(state.store));
+                flakeCache.push_back({resolvedRef, fetchedResolved.value()});
+                fetched.emplace(fetchedResolved.value());
+            }
+            else {
+                throw Error("'%s' is an indirect flake reference, but registry lookups are not allowed", originalRef);
+            }
+        }
+        flakeCache.push_back({originalRef, fetched.value()});
+    }
+
+    auto [tree, lockedRef] = fetched.value();
 
     debug("got tree '%s' from '%s'",
         state.store->printStorePath(tree.storePath), lockedRef);
 
-    flakeCache.push_back({originalRef, lockedRef});
-    flakeCache.push_back({resolvedRef, lockedRef});
 
     if (state.allowedPaths)
         state.allowedPaths->insert(tree.actualPath);

src/libfetchers/fetchers.cc

@@ -117,7 +117,7 @@ std::pair<Tree, Input> Input::fetch(ref<Store> store) const
 
             auto actualPath = store->toRealPath(storePath);
 
-            return {fetchers::Tree { .actualPath = actualPath, .storePath = std::move(storePath) }, *this};
+            return {fetchers::Tree(std::move(actualPath), std::move(storePath)), *this};
         } catch (Error & e) {
             debug("substitution of input '%s' failed: %s", to_string(), e.what());
         }

src/libfetchers/fetchers.hh

@@ -16,6 +16,8 @@ struct Tree
 {
     Path actualPath;
     StorePath storePath;
+    Tree(Path && actualPath, StorePath && storePath) : actualPath(actualPath), storePath(std::move(storePath)) {}
+    Tree (const Tree & rhs) : actualPath(rhs.actualPath), storePath(rhs.storePath.clone()) {}
 };
 
 struct InputScheme;

src/libfetchers/git.cc

@@ -189,10 +189,7 @@ struct GitInputScheme : InputScheme
                 input.attrs.insert_or_assign("revCount", getIntAttr(infoAttrs, "revCount"));
             input.attrs.insert_or_assign("lastModified", getIntAttr(infoAttrs, "lastModified"));
             return {
-                Tree {
-                    .actualPath = store->toRealPath(storePath),
-                    .storePath = std::move(storePath),
-                },
+                Tree(store->toRealPath(storePath), std::move(storePath)),
                 input
             };
         };
@@ -273,10 +270,8 @@ struct GitInputScheme : InputScheme
                     haveCommits ? std::stoull(runProgram("git", true, { "-C", actualUrl, "log", "-1", "--format=%ct", "HEAD" })) : 0);
 
                 return {
-                    Tree {
-                        .actualPath = store->printStorePath(storePath),
-                        .storePath = std::move(storePath),
-                    }, input
+                    Tree(store->printStorePath(storePath), std::move(storePath)),
+                    input
                 };
             }
         }

src/libfetchers/github.cc

@@ -140,10 +140,7 @@ struct GitArchiveInputScheme : InputScheme
         if (auto res = getCache()->lookup(store, immutableAttrs)) {
             input.attrs.insert_or_assign("lastModified", getIntAttr(res->first, "lastModified"));
             return {
-                Tree{
-                    .actualPath = store->toRealPath(res->second),
-                    .storePath = std::move(res->second),
-                },
+                Tree(store->toRealPath(res->second), std::move(res->second)),
                 input
             };
         }

src/libfetchers/mercurial.cc

@@ -164,10 +164,10 @@ struct MercurialInputScheme : InputScheme
 
                 auto storePath = store->addToStore("source", actualUrl, true, htSHA256, filter);
 
-                return {Tree {
-                    .actualPath = store->printStorePath(storePath),
-                    .storePath = std::move(storePath),
-                }, input};
+                return {
+                    Tree(store->printStorePath(storePath), std::move(storePath)),
+                    input
+                };
             }
         }
 
@@ -189,10 +189,7 @@ struct MercurialInputScheme : InputScheme
             assert(!_input.getRev() || _input.getRev() == input.getRev());
             input.attrs.insert_or_assign("revCount", getIntAttr(infoAttrs, "revCount"));
             return {
-                Tree{
-                    .actualPath = store->toRealPath(storePath),
-                    .storePath = std::move(storePath),
-                },
+                Tree(store->toRealPath(storePath), std::move(storePath)),
                 input
             };
         };

src/libfetchers/path.cc

@@ -96,10 +96,7 @@ struct PathInputScheme : InputScheme
             storePath = store->addToStore("source", path);
 
         return {
-            Tree {
-                .actualPath = store->toRealPath(*storePath),
-                .storePath = std::move(*storePath),
-            },
+            Tree(store->toRealPath(*storePath), std::move(*storePath)),
             input
         };
     }

src/libfetchers/tarball.cc

@@ -117,10 +117,7 @@ std::pair<Tree, time_t> downloadTarball(
 
     if (cached && !cached->expired)
         return {
-            Tree {
-                .actualPath = store->toRealPath(cached->storePath),
-                .storePath = std::move(cached->storePath),
-            },
+            Tree(store->toRealPath(cached->storePath), std::move(cached->storePath)),
             getIntAttr(cached->infoAttrs, "lastModified")
         };
 
@@ -157,10 +154,7 @@ std::pair<Tree, time_t> downloadTarball(
         immutable);
 
     return {
-        Tree {
-            .actualPath = store->toRealPath(*unpackedStorePath),
-            .storePath = std::move(*unpackedStorePath),
-        },
+        Tree(store->toRealPath(*unpackedStorePath), std::move(*unpackedStorePath)),
         lastModified,
     };
 }