nordic-dev.net/nix
nordic-dev.net/nix
2
0
Fork 0
mirror of https://github.com/NixOS/nix.git synced 2024-11-25 08:12:29 +00:00
Code Issues Packages Projects Releases Wiki Activity

Input: Replace 'locked' bool by isLocked() method

Browse Source 
It's better to just check whether the input has all the attributes
needed to consider itself locked (e.g. whether a Git input has an
'rev' attribute).

Also, the 'locked' field was actually incorrect for Git inputs: it
would be set to true even for dirty worktrees. As a result, we got
away with using fetchTree() internally even though fetchTree()
requires a locked input in pure mode. In particular, this allowed
'--override-input' to work by accident.

The fix is to pass a set of "overrides" to call-flake.nix for all the
unlocked inputs (i.e. the top-level flake and any --override-inputs).
This commit is contained in:
Eelco Dolstra 2024-02-16 17:00:07 +01:00
parent 78e7c98b02
commit 071dd2b3a4
16 changed files with 155 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

61
src/libexpr/flake/call-flake.nix
View File

@ -1,20 +1,52 @@
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
lockFile = builtins.fromJSON lockFileStr;
# Resolve a input spec into a node name. An input spec is
# either a node name, or a 'follows' path from the root
# node.
resolveInput = inputSpec:
if builtins.isList inputSpec
then getInputByPath lockFile.root inputSpec
else inputSpec;
# Follow an input path (e.g. ["dwarffs" "nixpkgs"]) from the
# root node, returning the final node.
getInputByPath = nodeName: path:
if path == []
then nodeName
else
getInputByPath
# Since this could be a 'follows' input, call resolveInput.
(resolveInput lockFile.nodes.${nodeName}.inputs.${builtins.head path})
(builtins.tail path);
allNodes =
builtins.mapAttrs
(key: node:
let
sourceInfo =
if key == lockFile.root
then rootSrc
else fetchTree (node.info or {} // removeAttrs node.locked ["dir"]);
if overrides ? ${key}
then
overrides.${key}.sourceInfo
else
# 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 "";
subdir = overrides.${key}.dir or node.locked.dir or "";
outPath = sourceInfo + ((if subdir == "" then "" else "/") + subdir);
@ -24,25 +56,6 @@ let
(inputName: inputSpec: allNodes.${resolveInput inputSpec})
(node.inputs or {});
# Resolve a input spec into a node name. An input spec is
# either a node name, or a 'follows' path from the root
# node.
resolveInput = inputSpec:
if builtins.isList inputSpec
then getInputByPath lockFile.root inputSpec
else inputSpec;
# Follow an input path (e.g. ["dwarffs" "nixpkgs"]) from the
# root node, returning the final node.
getInputByPath = nodeName: path:
if path == []
then nodeName
else
getInputByPath
# Since this could be a 'follows' input, call resolveInput.
(resolveInput lockFile.nodes.${nodeName}.inputs.${builtins.head path})
(builtins.tail path);
outputs = flake.outputs (inputs // { self = result; });
result =

84
src/libexpr/flake/flake.cc
View File

@ -365,6 +365,7 @@ LockedFlake lockFlake(
std::map<InputPath, FlakeInput> overrides;
std::set<InputPath> explicitCliOverrides;
std::set<InputPath> overridesUsed, updatesUsed;
std::map<ref<Node>, StorePath> nodePaths;
for (auto & i : lockFlags.inputOverrides) {
overrides.insert_or_assign(i.first, FlakeInput { .ref = i.second });
@ -535,11 +536,13 @@ LockedFlake lockFlake(
}
}
computeLocks(
mustRefetch
? getFlake(state, oldLock->lockedRef, false, flakeCache, inputPath).inputs
: fakeInputs,
childNode, inputPath, oldLock, lockRootPath, parentPath, !mustRefetch);
if (mustRefetch) {
auto inputFlake = getFlake(state, oldLock->lockedRef, false, flakeCache, inputPath);
nodePaths.emplace(childNode, inputFlake.storePath);
computeLocks(inputFlake.inputs, childNode, inputPath, oldLock, lockRootPath, parentPath, false);
} else {
computeLocks(fakeInputs, childNode, inputPath, oldLock, lockRootPath, parentPath, true);
}
} else {
/* We need to create a new lock file entry. So fetch
@ -584,6 +587,7 @@ 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.storePath);
computeLocks(
inputFlake.inputs, childNode, inputPath,
oldLock
@ -596,11 +600,13 @@ LockedFlake lockFlake(
}
else {
auto [sourceInfo, resolvedRef, lockedRef] = fetchOrSubstituteTree(
auto [storePath, resolvedRef, lockedRef] = fetchOrSubstituteTree(
state, *input.ref, useRegistries, flakeCache);
auto childNode = make_ref<LockedNode>(lockedRef, ref, false);
nodePaths.emplace(childNode, storePath);
node->inputs.insert_or_assign(id, childNode);
}
}
@ -615,6 +621,8 @@ LockedFlake lockFlake(
// Bring in the current ref for relative path resolution if we have it
auto parentPath = canonPath(state.store->toRealPath(flake.storePath) + "/" + flake.lockedRef.subdir, true);
nodePaths.emplace(newLockFile.root, flake.storePath);
computeLocks(
flake.inputs,
newLockFile.root,
@ -707,14 +715,6 @@ LockedFlake lockFlake(
flake.lockedRef.input.getRev() &&
prevLockedRef.input.getRev() != flake.lockedRef.input.getRev())
warn("committed new revision '%s'", flake.lockedRef.input.getRev()->gitRev());
/* Make sure that we picked up the change,
i.e. the tree should usually be dirty
now. Corner case: we could have reverted from a
dirty to a clean tree! */
if (flake.lockedRef.input == prevLockedRef.input
&& !flake.lockedRef.input.isLocked())
throw Error("'%s' did not change after I updated its 'flake.lock' file; is 'flake.lock' under version control?", flake.originalRef);
}
} else
throw Error("cannot write modified lock file of flake '%s' (use '--no-write-lock-file' to ignore)", topRef);
@ -724,7 +724,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) {
e.addTrace({}, "while updating the lock file of flake '%s'", flake.lockedRef.to_string());
@ -736,30 +740,48 @@ void callFlake(EvalState & state,
const LockedFlake & lockedFlake,
Value & vRes)
{
auto vLocks = state.allocValue();
auto vRootSrc = state.allocValue();
auto vRootSubdir = state.allocValue();
auto vTmp1 = state.allocValue();
auto vTmp2 = state.allocValue();
experimentalFeatureSettings.require(Xp::Flakes);
vLocks->mkString(lockedFlake.lockFile.to_string());
auto [lockFileStr, keyMap] = lockedFlake.lockFile.to_string();
emitTreeAttrs(
state,
lockedFlake.flake.storePath,
lockedFlake.flake.lockedRef.input,
*vRootSrc,
false,
lockedFlake.flake.forceDirty);
auto overrides = state.buildBindings(lockedFlake.nodePaths.size());
vRootSubdir->mkString(lockedFlake.flake.lockedRef.subdir);
for (auto & [node, storePath] : 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,
storePath,
lockedNode ? lockedNode->lockedRef.input : lockedFlake.flake.lockedRef.input,
vSourceInfo,
false,
!lockedNode && lockedFlake.flake.forceDirty);
auto key = keyMap.find(node);
assert(key != keyMap.end());
override
.alloc(state.symbols.create("dir"))
.mkString(lockedNode ? lockedNode->lockedRef.subdir : lockedFlake.flake.lockedRef.subdir);
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 vLocks = state.allocValue();
vLocks->mkString(lockFileStr);
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)

7
src/libexpr/flake/flake.hh
View File

@ -103,6 +103,13 @@ struct LockedFlake
Flake flake;
LockFile lockFile;
/**
* Store paths of nodes that have been fetched in
* lockFlake(); in particular, the root node and the overriden
* inputs.
*/
std::map<ref<Node>, StorePath> nodePaths;
Fingerprint getFingerprint() const;
};

17
src/libexpr/flake/lockfile.cc
View File

@ -38,7 +38,7 @@ LockedNode::LockedNode(const nlohmann::json & json)
, isFlake(json.find("flake") != json.end() ? (bool) json["flake"] : true)
{
if (!lockedRef.input.isLocked())
throw Error("lock file contains mutable lock '%s'",
throw Error("lock file contains unlocked input '%s'",
fetchers::attrsToJSON(lockedRef.input.toAttrs()));
}
@ -134,10 +134,10 @@ LockFile::LockFile(const nlohmann::json & json, const Path & 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, ref<const Node> node)> dumpNode;
@ -194,12 +194,13 @@ 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)};
}
LockFile LockFile::read(const Path & path)
@ -210,7 +211,7 @@ LockFile LockFile::read(const Path & path)
std::ostream & operator <<(std::ostream & stream, const LockFile & lockFile)
{
stream << lockFile.toJSON().dump(2);
stream << lockFile.toJSON().first.dump(2);
return stream;
}
@ -243,7 +244,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;
}
bool LockFile::operator !=(const LockFile & other) const

