Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

builtins.getFlake: also support path argument #12100

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/libflake/flake/flake.cc
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,11 @@ void initLib(const Settings & settings)
{
auto prim_getFlake = [&settings](EvalState & state, const PosIdx pos, Value * * args, Value & v)
{
std::string flakeRefS(state.forceStringNoCtx(*args[0], pos, "while evaluating the argument passed to builtins.getFlake"));
NixStringContext context; // no context
auto flakeRefS = state.coerceToString(pos, *args[0], context,
"while evaluating the argument passed to builtins.getFlake",
false, false, true).toOwned();
Comment on lines +826 to +829
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that because of the true argument, or a general coerceToString property since it also converts { outPath = "foo"; } to "foo"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


auto flakeRef = parseFlakeRef(state.fetchSettings, flakeRefS, {}, true);
if (state.settings.pureEval && !flakeRef.input.isLocked())
throw Error("cannot call 'getFlake' on unlocked flake reference '%s', at %s (use --impure to override)", flakeRefS, state.positions[pos]);
Expand All @@ -849,6 +853,12 @@ void initLib(const Settings & settings)
(builtins.getFlake "nix/55bc52401966fbffa525c574c14f67b00bc4fb3a").packages.x86_64-linux.nix
```

The function can also be used to load flakes from the file system. For example:

```nix
(builtins.getFlake ./.).packages.x86_64-linux.nix
```

Unless impure evaluation is allowed (`--impure`), the flake reference
must be "locked", e.g. contain a Git revision or content hash. An
example of an unlocked usage is:
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/flakes/shebang.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe leave this test and add this as a second one so it ensures the backwards compatibility?

#! nix let flake = (builtins.getFlake ../flake1).packages;
#! nix fooScript = flake.${builtins.currentSystem}.fooScript;
#! nix /* just a comment !@#$%^&*()__+ # */
#! nix in fooScript
Expand Down
4 changes: 2 additions & 2 deletions tests/functional/suggestions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ NIX_BUILD_STDERR_WITH_NO_CLOSE_SUGGESTION=$(! nix build .\#bar 2>&1 1>/dev/null)
[[ ! "$NIX_BUILD_STDERR_WITH_NO_CLOSE_SUGGESTION" =~ "Did you mean" ]] || \
fail "The nix build stderr shouldn’t suggest anything if there’s nothing relevant to suggest"

NIX_EVAL_STDERR_WITH_SUGGESTIONS=$(! nix build --impure --expr '(builtins.getFlake (builtins.toPath ./.)).packages.'"$system"'.fob' 2>&1 1>/dev/null)
[[ "$NIX_EVAL_STDERR_WITH_SUGGESTIONS" =~ "Did you mean one of fo1, fo2, foo or fooo?" ]] || \
NIX_EVAL_STDERR_WITH_SUGGESTIONS=$(! nix build --impure --expr '(builtins.getFlake ./.).packages.'"$system"'.fob' 2>&1 1>/dev/null)
[[ "$NIX_EVAL_STDERR_WITH_SUGGESTIONS" =~ "Did you mean one of fo1, fo2, foo or fooo?" ]] ||
fail "The evaluator should suggest the three closest possiblities"

NIX_EVAL_STDERR_WITH_SUGGESTIONS=$(! nix build --impure --expr '({ foo }: foo) { foo = 1; fob = 2; }' 2>&1 1>/dev/null)
Expand Down
Loading