Merge changes I07d2da41,I864d7340,I86612c64 into main

* changes:
  Change error messages about 'invalid paths' to 'path does not exist'.
  Add a clearer error message for InvalidPathError during evaluation
  Harmonise the Store::queryPathInfoUncached interface
This commit is contained in:
julia 2024-06-16 04:29:13 +00:00 committed by Gerrit Code Review
commit dd70044cde
9 changed files with 55 additions and 25 deletions

View file

@ -47,12 +47,16 @@ MakeError(MissingArgumentError, EvalError);
MakeError(RestrictedPathError, Error); MakeError(RestrictedPathError, Error);
MakeError(InfiniteRecursionError, EvalError); MakeError(InfiniteRecursionError, EvalError);
/**
* Represents an exception due to an invalid path; that is, it does not exist.
* It corresponds to `!Store::validPath()`.
*/
struct InvalidPathError : public EvalError struct InvalidPathError : public EvalError
{ {
public: public:
Path path; Path path;
InvalidPathError(EvalState & state, const Path & path) InvalidPathError(EvalState & state, const Path & path)
: EvalError(state, "path '%s' is not valid", path) : EvalError(state, "path '%s' did not exist in the store during evaluation", path)
{ {
} }
}; };

View file

@ -383,7 +383,8 @@ void prim_exec(EvalState & state, const PosIdx pos, Value * * args, Value & v)
try { try {
auto _ = state.realiseContext(context); // FIXME: Handle CA derivations auto _ = state.realiseContext(context); // FIXME: Handle CA derivations
} catch (InvalidPathError & e) { } catch (InvalidPathError & e) {
state.error<EvalError>("cannot execute '%1%', since path '%2%' is not valid", program, e.path).atPos(pos).debugThrow(); e.addTrace(state.positions[pos], "while realising the context for builtins.exec");
throw;
} }
auto output = runProgram(program, true, commandArgs); auto output = runProgram(program, true, commandArgs);

View file

