Don't rebuild coverage target on each coverage run #327
Conversation
src/project.rs
Outdated
| if build.coverage { | ||
| Ok(env::current_dir()? | ||
| .join("target") | ||
| .join(default_target()) | ||
| .join("coverage")) | ||
| } else { | ||
| Ok(build | ||
| .target_dir | ||
| .as_ref() | ||
| .map(PathBuf::from) | ||
| .unwrap_or_else(|| self.project_dir.join("target"))) |
There was a problem hiding this comment.
If the user explicitly passes in a --target-dir I think that should take priority over the default coverage target directory. So I think this should look something like:
if let Some(target_dir) = build.target_dir {
// use user-provided target dir...
} else if build.coverage {
// use coverage target dir...
} else {
// use default target dir...
}
src/project.rs
Outdated
| if build.target_dir.is_some() || build.coverage { | ||
| let target_dir = self.target_dir(&build)?; |
There was a problem hiding this comment.
Can we make the target_dir method return an Option and do if let Some(target) = self.target_dir(&build) here? That way we are less likely to specify the --target-dir flag to cargo when we intend to use the default (and the default might have been configured to a non-default location by the user in .cargo/config.toml or via env vars so it is best to omit the --target-dir flag than to guess its location by appending <project>/target or something like that)
src/project.rs
Outdated
| .target_dir | ||
| .as_ref() | ||
| .map(PathBuf::from) | ||
| .unwrap_or_else(|| self.project_dir.join("target"))) |
There was a problem hiding this comment.
self.project_dir.join("target") is going to be incorrect if the user using either the CARGO_TARGET_DIR=... environment variable or configuring it via .cargo/config.toml. In this case, we should return None.
|
Thanks for the quick review! Let me know if this works :) |
Aside from reducing the amount of output from repeated
cargo runexecutions (particularly when there's warnings), this also reduced the total time to runcargo fuzz coveragein my project from ~15 to ~4 seconds.The new behavior should still be safe from other code changes in the project being fuzzed while running (and other
cargo fuzzinvocations), because the coverage directory is kept separate.