Give nix daemon and nix-store --serve protocols separate serializers with version info

[Ericson2314]

This allows that the versioning parts for it to actually be correct.

Version agnostic serializers are deduped in common-protocol.hh/ namespace nix::common_proto, while version-specific ones written separately in {worker,serve-proto.hh/namespace nix::{worker,serve}_proto.

Motivation

For me, these are main motivations for this:

1. No more perverse incentives. fe1f34f did some awkward things because the serializers did not store the version. I don't want anyone making changes to be pushed towards keeping the serialization logic with the core data types because it's easier.

2. I am confident with the incremental approach I tried out in Factoring out parts of the serve protocol for Hydra to share #6134, we can get Hydra using ssh-ng:// (and the Store abstraction) pretty quickly. After that, I hope we can deprecate ssh:// for a few releases and then remove it. I don't want to worry about accidentally changing it / breaking it during that time, and having separated codes ensures it stays out of the way!

3. In general, for anyone else implementing the daemon protocol, the more we factor out logic from daemon.cc, remote-store.cc, etc., the easier things will be to understand. There are a number of projects looking to reimplement the daemon protocol, and this helps them.

The last two commits are especially notable, because they use the serialization refactoring to dedup and factor out / improves factored out seralizers for:

• BuildResult
• PathInfo

This demonstrates the sort of cleanup that this is supposed t enable

Context

The history in this PR has been delicately preserved to allow reviewing commit-by-commit. Please do so!

If a certain prefix of commits seems good, we can always merge them first too.

depends on #8360
depends on #9197

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

• agreed on idea
• agreed on implementation strategy
• tests, as appropriate
  • functional tests - tests/**.sh
  • unit tests - src/*/tests
  • integration tests - tests/nixos/*
• documentation in the manual
• code and comments are self-explanatory
• commit message explains why the change was made
• new feature or incompatible change: updated release notes

[Ericson2314]

Rebased this on top!

[thufschmitt]

I’m quite scared by these raw #include-d files that have a different semantic depending on where they are included.

A nicer way to do that might be to

1. Replace the worker_proto and serve_proto namespaces by classes
2. Add these classes as argument to all the templates defined here (or maybe use plain subclassing?) so that they can be reused “normally”

[Ericson2314]

@thufschmitt Those files are gone! Also, less boilerplate to handle versions now.

Combined with #6312 and likewise for valid path info, this will be much nicer.

[thufschmitt]

I do like the overall idea, but I find this really complex, and I don’t see what justifies all this this complexity. Do we really need all that cpp and namespace mangling? I’d much rather use plain inheritance or templating, even it it comes with more boilerplate

[Ericson2314]

@thufschmitt What CPP mangling is there now?

I agree the namespace tricks are unfortunate --- if we have just had Rust's Traits this would all be so much easier --- but I thought quite long about alternatives and I can't think of much different unless it become unextendable (e.g. fit set of method on some serializer, lots of virtual), which doesn't sit right with me but also is just feels like far more churn.

The namespace tricks are roughly how nlohmann json works, too, fwiw.

I would like to do this more more less as-is, and just hope we can get rid of the nix-store --serve protocol soon, after which things are simpler again.

[thufschmitt]

What CPP mangling is there now?

No CPP mangling, just namespace mangling ;).
But the WRAP_META_PROTO, MAKE_PROTO and friends intuitively feel like something that would better be replaced by a class holding both the read and write methods.

I can't think of much different unless it become unextendable (e.g. fit set of method on some serializer, lots of virtual)

Is it that bad? After all we’re in control of everything, so it’s fine to have one master-class declaring all the methods (beside it’s easier to have a view of everything that’s implemented)

(Just to be clear: I’m not saying that this approach is wrong. There’s a lot of nasty complexity here and I’m all ready to believe that we don’t have a choice. But in that case it deserves a clear justification)

[Ericson2314]

Is it that bad? After all we’re in control of everything, so it’s fine to have one master-class declaring all the methods (beside it’s easier to have a view of everything that’s implemented)

You are right its not a problem in practice. Just the non-modularity of it sits very uncomfortably with me. If we ever want to make a greenfield new protocol that is e.g. Protobuf or JSON, and easier for other tools I understand, to those tools usually take the open world approach so I feel like it's good to match it too.

The tricky parts here are trying to share code between daemon protocol / serve protocol / hooks / import/export / whatever else. As soon as we stop/don't need to do that, it gets easier again.

[thufschmitt]

If we ever want to make a greenfield new protocol that is e.g. Protobuf or JSON, and easier for other tools I understand, to those tools usually take the open world approach so I feel like it's good to match it too.

Well, these new protocols wouldn’t share anything with the current one, would they?

The tricky parts here are trying to share code between daemon protocol / serve protocol / hooks / import/export / whatever else. As soon as we stop/don't need to do that, it gets easier again.

Unfortunately that will probably have to stay for at least a couple years, so we’d better not treat this as too temporary

[Ericson2314]

Well, these new protocols wouldn’t share anything with the current one, would they?

Yes they wouldn't, and thus the downsides of the current approach would hopefully be avoided

Unfortunately that will probably have to stay for at least a couple years, so we’d better not treat this as too temporary

Perhaps I just need to overcome the "ick" reflex and just duplicate the code. Then the status quo will also be simpler and modular.

I think once Hydra can use ssh-ng://, we can freeze and deprecate ssh://. Then, it is perfectly OK for the code to drift out of sync, so that might be a good time to get over the "ick" and do the duplicate + simplify.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/a-proposal-for-replacing-the-nix-worker-protocol/20926/12

[dpulls]

🎉 All dependencies have been resolved !