Delete protocol support for versions less than 2.18 #510

Open
opened 2024-09-10 01:49:52 +00:00 by jade · 5 comments
Owner

This will eliminate all conditional code in the protocol, allowing us to replace it with the Rust implementation: https://github.com/tweag/nix-remote-rust

Also of note: addMultipleToStore is in our implementation but is actually unused. We could stub it and tell clients to go away if they try to use it. ah nope! it's used by copyPaths. I do wonder if it should actually exist in the future protocol though.

I am not sure if we should do this immediately, but it would not be a problem to do so immediately imo. Whoever is running 2.3 does not have extensive protocol compat testing, the only thing we guarantee compat with is 2.18 and ourselves and I guess CppNix assuming they don't break their protocol implementation, though we do not test against them.

This will eliminate all conditional code in the protocol, allowing us to replace it with the Rust implementation: https://github.com/tweag/nix-remote-rust ~~Also of note: addMultipleToStore is in our implementation but is actually unused. We could stub it and tell clients to go away if they try to use it.~~ ah nope! it's used by copyPaths. I do wonder if it should actually exist in the future protocol though. I am not sure if we should do this immediately, but it would not be a *problem* to do so immediately imo. Whoever is running 2.3 does not have extensive protocol compat testing, the only thing we guarantee compat with is 2.18 and ourselves and I guess CppNix assuming they don't break their protocol implementation, though we do not test against them.
jade added the
Area/protocol
label 2024-09-10 01:49:52 +00:00
Member

Let's create a separate issue for porting the protocol part to Rust, and make this issue only about simplifying the existing C++ code. This is a much easier, smaller task to handle, with immediate benefits.

