Merge pull request #9199 from edolstra/remove-tree

Remove fetchers::Tree and move tarball-related stuff into its own header

src/libcmd/common-eval-args.cc

@@ -9,6 +9,7 @@
 #include "flake/flakeref.hh"
 #include "store-api.hh"
 #include "command.hh"
+#include "tarball.hh"
 
 namespace nix {
 
@@ -168,14 +169,14 @@ SourcePath lookupFileArg(EvalState & state, std::string_view s)
 {
     if (EvalSettings::isPseudoUrl(s)) {
         auto storePath = fetchers::downloadTarball(
-            state.store, EvalSettings::resolvePseudoUrl(s), "source", false).tree.storePath;
+            state.store, EvalSettings::resolvePseudoUrl(s), "source", false).storePath;
         return state.rootPath(CanonPath(state.store->toRealPath(storePath)));
     }
 
     else if (hasPrefix(s, "flake:")) {
         experimentalFeatureSettings.require(Xp::Flakes);
         auto flakeRef = parseFlakeRef(std::string(s.substr(6)), {}, true, false);
-        auto storePath = flakeRef.resolve(state.store).fetchTree(state.store).first.storePath;
+        auto storePath = flakeRef.resolve(state.store).fetchTree(state.store).first;
         return state.rootPath(CanonPath(state.store->toRealPath(storePath)));
     }
 

src/libexpr/flake/flake.cc

@@ -15,7 +15,7 @@ using namespace flake;
 
 namespace flake {
 
-typedef std::pair<fetchers::Tree, FlakeRef> FetchedFlake;
+typedef std::pair<StorePath, FlakeRef> FetchedFlake;
 typedef std::vector<std::pair<FlakeRef, FetchedFlake>> FlakeCache;
 
 static std::optional<FetchedFlake> lookupInFlakeCache(
@@ -34,7 +34,7 @@ static std::optional<FetchedFlake> lookupInFlakeCache(
     return std::nullopt;
 }
 
-static std::tuple<fetchers::Tree, FlakeRef, FlakeRef> fetchOrSubstituteTree(
+static std::tuple<StorePath, FlakeRef, FlakeRef> fetchOrSubstituteTree(
     EvalState & state,
     const FlakeRef & originalRef,
     bool allowLookup,
@@ -61,16 +61,16 @@ static std::tuple<fetchers::Tree, FlakeRef, FlakeRef> fetchOrSubstituteTree(
         flakeCache.push_back({originalRef, *fetched});
     }
 
-    auto [tree, lockedRef] = *fetched;
+    auto [storePath, lockedRef] = *fetched;
 
     debug("got tree '%s' from '%s'",
-        state.store->printStorePath(tree.storePath), lockedRef);
+        state.store->printStorePath(storePath), lockedRef);
 
-    state.allowPath(tree.storePath);
+    state.allowPath(storePath);
 
-    assert(!originalRef.input.getNarHash() || tree.storePath == originalRef.input.computeStorePath(*state.store));
+    assert(!originalRef.input.getNarHash() || storePath == originalRef.input.computeStorePath(*state.store));
 
-    return {std::move(tree), resolvedRef, lockedRef};
+    return {std::move(storePath), resolvedRef, lockedRef};
 }
 
 static void forceTrivialValue(EvalState & state, Value & value, const PosIdx pos)
@@ -202,21 +202,21 @@ static Flake getFlake(
     FlakeCache & flakeCache,
     InputPath lockRootPath)
 {
-    auto [sourceInfo, resolvedRef, lockedRef] = fetchOrSubstituteTree(
+    auto [storePath, resolvedRef, lockedRef] = fetchOrSubstituteTree(
         state, originalRef, allowLookup, flakeCache);
 
     // Guard against symlink attacks.
-    auto flakeDir = canonPath(sourceInfo.actualPath + "/" + lockedRef.subdir, true);
+    auto flakeDir = canonPath(state.store->toRealPath(storePath) + "/" + lockedRef.subdir, true);
     auto flakeFile = canonPath(flakeDir + "/flake.nix", true);
-    if (!isInDir(flakeFile, sourceInfo.actualPath))
+    if (!isInDir(flakeFile, state.store->toRealPath(storePath)))
         throw Error("'flake.nix' file of flake '%s' escapes from '%s'",
-            lockedRef, state.store->printStorePath(sourceInfo.storePath));
+            lockedRef, state.store->printStorePath(storePath));
 
     Flake flake {
         .originalRef = originalRef,
         .resolvedRef = resolvedRef,
         .lockedRef = lockedRef,
-        .sourceInfo = std::make_shared<fetchers::Tree>(std::move(sourceInfo))
+        .storePath = storePath,
     };
 
     if (!pathExists(flakeFile))
@@ -346,7 +346,7 @@ LockedFlake lockFlake(
         // FIXME: symlink attack
         auto oldLockFile = LockFile::read(
             lockFlags.referenceLockFilePath.value_or(
-                flake.sourceInfo->actualPath + "/" + flake.lockedRef.subdir + "/flake.lock"));
+                state.store->toRealPath(flake.storePath) + "/" + flake.lockedRef.subdir + "/flake.lock"));
 
         debug("old lock file: %s", oldLockFile);
 
@@ -574,7 +574,7 @@ LockedFlake lockFlake(
                                 oldLock
                                 ? std::dynamic_pointer_cast<const Node>(oldLock)
                                 : LockFile::read(
-                                    inputFlake.sourceInfo->actualPath + "/" + inputFlake.lockedRef.subdir + "/flake.lock").root.get_ptr(),
+                                    state.store->toRealPath(inputFlake.storePath) + "/" + inputFlake.lockedRef.subdir + "/flake.lock").root.get_ptr(),
                                 oldLock ? lockRootPath : inputPath,
                                 localPath,
                                 false);
@@ -598,7 +598,7 @@ LockedFlake lockFlake(
         };
 
         // Bring in the current ref for relative path resolution if we have it
-        auto parentPath = canonPath(flake.sourceInfo->actualPath + "/" + flake.lockedRef.subdir, true);
+        auto parentPath = canonPath(state.store->toRealPath(flake.storePath) + "/" + flake.lockedRef.subdir, true);
 
         computeLocks(
             flake.inputs,
@@ -729,7 +729,7 @@ void callFlake(EvalState & state,
 
     emitTreeAttrs(
         state,
-        *lockedFlake.flake.sourceInfo,
+        lockedFlake.flake.storePath,
         lockedFlake.flake.lockedRef.input,
         *vRootSrc,
         false,
@@ -893,7 +893,7 @@ Fingerprint LockedFlake::getFingerprint() const
     // flake.sourceInfo.storePath for the fingerprint.
     return hashString(htSHA256,
         fmt("%s;%s;%d;%d;%s",
-            flake.sourceInfo->storePath.to_string(),
+            flake.storePath.to_string(),
             flake.lockedRef.subdir,
             flake.lockedRef.input.getRevCount().value_or(0),
             flake.lockedRef.input.getLastModified().value_or(0),

src/libexpr/flake/flake.hh

@@ -10,8 +10,6 @@ namespace nix {
 
 class EvalState;
 
-namespace fetchers { struct Tree; }
-
 namespace flake {
 
 struct FlakeInput;
@@ -84,7 +82,7 @@ struct Flake
      */
     bool forceDirty = false;
     std::optional<std::string> description;
-    std::shared_ptr<const fetchers::Tree> sourceInfo;
+    StorePath storePath;
     FlakeInputs inputs;
     /**
      * 'nixConfig' attribute
@@ -193,7 +191,7 @@ void callFlake(
 
 void emitTreeAttrs(
     EvalState & state,
-    const fetchers::Tree & tree,
+    const StorePath & storePath,
     const fetchers::Input & input,
     Value & v,
     bool emptyRevFallback = false,

src/libexpr/flake/flakeref.cc

@@ -272,10 +272,10 @@ FlakeRef FlakeRef::fromAttrs(const fetchers::Attrs & attrs)
         fetchers::maybeGetStrAttr(attrs, "dir").value_or(""));
 }
 
-std::pair<fetchers::Tree, FlakeRef> FlakeRef::fetchTree(ref<Store> store) const
+std::pair<StorePath, FlakeRef> FlakeRef::fetchTree(ref<Store> store) const
 {
-    auto [tree, lockedInput] = input.fetch(store);
+    auto [storePath, lockedInput] = input.fetch(store);
-    return {std::move(tree), FlakeRef(std::move(lockedInput), subdir)};
+    return {std::move(storePath), FlakeRef(std::move(lockedInput), subdir)};
 }
 
 std::tuple<FlakeRef, std::string, ExtendedOutputsSpec> parseFlakeRefWithFragmentAndExtendedOutputsSpec(

src/libexpr/flake/flakeref.hh

@@ -63,7 +63,7 @@ struct FlakeRef
 
     static FlakeRef fromAttrs(const fetchers::Attrs & attrs);
 
-    std::pair<fetchers::Tree, FlakeRef> fetchTree(ref<Store> store) const;
+    std::pair<StorePath, FlakeRef> fetchTree(ref<Store> store) const;
 };
 
 std::ostream & operator << (std::ostream & str, const FlakeRef & flakeRef);

src/libexpr/parser.y

@@ -646,7 +646,7 @@ formal
 
 #include "eval.hh"
 #include "filetransfer.hh"
-#include "fetchers.hh"
+#include "tarball.hh"
 #include "store-api.hh"
 #include "flake/flake.hh"
 
@@ -783,7 +783,7 @@ std::optional<std::string> EvalState::resolveSearchPathPath(const SearchPath::Pa
     if (EvalSettings::isPseudoUrl(value)) {
         try {
             auto storePath = fetchers::downloadTarball(
-                store, EvalSettings::resolvePseudoUrl(value), "source", false).tree.storePath;
+                store, EvalSettings::resolvePseudoUrl(value), "source", false).storePath;
             res = { store->toRealPath(storePath) };
         } catch (FileTransferError & e) {
             logWarning({
@@ -797,7 +797,7 @@ std::optional<std::string> EvalState::resolveSearchPathPath(const SearchPath::Pa
         experimentalFeatureSettings.require(Xp::Flakes);
         auto flakeRef = parseFlakeRef(value.substr(6), {}, true, false);
         debug("fetching flake search path element '%s''", value);
-        auto storePath = flakeRef.resolve(store).fetchTree(store).first.storePath;
+        auto storePath = flakeRef.resolve(store).fetchTree(store).first;
         res = { store->toRealPath(storePath) };
     }
 

src/libexpr/primops/fetchMercurial.cc

@@ -71,10 +71,10 @@ static void prim_fetchMercurial(EvalState & state, const PosIdx pos, Value * * a
     auto input = fetchers::Input::fromAttrs(std::move(attrs));
 
     // FIXME: use name
-    auto [tree, input2] = input.fetch(state.store);
+    auto [storePath, input2] = input.fetch(state.store);
 
     auto attrs2 = state.buildBindings(8);
-    state.mkStorePathString(tree.storePath, attrs2.alloc(state.sOutPath));
+    state.mkStorePathString(storePath, attrs2.alloc(state.sOutPath));
     if (input2.getRef())
         attrs2.alloc("branch").mkString(*input2.getRef());
     // Backward compatibility: set 'rev' to
@@ -86,7 +86,7 @@ static void prim_fetchMercurial(EvalState & state, const PosIdx pos, Value * * a
         attrs2.alloc("revCount").mkInt(*revCount);
     v.mkAttrs(attrs2);
 
-    state.allowPath(tree.storePath);
+    state.allowPath(storePath);
 }
 
 static RegisterPrimOp r_fetchMercurial({

src/libexpr/primops/fetchTree.cc

@@ -5,6 +5,7 @@
 #include "fetchers.hh"
 #include "filetransfer.hh"
 #include "registry.hh"
+#include "tarball.hh"
 #include "url.hh"
 
 #include <ctime>
@@ -15,7 +16,7 @@ namespace nix {
 
 void emitTreeAttrs(
     EvalState & state,
-    const fetchers::Tree & tree,
+    const StorePath & storePath,
     const fetchers::Input & input,
     Value & v,
     bool emptyRevFallback,
@@ -25,7 +26,7 @@ void emitTreeAttrs(
 
     auto attrs = state.buildBindings(10);
 
-    state.mkStorePathString(tree.storePath, attrs.alloc(state.sOutPath));
+    state.mkStorePathString(storePath, attrs.alloc(state.sOutPath));
 
     // FIXME: support arbitrary input attributes.
 
@@ -165,11 +166,11 @@ static void fetchTree(
 
     state.checkURI(input.toURLString());
 
-    auto [tree, input2] = input.fetch(state.store);
+    auto [storePath, input2] = input.fetch(state.store);
 
-    state.allowPath(tree.storePath);
+    state.allowPath(storePath);
 
-    emitTreeAttrs(state, tree, input2, v, params.emptyRevFallback, false);
+    emitTreeAttrs(state, storePath, input2, v, params.emptyRevFallback, false);
 }
 
 static void prim_fetchTree(EvalState & state, const PosIdx pos, Value * * args, Value & v)
@@ -288,7 +289,7 @@ static void fetch(EvalState & state, const PosIdx pos, Value * * args, Value & v
     //       https://github.com/NixOS/nix/issues/4313
     auto storePath =
         unpack
-        ? fetchers::downloadTarball(state.store, *url, name, (bool) expectedHash).tree.storePath
+        ? fetchers::downloadTarball(state.store, *url, name, (bool) expectedHash).storePath
         : fetchers::downloadFile(state.store, *url, name, (bool) expectedHash).storePath;
 
     if (expectedHash) {

src/libfetchers/fetchers.cc

@@ -109,7 +109,7 @@ bool Input::contains(const Input & other) const
     return false;
 }
 
-std::pair<Tree, Input> Input::fetch(ref<Store> store) const
+std::pair<StorePath, Input> Input::fetch(ref<Store> store) const
 {
     if (!scheme)
         throw Error("cannot fetch unsupported input '%s'", attrsToJSON(toAttrs()));
@@ -126,7 +126,7 @@ std::pair<Tree, Input> Input::fetch(ref<Store> store) const
             debug("using substituted/cached input '%s' in '%s'",
                 to_string(), store->printStorePath(storePath));
 
-            return {Tree { .actualPath = store->toRealPath(storePath), .storePath = std::move(storePath) }, *this};
+            return {std::move(storePath), *this};
         } catch (Error & e) {
             debug("substitution of input '%s' failed: %s", to_string(), e.what());
         }
@@ -141,18 +141,16 @@ std::pair<Tree, Input> Input::fetch(ref<Store> store) const
         }
     }();
 
-    Tree tree {
+    auto narHash = store->queryPathInfo(storePath)->narHash;
-        .actualPath = store->toRealPath(storePath),
-        .storePath = storePath,
-    };
-
-    auto narHash = store->queryPathInfo(tree.storePath)->narHash;
     input.attrs.insert_or_assign("narHash", narHash.to_string(HashFormat::SRI, true));
 
     if (auto prevNarHash = getNarHash()) {
         if (narHash != *prevNarHash)
             throw Error((unsigned int) 102, "NAR hash mismatch in input '%s' (%s), expected '%s', got '%s'",
-                to_string(), tree.actualPath, prevNarHash->to_string(HashFormat::SRI, true), narHash.to_string(HashFormat::SRI, true));
+                to_string(),
+                store->printStorePath(storePath),
+                prevNarHash->to_string(HashFormat::SRI, true),
+                narHash.to_string(HashFormat::SRI, true));
     }
 
     if (auto prevLastModified = getLastModified()) {
@@ -175,7 +173,7 @@ std::pair<Tree, Input> Input::fetch(ref<Store> store) const
 
     input.locked = true;
 
-    return {std::move(tree), input};
+    return {std::move(storePath), input};
 }
 
 Input Input::applyOverrides(

src/libfetchers/fetchers.hh

@@ -13,12 +13,6 @@ namespace nix { class Store; }
 
 namespace nix::fetchers {
 
-struct Tree
-{
-    Path actualPath;
-    StorePath storePath;
-};
-
 struct InputScheme;
 
 /**
@@ -83,10 +77,10 @@ public:
     bool contains(const Input & other) const;
 
     /**
-     * Fetch the input into the Nix store, returning the location in
+     * Fetch the entire input into the Nix store, returning the
-     * the Nix store and the locked input.
+     * location in the Nix store and the locked input.
      */
-    std::pair<Tree, Input> fetch(ref<Store> store) const;
+    std::pair<StorePath, Input> fetch(ref<Store> store) const;
 
     Input applyOverrides(
         std::optional<std::string> ref,
@@ -158,33 +152,4 @@ struct InputScheme
 
 void registerInputScheme(std::shared_ptr<InputScheme> && fetcher);
 
-struct DownloadFileResult
-{
-    StorePath storePath;
-    std::string etag;
-    std::string effectiveUrl;
-    std::optional<std::string> immutableUrl;
-};
-
-DownloadFileResult downloadFile(
-    ref<Store> store,
-    const std::string & url,
-    const std::string & name,
-    bool locked,
-    const Headers & headers = {});
-
-struct DownloadTarballResult
-{
-    Tree tree;
-    time_t lastModified;
-    std::optional<std::string> immutableUrl;
-};
-
-DownloadTarballResult downloadTarball(
-    ref<Store> store,
-    const std::string & url,
-    const std::string & name,
-    bool locked,
-    const Headers & headers = {});
-
 }

src/libfetchers/github.cc

@@ -7,6 +7,7 @@
 #include "git.hh"
 #include "fetchers.hh"
 #include "fetch-settings.hh"
+#include "tarball.hh"
 
 #include <optional>
 #include <nlohmann/json.hpp>
@@ -213,10 +214,10 @@ struct GitArchiveInputScheme : InputScheme
                 {"rev", rev->gitRev()},
                 {"lastModified", uint64_t(result.lastModified)}
             },
-            result.tree.storePath,
+            result.storePath,
             true);
 
-        return {result.tree.storePath, input};
+        return {result.storePath, input};
     }
 
     std::optional<ExperimentalFeature> experimentalFeature() override

src/libfetchers/registry.cc

@@ -1,5 +1,5 @@
 #include "registry.hh"
-#include "fetchers.hh"
+#include "tarball.hh"
 #include "util.hh"
 #include "globals.hh"
 #include "store-api.hh"

src/libfetchers/tarball.cc

@@ -1,3 +1,4 @@
+#include "tarball.hh"
 #include "fetchers.hh"
 #include "cache.hh"
 #include "filetransfer.hh"
@@ -133,7 +134,7 @@ DownloadTarballResult downloadTarball(
 
     if (cached && !cached->expired)
         return {
-            .tree = Tree { .actualPath = store->toRealPath(cached->storePath), .storePath = std::move(cached->storePath) },
+            .storePath = std::move(cached->storePath),
             .lastModified = (time_t) getIntAttr(cached->infoAttrs, "lastModified"),
             .immutableUrl = maybeGetStrAttr(cached->infoAttrs, "immutableUrl"),
         };
@@ -174,7 +175,7 @@ DownloadTarballResult downloadTarball(
         locked);
 
     return {
-        .tree = Tree { .actualPath = store->toRealPath(*unpackedStorePath), .storePath = std::move(*unpackedStorePath) },
+        .storePath = std::move(*unpackedStorePath),
         .lastModified = lastModified,
         .immutableUrl = res.immutableUrl,
     };
@@ -307,7 +308,7 @@ struct TarballInputScheme : CurlInputScheme
         if (result.lastModified && !input.attrs.contains("lastModified"))
             input.attrs.insert_or_assign("lastModified", uint64_t(result.lastModified));
 
-        return {result.tree.storePath, std::move(input)};
+        return {result.storePath, std::move(input)};
     }
 };
 

src/libfetchers/tarball.hh

@@ -0,0 +1,43 @@
+#pragma once
+
+#include "types.hh"
+#include "path.hh"
+
+#include <optional>
+
+namespace nix {
+class Store;
+}
+
+namespace nix::fetchers {
+
+struct DownloadFileResult
+{
+    StorePath storePath;
+    std::string etag;
+    std::string effectiveUrl;
+    std::optional<std::string> immutableUrl;
+};
+
+DownloadFileResult downloadFile(
+    ref<Store> store,
+    const std::string & url,
+    const std::string & name,
+    bool locked,
+    const Headers & headers = {});
+
+struct DownloadTarballResult
+{
+    StorePath storePath;
+    time_t lastModified;
+    std::optional<std::string> immutableUrl;
+};
+
+DownloadTarballResult downloadTarball(
+    ref<Store> store,
+    const std::string & url,
+    const std::string & name,
+    bool locked,
+    const Headers & headers = {});
+
+}

src/nix-channel/nix-channel.cc

@@ -4,9 +4,9 @@
 #include "filetransfer.hh"
 #include "store-api.hh"
 #include "legacy.hh"
-#include "fetchers.hh"
 #include "eval-settings.hh" // for defexpr
 #include "util.hh"
+#include "tarball.hh"
 
 #include <fcntl.h>
 #include <regex>

src/nix/flake.cc

@@ -186,7 +186,7 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON
                 j["revCount"] = *revCount;
             if (auto lastModified = flake.lockedRef.input.getLastModified())
                 j["lastModified"] = *lastModified;
-            j["path"] = store->printStorePath(flake.sourceInfo->storePath);
+            j["path"] = store->printStorePath(flake.storePath);
             j["locks"] = lockedFlake.lockFile.toJSON();
             logger->cout("%s", j.dump());
         } else {
@@ -202,7 +202,7 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON
                     *flake.description);
             logger->cout(
                 ANSI_BOLD "Path:" ANSI_NORMAL "          %s",
-                store->printStorePath(flake.sourceInfo->storePath));
+                store->printStorePath(flake.storePath));
             if (auto rev = flake.lockedRef.input.getRev())
                 logger->cout(
                     ANSI_BOLD "Revision:" ANSI_NORMAL "      %s",
@@ -976,7 +976,7 @@ struct CmdFlakeArchive : FlakeCommand, MixJSON, MixDryRun
 
         StorePathSet sources;
 
-        sources.insert(flake.flake.sourceInfo->storePath);
+        sources.insert(flake.flake.storePath);
 
         // FIXME: use graph output, handle cycles.
         std::function<nlohmann::json(const Node & node)> traverse;
@@ -988,7 +988,7 @@ struct CmdFlakeArchive : FlakeCommand, MixJSON, MixDryRun
                     auto storePath =
                         dryRun
                         ? (*inputNode)->lockedRef.input.computeStorePath(*store)
-                        : (*inputNode)->lockedRef.input.fetch(store).first.storePath;
+                        : (*inputNode)->lockedRef.input.fetch(store).first;
                     if (json) {
                         auto& jsonObj3 = jsonObj2[inputName];
                         jsonObj3["path"] = store->printStorePath(storePath);
@@ -1005,7 +1005,7 @@ struct CmdFlakeArchive : FlakeCommand, MixJSON, MixDryRun
 
         if (json) {
             nlohmann::json jsonRoot = {
-                {"path", store->printStorePath(flake.flake.sourceInfo->storePath)},
+                {"path", store->printStorePath(flake.flake.storePath)},
                 {"inputs", traverse(*flake.lockFile.root)},
             };
             logger->cout("%s", jsonRoot);
@@ -1339,12 +1339,12 @@ struct CmdFlakePrefetch : FlakeCommand, MixJSON
     {
         auto originalRef = getFlakeRef();
         auto resolvedRef = originalRef.resolve(store);
-        auto [tree, lockedRef] = resolvedRef.fetchTree(store);
+        auto [storePath, lockedRef] = resolvedRef.fetchTree(store);
-        auto hash = store->queryPathInfo(tree.storePath)->narHash;
+        auto hash = store->queryPathInfo(storePath)->narHash;
 
         if (json) {
             auto res = nlohmann::json::object();
-            res["storePath"] = store->printStorePath(tree.storePath);
+            res["storePath"] = store->printStorePath(storePath);
             res["hash"] = hash.to_string(HashFormat::SRI, true);
             res["original"] = fetchers::attrsToJSON(resolvedRef.toAttrs());
             res["locked"] = fetchers::attrsToJSON(lockedRef.toAttrs());
@@ -1352,7 +1352,7 @@ struct CmdFlakePrefetch : FlakeCommand, MixJSON
         } else {
             notice("Downloaded '%s' to '%s' (hash '%s').",
                 lockedRef.to_string(),
-                store->printStorePath(tree.storePath),
+                store->printStorePath(storePath),
                 hash.to_string(HashFormat::SRI, true));
         }
     }