Make the NIX_PATH environment variable take precedence over config files

[colonelpanic8]

Fixes #8890 #8784

Motivation

Almost every program that I can think of takes configuration parameters in this order of precedence:

1. Command line arguments
2. Environment variables
3. Configuration files

This ordering is based roughly on how dynamic/persistent each of these types of values are, which gives the user a lot of flexibility to override things in the way they see fit. It also feels like the most obvious and intuitive order. Chat GPT agrees with me that this is the standard and most intuitive way to do things:

https://chat.openai.com/share/68fb24c9-2b91-47bd-9fab-baa956702cca

There is some discussion here that seems to imply that maybe this is a deliberate behavior:

NixOS/nixpkgs#242098 (comment)

but this position doesn't make a ton of sense to me, given that, as I pointed out here:

NixOS/nixpkgs#242098 (comment)

users must opt in to impure behavior with the --impure flag for flake aware commands.

There is one open question in my mind, which is whether or not a NIX_PATH setting should actually clobber any settings from configuration files, or if the configuration file values should also be used.

As the code is currently written, the values in the configuration files will still take effect.

This is actually arguably inconsistent with what is written here:

nix/doc/manual/src/command-ref/env-common.md

Line 22 in 197afd0

If `NIX_PATH` is set to an empty string, resolving search paths will always fail.

The thing is, the current behavior is ALSO inconsistent with what is written there. If you set NIX_PATH="" but have values in your nix.conf files, those paths will still be searched (as things stand atm).

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
• documentation in the internal API docs
• 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.

[colonelpanic8]

@roberth Is there any chance this could be looked at/merged soon.

Do you have any thoughts about:

There is one open question in my mind, which is whether or not a NIX_PATH setting should actually clobber any settings from configuration files, or if the configuration file values should also be used.

[roberth]

Is there any chance this could be looked at/merged soon.

The team will triage this on Friday, and I hope we can decide soon.

whether or not a NIX_PATH setting should actually clobber any settings from configuration files

In a project that uses shell.nix to purify the environment, I'd expect the nix-path from config files to be ignored completely.

[nixos-discourse]

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

https://discourse.nixos.org/t/2023-09-15-nix-team-meeting-minutes-86/33171/1

[fricklerhandwerk]

Discussed in Nix team meeting:

The interaction between NIX_PATH and restrict-eval is a bit of a mess. @thufschmitt and me plan to sit down and figure out how to deal with the problem.

Complete discussion

• @edolstra: Attempted to fix earlier, had to revert

• @roberth: What about Restore NIX_PATH taking precedence over nix-path #7871

• @edolstra: Problem: Makes NIX_PATH take precedence over --nix-path, which isn't great

  • The current infra doesn't really allow having config files -> env var -> cli flag
  • @fricklerhandwerk: had getDefaultNixPath: actually respect {restrict,pure}-eval #7689 in the past, which touched the the application order for NIX_PATH, but got reverted because it did not behave as expected

• Could we treat it exactly as if we had NIX_CONFIG = nix-path = blah?

  • But doesn't play well with restricted-eval which doesn't handl NIX_CONFIG but only NIX_PATH

• @infinisil: Ask the contributor to satisfy the requirements instead of figuring it out for them?

• @roberth: requirements not 100% clear but can be adapted

  • @edolstra: Restore NIX_PATH taking precedence over nix-path #7871 has test cases, could be reused

• @fricklerhandwerk: possible solutions

  • Cheap: only document that extra-nix-path is treated as a config level thing
  • More involved: Add environment variables to the settings system

• @infinisil: A separate function for getting the nix path for when restrict eval is enabled?

  • @fricklerhandwerk: That would require direct access to the config file in the settings system?
  • @roberth: Setting<pair<vector,vector>>
  • @regnat: complexity might leak everywhere
    • @roberth: Having two structures makes sense: the intermediate state and the effective config
    • @fricklerhandwerk: Or expose the unmerged env and config

• We had discussed earlier that we could drop restrict-eval and let Hydra use it through C++

• @regnat: Warn when both restrict and NIX_PATH not empty

  • @fricklerhandwerk: or stop with error, wouldn't break assumptions
  • @edolstra: another possibility is not to touch the existing CLI and say it's legacy behavior
  • @fricklerhandwerk: not supporting the combination is more explicit, as we currently ignore NIX_PATH silently

• @regnat: too much detail for the meeting

Assigned to @fricklerhandwerk and @regnat to figure out.

[nixos-discourse]

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

https://discourse.nixos.org/t/2023-09-18-nix-team-meeting-minutes-87/33194/1

[colonelpanic8]

@fricklerhandwerk glad to hear that someone is actively thinking about it.

Can't quite say I understand what is wrong with the approach in the PR, but sounds like it might have something to do with restrict-eval?

[fricklerhandwerk]

@colonelpanic8 the comment with the mess was not referring to your work but the general situation. In #9066 we outlined what Nix should do. Someone still has to spend time on actually writing out those test cases and then making them pass. Would you be willing to work on some of that?

After looking at the various attempts at solving the problem for so long, I'd really not want to merge anything that doesn't test it through.

[colonelpanic8]

@fricklerhandwerk I'm not quite sure I understand how what I did doesn't do the right thing according to the comments here. I did add a small amount of testing but also happy to do more.

[fricklerhandwerk]

Arrgh the worst typo, I'm so sorry. It was not referring to your work.

[nixos-discourse]

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

https://discourse.nixos.org/t/nix-path-is-not-recognized/38404/6

[fricklerhandwerk]

Superceded by #11079

Thanks @colonelpanic8 for the preliminary work!