From 9e6b89c92c00c67ada5ea6f15b48b8f6c69b002b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 7 Feb 2025 14:58:22 +0100 Subject: [PATCH] lockFlake(): Always compute a NAR hash for inputs For the top-level flake, we don't need a NAR hash. But for inputs, we do. Also, add a test for the lazy behaviour of `nix flake metadata|lock`. --- src/libflake/flake/flake.cc | 51 ++++++++++++-------- src/libflake/flake/flake.hh | 13 ++++- src/libstore/store-api.cc | 8 ++- src/nix/flake.cc | 16 ++++-- tests/functional/flakes/flakes.sh | 2 +- tests/functional/flakes/follow-paths.sh | 9 ++-- tests/functional/flakes/unlocked-override.sh | 2 +- 7 files changed, 69 insertions(+), 32 deletions(-) diff --git a/src/libflake/flake/flake.cc b/src/libflake/flake/flake.cc index 90945f949..a0ba404cd 100644 --- a/src/libflake/flake/flake.cc +++ b/src/libflake/flake/flake.cc @@ -100,6 +100,20 @@ static StorePath copyInputToStore( return storePath; } +static SourcePath maybeCopyInputToStore( + EvalState & state, + fetchers::Input & input, + const fetchers::Input & originalInput, + ref accessor, + CopyMode copyMode) +{ + return copyMode == CopyMode::Lazy || (copyMode == CopyMode::RequireLockable && (input.isLocked() || input.getNarHash())) + ? SourcePath(accessor) + : state.rootPath( + state.store->toRealPath( + copyInputToStore(state, input, originalInput, accessor))); +} + static void forceTrivialValue(EvalState & state, Value & value, const PosIdx pos) { if (value.isThunk() && value.isTrivial()) @@ -398,7 +412,7 @@ static Flake getFlake( bool useRegistries, FlakeCache & flakeCache, const InputAttrPath & lockRootAttrPath, - bool forceLazy) + CopyMode copyMode) { // Fetch a lazy tree first. auto [accessor, resolvedRef, lockedRef] = fetchOrSubstituteTree( @@ -423,19 +437,14 @@ static Flake getFlake( // Re-parse flake.nix from the store. return readFlake( state, originalRef, resolvedRef, lockedRef, - forceLazy && lockedRef.input.isLocked() - ? SourcePath(accessor) - : // Copy the tree to the store. - state.rootPath( - state.store->toRealPath( - copyInputToStore(state, lockedRef.input, originalRef.input, accessor))), + maybeCopyInputToStore(state, lockedRef.input, originalRef.input, accessor, copyMode), lockRootAttrPath); } -Flake getFlake(EvalState & state, const FlakeRef & originalRef, bool useRegistries, bool forceLazy) +Flake getFlake(EvalState & state, const FlakeRef & originalRef, bool useRegistries, CopyMode copyMode) { FlakeCache flakeCache; - return getFlake(state, originalRef, useRegistries, flakeCache, {}, forceLazy); + return getFlake(state, originalRef, useRegistries, flakeCache, {}, copyMode); } static LockFile readLockFile( @@ -461,7 +470,7 @@ LockedFlake lockFlake( auto useRegistries = lockFlags.useRegistries.value_or(settings.useRegistries); - auto flake = getFlake(state, topRef, useRegistries, flakeCache, {}, lockFlags.forceLazy); + auto flake = getFlake(state, topRef, useRegistries, flakeCache, {}, lockFlags.copyMode); if (lockFlags.applyNixConfig) { flake.config.apply(settings); @@ -506,6 +515,13 @@ LockedFlake lockFlake( explicitCliOverrides.insert(i.first); } + /* For locking of inputs, we require at least a NAR + hash. I.e. we can't be fully lazy. */ + auto inputCopyMode = + lockFlags.copyMode == CopyMode::Lazy + ? CopyMode::RequireLockable + : lockFlags.copyMode; + LockFile newLockFile; std::vector parents; @@ -633,11 +649,10 @@ LockedFlake lockFlake( flakerefs relative to the parent flake. */ auto getInputFlake = [&]() { - if (auto resolvedPath = resolveRelativePath()) { + if (auto resolvedPath = resolveRelativePath()) return readFlake(state, *input.ref, *input.ref, *input.ref, *resolvedPath, inputAttrPath); - } else { - return getFlake(state, *input.ref, useRegistries, flakeCache, inputAttrPath, lockFlags.forceLazy); - } + else + return getFlake(state, *input.ref, useRegistries, flakeCache, inputAttrPath, inputCopyMode); }; /* Do we have an entry in the existing lock file? @@ -788,11 +803,7 @@ LockedFlake lockFlake( state, *input.ref, useRegistries, flakeCache); return { - lockFlags.forceLazy && lockedRef.input.isLocked() - ? SourcePath(accessor) - : state.rootPath( - state.store->toRealPath( - copyInputToStore(state, lockedRef.input, input.ref->input, accessor))), + maybeCopyInputToStore(state, lockedRef.input, input.ref->input, accessor, inputCopyMode), lockedRef }; } @@ -904,7 +915,7 @@ LockedFlake lockFlake( repo, so we should re-read it. FIXME: we could also just clear the 'rev' field... */ auto prevLockedRef = flake.lockedRef; - flake = getFlake(state, topRef, useRegistries, lockFlags.forceLazy); + flake = getFlake(state, topRef, useRegistries, lockFlags.copyMode); if (lockFlags.commitLockFile && flake.lockedRef.input.getRev() && diff --git a/src/libflake/flake/flake.hh b/src/libflake/flake/flake.hh index 3696fd110..93bd18188 100644 --- a/src/libflake/flake/flake.hh +++ b/src/libflake/flake/flake.hh @@ -123,11 +123,20 @@ struct Flake } }; +enum struct CopyMode { + //! Copy the input to the store. + RequireStorePath, + //! Ensure that the input is locked or has a NAR hash. + RequireLockable, + //! Just return a lazy source accessor. + Lazy, +}; + Flake getFlake( EvalState & state, const FlakeRef & flakeRef, bool useRegistries, - bool forceLazy = false); + CopyMode copyMode = CopyMode::RequireStorePath); /** * Fingerprint of a locked flake; used as a cache key. @@ -229,7 +238,7 @@ struct LockFlags /** * If set, do not copy the flake to the Nix store. */ - bool forceLazy = false; + CopyMode copyMode = CopyMode::RequireStorePath; }; LockedFlake lockFlake( diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 236622eae..25acdefc8 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -214,8 +214,12 @@ StorePath Store::addToStore( auto sink = sourceToSink([&](Source & source) { LengthSource lengthSource(source); storePath = addToStoreFromDump(lengthSource, name, fsm, method, hashAlgo, references, repair); - if (settings.warnLargePathThreshold && lengthSource.total >= settings.warnLargePathThreshold) - warn("copied large path '%s' to the store (%s)", path, renderSize(lengthSource.total)); + if (settings.warnLargePathThreshold && lengthSource.total >= settings.warnLargePathThreshold) { + static bool failOnLargePath = getEnv("_NIX_TEST_FAIL_ON_LARGE_PATH").value_or("") == "1"; + if (failOnLargePath) + throw Error("won't copy large path '%s' to the store (%d)", path, renderSize(lengthSource.total)); + warn("copied large path '%s' to the store (%d)", path, renderSize(lengthSource.total)); + } }); dumpPath(path, *sink, fsm, filter); sink->finish(); diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 1cc13bf59..37df51f37 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -133,7 +133,7 @@ public: lockFlags.recreateLockFile = updateAll; lockFlags.writeLockFile = true; lockFlags.applyNixConfig = true; - lockFlags.forceLazy = true; + lockFlags.copyMode = CopyMode::Lazy; lockFlake(); } @@ -166,7 +166,7 @@ struct CmdFlakeLock : FlakeCommand lockFlags.writeLockFile = true; lockFlags.failOnUnlocked = true; lockFlags.applyNixConfig = true; - lockFlags.forceLazy = true; + lockFlags.copyMode = CopyMode::Lazy; lockFlake(); } @@ -213,10 +213,14 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON void run(nix::ref store) override { - lockFlags.forceLazy = true; + lockFlags.copyMode = CopyMode::Lazy; auto lockedFlake = lockFlake(); auto & flake = lockedFlake.flake; + std::optional storePath; + if (flake.lockedRef.input.getNarHash()) + storePath = flake.lockedRef.input.computeStorePath(*store); + if (json) { nlohmann::json j; if (flake.description) @@ -237,6 +241,8 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON j["revCount"] = *revCount; if (auto lastModified = flake.lockedRef.input.getLastModified()) j["lastModified"] = *lastModified; + if (storePath) + j["path"] = store->printStorePath(*storePath); j["locks"] = lockedFlake.lockFile.toJSON().first; if (auto fingerprint = lockedFlake.getFingerprint(store, fetchSettings)) j["fingerprint"] = fingerprint->to_string(HashFormat::Base16, false); @@ -253,6 +259,10 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON logger->cout( ANSI_BOLD "Description:" ANSI_NORMAL " %s", *flake.description); + if (storePath) + logger->cout( + ANSI_BOLD "Path:" ANSI_NORMAL " %s", + store->printStorePath(*storePath)); if (auto rev = flake.lockedRef.input.getRev()) logger->cout( ANSI_BOLD "Revision:" ANSI_NORMAL " %s", diff --git a/tests/functional/flakes/flakes.sh b/tests/functional/flakes/flakes.sh index 8936afa22..f55d3a04d 100755 --- a/tests/functional/flakes/flakes.sh +++ b/tests/functional/flakes/flakes.sh @@ -75,7 +75,7 @@ hash1=$(echo "$json" | jq -r .revision) echo foo > "$flake1Dir/foo" git -C "$flake1Dir" add $flake1Dir/foo -[[ $(nix flake metadata flake1 --json --refresh | jq -r .dirtyRevision) == "$hash1-dirty" ]] +[[ $(_NIX_TEST_FAIL_ON_LARGE_PATH=1 nix flake metadata flake1 --json --refresh --warn-large-path-threshold 1 | jq -r .dirtyRevision) == "$hash1-dirty" ]] [[ "$(nix flake metadata flake1 --json | jq -r .fingerprint)" != null ]] echo -n '# foo' >> "$flake1Dir/flake.nix" diff --git a/tests/functional/flakes/follow-paths.sh b/tests/functional/flakes/follow-paths.sh index a71d4c6d7..c654e0650 100755 --- a/tests/functional/flakes/follow-paths.sh +++ b/tests/functional/flakes/follow-paths.sh @@ -118,20 +118,23 @@ nix flake lock $flakeFollowsA jq -r -c '.nodes | keys | .[]' $flakeFollowsA/flake.lock | grep "^foobar$" # Check that path: inputs cannot escape from their root. +# FIXME: this test is wonky because with lazy trees, ../flakeB at the root is equivalent to /flakeB and not an error. cat > $flakeFollowsA/flake.nix <&1 | grep '/flakeB.*is forbidden in pure evaluation mode' -expect 1 nix flake lock --impure $flakeFollowsA 2>&1 | grep '/flakeB.*does not exist' +expect 1 nix eval $flakeFollowsA#x 2>&1 | grep '/flakeB.*is forbidden in pure evaluation mode' +expect 1 nix eval --impure $flakeFollowsA#x 2>&1 | grep '/flakeB.*does not exist' # Test relative non-flake inputs. cat > $flakeFollowsA/flake.nix < "$flake1Dir"/x.nix expectStderr 1 nix flake lock "$flake2Dir" --override-input flake1 "$TEST_ROOT/flake1" | 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 +_NIX_TEST_FAIL_ON_LARGE_PATH=1 nix flake lock "$flake2Dir" --override-input flake1 "$TEST_ROOT/flake1" --allow-dirty-locks --warn-large-path-threshold 1 # Using a lock file with a dirty lock does not require --allow-dirty-locks, but should print a warning. expectStderr 0 nix eval "$flake2Dir#x" |