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

nixFlakes: enable flakes #120141

Merged
merged 3 commits into from
May 12, 2021
Merged

Conversation

DavHau
Copy link
Member

@DavHau DavHau commented Apr 22, 2021

Motivation for this change

I assume everyone who installs nixFlakes wants to use the flakes feature.
It is an unnecessary complication to have to edit the nix.conf or set config options via environment variables.
This makes it harder to instruct other people on how to install nix with flakes.
It also makes it harder to ship nix + flakes in a shell environment.

If someone really wants to have an unstable nix without flakes enabled, they can still refer to nixUnstable.

Things done

wrap nixFlakes to set NIX_CONFIG="experimental-features = nix-command flakes"

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

I think this needs to be a postInstall script instead of a wrapper because tools access the version of nixFlakes.

@DavHau
Copy link
Member Author

DavHau commented Apr 22, 2021

Thanks. I wanted to prevent a rebuild, but that obviously makes no sense.
I now included a patch for nix that just changes the defaults of the experimental-features option in globals.hh

@edolstra
Copy link
Member

I think we should remove the nixFlakes attribute since it's no longer needed. Originally it was added because nixFlakes was a separate branch, but that's no longer the case.

Maybe a case could be made for having a nix-experimental attribute that enables all the experimental features by default.

@DavHau
Copy link
Member Author

DavHau commented Apr 22, 2021

I think we should remove the nixFlakes attribute since it's no longer needed. Originally it was added because nixFlakes was a separate branch, but that's no longer the case.

The nixFlakes attribute is already mentioned in a lot of blog posts etc. For people who want to try flakes, removing the nixFlakes attribute would downgrade the UX even more.
We could just keep the nixFlakes attribute in there and in the future make it an alias for nix stable when flakes becomes stable.

Maybe a case could be made for having a nix-experimental attribute that enables all the experimental features by default.

Whatever name we give this attribute, I would love to see a nix + flakes version that just works. Currently nix + flakes throws us back to the works on my machine situation where users have to care about configuring stuff manually.

@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Apr 23, 2021

The nixFlakes attribute is already mentioned in a lot of blog posts etc. For people who want to try flakes, removing the nixFlakes attribute would downgrade the UX even more.

It would be an alias to nixUnstable which would work for almost anyone.

Maybe a case could be made for having a nix-experimental attribute that enables all the experimental features by default.

That would break at least nix search for people who avoid flakes because they are still using channels or niv.

I am required to use nixUnstable because of NixOS/nix#4664 but using flakes for doing development is a pain and using yet another lock file is also not great.

@edolstra
Copy link
Member

That would break at least nix search for people who avoid flakes because they are still using channels or niv.

I don't follow - why would enabling experimental features break anything?

@SuperSandro2000
Copy link
Member

On old versions of nix search you can run the following:

$ command nix search hello
warning: using cached results; pass '-u' to update the cache
* nixpkgs.gnome3.iagno (iagno-3.38.1)
  Computer version of the game Reversi, more popularly called Othello

* nixpkgs.hello (hello)
  A program that produces a familiar, friendly greeting

* nixpkgs.hello-unfree (example-unfree-package-1.0)
  An example package with unfree license (for testing)

* nixpkgs.hello-wayland (hello-wayland-unstable)
  Hello world Wayland client

* nixpkgs.javaPackages.mavenHello_1_0 (maven-hello)
  Maven Hello World

* nixpkgs.javaPackages.mavenHello_1_1 (maven-hello)
  Maven Hello World

When you do the same with newer versions you get one of these errors:

$ NIX_CONFIG="experimental-features =" command nix search hello
error: experimental Nix feature 'nix-command' is disabled; use '--experimental-features nix-command' to override
$ NIX_CONFIG="experimental-features = nix-command" command nix search hello
error: experimental Nix feature 'flakes' is disabled; use '--experimental-features flakes' to override
$ NIX_CONFIG="experimental-features = nix-command flakes" command nix search hello
error: cannot find flake 'flake:hello' in the flake registries
$ NIX_CONFIG="experimental-features = nix-command flakes" command nix search #hello
...
all packages that are searchable
...
$ NIX_CONFIG="experimental-features = nix-command flakes" command nix search nixpkgs#hello
* legacyPackages.x86_64-linux.hello (2.10)
  A program that produces a familiar, friendly greeting

Only way I found to get the old behavior back is nix-shell -p nix --run "nix search hello"

So I think that enabling experimental-features for nixUnstable by default is not really an option.

@DavHau
Copy link
Member Author

DavHau commented Apr 24, 2021

@SuperSandro2000 I think there was no intention to enable experimental features for nixUnstable. As I understood, the idea of @edolstra was to have another attribute like for example nixExperimental with all experimental features enabled by default.

Considering all concerns that have been expressed in this thread so far, what do you guys think about the following?:

  • nixUnstable: Nix unstable unchanged
  • nixExperimental: Nix unstable, with all experimental features enabled
  • nixFlakes: Nix with the minimum experimental features enabled that are necessary to use flakes comfortably

We could make nixFlakes an alias for nixExperimental. But maybe nix will introduce features in the future that break flakes, so we might be better off just enabling nix-command and flakes for nixFlakes?

@DavHau DavHau force-pushed the davhau-nixFlakes-enable-flakes branch from be755f0 to 8865881 Compare May 4, 2021 11:16
@DavHau
Copy link
Member Author

DavHau commented May 4, 2021

I haven't heard back from you guys. I now implemented it like proposed in my last comment.

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label May 4, 2021
@Pacman99
Copy link
Contributor

