nix-channel: do not set empty nix-path when disabling channels #323613
Conversation
github-actions
bot
added
6.topic: nixos
ofborg
bot
added
the
2.status: merge conflict
CyberShadow
force-pushed
the
fix-nix-path-without-channels-v2
branch
from
db18f9c
to
55d50b6
Compare
ofborg
bot
added
10.rebuild-darwin: 1-10
10.rebuild-linux: 1-10
and removed
2.status: merge conflict
|
@fricklerhandwerk and I are making progress on a fix in Nix proper, and while I wish we could do just that, not this, and keep it simple, realistically NixOS has to deal with the buggy behavior for some time to come. All that context aside, one small question.
Do I understand correctly that |
nixos/modules/config/nix-channel.nix
Outdated
| echo "Due to https://github.com/NixOS/nix/issues/9574, Nix may still use these channels when NIX_PATH is unset." 1>&2 | ||
| echo "Delete the above directory to prevent this." 1>&2 | ||
| fi | ||
| ''); |
There was a problem hiding this comment.
Shouldn't we also check for user channels?
There was a problem hiding this comment.
Yes, good idea. Even though we can't enumerate all possible values of HOME that Nix could be invoked with, we could at least check the home directories in the user catalog. Added.
There was a problem hiding this comment.
That easily catches 99% of users in my estimate, and that's a big win. 👍
Yes.
The warnings are printed regardless of whether The circumstance that I ran into where Nix was using an unintended default was running things with
Agreed! In my case, I expected the script to fail with |
An empty nix-path in nix.conf will disable NIX_PATH environment variable entirely, which is not necessarily implied by users who want to disable nix channels. NIX_PATH also has some usages in tools like nixos-rebuild or just as user aliases. That change is surprising and debatable, and also caused breakages in nixpkgs-review and user configs. See: - https://github.com/NixOS/nixpkgs/pull/242098/files#r1269891427 - Mic92/nixpkgs-review#343 - NixOS/nix#10998 Co-authored-by: oxalica <[email protected]>
CyberShadow
force-pushed
the
fix-nix-path-without-channels-v2
branch
from
55d50b6
to
1e6acab
Compare
We may want to clear NIX_PATH when channels are disabled, or maybe it has to be a separate option. This is just very frustrating to me.
There was a problem hiding this comment.
I've adjusted the warnings a bit (plus added the infra to make it nice; sorry for the scope creep), and I've done some testing, with the NixOS test as well as manually with a VM.
I think we may have an issue, which is that since recently NixOS sets a NIX_PATH=nixpkgs=flake:nixpkgs anyway, which I think is really bad, but that's a separate issue.
This PR is good now! 🚀
roberth
force-pushed
the
fix-nix-path-without-channels-v2
branch
from
d326855
to
d376ea6
Compare
roberth
force-pushed
the
fix-nix-path-without-channels-v2
branch
from
d376ea6
to
2d9a686
Compare
Apologies, I've only just now realized that you meant the warnings printed by Nix, not the ones printed by the activation script added here. And... I can no longer recall how I got Nix to print those warnings. 🤦 |
(cherry picked from commit 46df92b)
|
Successfully created backport PR for |
Description of changes
Continuation of #273170.
An empty nix-path in nix.conf will disable NIX_PATH environment variable entirely, which is not necessarily implied by users who want to disable nix channels. NIX_PATH also has some usages in tools like nixos-rebuild or just as user aliases.
That change is surprising and debatable, and also caused breakages in nixpkgs-review and user configs.
See:
os.environ["NIX_PATH"] = self.nixpkgs_path()silently get ignored whennix-pathis set innix.confMic92/nixpkgs-review#343Changes since #273170: implements @roberth's suggestion to warn when channel data still exists, as Nix (in the absence of a
NIX_PATH) will still use it.In this circumstance, Nix will now complain:
It is a little annoying, but also in a way reassuring.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.