7
src/libexpr/flake/lockfile.hh
View File

@ -59,14 +59,15 @@ struct LockFile
typedef std::map<ref<const Node>, std::string> KeyMap;
nlohmann::json toJSON() const;
std::pair<nlohmann::json, KeyMap> toJSON() const;
std::string to_string() const;
std::pair<std::string, KeyMap> to_string() const;
static LockFile read(const Path & path);
/**
* Check whether this lock file has any unlocked inputs.
* Check whether this lock file has any unlocked inputs. If so,
* return one.
*/
std::optional<FlakeRef> isUnlocked() const;

6
src/libexpr/primops/fetchTree.cc
View File

@ -24,8 +24,6 @@ void emitTreeAttrs(
bool emptyRevFallback,
bool forceDirty)
{
assert(input.isLocked());
auto attrs = state.buildBindings(100);
state.mkStorePathString(storePath, attrs.alloc(state.sOutPath));
@ -176,8 +174,8 @@ static void fetchTree(
fetcher = "fetchGit";
state.error<EvalError>(
"in pure evaluation mode, %s requires a locked input",
fetcher
"in pure evaluation mode, '%s' will not fetch unlocked input '%s'",
fetcher, input.to_string()
).atPos(pos).debugThrow();
}

11
src/libfetchers/fetchers.cc
View File

@ -45,12 +45,8 @@ static void fixupInput(Input & input)
// Check common attributes.
input.getType();
input.getRef();
if (input.getRev())
input.locked = true;
input.getRevCount();
input.getLastModified();
if (input.getNarHash())
input.locked = true;
}
Input Input::fromURL(const ParsedURL & url, bool requireTree)
@ -140,6 +136,11 @@ bool Input::isDirect() const
return !scheme || scheme->isDirect(*this);
}
bool Input::isLocked() const
{
return scheme && scheme->isLocked(*this);
}
Attrs Input::toAttrs() const
{
return attrs;
@ -222,8 +223,6 @@ std::pair<StorePath, Input> Input::fetch(ref<Store> store) const
input.to_string(), *prevRevCount);
}
input.locked = true;
return {std::move(storePath), input};
}

