Fix subflake handling

Relative 'path:' flake inputs now use the containing flake's
InputAccessor. This has the following implications:

* They're no longer locked in the lock file.

* They don't cause an additional copy to the store.

* They can reference the containing directory (i.e. a subflake can
  have an input '../foo', so long as it doesn't go outside the
  top-level containing flake).

Note: this is not a complete fix for subflake handling, since the lock
file currently makes it ambiguous what the containing flake is. We'll
probably need to add another field to the lock file for that.

Fixes #6352.
This commit is contained in:
Eelco Dolstra 2022-06-02 13:48:55 +02:00
parent 6ab3b86cf5
commit 8a0a55fe12
No known key found for this signature in database
GPG key ID: 8170B4726D7198DE
6 changed files with 69 additions and 43 deletions

View file

@ -91,7 +91,6 @@ static std::map<FlakeId, FlakeInput> parseFlakeInputs(
EvalState & state, EvalState & state,
Value * value, Value * value,
const PosIdx pos, const PosIdx pos,
const std::optional<Path> & baseDir,
const InputPath & lockRootPath); const InputPath & lockRootPath);
static FlakeInput parseFlakeInput( static FlakeInput parseFlakeInput(
@ -99,7 +98,6 @@ static FlakeInput parseFlakeInput(
const std::string & inputName, const std::string & inputName,
Value * value, Value * value,
const PosIdx pos, const PosIdx pos,
const std::optional<Path> & baseDir,
const InputPath & lockRootPath) const InputPath & lockRootPath)
{ {
expectType(state, nAttrs, *value, pos); expectType(state, nAttrs, *value, pos);
@ -124,7 +122,7 @@ static FlakeInput parseFlakeInput(
expectType(state, nBool, *attr.value, attr.pos); expectType(state, nBool, *attr.value, attr.pos);
input.isFlake = attr.value->boolean; input.isFlake = attr.value->boolean;
} else if (attr.name == sInputs) { } else if (attr.name == sInputs) {
input.overrides = parseFlakeInputs(state, attr.value, attr.pos, baseDir, lockRootPath); input.overrides = parseFlakeInputs(state, attr.value, attr.pos, lockRootPath);
} else if (attr.name == sFollows) { } else if (attr.name == sFollows) {
expectType(state, nString, *attr.value, attr.pos); expectType(state, nString, *attr.value, attr.pos);
auto follows(parseInputPath(attr.value->string.s)); auto follows(parseInputPath(attr.value->string.s));
@ -166,7 +164,7 @@ static FlakeInput parseFlakeInput(
if (!attrs.empty()) if (!attrs.empty())
throw Error("unexpected flake input attribute '%s', at %s", attrs.begin()->first, state.positions[pos]); throw Error("unexpected flake input attribute '%s', at %s", attrs.begin()->first, state.positions[pos]);
if (url) if (url)
input.ref = parseFlakeRef(*url, baseDir, true, input.isFlake); input.ref = parseFlakeRef(*url, {}, true, input.isFlake);
} }
if (!input.follows && !input.ref) if (!input.follows && !input.ref)
@ -179,7 +177,6 @@ static std::map<FlakeId, FlakeInput> parseFlakeInputs(
EvalState & state, EvalState & state,
Value * value, Value * value,
const PosIdx pos, const PosIdx pos,
const std::optional<Path> & baseDir,
const InputPath & lockRootPath) const InputPath & lockRootPath)
{ {
std::map<FlakeId, FlakeInput> inputs; std::map<FlakeId, FlakeInput> inputs;
@ -192,7 +189,6 @@ static std::map<FlakeId, FlakeInput> parseFlakeInputs(
state.symbols[inputAttr.name], state.symbols[inputAttr.name],
inputAttr.value, inputAttr.value,
inputAttr.pos, inputAttr.pos,
baseDir,
lockRootPath)); lockRootPath));
} }
@ -204,11 +200,11 @@ static Flake readFlake(
const FlakeRef & originalRef, const FlakeRef & originalRef,
const FlakeRef & resolvedRef, const FlakeRef & resolvedRef,
const FlakeRef & lockedRef, const FlakeRef & lockedRef,
InputAccessor & accessor, const SourcePath & rootDir,
const InputPath & lockRootPath) const InputPath & lockRootPath)
{ {
CanonPath flakeDir(resolvedRef.subdir); CanonPath flakeDir(resolvedRef.subdir);
SourcePath flakePath{accessor, flakeDir + CanonPath("flake.nix")}; auto flakePath = rootDir + flakeDir + "flake.nix";
if (!flakePath.pathExists()) if (!flakePath.pathExists())
throw Error("source tree referenced by '%s' does not contain a file named '%s'", resolvedRef, flakePath.path); throw Error("source tree referenced by '%s' does not contain a file named '%s'", resolvedRef, flakePath.path);
@ -233,7 +229,7 @@ static Flake readFlake(
auto sInputs = state.symbols.create("inputs"); auto sInputs = state.symbols.create("inputs");
if (auto inputs = vInfo.attrs->get(sInputs)) if (auto inputs = vInfo.attrs->get(sInputs))
flake.inputs = parseFlakeInputs(state, inputs->value, inputs->pos, flakeDir.abs(), lockRootPath); flake.inputs = parseFlakeInputs(state, inputs->value, inputs->pos, lockRootPath);
auto sOutputs = state.symbols.create("outputs"); auto sOutputs = state.symbols.create("outputs");
@ -322,7 +318,9 @@ static Flake getFlake(
auto [accessor, lockedRef] = resolvedRef.lazyFetch(state.store); auto [accessor, lockedRef] = resolvedRef.lazyFetch(state.store);
return readFlake(state, originalRef, resolvedRef, lockedRef, state.registerAccessor(accessor), lockRootPath); ;
return readFlake(state, originalRef, resolvedRef, lockedRef, SourcePath {state.registerAccessor(accessor), CanonPath::root}, lockRootPath);
} }
Flake getFlake(EvalState & state, const FlakeRef & originalRef, bool allowLookup, FlakeCache & flakeCache) Flake getFlake(EvalState & state, const FlakeRef & originalRef, bool allowLookup, FlakeCache & flakeCache)
@ -450,6 +448,22 @@ LockedFlake lockFlake(
assert(input.ref); assert(input.ref);
/* Get the input flake, resolve 'path:./...'
flakerefs relative to the parent flake. */
auto getInputFlake = [&]()
{
if (input.ref->input.isRelative()) {
SourcePath inputSourcePath {
parentPath.accessor,
CanonPath(*input.ref->input.getSourcePath(), *parentPath.path.parent())
};
// FIXME: we need to record in the lock
// file what the parent flake is.
return readFlake(state, *input.ref, *input.ref, *input.ref, inputSourcePath, inputPath);
} else
return getFlake(state, *input.ref, useRegistries, flakeCache, inputPath);
};
/* Do we have an entry in the existing lock file? And we /* Do we have an entry in the existing lock file? And we
don't have a --update-input flag for this input? */ don't have a --update-input flag for this input? */
std::shared_ptr<LockedNode> oldLock; std::shared_ptr<LockedNode> oldLock;
@ -523,39 +537,27 @@ LockedFlake lockFlake(
} }
} }
auto localPath(parentPath); if (mustRefetch) {
#if 0 auto inputFlake = getInputFlake();
// If this input is a path, recurse it down. computeLocks(inputFlake.inputs, childNode, inputPath, oldLock, lockRootPath, inputFlake.path, !mustRefetch);
// This allows us to resolve path inputs relative to the current flake. } else {
if ((*input.ref).input.getType() == "path") // FIXME: parentPath is wrong here, we
localPath = absPath(*input.ref->input.getSourcePath(), parentPath); // should pass a lambda that lazily
#endif // fetches the parent flake if needed
computeLocks( // (i.e. getInputFlake()).
mustRefetch computeLocks(fakeInputs, childNode, inputPath, oldLock, lockRootPath, parentPath, !mustRefetch);
? getFlake(state, oldLock->lockedRef, false, flakeCache, inputPath).inputs }
: fakeInputs,
childNode, inputPath, oldLock, lockRootPath, parentPath, !mustRefetch);
} else { } else {
/* We need to create a new lock file entry. So fetch /* We need to create a new lock file entry. So fetch
this input. */ this input. */
debug("creating new input '%s'", inputPathS); debug("creating new input '%s'", inputPathS);
if (!lockFlags.allowUnlocked && !input.ref->input.isLocked()) if (!lockFlags.allowUnlocked && !input.ref->input.isLocked() && !input.ref->input.isRelative())
throw Error("cannot update unlocked flake input '%s' in pure mode", inputPathS); throw Error("cannot update unlocked flake input '%s' in pure mode", inputPathS);
if (input.isFlake) { if (input.isFlake) {
auto localPath(parentPath); auto inputFlake = getInputFlake();
FlakeRef localRef = *input.ref;
#if 0
// If this input is a path, recurse it down.
// This allows us to resolve path inputs relative to the current flake.
if (localRef.input.getType() == "path")
localPath = absPath(*input.ref->input.getSourcePath(), parentPath);
#endif
auto inputFlake = getFlake(state, localRef, useRegistries, flakeCache, inputPath);
/* Note: in case of an --override-input, we use /* Note: in case of an --override-input, we use
the *original* ref (input2.ref) for the the *original* ref (input2.ref) for the
@ -585,7 +587,9 @@ LockedFlake lockFlake(
oldLock oldLock
? std::dynamic_pointer_cast<const Node>(oldLock) ? std::dynamic_pointer_cast<const Node>(oldLock)
: readLockFile(inputFlake).root, : readLockFile(inputFlake).root,
oldLock ? lockRootPath : inputPath, localPath, false); oldLock ? lockRootPath : inputPath,
inputFlake.path,
false);
} }
else { else {

View file

@ -35,7 +35,7 @@ LockedNode::LockedNode(const nlohmann::json & json)
, originalRef(getFlakeRef(json, "original", nullptr)) , originalRef(getFlakeRef(json, "original", nullptr))
, isFlake(json.find("flake") != json.end() ? (bool) json["flake"] : true) , isFlake(json.find("flake") != json.end() ? (bool) json["flake"] : true)
{ {
if (!lockedRef.input.isLocked()) if (!lockedRef.input.isLocked() && !lockedRef.input.isRelative())
throw Error("lock file contains unlocked input '%s'", throw Error("lock file contains unlocked input '%s'",
fetchers::attrsToJSON(lockedRef.input.toAttrs())); fetchers::attrsToJSON(lockedRef.input.toAttrs()));
} }
@ -215,8 +215,11 @@ bool LockFile::isLocked() const
for (auto & i : nodes) { for (auto & i : nodes) {
if (i == root) continue; if (i == root) continue;
auto lockedNode = std::dynamic_pointer_cast<const LockedNode>(i); auto node = std::dynamic_pointer_cast<const LockedNode>(i);
if (lockedNode && !lockedNode->lockedRef.input.isLocked()) return false; if (node
&& !node->lockedRef.input.isLocked()
&& !node->lockedRef.input.isRelative())
return false;
} }
return true; return true;

View file

@ -87,6 +87,11 @@ Attrs Input::toAttrs() const
return attrs; return attrs;
} }
bool Input::isRelative() const
{
return scheme->isRelative(*this);
}
bool Input::hasAllInfo() const bool Input::hasAllInfo() const
{ {
return getNarHash() && scheme && scheme->hasAllInfo(*this); return getNarHash() && scheme && scheme->hasAllInfo(*this);

View file

@ -63,6 +63,8 @@ public:
one that contains a commit hash or content hash. */ one that contains a commit hash or content hash. */
bool isLocked() const { return locked; } bool isLocked() const { return locked; }
bool isRelative() const;
bool hasAllInfo() const; bool hasAllInfo() const;
bool operator ==(const Input & other) const; bool operator ==(const Input & other) const;
@ -151,6 +153,9 @@ struct InputScheme
{ {
throw UnimplementedError("getAccessor"); throw UnimplementedError("getAccessor");
} }
virtual bool isRelative(const Input & input) const
{ return false; }
}; };
void registerInputScheme(std::shared_ptr<InputScheme> && fetcher); void registerInputScheme(std::shared_ptr<InputScheme> && fetcher);

View file

@ -66,6 +66,11 @@ struct PathInputScheme : InputScheme
}; };
} }
bool isRelative(const Input & input) const override
{
return !hasPrefix(*input.getSourcePath(), "/");
}
bool hasAllInfo(const Input & input) override bool hasAllInfo(const Input & input) override
{ {
return true; return true;

View file

@ -733,7 +733,9 @@ cat > $flakeFollowsA/flake.nix <<EOF
inputs.foobar.follows = "foobar"; inputs.foobar.follows = "foobar";
}; };
foobar.url = "path:$flakeFollowsA/flakeE"; # FIXME: currently absolute path: flakes cannot be locked.
#foobar.url = "path:$flakeFollowsA/flakeE";
foobar.url = "git+file://$flake1Dir";
}; };
outputs = { ... }: {}; outputs = { ... }: {};
} }
@ -743,7 +745,8 @@ cat > $flakeFollowsB/flake.nix <<EOF
{ {
description = "Flake B"; description = "Flake B";
inputs = { inputs = {
foobar.url = "path:$flakeFollowsA/flakeE"; #foobar.url = "path:$flakeFollowsA/flakeE";
foobar.url = "git+file://$flake1Dir";
goodoo.follows = "C/goodoo"; goodoo.follows = "C/goodoo";
C = { C = {
url = "path:./flakeC"; url = "path:./flakeC";
@ -758,7 +761,8 @@ cat > $flakeFollowsC/flake.nix <<EOF
{ {
description = "Flake C"; description = "Flake C";
inputs = { inputs = {
foobar.url = "path:$flakeFollowsA/flakeE"; #foobar.url = "path:$flakeFollowsA/flakeE";
foobar.url = "git+file://$flake1Dir";
goodoo.follows = "foobar"; goodoo.follows = "foobar";
}; };
outputs = { ... }: {}; outputs = { ... }: {};
@ -823,7 +827,7 @@ nix flake lock $flakeFollowsA
[[ $(jq -c .nodes.B.inputs.foobar $flakeFollowsA/flake.lock) = '"foobar"' ]] [[ $(jq -c .nodes.B.inputs.foobar $flakeFollowsA/flake.lock) = '"foobar"' ]]
jq -r -c '.nodes | keys | .[]' $flakeFollowsA/flake.lock | grep "^foobar$" jq -r -c '.nodes | keys | .[]' $flakeFollowsA/flake.lock | grep "^foobar$"
# Ensure a relative path is not allowed to go outside the store path # Check that subflakes are allowed to access flakes in the parent.
cat > $flakeFollowsA/flake.nix <<EOF cat > $flakeFollowsA/flake.nix <<EOF
{ {
description = "Flake A"; description = "Flake A";
@ -836,7 +840,7 @@ EOF
git -C $flakeFollowsA add flake.nix git -C $flakeFollowsA add flake.nix
nix flake lock $flakeFollowsA 2>&1 | grep 'points outside' nix flake lock $flakeFollowsA
# Test flake in store does not evaluate # Test flake in store does not evaluate
rm -rf $badFlakeDir rm -rf $badFlakeDir