nrabulinski/lix
0
0
Fork 0
forked from lix-project/lix
Code Pull requests Activity

flake.cc: computeLocks: Only verify overrides when they could change

Browse source 
When we check for disappeared overrides, we can get "false positives"
for follows and overrides which are defined in the dependencies of the
flake we are locking, since they are not parsed by
parseFlakeInputs. However, at that point we already know that the
overrides couldn't have possible been changed if the input itself
hasn't changed (since we check that oldLock->originalRef == *input.ref
for the input's parent). So, to prevent this, only perform this check
when it was possible that the flake changed (e.g. the flake we're
locking, or a new input, or the input has changed and mustRefetch ==
true).
This commit is contained in:
Alexander Bantyev 2021-12-28 15:30:06 +03:00
parent 0e90b13ab1
commit 2664a216e5
No known key found for this signature in database
GPG key ID: E081FF12ADCB4AD5
2 changed files with 25 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
src/libexpr/flake/flake.cc
View file

@ -344,7 +344,8 @@ LockedFlake lockFlake(
const InputPath & inputPathPrefix, const InputPath & inputPathPrefix,
std::shared_ptr<const Node> oldNode, std::shared_ptr<const Node> oldNode,
const LockParent & parent, const LockParent & parent,
const Path & parentPath)> const Path & parentPath,
bool trustLock)>
computeLocks; computeLocks;
computeLocks = [&]( computeLocks = [&](
@ -353,7 +354,8 @@ LockedFlake lockFlake(
const InputPath & inputPathPrefix, const InputPath & inputPathPrefix,
std::shared_ptr<const Node> oldNode, std::shared_ptr<const Node> oldNode,
const LockParent & parent, const LockParent & parent,
const Path & parentPath) const Path & parentPath,
bool trustLock)
{ {
debug("computing lock file node '%s'", printInputPath(inputPathPrefix)); debug("computing lock file node '%s'", printInputPath(inputPathPrefix));
@ -464,15 +466,21 @@ LockedFlake lockFlake(
.isFlake = (*lockedNode)->isFlake, .isFlake = (*lockedNode)->isFlake,
}); });
} else if (auto follows = std::get_if<1>(&i.second)) { } else if (auto follows = std::get_if<1>(&i.second)) {
auto o = input.overrides.find(i.first); if (! trustLock) {
// It is possible that the flake has changed,
// so we must confirm all the follows that are in the lockfile are also in the flake.
auto overridePath(inputPath);
overridePath.push_back(i.first);
auto o = overrides.find(overridePath);
// If the override disappeared, we have to refetch the flake, // If the override disappeared, we have to refetch the flake,
// since some of the inputs may not be present in the lockfile. // since some of the inputs may not be present in the lockfile.
if (o == input.overrides.end()) { if (o == overrides.end()) {
mustRefetch = true; mustRefetch = true;
// There's no point populating the rest of the fake inputs, // There's no point populating the rest of the fake inputs,
// since we'll refetch the flake anyways. // since we'll refetch the flake anyways.
break; break;
} }
}
fakeInputs.emplace(i.first, FlakeInput { fakeInputs.emplace(i.first, FlakeInput {
.follows = *follows, .follows = *follows,
}); });
@ -482,14 +490,14 @@ LockedFlake lockFlake(
LockParent newParent { LockParent newParent {
.path = inputPath, .path = inputPath,
.absolute = false .absolute = true
}; };
computeLocks( computeLocks(
mustRefetch mustRefetch
? getFlake(state, oldLock->lockedRef, false, flakeCache).inputs ? getFlake(state, oldLock->lockedRef, false, flakeCache).inputs
: fakeInputs, : fakeInputs,
childNode, inputPath, oldLock, newParent, parentPath); childNode, inputPath, oldLock, newParent, parentPath, !mustRefetch);
} else { } else {
/* We need to create a new lock file entry. So fetch /* We need to create a new lock file entry. So fetch
@ -546,7 +554,7 @@ LockedFlake lockFlake(
? std::dynamic_pointer_cast<const Node>(oldLock) ? std::dynamic_pointer_cast<const Node>(oldLock)
: LockFile::read( : LockFile::read(
inputFlake.sourceInfo->actualPath + "/" + inputFlake.lockedRef.subdir + "/flake.lock").root, inputFlake.sourceInfo->actualPath + "/" + inputFlake.lockedRef.subdir + "/flake.lock").root,
newParent, localPath); newParent, localPath, false);
} }
else { else {
@ -574,7 +582,7 @@ LockedFlake lockFlake(
computeLocks( computeLocks(
flake.inputs, newLockFile.root, {}, flake.inputs, newLockFile.root, {},
lockFlags.recreateLockFile ? nullptr : oldLockFile.root, parent, parentPath); lockFlags.recreateLockFile ? nullptr : oldLockFile.root, parent, parentPath, false);
for (auto & i : lockFlags.inputOverrides) for (auto & i : lockFlags.inputOverrides)
if (!overridesUsed.count(i.first)) if (!overridesUsed.count(i.first))

10
tests/flakes.sh
View file

@ -716,11 +716,10 @@ cat > $flakeFollowsA/flake.nix <<EOF
inputs = { inputs = {
B = { B = {
url = "path:./flakeB"; url = "path:./flakeB";
inputs.foobar.follows = "D"; inputs.foobar.follows = "foobar";
inputs.nonFlake.follows = "D";
}; };
D.url = "path:./flakeD"; foobar.url = "path:$flakeFollowsA/flakeE";
}; };
outputs = { ... }: {}; outputs = { ... }: {};
} }
@ -731,8 +730,6 @@ cat > $flakeFollowsB/flake.nix <<EOF
description = "Flake B"; description = "Flake B";
inputs = { inputs = {
foobar.url = "path:$flakeFollowsA/flakeE"; foobar.url = "path:$flakeFollowsA/flakeE";
nonFlake.url = "path:$nonFlakeDir";
goodoo.follows = "C/goodoo";
C = { C = {
url = "path:./flakeC"; url = "path:./flakeC";
inputs.foobar.follows = "foobar"; inputs.foobar.follows = "foobar";
@ -747,7 +744,6 @@ cat > $flakeFollowsC/flake.nix <<EOF
description = "Flake C"; description = "Flake C";
inputs = { inputs = {
foobar.url = "path:$flakeFollowsA/flakeE"; foobar.url = "path:$flakeFollowsA/flakeE";
goodoo.follows = "foobar";
}; };
outputs = { ... }: {}; outputs = { ... }: {};
} }
@ -785,7 +781,7 @@ newLock="$(cat "$flakeFollowsA/flake.lock")"
diff <(echo "$newLock") <(echo "$oldLock") diff <(echo "$newLock") <(echo "$oldLock")
[[ $(jq -c .nodes.B.inputs.C $flakeFollowsA/flake.lock) = '"C"' ]] [[ $(jq -c .nodes.B.inputs.C $flakeFollowsA/flake.lock) = '"C"' ]]
[[ $(jq -c .nodes.B.inputs.foobar $flakeFollowsA/flake.lock) = '["D"]' ]] [[ $(jq -c .nodes.B.inputs.foobar $flakeFollowsA/flake.lock) = '["foobar"]' ]]
[[ $(jq -c .nodes.C.inputs.foobar $flakeFollowsA/flake.lock) = '["B","foobar"]' ]] [[ $(jq -c .nodes.C.inputs.foobar $flakeFollowsA/flake.lock) = '["B","foobar"]' ]]
# Ensure removing follows from flake.nix removes them from the lockfile # Ensure removing follows from flake.nix removes them from the lockfile