Merge pull request #10152 from edolstra/fetcher-cleanups

Fetcher cleanups

src/libexpr/flake/flakeref.cc

@@ -284,7 +284,7 @@ FlakeRef FlakeRef::fromAttrs(const fetchers::Attrs & attrs)
 
 std::pair<StorePath, FlakeRef> FlakeRef::fetchTree(ref<Store> store) const
 {
-    auto [storePath, lockedInput] = input.fetch(store);
+    auto [storePath, lockedInput] = input.fetchToStore(store);
     return {std::move(storePath), FlakeRef(std::move(lockedInput), subdir)};
 }
 

src/libexpr/primops/fetchMercurial.cc

@@ -64,8 +64,7 @@ static void prim_fetchMercurial(EvalState & state, const PosIdx pos, Value * * a
     if (rev) attrs.insert_or_assign("rev", rev->gitRev());
     auto input = fetchers::Input::fromAttrs(std::move(attrs));
 
-    // FIXME: use name
-    auto [storePath, input2] = input.fetch(state.store);
+    auto [storePath, input2] = input.fetchToStore(state.store);
 
     auto attrs2 = state.buildBindings(8);
     state.mkStorePathString(storePath, attrs2.alloc(state.sOutPath));

src/libexpr/primops/fetchTree.cc

@@ -182,7 +182,7 @@ static void fetchTree(
 
     state.checkURI(input.toURLString());
 
-    auto [storePath, input2] = input.fetch(state.store);
+    auto [storePath, input2] = input.fetchToStore(state.store);
 
     state.allowPath(storePath);
 

src/libfetchers/fetchers.cc

@@ -161,7 +161,7 @@ bool Input::contains(const Input & other) const
     return false;
 }
 
-std::pair<StorePath, Input> Input::fetch(ref<Store> store) const
+std::pair<StorePath, Input> Input::fetchToStore(ref<Store> store) const
 {
     if (!scheme)
         throw Error("cannot fetch unsupported input '%s'", attrsToJSON(toAttrs()));
@@ -186,56 +186,85 @@ std::pair<StorePath, Input> Input::fetch(ref<Store> store) const
 
     auto [storePath, input] = [&]() -> std::pair<StorePath, Input> {
         try {
-            return scheme->fetch(store, *this);
+            auto [accessor, final] = getAccessorUnchecked(store);
+
+            auto storePath = nix::fetchToStore(*store, SourcePath(accessor), FetchMode::Copy, final.getName());
+
+            auto narHash = store->queryPathInfo(storePath)->narHash;
+            final.attrs.insert_or_assign("narHash", narHash.to_string(HashFormat::SRI, true));
+
+            scheme->checkLocks(*this, final);
+
+            return {storePath, final};
         } catch (Error & e) {
             e.addTrace({}, "while fetching the input '%s'", to_string());
             throw;
         }
     }();
 
-    auto narHash = store->queryPathInfo(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(),
-                store->printStorePath(storePath),
-                prevNarHash->to_string(HashFormat::SRI, true),
-                narHash.to_string(HashFormat::SRI, true));
-    }
-
-    if (auto prevLastModified = getLastModified()) {
-        if (input.getLastModified() != prevLastModified)
-            throw Error("'lastModified' attribute mismatch in input '%s', expected %d",
-                input.to_string(), *prevLastModified);
-    }
-
-    if (auto prevRev = getRev()) {
-        if (input.getRev() != prevRev)
-            throw Error("'rev' attribute mismatch in input '%s', expected %s",
-                input.to_string(), prevRev->gitRev());
-    }
-
-    if (auto prevRevCount = getRevCount()) {
-        if (input.getRevCount() != prevRevCount)
-            throw Error("'revCount' attribute mismatch in input '%s', expected %d",
-                input.to_string(), *prevRevCount);
-    }
-
     return {std::move(storePath), input};
 }
 
+void InputScheme::checkLocks(const Input & specified, const Input & final) const
+{
+    if (auto prevNarHash = specified.getNarHash()) {
+        if (final.getNarHash() != prevNarHash) {
+            if (final.getNarHash())
+                throw Error((unsigned int) 102, "NAR hash mismatch in input '%s', expected '%s' but got '%s'",
+                    specified.to_string(), prevNarHash->to_string(HashFormat::SRI, true), final.getNarHash()->to_string(HashFormat::SRI, true));
+            else
+                throw Error((unsigned int) 102, "NAR hash mismatch in input '%s', expected '%s' but got none",
+                    specified.to_string(), prevNarHash->to_string(HashFormat::SRI, true));
+        }
+    }
+
+    if (auto prevLastModified = specified.getLastModified()) {
+        if (final.getLastModified() != prevLastModified)
+            throw Error("'lastModified' attribute mismatch in input '%s', expected %d",
+                final.to_string(), *prevLastModified);
+    }
+
+    if (auto prevRev = specified.getRev()) {
+        if (final.getRev() != prevRev)
+            throw Error("'rev' attribute mismatch in input '%s', expected %s",
+                final.to_string(), prevRev->gitRev());
+    }
+
+    if (auto prevRevCount = specified.getRevCount()) {
+        if (final.getRevCount() != prevRevCount)
+            throw Error("'revCount' attribute mismatch in input '%s', expected %d",
+                final.to_string(), *prevRevCount);
+    }
+}
+
 std::pair<ref<InputAccessor>, Input> Input::getAccessor(ref<Store> store) const
 {
     try {
-        return scheme->getAccessor(store, *this);
+        auto [accessor, final] = getAccessorUnchecked(store);
+
+        scheme->checkLocks(*this, final);
+
+        return {accessor, std::move(final)};
     } catch (Error & e) {
         e.addTrace({}, "while fetching the input '%s'", to_string());
         throw;
     }
 }
 
