From 91593a237ff4b3de28edd2fd85bc0905efe4ea8b Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 17 Sep 2025 18:14:12 -0400 Subject: [PATCH] Convert Realisation JSON logic to standard style No behavior is changed, just: - Declare a canonical `nlohmnan::json::adl_serializer` - Use `json-utils.hh` to shorten code without getting worse error messages. Co-authored-by: Robert Hensing --- src/libstore/binary-cache-store.cc | 12 ++- src/libstore/common-protocol.cc | 9 +- src/libstore/include/nix/store/realisation.hh | 5 +- src/libstore/nar-info-disk-cache.cc | 16 ++-- src/libstore/realisation.cc | 88 +++++++++---------- src/nix/realisation.cc | 2 +- src/perl/lib/Nix/Store.xs | 2 +- 7 files changed, 73 insertions(+), 61 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index e08a1449b..d5184b1bf 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -515,8 +515,14 @@ void BinaryCacheStore::queryRealisationUncached( if (!data) return (*callbackPtr)({}); - auto realisation = Realisation::fromJSON(nlohmann::json::parse(*data), outputInfoFilePath); - return (*callbackPtr)(std::make_shared(realisation)); + std::shared_ptr realisation; + try { + realisation = std::make_shared(nlohmann::json::parse(*data)); + } catch (Error & e) { + e.addTrace({}, "while parsing file '%s' as a realisation", outputInfoFilePath); + throw; + } + return (*callbackPtr)(std::move(realisation)); } catch (...) { callbackPtr->rethrow(); } @@ -530,7 +536,7 @@ void BinaryCacheStore::registerDrvOutput(const Realisation & info) if (diskCache) diskCache->upsertRealisation(config.getReference().render(/*FIXME withParams=*/false), info); auto filePath = realisationsPrefix + "/" + info.id.to_string() + ".doi"; - upsertFile(filePath, info.toJSON().dump(), "application/json"); + upsertFile(filePath, static_cast(info).dump(), "application/json"); } ref BinaryCacheStore::getFSAccessor(bool requireValidPath) diff --git a/src/libstore/common-protocol.cc b/src/libstore/common-protocol.cc index d4f3efc9b..b069c9498 100644 --- a/src/libstore/common-protocol.cc +++ b/src/libstore/common-protocol.cc @@ -49,13 +49,18 @@ void CommonProto::Serialise::write( Realisation CommonProto::Serialise::read(const StoreDirConfig & store, CommonProto::ReadConn conn) { std::string rawInput = readString(conn.from); - return Realisation::fromJSON(nlohmann::json::parse(rawInput), "remote-protocol"); + try { + return nlohmann::json::parse(rawInput); + } catch (Error & e) { + e.addTrace({}, "while parsing a realisation object in the remote protocol"); + throw; + } } void CommonProto::Serialise::write( const StoreDirConfig & store, CommonProto::WriteConn conn, const Realisation & realisation) { - conn.to << realisation.toJSON().dump(); + conn.to << static_cast(realisation).dump(); } DrvOutput CommonProto::Serialise::read(const StoreDirConfig & store, CommonProto::ReadConn conn) diff --git a/src/libstore/include/nix/store/realisation.hh b/src/libstore/include/nix/store/realisation.hh index 6eb3eecf3..3424a39c9 100644 --- a/src/libstore/include/nix/store/realisation.hh +++ b/src/libstore/include/nix/store/realisation.hh @@ -64,9 +64,6 @@ struct Realisation */ std::map dependentRealisations; - nlohmann::json toJSON() const; - static Realisation fromJSON(const nlohmann::json & json, const std::string & whence); - std::string fingerprint() const; void sign(const Signer &); bool checkSignature(const PublicKeys & publicKeys, const std::string & sig) const; @@ -169,3 +166,5 @@ public: }; } // namespace nix + +JSON_IMPL(nix::Realisation) diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc index 69d8d2e14..11608a667 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -304,10 +304,15 @@ public: if (queryRealisation.isNull(0)) return {oInvalid, 0}; - auto realisation = std::make_shared( - Realisation::fromJSON(nlohmann::json::parse(queryRealisation.getStr(0)), "Local disk cache")); - - return {oValid, realisation}; + try { + return { + oValid, + std::make_shared(nlohmann::json::parse(queryRealisation.getStr(0))), + }; + } catch (Error & e) { + e.addTrace({}, "while parsing the local disk cache"); + throw; + } }); } @@ -349,7 +354,8 @@ public: auto & cache(getCache(*state, uri)); - state->insertRealisation.use()(cache.id)(realisation.id.to_string())(realisation.toJSON().dump())(time(0)) + state->insertRealisation + .use()(cache.id)(realisation.id.to_string())(static_cast(realisation).dump())(time(0)) .exec(); }); } diff --git a/src/libstore/realisation.cc b/src/libstore/realisation.cc index 8c3baa73b..d59f4b0ea 100644 --- a/src/libstore/realisation.cc +++ b/src/libstore/realisation.cc @@ -2,6 +2,7 @@ #include "nix/store/store-api.hh" #include "nix/util/closure.hh" #include "nix/util/signature/local-keys.hh" +#include "nix/util/json-utils.hh" #include namespace nix { @@ -60,54 +61,9 @@ void Realisation::closure(Store & store, const std::set & startOutp }); } -nlohmann::json Realisation::toJSON() const -{ - auto jsonDependentRealisations = nlohmann::json::object(); - for (auto & [depId, depOutPath] : dependentRealisations) - jsonDependentRealisations.emplace(depId.to_string(), depOutPath.to_string()); - return nlohmann::json{ - {"id", id.to_string()}, - {"outPath", outPath.to_string()}, - {"signatures", signatures}, - {"dependentRealisations", jsonDependentRealisations}, - }; -} - -Realisation Realisation::fromJSON(const nlohmann::json & json, const std::string & whence) -{ - auto getOptionalField = [&](std::string fieldName) -> std::optional { - auto fieldIterator = json.find(fieldName); - if (fieldIterator == json.end()) - return std::nullopt; - return {*fieldIterator}; - }; - auto getField = [&](std::string fieldName) -> std::string { - if (auto field = getOptionalField(fieldName)) - return *field; - else - throw Error("Drv output info file '%1%' is corrupt, missing field %2%", whence, fieldName); - }; - - StringSet signatures; - if (auto signaturesIterator = json.find("signatures"); signaturesIterator != json.end()) - signatures.insert(signaturesIterator->begin(), signaturesIterator->end()); - - std::map dependentRealisations; - if (auto jsonDependencies = json.find("dependentRealisations"); jsonDependencies != json.end()) - for (auto & [jsonDepId, jsonDepOutPath] : jsonDependencies->get()) - dependentRealisations.insert({DrvOutput::parse(jsonDepId), StorePath(jsonDepOutPath)}); - - return Realisation{ - .id = DrvOutput::parse(getField("id")), - .outPath = StorePath(getField("outPath")), - .signatures = signatures, - .dependentRealisations = dependentRealisations, - }; -} - std::string Realisation::fingerprint() const { - auto serialized = toJSON(); + nlohmann::json serialized = *this; serialized.erase("signatures"); return serialized.dump(); } @@ -183,3 +139,43 @@ RealisedPath::Set RealisedPath::closure(Store & store) const } } // namespace nix + +namespace nlohmann { + +using namespace nix; + +Realisation adl_serializer::from_json(const json & json0) +{ + auto json = getObject(json0); + + StringSet signatures; + if (auto signaturesOpt = optionalValueAt(json, "signatures")) + signatures = *signaturesOpt; + + std::map dependentRealisations; + if (auto jsonDependencies = optionalValueAt(json, "dependentRealisations")) + for (auto & [jsonDepId, jsonDepOutPath] : getObject(*jsonDependencies)) + dependentRealisations.insert({DrvOutput::parse(jsonDepId), jsonDepOutPath}); + + return Realisation{ + .id = DrvOutput::parse(valueAt(json, "id")), + .outPath = valueAt(json, "outPath"), + .signatures = signatures, + .dependentRealisations = dependentRealisations, + }; +} + +void adl_serializer::to_json(json & json, Realisation r) +{ + auto jsonDependentRealisations = nlohmann::json::object(); + for (auto & [depId, depOutPath] : r.dependentRealisations) + jsonDependentRealisations.emplace(depId.to_string(), depOutPath); + json = { + {"id", r.id.to_string()}, + {"outPath", r.outPath}, + {"signatures", r.signatures}, + {"dependentRealisations", jsonDependentRealisations}, + }; +} + +} // namespace nlohmann diff --git a/src/nix/realisation.cc b/src/nix/realisation.cc index a0e400f54..8dd608d23 100644 --- a/src/nix/realisation.cc +++ b/src/nix/realisation.cc @@ -59,7 +59,7 @@ struct CmdRealisationInfo : BuiltPathsCommand, MixJSON for (auto & path : realisations) { nlohmann::json currentPath; if (auto realisation = std::get_if(&path.raw)) - currentPath = realisation->toJSON(); + currentPath = *realisation; else currentPath["opaquePath"] = store->printStorePath(path.path()); diff --git a/src/perl/lib/Nix/Store.xs b/src/perl/lib/Nix/Store.xs index 7aa918ba0..93e9f0f95 100644 --- a/src/perl/lib/Nix/Store.xs +++ b/src/perl/lib/Nix/Store.xs @@ -168,7 +168,7 @@ StoreWrapper::queryRawRealisation(char * outputId) try { auto realisation = THIS->store->queryRealisation(DrvOutput::parse(outputId)); if (realisation) - XPUSHs(sv_2mortal(newSVpv(realisation->toJSON().dump().c_str(), 0))); + XPUSHs(sv_2mortal(newSVpv(static_cast(*realisation).dump().c_str(), 0))); else XPUSHs(sv_2mortal(newSVpv("", 0))); } catch (Error & e) {