From b2be6fed8600ee48c05cc9643c101d5eab4a5727 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra <edolstra@gmail.com> Date: Tue, 23 Apr 2024 16:00:52 +0200 Subject: [PATCH] Improve support for subflakes Subflakes are flakes in the same tree, accessed in flake inputs via relative paths (e.g. `inputs.foo.url = "path:./subdir"`). Previously these didn't work very well because they would be separately copied to the store, which is inefficient and makes references to parent directories tricky or impossible. Furthermore, they had their own NAR hash in the lock file, which is superfluous since the parent is already locked. Now subflakes are accessed via the accessor of the calling flake. This avoids the unnecessary copy and makes it possible for subflakes to depend on flakes in a parent directory (so long as they're in the same tree). Lock file nodes for relative flake inputs now have a new `parent` field: { "locked": { "path": "./subdir", "type": "path" }, "original": { "path": "./subdir", "type": "path" }, "parent": [ "foo", "bar" ] } which denotes that `./subdir` is to be interpreted relative to the directory of the `bar` input of the `foo` input of the root flake. Extracted from the lazy-trees branch. --- src/libexpr/flake/call-flake.nix | 7 + src/libexpr/flake/flake.cc | 160 +++++++++++++++------- src/libexpr/flake/flakeref.cc | 68 +++++---- src/libexpr/flake/flakeref.hh | 6 +- src/libexpr/flake/lockfile.cc | 9 +- src/libexpr/flake/lockfile.hh | 12 +- src/libexpr/primops/fetchTree.cc | 5 +- src/libfetchers/fetchers.cc | 6 + src/libfetchers/fetchers.hh | 14 +- src/libfetchers/path.cc | 25 +--- src/nix/flake.md | 25 +++- tests/functional/flakes/follow-paths.sh | 25 +++- tests/functional/flakes/relative-paths.sh | 89 ++++++++++++ tests/functional/local.mk | 1 + 14 files changed, 332 insertions(+), 120 deletions(-) create mode 100644 tests/functional/flakes/relative-paths.sh diff --git a/src/libexpr/flake/call-flake.nix b/src/libexpr/flake/call-flake.nix index a411564df..43ecb7f15 100644 --- a/src/libexpr/flake/call-flake.nix +++ b/src/libexpr/flake/call-flake.nix @@ -38,10 +38,17 @@ let (key: node: let + parentNode = allNodes.${getInputByPath lockFile.root node.parent}; + sourceInfo = if overrides ? ${key} then overrides.${key}.sourceInfo + else if node.locked.type == "path" && builtins.substring 0 1 node.locked.path != "/" + then + parentNode.sourceInfo // { + outPath = parentNode.sourceInfo.outPath + ("/" + node.locked.path); + } else # FIXME: remove obsolete node.info. fetchTree (node.info or {} // removeAttrs node.locked ["dir"]); diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 3af9ef14e..d4cabe68f 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -93,12 +93,17 @@ static void expectType(EvalState & state, ValueType type, } static std::map<FlakeId, FlakeInput> parseFlakeInputs( - EvalState & state, Value * value, const PosIdx pos, - const std::optional<Path> & baseDir, InputPath lockRootPath); + EvalState & state, + Value * value, + const PosIdx pos, + InputPath lockRootPath); -static FlakeInput parseFlakeInput(EvalState & state, - const std::string & inputName, Value * value, const PosIdx pos, - const std::optional<Path> & baseDir, InputPath lockRootPath) +static FlakeInput parseFlakeInput( + EvalState & state, + const std::string & inputName, + Value * value, + const PosIdx pos, + InputPath lockRootPath) { expectType(state, nAttrs, *value, pos); @@ -122,7 +127,7 @@ static FlakeInput parseFlakeInput(EvalState & state, expectType(state, nBool, *attr.value, attr.pos); input.isFlake = attr.value->boolean(); } 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) { expectType(state, nString, *attr.value, attr.pos); auto follows(parseInputPath(attr.value->c_str())); @@ -173,7 +178,7 @@ static FlakeInput parseFlakeInput(EvalState & state, if (!attrs.empty()) throw Error("unexpected flake input attribute '%s', at %s", attrs.begin()->first, state.positions[pos]); if (url) - input.ref = parseFlakeRef(*url, baseDir, true, input.isFlake); + input.ref = parseFlakeRef(*url, {}, true, input.isFlake, true); } if (!input.follows && !input.ref) @@ -183,8 +188,10 @@ static FlakeInput parseFlakeInput(EvalState & state, } static std::map<FlakeId, FlakeInput> parseFlakeInputs( - EvalState & state, Value * value, const PosIdx pos, - const std::optional<Path> & baseDir, InputPath lockRootPath) + EvalState & state, + Value * value, + const PosIdx pos, + InputPath lockRootPath) { std::map<FlakeId, FlakeInput> inputs; @@ -196,7 +203,6 @@ static std::map<FlakeId, FlakeInput> parseFlakeInputs( state.symbols[inputAttr.name], inputAttr.value, inputAttr.pos, - baseDir, lockRootPath)); } @@ -232,7 +238,7 @@ static Flake readFlake( auto sInputs = state.symbols.create("inputs"); if (auto inputs = vInfo.attrs()->get(sInputs)) - flake.inputs = parseFlakeInputs(state, inputs->value, inputs->pos, flakePath.parent().path.abs(), lockRootPath); // FIXME + flake.inputs = parseFlakeInputs(state, inputs->value, inputs->pos, lockRootPath); auto sOutputs = state.symbols.create("outputs"); @@ -366,13 +372,29 @@ LockedFlake lockFlake( debug("old lock file: %s", oldLockFile); - std::map<InputPath, FlakeInput> overrides; + struct Override + { + FlakeInput input; + SourcePath sourcePath; + std::optional<InputPath> parentPath; // FIXME: rename to inputPathPrefix? + }; + + std::map<InputPath, Override> overrides; std::set<InputPath> explicitCliOverrides; std::set<InputPath> overridesUsed, updatesUsed; std::map<ref<Node>, SourcePath> nodePaths; for (auto & i : lockFlags.inputOverrides) { - overrides.insert_or_assign(i.first, FlakeInput { .ref = i.second }); + overrides.emplace( + i.first, + Override { + .input = FlakeInput { .ref = i.second }, + /* Note: any relative overrides + (e.g. `--override-input B/C "path:./foo/bar"`) + are interpreted relative to the top-level + flake. */ + .sourcePath = flake.path, + }); explicitCliOverrides.insert(i.first); } @@ -386,7 +408,7 @@ LockedFlake lockFlake( const InputPath & inputPathPrefix, std::shared_ptr<const Node> oldNode, const InputPath & lockRootPath, - const Path & parentPath, + const SourcePath & sourcePath, bool trustLock)> computeLocks; @@ -402,7 +424,8 @@ LockedFlake lockFlake( copied. */ std::shared_ptr<const Node> oldNode, const InputPath & lockRootPath, - const Path & parentPath, + /* The source path of this node's flake. */ + const SourcePath & sourcePath, bool trustLock) { debug("computing lock file node '%s'", printInputPath(inputPathPrefix)); @@ -414,7 +437,12 @@ LockedFlake lockFlake( auto inputPath(inputPathPrefix); inputPath.push_back(id); inputPath.push_back(idOverride); - overrides.insert_or_assign(inputPath, inputOverride); + overrides.emplace(inputPath, + Override { + .input = inputOverride, + .sourcePath = sourcePath, + .parentPath = inputPathPrefix // FIXME: should this be inputPath? + }); } } @@ -446,13 +474,18 @@ LockedFlake lockFlake( auto i = overrides.find(inputPath); bool hasOverride = i != overrides.end(); bool hasCliOverride = explicitCliOverrides.contains(inputPath); - if (hasOverride) { + if (hasOverride) overridesUsed.insert(inputPath); - // Respect the “flakeness” of the input even if we - // override it - i->second.isFlake = input2.isFlake; - } - auto & input = hasOverride ? i->second : input2; + auto input = hasOverride ? i->second.input : input2; + + /* Resolve relative 'path:' inputs relative to + the source path of the overrider. */ + auto overridenSourcePath = hasOverride ? i->second.sourcePath : sourcePath; + + /* Respect the "flakeness" of the input even if we + override it. */ + if (hasOverride) + input.isFlake = input2.isFlake; /* Resolve 'follows' later (since it may refer to an input path we haven't processed yet. */ @@ -468,6 +501,33 @@ LockedFlake lockFlake( assert(input.ref); + auto overridenParentPath = + input.ref->input.isRelative() + ? std::optional<InputPath>(hasOverride ? i->second.parentPath : inputPathPrefix) + : std::nullopt; + + auto resolveRelativePath = [&]() -> std::optional<SourcePath> + { + if (auto relativePath = input.ref->input.isRelative()) { + return SourcePath { + overridenSourcePath.accessor, + CanonPath(*relativePath, overridenSourcePath.path.parent().value()) + }; + } else + return std::nullopt; + }; + + /* Get the input flake, resolve 'path:./...' + flakerefs relative to the parent flake. */ + auto getInputFlake = [&]() + { + if (auto resolvedPath = resolveRelativePath()) { + return readFlake(state, *input.ref, *input.ref, *input.ref, *resolvedPath, inputPath); + } else { + return getFlake(state, *input.ref, useRegistries, flakeCache, inputPath); + } + }; + /* Do we have an entry in the existing lock file? And the input is not in updateInputs? */ std::shared_ptr<LockedNode> oldLock; @@ -481,6 +541,7 @@ LockedFlake lockFlake( if (oldLock && oldLock->originalRef == *input.ref + && oldLock->parentPath == overridenParentPath && !hasCliOverride) { debug("keeping existing input '%s'", inputPathS); @@ -489,7 +550,10 @@ LockedFlake lockFlake( didn't change and there is no override from a higher level flake. */ auto childNode = make_ref<LockedNode>( - oldLock->lockedRef, oldLock->originalRef, oldLock->isFlake); + oldLock->lockedRef, + oldLock->originalRef, + oldLock->isFlake, + oldLock->parentPath); node->inputs.insert_or_assign(id, childNode); @@ -541,11 +605,15 @@ LockedFlake lockFlake( } if (mustRefetch) { - auto inputFlake = getFlake(state, oldLock->lockedRef, false, flakeCache, inputPath); + auto inputFlake = getInputFlake(); nodePaths.emplace(childNode, inputFlake.path.parent()); - computeLocks(inputFlake.inputs, childNode, inputPath, oldLock, lockRootPath, parentPath, false); + computeLocks(inputFlake.inputs, childNode, inputPath, oldLock, lockRootPath, inputFlake.path, false); } else { - computeLocks(fakeInputs, childNode, inputPath, oldLock, lockRootPath, parentPath, true); + // FIXME: sourcePath is wrong here, we + // should pass a lambda that lazily + // fetches the parent flake if needed + // (i.e. getInputFlake()). + computeLocks(fakeInputs, childNode, inputPath, oldLock, lockRootPath, sourcePath, true); } } else { @@ -553,7 +621,9 @@ LockedFlake lockFlake( this input. */ 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); /* Note: in case of an --override-input, we use @@ -566,17 +636,9 @@ LockedFlake lockFlake( auto ref = (input2.ref && explicitCliOverrides.contains(inputPath)) ? *input2.ref : *input.ref; if (input.isFlake) { - Path localPath = parentPath; - FlakeRef localRef = *input.ref; + auto inputFlake = getInputFlake(); - // 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); - - auto inputFlake = getFlake(state, localRef, useRegistries, flakeCache, inputPath); - - auto childNode = make_ref<LockedNode>(inputFlake.lockedRef, ref); + auto childNode = make_ref<LockedNode>(inputFlake.lockedRef, ref, true, overridenParentPath); node->inputs.insert_or_assign(id, childNode); @@ -598,17 +660,26 @@ LockedFlake lockFlake( ? std::dynamic_pointer_cast<const Node>(oldLock) : readLockFile(inputFlake.lockFilePath()).root.get_ptr(), oldLock ? lockRootPath : inputPath, - localPath, + inputFlake.path, false); } else { - auto [storePath, resolvedRef, lockedRef] = fetchOrSubstituteTree( - state, *input.ref, useRegistries, flakeCache); + auto [path, lockedRef] = [&]() -> std::tuple<SourcePath, FlakeRef> + { + // Handle non-flake 'path:./...' inputs. + if (auto resolvedPath = resolveRelativePath()) { + return {*resolvedPath, *input.ref}; + } else { + auto [storePath, resolvedRef, lockedRef] = fetchOrSubstituteTree( + state, *input.ref, useRegistries, flakeCache); + return {state.rootPath(state.store->toRealPath(storePath)), lockedRef}; + } + }(); - auto childNode = make_ref<LockedNode>(lockedRef, ref, false); + auto childNode = make_ref<LockedNode>(lockedRef, ref, false, overridenParentPath); - nodePaths.emplace(childNode, state.rootPath(state.store->toRealPath(storePath))); + nodePaths.emplace(childNode, path); node->inputs.insert_or_assign(id, childNode); } @@ -621,9 +692,6 @@ LockedFlake lockFlake( } }; - // Bring in the current ref for relative path resolution if we have it - auto parentPath = flake.path.parent().path.abs(); - nodePaths.emplace(newLockFile.root, flake.path.parent()); computeLocks( @@ -632,7 +700,7 @@ LockedFlake lockFlake( {}, lockFlags.recreateLockFile ? nullptr : oldLockFile.root.get_ptr(), {}, - parentPath, + flake.path, false); for (auto & i : lockFlags.inputOverrides) diff --git a/src/libexpr/flake/flakeref.cc b/src/libexpr/flake/flakeref.cc index 6e4aad64d..bac237a2a 100644 --- a/src/libexpr/flake/flakeref.cc +++ b/src/libexpr/flake/flakeref.cc @@ -51,9 +51,10 @@ FlakeRef parseFlakeRef( const std::string & url, const std::optional<Path> & baseDir, bool allowMissing, - bool isFlake) + bool isFlake, + bool allowRelative) { - auto [flakeRef, fragment] = parseFlakeRefWithFragment(url, baseDir, allowMissing, isFlake); + auto [flakeRef, fragment] = parseFlakeRefWithFragment(url, baseDir, allowMissing, isFlake, allowRelative); if (fragment != "") throw Error("unexpected fragment '%s' in flake reference '%s'", fragment, url); return flakeRef; @@ -69,11 +70,25 @@ std::optional<FlakeRef> maybeParseFlakeRef( } } +static std::pair<FlakeRef, std::string> fromParsedURL( + ParsedURL && parsedURL, + bool isFlake) +{ + auto dir = getOr(parsedURL.query, "dir", ""); + parsedURL.query.erase("dir"); + + std::string fragment; + std::swap(fragment, parsedURL.fragment); + + return std::make_pair(FlakeRef(fetchers::Input::fromURL(parsedURL, isFlake), dir), fragment); +} + std::pair<FlakeRef, std::string> parsePathFlakeRefWithFragment( const std::string & url, const std::optional<Path> & baseDir, bool allowMissing, - bool isFlake) + bool isFlake, + bool allowRelative) { std::string path = url; std::string fragment = ""; @@ -90,7 +105,7 @@ std::pair<FlakeRef, std::string> parsePathFlakeRefWithFragment( fragment = percentDecode(url.substr(fragmentStart+1)); } if (pathEnd != std::string::npos && fragmentStart != std::string::npos) { - query = decodeQuery(url.substr(pathEnd+1, fragmentStart-pathEnd-1)); + query = decodeQuery(url.substr(pathEnd + 1, fragmentStart - pathEnd - 1)); } if (baseDir) { @@ -154,6 +169,7 @@ std::pair<FlakeRef, std::string> parsePathFlakeRefWithFragment( .authority = "", .path = flakeRoot, .query = query, + .fragment = fragment, }; if (subdir != "") { @@ -165,9 +181,7 @@ std::pair<FlakeRef, std::string> parsePathFlakeRefWithFragment( if (pathExists(flakeRoot + "/.git/shallow")) parsedURL.query.insert_or_assign("shallow", "1"); - return std::make_pair( - FlakeRef(fetchers::Input::fromURL(parsedURL), getOr(parsedURL.query, "dir", "")), - fragment); + return fromParsedURL(std::move(parsedURL), isFlake); } subdir = std::string(baseNameOf(flakeRoot)) + (subdir.empty() ? "" : "/" + subdir); @@ -176,25 +190,30 @@ std::pair<FlakeRef, std::string> parsePathFlakeRefWithFragment( } } else { - if (!hasPrefix(path, "/")) + if (!allowRelative && !hasPrefix(path, "/")) throw BadURL("flake reference '%s' is not an absolute path", url); - path = canonPath(path + "/" + getOr(query, "dir", "")); } fetchers::Attrs attrs; attrs.insert_or_assign("type", "path"); attrs.insert_or_assign("path", path); - return std::make_pair(FlakeRef(fetchers::Input::fromAttrs(std::move(attrs)), ""), fragment); -}; - + return fromParsedURL({ + .url = path, + .base = path, + .scheme = "path", + .authority = "", + .path = path, + .query = query, + .fragment = fragment + }, isFlake); +} /* Check if 'url' is a flake ID. This is an abbreviated syntax for 'flake:<flake-id>?ref=<ref>&rev=<rev>'. */ std::optional<std::pair<FlakeRef, std::string>> parseFlakeIdRef( const std::string & url, - bool isFlake -) + bool isFlake) { std::smatch match; @@ -223,32 +242,21 @@ std::optional<std::pair<FlakeRef, std::string>> parseFlakeIdRef( std::optional<std::pair<FlakeRef, std::string>> parseURLFlakeRef( const std::string & url, const std::optional<Path> & baseDir, - bool isFlake -) + bool isFlake) { - ParsedURL parsedURL; try { - parsedURL = parseURL(url); + return fromParsedURL(parseURL(url), isFlake); } catch (BadURL &) { return std::nullopt; } - - std::string fragment; - std::swap(fragment, parsedURL.fragment); - - auto input = fetchers::Input::fromURL(parsedURL, isFlake); - input.parent = baseDir; - - return std::make_pair( - FlakeRef(std::move(input), getOr(parsedURL.query, "dir", "")), - fragment); } std::pair<FlakeRef, std::string> parseFlakeRefWithFragment( const std::string & url, const std::optional<Path> & baseDir, bool allowMissing, - bool isFlake) + bool isFlake, + bool allowRelative) { using namespace fetchers; @@ -259,7 +267,7 @@ std::pair<FlakeRef, std::string> parseFlakeRefWithFragment( } else if (auto res = parseURLFlakeRef(url, baseDir, isFlake)) { return *res; } else { - return parsePathFlakeRefWithFragment(url, baseDir, allowMissing, isFlake); + return parsePathFlakeRefWithFragment(url, baseDir, allowMissing, isFlake, allowRelative); } } diff --git a/src/libexpr/flake/flakeref.hh b/src/libexpr/flake/flakeref.hh index 04c812ed0..ea6c4e4d7 100644 --- a/src/libexpr/flake/flakeref.hh +++ b/src/libexpr/flake/flakeref.hh @@ -75,7 +75,8 @@ FlakeRef parseFlakeRef( const std::string & url, const std::optional<Path> & baseDir = {}, bool allowMissing = false, - bool isFlake = true); + bool isFlake = true, + bool allowRelative = false); /** * @param baseDir Optional [base directory](https://nixos.org/manual/nix/unstable/glossary#gloss-base-directory) @@ -90,7 +91,8 @@ std::pair<FlakeRef, std::string> parseFlakeRefWithFragment( const std::string & url, const std::optional<Path> & baseDir = {}, bool allowMissing = false, - bool isFlake = true); + bool isFlake = true, + bool allowRelative = false); /** * @param baseDir Optional [base directory](https://nixos.org/manual/nix/unstable/glossary#gloss-base-directory) diff --git a/src/libexpr/flake/lockfile.cc b/src/libexpr/flake/lockfile.cc index d252214dd..2884ca262 100644 --- a/src/libexpr/flake/lockfile.cc +++ b/src/libexpr/flake/lockfile.cc @@ -36,8 +36,9 @@ LockedNode::LockedNode(const nlohmann::json & json) : lockedRef(getFlakeRef(json, "locked", "info")) // FIXME: remove "info" , originalRef(getFlakeRef(json, "original", nullptr)) , isFlake(json.find("flake") != json.end() ? (bool) json["flake"] : true) + , parentPath(json.find("parent") != json.end() ? (std::optional<InputPath>) json["parent"] : std::nullopt) { - if (!lockedRef.input.isLocked()) + if (!lockedRef.input.isLocked() && !lockedRef.input.isRelative()) throw Error("lock file contains unlocked input '%s'", fetchers::attrsToJSON(lockedRef.input.toAttrs())); } @@ -184,6 +185,8 @@ std::pair<nlohmann::json, LockFile::KeyMap> LockFile::toJSON() const n["locked"] = fetchers::attrsToJSON(lockedNode->lockedRef.toAttrs()); if (!lockedNode->isFlake) n["flake"] = false; + if (lockedNode->parentPath) + n["parent"] = *lockedNode->parentPath; } nodes[key] = std::move(n); @@ -230,7 +233,9 @@ std::optional<FlakeRef> LockFile::isUnlocked() const for (auto & i : nodes) { if (i == ref<const Node>(root)) continue; auto node = i.dynamic_pointer_cast<const LockedNode>(); - if (node && !node->lockedRef.input.isLocked()) + if (node + && !node->lockedRef.input.isLocked() + && !node->lockedRef.input.isRelative()) return node->lockedRef; } diff --git a/src/libexpr/flake/lockfile.hh b/src/libexpr/flake/lockfile.hh index 7e62e6d09..aad805baf 100644 --- a/src/libexpr/flake/lockfile.hh +++ b/src/libexpr/flake/lockfile.hh @@ -38,11 +38,19 @@ struct LockedNode : Node FlakeRef lockedRef, originalRef; bool isFlake = true; + /* The node relative to which relative source paths + (e.g. 'path:../foo') are interpreted. */ + std::optional<InputPath> parentPath; + LockedNode( const FlakeRef & lockedRef, const FlakeRef & originalRef, - bool isFlake = true) - : lockedRef(lockedRef), originalRef(originalRef), isFlake(isFlake) + bool isFlake = true, + std::optional<InputPath> parentPath = {}) + : lockedRef(lockedRef) + , originalRef(originalRef) + , isFlake(isFlake) + , parentPath(parentPath) { } LockedNode(const nlohmann::json & json); diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index e27f30512..1c5cd4ec2 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -31,9 +31,8 @@ void emitTreeAttrs( // FIXME: support arbitrary input attributes. - auto narHash = input.getNarHash(); - assert(narHash); - attrs.alloc("narHash").mkString(narHash->to_string(HashFormat::SRI, true)); + if (auto narHash = input.getNarHash()) + attrs.alloc("narHash").mkString(narHash->to_string(HashFormat::SRI, true)); if (input.getType() == "git") attrs.alloc("submodules").mkBool( diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 73923907c..73cb4fea3 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -141,6 +141,12 @@ bool Input::isLocked() const return scheme && scheme->isLocked(*this); } +std::optional<std::string> Input::isRelative() const +{ + assert(scheme); + return scheme->isRelative(*this); +} + Attrs Input::toAttrs() const { return attrs; diff --git a/src/libfetchers/fetchers.hh b/src/libfetchers/fetchers.hh index 551be9a1f..66cac7064 100644 --- a/src/libfetchers/fetchers.hh +++ b/src/libfetchers/fetchers.hh @@ -31,11 +31,6 @@ struct Input std::shared_ptr<InputScheme> scheme; // note: can be null Attrs attrs; - /** - * path of the parent of this input, used for relative path resolution - */ - std::optional<Path> parent; - public: /** * Create an `Input` from a URL. @@ -73,6 +68,12 @@ public: */ bool isLocked() const; + /** + * Only for relative path flakes, i.e. 'path:./foo', returns the + * relative path, i.e. './foo'. + */ + std::optional<std::string> isRelative() const; + bool operator ==(const Input & other) const; bool contains(const Input & other) const; @@ -220,6 +221,9 @@ struct InputScheme * if there is a mismatch. */ virtual void checkLocks(const Input & specified, const Input & final) const; + + virtual std::optional<std::string> isRelative(const Input & input) const + { return std::nullopt; } }; void registerInputScheme(std::shared_ptr<InputScheme> && fetcher); diff --git a/src/libfetchers/path.cc b/src/libfetchers/path.cc index 68958d559..62500bbcb 100644 --- a/src/libfetchers/path.cc +++ b/src/libfetchers/path.cc @@ -116,31 +116,14 @@ struct PathInputScheme : InputScheme std::pair<ref<SourceAccessor>, Input> getAccessor(ref<Store> store, const Input & _input) const override { Input input(_input); - std::string absPath; auto path = getStrAttr(input.attrs, "path"); - if (path[0] != '/') { - if (!input.parent) - throw Error("cannot fetch input '%s' because it uses a relative path", input.to_string()); + auto absPath = getAbsPath(input); - auto parent = canonPath(*input.parent); - - // the path isn't relative, prefix it - absPath = nix::absPath(path, parent); - - // for security, ensure that if the parent is a store path, it's inside it - if (store->isInStore(parent)) { - auto storePath = store->printStorePath(store->toStorePath(parent).first); - if (!isDirOrInDir(absPath, storePath)) - throw BadStorePath("relative path '%s' points outside of its parent's store path '%s'", path, storePath); - } - } else - absPath = path; - - Activity act(*logger, lvlTalkative, actUnknown, fmt("copying '%s'", absPath)); + Activity act(*logger, lvlTalkative, actUnknown, fmt("copying '%s' to the store", absPath)); // FIXME: check whether access to 'path' is allowed. - auto storePath = store->maybeParseStorePath(absPath); + auto storePath = store->maybeParseStorePath(absPath.abs()); if (storePath) store->addTempRoot(*storePath); @@ -149,7 +132,7 @@ struct PathInputScheme : InputScheme if (!storePath || storePath->name() != "source" || !store->isValidPath(*storePath)) { // FIXME: try to substitute storePath. auto src = sinkToSource([&](Sink & sink) { - mtime = dumpPathAndGetMtime(absPath, sink, defaultPathFilter); + mtime = dumpPathAndGetMtime(absPath.abs(), sink, defaultPathFilter); }); storePath = store->addToStoreFromDump(*src, "source"); } diff --git a/src/nix/flake.md b/src/nix/flake.md index 661dd2f73..e60abe5fc 100644 --- a/src/nix/flake.md +++ b/src/nix/flake.md @@ -195,18 +195,29 @@ Currently the `type` attribute can be one of the following: If the flake at *path* is not inside a git repository, the `path:` prefix is implied and can be omitted. - *path* generally must be an absolute path. However, on the command - line, it can be a relative path (e.g. `.` or `./foo`) which is - interpreted as relative to the current directory. In this case, it - must start with `.` to avoid ambiguity with registry lookups - (e.g. `nixpkgs` is a registry lookup; `./nixpkgs` is a relative - path). + If *path* is a relative path (i.e. if it does not start with `/`), + it is interpreted as follows: + + - If *path* is a command line argument, it is interpreted relative + to the current directory. + + - If *path* is used in a `flake.nix`, it is interpreted relative to + the directory containing that `flake.nix`. However, the resolved + path must be in the same tree. For instance, a `flake.nix` in the + root of a tree can use `path:./foo` to access the flake in + subdirectory `foo`, but `path:../bar` is illegal. + + Note that if you omit `path:`, relative paths must start with `.` to + avoid ambiguity with registry lookups (e.g. `nixpkgs` is a registry + lookup; `./nixpkgs` is a relative path). For example, these are valid path flake references: * `path:/home/user/sub/dir` * `/home/user/sub/dir` (if `dir/flake.nix` is *not* in a git repository) - * `./sub/dir` (when used on the command line and `dir/flake.nix` is *not* in a git repository) + * `path:sub/dir` + * `./sub/dir` + * `path:../parent` * `git`: Git repositories. The location of the repository is specified by the attribute `url`. diff --git a/tests/functional/flakes/follow-paths.sh b/tests/functional/flakes/follow-paths.sh index 1afd91bd2..35f52f3ba 100644 --- a/tests/functional/flakes/follow-paths.sh +++ b/tests/functional/flakes/follow-paths.sh @@ -115,7 +115,7 @@ nix flake lock $flakeFollowsA [[ $(jq -c .nodes.B.inputs.foobar $flakeFollowsA/flake.lock) = '"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 path: inputs cannot escape from their root. cat > $flakeFollowsA/flake.nix <<EOF { description = "Flake A"; @@ -128,7 +128,28 @@ EOF git -C $flakeFollowsA add flake.nix -expect 1 nix flake lock $flakeFollowsA 2>&1 | grep 'points outside' +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' + +# Test relative non-flake inputs. +cat > $flakeFollowsA/flake.nix <<EOF +{ + description = "Flake A"; + inputs = { + E.flake = false; + E.url = "./foo.nix"; # test relative paths without 'path:' + }; + outputs = { E, ... }: { e = import E; }; +} +EOF + +echo 123 > $flakeFollowsA/foo.nix + +git -C $flakeFollowsA add flake.nix foo.nix + +nix flake lock $flakeFollowsA + +[[ $(nix eval --json $flakeFollowsA#e) = 123 ]] # Non-existant follows should print a warning. cat >$flakeFollowsA/flake.nix <<EOF diff --git a/tests/functional/flakes/relative-paths.sh b/tests/functional/flakes/relative-paths.sh new file mode 100644 index 000000000..cecda44d4 --- /dev/null +++ b/tests/functional/flakes/relative-paths.sh @@ -0,0 +1,89 @@ +source ./common.sh + +requireGit + +rootFlake=$TEST_ROOT/flake1 +subflake0=$rootFlake/sub0 +subflake1=$rootFlake/sub1 +subflake2=$rootFlake/sub2 + +rm -rf $rootFlake +mkdir -p $rootFlake $subflake0 $subflake1 $subflake2 + +cat > $rootFlake/flake.nix <<EOF +{ + inputs.sub0.url = "./sub0"; + outputs = { self, sub0 }: { + x = 2; + y = self.x * sub0.x; + }; +} +EOF + +cat > $subflake0/flake.nix <<EOF +{ + outputs = { self }: { + x = 7; + }; +} +EOF + +[[ $(nix eval $rootFlake#x) = 2 ]] +[[ $(nix eval $rootFlake#y) = 14 ]] + +cat > $subflake1/flake.nix <<EOF +{ + inputs.root.url = "../"; + outputs = { self, root }: { + x = 3; + y = self.x * root.x; + }; +} +EOF + +[[ $(nix eval $rootFlake?dir=sub1#y) = 6 ]] + +git init $rootFlake +git -C $rootFlake add flake.nix sub0/flake.nix sub1/flake.nix + +[[ $(nix eval $subflake1#y) = 6 ]] + +cat > $subflake2/flake.nix <<EOF +{ + inputs.root.url = "../"; + inputs.sub1.url = "../sub1"; + outputs = { self, root, sub1 }: { + x = 5; + y = self.x * sub1.x; + }; +} +EOF + +git -C $rootFlake add flake.nix sub2/flake.nix + +[[ $(nix eval $subflake2#y) = 15 ]] + +# Make sure there are no content locks for relative path flakes. +(! grep "$TEST_ROOT" $subflake2/flake.lock) +(! grep "$NIX_STORE_DIR" $subflake2/flake.lock) +(! grep narHash $subflake2/flake.lock) + +# Test circular relative path flakes. FIXME: doesn't work at the moment. +if false; then + +cat > $rootFlake/flake.nix <<EOF +{ + inputs.sub1.url = "./sub1"; + inputs.sub2.url = "./sub1"; + outputs = { self, sub1, sub2 }: { + x = 2; + y = self.x * sub1.x * sub2.x; + z = sub1.y * sub2.y; + }; +} +EOF + +[[ $(nix eval $rootFlake#x) = 30 ]] +[[ $(nix eval $rootFlake#z) = 90 ]] + +fi diff --git a/tests/functional/local.mk b/tests/functional/local.mk index 021af096c..06aa7cabf 100644 --- a/tests/functional/local.mk +++ b/tests/functional/local.mk @@ -7,6 +7,7 @@ nix_tests = \ flakes/circular.sh \ flakes/init.sh \ flakes/inputs.sh \ + flakes/relative-paths.sh \ flakes/follow-paths.sh \ flakes/bundle.sh \ flakes/check.sh \