Subject: Re: Tvix code structure

From: Florian Klink <[email-masked]>
Date: Tue, 31 Jan 2023 12:01:17 +0100

>We should start thinking about how to clean up the code structure of 
>Tvix, some things are a bit ad-hoc. It's not a problem right now, and 
>not something we need to fix in the next week or so, but something to 
>keep in the back of our minds.

Agreed! I'm not too happy with the current structure either, but
we discovered some things as we went.

For example, implementing `builtins.derivation` in a nixcpp-compatible
way, with all the ATerm hashing needed way less store interaction than
we initially thought.
So it's only normal this did grow a bit organically :-)

>Tvix currently consists of the following subprojects:
>
>* //tvix/eval: Language evaluator with no knowledge about building, 
>dependencies and so on. It is just an implementation of the Nix 
>language, the pure builtins, and some generalised impure builtins 
>behind the `EvalIO` interface.
>
>* //tvix/serde: Implementation of a serde::Deserializer using 
>//tvix/eval to deserialise Rust data structures from Nix code. Can be 
>used for e.g. tools that use configuration files written in Nix.
>
>* //tvix/derivation: Implementation of data structures for the Nix 
>derivation concept, and serialisation (to ATerm)/validation logic.
>
>* //tvix/nar: A serialiser for the NAR archive format (*not* a 
>deserialiser).

Not yet. If we were to implement nar-bridge in Rust as well (or speak
the nix daemon protocol for `ssh-ng://`), we'd also need to add a reader
here.

>
>* //tvix/nix_cli: C++ Nix compatibility CLI, using clap to parse the 
>same argument structure as C++ Nix. This is not wired up to anything 
>right now.
>
>* //tvix/proto: Evaluator proto only, not used at the moment.
>
>* //tvix/store: Protobuf for stores, and initial implementation of the 
>store service. However, also contains types like `StorePath` which are 
>used in different places.
>
>* //tvix/cli: Currently, Tvix REPL implementation and the place where 
>things like `builtins.derivation` are implemented. Currently depends 
>on almost everything from above (including e.g. the store *service*!)

Yeah, `//tvix/store` is pulled in because `StorePath` lives in there,
and that is used by `//tvix/derivation`.

>I want the language evaluator to be independent from any build-related 
>concerns (so that the language can be used for other use-cases). That 
>means that //tvix/eval should never depend on any of the crates 
>implementing C++ Nix concepts, should not "know" about derivations 
>(more than where absolutely necessary for correct functioning of the 
>language, e.g. printing values) and so on. For this reason I've been 
>resisting putting things like `builtins.derivation` into eval.

I'd be very careful with this until we are able to evaluate nixpkgs a
bit further - and confirm we actually don't need any more complicated
string context tracking. If we would need it, it might become a bit more
of a "language feature" and creep into `//tvix/eval`.

>At the same time, putting it into a crate called "cli" also feels 
>wrong.

We also don't do any of the "eval tests" for anything
derivation-related, which is a bummer. I'm still a bit on the fence on
whether this should be a separate, feature-gated module in
`//tvix/eval`, or a separate crate.

>There's also clearly a bug in that //tvix/store implements a 
>service, and exports a widely used type, and that ends up dragging in 
>hundreds (!) of dependencies on any crate using that type.

As I said, that's mostly due to `//tvix/derivation` pulling in
`//tvix/store` via `StorePath`, and `//tvix/store` being a bit of a
monolith right now (no proper separation between client code and
server implementations).

>My current thinking is that what we actually want is something like this:
>
>* //tvix/eval: as above
>* //tvix/serde: as above
>* //tvix/nix-compat: types and logic related to compatibility with C++ 
>Nix: NARs, derivations, store paths and so on.
>* //tvix/store: store backend (!) implementation
>* //tvix/cli: REPL and CLI interface tying the above together
>
>This would involve merging the nar and derivation crates, and moving 
>the store path logic in there, too, to create the nix-compat crate.

>
>Doing this would also allow us to have a clear boundary where it's 
>visible what is legacy from C++ Nix, and show where we have the 
>ability to come up with substitute concepts (or decide that that is 
>not necessary).

Even with different concepts for calculating output paths, I'd still
assume our store paths will probably look very similar, and we might
need to end up keeping other functionality too.

I'd be more in favor of keeping `derivation` and `nar` a separate crate
for now. I'll send a CL splitting out `StorePath` from `//tvix/store`,
that should solve the immediate concerns w.r.t. things pulled into eval
directly.

We can make sure we split things up further once we get to actually
talking to the store from nix-compat or cli (as a client).

flokli