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

Conversation

zimbatm
Copy link
Member

@zimbatm zimbatm commented Dec 24, 2024

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. And builtins.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:

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.


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

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.
@zimbatm zimbatm requested a review from edolstra as a code owner December 24, 2024 14:36
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Dec 24, 2024
@zimbatm zimbatm requested review from Ericson2314 and Mic92 and removed request for edolstra December 24, 2024 15:25
Copy link
Member

@Mic92 Mic92 left a comment

Choose a reason for hiding this comment

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

Sounds like a good idea.

@@ -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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
with-tests Issues related to testing. PRs with tests have some priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants