rust-fuzz / cargo-fuzz

Command line helpers for fuzzing
https://rust-fuzz.github.io/book/cargo-fuzz.html
Apache License 2.0
1.55k stars 109 forks source link

fix: use cargo-metadata to support init command in workspace #349

Closed 0xalpharush closed 11 months ago

0xalpharush commented 11 months ago

closes https://github.com/rust-fuzz/cargo-fuzz/issues/333

Instead of manually parsing the toml file and failing in workspace members, the package name and edition are obtained via cargo-metadata (this also seems to obviate the need to pass that path which is a breaking change to a public API). The way the tests are setup was making cargo-metadata think that each test is a member of cargo-fuzz so I added an empty workspace entry to the tests' Cargo.toml.

fitzgen commented 11 months ago

I've had issues with cargo-metadata blowing up compile times in the past. Can you provide the timings for cargo clean; cargo build both on main and on this feature branch?

0xalpharush commented 11 months ago

Using hyperfine on my M1, it adds ~1s

Feature branch:

Benchmark 1: cargo clean; cargo build
  Time (mean ± σ):      6.016 s ±  0.043 s    [User: 21.272 s, System: 2.145 s]
  Range (min … max):    5.968 s …  6.093 s    10 runs

Main branch:

 Benchmark 1: cargo clean; cargo build
  Time (mean ± σ):      5.009 s ±  0.113 s    [User: 14.871 s, System: 1.736 s]
  Range (min … max):    4.859 s …  5.199 s    10 runs
fitzgen commented 11 months ago

Looks like CI is failing? Don't have time to dig in myself at the moment.

0xalpharush commented 11 months ago

The tests were relying on the panic error format pre rustc 1.73.0 https://blog.rust-lang.org/2023/10/05/Rust-1.73.0.html