Merge pull request #14359 from obsidiansystems/structured-attrs-always-object

Use types to show that structured attrs are always JSON objects

src/libexpr/primops.cc

@@ -1374,7 +1374,7 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
             pos,
             "while evaluating the `__structuredAttrs` "
             "attribute passed to builtins.derivationStrict"))
-        jsonObject = StructuredAttrs{.structuredAttrs = json::object()};
+        jsonObject = StructuredAttrs{};
 
     /* Check whether null attributes should be ignored. */
     bool ignoreNulls = false;

src/libstore/build/derivation-env-desugar.cc

@@ -25,7 +25,7 @@ DesugaredEnv DesugaredEnv::create(
     if (drv.structuredAttrs) {
         auto json = drv.structuredAttrs->prepareStructuredAttrs(store, drvOptions, inputPaths, drv.outputs);
         res.atFileEnvPair("NIX_ATTRS_SH_FILE", ".attrs.sh") = StructuredAttrs::writeShell(json);
-        res.atFileEnvPair("NIX_ATTRS_JSON_FILE", ".attrs.json") = json.dump();
+        res.atFileEnvPair("NIX_ATTRS_JSON_FILE", ".attrs.json") = static_cast<nlohmann::json>(std::move(json)).dump();
     } else {
         /* In non-structured mode, set all bindings either directory in the
            environment or via a file, as specified by

src/libstore/derivation-options.cc

@@ -22,9 +22,9 @@ getStringAttr(const StringMap & env, const StructuredAttrs * parsed, const std::
         if (i == parsed->structuredAttrs.end())
             return {};
         else {
-            if (!i->is_string())
+            if (!i->second.is_string())
                 throw Error("attribute '%s' of must be a string", name);
-            return i->get<std::string>();
+            return i->second.get<std::string>();
         }
     } else {
         auto i = env.find(name);
@@ -42,9 +42,9 @@ static bool getBoolAttr(const StringMap & env, const StructuredAttrs * parsed, c
         if (i == parsed->structuredAttrs.end())
             return def;
         else {
-            if (!i->is_boolean())
+            if (!i->second.is_boolean())
                 throw Error("attribute '%s' must be a Boolean", name);
-            return i->get<bool>();
+            return i->second.get<bool>();
         }
     } else {
         auto i = env.find(name);
@@ -63,10 +63,11 @@ getStringsAttr(const StringMap & env, const StructuredAttrs * parsed, const std:
         if (i == parsed->structuredAttrs.end())
             return {};
         else {
-            if (!i->is_array())
+            if (!i->second.is_array())
                 throw Error("attribute '%s' must be a list of strings", name);
+            auto & a = getArray(i->second);
             Strings res;
-            for (auto j = i->begin(); j != i->end(); ++j) {
+            for (auto j = a.begin(); j != a.end(); ++j) {
                 if (!j->is_string())
                     throw Error("attribute '%s' must be a list of strings", name);
                 res.push_back(j->get<std::string>());
@@ -116,7 +117,7 @@ DerivationOptions::fromStructuredAttrs(const StringMap & env, const StructuredAt
     DerivationOptions defaults = {};
 
     if (shouldWarn && parsed) {
-        auto & structuredAttrs = getObject(parsed->structuredAttrs);
+        auto & structuredAttrs = parsed->structuredAttrs;
 
         if (get(structuredAttrs, "allowedReferences")) {
             warn(
@@ -147,7 +148,7 @@ DerivationOptions::fromStructuredAttrs(const StringMap & env, const StructuredAt
     return {
         .outputChecks = [&]() -> OutputChecksVariant {
             if (parsed) {
-                auto & structuredAttrs = getObject(parsed->structuredAttrs);
+                auto & structuredAttrs = parsed->structuredAttrs;
 
                 std::map<std::string, OutputChecks> res;
                 if (auto * outputChecks = get(structuredAttrs, "outputChecks")) {
@@ -201,7 +202,7 @@ DerivationOptions::fromStructuredAttrs(const StringMap & env, const StructuredAt
                 std::map<std::string, bool> res;
 
                 if (parsed) {
-                    auto & structuredAttrs = getObject(parsed->structuredAttrs);
+                    auto & structuredAttrs = parsed->structuredAttrs;
 
                     if (auto * udr = get(structuredAttrs, "unsafeDiscardReferences")) {
                         for (auto & [outputName, output] : getObject(*udr)) {
@@ -234,7 +235,7 @@ DerivationOptions::fromStructuredAttrs(const StringMap & env, const StructuredAt
                 std::map<std::string, StringSet> ret;
 
                 if (parsed) {
-                    auto e = optionalValueAt(getObject(parsed->structuredAttrs), "exportReferencesGraph");
+                    auto * e = optionalValueAt(parsed->structuredAttrs, "exportReferencesGraph");
                     if (!e || !e->is_object())
                         return ret;
                     for (auto & [key, value] : getObject(*e)) {

src/libstore/include/nix/store/parsed-derivations.hh

@@ -18,7 +18,7 @@ struct StructuredAttrs
 {
     static constexpr std::string_view envVarName{"__json"};
 
-    nlohmann::json structuredAttrs;
+    nlohmann::json::object_t structuredAttrs;
 
     bool operator==(const StructuredAttrs &) const = default;
 
@@ -45,7 +45,7 @@ struct StructuredAttrs
      */
     static void checkKeyNotInUse(const StringPairs & env);
 
-    nlohmann::json prepareStructuredAttrs(
+    nlohmann::json::object_t prepareStructuredAttrs(
         Store & store,
         const DerivationOptions & drvOptions,
         const StorePathSet & inputPaths,
@@ -62,7 +62,7 @@ struct StructuredAttrs
      * `prepareStructuredAttrs`, *not* the original `structuredAttrs`
      * field.
      */
-    static std::string writeShell(const nlohmann::json & prepared);
+    static std::string writeShell(const nlohmann::json::object_t & prepared);
 };
 
 } // namespace nix

src/libstore/parsed-derivations.cc

@@ -33,7 +33,8 @@ std::optional<StructuredAttrs> StructuredAttrs::tryExtract(StringPairs & env)
 
 std::pair<std::string_view, std::string> StructuredAttrs::unparse() const
 {
-    return {envVarName, structuredAttrs.dump()};
+    // TODO don't copy the JSON object just to dump it.
+    return {envVarName, static_cast<nlohmann::json>(structuredAttrs).dump()};
 }
 
 void StructuredAttrs::checkKeyNotInUse(const StringPairs & env)
@@ -97,7 +98,7 @@ static nlohmann::json pathInfoToJSON(Store & store, const StorePathSet & storePa
     return jsonList;
 }
 
-nlohmann::json StructuredAttrs::prepareStructuredAttrs(
+nlohmann::json::object_t StructuredAttrs::prepareStructuredAttrs(
     Store & store,
     const DerivationOptions & drvOptions,
     const StorePathSet & inputPaths,
@@ -120,7 +121,7 @@ nlohmann::json StructuredAttrs::prepareStructuredAttrs(
     return json;
 }
 
-std::string StructuredAttrs::writeShell(const nlohmann::json & json)
+std::string StructuredAttrs::writeShell(const nlohmann::json::object_t & json)
 {
 
     auto handleSimpleType = [](const nlohmann::json & value) -> std::optional<std::string> {
@@ -144,7 +145,7 @@ std::string StructuredAttrs::writeShell(const nlohmann::json & json)
 
     std::string jsonSh;
 
-    for (auto & [key, value] : json.items()) {
+    for (auto & [key, value] : json) {
 
         if (!std::regex_match(key, shVarName))
             continue;

src/nix/nix-build/nix-build.cc

@@ -600,7 +600,7 @@ static void main_nix_build(int argc, char ** argv)
             structuredAttrsRC = StructuredAttrs::writeShell(json);
 
             auto attrsJSON = (tmpDir.path() / ".attrs.json").string();
-            writeFile(attrsJSON, json.dump());
+            writeFile(attrsJSON, static_cast<nlohmann::json>(std::move(json)).dump());
 
             auto attrsSH = (tmpDir.path() / ".attrs.sh").string();
             writeFile(attrsSH, structuredAttrsRC);