feat: replace the --release flag with a generic --profile flag

[Ekleog-NEAR]

Also make the default profile "fuzz", and document that it should probably have codegen-units set to 1, in replacement to the semi-fix that had been implemented only when the release mode was set.

The drawback of this change is it forces people to have a fuzz profile by default, and to add codegen-units = 1 themselves. I think it is a good thing that said, because of rust’s philosophy of "better explicit than implicit" and the fact that 3 lines per crate are not too much boilerplate:

[profile.fuzz]
inherits = "release"
codegen-units = 1
debug-assertions = true # not strictly speaking necessary, but I think it’s useful

The big advantage is, it makes it much easier to eg. turn debug assertions on or off, and people can always use --profile release if they want to build in regular release mode (though then they’ll probably hit the codegen-units issue).

WDYT?

[Ekleog-NEAR]

Seeing the usual afl ci failures, AFAICT they’re unrelated, as 1.64 / ubuntu / afl / NONE in particular did pass and the failing ones were just other rustc versions with also ubuntu / afl / NONE

[camshaft]

Should we default to a profile that is there by default? My worry is this will make the UI worse and just complain about a missing profile with no actions on how to fix it for bolero.

[Ekleog-NEAR]

So my thinking was, it’s better to fail loudly than to fail silently.

If we default to release, then we open ourselves up to the LTO issue you mentioned above, and there won’t be debug-assertions. If we default to debug, then the fuzzer will be way too slow for reasonable running.

Right now, the error message for people who didn’t read through the book would be:

error: profile `fuzz` is not defined

I think it’s reasonable that the user would search the internet with this error message, and fall on the library-installation.md page (to which I just added this error message, to make sure it’s easy to find).

We could also add a log message at the beginning of each cargo-bolero invocation, reminding to read the book if it fails with this error message, but that’d feel overkill to me.

WDYT?

[camshaft]

Ideally the user wouldn't have to search to figure out what's wrong. Like, rustc's error messages usually provide all of the information you need to fix it inline. That being said, I do think profiles are a better way to go, especially considering the issues with LTO.

Can you open an issue to track possible solutions to improve the UX for this?

[Ekleog-NEAR]

Just did it! See #168 :)

[Ekleog-NEAR]

error: package linux-raw-sys v0.4.5 cannot be built because it requires rustc 1.63 or newer, while the currently active rustc version is 1.57.0

(in the CI failures)

I guess now’s a good time to revisit bumping the MSRV, as this PR doesn’t even change cargo.toml, so I guess one of bolero’s dependencies indirectly bumped its MSRV since the first version of this PR?

[camshaft]

Can you update the examples to include a fuzz profile, instead? The bolero workspace should probably also include one as well.

[Ekleog-NEAR]

Done, thank you for the idea :)

[camshaft]

All of the tests should be updated to use the fuzz profile. And then we're good to merge :)

[Ekleog-NEAR]

Oh right, thank you! I just pushed a commit removing the --profile argument altogether, as bolero defaults to the fuzz profile :)

Also just in case, for #161, I’m waiting for your feedback on whether the BOLERO_LIBTEST_HARNESS environment variable is a red herring or not, and on whether sometimes libfuzzer should be able to run the binary without going through the libtest harness :)

[Ekleog-NEAR]

Seems like cargo clippy probably added new checks, as it’s failing on code I didn’t touch. Should I fix them here? Also, WDYT about pinning the version of clippy in CI, so that this kind of issues doesn’t arise too often?

Also, for CI, unpinned dependencies are failing pre-rust-1.65, hence a lot of the tests failing. I’d suggest either bumping the MSRV, or committing a Cargo.lock so that at least CI will pass (but users won’t be able to get things to pass anyway, I personally won’t believe in MSRV until rust-lang/cargo#9930 gets done)

[camshaft]

Yeah I think we'll need to bump, unfortunately. We can merge this PR as is.