+std::pair<ref<InputAccessor>, Input> Input::getAccessorUnchecked(ref<Store> store) const
+{
+    // FIXME: cache the accessor
+
+    if (!scheme)
+        throw Error("cannot fetch unsupported input '%s'", attrsToJSON(toAttrs()));
+
+    auto [accessor, final] = scheme->getAccessor(store, *this);
+
+    accessor->fingerprint = scheme->getFingerprint(store, final);
+
+    return {accessor, std::move(final)};
+}
+
 Input Input::applyOverrides(
     std::optional<std::string> ref,
     std::optional<Hash> rev) const
@@ -372,18 +401,6 @@ void InputScheme::clone(const Input & input, const Path & destDir) const
     throw Error("do not know how to clone input '%s'", input.to_string());
 }
 
-std::pair<StorePath, Input> InputScheme::fetch(ref<Store> store, const Input & input)
-{
-    auto [accessor, input2] = getAccessor(store, input);
-    auto storePath = fetchToStore(*store, SourcePath(accessor), FetchMode::Copy, input2.getName());
-    return {storePath, input2};
-}
-
-std::pair<ref<InputAccessor>, Input> InputScheme::getAccessor(ref<Store> store, const Input & input) const
-{
-    throw UnimplementedError("InputScheme must implement fetch() or getAccessor()");
-}
-
 std::optional<ExperimentalFeature> InputScheme::experimentalFeature() const
 {
     return {};

src/libfetchers/fetchers.hh

@@ -80,10 +80,21 @@ public:
      * Fetch the entire input into the Nix store, returning the
      * location in the Nix store and the locked input.
      */
-    std::pair<StorePath, Input> fetch(ref<Store> store) const;
+    std::pair<StorePath, Input> fetchToStore(ref<Store> store) const;
 
+    /**
+     * Return an InputAccessor that allows access to files in the
+     * input without copying it to the store. Also return a possibly
+     * unlocked input.
+     */
     std::pair<ref<InputAccessor>, Input> getAccessor(ref<Store> store) const;
 
+private:
+
+    std::pair<ref<InputAccessor>, Input> getAccessorUnchecked(ref<Store> store) const;
+
+public:
+
     Input applyOverrides(
         std::optional<std::string> ref,
         std::optional<Hash> rev) const;
@@ -173,9 +184,7 @@ struct InputScheme
         std::string_view contents,
         std::optional<std::string> commitMsg) const;
 
-    virtual std::pair<StorePath, Input> fetch(ref<Store> store, const Input & input);
-
-    virtual std::pair<ref<InputAccessor>, Input> getAccessor(ref<Store> store, const Input & input) const;
+    virtual std::pair<ref<InputAccessor>, Input> getAccessor(ref<Store> store, const Input & input) const = 0;
 
     /**
      * Is this `InputScheme` part of an experimental feature?
@@ -202,6 +211,14 @@ struct InputScheme
      */
     virtual bool isLocked(const Input & input) const
     { return false; }
+
+    /**
+     * Check the locking attributes in `final` against
+     * `specified`. E.g. if `specified` has a `rev` attribute, then
+     * `final` must have the same `rev` attribute. Throw an exception
+     * if there is a mismatch.
+     */
+    virtual void checkLocks(const Input & specified, const Input & final) const;
 };
 
 void registerInputScheme(std::shared_ptr<InputScheme> && fetcher);

src/libfetchers/git.cc

@@ -761,8 +761,6 @@ struct GitInputScheme : InputScheme
             ? getAccessorFromCommit(store, repoInfo, std::move(input))
             : getAccessorFromWorkdir(store, repoInfo, std::move(input));
 
-        accessor->fingerprint = final.getFingerprint(store);
-
         return {accessor, std::move(final)};
     }
 

src/libfetchers/github.cc

@@ -275,8 +275,6 @@ struct GitArchiveInputScheme : InputScheme
 
         accessor->setPathDisplay("«" + input.to_string() + "»");
 
-        accessor->fingerprint = input.getFingerprint(store);
-
         return {accessor, input};
     }
 

src/libfetchers/indirect.cc

@@ -97,7 +97,7 @@ struct IndirectInputScheme : InputScheme
         return input;
     }
 
-    std::pair<StorePath, Input> fetch(ref<Store> store, const Input & input) override
+    std::pair<ref<InputAccessor>, Input> getAccessor(ref<Store> store, const Input & input) const override
     {
         throw Error("indirect input '%s' cannot be fetched directly", input.to_string());
     }

