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

Metrics dependencies are always pulled via protocol features #2286

Closed
thomaseizinger opened this issue Oct 13, 2021 · 5 comments
Closed

Metrics dependencies are always pulled via protocol features #2286

thomaseizinger opened this issue Oct 13, 2021 · 5 comments
Labels
difficulty:easy getting-started Issues that can be tackled if you don't know the internals of libp2p very well help wanted

Comments

@thomaseizinger
Copy link
Contributor

I am not sure if there is much we can do about it but I noticed that libp2p-metrics is automatically pulled in if any of the protocols that activate metrics features are activated.

When trying to upgrade to the most recent rust-libp2p HEAD, I saw libp2p-metrics come in as a dependency even though we are not specifying it as a feature. I suspect it is being pulled in by cargo due to ping activating libp2p-metrics/ping.

ping = ["libp2p-ping", "libp2p-metrics/ping"]

Is there a way of how we can design this differently? Perhaps having a dedicated metrics feature within each protocol would solve this?

@mxinden
Copy link
Member

mxinden commented Oct 14, 2021

Good catch!

Corresponding upstream tracking issue (weak dependency feature): rust-lang/cargo#8832

As far as I can tell there is no solution to this today.

I don't it hurts much, given that libp2p-metrics is a pretty light-weight dependency. Still it would be great to not pull it in each time e.g. when the ping feature is set.

@thomaseizinger
Copy link
Contributor Author

Thanks for pointing me to that, I had no idea that this is being worked on! Very exciting :)

@mxinden
Copy link
Member

mxinden commented Apr 19, 2022

If I am not mistaken, this can be fixed with Rust 1.60. Anyone interested in sending a patch?

https://blog.rust-lang.org/2022/04/07/Rust-1.60.0.html

@mxinden mxinden added difficulty:easy help wanted getting-started Issues that can be tackled if you don't know the internals of libp2p very well labels Apr 19, 2022
maschad added a commit to maschad/rust-libp2p that referenced this issue May 12, 2022
@maschad
Copy link
Member

maschad commented May 12, 2022

Hey @mxinden I've added a patch in this PR. Lmk what you think.

maschad added a commit to maschad/rust-libp2p that referenced this issue May 17, 2022
maschad added a commit to maschad/rust-libp2p that referenced this issue May 19, 2022
maschad added a commit to maschad/rust-libp2p that referenced this issue May 19, 2022
maschad added a commit to maschad/rust-libp2p that referenced this issue May 19, 2022
maschad added a commit to maschad/rust-libp2p that referenced this issue May 25, 2022
maschad added a commit to maschad/rust-libp2p that referenced this issue May 30, 2022
maschad added a commit to maschad/rust-libp2p that referenced this issue May 31, 2022
maschad added a commit to maschad/rust-libp2p that referenced this issue Jun 2, 2022
maschad added a commit to maschad/rust-libp2p that referenced this issue Jun 3, 2022
@mxinden
Copy link
Member

mxinden commented Jun 10, 2022

Closing here, since fixed by @maschad 🎉

@mxinden mxinden closed this as completed Jun 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty:easy getting-started Issues that can be tackled if you don't know the internals of libp2p very well help wanted
Projects
None yet
Development

No branches or pull requests

3 participants