12
src/libfetchers/fetchers.hh
View File

@ -29,7 +29,6 @@ struct Input
std::shared_ptr<InputScheme> scheme; // note: can be null
Attrs attrs;
bool locked = false;
/**
* path of the parent of this input, used for relative path resolution
@ -71,7 +70,7 @@ public:
* Check whether this is a "locked" input, that is,
* one that contains a commit hash or content hash.
*/
bool isLocked() const { return locked; }
bool isLocked() const;
bool operator ==(const Input & other) const;
@ -121,7 +120,6 @@ public:
std::optional<std::string> getFingerprint(ref<Store> store) const;
};
/**
* The `InputScheme` represents a type of fetcher. Each fetcher
* registers with nix at startup time. When processing an `Input`,
@ -196,6 +194,14 @@ struct InputScheme
*/
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; }
};
void registerInputScheme(std::shared_ptr<InputScheme> && fetcher);

7
src/libfetchers/git.cc
View File

@ -737,8 +737,6 @@ struct GitInputScheme : InputScheme
? getLastModified(repoInfo, repoInfo.url, *repoInfo.workdirInfo.headRev)
: 0);
input.locked = true; // FIXME
return {accessor, std::move(input)};
}
@ -775,6 +773,11 @@ struct GitInputScheme : InputScheme
else
return std::nullopt;
}
bool isLocked(const Input & input) const override
{
return (bool) input.getRev();
}
};
static auto rGitInputScheme = OnStartup([] { registerInputScheme(std::make_unique<GitInputScheme>()); });

