Merge pull request #12376 from DeterminateSystems/fetch-using-nar-hash

Allow fetching using NAR hash without --allow-dirty-locks

src/libexpr/primops/fetchTree.cc

@@ -183,11 +183,16 @@ static void fetchTree(
     if (!state.settings.pureEval && !input.isDirect() && experimentalFeatureSettings.isEnabled(Xp::Flakes))
         input = lookupInRegistries(state.store, input).first;
 
-    if (state.settings.pureEval && !input.isConsideredLocked(state.fetchSettings)) {
+    if (state.settings.pureEval && !input.isLocked()) {
+        if (input.getNarHash())
+            warn(
+                "Input '%s' is unlocked (e.g. lacks a Git revision) but does have a NAR hash. "
+                "This is deprecated since such inputs are verifiable but may not be reproducible.",
+                input.to_string());
+        else
             state.error<EvalError>(
                 "in pure evaluation mode, '%s' will not fetch unlocked input '%s'",
-            fetcher, input.to_string()
-        ).atPos(pos).debugThrow();
+                fetcher, input.to_string()).atPos(pos).debugThrow();
     }
 
     state.checkURI(input.toURLString());

src/libfetchers/fetchers.cc

@@ -155,12 +155,6 @@ bool Input::isLocked() const
     return scheme && scheme->isLocked(*this);
 }
 
