From 20a1a65d377cb49dd0a1b60a07623760bfc730d1 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 21 May 2019 14:55:43 +0200 Subject: [PATCH 1/2] Only rewrite the lockfile if it changed This removes spurious warnings about failure to write the lockfile. --- src/libexpr/primops/flake.cc | 28 ++++++++++++++++------------ src/libexpr/primops/flake.hh | 21 +++++++++++++++++++++ 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/src/libexpr/primops/flake.cc b/src/libexpr/primops/flake.cc index 3cbb0c1ef..788977c72 100644 --- a/src/libexpr/primops/flake.cc +++ b/src/libexpr/primops/flake.cc @@ -465,31 +465,35 @@ ResolvedFlake resolveFlakeFromLockFile(EvalState & state, const FlakeRef & flake ResolvedFlake resolveFlake(EvalState & state, const FlakeRef & topRef, HandleLockFile handleLockFile) { Flake flake = getFlake(state, topRef, allowedToUseRegistries(handleLockFile, true)); - LockFile lockFile; + LockFile oldLockFile; if (!recreateLockFile (handleLockFile)) { // If recreateLockFile, start with an empty lockfile - lockFile = readLockFile(flake.storePath + "/flake.lock"); // FIXME: symlink attack + oldLockFile = readLockFile(flake.storePath + "/flake.lock"); // FIXME: symlink attack } + LockFile lockFile(oldLockFile); + ResolvedFlake resFlake = resolveFlakeFromLockFile(state, topRef, handleLockFile, lockFile, true); lockFile = entryToLockFile(dependenciesToFlakeEntry(resFlake)); - if (allowedToWrite(handleLockFile)) { - if (auto refData = std::get_if(&topRef.data)) { - writeLockFile(lockFile, refData->path + (topRef.subdir == "" ? "" : "/" + topRef.subdir) + "/flake.lock"); + if (!(lockFile == oldLockFile)) { + if (allowedToWrite(handleLockFile)) { + if (auto refData = std::get_if(&topRef.data)) { + writeLockFile(lockFile, refData->path + (topRef.subdir == "" ? "" : "/" + topRef.subdir) + "/flake.lock"); - // Hack: Make sure that flake.lock is visible to Git, so it ends up in the Nix store. - runProgram("git", true, { "-C", refData->path, "add", - (topRef.subdir == "" ? "" : topRef.subdir + "/") + "flake.lock" }); - } else std::cout << "Cannot write lockfile because the FlakeRef isn't of the form IsPath." << std::endl; - } else if (handleLockFile != AllPure && handleLockFile != TopRefUsesRegistries) - std::cout << "Using updating lockfile without writing it to file" << std::endl; + // Hack: Make sure that flake.lock is visible to Git, so it ends up in the Nix store. + runProgram("git", true, { "-C", refData->path, "add", + (topRef.subdir == "" ? "" : topRef.subdir + "/") + "flake.lock" }); + } else std::cout << "Cannot write lockfile because the FlakeRef isn't of the form IsPath." << std::endl; + } else if (handleLockFile != AllPure && handleLockFile != TopRefUsesRegistries) + std::cout << "Using updating lockfile without writing it to file" << std::endl; + } return resFlake; } -void updateLockFile (EvalState & state, const FlakeUri & flakeUri, bool recreateLockFile) +void updateLockFile(EvalState & state, const FlakeUri & flakeUri, bool recreateLockFile) { FlakeRef flakeRef(flakeUri); resolveFlake(state, flakeRef, recreateLockFile ? RecreateLockFile : UpdateLockFile); diff --git a/src/libexpr/primops/flake.hh b/src/libexpr/primops/flake.hh index 6f91686a6..e43b860ee 100644 --- a/src/libexpr/primops/flake.hh +++ b/src/libexpr/primops/flake.hh @@ -24,6 +24,11 @@ struct LockFile FlakeRef ref; Hash contentHash; NonFlakeEntry(const FlakeRef & flakeRef, const Hash & hash) : ref(flakeRef), contentHash(hash) {}; + + bool operator ==(const NonFlakeEntry & other) const + { + return ref == other.ref && contentHash == other.contentHash; + } }; struct FlakeEntry @@ -33,10 +38,26 @@ struct LockFile std::map flakeEntries; std::map nonFlakeEntries; FlakeEntry(const FlakeRef & flakeRef, const Hash & hash) : ref(flakeRef), contentHash(hash) {}; + + bool operator ==(const FlakeEntry & other) const + { + return + ref == other.ref + && contentHash == other.contentHash + && flakeEntries == other.flakeEntries + && nonFlakeEntries == other.nonFlakeEntries; + } }; std::map flakeEntries; std::map nonFlakeEntries; + + bool operator ==(const LockFile & other) const + { + return + flakeEntries == other.flakeEntries + && nonFlakeEntries == other.nonFlakeEntries; + } }; typedef std::vector> Registries; From 5990b86391a7b51879c865eed39ee46f39cbdd11 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 21 May 2019 15:03:54 +0200 Subject: [PATCH 2/2] Use warn(), tweak messages --- src/libexpr/primops/flake.cc | 35 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/src/libexpr/primops/flake.cc b/src/libexpr/primops/flake.cc index 788977c72..6998536ec 100644 --- a/src/libexpr/primops/flake.cc +++ b/src/libexpr/primops/flake.cc @@ -387,29 +387,17 @@ LockFile::FlakeEntry dependenciesToFlakeEntry(const ResolvedFlake & resolvedFlak return entry; } -bool allowedToWrite (HandleLockFile handle) +bool allowedToWrite(HandleLockFile handle) { - if (handle == AllPure) return false; - else if (handle == TopRefUsesRegistries) return false; - else if (handle == UpdateLockFile) return true; - else if (handle == UseUpdatedLockFile) return false; - else if (handle == RecreateLockFile) return true; - else if (handle == UseNewLockFile) return false; - else assert(false); + return handle == UpdateLockFile || handle == RecreateLockFile; } -bool recreateLockFile (HandleLockFile handle) +bool recreateLockFile(HandleLockFile handle) { - if (handle == AllPure) return false; - else if (handle == TopRefUsesRegistries) return false; - else if (handle == UpdateLockFile) return false; - else if (handle == UseUpdatedLockFile) return false; - else if (handle == RecreateLockFile) return true; - else if (handle == UseNewLockFile) return true; - else assert(false); + return handle == RecreateLockFile || handle == UseNewLockFile; } -bool allowedToUseRegistries (HandleLockFile handle, bool isTopRef) +bool allowedToUseRegistries(HandleLockFile handle, bool isTopRef) { if (handle == AllPure) return false; else if (handle == TopRefUsesRegistries) return isTopRef; @@ -433,11 +421,11 @@ ResolvedFlake resolveFlakeFromLockFile(EvalState & state, const FlakeRef & flake if (i != lockFile.nonFlakeEntries.end()) { NonFlake nonFlake = getNonFlake(state, i->second.ref, nonFlakeInfo.first); if (nonFlake.hash != i->second.contentHash) - throw Error("the content hash of flakeref %s doesn't match", i->second.ref.to_string()); + throw Error("the content hash of flakeref '%s' doesn't match", i->second.ref.to_string()); deps.nonFlakeDeps.push_back(nonFlake); } else { if (handleLockFile == AllPure || handleLockFile == TopRefUsesRegistries) - throw Error("the lockfile requires updating nonflake dependency %s in AllPure mode", nonFlakeInfo.first); + throw Error("cannot update non-flake dependency '%s' in pure mode", nonFlakeInfo.first); deps.nonFlakeDeps.push_back(getNonFlake(state, nonFlakeInfo.second, nonFlakeInfo.first)); } } @@ -447,11 +435,11 @@ ResolvedFlake resolveFlakeFromLockFile(EvalState & state, const FlakeRef & flake if (i != lockFile.flakeEntries.end()) { // Propagate lockFile downwards if possible ResolvedFlake newResFlake = resolveFlakeFromLockFile(state, i->second.ref, handleLockFile, entryToLockFile(i->second)); if (newResFlake.flake.hash != i->second.contentHash) - throw Error("the content hash of flakeref %s doesn't match", i->second.ref.to_string()); + throw Error("the content hash of flakeref '%s' doesn't match", i->second.ref.to_string()); deps.flakeDeps.insert_or_assign(newFlakeRef, newResFlake); } else { if (handleLockFile == AllPure || handleLockFile == TopRefUsesRegistries) - throw Error("the lockfile requires updating flake dependency %s in AllPure mode", newFlakeRef.to_string()); + throw Error("cannot update flake dependency '%s' in pure mode", newFlakeRef.to_string()); deps.flakeDeps.insert_or_assign(newFlakeRef, resolveFlakeFromLockFile(state, newFlakeRef, handleLockFile)); } } @@ -485,9 +473,10 @@ ResolvedFlake resolveFlake(EvalState & state, const FlakeRef & topRef, HandleLoc // Hack: Make sure that flake.lock is visible to Git, so it ends up in the Nix store. runProgram("git", true, { "-C", refData->path, "add", (topRef.subdir == "" ? "" : topRef.subdir + "/") + "flake.lock" }); - } else std::cout << "Cannot write lockfile because the FlakeRef isn't of the form IsPath." << std::endl; + } else + warn("cannot write lockfile of remote flake '%s'", topRef); } else if (handleLockFile != AllPure && handleLockFile != TopRefUsesRegistries) - std::cout << "Using updating lockfile without writing it to file" << std::endl; + warn("using updated lockfile without writing it to file"); } return resFlake;