Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port to Rust #262

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kristof-mattei
Copy link

This is risky. I know, please bear with me.

I've had some fun in the last couple of day studying how a random project could be converted to Rust.

Things that were important:

  • End result: advantages, disadvantages
  • Any pitfalls now that we'd be using a Rust version
  • Do we have CLI compatibility? 90%? 100%?
  • Finding out the maturity of the Rust toolchain to integrate into the larger cohort
  • How many times I would still be dealing with raw pointers!

So, what's my expectation by opening this PR? Nothing really. I expect it'll be closed.

However, since I did spend a significant time on this it felt like a waste of time NOT to share this with the community.

Looking at it now, as someone who didn't do that much C/C++ it definitely feels more readable. In Rust things are just... more explicit. And yea, I know I'm talking to these wrappers of just the good ol' libc, but that doesn't take away from the fact I feel that intent is better expressed in the Rust version.

I learned about the definition of unsafe behavior, how to get around it, mapping back and forth between C and Rust and how to express things in a more Rust-manner way.

One thing I looked at was how to have the 5 global variables as part of main and then just pass them around all the time, this would avoid the need for them being static. But that felt unneeded given the single-threaded mode this application runs in.

Next to that I also looked into the arrays where the i32 representation of the signal is the index of an array, something seen quite often in C. I wanted to use a HashMap, but because of not wanting to pass everything around I also abandoned that.

Lastly, the array access is probably faster, a little bit more checks.

A good amount of time was then spent in understanding how tox works in conjunction with ./setup.py. Let this be reminder that the magic trick was to run python ./setup.py bdist to go through all the steps and find the hook point to copy our executable to the python bin folder.

Also not easy was tox itself. I still haven't figured out how to actually attach a debugger. I tried VS Code and PyCharm, both interfacing with WSL but I never got the experience I wanted to.

Lastly a good amount of time was spent on making the CLI interface match. The fact that dumb-init supports dumb-init sh -c echo hi didn't work with the structopts parsing library, so I had to switch to getopts. But surprisingly I got it to work. Having drop-in capabilities was extremely important for this. Can't change the contract of all of our customers right?

Rust dependencies worth used:

Missing:

  • Debian build
  • Code coverage

@kristof-mattei kristof-mattei marked this pull request as ready for review October 28, 2021 04:27
@kristof-mattei kristof-mattei changed the title Rewrite in Rust (no merge expectation) Port to Rust (no merge expectation) Oct 31, 2021
@kristof-mattei kristof-mattei changed the title Port to Rust (no merge expectation) Port to Rust Oct 31, 2021
@chriskuehl
Copy link
Contributor

Hey @kristof-mattei, thanks for sending this very cool PR! This is actually something I tried doing a few years ago for fun, but didn't get nearly as far along as you did here. It's quite neat that you were able to keep the existing tests as that's a great way to ensure that this rewrite is a drop-in replacement.

As you mentioned I don't think we'll be able to merge this PR just given its size and scope; there's nothing wrong with the code at all that I can tell, we just aren't really actively developing dumb-init at the moment beyond maintenance, and I don't think we have the bandwidth (or the expertise) to deal with a rewrite. I'm also a bit concerned that it would add an extra burden on package maintainers since dumb-init is distributed by a lot of Linux distros and my impression is that it can be more difficult currently to distribute a Rust app and its dependencies at the system level.

With that said, if you want to maintain this as a separate project or even just keep it in your GitHub as a cool project, we'd be more than happy to link it from the README like we do with tini to help people who are interested to find it.

Also not easy was tox itself. I still haven't figured out how to actually attach a debugger. I tried VS Code and PyCharm, both interfacing with WSL but I never got the experience I wanted to.

I'm not too familiar with those debuggers myself (I almost exclusively use pdb) but it may help to know that tox creates a virtualenv under .tox/py38 and you can use those executables directly without running tox (e.g. .tox/py38/bin/pytest tests to run tests). That may make it easier to hook up to a graphical debugger.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants