Merge pull request #9648 from cole-h/nix-shell-ordering

nix shell: reflect command line order in PATH order

src/libcmd/installables.cc

@@ -715,7 +715,7 @@ BuiltPaths Installable::toBuiltPaths(
     }
 }
 
-StorePathSet Installable::toStorePaths(
+StorePathSet Installable::toStorePathSet(
     ref<Store> evalStore,
     ref<Store> store,
     Realise mode, OperateOn operateOn,
@@ -729,13 +729,27 @@ StorePathSet Installable::toStorePaths(
     return outPaths;
 }
 
+StorePaths Installable::toStorePaths(
+    ref<Store> evalStore,
+    ref<Store> store,
+    Realise mode, OperateOn operateOn,
+    const Installables & installables)
+{
+    StorePaths outPaths;
+    for (auto & path : toBuiltPaths(evalStore, store, mode, operateOn, installables)) {
+        auto thisOutPaths = path.outPaths();
+        outPaths.insert(outPaths.end(), thisOutPaths.begin(), thisOutPaths.end());
+    }
+    return outPaths;
+}
+
 StorePath Installable::toStorePath(
     ref<Store> evalStore,
     ref<Store> store,
     Realise mode, OperateOn operateOn,
     ref<Installable> installable)
 {
-    auto paths = toStorePaths(evalStore, store, mode, operateOn, {installable});
+    auto paths = toStorePathSet(evalStore, store, mode, operateOn, {installable});
 
     if (paths.size() != 1)
         throw Error("argument '%s' should evaluate to one store path", installable->what());

src/libcmd/installables.hh

@@ -165,7 +165,14 @@ struct Installable
         const Installables & installables,
         BuildMode bMode = bmNormal);
 
-    static std::set<StorePath> toStorePaths(
+    static std::set<StorePath> toStorePathSet(
+        ref<Store> evalStore,
+        ref<Store> store,
+        Realise mode,
+        OperateOn operateOn,
+        const Installables & installables);
+
+    static std::vector<StorePath> toStorePaths(
         ref<Store> evalStore,
         ref<Store> store,
         Realise mode,

src/nix/develop.cc

@@ -376,7 +376,7 @@ struct Common : InstallableCommand, MixProfile
         for (auto & [installable_, dir_] : redirects) {
             auto dir = absPath(dir_);
             auto installable = parseInstallable(store, installable_);
-            auto builtPaths = Installable::toStorePaths(
+            auto builtPaths = Installable::toStorePathSet(
                 getEvalStore(), store, Realise::Nothing, OperateOn::Output, {installable});
             for (auto & path: builtPaths) {
                 auto from = store->printStorePath(path);
@@ -631,7 +631,7 @@ struct CmdDevelop : Common, MixEnvironment
 
             bool found = false;
 
-            for (auto & path : Installable::toStorePaths(getEvalStore(), store, Realise::Outputs, OperateOn::Output, {bashInstallable})) {
+            for (auto & path : Installable::toStorePathSet(getEvalStore(), store, Realise::Outputs, OperateOn::Output, {bashInstallable})) {
                 auto s = store->printStorePath(path) + "/bin/bash";
                 if (pathExists(s)) {
                     shell = s;

src/nix/run.cc

@@ -114,7 +114,7 @@ struct CmdShell : InstallablesCommand, MixEnvironment
 
         setEnviron();
 
-        auto unixPath = tokenizeString<Strings>(getEnv("PATH").value_or(""), ":");
+        std::vector<std::string> pathAdditions;
 
         while (!todo.empty()) {
             auto path = todo.front();
@@ -122,7 +122,7 @@ struct CmdShell : InstallablesCommand, MixEnvironment
             if (!done.insert(path).second) continue;
 
             if (true)
-                unixPath.push_front(store->printStorePath(path) + "/bin");
+                pathAdditions.push_back(store->printStorePath(path) + "/bin");
 
             auto propPath = CanonPath(store->printStorePath(path)) + "nix-support" + "propagated-user-env-packages";
             if (auto st = accessor->maybeLstat(propPath); st && st->type == SourceAccessor::tRegular) {
@@ -131,7 +131,10 @@ struct CmdShell : InstallablesCommand, MixEnvironment
             }
         }
 
-        setenv("PATH", concatStringsSep(":", unixPath).c_str(), 1);
+        auto unixPath = tokenizeString<Strings>(getEnv("PATH").value_or(""), ":");
+        unixPath.insert(unixPath.begin(), pathAdditions.begin(), pathAdditions.end());
+        auto unixPathString = concatStringsSep(":", unixPath);
+        setenv("PATH", unixPathString.c_str(), 1);
 
         Strings args;
         for (auto & arg : command) args.push_back(arg);

tests/functional/shell-hello.nix

@@ -23,4 +23,20 @@ with import ./config.nix;
         chmod +x $dev/bin/hello2
       '';
   };
+
+  salve-mundi = mkDerivation {
+    name = "salve-mundi";
+    outputs = [ "out" ];
+    meta.outputsToInstall = [ "out" ];
+    buildCommand =
+      ''
+        mkdir -p $out/bin
+
+        cat > $out/bin/hello <<EOF
+        #! ${shell}
+        echo "Salve Mundi from $out/bin/hello"
+        EOF
+        chmod +x $out/bin/hello
+      '';
+  };
 }

tests/functional/shell.sh

@@ -10,6 +10,14 @@ nix shell -f shell-hello.nix hello -c hello NixOS | grep 'Hello NixOS'
 nix shell -f shell-hello.nix hello^dev -c hello2 | grep 'Hello2'
 nix shell -f shell-hello.nix 'hello^*' -c hello2 | grep 'Hello2'
 
+
+if isDaemonNewer "2.20.0pre20231220"; then
+    # Test that command line attribute ordering is reflected in the PATH
+    # https://github.com/NixOS/nix/issues/7905
+    nix shell -f shell-hello.nix hello salve-mundi -c hello | grep 'Hello World'
+    nix shell -f shell-hello.nix salve-mundi hello -c hello | grep 'Salve Mundi'
+fi
+
 requireSandboxSupport
 
 chmod -R u+w $TEST_ROOT/store0 || true