About Rust: let's create a milestone (called "Oxidation", maybe?) Replacing protocol with Rust implementation would be a pretty feasible first step, and it would also allow non-C++ folks like me to contribute to the project in code! A couple of notes about moving to Rust, though:

  • The build system would have to be expanded. AFAIK, it shouldn't be too big of a problem for Meson, but still
  • It would make working with the codebase pretty painful for quite a while. I haven't worked with Rust in FFI context, but my understanding is that we'll have to maintain public Rust interfaces, public Rust FFI interfaces, and C++ interfaces at the same time. Maybe it's not a big deal when it comes to core interfaces that will rarely change (such as protocol, which we won't touch for a while) and "frontend" changes (such as rewriting the CLI part in Rust)
  • We would have a much easier time dealing with filesystem, because Rust has really solid Path implementation in stdlib (relevant for #499)
  • We would have to rewrite parts of Tweag's implementation. I don't love exposing anyhow errors in the library, and I'd assume there are some gotchas in error handling with FFI
  • Perhaps later down the line we'd be able to make use of some Tvix developments, and share some of ours. A lot of things are different between Nix 2.3 and 2.18, but maybe we can find some things that are easy to integrate in both
Let's create a separate issue for porting the protocol part to Rust, and make this issue only about simplifying the existing C++ code. This is a much easier, smaller task to handle, with immediate benefits. About Rust: let's create a milestone (called "Oxidation", maybe?) Replacing protocol with Rust implementation would be a pretty feasible first step, and it would also allow non-C++ folks like me to contribute to the project in code! A couple of notes about moving to Rust, though: - The build system would have to be expanded. AFAIK, it shouldn't be too big of a problem for Meson, but still - It would make working with the codebase pretty painful for quite a while. I haven't worked with Rust in FFI context, but my understanding is that we'll have to maintain public Rust interfaces, public Rust FFI interfaces, and C++ interfaces at the same time. Maybe it's not a big deal when it comes to core interfaces that will rarely change (such as protocol, which we won't touch for a while) and "frontend" changes (such as rewriting the CLI part in Rust) - We would have a much easier time dealing with filesystem, because Rust has really solid Path implementation in stdlib (relevant for #499) - We would have to rewrite parts of Tweag's implementation. I don't love exposing `anyhow` errors in the library, and I'd assume there are some gotchas in error handling with FFI - Perhaps later down the line we'd be able to make use of some [Tvix](https://tvix.dev/) developments, and share some of ours. A lot of things are different between Nix 2.3 and 2.18, but maybe we can find some things that are easy to integrate in both
Owner

@kfearsoff I appreciate the enthusiasm, just want to point out that a regular contributor may be in a better position to make these suggestions as they will probably own driving this sort of work.

If you are interested into building out what you laid in that message, I recommend you to ask questions to the team to verify if your plans are compatible with whatever in-flight plan there might be and maybe that you send more CLs so that the team can get to know you a bit more.

@kfearsoff I appreciate the enthusiasm, just want to point out that a regular contributor may be in a better position to make these suggestions as they will probably own driving this sort of work. If you are interested into building out what you laid in that message, I recommend you to ask questions to the team to verify if your plans are compatible with whatever in-flight plan there might be and maybe that you send more CLs so that the team can get to know you a bit more.
Author
Owner

@kfearsoff this is all unrelated to this issue filing: the decision to use more rust is a foregone conclusion. the meson work is one WIP CL away.

We aren't doing a fish-like rewrite it in rust project. It's not within our resource constraints. Likely there's going to be two parallel approaches to rust: rewriting things at the micro level and replacing things at the macro level. This is because ffi is hard.

But because of this approach, rewriting stuff in rust isn't a goal of the project, it's an option that can be taken where it makes sense. So an issue project is only useful to people more excited about killing c++ than more generally developing lix.

The issue you see above is sign posting the plans after we get a replacement protocol that is fully feature complete. That's at least a month or two away and design work hasn't yet started. There is a more immediately doable work item to rip out old protocol code though, which is the main subject of the issue.

@kfearsoff this is all unrelated to this issue filing: the decision to use more rust is a foregone conclusion. the meson work is one WIP CL away. We aren't doing a fish-like rewrite it in rust project. It's not within our resource constraints. Likely there's going to be two parallel approaches to rust: rewriting things at the micro level and replacing things at the macro level. This is because ffi is hard. But *because* of this approach, *rewriting stuff in rust* isn't a goal of the project, it's an option that can be taken where it makes sense. So an issue project is only useful to people more excited about killing c++ than more generally developing lix. The issue you see above is sign posting the plans *after* we get a replacement protocol that is fully feature complete. That's at least a month or two away and design work hasn't yet started. There is a more immediately doable work item to rip out old protocol code though, which is the main subject of the issue.
Member

Sorry sorry, I think it's my bad habit of thinking 20 years ahead showing up again. A concise way to formulate my thoughts would be more like this: "rewriting in Rust should be a separate issue/milestone, and the issue is about deleting legacy code paths, so let's not mention Rust here to not spread confusion". As you might have guessed - I got confused by the word "Rust" in the issue description and started talking about it, when the issue is 100% about C++ code.

About general excitement - I looked through the issues, but so far my Rusty brain hasn't found anything feasible for me besides documentation. Perhaps someone more comfortable with C++ can point me to good first issues (or even create a "good first issue" label) - that would be very helpful.

Sorry sorry, I think it's my bad habit of thinking 20 years ahead showing up again. A concise way to formulate my thoughts would be more like this: "rewriting in Rust should be a separate issue/milestone, and the issue is about deleting legacy code paths, so let's not mention Rust here to not spread confusion". As you might have guessed - I got confused by the word "Rust" in the issue description and started talking about it, when the issue is 100% about C++ code. About general excitement - I looked through the issues, but so far my Rusty brain hasn't found anything feasible for me besides documentation. Perhaps someone more comfortable with C++ can point me to good first issues (or even create a "good first issue" label) - that would be very helpful.
Author
Owner

Generally first issues are marked as both E/easy and help wanted by vibes of whether it sounds like a problem that is complicated. https://git.lix.systems/lix-project/lix/issues?q=&type=all&sort=&labels=154&state=open&milestone=0&project=0&assignee=0&poster=0

Generally first issues are marked as both E/easy and help wanted by vibes of whether it sounds like a problem that is complicated. https://git.lix.systems/lix-project/lix/issues?q=&type=all&sort=&labels=154&state=open&milestone=0&project=0&assignee=0&poster=0
Sign in to join this conversation.
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lix-project/lix#510
No description provided.