Remaining operations to serialize to reach a baseline of Nix 2.18

[lf-]

Hi! I'm writing as one of the maintainers of Lix. I was originally looking at this library to see if it was about to cause compatibility problems by using obsolete methods by accident but it doesn't use any of the obsolete methods (which were not really carefully comprehensively written down as such before we looked into it)! Fantastic work!

If it's useful to you, we have a slightly cleaner list of methods and their deprecation status than CppNix has, here: https://github.com/lix-project/lix/blob/4ac2c496d499a4a0e2d64edf32eb855268e7aa8d/src/libstore/worker-protocol.hh#L140-L185

We are doing initial research to potentially use this library in our project to replace the Nix protocol, by putting it in a rust shim. If we wind up going ahead with this plan, we would probably contribute these, so I would not strongly prioritize implementing them yourself. Nonetheless, here are the not-yet-implemented methods that one would have to implement to reach a 100% 2.18 no-legacy compatibility baseline:

• [ ] AddIndirectRoot (12)
• [ ] QuerySubstitutablePathInfo (21)
• [ ] QuerySubstitutablePathInfos (30)

The only remaining question is how much of import happened since proto 34 (which is the one supported by this project per the blog post) and proto 35 (CppNix 2.18). As far as I can tell, that is just:

https://github.com/lix-project/lix/blob/50472aa5bea95b9e800df833f4f7ec7691e63807/src/libstore/daemon.cc#L1018-L1026

Given that, it looks like this library is almost ideal for our use case already (assuming the tests are good; but we can potentially help if we decide to use it), and that's absolutely brilliant information to find out. Genuinely, thank you so much for writing it.

[jneem]

Thanks for the interest! We can definitely add those three missing ops (I'm a bit puzzled how we missed them, actually! I thought we'd done all the non-obsolete ones...)

Compatibility with proto 35 also looks straightforward, I agree.

Automatic testing is, sadly, almost nonexistent right now. We have a bunch of shell scripts (not committed to the repo) that we've been running manually to test our proxy against an actual CppNix daemon. I'll see if I can wrangle them into an automatic test suite.

[lf-]

I'm not sure if you have seen it, but there is a fair amount of test coverage of the serializers in CppNix with binary test files. We didn't forward-port a number of the changes to it post-2.18, so that section is likely most comprehensive in CppNix here: https://github.com/nixos/nix/blob/7c2981fc559e93131c845164907cec6cae11ce48/tests/unit/libstore/worker-protocol.cc#L72-L93

My suggestion for writing a test suite for this library as a former rust-analyzer dev is to steal some of their ideas (:P):

• Write a check() function that takes a file name and an Expect (https://docs.rs/expect_test) for the Debug output of the deserialized value, and also checks it round trips to identical binary, then port the CppNix tests. The Debug output can be manually compared against the equivalent CppNix test case while writing the tests. This should get very low cost per test while reusing the examples from CppNix or Lix.
• Possibly use https://github.com/matklad/arbtest or similar to extensively test the serialization machinery itself; the most likely bugs in this are round trip failures.
• Lastly, possibly link to CppNix or Lix via the C++ API (I am so sorry. It is a very unstable API, but I can almost promise the Lix serializers will get left alone indefinitely because we want to delete them, so that might be the safer choice) and verify each arbitrarily generated test case round trips through their library as well. It is harder to validate that they are exactly semantically equivalent as one would have to construct the data structures cross language (I would not do that without a code generator, personally, and at that point it's not likely worthwhile), but a bijection is already a pretty strong sign.

That should be useful for building confidence that the library is equivalent to the CppNix one. We can help out with testing work when we get to protocol work in the next few months.

[jneem]

Thanks for the tips! I got the first point working, and I'll look at arbtest (or maybe proptest, which I'm already familiar with) next.

I think our shell script end-to-end tests are still useful, and I'll try to get them cleaned up as well. For example, they test that we're correctly tagging the return type of each worker op.

[Ericson2314]

I wrote the round trip characterization tests with the explicit idea that the golden masters can be reused for other implementations ;)

[jneem]

I looked into the missing ops:

• QuerySubstitutablePathInfo seems to be only sent here, when the protocol version is less than 12
• QuerySubstitutablePathInfosis only used in RemoteStore::querySubstitutablePathInfos, which is only called in Store::queryMissing. But when used through a remote store, Store::queryMissing is only called as a fallback in RemoteStore::queryMissing when the protocol version is less than 19

So based on these, I think we decided that those two were obsolete. Our definition of "obsolete" was "we can't make a new nix client produce those ops."

AddIndirectRoot seems to be missing because it's only used over uds and we only tested over ssh. That one's trivial to add, though.