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.

Sign up for GitHub

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

Jump to bottom

nixos/installer: use the configured nix package for nixos-install #99615

Merged
merged 1 commit into from Oct 6, 2020
Merged

nixos/installer: use the configured nix package for nixos-install #99615

merged 1 commit into from Oct 6, 2020

Conversation

andir
Copy link
Member

@andir andir commented Oct 5, 2020 •
edited

Motivation for this change

The current state lead to having two version of Nix in each system
closure unless someone defaulted to nixUnstable. We should probably
only pick the package that is actually desired by the user.

If someone needs an exotic installer with nixUnstable then they should
probably build an image with it for their exotic use case.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
    • all the installer tests succeeded

@roberth
Copy link
Member

roberth commented Oct 5, 2020

cc @ju1m

roberth
roberth previously approved these changes Oct 5, 2020
View reviewed changes
Ma27
Ma27 previously approved these changes Oct 5, 2020
View reviewed changes
@Ma27 Ma27 added this to the 20.09 milestone Oct 5, 2020
@Ma27
Copy link
Member

Ma27 commented Oct 5, 2020

cc @worldofpeace @cole-h

@cole-h
Copy link
Member

cole-h commented Oct 5, 2020

I think we made a conscious decision to use pkgs.nixUnstable rather than the configured Nix package.

Unless I'm mistaken (entirely possible), our ISOs don't use nixUnstable. This means users would have to create their own ISO in order to install their flakes-based config. By adding pkgs.nixUnstable to the PATH, it will always be possible to install a flakes-based machine.

@Ma27
Copy link
Member

Ma27 commented Oct 5, 2020

OTOH we now have two Nix versions and one of them contains unstable features hidden behind a feature flag, so I agree with @andir here. While I like playing around with flakes and I'm fairly happy that we start to support this in nixpkgs, I'm afraid that this would send some kind of a wrong message - especially to newcomers.

I'm not a RM, but is there anything wrong with shipping another ISO with "experimental stuff" which uses e.g. nixUnstable by default?

@andir
Copy link
Member Author

andir commented Oct 5, 2020

As far as I am concerned add as nixos-install-flaky command that can be enabled via a NixOS option and we enable that in the official install images. I just don't want a second version that'll never be used on my installed systems.

@worldofpeace
Copy link
Contributor

@Ma27
I'm not a RM, but is there anything wrong with shipping another ISO with "experimental stuff" which uses e.g. nixUnstable by default?

@andir
As far as I am concerned add as nixos-install-flaky command that can be enabled via a NixOS option and we enable that in the official install images. I just don't want a second version that'll never be used on my installed systems.

I'd be fine with a nixos-install-flake in the installer ISO and an option.

worldofpeace
worldofpeace suggested changes Oct 5, 2020
View reviewed changes
Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

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

Forgot to request changes, see https://github.com/NixOS/nixpkgs/pull/99615/files#r499779090

@worldofpeace worldofpeace added this to In progress in 20.09 Blockers via automation Oct 5, 2020
@worldofpeace worldofpeace moved this from In progress to Review in progress in 20.09 Blockers Oct 5, 2020
@andir
nixos/installer: drop the extra nixUnstable in nixos-install
544059b 
The only nix version available in the installer should be the version
configure in the module system. If someone needs `nixUnstable` in their
`nixos-install` they should probably set the module option and not just
add it to the closure.
@andir andir force-pushed the use-configured-nix-for-installer branch from 3cb65cb to 544059b Compare October 5, 2020 23:13
@worldofpeace worldofpeace merged commit 0d47426 into NixOS:master Oct 6, 2020
20.09 Blockers automation moved this from Review in progress to Done Oct 6, 2020
@andir
Copy link
Member Author

andir commented Oct 6, 2020

@worldofpeace do you also want this in 20.09?

@andir andir deleted the use-configured-nix-for-installer branch October 6, 2020 20:21
@worldofpeace
Copy link
Contributor

@worldofpeace do you also want this in 20.09?

Yep, I'd say we for sure need it.

@andir andir mentioned this pull request Oct 6, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@worldofpeace worldofpeace worldofpeace requested changes

@cole-h cole-h cole-h approved these changes

@roberth roberth roberth left review comments

@Ma27 Ma27 Ma27 left review comments

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
No open projects
20.09 Blockers
  
Done
Milestone
20.09
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@andir @roberth @Ma27 @cole-h @worldofpeace