-bool Input::isConsideredLocked(
-    const Settings & settings) const
-{
-    return isLocked() || (settings.allowDirtyLocks && getNarHash());
-}
-
 bool Input::isFinal() const
 {
     return maybeGetBoolAttr(attrs, "__final").value_or(false);

src/libfetchers/fetchers.hh

@@ -90,15 +90,6 @@ public:
      */
     bool isLocked() const;
 
-    /**
-     * Return whether the input is either locked, or, if
-     * `allow-dirty-locks` is enabled, it has a NAR hash. In the
-     * latter case, we can verify the input but we may not be able to
-     * fetch it from anywhere.
-     */
-    bool isConsideredLocked(
-        const Settings & settings) const;
-
     /**
      * Only for relative path flakes, i.e. 'path:./foo', returns the
      * relative path, i.e. './foo'.

src/libflake/flake/lockfile.cc

@@ -1,7 +1,10 @@
 #include <unordered_set>
 
+#include "fetch-settings.hh"
+#include "flake/settings.hh"
 #include "lockfile.hh"
 #include "store-api.hh"
+#include "strings.hh"
 
 #include <algorithm>
 #include <iomanip>
@@ -9,8 +12,6 @@
 #include <iterator>
 #include <nlohmann/json.hpp>
 
-#include "strings.hh"
-#include "flake/settings.hh"
 
 namespace nix::flake {
 
@@ -45,9 +46,16 @@ LockedNode::LockedNode(
     , isFlake(json.find("flake") != json.end() ? (bool) json["flake"] : true)
     , parentInputAttrPath(json.find("parent") != json.end() ? (std::optional<InputAttrPath>) json["parent"] : std::nullopt)
 {
-    if (!lockedRef.input.isConsideredLocked(fetchSettings) && !lockedRef.input.isRelative())
+    if (!lockedRef.input.isLocked() && !lockedRef.input.isRelative()) {
+        if (lockedRef.input.getNarHash())
+            warn(
+                "Lock file entry '%s' is unlocked (e.g. lacks a Git revision) but does have a NAR hash. "
+                "This is deprecated since such inputs are verifiable but may not be reproducible.",
+                lockedRef.to_string());
+        else
             throw Error("Lock file contains unlocked input '%s'. Use '--allow-dirty-locks' to accept this lock file.",
                 fetchers::attrsToJSON(lockedRef.input.toAttrs()));
+    }
 
     // For backward compatibility, lock file entries are implicitly final.
     assert(!lockedRef.input.attrs.contains("__final"));
@@ -248,11 +256,20 @@ std::optional<FlakeRef> LockFile::isUnlocked(const fetchers::Settings & fetchSet
 
     visit(root);
 
+    /* Return whether the input is either locked, or, if
+       `allow-dirty-locks` is enabled, it has a NAR hash. In the
+       latter case, we can verify the input but we may not be able to
+       fetch it from anywhere. */
+    auto isConsideredLocked = [&](const fetchers::Input & input)
+    {
+        return input.isLocked() || (fetchSettings.allowDirtyLocks && input.getNarHash());
+    };
+
     for (auto & i : nodes) {
         if (i == ref<const Node>(root)) continue;
         auto node = i.dynamic_pointer_cast<const LockedNode>();
         if (node
-            && (!node->lockedRef.input.isConsideredLocked(fetchSettings)
+            && (!isConsideredLocked(node->lockedRef.input)
                 || !node->lockedRef.input.isFinal())
             && !node->lockedRef.input.isRelative())
             return node->lockedRef;

tests/functional/fetchGit.sh

@@ -64,7 +64,7 @@ git -C $repo add differentbranch
 git -C $repo commit -m 'Test2'
 git -C $repo checkout master
 devrev=$(git -C $repo rev-parse devtest)
-nix eval --impure --raw --expr "builtins.fetchGit { url = file://$repo; rev = \"$devrev\"; }"
+nix eval --raw --expr "builtins.fetchGit { url = file://$repo; rev = \"$devrev\"; }"
 
 [[ $(nix eval --raw --expr "builtins.readFile (builtins.fetchGit { url = file://$repo; rev = \"$devrev\"; allRefs = true; } + \"/differentbranch\")") = 'different file' ]]
 
@@ -141,13 +141,17 @@ path4=$(nix eval --impure --refresh --raw --expr "(builtins.fetchGit file://$rep
 [[ $(nix eval --impure --expr "builtins.hasAttr \"dirtyRev\" (builtins.fetchGit $repo)") == "false" ]]
 [[ $(nix eval --impure --expr "builtins.hasAttr \"dirtyShortRev\" (builtins.fetchGit $repo)") == "false" ]]
 
-status=0
-nix eval --impure --raw --expr "(builtins.fetchGit { url = $repo; rev = \"$rev2\"; narHash = \"sha256-B5yIPHhEm0eysJKEsO7nqxprh9vcblFxpJG11gXJus1=\"; }).outPath" || status=$?
-[[ "$status" = "102" ]]
+expect 102 nix eval --raw --expr "(builtins.fetchGit { url = $repo; rev = \"$rev2\"; narHash = \"sha256-B5yIPHhEm0eysJKEsO7nqxprh9vcblFxpJG11gXJus1=\"; }).outPath"
 
-path5=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = $repo; rev = \"$rev2\"; narHash = \"sha256-Hr8g6AqANb3xqX28eu1XnjK/3ab8Gv6TJSnkb1LezG9=\"; }).outPath")
+path5=$(nix eval --raw --expr "(builtins.fetchGit { url = $repo; rev = \"$rev2\"; narHash = \"sha256-Hr8g6AqANb3xqX28eu1XnjK/3ab8Gv6TJSnkb1LezG9=\"; }).outPath")
 [[ $path = $path5 ]]
 
+# Ensure that NAR hashes are checked.
+expectStderr 102 nix eval --raw --expr "(builtins.fetchGit { url = $repo; rev = \"$rev2\"; narHash = \"sha256-Hr8g6AqANb4xqX28eu1XnjK/3ab8Gv6TJSnkb1LezG9=\"; }).outPath" | grepQuiet "error: NAR hash mismatch"
+
+# It's allowed to use only a narHash, but you should get a warning.
+expectStderr 0 nix eval --raw --expr "(builtins.fetchGit { url = $repo; ref = \"tag2\"; narHash = \"sha256-Hr8g6AqANb3xqX28eu1XnjK/3ab8Gv6TJSnkb1LezG9=\"; }).outPath" | grepQuiet "warning: Input .* is unlocked"
+
 # tarball-ttl should be ignored if we specify a rev
 echo delft > $repo/hello
 git -C $repo add hello
@@ -255,7 +259,7 @@ echo "/exported-wonky export-ignore=wonk" >> $repo/.gitattributes
 git -C $repo add not-exported-file exported-wonky .gitattributes
 git -C $repo commit -m 'Bla6'
 rev5=$(git -C $repo rev-parse HEAD)
-path12=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \"$rev5\"; }).outPath")
+path12=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \"$rev5\"; }).outPath")
 [[ ! -e $path12/not-exported-file ]]
 [[ -e $path12/exported-wonky ]]
 

tests/functional/flakes/unlocked-override.sh

@@ -37,8 +37,8 @@ expectStderr 1 nix flake lock "$flake2Dir" --override-input flake1 "$TEST_ROOT/f
 
 nix flake lock "$flake2Dir" --override-input flake1 "$TEST_ROOT/flake1" --allow-dirty-locks
 
-# Using a lock file with a dirty lock requires --allow-dirty-locks as well.
-expectStderr 1 nix eval "$flake2Dir#x" |
-  grepQuiet "Lock file contains unlocked input"
+# Using a lock file with a dirty lock does not require --allow-dirty-locks, but should print a warning.
+expectStderr 0 nix eval "$flake2Dir#x" |
+  grepQuiet "warning: Lock file entry .* is unlocked"
 
-[[ $(nix eval "$flake2Dir#x" --allow-dirty-locks) = 456 ]]
+[[ $(nix eval "$flake2Dir#x") = 456 ]]