Create test to catch duplicate dependencies

[yaahc]

Is your feature request related to a problem? Please describe.

Recently we found a regression in our metrics system in zebrad. The issue ended up being that we'd accidentally duplicated the metrics dependency during a recent refactor. The crates on the old version would silently fail to access the exporter when reporting metrics because the variable containing the exporter was duplicated in each version, and only the newer version's global exporter had been initialized.

Describe the solution you'd like

We've already fixed this problem in #1561 but we'd like to prevent this from happening again in the future. The plan is to use guppy to write a test/lint to catch and report duplicated dependencies are part of our CI process. The author @sunshowers helpfully already pointed me to where they've already implemented this lint for their work codebase (link).

We should more or less vendor this lint into our codebase. If possible we'd also like to do so in a way that is reusable so that other rust projects can utilize this same lint easily.

Design / Mentorship Instructions

TBD

I still need to talk some more with Rain about the design for the reusable version of diem/guppy.rs. My expectation is that it would look similar to clippy, but Rain has thought much more about this than I have so I want to get my ducks in a row before I commit to writing out a design.

I've talked to Rain and the plan we came up with is to create a cargo subcommand like clippy which we've decided to call seamare that is based on the guppy code in diem. This subcommand can then be distributed on crates.io and run in CI.

MVP Checklist

• create three new crates in zebra
  • seamare: The main library that defines the lint engine and traits
  • seamare-lints: A separate library that depends on seamare and implements the various lints we export by default
  • cargo-seamare: A binary crate that acts as an external cargo subcommand, depends on seamare and seamare-lints, instantiates the engine, then passes in the list of lints provided by seamare-lints to be run
• copy the various lint trait definitions defined in guppy into seamare
  • https://github.com/diem/diem/blob/master/devtools/x-lint/src/project.rs#L8-L22
  • https://github.com/diem/diem/blob/master/devtools/x-lint/src/package.rs#L9-L16
  • https://github.com/diem/diem/blob/master/devtools/x-lint/src/content.rs
  • https://github.com/diem/diem/blob/master/devtools/x-lint/src/file_path.rs
• copy the duplicate dependencies lint from diem into seamare-lints/src/duplicate_deps.rs
• In cargo-seamare: create a LintEngine that loads the package graph via guppy, takes a set of types that implement these various traits, and then executes each of the lints via the traits against the package graph

Additional Goals

• Have LintEngine run all of the lints in parallel
• Copy the rest of the applicable lints that exist in diem into seamare-lints
  • Each lint has it's own file
  • Each lint has tests (clippy's repo will be useful for inspiration here)
• Support out of tree lints (Hard, ideas below)
  • Create an option in cargo-seamare for generating a skeleton project that is essentially a copy of itself, which can then be edited and have new lints added in via source
  • Support wasm plugins
  • dylib plugins???
• Move seamare, cargo-seamare, and seamare-lints into their own organization and repo to consolidate implementations with upstream source

Describe alternatives you've considered

I've not really considered any other alternatives, as far as I know guppy is the exact right tool to solve this problem, and this is what it was designed for.

[teor2345]

This looks good.

Once we have a potential contributor, let's schedule the mentoring and review work as part of our sprints?

[sepiropht]

i know some rust but never done crypto before, can i contribute ?

[yaahc]

i know some rust but never done crypto before, can i contribute ?

Absolutely!

[sepiropht]

Ok cool i take it !

[sepiropht]

I just have to follow all the check listed above ?

[yaahc]

I just have to follow all the check listed above ?

Yup, and let me know if you have any questions.

[teor2345]

@sepiropht try to focus on the "MVP Checklist" first.

The "Additional Goals" can wait until later.

[sepiropht]

Right now I'm not able to build the project, i have compilation errors in zebra-state zebra-state/src/service/check/difficulty.rs:138:14,
I use the current stable rustc toolcahin and i have all required dependencies
The only thing that i missing maybe is zebrad target for the toolchain, how do i set that ?

[teor2345]

@sepiropht please open another ticket with:

• the exact Zebra commit you're building
• the command you're using to build Zebra
• your rustc version
• the exact error you're getting

You might find the bug report template helpful, but it's designed for runtime bugs, not compilation bugs. So it asks for some information you won't have.

https://github.com/ZcashFoundation/zebra/issues/new/choose

[sepiropht]

Finally after rustup update i don't have this problem anymore and the project build. Thank @teor2345

[yaahc]

Hey @sepiropht, are you still working on this?

[WindSoilder]

Curious about this, does it needs to check the direct duplicate dependencies? Does nested dependencies need to be check?

[yaahc]

Curious about this, does it needs to check the direct duplicate dependencies? Does nested dependencies need to be check?

I think the check is already implemented upstream and that it checks for duplicate direct dependencies, but it might be a good idea to make it configurable.

[teor2345]

Done recently using cargo deny check bans.