nix-shell: look up shell.nix when argument is a directory (#11057)

* Refactor: rename runEnv -> isNixShell

* Refactor: rename left -> remainingArgs

* nix-build.cc: Refactor: extract baseDir variable

* nix-build.cc: Refactor: extract sourcePath, resolvedPath variables

* nix-shell: Look for shell.nix when directory is specified

* Add legacy setting: nix-shell-always-looks-for-shell-nix

* rl-next: Add note about shell.nix lookups

* tests/functional/shell.nix: Implement runHook for dummy stdenv

src/libcmd/common-eval-args.cc

@@ -11,6 +11,8 @@
 #include "command.hh"
 #include "tarball.hh"
 #include "fetch-to-store.hh"
+#include "compatibility-settings.hh"
+#include "eval-settings.hh"
 
 namespace nix {
 
@@ -33,6 +35,11 @@ EvalSettings evalSettings {
 
 static GlobalConfig::Register rEvalSettings(&evalSettings);
 
+CompatibilitySettings compatibilitySettings {};
+
+static GlobalConfig::Register rCompatibilitySettings(&compatibilitySettings);
+
+
 MixEvalArgs::MixEvalArgs()
 {
     addFlag({

src/libcmd/common-eval-args.hh

@@ -13,6 +13,7 @@ namespace nix {
 class Store;
 class EvalState;
 struct EvalSettings;
+struct CompatibilitySettings;
 class Bindings;
 struct SourcePath;
 
@@ -21,6 +22,11 @@ struct SourcePath;
  */
 extern EvalSettings evalSettings;
 
+/**
+ * Settings that control behaviors that have changed since Nix 2.3.
+ */
+extern CompatibilitySettings compatibilitySettings;
+
 struct MixEvalArgs : virtual Args, virtual MixRepair
 {
     static constexpr auto category = "Common evaluation options";

src/libcmd/compatibility-settings.hh

@@ -0,0 +1,19 @@
+#pragma once
+#include "config.hh"
+
+namespace nix {
+struct CompatibilitySettings : public Config
+{
+
+    CompatibilitySettings() = default;
+
+    Setting<bool> nixShellAlwaysLooksForShellNix{this, true, "nix-shell-always-looks-for-shell-nix", R"(
+        Before Nix 2.24, [`nix-shell`](@docroot@/command-ref/nix-shell.md) would only look at `shell.nix` if it was in the working directory - when no file was specified.
+
+        Since Nix 2.24, `nix-shell` always looks for a `shell.nix`, whether that's in the working directory, or in a directory that was passed as an argument.
+
+        You may set this to `false` to revert to the Nix 2.3 behavior.
+    )"};
+};
+
+};

src/libcmd/meson.build

@@ -97,6 +97,7 @@ headers = [config_h] + files(
   'command-installable-value.hh',
   'command.hh',
   'common-eval-args.hh',
+  'compatibility-settings.hh',
   'editor-for.hh',
   'installable-attr-path.hh',
   'installable-derived-path.hh',

src/libexpr/eval.cc

@@ -2650,7 +2650,7 @@ void EvalState::printStatistics()
 }
 
 
-SourcePath resolveExprPath(SourcePath path)
+SourcePath resolveExprPath(SourcePath path, bool addDefaultNix)
 {
     unsigned int followCount = 0, maxFollow = 1024;
 
@@ -2666,7 +2666,7 @@ SourcePath resolveExprPath(SourcePath path)
     }
 
     /* If `path' refers to a directory, append `/default.nix'. */
-    if (path.resolveSymlinks().lstat().type == SourceAccessor::tDirectory)
+    if (addDefaultNix && path.resolveSymlinks().lstat().type == SourceAccessor::tDirectory)
         return path / "default.nix";
 
     return path;

src/libexpr/eval.hh

@@ -850,8 +850,10 @@ std::string showType(const Value & v);
 
 /**
  * If `path` refers to a directory, then append "/default.nix".
+ *
+ * @param addDefaultNix Whether to append "/default.nix" after resolving symlinks.
  */
-SourcePath resolveExprPath(SourcePath path);
+SourcePath resolveExprPath(SourcePath path, bool addDefaultNix = true);
 
 /**
  * Whether a URI is allowed, assuming restrictEval is enabled

src/nix-build/nix-build.cc

@@ -26,6 +26,7 @@
 #include "legacy.hh"
 #include "users.hh"
 #include "network-proxy.hh"
+#include "compatibility-settings.hh"
 
 using namespace nix;
 using namespace std::string_literals;
@@ -90,24 +91,50 @@ static std::vector<std::string> shellwords(const std::string & s)
     return res;
 }
 
+/**
+ * Like `resolveExprPath`, but prefers `shell.nix` instead of `default.nix`,
+ * and if `path` was a directory, it checks eagerly whether `shell.nix` or
+ * `default.nix` exist, throwing an error if they don't.
+ */
+static SourcePath resolveShellExprPath(SourcePath path)
+{
+    auto resolvedOrDir = resolveExprPath(path, false);
+    if (resolvedOrDir.resolveSymlinks().lstat().type == SourceAccessor::tDirectory) {
+        if ((resolvedOrDir / "shell.nix").pathExists()) {
+            if (compatibilitySettings.nixShellAlwaysLooksForShellNix) {
+                return resolvedOrDir / "shell.nix";
+            } else {
+                warn("Skipping '%1%', because the setting '%2%' is disabled. This is a deprecated behavior. Consider enabling '%2%'.",
+                    resolvedOrDir / "shell.nix",
+                    "nix-shell-always-looks-for-shell-nix");
+            }
+        }
+        if ((resolvedOrDir / "default.nix").pathExists()) {
+            return resolvedOrDir / "default.nix";
+        }
+        throw Error("neither '%s' nor '%s' found in '%s'", "shell.nix", "default.nix", resolvedOrDir);
+    }
+    return resolvedOrDir;
+}
+
 static void main_nix_build(int argc, char * * argv)
 {
     auto dryRun = false;
-    auto runEnv = std::regex_search(argv[0], std::regex("nix-shell$"));
+    auto isNixShell = std::regex_search(argv[0], std::regex("nix-shell$"));
     auto pure = false;
     auto fromArgs = false;
     auto packages = false;
     // Same condition as bash uses for interactive shells
     auto interactive = isatty(STDIN_FILENO) && isatty(STDERR_FILENO);
     Strings attrPaths;
-    Strings left;
+    Strings remainingArgs;
     BuildMode buildMode = bmNormal;
     bool readStdin = false;
 
     std::string envCommand; // interactive shell
     Strings envExclude;
 
-    auto myName = runEnv ? "nix-shell" : "nix-build";
+    auto myName = isNixShell ? "nix-shell" : "nix-build";
 
     auto inShebang = false;
     std::string script;
@@ -132,7 +159,7 @@ static void main_nix_build(int argc, char * * argv)
     // Heuristic to see if we're invoked as a shebang script, namely,
     // if we have at least one argument, it's the name of an
     // executable file, and it starts with "#!".
-    if (runEnv && argc > 1) {
+    if (isNixShell && argc > 1) {
         script = argv[1];
         try {
             auto lines = tokenizeString<Strings>(readFile(script), "\n");
@@ -186,9 +213,9 @@ static void main_nix_build(int argc, char * * argv)
             dryRun = true;
 
         else if (*arg == "--run-env") // obsolete
-            runEnv = true;
+            isNixShell = true;
 
-        else if (runEnv && (*arg == "--command" || *arg == "--run")) {
+        else if (isNixShell && (*arg == "--command" || *arg == "--run")) {
             if (*arg == "--run")
                 interactive = false;
             envCommand = getArg(*arg, arg, end) + "\nexit";
@@ -206,7 +233,7 @@ static void main_nix_build(int argc, char * * argv)
         else if (*arg == "--pure") pure = true;
         else if (*arg == "--impure") pure = false;
 
-        else if (runEnv && (*arg == "--packages" || *arg == "-p"))
+        else if (isNixShell && (*arg == "--packages" || *arg == "-p"))
             packages = true;
 
         else if (inShebang && *arg == "-i") {
@@ -246,7 +273,7 @@ static void main_nix_build(int argc, char * * argv)
             return false;
 
         else
-            left.push_back(*arg);
+            remainingArgs.push_back(*arg);
 
         return true;
     });
@@ -266,7 +293,7 @@ static void main_nix_build(int argc, char * * argv)
     auto autoArgs = myArgs.getAutoArgs(*state);
 
     auto autoArgsWithInNixShell = autoArgs;
-    if (runEnv) {
+    if (isNixShell) {
         auto newArgs = state->buildBindings(autoArgsWithInNixShell->size() + 1);
         newArgs.alloc("inNixShell").mkBool(true);
         for (auto & i : *autoArgs) newArgs.insert(i);
@@ -276,19 +303,26 @@ static void main_nix_build(int argc, char * * argv)
     if (packages) {
         std::ostringstream joined;
         joined << "{...}@args: with import <nixpkgs> args; (pkgs.runCommandCC or pkgs.runCommand) \"shell\" { buildInputs = [ ";
-        for (const auto & i : left)
+        for (const auto & i : remainingArgs)
             joined << '(' << i << ") ";
         joined << "]; } \"\"";
         fromArgs = true;
-        left = {joined.str()};
-    } else if (!fromArgs) {
-        if (left.empty() && runEnv && pathExists("shell.nix"))
-            left = {"shell.nix"};
-        if (left.empty())
-            left = {"default.nix"};
+        remainingArgs = {joined.str()};
+    } else if (!fromArgs && remainingArgs.empty()) {
+        if (isNixShell && !compatibilitySettings.nixShellAlwaysLooksForShellNix && std::filesystem::exists("shell.nix")) {
+            // If we're in 2.3 compatibility mode, we need to look for shell.nix
+            // now, because it won't be done later.
+            remainingArgs = {"shell.nix"};
+        } else {
+            remainingArgs = {"."};
+
+            // Instead of letting it throw later, we throw here to give a more relevant error message
+            if (isNixShell && !std::filesystem::exists("shell.nix") && !std::filesystem::exists("default.nix"))
+                throw Error("no argument specified and no '%s' or '%s' file found in the working directory", "shell.nix", "default.nix");
+        }
     }
 
-    if (runEnv)
+    if (isNixShell)
         setEnv("IN_NIX_SHELL", pure ? "pure" : "impure");
 
     PackageInfos drvs;
@@ -299,7 +333,7 @@ static void main_nix_build(int argc, char * * argv)
     if (readStdin)
         exprs = {state->parseStdin()};
     else
-        for (auto i : left) {
+        for (auto i : remainingArgs) {
             if (fromArgs)
                 exprs.push_back(state->parseExprFromString(std::move(i), state->rootPath(".")));
             else {
@@ -310,14 +344,18 @@ static void main_nix_build(int argc, char * * argv)
                 auto [path, outputNames] = parsePathWithOutputs(absolute);
                 if (evalStore->isStorePath(path) && hasSuffix(path, ".drv"))
                     drvs.push_back(PackageInfo(*state, evalStore, absolute));
-                else
+                else {
                     /* If we're in a #! script, interpret filenames
                        relative to the script. */
-                    exprs.push_back(
-                        state->parseExprFromFile(
-                            resolveExprPath(
-                                lookupFileArg(*state,
-                                    inShebang && !packages ? absPath(i, absPath(dirOf(script))) : i))));
+                    auto baseDir = inShebang && !packages ? absPath(i, absPath(dirOf(script))) : i;
+
+                    auto sourcePath = lookupFileArg(*state,
+                                    baseDir);
+                    auto resolvedPath =
+                        isNixShell ? resolveShellExprPath(sourcePath) : resolveExprPath(sourcePath);
+
+                    exprs.push_back(state->parseExprFromFile(resolvedPath));
+                }
             }
         }
 
@@ -330,7 +368,7 @@ static void main_nix_build(int argc, char * * argv)
 
         std::function<bool(const Value & v)> takesNixShellAttr;
         takesNixShellAttr = [&](const Value & v) {
-            if (!runEnv) {
+            if (!isNixShell) {
                 return false;
             }
             bool add = false;
@@ -381,7 +419,7 @@ static void main_nix_build(int argc, char * * argv)
             store->buildPaths(paths, buildMode, evalStore);
     };
 
-    if (runEnv) {
+    if (isNixShell) {
         if (drvs.size() != 1)
             throw UsageError("nix-shell requires a single derivation");