Stream: git-wasmtime

Topic: wasmtime / PR #3490 Remove unused dependencies in Cranelift

view this post on Zulip Wasmtime GitHub notifications bot (Oct 29 2021 at 17:14):

bnjbvr opened PR #3490 from remove-unused-cranelift-deps to main:

An attempt at using a new tool I made that removes spurious dependencies in Cargo.toml in a very naive fashion. Seems that I could still compile with cargo test --all locally with these changes, let's see what CI thinks.

One package is actually removed from the Cargo.lock file, which is surprising. Usually, the spurious dependencies exist because these are former dependencies of the package, but that have moved elsewhere. Removing these dependencies can sometimes help reorder the crate compilation graph, and slightly reduce overall build times on machines with multiple cores.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 29 2021 at 17:16):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 29 2021 at 17:16):

bjorn3 created PR review comment:

Did this and the rand dep removal in https://github.com/bytecodealliance/wasmtime/pull/3478 (approved, but not yet merged) too.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 29 2021 at 17:18):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 29 2021 at 17:18):

bjorn3 created PR review comment:

Is cranelift-serde still declared as dependency of a crate inside the workspace with this change? If not, running cargo build -p cranelift-serde or cd cranelift/serde; cargo build will fail.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 29 2021 at 17:18):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 29 2021 at 17:18):

bjorn3 created PR review comment:

Nice!

view this post on Zulip Wasmtime GitHub notifications bot (Oct 29 2021 at 17:18):

bjorn3 edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 29 2021 at 17:20):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 29 2021 at 17:20):

bjorn3 created PR review comment:

(Sorry @bjorn3, didn't see you open #3478 before)

No problem. There are improvements in this PR I missed in my PR.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 29 2021 at 17:21):

bnjbvr submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 29 2021 at 17:21):

bnjbvr created PR review comment:

Ah right, that explains why it was removed, thanks! And indeed I can reproduce the error now. Would it make sense to manually add it to the workspace entries from the top-level Cargo.toml then, instead of marking it as an unused dependency?

view this post on Zulip Wasmtime GitHub notifications bot (Oct 29 2021 at 17:27):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 29 2021 at 17:27):

bjorn3 created PR review comment:

I guess so.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 02 2021 at 07:58):

bnjbvr updated PR #3490 from remove-unused-cranelift-deps to main.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 02 2021 at 09:08):

bnjbvr updated PR #3490 from remove-unused-cranelift-deps to main.

Last updated: Nov 22 2024 at 17:03 UTC