Skip to content

Commit

Permalink
lockFlake(): Always compute a NAR hash for inputs
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
edolstra committed Feb 7, 2025
1 parent 304a244 commit 823189d
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 32 deletions.
50 changes: 30 additions & 20 deletions src/libflake/flake/flake.cc
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,19 @@ static StorePath copyInputToStore(
return storePath;
}

static SourcePath maybeCopyInputToStore(
EvalState & state,
fetchers::Input & input,
ref<SourceAccessor> accessor,
CopyMode copyMode)
{
return copyMode == CopyMode::Lazy || (copyMode == CopyMode::RequireLockable && (input.isLocked() || input.getNarHash()))
? SourcePath(accessor)
: state.rootPath(
state.store->toRealPath(
copyInputToStore(state, input, accessor)));
}

static void forceTrivialValue(EvalState & state, Value & value, const PosIdx pos)
{
if (value.isThunk() && value.isTrivial())
Expand Down Expand Up @@ -399,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(
Expand All @@ -424,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, accessor))),
maybeCopyInputToStore(state, lockedRef.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(
Expand All @@ -462,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);
Expand Down Expand Up @@ -507,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<FlakeRef> parents;
Expand Down Expand Up @@ -635,11 +650,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?
Expand Down Expand Up @@ -790,11 +804,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, accessor))),
maybeCopyInputToStore(state, lockedRef.input, accessor, inputCopyMode),
lockedRef
};
}
Expand Down Expand Up @@ -906,7 +916,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() &&
Expand Down
13 changes: 11 additions & 2 deletions src/libflake/flake/flake.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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(
Expand Down
8 changes: 6 additions & 2 deletions src/libstore/store-api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
16 changes: 13 additions & 3 deletions src/nix/flake.cc
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ struct CmdFlakeUpdate : FlakeCommand
lockFlags.recreateLockFile = updateAll;
lockFlags.writeLockFile = true;
lockFlags.applyNixConfig = true;
lockFlags.forceLazy = true;
lockFlags.copyMode = CopyMode::Lazy;

lockFlake();
}
Expand Down Expand Up @@ -166,7 +166,7 @@ struct CmdFlakeLock : FlakeCommand
lockFlags.writeLockFile = true;
lockFlags.failOnUnlocked = true;
lockFlags.applyNixConfig = true;
lockFlags.forceLazy = true;
lockFlags.copyMode = CopyMode::Lazy;

lockFlake();
}
Expand Down Expand Up @@ -213,10 +213,14 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON

void run(nix::ref<nix::Store> store) override
{
lockFlags.forceLazy = true;
lockFlags.copyMode = CopyMode::Lazy;
auto lockedFlake = lockFlake();
auto & flake = lockedFlake.flake;

std::optional<StorePath> storePath;
if (flake.lockedRef.input.getNarHash())
storePath = flake.lockedRef.input.computeStorePath(*store);

if (json) {
nlohmann::json j;
if (flake.description)
Expand All @@ -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);
Expand All @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/flakes/flakes.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
9 changes: 6 additions & 3 deletions tests/functional/flakes/follow-paths.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 <<EOF
{
description = "Flake A";
inputs = {
B.url = "path:../flakeB";
};
outputs = { ... }: {};
outputs = { ... }: {
x = 123;
};
}
EOF

git -C $flakeFollowsA add flake.nix

expect 1 nix flake lock $flakeFollowsA 2>&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 <<EOF
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/flakes/unlocked-override.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ echo 456 > "$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" |
Expand Down

0 comments on commit 823189d

Please sign in to comment.