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

Rewrite scubainit in Rust #232

Merged
merged 15 commits into from
Jan 7, 2024
Merged

Rewrite scubainit in Rust #232

merged 15 commits into from
Jan 7, 2024

Conversation

JonathonReinhart
Copy link
Owner

@JonathonReinhart JonathonReinhart commented Dec 10, 2023

This PR rewrites scubainit -- previously implemented in C -- in Rust.

Rust provides stronger type- and memory-safety guarantees, and additional capabilities via a rich stdlib and crate ecosystem.

Copy link

github-actions bot commented Dec 10, 2023

Test Results

  5 files    5 suites   1m 32s ⏱️
169 tests 167 ✅  2 💤 0 ❌
845 runs  835 ✅ 10 💤 0 ❌

Results for commit a29a850.

♻️ This comment has been updated with latest results.

@JonathonReinhart
Copy link
Owner Author

@jammiess @elipsitz Care to review this?

@elipsitz
Copy link

Yeah sure, I can take a look next week when when I’m back at a computer

scubainit/src/lib.rs Outdated Show resolved Hide resolved
scubainit/src/main.rs Outdated Show resolved Hide resolved
scubainit/src/main.rs Show resolved Hide resolved
scubainit/src/main.rs Outdated Show resolved Hide resolved
scubainit/src/main.rs Outdated Show resolved Hide resolved
scubainit/src/entfiles.rs Outdated Show resolved Hide resolved
scubainit/src/shadow.rs Outdated Show resolved Hide resolved
scubainit/src/main.rs Outdated Show resolved Hide resolved
scubainit/src/util.rs Outdated Show resolved Hide resolved
scubainit/src/entfiles.rs Outdated Show resolved Hide resolved
@JonathonReinhart JonathonReinhart force-pushed the scubainit-rust branch 2 times, most recently from 98a25bf to 4646e48 Compare December 31, 2023 19:55
We need to set some special flags for static builds were are as small as
reasonably possible.

Ideally we'd handle this with only cargo, but:
- Cargo only supports a subset of rustc options, so we have to set
  RUSTFLAGS somewhere.
- The top-level Makefile shouldn't need to know about cargo

This also runs 'rustup add target' as a dependent command, for
simplicity. This adds ~100 ms after the initial setup.
Set USE_SCUBAINIT_RUST=1 to use the new scubainit-rs.
JonathonReinhart and others added 12 commits January 1, 2024 23:33
Previously, the code would simply skip invalid entries. Now, we pass
these errors up from the iterator to let the caller handle this as
desired.

This follows the pattern of std::fs::ReadDir whose Iterator impl has
Item = Result<...>.

scubainit main handles this in the most conservative way by aborting on
any invalid entry.
This reduces duplication and improves readability of the from_line()
implementations.
Rather than use lifetimes on the EntFileReader and EntFileWriter
structs, we can implement the common into_inner() convention. This
consumes the outer object returning the inner wrapped object, which in
this case is the File which we want to reuse. This matches e.g.
`std::io::BufReader::into_inner()`.

Note that this also requires changing the way the reader is iterated: We
use `for x in &mut reader` which avoids moving the `reader` object into
the loop, allowing `into_inner()` to be subsequently called. See the
discussion here:
#239 (comment).
This adds several psuedo-targets as listed in the help. The goal here is
to allow "make all" without requiring checks, which is useful during
debugging.
This runs tests, lints, and checks code formatting.
@JonathonReinhart JonathonReinhart added the scubainit Relating to the scubainit binary which runs at container startup label Jan 2, 2024
@JonathonReinhart JonathonReinhart merged commit 1e26d0e into main Jan 7, 2024
11 checks passed
@JonathonReinhart JonathonReinhart deleted the scubainit-rust branch January 7, 2024 03:46
JonathonReinhart added a commit that referenced this pull request Mar 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scubainit Relating to the scubainit binary which runs at container startup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants