Merge pull request #11218 from obsidiansystems/better-executable-path

Factor out `lookupExecutable` and other PATH improvements

.clang-format

@@ -31,3 +31,4 @@ AlwaysBreakBeforeMultilineStrings: true
 IndentPPDirectives: AfterHash
 PPIndentWidth: 2
 BinPackArguments: false
+BreakBeforeTernaryOperators: true

maintainers/flake-module.nix

@@ -275,7 +275,6 @@
             ''^src/libutil/current-process\.hh$''
             ''^src/libutil/english\.cc$''
             ''^src/libutil/english\.hh$''
-            ''^src/libutil/environment-variables\.cc$''
             ''^src/libutil/error\.cc$''
             ''^src/libutil/error\.hh$''
             ''^src/libutil/exit\.hh$''
@@ -357,7 +356,6 @@
             ''^src/libutil/util\.cc$''
             ''^src/libutil/util\.hh$''
             ''^src/libutil/variant-wrapper\.hh$''
-            ''^src/libutil/windows/environment-variables\.cc$''
             ''^src/libutil/windows/file-descriptor\.cc$''
             ''^src/libutil/windows/file-path\.cc$''
             ''^src/libutil/windows/processes\.cc$''
@@ -485,7 +483,6 @@
             ''^tests/unit/libutil/pool\.cc''
             ''^tests/unit/libutil/references\.cc''
             ''^tests/unit/libutil/suggestions\.cc''
-            ''^tests/unit/libutil/tests\.cc''
             ''^tests/unit/libutil/url\.cc''
             ''^tests/unit/libutil/xml-writer\.cc''
           ];

src/libutil/environment-variables.cc

