Merge pull request #5839 from tweag/balsoft/yet-another-follows-bugfix

flake.cc: computeLocks: Only verify overrides when they could change
This commit is contained in:
Eelco Dolstra 2022-01-03 20:51:58 +01:00 committed by GitHub
commit 96d08fcd66
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 25 additions and 21 deletions

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))

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