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

nix: don't pass --experimental-features to nix commands #1311

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gcurtis
Copy link
Collaborator

@gcurtis gcurtis commented Jul 25, 2023

When running nix commands, devbox passes two flags to enable experimental features:

  1. nix --extra-experimental-features ...
  2. nix --option experimental-features ...

The second flag causes issues because it overwrites the user's nix.conf. It doesn't seem to be doing anything, so remove it and just use --extra-experimental-features. There's also some cleanup to switch all callers of nix.ExperimentalFlags to use nix.Command instead. This function sets the correct flags and forwards SIGINTs to Nix instead of just killing the process when devbox gets interrupted.

Details

A specific bug caused by this flag is #1263. The DeterminateSystems installer configures Nix without a build users group and instead enables the auto-allocate-uids setting in /etc/nix/nix.conf on Linux. If this setting gets disabled by devbox, then the Nix daemon has no way of building flakes and fails.

The reason why --option experimental-features ... was added is because we were still seeing errors about ca-derivations not being enabled even though the --extra-experimental-features ca-derivations flag was set. Overwriting the config setting seemed to fix it, but that might've been a red herring.

The ca-derivations feature is unique in that it must be set in the Nix daemon, not just the client. This is because it changes the schema of the Nix database, requiring a restart of the daemon. The only reliable way to enable it is to add it to /etc/nix/nix.conf and restart the nix-daemon process.

Given that this flag is currently breaking some users and doesn't seem to be having the intended effect, I think we should remove it and figure out another way to ensure ca-derivations is enabled. Switching to using the DeterminateSystems installer and printing errors telling the user how to enable it are probably the best bet.

@gcurtis gcurtis requested review from savil and ipince July 25, 2023 22:11
@savil
Copy link
Collaborator

savil commented Jul 26, 2023

Switching to using the DeterminateSystems installer and printing errors telling the user how to enable it are probably the best bet.

yup, lets do this!

@savil
Copy link
Collaborator

savil commented Jul 26, 2023

The code looks good but there's quite a few failing cicd tests. Related to:

The reason why --option experimental-features ... was added is because we were still seeing errors about ca-derivations not being enabled even though the --extra-experimental-features ca-derivations flag was set. Overwriting the config setting seemed to fix it, but that might've been a red herring.

I tried looking into "why" but am failing.

When running nix commands, devbox passes two flags to enable
experimental features:

1. `nix --extra-experimental-features ...`
2. `nix --option experimental-features ...`

The second flag causes issues because it overwrites the user's nix.conf.
It doesn't seem to be doing anything, so remove it and just use
`--extra-experimental-features`. There's also some cleanup to switch
all callers of `nix.ExperimentalFlags` to use `nix.Command` instead.
This function sets the correct flags and forwards SIGINTs to Nix
instead of just killing the process when devbox gets interrupted.

Details
-------

A specific bug caused by this flag is
#1263. The DeterminateSystems
installer configures Nix without a build users group and instead
enables the `auto-allocate-uids` setting in `/etc/nix/nix.conf` on
Linux. If this setting gets disabled by devbox, then the Nix daemon has
no way of building flakes and fails.

The reason why `--option experimental-features ...` was added is because
we were still seeing errors about ca-derivations not being enabled
even though the `--extra-experimental-features ca-derivations` flag was
set. Overwriting the config setting seemed to fix it, but that might've
been a red herring.

The ca-derivations feature is unique in that it must be set in the Nix
daemon, not just the client. This is because it changes the schema of
the Nix database, requiring a restart of the daemon. The only reliable
way to enable it is to add it to `/etc/nix/nix.conf`.

Given that this flag is currently breaking some users and doesn't seem
to be having the intended effect, I think we should remove it and
figure out another way to ensure ca-derivations is enabled. Switching
to using the DeterminateSystems installer and printing errors telling
the user how to enable it are probably the best bet.
@gcurtis gcurtis force-pushed the gcurtis/exp-features branch 4 times, most recently from e75be49 to b5f7722 Compare July 26, 2023 19:09
@gcurtis
Copy link
Collaborator Author

gcurtis commented Jul 26, 2023

The macOS auto-install test is still failing, so I'm going to postpone this until next cycle. I think this PR will require a couple other changes:

  • Make devbox check that experimental-features = ca-derivations ... is set in nix show-config.
  • If it isn't, add it (will require sudo) and restart the daemon (if it's a multi-user install).

I'm really not sure why -o experimental-features ... made things work. I'm wondering if it's a bug in the CLI. Either way, we'll need to remove it so we don't break with Nix Linux installs.

@gcurtis gcurtis marked this pull request as draft July 26, 2023 20:05
@gcurtis gcurtis removed request for savil and ipince July 26, 2023 20:06
gcurtis added a commit that referenced this pull request Jul 26, 2023
Changes extracted from #1311 so they can be merged sooner.

- Upgrade the Nix installer action to v4 and configure the experimental
  features Devbox needs.
- Print out the Nix version, `/etc/nix/nix.conf`, and the resolved Nix
  config to make debugging easier.
- Do the same for `test-nix-versions` and add the last 3 Nix releases to
  the matrix. Nix 2.15.1 is the current stable version and 2.17.0 is the
  latest.
- Update `TestContentAddressedPath` to accept hashes from
  Nix <2.17 and 2.17+.
gcurtis added a commit that referenced this pull request Jul 26, 2023
Changes extracted from #1311 so they can be merged sooner.

- Upgrade the Nix installer action to v4 and configure the experimental
features Devbox needs.
- Print out the Nix version, `/etc/nix/nix.conf`, and the resolved Nix
config to make debugging easier.
- Do the same for `test-nix-versions` and add the last 3 Nix releases to
the matrix. Nix 2.15.1 is the current stable version and 2.17.0 is the
latest.
- Update `TestContentAddressedPath` to accept hashes from Nix <2.17 and
2.17+.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants