From f93af1db1fbe8765d547d94b51fb68b2f40c3e8e Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Tue, 17 Dec 2024 16:25:00 +0100 Subject: [PATCH] libutil: make SourcePath::path private use canonical() to get the disk path, to_string() to get the string form. Change-Id: I95bb6df53356f30290b487d1cca0aa2fb37249ed --- lix/legacy/nix-env.cc | 2 +- lix/legacy/nix-instantiate.cc | 2 +- lix/libcmd/editor-for.cc | 2 +- lix/libcmd/repl.cc | 2 +- lix/libexpr/attr-path.cc | 2 +- lix/libexpr/eval-cache.cc | 4 ++-- lix/libexpr/eval.cc | 12 ++++++------ lix/libexpr/parser/parser-impl1.inc.cc | 2 +- lix/libexpr/primops.cc | 20 ++++++++++---------- lix/libexpr/value.cc | 2 +- lix/libexpr/value.hh | 2 +- lix/libfetchers/fetch-to-store.cc | 4 ++-- lix/libutil/source-path.hh | 4 ++++ tests/unit/libcmd/args.cc | 8 ++++---- 14 files changed, 36 insertions(+), 32 deletions(-) diff --git a/lix/legacy/nix-env.cc b/lix/legacy/nix-env.cc index 97b59a7ab..079613cbb 100644 --- a/lix/legacy/nix-env.cc +++ b/lix/legacy/nix-env.cc @@ -146,7 +146,7 @@ static void getAllExprs(Evaluator & state, } /* Load the expression on demand. */ auto vArg = state.mem.allocValue(); - vArg->mkString(path2.path.abs()); + vArg->mkString(path2.canonical().abs()); if (seen.size() == maxAttrs) throw Error("too many Nix expressions in directory '%1%'", path); attrs.alloc(attrName).mkApp(&state.builtins.get("import"), vArg); diff --git a/lix/legacy/nix-instantiate.cc b/lix/legacy/nix-instantiate.cc index 6127ce27b..57653f8ab 100644 --- a/lix/legacy/nix-instantiate.cc +++ b/lix/legacy/nix-instantiate.cc @@ -168,7 +168,7 @@ static int main_nix_instantiate(std::string programName, Strings argv) if (findFile) { for (auto & i : files) { auto p = evaluator->paths.findFile(i); - std::cout << p.path.abs() << std::endl; + std::cout << p.canonical().abs() << std::endl; } return 0; } diff --git a/lix/libcmd/editor-for.cc b/lix/libcmd/editor-for.cc index 5eecd27d2..68ba5954a 100644 --- a/lix/libcmd/editor-for.cc +++ b/lix/libcmd/editor-for.cc @@ -15,7 +15,7 @@ Strings editorFor(const SourcePath & file, uint32_t line) editor.find("vim") != std::string::npos || editor.find("kak") != std::string::npos)) args.push_back(fmt("+%d", line)); - args.push_back(file.path.abs()); + args.push_back(file.canonical().abs()); return args; } diff --git a/lix/libcmd/repl.cc b/lix/libcmd/repl.cc index d1534a3d8..238565428 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().path.abs())) { + if (!evaluator.store->isInStore(path.resolveSymlinks().canonical().abs())) { state.resetFileCache(); reloadFiles(); } diff --git a/lix/libexpr/attr-path.cc b/lix/libexpr/attr-path.cc index 926e7c9fe..0aa62d355 100644 --- a/lix/libexpr/attr-path.cc +++ b/lix/libexpr/attr-path.cc @@ -178,7 +178,7 @@ std::pair findPackageFilename(EvalState & state, Value & v NixStringContext context; auto path = state.coerceToPath(noPos, *v2, context, "while evaluating the 'meta.position' attribute of a derivation"); - auto fn = path.path.abs(); + auto fn = path.canonical().abs(); auto fail = [fn]() { throw ParseError("cannot parse 'meta.position' attribute '%s'", fn); diff --git a/lix/libexpr/eval-cache.cc b/lix/libexpr/eval-cache.cc index 443177afd..2ca311dbf 100644 --- a/lix/libexpr/eval-cache.cc +++ b/lix/libexpr/eval-cache.cc @@ -439,8 +439,8 @@ Value & AttrCursor::forceValue(EvalState & state) cachedValue = {root->db->setString(getKey(), v.string.s, v.string.context), string_t{v.string.s, {}}}; else if (v.type() == nPath) { - auto path = v.path().path; - cachedValue = {root->db->setString(getKey(), path.abs()), string_t{path.abs(), {}}}; + auto path = v.path().canonical().abs(); + cachedValue = {root->db->setString(getKey(), path), string_t{path, {}}}; } else if (v.type() == nBool) cachedValue = {root->db->setBool(getKey(), v.boolean), v.boolean}; diff --git a/lix/libexpr/eval.cc b/lix/libexpr/eval.cc index dae7daafc..4dee03c79 100644 --- a/lix/libexpr/eval.cc +++ b/lix/libexpr/eval.cc @@ -383,7 +383,7 @@ SourcePath EvalPaths::checkSourcePath(const SourcePath & path_) { if (!allowedPaths) return path_; - auto i = resolvedPaths.find(path_.path.abs()); + auto i = resolvedPaths.find(path_.canonical().abs()); if (i != resolvedPaths.end()) return i->second; @@ -393,7 +393,7 @@ SourcePath EvalPaths::checkSourcePath(const SourcePath & path_) * attacker can't append ../../... to a path that would be in allowedPaths * and thus leak symlink targets. */ - Path abspath = canonPath(path_.path.abs()); + Path abspath = canonPath(path_.canonical().abs()); if (abspath.starts_with(corepkgsPrefix)) return CanonPath(abspath); @@ -416,8 +416,8 @@ SourcePath EvalPaths::checkSourcePath(const SourcePath & path_) SourcePath path = CanonPath(canonPath(abspath, true)); for (auto & i : *allowedPaths) { - if (isDirOrInDir(path.path.abs(), i)) { - resolvedPaths.insert_or_assign(path_.path.abs(), path); + if (isDirOrInDir(path.canonical().abs(), i)) { + resolvedPaths.insert_or_assign(path_.canonical().abs(), path); return path; } } @@ -2339,7 +2339,7 @@ BackedStringView EvalState::coerceToString( StorePath EvalPaths::copyPathToStore(NixStringContext & context, const SourcePath & path, RepairFlag repair) { - if (nix::isDerivation(path.path.abs())) + if (nix::isDerivation(path.canonical().abs())) errors.make("file names are not allowed to end in '%1%'", drvExtension).debugThrow(); auto i = srcToStore.find(path); @@ -2672,7 +2672,7 @@ SourcePath resolveExprPath(SourcePath path) if (++followCount >= maxFollow) throw Error("too many symbolic links encountered while traversing the path '%s'", path); if (path.lstat().type != InputAccessor::tSymlink) break; - path = {CanonPath(path.readLink(), path.path.parent().value_or(CanonPath::root))}; + path = {CanonPath(path.readLink(), path.canonical().parent().value_or(CanonPath::root))}; } /* If `path' refers to a directory, append `/default.nix'. */ diff --git a/lix/libexpr/parser/parser-impl1.inc.cc b/lix/libexpr/parser/parser-impl1.inc.cc index efea9619d..7f8a9bb88 100644 --- a/lix/libexpr/parser/parser-impl1.inc.cc +++ b/lix/libexpr/parser/parser-impl1.inc.cc @@ -605,7 +605,7 @@ template<> struct BuildAST : BuildAST struct BuildAST { static void apply(const auto & in, StringState & s, State & ps) { - Path path(absPath(in.string(), ps.basePath.path.abs())); + Path path(absPath(in.string(), ps.basePath.canonical().abs())); /* add back in the trailing '/' to the first segment */ if (in.string_view().ends_with('/') && in.size() > 1) path += "/"; diff --git a/lix/libexpr/primops.cc b/lix/libexpr/primops.cc index d88a5525a..4bf9a7fb9 100644 --- a/lix/libexpr/primops.cc +++ b/lix/libexpr/primops.cc @@ -131,7 +131,7 @@ static SourcePath realisePath(EvalState & state, const PosIdx pos, Value & v, co try { StringMap rewrites = state.ctx.paths.realiseContext(context); - auto realPath = CanonPath(state.ctx.paths.toRealPath(rewriteStrings(path.path.abs(), rewrites), context)); + auto realPath = CanonPath(state.ctx.paths.toRealPath(rewriteStrings(path.canonical().abs(), rewrites), context)); return flags.checkForPureEval ? state.ctx.paths.checkSourcePath(realPath) @@ -176,7 +176,7 @@ static void mkOutputString( static void import(EvalState & state, const PosIdx pos, Value & vPath, Value * vScope, Value & v) { auto path = realisePath(state, pos, vPath); - auto path2 = path.path.abs(); + auto path2 = path.canonical().abs(); // FIXME auto isValidDerivationInStore = [&]() -> std::optional { @@ -283,7 +283,7 @@ void prim_importNative(EvalState & state, const PosIdx pos, Value * * args, Valu std::string sym(state.forceStringNoCtx(*args[1], pos, "while evaluating the second argument passed to builtins.importNative")); - void *handle = dlopen(path.path.c_str(), RTLD_LAZY | RTLD_LOCAL); + void *handle = dlopen(path.canonical().c_str(), RTLD_LAZY | RTLD_LOCAL); if (!handle) state.ctx.errors.make("could not open '%1%': %2%", path, dlerror()).debugThrow(); @@ -1173,7 +1173,7 @@ static void prim_storePath(EvalState & state, const PosIdx pos, Value * * args, ).atPos(pos).debugThrow(); NixStringContext context; - auto path = state.ctx.paths.checkSourcePath(state.coerceToPath(pos, *args[0], context, "while evaluating the first argument passed to builtins.storePath")).path; + auto path = state.ctx.paths.checkSourcePath(state.coerceToPath(pos, *args[0], context, "while evaluating the first argument passed to builtins.storePath")).canonical(); /* Resolve symlinks in ‘path’, unless ‘path’ itself is a symlink directly in the store. The latter condition is necessary so e.g. nix-push does the right thing. */ @@ -1241,7 +1241,7 @@ static void prim_dirOf(EvalState & state, const PosIdx pos, Value * * args, Valu state.forceValue(*args[0], pos); if (args[0]->type() == nPath) { auto path = args[0]->path(); - v.mkPath(path.path.isRoot() ? path : path.parent()); + v.mkPath(path.canonical().isRoot() ? path : path.parent()); } else { NixStringContext context; auto path = state.coerceToString(pos, *args[0], context, @@ -1263,9 +1263,9 @@ static void prim_readFile(EvalState & state, const PosIdx pos, Value * * args, V path ).atPos(pos).debugThrow(); StorePathSet refs; - if (state.ctx.store->isInStore(path.path.abs())) { + if (state.ctx.store->isInStore(path.canonical().abs())) { try { - refs = state.ctx.store->queryPathInfo(state.ctx.store->toStorePath(path.path.abs()).first)->references; + refs = state.ctx.store->queryPathInfo(state.ctx.store->toStorePath(path.canonical().abs()).first)->references; } catch (Error &) { // FIXME: should be InvalidPathError } // Re-scan references to filter down to just the ones that actually occur in the file. @@ -1513,7 +1513,7 @@ static void addPath( path = evalSettings.pureEval && expectedHash ? path - : state.ctx.paths.checkSourcePath(CanonPath(path)).path.abs(); + : state.ctx.paths.checkSourcePath(CanonPath(path)).canonical().abs(); PathFilter filter = filterFun ? ([&](const Path & path) { auto st = lstat(path); @@ -1569,7 +1569,7 @@ static void prim_filterSource(EvalState & state, const PosIdx pos, Value * * arg auto path = state.coerceToPath(pos, *args[1], context, "while evaluating the second argument (the path to filter) passed to builtins.filterSource"); state.forceFunction(*args[0], pos, "while evaluating the first argument passed to builtins.filterSource"); - addPath(state, pos, path.baseName(), path.path.abs(), args[0], FileIngestionMethod::Recursive, std::nullopt, v, context); + addPath(state, pos, path.baseName(), path.canonical().abs(), args[0], FileIngestionMethod::Recursive, std::nullopt, v, context); } static void prim_path(EvalState & state, const PosIdx pos, Value * * args, Value & v) @@ -1608,7 +1608,7 @@ static void prim_path(EvalState & state, const PosIdx pos, Value * * args, Value if (name.empty()) name = path->baseName(); - addPath(state, pos, name, path->path.abs(), filterFun, method, expectedHash, v, context); + addPath(state, pos, name, path->canonical().abs(), filterFun, method, expectedHash, v, context); } diff --git a/lix/libexpr/value.cc b/lix/libexpr/value.cc index ae62db3e8..5aea57096 100644 --- a/lix/libexpr/value.cc +++ b/lix/libexpr/value.cc @@ -100,7 +100,7 @@ void Value::mkStringMove(const char * s, const NixStringContext & context) void Value::mkPath(const SourcePath & path) { - mkPath(gcCopyStringIfNeeded(path.path.abs())); + *this = Value(NewValueAs::path, path); } } diff --git a/lix/libexpr/value.hh b/lix/libexpr/value.hh index b69049130..5d3a25695 100644 --- a/lix/libexpr/value.hh +++ b/lix/libexpr/value.hh @@ -347,7 +347,7 @@ public: /// dynamic (GC) allocation to do so. Value(path_t, SourcePath const & path) : internalType(tPath) - , _path(gcCopyStringIfNeeded(path.path.abs())) + , _path(gcCopyStringIfNeeded(path.canonical().abs())) , _path_pad(0) { } diff --git a/lix/libfetchers/fetch-to-store.cc b/lix/libfetchers/fetch-to-store.cc index 3fb53a437..ee7304338 100644 --- a/lix/libfetchers/fetch-to-store.cc +++ b/lix/libfetchers/fetch-to-store.cc @@ -18,8 +18,8 @@ StorePath fetchToStore( return settings.readOnlyMode - ? store.computeStorePathForPath(name, path.path.abs(), method, HashType::SHA256, filter2).first - : store.addToStore(name, path.path.abs(), method, HashType::SHA256, filter2, repair); + ? store.computeStorePathForPath(name, path.canonical().abs(), method, HashType::SHA256, filter2).first + : store.addToStore(name, path.canonical().abs(), method, HashType::SHA256, filter2, repair); } diff --git a/lix/libutil/source-path.hh b/lix/libutil/source-path.hh index 1c9868b08..1ad66e87f 100644 --- a/lix/libutil/source-path.hh +++ b/lix/libutil/source-path.hh @@ -42,8 +42,10 @@ enum class SymlinkResolution { */ struct SourcePath { +private: CanonPath path; +public: SourcePath(CanonPath path) : path(std::move(path)) { } @@ -103,6 +105,8 @@ struct SourcePath PathFilter & filter = defaultPathFilter) const { sink << nix::dumpPath(path.abs(), filter); } + const CanonPath & canonical() const { return path; } + std::string to_string() const { return path.abs(); } diff --git a/tests/unit/libcmd/args.cc b/tests/unit/libcmd/args.cc index 4283dd663..9ed41e28e 100644 --- a/tests/unit/libcmd/args.cc +++ b/tests/unit/libcmd/args.cc @@ -34,12 +34,12 @@ TEST(Arguments, lookupFileArg) { auto state = std::make_shared(searchPath, store, store); SourcePath const foundUnitData = lookupFileArg(*state, ""); - EXPECT_EQ(foundUnitData.path, canonDataPath); + EXPECT_EQ(foundUnitData.canonical(), canonDataPath); // lookupFileArg should not resolve if anything else is before or after it. SourcePath const yepEvenSpaces = lookupFileArg(*state, " "); - EXPECT_EQ(yepEvenSpaces.path, CanonPath::fromCwd(" ")); - EXPECT_EQ(lookupFileArg(*state, "/nixos").path, CanonPath::fromCwd("/nixos")); + EXPECT_EQ(yepEvenSpaces.canonical(), CanonPath::fromCwd(" ")); + EXPECT_EQ(lookupFileArg(*state, "/nixos").canonical(), CanonPath::fromCwd("/nixos")); try { lookupFileArg(*state, INVALID_CHANNEL); @@ -49,7 +49,7 @@ TEST(Arguments, lookupFileArg) { } SourcePath const normalFile = lookupFileArg(*state, unitDataPath); - EXPECT_EQ(normalFile.path, canonDataPath); + EXPECT_EQ(normalFile.canonical(), canonDataPath); } }