Merge pull request NixOS#11701 from DeterminateSystems/flake-substitution

Restore input substitution

Author: edolstra

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;
-    baseEnv.values[0]->payload.attrs->push_back(Attr(symbols.create(primOp.name), v));
+
+    if (primOp.internal)
+        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,46 +205,73 @@ 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 (final.getNarHash())
+        if (result.getNarHash() != prevNarHash) {
+            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));
         }
     }
 
     if (auto prevLastModified = specified.getLastModified()) {
-        if (final.getLastModified() != prevLastModified)
-            throw Error("'lastModified' attribute mismatch in input '%s', expected %d",
-                final.to_string(), *prevLastModified);
+        if (result.getLastModified() != prevLastModified)
+            throw Error("'lastModified' attribute mismatch in input '%s', expected %d, got %d",
+                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(