Stream: git-wasmtime

Topic: wasmtime / Issue #2225 Add cargo-deny?

Wasmtime GitHub notifications bot (Sep 24 2020 at 13:29):

maxded opened Issue #2225:

Hey everyone, I was wondering if the cargo-deny plugin, a tool for linting your dependencies, could be added to the wasmtime repository and integrated into CI (or at least be discussed/considered if that hasn't been done yet :)). Some of the advantages it has to offer are:

1. License check: used to verify that every crate you use has a license term you find acceptable.
2. Bans check: used to deny (or allow) specific crates, as well as detect and handle multiple versions of the same crate.
3. Advisory check: used to detect issues for crates by looking in an advisory database.
4. Sources check: used to ensure crates only come from sources you trust.

At Embark Studios, we've just started making heavy use of wasmtime and, as we are running cargo-deny in our CI, found multiple versions of the same dependency within wasmtime. Specifically, wasmtime-debug, wasmtime-jit, and wasmtime-obj (all version 0.20.0) all have wasmparser 0.59.0 as a dependency but also object 0.21.1, which in turn has wasmparser 0.57.0 as a dependency.

Multiple versions of the same dependency can cause all sorts of issues so we like to limit this as much as we can! As wasmtime currently isn't running cargo-deny this "bug" might have gone by completely unnoticed. If you did notice but had your own reasons not to "fix" it, that's absolutely fine as well, just wanted to use this specific issue as an example!

Let me know what you guys think! Both the advantages and/or disadvantages of using cargo-deny for wasmtime.

Wasmtime GitHub notifications bot (Sep 24 2020 at 13:33):

bjorn3 commented on Issue #2225:

Specifically, wasmtime-debug, wasmtime-jit, and wasmtime-obj (all version 0.20.0) all have wasmparser 0.59.0 as a dependency but also object 0.21.1, which in turn has wasmparser 0.57.0 as a dependency.

cranelift-object doesn't activate the read feature of object, so wasmparser 0.57.0 is not compiled. This doesn't mean that using a single version of wasmparser is nice.

Wasmtime GitHub notifications bot (Sep 24 2020 at 13:34):

bjorn3 edited a comment on Issue #2225:

Specifically, wasmtime-debug, wasmtime-jit, and wasmtime-obj (all version 0.20.0) all have wasmparser 0.59.0 as a dependency but also object 0.21.1, which in turn has wasmparser 0.57.0 as a dependency.

cranelift-object doesn't activate the read feature of object, so wasmparser 0.57.0 is not compiled. This doesn't mean that using a single version of wasmparser isn't nice though.

Wasmtime GitHub notifications bot (Sep 24 2020 at 17:20):

abrown commented on Issue #2225:

In general I like the idea of adding this type of thing (cargo-audit also comes to mind); I wouldn't want it to be constantly breaking the build, though... How many other issues did it find?

Wasmtime GitHub notifications bot (Sep 24 2020 at 18:22):

pchickey commented on Issue #2225:

This seems like a good idea. I was not familiar with cargo-deny but it looks like a useful tool.

Fastly runs cargo-audit in CI, and I'd support doing the same here.

Agree that duplicate lock entries from direct dependencies should be avoided in CI - this shouldn't happen very frequently, and when it does its straightforward to fix.

It looks like we have two autocfg versions that are transitive deps, that is harder to resolve, so having an exceptions list seems like a reasonable rule there.

I don't have an opinion on the licensing checks. We could disable linting those until we can get legal advice on what we should allow.

Wasmtime GitHub notifications bot (Sep 24 2020 at 18:23):

pchickey edited a comment on Issue #2225:

This seems like a good idea. I was not familiar with cargo-deny but it looks like a useful tool.

Fastly runs cargo-audit in CI, and I'd support doing the same here.

Agree that duplicate lock entries from direct dependencies should be avoided in CI - this shouldn't happen very frequently, and when it does its straightforward to fix.

It looks like we have two autocfg versions that are transitive deps, that is harder to resolve, so having an exceptions list seems like a reasonable rule there.

I don't have an opinion on the licensing checks. We could disable linting those until we can get legal advice on what we should allow.

Wasmtime GitHub notifications bot (Sep 25 2020 at 08:04):

maxded commented on Issue #2225:

In general I like the idea of adding this type of thing (cargo-audit also comes to mind); I wouldn't want it to be constantly breaking the build, though... How many other issues did it find?

Some more issues that cargo-deny found are:

3 duplicate dependencies for object:
(wasmtime-debug, wasmtime-jit, and wasmtime-obj) -> object 0.21.1
wasmtime-profiling -> object 0.19.0.
(wasmtime and wasmtime-runtime) -> backtrace 0.3.50 -> object 0.20.0.

2 duplicate dependencies for gimli
(wasmtime and wasmtime-runtime) -> backtrace 0.3.50 -> addr2line 0.13.0 -> gimli 0.22.0
(cranelift-codegen, wasmtime-debug, wasmtime-environ, wasmtime-jit, and wasmtime-profiling) -> gimli 0.21.0

As for the constantly breaking the build problem, cargo-deny is extremely configurable so it's quite easy to have a temporary or permanent workaround. In our case, the duplicate dependency "broke" the build in CI so I investigated the issue, realized what was going on, and as a result told cargo-deny to skip those specific crates. So in your deny.toml simply add:

skip = [
    # object used by wasmtime-debug, wasmtime-jit, and wasmtime-obj
    # uses wasmparser 0.57.0
    { name = "wasmparser", version = "<0.59.0" },

    # cranelift-codegen, wasmtime-debug, wasmtime-environ, wasmtime-jit, and
    # wasmtime-profiling us old version of gimli.
    { name = "gimli", version = "0.21.0" },

    # wasmtime, wasmtime-runtime, and wasmtime-profiling use old version
    # of object
    { name = "object", version = "<0.21.1" },
]

As you can see, cargo-deny is by no mean an absolute enforcer of, in this case, duplicate dependencies. It's an extremely useful tool to at least be _aware_ that there are duplicates. How you resolve it is up to you :)

Wasmtime GitHub notifications bot (Sep 25 2020 at 08:14):

bjorn3 commented on Issue #2225:

3 duplicate dependencies for object:
(wasmtime-debug, wasmtime-jit, and wasmtime-obj) -> object 0.21.1
wasmtime-profiling -> object 0.19.0.

This one should be changed to object 0.21.1.

(wasmtime and wasmtime-runtime) -> backtrace 0.3.50 -> object 0.20.0.

This one will need backtrace to update the object dependency. Downgrading the object dependency for wasmtime crates won't work as we sometimes need new features in object (or at least I do).

2 duplicate dependencies for gimli
(wasmtime and wasmtime-runtime) -> backtrace 0.3.50 -> addr2line 0.13.0 -> gimli 0.22.0
(cranelift-codegen, wasmtime-debug, wasmtime-environ, wasmtime-jit, and wasmtime-profiling) -> gimli 0.21.0

Makes sense to update all to gimli 0.22.0. It contains several bug fixes, though those are only for the read side,

Wasmtime GitHub notifications bot (Dec 16 2020 at 10:30):

lpil commented on Issue #2225:

Hello all! Is there interest in having cargo-deny added to the CI build? If so I would be happy to pick up this task, along with resolving any issues that arise. Thanks

Wasmtime GitHub notifications bot (Dec 16 2020 at 19:52):

pchickey commented on Issue #2225:

It seems like this will be a reasonable, so please go ahead and open a PR that adds it to our CI. I expect that most of what it flags will be easy to resolve, but we can take a look at the PR if there are any lints we need to turn off.

Wasmtime GitHub notifications bot (Dec 18 2020 at 21:35):

pchickey closed Issue #2225:

Hey everyone, I was wondering if the cargo-deny plugin, a tool for linting your dependencies, could be added to the wasmtime repository and integrated into CI (or at least be discussed/considered if that hasn't been done yet :)). Some of the advantages it has to offer are:

1. License check: used to verify that every crate you use has a license term you find acceptable.
2. Bans check: used to deny (or allow) specific crates, as well as detect and handle multiple versions of the same crate.
3. Advisory check: used to detect issues for crates by looking in an advisory database.
4. Sources check: used to ensure crates only come from sources you trust.

At Embark Studios, we've just started making heavy use of wasmtime and, as we are running cargo-deny in our CI, found multiple versions of the same dependency within wasmtime. Specifically, wasmtime-debug, wasmtime-jit, and wasmtime-obj (all version 0.20.0) all have wasmparser 0.59.0 as a dependency but also object 0.21.1, which in turn has wasmparser 0.57.0 as a dependency.

Multiple versions of the same dependency can cause all sorts of issues so we like to limit this as much as we can! As wasmtime currently isn't running cargo-deny this "bug" might have gone by completely unnoticed. If you did notice but had your own reasons not to "fix" it, that's absolutely fine as well, just wanted to use this specific issue as an example!

Let me know what you guys think! Both the advantages and/or disadvantages of using cargo-deny for wasmtime.

Last updated: Nov 22 2024 at 16:03 UTC