Restore NIX_PATH taking precedence over nix-path

[edolstra]

Motivation

This restores the Nix <= 2.13 behaviour. Environment variables (which are dynamic) should take precedence over configuration file settings (which are static).

• Fixes NIX_PATH no longer overrides nix-path #7857.
• Fixes Make the NIX_PATH environment variable take precedence over config files #8902 (duplicate?)
• Fixes nix-env --query --available does not follow NIX_PATH #8784

Note however that it does make NIX_PATH take precedence over the --nix-path and --extra-nix-path command line options, due to the way settings are implemented. There really is no correct behaviour here, e.g. should --extra-nix-path override NIX_PATH or the configuration file? If --extra-nix-path is used in restricted eval mode, should it extend the default value (which includes the channels)?

Bottom line: it's best to stop using restricted eval and/or NIX_PATH.

Context

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

[edolstra]

Alternatively, we can revert #7689, which has the benefit of simply restoring the old behaviour.

[fricklerhandwerk]

I think there is correct behavior, and this is strictly

settings < environment variables < command line options

If the current code structure doesn't allow expressing it cleanly, probably that structure should be changed. I understand that this would be quite some work for mostly a side issue, and I think that the change in this PR is a net improvement over both the current state and reverting #7689.

[fricklerhandwerk]

Is there anything speaking against

if (evalSettings.pureEval) {
  createBaseEnv();
  return;
}

and then increasing looseness in the following?

The inverted logic makes reasoning quite cumbersome.

[Ericson2314]

There really is no correct behaviour here, e.g. should --extra-nix-path override NIX_PATH or the configuration file?

I would say the order would be this:

1. Configuration file
2. NIX_PATH overrides configuration file
3. --nix-path overrides NIX_PATH
4. --extra-nix-path extends current value, whatever it may be.

If the current code structure doesn't allow expressing it cleanly, probably that structure should be changed.

Agreed. And I am inclined to again hypothesize these globals are not helping.

[grahamc]

I would say the order would be this:

1. Configuration file

2. `NIX_PATH` overrides configuration file

3. `--nix-path` overrides `NIX_PATH`

4. `--extra-nix-path` extends current value, whatever it may be.

That makes good sense to me, personally.

[edolstra]

There are some fundamentally conflicting requirements here. --restrict-eval is supposed to ignore the nix-path setting, but not NIX_PATH. (Maybe that was a mistake...) So we can't implement NIX_PATH as just another way to override nix-path. It has to have special treatment, where it takes precedence over nix-path. But since --[extra-]nix-path is a way to override/extend the nix-path setting, it has no effect if NIX_PATH is set. BTW, there really is no need to ever use --extra-nix-path since -I exists which does work as expected.

Anyway, I've made #7911 which has the simpler fix of reverting #7689. I'll merge that before the next release tomorrow.

[fricklerhandwerk]

Bottom line: it's best to stop using [...] NIX_PATH.

This, very much. NIX_PATH was clearly a mistake that opened the floodgates for impurity, and impurity is the root of all confusion.

But getting away from NIX_PATH and channels is mostly a social problem that can be helped with better documentation and lots of convincing people to spend time and migrate their code to other ways of doing things.

As long as we're not there yet, let's try make that behavior as consistent as possible.

[NIX_PATH] has to have special treatment, where it takes precedence over nix-path. But since --[extra-]nix-path is a way to override/extend the nix-path setting, it has no effect if NIX_PATH is set.

This sounds like the way settings are constructed doesn't represent the precedence @grahamc outlined. While it would be really cumbersome to rework that, it would provide both some end-user value (by naturally fixing the surprising behavior and allowing for straightforward explanation on how settings are overridden) and improve developer experience significantly (by making the code in question tangibly easier to navigate, and therefore to work on).

This sounds like something fundamentally important we should consider approaching somehow - regardless the current problem at hand, because the CLI and settings will continue developing.

[Ericson2314]

(Maybe that was a mistake...)

Yeah I think it is. IMO --restrict-eval is kinda ad-hoc (as opposed to the more principled pure eval) and I think it would be good to change this so the ordering is consistent with everything else.

[fricklerhandwerk]

Documented the desired behavior in #7772. One could merge that and treat deviating behavior as a bug to be fixed.

[nixos-discourse]

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

https://discourse.nixos.org/t/tweag-nix-dev-update-45/26397/1

[nixos-discourse]

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

https://discourse.nixos.org/t/2023-03-31-nix-team-meeting-minutes-45/27002/1

[fricklerhandwerk]

Discussed in the Nix team meeting:

• @edolstra: the configuration system is currently built in a way where the proposed behavior cannot really be expressed. options:
  • we could put a lot of effort into this
  • ignore the problem
  • deprecate restrict-eval
    • Hydra uses it, can't just rip it out
• @Ericson2314: in favor of deprecating restrict-eval as long as Hydra can get this configuration somehow
  • may need a proper policy for deprecation an breaking changes, e.g. as proposed by @infinisil
• @roberth: Hydra doesn't use the CLI. We could deprecate the CLI but keep the mechanism internally.
• @edolstra: currently working on filesystem accessor from lazy trees; could use that to implement a policy in terms of a filter according to Hydra's needs.
• @thufschmitt: can add a warning with a prompt to complain in a GitHub issue to figure out how much the feature is used
• @fricklerhandwerk: originally the underlying issue was that restrict-eval was is not respected. the PR fixing that exposed an inconsistency in the CLI parameter handling
  • @edolstra: the problem is that there are a few ad-hoc legacy mechanisms interacting in weird ways, especially $NIX_PATH.
• @thufschmitt: restrict-eval not working exactly is less of a problem than NIX_PATH not working, we can postpone it
• @fricklerhandwerk will open an issue for adding a deprecation warning to restrict-eval

[nixos-discourse]

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

https://discourse.nixos.org/t/2023-05-05-nix-team-meeting-minutes-52/27893/1

[roberth]

I'm having trouble with this sometimes because I use nix-path = (empty) through NixOS via nix.channel.enable = false;. Anyone who opts into that won't be able to use NIX_PATH temporarily when they need it for whatever mediocre reason (such as a nix-shell that makes <nixpkgs> more hermetic).

I think we reached a decision. Is this blocked on anything?

[katandtonic]

Please fix this!

[fricklerhandwerk]

Superseded by #11079