From 5af069b248e7e47233d0317d1a6851d35176b905 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Tue, 17 Dec 2024 16:25:00 +0100 Subject: [PATCH] libutil: remove SourcePath::resolveSymlinks in pure mode it is entirely useless. in impure mode it's mostly useless since the way in which it is used is either equivalent to not being run at all, or is equivalent to turning the following lstat into a stat. we add a stat method instead for all those who need final symlinks stat'd. Change-Id: I801886d18eb34b26e62b4c05d53318c6421a69bf --- lix/legacy/nix-env.cc | 4 +-- lix/libcmd/repl.cc | 2 +- lix/libexpr/primops.cc | 11 +++--- lix/libutil/source-path.cc | 70 ++++++++++++++------------------------ lix/libutil/source-path.hh | 42 +++++++---------------- 5 files changed, 47 insertions(+), 82 deletions(-) diff --git a/lix/legacy/nix-env.cc b/lix/legacy/nix-env.cc index 079613cbb..38664f63b 100644 --- a/lix/legacy/nix-env.cc +++ b/lix/legacy/nix-env.cc @@ -123,7 +123,7 @@ static void getAllExprs(Evaluator & state, InputAccessor::Stat st; try { - st = path2.resolveSymlinks().lstat(); + st = path2.stat(); } catch (Error &) { continue; // ignore dangling symlinks in ~/.nix-defexpr } @@ -162,7 +162,7 @@ static void getAllExprs(Evaluator & state, static void loadSourceExpr(EvalState & state, const SourcePath & path, Value & v) { - auto st = path.resolveSymlinks().lstat(); + auto st = path.stat(); if (isNixExpr(path, st)) state.evalFile(path, v); diff --git a/lix/libcmd/repl.cc b/lix/libcmd/repl.cc index 238565428..521d2898b 100644 --- a/lix/libcmd/repl.cc +++ b/lix/libcmd/repl.cc @@ -676,7 +676,7 @@ ProcessLineResult NixRepl::processLine(std::string line) // Reload right after exiting the editor if path is not in store // Store is immutable, so there could be no changes, so there's no need to reload - if (!evaluator.store->isInStore(path.resolveSymlinks().canonical().abs())) { + if (!evaluator.store->isInStore(canonPath(path.canonical().abs(), true))) { state.resetFileCache(); reloadFiles(); } diff --git a/lix/libexpr/primops.cc b/lix/libexpr/primops.cc index 4bf9a7fb9..e2bddcd15 100644 --- a/lix/libexpr/primops.cc +++ b/lix/libexpr/primops.cc @@ -1206,12 +1206,13 @@ static void prim_pathExists(EvalState & state, const PosIdx pos, Value * * args, || arg.str().ends_with("/.")); try { - auto checked = state - .ctx.paths - .checkSourcePath(path) - .resolveSymlinks(mustBeDir ? SymlinkResolution::Full : SymlinkResolution::Ancestors); + auto checked = state.ctx.paths.checkSourcePath(path); - auto st = checked.maybeLstat(); + // previously we fully resolved symlinks in the mustBeDir case or in pure eval + // mode (by accident, since checkSourcePath does this in that case), and up to + // the last component otherwise. this is equivalent to calling stat and lstat, + // respectively. (in neither case do intermediate symlinks affect the result.) + auto st = mustBeDir ? checked.maybeStat() : checked.maybeLstat(); auto exists = st && (!mustBeDir || st->type == InputAccessor::tDirectory); v.mkBool(exists); } catch (SysError & e) { diff --git a/lix/libutil/source-path.cc b/lix/libutil/source-path.cc index ce7c77b49..392794049 100644 --- a/lix/libutil/source-path.cc +++ b/lix/libutil/source-path.cc @@ -1,4 +1,5 @@ #include "lix/libutil/source-path.hh" +#include "file-system.hh" #include "lix/libutil/strings.hh" namespace nix { @@ -21,9 +22,8 @@ SourcePath SourcePath::parent() const return std::move(*p); } -InputAccessor::Stat SourcePath::lstat() const +static InputAccessor::Stat convertStat(const struct stat & st) { - auto st = nix::lstat(path.abs()); return InputAccessor::Stat { .type = S_ISREG(st.st_mode) ? InputAccessor::tRegular : @@ -34,12 +34,32 @@ InputAccessor::Stat SourcePath::lstat() const }; } +InputAccessor::Stat SourcePath::lstat() const +{ + return convertStat(nix::lstat(path.abs())); +} + std::optional SourcePath::maybeLstat() const { - // FIXME: merge these into one operation. - if (!pathExists()) - return {}; - return lstat(); + if (auto st = nix::maybeLstat(path.abs())) { + return convertStat(*st); + } else { + return std::nullopt; + } +} + +InputAccessor::Stat SourcePath::stat() const +{ + return convertStat(nix::stat(path.abs())); +} + +std::optional SourcePath::maybeStat() const +{ + if (auto st = nix::maybeStat(path.abs())) { + return convertStat(*st); + } else { + return std::nullopt; + } } InputAccessor::DirEntries SourcePath::readDirectory() const @@ -58,42 +78,4 @@ InputAccessor::DirEntries SourcePath::readDirectory() const return res; } -SourcePath SourcePath::resolveSymlinks(SymlinkResolution mode) const -{ - SourcePath res(CanonPath::root); - - int linksAllowed = 1024; - - std::list todo; - for (auto & c : path) - todo.push_back(std::string(c)); - - bool resolve_last = mode == SymlinkResolution::Full; - - while (!todo.empty()) { - auto c = *todo.begin(); - todo.pop_front(); - if (c == "" || c == ".") - ; - else if (c == "..") - res.path.pop(); - else { - res.path.push(c); - if (resolve_last || !todo.empty()) { - if (auto st = res.maybeLstat(); st && st->type == InputAccessor::tSymlink) { - if (!linksAllowed--) - throw Error("infinite symlink recursion in path '%s'", path); - auto target = res.readLink(); - res.path.pop(); - if (target.starts_with("/")) - res.path = CanonPath::root; - todo.splice(todo.begin(), tokenizeString>(target, "/")); - } - } - } - } - - return res; -} - } diff --git a/lix/libutil/source-path.hh b/lix/libutil/source-path.hh index 1ad66e87f..8267cf5e2 100644 --- a/lix/libutil/source-path.hh +++ b/lix/libutil/source-path.hh @@ -14,26 +14,6 @@ namespace nix { -/** - * Note there is a decent chance this type soon goes away because the problem is solved another way. - * See the discussion in https://github.com/NixOS/nix/pull/9985. - */ -enum class SymlinkResolution { - /** - * Resolve symlinks in the ancestors only. - * - * Only the last component of the result is possibly a symlink. - */ - Ancestors, - - /** - * Resolve symlinks fully, realpath(3)-style. - * - * No component of the result will be a symlink. - */ - Full, -}; - /** * An abstraction for accessing source files during * evaluation. Currently, it's just a wrapper around `CanonPath` that @@ -84,6 +64,18 @@ public: */ std::optional maybeLstat() const; + /** + * Return stats about this `SourcePath`, or throw an exception if + * it doesn't exist. Symlinks are resolved by this function. + */ + InputAccessor::Stat stat() const; + + /** + * Return stats about this `SourcePath`, or std::nullopt if it + * doesn't exist. Symlinks are resolved by this function. + */ + std::optional maybeStat() const; + /** * If this `SourcePath` denotes a directory (not a symlink), * return its directory entries; otherwise throw an error. @@ -138,16 +130,6 @@ public: { return path < x.path; } - - /** - * Resolve any symlinks in this `SourcePath` according to the - * given resolution mode. - * - * @param mode might only be a temporary solution for this. - * See the discussion in https://github.com/NixOS/nix/pull/9985. - */ - SourcePath resolveSymlinks( - SymlinkResolution mode = SymlinkResolution::Full) const; }; std::ostream & operator << (std::ostream & str, const SourcePath & path);