fossa-cli icon indicating copy to clipboard operation
fossa-cli copied to clipboard

analysis of Rust workspace missing members

Open juntyr opened this issue 4 years ago • 3 comments

I run fossa analyze on a Rust repo with a workspace Cargo.toml file. My workspace contains several default members, which are all 'roots' of overlapping dependency trees of my project. Before the release of v3.0, I was manually configuring the analysis to just take the existing Cargo.lock file, which worked. Now, the automatic analysis seems to somehow miss all but one of the workspace default members as well as some (but not all) of their exclusive dependencies. How can I fix this issue? Thanks for your help!

juntyr avatar Nov 13 '21 20:11 juntyr

Thanks for the report, I did a quick write-up based on my experience with rust, cargo, and the actual analyzer in question:

I think this is just a simple oversight based on how we reinterpret workspaces and members. Having written that code myself (though that was a while ago), I'm pretty sure we expect workspaces to be root packages instead of virtual manifests.

Since I'm looking into it now, I'm going to just drop some findings here and we can link back later in our internal ticket tracker.

I don't think default-members should be considered at all, the cargo docs clearly state:

  • The intention of default-members is to control which workspaces are selected in default cargo behavior (in package commands, like build and check).
  • When specified, default-members must expand to a subset of members.

Based on these two facts, I think we just need to handle members.

Currently, we identify cargo packages by a Cargo.toml file, and run cargo generate-lockfile (idempotent if lockfile exists and user-level/system-level package metadata cache is up-to-date), and then parse the output of cargo metadata. Somewhere in there, I'd bet we're making an assumption about which packages are considered root, for the purposes of building the graph of dependencies. It's probably worth modeling the workspaces natively, and then performing lossy conversion to the current graph model (which will help us if we later add workspace support to the web app or analysis service).

This may also be the right time to add a static analysis tactic to cargo. There's good reason to believe that it would produce equivalent results.

skilly-lily avatar Nov 14 '21 21:11 skilly-lily

Hi @MomoLangenstein, we are planning to work on a fix for this issue soon. Tracked as ANE-139 internally.

zlav avatar May 21 '22 00:05 zlav

Hi @MomoLangenstein, we are planning to work on a fix for this issue soon. Tracked as ANE-139 internally.

That’s awesome, thanks!

juntyr avatar May 21 '22 02:05 juntyr