From 5ed1884875cc6a6e9330b6c5a2f24c35e685f5a0 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Thu, 21 Dec 2023 10:14:54 -0800 Subject: [PATCH 1/3] libcmd: Installable::toStorePaths -> Installable::toStorePathSet --- src/libcmd/installables.cc | 4 ++-- src/libcmd/installables.hh | 2 +- src/nix/develop.cc | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 6b3c82374..be9ebe9ca 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -715,7 +715,7 @@ BuiltPaths Installable::toBuiltPaths( } } -StorePathSet Installable::toStorePaths( +StorePathSet Installable::toStorePathSet( ref evalStore, ref store, Realise mode, OperateOn operateOn, @@ -735,7 +735,7 @@ StorePath Installable::toStorePath( Realise mode, OperateOn operateOn, ref 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()); diff --git a/src/libcmd/installables.hh b/src/libcmd/installables.hh index e087f935c..c8ad41388 100644 --- a/src/libcmd/installables.hh +++ b/src/libcmd/installables.hh @@ -165,7 +165,7 @@ struct Installable const Installables & installables, BuildMode bMode = bmNormal); - static std::set toStorePaths( + static std::set toStorePathSet( ref evalStore, ref store, Realise mode, diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 8db2de491..974020951 100644 --- a/src/nix/develop.cc +++ b/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; From 1fb43d1eee6f398686523c0bb80adb987c584c61 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Wed, 20 Dec 2023 10:25:22 -0800 Subject: [PATCH 2/3] tests: add a test for command line ordering --- tests/functional/shell-hello.nix | 16 ++++++++++++++++ tests/functional/shell.sh | 8 ++++++++ 2 files changed, 24 insertions(+) diff --git a/tests/functional/shell-hello.nix b/tests/functional/shell-hello.nix index 3fdd3501d..dfe66ef93 100644 --- a/tests/functional/shell-hello.nix +++ b/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 < Date: Mon, 18 Dec 2023 15:22:09 -0800 Subject: [PATCH 3/3] nix shell: reflect command line order in PATH order Prior to this change, Nix would prepend every installable to the PATH list in order to ensure that installables appeared before the current PATH from the ambient environment. With this change, all the installables are still prepended to the PATH, but in the same order as they appear on the command line. This means that the first of two packages that expose an executable `hello` would appear in the PATH first, and thus be executed first. See the test in the prior commit for a more concrete example. --- src/libcmd/installables.cc | 14 ++++++++++++++ src/libcmd/installables.hh | 7 +++++++ src/nix/run.cc | 9 ++++++--- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index be9ebe9ca..736c41a1e 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -729,6 +729,20 @@ StorePathSet Installable::toStorePathSet( return outPaths; } +StorePaths Installable::toStorePaths( + ref evalStore, + ref 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 evalStore, ref store, diff --git a/src/libcmd/installables.hh b/src/libcmd/installables.hh index c8ad41388..95e8841ca 100644 --- a/src/libcmd/installables.hh +++ b/src/libcmd/installables.hh @@ -172,6 +172,13 @@ struct Installable OperateOn operateOn, const Installables & installables); + static std::vector toStorePaths( + ref evalStore, + ref store, + Realise mode, + OperateOn operateOn, + const Installables & installables); + static StorePath toStorePath( ref evalStore, ref store, diff --git a/src/nix/run.cc b/src/nix/run.cc index efc0c56a1..9bca5b9d0 100644 --- a/src/nix/run.cc +++ b/src/nix/run.cc @@ -114,7 +114,7 @@ struct CmdShell : InstallablesCommand, MixEnvironment setEnviron(); - auto unixPath = tokenizeString(getEnv("PATH").value_or(""), ":"); + std::vector 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(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);