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

add support for dev-dependencies #338

Closed
BurntSushi opened this issue May 4, 2023 · 2 comments
Closed

add support for dev-dependencies #338

BurntSushi opened this issue May 4, 2023 · 2 comments

Comments

@BurntSushi
Copy link

My main ask is this: I would like a way to build fuzz targets such that they bring in the dev-dependencies of the crate being tested. This issue was discussed a little bit in #256, but I wanted to try again with a more clearly defined use case.

Folks have been working on improving the fuzzers for the regex crate. One such improvement is add arbitrary::Arbitrary impls for the various Ast types in regex-syntax instead of relying on parsing a pattern out of a &[u8]. The problem that has cropped up is that this seems to require adding a proper dependency on the arbitrary crate from regex-syntax. Even if we assume I take steps to make using this optional dependency on arbitrary difficult (like gate it on cfg(fuzzing) and name the feature something weird), then I don't want to do this for a few reasons:

  1. I want regex-syntax to remain free of dependencies, even optional ones. In the past, I've seen ways for even optional dependencies to impact cargo build even if they aren't used. This tends to come up with MSRV concerns. For example, if the current version of Cargo doesn't know how to read the manifest of an optional dependency. There may be other avenues for impact. It's a little fuzzy to me. But I know one thing: if it isn't a dependency then it can't have any impact.
  2. Regardless of what I do, Arbitrary impls are likely to be found to be useful by others, and it's likely they will find a way to use them. That puts them at risk of breakage.

Now I do have some other work-arounds that avoid adding new optional dependencies, but they're pretty gross. The leading contender at the moment is probably to duplicate the Ast types in the fuzzer target, derive(Arbitrary) on them and then convert them to the regex-syntax Ast types. The Ast types don't change much, but... there are a lot of them.

Also, popping up a level, cargo fuzz is supposed to be test infrastructure, right? If so, then it seems to me that, at least semantically, it should permit bringing in dev-dependencies just like cargo test does. That is, cargo fuzz seems to me like it's a lot closer to cargo test than it is to cargo build.

@Manishearth
Copy link
Member

Manishearth commented May 4, 2023

So the problem here is less a desire to do this, but rather that there is no way to do this cleanly.

Cargo does not offer much control over its internals. cargo fuzz works in a very specific way — it creates a new crate with binaries and runs them — because that's the only way to get cargo to run arbitrary binaries with the right dependencies built, without polluting examples/. That's the only option on the menu when it comes to dependency resolution; we can't ask Cargo "oh but you should include the dev deps of that other crate". Cargo is too complicated to emulate, that's a alsonon-starter for us (The guppy family of crates are magical and help here but they do not help with build invocations).

The custom test frameworks eRFC was in part intended to help projects like cargo-fuzz, though after a lot of pressure from the discussion some of the parts needed for cargo-fuzz were removed (and the RFC grew in complexity on the rustc side). It's never been fully implemented and it's a bit of a dead end now. Personally I'd recommend anyone intererested step back and implement the original, simpler proposal which did very little fancy stuff at the rustc level but instead exposed more knobs on cargo.


The workaround I suggest is either:

  • add a testing features; this is a common pattern amongst multi-crate workspaces
  • For your specific case, add an arbitrary feature. This is also somewhat common, especially since crates that want to derive Arbitrary may need to also depend on arbitrary impls from other crates: think of it like serde impls
  • define your fuzz targets using [[examples]] and use cargo rustc --example foo with the right flags

If the examples route can be made to work pleasantly I'm open to adding support for creating and running fuzz targets that way to cargo-fuzz


I'm closing this because we have no clear long- or short-term route to making this work, not because we don't want this. I consider "abuse examples/ to get it to work" to be something that needs a different issue, since it has huge impacts on the rest of cargo-fuzz's UX/interface and is basically creating a Whole New Tool.

If such a route appears in the future I'm happy to reopen this, and I do not mind this issue being used to track any progress/discussion on that route.

@Manishearth Manishearth closed this as not planned Won't fix, can't repro, duplicate, stale May 4, 2023
@BurntSushi
Copy link
Author

@Manishearth Thanks for the response! I was worried that this might be the case. That is, that this is less of a "we don't want this" and more of a "not feasible to support." Bummer. OK.

I'll check out the arbitrary crate in more thorough detail and reconsider that as an optional dependency. Otherwise, I'll probably just end up duplicating the Ast types within the fuzzer target. They almost never change, so it's a hopefully one-time "hold your nose and just do it" cost.

The example-abuse-hack seems neat, but I do definitely like the idea of custom test frameworks being the better option long term.

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

No branches or pull requests

2 participants