Skip to content

Clean up structured attrs parsing using JSON utils #14754

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Ericson2314 merged 2 commits into from
Jan 16, 2026
Merged

Clean up structured attrs parsing using JSON utils #14754

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aaccb73
`ptrToOwned` should be in the `nix` namespace
Ericson2314 Dec 9, 2025
eb653e5
Clean up structured attrs parsing using JSON utils
Ericson2314 Dec 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 53 additions & 91 deletions src/libstore/derivation-options.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,76 +21,53 @@ static std::optional<std::string>
getStringAttr(const StringMap & env, const StructuredAttrs * parsed, const std::string & name)
{
if (parsed) {
auto i = parsed->structuredAttrs.find(name);
if (i == parsed->structuredAttrs.end())
return {};
else {
if (!i->second.is_string())
throw Error("attribute '%s' of must be a string", name);
return i->second.get<std::string>();
}
if (auto * i = get(parsed->structuredAttrs, name))
try {
return getString(*i);
} catch (Error & e) {
e.addTrace({}, "while parsing attribute \"%s\"", name);
throw;
}
} else {
auto i = env.find(name);
if (i == env.end())
return {};
else
return i->second;
if (auto * i = get(env, name))
return *i;
}
return {};
}

static bool getBoolAttr(const StringMap & env, const StructuredAttrs * parsed, const std::string & name, bool def)
{
if (parsed) {
auto i = parsed->structuredAttrs.find(name);
if (i == parsed->structuredAttrs.end())
return def;
else {
if (!i->second.is_boolean())
throw Error("attribute '%s' must be a Boolean", name);
return i->second.get<bool>();
}
if (auto * i = get(parsed->structuredAttrs, name))
try {
return getBoolean(*i);
} catch (Error & e) {
e.addTrace({}, "while parsing attribute \"%s\"", name);
throw;
}
} else {
auto i = env.find(name);
if (i == env.end())
return def;
else
return i->second == "1";
if (auto * i = get(env, name))
return *i == "1";
}
return def;
}

static std::optional<Strings>
getStringsAttr(const StringMap & env, const StructuredAttrs * parsed, const std::string & name)
static std::optional<StringSet>
getStringSetAttr(const StringMap & env, const StructuredAttrs * parsed, const std::string & name)
{
if (parsed) {
auto i = parsed->structuredAttrs.find(name);
if (i == parsed->structuredAttrs.end())
return {};
else {
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 = 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>());
if (auto * i = get(parsed->structuredAttrs, name))
try {
return getStringSet(*i);
} catch (Error & e) {
e.addTrace({}, "while parsing attribute \"%s\"", name);
throw;
}
return res;
}
} else {
auto i = env.find(name);
if (i == env.end())
return {};
else
return tokenizeString<Strings>(i->second);
if (auto * i = get(env, name))
return tokenizeString<StringSet>(*i);
}
}

static std::optional<StringSet>
getStringSetAttr(const StringMap & env, const StructuredAttrs * parsed, const std::string & name)
{
auto ss = getStringsAttr(env, parsed, name);
return ss ? (std::optional{StringSet{ss->begin(), ss->end()}}) : (std::optional<StringSet>{});
return {};
}