@ -170,7 +170,7 @@ ref<const ValidPathInfo> BinaryCacheStore::addToStoreCommon(
if (ref != info.path) if (ref != info.path)
queryPathInfo(ref); queryPathInfo(ref);
} catch (InvalidPath &) { } catch (InvalidPath &) {
throw Error("cannot add '%s' to the binary cache because the reference '%s' is not valid", throw Error("cannot add '%s' to the binary cache because the reference '%s' does not exist",
printStorePath(info.path), printStorePath(ref)); printStorePath(info.path), printStorePath(ref));
} }
@ -361,7 +361,7 @@ std::shared_ptr<const ValidPathInfo> BinaryCacheStore::queryPathInfoUncached(con
auto data = getFile(narInfoFile); auto data = getFile(narInfoFile);
if (!data) return {}; if (!data) return nullptr;
stats.narInfoRead++; stats.narInfoRead++;

View file

@ -23,7 +23,7 @@ struct LocalStoreAccessor : public FSAccessor
{ {
auto storePath = store->toStorePath(path).first; auto storePath = store->toStorePath(path).first;
if (requireValidPath && !store->isValidPath(storePath)) if (requireValidPath && !store->isValidPath(storePath))
throw InvalidPath("path '%1%' is not a valid store path", store->printStorePath(storePath)); throw InvalidPath("path '%1%' does not exist in the store", store->printStorePath(storePath));
return store->getRealStoreDir() + std::string(path, store->storeDir.size()); return store->getRealStoreDir() + std::string(path, store->storeDir.size());
} }
@ -81,7 +81,7 @@ ref<FSAccessor> LocalFSStore::getFSAccessor()
void LocalFSStore::narFromPath(const StorePath & path, Sink & sink) void LocalFSStore::narFromPath(const StorePath & path, Sink & sink)
{ {
if (!isValidPath(path)) if (!isValidPath(path))
throw Error("path '%s' is not valid", printStorePath(path)); throw Error("path '%s' does not exist in store", printStorePath(path));
dumpPath(getRealStoreDir() + std::string(printStorePath(path), storeDir.size()), sink); dumpPath(getRealStoreDir() + std::string(printStorePath(path), storeDir.size()), sink);
} }

View file

@ -899,7 +899,7 @@ std::shared_ptr<const ValidPathInfo> LocalStore::queryPathInfoInternal(State & s
auto useQueryPathInfo(state.stmts->QueryPathInfo.use()(printStorePath(path))); auto useQueryPathInfo(state.stmts->QueryPathInfo.use()(printStorePath(path)));
if (!useQueryPathInfo.next()) if (!useQueryPathInfo.next())
return std::shared_ptr<ValidPathInfo>(); return nullptr;
auto id = useQueryPathInfo.getInt(0); auto id = useQueryPathInfo.getInt(0);
@ -907,7 +907,7 @@ std::shared_ptr<const ValidPathInfo> LocalStore::queryPathInfoInternal(State & s
try { try {
narHash = Hash::parseAnyPrefixed(useQueryPathInfo.getStr(1)); narHash = Hash::parseAnyPrefixed(useQueryPathInfo.getStr(1));
} catch (BadHash & e) { } catch (BadHash & e) {
throw Error("invalid-path entry for '%s': %s", printStorePath(path), e.what()); throw BadStorePath("bad hash in store path '%s': %s", printStorePath(path), e.what());
} }
auto info = std::make_shared<ValidPathInfo>(path, narHash); auto info = std::make_shared<ValidPathInfo>(path, narHash);
@ -957,8 +957,8 @@ void LocalStore::updatePathInfo(State & state, const ValidPathInfo & info)
uint64_t LocalStore::queryValidPathId(State & state, const StorePath & path) uint64_t LocalStore::queryValidPathId(State & state, const StorePath & path)
{ {
auto use(state.stmts->QueryPathInfo.use()(printStorePath(path))); auto use(state.stmts->QueryPathInfo.use()(printStorePath(path)));
if (!use.next()) if (!use.next()) // TODO: I guess if SQLITE got corrupted..?
throw InvalidPath("path '%s' is not valid", printStorePath(path)); throw InvalidPath("path '%s' does not exist", printStorePath(path));
return use.getInt(0); return use.getInt(0);
} }

View file

@ -55,7 +55,7 @@ std::pair<ref<FSAccessor>, Path> RemoteFSAccessor::fetch(const Path & path_, boo
auto [storePath, restPath] = store->toStorePath(path); auto [storePath, restPath] = store->toStorePath(path);
if (requireValidPath && !store->isValidPath(storePath)) if (requireValidPath && !store->isValidPath(storePath))
throw InvalidPath("path '%1%' is not a valid store path", store->printStorePath(storePath)); throw InvalidPath("path '%1%' does not exist in remote store", store->printStorePath(storePath));
auto i = nars.find(std::string(storePath.hashPart())); auto i = nars.find(std::string(storePath.hashPart()));
if (i != nars.end()) return {i->second, restPath}; if (i != nars.end()) return {i->second, restPath};

View file

@ -309,14 +309,14 @@ std::shared_ptr<const ValidPathInfo> RemoteStore::queryPathInfoUncached(const St
try { try {
conn.processStderr(); conn.processStderr();
} catch (Error & e) { } catch (Error & e) {
// Ugly backwards compatibility hack. // Ugly backwards compatibility hack. TODO(fj#325): remove.
if (e.msg().find("is not valid") != std::string::npos) if (e.msg().find("is not valid") != std::string::npos)
throw InvalidPath(std::move(e.info())); return nullptr;
throw; throw;
} }
if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 17) { if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 17) {
bool valid; conn->from >> valid; bool valid; conn->from >> valid;
if (!valid) throw InvalidPath("path '%s' is not valid", printStorePath(path)); if (!valid) return nullptr;
} }
return std::make_shared<ValidPathInfo>( return std::make_shared<ValidPathInfo>(
StorePath{path}, StorePath{path},

View file

@ -666,11 +666,19 @@ bool Store::isValidPathUncached(const StorePath & path)
} }
static bool goodStorePath(const StorePath & expected, const StorePath & actual) static void ensureGoodStorePath(Store * store, const StorePath & expected, const StorePath & actual)
{ {
return if (expected.hashPart() != actual.hashPart()) {
expected.hashPart() == actual.hashPart() throw Error(
&& (expected.name() == Store::MissingName || expected.name() == actual.name()); "the queried store path hash '%s' did not match expected '%s' while querying the store path '%s'",
expected.hashPart(), actual.hashPart(), store->printStorePath(expected)
);
} else if (expected.name() != Store::MissingName && expected.name() != actual.name()) {
throw Error(
"the queried store path name '%s' did not match expected '%s' while querying the store path '%s'",
expected.name(), actual.name(), store->printStorePath(expected)
);
}
} }
@ -683,7 +691,7 @@ ref<const ValidPathInfo> Store::queryPathInfo(const StorePath & storePath)
if (res && res->isKnownNow()) { if (res && res->isKnownNow()) {
stats.narInfoReadAverted++; stats.narInfoReadAverted++;
if (!res->didExist()) if (!res->didExist())
throw InvalidPath("path '%s' is not valid", printStorePath(storePath)); throw InvalidPath("path '%s' does not exist in the store", printStorePath(storePath));
return ref<const ValidPathInfo>(res->value); return ref<const ValidPathInfo>(res->value);
} }
} }
@ -696,27 +704,31 @@ ref<const ValidPathInfo> Store::queryPathInfo(const StorePath & storePath)
auto state_(state.lock()); auto state_(state.lock());
state_->pathInfoCache.upsert(std::string(storePath.to_string()), state_->pathInfoCache.upsert(std::string(storePath.to_string()),
res.first == NarInfoDiskCache::oInvalid ? PathInfoCacheValue{} : PathInfoCacheValue{ .value = res.second }); res.first == NarInfoDiskCache::oInvalid ? PathInfoCacheValue{} : PathInfoCacheValue{ .value = res.second });
if (res.first == NarInfoDiskCache::oInvalid || if (res.first == NarInfoDiskCache::oInvalid)
!goodStorePath(storePath, res.second->path)) throw InvalidPath("path '%s' does not exist in the store", printStorePath(storePath));
throw InvalidPath("path '%s' is not valid", printStorePath(storePath));
} }
return ref<const ValidPathInfo>(res.second); return ref<const ValidPathInfo>(res.second);
} }
} }
auto info = queryPathInfoUncached(storePath); auto info = queryPathInfoUncached(storePath);
if (info) {
// first, before we cache anything, check that the store gave us valid data.
ensureGoodStorePath(this, storePath, info->path);
}
if (diskCache) if (diskCache) {
diskCache->upsertNarInfo(getUri(), hashPart, info); diskCache->upsertNarInfo(getUri(), hashPart, info);
}
{ {
auto state_(state.lock()); auto state_(state.lock());
state_->pathInfoCache.upsert(std::string(storePath.to_string()), PathInfoCacheValue { .value = info }); state_->pathInfoCache.upsert(std::string(storePath.to_string()), PathInfoCacheValue { .value = info });
} }
if (!info || !goodStorePath(storePath, info->path)) { if (!info) {
stats.narInfoMissing++; stats.narInfoMissing++;
throw InvalidPath("path '%s' is not valid", printStorePath(storePath)); throw InvalidPath("path '%s' does not exist in the store", printStorePath(storePath));
} }
return ref<const ValidPathInfo>(info); return ref<const ValidPathInfo>(info);

