-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add repl-overlays
setting
#10203
base: master
Are you sure you want to change the base?
Add repl-overlays
setting
#10203
Conversation
0d21dcd
to
cf2a891
Compare
a7b5ef3
to
0e185d5
Compare
0e185d5
to
237ae78
Compare
This could alternatively be solved by improving the composition that can be done on the CLI. Only vaguely related as of now: #7076 |
Part of the whole point of this feature is that it doesn't require additional interaction to use — just a config file. |
Configuration is a double edged sword. It lets "power users" be more comfortable, most of the time, but it requires everyone to invest time into that sort of thing, or accept an unpleasant product. Meanwhile even power users aren't unaffected by the bad defaults, when they help someone with a different config, install onto a new machine, or need to do a bit of admin work on a remote host they don't own. The example fixes a pretty major annoyance, so I'm not sure if more configurability is the solution. Maybe it should just work better out of the box. While this doesn't completely solve the "configuration problem" I started with, it does solve it for popular flakes and flakes that use libraries like |
I do not see how having an extra feature be supported optionally has any impact on users that don't wish to use it. What is Nix but a tool ultimately that is extremely focused on power users? This proposal does not bind itself to flakes, such as all your examples do, which is a significant downside as not every workflow involves flakes. Furthermore if your desire is to keep Nix "lean" flexible configuration (possibly with sane defaults!) is the exact approach to addressing this issue. I don't see the point ultimately about hypothesizing endlessly about potential other solutions. I've seen this behavior on several of your other PR responses and it has personally made me feel like contributing to the Nix ecosystem is unproductive. The perfect is the enemy of the good, and this feature is good. What's happening here (and in Nix in general it seems) is endless bikeshedding while features people could start using today to improve their workflows languish, even when fully designed and ready with code. This is not the experience we want for people entering the Nix ecosystem, either as contributors or as users who desire these features. I'm in favor of this getting merged. @roberth please respond y/n on your opinion about merging this PR as is with only style/code cleanups. |
@Lunaphied please consider the point of view of the maintainers, who will be responsible for addressing issues that come up with this functionality, making sure that it's up to standards, responding to bug reports about why someone's repl doesn't have
Wow, I'd say you're more direct than the Dutch. |
Considering that you seem to prefer not to discuss, we have a set of issues labeled "idea approved" that have been discussed by the team and should be ready for implementation. Of course if those issues are not what you want to work on, that's ok, but then I would recommend to ask about it ahead of time. |
Just to make sure I understand the feature correctly: Can this be hacked together along these lines (except that # with-overlays.nix
{ overlays ? [] }:
let
info = { inherit (builtins) currentSystem; };
applyOverlays = # ...
in
applyOverlays info {
# base attrset goes here
} paths nix repl with-overlays.nix --arg overlays '[ ./overlay1.nix ./overlay2.nix ]' |
No, because the whole point is for something like |
I am well familiar with the difficulties and long-term consequences of adding and maintaining features, as well as Nix's learning curves. The problems you point out are ones that should be addressed by documentation and guidance, not by declining ideas that would improve Nix's overall UX. |
I'm not on the Nix team myself, but I really don't think Nix is in a good position to accept more features right now. There's already hundreds of existing bugs, half of what Nix does is poorly documented, and new regressions are seemingly being introduced with every release. Nix is in a bad spot and adding more features makes it harder to get out of it. My recommendation is to think about ideas how the Nix team can reduce their responsibilities. Concretely, this could mean helping with the C bindings and stabilising them, such that perhaps |
This position is understandable, but challenging to empathize with when PRs to add tests like #10075 are also rejected. |
Also, if Nix is in a feature freeze, or even a feature freeze for contributors outside the Nix team, that should be documented. But as-is, this feels like a post-hoc justification for rejecting my PR. (If bugs were a concern, your comment would have more likely read "can we add tests to this PR?" to which I would have replied: sure, if we merge the PR to add a decent test framework for |
@9999years there is no formal feature freeze, and certainly there is no deliberate bias towards or against a certain group of contributors. But we clearly express priorities for what is actively triaged and thus picked up into the review pipeline. New features are decidedly not on that list. |
`legacyPackages.${info.currentSystem}` (if that attribute is defined): | ||
|
||
```nix | ||
info: prev: final: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having three positional arguments, of which one is an attrset with fields like currentSystem
, seems unnecessarily complicated. I would prefer having a single attrset argument, e.g.
{ prev, final, currentSystem, ... }: ...
This would make it easier to add more fields in the future.
default bindings to [`nix | ||
repl`](@docroot@/command-ref/new-cli/nix3-repl.md) sessions. | ||
|
||
Each file is called with three arguments: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The help text documents what the arguments are, but not what the function is supposed to return.
@@ -137,6 +137,42 @@ struct EvalSettings : Config | |||
|
|||
This is useful for debugging warnings in third-party Nix code. | |||
)"}; | |||
|
|||
PathsSetting replOverlays{this, Paths(), "repl-overlays", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the interest of convention over configuration, it would be good if this had a default value like ~/.nix/config/repl.nix
.
The team has not yet reached a conclusion about this. There is a discussion about various options and paths forward. Specified in the flake # nixpkgs/flake.nix'
{
outputs = { self ,... }: {
legacyPackages = ...;
# (extraReplScope?)
# This is merged into the repl scope, not the flake
replScope = info: {
pkgs = self.legacyPackages.${info.currentSystem};
};
}
} Allows a flake to set attributes for convenience. Other consequence is that this is not a user-configuration, but an author-provided one. rc-script
replAs convenience, the repl can load [legacy]Pacakges.SYSTEM to top-level by default, to address this common need. No more configAdditional configuration reduces reproducibility, less teaching, discoverability about how to use the repl, obfuscation about data the repl is representing. If we have a default file/location, similar to overlays, one will have to ask users "do you have anything in your ~/.config/nix/nix.repl that might be changing your results or something in your settings?" On the other hand, without a default, one would have to keep specifying the config, not much better UX than today. @9999years the invite to come discuss this at a Nix meeting still stands. |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2023-04-08-nix-team-meeting-136/42963/1 |
Motivation
Fixes #9940 by adding a
repl-overlays
setting that can add bindings to thenix repl
.~/.config/nix/repl.nix
:Then:
Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.