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

Fix root channels location #8073

Merged
merged 6 commits into from
Mar 27, 2023
Merged

Conversation

thufschmitt
Copy link
Member

@thufschmitt thufschmitt commented Mar 20, 2023

Motivation

Fix for #7984. Pin the location of root's channels directory.
Note that this might break the use-cases where root doesn't have write-access
to /nix/var/nix (for instance if /nix is a read-only share). But since this
was already broken before #5226 I assume it's
not really a problem.

I'd like someone to double-check that this doesn't cause any migration issue, but I don't think so since:

  • For people running a Nix older than 2.13, this keeps the same location for the channels
  • For people running Nix 2.14, this changes the location, but
    1. The profiles are re-created from scratch each time, so the worst thing that can happen is a stray profile link under the old root's profile dir
    2. This /nix/var/nix/profiles/per-user/root/channels was really the public endpoint for the profiles, so we're just restoring it.

The implementation should be good, but I'm keeping this as a draft while I find a way to test it a bit more.

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

  • agreed on idea
  • agreed on implementation strategy
  • tests, as appropriate
    • functional tests - tests/**.sh
    • unit tests - src/*/tests
    • integration tests - tests/nixos/*
  • documentation in the manual
  • code and comments are self-explanatory
  • commit message explains why the change was made
  • new feature or incompatible change: updated release notes

Priorities

Add 👍 to pull requests you find important.

@@ -91,6 +91,7 @@ jobs:
- run: exec sh -c "nix-instantiate -E 'builtins.currentTime' --eval"
- run: exec zsh -c "nix-instantiate -E 'builtins.currentTime' --eval"
- run: exec fish -c "nix-instantiate -E 'builtins.currentTime' --eval"
- run: exec bash -c "nix-channel --update && nix-env -iA nixpkgs.hello && hello"
Copy link
Member

Choose a reason for hiding this comment

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

Does this make the test impure (i.e. fetching the latest nixpkgs from the network)?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's currently broken (because there's no channel added by the installer), but yes. The thing is that if we want to test that the installer works fine, this is an important code path to test, and I'm not sure how we can make it more pure. Is there a way to pin a channel to a specific version?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is there a way to pin a channel to a specific version?

So a 5mins investigation shows that nix-channel --add https://releases.nixos.org/nixos/unstable/nixos-23.05pre466020.60c1d71f2ba nixpkgs seems to do the trick. Is that a stable url that we can rely on? And is that an acceptable way of pinning the channels version?

Copy link
Member

@Ericson2314 Ericson2314 Mar 25, 2023

Choose a reason for hiding this comment

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

Do we we still want this since the "mock channel" test?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd rather keep this since the “mock channel” ones don't run on gha. But I'm OK with removing it if that's preferred

@cole-h
Copy link
Member

cole-h commented Mar 20, 2023

(Maybe you're still celebrating Pi Day, but there is no Nix 3.14 (or 3.13) as mentioned in your PR body, yet 😆 )

@thufschmitt
Copy link
Member Author

(Maybe you're still celebrating Pi Day, but there is no Nix 3.14 (or 3.13) as mentioned in your PR body, yet laughing )

Oops :) Thanks, fixed

@thufschmitt thufschmitt force-pushed the fix-root-channels-location branch 2 times, most recently from d7a1fe1 to ee94515 Compare March 21, 2023 14:17
@thufschmitt thufschmitt force-pushed the fix-root-channels-location branch from ee94515 to 717e81d Compare March 23, 2023 20:59
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Mar 23, 2023
@thufschmitt thufschmitt marked this pull request as ready for review March 24, 2023 12:40
These are proper documentation of the API, so they deserve to be here
@fricklerhandwerk fricklerhandwerk added the contributor-experience Developer experience for Nix contributors label Mar 27, 2023
@Ericson2314 Ericson2314 changed the title fix root channels location Fix root channels location Mar 27, 2023
@Ericson2314 Ericson2314 merged commit 1d539aa into NixOS:master Mar 27, 2023
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-03-23-nix-team-meeting-minutes-43/26758/1

@thufschmitt thufschmitt mentioned this pull request Mar 31, 2023
7 tasks
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-nix-dev-update-46/26872/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor-experience Developer experience for Nix contributors with-tests Issues related to testing. PRs with tests have some priority
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants