Merge pull request #11701 from DeterminateSystems/flake-substitution

Restore input substitution

doc/manual/source/release-notes/rl-2.25.md

@@ -70,12 +70,6 @@
 
   Author: [**@zimbatm**](https://github.com/zimbatm)
 
-- Flakes are no longer substituted [#10612](https://github.com/NixOS/nix/pull/10612)
-
-  Nix will no longer attempt to substitute the source code of flakes from a binary cache. This functionality was broken because it could lead to different evaluation results depending on whether the flake was available in the binary cache, or even depending on whether the flake was already in the local store.
-
-  Author: [**@edolstra**](https://github.com/edolstra)
-
 - `<nix/fetchurl.nix>` uses TLS verification [#11585](https://github.com/NixOS/nix/pull/11585)
 
   Previously `<nix/fetchurl.nix>` did not do TLS verification. This was because the Nix sandbox in the past did not have access to TLS certificates, and Nix checks the hash of the fetched file anyway. However, this can expose authentication data from `netrc` and URLs to man-in-the-middle attackers. In addition, Nix now in some cases (such as when using impure derivations) does *not* check the hash. Therefore we have now enabled TLS verification. This means that downloads by `<nix/fetchurl.nix>` will now fail if you're fetching from a HTTPS server that does not have a valid certificate.

src/libexpr/call-flake.nix

@@ -10,6 +10,9 @@ lockFileStr:
 # unlocked trees.
 overrides:
 
+# This is `prim_fetchFinalTree`.
+fetchTreeFinal:
+
 let
 
   lockFile = builtins.fromJSON lockFileStr;
@@ -44,7 +47,8 @@ let
               overrides.${key}.sourceInfo
             else
               # FIXME: remove obsolete node.info.
-              fetchTree (node.info or {} // removeAttrs node.locked ["dir"]);
+              # Note: lock file entries are always final.
+              fetchTreeFinal (node.info or {} // removeAttrs node.locked ["dir"]);
 
           subdir = overrides.${key}.dir or node.locked.dir or "";
 

src/libexpr/eval.cc

@@ -510,9 +510,15 @@ Value * EvalState::addPrimOp(PrimOp && primOp)
 
     Value * v = allocValue();
     v->mkPrimOp(new PrimOp(primOp));
-    staticBaseEnv->vars.emplace_back(envName, baseEnvDispl);
+
-    baseEnv.values[baseEnvDispl++] = v;
+    if (primOp.internal)
-    baseEnv.values[0]->payload.attrs->push_back(Attr(symbols.create(primOp.name), v));
+        internalPrimOps.emplace(primOp.name, v);
+    else {
+        staticBaseEnv->vars.emplace_back(envName, baseEnvDispl);
+        baseEnv.values[baseEnvDispl++] = v;
+        baseEnv.values[0]->payload.attrs->push_back(Attr(symbols.create(primOp.name), v));
+    }
+
     return v;
 }
 

src/libexpr/eval.hh

@@ -107,6 +107,11 @@ struct PrimOp
      */
     std::optional<ExperimentalFeature> experimentalFeature;
 
+    /**
+     * If true, this primop is not exposed to the user.
+     */
+    bool internal = false;
+
     /**
      * Validity check to be performed by functions that introduce primops,
      * such as RegisterPrimOp() and Value::mkPrimOp().
@@ -591,6 +596,11 @@ public:
      */
     std::shared_ptr<StaticEnv> staticBaseEnv; // !!! should be private
 
+    /**
+     * Internal primops not exposed to the user.
+     */
+    std::unordered_map<std::string, Value *, std::hash<std::string>, std::equal_to<std::string>, traceable_allocator<std::pair<const std::string, Value *>>> internalPrimOps;
+
     /**
      * Name and documentation about every constant.
      *

src/libexpr/primops/fetchTree.cc

@@ -78,6 +78,7 @@ struct FetchTreeParams {
     bool emptyRevFallback = false;
     bool allowNameArgument = false;
     bool isFetchGit = false;
+    bool isFinal = false;
 };
 
 static void fetchTree(
@@ -195,6 +196,13 @@ static void fetchTree(
 
     state.checkURI(input.toURLString());
 
+    if (params.isFinal) {
+        input.attrs.insert_or_assign("__final", Explicit<bool>(true));
+    } else {
+        if (input.isFinal())
+            throw Error("input '%s' is not allowed to use the '__final' attribute", input.to_string());
+    }
+
     auto [storePath, input2] = input.fetchToStore(state.store);
 
     state.allowPath(storePath);
@@ -431,6 +439,18 @@ static RegisterPrimOp primop_fetchTree({
     .experimentalFeature = Xp::FetchTree,
 });
 
+void prim_fetchFinalTree(EvalState & state, const PosIdx pos, Value * * args, Value & v)
+{
+    fetchTree(state, pos, args, v, {.isFinal = true});
+}
+
+static RegisterPrimOp primop_fetchFinalTree({
+    .name = "fetchFinalTree",
+    .args = {"input"},
+    .fun = prim_fetchFinalTree,
+    .internal = true,
+});
+
 static void fetch(EvalState & state, const PosIdx pos, Value * * args, Value & v,
     const std::string & who, bool unpack, std::string name)
 {

src/libfetchers/fetchers.cc

@@ -3,6 +3,7 @@
 #include "source-path.hh"
 #include "fetch-to-store.hh"
 #include "json-utils.hh"
+#include "store-path-accessor.hh"
 
 #include <nlohmann/json.hpp>
 
@@ -100,7 +101,7 @@ Input Input::fromAttrs(const Settings & settings, Attrs && attrs)
     auto allowedAttrs = inputScheme->allowedAttrs();
 
     for (auto & [name, _] : attrs)
-        if (name != "type" && allowedAttrs.count(name) == 0)
+        if (name != "type" && name != "__final" && allowedAttrs.count(name) == 0)
             throw Error("input attribute '%s' not supported by scheme '%s'", name, schemeName);
 
     auto res = inputScheme->inputFromAttrs(settings, attrs);
@@ -145,6 +146,11 @@ bool Input::isLocked() const
     return scheme && scheme->isLocked(*this);
 }
 
+bool Input::isFinal() const
+{
+    return maybeGetBoolAttr(attrs, "__final").value_or(false);
+}
+
 Attrs Input::toAttrs() const
 {
     return attrs;
@@ -172,16 +178,24 @@ std::pair<StorePath, Input> Input::fetchToStore(ref<Store> store) const
 
     auto [storePath, input] = [&]() -> std::pair<StorePath, Input> {
         try {
-            auto [accessor, final] = getAccessorUnchecked(store);
+            auto [accessor, result] = getAccessorUnchecked(store);
 
-            auto storePath = nix::fetchToStore(*store, SourcePath(accessor), FetchMode::Copy, final.getName());
+            auto storePath = nix::fetchToStore(*store, SourcePath(accessor), FetchMode::Copy, result.getName());
 
             auto narHash = store->queryPathInfo(storePath)->narHash;
-            final.attrs.insert_or_assign("narHash", narHash.to_string(HashFormat::SRI, true));
+            result.attrs.insert_or_assign("narHash", narHash.to_string(HashFormat::SRI, true));
 
-            scheme->checkLocks(*this, final);
+            // FIXME: we would like to mark inputs as final in
+            // getAccessorUnchecked(), but then we can't add
+            // narHash. Or maybe narHash should be excluded from the
+            // concept of "final" inputs?
+            result.attrs.insert_or_assign("__final", Explicit<bool>(true));
 
-            return {storePath, final};
+            assert(result.isFinal());
+
+            checkLocks(*this, result);
+
+            return {storePath, result};
         } catch (Error & e) {
             e.addTrace({}, "while fetching the input '%s'", to_string());
             throw;
@@ -191,13 +205,40 @@ std::pair<StorePath, Input> Input::fetchToStore(ref<Store> store) const
     return {std::move(storePath), input};
 }
 
-void InputScheme::checkLocks(const Input & specified, const Input & final) const
+void Input::checkLocks(Input specified, Input & result)
 {
+    /* If the original input is final, then we just return the
+       original attributes, dropping any new fields returned by the
+       fetcher. However, any fields that are in both the specified and
+       result input must be identical. */
+    if (specified.isFinal()) {
+
+        /* Backwards compatibility hack: we had some lock files in the
+           past that 'narHash' fields with incorrect base-64
+           formatting (lacking the trailing '=', e.g. 'sha256-ri...Mw'
+           instead of ''sha256-ri...Mw='). So fix that. */
+        if (auto prevNarHash = specified.getNarHash())
+            specified.attrs.insert_or_assign("narHash", prevNarHash->to_string(HashFormat::SRI, true));
+
+        for (auto & field : specified.attrs) {
+            auto field2 = result.attrs.find(field.first);
+            if (field2 != result.attrs.end() && field.second != field2->second)
+                throw Error("mismatch in field '%s' of input '%s', got '%s'",
+                    field.first,
+                    attrsToJSON(specified.attrs),
+                    attrsToJSON(result.attrs));
+        }
+
+        result.attrs = specified.attrs;
+
+        return;
+    }
+
     if (auto prevNarHash = specified.getNarHash()) {
-        if (final.getNarHash() != prevNarHash) {
+        if (result.getNarHash() != prevNarHash) {
-            if (final.getNarHash())
+            if (result.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));
+                    specified.to_string(), prevNarHash->to_string(HashFormat::SRI, true), result.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));
@@ -205,32 +246,32 @@ void InputScheme::checkLocks(const Input & specified, const Input & final) const
     }
 
     if (auto prevLastModified = specified.getLastModified()) {
-        if (final.getLastModified() != prevLastModified)
+        if (result.getLastModified() != prevLastModified)
-            throw Error("'lastModified' attribute mismatch in input '%s', expected %d",
+            throw Error("'lastModified' attribute mismatch in input '%s', expected %d, got %d",
-                final.to_string(), *prevLastModified);
+                result.to_string(), *prevLastModified, result.getLastModified().value_or(-1));
     }
 
     if (auto prevRev = specified.getRev()) {
-        if (final.getRev() != prevRev)
+        if (result.getRev() != prevRev)
             throw Error("'rev' attribute mismatch in input '%s', expected %s",
-                final.to_string(), prevRev->gitRev());
+                result.to_string(), prevRev->gitRev());
     }
 
     if (auto prevRevCount = specified.getRevCount()) {
-        if (final.getRevCount() != prevRevCount)
+        if (result.getRevCount() != prevRevCount)
             throw Error("'revCount' attribute mismatch in input '%s', expected %d",
-                final.to_string(), *prevRevCount);
+                result.to_string(), *prevRevCount);
     }
 }
 
 std::pair<ref<SourceAccessor>, Input> Input::getAccessor(ref<Store> store) const
 {
     try {
-        auto [accessor, final] = getAccessorUnchecked(store);
+        auto [accessor, result] = getAccessorUnchecked(store);
 
-        scheme->checkLocks(*this, final);
+        checkLocks(*this, result);
 
-        return {accessor, std::move(final)};
+        return {accessor, std::move(result)};
     } catch (Error & e) {
         e.addTrace({}, "while fetching the input '%s'", to_string());
         throw;
@@ -244,12 +285,42 @@ std::pair<ref<SourceAccessor>, Input> Input::getAccessorUnchecked(ref<Store> sto
     if (!scheme)
         throw Error("cannot fetch unsupported input '%s'", attrsToJSON(toAttrs()));
 
-    auto [accessor, final] = scheme->getAccessor(store, *this);
+    /* The tree may already be in the Nix store, or it could be
+       substituted (which is often faster than fetching from the
+       original source). So check that. We only do this for final
+       inputs, otherwise there is a risk that we don't return the
+       same attributes (like `lastModified`) that the "real" fetcher
+       would return.
+
+       FIXME: add a setting to disable this.
+       FIXME: substituting may be slower than fetching normally,
+       e.g. for fetchers like Git that are incremental!
+    */
+    if (isFinal() && getNarHash()) {
+        try {
+            auto storePath = computeStorePath(*store);
+
+            store->ensurePath(storePath);
+
+            debug("using substituted/cached input '%s' in '%s'",
+                to_string(), store->printStorePath(storePath));
+
+            auto accessor = makeStorePathAccessor(store, storePath);
+
+            accessor->fingerprint = scheme->getFingerprint(store, *this);
+
+            return {accessor, *this};
+        } catch (Error & e) {
+            debug("substitution of input '%s' failed: %s", to_string(), e.what());
+        }
+    }
+
+    auto [accessor, result] = scheme->getAccessor(store, *this);
 
     assert(!accessor->fingerprint);
-    accessor->fingerprint = scheme->getFingerprint(store, final);
+    accessor->fingerprint = scheme->getFingerprint(store, result);
 
-    return {accessor, std::move(final)};
+    return {accessor, std::move(result)};
 }
 
 Input Input::applyOverrides(

src/libfetchers/fetchers.hh

@@ -78,17 +78,30 @@ public:
     Attrs toAttrs() const;
 
     /**
-     * Check whether this is a "direct" input, that is, not
+     * Return whether this is a "direct" input, that is, not
      * one that goes through a registry.
      */
     bool isDirect() const;
 
     /**
-     * Check whether this is a "locked" input, that is,
+     * Return whether this is a "locked" input, that is, it has
-     * one that contains a commit hash or content hash.
+     * attributes like a Git revision or NAR hash that uniquely
+     * identify its contents.
      */
     bool isLocked() const;
 
+    /**
+     * Return whether this is a "final" input, meaning that fetching
+     * it will not add, remove or change any attributes. (See
+     * `checkLocks()` for the semantics.) Only "final" inputs can be
+     * substituted from a binary cache.
+     *
+     * The "final" state is denoted by the presence of an attribute
+     * `__final = true`. This attribute is currently undocumented and
+     * for internal use only.
+     */
+    bool isFinal() const;
+
     bool operator ==(const Input & other) const noexcept;
 
     bool contains(const Input & other) const;
@@ -99,6 +112,19 @@ public:
      */
     std::pair<StorePath, Input> fetchToStore(ref<Store> store) const;
 
+    /**
+     * Check the locking attributes in `result` against
+     * `specified`. E.g. if `specified` has a `rev` attribute, then
+     * `result` must have the same `rev` attribute. Throw an exception
+     * if there is a mismatch.
+     *
+     * If `specified` is marked final (i.e. has the `__final`
+     * attribute), then the intersection of attributes in `specified`
+     * and `result` must be equal, and `final.attrs` is set to
+     * `specified.attrs` (i.e. we discard any new attributes).
+     */
+    static void checkLocks(Input specified, Input & result);
+
     /**
      * Return a `SourceAccessor` that allows access to files in the
      * input without copying it to the store. Also return a possibly
@@ -144,6 +170,10 @@ public:
     /**
      * For locked inputs, return a string that uniquely specifies the
      * content of the input (typically a commit hash or content hash).
+     *
+     * Only known-equivalent inputs should return the same fingerprint.
+     *
+     * This is not a stable identifier between Nix versions, but not guaranteed to change either.
      */
     std::optional<std::string> getFingerprint(ref<Store> store) const;
 };
@@ -215,31 +245,11 @@ struct InputScheme
     virtual bool isDirect(const Input & input) const
     { return true; }
 
-    /**
-     * A sufficiently unique string that can be used as a cache key to identify the `input`.
-     *
-     * Only known-equivalent inputs should return the same fingerprint.
-     *
-     * This is not a stable identifier between Nix versions, but not guaranteed to change either.
-     */
     virtual std::optional<std::string> getFingerprint(ref<Store> store, const Input & input) const
     { return std::nullopt; }
 
-    /**
-     * Return `true` if this input is considered "locked", i.e. it has
-     * attributes like a Git revision or NAR hash that uniquely
-     * identify its contents.
-     */
     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/path.cc

@@ -72,6 +72,7 @@ struct PathInputScheme : InputScheme
         auto query = attrsToQuery(input.attrs);
         query.erase("path");
         query.erase("type");
+        query.erase("__final");
         return ParsedURL {
             .scheme = "path",
             .path = getStrAttr(input.attrs, "path"),

src/libflake/flake/flake.cc

@@ -85,7 +85,6 @@ static void forceTrivialValue(EvalState & state, Value & value, const PosIdx pos
         state.forceValue(value, pos);
 }
 
-
 static void expectType(EvalState & state, ValueType type,
     Value & value, const PosIdx pos)
 {
@@ -810,12 +809,14 @@ void callFlake(EvalState & state,
     auto vCallFlake = state.allocValue();
     state.evalFile(state.callFlakeInternal, *vCallFlake);
 
-    auto vTmp1 = state.allocValue();
     auto vLocks = state.allocValue();
     vLocks->mkString(lockFileStr);
-    state.callFunction(*vCallFlake, *vLocks, *vTmp1, noPos);
 
-    state.callFunction(*vTmp1, vOverrides, vRes, noPos);
+    auto vFetchFinalTree = get(state.internalPrimOps, "fetchFinalTree");
+    assert(vFetchFinalTree);
+
+    Value * args[] = {vLocks, &vOverrides, *vFetchFinalTree};
+    state.callFunction(*vCallFlake, 3, args, vRes, noPos);
 }
 
 void initLib(const Settings & settings)

src/libflake/flake/flake.hh

@@ -234,4 +234,11 @@ void emitTreeAttrs(
     bool emptyRevFallback = false,
     bool forceDirty = false);
 
+/**
+ * An internal builtin similar to `fetchTree`, except that it
+ * always treats the input as final (i.e. no attributes can be
+ * added/removed/changed).
+ */
+void prim_fetchFinalTree(EvalState & state, const PosIdx pos, Value * * args, Value & v);
+
 }

src/libflake/flake/lockfile.cc

@@ -46,6 +46,10 @@ LockedNode::LockedNode(
     if (!lockedRef.input.isLocked())
         throw Error("lock file contains unlocked input '%s'",
             fetchers::attrsToJSON(lockedRef.input.toAttrs()));
+
+    // For backward compatibility, lock file entries are implicitly final.
+    assert(!lockedRef.input.attrs.contains("__final"));
+    lockedRef.input.attrs.insert_or_assign("__final", Explicit<bool>(true));
 }
 
 StorePath LockedNode::computeStorePath(Store & store) const
@@ -53,7 +57,6 @@ StorePath LockedNode::computeStorePath(Store & store) const
     return lockedRef.input.computeStorePath(store);
 }
 
-
 static std::shared_ptr<Node> doFind(const ref<Node> & root, const InputPath & path, std::vector<InputPath> & visited)
 {
     auto pos = root;
@@ -191,6 +194,11 @@ std::pair<nlohmann::json, LockFile::KeyMap> LockFile::toJSON() const
         if (auto lockedNode = node.dynamic_pointer_cast<const LockedNode>()) {
             n["original"] = fetchers::attrsToJSON(lockedNode->originalRef.toAttrs());
             n["locked"] = fetchers::attrsToJSON(lockedNode->lockedRef.toAttrs());
+            /* For backward compatibility, omit the "__final"
+               attribute. We never allow non-final inputs in lock files
+               anyway. */
+            assert(lockedNode->lockedRef.input.isFinal());
+            n["locked"].erase("__final");
             if (!lockedNode->isFlake)
                 n["flake"] = false;
         }
@@ -239,7 +247,7 @@ std::optional<FlakeRef> LockFile::isUnlocked() const
     for (auto & i : nodes) {
         if (i == ref<const Node>(root)) continue;
         auto node = i.dynamic_pointer_cast<const LockedNode>();
-        if (node && !node->lockedRef.input.isLocked())
+        if (node && (!node->lockedRef.input.isLocked() || !node->lockedRef.input.isFinal()))
             return node->lockedRef;
     }
 

src/libflake/flake/lockfile.hh

@@ -68,8 +68,8 @@ struct LockFile
     std::pair<std::string, KeyMap> to_string() const;
 
     /**
-     * Check whether this lock file has any unlocked inputs. If so,
+     * Check whether this lock file has any unlocked or non-final
-     * return one.
+     * inputs. If so, return one.
      */
     std::optional<FlakeRef> isUnlocked() const;
 

src/nix/flake.md

@@ -148,7 +148,7 @@ reference types:
 
 * `ref`: A Git or Mercurial branch or tag name.
 
-Finally, some attribute are typically not specified by the user, but
+Finally, some attributes are typically not specified by the user, but
 can occur in *locked* flake references and are available to Nix code:
 
 * `revCount`: The number of ancestors of the commit `rev`.
@@ -666,6 +666,11 @@ following fields:
   other attributes are necessary because they provide information not
   stored in the store path.
 
+  The attributes in `locked` are considered "final", meaning that they are the only ones that are passed via the arguments of the `outputs` function of a flake.
+  For instance, if `locked` contains a `lastModified` attribute while the fetcher does not return a `lastModified` attribute, then the `lastModified` attribute will be passed to the `outputs` function.
+  Conversely, if `locked` does *not* contain a `lastModified` attribute while the fetcher *does* return a `lastModified` attribute, then no `lastModified` attribute will be passed.
+  If `locked` contains a `lastModifed` attribute and the fetcher returns a `lastModified` attribute, then they must have the same value.
+
 * `flake`: A Boolean denoting whether this is a flake or non-flake
   dependency. Corresponds to the `flake` attribute in the `inputs`
   attribute in `flake.nix`.