From e1613932991b719854a0b23ff0ab0b42fc9fc747 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 10 Jan 2025 16:27:40 +0100 Subject: [PATCH] Add setting 'allow-dirty-locks' This allows writing lock files with dirty inputs, so long as they have a NAR hash. (Currently they always have a NAR hash, but with lazy trees that may not always be the case.) Generally dirty locks are bad for reproducibility (we can detect if the dirty input has changed, but we have no way to fetch it except substitution). Hence we don't allow them by default. Fixes #11181. --- src/libcmd/installables.cc | 2 +- src/libexpr/primops/fetchTree.cc | 2 +- src/libfetchers/fetch-settings.hh | 16 ++++++++++++++++ src/libfetchers/fetchers.cc | 7 +++++++ src/libfetchers/fetchers.hh | 9 +++++++++ src/libflake/flake/flake.cc | 16 +++++++++++----- src/libflake/flake/flake.hh | 4 +++- src/libflake/flake/lockfile.cc | 9 +++++---- src/libflake/flake/lockfile.hh | 2 +- src/libflake/flake/settings.hh | 2 +- src/nix/flake.cc | 4 ++-- tests/functional/flakes/unlocked-override.sh | 11 ++++++++++- 12 files changed, 67 insertions(+), 17 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 250cd1413..ab3ab3104 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -450,7 +450,7 @@ ref openEvalCache( std::shared_ptr lockedFlake) { auto fingerprint = evalSettings.useEvalCache && evalSettings.pureEval - ? lockedFlake->getFingerprint(state.store) + ? lockedFlake->getFingerprint(state.store, state.fetchSettings) : std::nullopt; auto rootLoader = [&state, lockedFlake]() { diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index 47d1be1cc..fe42b88f1 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -182,7 +182,7 @@ static void fetchTree( if (!state.settings.pureEval && !input.isDirect() && experimentalFeatureSettings.isEnabled(Xp::Flakes)) input = lookupInRegistries(state.store, input).first; - if (state.settings.pureEval && !input.isLocked()) { + if (state.settings.pureEval && !input.isConsideredLocked(state.fetchSettings)) { auto fetcher = "fetchTree"; if (params.isFetchGit) fetcher = "fetchGit"; diff --git a/src/libfetchers/fetch-settings.hh b/src/libfetchers/fetch-settings.hh index f7cb34a02..2ad8aa327 100644 --- a/src/libfetchers/fetch-settings.hh +++ b/src/libfetchers/fetch-settings.hh @@ -70,6 +70,22 @@ struct Settings : public Config Setting warnDirty{this, true, "warn-dirty", "Whether to warn about dirty Git/Mercurial trees."}; + Setting allowDirtyLocks{ + this, + false, + "allow-dirty-locks", + R"( + Whether to allow dirty inputs (such as dirty Git workdirs) + to be locked via their NAR hash. This is generally bad + practice since Nix has no way to obtain such inputs if they + are subsequently modified. Therefore lock files with dirty + locks should generally only be used for local testing, and + should not be pushed to other users. + )", + {}, + true, + Xp::Flakes}; + Setting trustTarballsFromGitForges{ this, true, "trust-tarballs-from-git-forges", R"( diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 87bf0dd71..35976f6a3 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -4,6 +4,7 @@ #include "fetch-to-store.hh" #include "json-utils.hh" #include "store-path-accessor.hh" +#include "fetch-settings.hh" #include @@ -154,6 +155,12 @@ bool Input::isLocked() const return scheme && scheme->isLocked(*this); } +bool Input::isConsideredLocked( + const Settings & settings) const +{ + return isLocked() || (settings.allowDirtyLocks && getNarHash()); +} + bool Input::isFinal() const { return maybeGetBoolAttr(attrs, "__final").value_or(false); diff --git a/src/libfetchers/fetchers.hh b/src/libfetchers/fetchers.hh index 841a44041..8cabd5103 100644 --- a/src/libfetchers/fetchers.hh +++ b/src/libfetchers/fetchers.hh @@ -95,6 +95,15 @@ public: */ bool isLocked() const; + /** + * Return whether the input is either locked, or, if + * `allow-dirty-locks` is enabled, it has a NAR hash. In the + * latter case, we can verify the input but we may not be able to + * fetch it from anywhere. + */ + bool isConsideredLocked( + const Settings & settings) const; + /** * Return whether this is a "final" input, meaning that fetching * it will not add, remove or change any attributes. (See diff --git a/src/libflake/flake/flake.cc b/src/libflake/flake/flake.cc index 29090b900..8c0c36eb3 100644 --- a/src/libflake/flake/flake.cc +++ b/src/libflake/flake/flake.cc @@ -345,6 +345,7 @@ Flake getFlake(EvalState & state, const FlakeRef & originalRef, bool allowLookup } static LockFile readLockFile( + const Settings & settings, const fetchers::Settings & fetchSettings, const SourcePath & lockFilePath) { @@ -380,6 +381,7 @@ LockedFlake lockFlake( } auto oldLockFile = readLockFile( + settings, state.fetchSettings, lockFlags.referenceLockFilePath.value_or( flake.lockFilePath())); @@ -616,7 +618,7 @@ LockedFlake lockFlake( inputFlake.inputs, childNode, inputPath, oldLock ? std::dynamic_pointer_cast(oldLock) - : readLockFile(state.fetchSettings, inputFlake.lockFilePath()).root.get_ptr(), + : readLockFile(settings, state.fetchSettings, inputFlake.lockFilePath()).root.get_ptr(), oldLock ? lockRootPath : inputPath, localPath, false); @@ -678,9 +680,11 @@ LockedFlake lockFlake( if (lockFlags.writeLockFile) { if (sourcePath || lockFlags.outputLockFilePath) { - if (auto unlockedInput = newLockFile.isUnlocked()) { + if (auto unlockedInput = newLockFile.isUnlocked(state.fetchSettings)) { if (lockFlags.failOnUnlocked) - throw Error("cannot write lock file of flake '%s' because it has an unlocked input ('%s').\n", topRef, *unlockedInput); + throw Error( + "Will not write lock file of flake '%s' because it has an unlocked input ('%s'). " + "Use '--allow-dirty-locks' to allow this anyway.", topRef, *unlockedInput); if (state.fetchSettings.warnDirty) warn("will not write lock file of flake '%s' because it has an unlocked input ('%s')", topRef, *unlockedInput); } else { @@ -979,9 +983,11 @@ static RegisterPrimOp r4({ } -std::optional LockedFlake::getFingerprint(ref store) const +std::optional LockedFlake::getFingerprint( + ref store, + const fetchers::Settings & fetchSettings) const { - if (lockFile.isUnlocked()) return std::nullopt; + if (lockFile.isUnlocked(fetchSettings)) return std::nullopt; auto fingerprint = flake.lockedRef.input.getFingerprint(store); if (!fingerprint) return std::nullopt; diff --git a/src/libflake/flake/flake.hh b/src/libflake/flake/flake.hh index 0dfd9440d..f85671a41 100644 --- a/src/libflake/flake/flake.hh +++ b/src/libflake/flake/flake.hh @@ -129,7 +129,9 @@ struct LockedFlake */ std::map, SourcePath> nodePaths; - std::optional getFingerprint(ref store) const; + std::optional getFingerprint( + ref store, + const fetchers::Settings & fetchSettings) const; }; struct LockFlags diff --git a/src/libflake/flake/lockfile.cc b/src/libflake/flake/lockfile.cc index 668ed165f..336054f5b 100644 --- a/src/libflake/flake/lockfile.cc +++ b/src/libflake/flake/lockfile.cc @@ -10,6 +10,7 @@ #include #include "strings.hh" +#include "flake/settings.hh" namespace nix::flake { @@ -43,8 +44,8 @@ LockedNode::LockedNode( , originalRef(getFlakeRef(fetchSettings, json, "original", nullptr)) , isFlake(json.find("flake") != json.end() ? (bool) json["flake"] : true) { - if (!lockedRef.input.isLocked()) - throw Error("lock file contains unlocked input '%s'", + if (!lockedRef.input.isConsideredLocked(fetchSettings)) + throw Error("Lock file contains unlocked input '%s'. Use '--allow-dirty-locks' to accept this lock file.", fetchers::attrsToJSON(lockedRef.input.toAttrs())); // For backward compatibility, lock file entries are implicitly final. @@ -228,7 +229,7 @@ std::ostream & operator <<(std::ostream & stream, const LockFile & lockFile) return stream; } -std::optional LockFile::isUnlocked() const +std::optional LockFile::isUnlocked(const fetchers::Settings & fetchSettings) const { std::set> nodes; @@ -247,7 +248,7 @@ std::optional LockFile::isUnlocked() const for (auto & i : nodes) { if (i == ref(root)) continue; auto node = i.dynamic_pointer_cast(); - if (node && (!node->lockedRef.input.isLocked() || !node->lockedRef.input.isFinal())) + if (node && (!node->lockedRef.input.isConsideredLocked(fetchSettings) || !node->lockedRef.input.isFinal())) return node->lockedRef; } diff --git a/src/libflake/flake/lockfile.hh b/src/libflake/flake/lockfile.hh index a2711a516..bcc3c460c 100644 --- a/src/libflake/flake/lockfile.hh +++ b/src/libflake/flake/lockfile.hh @@ -71,7 +71,7 @@ struct LockFile * Check whether this lock file has any unlocked or non-final * inputs. If so, return one. */ - std::optional isUnlocked() const; + std::optional isUnlocked(const fetchers::Settings & fetchSettings) const; bool operator ==(const LockFile & other) const; diff --git a/src/libflake/flake/settings.hh b/src/libflake/flake/settings.hh index fee247a7d..991eaca1f 100644 --- a/src/libflake/flake/settings.hh +++ b/src/libflake/flake/settings.hh @@ -29,7 +29,7 @@ struct Settings : public Config this, false, "accept-flake-config", - "Whether to accept nix configuration from a flake without prompting.", + "Whether to accept Nix configuration settings from a flake without prompting.", {}, true, Xp::Flakes}; diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 7189689cc..d92f644d9 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -238,7 +238,7 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON j["lastModified"] = *lastModified; j["path"] = storePath; j["locks"] = lockedFlake.lockFile.toJSON().first; - if (auto fingerprint = lockedFlake.getFingerprint(store)) + if (auto fingerprint = lockedFlake.getFingerprint(store, fetchSettings)) j["fingerprint"] = fingerprint->to_string(HashFormat::Base16, false); logger->cout("%s", j.dump()); } else { @@ -272,7 +272,7 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON logger->cout( ANSI_BOLD "Last modified:" ANSI_NORMAL " %s", std::put_time(std::localtime(&*lastModified), "%F %T")); - if (auto fingerprint = lockedFlake.getFingerprint(store)) + if (auto fingerprint = lockedFlake.getFingerprint(store, fetchSettings)) logger->cout( ANSI_BOLD "Fingerprint:" ANSI_NORMAL " %s", fingerprint->to_string(HashFormat::Base16, false)); diff --git a/tests/functional/flakes/unlocked-override.sh b/tests/functional/flakes/unlocked-override.sh index ebad332d0..dcb427a8f 100755 --- a/tests/functional/flakes/unlocked-override.sh +++ b/tests/functional/flakes/unlocked-override.sh @@ -31,5 +31,14 @@ echo 456 > "$flake1Dir"/x.nix [[ $(nix eval --json "$flake2Dir#x" --override-input flake1 "$TEST_ROOT/flake1") = 456 ]] +# Dirty overrides require --allow-dirty-locks. expectStderr 1 nix flake lock "$flake2Dir" --override-input flake1 "$TEST_ROOT/flake1" | - grepQuiet "cannot write lock file.*because it has an unlocked input" + grepQuiet "Will not write lock file.*because it has an unlocked input" + +nix flake lock "$flake2Dir" --override-input flake1 "$TEST_ROOT/flake1" --allow-dirty-locks + +# Using a lock file with a dirty lock requires --allow-dirty-locks as well. +expectStderr 1 nix eval "$flake2Dir#x" | + grepQuiet "Lock file contains unlocked input" + +[[ $(nix eval "$flake2Dir#x" --allow-dirty-locks) = 456 ]]