From 98f20dee41e9d4dccb5a6bbbd956ab856c5f7929 Mon Sep 17 00:00:00 2001 From: Nick Van den Broeck Date: Wed, 1 May 2019 11:38:48 +0200 Subject: [PATCH] Give errors in resolveFlake If DontUpdate but the lockfile isn't correct --- src/libexpr/primops/flake.cc | 70 ++++++++++++++++++++---------------- src/libexpr/primops/flake.hh | 8 ++--- src/nix/flake.cc | 3 +- src/nix/installables.cc | 2 +- tests/config.nix | 20 +++++++++++ tests/flakes.sh | 8 ++--- 6 files changed, 70 insertions(+), 41 deletions(-) create mode 100644 tests/config.nix diff --git a/src/libexpr/primops/flake.cc b/src/libexpr/primops/flake.cc index 88eadff55..c576a8b3e 100644 --- a/src/libexpr/primops/flake.cc +++ b/src/libexpr/primops/flake.cc @@ -50,8 +50,7 @@ LockFile::FlakeEntry readFlakeEntry(nlohmann::json json) if (!flakeRef.isImmutable()) throw Error("cannot use mutable flake '%s' in pure mode", flakeRef); - Hash hash = Hash((std::string) json["contentHash"]); - LockFile::FlakeEntry entry(flakeRef, hash); + LockFile::FlakeEntry entry(flakeRef, Hash((std::string) json["contentHash"])); auto nonFlakeRequires = json["nonFlakeRequires"]; @@ -59,9 +58,8 @@ LockFile::FlakeEntry readFlakeEntry(nlohmann::json json) FlakeRef flakeRef(i->value("uri", "")); if (!flakeRef.isImmutable()) throw Error("requested to fetch FlakeRef '%s' purely, which is mutable", flakeRef); - Hash hash = Hash((std::string) i->value("contentHash", "")); - LockFile::NonFlakeEntry newEntry(flakeRef, hash); - entry.nonFlakeEntries.insert_or_assign(i.key(), newEntry); + LockFile::NonFlakeEntry nonEntry(flakeRef, Hash(i->value("contentHash", ""))); + entry.nonFlakeEntries.insert_or_assign(i.key(), nonEntry); } auto requires = json["requires"]; @@ -89,10 +87,10 @@ LockFile readLockFile(const Path & path) for (auto i = nonFlakeRequires.begin(); i != nonFlakeRequires.end(); ++i) { FlakeRef flakeRef(i->value("uri", "")); - LockFile::NonFlakeEntry entry(flakeRef, Hash((std::string) json["contentHash"])); + LockFile::NonFlakeEntry nonEntry(flakeRef, Hash(i->value("contentHash", ""))); if (!flakeRef.isImmutable()) - throw Error("requested to fetch FlakeRef '%s' purely, which is mutable", flakeRef); - lockFile.nonFlakeEntries.insert_or_assign(i.key(), entry); + throw Error("found mutable FlakeRef '%s' in lockfile at path %s", flakeRef, path); + lockFile.nonFlakeEntries.insert_or_assign(i.key(), nonEntry); } auto requires = json["requires"]; @@ -374,19 +372,25 @@ LockFile entryToLockFile(const LockFile::FlakeEntry & entry) return lockFile; } -ResolvedFlake resolveFlakeFromLockFile(EvalState & state, const FlakeRef & flakeRef, RegistryAccess registryAccess, - LockFile lockFile, bool isTopFlake = false) +ResolvedFlake resolveFlakeFromLockFile(EvalState & state, const FlakeRef & flakeRef, + ShouldUpdateLockFile update, LockFile lockFile = {}) { - bool allowRegistries = registryAccess == AllowRegistry || (registryAccess == AllowRegistryAtTop && isTopFlake); - Flake flake = getFlake(state, flakeRef, allowRegistries); + Flake flake = getFlake(state, flakeRef, update != DontUpdate); ResolvedFlake deps(flake); for (auto & nonFlakeInfo : flake.nonFlakeRequires) { FlakeRef ref = nonFlakeInfo.second; auto i = lockFile.nonFlakeEntries.find(nonFlakeInfo.first); - if (i != lockFile.nonFlakeEntries.end()) ref = i->second.ref; - deps.nonFlakeDeps.push_back(getNonFlake(state, ref, nonFlakeInfo.first)); + 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()); + deps.nonFlakeDeps.push_back(nonFlake); + } else { + if (update == DontUpdate) throw Error("the lockfile requires updating nonflake dependency %s in DontUpdate mode", nonFlakeInfo.first); + deps.nonFlakeDeps.push_back(getNonFlake(state, nonFlakeInfo.second, nonFlakeInfo.first)); + } } for (auto newFlakeRef : flake.requires) { @@ -394,10 +398,14 @@ ResolvedFlake resolveFlakeFromLockFile(EvalState & state, const FlakeRef & flake LockFile newLockFile; auto i = lockFile.flakeEntries.find(newFlakeRef); if (i != lockFile.flakeEntries.end()) { // Propagate lockFile downwards if possible - ref = i->second.ref; - newLockFile = entryToLockFile(i->second); + ResolvedFlake newResFlake = resolveFlakeFromLockFile(state, i->second.ref, update, 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()); + deps.flakeDeps.push_back(newResFlake); + } else { + if (update == DontUpdate) throw Error("the lockfile requires updating flake dependency %s in DontUpdate mode", newFlakeRef.to_string()); + deps.flakeDeps.push_back(resolveFlakeFromLockFile(state, newFlakeRef, update)); } - deps.flakeDeps.push_back(resolveFlakeFromLockFile(state, ref, registryAccess, newLockFile)); } return deps; @@ -406,17 +414,18 @@ ResolvedFlake resolveFlakeFromLockFile(EvalState & state, const FlakeRef & flake /* Given a flake reference, recursively fetch it and its dependencies. FIXME: this should return a graph of flakes. */ -ResolvedFlake resolveFlake(EvalState & state, const FlakeRef & topRef, RegistryAccess registryAccess, - bool recreateLockFile) +ResolvedFlake resolveFlake(EvalState & state, const FlakeRef & topRef, ShouldUpdateLockFile update) { - bool allowRegistries = registryAccess == AllowRegistry || registryAccess == AllowRegistryAtTop; - Flake flake = getFlake(state, topRef, allowRegistries); + if (!std::get_if(&topRef.data)) update = DontUpdate; + Flake flake = getFlake(state, topRef, update != DontUpdate); LockFile lockFile; - if (!recreateLockFile) // If recreateLockFile, start with an empty lockfile + if (update != RecreateLockFile) { + // If recreateLockFile, start with an empty lockfile lockFile = readLockFile(flake.storePath + "/flake.lock"); // FIXME: symlink attack + } - return resolveFlakeFromLockFile(state, topRef, registryAccess, lockFile, true); + return resolveFlakeFromLockFile(state, topRef, update, lockFile); } LockFile::FlakeEntry dependenciesToFlakeEntry(const ResolvedFlake & resolvedFlake) @@ -426,15 +435,17 @@ LockFile::FlakeEntry dependenciesToFlakeEntry(const ResolvedFlake & resolvedFlak for (auto & newResFlake : resolvedFlake.flakeDeps) entry.flakeEntries.insert_or_assign(newResFlake.flake.originalRef, dependenciesToFlakeEntry(newResFlake)); - for (auto & nonFlake : resolvedFlake.nonFlakeDeps) - entry.nonFlakeEntries.insert_or_assign(nonFlake.alias, LockFile::NonFlakeEntry(nonFlake.resolvedRef, nonFlake.hash)); + for (auto & nonFlake : resolvedFlake.nonFlakeDeps) { + LockFile::NonFlakeEntry nonEntry(nonFlake.resolvedRef, nonFlake.hash); + entry.nonFlakeEntries.insert_or_assign(nonFlake.alias, nonEntry); + } return entry; } static LockFile makeLockFile(EvalState & evalState, FlakeRef & flakeRef, bool recreateLockFile) { - ResolvedFlake resFlake = resolveFlake(evalState, flakeRef, AllowRegistry, recreateLockFile); + ResolvedFlake resFlake = resolveFlake(evalState, flakeRef, recreateLockFile ? RecreateLockFile : UpdateLockFile); return entryToLockFile(dependenciesToFlakeEntry(resFlake)); } @@ -501,17 +512,16 @@ void callFlake(EvalState & state, const ResolvedFlake & resFlake, Value & v) // Return the `provides` of the top flake, while assigning to `v` the provides // of the dependencies as well. -void makeFlakeValue(EvalState & state, const FlakeRef & flakeRef, RegistryAccess registryAccess, Value & v, bool recreateLockFile) +void makeFlakeValue(EvalState & state, const FlakeRef & flakeRef, ShouldUpdateLockFile update, Value & v) { - callFlake(state, resolveFlake(state, flakeRef, registryAccess, recreateLockFile), v); + callFlake(state, resolveFlake(state, flakeRef, update), v); } // This function is exposed to be used in nix files. static void prim_getFlake(EvalState & state, const Pos & pos, Value * * args, Value & v) { makeFlakeValue(state, state.forceStringNoCtx(*args[0], pos), - evalSettings.pureEval ? DisallowRegistry : AllowRegistryAtTop, v, false); - // `recreateLockFile == false` because this is the evaluation stage, which should be pure, and hence not recreate lockfiles. + evalSettings.pureEval ? DontUpdate : UpdateLockFile, v); } static RegisterPrimOp r2("getFlake", 1, prim_getFlake); diff --git a/src/libexpr/primops/flake.hh b/src/libexpr/primops/flake.hh index e3481e99e..132439b93 100644 --- a/src/libexpr/primops/flake.hh +++ b/src/libexpr/primops/flake.hh @@ -43,9 +43,9 @@ typedef std::vector> Registries; Path getUserRegistryPath(); -enum RegistryAccess { DisallowRegistry, AllowRegistry, AllowRegistryAtTop }; +enum ShouldUpdateLockFile { DontUpdate, UpdateLockFile, RecreateLockFile}; -void makeFlakeValue(EvalState & state, const FlakeRef & flakeRef, RegistryAccess registryAccess, Value & v, bool recreateLockFile); +void makeFlakeValue(EvalState &, const FlakeRef &, ShouldUpdateLockFile, Value &); std::shared_ptr readRegistry(const Path &); @@ -84,8 +84,8 @@ struct NonFlake FlakeRef originalRef; FlakeRef resolvedRef; std::optional revCount; + Hash hash; Path storePath; - Hash hash; // content hash // date NonFlake(const FlakeRef & origRef, const SourceInfo & sourceInfo) : originalRef(origRef), resolvedRef(sourceInfo.resolvedRef), revCount(sourceInfo.revCount), storePath(sourceInfo.storePath) {}; @@ -103,7 +103,7 @@ struct ResolvedFlake ResolvedFlake(const Flake & flake) : flake(flake) {} }; -ResolvedFlake resolveFlake(EvalState &, const FlakeRef &, RegistryAccess, bool recreateLockFile); +ResolvedFlake resolveFlake(EvalState &, const FlakeRef &, ShouldUpdateLockFile); void updateLockFile(EvalState &, const FlakeUri &, bool recreateLockFile); diff --git a/src/nix/flake.cc b/src/nix/flake.cc index d2cdf6fc9..fc0fc76b4 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -114,8 +114,7 @@ struct CmdFlakeDeps : FlakeCommand, MixJSON, StoreCommand, MixEvalArgs FlakeRef flakeRef(flakeUri); - bool recreateLockFile = false; - ResolvedFlake resFlake = resolveFlake(*evalState, flakeRef, AllowRegistryAtTop, recreateLockFile); + ResolvedFlake resFlake = resolveFlake(*evalState, flakeRef, UpdateLockFile); std::queue todo; todo.push(resFlake); diff --git a/src/nix/installables.cc b/src/nix/installables.cc index 6d784002a..25f3f4f9d 100644 --- a/src/nix/installables.cc +++ b/src/nix/installables.cc @@ -161,7 +161,7 @@ struct InstallableFlake : InstallableValue if (std::get_if(&flakeRef.data)) updateLockFile(state, flakeRef.to_string(), cmd.recreateLockFile); - makeFlakeValue(state, flakeRef, AllowRegistryAtTop, *vFlake, cmd.recreateLockFile); + makeFlakeValue(state, flakeRef, cmd.recreateLockFile ? RecreateLockFile : UpdateLockFile, *vFlake); auto vProvides = (*vFlake->attrs->get(state.symbols.create("provides")))->value; diff --git a/tests/config.nix b/tests/config.nix new file mode 100644 index 000000000..03810d57a --- /dev/null +++ b/tests/config.nix @@ -0,0 +1,20 @@ +with import ; + +rec { + inherit shell; + + path = coreutils; + + system = "x86_64-linux"; + + shared = builtins.getEnv "_NIX_TEST_SHARED"; + + mkDerivation = args: + derivation ({ + inherit system; + builder = shell; + args = ["-e" args.builder or (builtins.toFile "builder.sh" "if [ -e .attrs.sh ]; then source .attrs.sh; fi; eval \"$buildCommand\"")]; + PATH = path; + } // removeAttrs args ["builder" "meta"]) + // { meta = args.meta or {}; }; +} diff --git a/tests/flakes.sh b/tests/flakes.sh index 8b68aea65..d720eaf23 100644 --- a/tests/flakes.sh +++ b/tests/flakes.sh @@ -59,7 +59,7 @@ EOF git -C $flake2Dir add flake.nix git -C $flake2Dir commit -m 'Initial' -cat > $flake3/flake.nix < $flake3Dir/flake.nix < $flake3/flake.nix < $registry <