From cbade16f9ef1e06b40b379863556157b6222a13b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 29 Sep 2022 16:12:04 +0200 Subject: [PATCH] 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 | 19 +++++-- src/libexpr/flake/flake.cc | 86 ++++++++++++++++++++----------- src/libexpr/flake/flake.hh | 5 ++ src/libexpr/flake/lockfile.cc | 47 ++++++++--------- src/libexpr/flake/lockfile.hh | 10 ++-- src/nix/flake.cc | 6 +-- tests/flakes/unlocked-override.sh | 30 +++++++++++ tests/local.mk | 1 + 8 files changed, 139 insertions(+), 65 deletions(-) create mode 100644 tests/flakes/unlocked-override.sh diff --git a/src/libexpr/flake/call-flake.nix b/src/libexpr/flake/call-flake.nix index 2b0a47d3e..881624b37 100644 --- a/src/libexpr/flake/call-flake.nix +++ b/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 - then rootSrc + if overrides ? ${key} + 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")); diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 82adda4a7..e4210c0cf 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -310,6 +310,7 @@ LockedFlake lockFlake( std::map>> overrides; std::set overridesUsed, updatesUsed; + std::map, SourcePath> nodePaths; for (auto & i : lockFlags.inputOverrides) overrides.emplace( @@ -325,7 +326,7 @@ LockedFlake lockFlake( std::function node, + ref node, const InputPath & inputPathPrefix, std::shared_ptr 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, + ref 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( + auto childNode = make_ref( 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( + auto childNode = make_ref( 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(oldLock) - : readLockFile(inputFlake).root, + : (std::shared_ptr) 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, - std::make_shared(lockedRef, ref, false, overridenParentPath)); + auto childNode = make_ref(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) 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 vRootSrc = state.allocValue(); - auto vRootSubdir = state.allocValue(); + auto [lockFileStr, keyMap] = 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(); + + emitTreeAttrs( + state, + sourcePath, + lockedNode ? lockedNode->lockedRef.input : lockedFlake.flake.lockedRef.input, + vSourceInfo, + false, + !lockedNode && lockedFlake.flake.forceDirty); + + auto key = keyMap.find(node); + assert(key != keyMap.end()); + + overrides.alloc(state.symbols.create(key->second)).mkAttrs(override); + } + + auto & vOverrides = state.allocValue()->mkAttrs(overrides); + + auto vCallFlake = state.allocValue(); + state.evalFile(state.callFlakeInternal, *vCallFlake); + auto vTmp1 = state.allocValue(); - auto vTmp2 = state.allocValue(); + auto vLocks = state.allocValue(); + vLocks->mkString(lockFileStr); + state.callFunction(*vCallFlake, *vLocks, *vTmp1, noPos); - vLocks->mkString(lockedFlake.lockFile.to_string()); - - emitTreeAttrs( - state, - {lockedFlake.flake.path.accessor, CanonPath::root}, - lockedFlake.flake.lockedRef.input, - *vRootSrc, - false, - lockedFlake.flake.forceDirty); - - vRootSubdir->mkString(lockedFlake.flake.lockedRef.subdir); - - Value vCallFlake; - state.evalFile(state.callFlakeInternal, vCallFlake); - - state.callFunction(vCallFlake, *vLocks, *vTmp1, noPos); - state.callFunction(*vTmp1, *vRootSrc, *vTmp2, noPos); - state.callFunction(*vTmp2, *vRootSubdir, vRes, noPos); + state.callFunction(*vTmp1, vOverrides, vRes, noPos); } static void prim_getFlake(EvalState & state, const PosIdx pos, Value * * args, Value & v) diff --git a/src/libexpr/flake/flake.hh b/src/libexpr/flake/flake.hh index 6b44f9a9c..51e2daeb8 100644 --- a/src/libexpr/flake/flake.hh +++ b/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, SourcePath> nodePaths; + std::optional getFingerprint(ref store) const; }; diff --git a/src/libexpr/flake/lockfile.cc b/src/libexpr/flake/lockfile.cc index 166abe243..bbc803b80 100644 --- a/src/libexpr/flake/lockfile.cc +++ b/src/libexpr/flake/lockfile.cc @@ -43,14 +43,14 @@ LockedNode::LockedNode(const nlohmann::json & json) std::shared_ptr LockFile::findInput(const InputPath & path) { - auto pos = root; + std::shared_ptr 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) *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> nodeMap; + std::map> nodeMap; std::function 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(*jsonNode2); + auto input = make_ref(*jsonNode2); k = nodeMap.insert_or_assign(inputKey, input).first; getInputs(*input, *jsonNode2); } - if (auto child = std::dynamic_pointer_cast(k->second)) - node.inputs.insert_or_assign(i.key(), child); + if (auto child = k->second.dynamic_pointer_cast()) + 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 LockFile::toJSON() const { nlohmann::json nodes; - std::unordered_map, std::string> nodeKeys; + KeyMap nodeKeys; std::unordered_set keys; - std::function node)> dumpNode; + std::function node)> dumpNode; - dumpNode = [&](std::string key, std::shared_ptr node) -> std::string + dumpNode = [&](std::string key, ref 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(node)) { + if (auto lockedNode = node.dynamic_pointer_cast()) { 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 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 LockFile::isUnlocked() const { - std::unordered_set> nodes; + std::set> nodes; - std::function node)> visit; + std::function node)> visit; - visit = [&](std::shared_ptr node) + visit = [&](ref node) { if (!nodes.insert(node).second) return; for (auto & i : node->inputs) @@ -208,7 +209,7 @@ std::optional LockFile::isUnlocked() const for (auto & i : nodes) { if (i == root) continue; - auto node = std::dynamic_pointer_cast(i); + auto node = i.dynamic_pointer_cast(); if (node && !node->lockedRef.input.isLocked() && !node->lockedRef.input.isRelative()) @@ -221,7 +222,7 @@ std::optional 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 LockFile::getAllInputs() const { - std::unordered_set> done; + std::set> done; std::map res; - std::function node)> recurse; + std::function node)> recurse; - recurse = [&](const InputPath & prefix, std::shared_ptr node) + recurse = [&](const InputPath & prefix, ref node) { if (!done.insert(node).second) return; diff --git a/src/libexpr/flake/lockfile.hh b/src/libexpr/flake/lockfile.hh index 9edd2ef01..f40d73d6c 100644 --- a/src/libexpr/flake/lockfile.hh +++ b/src/libexpr/flake/lockfile.hh @@ -20,7 +20,7 @@ struct LockedNode; type LockedNode. */ struct Node : std::enable_shared_from_this { - typedef std::variant, InputPath> Edge; + typedef std::variant, InputPath> Edge; std::map inputs; @@ -50,14 +50,16 @@ struct LockedNode : Node struct LockFile { - std::shared_ptr root = std::make_shared(); + ref root = make_ref(); LockFile() {}; LockFile(std::string_view contents, std::string_view path); - nlohmann::json toJSON() const; + typedef std::map, std::string> KeyMap; - std::string to_string() const; + std::pair toJSON() const; + + std::pair to_string() const; /* Check whether this lock file has any unlocked inputs. If so, return one. */ diff --git a/src/nix/flake.cc b/src/nix/flake.cc index bf2bbf6f0..ac03dcadf 100644 --- a/src/nix/flake.cc +++ b/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> visited; + std::set> visited; std::function 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; diff --git a/tests/flakes/unlocked-override.sh b/tests/flakes/unlocked-override.sh new file mode 100644 index 000000000..8abc8b7d3 --- /dev/null +++ b/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 < $flake1Dir/x.nix +git -C $flake1Dir add flake.nix x.nix +git -C $flake1Dir commit -m Initial + +createGitRepo $flake2Dir +cat > $flake2Dir/flake.nix < $flake1Dir/x.nix + +[[ $(nix eval --json $flake2Dir#x --override-input flake1 $TEST_ROOT/flake1) = 456 ]] diff --git a/tests/local.mk b/tests/local.mk index b2fcb620a..2ff80d734 100644 --- a/tests/local.mk +++ b/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 \