Pacman99 commented May 5, 2021

I really like this proposal. I think its great UX for a user to be able to get into a shell with flakes completely ready.
Theres currently an annoying first step to get started with flakes. You have to enable the experimental feature and install nixUnstable before you can rebuild a system with proper nix flakes support. This PR would let all that be done in one nix shell command.

@mkg20001 mkg20001 merged commit 9c19ef7 into NixOS:master May 12, 2021
@edolstra
Copy link
Member

I'm not happy about this change and I think we should revert it. Usage of the nixFlakes attribute obscures the fact that it's an experimental feature. Providing a nixExperimental attribute (which people will inevitably start using on their production machines) makes it too easy to enable all experimental features, when you should explicitly opt in to the features that you want to try out.

Also, upstream Nix doesn't provide a "enable all experimental features" patch so neither should Nixpkgs.

@blaggacao
Copy link
Contributor

blaggacao commented May 19, 2021

Usage of the nixFlakes attribute obscures the fact that it's an experimental feature.

I think, this ought to be rephrased into:

Usage of the nixFlakes attribute obscures the claim that it's an experimental feature.

In reality, people that want to use it only had been bothered by a very bad UX to enable it. But, as a matter of fact, it's not unstable / experimental enough so that they wouldn't want to use it.

It seems that the meaning of the word "experimental" has already been eroded and diluted over the eons of times. Now, overall, that's probably the bigger issue. I'm not asked here to propose solutions, so I'm better not going to until I am.

@DavHau
Copy link
Member Author

DavHau commented May 20, 2021

I think what edolstra is referring to by the term experimental is not that the feature in it's current state is unreliable, but the fact that the feature is still subject to change. For example there are still discussions going on about changing the output format of flake.nix.
If this is going to happen, then the current nix will not be compatible anymore with future flakes and the opposite way round.

Thinking about that, I can understand that it would be very undesirable if someone accidentally starts depending on flakes without knowing this.

@blaggacao
Copy link
Contributor

blaggacao commented May 20, 2021

Thinking about that, I can understand that it would be very undesirable if someone accidentally starts depending on flakes without knowing this.

I agree, a CLI warning that an experimental feature is being used would ensure that no user is lead into false assumptions.

I guess one can see in nixExperimental an overshoot, but I still sustain and share your initial reasoning about nixFlakes.

So does "revert nixExperimemtal and add a warning into the nix command" sound like a solution?

@edolstra
Copy link
Member

@DavHau Yes, exactly. Experimental doesn't mean unusable.

@blaggacao

In reality, people that want to use it only had been bothered by a very bad UX to enable it.

Is it very bad UX? You add a line to a configuration file or set a flag on the command line. This seems pretty similar to how you enable experimental features in rustc, for instance.

I agree, a CLI warning that an experimental feature is being used would ensure that no user is lead into false assumptions.

That's possible, but warnings would be annoyingly spammy. I don't have to be reminded on every invocation that I enabled an experimental feature.

roberth pushed a commit that referenced this pull request May 21, 2021
Usage of the nixFlakes attribute obscures the fact that it's an
experimental feature. Providing a nixExperimental attribute (which
people will inevitably start using on their production machines) makes
it too easy to enable all experimental features, when you should
explicitly opt in to the features that you want to try out.

Also, upstream Nix doesn't provide an "enable all experimental
features" patch so neither should Nixpkgs.
@blaggacao
Copy link
Contributor

blaggacao commented May 21, 2021

Is it very bad UX? You add a line to a configuration file or set a flag on the command line. This seems pretty similar to how you enable experimental features in rustc, for instance.

The bad UX that I mean is really in that there are a perceived 10.000 different ways of enabling this at 10.000 different points.

  • Config
  • Environment variables
  • Command flag overrides
  • ...

in

  • CI
  • Nix
  • Nixos
  • Devshell
  • VM
  • ...

One has to do that every time, and it often times is nothing more than a cause for frustration.

So at least the nixFlakes patches are dear to me, since they solve exactly this.

mkg20001 added a commit to mkg20001/nixpkgs that referenced this pull request May 22, 2021
mkg20001 added a commit to mkg20001/nixpkgs that referenced this pull request Nov 9, 2021
mkg20001 added a commit to mkg20001/nixpkgs that referenced this pull request Nov 9, 2021
mkg20001 added a commit to mkg20001/nixpkgs that referenced this pull request Nov 10, 2021
mkg20001 added a commit to mkg20001/nixpkgs that referenced this pull request Nov 11, 2021
mkg20001 added a commit to mkg20001/nixpkgs that referenced this pull request Nov 13, 2021
mkg20001 added a commit to mkg20001/nixpkgs that referenced this pull request Nov 15, 2021
mkg20001 added a commit to mkg20001/nixpkgs that referenced this pull request Nov 18, 2021
mkg20001 added a commit to mkg20001/nixpkgs that referenced this pull request Nov 18, 2021
mkg20001 added a commit to mkg20001/nixpkgs that referenced this pull request Nov 20, 2021
mkg20001 added a commit to mkg20001/nixpkgs that referenced this pull request Nov 21, 2021
mkg20001 added a commit to mkg20001/nixpkgs that referenced this pull request Nov 21, 2021
mkg20001 added a commit to mkg20001/nixpkgs that referenced this pull request Nov 24, 2021
mkg20001 added a commit to mkg20001/nixpkgs that referenced this pull request Nov 28, 2021
mkg20001 added a commit to mkg20001/nixpkgs that referenced this pull request Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants