builtins.getFlake: also support path argument #12100
Conversation
This is a papercut that has been bugging me for a while. `:lf .` is fine
but doesn't show you what variables it adds to the context. And
`builtins.getFlake (toString ./.)` works but gets tedious after a while.
When inspecting a flake in the repl, the natural thing to do is to call:
x = builtins.getFlake ./.
Before this change, Nix would fail with:
error: expected a string but found a path: /path/to/flake
After this change, Nix automatically coerces the path to a string.
github-actions
bot
added
the
with-tests
zimbatm
requested review from
Ericson2314 and
Mic92
and removed request for
edolstra
| @@ -69,7 +69,7 @@ EOF | |||
| cat >> "$scriptDir/shebang-inline-expr.sh" <<"EOF" | |||
| #! nix --offline shell | |||
| #! nix --impure --expr `` | |||
| #! nix let flake = (builtins.getFlake (toString ../flake1)).packages; | |||
There was a problem hiding this comment.
Maybe leave this test and add this as a second one so it ensures the backwards compatibility?
| NixStringContext context; // no context | ||
| auto flakeRefS = state.coerceToString(pos, *args[0], context, | ||
| "while evaluating the argument passed to builtins.getFlake", | ||
| false, false, true).toOwned(); |
There was a problem hiding this comment.
It seems that this now allows a derivation output to be passed, but it won't be realised, resulting in an error if the output isn't already in the store.
There was a problem hiding this comment.
Is that because of the true argument, or a general coerceToString property since it also converts { outPath = "foo"; } to "foo"?
There was a problem hiding this comment.
It's a couple of things I guess. I think coerceToPath is a better match for this use case, although the SourcePath doesn't slot nicely into the flakeref parsing, but we shouldn't be using any flakeref parsing for a path value anyway.
It seems best to have a separate code path for when the argument's .type() is nPath.
|
We should carefully consider (also with respect to lazy trees) what the intended behaviour of these relative path arguments is. Presumably, This PR also conflicts with #11952. |
|
That would be in line with
Presumably this does not produce a restricted root either. |
Motivation
This is a papercut that has been bugging me for a while.
:lf .is fine but doesn't show you what variables it adds to the context. Andbuiltins.getFlake (toString ./.)works but gets tedious after a while.Context
When inspecting a flake in the repl, the natural thing to do is to call:
Before this change, Nix would fail with:
After this change, Nix automatically coerces the path to a string.
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.