From 0e07f81d2ba532e140539e91b57d6f85c952fee2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 4 Mar 2024 22:17:24 +0100 Subject: [PATCH] Fetcher cleanups * Convert all InputScheme::fetch() methods to getAccessor(). * Add checkLocks() method for checking lock attributes. * Rename fetch() to fetchToStore(). --- src/libexpr/flake/flakeref.cc | 2 +- src/libexpr/primops/fetchMercurial.cc | 3 +- src/libexpr/primops/fetchTree.cc | 2 +- src/libfetchers/fetchers.cc | 107 +++++++++++++++----------- src/libfetchers/fetchers.hh | 25 +++++- src/libfetchers/git.cc | 2 - src/libfetchers/github.cc | 2 - src/libfetchers/indirect.cc | 2 +- src/libfetchers/mercurial.cc | 26 ++++--- src/libfetchers/path.cc | 6 +- src/nix/flake.cc | 2 +- src/nix/registry.cc | 4 +- 12 files changed, 112 insertions(+), 71 deletions(-) diff --git a/src/libexpr/flake/flakeref.cc b/src/libexpr/flake/flakeref.cc index 86a0982f3..6fe64fd72 100644 --- a/src/libexpr/flake/flakeref.cc +++ b/src/libexpr/flake/flakeref.cc @@ -274,7 +274,7 @@ FlakeRef FlakeRef::fromAttrs(const fetchers::Attrs & attrs) std::pair FlakeRef::fetchTree(ref store) const { - auto [storePath, lockedInput] = input.fetch(store); + auto [storePath, lockedInput] = input.fetchToStore(store); return {std::move(storePath), FlakeRef(std::move(lockedInput), subdir)}; } diff --git a/src/libexpr/primops/fetchMercurial.cc b/src/libexpr/primops/fetchMercurial.cc index bb029b5b3..bfc19115a 100644 --- a/src/libexpr/primops/fetchMercurial.cc +++ b/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)); diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index cfedfa6c4..5061e40fd 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/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); diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 363ad018e..483796f0b 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -161,7 +161,7 @@ bool Input::contains(const Input & other) const return false; } -std::pair Input::fetch(ref store) const +std::pair Input::fetchToStore(ref store) const { if (!scheme) throw Error("cannot fetch unsupported input '%s'", attrsToJSON(toAttrs())); @@ -186,56 +186,85 @@ std::pair Input::fetch(ref store) const auto [storePath, input] = [&]() -> std::pair { 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, Input> Input::getAccessor(ref 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, Input> Input::getAccessorUnchecked(ref 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 ref, std::optional 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 InputScheme::fetch(ref 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, Input> InputScheme::getAccessor(ref store, const Input & input) const -{ - throw UnimplementedError("InputScheme must implement fetch() or getAccessor()"); -} - std::optional InputScheme::experimentalFeature() const { return {}; diff --git a/src/libfetchers/fetchers.hh b/src/libfetchers/fetchers.hh index 472fba6f4..cd11f9eae 100644 --- a/src/libfetchers/fetchers.hh +++ b/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 fetch(ref store) const; + std::pair fetchToStore(ref 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, Input> getAccessor(ref store) const; +private: + + std::pair, Input> getAccessorUnchecked(ref store) const; + +public: + Input applyOverrides( std::optional ref, std::optional rev) const; @@ -173,9 +184,7 @@ struct InputScheme std::string_view contents, std::optional commitMsg) const; - virtual std::pair fetch(ref store, const Input & input); - - virtual std::pair, Input> getAccessor(ref store, const Input & input) const; + virtual std::pair, Input> getAccessor(ref 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 && fetcher); diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 87d114276..25eabb1dc 100644 --- a/src/libfetchers/git.cc +++ b/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)}; } diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index a48c99a0b..d9d348756 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -275,8 +275,6 @@ struct GitArchiveInputScheme : InputScheme accessor->setPathDisplay("«" + input.to_string() + "»"); - accessor->fingerprint = input.getFingerprint(store); - return {accessor, input}; } diff --git a/src/libfetchers/indirect.cc b/src/libfetchers/indirect.cc index 002c0c292..3f21445e1 100644 --- a/src/libfetchers/indirect.cc +++ b/src/libfetchers/indirect.cc @@ -97,7 +97,7 @@ struct IndirectInputScheme : InputScheme return input; } - std::pair fetch(ref store, const Input & input) override + std::pair, Input> getAccessor(ref store, const Input & input) const override { throw Error("indirect input '%s' cannot be fetched directly", input.to_string()); } diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index a5f55a44e..a2702338f 100644 --- a/src/libfetchers/mercurial.cc +++ b/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 @@ -161,9 +161,9 @@ struct MercurialInputScheme : InputScheme return {isLocal, isLocal ? url.path : url.base}; } - std::pair fetch(ref store, const Input & _input) override + StorePath fetchToStore(ref 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 + 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, Input> getAccessor(ref 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(); diff --git a/src/libfetchers/path.cc b/src/libfetchers/path.cc index 276fd1b36..6cc482ebf 100644 --- a/src/libfetchers/path.cc +++ b/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 fetch(ref store, const Input & _input) override + std::pair, Input> getAccessor(ref 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 experimentalFeature() const override diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 5fc3f4166..5e4269588 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -1050,7 +1050,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); diff --git a/src/nix/registry.cc b/src/nix/registry.cc index 0346ec1e0..812429240 100644 --- a/src/nix/registry.cc +++ b/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);