fix NIX_PATH for real (#11079)

* fix NIX_PATH overriding

- test restricted evaluation
- test precedence for setting the search path

Co-authored-by: Robert Hensing <robert@roberthensing.nl>
Co-authored-by: John Ericson <git@JohnEricson.me>

src/libexpr/eval-gc.cc

@@ -1,5 +1,7 @@
 #include "error.hh"
 #include "environment-variables.hh"
+#include "eval-settings.hh"
+#include "config-global.hh"
 #include "serialise.hh"
 #include "eval-gc.hh"
 
@@ -230,6 +232,12 @@ void initGC()
     gcCyclesAfterInit = GC_get_gc_no();
 #endif
 
+    // NIX_PATH must override the regular setting
+    // See the comment in applyConfig
+    if (auto nixPathEnv = getEnv("NIX_PATH")) {
+        globalConfig.set("nix-path", concatStringsSep(" ", EvalSettings::parseNixPath(nixPathEnv.value())));
+    }
+
     gcInitialised = true;
 }
 

src/libexpr/eval-settings.cc

@@ -8,7 +8,7 @@ namespace nix {
 
 /* Very hacky way to parse $NIX_PATH, which is colon-separated, but
    can contain URLs (e.g. "nixpkgs=https://bla...:foo=https://"). */
-static Strings parseNixPath(const std::string & s)
+Strings EvalSettings::parseNixPath(const std::string & s)
 {
     Strings res;
 
@@ -48,10 +48,7 @@ EvalSettings::EvalSettings(bool & readOnlyMode, EvalSettings::LookupPathHooks lo
     : readOnlyMode{readOnlyMode}
     , lookupPathHooks{lookupPathHooks}
 {
-    auto var = getEnv("NIX_PATH");
+    auto var = getEnv("NIX_ABORT_ON_WARN");
-    if (var) nixPath = parseNixPath(*var);
-
-    var = getEnv("NIX_ABORT_ON_WARN");
     if (var && (var == "1" || var == "yes" || var == "true"))
         builtinsAbortOnWarn = true;
 }

src/libexpr/eval-settings.hh

@@ -47,6 +47,8 @@ struct EvalSettings : Config
 
     static bool isPseudoUrl(std::string_view s);
 
+    static Strings parseNixPath(const std::string & s);
+
     static std::string resolvePseudoUrl(std::string_view url);
 
     LookupPathHooks lookupPathHooks;
@@ -71,7 +73,7 @@ struct EvalSettings : Config
     )"};
 
     Setting<Strings> nixPath{
-        this, getDefaultNixPath(), "nix-path",
+        this, {}, "nix-path",
         R"(
           List of search paths to use for [lookup path](@docroot@/language/constructs/lookup-path.md) resolution.
           This setting determines the value of

src/libexpr/eval.cc

@@ -215,7 +215,7 @@ static Symbol getName(const AttrName & name, EvalState & state, Env & env)
 static constexpr size_t BASE_ENV_SIZE = 128;
 
 EvalState::EvalState(
-    const LookupPath & _lookupPath,
+    const LookupPath & lookupPathFromArguments,
     ref<Store> store,
     const fetchers::Settings & fetchSettings,
     const EvalSettings & settings,
@@ -331,13 +331,22 @@ EvalState::EvalState(
     vStringSymlink.mkString("symlink");
     vStringUnknown.mkString("unknown");
 
-    /* Initialise the Nix expression search path. */
+    /* Construct the Nix expression search path. */
+    assert(lookupPath.elements.empty());
     if (!settings.pureEval) {
-        for (auto & i : _lookupPath.elements)
+        for (auto & i : lookupPathFromArguments.elements) {
             lookupPath.elements.emplace_back(LookupPath::Elem {i});
-        for (auto & i : settings.nixPath.get())
+        }
+        /* $NIX_PATH overriding regular settings is implemented as a hack in `initGC()` */
+        for (auto & i : settings.nixPath.get()) {
             lookupPath.elements.emplace_back(LookupPath::Elem::parse(i));
         }
+        if (!settings.restrictEval) {
+            for (auto & i : EvalSettings::getDefaultNixPath()) {
+                lookupPath.elements.emplace_back(LookupPath::Elem::parse(i));
+            }
+        }
+    }
 
     /* Allow access to all paths in the search path. */
     if (rootFS.dynamic_pointer_cast<AllowListSourceAccessor>())

src/libutil/config.cc

@@ -1,6 +1,7 @@
 #include "config.hh"
 #include "args.hh"
 #include "abstract-setting-to-json.hh"
+#include "environment-variables.hh"
 #include "experimental-features.hh"
 #include "util.hh"
 #include "file-system.hh"
@@ -170,10 +171,19 @@ void AbstractConfig::applyConfig(const std::string & contents, const std::string
             set(name, value);
 
     // Then apply other settings
-    for (const auto & [name, value] : parsedContents)
+    // XXX: NIX_PATH must override the regular setting! This is done in `initGC()`
-        if (name != "experimental-features" && name != "extra-experimental-features")
+    // Environment variables overriding settings should probably be part of the Config mechanism,
+    // but at the time of writing it's not worth building that for just one thing
+    for (const auto & [name, value] : parsedContents) {
+        if (name != "experimental-features" && name != "extra-experimental-features") {
+            if ((name == "nix-path" || name == "extra-nix-path")
+                && getEnv("NIX_PATH").has_value()) {
+                continue;
+            }
             set(name, value);
         }
+    }
+}
 
 void Config::resetOverridden()
 {

tests/functional/nix_path.sh

@@ -14,3 +14,73 @@ nix-instantiate --eval -E '<by-relative-path/simple.nix>' --restrict-eval
 
 [[ $(nix-instantiate --find-file by-absolute-path/simple.nix) = $PWD/simple.nix ]]
 [[ $(nix-instantiate --find-file by-relative-path/simple.nix) = $PWD/simple.nix ]]
+
+# this is the human-readable specification for the following test cases of interactions between various ways of specifying NIX_PATH.
+# TODO: the actual tests are incomplete and too manual.
+# there should be 43 of them, since the table has 9 rows and columns, and 2 interactions are meaningless
+# ideally they would work off the table programmatically.
+#
+# | precedence             | hard-coded | nix-path in file | extra-nix-path in file | nix-path in env | extra-nix-path in env | NIX_PATH  | nix-path  | extra-nix-path  | -I              |
+# |------------------------|------------|------------------|------------------------|-----------------|-----------------------|-----------|-----------|-----------------|-----------------|
+# | hard-coded             | x          | ^override        | ^append                | ^override       | ^append               | ^override | ^override | ^append         | ^append         |
+# | nix-path in file       |            | last wins        | ^append                | ^override       | ^append               | ^override | ^override | ^append         | ^append         |
+# | extra-nix-path in file |            |                  | append in order        | ^override       | ^append               | ^override | ^override | ^append         | ^append         |
+# | nix-path in env        |            |                  |                        | last wins       | ^append               | ^override | ^override | ^append         | ^append         |
+# | extra-nix-path in env  |            |                  |                        |                 | append in order       | ^override | ^override | ^append         | ^append         |
+# | NIX_PATH               |            |                  |                        |                 |                       | x         | ^override | ^append         | ^append         |
+# | nix-path               |            |                  |                        |                 |                       |           | last wins | ^append         | ^append         |
+# | extra-nix-path         |            |                  |                        |                 |                       |           |           | append in order | append in order |
+# | -I                     |            |                  |                        |                 |                       |           |           |                 | append in order |
+
+unset NIX_PATH
+
+mkdir -p $TEST_ROOT/{from-nix-path-file,from-NIX_PATH,from-nix-path,from-extra-nix-path,from-I}
+for i in from-nix-path-file from-NIX_PATH from-nix-path from-extra-nix-path from-I; do
+    touch $TEST_ROOT/$i/only-$i.nix
+done
+
+# finding something that's not in any of the default paths fails
+( ! $(nix-instantiate --find-file test) )
+
+echo "nix-path = test=$TEST_ROOT/from-nix-path-file" >> "$test_nix_conf"
+
+# Use nix.conf in absence of NIX_PATH
+[[ $(nix-instantiate --find-file test) = $TEST_ROOT/from-nix-path-file ]]
+
+# NIX_PATH overrides nix.conf
+[[ $(NIX_PATH=test=$TEST_ROOT/from-NIX_PATH nix-instantiate --find-file test) = $TEST_ROOT/from-NIX_PATH ]]
+# if NIX_PATH does not have the desired entry, it fails
+(! NIX_PATH=test=$TEST_ROOT nix-instantiate --find-file test/only-from-nix-path-file.nix)
+
+# -I extends nix.conf
+[[ $(nix-instantiate -I test=$TEST_ROOT/from-I --find-file test/only-from-I.nix) = $TEST_ROOT/from-I/only-from-I.nix ]]
+# if -I does not have the desired entry, the value from nix.conf is used
+[[ $(nix-instantiate -I test=$TEST_ROOT/from-I --find-file test/only-from-nix-path-file.nix) = $TEST_ROOT/from-nix-path-file/only-from-nix-path-file.nix ]]
+
+# -I extends NIX_PATH
+[[ $(NIX_PATH=test=$TEST_ROOT/from-NIX_PATH nix-instantiate -I test=$TEST_ROOT/from-I --find-file test/only-from-I.nix) = $TEST_ROOT/from-I/only-from-I.nix ]]
+# if -I does not have the desired entry, the value from NIX_PATH is used
+[[ $(NIX_PATH=test=$TEST_ROOT/from-NIX_PATH nix-instantiate -I test=$TEST_ROOT/from-I --find-file test/only-from-NIX_PATH.nix) = $TEST_ROOT/from-NIX_PATH/only-from-NIX_PATH.nix ]]
+
+# --extra-nix-path extends NIX_PATH
+[[ $(NIX_PATH=test=$TEST_ROOT/from-NIX_PATH nix-instantiate --extra-nix-path test=$TEST_ROOT/from-extra-nix-path --find-file test/only-from-extra-nix-path.nix) = $TEST_ROOT/from-extra-nix-path/only-from-extra-nix-path.nix ]]
+# if --extra-nix-path does not have the desired entry, the value from NIX_PATH is used
+[[ $(NIX_PATH=test=$TEST_ROOT/from-NIX_PATH nix-instantiate --extra-nix-path test=$TEST_ROOT/from-extra-nix-path --find-file test/only-from-NIX_PATH.nix) = $TEST_ROOT/from-NIX_PATH/only-from-NIX_PATH.nix ]]
+
+# --nix-path overrides NIX_PATH
+[[ $(NIX_PATH=test=$TEST_ROOT/from-NIX_PATH nix-instantiate --nix-path test=$TEST_ROOT/from-nix-path --find-file test) = $TEST_ROOT/from-nix-path ]]
+# if --nix-path does not have the desired entry, it fails
+(! NIX_PATH=test=$TEST_ROOT/from-NIX_PATH nix-instantiate --nix-path test=$TEST_ROOT/from-nix-path --find-file test/only-from-NIX_PATH.nix)
+
+# --nix-path overrides nix.conf
+[[ $(nix-instantiate --nix-path test=$TEST_ROOT/from-nix-path --find-file test) = $TEST_ROOT/from-nix-path ]]
+(! nix-instantiate --nix-path test=$TEST_ROOT/from-nix-path --find-file test/only-from-nix-path-file.nix)
+
+# --extra-nix-path extends nix.conf
+[[ $(nix-instantiate --extra-nix-path test=$TEST_ROOT/from-extra-nix-path --find-file test/only-from-extra-nix-path.nix) = $TEST_ROOT/from-extra-nix-path/only-from-extra-nix-path.nix ]]
+# if --extra-nix-path does not have the desired entry, it is taken from nix.conf
+[[ $(nix-instantiate --extra-nix-path test=$TEST_ROOT/from-extra-nix-path --find-file test) = $TEST_ROOT/from-nix-path-file ]]
+
+# -I extends --nix-path
+[[ $(nix-instantiate --nix-path test=$TEST_ROOT/from-nix-path -I test=$TEST_ROOT/from-I --find-file test/only-from-I.nix) = $TEST_ROOT/from-I/only-from-I.nix ]]
+[[ $(nix-instantiate --nix-path test=$TEST_ROOT/from-nix-path -I test=$TEST_ROOT/from-I --find-file test/only-from-nix-path.nix) = $TEST_ROOT/from-nix-path/only-from-nix-path.nix ]]

tests/functional/restricted.sh

@@ -10,6 +10,9 @@ nix-instantiate --restrict-eval --eval -E '1 + 2'
 nix-instantiate --restrict-eval ./simple.nix -I src=.
 nix-instantiate --restrict-eval ./simple.nix -I src1=simple.nix -I src2=config.nix -I src3=./simple.builder.sh
 
+# no default NIX_PATH
+(unset NIX_PATH; ! nix-instantiate --restrict-eval --find-file .)
+
 (! nix-instantiate --restrict-eval --eval -E 'builtins.readFile ./simple.nix')
 nix-instantiate --restrict-eval --eval -E 'builtins.readFile ./simple.nix' -I src=../..