getDefaultNixPath: actually respect {restrict,pure}-eval#7689
getDefaultNixPath: actually respect {restrict,pure}-eval#7689thufschmitt merged 2 commits intoNixOS:masterfrom
{restrict,pure}-eval#7689Conversation
thufschmitt
left a comment
thufschmitt
left a comment
There was a problem hiding this comment.
LGTM 👍, just some minor nitpicking, and some comments not directly related to this PR that can be addressed separately.
I've tested it by strace-ing the eval and confirms that Nix doesn't try to access these files anymore when it doesn't have to. Would be nice to have a test like that, but since we don't already depend on strace I don't know of an easy way to do it. So unless someone has a good simple idea we can skip this.
Previously, getDefaultNixPath was called too early: at initialisation time, before CLI and config have been processed, when `restrictEval` and `pureEval` both have their default value `false`. Call it when initialising the EvalState instead, and use `setDefault`.
ncfavier
force-pushed
the
nix-path-restrict-eval
branch
from
b60f41d to
dba9173
Compare
Isn't this test enough? The idea is that we can ensure that the Nix path is empty using |
It's not enough for #5884 in that it doesn't check that Nix doesn't try to access these paths at all. But that shouldn't prevent us from merging this. |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
3 participants
#4707 didn't do anything because getDefaultNixPath was called too early: at initialisation time, before CLI and config have been processed, when
restrictEvalandpureEvalboth have their default valuefalse. Call it when initialising the EvalState instead, and usesetDefault.Add tests for the
nix-pathoption and for--find-file .failing in restricted eval mode with no NIX_PATH.Checklist for maintainers
Maintainers: tick if completed or explain if not relevant
tests/**.shsrc/*/tests