setup: cargo xtask fuzz or xflowey restore should install lcov

[mattkur]

Filing the feedback for discussion, but it's unclear where we draw the line of auto installed tooling. It was mildly annoying to install lcov only after I did a fuzz run. The docs already warn you to install it manually, but it would be nice if the tooling did it for ya.

[EMachin3]

Hello, I’m starting to look into working on this project, and I have a question about this issue. Right now, I'm assuming that the current approach relies on the executable being installed through the package manager, which can’t be done using the build tools. Would the plan be to somehow install lcov using a Rust dependency so that the system package manager doesn’t have to be involved?

[mattkur]

Hi @EMachin3 , thanks for your question and interest in OpenVMM.

You're spot on. cargo xflowey restore-packages already has some ability to install packages using the system's package installer; I'm suggesting that we also include the necessary tooling for fuzzing and fuzz coverage.

[mattkur]

@daprilik : it looks like this is a non-trivial amount of work (not a major effort, but not a one liner). It seems we would need to do something like this:

1. Adjust restore-packages to install cargo fuzz ... (below, and this needs to be modified to bring in assert_cmd)
2. Ditto, but also install rust nightly
3. Add a new step (e.g. copy install_git.rs to install_lcov.rs and install that package), and reference it in restore_packages.rs

This makes me think then that we'd want to refactor this all out into a init_fuzz task, and have that depend on those 3 steps.

diff --git a/flowey/flowey_lib_hvlite/src/_jobs/local_restore_packages.rs b/flowey/flowey_lib_hvlite/src/_jobs/local_restore_packages.rs
index c814a0c..1fa5941 100644
--- a/flowey/flowey_lib_hvlite/src/_jobs/local_restore_packages.rs
+++ b/flowey/flowey_lib_hvlite/src/_jobs/local_restore_packages.rs
@@ -7,6 +7,7 @@
 use crate::init_openvmm_magicpath_openhcl_sysroot::OpenvmmSysrootArch;
 use crate::run_cargo_build::common::CommonArch;
 use flowey::node::prelude::*;
+use flowey_lib_common::download_cargo_fuzz;

 flowey_request! {
     pub struct Request{
@@ -26,12 +27,15 @@ fn imports(ctx: &mut ImportCtx<'_>) {
         ctx.import::<crate::init_openvmm_magicpath_openhcl_sysroot::Node>();
         ctx.import::<crate::init_openvmm_magicpath_protoc::Node>();
         ctx.import::<crate::init_openvmm_magicpath_uefi_mu_msvm::Node>();
+        ctx.import::<download_cargo_fuzz::Node>();
     }

     fn process_request(request: Self::Request, ctx: &mut NodeCtx<'_>) -> anyhow::Result<()> {
         let Request { arch, done } = request;

-        let mut deps = vec![ctx.reqv(crate::init_openvmm_magicpath_protoc::Request)];
+        let mut deps = vec![
+            ctx.reqv(crate::init_openvmm_magicpath_protoc::Request),
+            ctx.reqv(download_cargo_fuzz::Request::InstallWithCargo)];

         match arch {
             CommonArch::X86_64 => {

[daprilik]

I would warn against equating "trivial" with "one-liner", both in flowey, and in general. There are plenty of "one-liners" that are non-trivial, and many hunks of code that are verbose, but easy to follow. For better or worse, flowey currently falls firmly into the latter category (at least, most of the time).

In any case, regarding the steps you've outlined:

1. Installing cargo fuzz should indeed be easy using the diff you suggested
2. This will be a bit trickier... I'll expand on this more below
3. Having a new install_lcov might be overkill, when a simple inline call to install_dist_pkg would suffice
   • similarly, while we could have a init_fuzz Node to bundle these ops together, that seems like a judgement call we can make during PR review / discussion. I'm not sure its necessary

---

So yeah, regarding nightly Rust...

At the moment, the install_rust Node is somewhat limited, insofar as it assumes each pipeline only runs using a single Rust toolchain version. See the TODO on line 25 of install_rust.rs for more context. This limitation hasn't been particularly stifling thus-far, as we don't currently run anything in CI using the nightly toolchain. Internally, the one pipeline that does nightly-related stuff uses a sneaky-hack to use the stable toolchain instead, where it sets "RUSTC_BOOTSTRAP=1" (which is not a hack we should cement in any way, shape, or form!).

That said, now that we are open-source, I expect we'll want to run various tools that require nightly in CI (notably: running tests under miri), so this is a bridge that we'll need to cross sooner or later...

I see 2 options here:

• The Easy Approach: have users manually install nightly Rust, and sidestep this complexity entirely. This could be made more ergonomic by updating cargo xtask fuzz to check if nightly rust is installed, and emit a helpful error message if its not found.
• The Hard Approach: spend a bit of time yak-shaving the question of installing + executing jobs using multiple Rust toolchains in a single Job, by updating the install_rust.rs logic.

IMO, I would lean towards the easy approach for now, and in the context of this particular Issue, simply update the restore-packages code to install lcov and cargo fuzz.

---

I'll also note that we currently have an internal-only flowey pipeline which uses these various fuzzer-related nodes to build and submit fuzzers to our OneFuzz infrastructure. That's not particularly helpful here, ofc, given that you'd need to be a MSFT employee to see it...

That said, there's nothing in that pipeline that is particularly complicated / useful to bring back to oss, so aside from being a useful point-of-reference for folks at MSFT, it's not directly related to the problem at hand.

[mattkur]

That's all fair enough, and thanks Daniel. I'll look at the internal pipeline to see if I glean anything over.

FWIW, the "trivial" to which I was referring was in part what you mentioned: the downstream implications of juggling different rust toolchains, transitive dependencies, and the like. While I live in the systems space, where single line (or bit!) differences have significant downstream consequences, I am leery of one-line config-is-code changes as well. I think we're saying roughly the same thing here: I just want to emphasize that I hear and support what you're saying about conflating amount of code vs. ease of implementation.