src/libfetchers/mercurial.cc

@@ -6,8 +6,8 @@
 #include "tarfile.hh"
 #include "store-api.hh"
 #include "url-parts.hh"
+#include "fs-input-accessor.hh"
 #include "posix-source-accessor.hh"
-
 #include "fetch-settings.hh"
 
 #include <sys/time.h>
@@ -161,9 +161,9 @@ struct MercurialInputScheme : InputScheme
         return {isLocal, isLocal ? url.path : url.base};
     }
 
-    std::pair<StorePath, Input> fetch(ref<Store> store, const Input & _input) override
+    StorePath fetchToStore(ref<Store> store, Input & input) const
     {
-        Input input(_input);
+        auto origRev = input.getRev();
 
         auto name = input.getName();
 
@@ -218,7 +218,7 @@ struct MercurialInputScheme : InputScheme
                     FileIngestionMethod::Recursive, HashAlgorithm::SHA256, {},
                     filter);
 
-                return {std::move(storePath), input};
+                return storePath;
             }
         }
 
@@ -242,13 +242,12 @@ struct MercurialInputScheme : InputScheme
             });
         };
 
-        auto makeResult = [&](const Attrs & infoAttrs, StorePath && storePath)
-            -> std::pair<StorePath, Input>
+        auto makeResult = [&](const Attrs & infoAttrs, const StorePath & storePath) -> StorePath
         {
             assert(input.getRev());
-            assert(!_input.getRev() || _input.getRev() == input.getRev());
+            assert(!origRev || origRev == input.getRev());
             input.attrs.insert_or_assign("revCount", getIntAttr(infoAttrs, "revCount"));
-            return {std::move(storePath), input};
+            return storePath;
         };
 
         if (input.getRev()) {
@@ -329,7 +328,7 @@ struct MercurialInputScheme : InputScheme
             {"revCount", (uint64_t) revCount},
         });
 
-        if (!_input.getRev())
+        if (!origRev)
             getCache()->add(
                 *store,
                 unlockedAttrs,
@@ -347,6 +346,15 @@ struct MercurialInputScheme : InputScheme
         return makeResult(infoAttrs, std::move(storePath));
     }
 
+    std::pair<ref<InputAccessor>, Input> getAccessor(ref<Store> store, const Input & _input) const override
+    {
+        Input input(_input);
+
+        auto storePath = fetchToStore(store, input);
+
+        return {makeStorePathAccessor(store, storePath), input};
+    }
+
     bool isLocked(const Input & input) const override
     {
         return (bool) input.getRev();

src/libfetchers/path.cc

@@ -1,6 +1,8 @@
 #include "fetchers.hh"
 #include "store-api.hh"
 #include "archive.hh"
+#include "fs-input-accessor.hh"
+#include "posix-source-accessor.hh"
 
 namespace nix::fetchers {
 
@@ -102,7 +104,7 @@ struct PathInputScheme : InputScheme
         throw Error("cannot fetch input '%s' because it uses a relative path", input.to_string());
     }
 
-    std::pair<StorePath, Input> fetch(ref<Store> store, const Input & _input) override
+    std::pair<ref<InputAccessor>, Input> getAccessor(ref<Store> store, const Input & _input) const override
     {
         Input input(_input);
         std::string absPath;
@@ -144,7 +146,7 @@ struct PathInputScheme : InputScheme
         }
         input.attrs.insert_or_assign("lastModified", uint64_t(mtime));
 
-        return {std::move(*storePath), input};
+        return {makeStorePathAccessor(store, *storePath), std::move(input)};
     }
 
     std::optional<std::string> getFingerprint(ref<Store> store, const Input & input) const override

src/nix/flake.cc

@@ -1053,7 +1053,7 @@ struct CmdFlakeArchive : FlakeCommand, MixJSON, MixDryRun
                     auto storePath =
                         dryRun
                         ? (*inputNode)->lockedRef.input.computeStorePath(*store)
-                        : (*inputNode)->lockedRef.input.fetch(store).first;
+                        : (*inputNode)->lockedRef.input.fetchToStore(store).first;
                     if (json) {
                         auto& jsonObj3 = jsonObj2[inputName];
                         jsonObj3["path"] = store->printStorePath(storePath);

src/nix/registry.cc

@@ -188,7 +188,9 @@ struct CmdRegistryPin : RegistryCommand, EvalCommand
         auto ref = parseFlakeRef(url);
         auto lockedRef = parseFlakeRef(locked);
         registry->remove(ref.input);
-        auto [tree, resolved] = lockedRef.resolve(store).input.fetch(store);
+        auto resolved = lockedRef.resolve(store).input.getAccessor(store).second;
+        if (!resolved.isLocked())
+            warn("flake '%s' is not locked", resolved.to_string());
         fetchers::Attrs extraAttrs;
         if (ref.subdir != "") extraAttrs["dir"] = ref.subdir;
         registry->add(ref.input, resolved, extraAttrs);