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

fix(examples): enable mplex feature for libp2p dep #3809

Closed
wants to merge 2 commits into from
Closed

fix(examples): enable mplex feature for libp2p dep #3809

wants to merge 2 commits into from

Conversation

youngjoon-lee
Copy link

@youngjoon-lee youngjoon-lee commented Apr 20, 2023

Description

Some rust-libp2p examples uses development_transport(...) that requires the mplex feature enabled. But, the Cargo.tomls of those examples don't enable the mplex.
This isn't an issue in this repo because those examples refer to the libp2p using path = "../../libp2p", but if you try to rewrite those examples from scratch by referring to libp2p = { version = "...", ...}, the following error will occur:

error[E0432]: unresolved import `libp2p::development_transport`

Notes & open questions

I just enable the mplex feature for those examples.

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

@youngjoon-lee youngjoon-lee marked this pull request as draft April 20, 2023 06:52
@youngjoon-lee youngjoon-lee marked this pull request as ready for review April 20, 2023 06:56
@thomaseizinger
Copy link
Contributor

thomaseizinger commented Apr 20, 2023

mplex is deprecated and thus not active purposely.

If you are following examples, make sure you are looking at the correct source code, i.e. if you depend on 0.51.3, checkout that tag. The examples will work for that version.

@youngjoon-lee youngjoon-lee deleted the mplex-for-examples branch April 20, 2023 07:15
@youngjoon-lee
Copy link
Author

youngjoon-lee commented Apr 20, 2023

mplex is deprecated and thus not active purposely.

If you are following examples, make sure you are looking at the correct source code, i.e. if you depend on 0.51.3, checkout that tag. The examples will work for that version.

Thanks @thomaseizinger . I checked-out the tag libp2p-v0.51.3, but that is still not compiled because of the development_transport (which seems to require mplex).

git checkout libp2p-v0.51.3
cd examples/distributed-key-value-store
cargo build

error[E0432]: unresolved import `libp2p::development_transport`
  --> examples/distributed-key-value-store/src/main.rs:51:5
   |
51 |     development_transport, identity, mdns,
   |     ^^^^^^^^^^^^^^^^^^^^^ no `development_transport` in the root

For more information about this error, try `rustc --explain E0432`.
error: could not compile `distributed-key-value-store` due to previous error

@thomaseizinger
Copy link
Contributor

Thanks for the report! That is strange. Let me investigate.

@thomaseizinger
Copy link
Contributor

It seems that by running a full build on the workspace with --all-features, we mitigate the fact that we have to specify features for our examples. Dang.

Let me see if I can fix this with a new CI job.

@thomaseizinger
Copy link
Contributor

It seems that by running a full build on the workspace with --all-features, we mitigate the fact that we have to specify features for our examples. Dang.

Let me see if I can fix this with a new CI job.

Submitted a PR here: #3811

@youngjoon-lee
Copy link
Author

Submitted a PR here: #3811

Thank you!

mergify bot pushed a commit that referenced this pull request Apr 21, 2023
Due to cargo's feature unification, a full build of our workspace doesn't actually check whether the examples compile as standalone projects.

We add a new CI check that iterates through all crates in the `examples/` directory and runs a plain `cargo check` on them. Any failure is bubbled up via `set -e`, thus failing CI in case one of the `cargo check` commands fails.

To fix the current failures, we construct a simple TCP transport everywhere where we were previously using `development_transport`. That is because `development_transport` requires `mplex` which is now deprecated.

Related #3657.
Related #3809.

Pull-Request: #3811.
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

Successfully merging this pull request may close these issues.

2 participants