Skip to content

Commit

Permalink
Factor out lookupExecutable and other PATH improvments
Browse files Browse the repository at this point in the history
This ended up motivating a good deal of other infra improvements in
order to get Windows right:

- `OsString` to complement `std::filesystem::path`

- env var code for working with the underlying `OsString`s

- Rename `PATHNG_LITERAL` to `OS_STR`

- `NativePathTrait` renamed to `OsPathTrait`, given a character template
  parameter until NixOS#9205 is complete.

Split `tests.cc` matching split of `util.{cc,hh}` last year.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
  • Loading branch information
Ericson2314 and roberth committed Aug 7, 2024
1 parent 0836888 commit 6c861b9
Show file tree
Hide file tree
Showing 32 changed files with 616 additions and 97 deletions.
1 change: 1 addition & 0 deletions .clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,4 @@ AlwaysBreakBeforeMultilineStrings: true
IndentPPDirectives: AfterHash
PPIndentWidth: 2
BinPackArguments: false
BreakBeforeTernaryOperators: true
3 changes: 0 additions & 3 deletions maintainers/flake-module.nix
Original file line number Diff line number Diff line change
Expand Up @@ -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$''
Expand Down Expand Up @@ -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$''
Expand Down Expand Up @@ -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''
];
Expand Down
11 changes: 7 additions & 4 deletions src/libutil/environment-variables.cc
Original file line number Diff line number Diff line change
@@ -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;
}

Expand Down
11 changes: 11 additions & 0 deletions src/libutil/environment-variables.hh
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <optional>

#include "types.hh"
#include "file-path.hh"

namespace nix {

Expand All @@ -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 ""
Expand All @@ -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.
*/
Expand Down
79 changes: 79 additions & 0 deletions src/libutil/executable-path.cc
Original file line number Diff line number Diff line change
@@ -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
64 changes: 64 additions & 0 deletions src/libutil/executable-path.hh
Original file line number Diff line number Diff line change
@@ -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
9 changes: 3 additions & 6 deletions src/libutil/file-path-impl.hh
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,10 @@ struct WindowsPathTrait
};


/**
* @todo Revisit choice of `char` or `wchar_t` for `WindowsPathTrait`
* argument.
*/
using NativePathTrait =
template<typename CharT>
using OsPathTrait =
#ifdef _WIN32
WindowsPathTrait<char>
WindowsPathTrait<CharT>
#else
UnixPathTrait
#endif
Expand Down
25 changes: 6 additions & 19 deletions src/libutil/file-path.hh
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
#pragma once
///@file

#include <optional>
#include <filesystem>

#include "types.hh"
#include "os-string.hh"

namespace nix {

Expand All @@ -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)
: std::basic_string_view<std::filesystem::path::value_type>(path)
PathViewNG(const OsString & 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

}
23 changes: 18 additions & 5 deletions src/libutil/file-system.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -685,4 +685,17 @@ 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.
return std::filesystem::is_regular_file(exe)
&& access(exe.string().c_str(),
#ifdef WIN32
0 // TODO do better
#else
X_OK
#endif
) == 0;
}

}
6 changes: 6 additions & 0 deletions src/libutil/file-system.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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 exists AND is executable.
*/
bool isExecutableFileAmbient(const std::filesystem::path & exe);

/**
* Used in various places.
*/
Expand Down
Loading

0 comments on commit 6c861b9

Please sign in to comment.