[Nix#9515] PATH handling debt and execvp #92

New issue

Open

opened 2024-03-16 06:44:52 +00:00 by lix-bot · 0 comments

lix-bot commented

2024-03-16 06:44:52 +00:00

Member

Upstream-Issue: NixOS/nix#9515

Currently we have duplicated $PATH parsing calls.

Duplicated code

$ grep -RnU3 'getEnv("PATH")' .
./src/nix/upgrade-nix.cc-104-    {
./src/nix/upgrade-nix.cc-105-        Path where;
./src/nix/upgrade-nix.cc-106-
./src/nix/upgrade-nix.cc:107:        for (auto & dir : tokenizeString<Strings>(getEnv("PATH").value_or(""), ":"))
./src/nix/upgrade-nix.cc-108-            if (pathExists(dir + "/nix-env")) {
./src/nix/upgrade-nix.cc-109-                where = dir;
./src/nix/upgrade-nix.cc-110-                break;
--
./src/nix/config-check.cc-76-    {
./src/nix/config-check.cc-77-        PathSet dirs;
./src/nix/config-check.cc-78-
./src/nix/config-check.cc:79:        for (auto & dir : tokenizeString<Strings>(getEnv("PATH").value_or(""), ":"))
./src/nix/config-check.cc-80-            if (pathExists(dir + "/nix-env"))
./src/nix/config-check.cc-81-                dirs.insert(dirOf(canonPath(dir + "/nix-env", true)));
./src/nix/config-check.cc-82-
--
./src/nix/config-check.cc-95-    {
./src/nix/config-check.cc-96-        PathSet dirs;
./src/nix/config-check.cc-97-
./src/nix/config-check.cc:98:        for (auto & dir : tokenizeString<Strings>(getEnv("PATH").value_or(""), ":")) {
./src/nix/config-check.cc-99-            Path profileDir = dirOf(dir);
./src/nix/config-check.cc-100-            try {
./src/nix/config-check.cc-101-                Path userEnv = canonPath(profileDir, true);
--
./src/nix/run.cc-114-
./src/nix/run.cc-115-        setEnviron();
./src/nix/run.cc-116-
./src/nix/run.cc:117:        auto unixPath = tokenizeString<Strings>(getEnv("PATH").value_or(""), ":");
./src/nix/run.cc-118-
./src/nix/run.cc-119-        while (!todo.empty()) {
./src/nix/run.cc-120-            auto path = todo.front();

execvp is weird and does too much, as explained by @sellout in https://github.com/NixOS/nix/pull/9494.
Instead, Nix can implement this logic itself, and provide a helper function to launch a process with it, so that we don't expose ourselves to this unix quirk. See discussion there.

Upstream-Issue: https://git.lix.systems/NixOS/nix/issues/9515 <ol><li> Currently we have duplicated `$PATH` parsing calls. <details><summary>Duplicated code</summary> ``` $ grep -RnU3 'getEnv("PATH")' . ./src/nix/upgrade-nix.cc-104- { ./src/nix/upgrade-nix.cc-105- Path where; ./src/nix/upgrade-nix.cc-106- ./src/nix/upgrade-nix.cc:107: for (auto & dir : tokenizeString<Strings>(getEnv("PATH").value_or(""), ":")) ./src/nix/upgrade-nix.cc-108- if (pathExists(dir + "/nix-env")) { ./src/nix/upgrade-nix.cc-109- where = dir; ./src/nix/upgrade-nix.cc-110- break; -- ./src/nix/config-check.cc-76- { ./src/nix/config-check.cc-77- PathSet dirs; ./src/nix/config-check.cc-78- ./src/nix/config-check.cc:79: for (auto & dir : tokenizeString<Strings>(getEnv("PATH").value_or(""), ":")) ./src/nix/config-check.cc-80- if (pathExists(dir + "/nix-env")) ./src/nix/config-check.cc-81- dirs.insert(dirOf(canonPath(dir + "/nix-env", true))); ./src/nix/config-check.cc-82- -- ./src/nix/config-check.cc-95- { ./src/nix/config-check.cc-96- PathSet dirs; ./src/nix/config-check.cc-97- ./src/nix/config-check.cc:98: for (auto & dir : tokenizeString<Strings>(getEnv("PATH").value_or(""), ":")) { ./src/nix/config-check.cc-99- Path profileDir = dirOf(dir); ./src/nix/config-check.cc-100- try { ./src/nix/config-check.cc-101- Path userEnv = canonPath(profileDir, true); -- ./src/nix/run.cc-114- ./src/nix/run.cc-115- setEnviron(); ./src/nix/run.cc-116- ./src/nix/run.cc:117: auto unixPath = tokenizeString<Strings>(getEnv("PATH").value_or(""), ":"); ./src/nix/run.cc-118- ./src/nix/run.cc-119- while (!todo.empty()) { ./src/nix/run.cc-120- auto path = todo.front(); ``` </details> </li> <li> `execvp` is weird and does too much, as explained by @sellout in https://github.com/NixOS/nix/pull/9494. Instead, Nix can implement this logic itself, and provide a helper function to launch a process with it, so that we don't expose ourselves to this unix quirk. See discussion there.