Factor out lookupExecutable and other PATH improvments

This ended up motivating a good deal of other infra improvements in
order to get Windows right.

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$''

src/libexpr/eval.cc

@@ -2860,8 +2860,10 @@ void EvalState::printStatistics()
     topObj["cpuTime"] = cpuTime;
 #endif
     topObj["time"] = {
+#ifndef _WIN32 // TODO implement
         {"cpu", cpuTime},
-#ifdef HAVE_BOEHMGC
+#endif
+#if HAVE_BOEHMGC
         {GC_is_incremental_mode() ? "gcNonIncremental" : "gc", gcFullOnlyTime},
         {GC_is_incremental_mode() ? "gcNonIncrementalFraction" : "gcFraction", gcFullOnlyTime / cpuTime},
 #endif

src/libstore/gc.cc

@@ -891,7 +891,7 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results)
 
 void LocalStore::autoGC(bool sync)
 {
-#ifdef HAVE_STATVFS
+#if HAVE_STATVFS
     static auto fakeFreeSpaceFile = getEnv("_NIX_TEST_FREE_SPACE_FILE");
 
     auto getAvail = [this]() -> uint64_t {

src/libutil/environment-variables.cc

@@ -1,20 +1,23 @@
 #include "util.hh"
 #include "environment-variables.hh"
 
-extern char * * environ __attribute__((weak));
+extern char ** environ __attribute__((weak));
 
 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 coversions.
+ */
+std::optional<OsString> getEnvNative(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 coversions.
+ */
+int setEnvNative(const OsString & name, const OsString & value);
+
 /**
  * Clear the environment.
  */

src/libutil/executable-path.cc

@@ -0,0 +1,67 @@
+#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_separator =
+#ifdef WIN32
+    L";"
+#else
+    ":"
+#endif
+    ;
+
+std::vector<fs::path> parsePATH()
+{
+    return parsePATH(getEnvNative(PATHNG_LITERAL("PATH")).value_or(PATHNG_LITERAL("")));
+}
+
+std::vector<fs::path> parsePATH(const OsString & path)
+{
+    auto strings = basicTokenizeString<std::list<OsString>, OsString::value_type>(path, pATH_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{std::move(str)}; });
+
+    return ret;
+}
+
+OsString renderPATH(const std::vector<fs::path> & path)
+{
+    std::vector<PathViewNG> path2;
+    for (auto && p : path)
+        path2.push_back(p.native());
+    return basicConcatStringsSep(pATH_separator, path2);
+}
+
+std::optional<fs::path> lookupExecutable(const fs::path & exe)
+{
+    // Check if 'exe' contains any directory separators
+    if (auto pos = NativePathTrait<char>::rfindPathSep(exe.native()); pos != exe.native().npos) {
+        if (isExecutable(exe))
+            return std::filesystem::canonical(exe);
+        else
+            return std::nullopt;
+    }
+
+    for (auto & dir : parsePATH()) {
+        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,37 @@
+#pragma once
+///@file
+
+#include "file-system.hh"
+
+namespace nix {
+
+/**
+ * Parse the `PATH` environment variable into a list of paths.
+ *
+ * On Unix we split on `:`, on Windows we split on `;`.
+ */
+std::vector<std::filesystem::path> parsePATH();
+
+/**
+ * Use the given string instead of the actual environment variable.
+ */
+std::vector<std::filesystem::path> parsePATH(const OsString & path);
+
+/**
+ * Opposite of `parsePATH`
+ */
+OsString renderPATH(const std::vector<std::filesystem::path> & path);
+
+/**
+ * Lookup an executable.
+ *
+ * If it is just a name, lookup in the `PATH` environment variable. Otherwise
+ * (if it is an absolute path or a relative path containing a directory
+ * separator) keep as is.
+ *
+ * In either case, then canonicalize the result if the path exists and is
+ * executable, otherwise return `std::nulllopt`.
+ */
+std::optional<std::filesystem::path> lookupExecutable(const std::filesystem::path & exe);
+
+}

src/libutil/file-path-impl.hh

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

src/libutil/file-path.hh

@@ -8,6 +8,17 @@
 
 namespace nix {
 
+/**
+ * Named because it is similar to the Rust type, except it is in the
+ * native encoding not WTF-8.
+ */
+using OsString = std::filesystem::path::string_type;
+
+/**
+ * `std::string_view` counterpart for `OsString`.
+ */
+using OsStringView = std::basic_string_view<OsString::value_type>;
+
 /**
  * Paths are just `std::filesystem::path`s.
  *
@@ -22,18 +33,18 @@ 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)
-        : std::basic_string_view<std::filesystem::path::value_type>(path)
+    PathViewNG(const OsString & path)
+        : OsStringView{path}
     { }
 
     const string_view & native() const { return *this; }
@@ -42,7 +53,7 @@ struct PathViewNG : std::basic_string_view<std::filesystem::path::value_type>
 
 std::string os_string_to_string(PathViewNG::string_view path);
 
-std::filesystem::path::string_type string_to_os_string(std::string_view s);
+OsString string_to_os_string(std::string_view s);
 
 std::optional<std::filesystem::path> maybePath(PathView path);
 

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<NativePathTrait<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 = NativePathTrait<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 && NativePathTrait<char>::isPathSep(path[last]))
         last -= 1;
 
-    auto pos = NativePathTrait::rfindPathSep(path, last);
+    auto pos = NativePathTrait<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,14 @@ void moveFile(const Path & oldName, const Path & newName)
 
 //////////////////////////////////////////////////////////////////////
 
+bool isExecutable(const fs::path & exe) {
+    return access(exe.string().c_str(),
+#ifdef WIN32
+        0 // TODO do better
+#else
+        X_OK
+#endif
+        ) == 0;
+}
+
 }

src/libutil/file-system.hh

@@ -263,6 +263,11 @@ std::pair<AutoCloseFD, Path> createTempFile(const Path & prefix = "nix");
  */
 Path defaultTempDir();
 
+/**
+ * Check whether the file is executable.
+ */
+bool isExecutable(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',

src/libutil/strings-inline.hh

@@ -4,19 +4,19 @@
 
 namespace nix {
 
-template<class C>
-std::string concatStringsSep(const std::string_view sep, const C & ss)
+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;
     // need a cast to string_view since this is also called with Symbols
     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) {
@@ -28,4 +28,10 @@ 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);
+}
+
 } // namespace nix