template<typename Inputs>
Expand Down Expand Up @@ -233,45 +210,30 @@ DerivationOptions<SingleDerivedPath> derivationOptionsFromStructuredAttrs(
std::map<std::string, OutputChecks<SingleDerivedPath>> res;
if (auto * outputChecks = get(structuredAttrs, "outputChecks")) {
for (auto & [outputName, output_] : getObject(*outputChecks)) {
OutputChecks<SingleDerivedPath> checks;

auto & output = getObject(output_);

if (auto maxSize = get(output, "maxSize"))
checks.maxSize = maxSize->get<uint64_t>();

if (auto maxClosureSize = get(output, "maxClosureSize"))
checks.maxClosureSize = maxClosureSize->get<uint64_t>();

auto get_ =
[&](const std::string & name) -> std::optional<std::set<DrvRef<SingleDerivedPath>>> {
if (auto i = get(output, name)) {
std::set<DrvRef<SingleDerivedPath>> res;
for (auto j = i->begin(); j != i->end(); ++j) {
if (!j->is_string())
throw Error("attribute '%s' must be a list of strings", name);
res.insert(parseRef(j->get<std::string>()));
if (auto * i = get(output, name)) {
try {
std::set<DrvRef<SingleDerivedPath>> res;
for (auto & s : getStringList(*i))
res.insert(parseRef(s));
return res;
} catch (Error & e) {
e.addTrace(
{}, "while parsing attribute 'outputChecks.\"%s\".%s'", outputName, name);
throw;
}
return res;
}
return {};
};

res.insert_or_assign(
outputName,
OutputChecks<SingleDerivedPath>{
.maxSize = [&]() -> std::optional<uint64_t> {
if (auto maxSize = get(output, "maxSize"))
return maxSize->get<uint64_t>();
else
return std::nullopt;
}(),
.maxClosureSize = [&]() -> std::optional<uint64_t> {
if (auto maxClosureSize = get(output, "maxClosureSize"))
return maxClosureSize->get<uint64_t>();
else
return std::nullopt;
}(),
.maxSize = ptrToOwned<uint64_t>(get(output, "maxSize")),
.maxClosureSize = ptrToOwned<uint64_t>(get(output, "maxClosureSize")),
.allowedReferences = get_("allowedReferences"),
.disallowedReferences =
get_("disallowedReferences").value_or(std::set<DrvRef<SingleDerivedPath>>{}),
Expand Down Expand Up @@ -307,13 +269,13 @@ DerivationOptions<SingleDerivedPath> derivationOptionsFromStructuredAttrs(
std::map<std::string, bool> res;

if (parsed) {
auto & structuredAttrs = parsed->structuredAttrs;

if (auto * udr = get(structuredAttrs, "unsafeDiscardReferences")) {
for (auto & [outputName, output] : getObject(*udr)) {
if (!output.is_boolean())
throw Error("attribute 'unsafeDiscardReferences.\"%s\"' must be a Boolean", outputName);
res.insert_or_assign(outputName, output.get<bool>());
if (auto * udr = get(parsed->structuredAttrs, "unsafeDiscardReferences")) {
try {
for (auto & [outputName, output] : getObject(*udr))
res.insert_or_assign(outputName, getBoolean(output));
} catch (Error & e) {
e.addTrace({}, "while parsing attribute 'unsafeDiscardReferences'");
Copy link
Member Author

@Ericson2314 Ericson2314 Dec 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we don't loose context versus old throw

throw;
}
}
}
Expand All @@ -340,7 +302,7 @@ DerivationOptions<SingleDerivedPath> derivationOptionsFromStructuredAttrs(
std::map<std::string, std::set<SingleDerivedPath>> ret;

if (parsed) {
auto * e = optionalValueAt(parsed->structuredAttrs, "exportReferencesGraph");
auto * e = get(parsed->structuredAttrs, "exportReferencesGraph");
if (!e)
return ret;
if (!e->is_object()) {
Expand Down Expand Up @@ -595,8 +557,8 @@ DerivationOptions<SingleDerivedPath> adl_serializer<DerivationOptions<SingleDeri
.outputChecks = [&]() -> OutputChecksVariant<SingleDerivedPath> {
auto outputChecks = getObject(valueAt(json, "outputChecks"));

auto forAllOutputsOpt = optionalValueAt(outputChecks, "forAllOutputs");
auto perOutputOpt = optionalValueAt(outputChecks, "perOutput");
auto forAllOutputsOpt = get(outputChecks, "forAllOutputs");
auto perOutputOpt = get(outputChecks, "perOutput");

if (forAllOutputsOpt && !perOutputOpt) {
return static_cast<OutputChecks<SingleDerivedPath>>(*forAllOutputsOpt);
Expand Down
18 changes: 9 additions & 9 deletions src/libutil/include/nix/util/json-utils.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,15 @@ Strings getStringList(const nlohmann::json & value);
StringMap getStringMap(const nlohmann::json & value);
StringSet getStringSet(const nlohmann::json & value);

template<typename T>
static inline std::optional<T> ptrToOwned(const nlohmann::json * ptr)
Comment on lines +78 to +79
Copy link
Member Author

@Ericson2314 Ericson2314 Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moving it to the right namespace

{
if (ptr)
return std::optional{*ptr};
else
return std::nullopt;
}

} // namespace nix

namespace nlohmann {
Expand Down Expand Up @@ -114,13 +123,4 @@ struct adl_serializer<std::optional<T>>
}
};

template<typename T>
static inline std::optional<T> ptrToOwned(const json * ptr)
{
if (ptr)
return std::optional{*ptr};
else
return std::nullopt;
}

} // namespace nlohmann
Loading