@@ -8,13 +8,16 @@ namespace nix {
 std::optional<std::string> getEnv(const std::string & key)
 {
     char * value = getenv(key.c_str());
-    if (!value) return {};
+    if (!value)
+        return {};
     return std::string(value);
 }
 
-std::optional<std::string> getEnvNonEmpty(const std::string & key) {
+std::optional<std::string> getEnvNonEmpty(const std::string & key)
+{
     auto value = getEnv(key);
-    if (value == "") return {};
+    if (value == "")
+        return {};
     return value;
 }
 

src/libutil/environment-variables.hh

@@ -9,6 +9,7 @@
 #include <optional>
 
 #include "types.hh"
+#include "file-path.hh"
 
 namespace nix {
 
@@ -17,6 +18,11 @@ namespace nix {
  */
 std::optional<std::string> getEnv(const std::string & key);
 
+/**
+ * Like `getEnv`, but using `OsString` to avoid coercions.
+ */
+std::optional<OsString> getEnvOs(const OsString & key);
+
 /**
  * @return a non empty environment variable. Returns nullopt if the env
  * variable is set to ""
@@ -43,6 +49,11 @@ int unsetenv(const char * name);
  */
 int setEnv(const char * name, const char * value);
 
+/**
+ * Like `setEnv`, but using `OsString` to avoid coercions.
+ */
+int setEnvOs(const OsString & name, const OsString & value);
+
 /**
  * Clear the environment.
  */

src/libutil/executable-path.cc

@@ -0,0 +1,79 @@
+#include "environment-variables.hh"
+#include "executable-path.hh"
+#include "strings-inline.hh"
+#include "util.hh"
+#include "file-path-impl.hh"
+
+namespace nix {
+
+namespace fs = std::filesystem;
+
+constexpr static const OsStringView path_var_separator{
+    &ExecutablePath::separator,
+    1,
+};
+
+ExecutablePath ExecutablePath::load()
+{
+    // "If PATH is unset or is set to null, the path search is
+    // implementation-defined."
+    // https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03
+    return ExecutablePath::parse(getEnvOs(OS_STR("PATH")).value_or(OS_STR("")));
+}
+
+ExecutablePath ExecutablePath::parse(const OsString & path)
+{
+    auto strings = path.empty() ? (std::list<OsString>{})
+                                : basicSplitString<std::list<OsString>, OsString::value_type>(path, path_var_separator);
+
+    std::vector<fs::path> ret;
+    ret.reserve(strings.size());
+
+    std::transform(
+        std::make_move_iterator(strings.begin()),
+        std::make_move_iterator(strings.end()),
+        std::back_inserter(ret),
+        [](auto && str) {
+            return fs::path{
+                str.empty()
+                    // "A zero-length prefix is a legacy feature that
+                    // indicates the current working directory. It
+                    // appears as two adjacent <colon> characters
+                    // ("::"), as an initial <colon> preceding the rest
+                    // of the list, or as a trailing <colon> following
+                    // the rest of the list."
+                    // https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03
+                    ? OS_STR(".")
+                    : std::move(str),
+            };
+        });
+
+    return {ret};
+}
+
+OsString ExecutablePath::render() const
+{
+    std::vector<PathViewNG> path2;
+    for (auto & p : directories)
+        path2.push_back(p.native());
+    return basicConcatStringsSep(path_var_separator, path2);
+}
+
+std::optional<fs::path>
+ExecutablePath::find(const OsString & exe, std::function<bool(const fs::path &)> isExecutable) const
+{
+    // "If the pathname being sought contains a <slash>, the search
+    // through the path prefixes shall not be performed."
+    // https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03
+    assert(OsPathTrait<fs::path::value_type>::rfindPathSep(exe) == exe.npos);
+
+    for (auto & dir : directories) {
+        auto candidate = dir / exe;
+        if (isExecutable(candidate))
+            return std::filesystem::canonical(candidate);
+    }
+
+    return std::nullopt;
+}
+
+} // namespace nix

src/libutil/executable-path.hh

@@ -0,0 +1,64 @@
+#pragma once
+///@file
+
+#include "file-system.hh"
+
+namespace nix {
+
+struct ExecutablePath
+{
+    std::vector<std::filesystem::path> directories;
+
+    constexpr static const OsString::value_type separator =
+#ifdef WIN32
+        L';'
+#else
+        ':'
+#endif
+        ;
+
+    /**
+     * Parse `path` into a list of paths.
+     *
+     * On Unix we split on `:`, on Windows we split on `;`.
+     *
+     * For Unix, this is according to the POSIX spec for `PATH`.
+     * https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03
+     */
+    static ExecutablePath parse(const OsString & path);
+
+    /**
+     * Load the `PATH` environment variable and `parse` it.
+     */
+    static ExecutablePath load();
+
+    /**
+     * Opposite of `parse`
+     */
+    OsString render() const;
+
+    /**
+     * Search for an executable.
+     *
+     * For Unix, this is according to the POSIX spec for `PATH`.
+     * https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03
+     *
+     * @param exe This must just be a name, and not contain any `/` (or
+     * `\` on Windows). in case it does, per the spec no lookup should
+     * be perfomed, and the path (it is not just a file name) as is.
+     * This is the caller's respsonsibility.
+     *
+     * This is a pure function, except for the default `isExecutable`
+     * argument, which uses the ambient file system to check if a file is
+     * executable (and exists).
+     *
+     * @return path to a resolved executable
+     */
+    std::optional<std::filesystem::path> find(
+        const OsString & exe,
+        std::function<bool(const std::filesystem::path &)> isExecutableFile = isExecutableFileAmbient) const;
+
+    bool operator==(const ExecutablePath &) const = default;
+};
+
+} // namespace nix

src/libutil/file-path-impl.hh

@@ -91,13 +91,10 @@ struct WindowsPathTrait
 };
 
 
-/**
+template<typename CharT>
- * @todo Revisit choice of `char` or `wchar_t` for `WindowsPathTrait`
+using OsPathTrait =
- * argument.
- */
-using NativePathTrait =
 #ifdef _WIN32
-    WindowsPathTrait<char>
+    WindowsPathTrait<CharT>
 #else
     UnixPathTrait
 #endif

src/libutil/file-path.hh

@@ -1,10 +1,10 @@
 #pragma once
 ///@file
 
-#include <optional>
 #include <filesystem>
 
 #include "types.hh"
+#include "os-string.hh"
 
 namespace nix {
 
@@ -22,39 +22,26 @@ typedef std::set<std::filesystem::path> PathSetNG;
  *
  * @todo drop `NG` suffix and replace the one in `types.hh`.
  */
-struct PathViewNG : std::basic_string_view<std::filesystem::path::value_type>
+struct PathViewNG : OsStringView
 {
-    using string_view = std::basic_string_view<std::filesystem::path::value_type>;
+    using string_view = OsStringView;
 
     using string_view::string_view;
 
     PathViewNG(const std::filesystem::path & path)
-        : std::basic_string_view<std::filesystem::path::value_type>(path.native())
+        : OsStringView{path.native()}
     { }
 
-    PathViewNG(const std::filesystem::path::string_type & path)
+    PathViewNG(const OsString & path)
-        : std::basic_string_view<std::filesystem::path::value_type>(path)
+        : OsStringView{path}
     { }
 
     const string_view & native() const { return *this; }
     string_view & native() { return *this; }
 };
 
-std::string os_string_to_string(PathViewNG::string_view path);
-
-std::filesystem::path::string_type string_to_os_string(std::string_view s);
-
 std::optional<std::filesystem::path> maybePath(PathView path);
 
 std::filesystem::path pathNG(PathView path);
 
-/**
- * Create string literals with the native character width of paths
- */
-#ifndef _WIN32
-# define PATHNG_LITERAL(s) s
-#else
-# define PATHNG_LITERAL(s) L ## s
-#endif
-
 }

src/libutil/file-system.cc

@@ -92,7 +92,7 @@ Path canonPath(PathView path, bool resolveSymlinks)
        arbitrary (but high) limit to prevent infinite loops. */
     unsigned int followCount = 0, maxFollow = 1024;
 
-    auto ret = canonPathInner<NativePathTrait>(
+    auto ret = canonPathInner<OsPathTrait<char>>(
         path,
         [&followCount, &temp, maxFollow, resolveSymlinks]
         (std::string & result, std::string_view & remaining) {
@@ -122,7 +122,7 @@ Path canonPath(PathView path, bool resolveSymlinks)
 
 Path dirOf(const PathView path)
 {
-    Path::size_type pos = NativePathTrait::rfindPathSep(path);
+    Path::size_type pos = OsPathTrait<char>::rfindPathSep(path);
     if (pos == path.npos)
         return ".";
     return fs::path{path}.parent_path().string();
@@ -135,10 +135,10 @@ std::string_view baseNameOf(std::string_view path)
         return "";
 
     auto last = path.size() - 1;
-    while (last > 0 && NativePathTrait::isPathSep(path[last]))
+    while (last > 0 && OsPathTrait<char>::isPathSep(path[last]))
         last -= 1;
 
-    auto pos = NativePathTrait::rfindPathSep(path, last);
+    auto pos = OsPathTrait<char>::rfindPathSep(path, last);
     if (pos == path.npos)
         pos = 0;
     else
@@ -569,7 +569,7 @@ void replaceSymlink(const Path & target, const Path & link)
 }
 
 void setWriteTime(
-    const std::filesystem::path & path,
+    const fs::path & path,
     time_t accessedTime,
     time_t modificationTime,
     std::optional<bool> optIsSymlink)
@@ -685,4 +685,18 @@ void moveFile(const Path & oldName, const Path & newName)
 
 //////////////////////////////////////////////////////////////////////
 
+bool isExecutableFileAmbient(const fs::path & exe) {
+    // Check file type, because directory being executable means
+    // something completely different.
+    // `is_regular_file` follows symlinks before checking.
+    return std::filesystem::is_regular_file(exe)
+        && access(exe.string().c_str(),
+#ifdef WIN32
+        0 // TODO do better
+#else
+        X_OK
+#endif
+        ) == 0;
+}
+
 }

src/libutil/file-system.hh

@@ -263,6 +263,12 @@ std::pair<AutoCloseFD, Path> createTempFile(const Path & prefix = "nix");
  */
 Path defaultTempDir();
 
+/**
+ * Interpret `exe` as a location in the ambient file system and return
+ * whether it resolves to a file that is executable.
+ */
+bool isExecutableFileAmbient(const std::filesystem::path & exe);
+
 /**
  * Used in various places.
  */

src/libutil/meson.build

@@ -129,6 +129,7 @@ sources = files(
   'english.cc',
   'environment-variables.cc',
   'error.cc',
+  'executable-path.cc',
   'exit.cc',
   'experimental-features.cc',
   'file-content-address.cc',
@@ -183,6 +184,7 @@ headers = [config_h] + files(
   'english.hh',
   'environment-variables.hh',
   'error.hh',
+  'executable-path.hh',
   'exit.hh',
   'experimental-features.hh',
   'file-content-address.hh',
@@ -202,6 +204,7 @@ headers = [config_h] + files(
   'lru-cache.hh',
   'memory-source-accessor.hh',
   'muxable-pipe.hh',
+  'os-string.hh',
   'pool.hh',
   'position.hh',
   'posix-source-accessor.hh',

src/libutil/os-string.hh

@@ -0,0 +1,43 @@
+#pragma once
+///@file
+
+#include <optional>
+#include <string>
+#include <string_view>
+
+namespace nix {
+
+/**
+ * Named because it is similar to the Rust type, except it is in the
+ * native encoding not WTF-8.
+ *
+ * Same as `std::filesystem::path::string_type`, but manually defined to
+ * avoid including a much more complex header.
+ */
+using OsString = std::basic_string<
+#if defined(_WIN32) && !defined(__CYGWIN__)
+    wchar_t
+#else
+    char
+#endif
+    >;
+
+/**
+ * `std::string_view` counterpart for `OsString`.
+ */
+using OsStringView = std::basic_string_view<OsString::value_type>;
+
+std::string os_string_to_string(OsStringView path);
+
+OsString string_to_os_string(std::string_view s);
+
+/**
+ * Create string literals with the native character width of paths
+ */
+#ifndef _WIN32
+#  define OS_STR(s) s
+#else
+#  define OS_STR(s) L##s
+#endif
+
+}

src/libutil/strings-inline.hh

@@ -4,8 +4,8 @@
 
 namespace nix {
 
-template<class C>
+template<class C, class CharT>
-C tokenizeString(std::string_view s, std::string_view separators)
+C basicTokenizeString(std::basic_string_view<CharT> s, std::basic_string_view<CharT> separators)
 {
     C result;
     auto pos = s.find_first_not_of(separators, 0);
@@ -13,14 +13,42 @@ C tokenizeString(std::string_view s, std::string_view separators)
         auto end = s.find_first_of(separators, pos + 1);
         if (end == s.npos)
             end = s.size();
-        result.insert(result.end(), std::string(s, pos, end - pos));
+        result.insert(result.end(), std::basic_string<CharT>(s, pos, end - pos));
         pos = s.find_first_not_of(separators, end);
     }
     return result;
 }
 
 template<class C>
-std::string concatStringsSep(const std::string_view sep, const C & ss)
+C tokenizeString(std::string_view s, std::string_view separators)
+{
+    return basicTokenizeString<C, char>(s, separators);
+}
+
+template<class C, class CharT>
+C basicSplitString(std::basic_string_view<CharT> s, std::basic_string_view<CharT> separators)
+{
+    C result;
+    size_t pos = 0;
+    while (pos <= s.size()) {
+        auto end = s.find_first_of(separators, pos);
+        if (end == s.npos)
+            end = s.size();
+        result.insert(result.end(), std::basic_string<CharT>(s, pos, end - pos));
+        pos = end + 1;
+    }
+
+    return result;
+}
+
+template<class C>
+C splitString(std::string_view s, std::string_view separators)
+{
+    return basicSplitString<C, char>(s, separators);
+}
+
+template<class CharT, class C>
+std::basic_string<CharT> basicConcatStringsSep(const std::basic_string_view<CharT> sep, const C & ss)
 {
     size_t size = 0;
     bool tail = false;
@@ -28,10 +56,10 @@ std::string concatStringsSep(const std::string_view sep, const C & ss)
     for (const auto & s : ss) {
         if (tail)
             size += sep.size();
-        size += std::string_view(s).size();
+        size += std::basic_string_view<CharT>{s}.size();
         tail = true;
     }
-    std::string s;
+    std::basic_string<CharT> s;
     s.reserve(size);
     tail = false;
     for (auto & i : ss) {
@@ -43,6 +71,12 @@ std::string concatStringsSep(const std::string_view sep, const C & ss)
     return s;
 }
 
+template<class C>
+std::string concatStringsSep(const std::string_view sep, const C & ss)
+{
+    return basicConcatStringsSep<char, C>(sep, ss);
+}
+
 template<class C>
 std::string dropEmptyInitThenConcatStringsSep(const std::string_view sep, const C & ss)
 {

src/libutil/strings.cc

@@ -1,6 +1,8 @@
+#include <filesystem>
 #include <string>
 
 #include "strings-inline.hh"
+#include "os-string.hh"
 
 namespace nix {
 
@@ -8,6 +10,13 @@ template std::list<std::string> tokenizeString(std::string_view s, std::string_v
 template std::set<std::string> tokenizeString(std::string_view s, std::string_view separators);
 template std::vector<std::string> tokenizeString(std::string_view s, std::string_view separators);
 
+template std::list<std::string> splitString(std::string_view s, std::string_view separators);
+template std::set<std::string> splitString(std::string_view s, std::string_view separators);
+template std::vector<std::string> splitString(std::string_view s, std::string_view separators);
+
+template std::list<OsString> basicSplitString(
+    std::basic_string_view<OsString::value_type> s, std::basic_string_view<OsString::value_type> separators);
+
 template std::string concatStringsSep(std::string_view, const std::list<std::string> &);
 template std::string concatStringsSep(std::string_view, const std::set<std::string> &);
 template std::string concatStringsSep(std::string_view, const std::vector<std::string> &);

src/libutil/strings.hh

@@ -13,6 +13,12 @@ namespace nix {
  *
  * See also `basicSplitString()`, which preserves empty strings between separators, as well as at the start and end.
  */
+template<class C, class CharT = char>
+C basicTokenizeString(std::basic_string_view<CharT> s, std::basic_string_view<CharT> separators);
+
+/**
+ * Like `basicTokenizeString` but specialized to the default `char`
+ */
 template<class C>
 C tokenizeString(std::string_view s, std::string_view separators = " \t\n\r");
 
@@ -20,6 +26,20 @@ extern template std::list<std::string> tokenizeString(std::string_view s, std::s
 extern template std::set<std::string> tokenizeString(std::string_view s, std::string_view separators);
 extern template std::vector<std::string> tokenizeString(std::string_view s, std::string_view separators);
 
+/**
+ * Split a string, preserving empty strings between separators, as well as at the start and end.
+ *
+ * Returns a non-empty collection of strings.
+ */
+template<class C, class CharT = char>
+C basicSplitString(std::basic_string_view<CharT> s, std::basic_string_view<CharT> separators);
+template<typename C>
+C splitString(std::string_view s, std::string_view separators);
+
+extern template std::list<std::string> splitString(std::string_view s, std::string_view separators);
+extern template std::set<std::string> splitString(std::string_view s, std::string_view separators);
+extern template std::vector<std::string> splitString(std::string_view s, std::string_view separators);
+
 /**
  * Concatenate the given strings with a separator between the elements.
  */

src/libutil/unix/environment-variables.cc

@@ -9,4 +9,14 @@ int setEnv(const char * name, const char * value)
     return ::setenv(name, value, 1);
 }
 
+std::optional<std::string> getEnvOs(const std::string & key)
+{
+    return getEnv(key);
+}
+
+int setEnvOs(const OsString & name, const OsString & value)
+{
+    return setEnv(name.c_str(), value.c_str());
+}
+
 }

src/libutil/unix/file-path.cc

@@ -8,16 +8,6 @@
 
 namespace nix {
 
-std::string os_string_to_string(PathViewNG::string_view path)
-{
-    return std::string { path };
-}
-
-std::filesystem::path::string_type string_to_os_string(std::string_view s)
-{
-    return std::string { s };
-}
-
 std::optional<std::filesystem::path> maybePath(PathView path)
 {
     return { path };

src/libutil/unix/meson.build

@@ -4,6 +4,7 @@ sources += files(
   'file-path.cc',
   'file-system.cc',
   'muxable-pipe.cc',
+  'os-string.cc',
   'processes.cc',
   'signals.cc',
   'users.cc',

src/libutil/unix/os-string.cc

@@ -0,0 +1,21 @@
+#include <algorithm>
+#include <codecvt>
+#include <iostream>
+#include <locale>
+
+#include "file-path.hh"
+#include "util.hh"
+
+namespace nix {
+
+std::string os_string_to_string(PathViewNG::string_view path)
+{
+    return std::string{path};
+}
+
+std::filesystem::path::string_type string_to_os_string(std::string_view s)
+{
+    return std::string{s};
+}
+
+}

src/libutil/util.cc

@@ -1,5 +1,6 @@
 #include "util.hh"
 #include "fmt.hh"
+#include "file-path.hh"
 
 #include <array>
 #include <cctype>

src/libutil/windows/environment-variables.cc

@@ -4,6 +4,29 @@
 
 namespace nix {
 
+std::optional<OsString> getEnvOs(const OsString & key)
+{
+    // Determine the required buffer size for the environment variable value
+    DWORD bufferSize = GetEnvironmentVariableW(key.c_str(), nullptr, 0);
+    if (bufferSize == 0) {
+        return std::nullopt;
+    }
+
+    // Allocate a buffer to hold the environment variable value
+    std::wstring value{L'\0', bufferSize};
+
+    // Retrieve the environment variable value
+    DWORD resultSize = GetEnvironmentVariableW(key.c_str(), &value[0], bufferSize);
+    if (resultSize == 0) {
+        return std::nullopt;
+    }
+
+    // Resize the string to remove the extra null characters
+    value.resize(resultSize);
+
+    return value;
+}
+
 int unsetenv(const char * name)
 {
     return -SetEnvironmentVariableA(name, nullptr);
@@ -14,4 +37,9 @@ int setEnv(const char * name, const char * value)
     return -SetEnvironmentVariableA(name, value);
 }
 
+int setEnvOs(const OsString & name, const OsString & value)
+{
+    return -SetEnvironmentVariableW(name.c_str(), value.c_str());
+}
+
 }

src/libutil/windows/file-path.cc

@@ -9,18 +9,6 @@
 
 namespace nix {
 
-std::string os_string_to_string(PathViewNG::string_view path)
-{
-    std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>> converter;
-    return converter.to_bytes(std::filesystem::path::string_type { path });
-}
-
-std::filesystem::path::string_type string_to_os_string(std::string_view s)
-{
-    std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>> converter;
-    return converter.from_bytes(std::string { s });
-}
-
 std::optional<std::filesystem::path> maybePath(PathView path)
 {
     if (path.length() >= 3 && (('A' <= path[0] && path[0] <= 'Z') || ('a' <= path[0] && path[0] <= 'z')) && path[1] == ':' && WindowsPathTrait<char>::isPathSep(path[2])) {

src/libutil/windows/meson.build

@@ -4,6 +4,7 @@ sources += files(
   'file-path.cc',
   'file-system.cc',
   'muxable-pipe.cc',
+  'os-string.cc',
   'processes.cc',
   'users.cc',
   'windows-async-pipe.cc',

src/libutil/windows/os-string.cc

@@ -0,0 +1,24 @@
+#include <algorithm>
+#include <codecvt>
+#include <iostream>
+#include <locale>
+
+#include "file-path.hh"
+#include "file-path-impl.hh"
+#include "util.hh"
+
+namespace nix {
+
+std::string os_string_to_string(PathViewNG::string_view path)
+{
+    std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>> converter;
+    return converter.to_bytes(std::filesystem::path::string_type{path});
+}
+
+std::filesystem::path::string_type string_to_os_string(std::string_view s)
+{
+    std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>> converter;
+    return converter.from_bytes(std::string{s});
+}
+
+}

src/nix/config-check.cc

@@ -8,6 +8,7 @@
 #include "store-api.hh"
 #include "local-fs-store.hh"
 #include "worker-protocol.hh"
+#include "executable-path.hh"
 
 using namespace nix;
 
@@ -39,6 +40,8 @@ void checkInfo(const std::string & msg) {
 
 }
 
+namespace fs = std::filesystem;
+
 struct CmdConfigCheck : StoreCommand
 {
     bool success = true;
@@ -75,11 +78,13 @@ struct CmdConfigCheck : StoreCommand
 
     bool checkNixInPath()
     {
-        PathSet dirs;
+        std::set<fs::path> dirs;
 
-        for (auto & dir : tokenizeString<Strings>(getEnv("PATH").value_or(""), ":"))
+        for (auto & dir : ExecutablePath::load().directories) {
-            if (pathExists(dir + "/nix-env"))
+            auto candidate = dir / "nix-env";
-                dirs.insert(dirOf(canonPath(dir + "/nix-env", true)));
+            if (fs::exists(candidate))
+                dirs.insert(fs::canonical(candidate).parent_path() );
+        }
 
         if (dirs.size() != 1) {
             std::stringstream ss;
@@ -94,18 +99,25 @@ struct CmdConfigCheck : StoreCommand
 
     bool checkProfileRoots(ref<Store> store)
     {
-        PathSet dirs;
+        std::set<fs::path> dirs;
 
-        for (auto & dir : tokenizeString<Strings>(getEnv("PATH").value_or(""), ":")) {
+        for (auto & dir : ExecutablePath::load().directories) {
-            Path profileDir = dirOf(dir);
+            auto profileDir = dir.parent_path();
             try {
-                Path userEnv = canonPath(profileDir, true);
+                auto userEnv = fs::weakly_canonical(profileDir);
 
-                if (store->isStorePath(userEnv) && hasSuffix(userEnv, "user-environment")) {
+                auto noContainsProfiles = [&]{
-                    while (profileDir.find("/profiles/") == std::string::npos && std::filesystem::is_symlink(profileDir))
+                    for (auto && part : profileDir)
-                        profileDir = absPath(readLink(profileDir), dirOf(profileDir));
+                        if (part == "profiles") return false;
+                    return true;
+                };
 
-                    if (profileDir.find("/profiles/") == std::string::npos)
+                if (store->isStorePath(userEnv.string()) && hasSuffix(userEnv.string(), "user-environment")) {
+                    while (noContainsProfiles() && std::filesystem::is_symlink(profileDir))
+                        profileDir = fs::weakly_canonical(
+                            profileDir.parent_path() / fs::read_symlink(profileDir));
+
+                    if (noContainsProfiles())
                         dirs.insert(dir);
                 }
             } catch (SystemError &) {

src/nix/develop.cc

@@ -415,7 +415,7 @@ struct Common : InstallableCommand, MixProfile
 
         if (buildEnvironment.providesStructuredAttrs()) {
             fixupStructuredAttrs(
-                PATHNG_LITERAL("sh"),
+                OS_STR("sh"),
                 "NIX_ATTRS_SH_FILE",
                 buildEnvironment.getAttrsSH(),
                 rewrites,
@@ -423,7 +423,7 @@ struct Common : InstallableCommand, MixProfile
                 tmpDir
             );
             fixupStructuredAttrs(
-                PATHNG_LITERAL("json"),
+                OS_STR("json"),
                 "NIX_ATTRS_JSON_FILE",
                 buildEnvironment.getAttrsJSON(),
                 rewrites,
@@ -447,7 +447,7 @@ struct Common : InstallableCommand, MixProfile
         const BuildEnvironment & buildEnvironment,
         const std::filesystem::path & tmpDir)
     {
-        auto targetFilePath = tmpDir / PATHNG_LITERAL(".attrs.");
+        auto targetFilePath = tmpDir / OS_STR(".attrs.");
         targetFilePath += ext;
 
         writeFile(targetFilePath.string(), content);

src/nix/env.cc

@@ -5,6 +5,7 @@
 #include "eval.hh"
 #include "run.hh"
 #include "strings.hh"
+#include "executable-path.hh"
 
 using namespace nix;
 
@@ -95,10 +96,10 @@ struct CmdShell : InstallablesCommand, MixEnvironment
         }
 
         // TODO: split losslessly; empty means .
-        auto unixPath = tokenizeString<Strings>(getEnv("PATH").value_or(""), ":");
+        auto unixPath = ExecutablePath::load();
-        unixPath.insert(unixPath.begin(), pathAdditions.begin(), pathAdditions.end());
+        unixPath.directories.insert(unixPath.directories.begin(), pathAdditions.begin(), pathAdditions.end());
-        auto unixPathString = concatStringsSep(":", unixPath);
+        auto unixPathString = unixPath.render();
-        setEnv("PATH", unixPathString.c_str());
+        setEnvOs(OS_STR("PATH"), unixPathString.c_str());
 
         Strings args;
         for (auto & arg : command)

src/nix/search.cc

@@ -10,6 +10,7 @@
 #include "eval-cache.hh"
 #include "attr-path.hh"
 #include "hilite.hh"
+#include "strings-inline.hh"
 
 #include <regex>
 #include <fstream>

src/nix/upgrade-nix.cc

@@ -8,6 +8,7 @@
 #include "attr-path.hh"
 #include "names.hh"
 #include "progress-bar.hh"
+#include "executable-path.hh"
 
 using namespace nix;
 
@@ -102,23 +103,17 @@ struct CmdUpgradeNix : MixDryRun, StoreCommand
     /* Return the profile in which Nix is installed. */
     Path getProfileDir(ref<Store> store)
     {
-        Path where;
+        auto whereOpt = ExecutablePath::load().find(OS_STR("nix-env"));
-
+        if (!whereOpt)
-        for (auto & dir : tokenizeString<Strings>(getEnv("PATH").value_or(""), ":"))
-            if (pathExists(dir + "/nix-env")) {
-                where = dir;
-                break;
-            }
-
-        if (where == "")
             throw Error("couldn't figure out how Nix is installed, so I can't upgrade it");
+        auto & where = *whereOpt;
 
         printInfo("found Nix in '%s'", where);
 
-        if (hasPrefix(where, "/run/current-system"))
+        if (hasPrefix(where.string(), "/run/current-system"))
             throw Error("Nix on NixOS must be upgraded via 'nixos-rebuild'");
 
-        Path profileDir = dirOf(where);
+        Path profileDir = where.parent_path().string();
 
         // Resolve profile to /nix/var/nix/profiles/<name> link.
         while (canonPath(profileDir).find("/profiles/") == std::string::npos && std::filesystem::is_symlink(profileDir))
@@ -128,7 +123,7 @@ struct CmdUpgradeNix : MixDryRun, StoreCommand
 
         Path userEnv = canonPath(profileDir, true);
 
-        if (baseNameOf(where) != "bin" ||
+        if (where.filename() != "bin" ||
             !hasSuffix(userEnv, "user-environment"))
             throw Error("directory '%s' does not appear to be part of a Nix profile", where);
 

tests/unit/libutil/executable-path.cc

@@ -0,0 +1,64 @@
+#include <gtest/gtest.h>
+
+#include "executable-path.hh"
+
+namespace nix {
+
+#ifdef WIN32
+#  define PATH_VAR_SEP L";"
+#else
+#  define PATH_VAR_SEP ":"
+#endif
+
+#define PATH_ENV_ROUND_TRIP(NAME, STRING_LIT, CXX_LIT) \
+    TEST(ExecutablePath, NAME)                         \
+    {                                                  \
+        OsString s = STRING_LIT;                       \
+        auto v = ExecutablePath::parse(s);             \
+        EXPECT_EQ(v, (ExecutablePath CXX_LIT));        \
+        auto s2 = v.render();                          \
+        EXPECT_EQ(s2, s);                              \
+    }
+
+PATH_ENV_ROUND_TRIP(emptyRoundTrip, OS_STR(""), ({}))
+
+PATH_ENV_ROUND_TRIP(
+    oneElemRoundTrip,
+    OS_STR("/foo"),
+    ({
+        OS_STR("/foo"),
+    }))
+
+PATH_ENV_ROUND_TRIP(
+    twoElemsRoundTrip,
+    OS_STR("/foo" PATH_VAR_SEP "/bar"),
+    ({
+        OS_STR("/foo"),
+        OS_STR("/bar"),
+    }))
+
+PATH_ENV_ROUND_TRIP(
+    threeElemsRoundTrip,
+    OS_STR("/foo" PATH_VAR_SEP "." PATH_VAR_SEP "/bar"),
+    ({
+        OS_STR("/foo"),
+        OS_STR("."),
+        OS_STR("/bar"),
+    }))
+
+TEST(ExecutablePath, elementyElemNormalize)
+{
+    auto v = ExecutablePath::parse(PATH_VAR_SEP PATH_VAR_SEP PATH_VAR_SEP);
+    EXPECT_EQ(
+        v,
+        (ExecutablePath{{
+            OS_STR("."),
+            OS_STR("."),
+            OS_STR("."),
+            OS_STR("."),
+        }}));
+    auto s2 = v.render();
+    EXPECT_EQ(s2, OS_STR("." PATH_VAR_SEP "." PATH_VAR_SEP "." PATH_VAR_SEP "."));
+}
+
+}

tests/unit/libutil/meson.build

@@ -52,6 +52,7 @@ sources = files(
   'closure.cc',
   'compression.cc',
   'config.cc',
+  'executable-path.cc',
   'file-content-address.cc',
   'git.cc',
   'hash.cc',

tests/unit/libutil/strings.cc

@@ -1,4 +1,5 @@
 #include <gtest/gtest.h>
+#include <rapidcheck/gtest.h>
 
 #include "strings.hh"
 
@@ -231,4 +232,117 @@ TEST(tokenizeString, tokenizeSepEmpty)
     ASSERT_EQ(tokenizeString<Strings>(s, ","), expected);
 }
 
+/* ----------------------------------------------------------------------------
+ * splitString
+ * --------------------------------------------------------------------------*/
+
+TEST(splitString, empty)
+{
+    Strings expected = {""};
+
+    ASSERT_EQ(splitString<Strings>("", " \t\n\r"), expected);
+}
+
+TEST(splitString, oneSep)
+{
+    Strings expected = {"", ""};
+
+    ASSERT_EQ(splitString<Strings>(" ", " \t\n\r"), expected);
+}
+
+TEST(splitString, twoSep)
+{
+    Strings expected = {"", "", ""};
+
+    ASSERT_EQ(splitString<Strings>(" \n", " \t\n\r"), expected);
+}
+
+TEST(splitString, tokenizeSpacesWithSpaces)
+{
+    auto s = "foo bar baz";
+    Strings expected = {"foo", "bar", "baz"};
+
+    ASSERT_EQ(splitString<Strings>(s, " \t\n\r"), expected);
+}
+
+TEST(splitString, tokenizeTabsWithDefaults)
+{
+    auto s = "foo\tbar\tbaz";
+    // Using it like this is weird, but shows the difference with tokenizeString, which also has this test
+    Strings expected = {"foo", "bar", "baz"};
+
+    ASSERT_EQ(splitString<Strings>(s, " \t\n\r"), expected);
+}
+
+TEST(splitString, tokenizeTabsSpacesWithDefaults)
+{
+    auto s = "foo\t bar\t baz";
+    // Using it like this is weird, but shows the difference with tokenizeString, which also has this test
+    Strings expected = {"foo", "", "bar", "", "baz"};
+
+    ASSERT_EQ(splitString<Strings>(s, " \t\n\r"), expected);
+}
+
+TEST(splitString, tokenizeTabsSpacesNewlineWithDefaults)
+{
+    auto s = "foo\t\n bar\t\n baz";
+    // Using it like this is weird, but shows the difference with tokenizeString, which also has this test
+    Strings expected = {"foo", "", "", "bar", "", "", "baz"};
+
+    ASSERT_EQ(splitString<Strings>(s, " \t\n\r"), expected);
+}
+
+TEST(splitString, tokenizeTabsSpacesNewlineRetWithDefaults)
+{
+    auto s = "foo\t\n\r bar\t\n\r baz";
+    // Using it like this is weird, but shows the difference with tokenizeString, which also has this test
+    Strings expected = {"foo", "", "", "", "bar", "", "", "", "baz"};
+
+    ASSERT_EQ(splitString<Strings>(s, " \t\n\r"), expected);
+
+    auto s2 = "foo \t\n\r bar \t\n\r baz";
+    Strings expected2 = {"foo", "", "", "", "", "bar", "", "", "", "", "baz"};
+
+    ASSERT_EQ(splitString<Strings>(s2, " \t\n\r"), expected2);
+}
+
+TEST(splitString, tokenizeWithCustomSep)
+{
+    auto s = "foo\n,bar\n,baz\n";
+    Strings expected = {"foo\n", "bar\n", "baz\n"};
+
+    ASSERT_EQ(splitString<Strings>(s, ","), expected);
+}
+
+TEST(splitString, tokenizeSepAtStart)
+{
+    auto s = ",foo,bar,baz";
+    Strings expected = {"", "foo", "bar", "baz"};
+
+    ASSERT_EQ(splitString<Strings>(s, ","), expected);
+}
+
+TEST(splitString, tokenizeSepAtEnd)
+{
+    auto s = "foo,bar,baz,";
+    Strings expected = {"foo", "bar", "baz", ""};
+
+    ASSERT_EQ(splitString<Strings>(s, ","), expected);
+}
+
+TEST(splitString, tokenizeSepEmpty)
+{
+    auto s = "foo,,baz";
+    Strings expected = {"foo", "", "baz"};
+
+    ASSERT_EQ(splitString<Strings>(s, ","), expected);
+}
+
+// concatStringsSep sep . splitString sep = id   if sep is 1 char
+RC_GTEST_PROP(splitString, recoveredByConcatStringsSep, (const std::string & s))
+{
+    RC_ASSERT(concatStringsSep("/", splitString<Strings>(s, "/")) == s);
+    RC_ASSERT(concatStringsSep("a", splitString<Strings>(s, "a")) == s);
+}
+
 } // namespace nix