View file

@ -62,10 +62,18 @@ MakeError(SubstError, Error);
* denotes a permanent build failure * denotes a permanent build failure
*/ */
MakeError(BuildError, Error); MakeError(BuildError, Error);
/**
* denotes that a path in the store did not exist (but it could, had it
* been put there, i.e. it is still legal).
*/
MakeError(InvalidPath, Error); MakeError(InvalidPath, Error);
MakeError(Unsupported, Error); MakeError(Unsupported, Error);
MakeError(SubstituteGone, Error); MakeError(SubstituteGone, Error);
MakeError(SubstituterDisabled, Error); MakeError(SubstituterDisabled, Error);
/**
* denotes that a path could not possibly be a store path.
* e.g. outside of the nix store, illegal characters in the name, etc.
*/
MakeError(BadStorePath, Error); MakeError(BadStorePath, Error);
MakeError(InvalidStoreURI, Error); MakeError(InvalidStoreURI, Error);
@ -328,6 +336,7 @@ public:
/** /**
* Check whether a path is valid. * Check whether a path is valid.
* A path is valid when it exists in the store *now*.
*/ */
bool isValidPath(const StorePath & path); bool isValidPath(const StorePath & path);
@ -399,6 +408,10 @@ public:
protected: protected:
/**
* Queries the path info without caching.
* Note to implementors: should return `nullptr` when the path is not found.
*/
virtual std::shared_ptr<const ValidPathInfo> queryPathInfoUncached(const StorePath & path) = 0; virtual std::shared_ptr<const ValidPathInfo> queryPathInfoUncached(const StorePath & path) = 0;
virtual std::shared_ptr<const Realisation> queryRealisationUncached(const DrvOutput &) = 0; virtual std::shared_ptr<const Realisation> queryRealisationUncached(const DrvOutput &) = 0;