From a6737b7e179fba2681393335c69c97df9bd5a2b0 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 5 Feb 2024 15:13:11 +0100 Subject: [PATCH] CanonPath, SourcePath: Change operator + to / This is less confusing and makes it more similar to std::filesystem::path. --- src/libexpr/eval.cc | 4 ++-- src/libexpr/primops.cc | 2 +- src/libfetchers/filtering-input-accessor.cc | 14 +++++++------- src/libfetchers/fs-input-accessor.cc | 2 +- src/libfetchers/git-utils.cc | 2 +- src/libfetchers/git.cc | 4 ++-- src/libfetchers/mercurial.cc | 2 +- src/libfetchers/path.cc | 2 +- src/libstore/binary-cache-store.cc | 4 ++-- src/libstore/local-fs-store.cc | 2 +- src/libstore/nar-accessor.cc | 2 +- src/libutil/archive.cc | 10 +++++----- src/libutil/canon-path.cc | 4 ++-- src/libutil/canon-path.hh | 4 ++-- src/libutil/fs-sink.cc | 2 +- src/libutil/git.cc | 2 +- src/libutil/source-path.cc | 8 ++++---- src/libutil/source-path.hh | 5 +++-- src/nix-env/nix-env.cc | 4 ++-- src/nix/ls.cc | 2 +- src/nix/run.cc | 2 +- src/nix/why-depends.cc | 2 +- tests/unit/libutil/canon-path.cc | 10 +++++----- 23 files changed, 48 insertions(+), 47 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 91fd3ddf8..bebc94873 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2689,14 +2689,14 @@ SourcePath resolveExprPath(SourcePath path) // Basic cycle/depth limit to avoid infinite loops. if (++followCount >= maxFollow) throw Error("too many symbolic links encountered while traversing the path '%s'", path); - auto p = path.parent().resolveSymlinks() + path.baseName(); + auto p = path.parent().resolveSymlinks() / path.baseName(); if (p.lstat().type != InputAccessor::tSymlink) break; path = {path.accessor, CanonPath(p.readLink(), path.path.parent().value_or(CanonPath::root))}; } /* If `path' refers to a directory, append `/default.nix'. */ if (path.resolveSymlinks().lstat().type == InputAccessor::tDirectory) - return path + "default.nix"; + return path / "default.nix"; return path; } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 1197b6e13..f8ded0cf8 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1816,7 +1816,7 @@ static void prim_readDir(EvalState & state, const PosIdx pos, Value * * args, Va // detailed node info quickly in this case we produce a thunk to // query the file type lazily. auto epath = state.allocValue(); - epath->mkPath(path + name); + epath->mkPath(path / name); if (!readFileType) readFileType = &state.getBuiltin("readFileType"); attr.mkApp(readFileType, epath); diff --git a/src/libfetchers/filtering-input-accessor.cc b/src/libfetchers/filtering-input-accessor.cc index 581ce3c1d..087a100af 100644 --- a/src/libfetchers/filtering-input-accessor.cc +++ b/src/libfetchers/filtering-input-accessor.cc @@ -5,26 +5,26 @@ namespace nix { std::string FilteringInputAccessor::readFile(const CanonPath & path) { checkAccess(path); - return next->readFile(prefix + path); + return next->readFile(prefix / path); } bool FilteringInputAccessor::pathExists(const CanonPath & path) { - return isAllowed(path) && next->pathExists(prefix + path); + return isAllowed(path) && next->pathExists(prefix / path); } std::optional FilteringInputAccessor::maybeLstat(const CanonPath & path) { checkAccess(path); - return next->maybeLstat(prefix + path); + return next->maybeLstat(prefix / path); } InputAccessor::DirEntries FilteringInputAccessor::readDirectory(const CanonPath & path) { checkAccess(path); DirEntries entries; - for (auto & entry : next->readDirectory(prefix + path)) { - if (isAllowed(path + entry.first)) + for (auto & entry : next->readDirectory(prefix / path)) { + if (isAllowed(path / entry.first)) entries.insert(std::move(entry)); } return entries; @@ -33,12 +33,12 @@ InputAccessor::DirEntries FilteringInputAccessor::readDirectory(const CanonPath std::string FilteringInputAccessor::readLink(const CanonPath & path) { checkAccess(path); - return next->readLink(prefix + path); + return next->readLink(prefix / path); } std::string FilteringInputAccessor::showPath(const CanonPath & path) { - return next->showPath(prefix + path); + return next->showPath(prefix / path); } void FilteringInputAccessor::checkAccess(const CanonPath & path) diff --git a/src/libfetchers/fs-input-accessor.cc b/src/libfetchers/fs-input-accessor.cc index c3d8d273c..46bc6b70d 100644 --- a/src/libfetchers/fs-input-accessor.cc +++ b/src/libfetchers/fs-input-accessor.cc @@ -48,7 +48,7 @@ struct FSInputAccessor : InputAccessor, PosixSourceAccessor CanonPath makeAbsPath(const CanonPath & path) { - return root + path; + return root / path; } std::optional getPhysicalPath(const CanonPath & path) override diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc index 382a363f0..1256a4c2c 100644 --- a/src/libfetchers/git-utils.cc +++ b/src/libfetchers/git-utils.cc @@ -295,7 +295,7 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this throw Error("getting working directory status: %s", git_error_last()->message); /* Get submodule info. */ - auto modulesFile = path + ".gitmodules"; + auto modulesFile = path / ".gitmodules"; if (pathExists(modulesFile.abs())) info.submodules = parseSubmodules(modulesFile); diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index f9a1cb1bc..26fe79596 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -319,7 +319,7 @@ struct GitInputScheme : InputScheme if (!repoInfo.isLocal) throw Error("cannot commit '%s' to Git repository '%s' because it's not a working tree", path, input.to_string()); - writeFile((CanonPath(repoInfo.url) + path).abs(), contents); + writeFile((CanonPath(repoInfo.url) / path).abs(), contents); auto result = runProgram(RunOptions { .program = "git", @@ -680,7 +680,7 @@ struct GitInputScheme : InputScheme std::map> mounts; for (auto & submodule : repoInfo.workdirInfo.submodules) { - auto submodulePath = CanonPath(repoInfo.url) + submodule.path; + auto submodulePath = CanonPath(repoInfo.url) / submodule.path; fetchers::Attrs attrs; attrs.insert_or_assign("type", "git"); attrs.insert_or_assign("url", submodulePath.abs()); diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index 9982389ab..55e2eae03 100644 --- a/src/libfetchers/mercurial.cc +++ b/src/libfetchers/mercurial.cc @@ -141,7 +141,7 @@ struct MercurialInputScheme : InputScheme if (!isLocal) throw Error("cannot commit '%s' to Mercurial repository '%s' because it's not a working tree", path, input.to_string()); - auto absPath = CanonPath(repoPath) + path; + auto absPath = CanonPath(repoPath) / path; writeFile(absPath.abs(), contents); diff --git a/src/libfetchers/path.cc b/src/libfetchers/path.cc index f9b973320..d3b0e475d 100644 --- a/src/libfetchers/path.cc +++ b/src/libfetchers/path.cc @@ -84,7 +84,7 @@ struct PathInputScheme : InputScheme std::string_view contents, std::optional commitMsg) const override { - writeFile((CanonPath(getAbsPath(input)) + path).abs(), contents); + writeFile((CanonPath(getAbsPath(input)) / path).abs(), contents); } CanonPath getAbsPath(const Input & input) const diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index ea1279e2e..189d1d305 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -235,14 +235,14 @@ ref BinaryCacheStore::addToStoreCommon( std::regex regex2("^[0-9a-f]{38}\\.debug$"); for (auto & [s1, _type] : narAccessor->readDirectory(buildIdDir)) { - auto dir = buildIdDir + s1; + auto dir = buildIdDir / s1; if (narAccessor->lstat(dir).type != SourceAccessor::tDirectory || !std::regex_match(s1, regex1)) continue; for (auto & [s2, _type] : narAccessor->readDirectory(dir)) { - auto debugPath = dir + s2; + auto debugPath = dir / s2; if (narAccessor->lstat(debugPath).type != SourceAccessor::tRegular || !std::regex_match(s2, regex2)) diff --git a/src/libstore/local-fs-store.cc b/src/libstore/local-fs-store.cc index 953f3a264..81c385ddb 100644 --- a/src/libstore/local-fs-store.cc +++ b/src/libstore/local-fs-store.cc @@ -28,7 +28,7 @@ struct LocalStoreAccessor : PosixSourceAccessor auto [storePath, rest] = store->toStorePath(path.abs()); if (requireValidPath && !store->isValidPath(storePath)) throw InvalidPath("path '%1%' is not a valid store path", store->printStorePath(storePath)); - return CanonPath(store->getRealStoreDir()) + storePath.to_string() + CanonPath(rest); + return CanonPath(store->getRealStoreDir()) / storePath.to_string() / CanonPath(rest); } std::optional maybeLstat(const CanonPath & path) override diff --git a/src/libstore/nar-accessor.cc b/src/libstore/nar-accessor.cc index b13e4c52c..cecf8148f 100644 --- a/src/libstore/nar-accessor.cc +++ b/src/libstore/nar-accessor.cc @@ -277,7 +277,7 @@ json listNar(ref accessor, const CanonPath & path, bool recurse) json &res2 = obj["entries"]; for (const auto & [name, type] : accessor->readDirectory(path)) { if (recurse) { - res2[name] = listNar(accessor, path + name, true); + res2[name] = listNar(accessor, path / name, true); } else res2[name] = json::object(); } diff --git a/src/libutil/archive.cc b/src/libutil/archive.cc index 6062392cd..b783b29e0 100644 --- a/src/libutil/archive.cc +++ b/src/libutil/archive.cc @@ -77,20 +77,20 @@ void SourceAccessor::dumpPath( std::string name(i.first); size_t pos = i.first.find(caseHackSuffix); if (pos != std::string::npos) { - debug("removing case hack suffix from '%s'", path + i.first); + debug("removing case hack suffix from '%s'", path / i.first); name.erase(pos); } if (!unhacked.emplace(name, i.first).second) throw Error("file name collision in between '%s' and '%s'", - (path + unhacked[name]), - (path + i.first)); + (path / unhacked[name]), + (path / i.first)); } else unhacked.emplace(i.first, i.first); for (auto & i : unhacked) - if (filter((path + i.first).abs())) { + if (filter((path / i.first).abs())) { sink << "entry" << "(" << "name" << i.first << "node"; - dump(path + i.second); + dump(path / i.second); sink << ")"; } } diff --git a/src/libutil/canon-path.cc b/src/libutil/canon-path.cc index 0a0f96a05..bf948be5d 100644 --- a/src/libutil/canon-path.cc +++ b/src/libutil/canon-path.cc @@ -63,7 +63,7 @@ void CanonPath::extend(const CanonPath & x) path += x.abs(); } -CanonPath CanonPath::operator + (const CanonPath & x) const +CanonPath CanonPath::operator / (const CanonPath & x) const { auto res = *this; res.extend(x); @@ -78,7 +78,7 @@ void CanonPath::push(std::string_view c) path += c; } -CanonPath CanonPath::operator + (std::string_view c) const +CanonPath CanonPath::operator / (std::string_view c) const { auto res = *this; res.push(c); diff --git a/src/libutil/canon-path.hh b/src/libutil/canon-path.hh index 997c8c731..fb2d9244b 100644 --- a/src/libutil/canon-path.hh +++ b/src/libutil/canon-path.hh @@ -190,14 +190,14 @@ public: /** * Concatenate two paths. */ - CanonPath operator + (const CanonPath & x) const; + CanonPath operator / (const CanonPath & x) const; /** * Add a path component to this one. It must not contain any slashes. */ void push(std::string_view c); - CanonPath operator + (std::string_view c) const; + CanonPath operator / (std::string_view c) const; /** * Check whether access to this path is allowed, which is the case diff --git a/src/libutil/fs-sink.cc b/src/libutil/fs-sink.cc index b6f8db592..95b6088da 100644 --- a/src/libutil/fs-sink.cc +++ b/src/libutil/fs-sink.cc @@ -34,7 +34,7 @@ void copyRecursive( sink.createDirectory(to); for (auto & [name, _] : accessor.readDirectory(from)) { copyRecursive( - accessor, from + name, + accessor, from / name, sink, to + "/" + name); break; } diff --git a/src/libutil/git.cc b/src/libutil/git.cc index 3b8c3ebac..5733531fa 100644 --- a/src/libutil/git.cc +++ b/src/libutil/git.cc @@ -259,7 +259,7 @@ Mode dump( { Tree entries; for (auto & [name, _] : accessor.readDirectory(path)) { - auto child = path + name; + auto child = path / name; if (!filter(child.abs())) continue; auto entry = hook(child); diff --git a/src/libutil/source-path.cc b/src/libutil/source-path.cc index d85b0b7fe..341daf39c 100644 --- a/src/libutil/source-path.cc +++ b/src/libutil/source-path.cc @@ -41,11 +41,11 @@ std::optional SourcePath::getPhysicalPath() const std::string SourcePath::to_string() const { return accessor->showPath(path); } -SourcePath SourcePath::operator+(const CanonPath & x) const -{ return {accessor, path + x}; } +SourcePath SourcePath::operator / (const CanonPath & x) const +{ return {accessor, path / x}; } -SourcePath SourcePath::operator+(std::string_view c) const -{ return {accessor, path + c}; } +SourcePath SourcePath::operator / (std::string_view c) const +{ return {accessor, path / c}; } bool SourcePath::operator==(const SourcePath & x) const { diff --git a/src/libutil/source-path.hh b/src/libutil/source-path.hh index bf5625ca5..bde07b08f 100644 --- a/src/libutil/source-path.hh +++ b/src/libutil/source-path.hh @@ -89,14 +89,15 @@ struct SourcePath /** * Append a `CanonPath` to this path. */ - SourcePath operator + (const CanonPath & x) const; + SourcePath operator / (const CanonPath & x) const; /** * Append a single component `c` to this path. `c` must not * contain a slash. A slash is implicitly added between this path * and `c`. */ - SourcePath operator+(std::string_view c) const; + SourcePath operator / (std::string_view c) const; + bool operator==(const SourcePath & x) const; bool operator!=(const SourcePath & x) const; bool operator<(const SourcePath & x) const; diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index d5b46c57a..dfc6e70eb 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -97,7 +97,7 @@ static bool isNixExpr(const SourcePath & path, struct InputAccessor::Stat & st) { return st.type == InputAccessor::tRegular - || (st.type == InputAccessor::tDirectory && (path + "default.nix").resolveSymlinks().pathExists()); + || (st.type == InputAccessor::tDirectory && (path / "default.nix").resolveSymlinks().pathExists()); } @@ -116,7 +116,7 @@ static void getAllExprs(EvalState & state, are implemented using profiles). */ if (i == "manifest.nix") continue; - auto path2 = (path + i).resolveSymlinks(); + auto path2 = (path / i).resolveSymlinks(); InputAccessor::Stat st; try { diff --git a/src/nix/ls.cc b/src/nix/ls.cc index 231456c9c..63f97f2d3 100644 --- a/src/nix/ls.cc +++ b/src/nix/ls.cc @@ -72,7 +72,7 @@ struct MixLs : virtual Args, MixJSON if (st.type == SourceAccessor::Type::tDirectory && !showDirectory) { auto names = accessor->readDirectory(curPath); for (auto & [name, type] : names) - showFile(curPath + name, relPath + "/" + name); + showFile(curPath / name, relPath + "/" + name); } else showFile(curPath, relPath); }; diff --git a/src/nix/run.cc b/src/nix/run.cc index 9bca5b9d0..e86837679 100644 --- a/src/nix/run.cc +++ b/src/nix/run.cc @@ -124,7 +124,7 @@ struct CmdShell : InstallablesCommand, MixEnvironment if (true) pathAdditions.push_back(store->printStorePath(path) + "/bin"); - auto propPath = CanonPath(store->printStorePath(path)) + "nix-support" + "propagated-user-env-packages"; + auto propPath = CanonPath(store->printStorePath(path)) / "nix-support" / "propagated-user-env-packages"; if (auto st = accessor->maybeLstat(propPath); st && st->type == SourceAccessor::tRegular) { for (auto & p : tokenizeString(accessor->readFile(propPath))) todo.push(store->parseStorePath(p)); diff --git a/src/nix/why-depends.cc b/src/nix/why-depends.cc index aecf65922..e299585ff 100644 --- a/src/nix/why-depends.cc +++ b/src/nix/why-depends.cc @@ -225,7 +225,7 @@ struct CmdWhyDepends : SourceExprCommand, MixOperateOnOptions if (st->type == SourceAccessor::Type::tDirectory) { auto names = accessor->readDirectory(p); for (auto & [name, type] : names) - visitPath(p + name); + visitPath(p / name); } else if (st->type == SourceAccessor::Type::tRegular) { diff --git a/tests/unit/libutil/canon-path.cc b/tests/unit/libutil/canon-path.cc index fc94ccc3d..bf11abe3e 100644 --- a/tests/unit/libutil/canon-path.cc +++ b/tests/unit/libutil/canon-path.cc @@ -80,29 +80,29 @@ namespace nix { { CanonPath p1("a//foo/bar//"); CanonPath p2("xyzzy/bla"); - ASSERT_EQ((p1 + p2).abs(), "/a/foo/bar/xyzzy/bla"); + ASSERT_EQ((p1 / p2).abs(), "/a/foo/bar/xyzzy/bla"); } { CanonPath p1("/"); CanonPath p2("/a/b"); - ASSERT_EQ((p1 + p2).abs(), "/a/b"); + ASSERT_EQ((p1 / p2).abs(), "/a/b"); } { CanonPath p1("/a/b"); CanonPath p2("/"); - ASSERT_EQ((p1 + p2).abs(), "/a/b"); + ASSERT_EQ((p1 / p2).abs(), "/a/b"); } { CanonPath p("/foo/bar"); - ASSERT_EQ((p + "x").abs(), "/foo/bar/x"); + ASSERT_EQ((p / "x").abs(), "/foo/bar/x"); } { CanonPath p("/"); - ASSERT_EQ((p + "foo" + "bar").abs(), "/foo/bar"); + ASSERT_EQ((p / "foo" / "bar").abs(), "/foo/bar"); } }