5
src/libfetchers/github.cc
View File

@ -280,6 +280,11 @@ struct GitArchiveInputScheme : InputScheme
return {accessor, input};
}
bool isLocked(const Input & input) const override
{
return (bool) input.getRev();
}
std::optional<ExperimentalFeature> experimentalFeature() const override
{
return Xp::Flakes;

5
src/libfetchers/mercurial.cc
View File

@ -347,6 +347,11 @@ struct MercurialInputScheme : InputScheme
return makeResult(infoAttrs, std::move(storePath));
}
bool isLocked(const Input & input) const override
{
return (bool) input.getRev();
}
std::optional<std::string> getFingerprint(ref<Store> store, const Input & input) const override
{
if (auto rev = input.getRev())

5
src/libfetchers/path.cc
View File

@ -87,6 +87,11 @@ struct PathInputScheme : InputScheme
writeFile((CanonPath(getAbsPath(input)) / path).abs(), contents);
}
bool isLocked(const Input & input) const override
{
return (bool) input.getNarHash();
}
CanonPath getAbsPath(const Input & input) const
{
auto path = getStrAttr(input.attrs, "path");

5
src/libfetchers/tarball.cc
View File

@ -260,6 +260,11 @@ struct CurlInputScheme : InputScheme
url.query.insert_or_assign("narHash", narHash->to_string(HashFormat::SRI, true));
return url;
}
bool isLocked(const Input & input) const override
{
return (bool) input.getNarHash();
}
};
struct FileInputScheme : CurlInputScheme

2
src/nix-env/nix-env.cc
View File

@ -143,7 +143,7 @@ static void getAllExprs(EvalState & state,
}
/* Load the expression on demand. */
auto vArg = state.allocValue();
vArg->mkString(path2.path.abs());
vArg->mkPath(path2);
if (seen.size() == maxAttrs)
throw Error("too many Nix expressions in directory '%1%'", path);
attrs.alloc(attrName).mkApp(&state.getBuiltin("import"), vArg);

2
src/nix/flake.cc
View File

@ -224,7 +224,7 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON
if (auto lastModified = flake.lockedRef.input.getLastModified())
j["lastModified"] = *lastModified;
j["path"] = store->printStorePath(flake.storePath);
j["locks"] = lockedFlake.lockFile.toJSON();
j["locks"] = lockedFlake.lockFile.toJSON().first;
logger->cout("%s", j.dump());
} else {
logger->cout(

4
tests/functional/fetchGit.sh
View File

@ -70,7 +70,7 @@ path2=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \"
[[ $(nix eval --raw --expr "builtins.readFile (fetchGit { url = file://$repo; rev = \"$rev2\"; } + \"/hello\")") = world ]]
# But without a hash, it fails
expectStderr 1 nix eval --expr 'builtins.fetchGit "file:///foo"' | grepQuiet "fetchGit requires a locked input"
expectStderr 1 nix eval --expr 'builtins.fetchGit "file:///foo"' | grepQuiet "'fetchGit' will not fetch unlocked input"
# Fetch again. This should be cached.
mv $repo ${repo}-tmp
@ -211,7 +211,7 @@ path6=$(nix eval --impure --raw --expr "(builtins.fetchTree { type = \"git\"; ur
[[ $path3 = $path6 ]]
[[ $(nix eval --impure --expr "(builtins.fetchTree { type = \"git\"; url = \"file://$TEST_ROOT/shallow\"; ref = \"dev\"; shallow = true; }).revCount or 123") == 123 ]]
expectStderr 1 nix eval --expr 'builtins.fetchTree { type = "git"; url = "file:///foo"; }' | grepQuiet "fetchTree requires a locked input"
expectStderr 1 nix eval --expr 'builtins.fetchTree { type = "git"; url = "file:///foo"; }' | grepQuiet "'fetchTree' will not fetch unlocked input"
# Explicit ref = "HEAD" should work, and produce the same outPath as without ref
path7=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"HEAD\"; }).outPath")