Store the realisations as JSON in the binary cache #4333
Conversation
|
@regnat I think the depends and the target branch clash, which is why |
Nah, unless I misunderstood something dpulls only complains because the dependency isn't merged. Should be green once we merge it |
thufschmitt
force-pushed
the
ca/properly-store-outputs
branch
from
5bce6ad
to
4b4fc7f
Compare
|
@regnat but the PR is into |
Oh yes, I see what you mean. Well the thing is that something (either dpulls or github itself) automatically changes the target branch to
Then both PRs have a nice diff (showing only the incremental changes from the previous one) and only TL;DR: This is on purpose to ensure both a clean diff and a proper ordering of the PRs |
|
Oh wow, how odd. Well, I feel like we might as well actually merge this into the other one now that it's finished? (and it doesn't add more complexity.) |
thufschmitt
force-pushed
the
ca/properly-store-outputs
branch
from
2557e8a
to
3ac9d74
Compare
thufschmitt
force-pushed
the
ca/json-realisations-in-binary-caches
branch
from
6182254
to
eee5270
Compare
| } else { | ||
| return std::nullopt; | ||
| } | ||
| } | ||
|
|
||
| void BinaryCacheStore::registerDrvOutput(const Realisation& info) { | ||
| auto filePath = realisationsPrefix + "/" + info.id.to_string() + ".doi"; | ||
| upsertFile(filePath, info.to_string(), "text/x-nix-derivertopath"); | ||
| upsertFile(filePath, info.toJSON(), "application/json"); |
There was a problem hiding this comment.
is the out of toJSON reproducible?
There was a problem hiding this comment.
Good question… I guess the answer is “no, but we don't really care here”:
- No because:
- It probably won't be reproducible across Nix versions because we might want to add new fields or extend existing fields (like we already occasionally do for the narinfo files)
nlohmann::json(the library we're using behind the scenes to convert to json) doesn't explicitely guarantees that its output is reproducible between versions − although it seems to hold in practice. I've asked about it though because it could be needed in other places
- But we don't care because these only have to be semantically equivalent (we never try to content-address them or whatever, and we want to give us the possibility of extending their format in the future anyways)
There was a problem hiding this comment.
If we start using more JSON outputs, it would be nice if the fields were alphabetically sorted by default.
There was a problem hiding this comment.
If it's just that, it's already the case − and even if there's no guaranty that the default won't change, that bit of the library is pluggable − so I'm not too worried.
The bits that might change is stuff like the representation of numbers (1000 vs 1E3 for example). That bit is hardcoded and the author didn't give any formal guaranty that it wouldn't change in the future.
Fix #4332