Merge pull request #14012 from obsidiansystems/drv-realisation-issue-13570

roberth · web-flow · commit 01357d080880 · 2025-09-22T20:25:06.000+02:00
Convert Realisation JSON logic to standard style
diff --git a/doc/manual/rl-next/derivation-json.md b/doc/manual/rl-next/derivation-json.md
@@ -4,14 +4,12 @@ prs: [13980]
 issues: [13570]
 ---
 
-Experience with many JSON frameworks (e.g. nlohmann/json in C++, Serde
-in Rust, and Aeson in Haskell), has show that the use of the store dir
-in JSON formats is an impediment to systematic JSON formats, because it
-requires the serializer/deserializer to take an extra paramater (the
-store dir).
+Experience with many JSON frameworks (e.g. nlohmann/json in C++, Serde in Rust, and Aeson in Haskell), has shown that the use of the store dir in JSON formats is an impediment to systematic JSON formats,
+because it requires the serializer/deserializer to take an extra paramater (the store dir).
 
-We ultimately want to rectify this issue with all (non-stable, able to
-be changed) JSON formats. To start with, we are changing the JSON format
-for derivations because the `nix derivation` commands are --- in
-addition to being formally unstable --- less widely used than other
-unstable commands.
+We ultimately want to rectify this issue with all (non-stable, able to be changed) JSON formats.
+To start with, we are changing the JSON format for derivations because the `nix derivation` commands are
+--- in addition to being formally unstable
+--- less widely used than other unstable commands.
+
+See the documentation on the [JSON format for derivations](@docroot@/protocols/json/derivation.md) for further details.
diff --git 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<const Realisation>(realisation));
+            std::shared_ptr<const Realisation> realisation;
+            try {
+                realisation = std::make_shared<const Realisation>(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<nlohmann::json>(info).dump(), "application/json");
 }
 
 ref<SourceAccessor> BinaryCacheStore::getFSAccessor(bool requireValidPath)
diff --git a/src/libstore/common-protocol.cc b/src/libstore/common-protocol.cc
@@ -49,13 +49,18 @@ void CommonProto::Serialise<ContentAddress>::write(
 Realisation CommonProto::Serialise<Realisation>::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<Realisation>::write(
     const StoreDirConfig & store, CommonProto::WriteConn conn, const Realisation & realisation)
 {
-    conn.to << realisation.toJSON().dump();
+    conn.to << static_cast<nlohmann::json>(realisation).dump();
 }
 
 DrvOutput CommonProto::Serialise<DrvOutput>::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
@@ -64,9 +64,6 @@ struct Realisation
      */
     std::map<DrvOutput, StorePath> 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
@@ -304,10 +304,15 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache
                 if (queryRealisation.isNull(0))
                     return {oInvalid, 0};
 
-                auto realisation = std::make_shared<Realisation>(
-                    Realisation::fromJSON(nlohmann::json::parse(queryRealisation.getStr(0)), "Local disk cache"));
-
-                return {oValid, realisation};
+                try {
+                    return {
+                        oValid,
+                        std::make_shared<Realisation>(nlohmann::json::parse(queryRealisation.getStr(0))),
+                    };
+                } catch (Error & e) {
+                    e.addTrace({}, "while parsing the local disk cache");
+                    throw;
+                }
             });
     }
 
@@ -349,7 +354,8 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache
 
             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<nlohmann::json>(realisation).dump())(time(0))
                 .exec();
         });
     }
diff --git 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 <nlohmann/json.hpp>
 
 namespace nix {
@@ -60,54 +61,9 @@ void Realisation::closure(Store & store, const std::set<Realisation> & 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<std::string> {
-        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<DrvOutput, StorePath> dependentRealisations;
-    if (auto jsonDependencies = json.find("dependentRealisations"); jsonDependencies != json.end())
-        for (auto & [jsonDepId, jsonDepOutPath] : jsonDependencies->get<StringMap>())
-            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<Realisation>::from_json(const json & json0)
+{
+    auto json = getObject(json0);
+
+    StringSet signatures;
+    if (auto signaturesOpt = optionalValueAt(json, "signatures"))
+        signatures = *signaturesOpt;
+
+    std::map<DrvOutput, StorePath> 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<Realisation>::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
@@ -59,7 +59,7 @@ struct CmdRealisationInfo : BuiltPathsCommand, MixJSON
             for (auto & path : realisations) {
                 nlohmann::json currentPath;
                 if (auto realisation = std::get_if<Realisation>(&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
@@ -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<nlohmann::json>(*realisation).dump().c_str(), 0)));
         else
             XPUSHs(sv_2mortal(newSVpv("", 0)));
       } catch (Error & e) {

Original file line number	Diff line number	Diff line change
`@@ -49,13 +49,18 @@ void CommonProto::Serialise<ContentAddress>::write(`
`49`	`49`	`Realisation CommonProto::Serialise<Realisation>::read(const StoreDirConfig & store, CommonProto::ReadConn conn)`
`50`	`50`	`{`
`51`	`51`	`std::string rawInput = readString(conn.from);`
`52`		`- return Realisation::fromJSON(nlohmann::json::parse(rawInput), "remote-protocol");`
	`52`	`+ try {`
	`53`	`+ return nlohmann::json::parse(rawInput);`
	`54`	`+ } catch (Error & e) {`
	`55`	`+ e.addTrace({}, "while parsing a realisation object in the remote protocol");`
	`56`	`+ throw;`
	`57`	`+ }`
`53`	`58`	`}`
`54`	`59`
`55`	`60`	`void CommonProto::Serialise<Realisation>::write(`
`56`	`61`	`const StoreDirConfig & store, CommonProto::WriteConn conn, const Realisation & realisation)`
`57`	`62`	`{`
`58`		`- conn.to << realisation.toJSON().dump();`
	`63`	`+ conn.to << static_cast<nlohmann::json>(realisation).dump();`
`59`	`64`	`}`
`60`	`65`
`61`	`66`	`DrvOutput CommonProto::Serialise<DrvOutput>::read(const StoreDirConfig & store, CommonProto::ReadConn conn)`