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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

flakes: adopt repl-flake behavior as default #9043

Merged
merged 1 commit into from Sep 28, 2023

Conversation

Kranzes
Copy link
Member

@Kranzes Kranzes commented Sep 25, 2023

Motivation

I think the behavior of repl-flake makes sense to be the default when flakes are enabled. Robert and I's idea was to avoid actually deprecating the repl-flake experimental feature with a warning to avoid annoyance and because it's frankly not the most important thing we want to deal with, so this should be pretty straightforward.

I already discussed with @fricklerhandwerk about making this change at NixCon and I finally implemented it with @roberth at NixCamp.

Context

Priorities

Add 馃憤 to pull requests you find important.

@github-actions github-actions bot added documentation new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority repl The Read Eval Print Loop, "nix repl" command and debugger labels Sep 25, 2023
@roberth
Copy link
Member

roberth commented Sep 25, 2023

to avoid annoyance

and potential conflict when the warning needs to be resolved in a file that's used with both nix versions (before and after this). E.g. a shared NixOS module where some users use nixos 23.05 (nix 2.13, before this PR) and other use nixos-unstable (will have this PR).

tests/repl.sh Show resolved Hide resolved
tests/repl.sh Show resolved Hide resolved
@tomberek
Copy link
Contributor

tomberek commented Sep 25, 2023

The breaking change should be more prominent or clear in the notes?

Primary breaking change
  is for the common usage of `nix repl '<nixpkgs>'` which can be recovered with
  `nix repl --file '<nixpkgs>'` or `nix repl --expr 'import <nixpkgs>{}'`.

https://github.com/NixOS/nix/blob/b19bd4f348aae8346563b63c806da2c82ca90ab3/doc/manual/src/release-notes/rl-2.10.md?plain=1#L4C55-L6C75

@roberth
Copy link
Member

roberth commented Sep 25, 2023

The breaking change

Formally not a breaking change, but it would be nice to address the flakes audience. I think you can use Tom's quote to tell flakes users what to do in the release notes.

@Kranzes Kranzes force-pushed the repl-flake-begone branch 2 times, most recently from 650956b to 6a15109 Compare September 27, 2023 17:52
@Ericson2314
Copy link
Member

I agree we should be making this more stable, but I a have a few doubts about this. IMO the name "repl flakes" is misleading: the point was that nix repl uses the same CLI syntax as all the other commands, and it just so happens that that CLI syntax (installables) today features flakes prominently.

Whether or not flakes are in used / the flakes experimental feature is enabled, I think it is is important that we make the nix repl CLI consistent. I think that means I would rather get rid of the the ability to disable ReplFlakes entirely than make it part of Flakes.

@roberth
Copy link
Member

roberth commented Sep 27, 2023

@Ericson2314

[regardless] the flakes experimental feature is enabled, I think it is is important that we make the nix repl CLI consistent.

Do you mean that the nix repl ./file behavior should first be removed in order to achieve consistency with the rest of the CLI?

By "promoting" this change from a niche flag to a flag with a significant user base (...), we can effectively do a partial roll-out of this change that doesn't affect non-flakes users yet. This PR could then be a step towards an unconditional removal of nix repl ./file.

@Ericson2314
Copy link
Member

@roberth I think we would then want to make it so either flakes or repl-flake is enabled.

To make it consistent we'd need to eventually merge repl-flake with nix-command --- it makes perfect sense for flakes to be disabled but to still have a consistent CLI. Unless we think that contra RFC 136 Flakes will be stabilized before the CLI (and I don't) it doesn't really make sense to make the only way of getting the new syntax enabling Flakes.

@tomberek tomberek merged commit 13ed5d7 into NixOS:master Sep 28, 2023
8 checks passed
@Kranzes
Copy link
Member Author

Kranzes commented Sep 28, 2023

This PR was merged over an iPad on wheels.
IMG_2023-09-28-01-49-28-396

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation new-cli Relating to the "nix" command repl The Read Eval Print Loop, "nix repl" command and debugger with-tests Issues related to testing. PRs with tests have some priority
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

5 participants