proc-macro-crate icon indicating copy to clipboard operation
proc-macro-crate copied to clipboard

Switch to tomling crate for reduced dependencies

Open zeenix opened this issue 10 months ago • 3 comments

tomling crate was specifically created to provide a simple TOML parser with fewer dependencies and focus on Cargo.toml parsing.

Fixes #37.


Creating a draft PR first to get some input before I roll out the tomling release and then switching to released version in this PR.

zeenix avatar Jan 11 '25 17:01 zeenix

However, I have some questions before :)

Fair. :)

  1. Did you make any compile time comparisons? What is the difference?

Right now there is no noticeable difference tbh. However:

  1. I've not yet applied any optimizations to tomling. There are some rather low-hanging ones, such as parsing the string as bytes (which in other projects have yielded huge improvements) so I'm expecting this to improve.
  2. The release binary size (with LTO enabled), already show a slight improvement (around 7KB in busd).

I understand completely if you'd like to wait till tomling brings real significant improvements before switching though.

2. How good is tomling tested? Did you run any tests against some larger set of toml files to ensure it doesn't crash etc?

So we've added the amazing toml-test-harness to our test/CI, which tests all kinds of corner cases etc. While we don't have any crashes, since we're not yet 100% spec-compliant (e.g string escaping isn't supported yet), not all tests pass and hence explicitly ignored.

test result: ok. 399 passed; 0 failed; 165 ignored; 0 measured; 0 filtered out; finished in 0.01s

To save you some math, that's 70% coverage. Other than that, we also have fuzzing enabled (which is why I'm fairly confident we don't crash on any input) and some other test cases based on real world Cago.toml files (currently zbus and tokio). The test case of proc-macro-crate also help here.

Again, if you'd like to wait for full spec compliance, I understand completely.

zeenix avatar Jan 13 '25 13:01 zeenix

Right now there is no noticeable difference tbh. However:

I mean this crate should compile faster? That being the entire point of using a slimmer dependency?

bkchr avatar Jan 13 '25 20:01 bkchr

Right now there is no noticeable difference tbh. However:

I mean this crate should compile faster?

It should, yes. :) However, at the moment, it doesn't for the reasons I mentioned. As I wrote, I understand if you want to wait till that is the case before merging. I'm hopeful it will still be soon.

That being the entire point of using a slimmer dependency?

My impression was that it was mainly about the binary size (at least for me that was the only motivation) but yeah, this would be one of the 2 benefits in the end.

Another benefit would be "perceived" compile speed. People often look at the number of deps being downloaded, instead of actual compile time to judge Rust projects. It's super silly, I know but it is what it is. :shrug:

Anyway, let me know if you want to wait.

zeenix avatar Jan 13 '25 21:01 zeenix

I've deprecated tomling in favour of the latest version of toml, so closing this (it's not like it was going anywhere).

zeenix avatar Jul 23 '25 07:07 zeenix