From c0dd05131e08102d560a737ff68c66bc8314f5fa Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 13 Jul 2020 16:19:37 +0200 Subject: [PATCH] toStorePath(): Return a StorePath and the suffix --- src/libexpr/eval.cc | 2 +- src/libexpr/primops.cc | 6 +++--- src/libstore/build.cc | 4 ++-- src/libstore/gc.cc | 30 ++++++++++++++------------- src/libstore/local-fs-store.cc | 6 +++--- src/libstore/path.cc | 2 -- src/libstore/remote-fs-accessor.cc | 33 +++++++++++++++--------------- src/libstore/remote-fs-accessor.hh | 6 +++--- src/libstore/store-api.cc | 10 ++++----- src/libstore/store-api.hh | 8 ++++---- src/nix/installables.cc | 14 ++++++------- 11 files changed, 60 insertions(+), 61 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index c1a9af9b2..58066fa48 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -366,7 +366,7 @@ EvalState::EvalState(const Strings & _searchPath, ref store) if (store->isInStore(r.second)) { StorePathSet closure; - store->computeFSClosure(store->parseStorePath(store->toStorePath(r.second)), closure); + store->computeFSClosure(store->toStorePath(r.second).first, closure); for (auto & path : closure) allowedPaths->insert(store->printStorePath(path)); } else diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index dec917b38..1c6fd5b33 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -883,10 +883,10 @@ static void prim_storePath(EvalState & state, const Pos & pos, Value * * args, V .hint = hintfmt("path '%1%' is not in the Nix store", path), .errPos = pos }); - Path path2 = state.store->toStorePath(path); + auto path2 = state.store->toStorePath(path).first; if (!settings.readOnlyMode) - state.store->ensurePath(state.store->parseStorePath(path2)); - context.insert(path2); + state.store->ensurePath(path2); + context.insert(state.store->printStorePath(path2)); mkString(v, path, context); } diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 0ef2f288f..96f7f2603 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2044,7 +2044,7 @@ void DerivationGoal::startBuilder() auto storePathS = *i++; if (!worker.store.isInStore(storePathS)) throw BuildError("'exportReferencesGraph' contains a non-store path '%1%'", storePathS); - auto storePath = worker.store.parseStorePath(worker.store.toStorePath(storePathS)); + auto storePath = worker.store.toStorePath(storePathS).first; /* Write closure info to . */ writeFile(tmpDir + "/" + fileName, @@ -2083,7 +2083,7 @@ void DerivationGoal::startBuilder() for (auto & i : dirsInChroot) try { if (worker.store.isInStore(i.second.source)) - worker.store.computeFSClosure(worker.store.parseStorePath(worker.store.toStorePath(i.second.source)), closure); + worker.store.computeFSClosure(worker.store.toStorePath(i.second.source).first, closure); } catch (InvalidPath & e) { } catch (Error & e) { throw Error("while processing 'sandbox-paths': %s", e.what()); diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index 57fb20845..aaed5c218 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -262,11 +262,13 @@ void LocalStore::findTempRoots(FDs & fds, Roots & tempRoots, bool censor) void LocalStore::findRoots(const Path & path, unsigned char type, Roots & roots) { auto foundRoot = [&](const Path & path, const Path & target) { - auto storePath = maybeParseStorePath(toStorePath(target)); - if (storePath && isValidPath(*storePath)) - roots[std::move(*storePath)].emplace(path); - else - printInfo("skipping invalid root from '%1%' to '%2%'", path, target); + try { + auto storePath = toStorePath(target).first; + if (isValidPath(storePath)) + roots[std::move(storePath)].emplace(path); + else + printInfo("skipping invalid root from '%1%' to '%2%'", path, target); + } catch (BadStorePath &) { } }; try { @@ -472,15 +474,15 @@ void LocalStore::findRuntimeRoots(Roots & roots, bool censor) for (auto & [target, links] : unchecked) { if (!isInStore(target)) continue; - Path pathS = toStorePath(target); - if (!isStorePath(pathS)) continue; - auto path = parseStorePath(pathS); - if (!isValidPath(path)) continue; - debug("got additional root '%1%'", pathS); - if (censor) - roots[path].insert(censored); - else - roots[path].insert(links.begin(), links.end()); + try { + auto path = toStorePath(target).first; + if (!isValidPath(path)) continue; + debug("got additional root '%1%'", printStorePath(path)); + if (censor) + roots[path].insert(censored); + else + roots[path].insert(links.begin(), links.end()); + } catch (BadStorePath &) { } } } diff --git a/src/libstore/local-fs-store.cc b/src/libstore/local-fs-store.cc index dd96d2578..2f1d9663a 100644 --- a/src/libstore/local-fs-store.cc +++ b/src/libstore/local-fs-store.cc @@ -20,9 +20,9 @@ struct LocalStoreAccessor : public FSAccessor Path toRealPath(const Path & path) { - Path storePath = store->toStorePath(path); - if (!store->isValidPath(store->parseStorePath(storePath))) - throw InvalidPath("path '%1%' is not a valid store path", storePath); + auto storePath = store->toStorePath(path).first; + if (!store->isValidPath(storePath)) + throw InvalidPath("path '%1%' is not a valid store path", store->printStorePath(storePath)); return store->getRealStoreDir() + std::string(path, store->storeDir.size()); } diff --git a/src/libstore/path.cc b/src/libstore/path.cc index b3d8ce95c..dc9dc3897 100644 --- a/src/libstore/path.cc +++ b/src/libstore/path.cc @@ -2,8 +2,6 @@ namespace nix { -MakeError(BadStorePath, Error); - static void checkName(std::string_view path, std::string_view name) { if (name.empty()) diff --git a/src/libstore/remote-fs-accessor.cc b/src/libstore/remote-fs-accessor.cc index bd698d781..2d02a181b 100644 --- a/src/libstore/remote-fs-accessor.cc +++ b/src/libstore/remote-fs-accessor.cc @@ -16,26 +16,26 @@ RemoteFSAccessor::RemoteFSAccessor(ref store, const Path & cacheDir) createDirs(cacheDir); } -Path RemoteFSAccessor::makeCacheFile(const Path & storePath, const std::string & ext) +Path RemoteFSAccessor::makeCacheFile(std::string_view hashPart, const std::string & ext) { assert(cacheDir != ""); - return fmt("%s/%s.%s", cacheDir, store->parseStorePath(storePath).hashPart(), ext); + return fmt("%s/%s.%s", cacheDir, hashPart, ext); } -void RemoteFSAccessor::addToCache(const Path & storePath, const std::string & nar, +void RemoteFSAccessor::addToCache(std::string_view hashPart, const std::string & nar, ref narAccessor) { - nars.emplace(storePath, narAccessor); + nars.emplace(hashPart, narAccessor); if (cacheDir != "") { try { std::ostringstream str; JSONPlaceholder jsonRoot(str); listNar(jsonRoot, narAccessor, "", true); - writeFile(makeCacheFile(storePath, "ls"), str.str()); + writeFile(makeCacheFile(hashPart, "ls"), str.str()); /* FIXME: do this asynchronously. */ - writeFile(makeCacheFile(storePath, "nar"), nar); + writeFile(makeCacheFile(hashPart, "nar"), nar); } catch (...) { ignoreException(); @@ -47,23 +47,22 @@ std::pair, Path> RemoteFSAccessor::fetch(const Path & path_) { auto path = canonPath(path_); - auto storePath = store->toStorePath(path); - std::string restPath = std::string(path, storePath.size()); + auto [storePath, restPath] = store->toStorePath(path); - if (!store->isValidPath(store->parseStorePath(storePath))) - throw InvalidPath("path '%1%' is not a valid store path", storePath); + if (!store->isValidPath(storePath)) + throw InvalidPath("path '%1%' is not a valid store path", store->printStorePath(storePath)); - auto i = nars.find(storePath); + auto i = nars.find(std::string(storePath.hashPart())); if (i != nars.end()) return {i->second, restPath}; StringSink sink; std::string listing; Path cacheFile; - if (cacheDir != "" && pathExists(cacheFile = makeCacheFile(storePath, "nar"))) { + if (cacheDir != "" && pathExists(cacheFile = makeCacheFile(storePath.hashPart(), "nar"))) { try { - listing = nix::readFile(makeCacheFile(storePath, "ls")); + listing = nix::readFile(makeCacheFile(storePath.hashPart(), "ls")); auto narAccessor = makeLazyNarAccessor(listing, [cacheFile](uint64_t offset, uint64_t length) { @@ -81,7 +80,7 @@ std::pair, Path> RemoteFSAccessor::fetch(const Path & path_) return buf; }); - nars.emplace(storePath, narAccessor); + nars.emplace(storePath.hashPart(), narAccessor); return {narAccessor, restPath}; } catch (SysError &) { } @@ -90,15 +89,15 @@ std::pair, Path> RemoteFSAccessor::fetch(const Path & path_) *sink.s = nix::readFile(cacheFile); auto narAccessor = makeNarAccessor(sink.s); - nars.emplace(storePath, narAccessor); + nars.emplace(storePath.hashPart(), narAccessor); return {narAccessor, restPath}; } catch (SysError &) { } } - store->narFromPath(store->parseStorePath(storePath), sink); + store->narFromPath(storePath, sink); auto narAccessor = makeNarAccessor(sink.s); - addToCache(storePath, *sink.s, narAccessor); + addToCache(storePath.hashPart(), *sink.s, narAccessor); return {narAccessor, restPath}; } diff --git a/src/libstore/remote-fs-accessor.hh b/src/libstore/remote-fs-accessor.hh index 4afb3be95..347cf5764 100644 --- a/src/libstore/remote-fs-accessor.hh +++ b/src/libstore/remote-fs-accessor.hh @@ -10,7 +10,7 @@ class RemoteFSAccessor : public FSAccessor { ref store; - std::map> nars; + std::map> nars; Path cacheDir; @@ -18,9 +18,9 @@ class RemoteFSAccessor : public FSAccessor friend class BinaryCacheStore; - Path makeCacheFile(const Path & storePath, const std::string & ext); + Path makeCacheFile(std::string_view hashPart, const std::string & ext); - void addToCache(const Path & storePath, const std::string & nar, + void addToCache(std::string_view hashPart, const std::string & nar, ref narAccessor); public: diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 0c6788b69..e67f4e359 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -21,15 +21,15 @@ bool Store::isInStore(const Path & path) const } -Path Store::toStorePath(const Path & path) const +std::pair Store::toStorePath(const Path & path) const { if (!isInStore(path)) throw Error("path '%1%' is not in the Nix store", path); Path::size_type slash = path.find('/', storeDir.size() + 1); if (slash == Path::npos) - return path; + return {parseStorePath(path), ""}; else - return Path(path, 0, slash); + return {parseStorePath(std::string_view(path).substr(0, slash)), path.substr(slash)}; } @@ -42,14 +42,14 @@ Path Store::followLinksToStore(std::string_view _path) const path = absPath(target, dirOf(path)); } if (!isInStore(path)) - throw NotInStore("path '%1%' is not in the Nix store", path); + throw BadStorePath("path '%1%' is not in the Nix store", path); return path; } StorePath Store::followLinksToStorePath(std::string_view path) const { - return parseStorePath(toStorePath(followLinksToStore(path))); + return toStorePath(followLinksToStore(path)).first; } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 0be0021f5..6f5e5f93a 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -31,7 +31,7 @@ MakeError(InvalidPath, Error); MakeError(Unsupported, Error); MakeError(SubstituteGone, Error); MakeError(SubstituterDisabled, Error); -MakeError(NotInStore, Error); +MakeError(BadStorePath, Error); class FSAccessor; @@ -317,9 +317,9 @@ public: the Nix store. */ bool isStorePath(std::string_view path) const; - /* Chop off the parts after the top-level store name, e.g., - /nix/store/abcd-foo/bar => /nix/store/abcd-foo. */ - Path toStorePath(const Path & path) const; + /* Split a path like /nix/store/-/ into + /nix/store/- and /. */ + std::pair toStorePath(const Path & path) const; /* Follow symlinks until we end up with a path in the Nix store. */ Path followLinksToStore(std::string_view path) const; diff --git a/src/nix/installables.cc b/src/nix/installables.cc index 708a0dc88..1b04b10d5 100644 --- a/src/nix/installables.cc +++ b/src/nix/installables.cc @@ -94,8 +94,8 @@ struct InstallableStorePath : Installable ref store; StorePath storePath; - InstallableStorePath(ref store, const Path & storePath) - : store(store), storePath(store->parseStorePath(storePath)) { } + InstallableStorePath(ref store, StorePath && storePath) + : store(store), storePath(std::move(storePath)) { } std::string what() override { return store->printStorePath(storePath); } @@ -228,11 +228,11 @@ static std::vector> parseInstallables( result.push_back(std::make_shared(cmd, s)); else if (s.find("/") != std::string::npos) { - - auto path = store->toStorePath(store->followLinksToStore(s)); - - if (store->isStorePath(path)) - result.push_back(std::make_shared(store, path)); + try { + result.push_back(std::make_shared( + store, + store->toStorePath(store->followLinksToStore(s)).first)); + } catch (BadStorePath &) { } } else if (s == "" || std::regex_match(s, attrPathRegex))