Handle unlocked overriden inputs

This fixes the error

  in pure evaluation mode, 'fetchTree' requires a locked input

when using '--override-input X Y' where Y is an unlocked input (e.g. a
dirty Git tree).

Also, make LockFile use ref instead of std::shared_ptr.

src/libexpr/flake/call-flake.nix

@@ -1,4 +1,14 @@
-lockFileStr: rootSrc: rootSubdir:
+# This is a helper to callFlake() to lazily fetch flake inputs.
+
+# The contents of the lock file, in JSON format.
+lockFileStr:
+
+# A mapping of lock file node IDs to { sourceInfo, subdir } attrsets,
+# with sourceInfo.outPath providing an InputAccessor to a previously
+# fetched tree. This is necessary for possibly unlocked inputs, in
+# particular the root input, but also --override-inputs pointing to
+# unlocked trees.
+overrides:
 
 let
 
@@ -29,8 +39,8 @@ let
         let
 
           sourceInfo =
-            if key == lockFile.root
+            if overrides ? ${key}
-            then rootSrc
+            then overrides.${key}.sourceInfo
             else if node.locked.type == "path" && builtins.substring 0 1 node.locked.path != "/"
             then
               let
@@ -42,7 +52,8 @@ let
               # FIXME: remove obsolete node.info.
               fetchTree (node.info or {} // removeAttrs node.locked ["dir"]);
 
-          subdir = if key == lockFile.root then rootSubdir else node.locked.dir or "";
+          # With overrides, the accessor already points to the right subdirectory.
+          subdir = if overrides ? ${key} then "" else node.locked.dir or "";
 
           flake =
             import (sourceInfo.outPath + ((if subdir != "" then "/" else "") + subdir + "/flake.nix"));

src/libexpr/flake/flake.cc

@@ -310,6 +310,7 @@ LockedFlake lockFlake(
 
         std::map<InputPath, std::tuple<FlakeInput, SourcePath, std::optional<InputPath>>> overrides;
         std::set<InputPath> overridesUsed, updatesUsed;
+        std::map<ref<Node>, SourcePath> nodePaths;
 
         for (auto & i : lockFlags.inputOverrides)
             overrides.emplace(
@@ -325,7 +326,7 @@ LockedFlake lockFlake(
 
         std::function<void(
             const FlakeInputs & flakeInputs,
-            std::shared_ptr<Node> node,
+            ref<Node> node,
             const InputPath & inputPathPrefix,
             std::shared_ptr<const Node> oldNode,
             const InputPath & followsPrefix,
@@ -338,7 +339,7 @@ LockedFlake lockFlake(
                flake.lock */
             const FlakeInputs & flakeInputs,
             /* The node whose locks are to be updated.*/
-            std::shared_ptr<Node> node,
+            ref<Node> node,
             /* The path to this node in the lock file graph. */
             const InputPath & inputPathPrefix,
             /* The old node, if any, from which locks can be
@@ -461,7 +462,7 @@ LockedFlake lockFlake(
                         /* Copy the input from the old lock since its flakeref
                            didn't change and there is no override from a
                            higher level flake. */
-                        auto childNode = std::make_shared<LockedNode>(
+                        auto childNode = make_ref<LockedNode>(
                             oldLock->lockedRef, oldLock->originalRef, oldLock->isFlake,
                             oldLock->parentPath);
 
@@ -517,6 +518,7 @@ LockedFlake lockFlake(
 
                         if (mustRefetch) {
                             auto inputFlake = getInputFlake();
+                            nodePaths.emplace(childNode, inputFlake.path.parent());
                             computeLocks(inputFlake.inputs, childNode, inputPath, oldLock, followsPrefix,
                                 inputFlake.path, !mustRefetch);
                         } else {
@@ -547,7 +549,7 @@ LockedFlake lockFlake(
                         if (input.isFlake) {
                             auto inputFlake = getInputFlake();
 
-                            auto childNode = std::make_shared<LockedNode>(
+                            auto childNode = make_ref<LockedNode>(
                                 inputFlake.lockedRef, ref, true,
                                 overridenParentPath);
 
@@ -564,11 +566,12 @@ LockedFlake lockFlake(
                                flake. Also, unless we already have this flake
                                in the top-level lock file, use this flake's
                                own lock file. */
+                            nodePaths.emplace(childNode, inputFlake.path.parent());
                             computeLocks(
                                 inputFlake.inputs, childNode, inputPath,
                                 oldLock
                                 ? std::dynamic_pointer_cast<const Node>(oldLock)
-                                : readLockFile(inputFlake).root,
+                                : (std::shared_ptr<Node>) readLockFile(inputFlake).root,
                                 oldLock ? followsPrefix : inputPath,
                                 inputFlake.path,
                                 false);
@@ -579,8 +582,11 @@ LockedFlake lockFlake(
 
                             auto [accessor, lockedRef] = resolvedRef.lazyFetch(state.store);
 
-                            node->inputs.insert_or_assign(id,
+                            auto childNode = make_ref<LockedNode>(lockedRef, ref, false, overridenParentPath);
-                                std::make_shared<LockedNode>(lockedRef, ref, false, overridenParentPath));
+
+                            nodePaths.emplace(childNode, accessor->root());
+
+                            node->inputs.insert_or_assign(id, childNode);
                         }
                     }
 
@@ -591,11 +597,13 @@ LockedFlake lockFlake(
             }
         };
 
+        nodePaths.emplace(newLockFile.root, flake->path.parent());
+
         computeLocks(
             flake->inputs,
             newLockFile.root,
             {},
-            lockFlags.recreateLockFile ? nullptr : oldLockFile.root,
+            lockFlags.recreateLockFile ? nullptr : (std::shared_ptr<Node>) oldLockFile.root,
             {},
             flake->path,
             false);
@@ -673,7 +681,11 @@ LockedFlake lockFlake(
             }
         }
 
-        return LockedFlake { .flake = std::move(*flake), .lockFile = std::move(newLockFile) };
+        return LockedFlake {
+            .flake = std::move(*flake),
+            .lockFile = std::move(newLockFile),
+            .nodePaths = std::move(nodePaths)
+        };
 
     } catch (Error & e) {
         if (flake)
@@ -686,30 +698,42 @@ void callFlake(EvalState & state,
     const LockedFlake & lockedFlake,
     Value & vRes)
 {
-    auto vLocks = state.allocValue();
+    auto [lockFileStr, keyMap] = lockedFlake.lockFile.to_string();
-    auto vRootSrc = state.allocValue();
-    auto vRootSubdir = state.allocValue();
-    auto vTmp1 = state.allocValue();
-    auto vTmp2 = state.allocValue();
 
-    vLocks->mkString(lockedFlake.lockFile.to_string());
+    auto overrides = state.buildBindings(lockedFlake.nodePaths.size());
+
+    for (auto & [node, sourcePath] : lockedFlake.nodePaths) {
+        auto override = state.buildBindings(2);
+
+        auto & vSourceInfo = override.alloc(state.symbols.create("sourceInfo"));
+
+        auto lockedNode = node.dynamic_pointer_cast<const LockedNode>();
 
         emitTreeAttrs(
             state,
-        {lockedFlake.flake.path.accessor, CanonPath::root},
+            sourcePath,
-        lockedFlake.flake.lockedRef.input,
+            lockedNode ? lockedNode->lockedRef.input : lockedFlake.flake.lockedRef.input,
-        *vRootSrc,
+            vSourceInfo,
             false,
-        lockedFlake.flake.forceDirty);
+            !lockedNode && lockedFlake.flake.forceDirty);
 
-    vRootSubdir->mkString(lockedFlake.flake.lockedRef.subdir);
+        auto key = keyMap.find(node);
+        assert(key != keyMap.end());
 
-    Value vCallFlake;
+        overrides.alloc(state.symbols.create(key->second)).mkAttrs(override);
-    state.evalFile(state.callFlakeInternal, vCallFlake);
+    }
 
-    state.callFunction(vCallFlake, *vLocks, *vTmp1, noPos);
+    auto & vOverrides = state.allocValue()->mkAttrs(overrides);
-    state.callFunction(*vTmp1, *vRootSrc, *vTmp2, noPos);
+
-    state.callFunction(*vTmp2, *vRootSubdir, vRes, noPos);
+    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);
 }
 
 static void prim_getFlake(EvalState & state, const PosIdx pos, Value * * args, Value & v)

src/libexpr/flake/flake.hh

@@ -79,6 +79,11 @@ struct LockedFlake
     Flake flake;
     LockFile lockFile;
 
+    /* Source tree accessors for nodes that have been fetched in
+       lockFlake(); in particular, the root node and the overriden
+       inputs. */
+    std::map<ref<Node>, SourcePath> nodePaths;
+
     std::optional<Fingerprint> getFingerprint(ref<Store> store) const;
 };
 

src/libexpr/flake/lockfile.cc

@@ -43,14 +43,14 @@ LockedNode::LockedNode(const nlohmann::json & json)
 
 std::shared_ptr<Node> LockFile::findInput(const InputPath & path)
 {
-    auto pos = root;
+    std::shared_ptr<Node> pos = root;
 
     if (!pos) return {};
 
     for (auto & elem : path) {
         if (auto i = get(pos->inputs, elem)) {
             if (auto node = std::get_if<0>(&*i))
-                pos = *node;
+                pos = (std::shared_ptr<LockedNode>) *node;
             else if (auto follows = std::get_if<1>(&*i)) {
                 pos = findInput(*follows);
                 if (!pos) return {};
@@ -70,7 +70,7 @@ LockFile::LockFile(std::string_view contents, std::string_view path)
     if (version < 5 || version > 7)
         throw Error("lock file '%s' has unsupported version %d", path, version);
 
-    std::unordered_map<std::string, std::shared_ptr<Node>> nodeMap;
+    std::map<std::string, ref<Node>> nodeMap;
 
     std::function<void(Node & node, const nlohmann::json & jsonNode)> getInputs;
 
@@ -91,12 +91,12 @@ LockFile::LockFile(std::string_view contents, std::string_view path)
                     auto jsonNode2 = nodes.find(inputKey);
                     if (jsonNode2 == nodes.end())
                         throw Error("lock file references missing node '%s'", inputKey);
-                    auto input = std::make_shared<LockedNode>(*jsonNode2);
+                    auto input = make_ref<LockedNode>(*jsonNode2);
                     k = nodeMap.insert_or_assign(inputKey, input).first;
                     getInputs(*input, *jsonNode2);
                 }
-                if (auto child = std::dynamic_pointer_cast<LockedNode>(k->second))
+                if (auto child = k->second.dynamic_pointer_cast<LockedNode>())
-                    node.inputs.insert_or_assign(i.key(), child);
+                    node.inputs.insert_or_assign(i.key(), ref(child));
                 else
                     // FIXME: replace by follows node
                     throw Error("lock file contains cycle to root node");
@@ -114,15 +114,15 @@ LockFile::LockFile(std::string_view contents, std::string_view path)
     // a bit since we don't need to worry about cycles.
 }
 
-nlohmann::json LockFile::toJSON() const
+std::pair<nlohmann::json, LockFile::KeyMap> LockFile::toJSON() const
 {
     nlohmann::json nodes;
-    std::unordered_map<std::shared_ptr<const Node>, std::string> nodeKeys;
+    KeyMap nodeKeys;
     std::unordered_set<std::string> keys;
 
-    std::function<std::string(const std::string & key, std::shared_ptr<const Node> node)> dumpNode;
+    std::function<std::string(const std::string & key, ref<const Node> node)> dumpNode;
 
-    dumpNode = [&](std::string key, std::shared_ptr<const Node> node) -> std::string
+    dumpNode = [&](std::string key, ref<const Node> node) -> std::string
     {
         auto k = nodeKeys.find(node);
         if (k != nodeKeys.end())
@@ -157,7 +157,7 @@ nlohmann::json LockFile::toJSON() const
             n["inputs"] = std::move(inputs);
         }
 
-        if (auto lockedNode = std::dynamic_pointer_cast<const LockedNode>(node)) {
+        if (auto lockedNode = node.dynamic_pointer_cast<const LockedNode>()) {
             n["original"] = fetchers::attrsToJSON(lockedNode->originalRef.toAttrs());
             n["locked"] = fetchers::attrsToJSON(lockedNode->lockedRef.toAttrs());
             if (!lockedNode->isFlake)
@@ -176,27 +176,28 @@ nlohmann::json LockFile::toJSON() const
     json["root"] = dumpNode("root", root);
     json["nodes"] = std::move(nodes);
 
-    return json;
+    return {json, std::move(nodeKeys)};
 }
 
-std::string LockFile::to_string() const
+std::pair<std::string, LockFile::KeyMap> LockFile::to_string() const
 {
-    return toJSON().dump(2);
+    auto [json, nodeKeys] = toJSON();
+    return {json.dump(2), std::move(nodeKeys)};
 }
 
 std::ostream & operator <<(std::ostream & stream, const LockFile & lockFile)
 {
-    stream << lockFile.toJSON().dump(2);
+    stream << lockFile.toJSON().first.dump(2);
     return stream;
 }
 
 std::optional<FlakeRef> LockFile::isUnlocked() const
 {
-    std::unordered_set<std::shared_ptr<const Node>> nodes;
+    std::set<ref<const Node>> nodes;
 
-    std::function<void(std::shared_ptr<const Node> node)> visit;
+    std::function<void(ref<const Node> node)> visit;
 
-    visit = [&](std::shared_ptr<const Node> node)
+    visit = [&](ref<const Node> node)
     {
         if (!nodes.insert(node).second) return;
         for (auto & i : node->inputs)
@@ -208,7 +209,7 @@ std::optional<FlakeRef> LockFile::isUnlocked() const
 
     for (auto & i : nodes) {
         if (i == root) continue;
-        auto node = std::dynamic_pointer_cast<const LockedNode>(i);
+        auto node = i.dynamic_pointer_cast<const LockedNode>();
         if (node
             && !node->lockedRef.input.isLocked()
             && !node->lockedRef.input.isRelative())
@@ -221,7 +222,7 @@ std::optional<FlakeRef> LockFile::isUnlocked() const
 bool LockFile::operator ==(const LockFile & other) const
 {
     // FIXME: slow
-    return toJSON() == other.toJSON();
+    return toJSON().first == other.toJSON().first;
 }
 
 InputPath parseInputPath(std::string_view s)
@@ -239,12 +240,12 @@ InputPath parseInputPath(std::string_view s)
 
 std::map<InputPath, Node::Edge> LockFile::getAllInputs() const
 {
-    std::unordered_set<std::shared_ptr<Node>> done;
+    std::set<ref<Node>> done;
     std::map<InputPath, Node::Edge> res;
 
-    std::function<void(const InputPath & prefix, std::shared_ptr<Node> node)> recurse;
+    std::function<void(const InputPath & prefix, ref<Node> node)> recurse;
 
-    recurse = [&](const InputPath & prefix, std::shared_ptr<Node> node)
+    recurse = [&](const InputPath & prefix, ref<Node> node)
     {
         if (!done.insert(node).second) return;
 

src/libexpr/flake/lockfile.hh

@@ -20,7 +20,7 @@ struct LockedNode;
    type LockedNode. */
 struct Node : std::enable_shared_from_this<Node>
 {
-    typedef std::variant<std::shared_ptr<LockedNode>, InputPath> Edge;
+    typedef std::variant<ref<LockedNode>, InputPath> Edge;
 
     std::map<FlakeId, Edge> inputs;
 
@@ -50,14 +50,16 @@ struct LockedNode : Node
 
 struct LockFile
 {
-    std::shared_ptr<Node> root = std::make_shared<Node>();
+    ref<Node> root = make_ref<Node>();
 
     LockFile() {};
     LockFile(std::string_view contents, std::string_view path);
 
-    nlohmann::json toJSON() const;
+    typedef std::map<ref<const Node>, std::string> KeyMap;
 
-    std::string to_string() const;
+    std::pair<nlohmann::json, KeyMap> toJSON() const;
+
+    std::pair<std::string, KeyMap> to_string() const;
 
     /* Check whether this lock file has any unlocked inputs. If so,
        return one. */

src/nix/flake.cc

@@ -182,7 +182,7 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON
                 j["revCount"] = *revCount;
             if (auto lastModified = flake.lockedRef.input.getLastModified())
                 j["lastModified"] = *lastModified;
-            j["locks"] = lockedFlake.lockFile.toJSON();
+            j["locks"] = lockedFlake.lockFile.toJSON().first;
             logger->cout("%s", j.dump());
         } else {
             logger->cout(
@@ -211,7 +211,7 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON
             if (!lockedFlake.lockFile.root->inputs.empty())
                 logger->cout(ANSI_BOLD "Inputs:" ANSI_NORMAL);
 
-            std::unordered_set<std::shared_ptr<Node>> visited;
+            std::set<ref<Node>> visited;
 
             std::function<void(const Node & node, const std::string & prefix)> recurse;
 
@@ -223,7 +223,7 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON
                     if (auto lockedNode = std::get_if<0>(&input.second)) {
                         logger->cout("%s" ANSI_BOLD "%s" ANSI_NORMAL ": %s",
                             prefix + (last ? treeLast : treeConn), input.first,
-                            *lockedNode ? (*lockedNode)->lockedRef : flake.lockedRef);
+                            (*lockedNode)->lockedRef);
 
                         bool firstVisit = visited.insert(*lockedNode).second;
 

tests/flakes/unlocked-override.sh

@@ -0,0 +1,30 @@
+source ./common.sh
+
+requireGit
+
+flake1Dir=$TEST_ROOT/flake1
+flake2Dir=$TEST_ROOT/flake2
+
+createGitRepo $flake1Dir
+cat > $flake1Dir/flake.nix <<EOF
+{
+    outputs = { self }: { x = import ./x.nix; };
+}
+EOF
+echo 123 > $flake1Dir/x.nix
+git -C $flake1Dir add flake.nix x.nix
+git -C $flake1Dir commit -m Initial
+
+createGitRepo $flake2Dir
+cat > $flake2Dir/flake.nix <<EOF
+{
+    outputs = { self, flake1 }: { x = flake1.x; };
+}
+EOF
+git -C $flake2Dir add flake.nix
+
+[[ $(nix eval --json $flake2Dir#x --override-input flake1 $TEST_ROOT/flake1) = 123 ]]
+
+echo 456 > $flake1Dir/x.nix
+
+[[ $(nix eval --json $flake2Dir#x --override-input flake1 $TEST_ROOT/flake1) = 456 ]]

tests/local.mk

@@ -7,6 +7,7 @@ nix_tests = \
   flakes/follow-paths.sh \
   flakes/bundle.sh \
   flakes/check.sh \
+  flakes/unlocked-override.sh \
   ca/gc.sh \
   gc.sh \
   remote-store.sh \