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

tree-wide: Fix with nixUnstable #80724

Closed
wants to merge 4 commits into from
Closed

Conversation

@jtojnar
Copy link
Contributor

jtojnar commented Feb 21, 2020

Motivation for this change

It is great that the experimental commands are now gated but since we are dogfooding them, we need to open the gates.

I ran nixos-rebuild switch with the following in my config:

Things done
{
  nixpkgs.overlays = [
    (self: super: {
      nix = super.nixUnstable;
    })
  ];

  nix.package = pkgs.nixUnstable;
}
  • Verified that nixos-option builds
  • Verified that system is activated correctly
  • Did not check nixos-install
jtojnar added 4 commits Feb 21, 2020
nix::baseNameOf returns std::string_view instead of std::string in unstable Nix,
which cannot be autocast to `const char*` expected by `nix::LegacyArgs::LegacyArgs` constructor.
Otherwise it fails with nix = nixUnstable overlay. It will also make it easier to find it when we decide to change the experimental command's syntax.
Otherwise the nix activation script fails with nix = nixUnstable overlay. It will also make it easier to find it when we decide to change the experimental command's syntax.
Otherwise the command will likely fail with nix.package = nixUnstable set. It will also make it easier to find it when we decide to change the experimental command's syntax.
@jtojnar jtojnar requested a review from edolstra Feb 21, 2020
@jtojnar jtojnar changed the title Experimental nix tree-wide: Fix with nixUnstable Feb 21, 2020
@@ -27,6 +27,7 @@ profile=/nix/var/nix/profiles/system
buildHost=
targetHost=
maybeSudo=()
NIX="nix --option experimental-features nix-command"

This comment has been minimized.

Copy link
@edolstra

edolstra Feb 21, 2020

Member

We can't do this, because passing --experimental-features overrides the setting in nix.conf. So it might disable other features.

This comment has been minimized.

Copy link
@jtojnar

jtojnar Feb 21, 2020

Author Contributor

That is really unfortunate, it means we cannot dogfood nix commands.

@@ -452,7 +452,7 @@ in
system.activationScripts.nix = stringAfter [ "etc" "users" ]
''
# Create directories in /nix.
${nix}/bin/nix ping-store --no-net
${nix}/bin/nix --option experimental-features nix-command ping-store --no-net

This comment has been minimized.

Copy link
@edolstra

edolstra Feb 21, 2020

Member

Probably we should replace nix ping-store by some other dummy command that opens the store. I think even something like nix-store -q without arguments will do this.

@@ -281,7 +282,7 @@ fi

# Resolve the flake.
if [[ -n $flake ]]; then
flake=$(nix flake info --json "${extraBuildFlags[@]}" "${lockFlags[@]}" -- "$flake" | jq -r .url)
flake=$($NIX flake info --json "${extraBuildFlags[@]}" "${lockFlags[@]}" -- "$flake" | jq -r .url)

This comment has been minimized.

Copy link
@edolstra

edolstra Feb 21, 2020

Member

This also requires the flakes experimental feature. In any case, I think the user should affirmatively enable this in nix.conf themselves. Otherwise it kind of defeats the point of experimental-features, which is to make people aware of the fact that they're using an experimental feature.

@@ -102,6 +102,7 @@ if [[ -z $system ]]; then
outLink="$tmpdir/system"
nix build --out-link "$outLink" --store "$mountPoint" "${extraBuildFlags[@]}" \
--extra-substituters "$sub" \
--option experimental-features nix-command \

This comment has been minimized.

Copy link
@edolstra

edolstra Feb 21, 2020

Member

We could just change this back to nix-build (making nixos-install consistent with nixos-rebuild).

This comment has been minimized.

Copy link
@jtojnar

jtojnar Feb 21, 2020

Author Contributor

I can do that but I thought we wanted to dogfood the stuff.

This comment has been minimized.

Copy link
@edolstra

edolstra Feb 21, 2020

Member

Dogfooding == using it yourself, not making other users use it :-)

This comment has been minimized.

Copy link
@jtojnar

jtojnar Feb 21, 2020

Author Contributor

Do we revert e88f289?

This comment has been minimized.

Copy link
@edolstra

edolstra Feb 21, 2020

Member

No, I think we only have to change nix build to nix-build. IIRC it also supports the --store argument which is what allows an install into a chroot.


struct MyArgs : nix::LegacyArgs, nix::MixEvalArgs
{
using nix::LegacyArgs::LegacyArgs;
};

MyArgs myArgs(nix::baseNameOf(argv[0]), [&](Strings::iterator & arg, const Strings::iterator & end) {
MyArgs myArgs(pname.data(), [&](Strings::iterator & arg, const Strings::iterator & end) {

This comment has been minimized.

Copy link
@jtojnar

jtojnar Feb 21, 2020

Author Contributor

Perhaps making nix::LegacyArgs::LegacyArgs accept std::string_view is a cleaner idea.

@jtojnar jtojnar mentioned this pull request Feb 22, 2020
2 of 2 tasks complete
@jtojnar
Copy link
Contributor Author

jtojnar commented May 11, 2020

Mostly fixed by #87182

@jtojnar jtojnar closed this May 11, 2020
@jtojnar jtojnar deleted the jtojnar:experimental-nix branch May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.