Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use cargo-deny to prevent duplicate dependencies #349

Open
emilk opened this issue Feb 1, 2024 · 5 comments
Open

Use cargo-deny to prevent duplicate dependencies #349

emilk opened this issue Feb 1, 2024 · 5 comments

Comments

@emilk
Copy link
Contributor

emilk commented Feb 1, 2024

By running cargo-deny on CI for all supported platforms, we can ensure users of accesskit won't have duplicated dependencies. Reducing duplicated dependencies reduces compile times and binary bloat.

Here's how it is done in winit, for instance:

https://github.com/rust-windowing/winit/blob/4d4d6e5052877d9578bac73f52eaf323e9089998/.github/workflows/ci.yml#L180-L203

and their deny.toml:

https://github.com/rust-windowing/winit/blob/master/deny.toml

@mwcampbell
Copy link
Contributor

@emilk Thanks for the feedback. We actually already added cargo deny in a recent PR. I guess our deny.toml might need tweaking though. Are you having a problem now with AccessKit introducing duplicate dependencies in egui?

@emilk
Copy link
Contributor Author

emilk commented Feb 1, 2024

Oh sorry, I thought I checked first 🤦

Yeah I got some duplicated deps via accesskit-winit and accesskit-unix… I'll see if I can reproduce

@mwcampbell
Copy link
Contributor

We haven't actually published a release since adding cargo deny.

@emilk
Copy link
Contributor Author

emilk commented Feb 1, 2024

Ah, yes, the deny.toml does not prevent duplicated dependencies. What is needed is:

--- a/deny.toml
+++ b/deny.toml
@@ -34,7 +34,7 @@ exceptions = [
 ]
 
 [bans]
-multiple-versions = "warn"
+multiple-versions = "deny"
 wildcards = "allow"
 highlight = "all"
 workspace-default-features = "allow"

and then duplicates popup fast when checking different platforms with

cargo deny --all-features --log-level error --target aarch64-apple-darwin check
cargo deny --all-features --log-level error --target aarch64-linux-android check
cargo deny --all-features --log-level error --target i686-pc-windows-gnu check
cargo deny --all-features --log-level error --target i686-pc-windows-msvc check
cargo deny --all-features --log-level error --target i686-unknown-linux-gnu check
cargo deny --all-features --log-level error --target wasm32-unknown-unknown check
cargo deny --all-features --log-level error --target x86_64-apple-darwin check
cargo deny --all-features --log-level error --target x86_64-pc-windows-gnu check
cargo deny --all-features --log-level error --target x86_64-pc-windows-msvc check
cargo deny --all-features --log-level error --target x86_64-unknown-linux-gnu check
cargo deny --all-features --log-level error --target x86_64-unknown-linux-musl check
cargo deny --all-features --log-level error --target x86_64-unknown-redox check

Some are small things like raw-window-handle that should just be ignored, but there are also a lot of large crates that are duplicated.

There are for instance multiple version of socket2, polling, event-listener, async-channel, objc2-encode, …

@emilk emilk changed the title Consider Adding cargo-deny to CI to prevent duplicate dependencies Use cargo-denyto prevent duplicate dependencies Feb 1, 2024
@emilk emilk changed the title Use cargo-denyto prevent duplicate dependencies Use cargo-deny to prevent duplicate dependencies Feb 1, 2024
@DataTriny
Copy link
Member

Hello,

I won't comment about deny.toml needing some tweaking at this time, getting rid of the remaining duplicates might not be that easy.

I just wanted to say that I don't think we need to run cargo deny on each platform. My guess is that's because we're checking out Cargo.lock into the repo, so the dependency graph is already there for the tool to explore. Passing different values to the --target option seem to always yield the same results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants